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

[qinjian[覃健]]

> 
> 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.