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

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

[Arnd Bergmann]

On Wednesday 06 July 2011, Barry Song wrote:
> From: Binghua Duan <binghua.duan@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@csr.com>
> Signed-off-by: Rongjun Ying <Rongjun.Ying@csr.com>
> Signed-off-by: Zhiwu Song <Zhiwu.Song@csr.com>
> Signed-off-by: Yuping Luo <Yuping.Luo@csr.com>
> Signed-off-by: Bin Shi <Bin.Shi@csr.com>
> Signed-off-by: Huayi Li <Huayi.Li@csr.com>
> Signed-off-by: Barry Song <Baohua.Song@csr.com>

Reviewed-by: Arnd Bergmann <arnd@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@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
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

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

[Barry Song]

Hi Arnd,
thanks.

2011/7/6 Arnd Bergmann <arnd@arndb.de>:
> On Wednesday 06 July 2011, Barry Song wrote:
>> From: Binghua Duan <binghua.duan@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@csr.com>
>> Signed-off-by: Rongjun Ying <Rongjun.Ying@csr.com>
>> Signed-off-by: Zhiwu Song <Zhiwu.Song@csr.com>
>> Signed-off-by: Yuping Luo <Yuping.Luo@csr.com>
>> Signed-off-by: Bin Shi <Bin.Shi@csr.com>
>> Signed-off-by: Huayi Li <Huayi.Li@csr.com>
>> Signed-off-by: Barry Song <Baohua.Song@csr.com>
>
> Reviewed-by: Arnd Bergmann <arnd@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@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.

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

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

it is not compatible with 8250 .

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

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

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

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

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

[Arnd Bergmann]

On Wednesday 06 July 2011, Barry Song wrote:

> > 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>;
> >                        };
> >                }
> >        }
> 
> 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....>;".

So each iobg still passes down the entire 32-bit address?

Note that you never have to do the calculation in the driver
source, of_iomap and the resource logic both take care of this.

There are multiple ways to handle this, and an empty ranges property
usually works fine, but I find that less readable.

Another way to handle these is to have a separate range for
each child bus, as in arch/powerpc/boot/dts/gef_ppc9a.dts

To stay in the example, this would mean doing something like

        axi {
		#address-cells = <2>;
                #size-cells = <1>;

                ranges = <0 0 0x80000000 0x08000000 // axi devices
			  1 0 0x88000000 0x08000000 // sys-iobg
			  2 0 0x90000000 0x00010000 // mem-iobg
			  3 0 0x90010000 0x07fe0000 // disp-iobg
                          ... >;

                l2-cache-controller@80040000 {
                        compatible = "arm,pl310-cache";
                        reg = <0 0x40000 0x1000>;
                        interrupts = <59>;
                };

                sys-iobg {
			#address-cells = <1>;
        	        #size-cells = <1>;
                        ranges = <1 0 0 0x40000>;
                        clock-controller@88000000 {
                               compatible = "sirf,prima2-clkc";
                               reg = <0 0x1000>;
                        }

                        reset-controller@88010000 {
                               compatible = "sirf,prima2-rstc";
                               reg = <0x10000 0x1000>;
                        };
                }
        }



> >> +
> >> +                     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.
> 
> GPU is powervr sgx 531, so could we define compatible as "powervr,sgx531"?

Probably yes. You should have a look if there are already bindings for
this that define other attributes. Also, if there is any customization
inside of the chip, you should have another more specific identifier
that makes it possible that this is the version that csr has modified.

> >> +                     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?
> 
>  video decoding.

sirf,prima2-video-codec is probably better than, but if anyone has other
suggestions, you could use something else.

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

ok

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

Ok.

	Arnd

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

[Barry Song]

2011/7/6 Arnd Bergmann <arnd@arndb.de>:
> On Wednesday 06 July 2011, Barry Song wrote:
>
>> > 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>;
>> >                        };
>> >                }
>> >        }
>>
>> 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....>;".
>
> So each iobg still passes down the entire 32-bit address?

yes. each iobg is basically transparent for address transferring.
ranges = <0x40000000 0x40000000 0x80000000>; should be ok.

>
> Note that you never have to do the calculation in the driver
> source, of_iomap and the resource logic both take care of this.
>
> There are multiple ways to handle this, and an empty ranges property
> usually works fine, but I find that less readable.
>
> Another way to handle these is to have a separate range for
> each child bus, as in arch/powerpc/boot/dts/gef_ppc9a.dts
>
> To stay in the example, this would mean doing something like
>
>        axi {
>                #address-cells = <2>;
>                #size-cells = <1>;
>
>                ranges = <0 0 0x80000000 0x08000000 // axi devices
>                          1 0 0x88000000 0x08000000 // sys-iobg
>                          2 0 0x90000000 0x00010000 // mem-iobg
>                          3 0 0x90010000 0x07fe0000 // disp-iobg
>                          ... >;
>
>                l2-cache-controller@80040000 {
>                        compatible = "arm,pl310-cache";
>                        reg = <0 0x40000 0x1000>;
>                        interrupts = <59>;
>                };
>
>                sys-iobg {
>                        #address-cells = <1>;
>                        #size-cells = <1>;
>                        ranges = <1 0 0 0x40000>;
>                        clock-controller@88000000 {
>                               compatible = "sirf,prima2-clkc";
>                               reg = <0 0x1000>;
>                        }
>
>                        reset-controller@88010000 {
>                               compatible = "sirf,prima2-rstc";
>                               reg = <0x10000 0x1000>;
>                        };
>                }
>        }
>
>
>
>> >> +
>> >> +                     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.
>>
>> GPU is powervr sgx 531, so could we define compatible as "powervr,sgx531"?
>
> Probably yes. You should have a look if there are already bindings for
> this that define other attributes. Also, if there is any customization
> inside of the chip, you should have another more specific identifier
> that makes it possible that this is the version that csr has modified.
>
>> >> +                     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?
>>
>>  video decoding.
>
> sirf,prima2-video-codec is probably better than, but if anyone has other
> suggestions, you could use something else.
>
>> > 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 .
>
> ok
>
>> > 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.
>
> Ok.
>
>        Arnd
>
_______________________________________________
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss