fdt: Try to read #address-cells/size-cells from parent

Read #address-cells and #size-cells from parent if they are not present in current node.

Signed-off-by Michal Simek <[email protected]>

---

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(-)

lib/libfdt/fdt_addresses.c +8 -3

index 76054d98e5fd..b164d0988079 100644
@@ -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)

Hi Simon,

Quoted 56 lines · Michal Simek

Read #address-cells and #size-cells from parent if they are not present in current node.

Signed-off-by Michal Simek <[email protected]>

---

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(-)

lib/libfdt/fdt_addresses.c +14 -4

index 76054d98e5fd..b164d0988079 100644
@@ -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;
+     }
 

lib/libfdt/fdt_addresses.c int fdt_size_cells(const void *fdt, int nodeoffset) · line 58

Quoted 3 lines

lib/libfdt/fdt_addresses.c +0 -0

      if (len != sizeof(*sc))
              return -FDT_ERR_BADNCELLS;

Simon: Any comment?

Hi Michal,

Quoted 59 lines · Michal Simek

Hi Simon,

Quoted 56 lines · Michal Simek

Read #address-cells and #size-cells from parent if they are not present in current node.

Signed-off-by Michal Simek <[email protected]>

---

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(-)

lib/libfdt/fdt_addresses.c +14 -4

index 76054d98e5fd..b164d0988079 100644
@@ -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;
+     }
 

Quoted 5 lines

lib/libfdt/fdt_addresses.c int fdt_size_cells(const void *fdt, int nodeoffset) · line 58

Quoted 3 lines

lib/libfdt/fdt_addresses.c +0 -0

      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?

Hi Simon,

Quoted 68 lines · Simon Glass

Hi Michal,

Quoted 59 lines · Michal Simek

Hi Simon,

Quoted 56 lines · Michal Simek

Read #address-cells and #size-cells from parent if they are not present in current node.

Signed-off-by Michal Simek <[email protected]>

---

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(-)

lib/libfdt/fdt_addresses.c +14 -4

index 76054d98e5fd..b164d0988079 100644
@@ -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;
+     }
 

Quoted 5 lines

lib/libfdt/fdt_addresses.c int fdt_size_cells(const void *fdt, int nodeoffset) · line 58

Quoted 3 lines

lib/libfdt/fdt_addresses.c +0 -0

      if (len != sizeof(*sc))
              return -FDT_ERR_BADNCELLS;

Simon: Any comment?

Quoted 3 lines

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

Hi Michal,

Quoted 90 lines · Michal Simek

Hi Simon,

Quoted 68 lines · Simon Glass

Hi Michal,

Quoted 59 lines · Michal Simek

Hi Simon,

Quoted 56 lines · Michal Simek

Read #address-cells and #size-cells from parent if they are not present in current node.

Signed-off-by Michal Simek <[email protected]>

---

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(-)

lib/libfdt/fdt_addresses.c +14 -4

index 76054d98e5fd..b164d0988079 100644
@@ -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;
+     }
 

Quoted 5 lines

lib/libfdt/fdt_addresses.c int fdt_size_cells(const void *fdt, int nodeoffset) · line 58

Quoted 3 lines

lib/libfdt/fdt_addresses.c +0 -0

      if (len != sizeof(*sc))
              return -FDT_ERR_BADNCELLS;

Simon: Any comment?

Quoted 3 lines

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>;
        };

Quoted 7 lines

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.

I think this should go in a higher-level function. I very much doubt that this patch would be accepted upstream.

Can you find the caller and make it call this function again (for the parent) when no nothing is found on the first call? Hopefully this caller will have access to the parent node and will not need to call fdt_parent_offset().

Quoted 104 lines · Simon Glass

Hi Michal,

Quoted 90 lines · Michal Simek

Hi Simon,

Quoted 68 lines · Simon Glass

Hi Michal,

Quoted 59 lines · Michal Simek

Hi Simon,

Quoted 56 lines · Michal Simek

Read #address-cells and #size-cells from parent if they are not present in current node.

