Re: [PATCH] arm64: dts: rockchip: fix nodename warning on wolfvision-pf5-display

From: "Heiko Stübner" <heiko@sntech.de>
To: Michael Riesch <michael.riesch@wolfvision.net>
Cc: linux-arm-kernel@lists.infradead.org,
	linux-rockchip@lists.infradead.org, linux-kernel@vger.kernel.org,
	Arnd Bergmann <arnd@kernel.org>,
	devicetree <devicetree@vger.kernel.org>
Subject: Re: [PATCH] arm64: dts: rockchip: fix nodename warning on wolfvision-pf5-display
Date: Tue, 23 Apr 2024 13:03:12 +0200	[thread overview]
Message-ID: <5189813.GXAFRqVoOG@diego> (raw)
In-Reply-To: <3f93ecb0-a649-4492-8798-a00de26236c8@wolfvision.net>

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"

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip