| Message ID | 20241220071126.1066691-1-ivo.g.dimitrov.75@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [RFC] soc: audio-graph-card2: use correct endpoint when getting link parameters | expand |
ping On 20.12.24 г. 9:11 ч., Ivaylo Dimitrov wrote: > We may have multiple links between ports, with each link > having different parameters. Currently, no matter the topology, > it is always port endpoint 0 that is used when setting parameters. > > On a complex sound system, like the one found on Motorola droid4, > hifi and voice DAIs require differents formats (i2s vs dsp_a) > and curently it is impossible to use DT to set that. > > Implementing the change leads to partially dropping of at least > 0dedbde5062d (ASoC: cpcap: Implement set_tdm_slot for voice call > support), as core does most of what is needed to configure voice DAI. > > We (on Maemo Leste ) use the patch (along with few others) to have > voice calls working properly on d4 through UCM. > > The patch is for linux 6.6, I want to know whether the > approach would be accepted before sending a proper patch for > current master. > > the original commit message follows: > > When link parameters are parsed, it is always endpoint@0 that is used and > parameters set to other endpoints are ignored. > > Fix that by using endpoint that is set in DT when parsing link parameters. > > Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> > --- > sound/soc/generic/audio-graph-card2.c | 59 +++++++++++++-------------- > 1 file changed, 28 insertions(+), 31 deletions(-) > > diff --git a/sound/soc/generic/audio-graph-card2.c b/sound/soc/generic/audio-graph-card2.c > index b1c675c6b6db..163a20c8ffee 100644 > --- a/sound/soc/generic/audio-graph-card2.c > +++ b/sound/soc/generic/audio-graph-card2.c > @@ -508,17 +508,16 @@ static int __graph_parse_node(struct asoc_simple_priv *priv, > > static int graph_parse_node(struct asoc_simple_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; > int ret = 0; > + struct device_node *port = of_get_parent(ep); > + bool is_multi = graph_lnk_is_multi(port); > > - if (graph_lnk_is_multi(port)) { > + if (is_multi) { > int idx; > > - of_node_get(port); > - > for (idx = 0;; idx++) { > ep = graph_get_next_multi_ep(&port); > if (!ep) > @@ -532,9 +531,8 @@ static int graph_parse_node(struct asoc_simple_priv *priv, > } > } else { > /* Single CPU / Codec */ > - ep = port_to_endpoint(port); > + of_node_put(port); > ret = __graph_parse_node(priv, gtype, ep, li, is_cpu, 0); > - of_node_put(ep); > } > > return ret; > @@ -591,22 +589,20 @@ static void graph_parse_daifmt(struct device_node *node, > } > > static void graph_link_init(struct asoc_simple_priv *priv, > - struct device_node *port, > + struct device_node *ep, > struct link_info *li, > int is_cpu_node) > { > struct snd_soc_dai_link *dai_link = simple_priv_to_link(priv, li->link); > - struct device_node *ep; > + struct device_node *port = of_get_parent(ep); > + bool is_multi = graph_lnk_is_multi(port); > struct device_node *ports; > unsigned int daifmt = 0, daiclk = 0; > unsigned int bit_frame = 0; > > - if (graph_lnk_is_multi(port)) { > - of_node_get(port); > + if (is_multi) { > ep = graph_get_next_multi_ep(&port); > port = of_get_parent(ep); > - } else { > - ep = port_to_endpoint(port); > } > > ports = of_get_parent(port); > @@ -642,6 +638,9 @@ static void graph_link_init(struct asoc_simple_priv *priv, > dai_link->ops = &graph_ops; > if (priv->ops) > dai_link->ops = priv->ops; > + > + of_node_put(port); > + of_node_put(ports); > } > > int audio_graph2_link_normal(struct asoc_simple_priv *priv, > @@ -650,7 +649,7 @@ int audio_graph2_link_normal(struct asoc_simple_priv *priv, > { > struct device_node *cpu_port = lnk; > struct device_node *cpu_ep = port_to_endpoint(cpu_port); > - struct device_node *codec_port = of_graph_get_remote_port(cpu_ep); > + struct device_node *codec_ep = of_graph_get_remote_endpoint(cpu_ep); > int ret; > > /* > @@ -658,20 +657,20 @@ int audio_graph2_link_normal(struct asoc_simple_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) > goto err; > > /* > * 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) > goto err; > > - graph_link_init(priv, cpu_port, li, 1); > + graph_link_init(priv, cpu_ep, li, 1); > err: > - of_node_put(codec_port); > + of_node_put(codec_ep); > of_node_put(cpu_ep); > > return ret; > @@ -684,7 +683,6 @@ int audio_graph2_link_dpcm(struct asoc_simple_priv *priv, > { > struct device_node *ep = port_to_endpoint(lnk); > struct device_node *rep = of_graph_get_remote_endpoint(ep); > - struct device_node *rport = of_graph_get_remote_port(ep); > 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 = asoc_graph_is_ports0(lnk); > @@ -718,7 +716,7 @@ int audio_graph2_link_dpcm(struct asoc_simple_priv *priv, > dai_link->dynamic = 1; > dai_link->dpcm_merged_format = 1; > > - ret = graph_parse_node(priv, GRAPH_DPCM, rport, li, 1); > + ret = graph_parse_node(priv, GRAPH_DPCM, rep, li, 1); > if (ret) > goto err; > } else { > @@ -751,7 +749,7 @@ int audio_graph2_link_dpcm(struct asoc_simple_priv *priv, > dai_link->no_pcm = 1; > dai_link->be_hw_params_fixup = asoc_simple_be_hw_params_fixup; > > - ret = graph_parse_node(priv, GRAPH_DPCM, rport, li, 0); > + ret = graph_parse_node(priv, GRAPH_DPCM, rep, li, 0); > if (ret < 0) > goto err; > } > @@ -761,11 +759,10 @@ int audio_graph2_link_dpcm(struct asoc_simple_priv *priv, > > snd_soc_dai_link_set_capabilities(dai_link); > > - graph_link_init(priv, rport, li, is_cpu); > + graph_link_init(priv, rep, li, is_cpu); > err: > of_node_put(ep); > of_node_put(rep); > - of_node_put(rport); > > return ret; > } > @@ -777,7 +774,7 @@ int audio_graph2_link_c2c(struct asoc_simple_priv *priv, > { > struct snd_soc_dai_link *dai_link = simple_priv_to_link(priv, li->link); > struct device_node *port0, *port1, *ports; > - struct device_node *codec0_port, *codec1_port; > + struct device_node *codec0_ep, *codec1_ep; > struct device_node *ep0, *ep1; > u32 val = 0; > int ret = -EINVAL; > @@ -834,31 +831,31 @@ int audio_graph2_link_c2c(struct asoc_simple_priv *priv, > ep0 = port_to_endpoint(port0); > ep1 = port_to_endpoint(port1); > > - codec0_port = of_graph_get_remote_port(ep0); > - codec1_port = of_graph_get_remote_port(ep1); > + codec0_ep = of_graph_get_remote_endpoint(ep0); > + codec1_ep = 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) > goto err2; > > /* > * 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) > goto err2; > > - graph_link_init(priv, codec0_port, li, 1); > + graph_link_init(priv, codec0_ep, li, 1); > err2: > of_node_put(ep0); > of_node_put(ep1); > - of_node_put(codec0_port); > - of_node_put(codec1_port); > + of_node_put(codec0_ep); > + of_node_put(codec1_ep); > err1: > of_node_put(ports); > of_node_put(port0);
On Mon, Jan 13, 2025 at 07:55:30AM +0200, Ivaylo Dimitrov wrote:
> ping
Please don't send content free pings and please allow a reasonable time
for review. People get busy, go on holiday, attend conferences and so
on so unless there is some reason for urgency (like critical bug fixes)
please allow at least a couple of weeks for review. If there have been
review comments then people may be waiting for those to be addressed.
Sending content free pings adds to the mail volume (if they are seen at
all) which is often the problem and since they can't be reviewed
directly if something has gone wrong you'll have to resend the patches
anyway, so sending again is generally a better approach though there are
some other maintainers who like them - if in doubt look at how patches
for the subsystem are normally handled.
Please don't top post, reply in line with needed context. This allows
readers to readily follow the flow of conversation and understand what
you are talking about and also helps ensure that everything in the
discussion is being addressed.
Hi, On 13.01.25 г. 15:40 ч., Mark Brown wrote: > On Mon, Jan 13, 2025 at 07:55:30AM +0200, Ivaylo Dimitrov wrote: >> ping > > Please don't send content free pings and please allow a reasonable time > for review. People get busy, go on holiday, attend conferences and so > on so unless there is some reason for urgency (like critical bug fixes) > please allow at least a couple of weeks for review. If there have been > review comments then people may be waiting for those to be addressed. > > Sending content free pings adds to the mail volume (if they are seen at > all) which is often the problem and since they can't be reviewed > directly if something has gone wrong you'll have to resend the patches > anyway, so sending again is generally a better approach though there are > some other maintainers who like them - if in doubt look at how patches > for the subsystem are normally handled. > > Please don't top post, reply in line with needed context. This allows > readers to readily follow the flow of conversation and understand what > you are talking about and also helps ensure that everything in the > discussion is being addressed. I understand people are busy, but I also see community sent patches being treated with low priority, or being silently ignored too often lately, but lets not go into that. I sent that RFC patch on 20.12.2024, today is 13.10.2025 - if this is not a reasonable time, well, what is? By the same time I sent 2 other patches and they are already in -next. In the meanwhile I see patches sent in the morning to be reviewed till the end of the day - not critical bugfixes patches but new functionality. Also, I don't understand how the ping was content free, given that it was on top of the original patch, unless I don't get what "content free" is supposed to mean, possible, I am not native English speaker. Regards, Ivo
On Mon, Jan 13, 2025 at 06:24:18PM +0200, Ivaylo Dimitrov wrote: > I understand people are busy, but I also see community sent patches being > treated with low priority, or being silently ignored too often lately, but > lets not go into that. > I sent that RFC patch on 20.12.2024, today is 13.10.2025 - if this is not a > reasonable time, well, what is? By the same time I sent 2 other patches and > they are already in -next. In the meanwhile I see patches sent in the > morning to be reviewed till the end of the day - not critical bugfixes > patches but new functionality. Well, you've used a subject line for a different subsystem so there's a good chance that I simply didn't look at the mail beyond that (many of us get a lot of random CCs). You also don't seem to have CCed the ALSA list, nor for that matter Morimoto-san who maintains the generic card so perhaps I was just waiting for his review. I honestly can't remember. I'll also note that there's only been a week of work time for me so far this year, and you sent this on the last day I worked last year. > Also, I don't understand how the ping was content free, given that it was on > top of the original patch, unless I don't get what "content free" is > supposed to mean, possible, I am not native English speaker. Your mail added the single word "ping". That is not saying anything meaningful so adds nothing, and as the form letter I sent indicated results in a mail that's not directly actionable. As it says: | all) which is often the problem and since they can't be reviewed | directly if something has gone wrong you'll have to resend the patches | anyway, so sending again is generally a better approach though there are | some other maintainers who like them - if in doubt look at how patches | for the subsystem are normally handled. Blub for the subject letter thing: Please submit patches using subject lines reflecting the style for the subsystem, this makes it easier for people to identify relevant patches. Look at what existing commits in the area you're changing are doing and make sure your subject lines visually resemble what they're doing. There's no need to resubmit to fix this alone.
On 13.01.25 г. 19:01 ч., Mark Brown wrote: > On Mon, Jan 13, 2025 at 06:24:18PM +0200, Ivaylo Dimitrov wrote: > >> I understand people are busy, but I also see community sent patches being >> treated with low priority, or being silently ignored too often lately, but >> lets not go into that. > >> I sent that RFC patch on 20.12.2024, today is 13.10.2025 - if this is not a >> reasonable time, well, what is? By the same time I sent 2 other patches and >> they are already in -next. In the meanwhile I see patches sent in the >> morning to be reviewed till the end of the day - not critical bugfixes >> patches but new functionality. > > Well, you've used a subject line for a different subsystem so there's a > good chance that I simply didn't look at the mail beyond that (many of > us get a lot of random CCs). You also don't seem to have CCed the ALSA > list, nor for that matter Morimoto-san who maintains the generic card so > perhaps I was just waiting for his review. I honestly can't remember. > I'll also note that there's only been a week of work time for me so far > this year, and you sent this on the last day I worked last year. > Honestly, I was surprised Morimoto-san was missing, but see: ivo@ivo-H81M-S2PV:/mnt/VM/home/user/linux/droid4-linux$ ./scripts/get_maintainer.pl 0001-soc-audio-graph-card2-use-correct-endpoint-when-gett.patch Liam Girdwood <lgirdwood@gmail.com> (supporter:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...) Mark Brown <broonie@kernel.org> (supporter:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...) Jaroslav Kysela <perex@perex.cz> (maintainer:SOUND) Takashi Iwai <tiwai@suse.com> (maintainer:SOUND) Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> (commit_signer:1/1=100%,authored:1/1=100%,added_lines:28/28=100%,removed_lines:31/31=100%) alsa-devel@alsa-project.org (moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...) linux-kernel@vger.kernel.org (open list) this is on 6.6.y, against which the RFC patch is. Perhaps I should have called get_maintainer.pl in -next tree, my bad, will note for the future. >> Also, I don't understand how the ping was content free, given that it was on >> top of the original patch, unless I don't get what "content free" is >> supposed to mean, possible, I am not native English speaker. > > Your mail added the single word "ping". That is not saying anything > meaningful so adds nothing, and as the form letter I sent indicated > results in a mail that's not directly actionable. As it says: > > | all) which is often the problem and since they can't be reviewed > | directly if something has gone wrong you'll have to resend the patches > | anyway, so sending again is generally a better approach though there are > | some other maintainers who like them - if in doubt look at how patches > | for the subsystem are normally handled. > > Blub for the subject letter thing: > > Please submit patches using subject lines reflecting the style for the > subsystem, this makes it easier for people to identify relevant patches. > Look at what existing commits in the area you're changing are doing and > make sure your subject lines visually resemble what they're doing. > There's no need to resubmit to fix this alone. oh, yeah, sorry, that should have been ASoC:, not soc: Ok, I think both of us wasted lots of cycles in vain, please, just confirm if I shall do anything else but wait. Thanks and regards, Ivo
Hi Ivaylo Sorry for the late review. > We may have multiple links between ports, with each link > having different parameters. Currently, no matter the topology, > it is always port endpoint 0 that is used when setting parameters. > > On a complex sound system, like the one found on Motorola droid4, > hifi and voice DAIs require differents formats (i2s vs dsp_a) > and curently it is impossible to use DT to set that. > > Implementing the change leads to partially dropping of at least > 0dedbde5062d (ASoC: cpcap: Implement set_tdm_slot for voice call > support), as core does most of what is needed to configure voice DAI. > > We (on Maemo Leste ) use the patch (along with few others) to have > voice calls working properly on d4 through UCM. > > The patch is for linux 6.6, I want to know whether the > approach would be accepted before sending a proper patch for > current master. > > the original commit message follows: > > When link parameters are parsed, it is always endpoint@0 that is used and > parameters set to other endpoints are ignored. > > Fix that by using endpoint that is set in DT when parsing link parameters. > > Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> > --- (snip) > @@ -684,7 +683,6 @@ int audio_graph2_link_dpcm(struct asoc_simple_priv *priv, > { > struct device_node *ep = port_to_endpoint(lnk); > struct device_node *rep = of_graph_get_remote_endpoint(ep); > - struct device_node *rport = of_graph_get_remote_port(ep); > 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 = asoc_graph_is_ports0(lnk); > @@ -718,7 +716,7 @@ int audio_graph2_link_dpcm(struct asoc_simple_priv *priv, > dai_link->dynamic = 1; > dai_link->dpcm_merged_format = 1; > > - ret = graph_parse_node(priv, GRAPH_DPCM, rport, li, 1); > + ret = graph_parse_node(priv, GRAPH_DPCM, rep, li, 1); Please correct me if I was misunderstanding Is the main issue "remote" side endpoint ? You want to parse "remote" endpoint (= rep) directly, but the function requests "port" (= rport), and it will use endpoint0 ( != rep). Is this the main issue you want to fix ? Thank you for your help !! Best regards --- Kuninori Morimoto
Hi Morimoto-san On 14.01.25 г. 8:44 ч., Kuninori Morimoto wrote: > > Hi Ivaylo > > Sorry for the late review. > And sorry for the noise on my side. >> We may have multiple links between ports, with each link >> having different parameters. Currently, no matter the topology, >> it is always port endpoint 0 that is used when setting parameters. >> >> On a complex sound system, like the one found on Motorola droid4, >> hifi and voice DAIs require differents formats (i2s vs dsp_a) >> and curently it is impossible to use DT to set that. >> >> Implementing the change leads to partially dropping of at least >> 0dedbde5062d (ASoC: cpcap: Implement set_tdm_slot for voice call >> support), as core does most of what is needed to configure voice DAI. >> >> We (on Maemo Leste ) use the patch (along with few others) to have >> voice calls working properly on d4 through UCM. >> >> The patch is for linux 6.6, I want to know whether the >> approach would be accepted before sending a proper patch for >> current master. >> >> the original commit message follows: >> >> When link parameters are parsed, it is always endpoint@0 that is used and >> parameters set to other endpoints are ignored. >> >> Fix that by using endpoint that is set in DT when parsing link parameters. >> >> Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> >> --- > (snip) >> @@ -684,7 +683,6 @@ int audio_graph2_link_dpcm(struct asoc_simple_priv *priv, >> { >> struct device_node *ep = port_to_endpoint(lnk); >> struct device_node *rep = of_graph_get_remote_endpoint(ep); >> - struct device_node *rport = of_graph_get_remote_port(ep); >> 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 = asoc_graph_is_ports0(lnk); >> @@ -718,7 +716,7 @@ int audio_graph2_link_dpcm(struct asoc_simple_priv *priv, >> dai_link->dynamic = 1; >> dai_link->dpcm_merged_format = 1; >> >> - ret = graph_parse_node(priv, GRAPH_DPCM, rport, li, 1); >> + ret = graph_parse_node(priv, GRAPH_DPCM, rep, li, 1); > > Please correct me if I was misunderstanding > Is the main issue "remote" side endpoint ? > > You want to parse "remote" endpoint (= rep) directly, but the function > requests "port" (= rport), and it will use endpoint0 ( != rep). > Is this the main issue you want to fix ? > Yes, it is the 'remote' side endpoint, currently it is always remote endpoint0 that is used, because when you get 'port', it is endpoint0 of that port that core uses. See: https://github.com/maemo-leste/droid4-linux/blob/maemo-6.6.y/arch/arm/boot/dts/ti/omap/motorola-cpcap-mapphone.dtsi#L91 https://github.com/maemo-leste/droid4-linux/blob/maemo-6.6.y/arch/arm/boot/dts/ti/omap/motorola-mapphone-handset.dtsi#L65 and https://github.com/maemo-leste/droid4-linux/blob/maemo-6.6.y/arch/arm/boot/dts/ti/omap/motorola-mapphone-common.dtsi#L476 as an example DTS that is using multiple endpoints per port and also https://lkml.org/lkml/2018/3/27/1225 for what audio wiring looks like. For voice calls the device does not use CPU, but we have C2C link between modem and cpcap instead. However, we must correctly set DAI format on cpcap side for for that link to work properly. General speaking, we might have multiple endpoints connected for a single port and when getting "link properties" I think we should use remote endpoint that is linked to local endpoint, not always remote endpoint0. If it is still not clear what $subject patch tries to achieve, please LMK and I'll try to elaborate even more, if possible. Also, if you think core allows such 'routing' to be implemented without the $subject functionality, please elaborate. I spent a good amount of time back then with no luck. Thanks and regards, Ivo
Hi Ivaylo Thank you for clarify your situation. > > You want to parse "remote" endpoint (= rep) directly, but the function > > requests "port" (= rport), and it will use endpoint0 ( != rep). > > Is this the main issue you want to fix ? > > > > Yes, it is the 'remote' side endpoint, currently it is always remote > endpoint0 that is used, because when you get 'port', it is endpoint0 of > that port that core uses. OK, I could understand, and I can agree to your idea. Getting "port" from "endpoint" is always stable, but getting "endpoint" from "port" without parameter will be issue, indeed. But I guess your original patch is based on very old kernel ? It can't be applied to Mark's for-6.14 branch as-is. Please based on latest branch. And about git-comment, When link parameters are parsed, it is always endpoint@0 that is used and parameters set to other endpoints are ignored. Please indicate that current function requests "port" as parameter, thus, it always selects endpoint0, etc. That is easy to understand. Thank you for your help !! Best regards --- Kuninori Morimoto
Hi Morimoto-san, On 15.01.25 г. 1:49 ч., Kuninori Morimoto wrote: > > Hi Ivaylo > > Thank you for clarify your situation. > >>> You want to parse "remote" endpoint (= rep) directly, but the function >>> requests "port" (= rport), and it will use endpoint0 ( != rep). >>> Is this the main issue you want to fix ? >>> >> >> Yes, it is the 'remote' side endpoint, currently it is always remote >> endpoint0 that is used, because when you get 'port', it is endpoint0 of >> that port that core uses. > > OK, I could understand, and I can agree to your idea. > Getting "port" from "endpoint" is always stable, but getting "endpoint" > from "port" without parameter will be issue, indeed. > > But I guess your original patch is based on very old kernel ? > It can't be applied to Mark's for-6.14 branch as-is. > Please based on latest branch. > Yes, it is based in 6.6, that's why I sent RFC patch, as rebasing will not be trivial and I didn't want to spend time on something that could possibly be rejected. > And about git-comment, > > When link parameters are parsed, it is always endpoint@0 that is used and > parameters set to other endpoints are ignored. > > Please indicate that current function requests "port" as parameter, > thus, it always selects endpoint0, etc. That is easy to understand. > Ok, will do. Thanks! Ivo
diff --git a/sound/soc/generic/audio-graph-card2.c b/sound/soc/generic/audio-graph-card2.c index b1c675c6b6db..163a20c8ffee 100644 --- a/sound/soc/generic/audio-graph-card2.c +++ b/sound/soc/generic/audio-graph-card2.c @@ -508,17 +508,16 @@ static int __graph_parse_node(struct asoc_simple_priv *priv, static int graph_parse_node(struct asoc_simple_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; int ret = 0; + struct device_node *port = of_get_parent(ep); + bool is_multi = graph_lnk_is_multi(port); - if (graph_lnk_is_multi(port)) { + if (is_multi) { int idx; - of_node_get(port); - for (idx = 0;; idx++) { ep = graph_get_next_multi_ep(&port); if (!ep) @@ -532,9 +531,8 @@ static int graph_parse_node(struct asoc_simple_priv *priv, } } else { /* Single CPU / Codec */ - ep = port_to_endpoint(port); + of_node_put(port); ret = __graph_parse_node(priv, gtype, ep, li, is_cpu, 0); - of_node_put(ep); } return ret; @@ -591,22 +589,20 @@ static void graph_parse_daifmt(struct device_node *node, } static void graph_link_init(struct asoc_simple_priv *priv, - struct device_node *port, + struct device_node *ep, struct link_info *li, int is_cpu_node) { struct snd_soc_dai_link *dai_link = simple_priv_to_link(priv, li->link); - struct device_node *ep; + struct device_node *port = of_get_parent(ep); + bool is_multi = graph_lnk_is_multi(port); struct device_node *ports; unsigned int daifmt = 0, daiclk = 0; unsigned int bit_frame = 0; - if (graph_lnk_is_multi(port)) { - of_node_get(port); + if (is_multi) { ep = graph_get_next_multi_ep(&port); port = of_get_parent(ep); - } else { - ep = port_to_endpoint(port); } ports = of_get_parent(port); @@ -642,6 +638,9 @@ static void graph_link_init(struct asoc_simple_priv *priv, dai_link->ops = &graph_ops; if (priv->ops) dai_link->ops = priv->ops; + + of_node_put(port); + of_node_put(ports); } int audio_graph2_link_normal(struct asoc_simple_priv *priv, @@ -650,7 +649,7 @@ int audio_graph2_link_normal(struct asoc_simple_priv *priv, { struct device_node *cpu_port = lnk; struct device_node *cpu_ep = port_to_endpoint(cpu_port); - struct device_node *codec_port = of_graph_get_remote_port(cpu_ep); + struct device_node *codec_ep = of_graph_get_remote_endpoint(cpu_ep); int ret; /* @@ -658,20 +657,20 @@ int audio_graph2_link_normal(struct asoc_simple_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) goto err; /* * 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) goto err; - graph_link_init(priv, cpu_port, li, 1); + graph_link_init(priv, cpu_ep, li, 1); err: - of_node_put(codec_port); + of_node_put(codec_ep); of_node_put(cpu_ep); return ret; @@ -684,7 +683,6 @@ int audio_graph2_link_dpcm(struct asoc_simple_priv *priv, { struct device_node *ep = port_to_endpoint(lnk); struct device_node *rep = of_graph_get_remote_endpoint(ep); - struct device_node *rport = of_graph_get_remote_port(ep); 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 = asoc_graph_is_ports0(lnk); @@ -718,7 +716,7 @@ int audio_graph2_link_dpcm(struct asoc_simple_priv *priv, dai_link->dynamic = 1; dai_link->dpcm_merged_format = 1; - ret = graph_parse_node(priv, GRAPH_DPCM, rport, li, 1); + ret = graph_parse_node(priv, GRAPH_DPCM, rep, li, 1); if (ret) goto err; } else { @@ -751,7 +749,7 @@ int audio_graph2_link_dpcm(struct asoc_simple_priv *priv, dai_link->no_pcm = 1; dai_link->be_hw_params_fixup = asoc_simple_be_hw_params_fixup; - ret = graph_parse_node(priv, GRAPH_DPCM, rport, li, 0); + ret = graph_parse_node(priv, GRAPH_DPCM, rep, li, 0); if (ret < 0) goto err; } @@ -761,11 +759,10 @@ int audio_graph2_link_dpcm(struct asoc_simple_priv *priv, snd_soc_dai_link_set_capabilities(dai_link); - graph_link_init(priv, rport, li, is_cpu); + graph_link_init(priv, rep, li, is_cpu); err: of_node_put(ep); of_node_put(rep); - of_node_put(rport); return ret; } @@ -777,7 +774,7 @@ int audio_graph2_link_c2c(struct asoc_simple_priv *priv, { struct snd_soc_dai_link *dai_link = simple_priv_to_link(priv, li->link); struct device_node *port0, *port1, *ports; - struct device_node *codec0_port, *codec1_port; + struct device_node *codec0_ep, *codec1_ep; struct device_node *ep0, *ep1; u32 val = 0; int ret = -EINVAL; @@ -834,31 +831,31 @@ int audio_graph2_link_c2c(struct asoc_simple_priv *priv, ep0 = port_to_endpoint(port0); ep1 = port_to_endpoint(port1); - codec0_port = of_graph_get_remote_port(ep0); - codec1_port = of_graph_get_remote_port(ep1); + codec0_ep = of_graph_get_remote_endpoint(ep0); + codec1_ep = 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) goto err2; /* * 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) goto err2; - graph_link_init(priv, codec0_port, li, 1); + graph_link_init(priv, codec0_ep, li, 1); err2: of_node_put(ep0); of_node_put(ep1); - of_node_put(codec0_port); - of_node_put(codec1_port); + of_node_put(codec0_ep); + of_node_put(codec1_ep); err1: of_node_put(ports); of_node_put(port0);
We may have multiple links between ports, with each link having different parameters. Currently, no matter the topology, it is always port endpoint 0 that is used when setting parameters. On a complex sound system, like the one found on Motorola droid4, hifi and voice DAIs require differents formats (i2s vs dsp_a) and curently it is impossible to use DT to set that. Implementing the change leads to partially dropping of at least 0dedbde5062d (ASoC: cpcap: Implement set_tdm_slot for voice call support), as core does most of what is needed to configure voice DAI. We (on Maemo Leste ) use the patch (along with few others) to have voice calls working properly on d4 through UCM. The patch is for linux 6.6, I want to know whether the approach would be accepted before sending a proper patch for current master. the original commit message follows: When link parameters are parsed, it is always endpoint@0 that is used and parameters set to other endpoints are ignored. Fix that by using endpoint that is set in DT when parsing link parameters. Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> --- sound/soc/generic/audio-graph-card2.c | 59 +++++++++++++-------------- 1 file changed, 28 insertions(+), 31 deletions(-)