| Message ID | 1371569479-31498-1-git-send-email-ezequiel.garcia@free-electrons.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 06/18/2013 05:31 PM, Ezequiel Garcia wrote: > Although the internal register window size is 1 MiB, the previous > ranges translation for the internal register space had a size of > 0x4000000. This was done to allow the crypto and nand node to access > the corresponding 'sram' and 'nand' decoding windows. > > In order to describe the hardware more accurately, we declare the > real 1 MiB internal register space in the ranges, and add a translation > entry for the nand node to access the 'nand' window. > > This commit will make future improvements on the MBus DT binding easier. > > Signed-off-by: Ezequiel Garcia<ezequiel.garcia@free-electrons.com> > --- > Tested on Plathome Openblocks A6 board. > > arch/arm/boot/dts/kirkwood.dtsi | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/arch/arm/boot/dts/kirkwood.dtsi b/arch/arm/boot/dts/kirkwood.dtsi > index 8a1e3bb..910fabc 100644 > --- a/arch/arm/boot/dts/kirkwood.dtsi > +++ b/arch/arm/boot/dts/kirkwood.dtsi > @@ -38,7 +38,8 @@ > > ocp@f1000000 { > compatible = "simple-bus"; > - ranges =<0x00000000 0xf1000000 0x4000000 > + ranges =<0x00000000 0xf1000000 0x0100000 > + 0xf4000000 0xf4000000 0x0000400 > 0xf5000000 0xf5000000 0x0000400>; Ezequiel, maybe you should also put a comment at the end of each ranges line to name the remap it does? Also, as already mentioned on IRC, it would be great to also have dove.dtsi updated accordingly. Sebastian > #address-cells =<1>; > #size-cells =<1>; > @@ -171,7 +172,7 @@ > ale =<1>; > bank-width =<1>; > compatible = "marvell,orion-nand"; > - reg =<0x3000000 0x400>; > + reg =<0xf4000000 0x400>; > chip-delay =<25>; > /* set partition map and/or chip-delay in board dts */ > clocks =<&gate_clk 7>;
On Tue, Jun 18, 2013 at 09:42:48PM +0200, Sebastian Hesselbarth wrote: > On 06/18/2013 05:31 PM, Ezequiel Garcia wrote: > > Although the internal register window size is 1 MiB, the previous > > ranges translation for the internal register space had a size of > > 0x4000000. This was done to allow the crypto and nand node to access > > the corresponding 'sram' and 'nand' decoding windows. > > > > In order to describe the hardware more accurately, we declare the > > real 1 MiB internal register space in the ranges, and add a translation > > entry for the nand node to access the 'nand' window. > > > > This commit will make future improvements on the MBus DT binding easier. > > > > Signed-off-by: Ezequiel Garcia<ezequiel.garcia@free-electrons.com> > > --- > > Tested on Plathome Openblocks A6 board. > > > > arch/arm/boot/dts/kirkwood.dtsi | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/arch/arm/boot/dts/kirkwood.dtsi b/arch/arm/boot/dts/kirkwood.dtsi > > index 8a1e3bb..910fabc 100644 > > --- a/arch/arm/boot/dts/kirkwood.dtsi > > +++ b/arch/arm/boot/dts/kirkwood.dtsi > > @@ -38,7 +38,8 @@ > > > > ocp@f1000000 { > > compatible = "simple-bus"; > > - ranges =<0x00000000 0xf1000000 0x4000000 > > + ranges =<0x00000000 0xf1000000 0x0100000 > > + 0xf4000000 0xf4000000 0x0000400 > > 0xf5000000 0xf5000000 0x0000400>; > > Ezequiel, > > maybe you should also put a comment at the end of each ranges > line to name the remap it does? > > Also, as already mentioned on IRC, it would be great to also > have dove.dtsi updated accordingly. > Indeed, I will do both. Thanks!
On Tue, Jun 18, 2013 at 04:47:57PM -0300, Ezequiel Garcia wrote: > On Tue, Jun 18, 2013 at 09:42:48PM +0200, Sebastian Hesselbarth wrote: > > On 06/18/2013 05:31 PM, Ezequiel Garcia wrote: > > > Although the internal register window size is 1 MiB, the previous > > > ranges translation for the internal register space had a size of > > > 0x4000000. This was done to allow the crypto and nand node to access > > > the corresponding 'sram' and 'nand' decoding windows. > > > > > > In order to describe the hardware more accurately, we declare the > > > real 1 MiB internal register space in the ranges, and add a translation > > > entry for the nand node to access the 'nand' window. > > > > > > This commit will make future improvements on the MBus DT binding easier. > > > > > > Signed-off-by: Ezequiel Garcia<ezequiel.garcia@free-electrons.com> > > > --- > > > Tested on Plathome Openblocks A6 board. > > > > > > arch/arm/boot/dts/kirkwood.dtsi | 5 +++-- > > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > > > diff --git a/arch/arm/boot/dts/kirkwood.dtsi b/arch/arm/boot/dts/kirkwood.dtsi > > > index 8a1e3bb..910fabc 100644 > > > --- a/arch/arm/boot/dts/kirkwood.dtsi > > > +++ b/arch/arm/boot/dts/kirkwood.dtsi > > > @@ -38,7 +38,8 @@ > > > > > > ocp@f1000000 { > > > compatible = "simple-bus"; > > > - ranges =<0x00000000 0xf1000000 0x4000000 > > > + ranges =<0x00000000 0xf1000000 0x0100000 > > > + 0xf4000000 0xf4000000 0x0000400 > > > 0xf5000000 0xf5000000 0x0000400>; > > > > Ezequiel, > > > > maybe you should also put a comment at the end of each ranges > > line to name the remap it does? > > > > Also, as already mentioned on IRC, it would be great to also > > have dove.dtsi updated accordingly. > > > > Indeed, I will do both. The sooner, the better. This is probably the last patch to get in for this merge window for mvebu... thx, Jason.
On Wed, Jun 19, 2013 at 3:36 PM, Jason Cooper <jason@lakedaemon.net> wrote: > On Tue, Jun 18, 2013 at 04:47:57PM -0300, Ezequiel Garcia wrote: >> On Tue, Jun 18, 2013 at 09:42:48PM +0200, Sebastian Hesselbarth wrote: >> > On 06/18/2013 05:31 PM, Ezequiel Garcia wrote: >> > > Although the internal register window size is 1 MiB, the previous >> > > ranges translation for the internal register space had a size of >> > > 0x4000000. This was done to allow the crypto and nand node to access >> > > the corresponding 'sram' and 'nand' decoding windows. >> > > >> > > In order to describe the hardware more accurately, we declare the >> > > real 1 MiB internal register space in the ranges, and add a translation >> > > entry for the nand node to access the 'nand' window. >> > > >> > > This commit will make future improvements on the MBus DT binding easier. >> > > >> > > Signed-off-by: Ezequiel Garcia<ezequiel.garcia@free-electrons.com> >> > > --- >> > > Tested on Plathome Openblocks A6 board. >> > > >> > > arch/arm/boot/dts/kirkwood.dtsi | 5 +++-- >> > > 1 file changed, 3 insertions(+), 2 deletions(-) >> > > >> > > diff --git a/arch/arm/boot/dts/kirkwood.dtsi b/arch/arm/boot/dts/kirkwood.dtsi >> > > index 8a1e3bb..910fabc 100644 >> > > --- a/arch/arm/boot/dts/kirkwood.dtsi >> > > +++ b/arch/arm/boot/dts/kirkwood.dtsi >> > > @@ -38,7 +38,8 @@ >> > > >> > > ocp@f1000000 { >> > > compatible = "simple-bus"; >> > > - ranges =<0x00000000 0xf1000000 0x4000000 >> > > + ranges =<0x00000000 0xf1000000 0x0100000 >> > > + 0xf4000000 0xf4000000 0x0000400 >> > > 0xf5000000 0xf5000000 0x0000400>; >> > >> > Ezequiel, >> > >> > maybe you should also put a comment at the end of each ranges >> > line to name the remap it does? >> > >> > Also, as already mentioned on IRC, it would be great to also >> > have dove.dtsi updated accordingly. >> > >> >> Indeed, I will do both. > > The sooner, the better. This is probably the last patch to get in for > this merge window for mvebu... > Right. Please don't wait for this patch if it's not in your mailbox by tomorrow. Thanks for the notice!
On Wed, Jun 19, 2013 at 03:42:27PM -0300, Ezequiel Garcia wrote: > On Wed, Jun 19, 2013 at 3:36 PM, Jason Cooper <jason@lakedaemon.net> wrote: > > On Tue, Jun 18, 2013 at 04:47:57PM -0300, Ezequiel Garcia wrote: > >> On Tue, Jun 18, 2013 at 09:42:48PM +0200, Sebastian Hesselbarth wrote: > >> > On 06/18/2013 05:31 PM, Ezequiel Garcia wrote: > >> > > Although the internal register window size is 1 MiB, the previous > >> > > ranges translation for the internal register space had a size of > >> > > 0x4000000. This was done to allow the crypto and nand node to access > >> > > the corresponding 'sram' and 'nand' decoding windows. > >> > > > >> > > In order to describe the hardware more accurately, we declare the > >> > > real 1 MiB internal register space in the ranges, and add a translation > >> > > entry for the nand node to access the 'nand' window. > >> > > > >> > > This commit will make future improvements on the MBus DT binding easier. > >> > > > >> > > Signed-off-by: Ezequiel Garcia<ezequiel.garcia@free-electrons.com> > >> > > --- > >> > > Tested on Plathome Openblocks A6 board. > >> > > > >> > > arch/arm/boot/dts/kirkwood.dtsi | 5 +++-- > >> > > 1 file changed, 3 insertions(+), 2 deletions(-) > >> > > > >> > > diff --git a/arch/arm/boot/dts/kirkwood.dtsi b/arch/arm/boot/dts/kirkwood.dtsi > >> > > index 8a1e3bb..910fabc 100644 > >> > > --- a/arch/arm/boot/dts/kirkwood.dtsi > >> > > +++ b/arch/arm/boot/dts/kirkwood.dtsi > >> > > @@ -38,7 +38,8 @@ > >> > > > >> > > ocp@f1000000 { > >> > > compatible = "simple-bus"; > >> > > - ranges =<0x00000000 0xf1000000 0x4000000 > >> > > + ranges =<0x00000000 0xf1000000 0x0100000 > >> > > + 0xf4000000 0xf4000000 0x0000400 > >> > > 0xf5000000 0xf5000000 0x0000400>; > >> > > >> > Ezequiel, > >> > > >> > maybe you should also put a comment at the end of each ranges > >> > line to name the remap it does? > >> > > >> > Also, as already mentioned on IRC, it would be great to also > >> > have dove.dtsi updated accordingly. > >> > > >> > >> Indeed, I will do both. > > > > The sooner, the better. This is probably the last patch to get in for > > this merge window for mvebu... > > > > Right. Please don't wait for this patch if it's not in your mailbox by tomorrow. Ok. thx, Jason.
On Tue, Jun 18, 2013 at 12:31:19PM -0300, Ezequiel Garcia wrote: > Although the internal register window size is 1 MiB, the previous > ranges translation for the internal register space had a size of > 0x4000000. This was done to allow the crypto and nand node to access > the corresponding 'sram' and 'nand' decoding windows. > > In order to describe the hardware more accurately, we declare the > real 1 MiB internal register space in the ranges, and add a translation > entry for the nand node to access the 'nand' window. > > This commit will make future improvements on the MBus DT binding easier. > > Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com> > --- > Tested on Plathome Openblocks A6 board. > > arch/arm/boot/dts/kirkwood.dtsi | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) Applied to mvebu/dt thx, Jason.
Hi Ezequiel, apologies in advance for commenting on an already-merged patch. On 06/18/2013 05:31 PM, Ezequiel Garcia wrote: > Although the internal register window size is 1 MiB, the previous > ranges translation for the internal register space had a size of > 0x4000000. This was done to allow the crypto and nand node to access > the corresponding 'sram' and 'nand' decoding windows. > > In order to describe the hardware more accurately, we declare the > real 1 MiB internal register space in the ranges, and add a translation > entry for the nand node to access the 'nand' window. > > This commit will make future improvements on the MBus DT binding easier. > > Signed-off-by: Ezequiel Garcia <ezequiel.garcia-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> > --- > Tested on Plathome Openblocks A6 board. > > arch/arm/boot/dts/kirkwood.dtsi | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/arch/arm/boot/dts/kirkwood.dtsi b/arch/arm/boot/dts/kirkwood.dtsi > index 8a1e3bb..910fabc 100644 > --- a/arch/arm/boot/dts/kirkwood.dtsi > +++ b/arch/arm/boot/dts/kirkwood.dtsi > @@ -38,7 +38,8 @@ > > ocp@f1000000 { > compatible = "simple-bus"; > - ranges = <0x00000000 0xf1000000 0x4000000 > + ranges = <0x00000000 0xf1000000 0x0100000 > + 0xf4000000 0xf4000000 0x0000400 > 0xf5000000 0xf5000000 0x0000400>; Apart from "consistency" with the following range (0xf5000000) used by the crypto node, is there any reason why you did not do something like this instead (which Valentin suggested, but I will take the blame for): - ranges = <0x00000000 0xf1000000 0x4000000 + ranges = <0x00000000 0xf1000000 0x0100000 + 0x03000000 0xf4000000 0x0000400 0xf5000000 0xf5000000 0x0000400>; This would keep a consistent addressing within the child device bus, and avoid a later incosistency between the "unit-address" and the first "reg" address: > #address-cells = <1>; > #size-cells = <1>; > @@ -171,7 +172,7 @@ > nand@3000000 { ^^^^^^^ > #address-cells = <1>; > #size-cells = <1>; > cle = <0>; > ale = <1>; > bank-width = <1>; > compatible = "marvell,orion-nand"; > - reg = <0x3000000 0x400>; > + reg = <0xf4000000 0x400>; ^^^^^^^^^ > chip-delay = <25>; > /* set partition map and/or chip-delay in board dts */ > clocks = <&gate_clk 7>; > I guess the same could be done for the third range and for the crypto node as well: - ranges = <0x00000000 0xf1000000 0x4000000 - 0xf5000000 0xf5000000 0x0000400>; + ranges = <0x00000000 0xf1000000 0x0100000 + 0x03000000 0xf4000000 0x0000400 + 0x04000000 0xf5000000 0x0000400>; crypto@30000 { compatible = "marvell,orion-crypto"; reg = <0x30000 0x10000>, - <0xf5000000 0x800>; + <0x04000000 0x800>; reg-names = "regs", "sram"; interrupts = <22>; clocks = <&gate_clk 17>; status = "okay"; }; Does the above make sense? Or would that clash with your latest mvebu-mbus device tree binding? (Which, BTW, I have not quite understood yet). Thank you, Gerlando
Hi Gerlando, On Tue, Jul 16, 2013 at 11:37:30AM +0200, Gerlando Falauto wrote: > > apologies in advance for commenting on an already-merged patch. Sure, no problem. > > On 06/18/2013 05:31 PM, Ezequiel Garcia wrote: > > Although the internal register window size is 1 MiB, the previous > > ranges translation for the internal register space had a size of > > 0x4000000. This was done to allow the crypto and nand node to access > > the corresponding 'sram' and 'nand' decoding windows. > > > > In order to describe the hardware more accurately, we declare the > > real 1 MiB internal register space in the ranges, and add a translation > > entry for the nand node to access the 'nand' window. > > > > This commit will make future improvements on the MBus DT binding easier. > > > > Signed-off-by: Ezequiel Garcia <ezequiel.garcia-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> > > --- > > Tested on Plathome Openblocks A6 board. > > > > arch/arm/boot/dts/kirkwood.dtsi | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/arch/arm/boot/dts/kirkwood.dtsi b/arch/arm/boot/dts/kirkwood.dtsi > > index 8a1e3bb..910fabc 100644 > > --- a/arch/arm/boot/dts/kirkwood.dtsi > > +++ b/arch/arm/boot/dts/kirkwood.dtsi > > @@ -38,7 +38,8 @@ > > > > ocp@f1000000 { > > compatible = "simple-bus"; > > - ranges = <0x00000000 0xf1000000 0x4000000 > > + ranges = <0x00000000 0xf1000000 0x0100000 > > + 0xf4000000 0xf4000000 0x0000400 > > 0xf5000000 0xf5000000 0x0000400>; > > Apart from "consistency" with the following range (0xf5000000) used by > the crypto node, is there any reason why you did not do something like > this instead (which Valentin suggested, but I will take the blame for): > I'm not sure the reason is "consistency with the crypto node". There's an MBus window at 0xf4000000 for NAND, and that is what is described in the snippet above; and this is a better reason. That said, technically speaking, you can have any translation scheme you want, as long as it ends up in 0xf4000000. > - ranges = <0x00000000 0xf1000000 0x4000000 > + ranges = <0x00000000 0xf1000000 0x0100000 > + 0x03000000 0xf4000000 0x0000400 > 0xf5000000 0xf5000000 0x0000400>; > > This would keep a consistent addressing within the child device bus, and Could you explain how this "keeps a consistent addressing"? Frankly, I don't understand why you choose 0x3000000 ... am I missing something? Also, speaking of "device bus" this nand node should be behind a devicebus node. ranges = <MBUS_ID(0xf0, 0x01) 0 0 0xf1000000 0x100000 /* internal-regs */ MBUS_ID(0x01, 0x2f) 0 0 0xf4000000 0x400>; devbus { status = "okay"; ranges = <0 MBUS_ID(0x01, 0x2f) 0 0x400>; /* nand */ nand { compatible = "marvell,orion-nand"; reg = <0 0x400>; }; }; (notice this will allow you to relocate the base address of the NAND windows easily if it conflicts with your PCIe needs). > avoid a later incosistency between the "unit-address" and the first > "reg" address: > > > #address-cells = <1>; > > #size-cells = <1>; > > @@ -171,7 +172,7 @@ > > nand@3000000 { > ^^^^^^^ Oh, this should be fixed. I just missed it, and nobody noticed either.
Hi Ezequiel, On 07/16/2013 02:56 PM, Ezequiel Garcia wrote: > Hi Gerlando, > > On Tue, Jul 16, 2013 at 11:37:30AM +0200, Gerlando Falauto wrote: >> >> apologies in advance for commenting on an already-merged patch. > > Sure, no problem. > >> >> On 06/18/2013 05:31 PM, Ezequiel Garcia wrote: >>> Although the internal register window size is 1 MiB, the previous >>> ranges translation for the internal register space had a size of >>> 0x4000000. This was done to allow the crypto and nand node to access >>> the corresponding 'sram' and 'nand' decoding windows. >>> >>> In order to describe the hardware more accurately, we declare the >>> real 1 MiB internal register space in the ranges, and add a translation >>> entry for the nand node to access the 'nand' window. >>> >>> This commit will make future improvements on the MBus DT binding easier. >>> >>> Signed-off-by: Ezequiel Garcia <ezequiel.garcia-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> >>> --- >>> Tested on Plathome Openblocks A6 board. >>> >>> arch/arm/boot/dts/kirkwood.dtsi | 5 +++-- >>> 1 file changed, 3 insertions(+), 2 deletions(-) >>> >>> diff --git a/arch/arm/boot/dts/kirkwood.dtsi b/arch/arm/boot/dts/kirkwood.dtsi >>> index 8a1e3bb..910fabc 100644 >>> --- a/arch/arm/boot/dts/kirkwood.dtsi >>> +++ b/arch/arm/boot/dts/kirkwood.dtsi >>> @@ -38,7 +38,8 @@ >>> >>> ocp@f1000000 { >>> compatible = "simple-bus"; >>> - ranges = <0x00000000 0xf1000000 0x4000000 >>> + ranges = <0x00000000 0xf1000000 0x0100000 >>> + 0xf4000000 0xf4000000 0x0000400 >>> 0xf5000000 0xf5000000 0x0000400>; >> >> Apart from "consistency" with the following range (0xf5000000) used by >> the crypto node, is there any reason why you did not do something like >> this instead (which Valentin suggested, but I will take the blame for): >> > > I'm not sure the reason is "consistency with the crypto node". > There's an MBus window at 0xf4000000 for NAND, and that is what is described > in the snippet above; and this is a better reason. > > That said, technically speaking, you can have any translation scheme you want, > as long as it ends up in 0xf4000000. > >> - ranges = <0x00000000 0xf1000000 0x4000000 >> + ranges = <0x00000000 0xf1000000 0x0100000 >> + 0x03000000 0xf4000000 0x0000400 >> 0xf5000000 0xf5000000 0x0000400>; >> >> This would keep a consistent addressing within the child device bus, and > > Could you explain how this "keeps a consistent addressing"? > Frankly, I don't understand why you choose 0x3000000 ... am I missing something? Actually, the only one missing something here is myself, as I do not quite have the full picture :-) Mine was a bit of a "reverse allocation" -- we know we must end up at 0xf4000000. Since all the addresses are shifted by 0xf1000000, here comes 0x03000000... But please see my last question further down. > > Also, speaking of "device bus" this nand node should be behind a devicebus node. > > ranges = <MBUS_ID(0xf0, 0x01) 0 0 0xf1000000 0x100000 /* internal-regs */ > MBUS_ID(0x01, 0x2f) 0 0 0xf4000000 0x400>; > > devbus { > status = "okay"; > ranges = <0 MBUS_ID(0x01, 0x2f) 0 0x400>; > > /* nand */ > nand { > compatible = "marvell,orion-nand"; > reg = <0 0x400>; > }; > }; I believe that makes a lot more sense this way... I guess this feature (device bus) requires your latest set of patches, right? (either v7 as you posted yesterday or your tree at git://github.com/MISL-EBU-System-SW/mainline-public.git/marvell-mvebu-mbus-v7) > (notice this will allow you to relocate the base address of the NAND windows > easily if it conflicts with your PCIe needs). I sort of had the impression I could do already do that somehow, though I am not quite sure anymore... >> avoid a later incosistency between the "unit-address" and the first >> "reg" address: >> >>> #address-cells = <1>; >>> #size-cells = <1>; >>> @@ -171,7 +172,7 @@ >> > nand@3000000 { >> ^^^^^^^ > > Oh, this should be fixed. I just missed it, and nobody noticed either. > So, in the end, you think it's OK to have a set of nodes with "relative" addresses (gpio@10140, serial@12000 etc...) and some with "absolute" addresses (nand@0xf4000000, where the ranges property does a 0-offset translation)? Even though I understand this is just some transitional state, and it will all be fixed like your example above, once we get the rest of the rework merged (mbus/devbus). Thanks a lot for your patience! Gerlando
Hi Ezequiel, On 07/16/2013 02:56 PM, Ezequiel Garcia wrote: [...] > Also, speaking of "device bus" this nand node should be behind a devicebus node. > > ranges = <MBUS_ID(0xf0, 0x01) 0 0 0xf1000000 0x100000 /* internal-regs */ > MBUS_ID(0x01, 0x2f) 0 0 0xf4000000 0x400>; > > devbus { > status = "okay"; > ranges = <0 MBUS_ID(0x01, 0x2f) 0 0x400>; > > /* nand */ > nand { > compatible = "marvell,orion-nand"; > reg = <0 0x400>; > }; > }; > > (notice this will allow you to relocate the base address of the NAND windows > easily if it conflicts with your PCIe needs). I am MAYBE slowly starting to understand this whole mbus rework. Just one remark though: don't you think it would make sense to add something like: #define MBUS_ID_INTERNAL_REGS MBUS_ID(0xf0, 0x01) #define MBUS_ID_NAND MBUS_ID(0x01, 0x2f) I personally have a hard time reading numeric values for GPIO_ACTIVE_LOW/GPIO_ACTIVE_HIGH. Thanks, Gerlando > >> avoid a later incosistency between the "unit-address" and the first >> "reg" address: >> >>> #address-cells = <1>; >>> #size-cells = <1>; >>> @@ -171,7 +172,7 @@ >> > nand@3000000 { >> ^^^^^^^ > > Oh, this should be fixed. I just missed it, and nobody noticed either. >
Hi Gerlando, On Wed, Jul 17, 2013 at 08:35:38AM +0200, Gerlando Falauto wrote: > On 07/16/2013 02:56 PM, Ezequiel Garcia wrote: > [...] > > Also, speaking of "device bus" this nand node should be behind a devicebus node. > > > > ranges = <MBUS_ID(0xf0, 0x01) 0 0 0xf1000000 0x100000 /* internal-regs */ > > MBUS_ID(0x01, 0x2f) 0 0 0xf4000000 0x400>; > > > > devbus { > > status = "okay"; > > ranges = <0 MBUS_ID(0x01, 0x2f) 0 0x400>; > > > > /* nand */ > > nand { > > compatible = "marvell,orion-nand"; > > reg = <0 0x400>; > > }; > > }; > > > > (notice this will allow you to relocate the base address of the NAND windows > > easily if it conflicts with your PCIe needs). > > I am MAYBE slowly starting to understand this whole mbus rework. > Just one remark though: don't you think it would make sense to add > something like: > > #define MBUS_ID_INTERNAL_REGS MBUS_ID(0xf0, 0x01) > #define MBUS_ID_NAND MBUS_ID(0x01, 0x2f) > Yeah, maybe it would make sense. This has been discussed in the past and others were against, so that's the reason it's not included in the series I submitted. But feel free to send a patch proposing it once the MBus is merged! Thanks,
Gerlando, On Tue, Jul 16, 2013 at 08:51:37PM +0200, Gerlando Falauto wrote: [...] > > > > Also, speaking of "device bus" this nand node should be behind a devicebus node. > > > > ranges = <MBUS_ID(0xf0, 0x01) 0 0 0xf1000000 0x100000 /* internal-regs */ > > MBUS_ID(0x01, 0x2f) 0 0 0xf4000000 0x400>; > > > > devbus { > > status = "okay"; > > ranges = <0 MBUS_ID(0x01, 0x2f) 0 0x400>; > > > > /* nand */ > > nand { > > compatible = "marvell,orion-nand"; > > reg = <0 0x400>; > > }; > > }; > > I believe that makes a lot more sense this way... I guess this feature > (device bus) requires your latest set of patches, right? (either v7 as > you posted yesterday or your tree at > git://github.com/MISL-EBU-System-SW/mainline-public.git/marvell-mvebu-mbus-v7) > No, not really. The device-bus driver is already merged, so we only need to use it, and maybe extend it a bit to support kirkwood (in case it's needed). You can see in: arch/arm/boot/dts/armada-xp-openblocks-ax3-4.dts for an example. I would prefer that to be done *after* the MBus is accepted... but that's just a personal preference. > > (notice this will allow you to relocate the base address of the NAND windows > > easily if it conflicts with your PCIe needs). > > I sort of had the impression I could do already do that somehow, though > I am not quite sure anymore... > Well, you can do it by patching and re-building the kernel as the MBus windows are created from C-files. With the MBus DT binding, all you need is to change the DT. > >> avoid a later incosistency between the "unit-address" and the first > >> "reg" address: > >> > >>> #address-cells = <1>; > >>> #size-cells = <1>; > >>> @@ -171,7 +172,7 @@ > >> > nand@3000000 { > >> ^^^^^^^ > > > > Oh, this should be fixed. I just missed it, and nobody noticed either. > > > > So, in the end, you think it's OK to have a set of nodes with "relative" > addresses (gpio@10140, serial@12000 etc...) and some with "absolute" > addresses (nand@0xf4000000, where the ranges property does a 0-offset > translation)? > Even though I understand this is just some transitional state, and it > will all be fixed like your example above, once we get the rest of the > rework merged (mbus/devbus). Mmm.. I see your point. In the end, it's just as you say, the current state is only transitional. There's no way to have a proper DT without the MBus DT binding. The reason for this mess is that the current layout mixes the actual address space. Feel free to take a look at the latest MBus series and try to add MBus DT binding for Kirkwood. If you do, don't hesitate in asking as many questions as you need and/or posting RFCs for us to look at.
diff --git a/arch/arm/boot/dts/kirkwood.dtsi b/arch/arm/boot/dts/kirkwood.dtsi index 8a1e3bb..910fabc 100644 --- a/arch/arm/boot/dts/kirkwood.dtsi +++ b/arch/arm/boot/dts/kirkwood.dtsi @@ -38,7 +38,8 @@ ocp@f1000000 { compatible = "simple-bus"; - ranges = <0x00000000 0xf1000000 0x4000000 + ranges = <0x00000000 0xf1000000 0x0100000 + 0xf4000000 0xf4000000 0x0000400 0xf5000000 0xf5000000 0x0000400>; #address-cells = <1>; #size-cells = <1>; @@ -171,7 +172,7 @@ ale = <1>; bank-width = <1>; compatible = "marvell,orion-nand"; - reg = <0x3000000 0x400>; + reg = <0xf4000000 0x400>; chip-delay = <25>; /* set partition map and/or chip-delay in board dts */ clocks = <&gate_clk 7>;
Although the internal register window size is 1 MiB, the previous ranges translation for the internal register space had a size of 0x4000000. This was done to allow the crypto and nand node to access the corresponding 'sram' and 'nand' decoding windows. In order to describe the hardware more accurately, we declare the real 1 MiB internal register space in the ranges, and add a translation entry for the nand node to access the 'nand' window. This commit will make future improvements on the MBus DT binding easier. Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com> --- Tested on Plathome Openblocks A6 board. arch/arm/boot/dts/kirkwood.dtsi | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)