Hi, On 3 November 2015 at 02:57, Przemyslaw Marczak <p.marc...@samsung.com> wrote: > Hello All, > > > On 10/29/2015 06:15 PM, Simon Glass wrote: >> >> Hi Stefan, >> >> On 28 October 2015 at 08:37, Przemyslaw Marczak <p.marc...@samsung.com> >> 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 <p.marc...@samsung.com> >>> Cc: Masahiro Yamada <yamada.masah...@socionext.com> >>> Cc: Lukasz Majewski <l.majew...@samsung.com> >>> Cc: Jaehoon Chung <jh80.ch...@samsung.com> >>> Cc: Stefan Roese <s...@denx.de> >>> Cc: Simon Glass <s...@chromium.org> >>> Cc: Bin Meng <bmeng...@gmail.com> >>> Cc: Marek Vasut <ma...@denx.de> >>> --- >>> 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.
That's what I am worried about. Presumably the comment is accurate today and this check has some value. I was hoping Stefan might know. > > Best regards, > -- > Przemyslaw Marczak > Samsung R&D Institute Poland > Samsung Electronics > p.marc...@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 >> > Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot