diff mbox series

[v2] ASoC: audio-graph-card2: use correct endpoint when getting link parameters

Message ID 20250121064815.741820-1-ivo.g.dimitrov.75@gmail.com (mailing list archive)
State New
Headers show
Series [v2] ASoC: audio-graph-card2: use correct endpoint when getting link parameters | expand

Commit Message

Ivaylo Dimitrov Jan. 21, 2025, 6:48 a.m. UTC 
When link DT nodes are parsed, most functions get port as a parameter,
which results in port endpoint@0 always being used. However, each endpoint
might have different settings, but those are currently ignored.

Fix that by passing endpoint instead of port when parsing link parameters.

Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
Acked-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/generic/audio-graph-card2.c | 62 +++++++++++++--------------
 1 file changed, 29 insertions(+), 33 deletions(-)

Comments

Mark Brown Jan. 21, 2025, 12:18 p.m. UTC | #1 
On Tue, Jan 21, 2025 at 08:48:15AM +0200, Ivaylo Dimitrov wrote:
> When link DT nodes are parsed, most functions get port as a parameter,
> which results in port endpoint@0 always being used. However, each endpoint
> might have different settings, but those are currently ignored.

Please don't send new patches in reply to old patches or serieses, this
makes it harder for both people and tools to understand what is going
on - it can bury things in mailboxes and make it difficult to keep track
of what current patches are, both for the new patches and the old ones.
Ivaylo Dimitrov Jan. 21, 2025, 1:33 p.m. UTC | #2 
On 21.01.25 г. 14:18 ч., Mark Brown wrote:
> On Tue, Jan 21, 2025 at 08:48:15AM +0200, Ivaylo Dimitrov wrote:
>> When link DT nodes are parsed, most functions get port as a parameter,
>> which results in port endpoint@0 always being used. However, each endpoint
>> might have different settings, but those are currently ignored.
> 
> Please don't send new patches in reply to old patches or serieses, this
> makes it harder for both people and tools to understand what is going
> on - it can bury things in mailboxes and make it difficult to keep track
> of what current patches are, both for the new patches and the old ones.
Ok, will know for the future. Shall I resend?

Thanks,
Ivo
Mark Brown Jan. 21, 2025, 1:41 p.m. UTC | #3 
On Tue, Jan 21, 2025 at 03:33:30PM +0200, Ivaylo Dimitrov wrote:

> Ok, will know for the future. Shall I resend?

No need, it's fine - hopefully Morimoto-san can review.
Kuninori Morimoto Jan. 21, 2025, 11:22 p.m. UTC | #4 
Hi Mark, Ivaylo

Thank you for the patch

> When link DT nodes are parsed, most functions get port as a parameter,
> which results in port endpoint@0 always being used. However, each endpoint
> might have different settings, but those are currently ignored.
> 
> Fix that by passing endpoint instead of port when parsing link parameters.
> 
> Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
> Acked-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> ---

Looks good to me. Thanks
It already has the tag, but...

Acked-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>


Thank you for your help !!

Best regards
---
Kuninori Morimoto
diff mbox series

Patch

