Fix bug adding properties with flatdevtree.c's ft_set_prop()

Fix bug adding properties with flatdevtree.c's ft_set_prop()

[David Gibson]

ft_set_prop() from flatdevtree.c in the zImage wrapper will either
replace an existing property in the flat device tree, or add a new
property definiion if the given property isn't present.

However, when adding properties, it adds the property definition
immediately before the node's END_NODE tag, potentially after any
subnode definitions for the node.  This confuses the kernel flat tree
parser in prom.c which assumes that all property definitions for a
node come before all subnode definitions.

This patch corrects ft_set_prop() so that it adds new properties
before the first subnode, instead of before the END_NODE tag.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>

Index: working-2.6/arch/powerpc/boot/flatdevtree.c
===================================================================
--- working-2.6.orig/arch/powerpc/boot/flatdevtree.c	2007-05-14 12:00:23.000000000 +1000
+++ working-2.6/arch/powerpc/boot/flatdevtree.c	2007-05-14 13:00:10.000000000 +1000
@@ -891,28 +891,27 @@ int ft_set_prop(struct ft_cxt *cxt, cons
 	struct ft_atom atom;
 	void *node;
 	char *p, *next;
-	int nextra, depth;
+	int nextra;
 
 	node = ft_node_ph2node(cxt, phandle);
 	if (node == NULL)
 		return -1;
 
-	depth = 0;
-	p = node;
+	next = ft_next(cxt, node, &atom);
+	if (atom.tag != OF_DT_BEGIN_NODE)
+		/* phandle didn't point to a node */
+		return -1;
+	p = next;
 
 	while ((next = ft_next(cxt, p, &atom)) != NULL) {
 		switch (atom.tag) {
-		case OF_DT_BEGIN_NODE:
-			++depth;
-			break;
+		case OF_DT_BEGIN_NODE: /* properties must go before subnodes */
 		case OF_DT_END_NODE:
-			if (--depth > 0)
-				break;
 			/* haven't found the property, insert here */
 			cxt->p = p;
 			return ft_prop(cxt, propname, buf, buflen);
 		case OF_DT_PROP:
-			if ((depth != 1) || strcmp(atom.name, propname))
+			if (strcmp(atom.name, propname))
 				break;
 			/* found an existing property, overwrite it */
 			nextra = _ALIGN(buflen, 4) - _ALIGN(atom.size, 4);

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: Fix bug adding properties with flatdevtree.c's ft_set_prop()

[Scott Wood]

On Mon, May 14, 2007 at 01:13:57PM +1000, David Gibson wrote:
> This patch corrects ft_set_prop() so that it adds new properties
> before the first subnode, instead of before the END_NODE tag.

Is there any reason not to just insert at the beginning of the node?

-Scott

Re: Fix bug adding properties with flatdevtree.c's ft_set_prop()

[David Gibson]

On Mon, May 14, 2007 at 11:28:16AM -0500, Scott Wood wrote:
> On Mon, May 14, 2007 at 01:13:57PM +1000, David Gibson wrote:
> > This patch corrects ft_set_prop() so that it adds new properties
> > before the first subnode, instead of before the END_NODE tag.
> 
> Is there any reason not to just insert at the beginning of the node?

Only that we have to search through the node first to see if the
property already exists, so we've already reached the end by the time
we know we need to insert.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: Fix bug adding properties with flatdevtree.c's ft_set_prop()

[Mark A. Greer]

On Tue, May 15, 2007 at 10:26:48AM +1000, David Gibson wrote:
> On Mon, May 14, 2007 at 11:28:16AM -0500, Scott Wood wrote:
> > On Mon, May 14, 2007 at 01:13:57PM +1000, David Gibson wrote:
> > > This patch corrects ft_set_prop() so that it adds new properties
> > > before the first subnode, instead of before the END_NODE tag.
> > 
> > Is there any reason not to just insert at the beginning of the node?
> 
> Only that we have to search through the node first to see if the
> property already exists, so we've already reached the end by the time
> we know we need to insert.

David, Jon,

Does dtc guarantee that there aren't any properties after a subnode?

Mark

Re: Fix bug adding properties with flatdevtree.c's ft_set_prop()

[David Gibson]

On Tue, May 15, 2007 at 11:11:40AM -0700, Mark A. Greer wrote:
> On Tue, May 15, 2007 at 10:26:48AM +1000, David Gibson wrote:
> > On Mon, May 14, 2007 at 11:28:16AM -0500, Scott Wood wrote:
> > > On Mon, May 14, 2007 at 01:13:57PM +1000, David Gibson wrote:
> > > > This patch corrects ft_set_prop() so that it adds new properties
> > > > before the first subnode, instead of before the END_NODE tag.
> > > 
> > > Is there any reason not to just insert at the beginning of the node?
> > 
> > Only that we have to search through the node first to see if the
> > property already exists, so we've already reached the end by the time
> > we know we need to insert.
> 
> David, Jon,
> 
> Does dtc guarantee that there aren't any properties after a subnode?

Yes.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: Fix bug adding properties with flatdevtree.c's ft_set_prop()

[Scott Wood]

On Mon, May 14, 2007 at 01:13:57PM +1000, David Gibson wrote:
> ft_set_prop() from flatdevtree.c in the zImage wrapper will either
> replace an existing property in the flat device tree, or add a new
> property definiion if the given property isn't present.
> 
> However, when adding properties, it adds the property definition
> immediately before the node's END_NODE tag, potentially after any
> subnode definitions for the node.  This confuses the kernel flat tree
> parser in prom.c which assumes that all property definitions for a
> node come before all subnode definitions.
> 
> This patch corrects ft_set_prop() so that it adds new properties
> before the first subnode, instead of before the END_NODE tag.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>

Acked-by: Scott Wood <scottwood@freescale.com>

-Scott

Re: Fix bug adding properties with flatdevtree.c's ft_set_prop()

[Mark A. Greer]

On Mon, May 14, 2007 at 01:13:57PM +1000, David Gibson wrote:
> ft_set_prop() from flatdevtree.c in the zImage wrapper will either
> replace an existing property in the flat device tree, or add a new
> property definiion if the given property isn't present.
> 
> However, when adding properties, it adds the property definition
> immediately before the node's END_NODE tag, potentially after any
> subnode definitions for the node.  This confuses the kernel flat tree
> parser in prom.c which assumes that all property definitions for a
> node come before all subnode definitions.

Oops. :)

> This patch corrects ft_set_prop() so that it adds new properties
> before the first subnode, instead of before the END_NODE tag.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>

Acked-by: Mark A. Greer <mgreer@mvista.com>