[U-Boot] [PATCH 1/3] fix: fdtdec: allow parse 'reg' property with zero value in '#size-cells'

[Przemyslaw Marczak <p.marczak@samsung.com>]

Hello Stephen,

On 09/24/2015 07:14 PM, Stephen Warren wrote:
> On 09/24/2015 09:29 AM, Przemyslaw Marczak wrote:
>> After rework of lib/fdtdec.c by commit:
>>
>> commit 02464e386bb5f0a022c121f95ae75cf583759d95
>> Author: Stephen Warren <swarren@nvidia.com>
>> Date:   Thu Aug 6 15:31:02 2015 -0600
>
> That'd usually be abbreviated as:
>
> Commit 02464e386bb5 "fdt: add new fdt address parsing functions".

Ok, I will update the commit message.

>
> Of course, if you want to shame me that's justified too:-) Tracking down
> regressions sucks:-(
>

Oh no no... maybe a little :)

>> the function fdtdec_get_addr() doesn't work as previous,
>> because the implementation assumes that properties '#address-cells'
>> and '#size-cells' are equal to 1, which can be not true sometimes.
>
> "are equal to" should be "is at least"; the purpose of that rework was
> to support values greater than one.
>

But it describe the fdtdec_get_addr(), which calls

fdtdec_get_addr_size_fixed(...)

and for this call we have:

na = sizeof(fdt_addr_t) / sizeof(fdt32_t) == 1

ns = sizeof(fdt_size_t) / sizeof(fdt32_t) == 1

This is consistent with the description for this function in 
include/fdtdec.h.

>> The new API introduced fdtdec_get_addr_size_auto_parent() for the 'reg'
>> property parsing, but the implementation assumes, that #size-cells
>> can't be less than 1.
>>
>> This causes that the following children's 'reg' property can't be
>> reached:
>>
>> parent at 0x0 {
>>       #address-cells = <1>;
>>       #size-cells = <0>;
>>       children at 0x100 {
>>           reg = < 0x100 >;
>>       };
>> };
>>
>> Change the condition value from '1' to '0', which allows parsing property
>> with at least zero #size-cells, fixes the issue.
>>
>> Now, fdtdec_get_addr_size_auto_parent() works properly.
>
> Sorry about that. This patch,

Don't worry, no one is infallible :)

>
> Acked-by: Stephen Warren <swarren@nvidia.com>
>
> (but not tested, but since this allows a previously failing case, it's
> hard to see how this patch could cause any problems.)
>

This just fixes the problem, which I noticed, but it looks, that it 
shouldn't break other things.

Best regards,
-- 
Przemyslaw Marczak
Samsung R&D Institute Poland
Samsung Electronics
p.marczak at samsung.com