sound/soc/generic/audio-graph-card2.c | 59 +++++++++++++-------------- 1 file changed, 28 insertions(+), 31 deletions(-)
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);
--
2.30.2
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
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>
---
sound/soc/generic/audio-graph-card2.c | 65 +++++++++++++--------------
1 file changed, 30 insertions(+), 35 deletions(-)
diff --git a/sound/soc/generic/audio-graph-card2.c b/sound/soc/generic/audio-graph-card2.c
index c36b1a2ac949..ea2da1b78b41 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);
@@ -831,9 +830,8 @@ int audio_graph2_link_normal(struct simple_util_priv *priv,
struct device_node *lnk,
struct link_info *li)
{
- 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 *cpu_ep __free(device_node) = of_graph_get_next_port_endpoint(lnk, NULL);
+ struct device_node *codec_ep __free(device_node) = of_graph_get_remote_endpoint(cpu_ep);
int ret;
/*
@@ -841,18 +839,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 +862,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 +899,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 +935,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 +1008,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;
}
--
2.30.2
Hi 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>
> ---
(snip)
> @@ -831,9 +830,8 @@ int audio_graph2_link_normal(struct simple_util_priv *priv,
> struct device_node *lnk,
> struct link_info *li)
> {
> - 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 *cpu_ep __free(device_node) = of_graph_get_next_port_endpoint(lnk, NULL);
> + struct device_node *codec_ep __free(device_node) = of_graph_get_remote_endpoint(cpu_ep);
> int ret;
You don't need to change cpu_port/cpu_ep here ?
And, I would like to keep "cpu_port = lnk" here.
Except above
Acked-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Thank you for your help !!
Best regards
---
Kuninori Morimoto
Hi Morimoto-san,
On 21.01.25 г. 1:35 ч., Kuninori Morimoto wrote:
>
> Hi 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>
>> ---
> (snip)
>> @@ -831,9 +830,8 @@ int audio_graph2_link_normal(struct simple_util_priv *priv,
>> struct device_node *lnk,
>> struct link_info *li)
>> {
>> - 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 *cpu_ep __free(device_node) = of_graph_get_next_port_endpoint(lnk, NULL);
>> + struct device_node *codec_ep __free(device_node) = of_graph_get_remote_endpoint(cpu_ep);
>> int ret;
>
> You don't need to change cpu_port/cpu_ep here ?
> And, I would like to keep "cpu_port = lnk" here.
>
cpu_port will be used on the next line only, but ok, will send v2 with
the above changes.
Thanks,
Ivo
> Except above
> Acked-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
>
> Thank you for your help !!
>
> Best regards
> ---
> Kuninori Morimoto
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(-)
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;
}
--
2.25.1On Tue, 21 Jan 2025 08:48:15 +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.
>
> Fix that by passing endpoint instead of port when parsing link parameters.
>
>
> [...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/1] ASoC: audio-graph-card2: use correct endpoint when getting link parameters
commit: e935f903ab9bee43f3375883c230a32138ae3d1d
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
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
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.
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
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.
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
© 2016 - 2026 Red Hat, Inc.