From mboxrd@z Thu Jan 1 00:00:00 1970 From: Przemyslaw Marczak Date: Tue, 03 Nov 2015 10:57:33 +0100 Subject: [U-Boot] [PATCH] fdt: fix address cell count checking in fdt_translate_address() In-Reply-To: References: <1446043077-21005-1-git-send-email-p.marczak@samsung.com> Message-ID: <5638850D.2080205@samsung.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hello All, On 10/29/2015 06:15 PM, Simon Glass wrote: > Hi Stefan, > > On 28 October 2015 at 08:37, Przemyslaw Marczak wrote: >> Commit: dm: core: Enable optional use of fdt_translate_address() >> >> Enables use of this function as default, but after this it's not >> possible to get dev address for the case in which: '#size-cells == 0' >> >> This causes errors when getting address for some GPIOs, for which >> the '#size-cells' is set to 0. >> >> Example error: >> '__of_translate_address: Bad cell count for gpx0' >> >> Allowing for that case by modifying the macro 'OF_CHECK_COUNTS', >> (called from )__of_translate_address(), fixes the issue. >> >> Now, this macro doesn't check, that '#size-cells' is greater than 0. >> >> This is possible from the specification point of view, but I'm not sure >> that it doesn't introduce a regression for other configs. >> >> Please test and share the results. >> >> Tested-on: Odroid U3, Odroid X2, Odroid XU3, Sandbox. >> >> Signed-off-by: Przemyslaw Marczak >> Cc: Masahiro Yamada >> Cc: Lukasz Majewski >> Cc: Jaehoon Chung >> Cc: Stefan Roese >> Cc: Simon Glass >> Cc: Bin Meng >> Cc: Marek Vasut >> --- >> common/fdt_support.c | 7 +++---- >> 1 file changed, 3 insertions(+), 4 deletions(-) >> >> diff --git a/common/fdt_support.c b/common/fdt_support.c >> index f86365e..5f808cc 100644 >> --- a/common/fdt_support.c >> +++ b/common/fdt_support.c >> @@ -946,8 +946,7 @@ 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 ((u64)-1) >> -#define OF_CHECK_COUNTS(na, ns) ((na) > 0 && (na) <= OF_MAX_ADDR_CELLS && \ >> - (ns) > 0) >> +#define OF_CHECK_COUNTS(na) ((na) > 0 && (na) <= OF_MAX_ADDR_CELLS) >> >> /* Debug utility */ >> #ifdef DEBUG >> @@ -1115,7 +1114,7 @@ static u64 __of_translate_address(void *blob, int node_offset, const fdt32_t *in >> >> /* Cound address cells & copy address locally */ >> bus->count_cells(blob, parent, &na, &ns); >> - if (!OF_CHECK_COUNTS(na, ns)) { >> + if (!OF_CHECK_COUNTS(na)) { > > This seems to conflict with the comment at the top of this function: > > * 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 > > What should we do here? > Is that commit acceptable? I would like send V2 with removing the above comment. Best regards, -- Przemyslaw Marczak Samsung R&D Institute Poland Samsung Electronics p.marczak at samsung.com >> printf("%s: Bad cell count for %s\n", __FUNCTION__, >> fdt_get_name(blob, node_offset, NULL)); >> goto bail; >> @@ -1142,7 +1141,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)) { >> printf("%s: Bad cell count for %s\n", __FUNCTION__, >> fdt_get_name(blob, node_offset, NULL)); >> break; >> -- >> 1.9.1 >> > > Regards, > Simon >