Skip to contentSkip to conversation

ASoC: Add support for BCM2835

RFC Superseded Florian Meier 2 participants 2 messages Nov 12, 2013 – Nov 16, 2013

Subsystems (5)

v2 of 10 Superseded Latest is v10 →

lore
Share & tools
lore
#0 lore raw

RFC v2 1/1 ASoC: Add support for BCM2835

This driver adds support for digital audio (I2S)
for the BCM2835 SoC that is used by the
Raspberry Pi. External audio codecs can be
connected to the Raspberry Pi via P5 header.

It relies on cyclic DMA engine support for BCM2835.

---

Second version.
- snd_pcm_hardware struct was removed - now calculated by core.
- FIFO address is now calculated from the device tree parameters.
- In a case of over- or underrun, only the associated FIFO is cleared. According to the datasheet this might induce problems with channel swapping, but testing has shown no evidence.
- dai->active is used instead of requesting state from hardware or building up own method.
- Some minor fixes

 arch/arm/boot/dts/bcm2835.dtsi |   12 +
 sound/soc/Kconfig              |    1 +
 sound/soc/Makefile             |    1 +
 sound/soc/bcm/Kconfig          |   15 +
 sound/soc/bcm/Makefile         |    7 +
 sound/soc/bcm/bcm2835-i2s.c    |  892 ++++++++++++++++++++++++++++++++++++++++
 sound/soc/bcm/bcm2835-pcm.c    |   52 +++
 sound/soc/bcm/bcm2835-pcm.h    |   29 ++
 8 files changed, 1009 insertions(+)
 create mode 100644 sound/soc/bcm/Kconfig
 create mode 100644 sound/soc/bcm/Makefile
 create mode 100644 sound/soc/bcm/bcm2835-i2s.c
 create mode 100644 sound/soc/bcm/bcm2835-pcm.c
 create mode 100644 sound/soc/bcm/bcm2835-pcm.h
arch/arm/boot/dts/bcm2835.dtsi +12 -0
index 7389405..facba9e 100644
@@ -126,6 +126,18 @@
 			dma-requests = <32>;
 			dma-channel-mask = <0x7f35>;
 		};
