Re: [PATCH] ARM: Kirkwood: Fix the internal register ranges translation
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 0xf100 0x10 /*
internal-regs */
MBUS_ID(0x01, 0x2f) 0 0 0xf400 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,
--
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH] ARM: Kirkwood: Fix the internal register ranges translation
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 0xf100 0x10 /*
internal-regs */
MBUS_ID(0x01, 0x2f) 0 0 0xf400 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@300 {
^^^
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@0xf400, 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.
--
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH] ARM: Kirkwood: Fix the internal register ranges translation
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
0x400. 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/3fjtnbpr1lh4...@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@f100 {
compatible = simple-bus;
- ranges = 0x 0xf100 0x400
+ ranges = 0x 0xf100 0x010
+ 0xf400 0xf400 0x400
0xf500 0xf500 0x400;
Apart from consistency with the following range (0xf500) 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 0xf400 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 0xf400.
- ranges = 0x 0xf100 0x400
+ ranges = 0x 0xf100 0x010
+ 0x0300 0xf400 0x400
0xf500 0xf500 0x400;
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 0x300 ... 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 0xf100 0x10 /*
internal-regs */
MBUS_ID(0x01, 0x2f) 0 0 0xf400 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@300 {
^^^
Oh, this should be fixed. I just missed it, and nobody noticed either.
--
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH] ARM: Kirkwood: Fix the internal register ranges translation
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
0x400. 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 Garciaezequiel.gar...@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@f100 {
compatible = simple-bus;
- ranges =0x 0xf100 0x400
+ ranges =0x 0xf100 0x010
+ 0xf400 0xf400 0x400
0xf500 0xf500 0x400;
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.
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH] ARM: Kirkwood: Fix the internal register ranges translation
On Wed, Jun 19, 2013 at 3:36 PM, Jason Cooper ja...@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
0x400. 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 Garciaezequiel.gar...@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@f100 {
compatible = simple-bus;
- ranges =0x 0xf100 0x400
+ ranges =0x 0xf100 0x010
+ 0xf400 0xf400 0x400
0xf500 0xf500 0x400;
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!
--
Ezequiel
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH] ARM: Kirkwood: Fix the internal register ranges translation
On Wed, Jun 19, 2013 at 03:42:27PM -0300, Ezequiel Garcia wrote:
On Wed, Jun 19, 2013 at 3:36 PM, Jason Cooper ja...@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
0x400. 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 Garciaezequiel.gar...@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@f100 {
compatible = simple-bus;
- ranges =0x 0xf100 0x400
+ ranges =0x 0xf100 0x010
+ 0xf400 0xf400 0x400
0xf500 0xf500 0x400;
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.
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH] ARM: Kirkwood: Fix the internal register ranges translation
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
0x400. 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 Garciaezequiel.gar...@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@f100 {
compatible = simple-bus;
- ranges =0x 0xf100 0x400
+ ranges =0x 0xf100 0x010
+ 0xf400 0xf400 0x400
0xf500 0xf500 0x400;
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 =0x300 0x400;
+ reg =0xf400 0x400;
chip-delay =25;
/* set partition map and/or chip-delay in board dts */
clocks =gate_clk 7;
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH] ARM: Kirkwood: Fix the internal register ranges translation
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
0x400. 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 Garciaezequiel.gar...@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@f100 {
compatible = simple-bus;
- ranges =0x 0xf100 0x400
+ ranges =0x 0xf100 0x010
+ 0xf400 0xf400 0x400
0xf500 0xf500 0x400;
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!
--
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