Signed-off-by Michal Simek <[email protected]>

---

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(-)

lib/libfdt/fdt_addresses.c +14 -4

index 76054d98e5fd..b164d0988079 100644
@@ -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;
+     }
 

Quoted 5 lines

lib/libfdt/fdt_addresses.c int fdt_size_cells(const void *fdt, int nodeoffset) · line 58

Quoted 3 lines

lib/libfdt/fdt_addresses.c +0 -0

      if (len != sizeof(*sc))
              return -FDT_ERR_BADNCELLS;

Simon: Any comment?

Quoted 3 lines

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>;
        };

Quoted 7 lines

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.

I think this should go in a higher-level function. I very much doubt that this patch would be accepted upstream.

Quoted 4 lines

Can you find the caller and make it call this function again (for the parent) when no nothing is found on the first call? Hopefully this caller will have access to the parent node and will not need to call fdt_parent_offset().

The funny part is that nothing is found means return 2. If this returns something <0 then there is not a problem to try it with parents.

Quoted 105 lines · Michal Simek

Quoted 104 lines · Simon Glass

Hi Michal,

Quoted 90 lines · Michal Simek

Hi Simon,

Quoted 68 lines · Simon Glass

Hi Michal,

Quoted 59 lines · Michal Simek

Hi Simon,

Quoted 56 lines · Michal Simek

Read #address-cells and #size-cells from parent if they are not present in current node.

Signed-off-by Michal Simek <[email protected]>

---

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(-)

lib/libfdt/fdt_addresses.c +14 -4

index 76054d98e5fd..b164d0988079 100644
@@ -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;
+     }
 

Quoted 5 lines

lib/libfdt/fdt_addresses.c int fdt_size_cells(const void *fdt, int nodeoffset) · line 58

Quoted 3 lines

lib/libfdt/fdt_addresses.c +0 -0

      if (len != sizeof(*sc))
              return -FDT_ERR_BADNCELLS;

Simon: Any comment?

Quoted 3 lines

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>;
        };

Quoted 7 lines

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.

I think this should go in a higher-level function. I very much doubt that this patch would be accepted upstream.

Quoted 7 lines

Quoted 4 lines

Can you find the caller and make it call this function again (for the parent) when no nothing is found on the first call? Hopefully this caller will have access to the parent node and will not need to call fdt_parent_offset().

The funny part is that nothing is found means return 2. If this returns something <0 then there is not a problem to try it with parents.

I don't have the full context of this thread, so it's a bit hard to be sure, but this doesn't look right from what I can see. Two things to remember here:

* #address-cells and #size-cells describe the format of addresses for children of this node, not this node itself. So if you're looking to parse 'reg' for this node, you *always* need to look at the parent, not just as a fallback.

* #address-cells and #size-cells are *not* inherited. If they're missing in a node, then the format for its children's addresses is 2 cell addresses and 2 cell sizes, it is *not* correct to look at the next parent up for these properties.

Hi David,

Quoted 114 lines · David Gibson

Quoted 105 lines · Michal Simek

Quoted 104 lines · Simon Glass

Hi Michal,

Quoted 90 lines · Michal Simek

Hi Simon,

Quoted 68 lines · Simon Glass

Hi Michal,

Quoted 59 lines · Michal Simek

Hi Simon,

Quoted 56 lines · Michal Simek

Read #address-cells and #size-cells from parent if they are not present in current node.

Signed-off-by Michal Simek <[email protected]>

---

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(-)

lib/libfdt/fdt_addresses.c +14 -4

index 76054d98e5fd..b164d0988079 100644
@@ -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;
+     }
 

Quoted 5 lines

lib/libfdt/fdt_addresses.c int fdt_size_cells(const void *fdt, int nodeoffset) · line 58

Quoted 3 lines

lib/libfdt/fdt_addresses.c +0 -0

      if (len != sizeof(*sc))
              return -FDT_ERR_BADNCELLS;

Simon: Any comment?

Quoted 3 lines

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>;
        };

Quoted 7 lines

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.

