[alsa-devel] [PATCH 00/14] ASoC: Cleanup dmaengine PCM users
Hi,
This series cleans up a few of the current users of the dmaengine pcm lib in order to make the introduction of a new more generic PCM dmaengine driver more straight forward.
The series consists mainly of two types of changes. One is removing all unnecessary wrappers of snd_soc_dmaengine_close(). For some reason quite a few drivers started providing their own pcm_close callback, which does nothing but calls snd_soc_dmaengine_close, although snd_soc_dmaengine_close can be used directly as the pcm_close callback if nothing else needs to be done.
The other is getting rid of snd_dmaengine_pcm_{get,set}_data(). These two functions were initially introduced to deal with a few oddball dmaengine drivers which require that data is passed to them via the private_data field of the channel and we had to keep the data allocated until the channel had been released. A few other driver started using these functions although there is no need to do so. E.g. some drivers just call snd_dmaengine_pcm_set_data(), but never attempt to read the data again using snd_dmaengine_pcm_get_data(). For the drivers, for which snd_dmaengine_pcm_{set,get}_data() was originally introduced, the responsibility of keeping the dma config allocated is shifted from the pcm driver to the i2s drivers. Which in turn makes the code much more straight forward since for most i2s drivers the dma config is either static or setup once at driver probe time.
The series has only been compile tested, so please test these patches, especially the non trivial ones for ep93xx, mmp, imx and mxs.
- Lars
Lars-Peter Clausen (14): ASoC: ux500_pcm: Remove duplicated SNDRV_PCM_HW_PARAM_PERIODS constraint ASoC: spear_pcm: No need to wrap snd_dmaengine_pcm_close() ASoC: omap-pcm: No need to wrap snd_dmaengine_pcm_close() ASoC: tegra_pcm: No need to wrap snd_dmaengine_pcm_close() ASoC: ux500_pcm: No need to wrap snd_dmaengine_pcm_close() ASoC: atmel-pcm-dma: No need to wrap snd_dmaengine_pcm_close() ASoC: ux500_pcm: No need to use snd_dmaengine_pcm_set_data() ASoC: speaer_pcm: No need to use snd_dmaengine_pcm_set_data() ASoC: atmel-pcm-dma: Do not use snd_dmaengine_pcm_{set,get}_data() ASoC: ep93xx: Use ep93xx_dma_params instead of ep93xx_pcm_dma_params ASoC: mmp-pcm: Allocate dma filter parameters on the stack ASoC: imx-pcm: Embed the imx_dma_data struct in the dma_params struct ASoC: mxs: Embed the mxs_dma_data struct in the mxs_pcm_dma_params struct ASoC: dmaengine-pcm: Remove snd_dmaengine_pcm_{set,get}_data
include/sound/dmaengine_pcm.h | 3 --- sound/soc/atmel/atmel-pcm-dma.c | 23 +++++++--------------- sound/soc/cirrus/edb93xx.c | 1 - sound/soc/cirrus/ep93xx-ac97.c | 9 +++++---- sound/soc/cirrus/ep93xx-i2s.c | 12 ++++++------ sound/soc/cirrus/ep93xx-pcm.c | 37 +++-------------------------------- sound/soc/cirrus/ep93xx-pcm.h | 20 ------------------- sound/soc/cirrus/simone.c | 2 -- sound/soc/cirrus/snappercl15.c | 1 - sound/soc/fsl/fsl_ssi.c | 17 ++++++++-------- sound/soc/fsl/imx-pcm-dma.c | 35 ++------------------------------- sound/soc/fsl/imx-pcm.h | 17 ++++++++++++++-- sound/soc/fsl/imx-ssi.c | 12 ++++++++---- sound/soc/mxs/mxs-pcm.c | 43 +++++------------------------------------ sound/soc/mxs/mxs-pcm.h | 4 +++- sound/soc/mxs/mxs-saif.c | 6 +++--- sound/soc/omap/omap-pcm.c | 8 +------- sound/soc/pxa/mmp-pcm.c | 33 +++++-------------------------- sound/soc/soc-dmaengine-pcm.c | 29 --------------------------- sound/soc/spear/spear_pcm.c | 18 ++--------------- sound/soc/tegra/tegra_pcm.c | 8 +------- sound/soc/ux500/ux500_pcm.c | 25 +----------------------- 22 files changed, 76 insertions(+), 287 deletions(-) delete mode 100644 sound/soc/cirrus/ep93xx-pcm.h
The generic dmaengine based PCM driver code takes care of setting this constraint, there is no need of doing it manually in the ux500 driver.
Signed-off-by: Lars-Peter Clausen lars@metafoo.de Cc: Ola Lilja ola.o.lilja@stericsson.com --- sound/soc/ux500/ux500_pcm.c | 9 --------- 1 file changed, 9 deletions(-)
diff --git a/sound/soc/ux500/ux500_pcm.c b/sound/soc/ux500/ux500_pcm.c index 846fa82..375ca6b 100644 --- a/sound/soc/ux500/ux500_pcm.c +++ b/sound/soc/ux500/ux500_pcm.c @@ -111,15 +111,6 @@ static int ux500_pcm_open(struct snd_pcm_substream *substream) snd_soc_set_runtime_hwparams(substream, &ux500_pcm_hw_capture);
- /* ensure that buffer size is a multiple of period size */ - ret = snd_pcm_hw_constraint_integer(runtime, - SNDRV_PCM_HW_PARAM_PERIODS); - if (ret < 0) { - dev_err(dev, "%s: Error: snd_pcm_hw_constraints failed (%d)\n", - __func__, ret); - return ret; - } - dev_dbg(dev, "%s: Set hw-struct for %s.\n", __func__, snd_pcm_stream_str(substream)); runtime->hw = (stream_id == SNDRV_PCM_STREAM_PLAYBACK) ?
Acked.
Regards, Ola
On 03/22/2013 02:12 PM, Lars-Peter Clausen wrote:
The generic dmaengine based PCM driver code takes care of setting this constraint, there is no need of doing it manually in the ux500 driver.
Signed-off-by: Lars-Peter Clausen lars@metafoo.de Cc: Ola Lilja ola.o.lilja@stericsson.com
sound/soc/ux500/ux500_pcm.c | 9 --------- 1 file changed, 9 deletions(-)
diff --git a/sound/soc/ux500/ux500_pcm.c b/sound/soc/ux500/ux500_pcm.c index 846fa82..375ca6b 100644 --- a/sound/soc/ux500/ux500_pcm.c +++ b/sound/soc/ux500/ux500_pcm.c @@ -111,15 +111,6 @@ static int ux500_pcm_open(struct snd_pcm_substream *substream) snd_soc_set_runtime_hwparams(substream, &ux500_pcm_hw_capture);
- /* ensure that buffer size is a multiple of period size */
- ret = snd_pcm_hw_constraint_integer(runtime,
SNDRV_PCM_HW_PARAM_PERIODS);- if (ret < 0) {
dev_err(dev, "%s: Error: snd_pcm_hw_constraints failed (%d)\n",__func__, ret);return ret;- }
- dev_dbg(dev, "%s: Set hw-struct for %s.\n", __func__, snd_pcm_stream_str(substream)); runtime->hw = (stream_id == SNDRV_PCM_STREAM_PLAYBACK) ?
If a PCM driver using the dmaengine PCM helper functions doesn't need to do anything special in its pcm_close callback, snd_dmaengine_pcm_close can be used directly for as the pcm_close callback and there is no need to wrap it in a custom function.
Signed-off-by: Lars-Peter Clausen lars@metafoo.de Cc: Rajeev Kumar rajeev-dlh.kumar@st.com --- sound/soc/spear/spear_pcm.c | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-)
diff --git a/sound/soc/spear/spear_pcm.c b/sound/soc/spear/spear_pcm.c index dd71167..903ac15 100644 --- a/sound/soc/spear/spear_pcm.c +++ b/sound/soc/spear/spear_pcm.c @@ -73,14 +73,6 @@ static int spear_pcm_open(struct snd_pcm_substream *substream) return 0; }
-static int spear_pcm_close(struct snd_pcm_substream *substream) -{ - - snd_dmaengine_pcm_close(substream); - - return 0; -} - static int spear_pcm_mmap(struct snd_pcm_substream *substream, struct vm_area_struct *vma) { @@ -93,7 +85,7 @@ static int spear_pcm_mmap(struct snd_pcm_substream *substream,
static struct snd_pcm_ops spear_pcm_ops = { .open = spear_pcm_open, - .close = spear_pcm_close, + .close = snd_dmaengine_pcm_close, .ioctl = snd_pcm_lib_ioctl, .hw_params = spear_pcm_hw_params, .hw_free = spear_pcm_hw_free,
On 3/22/2013 6:42 PM, Lars-Peter Clausen wrote:
If a PCM driver using the dmaengine PCM helper functions doesn't need to do anything special in its pcm_close callback, snd_dmaengine_pcm_close can be used directly for as the pcm_close callback and there is no need to wrap it in a custom function.
Signed-off-by: Lars-Peter Clausenlars@metafoo.de Cc: Rajeev Kumarrajeev-dlh.kumar@st.com
sound/soc/spear/spear_pcm.c | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-)
diff --git a/sound/soc/spear/spear_pcm.c b/sound/soc/spear/spear_pcm.c index dd71167..903ac15 100644 --- a/sound/soc/spear/spear_pcm.c +++ b/sound/soc/spear/spear_pcm.c @@ -73,14 +73,6 @@ static int spear_pcm_open(struct snd_pcm_substream *substream) return 0; }
-static int spear_pcm_close(struct snd_pcm_substream *substream) -{
- snd_dmaengine_pcm_close(substream);
- return 0;
-}
- static int spear_pcm_mmap(struct snd_pcm_substream *substream, struct vm_area_struct *vma) {
@@ -93,7 +85,7 @@ static int spear_pcm_mmap(struct snd_pcm_substream *substream,
static struct snd_pcm_ops spear_pcm_ops = { .open = spear_pcm_open,
- .close = spear_pcm_close,
- .close = snd_dmaengine_pcm_close, .ioctl = snd_pcm_lib_ioctl, .hw_params = spear_pcm_hw_params, .hw_free = spear_pcm_hw_free,
Acked-by: Rajeev Kumar rajeev-dlh.kumar@st.com
If a PCM driver using the dmaengine PCM helper functions doesn't need to do anything special in its pcm_close callback, snd_dmaengine_pcm_close can be used directly for as the pcm_close callback and there is no need to wrap it in a custom function.
Signed-off-by: Lars-Peter Clausen lars@metafoo.de Cc: Peter Ujfalusi peter.ujfalusi@ti.com Cc: Jarkko Nikula jarkko.nikula@bitmer.com Cc: Sebastien Guiriec s-guiriec@ti.com --- sound/soc/omap/omap-pcm.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/sound/soc/omap/omap-pcm.c b/sound/soc/omap/omap-pcm.c index c722c2e..c6f477c 100644 --- a/sound/soc/omap/omap-pcm.c +++ b/sound/soc/omap/omap-pcm.c @@ -185,12 +185,6 @@ static int omap_pcm_open(struct snd_pcm_substream *substream) &dma_data->dma_req); }
-static int omap_pcm_close(struct snd_pcm_substream *substream) -{ - snd_dmaengine_pcm_close(substream); - return 0; -} - static int omap_pcm_mmap(struct snd_pcm_substream *substream, struct vm_area_struct *vma) { @@ -204,7 +198,7 @@ static int omap_pcm_mmap(struct snd_pcm_substream *substream,
static struct snd_pcm_ops omap_pcm_ops = { .open = omap_pcm_open, - .close = omap_pcm_close, + .close = snd_dmaengine_pcm_close, .ioctl = snd_pcm_lib_ioctl, .hw_params = omap_pcm_hw_params, .hw_free = omap_pcm_hw_free,
On 03/22/2013 02:12 PM, Lars-Peter Clausen wrote:
If a PCM driver using the dmaengine PCM helper functions doesn't need to do anything special in its pcm_close callback, snd_dmaengine_pcm_close can be used directly for as the pcm_close callback and there is no need to wrap it in a custom function.
Signed-off-by: Lars-Peter Clausen lars@metafoo.de Cc: Peter Ujfalusi peter.ujfalusi@ti.com Cc: Jarkko Nikula jarkko.nikula@bitmer.com Cc: Sebastien Guiriec s-guiriec@ti.com
Acked-by: Peter Ujfalusi peter.ujfalusi@ti.com
sound/soc/omap/omap-pcm.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/sound/soc/omap/omap-pcm.c b/sound/soc/omap/omap-pcm.c index c722c2e..c6f477c 100644 --- a/sound/soc/omap/omap-pcm.c +++ b/sound/soc/omap/omap-pcm.c @@ -185,12 +185,6 @@ static int omap_pcm_open(struct snd_pcm_substream *substream) &dma_data->dma_req); }
-static int omap_pcm_close(struct snd_pcm_substream *substream) -{
- snd_dmaengine_pcm_close(substream);
- return 0;
-}
static int omap_pcm_mmap(struct snd_pcm_substream *substream, struct vm_area_struct *vma) { @@ -204,7 +198,7 @@ static int omap_pcm_mmap(struct snd_pcm_substream *substream,
static struct snd_pcm_ops omap_pcm_ops = { .open = omap_pcm_open,
- .close = omap_pcm_close,
- .close = snd_dmaengine_pcm_close, .ioctl = snd_pcm_lib_ioctl, .hw_params = omap_pcm_hw_params, .hw_free = omap_pcm_hw_free,
On Fri, 22 Mar 2013 14:13:12 +0100 Peter Ujfalusi peter.ujfalusi@ti.com wrote:
On 03/22/2013 02:12 PM, Lars-Peter Clausen wrote:
If a PCM driver using the dmaengine PCM helper functions doesn't need to do anything special in its pcm_close callback, snd_dmaengine_pcm_close can be used directly for as the pcm_close callback and there is no need to wrap it in a custom function.
Signed-off-by: Lars-Peter Clausen lars@metafoo.de Cc: Peter Ujfalusi peter.ujfalusi@ti.com Cc: Jarkko Nikula jarkko.nikula@bitmer.com Cc: Sebastien Guiriec s-guiriec@ti.com
Acked-by: Peter Ujfalusi peter.ujfalusi@ti.com
Acked-by: Jarkko Nikula jarkko.nikula@bitmer.com
If a PCM driver using the dmaengine PCM helper functions doesn't need to do anything special in its pcm_close callback, snd_dmaengine_pcm_close can be used directly for as the pcm_close callback and there is no need to wrap it in a custom function.
Signed-off-by: Lars-Peter Clausen lars@metafoo.de Cc: Stephen Warren swarren@wwwdotorg.org Cc: Laxman Dewangan ldewangan@nvidia.com --- sound/soc/tegra/tegra_pcm.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/sound/soc/tegra/tegra_pcm.c b/sound/soc/tegra/tegra_pcm.c index c925ab0..e67af0b 100644 --- a/sound/soc/tegra/tegra_pcm.c +++ b/sound/soc/tegra/tegra_pcm.c @@ -75,12 +75,6 @@ static int tegra_pcm_open(struct snd_pcm_substream *substream) return 0; }
-static int tegra_pcm_close(struct snd_pcm_substream *substream) -{ - snd_dmaengine_pcm_close(substream); - return 0; -} - static int tegra_pcm_hw_params(struct snd_pcm_substream *substream, struct snd_pcm_hw_params *params) { @@ -160,7 +154,7 @@ static int tegra_pcm_mmap(struct snd_pcm_substream *substream,
static struct snd_pcm_ops tegra_pcm_ops = { .open = tegra_pcm_open, - .close = tegra_pcm_close, + .close = snd_dmaengine_pcm_close, .ioctl = snd_pcm_lib_ioctl, .hw_params = tegra_pcm_hw_params, .hw_free = tegra_pcm_hw_free,
On 03/22/2013 07:12 AM, Lars-Peter Clausen wrote:
If a PCM driver using the dmaengine PCM helper functions doesn't need to do anything special in its pcm_close callback, snd_dmaengine_pcm_close can be used directly for as the pcm_close callback and there is no need to wrap it in a custom function.
Acked-by: Stephen Warren swarren@nvidia.com
If a PCM driver using the dmaengine PCM helper functions doesn't need to do anything special in its pcm_close callback, snd_dmaengine_pcm_close can be used directly for as the pcm_close callback and there is no need to wrap it in a custom function.
Signed-off-by: Lars-Peter Clausen lars@metafoo.de Cc: Ola Lilja ola.o.lilja@stericsson.com --- sound/soc/ux500/ux500_pcm.c | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-)
diff --git a/sound/soc/ux500/ux500_pcm.c b/sound/soc/ux500/ux500_pcm.c index 375ca6b..d000ba2 100644 --- a/sound/soc/ux500/ux500_pcm.c +++ b/sound/soc/ux500/ux500_pcm.c @@ -160,18 +160,6 @@ static int ux500_pcm_open(struct snd_pcm_substream *substream) return 0; }
-static int ux500_pcm_close(struct snd_pcm_substream *substream) -{ - struct snd_soc_pcm_runtime *rtd = substream->private_data; - struct snd_soc_dai *dai = rtd->cpu_dai; - - dev_dbg(dai->dev, "%s: Enter\n", __func__); - - snd_dmaengine_pcm_close(substream); - - return 0; -} - static int ux500_pcm_hw_params(struct snd_pcm_substream *substream, struct snd_pcm_hw_params *hw_params) { @@ -246,7 +234,7 @@ static int ux500_pcm_mmap(struct snd_pcm_substream *substream,
static struct snd_pcm_ops ux500_pcm_ops = { .open = ux500_pcm_open, - .close = ux500_pcm_close, + .close = snd_dmaengine_pcm_close, .ioctl = snd_pcm_lib_ioctl, .hw_params = ux500_pcm_hw_params, .hw_free = ux500_pcm_hw_free,
Acked.
Regards, Ola
On 03/22/2013 02:12 PM, Lars-Peter Clausen wrote:
If a PCM driver using the dmaengine PCM helper functions doesn't need to do anything special in its pcm_close callback, snd_dmaengine_pcm_close can be used directly for as the pcm_close callback and there is no need to wrap it in a custom function.
Signed-off-by: Lars-Peter Clausen lars@metafoo.de Cc: Ola Lilja ola.o.lilja@stericsson.com
sound/soc/ux500/ux500_pcm.c | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-)
diff --git a/sound/soc/ux500/ux500_pcm.c b/sound/soc/ux500/ux500_pcm.c index 375ca6b..d000ba2 100644 --- a/sound/soc/ux500/ux500_pcm.c +++ b/sound/soc/ux500/ux500_pcm.c @@ -160,18 +160,6 @@ static int ux500_pcm_open(struct snd_pcm_substream *substream) return 0; }
-static int ux500_pcm_close(struct snd_pcm_substream *substream) -{
- struct snd_soc_pcm_runtime *rtd = substream->private_data;
- struct snd_soc_dai *dai = rtd->cpu_dai;
- dev_dbg(dai->dev, "%s: Enter\n", __func__);
- snd_dmaengine_pcm_close(substream);
- return 0;
-}
static int ux500_pcm_hw_params(struct snd_pcm_substream *substream, struct snd_pcm_hw_params *hw_params) { @@ -246,7 +234,7 @@ static int ux500_pcm_mmap(struct snd_pcm_substream *substream,
static struct snd_pcm_ops ux500_pcm_ops = { .open = ux500_pcm_open,
- .close = ux500_pcm_close,
- .close = snd_dmaengine_pcm_close, .ioctl = snd_pcm_lib_ioctl, .hw_params = ux500_pcm_hw_params, .hw_free = ux500_pcm_hw_free,
If a PCM driver using the dmaengine PCM helper functions doesn't need to do anything special in its pcm_close callback, snd_dmaengine_pcm_close can be used directly for as the pcm_close callback and there is no need to wrap it in a custom function.
Signed-off-by: Lars-Peter Clausen lars@metafoo.de Cc: Bo Shen voice.shen@atmel.com Cc: Nicolas Ferre nicolas.ferre@atmel.com --- sound/soc/atmel/atmel-pcm-dma.c | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/sound/soc/atmel/atmel-pcm-dma.c b/sound/soc/atmel/atmel-pcm-dma.c index 30184a4..396bf78 100644 --- a/sound/soc/atmel/atmel-pcm-dma.c +++ b/sound/soc/atmel/atmel-pcm-dma.c @@ -199,16 +199,9 @@ static int atmel_pcm_open(struct snd_pcm_substream *substream) return 0; }
-static int atmel_pcm_close(struct snd_pcm_substream *substream) -{ - snd_dmaengine_pcm_close(substream); - - return 0; -} - static struct snd_pcm_ops atmel_pcm_ops = { .open = atmel_pcm_open, - .close = atmel_pcm_close, + .close = snd_dmaengine_pcm_close, .ioctl = snd_pcm_lib_ioctl, .hw_params = atmel_pcm_hw_params, .prepare = atmel_pcm_dma_prepare,
Hi Lars,
On 3/22/2013 21:12, Lars-Peter Clausen wrote:
If a PCM driver using the dmaengine PCM helper functions doesn't need to do anything special in its pcm_close callback, snd_dmaengine_pcm_close can be used directly for as the pcm_close callback and there is no need to wrap it in a custom function.
Signed-off-by: Lars-Peter Clausen lars@metafoo.de Cc: Bo Shen voice.shen@atmel.com
Tested-by: Bo Shen voice.shen@atmel.com Acked-by: Bo Shen voice.shen@atmel.com
Cc: Nicolas Ferre nicolas.ferre@atmel.com
sound/soc/atmel/atmel-pcm-dma.c | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/sound/soc/atmel/atmel-pcm-dma.c b/sound/soc/atmel/atmel-pcm-dma.c index 30184a4..396bf78 100644 --- a/sound/soc/atmel/atmel-pcm-dma.c +++ b/sound/soc/atmel/atmel-pcm-dma.c @@ -199,16 +199,9 @@ static int atmel_pcm_open(struct snd_pcm_substream *substream) return 0; }
-static int atmel_pcm_close(struct snd_pcm_substream *substream) -{
- snd_dmaengine_pcm_close(substream);
- return 0;
-}
- static struct snd_pcm_ops atmel_pcm_ops = { .open = atmel_pcm_open,
- .close = atmel_pcm_close,
- .close = snd_dmaengine_pcm_close, .ioctl = snd_pcm_lib_ioctl, .hw_params = atmel_pcm_hw_params, .prepare = atmel_pcm_dma_prepare,
The driver never uses snd_dmaengine_pcm_get_data(), so there is no need to use snd_dmaengine_pcm_set_data().
Signed-off-by: Lars-Peter Clausen lars@metafoo.de Cc: Ola Lilja ola.o.lilja@stericsson.com --- sound/soc/ux500/ux500_pcm.c | 2 -- 1 file changed, 2 deletions(-)
diff --git a/sound/soc/ux500/ux500_pcm.c b/sound/soc/ux500/ux500_pcm.c index d000ba2..1ab36fa 100644 --- a/sound/soc/ux500/ux500_pcm.c +++ b/sound/soc/ux500/ux500_pcm.c @@ -155,8 +155,6 @@ static int ux500_pcm_open(struct snd_pcm_substream *substream) return ret; }
- snd_dmaengine_pcm_set_data(substream, dma_cfg); - return 0; }
Acked.
Regards, Ola
On 03/22/2013 02:12 PM, Lars-Peter Clausen wrote:
The driver never uses snd_dmaengine_pcm_get_data(), so there is no need to use snd_dmaengine_pcm_set_data().
Signed-off-by: Lars-Peter Clausen lars@metafoo.de Cc: Ola Lilja ola.o.lilja@stericsson.com
sound/soc/ux500/ux500_pcm.c | 2 -- 1 file changed, 2 deletions(-)
diff --git a/sound/soc/ux500/ux500_pcm.c b/sound/soc/ux500/ux500_pcm.c index d000ba2..1ab36fa 100644 --- a/sound/soc/ux500/ux500_pcm.c +++ b/sound/soc/ux500/ux500_pcm.c @@ -155,8 +155,6 @@ static int ux500_pcm_open(struct snd_pcm_substream *substream) return ret; }
- snd_dmaengine_pcm_set_data(substream, dma_cfg);
- return 0;
}
On Fri, Mar 22, 2013 at 04:01:04PM +0100, Ola Lilja wrote:
Acked.
Regards, Ola
Don't top post and it's helpful to write out the ack fully so tools work well with it (things like patchwork for example, or cut'n'paste in my case).
The driver never uses snd_dmaengine_pcm_get_data(), so there is no need to use snd_dmaengine_pcm_set_data().
Signed-off-by: Lars-Peter Clausen lars@metafoo.de Cc: Rajeev Kumar rajeev-dlh.kumar@st.com --- sound/soc/spear/spear_pcm.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/sound/soc/spear/spear_pcm.c b/sound/soc/spear/spear_pcm.c index 903ac15..bfbcc1f 100644 --- a/sound/soc/spear/spear_pcm.c +++ b/sound/soc/spear/spear_pcm.c @@ -64,13 +64,7 @@ static int spear_pcm_open(struct snd_pcm_substream *substream) if (ret) return ret;
- ret = snd_dmaengine_pcm_open(substream, dma_data->filter, dma_data); - if (ret) - return ret; - - snd_dmaengine_pcm_set_data(substream, dma_data); - - return 0; + return snd_dmaengine_pcm_open(substream, dma_data->filter, dma_data) }
static int spear_pcm_mmap(struct snd_pcm_substream *substream,
Hello Lars,
On 3/22/2013 6:42 PM, Lars-Peter Clausen wrote:
The driver never uses snd_dmaengine_pcm_get_data(), so there is no need to use snd_dmaengine_pcm_set_data().
Signed-off-by: Lars-Peter Clausenlars@metafoo.de Cc: Rajeev Kumarrajeev-dlh.kumar@st.com
sound/soc/spear/spear_pcm.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/sound/soc/spear/spear_pcm.c b/sound/soc/spear/spear_pcm.c index 903ac15..bfbcc1f 100644 --- a/sound/soc/spear/spear_pcm.c +++ b/sound/soc/spear/spear_pcm.c @@ -64,13 +64,7 @@ static int spear_pcm_open(struct snd_pcm_substream *substream) if (ret) return ret;
- ret = snd_dmaengine_pcm_open(substream, dma_data->filter, dma_data);
- if (ret)
return ret;- snd_dmaengine_pcm_set_data(substream, dma_data);
The dma data is used to set dma parameters to transfer data and it will required by the dma engine.
Best Regards Rajeev
- return 0;
return snd_dmaengine_pcm_open(substream, dma_data->filter, dma_data) }
static int spear_pcm_mmap(struct snd_pcm_substream *substream,
On 03/25/2013 04:59 AM, Rajeev kumar wrote:
Hello Lars,
On 3/22/2013 6:42 PM, Lars-Peter Clausen wrote:
The driver never uses snd_dmaengine_pcm_get_data(), so there is no need to use snd_dmaengine_pcm_set_data().
Signed-off-by: Lars-Peter Clausenlars@metafoo.de Cc: Rajeev Kumarrajeev-dlh.kumar@st.com
sound/soc/spear/spear_pcm.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/sound/soc/spear/spear_pcm.c b/sound/soc/spear/spear_pcm.c index 903ac15..bfbcc1f 100644 --- a/sound/soc/spear/spear_pcm.c +++ b/sound/soc/spear/spear_pcm.c @@ -64,13 +64,7 @@ static int spear_pcm_open(struct snd_pcm_substream *substream) if (ret) return ret;
- ret = snd_dmaengine_pcm_open(substream, dma_data->filter, dma_data);
- if (ret)
return ret;- snd_dmaengine_pcm_set_data(substream, dma_data);
Hi,
The dma data is used to set dma parameters to transfer data and it will required by the dma engine.
snd_dmaengine_pcm_set_data() sets private data for the dmaengine pcm, this data can be retrieved later using snd_dmaengine_pcm_get_data(). The spear pcm driver never uses snd_dmaengine_pcm_get_data(), so calling snd_dmaengine_pcm_set_data() shouldn't be necessary.
- Lars
On 3/25/2013 1:52 PM, Lars-Peter Clausen wrote:
On 03/25/2013 04:59 AM, Rajeev kumar wrote:
Hello Lars,
On 3/22/2013 6:42 PM, Lars-Peter Clausen wrote:
The driver never uses snd_dmaengine_pcm_get_data(), so there is no need to use snd_dmaengine_pcm_set_data().
Signed-off-by: Lars-Peter Clausenlars@metafoo.de Cc: Rajeev Kumarrajeev-dlh.kumar@st.com
sound/soc/spear/spear_pcm.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-)diff --git a/sound/soc/spear/spear_pcm.c b/sound/soc/spear/spear_pcm.c index 903ac15..bfbcc1f 100644 --- a/sound/soc/spear/spear_pcm.c +++ b/sound/soc/spear/spear_pcm.c
Acked-By: Rajeev Kumarrajeev-dlh.kumar@st.com
We want to get rid of snd_dmaengine_pcm_{set,get}_data(). All instances of snd_dmaengine_pcm_get_data() in the atmel pcm driver can easily be replaced with snd_soc_dai_get_dma_data().
Signed-off-by: Lars-Peter Clausen lars@metafoo.de Cc: Bo Shen voice.shen@atmel.com Cc: Nicolas Ferre nicolas.ferre@atmel.com --- sound/soc/atmel/atmel-pcm-dma.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/sound/soc/atmel/atmel-pcm-dma.c b/sound/soc/atmel/atmel-pcm-dma.c index 396bf78..b8570e3 100644 --- a/sound/soc/atmel/atmel-pcm-dma.c +++ b/sound/soc/atmel/atmel-pcm-dma.c @@ -67,9 +67,10 @@ static const struct snd_pcm_hardware atmel_pcm_dma_hardware = { static void atmel_pcm_dma_irq(u32 ssc_sr, struct snd_pcm_substream *substream) { + struct snd_soc_pcm_runtime *rtd = substream->private_data; struct atmel_pcm_dma_params *prtd;
- prtd = snd_dmaengine_pcm_get_data(substream); + prtd = snd_soc_dai_get_dma_data(rtd->cpu_dai, substream);
if (ssc_sr & prtd->mask->ssc_error) { if (snd_pcm_running(substream)) @@ -104,15 +105,13 @@ static bool filter(struct dma_chan *chan, void *slave) }
static int atmel_pcm_configure_dma(struct snd_pcm_substream *substream, - struct snd_pcm_hw_params *params) + struct snd_pcm_hw_params *params, struct atmel_pcm_dma_params *prtd) { - struct atmel_pcm_dma_params *prtd; struct ssc_device *ssc; struct dma_chan *dma_chan; struct dma_slave_config slave_config; int ret;
- prtd = snd_dmaengine_pcm_get_data(substream); ssc = prtd->ssc;
ret = snd_hwparams_to_dma_slave_config(substream, params, @@ -164,9 +163,7 @@ static int atmel_pcm_hw_params(struct snd_pcm_substream *substream, return -EINVAL; }
- snd_dmaengine_pcm_set_data(substream, prtd); - - ret = atmel_pcm_configure_dma(substream, params); + ret = atmel_pcm_configure_dma(substream, params, prtd); if (ret) { pr_err("atmel-pcm: failed to configure dmai\n"); goto err; @@ -182,9 +179,10 @@ err:
static int atmel_pcm_dma_prepare(struct snd_pcm_substream *substream) { + struct snd_soc_pcm_runtime *rtd = substream->private_data; struct atmel_pcm_dma_params *prtd;
- prtd = snd_dmaengine_pcm_get_data(substream); + prtd = snd_soc_dai_get_dma_data(rtd->cpu_dai, substream);
ssc_writex(prtd->ssc->regs, SSC_IER, prtd->mask->ssc_error); ssc_writex(prtd->ssc->regs, SSC_CR, prtd->mask->ssc_enable);
Hi Lars,
On 3/22/2013 21:12, Lars-Peter Clausen wrote:
We want to get rid of snd_dmaengine_pcm_{set,get}_data(). All instances of snd_dmaengine_pcm_get_data() in the atmel pcm driver can easily be replaced with snd_soc_dai_get_dma_data().
Signed-off-by: Lars-Peter Clausen lars@metafoo.de Cc: Bo Shen voice.shen@atmel.com
Tested-by: Bo Shen voice.shen@atmel.com Acked-by: Bo Shen voice.shen@atmel.com
Cc: Nicolas Ferre nicolas.ferre@atmel.com
sound/soc/atmel/atmel-pcm-dma.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/sound/soc/atmel/atmel-pcm-dma.c b/sound/soc/atmel/atmel-pcm-dma.c index 396bf78..b8570e3 100644 --- a/sound/soc/atmel/atmel-pcm-dma.c +++ b/sound/soc/atmel/atmel-pcm-dma.c @@ -67,9 +67,10 @@ static const struct snd_pcm_hardware atmel_pcm_dma_hardware = { static void atmel_pcm_dma_irq(u32 ssc_sr, struct snd_pcm_substream *substream) {
- struct snd_soc_pcm_runtime *rtd = substream->private_data; struct atmel_pcm_dma_params *prtd;
- prtd = snd_dmaengine_pcm_get_data(substream);
prtd = snd_soc_dai_get_dma_data(rtd->cpu_dai, substream);
if (ssc_sr & prtd->mask->ssc_error) { if (snd_pcm_running(substream))
@@ -104,15 +105,13 @@ static bool filter(struct dma_chan *chan, void *slave) }
static int atmel_pcm_configure_dma(struct snd_pcm_substream *substream,
- struct snd_pcm_hw_params *params)
- struct snd_pcm_hw_params *params, struct atmel_pcm_dma_params *prtd) {
struct atmel_pcm_dma_params *prtd; struct ssc_device *ssc; struct dma_chan *dma_chan; struct dma_slave_config slave_config; int ret;
prtd = snd_dmaengine_pcm_get_data(substream); ssc = prtd->ssc;
ret = snd_hwparams_to_dma_slave_config(substream, params,
@@ -164,9 +163,7 @@ static int atmel_pcm_hw_params(struct snd_pcm_substream *substream, return -EINVAL; }
- snd_dmaengine_pcm_set_data(substream, prtd);
- ret = atmel_pcm_configure_dma(substream, params);
- ret = atmel_pcm_configure_dma(substream, params, prtd); if (ret) { pr_err("atmel-pcm: failed to configure dmai\n"); goto err;
@@ -182,9 +179,10 @@ err:
static int atmel_pcm_dma_prepare(struct snd_pcm_substream *substream) {
- struct snd_soc_pcm_runtime *rtd = substream->private_data; struct atmel_pcm_dma_params *prtd;
- prtd = snd_dmaengine_pcm_get_data(substream);
prtd = snd_soc_dai_get_dma_data(rtd->cpu_dai, substream);
ssc_writex(prtd->ssc->regs, SSC_IER, prtd->mask->ssc_error); ssc_writex(prtd->ssc->regs, SSC_CR, prtd->mask->ssc_enable);
Currently the ep93xx_dma_params struct which is passed to the dmaengine driver is constructed at runtime from the ep93xx_pcm_dma_params that gets passed to the ep93xx PCM driver from one of the ep93xx DAI drivers. The ep93xx_pcm_dma_params struct is almost identical to the ep93xx_dma_params struct. The only missing field is the 'direction' field, which is computed at runtime in the PCM driver based on the current substream. Since we know in advance which ep93xx_pcm_dma_params struct is being used for which substream at compile time, we also already know which direction to use at compile time. So we can easily replace all instances of ep93xx_pcm_dma_params with their ep93xx_dma_params counterpart. This allows us to simplify the code in the ep93xx pcm driver quite a bit.
Signed-off-by: Lars-Peter Clausen lars@metafoo.de Cc: Hartley Sweeten hsweeten@visionengravers.com Cc: Ryan Mallon rmallon@gmail.com --- sound/soc/cirrus/edb93xx.c | 1 - sound/soc/cirrus/ep93xx-ac97.c | 9 +++++---- sound/soc/cirrus/ep93xx-i2s.c | 12 ++++++------ sound/soc/cirrus/ep93xx-pcm.c | 37 +++---------------------------------- sound/soc/cirrus/ep93xx-pcm.h | 20 -------------------- sound/soc/cirrus/simone.c | 2 -- sound/soc/cirrus/snappercl15.c | 1 - 7 files changed, 14 insertions(+), 68 deletions(-) delete mode 100644 sound/soc/cirrus/ep93xx-pcm.h
diff --git a/sound/soc/cirrus/edb93xx.c b/sound/soc/cirrus/edb93xx.c index 5db68cf..c43fb21 100644 --- a/sound/soc/cirrus/edb93xx.c +++ b/sound/soc/cirrus/edb93xx.c @@ -27,7 +27,6 @@ #include <sound/soc.h> #include <asm/mach-types.h> #include <mach/hardware.h> -#include "ep93xx-pcm.h"
static int edb93xx_hw_params(struct snd_pcm_substream *substream, struct snd_pcm_hw_params *params) diff --git a/sound/soc/cirrus/ep93xx-ac97.c b/sound/soc/cirrus/ep93xx-ac97.c index 1738d28..8d30886 100644 --- a/sound/soc/cirrus/ep93xx-ac97.c +++ b/sound/soc/cirrus/ep93xx-ac97.c @@ -23,7 +23,6 @@ #include <sound/soc.h>
#include <linux/platform_data/dma-ep93xx.h> -#include "ep93xx-pcm.h"
/* * Per channel (1-4) registers. @@ -101,14 +100,16 @@ struct ep93xx_ac97_info { /* currently ALSA only supports a single AC97 device */ static struct ep93xx_ac97_info *ep93xx_ac97_info;
-static struct ep93xx_pcm_dma_params ep93xx_ac97_pcm_out = { +static struct ep93xx_dma_data ep93xx_ac97_pcm_out = { .name = "ac97-pcm-out", .dma_port = EP93XX_DMA_AAC1, + .direction = DMA_MEM_TO_DEV, };
-static struct ep93xx_pcm_dma_params ep93xx_ac97_pcm_in = { +static struct ep93xx_dma_data ep93xx_ac97_pcm_in = { .name = "ac97-pcm-in", .dma_port = EP93XX_DMA_AAC1, + .direction = DMA_DEV_TO_MEM, };
static inline unsigned ep93xx_ac97_read_reg(struct ep93xx_ac97_info *info, @@ -316,7 +317,7 @@ static int ep93xx_ac97_trigger(struct snd_pcm_substream *substream, static int ep93xx_ac97_startup(struct snd_pcm_substream *substream, struct snd_soc_dai *dai) { - struct ep93xx_pcm_dma_params *dma_data; + struct ep93xx_dma_data *dma_data;
if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) dma_data = &ep93xx_ac97_pcm_out; diff --git a/sound/soc/cirrus/ep93xx-i2s.c b/sound/soc/cirrus/ep93xx-i2s.c index 323ed69..aa124f8 100644 --- a/sound/soc/cirrus/ep93xx-i2s.c +++ b/sound/soc/cirrus/ep93xx-i2s.c @@ -30,8 +30,6 @@ #include <mach/ep93xx-regs.h> #include <linux/platform_data/dma-ep93xx.h>
-#include "ep93xx-pcm.h" - #define EP93XX_I2S_TXCLKCFG 0x00 #define EP93XX_I2S_RXCLKCFG 0x04 #define EP93XX_I2S_GLCTRL 0x0C @@ -62,18 +60,20 @@ struct ep93xx_i2s_info { struct clk *mclk; struct clk *sclk; struct clk *lrclk; - struct ep93xx_pcm_dma_params *dma_params; + struct ep93xx_dma_data *dma_data; void __iomem *regs; };
-struct ep93xx_pcm_dma_params ep93xx_i2s_dma_params[] = { +struct ep93xx_dma_data ep93xx_i2s_dma_data[] = { [SNDRV_PCM_STREAM_PLAYBACK] = { .name = "i2s-pcm-out", .dma_port = EP93XX_DMA_I2S1, + .direction = DMA_MEM_TO_DEV, }, [SNDRV_PCM_STREAM_CAPTURE] = { .name = "i2s-pcm-in", .dma_port = EP93XX_DMA_I2S1, + .direction = DMA_DEV_TO_MEM, }, };
@@ -147,7 +147,7 @@ static int ep93xx_i2s_startup(struct snd_pcm_substream *substream, struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
snd_soc_dai_set_dma_data(cpu_dai, substream, - &info->dma_params[substream->stream]); + &info->dma_data[substream->stream]); return 0; }
@@ -403,7 +403,7 @@ static int ep93xx_i2s_probe(struct platform_device *pdev) }
dev_set_drvdata(&pdev->dev, info); - info->dma_params = ep93xx_i2s_dma_params; + info->dma_data = ep93xx_i2s_dma_data;
err = snd_soc_register_dai(&pdev->dev, &ep93xx_i2s_dai); if (err) diff --git a/sound/soc/cirrus/ep93xx-pcm.c b/sound/soc/cirrus/ep93xx-pcm.c index 72eb7a4..298946f 100644 --- a/sound/soc/cirrus/ep93xx-pcm.c +++ b/sound/soc/cirrus/ep93xx-pcm.c @@ -29,8 +29,6 @@ #include <mach/hardware.h> #include <mach/ep93xx-regs.h>
-#include "ep93xx-pcm.h" - static const struct snd_pcm_hardware ep93xx_pcm_hardware = { .info = (SNDRV_PCM_INFO_MMAP | SNDRV_PCM_INFO_MMAP_VALID | @@ -68,40 +66,11 @@ static bool ep93xx_pcm_dma_filter(struct dma_chan *chan, void *filter_param) static int ep93xx_pcm_open(struct snd_pcm_substream *substream) { struct snd_soc_pcm_runtime *rtd = substream->private_data; - struct snd_soc_dai *cpu_dai = rtd->cpu_dai; - struct ep93xx_pcm_dma_params *dma_params; - struct ep93xx_dma_data *dma_data; - int ret;
snd_soc_set_runtime_hwparams(substream, &ep93xx_pcm_hardware);
- dma_data = kmalloc(sizeof(*dma_data), GFP_KERNEL); - if (!dma_data) - return -ENOMEM; - - dma_params = snd_soc_dai_get_dma_data(cpu_dai, substream); - dma_data->port = dma_params->dma_port; - dma_data->name = dma_params->name; - dma_data->direction = snd_pcm_substream_to_dma_direction(substream); - - ret = snd_dmaengine_pcm_open(substream, ep93xx_pcm_dma_filter, dma_data); - if (ret) { - kfree(dma_data); - return ret; - } - - snd_dmaengine_pcm_set_data(substream, dma_data); - - return 0; -} - -static int ep93xx_pcm_close(struct snd_pcm_substream *substream) -{ - struct dma_data *dma_data = snd_dmaengine_pcm_get_data(substream); - - snd_dmaengine_pcm_close(substream); - kfree(dma_data); - return 0; + return snd_dmaengine_pcm_open(substream, ep93xx_pcm_dma_filter, + snd_soc_dai_get_dma_data(rtd->cpu_dai, substream)); }
static int ep93xx_pcm_hw_params(struct snd_pcm_substream *substream, @@ -131,7 +100,7 @@ static int ep93xx_pcm_mmap(struct snd_pcm_substream *substream,
static struct snd_pcm_ops ep93xx_pcm_ops = { .open = ep93xx_pcm_open, - .close = ep93xx_pcm_close, + .close = snd_dmaengine_pcm_close, .ioctl = snd_pcm_lib_ioctl, .hw_params = ep93xx_pcm_hw_params, .hw_free = ep93xx_pcm_hw_free, diff --git a/sound/soc/cirrus/ep93xx-pcm.h b/sound/soc/cirrus/ep93xx-pcm.h deleted file mode 100644 index 111e112..0000000 --- a/sound/soc/cirrus/ep93xx-pcm.h +++ /dev/null @@ -1,20 +0,0 @@ -/* - * sound/soc/ep93xx/ep93xx-pcm.h - EP93xx ALSA PCM interface - * - * Copyright (C) 2006 Lennert Buytenhek buytenh@wantstofly.org - * Copyright (C) 2006 Applied Data Systems - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License version 2 as - * published by the Free Software Foundation. - */ - -#ifndef _EP93XX_SND_SOC_PCM_H -#define _EP93XX_SND_SOC_PCM_H - -struct ep93xx_pcm_dma_params { - char *name; - int dma_port; -}; - -#endif /* _EP93XX_SND_SOC_PCM_H */ diff --git a/sound/soc/cirrus/simone.c b/sound/soc/cirrus/simone.c index a397bb0..4d094d0 100644 --- a/sound/soc/cirrus/simone.c +++ b/sound/soc/cirrus/simone.c @@ -21,8 +21,6 @@ #include <asm/mach-types.h> #include <mach/hardware.h>
-#include "ep93xx-pcm.h" - static struct snd_soc_dai_link simone_dai = { .name = "AC97", .stream_name = "AC97 HiFi", diff --git a/sound/soc/cirrus/snappercl15.c b/sound/soc/cirrus/snappercl15.c index 9d77fe2..6904107 100644 --- a/sound/soc/cirrus/snappercl15.c +++ b/sound/soc/cirrus/snappercl15.c @@ -21,7 +21,6 @@ #include <mach/hardware.h>
#include "../codecs/tlv320aic23.h" -#include "ep93xx-pcm.h"
#define CODEC_CLOCK 5644800
The dma filter parameters are only used within filter callback, so there is no need to allocate them on the heap and keep them around until the PCM has been closed.
Signed-off-by: Lars-Peter Clausen lars@metafoo.de Cc: Zhangfei Gao zhangfei.gao@marvell.com --- sound/soc/pxa/mmp-pcm.c | 33 +++++---------------------------- 1 file changed, 5 insertions(+), 28 deletions(-)
diff --git a/sound/soc/pxa/mmp-pcm.c b/sound/soc/pxa/mmp-pcm.c index 190eb0b..6c39802 100644 --- a/sound/soc/pxa/mmp-pcm.c +++ b/sound/soc/pxa/mmp-pcm.c @@ -118,9 +118,8 @@ static int mmp_pcm_open(struct snd_pcm_substream *substream) struct snd_soc_pcm_runtime *rtd = substream->private_data; struct platform_device *pdev = to_platform_device(rtd->platform->dev); struct snd_soc_dai *cpu_dai = rtd->cpu_dai; - struct mmp_dma_data *dma_data; + struct mmp_dma_data dma_data; struct resource *r; - int ret;
r = platform_get_resource(pdev, IORESOURCE_DMA, substream->stream); if (!r) @@ -128,33 +127,11 @@ static int mmp_pcm_open(struct snd_pcm_substream *substream)
snd_soc_set_runtime_hwparams(substream, &mmp_pcm_hardware[substream->stream]); - dma_data = devm_kzalloc(&pdev->dev, - sizeof(struct mmp_dma_data), GFP_KERNEL); - if (dma_data == NULL) - return -ENOMEM;
- dma_data->dma_res = r; - dma_data->ssp_id = cpu_dai->id; + dma_data.dma_res = r; + dma_data.ssp_id = cpu_dai->id;
- ret = snd_dmaengine_pcm_open(substream, filter, dma_data); - if (ret) { - devm_kfree(&pdev->dev, dma_data); - return ret; - } - - snd_dmaengine_pcm_set_data(substream, dma_data); - return 0; -} - -static int mmp_pcm_close(struct snd_pcm_substream *substream) -{ - struct mmp_dma_data *dma_data = snd_dmaengine_pcm_get_data(substream); - struct snd_soc_pcm_runtime *rtd = substream->private_data; - struct platform_device *pdev = to_platform_device(rtd->platform->dev); - - snd_dmaengine_pcm_close(substream); - devm_kfree(&pdev->dev, dma_data); - return 0; + return snd_dmaengine_pcm_open(substream, filter, &dma_data); }
static int mmp_pcm_mmap(struct snd_pcm_substream *substream, @@ -171,7 +148,7 @@ static int mmp_pcm_mmap(struct snd_pcm_substream *substream,
struct snd_pcm_ops mmp_pcm_ops = { .open = mmp_pcm_open, - .close = mmp_pcm_close, + .close = snd_dmaengine_pcm_close, .ioctl = snd_pcm_lib_ioctl, .hw_params = mmp_pcm_hw_params, .trigger = snd_dmaengine_pcm_trigger,
Currently the imx_dma_data struct, which gets passed to the dmaengine driver, is allocated and constructed in the pcm driver from the data stored in the dma_params struct. The dma_params struct gets passed to the pcm driver from the dai driver. Instead of going this route of indirection embed the dma_data struct directly into the dma_params struct and let the dai driver fill it in. This allows us to simplify the imx-pcm-dma driver quite a bit, since it doesn't have care about memory managing the imx_dma_data struct anymore.
Signed-off-by: Lars-Peter Clausen lars@metafoo.de Cc: Shawn Guo shawn.guo@linaro.org Cc: Sascha Hauer kernel@pengutronix.de --- sound/soc/fsl/fsl_ssi.c | 17 +++++++++-------- sound/soc/fsl/imx-pcm-dma.c | 35 ++--------------------------------- sound/soc/fsl/imx-pcm.h | 17 +++++++++++++++-- sound/soc/fsl/imx-ssi.c | 12 ++++++++---- 4 files changed, 34 insertions(+), 47 deletions(-)
diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c index 7decbd9..2cce1ce 100644 --- a/sound/soc/fsl/fsl_ssi.c +++ b/sound/soc/fsl/fsl_ssi.c @@ -649,6 +649,7 @@ static int fsl_ssi_probe(struct platform_device *pdev) const uint32_t *iprop; struct resource res; char name[64]; + bool shared;
/* SSIs that are not connected on the board should have a * status = "disabled" @@ -755,14 +756,14 @@ static int fsl_ssi_probe(struct platform_device *pdev) dev_err(&pdev->dev, "could not get dma events\n"); goto error_clk; } - ssi_private->dma_params_tx.dma = dma_events[0]; - ssi_private->dma_params_rx.dma = dma_events[1]; - - ssi_private->dma_params_tx.shared_peripheral = - of_device_is_compatible(of_get_parent(np), - "fsl,spba-bus"); - ssi_private->dma_params_rx.shared_peripheral = - ssi_private->dma_params_tx.shared_peripheral; + + shared = of_device_is_compatible(of_get_parent(np), + "fsl,spba-bus"); + + imx_pcm_dma_params_init_data(&ssi_private->dma_params_tx, + dma_events[0], shared); + imx_pcm_dma_params_init_data(&ssi_private->dma_params_rx, + dma_events[1], shared); }
/* Initialize the the device_attribute structure */ diff --git a/sound/soc/fsl/imx-pcm-dma.c b/sound/soc/fsl/imx-pcm-dma.c index 500f8ce..6832c49 100644 --- a/sound/soc/fsl/imx-pcm-dma.c +++ b/sound/soc/fsl/imx-pcm-dma.c @@ -30,8 +30,6 @@ #include <sound/soc.h> #include <sound/dmaengine_pcm.h>
-#include <linux/platform_data/dma-imx.h> - #include "imx-pcm.h"
static bool filter(struct dma_chan *chan, void *param) @@ -101,46 +99,17 @@ static int snd_imx_open(struct snd_pcm_substream *substream) { struct snd_soc_pcm_runtime *rtd = substream->private_data; struct imx_pcm_dma_params *dma_params; - struct imx_dma_data *dma_data; - int ret;
snd_soc_set_runtime_hwparams(substream, &snd_imx_hardware);
dma_params = snd_soc_dai_get_dma_data(rtd->cpu_dai, substream);
- dma_data = kzalloc(sizeof(*dma_data), GFP_KERNEL); - if (!dma_data) - return -ENOMEM; - - dma_data->peripheral_type = dma_params->shared_peripheral ? - IMX_DMATYPE_SSI_SP : IMX_DMATYPE_SSI; - dma_data->priority = DMA_PRIO_HIGH; - dma_data->dma_request = dma_params->dma; - - ret = snd_dmaengine_pcm_open(substream, filter, dma_data); - if (ret) { - kfree(dma_data); - return ret; - } - - snd_dmaengine_pcm_set_data(substream, dma_data); - - return 0; -} - -static int snd_imx_close(struct snd_pcm_substream *substream) -{ - struct imx_dma_data *dma_data = snd_dmaengine_pcm_get_data(substream); - - snd_dmaengine_pcm_close(substream); - kfree(dma_data); - - return 0; + return snd_dmaengine_pcm_open(substream, filter, &dma_params->dma_data); }
static struct snd_pcm_ops imx_pcm_ops = { .open = snd_imx_open, - .close = snd_imx_close, + .close = snd_dmaengine_pcm_close, .ioctl = snd_pcm_lib_ioctl, .hw_params = snd_imx_pcm_hw_params, .trigger = snd_dmaengine_pcm_trigger, diff --git a/sound/soc/fsl/imx-pcm.h b/sound/soc/fsl/imx-pcm.h index 5ae13a1..16eaf5a 100644 --- a/sound/soc/fsl/imx-pcm.h +++ b/sound/soc/fsl/imx-pcm.h @@ -13,18 +13,31 @@ #ifndef _IMX_PCM_H #define _IMX_PCM_H
+#include <linux/platform_data/dma-imx.h> + /* * Do not change this as the FIQ handler depends on this size */ #define IMX_SSI_DMABUF_SIZE (64 * 1024)
struct imx_pcm_dma_params { - int dma; unsigned long dma_addr; int burstsize; - bool shared_peripheral; /* The peripheral is on SPBA bus */ + struct imx_dma_data dma_data; };
+static inline void +imx_pcm_dma_params_init_data(struct imx_pcm_dma_params *params, + int dma, bool shared) +{ + params->dma_data.dma_request = dma; + params->dma_data.priority = DMA_PRIO_HIGH; + if (shared) + params->dma_data.peripheral_type = IMX_DMATYPE_SSI_SP; + else + params->dma_data.peripheral_type = IMX_DMATYPE_SSI; +} + int snd_imx_pcm_mmap(struct snd_pcm_substream *substream, struct vm_area_struct *vma); int imx_pcm_new(struct snd_soc_pcm_runtime *rtd); diff --git a/sound/soc/fsl/imx-ssi.c b/sound/soc/fsl/imx-ssi.c index 0e3fc8d..ffc518b 100644 --- a/sound/soc/fsl/imx-ssi.c +++ b/sound/soc/fsl/imx-ssi.c @@ -582,12 +582,16 @@ static int imx_ssi_probe(struct platform_device *pdev) ssi->dma_params_rx.burstsize = 4;
res = platform_get_resource_byname(pdev, IORESOURCE_DMA, "tx0"); - if (res) - ssi->dma_params_tx.dma = res->start; + if (res) { + imx_pcm_dma_params_init_data(&ssi->dma_params_tx, res->start, + false); + }
res = platform_get_resource_byname(pdev, IORESOURCE_DMA, "rx0"); - if (res) - ssi->dma_params_rx.dma = res->start; + if (res) { + imx_pcm_dma_params_init_data(&ssi->dma_params_rx, res->start, + false); + }
platform_set_drvdata(pdev, ssi);
On Fri, Mar 22, 2013 at 02:12:12PM +0100, Lars-Peter Clausen wrote:
Currently the imx_dma_data struct, which gets passed to the dmaengine driver, is allocated and constructed in the pcm driver from the data stored in the dma_params struct. The dma_params struct gets passed to the pcm driver from the dai driver. Instead of going this route of indirection embed the dma_data struct directly into the dma_params struct and let the dai driver fill it in. This allows us to simplify the imx-pcm-dma driver quite a bit, since it doesn't have care about memory managing the imx_dma_data struct anymore.
Signed-off-by: Lars-Peter Clausen lars@metafoo.de Cc: Shawn Guo shawn.guo@linaro.org Cc: Sascha Hauer kernel@pengutronix.de
On imx5 and imx6,
Tested-by: Shawn Guo shawn.guo@linaro.org
One nit below though ...
sound/soc/fsl/fsl_ssi.c | 17 +++++++++-------- sound/soc/fsl/imx-pcm-dma.c | 35 ++--------------------------------- sound/soc/fsl/imx-pcm.h | 17 +++++++++++++++-- sound/soc/fsl/imx-ssi.c | 12 ++++++++---- 4 files changed, 34 insertions(+), 47 deletions(-)
diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c index 7decbd9..2cce1ce 100644 --- a/sound/soc/fsl/fsl_ssi.c +++ b/sound/soc/fsl/fsl_ssi.c @@ -649,6 +649,7 @@ static int fsl_ssi_probe(struct platform_device *pdev) const uint32_t *iprop; struct resource res; char name[64];
bool shared;
/* SSIs that are not connected on the board should have a
status = "disabled"@@ -755,14 +756,14 @@ static int fsl_ssi_probe(struct platform_device *pdev) dev_err(&pdev->dev, "could not get dma events\n"); goto error_clk; }
ssi_private->dma_params_tx.dma = dma_events[0];ssi_private->dma_params_rx.dma = dma_events[1];ssi_private->dma_params_tx.shared_peripheral =of_device_is_compatible(of_get_parent(np),"fsl,spba-bus");ssi_private->dma_params_rx.shared_peripheral =ssi_private->dma_params_tx.shared_peripheral;
shared = of_device_is_compatible(of_get_parent(np),"fsl,spba-bus");imx_pcm_dma_params_init_data(&ssi_private->dma_params_tx,dma_events[0], shared);imx_pcm_dma_params_init_data(&ssi_private->dma_params_rx,dma_events[1], shared);}
/* Initialize the the device_attribute structure */
diff --git a/sound/soc/fsl/imx-pcm-dma.c b/sound/soc/fsl/imx-pcm-dma.c index 500f8ce..6832c49 100644 --- a/sound/soc/fsl/imx-pcm-dma.c +++ b/sound/soc/fsl/imx-pcm-dma.c @@ -30,8 +30,6 @@ #include <sound/soc.h> #include <sound/dmaengine_pcm.h>
-#include <linux/platform_data/dma-imx.h>
#include "imx-pcm.h"
static bool filter(struct dma_chan *chan, void *param) @@ -101,46 +99,17 @@ static int snd_imx_open(struct snd_pcm_substream *substream) { struct snd_soc_pcm_runtime *rtd = substream->private_data; struct imx_pcm_dma_params *dma_params;
struct imx_dma_data *dma_data;
int ret;
snd_soc_set_runtime_hwparams(substream, &snd_imx_hardware);
dma_params = snd_soc_dai_get_dma_data(rtd->cpu_dai, substream);
dma_data = kzalloc(sizeof(*dma_data), GFP_KERNEL);
if (!dma_data)
return -ENOMEM;dma_data->peripheral_type = dma_params->shared_peripheral ?
IMX_DMATYPE_SSI_SP : IMX_DMATYPE_SSI;dma_data->priority = DMA_PRIO_HIGH;
dma_data->dma_request = dma_params->dma;
ret = snd_dmaengine_pcm_open(substream, filter, dma_data);
if (ret) {
kfree(dma_data);return ret;}
snd_dmaengine_pcm_set_data(substream, dma_data);
return 0;
-}
-static int snd_imx_close(struct snd_pcm_substream *substream) -{
- struct imx_dma_data *dma_data = snd_dmaengine_pcm_get_data(substream);
- snd_dmaengine_pcm_close(substream);
- kfree(dma_data);
- return 0;
- return snd_dmaengine_pcm_open(substream, filter, &dma_params->dma_data);
}
static struct snd_pcm_ops imx_pcm_ops = { .open = snd_imx_open,
- .close = snd_imx_close,
- .close = snd_dmaengine_pcm_close, .ioctl = snd_pcm_lib_ioctl, .hw_params = snd_imx_pcm_hw_params, .trigger = snd_dmaengine_pcm_trigger,
diff --git a/sound/soc/fsl/imx-pcm.h b/sound/soc/fsl/imx-pcm.h index 5ae13a1..16eaf5a 100644 --- a/sound/soc/fsl/imx-pcm.h +++ b/sound/soc/fsl/imx-pcm.h @@ -13,18 +13,31 @@ #ifndef _IMX_PCM_H #define _IMX_PCM_H
+#include <linux/platform_data/dma-imx.h>
/*
- Do not change this as the FIQ handler depends on this size
*/ #define IMX_SSI_DMABUF_SIZE (64 * 1024)
struct imx_pcm_dma_params {
- int dma; unsigned long dma_addr; int burstsize;
- bool shared_peripheral; /* The peripheral is on SPBA bus */
- struct imx_dma_data dma_data;
};
+static inline void +imx_pcm_dma_params_init_data(struct imx_pcm_dma_params *params,
- int dma, bool shared)
+{
- params->dma_data.dma_request = dma;
- params->dma_data.priority = DMA_PRIO_HIGH;
- if (shared)
params->dma_data.peripheral_type = IMX_DMATYPE_SSI_SP;- else
params->dma_data.peripheral_type = IMX_DMATYPE_SSI;+}
int snd_imx_pcm_mmap(struct snd_pcm_substream *substream, struct vm_area_struct *vma); int imx_pcm_new(struct snd_soc_pcm_runtime *rtd); diff --git a/sound/soc/fsl/imx-ssi.c b/sound/soc/fsl/imx-ssi.c index 0e3fc8d..ffc518b 100644 --- a/sound/soc/fsl/imx-ssi.c +++ b/sound/soc/fsl/imx-ssi.c @@ -582,12 +582,16 @@ static int imx_ssi_probe(struct platform_device *pdev) ssi->dma_params_rx.burstsize = 4;
res = platform_get_resource_byname(pdev, IORESOURCE_DMA, "tx0");
- if (res)
ssi->dma_params_tx.dma = res->start;
- if (res) {
imx_pcm_dma_params_init_data(&ssi->dma_params_tx, res->start,false);- }
Unneeded braces.
res = platform_get_resource_byname(pdev, IORESOURCE_DMA, "rx0");
- if (res)
ssi->dma_params_rx.dma = res->start;
- if (res) {
imx_pcm_dma_params_init_data(&ssi->dma_params_rx, res->start,false);- }
Ditto.
Shawn
platform_set_drvdata(pdev, ssi);
-- 1.8.0
Currently the mxs_dma_data struct, which gets passed to the dmaengine driver, is allocated in the pcm driver's open callback. The mxs_dma_data struct has exactly one field which is initialized from the the same field in the mxs_pcm_dma_params struct. The mxs_pcm_dma_params struct gets passed to the pcm driver from the dai driver. Instead of taking this indirection embed the mxs_dma_data struct directly in the mxs_pcm_dma_params struct. This allows us to simplify the pcm driver quite a bit, since we don't have to care about memory managing the mxs_dma_data struct anymore.
Signed-off-by: Lars-Peter Clausen lars@metafoo.de Cc: Shawn Guo shawn.guo@linaro.org --- sound/soc/mxs/mxs-pcm.c | 43 +++++-------------------------------------- sound/soc/mxs/mxs-pcm.h | 4 +++- sound/soc/mxs/mxs-saif.c | 6 +++--- 3 files changed, 11 insertions(+), 42 deletions(-)
diff --git a/sound/soc/mxs/mxs-pcm.c b/sound/soc/mxs/mxs-pcm.c index 564b5b6..ebbef85 100644 --- a/sound/soc/mxs/mxs-pcm.c +++ b/sound/soc/mxs/mxs-pcm.c @@ -28,7 +28,6 @@ #include <linux/platform_device.h> #include <linux/slab.h> #include <linux/dmaengine.h> -#include <linux/fsl/mxs-dma.h>
#include <sound/core.h> #include <sound/initval.h> @@ -39,11 +38,6 @@
#include "mxs-pcm.h"
-struct mxs_pcm_dma_data { - struct mxs_dma_data dma_data; - struct mxs_pcm_dma_params *dma_params; -}; - static struct snd_pcm_hardware snd_mxs_hardware = { .info = SNDRV_PCM_INFO_MMAP | SNDRV_PCM_INFO_MMAP_VALID | @@ -66,8 +60,7 @@ static struct snd_pcm_hardware snd_mxs_hardware = {
static bool filter(struct dma_chan *chan, void *param) { - struct mxs_pcm_dma_data *pcm_dma_data = param; - struct mxs_pcm_dma_params *dma_params = pcm_dma_data->dma_params; + struct mxs_pcm_dma_params *dma_params = param;
if (!mxs_dma_is_apbx(chan)) return false; @@ -75,7 +68,7 @@ static bool filter(struct dma_chan *chan, void *param) if (chan->chan_id != dma_params->chan_num) return false;
- chan->private = &pcm_dma_data->dma_data; + chan->private = &dma_params->dma_data;
return true; } @@ -91,37 +84,11 @@ static int snd_mxs_pcm_hw_params(struct snd_pcm_substream *substream, static int snd_mxs_open(struct snd_pcm_substream *substream) { struct snd_soc_pcm_runtime *rtd = substream->private_data; - struct mxs_pcm_dma_data *pcm_dma_data; - int ret; - - pcm_dma_data = kzalloc(sizeof(*pcm_dma_data), GFP_KERNEL); - if (pcm_dma_data == NULL) - return -ENOMEM; - - pcm_dma_data->dma_params = snd_soc_dai_get_dma_data(rtd->cpu_dai, substream); - pcm_dma_data->dma_data.chan_irq = pcm_dma_data->dma_params->chan_irq; - - ret = snd_dmaengine_pcm_open(substream, filter, pcm_dma_data); - if (ret) { - kfree(pcm_dma_data); - return ret; - }
snd_soc_set_runtime_hwparams(substream, &snd_mxs_hardware);
- snd_dmaengine_pcm_set_data(substream, pcm_dma_data); - - return 0; -} - -static int snd_mxs_close(struct snd_pcm_substream *substream) -{ - struct mxs_pcm_dma_data *pcm_dma_data = snd_dmaengine_pcm_get_data(substream); - - snd_dmaengine_pcm_close(substream); - kfree(pcm_dma_data); - - return 0; + return snd_dmaengine_pcm_open(substream, filter, + snd_soc_dai_get_dma_data(rtd->cpu_dai, substream)); }
static int snd_mxs_pcm_mmap(struct snd_pcm_substream *substream, @@ -137,7 +104,7 @@ static int snd_mxs_pcm_mmap(struct snd_pcm_substream *substream,
static struct snd_pcm_ops mxs_pcm_ops = { .open = snd_mxs_open, - .close = snd_mxs_close, + .close = snd_dmaengine_pcm_close, .ioctl = snd_pcm_lib_ioctl, .hw_params = snd_mxs_pcm_hw_params, .trigger = snd_dmaengine_pcm_trigger, diff --git a/sound/soc/mxs/mxs-pcm.h b/sound/soc/mxs/mxs-pcm.h index 35ba2ca..3aa918f 100644 --- a/sound/soc/mxs/mxs-pcm.h +++ b/sound/soc/mxs/mxs-pcm.h @@ -19,8 +19,10 @@ #ifndef _MXS_PCM_H #define _MXS_PCM_H
+#include <linux/fsl/mxs-dma.h> + struct mxs_pcm_dma_params { - int chan_irq; + struct mxs_dma_data dma_data; int chan_num; };
diff --git a/sound/soc/mxs/mxs-saif.c b/sound/soc/mxs/mxs-saif.c index 3a2aa1d..f13bd87 100644 --- a/sound/soc/mxs/mxs-saif.c +++ b/sound/soc/mxs/mxs-saif.c @@ -753,9 +753,9 @@ static int mxs_saif_probe(struct platform_device *pdev) return ret; }
- saif->dma_param.chan_irq = platform_get_irq(pdev, 1); - if (saif->dma_param.chan_irq < 0) { - ret = saif->dma_param.chan_irq; + saif->dma_param.dma_data.chan_irq = platform_get_irq(pdev, 1); + if (saif->dma_param.dma_data.chan_irq < 0) { + ret = saif->dma_param.dma_data.chan_irq; dev_err(&pdev->dev, "failed to get dma irq resource: %d\n", ret); return ret;
On Fri, Mar 22, 2013 at 02:12:13PM +0100, Lars-Peter Clausen wrote:
Currently the mxs_dma_data struct, which gets passed to the dmaengine driver, is allocated in the pcm driver's open callback. The mxs_dma_data struct has exactly one field which is initialized from the the same field in the mxs_pcm_dma_params struct. The mxs_pcm_dma_params struct gets passed to the pcm driver from the dai driver. Instead of taking this indirection embed the mxs_dma_data struct directly in the mxs_pcm_dma_params struct. This allows us to simplify the pcm driver quite a bit, since we don't have to care about memory managing the mxs_dma_data struct anymore.
Signed-off-by: Lars-Peter Clausen lars@metafoo.de Cc: Shawn Guo shawn.guo@linaro.org
Tested-by: Shawn Guo shawn.guo@linaro.org
These functions were initially added to be able to support some oddball dma drivers, but all users have been updated to deal with the situation without the help of snd_dmaengine_pcm_{set,get}_data, so these two functions can be removed.
Signed-off-by: Lars-Peter Clausen lars@metafoo.de --- include/sound/dmaengine_pcm.h | 3 --- sound/soc/soc-dmaengine-pcm.c | 29 ----------------------------- 2 files changed, 32 deletions(-)
diff --git a/include/sound/dmaengine_pcm.h b/include/sound/dmaengine_pcm.h index b877334..f8a7031 100644 --- a/include/sound/dmaengine_pcm.h +++ b/include/sound/dmaengine_pcm.h @@ -32,9 +32,6 @@ snd_pcm_substream_to_dma_direction(const struct snd_pcm_substream *substream) return DMA_DEV_TO_MEM; }
-void snd_dmaengine_pcm_set_data(struct snd_pcm_substream *substream, void *data); -void *snd_dmaengine_pcm_get_data(struct snd_pcm_substream *substream); - int snd_hwparams_to_dma_slave_config(const struct snd_pcm_substream *substream, const struct snd_pcm_hw_params *params, struct dma_slave_config *slave_config); int snd_dmaengine_pcm_trigger(struct snd_pcm_substream *substream, int cmd); diff --git a/sound/soc/soc-dmaengine-pcm.c b/sound/soc/soc-dmaengine-pcm.c index 111b7d92..e8b1215 100644 --- a/sound/soc/soc-dmaengine-pcm.c +++ b/sound/soc/soc-dmaengine-pcm.c @@ -33,8 +33,6 @@ struct dmaengine_pcm_runtime_data { dma_cookie_t cookie;
unsigned int pos; - - void *data; };
static inline struct dmaengine_pcm_runtime_data *substream_to_prtd( @@ -43,33 +41,6 @@ static inline struct dmaengine_pcm_runtime_data *substream_to_prtd( return substream->runtime->private_data; }
-/** - * snd_dmaengine_pcm_set_data - Set dmaengine substream private data - * @substream: PCM substream - * @data: Data to set - */ -void snd_dmaengine_pcm_set_data(struct snd_pcm_substream *substream, void *data) -{ - struct dmaengine_pcm_runtime_data *prtd = substream_to_prtd(substream); - - prtd->data = data; -} -EXPORT_SYMBOL_GPL(snd_dmaengine_pcm_set_data); - -/** - * snd_dmaengine_pcm_get_data - Get dmaeinge substream private data - * @substream: PCM substream - * - * Returns the data previously set with snd_dmaengine_pcm_set_data - */ -void *snd_dmaengine_pcm_get_data(struct snd_pcm_substream *substream) -{ - struct dmaengine_pcm_runtime_data *prtd = substream_to_prtd(substream); - - return prtd->data; -} -EXPORT_SYMBOL_GPL(snd_dmaengine_pcm_get_data); - struct dma_chan *snd_dmaengine_pcm_get_chan(struct snd_pcm_substream *substream) { struct dmaengine_pcm_runtime_data *prtd = substream_to_prtd(substream);
participants (9)
-
Bo Shen -
Jarkko Nikula -
Lars-Peter Clausen -
Mark Brown -
Ola Lilja -
Peter Ujfalusi -
Rajeev kumar -
Shawn Guo -
Stephen Warren