alsa-devel.alsa-project.org archive mirror
 help / color / mirror / Atom feed
* [alsa-devel] [PATCH v3 00/19] ASoC: soc-core cleanup - step 4
@ 2019-11-05  6:45 Kuninori Morimoto
  2019-11-05  6:45 ` [alsa-devel] [PATCH v3 01/19] ASoC: soc-core: move soc_init_dai_link() Kuninori Morimoto
                   ` (19 more replies)
  0 siblings, 20 replies; 60+ messages in thread
From: Kuninori Morimoto @ 2019-11-05  6:45 UTC (permalink / raw)
  To: Mark Brown; +Cc: Linux-ALSA, Pierre-Louis Bossart, Ranjani Sridharan


Hi Mark

These are v3 of soc-core cleanup step4.
These are based on mark/for-5.5 branch.
These had been tested on Intel CI, and had no issue.
And are reviewed by Pierre-Louis, Ranjani.

Kuninori Morimoto (19):
  ASoC: soc-core: move soc_init_dai_link()
  ASoC: soc-core: tidyup soc_init_dai_link()
  ASoC: soc-core: typo fix at soc_dai_link_sanity_check()
  ASoC: soc-core: remove duplicated soc_is_dai_link_bound()
  ASoC: soc-core: call soc_bind_dai_link() under snd_soc_add_dai_link()
  ASoC: soc-core: add soc_unbind_dai_link()
  ASoC: soc-core: move snd_soc_lookup_component()
  ASoC: soc-core: tidyup snd_soc_lookup_component()
  ASoC: soc-core: add snd_soc_del_component_unlocked()
  ASoC: soc-core: remove snd_soc_component_add/del()
  ASoC: soc-core: use snd_soc_lookup_component() at snd_soc_unregister_component()
  ASoC: soc-core: move snd_soc_register_dai()
  ASoC: soc-core: move snd_soc_unregister_dais()
  ASoC: soc-core: add snd_soc_unregister_dai()
  ASoC: soc-core: have legacy_dai_naming at snd_soc_register_dai()
  ASoC: soc-core: don't call snd_soc_dapm_new_dai_widgets() at snd_soc_register_dai()
  ASoC: soc-core: call snd_soc_register_dai() from snd_soc_register_dais()
  ASoC: soc-core: remove topology specific operation
  ASoC: soc.h: dobj is used only when SND_SOC_TOPOLOGY

 include/sound/soc.h      |  15 +-
 sound/soc/soc-core.c     | 526 ++++++++++++++++++++---------------------------
 sound/soc/soc-topology.c |  17 +-
 3 files changed, 255 insertions(+), 303 deletions(-)

-- 
2.7.4

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

^ permalink raw reply	[flat|nested] 60+ messages in thread

* [alsa-devel] [PATCH v3 01/19] ASoC: soc-core: move soc_init_dai_link()
  2019-11-05  6:45 [alsa-devel] [PATCH v3 00/19] ASoC: soc-core cleanup - step 4 Kuninori Morimoto
@ 2019-11-05  6:45 ` Kuninori Morimoto
  2019-11-05 23:51   ` [alsa-devel] Applied "ASoC: soc-core: move soc_init_dai_link()" to the asoc tree Mark Brown
  2019-11-05  6:45 ` [alsa-devel] [PATCH v3 02/19] ASoC: soc-core: tidyup soc_init_dai_link() Kuninori Morimoto
                   ` (18 subsequent siblings)
  19 siblings, 1 reply; 60+ messages in thread
From: Kuninori Morimoto @ 2019-11-05  6:45 UTC (permalink / raw)
  To: Mark Brown; +Cc: Linux-ALSA, Pierre-Louis Bossart, Ranjani Sridharan


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

