Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220427185243.173594-1-detlev.casanova@collabora.com>
 <20220427185243.173594-4-detlev.casanova@collabora.com> <YmmyvdjiG7s/Qil4@robh.at.kernel.org>
In-Reply-To: <YmmyvdjiG7s/Qil4@robh.at.kernel.org>
From:   Geert Uytterhoeven <geert@linux-m68k.org>
Date:   Thu, 28 Apr 2022 08:44:17 +0200
Message-ID: <CAMuHMdXHRQOOrmUO1AyDXye+nnRtpzx7WHiC__whcg0aBtzAmw@mail.gmail.com>
Subject: Re: [RFC PATCH v2 3/3] ARM: dto: Add bcm2711-rpi-7-inches-ts.dts overlay
To:     Rob Herring <robh@kernel.org>
Cc:     Detlev Casanova <detlev.casanova@collabora.com>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Arnd Bergmann <arnd@arndb.de>,
        bcm-kernel-feedback-list <bcm-kernel-feedback-list@broadcom.com>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@vger.kernel.org>,
        Florian Fainelli <f.fainelli@gmail.com>,
        Frank Rowand <frowand.list@gmail.com>,
        Linux ARM <linux-arm-kernel@lists.infradead.org>,
        linux-rpi-kernel <linux-rpi-kernel@lists.infradead.org>,
        Masahiro Yamada <masahiroy@kernel.org>,
        Michal Marek <michal.lkml@markovi.net>,
        Nick Desaulniers <ndesaulniers@google.com>,
        Nicolas Saenz Julienne <nsaenz@kernel.org>,
        Olof Johansson <olof@lixom.net>, Ray Jui <rjui@broadcom.com>,
        Scott Branden <sbranden@broadcom.com>,
        arm-soc <soc@kernel.org>, Stefan Wahren <stefan.wahren@i2se.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi Rob,

On Wed, Apr 27, 2022 at 11:23 PM Rob Herring <robh@kernel.org> wrote:
> On Wed, Apr 27, 2022 at 02:52:43PM -0400, Detlev Casanova wrote:
> > Add a device tree overlay to support the official Raspberrypi 7" touchscreen for
> > the bcm2711 devices.
> >
> > The panel is connected on the DSI 1 port and uses the simple-panel
> > driver.
> >
> > The device tree also makes sure to activate the pixelvalve[0-4] CRTC modules
> >
> > Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com>
> > ---
> >  arch/arm/boot/dts/Makefile                    |   4 +
> >  arch/arm/boot/dts/overlays/Makefile           |   3 +
> >  .../dts/overlays/bcm2711-rpi-7-inches-ts.dts  | 125 ++++++++++++++++++
>
> .dtso is preferred. I think... It was discussed, but I never got an
> updated patch to switch.

Unfortunately that switch indeed hasn't happened yet.
My main gripe with .dts for overlays is that you cannot know whether
it's an overlay or not without reading the file's contents.
Hence tools like make also cannot know, and you need to e.g. list
all files explicitly in a Makefile.

> >  arch/arm64/boot/dts/broadcom/Makefile         |   4 +
> >  .../arm64/boot/dts/broadcom/overlays/Makefile |   3 +
> >  .../overlays/bcm2711-rpi-7-inches-ts.dts      |   2 +
> >  6 files changed, 141 insertions(+)
> >  create mode 100644 arch/arm/boot/dts/overlays/Makefile
> >  create mode 100644 arch/arm/boot/dts/overlays/bcm2711-rpi-7-inches-ts.dts
>
> A global (to arm) 'overlays' directory will create the same mess that we
> have in arch/arm/boot/dts/. IMO, first you should move all the Broadcom
> dts files to a 'broadcom' subdirectory like we have for arm64.

As I believe this display is not only used with real Raspberry Pi
devices, it makes sense to not have it a broadcom directory.
In fact it may be used on other architectures than arm, too, so I
think we need an arch-agnostic directory for overlays[1]?
This may need remapping of labels. I'm aware the rpi infrastructure has
support for remapping labels when applying overlays during boot, but
AFAIK this is not yet supported by fdtoverlay (or perhaps by a fork?)?
Note that the remapping is also needed if you want to apply two
instances of the same overlay.

> >  create mode 100644 arch/arm64/boot/dts/broadcom/overlays/Makefile
> >  create mode 100644 arch/arm64/boot/dts/broadcom/overlays/bcm2711-rpi-7-inches-ts.dts

And this one just includes the former, and thus sort-of serves as an
example of my point above ;-)

[1] Note that this does not only apply to overlays: soon we will
    have a full SoC peripheral description in r9a07g043.dtsi to
    share between RZ/G2UL (arm64) and RZ/Five (riscv)).

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds