[PATCH 0/2] Reading size-cells and address-cells from a node should walk up the

From: Matthias Brugger mbrugger@suse.com
parent stack to find the property. This is reflected through comments in the code, but actually not implemented. This leads to the fact that some DTBs worked by accident. Fix this by walking the tree when reading this properties.
After fixing the default size-cells returned when the property is not present this problem was made visible. When adding flash devices in qemu for arm64, the size-cells was wrongly inpterpreted and broke access to the second flash bank.
Matthias Brugger (2): libfdt: Make fdt_cells function accessible dm: core: Walk the tree to find address- and size-cells properties
drivers/core/ofnode.c | 8 +++---- include/dm/ofnode.h | 36 ++++++++++++++++++++++++++++++ include/dm/read.h | 6 ++--- scripts/dtc/libfdt/fdt_addresses.c | 2 +- scripts/dtc/libfdt/libfdt.h | 2 ++ 5 files changed, 45 insertions(+), 9 deletions(-)

From: Matthias Brugger mbrugger@suse.com
For reading address-cells and size-cells, the libfdt only provides functions which do not return in case the node does not provide the property. For traversing the DT to find the parent node which provides this property we will need to know that.
Make fdt_cells accessible from outside of libfdt so that we can detect not present size- and address-cells properties in a node.
Signed-off-by: Matthias Brugger mbrugger@suse.com ---
scripts/dtc/libfdt/fdt_addresses.c | 2 +- scripts/dtc/libfdt/libfdt.h | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/scripts/dtc/libfdt/fdt_addresses.c b/scripts/dtc/libfdt/fdt_addresses.c index 9a82cd0ba2..ead9460e95 100644 --- a/scripts/dtc/libfdt/fdt_addresses.c +++ b/scripts/dtc/libfdt/fdt_addresses.c @@ -11,7 +11,7 @@
#include "libfdt_internal.h"
-static int fdt_cells(const void *fdt, int nodeoffset, const char *name) +int fdt_cells(const void *fdt, int nodeoffset, const char *name) { const fdt32_t *c; uint32_t val; diff --git a/scripts/dtc/libfdt/libfdt.h b/scripts/dtc/libfdt/libfdt.h index fa63fffe28..b0eca12491 100644 --- a/scripts/dtc/libfdt/libfdt.h +++ b/scripts/dtc/libfdt/libfdt.h @@ -1121,6 +1121,8 @@ const char *fdt_stringlist_get(const void *fdt, int nodeoffset, */ #define FDT_MAX_NCELLS 4
+int fdt_cells(const void *fdt, int nodeoffset, const char *name); + /** * fdt_address_cells - retrieve address size for a bus represented in the tree * @fdt: pointer to the device tree blob

Hi Matthias,
On Wed, 8 Apr 2020 at 03:35, matthias.bgg@kernel.org wrote:
From: Matthias Brugger mbrugger@suse.com
For reading address-cells and size-cells, the libfdt only provides functions which do not return in case the node does not provide the property. For traversing the DT to find the parent node which provides this property we will need to know that.
Make fdt_cells accessible from outside of libfdt so that we can detect not present size- and address-cells properties in a node.
Signed-off-by: Matthias Brugger mbrugger@suse.com
scripts/dtc/libfdt/fdt_addresses.c | 2 +- scripts/dtc/libfdt/libfdt.h | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-)
Can you please send these patches upstream too?
Thanks, Simon

From: Matthias Brugger mbrugger@suse.com
Walk the tree when reading size-cells or address-cells properties.
Reported-by: Robin Randhawa Robin.Randhawa@ARM.com Signed-off-by: Matthias Brugger mbrugger@suse.com
---
drivers/core/ofnode.c | 8 ++++---- include/dm/ofnode.h | 36 ++++++++++++++++++++++++++++++++++++ include/dm/read.h | 6 ++---- 3 files changed, 42 insertions(+), 8 deletions(-)
diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c index 96a5dd20bd..5f23826b70 100644 --- a/drivers/core/ofnode.c +++ b/drivers/core/ofnode.c @@ -697,16 +697,16 @@ int ofnode_read_addr_cells(ofnode node) { if (ofnode_is_np(node)) return of_n_addr_cells(ofnode_to_np(node)); - else /* NOTE: this call should walk up the parent stack */ - return fdt_address_cells(gd->fdt_blob, ofnode_to_offset(node)); + else + return __ofnode_read_address_cells(node); }
int ofnode_read_size_cells(ofnode node) { if (ofnode_is_np(node)) return of_n_size_cells(ofnode_to_np(node)); - else /* NOTE: this call should walk up the parent stack */ - return fdt_size_cells(gd->fdt_blob, ofnode_to_offset(node)); + else + return __ofnode_read_size_cells(node); }
int ofnode_read_simple_addr_cells(ofnode node) diff --git a/include/dm/ofnode.h b/include/dm/ofnode.h index b5a50e8849..c6b768763d 100644 --- a/include/dm/ofnode.h +++ b/include/dm/ofnode.h @@ -660,6 +660,24 @@ int ofnode_read_pci_vendev(ofnode node, u16 *vendor, u16 *device); */ int ofnode_read_addr_cells(ofnode node);
+static inline int __ofnode_read_address_cells(ofnode node) +{ + /* NOTE: this call walks up the parent stack */ + int val = -FDT_ERR_NOTFOUND; + ofnode nd = node; + + while (val == -FDT_ERR_NOTFOUND) { + val = fdt_cells(gd->fdt_blob, ofnode_to_offset(nd), + "#address-cells"); + nd = ofnode_get_parent(nd); + } + + if (val == -FDT_ERR_NOTFOUND) + return OF_ROOT_NODE_ADDR_CELLS_DEFAULT; + else + return val; +} + /** * ofnode_read_size_cells() - Get the number of size cells for a node * @@ -671,6 +689,24 @@ int ofnode_read_addr_cells(ofnode node); */ int ofnode_read_size_cells(ofnode node);
+static inline int __ofnode_read_size_cells(ofnode node) +{ + /* NOTE: this call walks up the parent stack */ + int val = -FDT_ERR_NOTFOUND; + ofnode nd = node; + + while (val == -FDT_ERR_NOTFOUND) { + val = fdt_cells(gd->fdt_blob, ofnode_to_offset(nd), + "#size-cells"); + nd = ofnode_get_parent(nd); + } + + if (val == -FDT_ERR_NOTFOUND) + return OF_ROOT_NODE_SIZE_CELLS_DEFAULT; + else + return val; +} + /** * ofnode_read_simple_addr_cells() - Get the address cells property in a node * diff --git a/include/dm/read.h b/include/dm/read.h index da8c7f25e7..0302c7bffb 100644 --- a/include/dm/read.h +++ b/include/dm/read.h @@ -789,14 +789,12 @@ static inline int dev_count_phandle_with_args(const struct udevice *dev,
static inline int dev_read_addr_cells(const struct udevice *dev) { - /* NOTE: this call should walk up the parent stack */ - return fdt_address_cells(gd->fdt_blob, dev_of_offset(dev)); + return __ofnode_read_address_cells(dev_ofnode(dev)); }
static inline int dev_read_size_cells(const struct udevice *dev) { - /* NOTE: this call should walk up the parent stack */ - return fdt_size_cells(gd->fdt_blob, dev_of_offset(dev)); + return __ofnode_read_size_cells(dev_ofnode(dev)); }
static inline int dev_read_simple_addr_cells(const struct udevice *dev)