This patch moves soc_init_dai_link() next to soc_bind_dai_link().
This is prepare for soc_bind_dai_link() cleanup.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
v2 -> v3
	- no change
	- add Reviewed-by

 sound/soc/soc-core.c | 192 +++++++++++++++++++++++++--------------------------
 1 file changed, 96 insertions(+), 96 deletions(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index b07ecfa..a141828 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -941,6 +941,102 @@ static bool soc_is_dai_link_bound(struct snd_soc_card *card,
 	return false;
 }
 
+static int soc_init_dai_link(struct snd_soc_card *card,
+			     struct snd_soc_dai_link *link)
+{
+	int i;
+	struct snd_soc_dai_link_component *codec, *platform;
+
+	for_each_link_codecs(link, i, codec) {
+		/*
+		 * Codec must be specified by 1 of name or OF node,
+		 * not both or neither.
+		 */
+		if (!!codec->name == !!codec->of_node) {
+			dev_err(card->dev, "ASoC: Neither/both codec name/of_node are set for %s\n",
+				link->name);
+			return -EINVAL;
+		}
+
+		/* Codec DAI name must be specified */
+		if (!codec->dai_name) {
+			dev_err(card->dev, "ASoC: codec_dai_name not set for %s\n",
+				link->name);
+			return -EINVAL;
+		}
+
+		/*
+		 * Defer card registration if codec component is not added to
+		 * component list.
+		 */
+		if (!soc_find_component(codec))
+			return -EPROBE_DEFER;
+	}
+
+	for_each_link_platforms(link, i, platform) {
+		/*
+		 * Platform may be specified by either name or OF node, but it
+		 * can be left unspecified, then no components will be inserted
+		 * in the rtdcom list
+		 */
+		if (!!platform->name == !!platform->of_node) {
+			dev_err(card->dev,
+				"ASoC: Neither/both platform name/of_node are set for %s\n",
+				link->name);
+			return -EINVAL;
+		}
+
+		/*
+		 * Defer card registration if platform component is not added to
+		 * component list.
+		 */
+		if (!soc_find_component(platform))
+			return -EPROBE_DEFER;
+	}
+
+	/* FIXME */
+	if (link->num_cpus > 1) {
+		dev_err(card->dev,
+			"ASoC: multi cpu is not yet supported %s\n",
+			link->name);
+		return -EINVAL;
+	}
+
+	/*
+	 * CPU device may be specified by either name or OF node, but
+	 * can be left unspecified, and will be matched based on DAI
+	 * name alone..
+	 */
+	if (link->cpus->name && link->cpus->of_node) {
+		dev_err(card->dev,
+			"ASoC: Neither/both cpu name/of_node are set for %s\n",
+			link->name);
+		return -EINVAL;
+	}
+
+	/*
+	 * Defer card registartion if cpu dai component is not added to
+	 * component list.
+	 */
+	if ((link->cpus->of_node || link->cpus->name) &&
+	    !soc_find_component(link->cpus))
+		return -EPROBE_DEFER;
+
+	/*
+	 * At least one of CPU DAI name or CPU device name/node must be
+	 * specified
+	 */
+	if (!link->cpus->dai_name &&
+	    !(link->cpus->name || link->cpus->of_node)) {
+		dev_err(card->dev,
+			"ASoC: Neither cpu_dai_name nor cpu_name/of_node are set for %s\n",
+			link->name);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static int soc_bind_dai_link(struct snd_soc_card *card,
 	struct snd_soc_dai_link *dai_link)
 {
@@ -1283,102 +1379,6 @@ static int soc_probe_link_components(struct snd_soc_card *card)
 	return 0;
 }
 
-static int soc_init_dai_link(struct snd_soc_card *card,
-			     struct snd_soc_dai_link *link)
-{
-	int i;
-	struct snd_soc_dai_link_component *codec, *platform;
-
-	for_each_link_codecs(link, i, codec) {
-		/*
-		 * Codec must be specified by 1 of name or OF node,
-		 * not both or neither.
-		 */
-		if (!!codec->name == !!codec->of_node) {
-			dev_err(card->dev, "ASoC: Neither/both codec name/of_node are set for %s\n",
-				link->name);
-			return -EINVAL;
-		}
-
-		/* Codec DAI name must be specified */
-		if (!codec->dai_name) {
-			dev_err(card->dev, "ASoC: codec_dai_name not set for %s\n",
-				link->name);
-			return -EINVAL;
-		}
-
-		/*
-		 * Defer card registration if codec component is not added to
-		 * component list.
-		 */
-		if (!soc_find_component(codec))
-			return -EPROBE_DEFER;
-	}
-
-	for_each_link_platforms(link, i, platform) {
-		/*
-		 * Platform may be specified by either name or OF node, but it
-		 * can be left unspecified, then no components will be inserted
-		 * in the rtdcom list
-		 */
-		if (!!platform->name == !!platform->of_node) {
-			dev_err(card->dev,
-				"ASoC: Neither/both platform name/of_node are set for %s\n",
-				link->name);
-			return -EINVAL;
-		}
-
-		/*
-		 * Defer card registration if platform component is not added to
-		 * component list.
-		 */
-		if (!soc_find_component(platform))
-			return -EPROBE_DEFER;
-	}
-
-	/* FIXME */
-	if (link->num_cpus > 1) {
-		dev_err(card->dev,
-			"ASoC: multi cpu is not yet supported %s\n",
-			link->name);
-		return -EINVAL;
-	}
-
-	/*
-	 * CPU device may be specified by either name or OF node, but
-	 * can be left unspecified, and will be matched based on DAI
-	 * name alone..
-	 */
-	if (link->cpus->name && link->cpus->of_node) {
-		dev_err(card->dev,
-			"ASoC: Neither/both cpu name/of_node are set for %s\n",
-			link->name);
-		return -EINVAL;
-	}
-
-	/*
-	 * Defer card registartion if cpu dai component is not added to
-	 * component list.
-	 */
-	if ((link->cpus->of_node || link->cpus->name) &&
-	    !soc_find_component(link->cpus))
-		return -EPROBE_DEFER;
-
-	/*
-	 * At least one of CPU DAI name or CPU device name/node must be
-	 * specified
-	 */
-	if (!link->cpus->dai_name &&
-	    !(link->cpus->name || link->cpus->of_node)) {
-		dev_err(card->dev,
-			"ASoC: Neither cpu_dai_name nor cpu_name/of_node are set for %s\n",
-			link->name);
-		return -EINVAL;
-	}
-
-	return 0;
-}
-
 void snd_soc_disconnect_sync(struct device *dev)
 {
 	struct snd_soc_component *component =
-- 
2.7.4

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

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [alsa-devel] [PATCH v3 02/19] ASoC: soc-core: tidyup soc_init_dai_link()
  2019-11-05  6:45 [alsa-devel] [PATCH v3 00/19] ASoC: soc-core cleanup - step 4 Kuninori Morimoto
  2019-11-05  6:45 ` [alsa-devel] [PATCH v3 01/19] ASoC: soc-core: move soc_init_dai_link() Kuninori Morimoto
@ 2019-11-05  6:45 ` Kuninori Morimoto
  2019-11-05 23:51   ` [alsa-devel] Applied "ASoC: soc-core: tidyup soc_init_dai_link()" to the asoc tree Mark Brown
  2019-11-11 14:43   ` [alsa-devel] [PATCH v3 02/19] ASoC: soc-core: tidyup soc_init_dai_link() Jon Hunter
  2019-11-05  6:46 ` [alsa-devel] [PATCH v3 03/19] ASoC: soc-core: typo fix at soc_dai_link_sanity_check() Kuninori Morimoto
                   ` (17 subsequent siblings)
  19 siblings, 2 replies; 60+ messages in thread
From: Kuninori Morimoto @ 2019-11-05  6:45 UTC (permalink / raw)
  To: Mark Brown; +Cc: Linux-ALSA, Pierre-Louis Bossart, Ranjani Sridharan


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

soc_init_dai_link() is needed to be called before soc_bind_dai_link().

	int snd_soc_instantiate_card()
	{
		for_each_card_prelinks(...) {
(1)			ret = soc_init_dai_link(...);
			...
		}
		...
		for_each_card_prelinks(...) {
(2)			ret = soc_bind_dai_link(...);
			...
		}
		...
		for_each_card_links(...) {
			...
(A)			ret = soc_init_dai_link(...);
			...
(B)			ret = soc_bind_dai_link(...);
		}
		...

(1) is for (2), and (A) is for (B)
(1) and (2) are for card prelink   dai_link.
(A) and (B) are for topology added dai_link.

soc_init_dai_link() is sanity check for dai_link, not initializing today.
Therefore, it is confusable naming. We can rename it as sanity_check.

And this check is for soc_bind_dai_link().
It can be more simple code if we can call it from soc_bind_dai_link().

This patch renames it to soc_dai_link_sanity_check(), and
call it from soc_bind_dai_link().

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
v2 -> v3
	- add Reviewed-by
	- call soc_dai_link_sanity_check() after soc_is_dai_link_bound()

 sound/soc/soc-core.c | 22 +++++++---------------
 1 file changed, 7 insertions(+), 15 deletions(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index a141828..827625b 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -941,8 +941,8 @@ static bool soc_is_dai_link_bound(struct snd_soc_card *card,
 	return false;
 }
 
-static int soc_init_dai_link(struct snd_soc_card *card,
-			     struct snd_soc_dai_link *link)
+static int soc_dai_link_sanity_check(struct snd_soc_card *card,
+				     struct snd_soc_dai_link *link)
 {
 	int i;
 	struct snd_soc_dai_link_component *codec, *platform;
@@ -1043,7 +1043,7 @@ static int soc_bind_dai_link(struct snd_soc_card *card,
 	struct snd_soc_pcm_runtime *rtd;
 	struct snd_soc_dai_link_component *codec, *platform;
 	struct snd_soc_component *component;
-	int i;
+	int i, ret;
 
 	if (dai_link->ignore)
 		return 0;
@@ -1056,6 +1056,10 @@ static int soc_bind_dai_link(struct snd_soc_card *card,
 		return 0;
 	}
 
+	ret = soc_dai_link_sanity_check(card, dai_link);
+	if (ret < 0)
+		return ret;
+
 	rtd = soc_new_pcm_runtime(card, dai_link);
 	if (!rtd)
 		return -ENOMEM;
@@ -1985,15 +1989,6 @@ static int snd_soc_instantiate_card(struct snd_soc_card *card)
 	int ret, i;
 
 	mutex_lock(&client_mutex);
-	for_each_card_prelinks(card, i, dai_link) {
-		ret = soc_init_dai_link(card, dai_link);
-		if (ret) {
-			dev_err(card->dev, "ASoC: failed to init link %s: %d\n",
-				dai_link->name, ret);
-			mutex_unlock(&client_mutex);
-			return ret;
-		}
-	}
 	mutex_lock_nested(&card->mutex, SND_SOC_CARD_CLASS_INIT);
 
 	snd_soc_dapm_init(&card->dapm, card, NULL);
@@ -2073,9 +2068,6 @@ static int snd_soc_instantiate_card(struct snd_soc_card *card)
 		if (soc_is_dai_link_bound(card, dai_link))
 			continue;
 
-		ret = soc_init_dai_link(card, dai_link);
-		if (ret)
-			goto probe_end;
 		ret = soc_bind_dai_link(card, dai_link);
 		if (ret)
 			goto probe_end;
-- 
2.7.4

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

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [alsa-devel] [PATCH v3 03/19] ASoC: soc-core: typo fix at soc_dai_link_sanity_check()
  2019-11-05  6:45 [alsa-devel] [PATCH v3 00/19] ASoC: soc-core cleanup - step 4 Kuninori Morimoto
  2019-11-05  6:45 ` [alsa-devel] [PATCH v3 01/19] ASoC: soc-core: move soc_init_dai_link() Kuninori Morimoto
  2019-11-05  6:45 ` [alsa-devel] [PATCH v3 02/19] ASoC: soc-core: tidyup soc_init_dai_link() Kuninori Morimoto
@ 2019-11-05  6:46 ` Kuninori Morimoto
  2019-11-05 23:51   ` [alsa-devel] Applied "ASoC: soc-core: typo fix at soc_dai_link_sanity_check()" to the asoc tree Mark Brown
  2019-11-05  6:46 ` [alsa-devel] [PATCH v3 04/19] ASoC: soc-core: remove duplicated soc_is_dai_link_bound() Kuninori Morimoto
                   ` (16 subsequent siblings)
  19 siblings, 1 reply; 60+ messages in thread
From: Kuninori Morimoto @ 2019-11-05  6:46 UTC (permalink / raw)
  To: Mark Brown; +Cc: Linux-ALSA, Pierre-Louis Bossart, Ranjani Sridharan

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

Reported-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
v2 -> v3
	- new patch

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

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 827625b..e1b0d86 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -1015,7 +1015,7 @@ static int soc_dai_link_sanity_check(struct snd_soc_card *card,
 	}
 
 	/*
-	 * Defer card registartion if cpu dai component is not added to
+	 * Defer card registration if cpu dai component is not added to
 	 * component list.
 	 */
 	if ((link->cpus->of_node || link->cpus->name) &&
-- 
2.7.4

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

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [alsa-devel] [PATCH v3 04/19] ASoC: soc-core: remove duplicated soc_is_dai_link_bound()
  2019-11-05  6:45 [alsa-devel] [PATCH v3 00/19] ASoC: soc-core cleanup - step 4 Kuninori Morimoto
                   ` (2 preceding siblings ...)
  2019-11-05  6:46 ` [alsa-devel] [PATCH v3 03/19] ASoC: soc-core: typo fix at soc_dai_link_sanity_check() Kuninori Morimoto
@ 2019-11-05  6:46 ` Kuninori Morimoto
  2019-11-05 23:51   ` [alsa-devel] Applied "ASoC: soc-core: remove duplicated soc_is_dai_link_bound()" to the asoc tree Mark Brown
  2019-11-05  6:46 ` [alsa-devel] [PATCH v3 05/19] ASoC: soc-core: call soc_bind_dai_link() under snd_soc_add_dai_link() Kuninori Morimoto
                   ` (15 subsequent siblings)
  19 siblings, 1 reply; 60+ messages in thread
From: Kuninori Morimoto @ 2019-11-05  6:46 UTC (permalink / raw)
  To: Mark Brown; +Cc: Linux-ALSA, Pierre-Louis Bossart, Ranjani Sridharan

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

soc_is_dai_link_bound() check will be called both
*before* soc_bind_dai_link() (A), and
*under*  soc_bind_dai_link() (B).
These are very verbose code. Let's remove one of them.

*	static int soc_bind_dai_link(...)
	{
		...
(B)		if (soc_is_dai_link_bound(...)) {
			...
			return 0;
		}
		...
	}

	static int snd_soc_instantiate_card(...)
	{
		...
		for_each_card_links(...) {
(A)			if (soc_is_dai_link_bound(...))
				continue;

*			ret = soc_bind_dai_link(...);
			if (ret)
				goto probe_end;
		}
		...
	}

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
v2 -> v3
	- no change
	- add Reviewed-by

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

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index e1b0d86..3cc36c2 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -2065,9 +2065,6 @@ static int snd_soc_instantiate_card(struct snd_soc_card *card)
 	 * Components with topology may bring new DAIs and DAI links.
 	 */
 	for_each_card_links(card, dai_link) {
-		if (soc_is_dai_link_bound(card, dai_link))
-			continue;
-
 		ret = soc_bind_dai_link(card, dai_link);
 		if (ret)
 			goto probe_end;
-- 
2.7.4

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

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [alsa-devel] [PATCH v3 05/19] ASoC: soc-core: call soc_bind_dai_link() under snd_soc_add_dai_link()
  2019-11-05  6:45 [alsa-devel] [PATCH v3 00/19] ASoC: soc-core cleanup - step 4 Kuninori Morimoto
                   ` (3 preceding siblings ...)
  2019-11-05  6:46 ` [alsa-devel] [PATCH v3 04/19] ASoC: soc-core: remove duplicated soc_is_dai_link_bound() Kuninori Morimoto
@ 2019-11-05  6:46 ` Kuninori Morimoto
  2019-11-05 23:51   ` [alsa-devel] Applied "ASoC: soc-core: call soc_bind_dai_link() under snd_soc_add_dai_link()" to the asoc tree Mark Brown
  2019-11-05  6:46 ` [alsa-devel] [PATCH v3 06/19] ASoC: soc-core: add soc_unbind_dai_link() Kuninori Morimoto
                   ` (14 subsequent siblings)
  19 siblings, 1 reply; 60+ messages in thread
From: Kuninori Morimoto @ 2019-11-05  6:46 UTC (permalink / raw)
  To: Mark Brown; +Cc: Linux-ALSA, Pierre-Louis Bossart, Ranjani Sridharan

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

If we focus to soc_bind_dai_link() at snd_soc_instantiate_card(),
we will notice very complex operation.

static int snd_soc_instantiate_card(...)
{
	...
	/*
	 * (1) Bind dai_link via card pre-linked dai_link
	 *
	 * Bind dai_link via card pre-linked.
	 * 1 dai_link will be 1 rtd, and connected to card.
	 * for_each_card_prelinks() is for card pre-linked dai_link.
	 *
	 * Image
	 *
	 * card
	 * - rtd(A)
	 * - rtd(A)
	 */
	for_each_card_prelinks(card, i, dai_link) {
		ret = soc_bind_dai_link(card, dai_link);
		...
	}
	...
	/*
	 * (2) Connect card pre-linked dai_link to card list
	 *
	 * Connect all card pre-linked dai_link to *card list*.
	 * Here, (A) means from card pre-linked.
	 *
	 * Image
	 *
	 * card		card list
	 *  - rtd(A)	 - dai_link(A)
	 *  - rtd(A)	 - dai_link(A)
	 *  - ...	 - ...
	 */
	for_each_card_prelinks(card, i, dai_link) {
		ret = snd_soc_add_dai_link(card, dai_link);
		...
	}
	...
	/*
	 * (3) Probe binded component
	 *
	 * Each rtd has many components.
	 * Here probes each rtd connected components.
	 * rtd(A) in Image is the probe target.
	 *
	 * During this component probe, topology may add new dai_link to
	 * *card list* by using snd_soc_add_dai_link() which is
	 * used at (2).
	 * Here, (B) means from topology
	 *
	 * Image
	 *
	 * card		card list
	 *  - rtd(A)	 - dai_link(A)
	 *  - rtd(A)	 - dai_link(A)
	 *  - ...	 - ...
	 *		 - dai_link(B)
	 *		 - dai_link(B)
	 */
	ret = soc_probe_link_components(card);
	...

	/*
	 * (4) Bind dai_link again
	 *
	 * Bind dai_link again for topology.
	 * Note, (1) used for_each_card_prelinks(),
	 * here is using  for_each_card_links()
	 *
	 * This means from card list.
	 * As Image indicating, it has dai_link(A) (from card pre-link)
	 * and dai_link(B) (from topology).
	 * main target here is dai_link(B).
	 * soc_bind_dai_link() ignores already used
	 * dai_link (= dai_link(A))
	 *
	 * Image
	 *
	 * card		card list
	 *  - rtd(A)	 - dai_link(A)
	 *  - rtd(A)	 - dai_link(A)
	 *  - ...	 - ...
	 *  - rtd(B)	 - dai_link(B)
	 *  - rtd(B)	 - dai_link(B)
	 */
	for_each_card_links(card, dai_link) {
		ret = soc_bind_dai_link(card, dai_link);
		...
	}
	...
}

As you see above, it is doing very complex method.
The problem is binding dai_link via "card pre-linked" (= (1)) and
"topology added dai_link" (= (3)) are separated.
The code can be simple if we can bind dai_link when dai_link
is connected to *card list*.
This patch do it.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
v2 -> v3
	- no change
	- add Reviewed-by

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

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 3cc36c2..e8ff6f2 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -1409,6 +1409,8 @@ EXPORT_SYMBOL_GPL(snd_soc_disconnect_sync);
 int snd_soc_add_dai_link(struct snd_soc_card *card,
 		struct snd_soc_dai_link *dai_link)
 {
+	int ret;
+
 	if (dai_link->dobj.type
 	    && dai_link->dobj.type != SND_SOC_DOBJ_DAI_LINK) {
 		dev_err(card->dev, "Invalid dai link type %d\n",
@@ -1424,6 +1426,10 @@ 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);
 
+	ret = soc_bind_dai_link(card, dai_link);
+	if (ret < 0)
+		return ret;
+
 	/* see for_each_card_links */
 	list_add_tail(&dai_link->list, &card->dai_link_list);
 
@@ -1996,13 +2002,6 @@ static int snd_soc_instantiate_card(struct snd_soc_card *card)
 	/* check whether any platform is ignore machine FE and using topology */
 	soc_check_tplg_fes(card);
 
-	/* bind DAIs */
-	for_each_card_prelinks(card, i, dai_link) {
-		ret = soc_bind_dai_link(card, dai_link);
-		if (ret != 0)
-			goto probe_end;
-	}
-
 	/* bind aux_devs too */
 	ret = soc_bind_aux_dev(card);
 	if (ret < 0)
@@ -2060,16 +2059,6 @@ static int snd_soc_instantiate_card(struct snd_soc_card *card)
 	if (ret < 0)
 		goto probe_end;
 
-	/*
-	 * Find new DAI links added during probing components and bind them.
-	 * Components with topology may bring new DAIs and DAI links.
-	 */
-	for_each_card_links(card, dai_link) {
-		ret = soc_bind_dai_link(card, dai_link);
-		if (ret)
-			goto probe_end;
-	}
-
 	/* probe all DAI links on this card */
 	ret = soc_probe_link_dais(card);
 	if (ret < 0) {
-- 
2.7.4

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

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [alsa-devel] [PATCH v3 06/19] ASoC: soc-core: add soc_unbind_dai_link()
  2019-11-05  6:45 [alsa-devel] [PATCH v3 00/19] ASoC: soc-core cleanup - step 4 Kuninori Morimoto
                   ` (4 preceding siblings ...)
  2019-11-05  6:46 ` [alsa-devel] [PATCH v3 05/19] ASoC: soc-core: call soc_bind_dai_link() under snd_soc_add_dai_link() Kuninori Morimoto
@ 2019-11-05  6:46 ` Kuninori Morimoto
  2019-11-05 23:51   ` [alsa-devel] Applied "ASoC: soc-core: add soc_unbind_dai_link()" to the asoc tree Mark Brown
  2019-11-12  5:21   ` [alsa-devel] [PATCH v3 06/19] ASoC: soc-core: add soc_unbind_dai_link() Pierre-Louis Bossart
  2019-11-05  6:46 ` [alsa-devel] [PATCH v3 07/19] ASoC: soc-core: move snd_soc_lookup_component() Kuninori Morimoto
                   ` (13 subsequent siblings)
  19 siblings, 2 replies; 60+ messages in thread
From: Kuninori Morimoto @ 2019-11-05  6:46 UTC (permalink / raw)
  To: Mark Brown; +Cc: Linux-ALSA, Pierre-Louis Bossart, Ranjani Sridharan


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

It is easy to read code if it is cleanly using paired function/naming,
like start <-> stop, register <-> unregister, etc, etc.
But, current ALSA SoC code is very random, unbalance, not paired, etc.
It is easy to create bug at the such code, and it will be difficult to
debug.

ALSA SoC has soc_bind_dai_link(), but its paired soc_unbind_dai_link()
is not implemented.
More confusable is that soc_remove_pcm_runtimes() which should be
soc_unbind_dai_link() is implemented without synchronised
to soc_bind_dai_link().

This patch cleanup this unbalance.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
v2 -> v3
	- no change
	- add Reviewed-by

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

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index e8ff6f2..1e8dd19 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -470,14 +470,6 @@ static struct snd_soc_pcm_runtime *soc_new_pcm_runtime(
 	return NULL;
 }
 
-static void soc_remove_pcm_runtimes(struct snd_soc_card *card)
-{
-	struct snd_soc_pcm_runtime *rtd, *_rtd;
-
-	for_each_card_rtds_safe(card, rtd, _rtd)
-		soc_free_pcm_runtime(rtd);
-}
-
 struct snd_soc_pcm_runtime *snd_soc_get_pcm_runtime(struct snd_soc_card *card,
 		const char *dai_link)
 {
@@ -1037,6 +1029,16 @@ static int soc_dai_link_sanity_check(struct snd_soc_card *card,
 	return 0;
 }
 
+static void soc_unbind_dai_link(struct snd_soc_card *card,
+				struct snd_soc_dai_link *dai_link)
+{
+	struct snd_soc_pcm_runtime *rtd;
+
+	rtd = snd_soc_get_pcm_runtime(card, dai_link->name);
+	if (rtd)
+		soc_free_pcm_runtime(rtd);
+}
+
 static int soc_bind_dai_link(struct snd_soc_card *card,
 	struct snd_soc_dai_link *dai_link)
 {
@@ -1466,6 +1468,8 @@ void snd_soc_remove_dai_link(struct snd_soc_card *card,
 		card->remove_dai_link(card, dai_link);
 
 	list_del(&dai_link->list);
+
+	soc_unbind_dai_link(card, dai_link);
 }
 EXPORT_SYMBOL_GPL(snd_soc_remove_dai_link);
 
@@ -1974,8 +1978,6 @@ static void soc_cleanup_card_resources(struct snd_soc_card *card)
 	for_each_card_links_safe(card, link, _link)
 		snd_soc_remove_dai_link(card, link);
 
-	soc_remove_pcm_runtimes(card);
-
 	/* remove auxiliary devices */
 	soc_remove_aux_devices(card);
 	soc_unbind_aux_dev(card);
-- 
2.7.4

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

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [alsa-devel] [PATCH v3 07/19] ASoC: soc-core: move snd_soc_lookup_component()
  2019-11-05  6:45 [alsa-devel] [PATCH v3 00/19] ASoC: soc-core cleanup - step 4 Kuninori Morimoto
                   ` (5 preceding siblings ...)
  2019-11-05  6:46 ` [alsa-devel] [PATCH v3 06/19] ASoC: soc-core: add soc_unbind_dai_link() Kuninori Morimoto
@ 2019-11-05  6:46 ` Kuninori Morimoto
  2019-11-05 23:51   ` [alsa-devel] Applied "ASoC: soc-core: move snd_soc_lookup_component()" to the asoc tree Mark Brown
  2019-11-05  6:46 ` [alsa-devel] [PATCH v3 08/19] ASoC: soc-core: tidyup snd_soc_lookup_component() Kuninori Morimoto
                   ` (12 subsequent siblings)
  19 siblings, 1 reply; 60+ messages in thread
From: Kuninori Morimoto @ 2019-11-05  6:46 UTC (permalink / raw)
  To: Mark Brown; +Cc: Linux-ALSA, Pierre-Louis Bossart, Ranjani Sridharan


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

This patch moves snd_soc_lookup_component() to upper side.
This is prepare for snd_soc_unregister_component()

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
v2 -> v3
	- no change
	- add Reviewed-by

 sound/soc/soc-core.c | 52 ++++++++++++++++++++++++++--------------------------
 1 file changed, 26 insertions(+), 26 deletions(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 1e8dd19..b71bddb 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -356,6 +356,32 @@ struct snd_soc_component *snd_soc_rtdcom_lookup(struct snd_soc_pcm_runtime *rtd,
 }
 EXPORT_SYMBOL_GPL(snd_soc_rtdcom_lookup);
 
+struct snd_soc_component *snd_soc_lookup_component(struct device *dev,
+						   const char *driver_name)
+{
+	struct snd_soc_component *component;
+	struct snd_soc_component *ret;
+
+	ret = NULL;
+	mutex_lock(&client_mutex);
+	for_each_component(component) {
+		if (dev != component->dev)
+			continue;
+
+		if (driver_name &&
+		    (driver_name != component->driver->name) &&
+		    (strcmp(component->driver->name, driver_name) != 0))
+			continue;
+
+		ret = component;
+		break;
+	}
+	mutex_unlock(&client_mutex);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(snd_soc_lookup_component);
+
 struct snd_pcm_substream *snd_soc_get_dai_substream(struct snd_soc_card *card,
 		const char *dai_link, int stream)
 {
@@ -2889,32 +2915,6 @@ void snd_soc_unregister_component(struct device *dev)
 }
 EXPORT_SYMBOL_GPL(snd_soc_unregister_component);
 
-struct snd_soc_component *snd_soc_lookup_component(struct device *dev,
-						   const char *driver_name)
-{
-	struct snd_soc_component *component;
-	struct snd_soc_component *ret;
-
-	ret = NULL;
-	mutex_lock(&client_mutex);
-	for_each_component(component) {
-		if (dev != component->dev)
-			continue;
-
-		if (driver_name &&
-		    (driver_name != component->driver->name) &&
-		    (strcmp(component->driver->name, driver_name) != 0))
-			continue;
-
-		ret = component;
-		break;
-	}
-	mutex_unlock(&client_mutex);
-
-	return ret;
-}
-EXPORT_SYMBOL_GPL(snd_soc_lookup_component);
-
 /* Retrieve a card's name from device tree */
 int snd_soc_of_parse_card_name(struct snd_soc_card *card,
 			       const char *propname)
-- 
2.7.4

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

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [alsa-devel] [PATCH v3 08/19] ASoC: soc-core: tidyup snd_soc_lookup_component()
  2019-11-05  6:45 [alsa-devel] [PATCH v3 00/19] ASoC: soc-core cleanup - step 4 Kuninori Morimoto
                   ` (6 preceding siblings ...)
  2019-11-05  6:46 ` [alsa-devel] [PATCH v3 07/19] ASoC: soc-core: move snd_soc_lookup_component() Kuninori Morimoto
@ 2019-11-05  6:46 ` Kuninori Morimoto
  2019-11-05 23:51   ` [alsa-devel] Applied "ASoC: soc-core: tidyup snd_soc_lookup_component()" to the asoc tree Mark Brown
  2019-11-05  6:46 ` [alsa-devel] [PATCH v3 09/19] ASoC: soc-core: add snd_soc_del_component_unlocked() Kuninori Morimoto
                   ` (11 subsequent siblings)
  19 siblings, 1 reply; 60+ messages in thread
From: Kuninori Morimoto @ 2019-11-05  6:46 UTC (permalink / raw)
  To: Mark Brown; +Cc: Linux-ALSA, Pierre-Louis Bossart, Ranjani Sridharan


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

snd_soc_lookup_component() is using mix of continue and break
in the same loop. It is odd.
This patch cleanup it.

Reported-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
v2 -> v3
	- new patch

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

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index b71bddb..acbaed4 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -360,25 +360,22 @@ struct snd_soc_component *snd_soc_lookup_component(struct device *dev,
 						   const char *driver_name)
 {
 	struct snd_soc_component *component;
-	struct snd_soc_component *ret;
+	struct snd_soc_component *found_component;
 
-	ret = NULL;
+	found_component = NULL;
 	mutex_lock(&client_mutex);
 	for_each_component(component) {
-		if (dev != component->dev)
-			continue;
-
-		if (driver_name &&
-		    (driver_name != component->driver->name) &&
-		    (strcmp(component->driver->name, driver_name) != 0))
-			continue;
-
-		ret = component;
-		break;
+		if ((dev == component->dev) &&
+		    (!driver_name ||
+		     (driver_name == component->driver->name) ||
+		     (strcmp(component->driver->name, driver_name) == 0))) {
+			found_component = component;
+			break;
+		}
 	}
 	mutex_unlock(&client_mutex);
 
-	return ret;
+	return found_component;
 }
 EXPORT_SYMBOL_GPL(snd_soc_lookup_component);
 
-- 
2.7.4

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

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [alsa-devel] [PATCH v3 09/19] ASoC: soc-core: add snd_soc_del_component_unlocked()
  2019-11-05  6:45 [alsa-devel] [PATCH v3 00/19] ASoC: soc-core cleanup - step 4 Kuninori Morimoto
                   ` (7 preceding siblings ...)
  2019-11-05  6:46 ` [alsa-devel] [PATCH v3 08/19] ASoC: soc-core: tidyup snd_soc_lookup_component() Kuninori Morimoto
@ 2019-11-05  6:46 ` Kuninori Morimoto
  2019-11-05 23:51   ` [alsa-devel] Applied "ASoC: soc-core: add snd_soc_del_component_unlocked()" to the asoc tree Mark Brown
  2019-11-05  6:46 ` [alsa-devel] [PATCH v3 10/19] ASoC: soc-core: remove snd_soc_component_add/del() Kuninori Morimoto
                   ` (10 subsequent siblings)
  19 siblings, 1 reply; 60+ messages in thread
From: Kuninori Morimoto @ 2019-11-05  6:46 UTC (permalink / raw)
  To: Mark Brown; +Cc: Linux-ALSA, Pierre-Louis Bossart, Ranjani Sridharan


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

It is easy to read code if it is cleanly using paired function/naming,
like start <-> stop, register <-> unregister, etc, etc.
But, current ALSA SoC code is very random, unbalance, not paired, etc.
It is easy to create bug at the such code, and is difficult to debug.

Now ALSA SoC has snd_soc_add_component(), but there is no paired
snd_soc_del_component(). Thus, snd_soc_unregister_component() is
calling cleanup function randomly. it is difficult to read.
This patch adds missing snd_soc_del_component_unlocked() and
balance up code.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
v2 -> v3
	- no change
	- add Reviewed-by

 sound/soc/soc-core.c | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index acbaed4..e91325b 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -2764,12 +2764,7 @@ static void snd_soc_component_add(struct snd_soc_component *component)
 	mutex_unlock(&client_mutex);
 }
 
-static void snd_soc_component_cleanup(struct snd_soc_component *component)
-{
-	snd_soc_unregister_dais(component);
-}
-
-static void snd_soc_component_del_unlocked(struct snd_soc_component *component)
+static void snd_soc_component_del(struct snd_soc_component *component)
 {
 	struct snd_soc_card *card = component->card;
 
@@ -2823,6 +2818,12 @@ static void snd_soc_try_rebind_card(void)
 			list_del(&card->list);
 }
 
+static void snd_soc_del_component_unlocked(struct snd_soc_component *component)
+{
+	snd_soc_unregister_dais(component);
+	snd_soc_component_del(component);
+}
+
 int snd_soc_add_component(struct device *dev,
 			struct snd_soc_component *component,
 			const struct snd_soc_component_driver *component_driver,
@@ -2855,7 +2856,7 @@ int snd_soc_add_component(struct device *dev,
 	return 0;
 
 err_cleanup:
-	snd_soc_component_cleanup(component);
+	snd_soc_del_component_unlocked(component);
 err_free:
 	return ret;
 }
@@ -2893,15 +2894,12 @@ static int __snd_soc_unregister_component(struct device *dev)
 		if (dev != component->dev)
 			continue;
 
-		snd_soc_component_del_unlocked(component);
+		snd_soc_del_component_unlocked(component);
 		found = 1;
 		break;
 	}
 	mutex_unlock(&client_mutex);
 
-	if (found)
-		snd_soc_component_cleanup(component);
-
 	return found;
 }
 
-- 
2.7.4

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

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [alsa-devel] [PATCH v3 10/19] ASoC: soc-core: remove snd_soc_component_add/del()
  2019-11-05  6:45 [alsa-devel] [PATCH v3 00/19] ASoC: soc-core cleanup - step 4 Kuninori Morimoto
                   ` (8 preceding siblings ...)
  2019-11-05  6:46 ` [alsa-devel] [PATCH v3 09/19] ASoC: soc-core: add snd_soc_del_component_unlocked() Kuninori Morimoto
@ 2019-11-05  6:46 ` Kuninori Morimoto
  2019-11-05 23:51   ` [alsa-devel] Applied "ASoC: soc-core: remove snd_soc_component_add/del()" to the asoc tree Mark Brown
  2019-11-05  6:46 ` [alsa-devel] [PATCH v3 11/19] ASoC: soc-core: use snd_soc_lookup_component() at snd_soc_unregister_component() Kuninori Morimoto
                   ` (9 subsequent siblings)
  19 siblings, 1 reply; 60+ messages in thread
From: Kuninori Morimoto @ 2019-11-05  6:46 UTC (permalink / raw)
  To: Mark Brown; +Cc: Linux-ALSA, Pierre-Louis Bossart, Ranjani Sridharan


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

soc-core has
snd_soc_add_component(), snd_soc_component_add(),
snd_soc_del_component(), snd_soc_component_del().

These are very confusing naming.
snd_soc_component_xxx() are called from snd_soc_xxx_component(),
and these are very small.
Let's merge these into snd_soc_xxx_component(), and
remove snd_soc_component_xxx().

Reported-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
v2 -> v3
	- no change
	- add Reviewed-by
	- add missing Reported-by

 sound/soc/soc-core.c | 58 ++++++++++++++++++++++------------------------------
 1 file changed, 25 insertions(+), 33 deletions(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index e91325b..bb05921 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -2746,34 +2746,6 @@ EXPORT_SYMBOL_GPL(snd_soc_component_exit_regmap);
 
 #endif
 
-static void snd_soc_component_add(struct snd_soc_component *component)
-{
-	mutex_lock(&client_mutex);
-
-	if (!component->driver->write && !component->driver->read) {
-		if (!component->regmap)
-			component->regmap = dev_get_regmap(component->dev,
-							   NULL);
-		if (component->regmap)
-			snd_soc_component_setup_regmap(component);
-	}
-
-	/* see for_each_component */
-	list_add(&component->list, &component_list);
-
-	mutex_unlock(&client_mutex);
-}
-
-static void snd_soc_component_del(struct snd_soc_component *component)
-{
-	struct snd_soc_card *card = component->card;
-
-	if (card)
-		snd_soc_unbind_card(card, false);
-
-	list_del(&component->list);
-}
-
 #define ENDIANNESS_MAP(name) \
 	(SNDRV_PCM_FMTBIT_##name##LE | SNDRV_PCM_FMTBIT_##name##BE)
 static u64 endianness_format_map[] = {
@@ -2820,8 +2792,14 @@ static void snd_soc_try_rebind_card(void)
 
 static void snd_soc_del_component_unlocked(struct snd_soc_component *component)
 {
+	struct snd_soc_card *card = component->card;
+
 	snd_soc_unregister_dais(component);
-	snd_soc_component_del(component);
+
+	if (card)
+		snd_soc_unbind_card(card, false);
+
+	list_del(&component->list);
 }
 
 int snd_soc_add_component(struct device *dev,
@@ -2833,6 +2811,8 @@ int snd_soc_add_component(struct device *dev,
 	int ret;
 	int i;
 
+	mutex_lock(&client_mutex);
+
 	ret = snd_soc_component_initialize(component, component_driver, dev);
 	if (ret)
 		goto err_free;
@@ -2850,14 +2830,26 @@ int snd_soc_add_component(struct device *dev,
 		goto err_cleanup;
 	}
 
-	snd_soc_component_add(component);
-	snd_soc_try_rebind_card();
+	if (!component->driver->write && !component->driver->read) {
+		if (!component->regmap)
+			component->regmap = dev_get_regmap(component->dev,
+							   NULL);
+		if (component->regmap)
+			snd_soc_component_setup_regmap(component);
+	}
 
-	return 0;
+	/* see for_each_component */
+	list_add(&component->list, &component_list);
 
 err_cleanup:
-	snd_soc_del_component_unlocked(component);
+	if (ret < 0)
+		snd_soc_del_component_unlocked(component);
 err_free:
+	mutex_unlock(&client_mutex);
+
+	if (ret == 0)
+		snd_soc_try_rebind_card();
+
 	return ret;
 }
 EXPORT_SYMBOL_GPL(snd_soc_add_component);
-- 
2.7.4

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

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [alsa-devel] [PATCH v3 11/19] ASoC: soc-core: use snd_soc_lookup_component() at snd_soc_unregister_component()
  2019-11-05  6:45 [alsa-devel] [PATCH v3 00/19] ASoC: soc-core cleanup - step 4 Kuninori Morimoto
                   ` (9 preceding siblings ...)
  2019-11-05  6:46 ` [alsa-devel] [PATCH v3 10/19] ASoC: soc-core: remove snd_soc_component_add/del() Kuninori Morimoto
@ 2019-11-05  6:46 ` Kuninori Morimoto
  2019-11-05 23:51   ` [alsa-devel] Applied "ASoC: soc-core: use snd_soc_lookup_component() at snd_soc_unregister_component()" to the asoc tree Mark Brown
  2019-11-05  6:46 ` [alsa-devel] [PATCH v3 12/19] ASoC: soc-core: move snd_soc_register_dai() Kuninori Morimoto
                   ` (8 subsequent siblings)
  19 siblings, 1 reply; 60+ messages in thread
From: Kuninori Morimoto @ 2019-11-05  6:46 UTC (permalink / raw)
  To: Mark Brown; +Cc: Linux-ALSA, Pierre-Louis Bossart, Ranjani Sridharan

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

snd_soc_unregister_component() is now finding component manually,
but we already have snd_soc_lookup_component() to find component;
Let's use existing function.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
v2 -> v3
	- no change
	- add Reviewed-by

 sound/soc/soc-core.c | 20 +++++---------------
 1 file changed, 5 insertions(+), 15 deletions(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index bb05921..0ce3336 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -2876,29 +2876,19 @@ EXPORT_SYMBOL_GPL(snd_soc_register_component);
  *
  * @dev: The device to unregister
  */
-static int __snd_soc_unregister_component(struct device *dev)
+void snd_soc_unregister_component(struct device *dev)
 {
 	struct snd_soc_component *component;
-	int found = 0;
 
 	mutex_lock(&client_mutex);
-	for_each_component(component) {
-		if (dev != component->dev)
-			continue;
+	while (1) {
+		component = snd_soc_lookup_component(dev, NULL);
+		if (!component)
+			break;
 
 		snd_soc_del_component_unlocked(component);
-		found = 1;
-		break;
 	}
 	mutex_unlock(&client_mutex);
-
-	return found;
-}
-
-void snd_soc_unregister_component(struct device *dev)
-{
-	while (__snd_soc_unregister_component(dev))
-		;
 }
 EXPORT_SYMBOL_GPL(snd_soc_unregister_component);
 
-- 
2.7.4

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

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [alsa-devel] [PATCH v3 12/19] ASoC: soc-core: move snd_soc_register_dai()
  2019-11-05  6:45 [alsa-devel] [PATCH v3 00/19] ASoC: soc-core cleanup - step 4 Kuninori Morimoto
                   ` (10 preceding siblings ...)
  2019-11-05  6:46 ` [alsa-devel] [PATCH v3 11/19] ASoC: soc-core: use snd_soc_lookup_component() at snd_soc_unregister_component() Kuninori Morimoto
@ 2019-11-05  6:46 ` Kuninori Morimoto
  2019-11-05 23:51   ` [alsa-devel] Applied "ASoC: soc-core: move snd_soc_register_dai()" to the asoc tree Mark Brown
  2019-11-05  6:47 ` [alsa-devel] [PATCH v3 13/19] ASoC: soc-core: move snd_soc_unregister_dais() Kuninori Morimoto
                   ` (7 subsequent siblings)
  19 siblings, 1 reply; 60+ messages in thread
From: Kuninori Morimoto @ 2019-11-05  6:46 UTC (permalink / raw)
  To: Mark Brown; +Cc: Linux-ALSA, Pierre-Louis Bossart, Ranjani Sridharan


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

This patch moves snd_soc_register_dai() next to
snd_soc_register_dais().
This is prepare for snd_soc_register_dais() cleanup.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
v2 -> v3
	- no change
	- add Reviewed-by

 sound/soc/soc-core.c | 72 ++++++++++++++++++++++++++--------------------------
 1 file changed, 36 insertions(+), 36 deletions(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 0ce3336..fb5f014 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -2598,42 +2598,6 @@ static struct snd_soc_dai *soc_add_dai(struct snd_soc_component *component,
 }
 
 /**
- * snd_soc_register_dais - Register a DAI with the ASoC core
- *
- * @component: The component the DAIs are registered for
- * @dai_drv: DAI driver to use for the DAIs
- * @count: Number of DAIs
- */
-static int snd_soc_register_dais(struct snd_soc_component *component,
-				 struct snd_soc_dai_driver *dai_drv,
-				 size_t count)
-{
-	struct device *dev = component->dev;
-	struct snd_soc_dai *dai;
-	unsigned int i;
-	int ret;
-
-	dev_dbg(dev, "ASoC: dai register %s #%zu\n", dev_name(dev), count);
-
-	for (i = 0; i < count; i++) {
-
-		dai = soc_add_dai(component, dai_drv + i, count == 1 &&
-				  !component->driver->non_legacy_dai_naming);
-		if (dai == NULL) {
-			ret = -ENOMEM;
-			goto err;
-		}
-	}
-
-	return 0;
-
-err:
-	snd_soc_unregister_dais(component);
-
-	return ret;
-}
-
-/**
  * snd_soc_register_dai - Register a DAI dynamically & create its widgets
  *
  * @component: The component the DAIs are registered for
@@ -2676,6 +2640,42 @@ int snd_soc_register_dai(struct snd_soc_component *component,
 }
 EXPORT_SYMBOL_GPL(snd_soc_register_dai);
 
+/**
+ * snd_soc_register_dais - Register a DAI with the ASoC core
+ *
+ * @component: The component the DAIs are registered for
+ * @dai_drv: DAI driver to use for the DAIs
+ * @count: Number of DAIs
+ */
+static int snd_soc_register_dais(struct snd_soc_component *component,
+				 struct snd_soc_dai_driver *dai_drv,
+				 size_t count)
+{
+	struct device *dev = component->dev;
+	struct snd_soc_dai *dai;
+	unsigned int i;
+	int ret;
+
+	dev_dbg(dev, "ASoC: dai register %s #%zu\n", dev_name(dev), count);
+
+	for (i = 0; i < count; i++) {
+
+		dai = soc_add_dai(component, dai_drv + i, count == 1 &&
+				  !component->driver->non_legacy_dai_naming);
+		if (dai == NULL) {
+			ret = -ENOMEM;
+			goto err;
+		}
+	}
+
+	return 0;
+
+err:
+	snd_soc_unregister_dais(component);
+
+	return ret;
+}
+
 static int snd_soc_component_initialize(struct snd_soc_component *component,
 	const struct snd_soc_component_driver *driver, struct device *dev)
 {
-- 
2.7.4

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

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [alsa-devel] [PATCH v3 13/19] ASoC: soc-core: move snd_soc_unregister_dais()
  2019-11-05  6:45 [alsa-devel] [PATCH v3 00/19] ASoC: soc-core cleanup - step 4 Kuninori Morimoto
                   ` (11 preceding siblings ...)
  2019-11-05  6:46 ` [alsa-devel] [PATCH v3 12/19] ASoC: soc-core: move snd_soc_register_dai() Kuninori Morimoto
@ 2019-11-05  6:47 ` Kuninori Morimoto
  2019-11-05 23:51   ` [alsa-devel] Applied "ASoC: soc-core: move snd_soc_unregister_dais()" to the asoc tree Mark Brown
  2019-11-05  6:47 ` [alsa-devel] [PATCH v3 14/19] ASoC: soc-core: add snd_soc_unregister_dai() Kuninori Morimoto
                   ` (6 subsequent siblings)
  19 siblings, 1 reply; 60+ messages in thread
From: Kuninori Morimoto @ 2019-11-05  6:47 UTC (permalink / raw)
  To: Mark Brown; +Cc: Linux-ALSA, Pierre-Louis Bossart, Ranjani Sridharan


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

This patch moves snd_soc_unregister_dais() next to
snd_soc_register_dais().
This is prepare for snd_soc_register_dais() cleanup

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
v2 -> v3
	- no change
	- add Reviewed-by

 sound/soc/soc-core.c | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index fb5f014..c803447 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -2532,22 +2532,6 @@ static inline char *fmt_multiple_name(struct device *dev,
 	return devm_kstrdup(dev, dai_drv->name, GFP_KERNEL);
 }
 
-/**
- * snd_soc_unregister_dai - Unregister DAIs from the ASoC core
- *
- * @component: The component for which the DAIs should be unregistered
- */
-static void snd_soc_unregister_dais(struct snd_soc_component *component)
-{
-	struct snd_soc_dai *dai, *_dai;
-
-	for_each_component_dais_safe(component, dai, _dai) {
-		dev_dbg(component->dev, "ASoC: Unregistered DAI '%s'\n",
-			dai->name);
-		list_del(&dai->list);
-	}
-}
-
 /* Create a DAI and add it to the component's DAI list */
 static struct snd_soc_dai *soc_add_dai(struct snd_soc_component *component,
 	struct snd_soc_dai_driver *dai_drv,
@@ -2641,6 +2625,22 @@ int snd_soc_register_dai(struct snd_soc_component *component,
 EXPORT_SYMBOL_GPL(snd_soc_register_dai);
 
 /**
+ * snd_soc_unregister_dai - Unregister DAIs from the ASoC core
+ *
+ * @component: The component for which the DAIs should be unregistered
+ */
+static void snd_soc_unregister_dais(struct snd_soc_component *component)
+{
+	struct snd_soc_dai *dai, *_dai;
+
+	for_each_component_dais_safe(component, dai, _dai) {
+		dev_dbg(component->dev, "ASoC: Unregistered DAI '%s'\n",
+			dai->name);
+		list_del(&dai->list);
+	}
+}
+
+/**
  * snd_soc_register_dais - Register a DAI with the ASoC core
  *
  * @component: The component the DAIs are registered for
-- 
2.7.4

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

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [alsa-devel] [PATCH v3 14/19] ASoC: soc-core: add snd_soc_unregister_dai()
  2019-11-05  6:45 [alsa-devel] [PATCH v3 00/19] ASoC: soc-core cleanup - step 4 Kuninori Morimoto
                   ` (12 preceding siblings ...)
  2019-11-05  6:47 ` [alsa-devel] [PATCH v3 13/19] ASoC: soc-core: move snd_soc_unregister_dais() Kuninori Morimoto
@ 2019-11-05  6:47 ` Kuninori Morimoto
  2019-11-05 23:51   ` [alsa-devel] Applied "ASoC: soc-core: add snd_soc_unregister_dai()" to the asoc tree Mark Brown
  2019-11-05  6:47 ` [alsa-devel] [PATCH v3 15/19] ASoC: soc-core: have legacy_dai_naming at snd_soc_register_dai() Kuninori Morimoto
                   ` (5 subsequent siblings)
  19 siblings, 1 reply; 60+ messages in thread
From: Kuninori Morimoto @ 2019-11-05  6:47 UTC (permalink / raw)
  To: Mark Brown; +Cc: Linux-ALSA, Pierre-Louis Bossart, Ranjani Sridharan


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

It is easy to read code if it is cleanly using paired function/naming,
like start <-> stop, register <-> unregister, etc, etc.
But, current ALSA SoC code is very random, unbalance, not paired, etc.
It is easy to create bug at the such code, and is difficult to debug.

This patch adds missing soc_del_dai() and snd_soc_unregister_dai().

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
v2 -> v3
	- no change
	- add Reviewed-by

 include/sound/soc.h  |  1 +
 sound/soc/soc-core.c | 19 ++++++++++++++-----
 2 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/include/sound/soc.h b/include/sound/soc.h
index 320dcd4..daa0e10 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -1328,6 +1328,7 @@ struct snd_soc_dai_link *snd_soc_find_dai_link(struct snd_soc_card *card,
 
 int snd_soc_register_dai(struct snd_soc_component *component,
 	struct snd_soc_dai_driver *dai_drv);
+void snd_soc_unregister_dai(struct snd_soc_dai *dai);
 
 struct snd_soc_dai *snd_soc_find_dai(
 	const struct snd_soc_dai_link_component *dlc);
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index c803447..38199cd 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -2532,6 +2532,12 @@ static inline char *fmt_multiple_name(struct device *dev,
 	return devm_kstrdup(dev, dai_drv->name, GFP_KERNEL);
 }
 
+static void soc_del_dai(struct snd_soc_dai *dai)
+{
+	dev_dbg(dai->dev, "ASoC: Unregistered DAI '%s'\n", dai->name);
+	list_del(&dai->list);
+}
+
 /* Create a DAI and add it to the component's DAI list */
 static struct snd_soc_dai *soc_add_dai(struct snd_soc_component *component,
 	struct snd_soc_dai_driver *dai_drv,
@@ -2581,6 +2587,12 @@ static struct snd_soc_dai *soc_add_dai(struct snd_soc_component *component,
 	return dai;
 }
 
+void snd_soc_unregister_dai(struct snd_soc_dai *dai)
+{
+	soc_del_dai(dai);
+}
+EXPORT_SYMBOL_GPL(snd_soc_unregister_dai);
+
 /**
  * snd_soc_register_dai - Register a DAI dynamically & create its widgets
  *
@@ -2633,11 +2645,8 @@ static void snd_soc_unregister_dais(struct snd_soc_component *component)
 {
 	struct snd_soc_dai *dai, *_dai;
 
-	for_each_component_dais_safe(component, dai, _dai) {
-		dev_dbg(component->dev, "ASoC: Unregistered DAI '%s'\n",
-			dai->name);
-		list_del(&dai->list);
-	}
+	for_each_component_dais_safe(component, dai, _dai)
+		snd_soc_unregister_dai(dai);
 }
 
 /**
-- 
2.7.4

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

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [alsa-devel] [PATCH v3 15/19] ASoC: soc-core: have legacy_dai_naming at snd_soc_register_dai()
  2019-11-05  6:45 [alsa-devel] [PATCH v3 00/19] ASoC: soc-core cleanup - step 4 Kuninori Morimoto
                   ` (13 preceding siblings ...)
  2019-11-05  6:47 ` [alsa-devel] [PATCH v3 14/19] ASoC: soc-core: add snd_soc_unregister_dai() Kuninori Morimoto
@ 2019-11-05  6:47 ` Kuninori Morimoto
  2019-11-05 23:51   ` [alsa-devel] Applied "ASoC: soc-core: have legacy_dai_naming at snd_soc_register_dai()" to the asoc tree Mark Brown
  2019-11-05  6:47 ` [alsa-devel] [PATCH v3 16/19] ASoC: soc-core: don't call snd_soc_dapm_new_dai_widgets() at snd_soc_register_dai() Kuninori Morimoto
                   ` (4 subsequent siblings)
  19 siblings, 1 reply; 60+ messages in thread
From: Kuninori Morimoto @ 2019-11-05  6:47 UTC (permalink / raw)
  To: Mark Brown; +Cc: Linux-ALSA, Pierre-Louis Bossart, Ranjani Sridharan

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

ALSA SoC has 2 functions.
snd_soc_register_dai()  is used from topology
snd_soc_register_dais() is used from snd_soc_add_component()

In general, people think like _dai() is called from _dais()
with for loop. But in reality, these are very similar
but different implementation.
We shouldn't have duplicated and confusing implementation.

snd_soc_register_dai() is now used from topology.
But to reduce duplicated code, it should be used from _dais(), too.
To prepare it, this patch adds missing parameter legacy_dai_naming
to snd_soc_register_dai().

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
v2 -> v3
	- no change
	- add Reviewed-by

 include/sound/soc.h      | 3 ++-
 sound/soc/soc-core.c     | 5 +++--
 sound/soc/soc-topology.c | 2 +-
 3 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/include/sound/soc.h b/include/sound/soc.h
index daa0e10..766fa81 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -1327,7 +1327,8 @@ struct snd_soc_dai_link *snd_soc_find_dai_link(struct snd_soc_card *card,
 					       const char *stream_name);
 
 int snd_soc_register_dai(struct snd_soc_component *component,
-	struct snd_soc_dai_driver *dai_drv);
+			 struct snd_soc_dai_driver *dai_drv,
+			 bool legacy_dai_naming);
 void snd_soc_unregister_dai(struct snd_soc_dai *dai);
 
 struct snd_soc_dai *snd_soc_find_dai(
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 38199cd..6f4933f 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -2604,7 +2604,8 @@ EXPORT_SYMBOL_GPL(snd_soc_unregister_dai);
  * will be freed in the component cleanup.
  */
 int snd_soc_register_dai(struct snd_soc_component *component,
-	struct snd_soc_dai_driver *dai_drv)
+			 struct snd_soc_dai_driver *dai_drv,
+			 bool legacy_dai_naming)
 {
 	struct snd_soc_dapm_context *dapm =
 		snd_soc_component_get_dapm(component);
@@ -2618,7 +2619,7 @@ int snd_soc_register_dai(struct snd_soc_component *component,
 	}
 
 	lockdep_assert_held(&client_mutex);
-	dai = soc_add_dai(component, dai_drv, false);
+	dai = soc_add_dai(component, dai_drv, legacy_dai_naming);
 	if (!dai)
 		return -ENOMEM;
 
diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c
index 0fd0329..b6e1456 100644
--- a/sound/soc/soc-topology.c
+++ b/sound/soc/soc-topology.c
@@ -1842,7 +1842,7 @@ static int soc_tplg_dai_create(struct soc_tplg *tplg,
 	list_add(&dai_drv->dobj.list, &tplg->comp->dobj_list);
 
 	/* register the DAI to the component */
-	return snd_soc_register_dai(tplg->comp, dai_drv);
+	return snd_soc_register_dai(tplg->comp, dai_drv, false);
 }
 
 static void set_link_flags(struct snd_soc_dai_link *link,
-- 
2.7.4

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

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [alsa-devel] [PATCH v3 16/19] ASoC: soc-core: don't call snd_soc_dapm_new_dai_widgets() at snd_soc_register_dai()
  2019-11-05  6:45 [alsa-devel] [PATCH v3 00/19] ASoC: soc-core cleanup - step 4 Kuninori Morimoto
                   ` (14 preceding siblings ...)
  2019-11-05  6:47 ` [alsa-devel] [PATCH v3 15/19] ASoC: soc-core: have legacy_dai_naming at snd_soc_register_dai() Kuninori Morimoto
@ 2019-11-05  6:47 ` Kuninori Morimoto
  2019-11-05 23:51   ` [alsa-devel] Applied "ASoC: soc-core: don't call snd_soc_dapm_new_dai_widgets() at snd_soc_register_dai()" to the asoc tree Mark Brown
  2019-11-05  6:47 ` [alsa-devel] [PATCH v3 17/19] ASoC: soc-core: call snd_soc_register_dai() from snd_soc_register_dais() Kuninori Morimoto
                   ` (3 subsequent siblings)
  19 siblings, 1 reply; 60+ messages in thread
From: Kuninori Morimoto @ 2019-11-05  6:47 UTC (permalink / raw)
  To: Mark Brown; +Cc: Linux-ALSA, Pierre-Louis Bossart, Ranjani Sridharan

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

ALSA SoC has 2 functions.
snd_soc_register_dai()  is used from topology
snd_soc_register_dais() is used from snd_soc_add_component()

In general, people think like _dai() is called from _dais()
with for loop. But in reality, these are very similar
but different implementation.
We shouldn't have duplicated and confusing implementation.

snd_soc_register_dai() is now used from topology.
But to reduce duplicated code, it should be used from _dais(), too.

Because of topology side specific reason,
it is calling snd_soc_dapm_new_dai_widgets(),
but it is not needed _dais() side.

This patch factorizes snd_soc_register_dai() to
topology / _dais() common part, and topology specific part.
And do topology specific part at soc-topology.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
v2 -> v3
	- add Reviewed-by
	- directly return soc_add_dai()

 include/sound/soc.h      |  6 +++---
 sound/soc/soc-core.c     | 29 +++++------------------------
 sound/soc/soc-topology.c | 17 ++++++++++++++++-
 3 files changed, 24 insertions(+), 28 deletions(-)

diff --git a/include/sound/soc.h b/include/sound/soc.h
index 766fa81..4e38ee6 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -1326,9 +1326,9 @@ struct snd_soc_dai_link *snd_soc_find_dai_link(struct snd_soc_card *card,
 					       int id, const char *name,
 					       const char *stream_name);
 
-int snd_soc_register_dai(struct snd_soc_component *component,
-			 struct snd_soc_dai_driver *dai_drv,
-			 bool legacy_dai_naming);
+struct snd_soc_dai *snd_soc_register_dai(struct snd_soc_component *component,
+					 struct snd_soc_dai_driver *dai_drv,
+					 bool legacy_dai_naming);
 void snd_soc_unregister_dai(struct snd_soc_dai *dai);
 
 struct snd_soc_dai *snd_soc_find_dai(
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 6f4933f..55b13c0 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -2603,37 +2603,18 @@ EXPORT_SYMBOL_GPL(snd_soc_unregister_dai);
  * These DAIs's widgets will be freed in the card cleanup and the DAIs
  * will be freed in the component cleanup.
  */
-int snd_soc_register_dai(struct snd_soc_component *component,
-			 struct snd_soc_dai_driver *dai_drv,
-			 bool legacy_dai_naming)
+struct snd_soc_dai *snd_soc_register_dai(struct snd_soc_component *component,
+					 struct snd_soc_dai_driver *dai_drv,
+					 bool legacy_dai_naming)
 {
-	struct snd_soc_dapm_context *dapm =
-		snd_soc_component_get_dapm(component);
-	struct snd_soc_dai *dai;
-	int ret;
-
 	if (dai_drv->dobj.type != SND_SOC_DOBJ_PCM) {
 		dev_err(component->dev, "Invalid dai type %d\n",
 			dai_drv->dobj.type);
-		return -EINVAL;
+		return NULL;
 	}
 
 	lockdep_assert_held(&client_mutex);
-	dai = soc_add_dai(component, dai_drv, legacy_dai_naming);
-	if (!dai)
-		return -ENOMEM;
-
-	/*
-	 * Create the DAI widgets here. After adding DAIs, topology may
-	 * also add routes that need these widgets as source or sink.
-	 */
-	ret = snd_soc_dapm_new_dai_widgets(dapm, dai);
-	if (ret != 0) {
-		dev_err(component->dev,
-			"Failed to create DAI widgets %d\n", ret);
-	}
-
-	return ret;
+	return soc_add_dai(component, dai_drv, legacy_dai_naming);
 }
 EXPORT_SYMBOL_GPL(snd_soc_register_dai);
 
diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c
index b6e1456..81d2af0 100644
--- a/sound/soc/soc-topology.c
+++ b/sound/soc/soc-topology.c
@@ -1800,6 +1800,9 @@ static int soc_tplg_dai_create(struct soc_tplg *tplg,
 	struct snd_soc_dai_driver *dai_drv;
 	struct snd_soc_pcm_stream *stream;
 	struct snd_soc_tplg_stream_caps *caps;
+	struct snd_soc_dai *dai;
+	struct snd_soc_dapm_context *dapm =
+		snd_soc_component_get_dapm(tplg->comp);
 	int ret;
 
 	dai_drv = kzalloc(sizeof(struct snd_soc_dai_driver), GFP_KERNEL);
@@ -1842,7 +1845,19 @@ static int soc_tplg_dai_create(struct soc_tplg *tplg,
 	list_add(&dai_drv->dobj.list, &tplg->comp->dobj_list);
 
 	/* register the DAI to the component */
-	return snd_soc_register_dai(tplg->comp, dai_drv, false);
+	dai = snd_soc_register_dai(tplg->comp, dai_drv, false);
+	if (!dai)
+		return -ENOMEM;
+
+	/* Create the DAI widgets here */
+	ret = snd_soc_dapm_new_dai_widgets(dapm, dai);
+	if (ret != 0) {
+		dev_err(dai->dev, "Failed to create DAI widgets %d\n", ret);
+		snd_soc_unregister_dai(dai);
+		return ret;
+	}
+
+	return ret;
 }
 
 static void set_link_flags(struct snd_soc_dai_link *link,
-- 
2.7.4

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

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [alsa-devel] [PATCH v3 17/19] ASoC: soc-core: call snd_soc_register_dai() from snd_soc_register_dais()
  2019-11-05  6:45 [alsa-devel] [PATCH v3 00/19] ASoC: soc-core cleanup - step 4 Kuninori Morimoto
                   ` (15 preceding siblings ...)
  2019-11-05  6:47 ` [alsa-devel] [PATCH v3 16/19] ASoC: soc-core: don't call snd_soc_dapm_new_dai_widgets() at snd_soc_register_dai() Kuninori Morimoto
@ 2019-11-05  6:47 ` Kuninori Morimoto
  2019-11-05 23:51   ` [alsa-devel] Applied "ASoC: soc-core: call snd_soc_register_dai() from snd_soc_register_dais()" to the asoc tree Mark Brown
  2019-11-05  6:47 ` [alsa-devel] [PATCH v3 18/19] ASoC: soc-core: remove topology specific operation Kuninori Morimoto
                   ` (2 subsequent siblings)
  19 siblings, 1 reply; 60+ messages in thread
From: Kuninori Morimoto @ 2019-11-05  6:47 UTC (permalink / raw)
  To: Mark Brown; +Cc: Linux-ALSA, Pierre-Louis Bossart, Ranjani Sridharan

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

ALSA SoC has 2 functions.
snd_soc_register_dai()  is used from topology
snd_soc_register_dais() is used from snd_soc_add_component()

In general, people think like _dai() is called from _dais()
with for loop. But in reality, these are very similar
but different implementation.
We shouldn't have duplicated and confusing implementation.

This patch calls snd_soc_register_dai() from snd_soc_register_dais()

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
v2 -> v3
	- no change
	- add Reviewed-by

 sound/soc/soc-core.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 55b13c0..86c45f7 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -2607,12 +2607,16 @@ struct snd_soc_dai *snd_soc_register_dai(struct snd_soc_component *component,
 					 struct snd_soc_dai_driver *dai_drv,
 					 bool legacy_dai_naming)
 {
-	if (dai_drv->dobj.type != SND_SOC_DOBJ_PCM) {
-		dev_err(component->dev, "Invalid dai type %d\n",
-			dai_drv->dobj.type);
+	struct device *dev = component->dev;
+
+	if (dai_drv->dobj.type &&
+	    dai_drv->dobj.type != SND_SOC_DOBJ_PCM) {
+		dev_err(dev, "Invalid dai type %d\n", dai_drv->dobj.type);
 		return NULL;
 	}
 
+	dev_dbg(dev, "ASoC: dai register %s\n", dai_drv->name);
+
 	lockdep_assert_held(&client_mutex);
 	return soc_add_dai(component, dai_drv, legacy_dai_naming);
 }
@@ -2642,16 +2646,12 @@ static int snd_soc_register_dais(struct snd_soc_component *component,
 				 struct snd_soc_dai_driver *dai_drv,
 				 size_t count)
 {
-	struct device *dev = component->dev;
 	struct snd_soc_dai *dai;
 	unsigned int i;
 	int ret;
 
-	dev_dbg(dev, "ASoC: dai register %s #%zu\n", dev_name(dev), count);
-
 	for (i = 0; i < count; i++) {
-
-		dai = soc_add_dai(component, dai_drv + i, count == 1 &&
+		dai = snd_soc_register_dai(component, dai_drv + i, count == 1 &&
 				  !component->driver->non_legacy_dai_naming);
 		if (dai == NULL) {
 			ret = -ENOMEM;
-- 
2.7.4

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

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [alsa-devel] [PATCH v3 18/19] ASoC: soc-core: remove topology specific operation
  2019-11-05  6:45 [alsa-devel] [PATCH v3 00/19] ASoC: soc-core cleanup - step 4 Kuninori Morimoto
                   ` (16 preceding siblings ...)
  2019-11-05  6:47 ` [alsa-devel] [PATCH v3 17/19] ASoC: soc-core: call snd_soc_register_dai() from snd_soc_register_dais() Kuninori Morimoto
@ 2019-11-05  6:47 ` Kuninori Morimoto
  2019-11-05 23:51   ` [alsa-devel] Applied "ASoC: soc-core: remove topology specific operation" to the asoc tree Mark Brown
  2019-11-05  6:47 ` [alsa-devel] [PATCH v3 19/19] ASoC: soc.h: dobj is used only when SND_SOC_TOPOLOGY Kuninori Morimoto
  2019-11-05 16:18 ` [alsa-devel] [PATCH v3 00/19] ASoC: soc-core cleanup - step 4 Ranjani Sridharan
  19 siblings, 1 reply; 60+ messages in thread
From: Kuninori Morimoto @ 2019-11-05  6:47 UTC (permalink / raw)
  To: Mark Brown; +Cc: Linux-ALSA, Pierre-Louis Bossart, Ranjani Sridharan


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

soc-core has some API which is used from topology, but it is doing
topology specific operation at soc-core.
soc-core should care about core things, and topology should care
about topology things, otherwise, it is very confusable.

For example topology type is not related to soc-core,
it is topology side issue.

This patch removes meaningless check from soc-core.

This patch keeps extra initialization/destruction at
snd_soc_add_dai_link() / snd_soc_remove_dai_link()
which were for topology.
From this patch, non-topology card can use it.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
v2 -> v3
	- no change
	- add Reviewed-by

 sound/soc/soc-core.c | 28 ++++------------------------
 1 file changed, 4 insertions(+), 24 deletions(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 86c45f7..cc59687 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -1436,19 +1436,12 @@ int snd_soc_add_dai_link(struct snd_soc_card *card,
 {
 	int ret;
 
-	if (dai_link->dobj.type
-	    && dai_link->dobj.type != SND_SOC_DOBJ_DAI_LINK) {
-		dev_err(card->dev, "Invalid dai link type %d\n",
-			dai_link->dobj.type);
-		return -EINVAL;
-	}
-
 	lockdep_assert_held(&client_mutex);
+
 	/*
 	 * Notify the machine driver for extra initialization
-	 * on the link created by topology.
 	 */
-	if (dai_link->dobj.type && card->add_dai_link)
+	if (card->add_dai_link)
 		card->add_dai_link(card, dai_link);
 
 	ret = soc_bind_dai_link(card, dai_link);
@@ -1475,19 +1468,12 @@ EXPORT_SYMBOL_GPL(snd_soc_add_dai_link);
 void snd_soc_remove_dai_link(struct snd_soc_card *card,
 			     struct snd_soc_dai_link *dai_link)
 {
-	if (dai_link->dobj.type
-	    && dai_link->dobj.type != SND_SOC_DOBJ_DAI_LINK) {
-		dev_err(card->dev, "Invalid dai link type %d\n",
-			dai_link->dobj.type);
-		return;
-	}
-
 	lockdep_assert_held(&client_mutex);
+
 	/*
 	 * Notify the machine driver for extra destruction
-	 * on the link created by topology.
 	 */
-	if (dai_link->dobj.type && card->remove_dai_link)
+	if (card->remove_dai_link)
 		card->remove_dai_link(card, dai_link);
 
 	list_del(&dai_link->list);
@@ -2609,12 +2595,6 @@ struct snd_soc_dai *snd_soc_register_dai(struct snd_soc_component *component,
 {
 	struct device *dev = component->dev;
 
-	if (dai_drv->dobj.type &&
-	    dai_drv->dobj.type != SND_SOC_DOBJ_PCM) {
-		dev_err(dev, "Invalid dai type %d\n", dai_drv->dobj.type);
-		return NULL;
-	}
-
 	dev_dbg(dev, "ASoC: dai register %s\n", dai_drv->name);
 
 	lockdep_assert_held(&client_mutex);
-- 
2.7.4

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

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [alsa-devel] [PATCH v3 19/19] ASoC: soc.h: dobj is used only when SND_SOC_TOPOLOGY
  2019-11-05  6:45 [alsa-devel] [PATCH v3 00/19] ASoC: soc-core cleanup - step 4 Kuninori Morimoto
                   ` (17 preceding siblings ...)
  2019-11-05  6:47 ` [alsa-devel] [PATCH v3 18/19] ASoC: soc-core: remove topology specific operation Kuninori Morimoto
@ 2019-11-05  6:47 ` Kuninori Morimoto
  2019-11-05 23:51   ` [alsa-devel] Applied "ASoC: soc.h: dobj is used only when SND_SOC_TOPOLOGY" to the asoc tree Mark Brown
  2019-11-05 16:18 ` [alsa-devel] [PATCH v3 00/19] ASoC: soc-core cleanup - step 4 Ranjani Sridharan
  19 siblings, 1 reply; 60+ messages in thread
From: Kuninori Morimoto @ 2019-11-05  6:47 UTC (permalink / raw)
  To: Mark Brown; +Cc: Linux-ALSA, Pierre-Louis Bossart, Ranjani Sridharan


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

snd_soc_dobj is used only when SND_SOC_TOPOLOGY was selected.
Let's enable it under SND_SOC_TOPOLOGY.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
v2 -> v3
	- no change
	- add Reviewed-by

 include/sound/soc.h | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/include/sound/soc.h b/include/sound/soc.h
index 4e38ee6..e0855dc 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -847,7 +847,9 @@ struct snd_soc_dai_link {
 	unsigned int ignore:1;
 
 	struct list_head list; /* DAI link list of the soc card */
+#ifdef CONFIG_SND_SOC_TOPOLOGY
 	struct snd_soc_dobj dobj; /* For topology */
+#endif
 };
 #define for_each_link_codecs(link, i, codec)				\
 	for ((i) = 0;							\
@@ -1169,7 +1171,9 @@ struct soc_mixer_control {
 	unsigned int sign_bit;
 	unsigned int invert:1;
 	unsigned int autodisable:1;
+#ifdef CONFIG_SND_SOC_TOPOLOGY
 	struct snd_soc_dobj dobj;
+#endif
 };
 
 struct soc_bytes {
@@ -1180,8 +1184,9 @@ struct soc_bytes {
 
 struct soc_bytes_ext {
 	int max;
+#ifdef CONFIG_SND_SOC_TOPOLOGY
 	struct snd_soc_dobj dobj;
-
+#endif
 	/* used for TLV byte control */
 	int (*get)(struct snd_kcontrol *kcontrol, unsigned int __user *bytes,
 			unsigned int size);
@@ -1205,7 +1210,9 @@ struct soc_enum {
 	const char * const *texts;
 	const unsigned int *values;
 	unsigned int autodisable:1;
+#ifdef CONFIG_SND_SOC_TOPOLOGY
 	struct snd_soc_dobj dobj;
+#endif
 };
 
 /* device driver data */
-- 
2.7.4

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

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* Re: [alsa-devel] [PATCH v3 00/19] ASoC: soc-core cleanup - step 4
  2019-11-05  6:45 [alsa-devel] [PATCH v3 00/19] ASoC: soc-core cleanup - step 4 Kuninori Morimoto
                   ` (18 preceding siblings ...)
  2019-11-05  6:47 ` [alsa-devel] [PATCH v3 19/19] ASoC: soc.h: dobj is used only when SND_SOC_TOPOLOGY Kuninori Morimoto
@ 2019-11-05 16:18 ` Ranjani Sridharan
  19 siblings, 0 replies; 60+ messages in thread
From: Ranjani Sridharan @ 2019-11-05 16:18 UTC (permalink / raw)
  To: Kuninori Morimoto, Mark Brown; +Cc: Linux-ALSA, Pierre-Louis Bossart

On Tue, 2019-11-05 at 15:45 +0900, Kuninori Morimoto wrote:
> Hi Mark
> 
> These are v3 of soc-core cleanup step4.
> These are based on mark/for-5.5 branch.
> These had been tested on Intel CI, and had no issue.
> And are reviewed by Pierre-Louis, Ranjani.
Thanks for this clean-up, Morimoto-san.
v3 patchset looks good!

Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
> 
> Kuninori Morimoto (19):
>   ASoC: soc-core: move soc_init_dai_link()
>   ASoC: soc-core: tidyup soc_init_dai_link()
>   ASoC: soc-core: typo fix at soc_dai_link_sanity_check()
>   ASoC: soc-core: remove duplicated soc_is_dai_link_bound()
>   ASoC: soc-core: call soc_bind_dai_link() under
> snd_soc_add_dai_link()
>   ASoC: soc-core: add soc_unbind_dai_link()
>   ASoC: soc-core: move snd_soc_lookup_component()
>   ASoC: soc-core: tidyup snd_soc_lookup_component()
>   ASoC: soc-core: add snd_soc_del_component_unlocked()
>   ASoC: soc-core: remove snd_soc_component_add/del()
>   ASoC: soc-core: use snd_soc_lookup_component() at
> snd_soc_unregister_component()
>   ASoC: soc-core: move snd_soc_register_dai()
>   ASoC: soc-core: move snd_soc_unregister_dais()
>   ASoC: soc-core: add snd_soc_unregister_dai()
>   ASoC: soc-core: have legacy_dai_naming at snd_soc_register_dai()
>   ASoC: soc-core: don't call snd_soc_dapm_new_dai_widgets() at
> snd_soc_register_dai()
>   ASoC: soc-core: call snd_soc_register_dai() from
> snd_soc_register_dais()
>   ASoC: soc-core: remove topology specific operation
>   ASoC: soc.h: dobj is used only when SND_SOC_TOPOLOGY
> 
>  include/sound/soc.h      |  15 +-
>  sound/soc/soc-core.c     | 526 ++++++++++++++++++++-----------------
> ----------
>  sound/soc/soc-topology.c |  17 +-
>  3 files changed, 255 insertions(+), 303 deletions(-)
> 

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

^ permalink raw reply	[flat|nested] 60+ messages in thread

* [alsa-devel] Applied "ASoC: soc-core: call snd_soc_register_dai() from snd_soc_register_dais()" to the asoc tree
  2019-11-05  6:47 ` [alsa-devel] [PATCH v3 17/19] ASoC: soc-core: call snd_soc_register_dai() from snd_soc_register_dais() Kuninori Morimoto
@ 2019-11-05 23:51   ` Mark Brown
  0 siblings, 0 replies; 60+ messages in thread
From: Mark Brown @ 2019-11-05 23:51 UTC (permalink / raw)
  To: Kuninori Morimoto
  Cc: Linux-ALSA, Mark Brown, Pierre-Louis Bossart, Ranjani Sridharan

The patch

   ASoC: soc-core: call snd_soc_register_dai() from snd_soc_register_dais()

has been applied to the asoc tree at

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

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 71cb85f5e9da4aa3ab62020389b513275121083d Mon Sep 17 00:00:00 2001
From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Date: Tue, 5 Nov 2019 15:47:18 +0900
Subject: [PATCH] ASoC: soc-core: call snd_soc_register_dai() from
 snd_soc_register_dais()

ALSA SoC has 2 functions.
snd_soc_register_dai()  is used from topology
snd_soc_register_dais() is used from snd_soc_add_component()

In general, people think like _dai() is called from _dais()
with for loop. But in reality, these are very similar
but different implementation.
We shouldn't have duplicated and confusing implementation.

This patch calls snd_soc_register_dai() from snd_soc_register_dais()

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Link: https://lore.kernel.org/r/87r22m251l.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/soc-core.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 55b13c0037d2..86c45f751598 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -2607,12 +2607,16 @@ struct snd_soc_dai *snd_soc_register_dai(struct snd_soc_component *component,
 					 struct snd_soc_dai_driver *dai_drv,
 					 bool legacy_dai_naming)
 {
-	if (dai_drv->dobj.type != SND_SOC_DOBJ_PCM) {
-		dev_err(component->dev, "Invalid dai type %d\n",
-			dai_drv->dobj.type);
+	struct device *dev = component->dev;
+
+	if (dai_drv->dobj.type &&
+	    dai_drv->dobj.type != SND_SOC_DOBJ_PCM) {
+		dev_err(dev, "Invalid dai type %d\n", dai_drv->dobj.type);
 		return NULL;
 	}
 
+	dev_dbg(dev, "ASoC: dai register %s\n", dai_drv->name);
+
 	lockdep_assert_held(&client_mutex);
 	return soc_add_dai(component, dai_drv, legacy_dai_naming);
 }
@@ -2642,16 +2646,12 @@ static int snd_soc_register_dais(struct snd_soc_component *component,
 				 struct snd_soc_dai_driver *dai_drv,
 				 size_t count)
 {
-	struct device *dev = component->dev;
 	struct snd_soc_dai *dai;
 	unsigned int i;
 	int ret;
 
-	dev_dbg(dev, "ASoC: dai register %s #%zu\n", dev_name(dev), count);
-
 	for (i = 0; i < count; i++) {
-
-		dai = soc_add_dai(component, dai_drv + i, count == 1 &&
+		dai = snd_soc_register_dai(component, dai_drv + i, count == 1 &&
 				  !component->driver->non_legacy_dai_naming);
 		if (dai == NULL) {
 			ret = -ENOMEM;
-- 
2.20.1

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

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [alsa-devel] Applied "ASoC: soc-core: don't call snd_soc_dapm_new_dai_widgets() at snd_soc_register_dai()" to the asoc tree
  2019-11-05  6:47 ` [alsa-devel] [PATCH v3 16/19] ASoC: soc-core: don't call snd_soc_dapm_new_dai_widgets() at snd_soc_register_dai() Kuninori Morimoto
@ 2019-11-05 23:51   ` Mark Brown
  0 siblings, 0 replies; 60+ messages in thread
From: Mark Brown @ 2019-11-05 23:51 UTC (permalink / raw)
  To: Kuninori Morimoto
  Cc: Linux-ALSA, Mark Brown, Pierre-Louis Bossart, Ranjani Sridharan

The patch

   ASoC: soc-core: don't call snd_soc_dapm_new_dai_widgets() at snd_soc_register_dai()

has been applied to the asoc tree at

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

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 e443c20593de9f8efd9b2935ed40eb0bbacce30b Mon Sep 17 00:00:00 2001
From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Date: Tue, 5 Nov 2019 15:47:14 +0900
Subject: [PATCH] ASoC: soc-core: don't call snd_soc_dapm_new_dai_widgets() at
 snd_soc_register_dai()

ALSA SoC has 2 functions.
snd_soc_register_dai()  is used from topology
snd_soc_register_dais() is used from snd_soc_add_component()

In general, people think like _dai() is called from _dais()
with for loop. But in reality, these are very similar
but different implementation.
We shouldn't have duplicated and confusing implementation.

snd_soc_register_dai() is now used from topology.
But to reduce duplicated code, it should be used from _dais(), too.

Because of topology side specific reason,
it is calling snd_soc_dapm_new_dai_widgets(),
but it is not needed _dais() side.

This patch factorizes snd_soc_register_dai() to
topology / _dais() common part, and topology specific part.
And do topology specific part at soc-topology.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Link: https://lore.kernel.org/r/87sgn2251p.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 include/sound/soc.h      |  6 +++---
 sound/soc/soc-core.c     | 29 +++++------------------------
 sound/soc/soc-topology.c | 17 ++++++++++++++++-
 3 files changed, 24 insertions(+), 28 deletions(-)

diff --git a/include/sound/soc.h b/include/sound/soc.h
index 766fa81494c3..4e38ee656c4b 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -1326,9 +1326,9 @@ struct snd_soc_dai_link *snd_soc_find_dai_link(struct snd_soc_card *card,
 					       int id, const char *name,
 					       const char *stream_name);
 
-int snd_soc_register_dai(struct snd_soc_component *component,
-			 struct snd_soc_dai_driver *dai_drv,
-			 bool legacy_dai_naming);
+struct snd_soc_dai *snd_soc_register_dai(struct snd_soc_component *component,
+					 struct snd_soc_dai_driver *dai_drv,
+					 bool legacy_dai_naming);
 void snd_soc_unregister_dai(struct snd_soc_dai *dai);
 
 struct snd_soc_dai *snd_soc_find_dai(
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 6f4933f13b08..55b13c0037d2 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -2603,37 +2603,18 @@ EXPORT_SYMBOL_GPL(snd_soc_unregister_dai);
  * These DAIs's widgets will be freed in the card cleanup and the DAIs
  * will be freed in the component cleanup.
  */
-int snd_soc_register_dai(struct snd_soc_component *component,
-			 struct snd_soc_dai_driver *dai_drv,
-			 bool legacy_dai_naming)
+struct snd_soc_dai *snd_soc_register_dai(struct snd_soc_component *component,
+					 struct snd_soc_dai_driver *dai_drv,
+					 bool legacy_dai_naming)
 {
-	struct snd_soc_dapm_context *dapm =
-		snd_soc_component_get_dapm(component);
-	struct snd_soc_dai *dai;
-	int ret;
-
 	if (dai_drv->dobj.type != SND_SOC_DOBJ_PCM) {
 		dev_err(component->dev, "Invalid dai type %d\n",
 			dai_drv->dobj.type);
-		return -EINVAL;
+		return NULL;
 	}
 
 	lockdep_assert_held(&client_mutex);
-	dai = soc_add_dai(component, dai_drv, legacy_dai_naming);
-	if (!dai)
-		return -ENOMEM;
-
-	/*
-	 * Create the DAI widgets here. After adding DAIs, topology may
-	 * also add routes that need these widgets as source or sink.
-	 */
-	ret = snd_soc_dapm_new_dai_widgets(dapm, dai);
-	if (ret != 0) {
-		dev_err(component->dev,
-			"Failed to create DAI widgets %d\n", ret);
-	}
-
-	return ret;
+	return soc_add_dai(component, dai_drv, legacy_dai_naming);
 }
 EXPORT_SYMBOL_GPL(snd_soc_register_dai);
 
diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c
index b6e145627ab4..81d2af000a5c 100644
--- a/sound/soc/soc-topology.c
+++ b/sound/soc/soc-topology.c
@@ -1800,6 +1800,9 @@ static int soc_tplg_dai_create(struct soc_tplg *tplg,
 	struct snd_soc_dai_driver *dai_drv;
 	struct snd_soc_pcm_stream *stream;
 	struct snd_soc_tplg_stream_caps *caps;
+	struct snd_soc_dai *dai;
+	struct snd_soc_dapm_context *dapm =
+		snd_soc_component_get_dapm(tplg->comp);
 	int ret;
 
 	dai_drv = kzalloc(sizeof(struct snd_soc_dai_driver), GFP_KERNEL);
@@ -1842,7 +1845,19 @@ static int soc_tplg_dai_create(struct soc_tplg *tplg,
 	list_add(&dai_drv->dobj.list, &tplg->comp->dobj_list);
 
 	/* register the DAI to the component */
-	return snd_soc_register_dai(tplg->comp, dai_drv, false);
+	dai = snd_soc_register_dai(tplg->comp, dai_drv, false);
+	if (!dai)
+		return -ENOMEM;
+
+	/* Create the DAI widgets here */
+	ret = snd_soc_dapm_new_dai_widgets(dapm, dai);
+	if (ret != 0) {
+		dev_err(dai->dev, "Failed to create DAI widgets %d\n", ret);
+		snd_soc_unregister_dai(dai);
+		return ret;
+	}
+
+	return ret;
 }
 
 static void set_link_flags(struct snd_soc_dai_link *link,
-- 
2.20.1

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

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [alsa-devel] Applied "ASoC: soc-core: have legacy_dai_naming at snd_soc_register_dai()" to the asoc tree
  2019-11-05  6:47 ` [alsa-devel] [PATCH v3 15/19] ASoC: soc-core: have legacy_dai_naming at snd_soc_register_dai() Kuninori Morimoto
@ 2019-11-05 23:51   ` Mark Brown
  0 siblings, 0 replies; 60+ messages in thread
From: Mark Brown @ 2019-11-05 23:51 UTC (permalink / raw)
  To: Kuninori Morimoto
  Cc: Linux-ALSA, Mark Brown, Pierre-Louis Bossart, Ranjani Sridharan

The patch

   ASoC: soc-core: have legacy_dai_naming at snd_soc_register_dai()

has been applied to the asoc tree at

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

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 5d07519703bc2f0bf19d33652401552a480d68b8 Mon Sep 17 00:00:00 2001
From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Date: Tue, 5 Nov 2019 15:47:09 +0900
Subject: [PATCH] ASoC: soc-core: have legacy_dai_naming at
 snd_soc_register_dai()

ALSA SoC has 2 functions.
snd_soc_register_dai()  is used from topology
snd_soc_register_dais() is used from snd_soc_add_component()

In general, people think like _dai() is called from _dais()
with for loop. But in reality, these are very similar
but different implementation.
We shouldn't have duplicated and confusing implementation.

snd_soc_register_dai() is now used from topology.
But to reduce duplicated code, it should be used from _dais(), too.
To prepare it, this patch adds missing parameter legacy_dai_naming
to snd_soc_register_dai().

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Link: https://lore.kernel.org/r/87tv7i251u.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 include/sound/soc.h      | 3 ++-
 sound/soc/soc-core.c     | 5 +++--
 sound/soc/soc-topology.c | 2 +-
 3 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/include/sound/soc.h b/include/sound/soc.h
index daa0e100ecd9..766fa81494c3 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -1327,7 +1327,8 @@ struct snd_soc_dai_link *snd_soc_find_dai_link(struct snd_soc_card *card,
 					       const char *stream_name);
 
 int snd_soc_register_dai(struct snd_soc_component *component,
-	struct snd_soc_dai_driver *dai_drv);
+			 struct snd_soc_dai_driver *dai_drv,
+			 bool legacy_dai_naming);
 void snd_soc_unregister_dai(struct snd_soc_dai *dai);
 
 struct snd_soc_dai *snd_soc_find_dai(
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 38199cd7ce97..6f4933f13b08 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -2604,7 +2604,8 @@ EXPORT_SYMBOL_GPL(snd_soc_unregister_dai);
  * will be freed in the component cleanup.
  */
 int snd_soc_register_dai(struct snd_soc_component *component,
-	struct snd_soc_dai_driver *dai_drv)
+			 struct snd_soc_dai_driver *dai_drv,
+			 bool legacy_dai_naming)
 {
 	struct snd_soc_dapm_context *dapm =
 		snd_soc_component_get_dapm(component);
@@ -2618,7 +2619,7 @@ int snd_soc_register_dai(struct snd_soc_component *component,
 	}
 
 	lockdep_assert_held(&client_mutex);
-	dai = soc_add_dai(component, dai_drv, false);
+	dai = soc_add_dai(component, dai_drv, legacy_dai_naming);
 	if (!dai)
 		return -ENOMEM;
 
diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c
index 0fd032914a31..b6e145627ab4 100644
--- a/sound/soc/soc-topology.c
+++ b/sound/soc/soc-topology.c
@@ -1842,7 +1842,7 @@ static int soc_tplg_dai_create(struct soc_tplg *tplg,
 	list_add(&dai_drv->dobj.list, &tplg->comp->dobj_list);
 
 	/* register the DAI to the component */
-	return snd_soc_register_dai(tplg->comp, dai_drv);
+	return snd_soc_register_dai(tplg->comp, dai_drv, false);
 }
 
 static void set_link_flags(struct snd_soc_dai_link *link,
-- 
2.20.1

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

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [alsa-devel] Applied "ASoC: soc.h: dobj is used only when SND_SOC_TOPOLOGY" to the asoc tree
  2019-11-05  6:47 ` [alsa-devel] [PATCH v3 19/19] ASoC: soc.h: dobj is used only when SND_SOC_TOPOLOGY Kuninori Morimoto
@ 2019-11-05 23:51   ` Mark Brown
  0 siblings, 0 replies; 60+ messages in thread
From: Mark Brown @ 2019-11-05 23:51 UTC (permalink / raw)
  To: Kuninori Morimoto
  Cc: Linux-ALSA, Mark Brown, Pierre-Louis Bossart, Ranjani Sridharan

The patch

   ASoC: soc.h: dobj is used only when SND_SOC_TOPOLOGY

has been applied to the asoc tree at

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

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 509ba54fcfd1e45bceebe8ccea59dc496312f1a0 Mon Sep 17 00:00:00 2001
From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Date: Tue, 5 Nov 2019 15:47:26 +0900
Subject: [PATCH] ASoC: soc.h: dobj is used only when SND_SOC_TOPOLOGY

snd_soc_dobj is used only when SND_SOC_TOPOLOGY was selected.
Let's enable it under SND_SOC_TOPOLOGY.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Link: https://lore.kernel.org/r/87o8xq251d.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 include/sound/soc.h | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/include/sound/soc.h b/include/sound/soc.h
index 4e38ee656c4b..e0855dc08d30 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -847,7 +847,9 @@ struct snd_soc_dai_link {
 	unsigned int ignore:1;
 
 	struct list_head list; /* DAI link list of the soc card */
+#ifdef CONFIG_SND_SOC_TOPOLOGY
 	struct snd_soc_dobj dobj; /* For topology */
+#endif
 };
 #define for_each_link_codecs(link, i, codec)				\
 	for ((i) = 0;							\
@@ -1169,7 +1171,9 @@ struct soc_mixer_control {
 	unsigned int sign_bit;
 	unsigned int invert:1;
 	unsigned int autodisable:1;
+#ifdef CONFIG_SND_SOC_TOPOLOGY
 	struct snd_soc_dobj dobj;
+#endif
 };
 
 struct soc_bytes {
@@ -1180,8 +1184,9 @@ struct soc_bytes {
 
 struct soc_bytes_ext {
 	int max;
+#ifdef CONFIG_SND_SOC_TOPOLOGY
 	struct snd_soc_dobj dobj;
-
+#endif
 	/* used for TLV byte control */
 	int (*get)(struct snd_kcontrol *kcontrol, unsigned int __user *bytes,
 			unsigned int size);
@@ -1205,7 +1210,9 @@ struct soc_enum {
 	const char * const *texts;
 	const unsigned int *values;
 	unsigned int autodisable:1;
+#ifdef CONFIG_SND_SOC_TOPOLOGY
 	struct snd_soc_dobj dobj;
+#endif
 };
 
 /* device driver data */
-- 
2.20.1

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

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [alsa-devel] Applied "ASoC: soc-core: remove topology specific operation" to the asoc tree
  2019-11-05  6:47 ` [alsa-devel] [PATCH v3 18/19] ASoC: soc-core: remove topology specific operation Kuninori Morimoto
@ 2019-11-05 23:51   ` Mark Brown
  0 siblings, 0 replies; 60+ messages in thread
From: Mark Brown @ 2019-11-05 23:51 UTC (permalink / raw)
  To: Kuninori Morimoto
  Cc: Linux-ALSA, Mark Brown, Pierre-Louis Bossart, Ranjani Sridharan

The patch

   ASoC: soc-core: remove topology specific operation

has been applied to the asoc tree at

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

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 237d19080cd37e1ccf5462e63d8577d713f6da46 Mon Sep 17 00:00:00 2001
From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Date: Tue, 5 Nov 2019 15:47:22 +0900
Subject: [PATCH] ASoC: soc-core: remove topology specific operation

soc-core has some API which is used from topology, but it is doing
topology specific operation at soc-core.
soc-core should care about core things, and topology should care
about topology things, otherwise, it is very confusable.

For example topology type is not related to soc-core,
it is topology side issue.

This patch removes meaningless check from soc-core.

This patch keeps extra initialization/destruction at
snd_soc_add_dai_link() / snd_soc_remove_dai_link()
which were for topology.
From this patch, non-topology card can use it.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Link: https://lore.kernel.org/r/87pni6251h.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/soc-core.c | 28 ++++------------------------
 1 file changed, 4 insertions(+), 24 deletions(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 86c45f751598..cc596871ba7f 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -1436,19 +1436,12 @@ int snd_soc_add_dai_link(struct snd_soc_card *card,
 {
 	int ret;
 
-	if (dai_link->dobj.type
-	    && dai_link->dobj.type != SND_SOC_DOBJ_DAI_LINK) {
-		dev_err(card->dev, "Invalid dai link type %d\n",
-			dai_link->dobj.type);
-		return -EINVAL;
-	}
-
 	lockdep_assert_held(&client_mutex);
+
 	/*
 	 * Notify the machine driver for extra initialization
-	 * on the link created by topology.
 	 */
-	if (dai_link->dobj.type && card->add_dai_link)
+	if (card->add_dai_link)
 		card->add_dai_link(card, dai_link);
 
 	ret = soc_bind_dai_link(card, dai_link);
@@ -1475,19 +1468,12 @@ EXPORT_SYMBOL_GPL(snd_soc_add_dai_link);
 void snd_soc_remove_dai_link(struct snd_soc_card *card,
 			     struct snd_soc_dai_link *dai_link)
 {
-	if (dai_link->dobj.type
-	    && dai_link->dobj.type != SND_SOC_DOBJ_DAI_LINK) {
-		dev_err(card->dev, "Invalid dai link type %d\n",
-			dai_link->dobj.type);
-		return;
-	}
-
 	lockdep_assert_held(&client_mutex);
+
 	/*
 	 * Notify the machine driver for extra destruction
-	 * on the link created by topology.
 	 */
-	if (dai_link->dobj.type && card->remove_dai_link)
+	if (card->remove_dai_link)
 		card->remove_dai_link(card, dai_link);
 
 	list_del(&dai_link->list);
@@ -2609,12 +2595,6 @@ struct snd_soc_dai *snd_soc_register_dai(struct snd_soc_component *component,
 {
 	struct device *dev = component->dev;
 
-	if (dai_drv->dobj.type &&
-	    dai_drv->dobj.type != SND_SOC_DOBJ_PCM) {
-		dev_err(dev, "Invalid dai type %d\n", dai_drv->dobj.type);
-		return NULL;
-	}
-
 	dev_dbg(dev, "ASoC: dai register %s\n", dai_drv->name);
 
 	lockdep_assert_held(&client_mutex);
-- 
2.20.1

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

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [alsa-devel] Applied "ASoC: soc-core: add snd_soc_unregister_dai()" to the asoc tree
  2019-11-05  6:47 ` [alsa-devel] [PATCH v3 14/19] ASoC: soc-core: add snd_soc_unregister_dai() Kuninori Morimoto
@ 2019-11-05 23:51   ` Mark Brown
  0 siblings, 0 replies; 60+ messages in thread
From: Mark Brown @ 2019-11-05 23:51 UTC (permalink / raw)
  To: Kuninori Morimoto
  Cc: Linux-ALSA, Mark Brown, Pierre-Louis Bossart, Ranjani Sridharan

The patch

   ASoC: soc-core: add snd_soc_unregister_dai()

has been applied to the asoc tree at

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

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 e11381f38f34789b374880c4a149e25e8d7f0cfd Mon Sep 17 00:00:00 2001
From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Date: Tue, 5 Nov 2019 15:47:04 +0900
Subject: [PATCH] ASoC: soc-core: add snd_soc_unregister_dai()

It is easy to read code if it is cleanly using paired function/naming,
like start <-> stop, register <-> unregister, etc, etc.
But, current ALSA SoC code is very random, unbalance, not paired, etc.
It is easy to create bug at the such code, and is difficult to debug.

This patch adds missing soc_del_dai() and snd_soc_unregister_dai().

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Link: https://lore.kernel.org/r/87v9ry251z.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 include/sound/soc.h  |  1 +
 sound/soc/soc-core.c | 19 ++++++++++++++-----
 2 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/include/sound/soc.h b/include/sound/soc.h
index 320dcd440dab..daa0e100ecd9 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -1328,6 +1328,7 @@ struct snd_soc_dai_link *snd_soc_find_dai_link(struct snd_soc_card *card,
 
 int snd_soc_register_dai(struct snd_soc_component *component,
 	struct snd_soc_dai_driver *dai_drv);
+void snd_soc_unregister_dai(struct snd_soc_dai *dai);
 
 struct snd_soc_dai *snd_soc_find_dai(
 	const struct snd_soc_dai_link_component *dlc);
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index c803447fe639..38199cd7ce97 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -2532,6 +2532,12 @@ static inline char *fmt_multiple_name(struct device *dev,
 	return devm_kstrdup(dev, dai_drv->name, GFP_KERNEL);
 }
 
+static void soc_del_dai(struct snd_soc_dai *dai)
+{
+	dev_dbg(dai->dev, "ASoC: Unregistered DAI '%s'\n", dai->name);
+	list_del(&dai->list);
+}
+
 /* Create a DAI and add it to the component's DAI list */
 static struct snd_soc_dai *soc_add_dai(struct snd_soc_component *component,
 	struct snd_soc_dai_driver *dai_drv,
@@ -2581,6 +2587,12 @@ static struct snd_soc_dai *soc_add_dai(struct snd_soc_component *component,
 	return dai;
 }
 
+void snd_soc_unregister_dai(struct snd_soc_dai *dai)
+{
+	soc_del_dai(dai);
+}
+EXPORT_SYMBOL_GPL(snd_soc_unregister_dai);
+
 /**
  * snd_soc_register_dai - Register a DAI dynamically & create its widgets
  *
@@ -2633,11 +2645,8 @@ static void snd_soc_unregister_dais(struct snd_soc_component *component)
 {
 	struct snd_soc_dai *dai, *_dai;
 
-	for_each_component_dais_safe(component, dai, _dai) {
-		dev_dbg(component->dev, "ASoC: Unregistered DAI '%s'\n",
-			dai->name);
-		list_del(&dai->list);
-	}
+	for_each_component_dais_safe(component, dai, _dai)
+		snd_soc_unregister_dai(dai);
 }
 
 /**
-- 
2.20.1

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

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [alsa-devel] Applied "ASoC: soc-core: move snd_soc_register_dai()" to the asoc tree
  2019-11-05  6:46 ` [alsa-devel] [PATCH v3 12/19] ASoC: soc-core: move snd_soc_register_dai() Kuninori Morimoto
@ 2019-11-05 23:51   ` Mark Brown
  0 siblings, 0 replies; 60+ messages in thread
From: Mark Brown @ 2019-11-05 23:51 UTC (permalink / raw)
  To: Kuninori Morimoto
  Cc: Linux-ALSA, Mark Brown, Pierre-Louis Bossart, Ranjani Sridharan

The patch

   ASoC: soc-core: move snd_soc_register_dai()

has been applied to the asoc tree at

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

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 daf7737335bf555abf14031530fe8e47b46b373a Mon Sep 17 00:00:00 2001
From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Date: Tue, 5 Nov 2019 15:46:55 +0900
Subject: [PATCH] ASoC: soc-core: move snd_soc_register_dai()

This patch moves snd_soc_register_dai() next to
snd_soc_register_dais().
This is prepare for snd_soc_register_dais() cleanup.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Link: https://lore.kernel.org/r/87y2wu2528.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/soc-core.c | 72 ++++++++++++++++++++++----------------------
 1 file changed, 36 insertions(+), 36 deletions(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 0ce333669138..fb5f01497498 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -2597,42 +2597,6 @@ static struct snd_soc_dai *soc_add_dai(struct snd_soc_component *component,
 	return dai;
 }
 
-/**
- * snd_soc_register_dais - Register a DAI with the ASoC core
- *
- * @component: The component the DAIs are registered for
- * @dai_drv: DAI driver to use for the DAIs
- * @count: Number of DAIs
- */
-static int snd_soc_register_dais(struct snd_soc_component *component,
-				 struct snd_soc_dai_driver *dai_drv,
-				 size_t count)
-{
-	struct device *dev = component->dev;
-	struct snd_soc_dai *dai;
-	unsigned int i;
-	int ret;
-
-	dev_dbg(dev, "ASoC: dai register %s #%zu\n", dev_name(dev), count);
-
-	for (i = 0; i < count; i++) {
-
-		dai = soc_add_dai(component, dai_drv + i, count == 1 &&
-				  !component->driver->non_legacy_dai_naming);
-		if (dai == NULL) {
-			ret = -ENOMEM;
-			goto err;
-		}
-	}
-
-	return 0;
-
-err:
-	snd_soc_unregister_dais(component);
-
-	return ret;
-}
-
 /**
  * snd_soc_register_dai - Register a DAI dynamically & create its widgets
  *
@@ -2676,6 +2640,42 @@ int snd_soc_register_dai(struct snd_soc_component *component,
 }
 EXPORT_SYMBOL_GPL(snd_soc_register_dai);
 
+/**
+ * snd_soc_register_dais - Register a DAI with the ASoC core
+ *
+ * @component: The component the DAIs are registered for
+ * @dai_drv: DAI driver to use for the DAIs
+ * @count: Number of DAIs
+ */
+static int snd_soc_register_dais(struct snd_soc_component *component,
+				 struct snd_soc_dai_driver *dai_drv,
+				 size_t count)
+{
+	struct device *dev = component->dev;
+	struct snd_soc_dai *dai;
+	unsigned int i;
+	int ret;
+
+	dev_dbg(dev, "ASoC: dai register %s #%zu\n", dev_name(dev), count);
+
+	for (i = 0; i < count; i++) {
+
+		dai = soc_add_dai(component, dai_drv + i, count == 1 &&
+				  !component->driver->non_legacy_dai_naming);
+		if (dai == NULL) {
+			ret = -ENOMEM;
+			goto err;
+		}
+	}
+
+	return 0;
+
+err:
+	snd_soc_unregister_dais(component);
+
+	return ret;
+}
+
 static int snd_soc_component_initialize(struct snd_soc_component *component,
 	const struct snd_soc_component_driver *driver, struct device *dev)
 {
-- 
2.20.1

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

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [alsa-devel] Applied "ASoC: soc-core: move snd_soc_unregister_dais()" to the asoc tree
  2019-11-05  6:47 ` [alsa-devel] [PATCH v3 13/19] ASoC: soc-core: move snd_soc_unregister_dais() Kuninori Morimoto
@ 2019-11-05 23:51   ` Mark Brown
  0 siblings, 0 replies; 60+ messages in thread
From: Mark Brown @ 2019-11-05 23:51 UTC (permalink / raw)
  To: Kuninori Morimoto
  Cc: Linux-ALSA, Mark Brown, Pierre-Louis Bossart, Ranjani Sridharan

The patch

   ASoC: soc-core: move snd_soc_unregister_dais()

has been applied to the asoc tree at

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

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 3f6674ae13a1e498f249b0255659bfc4f692a7e0 Mon Sep 17 00:00:00 2001
From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Date: Tue, 5 Nov 2019 15:47:00 +0900
Subject: [PATCH] ASoC: soc-core: move snd_soc_unregister_dais()

This patch moves snd_soc_unregister_dais() next to
snd_soc_register_dais().
This is prepare for snd_soc_register_dais() cleanup

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Link: https://lore.kernel.org/r/87woce2524.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/soc-core.c | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index fb5f01497498..c803447fe639 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -2532,22 +2532,6 @@ static inline char *fmt_multiple_name(struct device *dev,
 	return devm_kstrdup(dev, dai_drv->name, GFP_KERNEL);
 }
 
-/**
- * snd_soc_unregister_dai - Unregister DAIs from the ASoC core
- *
- * @component: The component for which the DAIs should be unregistered
- */
-static void snd_soc_unregister_dais(struct snd_soc_component *component)
-{
-	struct snd_soc_dai *dai, *_dai;
-
-	for_each_component_dais_safe(component, dai, _dai) {
-		dev_dbg(component->dev, "ASoC: Unregistered DAI '%s'\n",
-			dai->name);
-		list_del(&dai->list);
-	}
-}
-
 /* Create a DAI and add it to the component's DAI list */
 static struct snd_soc_dai *soc_add_dai(struct snd_soc_component *component,
 	struct snd_soc_dai_driver *dai_drv,
@@ -2640,6 +2624,22 @@ int snd_soc_register_dai(struct snd_soc_component *component,
 }
 EXPORT_SYMBOL_GPL(snd_soc_register_dai);
 
+/**
+ * snd_soc_unregister_dai - Unregister DAIs from the ASoC core
+ *
+ * @component: The component for which the DAIs should be unregistered
+ */
+static void snd_soc_unregister_dais(struct snd_soc_component *component)
+{
+	struct snd_soc_dai *dai, *_dai;
+
+	for_each_component_dais_safe(component, dai, _dai) {
+		dev_dbg(component->dev, "ASoC: Unregistered DAI '%s'\n",
+			dai->name);
+		list_del(&dai->list);
+	}
+}
+
 /**
  * snd_soc_register_dais - Register a DAI with the ASoC core
  *
-- 
2.20.1

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

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [alsa-devel] Applied "ASoC: soc-core: use snd_soc_lookup_component() at snd_soc_unregister_component()" to the asoc tree
  2019-11-05  6:46 ` [alsa-devel] [PATCH v3 11/19] ASoC: soc-core: use snd_soc_lookup_component() at snd_soc_unregister_component() Kuninori Morimoto
@ 2019-11-05 23:51   ` Mark Brown
  0 siblings, 0 replies; 60+ messages in thread
From: Mark Brown @ 2019-11-05 23:51 UTC (permalink / raw)
  To: Kuninori Morimoto
  Cc: Linux-ALSA, Mark Brown, Pierre-Louis Bossart, Ranjani Sridharan

The patch

   ASoC: soc-core: use snd_soc_lookup_component() at snd_soc_unregister_component()

has been applied to the asoc tree at

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

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 ac6a4dd3e9f09697ab6a1774d7ab6a34e7ab36fa Mon Sep 17 00:00:00 2001
From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Date: Tue, 5 Nov 2019 15:46:51 +0900
Subject: [PATCH] ASoC: soc-core: use snd_soc_lookup_component() at
 snd_soc_unregister_component()

snd_soc_unregister_component() is now finding component manually,
but we already have snd_soc_lookup_component() to find component;
Let's use existing function.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Link: https://lore.kernel.org/r/87zhha252c.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/soc-core.c | 20 +++++---------------
 1 file changed, 5 insertions(+), 15 deletions(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index bb0592159414..0ce333669138 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -2876,29 +2876,19 @@ EXPORT_SYMBOL_GPL(snd_soc_register_component);
  *
  * @dev: The device to unregister
  */
-static int __snd_soc_unregister_component(struct device *dev)
+void snd_soc_unregister_component(struct device *dev)
 {
 	struct snd_soc_component *component;
-	int found = 0;
 
 	mutex_lock(&client_mutex);
-	for_each_component(component) {
-		if (dev != component->dev)
-			continue;
+	while (1) {
+		component = snd_soc_lookup_component(dev, NULL);
+		if (!component)
+			break;
 
 		snd_soc_del_component_unlocked(component);
-		found = 1;
-		break;
 	}
 	mutex_unlock(&client_mutex);
-
-	return found;
-}
-
-void snd_soc_unregister_component(struct device *dev)
-{
-	while (__snd_soc_unregister_component(dev))
-		;
 }
 EXPORT_SYMBOL_GPL(snd_soc_unregister_component);
 
-- 
2.20.1

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

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [alsa-devel] Applied "ASoC: soc-core: remove snd_soc_component_add/del()" to the asoc tree
  2019-11-05  6:46 ` [alsa-devel] [PATCH v3 10/19] ASoC: soc-core: remove snd_soc_component_add/del() Kuninori Morimoto
@ 2019-11-05 23:51   ` Mark Brown
  0 siblings, 0 replies; 60+ messages in thread
From: Mark Brown @ 2019-11-05 23:51 UTC (permalink / raw)
  To: Kuninori Morimoto
  Cc: Linux-ALSA, Mark Brown, Pierre-Louis Bossart, Ranjani Sridharan

The patch

   ASoC: soc-core: remove snd_soc_component_add/del()

has been applied to the asoc tree at

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

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 b18768f56162964f70bbb9119dba59a947d7d577 Mon Sep 17 00:00:00 2001
From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Date: Tue, 5 Nov 2019 15:46:45 +0900
Subject: [PATCH] ASoC: soc-core: remove snd_soc_component_add/del()

soc-core has
snd_soc_add_component(), snd_soc_component_add(),
snd_soc_del_component(), snd_soc_component_del().

These are very confusing naming.
snd_soc_component_xxx() are called from snd_soc_xxx_component(),
and these are very small.
Let's merge these into snd_soc_xxx_component(), and
remove snd_soc_component_xxx().

Reported-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Link: https://lore.kernel.org/r/871rum3jmy.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/soc-core.c | 58 +++++++++++++++++++-------------------------
 1 file changed, 25 insertions(+), 33 deletions(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index e91325b688f2..bb0592159414 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -2746,34 +2746,6 @@ EXPORT_SYMBOL_GPL(snd_soc_component_exit_regmap);
 
 #endif
 
-static void snd_soc_component_add(struct snd_soc_component *component)
-{
-	mutex_lock(&client_mutex);
-
-	if (!component->driver->write && !component->driver->read) {
-		if (!component->regmap)
-			component->regmap = dev_get_regmap(component->dev,
-							   NULL);
-		if (component->regmap)
-			snd_soc_component_setup_regmap(component);
-	}
-
-	/* see for_each_component */
-	list_add(&component->list, &component_list);
-
-	mutex_unlock(&client_mutex);
-}
-
-static void snd_soc_component_del(struct snd_soc_component *component)
-{
-	struct snd_soc_card *card = component->card;
-
-	if (card)
-		snd_soc_unbind_card(card, false);
-
-	list_del(&component->list);
-}
-
 #define ENDIANNESS_MAP(name) \
 	(SNDRV_PCM_FMTBIT_##name##LE | SNDRV_PCM_FMTBIT_##name##BE)
 static u64 endianness_format_map[] = {
@@ -2820,8 +2792,14 @@ static void snd_soc_try_rebind_card(void)
 
 static void snd_soc_del_component_unlocked(struct snd_soc_component *component)
 {
+	struct snd_soc_card *card = component->card;
+
 	snd_soc_unregister_dais(component);
-	snd_soc_component_del(component);
+
+	if (card)
+		snd_soc_unbind_card(card, false);
+
+	list_del(&component->list);
 }
 
 int snd_soc_add_component(struct device *dev,
@@ -2833,6 +2811,8 @@ int snd_soc_add_component(struct device *dev,
 	int ret;
 	int i;
 
+	mutex_lock(&client_mutex);
+
 	ret = snd_soc_component_initialize(component, component_driver, dev);
 	if (ret)
 		goto err_free;
@@ -2850,14 +2830,26 @@ int snd_soc_add_component(struct device *dev,
 		goto err_cleanup;
 	}
 
-	snd_soc_component_add(component);
-	snd_soc_try_rebind_card();
+	if (!component->driver->write && !component->driver->read) {
+		if (!component->regmap)
+			component->regmap = dev_get_regmap(component->dev,
+							   NULL);
+		if (component->regmap)
+			snd_soc_component_setup_regmap(component);
+	}
 
-	return 0;
+	/* see for_each_component */
+	list_add(&component->list, &component_list);
 
 err_cleanup:
-	snd_soc_del_component_unlocked(component);
+	if (ret < 0)
+		snd_soc_del_component_unlocked(component);
 err_free:
+	mutex_unlock(&client_mutex);
+
+	if (ret == 0)
+		snd_soc_try_rebind_card();
+
 	return ret;
 }
 EXPORT_SYMBOL_GPL(snd_soc_add_component);
-- 
2.20.1

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

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [alsa-devel] Applied "ASoC: soc-core: tidyup snd_soc_lookup_component()" to the asoc tree
  2019-11-05  6:46 ` [alsa-devel] [PATCH v3 08/19] ASoC: soc-core: tidyup snd_soc_lookup_component() Kuninori Morimoto
@ 2019-11-05 23:51   ` Mark Brown
  0 siblings, 0 replies; 60+ messages in thread
From: Mark Brown @ 2019-11-05 23:51 UTC (permalink / raw)
  To: Kuninori Morimoto
  Cc: Linux-ALSA, Mark Brown, Pierre-Louis Bossart, Ranjani Sridharan

The patch

   ASoC: soc-core: tidyup snd_soc_lookup_component()

has been applied to the asoc tree at

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

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 5bd7e08b3c5f3924259643e1f413e10ca6c97634 Mon Sep 17 00:00:00 2001
From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Date: Tue, 5 Nov 2019 15:46:35 +0900
Subject: [PATCH] ASoC: soc-core: tidyup snd_soc_lookup_component()

snd_soc_lookup_component() is using mix of continue and break
in the same loop. It is odd.
This patch cleanup it.

Reported-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Link: https://lore.kernel.org/r/874kzi3jn8.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/soc-core.c | 23 ++++++++++-------------
 1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index b71bddb30db1..acbaed4e4e9d 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -360,25 +360,22 @@ struct snd_soc_component *snd_soc_lookup_component(struct device *dev,
 						   const char *driver_name)
 {
 	struct snd_soc_component *component;
-	struct snd_soc_component *ret;
+	struct snd_soc_component *found_component;
 
-	ret = NULL;
+	found_component = NULL;
 	mutex_lock(&client_mutex);
 	for_each_component(component) {
-		if (dev != component->dev)
-			continue;
-
-		if (driver_name &&
-		    (driver_name != component->driver->name) &&
-		    (strcmp(component->driver->name, driver_name) != 0))
-			continue;
-
-		ret = component;
-		break;
+		if ((dev == component->dev) &&
+		    (!driver_name ||
+		     (driver_name == component->driver->name) ||
+		     (strcmp(component->driver->name, driver_name) == 0))) {
+			found_component = component;
+			break;
+		}
 	}
 	mutex_unlock(&client_mutex);
 
-	return ret;
+	return found_component;
 }
 EXPORT_SYMBOL_GPL(snd_soc_lookup_component);
 
-- 
2.20.1

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

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [alsa-devel] Applied "ASoC: soc-core: add snd_soc_del_component_unlocked()" to the asoc tree
  2019-11-05  6:46 ` [alsa-devel] [PATCH v3 09/19] ASoC: soc-core: add snd_soc_del_component_unlocked() Kuninori Morimoto
@ 2019-11-05 23:51   ` Mark Brown
  0 siblings, 0 replies; 60+ messages in thread
From: Mark Brown @ 2019-11-05 23:51 UTC (permalink / raw)
  To: Kuninori Morimoto
  Cc: Linux-ALSA, Mark Brown, Pierre-Louis Bossart, Ranjani Sridharan

The patch

   ASoC: soc-core: add snd_soc_del_component_unlocked()

has been applied to the asoc tree at

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

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 486c7978ff665eb763f70cc9477e0de6326e1c41 Mon Sep 17 00:00:00 2001
From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Date: Tue, 5 Nov 2019 15:46:39 +0900
Subject: [PATCH] ASoC: soc-core: add snd_soc_del_component_unlocked()

It is easy to read code if it is cleanly using paired function/naming,
like start <-> stop, register <-> unregister, etc, etc.
But, current ALSA SoC code is very random, unbalance, not paired, etc.
It is easy to create bug at the such code, and is difficult to debug.

Now ALSA SoC has snd_soc_add_component(), but there is no paired
snd_soc_del_component(). Thus, snd_soc_unregister_component() is
calling cleanup function randomly. it is difficult to read.
This patch adds missing snd_soc_del_component_unlocked() and
balance up code.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Link: https://lore.kernel.org/r/8736f23jn4.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/soc-core.c | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index acbaed4e4e9d..e91325b688f2 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -2764,12 +2764,7 @@ static void snd_soc_component_add(struct snd_soc_component *component)
 	mutex_unlock(&client_mutex);
 }
 
-static void snd_soc_component_cleanup(struct snd_soc_component *component)
-{
-	snd_soc_unregister_dais(component);
-}
-
-static void snd_soc_component_del_unlocked(struct snd_soc_component *component)
+static void snd_soc_component_del(struct snd_soc_component *component)
 {
 	struct snd_soc_card *card = component->card;
 
@@ -2823,6 +2818,12 @@ static void snd_soc_try_rebind_card(void)
 			list_del(&card->list);
 }
 
+static void snd_soc_del_component_unlocked(struct snd_soc_component *component)
+{
+	snd_soc_unregister_dais(component);
+	snd_soc_component_del(component);
+}
+
 int snd_soc_add_component(struct device *dev,
 			struct snd_soc_component *component,
 			const struct snd_soc_component_driver *component_driver,
@@ -2855,7 +2856,7 @@ int snd_soc_add_component(struct device *dev,
 	return 0;
 
 err_cleanup:
-	snd_soc_component_cleanup(component);
+	snd_soc_del_component_unlocked(component);
 err_free:
 	return ret;
 }
@@ -2893,15 +2894,12 @@ static int __snd_soc_unregister_component(struct device *dev)
 		if (dev != component->dev)
 			continue;
 
-		snd_soc_component_del_unlocked(component);
+		snd_soc_del_component_unlocked(component);
 		found = 1;
 		break;
 	}
 	mutex_unlock(&client_mutex);
 
-	if (found)
-		snd_soc_component_cleanup(component);
-
 	return found;
 }
 
-- 
2.20.1

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

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [alsa-devel] Applied "ASoC: soc-core: move snd_soc_lookup_component()" to the asoc tree
  2019-11-05  6:46 ` [alsa-devel] [PATCH v3 07/19] ASoC: soc-core: move snd_soc_lookup_component() Kuninori Morimoto
@ 2019-11-05 23:51   ` Mark Brown
  0 siblings, 0 replies; 60+ messages in thread
From: Mark Brown @ 2019-11-05 23:51 UTC (permalink / raw)
  To: Kuninori Morimoto
  Cc: Linux-ALSA, Mark Brown, Pierre-Louis Bossart, Ranjani Sridharan

The patch

   ASoC: soc-core: move snd_soc_lookup_component()

has been applied to the asoc tree at

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

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 b8132657990b5a09ad8e1c9e2c8efc20b5f9372a Mon Sep 17 00:00:00 2001
From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Date: Tue, 5 Nov 2019 15:46:30 +0900
Subject: [PATCH] ASoC: soc-core: move snd_soc_lookup_component()

This patch moves snd_soc_lookup_component() to upper side.
This is prepare for snd_soc_unregister_component()

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Link: https://lore.kernel.org/r/875zjy3jnd.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/soc-core.c | 52 ++++++++++++++++++++++----------------------
 1 file changed, 26 insertions(+), 26 deletions(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 1e8dd19cba24..b71bddb30db1 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -356,6 +356,32 @@ struct snd_soc_component *snd_soc_rtdcom_lookup(struct snd_soc_pcm_runtime *rtd,
 }
 EXPORT_SYMBOL_GPL(snd_soc_rtdcom_lookup);
 
+struct snd_soc_component *snd_soc_lookup_component(struct device *dev,
+						   const char *driver_name)
+{
+	struct snd_soc_component *component;
+	struct snd_soc_component *ret;
+
+	ret = NULL;
+	mutex_lock(&client_mutex);
+	for_each_component(component) {
+		if (dev != component->dev)
+			continue;
+
+		if (driver_name &&
+		    (driver_name != component->driver->name) &&
+		    (strcmp(component->driver->name, driver_name) != 0))
+			continue;
+
+		ret = component;
+		break;
+	}
+	mutex_unlock(&client_mutex);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(snd_soc_lookup_component);
+
 struct snd_pcm_substream *snd_soc_get_dai_substream(struct snd_soc_card *card,
 		const char *dai_link, int stream)
 {
@@ -2889,32 +2915,6 @@ void snd_soc_unregister_component(struct device *dev)
 }
 EXPORT_SYMBOL_GPL(snd_soc_unregister_component);
 
-struct snd_soc_component *snd_soc_lookup_component(struct device *dev,
-						   const char *driver_name)
-{
-	struct snd_soc_component *component;
-	struct snd_soc_component *ret;
-
-	ret = NULL;
-	mutex_lock(&client_mutex);
-	for_each_component(component) {
-		if (dev != component->dev)
-			continue;
-
-		if (driver_name &&
-		    (driver_name != component->driver->name) &&
-		    (strcmp(component->driver->name, driver_name) != 0))
-			continue;
-
-		ret = component;
-		break;
-	}
-	mutex_unlock(&client_mutex);
-
-	return ret;
-}
-EXPORT_SYMBOL_GPL(snd_soc_lookup_component);
-
 /* Retrieve a card's name from device tree */
 int snd_soc_of_parse_card_name(struct snd_soc_card *card,
 			       const char *propname)
-- 
2.20.1

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

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [alsa-devel] Applied "ASoC: soc-core: add soc_unbind_dai_link()" to the asoc tree
  2019-11-05  6:46 ` [alsa-devel] [PATCH v3 06/19] ASoC: soc-core: add soc_unbind_dai_link() Kuninori Morimoto
@ 2019-11-05 23:51   ` Mark Brown
  2019-11-12  5:21   ` [alsa-devel] [PATCH v3 06/19] ASoC: soc-core: add soc_unbind_dai_link() Pierre-Louis Bossart
  1 sibling, 0 replies; 60+ messages in thread
From: Mark Brown @ 2019-11-05 23:51 UTC (permalink / raw)
  To: Kuninori Morimoto
  Cc: Linux-ALSA, Mark Brown, Pierre-Louis Bossart, Ranjani Sridharan

The patch

   ASoC: soc-core: add soc_unbind_dai_link()

has been applied to the asoc tree at

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

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 bc7a9091e5b927ecc20dbb3bc07a5a09783fc27b Mon Sep 17 00:00:00 2001
From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Date: Tue, 5 Nov 2019 15:46:25 +0900
Subject: [PATCH] ASoC: soc-core: add soc_unbind_dai_link()

It is easy to read code if it is cleanly using paired function/naming,
like start <-> stop, register <-> unregister, etc, etc.
But, current ALSA SoC code is very random, unbalance, not paired, etc.
It is easy to create bug at the such code, and it will be difficult to
debug.

ALSA SoC has soc_bind_dai_link(), but its paired soc_unbind_dai_link()
is not implemented.
More confusable is that soc_remove_pcm_runtimes() which should be
soc_unbind_dai_link() is implemented without synchronised
to soc_bind_dai_link().

This patch cleanup this unbalance.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Link: https://lore.kernel.org/r/877e4e3jni.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/soc-core.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index e8ff6f2f58ba..1e8dd19cba24 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -470,14 +470,6 @@ static struct snd_soc_pcm_runtime *soc_new_pcm_runtime(
 	return NULL;
 }
 
-static void soc_remove_pcm_runtimes(struct snd_soc_card *card)
-{
-	struct snd_soc_pcm_runtime *rtd, *_rtd;
-
-	for_each_card_rtds_safe(card, rtd, _rtd)
-		soc_free_pcm_runtime(rtd);
-}
-
 struct snd_soc_pcm_runtime *snd_soc_get_pcm_runtime(struct snd_soc_card *card,
 		const char *dai_link)
 {
@@ -1037,6 +1029,16 @@ static int soc_dai_link_sanity_check(struct snd_soc_card *card,
 	return 0;
 }
 
+static void soc_unbind_dai_link(struct snd_soc_card *card,
+				struct snd_soc_dai_link *dai_link)
+{
+	struct snd_soc_pcm_runtime *rtd;
+
+	rtd = snd_soc_get_pcm_runtime(card, dai_link->name);
+	if (rtd)
+		soc_free_pcm_runtime(rtd);
+}
+
 static int soc_bind_dai_link(struct snd_soc_card *card,
 	struct snd_soc_dai_link *dai_link)
 {
@@ -1466,6 +1468,8 @@ void snd_soc_remove_dai_link(struct snd_soc_card *card,
 		card->remove_dai_link(card, dai_link);
 
 	list_del(&dai_link->list);
+
+	soc_unbind_dai_link(card, dai_link);
 }
 EXPORT_SYMBOL_GPL(snd_soc_remove_dai_link);
 
@@ -1974,8 +1978,6 @@ static void soc_cleanup_card_resources(struct snd_soc_card *card)
 	for_each_card_links_safe(card, link, _link)
 		snd_soc_remove_dai_link(card, link);
 
-	soc_remove_pcm_runtimes(card);
-
 	/* remove auxiliary devices */
 	soc_remove_aux_devices(card);
 	soc_unbind_aux_dev(card);
-- 
2.20.1

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

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [alsa-devel] Applied "ASoC: soc-core: call soc_bind_dai_link() under snd_soc_add_dai_link()" to the asoc tree
  2019-11-05  6:46 ` [alsa-devel] [PATCH v3 05/19] ASoC: soc-core: call soc_bind_dai_link() under snd_soc_add_dai_link() Kuninori Morimoto
@ 2019-11-05 23:51   ` Mark Brown
  0 siblings, 0 replies; 60+ messages in thread
From: Mark Brown @ 2019-11-05 23:51 UTC (permalink / raw)
  To: Kuninori Morimoto
  Cc: Linux-ALSA, Mark Brown, Pierre-Louis Bossart, Ranjani Sridharan

The patch

   ASoC: soc-core: call soc_bind_dai_link() under 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.5

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 6b1dff0266a30df16846a20d1109ab25b985f0d7 Mon Sep 17 00:00:00 2001
From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Date: Tue, 5 Nov 2019 15:46:20 +0900
Subject: [PATCH] ASoC: soc-core: call soc_bind_dai_link() under
 snd_soc_add_dai_link()

If we focus to soc_bind_dai_link() at snd_soc_instantiate_card(),
we will notice very complex operation.

static int snd_soc_instantiate_card(...)
{
	...
	/*
	 * (1) Bind dai_link via card pre-linked dai_link
	 *
	 * Bind dai_link via card pre-linked.
	 * 1 dai_link will be 1 rtd, and connected to card.
	 * for_each_card_prelinks() is for card pre-linked dai_link.
	 *
	 * Image
	 *
	 * card
	 * - rtd(A)
	 * - rtd(A)
	 */
	for_each_card_prelinks(card, i, dai_link) {
		ret = soc_bind_dai_link(card, dai_link);
		...
	}
	...
	/*
	 * (2) Connect card pre-linked dai_link to card list
	 *
	 * Connect all card pre-linked dai_link to *card list*.
	 * Here, (A) means from card pre-linked.
	 *
	 * Image
	 *
	 * card		card list
	 *  - rtd(A)	 - dai_link(A)
	 *  - rtd(A)	 - dai_link(A)
	 *  - ...	 - ...
	 */
	for_each_card_prelinks(card, i, dai_link) {
		ret = snd_soc_add_dai_link(card, dai_link);
		...
	}
	...
	/*
	 * (3) Probe binded component
	 *
	 * Each rtd has many components.
	 * Here probes each rtd connected components.
	 * rtd(A) in Image is the probe target.
	 *
	 * During this component probe, topology may add new dai_link to
	 * *card list* by using snd_soc_add_dai_link() which is
	 * used at (2).
	 * Here, (B) means from topology
	 *
	 * Image
	 *
	 * card		card list
	 *  - rtd(A)	 - dai_link(A)
	 *  - rtd(A)	 - dai_link(A)
	 *  - ...	 - ...
	 *		 - dai_link(B)
	 *		 - dai_link(B)
	 */
	ret = soc_probe_link_components(card);
	...

	/*
	 * (4) Bind dai_link again
	 *
	 * Bind dai_link again for topology.
	 * Note, (1) used for_each_card_prelinks(),
	 * here is using  for_each_card_links()
	 *
	 * This means from card list.
	 * As Image indicating, it has dai_link(A) (from card pre-link)
	 * and dai_link(B) (from topology).
	 * main target here is dai_link(B).
	 * soc_bind_dai_link() ignores already used
	 * dai_link (= dai_link(A))
	 *
	 * Image
	 *
	 * card		card list
	 *  - rtd(A)	 - dai_link(A)
	 *  - rtd(A)	 - dai_link(A)
	 *  - ...	 - ...
	 *  - rtd(B)	 - dai_link(B)
	 *  - rtd(B)	 - dai_link(B)
	 */
	for_each_card_links(card, dai_link) {
		ret = soc_bind_dai_link(card, dai_link);
		...
	}
	...
}

As you see above, it is doing very complex method.
The problem is binding dai_link via "card pre-linked" (= (1)) and
"topology added dai_link" (= (3)) are separated.
The code can be simple if we can bind dai_link when dai_link
is connected to *card list*.
This patch do it.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Link: https://lore.kernel.org/r/878sou3jnn.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/soc-core.c | 23 ++++++-----------------
 1 file changed, 6 insertions(+), 17 deletions(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 3cc36c2d99da..e8ff6f2f58ba 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -1409,6 +1409,8 @@ EXPORT_SYMBOL_GPL(snd_soc_disconnect_sync);
 int snd_soc_add_dai_link(struct snd_soc_card *card,
 		struct snd_soc_dai_link *dai_link)
 {
+	int ret;
+
 	if (dai_link->dobj.type
 	    && dai_link->dobj.type != SND_SOC_DOBJ_DAI_LINK) {
 		dev_err(card->dev, "Invalid dai link type %d\n",
@@ -1424,6 +1426,10 @@ 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);
 
+	ret = soc_bind_dai_link(card, dai_link);
+	if (ret < 0)
+		return ret;
+
 	/* see for_each_card_links */
 	list_add_tail(&dai_link->list, &card->dai_link_list);
 
@@ -1996,13 +2002,6 @@ static int snd_soc_instantiate_card(struct snd_soc_card *card)
 	/* check whether any platform is ignore machine FE and using topology */
 	soc_check_tplg_fes(card);
 
-	/* bind DAIs */
-	for_each_card_prelinks(card, i, dai_link) {
-		ret = soc_bind_dai_link(card, dai_link);
-		if (ret != 0)
-			goto probe_end;
-	}
-
 	/* bind aux_devs too */
 	ret = soc_bind_aux_dev(card);
 	if (ret < 0)
@@ -2060,16 +2059,6 @@ static int snd_soc_instantiate_card(struct snd_soc_card *card)
 	if (ret < 0)
 		goto probe_end;
 
-	/*
-	 * Find new DAI links added during probing components and bind them.
-	 * Components with topology may bring new DAIs and DAI links.
-	 */
-	for_each_card_links(card, dai_link) {
-		ret = soc_bind_dai_link(card, dai_link);
-		if (ret)
-			goto probe_end;
-	}
-
 	/* probe all DAI links on this card */
 	ret = soc_probe_link_dais(card);
 	if (ret < 0) {
-- 
2.20.1

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

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [alsa-devel] Applied "ASoC: soc-core: remove duplicated soc_is_dai_link_bound()" to the asoc tree
  2019-11-05  6:46 ` [alsa-devel] [PATCH v3 04/19] ASoC: soc-core: remove duplicated soc_is_dai_link_bound() Kuninori Morimoto
@ 2019-11-05 23:51   ` Mark Brown
  0 siblings, 0 replies; 60+ messages in thread
From: Mark Brown @ 2019-11-05 23:51 UTC (permalink / raw)
  To: Kuninori Morimoto
  Cc: Linux-ALSA, Mark Brown, Pierre-Louis Bossart, Ranjani Sridharan

The patch

   ASoC: soc-core: remove duplicated soc_is_dai_link_bound()

has been applied to the asoc tree at

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

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 95b562e57f0b3a21a3945297862cb51bc2072c7b Mon Sep 17 00:00:00 2001
From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Date: Tue, 5 Nov 2019 15:46:15 +0900
Subject: [PATCH] ASoC: soc-core: remove duplicated soc_is_dai_link_bound()

soc_is_dai_link_bound() check will be called both
*before* soc_bind_dai_link() (A), and
*under*  soc_bind_dai_link() (B).
These are very verbose code. Let's remove one of them.

*	static int soc_bind_dai_link(...)
	{
		...
(B)		if (soc_is_dai_link_bound(...)) {
			...
			return 0;
		}
		...
	}

	static int snd_soc_instantiate_card(...)
	{
		...
		for_each_card_links(...) {
(A)			if (soc_is_dai_link_bound(...))
				continue;

*			ret = soc_bind_dai_link(...);
			if (ret)
				goto probe_end;
		}
		...
	}

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Link: https://lore.kernel.org/r/87a79a3jns.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/soc-core.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index e1b0d861807c..3cc36c2d99da 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -2065,9 +2065,6 @@ static int snd_soc_instantiate_card(struct snd_soc_card *card)
 	 * Components with topology may bring new DAIs and DAI links.
 	 */
 	for_each_card_links(card, dai_link) {
-		if (soc_is_dai_link_bound(card, dai_link))
-			continue;
-
 		ret = soc_bind_dai_link(card, dai_link);
 		if (ret)
 			goto probe_end;
-- 
2.20.1

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

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [alsa-devel] Applied "ASoC: soc-core: typo fix at soc_dai_link_sanity_check()" to the asoc tree
  2019-11-05  6:46 ` [alsa-devel] [PATCH v3 03/19] ASoC: soc-core: typo fix at soc_dai_link_sanity_check() Kuninori Morimoto
@ 2019-11-05 23:51   ` Mark Brown
  0 siblings, 0 replies; 60+ messages in thread
From: Mark Brown @ 2019-11-05 23:51 UTC (permalink / raw)
  To: Kuninori Morimoto
  Cc: Linux-ALSA, Mark Brown, Pierre-Louis Bossart, Ranjani Sridharan

The patch

   ASoC: soc-core: typo fix at soc_dai_link_sanity_check()

has been applied to the asoc tree at

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

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 cd3c5ad7b2503f4fb4dfcc095b3fccc2b3603c36 Mon Sep 17 00:00:00 2001
From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Date: Tue, 5 Nov 2019 15:46:00 +0900
Subject: [PATCH] ASoC: soc-core: typo fix at soc_dai_link_sanity_check()

Reported-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Link: https://lore.kernel.org/r/87bltq3jo7.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/soc-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 827625bd35cd..e1b0d861807c 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -1015,7 +1015,7 @@ static int soc_dai_link_sanity_check(struct snd_soc_card *card,
 	}
 
 	/*
-	 * Defer card registartion if cpu dai component is not added to
+	 * Defer card registration if cpu dai component is not added to
 	 * component list.
 	 */
 	if ((link->cpus->of_node || link->cpus->name) &&
-- 
2.20.1

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

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [alsa-devel] Applied "ASoC: soc-core: tidyup soc_init_dai_link()" to the asoc tree
  2019-11-05  6:45 ` [alsa-devel] [PATCH v3 02/19] ASoC: soc-core: tidyup soc_init_dai_link() Kuninori Morimoto
@ 2019-11-05 23:51   ` Mark Brown
  2019-11-11 14:43   ` [alsa-devel] [PATCH v3 02/19] ASoC: soc-core: tidyup soc_init_dai_link() Jon Hunter
  1 sibling, 0 replies; 60+ messages in thread
From: Mark Brown @ 2019-11-05 23:51 UTC (permalink / raw)
  To: Kuninori Morimoto
  Cc: Linux-ALSA, Mark Brown, Pierre-Louis Bossart, Ranjani Sridharan

The patch

   ASoC: soc-core: tidyup soc_init_dai_link()

has been applied to the asoc tree at

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

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 bfce78a559655c5c4512a898a7e5d3a796fbb473 Mon Sep 17 00:00:00 2001
From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Date: Tue, 5 Nov 2019 15:45:50 +0900
Subject: [PATCH] ASoC: soc-core: tidyup soc_init_dai_link()

soc_init_dai_link() is needed to be called before soc_bind_dai_link().

	int snd_soc_instantiate_card()
	{
		for_each_card_prelinks(...) {
(1)			ret = soc_init_dai_link(...);
			...
		}
		...
		for_each_card_prelinks(...) {
(2)			ret = soc_bind_dai_link(...);
			...
		}
		...
		for_each_card_links(...) {
			...
(A)			ret = soc_init_dai_link(...);
			...
(B)			ret = soc_bind_dai_link(...);
		}
		...

(1) is for (2), and (A) is for (B)
(1) and (2) are for card prelink   dai_link.
(A) and (B) are for topology added dai_link.

soc_init_dai_link() is sanity check for dai_link, not initializing today.
Therefore, it is confusable naming. We can rename it as sanity_check.

And this check is for soc_bind_dai_link().
It can be more simple code if we can call it from soc_bind_dai_link().

This patch renames it to soc_dai_link_sanity_check(), and
call it from soc_bind_dai_link().

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Link: https://lore.kernel.org/r/87d0e63joh.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/soc-core.c | 22 +++++++---------------
 1 file changed, 7 insertions(+), 15 deletions(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index a141828f8638..827625bd35cd 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -941,8 +941,8 @@ static bool soc_is_dai_link_bound(struct snd_soc_card *card,
 	return false;
 }
 
-static int soc_init_dai_link(struct snd_soc_card *card,
-			     struct snd_soc_dai_link *link)
+static int soc_dai_link_sanity_check(struct snd_soc_card *card,
+				     struct snd_soc_dai_link *link)
 {
 	int i;
 	struct snd_soc_dai_link_component *codec, *platform;
@@ -1043,7 +1043,7 @@ static int soc_bind_dai_link(struct snd_soc_card *card,
 	struct snd_soc_pcm_runtime *rtd;
 	struct snd_soc_dai_link_component *codec, *platform;
 	struct snd_soc_component *component;
-	int i;
+	int i, ret;
 
 	if (dai_link->ignore)
 		return 0;
@@ -1056,6 +1056,10 @@ static int soc_bind_dai_link(struct snd_soc_card *card,
 		return 0;
 	}
 
+	ret = soc_dai_link_sanity_check(card, dai_link);
+	if (ret < 0)
+		return ret;
+
 	rtd = soc_new_pcm_runtime(card, dai_link);
 	if (!rtd)
 		return -ENOMEM;
@@ -1985,15 +1989,6 @@ static int snd_soc_instantiate_card(struct snd_soc_card *card)
 	int ret, i;
 
 	mutex_lock(&client_mutex);
-	for_each_card_prelinks(card, i, dai_link) {
-		ret = soc_init_dai_link(card, dai_link);
-		if (ret) {
-			dev_err(card->dev, "ASoC: failed to init link %s: %d\n",
-				dai_link->name, ret);
-			mutex_unlock(&client_mutex);
-			return ret;
-		}
-	}
 	mutex_lock_nested(&card->mutex, SND_SOC_CARD_CLASS_INIT);
 
 	snd_soc_dapm_init(&card->dapm, card, NULL);
@@ -2073,9 +2068,6 @@ static int snd_soc_instantiate_card(struct snd_soc_card *card)
 		if (soc_is_dai_link_bound(card, dai_link))
 			continue;
 
-		ret = soc_init_dai_link(card, dai_link);
-		if (ret)
-			goto probe_end;
 		ret = soc_bind_dai_link(card, dai_link);
 		if (ret)
 			goto probe_end;
-- 
2.20.1

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

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [alsa-devel] Applied "ASoC: soc-core: move soc_init_dai_link()" to the asoc tree
  2019-11-05  6:45 ` [alsa-devel] [PATCH v3 01/19] ASoC: soc-core: move soc_init_dai_link() Kuninori Morimoto
@ 2019-11-05 23:51   ` Mark Brown
  0 siblings, 0 replies; 60+ messages in thread
From: Mark Brown @ 2019-11-05 23:51 UTC (permalink / raw)
  To: Kuninori Morimoto
  Cc: Linux-ALSA, Mark Brown, Pierre-Louis Bossart, Ranjani Sridharan

The patch

   ASoC: soc-core: move soc_init_dai_link()

has been applied to the asoc tree at

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

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 36794902de1fe6f46f8aa5e0d6a8d9884eecae1d Mon Sep 17 00:00:00 2001
From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Date: Tue, 5 Nov 2019 15:45:41 +0900
Subject: [PATCH] ASoC: soc-core: move soc_init_dai_link()

This patch moves soc_init_dai_link() next to soc_bind_dai_link().
This is prepare for soc_bind_dai_link() cleanup.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Link: https://lore.kernel.org/r/87eeym3joq.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/soc-core.c | 192 +++++++++++++++++++++----------------------
 1 file changed, 96 insertions(+), 96 deletions(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index b07ecfac39d7..a141828f8638 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -941,6 +941,102 @@ static bool soc_is_dai_link_bound(struct snd_soc_card *card,
 	return false;
 }
 
+static int soc_init_dai_link(struct snd_soc_card *card,
+			     struct snd_soc_dai_link *link)
+{
+	int i;
+	struct snd_soc_dai_link_component *codec, *platform;
+
+	for_each_link_codecs(link, i, codec) {
+		/*
+		 * Codec must be specified by 1 of name or OF node,
+		 * not both or neither.
+		 */
+		if (!!codec->name == !!codec->of_node) {
+			dev_err(card->dev, "ASoC: Neither/both codec name/of_node are set for %s\n",
+				link->name);
+			return -EINVAL;
+		}
+
+		/* Codec DAI name must be specified */
+		if (!codec->dai_name) {
+			dev_err(card->dev, "ASoC: codec_dai_name not set for %s\n",
+				link->name);
+			return -EINVAL;
+		}
+
+		/*
+		 * Defer card registration if codec component is not added to
+		 * component list.
+		 */
+		if (!soc_find_component(codec))
+			return -EPROBE_DEFER;
+	}
+
+	for_each_link_platforms(link, i, platform) {
+		/*
+		 * Platform may be specified by either name or OF node, but it
+		 * can be left unspecified, then no components will be inserted
+		 * in the rtdcom list
+		 */
+		if (!!platform->name == !!platform->of_node) {
+			dev_err(card->dev,
+				"ASoC: Neither/both platform name/of_node are set for %s\n",
+				link->name);
+			return -EINVAL;
+		}
+
+		/*
+		 * Defer card registration if platform component is not added to
+		 * component list.
+		 */
+		if (!soc_find_component(platform))
+			return -EPROBE_DEFER;
+	}
+
+	/* FIXME */
+	if (link->num_cpus > 1) {
+		dev_err(card->dev,
+			"ASoC: multi cpu is not yet supported %s\n",
+			link->name);
+		return -EINVAL;
+	}
+
+	/*
+	 * CPU device may be specified by either name or OF node, but
+	 * can be left unspecified, and will be matched based on DAI
+	 * name alone..
+	 */
+	if (link->cpus->name && link->cpus->of_node) {
+		dev_err(card->dev,
+			"ASoC: Neither/both cpu name/of_node are set for %s\n",
+			link->name);
+		return -EINVAL;
+	}
+
+	/*
+	 * Defer card registartion if cpu dai component is not added to
+	 * component list.
+	 */
+	if ((link->cpus->of_node || link->cpus->name) &&
+	    !soc_find_component(link->cpus))
+		return -EPROBE_DEFER;
+
+	/*
+	 * At least one of CPU DAI name or CPU device name/node must be
+	 * specified
+	 */
+	if (!link->cpus->dai_name &&
+	    !(link->cpus->name || link->cpus->of_node)) {
+		dev_err(card->dev,
+			"ASoC: Neither cpu_dai_name nor cpu_name/of_node are set for %s\n",
+			link->name);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static int soc_bind_dai_link(struct snd_soc_card *card,
 	struct snd_soc_dai_link *dai_link)
 {
@@ -1283,102 +1379,6 @@ static int soc_probe_link_components(struct snd_soc_card *card)
 	return 0;
 }
 
-static int soc_init_dai_link(struct snd_soc_card *card,
-			     struct snd_soc_dai_link *link)
-{
-	int i;
-	struct snd_soc_dai_link_component *codec, *platform;
-
-	for_each_link_codecs(link, i, codec) {
-		/*
-		 * Codec must be specified by 1 of name or OF node,
-		 * not both or neither.
-		 */
-		if (!!codec->name == !!codec->of_node) {
-			dev_err(card->dev, "ASoC: Neither/both codec name/of_node are set for %s\n",
-				link->name);
-			return -EINVAL;
-		}
-
-		/* Codec DAI name must be specified */
-		if (!codec->dai_name) {
-			dev_err(card->dev, "ASoC: codec_dai_name not set for %s\n",
-				link->name);
-			return -EINVAL;
-		}
-
-		/*
-		 * Defer card registration if codec component is not added to
-		 * component list.
-		 */
-		if (!soc_find_component(codec))
-			return -EPROBE_DEFER;
-	}
-
-	for_each_link_platforms(link, i, platform) {
-		/*
-		 * Platform may be specified by either name or OF node, but it
-		 * can be left unspecified, then no components will be inserted
-		 * in the rtdcom list
-		 */
-		if (!!platform->name == !!platform->of_node) {
-			dev_err(card->dev,
-				"ASoC: Neither/both platform name/of_node are set for %s\n",
-				link->name);
-			return -EINVAL;
-		}
-
-		/*
-		 * Defer card registration if platform component is not added to
-		 * component list.
-		 */
-		if (!soc_find_component(platform))
-			return -EPROBE_DEFER;
-	}
-
-	/* FIXME */
-	if (link->num_cpus > 1) {
-		dev_err(card->dev,
-			"ASoC: multi cpu is not yet supported %s\n",
-			link->name);
-		return -EINVAL;
-	}
-
-	/*
-	 * CPU device may be specified by either name or OF node, but
-	 * can be left unspecified, and will be matched based on DAI
-	 * name alone..
-	 */
-	if (link->cpus->name && link->cpus->of_node) {
-		dev_err(card->dev,
-			"ASoC: Neither/both cpu name/of_node are set for %s\n",
-			link->name);
-		return -EINVAL;
-	}
-
-	/*
-	 * Defer card registartion if cpu dai component is not added to
-	 * component list.
-	 */
-	if ((link->cpus->of_node || link->cpus->name) &&
-	    !soc_find_component(link->cpus))
-		return -EPROBE_DEFER;
-
-	/*
-	 * At least one of CPU DAI name or CPU device name/node must be
-	 * specified
-	 */
-	if (!link->cpus->dai_name &&
-	    !(link->cpus->name || link->cpus->of_node)) {
-		dev_err(card->dev,
-			"ASoC: Neither cpu_dai_name nor cpu_name/of_node are set for %s\n",
-			link->name);
-		return -EINVAL;
-	}
-
-	return 0;
-}
-
 void snd_soc_disconnect_sync(struct device *dev)
 {
 	struct snd_soc_component *component =
-- 
2.20.1

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

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* Re: [alsa-devel] [PATCH v3 02/19] ASoC: soc-core: tidyup soc_init_dai_link()
  2019-11-05  6:45 ` [alsa-devel] [PATCH v3 02/19] ASoC: soc-core: tidyup soc_init_dai_link() Kuninori Morimoto
  2019-11-05 23:51   ` [alsa-devel] Applied "ASoC: soc-core: tidyup soc_init_dai_link()" to the asoc tree Mark Brown
@ 2019-11-11 14:43   ` Jon Hunter
  2019-11-12  0:24     ` Kuninori Morimoto
  1 sibling, 1 reply; 60+ messages in thread
From: Jon Hunter @ 2019-11-11 14:43 UTC (permalink / raw)
  To: Kuninori Morimoto, Mark Brown
  Cc: linux-tegra, Linux-ALSA, Pierre-Louis Bossart, Ranjani Sridharan


On 05/11/2019 06:45, Kuninori Morimoto wrote:
> 
> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> 
> soc_init_dai_link() is needed to be called before soc_bind_dai_link().
> 
> 	int snd_soc_instantiate_card()
> 	{
> 		for_each_card_prelinks(...) {
> (1)			ret = soc_init_dai_link(...);
> 			...
> 		}
> 		...
> 		for_each_card_prelinks(...) {
> (2)			ret = soc_bind_dai_link(...);
> 			...
> 		}
> 		...
> 		for_each_card_links(...) {
> 			...
> (A)			ret = soc_init_dai_link(...);
> 			...
> (B)			ret = soc_bind_dai_link(...);
> 		}
> 		...
> 
> (1) is for (2), and (A) is for (B)
> (1) and (2) are for card prelink   dai_link.
> (A) and (B) are for topology added dai_link.
> 
> soc_init_dai_link() is sanity check for dai_link, not initializing today.
> Therefore, it is confusable naming. We can rename it as sanity_check.
> 
> And this check is for soc_bind_dai_link().
> It can be more simple code if we can call it from soc_bind_dai_link().
> 
> This patch renames it to soc_dai_link_sanity_check(), and
> call it from soc_bind_dai_link().
> 
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> ---
> v2 -> v3
> 	- add Reviewed-by
> 	- call soc_dai_link_sanity_check() after soc_is_dai_link_bound()
> 
>  sound/soc/soc-core.c | 22 +++++++---------------
>  1 file changed, 7 insertions(+), 15 deletions(-)

I am seeing an audio regression on -next and bisect is pointing to
this commit. I am seeing the following crash on boot during probe
deferral of the soundcard ...

[   13.520888] 8<--- cut here ---
[   13.531044] Unable to handle kernel NULL pointer dereference at virtual address 00000558
[   13.546555] pgd = 5c53a857
[   13.556672] [00000558] *pgd=00000000
[   13.574504] Internal error: Oops: 5 [#1] SMP ARM
[   13.585865] Modules linked in: snd_soc_tegra_wm8903(+) snd_soc_tegra_utils snd_soc_wm8903 brcmutil snd_soc_core ac97_bus snd_pcm_dmaengine snd_pcm snd_timer snd soundcore snd_soc_tegra30_ahub tegra30_devfreq tegra_wdt
[   13.608218] cfg80211: Loading compiled-in X.509 certificates for regulatory database
[   13.617591] CPU: 1 PID: 178 Comm: systemd-udevd Not tainted 5.4.0-rc1-00276-gbfce78a55965 #11
[   13.617598] Hardware name: NVIDIA Tegra SoC (Flattened Device Tree)
[   13.617638] PC is at tegra_wm8903_remove+0x10/0x30 [snd_soc_tegra_wm8903]
[   13.617661] LR is at tegra_wm8903_remove+0x10/0x30 [snd_soc_tegra_wm8903]
[   13.685077] pc : [<bf09f254>]    lr : [<bf09f254>]    psr: 60000013
[   13.697095] sp : e9231c60  ip : 00000100  fp : c1704c48
[   13.710229] r10: 00000001  r9 : bf082ca8  r8 : bf087024
[   13.722152] r7 : 00000078  r6 : 00000000  r5 : bf0a1090  r4 : fffffdfb
[   13.734811] r3 : bf0a1280  r2 : 00000000  r1 : bf0a068c  r0 : 00000000
[   13.748859] Flags: nZCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment none
[   13.762711] Control: 10c5387d  Table: a908004a  DAC: 00000051
[   13.775069] Process systemd-udevd (pid: 178, stack limit = 0x97e0dae2)
[   13.789094] Stack: (0xe9231c60 to 0xe9232000)
[   13.802489] 1c60: fffffdfb bf06e6a0 ffffffff 21a54800 bf0a1118 c1704c48 bf0a1228 bf087000
[   13.817940] 1c80: bf082c74 bf082c94 21a54800 3342fbf9 e9268c54 bf0a1090 00000000 bf0a1000
[   13.834685] 1ca0: ea9ee010 00000000 bf0a1664 00000000 c1704c48 bf06ef68 e9268c54 eb7d7880
[   13.851011] 1cc0: bf0a1000 bf09f518 ea9ee010 00000000 bf0a130c 00000000 bf0a130c 00000018
[   13.867641] 1ce0: 00000000 c095d8dc c18e52a4 ea9ee010 c18e52a8 c095b980 ea9ee010 bf0a130c
[   13.884376] 1d00: bf0a130c c1704c48 00000000 e91a4a64 bf0a1380 c095bc10 bf0a1380 c0cdeed0
[   13.899398] 1d20: bf0a0024 ea9ee010 00000000 bf0a130c c1704c48 00000000 e91a4a64 bf0a1380
[   13.915997] 1d40: c1704c48 c095bec0 00000000 bf0a130c ea9ee010 c095bf48 00000000 bf0a130c
[   13.932785] 1d60: c095bec8 c0959cbc e91a4a64 ea891858 ea9da3b4 3342fbf9 c184b908 bf0a130c
[   13.949018] 1d80: e9268c80 c184b908 00000000 c095acb0 bf0a0668 00210d00 bf0a130c bf0a130c
[   13.964549] 1da0: c18934e0 ffffe000 bf02f000 c095c9cc c1704c48 c18934e0 ffffe000 c0302ec8
[   13.981361] 1dc0: 8040003f e9266840 c1704c48 e93dcc00 f0821fff ebcffb80 e93dcc40 ea801e40
[   13.997018] 1de0: ebcffb80 e93dcc40 8040003e c046e908 00000000 3342fbf9 00000001 ebcffb80
[   14.013462] 1e00: e93dcc00 ea801e40 bf0a1380 3342fbf9 bf0a1380 00000001 e91a4b00 00000001
[   14.030635] 1e20: 00000000 c03dbf44 e9231f38 00000001 e9231f38 00000001 e91a4a40 c03db0a0
[   14.045868] 1e40: bf0a138c 00007fff bf0a1380 c03d7e54 00000000 bf0a13c8 b6df81bc bf0a1494
[   14.061143] 1e60: c1254fb4 bf0a1578 bf030f7d c1704c48 c1254f00 c1254f0c c1254f64 c1704c48
[   14.076164] 1e80: 00000000 00000000 ffffe000 bf000000 00003020 00000000 00000000 00000000
[   14.091152] 1ea0: 00000000 00000000 00000000 00000000 5f646e73 5f636f73 65726f63 00000000
[   14.105966] 1ec0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[   14.121024] 1ee0: 00000000 00000000 00000000 3342fbf9 7fffffff c1704c48 00000000 00000010
[   14.136351] 1f00: b6df81bc c0301204 e9230000 0000017b 00539720 c03db76c 7fffffff 00000000
[   14.151307] 1f20: 00000003 40000028 b6ea0f42 f081d000 00003020 00000000 f081db14 f081e1c0
[   14.166606] 1f40: f081d000 00003020 f081faa8 f081f94c f081ee00 00003000 00003100 00001f14
[   14.181789] 1f60: 000031de 00000000 00000000 00000000 00001f04 00000020 00000021 00000015
[   14.197068] 1f80: 00000000 00000010 00000000 3342fbf9 b6df8a8c b6df8a8c 00000000 b5caa400
[   14.212561] 1fa0: 0000017b c03011e0 b6df8a8c 00000000 00000010 b6df81bc 00000000 b6df8cd0
[   14.227942] 1fc0: b6df8a8c 00000000 b5caa400 0000017b 005383d0 bea569ac 00000000 00539720
[   14.243189] 1fe0: bea568a8 bea56898 b6df2951 b6ea0f42 400f0030 00000010 00000000 00000000
[   14.258485] [<bf09f254>] (tegra_wm8903_remove [snd_soc_tegra_wm8903]) from [<bf06e6a0>] (snd_soc_instantiate_card+0x2a4/0xa80 [snd_soc_core])
[   14.278509] [<bf06e6a0>] (snd_soc_instantiate_card [snd_soc_core]) from [<bf06ef68>] (snd_soc_register_card+0xec/0x110 [snd_soc_core])
[   14.298197] [<bf06ef68>] (snd_soc_register_card [snd_soc_core]) from [<bf09f518>] (tegra_wm8903_driver_probe+0x2a4/0x310 [snd_soc_tegra_wm8903])
[   14.307225] cfg80211: Loaded X.509 cert 'sforshee: 00b28ddf47aef9cea7'
[   14.318550] [<bf09f518>] (tegra_wm8903_driver_probe [snd_soc_tegra_wm8903]) from [<c095d8dc>] (platform_drv_probe+0x48/0x98)
[   14.318582] [<c095d8dc>] (platform_drv_probe) from [<c095b980>] (really_probe+0x234/0x34c)
[   14.366860] [<c095b980>] (really_probe) from [<c095bc10>] (driver_probe_device+0x60/0x168)
[   14.384433] [<c095bc10>] (driver_probe_device) from [<c095bec0>] (device_driver_attach+0x58/0x60)
[   14.400631] [<c095bec0>] (device_driver_attach) from [<c095bf48>] (__driver_attach+0x80/0xbc)
[   14.410962] brcmfmac: brcmf_fw_alloc_request: using brcm/brcmfmac4329-sdio for chip BCM4329/3
[   14.416550] [<c095bf48>] (__driver_attach) from [<c0959cbc>] (bus_for_each_dev+0x74/0xb4)
[   14.447878] [<c0959cbc>] (bus_for_each_dev) from [<c095acb0>] (bus_add_driver+0x164/0x1e8)
[   14.464924] brcmfmac mmc1:0001:1: Direct firmware load for brcm/brcmfmac4329-sdio.nvidia,cardhu-a04.txt failed with error -2
[   14.465849] [<c095acb0>] (bus_add_driver) from [<c095c9cc>] (driver_register+0x7c/0x114)
[   14.487509] brcmfmac mmc1:0001:1: Direct firmware load for brcm/brcmfmac4329-sdio.txt failed with error -2
[   14.500541] [<c095c9cc>] (driver_register) from [<c0302ec8>] (do_one_initcall+0x54/0x22c)
[   14.500574] [<c0302ec8>] (do_one_initcall) from [<c03dbf44>] (do_init_module+0x60/0x220)
[   14.549972] [<c03dbf44>] (do_init_module) from [<c03db0a0>] (load_module+0x1ffc/0x2494)
[   14.565845] [<c03db0a0>] (load_module) from [<c03db76c>] (sys_finit_module+0xac/0xd8)
[   14.581476] [<c03db76c>] (sys_finit_module) from [<c03011e0>] (__sys_trace_return+0x0/0x20)
[   14.597680] Exception stack(0xe9231fa8 to 0xe9231ff0)
[   14.610535] 1fa0:                   b6df8a8c 00000000 00000010 b6df81bc 00000000 b6df8cd0
[   14.626607] 1fc0: b6df8a8c 00000000 b5caa400 0000017b 005383d0 bea569ac 00000000 00539720
[   14.642640] 1fe0: bea568a8 bea56898 b6df2951 b6ea0f42
[   14.655529] Code: e92d4010 e5903100 e5931000 ebff2fa9 (e5900558) 
[   14.669547] ---[ end trace 6c5c1edf43b78cdc ]---

Cheers
Jon

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

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [alsa-devel] [PATCH v3 02/19] ASoC: soc-core: tidyup soc_init_dai_link()
  2019-11-11 14:43   ` [alsa-devel] [PATCH v3 02/19] ASoC: soc-core: tidyup soc_init_dai_link() Jon Hunter
@ 2019-11-12  0:24     ` Kuninori Morimoto
  2019-11-12 10:51       ` Jon Hunter
  0 siblings, 1 reply; 60+ messages in thread
From: Kuninori Morimoto @ 2019-11-12  0:24 UTC (permalink / raw)
  To: Jon Hunter
  Cc: linux-tegra, Linux-ALSA, Mark Brown, Pierre-Louis Bossart,
	Ranjani Sridharan


Hi Jon

Thank you for reporting.

> I am seeing an audio regression on -next and bisect is pointing to
> this commit. I am seeing the following crash on boot during probe
> deferral of the soundcard ...

It seems timing bug.
I have a plan to post below patch if my current posting patch are accepted,
but it seems it is necessary immediately.
I believe your issue will be solved by this patch,
but can you please test it ?
I will formally post it with your tested-by if it was OK.

# It will be more cleanuped in the future,
# but it needs more other cleanup patches...

--------------------
Subject: [PATCH] ASoC: soc-core: care card_probed at soc_cleanup_card_resources()

soc_cleanup_card_resources() will call card->remove(), but it should be
called if card->probe() or card->late_probe() are called.
snd_soc_bind_card() might be error before calling
card->probe() / card->late_probe().
In that time, card->remove() will be called.
This patch adds card_probed parameter to judge it.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/soc-core.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 55014e7..b078227 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -1980,7 +1980,8 @@ static void __soc_setup_card_name(char *name, int len,
 	}
 }
 
-static void soc_cleanup_card_resources(struct snd_soc_card *card)
+static void soc_cleanup_card_resources(struct snd_soc_card *card,
+				       int card_probed)
 {
 	struct snd_soc_dai_link *link, *_link;
 
@@ -2007,7 +2008,7 @@ static void soc_cleanup_card_resources(struct snd_soc_card *card)
 	soc_cleanup_card_debugfs(card);
 
 	/* remove the card */
-	if (card->remove)
+	if (card_probed && card->remove)
 		card->remove(card);
 }
 
@@ -2015,7 +2016,7 @@ static int snd_soc_instantiate_card(struct snd_soc_card *card)
 {
 	struct snd_soc_pcm_runtime *rtd;
 	struct snd_soc_dai_link *dai_link;
-	int ret, i;
+	int ret, i, card_probed = 0;
 
 	mutex_lock(&client_mutex);
 	mutex_lock_nested(&card->mutex, SND_SOC_CARD_CLASS_INIT);
@@ -2067,6 +2068,7 @@ static int snd_soc_instantiate_card(struct snd_soc_card *card)
 		ret = card->probe(card);
 		if (ret < 0)
 			goto probe_end;
+		card_probed = 1;
 	}
 
 	/* probe all components used by DAI links on this card */
@@ -2129,6 +2131,7 @@ static int snd_soc_instantiate_card(struct snd_soc_card *card)
 			goto probe_end;
 		}
 	}
+	card_probed = 1;
 
 	snd_soc_dapm_new_widgets(card);
 
@@ -2145,7 +2148,7 @@ static int snd_soc_instantiate_card(struct snd_soc_card *card)
 
 probe_end:
 	if (ret < 0)
-		soc_cleanup_card_resources(card);
+		soc_cleanup_card_resources(card, card_probed);
 
 	mutex_unlock(&card->mutex);
 	mutex_unlock(&client_mutex);
@@ -2439,11 +2442,13 @@ EXPORT_SYMBOL_GPL(snd_soc_register_card);
 static void snd_soc_unbind_card(struct snd_soc_card *card, bool unregister)
 {
 	if (card->instantiated) {
+		int card_probed = 1;
+
 		card->instantiated = false;
 		snd_soc_dapm_shutdown(card);
 		snd_soc_flush_all_delayed_work(card);
 
-		soc_cleanup_card_resources(card);
+		soc_cleanup_card_resources(card, card_probed);
 		if (!unregister)
 			list_add(&card->list, &unbind_card_list);
 	} else {
-- 
2.7.4




Thank you for your help !!
Best regards
---
Kuninori Morimoto
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* Re: [alsa-devel] [PATCH v3 06/19] ASoC: soc-core: add soc_unbind_dai_link()
  2019-11-05  6:46 ` [alsa-devel] [PATCH v3 06/19] ASoC: soc-core: add soc_unbind_dai_link() Kuninori Morimoto
  2019-11-05 23:51   ` [alsa-devel] Applied "ASoC: soc-core: add soc_unbind_dai_link()" to the asoc tree Mark Brown
@ 2019-11-12  5:21   ` Pierre-Louis Bossart
  2019-11-12  5:37     ` Kuninori Morimoto
  1 sibling, 1 reply; 60+ messages in thread
From: Pierre-Louis Bossart @ 2019-11-12  5:21 UTC (permalink / raw)
  To: Kuninori Morimoto, Mark Brown; +Cc: Linux-ALSA, Ranjani Sridharan



On 11/5/19 12:46 AM, Kuninori Morimoto wrote:
> 
> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> 
> It is easy to read code if it is cleanly using paired function/naming,
> like start <-> stop, register <-> unregister, etc, etc.
> But, current ALSA SoC code is very random, unbalance, not paired, etc.
> It is easy to create bug at the such code, and it will be difficult to
> debug.
> 
> ALSA SoC has soc_bind_dai_link(), but its paired soc_unbind_dai_link()
> is not implemented.
> More confusable is that soc_remove_pcm_runtimes() which should be
> soc_unbind_dai_link() is implemented without synchronised
> to soc_bind_dai_link().
> 
> This patch cleanup this unbalance.
> 
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

Morimoto-san, this patch seems to introduce a regression in our SOF 
module removal tests. Without a couple of load/unload modules cycles, we 
hit a kernel oops while freeing the card.

see https://github.com/thesofproject/linux/issues/1466 for some logs.

This issue did not happen with our November 6 rebase on Mark's tree, and 
showed up today. I couldn't really bisect the whole tree due to other 
issues, so manually applied your patches on top of this 11/06 tree and 
bisected from there.

I will need to confirm this finding (it's quite late for me) but looking 
at the code I wonder if the move of pcm_runtime deletion is correct?


> ---
> v2 -> v3
> 	- no change
> 	- add Reviewed-by
> 
>   sound/soc/soc-core.c | 22 ++++++++++++----------
>   1 file changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
> index e8ff6f2..1e8dd19 100644
> --- a/sound/soc/soc-core.c
> +++ b/sound/soc/soc-core.c
> @@ -470,14 +470,6 @@ static struct snd_soc_pcm_runtime *soc_new_pcm_runtime(
>   	return NULL;
>   }
>   
> -static void soc_remove_pcm_runtimes(struct snd_soc_card *card)
> -{
> -	struct snd_soc_pcm_runtime *rtd, *_rtd;
> -
> -	for_each_card_rtds_safe(card, rtd, _rtd)
> -		soc_free_pcm_runtime(rtd);
> -}
> -
>   struct snd_soc_pcm_runtime *snd_soc_get_pcm_runtime(struct snd_soc_card *card,
>   		const char *dai_link)
>   {
> @@ -1037,6 +1029,16 @@ static int soc_dai_link_sanity_check(struct snd_soc_card *card,
>   	return 0;
>   }
>   
> +static void soc_unbind_dai_link(struct snd_soc_card *card,
> +				struct snd_soc_dai_link *dai_link)
> +{
> +	struct snd_soc_pcm_runtime *rtd;
> +
> +	rtd = snd_soc_get_pcm_runtime(card, dai_link->name);
> +	if (rtd)
> +		soc_free_pcm_runtime(rtd);
> +}
> +
>   static int soc_bind_dai_link(struct snd_soc_card *card,
>   	struct snd_soc_dai_link *dai_link)
>   {
> @@ -1466,6 +1468,8 @@ void snd_soc_remove_dai_link(struct snd_soc_card *card,
>   		card->remove_dai_link(card, dai_link);
>   
>   	list_del(&dai_link->list);
> +
> +	soc_unbind_dai_link(card, dai_link);
>   }
>   EXPORT_SYMBOL_GPL(snd_soc_remove_dai_link);
>   
> @@ -1974,8 +1978,6 @@ static void soc_cleanup_card_resources(struct snd_soc_card *card)
>   	for_each_card_links_safe(card, link, _link)
>   		snd_soc_remove_dai_link(card, link);
>   
> -	soc_remove_pcm_runtimes(card);
> -
>   	/* remove auxiliary devices */
>   	soc_remove_aux_devices(card);
>   	soc_unbind_aux_dev(card);
> 
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [alsa-devel] [PATCH v3 06/19] ASoC: soc-core: add soc_unbind_dai_link()
  2019-11-12  5:21   ` [alsa-devel] [PATCH v3 06/19] ASoC: soc-core: add soc_unbind_dai_link() Pierre-Louis Bossart
@ 2019-11-12  5:37     ` Kuninori Morimoto
  2019-11-12  5:50       ` Kuninori Morimoto
  0 siblings, 1 reply; 60+ messages in thread
From: Kuninori Morimoto @ 2019-11-12  5:37 UTC (permalink / raw)
  To: Pierre-Louis Bossart; +Cc: Linux-ALSA, Mark Brown, Ranjani Sridharan


Hi Pierre-Louis

Thank you for reporting

> > From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> > 
> > It is easy to read code if it is cleanly using paired function/naming,
> > like start <-> stop, register <-> unregister, etc, etc.
> > But, current ALSA SoC code is very random, unbalance, not paired, etc.
> > It is easy to create bug at the such code, and it will be difficult to
> > debug.
> > 
> > ALSA SoC has soc_bind_dai_link(), but its paired soc_unbind_dai_link()
> > is not implemented.
> > More confusable is that soc_remove_pcm_runtimes() which should be
> > soc_unbind_dai_link() is implemented without synchronised
> > to soc_bind_dai_link().
> > 
> > This patch cleanup this unbalance.
> > 
> > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> > Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> 
> Morimoto-san, this patch seems to introduce a regression in our SOF
> module removal tests. Without a couple of load/unload modules cycles,
> we hit a kernel oops while freeing the card.
> 
> see https://github.com/thesofproject/linux/issues/1466 for some logs.
> 
> This issue did not happen with our November 6 rebase on Mark's tree,
> and showed up today. I couldn't really bisect the whole tree due to
> other issues, so manually applied your patches on top of this 11/06
> tree and bisected from there.
> 
> I will need to confirm this finding (it's quite late for me) but
> looking at the code I wonder if the move of pcm_runtime deletion is
> correct?

Hmm...
It is just merged verbose 2 functions into 1,
nothing changed from logic point of view
if my understanding was correct.

And now, I tried unbind test for cpu/codec/card on my side,
but nothing happen...

I'm using this commit

	bc7a9091e5b927ecc20dbb3bc07a5a09783fc27b
	("ASoC: soc-core: add soc_unbind_dai_link()")


Thank you for your help !!
Best regards
---
Kuninori Morimoto
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [alsa-devel] [PATCH v3 06/19] ASoC: soc-core: add soc_unbind_dai_link()
  2019-11-12  5:37     ` Kuninori Morimoto
@ 2019-11-12  5:50       ` Kuninori Morimoto
  2019-11-12  6:42         ` Kuninori Morimoto
  0 siblings, 1 reply; 60+ messages in thread
From: Kuninori Morimoto @ 2019-11-12  5:50 UTC (permalink / raw)
  To: Pierre-Louis Bossart; +Cc: Linux-ALSA, Mark Brown, Ranjani Sridharan


Hi Pierre-Louis, again

> > Morimoto-san, this patch seems to introduce a regression in our SOF
> > module removal tests. Without a couple of load/unload modules cycles,
> > we hit a kernel oops while freeing the card.
> > 
> > see https://github.com/thesofproject/linux/issues/1466 for some logs.
> > 
> > This issue did not happen with our November 6 rebase on Mark's tree,
> > and showed up today. I couldn't really bisect the whole tree due to
> > other issues, so manually applied your patches on top of this 11/06
> > tree and bisected from there.
> > 
> > I will need to confirm this finding (it's quite late for me) but
> > looking at the code I wonder if the move of pcm_runtime deletion is
> > correct?
> 
> Hmm...
> It is just merged verbose 2 functions into 1,
> nothing changed from logic point of view
> if my understanding was correct.
> 
> And now, I tried unbind test for cpu/codec/card on my side,
> but nothing happen...
> 
> I'm using this commit
> 
> 	bc7a9091e5b927ecc20dbb3bc07a5a09783fc27b
> 	("ASoC: soc-core: add soc_unbind_dai_link()")

Does it happen from soc-topology.c :: remove_link ?

Thank you for your help !!
Best regards
---
Kuninori Morimoto
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [alsa-devel] [PATCH v3 06/19] ASoC: soc-core: add soc_unbind_dai_link()
  2019-11-12  5:50       ` Kuninori Morimoto
@ 2019-11-12  6:42         ` Kuninori Morimoto
  2019-11-12 17:11           ` Pierre-Louis Bossart
  0 siblings, 1 reply; 60+ messages in thread
From: Kuninori Morimoto @ 2019-11-12  6:42 UTC (permalink / raw)
  To: Pierre-Louis Bossart; +Cc: Linux-ALSA, Mark Brown, Ranjani Sridharan


Hi Pierre-Louis, again and again

> > I'm using this commit
> > 
> > 	bc7a9091e5b927ecc20dbb3bc07a5a09783fc27b
> > 	("ASoC: soc-core: add soc_unbind_dai_link()")
> 
> Does it happen from soc-topology.c :: remove_link ?

I can't test, but can this patch solve your issue?

I guess topology related rtd free timing was changed by
this balanceup.

-----------
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 1e8dd19..af89aad 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -384,6 +384,9 @@ static void soc_free_pcm_runtime(struct snd_soc_pcm_runtime *rtd)
 	if (!rtd)
 		return;
 
+	/* need to sync the delayed work before releasing resources */
+	flush_delayed_work(&rtd->delayed_work);
+
 	list_del(&rtd->list);
 
 	/*
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index 1c00119..9865a2d 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -2883,15 +2883,6 @@ static int dpcm_fe_dai_close(struct snd_pcm_substream *fe_substream)
 	return ret;
 }
 
-static void soc_pcm_private_free(struct snd_pcm *pcm)
-{
-	struct snd_soc_pcm_runtime *rtd = pcm->private_data;
-
-	/* need to sync the delayed work before releasing resources */
-	flush_delayed_work(&rtd->delayed_work);
-	snd_soc_pcm_component_free(pcm);
-}
-
 /* create a new pcm */
 int soc_new_pcm(struct snd_soc_pcm_runtime *rtd, int num)
 {
@@ -3033,7 +3024,7 @@ int soc_new_pcm(struct snd_soc_pcm_runtime *rtd, int num)
 		return ret;
 	}
 
-	pcm->private_free = soc_pcm_private_free;
+	pcm->private_free = snd_soc_pcm_component_free;
 	pcm->no_device_suspend = true;
 out:
 	dev_info(rtd->card->dev, "%s <-> %s mapping ok\n",

 
Thank you for your help !!
Best regards
---
Kuninori Morimoto
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* Re: [alsa-devel] [PATCH v3 02/19] ASoC: soc-core: tidyup soc_init_dai_link()
  2019-11-12  0:24     ` Kuninori Morimoto
@ 2019-11-12 10:51       ` Jon Hunter
  2019-11-13  0:29         ` Kuninori Morimoto
  0 siblings, 1 reply; 60+ messages in thread
From: Jon Hunter @ 2019-11-12 10:51 UTC (permalink / raw)
  To: Kuninori Morimoto
  Cc: linux-tegra, Linux-ALSA, Mark Brown, Pierre-Louis Bossart,
	Ranjani Sridharan


On 12/11/2019 00:24, Kuninori Morimoto wrote:
> 
> Hi Jon
> 
> Thank you for reporting.
> 
>> I am seeing an audio regression on -next and bisect is pointing to
>> this commit. I am seeing the following crash on boot during probe
>> deferral of the soundcard ...
> 
> It seems timing bug.
> I have a plan to post below patch if my current posting patch are accepted,
> but it seems it is necessary immediately.
> I believe your issue will be solved by this patch,
> but can you please test it ?
> I will formally post it with your tested-by if it was OK.
> 
> # It will be more cleanuped in the future,
> # but it needs more other cleanup patches...
> 
> --------------------
> Subject: [PATCH] ASoC: soc-core: care card_probed at soc_cleanup_card_resources()
> 
> soc_cleanup_card_resources() will call card->remove(), but it should be
> called if card->probe() or card->late_probe() are called.
> snd_soc_bind_card() might be error before calling
> card->probe() / card->late_probe().
> In that time, card->remove() will be called.
> This patch adds card_probed parameter to judge it.
> 
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> ---
>  sound/soc/soc-core.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)

Thanks! I can confirm that this works, so ...

Tested-by: Jon Hunter <jonathanh@nvidia.com>

Cheers
Jon

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

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [alsa-devel] [PATCH v3 06/19] ASoC: soc-core: add soc_unbind_dai_link()
  2019-11-12  6:42         ` Kuninori Morimoto
@ 2019-11-12 17:11           ` Pierre-Louis Bossart
  2019-11-12 19:03             ` Mark Brown
  0 siblings, 1 reply; 60+ messages in thread
From: Pierre-Louis Bossart @ 2019-11-12 17:11 UTC (permalink / raw)
  To: Kuninori Morimoto; +Cc: Linux-ALSA, Mark Brown, Ranjani Sridharan



>> Does it happen from soc-topology.c :: remove_link ?

it seems to happen after the topology remove link, see the traces below

> 
> I can't test, but can this patch solve your issue?

No, the problem remains after applying your suggested fix.

I added a bunch of traces and it seems we have a nasty case of corrupted 
linked lists:

diff --git a/sound/soc/soc-component.c b/sound/soc/soc-component.c
index 98ef0666add2..5b0139ebe8f3 100644
--- a/sound/soc/soc-component.c
+++ b/sound/soc/soc-component.c
@@ -518,11 +518,39 @@ int snd_soc_pcm_component_new(struct snd_pcm *pcm)

  void snd_soc_pcm_component_free(struct snd_pcm *pcm)
  {
-       struct snd_soc_pcm_runtime *rtd = pcm->private_data;
+       struct snd_soc_pcm_runtime *rtd;
         struct snd_soc_rtdcom_list *rtdcom;
         struct snd_soc_component *component;

-       for_each_rtd_components(rtd, rtdcom, component)
-               if (component->driver->pcm_destruct)
+       pr_err("plb: %s start\n", __func__);
+
+       if (!pcm)
+               pr_err("plb: %s PCM is NULL\n", __func__);
+
+       pr_err("plb: %s accessing private data\n", __func__);
+       rtd = pcm->private_data;
+       pr_err("plb: %s accessed private data\n", __func__);
+
+       if (!rtd)
+               pr_err("plb: %s RTD is NULL\n", __func__);
+
+       pr_err("plb: %s accessing components\n", __func__);
+       for_each_rtd_components(rtd, rtdcom, component) {
+               pr_err("plb: %s processing component\n", __func__);
+               if (!component)
+                       pr_err("plb: %s component is NULL\n", __func__);
+
+               if (!component->driver)
+                       pr_err("plb: %s component driver is NULL\n", 
__func__);
+
+               pr_err("plb: %s pcm_destruct checks\n", __func__);
+               if (component->driver->pcm_destruct) {
+                       pr_err("plb: %s pcm_destruct start\n", __func__);
                         component->driver->pcm_destruct(component, pcm);
+                       pr_err("plb: %s pcm_destruct done\n", __func__);
+               }
+               pr_err("plb: %s processing component done\n", __func__);
+       }
+
+       pr_err("plb: %s done\n", __func__);
  }

And the results show the for_each_rtd_components loop goes in the weeds.

    82.069990] sof-audio-pci 0000:00:1f.3: plb: remove_link start
[   82.069993] sof-audio-pci 0000:00:1f.3: plb: remove_link 2
[   82.069996] sof-audio-pci 0000:00:1f.3: plb: remove_link before 
snd_soc_remove_dai_link
[   82.069998] plb: snd_soc_remove_dai_link start
[   82.070016] plb: snd_soc_remove_dai_link done
[   82.070020] sof-audio-pci 0000:00:1f.3: plb: remove_link done
<removed DSP power down sequence>
[   82.179021] plb: snd_soc_pcm_component_free start
[   82.179023] plb: snd_soc_pcm_component_free accessing private data
[   82.179024] plb: snd_soc_pcm_component_free accessed private data
[   82.179025] plb: snd_soc_pcm_component_free accessing components
[   82.179025] plb: snd_soc_pcm_component_free processing component
[   82.179029] BUG: kernel NULL pointer dereference, address: 
0000000000000064
[   82.179030] #PF: supervisor read access in kernel mode
[   82.179031] #PF: error_code(0x0000) - not-present page
[   82.179032] PGD 0 P4D 0
[   82.179034] Oops: 0000 [#1] SMP NOPTI
[   82.179036] CPU: 3 PID: 768 Comm: pulseaudio Not tainted 
5.4.0-rc5-test+ #31
[   82.179036] Hardware name: Acer Swift SF314-55/MILLER_WL, BIOS V1.05 
10/03/2018
[   82.179042] RIP: 0010:snd_soc_pcm_component_free+0xc7/0x16a 
[snd_soc_core]
[   82.179043] Code: 43 08 48 c7 c6 f0 24 6e c0 4c 39 e0 0f 84 a9 00 00 
00 48 8b 2b 48 85 ed 0f 84 9d 00 00 00 48 c7 c7 00 51 6e c0 e8 d2 5d 5d 
f2 <48> 83 7d 60 00 75 13 48 c7 c6 f0 24 6e c0 48 c7 c7 20 51 6e c0 e8
[   82.179044] RSP: 0018:ffffa70180bf3d78 EFLAGS: 00010246
[   82.179046] RAX: 0000000000000034 RBX: ffffa00f7aaf3968 RCX: 
0000000000000006
[   82.179047] RDX: 0000000000000000 RSI: 0000000000000092 RDI: 
ffffa00fa5ad63d0
[   82.179048] RBP: 0000000000000004 R08: ffffa70180bf3c3d R09: 
0000000000001518
[   82.179049] R10: ffffa70180bf3c38 R11: ffffa70180bf3c3d R12: 
ffffa00fa1be4eb0
[   82.179050] R13: ffffa00fa27aa000 R14: dead000000000122 R15: 
dead000000000100
[   82.179052] FS:  00007f4e7e5ebc80(0000) GS:ffffa00fa5ac0000(0000) 
knlGS:0000000000000000
[   82.179054] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   82.179055] CR2: 0000000000000064 CR3: 0000000253d68005 CR4: 
00000000003606e0
[   82.179056] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 
0000000000000000
[   82.179057] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 
0000000000000400
[   82.179058] Call Trace:
[   82.179064]  snd_pcm_free+0x1a/0x50 [snd_pcm]


I have absolutely no idea what all these data structures are, just 
reporting this.

reverting "ASoC: soc-core: add soc_unbind_dai_link()" is the only 
work-around at this point. i've tested this module load/unload for hours 
without issues.

It's actually quite interesting since this snd_soc_pcm_component_free() 
calls a .pcm_destruct() callback that's not used by the SOF driver. It's 
only used on Intel platforms for the Skylake/SST driver, not sure why 
and if SOF is missing something.
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* Re: [alsa-devel] [PATCH v3 06/19] ASoC: soc-core: add soc_unbind_dai_link()
  2019-11-12 17:11           ` Pierre-Louis Bossart
@ 2019-11-12 19:03             ` Mark Brown
  2019-11-12 21:08               ` Pierre-Louis Bossart
  0 siblings, 1 reply; 60+ messages in thread
From: Mark Brown @ 2019-11-12 19:03 UTC (permalink / raw)
  To: Pierre-Louis Bossart; +Cc: Linux-ALSA, Ranjani Sridharan, Kuninori Morimoto


[-- Attachment #1.1: Type: text/plain, Size: 502 bytes --]

On Tue, Nov 12, 2019 at 11:11:32AM -0600, Pierre-Louis Bossart wrote:

> +       for_each_rtd_components(rtd, rtdcom, component) {
> +               pr_err("plb: %s processing component\n", __func__);
> +               if (!component)
> +                       pr_err("plb: %s component is NULL\n", __func__);

Could you perhaps add traces of which components are being accessed at
each stage?  We might want to go through and just add something like
that in the code anyway to help figure things out.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

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

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [alsa-devel] [PATCH v3 06/19] ASoC: soc-core: add soc_unbind_dai_link()
  2019-11-12 19:03             ` Mark Brown
@ 2019-11-12 21:08               ` Pierre-Louis Bossart
  2019-11-13  4:37                 ` Kuninori Morimoto
  0 siblings, 1 reply; 60+ messages in thread
From: Pierre-Louis Bossart @ 2019-11-12 21:08 UTC (permalink / raw)
  To: Mark Brown; +Cc: Linux-ALSA, Ranjani Sridharan, Kuninori Morimoto


>> +       for_each_rtd_components(rtd, rtdcom, component) {
>> +               pr_err("plb: %s processing component\n", __func__);
>> +               if (!component)
>> +                       pr_err("plb: %s component is NULL\n", __func__);
> 
> Could you perhaps add traces of which components are being accessed at
> each stage?  We might want to go through and just add something like
> that in the code anyway to help figure things out.

I tried to add more traces but couldn't triangulate on a clear issue, 
and the traces have an Heisenbug effect.

So I switched to higher-level code analysis: it turns out that 
soc_dai_link_remove() routine is called from both topology and on card 
cleanup.

The patch 06/19 in this series essentially forces the pcm_runtimes to be 
freed in both cases, so possibly twice for topology-managed dailinks - 
or using information that's been freed already.

I 'fixed' this by adding an additional parameter to avoid doing the pcm 
runtime free from the topology (as was done before), and the kernel oops 
goes away. My tests have been running for 45mn now, when without change 
I get a kernel oops in less than 10-20 cycles (but still more than 
apparently our CI tracks, something to improve).

I pushed the code on GitHub to check if there are any negative points 
reported by the Intel CI, should be complete shortly:
https://github.com/thesofproject/linux/pull/1469

I am not sure the suggested fix is correct, I don't fully get what the 
topology and card cleanups should do and how the work is split, if at all.
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [alsa-devel] [PATCH v3 02/19] ASoC: soc-core: tidyup soc_init_dai_link()
  2019-11-12 10:51       ` Jon Hunter
@ 2019-11-13  0:29         ` Kuninori Morimoto
  0 siblings, 0 replies; 60+ messages in thread
From: Kuninori Morimoto @ 2019-11-13  0:29 UTC (permalink / raw)
  To: Jon Hunter
  Cc: linux-tegra, Linux-ALSA, Mark Brown, Pierre-Louis Bossart,
	Ranjani Sridharan


Hi Jon

> >> I am seeing an audio regression on -next and bisect is pointing to
> >> this commit. I am seeing the following crash on boot during probe
> >> deferral of the soundcard ...
> > 
> > It seems timing bug.
> > I have a plan to post below patch if my current posting patch are accepted,
> > but it seems it is necessary immediately.
> > I believe your issue will be solved by this patch,
> > but can you please test it ?
> > I will formally post it with your tested-by if it was OK.
> > 
> > # It will be more cleanuped in the future,
> > # but it needs more other cleanup patches...
> > 
> > --------------------
> > Subject: [PATCH] ASoC: soc-core: care card_probed at soc_cleanup_card_resources()
> > 
> > soc_cleanup_card_resources() will call card->remove(), but it should be
> > called if card->probe() or card->late_probe() are called.
> > snd_soc_bind_card() might be error before calling
> > card->probe() / card->late_probe().
> > In that time, card->remove() will be called.
> > This patch adds card_probed parameter to judge it.
> > 
> > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> > ---
> >  sound/soc/soc-core.c | 15 ++++++++++-----
> >  1 file changed, 10 insertions(+), 5 deletions(-)
> 
> Thanks! I can confirm that this works, so ...
> 
> Tested-by: Jon Hunter <jonathanh@nvidia.com>

Thanks. I will post formal patch, soon

Thank you for your help !!
Best regards
---
Kuninori Morimoto
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [alsa-devel] [PATCH v3 06/19] ASoC: soc-core: add soc_unbind_dai_link()
  2019-11-12 21:08               ` Pierre-Louis Bossart
@ 2019-11-13  4:37                 ` Kuninori Morimoto
  2019-11-13  5:57                   ` Takashi Iwai
  2019-11-13 16:05                   ` Pierre-Louis Bossart
  0 siblings, 2 replies; 60+ messages in thread
From: Kuninori Morimoto @ 2019-11-13  4:37 UTC (permalink / raw)
  To: Pierre-Louis Bossart; +Cc: Linux-ALSA, Mark Brown, Ranjani Sridharan

[-- Attachment #1: Type: text/plain, Size: 2205 bytes --]


Hi Pierre-Louis

Thank you for your report

> >> +       for_each_rtd_components(rtd, rtdcom, component) {
> >> +               pr_err("plb: %s processing component\n", __func__);
> >> +               if (!component)
> >> +                       pr_err("plb: %s component is NULL\n", __func__);
> > 
> > Could you perhaps add traces of which components are being accessed at
> > each stage?  We might want to go through and just add something like
> > that in the code anyway to help figure things out.
> 
> I tried to add more traces but couldn't triangulate on a clear issue,
> and the traces have an Heisenbug effect.
> 
> So I switched to higher-level code analysis: it turns out that
> soc_dai_link_remove() routine is called from both topology and on card
> cleanup.
> 
> The patch 06/19 in this series essentially forces the pcm_runtimes to
> be freed in both cases, so possibly twice for topology-managed
> dailinks - or using information that's been freed already.
> 
> I 'fixed' this by adding an additional parameter to avoid doing the
> pcm runtime free from the topology (as was done before), and the
> kernel oops goes away. My tests have been running for 45mn now, when
> without change I get a kernel oops in less than 10-20 cycles (but
> still more than apparently our CI tracks, something to improve).
> 
> I pushed the code on GitHub to check if there are any negative points
> reported by the Intel CI, should be complete shortly:
> https://github.com/thesofproject/linux/pull/1469
> 
> I am not sure the suggested fix is correct, I don't fully get what the
> topology and card cleanups should do and how the work is split, if at
> all.

BTW, I guess your kernel is appling this patch,
but, is it correct ?

	df95a16d2a9626dcfc3f2b3671c9b91fa076c997
	("ASoC: soc-core: fix RIP warning on card removal")

If so, I think I could understand the issue.
But, before explaining detail,
I want to confirm that my solution is correct or not first.

Can you please check attached patch ?
Then, please remove above RIP warning patch.
It is not clean patch, but OK to confirming, so far.
I think
	- no kernel Oops
	- no RIP warning

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

[-- Attachment #2: 0001-topology-die-fixup-patch-candidate-1.patch --]
[-- Type: text/plain, Size: 3782 bytes --]

From 0d825237eea4baad0c489e1c43a58373f41a3632 Mon Sep 17 00:00:00 2001
From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Date: Wed, 13 Nov 2019 11:58:18 +0900
Subject: [PATCH] topology die fixup patch candidate 1

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 include/sound/soc-component.h |  2 +-
 sound/soc/soc-component.c     |  5 ++---
 sound/soc/soc-core.c          | 14 ++++++++------
 sound/soc/soc-pcm.c           | 10 ----------
 4 files changed, 11 insertions(+), 20 deletions(-)

diff --git a/include/sound/soc-component.h b/include/sound/soc-component.h
index 6aa3ecb..cf726a5 100644
--- a/include/sound/soc-component.h
+++ b/include/sound/soc-component.h
@@ -413,6 +413,6 @@ struct page *snd_soc_pcm_component_page(struct snd_pcm_substream *substream,
 int snd_soc_pcm_component_mmap(struct snd_pcm_substream *substream,
 			       struct vm_area_struct *vma);
 int snd_soc_pcm_component_new(struct snd_pcm *pcm);
-void snd_soc_pcm_component_free(struct snd_pcm *pcm);
+void snd_soc_pcm_component_free(struct snd_soc_pcm_runtime *rtd);
 
 #endif /* __SOC_COMPONENT_H */
diff --git a/sound/soc/soc-component.c b/sound/soc/soc-component.c
index 98ef066..195979f 100644
--- a/sound/soc/soc-component.c
+++ b/sound/soc/soc-component.c
@@ -516,13 +516,12 @@ int snd_soc_pcm_component_new(struct snd_pcm *pcm)
 	return 0;
 }
 
-void snd_soc_pcm_component_free(struct snd_pcm *pcm)
+void snd_soc_pcm_component_free(struct snd_soc_pcm_runtime *rtd)
 {
-	struct snd_soc_pcm_runtime *rtd = pcm->private_data;
 	struct snd_soc_rtdcom_list *rtdcom;
 	struct snd_soc_component *component;
 
 	for_each_rtd_components(rtd, rtdcom, component)
 		if (component->driver->pcm_destruct)
-			component->driver->pcm_destruct(component, pcm);
+			component->driver->pcm_destruct(component, rtd->pcm);
 }
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 1e8dd19..8e7ff7d 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -386,6 +386,9 @@ static void soc_free_pcm_runtime(struct snd_soc_pcm_runtime *rtd)
 
 	list_del(&rtd->list);
 
+	flush_delayed_work(&rtd->delayed_work);
+	snd_soc_pcm_component_free(rtd);
+
 	/*
 	 * we don't need to call kfree() for rtd->dev
 	 * see
@@ -1965,12 +1968,6 @@ static void soc_cleanup_card_resources(struct snd_soc_card *card)
 {
 	struct snd_soc_dai_link *link, *_link;
 
-	/* free the ALSA card at first; this syncs with pending operations */
-	if (card->snd_card) {
-		snd_card_free(card->snd_card);
-		card->snd_card = NULL;
-	}
-
 	/* remove and free each DAI */
 	soc_remove_link_dais(card);
 	soc_remove_link_components(card);
@@ -1988,6 +1985,11 @@ static void soc_cleanup_card_resources(struct snd_soc_card *card)
 	/* remove the card */
 	if (card->remove)
 		card->remove(card);
+
+	if (card->snd_card) {
+		snd_card_free(card->snd_card);
+		card->snd_card = NULL;
+	}
 }
 
 static int snd_soc_instantiate_card(struct snd_soc_card *card)
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index 1c00119..a60e381 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -2883,15 +2883,6 @@ static int dpcm_fe_dai_close(struct snd_pcm_substream *fe_substream)
 	return ret;
 }
 
-static void soc_pcm_private_free(struct snd_pcm *pcm)
-{
-	struct snd_soc_pcm_runtime *rtd = pcm->private_data;
-
-	/* need to sync the delayed work before releasing resources */
-	flush_delayed_work(&rtd->delayed_work);
-	snd_soc_pcm_component_free(pcm);
-}
-
 /* create a new pcm */
 int soc_new_pcm(struct snd_soc_pcm_runtime *rtd, int num)
 {
@@ -3033,7 +3024,6 @@ int soc_new_pcm(struct snd_soc_pcm_runtime *rtd, int num)
 		return ret;
 	}
 
-	pcm->private_free = soc_pcm_private_free;
 	pcm->no_device_suspend = true;
 out:
 	dev_info(rtd->card->dev, "%s <-> %s mapping ok\n",
-- 
2.7.4


[-- Attachment #3: Type: text/plain, Size: 161 bytes --]

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

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* Re: [alsa-devel] [PATCH v3 06/19] ASoC: soc-core: add soc_unbind_dai_link()
  2019-11-13  4:37                 ` Kuninori Morimoto
@ 2019-11-13  5:57                   ` Takashi Iwai
  2019-11-13  6:33                     ` Kuninori Morimoto
  2019-11-13 16:05                   ` Pierre-Louis Bossart
  1 sibling, 1 reply; 60+ messages in thread
From: Takashi Iwai @ 2019-11-13  5:57 UTC (permalink / raw)
  To: Kuninori Morimoto
  Cc: Linux-ALSA, Mark Brown, Pierre-Louis Bossart, Ranjani Sridharan

On Wed, 13 Nov 2019 05:37:46 +0100,
Kuninori Morimoto wrote:
> 
> @@ -1965,12 +1968,6 @@ static void soc_cleanup_card_resources(struct snd_soc_card *card)
>  {
>  	struct snd_soc_dai_link *link, *_link;
>  
> -	/* free the ALSA card at first; this syncs with pending operations */
> -	if (card->snd_card) {
> -		snd_card_free(card->snd_card);
> -		card->snd_card = NULL;
> -	}
> -
>  	/* remove and free each DAI */
>  	soc_remove_link_dais(card);
>  	soc_remove_link_components(card);
> @@ -1988,6 +1985,11 @@ static void soc_cleanup_card_resources(struct snd_soc_card *card)
>  	/* remove the card */
>  	if (card->remove)
>  		card->remove(card);
> +
> +	if (card->snd_card) {
> +		snd_card_free(card->snd_card);
> +		card->snd_card = NULL;
> +	}

This will likely break unbind again; when unbind is performed in a
busy state, the code may release still-in-use resources.  The rcar
driver seems to have its own disconnect_sync() call so it'd work even
with this change, but others wouldn't.

At least you need to call snd_card_disconnect_sync() at the first
place, then release the rest at the second place.


thanks,

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

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [alsa-devel] [PATCH v3 06/19] ASoC: soc-core: add soc_unbind_dai_link()
  2019-11-13  5:57                   ` Takashi Iwai
@ 2019-11-13  6:33                     ` Kuninori Morimoto
  0 siblings, 0 replies; 60+ messages in thread
From: Kuninori Morimoto @ 2019-11-13  6:33 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: Linux-ALSA, Mark Brown, Pierre-Louis Bossart, Ranjani Sridharan


Hi Takashi-san

> > @@ -1965,12 +1968,6 @@ static void soc_cleanup_card_resources(struct snd_soc_card *card)
> >  {
> >  	struct snd_soc_dai_link *link, *_link;
> >  
> > -	/* free the ALSA card at first; this syncs with pending operations */
> > -	if (card->snd_card) {
> > -		snd_card_free(card->snd_card);
> > -		card->snd_card = NULL;
> > -	}
> > -
> >  	/* remove and free each DAI */
> >  	soc_remove_link_dais(card);
> >  	soc_remove_link_components(card);
> > @@ -1988,6 +1985,11 @@ static void soc_cleanup_card_resources(struct snd_soc_card *card)
> >  	/* remove the card */
> >  	if (card->remove)
> >  		card->remove(card);
> > +
> > +	if (card->snd_card) {
> > +		snd_card_free(card->snd_card);
> > +		card->snd_card = NULL;
> > +	}
> 
> This will likely break unbind again; when unbind is performed in a
> busy state, the code may release still-in-use resources.  The rcar
> driver seems to have its own disconnect_sync() call so it'd work even
> with this change, but others wouldn't.
> 
> At least you need to call snd_card_disconnect_sync() at the first
> place, then release the rest at the second place.

Ahh, I didn't notice about busy state and async process.
Thank you for pointing it.

Thank you for your help !!
Best regards
---
Kuninori Morimoto
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [alsa-devel] [PATCH v3 06/19] ASoC: soc-core: add soc_unbind_dai_link()
  2019-11-13  4:37                 ` Kuninori Morimoto
  2019-11-13  5:57                   ` Takashi Iwai
@ 2019-11-13 16:05                   ` Pierre-Louis Bossart
  2019-11-14  0:18                     ` Kuninori Morimoto
  1 sibling, 1 reply; 60+ messages in thread
From: Pierre-Louis Bossart @ 2019-11-13 16:05 UTC (permalink / raw)
  To: Kuninori Morimoto; +Cc: Linux-ALSA, Mark Brown, Ranjani Sridharan



On 11/12/19 10:37 PM, Kuninori Morimoto wrote:
> 
> Hi Pierre-Louis
> 
> Thank you for your report
> 
>>>> +       for_each_rtd_components(rtd, rtdcom, component) {
>>>> +               pr_err("plb: %s processing component\n", __func__);
>>>> +               if (!component)
>>>> +                       pr_err("plb: %s component is NULL\n", __func__);
>>>
>>> Could you perhaps add traces of which components are being accessed at
>>> each stage?  We might want to go through and just add something like
>>> that in the code anyway to help figure things out.
>>
>> I tried to add more traces but couldn't triangulate on a clear issue,
>> and the traces have an Heisenbug effect.
>>
>> So I switched to higher-level code analysis: it turns out that
>> soc_dai_link_remove() routine is called from both topology and on card
>> cleanup.
>>
>> The patch 06/19 in this series essentially forces the pcm_runtimes to
>> be freed in both cases, so possibly twice for topology-managed
>> dailinks - or using information that's been freed already.
>>
>> I 'fixed' this by adding an additional parameter to avoid doing the
>> pcm runtime free from the topology (as was done before), and the
>> kernel oops goes away. My tests have been running for 45mn now, when
>> without change I get a kernel oops in less than 10-20 cycles (but
>> still more than apparently our CI tracks, something to improve).
>>
>> I pushed the code on GitHub to check if there are any negative points
>> reported by the Intel CI, should be complete shortly:
>> https://github.com/thesofproject/linux/pull/1469
>>
>> I am not sure the suggested fix is correct, I don't fully get what the
>> topology and card cleanups should do and how the work is split, if at
>> all.
> 
> BTW, I guess your kernel is appling this patch,
> but, is it correct ?
> 
> 	df95a16d2a9626dcfc3f2b3671c9b91fa076c997
> 	("ASoC: soc-core: fix RIP warning on card removal")

Sorry morimoto-san, I am not getting the question.

Are you asking if the patch is correct?

Or are you asking if the kernel used for this test include this patch? 
The answer is yes, this patch ("ASoC: soc-core: fix RIP warning on card 
removal") was merged by Mark and the SOF tree does use it, since we 
follow Mark's tree.

> If so, I think I could understand the issue.
> But, before explaining detail,
> I want to confirm that my solution is correct or not first.
> 
> Can you please check attached patch ?

Takashi's feedback seems to hint at problems with this patch, so will 
wait for further instructions here if you want me to test.
Thanks!

> Then, please remove above RIP warning patch.
> It is not clean patch, but OK to confirming, so far.
> I think
> 	- no kernel Oops
> 	- no RIP warning
> 
> Thank you for your help !!
> Best regards
> ---
> Kuninori Morimoto
> 
> 
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
> 
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [alsa-devel] [PATCH v3 06/19] ASoC: soc-core: add soc_unbind_dai_link()
  2019-11-13 16:05                   ` Pierre-Louis Bossart
@ 2019-11-14  0:18                     ` Kuninori Morimoto
  2019-11-14  1:03                       ` Kuninori Morimoto
  0 siblings, 1 reply; 60+ messages in thread
From: Kuninori Morimoto @ 2019-11-14  0:18 UTC (permalink / raw)
  To: Pierre-Louis Bossart; +Cc: Linux-ALSA, Mark Brown, Ranjani Sridharan


Hi Pierre-Louis

> > BTW, I guess your kernel is appling this patch,
> > but, is it correct ?
> > 
> > 	df95a16d2a9626dcfc3f2b3671c9b91fa076c997
> > 	("ASoC: soc-core: fix RIP warning on card removal")
> 
> Sorry morimoto-san, I am not getting the question.
> 
> Are you asking if the patch is correct?
> 
> Or are you asking if the kernel used for this test include this patch?
> The answer is yes, this patch ("ASoC: soc-core: fix RIP warning on
> card removal") was merged by Mark and the SOF tree does use it, since
> we follow Mark's tree.

Oops, sorry for my noise.
I thought you are using a little bit old kernel + extra patch + SOF tree.

> > If so, I think I could understand the issue.
> > But, before explaining detail,
> > I want to confirm that my solution is correct or not first.
> > 
> > Can you please check attached patch ?
> 
> Takashi's feedback seems to hint at problems with this patch, so will
> wait for further instructions here if you want me to test.
> Thanks!

OK, I will post patch as "RFC".
Because I can't test it.

Thank you for your help !!
Best regards
---
Kuninori Morimoto
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [alsa-devel] [PATCH v3 06/19] ASoC: soc-core: add soc_unbind_dai_link()
  2019-11-14  0:18                     ` Kuninori Morimoto
@ 2019-11-14  1:03                       ` Kuninori Morimoto
  2019-11-16  0:19                         ` Pierre-Louis Bossart
  0 siblings, 1 reply; 60+ messages in thread
From: Kuninori Morimoto @ 2019-11-14  1:03 UTC (permalink / raw)
  To: Pierre-Louis Bossart, Linux-ALSA, Mark Brown, Ranjani Sridharan
  Cc: Takashi Iwai


Hi Pierre-Louis, again
Cc Takashi-san

> > Takashi's feedback seems to hint at problems with this patch, so will
> > wait for further instructions here if you want me to test.
> > Thanks!
> 
> OK, I will post patch as "RFC".
> Because I can't test it.

I try to explain it here, not at RFC :)

I'm not 100% sure detail of SOF / topology, but in my understanding,
topology want to add *extra* dai_link by using snd_soc_add_dai_link().
And it will be removed by snd_soc_romove_dai_link().

Before, this snd_soc_add/remove_dai_link() and/or its related
functions are unbalanced.
Now, these are balance-uping, and it finds the random operation issue.

topology will call snd_soc_remove_dai_link() via (A).
In my understanding, currently, SOF and skylake are calling it.

	static void soc_cleanup_card_resources(struct snd_soc_card *card)
	{
		struct snd_soc_dai_link *link, *_link;

		/* This should be called before snd_card_free() */
	(A)	soc_remove_link_components(card);

		/* free the ALSA card at first; this syncs with pending operations */
		if (card->snd_card) {
	(B)		snd_card_free(card->snd_card);
			card->snd_card = NULL;
		}

		/* remove and free each DAI */
	(X)	soc_remove_link_dais(card);

		for_each_card_links_safe(card, link, _link)
	(C)		snd_soc_remove_dai_link(card, link);

		...
	}

At (A), topology calls snd_soc_remove_dai_link().
Then topology rtd, and its related all data are freed.

Next, (B) is called, and then, pcm->private_free = soc_pcm_private_free()
is called.

	static void soc_pcm_private_free(struct snd_pcm *pcm)
	{
		struct snd_soc_pcm_runtime *rtd = pcm->private_data;

		/* need to sync the delayed work before releasing resources */
		flush_delayed_work(&rtd->delayed_work);
		snd_soc_pcm_component_free(pcm);
	}

Here, it gets rtd via pcm->private_data.
But, topology related rtd are already freed at (A).
Above both flush_delayed_work() and snd_soc_pcm_component_free()
are using rtd via pcm->private_data.
I guess, your kernel access to already freed memory, and get Oops.

Normal sound card is freeing rtd at (C), thus no damage.

These flush_delayed_work() and snd_soc_pcm_component_free()
are rtd related data's finalization.
If so, these should be called when rtd was freed,
not sound card was freed.
It is very natural and understandable, I think.

In other words, pcm->private_free = soc_pcm_private_free()
is no longer needed.

But, here one other issue exist.
If we do above idea, there is zero chance to call
soc_remove_dai() to topology related dai at (X).
Because (A) removes rtd from card too, and,
(X) is based on card connected rtd.

This means, (X) need to be called before (C) (= for normal sound)
and (A) (= for topology).

Now, I want to focus this patch which is the reason why
snd_card_free() = (B) is located there.

	4efda5f2130da033aeedc5b3205569893b910de2
	("ASoC: Fix use-after-free at card unregistration")

Original snd_card_free() was called last of this function.
But moved to top to avoid use-after-free issue.
The issue was happen at soc_pcm_free() which was pcm->private_free,
today it is updated to soc_pcm_private_free().

In other words, (B) need to be called before (C) (= for normal sound)
and (A) (= for topology), because it needs (not yet freed) rtd.
But, (A) need to be called before (B),
because it needs card->snd_card pointer.

If we call flush_delayed_work() and snd_soc_pcm_component_free()
(= same as soc_pcm_private_free()) when rtd was freed (= (C), (A)),
there is no reason to call snd_card_free() at top of this function.
It can be called end of this function, again.

But, in such case, it will likely break unbind again, as Takashi-san said.
when unbind is performed in a busy state, the code may release still-in-use resources.
At least we need to call snd_card_disconnect_sync() at the first place.

The final code can be...

	static void soc_cleanup_card_resources(struct snd_soc_card *card)
	{
		struct snd_soc_dai_link *link, *_link;

		if (card->snd_card)
	(Z)		snd_card_disconnect_sync(card->snd_card);

	(X)	soc_remove_link_dais(card);
	(A)	soc_remove_link_components(card);

		for_each_card_links_safe(card, link, _link)
	(C)		snd_soc_remove_dai_link(card, link);

		...
		if (card->snd_card) {
	(B)		snd_card_free(card->snd_card);
			card->snd_card = NULL;
		}
	}

To avoid release still-in-use resources,
call snd_card_disconnect_sync() at (Z).

(X) is called for both non-topology and topology.

    topology removes rtd via (A), and
non topology removes rtd via (C).

And snd_card_free() is no longer related to
use-after-free issue. Thus, (B) is OK.

But, these are just my guess.
It works for me, but I can't re-produce the issue.

Below is the patch for "testing".

-------------
diff --git a/include/sound/soc-component.h b/include/sound/soc-component.h
index 6aa3ecb..cf726a5 100644
--- a/include/sound/soc-component.h
+++ b/include/sound/soc-component.h
@@ -413,6 +413,6 @@ struct page *snd_soc_pcm_component_page(struct snd_pcm_substream *substream,
 int snd_soc_pcm_component_mmap(struct snd_pcm_substream *substream,
 			       struct vm_area_struct *vma);
 int snd_soc_pcm_component_new(struct snd_pcm *pcm);
-void snd_soc_pcm_component_free(struct snd_pcm *pcm);
+void snd_soc_pcm_component_free(struct snd_soc_pcm_runtime *rtd);
 
 #endif /* __SOC_COMPONENT_H */
diff --git a/sound/soc/soc-component.c b/sound/soc/soc-component.c
index 98ef066..195979f 100644
--- a/sound/soc/soc-component.c
+++ b/sound/soc/soc-component.c
@@ -516,13 +516,12 @@ int snd_soc_pcm_component_new(struct snd_pcm *pcm)
 	return 0;
 }
 
-void snd_soc_pcm_component_free(struct snd_pcm *pcm)
+void snd_soc_pcm_component_free(struct snd_soc_pcm_runtime *rtd)
 {
-	struct snd_soc_pcm_runtime *rtd = pcm->private_data;
 	struct snd_soc_rtdcom_list *rtdcom;
 	struct snd_soc_component *component;
 
 	for_each_rtd_components(rtd, rtdcom, component)
 		if (component->driver->pcm_destruct)
-			component->driver->pcm_destruct(component, pcm);
+			component->driver->pcm_destruct(component, rtd->pcm);
 }
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 92260a9..42d87bb 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -419,6 +419,9 @@ static void soc_free_pcm_runtime(struct snd_soc_pcm_runtime *rtd)
 
 	list_del(&rtd->list);
 
+	flush_delayed_work(&rtd->delayed_work);
+	snd_soc_pcm_component_free(rtd);
+
 	/*
 	 * we don't need to call kfree() for rtd->dev
 	 * see
@@ -1944,17 +1947,12 @@ static void soc_cleanup_card_resources(struct snd_soc_card *card)
 {
 	struct snd_soc_dai_link *link, *_link;
 
-	/* This should be called before snd_card_free() */
-	soc_remove_link_components(card);
-
-	/* free the ALSA card at first; this syncs with pending operations */
-	if (card->snd_card) {
-		snd_card_free(card->snd_card);
-		card->snd_card = NULL;
-	}
+	if (card->snd_card)
+		snd_card_disconnect_sync(card->snd_card);
 
 	/* remove and free each DAI */
 	soc_remove_link_dais(card);
+	soc_remove_link_components(card);
 
 	for_each_card_links_safe(card, link, _link)
 		snd_soc_remove_dai_link(card, link);
@@ -1969,6 +1967,11 @@ static void soc_cleanup_card_resources(struct snd_soc_card *card)
 	/* remove the card */
 	if (card->remove)
 		card->remove(card);
+
+	if (card->snd_card) {
+		snd_card_free(card->snd_card);
+		card->snd_card = NULL;
+	}
 }
 
 static int snd_soc_bind_card(struct snd_soc_card *card)
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index 4bf71e3..6e24f0a5 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -2892,15 +2892,6 @@ static int dpcm_fe_dai_close(struct snd_pcm_substream *fe_substream)
 	return ret;
 }
 
-static void soc_pcm_private_free(struct snd_pcm *pcm)
-{
-	struct snd_soc_pcm_runtime *rtd = pcm->private_data;
-
-	/* need to sync the delayed work before releasing resources */
-	flush_delayed_work(&rtd->delayed_work);
-	snd_soc_pcm_component_free(pcm);
-}
-
 /* create a new pcm */
 int soc_new_pcm(struct snd_soc_pcm_runtime *rtd, int num)
 {
@@ -3042,7 +3033,6 @@ int soc_new_pcm(struct snd_soc_pcm_runtime *rtd, int num)
 		return ret;
 	}
 
-	pcm->private_free = soc_pcm_private_free;
 	pcm->no_device_suspend = true;
 out:
 	dev_info(rtd->card->dev, "%s <-> %s mapping ok\n",



Thank you for your help !!
Best regards
---
Kuninori Morimoto
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* Re: [alsa-devel] [PATCH v3 06/19] ASoC: soc-core: add soc_unbind_dai_link()
  2019-11-14  1:03                       ` Kuninori Morimoto
@ 2019-11-16  0:19                         ` Pierre-Louis Bossart
  2019-11-18  0:47                           ` Kuninori Morimoto
  0 siblings, 1 reply; 60+ messages in thread
From: Pierre-Louis Bossart @ 2019-11-16  0:19 UTC (permalink / raw)
  To: Kuninori Morimoto, Linux-ALSA, Mark Brown, Ranjani Sridharan; +Cc: Takashi Iwai


> But, these are just my guess.
> It works for me, but I can't re-produce the issue.
> 
> Below is the patch for "testing".

Morimoto-san, I haven't fully tested the logic and all the long 
description, but the patch suggested seems to work on all the platforms 
I tested on - and actually improves things on Baytrail/Cherrytrail.

I haven't seen any oopses in several hours now and no regression 
reported by Intel CI https://github.com/thesofproject/linux/pull/1504

Let's see what others think or tested.

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

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [alsa-devel] [PATCH v3 06/19] ASoC: soc-core: add soc_unbind_dai_link()
  2019-11-16  0:19                         ` Pierre-Louis Bossart
@ 2019-11-18  0:47                           ` Kuninori Morimoto
  2019-11-18 15:14                             ` Pierre-Louis Bossart
  0 siblings, 1 reply; 60+ messages in thread
From: Kuninori Morimoto @ 2019-11-18  0:47 UTC (permalink / raw)
  To: Pierre-Louis Bossart
  Cc: Takashi Iwai, Linux-ALSA, Mark Brown, Ranjani Sridharan


Hi Pierre-Louis

Thank you for testing

> > But, these are just my guess.
> > It works for me, but I can't re-produce the issue.
> > 
> > Below is the patch for "testing".
> 
> Morimoto-san, I haven't fully tested the logic and all the long
> description, but the patch suggested seems to work on all the
> platforms I tested on - and actually improves things on
> Baytrail/Cherrytrail.
> 
> I haven't seen any oopses in several hours now and no regression
> reported by Intel CI https://github.com/thesofproject/linux/pull/1504

Wow !! Nice to know !!
I'm very happy about that !!

OK, I will post the patch officially with your tested by.

> Let's see what others think or tested.

Yes, it is nice idea

Thank you for your help !!
Best regards
---
Kuninori Morimoto
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [alsa-devel] [PATCH v3 06/19] ASoC: soc-core: add soc_unbind_dai_link()
  2019-11-18  0:47                           ` Kuninori Morimoto
@ 2019-11-18 15:14                             ` Pierre-Louis Bossart
  0 siblings, 0 replies; 60+ messages in thread
From: Pierre-Louis Bossart @ 2019-11-18 15:14 UTC (permalink / raw)
  To: Kuninori Morimoto; +Cc: Takashi Iwai, Linux-ALSA, Mark Brown, Ranjani Sridharan



On 11/17/19 6:47 PM, Kuninori Morimoto wrote:
> 
> Hi Pierre-Louis
> 
> Thank you for testing
> 
>>> But, these are just my guess.
>>> It works for me, but I can't re-produce the issue.
>>>
>>> Below is the patch for "testing".
>>
>> Morimoto-san, I haven't fully tested the logic and all the long
>> description, but the patch suggested seems to work on all the
>> platforms I tested on - and actually improves things on
>> Baytrail/Cherrytrail.
>>
>> I haven't seen any oopses in several hours now and no regression
>> reported by Intel CI https://github.com/thesofproject/linux/pull/1504
> 
> Wow !! Nice to know !!
> I'm very happy about that !!
> 
> OK, I will post the patch officially with your tested by.
> 
>> Let's see what others think or tested.

the only issue we found in additional testing is that we need to insert 
a ~20s delay after each of module insertion/removal sequences, or we get 
stuck in at some point. That is a likely sign of delayed work not 
properly canceled - it may however be a different topic altogether, I 
saw the same sighting with my initial patch which reverted the "ASoC: 
soc-core: add soc_unbind_dai_link()" patch.

Others at Intel redid my tests and also found that the module 
load/unload tests worked across the board on CI devices.

from the Intel side we're good.
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 60+ messages in thread

end of thread, other threads:[~2019-11-18 15:46 UTC | newest]

Thread overview: 60+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-05  6:45 [alsa-devel] [PATCH v3 00/19] ASoC: soc-core cleanup - step 4 Kuninori Morimoto
2019-11-05  6:45 ` [alsa-devel] [PATCH v3 01/19] ASoC: soc-core: move soc_init_dai_link() Kuninori Morimoto
2019-11-05 23:51   ` [alsa-devel] Applied "ASoC: soc-core: move soc_init_dai_link()" to the asoc tree Mark Brown
2019-11-05  6:45 ` [alsa-devel] [PATCH v3 02/19] ASoC: soc-core: tidyup soc_init_dai_link() Kuninori Morimoto
2019-11-05 23:51   ` [alsa-devel] Applied "ASoC: soc-core: tidyup soc_init_dai_link()" to the asoc tree Mark Brown
2019-11-11 14:43   ` [alsa-devel] [PATCH v3 02/19] ASoC: soc-core: tidyup soc_init_dai_link() Jon Hunter
2019-11-12  0:24     ` Kuninori Morimoto
2019-11-12 10:51       ` Jon Hunter
2019-11-13  0:29         ` Kuninori Morimoto
2019-11-05  6:46 ` [alsa-devel] [PATCH v3 03/19] ASoC: soc-core: typo fix at soc_dai_link_sanity_check() Kuninori Morimoto
2019-11-05 23:51   ` [alsa-devel] Applied "ASoC: soc-core: typo fix at soc_dai_link_sanity_check()" to the asoc tree Mark Brown
2019-11-05  6:46 ` [alsa-devel] [PATCH v3 04/19] ASoC: soc-core: remove duplicated soc_is_dai_link_bound() Kuninori Morimoto
2019-11-05 23:51   ` [alsa-devel] Applied "ASoC: soc-core: remove duplicated soc_is_dai_link_bound()" to the asoc tree Mark Brown
2019-11-05  6:46 ` [alsa-devel] [PATCH v3 05/19] ASoC: soc-core: call soc_bind_dai_link() under snd_soc_add_dai_link() Kuninori Morimoto
2019-11-05 23:51   ` [alsa-devel] Applied "ASoC: soc-core: call soc_bind_dai_link() under snd_soc_add_dai_link()" to the asoc tree Mark Brown
2019-11-05  6:46 ` [alsa-devel] [PATCH v3 06/19] ASoC: soc-core: add soc_unbind_dai_link() Kuninori Morimoto
2019-11-05 23:51   ` [alsa-devel] Applied "ASoC: soc-core: add soc_unbind_dai_link()" to the asoc tree Mark Brown
2019-11-12  5:21   ` [alsa-devel] [PATCH v3 06/19] ASoC: soc-core: add soc_unbind_dai_link() Pierre-Louis Bossart
2019-11-12  5:37     ` Kuninori Morimoto
2019-11-12  5:50       ` Kuninori Morimoto
2019-11-12  6:42         ` Kuninori Morimoto
2019-11-12 17:11           ` Pierre-Louis Bossart
2019-11-12 19:03             ` Mark Brown
2019-11-12 21:08               ` Pierre-Louis Bossart
2019-11-13  4:37                 ` Kuninori Morimoto
2019-11-13  5:57                   ` Takashi Iwai
2019-11-13  6:33                     ` Kuninori Morimoto
2019-11-13 16:05                   ` Pierre-Louis Bossart
2019-11-14  0:18                     ` Kuninori Morimoto
2019-11-14  1:03                       ` Kuninori Morimoto
2019-11-16  0:19                         ` Pierre-Louis Bossart
2019-11-18  0:47                           ` Kuninori Morimoto
2019-11-18 15:14                             ` Pierre-Louis Bossart
2019-11-05  6:46 ` [alsa-devel] [PATCH v3 07/19] ASoC: soc-core: move snd_soc_lookup_component() Kuninori Morimoto
2019-11-05 23:51   ` [alsa-devel] Applied "ASoC: soc-core: move snd_soc_lookup_component()" to the asoc tree Mark Brown
2019-11-05  6:46 ` [alsa-devel] [PATCH v3 08/19] ASoC: soc-core: tidyup snd_soc_lookup_component() Kuninori Morimoto
2019-11-05 23:51   ` [alsa-devel] Applied "ASoC: soc-core: tidyup snd_soc_lookup_component()" to the asoc tree Mark Brown
2019-11-05  6:46 ` [alsa-devel] [PATCH v3 09/19] ASoC: soc-core: add snd_soc_del_component_unlocked() Kuninori Morimoto
2019-11-05 23:51   ` [alsa-devel] Applied "ASoC: soc-core: add snd_soc_del_component_unlocked()" to the asoc tree Mark Brown
2019-11-05  6:46 ` [alsa-devel] [PATCH v3 10/19] ASoC: soc-core: remove snd_soc_component_add/del() Kuninori Morimoto
2019-11-05 23:51   ` [alsa-devel] Applied "ASoC: soc-core: remove snd_soc_component_add/del()" to the asoc tree Mark Brown
2019-11-05  6:46 ` [alsa-devel] [PATCH v3 11/19] ASoC: soc-core: use snd_soc_lookup_component() at snd_soc_unregister_component() Kuninori Morimoto
2019-11-05 23:51   ` [alsa-devel] Applied "ASoC: soc-core: use snd_soc_lookup_component() at snd_soc_unregister_component()" to the asoc tree Mark Brown
2019-11-05  6:46 ` [alsa-devel] [PATCH v3 12/19] ASoC: soc-core: move snd_soc_register_dai() Kuninori Morimoto
2019-11-05 23:51   ` [alsa-devel] Applied "ASoC: soc-core: move snd_soc_register_dai()" to the asoc tree Mark Brown
2019-11-05  6:47 ` [alsa-devel] [PATCH v3 13/19] ASoC: soc-core: move snd_soc_unregister_dais() Kuninori Morimoto
2019-11-05 23:51   ` [alsa-devel] Applied "ASoC: soc-core: move snd_soc_unregister_dais()" to the asoc tree Mark Brown
2019-11-05  6:47 ` [alsa-devel] [PATCH v3 14/19] ASoC: soc-core: add snd_soc_unregister_dai() Kuninori Morimoto
2019-11-05 23:51   ` [alsa-devel] Applied "ASoC: soc-core: add snd_soc_unregister_dai()" to the asoc tree Mark Brown
2019-11-05  6:47 ` [alsa-devel] [PATCH v3 15/19] ASoC: soc-core: have legacy_dai_naming at snd_soc_register_dai() Kuninori Morimoto
2019-11-05 23:51   ` [alsa-devel] Applied "ASoC: soc-core: have legacy_dai_naming at snd_soc_register_dai()" to the asoc tree Mark Brown
2019-11-05  6:47 ` [alsa-devel] [PATCH v3 16/19] ASoC: soc-core: don't call snd_soc_dapm_new_dai_widgets() at snd_soc_register_dai() Kuninori Morimoto
2019-11-05 23:51   ` [alsa-devel] Applied "ASoC: soc-core: don't call snd_soc_dapm_new_dai_widgets() at snd_soc_register_dai()" to the asoc tree Mark Brown
2019-11-05  6:47 ` [alsa-devel] [PATCH v3 17/19] ASoC: soc-core: call snd_soc_register_dai() from snd_soc_register_dais() Kuninori Morimoto
2019-11-05 23:51   ` [alsa-devel] Applied "ASoC: soc-core: call snd_soc_register_dai() from snd_soc_register_dais()" to the asoc tree Mark Brown
2019-11-05  6:47 ` [alsa-devel] [PATCH v3 18/19] ASoC: soc-core: remove topology specific operation Kuninori Morimoto
2019-11-05 23:51   ` [alsa-devel] Applied "ASoC: soc-core: remove topology specific operation" to the asoc tree Mark Brown
2019-11-05  6:47 ` [alsa-devel] [PATCH v3 19/19] ASoC: soc.h: dobj is used only when SND_SOC_TOPOLOGY Kuninori Morimoto
2019-11-05 23:51   ` [alsa-devel] Applied "ASoC: soc.h: dobj is used only when SND_SOC_TOPOLOGY" to the asoc tree Mark Brown
2019-11-05 16:18 ` [alsa-devel] [PATCH v3 00/19] ASoC: soc-core cleanup - step 4 Ranjani Sridharan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).