[PATCH 1/2] of: Return copy of live tree as fixed tree

[PATCH 1/2] of: Return copy of live tree as fixed tree

[Sascha Hauer]

the of fixups modify the the tree passed to them. It is undesired that
these modify the live tree, so when we are requested to return the fixed
live tree then make a copy first.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/oftree.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/common/oftree.c b/common/oftree.c
index 075b9d6b8b..4868d8812e 100644
--- a/common/oftree.c
+++ b/common/oftree.c
@@ -325,6 +325,9 @@ struct fdt_header *of_get_fixed_tree(struct device_node *node)
 		node = of_get_root_node();
 		if (!node)
 			return NULL;
+		node = of_copy_node(NULL, node);
+		if (!node)
+			return NULL;
 	}
 
 	ret = of_fix_tree(node);
-- 
2.29.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 2/2] reboot-mode: drop now unnecessary check

[Sascha Hauer]

The reboot mode fixup tests if it is the live tree before modifying
anything. This is unnecessary now as the live tree is never passed
to fixups anymore.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/power/reset/reboot-mode.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/power/reset/reboot-mode.c b/drivers/power/reset/reboot-mode.c
index df3be03edf..d9b51dfea6 100644
--- a/drivers/power/reset/reboot-mode.c
+++ b/drivers/power/reset/reboot-mode.c
@@ -71,10 +71,6 @@ static int of_reboot_mode_fixup(struct device_node *root, void *ctx)
 	srcnp = reboot->dev->device_node;
 	dstnp = of_get_node_by_reproducible_name(root, srcnp);
 
-	/* nothing to do when called on barebox-internal tree */
-	if (srcnp == dstnp)
-		return 0;
-
 	if (dstnp) {
 		dstparent = dstnp->parent;
 		of_delete_node(dstnp);
-- 
2.29.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 1/2] of: Return copy of live tree as fixed tree

[Ahmad Fatoum]

Hi,

On 22.03.21 12:24, Sascha Hauer wrote:
> the of fixups modify the the tree passed to them. It is undesired that
> these modify the live tree, so when we are requested to return the fixed
> live tree then make a copy first.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  common/oftree.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/common/oftree.c b/common/oftree.c
> index 075b9d6b8b..4868d8812e 100644
> --- a/common/oftree.c
> +++ b/common/oftree.c
> @@ -325,6 +325,9 @@ struct fdt_header *of_get_fixed_tree(struct device_node *node)
>  		node = of_get_root_node();
>  		if (!node)
>  			return NULL;
> +		node = of_copy_node(NULL, node);
> +		if (!node)
> +			return NULL;

Shouldn't this be freed somewhere?

>  	}
>  
>  	ret = of_fix_tree(node);
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 2/2] reboot-mode: drop now unnecessary check

[Ahmad Fatoum]

On 22.03.21 12:24, Sascha Hauer wrote:
> The reboot mode fixup tests if it is the live tree before modifying
> anything. This is unnecessary now as the live tree is never passed
> to fixups anymore.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>

Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

> ---
>  drivers/power/reset/reboot-mode.c | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/drivers/power/reset/reboot-mode.c b/drivers/power/reset/reboot-mode.c
> index df3be03edf..d9b51dfea6 100644
> --- a/drivers/power/reset/reboot-mode.c
> +++ b/drivers/power/reset/reboot-mode.c
> @@ -71,10 +71,6 @@ static int of_reboot_mode_fixup(struct device_node *root, void *ctx)
>  	srcnp = reboot->dev->device_node;
>  	dstnp = of_get_node_by_reproducible_name(root, srcnp);
>  
> -	/* nothing to do when called on barebox-internal tree */
> -	if (srcnp == dstnp)
> -		return 0;
> -
>  	if (dstnp) {
>  		dstparent = dstnp->parent;
>  		of_delete_node(dstnp);
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 1/2] of: Return copy of live tree as fixed tree

[Sascha Hauer]

On Mon, Mar 22, 2021 at 12:48:58PM +0100, Ahmad Fatoum wrote:
> Hi,
> 
> On 22.03.21 12:24, Sascha Hauer wrote:
> > the of fixups modify the the tree passed to them. It is undesired that
> > these modify the live tree, so when we are requested to return the fixed
> > live tree then make a copy first.
> > 
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > ---
> >  common/oftree.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/common/oftree.c b/common/oftree.c
> > index 075b9d6b8b..4868d8812e 100644
> > --- a/common/oftree.c
> > +++ b/common/oftree.c
> > @@ -325,6 +325,9 @@ struct fdt_header *of_get_fixed_tree(struct device_node *node)
> >  		node = of_get_root_node();
> >  		if (!node)
> >  			return NULL;
> > +		node = of_copy_node(NULL, node);
> > +		if (!node)
> > +			return NULL;
> 
> Shouldn't this be freed somewhere?

Hm, you're right. Somehow I had the assumption this is freed by the
caller, but it's not the device_node * that is returned here.

-------------------------------8<------------------------------------

>From 9408403cac204ffd46742f665b97de381eec3b99 Mon Sep 17 00:00:00 2001
From: Sascha Hauer <s.hauer@pengutronix.de>
Date: Mon, 22 Mar 2021 12:20:26 +0100
Subject: [PATCH 1/2] of: Return copy of live tree as fixed tree

the of fixups modify the the tree passed to them. It is undesired that
these modify the live tree, so when we are requested to return the fixed
live tree then make a copy first.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/oftree.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/common/oftree.c b/common/oftree.c
index 075b9d6b8b..60d4327155 100644
--- a/common/oftree.c
+++ b/common/oftree.c
@@ -319,21 +319,27 @@ int of_fix_tree(struct device_node *node)
 struct fdt_header *of_get_fixed_tree(struct device_node *node)
 {
 	int ret;
-	struct fdt_header *fdt;
+	struct fdt_header *fdt = NULL;
+	struct device_node *freenp = NULL;
 
 	if (!node) {
 		node = of_get_root_node();
 		if (!node)
 			return NULL;
+
+		freenp = node = of_copy_node(NULL, node);
+		if (!node)
+			return NULL;
 	}
 
 	ret = of_fix_tree(node);
 	if (ret)
-		return NULL;
+		goto out;
 
 	fdt = of_flatten_dtb(node);
-	if (!fdt)
-		return NULL;
+
+out:
+	of_delete_node(freenp);
 
 	return fdt;
 }
-- 
2.29.2

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox