[PATCH 0/3] ASoC: simple-card-utils: tidyup for Multi connection

[PATCH 0/3] ASoC: simple-card-utils: tidyup for Multi connection

[Kuninori Morimoto]

Hi Mark

These patches tidyup simple-card-utils for Multi connection of
Audio Graph Card, Because of DT node parsing, it should check port
1st instead of endpoint. Otherwise, it can't handle DAI correctly.

Kuninori Morimoto (3):
  ASoC: simple-card-utils: use __free(device_node) for device node
  ASoC: simple-card-utils: check port reg first on graph_get_dai_id()
  ASoC: simple-card-utils: use for_each_of_graph_port() on graph_get_dai_id()

 sound/soc/generic/simple-card-utils.c | 58 +++++++++++----------------
 1 file changed, 23 insertions(+), 35 deletions(-)

-- 
2.43.0

[PATCH 1/3] ASoC: simple-card-utils: use __free(device_node) for device node

[Kuninori Morimoto]

simple-card-utils handles many type of device_node, thus need to
use of_node_put() in many place. Let's use __free(device_node)
and avoid it.

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

diff --git a/sound/soc/generic/simple-card-utils.c b/sound/soc/generic/simple-card-utils.c
index 24b371f320663..d2307d135931b 100644
--- a/sound/soc/generic/simple-card-utils.c
+++ b/sound/soc/generic/simple-card-utils.c
@@ -1005,35 +1005,27 @@ EXPORT_SYMBOL_GPL(graph_util_card_probe);
 
 int graph_util_is_ports0(struct device_node *np)
 {
-	struct device_node *port, *ports, *ports0, *top;
-	int ret;
+	struct device_node *parent __free(device_node) = of_get_parent(np);
+	struct device_node *port;
 
 	/* np is "endpoint" or "port" */
-	if (of_node_name_eq(np, "endpoint")) {
-		port = of_get_parent(np);
-	} else {
+	if (of_node_name_eq(np, "endpoint"))
+		port = parent;
+	else
 		port = np;
-		of_node_get(port);
-	}
-
-	ports	= of_get_parent(port);
-	top	= of_get_parent(ports);
-	ports0	= of_get_child_by_name(top, "ports");
-
-	ret = ports0 == ports;
 
-	of_node_put(port);
-	of_node_put(ports);
-	of_node_put(ports0);
-	of_node_put(top);
+	struct device_node *ports  __free(device_node) = of_get_parent(port);
+	struct device_node *top    __free(device_node) = of_get_parent(ports);
+	struct device_node *ports0 __free(device_node) = of_get_child_by_name(top, "ports");
 
-	return ret;
+	return ports0 == ports;
 }
 EXPORT_SYMBOL_GPL(graph_util_is_ports0);
 
 static int graph_get_dai_id(struct device_node *ep)
 {
-	struct device_node *node;
+	struct device_node *node __free(device_node) = of_graph_get_port_parent(ep);
+	struct device_node *port __free(device_node) = of_get_parent(ep);
 	struct device_node *endpoint;
 	struct of_endpoint info;
 	int i, id;
@@ -1056,13 +1048,10 @@ static int graph_get_dai_id(struct device_node *ep)
 		if (of_property_present(ep,   "reg"))
 			return info.id;
 
-		node = of_get_parent(ep);
-		ret = of_property_present(node, "reg");
-		of_node_put(node);
+		ret = of_property_present(port, "reg");
 		if (ret)
 			return info.port;
 	}
-	node = of_graph_get_port_parent(ep);
 
 	/*
 	 * Non HDMI sound case, counting port/endpoint on its DT
@@ -1076,8 +1065,6 @@ static int graph_get_dai_id(struct device_node *ep)
 		i++;
 	}
 
-	of_node_put(node);
-
 	if (id < 0)
 		return -ENODEV;
 
@@ -1087,7 +1074,6 @@ static int graph_get_dai_id(struct device_node *ep)
 int graph_util_parse_dai(struct device *dev, struct device_node *ep,
 			 struct snd_soc_dai_link_component *dlc, int *is_single_link)
 {
-	struct device_node *node;
 	struct of_phandle_args args = {};
 	struct snd_soc_dai *dai;
 	int ret;
@@ -1095,7 +1081,7 @@ int graph_util_parse_dai(struct device *dev, struct device_node *ep,
 	if (!ep)
 		return 0;
 
-	node = of_graph_get_port_parent(ep);
+	struct device_node *node __free(device_node) = of_graph_get_port_parent(ep);
 
 	/*
 	 * Try to find from DAI node
@@ -1136,10 +1122,8 @@ int graph_util_parse_dai(struct device *dev, struct device_node *ep,
 	 *    if he unbinded CPU or Codec.
 	 */
 	ret = snd_soc_get_dlc(&args, dlc);
-	if (ret < 0) {
-		of_node_put(node);
+	if (ret < 0)
 		return ret;
-	}
 
 parse_dai_end:
 	if (is_single_link)
-- 
2.43.0

[PATCH 2/3] ASoC: simple-card-utils: check port reg first on graph_get_dai_id()

[Kuninori Morimoto]

Because DT check when compiling become very strict in these days,
we need to add reg = <x> if it has multi port/endpoint, otherwise
it will get error or warning. But it was not so strict and/or
mandatry before.

Current code uses reg number as DAI ID, but it will use "endpoint"
reg first and use "port" reg 2nd. But it should use port number as 1st (A)
if it was used for multi connected case. There is no priority for
port/endpoint if it was not multi connected (B).

