sound/soc/sh/rcar/core.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
On the HiHope boards, we have a single port with a single endpoint defined
as below:
....
rsnd_port: port {
rsnd_endpoint: endpoint {
remote-endpoint = <&dw_hdmi0_snd_in>;
dai-format = "i2s";
bitclock-master = <&rsnd_endpoint>;
frame-master = <&rsnd_endpoint>;
playback = <&ssi2>;
};
};
....
With commit 547b02f74e4a ("ASoC: rsnd: enable multi Component support for
Audio Graph Card/Card2"), support for multiple ports was added. This caused
probe failures on HiHope boards, as the endpoint could not be retrieved due
to incorrect device node pointers being used.
This patch fixes the issue by updating the `rsnd_dai_of_node()` and
`rsnd_dai_probe()` functions to use the correct device node pointers based
on the port names ('port' or 'ports'). It ensures that the endpoint is
properly parsed for both single and multi-port configurations, restoring
compatibility with HiHope boards.
Fixes: 547b02f74e4a ("ASoC: rsnd: enable multi Component support for Audio Graph Card/Card2")
Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
Hi Geert,
I was intending to rename the sh folder to renesas, do you think that should be OK?
Cheers, Prabhakar
---
sound/soc/sh/rcar/core.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/sound/soc/sh/rcar/core.c b/sound/soc/sh/rcar/core.c
index 9784718a2b6f..eca5ce096e54 100644
--- a/sound/soc/sh/rcar/core.c
+++ b/sound/soc/sh/rcar/core.c
@@ -1281,7 +1281,9 @@ static int rsnd_dai_of_node(struct rsnd_priv *priv, int *is_graph)
if (!of_node_name_eq(ports, "ports") &&
!of_node_name_eq(ports, "port"))
continue;
- priv->component_dais[i] = of_graph_get_endpoint_count(ports);
+ priv->component_dais[i] =
+ of_graph_get_endpoint_count(of_node_name_eq(ports, "ports") ?
+ ports : np);
nr += priv->component_dais[i];
i++;
if (i >= RSND_MAX_COMPONENT) {
@@ -1493,7 +1495,8 @@ static int rsnd_dai_probe(struct rsnd_priv *priv)
if (!of_node_name_eq(ports, "ports") &&
!of_node_name_eq(ports, "port"))
continue;
- for_each_endpoint_of_node(ports, dai_np) {
+ for_each_endpoint_of_node(of_node_name_eq(ports, "ports") ?
+ ports : np, dai_np) {
__rsnd_dai_probe(priv, dai_np, dai_np, 0, dai_i);
if (!rsnd_is_gen1(priv) && !rsnd_is_gen2(priv)) {
rdai = rsnd_rdai_get(priv, dai_i);
--
2.43.0On Thu, 10 Oct 2024 15:14:32 +0100, Prabhakar wrote:
> On the HiHope boards, we have a single port with a single endpoint defined
> as below:
> ....
> rsnd_port: port {
> rsnd_endpoint: endpoint {
> remote-endpoint = <&dw_hdmi0_snd_in>;
>
> [...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/1] ASoC: rsnd: Fix probe failure on HiHope boards due to endpoint parsing
commit: 9b064d200aa8fee9d1d7ced05d8a617e45966715
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 Prabhakar,
On Thu, Oct 10, 2024 at 4:15 PM Prabhakar <prabhakar.csengg@gmail.com> wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> On the HiHope boards, we have a single port with a single endpoint defined
> as below:
> ....
> rsnd_port: port {
> rsnd_endpoint: endpoint {
> remote-endpoint = <&dw_hdmi0_snd_in>;
>
> dai-format = "i2s";
> bitclock-master = <&rsnd_endpoint>;
> frame-master = <&rsnd_endpoint>;
>
> playback = <&ssi2>;
> };
> };
> ....
>
> With commit 547b02f74e4a ("ASoC: rsnd: enable multi Component support for
> Audio Graph Card/Card2"), support for multiple ports was added. This caused
> probe failures on HiHope boards, as the endpoint could not be retrieved due
> to incorrect device node pointers being used.
>
> This patch fixes the issue by updating the `rsnd_dai_of_node()` and
> `rsnd_dai_probe()` functions to use the correct device node pointers based
> on the port names ('port' or 'ports'). It ensures that the endpoint is
> properly parsed for both single and multi-port configurations, restoring
> compatibility with HiHope boards.
>
> Fixes: 547b02f74e4a ("ASoC: rsnd: enable multi Component support for Audio Graph Card/Card2")
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Thanks for your patch!
> --- a/sound/soc/sh/rcar/core.c
> +++ b/sound/soc/sh/rcar/core.c
> @@ -1281,7 +1281,9 @@ static int rsnd_dai_of_node(struct rsnd_priv *priv, int *is_graph)
> if (!of_node_name_eq(ports, "ports") &&
> !of_node_name_eq(ports, "port"))
> continue;
> - priv->component_dais[i] = of_graph_get_endpoint_count(ports);
> + priv->component_dais[i] =
> + of_graph_get_endpoint_count(of_node_name_eq(ports, "ports") ?
> + ports : np);
As of_node_name_eq() is not inline or __pure, of_node_name_eq(ports,
"ports") will be called twice. So it may make sense to add a helper,
combining the selection with the validation above:
const struct device_node *pick_endpoint_node_for_ports(const
struct device_node *np,
const struct device_node *e_ports, const struct
device_node *e_port)
{
if (of_node_name_eq(ports, "ports"))
return e_ports;
if (of_node_name_eq(ports, "port"))
return e_port;
return NULL;
}
> nr += priv->component_dais[i];
> i++;
> if (i >= RSND_MAX_COMPONENT) {
> @@ -1493,7 +1495,8 @@ static int rsnd_dai_probe(struct rsnd_priv *priv)
> if (!of_node_name_eq(ports, "ports") &&
> !of_node_name_eq(ports, "port"))
> continue;
> - for_each_endpoint_of_node(ports, dai_np) {
> + for_each_endpoint_of_node(of_node_name_eq(ports, "ports") ?
> + ports : np, dai_np) {
Likewise.
> __rsnd_dai_probe(priv, dai_np, dai_np, 0, dai_i);
> if (!rsnd_is_gen1(priv) && !rsnd_is_gen2(priv)) {
> rdai = rsnd_rdai_get(priv, dai_i);
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Hi Geert,
Thank you for the review.
On Fri, Oct 11, 2024 at 7:42 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Prabhakar,
>
> On Thu, Oct 10, 2024 at 4:15 PM Prabhakar <prabhakar.csengg@gmail.com> wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > On the HiHope boards, we have a single port with a single endpoint defined
> > as below:
> > ....
> > rsnd_port: port {
> > rsnd_endpoint: endpoint {
> > remote-endpoint = <&dw_hdmi0_snd_in>;
> >
> > dai-format = "i2s";
> > bitclock-master = <&rsnd_endpoint>;
> > frame-master = <&rsnd_endpoint>;
> >
> > playback = <&ssi2>;
> > };
> > };
> > ....
> >
> > With commit 547b02f74e4a ("ASoC: rsnd: enable multi Component support for
> > Audio Graph Card/Card2"), support for multiple ports was added. This caused
> > probe failures on HiHope boards, as the endpoint could not be retrieved due
> > to incorrect device node pointers being used.
> >
> > This patch fixes the issue by updating the `rsnd_dai_of_node()` and
> > `rsnd_dai_probe()` functions to use the correct device node pointers based
> > on the port names ('port' or 'ports'). It ensures that the endpoint is
> > properly parsed for both single and multi-port configurations, restoring
> > compatibility with HiHope boards.
> >
> > Fixes: 547b02f74e4a ("ASoC: rsnd: enable multi Component support for Audio Graph Card/Card2")
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Thanks for your patch!
>
> > --- a/sound/soc/sh/rcar/core.c
> > +++ b/sound/soc/sh/rcar/core.c
> > @@ -1281,7 +1281,9 @@ static int rsnd_dai_of_node(struct rsnd_priv *priv, int *is_graph)
> > if (!of_node_name_eq(ports, "ports") &&
> > !of_node_name_eq(ports, "port"))
> > continue;
> > - priv->component_dais[i] = of_graph_get_endpoint_count(ports);
> > + priv->component_dais[i] =
> > + of_graph_get_endpoint_count(of_node_name_eq(ports, "ports") ?
> > + ports : np);
>
> As of_node_name_eq() is not inline or __pure, of_node_name_eq(ports,
> "ports") will be called twice. So it may make sense to add a helper,
> combining the selection with the validation above:
>
> const struct device_node *pick_endpoint_node_for_ports(const
> struct device_node *np,
> const struct device_node *e_ports, const struct
> device_node *e_port)
> {
> if (of_node_name_eq(ports, "ports"))
> return e_ports;
> if (of_node_name_eq(ports, "port"))
> return e_port;
> return NULL;
> }
>
I'll drop the const maybe to reduce the diff and have something like below:
+static struct device_node*
+ pick_endpoint_node_for_ports(struct device_node *e_ports,
+ struct device_node *e_port)
+{
+ if (of_node_name_eq(e_ports, "ports"))
+ return e_ports;
+
+ if (of_node_name_eq(e_ports, "port"))
+ return e_port;
+
+ return NULL;
+}
and use it like below in the rsnd_dai_of_node() function:
@@ -1278,10 +1291,10 @@ static int rsnd_dai_of_node(struct rsnd_priv
*priv, int *is_graph)
* Audio-Graph-Card
*/
for_each_child_of_node(np, ports) {
- if (!of_node_name_eq(ports, "ports") &&
- !of_node_name_eq(ports, "port"))
+ node = pick_endpoint_node_for_ports(ports, np);
+ if (!node)
continue;
- priv->component_dais[i] = of_graph_get_endpoint_count(ports);
+ priv->component_dais[i] = of_graph_get_endpoint_count(node);
> > nr += priv->component_dais[i];
> > i++;
> > if (i >= RSND_MAX_COMPONENT) {
> > @@ -1493,7 +1495,8 @@ static int rsnd_dai_probe(struct rsnd_priv *priv)
> > if (!of_node_name_eq(ports, "ports") &&
> > !of_node_name_eq(ports, "port"))
> > continue;
> > - for_each_endpoint_of_node(ports, dai_np) {
> > + for_each_endpoint_of_node(of_node_name_eq(ports, "ports") ?
> > + ports : np, dai_np) {
>
> Likewise.
>
Ok, now I will have to create a local variable for this loop, which
will look like something below:
@@ -1486,14 +1499,15 @@ static int rsnd_dai_probe(struct rsnd_priv *priv)
*/
dai_i = 0;
if (is_graph) {
+ struct device_node *dai_np_port;
struct device_node *ports;
struct device_node *dai_np;
for_each_child_of_node(np, ports) {
- if (!of_node_name_eq(ports, "ports") &&
- !of_node_name_eq(ports, "port"))
+ dai_np_port = pick_endpoint_node_for_ports(ports, np);
+ if (!dai_np_port)
continue;
- for_each_endpoint_of_node(ports, dai_np) {
+ for_each_endpoint_of_node(dai_np_port, dai_np) {
__rsnd_dai_probe(priv, dai_np, dai_np,
0, dai_i);
if (!rsnd_is_gen1(priv) &&
!rsnd_is_gen2(priv)) {
rdai = rsnd_rdai_get(priv, dai_i);
Does that sound OK?
Cheers,
Prabhakar
Hi Prabhakar,
On Thu, Oct 10, 2024 at 4:15 PM Prabhakar <prabhakar.csengg@gmail.com> wrote:
> I was intending to rename the sh folder to renesas, do you think that should be OK?
That's up to Morimoto-san and the other sound maintainers.
> sound/soc/sh/rcar/core.c | 7 +++++--
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Hi Geert and Kuninori, On Fri, Oct 11, 2024 at 7:27 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > Hi Prabhakar, > > On Thu, Oct 10, 2024 at 4:15 PM Prabhakar <prabhakar.csengg@gmail.com> wrote: > > I was intending to rename the sh folder to renesas, do you think that should be OK? > > That's up to Morimoto-san and the other sound maintainers. > Thanks Geert. Morimoto-san, what do you think of renaming the sh folder to renesas? Cheers, Prabhakar
Hi Lad > > > I was intending to rename the sh folder to renesas, do you think that should be OK? (snip) > Morimoto-san, what do you think of renaming the sh folder to renesas? Yeah, it sounds good idea ! Best regards --- Kuninori Morimoto
Hi
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> On the HiHope boards, we have a single port with a single endpoint defined
> as below:
> ....
> rsnd_port: port {
> rsnd_endpoint: endpoint {
> remote-endpoint = <&dw_hdmi0_snd_in>;
>
> dai-format = "i2s";
> bitclock-master = <&rsnd_endpoint>;
> frame-master = <&rsnd_endpoint>;
>
> playback = <&ssi2>;
> };
> };
> ....
>
> With commit 547b02f74e4a ("ASoC: rsnd: enable multi Component support for
> Audio Graph Card/Card2"), support for multiple ports was added. This caused
> probe failures on HiHope boards, as the endpoint could not be retrieved due
> to incorrect device node pointers being used.
>
> This patch fixes the issue by updating the `rsnd_dai_of_node()` and
> `rsnd_dai_probe()` functions to use the correct device node pointers based
> on the port names ('port' or 'ports'). It ensures that the endpoint is
> properly parsed for both single and multi-port configurations, restoring
> compatibility with HiHope boards.
>
> Fixes: 547b02f74e4a ("ASoC: rsnd: enable multi Component support for Audio Graph Card/Card2")
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
> Hi Geert,
> I was intending to rename the sh folder to renesas, do you think that should be OK?
> Cheers, Prabhakar
> ---
> sound/soc/sh/rcar/core.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/sound/soc/sh/rcar/core.c b/sound/soc/sh/rcar/core.c
> index 9784718a2b6f..eca5ce096e54 100644
> --- a/sound/soc/sh/rcar/core.c
> +++ b/sound/soc/sh/rcar/core.c
> @@ -1281,7 +1281,9 @@ static int rsnd_dai_of_node(struct rsnd_priv *priv, int *is_graph)
> if (!of_node_name_eq(ports, "ports") &&
> !of_node_name_eq(ports, "port"))
> continue;
> - priv->component_dais[i] = of_graph_get_endpoint_count(ports);
> + priv->component_dais[i] =
> + of_graph_get_endpoint_count(of_node_name_eq(ports, "ports") ?
> + ports : np);
> nr += priv->component_dais[i];
> i++;
> if (i >= RSND_MAX_COMPONENT) {
> @@ -1493,7 +1495,8 @@ static int rsnd_dai_probe(struct rsnd_priv *priv)
> if (!of_node_name_eq(ports, "ports") &&
> !of_node_name_eq(ports, "port"))
> continue;
> - for_each_endpoint_of_node(ports, dai_np) {
> + for_each_endpoint_of_node(of_node_name_eq(ports, "ports") ?
> + ports : np, dai_np) {
> __rsnd_dai_probe(priv, dai_np, dai_np, 0, dai_i);
> if (!rsnd_is_gen1(priv) && !rsnd_is_gen2(priv)) {
> rdai = rsnd_rdai_get(priv, dai_i);
Ah, I had a plan to clean-up around here, but Of-Graph rejected the idea.
Indeed it will be issue if DT doesn't have "ports".
I would like to re-cleanup around here in the future.
But anyway, for this patch
Acked-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Thank you for your help !!
Best regards
---
Kuninori Morimoto
© 2016 - 2024 Red Hat, Inc.