From mboxrd@z Thu Jan 1 00:00:00 1970 From: Lukasz Majewski Date: Thu, 07 Jan 2016 14:03:52 +0100 Subject: [U-Boot] [PATCH] fdt: __of_translate_address(): check parent's 'ranges' before translate In-Reply-To: <1452166849-24461-1-git-send-email-p.marczak@samsung.com> References: <1452166849-24461-1-git-send-email-p.marczak@samsung.com> Message-ID: <20160107140352.284dbf2a@amdc2363> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 > Cc: Stefan Roese > Cc: Tom Rini > Cc: Simon Glass > Cc: Stephen Warren > Cc: Stephen Warren > --- > 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 -- Best regards, Lukasz Majewski Samsung R&D Institute Poland (SRPOL) | Linux Platform Group