[alsa-devel] [PATCH 0/6] ASoC: platform cleanup

Kuninori Morimoto

19 Dec 2016 19 Dec '16

8:33 a.m.

Hi Mark, Lars-Peter

These removes un-used platform feature, and move last platform feature to component. If these are OK, all platform feature goes to component. Last platform piece is platform_list on soc-core.c

Kuninori Morimoto (6): ASoC: remove .delay from snd_soc_platform_driver ASoC: remove .bespoke_trigger from snd_soc_platform_driver ASoC: remove snd_soc_platform_trigger() ASoC: add for_each_component{_safe} ASoC: add Component level struct snd_compr_ops ASoC: add Component level struct snd_pcm_ops

include/sound/soc.h | 23 +++-- sound/soc/soc-compress.c | 228 ++++++++++++++++++++++++++++--------------- sound/soc/soc-core.c | 14 +-- sound/soc/soc-pcm.c | 244 +++++++++++++++++++++++++++++++++-------------- 4 files changed, 339 insertions(+), 170 deletions(-)

-- 1.9.1

Show replies by date

Kuninori Morimoto

19 Dec 19 Dec

8:36 a.m.

New subject: [alsa-devel] [PATCH 1/6] ASoC: remove .delay from snd_soc_platform_driver

From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com

No existing platform is using .delay. Let's remove it.

Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- include/sound/soc.h | 7 ------- sound/soc/soc-pcm.c | 7 ------- 2 files changed, 14 deletions(-)

diff --git a/include/sound/soc.h b/include/sound/soc.h index e580a67..06515e5 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -946,13 +946,6 @@ struct snd_soc_platform_driver { int (*pcm_new)(struct snd_soc_pcm_runtime *); void (*pcm_free)(struct snd_pcm *);

- /* - * For platform caused delay reporting. - * Optional. - */ - snd_pcm_sframes_t (*delay)(struct snd_pcm_substream *, - struct snd_soc_dai *); - /* platform stream pcm ops */ const struct snd_pcm_ops *ops;

diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index a9ef8ae..a4c93a9 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -1116,13 +1116,6 @@ static snd_pcm_uframes_t soc_pcm_pointer(struct snd_pcm_substream *substream) } delay += codec_delay;

- /* - * None of the existing platform drivers implement delay(), so - * for now the codec_dai of first multicodec entry is used - */ - if (platform->driver->delay) - delay += platform->driver->delay(substream, rtd->codec_dais[0]); - runtime->delay = delay;

return offset;

-- 1.9.1

Mark Brown

9 Jan 9 Jan

9:01 p.m.

New subject: [alsa-devel] Applied "ASoC: remove .delay from snd_soc_platform_driver" to the asoc tree

The patch

ASoC: remove .delay from snd_soc_platform_driver

has been applied to the asoc tree at

git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git

All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying to this mail.

Thanks, Mark

...

From fcff45f8e092c17d324028fb6f632fde98983f17 Mon Sep 17 00:00:00 2001

From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Date: Mon, 19 Dec 2016 07:36:58 +0000 Subject: [PATCH] ASoC: remove .delay from snd_soc_platform_driver

No existing platform is using .delay. Let's remove it.

Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Signed-off-by: Mark Brown broonie@kernel.org --- include/sound/soc.h | 7 ------- sound/soc/soc-pcm.c | 7 ------- 2 files changed, 14 deletions(-)

diff --git a/include/sound/soc.h b/include/sound/soc.h index e580a675ea77..06515e5ca018 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -946,13 +946,6 @@ struct snd_soc_platform_driver { int (*pcm_new)(struct snd_soc_pcm_runtime *); void (*pcm_free)(struct snd_pcm *);

diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index a9ef8ae20e44..a4c93a90b8e9 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -1116,13 +1116,6 @@ static snd_pcm_uframes_t soc_pcm_pointer(struct snd_pcm_substream *substream) } delay += codec_delay;

return offset;

-- 2.11.0

Kuninori Morimoto

19 Dec 19 Dec

8:37 a.m.

New subject: [alsa-devel] [PATCH 2/6] ASoC: remove .bespoke_trigger from snd_soc_platform_driver

From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com

No existing platform is using .bespoke_trigger. Let's remove it.

Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- include/sound/soc.h | 2 -- sound/soc/soc-pcm.c | 7 ------- 2 files changed, 9 deletions(-)

diff --git a/include/sound/soc.h b/include/sound/soc.h index 06515e5..1a4311d 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -951,8 +951,6 @@ struct snd_soc_platform_driver {

/* platform stream compress ops */ const struct snd_compr_ops *compr_ops; - - int (*bespoke_trigger)(struct snd_pcm_substream *, int); };

