[PATCH 0/4] ASoC: simple-card / audio-graph re-cleanup

[PATCH 0/4] ASoC: simple-card / audio-graph re-cleanup

[Kuninori Morimoto]

Hi Mark, Guillaume

KernelCI had reported that my previous cleanup patches for simple-card / audio-graph
broke kontron-sl28-var3-ads2 sound card probing.
It reported that it is using same name for dailink->name.

At first I thought that the issue was fsl,vf610-sai doesn't have .name on driver.
But real issue was that cpu->dai_name removed on simple_parse_node(),
and dailink->name was based on it.
We need to set/get dailink->name first, and call simple_parse_node().
This patches are do it.
audio-graph has same issue. [4/4] patch is for it.

I hope these patches has no issues on kontron-sl28-var3-ads2.

Link: https://lore.kernel.org/r/87h7k0i437.wl-kuninori.morimoto.gx@renesas.com
Link: https://lore.kernel.org/r/20210423175318.13990-1-broonie@kernel.org
Link: https://lore.kernel.org/r/3ca62063-41b4-c25b-a7bc-8a8160e7b684@collabora.com

Kuninori Morimoto (4):
  ASoC: simple-card: add simple_parse_node()
  ASoC: simple-card: add simple_link_init()
  ASoC: audio-graph: tidyup graph_dai_link_of_dpcm()
  ASoC: audio-graph: tidyup dai_name seting timing

 sound/soc/generic/audio-graph-card.c |  48 +++----
 sound/soc/generic/simple-card.c      | 187 ++++++++++++++-------------
 2 files changed, 122 insertions(+), 113 deletions(-)

-- 
2.25.1

[PATCH 1/4] ASoC: simple-card: add simple_parse_node()

[Kuninori Morimoto]

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

Parse dai/tdm/clk are common for both CPU/Codec node.
This patch creates simple_parse_node() for it and share the code.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/generic/simple-card.c | 145 ++++++++++++++++----------------
 1 file changed, 74 insertions(+), 71 deletions(-)

diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c
index a1373be4558f..5288aacfc628 100644
--- a/sound/soc/generic/simple-card.c
+++ b/sound/soc/generic/simple-card.c
@@ -93,12 +93,11 @@ static void simple_parse_convert(struct device *dev,
 }
 
 static void simple_parse_mclk_fs(struct device_node *top,
-				 struct device_node *cpu,
-				 struct device_node *codec,
+				 struct device_node *np,
 				 struct simple_dai_props *props,
 				 char *prefix)
 {
-	struct device_node *node = of_get_parent(cpu);
+	struct device_node *node = of_get_parent(np);
 	char prop[128];
 
 	snprintf(prop, sizeof(prop), "%smclk-fs", PREFIX);
@@ -106,12 +105,57 @@ static void simple_parse_mclk_fs(struct device_node *top,
 
 	snprintf(prop, sizeof(prop), "%smclk-fs", prefix);
 	of_property_read_u32(node,	prop, &props->mclk_fs);
-	of_property_read_u32(cpu,	prop, &props->mclk_fs);
-	of_property_read_u32(codec,	prop, &props->mclk_fs);
+	of_property_read_u32(np,	prop, &props->mclk_fs);
 
 	of_node_put(node);
 }
 
+static int simple_parse_node(struct asoc_simple_priv *priv,
+			     struct device_node *np,
+			     struct link_info *li,
+			     char *prefix,
+			     int is_cpu)
+{
+	struct device *dev = simple_priv_to_dev(priv);
+	struct device_node *top = dev->of_node;
+	struct snd_soc_dai_link *dai_link = simple_priv_to_link(priv, li->link);
+	struct simple_dai_props *dai_props = simple_priv_to_props(priv, li->link);
+	struct snd_soc_dai_link_component *dlc;
+	struct asoc_simple_dai *dai;
+	int ret, single = 0;
+
+	if (is_cpu) {
+		dlc = asoc_link_to_cpu(dai_link, 0);
+		dai = simple_props_to_dai_cpu(dai_props, 0);
+	} else {
+		dlc = asoc_link_to_codec(dai_link, 0);
+		dai = simple_props_to_dai_codec(dai_props, 0);
+	}
+
+	simple_parse_mclk_fs(top, np, dai_props, prefix);
+
+	ret = asoc_simple_parse_dai(np, dlc, &single);
+	if (ret)
+		return ret;
+
+	ret = asoc_simple_parse_clk(dev, np, dai, dlc);
+	if (ret)
+		return ret;
+
+	ret = asoc_simple_parse_tdm(np, dai);
+	if (ret)
+		return ret;
+
+	if (is_cpu) {
+		struct snd_soc_dai_link_component *platforms = asoc_link_to_platform(dai_link, 0);
+
+		asoc_simple_canonicalize_cpu(dlc, single);
+		asoc_simple_canonicalize_platform(platforms, dlc);
+	}
+
+	return 0;
+}
+
 static int simple_dai_link_of_dpcm(struct asoc_simple_priv *priv,
 				   struct device_node *np,
 				   struct device_node *codec,
@@ -121,10 +165,8 @@ static int simple_dai_link_of_dpcm(struct asoc_simple_priv *priv,
 	struct device *dev = simple_priv_to_dev(priv);
 	struct snd_soc_dai_link *dai_link = simple_priv_to_link(priv, li->link);
 	struct simple_dai_props *dai_props = simple_priv_to_props(priv, li->link);
-	struct asoc_simple_dai *dai;
 	struct snd_soc_dai_link_component *cpus = asoc_link_to_cpu(dai_link, 0);
 	struct snd_soc_dai_link_component *codecs = asoc_link_to_codec(dai_link, 0);
-	struct snd_soc_dai_link_component *platforms = asoc_link_to_platform(dai_link, 0);
 	struct device_node *top = dev->of_node;
 	struct device_node *node = of_get_parent(np);
 	char *prefix = "";
@@ -132,39 +174,30 @@ static int simple_dai_link_of_dpcm(struct asoc_simple_priv *priv,
 
 	dev_dbg(dev, "link_of DPCM (%pOF)\n", np);
 
-	li->link++;
-
 	/* For single DAI link & old style of DT node */
 	if (is_top)
 		prefix = PREFIX;
 
 	if (li->cpu) {
-		int is_single_links = 0;
-
 		/* Codec is dummy */
 
 		/* FE settings */
 		dai_link->dynamic		= 1;
 		dai_link->dpcm_merged_format	= 1;
 
-		dai = simple_props_to_dai_cpu(dai_props, 0);
-
-		ret = asoc_simple_parse_dai(np, cpus, &is_single_links);
-		if (ret)
-			goto out_put_node;
-
-		ret = asoc_simple_parse_clk(dev, np, dai, cpus);
-		if (ret < 0)
-			goto out_put_node;
-
+		/*
+		 * next simple_parse_node() might remove cpus->dai_name.
+		 * set dailink_name before it.
+		 */
 		ret = asoc_simple_set_dailink_name(dev, dai_link,
 						   "fe.%s",
 						   cpus->dai_name);
 		if (ret < 0)
 			goto out_put_node;
 
-		asoc_simple_canonicalize_cpu(cpus, is_single_links);
-		asoc_simple_canonicalize_platform(platforms, cpus);
+		ret = simple_parse_node(priv, np, li, prefix, 1);
+		if (ret < 0)
+			goto out_put_node;
 	} else {
 		struct snd_soc_codec_conf *cconf;
 
@@ -174,23 +207,18 @@ static int simple_dai_link_of_dpcm(struct asoc_simple_priv *priv,
 		dai_link->no_pcm		= 1;
 		dai_link->be_hw_params_fixup	= asoc_simple_be_hw_params_fixup;
 
-		dai	= simple_props_to_dai_codec(dai_props, 0);
 		cconf	= simple_props_to_codec_conf(dai_props, 0);
 
-		ret = asoc_simple_parse_dai(np, codecs, NULL);
-		if (ret < 0)
-			goto out_put_node;
-
-		ret = asoc_simple_parse_clk(dev, np, dai, codecs);
-		if (ret < 0)
-			goto out_put_node;
-
 		ret = asoc_simple_set_dailink_name(dev, dai_link,
 						   "be.%s",
 						   codecs->dai_name);
 		if (ret < 0)
 			goto out_put_node;
 
+		ret = simple_parse_node(priv, np, li, prefix, 0);
+		if (ret < 0)
+			goto out_put_node;
+
 		/* check "prefix" from top node */
 		snd_soc_of_parse_node_prefix(top, cconf, codecs->of_node,
 					      PREFIX "prefix");
@@ -201,11 +229,6 @@ static int simple_dai_link_of_dpcm(struct asoc_simple_priv *priv,
 	}
 
 	simple_parse_convert(dev, np, &dai_props->adata);
-	simple_parse_mclk_fs(top, np, codec, dai_props, prefix);
-
-	ret = asoc_simple_parse_tdm(np, dai);
-	if (ret)
-		goto out_put_node;
 
 	ret = asoc_simple_parse_daifmt(dev, node, codec,
 				       prefix, &dai_link->dai_fmt);
@@ -218,6 +241,8 @@ static int simple_dai_link_of_dpcm(struct asoc_simple_priv *priv,
 	dai_link->init			= asoc_simple_dai_init;
 
 out_put_node:
+	li->link++;
+
 	of_node_put(node);
 	return ret;
 }
@@ -230,23 +255,18 @@ static int simple_dai_link_of(struct asoc_simple_priv *priv,
 {
 	struct device *dev = simple_priv_to_dev(priv);
 	struct snd_soc_dai_link *dai_link = simple_priv_to_link(priv, li->link);
-	struct simple_dai_props *dai_props = simple_priv_to_props(priv, li->link);
-	struct asoc_simple_dai *cpu_dai	= simple_props_to_dai_cpu(dai_props, 0);
-	struct asoc_simple_dai *codec_dai = simple_props_to_dai_codec(dai_props, 0);
 	struct snd_soc_dai_link_component *cpus = asoc_link_to_cpu(dai_link, 0);
 	struct snd_soc_dai_link_component *codecs = asoc_link_to_codec(dai_link, 0);
 	struct snd_soc_dai_link_component *platforms = asoc_link_to_platform(dai_link, 0);
-	struct device_node *top = dev->of_node;
 	struct device_node *cpu = NULL;
 	struct device_node *node = NULL;
 	struct device_node *plat = NULL;
 	char prop[128];
 	char *prefix = "";
-	int ret, single_cpu = 0;
+	int ret;
 
 	cpu  = np;
 	node = of_get_parent(np);
-	li->link++;
 
 	dev_dbg(dev, "link_of (%pOF)\n", node);
 
@@ -262,53 +282,36 @@ static int simple_dai_link_of(struct asoc_simple_priv *priv,
 	if (ret < 0)
 		goto dai_link_of_err;
 
-	simple_parse_mclk_fs(top, cpu, codec, dai_props, prefix);
-
-	ret = asoc_simple_parse_dai(cpu, cpus, &single_cpu);
-	if (ret < 0)
-		goto dai_link_of_err;
-
-	ret = asoc_simple_parse_dai(codec, codecs, NULL);
-	if (ret < 0)
-		goto dai_link_of_err;
-
-	ret = asoc_simple_parse_dai(plat, platforms, NULL);
-	if (ret < 0)
-		goto dai_link_of_err;
-
-	ret = asoc_simple_parse_tdm(cpu, cpu_dai);
-	if (ret < 0)
-		goto dai_link_of_err;
-
-	ret = asoc_simple_parse_tdm(codec, codec_dai);
+	/*
+	 * next simple_parse_node() might remove cpus->dai_name.
+	 * set dailink_name before it.
+	 */
+	ret = asoc_simple_set_dailink_name(dev, dai_link, "%s-%s",
+					   cpus->dai_name, codecs->dai_name);
 	if (ret < 0)
 		goto dai_link_of_err;
 
-	ret = asoc_simple_parse_clk(dev, cpu, cpu_dai, cpus);
+	ret = simple_parse_node(priv, cpu, li, prefix, 1);
 	if (ret < 0)
 		goto dai_link_of_err;
 
-	ret = asoc_simple_parse_clk(dev, codec, codec_dai, codecs);
+	ret = simple_parse_node(priv, codec, li, prefix, 0);
 	if (ret < 0)
 		goto dai_link_of_err;
 
-	ret = asoc_simple_set_dailink_name(dev, dai_link,
-					   "%s-%s",
-					   cpus->dai_name,
-					   codecs->dai_name);
+	ret = asoc_simple_parse_dai(plat, platforms, NULL);
 	if (ret < 0)
 		goto dai_link_of_err;
 
 	dai_link->ops = &simple_ops;
 	dai_link->init = asoc_simple_dai_init;
 
-	asoc_simple_canonicalize_cpu(cpus, single_cpu);
-	asoc_simple_canonicalize_platform(platforms, cpus);
-
 dai_link_of_err:
 	of_node_put(plat);
 	of_node_put(node);
 
+	li->link++;
+
 	return ret;
 }
 
-- 
2.25.1

[PATCH 2/4] ASoC: simple-card: add simple_link_init()

[Kuninori Morimoto]

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

This patch adds simple_link_init() and share dai_link setting code.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/generic/simple-card.c | 62 +++++++++++++++++----------------
 1 file changed, 32 insertions(+), 30 deletions(-)

diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c
index 5288aacfc628..c9edde04a4a9 100644
--- a/sound/soc/generic/simple-card.c
+++ b/sound/soc/generic/simple-card.c
@@ -156,6 +156,28 @@ static int simple_parse_node(struct asoc_simple_priv *priv,
 	return 0;
 }
 
+static int simple_link_init(struct asoc_simple_priv *priv,
+			    struct device_node *node,
+			    struct device_node *codec,
+			    struct link_info *li,
+			    char *prefix,
+			    char *name)
+{
+	struct device *dev = simple_priv_to_dev(priv);
+	struct snd_soc_dai_link *dai_link = simple_priv_to_link(priv, li->link);
+	int ret;
+
+	ret = asoc_simple_parse_daifmt(dev, node, codec,
+				       prefix, &dai_link->dai_fmt);
+	if (ret < 0)
+		return 0;
+
+	dai_link->init			= asoc_simple_dai_init;
+	dai_link->ops			= &simple_ops;
+
+	return asoc_simple_set_dailink_name(dev, dai_link, name);
+}
+
 static int simple_dai_link_of_dpcm(struct asoc_simple_priv *priv,
 				   struct device_node *np,
 				   struct device_node *codec,
@@ -170,6 +192,7 @@ static int simple_dai_link_of_dpcm(struct asoc_simple_priv *priv,
 	struct device_node *top = dev->of_node;
 	struct device_node *node = of_get_parent(np);
 	char *prefix = "";
+	char dai_name[64];
 	int ret;
 
 	dev_dbg(dev, "link_of DPCM (%pOF)\n", np);
@@ -187,13 +210,9 @@ static int simple_dai_link_of_dpcm(struct asoc_simple_priv *priv,
 
 		/*
 		 * next simple_parse_node() might remove cpus->dai_name.
-		 * set dailink_name before it.
+		 * get dai_name before it.
 		 */
-		ret = asoc_simple_set_dailink_name(dev, dai_link,
-						   "fe.%s",
-						   cpus->dai_name);
-		if (ret < 0)
-			goto out_put_node;
+		snprintf(dai_name, sizeof(dai_name), "fe.%s", cpus->dai_name);
 
 		ret = simple_parse_node(priv, np, li, prefix, 1);
 		if (ret < 0)
@@ -209,11 +228,7 @@ static int simple_dai_link_of_dpcm(struct asoc_simple_priv *priv,
 
 		cconf	= simple_props_to_codec_conf(dai_props, 0);
 
-		ret = asoc_simple_set_dailink_name(dev, dai_link,
-						   "be.%s",
-						   codecs->dai_name);
-		if (ret < 0)
-			goto out_put_node;
+		snprintf(dai_name, sizeof(dai_name), "be.%s", codecs->dai_name);
 
 		ret = simple_parse_node(priv, np, li, prefix, 0);
 		if (ret < 0)
@@ -230,15 +245,9 @@ static int simple_dai_link_of_dpcm(struct asoc_simple_priv *priv,
 
 	simple_parse_convert(dev, np, &dai_props->adata);
 
-	ret = asoc_simple_parse_daifmt(dev, node, codec,
-				       prefix, &dai_link->dai_fmt);
-	if (ret < 0)
-		goto out_put_node;
-
 	snd_soc_dai_link_set_capabilities(dai_link);
 
-	dai_link->ops			= &simple_ops;
-	dai_link->init			= asoc_simple_dai_init;
+	ret = simple_link_init(priv, node, codec, li, prefix, dai_name);
 
 out_put_node:
 	li->link++;
@@ -261,6 +270,7 @@ static int simple_dai_link_of(struct asoc_simple_priv *priv,
 	struct device_node *cpu = NULL;
 	struct device_node *node = NULL;
 	struct device_node *plat = NULL;
+	char dai_name[64];
 	char prop[128];
 	char *prefix = "";
 	int ret;
@@ -277,19 +287,12 @@ static int simple_dai_link_of(struct asoc_simple_priv *priv,
 	snprintf(prop, sizeof(prop), "%splat", prefix);
 	plat = of_get_child_by_name(node, prop);
 
-	ret = asoc_simple_parse_daifmt(dev, node, codec,
-				       prefix, &dai_link->dai_fmt);
-	if (ret < 0)
-		goto dai_link_of_err;
-
 	/*
 	 * next simple_parse_node() might remove cpus->dai_name.
-	 * set dailink_name before it.
+	 * get dai_name before it.
 	 */
-	ret = asoc_simple_set_dailink_name(dev, dai_link, "%s-%s",
-					   cpus->dai_name, codecs->dai_name);
-	if (ret < 0)
-		goto dai_link_of_err;
+	snprintf(dai_name, sizeof(dai_name),
+		 "%s-%s", cpus->dai_name, codecs->dai_name);
 
 	ret = simple_parse_node(priv, cpu, li, prefix, 1);
 	if (ret < 0)
@@ -303,8 +306,7 @@ static int simple_dai_link_of(struct asoc_simple_priv *priv,
 	if (ret < 0)
 		goto dai_link_of_err;
 
-	dai_link->ops = &simple_ops;
-	dai_link->init = asoc_simple_dai_init;
+	ret = simple_link_init(priv, node, codec, li, prefix, dai_name);
 
 dai_link_of_err:
 	of_node_put(plat);
-- 
2.25.1

[PATCH 3/4] ASoC: audio-graph: tidyup graph_dai_link_of_dpcm()

[Kuninori Morimoto]

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

Use local variable at local area only.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/generic/audio-graph-card.c | 30 +++++++++++++---------------
 1 file changed, 14 insertions(+), 16 deletions(-)

diff --git a/sound/soc/generic/audio-graph-card.c b/sound/soc/generic/audio-graph-card.c
index 2c8a2fcb7922..0159a4576e9c 100644
--- a/sound/soc/generic/audio-graph-card.c
+++ b/sound/soc/generic/audio-graph-card.c
@@ -276,24 +276,19 @@ static int graph_dai_link_of_dpcm(struct asoc_simple_priv *priv,
 				  struct link_info *li)
 {
 	struct device *dev = simple_priv_to_dev(priv);
-	struct snd_soc_card *card = simple_priv_to_card(priv);
 	struct snd_soc_dai_link *dai_link = simple_priv_to_link(priv, li->link);
 	struct simple_dai_props *dai_props = simple_priv_to_props(priv, li->link);
 	struct device_node *top = dev->of_node;
 	struct device_node *ep = li->cpu ? cpu_ep : codec_ep;
-	struct device_node *port;
-	struct device_node *ports;
-	struct snd_soc_dai_link_component *cpus = asoc_link_to_cpu(dai_link, 0);
-	struct snd_soc_dai_link_component *codecs = asoc_link_to_codec(dai_link, 0);
 	char dai_name[64];
 	int ret;
 
-	port	= of_get_parent(ep);
-	ports	= of_get_parent(port);
-
 	dev_dbg(dev, "link_of DPCM (%pOF)\n", ep);
 
 	if (li->cpu) {
+		struct snd_soc_card *card = simple_priv_to_card(priv);
+		struct snd_soc_dai_link_component *cpus = asoc_link_to_cpu(dai_link, 0);
+
 		/* Codec is dummy */
 
 		/* FE settings */
@@ -302,7 +297,7 @@ static int graph_dai_link_of_dpcm(struct asoc_simple_priv *priv,
 
 		ret = graph_parse_node(priv, cpu_ep, li, 1);
 		if (ret)
-			goto out_put_node;
+			return ret;
 
 		snprintf(dai_name, sizeof(dai_name),
 			 "fe.%pOFP.%s", cpus->of_node, cpus->dai_name);
@@ -319,7 +314,10 @@ static int graph_dai_link_of_dpcm(struct asoc_simple_priv *priv,
 		if (card->component_chaining && !soc_component_is_pcm(cpus))
 			dai_link->no_pcm = 1;
 	} else {
-		struct snd_soc_codec_conf *cconf;
+		struct snd_soc_codec_conf *cconf = simple_props_to_codec_conf(dai_props, 0);
+		struct snd_soc_dai_link_component *codecs = asoc_link_to_codec(dai_link, 0);
+		struct device_node *port;
+		struct device_node *ports;
 
 		/* CPU is dummy */
 
@@ -327,22 +325,25 @@ static int graph_dai_link_of_dpcm(struct asoc_simple_priv *priv,
 		dai_link->no_pcm		= 1;
 		dai_link->be_hw_params_fixup	= asoc_simple_be_hw_params_fixup;
 
-		cconf	= simple_props_to_codec_conf(dai_props, 0);
-
 		ret = graph_parse_node(priv, codec_ep, li, 0);
 		if (ret < 0)
-			goto out_put_node;
+			return ret;
 
 		snprintf(dai_name, sizeof(dai_name),
 			 "be.%pOFP.%s", codecs->of_node, codecs->dai_name);
 
 		/* check "prefix" from top node */
+		port = of_get_parent(ep);
+		ports = of_get_parent(port);
 		snd_soc_of_parse_node_prefix(top, cconf, codecs->of_node,
 					      "prefix");
 		if (of_node_name_eq(ports, "ports"))
 			snd_soc_of_parse_node_prefix(ports, cconf, codecs->of_node, "prefix");
 		snd_soc_of_parse_node_prefix(port, cconf, codecs->of_node,
 					     "prefix");
+
+		of_node_put(ports);
+		of_node_put(port);
 	}
 
 	graph_parse_convert(dev, ep, &dai_props->adata);
@@ -351,11 +352,8 @@ static int graph_dai_link_of_dpcm(struct asoc_simple_priv *priv,
 
 	ret = graph_link_init(priv, cpu_ep, codec_ep, li, dai_name);
 
-out_put_node:
 	li->link++;
 
-	of_node_put(ports);
-	of_node_put(port);
 	return ret;
 }
 
-- 
2.25.1

[PATCH 4/4] ASoC: audio-graph: tidyup dai_name seting timing

[Kuninori Morimoto]

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

audio-graph is using cpus->dai_name / codecs->dai_name
for dailink->name, but cpus->dai_name might be removed
under simple_parse_node() (= asoc_simple_canonicalize_cpu()).

Thus we need to get dai_name before calling simple_parse_node().
This patch fixup it.
To reduce future confusion, this patch follow same style
for similar parts too.

Fixes: 8859f809c7d5813 ("ASoC: audio-graph: add graph_parse_node()")
Fixes: e51237b8d305225 ("ASoC: audio-graph: add graph_link_init()")
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/generic/audio-graph-card.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/sound/soc/generic/audio-graph-card.c b/sound/soc/generic/audio-graph-card.c
index 0159a4576e9c..f8fc3222710b 100644
--- a/sound/soc/generic/audio-graph-card.c
+++ b/sound/soc/generic/audio-graph-card.c
@@ -295,12 +295,13 @@ static int graph_dai_link_of_dpcm(struct asoc_simple_priv *priv,
 		dai_link->dynamic		= 1;
 		dai_link->dpcm_merged_format	= 1;
 
+		snprintf(dai_name, sizeof(dai_name),
+			 "fe.%pOFP.%s", cpus->of_node, cpus->dai_name);
+
 		ret = graph_parse_node(priv, cpu_ep, li, 1);
 		if (ret)
 			return ret;
 
-		snprintf(dai_name, sizeof(dai_name),
-			 "fe.%pOFP.%s", cpus->of_node, cpus->dai_name);
 		/*
 		 * In BE<->BE connections it is not required to create
 		 * PCM devices at CPU end of the dai link and thus 'no_pcm'
@@ -325,13 +326,13 @@ static int graph_dai_link_of_dpcm(struct asoc_simple_priv *priv,
 		dai_link->no_pcm		= 1;
 		dai_link->be_hw_params_fixup	= asoc_simple_be_hw_params_fixup;
 
+		snprintf(dai_name, sizeof(dai_name),
+			 "be.%pOFP.%s", codecs->of_node, codecs->dai_name);
+
 		ret = graph_parse_node(priv, codec_ep, li, 0);
 		if (ret < 0)
 			return ret;
 
-		snprintf(dai_name, sizeof(dai_name),
-			 "be.%pOFP.%s", codecs->of_node, codecs->dai_name);
-
 		/* check "prefix" from top node */
 		port = of_get_parent(ep);
 		ports = of_get_parent(port);
@@ -371,6 +372,13 @@ static int graph_dai_link_of(struct asoc_simple_priv *priv,
 
 	dev_dbg(dev, "link_of (%pOF)\n", cpu_ep);
 
+	/*
+	 * next graph_parse_node() might remove cpus->dai_name.
+	 * get dai_name before it.
+	 */
+	snprintf(dai_name, sizeof(dai_name),
+		 "%s-%s", cpus->dai_name, codecs->dai_name);
+
 	ret = graph_parse_node(priv, cpu_ep, li, 1);
 	if (ret < 0)
 		return ret;
@@ -379,8 +387,6 @@ static int graph_dai_link_of(struct asoc_simple_priv *priv,
 	if (ret < 0)
 		return ret;
 
-	snprintf(dai_name, sizeof(dai_name),
-		 "%s-%s", cpus->dai_name, codecs->dai_name);
 	ret = graph_link_init(priv, cpu_ep, codec_ep, li, dai_name);
 	if (ret < 0)
 		return ret;
-- 
2.25.1

Re: [PATCH 0/4] ASoC: simple-card / audio-graph re-cleanup

[Michael Walle]

Hi,

Am 2021-05-10 03:51, schrieb Kuninori Morimoto:
> Hi Mark, Guillaume
> 
> KernelCI had reported that my previous cleanup patches for simple-card
> / audio-graph
> broke kontron-sl28-var3-ads2 sound card probing.
> It reported that it is using same name for dailink->name.
> 
> At first I thought that the issue was fsl,vf610-sai doesn't have .name
> on driver.
> But real issue was that cpu->dai_name removed on simple_parse_node(),
> and dailink->name was based on it.
> We need to set/get dailink->name first, and call simple_parse_node().
> This patches are do it.
> audio-graph has same issue. [4/4] patch is for it.
> 
> I hope these patches has no issues on kontron-sl28-var3-ads2.

I've just tested this on kontron-sl28-var3-ads2 (based on the latest
next). Now I'm getting even less useful names ;)

[    6.769932] sysfs: cannot create duplicate filename 
'/devices/platform/sound/(null)-(null)'
[    6.778529] CPU: 1 PID: 65 Comm: kworker/u4:1 Not tainted 
5.12.0-next-20210506+ #527
[    6.786397] Hardware name: Kontron SMARC-sAL28 (Single PHY) on SMARC 
Eval 2.0 carrier (DT)
[    6.794789] Workqueue: events_unbound deferred_probe_work_func
[    6.800727] Call trace:
[    6.803209]  dump_backtrace+0x0/0x1b8
[    6.806933]  show_stack+0x20/0x30
[    6.810301]  dump_stack+0x100/0x170
[    6.813846]  sysfs_warn_dup+0x6c/0x88
[    6.817565]  sysfs_create_dir_ns+0xe8/0x100
[    6.821813]  kobject_add_internal+0x9c/0x290
[    6.826150]  kobject_add+0xa0/0x108
[    6.829693]  device_add+0xfc/0x840
[    6.833147]  device_register+0x28/0x38
[    6.836954]  snd_soc_add_pcm_runtime+0x274/0x760
[    6.841647]  snd_soc_bind_card+0x330/0x9b8
[    6.842884] hid-generic 0003:064F:2AF9.0001: device has no listeners, 
quitting
[    6.845811]  snd_soc_register_card+0x10c/0x128
[    6.845823]  devm_snd_soc_register_card+0x4c/0xa8
[    6.845830]  asoc_simple_probe+0x1e8/0x3c8
[    6.845838]  platform_probe+0x70/0xe0
[    6.845846]  really_probe+0xec/0x3c0
[    6.845853]  driver_probe_device+0x6c/0xd0
[    6.845861]  __device_attach_driver+0x98/0xe0
[    6.845869]  bus_for_each_drv+0x84/0xd8
[    6.886424]  __device_attach+0xf0/0x150
[    6.890322]  device_initial_probe+0x1c/0x28
[    6.894572]  bus_probe_device+0xa4/0xb0
[    6.898468]  deferred_probe_work_func+0x90/0xd0
[    6.903069]  process_one_work+0x2b8/0x720
[    6.907141]  worker_thread+0x4c/0x488
[    6.910860]  kthread+0x164/0x168
[    6.914141]  ret_from_fork+0x10/0x30
[    6.917924] kobject_add_internal failed for (null)-(null) with 
-EEXIST, don't try to register things with the same name in the same 
directory.
[    6.931455] asoc-simple-card: probe of sound failed with error -12

-michael

Re: [PATCH 0/4] ASoC: simple-card / audio-graph re-cleanup

[Guillaume Tucker]

On 10/05/2021 02:51, Kuninori Morimoto wrote:
> 
> Hi Mark, Guillaume
> 
> KernelCI had reported that my previous cleanup patches for simple-card / audio-graph
> broke kontron-sl28-var3-ads2 sound card probing.

Could you please add this trailer, or maybe the maintainers can
add it when applying the patches?

  Reported-by: "kernelci.org bot" <bot@kernelci.org>

Thanks,
Guillaume

> It reported that it is using same name for dailink->name.
> 
> At first I thought that the issue was fsl,vf610-sai doesn't have .name on driver.
> But real issue was that cpu->dai_name removed on simple_parse_node(),
> and dailink->name was based on it.
> We need to set/get dailink->name first, and call simple_parse_node().
> This patches are do it.
> audio-graph has same issue. [4/4] patch is for it.
> 
> I hope these patches has no issues on kontron-sl28-var3-ads2.
> 
> Link: https://lore.kernel.org/r/87h7k0i437.wl-kuninori.morimoto.gx@renesas.com
> Link: https://lore.kernel.org/r/20210423175318.13990-1-broonie@kernel.org
> Link: https://lore.kernel.org/r/3ca62063-41b4-c25b-a7bc-8a8160e7b684@collabora.com
> 
> Kuninori Morimoto (4):
>   ASoC: simple-card: add simple_parse_node()
>   ASoC: simple-card: add simple_link_init()
>   ASoC: audio-graph: tidyup graph_dai_link_of_dpcm()
>   ASoC: audio-graph: tidyup dai_name seting timing
> 
>  sound/soc/generic/audio-graph-card.c |  48 +++----
>  sound/soc/generic/simple-card.c      | 187 ++++++++++++++-------------
>  2 files changed, 122 insertions(+), 113 deletions(-)
> 

Re: [PATCH 0/4] ASoC: simple-card / audio-graph re-cleanup

[Mark Brown]

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

On Mon, May 10, 2021 at 01:27:17PM +0100, Guillaume Tucker wrote:

> Could you please add this trailer, or maybe the maintainers can
> add it when applying the patches?

>   Reported-by: "kernelci.org bot" <bot@kernelci.org>

If you're going to send stuff like this please send any tags you're
trying to get picked up in the normal format without any indentation at
the start of the line, that way tooling will pick it up.  Though in this
case Morimoto-san had already put this on patch 4 anyway.

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