From: Rob Herring <robh@kernel.org>
To: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Cc: Thierry Reding <thierry.reding@gmail.com>,
Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt>,
sam@ravnborg.org, airlied@linux.ie, daniel@ffwll.ch,
krzysztof.kozlowski+dt@linaro.org, jonathanh@nvidia.com,
arnd@arndb.de, dri-devel@lists.freedesktop.org,
devicetree@vger.kernel.org, linux-tegra@vger.kernel.org
Subject: Re: [PATCH 4/4] arm64: dts: smaug: Add display panel node
Date: Fri, 30 Sep 2022 16:14:24 -0500 [thread overview]
Message-ID: <20220930211424.GA913368-robh@kernel.org> (raw)
In-Reply-To: <a0254559-a76d-de87-3458-e7dc148a8daf@linaro.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
WARNING: multiple messages have this Message-ID (diff)
From: Rob Herring <robh@kernel.org>
To: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Cc: devicetree@vger.kernel.org, arnd@arndb.de, airlied@linux.ie,
dri-devel@lists.freedesktop.org, jonathanh@nvidia.com,
Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt>,
Thierry Reding <thierry.reding@gmail.com>,
krzysztof.kozlowski+dt@linaro.org, linux-tegra@vger.kernel.org,
sam@ravnborg.org
Subject: Re: [PATCH 4/4] arm64: dts: smaug: Add display panel node
Date: Fri, 30 Sep 2022 16:14:24 -0500 [thread overview]
Message-ID: <20220930211424.GA913368-robh@kernel.org> (raw)
In-Reply-To: <a0254559-a76d-de87-3458-e7dc148a8daf@linaro.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
next prev parent reply other threads:[~2022-09-30 21:14 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-09-29 17:04 [PATCH 0/4] Add JDI LPM102A188A display panel support Diogo Ivo
2022-09-29 17:04 ` Diogo Ivo
2022-09-29 17:04 ` [PATCH 1/4] dt-bindings: display: Add bindings for JDI LPM102A188A Diogo Ivo
2022-09-29 17:04 ` Diogo Ivo
2022-09-30 10:49 ` Krzysztof Kozlowski
2022-09-30 10:49 ` Krzysztof Kozlowski
2022-10-03 17:06 ` Diogo Ivo
2022-10-03 17:06 ` Diogo Ivo
2022-10-04 11:05 ` Krzysztof Kozlowski
2022-10-04 11:05 ` Krzysztof Kozlowski
2022-10-04 16:37 ` Diogo Ivo
2022-10-04 16:37 ` Diogo Ivo
2022-09-29 17:05 ` [PATCH 2/4] drm/tegra: dsi: Clear enable register if powered by bootloader Diogo Ivo
2022-09-29 17:05 ` Diogo Ivo
2022-09-30 11:11 ` Thierry Reding
2022-09-30 11:11 ` Thierry Reding
2022-10-03 18:13 ` Diogo Ivo
2022-10-03 18:13 ` Diogo Ivo
2022-09-29 17:05 ` [PATCH 3/4] drm/panel: Add driver for JDI LPM102A188A Diogo Ivo
2022-09-29 17:05 ` Diogo Ivo
2022-09-29 17:05 ` [PATCH 4/4] arm64: dts: smaug: Add display panel node Diogo Ivo
2022-09-29 17:05 ` Diogo Ivo
2022-09-30 10:51 ` Krzysztof Kozlowski
2022-09-30 10:51 ` Krzysztof Kozlowski
2022-09-30 11:15 ` Thierry Reding
2022-09-30 11:15 ` Thierry Reding
2022-09-30 11:20 ` Krzysztof Kozlowski
2022-09-30 11:20 ` Krzysztof Kozlowski
2022-09-30 21:14 ` Rob Herring [this message]
2022-09-30 21:14 ` Rob Herring
2022-10-01 9:53 ` Krzysztof Kozlowski
2022-10-01 9:53 ` Krzysztof Kozlowski
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20220930211424.GA913368-robh@kernel.org \
--to=robh@kernel.org \
--cc=airlied@linux.ie \
--cc=arnd@arndb.de \
--cc=daniel@ffwll.ch \
--cc=devicetree@vger.kernel.org \
--cc=diogo.ivo@tecnico.ulisboa.pt \
--cc=dri-devel@lists.freedesktop.org \
--cc=jonathanh@nvidia.com \
--cc=krzysztof.kozlowski+dt@linaro.org \
--cc=krzysztof.kozlowski@linaro.org \
--cc=linux-tegra@vger.kernel.org \
--cc=sam@ravnborg.org \
--cc=thierry.reding@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.