[PATCH 1/3] ASoC: ux500_pcm: Stop pretending that we support varying address widths

[PATCH 1/3] ASoC: ux500_pcm: Stop pretending that we support varying address widths

[Lee Jones]

The Slave Config's addr_width attribute is populated by data_width of
dma_cfg, which in turn is derived from dma_params' data_size attribute
and that comes from the slot_width which is always 16 bits (2 Bytes).
We're cutting out the middle man here and just setting the DMA Slave
Config directly.

Cc: alsa-devel at alsa-project.org
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 sound/soc/ux500/ux500_pcm.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/sound/soc/ux500/ux500_pcm.c b/sound/soc/ux500/ux500_pcm.c
index 6162f70..2f1bdb7 100644
--- a/sound/soc/ux500/ux500_pcm.c
+++ b/sound/soc/ux500/ux500_pcm.c
@@ -104,26 +104,25 @@ static int ux500_pcm_prepare_slave_config(struct snd_pcm_substream *substream,
 		struct dma_slave_config *slave_config)
 {
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
-	struct ux500_msp_dma_params *dma_params;
-	struct stedma40_chan_cfg *dma_cfg;
+	struct snd_dmaengine_dai_dma_data *dma_params;
 	int ret;
 
 	dma_params = snd_soc_dai_get_dma_data(rtd->cpu_dai, substream);
-	dma_cfg = dma_params->dma_cfg;
 
 	ret = snd_hwparams_to_dma_slave_config(substream, params, slave_config);
 	if (ret)
 		return ret;
 
 	slave_config->dst_maxburst = 4;
-	slave_config->dst_addr_width = dma_cfg->dst_info.data_width;
 	slave_config->src_maxburst = 4;
-	slave_config->src_addr_width = dma_cfg->src_info.data_width;
+
+	slave_config->src_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
+	slave_config->dst_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
 
 	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
-		slave_config->dst_addr = dma_params->tx_rx_addr;
+		slave_config->dst_addr = dma_params->addr;
 	else
-		slave_config->src_addr = dma_params->tx_rx_addr;
+		slave_config->src_addr = dma_params->addr;
 
 	return 0;
 }
-- 
1.8.3.2

[PATCH 2/3] ASoC: ux500_pcm: Differentiate between pdata and DT initialisation

[Lee Jones]

If booting with full DT support (i.e. DMA too, the last piece of the
puzzle), then we don't need to use the compatible request channel call
back. We also require slightly different flags to inform the core that
we are booting with DT.

Cc: alsa-devel at alsa-project.org
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 sound/soc/ux500/ux500_pcm.c | 26 +++++++++++++++++++++-----
 1 file changed, 21 insertions(+), 5 deletions(-)

diff --git a/sound/soc/ux500/ux500_pcm.c b/sound/soc/ux500/ux500_pcm.c
index 2f1bdb7..00cdd7b 100644
--- a/sound/soc/ux500/ux500_pcm.c
+++ b/sound/soc/ux500/ux500_pcm.c
@@ -134,15 +134,31 @@ static const struct snd_dmaengine_pcm_config ux500_dmaengine_pcm_config = {
 	.prepare_slave_config = ux500_pcm_prepare_slave_config,
 };
 
