diff mbox series

[U-Boot] core: ofnode: Fix translation for #size-cells == 0

Message ID 20180306065442.26322-1-mario.six@gdsys.cc
State Superseded
Delegated to: Simon Glass
Headers show
Series [U-Boot] core: ofnode: Fix translation for #size-cells == 0 | expand

Commit Message

Mario Six March 6, 2018, 6:54 a.m. UTC
Commit 286ede6 ("drivers: core: Add translation in live tree case") made
dev_get_addr always use proper bus translations for addresses read from
the device tree. But this leads to problems with certain busses, e.g.
I2C busses, which run into an error during translation, and hence stop
working.

It turns out that of_translate_address() and fdt_translate_address()
stop the address translation with an error when they're asked to
translate addresses for busses where #size-cells == 0 (comment from
drivers/core/of_addr.c):

 * Note: We consider that crossing any level with #size-cells == 0 to mean
 * that translation is impossible (that is we are not dealing with a value
 * that can be mapped to a cpu physical address). This is not really specified
 * that way, but this is traditionally the way IBM at least do things

To fix this case, we check in both the live-tree and non-live tree-case,
whether the bus of the device whose address is about to be translated
has size-cell size zero. If this is the case, we just read the address
as a plain integer and return it, and only apply bus translations if the
size-cell size if greater than zero.

Signed-off-by: Mario Six <mario.six@gdsys.cc>
Signed-off-by: Martin Fuzzey <mfuzzey@parkeon.com>
Reported-by: Martin Fuzzey <mfuzzey@parkeon.com>
Fixes: 286ede6 ("drivers: core: Add translation in live tree case")
---
 drivers/core/fdtaddr.c | 17 +++++++++++------
 drivers/core/ofnode.c  |  5 ++++-
 2 files changed, 15 insertions(+), 7 deletions(-)

Comments

Simon Glass March 6, 2018, 5:51 p.m. UTC | #1
Hi Mario,

On 5 March 2018 at 23:54, Mario Six <mario.six@gdsys.cc> wrote:
> Commit 286ede6 ("drivers: core: Add translation in live tree case") made
> dev_get_addr always use proper bus translations for addresses read from
> the device tree. But this leads to problems with certain busses, e.g.
> I2C busses, which run into an error during translation, and hence stop
> working.
>
> It turns out that of_translate_address() and fdt_translate_address()
> stop the address translation with an error when they're asked to
> translate addresses for busses where #size-cells == 0 (comment from
> drivers/core/of_addr.c):
>
>  * Note: We consider that crossing any level with #size-cells == 0 to mean
>  * that translation is impossible (that is we are not dealing with a value
>  * that can be mapped to a cpu physical address). This is not really specified
>  * that way, but this is traditionally the way IBM at least do things
>
> To fix this case, we check in both the live-tree and non-live tree-case,
> whether the bus of the device whose address is about to be translated
> has size-cell size zero. If this is the case, we just read the address
> as a plain integer and return it, and only apply bus translations if the
> size-cell size if greater than zero.
>
> Signed-off-by: Mario Six <mario.six@gdsys.cc>
> Signed-off-by: Martin Fuzzey <mfuzzey@parkeon.com>
> Reported-by: Martin Fuzzey <mfuzzey@parkeon.com>
> Fixes: 286ede6 ("drivers: core: Add translation in live tree case")
> ---
>  drivers/core/fdtaddr.c | 17 +++++++++++------
>  drivers/core/ofnode.c  |  5 ++++-
>  2 files changed, 15 insertions(+), 7 deletions(-)

This looks right to me, but I really think we need to create a simple
test for it. Perhaps call this function in test/dm/test-fdt.c for a
few cases? You can add whatever you like to test.dts.

Regards,
Simon
diff mbox series

Patch

diff --git a/drivers/core/fdtaddr.c b/drivers/core/fdtaddr.c
index 3847dd836e..9a3b4c312a 100644
--- a/drivers/core/fdtaddr.c
+++ b/drivers/core/fdtaddr.c
@@ -49,12 +49,17 @@  fdt_addr_t devfdt_get_addr_index(struct udevice *dev, int index)
 
 		reg += index * (na + ns);
 
-		/*
-		 * Use the full-fledged translate function for complex
-		 * bus setups.
-		 */
-		addr = fdt_translate_address((void *)gd->fdt_blob,
-					     dev_of_offset(dev), reg);
+		if (ns) {
+			/*
+			 * Use the full-fledged translate function for complex
+			 * bus setups.
+			 */
+			addr = fdt_translate_address((void *)gd->fdt_blob,
+						     dev_of_offset(dev), reg);
+		} else {
+			/* Non translatable if #size-cells == 0 */
+			addr = fdt_read_number(reg, na);
+		}
 	} else {
 		/*
 		 * Use the "simple" translate function for less complex
diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c
index 98f4b539ea..b2f6ffe385 100644
--- a/drivers/core/ofnode.c
+++ b/drivers/core/ofnode.c
@@ -200,13 +200,16 @@  fdt_addr_t ofnode_get_addr_index(ofnode node, int index)
 		uint flags;
 		u64 size;
 		int na;
+		int ns;
 
 		prop_val = of_get_address(ofnode_to_np(node), index, &size,
 					  &flags);
 		if (!prop_val)
 			return FDT_ADDR_T_NONE;
 
-		if (IS_ENABLED(CONFIG_OF_TRANSLATE)) {
+		ns = of_n_size_cells(ofnode_to_np(node));
+
+		if (IS_ENABLED(CONFIG_OF_TRANSLATE) && ns > 0) {
 			return of_translate_address(ofnode_to_np(node), prop_val);
 		} else {
 			na = of_n_addr_cells(ofnode_to_np(node));