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

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

[matthias.bgg at kernel.org]

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

-- 
2.26.0

[PATCH 1/2] libfdt: Make fdt_cells function accessible

[matthias.bgg at kernel.org]

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
-- 
2.26.0

[PATCH 1/2] libfdt: Make fdt_cells function accessible

[Simon Glass]

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

[PATCH 2/2] dm: core: Walk the tree to find address- and size-cells properties

[matthias.bgg at kernel.org]

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

[PATCH 2/2] dm: core: Walk the tree to find address- and size-cells properties

[Simon Glass]

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

[PATCH 2/2] dm: core: Walk the tree to find address- and size-cells properties

[Matthias Brugger]

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

[PATCH 2/2] dm: core: Walk the tree to find address- and size-cells properties

[Simon Glass]

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

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

[Robin Randhawa]

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