[PATCH 1/3] ARM: CSR: Adding CSR SiRFprimaII board support

Barry Song 21cnbao at gmail.com
Wed Jul 6 08:22:57 EDT 2011


Hi Arnd,
thanks.

2011/7/6 Arnd Bergmann <arnd at arndb.de>:
> On Wednesday 06 July 2011, Barry Song wrote:
>> From: Binghua Duan <binghua.duan at csr.com>
>>
>> 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 <Binghua.Duan at csr.com>
>> Signed-off-by: Rongjun Ying <Rongjun.Ying at csr.com>
>> Signed-off-by: Zhiwu Song <Zhiwu.Song at csr.com>
>> Signed-off-by: Yuping Luo <Yuping.Luo at csr.com>
>> Signed-off-by: Bin Shi <Bin.Shi at csr.com>
>> Signed-off-by: Huayi Li <Huayi.Li at csr.com>
>> Signed-off-by: Barry Song <Baohua.Song at csr.com>
>
> Reviewed-by: Arnd Bergmann <arnd at arndb.de>
>
> 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 at 0x80040000 {
>> +                             compatible = "arm,pl310-cache";
>> +                             reg = <0x80040000 0x1000>;
>> +                             interrupts = <59>;
>> +                     };
>> +
>> +                     intc: interrupt-controller at 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 at 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.

ok.

>
> 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 at 0x88000000 {
>                               compatible = "sirf,prima2-clkc";
>                               reg = <0 0x1000>;
>                        }
>
>                        reset-controller at 0x88010000 {
>                               compatible = "sirf,prima2-rstc";
>                               reg = <0x10000 0x1000>;
>                        };
>                }
>        }

i am not sure whether it make us a little more difficult to know the
real address at first glance.we will need to calculate.
all addresses are 1:1 mapped in this chip. bus map can work even
though we only give "ranges;" without real "ranges = <0x....>;".

i also find i missed grant's comment about deleting "0x" after "@" in
this patch.

>
>
>> +                     disp-iobg {
>> +                             compatible = "simple-bus";
>> +                             #address-cells = <1>;
>> +                             #size-cells = <1>;
>> +                             ranges = <0x90010000 0x90010000 0x30000>;
>> +
>> +                             display at 0x90010000 {
>> +                                     compatible = "sirf,prima2-lcd";
>> +                                     reg = <0x90010000 0x20000>;
>> +                                     interrupts = <30>;
>> +                             };
>> +
>> +                             vpp at 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 at 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.

GPU is powervr sgx 531, so could we define compatible as "powervr,sgx531"?

>
>> +                     multimedia-iobg {
>> +                             compatible = "simple-bus";
>> +                             #address-cells = <1>;
>> +                             #size-cells = <1>;
>> +                             ranges = <0xa0000000 0xa0000000 0x8000000>;
>> +
>> +                             multimedia at 0xa0000000 {
>> +                                     compatible = "sirf,prima2-multimedia";
>> +                                     reg = <0xa0000000 0x8000000>;
>> +                                     interrupts = <5>;
>> +                             };
>> +                     };
>
> "multimedia" sounds like a too generic term. What does this do?

 video decoding.

>
>> +                             uart0: uart at 0xb0050000 {
>> +                                     cell-index = <0>;
>> +                                     compatible = "sirf,prima2-uart";
>> +                                     reg = <0xb0050000 0x10000>;
>> +                                     interrupts = <17>;
>> +                             };
>> +
>> +                             uart1: uart at 0xb0060000 {
>> +                                     cell-index = <1>;
>> +                                     compatible = "sirf,prima2-uart";
>> +                                     reg = <0xb0060000 0x10000>;
>> +                                     interrupts = <18>;
>> +                             };
>> +
>> +                             uart2: uart at 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.

it is not compatible with 8250 .

>
>> +                     rtc-iobg {
>> +                             compatible = "sirf,prima2-rtciobg", "simple-bus";
>> +                             #address-cells = <1>;
>> +                             #size-cells = <1>;
>> +                             reg = <0x80030000 0x10000>;
>> +
>> +                             gpsrtc at 0x1000 {
>> +                                     compatible = "sirf,prima2-gpsrtc";
>> +                                     reg = <0x1000 0x1000>;
>> +                                     interrupts = <55 56 57>;
>> +                             };
>> +
>> +                             sysrtc at 0x2000 {
>> +                                     compatible = "sirf,prima2-sysrtc";
>> +                                     reg = <0x2000 0x1000>;
>> +                                     interrupts = <52 53 54>;
>> +                             };
>> +
>> +                             pwrc at 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.

in fact, because they are slow, they can't be accessed by mapped
address directly, the only common point they have is we need to access
them through mapped address in rtc-iobg indirectly just like we access
i2c/spi/nand devices.

they are three different devices with different purpose and register
layout in fact.

>
>> +                     uus-iobg {
>> +                             compatible = "simple-bus";
>> +                             #address-cells = <1>;
>> +                             #size-cells = <1>;
>> +                             ranges = <0xb8000000 0xb8000000 0x40000>;
>> +
>> +                             usb0: usb at 0xb00E0000 {
>> +                                     compatible = "sirf,prima2-usb";
>> +                                     reg = <0xb8000000 0x10000>;
>> +                                     interrupts = <10>;
>> +                             };
>> +
>> +                             usb1: usb at 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 at 0xb00f0000 {
>> +                                     compatible = "sirf,prima2-sata";
>> +                                     reg = <0xb8020000 0x10000>;
>> +                                     interrupts = <37>;
>> +                             };
>
> Same thing here. Most sata controllers are compatible to some
> standard implementation.

ok. i see :-). let me have some check with ic guys and send v4 with these fixes.

>
>> +                             security at 0xb00f0000 {
>> +                                     compatible = "sirf,prima2-security";
>> +                                     reg = <0xb8030000 0x10000>;
>> +                                     interrupts = <42>;
>> +                             };
>> +                     };
>> +             };
>> +     };
>> +};
>
>        Arnd
> _______________________________________________
> devicetree-discuss mailing list
> devicetree-discuss at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/devicetree-discuss
>
Thanks
barry


More information about the linux-arm-kernel mailing list