linux-kernel - Re: [PATCH] docs: dt-bindings: add DTS Coding Style document

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAMuHMdWgzasx-8X6TJJE1f5Y8wi+5Tv7pTEFvJVPGGaY2pEfeQ@mail.gmail.com>
Date:   Sat, 18 Nov 2023 12:25:37 +0100
From:   Geert Uytterhoeven <geert@...ux-m68k.org>
To:     Heiko Stübner <heiko@...ech.de>
Cc:     Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Conor Dooley <conor+dt@...nel.org>, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org,
        Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
        Andrew Davis <afd@...com>,
        AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@...labora.com>,
        Arnd Bergmann <arnd@...db.de>,
        Bjorn Andersson <andersson@...nel.org>,
        Geert Uytterhoeven <geert+renesas@...der.be>,
        Konrad Dybcio <konrad.dybcio@...aro.org>,
        Matthias Brugger <matthias.bgg@...il.com>,
        Michal Simek <michal.simek@....com>,
        Neil Armstrong <neil.armstrong@...aro.org>,
        Nishanth Menon <nm@...com>, Olof Johansson <olof@...om.net>
Subject: Re: [PATCH] docs: dt-bindings: add DTS Coding Style document

Hi Heiko,

On Fri, Nov 17, 2023 at 8:38 PM Heiko Stübner <heiko@...ech.de> wrote:
> Am Freitag, 17. November 2023, 15:03:38 CET schrieb Andrew Davis:
> > On 11/16/23 2:33 PM, Heiko Stuebner wrote:
> > > Am Donnerstag, 16. November 2023, 21:23:20 CET schrieb Krzysztof Kozlowski:
> > >> On 16/11/2023 21:03, Heiko Stuebner wrote:
> > > going with the vcc5v0_host regulator of the rk3588-quartzpro64 and
> > >
> > > +1. compatible
> > > +2. reg
> > > +3. ranges
> > > +4. All properties with values
> > > +5. Boolean properties
> > > +6. status (if applicable)
> > > +7. Child nodes
> > >
> > > we'd end up with
> > >
> > >          vcc5v0_host: vcc5v0-host-regulator {
> > > /* 1. */        compatible = "regulator-fixed";
> > > /* 4. */        gpio = <&gpio4 RK_PB0 GPIO_ACTIVE_HIGH>;
> > >                  pinctrl-names = "default";
> > >                  pinctrl-0 = <&vcc5v0_host_en>;
> > >                  regulator-min-microvolt = <5000000>;
> > >                  regulator-max-microvolt = <5000000>;
> > >                  regulator-name = "vcc5v0_host";
> > >                  vin-supply = <&vcc5v0_usb>;
> > > /* 5. */        enable-active-high;
> > >                  regulator-always-on;
> > >                  regulator-boot-on;
> > >          };
> > >
> >
> > How about grouping like properties (defined in the same schema),
> > then sorting within that group. Would also allow for defining
> > where to add spacing.
> >
> > 1. compatible
> > 2. reg
> > 3. ranges
> > 4. All property groups
> >    4.1 Properties with values
> >    4.2 Boolean properties
> >    4.3 Separating space
> > 6. status (if applicable)
> > 7. Child nodes
> >
> > Your node then would look like we expect:
> >
> > vcc5v0_host: vcc5v0-host-regulator {
> > /* 1   */   compatible = "regulator-fixed";
> >
> > /* 4.1 */   pinctrl-names = "default";
> > /* 4.1 */   pinctrl-0 = <&vcc5v0_host_en>;
> > /* 4.3 */
> > /* 4.1 */   regulator-min-microvolt = <5000000>;
> > /* 4.1 */   regulator-max-microvolt = <5000000>;
> > /* 4.1 */   regulator-name = "vcc5v0_host";
> > /* 4.2 */   regulator-always-on;
> > /* 4.2 */   regulator-boot-on;
> > /* 4.2 */   enable-active-high;
> > /* 4.3 */
> > /* 4.1 */   gpio = <&gpio4 RK_PB0 GPIO_ACTIVE_HIGH>;
> > ...
> > };
>
> I'm really not sure about adding big sets of rules.
> In the above example you'd also need to define which schema has a higher
> priority? ;-)
>
>
> When I started with Rockchip stuff, I also had some fancy way of sorting
> elements in mind that was really intuitive to myself :-) .
> Over time I realized that it was quite complex - especially when I had to
> explain it to people.

Feel familiar.

> There are definite advantages for having compatible + reg + status in
> fixed positions, as it helps going over a whole dt to spot the huge
> mistakes (accidentially disabled, wrong address), but for the rest a
> simple alphabetical sorting is easiest to explain to people :-) .
>
> And alphabetic elements are also easier on my eyes.

Alphabetical does break logical ordering and grouping.

Apart from compatible/reg order, on Renesas SoCs we usually
also have the clocks/resets/power-domains block.

> I just think having a short clean set of rules like Krzysztof proposed,
> is easier to follow and "enforce" and also most likely doesn't deter
> people from contributing, if mainline work is not their main occupation.

And in reality, most of this is created by copy-and-corrupt^Wmodify...

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-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