The patch

ASoC: soc-core: set component->debugfs_root NULL

has been applied to the asoc tree at

https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.4

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 ad64bfbd09d7b7997a1b1510983350836bcd6ed9 Mon Sep 17 00:00:00 2001

From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Date: Wed, 7 Aug 2019 10:30:13 +0900 Subject: [PATCH] ASoC: soc-core: set component->debugfs_root NULL

To be more safety code, let's set NULL to component->debugfs_root when it was cleanuped.

Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Link: https://lore.kernel.org/r/87muglahq0.wl-kuninori.morimoto.gx@renesas.com Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/soc-core.c | 3 +++ 1 file changed, 3 insertions(+)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 0f75dac4bb26..2f37c4e0974b 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -171,7 +171,10 @@ static void soc_init_component_debugfs(struct snd_soc_component *component)

static void soc_cleanup_component_debugfs(struct snd_soc_component *component) { + if (!component->debugfs_root) + return; debugfs_remove_recursive(component->debugfs_root); + component->debugfs_root = NULL; }

static int dai_list_show(struct seq_file *m, void *v)

The patch

ASoC: soc-core: add comment for for_each_xxx

has been applied to the asoc tree at

https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.4

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 6634e3d6ea8cd92fce10d1d2da0c8e42ab84da32 Mon Sep 17 00:00:00 2001

From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Date: Wed, 7 Aug 2019 10:30:31 +0900 Subject: [PATCH] ASoC: soc-core: add comment for for_each_xxx

soc-core has many for_each_xxx, but it is a little bit difficult to know which list is relead to which for_each_xxx. This patch adds missing comment for it.

Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Link: https://lore.kernel.org/r/87lfw5ahpj.wl-kuninori.morimoto.gx@renesas.com Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/soc-core.c | 2 ++ 1 file changed, 2 insertions(+)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 2f37c4e0974b..5e6a48a0f79e 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -380,6 +380,7 @@ static void soc_free_pcm_runtime(struct snd_soc_pcm_runtime *rtd) static void soc_add_pcm_runtime(struct snd_soc_card *card, struct snd_soc_pcm_runtime *rtd) { + /* see for_each_card_rtds */ list_add_tail(&rtd->list, &card->rtd_list); rtd->num = card->num_rtd; card->num_rtd++; @@ -1149,6 +1150,7 @@ int snd_soc_add_dai_link(struct snd_soc_card *card, if (dai_link->dobj.type && card->add_dai_link) card->add_dai_link(card, dai_link);

+ /* see for_each_card_links */ list_add_tail(&dai_link->list, &card->dai_link_list);

return 0;

The patch

ASoC: soc-core: check return value of snd_soc_add_dai_link()

has been applied to the asoc tree at

https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.4

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 5b99a0aad08a3428cc3262ecee29a71c88c981c2 Mon Sep 17 00:00:00 2001

From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Date: Wed, 7 Aug 2019 10:30:36 +0900 Subject: [PATCH] ASoC: soc-core: check return value of snd_soc_add_dai_link()

snd_soc_add_dai_link() might return error, we need to check it.

Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Link: https://lore.kernel.org/r/87k1bpahpd.wl-kuninori.morimoto.gx@renesas.com Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/soc-core.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 5e6a48a0f79e..7345679d4903 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -1964,8 +1964,11 @@ static int snd_soc_instantiate_card(struct snd_soc_card *card) }

/* add predefined DAI links to the list */ - for_each_card_prelinks(card, i, dai_link) - snd_soc_add_dai_link(card, dai_link); + for_each_card_prelinks(card, i, dai_link) { + ret = snd_soc_add_dai_link(card, dai_link); + if (ret < 0) + goto probe_end; + }

/* card bind complete so register a sound card */ ret = snd_card_new(card->dev, SNDRV_DEFAULT_IDX1, SNDRV_DEFAULT_STR1,

The patch

ASoC: soc-core: don't use for_each_card_links_safe() at snd_soc_find_dai_link()

has been applied to the asoc tree at

https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.4

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 42849064500b24892e2abfe8679507dfffd585f1 Mon Sep 17 00:00:00 2001

From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Date: Wed, 7 Aug 2019 10:30:41 +0900 Subject: [PATCH] ASoC: soc-core: don't use for_each_card_links_safe() at snd_soc_find_dai_link()

It doesn't removes list during loop at snd_soc_find_dai_link(). We don't need to use _safe loop.

Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Link: https://lore.kernel.org/r/87imr9ahp9.wl-kuninori.morimoto.gx@renesas.com Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/soc-core.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 7345679d4903..e176b972e4e6 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -817,11 +817,11 @@ struct snd_soc_dai_link *snd_soc_find_dai_link(struct snd_soc_card *card, int id, const char *name, const char *stream_name) { - struct snd_soc_dai_link *link, *_link; + struct snd_soc_dai_link *link;

lockdep_assert_held(&client_mutex);

- for_each_card_links_safe(card, link, _link) { + for_each_card_links(card, link) { if (link->id != id) continue;

The patch

ASoC: soc-core: reuse rtdcom at snd_soc_rtdcom_add()

has been applied to the asoc tree at

https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.4

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 32d2c172fe88705058310635a732259c0a98e89b Mon Sep 17 00:00:00 2001

From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Date: Wed, 7 Aug 2019 10:30:47 +0900 Subject: [PATCH] ASoC: soc-core: reuse rtdcom at snd_soc_rtdcom_add()

snd_soc_rtdcom_add() is using both "rtdcom" and "new_rtdcom" as variable name, but these are not used at same time. Let's reuse rtdcom.

Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Link: https://lore.kernel.org/r/87h86tahp2.wl-kuninori.morimoto.gx@renesas.com Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/soc-core.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index e176b972e4e6..d12054ab8741 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -274,7 +274,6 @@ static int snd_soc_rtdcom_add(struct snd_soc_pcm_runtime *rtd, struct snd_soc_component *component) { struct snd_soc_rtdcom_list *rtdcom; - struct snd_soc_rtdcom_list *new_rtdcom;

for_each_rtdcom(rtd, rtdcom) { /* already connected */ @@ -282,14 +281,14 @@ static int snd_soc_rtdcom_add(struct snd_soc_pcm_runtime *rtd, return 0; }

- new_rtdcom = kmalloc(sizeof(*new_rtdcom), GFP_KERNEL); - if (!new_rtdcom) + rtdcom = kmalloc(sizeof(*rtdcom), GFP_KERNEL); + if (!rtdcom) return -ENOMEM;

- new_rtdcom->component = component; - INIT_LIST_HEAD(&new_rtdcom->list); + rtdcom->component = component; + INIT_LIST_HEAD(&rtdcom->list);

- list_add_tail(&new_rtdcom->list, &rtd->component_list); + list_add_tail(&rtdcom->list, &rtd->component_list);

return 0; }

The patch

ASoC: soc-core: tidyup for snd_soc_dapm_new_controls()

has been applied to the asoc tree at

https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.4

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 b8ba3b572c70c8559ef76c1e0ca02e7b05126e8c Mon Sep 17 00:00:00 2001

From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Date: Wed, 7 Aug 2019 10:30:53 +0900 Subject: [PATCH] ASoC: soc-core: tidyup for snd_soc_dapm_new_controls()

snd_soc_dapm_new_controls() registers controls by using for(... i < num; ...). It means if widget was NULL, num should be zero. Thus, we don't need to check about widget pointer. This patch also cares missing return value.

Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Link: https://lore.kernel.org/r/87ftmdahow.wl-kuninori.morimoto.gx@renesas.com Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/soc-core.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index d12054ab8741..bb1e9e2c4ff4 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -1264,16 +1264,14 @@ static int soc_probe_component(struct snd_soc_card *card,

soc_init_component_debugfs(component);

- if (component->driver->dapm_widgets) { - ret = snd_soc_dapm_new_controls(dapm, + ret = snd_soc_dapm_new_controls(dapm, component->driver->dapm_widgets, component->driver->num_dapm_widgets);

- if (ret != 0) { - dev_err(component->dev, - "Failed to create new controls %d\n", ret); - goto err_probe; - } + if (ret != 0) { + dev_err(component->dev, + "Failed to create new controls %d\n", ret); + goto err_probe; }

for_each_component_dais(component, dai) { @@ -1990,13 +1988,15 @@ static int snd_soc_instantiate_card(struct snd_soc_card *card) INIT_WORK(&card->deferred_resume_work, soc_resume_deferred); #endif

- if (card->dapm_widgets) - snd_soc_dapm_new_controls(&card->dapm, card->dapm_widgets, - card->num_dapm_widgets); + ret = snd_soc_dapm_new_controls(&card->dapm, card->dapm_widgets, + card->num_dapm_widgets); + if (ret < 0) + goto probe_end;

- if (card->of_dapm_widgets) - snd_soc_dapm_new_controls(&card->dapm, card->of_dapm_widgets, - card->num_of_dapm_widgets); + ret = snd_soc_dapm_new_controls(&card->dapm, card->of_dapm_widgets, + card->num_of_dapm_widgets); + if (ret < 0) + goto probe_end;

/* initialise the sound card only once */ if (card->probe) {

The patch

ASoC: soc-core: tidyup for snd_soc_add_component_controls()

has been applied to the asoc tree at

https://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 e6d7020c2946bef2efab7c70339eee6a6b3cb6a6 Mon Sep 17 00:00:00 2001

From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Date: Wed, 7 Aug 2019 10:30:58 +0900 Subject: [PATCH] ASoC: soc-core: tidyup for snd_soc_add_component_controls()

snd_soc_add_component_controls() registers controls by using for(... i < num; ...). If controls was NULL, num should be zero. Thus, we don't need to check about controls pointer. This patch also cares missing return value.

Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Link: https://lore.kernel.org/r/87ef1xahor.wl-kuninori.morimoto.gx@renesas.com Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/soc-core.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index bb1e9e2c4ff4..e481f9999bfb 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -1304,10 +1304,12 @@ static int soc_probe_component(struct snd_soc_card *card, } }

- if (component->driver->controls) - snd_soc_add_component_controls(component, - component->driver->controls, - component->driver->num_controls); + ret = snd_soc_add_component_controls(component, + component->driver->controls, + component->driver->num_controls); + if (ret < 0) + goto err_probe; + if (component->driver->dapm_routes) snd_soc_dapm_add_routes(dapm, component->driver->dapm_routes,

The patch

ASoC: soc-core: tidyup for snd_soc_dapm_add_routes()

has been applied to the asoc tree at

https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.4

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 daa480bde6b3a9b728965e52efffc329ccee3f77 Mon Sep 17 00:00:00 2001

From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Date: Wed, 7 Aug 2019 10:31:03 +0900 Subject: [PATCH] ASoC: soc-core: tidyup for snd_soc_dapm_add_routes()

snd_soc_dapm_add_routes() registers routes by using for(... i < num; ...). If routes was NULL, num should be zero. Thus, we don't need to check about route pointer. This patch also cares missing return value.

Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Link: https://lore.kernel.org/r/87d0hhahon.wl-kuninori.morimoto.gx@renesas.com Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/soc-core.c | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index e481f9999bfb..de95b68ce9ee 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -1310,10 +1310,11 @@ static int soc_probe_component(struct snd_soc_card *card, if (ret < 0) goto err_probe;

- if (component->driver->dapm_routes) - snd_soc_dapm_add_routes(dapm, - component->driver->dapm_routes, - component->driver->num_dapm_routes); + ret = snd_soc_dapm_add_routes(dapm, + component->driver->dapm_routes, + component->driver->num_dapm_routes); + if (ret < 0) + goto err_probe;

list_add(&dapm->list, &card->dapm_list); /* see for_each_card_components */ @@ -2061,13 +2062,15 @@ static int snd_soc_instantiate_card(struct snd_soc_card *card) snd_soc_add_card_controls(card, card->controls, card->num_controls);

- if (card->dapm_routes) - snd_soc_dapm_add_routes(&card->dapm, card->dapm_routes, - card->num_dapm_routes); + ret = snd_soc_dapm_add_routes(&card->dapm, card->dapm_routes, + card->num_dapm_routes); + if (ret < 0) + goto probe_end;

- if (card->of_dapm_routes) - snd_soc_dapm_add_routes(&card->dapm, card->of_dapm_routes, - card->num_of_dapm_routes); + ret = snd_soc_dapm_add_routes(&card->dapm, card->of_dapm_routes, + card->num_of_dapm_routes); + if (ret < 0) + goto probe_end;

/* try to set some sane longname if DMI is available */ snd_soc_set_dmi_name(card, NULL);

On 8/20/19 6:18 AM, Cezary Rojewski wrote:

On 2019-08-07 03:31, Kuninori Morimoto wrote:

...

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

snd_soc_dapm_add_routes() registers routes by using for(... i < num; ...). If routes was NULL, num should be zero. Thus, we don't need to check about route pointer. This patch also cares missing return value.

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

v1 -> v2

- check return value     - change Subject

sound/soc/soc-core.c | 23 +++++++++++++----------   1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 21cdd3c..ca1b04c 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -1310,10 +1310,11 @@ static int soc_probe_component(struct snd_soc_card *card,       if (ret < 0)           goto err_probe; -    if (component->driver->dapm_routes) -        snd_soc_dapm_add_routes(dapm, -                    component->driver->dapm_routes, -                    component->driver->num_dapm_routes); +    ret = snd_soc_dapm_add_routes(dapm, +                      component->driver->dapm_routes, +                      component->driver->num_dapm_routes); +    if (ret < 0) +        goto err_probe;       list_add(&dapm->list, &card->dapm_list);       /* see for_each_card_components */ @@ -2060,13 +2061,15 @@ static int snd_soc_instantiate_card(struct snd_soc_card *card)           snd_soc_add_card_controls(card, card->controls,                         card->num_controls); -    if (card->dapm_routes) -        snd_soc_dapm_add_routes(&card->dapm, card->dapm_routes, -                    card->num_dapm_routes); +    ret = snd_soc_dapm_add_routes(&card->dapm, card->dapm_routes, +                      card->num_dapm_routes); +    if (ret < 0) +        goto probe_end; -    if (card->of_dapm_routes) -        snd_soc_dapm_add_routes(&card->dapm, card->of_dapm_routes, -                    card->num_of_dapm_routes); +    ret = snd_soc_dapm_add_routes(&card->dapm, card->of_dapm_routes, +                      card->num_of_dapm_routes); +    if (ret < 0) +        goto probe_end;       /* try to set some sane longname if DMI is available */       snd_soc_set_dmi_name(card, NULL);

Hello there,

I've run a validation cycle on recent broonie/for-next and this commit caused regression. However, it may be simply an error on board side instead.

Previously, ret from snd_soc_dapm_add_routes has been ignored thus it was permissive for addition of several routes to fail. As long as some routes succeeded, card was working just fine. Now it's no longer the case - behavior of the card initialization has changed: it is required for ALL routes to succeed before card can be fully instantiated.

Must say collapsing snd_soc_instantiate_card is a wonderful way to test your card's removal flow (soc__cleanup_card_resources and friends)..

Question is simple: are we staying with all-for-one/ one-for-all approach or we reverting to permissive behavior?

Can you elaborate in which test case this patch creates a problem? Just curious why the route addition fails in the first place.

On 8/20/19 8:38 AM, Cezary Rojewski wrote:

On 2019-08-20 14:36, Pierre-Louis Bossart wrote:

...

On 8/20/19 6:18 AM, Cezary Rojewski wrote:

...

On 2019-08-07 03:31, Kuninori Morimoto wrote:

...

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

snd_soc_dapm_add_routes() registers routes by using for(... i < num; ...). If routes was NULL, num should be zero. Thus, we don't need to check about route pointer. This patch also cares missing return value.

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

v1 -> v2

- check return value     - change Subject

sound/soc/soc-core.c | 23 +++++++++++++----------   1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 21cdd3c..ca1b04c 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -1310,10 +1310,11 @@ static int soc_probe_component(struct snd_soc_card *card,       if (ret < 0)           goto err_probe; -    if (component->driver->dapm_routes) -        snd_soc_dapm_add_routes(dapm, -                    component->driver->dapm_routes, -                    component->driver->num_dapm_routes); +    ret = snd_soc_dapm_add_routes(dapm, +                      component->driver->dapm_routes, +                      component->driver->num_dapm_routes); +    if (ret < 0) +        goto err_probe;       list_add(&dapm->list, &card->dapm_list);       /* see for_each_card_components */ @@ -2060,13 +2061,15 @@ static int snd_soc_instantiate_card(struct snd_soc_card *card)           snd_soc_add_card_controls(card, card->controls,                         card->num_controls); -    if (card->dapm_routes) -        snd_soc_dapm_add_routes(&card->dapm, card->dapm_routes, -                    card->num_dapm_routes); +    ret = snd_soc_dapm_add_routes(&card->dapm, card->dapm_routes, +                      card->num_dapm_routes); +    if (ret < 0) +        goto probe_end; -    if (card->of_dapm_routes) -        snd_soc_dapm_add_routes(&card->dapm, card->of_dapm_routes, -                    card->num_of_dapm_routes); +    ret = snd_soc_dapm_add_routes(&card->dapm, card->of_dapm_routes, +                      card->num_of_dapm_routes); +    if (ret < 0) +        goto probe_end;       /* try to set some sane longname if DMI is available */       snd_soc_set_dmi_name(card, NULL);

Hello there,

I've run a validation cycle on recent broonie/for-next and this commit caused regression. However, it may be simply an error on board side instead.

Previously, ret from snd_soc_dapm_add_routes has been ignored thus it was permissive for addition of several routes to fail. As long as some routes succeeded, card was working just fine. Now it's no longer the case - behavior of the card initialization has changed: it is required for ALL routes to succeed before card can be fully instantiated.

Must say collapsing snd_soc_instantiate_card is a wonderful way to test your card's removal flow (soc__cleanup_card_resources and friends)..

Question is simple: are we staying with all-for-one/ one-for-all approach or we reverting to permissive behavior?

Can you elaborate in which test case this patch creates a problem? Just curious why the route addition fails in the first place.

If snd_soc_instantiate_card fails so does any test, really. Red wall was easy to spot even for a hungry developer : )

Our cnl_rt274 board declares several routes, yet our topology does not provide necessary info for all of them. And thus, addition of some routes fails. This was fine till now. That's also why I'd mentioned in the very first sentence: it might be simply a board issue. Maybe we should have never abused permissive behavior in the first place.

Yep, and that driver is not upstream as well so Intel can't complain here...

On 2019-08-07 03:31, Kuninori Morimoto wrote:

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

snd_soc_component_set_jack() is used for both setting/clearing. Setting purpose is used under each driver. Hence, clearing purpose should be used under each driver, not soc-core.

soc-core shouldn't touch it even though its purpose was for clearing, otherwise, code becomes very confusable. This patch removes snd_soc_component_set_jack() from soc-core.c

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

v1 -> v2

• no change

sound/soc/soc-core.c | 1 - 1 file changed, 1 deletion(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 80703618..e708095 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -938,7 +938,6 @@ static int soc_bind_dai_link(struct snd_soc_card *card,

static void soc_cleanup_component(struct snd_soc_component *component) {

• snd_soc_component_set_jack(component, NULL, NULL); list_del(&component->card_list); snd_soc_dapm_free(snd_soc_component_get_dapm(component)); soc_cleanup_component_debugfs(component);

This has been added lately for a reason - reload/ unload series. Amadeusz, could you comment on this change?

Hi

...

...

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

snd_soc_component_set_jack() is used for both setting/clearing. Setting purpose is used under each driver. Hence, clearing purpose should be used under each driver, not soc-core.

soc-core shouldn't touch it even though its purpose was for clearing, otherwise, code becomes very confusable. This patch removes snd_soc_component_set_jack() from soc-core.c

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

v1 -> v2

• no change

sound/soc/soc-core.c | 1 - 1 file changed, 1 deletion(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 80703618..e708095 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -938,7 +938,6 @@ static int soc_bind_dai_link(struct snd_soc_card *card, static void soc_cleanup_component(struct snd_soc_component *component) {

• snd_soc_component_set_jack(component, NULL, NULL); list_del(&component->card_list); snd_soc_dapm_free(snd_soc_component_get_dapm(component)); soc_cleanup_component_debugfs(component);

This has been added lately for a reason - reload/ unload series. Amadeusz, could you comment on this change?

This was done on assumption that we want to always make sure that it is cleaned up, independent of if driver author accidentally forgets to do this.

We can of course add handler to our driver to do this, first version of patch actually did this, before we decided on global option.

I double-checked framework. *All* drivers which are using snd_soc_component_set_jack() doesn't care reset jack. I think it should be done under driver, not framework, but this patch seems have big effect. OK, let's skip it.

Thank you for your help !! Best regards --- Kuninori Morimoto

On Tue, Aug 6, 2019 at 6:48 PM Kuninori Morimoto < kuninori.morimoto.gx@renesas.com> wrote:

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

commit 52abe6cc1866a ("ASoC: topology: fix oops/use-after-free case with dai driver") fixups remove_dai() error, but it is using list_for_each_entry() for component->dai_list.

We already have for_each_component_dais() macro for it. Let's use exising method.

The series looks good now. Thanks for the quick turnaround, Morimoto-san.

Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com

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

v1 -> v2

    - no change

sound/soc/soc-topology.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c index dc463f1..b869071 100644 --- a/sound/soc/soc-topology.c +++ b/sound/soc/soc-topology.c @@ -530,7 +530,7 @@ static void remove_dai(struct snd_soc_component *comp, if (dobj->ops && dobj->ops->dai_unload) dobj->ops->dai_unload(comp, dobj);

•   list_for_each_entry(dai, &comp->dai_list, list)
  

•   for_each_component_dais(comp, dai)
            if (dai->driver == dai_drv)
                    dai->driver = NULL;
  
  

-- 2.7.4

---

Alsa-devel mailing list Alsa-devel@alsa-project.org https://mailman.alsa-project.org/mailman/listinfo/alsa-devel