ARM: Kirkwood: Fix the internal register ranges translation
PATCH 1/1 ARM: Kirkwood: Fix the internal register ranges translation
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.
---
Tested on Plathome Openblocks A6 board.
arch/arm/boot/dts/kirkwood.dtsi | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
arch/arm/boot/dts/kirkwood.dtsi +3 -2
@@ -38,7 +38,8 @@ ocp at 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>;
Signature · 1 line
1.8.1.5
PATCH 1/1 ARM: Kirkwood: Fix the internal register ranges translation
Quoted 23 lines · Ezequiel Garcia
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<[email protected]>---
Tested on Plathome Openblocks A6 board.arch/arm/boot/dts/kirkwood.dtsi | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)arch/arm/boot/dts/kirkwood.dtsi +0
-0
arch/arm/boot/dts/kirkwood.dtsi line 43
Quoted 7 lines
arch/arm/boot/dts/kirkwood.dtsi +2
-1@@ -38,7 +38,8 @@ ocp at 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
Quoted 11 lines
arch/arm/boot/dts/kirkwood.dtsi +1
-1#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>;
PATCH 1/1 ARM: Kirkwood: Fix the internal register ranges translation
Quoted 34 lines · Sebastian Hesselbarth
Quoted 23 lines · Ezequiel Garcia
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<[email protected]>---
Tested on Plathome Openblocks A6 board.arch/arm/boot/dts/kirkwood.dtsi | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)arch/arm/boot/dts/kirkwood.dtsi +0
-0arch/arm/boot/dts/kirkwood.dtsi line 43
Quoted 7 lines
arch/arm/boot/dts/kirkwood.dtsi +2
-1@@ -38,7 +38,8 @@ ocp at f1000000 { compatible = "simple-bus"; - ranges =<0x00000000 0xf1000000 0x4000000 + ranges =<0x00000000 0xf1000000 0x0100000 + 0xf4000000 0xf4000000 0x0000400 0xf5000000 0xf5000000 0x0000400>;Ezequiel,
Quoted 6 lines
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.dtsiupdated accordingly.
Indeed, I will do both.
Thanks!
Signature · 3 lines
Ezequiel Garc?a, Free Electrons Embedded Linux, Kernel and Android Engineering http://free-electrons.com
PATCH 1/1 ARM: Kirkwood: Fix the internal register ranges translation
Quoted 35 lines · Ezequiel Garcia
Quoted 34 lines · Sebastian Hesselbarth
Quoted 23 lines · Ezequiel Garcia
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<[email protected]>---
Tested on Plathome Openblocks A6 board.arch/arm/boot/dts/kirkwood.dtsi | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)arch/arm/boot/dts/kirkwood.dtsi +0
-0arch/arm/boot/dts/kirkwood.dtsi line 43
Quoted 7 lines
arch/arm/boot/dts/kirkwood.dtsi +2
-1@@ -38,7 +38,8 @@ ocp at f1000000 { compatible = "simple-bus"; - ranges =<0x00000000 0xf1000000 0x4000000 + ranges =<0x00000000 0xf1000000 0x0100000 + 0xf4000000 0xf4000000 0x0000400 0xf5000000 0xf5000000 0x0000400>;Ezequiel,
Quoted 8 lines
Quoted 6 lines
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.dtsiupdated 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.
PATCH 1/1 ARM: Kirkwood: Fix the internal register ranges translation
Quoted 39 lines · Jason Cooper
Quoted 38 lines · Ezequiel Garcia
Quoted 37 lines · Sebastian Hesselbarth
Quoted 23 lines · Ezequiel Garcia
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<[email protected]>---
Tested on Plathome Openblocks A6 board.arch/arm/boot/dts/kirkwood.dtsi | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)arch/arm/boot/dts/kirkwood.dtsi +0
-0arch/arm/boot/dts/kirkwood.dtsi line 43
Quoted 7 lines
arch/arm/boot/dts/kirkwood.dtsi +2
-1@@ -38,7 +38,8 @@ ocp at 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?
Quoted 9 lines
Quoted 5 lines
Quoted 3 lines
Also, as already mentioned on IRC, it would be great to also have
dove.dtsiupdated 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!
Signature · 1 line
Ezequiel
PATCH 1/1 ARM: Kirkwood: Fix the internal register ranges translation
Quoted 44 lines · Ezequiel Garcia
Quoted 43 lines · Jason Cooper
Quoted 42 lines · Ezequiel Garcia
Quoted 40 lines · Sebastian Hesselbarth
Quoted 23 lines · Ezequiel Garcia
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<[email protected]>---
Tested on Plathome Openblocks A6 board.arch/arm/boot/dts/kirkwood.dtsi | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)arch/arm/boot/dts/kirkwood.dtsi +0
-0arch/arm/boot/dts/kirkwood.dtsi line 43
Quoted 7 lines
arch/arm/boot/dts/kirkwood.dtsi +2
-1@@ -38,7 +38,8 @@ ocp at 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.dtsiupdated accordingly.
Quoted 7 lines
Quoted 5 lines
Quoted 1 line
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.
Jason Cooper
AppliedPATCH 1/1 ARM: Kirkwood: Fix the internal register ranges translation
In reply to Ezequiel GarciaQuoted 9 lines · Ezequiel Garcia
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.
Quoted 8 lines
This commit will make future improvements on the MBus DT binding easier.
Signed-off-by Ezequiel Garcia <[email protected]>---
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.
PATCH 1/1 ARM: Kirkwood: Fix the internal register ranges translation
In reply to Ezequiel GarciaHi Ezequiel,
apologies in advance for commenting on an already-merged patch.
Quoted 23 lines · Ezequiel Garcia
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.
---
Tested on Plathome Openblocks A6 board.arch/arm/boot/dts/kirkwood.dtsi | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)arch/arm/boot/dts/kirkwood.dtsi +0
-0
arch/arm/boot/dts/kirkwood.dtsi line 43
Quoted 7 lines
arch/arm/boot/dts/kirkwood.dtsi +2
-1@@ -38,7 +38,8 @@ ocp at 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:
arch/arm/boot/dts/kirkwood.dtsi line 172
Quoted 4 lines
arch/arm/boot/dts/kirkwood.dtsi +0
-0#address-cells = <1>; #size-cells = <1>;@@ -171,7 +172,7 @@ nand at 3000000 {
^^^^^^^
arch/arm/boot/dts/kirkwood.dtsi line 175
Quoted 8 lines
arch/arm/boot/dts/kirkwood.dtsi +1
-1#address-cells = <1>; #size-cells = <1>; cle = <0>; ale = <1>; bank-width = <1>; compatible = "marvell,orion-nand"; - reg = <0x3000000 0x400>; + reg = <0xf4000000 0x400>;
^^^^^^^^^
Quoted 4 lines
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 at 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
PATCH 1/1 ARM: Kirkwood: Fix the internal register ranges translation
Hi Gerlando,
Quoted 2 lines · Gerlando Falauto
apologies in advance for commenting on an already-merged patch.
Sure, no problem.
Quoted 26 lines · Ezequiel Garcia
Quoted 24 lines
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.
---
Tested on Plathome Openblocks A6 board.arch/arm/boot/dts/kirkwood.dtsi | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)arch/arm/boot/dts/kirkwood.dtsi +0
-0@@ -38,7 +38,8 @@
Quoted 11 lines
arch/arm/boot/dts/kirkwood.dtsi line 43
Quoted 6 lines
arch/arm/boot/dts/kirkwood.dtsi +2
-1ocp at 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.
Quoted 6 lines
arch/arm/boot/dts/kirkwood.dtsi +2
-1- 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).
Quoted 8 lines
avoid a later incosistency between the "unit-address" and the first "reg" address:
arch/arm/boot/dts/kirkwood.dtsi line 172
Quoted 4 lines
arch/arm/boot/dts/kirkwood.dtsi +0
-0#address-cells = <1>; #size-cells = <1>;@@ -171,7 +172,7 @@ nand at 3000000 {^^^^^^^
Oh, this should be fixed. I just missed it, and nobody noticed either.
Signature · 3 lines
Ezequiel Garc?a, Free Electrons Embedded Linux, Kernel and Android Engineering http://free-electrons.com
PATCH 1/1 ARM: Kirkwood: Fix the internal register ranges translation
Hi Ezequiel,
Quoted 58 lines · Ezequiel Garcia
Hi Gerlando,
Quoted 2 lines · Gerlando Falauto
apologies in advance for commenting on an already-merged patch.
Sure, no problem.
Quoted 26 lines · Ezequiel Garcia
Quoted 24 lines
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.
---
Tested on Plathome Openblocks A6 board.arch/arm/boot/dts/kirkwood.dtsi | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)arch/arm/boot/dts/kirkwood.dtsi +0
-0@@ -38,7 +38,8 @@Quoted 11 lines
arch/arm/boot/dts/kirkwood.dtsi line 43
Quoted 6 lines
arch/arm/boot/dts/kirkwood.dtsi +2
-1ocp at 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
0xf4000000for 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.Quoted 5 lines
arch/arm/boot/dts/kirkwood.dtsi +2
-1- ranges = <0x00000000 0xf1000000 0x4000000 + ranges = <0x00000000 0xf1000000 0x0100000 + 0x03000000 0xf4000000 0x0000400 0xf5000000 0xf5000000 0x0000400>;
Quoted 4 lines
Quoted 1 line
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.
Quoted 16 lines
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 atgit://github.com/MISL-EBU-System-SW/mainline-public.git/marvell-mvebu-mbus-v7)
Quoted 2 lines
(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...
Quoted 11 lines
Quoted 8 lines
avoid a later incosistency between the "unit-address" and the first "reg" address:
arch/arm/boot/dts/kirkwood.dtsi line 172
Quoted 4 lines
arch/arm/boot/dts/kirkwood.dtsi +0
-0#address-cells = <1>; #size-cells = <1>;@@ -171,7 +172,7 @@ nand at 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 at 10140, serial at 12000 etc...) and some with "absolute" addresses (nand at 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
PATCH 1/1 ARM: Kirkwood: Fix the internal register ranges translation
Gerlando,
On Tue, Jul 16, 2013 at 08:51:37PM +0200, Gerlando Falauto wrote:
[...]
Quoted 17 lines
Quoted 16 lines
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>; }; };
Quoted 5 lines
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.
Quoted 6 lines
Quoted 2 lines
(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.
Quoted 12 lines
Quoted 11 lines
Quoted 8 lines
avoid a later incosistency between the "unit-address" and the first "reg" address:
Review line 172
Quoted 4 lines
#address-cells = <1>; #size-cells = <1>;(quoted diff) +0
-0@@ -171,7 +172,7 @@ nand at 3000000 {^^^^^^^Oh, this should be fixed. I just missed it, and nobody noticed either.
Quoted 7 lines
So, in the end, you think it's OK to have a set of nodes with "relative" addresses (gpio at 10140, serial at 12000 etc...) and some with "absolute" addresses (nand at
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.
Signature · 3 lines
Ezequiel Garc?a, Free Electrons Embedded Linux, Kernel and Android Engineering http://free-electrons.com
PATCH 1/1 ARM: Kirkwood: Fix the internal register ranges translation
In reply to Ezequiel GarciaHi Ezequiel,
On 07/16/2013 02:56 PM, Ezequiel Garcia wrote:
[...]
Quoted 16 lines
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>; }; };
Quoted 2 lines
(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
Quoted 12 lines
Quoted 9 lines
avoid a later incosistency between the "unit-address" and the first "reg" address:
Review line 172
Quoted 4 lines
#address-cells = <1>; #size-cells = <1>;(quoted diff) +0
-0@@ -171,7 +172,7 @@ nand at 3000000 {^^^^^^^Oh, this should be fixed. I just missed it, and nobody noticed either.
PATCH 1/1 ARM: Kirkwood: Fix the internal register ranges translation
Hi Gerlando,
Quoted 21 lines · Gerlando Falauto
On 07/16/2013 02:56 PM, Ezequiel Garcia wrote:
[...]Quoted 16 lines
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>; }; };Quoted 2 lines
(notice this will allow you to relocate the base address of the NAND windows easily if it conflicts with your PCIe needs).
Quoted 7 lines
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_REGSMBUS_ID(0xf0, 0x01)
#defineMBUS_ID_NANDMBUS_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,
Signature · 3 lines
Ezequiel Garc?a, Free Electrons Embedded Linux, Kernel and Android Engineering http://free-electrons.com