+static const struct snd_dmaengine_pcm_config ux500_dmaengine_of_pcm_config = {
+	.prepare_slave_config = ux500_pcm_prepare_slave_config,
+};
+
 int ux500_pcm_register_platform(struct platform_device *pdev)
 {
+	const struct snd_dmaengine_pcm_config *pcm_config;
+	struct device_node *np = pdev->dev.of_node;
+	unsigned int pcm_flags;
 	int ret;
 
-	ret = snd_dmaengine_pcm_register(&pdev->dev,
-			&ux500_dmaengine_pcm_config,
-			SND_DMAENGINE_PCM_FLAG_NO_RESIDUE |
-			SND_DMAENGINE_PCM_FLAG_COMPAT |
-			SND_DMAENGINE_PCM_FLAG_NO_DT);
+	if (np) {
+		pcm_config = &ux500_dmaengine_of_pcm_config;
+
+		pcm_flags = SND_DMAENGINE_PCM_FLAG_NO_RESIDUE |
+			SND_DMAENGINE_PCM_FLAG_COMPAT;
+	} else {
+		pcm_config = &ux500_dmaengine_pcm_config;
+
+		pcm_flags = SND_DMAENGINE_PCM_FLAG_NO_RESIDUE |
+			SND_DMAENGINE_PCM_FLAG_NO_DT |
+			SND_DMAENGINE_PCM_FLAG_COMPAT;
+	}
+
+	ret = snd_dmaengine_pcm_register(&pdev->dev, pcm_config, pcm_flags);
 	if (ret < 0) {
 		dev_err(&pdev->dev,
 			"%s: ERROR: Failed to register platform '%s' (%d)!\n",
-- 
1.8.3.2

[PATCH 3/3] ASoC: ux500: Dynamically fill DAI driver data on probe

[Lee Jones]

We no longer have a means to differentiate MSP devices at probe time,
mainline because we don't really have to. So rather than have an over-
sized static data structure in place, where the only difference between
devices is the ID and name (which are unused), we'll create them only
when required and omit the unnecessary information.

Cc: alsa-devel at alsa-project.org
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 sound/soc/ux500/ux500_msp_dai.c | 108 ++++++++++------------------------------
 1 file changed, 26 insertions(+), 82 deletions(-)

diff --git a/sound/soc/ux500/ux500_msp_dai.c b/sound/soc/ux500/ux500_msp_dai.c
index 8d1b01f..153fb3f 100644
--- a/sound/soc/ux500/ux500_msp_dai.c
+++ b/sound/soc/ux500/ux500_msp_dai.c
@@ -718,87 +718,20 @@ static struct snd_soc_dai_ops ux500_msp_dai_ops[] = {
 	}
 };
 
-static struct snd_soc_dai_driver ux500_msp_dai_drv[UX500_NBR_OF_DAI] = {
-	{
-		.name = "ux500-msp-i2s.0",
-		.probe = ux500_msp_dai_probe,
-		.id = 0,
-		.suspend = NULL,
-		.resume = NULL,
-		.playback = {
-			.channels_min = UX500_MSP_MIN_CHANNELS,
-			.channels_max = UX500_MSP_MAX_CHANNELS,
-			.rates = UX500_I2S_RATES,
-			.formats = UX500_I2S_FORMATS,
-		},
-		.capture = {
-			.channels_min = UX500_MSP_MIN_CHANNELS,
-			.channels_max = UX500_MSP_MAX_CHANNELS,
-			.rates = UX500_I2S_RATES,
-			.formats = UX500_I2S_FORMATS,
-		},
-		.ops = ux500_msp_dai_ops,
-	},
-	{
-		.name = "ux500-msp-i2s.1",
-		.probe = ux500_msp_dai_probe,
-		.id = 1,
-		.suspend = NULL,
-		.resume = NULL,
-		.playback = {
-			.channels_min = UX500_MSP_MIN_CHANNELS,
-			.channels_max = UX500_MSP_MAX_CHANNELS,
-			.rates = UX500_I2S_RATES,
-			.formats = UX500_I2S_FORMATS,
-		},
-		.capture = {
-			.channels_min = UX500_MSP_MIN_CHANNELS,
-			.channels_max = UX500_MSP_MAX_CHANNELS,
-			.rates = UX500_I2S_RATES,
-			.formats = UX500_I2S_FORMATS,
-		},
-		.ops = ux500_msp_dai_ops,
-	},
-	{
-		.name = "ux500-msp-i2s.2",
-		.id = 2,
-		.probe = ux500_msp_dai_probe,
-		.suspend = NULL,
-		.resume = NULL,
-		.playback = {
-			.channels_min = UX500_MSP_MIN_CHANNELS,
-			.channels_max = UX500_MSP_MAX_CHANNELS,
-			.rates = UX500_I2S_RATES,
-			.formats = UX500_I2S_FORMATS,
-		},
-		.capture = {
-			.channels_min = UX500_MSP_MIN_CHANNELS,
-			.channels_max = UX500_MSP_MAX_CHANNELS,
-			.rates = UX500_I2S_RATES,
-			.formats = UX500_I2S_FORMATS,
-		},
-		.ops = ux500_msp_dai_ops,
-	},
-	{
-		.name = "ux500-msp-i2s.3",
-		.probe = ux500_msp_dai_probe,
-		.id = 3,
-		.suspend = NULL,
-		.resume = NULL,
-		.playback = {
-			.channels_min = UX500_MSP_MIN_CHANNELS,
-			.channels_max = UX500_MSP_MAX_CHANNELS,
-			.rates = UX500_I2S_RATES,
-			.formats = UX500_I2S_FORMATS,
-		},
-		.capture = {
-			.channels_min = UX500_MSP_MIN_CHANNELS,
-			.channels_max = UX500_MSP_MAX_CHANNELS,
-			.rates = UX500_I2S_RATES,
-			.formats = UX500_I2S_FORMATS,
-		},
-		.ops = ux500_msp_dai_ops,
-	},
+void ux500_msp_populate_dai_drv(struct snd_soc_dai_driver *ux500_msp_dai_drv)
+{
+	ux500_msp_dai_drv->probe                 = ux500_msp_dai_probe;
+	ux500_msp_dai_drv->suspend               = NULL;
+	ux500_msp_dai_drv->resume                = NULL;
+	ux500_msp_dai_drv->playback.channels_min = UX500_MSP_MIN_CHANNELS;
+	ux500_msp_dai_drv->playback.channels_max = UX500_MSP_MAX_CHANNELS;
+	ux500_msp_dai_drv->playback.rates        = UX500_I2S_RATES;
+	ux500_msp_dai_drv->playback.formats      = UX500_I2S_FORMATS;
+	ux500_msp_dai_drv->capture.channels_min  = UX500_MSP_MIN_CHANNELS;
+	ux500_msp_dai_drv->capture.channels_max  = UX500_MSP_MAX_CHANNELS;
+	ux500_msp_dai_drv->capture.rates         = UX500_I2S_RATES;
+	ux500_msp_dai_drv->capture.formats       = UX500_I2S_FORMATS;
+	ux500_msp_dai_drv->ops                   = ux500_msp_dai_ops;
 };
 
 static const struct snd_soc_component_driver ux500_msp_component = {
@@ -809,6 +742,7 @@ static const struct snd_soc_component_driver ux500_msp_component = {
 static int ux500_msp_drv_probe(struct platform_device *pdev)
 {
 	struct ux500_msp_i2s_drvdata *drvdata;
+	struct snd_soc_dai_driver *ux500_msp_dai_drv;
 	int ret = 0;
 
 	dev_dbg(&pdev->dev, "%s: Enter (pdev->name = %s).\n", __func__,
@@ -863,8 +797,18 @@ static int ux500_msp_drv_probe(struct platform_device *pdev)
 	}
 	dev_set_drvdata(&pdev->dev, drvdata);
 
+	ux500_msp_dai_drv = devm_kzalloc(&pdev->dev,
+					 sizeof(*ux500_msp_dai_drv),
+					 GFP_KERNEL);
+	if (!ux500_msp_dai_drv) {
+		ret = -ENOMEM;
+		goto err_init_msp;
+	}
+
+	ux500_msp_populate_dai_drv(ux500_msp_dai_drv);
+
 	ret = snd_soc_register_component(&pdev->dev, &ux500_msp_component,
-					 &ux500_msp_dai_drv[drvdata->msp->id], 1);
+					 ux500_msp_dai_drv, 1);
 	if (ret < 0) {
 		dev_err(&pdev->dev, "Error: %s: Failed to register MSP%d!\n",
 			__func__, drvdata->msp->id);
-- 
1.8.3.2

[alsa-devel] [PATCH 3/3] ASoC: ux500: Dynamically fill DAI driver data on probe

[Lars-Peter Clausen]

On 12/02/2013 07:00 PM, Lee Jones wrote:
[...]
> +void ux500_msp_populate_dai_drv(struct snd_soc_dai_driver *ux500_msp_dai_drv)
> +{
> +	ux500_msp_dai_drv->probe                 = ux500_msp_dai_probe;
> +	ux500_msp_dai_drv->suspend               = NULL;
> +	ux500_msp_dai_drv->resume                = NULL;
> +	ux500_msp_dai_drv->playback.channels_min = UX500_MSP_MIN_CHANNELS;
> +	ux500_msp_dai_drv->playback.channels_max = UX500_MSP_MAX_CHANNELS;
> +	ux500_msp_dai_drv->playback.rates        = UX500_I2S_RATES;
> +	ux500_msp_dai_drv->playback.formats      = UX500_I2S_FORMATS;
> +	ux500_msp_dai_drv->capture.channels_min  = UX500_MSP_MIN_CHANNELS;
> +	ux500_msp_dai_drv->capture.channels_max  = UX500_MSP_MAX_CHANNELS;
> +	ux500_msp_dai_drv->capture.rates         = UX500_I2S_RATES;
> +	ux500_msp_dai_drv->capture.formats       = UX500_I2S_FORMATS;
> +	ux500_msp_dai_drv->ops                   = ux500_msp_dai_ops;
>  };

You can just use the same static driver for all devices. No need to
dynamically allocate it.

>  
>  static const struct snd_soc_component_driver ux500_msp_component = {
> @@ -809,6 +742,7 @@ static const struct snd_soc_component_driver ux500_msp_component = {
>  static int ux500_msp_drv_probe(struct platform_device *pdev)
>  {
>  	struct ux500_msp_i2s_drvdata *drvdata;
> +	struct snd_soc_dai_driver *ux500_msp_dai_drv;
>  	int ret = 0;
>  
>  	dev_dbg(&pdev->dev, "%s: Enter (pdev->name = %s).\n", __func__,
> @@ -863,8 +797,18 @@ static int ux500_msp_drv_probe(struct platform_device *pdev)
>  	}
>  	dev_set_drvdata(&pdev->dev, drvdata);
>  
> +	ux500_msp_dai_drv = devm_kzalloc(&pdev->dev,
> +					 sizeof(*ux500_msp_dai_drv),
> +					 GFP_KERNEL);
> +	if (!ux500_msp_dai_drv) {
> +		ret = -ENOMEM;
> +		goto err_init_msp;
> +	}
> +
> +	ux500_msp_populate_dai_drv(ux500_msp_dai_drv);
> +
>  	ret = snd_soc_register_component(&pdev->dev, &ux500_msp_component,
> -					 &ux500_msp_dai_drv[drvdata->msp->id], 1);
> +					 ux500_msp_dai_drv, 1);
>  	if (ret < 0) {
>  		dev_err(&pdev->dev, "Error: %s: Failed to register MSP%d!\n",
>  			__func__, drvdata->msp->id);
> 

[alsa-devel] [PATCH 2/3] ASoC: ux500_pcm: Differentiate between pdata and DT initialisation

[Lars-Peter Clausen]

On 12/02/2013 07:00 PM, Lee Jones wrote:
> If booting with full DT support (i.e. DMA too, the last piece of the
> puzzle), then we don't need to use the compatible request channel call
> back. We also require slightly different flags to inform the core that
> we are booting with DT.

I don't think you need differentiate between DT and non-DT here.
If the SND_DMAENGINE_PCM_FLAG_COMPAT is set and the
SND_DMAENGINE_PCM_FLAG_NO_DT is not set it will first try to request the
channels from the DT if that fails it will fallback to the compat path. So
all this patch needs to do is remove the SND_DMAENGINE_PCM_FLAG_NO_DT, that
should be it.

> 
> Cc: alsa-devel at alsa-project.org
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---
>  sound/soc/ux500/ux500_pcm.c | 26 +++++++++++++++++++++-----
>  1 file changed, 21 insertions(+), 5 deletions(-)
> 
> diff --git a/sound/soc/ux500/ux500_pcm.c b/sound/soc/ux500/ux500_pcm.c
> index 2f1bdb7..00cdd7b 100644
> --- a/sound/soc/ux500/ux500_pcm.c
> +++ b/sound/soc/ux500/ux500_pcm.c
> @@ -134,15 +134,31 @@ static const struct snd_dmaengine_pcm_config ux500_dmaengine_pcm_config = {
>  	.prepare_slave_config = ux500_pcm_prepare_slave_config,
>  };
>  
> +static const struct snd_dmaengine_pcm_config ux500_dmaengine_of_pcm_config = {
> +	.prepare_slave_config = ux500_pcm_prepare_slave_config,
> +};
> +
>  int ux500_pcm_register_platform(struct platform_device *pdev)
>  {
> +	const struct snd_dmaengine_pcm_config *pcm_config;
> +	struct device_node *np = pdev->dev.of_node;
> +	unsigned int pcm_flags;
>  	int ret;
>  
> -	ret = snd_dmaengine_pcm_register(&pdev->dev,
> -			&ux500_dmaengine_pcm_config,
> -			SND_DMAENGINE_PCM_FLAG_NO_RESIDUE |
> -			SND_DMAENGINE_PCM_FLAG_COMPAT |
> -			SND_DMAENGINE_PCM_FLAG_NO_DT);
> +	if (np) {
> +		pcm_config = &ux500_dmaengine_of_pcm_config;
> +
> +		pcm_flags = SND_DMAENGINE_PCM_FLAG_NO_RESIDUE |
> +			SND_DMAENGINE_PCM_FLAG_COMPAT;
> +	} else {
> +		pcm_config = &ux500_dmaengine_pcm_config;
> +
> +		pcm_flags = SND_DMAENGINE_PCM_FLAG_NO_RESIDUE |
> +			SND_DMAENGINE_PCM_FLAG_NO_DT |
> +			SND_DMAENGINE_PCM_FLAG_COMPAT;
> +	}
> +
> +	ret = snd_dmaengine_pcm_register(&pdev->dev, pcm_config, pcm_flags);
>  	if (ret < 0) {
>  		dev_err(&pdev->dev,
>  			"%s: ERROR: Failed to register platform '%s' (%d)!\n",
> 

[alsa-devel] [PATCH 1/3] ASoC: ux500_pcm: Stop pretending that we support varying address widths

[Lars-Peter Clausen]

On 12/02/2013 07:00 PM, Lee Jones wrote:
> The Slave Config's addr_width attribute is populated by data_width of
> dma_cfg, which in turn is derived from dma_params' data_size attribute
> and that comes from the slot_width which is always 16 bits (2 Bytes).
> We're cutting out the middle man here and just setting the DMA Slave
> Config directly.
> 
> Cc: alsa-devel at alsa-project.org
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

Patch looks good. But as a follow up patch I think you can now switch to the
generic snd_dmaengine_pcm_prepare_slave_config.

> ---
>  sound/soc/ux500/ux500_pcm.c | 13 ++++++-------
>  1 file changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/sound/soc/ux500/ux500_pcm.c b/sound/soc/ux500/ux500_pcm.c
> index 6162f70..2f1bdb7 100644
> --- a/sound/soc/ux500/ux500_pcm.c
> +++ b/sound/soc/ux500/ux500_pcm.c
> @@ -104,26 +104,25 @@ static int ux500_pcm_prepare_slave_config(struct snd_pcm_substream *substream,
>  		struct dma_slave_config *slave_config)
>  {
>  	struct snd_soc_pcm_runtime *rtd = substream->private_data;
> -	struct ux500_msp_dma_params *dma_params;
> -	struct stedma40_chan_cfg *dma_cfg;
> +	struct snd_dmaengine_dai_dma_data *dma_params;
>  	int ret;
>  
>  	dma_params = snd_soc_dai_get_dma_data(rtd->cpu_dai, substream);
> -	dma_cfg = dma_params->dma_cfg;
>  
>  	ret = snd_hwparams_to_dma_slave_config(substream, params, slave_config);
>  	if (ret)
>  		return ret;
>  
>  	slave_config->dst_maxburst = 4;
> -	slave_config->dst_addr_width = dma_cfg->dst_info.data_width;
>  	slave_config->src_maxburst = 4;
> -	slave_config->src_addr_width = dma_cfg->src_info.data_width;
> +
> +	slave_config->src_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
> +	slave_config->dst_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
>  
>  	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
> -		slave_config->dst_addr = dma_params->tx_rx_addr;
> +		slave_config->dst_addr = dma_params->addr;
>  	else
> -		slave_config->src_addr = dma_params->tx_rx_addr;
> +		slave_config->src_addr = dma_params->addr;
>  
>  	return 0;
>  }
> 

[PATCH 1/3] ASoC: ux500_pcm: Stop pretending that we support varying address widths

[Mark Brown]

On Mon, Dec 02, 2013 at 06:00:20PM +0000, Lee Jones wrote:
> The Slave Config's addr_width attribute is populated by data_width of
> dma_cfg, which in turn is derived from dma_params' data_size attribute
> and that comes from the slot_width which is always 16 bits (2 Bytes).
> We're cutting out the middle man here and just setting the DMA Slave
> Config directly.

Applied, thanks.
-------------- 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/20131202/c6d196b8/attachment-0001.sig>

[alsa-devel] [PATCH 1/3] ASoC: ux500_pcm: Stop pretending that we support varying address widths

[Lee Jones]

On Mon, 02 Dec 2013, Lars-Peter Clausen wrote:

> On 12/02/2013 07:00 PM, Lee Jones wrote:
> > The Slave Config's addr_width attribute is populated by data_width of
> > dma_cfg, which in turn is derived from dma_params' data_size attribute
> > and that comes from the slot_width which is always 16 bits (2 Bytes).
> > We're cutting out the middle man here and just setting the DMA Slave
> > Config directly.
> > 
> > Cc: alsa-devel at alsa-project.org
> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> 
> Patch looks good. But as a follow up patch I think you can now switch to the
> generic snd_dmaengine_pcm_prepare_slave_config.

No, I don't think we can. Your version only populates the DMA settings
for the correct stream (Playback OR Capture), but our DMA Controller
requires both source and destination to be setup.

> > ---
> >  sound/soc/ux500/ux500_pcm.c | 13 ++++++-------
> >  1 file changed, 6 insertions(+), 7 deletions(-)
> > 
> > diff --git a/sound/soc/ux500/ux500_pcm.c b/sound/soc/ux500/ux500_pcm.c
> > index 6162f70..2f1bdb7 100644
> > --- a/sound/soc/ux500/ux500_pcm.c
> > +++ b/sound/soc/ux500/ux500_pcm.c
> > @@ -104,26 +104,25 @@ static int ux500_pcm_prepare_slave_config(struct snd_pcm_substream *substream,
> >  		struct dma_slave_config *slave_config)
> >  {
> >  	struct snd_soc_pcm_runtime *rtd = substream->private_data;
> > -	struct ux500_msp_dma_params *dma_params;
> > -	struct stedma40_chan_cfg *dma_cfg;
> > +	struct snd_dmaengine_dai_dma_data *dma_params;
> >  	int ret;
> >  
> >  	dma_params = snd_soc_dai_get_dma_data(rtd->cpu_dai, substream);
> > -	dma_cfg = dma_params->dma_cfg;
> >  
> >  	ret = snd_hwparams_to_dma_slave_config(substream, params, slave_config);
> >  	if (ret)
> >  		return ret;
> >  
> >  	slave_config->dst_maxburst = 4;
> > -	slave_config->dst_addr_width = dma_cfg->dst_info.data_width;
> >  	slave_config->src_maxburst = 4;
> > -	slave_config->src_addr_width = dma_cfg->src_info.data_width;
> > +
> > +	slave_config->src_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
> > +	slave_config->dst_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
> >  
> >  	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
> > -		slave_config->dst_addr = dma_params->tx_rx_addr;
> > +		slave_config->dst_addr = dma_params->addr;
> >  	else
> > -		slave_config->src_addr = dma_params->tx_rx_addr;
> > +		slave_config->src_addr = dma_params->addr;
> >  
> >  	return 0;
> >  }
> > 
> 

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[alsa-devel] [PATCH 2/3] ASoC: ux500_pcm: Differentiate between pdata and DT initialisation

[Lee Jones]

On Mon, 02 Dec 2013, Lars-Peter Clausen wrote:

> On 12/02/2013 07:00 PM, Lee Jones wrote:
> > If booting with full DT support (i.e. DMA too, the last piece of the
> > puzzle), then we don't need to use the compatible request channel call
> > back. We also require slightly different flags to inform the core that
> > we are booting with DT.
> 
> I don't think you need differentiate between DT and non-DT here.
> If the SND_DMAENGINE_PCM_FLAG_COMPAT is set and the
> SND_DMAENGINE_PCM_FLAG_NO_DT is not set it will first try to request the
> channels from the DT if that fails it will fallback to the compat path. So
> all this patch needs to do is remove the SND_DMAENGINE_PCM_FLAG_NO_DT, that
> should be it.

I have no way of testing that, as we are currently 80% through
shutting down platform data altogether. In fact, I think there is only
ASoC to be converted. Most of this stuff is going to be ripped out
within the next couple of weeks in any case. It will happen as soon as
I can find an hour or two to work on it.

For that reason I didn't want to disturb the current semantics. I've
already found that a bunch of he generic (oh, you can just use the
central) functions (for that) don't actually work for us for one
reason or another.

I plan to a) convert everything over to DT only and b) try to make the
generic functions work for us, so we can eliminate yet more code in
the very near future.

> > Cc: alsa-devel at alsa-project.org
> > Acked-by: Linus Walleij <linus.walleij@linaro.org>
> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> > ---
> >  sound/soc/ux500/ux500_pcm.c | 26 +++++++++++++++++++++-----
> >  1 file changed, 21 insertions(+), 5 deletions(-)
> > 
> > diff --git a/sound/soc/ux500/ux500_pcm.c b/sound/soc/ux500/ux500_pcm.c
> > index 2f1bdb7..00cdd7b 100644
> > --- a/sound/soc/ux500/ux500_pcm.c
> > +++ b/sound/soc/ux500/ux500_pcm.c
> > @@ -134,15 +134,31 @@ static const struct snd_dmaengine_pcm_config ux500_dmaengine_pcm_config = {
> >  	.prepare_slave_config = ux500_pcm_prepare_slave_config,
> >  };
> >  
> > +static const struct snd_dmaengine_pcm_config ux500_dmaengine_of_pcm_config = {
> > +	.prepare_slave_config = ux500_pcm_prepare_slave_config,
> > +};
> > +
> >  int ux500_pcm_register_platform(struct platform_device *pdev)
> >  {
> > +	const struct snd_dmaengine_pcm_config *pcm_config;
> > +	struct device_node *np = pdev->dev.of_node;
> > +	unsigned int pcm_flags;
> >  	int ret;
> >  
> > -	ret = snd_dmaengine_pcm_register(&pdev->dev,
> > -			&ux500_dmaengine_pcm_config,
> > -			SND_DMAENGINE_PCM_FLAG_NO_RESIDUE |
> > -			SND_DMAENGINE_PCM_FLAG_COMPAT |
> > -			SND_DMAENGINE_PCM_FLAG_NO_DT);
> > +	if (np) {
> > +		pcm_config = &ux500_dmaengine_of_pcm_config;
> > +
> > +		pcm_flags = SND_DMAENGINE_PCM_FLAG_NO_RESIDUE |
> > +			SND_DMAENGINE_PCM_FLAG_COMPAT;
> > +	} else {
> > +		pcm_config = &ux500_dmaengine_pcm_config;
> > +
> > +		pcm_flags = SND_DMAENGINE_PCM_FLAG_NO_RESIDUE |
> > +			SND_DMAENGINE_PCM_FLAG_NO_DT |
> > +			SND_DMAENGINE_PCM_FLAG_COMPAT;
> > +	}
> > +
> > +	ret = snd_dmaengine_pcm_register(&pdev->dev, pcm_config, pcm_flags);
> >  	if (ret < 0) {
> >  		dev_err(&pdev->dev,
> >  			"%s: ERROR: Failed to register platform '%s' (%d)!\n",
> > 
> 

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[alsa-devel] [PATCH 3/3] ASoC: ux500: Dynamically fill DAI driver data on probe

[Lee Jones]

On Mon, 02 Dec 2013, Lars-Peter Clausen wrote:

> On 12/02/2013 07:00 PM, Lee Jones wrote:
> [...]
> > +void ux500_msp_populate_dai_drv(struct snd_soc_dai_driver *ux500_msp_dai_drv)
> > +{
> > +	ux500_msp_dai_drv->probe                 = ux500_msp_dai_probe;
> > +	ux500_msp_dai_drv->suspend               = NULL;
> > +	ux500_msp_dai_drv->resume                = NULL;
> > +	ux500_msp_dai_drv->playback.channels_min = UX500_MSP_MIN_CHANNELS;
> > +	ux500_msp_dai_drv->playback.channels_max = UX500_MSP_MAX_CHANNELS;
> > +	ux500_msp_dai_drv->playback.rates        = UX500_I2S_RATES;
> > +	ux500_msp_dai_drv->playback.formats      = UX500_I2S_FORMATS;
> > +	ux500_msp_dai_drv->capture.channels_min  = UX500_MSP_MIN_CHANNELS;
> > +	ux500_msp_dai_drv->capture.channels_max  = UX500_MSP_MAX_CHANNELS;
> > +	ux500_msp_dai_drv->capture.rates         = UX500_I2S_RATES;
> > +	ux500_msp_dai_drv->capture.formats       = UX500_I2S_FORMATS;
> > +	ux500_msp_dai_drv->ops                   = ux500_msp_dai_ops;
> >  };
> 
> You can just use the same static driver for all devices. No need to
> dynamically allocate it.

How do you mean? Just create a 'static struct' instead?

> >  static const struct snd_soc_component_driver ux500_msp_component = {
> > @@ -809,6 +742,7 @@ static const struct snd_soc_component_driver ux500_msp_component = {
> >  static int ux500_msp_drv_probe(struct platform_device *pdev)
> >  {
> >  	struct ux500_msp_i2s_drvdata *drvdata;
> > +	struct snd_soc_dai_driver *ux500_msp_dai_drv;
> >  	int ret = 0;
> >  
> >  	dev_dbg(&pdev->dev, "%s: Enter (pdev->name = %s).\n", __func__,
> > @@ -863,8 +797,18 @@ static int ux500_msp_drv_probe(struct platform_device *pdev)
> >  	}
> >  	dev_set_drvdata(&pdev->dev, drvdata);
> >  
> > +	ux500_msp_dai_drv = devm_kzalloc(&pdev->dev,
> > +					 sizeof(*ux500_msp_dai_drv),
> > +					 GFP_KERNEL);
> > +	if (!ux500_msp_dai_drv) {
> > +		ret = -ENOMEM;
> > +		goto err_init_msp;
> > +	}
> > +
> > +	ux500_msp_populate_dai_drv(ux500_msp_dai_drv);
> > +
> >  	ret = snd_soc_register_component(&pdev->dev, &ux500_msp_component,
> > -					 &ux500_msp_dai_drv[drvdata->msp->id], 1);
> > +					 ux500_msp_dai_drv, 1);
> >  	if (ret < 0) {
> >  		dev_err(&pdev->dev, "Error: %s: Failed to register MSP%d!\n",
> >  			__func__, drvdata->msp->id);
> > 
> 

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[alsa-devel] [PATCH 2/3] ASoC: ux500_pcm: Differentiate between pdata and DT initialisation

[Lars-Peter Clausen]

On 12/03/2013 09:36 AM, Lee Jones wrote:
> On Mon, 02 Dec 2013, Lars-Peter Clausen wrote:
>
>> On 12/02/2013 07:00 PM, Lee Jones wrote:
>>> If booting with full DT support (i.e. DMA too, the last piece of the
>>> puzzle), then we don't need to use the compatible request channel call
>>> back. We also require slightly different flags to inform the core that
>>> we are booting with DT.
>>
>> I don't think you need differentiate between DT and non-DT here.
>> If the SND_DMAENGINE_PCM_FLAG_COMPAT is set and the
>> SND_DMAENGINE_PCM_FLAG_NO_DT is not set it will first try to request the
>> channels from the DT if that fails it will fallback to the compat path. So
>> all this patch needs to do is remove the SND_DMAENGINE_PCM_FLAG_NO_DT, that
>> should be it.
>
> I have no way of testing that, as we are currently 80% through
> shutting down platform data altogether. In fact, I think there is only
> ASoC to be converted. Most of this stuff is going to be ripped out
> within the next couple of weeks in any case. It will happen as soon as
> I can find an hour or two to work on it.
>
> For that reason I didn't want to disturb the current semantics. I've
> already found that a bunch of he generic (oh, you can just use the
> central) functions (for that) don't actually work for us for one
> reason or another.

All SND_DMAENGINE_PCM_FLAG_NO_DT does is to skip trying to request the channel 
from the dt. If of_node is NULL this step will be skipped anyway. So only 
setting SND_DMAENGINE_PCM_FLAG_NO_DT if we already know that the node is NULL 
is redundant.

>
>>> Cc: alsa-devel at alsa-project.org
>>> Acked-by: Linus Walleij <linus.walleij@linaro.org>
>>> Signed-off-by: Lee Jones <lee.jones@linaro.org>
>>> ---
>>>   sound/soc/ux500/ux500_pcm.c | 26 +++++++++++++++++++++-----
>>>   1 file changed, 21 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/sound/soc/ux500/ux500_pcm.c b/sound/soc/ux500/ux500_pcm.c
>>> index 2f1bdb7..00cdd7b 100644
>>> --- a/sound/soc/ux500/ux500_pcm.c
>>> +++ b/sound/soc/ux500/ux500_pcm.c
>>> @@ -134,15 +134,31 @@ static const struct snd_dmaengine_pcm_config ux500_dmaengine_pcm_config = {
>>>   	.prepare_slave_config = ux500_pcm_prepare_slave_config,
>>>   };
>>>
>>> +static const struct snd_dmaengine_pcm_config ux500_dmaengine_of_pcm_config = {
>>> +	.prepare_slave_config = ux500_pcm_prepare_slave_config,
>>> +};
>>> +
>>>   int ux500_pcm_register_platform(struct platform_device *pdev)
>>>   {
>>> +	const struct snd_dmaengine_pcm_config *pcm_config;
>>> +	struct device_node *np = pdev->dev.of_node;
>>> +	unsigned int pcm_flags;
>>>   	int ret;
>>>
>>> -	ret = snd_dmaengine_pcm_register(&pdev->dev,
>>> -			&ux500_dmaengine_pcm_config,
>>> -			SND_DMAENGINE_PCM_FLAG_NO_RESIDUE |
>>> -			SND_DMAENGINE_PCM_FLAG_COMPAT |
>>> -			SND_DMAENGINE_PCM_FLAG_NO_DT);
>>> +	if (np) {
>>> +		pcm_config = &ux500_dmaengine_of_pcm_config;
>>> +
>>> +		pcm_flags = SND_DMAENGINE_PCM_FLAG_NO_RESIDUE |
>>> +			SND_DMAENGINE_PCM_FLAG_COMPAT;
>>> +	} else {
>>> +		pcm_config = &ux500_dmaengine_pcm_config;
>>> +
>>> +		pcm_flags = SND_DMAENGINE_PCM_FLAG_NO_RESIDUE |
>>> +			SND_DMAENGINE_PCM_FLAG_NO_DT |
>>> +			SND_DMAENGINE_PCM_FLAG_COMPAT;
>>> +	}
>>> +
>>> +	ret = snd_dmaengine_pcm_register(&pdev->dev, pcm_config, pcm_flags);
>>>   	if (ret < 0) {
>>>   		dev_err(&pdev->dev,
>>>   			"%s: ERROR: Failed to register platform '%s' (%d)!\n",
>>>
>>
>

[alsa-devel] [PATCH 3/3] ASoC: ux500: Dynamically fill DAI driver data on probe

[Lars-Peter Clausen]

On 12/03/2013 09:40 AM, Lee Jones wrote:
> On Mon, 02 Dec 2013, Lars-Peter Clausen wrote:
>
>> On 12/02/2013 07:00 PM, Lee Jones wrote:
>> [...]
>>> +void ux500_msp_populate_dai_drv(struct snd_soc_dai_driver *ux500_msp_dai_drv)
>>> +{
>>> +	ux500_msp_dai_drv->probe                 = ux500_msp_dai_probe;
>>> +	ux500_msp_dai_drv->suspend               = NULL;
>>> +	ux500_msp_dai_drv->resume                = NULL;
>>> +	ux500_msp_dai_drv->playback.channels_min = UX500_MSP_MIN_CHANNELS;
>>> +	ux500_msp_dai_drv->playback.channels_max = UX500_MSP_MAX_CHANNELS;
>>> +	ux500_msp_dai_drv->playback.rates        = UX500_I2S_RATES;
>>> +	ux500_msp_dai_drv->playback.formats      = UX500_I2S_FORMATS;
>>> +	ux500_msp_dai_drv->capture.channels_min  = UX500_MSP_MIN_CHANNELS;
>>> +	ux500_msp_dai_drv->capture.channels_max  = UX500_MSP_MAX_CHANNELS;
>>> +	ux500_msp_dai_drv->capture.rates         = UX500_I2S_RATES;
>>> +	ux500_msp_dai_drv->capture.formats       = UX500_I2S_FORMATS;
>>> +	ux500_msp_dai_drv->ops                   = ux500_msp_dai_ops;
>>>   };
>>
>> You can just use the same static driver for all devices. No need to
>> dynamically allocate it.
>
> How do you mean? Just create a 'static struct' instead?

Yes.

static struct snd_soc_dai_driver ux500_msp_dai_driver = {
	...
};

>
>>>   static const struct snd_soc_component_driver ux500_msp_component = {
>>> @@ -809,6 +742,7 @@ static const struct snd_soc_component_driver ux500_msp_component = {
>>>   static int ux500_msp_drv_probe(struct platform_device *pdev)
>>>   {
>>>   	struct ux500_msp_i2s_drvdata *drvdata;
>>> +	struct snd_soc_dai_driver *ux500_msp_dai_drv;
>>>   	int ret = 0;
>>>
>>>   	dev_dbg(&pdev->dev, "%s: Enter (pdev->name = %s).\n", __func__,
>>> @@ -863,8 +797,18 @@ static int ux500_msp_drv_probe(struct platform_device *pdev)
>>>   	}
>>>   	dev_set_drvdata(&pdev->dev, drvdata);
>>>
>>> +	ux500_msp_dai_drv = devm_kzalloc(&pdev->dev,
>>> +					 sizeof(*ux500_msp_dai_drv),
>>> +					 GFP_KERNEL);
>>> +	if (!ux500_msp_dai_drv) {
>>> +		ret = -ENOMEM;
>>> +		goto err_init_msp;
>>> +	}
>>> +
>>> +	ux500_msp_populate_dai_drv(ux500_msp_dai_drv);
>>> +
>>>   	ret = snd_soc_register_component(&pdev->dev, &ux500_msp_component,
>>> -					 &ux500_msp_dai_drv[drvdata->msp->id], 1);
>>> +					 ux500_msp_dai_drv, 1);
>>>   	if (ret < 0) {
>>>   		dev_err(&pdev->dev, "Error: %s: Failed to register MSP%d!\n",
>>>   			__func__, drvdata->msp->id);
>>>
>>
>

[alsa-devel] [PATCH 3/3] ASoC: ux500: Dynamically fill DAI driver data on probe

[Lee Jones]

> >>[...]
> >>>+void ux500_msp_populate_dai_drv(struct snd_soc_dai_driver *ux500_msp_dai_drv)
> >>>+{
> >>>+	ux500_msp_dai_drv->probe                 = ux500_msp_dai_probe;
> >>>+	ux500_msp_dai_drv->suspend               = NULL;
> >>>+	ux500_msp_dai_drv->resume                = NULL;
> >>>+	ux500_msp_dai_drv->playback.channels_min = UX500_MSP_MIN_CHANNELS;
> >>>+	ux500_msp_dai_drv->playback.channels_max = UX500_MSP_MAX_CHANNELS;
> >>>+	ux500_msp_dai_drv->playback.rates        = UX500_I2S_RATES;
> >>>+	ux500_msp_dai_drv->playback.formats      = UX500_I2S_FORMATS;
> >>>+	ux500_msp_dai_drv->capture.channels_min  = UX500_MSP_MIN_CHANNELS;
> >>>+	ux500_msp_dai_drv->capture.channels_max  = UX500_MSP_MAX_CHANNELS;
> >>>+	ux500_msp_dai_drv->capture.rates         = UX500_I2S_RATES;
> >>>+	ux500_msp_dai_drv->capture.formats       = UX500_I2S_FORMATS;
> >>>+	ux500_msp_dai_drv->ops                   = ux500_msp_dai_ops;
> >>>  };
> >>
> >>You can just use the same static driver for all devices. No need to
> >>dynamically allocate it.
> >
> >How do you mean? Just create a 'static struct' instead?
> 
> Yes.
> 
> static struct snd_soc_dai_driver ux500_msp_dai_driver = {
> 	...
> };

Yes, that's no issue. I was more focused on the reduction of code
than anything else, but that is a better idea.

I'll resubmit.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[alsa-devel] [PATCH 2/3] ASoC: ux500_pcm: Differentiate between pdata and DT initialisation

[Lee Jones]

On Tue, 03 Dec 2013, Lars-Peter Clausen wrote:

> On 12/03/2013 09:36 AM, Lee Jones wrote:
> >On Mon, 02 Dec 2013, Lars-Peter Clausen wrote:
> >
> >>On 12/02/2013 07:00 PM, Lee Jones wrote:
> >>>If booting with full DT support (i.e. DMA too, the last piece of the
> >>>puzzle), then we don't need to use the compatible request channel call
> >>>back. We also require slightly different flags to inform the core that
> >>>we are booting with DT.
> >>
> >>I don't think you need differentiate between DT and non-DT here.
> >>If the SND_DMAENGINE_PCM_FLAG_COMPAT is set and the
> >>SND_DMAENGINE_PCM_FLAG_NO_DT is not set it will first try to request the
> >>channels from the DT if that fails it will fallback to the compat path. So
> >>all this patch needs to do is remove the SND_DMAENGINE_PCM_FLAG_NO_DT, that
> >>should be it.
> >
> >I have no way of testing that, as we are currently 80% through
> >shutting down platform data altogether. In fact, I think there is only
> >ASoC to be converted. Most of this stuff is going to be ripped out
> >within the next couple of weeks in any case. It will happen as soon as
> >I can find an hour or two to work on it.
> >
> >For that reason I didn't want to disturb the current semantics. I've
> >already found that a bunch of he generic (oh, you can just use the
> >central) functions (for that) don't actually work for us for one
> >reason or another.
> 
> All SND_DMAENGINE_PCM_FLAG_NO_DT does is to skip trying to request
> the channel from the dt. If of_node is NULL this step will be
> skipped anyway. So only setting SND_DMAENGINE_PCM_FLAG_NO_DT if we
> already know that the node is NULL is redundant.

I'll just have to trust you on this and resubmit with the other patch.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog