On Mon, Nov 16, 2015 at 01:09:03PM +0200, Peter Ujfalusi wrote:
> f93178291712 dmaengine: bcm2835-dma: Fix memory leak when stopping a
> running transfer
>
> Fixed the memleak, but introduced another issue: the terminate_all callback
> might be called with interrupts disabled and the dma_free_coherent() is
> not allowed to be called when IRQs are disabled.
> Convert the driver to use dma_pool_* for managing the list of control
> blocks for the transfer.
>
> Fixes: f93178291712 ("dmaengine: bcm2835-dma: Fix memory leak when stopping a running transfer")
> Signed-off-by: Peter Ujfalusi <peter.ujfalusi at ti.com>
Tested-by: Matthias Reichl <hias at horus.com>
Thanks a lot, the fix seems to be working fine with mainline 4.4-rc1,
4.4-rc2 and the 4.3 Raspberry Pi tree.
On the RPi tree you can simply enable the rpi-dac overlay for testing,
"dtoverlay=rpi-dac" in config.txt, it'll work without any additional
hardware.
As Peter mentioned I extended his patch to the non-mainlined slave_sg
code in the RPi tree. I've been using this during the last week and
haven't noticed any issues.
For testing with 4.4-rc mainline I used the (quick-and-dirty) code
below. It adds the missing PCM configuration to bcm2835-i2s, adds
some DT fixes (so DT and bcm2835-i2s are similar to the RPi tree)
and installs a card with a dummy-codec. You can then use aplay to
test the DMA code path and don't need any additional hardware.
I've checked the I2S GPIOs on a RPi B+ with my scope, PCM-clock
and frame-clock are fine and pcm-data-out also looks OK.
There's a small gotcha though: bcm2835-i2s hasn't been converted
to the new clock driver yet and also can't coexist with it
(overlapping iomem regions...). Fortunately, this can be solved
by disabling the new clock driver in the devicetree.
Here are the steps I used for testing:
- revert commits 121432c7a02f and 94cb7f76caa0 to disable the
new clock driver in DT
- apply the test-code patch below
- make bcm2835_defconfig
- enable bcm2835-dma and bcm2835-i2s in kernel config:
CONFIG_SOUND=y
CONFIG_SND=y
CONFIG_SND_SOC=y
CONFIG_SND_BCM2835_SOC_I2S=y
CONFIG_DMADEVICES=y
CONFIG_DMA_BCM2835=y
- (optionally) apply Peter's DMA patch
- on the RPi use aplay to play a 44.1kHz 16-bit stereo WAV
(32bit and other sample rates will work, too)
so long,
Hias
---
diff --git a/arch/arm/boot/dts/bcm2835.dtsi b/arch/arm/boot/dts/bcm2835.dtsi
index 2029394..c3a1cfc 100644
--- a/arch/arm/boot/dts/bcm2835.dtsi
+++ b/arch/arm/boot/dts/bcm2835.dtsi
@@ -102,13 +102,13 @@
i2s: i2s at 7e203000 {
compatible = "brcm,bcm2835-i2s";
- reg = <0x7e203000 0x20>,
- <0x7e101098 0x02>;
+ reg = <0x7e203000 0x24>,
+ <0x7e101098 0x08>;
dmas = <&dma 2>,
<&dma 3>;
dma-names = "tx", "rx";
- status = "disabled";
+ status = "okay";
};
spi: spi at 7e204000 {
@@ -189,4 +189,9 @@
clock-frequency = <250000000>;
};
};
+
+ sound {
+ compatible = "brcm,i2s-dummy-card";
+ i2s-controller = <&i2s>;
+ };
};
diff --git a/sound/soc/bcm/Makefile b/sound/soc/bcm/Makefile
index bc816b7..d599b62 100644
--- a/sound/soc/bcm/Makefile
+++ b/sound/soc/bcm/Makefile
@@ -1,5 +1,7 @@
# BCM2835 Platform Support
snd-soc-bcm2835-i2s-objs := bcm2835-i2s.o
+snd-soc-i2s-dummy-card-objs := i2s-dummy-card.o
obj-$(CONFIG_SND_BCM2835_SOC_I2S) += snd-soc-bcm2835-i2s.o
+obj-$(CONFIG_SND_BCM2835_SOC_I2S) += snd-soc-i2s-dummy-card.o
diff --git a/sound/soc/bcm/bcm2835-i2s.c b/sound/soc/bcm/bcm2835-i2s.c
index 8c435be..b1ff172 100644
--- a/sound/soc/bcm/bcm2835-i2s.c
+++ b/sound/soc/bcm/bcm2835-i2s.c
@@ -784,6 +784,24 @@ static const struct snd_soc_component_driver bcm2835_i2s_component = {
.name = "bcm2835-i2s-comp",
};
+static const struct snd_pcm_hardware bcm2835_pcm_hardware = {
+ .info = SNDRV_PCM_INFO_INTERLEAVED |
+ SNDRV_PCM_INFO_JOINT_DUPLEX,
+ .formats = SNDRV_PCM_FMTBIT_S16_LE |
+ SNDRV_PCM_FMTBIT_S32_LE,
+ .period_bytes_min = 32,
+ .period_bytes_max = SZ_64K - 4,
+ .periods_min = 2,
+ .periods_max = 255,
+ .buffer_bytes_max = SZ_512K
+};
+
+static const struct snd_dmaengine_pcm_config bcm2835_dmaengine_pcm_config = {
+ .pcm_hardware = &bcm2835_pcm_hardware,
+ .prepare_slave_config = snd_dmaengine_pcm_prepare_slave_config,
+ .prealloc_buffer_size = SZ_1M,
+};
+
static int bcm2835_i2s_probe(struct platform_device *pdev)
{
struct bcm2835_i2s_dev *dev;
@@ -848,7 +866,9 @@ static int bcm2835_i2s_probe(struct platform_device *pdev)
return ret;
}
- ret = devm_snd_dmaengine_pcm_register(&pdev->dev, NULL, 0);
+ ret = devm_snd_dmaengine_pcm_register(&pdev->dev,
+ &bcm2835_dmaengine_pcm_config,
+ SND_DMAENGINE_PCM_FLAG_COMPAT);
if (ret) {
dev_err(&pdev->dev, "Could not register PCM: %d\n", ret);
return ret;
diff --git a/sound/soc/bcm/i2s-dummy-card.c b/sound/soc/bcm/i2s-dummy-card.c
new file mode 100644
index 0000000..b3ad842
--- /dev/null
+++ b/sound/soc/bcm/i2s-dummy-card.c
@@ -0,0 +1,87 @@
+/*
+ * Dummy card driver for testing bcm2835-i2s
+ *
+ * Author: Matthias Reichl <hias at horus.com>
+ * Copyright 2015
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ */
+
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+
+#include <sound/core.h>
+#include <sound/soc.h>
+
+static struct snd_soc_dai_link snd_i2s_dummy_card_dai[] = {
+{
+ .name = "i2s-dummy",
+ .stream_name = "i2s-dummy HiFi",
+ .codec_name = "snd-soc-dummy",
+ .codec_dai_name = "snd-soc-dummy-dai",
+ .dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF |
+ SND_SOC_DAIFMT_CBS_CFS,
+},
+};
+
+static struct snd_soc_card snd_i2s_dummy_card = {
+ .name = "i2s_dummy_card",
+ .dai_link = snd_i2s_dummy_card_dai,
+ .num_links = ARRAY_SIZE(snd_i2s_dummy_card_dai),
+};
+
+static int snd_i2s_dummy_card_probe(struct platform_device *pdev)
+{
+ int ret = 0;
+ struct device_node *i2s_node;
+
+ if (!pdev->dev.of_node) {
+ dev_err(&pdev->dev, "no of_node, exiting\n");
+ return -ENODEV;
+ }
+
+ i2s_node = of_parse_phandle(pdev->dev.of_node, "i2s-controller", 0);
+
+ if (!i2s_node) {
+ dev_err(&pdev->dev, "error getting i2s-controller, exiting\n");
+ return -ENODEV;
+ }
+
+ snd_i2s_dummy_card.dev = &pdev->dev;
+ snd_i2s_dummy_card_dai[0].cpu_of_node = i2s_node;
+ snd_i2s_dummy_card_dai[0].platform_of_node = i2s_node;
+
+ ret = devm_snd_soc_register_card(&pdev->dev, &snd_i2s_dummy_card);
+ if (ret)
+ dev_err(&pdev->dev, "snd_soc_register_card() failed: %d\n", ret);
+
+ return ret;
+}
+
+static const struct of_device_id snd_i2s_dummy_card_of_match[] = {
+ { .compatible = "brcm,i2s-dummy-card", },
+ {},
+};
+MODULE_DEVICE_TABLE(of, snd_i2s_dummy_card_of_match);
+
+static struct platform_driver snd_i2s_dummy_card_driver = {
+ .driver = {
+ .name = "snd-i2s-dummy-card",
+ .of_match_table = snd_i2s_dummy_card_of_match,
+ },
+ .probe = snd_i2s_dummy_card_probe,
+};
+
+module_platform_driver(snd_i2s_dummy_card_driver);
+
+MODULE_AUTHOR("Matthias Reichl <hias at horus.com>");
+MODULE_DESCRIPTION("ASoC dummy card driver for testing bcm2835-i2s");
+MODULE_LICENSE("GPL v2");