Re: [PATCH 12/23] arm: dts: k3-am642-r5-sk: Use am642-sk.dts and k3-am642-sk-u-boot.dtsi

From: Manorit Chawdhry <m-chawdhry@ti.com>
To: Nishanth Menon <nm@ti.com>
Cc: Simon Glass <sjg@chromium.org>, Roger Quadros <rogerq@kernel.org>,
	"Vignesh Raghavendra" <vigneshr@ti.com>,
	Tom Rini <trini@konsulko.com>,
	Neil Armstrong <neil.armstrong@linaro.org>,
	Anand Gadiyar <gadiyar@ti.com>, Georgi Vlaev <g-vlaev@ti.com>,
	<u-boot@lists.denx.de>, Neha Malcom Francis <n-francis@ti.com>,
	Nikhil M Jain <n-jain1@ti.com>
Subject: Re: [PATCH 12/23] arm: dts: k3-am642-r5-sk: Use am642-sk.dts and k3-am642-sk-u-boot.dtsi
Date: Wed, 19 Apr 2023 10:23:50 +0530	[thread overview]
Message-ID: <20230419045350.3xzojrsqi5ionwyt@ula0497581> (raw)
In-Reply-To: <20230418123455.7xrp6vv4pjjid6or@culminate>

On 07:34-20230418, Nishanth Menon wrote:
> On 14:45-20230418, Manorit Chawdhry wrote:
> > Hi Nishanth,
> > 
> > On 14/04/23 13:27, Nishanth Menon wrote:
> > > Use am642-sk.dts to represent the Board and selectively enable/override
> > > the configurations necessary. And since am642-sk-u-boot.dtsi also needs
> > > to setup common properties, instead of re-inventing everything, just
> > > reuse the definitions.
> > > 
> > > Signed-off-by: Nishanth Menon <nm@ti.com>
> > > ---
> > >   arch/arm/dts/k3-am642-r5-sk.dts | 194 +++-----------------------------
> > >   1 file changed, 18 insertions(+), 176 deletions(-)
> > > 
> > > diff --git a/arch/arm/dts/k3-am642-r5-sk.dts b/arch/arm/dts/k3-am642-r5-sk.dts
> > > index 9ff4dd3dd365..8ac6cc77256c 100644
> > > --- a/arch/arm/dts/k3-am642-r5-sk.dts
> > > +++ b/arch/arm/dts/k3-am642-r5-sk.dts
> > > @@ -5,31 +5,18 @@
> > >   /dts-v1/;
> > > -#include <dt-bindings/mux/ti-serdes.h>
> > > -#include <dt-bindings/phy/phy.h>
> > > -#include <dt-bindings/net/ti-dp83867.h>
> > > -#include "k3-am642.dtsi"
> > > +#include "k3-am642-sk.dts"
> > >   #include "k3-am64-sk-lp4-1600MTs.dtsi"
> > >   #include "k3-am64-ddr.dtsi"
> > > -/ {
> > > -	chosen {
> > > -		stdout-path = "serial2:115200n8";
> > > -		tick-timer = &timer1;
> > > -	};
> > > +#include "k3-am642-sk-u-boot.dtsi"
> > would changing the location of the include affect the dt nodes somehow?
> 
> yes, ofcourse, depends on the overlap ofcourse, in this series, I have tried to
> ensure the overlap occurs only where necessary.
> board.dts:
> &node {
> 	property-x = <0>;
> };
> 
> board-u-boot.dts:
> &node {
> 	property-x = <1>;
> };
> 
> board-r5.dts:
> &node {
> 	property-x = <2>;
> };
> 
> Then:
> case a) u-boot.dtsi included in r5.dts at the very end will make the
> property 1! - but then what was the point in overriding node?
> 
> case b) u-boot.dtsi included early in r5.dts will make the property 2 -
> which was intended when we introduced the override.
> 
> Are you seeing a specific problem in this series?
> 

Ah no, I was just worried since you had moved this specific include from
bottom of the file to top so was thinking if it could change the order
of overrides, but looks like there are no common nodes as such in
u-boot.dtsi and r5-sk.dtsi to change the override order. LGTM.

[override check for u-boot.dtsi and r5-sk.dtsi]
Reviewed-by: Manorit Chawdhry <m-chawdhry@ti.com>

Regards,
Manorit

> -- 
> Regards,
> Nishanth Menon
> Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D