[PATCH] checks: relax graph checks for overlays

[PATCH] checks: relax graph checks for overlays

[Michael Riesch]

In device tree overlays, the following patterns occur frequently:

board.dts:
/dts-v1/;

/ {
	display-controller {
		ports {
			#address-cells = <1>;
			#size-cells = <0>;

			vp0: port@0 {
				reg = <0>;

				vp0_out: endpoint {
				};
			};

			vp1: port@1 {
				reg = <1>;
			};
		};
	};
};

overlay-endpoint.dtso:
/dts-v1/;
/plugin/;

&{/} {
	hdmi-tx-connector {
		port {
			hdmi_tx_in: endpoint {
				remote-endpoint = <&vp0_out>;
			};
		};
	};
};

&vp0_out {
	remote-endpoint = <&hdmi_tx_in>;
};

In this case, dtc expects that the node referenced by &vp0_out is
named "endpoint", but the name cannot be inferred. Also, dtc
complains about the connections between the endpoints not being
bidirectional.

Similarly, for a different overlay overlay-port.dtso:
/dts-v1/;
/plugin/;

&{/} {
	panel {
		port {
			panel_in: endpoint {
				remote-endpoint = <&vp1_out>;
			};
		};
	};
};

&vp1 {
	vp1_out: endpoint {
		remote-endpoint = <&panel_in>;
	};
};

dtc expects that the node referenced by &vp1 is named "port", but the
name cannot be inferred.

Relax the corresponding checks and skip the parts that are not reasonable
for device tree overlays.

Cc: Heiko Stuebner <heiko@sntech.de>
Signed-off-by: Michael Riesch <michael.riesch@wolfvision.net>
---
Habidere,

This patch fixes several warnings 
 - Warning (graph_port): /fragment@3: graph port node name should be
   'port'
 - Warning (graph_endpoint): /fragment@3/__overlay__: graph endpoint
   node name should be 'endpoint'
 - Warning (graph_endpoint): /fragment@3/__overlay__: graph connection
   to node '/fragment@2/__overlay__/.../port/endpoint' is not
   bidirectional
that appear when compiling device tree overlays.

This first warning popped up e.g., in the scope of [1], [2]. The latter
two warnings appear regularly when compiling our downstream overlays. As
we plan to submit them to mainline soon, we'll hit that blocker sooner or
later.

Looking forward to your comments!

Link: https://lore.kernel.org/lkml/20240412-feature-wolfvision-pf5-display-v1-1-f032f32dba1a@wolfvision.net/ [1]
Link: https://lore.kernel.org/lkml/20240423082941.2626102-1-heiko@sntech.de/T/ [2]
---
 checks.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/checks.c b/checks.c
index 2fb7ee5..2ea5913 100644
--- a/checks.c
+++ b/checks.c
@@ -1822,10 +1822,14 @@ static void check_graph_port(struct check *c, struct dt_info *dti,
 	if (node->bus != &graph_port_bus)
 		return;
 
+	check_graph_reg(c, dti, node);
+
+	/* skip checks below for overlays */
+	if (dti->dtsflags & DTSF_PLUGIN)
+		return;
+
 	if (!strprefixeq(node->name, node->basenamelen, "port"))
 		FAIL(c, dti, node, "graph port node name should be 'port'");
-
-	check_graph_reg(c, dti, node);
 }
 WARNING(graph_port, check_graph_port, NULL, &graph_nodes);
 
