All of lore.kernel.org
 help / color / mirror / Atom feed
From: "qinjian[覃健]" <qinjian@cqplus1.com>
To: Arnd Bergmann <arnd@arndb.de>
Cc: Stephen Boyd <sboyd@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Rob Herring <robh+dt@kernel.org>,
	Michael Turquette <mturquette@baylibre.com>,
	Russell King - ARM Linux <linux@armlinux.org.uk>,
	Olof Johansson <olof@lixom.net>, SoC Team <soc@kernel.org>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>,
	DTML <devicetree@vger.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	linux-clk <linux-clk@vger.kernel.org>
Subject: RE: [PATCH v18 10/10] ARM: dts: Add Sunplus SP7021-Demo-V3 board device tree
Date: Wed, 15 Jun 2022 03:31:50 +0000	[thread overview]
Message-ID: <f19b479d70704784ae5fb00ca610fda3@cqplus1.com> (raw)
In-Reply-To: <CAK8P3a0XgDS-iJEfOrTjTRGODosxj0gc748PAH_hc534DkyswQ@mail.gmail.com>

> 
> 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@9c000780 {
> > +                       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@9c000004 {
> > +                       compatible = "sunplus,sp7021-clkc";
> > +                       reg = <0x9c000004 0x28>,
> > +                             <0x9c000200 0x44>,
> > +                             <0x9c000268 0x04>;
> > +                       clocks = <&extclk>;
> > +                       #clock-cells = <1>;
> > +               };
> > +
> > +               rstc: reset@9c000054 {
> > +                       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.


WARNING: multiple messages have this Message-ID (diff)
From: "qinjian[覃健]" <qinjian@cqplus1.com>
To: Arnd Bergmann <arnd@arndb.de>
Cc: Stephen Boyd <sboyd@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Rob Herring <robh+dt@kernel.org>,
	Michael Turquette <mturquette@baylibre.com>,
	Russell King - ARM Linux <linux@armlinux.org.uk>,
	Olof Johansson <olof@lixom.net>, SoC Team <soc@kernel.org>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>,
	DTML <devicetree@vger.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	linux-clk <linux-clk@vger.kernel.org>
Subject: RE: [PATCH v18 10/10] ARM: dts: Add Sunplus SP7021-Demo-V3 board device tree
Date: Wed, 15 Jun 2022 03:31:50 +0000	[thread overview]
Message-ID: <f19b479d70704784ae5fb00ca610fda3@cqplus1.com> (raw)
In-Reply-To: <CAK8P3a0XgDS-iJEfOrTjTRGODosxj0gc748PAH_hc534DkyswQ@mail.gmail.com>

> 
> 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@9c000780 {
> > +                       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@9c000004 {
> > +                       compatible = "sunplus,sp7021-clkc";
> > +                       reg = <0x9c000004 0x28>,
> > +                             <0x9c000200 0x44>,
> > +                             <0x9c000268 0x04>;
> > +                       clocks = <&extclk>;
> > +                       #clock-cells = <1>;
> > +               };
> > +
> > +               rstc: reset@9c000054 {
> > +                       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.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2022-06-15  3:40 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-14  8:30 [PATCH v18 00/10] Add Sunplus SP7021 SoC Support Qin Jian
2022-06-14  8:30 ` Qin Jian
2022-06-14  8:31 ` [PATCH v18 01/10] dt-bindings: arm: sunplus: Add bindings for Sunplus SP7021 SoC boards Qin Jian
2022-06-14  8:31   ` Qin Jian
2022-06-14  8:31 ` [PATCH v18 02/10] dt-bindings: reset: Add bindings for SP7021 reset driver Qin Jian
2022-06-14  8:31   ` Qin Jian
2022-06-14  8:31 ` [PATCH v18 03/10] reset: Add Sunplus " Qin Jian
2022-06-14  8:31   ` Qin Jian
2022-06-14 14:50   ` Randy Dunlap
2022-06-14 14:50     ` Randy Dunlap
2022-06-14  8:31 ` [PATCH v18 04/10] dt-bindings: clock: Add bindings for SP7021 clock driver Qin Jian
2022-06-14  8:31   ` Qin Jian
2022-06-14 19:26   ` Stephen Boyd
2022-06-14 19:26     ` Stephen Boyd
2022-06-14  8:31 ` [PATCH v18 05/10] clk: Add Sunplus " Qin Jian
2022-06-14  8:31   ` Qin Jian
2022-06-14 10:44   ` Arnd Bergmann
2022-06-14 10:44     ` Arnd Bergmann
2022-06-15  3:43     ` qinjian[覃健]
2022-06-15  3:43       ` qinjian[覃健]
2022-06-14 19:25   ` Stephen Boyd
2022-06-14 19:25     ` Stephen Boyd
2022-06-15  2:25     ` qinjian[覃健]
2022-06-15  2:25       ` qinjian[覃健]
2022-06-15 19:01       ` Stephen Boyd
2022-06-14  8:31 ` [PATCH v18 06/10] dt-bindings: interrupt-controller: Add bindings for SP7021 interrupt controller Qin Jian
2022-06-14  8:31   ` Qin Jian
2022-06-14  8:31 ` [PATCH v18 07/10] irqchip: Add Sunplus SP7021 interrupt controller driver Qin Jian
2022-06-14  8:31   ` Qin Jian
2022-06-14  8:31 ` [PATCH v18 08/10] ARM: sunplus: Add initial support for Sunplus SP7021 SoC Qin Jian
2022-06-14  8:31   ` Qin Jian
2022-06-14 14:51   ` Randy Dunlap
2022-06-14 14:51     ` Randy Dunlap
2022-06-14  8:31 ` [PATCH v18 09/10] ARM: sp7021_defconfig: Add Sunplus SP7021 defconfig Qin Jian
2022-06-14  8:31   ` Qin Jian
2022-06-14  8:31 ` [PATCH v18 10/10] ARM: dts: Add Sunplus SP7021-Demo-V3 board device tree Qin Jian
2022-06-14  8:31   ` Qin Jian
2022-06-14 10:42   ` Arnd Bergmann
2022-06-14 10:42     ` Arnd Bergmann
2022-06-15  3:31     ` qinjian[覃健] [this message]
2022-06-15  3:31       ` qinjian[覃健]
2022-06-15 14:02   ` Krzysztof Kozlowski
2022-06-15 14:02     ` Krzysztof Kozlowski
2022-06-19 14:12   ` Pavel Machek
2022-06-19 14:12     ` Pavel Machek

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=f19b479d70704784ae5fb00ca610fda3@cqplus1.com \
    --to=qinjian@cqplus1.com \
    --cc=arnd@arndb.de \
    --cc=devicetree@vger.kernel.org \
    --cc=krzysztof.kozlowski+dt@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-clk@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=mturquette@baylibre.com \
    --cc=olof@lixom.net \
    --cc=robh+dt@kernel.org \
    --cc=sboyd@kernel.org \
    --cc=soc@kernel.org \
    /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.