[alsa-devel] [PATCH v2 0/6] ASoC: dwc: Add device tree support to designware I2S
From: Andrew Jackson Andrew.Jackson@arm.com
This patch set extends the DesignWare I2S driver to provide device tree support and fixes a couple of small faults.
Changes v1->v2 + Drop negative use count patch [Mark Brown] + Remove unnecessary debug print messages [Lars-Peter Clausen] + Rewrite iteration as for loop rather than do...while [Mark Brown] + Reorder patches to send fixes first [Mark Brown] + Simplify device tree code [Mark Brown] + Split device tree patch in two [Mark Brown] + Expand explanatory comment on channel configuration [Rajeev Kumar]
Arnd: I've not forgotten about updating the spear entries
Andrew Jackson (6): ASoC: dwc: Remove unnecessary debug messages and tests ASoC: dwc: Ensure FIFOs are flushed to prevent channel swap ASoC: dwc: Iterate over all channels ASoC: dwc: Reorder code in preparation for DT support ASoC: dwc: Add devicetree support for Designware I2S ASoC: dwc: Add documentation for I2S DT
.../devicetree/bindings/sound/designware-i2s.txt | 32 +++ sound/soc/dwc/Kconfig | 1 + sound/soc/dwc/designware_i2s.c | 294 ++++++++++++++------ 3 files changed, 248 insertions(+), 79 deletions(-) create mode 100644 Documentation/devicetree/bindings/sound/designware-i2s.txt
From: Andrew Jackson Andrew.Jackson@arm.com
The devm_XXX allocation functions print a message on failure, so additional messages are not required.
Signed-off-by: Andrew Jackson Andrew.Jackson@arm.com --- sound/soc/dwc/designware_i2s.c | 13 ++----------- 1 files changed, 2 insertions(+), 11 deletions(-)
diff --git a/sound/soc/dwc/designware_i2s.c b/sound/soc/dwc/designware_i2s.c index d202c7c..ef771ea 100644 --- a/sound/soc/dwc/designware_i2s.c +++ b/sound/soc/dwc/designware_i2s.c @@ -345,26 +345,17 @@ static int dw_i2s_probe(struct platform_device *pdev) }
dw_i2s_dai = devm_kzalloc(&pdev->dev, sizeof(*dw_i2s_dai), GFP_KERNEL); - if (!dw_i2s_dai) { - dev_err(&pdev->dev, "mem allocation failed for dai driver\n"); + if (!dw_i2s_dai) return -ENOMEM; - }
dw_i2s_dai->ops = &dw_i2s_dai_ops; dw_i2s_dai->suspend = dw_i2s_suspend; dw_i2s_dai->resume = dw_i2s_resume;
res = platform_get_resource(pdev, IORESOURCE_MEM, 0); - if (!res) { - dev_err(&pdev->dev, "no i2s resource defined\n"); - return -ENODEV; - } - dev->i2s_base = devm_ioremap_resource(&pdev->dev, res); - if (IS_ERR(dev->i2s_base)) { - dev_err(&pdev->dev, "ioremap fail for i2s_region\n"); + if (IS_ERR(dev->i2s_base)) return PTR_ERR(dev->i2s_base); - }
cap = pdata->cap; dev->capability = cap;
On Fri, 2014-12-12 at 09:25 +0000, Andrew Jackson wrote:
The devm_XXX allocation functions print a message on failure, so additional messages are not required.
[]
diff --git a/sound/soc/dwc/designware_i2s.c b/sound/soc/dwc/designware_i2s.c
[]
@@ -345,26 +345,17 @@ static int dw_i2s_probe(struct platform_device *pdev) }
dw_i2s_dai = devm_kzalloc(&pdev->dev, sizeof(*dw_i2s_dai), GFP_KERNEL);
- if (!dw_i2s_dai) {
dev_err(&pdev->dev, "mem allocation failed for dai driver\n");
- if (!dw_i2s_dai) return -ENOMEM;
- }
ok.
dw_i2s_dai->ops = &dw_i2s_dai_ops; dw_i2s_dai->suspend = dw_i2s_suspend; dw_i2s_dai->resume = dw_i2s_resume;
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (!res) {
dev_err(&pdev->dev, "no i2s resource defined\n");
return -ENODEV;
- }
Why delete this?
dev->i2s_base = devm_ioremap_resource(&pdev->dev, res);
- if (IS_ERR(dev->i2s_base)) {
dev_err(&pdev->dev, "ioremap fail for i2s_region\n");
- if (IS_ERR(dev->i2s_base)) return PTR_ERR(dev->i2s_base);
- }
or this?
On 12/12/2014 10:31 AM, Joe Perches wrote: [...]
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (!res) {
dev_err(&pdev->dev, "no i2s resource defined\n");
return -ENODEV;
- }
Why delete this?
dev->i2s_base = devm_ioremap_resource(&pdev->dev, res);
- if (IS_ERR(dev->i2s_base)) {
dev_err(&pdev->dev, "ioremap fail for i2s_region\n");
- if (IS_ERR(dev->i2s_base)) return PTR_ERR(dev->i2s_base);
- }
or this?
devm_ioremap_resource both checks if res is NULL and does also its own error reporting. So the code in the driver is redundant.
On 12/12/14 09:31, Joe Perches wrote:
On Fri, 2014-12-12 at 09:25 +0000, Andrew Jackson wrote:
The devm_XXX allocation functions print a message on failure, so additional messages are not required.
[]
diff --git a/sound/soc/dwc/designware_i2s.c b/sound/soc/dwc/designware_i2s.c
[]
@@ -345,26 +345,17 @@ static int dw_i2s_probe(struct platform_device *pdev) }
dw_i2s_dai = devm_kzalloc(&pdev->dev, sizeof(*dw_i2s_dai), GFP_KERNEL);
- if (!dw_i2s_dai) {
dev_err(&pdev->dev, "mem allocation failed for dai driver\n");
- if (!dw_i2s_dai) return -ENOMEM;
- }
ok.
dw_i2s_dai->ops = &dw_i2s_dai_ops; dw_i2s_dai->suspend = dw_i2s_suspend; dw_i2s_dai->resume = dw_i2s_resume;
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (!res) {
dev_err(&pdev->dev, "no i2s resource defined\n");
return -ENODEV;
- }
Why delete this?
Lars-Peter said that it was unnecessary: see http://lists.infradead.org/pipermail/linux-arm-kernel/2014-December/308416.html.
dev->i2s_base = devm_ioremap_resource(&pdev->dev, res);
- if (IS_ERR(dev->i2s_base)) {
dev_err(&pdev->dev, "ioremap fail for i2s_region\n");
- if (IS_ERR(dev->i2s_base)) return PTR_ERR(dev->i2s_base);
- }
or this?
Ditto
Andrew
On Fri, 2014-12-12 at 09:39 +0000, Andrew Jackson wrote:
On 12/12/14 09:31, Joe Perches wrote:
On Fri, 2014-12-12 at 09:25 +0000, Andrew Jackson wrote:
The devm_XXX allocation functions print a message on failure, so additional messages are not required.
[]
diff --git a/sound/soc/dwc/designware_i2s.c b/sound/soc/dwc/designware_i2s.c
[]
@@ -345,26 +345,17 @@ static int dw_i2s_probe(struct platform_device *pdev) }
dw_i2s_dai = devm_kzalloc(&pdev->dev, sizeof(*dw_i2s_dai), GFP_KERNEL);
- if (!dw_i2s_dai) {
dev_err(&pdev->dev, "mem allocation failed for dai driver\n");
- if (!dw_i2s_dai) return -ENOMEM;
- }
ok.
dw_i2s_dai->ops = &dw_i2s_dai_ops; dw_i2s_dai->suspend = dw_i2s_suspend; dw_i2s_dai->resume = dw_i2s_resume;
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (!res) {
dev_err(&pdev->dev, "no i2s resource defined\n");
return -ENODEV;
- }
Why delete this?
Lars-Peter said that it was unnecessary: see http://lists.infradead.org/pipermail/linux-arm-kernel/2014-December/308416.html.
dev->i2s_base = devm_ioremap_resource(&pdev->dev, res);
- if (IS_ERR(dev->i2s_base)) {
dev_err(&pdev->dev, "ioremap fail for i2s_region\n");
- if (IS_ERR(dev->i2s_base)) return PTR_ERR(dev->i2s_base);
- }
or this?
Ditto
OK, but your commit message doesn't match the changes.
These messages are redundant not what most consider debugging messages (ie: emitted at KERN_DEBUG).
The 2nd one isn't a devm_<foo> function and the last one isn't an allocation.
On Fri, Dec 12, 2014 at 09:25:00AM +0000, Andrew Jackson wrote:
From: Andrew Jackson Andrew.Jackson@arm.com
The devm_XXX allocation functions print a message on failure, so additional messages are not required.
Applied, thanks.
From: Andrew Jackson Andrew.Jackson@arm.com
If the FIFOs aren't flushed, the left/right channels may be swapped: this may occur if the FIFOs are not empty when the streams start.
Signed-off-by: Andrew Jackson Andrew.Jackson@arm.com --- sound/soc/dwc/designware_i2s.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/sound/soc/dwc/designware_i2s.c b/sound/soc/dwc/designware_i2s.c index ef771ea..b9d6a25 100644 --- a/sound/soc/dwc/designware_i2s.c +++ b/sound/soc/dwc/designware_i2s.c @@ -228,12 +228,14 @@ static int dw_i2s_hw_params(struct snd_pcm_substream *substream, i2s_disable_channels(dev, substream->stream);
if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) { + i2s_write_reg(dev->i2s_base, TXFFR, 1); i2s_write_reg(dev->i2s_base, TCR(ch_reg), xfer_resolution); i2s_write_reg(dev->i2s_base, TFCR(ch_reg), 0x02); irq = i2s_read_reg(dev->i2s_base, IMR(ch_reg)); i2s_write_reg(dev->i2s_base, IMR(ch_reg), irq & ~0x30); i2s_write_reg(dev->i2s_base, TER(ch_reg), 1); } else { + i2s_write_reg(dev->i2s_base, RXFFR, 1); i2s_write_reg(dev->i2s_base, RCR(ch_reg), xfer_resolution); i2s_write_reg(dev->i2s_base, RFCR(ch_reg), 0x07); irq = i2s_read_reg(dev->i2s_base, IMR(ch_reg));
On 12/12/2014 10:25 AM, Andrew Jackson wrote:
From: Andrew Jackson Andrew.Jackson@arm.com
If the FIFOs aren't flushed, the left/right channels may be swapped: this may occur if the FIFOs are not empty when the streams start.
Signed-off-by: Andrew Jackson Andrew.Jackson@arm.com
sound/soc/dwc/designware_i2s.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/sound/soc/dwc/designware_i2s.c b/sound/soc/dwc/designware_i2s.c index ef771ea..b9d6a25 100644 --- a/sound/soc/dwc/designware_i2s.c +++ b/sound/soc/dwc/designware_i2s.c @@ -228,12 +228,14 @@ static int dw_i2s_hw_params(struct snd_pcm_substream *substream, i2s_disable_channels(dev, substream->stream);
if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
i2s_write_reg(dev->i2s_base, TCR(ch_reg), xfer_resolution); i2s_write_reg(dev->i2s_base, TFCR(ch_reg), 0x02); irq = i2s_read_reg(dev->i2s_base, IMR(ch_reg)); i2s_write_reg(dev->i2s_base, IMR(ch_reg), irq & ~0x30); i2s_write_reg(dev->i2s_base, TER(ch_reg), 1); } else {i2s_write_reg(dev->i2s_base, TXFFR, 1);
i2s_write_reg(dev->i2s_base, RCR(ch_reg), xfer_resolution); i2s_write_reg(dev->i2s_base, RFCR(ch_reg), 0x07); irq = i2s_read_reg(dev->i2s_base, IMR(ch_reg));i2s_write_reg(dev->i2s_base, RXFFR, 1);
This should probably go into the prepare callback. prepare is for example also called when recovering from a underrun/overrun. Whereas hwparams is only called during initial setup of the stream.
On Fri, Dec 12, 2014 at 10:37:34AM +0100, Lars-Peter Clausen wrote:
This should probably go into the prepare callback. prepare is for example also called when recovering from a underrun/overrun. Whereas hwparams is only called during initial setup of the stream.
Indeed, that's a better place.
On 12/12/14 09:37, Lars-Peter Clausen wrote:
On 12/12/2014 10:25 AM, Andrew Jackson wrote:
From: Andrew Jackson Andrew.Jackson@arm.com
If the FIFOs aren't flushed, the left/right channels may be swapped: this may occur if the FIFOs are not empty when the streams start.
Signed-off-by: Andrew Jackson Andrew.Jackson@arm.com
sound/soc/dwc/designware_i2s.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/sound/soc/dwc/designware_i2s.c b/sound/soc/dwc/designware_i2s.c index ef771ea..b9d6a25 100644 --- a/sound/soc/dwc/designware_i2s.c +++ b/sound/soc/dwc/designware_i2s.c @@ -228,12 +228,14 @@ static int dw_i2s_hw_params(struct snd_pcm_substream *substream, i2s_disable_channels(dev, substream->stream);
if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
i2s_write_reg(dev->i2s_base, TCR(ch_reg), xfer_resolution); i2s_write_reg(dev->i2s_base, TFCR(ch_reg), 0x02); irq = i2s_read_reg(dev->i2s_base, IMR(ch_reg)); i2s_write_reg(dev->i2s_base, IMR(ch_reg), irq & ~0x30); i2s_write_reg(dev->i2s_base, TER(ch_reg), 1); } else {i2s_write_reg(dev->i2s_base, TXFFR, 1);
i2s_write_reg(dev->i2s_base, RCR(ch_reg), xfer_resolution); i2s_write_reg(dev->i2s_base, RFCR(ch_reg), 0x07); irq = i2s_read_reg(dev->i2s_base, IMR(ch_reg));i2s_write_reg(dev->i2s_base, RXFFR, 1);
This should probably go into the prepare callback. prepare is for example also called when recovering from a underrun/overrun. Whereas hwparams is only called during initial setup of the stream.
That's a good idea: thank you. I'll rework the patch with this in mind.
Andrew
From: Andrew Jackson Andrew.Jackson@arm.com
The Designware core can be configured with up to four stereo channels. Each stereo channel is individually configured so, when the driver's hw_params call is made, each requested stereo channel has to be programmed.
Signed-off-by: Andrew Jackson Andrew.Jackson@arm.com --- sound/soc/dwc/designware_i2s.c | 39 ++++++++++++++++++--------------------- 1 files changed, 18 insertions(+), 21 deletions(-)
diff --git a/sound/soc/dwc/designware_i2s.c b/sound/soc/dwc/designware_i2s.c index b9d6a25..1d26ded 100644 --- a/sound/soc/dwc/designware_i2s.c +++ b/sound/soc/dwc/designware_i2s.c @@ -209,16 +209,9 @@ static int dw_i2s_hw_params(struct snd_pcm_substream *substream,
switch (config->chan_nr) { case EIGHT_CHANNEL_SUPPORT: - ch_reg = 3; - break; case SIX_CHANNEL_SUPPORT: - ch_reg = 2; - break; case FOUR_CHANNEL_SUPPORT: - ch_reg = 1; - break; case TWO_CHANNEL_SUPPORT: - ch_reg = 0; break; default: dev_err(dev->dev, "channel not supported\n"); @@ -227,20 +220,24 @@ static int dw_i2s_hw_params(struct snd_pcm_substream *substream,
i2s_disable_channels(dev, substream->stream);
- if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) { - i2s_write_reg(dev->i2s_base, TXFFR, 1); - i2s_write_reg(dev->i2s_base, TCR(ch_reg), xfer_resolution); - i2s_write_reg(dev->i2s_base, TFCR(ch_reg), 0x02); - irq = i2s_read_reg(dev->i2s_base, IMR(ch_reg)); - i2s_write_reg(dev->i2s_base, IMR(ch_reg), irq & ~0x30); - i2s_write_reg(dev->i2s_base, TER(ch_reg), 1); - } else { - i2s_write_reg(dev->i2s_base, RXFFR, 1); - i2s_write_reg(dev->i2s_base, RCR(ch_reg), xfer_resolution); - i2s_write_reg(dev->i2s_base, RFCR(ch_reg), 0x07); - irq = i2s_read_reg(dev->i2s_base, IMR(ch_reg)); - i2s_write_reg(dev->i2s_base, IMR(ch_reg), irq & ~0x03); - i2s_write_reg(dev->i2s_base, RER(ch_reg), 1); + for (ch_reg = 0; ch_reg < (config->chan_nr / 2); ch_reg++) { + if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) { + i2s_write_reg(dev->i2s_base, TXFFR, 1); + i2s_write_reg(dev->i2s_base, TCR(ch_reg), + xfer_resolution); + i2s_write_reg(dev->i2s_base, TFCR(ch_reg), 0x02); + irq = i2s_read_reg(dev->i2s_base, IMR(ch_reg)); + i2s_write_reg(dev->i2s_base, IMR(ch_reg), irq & ~0x30); + i2s_write_reg(dev->i2s_base, TER(ch_reg), 1); + } else { + i2s_write_reg(dev->i2s_base, RXFFR, 1); + i2s_write_reg(dev->i2s_base, RCR(ch_reg), + xfer_resolution); + i2s_write_reg(dev->i2s_base, RFCR(ch_reg), 0x07); + irq = i2s_read_reg(dev->i2s_base, IMR(ch_reg)); + i2s_write_reg(dev->i2s_base, IMR(ch_reg), irq & ~0x03); + i2s_write_reg(dev->i2s_base, RER(ch_reg), 1); + } }
i2s_write_reg(dev->i2s_base, CCR, ccr);
From: Andrew Jackson Andrew.Jackson@arm.com
Move code that configures the DAI and DMA into a separate function. This reduces the size of the dw_i2s_probe function and will make it easier to add support for device tree to the driver.
Signed-off-by: Andrew Jackson Andrew.Jackson@arm.com --- sound/soc/dwc/designware_i2s.c | 73 +++++++++++++++++++++------------------ 1 files changed, 39 insertions(+), 34 deletions(-)
diff --git a/sound/soc/dwc/designware_i2s.c b/sound/soc/dwc/designware_i2s.c index 1d26ded..343f5e7 100644 --- a/sound/soc/dwc/designware_i2s.c +++ b/sound/soc/dwc/designware_i2s.c @@ -323,13 +323,47 @@ static int dw_i2s_resume(struct snd_soc_dai *dai) #define dw_i2s_resume NULL #endif
+static void dw_configure_dai_by_pd(struct dw_i2s_dev *dev, + struct snd_soc_dai_driver *dw_i2s_dai, + struct resource *res, + const struct i2s_platform_data *pdata) +{ + /* Set DMA slaves info */ + + dev->play_dma_data.data = pdata->play_dma_data; + dev->capture_dma_data.data = pdata->capture_dma_data; + dev->play_dma_data.addr = res->start + I2S_TXDMA; + dev->capture_dma_data.addr = res->start + I2S_RXDMA; + dev->play_dma_data.max_burst = 16; + dev->capture_dma_data.max_burst = 16; + dev->play_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES; + dev->capture_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES; + dev->play_dma_data.filter = pdata->filter; + dev->capture_dma_data.filter = pdata->filter; + + if (pdata->cap & DWC_I2S_PLAY) { + dev_dbg(dev->dev, " designware: play supported\n"); + dw_i2s_dai->playback.channels_min = MIN_CHANNEL_NUM; + dw_i2s_dai->playback.channels_max = pdata->channel; + dw_i2s_dai->playback.formats = pdata->snd_fmts; + dw_i2s_dai->playback.rates = pdata->snd_rates; + } + + if (pdata->cap & DWC_I2S_RECORD) { + dev_dbg(dev->dev, "designware: record supported\n"); + dw_i2s_dai->capture.channels_min = MIN_CHANNEL_NUM; + dw_i2s_dai->capture.channels_max = pdata->channel; + dw_i2s_dai->capture.formats = pdata->snd_fmts; + dw_i2s_dai->capture.rates = pdata->snd_rates; + } +} + static int dw_i2s_probe(struct platform_device *pdev) { const struct i2s_platform_data *pdata = pdev->dev.platform_data; struct dw_i2s_dev *dev; struct resource *res; int ret; - unsigned int cap; struct snd_soc_dai_driver *dw_i2s_dai;
if (!pdata) { @@ -356,23 +390,11 @@ static int dw_i2s_probe(struct platform_device *pdev) if (IS_ERR(dev->i2s_base)) return PTR_ERR(dev->i2s_base);
- cap = pdata->cap; - dev->capability = cap; - dev->i2s_clk_cfg = pdata->i2s_clk_cfg; - - /* Set DMA slaves info */ - - dev->play_dma_data.data = pdata->play_dma_data; - dev->capture_dma_data.data = pdata->capture_dma_data; - dev->play_dma_data.addr = res->start + I2S_TXDMA; - dev->capture_dma_data.addr = res->start + I2S_RXDMA; - dev->play_dma_data.max_burst = 16; - dev->capture_dma_data.max_burst = 16; - dev->play_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES; - dev->capture_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES; - dev->play_dma_data.filter = pdata->filter; - dev->capture_dma_data.filter = pdata->filter; + dev->dev = &pdev->dev; + dw_configure_dai_by_pd(dev, dw_i2s_dai, res, pdata);
+ dev->capability = pdata->cap; + dev->i2s_clk_cfg = pdata->i2s_clk_cfg; dev->clk = clk_get(&pdev->dev, NULL); if (IS_ERR(dev->clk)) return PTR_ERR(dev->clk); @@ -381,23 +403,6 @@ static int dw_i2s_probe(struct platform_device *pdev) if (ret < 0) goto err_clk_put;
- if (cap & DWC_I2S_PLAY) { - dev_dbg(&pdev->dev, " designware: play supported\n"); - dw_i2s_dai->playback.channels_min = MIN_CHANNEL_NUM; - dw_i2s_dai->playback.channels_max = pdata->channel; - dw_i2s_dai->playback.formats = pdata->snd_fmts; - dw_i2s_dai->playback.rates = pdata->snd_rates; - } - - if (cap & DWC_I2S_RECORD) { - dev_dbg(&pdev->dev, "designware: record supported\n"); - dw_i2s_dai->capture.channels_min = MIN_CHANNEL_NUM; - dw_i2s_dai->capture.channels_max = pdata->channel; - dw_i2s_dai->capture.formats = pdata->snd_fmts; - dw_i2s_dai->capture.rates = pdata->snd_rates; - } - - dev->dev = &pdev->dev; dev_set_drvdata(&pdev->dev, dev); ret = snd_soc_register_component(&pdev->dev, &dw_i2s_component, dw_i2s_dai, 1);
From: Andrew Jackson Andrew.Jackson@arm.com
Allow the driver to be configured through a device tree rather than platform data. When using device-tree, read the I2S block's configuration from the relevant registers: this reduces the amount of information required in the device tree.
Signed-off-by: Andrew Jackson Andrew.Jackson@arm.com --- sound/soc/dwc/Kconfig | 1 + sound/soc/dwc/designware_i2s.c | 199 ++++++++++++++++++++++++++++++++++------ 2 files changed, 171 insertions(+), 29 deletions(-)
diff --git a/sound/soc/dwc/Kconfig b/sound/soc/dwc/Kconfig index e334900..d50e085 100644 --- a/sound/soc/dwc/Kconfig +++ b/sound/soc/dwc/Kconfig @@ -1,6 +1,7 @@ config SND_DESIGNWARE_I2S tristate "Synopsys I2S Device Driver" depends on CLKDEV_LOOKUP + select SND_SOC_GENERIC_DMAENGINE_PCM help Say Y or M if you want to add support for I2S driver for Synopsys desigwnware I2S device. The device supports upto diff --git a/sound/soc/dwc/designware_i2s.c b/sound/soc/dwc/designware_i2s.c index 343f5e7..9e1f547 100644 --- a/sound/soc/dwc/designware_i2s.c +++ b/sound/soc/dwc/designware_i2s.c @@ -22,6 +22,7 @@ #include <sound/pcm.h> #include <sound/pcm_params.h> #include <sound/soc.h> +#include <sound/dmaengine_pcm.h>
/* common register for all channel */ #define IER 0x000 @@ -54,9 +55,35 @@ #define I2S_COMP_VERSION 0x01F8 #define I2S_COMP_TYPE 0x01FC
+/* + * Component parameter register fields - define the I2S block's + * configuration. + */ +#define COMP1_TX_WORDSIZE_3(r) (((r) & GENMASK(27, 25)) >> 25) +#define COMP1_TX_WORDSIZE_2(r) (((r) & GENMASK(24, 22)) >> 22) +#define COMP1_TX_WORDSIZE_1(r) (((r) & GENMASK(21, 19)) >> 19) +#define COMP1_TX_WORDSIZE_0(r) (((r) & GENMASK(18, 16)) >> 16) +#define COMP1_TX_CHANNELS(r) (((r) & GENMASK(10, 9)) >> 9) +#define COMP1_RX_CHANNELS(r) (((r) & GENMASK(8, 7)) >> 7) +#define COMP1_RX_ENABLED(r) (((r) & BIT(6)) >> 6) +#define COMP1_TX_ENABLED(r) (((r) & BIT(5)) >> 5) +#define COMP1_MODE_EN(r) (((r) & BIT(4)) >> 4) +#define COMP1_FIFO_DEPTH_GLOBAL(r) (((r) & GENMASK(3, 2)) >> 2) +#define COMP1_APB_DATA_WIDTH(r) (((r) & GENMASK(1, 0)) >> 0) + +#define COMP2_RX_WORDSIZE_3(r) (((r) & GENMASK(12, 10)) >> 10) +#define COMP2_RX_WORDSIZE_2(r) (((r) & GENMASK(9, 7)) >> 7) +#define COMP2_RX_WORDSIZE_1(r) (((r) & GENMASK(5, 3)) >> 3) +#define COMP2_RX_WORDSIZE_0(r) (((r) & GENMASK(2, 0)) >> 0) + #define MAX_CHANNEL_NUM 8 #define MIN_CHANNEL_NUM 2
+union snd_dma_data { + struct i2s_dma_data pd; + struct snd_dmaengine_dai_dma_data dt; +}; + struct dw_i2s_dev { void __iomem *i2s_base; struct clk *clk; @@ -65,8 +92,8 @@ struct dw_i2s_dev { struct device *dev;
/* data related to DMA transfers b/w i2s and DMAC */ - struct i2s_dma_data play_dma_data; - struct i2s_dma_data capture_dma_data; + union snd_dma_data play_dma_data; + union snd_dma_data capture_dma_data; struct i2s_clk_config_data config; int (*i2s_clk_cfg)(struct i2s_clk_config_data *config); }; @@ -153,7 +180,7 @@ static int dw_i2s_startup(struct snd_pcm_substream *substream, struct snd_soc_dai *cpu_dai) { struct dw_i2s_dev *dev = snd_soc_dai_get_drvdata(cpu_dai); - struct i2s_dma_data *dma_data = NULL; + union snd_dma_data *dma_data = NULL;
if (!(dev->capability & DWC_I2S_RECORD) && (substream->stream == SNDRV_PCM_STREAM_CAPTURE)) @@ -244,13 +271,18 @@ static int dw_i2s_hw_params(struct snd_pcm_substream *substream,
config->sample_rate = params_rate(params);
- if (!dev->i2s_clk_cfg) - return -EINVAL; + if (dev->i2s_clk_cfg) { + ret = dev->i2s_clk_cfg(config); + if (ret < 0) { + dev_err(dev->dev, "runtime audio clk config fail\n"); + return ret; + } + } else { + u32 bitclk;
- ret = dev->i2s_clk_cfg(config); - if (ret < 0) { - dev_err(dev->dev, "runtime audio clk config fail\n"); - return ret; + /* TODO: Validate sample rate against permissible set */ + bitclk = config->sample_rate * config->data_width * 2; + clk_set_rate(dev->clk, bitclk); }
return 0; @@ -323,6 +355,31 @@ static int dw_i2s_resume(struct snd_soc_dai *dai) #define dw_i2s_resume NULL #endif
+/* Maximum resolution of a channel - not uniformly spaced */ +static const u32 fifo_width[] = { + 12, 16, 20, 24, 32, 0, 0, 0 +}; + +/* Width of (DMA) bus */ +static const u32 bus_widths[] = { + DMA_SLAVE_BUSWIDTH_1_BYTE, + DMA_SLAVE_BUSWIDTH_2_BYTES, + DMA_SLAVE_BUSWIDTH_4_BYTES, + DMA_SLAVE_BUSWIDTH_UNDEFINED +}; + +/* PCM format to support channel resolution */ +static const u32 formats[] = { + SNDRV_PCM_FMTBIT_S16_LE, + SNDRV_PCM_FMTBIT_S16_LE, + SNDRV_PCM_FMTBIT_S24_LE, + SNDRV_PCM_FMTBIT_S24_LE, + SNDRV_PCM_FMTBIT_S32_LE, + 0, + 0, + 0 +}; + static void dw_configure_dai_by_pd(struct dw_i2s_dev *dev, struct snd_soc_dai_driver *dw_i2s_dai, struct resource *res, @@ -330,16 +387,16 @@ static void dw_configure_dai_by_pd(struct dw_i2s_dev *dev, { /* Set DMA slaves info */
- dev->play_dma_data.data = pdata->play_dma_data; - dev->capture_dma_data.data = pdata->capture_dma_data; - dev->play_dma_data.addr = res->start + I2S_TXDMA; - dev->capture_dma_data.addr = res->start + I2S_RXDMA; - dev->play_dma_data.max_burst = 16; - dev->capture_dma_data.max_burst = 16; - dev->play_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES; - dev->capture_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES; - dev->play_dma_data.filter = pdata->filter; - dev->capture_dma_data.filter = pdata->filter; + dev->play_dma_data.pd.data = pdata->play_dma_data; + dev->capture_dma_data.pd.data = pdata->capture_dma_data; + dev->play_dma_data.pd.addr = res->start + I2S_TXDMA; + dev->capture_dma_data.pd.addr = res->start + I2S_RXDMA; + dev->play_dma_data.pd.max_burst = 16; + dev->capture_dma_data.pd.max_burst = 16; + dev->play_dma_data.pd.addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES; + dev->capture_dma_data.pd.addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES; + dev->play_dma_data.pd.filter = pdata->filter; + dev->capture_dma_data.pd.filter = pdata->filter;
if (pdata->cap & DWC_I2S_PLAY) { dev_dbg(dev->dev, " designware: play supported\n"); @@ -358,6 +415,59 @@ static void dw_configure_dai_by_pd(struct dw_i2s_dev *dev, } }
+static void dw_configure_dai_by_dt(struct dw_i2s_dev *dev, + struct snd_soc_dai_driver *dw_i2s_dai, + struct resource *res) +{ + /* + * Read component parameter registers to extract + * the I2S block's configuration. + */ + u32 comp1 = i2s_read_reg(dev->i2s_base, I2S_COMP_PARAM_1); + u32 comp2 = i2s_read_reg(dev->i2s_base, I2S_COMP_PARAM_2); + u32 bus_width = bus_widths[COMP1_APB_DATA_WIDTH(comp1)]; + u32 fifo_depth = 1 << (1 + COMP1_FIFO_DEPTH_GLOBAL(comp1)); + u32 max_size; + + /* + * All channels' hardware configuration assumed to be the same. + */ + if (COMP1_TX_ENABLED(comp1)) { + dev_dbg(dev->dev, "playback capable\n"); + + dev->capability |= DWC_I2S_PLAY; + max_size = COMP1_TX_WORDSIZE_0(comp1); + dev->play_dma_data.dt.addr = res->start + I2S_TXDMA; + dev->play_dma_data.dt.addr_width = bus_width; + dev->play_dma_data.dt.chan_name = "TX"; + dev->play_dma_data.dt.fifo_size = fifo_depth * + (fifo_width[max_size]) >> 8; + dev->play_dma_data.dt.maxburst = 16; + dw_i2s_dai->playback.channels_min = MIN_CHANNEL_NUM; + dw_i2s_dai->playback.channels_max = + 1 << (COMP1_TX_CHANNELS(comp1) + 1); + dw_i2s_dai->playback.formats = formats[max_size]; + dw_i2s_dai->playback.rates = SNDRV_PCM_RATE_8000_192000; + } + if (COMP1_RX_ENABLED(comp1)) { + dev_dbg(dev->dev, "record capable\n"); + + dev->capability |= DWC_I2S_RECORD; + max_size = COMP2_RX_WORDSIZE_0(comp2); + dev->capture_dma_data.dt.addr = res->start + I2S_RXDMA; + dev->capture_dma_data.dt.addr_width = bus_width; + dev->capture_dma_data.dt.chan_name = "RX"; + dev->capture_dma_data.dt.fifo_size = fifo_depth * + (fifo_width[max_size] >> 8); + dev->capture_dma_data.dt.maxburst = 16; + dw_i2s_dai->capture.channels_min = MIN_CHANNEL_NUM; + dw_i2s_dai->capture.channels_max = + 1 << (COMP1_RX_CHANNELS(comp1) + 1); + dw_i2s_dai->capture.formats = formats[max_size]; + dw_i2s_dai->capture.rates = SNDRV_PCM_RATE_8000_192000; + } +} + static int dw_i2s_probe(struct platform_device *pdev) { const struct i2s_platform_data *pdata = pdev->dev.platform_data; @@ -366,11 +476,6 @@ static int dw_i2s_probe(struct platform_device *pdev) int ret; struct snd_soc_dai_driver *dw_i2s_dai;
- if (!pdata) { - dev_err(&pdev->dev, "Invalid platform data\n"); - return -EINVAL; - } - dev = devm_kzalloc(&pdev->dev, sizeof(*dev), GFP_KERNEL); if (!dev) { dev_warn(&pdev->dev, "kzalloc fail\n"); @@ -391,13 +496,28 @@ static int dw_i2s_probe(struct platform_device *pdev) return PTR_ERR(dev->i2s_base);
dev->dev = &pdev->dev; - dw_configure_dai_by_pd(dev, dw_i2s_dai, res, pdata); + if (pdata) { + dw_configure_dai_by_pd(dev, dw_i2s_dai, res, pdata); + + dev->capability = pdata->cap; + dev->i2s_clk_cfg = pdata->i2s_clk_cfg; + if (!dev->i2s_clk_cfg) { + dev_err(&pdev->dev, "no clock configure method\n"); + return -ENODEV; + }
- dev->capability = pdata->cap; - dev->i2s_clk_cfg = pdata->i2s_clk_cfg; - dev->clk = clk_get(&pdev->dev, NULL); + dev->clk = clk_get(&pdev->dev, NULL); + } else { + dw_configure_dai_by_dt(dev, dw_i2s_dai, res); + + dev->clk = devm_clk_get(&pdev->dev, "i2sclk"); + } if (IS_ERR(dev->clk)) - return PTR_ERR(dev->clk); + return PTR_ERR(dev->clk); + + ret = clk_prepare(dev->clk); + if (ret < 0) + goto err_clk_put;
ret = clk_enable(dev->clk); if (ret < 0) @@ -411,6 +531,15 @@ static int dw_i2s_probe(struct platform_device *pdev) goto err_clk_disable; }
+ if (!pdata) { + ret = devm_snd_dmaengine_pcm_register(&pdev->dev, NULL, 0); + if (ret) { + dev_err(&pdev->dev, + "Could not register PCM: %d\n", ret); + return ret; + } + } + return 0;
err_clk_disable: @@ -431,12 +560,24 @@ static int dw_i2s_remove(struct platform_device *pdev) return 0; }
+#ifdef CONFIG_OF +static const struct of_device_id dw_i2s_of_match[] = { + { .compatible = "snps,designware-i2s", }, + {}, +}; + +MODULE_DEVICE_TABLE(of, dw_i2s_of_match); +#endif + static struct platform_driver dw_i2s_driver = { .probe = dw_i2s_probe, .remove = dw_i2s_remove, .driver = { .name = "designware-i2s", .owner = THIS_MODULE, +#ifdef CONFIG_OF + .of_match_table = dw_i2s_of_match, +#endif }, };
From: Andrew Jackson Andrew.Jackson@arm.com
Add documentation for Designware I2S hardware block. The block requires two clocks (one for audio sampling, the other for APB) and DMA channels for receive and transmit.
Signed-off-by: Andrew Jackson Andrew.Jackson@arm.com --- .../devicetree/bindings/sound/designware-i2s.txt | 32 ++++++++++++++++++++ 1 files changed, 32 insertions(+), 0 deletions(-) create mode 100644 Documentation/devicetree/bindings/sound/designware-i2s.txt
diff --git a/Documentation/devicetree/bindings/sound/designware-i2s.txt b/Documentation/devicetree/bindings/sound/designware-i2s.txt new file mode 100644 index 0000000..cdee591 --- /dev/null +++ b/Documentation/devicetree/bindings/sound/designware-i2s.txt @@ -0,0 +1,32 @@ +DesignWare I2S controller + +Required properties: + - compatible : Must be "snps,designware-i2s" + - reg : Must contain I2S core's registers location and length + - clocks : Pairs of phandle and specifier referencing the controller's clocks. + The controller expects two clocks, the clock used for the APB interface and + the clock used as the sampling rate reference clock sample. + - clock-names : "apb_plck" for the clock to the APB interface, "i2sclk" for the sample + rate reference clock. + - dmas: Pairs of phandle and specifier for the DMA channels that are used by + the core. The core expects one or two dma channels, one for transmit and one for + receive. + - dma-names : "tx" for the transmit channel, "rx" for the receive channel. + +For more details on the 'dma', 'dma-names', 'clock' and 'clock-names' properties +please check: + * resource-names.txt + * clock/clock-bindings.txt + * dma/dma.txt + +Example: + + soc_i2s: i2s@7ff90000 { + compatible = "snps,designware-i2s"; + reg = <0x0 0x7ff90000 0x0 0x1000>; + clocks = <&scpi_i2sclk 0>, <&soc_refclk100mhz>; + clock-names = "i2sclk", "apb_pclk"; + #sound-dai-cells = <0>; + dmas = <&dma0 5>; + dma-names = "tx"; + };
participants (4)
-
Andrew Jackson
-
Joe Perches
-
Lars-Peter Clausen
-
Mark Brown