The dtbs check throws a warning about node naming with the recently
added pf5-display-overlay:
rockchip/rk3568-wolfvision-pf5-display.dtsi:113.6-121.3: Warning (graph_port): /fragment@4/__overlay__: graph port node name should be 'port'

This comes from the overlay just referencing the vp2-port-node via
its phandle and then adding an endpoint beneath it.

While this is possible something to handle inside the dtbs check,
carrying around the warning is not pretty, so change the description
to go around it.

Starting from the vop_out phandle and then referencing the port
via its generic port@2 nodename will satisfy the port<->endpoint
naming dependency while keeping the same structure once the overlay
is applied.

Reported-by: Arnd Bergmann <arnd@kernel.org>
Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 .../rockchip/rk3568-wolfvision-pf5-display.dtsi    | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3568-wolfvision-pf5-display.dtsi b/arch/arm64/boot/dts/rockchip/rk3568-wolfvision-pf5-display.dtsi
index b22bb543ecbb..18c807c39e56 100644
--- a/arch/arm64/boot/dts/rockchip/rk3568-wolfvision-pf5-display.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3568-wolfvision-pf5-display.dtsi
@@ -110,12 +110,14 @@ &pwm10 {
 	status = "okay";
 };
 
-&vp2 {
-	#address-cells = <1>;
-	#size-cells = <0>;
+&vop_out {
+	port@2 {
+		#address-cells = <1>;
+		#size-cells = <0>;
 
-	vp2_out_rgb: endpoint@ROCKCHIP_VOP2_EP_RGB0 {
-		reg = <ROCKCHIP_VOP2_EP_RGB0>;
-		remote-endpoint = <&panel_in_vp2>;
+		vp2_out_rgb: endpoint@ROCKCHIP_VOP2_EP_RGB0 {
+			reg = <ROCKCHIP_VOP2_EP_RGB0>;
+			remote-endpoint = <&panel_in_vp2>;
+		};
 	};
 };
-- 
2.39.2

Hi Heiko,

First of all, thanks a lot for doing this!

On 4/23/24 10:29, Heiko Stuebner wrote:
> The dtbs check throws a warning about node naming with the recently
> added pf5-display-overlay:
> rockchip/rk3568-wolfvision-pf5-display.dtsi:113.6-121.3: Warning (graph_port): /fragment@4/__overlay__: graph port node name should be 'port'
> 
> This comes from the overlay just referencing the vp2-port-node via
> its phandle and then adding an endpoint beneath it.
> 
> While this is possible something to handle inside the dtbs check,
> carrying around the warning is not pretty, so change the description
> to go around it.

What is the rationale behind that check? Describing a port in a SoC dtsi
or board dts and using the reference in an overlay is quite convenient
and above all concise.

Cc: device tree list
> Starting from the vop_out phandle and then referencing the port
> via its generic port@2 nodename will satisfy the port<->endpoint
> naming dependency while keeping the same structure once the overlay
> is applied.
> 
> Reported-by: Arnd Bergmann <arnd@kernel.org>
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> ---
>  .../rockchip/rk3568-wolfvision-pf5-display.dtsi    | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3568-wolfvision-pf5-display.dtsi b/arch/arm64/boot/dts/rockchip/rk3568-wolfvision-pf5-display.dtsi
> index b22bb543ecbb..18c807c39e56 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3568-wolfvision-pf5-display.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3568-wolfvision-pf5-display.dtsi
> @@ -110,12 +110,14 @@ &pwm10 {
>  	status = "okay";
>  };
>  
> -&vp2 {
> -	#address-cells = <1>;
> -	#size-cells = <0>;
> +&vop_out {
> +	port@2 {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
>  
> -	vp2_out_rgb: endpoint@ROCKCHIP_VOP2_EP_RGB0 {
> -		reg = <ROCKCHIP_VOP2_EP_RGB0>;
> -		remote-endpoint = <&panel_in_vp2>;
> +		vp2_out_rgb: endpoint@ROCKCHIP_VOP2_EP_RGB0 {
> +			reg = <ROCKCHIP_VOP2_EP_RGB0>;
> +			remote-endpoint = <&panel_in_vp2>;
> +		};
>  	};
>  };

With this patch applied the DTC warning "Warning (graph_port):
/fragment@4/__overlay__: graph port node name should be 'port'"
vanishes, but a different DTC warning "Warning (unit_address_vs_reg):
/fragment@4/__overlay__/port@2: node has a unit name, but no reg or
ranges property" appears. Can you reproduce this?

I tried to fix that by adding the reg property, but then DTC complained
about "Warning (graph_port): /fragment@9/__overlay__/ports/port@0: graph
node '#size-cells' is -1, must be 0"

Then, I added the #size-cells property to avoid this. However, DTC
complained about this property not being necessary as there is only one
port. I stopped at this point.

I would say the real question is how this hardware should look like in
the device tree (overlay). Then, the compiler and/or build scripts can
be adjusted to tolerate this.

Thanks and best regards,
Michael

Am Dienstag, 23. April 2024, 11:39:26 CEST schrieb Michael Riesch:
> On 4/23/24 10:29, Heiko Stuebner wrote:
> > The dtbs check throws a warning about node naming with the recently
> > added pf5-display-overlay:
> > rockchip/rk3568-wolfvision-pf5-display.dtsi:113.6-121.3: Warning (graph_port): /fragment@4/__overlay__: graph port node name should be 'port'
> > 
> > This comes from the overlay just referencing the vp2-port-node via
> > its phandle and then adding an endpoint beneath it.
> > 
> > While this is possible something to handle inside the dtbs check,
> > carrying around the warning is not pretty, so change the description
> > to go around it.
> 
> What is the rationale behind that check? Describing a port in a SoC dtsi
> or board dts and using the reference in an overlay is quite convenient
> and above all concise.

I guess this is mainly a problem of the overlay thing.
Also it does not change with or without the "-@" option to dtc.

So I guess the main thing seems to be that overlays and the whole
checks don't seem to be well-trodden paths yet.


> Cc: device tree list
> > Starting from the vop_out phandle and then referencing the port
> > via its generic port@2 nodename will satisfy the port<->endpoint
> > naming dependency while keeping the same structure once the overlay
> > is applied.
> > 
> > Reported-by: Arnd Bergmann <arnd@kernel.org>
> > Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> > ---
> >  .../rockchip/rk3568-wolfvision-pf5-display.dtsi    | 14 ++++++++------
> >  1 file changed, 8 insertions(+), 6 deletions(-)
> > 
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3568-wolfvision-pf5-display.dtsi b/arch/arm64/boot/dts/rockchip/rk3568-wolfvision-pf5-display.dtsi
> > index b22bb543ecbb..18c807c39e56 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3568-wolfvision-pf5-display.dtsi
> > +++ b/arch/arm64/boot/dts/rockchip/rk3568-wolfvision-pf5-display.dtsi
> > @@ -110,12 +110,14 @@ &pwm10 {
> >  	status = "okay";
> >  };
> >  
> > -&vp2 {
> > -	#address-cells = <1>;
> > -	#size-cells = <0>;
> > +&vop_out {
> > +	port@2 {
> > +		#address-cells = <1>;
> > +		#size-cells = <0>;
> >  
> > -	vp2_out_rgb: endpoint@ROCKCHIP_VOP2_EP_RGB0 {
> > -		reg = <ROCKCHIP_VOP2_EP_RGB0>;
> > -		remote-endpoint = <&panel_in_vp2>;
> > +		vp2_out_rgb: endpoint@ROCKCHIP_VOP2_EP_RGB0 {
> > +			reg = <ROCKCHIP_VOP2_EP_RGB0>;
> > +			remote-endpoint = <&panel_in_vp2>;
> > +		};
> >  	};
> >  };
> 
> With this patch applied the DTC warning "Warning (graph_port):
> /fragment@4/__overlay__: graph port node name should be 'port'"
> vanishes, but a different DTC warning "Warning (unit_address_vs_reg):
> /fragment@4/__overlay__/port@2: node has a unit name, but no reg or
> ranges property" appears. Can you reproduce this?
> 
> I tried to fix that by adding the reg property, but then DTC complained
> about "Warning (graph_port): /fragment@9/__overlay__/ports/port@0: graph
> node '#size-cells' is -1, must be 0"
> 
> Then, I added the #size-cells property to avoid this. However, DTC
> complained about this property not being necessary as there is only one
> port. I stopped at this point.
> 
> I would say the real question is how this hardware should look like in
> the device tree (overlay). Then, the compiler and/or build scripts can
> be adjusted to tolerate this.

When I checked, my "workaround" the check was silent, but I guess I
need to update the schema python module again and would get that
rabbit hole you went down.

But yes definitly. Especially with those follow up problems you
encountered, this makes it a problem to solve in the checker.

So the original layout is the best one to represent the endpoint and
I guess with the parent node being named "__overlay__" it should be
somewhat ok for the checker going "nah, it'll be fine - we're an overlay"