diff mbox series

[1/1] dm: fix ofnode_read_addr/size_cells()

Message ID 20200725193849.48289-1-xypron.glpk@gmx.de
State Accepted
Commit ae6b33dcc342e8539f4f69aba238748e9e89280a
Delegated to: Simon Glass
Headers show
Series [1/1] dm: fix ofnode_read_addr/size_cells() | expand

Commit Message

Heinrich Schuchardt July 25, 2020, 7:38 p.m. UTC
In the case of the live tree ofnode_read_addr_cells() and
ofnode_read_size_cells() return the #address-cells and #size-cells defined
in the parent node. With the patch the same is done for a non-live tree.

The only consumer of these functions is currently the CFI flash driver.

This patch fixes the incorrect parsing of the device tree leading to
'saveenv' failing on qemu_arm64_defconfig.

For testing qemu-system-aarch64 has to be called with

    -drive if=pflash,format=raw,index=1,file=envstore.img

to provide the flash memory. envstore.img must be 64 MiB large.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 drivers/core/ofnode.c | 20 ++++++++++++++------
 include/dm/read.h     | 10 ++++++----
 2 files changed, 20 insertions(+), 10 deletions(-)

--
2.27.0

Comments

Simon Glass July 27, 2020, 11:17 p.m. UTC | #1
Hi Heinrich,

On Sat, 25 Jul 2020 at 13:39, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> In the case of the live tree ofnode_read_addr_cells() and
> ofnode_read_size_cells() return the #address-cells and #size-cells defined
> in the parent node. With the patch the same is done for a non-live tree.
>
> The only consumer of these functions is currently the CFI flash driver.
>
> This patch fixes the incorrect parsing of the device tree leading to
> 'saveenv' failing on qemu_arm64_defconfig.
>
> For testing qemu-system-aarch64 has to be called with
>
>     -drive if=pflash,format=raw,index=1,file=envstore.img
>
> to provide the flash memory. envstore.img must be 64 MiB large.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
>  drivers/core/ofnode.c | 20 ++++++++++++++------
>  include/dm/read.h     | 10 ++++++----
>  2 files changed, 20 insertions(+), 10 deletions(-)
>

Please can we have a test for this in test/dm/ofnode.c ?

Regards,
Simon
Stefan Roese July 30, 2020, 2:58 p.m. UTC | #2
On 25.07.20 21:38, Heinrich Schuchardt wrote:
> In the case of the live tree ofnode_read_addr_cells() and
> ofnode_read_size_cells() return the #address-cells and #size-cells defined
> in the parent node. With the patch the same is done for a non-live tree.
> 
> The only consumer of these functions is currently the CFI flash driver.
> 
> This patch fixes the incorrect parsing of the device tree leading to
> 'saveenv' failing on qemu_arm64_defconfig.
> 
> For testing qemu-system-aarch64 has to be called with
> 
>      -drive if=pflash,format=raw,index=1,file=envstore.img
> 
> to provide the flash memory. envstore.img must be 64 MiB large.
> 
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

> ---
>   drivers/core/ofnode.c | 20 ++++++++++++++------
>   include/dm/read.h     | 10 ++++++----
>   2 files changed, 20 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c
> index c37afa1fe6..d02d8d33fe 100644
> --- a/drivers/core/ofnode.c
> +++ b/drivers/core/ofnode.c
> @@ -776,18 +776,26 @@ int ofnode_read_pci_vendev(ofnode node, u16 *vendor, u16 *device)
> 
>   int ofnode_read_addr_cells(ofnode node)
>   {
> -	if (ofnode_is_np(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 {
> +		int parent = fdt_parent_offset(gd->fdt_blob,
> +					       ofnode_to_offset(node));
> +
> +		return fdt_address_cells(gd->fdt_blob, parent);
> +	}
>   }
> 
>   int ofnode_read_size_cells(ofnode node)
>   {
> -	if (ofnode_is_np(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 {
> +		int parent = fdt_parent_offset(gd->fdt_blob,
> +					       ofnode_to_offset(node));
> +
> +		return fdt_size_cells(gd->fdt_blob, parent);
> +	}
>   }
> 
>   int ofnode_read_simple_addr_cells(ofnode node)
> diff --git a/include/dm/read.h b/include/dm/read.h
> index f02ec95954..3c7350beef 100644
> --- a/include/dm/read.h
> +++ b/include/dm/read.h
> @@ -875,14 +875,16 @@ 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));
> +	int parent = fdt_parent_offset(gd->fdt_blob, dev_of_offset(dev));
> +
> +	return fdt_address_cells(gd->fdt_blob, parent);
>   }
> 
>   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));
> +	int parent = fdt_parent_offset(gd->fdt_blob, dev_of_offset(dev));
> +
> +	return fdt_size_cells(gd->fdt_blob, parent);
>   }
> 
>   static inline int dev_read_simple_addr_cells(const struct udevice *dev)
> --
> 2.27.0
> 


Viele Grüße,
Stefan
Simon Glass Aug. 22, 2020, 11:18 p.m. UTC | #3
On 25.07.20 21:38, Heinrich Schuchardt wrote:
> In the case of the live tree ofnode_read_addr_cells() and
> ofnode_read_size_cells() return the #address-cells and #size-cells defined
> in the parent node. With the patch the same is done for a non-live tree.
>
> The only consumer of these functions is currently the CFI flash driver.
>
> This patch fixes the incorrect parsing of the device tree leading to
> 'saveenv' failing on qemu_arm64_defconfig.
>
> For testing qemu-system-aarch64 has to be called with
>
>      -drive if=pflash,format=raw,index=1,file=envstore.img
>
> to provide the flash memory. envstore.img must be 64 MiB large.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

> ---
>   drivers/core/ofnode.c | 20 ++++++++++++++------
>   include/dm/read.h     | 10 ++++++----
>   2 files changed, 20 insertions(+), 10 deletions(-)
>
Applied to u-boot-dm, thanks!
diff mbox series

Patch

diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c
index c37afa1fe6..d02d8d33fe 100644
--- a/drivers/core/ofnode.c
+++ b/drivers/core/ofnode.c
@@ -776,18 +776,26 @@  int ofnode_read_pci_vendev(ofnode node, u16 *vendor, u16 *device)

 int ofnode_read_addr_cells(ofnode node)
 {
-	if (ofnode_is_np(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 {
+		int parent = fdt_parent_offset(gd->fdt_blob,
+					       ofnode_to_offset(node));
+
+		return fdt_address_cells(gd->fdt_blob, parent);
+	}
 }

 int ofnode_read_size_cells(ofnode node)
 {
-	if (ofnode_is_np(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 {
+		int parent = fdt_parent_offset(gd->fdt_blob,
+					       ofnode_to_offset(node));
+
+		return fdt_size_cells(gd->fdt_blob, parent);
+	}
 }

 int ofnode_read_simple_addr_cells(ofnode node)
diff --git a/include/dm/read.h b/include/dm/read.h
index f02ec95954..3c7350beef 100644
--- a/include/dm/read.h
+++ b/include/dm/read.h
@@ -875,14 +875,16 @@  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));
+	int parent = fdt_parent_offset(gd->fdt_blob, dev_of_offset(dev));
+
+	return fdt_address_cells(gd->fdt_blob, parent);
 }

 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));
+	int parent = fdt_parent_offset(gd->fdt_blob, dev_of_offset(dev));
+
+	return fdt_size_cells(gd->fdt_blob, parent);
 }

 static inline int dev_read_simple_addr_cells(const struct udevice *dev)