+
+		bcm2835_i2s: i2s at 7e203000 {
+			compatible = "brcm,bcm2835-i2s";
+			reg = <	0x7e203000 0x20
+				0x7e101098 0x02>;
+
+			dmas = <&dma 2
+				&dma 3>;
+			dma-names = "tx", "rx";
+
+			status = "disabled";
+		};
 	};
 
 	clocks {
sound/soc/Kconfig +1 -0
index 5138b84..a5e3a70 100644
@@ -33,6 +33,7 @@ config SND_SOC_GENERIC_DMAENGINE_PCM
 # All the supported SoCs
 source "sound/soc/atmel/Kconfig"
 source "sound/soc/au1x/Kconfig"
+source "sound/soc/bcm/Kconfig"
 source "sound/soc/blackfin/Kconfig"
 source "sound/soc/cirrus/Kconfig"
 source "sound/soc/davinci/Kconfig"
sound/soc/Makefile +1 -0
index 8b9e701..b52d4aa 100644
@@ -10,6 +10,7 @@ obj-$(CONFIG_SND_SOC)	+= codecs/
 obj-$(CONFIG_SND_SOC)	+= generic/
 obj-$(CONFIG_SND_SOC)	+= atmel/
 obj-$(CONFIG_SND_SOC)	+= au1x/
+obj-$(CONFIG_SND_SOC)	+= bcm/
 obj-$(CONFIG_SND_SOC)	+= blackfin/
 obj-$(CONFIG_SND_SOC)	+= cirrus/
 obj-$(CONFIG_SND_SOC)	+= davinci/
sound/soc/bcm/Kconfig new file +15 -0
new file mode 100644
index 0000000..93619ec
@@ -0,0 +1,15 @@
+config SND_BCM2835_SOC_I2S
+	tristate
+
+config SND_BCM2835_SOC
+	tristate "SoC Audio support for the Broadcom BCM2835 I2S module"
+	depends on ARCH_BCM2835
+	select SND_SOC_DMAENGINE_PCM
+	select DMADEVICES
+	select DMA_BCM2835
+	select SND_SOC_GENERIC_DMAENGINE_PCM
+	select REGMAP_MMIO
+	help
+	  Say Y or M if you want to add support for codecs attached to
+	  the BCM2835 I2S interface. You will also need
+	  to select the audio interfaces to support below.
sound/soc/bcm/Makefile new file +7 -0
new file mode 100644
index 0000000..46d8cfb
@@ -0,0 +1,7 @@
+# BCM2835 Platform Support
+snd-soc-bcm2835-objs := bcm2835-pcm.o
+snd-soc-bcm2835-i2s-objs := bcm2835-i2s.o
+
+obj-$(CONFIG_SND_BCM2835_SOC) += snd-soc-bcm2835.o
+obj-$(CONFIG_SND_BCM2835_SOC_I2S) += snd-soc-bcm2835-i2s.o
+
sound/soc/bcm/bcm2835-i2s.c new file +892 -0

892 lines not on this page · raw message

sound/soc/bcm/bcm2835-pcm.c new file +52 -0

52 lines not on this page · raw message

sound/soc/bcm/bcm2835-pcm.h new file +29 -0
new file mode 100644
index 0000000..07a70ca
@@ -0,0 +1,29 @@
+/*
+ * ALSA PCM interface for Broadcom BCM2835 SoC
+ *
+ * Author:      Florian Meier <[email protected]>
+ *              Copyright 2013
+ *
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
+ * 02110-1301 USA
+ *
+ */
+
+#ifndef __BCM2835_PCM_H__
+#define __BCM2835_PCM_H__
+
+int bcm2835_pcm_platform_register(struct device *dev);
+void bcm2835_pcm_platform_unregister(struct device *dev);
+
+#endif
Signature · 1 line
1.7.9.5
#1 lore raw
Quoted 8 lines · Florian Meier

This driver adds support for digital audio (I2S)
for the BCM2835 SoC that is used by the
Raspberry Pi. External audio codecs can be
connected to the Raspberry Pi via P5 header.

It relies on cyclic DMA engine support for BCM2835.

Looks mostly good in my opinion. A few comments on minor bits and pieces.

sound/soc/bcm/Kconfig line 6

Quoted 12 lines
sound/soc/bcm/Kconfig new file +6 -0
new file mode 100644
index 0000000..93619ec
@@ -0,0 +1,15 @@
+config SND_BCM2835_SOC_I2S
+	tristate
+
+config SND_BCM2835_SOC
+	tristate "SoC Audio support for the Broadcom BCM2835 I2S module"
+	depends on ARCH_BCM2835

I think there is no compile time dependency on ARCH_BCM2835. Changing this to 'depends on ARCH_BCM2835 || COMPILE_TEST' allows to archive better build test coverage for the driver.

sound/soc/bcm/Kconfig line 9

Quoted 3 lines
sound/soc/bcm/Kconfig new file +3 -0
+	select SND_SOC_DMAENGINE_PCM
+	select DMADEVICES
+	select DMA_BCM2835

I don't think its a good idea to select DMADEVICES or DMA_BCM2835 here, since those are user selectable symbols from another subsystem. Either 'depends on DMA_BCM2835' or nothing. Will I think nothing is better since there is no compile time dependency.

Quoted 6 lines
sound/soc/bcm/Kconfig new file +6 -0
+	select SND_SOC_GENERIC_DMAENGINE_PCM
+	select REGMAP_MMIO
+	help
+	  Say Y or M if you want to add support for codecs attached to
+	  the BCM2835 I2S interface. You will also need
+	  to select the audio interfaces to support below.

[...]

Quoted 30 lines
sound/soc/bcm/Kconfig new file +30 -0
+static void bcm2835_i2s_clear_fifos(struct bcm2835_i2s_dev *dev,
+				    bool tx, bool rx)
+{
+	int timeout = 1000;
+	uint32_t syncval;
+	uint32_t csreg;
+	uint32_t i2s_active_state;
+	uint32_t clkreg;
+	uint32_t clk_active_state;
+	uint32_t off;
+	uint32_t clr;
+
+	off =  tx ? BCM2835_I2S_TXON : 0;
+	off |= rx ? BCM2835_I2S_RXON : 0;
+
+	clr =  tx ? BCM2835_I2S_TXCLR : 0;
+	clr |= rx ? BCM2835_I2S_RXCLR : 0;
+
+	/* Backup the current state */
+	regmap_read(dev->i2s_regmap, BCM2835_I2S_CS_A_REG, &csreg);
+	i2s_active_state = csreg & (BCM2835_I2S_RXON | BCM2835_I2S_TXON);
+
+	regmap_read(dev->clk_regmap, BCM2835_CLK_PCMCTL_REG, &clkreg);
+	clk_active_state = clkreg & BCM2835_CLK_ENAB;
+
+	/* Start clock if not running */
+	if (!clk_active_state) {
+		regmap_update_bits(dev->clk_regmap, BCM2835_CLK_PCMCTL_REG,
+			BCM2835_CLK_PASSWD_MASK | BCM2835_CLK_ENAB,
+			BCM2835_CLK_PASSWD | BCM2835_CLK_ENAB);

sound/soc/bcm/Kconfig

Quoted 10 lines
sound/soc/bcm/Kconfig new file +10 -0
+	}
+
+	/* Stop I2S module */
+	regmap_update_bits(dev->i2s_regmap, BCM2835_I2S_CS_A_REG, off, 0);
+
+	/*
+	 * Clear the FIFOs
+	 * Requires at least 2 PCM clock cycles to take effect
+	 */
+	regmap_update_bits(dev->i2s_regmap, BCM2835_I2S_CS_A_REG, clr, -1);

I think the -1 can be confusing. Better use either clr or 0xffffffff. Same applies to a few other places in the driver.

Quoted 34 lines
sound/soc/bcm/Kconfig new file +34 -0
+
+	/* Wait for 2 PCM clock cycles */
+
+	/*
+	 * Toggle the SYNC flag - after 2 PCM clock cycles it can be read back
+	 * FIXME: This does not seem to work for slave mode!
+	 */
+	regmap_read(dev->i2s_regmap, BCM2835_I2S_CS_A_REG, &syncval);
+	syncval &= BCM2835_I2S_SYNC;
+
+	regmap_update_bits(dev->i2s_regmap, BCM2835_I2S_CS_A_REG,
+			BCM2835_I2S_SYNC, ~syncval);
+
+	/* Wait for the SYNC flag changing it's state */
+	while (timeout > 0) {
+		timeout--;
+
+		regmap_read(dev->i2s_regmap, BCM2835_I2S_CS_A_REG, &csreg);
+		if ((csreg & BCM2835_I2S_SYNC) != syncval)
+			break;
+	}
+
+	if (timeout <= 0)
+		dev_err(dev->dev, "I2S SYNC error!\n");
+
+	/* Stop clock if it was not running before */
+	if (!clk_active_state)
+		bcm2835_i2s_stop_clock(dev);
+
+	/* Restore I2S state */
+	regmap_update_bits(dev->i2s_regmap, BCM2835_I2S_CS_A_REG,
+			BCM2835_I2S_RXON | BCM2835_I2S_TXON, i2s_active_state);
+}
+

[...]

sound/soc/bcm/Kconfig

Quoted 12 lines
sound/soc/bcm/Kconfig new file +12 -0
+static int bcm2835_i2s_dai_probe(struct snd_soc_dai *dai)
+{
+	struct bcm2835_i2s_dev *dev = snd_soc_dai_get_drvdata(dai);
+
+	dev->dma_data[SNDRV_PCM_STREAM_PLAYBACK].addr_width =
+		DMA_SLAVE_BUSWIDTH_4_BYTES;
+	dev->dma_data[SNDRV_PCM_STREAM_CAPTURE].addr_width =
+		DMA_SLAVE_BUSWIDTH_4_BYTES;
+
+	/* TODO other burst parameters possible? */
+	dev->dma_data[SNDRV_PCM_STREAM_PLAYBACK].maxburst = 2;
+	dev->dma_data[SNDRV_PCM_STREAM_CAPTURE].maxburst = 2;

I'd move the initialization of dma_data to the driver probe() function.

sound/soc/bcm/Kconfig

Quoted 3 lines
sound/soc/bcm/Kconfig new file +3 -0
+
+	dai->playback_dma_data = &dev->dma_data[SNDRV_PCM_STREAM_PLAYBACK];
+	dai->capture_dma_data = &dev->dma_data[SNDRV_PCM_STREAM_CAPTURE];

snd_soc_dai_init_dma_data()

Quoted 4 lines
sound/soc/bcm/Kconfig new file +4 -0
+
+	return 0;
+}
+

[...]

Quoted 24 lines
sound/soc/bcm/Kconfig new file +24 -0
+
+static int bcm2835_i2s_probe(struct platform_device *pdev)
+{
+	struct bcm2835_i2s_dev *dev;
+	int i;
+	int ret;
+	struct regmap *regmap[2];
+	struct resource *mem[2];
+
+	/* request both ioareas */
+	for (i = 0; i <= 1; i++) {
+		void __iomem *base;
+
+		mem[i] = platform_get_resource(pdev, IORESOURCE_MEM, i);
+		if (!mem[i]) {
+			dev_err(&pdev->dev, "I2S probe: Memory resource could not be found.\n");
+			return -ENODEV;
+		}
+
+		base = devm_ioremap_resource(&pdev->dev, mem[i]);
+		if (!base) {
+			dev_err(&pdev->dev, "I2S probe: ioremap failed.\n");
+			return -ENOMEM;
+		}

sound/soc/bcm/Kconfig

Quoted 10 lines
sound/soc/bcm/Kconfig new file +10 -0
+
+		regmap[i] = devm_regmap_init_mmio(&pdev->dev, base,
+					    &bcm2835_regmap_config[i]);
+		if (IS_ERR(regmap[i])) {
+			dev_err(&pdev->dev, "I2S probe: regmap init failed\n");
+			return PTR_ERR(regmap[i]);
+		}
+	}
+
+	dev = devm_kzalloc(&pdev->dev, sizeof(struct bcm2835_i2s_dev),

sizeof(*dev)

Quoted 33 lines
sound/soc/bcm/Kconfig new file +33 -0
+			   GFP_KERNEL);
+	if (!dev) {
+		dev_err(&pdev->dev, "I2S probe: kzalloc failed.\n");
+		return -ENOMEM;
+	}
+
+	dev->i2s_regmap = regmap[0];
+	dev->clk_regmap = regmap[1];
+
+	/* Set the DMA address */
+	dev->dma_data[SNDRV_PCM_STREAM_PLAYBACK].addr =
+		(dma_addr_t)mem[0]->start + BCM2835_I2S_FIFO_A_REG
+					  + BCM2835_VCMMU_SHIFT;
+
+	dev->dma_data[SNDRV_PCM_STREAM_CAPTURE].addr =
+		(dma_addr_t)mem[0]->start + BCM2835_I2S_FIFO_A_REG
+					  + BCM2835_VCMMU_SHIFT;
+
+	/* Store the pdev */
+	dev->dev = &pdev->dev;
+	dev_set_drvdata(&pdev->dev, dev);
+
+	ret = snd_soc_register_component(&pdev->dev,
+			&bcm2835_i2s_component, &bcm2835_i2s_dai, 1);
+
+	if (ret) {
+		dev_err(&pdev->dev, "Could not register DAI: %d\n", ret);
+		ret = -ENOMEM;
+		return ret;
+	}
+
+	ret = bcm2835_pcm_platform_register(&pdev->dev);
+	if (ret) {

sound/soc/bcm/Kconfig

Quoted 10 lines
sound/soc/bcm/Kconfig new file +10 -0
+		dev_err(&pdev->dev, "Could not register PCM: %d\n", ret);
+		snd_soc_unregister_component(&pdev->dev);
+		return ret;
+	}
+
+	return 0;
+}
+
+static const struct of_device_id bcm2835_i2s_of_match[] = {
+	{ .compatible = "brcm,bcm2835-i2s", },

Bindings documentation is missing.

Quoted 2 lines
sound/soc/bcm/Kconfig new file +2 -0
+	{},
+};

[...]

sound/soc/bcm/Kconfig

Quoted 4 lines
sound/soc/bcm/Kconfig new file +4 -0
+
+int bcm2835_pcm_platform_register(struct device *dev)
+{
+	return snd_dmaengine_pcm_register(dev, NULL, 0);

Now that these functions are just simple wrappers for snd_dmaengine_pcm_{register,unregister}() there is really no need to keep them around. Just remove bcm2835-pcm.h and .c and call them directly from the i2s driver.

Quoted 2 lines
sound/soc/bcm/Kconfig new file +2 -0
+}
+EXPORT_SYMBOL_GPL(bcm2835_pcm_platform_register);

[...]