From: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>

An explicitly disabled link is a DAI link in which one of its device
endpoints (e.g: codec or CPU) has been disabled in the DTS via the
"status" property. Formally speaking:

	OF_LINK_IS_DISABLED(lnk) = OF_NODE_IS_DISABLED(dev0) ||
	                           OF_NODE_IS_DISABLED(dev1);

where dev0 and dev1 are the two devices (CPU/codec) that make up the
link.

If at least one link was explicitly disabled that means DAPM routes
passed through the OF property "routing" can fail as some widgets might
not exist. Consider the following example:

	CODEC A has widgets A0, A1.
	CODEC B has widgets B0, B1.

	my-card {
		compatible = "audio-graph-card2":
		label = "my-label";
		links = <&cpu0_port>, <&cpu1_port>;
		routing = "A0", "A1",
		          "B0", "B1";
	};

	CODEC A's DT node was disabled.
	CODEC B's DT node is enabled.
	CPU0's DT node is enabled.
	CPU1's DT node is enabled.

If CODEC A's DT node is disabled via the 'status = "disabled"' property
that means the A0 -> A1 route cannot be created. This doesn't affect the
B0 -> B1 route though as CODEC B was never disabled in the DT.

This is why, if any explicitly disabled link is discovered, the
"disable_of_route_checks" flag is turned on.

If all links were explicitly disabled the sound card creation will fail.
Otherwise, if there's at least one link which wasn't explicitly disabled
then the sound card creation will succeed.

Signed-off-by: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
---
 include/sound/simple_card_utils.h     |   3 +-
 sound/soc/generic/audio-graph-card2.c | 231 ++++++++++++++++++++++++++
 2 files changed, 233 insertions(+), 1 deletion(-)

