Re: [U-Boot] [PATCH] fdt: __of_translate_address(): check parent's 'ranges' before translate

Hi Przemyslaw,

The present implementation of __of_translate_address() taken from the Linux, is designed for translate bus/child address mappings by using 'ranges' property - and it doesn't allow for checking an address for a device's node with zero size-cells.

The 'size-cells > 0' is required for bus/child address mapping, but is not required for non-memory mapped address, e.g.: I2C chip. Then when we need only raw 'reg' property's value.

Since the I2C device address goes to a single-cell reg property, support for that case is welcome, but currently calling dev_get_addr() for I2C device will return 'FDT_ADDR_T_NONE', and print the warning:

warning: __of_translate_address: Bad cell count for 'some-dev'

The fix: The proper result by this commit, is achieved by skipping the condition check: 'size-cells > 0', when the parent doesn't provide the 'ranges' property and allows to return 1:1 address translation, if caller want's just the reg property's value.

No additional argument is needed, the 'ranges' property existence decides, what type of translation is done when calling dev_get_addr(). And this should be, what the compatible driver expects.

Now, by this commit the function __of_translate_address() can be used for the both reg property use-cases:

Case 1: (ranges)

some-bus { address-cells = <1>; size-cells = <1>; ranges = <0x0 0x10000000 0x1000>; reg = <0x10000000 0x1000>;

child1 { reg = <0xa00 0x100>; };

child2 { reg = <0xb00 0x100>; }; };

Return values (CONFIG_OF_TRANSLATE=y):

• dev_get_addr(some-bus) - retrurns: 0x10000000 - correct
• dev_get_addr(child1) - retrurns: 0x10000a00 - correct
• dev_get_addr(child2) - retrurns: 0x10000b00 - correct

This works as previous - this commit have no impact on this case.

Case 2: (no ranges - e.g. I2C bus childs) - fixed

I2C-bus { address-cells = <1>; size-cells = <0>; reg = <0x10000000>;

chip1 { reg = <0xa00>; };

chip2 { reg = <0xb00>; }; };

Return values (CONFIG_OF_TRANSLATE=y):

• dev_get_addr(I2C-bus) - retrurns: 0x10000000 - correct
• dev_get_addr(chip1) - retrurns: 0xa00 - correct
• dev_get_addr(chip2) - retrurns: 0xb00 - correct

This is fixed, since the previously returned value for chip1 and chip2 was: 0xffffffff, which means: 'FDT_ADDR_T_NONE'.

Signed-off-by: Przemyslaw Marczak p.marczak@samsung.com Cc: Stefan Roese sr@denx.de Cc: Tom Rini trini@konsulko.com Cc: Simon Glass sjg@chromium.org Cc: Stephen Warren swarren@nvidia.com Cc: Stephen Warren swarren@wwwdotorg.org

---

common/fdt_support.c | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/common/fdt_support.c b/common/fdt_support.c index 66464db..1f472ca 100644 --- a/common/fdt_support.c +++ b/common/fdt_support.c @@ -952,8 +952,9 @@ void fdt_del_node_and_alias(void *blob, const char *alias) /* Max address size we deal with */ #define OF_MAX_ADDR_CELLS 4 #define OF_BAD_ADDR FDT_ADDR_T_NONE -#define OF_CHECK_COUNTS(na, ns) ((na) > 0 && (na) <= OF_MAX_ADDR_CELLS && \

• 	(ns) > 0)
  

+#define OF_CHECK_COUNTS(na, ns, skip_ns) \

• 	((na) > 0 && (na) <= OF_MAX_ADDR_CELLS && \
  

• 	(skip_ns || (ns) > 0))
  
  

/* Debug utility */ #ifdef DEBUG @@ -1104,11 +1105,12 @@ static int of_translate_one(void * blob, int parent, struct of_bus *bus, static u64 __of_translate_address(void *blob, int node_offset, const fdt32_t *in_addr, const char *rprop) {

• int parent;
• struct of_bus *bus, *pbus;

• const fdt32_t *pranges; fdt32_t addr[OF_MAX_ADDR_CELLS];

• int na, ns, pna, pns;

• int na, ns, pna, pns, parent;

• struct of_bus *bus, *pbus; u64 result = OF_BAD_ADDR;

• bool skip_ns_check;

  debug("OF: ** translation for device %s **\n", fdt_get_name(blob, node_offset, NULL));

@@ -1119,9 +1121,16 @@ static u64 __of_translate_address(void *blob, int node_offset, const fdt32_t *in goto bail; bus = &of_busses[0];

• /* Cound address cells & copy address locally */

• /* Chek 'ns' only if parent provides 'ranges' property */
• pranges = fdt_getprop(blob, parent, rprop, NULL);
• if (pranges)

• skip_ns_check = false;
  

• else

• skip_ns_check = true;
  

• /* Count address cells & copy address locally */ bus->count_cells(blob, parent, &na, &ns);

• if (!OF_CHECK_COUNTS(na, ns)) {

• if (!OF_CHECK_COUNTS(na, ns, skip_ns_check)) { printf("%s: Bad cell count for %s\n", __FUNCTION__, fdt_get_name(blob, node_offset, NULL)); goto bail;

@@ -1148,7 +1157,7 @@ static u64 __of_translate_address(void *blob, int node_offset, const fdt32_t *in /* Get new parent bus and counts */ pbus = &of_busses[0]; pbus->count_cells(blob, parent, &pna, &pns);

• if (!OF_CHECK_COUNTS(pna, pns)) {
  

• if (!OF_CHECK_COUNTS(pna, pns, skip_ns_check)) {
  printf("%s: Bad cell count for %s\n",
  

__FUNCTION__, fdt_get_name(blob, node_offset, NULL)); break;

Reviewed-by: Lukasz Majewski l.majewski@samsung.com