struct snd_soc_dai_link_component { diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index a4c93a9..1739573 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -1055,7 +1055,6 @@ static int soc_pcm_bespoke_trigger(struct snd_pcm_substream *substream, int cmd) { struct snd_soc_pcm_runtime *rtd = substream->private_data; - struct snd_soc_platform *platform = rtd->platform; struct snd_soc_dai *cpu_dai = rtd->cpu_dai; struct snd_soc_dai *codec_dai; int i, ret; @@ -1071,12 +1070,6 @@ static int soc_pcm_bespoke_trigger(struct snd_pcm_substream *substream, } }

- if (platform->driver->bespoke_trigger) { - ret = platform->driver->bespoke_trigger(substream, cmd); - if (ret < 0) - return ret; - } - if (cpu_dai->driver->ops && cpu_dai->driver->ops->bespoke_trigger) { ret = cpu_dai->driver->ops->bespoke_trigger(substream, cmd, cpu_dai); if (ret < 0)

-- 1.9.1

Mark Brown

9 Jan 9 Jan

9:01 p.m.

New subject: [alsa-devel] Applied "ASoC: remove .bespoke_trigger from snd_soc_platform_driver" to the asoc tree

The patch

ASoC: remove .bespoke_trigger from snd_soc_platform_driver

has been applied to the asoc tree at

git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git

If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying to this mail.

Thanks, Mark

...

From 10611e1b0b7ab2a82dd7838e5e928fa1501d353c Mon Sep 17 00:00:00 2001

From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Date: Mon, 19 Dec 2016 07:37:18 +0000 Subject: [PATCH] ASoC: remove .bespoke_trigger from snd_soc_platform_driver

No existing platform is using .bespoke_trigger. Let's remove it.

Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Signed-off-by: Mark Brown broonie@kernel.org --- include/sound/soc.h | 2 -- sound/soc/soc-pcm.c | 7 ------- 2 files changed, 9 deletions(-)

diff --git a/include/sound/soc.h b/include/sound/soc.h index 06515e5ca018..1a4311da6126 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -951,8 +951,6 @@ struct snd_soc_platform_driver {

/* platform stream compress ops */ const struct snd_compr_ops *compr_ops; - - int (*bespoke_trigger)(struct snd_pcm_substream *, int); };

struct snd_soc_dai_link_component { diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index a4c93a90b8e9..1739573dcd6a 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -1055,7 +1055,6 @@ static int soc_pcm_bespoke_trigger(struct snd_pcm_substream *substream, int cmd) { struct snd_soc_pcm_runtime *rtd = substream->private_data; - struct snd_soc_platform *platform = rtd->platform; struct snd_soc_dai *cpu_dai = rtd->cpu_dai; struct snd_soc_dai *codec_dai; int i, ret; @@ -1071,12 +1070,6 @@ static int soc_pcm_bespoke_trigger(struct snd_pcm_substream *substream, } }

-- 2.11.0

Kuninori Morimoto

19 Dec 19 Dec

8:37 a.m.

New subject: [alsa-devel] [PATCH 3/6] ASoC: remove snd_soc_platform_trigger()

From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com

No one is using snd_soc_platform_trigger(). Let's remove it.

Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- include/sound/soc.h | 3 --- sound/soc/soc-pcm.c | 9 --------- 2 files changed, 12 deletions(-)

diff --git a/include/sound/soc.h b/include/sound/soc.h index 1a4311d..4504920 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -507,9 +507,6 @@ int snd_soc_runtime_set_dai_fmt(struct snd_soc_pcm_runtime *rtd, int snd_soc_set_runtime_hwparams(struct snd_pcm_substream *substream, const struct snd_pcm_hardware *hw);

-int snd_soc_platform_trigger(struct snd_pcm_substream *substream, - int cmd, struct snd_soc_platform *platform); - int soc_dai_hw_params(struct snd_pcm_substream *substream, struct snd_pcm_hw_params *params, struct snd_soc_dai *dai); diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index 1739573..dec0b20 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -2872,15 +2872,6 @@ int snd_soc_dpcm_can_be_params(struct snd_soc_pcm_runtime *fe, } EXPORT_SYMBOL_GPL(snd_soc_dpcm_can_be_params);

-int snd_soc_platform_trigger(struct snd_pcm_substream *substream, - int cmd, struct snd_soc_platform *platform) -{ - if (platform->driver->ops && platform->driver->ops->trigger) - return platform->driver->ops->trigger(substream, cmd); - return 0; -} -EXPORT_SYMBOL_GPL(snd_soc_platform_trigger); - #ifdef CONFIG_DEBUG_FS static const char *dpcm_state_string(enum snd_soc_dpcm_state state) {

-- 1.9.1

Mark Brown

9 Jan 9 Jan

9:01 p.m.

New subject: [alsa-devel] Applied "ASoC: remove snd_soc_platform_trigger()" to the asoc tree

The patch

ASoC: remove snd_soc_platform_trigger()

has been applied to the asoc tree at

git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git

If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying to this mail.

Thanks, Mark

...

From 322962384da9739a5000f58e5f69c7832c448e0d Mon Sep 17 00:00:00 2001

From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Date: Mon, 19 Dec 2016 07:37:37 +0000 Subject: [PATCH] ASoC: remove snd_soc_platform_trigger()

No one is using snd_soc_platform_trigger(). Let's remove it.

Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Signed-off-by: Mark Brown broonie@kernel.org --- include/sound/soc.h | 3 --- sound/soc/soc-pcm.c | 9 --------- 2 files changed, 12 deletions(-)

diff --git a/include/sound/soc.h b/include/sound/soc.h index b37b21a3471f..5db4a90b1a02 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -507,9 +507,6 @@ int snd_soc_params_to_bclk(struct snd_pcm_hw_params *parms); int snd_soc_set_runtime_hwparams(struct snd_pcm_substream *substream, const struct snd_pcm_hardware *hw);

-int snd_soc_platform_trigger(struct snd_pcm_substream *substream, - int cmd, struct snd_soc_platform *platform); - int soc_dai_hw_params(struct snd_pcm_substream *substream, struct snd_pcm_hw_params *params, struct snd_soc_dai *dai); diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index 51927dc93c41..ff31bf5e646a 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -2871,15 +2871,6 @@ int snd_soc_dpcm_can_be_params(struct snd_soc_pcm_runtime *fe, } EXPORT_SYMBOL_GPL(snd_soc_dpcm_can_be_params);

-- 2.11.0

Kuninori Morimoto

19 Dec 19 Dec

8:37 a.m.

New subject: [alsa-devel] [PATCH 4/6] ASoC: add for_each_component{_safe}

From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com

To be more readable code, this patch adds new for_each_component macro.

Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- include/sound/soc.h | 5 +++++ sound/soc/soc-core.c | 10 ++++------ sound/soc/soc-pcm.c | 5 ++--- 3 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/include/sound/soc.h b/include/sound/soc.h index 4504920..5db4a90 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -1190,6 +1190,11 @@ struct snd_soc_card {

void *drvdata; }; +#define for_each_component(component, card) \ + list_for_each_entry(component, &(card)->component_dev_list, card_list) +#define for_each_component_safe(component, _component, card) \ + list_for_each_entry_safe(component, _component, \ + &(card)->component_dev_list, card_list)

/* SoC machine DAI configuration, glues a codec and cpu DAI together */ struct snd_soc_pcm_runtime { diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 981443e..9f2b9e1 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -703,7 +703,7 @@ int snd_soc_suspend(struct device *dev) snd_soc_dapm_sync(&card->dapm);

/* suspend all COMPONENTs */ - list_for_each_entry(component, &card->component_dev_list, card_list) { + for_each_component(component, card) { struct snd_soc_dapm_context *dapm = snd_soc_component_get_dapm(component);

/* If there are paths active then the COMPONENT will be held with @@ -794,7 +794,7 @@ static void soc_resume_deferred(struct work_struct *work) cpu_dai->driver->resume(cpu_dai); }

- list_for_each_entry(component, &card->component_dev_list, card_list) { + for_each_component(component, card) { if (component->suspended) { if (component->resume) component->resume(component); @@ -1764,7 +1764,7 @@ static int soc_probe_aux_devices(struct snd_soc_card *card)

for (order = SND_SOC_COMP_ORDER_FIRST; order <= SND_SOC_COMP_ORDER_LAST; order++) { - list_for_each_entry(comp, &card->component_dev_list, card_list) { + for_each_component(comp, card) { if (!comp->auxiliary) continue;

@@ -1790,9 +1790,7 @@ static void soc_remove_aux_devices(struct snd_soc_card *card)

for (order = SND_SOC_COMP_ORDER_FIRST; order <= SND_SOC_COMP_ORDER_LAST; order++) { - list_for_each_entry_safe(comp, _comp, - &card->component_dev_list, card_list) { - + for_each_component_safe(comp, _comp, card) { if (!comp->auxiliary) continue;

diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index dec0b20..ff31bf5 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -2631,8 +2631,7 @@ static void soc_pcm_free(struct snd_pcm *pcm) struct snd_soc_pcm_runtime *rtd = pcm->private_data; struct snd_soc_component *component;

- list_for_each_entry(component, &rtd->card->component_dev_list, - card_list) { + for_each_component(component, rtd->card) { if (component->pcm_free) component->pcm_free(pcm); } @@ -2753,7 +2752,7 @@ int soc_new_pcm(struct snd_soc_pcm_runtime *rtd, int num) if (capture) snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_CAPTURE, &rtd->ops);

- list_for_each_entry(component, &rtd->card->component_dev_list, card_list) { + for_each_component(component, rtd->card) { if (component->pcm_new) { ret = component->pcm_new(rtd); if (ret < 0) {

-- 1.9.1

Mark Brown

9 Jan 9 Jan

9:01 p.m.

New subject: [alsa-devel] Applied "ASoC: add for_each_component{_safe}" to the asoc tree

The patch

ASoC: add for_each_component{_safe}

has been applied to the asoc tree at

git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git

If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying to this mail.

Thanks, Mark

...

From d75e460ad330518b56635cfccfba78c90ff0c9df Mon Sep 17 00:00:00 2001

From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Date: Mon, 19 Dec 2016 07:37:57 +0000 Subject: [PATCH] ASoC: add for_each_component{_safe}

To be more readable code, this patch adds new for_each_component macro.

Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Signed-off-by: Mark Brown broonie@kernel.org --- include/sound/soc.h | 5 +++++ sound/soc/soc-core.c | 10 ++++------ sound/soc/soc-pcm.c | 5 ++--- 3 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/include/sound/soc.h b/include/sound/soc.h index 1a4311da6126..b37b21a3471f 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -1193,6 +1193,11 @@ struct snd_soc_card {

/* SoC machine DAI configuration, glues a codec and cpu DAI together */ struct snd_soc_pcm_runtime { diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 981443e444d1..9f2b9e1a1cee 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -703,7 +703,7 @@ int snd_soc_suspend(struct device *dev) snd_soc_dapm_sync(&card->dapm);

/* If there are paths active then the COMPONENT will be held with @@ -794,7 +794,7 @@ static void soc_resume_deferred(struct work_struct *work) cpu_dai->driver->resume(cpu_dai); }

@@ -1790,9 +1790,7 @@ static void soc_remove_aux_devices(struct snd_soc_card *card)

diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index 1739573dcd6a..51927dc93c41 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -2631,8 +2631,7 @@ static void soc_pcm_free(struct snd_pcm *pcm) struct snd_soc_pcm_runtime *rtd = pcm->private_data; struct snd_soc_component *component;

- list_for_each_entry(component, &rtd->card->component_dev_list, card_list) { + for_each_component(component, rtd->card) { if (component->pcm_new) { ret = component->pcm_new(rtd); if (ret < 0) {

-- 2.11.0

Mark Brown

10 Jan 10 Jan

1:13 p.m.

New subject: [alsa-devel] Applied "ASoC: add for_each_component{_safe}" to the asoc tree

On Mon, Jan 09, 2017 at 08:01:24PM +0000, Mark Brown wrote:

...

The patch

ASoC: add for_each_component{_safe}

has been applied to the asoc tree at

git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git

Sorry, I dropped this one - it was conflicting nastily with the fix that went in for handling of aux_devs.

Kuninori Morimoto

11 Jan 11 Jan

7:46 a.m.

New subject: [alsa-devel] Applied "ASoC: add for_each_component{_safe}" to the asoc tree

Hi Mark

...

...
The patch

ASoC: add for_each_component{_safe}

has been applied to the asoc tree at

git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git

Sorry, I dropped this one - it was conflicting nastily with the fix that went in for handling of aux_devs.

OK, no problem

Kuninori Morimoto

19 Dec 19 Dec

8:38 a.m.

New subject: [alsa-devel] [PATCH 5/6] ASoC: add Component level struct snd_compr_ops

From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com

In current ALSA SoC, Platform has struct snd_compr_ops feature, but it should be supported on Component level. This patch adds it.

If component level has it, many snd_pcm_ops can be called, but, 1st ops function only will be used now. It should/will be fixed in the future.

Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- include/sound/soc.h | 4 + sound/soc/soc-compress.c | 246 ++++++++++++++++++++++++++++++++--------------- sound/soc/soc-core.c | 2 + 3 files changed, 175 insertions(+), 77 deletions(-)

diff --git a/include/sound/soc.h b/include/sound/soc.h index 5db4a90..225e9b6 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -777,6 +777,8 @@ struct snd_soc_component_driver { const struct snd_soc_dapm_route *dapm_routes; unsigned int num_dapm_routes;

+ const struct snd_compr_ops *compr_ops; + int (*probe)(struct snd_soc_component *); void (*remove)(struct snd_soc_component *); int (*suspend)(struct snd_soc_component *); diff --git a/sound/soc/soc-compress.c b/sound/soc/soc-compress.c index bfd71b8..0362dd7 100644 --- a/sound/soc/soc-compress.c +++ b/sound/soc/soc-compress.c @@ -29,7 +29,7 @@ static int soc_compr_open(struct snd_compr_stream *cstream) { struct snd_soc_pcm_runtime *rtd = cstream->private_data; - struct snd_soc_platform *platform = rtd->platform; + struct snd_soc_component *component; struct snd_soc_dai *cpu_dai = rtd->cpu_dai; int ret = 0;

@@ -44,14 +44,22 @@ static int soc_compr_open(struct snd_compr_stream *cstream) } }

- if (platform->driver->compr_ops && platform->driver->compr_ops->open) { - ret = platform->driver->compr_ops->open(cstream); - if (ret < 0) { - pr_err("compress asoc: can't open platform %s\n", - platform->component.name); - goto plat_err; + ret = 0; + for_each_component(component, rtd->card) { + const struct snd_compr_ops *compr_ops = component->compr_ops; + + if (compr_ops && compr_ops->open) { + int _ret = compr_ops->open(cstream); + + if (_ret < 0) { + pr_err("compress asoc: can't open component %s\n", + component->name); + ret |= _ret; + } } } + if (ret < 0) + goto machine_err;

if (rtd->dai_link->compr_ops && rtd->dai_link->compr_ops->startup) { ret = rtd->dai_link->compr_ops->startup(cstream); @@ -68,9 +76,13 @@ static int soc_compr_open(struct snd_compr_stream *cstream) return 0;

machine_err: - if (platform->driver->compr_ops && platform->driver->compr_ops->free) - platform->driver->compr_ops->free(cstream); -plat_err: + for_each_component(component, rtd->card) { + const struct snd_compr_ops *compr_ops = component->compr_ops; + + if (compr_ops && compr_ops->free) + compr_ops->free(cstream); + } + if (cpu_dai->driver->cops && cpu_dai->driver->cops->shutdown) cpu_dai->driver->cops->shutdown(cstream, cpu_dai); out: @@ -82,7 +94,7 @@ static int soc_compr_open_fe(struct snd_compr_stream *cstream) { struct snd_soc_pcm_runtime *fe = cstream->private_data; struct snd_pcm_substream *fe_substream = fe->pcm->streams[0].substream; - struct snd_soc_platform *platform = fe->platform; + struct snd_soc_component *component; struct snd_soc_dai *cpu_dai = fe->cpu_dai; struct snd_soc_dpcm *dpcm; struct snd_soc_dapm_widget_list *list; @@ -105,15 +117,22 @@ static int soc_compr_open_fe(struct snd_compr_stream *cstream) } }

+ ret = 0; + for_each_component(component, fe->card) { + const struct snd_compr_ops *compr_ops = component->compr_ops;

- if (platform->driver->compr_ops && platform->driver->compr_ops->open) { - ret = platform->driver->compr_ops->open(cstream); - if (ret < 0) { - pr_err("compress asoc: can't open platform %s\n", - platform->component.name); - goto plat_err; + if (compr_ops && compr_ops->open) { + int _ret = compr_ops->open(cstream); + + if (_ret < 0) { + pr_err("compress asoc: can't open component %s\n", + component->name); + ret |= _ret; + } } } + if (ret < 0) + goto machine_err;

if (fe->dai_link->compr_ops && fe->dai_link->compr_ops->startup) { ret = fe->dai_link->compr_ops->startup(cstream); @@ -166,9 +185,13 @@ static int soc_compr_open_fe(struct snd_compr_stream *cstream) if (fe->dai_link->compr_ops && fe->dai_link->compr_ops->shutdown) fe->dai_link->compr_ops->shutdown(cstream); machine_err: - if (platform->driver->compr_ops && platform->driver->compr_ops->free) - platform->driver->compr_ops->free(cstream); -plat_err: + for_each_component(component, fe->card) { + const struct snd_compr_ops *compr_ops = component->compr_ops; + + if (compr_ops && compr_ops->free) + compr_ops->free(cstream); + } + if (cpu_dai->driver->cops && cpu_dai->driver->cops->shutdown) cpu_dai->driver->cops->shutdown(cstream, cpu_dai); out: @@ -208,7 +231,7 @@ static void close_delayed_work(struct work_struct *work) static int soc_compr_free(struct snd_compr_stream *cstream) { struct snd_soc_pcm_runtime *rtd = cstream->private_data; - struct snd_soc_platform *platform = rtd->platform; + struct snd_soc_component *component; struct snd_soc_dai *cpu_dai = rtd->cpu_dai; struct snd_soc_dai *codec_dai = rtd->codec_dai; int stream; @@ -234,8 +257,12 @@ static int soc_compr_free(struct snd_compr_stream *cstream) if (rtd->dai_link->compr_ops && rtd->dai_link->compr_ops->shutdown) rtd->dai_link->compr_ops->shutdown(cstream);

- if (platform->driver->compr_ops && platform->driver->compr_ops->free) - platform->driver->compr_ops->free(cstream); + for_each_component(component, rtd->card) { + const struct snd_compr_ops *compr_ops = component->compr_ops; + + if (compr_ops && compr_ops->free) + compr_ops->free(cstream); + }

if (cpu_dai->driver->cops && cpu_dai->driver->cops->shutdown) cpu_dai->driver->cops->shutdown(cstream, cpu_dai); @@ -265,7 +292,7 @@ static int soc_compr_free(struct snd_compr_stream *cstream) static int soc_compr_free_fe(struct snd_compr_stream *cstream) { struct snd_soc_pcm_runtime *fe = cstream->private_data; - struct snd_soc_platform *platform = fe->platform; + struct snd_soc_component *component; struct snd_soc_dai *cpu_dai = fe->cpu_dai; struct snd_soc_dpcm *dpcm; int stream, ret; @@ -303,8 +330,12 @@ static int soc_compr_free_fe(struct snd_compr_stream *cstream) if (fe->dai_link->compr_ops && fe->dai_link->compr_ops->shutdown) fe->dai_link->compr_ops->shutdown(cstream);

- if (platform->driver->compr_ops && platform->driver->compr_ops->free) - platform->driver->compr_ops->free(cstream); + for_each_component(component, fe->card) { + const struct snd_compr_ops *compr_ops = component->compr_ops; + + if (compr_ops && compr_ops->free) + compr_ops->free(cstream); + }

if (cpu_dai->driver->cops && cpu_dai->driver->cops->shutdown) cpu_dai->driver->cops->shutdown(cstream, cpu_dai); @@ -317,17 +348,21 @@ static int soc_compr_trigger(struct snd_compr_stream *cstream, int cmd) {

struct snd_soc_pcm_runtime *rtd = cstream->private_data; - struct snd_soc_platform *platform = rtd->platform; + struct snd_soc_component *component; struct snd_soc_dai *codec_dai = rtd->codec_dai; struct snd_soc_dai *cpu_dai = rtd->cpu_dai; int ret = 0;

mutex_lock_nested(&rtd->pcm_mutex, rtd->pcm_subclass);

- if (platform->driver->compr_ops && platform->driver->compr_ops->trigger) { - ret = platform->driver->compr_ops->trigger(cstream, cmd); - if (ret < 0) - goto out; + for_each_component(component, rtd->card) { + const struct snd_compr_ops *compr_ops = component->compr_ops; + + if (compr_ops && compr_ops->trigger) { + ret = compr_ops->trigger(cstream, cmd); + if (ret < 0) + goto out; + } }

if (cpu_dai->driver->cops && cpu_dai->driver->cops->trigger) @@ -351,17 +386,20 @@ static int soc_compr_trigger(struct snd_compr_stream *cstream, int cmd) static int soc_compr_trigger_fe(struct snd_compr_stream *cstream, int cmd) { struct snd_soc_pcm_runtime *fe = cstream->private_data; - struct snd_soc_platform *platform = fe->platform; + struct snd_soc_component *component; struct snd_soc_dai *cpu_dai = fe->cpu_dai; int ret = 0, stream;

if (cmd == SND_COMPR_TRIGGER_PARTIAL_DRAIN || cmd == SND_COMPR_TRIGGER_DRAIN) {

- if (platform->driver->compr_ops && - platform->driver->compr_ops->trigger) - return platform->driver->compr_ops->trigger(cstream, - cmd); + for_each_component(component, fe->card) { + const struct snd_compr_ops *compr_ops = component->compr_ops; + + /* FIXME: It uses 1st function only now */ + if (compr_ops && compr_ops->trigger) + return compr_ops->trigger(cstream, cmd); + } }

if (cstream->direction == SND_COMPRESS_PLAYBACK) @@ -378,10 +416,14 @@ static int soc_compr_trigger_fe(struct snd_compr_stream *cstream, int cmd) goto out; }

- if (platform->driver->compr_ops && platform->driver->compr_ops->trigger) { - ret = platform->driver->compr_ops->trigger(cstream, cmd); - if (ret < 0) - goto out; + for_each_component(component, fe->card) { + const struct snd_compr_ops *compr_ops = component->compr_ops; + + if (compr_ops && compr_ops->trigger) { + ret = compr_ops->trigger(cstream, cmd); + if (ret < 0) + goto out; + } }

fe->dpcm[stream].runtime_update = SND_SOC_DPCM_UPDATE_FE; @@ -413,7 +455,7 @@ static int soc_compr_set_params(struct snd_compr_stream *cstream, struct snd_compr_params *params) { struct snd_soc_pcm_runtime *rtd = cstream->private_data; - struct snd_soc_platform *platform = rtd->platform; + struct snd_soc_component *component; struct snd_soc_dai *cpu_dai = rtd->cpu_dai; int ret = 0;

@@ -431,10 +473,14 @@ static int soc_compr_set_params(struct snd_compr_stream *cstream, goto err; }

- if (platform->driver->compr_ops && platform->driver->compr_ops->set_params) { - ret = platform->driver->compr_ops->set_params(cstream, params); - if (ret < 0) - goto err; + for_each_component(component, rtd->card) { + const struct snd_compr_ops *compr_ops = component->compr_ops; + + if (compr_ops && compr_ops->set_params) { + ret = compr_ops->set_params(cstream, params); + if (ret < 0) + goto err; + } }

if (rtd->dai_link->compr_ops && rtd->dai_link->compr_ops->set_params) { @@ -468,7 +514,7 @@ static int soc_compr_set_params_fe(struct snd_compr_stream *cstream, { struct snd_soc_pcm_runtime *fe = cstream->private_data; struct snd_pcm_substream *fe_substream = fe->pcm->streams[0].substream; - struct snd_soc_platform *platform = fe->platform; + struct snd_soc_component *component; struct snd_soc_dai *cpu_dai = fe->cpu_dai; int ret = 0, stream;

@@ -485,10 +531,14 @@ static int soc_compr_set_params_fe(struct snd_compr_stream *cstream, goto out; }

- if (platform->driver->compr_ops && platform->driver->compr_ops->set_params) { - ret = platform->driver->compr_ops->set_params(cstream, params); - if (ret < 0) - goto out; + for_each_component(component, fe->card) { + const struct snd_compr_ops *compr_ops = component->compr_ops; + + if (compr_ops && compr_ops->set_params) { + ret = compr_ops->set_params(cstream, params); + if (ret < 0) + goto out; + } }

if (fe->dai_link->compr_ops && fe->dai_link->compr_ops->set_params) { @@ -528,7 +578,7 @@ static int soc_compr_get_params(struct snd_compr_stream *cstream, struct snd_codec *params) { struct snd_soc_pcm_runtime *rtd = cstream->private_data; - struct snd_soc_platform *platform = rtd->platform; + struct snd_soc_component *component; struct snd_soc_dai *cpu_dai = rtd->cpu_dai; int ret = 0;

@@ -540,8 +590,13 @@ static int soc_compr_get_params(struct snd_compr_stream *cstream, goto err; }

- if (platform->driver->compr_ops && platform->driver->compr_ops->get_params) - ret = platform->driver->compr_ops->get_params(cstream, params); + ret = 0; + for_each_component(component, rtd->card) { + const struct snd_compr_ops *compr_ops = component->compr_ops; + + if (compr_ops && compr_ops->get_params) + ret |= compr_ops->get_params(cstream, params); + }

err: mutex_unlock(&rtd->pcm_mutex); @@ -552,13 +607,17 @@ static int soc_compr_get_caps(struct snd_compr_stream *cstream, struct snd_compr_caps *caps) { struct snd_soc_pcm_runtime *rtd = cstream->private_data; - struct snd_soc_platform *platform = rtd->platform; + struct snd_soc_component *component; int ret = 0;

mutex_lock_nested(&rtd->pcm_mutex, rtd->pcm_subclass);

- if (platform->driver->compr_ops && platform->driver->compr_ops->get_caps) - ret = platform->driver->compr_ops->get_caps(cstream, caps); + for_each_component(component, rtd->card) { + const struct snd_compr_ops *compr_ops = component->compr_ops; + + if (compr_ops && compr_ops->get_caps) + ret |= compr_ops->get_caps(cstream, caps); + }

mutex_unlock(&rtd->pcm_mutex); return ret; @@ -568,13 +627,17 @@ static int soc_compr_get_codec_caps(struct snd_compr_stream *cstream, struct snd_compr_codec_caps *codec) { struct snd_soc_pcm_runtime *rtd = cstream->private_data; - struct snd_soc_platform *platform = rtd->platform; + struct snd_soc_component *component; int ret = 0;

mutex_lock_nested(&rtd->pcm_mutex, rtd->pcm_subclass);

- if (platform->driver->compr_ops && platform->driver->compr_ops->get_codec_caps) - ret = platform->driver->compr_ops->get_codec_caps(cstream, codec); + for_each_component(component, rtd->card) { + const struct snd_compr_ops *compr_ops = component->compr_ops; + + if (compr_ops && compr_ops->get_codec_caps) + ret |= compr_ops->get_codec_caps(cstream, codec); + }

mutex_unlock(&rtd->pcm_mutex); return ret; @@ -583,7 +646,7 @@ static int soc_compr_get_codec_caps(struct snd_compr_stream *cstream, static int soc_compr_ack(struct snd_compr_stream *cstream, size_t bytes) { struct snd_soc_pcm_runtime *rtd = cstream->private_data; - struct snd_soc_platform *platform = rtd->platform; + struct snd_soc_component *component; struct snd_soc_dai *cpu_dai = rtd->cpu_dai; int ret = 0;

@@ -595,9 +658,13 @@ static int soc_compr_ack(struct snd_compr_stream *cstream, size_t bytes) goto err; }

- if (platform->driver->compr_ops && platform->driver->compr_ops->ack) - ret = platform->driver->compr_ops->ack(cstream, bytes); + ret = 0; + for_each_component(component, rtd->card) { + const struct snd_compr_ops *compr_ops = component->compr_ops;

+ if (compr_ops && compr_ops->ack) + ret |= compr_ops->ack(cstream, bytes); + } err: mutex_unlock(&rtd->pcm_mutex); return ret; @@ -607,7 +674,7 @@ static int soc_compr_pointer(struct snd_compr_stream *cstream, struct snd_compr_tstamp *tstamp) { struct snd_soc_pcm_runtime *rtd = cstream->private_data; - struct snd_soc_platform *platform = rtd->platform; + struct snd_soc_component *component; int ret = 0; struct snd_soc_dai *cpu_dai = rtd->cpu_dai;

@@ -616,8 +683,13 @@ static int soc_compr_pointer(struct snd_compr_stream *cstream, if (cpu_dai->driver->cops && cpu_dai->driver->cops->pointer) cpu_dai->driver->cops->pointer(cstream, tstamp, cpu_dai);

- if (platform->driver->compr_ops && platform->driver->compr_ops->pointer) - ret = platform->driver->compr_ops->pointer(cstream, tstamp); + ret = 0; + for_each_component(component, rtd->card) { + const struct snd_compr_ops *compr_ops = component->compr_ops; + + if (compr_ops && compr_ops->pointer) + ret |= compr_ops->pointer(cstream, tstamp); + }

mutex_unlock(&rtd->pcm_mutex); return ret; @@ -627,13 +699,17 @@ static int soc_compr_copy(struct snd_compr_stream *cstream, char __user *buf, size_t count) { struct snd_soc_pcm_runtime *rtd = cstream->private_data; - struct snd_soc_platform *platform = rtd->platform; + struct snd_soc_component *component; int ret = 0;

mutex_lock_nested(&rtd->pcm_mutex, rtd->pcm_subclass);

- if (platform->driver->compr_ops && platform->driver->compr_ops->copy) - ret = platform->driver->compr_ops->copy(cstream, buf, count); + for_each_component(component, rtd->card) { + const struct snd_compr_ops *compr_ops = component->compr_ops; + + if (compr_ops && compr_ops->copy) + ret |= compr_ops->copy(cstream, buf, count); + }

mutex_unlock(&rtd->pcm_mutex); return ret; @@ -643,7 +719,7 @@ static int soc_compr_set_metadata(struct snd_compr_stream *cstream, struct snd_compr_metadata *metadata) { struct snd_soc_pcm_runtime *rtd = cstream->private_data; - struct snd_soc_platform *platform = rtd->platform; + struct snd_soc_component *component; struct snd_soc_dai *cpu_dai = rtd->cpu_dai; int ret = 0;

@@ -653,8 +729,13 @@ static int soc_compr_set_metadata(struct snd_compr_stream *cstream, return ret; }

- if (platform->driver->compr_ops && platform->driver->compr_ops->set_metadata) - ret = platform->driver->compr_ops->set_metadata(cstream, metadata); + ret = 0; + for_each_component(component, rtd->card) { + const struct snd_compr_ops *compr_ops = component->compr_ops; + + if (compr_ops && compr_ops->set_metadata) + ret |= compr_ops->set_metadata(cstream, metadata); + }

return ret; } @@ -663,7 +744,7 @@ static int soc_compr_get_metadata(struct snd_compr_stream *cstream, struct snd_compr_metadata *metadata) { struct snd_soc_pcm_runtime *rtd = cstream->private_data; - struct snd_soc_platform *platform = rtd->platform; + struct snd_soc_component *component; struct snd_soc_dai *cpu_dai = rtd->cpu_dai; int ret = 0;

@@ -673,8 +754,13 @@ static int soc_compr_get_metadata(struct snd_compr_stream *cstream, return ret; }

- if (platform->driver->compr_ops && platform->driver->compr_ops->get_metadata) - ret = platform->driver->compr_ops->get_metadata(cstream, metadata); + ret = 0; + for_each_component(component, rtd->card) { + const struct snd_compr_ops *compr_ops = component->compr_ops; + + if (compr_ops && compr_ops->get_metadata) + ret |= compr_ops->get_metadata(cstream, metadata); + }

return ret; } @@ -720,11 +806,11 @@ static int soc_compr_get_metadata(struct snd_compr_stream *cstream, int snd_soc_new_compress(struct snd_soc_pcm_runtime *rtd, int num) { struct snd_soc_codec *codec = rtd->codec; - struct snd_soc_platform *platform = rtd->platform; struct snd_soc_dai *codec_dai = rtd->codec_dai; struct snd_soc_dai *cpu_dai = rtd->cpu_dai; struct snd_compr *compr; struct snd_pcm *be_pcm; + struct snd_soc_component *component; char new_name[64]; int ret = 0, direction = 0; int playback = 0, capture = 0; @@ -799,8 +885,14 @@ int snd_soc_new_compress(struct snd_soc_pcm_runtime *rtd, int num) memcpy(compr->ops, &soc_compr_ops, sizeof(soc_compr_ops));

/* Add copy callback for not memory mapped DSPs */ - if (platform->driver->compr_ops && platform->driver->compr_ops->copy) - compr->ops->copy = soc_compr_copy; + for_each_component(component, rtd->card) { + const struct snd_compr_ops *compr_ops = component->compr_ops; + + if (compr_ops && compr_ops->copy) { + compr->ops->copy = soc_compr_copy; + break; + } + }

mutex_init(&compr->lock);

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 9f2b9e1..ecadebe 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -3200,6 +3200,8 @@ int snd_soc_add_platform(struct device *dev, struct snd_soc_platform *platform, platform->component.pcm_new = snd_soc_platform_drv_pcm_new; if (platform_drv->pcm_free) platform->component.pcm_free = snd_soc_platform_drv_pcm_free; + if (platform_drv->compr_ops) + platform->component.compr_ops = platform_drv->compr_ops;

#ifdef CONFIG_DEBUG_FS platform->component.debugfs_prefix = "platform";

-- 1.9.1

Charles Keepax

12:12 p.m.

New subject: [alsa-devel] [PATCH 5/6] ASoC: add Component level struct snd_compr_ops

On Mon, Dec 19, 2016 at 07:38:18AM +0000, Kuninori Morimoto wrote:

...

From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com

In current ALSA SoC, Platform has struct snd_compr_ops feature, but it should be supported on Component level. This patch adds it.

If component level has it, many snd_pcm_ops can be called, but, 1st ops function only will be used now. It should/will be fixed in the future.

Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com

include/sound/soc.h | 4 + sound/soc/soc-compress.c | 246 ++++++++++++++++++++++++++++++++--------------- sound/soc/soc-core.c | 2 + 3 files changed, 175 insertions(+), 77 deletions(-)

diff --git a/include/sound/soc.h b/include/sound/soc.h index 5db4a90..225e9b6 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -777,6 +777,8 @@ struct snd_soc_component_driver { const struct snd_soc_dapm_route *dapm_routes; unsigned int num_dapm_routes;

const struct snd_compr_ops *compr_ops;

int (*probe)(struct snd_soc_component *); void (*remove)(struct snd_soc_component *); int (*suspend)(struct snd_soc_component *);

@@ -855,6 +857,8 @@ struct snd_soc_component { unsigned int num_dapm_routes; struct snd_soc_codec *codec;

const struct snd_compr_ops *compr_ops;

int (*probe)(struct snd_soc_component *); void (*remove)(struct snd_soc_component *); int (*suspend)(struct snd_soc_component *);

diff --git a/sound/soc/soc-compress.c b/sound/soc/soc-compress.c index bfd71b8..0362dd7 100644 --- a/sound/soc/soc-compress.c +++ b/sound/soc/soc-compress.c @@ -29,7 +29,7 @@ static int soc_compr_open(struct snd_compr_stream *cstream) { struct snd_soc_pcm_runtime *rtd = cstream->private_data;

struct snd_soc_platform *platform = rtd->platform;

struct snd_soc_component *component; struct snd_soc_dai *cpu_dai = rtd->cpu_dai; int ret = 0;

@@ -44,14 +44,22 @@ static int soc_compr_open(struct snd_compr_stream *cstream) } }
if (platform->driver->compr_ops && platform->driver->compr_ops->open) {
ret = platform->driver->compr_ops->open(cstream);
if (ret < 0) {
	pr_err("compress asoc: can't open platform %s\n",
		platform->component.name);
	goto plat_err;
ret = 0;

for_each_component(component, rtd->card) {
const struct snd_compr_ops *compr_ops = component->compr_ops;
if (compr_ops && compr_ops->open) {
	int _ret = compr_ops->open(cstream);
	if (_ret < 0) {
		pr_err("compress asoc: can't open component %s\n",
		       component->name);
		ret |= _ret;
	}

Apologies if I am missing something but this really doesn't look like equivalent code? The old code calls the ops for the platform associated with the stream, the new code looks like it will call the ops for every single component in the system that has compressed ops, which would definitely cause issue. Am I missing something here?

Thanks, Charles

Kuninori Morimoto

20 Dec 20 Dec

2:59 a.m.

New subject: [alsa-devel] [PATCH 5/6] ASoC: add Component level struct snd_compr_ops

Hi Charles

Thank you for your review

...

...
if (platform->driver->compr_ops && platform->driver->compr_ops->open) {
ret = platform->driver->compr_ops->open(cstream);
if (ret < 0) {
	pr_err("compress asoc: can't open platform %s\n",
		platform->component.name);
	goto plat_err;
ret = 0;

for_each_component(component, rtd->card) {
const struct snd_compr_ops *compr_ops = component->compr_ops;
if (compr_ops && compr_ops->open) {
	int _ret = compr_ops->open(cstream);
	if (_ret < 0) {
		pr_err("compress asoc: can't open component %s\n",
		       component->name);
		ret |= _ret;
	}
Apologies if I am missing something but this really doesn't look like equivalent code? The old code calls the ops for the platform associated with the stream, the new code looks like it will call the ops for every single component in the system that has compressed ops, which would definitely cause issue. Am I missing something here?

Basically in *current* existing code, platform only has compr_ops. So, converting existing code to new code should be OK.

Current ALSA SoC clearly separating CPU/Codec/Platform, but in these days, new sound device has many features. Thus, it becaming difficult to say "it is XXX" today. For example, some Codec has CPU feature, some CPU has Platform feature etc. So, our (= Me/Lars) plan is that remove current CPU/Codec/Platform separation, and all will be Component connection.

Above code will call every component's compr_ops, currently, it should be platform now. But in the future device, Codec and/or CPU can have compr_ops.

If you want, currently, it can call 1st found compr_ops (= should be platform) only with FIXME comment at this point.

Does this clear answer for you ?

for_each_component(component, rtd->card) { const struct snd_compr_ops *compr_ops = component->compr_ops;

/* FIXME: 1st compr_ops only at this point */ if (compr_ops && compr_ops->open) { ret = compr_ops->open(cstream); if (ret < 0) { pr_err("compress asoc: can't open component %s\n", component->name); } break; } }

Charles Keepax

11:02 a.m.

New subject: [alsa-devel] [PATCH 5/6] ASoC: add Component level struct snd_compr_ops

On Tue, Dec 20, 2016 at 01:59:43AM +0000, Kuninori Morimoto wrote:

...

...
...
for_each_component(component, rtd->card) {
const struct snd_compr_ops *compr_ops = component->compr_ops;
if (compr_ops && compr_ops->open) {
	int _ret = compr_ops->open(cstream);
	if (_ret < 0) {
		pr_err("compress asoc: can't open component %s\n",
		       component->name);
		ret |= _ret;
	}
Apologies if I am missing something but this really doesn't look like equivalent code? The old code calls the ops for the platform associated with the stream, the new code looks like it will call the ops for every single component in the system that has compressed ops, which would definitely cause issue. Am I missing something here?
Basically in *current* existing code, platform only has compr_ops. So, converting existing code to new code should be OK.

Current ALSA SoC clearly separating CPU/Codec/Platform, but in these days, new sound device has many features. Thus, it becaming difficult to say "it is XXX" today. For example, some Codec has CPU feature, some CPU has Platform feature etc. So, our (= Me/Lars) plan is that remove current CPU/Codec/Platform separation, and all will be Component connection.

Above code will call every component's compr_ops, currently, it should be platform now. But in the future device, Codec and/or CPU can have compr_ops.

If you want, currently, it can call 1st found compr_ops (= should be platform) only with FIXME comment at this point.

Does this clear answer for you ?

for_each_component(component, rtd->card) { const struct snd_compr_ops *compr_ops = component->compr_ops;
/* FIXME: 1st compr_ops only at this point */
if (compr_ops && compr_ops->open) {

But how do you know this is the correct compressed open here? The system could have multiple platforms providing compressed ops and you have just picked the first one in the card list. I think you are making the assumption that there is only one platform providing compressed ops and that seems like a very dangerous assumption to me. Our CODECs provide some compressed features as do many applications processors, which would easily give you at two platforms. But I could even imagine APs registering multiple compressed platforms for different DSPs or some such.

Thanks, Charles

...

	ret = compr_ops->open(cstream);
	if (ret < 0) {
		pr_err("compress asoc: can't open component %s\n",
		       component->name);
	}
	break;
}

}

Kuninori Morimoto

21 Dec 21 Dec

1:42 a.m.

New subject: [alsa-devel] [PATCH 5/6] ASoC: add Component level struct snd_compr_ops

Hi Charles

Thank you for your feedback

...

...
for_each_component(component, rtd->card) { const struct snd_compr_ops *compr_ops = component->compr_ops;
/* FIXME: 1st compr_ops only at this point */
if (compr_ops && compr_ops->open) {
But how do you know this is the correct compressed open here? The system could have multiple platforms providing compressed ops and you have just picked the first one in the card list. I think you are making the assumption that there is only one platform providing compressed ops and that seems like a very dangerous assumption to me. Our CODECs provide some compressed features as do many applications processors, which would easily give you at two platforms. But I could even imagine APs registering multiple compressed platforms for different DSPs or some such.

I agree your opinion, and yes it can be issue in the future. But, this is the reason why it has /* FIXME */ comment here (will has in v2, not yet in v1). I don't know which one should be solved first, but this patch is focusing to part of platform -> component conversion now. No existing code/card get damage at this point, I think ?

So, how about this in v2 ? In comment and git log indicates "It is still assumeing that system has 1 compressed, same as current situation. Multi-compress is TBD."

Best regards --- Kuninori Morimoto

Charles Keepax

10:29 a.m.

New subject: [alsa-devel] [PATCH 5/6] ASoC: add Component level struct snd_compr_ops

On Wed, Dec 21, 2016 at 12:42:21AM +0000, Kuninori Morimoto wrote:

...

Hi Charles

Thank you for your feedback

...
...
for_each_component(component, rtd->card) { const struct snd_compr_ops *compr_ops = component->compr_ops;
/* FIXME: 1st compr_ops only at this point */
if (compr_ops && compr_ops->open) {
But how do you know this is the correct compressed open here? The system could have multiple platforms providing compressed ops and you have just picked the first one in the card list. I think you are making the assumption that there is only one platform providing compressed ops and that seems like a very dangerous assumption to me. Our CODECs provide some compressed features as do many applications processors, which would easily give you at two platforms. But I could even imagine APs registering multiple compressed platforms for different DSPs or some such.
I agree your opinion, and yes it can be issue in the future. But, this is the reason why it has /* FIXME */ comment here (will has in v2, not yet in v1). I don't know which one should be solved first, but this patch is focusing to part of platform -> component conversion now. No existing code/card get damage at this point, I think ?

So, how about this in v2 ? In comment and git log indicates "It is still assumeing that system has 1 compressed, same as current situation. Multi-compress is TBD."

Admittedly there are no machines currently in mainline that do this but it will basically break on every single one of our customer integrations, as most AP systems are supporting some sort of compressed stream stuff now and our CODEC does as well.

I would really rather see this issue fixed before these patches are merged, as if it gets merged I basically have to fix it straight away anyway, or face some very painful integrations in the future. I will try to find some time to look at this and see if I can come up with a better solution for these parts of the code but given the current time frame it is likely to push into the new year.

Thanks, Charles

Kuninori Morimoto

22 Dec 22 Dec

12:45 a.m.

New subject: [alsa-devel] [PATCH 5/6] ASoC: add Component level struct snd_compr_ops

Hi Charles

Thank you for your feedback

...

Admittedly there are no machines currently in mainline that do this but it will basically break on every single one of our customer integrations, as most AP systems are supporting some sort of compressed stream stuff now and our CODEC does as well.

I would really rather see this issue fixed before these patches are merged, as if it gets merged I basically have to fix it straight away anyway, or face some very painful integrations in the future. I will try to find some time to look at this and see if I can come up with a better solution for these parts of the code but given the current time frame it is likely to push into the new year.

OK, Thanks. Let's drop it at this point.

Best regards --- Kuninori Morimoto

Kuninori Morimoto

19 Dec 19 Dec

8:38 a.m.

New subject: [alsa-devel] [PATCH 6/6] ASoC: add Component level struct snd_pcm_ops

From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com

In current ALSA SoC, Platform has struct snd_pcm_ops feature, but it should be supported on Component level. This patch adds it.

If component level has it, many snd_pcm_ops can be called, but, 1st ops function only will be used now. It should/will be fixed in the future.

Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- include/sound/soc.h | 2 + sound/soc/soc-core.c | 2 + sound/soc/soc-pcm.c | 235 ++++++++++++++++++++++++++++++++++++++++----------- 3 files changed, 192 insertions(+), 47 deletions(-)

diff --git a/include/sound/soc.h b/include/sound/soc.h index 225e9b6..f3a3601 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -777,6 +777,7 @@ struct snd_soc_component_driver { const struct snd_soc_dapm_route *dapm_routes; unsigned int num_dapm_routes;

+ const struct snd_pcm_ops *ops; const struct snd_compr_ops *compr_ops;

int (*probe)(struct snd_soc_component *); @@ -857,6 +858,7 @@ struct snd_soc_component { unsigned int num_dapm_routes; struct snd_soc_codec *codec;

+ const struct snd_pcm_ops *ops; const struct snd_compr_ops *compr_ops;

int (*probe)(struct snd_soc_component *); diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index ecadebe..ffa51ab 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -3202,6 +3202,8 @@ int snd_soc_add_platform(struct device *dev, struct snd_soc_platform *platform, platform->component.pcm_free = snd_soc_platform_drv_pcm_free; if (platform_drv->compr_ops) platform->component.compr_ops = platform_drv->compr_ops; + if (platform_drv->ops) + platform->component.ops = platform_drv->ops;

#ifdef CONFIG_DEBUG_FS platform->component.debugfs_prefix = "platform"; diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index ff31bf5..82a3c73 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -450,6 +450,7 @@ static int soc_pcm_open(struct snd_pcm_substream *substream) struct snd_soc_pcm_runtime *rtd = substream->private_data; struct snd_pcm_runtime *runtime = substream->runtime; struct snd_soc_platform *platform = rtd->platform; + struct snd_soc_component *component; struct snd_soc_dai *cpu_dai = rtd->cpu_dai; struct snd_soc_dai *codec_dai; const char *codec_dai_name = "multicodec"; @@ -475,14 +476,22 @@ static int soc_pcm_open(struct snd_pcm_substream *substream) } }

- if (platform->driver->ops && platform->driver->ops->open) { - ret = platform->driver->ops->open(substream); - if (ret < 0) { - dev_err(platform->dev, "ASoC: can't open platform" - " %s: %d\n", platform->component.name, ret); - goto platform_err; + ret = 0; + for_each_component(component, rtd->card) { + const struct snd_pcm_ops *ops = component->ops; + + if (ops && ops->open) { + int _ret = ops->open(substream); + + if (_ret < 0) + dev_err(component->dev, + "ASoC: can't open component %s: %d\n", + component->name, ret); + ret |= _ret; } } + if (ret < 0) + goto component_err;

for (i = 0; i < rtd->num_codecs; i++) { codec_dai = rtd->codec_dais[i]; @@ -590,10 +599,14 @@ static int soc_pcm_open(struct snd_pcm_substream *substream) codec_dai->driver->ops->shutdown(substream, codec_dai); }

- if (platform->driver->ops && platform->driver->ops->close) - platform->driver->ops->close(substream); +component_err: + for_each_component(component, rtd->card) { + const struct snd_pcm_ops *ops = component->ops; + + if (ops && ops->close) + ops->close(substream); + }

-platform_err: if (cpu_dai->driver->ops->shutdown) cpu_dai->driver->ops->shutdown(substream, cpu_dai); out: @@ -655,6 +668,7 @@ static int soc_pcm_close(struct snd_pcm_substream *substream) { struct snd_soc_pcm_runtime *rtd = substream->private_data; struct snd_soc_platform *platform = rtd->platform; + struct snd_soc_component *component; struct snd_soc_dai *cpu_dai = rtd->cpu_dai; struct snd_soc_dai *codec_dai; int i; @@ -687,8 +701,12 @@ static int soc_pcm_close(struct snd_pcm_substream *substream) if (rtd->dai_link->ops && rtd->dai_link->ops->shutdown) rtd->dai_link->ops->shutdown(substream);

- if (platform->driver->ops && platform->driver->ops->close) - platform->driver->ops->close(substream); + for_each_component(component, rtd->card) { + const struct snd_pcm_ops *ops = component->ops; + + if (ops && ops->close) + ops->close(substream); + }

if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) { if (snd_soc_runtime_ignore_pmdown_time(rtd)) { @@ -740,7 +758,7 @@ static int soc_pcm_close(struct snd_pcm_substream *substream) static int soc_pcm_prepare(struct snd_pcm_substream *substream) { struct snd_soc_pcm_runtime *rtd = substream->private_data; - struct snd_soc_platform *platform = rtd->platform; + struct snd_soc_component *component; struct snd_soc_dai *cpu_dai = rtd->cpu_dai; struct snd_soc_dai *codec_dai; int i, ret = 0; @@ -756,12 +774,17 @@ static int soc_pcm_prepare(struct snd_pcm_substream *substream) } }

- if (platform->driver->ops && platform->driver->ops->prepare) { - ret = platform->driver->ops->prepare(substream); - if (ret < 0) { - dev_err(platform->dev, "ASoC: platform prepare error:" - " %d\n", ret); - goto out; + for_each_component(component, rtd->card) { + const struct snd_pcm_ops *ops = component->ops; + + if (ops && ops->prepare) { + ret = ops->prepare(substream); + if (ret < 0) { + dev_err(component->dev, + "ASoC: component prepare error: %d\n", + ret); + goto out; + } } }

@@ -846,7 +869,7 @@ static int soc_pcm_hw_params(struct snd_pcm_substream *substream, struct snd_pcm_hw_params *params) { struct snd_soc_pcm_runtime *rtd = substream->private_data; - struct snd_soc_platform *platform = rtd->platform; + struct snd_soc_component *component; struct snd_soc_dai *cpu_dai = rtd->cpu_dai; int i, ret = 0;

@@ -911,12 +934,16 @@ static int soc_pcm_hw_params(struct snd_pcm_substream *substream, if (ret < 0) goto interface_err;

- if (platform->driver->ops && platform->driver->ops->hw_params) { - ret = platform->driver->ops->hw_params(substream, params); - if (ret < 0) { - dev_err(platform->dev, "ASoC: %s hw params failed: %d\n", - platform->component.name, ret); - goto platform_err; + for_each_component(component, rtd->card) { + const struct snd_pcm_ops *ops = component->ops; + + if (ops && ops->hw_params) { + ret = ops->hw_params(substream, params); + if (ret < 0) { + dev_err(component->dev, "ASoC: %s hw params failed: %d\n", + component->name, ret); + goto component_err; + } } }

@@ -930,7 +957,7 @@ static int soc_pcm_hw_params(struct snd_pcm_substream *substream, mutex_unlock(&rtd->pcm_mutex); return ret;

-platform_err: +component_err: if (cpu_dai->driver->ops && cpu_dai->driver->ops->hw_free) cpu_dai->driver->ops->hw_free(substream, cpu_dai);

@@ -958,7 +985,7 @@ static int soc_pcm_hw_params(struct snd_pcm_substream *substream, static int soc_pcm_hw_free(struct snd_pcm_substream *substream) { struct snd_soc_pcm_runtime *rtd = substream->private_data; - struct snd_soc_platform *platform = rtd->platform; + struct snd_soc_component *component; struct snd_soc_dai *cpu_dai = rtd->cpu_dai; struct snd_soc_dai *codec_dai; bool playback = substream->stream == SNDRV_PCM_STREAM_PLAYBACK; @@ -995,8 +1022,12 @@ static int soc_pcm_hw_free(struct snd_pcm_substream *substream) rtd->dai_link->ops->hw_free(substream);

/* free any DMA resources */ - if (platform->driver->ops && platform->driver->ops->hw_free) - platform->driver->ops->hw_free(substream); + for_each_component(component, rtd->card) { + const struct snd_pcm_ops *ops = component->ops; + + if (ops && ops->hw_free) + ops->hw_free(substream); + }

/* now free hw params for the DAIs */ for (i = 0; i < rtd->num_codecs; i++) { @@ -1015,7 +1046,7 @@ static int soc_pcm_hw_free(struct snd_pcm_substream *substream) static int soc_pcm_trigger(struct snd_pcm_substream *substream, int cmd) { struct snd_soc_pcm_runtime *rtd = substream->private_data; - struct snd_soc_platform *platform = rtd->platform; + struct snd_soc_component *component; struct snd_soc_dai *cpu_dai = rtd->cpu_dai; struct snd_soc_dai *codec_dai; int i, ret; @@ -1030,10 +1061,14 @@ static int soc_pcm_trigger(struct snd_pcm_substream *substream, int cmd) } }

- if (platform->driver->ops && platform->driver->ops->trigger) { - ret = platform->driver->ops->trigger(substream, cmd); - if (ret < 0) - return ret; + for_each_component(component, rtd->card) { + const struct snd_pcm_ops *ops = component->ops; + + if (ops && ops->trigger) { + ret = ops->trigger(substream, cmd); + if (ret < 0) + return ret; + } }

if (cpu_dai->driver->ops && cpu_dai->driver->ops->trigger) { @@ -1085,7 +1120,7 @@ static int soc_pcm_bespoke_trigger(struct snd_pcm_substream *substream, static snd_pcm_uframes_t soc_pcm_pointer(struct snd_pcm_substream *substream) { struct snd_soc_pcm_runtime *rtd = substream->private_data; - struct snd_soc_platform *platform = rtd->platform; + struct snd_soc_component *component; struct snd_soc_dai *cpu_dai = rtd->cpu_dai; struct snd_soc_dai *codec_dai; struct snd_pcm_runtime *runtime = substream->runtime; @@ -1094,8 +1129,15 @@ static snd_pcm_uframes_t soc_pcm_pointer(struct snd_pcm_substream *substream) snd_pcm_sframes_t codec_delay = 0; int i;

- if (platform->driver->ops && platform->driver->ops->pointer) - offset = platform->driver->ops->pointer(substream); + for_each_component(component, rtd->card) { + const struct snd_pcm_ops *ops = component->ops; + + /* FIXME: It uses 1st function only now */ + if (ops && ops->pointer) { + offset = ops->pointer(substream); + break; + } + }

if (cpu_dai->driver->ops && cpu_dai->driver->ops->delay) delay += cpu_dai->driver->ops->delay(substream, cpu_dai); @@ -2278,10 +2320,16 @@ static int soc_pcm_ioctl(struct snd_pcm_substream *substream, unsigned int cmd, void *arg) { struct snd_soc_pcm_runtime *rtd = substream->private_data; - struct snd_soc_platform *platform = rtd->platform; + struct snd_soc_component *component; + + for_each_component(component, rtd->card) { + const struct snd_pcm_ops *ops = component->ops; + + /* FIXME: It uses 1st function only now */ + if (ops && ops->ioctl) + return ops->ioctl(substream, cmd, arg); + }

- if (platform->driver->ops && platform->driver->ops->ioctl) - return platform->driver->ops->ioctl(substream, cmd, arg); return snd_pcm_lib_ioctl(substream, cmd, arg); }

@@ -2637,10 +2685,94 @@ static void soc_pcm_free(struct snd_pcm *pcm) } }

+static int soc_pcm_copy(struct snd_pcm_substream *substream, int channel, + snd_pcm_uframes_t pos, + void __user *buf, snd_pcm_uframes_t count) +{ + struct snd_soc_pcm_runtime *rtd = substream->private_data; + struct snd_soc_component *component; + + for_each_component(component, rtd->card) { + const struct snd_pcm_ops *ops = component->ops; + + /* FIXME: It uses 1st function only now */ + if (ops && ops->copy) + return ops->copy(substream, channel, pos, buf, count); + } + + return -EIO; +} + +static int soc_pcm_silence(struct snd_pcm_substream *substream, int channel, + snd_pcm_uframes_t pos, snd_pcm_uframes_t count) +{ + struct snd_soc_pcm_runtime *rtd = substream->private_data; + struct snd_soc_component *component; + + for_each_component(component, rtd->card) { + const struct snd_pcm_ops *ops = component->ops; + + /* FIXME: It uses 1st function only now */ + if (ops && ops->silence) + return ops->silence(substream, channel, pos, count); + } + + return -EIO; +} + +static struct page *soc_pcm_page(struct snd_pcm_substream *substream, + unsigned long offset) +{ + struct snd_soc_pcm_runtime *rtd = substream->private_data; + struct snd_soc_component *component; + + for_each_component(component, rtd->card) { + const struct snd_pcm_ops *ops = component->ops; + + /* FIXME: It uses 1st function only now */ + if (ops && ops->page) + return ops->page(substream, offset); + } + + return ERR_PTR(-EIO); +} + +static int soc_pcm_mmap(struct snd_pcm_substream *substream, + struct vm_area_struct *vma) +{ + struct snd_soc_pcm_runtime *rtd = substream->private_data; + struct snd_soc_component *component; + + for_each_component(component, rtd->card) { + const struct snd_pcm_ops *ops = component->ops; + + /* FIXME: It uses 1st function only now */ + if (ops && ops->mmap) + return ops->mmap(substream, vma); + } + + return -EIO; +} + +static int soc_pcm_ack(struct snd_pcm_substream *substream) +{ + struct snd_soc_pcm_runtime *rtd = substream->private_data; + struct snd_soc_component *component; + + for_each_component(component, rtd->card) { + const struct snd_pcm_ops *ops = component->ops; + + /* FIXME: It uses 1st function only now */ + if (ops && ops->ack) + return ops->ack(substream); + } + + return -EIO; +} + /* create a new pcm */ int soc_new_pcm(struct snd_soc_pcm_runtime *rtd, int num) { - struct snd_soc_platform *platform = rtd->platform; struct snd_soc_dai *codec_dai; struct snd_soc_dai *cpu_dai = rtd->cpu_dai; struct snd_soc_component *component; @@ -2738,12 +2870,21 @@ int soc_new_pcm(struct snd_soc_pcm_runtime *rtd, int num) rtd->ops.ioctl = soc_pcm_ioctl; }

- if (platform->driver->ops) { - rtd->ops.ack = platform->driver->ops->ack; - rtd->ops.copy = platform->driver->ops->copy; - rtd->ops.silence = platform->driver->ops->silence; - rtd->ops.page = platform->driver->ops->page; - rtd->ops.mmap = platform->driver->ops->mmap; + for_each_component(component, rtd->card) { + const struct snd_pcm_ops *ops = component->ops; + + if (ops) { + if (ops->ack) + rtd->ops.ack = soc_pcm_ack; + if (ops->copy) + rtd->ops.copy = soc_pcm_copy; + if (ops->silence) + rtd->ops.silence = soc_pcm_silence; + if (ops->page) + rtd->ops.page = soc_pcm_page; + if (ops->mmap) + rtd->ops.mmap = soc_pcm_mmap; + } }

if (playback)

-- 1.9.1

2902

Age (days ago)

2925

Last active (days ago)

List overview

Download

18 comments

3 participants

tags (0)

participants (3)

Charles Keepax
Kuninori Morimoto
Mark Brown