[PATCH] of: property: fw_devlink: Fix stupid bug in remote-endpoint parsing

[PATCH] of: property: fw_devlink: Fix stupid bug in remote-endpoint parsing

[Saravana Kannan]

Introduced a stupid bug in commit 782bfd03c3ae ("of: property: Improve
finding the supplier of a remote-endpoint property") due to a last minute
incorrect edit of "index !=0" into "!index". This patch fixes it to be
"index > 0" to match the comment right next to it.

Reported-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
Link: https://lore.kernel.org/lkml/20240223171849.10f9901d@booty/
Fixes: 782bfd03c3ae ("of: property: Improve finding the supplier of a remote-endpoint property")
Signed-off-by: Saravana Kannan <saravanak@google.com>
---
Using Link: instead of Closes: because Luca reported two separate issues.

Sorry about introducing a stupid bug in an -rcX Rob.

-Saravana

 drivers/of/property.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/of/property.c b/drivers/of/property.c
index b71267c6667c..fa8cd33be131 100644
--- a/drivers/of/property.c
+++ b/drivers/of/property.c
@@ -1304,7 +1304,7 @@ static struct device_node *parse_remote_endpoint(struct device_node *np,
 						 int index)
 {
 	/* Return NULL for index > 0 to signify end of remote-endpoints. */
-	if (!index || strcmp(prop_name, "remote-endpoint"))
+	if (index > 0 || strcmp(prop_name, "remote-endpoint"))
 		return NULL;
 
 	return of_graph_get_remote_port_parent(np);
-- 
2.44.0.rc0.258.g7320e95886-goog

Re: [PATCH] of: property: fw_devlink: Fix stupid bug in remote-endpoint parsing

[Saravana Kannan]

On Fri, Feb 23, 2024 at 9:24 PM Saravana Kannan <saravanak@google.com> wrote:
>
> Introduced a stupid bug in commit 782bfd03c3ae ("of: property: Improve
> finding the supplier of a remote-endpoint property") due to a last minute
> incorrect edit of "index !=0" into "!index". This patch fixes it to be
> "index > 0" to match the comment right next to it.

Greg, this needs to land in the stable branches once Rob picks it up
for the next 6.8-rc.

-Saravana

>
> Reported-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> Link: https://lore.kernel.org/lkml/20240223171849.10f9901d@booty/
> Fixes: 782bfd03c3ae ("of: property: Improve finding the supplier of a remote-endpoint property")
> Signed-off-by: Saravana Kannan <saravanak@google.com>
> ---
> Using Link: instead of Closes: because Luca reported two separate issues.
>
> Sorry about introducing a stupid bug in an -rcX Rob.
>
> -Saravana
>
>  drivers/of/property.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/of/property.c b/drivers/of/property.c
> index b71267c6667c..fa8cd33be131 100644
> --- a/drivers/of/property.c
> +++ b/drivers/of/property.c
> @@ -1304,7 +1304,7 @@ static struct device_node *parse_remote_endpoint(struct device_node *np,
>                                                  int index)
>  {
>         /* Return NULL for index > 0 to signify end of remote-endpoints. */
> -       if (!index || strcmp(prop_name, "remote-endpoint"))
> +       if (index > 0 || strcmp(prop_name, "remote-endpoint"))
>                 return NULL;
>
>         return of_graph_get_remote_port_parent(np);
> --
> 2.44.0.rc0.258.g7320e95886-goog
>

Re: [PATCH] of: property: fw_devlink: Fix stupid bug in remote-endpoint parsing

[Luca Ceresoli]

On Fri, 23 Feb 2024 21:24:35 -0800
Saravana Kannan <saravanak@google.com> wrote:

> Introduced a stupid bug in commit 782bfd03c3ae ("of: property: Improve
> finding the supplier of a remote-endpoint property") due to a last minute
> incorrect edit of "index !=0" into "!index". This patch fixes it to be
> "index > 0" to match the comment right next to it.
> 
> Reported-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> Link: https://lore.kernel.org/lkml/20240223171849.10f9901d@booty/
> Fixes: 782bfd03c3ae ("of: property: Improve finding the supplier of a remote-endpoint property")
> Signed-off-by: Saravana Kannan <saravanak@google.com>

Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
Tested-by: Luca Ceresoli <luca.ceresoli@bootlin.com>

-- 
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH] of: property: fw_devlink: Fix stupid bug in remote-endpoint parsing

[Herve Codina]

Hi Saravana,

On Fri, 23 Feb 2024 21:24:35 -0800
Saravana Kannan <saravanak@google.com> wrote:

> Introduced a stupid bug in commit 782bfd03c3ae ("of: property: Improve
> finding the supplier of a remote-endpoint property") due to a last minute
> incorrect edit of "index !=0" into "!index". This patch fixes it to be
> "index > 0" to match the comment right next to it.
> 
> Reported-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> Link: https://lore.kernel.org/lkml/20240223171849.10f9901d@booty/
> Fixes: 782bfd03c3ae ("of: property: Improve finding the supplier of a remote-endpoint property")
> Signed-off-by: Saravana Kannan <saravanak@google.com>
> ---
> Using Link: instead of Closes: because Luca reported two separate issues.
> 
> Sorry about introducing a stupid bug in an -rcX Rob.
> 

Reviewed-by: Herve Codina <herve.codina@bootlin.com>

Best regards,
Hervé

Re: [PATCH] of: property: fw_devlink: Fix stupid bug in remote-endpoint parsing

[Luca Ceresoli]

Hello Greg, Saravana,

On Fri, 23 Feb 2024 21:28:18 -0800
Saravana Kannan <saravanak@google.com> wrote:

> On Fri, Feb 23, 2024 at 9:24 PM Saravana Kannan <saravanak@google.com> wrote:
> >
> > Introduced a stupid bug in commit 782bfd03c3ae ("of: property: Improve
> > finding the supplier of a remote-endpoint property") due to a last minute
> > incorrect edit of "index !=0" into "!index". This patch fixes it to be
> > "index > 0" to match the comment right next to it.  
> 
> Greg, this needs to land in the stable branches once Rob picks it up
> for the next 6.8-rc.
> 
> -Saravana
> 
> >
> > Reported-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> > Link: https://lore.kernel.org/lkml/20240223171849.10f9901d@booty/
> > Fixes: 782bfd03c3ae ("of: property: Improve finding the supplier of a remote-endpoint property")
> > Signed-off-by: Saravana Kannan <saravanak@google.com>
> > ---
> > Using Link: instead of Closes: because Luca reported two separate issues.

As Saravana mentioned, this fixes only one bug in the original commit.

Unless there is a quick solution for the other bug, we are still left with
a regression since that got merged in 6.8-rc5.

So I propose to revert 782bfd03c3ae instead of applying this one, to
leave the needed time for a correct solution to be deviesd.

Luca

-- 
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH] of: property: fw_devlink: Fix stupid bug in remote-endpoint parsing

[Rob Herring]

On Fri, Feb 23, 2024 at 11:29 PM Saravana Kannan <saravanak@google.com> wrote:
>
> On Fri, Feb 23, 2024 at 9:24 PM Saravana Kannan <saravanak@google.com> wrote:
> >
> > Introduced a stupid bug in commit 782bfd03c3ae ("of: property: Improve
> > finding the supplier of a remote-endpoint property") due to a last minute
> > incorrect edit of "index !=0" into "!index". This patch fixes it to be
> > "index > 0" to match the comment right next to it.
>
> Greg, this needs to land in the stable branches once Rob picks it up
> for the next 6.8-rc.

Uh, what? Only if I ignore this patch until 6.8 is released.
Otherwise, the bug and fix are both landing in 6.8.

Rob

Re: [PATCH] of: property: fw_devlink: Fix stupid bug in remote-endpoint parsing

[Geert Uytterhoeven]

Hi Saravana,

On Sat, Feb 24, 2024 at 6:25 AM Saravana Kannan <saravanak@google.com> wrote:
> Introduced a stupid bug in commit 782bfd03c3ae ("of: property: Improve
> finding the supplier of a remote-endpoint property") due to a last minute
> incorrect edit of "index !=0" into "!index". This patch fixes it to be
> "index > 0" to match the comment right next to it.
>
> Reported-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> Link: https://lore.kernel.org/lkml/20240223171849.10f9901d@booty/
> Fixes: 782bfd03c3ae ("of: property: Improve finding the supplier of a remote-endpoint property")
> Signed-off-by: Saravana Kannan <saravanak@google.com>

Thanks for your patch!

> --- a/drivers/of/property.c
> +++ b/drivers/of/property.c
> @@ -1304,7 +1304,7 @@ static struct device_node *parse_remote_endpoint(struct device_node *np,
>                                                  int index)
>  {
>         /* Return NULL for index > 0 to signify end of remote-endpoints. */
> -       if (!index || strcmp(prop_name, "remote-endpoint"))
> +       if (index > 0 || strcmp(prop_name, "remote-endpoint"))
>                 return NULL;
>
>         return of_graph_get_remote_port_parent(np);
> --
> 2.44.0.rc0.258.g7320e95886-goog

After this, the "Fixed dependency cycle" messages I reported to be
gone in [1] are back.

In fact, they are slightly different, and there are now even more of them:

-platform fea80000.csi2: Fixed dependency cycle(s) with
/soc/video@e6ef7000/ports/port@1/endpoint@0
-platform fea80000.csi2: Fixed dependency cycle(s) with
/soc/video@e6ef6000/ports/port@1/endpoint@0
-platform fea80000.csi2: Fixed dependency cycle(s) with
/soc/video@e6ef5000/ports/port@1/endpoint@0
-platform fea80000.csi2: Fixed dependency cycle(s) with
/soc/video@e6ef4000/ports/port@1/endpoint@0
-platform fea80000.csi2: Fixed dependency cycle(s) with
/soc/video@e6ef3000/ports/port@1/endpoint@0
-platform fea80000.csi2: Fixed dependency cycle(s) with
/soc/video@e6ef2000/ports/port@1/endpoint@0
-platform fea80000.csi2: Fixed dependency cycle(s) with
/soc/video@e6ef1000/ports/port@1/endpoint@0
-platform fea80000.csi2: Fixed dependency cycle(s) with
/soc/video@e6ef0000/ports/port@1/endpoint@0
-platform feaa0000.csi2: Fixed dependency cycle(s) with
/soc/video@e6ef3000/ports/port@1/endpoint@2
-platform feaa0000.csi2: Fixed dependency cycle(s) with
/soc/video@e6ef2000/ports/port@1/endpoint@2
-platform feaa0000.csi2: Fixed dependency cycle(s) with
/soc/video@e6ef1000/ports/port@1/endpoint@2
-platform feaa0000.csi2: Fixed dependency cycle(s) with
/soc/video@e6ef0000/ports/port@1/endpoint@2
-platform fead0000.hdmi: Fixed dependency cycle(s) with
/soc/sound@ec500000/ports/port@1/endpoint
-platform feae0000.hdmi: Fixed dependency cycle(s) with
/soc/sound@ec500000/ports/port@2/endpoint
-platform feb00000.display: Fixed dependency cycle(s) with
/soc/hdmi@feae0000/ports/port@0/endpoint
-platform feb00000.display: Fixed dependency cycle(s) with
/soc/hdmi@fead0000/ports/port@0/endpoint
-platform hdmi0-out: Fixed dependency cycle(s) with
/soc/hdmi@fead0000/ports/port@1/endpoint
-platform hdmi1-out: Fixed dependency cycle(s) with
/soc/hdmi@feae0000/ports/port@1/endpoint
-platform vga-encoder: Fixed dependency cycle(s) with /vga/port/endpoint
-platform vga-encoder: Fixed dependency cycle(s) with
/soc/display@feb00000/ports/port@0/endpoint
+platform e6ef0000.video: Fixed dependency cycle(s) with /soc/csi2@feaa0000
+platform e6ef0000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
+platform e6ef1000.video: Fixed dependency cycle(s) with /soc/csi2@feaa0000
+platform e6ef1000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
+platform e6ef2000.video: Fixed dependency cycle(s) with /soc/csi2@feaa0000
+platform e6ef2000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
+platform e6ef3000.video: Fixed dependency cycle(s) with /soc/csi2@feaa0000
+platform e6ef3000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
+platform e6ef4000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
+platform e6ef5000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
+platform e6ef6000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
+platform e6ef7000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
+platform ec500000.sound: Fixed dependency cycle(s) with /soc/hdmi@feae0000
+platform ec500000.sound: Fixed dependency cycle(s) with /soc/hdmi@fead0000
+platform ec500000.sound: Fixed dependency cycle(s) with
/soc/i2c@e6510000/codec@10
+platform e6ef7000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
+platform e6ef6000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
+platform e6ef5000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
+platform e6ef4000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
+platform e6ef3000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
+platform e6ef2000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
+platform e6ef1000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
+platform e6ef0000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
+platform fea80000.csi2: Fixed dependency cycle(s) with /soc/video@e6ef7000
+platform fea80000.csi2: Fixed dependency cycle(s) with /soc/video@e6ef6000
+platform fea80000.csi2: Fixed dependency cycle(s) with /soc/video@e6ef5000
+platform fea80000.csi2: Fixed dependency cycle(s) with /soc/video@e6ef4000
+platform fea80000.csi2: Fixed dependency cycle(s) with /soc/video@e6ef3000
+platform fea80000.csi2: Fixed dependency cycle(s) with /soc/video@e6ef2000
+platform fea80000.csi2: Fixed dependency cycle(s) with /soc/video@e6ef1000
+platform fea80000.csi2: Fixed dependency cycle(s) with /soc/video@e6ef0000
+platform fea80000.csi2: Fixed dependency cycle(s) with
/soc/i2c@e66d8000/video-receiver@70
+platform e6ef3000.video: Fixed dependency cycle(s) with /soc/csi2@feaa0000
+platform e6ef2000.video: Fixed dependency cycle(s) with /soc/csi2@feaa0000
+platform e6ef1000.video: Fixed dependency cycle(s) with /soc/csi2@feaa0000
+platform e6ef0000.video: Fixed dependency cycle(s) with /soc/csi2@feaa0000
+platform feaa0000.csi2: Fixed dependency cycle(s) with /soc/video@e6ef3000
+platform feaa0000.csi2: Fixed dependency cycle(s) with /soc/video@e6ef2000
+platform feaa0000.csi2: Fixed dependency cycle(s) with /soc/video@e6ef1000
+platform feaa0000.csi2: Fixed dependency cycle(s) with /soc/video@e6ef0000
+platform feaa0000.csi2: Fixed dependency cycle(s) with
/soc/i2c@e66d8000/video-receiver@70
+platform ec500000.sound: Fixed dependency cycle(s) with /soc/hdmi@fead0000
+platform fead0000.hdmi: Fixed dependency cycle(s) with /soc/sound@ec500000
+platform fead0000.hdmi: Fixed dependency cycle(s) with /soc/display@feb00000
+platform ec500000.sound: Fixed dependency cycle(s) with /soc/hdmi@feae0000
+platform feae0000.hdmi: Fixed dependency cycle(s) with /soc/sound@ec500000
+platform feae0000.hdmi: Fixed dependency cycle(s) with /soc/display@feb00000
+platform feae0000.hdmi: Fixed dependency cycle(s) with /soc/display@feb00000
+platform fead0000.hdmi: Fixed dependency cycle(s) with /soc/display@feb00000
+platform feb00000.display: Fixed dependency cycle(s) with /soc/hdmi@feae0000
+platform feb00000.display: Fixed dependency cycle(s) with /soc/hdmi@fead0000
+platform cvbs-in: Fixed dependency cycle(s) with
/soc/i2c@e66d8000/video-receiver@70
+platform hdmi-in: Fixed dependency cycle(s) with
/soc/i2c@e66d8000/video-receiver@70
+platform fead0000.hdmi: Fixed dependency cycle(s) with /hdmi0-out
+platform hdmi0-out: Fixed dependency cycle(s) with /soc/hdmi@fead0000
+platform feae0000.hdmi: Fixed dependency cycle(s) with /hdmi1-out
+platform hdmi1-out: Fixed dependency cycle(s) with /soc/hdmi@feae0000
+platform vga: Fixed dependency cycle(s) with /vga-encoder
+platform feb00000.display: Fixed dependency cycle(s) with /vga-encoder
+platform vga-encoder: Fixed dependency cycle(s) with /vga
+platform vga-encoder: Fixed dependency cycle(s) with /soc/display@feb00000

-i2c 2-0010: Fixed dependency cycle(s) with
/soc/sound@ec500000/ports/port@0/endpoint
+platform ec500000.sound: Fixed dependency cycle(s) with
/soc/i2c@e6510000/codec@10

-i2c 4-0070: Fixed dependency cycle(s) with
/soc/csi2@fea80000/ports/port@0/endpoint
-i2c 4-0070: Fixed dependency cycle(s) with
/soc/csi2@feaa0000/ports/port@0/endpoint
-i2c 4-0070: Fixed dependency cycle(s) with /hdmi-in/port/endpoint
-i2c 4-0070: Fixed dependency cycle(s) with /cvbs-in/port/endpoint
+platform feaa0000.csi2: Fixed dependency cycle(s) with
/soc/i2c@e66d8000/video-receiver@70
+platform fea80000.csi2: Fixed dependency cycle(s) with
/soc/i2c@e66d8000/video-receiver@70
+i2c 4-0070: Fixed dependency cycle(s) with /soc/csi2@fea80000
+i2c 4-0070: Fixed dependency cycle(s) with /soc/csi2@feaa0000

I guess all of that is expected?

[1] https://lore.kernel.org/all/CAMuHMdVon3mdivZQ0O6D4+va0nGBrUQbDp23bEq661QD=4t7+g@mail.gmail.com/

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

Re: [PATCH] of: property: fw_devlink: Fix stupid bug in remote-endpoint parsing

[Saravana Kannan]

On Thu, Feb 29, 2024 at 2:11 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Saravana,
>
> On Sat, Feb 24, 2024 at 6:25 AM Saravana Kannan <saravanak@google.com> wrote:
> > Introduced a stupid bug in commit 782bfd03c3ae ("of: property: Improve
> > finding the supplier of a remote-endpoint property") due to a last minute
> > incorrect edit of "index !=0" into "!index". This patch fixes it to be
> > "index > 0" to match the comment right next to it.
> >
> > Reported-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> > Link: https://lore.kernel.org/lkml/20240223171849.10f9901d@booty/
> > Fixes: 782bfd03c3ae ("of: property: Improve finding the supplier of a remote-endpoint property")
> > Signed-off-by: Saravana Kannan <saravanak@google.com>
>
> Thanks for your patch!
>
> > --- a/drivers/of/property.c
> > +++ b/drivers/of/property.c
> > @@ -1304,7 +1304,7 @@ static struct device_node *parse_remote_endpoint(struct device_node *np,
> >                                                  int index)
> >  {
> >         /* Return NULL for index > 0 to signify end of remote-endpoints. */
> > -       if (!index || strcmp(prop_name, "remote-endpoint"))
> > +       if (index > 0 || strcmp(prop_name, "remote-endpoint"))
> >                 return NULL;
> >
> >         return of_graph_get_remote_port_parent(np);
> > --
> > 2.44.0.rc0.258.g7320e95886-goog
>
> After this, the "Fixed dependency cycle" messages I reported to be
> gone in [1] are back.
>
> In fact, they are slightly different, and there are now even more of them:
>
> -platform fea80000.csi2: Fixed dependency cycle(s) with
> /soc/video@e6ef7000/ports/port@1/endpoint@0
> -platform fea80000.csi2: Fixed dependency cycle(s) with
> /soc/video@e6ef6000/ports/port@1/endpoint@0
> -platform fea80000.csi2: Fixed dependency cycle(s) with
> /soc/video@e6ef5000/ports/port@1/endpoint@0
> -platform fea80000.csi2: Fixed dependency cycle(s) with
> /soc/video@e6ef4000/ports/port@1/endpoint@0
> -platform fea80000.csi2: Fixed dependency cycle(s) with
> /soc/video@e6ef3000/ports/port@1/endpoint@0
> -platform fea80000.csi2: Fixed dependency cycle(s) with
> /soc/video@e6ef2000/ports/port@1/endpoint@0
> -platform fea80000.csi2: Fixed dependency cycle(s) with
> /soc/video@e6ef1000/ports/port@1/endpoint@0
> -platform fea80000.csi2: Fixed dependency cycle(s) with
> /soc/video@e6ef0000/ports/port@1/endpoint@0
> -platform feaa0000.csi2: Fixed dependency cycle(s) with
> /soc/video@e6ef3000/ports/port@1/endpoint@2
> -platform feaa0000.csi2: Fixed dependency cycle(s) with
> /soc/video@e6ef2000/ports/port@1/endpoint@2
> -platform feaa0000.csi2: Fixed dependency cycle(s) with
> /soc/video@e6ef1000/ports/port@1/endpoint@2
> -platform feaa0000.csi2: Fixed dependency cycle(s) with
> /soc/video@e6ef0000/ports/port@1/endpoint@2
> -platform fead0000.hdmi: Fixed dependency cycle(s) with
> /soc/sound@ec500000/ports/port@1/endpoint
> -platform feae0000.hdmi: Fixed dependency cycle(s) with
> /soc/sound@ec500000/ports/port@2/endpoint
> -platform feb00000.display: Fixed dependency cycle(s) with
> /soc/hdmi@feae0000/ports/port@0/endpoint
> -platform feb00000.display: Fixed dependency cycle(s) with
> /soc/hdmi@fead0000/ports/port@0/endpoint
> -platform hdmi0-out: Fixed dependency cycle(s) with
> /soc/hdmi@fead0000/ports/port@1/endpoint
> -platform hdmi1-out: Fixed dependency cycle(s) with
> /soc/hdmi@feae0000/ports/port@1/endpoint
> -platform vga-encoder: Fixed dependency cycle(s) with /vga/port/endpoint
> -platform vga-encoder: Fixed dependency cycle(s) with
> /soc/display@feb00000/ports/port@0/endpoint
> +platform e6ef0000.video: Fixed dependency cycle(s) with /soc/csi2@feaa0000
> +platform e6ef0000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> +platform e6ef1000.video: Fixed dependency cycle(s) with /soc/csi2@feaa0000
> +platform e6ef1000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> +platform e6ef2000.video: Fixed dependency cycle(s) with /soc/csi2@feaa0000
> +platform e6ef2000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> +platform e6ef3000.video: Fixed dependency cycle(s) with /soc/csi2@feaa0000
> +platform e6ef3000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> +platform e6ef4000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> +platform e6ef5000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> +platform e6ef6000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> +platform e6ef7000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> +platform ec500000.sound: Fixed dependency cycle(s) with /soc/hdmi@feae0000
> +platform ec500000.sound: Fixed dependency cycle(s) with /soc/hdmi@fead0000
> +platform ec500000.sound: Fixed dependency cycle(s) with
> /soc/i2c@e6510000/codec@10
> +platform e6ef7000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> +platform e6ef6000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> +platform e6ef5000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> +platform e6ef4000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> +platform e6ef3000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> +platform e6ef2000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> +platform e6ef1000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> +platform e6ef0000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> +platform fea80000.csi2: Fixed dependency cycle(s) with /soc/video@e6ef7000
> +platform fea80000.csi2: Fixed dependency cycle(s) with /soc/video@e6ef6000
> +platform fea80000.csi2: Fixed dependency cycle(s) with /soc/video@e6ef5000
> +platform fea80000.csi2: Fixed dependency cycle(s) with /soc/video@e6ef4000
> +platform fea80000.csi2: Fixed dependency cycle(s) with /soc/video@e6ef3000
> +platform fea80000.csi2: Fixed dependency cycle(s) with /soc/video@e6ef2000
> +platform fea80000.csi2: Fixed dependency cycle(s) with /soc/video@e6ef1000
> +platform fea80000.csi2: Fixed dependency cycle(s) with /soc/video@e6ef0000
> +platform fea80000.csi2: Fixed dependency cycle(s) with
> /soc/i2c@e66d8000/video-receiver@70
> +platform e6ef3000.video: Fixed dependency cycle(s) with /soc/csi2@feaa0000
> +platform e6ef2000.video: Fixed dependency cycle(s) with /soc/csi2@feaa0000
> +platform e6ef1000.video: Fixed dependency cycle(s) with /soc/csi2@feaa0000
> +platform e6ef0000.video: Fixed dependency cycle(s) with /soc/csi2@feaa0000
> +platform feaa0000.csi2: Fixed dependency cycle(s) with /soc/video@e6ef3000
> +platform feaa0000.csi2: Fixed dependency cycle(s) with /soc/video@e6ef2000
> +platform feaa0000.csi2: Fixed dependency cycle(s) with /soc/video@e6ef1000
> +platform feaa0000.csi2: Fixed dependency cycle(s) with /soc/video@e6ef0000
> +platform feaa0000.csi2: Fixed dependency cycle(s) with
> /soc/i2c@e66d8000/video-receiver@70
> +platform ec500000.sound: Fixed dependency cycle(s) with /soc/hdmi@fead0000
> +platform fead0000.hdmi: Fixed dependency cycle(s) with /soc/sound@ec500000
> +platform fead0000.hdmi: Fixed dependency cycle(s) with /soc/display@feb00000
> +platform ec500000.sound: Fixed dependency cycle(s) with /soc/hdmi@feae0000
> +platform feae0000.hdmi: Fixed dependency cycle(s) with /soc/sound@ec500000
> +platform feae0000.hdmi: Fixed dependency cycle(s) with /soc/display@feb00000
> +platform feae0000.hdmi: Fixed dependency cycle(s) with /soc/display@feb00000
> +platform fead0000.hdmi: Fixed dependency cycle(s) with /soc/display@feb00000
> +platform feb00000.display: Fixed dependency cycle(s) with /soc/hdmi@feae0000
> +platform feb00000.display: Fixed dependency cycle(s) with /soc/hdmi@fead0000
> +platform cvbs-in: Fixed dependency cycle(s) with
> /soc/i2c@e66d8000/video-receiver@70
> +platform hdmi-in: Fixed dependency cycle(s) with
> /soc/i2c@e66d8000/video-receiver@70
> +platform fead0000.hdmi: Fixed dependency cycle(s) with /hdmi0-out
> +platform hdmi0-out: Fixed dependency cycle(s) with /soc/hdmi@fead0000
> +platform feae0000.hdmi: Fixed dependency cycle(s) with /hdmi1-out
> +platform hdmi1-out: Fixed dependency cycle(s) with /soc/hdmi@feae0000
> +platform vga: Fixed dependency cycle(s) with /vga-encoder
> +platform feb00000.display: Fixed dependency cycle(s) with /vga-encoder
> +platform vga-encoder: Fixed dependency cycle(s) with /vga
> +platform vga-encoder: Fixed dependency cycle(s) with /soc/display@feb00000
>
> -i2c 2-0010: Fixed dependency cycle(s) with
> /soc/sound@ec500000/ports/port@0/endpoint
> +platform ec500000.sound: Fixed dependency cycle(s) with
> /soc/i2c@e6510000/codec@10
>
> -i2c 4-0070: Fixed dependency cycle(s) with
> /soc/csi2@fea80000/ports/port@0/endpoint
> -i2c 4-0070: Fixed dependency cycle(s) with
> /soc/csi2@feaa0000/ports/port@0/endpoint
> -i2c 4-0070: Fixed dependency cycle(s) with /hdmi-in/port/endpoint
> -i2c 4-0070: Fixed dependency cycle(s) with /cvbs-in/port/endpoint
> +platform feaa0000.csi2: Fixed dependency cycle(s) with
> /soc/i2c@e66d8000/video-receiver@70
> +platform fea80000.csi2: Fixed dependency cycle(s) with
> /soc/i2c@e66d8000/video-receiver@70
> +i2c 4-0070: Fixed dependency cycle(s) with /soc/csi2@fea80000
> +i2c 4-0070: Fixed dependency cycle(s) with /soc/csi2@feaa0000
>
> I guess all of that is expected?

I'm assuming all of these cycles are between devices that use
remote-endpoint/port(s)? If so, yes, these are all expected. It's just
logging things more accurately now.

Once post-init-providers lands, you can use that property to break the
cycles and also allow a better probe/suspend/resume ordering between
these devices.

-Saravana

> [1] https://lore.kernel.org/all/CAMuHMdVon3mdivZQ0O6D4+va0nGBrUQbDp23bEq661QD=4t7+g@mail.gmail.com/
>
> 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

Re: [PATCH] of: property: fw_devlink: Fix stupid bug in remote-endpoint parsing

[Saravana Kannan]

On Thu, Feb 29, 2024 at 2:45 PM Saravana Kannan <saravanak@google.com> wrote:
>
> On Thu, Feb 29, 2024 at 2:11 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> >
> > Hi Saravana,
> >
> > On Sat, Feb 24, 2024 at 6:25 AM Saravana Kannan <saravanak@google.com> wrote:
> > > Introduced a stupid bug in commit 782bfd03c3ae ("of: property: Improve
> > > finding the supplier of a remote-endpoint property") due to a last minute
> > > incorrect edit of "index !=0" into "!index". This patch fixes it to be
> > > "index > 0" to match the comment right next to it.
> > >
> > > Reported-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> > > Link: https://lore.kernel.org/lkml/20240223171849.10f9901d@booty/
> > > Fixes: 782bfd03c3ae ("of: property: Improve finding the supplier of a remote-endpoint property")
> > > Signed-off-by: Saravana Kannan <saravanak@google.com>
> >
> > Thanks for your patch!
> >
> > > --- a/drivers/of/property.c
> > > +++ b/drivers/of/property.c
> > > @@ -1304,7 +1304,7 @@ static struct device_node *parse_remote_endpoint(struct device_node *np,
> > >                                                  int index)
> > >  {
> > >         /* Return NULL for index > 0 to signify end of remote-endpoints. */
> > > -       if (!index || strcmp(prop_name, "remote-endpoint"))
> > > +       if (index > 0 || strcmp(prop_name, "remote-endpoint"))
> > >                 return NULL;
> > >
> > >         return of_graph_get_remote_port_parent(np);
> > > --
> > > 2.44.0.rc0.258.g7320e95886-goog
> >
> > After this, the "Fixed dependency cycle" messages I reported to be
> > gone in [1] are back.
> >
> > In fact, they are slightly different, and there are now even more of them:
> >
> > -platform fea80000.csi2: Fixed dependency cycle(s) with
> > /soc/video@e6ef7000/ports/port@1/endpoint@0
> > -platform fea80000.csi2: Fixed dependency cycle(s) with
> > /soc/video@e6ef6000/ports/port@1/endpoint@0
> > -platform fea80000.csi2: Fixed dependency cycle(s) with
> > /soc/video@e6ef5000/ports/port@1/endpoint@0
> > -platform fea80000.csi2: Fixed dependency cycle(s) with
> > /soc/video@e6ef4000/ports/port@1/endpoint@0
> > -platform fea80000.csi2: Fixed dependency cycle(s) with
> > /soc/video@e6ef3000/ports/port@1/endpoint@0
> > -platform fea80000.csi2: Fixed dependency cycle(s) with
> > /soc/video@e6ef2000/ports/port@1/endpoint@0
> > -platform fea80000.csi2: Fixed dependency cycle(s) with
> > /soc/video@e6ef1000/ports/port@1/endpoint@0
> > -platform fea80000.csi2: Fixed dependency cycle(s) with
> > /soc/video@e6ef0000/ports/port@1/endpoint@0
> > -platform feaa0000.csi2: Fixed dependency cycle(s) with
> > /soc/video@e6ef3000/ports/port@1/endpoint@2
> > -platform feaa0000.csi2: Fixed dependency cycle(s) with
> > /soc/video@e6ef2000/ports/port@1/endpoint@2
> > -platform feaa0000.csi2: Fixed dependency cycle(s) with
> > /soc/video@e6ef1000/ports/port@1/endpoint@2
> > -platform feaa0000.csi2: Fixed dependency cycle(s) with
> > /soc/video@e6ef0000/ports/port@1/endpoint@2
> > -platform fead0000.hdmi: Fixed dependency cycle(s) with
> > /soc/sound@ec500000/ports/port@1/endpoint
> > -platform feae0000.hdmi: Fixed dependency cycle(s) with
> > /soc/sound@ec500000/ports/port@2/endpoint
> > -platform feb00000.display: Fixed dependency cycle(s) with
> > /soc/hdmi@feae0000/ports/port@0/endpoint
> > -platform feb00000.display: Fixed dependency cycle(s) with
> > /soc/hdmi@fead0000/ports/port@0/endpoint
> > -platform hdmi0-out: Fixed dependency cycle(s) with
> > /soc/hdmi@fead0000/ports/port@1/endpoint
> > -platform hdmi1-out: Fixed dependency cycle(s) with
> > /soc/hdmi@feae0000/ports/port@1/endpoint
> > -platform vga-encoder: Fixed dependency cycle(s) with /vga/port/endpoint
> > -platform vga-encoder: Fixed dependency cycle(s) with
> > /soc/display@feb00000/ports/port@0/endpoint
> > +platform e6ef0000.video: Fixed dependency cycle(s) with /soc/csi2@feaa0000
> > +platform e6ef0000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> > +platform e6ef1000.video: Fixed dependency cycle(s) with /soc/csi2@feaa0000
> > +platform e6ef1000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> > +platform e6ef2000.video: Fixed dependency cycle(s) with /soc/csi2@feaa0000
> > +platform e6ef2000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> > +platform e6ef3000.video: Fixed dependency cycle(s) with /soc/csi2@feaa0000
> > +platform e6ef3000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> > +platform e6ef4000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> > +platform e6ef5000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> > +platform e6ef6000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> > +platform e6ef7000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> > +platform ec500000.sound: Fixed dependency cycle(s) with /soc/hdmi@feae0000
> > +platform ec500000.sound: Fixed dependency cycle(s) with /soc/hdmi@fead0000
> > +platform ec500000.sound: Fixed dependency cycle(s) with
> > /soc/i2c@e6510000/codec@10
> > +platform e6ef7000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> > +platform e6ef6000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> > +platform e6ef5000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> > +platform e6ef4000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> > +platform e6ef3000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> > +platform e6ef2000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> > +platform e6ef1000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> > +platform e6ef0000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> > +platform fea80000.csi2: Fixed dependency cycle(s) with /soc/video@e6ef7000
> > +platform fea80000.csi2: Fixed dependency cycle(s) with /soc/video@e6ef6000
> > +platform fea80000.csi2: Fixed dependency cycle(s) with /soc/video@e6ef5000
> > +platform fea80000.csi2: Fixed dependency cycle(s) with /soc/video@e6ef4000
> > +platform fea80000.csi2: Fixed dependency cycle(s) with /soc/video@e6ef3000
> > +platform fea80000.csi2: Fixed dependency cycle(s) with /soc/video@e6ef2000
> > +platform fea80000.csi2: Fixed dependency cycle(s) with /soc/video@e6ef1000
> > +platform fea80000.csi2: Fixed dependency cycle(s) with /soc/video@e6ef0000
> > +platform fea80000.csi2: Fixed dependency cycle(s) with
> > /soc/i2c@e66d8000/video-receiver@70
> > +platform e6ef3000.video: Fixed dependency cycle(s) with /soc/csi2@feaa0000
> > +platform e6ef2000.video: Fixed dependency cycle(s) with /soc/csi2@feaa0000
> > +platform e6ef1000.video: Fixed dependency cycle(s) with /soc/csi2@feaa0000
> > +platform e6ef0000.video: Fixed dependency cycle(s) with /soc/csi2@feaa0000
> > +platform feaa0000.csi2: Fixed dependency cycle(s) with /soc/video@e6ef3000
> > +platform feaa0000.csi2: Fixed dependency cycle(s) with /soc/video@e6ef2000
> > +platform feaa0000.csi2: Fixed dependency cycle(s) with /soc/video@e6ef1000
> > +platform feaa0000.csi2: Fixed dependency cycle(s) with /soc/video@e6ef0000
> > +platform feaa0000.csi2: Fixed dependency cycle(s) with
> > /soc/i2c@e66d8000/video-receiver@70
> > +platform ec500000.sound: Fixed dependency cycle(s) with /soc/hdmi@fead0000
> > +platform fead0000.hdmi: Fixed dependency cycle(s) with /soc/sound@ec500000
> > +platform fead0000.hdmi: Fixed dependency cycle(s) with /soc/display@feb00000
> > +platform ec500000.sound: Fixed dependency cycle(s) with /soc/hdmi@feae0000
> > +platform feae0000.hdmi: Fixed dependency cycle(s) with /soc/sound@ec500000
> > +platform feae0000.hdmi: Fixed dependency cycle(s) with /soc/display@feb00000
> > +platform feae0000.hdmi: Fixed dependency cycle(s) with /soc/display@feb00000
> > +platform fead0000.hdmi: Fixed dependency cycle(s) with /soc/display@feb00000
> > +platform feb00000.display: Fixed dependency cycle(s) with /soc/hdmi@feae0000
> > +platform feb00000.display: Fixed dependency cycle(s) with /soc/hdmi@fead0000
> > +platform cvbs-in: Fixed dependency cycle(s) with
> > /soc/i2c@e66d8000/video-receiver@70
> > +platform hdmi-in: Fixed dependency cycle(s) with
> > /soc/i2c@e66d8000/video-receiver@70
> > +platform fead0000.hdmi: Fixed dependency cycle(s) with /hdmi0-out
> > +platform hdmi0-out: Fixed dependency cycle(s) with /soc/hdmi@fead0000
> > +platform feae0000.hdmi: Fixed dependency cycle(s) with /hdmi1-out
> > +platform hdmi1-out: Fixed dependency cycle(s) with /soc/hdmi@feae0000
> > +platform vga: Fixed dependency cycle(s) with /vga-encoder
> > +platform feb00000.display: Fixed dependency cycle(s) with /vga-encoder
> > +platform vga-encoder: Fixed dependency cycle(s) with /vga
> > +platform vga-encoder: Fixed dependency cycle(s) with /soc/display@feb00000
> >
> > -i2c 2-0010: Fixed dependency cycle(s) with
> > /soc/sound@ec500000/ports/port@0/endpoint
> > +platform ec500000.sound: Fixed dependency cycle(s) with
> > /soc/i2c@e6510000/codec@10
> >
> > -i2c 4-0070: Fixed dependency cycle(s) with
> > /soc/csi2@fea80000/ports/port@0/endpoint
> > -i2c 4-0070: Fixed dependency cycle(s) with
> > /soc/csi2@feaa0000/ports/port@0/endpoint
> > -i2c 4-0070: Fixed dependency cycle(s) with /hdmi-in/port/endpoint
> > -i2c 4-0070: Fixed dependency cycle(s) with /cvbs-in/port/endpoint
> > +platform feaa0000.csi2: Fixed dependency cycle(s) with
> > /soc/i2c@e66d8000/video-receiver@70
> > +platform fea80000.csi2: Fixed dependency cycle(s) with
> > /soc/i2c@e66d8000/video-receiver@70
> > +i2c 4-0070: Fixed dependency cycle(s) with /soc/csi2@fea80000
> > +i2c 4-0070: Fixed dependency cycle(s) with /soc/csi2@feaa0000
> >
> > I guess all of that is expected?
>
> I'm assuming all of these cycles are between devices that use
> remote-endpoint/port(s)? If so, yes, these are all expected. It's just
> logging things more accurately now.
>
> Once post-init-providers lands, you can use that property to break the
> cycles and also allow a better probe/suspend/resume ordering between
> these devices.

To be clear, this is just extra and more accurate logging in your case
and it's saying fw_devlink isn't enforcing ordering between these
devices. So, unless there's a bug in the drivers themselves, this
isn't breaking anything.

-Saravana

Re: [PATCH] of: property: fw_devlink: Fix stupid bug in remote-endpoint parsing

[Rob Herring]

On Fri, 23 Feb 2024 21:24:35 -0800, Saravana Kannan wrote:
> Introduced a stupid bug in commit 782bfd03c3ae ("of: property: Improve
> finding the supplier of a remote-endpoint property") due to a last minute
> incorrect edit of "index !=0" into "!index". This patch fixes it to be
> "index > 0" to match the comment right next to it.
> 
> Reported-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> Link: https://lore.kernel.org/lkml/20240223171849.10f9901d@booty/
> Fixes: 782bfd03c3ae ("of: property: Improve finding the supplier of a remote-endpoint property")
> Signed-off-by: Saravana Kannan <saravanak@google.com>
> ---
> Using Link: instead of Closes: because Luca reported two separate issues.
> 
> Sorry about introducing a stupid bug in an -rcX Rob.
> 
> -Saravana
> 
>  drivers/of/property.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 

Applied, thanks!

Re: [PATCH] of: property: fw_devlink: Fix stupid bug in remote-endpoint parsing

[Saravana Kannan]

On Thu, Feb 29, 2024 at 2:11 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Saravana,
>
> On Sat, Feb 24, 2024 at 6:25 AM Saravana Kannan <saravanak@google.com> wrote:
> > Introduced a stupid bug in commit 782bfd03c3ae ("of: property: Improve
> > finding the supplier of a remote-endpoint property") due to a last minute
> > incorrect edit of "index !=0" into "!index". This patch fixes it to be
> > "index > 0" to match the comment right next to it.
> >
> > Reported-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> > Link: https://lore.kernel.org/lkml/20240223171849.10f9901d@booty/
> > Fixes: 782bfd03c3ae ("of: property: Improve finding the supplier of a remote-endpoint property")
> > Signed-off-by: Saravana Kannan <saravanak@google.com>
>
> Thanks for your patch!
>
> > --- a/drivers/of/property.c
> > +++ b/drivers/of/property.c
> > @@ -1304,7 +1304,7 @@ static struct device_node *parse_remote_endpoint(struct device_node *np,
> >                                                  int index)
> >  {
> >         /* Return NULL for index > 0 to signify end of remote-endpoints. */
> > -       if (!index || strcmp(prop_name, "remote-endpoint"))
> > +       if (index > 0 || strcmp(prop_name, "remote-endpoint"))
> >                 return NULL;
> >
> >         return of_graph_get_remote_port_parent(np);
> > --
> > 2.44.0.rc0.258.g7320e95886-goog
>
> After this, the "Fixed dependency cycle" messages I reported to be
> gone in [1] are back.
>
> In fact, they are slightly different, and there are now even more of them:
>
> -platform fea80000.csi2: Fixed dependency cycle(s) with
> /soc/video@e6ef7000/ports/port@1/endpoint@0
> -platform fea80000.csi2: Fixed dependency cycle(s) with
> /soc/video@e6ef6000/ports/port@1/endpoint@0
> -platform fea80000.csi2: Fixed dependency cycle(s) with
> /soc/video@e6ef5000/ports/port@1/endpoint@0
> -platform fea80000.csi2: Fixed dependency cycle(s) with
> /soc/video@e6ef4000/ports/port@1/endpoint@0
> -platform fea80000.csi2: Fixed dependency cycle(s) with
> /soc/video@e6ef3000/ports/port@1/endpoint@0
> -platform fea80000.csi2: Fixed dependency cycle(s) with
> /soc/video@e6ef2000/ports/port@1/endpoint@0
> -platform fea80000.csi2: Fixed dependency cycle(s) with
> /soc/video@e6ef1000/ports/port@1/endpoint@0
> -platform fea80000.csi2: Fixed dependency cycle(s) with
> /soc/video@e6ef0000/ports/port@1/endpoint@0
> -platform feaa0000.csi2: Fixed dependency cycle(s) with
> /soc/video@e6ef3000/ports/port@1/endpoint@2
> -platform feaa0000.csi2: Fixed dependency cycle(s) with
> /soc/video@e6ef2000/ports/port@1/endpoint@2
> -platform feaa0000.csi2: Fixed dependency cycle(s) with
> /soc/video@e6ef1000/ports/port@1/endpoint@2
> -platform feaa0000.csi2: Fixed dependency cycle(s) with
> /soc/video@e6ef0000/ports/port@1/endpoint@2
> -platform fead0000.hdmi: Fixed dependency cycle(s) with
> /soc/sound@ec500000/ports/port@1/endpoint
> -platform feae0000.hdmi: Fixed dependency cycle(s) with
> /soc/sound@ec500000/ports/port@2/endpoint
> -platform feb00000.display: Fixed dependency cycle(s) with
> /soc/hdmi@feae0000/ports/port@0/endpoint
> -platform feb00000.display: Fixed dependency cycle(s) with
> /soc/hdmi@fead0000/ports/port@0/endpoint
> -platform hdmi0-out: Fixed dependency cycle(s) with
> /soc/hdmi@fead0000/ports/port@1/endpoint
> -platform hdmi1-out: Fixed dependency cycle(s) with
> /soc/hdmi@feae0000/ports/port@1/endpoint
> -platform vga-encoder: Fixed dependency cycle(s) with /vga/port/endpoint
> -platform vga-encoder: Fixed dependency cycle(s) with
> /soc/display@feb00000/ports/port@0/endpoint
> +platform e6ef0000.video: Fixed dependency cycle(s) with /soc/csi2@feaa0000
> +platform e6ef0000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> +platform e6ef1000.video: Fixed dependency cycle(s) with /soc/csi2@feaa0000
> +platform e6ef1000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> +platform e6ef2000.video: Fixed dependency cycle(s) with /soc/csi2@feaa0000
> +platform e6ef2000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> +platform e6ef3000.video: Fixed dependency cycle(s) with /soc/csi2@feaa0000
> +platform e6ef3000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> +platform e6ef4000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> +platform e6ef5000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> +platform e6ef6000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> +platform e6ef7000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> +platform ec500000.sound: Fixed dependency cycle(s) with /soc/hdmi@feae0000
> +platform ec500000.sound: Fixed dependency cycle(s) with /soc/hdmi@fead0000
> +platform ec500000.sound: Fixed dependency cycle(s) with
> /soc/i2c@e6510000/codec@10
> +platform e6ef7000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> +platform e6ef6000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> +platform e6ef5000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> +platform e6ef4000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> +platform e6ef3000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> +platform e6ef2000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> +platform e6ef1000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> +platform e6ef0000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> +platform fea80000.csi2: Fixed dependency cycle(s) with /soc/video@e6ef7000
> +platform fea80000.csi2: Fixed dependency cycle(s) with /soc/video@e6ef6000
> +platform fea80000.csi2: Fixed dependency cycle(s) with /soc/video@e6ef5000
> +platform fea80000.csi2: Fixed dependency cycle(s) with /soc/video@e6ef4000
> +platform fea80000.csi2: Fixed dependency cycle(s) with /soc/video@e6ef3000
> +platform fea80000.csi2: Fixed dependency cycle(s) with /soc/video@e6ef2000
> +platform fea80000.csi2: Fixed dependency cycle(s) with /soc/video@e6ef1000
> +platform fea80000.csi2: Fixed dependency cycle(s) with /soc/video@e6ef0000
> +platform fea80000.csi2: Fixed dependency cycle(s) with
> /soc/i2c@e66d8000/video-receiver@70
> +platform e6ef3000.video: Fixed dependency cycle(s) with /soc/csi2@feaa0000
> +platform e6ef2000.video: Fixed dependency cycle(s) with /soc/csi2@feaa0000
> +platform e6ef1000.video: Fixed dependency cycle(s) with /soc/csi2@feaa0000
> +platform e6ef0000.video: Fixed dependency cycle(s) with /soc/csi2@feaa0000
> +platform feaa0000.csi2: Fixed dependency cycle(s) with /soc/video@e6ef3000
> +platform feaa0000.csi2: Fixed dependency cycle(s) with /soc/video@e6ef2000
> +platform feaa0000.csi2: Fixed dependency cycle(s) with /soc/video@e6ef1000
> +platform feaa0000.csi2: Fixed dependency cycle(s) with /soc/video@e6ef0000
> +platform feaa0000.csi2: Fixed dependency cycle(s) with
> /soc/i2c@e66d8000/video-receiver@70
> +platform ec500000.sound: Fixed dependency cycle(s) with /soc/hdmi@fead0000
> +platform fead0000.hdmi: Fixed dependency cycle(s) with /soc/sound@ec500000
> +platform fead0000.hdmi: Fixed dependency cycle(s) with /soc/display@feb00000
> +platform ec500000.sound: Fixed dependency cycle(s) with /soc/hdmi@feae0000
> +platform feae0000.hdmi: Fixed dependency cycle(s) with /soc/sound@ec500000
> +platform feae0000.hdmi: Fixed dependency cycle(s) with /soc/display@feb00000
> +platform feae0000.hdmi: Fixed dependency cycle(s) with /soc/display@feb00000
> +platform fead0000.hdmi: Fixed dependency cycle(s) with /soc/display@feb00000
> +platform feb00000.display: Fixed dependency cycle(s) with /soc/hdmi@feae0000
> +platform feb00000.display: Fixed dependency cycle(s) with /soc/hdmi@fead0000
> +platform cvbs-in: Fixed dependency cycle(s) with
> /soc/i2c@e66d8000/video-receiver@70
> +platform hdmi-in: Fixed dependency cycle(s) with
> /soc/i2c@e66d8000/video-receiver@70
> +platform fead0000.hdmi: Fixed dependency cycle(s) with /hdmi0-out
> +platform hdmi0-out: Fixed dependency cycle(s) with /soc/hdmi@fead0000
> +platform feae0000.hdmi: Fixed dependency cycle(s) with /hdmi1-out
> +platform hdmi1-out: Fixed dependency cycle(s) with /soc/hdmi@feae0000
> +platform vga: Fixed dependency cycle(s) with /vga-encoder
> +platform feb00000.display: Fixed dependency cycle(s) with /vga-encoder
> +platform vga-encoder: Fixed dependency cycle(s) with /vga
> +platform vga-encoder: Fixed dependency cycle(s) with /soc/display@feb00000
>
> -i2c 2-0010: Fixed dependency cycle(s) with
> /soc/sound@ec500000/ports/port@0/endpoint
> +platform ec500000.sound: Fixed dependency cycle(s) with
> /soc/i2c@e6510000/codec@10
>
> -i2c 4-0070: Fixed dependency cycle(s) with
> /soc/csi2@fea80000/ports/port@0/endpoint
> -i2c 4-0070: Fixed dependency cycle(s) with
> /soc/csi2@feaa0000/ports/port@0/endpoint
> -i2c 4-0070: Fixed dependency cycle(s) with /hdmi-in/port/endpoint
> -i2c 4-0070: Fixed dependency cycle(s) with /cvbs-in/port/endpoint
> +platform feaa0000.csi2: Fixed dependency cycle(s) with
> /soc/i2c@e66d8000/video-receiver@70
> +platform fea80000.csi2: Fixed dependency cycle(s) with
> /soc/i2c@e66d8000/video-receiver@70
> +i2c 4-0070: Fixed dependency cycle(s) with /soc/csi2@fea80000
> +i2c 4-0070: Fixed dependency cycle(s) with /soc/csi2@feaa0000
>
> I guess all of that is expected?

Hi Geert,

Greg has picked up my "post-init-providers" series in his driver core.
Once you pull that in, you should be able to use the
post-init-providers property to break these cycles. Basically treat it
like any other supplier binding, but use it to mark the link in the
cycle that's not real. For the remote-endpoints case, you need to list
property at the device level. Not the endpoint, port or ports nodes.

Once you add this property, you should see an increase in the number
of device links that are present after all device probing is done.
Also, a bunch of existing device links should get converted from
sync_state_only device links to normal managed device links. Meaning,
the sync_state_only file under those /class/devlink/<device-link-x>/
folder should change from "1" to "0".

If that's what you see and it works, then go ahead and send out a
patch so that the boards you care about have a more
deterministic/stable probe/suspend/resume ordering.

Thanks,
Saravana



>
> [1] https://lore.kernel.org/all/CAMuHMdVon3mdivZQ0O6D4+va0nGBrUQbDp23bEq661QD=4t7+g@mail.gmail.com/
>
> 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

Re: [PATCH] of: property: fw_devlink: Fix stupid bug in remote-endpoint parsing

[Geert Uytterhoeven]

Hi Saravana,

On Thu, Mar 14, 2024 at 2:48 AM Saravana Kannan <saravanak@google.com> wrote:
> On Thu, Feb 29, 2024 at 2:11 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > On Sat, Feb 24, 2024 at 6:25 AM Saravana Kannan <saravanak@google.com> wrote:
> > > Introduced a stupid bug in commit 782bfd03c3ae ("of: property: Improve
> > > finding the supplier of a remote-endpoint property") due to a last minute
> > > incorrect edit of "index !=0" into "!index". This patch fixes it to be
> > > "index > 0" to match the comment right next to it.
> > >
> > > Reported-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> > > Link: https://lore.kernel.org/lkml/20240223171849.10f9901d@booty/
> > > Fixes: 782bfd03c3ae ("of: property: Improve finding the supplier of a remote-endpoint property")
> > > Signed-off-by: Saravana Kannan <saravanak@google.com>
> >
> > Thanks for your patch!
> >
> > > --- a/drivers/of/property.c
> > > +++ b/drivers/of/property.c
> > > @@ -1304,7 +1304,7 @@ static struct device_node *parse_remote_endpoint(struct device_node *np,
> > >                                                  int index)
> > >  {
> > >         /* Return NULL for index > 0 to signify end of remote-endpoints. */
> > > -       if (!index || strcmp(prop_name, "remote-endpoint"))
> > > +       if (index > 0 || strcmp(prop_name, "remote-endpoint"))
> > >                 return NULL;
> > >
> > >         return of_graph_get_remote_port_parent(np);
> > > --
> > > 2.44.0.rc0.258.g7320e95886-goog
> >
> > After this, the "Fixed dependency cycle" messages I reported to be
> > gone in [1] are back.
> >
> > In fact, they are slightly different, and there are now even more of them:
> >
> > -platform fea80000.csi2: Fixed dependency cycle(s) with
> > /soc/video@e6ef7000/ports/port@1/endpoint@0
> > -platform fea80000.csi2: Fixed dependency cycle(s) with
> > /soc/video@e6ef6000/ports/port@1/endpoint@0
> > -platform fea80000.csi2: Fixed dependency cycle(s) with
> > /soc/video@e6ef5000/ports/port@1/endpoint@0
> > -platform fea80000.csi2: Fixed dependency cycle(s) with
> > /soc/video@e6ef4000/ports/port@1/endpoint@0
> > -platform fea80000.csi2: Fixed dependency cycle(s) with
> > /soc/video@e6ef3000/ports/port@1/endpoint@0
> > -platform fea80000.csi2: Fixed dependency cycle(s) with
> > /soc/video@e6ef2000/ports/port@1/endpoint@0
> > -platform fea80000.csi2: Fixed dependency cycle(s) with
> > /soc/video@e6ef1000/ports/port@1/endpoint@0
> > -platform fea80000.csi2: Fixed dependency cycle(s) with
> > /soc/video@e6ef0000/ports/port@1/endpoint@0
> > -platform feaa0000.csi2: Fixed dependency cycle(s) with
> > /soc/video@e6ef3000/ports/port@1/endpoint@2
> > -platform feaa0000.csi2: Fixed dependency cycle(s) with
> > /soc/video@e6ef2000/ports/port@1/endpoint@2
> > -platform feaa0000.csi2: Fixed dependency cycle(s) with
> > /soc/video@e6ef1000/ports/port@1/endpoint@2
> > -platform feaa0000.csi2: Fixed dependency cycle(s) with
> > /soc/video@e6ef0000/ports/port@1/endpoint@2
> > -platform fead0000.hdmi: Fixed dependency cycle(s) with
> > /soc/sound@ec500000/ports/port@1/endpoint
> > -platform feae0000.hdmi: Fixed dependency cycle(s) with
> > /soc/sound@ec500000/ports/port@2/endpoint
> > -platform feb00000.display: Fixed dependency cycle(s) with
> > /soc/hdmi@feae0000/ports/port@0/endpoint
> > -platform feb00000.display: Fixed dependency cycle(s) with
> > /soc/hdmi@fead0000/ports/port@0/endpoint
> > -platform hdmi0-out: Fixed dependency cycle(s) with
> > /soc/hdmi@fead0000/ports/port@1/endpoint
> > -platform hdmi1-out: Fixed dependency cycle(s) with
> > /soc/hdmi@feae0000/ports/port@1/endpoint
> > -platform vga-encoder: Fixed dependency cycle(s) with /vga/port/endpoint
> > -platform vga-encoder: Fixed dependency cycle(s) with
> > /soc/display@feb00000/ports/port@0/endpoint
> > +platform e6ef0000.video: Fixed dependency cycle(s) with /soc/csi2@feaa0000
> > +platform e6ef0000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> > +platform e6ef1000.video: Fixed dependency cycle(s) with /soc/csi2@feaa0000
> > +platform e6ef1000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> > +platform e6ef2000.video: Fixed dependency cycle(s) with /soc/csi2@feaa0000
> > +platform e6ef2000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> > +platform e6ef3000.video: Fixed dependency cycle(s) with /soc/csi2@feaa0000
> > +platform e6ef3000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> > +platform e6ef4000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> > +platform e6ef5000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> > +platform e6ef6000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> > +platform e6ef7000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> > +platform ec500000.sound: Fixed dependency cycle(s) with /soc/hdmi@feae0000
> > +platform ec500000.sound: Fixed dependency cycle(s) with /soc/hdmi@fead0000
> > +platform ec500000.sound: Fixed dependency cycle(s) with
> > /soc/i2c@e6510000/codec@10
> > +platform e6ef7000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> > +platform e6ef6000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> > +platform e6ef5000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> > +platform e6ef4000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> > +platform e6ef3000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> > +platform e6ef2000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> > +platform e6ef1000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> > +platform e6ef0000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> > +platform fea80000.csi2: Fixed dependency cycle(s) with /soc/video@e6ef7000
> > +platform fea80000.csi2: Fixed dependency cycle(s) with /soc/video@e6ef6000
> > +platform fea80000.csi2: Fixed dependency cycle(s) with /soc/video@e6ef5000
> > +platform fea80000.csi2: Fixed dependency cycle(s) with /soc/video@e6ef4000
> > +platform fea80000.csi2: Fixed dependency cycle(s) with /soc/video@e6ef3000
> > +platform fea80000.csi2: Fixed dependency cycle(s) with /soc/video@e6ef2000
> > +platform fea80000.csi2: Fixed dependency cycle(s) with /soc/video@e6ef1000
> > +platform fea80000.csi2: Fixed dependency cycle(s) with /soc/video@e6ef0000
> > +platform fea80000.csi2: Fixed dependency cycle(s) with
> > /soc/i2c@e66d8000/video-receiver@70
> > +platform e6ef3000.video: Fixed dependency cycle(s) with /soc/csi2@feaa0000
> > +platform e6ef2000.video: Fixed dependency cycle(s) with /soc/csi2@feaa0000
> > +platform e6ef1000.video: Fixed dependency cycle(s) with /soc/csi2@feaa0000
> > +platform e6ef0000.video: Fixed dependency cycle(s) with /soc/csi2@feaa0000
> > +platform feaa0000.csi2: Fixed dependency cycle(s) with /soc/video@e6ef3000
> > +platform feaa0000.csi2: Fixed dependency cycle(s) with /soc/video@e6ef2000
> > +platform feaa0000.csi2: Fixed dependency cycle(s) with /soc/video@e6ef1000
> > +platform feaa0000.csi2: Fixed dependency cycle(s) with /soc/video@e6ef0000
> > +platform feaa0000.csi2: Fixed dependency cycle(s) with
> > /soc/i2c@e66d8000/video-receiver@70
> > +platform ec500000.sound: Fixed dependency cycle(s) with /soc/hdmi@fead0000
> > +platform fead0000.hdmi: Fixed dependency cycle(s) with /soc/sound@ec500000
> > +platform fead0000.hdmi: Fixed dependency cycle(s) with /soc/display@feb00000
> > +platform ec500000.sound: Fixed dependency cycle(s) with /soc/hdmi@feae0000
> > +platform feae0000.hdmi: Fixed dependency cycle(s) with /soc/sound@ec500000
> > +platform feae0000.hdmi: Fixed dependency cycle(s) with /soc/display@feb00000
> > +platform feae0000.hdmi: Fixed dependency cycle(s) with /soc/display@feb00000
> > +platform fead0000.hdmi: Fixed dependency cycle(s) with /soc/display@feb00000
> > +platform feb00000.display: Fixed dependency cycle(s) with /soc/hdmi@feae0000
> > +platform feb00000.display: Fixed dependency cycle(s) with /soc/hdmi@fead0000
> > +platform cvbs-in: Fixed dependency cycle(s) with
> > /soc/i2c@e66d8000/video-receiver@70
> > +platform hdmi-in: Fixed dependency cycle(s) with
> > /soc/i2c@e66d8000/video-receiver@70
> > +platform fead0000.hdmi: Fixed dependency cycle(s) with /hdmi0-out
> > +platform hdmi0-out: Fixed dependency cycle(s) with /soc/hdmi@fead0000
> > +platform feae0000.hdmi: Fixed dependency cycle(s) with /hdmi1-out
> > +platform hdmi1-out: Fixed dependency cycle(s) with /soc/hdmi@feae0000
> > +platform vga: Fixed dependency cycle(s) with /vga-encoder
> > +platform feb00000.display: Fixed dependency cycle(s) with /vga-encoder
> > +platform vga-encoder: Fixed dependency cycle(s) with /vga
> > +platform vga-encoder: Fixed dependency cycle(s) with /soc/display@feb00000
> >
> > -i2c 2-0010: Fixed dependency cycle(s) with
> > /soc/sound@ec500000/ports/port@0/endpoint
> > +platform ec500000.sound: Fixed dependency cycle(s) with
> > /soc/i2c@e6510000/codec@10
> >
> > -i2c 4-0070: Fixed dependency cycle(s) with
> > /soc/csi2@fea80000/ports/port@0/endpoint
> > -i2c 4-0070: Fixed dependency cycle(s) with
> > /soc/csi2@feaa0000/ports/port@0/endpoint
> > -i2c 4-0070: Fixed dependency cycle(s) with /hdmi-in/port/endpoint
> > -i2c 4-0070: Fixed dependency cycle(s) with /cvbs-in/port/endpoint
> > +platform feaa0000.csi2: Fixed dependency cycle(s) with
> > /soc/i2c@e66d8000/video-receiver@70
> > +platform fea80000.csi2: Fixed dependency cycle(s) with
> > /soc/i2c@e66d8000/video-receiver@70
> > +i2c 4-0070: Fixed dependency cycle(s) with /soc/csi2@fea80000
> > +i2c 4-0070: Fixed dependency cycle(s) with /soc/csi2@feaa0000
> >
> > I guess all of that is expected?
>
> Hi Geert,
>
> Greg has picked up my "post-init-providers" series in his driver core.

You mean https://lore.kernel.org/all/20240305050458.1400667-2-saravanak@google.com/?

> Once you pull that in, you should be able to use the
> post-init-providers property to break these cycles. Basically treat it
> like any other supplier binding, but use it to mark the link in the
> cycle that's not real. For the remote-endpoints case, you need to list
> property at the device level. Not the endpoint, port or ports nodes.
>
> Once you add this property, you should see an increase in the number
> of device links that are present after all device probing is done.
> Also, a bunch of existing device links should get converted from
> sync_state_only device links to normal managed device links. Meaning,
> the sync_state_only file under those /class/devlink/<device-link-x>/
> folder should change from "1" to "0".
>
> If that's what you see and it works, then go ahead and send out a
> patch so that the boards you care about have a more
> deterministic/stable probe/suspend/resume ordering.

You mean we have to add additional properties to the DTS?
What about compatibility with old DTBs?

Where are the DT bindings for "post-init-providers"?
I see it was part of earlier submissions, but I cannot find any evidence
they have ever been accepted by the DT maintainers?

Thanks!

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

Re: [PATCH] of: property: fw_devlink: Fix stupid bug in remote-endpoint parsing

[Saravana Kannan]

On Thu, Mar 14, 2024 at 1:46 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Saravana,
>
> On Thu, Mar 14, 2024 at 2:48 AM Saravana Kannan <saravanak@google.com> wrote:
> > On Thu, Feb 29, 2024 at 2:11 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > On Sat, Feb 24, 2024 at 6:25 AM Saravana Kannan <saravanak@google.com> wrote:
> > > > Introduced a stupid bug in commit 782bfd03c3ae ("of: property: Improve
> > > > finding the supplier of a remote-endpoint property") due to a last minute
> > > > incorrect edit of "index !=0" into "!index". This patch fixes it to be
> > > > "index > 0" to match the comment right next to it.
> > > >
> > > > Reported-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> > > > Link: https://lore.kernel.org/lkml/20240223171849.10f9901d@booty/
> > > > Fixes: 782bfd03c3ae ("of: property: Improve finding the supplier of a remote-endpoint property")
> > > > Signed-off-by: Saravana Kannan <saravanak@google.com>
> > >
> > > Thanks for your patch!
> > >
> > > > --- a/drivers/of/property.c
> > > > +++ b/drivers/of/property.c
> > > > @@ -1304,7 +1304,7 @@ static struct device_node *parse_remote_endpoint(struct device_node *np,
> > > >                                                  int index)
> > > >  {
> > > >         /* Return NULL for index > 0 to signify end of remote-endpoints. */
> > > > -       if (!index || strcmp(prop_name, "remote-endpoint"))
> > > > +       if (index > 0 || strcmp(prop_name, "remote-endpoint"))
> > > >                 return NULL;
> > > >
> > > >         return of_graph_get_remote_port_parent(np);
> > > > --
> > > > 2.44.0.rc0.258.g7320e95886-goog
> > >
> > > After this, the "Fixed dependency cycle" messages I reported to be
> > > gone in [1] are back.
> > >
> > > In fact, they are slightly different, and there are now even more of them:
> > >
> > > -platform fea80000.csi2: Fixed dependency cycle(s) with
> > > /soc/video@e6ef7000/ports/port@1/endpoint@0
> > > -platform fea80000.csi2: Fixed dependency cycle(s) with
> > > /soc/video@e6ef6000/ports/port@1/endpoint@0
> > > -platform fea80000.csi2: Fixed dependency cycle(s) with
> > > /soc/video@e6ef5000/ports/port@1/endpoint@0
> > > -platform fea80000.csi2: Fixed dependency cycle(s) with
> > > /soc/video@e6ef4000/ports/port@1/endpoint@0
> > > -platform fea80000.csi2: Fixed dependency cycle(s) with
> > > /soc/video@e6ef3000/ports/port@1/endpoint@0
> > > -platform fea80000.csi2: Fixed dependency cycle(s) with
> > > /soc/video@e6ef2000/ports/port@1/endpoint@0
> > > -platform fea80000.csi2: Fixed dependency cycle(s) with
> > > /soc/video@e6ef1000/ports/port@1/endpoint@0
> > > -platform fea80000.csi2: Fixed dependency cycle(s) with
> > > /soc/video@e6ef0000/ports/port@1/endpoint@0
> > > -platform feaa0000.csi2: Fixed dependency cycle(s) with
> > > /soc/video@e6ef3000/ports/port@1/endpoint@2
> > > -platform feaa0000.csi2: Fixed dependency cycle(s) with
> > > /soc/video@e6ef2000/ports/port@1/endpoint@2
> > > -platform feaa0000.csi2: Fixed dependency cycle(s) with
> > > /soc/video@e6ef1000/ports/port@1/endpoint@2
> > > -platform feaa0000.csi2: Fixed dependency cycle(s) with
> > > /soc/video@e6ef0000/ports/port@1/endpoint@2
> > > -platform fead0000.hdmi: Fixed dependency cycle(s) with
> > > /soc/sound@ec500000/ports/port@1/endpoint
> > > -platform feae0000.hdmi: Fixed dependency cycle(s) with
> > > /soc/sound@ec500000/ports/port@2/endpoint
> > > -platform feb00000.display: Fixed dependency cycle(s) with
> > > /soc/hdmi@feae0000/ports/port@0/endpoint
> > > -platform feb00000.display: Fixed dependency cycle(s) with
> > > /soc/hdmi@fead0000/ports/port@0/endpoint
> > > -platform hdmi0-out: Fixed dependency cycle(s) with
> > > /soc/hdmi@fead0000/ports/port@1/endpoint
> > > -platform hdmi1-out: Fixed dependency cycle(s) with
> > > /soc/hdmi@feae0000/ports/port@1/endpoint
> > > -platform vga-encoder: Fixed dependency cycle(s) with /vga/port/endpoint
> > > -platform vga-encoder: Fixed dependency cycle(s) with
> > > /soc/display@feb00000/ports/port@0/endpoint
> > > +platform e6ef0000.video: Fixed dependency cycle(s) with /soc/csi2@feaa0000
> > > +platform e6ef0000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> > > +platform e6ef1000.video: Fixed dependency cycle(s) with /soc/csi2@feaa0000
> > > +platform e6ef1000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> > > +platform e6ef2000.video: Fixed dependency cycle(s) with /soc/csi2@feaa0000
> > > +platform e6ef2000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> > > +platform e6ef3000.video: Fixed dependency cycle(s) with /soc/csi2@feaa0000
> > > +platform e6ef3000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> > > +platform e6ef4000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> > > +platform e6ef5000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> > > +platform e6ef6000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> > > +platform e6ef7000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> > > +platform ec500000.sound: Fixed dependency cycle(s) with /soc/hdmi@feae0000
> > > +platform ec500000.sound: Fixed dependency cycle(s) with /soc/hdmi@fead0000
> > > +platform ec500000.sound: Fixed dependency cycle(s) with
> > > /soc/i2c@e6510000/codec@10
> > > +platform e6ef7000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> > > +platform e6ef6000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> > > +platform e6ef5000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> > > +platform e6ef4000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> > > +platform e6ef3000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> > > +platform e6ef2000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> > > +platform e6ef1000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> > > +platform e6ef0000.video: Fixed dependency cycle(s) with /soc/csi2@fea80000
> > > +platform fea80000.csi2: Fixed dependency cycle(s) with /soc/video@e6ef7000
> > > +platform fea80000.csi2: Fixed dependency cycle(s) with /soc/video@e6ef6000
> > > +platform fea80000.csi2: Fixed dependency cycle(s) with /soc/video@e6ef5000
> > > +platform fea80000.csi2: Fixed dependency cycle(s) with /soc/video@e6ef4000
> > > +platform fea80000.csi2: Fixed dependency cycle(s) with /soc/video@e6ef3000
> > > +platform fea80000.csi2: Fixed dependency cycle(s) with /soc/video@e6ef2000
> > > +platform fea80000.csi2: Fixed dependency cycle(s) with /soc/video@e6ef1000
> > > +platform fea80000.csi2: Fixed dependency cycle(s) with /soc/video@e6ef0000
> > > +platform fea80000.csi2: Fixed dependency cycle(s) with
> > > /soc/i2c@e66d8000/video-receiver@70
> > > +platform e6ef3000.video: Fixed dependency cycle(s) with /soc/csi2@feaa0000
> > > +platform e6ef2000.video: Fixed dependency cycle(s) with /soc/csi2@feaa0000
> > > +platform e6ef1000.video: Fixed dependency cycle(s) with /soc/csi2@feaa0000
> > > +platform e6ef0000.video: Fixed dependency cycle(s) with /soc/csi2@feaa0000
> > > +platform feaa0000.csi2: Fixed dependency cycle(s) with /soc/video@e6ef3000
> > > +platform feaa0000.csi2: Fixed dependency cycle(s) with /soc/video@e6ef2000
> > > +platform feaa0000.csi2: Fixed dependency cycle(s) with /soc/video@e6ef1000
> > > +platform feaa0000.csi2: Fixed dependency cycle(s) with /soc/video@e6ef0000
> > > +platform feaa0000.csi2: Fixed dependency cycle(s) with
> > > /soc/i2c@e66d8000/video-receiver@70
> > > +platform ec500000.sound: Fixed dependency cycle(s) with /soc/hdmi@fead0000
> > > +platform fead0000.hdmi: Fixed dependency cycle(s) with /soc/sound@ec500000
> > > +platform fead0000.hdmi: Fixed dependency cycle(s) with /soc/display@feb00000
> > > +platform ec500000.sound: Fixed dependency cycle(s) with /soc/hdmi@feae0000
> > > +platform feae0000.hdmi: Fixed dependency cycle(s) with /soc/sound@ec500000
> > > +platform feae0000.hdmi: Fixed dependency cycle(s) with /soc/display@feb00000
> > > +platform feae0000.hdmi: Fixed dependency cycle(s) with /soc/display@feb00000
> > > +platform fead0000.hdmi: Fixed dependency cycle(s) with /soc/display@feb00000
> > > +platform feb00000.display: Fixed dependency cycle(s) with /soc/hdmi@feae0000
> > > +platform feb00000.display: Fixed dependency cycle(s) with /soc/hdmi@fead0000
> > > +platform cvbs-in: Fixed dependency cycle(s) with
> > > /soc/i2c@e66d8000/video-receiver@70
> > > +platform hdmi-in: Fixed dependency cycle(s) with
> > > /soc/i2c@e66d8000/video-receiver@70
> > > +platform fead0000.hdmi: Fixed dependency cycle(s) with /hdmi0-out
> > > +platform hdmi0-out: Fixed dependency cycle(s) with /soc/hdmi@fead0000
> > > +platform feae0000.hdmi: Fixed dependency cycle(s) with /hdmi1-out
> > > +platform hdmi1-out: Fixed dependency cycle(s) with /soc/hdmi@feae0000
> > > +platform vga: Fixed dependency cycle(s) with /vga-encoder
> > > +platform feb00000.display: Fixed dependency cycle(s) with /vga-encoder
> > > +platform vga-encoder: Fixed dependency cycle(s) with /vga
> > > +platform vga-encoder: Fixed dependency cycle(s) with /soc/display@feb00000
> > >
> > > -i2c 2-0010: Fixed dependency cycle(s) with
> > > /soc/sound@ec500000/ports/port@0/endpoint
> > > +platform ec500000.sound: Fixed dependency cycle(s) with
> > > /soc/i2c@e6510000/codec@10
> > >
> > > -i2c 4-0070: Fixed dependency cycle(s) with
> > > /soc/csi2@fea80000/ports/port@0/endpoint
> > > -i2c 4-0070: Fixed dependency cycle(s) with
> > > /soc/csi2@feaa0000/ports/port@0/endpoint
> > > -i2c 4-0070: Fixed dependency cycle(s) with /hdmi-in/port/endpoint
> > > -i2c 4-0070: Fixed dependency cycle(s) with /cvbs-in/port/endpoint
> > > +platform feaa0000.csi2: Fixed dependency cycle(s) with
> > > /soc/i2c@e66d8000/video-receiver@70
> > > +platform fea80000.csi2: Fixed dependency cycle(s) with
> > > /soc/i2c@e66d8000/video-receiver@70
> > > +i2c 4-0070: Fixed dependency cycle(s) with /soc/csi2@fea80000
> > > +i2c 4-0070: Fixed dependency cycle(s) with /soc/csi2@feaa0000
> > >
> > > I guess all of that is expected?
> >
> > Hi Geert,
> >
> > Greg has picked up my "post-init-providers" series in his driver core.
>
> You mean https://lore.kernel.org/all/20240305050458.1400667-2-saravanak@google.com/?

Yes.

>
> > Once you pull that in, you should be able to use the
> > post-init-providers property to break these cycles. Basically treat it
> > like any other supplier binding, but use it to mark the link in the
> > cycle that's not real. For the remote-endpoints case, you need to list
> > property at the device level. Not the endpoint, port or ports nodes.
> >
> > Once you add this property, you should see an increase in the number
> > of device links that are present after all device probing is done.
> > Also, a bunch of existing device links should get converted from
> > sync_state_only device links to normal managed device links. Meaning,
> > the sync_state_only file under those /class/devlink/<device-link-x>/
> > folder should change from "1" to "0".
> >
> > If that's what you see and it works, then go ahead and send out a
> > patch so that the boards you care about have a more
> > deterministic/stable probe/suspend/resume ordering.
>
> You mean we have to add additional properties to the DTS?

Yes.

> What about compatibility with old DTBs?

It'll continue working like today.

> Where are the DT bindings for "post-init-providers"?
> I see it was part of earlier submissions, but I cannot find any evidence
> they have ever been accepted by the DT maintainers?

It's been submitted to dt-schema. Rob is okay with it. So it'll land.

You don't need to merge your patches till it lands. But it'll be good
to test it for your case and confirm it makes things better for you.

-Saravana

>
> Thanks!
>
> 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

Re: [PATCH] of: property: fw_devlink: Fix stupid bug in remote-endpoint parsing

[John Watts]

On Fri, Feb 23, 2024 at 09:24:35PM -0800, Saravana Kannan wrote:
> Introduced a stupid bug in commit 782bfd03c3ae ("of: property: Improve
> finding the supplier of a remote-endpoint property") due to a last minute
> incorrect edit of "index !=0" into "!index". This patch fixes it to be
> "index > 0" to match the comment right next to it.
> 
> Reported-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> Link: https://lore.kernel.org/lkml/20240223171849.10f9901d@booty/
> Fixes: 782bfd03c3ae ("of: property: Improve finding the supplier of a remote-endpoint property")
> Signed-off-by: Saravana Kannan <saravanak@google.com>
> ---
> Using Link: instead of Closes: because Luca reported two separate issues.
> 
> Sorry about introducing a stupid bug in an -rcX Rob.
> 
> -Saravana

Hi there,

Could this be reverted? It breaks my audio-graph-card2 setup:

[   17.116290] platform 2034000.i2s: deferred probe pending: platform: wait for supplier /sound/multi
[   17.125370] platform test_codec: deferred probe pending: platform: wait for supplier /sound/multi
[   17.134257] platform sound: deferred probe pending: asoc-audio-graph-card2: parse error

/ {
	...


	test_codec {
		compatible = "test-codec";
		prefix = "Test codec";
		#sound-dai-cells = <0>;
		port {
			test_ep: endpoint {
				remote-endpoint = <&card_ep_2>;
			};
		};
	};

	sound {
		compatible = "audio-graph-card2";
		label = "CS5368";
		links = <&i2s2_port>;
		multi {
			#address-cells = <1>;
			#size-cells = <0>;
			convert-channels = <16>;
			port@0 {
				reg = <0>;
				card_ep_0: endpoint {
					remote-endpoint = <&i2s2_ep>;
				};
			};
			//port@1 {
			//	reg = <1>;
			//	card_ep_1: endpoint {
			//		remote-endpoint = <&cs5368_ep>;
			//	};
			//};
			port@1 {
				reg = <1>;
				card_ep_2: endpoint {
					remote-endpoint = <&test_ep>;
				};
			};
		};
	};
};


&i2s2 {
	pinctrl-0 = <&i2s2_pins>, <&i2s2_din_pins>;
	pinctrl-names = "default";
	status = "okay";
	i2s2_port: port {
		i2s2_ep: endpoint {
			format = "dsp_a";
			bitclock-master;
			frame-master;
			remote-endpoint = <&card_ep_0>;
		};
	};
};

John.

Re: [PATCH] of: property: fw_devlink: Fix stupid bug in remote-endpoint parsing

[Saravana Kannan]

On Wed, Mar 20, 2024 at 11:05 PM John Watts <contact@jookia.org> wrote:
>
> On Fri, Feb 23, 2024 at 09:24:35PM -0800, Saravana Kannan wrote:
> > Introduced a stupid bug in commit 782bfd03c3ae ("of: property: Improve
> > finding the supplier of a remote-endpoint property") due to a last minute
> > incorrect edit of "index !=0" into "!index". This patch fixes it to be
> > "index > 0" to match the comment right next to it.
> >
> > Reported-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> > Link: https://lore.kernel.org/lkml/20240223171849.10f9901d@booty/
> > Fixes: 782bfd03c3ae ("of: property: Improve finding the supplier of a remote-endpoint property")
> > Signed-off-by: Saravana Kannan <saravanak@google.com>
> > ---
> > Using Link: instead of Closes: because Luca reported two separate issues.
> >
> > Sorry about introducing a stupid bug in an -rcX Rob.
> >
> > -Saravana
>
> Hi there,
>
> Could this be reverted? It breaks my audio-graph-card2 setup:
>
> [   17.116290] platform 2034000.i2s: deferred probe pending: platform: wait for supplier /sound/multi
> [   17.125370] platform test_codec: deferred probe pending: platform: wait for supplier /sound/multi
> [   17.134257] platform sound: deferred probe pending: asoc-audio-graph-card2: parse error

Hmmm.... cycle detection should work here and not enforce probe
ordering. I'd appreciate help with debugging that. Let me look at it
on Monday. Can you enabled all the debug logs in drivers/base/core.c
and tell me what cycle detection is telling about these nodes?

But the better fix would be to use the new "post-init-providers"
property. See below.

>
> / {
>         ...
>
>
>         test_codec {
>                 compatible = "test-codec";
>                 prefix = "Test codec";
>                 #sound-dai-cells = <0>;

post-init-provider = <&multi>;

Right now there's a cyclic dependency between test_codec and multi and
this tells the kernel that test codec needs to probe first.

Similar additions to the other nodes blocked on multi.

Thanks,
Saravana

>                 port {
>                         test_ep: endpoint {
>                                 remote-endpoint = <&card_ep_2>;
>                         };
>                 };
>         };
>
>         sound {
>                 compatible = "audio-graph-card2";
>                 label = "CS5368";
>                 links = <&i2s2_port>;
>                 multi {
>                         #address-cells = <1>;
>                         #size-cells = <0>;
>                         convert-channels = <16>;
>                         port@0 {
>                                 reg = <0>;
>                                 card_ep_0: endpoint {
>                                         remote-endpoint = <&i2s2_ep>;
>                                 };
>                         };
>                         //port@1 {
>                         //      reg = <1>;
>                         //      card_ep_1: endpoint {
>                         //              remote-endpoint = <&cs5368_ep>;
>                         //      };
>                         //};
>                         port@1 {
>                                 reg = <1>;
>                                 card_ep_2: endpoint {
>                                         remote-endpoint = <&test_ep>;
>                                 };
>                         };
>                 };
>         };
> };
>
>
> &i2s2 {
>         pinctrl-0 = <&i2s2_pins>, <&i2s2_din_pins>;
>         pinctrl-names = "default";
>         status = "okay";
>         i2s2_port: port {
>                 i2s2_ep: endpoint {
>                         format = "dsp_a";
>                         bitclock-master;
>                         frame-master;
>                         remote-endpoint = <&card_ep_0>;
>                 };
>         };
> };
>
> John.

Re: [PATCH] of: property: fw_devlink: Fix stupid bug in remote-endpoint parsing

[John Watts]

Hello again,

On Fri, Mar 22, 2024 at 06:53:57PM -0700, Saravana Kannan wrote:
> Hmmm.... cycle detection should work here and not enforce probe
> ordering. I'd appreciate help with debugging that. Let me look at it
> on Monday. Can you enabled all the debug logs in drivers/base/core.c
> and tell me what cycle detection is telling about these nodes?

Hmm. It's not saying anything more than what I've already sent.

I think this is because /sound/multi isn't a device, it's just a
subnode used in audio-graph-card2.
Removing the multi { } section and using direct graph connections
'fixes' this.

I think this might be because usually in a graph each node containing
ports is a device, such as a display panel, a bridge, an LCD
controller. These kind of form a dependency chain.

In this case all the ports in multi act as a way to glue multiple
ports together for the audio-graph-card2.

Does that help?

> But the better fix would be to use the new "post-init-providers"
> property. See below.
> 
> >
> > / {
> >         ...
> >
> >
> >         test_codec {
> >                 compatible = "test-codec";
> >                 prefix = "Test codec";
> >                 #sound-dai-cells = <0>;
> 
> post-init-provider = <&multi>;
> 
> Right now there's a cyclic dependency between test_codec and multi and
> this tells the kernel that test codec needs to probe first.
> 
> Similar additions to the other nodes blocked on multi.
> 
> Thanks,
> Saravana

John.

Re: [PATCH] of: property: fw_devlink: Fix stupid bug in remote-endpoint parsing

[Saravana Kannan]

On Sat, Mar 23, 2024 at 5:20 AM John Watts <contact@jookia.org> wrote:
>
> Hello again,
>
> On Fri, Mar 22, 2024 at 06:53:57PM -0700, Saravana Kannan wrote:
> > Hmmm.... cycle detection should work here and not enforce probe
> > ordering. I'd appreciate help with debugging that. Let me look at it
> > on Monday. Can you enabled all the debug logs in drivers/base/core.c
> > and tell me what cycle detection is telling about these nodes?
>
> Hmm. It's not saying anything more than what I've already sent.

Sorry, I was asking for the logs. But now I'm looking at this again, I
think I understand what's going on.

> I think this is because /sound/multi isn't a device, it's just a
> subnode used in audio-graph-card2.

Ok, I think I understand now what's going on. fw_devlink does not know
that "sound" device will not populate "multi" as a child device.
Typically in such situations, "sound" would probe as a device and add
its child DT nodes devices. At that point, the cycle is only between
"multi" and "test_codec" and fw_devlink will detect that and not
enforce any ordering. However, in this case, "sound" doesn't have any
child devices and just depends on the remote endpoints directly.

We already have "ports", "in-ports" and "out-ports". Is there a reason
none of them will work for your use case and it has to be "multi"?
When you use one of those 3 recognized node names, things are handled
correctly.

Btw, between "test_codec" and "sound", which one is supposed to probe
first? I'm guessing "test_codec" needs to probe first for "sound" to
probe?

> Removing the multi { } section and using direct graph connections
> 'fixes' this.

I think the right fix is the use of post-init-providers. Because even
if you do the above, all it does is let fw_devlink see that there's a
cyclic dependency in DT. And it'll stop enforcing the probe and
suspend/resume ordering. Ideally we want to enforce a specific order
here. test_codec first and then sound.

> I think this might be because usually in a graph each node containing
> ports is a device, such as a display panel, a bridge, an LCD
> controller. These kind of form a dependency chain.
>
> In this case all the ports in multi act as a way to glue multiple
> ports together for the audio-graph-card2.
>
> Does that help?

Maybe. But the logs would be more helpful.

>
> > But the better fix would be to use the new "post-init-providers"
> > property. See below.
> >
> > >
> > > / {
> > >         ...
> > >
> > >
> > >         test_codec {
> > >                 compatible = "test-codec";
> > >                 prefix = "Test codec";
> > >                 #sound-dai-cells = <0>;
> >
> > post-init-provider = <&multi>;

Did you try this? Did it help?

-Saravana

> >
> > Right now there's a cyclic dependency between test_codec and multi and
> > this tells the kernel that test codec needs to probe first.
> >
> > Similar additions to the other nodes blocked on multi.
> >
> > Thanks,
> > Saravana
>
> John.

Re: [PATCH] of: property: fw_devlink: Fix stupid bug in remote-endpoint parsing

[John Watts]

Hello there,

On Mon, Mar 25, 2024 at 03:49:44PM -0700, Saravana Kannan wrote:
> Ok, I think I understand now what's going on. fw_devlink does not know
> that "sound" device will not populate "multi" as a child device.
> Typically in such situations, "sound" would probe as a device and add
> its child DT nodes devices. At that point, the cycle is only between
> "multi" and "test_codec" and fw_devlink will detect that and not
> enforce any ordering. However, in this case, "sound" doesn't have any
> child devices and just depends on the remote endpoints directly.
> 
> We already have "ports", "in-ports" and "out-ports". Is there a reason
> none of them will work for your use case and it has to be "multi"?
> When you use one of those 3 recognized node names, things are handled
> correctly.

audio-graph-card2 uses 'multi' to define DAI links that have multiple
endpoints. It also suports codec2codec and dpcm.

> I think the right fix is the use of post-init-providers. Because even
> if you do the above, all it does is let fw_devlink see that there's a
> cyclic dependency in DT. And it'll stop enforcing the probe and
> suspend/resume ordering. Ideally we want to enforce a specific order
> here. test_codec first and then sound.

Is there a way to do this automatically so all the existing audio-graph-card2
device trees aren't broken? As it stands it seems like this driver is now
broken due to this change.

> Maybe. But the logs would be more helpful.

If you have a way for me to get more logs please tell me.

> > > post-init-provider = <&multi>;
> 
> Did you try this? Did it help?
> 
> -Saravana

No I haven't tried this yet. I shall try it soon. But I wouldn't consider
this a useful fix as it requires upgrading existing device trees.

John.

Re: [PATCH] of: property: fw_devlink: Fix stupid bug in remote-endpoint parsing

[Saravana Kannan]

On Mon, Mar 25, 2024 at 5:42 PM John Watts <contact@jookia.org> wrote:
>
> Hello there,
>
> On Mon, Mar 25, 2024 at 03:49:44PM -0700, Saravana Kannan wrote:
> > Ok, I think I understand now what's going on. fw_devlink does not know
> > that "sound" device will not populate "multi" as a child device.
> > Typically in such situations, "sound" would probe as a device and add
> > its child DT nodes devices. At that point, the cycle is only between
> > "multi" and "test_codec" and fw_devlink will detect that and not
> > enforce any ordering. However, in this case, "sound" doesn't have any
> > child devices and just depends on the remote endpoints directly.
> >
> > We already have "ports", "in-ports" and "out-ports". Is there a reason
> > none of them will work for your use case and it has to be "multi"?
> > When you use one of those 3 recognized node names, things are handled
> > correctly.
>
> audio-graph-card2 uses 'multi' to define DAI links that have multiple
> endpoints. It also suports codec2codec and dpcm.
>
> > I think the right fix is the use of post-init-providers. Because even
> > if you do the above, all it does is let fw_devlink see that there's a
> > cyclic dependency in DT. And it'll stop enforcing the probe and
> > suspend/resume ordering. Ideally we want to enforce a specific order
> > here. test_codec first and then sound.
>
> Is there a way to do this automatically so all the existing audio-graph-card2
> device trees aren't broken? As it stands it seems like this driver is now
> broken due to this change.

Ok, I have a solution. Have the audio-graph-card2 find the fwnode of
"multi" and mark it as "not a device" by doing something like this in
the driver. That should help fw_devlink handle this correctly.

fwnode.flags |= FWNODE_FLAG_NOT_DEVICE;

>
> > Maybe. But the logs would be more helpful.
>
> If you have a way for me to get more logs please tell me.
>
> > > > post-init-provider = <&multi>;
> >
> > Did you try this? Did it help?
> >
> > -Saravana
>
> No I haven't tried this yet. I shall try it soon. But I wouldn't consider
> this a useful fix as it requires upgrading existing device trees.

Definitely do this though as a forward looking improvement. It'll help
make the suspend/resume more deterministic and will eventually let
things happen in an async manner.


-Saravana

Re: [PATCH] of: property: fw_devlink: Fix stupid bug in remote-endpoint parsing

[John Watts]

On Mon, Mar 25, 2024 at 06:35:45PM -0700, Saravana Kannan wrote:
> Ok, I have a solution. Have the audio-graph-card2 find the fwnode of
> "multi" and mark it as "not a device" by doing something like this in
> the driver. That should help fw_devlink handle this correctly.
> 
> fwnode.flags |= FWNODE_FLAG_NOT_DEVICE;

I'll test this out, thanks.

> Definitely do this though as a forward looking improvement. It'll help
> make the suspend/resume more deterministic and will eventually let
> things happen in an async manner.

Is there a way to also do this in the driver?

> -Saravana

John.

Re: [PATCH] of: property: fw_devlink: Fix stupid bug in remote-endpoint parsing

[John Watts]

Hello again,

On Mon, Mar 25, 2024 at 06:35:45PM -0700, Saravana Kannan wrote:
> Ok, I have a solution. Have the audio-graph-card2 find the fwnode of
> "multi" and mark it as "not a device" by doing something like this in
> the driver. That should help fw_devlink handle this correctly.
> 
> fwnode.flags |= FWNODE_FLAG_NOT_DEVICE;

I have done this:


	struct device_node *node = dev->of_node;
	struct device_node *node2;
	node2 = of_get_child_by_name(node, "multi");
	printk("node2 %pOF %pfw %x\n", node2, node2->fwnode, node2->fwnode.flags);
	node2->fwnode.flags |= FWNODE_FLAG_NOT_DEVICE;
	of_node_put(node2);

This doesn't do anything, but this does:

	struct device_node *node = dev->of_node;
	struct device_node *node2;
	node2 = of_get_child_by_name(node, "multi");
	fw_devlink_purge_absent_suppliers(&node2->fwnode);
	printk("node2 %pOF %pfw %x\n", node2, node2->fwnode, node2->fwnode.flags);
	of_node_put(node2);

Should I be using fw_devlink_purge_absent_suppliers?

> -Saravana

John.