Hi Matthias,
On Wed, 8 Apr 2020 at 03:35, matthias.bgg@kernel.org wrote:
From: Matthias Brugger mbrugger@suse.com
Walk the tree when reading size-cells or address-cells properties.
Reported-by: Robin Randhawa Robin.Randhawa@ARM.com Signed-off-by: Matthias Brugger mbrugger@suse.com
drivers/core/ofnode.c | 8 ++++---- include/dm/ofnode.h | 36 ++++++++++++++++++++++++++++++++++++ include/dm/read.h | 6 ++---- 3 files changed, 42 insertions(+), 8 deletions(-)
Please can you send these upstream? I think you'll need to add a test when doing that, too.
Regards, Simon

Hi Simon,
On 4/9/20 6:25 PM, Simon Glass wrote:
Hi Matthias,
On Wed, 8 Apr 2020 at 03:35, matthias.bgg@kernel.org wrote:
From: Matthias Brugger mbrugger@suse.com
Walk the tree when reading size-cells or address-cells properties.
Reported-by: Robin Randhawa Robin.Randhawa@ARM.com Signed-off-by: Matthias Brugger mbrugger@suse.com
drivers/core/ofnode.c | 8 ++++---- include/dm/ofnode.h | 36 ++++++++++++++++++++++++++++++++++++ include/dm/read.h | 6 ++---- 3 files changed, 42 insertions(+), 8 deletions(-)
Please can you send these upstream? I think you'll need to add a test when doing that, too.
What is the upstream project for drivers/core/ofnode.c? I wasn't able to find it.
Regards, Matthias

Hi Matthias,
On Sat, 11 Apr 2020 at 12:05, Matthias Brugger matthias.bgg@gmail.com wrote:
Hi Simon,
On 4/9/20 6:25 PM, Simon Glass wrote:
Hi Matthias,
On Wed, 8 Apr 2020 at 03:35, matthias.bgg@kernel.org wrote:
From: Matthias Brugger mbrugger@suse.com
Walk the tree when reading size-cells or address-cells properties.
Reported-by: Robin Randhawa Robin.Randhawa@ARM.com Signed-off-by: Matthias Brugger mbrugger@suse.com
drivers/core/ofnode.c | 8 ++++---- include/dm/ofnode.h | 36 ++++++++++++++++++++++++++++++++++++ include/dm/read.h | 6 ++---- 3 files changed, 42 insertions(+), 8 deletions(-)
Please can you send these upstream? I think you'll need to add a test when doing that, too.
What is the upstream project for drivers/core/ofnode.c? I wasn't able to find it.
Sorry I meant the other patch...
This one:
Reviewed-by: Simon Glass sjg@chromium.org
Regards, Simon

Hi Matthias.
Thanks for this.
I can confirm that this fixes the reported problem. You have my Tested-By. FWIW, the changes look fine to me too.
Cheers, Robin
participants (4)
-
Matthias Brugger
-
matthias.bgg@kernel.org
-
Robin Randhawa
-
Simon Glass