case (A)
	port {
		/*
		 * "port" and "endpoint" are using different reg number.
		 * It should use <x> as DAI ID, not <y> not <z>
		 */
		reg = <x>;
		endpoint@y { reg = <y>; ... };
		endpoint@z { reg = <z>; ... };
	};

case (B)
	port {
		/*
		 * Both port/endpoint are using same reg numer <x>.
		 */
		reg = <x>;
		endpoint { reg = <x>; ... };
	};

It will be issue if Audio-Graph-Card is used with Multi Connection.
No issue will be happen with Audio-Graph-Card2 / Simple-Card.

This patch swtich port/endpoint priority.

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

diff --git a/sound/soc/generic/simple-card-utils.c b/sound/soc/generic/simple-card-utils.c
index d2307d135931b..f67a1e58e821c 100644
--- a/sound/soc/generic/simple-card-utils.c
+++ b/sound/soc/generic/simple-card-utils.c
@@ -1045,12 +1045,15 @@ static int graph_get_dai_id(struct device_node *ep)
 		 * only of_graph_parse_endpoint().
 		 * We need to check "reg" property
 		 */
-		if (of_property_present(ep,   "reg"))
-			return info.id;
 
+		/* check port first */
 		ret = of_property_present(port, "reg");
 		if (ret)
 			return info.port;
+
+		/* check endpoint 2nd as backup */
+		if (of_property_present(ep,   "reg"))
+			return info.id;
 	}
 
 	/*
-- 
2.43.0

[PATCH 3/3] ASoC: simple-card-utils: use for_each_of_graph_port() on graph_get_dai_id()

[Kuninori Morimoto]

Because DT check when compiling become very strict in these days,
we need to add reg = <x> if it has multi port/endpoint, otherwise
it will get error or warning. But it was not so strict and/or
mandatry before.

Current code is counting "endpoint" to get DAI ID, but it should count
"port" instead, otherwise strange ID will be used for DAI if it was multi
connected case (A). There is no issue if it was not multi connected (B).

One note is that this code will be used if neither port/endpoint doesn't
have reg = <x> property on DT.

case (A)

	/* This should be handled as DAI-0 */
	port@0 {
		endpoint@0 {  }	/* It will be DAI-0 by endpoint count */
		endpoint@1 {  }	/* It will be DAI-1 by endpoint count */
	};
	/* This should be handled as DAI-1 */
	port@1 {
		endpoint {  }	/* It will be DAI-2 by endpoint count */
	};

case (B)
	/* both endpoint cound and port count are same */
	port@0 {
		endpoint { ... }
	};
	port@1 {
		endpoint { ... }
	};

It will be issue if Audio-Graph-Card is used with Multi Connection.
No issue will be happen with Audio-Graph-Card2 / Simple-Card.

This patch uses for_each_of_graph_port() instead of
for_each_endpoint_of_node(), and thus, we can use "break" to quit
from loop. Because for_each_of_graph_port() uses __free(device_node)
inside.

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

diff --git a/sound/soc/generic/simple-card-utils.c b/sound/soc/generic/simple-card-utils.c
index f67a1e58e821c..6c5a1c5a6b3b2 100644
--- a/sound/soc/generic/simple-card-utils.c
+++ b/sound/soc/generic/simple-card-utils.c
@@ -1026,7 +1026,6 @@ static int graph_get_dai_id(struct device_node *ep)
 {
 	struct device_node *node __free(device_node) = of_graph_get_port_parent(ep);
 	struct device_node *port __free(device_node) = of_get_parent(ep);
-	struct device_node *endpoint;
 	struct of_endpoint info;
 	int i, id;
 	int ret;
@@ -1062,9 +1061,11 @@ static int graph_get_dai_id(struct device_node *ep)
 	 */
 	i = 0;
 	id = -1;
-	for_each_endpoint_of_node(node, endpoint) {
-		if (endpoint == ep)
+	for_each_of_graph_port(node, p) {
+		if (port == p) {
 			id = i;
+			break;
+		}
 		i++;
 	}
 
-- 
2.43.0

Re: [PATCH 0/3] ASoC: simple-card-utils: tidyup for Multi connection

[Mark Brown]

On Tue, 03 Dec 2024 02:10:06 +0000, Kuninori Morimoto wrote:
> These patches tidyup simple-card-utils for Multi connection of
> Audio Graph Card, Because of DT node parsing, it should check port
> 1st instead of endpoint. Otherwise, it can't handle DAI correctly.
> 
> Kuninori Morimoto (3):
>   ASoC: simple-card-utils: use __free(device_node) for device node
>   ASoC: simple-card-utils: check port reg first on graph_get_dai_id()
>   ASoC: simple-card-utils: use for_each_of_graph_port() on graph_get_dai_id()
> 
> [...]

Applied to

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

Thanks!

[1/3] ASoC: simple-card-utils: use __free(device_node) for device node
      commit: 419d1918105e5d9926ab02f1f834bb416dc76f65
[2/3] ASoC: simple-card-utils: check port reg first on graph_get_dai_id()
      commit: 76deee29153b8313cc9629d3db45e56024b3dd26
[3/3] ASoC: simple-card-utils: use for_each_of_graph_port() on graph_get_dai_id()
      commit: bd4a5c8d5356fa42a1d63b684d34cf58a21eb8f7

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