linux-kernel - RE: [PATCH v18 10/10] ARM: dts: Add Sunplus SP7021-Demo-V3 board device tree

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]

Date:   Wed, 15 Jun 2022 03:31:50 +0000
From:   qinjian[覃健] <qinjian@...lus1.com>
To:     Arnd Bergmann <arnd@...db.de>
CC:     Stephen Boyd <sboyd@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Rob Herring <robh+dt@...nel.org>,
        Michael Turquette <mturquette@...libre.com>,
        Russell King - ARM Linux <linux@...linux.org.uk>,
        Olof Johansson <olof@...om.net>, SoC Team <soc@...nel.org>,
        Linux ARM <linux-arm-kernel@...ts.infradead.org>,
        DTML <devicetree@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        linux-clk <linux-clk@...r.kernel.org>
Subject: RE: [PATCH v18 10/10] ARM: dts: Add Sunplus SP7021-Demo-V3 board
 device tree

> 
> I'll wait for a final Ack from Krzysztof before applying this.
> 
> 
> A few more details I noticed:
> 
> 
> > +/ {
> > +       compatible = "sunplus,sp7021";
> > +       model = "Sunplus SP7021";
> > +
> > +       aliases {
> > +               serial0 = &uart0;
> > +               serial1 = &uart1;
> > +               serial2 = &uart2;
> > +               serial3 = &uart3;
> > +               serial4 = &uart4;
> > +       };
> 
> Some of these are disabled, presumably because they are not
> actually connected on all boards. Better move the aliases to the .dts file
> and only list the ports that are in fact used, using the numbering that
> matches the labels on the board, not the numbers inside of the SoC
> 
> > +
> > +       soc {
> > +               compatible = "simple-bus";
> > +               #address-cells = <1>;
> > +               #size-cells = <1>;
> > +               ranges;
> > +               interrupt-parent = <&intc>;
> > +
> > +               intc: interrupt-controller@...00780 {
> > +                       compatible = "sunplus,sp7021-intc";
> > +                       reg = <0x9c000780 0x80>, <0x9c000a80 0x80>;
> > +                       interrupt-controller;
> > +                       #interrupt-cells = <2>;
> > +               };
> 
> All child nodes below /soc have registers in the 0x9c000000 range. I would
> suggest you use a matching 'ranges' property to translate these into a
> zero-based
> address like
> 
> /soc {
>        ranges = <0 0x9c000000 0x10000>;
> 
>       interrupt-controller@780 {
>              reg = <0x780 0x80>, <0xa80 0x80>;
>              ...
>       };
> };
> 
> > +               clkc: clock-controller@...00004 {
> > +                       compatible = "sunplus,sp7021-clkc";
> > +                       reg = <0x9c000004 0x28>,
> > +                             <0x9c000200 0x44>,
> > +                             <0x9c000268 0x04>;
> > +                       clocks = <&extclk>;
> > +                       #clock-cells = <1>;
> > +               };
> > +
> > +               rstc: reset@...00054 {
> > +                       compatible = "sunplus,sp7021-reset";
> > +                       reg = <0x9c000054 0x28>;
> > +                       #reset-cells = <1>;
> > +               };
> 
> 
> Maybe sort the nodes by address, or possibly by name.
> 
>        Arnd

Thanks for your comments, I'll fix these in next patch.