Re: [PATCH 4/4] arm64: dts: smaug: Add display panel node

[Rob Herring <robh@kernel.org>]

On Fri, Sep 30, 2022 at 01:20:50PM +0200, Krzysztof Kozlowski wrote:
> On 30/09/2022 13:15, Thierry Reding wrote:
> > On Fri, Sep 30, 2022 at 12:51:07PM +0200, Krzysztof Kozlowski wrote:
> >> On 29/09/2022 19:05, Diogo Ivo wrote:
> >>> The Google Pixel C has a JDI LPM102A188A display panel. Add a
> >>> DT node for it. Tested on Pixel C.
> >>>
> >>> Signed-off-by: Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt>
> >>> ---
> >>>  arch/arm64/boot/dts/nvidia/tegra210-smaug.dts | 72 +++++++++++++++++++
> >>>  1 file changed, 72 insertions(+)
> >>>
> >>> diff --git a/arch/arm64/boot/dts/nvidia/tegra210-smaug.dts b/arch/arm64/boot/dts/nvidia/tegra210-smaug.dts
> >>> index 20d092812984..271ef70747f1 100644
> >>> --- a/arch/arm64/boot/dts/nvidia/tegra210-smaug.dts
> >>> +++ b/arch/arm64/boot/dts/nvidia/tegra210-smaug.dts
> >>> @@ -31,6 +31,39 @@ memory {
> >>>  	};
> >>>  
> >>>  	host1x@50000000 {
> >>> +		dc@54200000 {
> >>> +			status = "okay";
> >>
> >> You should override by labels, not by full path.
> > 
> > Why exactly is that? I've always stayed away from that (and asked others
> > not to do so, at least on Tegra) because I find it impossible to parse
> > for my human brain. Replicating the original full hierarchy makes it
> > much more obvious to me where the changes are happening than the
> > spaghetti-like mess that you get from overriding by label reference.
> 
> Sure, it's entirely up to you. I forgot your preference.
> 
> But it is a really nice way to have duplicated nodes and mistakes (which
> happen from time to time).

We could have a schema or dtc check for that. We already warn for 
duplicate unit-addresses which would catch some typos. Checking for a 
node with only 'status' would probably work when that's the only 
addition. Maybe status without a compatible would be better? We also 
check for nodes without a specific schema, but child nodes in schemas 
aren't handled.

Rob