ASoC: dwc: Add devicetree support for Designware I2S
Subsystems (3)
PATCH 4/5 ASoC: dwc: Add devicetree support for Designware I2S
Convert to driver to use either platform_data or device-tree for configuration of the device. When using device-tree, the I2S block's configuration is read from the relevant registers: this reduces the amount of information required in the device tree.
--- .../devicetree/bindings/sound/designware-i2s.txt | 32 +++ sound/soc/dwc/Kconfig | 1 + sound/soc/dwc/designware_i2s.c | 238 ++++++++++++++++---- 3 files changed, 222 insertions(+), 49 deletions(-) create mode 100644 Documentation/devicetree/bindings/sound/designware-i2s.txt
Documentation/devicetree/bindings/sound/designware-i2s.txt new file +32 -0
@@ -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"; + };
sound/soc/dwc/Kconfig +1 -0
@@ -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
sound/soc/dwc/designware_i2s.c +189 -49
324 lines not on this page · raw message
Signature · 1 line
1.7.1
Mark Brown
maintainerQuoted 4 lines · Andrew Jackson
Convert to driver to use either
platform_dataor device-tree for configuration of the device. When using device-tree, the I2S block's configuration is read from the relevant registers: this reduces the amount of information required in the device tree.
This really needs to be split into two or more patches, there's a whole bunch of refactoring to support this DT stuff which should be separate from the DT addition itself. Right now it's hard to tell what each individual bit of the code is supposed to be doing, the patch is far too large and doing far too many individual things.
sound/soc/dwc/designware_i2s.c static int dw_i2s_hw_params(struct snd_pcm_substream *substream, · line 288
Quoted 8 lines
sound/soc/dwc/designware_i2s.c +8
-0+if (dev->using_pd) { + ret = dev->i2s_clk_cfg (config); +if (ret <0 ) { +dev_err (dev->dev,"runtime audio clk config fail\n" ); +return ret; + } + }else { +u32 bitclk;
Having this whole separate path for using platform data feels icky, we don't want to have completely separate flows like this. Checking for the callbacks being there is probably fine but just having totally separate code paths is a bit icky.
Quoted 7 lines · Mark Brown
Quoted 4 lines · Andrew Jackson
Convert to driver to use either
platform_dataor device-tree for configuration of the device. When using device-tree, the I2S block's configuration is read from the relevant registers: this reduces the amount of information required in the device tree.
Quoted 5 lines
This really needs to be split into two or more patches, there's a whole bunch of refactoring to support this DT stuff which should be separate from the DT addition itself. Right now it's hard to tell what each individual bit of the code is supposed to be doing, the patch is far too large and doing far too many individual things.
I will have look at how it might be split. The majority of the new code is in reading and processing the device's configuration: I didn't want to change the platform data handling to do that because some of the comments in the driver suggested that there were ST specific changes to the Designware IP. I wasn't in a position to know whether, if I changed the configuration reading, the driver would still function correctly on the SPEAR platform.
Quoted 5 lines
Quoted 5 lines
sound/soc/dwc/designware_i2s.c +5
-0+if (dev->using_pd) { + ret = dev->i2s_clk_cfg (config); +if (ret <0 ) { +dev_err (dev->dev,"runtime audio clk config fail\n" ); +return ret;
Quoted 8 lines
sound/soc/dwc/designware_i2s.c static int dw_i2s_hw_params(struct snd_pcm_substream *substream, · line 288
Quoted 3 lines
sound/soc/dwc/designware_i2s.c +3
-0+ } + }else { +u32 bitclk;Having this whole separate path for using platform data feels icky, we don't want to have completely separate flows like this. Checking for the callbacks being there is probably fine but just having totally separate code paths is a bit icky.
I wasn't very happy either but making the test explicit seemed reasonable at the time. I'll change the code to test for the presence of the callback instead.
Andrew
Quoted 6 lines · On Wednesday 03 December 2014 16:39:08 Andrew Jackson wrote:
Convert to driver to use either
platform_dataor device-tree for configuration of the device. When using device-tree, the I2S block's configuration is read from the relevant registers: this reduces the amount of information required in the device tree.Signed-off-by Andrew Jackson <[email protected]>
I don't think we even have to worry about the platform_data case here:
the only platform using this hardware in Linux is arm/mach-spear, and it defines a device node with a binding that is similar to the one you document here but that is not implemented in the driver.
So, I think for all practical purposes we can assume that nobody cares if you make incompatible changes as long as you don't introduce build regression.
Also, please adapt the arch/arm/boot/dts/spear13*.dts{,i} files
as good as you can. They are broken in other ways too that you don't have to fix, just make them conform to the binding you add.
Arnd
Mark Brown
maintainerQuoted 4 lines · Arnd Bergmann
I don't think we even have to worry about the
platform_datacase here:
the only platform using this hardware in Linux is arm/mach-spear, and it defines a device node with a binding that is similar to the one you document here but that is not implemented in the driver.
Not all the world uses DT, this is a DesignWare IP so it's likely to get
deployed widely - potentially on PCI cards and things like that.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20141203/be4d7bfa/attachment.sig>
Quoted 9 lines · On Wednesday 03 December 2014 20:19:05 Mark Brown wrote:
Quoted 4 lines · Arnd Bergmann
I don't think we even have to worry about the
platform_datacase here:
the only platform using this hardware in Linux is arm/mach-spear, and it defines a device node with a binding that is similar to the one you document here but that is not implemented in the driver.Not all the world uses DT, this is a DesignWare IP so it's likely to get deployed widely - potentially on PCI cards and things like that.
It's not really new though, and we are only seeing the second user. Maybe this one just isn't as common as a lot of the other designware IP?
Arnd
Mark Brown
maintainerQuoted 7 lines · Arnd Bergmann
Quoted 2 lines · On Wednesday 03 December 2014 20:19:05 Mark Brown wrote:
Not all the world uses DT, this is a DesignWare IP so it's likely to get deployed widely - potentially on PCI cards and things like that.
It's not really new though, and we are only seeing the second user. Maybe this one just isn't as common as a lot of the other designware IP?
It's relatively common with newer companies - it's like UARTs, any company that's been around for a long time will have a homebrew IP already but places that don't (or get fed up with their existing IP) will tend to buy something in.
Quoted 6 lines · On Thursday 04 December 2014 11:07:46 Mark Brown wrote:
Quoted 4 lines · Arnd Bergmann
Quoted 2 lines · On Wednesday 03 December 2014 20:19:05 Mark Brown wrote:
Not all the world uses DT, this is a DesignWare IP so it's likely to get deployed widely - potentially on PCI cards and things like that.
Quoted 7 lines
Quoted 2 lines
It's not really new though, and we are only seeing the second user. Maybe this one just isn't as common as a lot of the other designware IP?
It's relatively common with newer companies - it's like UARTs, any company that's been around for a long time will have a homebrew IP already but places that don't (or get fed up with their existing IP) will tend to buy something in.
I see, but are any of the newer designs going to use platform_data? My general feeling was that the only places that still depend on that are legacy platforms and architectures.
Arnd
Mark Brown
maintainerQuoted 10 lines · Arnd Bergmann
Quoted 4 lines · On Thursday 04 December 2014 11:07:46 Mark Brown wrote:
It's relatively common with newer companies - it's like UARTs, any company that's been around for a long time will have a homebrew IP already but places that don't (or get fed up with their existing IP) will tend to buy something in.
I see, but are any of the newer designs going to use
platform_data? My general feeling was that the only places that still depend on that are legacy platforms and architectures.
And x86 which does have things like PCI cards.
Quoted 13 lines · Arnd Bergmann
Quoted 6 lines · On Wednesday 03 December 2014 16:39:08 Andrew Jackson wrote:
Convert to driver to use either
platform_dataor device-tree for configuration of the device. When using device-tree, the I2S block's configuration is read from the relevant registers: this reduces the amount of information required in the device tree.Signed-off-by Andrew Jackson <[email protected]>I don't think we even have to worry about the
platform_datacase here:
the only platform using this hardware in Linux is arm/mach-spear, and it defines a device node with a binding that is similar to the one you document here but that is not implemented in the driver.
Quoted 8 lines
So, I think for all practical purposes we can assume that nobody cares if you make incompatible changes as long as you don't introduce build regression.
Also, please adapt the
arch/arm/boot/dts/spear13*.dts{,i}files
as good as you can. They are broken in other ways too that you don't have to fix, just make them conform to the binding you add.
Will do. Although I had noticed those DT entries, I couldn't find anything in support of them. I wasn't sure whether someone else was waiting in the wings (so as to speak) with a DT enabled Designware I2S driver.
Quoted 3 lines
Arnd
Quoted 404 lines · Andrew Jackson
Convert to driver to use either
platform_dataor device-tree for configuration of the device. When using device-tree, the I2S block's configuration is read from the relevant registers: this reduces the amount of information required in the device tree.Signed-off-by Andrew Jackson <[email protected]>--- .../devicetree/bindings/sound/designware-i2s.txt | 32 +++ sound/soc/dwc/Kconfig | 1 + sound/soc/dwc/designware_i2s.c | 238 ++++++++++++++++---- 3 files changed, 222 insertions(+), 49 deletions(-) create mode 100644 Documentation/devicetree/bindings/sound/designware-i2s.txtDocumentation/devicetree/bindings/sound/designware-i2s.txt new file +32
-0@@ -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 at 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"; + };sound/soc/dwc/Kconfig +1
-0@@ -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 uptosound/soc/dwc/designware_i2s.c +189
-50@@ -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,19 +55,46 @@#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;int active;unsigned int capability;struct device *dev; +bool using_pd;/* 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 +181,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 ))@@ -227,8 +255,7 @@ static int dw_i2s_hw_params(struct snd_pcm_substream *substream,i2s_disable_channels (dev, substream->stream); -/* Iterate over set of channels - independently controlled. -*/ +/* Iterate over set of channels - independently controlled. */ do { if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) { i2s_write_reg(dev->i2s_base, TCR(ch_reg),@@ -251,13 +278,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->using_pd) { + 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 ;@@ -331,6 +363,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 int dw_i2s_probe (struct platform_device *pdev) {const struct i2s_platform_data *pdata = pdev->dev.platform_data;@@ -340,11 +397,6 @@ static int dw_i2s_probe(struct platform_device *pdev)unsigned int cap;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" );@@ -373,47 +425,114 @@ static int dw_i2s_probe(struct platform_device *pdev)return PTR_ERR (dev->i2s_base); } - cap = pdata->cap; - dev->capability = cap; - dev->i2s_clk_cfg = pdata->i2s_clk_cfg; +if (pdata) { + dev->using_pd = true; + cap = pdata->cap; + dev->capability = cap; + dev->i2s_clk_cfg = pdata->i2s_clk_cfg; +if (!dev->i2s_clk_cfg) { +dev_err (&pdev->dev,"no clock device\n" ); +return -ENODEV ; + } -/* Set DMA slaves info */ +/* Set DMA slaves info */ + + 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; + + dev->clk =clk_get (&pdev->dev,NULL ); +if (IS_ERR (dev->clk)) +return PTR_ERR (dev->clk); + +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; + } - 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 (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; + } + }else { +/* +* 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; + + dev->using_pd = false; + + dev->clk =devm_clk_get (&pdev->dev,"i2sclk" ); +if (IS_ERR (dev->clk)) +return PTR_ERR (dev->clk); + +/* +* Code presumes all channels are configured with the same +* word size. +*/ +if (COMP1_TX_ENABLED (comp1)) { +dev_dbg (&pdev->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 (&pdev->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 ; + } + } - dev->clk =clk_get (&pdev->dev,NULL ); -if (IS_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 ) 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,@@ -423,6 +542,15 @@ static int dw_i2s_probe(struct platform_device *pdev) goto err_clk_disable; } +if (!dev->using_pd) { + 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:@@ -443,12 +571,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 }, }; --1.7.1
Sorry for the last blank mail.
Quoted 134 lines · Andrew Jackson
Convert to driver to use either
platform_dataor device-tree for configuration of the device. When using device-tree, the I2S block's configuration is read from the relevant registers: this reduces the amount of information required in the device tree.Signed-off-by Andrew Jackson <[email protected]>--- .../devicetree/bindings/sound/designware-i2s.txt | 32 +++ sound/soc/dwc/Kconfig | 1 + sound/soc/dwc/designware_i2s.c | 238 ++++++++++++++++---- 3 files changed, 222 insertions(+), 49 deletions(-) create mode 100644 Documentation/devicetree/bindings/sound/designware-i2s.txtDocumentation/devicetree/bindings/sound/designware-i2s.txt new file +32
-0@@ -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"; + };sound/soc/dwc/Kconfig +1
-0@@ -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 uptosound/soc/dwc/designware_i2s.c +31
-3@@ -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,19 +55,46 @@#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;int active;unsigned int capability;struct device *dev; +bool using_pd;/* 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 +181,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 ))
sound/soc/dwc/designware_i2s.c static int dw_i2s_hw_params(struct snd_pcm_substream *substream, · line 258
Quoted 6 lines
sound/soc/dwc/designware_i2s.c +1
-2@@ -227,8 +255,7 @@ static int dw_i2s_hw_params(struct snd_pcm_substream *substream,i2s_disable_channels (dev, substream->stream); -/* Iterate over set of channels - independently controlled. -*/ +/* Iterate over set of channels - independently controlled. */
I dont think so this iteration is required as there are independent channels available.
I don't have the h/w to test this patch. Can anyone test this ?
Quoted 61 lines
sound/soc/dwc/designware_i2s.c +36
-6do { if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) { i2s_write_reg(dev->i2s_base, TCR(ch_reg),@@ -251,13 +278,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->using_pd) { + 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 ;@@ -331,6 +363,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 int dw_i2s_probe (struct platform_device *pdev) {const struct i2s_platform_data *pdata = pdev->dev.platform_data;
sound/soc/dwc/designware_i2s.c static int dw_i2s_probe(struct platform_device *pdev) · line 347
Quoted 8 lines
sound/soc/dwc/designware_i2s.c +0
-5@@ -340,11 +397,6 @@ static int dw_i2s_probe(struct platform_device *pdev)unsigned int cap;struct snd_soc_dai_driver *dw_i2s_dai; -if (!pdata) { -dev_err (&pdev->dev,"Invalid platform data\n" ); -return -EINVAL ; - } -
This check is required..
Quoted 4 lines
sound/soc/dwc/designware_i2s.c +0
-0dev =devm_kzalloc (&pdev->dev,sizeof (*dev),GFP_KERNEL );if (!dev) {dev_warn (&pdev->dev,"kzalloc fail\n" );
sound/soc/dwc/designware_i2s.c static int dw_i2s_probe(struct platform_device *pdev) · line 431
Quoted 10 lines
sound/soc/dwc/designware_i2s.c +4
-3@@ -373,47 +425,114 @@ static int dw_i2s_probe(struct platform_device *pdev)return PTR_ERR (dev->i2s_base); } - cap = pdata->cap; - dev->capability = cap; - dev->i2s_clk_cfg = pdata->i2s_clk_cfg; +if (pdata) { + dev->using_pd = true; + cap = pdata->cap; + dev->capability = cap;
We should take take capability from DT, as
if (of_get_property(np, "play", NULL))
cap |= DWC_I2S_PLAY;
if (of_get_property(np, "record", NULL))
cap |= DWC_I2S_RECORD;sound/soc/dwc/designware_i2s.c static int dw_i2s_probe(struct platform_device *pdev) · line 440
Quoted 10 lines
sound/soc/dwc/designware_i2s.c +8
-1+ dev->i2s_clk_cfg = pdata->i2s_clk_cfg; +if (!dev->i2s_clk_cfg) { +dev_err (&pdev->dev,"no clock device\n" ); +return -ENODEV ; + } -/* Set DMA slaves info */ +/* Set DMA slaves info */ + + dev->play_dma_data.pd.data = pdata->play_dma_data;
As pdata null check is removed it may create issue here !
Quoted 51 lines
sound/soc/dwc/designware_i2s.c +40
-10+ 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; + + dev->clk =clk_get (&pdev->dev,NULL ); +if (IS_ERR (dev->clk)) +return PTR_ERR (dev->clk); + +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; + } - 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 (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; + } + }else { +/* +* 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; +
sound/soc/dwc/designware_i2s.c static int dw_i2s_probe(struct platform_device *pdev) · line 491
Quoted 10 lines
sound/soc/dwc/designware_i2s.c +10
-0+ dev->using_pd = false; + + dev->clk =devm_clk_get (&pdev->dev,"i2sclk" ); +if (IS_ERR (dev->clk)) +return PTR_ERR (dev->clk); + +/* +* Code presumes all channels are configured with the same +* word size. +*/
But why all channels ? You should configure only those channel which are required. The number of channel you can pass from either pdata or DT.
This is also not required as before play start you are going to enable only those channel which are required and disable others.
Best Regards
Rajeev
Quoted 110 lines
sound/soc/dwc/designware_i2s.c +59
-20+if (COMP1_TX_ENABLED (comp1)) { +dev_dbg (&pdev->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 (&pdev->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 ; + } + } - dev->clk =clk_get (&pdev->dev,NULL ); -if (IS_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 ) 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,@@ -423,6 +542,15 @@ static int dw_i2s_probe(struct platform_device *pdev) goto err_clk_disable; } +if (!dev->using_pd) { + 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:@@ -443,12 +571,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 }, }; --1.7.1
Quoted 138 lines · rajeev kumar
Sorry for the last blank mail.
Quoted 135 lines · Andrew Jackson
Convert to driver to use either
platform_dataor device-tree for configuration of the device. When using device-tree, the I2S block's configuration is read from the relevant registers: this reduces the amount of information required in the device tree.Signed-off-by Andrew Jackson <[email protected]>--- .../devicetree/bindings/sound/designware-i2s.txt | 32 +++ sound/soc/dwc/Kconfig | 1 + sound/soc/dwc/designware_i2s.c | 238 ++++++++++++++++---- 3 files changed, 222 insertions(+), 49 deletions(-) create mode 100644 Documentation/devicetree/bindings/sound/designware-i2s.txtDocumentation/devicetree/bindings/sound/designware-i2s.txt new file +32
-0@@ -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 at 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"; + };sound/soc/dwc/Kconfig +1
-0@@ -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 uptosound/soc/dwc/designware_i2s.c +31
-3@@ -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,19 +55,46 @@#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;int active;unsigned int capability;struct device *dev; +bool using_pd;/* 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 +181,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 ))@@ -227,8 +255,7 @@ static int dw_i2s_hw_params(struct snd_pcm_substream *substream,
Quoted 9 lines
sound/soc/dwc/designware_i2s.c static int dw_i2s_hw_params(struct snd_pcm_substream *substream, · line 258
Quoted 5 lines
sound/soc/dwc/designware_i2s.c +1
-2i2s_disable_channels (dev, substream->stream); -/* Iterate over set of channels - independently controlled. -*/ +/* Iterate over set of channels - independently controlled. */ I dont think so this iteration is required as there are independent channels available.
See other email.
Quoted 66 lines
I don't have the h/w to test this patch. Can anyone test this ?
Quoted 64 lines
sound/soc/dwc/designware_i2s.c +36
-6do { if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) { i2s_write_reg(dev->i2s_base, TCR(ch_reg),@@ -251,13 +278,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->using_pd) { + 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 ;@@ -331,6 +363,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 int dw_i2s_probe (struct platform_device *pdev) {const struct i2s_platform_data *pdata = pdev->dev.platform_data;@@ -340,11 +397,6 @@ static int dw_i2s_probe(struct platform_device *pdev)unsigned int cap;struct snd_soc_dai_driver *dw_i2s_dai;
Quoted 7 lines
sound/soc/dwc/designware_i2s.c static int dw_i2s_probe(struct platform_device *pdev) · line 347
Quoted 5 lines
sound/soc/dwc/designware_i2s.c +0
-5-if (!pdata) { -dev_err (&pdev->dev,"Invalid platform data\n" ); -return -EINVAL ; - } -This check is required..
It is handled below: the absence of platform data (now) implies a device tree implementation.
Quoted 29 lines
Quoted 5 lines
sound/soc/dwc/designware_i2s.c +0
-0dev =devm_kzalloc (&pdev->dev,sizeof (*dev),GFP_KERNEL );if (!dev) {dev_warn (&pdev->dev,"kzalloc fail\n" );sound/soc/dwc/designware_i2s.c static int dw_i2s_probe(struct platform_device *pdev) · line 431
Quoted 10 lines
sound/soc/dwc/designware_i2s.c +4
-3@@ -373,47 +425,114 @@ static int dw_i2s_probe(struct platform_device *pdev)return PTR_ERR (dev->i2s_base); } - cap = pdata->cap; - dev->capability = cap; - dev->i2s_clk_cfg = pdata->i2s_clk_cfg; +if (pdata) { + dev->using_pd = true; + cap = pdata->cap; + dev->capability = cap;We should take take capability from DT, as
if (of_get_property(np, "play", NULL)) cap |= DWC_I2S_PLAY; if (of_get_property(np, "record", NULL)) cap |= DWC_I2S_RECORD;Quoted 6 lines
sound/soc/dwc/designware_i2s.c +5
-0+ dev->i2s_clk_cfg = pdata->i2s_clk_cfg; +if (!dev->i2s_clk_cfg) { +dev_err (&pdev->dev,"no clock device\n" ); +return -ENODEV ; + }
Quoted 6 lines
sound/soc/dwc/designware_i2s.c static int dw_i2s_probe(struct platform_device *pdev) · line 440
Quoted 4 lines
sound/soc/dwc/designware_i2s.c +3
-1-/* Set DMA slaves info */ +/* Set DMA slaves info */ + + dev->play_dma_data.pd.data = pdata->play_dma_data;As pdata null check is removed it may create issue here !
No, because we're in the 'if (pdata)' section of code.
Quoted 60 lines
Quoted 60 lines
sound/soc/dwc/designware_i2s.c +48
-10+ 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; + + dev->clk =clk_get (&pdev->dev,NULL ); +if (IS_ERR (dev->clk)) +return PTR_ERR (dev->clk); + +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; + } - 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 (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; + } + }else { +/* +* 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; + + dev->using_pd = false; + + dev->clk =devm_clk_get (&pdev->dev,"i2sclk" ); +if (IS_ERR (dev->clk)) +return PTR_ERR (dev->clk); + +/* +* Code presumes all channels are configured with the same
Quoted 8 lines
sound/soc/dwc/designware_i2s.c static int dw_i2s_probe(struct platform_device *pdev) · line 491
Quoted 2 lines
sound/soc/dwc/designware_i2s.c +2
-0+* word size. +*/ But why all channels ? You should configure only those channel which are required. The number of channel you can pass from either pdata or DT.
This is also not required as before play start you are going to enable only those channel which are required and disable others.
I will change the comment to say "configured in the IP" i.e. at design time. This is the physical configuration of the IP block.
Andrew
Quoted 116 lines
Best Regards
RajeevQuoted 110 lines
sound/soc/dwc/designware_i2s.c +59
-20+if (COMP1_TX_ENABLED (comp1)) { +dev_dbg (&pdev->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 (&pdev->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 ; + } + } - dev->clk =clk_get (&pdev->dev,NULL ); -if (IS_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 ) 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,@@ -423,6 +542,15 @@ static int dw_i2s_probe(struct platform_device *pdev) goto err_clk_disable; } +if (!dev->using_pd) { + 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:@@ -443,12 +571,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 }, }; --1.7.1