diff --git a/include/sound/simple_card_utils.h b/include/sound/simple_card_utils.h
index 69a9c9c4d0e9..3fde0314dbb2 100644
--- a/include/sound/simple_card_utils.h
+++ b/include/sound/simple_card_utils.h
@@ -132,9 +132,10 @@ struct simple_util_priv {
 #define SNDRV_MAX_LINKS 512
 
 struct link_info {
-	int link; /* number of link */
+	int link; /* number of available links */
 	int cpu;  /* turn for CPU / Codec */
 	struct prop_nums num[SNDRV_MAX_LINKS];
+	bool disabled_link; /* true if there's at least one explicitly disabled link */
 };
 
 int simple_util_parse_daifmt(struct device *dev,
diff --git a/sound/soc/generic/audio-graph-card2.c b/sound/soc/generic/audio-graph-card2.c
index 5dcc78c551a2..92774361a688 100644
--- a/sound/soc/generic/audio-graph-card2.c
+++ b/sound/soc/generic/audio-graph-card2.c
@@ -1269,6 +1269,226 @@ static int graph_count(struct simple_util_priv *priv,
 	return graph_ret(priv, ret);
 }
 
+static bool single_port_device_is_disabled(struct device_node *port)
+{
+	struct device_node *port_device __free(device_node) = NULL;
+	struct device_node *ports __free(device_node) = port_to_ports(port);
+
+	if (!ports)
+		port_device = of_get_parent(port);
+	else
+		port_device = of_get_parent(ports);
+
+	return !of_device_is_available(port_device);
+}
+
+static bool multi_nm_port_device_is_disabled(struct device_node *port)
+{
+	struct device_node *ep, *prev = NULL;
+	struct device_node *remote_ep __free(device_node) = NULL;
+	struct device_node *remote_port __free(device_node) = NULL;
+	struct device_node *remote_device __free(device_node) = NULL;
+
+	while (1) {
+		ep = of_graph_get_next_port_endpoint(port, prev);
+		if (!ep)
+			break;
+
+		/* first endpoint is special because it points to the remote device */
+		if (!prev) {
+			remote_device = of_graph_get_remote_port(ep);
+
+			if (single_port_device_is_disabled(remote_device))
+				return true;
+
+			prev = ep;
+
+			continue;
+		}
+
+		/* first get the remote port associated with the current endpoint */
+		remote_port = of_graph_get_remote_port(ep);
+		if (!remote_port)
+			break;
+
+		/* ... then get the first endpoint in remote port */
+		remote_ep = of_graph_get_next_port_endpoint(remote_port, NULL);
+		if (!remote_ep)
+			break;
+
+		/* ... and finally get the remote device node */
+		remote_device = of_graph_get_remote_port(remote_ep);
+		if (!remote_device)
+			break;
+
+		if (single_port_device_is_disabled(remote_device))
+			return true;
+
+		prev = ep;
+	}
+
+	return false;
+}
+
+static int graph_get_port_endpoint_count(struct device_node *port)
+{
+	int num = 0;
+
+	for_each_of_graph_port_endpoint(port, it)
+		num++;
+
+	return num;
+}
+
+static bool multi_port_device_is_disabled(struct device_node *lnk)
+{
+	int i, ep_count;
+	struct device_node *port __free(device_node) = NULL;
+	struct device_node *port_ep __free(device_node) = NULL;
+	struct device_node *remote_port __free(device_node) = NULL;
+	struct device_node *ports __free(device_node) = port_to_ports(lnk);
+
+	if (!ports)
+		return false;
+
+	for (i = 0;; i++) {
+		port = of_graph_get_port_by_id(ports, i + 1);
+		if (!port)
+			break;
+
+		/* N CPUs to M CODECs will have the endpoint count > 1 */
+		ep_count = graph_get_port_endpoint_count(port);
+		if (!ep_count)
+			break;
+
+		if (ep_count > 1) {
+			if (multi_nm_port_device_is_disabled(port))
+				return true;
+
+			continue;
+		}
+
+		port_ep = of_graph_get_next_port_endpoint(port, NULL);
+		if (!port_ep)
+			break;
+
+		remote_port = of_graph_get_remote_port(port_ep);
+		if (!remote_port)
+			break;
+
+		/*
+		 * if one port device is disabled then the whole link will
+		 * be disabled, thus we can stop at the first disabled device.
+		 */
+		if (single_port_device_is_disabled(remote_port))
+			return true;
+	}
+
+	return false;
+}
+
+static bool normal_port_device_is_disabled(struct device_node *port)
+{
+	if (graph_lnk_is_multi(port))
+		return multi_port_device_is_disabled(port);
+	else
+		return single_port_device_is_disabled(port);
+}
+
+static bool _dpcm_c2c_link_is_disabled(struct snd_soc_card *card,
+				       struct device_node *lnk)
+{
+	struct device_node *ep __free(device_node) = NULL;
+	struct device_node *remote_port __free(device_node) = NULL;
+
+	ep = of_graph_get_next_port_endpoint(lnk, NULL);
+	if (!ep) {
+		dev_err(card->dev, "port has no endpoint\n");
+		return false;
+	}
+
+	remote_port = of_graph_get_remote_port(ep);
+	if (!remote_port) {
+		dev_err(card->dev, "failed to fetch remote port\n");
+		return false;
+	}
+
+	if (__graph_get_type(remote_port) == GRAPH_MULTI)
+		return multi_port_device_is_disabled(remote_port);
+	else
+		return single_port_device_is_disabled(remote_port);
+}
+
+static bool c2c_link_is_disabled(struct snd_soc_card *card,
+				 struct device_node *lnk)
+{
+	struct device_node *ports __free(device_node) = NULL;
+
+	ports = port_to_ports(lnk);
+
+	if (!ports) {
+		dev_err(card->dev, "C2C port should be child of 'ports'\n");
+		return false;
+	}
+
+	for_each_of_graph_port(ports, it) {
+		if (_dpcm_c2c_link_is_disabled(card, it))
+			return true;
+	};
+
+	return false;
+}
+
+static bool normal_link_is_disabled(struct snd_soc_card *card,
+				    struct device_node *lnk)
+{
+	struct device_node *cpu_port;
+	struct device_node *cpu_ep __free(device_node) = NULL;
+	struct device_node *codec_port  __free(device_node) = NULL;
+
+	cpu_port = lnk;
+
+	cpu_ep = of_graph_get_next_port_endpoint(cpu_port, NULL);
+	if (!cpu_ep) {
+		dev_err(card->dev, "CPU port has no endpoint\n");
+		return false;
+	}
+
+	codec_port = of_graph_get_remote_port(cpu_ep);
+	if (!codec_port) {
+		dev_err(card->dev, "unable to find remote codec port\n");
+		return false;
+	}
+
+	return normal_port_device_is_disabled(codec_port) ||
+		normal_port_device_is_disabled(cpu_port);
+}
+
+static bool graph_link_is_disabled(struct simple_util_priv *priv,
+				   enum graph_type gtype,
+				   struct device_node *lnk,
+				   struct link_info *li)
+{
+	bool link_disabled = false;
+	struct snd_soc_card *card = simple_priv_to_card(priv);
+
+	switch (gtype) {
+	case GRAPH_NORMAL:
+		link_disabled = normal_link_is_disabled(card, lnk);
+		break;
+	case GRAPH_DPCM:
+		link_disabled = _dpcm_c2c_link_is_disabled(card, lnk);
+		break;
+	case GRAPH_C2C:
+		link_disabled = c2c_link_is_disabled(card, lnk);
+		break;
+	default:
+		break;
+	}
+
+	return link_disabled;
+}
+
 static int graph_for_each_link(struct simple_util_priv *priv,
 			       struct graph2_custom_hooks *hooks,
 			       struct link_info *li,
@@ -1291,6 +1511,12 @@ static int graph_for_each_link(struct simple_util_priv *priv,
 
 		gtype = graph_get_type(priv, lnk);
 
+		if (graph_link_is_disabled(priv, gtype, lnk, li)) {
+			if (!li->disabled_link)
+				li->disabled_link = true;
+			continue;
+		}
+
 		ret = func(priv, hooks, gtype, lnk, li);
 		if (ret < 0)
 			break;
@@ -1325,6 +1551,11 @@ int audio_graph2_parse_of(struct simple_util_priv *priv, struct device *dev,
 	if (ret < 0)
 		goto err;
 
+	if (li->disabled_link) {
+		dev_info(dev, "detected disabled link(s) - route creation may fail\n");
+		card->disable_of_route_checks = 1;
+	}
+
 	ret = simple_util_init_priv(priv, li);
 	if (ret < 0)
 		goto err;
-- 
2.34.1

Hi Laurentiu

Thank you for the patch

> An explicitly disabled link is a DAI link in which one of its device
> endpoints (e.g: codec or CPU) has been disabled in the DTS via the
> "status" property. Formally speaking:
> 
> 	OF_LINK_IS_DISABLED(lnk) = OF_NODE_IS_DISABLED(dev0) ||
> 	                           OF_NODE_IS_DISABLED(dev1);
> 
> where dev0 and dev1 are the two devices (CPU/codec) that make up the
> link.
> 
> If at least one link was explicitly disabled that means DAPM routes
> passed through the OF property "routing" can fail as some widgets might
> not exist. Consider the following example:
> 
> 	CODEC A has widgets A0, A1.
> 	CODEC B has widgets B0, B1.
> 
> 	my-card {
> 		compatible = "audio-graph-card2":
> 		label = "my-label";
> 		links = <&cpu0_port>, <&cpu1_port>;
> 		routing = "A0", "A1",
> 		          "B0", "B1";
> 	};
> 
> 	CODEC A's DT node was disabled.
> 	CODEC B's DT node is enabled.
> 	CPU0's DT node is enabled.
> 	CPU1's DT node is enabled.
> 
> If CODEC A's DT node is disabled via the 'status = "disabled"' property
> that means the A0 -> A1 route cannot be created. This doesn't affect the
> B0 -> B1 route though as CODEC B was never disabled in the DT.
> 
> This is why, if any explicitly disabled link is discovered, the
> "disable_of_route_checks" flag is turned on.
> 
> If all links were explicitly disabled the sound card creation will fail.
> Otherwise, if there's at least one link which wasn't explicitly disabled
> then the sound card creation will succeed.
> 
> Signed-off-by: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
> ---

I think I could understand your situation, but the solution (= this patch) is
too much complicated for me. Indeed it might detect disabled device, but some
board might want to detect it as error, unlike your case.

You want to add "disable_of_route_checks" flag to the card, right ?
How about to add new property, like "force detect xxx", or
"DAI might not be detected", etc, etc, etc...

If we can have such property, it will be more simple code.

	if (it_has_flag("force_detect_xxx")) {
		dev_info(dev, "xxx");
		card->disable_of_route_checks = 1;
	}

Thank you for your help !!

Best regards
---
Kuninori Morimoto

On 16.05.2025 04:36, Kuninori Morimoto wrote:
> Hi Laurentiu
>
> Thank you for the patch
>
>> An explicitly disabled link is a DAI link in which one of its device
>> endpoints (e.g: codec or CPU) has been disabled in the DTS via the
>> "status" property. Formally speaking:
>>
>> 	OF_LINK_IS_DISABLED(lnk) = OF_NODE_IS_DISABLED(dev0) ||
>> 	                           OF_NODE_IS_DISABLED(dev1);
>>
>> where dev0 and dev1 are the two devices (CPU/codec) that make up the
>> link.
>>
>> If at least one link was explicitly disabled that means DAPM routes
>> passed through the OF property "routing" can fail as some widgets might
>> not exist. Consider the following example:
>>
>> 	CODEC A has widgets A0, A1.
>> 	CODEC B has widgets B0, B1.
>>
>> 	my-card {
>> 		compatible = "audio-graph-card2":
>> 		label = "my-label";
>> 		links = <&cpu0_port>, <&cpu1_port>;
>> 		routing = "A0", "A1",
>> 		          "B0", "B1";
>> 	};
>>
>> 	CODEC A's DT node was disabled.
>> 	CODEC B's DT node is enabled.
>> 	CPU0's DT node is enabled.
>> 	CPU1's DT node is enabled.
>>
>> If CODEC A's DT node is disabled via the 'status = "disabled"' property
>> that means the A0 -> A1 route cannot be created. This doesn't affect the
>> B0 -> B1 route though as CODEC B was never disabled in the DT.
>>
>> This is why, if any explicitly disabled link is discovered, the
>> "disable_of_route_checks" flag is turned on.
>>
>> If all links were explicitly disabled the sound card creation will fail.
>> Otherwise, if there's at least one link which wasn't explicitly disabled
>> then the sound card creation will succeed.
>>
>> Signed-off-by: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
>> ---
> I think I could understand your situation, but the solution (= this patch) is
> too much complicated for me. Indeed it might detect disabled device, but some
> board might want to detect it as error, unlike your case.


that is true. One problem with this solution is the fact that you can't

really distinguish between links that were intentionally disabled (see example

from RFC's cover letter) and links that were accidentally disabled (e.g. user

forgot to set 'status = "okay"' for one of the link's devices)


>
> You want to add "disable_of_route_checks" flag to the card, right ?
> How about to add new property, like "force detect xxx", or
> "DAI might not be detected", etc, etc, etc...
>
> If we can have such property, it will be more simple code.
>
> 	if (it_has_flag("force_detect_xxx")) {
> 		dev_info(dev, "xxx");
> 		card->disable_of_route_checks = 1;
> 	}


well, I think the solution is made up of 2 parts:

    1) the part that allows link devices to be disabled

    2) the part that allows routes to fails


what you're suggesting is dropping the first part and going with just the second one.

The problem I see with this (let's assume we have the BASE-PLUGIN example presented

in the RFC's cover letter) is that:

    1) You'll have to specify the links inside of PLUGIN's DT overlay (instead of BASE's DTS)

    2) You'll have to do the link connection part inside PLUGIN's DT overlay.


So, our example DTS and DT overlay would look like this:


[snippet from base.dts]

	my_card: card {
		compatible = "audio-graph-card2";
		links = <&l2>; /* here, we're only allowed to specify links that exist */
		routing = "Headphones", "C20",
			  "Headphones", "C21",
			  "Line", "C01";
	};

	d0: cpu@0 {
		l0: port { l0_ep: endpoint { /* connected when plugin.dtbo is applied */ } };
	};

	d1: cpu@1 {
		l1: port { l1_ep: endpoint { /* connected when plugin.dtbo is applied */ } };
	};

	d2: cpu@2 {
		l2: port { l2_ep: endpoint { remote-endpoint = <&c2_ep>; } };
	};

	c2: codec@2 {
		port { c2_ep: endpoint { remote-endpoint = <&l2_ep>; } };
	};


[snippet from plugin.dtso]

&my_card {
	/* here we're forced to also specify l2 even if this is already done
         * in base.dts. This is because DT overlays don't support appending to
	 * properties.
         */
	remote-endpoint = <&l0>, <&l1>, <&l2>;
};

&l0_ep {
	remote-endpoint = <&c1_ep>;
};

&l1_ep {
	remote-endpoint = <&c2_ep>;
};

c0: codec@0 {
	port { c0_ep: endpoint { remote-endpoint = <&l0_ep>; } };
};

c1: codec@1 {
	port { c1_ep: endpoint { remote-endpoint = <&l1_ep>; } };
};

Assume that we also have BASE1 that is compatible with PLUGIN but
C0 and C1 are connected to BASE1's D0 and D5. Since there's no D1-C1
connection that means you'll have to create another DT overlay. Thus,
the scalability of plugin.dtso decreases.

Now, for our particular case, we have BASE0 and BASE1 with the following
DAIs and CODECS (these are physically present on the base boards):

BASE0 has DAIs D0, D1 and CODEC C0
BASE1 has DAIs D0, D1 and CODEC C1

Both of these boards are compatible with plugin PLUGIN that has codec C2.
The possible DAI links are:

For BASE0:

D0 <----> C0
D1 <----> C2 (only possible with PLUGIN connected)

For BASE1:

D0 <----> C1
D1 <----> C2 (only possible with PLUGIN connected)

Since the D1 <---> C2 connection is valid for both BASE0-PLUGIN and
BASE1-PLUGIN combinations I think we can make do without the support
for explicitly disabled links. But I don't think this is ideal because:

1) If we ever need to support board BASE3 that is compatible with PLUGIN
and uses Dn != D1 to connect with PLUGIN's C2 codec then we're going to
either modify our devicetrees (to make plugin.dtso applicable to BASE3
as well) or add another DT overlay (not really desirable because we already
have a lot of devicetrees). (note: more of an _if_ situation. Don't think we
can actually use this as an argument. I just wanted to have this out in the
open)

2) People might get confused when looking at the "links" and "routing"
properties. Now, "links" will only contain links that actually exist, while
"routing" can contain widgets from codecs that don't exist on the base board.
(note: perhaps not a deal breaker? again, just wanted to have this out in the
open)


Either way, I believe your concern is valid. This new feature adds a lot of
extra code and validating it is a pain. Still, even if we go with just the
extra DT property as you suggested I believe this is a step forward so I think
we can go with that for now and see how well this scales.


BTW, thanks a lot for taking the time to review this huge series!!

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

Hi Mihalcea

Thank you for clarify details.

> [snippet from base.dts]
> 
> 	my_card: card {
> 		compatible = "audio-graph-card2";
> 		links = <&l2>; /* here, we're only allowed to specify links that exist */
> 		routing = "Headphones", "C20",
> 			  "Headphones", "C21",
> 			  "Line", "C01";
> 	};
(snip)
> &my_card {
> 	/* here we're forced to also specify l2 even if this is already done
>          * in base.dts. This is because DT overlays don't support appending to
> 	 * properties.
>          */
> 	remote-endpoint = <&l0>, <&l1>, <&l2>;
> };

This is very nit pickking, but I need to confirm.
You want to indicate here is this ?

	&my_card {
-		remote-endpoint = <&l0>, <&l1>, <&l2>;
+		links = <&l0>, <&l1>, <&l2>;
	};

> &l0_ep {
> 	remote-endpoint = <&c1_ep>;
> };
> 
> &l1_ep {
> 	remote-endpoint = <&c2_ep>;
> };
> 
> c0: codec@0 {
> 	port { c0_ep: endpoint { remote-endpoint = <&l0_ep>; } };
> };
> 
> c1: codec@1 {
> 	port { c1_ep: endpoint { remote-endpoint = <&l1_ep>; } };
> };

This is also nit pickking, but I think above is wrong.
I guess you want to indicate is...

	&l0_ep {
-		remote-endpoint = <&c1_ep>;
+		remote-endpoint = <&c0_ep>;
	};

	&l1_ep {
-		remote-endpoint = <&c2_ep>;
+		remote-endpoint = <&c1_ep>;
	};


Your are indicating very confusable naming, so I want to understand
correctly as much as possible.

> >> 	CODEC A has widgets A0, A1.
> >> 	CODEC B has widgets B0, B1.
> >>
> >> 	my-card {
> >> 		compatible = "audio-graph-card2":
> >> 		label = "my-label";
> >> 		links = <&cpu0_port>, <&cpu1_port>;
> >> 		routing = "A0", "A1",
> >> 		          "B0", "B1";
> >> 	};
> >>
> >> 	CODEC A's DT node was disabled.
> >> 	CODEC B's DT node is enabled.
> >> 	CPU0's DT node is enabled.
> >> 	CPU1's DT node is enabled.
(snip)
> Assume that we also have BASE1 that is compatible with PLUGIN but
> C0 and C1 are connected to BASE1's D0 and D5. Since there's no D1-C1
> connection that means you'll have to create another DT overlay. Thus,
> the scalability of plugin.dtso decreases.
> 
> Now, for our particular case, we have BASE0 and BASE1 with the following
> DAIs and CODECS (these are physically present on the base boards):
> 
> BASE0 has DAIs D0, D1 and CODEC C0
> BASE1 has DAIs D0, D1 and CODEC C1
> 
> Both of these boards are compatible with plugin PLUGIN that has codec C2.
> The possible DAI links are:
> 
> For BASE0:
> 
> D0 <----> C0
> D1 <----> C2 (only possible with PLUGIN connected)
> 
> For BASE1:
> 
> D0 <----> C1
> D1 <----> C2 (only possible with PLUGIN connected)
> 
> Since the D1 <---> C2 connection is valid for both BASE0-PLUGIN and
> BASE1-PLUGIN combinations I think we can make do without the support
> for explicitly disabled links. But I don't think this is ideal because:

Let's avoid BASE3 case here to avoid _if_ story.
You are now indicating too many complex/cofusable situations with wrong
setting samples (D0/D1/D2..., C0/C1/C2. BASEx has no D1-C1..., etc, etc...)

I noticed that why you need to add disabled Codec routing on BASE DT ?
It is the reason why you can't detect BASE only sound card, right ?

	---- 8< ---- 8< ---- 8< ---- 8< ---- 8< ----
[snippet from base.dts]

	my_card: card {
		compatible = "audio-graph-card2";
		links = <&l2>; /* here, we're only allowed to specify links that exist */
		routing = "Headphones", "C20",
			  "Headphones", "C21",
=>			  "Line", "C01";
	};

	---- 8< ---- 8< ---- 8< ---- 8< ---- 8< ----

If my understanding was correct, your system can be indicated like below
(It is not same as your D0/D1/D2 sample, but I think same things, and
 not confusable sample)

	BASE			  PLUGIN
	+-----------------+
	| CPU0 <-> Codec0 |     +--------+
	| CPU1		  | <-> | Codec1 |
	| CPU2		  | <-> | Codec2 |
	+-----------------+     +--------+

How it works by below ?

BASE
	/*
	 * detect CPU0-Codec0 connection only
	 * Codec1/2 are disabled, but not related to BASE
	 */
	my_card: card {
		links = <&cpu0>;
		routing = "Headphone0", "Codec0"; /* for CPU0-Codec0 */
	};

PLUGIN
	/* detect all
	 * CPU0-Codec0 connection
	 * CPU1-Codec1 connection
	 * CPU2-Codec2 connection, and its routings */
	&my_card {
		links = <&cpu0>, <&cpu1>, <&cpu2>;
		routing = "Headphone0", "Codec0", /* for CPU0-Codec0 */
			  "Headphone1", "Codec1", /* for CPU1-Codec1 */
			  "Headphone2", "Codec2"; /* for CPU2-Codec2 */
	};


And/Or your situation is similar as mine (I should have noticed
about this sooner).

	d70be079c3cf34bd91e1c8f7b4bc760356c9150c
	("arm64: dts: renesas: ulcb/kf: Use multi Component sound")

	547b02f74e4ac1e7d295a6266d5bc93a647cd4ac
	("ASoC: rsnd: enable multi Component support for Audio Graph Card/Card2")

	45655ec69cb954d7fa594054bec33d6d5b99f8d5
	("ASoC: soc-core.c: enable multi Component")

My board is handling above sample as 2 cards, by using "multi Component"

	BASE			  PLUGIN
	+-----------------+			^
	| CPU0 <-> Codec0 |			| Card1
	|		  |			v
	|		  |     +--------+	^
	| CPU1		  | <-> | Codec1 |	| Card2
	| CPU2		  | <-> | Codec2 |	|
	+-----------------+     +--------+	v


Thank you for your help !!

Best regards
---
Kuninori Morimoto

On 19.05.2025 04:15, Kuninori Morimoto wrote:
> Hi Mihalcea
>
> Thank you for clarify details.
>
>> [snippet from base.dts]
>>
>> 	my_card: card {
>> 		compatible = "audio-graph-card2";
>> 		links = <&l2>; /* here, we're only allowed to specify links that exist */
>> 		routing = "Headphones", "C20",
>> 			  "Headphones", "C21",
>> 			  "Line", "C01";
>> 	};
> (snip)
>> &my_card {
>> 	/* here we're forced to also specify l2 even if this is already done
>>          * in base.dts. This is because DT overlays don't support appending to
>> 	 * properties.
>>          */
>> 	remote-endpoint = <&l0>, <&l1>, <&l2>;
>> };
> This is very nit pickking, but I need to confirm.
> You want to indicate here is this ?
>
> 	&my_card {
> -		remote-endpoint = <&l0>, <&l1>, <&l2>;
> +		links = <&l0>, <&l1>, <&l2>;
> 	};


yes, I'm very sorry for the mistakes....


>
>> &l0_ep {
>> 	remote-endpoint = <&c1_ep>;
>> };
>>
>> &l1_ep {
>> 	remote-endpoint = <&c2_ep>;
>> };
>>
>> c0: codec@0 {
>> 	port { c0_ep: endpoint { remote-endpoint = <&l0_ep>; } };
>> };
>>
>> c1: codec@1 {
>> 	port { c1_ep: endpoint { remote-endpoint = <&l1_ep>; } };
>> };
> This is also nit pickking, but I think above is wrong.
> I guess you want to indicate is...
>
> 	&l0_ep {
> -		remote-endpoint = <&c1_ep>;
> +		remote-endpoint = <&c0_ep>;
> 	};
>
> 	&l1_ep {
> -		remote-endpoint = <&c2_ep>;
> +		remote-endpoint = <&c1_ep>;
> 	};
>
>
> Your are indicating very confusable naming, so I want to understand
> correctly as much as possible.


yep, you're right. Again, very sorry.


>
>>>> 	CODEC A has widgets A0, A1.
>>>> 	CODEC B has widgets B0, B1.
>>>>
>>>> 	my-card {
>>>> 		compatible = "audio-graph-card2":
>>>> 		label = "my-label";
>>>> 		links = <&cpu0_port>, <&cpu1_port>;
>>>> 		routing = "A0", "A1",
>>>> 		          "B0", "B1";
>>>> 	};
>>>>
>>>> 	CODEC A's DT node was disabled.
>>>> 	CODEC B's DT node is enabled.
>>>> 	CPU0's DT node is enabled.
>>>> 	CPU1's DT node is enabled.
> (snip)
>> Assume that we also have BASE1 that is compatible with PLUGIN but
>> C0 and C1 are connected to BASE1's D0 and D5. Since there's no D1-C1
>> connection that means you'll have to create another DT overlay. Thus,
>> the scalability of plugin.dtso decreases.
>>
>> Now, for our particular case, we have BASE0 and BASE1 with the following
>> DAIs and CODECS (these are physically present on the base boards):
>>
>> BASE0 has DAIs D0, D1 and CODEC C0
>> BASE1 has DAIs D0, D1 and CODEC C1
>>
>> Both of these boards are compatible with plugin PLUGIN that has codec C2.
>> The possible DAI links are:
>>
>> For BASE0:
>>
>> D0 <----> C0
>> D1 <----> C2 (only possible with PLUGIN connected)
>>
>> For BASE1:
>>
>> D0 <----> C1
>> D1 <----> C2 (only possible with PLUGIN connected)
>>
>> Since the D1 <---> C2 connection is valid for both BASE0-PLUGIN and
>> BASE1-PLUGIN combinations I think we can make do without the support
>> for explicitly disabled links. But I don't think this is ideal because:
> Let's avoid BASE3 case here to avoid _if_ story.
> You are now indicating too many complex/cofusable situations with wrong
> setting samples (D0/D1/D2..., C0/C1/C2. BASEx has no D1-C1..., etc, etc...)
>
> I noticed that why you need to add disabled Codec routing on BASE DT ?
> It is the reason why you can't detect BASE only sound card, right ?


exactly!!!


>
> 	---- 8< ---- 8< ---- 8< ---- 8< ---- 8< ----
> [snippet from base.dts]
>
> 	my_card: card {
> 		compatible = "audio-graph-card2";
> 		links = <&l2>; /* here, we're only allowed to specify links that exist */
> 		routing = "Headphones", "C20",
> 			  "Headphones", "C21",
> =>			  "Line", "C01";
> 	};
>
> 	---- 8< ---- 8< ---- 8< ---- 8< ---- 8< ----
>
> If my understanding was correct, your system can be indicated like below
> (It is not same as your D0/D1/D2 sample, but I think same things, and
>  not confusable sample)
>
> 	BASE			  PLUGIN
> 	+-----------------+
> 	| CPU0 <-> Codec0 |     +--------+
> 	| CPU1		  | <-> | Codec1 |
> 	| CPU2		  | <-> | Codec2 |
> 	+-----------------+     +--------+


pretty much. The only difference would be that PLUGIN has only 1 codec

in our case. I think it'll be much easier to just stick to your naming conventions...


>
> How it works by below ?
>
> BASE
> 	/*
> 	 * detect CPU0-Codec0 connection only
> 	 * Codec1/2 are disabled, but not related to BASE
> 	 */
> 	my_card: card {
> 		links = <&cpu0>;
> 		routing = "Headphone0", "Codec0"; /* for CPU0-Codec0 */
> 	};
>
> PLUGIN
> 	/* detect all
> 	 * CPU0-Codec0 connection
> 	 * CPU1-Codec1 connection
> 	 * CPU2-Codec2 connection, and its routings */
> 	&my_card {
> 		links = <&cpu0>, <&cpu1>, <&cpu2>;
> 		routing = "Headphone0", "Codec0", /* for CPU0-Codec0 */
> 			  "Headphone1", "Codec1", /* for CPU1-Codec1 */
> 			  "Headphone2", "Codec2"; /* for CPU2-Codec2 */
> 	};


so, the problem with this is the fact that (assuming you've used a DT overlay

for the PLUGIN) you won't be able to use the DT overlay on other boards because

you've also added the "Headphone0", "Codec0" route which is specific to BASE's

Codec0. We have multiple boards so our system would look like this:


	BASE0			  PLUGIN
	+-----------------+
	| CPU0 <-> Codec0 |     +--------+
	| CPU1		  | <-> | Codec1 |
	+-----------------+     +--------+


	BASE1			  PLUGIN
	+-----------------+
	| CPU0 <-> Codec3 |     +--------+
	| CPU1		  | <-> | Codec1 |
	+-----------------+     +--------+


The plugin is the same. The only difference between BASE1 and BASE0 is the fact that CPU0

is connected to Codec0 on BASE0, while, on BASE1, CPU0 is connected to a different codec: Codec3.


>
>
> And/Or your situation is similar as mine (I should have noticed
> about this sooner).
>
> 	d70be079c3cf34bd91e1c8f7b4bc760356c9150c
> 	("arm64: dts: renesas: ulcb/kf: Use multi Component sound")
>
> 	547b02f74e4ac1e7d295a6266d5bc93a647cd4ac
> 	("ASoC: rsnd: enable multi Component support for Audio Graph Card/Card2")
>
> 	45655ec69cb954d7fa594054bec33d6d5b99f8d5
> 	("ASoC: soc-core.c: enable multi Component")
>
> My board is handling above sample as 2 cards, by using "multi Component"
>
> 	BASE			  PLUGIN
> 	+-----------------+			^
> 	| CPU0 <-> Codec0 |			| Card1
> 	|		  |			v
> 	|		  |     +--------+	^
> 	| CPU1		  | <-> | Codec1 |	| Card2
> 	| CPU2		  | <-> | Codec2 |	|
> 	+-----------------+     +--------+	v


one important thing to note here is the fact that we can only

have 1 sound card because all DAIs (CPU0, CPU1, CPU2) belong

to the same component.


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

Hi Laurentiu

> so, the problem with this is the fact that (assuming you've used a DT overlay
> for the PLUGIN) you won't be able to use the DT overlay on other boards because
> you've also added the "Headphone0", "Codec0" route which is specific to BASE's
> Codec0. We have multiple boards so our system would look like this:
> 
> 	BASE0			  PLUGIN
> 	+-----------------+
> 	| CPU0 <-> Codec0 |     +--------+
> 	| CPU1		  | <-> | Codec1 |
> 	+-----------------+     +--------+
> 
> 
> 	BASE1			  PLUGIN
> 	+-----------------+
> 	| CPU0 <-> Codec3 |     +--------+
> 	| CPU1		  | <-> | Codec1 |
> 	+-----------------+     +--------+
> 
> 
> The plugin is the same. The only difference between BASE1 and BASE0 is the fact that CPU0
> is connected to Codec0 on BASE0, while, on BASE1, CPU0 is connected to a different codec: Codec3.

Ah, OK, that it the reason why you added the route on BASE side...

Hmm... I think my previous suggested idea (new flag) is reasonable, but you
mentioned that you want to check whether it was "disabled" or not.
So, how about to add "plugin-route" and "plugin-links" instead ?

BASE
	my_card: card {
		links = <&cpu0>;
		routing = "Headphone0", "Codec0"; /* for CPU0-Codec0 */
	};

PLUGIN
	&my_card {
		plugin-links = <&cpu1>, <&cpu2>
		plugin-routing = "Headphone1", "Codec1", /* for CPU1-Codec1 */
		^^^^^^		 "Headphone2", "Codec2"; /* for CPU2-Codec2 */
	};

Audio Card2 parses "links" + "plugin-links", and
"routing" + "plugin-routing". It is more intuitive ?

> > 	BASE			  PLUGIN
> > 	+-----------------+			^
> > 	| CPU0 <-> Codec0 |			| Card1
> > 	|		  |			v
> > 	|		  |     +--------+	^
> > 	| CPU1		  | <-> | Codec1 |	| Card2
> > 	| CPU2		  | <-> | Codec2 |	|
> > 	+-----------------+     +--------+	v
> 
> one important thing to note here is the fact that we can only
> have 1 sound card because all DAIs (CPU0, CPU1, CPU2) belong
> to the same component.

Indeed it depens on the CPU side driver style.
I have updated my driver to allow to be multi components by checking DT.

I'm not sure which one (= use plugin-xxx flag or use multi Cards) is
more intuitive, but supporting both is not bad idea ?

Thank you for your help !!

Best regards
---
Kuninori Morimoto

On 20.05.2025 03:38, Kuninori Morimoto wrote:
> Hi Laurentiu
>
>> so, the problem with this is the fact that (assuming you've used a DT overlay
>> for the PLUGIN) you won't be able to use the DT overlay on other boards because
>> you've also added the "Headphone0", "Codec0" route which is specific to BASE's
>> Codec0. We have multiple boards so our system would look like this:
>>
>> 	BASE0			  PLUGIN
>> 	+-----------------+
>> 	| CPU0 <-> Codec0 |     +--------+
>> 	| CPU1		  | <-> | Codec1 |
>> 	+-----------------+     +--------+
>>
>>
>> 	BASE1			  PLUGIN
>> 	+-----------------+
>> 	| CPU0 <-> Codec3 |     +--------+
>> 	| CPU1		  | <-> | Codec1 |
>> 	+-----------------+     +--------+
>>
>>
>> The plugin is the same. The only difference between BASE1 and BASE0 is the fact that CPU0
>> is connected to Codec0 on BASE0, while, on BASE1, CPU0 is connected to a different codec: Codec3.
> Ah, OK, that it the reason why you added the route on BASE side...


exactly!


>
> Hmm... I think my previous suggested idea (new flag) is reasonable, but you
> mentioned that you want to check whether it was "disabled" or not.
> So, how about to add "plugin-route" and "plugin-links" instead ?
>
> BASE
> 	my_card: card {
> 		links = <&cpu0>;
> 		routing = "Headphone0", "Codec0"; /* for CPU0-Codec0 */
> 	};
>
> PLUGIN
> 	&my_card {
> 		plugin-links = <&cpu1>, <&cpu2>
> 		plugin-routing = "Headphone1", "Codec1", /* for CPU1-Codec1 */
> 		^^^^^^		 "Headphone2", "Codec2"; /* for CPU2-Codec2 */
> 	};
>
> Audio Card2 parses "links" + "plugin-links", and
> "routing" + "plugin-routing". It is more intuitive ?


hm, I believe this _should_ work. I also think that we can just drop the whole

"ignore_route_check" flag idea since you can just use "plugin-routing" in

your DT overlay to specify the CODEC-specific routes (instead of having

them in your BASE DTS). This way, you'll avoid having routes that might

not exist in your BASE DTS.


if we go for this though I think we need to clarify the usage of the

"plugin-links" and "plugin-routing" properties. For me, these properties

only make sense if you use them in a DT overlay to add additional links/routes

introduced by the PLUGIN board. This is basically a workaround the fact

that DT overlays don't support appending to the properties of the BASE

DTS.


also, I believe we can drop the whole "explicitly disabled links" idea

since IMO, links passed via the "plugin-links" property _must_ exist.


anyhow, I will have test out this new idea on our particular scenario and see

how well it works. Thank you very much for this discussion! It was really, really

helpful!


>
>>> 	BASE			  PLUGIN
>>> 	+-----------------+			^
>>> 	| CPU0 <-> Codec0 |			| Card1
>>> 	|		  |			v
>>> 	|		  |     +--------+	^
>>> 	| CPU1		  | <-> | Codec1 |	| Card2
>>> 	| CPU2		  | <-> | Codec2 |	|
>>> 	+-----------------+     +--------+	v
>> one important thing to note here is the fact that we can only
>> have 1 sound card because all DAIs (CPU0, CPU1, CPU2) belong
>> to the same component.
> Indeed it depens on the CPU side driver style.
> I have updated my driver to allow to be multi components by checking DT.
>
> I'm not sure which one (= use plugin-xxx flag or use multi Cards) is
> more intuitive, but supporting both is not bad idea ?
>
> Thank you for your help !!
>
> Best regards
> ---
> Kuninori Morimoto

Hi Mihalcea

Thank you for rhw the reply

> >> 	BASE0			  PLUGIN
> >> 	+-----------------+
> >> 	| CPU0 <-> Codec0 |     +--------+
> >> 	| CPU1		  | <-> | Codec1 |
> >> 	+-----------------+     +--------+
> >>
> >>
> >> 	BASE1			  PLUGIN
> >> 	+-----------------+
> >> 	| CPU0 <-> Codec3 |     +--------+
> >> 	| CPU1		  | <-> | Codec1 |
> >> 	+-----------------+     +--------+
(snip)
> > BASE
> > 	my_card: card {
> > 		links = <&cpu0>;
> > 		routing = "Headphone0", "Codec0"; /* for CPU0-Codec0 */
> > 	};
> >
> > PLUGIN
> > 	&my_card {
> > 		plugin-links = <&cpu1>, <&cpu2>
> > 		plugin-routing = "Headphone1", "Codec1", /* for CPU1-Codec1 */
> > 		^^^^^^		 "Headphone2", "Codec2"; /* for CPU2-Codec2 */
> > 	};
(snip)
> hm, I believe this _should_ work.

I hope so.

>  I also think that we can just drop the whole
> "ignore_route_check" flag idea
(snip)
> also, I believe we can drop the whole "explicitly disabled links" idea
> since IMO, links passed via the "plugin-links" property _must_ exist.

Yes, agree. It is no longer needed on new plugin-xxx idea.

> if we go for this though I think we need to clarify the usage of the
> "plugin-links" and "plugin-routing" properties.

Yes. I think you need to confirm or persuade to DT maintainer that whether
it can be accepted idea or not.

> Thank you very much for this discussion! It was really, really
> helpful!

Same here. I'm very happy could do that


Thank you for your help !!

Best regards
---
Kuninori Morimoto