I think this should go in a higher-level function. I very much doubt that this patch would be accepted upstream.

Quoted 7 lines

Quoted 4 lines

Can you find the caller and make it call this function again (for the parent) when no nothing is found on the first call? Hopefully this caller will have access to the parent node and will not need to call fdt_parent_offset().

The funny part is that nothing is found means return 2. If this returns something <0 then there is not a problem to try it with parents.

Quoted 8 lines

I don't have the full context of this thread, so it's a bit hard to be sure, but this doesn't look right from what I can see. Two things to remember here:

* #address-cells and #size-cells describe the format of addresses for children of this node, not this node itself. So if you're looking to parse 'reg' for this node, you *always* need to look at the parent, not just as a fallback.

ok that means that I should fix my code to find parent of current node and then read address and size cells.

fdt - actual memory node
parent = fdt_parent_offset(fdt, nodeoffset); address_cells = fdt_address_cells(parent, nodeoffset); size_cells = fdt_size_cells(parent, nodeoffset);

Quoted 5 lines

* #address-cells and #size-cells are *not* inherited. If they're missing in a node, then the format for its children's addresses is 2 cell addresses and 2 cell sizes, it is *not* correct to look at the next parent up for these properties.

ok. And I expect that this is in spec.

Definitely thank you for your input it was very helpful.

Thanks,
Michal

Quoted 126 lines · Michal Simek

Hi David,

Quoted 114 lines · David Gibson

Quoted 105 lines · Michal Simek

Quoted 104 lines · Simon Glass

Hi Michal,

Quoted 90 lines · Michal Simek

Hi Simon,

Quoted 68 lines · Simon Glass

Hi Michal,

Quoted 59 lines · Michal Simek

Hi Simon,

Quoted 56 lines · Michal Simek

Read #address-cells and #size-cells from parent if they are not present in current node.

Signed-off-by Michal Simek <[email protected]>

---

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(-)

lib/libfdt/fdt_addresses.c +14 -4

index 76054d98e5fd..b164d0988079 100644
@@ -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;
+     }
 

Quoted 5 lines

lib/libfdt/fdt_addresses.c int fdt_size_cells(const void *fdt, int nodeoffset) · line 58

Quoted 3 lines

lib/libfdt/fdt_addresses.c +0 -0

      if (len != sizeof(*sc))
              return -FDT_ERR_BADNCELLS;

Simon: Any comment?

Quoted 3 lines

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>;
        };

Quoted 7 lines

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.

I think this should go in a higher-level function. I very much doubt that this patch would be accepted upstream.

Quoted 7 lines

Quoted 4 lines

Can you find the caller and make it call this function again (for the parent) when no nothing is found on the first call? Hopefully this caller will have access to the parent node and will not need to call fdt_parent_offset().

The funny part is that nothing is found means return 2. If this returns something <0 then there is not a problem to try it with parents.

Quoted 8 lines

I don't have the full context of this thread, so it's a bit hard to be sure, but this doesn't look right from what I can see. Two things to remember here:

* #address-cells and #size-cells describe the format of addresses for children of this node, not this node itself. So if you're looking to parse 'reg' for this node, you *always* need to look at the parent, not just as a fallback.

Quoted 7 lines

ok that means that I should fix my code to find parent of current node and then read address and size cells.

fdt - actual memory node
parent = fdt_parent_offset(fdt, nodeoffset); address_cells = fdt_address_cells(parent, nodeoffset); size_cells = fdt_size_cells(parent, nodeoffset);

That's correct. One way to look at it that #address-cells and #size-cells are properties of the bus anchored at this node, rather than properties of the node itself.

Quoted 6 lines

Quoted 5 lines

* #address-cells and #size-cells are *not* inherited. If they're missing in a node, then the format for its children's addresses is 2 cell addresses and 2 cell sizes, it is *not* correct to look at the next parent up for these properties.

Quoted 1 line

ok. And I expect that this is in spec.

Yes. Actually relying on the default values is discouraged, of
course: you should include #address-cells and #size-cells for any node with children.