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

Hi Simon,

On 16.2.2016 17:00, Simon Glass wrote:

Hi Michal,

On 15 February 2016 at 02:58, Michal Simek michal.simek@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@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...

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