@@ -1860,11 +1864,15 @@ static void check_graph_endpoint(struct check *c, struct dt_info *dti,
 	if (!node->parent || node->parent->bus != &graph_port_bus)
 		return;
 
+	check_graph_reg(c, dti, node);
+
+	/* skip checks below for overlays */
+	if (dti->dtsflags & DTSF_PLUGIN)
+		return;
+
 	if (!strprefixeq(node->name, node->basenamelen, "endpoint"))
 		FAIL(c, dti, node, "graph endpoint node name should be 'endpoint'");
 
-	check_graph_reg(c, dti, node);
-
 	remote_node = get_remote_endpoint(c, dti, node);
 	if (!remote_node)
 		return;

---
base-commit: ae26223a056e040b2d812202283d47c6e034d063
change-id: 20240522-bugfix-relax-checks-for-overlays-e72b11df44a1

Best regards,
-- 
Michael Riesch <michael.riesch@wolfvision.net>

Re: [PATCH] checks: relax graph checks for overlays

[Michael Riesch]

Hi all,

This is a gentle ping with David and the DT maintainers in Cc: (I am not
sure what the protocol is here, so I'll simply give this a try).

On 5/22/24 16:32, Michael Riesch wrote:
> In device tree overlays, the following patterns occur frequently:
> 
> board.dts:
> /dts-v1/;
> 
> / {
> 	display-controller {
> 		ports {
> 			#address-cells = <1>;
> 			#size-cells = <0>;
> 
> 			vp0: port@0 {
> 				reg = <0>;
> 
> 				vp0_out: endpoint {
> 				};
> 			};
> 
> 			vp1: port@1 {
> 				reg = <1>;
> 			};
> 		};
> 	};
> };
> 
> overlay-endpoint.dtso:
> /dts-v1/;
> /plugin/;
> 
> &{/} {
> 	hdmi-tx-connector {
> 		port {
> 			hdmi_tx_in: endpoint {
> 				remote-endpoint = <&vp0_out>;
> 			};
> 		};
> 	};
> };
> 
> &vp0_out {
> 	remote-endpoint = <&hdmi_tx_in>;
> };
> 
> In this case, dtc expects that the node referenced by &vp0_out is
> named "endpoint", but the name cannot be inferred. Also, dtc
> complains about the connections between the endpoints not being
> bidirectional.
> 
> Similarly, for a different overlay overlay-port.dtso:
> /dts-v1/;
> /plugin/;
> 
> &{/} {
> 	panel {
> 		port {
> 			panel_in: endpoint {
> 				remote-endpoint = <&vp1_out>;
> 			};
> 		};
> 	};
> };
> 
> &vp1 {
> 	vp1_out: endpoint {
> 		remote-endpoint = <&panel_in>;
> 	};
> };
> 
> dtc expects that the node referenced by &vp1 is named "port", but the
> name cannot be inferred.
> 
> Relax the corresponding checks and skip the parts that are not reasonable
> for device tree overlays.
> 
> Cc: Heiko Stuebner <heiko@sntech.de>
> Signed-off-by: Michael Riesch <michael.riesch@wolfvision.net>
> ---
> Habidere,
> 
> This patch fixes several warnings 
>  - Warning (graph_port): /fragment@3: graph port node name should be
>    'port'
>  - Warning (graph_endpoint): /fragment@3/__overlay__: graph endpoint
>    node name should be 'endpoint'
>  - Warning (graph_endpoint): /fragment@3/__overlay__: graph connection
>    to node '/fragment@2/__overlay__/.../port/endpoint' is not
>    bidirectional
> that appear when compiling device tree overlays.
> 
> This first warning popped up e.g., in the scope of [1], [2]. The latter
> two warnings appear regularly when compiling our downstream overlays. As
> we plan to submit them to mainline soon, we'll hit that blocker sooner or
> later.
> 
> Looking forward to your comments!
> 
> Link: https://lore.kernel.org/lkml/20240412-feature-wolfvision-pf5-display-v1-1-f032f32dba1a@wolfvision.net/ [1]
> Link: https://lore.kernel.org/lkml/20240423082941.2626102-1-heiko@sntech.de/T/ [2]

Feedback on this issue would be most welcome!

Thanks a lot and best regards,
Michael

> ---
>  checks.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/checks.c b/checks.c
> index 2fb7ee5..2ea5913 100644
> --- a/checks.c
> +++ b/checks.c
> @@ -1822,10 +1822,14 @@ static void check_graph_port(struct check *c, struct dt_info *dti,
>  	if (node->bus != &graph_port_bus)
>  		return;
>  
> +	check_graph_reg(c, dti, node);
> +
> +	/* skip checks below for overlays */
> +	if (dti->dtsflags & DTSF_PLUGIN)
> +		return;
> +
>  	if (!strprefixeq(node->name, node->basenamelen, "port"))
>  		FAIL(c, dti, node, "graph port node name should be 'port'");
> -
> -	check_graph_reg(c, dti, node);
>  }
>  WARNING(graph_port, check_graph_port, NULL, &graph_nodes);
>  
> @@ -1860,11 +1864,15 @@ static void check_graph_endpoint(struct check *c, struct dt_info *dti,
>  	if (!node->parent || node->parent->bus != &graph_port_bus)
>  		return;
>  
> +	check_graph_reg(c, dti, node);
> +
> +	/* skip checks below for overlays */
> +	if (dti->dtsflags & DTSF_PLUGIN)
> +		return;
> +
>  	if (!strprefixeq(node->name, node->basenamelen, "endpoint"))
>  		FAIL(c, dti, node, "graph endpoint node name should be 'endpoint'");
>  
> -	check_graph_reg(c, dti, node);
> -
>  	remote_node = get_remote_endpoint(c, dti, node);
>  	if (!remote_node)
>  		return;
> 
> ---
> base-commit: ae26223a056e040b2d812202283d47c6e034d063
> change-id: 20240522-bugfix-relax-checks-for-overlays-e72b11df44a1
> 
> Best regards,

Re: [PATCH] checks: relax graph checks for overlays

[Heiko Stübner]

Hey dt and dtc people :-) ,

Am Dienstag, 4. Juni 2024, 16:48:46 CEST schrieb Michael Riesch:
> This is a gentle ping with David and the DT maintainers in Cc: (I am not
> sure what the protocol is here, so I'll simply give this a try).

it would be really great if someone could look at the patch and tell us
if that is the right direction to go, or something else needs to be done
instead.

Thanks a lot
Heiko

> On 5/22/24 16:32, Michael Riesch wrote:
> > In device tree overlays, the following patterns occur frequently:
> > 
> > board.dts:
> > /dts-v1/;
> > 
> > / {
> > 	display-controller {
> > 		ports {
> > 			#address-cells = <1>;
> > 			#size-cells = <0>;
> > 
> > 			vp0: port@0 {
> > 				reg = <0>;
> > 
> > 				vp0_out: endpoint {
> > 				};
> > 			};
> > 
> > 			vp1: port@1 {
> > 				reg = <1>;
> > 			};
> > 		};
> > 	};
> > };
> > 
> > overlay-endpoint.dtso:
> > /dts-v1/;
> > /plugin/;
> > 
> > &{/} {
> > 	hdmi-tx-connector {
> > 		port {
> > 			hdmi_tx_in: endpoint {
> > 				remote-endpoint = <&vp0_out>;
> > 			};
> > 		};
> > 	};
> > };
> > 
> > &vp0_out {
> > 	remote-endpoint = <&hdmi_tx_in>;
> > };
> > 
> > In this case, dtc expects that the node referenced by &vp0_out is
> > named "endpoint", but the name cannot be inferred. Also, dtc
> > complains about the connections between the endpoints not being
> > bidirectional.
> > 
> > Similarly, for a different overlay overlay-port.dtso:
> > /dts-v1/;
> > /plugin/;
> > 
> > &{/} {
> > 	panel {
> > 		port {
> > 			panel_in: endpoint {
> > 				remote-endpoint = <&vp1_out>;
> > 			};
> > 		};
> > 	};
> > };
> > 
> > &vp1 {
> > 	vp1_out: endpoint {
> > 		remote-endpoint = <&panel_in>;
> > 	};
> > };
> > 
> > dtc expects that the node referenced by &vp1 is named "port", but the
> > name cannot be inferred.
> > 
> > Relax the corresponding checks and skip the parts that are not reasonable
> > for device tree overlays.
> > 
> > Cc: Heiko Stuebner <heiko@sntech.de>
> > Signed-off-by: Michael Riesch <michael.riesch@wolfvision.net>
> > ---
> > Habidere,
> > 
> > This patch fixes several warnings 
> >  - Warning (graph_port): /fragment@3: graph port node name should be
> >    'port'
> >  - Warning (graph_endpoint): /fragment@3/__overlay__: graph endpoint
> >    node name should be 'endpoint'
> >  - Warning (graph_endpoint): /fragment@3/__overlay__: graph connection
> >    to node '/fragment@2/__overlay__/.../port/endpoint' is not
> >    bidirectional
> > that appear when compiling device tree overlays.
> > 
> > This first warning popped up e.g., in the scope of [1], [2]. The latter
> > two warnings appear regularly when compiling our downstream overlays. As
> > we plan to submit them to mainline soon, we'll hit that blocker sooner or
> > later.
> > 
> > Looking forward to your comments!
> > 
> > Link: https://lore.kernel.org/lkml/20240412-feature-wolfvision-pf5-display-v1-1-f032f32dba1a@wolfvision.net/ [1]
> > Link: https://lore.kernel.org/lkml/20240423082941.2626102-1-heiko@sntech.de/T/ [2]
> 
> Feedback on this issue would be most welcome!
> 
> Thanks a lot and best regards,
> Michael
> 
> > ---
> >  checks.c | 16 ++++++++++++----
> >  1 file changed, 12 insertions(+), 4 deletions(-)
> > 
> > diff --git a/checks.c b/checks.c
> > index 2fb7ee5..2ea5913 100644
> > --- a/checks.c
> > +++ b/checks.c
> > @@ -1822,10 +1822,14 @@ static void check_graph_port(struct check *c, struct dt_info *dti,
> >  	if (node->bus != &graph_port_bus)
> >  		return;
> >  
> > +	check_graph_reg(c, dti, node);
> > +
> > +	/* skip checks below for overlays */
> > +	if (dti->dtsflags & DTSF_PLUGIN)
> > +		return;
> > +
> >  	if (!strprefixeq(node->name, node->basenamelen, "port"))
> >  		FAIL(c, dti, node, "graph port node name should be 'port'");
> > -
> > -	check_graph_reg(c, dti, node);
> >  }
> >  WARNING(graph_port, check_graph_port, NULL, &graph_nodes);
> >  
> > @@ -1860,11 +1864,15 @@ static void check_graph_endpoint(struct check *c, struct dt_info *dti,
> >  	if (!node->parent || node->parent->bus != &graph_port_bus)
> >  		return;
> >  
> > +	check_graph_reg(c, dti, node);
> > +
> > +	/* skip checks below for overlays */
> > +	if (dti->dtsflags & DTSF_PLUGIN)
> > +		return;
> > +
> >  	if (!strprefixeq(node->name, node->basenamelen, "endpoint"))
> >  		FAIL(c, dti, node, "graph endpoint node name should be 'endpoint'");
> >  
> > -	check_graph_reg(c, dti, node);
> > -
> >  	remote_node = get_remote_endpoint(c, dti, node);
> >  	if (!remote_node)
> >  		return;
> > 
> > ---
> > base-commit: ae26223a056e040b2d812202283d47c6e034d063
> > change-id: 20240522-bugfix-relax-checks-for-overlays-e72b11df44a1
> > 
> > Best regards,
> 

Re: [PATCH] checks: relax graph checks for overlays

[Rob Herring]

On Tue, Jun 18, 2024 at 2:04 PM Heiko Stübner <heiko@sntech.de> wrote:
>
> Hey dt and dtc people :-) ,
>
> Am Dienstag, 4. Juni 2024, 16:48:46 CEST schrieb Michael Riesch:
> > This is a gentle ping with David and the DT maintainers in Cc: (I am not
> > sure what the protocol is here, so I'll simply give this a try).
>
> it would be really great if someone could look at the patch and tell us
> if that is the right direction to go, or something else needs to be done
> instead.

It is similar to other fixes in this area, so I guess it is the right
direction. I don't really like disabling checks, but don't have a
better suggestion.

Not much else I can do here, it's up to David to pick this up.

Rob

Re: [PATCH] checks: relax graph checks for overlays

[Michael Riesch]

Hi David,

On 6/25/24 22:15, Rob Herring wrote:
> On Tue, Jun 18, 2024 at 2:04 PM Heiko Stübner <heiko@sntech.de> wrote:
>>
>> Hey dt and dtc people :-) ,
>>
>> Am Dienstag, 4. Juni 2024, 16:48:46 CEST schrieb Michael Riesch:
>>> This is a gentle ping with David and the DT maintainers in Cc: (I am not
>>> sure what the protocol is here, so I'll simply give this a try).
>>
>> it would be really great if someone could look at the patch and tell us
>> if that is the right direction to go, or something else needs to be done
>> instead.
> 
> It is similar to other fixes in this area, so I guess it is the right
> direction. I don't really like disabling checks, but don't have a
> better suggestion.
> 
> Not much else I can do here, it's up to David to pick this up.
> 
> Rob

Any comments from your side? Or are you going to apply this anyway and I
should just be patient ;-)

BTW would you prefer a pull request on GitHub?

Thanks and regards,
Michael

Re: [PATCH] checks: relax graph checks for overlays

[Javier Carrasco]

On 22/05/2024 16:32, Michael Riesch wrote:
> In device tree overlays, the following patterns occur frequently:
> 
> board.dts:
> /dts-v1/;
> 
> / {
> 	display-controller {
> 		ports {
> 			#address-cells = <1>;
> 			#size-cells = <0>;
> 
> 			vp0: port@0 {
> 				reg = <0>;
> 
> 				vp0_out: endpoint {
> 				};
> 			};
> 
> 			vp1: port@1 {
> 				reg = <1>;
> 			};
> 		};
> 	};
> };
> 
> overlay-endpoint.dtso:
> /dts-v1/;
> /plugin/;
> 
> &{/} {
> 	hdmi-tx-connector {
> 		port {
> 			hdmi_tx_in: endpoint {
> 				remote-endpoint = <&vp0_out>;
> 			};
> 		};
> 	};
> };
> 
> &vp0_out {
> 	remote-endpoint = <&hdmi_tx_in>;
> };
> 
> In this case, dtc expects that the node referenced by &vp0_out is
> named "endpoint", but the name cannot be inferred. Also, dtc
> complains about the connections between the endpoints not being
> bidirectional.
> 
> Similarly, for a different overlay overlay-port.dtso:
> /dts-v1/;
> /plugin/;
> 
> &{/} {
> 	panel {
> 		port {
> 			panel_in: endpoint {
> 				remote-endpoint = <&vp1_out>;
> 			};
> 		};
> 	};
> };
> 
> &vp1 {
> 	vp1_out: endpoint {
> 		remote-endpoint = <&panel_in>;
> 	};
> };
> 
> dtc expects that the node referenced by &vp1 is named "port", but the
> name cannot be inferred.
> 
> Relax the corresponding checks and skip the parts that are not reasonable
> for device tree overlays.
> 
> Cc: Heiko Stuebner <heiko@sntech.de>
> Signed-off-by: Michael Riesch <michael.riesch@wolfvision.net>
> ---
> Habidere,
> 
> This patch fixes several warnings 
>  - Warning (graph_port): /fragment@3: graph port node name should be
>    'port'
>  - Warning (graph_endpoint): /fragment@3/__overlay__: graph endpoint
>    node name should be 'endpoint'
>  - Warning (graph_endpoint): /fragment@3/__overlay__: graph connection
>    to node '/fragment@2/__overlay__/.../port/endpoint' is not
>    bidirectional
> that appear when compiling device tree overlays.
> 
> This first warning popped up e.g., in the scope of [1], [2]. The latter
> two warnings appear regularly when compiling our downstream overlays. As
> we plan to submit them to mainline soon, we'll hit that blocker sooner or
> later.
> 
> Looking forward to your comments!
> 
> Link: https://lore.kernel.org/lkml/20240412-feature-wolfvision-pf5-display-v1-1-f032f32dba1a@wolfvision.net/ [1]
> Link: https://lore.kernel.org/lkml/20240423082941.2626102-1-heiko@sntech.de/T/ [2]
> ---
>  checks.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/checks.c b/checks.c
> index 2fb7ee5..2ea5913 100644
> --- a/checks.c
> +++ b/checks.c
> @@ -1822,10 +1822,14 @@ static void check_graph_port(struct check *c, struct dt_info *dti,
>  	if (node->bus != &graph_port_bus)
>  		return;
>  
> +	check_graph_reg(c, dti, node);
> +
> +	/* skip checks below for overlays */
> +	if (dti->dtsflags & DTSF_PLUGIN)
> +		return;
> +
>  	if (!strprefixeq(node->name, node->basenamelen, "port"))
>  		FAIL(c, dti, node, "graph port node name should be 'port'");
> -
> -	check_graph_reg(c, dti, node);
>  }
>  WARNING(graph_port, check_graph_port, NULL, &graph_nodes);
>  
> @@ -1860,11 +1864,15 @@ static void check_graph_endpoint(struct check *c, struct dt_info *dti,
>  	if (!node->parent || node->parent->bus != &graph_port_bus)
>  		return;
>  
> +	check_graph_reg(c, dti, node);
> +
> +	/* skip checks below for overlays */
> +	if (dti->dtsflags & DTSF_PLUGIN)
> +		return;
> +
>  	if (!strprefixeq(node->name, node->basenamelen, "endpoint"))
>  		FAIL(c, dti, node, "graph endpoint node name should be 'endpoint'");
>  
> -	check_graph_reg(c, dti, node);
> -
>  	remote_node = get_remote_endpoint(c, dti, node);
>  	if (!remote_node)
>  		return;
> 
> ---
> base-commit: ae26223a056e040b2d812202283d47c6e034d063
> change-id: 20240522-bugfix-relax-checks-for-overlays-e72b11df44a1
> 
> Best regards,


Not sure if that might be a valid solution, but including "rk356x.dtsi"
(where the nodes referenced by the phandles reside) removes those warnings.

Best regards,
Javier Carrasco