On Wednesday 06 July 2011, Barry Song wrote: > From: Binghua Duan <[email protected]> > > SiRFprimaII is the latest generation application processor from CSR’s > Multifunction SoC product family. Designed around an ARM cortex A9 core, > high-speed memory bus, advanced 3D accelerator and full-HD multi-format > video decoder, SiRFprimaII is able to meet the needs of complicated > applications for modern multifunction devices that require heavy concurrent > applications and fluid user experience. Integrated with GPS baseband, > analog and PMU, this new platform is designed to provide a cost effective > solution for Automotive and Consumer markets. > > This patch adds the basic support for this SoC and EVB board based on device > tree. It is following the ZYNQ of Grant Likely in some degree. > > Signed-off-by: Binghua Duan <[email protected]> > Signed-off-by: Rongjun Ying <[email protected]> > Signed-off-by: Zhiwu Song <[email protected]> > Signed-off-by: Yuping Luo <[email protected]> > Signed-off-by: Bin Shi <[email protected]> > Signed-off-by: Huayi Li <[email protected]> > Signed-off-by: Barry Song <[email protected]>
Reviewed-by: Arnd Bergmann <[email protected]> I think this is good for 3.1, but there are still a few things about the device tree file could be improved. > + axi { > + compatible = "simple-bus"; > + #address-cells = <1>; > + #size-cells = <1>; > + ranges = <0x40000000 0x40000000 0x80000000>; > + > + sirfsoc-iobus { > + compatible = "simple-bus"; > + #address-cells = <1>; > + #size-cells = <1>; > + ranges = <0x40000000 0x40000000 0x80000000>; > + > + l2-cache-controller@0x80040000 { > + compatible = "arm,pl310-cache"; > + reg = <0x80040000 0x1000>; > + interrupts = <59>; > + }; > + > + intc: interrupt-controller@0x80020000 { > + #interrupt-cells = <1>; > + interrupt-controller; > + compatible = "sirf,prima2-intc"; > + reg = <0x80020000 0x1000>; > + }; > + > + sys-iobg { > + compatible = "simple-bus"; > + #address-cells = <1>; > + #size-cells = <1>; > + ranges = <0x88000000 0x88000000 0x40000>; > + > + clock-controller@0x88000000 { > + compatible = "sirf,prima2-clkc"; > + reg = <0x88000000 0x1000>; > + interrupts = <3>; > + }; The axi bus and the sirfsoc-iobus seem to be identical in their scope, so it's probably enough to model one of them. I would normally recommend defining the ranges so that addresses are local to the respective bus, like axi { ranges = <0 0x40000000 0x80000000>; sys-iobg { ranges = <0 0x48000000 0x40000>; clock-controller@0x88000000 { compatible = "sirf,prima2-clkc"; reg = <0 0x1000>; } reset-controller@0x88010000 { compatible = "sirf,prima2-rstc"; reg = <0x10000 0x1000>; }; } } > + disp-iobg { > + compatible = "simple-bus"; > + #address-cells = <1>; > + #size-cells = <1>; > + ranges = <0x90010000 0x90010000 0x30000>; > + > + display@0x90010000 { > + compatible = "sirf,prima2-lcd"; > + reg = <0x90010000 0x20000>; > + interrupts = <30>; > + }; > + > + vpp@0x90020000 { > + compatible = "sirf,prima2-vpp"; > + reg = <0x90020000 0x10000>; > + interrupts = <31>; > + }; > + }; > + > + graphics-iobg { > + compatible = "simple-bus"; > + #address-cells = <1>; > + #size-cells = <1>; > + ranges = <0x98000000 0x98000000 0x8000000>; > + > + graphics@0x98000000 { > + compatible = "sirf,prima2-graphics"; > + reg = <0x98000000 0x8000000>; > + interrupts = <6>; > + }; > + }; Are the display and graphics units CSR developments? If the GPU is in fact licensed from someone else (powervr, arm, ...), you should probably list the actual name of the device. > + multimedia-iobg { > + compatible = "simple-bus"; > + #address-cells = <1>; > + #size-cells = <1>; > + ranges = <0xa0000000 0xa0000000 0x8000000>; > + > + multimedia@0xa0000000 { > + compatible = "sirf,prima2-multimedia"; > + reg = <0xa0000000 0x8000000>; > + interrupts = <5>; > + }; > + }; "multimedia" sounds like a too generic term. What does this do? > + uart0: uart@0xb0050000 { > + cell-index = <0>; > + compatible = "sirf,prima2-uart"; > + reg = <0xb0050000 0x10000>; > + interrupts = <17>; > + }; > + > + uart1: uart@0xb0060000 { > + cell-index = <1>; > + compatible = "sirf,prima2-uart"; > + reg = <0xb0060000 0x10000>; > + interrupts = <18>; > + }; > + > + uart2: uart@0xb0070000 { > + cell-index = <2>; > + compatible = "sirf,prima2-uart"; > + reg = <0xb0070000 0x10000>; > + interrupts = <19>; > + }; Are these proprietary uarts, or are they compatible to 8250 and the like? You might want to set a clock-frequency property as of_serial.c uses. > + rtc-iobg { > + compatible = "sirf,prima2-rtciobg", > "simple-bus"; > + #address-cells = <1>; > + #size-cells = <1>; > + reg = <0x80030000 0x10000>; > + > + gpsrtc@0x1000 { > + compatible = "sirf,prima2-gpsrtc"; > + reg = <0x1000 0x1000>; > + interrupts = <55 56 57>; > + }; > + > + sysrtc@0x2000 { > + compatible = "sirf,prima2-sysrtc"; > + reg = <0x2000 0x1000>; > + interrupts = <52 53 54>; > + }; > + > + pwrc@0x3000 { > + compatible = "sirf,prima2-pwrc"; > + reg = <0x3000 0x1000>; > + interrupts = <32>; > + }; > + }; Are these rtc implementations related? From the register layout, I would guess that they are supposed to be used by the same driver, so it's probably a good idea to add a "compatible" property with a common name for all three. > + uus-iobg { > + compatible = "simple-bus"; > + #address-cells = <1>; > + #size-cells = <1>; > + ranges = <0xb8000000 0xb8000000 0x40000>; > + > + usb0: usb@0xb00E0000 { > + compatible = "sirf,prima2-usb"; > + reg = <0xb8000000 0x10000>; > + interrupts = <10>; > + }; > + > + usb1: usb@0xb00f0000 { > + compatible = "sirf,prima2-usb"; > + reg = <0xb8010000 0x10000>; > + interrupts = <11>; > + }; Is the usb implementation compatible to an existing one? Many SoCs use one of ehci, ohci or musb. If that's the case, you should look at the respective bindings. > + sata@0xb00f0000 { > + compatible = "sirf,prima2-sata"; > + reg = <0xb8020000 0x10000>; > + interrupts = <37>; > + }; Same thing here. Most sata controllers are compatible to some standard implementation. > + security@0xb00f0000 { > + compatible = "sirf,prima2-security"; > + reg = <0xb8030000 0x10000>; > + interrupts = <42>; > + }; > + }; > + }; > + }; > +}; Arnd _______________________________________________ devicetree-discuss mailing list [email protected] https://lists.ozlabs.org/listinfo/devicetree-discuss