diff --git a/sound/soc/generic/audio-graph-card2.c b/sound/soc/generic/audio-graph-card2.c
index c36b1a2ac949..ee94b256b770 100644
--- a/sound/soc/generic/audio-graph-card2.c
+++ b/sound/soc/generic/audio-graph-card2.c
@@ -648,23 +648,23 @@  static int graph_parse_node_multi(struct simple_util_priv *priv,
 
 static int graph_parse_node_single(struct simple_util_priv *priv,
 				   enum graph_type gtype,
-				   struct device_node *port,
+				   struct device_node *ep,
 				   struct link_info *li, int is_cpu)
 {
-	struct device_node *ep __free(device_node) = of_graph_get_next_port_endpoint(port, NULL);
-
 	return __graph_parse_node(priv, gtype, ep, li, is_cpu, 0);
 }
 
 static int graph_parse_node(struct simple_util_priv *priv,
 			    enum graph_type gtype,
-			    struct device_node *port,
+			    struct device_node *ep,
 			    struct link_info *li, int is_cpu)
 {
+	struct device_node *port __free(device_node) = ep_to_port(ep);
+
 	if (graph_lnk_is_multi(port))
 		return graph_parse_node_multi(priv, gtype, port, li, is_cpu);
 	else
-		return graph_parse_node_single(priv, gtype, port, li, is_cpu);
+		return graph_parse_node_single(priv, gtype, ep, li, is_cpu);
 }
 
 static void graph_parse_daifmt(struct device_node *node, unsigned int *daifmt)
@@ -722,14 +722,15 @@  static unsigned int graph_parse_bitframe(struct device_node *ep)
 
 static void graph_link_init(struct simple_util_priv *priv,
 			    struct device_node *lnk,
-			    struct device_node *port_cpu,
-			    struct device_node *port_codec,
+			    struct device_node *ep_cpu,
+			    struct device_node *ep_codec,
 			    struct link_info *li,
 			    int is_cpu_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 device_node *ep_cpu, *ep_codec;
+	struct device_node *port_cpu = ep_to_port(ep_cpu);
+	struct device_node *port_codec = ep_to_port(ep_codec);
 	struct device_node *multi_cpu_port = NULL, *multi_codec_port = NULL;
 	struct snd_soc_dai_link_component *dlc;
 	unsigned int daifmt = 0;
@@ -739,25 +740,23 @@  static void graph_link_init(struct simple_util_priv *priv,
 	int multi_cpu_port_idx = 1, multi_codec_port_idx = 1;
 	int i;
 
-	of_node_get(port_cpu);
 	if (graph_lnk_is_multi(port_cpu)) {
 		multi_cpu_port = port_cpu;
 		ep_cpu = graph_get_next_multi_ep(&multi_cpu_port, multi_cpu_port_idx++);
 		of_node_put(port_cpu);
 		port_cpu = ep_to_port(ep_cpu);
 	} else {
-		ep_cpu = of_graph_get_next_port_endpoint(port_cpu, NULL);
+		of_node_get(ep_cpu);
 	}
 	struct device_node *ports_cpu __free(device_node) = port_to_ports(port_cpu);
 
-	of_node_get(port_codec);
 	if (graph_lnk_is_multi(port_codec)) {
 		multi_codec_port = port_codec;
 		ep_codec = graph_get_next_multi_ep(&multi_codec_port, multi_codec_port_idx++);
 		of_node_put(port_codec);
 		port_codec = ep_to_port(ep_codec);
 	} else {
-		ep_codec = of_graph_get_next_port_endpoint(port_codec, NULL);
+		of_node_get(ep_codec);
 	}
 	struct device_node *ports_codec __free(device_node) = port_to_ports(port_codec);
 
@@ -833,7 +832,7 @@  int audio_graph2_link_normal(struct simple_util_priv *priv,
 {
 	struct device_node *cpu_port = lnk;
 	struct device_node *cpu_ep	__free(device_node) = of_graph_get_next_port_endpoint(cpu_port, NULL);
-	struct device_node *codec_port	__free(device_node) = of_graph_get_remote_port(cpu_ep);
+	struct device_node *codec_ep	__free(device_node) = of_graph_get_remote_endpoint(cpu_ep);
 	int ret;
 
 	/*
@@ -841,18 +840,18 @@  int audio_graph2_link_normal(struct simple_util_priv *priv,
 	 * see
 	 *	__graph_parse_node() :: DAI Naming
 	 */
-	ret = graph_parse_node(priv, GRAPH_NORMAL, codec_port, li, 0);
+	ret = graph_parse_node(priv, GRAPH_NORMAL, codec_ep, li, 0);
 	if (ret < 0)
 		return ret;
 
 	/*
 	 * call CPU, and set DAI Name
 	 */
-	ret = graph_parse_node(priv, GRAPH_NORMAL, cpu_port, li, 1);
+	ret = graph_parse_node(priv, GRAPH_NORMAL, cpu_ep, li, 1);
 	if (ret < 0)
 		return ret;
 
-	graph_link_init(priv, lnk, cpu_port, codec_port, li, 1);
+	graph_link_init(priv, lnk, cpu_ep, codec_ep, li, 1);
 
 	return ret;
 }
@@ -864,15 +863,15 @@  int audio_graph2_link_dpcm(struct simple_util_priv *priv,
 {
 	struct device_node *ep	__free(device_node) = of_graph_get_next_port_endpoint(lnk, NULL);
 	struct device_node *rep	__free(device_node) = of_graph_get_remote_endpoint(ep);
-	struct device_node *cpu_port = NULL;
-	struct device_node *codec_port = NULL;
+	struct device_node *cpu_ep = NULL;
+	struct device_node *codec_ep = NULL;
 	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);
 	int is_cpu = graph_util_is_ports0(lnk);
 	int ret;
 
 	if (is_cpu) {
-		cpu_port = of_graph_get_remote_port(ep); /* rport */
+		cpu_ep = rep;
 
 		/*
 		 * dpcm {
@@ -901,12 +900,12 @@  int audio_graph2_link_dpcm(struct simple_util_priv *priv,
 		dai_link->dynamic		= 1;
 		dai_link->dpcm_merged_format	= 1;
 
-		ret = graph_parse_node(priv, GRAPH_DPCM, cpu_port, li, 1);
+		ret = graph_parse_node(priv, GRAPH_DPCM, cpu_ep, li, 1);
 		if (ret)
-			goto err;
+			return ret;
 
 	} else {
-		codec_port = of_graph_get_remote_port(ep); /* rport */
+		codec_ep = rep;
 
 		/*
 		 * dpcm {
@@ -937,18 +936,15 @@  int audio_graph2_link_dpcm(struct simple_util_priv *priv,
 		dai_link->no_pcm		= 1;
 		dai_link->be_hw_params_fixup	= simple_util_be_hw_params_fixup;
 
-		ret = graph_parse_node(priv, GRAPH_DPCM, codec_port, li, 0);
+		ret = graph_parse_node(priv, GRAPH_DPCM, codec_ep, li, 0);
 		if (ret < 0)
-			goto err;
+			return ret;
 	}
 
 	graph_parse_convert(ep,  dai_props); /* at node of <dpcm> */
 	graph_parse_convert(rep, dai_props); /* at node of <CPU/Codec> */
 
-	graph_link_init(priv, lnk, cpu_port, codec_port, li, is_cpu);
-err:
-	of_node_put(cpu_port);
-	of_node_put(codec_port);
+	graph_link_init(priv, lnk, cpu_ep, codec_ep, li, is_cpu);
 
 	return ret;
 }
@@ -1013,26 +1009,26 @@  int audio_graph2_link_c2c(struct simple_util_priv *priv,
 	struct device_node *ep0 __free(device_node) = of_graph_get_next_port_endpoint(port0, NULL);
 	struct device_node *ep1 __free(device_node) = of_graph_get_next_port_endpoint(port1, NULL);
 
-	struct device_node *codec0_port __free(device_node) = of_graph_get_remote_port(ep0);
-	struct device_node *codec1_port __free(device_node) = of_graph_get_remote_port(ep1);
+	struct device_node *codec0_ep __free(device_node) = of_graph_get_remote_endpoint(ep0);
+	struct device_node *codec1_ep __free(device_node) = of_graph_get_remote_endpoint(ep1);
 
 	/*
 	 * call Codec first.
 	 * see
 	 *	__graph_parse_node() :: DAI Naming
 	 */
-	ret = graph_parse_node(priv, GRAPH_C2C, codec1_port, li, 0);
+	ret = graph_parse_node(priv, GRAPH_C2C, codec1_ep, li, 0);
 	if (ret < 0)
 		return ret;
 
 	/*
 	 * call CPU, and set DAI Name
 	 */
-	ret = graph_parse_node(priv, GRAPH_C2C, codec0_port, li, 1);
+	ret = graph_parse_node(priv, GRAPH_C2C, codec0_ep, li, 1);
 	if (ret < 0)
 		return ret;
 
-	graph_link_init(priv, lnk, codec0_port, codec1_port, li, 1);
+	graph_link_init(priv, lnk, codec0_ep, codec1_ep, li, 1);
 
 	return ret;
 }