From mboxrd@z Thu Jan 1 00:00:00 1970 From: arnd@arndb.de (Arnd Bergmann) Date: Wed, 6 Jul 2011 15:44:20 +0200 Subject: [PATCH 1/3] ARM: CSR: Adding CSR SiRFprimaII board support In-Reply-To: References: <1309945678-18813-1-git-send-email-bs14@csr.com> <201107061341.38591.arnd@arndb.de> Message-ID: <201107061544.20859.arnd@arndb.de> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org 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 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....>;". 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 at 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 at 88000000 { compatible = "sirf,prima2-clkc"; reg = <0 0x1000>; } reset-controller at 88010000 { compatible = "sirf,prima2-rstc"; reg = <0x10000 0x1000>; }; } } > >> + > >> + 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"? 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 at 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 From mboxrd@z Thu Jan 1 00:00:00 1970 From: Arnd Bergmann Subject: Re: [PATCH 1/3] ARM: CSR: Adding CSR SiRFprimaII board support Date: Wed, 6 Jul 2011 15:44:20 +0200 Message-ID: <201107061544.20859.arnd@arndb.de> References: <1309945678-18813-1-git-send-email-bs14@csr.com> <201107061341.38591.arnd@arndb.de> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: devicetree-discuss-bounces+gldd-devicetree-discuss=m.gmane.org-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org Sender: devicetree-discuss-bounces+gldd-devicetree-discuss=m.gmane.org-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org To: Barry Song <21cnbao-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> Cc: linux-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org, Barry Song , Bin Shi , devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org, workgroup.linux-kQvG35nSl+M@public.gmane.org, Zhiwu Song , Rongjun Ying , Binghua Duan , Barry Song , tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org, Yuping Luo , Huayi Li , linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org List-Id: devicetree@vger.kernel.org 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