[U-Boot] [PATCH] fdt: Try to read #address-cells/size-cells from parent

[Michal Simek]

Hi Simon,

On 16.2.2016 17:00, Simon Glass wrote:
> Hi Michal,
> 
> On 15 February 2016 at 02:58, Michal Simek <michal.simek at xilinx.com> wrote:
>> Hi Simon,
>>
>> On 10.2.2016 13:04, Michal Simek wrote:
>>> Read #address-cells and #size-cells from parent if they are not present in
>>> current node.
>>>
>>> Signed-off-by: Michal Simek <michal.simek at xilinx.com>
>>> ---
>>>
>>> I have code which read information about memory for zynqmp but memory
>>> node most of the time doesn't contain #address/size-cells which are
>>> present in parent node.
>>> That's why let's try to read it from parent.
>>>
>>> Also I think that we shouldn't return 2 if property is not found because
>>> it has side effect on 32bit systems with #address/size-cells = <1>;
>>>
>>> ---
>>>  lib/libfdt/fdt_addresses.c | 18 ++++++++++++++----
>>>  1 file changed, 14 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/lib/libfdt/fdt_addresses.c b/lib/libfdt/fdt_addresses.c
>>> index 76054d98e5fd..b164d0988079 100644
>>> --- a/lib/libfdt/fdt_addresses.c
>>> +++ b/lib/libfdt/fdt_addresses.c
>>> @@ -19,10 +19,15 @@ int fdt_address_cells(const void *fdt, int nodeoffset)
>>>       const fdt32_t *ac;
>>>       int val;
>>>       int len;
>>> +     int parent;
>>>
>>>       ac = fdt_getprop(fdt, nodeoffset, "#address-cells", &len);
>>> -     if (!ac)
>>> -             return 2;
>>> +     if (!ac) {
>>> +             parent = fdt_parent_offset(fdt, nodeoffset);
>>> +             ac = fdt_getprop(fdt, parent, "#address-cells", &len);
>>> +             if (!ac)
>>> +                     return 2;
>>> +     }
>>>
>>>       if (len != sizeof(*ac))
>>>               return -FDT_ERR_BADNCELLS;
>>> @@ -39,10 +44,15 @@ int fdt_size_cells(const void *fdt, int nodeoffset)
>>>       const fdt32_t *sc;
>>>       int val;
>>>       int len;
>>> +     int parent;
>>>
>>>       sc = fdt_getprop(fdt, nodeoffset, "#size-cells", &len);
>>> -     if (!sc)
>>> -             return 2;
>>> +     if (!sc) {
>>> +             parent = fdt_parent_offset(fdt, nodeoffset);
>>> +             sc = fdt_getprop(fdt, parent, "#size-cells", &len);
>>> +             if (!sc)
>>> +                     return 2;
>>> +     }
>>>
>>>       if (len != sizeof(*sc))
>>>               return -FDT_ERR_BADNCELLS;
>>>
>>
>> Simon: Any comment?
> 
> It seems risky to change the behaviour here. Also fdt_parent_offset() is slow.
> 
> Can you point me to the binding / example DT that you are trying to parse?

Look at dram_init(), etc.
https://github.com/Xilinx/u-boot-xlnx/blob/master/board/xilinx/zynqmp/zynqmp.c

fdt_get_reg() is calling fdt_size_cells()


And this is DTS fragment.
	#address-cells = <2>;
	#size-cells = <1>;

	memory {
		device_type = "memory";
		reg = <0x0 0x0 0x80000000>, <0x8 0x00000000 0x80000000>;
	};

Code is in memory node I need to work with and asking for size-cells.
Current code returns 2 instead of error and the rest of code just works
with size = 2 which is incorrect for this setup.

I have already changed size-cells = 2 in our repo because I need to
support for more than 4GB memory anyway but this should point to the
problem in that generic functions.

Thanks,
Michal