Re: [PATCH 3/3] ARM: dts: clps711x: Add basic Cirrus Logic CDB89712 Development board

Alexander Shiyan
2016-05-13T12:30:23Z

Hello.

> ???????, 13 ??? 2016, 15:00 +03:00 ?? Arnd Bergmann <arnd@arndb.de>:
>
> On Friday 13 May 2016 14:26:28 Alexander Shiyan wrote:
...
> > + soc {
> > + #address-cells = <1>;
> > + #size-cells = <1>;
> > + compatible = "simple-bus";
> > + interrupt-parent = <&intc>;
> > + ranges;
>
> All child devices seem to be in the 0x80000000-0x90000000
> range, maybe set the ranges property accordingly?

I do not quite understand that you want to change here.

>
> > + clks: clks at 80000000 {
> > + #clock-cells = <1>;
> > + compatible = "cirrus,clps711x-clk";
> > + reg = <0x80000000 0xc000>;
> > + startup-frequency = <73728000>;
> > + };
...
> > + intc: intc at 80000000 {
> > + compatible = "cirrus,clps711x-intc";
> > + reg = <0x80000000 0x4000>;
> > + interrupt-controller;
> > + #interrupt-cells = <1>;
> > + };
>
> Better make the register ranges non-overlapping. This appears
> to start at the same place as the 'clks' node.

CLK driver uses:
#define CLPS711X_SYSCON1 (0x0100)
#define CLPS711X_SYSCON2 (0x1100)
...
#define CLPS711X_PLLR (0xa5a8)

IRQCHIP driver uses:
#define CLPS711X_INTSR1 (0x0240)
...
#define CLPS711X_INTSR2 (0x1240)
...
#define CLPS711X_INTMR3 (0x2280)

So there is no way to do any else.

> > + porta: gpio at 80000000 {
> > + compatible = "cirrus,clps711x-gpio";
> > + reg = <0x80000000 0x1 0x80000040 0x1>;
> > + gpio-controller;
> > + #gpio-cells = <2>;
> > + };
> > +
> > + portb: gpio at 80000001 {
> > + compatible = "cirrus,clps711x-gpio";
> > + reg = <0x80000001 0x1 0x80000041 0x1>;
> > + gpio-controller;
> > + #gpio-cells = <2>;
> > + };
> > +
> > + portc: gpio at 80000002 {
> > + compatible = "cirrus,clps711x-gpio";
> > + reg = <0x80000002 0x1 0x80000042 0x1>;
> > + gpio-controller;
> > + #gpio-cells = <2>;
> > + status = "disabled";
> > + };
> > +
> > + portd: gpio at 80000003 {
> > + compatible = "cirrus,clps711x-gpio";
> > + reg = <0x80000003 0x1 0x80000043 0x1>;
> > + gpio-controller;
> > + #gpio-cells = <2>;
> > + };
>
> These are all just single bytes. My guess is that there is
> actually one IP block that contains all the gpios, not
> four identical blocks.

These is a 8-bit registers with different properties, which parses in the
GPIO driver depending of alias (GPIO count, inverted logic etc.).

Thanks.

---