ASoC: Add support for BCM2708
PATCH 1/1 ASoC: Add support for BCM2708
This driver adds support for digital audio (I2S)
for the BCM2708 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 BCM2708 that is not included in this patch.
The currently supported audio codecs are
TLV320AIC23 and WM8731.
---
A Raspberry Pi kernel featuring this driver can be found at
https://github.com/koalo/linux rpi-3.8.y-asocdev
You can find more information about how to use it at
http://blog.koalo.de/2013/05/i2s-support-for-raspberry-pi.html
sound/soc/Kconfig | 1 + sound/soc/Makefile | 1 + sound/soc/bcm2708/Kconfig | 29 ++ sound/soc/bcm2708/Makefile | 12 + sound/soc/bcm2708/bcm2708-i2s.c | 964 +++++++++++++++++++++++++++++++++++++++ sound/soc/bcm2708/bcm2708-pcm.c | 303 ++++++++++++ sound/soc/bcm2708/bcm2708-pcm.h | 40 ++ sound/soc/bcm2708/rpi-mbed.c | 113 +++++ sound/soc/bcm2708/rpi-proto.c | 131 ++++++ 9 files changed, 1594 insertions(+) create mode 100644 sound/soc/bcm2708/Kconfig create mode 100644 sound/soc/bcm2708/Makefile create mode 100644 sound/soc/bcm2708/bcm2708-i2s.c create mode 100644 sound/soc/bcm2708/bcm2708-pcm.c create mode 100644 sound/soc/bcm2708/bcm2708-pcm.h create mode 100644 sound/soc/bcm2708/rpi-mbed.c create mode 100644 sound/soc/bcm2708/rpi-proto.c
sound/soc/Kconfig +1 -0
@@ -36,6 +36,7 @@ config SND_SOC_GENERIC_DMAENGINE_PCM # All the supported SoCs source "sound/soc/atmel/Kconfig" source "sound/soc/au1x/Kconfig" +source "sound/soc/bcm2708/Kconfig" source "sound/soc/blackfin/Kconfig" source "sound/soc/cirrus/Kconfig" source "sound/soc/davinci/Kconfig"
sound/soc/Makefile +1 -0
@@ -14,6 +14,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) += bcm2708/ obj-$(CONFIG_SND_SOC) += blackfin/ obj-$(CONFIG_SND_SOC) += cirrus/ obj-$(CONFIG_SND_SOC) += davinci/
sound/soc/bcm2708/Kconfig new file +29 -0
@@ -0,0 +1,29 @@ +config SND_BCM2708_SOC_I2S + tristate + +config SND_BCM2708_SOC + tristate "SoC Audio support for the Broadcom BCM2708 I2S module" + depends on MACH_BCM2708 + select SND_SOC_DMAENGINE_PCM + select DMA_BCM2708 + select SND_SOC_GENERIC_DMAENGINE_PCM + help + Say Y or M if you want to add support for codecs attached to + the BCM2708 I2S interface. You will also need + to select the audio interfaces to support below. + +config SND_BCM2708_SOC_RPI_CODEC_MBED + tristate "Support for AudioCODEC for mbed (TLV320AIC32B)" + depends on SND_BCM2708_SOC + select SND_BCM2708_SOC_I2S + select SND_SOC_TLV320AIC3X + help + Say Y if you want to add support for AudioCODEC for mbed (TLV320AIC32B) + +config SND_BCM2708_SOC_RPI_CODEC_PROTO + tristate "Support for Audio Codec Board - PROTO (WM8731)" + depends on SND_BCM2708_SOC + select SND_BCM2708_SOC_I2S + select SND_SOC_WM8731 + help + Say Y if you want to add support for Audio Codec Board - PROTO (WM8731)
sound/soc/bcm2708/Makefile new file +12 -0
@@ -0,0 +1,12 @@ +# BCM2708 Platform Support +snd-soc-bcm2708-objs := bcm2708-pcm.o +snd-soc-bcm2708-i2s-objs := bcm2708-i2s.o +snd-soc-rpi-mbed-objs := rpi-mbed.o +snd-soc-rpi-proto-objs := rpi-proto.o + +obj-$(CONFIG_SND_BCM2708_SOC) += snd-soc-bcm2708.o +obj-$(CONFIG_SND_BCM2708_SOC_I2S) += snd-soc-bcm2708-i2s.o + +# BCM2708 Machine Support +obj-$(CONFIG_SND_BCM2708_SOC_RPI_CODEC_MBED) += snd-soc-rpi-mbed.o +obj-$(CONFIG_SND_BCM2708_SOC_RPI_CODEC_PROTO) += snd-soc-rpi-proto.o
sound/soc/bcm2708/bcm2708-i2s.c new file +964 -0
964 lines not on this page · raw message
sound/soc/bcm2708/bcm2708-pcm.c new file +303 -0
303 lines not on this page · raw message
sound/soc/bcm2708/bcm2708-pcm.h new file +40 -0
@@ -0,0 +1,40 @@ +/* +* ALSA PCM interface for Broadcom BCM2708 SoC +* +* Author: Florian Meier, <[email protected]> +* Copyright 2013 +* +* based on +* ALSA PCM interface for the OMAP SoC +* Copyright (C) 2008 Nokia Corporation +* Contact: Jarkko Nikula <[email protected]> +* Peter Ujfalusi <[email protected]> +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of the GNU General Public License as published by +* the Free Software Foundation; either version 2 of the License, or +* (at your option) any later version. +* +* 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., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA +*/ + +#ifndef __BCM2708_PCM_H__ +#define __BCM2708_PCM_H__ + +struct snd_pcm_substream ; + +struct bcm2708_pcm_dma_data { +char *name;/* stream identifier */ +int dma_req;/* DMA request line */ +unsigned long port_addr;/* transmit/receive register */ +int packet_size; +}; + +#endif
sound/soc/bcm2708/rpi-mbed.c new file +113 -0
113 lines not on this page · raw message
sound/soc/bcm2708/rpi-proto.c new file +131 -0
131 lines not on this page · raw message
Signature · 1 line
1.7.9.5
Quoted 7 lines · Florian Meier
This driver adds support for digital audio (I2S) for the BCM2708 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 BCM2708 that is not included in this patch.
This SoC is actually the BCM2835, so the files should be named for that.
Upstream only uses device tree for BCM2835. Does this driver support DT? Where is the DT binding definition file?
I assume you're planning on sending the DMA driver upstream too? I guess this driver isn't useful without it.
I'll take a look at the actual patch content later.
Quoted 10 lines · Stephen Warren
Quoted 7 lines · Florian Meier
This driver adds support for digital audio (I2S) for the BCM2708 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 BCM2708 that is not included in this patch.
This SoC is actually the BCM2835, so the files should be named for that.
Actually, I changed the name several times and in the end I sticked to
https://github.com/raspberrypi/linux/issues/22
but if you think it is better to use BCM2835 I am happy to agree with you and change it.
Quoted 2 lines
Upstream only uses device tree for BCM2835. Does this driver support DT? Where is the DT binding definition file?
I have not accomplished to use the mainline kernel and DT with the Raspberry Pi. Do you have any introductive information about how to do this?
Quoted 2 lines
I assume you're planning on sending the DMA driver upstream too? I guess this driver isn't useful without it.
I would like to, but it only supports cyclic DMA. Should I send it upstream anyway? In case you are interested:
https://github.com/koalo/linux/blob/rpi-3.8.y-asocdev/drivers/dma/bcm2708-dmaengine.c
Quoted 1 line
I'll take a look at the actual patch content later.
That would be great!
Quoted 17 lines · Florian Meier
Quoted 10 lines · Stephen Warren
Quoted 7 lines · Florian Meier
This driver adds support for digital audio (I2S) for the BCM2708 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 BCM2708 that is not included in this patch.
This SoC is actually the BCM2835, so the files should be named for that.
Actually, I changed the name several times and in the end I sticked to
https://github.com/raspberrypi/linux/issues/22
but if you think it is better to use BCM2835 I am happy to agree with you and change it.
Quoted 5 lines
Quoted 2 lines
Upstream only uses device tree for BCM2835. Does this driver support DT? Where is the DT binding definition file?
I have not accomplished to use the mainline kernel and DT with the Raspberry Pi. Do you have any introductive information about how to do this?
Oh, so is this patch intended for the downstream Raspberry Pi Foundation kernel, or the upstream kernel?
The fact you haven't run it on the upstream kernel implies that you intend the patch to be applied to the downstream kernel.
The fact you sent the driver to all the upstream mailing lists implies that you intend the patch to be applied to the upstream kernel.
BTW, I wrote a Google+ post on using the upstream kernel on the Pi:
https://plus.google.com/101715070573995136397/posts/gWkwrfNfYVm
Quoted 5 lines
Quoted 2 lines
I assume you're planning on sending the DMA driver upstream too? I guess this driver isn't useful without it.
I would like to, but it only supports cyclic DMA. Should I send it upstream anyway? In case you are interested:
I don't think that limitation would stop it being useful upstream; somebody else might always enhance it.
Quoted 27 lines · Stephen Warren
Quoted 17 lines · Florian Meier
Quoted 10 lines · Stephen Warren
Quoted 7 lines · Florian Meier
This driver adds support for digital audio (I2S) for the BCM2708 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 BCM2708 that is not included in this patch.
This SoC is actually the BCM2835, so the files should be named for that.
Actually, I changed the name several times and in the end I sticked to
https://github.com/raspberrypi/linux/issues/22
but if you think it is better to use BCM2835 I am happy to agree with you and change it.Quoted 5 lines
Quoted 2 lines
Upstream only uses device tree for BCM2835. Does this driver support DT? Where is the DT binding definition file?
I have not accomplished to use the mainline kernel and DT with the Raspberry Pi. Do you have any introductive information about how to do this?
Oh, so is this patch intended for the downstream Raspberry Pi Foundation kernel, or the upstream kernel?
Quoted 8 lines
The fact you haven't run it on the upstream kernel implies that you intend the patch to be applied to the downstream kernel.
The fact you sent the driver to all the upstream mailing lists implies that you intend the patch to be applied to the upstream kernel.
BTW, I wrote a Google+ post on using the upstream kernel on the Pi:
https://plus.google.com/101715070573995136397/posts/gWkwrfNfYVm
I chatted with Mark Brown about this issue and if I understood him correctly he suggested to send it upstream anyway.
Either way, your blog post gives me great hope to get this running tomorrow so this won't be an issue anymore.
Mark Brown
maintainerQuoted 7 lines · Florian Meier
Quoted 2 lines · Stephen Warren
I assume you're planning on sending the DMA driver upstream too? I guess this driver isn't useful without it.
I would like to, but it only supports cyclic DMA. Should I send it upstream anyway? In case you are interested:
It seems better to send something partially featured upstream so that
people can work on enhancing it rather than having nothing all upstream.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130522/d5bbd7bc/attachment.sig>
Mark Brown
maintainerQuoted 4 lines · Florian Meier
This driver adds support for digital audio (I2S) for the BCM2708 SoC that is used by the Raspberry Pi. External audio codecs can be connected to the Raspberry Pi via P5 header.
Split this up into a patch series, for example one per CPU side driver and one per machine driver. I've given the code a relatively quick run through here, it looks mostly sensible though DT would be nice but there's a few comments.
sound/soc/bcm2708/bcm2708-i2s.c line 187
Quoted 7 lines
sound/soc/bcm2708/bcm2708-i2s.c new file +7
-0+static inline void bcm2708_i2s_write_reg (struct bcm2708_i2s_dev *dev, +int reg,u32 val) +{ +dev_dbg (dev->dev,"I2S write to register %p = %x\n" , + dev->clk_base + reg, val); +__raw_writel (val, dev->i2s_base + reg); +}
This all looks like you want to use regmap-mmio - that'll get you all the logging and so on for free too.
sound/soc/bcm2708/bcm2708-i2s.c line 228
Quoted 8 lines
sound/soc/bcm2708/bcm2708-i2s.c new file +8
-0+static void bcm2708_i2s_start_clock (struct bcm2708_i2s_dev *dev) +{ +/* +* Start the clock if in master mode. +*/ +unsigned int master = dev->fmt &SND_SOC_DAIFMT_MASTER_MASK ; +if (master ==SND_SOC_DAIFMT_CBS_CFS + || master ==SND_SOC_DAIFMT_CBS_CFM ) {
This looks like it should be a switch statement.
sound/soc/bcm2708/bcm2708-i2s.c line 233
Quoted 5 lines
sound/soc/bcm2708/bcm2708-i2s.c new file +5
-0+unsigned int clkreg =bcm2708_clk_read_reg (dev, +BCM2708_CLK_PCMCTL_REG ); +bcm2708_clk_write_reg (dev,BCM2708_CLK_PCMCTL_REG , +BCM2708_CLK_PASSWD | clkreg |BCM2708_CLK_ENAB ); + }
Shouldn't this be using your set bits operation?
Quoted 7 lines
sound/soc/bcm2708/bcm2708-i2s.c new file +7
-0+static bool bcm2708_i2s_check_other_stream (struct bcm2708_i2s_dev *dev, +struct snd_pcm_substream *substream, +struct snd_pcm_substream *other_stream, +struct snd_pcm_hw_params *params) +{ +if (other_stream->runtime->format && + (other_stream->runtime->format !=params_format (params))) {
sound/soc/bcm2708/bcm2708-i2s.c line 389
Quoted 10 lines
sound/soc/bcm2708/bcm2708-i2s.c new file +10
-0+dev_err (dev->dev, +"Sample formats of streams are different. %i (%s) != %i (%s) Initialization failed!\n" , + other_stream->runtime->format, + (other_stream->stream == +SNDRV_PCM_STREAM_PLAYBACK ? +"playback" :"capture" ), +params_format (params), + (substream->stream == +SNDRV_PCM_STREAM_PLAYBACK ? +"playback" :"capture" ));
This is illegible.
sound/soc/bcm2708/bcm2708-i2s.c line 433
Quoted 4 lines
sound/soc/bcm2708/bcm2708-i2s.c new file +4
-0+/* Ensure, that both streams have the same settings */ +struct snd_pcm_substream *other_stream = dev->first_stream; +if (other_stream == substream) + other_stream = dev->second_stream;
Set constraints for this so that the upper layers will stop mismatched settings being configured.
sound/soc/bcm2708/bcm2708-i2s.c line 685
Quoted 6 lines
sound/soc/bcm2708/bcm2708-i2s.c new file +6
-0+/* +* Clear both FIFOs if the one that should be started +* is not empty at the moment. This should only happen +* after overrun. Otherwise, hw_params would have cleared +* the FIFO. +*/
Why both?
sound/soc/bcm2708/bcm2708-i2s.c line 711
Quoted 4 lines
sound/soc/bcm2708/bcm2708-i2s.c new file +4
-0+case SNDRV_PCM_TRIGGER_START : +case SNDRV_PCM_TRIGGER_RESUME : +case SNDRV_PCM_TRIGGER_PAUSE_RELEASE : +bcm2708_i2s_start (dev, substream);
These functions are pretty small, I'd just inline them.
Quoted 14 lines
sound/soc/bcm2708/bcm2708-i2s.c new file +14
-0+/* +* This is the second stream open, so we need to impose +* sample size and sampling rate constraints. +* This is because frame length and clock cannot be specified +* seperately. +* +* Note that this can cause a race condition if the +* second stream is opened before the first stream is +* fully initialized. We provide some protection by +* checking to make sure the first stream is +* initialized, but it's not perfect. ALSA sometimes +* re-initializes the driver with a different sample +* rate or size. If the second stream is opened +* before the first stream has received its final
sound/soc/bcm2708/bcm2708-i2s.c line 772
Quoted 10 lines
sound/soc/bcm2708/bcm2708-i2s.c new file +10
-0+* parameters, then the second stream may be +* constrained to the wrong sample rate or size. +* +* We will continue in case of failure and recheck the +* constraint in hw_params. +*/ +if (!first_runtime->format) { + dev_err(substream->pcm->card->dev, + "Set format in %s stream first! " + "Initialization may fail.\n",
Don't split strings, but in any case you should just let people set up - hopefully they'll try to set something that might work anyway. Any code for fixing this up should be in the core, like symmetric_rates.
sound/soc/bcm2708/bcm2708-i2s.c line 867
Quoted 8 lines
sound/soc/bcm2708/bcm2708-i2s.c new file +8
-0+static void bcm2708_i2s_setup_gpio (void ) +{ +/* +* This is the common way to handle the GPIO pins for +* the Raspberry Pi. +* TODO Better way would be to handle +* this in the device tree! +*/
Yup.
sound/soc/bcm2708/bcm2708-i2s.c line 894
Quoted 5 lines
sound/soc/bcm2708/bcm2708-i2s.c new file +5
-0+/* request both ioareas */ +for (i =0 ; i <=1 ; i++) { +struct resource *mem, *ioarea; + mem =platform_get_resource (pdev,IORESOURCE_MEM , i); +if (!mem) {
devm_ioremap_resource().
sound/soc/bcm2708/bcm2708-pcm.c line 79
Quoted 9 lines
sound/soc/bcm2708/bcm2708-pcm.c new file +9
-0+/* get DMA data (e.g. FIFO address and DREQ) */ + dma_data =snd_soc_dai_get_dma_data (rtd->cpu_dai, substream); + +/* +* There seems to be no use for bufferless +* transfer with this SoC. +*/ +if (!dma_data) +return 0 ;
This is now supported properly by the framework, no need for this.
Quoted 8 lines
sound/soc/bcm2708/bcm2708-pcm.c new file +8
-0+ dma_data =snd_soc_dai_get_dma_data (rtd->cpu_dai, substream); + +switch (cmd) { +case SNDRV_PCM_TRIGGER_START : +case SNDRV_PCM_TRIGGER_RESUME : +case SNDRV_PCM_TRIGGER_PAUSE_RELEASE : +break ; +
sound/soc/bcm2708/bcm2708-pcm.c line 137
Quoted 10 lines
sound/soc/bcm2708/bcm2708-pcm.c new file +10
-0+case SNDRV_PCM_TRIGGER_STOP : +case SNDRV_PCM_TRIGGER_SUSPEND : +case SNDRV_PCM_TRIGGER_PAUSE_PUSH : +break ; +default : + ret = -EINVAL ; + } + +if (ret ==0 ) + ret =snd_dmaengine_pcm_trigger (substream, cmd);
Juse don't have switch statement.
sound/soc/bcm2708/bcm2708-pcm.c line 146
Quoted 5 lines
sound/soc/bcm2708/bcm2708-pcm.c new file +5
-0+static snd_pcm_uframes_t bcm2708_pcm_pointer ( +struct snd_pcm_substream *substream) +{ +return snd_dmaengine_pcm_pointer (substream); +}
Should just be able to use snd_dmaengine_pcm_pointer() as the ops.
sound/soc/bcm2708/bcm2708-pcm.c line 174
Quoted 2 lines
sound/soc/bcm2708/bcm2708-pcm.c new file +2
-0+static int bcm2708_pcm_mmap (struct snd_pcm_substream *substream, +struct vm_area_struct *vma)
This should be a generic op...
sound/soc/bcm2708/bcm2708-pcm.c line 178
Quoted 4 lines
sound/soc/bcm2708/bcm2708-pcm.c new file +4
-0+{ +struct snd_pcm_runtime *runtime = substream->runtime; + + return dma_mmap_writecombine(substream->pcm->card->dev, vma,
Should be using the DMA controller device not the card.
Quoted 4 lines
sound/soc/bcm2708/bcm2708-pcm.c new file +4
-0+ runtime->dma_area, + runtime->dma_addr, + runtime->dma_bytes); +}
sound/soc/bcm2708/bcm2708-pcm.c line 205
Quoted 10 lines
sound/soc/bcm2708/bcm2708-pcm.c new file +9
-0+static int bcm2708_pcm_preallocate_dma_buffer (struct snd_pcm *pcm, +int stream) +{ +struct snd_pcm_substream *substream = pcm->streams[stream].substream; +struct snd_dma_buffer *buf = &substream->dma_buffer; +size_t size = bcm2708_pcm_hardware.buffer_bytes_max; + + buf->dev.type =SNDRV_DMA_TYPE_DEV ; + buf->dev.dev = pcm->card->dev;
DMA controller device.
sound/soc/bcm2708/rpi-mbed.c line 36
Quoted 4 lines
sound/soc/bcm2708/rpi-mbed.c new file +4
-0+static int snd_rpi_mbed_init (struct snd_soc_pcm_runtime *rtd) +{ +return 0 ; +}
Empty functions can just be removed.
Quoted 7 lines
sound/soc/bcm2708/rpi-proto.c new file +7
-0+static const unsigned int wm8731_rates_12288000[] = { +8000 ,32000 ,48000 ,96000 , +}; + +static struct snd_pcm_hw_constraint_list wm8731_constraints_12288000 = { + .list = wm8731_rates_12288000, + .count =ARRAY_SIZE (wm8731_rates_12288000),
sound/soc/bcm2708/rpi-proto.c line 49
Quoted 10 lines
sound/soc/bcm2708/rpi-proto.c new file +10
-0+}; + +static int snd_rpi_proto_startup (struct snd_pcm_substream *substream) +{ +/* Setup constraints, because there is a 12.288 MHz XTAL on the board */ +snd_pcm_hw_constraint_list (substream->runtime,0 , +SNDRV_PCM_HW_PARAM_RATE , + &wm8731_constraints_12288000); +return 0 ; +}
Push this into the CODEC driver, this will be true for the device on any platform with a fixed clock.
Quoted 5 lines
sound/soc/bcm2708/rpi-proto.c new file +5
-0+static int snd_rpi_proto_hw_params (struct snd_pcm_substream *substream, +struct snd_pcm_hw_params *params) +{ +struct snd_soc_pcm_runtime *rtd = substream->private_data; +struct snd_soc_dai *codec_dai = rtd->codec_dai;
sound/soc/bcm2708/rpi-proto.c line 65
Quoted 10 lines
sound/soc/bcm2708/rpi-proto.c new file +10
-0+int sysclk =12288000 ;/* This is fixed on this board */ + +/* Set proto sysclk */ +int ret =snd_soc_dai_set_sysclk (codec_dai,WM8731_SYSCLK_XTAL , + sysclk,SND_SOC_CLOCK_IN ); +if (ret <0 ) { +dev_err (substream->pcm->dev, +"Failed to set WM8731 SYSCLK: %d\n" , ret); +return ret; + }
Since this is fixed just do it once on init (eg, in late_probe()) rather
than every single time.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130522/feeabfa5/attachment.sig>
Quoted 12 lines · Mark Brown
Quoted 4 lines · Florian Meier
This driver adds support for digital audio (I2S) for the BCM2708 SoC that is used by the Raspberry Pi. External audio codecs can be connected to the Raspberry Pi via P5 header.
Split this up into a patch series, for example one per CPU side driver and one per machine driver. I've given the code a relatively quick run through here, it looks mostly sensible though DT would be nice but there's a few comments.
Quoted 9 lines
sound/soc/bcm2708/bcm2708-i2s.c line 187
Quoted 7 lines
sound/soc/bcm2708/bcm2708-i2s.c new file +7
-0+static inline void bcm2708_i2s_write_reg (struct bcm2708_i2s_dev *dev, +int reg,u32 val) +{ +dev_dbg (dev->dev,"I2S write to register %p = %x\n" , + dev->clk_base + reg, val); +__raw_writel (val, dev->i2s_base + reg); +}This all looks like you want to use regmap-mmio.........
Thank you for your comments! I will have a look at them and send a new patch series taking them into account.
Before that I will create a patch for the DMA engine driver.
On 05/22/2013 04:10 PM, Florian Meier wrote:
[...]
Quoted 6 lines
sound/soc/bcm2708/bcm2708-i2s.c new file +0
-0@@ -0,0 +1,964 @@
[...]
Quoted 61 lines
sound/soc/bcm2708/bcm2708-i2s.c new file +61
-0+static int bcm2708_i2s_startup (struct snd_pcm_substream *substream, +struct snd_soc_dai *dai) +{ +struct bcm2708_i2s_dev *dev =snd_soc_dai_get_drvdata (dai); + +if (!dev->first_stream) { + dev->first_stream = substream; + +/* should this still be running stop it */ +bcm2708_i2s_stop_clock (dev); + +/* enable PCM block */ +bcm2708_i2s_set_bits (dev,BCM2708_I2S_CS_A_REG ,BCM2708_I2S_EN ); + +/* +* Disable STBY +* Requires at least 4 PCM clock cycles to take effect +*/ +bcm2708_i2s_set_bits (dev,BCM2708_I2S_CS_A_REG , +BCM2708_I2S_STBY ); + }else { +struct snd_pcm_runtime *first_runtime = + dev->first_stream->runtime; + +/* +* This is the second stream open, so we need to impose +* sample size and sampling rate constraints. +* This is because frame length and clock cannot be specified +* seperately. +* +* Note that this can cause a race condition if the +* second stream is opened before the first stream is +* fully initialized. We provide some protection by +* checking to make sure the first stream is +* initialized, but it's not perfect. ALSA sometimes +* re-initializes the driver with a different sample +* rate or size. If the second stream is opened +* before the first stream has received its final +* parameters, then the second stream may be +* constrained to the wrong sample rate or size. +* +* We will continue in case of failure and recheck the +* constraint in hw_params. +*/ +if (!first_runtime->format) { +dev_err (substream->pcm->card->dev, +"Set format in %s stream first! " +"Initialization may fail.\n" , + substream->stream == +SNDRV_PCM_STREAM_PLAYBACK + ?"capture" :"playback" ); + }else { +snd_pcm_hw_constraint_minmax (substream->runtime, +SNDRV_PCM_HW_PARAM_FORMAT , + first_runtime->format, + first_runtime->format); + } + +if (!first_runtime->rate) { +dev_err (substream->pcm->card->dev, + "Set sampling rate in %s stream first! "
sound/soc/bcm2708/bcm2708-i2s.c line 795
Quoted 10 lines
sound/soc/bcm2708/bcm2708-i2s.c new file +10
-0+"Initialization may fail!\n" , + substream->stream == +SNDRV_PCM_STREAM_PLAYBACK + ? "capture" : "playback"); + }else { +snd_pcm_hw_constraint_minmax (substream->runtime, +SNDRV_PCM_HW_PARAM_RATE , + first_runtime->rate, + first_runtime->rate); + }
Setting the symmetric_rates lets the core take care of setting the proper constraints on the rate. It probably makes sense to also add a symmetric_formats since this isn't the only driver that has the requirement that the formats for playback and capture match.
Quoted 35 lines
sound/soc/bcm2708/bcm2708-i2s.c new file +35
-0+ + dev->second_stream = substream; + } + +snd_soc_dai_set_dma_data (dai, substream, + &dev->dma_params[substream->stream]); + +return 0 ; +} + +static void bcm2708_i2s_shutdown (struct snd_pcm_substream *substream, +struct snd_soc_dai *dai) +{ +struct bcm2708_i2s_dev *dev =snd_soc_dai_get_drvdata (dai); + +bcm2708_i2s_stop (dev, substream); + +if (dev->first_stream == substream) + dev->first_stream = dev->second_stream; + + dev->second_stream =NULL ; + +/* If both streams are stopped, disable module and clock */ +if (!dev->first_stream) { +/* Disable the module */ +bcm2708_i2s_clear_bits (dev,BCM2708_I2S_CS_A_REG , +BCM2708_I2S_EN ); + +/* +* Stopping clock is necessary, because stop does +* not stop the clock when SND_SOC_DAIFMT_CONT +*/ +bcm2708_i2s_stop_clock (dev); + } +}
[...]
sound/soc/bcm2708/bcm2708-pcm.c
Quoted 6 lines
sound/soc/bcm2708/bcm2708-pcm.c new file +0
-0@@ -0,0 +1,303 @@
I think you should be able to remove the bulk of this driver by using the generic PCM dmaengine driver.
[...]
sound/soc/bcm2708/bcm2708-pcm.c line 51
Quoted 11 lines
sound/soc/bcm2708/bcm2708-pcm.c new file +11
-0+ +#include "bcm2708-pcm.h" + +static const struct snd_pcm_hardware bcm2708_pcm_hardware = { + .info =SNDRV_PCM_INFO_MMAP | +SNDRV_PCM_INFO_MMAP_VALID | +SNDRV_PCM_INFO_INTERLEAVED | +SNDRV_PCM_INFO_PAUSE | +SNDRV_PCM_INFO_RESUME | +SNDRV_PCM_INFO_JOINT_DUPLEX | +SNDRV_PCM_INFO_NO_PERIOD_WAKEUP ,
I don't think you actually support the last one, and neither pause and resume are supported.
Quoted 9 lines
sound/soc/bcm2708/bcm2708-pcm.c new file +9
-0+ .formats = SNDRV_PCM_FMTBIT_S16_LE | + SNDRV_PCM_FMTBIT_S32_LE, + .period_bytes_min = 32, + .period_bytes_max = 64 * 1024, + .periods_min = 2, + .periods_max = 255, + .buffer_bytes_max = 128 * 1024, +}; +
[...]
2013/5/22 Lars-Peter Clausen <[email protected]>
Quoted 19 lines · Florian Meier
[...]
Quoted 6 lines
sound/soc/bcm2708/bcm2708-i2s.c new file +0
-0@@ -0,0 +1,964 @@[...]
Quoted 9 lines
sound/soc/bcm2708/bcm2708-i2s.c new file +8
-0+static int bcm2708_i2s_startup (struct snd_pcm_substream *substream, +struct snd_soc_dai *dai) +{ +struct bcm2708_i2s_dev *dev =snd_soc_dai_get_drvdata (dai); + + } else { + snd_pcm_hw_constraint_minmax(substream->runtime, + SNDRV_PCM_HW_PARAM_RATE,
Quoted 8 lines
sound/soc/bcm2708/bcm2708-i2s.c line 795
Quoted 3 lines
sound/soc/bcm2708/bcm2708-i2s.c new file +3
-0+ first_runtime->rate, + first_runtime->rate); + }Setting the
symmetric_rateslets the core take care of setting the proper constraints on the rate. It probably makes sense to also add asymmetric_formatssince this isn't the only driver that has the requirement that the formats for playback and capture match.
You are right! I forgot to remove this after I found the symmetric_rates flag. For symmetric_formats I will have a closer look into the ALSA sources.
Quoted 10 lines
[...]
sound/soc/bcm2708/bcm2708-pcm.c
Quoted 6 lines
sound/soc/bcm2708/bcm2708-pcm.c new file +0
-0@@ -0,0 +1,303 @@I think you should be able to remove the bulk of this driver by using the generic PCM dmaengine driver.
As implied in the header of this file, I would really like to use the generic PCM dmaengine driver,
but I would also like to maintain compatibility to the Raspberry Pi downstream kernel.
Therefore, I have to wait until the downstream kernel migrates to the new version.
Quoted 15 lines
[...]
sound/soc/bcm2708/bcm2708-pcm.c line 51
Quoted 11 lines
sound/soc/bcm2708/bcm2708-pcm.c new file +11
-0+ +#include "bcm2708-pcm.h" + +static const struct snd_pcm_hardware bcm2708_pcm_hardware = { + .info =SNDRV_PCM_INFO_MMAP | +SNDRV_PCM_INFO_MMAP_VALID | +SNDRV_PCM_INFO_INTERLEAVED | +SNDRV_PCM_INFO_PAUSE | +SNDRV_PCM_INFO_RESUME | +SNDRV_PCM_INFO_JOINT_DUPLEX | +SNDRV_PCM_INFO_NO_PERIOD_WAKEUP ,I don't think you actually support the last one, and neither pause and resume are supported.
I will have a closer look at these flags. Thanks!