[PATCH 0/4] Series short description

[PATCH 0/4] Series short description

[John Bonesio]

The following series implements...

1) new patch to update error messaging
2) new patch to remove existing nodes
3) patch to reference nodes by path or label/path at the root level
   I chose the following syntax:

   &{/path/to/a/node}       [existing syntax, now is allowed at the root level]
   &label                   [existing syntax]
   &{label}                 [new syntax]
   &{label/path/to/subnode} [new syntax]

4) patch to undefine properties

- John

---

John Bonesio (4):
      Create new and use new print_error that uses printf style formatting.
      Implements new features for updating existing device tree nodes.
      Allow nodes at the root to be specified by path as well as by label.
      Create a new property value that means 'undefined'.


 dtc-lexer.l  |   34 ++++++++++++++++++++++++--
 dtc-parser.y |   47 +++++++++++++++++++++++++++---------
 dtc.h        |    8 ++++++
 flattree.c   |    3 ++
 livetree.c   |   76 +++++++++++++++++++++++++++++++++++++++++++++++++++++-----
 srcpos.c     |   21 ++++++++++------
 srcpos.h     |    2 ++
 7 files changed, 163 insertions(+), 28 deletions(-)

-- 
Signature

[PATCH 1/4] Create new and use new print_error that uses printf style formatting.

[John Bonesio]

yyerror is meant to be called by the parser internal code, and it's interface
is limited. Instead create and call a new error message routine that allows
formatted strings to be used.

yyerror uses the new routine so error formatting remains consistent.

Signed-of-by: John Bonesio <bones-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
---

 dtc-parser.y |   28 ++++++++++++++++++----------
 srcpos.c     |   21 +++++++++++++--------
 srcpos.h     |    2 ++
 3 files changed, 33 insertions(+), 18 deletions(-)

diff --git a/dtc-parser.y b/dtc-parser.y
index 0aaf8e8..b58ba8e 100644
--- a/dtc-parser.y
+++ b/dtc-parser.y
@@ -27,6 +27,7 @@
 YYLTYPE yylloc;
 
 extern int yylex(void);
+extern void print_error(char const *fmt, ...);
 extern void yyerror(char const *s);
 
 extern struct boot_info *the_boot_info;
@@ -136,8 +137,7 @@ devicetree:
 			if (target)
 				merge_nodes(target, $3);
 			else
-				yyerror("label does not exist in "
-					" node redefinition");
+				print_error("label, '%s' not found", $2);
 			$$ = $1;
 		}
 	;
@@ -200,8 +200,7 @@ propdata:
 
 			if ($6 != 0)
 				if (fseek(f, $6, SEEK_SET) != 0)
-					srcpos_error(&yylloc,
-						     "Couldn't seek to offset %llu in \"%s\": %s",
+					print_error("Couldn't seek to offset %llu in \"%s\": %s",
 						     (unsigned long long)$6,
 						     $4.val,
 						     strerror(errno));
@@ -295,7 +294,7 @@ subnodes:
 		}
 	| subnode propdef
 		{
-			yyerror("syntax error: properties must precede subnodes");
+			print_error("syntax error: properties must precede subnodes");
 			YYERROR;
 		}
 	;
@@ -314,12 +313,21 @@ subnode:
 
 %%
 
-void yyerror(char const *s)
+void print_error(char const *fmt, ...)
 {
-	srcpos_error(&yylloc, "%s", s);
+	va_list va;
+
+	va_start(va, fmt);
+	srcpos_verror(&yylloc, fmt, va);
+	va_end(va);
+
 	treesource_error = 1;
 }
 
+void yyerror(char const *s) {
+	print_error("%s", s);
+}
+
 static unsigned long long eval_literal(const char *s, int base, int bits)
 {
 	unsigned long long val;
@@ -328,11 +336,11 @@ static unsigned long long eval_literal(const char *s, int base, int bits)
 	errno = 0;
 	val = strtoull(s, &e, base);
 	if (*e)
-		yyerror("bad characters in literal");
+		print_error("bad characters in literal");
 	else if ((errno == ERANGE)
 		 || ((bits < 64) && (val >= (1ULL << bits))))
-		yyerror("literal out of range");
+		print_error("literal out of range");
 	else if (errno != 0)
-		yyerror("bad literal");
+		print_error("bad literal");
 	return val;
 }
diff --git a/srcpos.c b/srcpos.c
index 87d7f17..2dbc874 100644
--- a/srcpos.c
+++ b/srcpos.c
@@ -208,20 +208,25 @@ srcpos_string(struct srcpos *pos)
 	return pos_str;
 }
 
+void
+srcpos_verror(struct srcpos *pos, char const *fmt, va_list va)
+{
+       const char *srcstr;
+
+       srcstr = srcpos_string(pos);
+
+       fprintf(stdout, "Error: %s ", srcstr);
+       vfprintf(stdout, fmt, va);
+       fprintf(stdout, "\n");
+}
 
 void
 srcpos_error(struct srcpos *pos, char const *fmt, ...)
 {
-	const char *srcstr;
 	va_list va;
-	va_start(va, fmt);
-
-	srcstr = srcpos_string(pos);
-
-	fprintf(stderr, "Error: %s ", srcstr);
-	vfprintf(stderr, fmt, va);
-	fprintf(stderr, "\n");
 
+	va_start(va, fmt);
+	srcpos_verror(pos, fmt, va);
 	va_end(va);
 }
 
diff --git a/srcpos.h b/srcpos.h
index 985f847..bd7966e 100644
--- a/srcpos.h
+++ b/srcpos.h
@@ -76,6 +76,8 @@ extern struct srcpos *srcpos_copy(struct srcpos *pos);
 extern char *srcpos_string(struct srcpos *pos);
 extern void srcpos_dump(struct srcpos *pos);
 
+extern void srcpos_verror(struct srcpos *pos, char const *, va_list va)
+     __attribute__((format(printf, 2, 0)));
 extern void srcpos_error(struct srcpos *pos, char const *, ...)
      __attribute__((format(printf, 2, 3)));
 extern void srcpos_warn(struct srcpos *pos, char const *, ...)

Re: [PATCH 1/4] Create new and use new print_error that uses printf style formatting.

[Grant Likely]

On Wed, Oct 20, 2010 at 02:44:58PM -0700, John Bonesio wrote:
> yyerror is meant to be called by the parser internal code, and it's interface
> is limited. Instead create and call a new error message routine that allows
> formatted strings to be used.
> 
> yyerror uses the new routine so error formatting remains consistent.
> 
> Signed-of-by: John Bonesio <bones-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>

Picked up and pushed out to my git tree.  Jon is a little tied up for
the moment, so I'll maintain a working tree at
git://git.secretlab.ca/git/dtc.git until we've got this work finished
and we can ask him to merge it.

g.

> ---
> 
>  dtc-parser.y |   28 ++++++++++++++++++----------
>  srcpos.c     |   21 +++++++++++++--------
>  srcpos.h     |    2 ++
>  3 files changed, 33 insertions(+), 18 deletions(-)
> 
> diff --git a/dtc-parser.y b/dtc-parser.y
> index 0aaf8e8..b58ba8e 100644
> --- a/dtc-parser.y
> +++ b/dtc-parser.y
> @@ -27,6 +27,7 @@
>  YYLTYPE yylloc;
>  
>  extern int yylex(void);
> +extern void print_error(char const *fmt, ...);
>  extern void yyerror(char const *s);
>  
>  extern struct boot_info *the_boot_info;
> @@ -136,8 +137,7 @@ devicetree:
>  			if (target)
>  				merge_nodes(target, $3);
>  			else
> -				yyerror("label does not exist in "
> -					" node redefinition");
> +				print_error("label, '%s' not found", $2);
>  			$$ = $1;
>  		}
>  	;
> @@ -200,8 +200,7 @@ propdata:
>  
>  			if ($6 != 0)
>  				if (fseek(f, $6, SEEK_SET) != 0)
> -					srcpos_error(&yylloc,
> -						     "Couldn't seek to offset %llu in \"%s\": %s",
> +					print_error("Couldn't seek to offset %llu in \"%s\": %s",
>  						     (unsigned long long)$6,
>  						     $4.val,
>  						     strerror(errno));
> @@ -295,7 +294,7 @@ subnodes:
>  		}
>  	| subnode propdef
>  		{
> -			yyerror("syntax error: properties must precede subnodes");
> +			print_error("syntax error: properties must precede subnodes");
>  			YYERROR;
>  		}
>  	;
> @@ -314,12 +313,21 @@ subnode:
>  
>  %%
>  
> -void yyerror(char const *s)
> +void print_error(char const *fmt, ...)
>  {
> -	srcpos_error(&yylloc, "%s", s);
> +	va_list va;
> +
> +	va_start(va, fmt);
> +	srcpos_verror(&yylloc, fmt, va);
> +	va_end(va);
> +
>  	treesource_error = 1;
>  }
>  
> +void yyerror(char const *s) {
> +	print_error("%s", s);
> +}
> +
>  static unsigned long long eval_literal(const char *s, int base, int bits)
>  {
>  	unsigned long long val;
> @@ -328,11 +336,11 @@ static unsigned long long eval_literal(const char *s, int base, int bits)
>  	errno = 0;
>  	val = strtoull(s, &e, base);
>  	if (*e)
> -		yyerror("bad characters in literal");
> +		print_error("bad characters in literal");
>  	else if ((errno == ERANGE)
>  		 || ((bits < 64) && (val >= (1ULL << bits))))
> -		yyerror("literal out of range");
> +		print_error("literal out of range");
>  	else if (errno != 0)
> -		yyerror("bad literal");
> +		print_error("bad literal");
>  	return val;
>  }
> diff --git a/srcpos.c b/srcpos.c
> index 87d7f17..2dbc874 100644
> --- a/srcpos.c
> +++ b/srcpos.c
> @@ -208,20 +208,25 @@ srcpos_string(struct srcpos *pos)
>  	return pos_str;
>  }
>  
> +void
> +srcpos_verror(struct srcpos *pos, char const *fmt, va_list va)
> +{
> +       const char *srcstr;
> +
> +       srcstr = srcpos_string(pos);
> +
> +       fprintf(stdout, "Error: %s ", srcstr);
> +       vfprintf(stdout, fmt, va);
> +       fprintf(stdout, "\n");
> +}
>  
>  void
>  srcpos_error(struct srcpos *pos, char const *fmt, ...)
>  {
> -	const char *srcstr;
>  	va_list va;
> -	va_start(va, fmt);
> -
> -	srcstr = srcpos_string(pos);
> -
> -	fprintf(stderr, "Error: %s ", srcstr);
> -	vfprintf(stderr, fmt, va);
> -	fprintf(stderr, "\n");
>  
> +	va_start(va, fmt);
> +	srcpos_verror(pos, fmt, va);
>  	va_end(va);
>  }
>  
> diff --git a/srcpos.h b/srcpos.h
> index 985f847..bd7966e 100644
> --- a/srcpos.h
> +++ b/srcpos.h
> @@ -76,6 +76,8 @@ extern struct srcpos *srcpos_copy(struct srcpos *pos);
>  extern char *srcpos_string(struct srcpos *pos);
>  extern void srcpos_dump(struct srcpos *pos);
>  
> +extern void srcpos_verror(struct srcpos *pos, char const *, va_list va)
> +     __attribute__((format(printf, 2, 0)));
>  extern void srcpos_error(struct srcpos *pos, char const *, ...)
>       __attribute__((format(printf, 2, 3)));
>  extern void srcpos_warn(struct srcpos *pos, char const *, ...)
> 

Re: [PATCH 1/4] Create new and use new print_error that uses printf style formatting.

[Jon Loeliger]

> On Wed, Oct 20, 2010 at 02:44:58PM -0700, John Bonesio wrote:
> > yyerror is meant to be called by the parser internal code, and it's interface
> > is limited. Instead create and call a new error message routine that allows
> > formatted strings to be used.
> > 
> > yyerror uses the new routine so error formatting remains consistent.
> > 
> > Signed-of-by: John Bonesio <bones-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
> 
> Picked up and pushed out to my git tree.  Jon is a little tied up for
> the moment, so I'll maintain a working tree at
> git://git.secretlab.ca/git/dtc.git until we've got this work finished
> and we can ask him to merge it.
> 
> g.


Thanks, Grant.  I'll pick this patch up now too, and the
subsequent patch 1/4 from the next series about labels refs.

jdl

[PATCH 2/4] Implements new features for updating existing device tree nodes.

[John Bonesio]

This is interesting when the /include/ "<filename>" feature is used. This way
we can create base device tree source files for a family of systems and modify
the device tree for a specific system.

The current sytem allows an existing node to be extended with new properties
and subnodes.

The new features allow an existing node to be replaced completely by the new
properties and subnodes. The new features also allow an existing node to be
deleted.

Signed-off-by: John Bonesio <bones-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
---

 dtc-lexer.l  |    6 ++++++
 dtc-parser.y |   11 +++++++++++
 dtc.h        |    1 +
 livetree.c   |   18 ++++++++++++++++++
 4 files changed, 36 insertions(+), 0 deletions(-)

diff --git a/dtc-lexer.l b/dtc-lexer.l
index 081e13a..80a886a 100644
--- a/dtc-lexer.l
+++ b/dtc-lexer.l
@@ -96,6 +96,12 @@ static int pop_input_file(void);
 			return DT_MEMRESERVE;
 		}
 
+<*>"/remove-node/"	{
+			DPRINT("Keyword: /remove-node/\n");
+			BEGIN_DEFAULT();
+			return DT_REMOVENODE;
+		}
+
 <*>{LABEL}:	{
 			DPRINT("Label: %s\n", yytext);
 			yylval.labelref = xstrdup(yytext);
diff --git a/dtc-parser.y b/dtc-parser.y
index b58ba8e..1ba0478 100644
--- a/dtc-parser.y
+++ b/dtc-parser.y
@@ -55,6 +55,7 @@ static unsigned long long eval_literal(const char *s, int base, int bits);
 
 %token DT_V1
 %token DT_MEMRESERVE
+%token DT_REMOVENODE
 %token <propnodename> DT_PROPNODENAME
 %token <literal> DT_LITERAL
 %token <cbase> DT_BASE
@@ -140,6 +141,16 @@ devicetree:
 				print_error("label, '%s' not found", $2);
 			$$ = $1;
 		}
+	| devicetree DT_REMOVENODE DT_REF ';'
+		{
+			struct node *target;
+
+			target = get_node_by_label($1, $3);
+			if (target)
+				remove_child(target->parent, target);
+			else
+				print_error("label, '%s' not found", $3);
+		}
 	;
 
 nodedef:
diff --git a/dtc.h b/dtc.h
index b36ac5d..a7f3667 100644
--- a/dtc.h
+++ b/dtc.h
@@ -178,6 +178,7 @@ struct node *merge_nodes(struct node *old_node, struct node *new_node);
 
 void add_property(struct node *node, struct property *prop);
 void add_child(struct node *parent, struct node *child);
+void remove_child(struct node *parent, struct node *child);
 
 const char *get_unitname(struct node *node);
 struct property *get_property(struct node *node, const char *propname);
diff --git a/livetree.c b/livetree.c
index 13c5f10..21a4c48 100644
--- a/livetree.c
+++ b/livetree.c
@@ -202,6 +202,24 @@ void add_child(struct node *parent, struct node *child)
 	*p = child;
 }
 
+void remove_child(struct node *parent, struct node *child)
+{
+	struct node **p;
+
+	/* Make sure we've got a consistent tree here */
+	assert(child->parent == parent);
+
+	p = &parent->children;
+	while (*p) {
+		if (*p == child) {
+			*p = (*p)->next_sibling;
+			break;
+		}
+		p = &((*p)->next_sibling);
+	}
+	child->parent = NULL;
+}
+
 struct reserve_info *build_reserve_entry(uint64_t address, uint64_t size)
 {
 	struct reserve_info *new = xmalloc(sizeof(*new));

Re: [PATCH 2/4] Implements new features for updating existing device tree nodes.

[Grant Likely]

On Wed, Oct 20, 2010 at 02:45:06PM -0700, John Bonesio wrote:
> This is interesting when the /include/ "<filename>" feature is used. This way
> we can create base device tree source files for a family of systems and modify
> the device tree for a specific system.
> 
> The current sytem allows an existing node to be extended with new properties
> and subnodes.
> 
> The new features allow an existing node to be replaced completely by the new
> properties and subnodes. The new features also allow an existing node to be
> deleted.
> 
> Signed-off-by: John Bonesio <bones-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>

Patch looks good, but it needs a test case before it can be merged.
Also, as mentioned earlier, the switch to get_node_by_ref() should be
applied before this patch.

g.

> ---
> 
>  dtc-lexer.l  |    6 ++++++
>  dtc-parser.y |   11 +++++++++++
>  dtc.h        |    1 +
>  livetree.c   |   18 ++++++++++++++++++
>  4 files changed, 36 insertions(+), 0 deletions(-)
> 
> diff --git a/dtc-lexer.l b/dtc-lexer.l
> index 081e13a..80a886a 100644
> --- a/dtc-lexer.l
> +++ b/dtc-lexer.l
> @@ -96,6 +96,12 @@ static int pop_input_file(void);
>  			return DT_MEMRESERVE;
>  		}
>  
> +<*>"/remove-node/"	{
> +			DPRINT("Keyword: /remove-node/\n");
> +			BEGIN_DEFAULT();
> +			return DT_REMOVENODE;
> +		}
> +
>  <*>{LABEL}:	{
>  			DPRINT("Label: %s\n", yytext);
>  			yylval.labelref = xstrdup(yytext);
> diff --git a/dtc-parser.y b/dtc-parser.y
> index b58ba8e..1ba0478 100644
> --- a/dtc-parser.y
> +++ b/dtc-parser.y
> @@ -55,6 +55,7 @@ static unsigned long long eval_literal(const char *s, int base, int bits);
>  
>  %token DT_V1
>  %token DT_MEMRESERVE
> +%token DT_REMOVENODE
>  %token <propnodename> DT_PROPNODENAME
>  %token <literal> DT_LITERAL
>  %token <cbase> DT_BASE
> @@ -140,6 +141,16 @@ devicetree:
>  				print_error("label, '%s' not found", $2);
>  			$$ = $1;
>  		}
> +	| devicetree DT_REMOVENODE DT_REF ';'
> +		{
> +			struct node *target;
> +
> +			target = get_node_by_label($1, $3);
> +			if (target)
> +				remove_child(target->parent, target);
> +			else
> +				print_error("label, '%s' not found", $3);
> +		}
>  	;
>  
>  nodedef:
> diff --git a/dtc.h b/dtc.h
> index b36ac5d..a7f3667 100644
> --- a/dtc.h
> +++ b/dtc.h
> @@ -178,6 +178,7 @@ struct node *merge_nodes(struct node *old_node, struct node *new_node);
>  
>  void add_property(struct node *node, struct property *prop);
>  void add_child(struct node *parent, struct node *child);
> +void remove_child(struct node *parent, struct node *child);
>  
>  const char *get_unitname(struct node *node);
>  struct property *get_property(struct node *node, const char *propname);
> diff --git a/livetree.c b/livetree.c
> index 13c5f10..21a4c48 100644
> --- a/livetree.c
> +++ b/livetree.c
> @@ -202,6 +202,24 @@ void add_child(struct node *parent, struct node *child)
>  	*p = child;
>  }
>  
> +void remove_child(struct node *parent, struct node *child)
> +{
> +	struct node **p;
> +
> +	/* Make sure we've got a consistent tree here */
> +	assert(child->parent == parent);
> +
> +	p = &parent->children;
> +	while (*p) {
> +		if (*p == child) {
> +			*p = (*p)->next_sibling;
> +			break;
> +		}
> +		p = &((*p)->next_sibling);
> +	}
> +	child->parent = NULL;
> +}
> +
>  struct reserve_info *build_reserve_entry(uint64_t address, uint64_t size)
>  {
>  	struct reserve_info *new = xmalloc(sizeof(*new));
> 

Re: [PATCH 2/4] Implements new features for updating existing device tree nodes.

[David Gibson]

On Wed, Oct 20, 2010 at 02:45:06PM -0700, John Bonesio wrote:
> This is interesting when the /include/ "<filename>" feature is used. This way
> we can create base device tree source files for a family of systems and modify
> the device tree for a specific system.
> 
> The current sytem allows an existing node to be extended with new properties
> and subnodes.
> 
> The new features allow an existing node to be replaced completely by the new
> properties and subnodes. The new features also allow an existing node to be
> deleted.

[snip]
> +	| devicetree DT_REMOVENODE DT_REF ';'
> +		{
> +			struct node *target;
> +
> +			target = get_node_by_label($1, $3);
> +			if (target)
> +				remove_child(target->parent, target);
> +			else
> +				print_error("label, '%s' not found", $3);
> +		}

This still has the problem of labels disappearing into the ether.

>  	;
>  
>  nodedef:
> diff --git a/dtc.h b/dtc.h
> index b36ac5d..a7f3667 100644
> --- a/dtc.h
> +++ b/dtc.h
> @@ -178,6 +178,7 @@ struct node *merge_nodes(struct node *old_node, struct node *new_node);
>  
>  void add_property(struct node *node, struct property *prop);
>  void add_child(struct node *parent, struct node *child);
> +void remove_child(struct node *parent, struct node *child);
>  
>  const char *get_unitname(struct node *node);
>  struct property *get_property(struct node *node, const char *propname);
> diff --git a/livetree.c b/livetree.c
> index 13c5f10..21a4c48 100644
> --- a/livetree.c
> +++ b/livetree.c
> @@ -202,6 +202,24 @@ void add_child(struct node *parent, struct node *child)
>  	*p = child;
>  }
>  
> +void remove_child(struct node *parent, struct node *child)
> +{
> +	struct node **p;
> +
> +	/* Make sure we've got a consistent tree here */
> +	assert(child->parent == parent);
> +
> +	p = &parent->children;
> +	while (*p) {
> +		if (*p == child) {
> +			*p = (*p)->next_sibling;
> +			break;
> +		}
> +		p = &((*p)->next_sibling);
> +	}
> +	child->parent = NULL;
> +}
> +
>  struct reserve_info *build_reserve_entry(uint64_t address, uint64_t size)
>  {
>  	struct reserve_info *new = xmalloc(sizeof(*new));
> 

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

[PATCH 3/4] Allow nodes at the root to be specified by path as well as by label.

[John Bonesio]

Changes to allow us to specify a node by it's path. A path can be used in
place of a label.

This is particularly useful when overriding existing nodes.
This way we don't have to label every possible node in a device tree we know
is a base device tree for a class of systems, and we know the tree will be
modified for the specific systems.

Signed-off-by: John Bonesio <bones-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
---

 dtc-lexer.l  |   26 ++++++++++++++++++++++----
 dtc-parser.y |   10 ++++------
 livetree.c   |   20 +++++++++++++++++++-
 3 files changed, 45 insertions(+), 11 deletions(-)

diff --git a/dtc-lexer.l b/dtc-lexer.l
index 80a886a..216a3d2 100644
--- a/dtc-lexer.l
+++ b/dtc-lexer.l
@@ -115,13 +115,31 @@ static int pop_input_file(void);
 			return DT_LITERAL;
 		}
 
-<*>\&{LABEL}	{	/* label reference */
-			DPRINT("Ref: %s\n", yytext+1);
-			yylval.labelref = xstrdup(yytext+1);
+
+<*>\&{LABEL}    {       /* label reference without the braces*/
+                        DPRINT("Ref: %s\n", yytext+1);
+                        yylval.labelref = xstrdup(yytext+1);
+                        return DT_REF;
+		}
+
+<*>"&{"{LABEL}{PATHCHAR}*\} { /* label and/or path refererence with braces */
+			/*
+			 * Possibly this could be parsed by the parser rather
+			 * than as a lexical element.
+			 *
+			 * What is intended here is to support the following
+			 * type of references:
+			 * a) &{/path/to/the/node/reference}
+			 * b) &{label}
+			 * c) &{label/path/from/labeled/node/to/reference}
+			 */
+			yytext[yyleng-1] = '\0';
+			DPRINT("Ref: %s\n", yytext+2);
+			yylval.labelref = xstrdup(yytext+2);
 			return DT_REF;
 		}
 
-"&{/"{PATHCHAR}+\}	{	/* new-style path reference */
+<*>"&{/"{PATHCHAR}+\}	{	/* new-style path reference (with braces) */
 			yytext[yyleng-1] = '\0';
 			DPRINT("Ref: %s\n", yytext+2);
 			yylval.labelref = xstrdup(yytext+2);
diff --git a/dtc-parser.y b/dtc-parser.y
index 1ba0478..0a74c86 100644
--- a/dtc-parser.y
+++ b/dtc-parser.y
@@ -132,24 +132,22 @@ devicetree:
 		}
 	| devicetree DT_REF nodedef
 		{
-			struct node *target;
+			struct node *target = get_node_by_ref($1, $2);
 
-			target = get_node_by_label($1, $2);
 			if (target)
 				merge_nodes(target, $3);
 			else
-				print_error("label, '%s' not found", $2);
+				print_error("label or path, '%s', not found", $2);
 			$$ = $1;
 		}
 	| devicetree DT_REMOVENODE DT_REF ';'
 		{
-			struct node *target;
+			struct node *target = get_node_by_ref($1, $3);
 
-			target = get_node_by_label($1, $3);
 			if (target)
 				remove_child(target->parent, target);
 			else
-				print_error("label, '%s' not found", $3);
+				print_error("label or path, '%s', not found", $3);
 		}
 	;
 
diff --git a/livetree.c b/livetree.c
index 21a4c48..bf8796b 100644
--- a/livetree.c
+++ b/livetree.c
@@ -429,10 +429,28 @@ struct node *get_node_by_phandle(struct node *tree, cell_t phandle)
 
 struct node *get_node_by_ref(struct node *tree, const char *ref)
 {
+	struct node *node;
+
 	if (ref[0] == '/')
 		return get_node_by_path(tree, ref);
-	else
+	else {
+		/*
+		 * Support finding a node reference that is rooted by
+		 * a label reference.
+		 *
+		 * References rooted by a label have a '/' in them.
+		 */
+		char *p = strchr(ref, '/');
+
+		if (p) {
+			*p = '\0';
+			node = get_node_by_label(tree, ref);
+			*p = '/';
+			return get_node_by_path(node, p+1);
+		}
+
 		return get_node_by_label(tree, ref);
+	}
 }
 
 cell_t get_node_phandle(struct node *root, struct node *node)

Re: [PATCH 3/4] Allow nodes at the root to be specified by path as well as by label.

[Grant Likely]

On Wed, Oct 20, 2010 at 02:45:13PM -0700, John Bonesio wrote:
> Changes to allow us to specify a node by it's path. A path can be used in
> place of a label.
> 
> This is particularly useful when overriding existing nodes.
> This way we don't have to label every possible node in a device tree we know
> is a base device tree for a class of systems, and we know the tree will be
> modified for the specific systems.
> 
> Signed-off-by: John Bonesio <bones-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>

As already mentioned, but I'm reposting here for the list archive
record, this syntax conflicts with existing convention for referencing
nodes by names defined in the /aliases node.

One more comment below.

> ---
> 
>  dtc-lexer.l  |   26 ++++++++++++++++++++++----
>  dtc-parser.y |   10 ++++------
>  livetree.c   |   20 +++++++++++++++++++-
>  3 files changed, 45 insertions(+), 11 deletions(-)
> 
> diff --git a/dtc-lexer.l b/dtc-lexer.l
> index 80a886a..216a3d2 100644
> --- a/dtc-lexer.l
> +++ b/dtc-lexer.l
> @@ -115,13 +115,31 @@ static int pop_input_file(void);
>  			return DT_LITERAL;
>  		}
>  
> -<*>\&{LABEL}	{	/* label reference */
> -			DPRINT("Ref: %s\n", yytext+1);
> -			yylval.labelref = xstrdup(yytext+1);
> +
> +<*>\&{LABEL}    {       /* label reference without the braces*/
> +                        DPRINT("Ref: %s\n", yytext+1);
> +                        yylval.labelref = xstrdup(yytext+1);
> +                        return DT_REF;
> +		}
> +
> +<*>"&{"{LABEL}{PATHCHAR}*\} { /* label and/or path refererence with braces */
> +			/*
> +			 * Possibly this could be parsed by the parser rather
> +			 * than as a lexical element.
> +			 *
> +			 * What is intended here is to support the following
> +			 * type of references:
> +			 * a) &{/path/to/the/node/reference}
> +			 * b) &{label}
> +			 * c) &{label/path/from/labeled/node/to/reference}
> +			 */
> +			yytext[yyleng-1] = '\0';
> +			DPRINT("Ref: %s\n", yytext+2);
> +			yylval.labelref = xstrdup(yytext+2);
>  			return DT_REF;
>  		}
>  
> -"&{/"{PATHCHAR}+\}	{	/* new-style path reference */
> +<*>"&{/"{PATHCHAR}+\}	{	/* new-style path reference (with braces) */
>  			yytext[yyleng-1] = '\0';
>  			DPRINT("Ref: %s\n", yytext+2);
>  			yylval.labelref = xstrdup(yytext+2);
> diff --git a/dtc-parser.y b/dtc-parser.y
> index 1ba0478..0a74c86 100644
> --- a/dtc-parser.y
> +++ b/dtc-parser.y
> @@ -132,24 +132,22 @@ devicetree:
>  		}
>  	| devicetree DT_REF nodedef
>  		{
> -			struct node *target;
> +			struct node *target = get_node_by_ref($1, $2);
>  
> -			target = get_node_by_label($1, $2);
>  			if (target)
>  				merge_nodes(target, $3);
>  			else
> -				print_error("label, '%s' not found", $2);
> +				print_error("label or path, '%s', not found", $2);

For clarity, the above hunk should be in a separate patch applied
between patches #1 and #2.  That will avoid having one patch introduce
the DT_REMOVENODE hunk below, and then immediately modify it again in
this patch.  The switch to get_node_by_ref() is useful even without
the label+path syntax, and it should also include a matching testcase.

>  			$$ = $1;
>  		}
>  	| devicetree DT_REMOVENODE DT_REF ';'
>  		{
> -			struct node *target;
> +			struct node *target = get_node_by_ref($1, $3);
>  
> -			target = get_node_by_label($1, $3);
>  			if (target)
>  				remove_child(target->parent, target);
>  			else
> -				print_error("label, '%s' not found", $3);
> +				print_error("label or path, '%s', not found", $3);
>  		}
>  	;
>  
> diff --git a/livetree.c b/livetree.c
> index 21a4c48..bf8796b 100644
> --- a/livetree.c
> +++ b/livetree.c
> @@ -429,10 +429,28 @@ struct node *get_node_by_phandle(struct node *tree, cell_t phandle)
>  
>  struct node *get_node_by_ref(struct node *tree, const char *ref)
>  {
> +	struct node *node;
> +
>  	if (ref[0] == '/')
>  		return get_node_by_path(tree, ref);
> -	else
> +	else {
> +		/*
> +		 * Support finding a node reference that is rooted by
> +		 * a label reference.
> +		 *
> +		 * References rooted by a label have a '/' in them.
> +		 */
> +		char *p = strchr(ref, '/');
> +
> +		if (p) {
> +			*p = '\0';
> +			node = get_node_by_label(tree, ref);
> +			*p = '/';
> +			return get_node_by_path(node, p+1);
> +		}
> +
>  		return get_node_by_label(tree, ref);
> +	}
>  }
>  
>  cell_t get_node_phandle(struct node *root, struct node *node)
> 

Re: [PATCH 3/4] Allow nodes at the root to be specified by path as well as by label.

[David Gibson]

On Wed, Oct 20, 2010 at 02:45:13PM -0700, John Bonesio wrote:
> Changes to allow us to specify a node by it's path. A path can be used in
> place of a label.
> 
> This is particularly useful when overriding existing nodes.
> This way we don't have to label every possible node in a device tree we know
> is a base device tree for a class of systems, and we know the tree will be
> modified for the specific systems.

[snip]
> diff --git a/livetree.c b/livetree.c
> index 21a4c48..bf8796b 100644
> --- a/livetree.c
> +++ b/livetree.c
> @@ -429,10 +429,28 @@ struct node *get_node_by_phandle(struct node *tree, cell_t phandle)
>  
>  struct node *get_node_by_ref(struct node *tree, const char *ref)
>  {
> +	struct node *node;
> +
>  	if (ref[0] == '/')
>  		return get_node_by_path(tree, ref);
> -	else
> +	else {
> +		/*
> +		 * Support finding a node reference that is rooted by
> +		 * a label reference.
> +		 *
> +		 * References rooted by a label have a '/' in them.
> +		 */
> +		char *p = strchr(ref, '/');

So, this line is effectively un-const-ing part of the string that ref
points to.

> +
> +		if (p) {
> +			*p = '\0';

And, then you write to it.  Not good at all.  I'm afraid you're going
to have to use a strndup() here.  Or else modify get_node_by_label()
to ignore text after a /.

> +			node = get_node_by_label(tree, ref);
> +			*p = '/';
> +			return get_node_by_path(node, p+1);
> +		}
> +
>  		return get_node_by_label(tree, ref);
> +	}
>  }
>  
>  cell_t get_node_phandle(struct node *root, struct node *node)
> 

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

[PATCH 4/4] Create a new property value that means 'undefined'.

[John Bonesio]

When updating existing nodes in a device tree merge operation, properties
can be removed by setting the value to /undef-prop/.

if /undef-prop/ is assigned to a property that doesn't exist, the property
is treated the same as if it had not been declared.

Signed-off-by: John Bonesio <bones-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
---

 dtc-lexer.l  |   14 ++++++++++----
 dtc-parser.y |    6 ++++++
 dtc.h        |    7 +++++++
 flattree.c   |    3 +++
 livetree.c   |   38 +++++++++++++++++++++++++++++++++-----
 5 files changed, 59 insertions(+), 9 deletions(-)

diff --git a/dtc-lexer.l b/dtc-lexer.l
index 216a3d2..efa89b4 100644
--- a/dtc-lexer.l
+++ b/dtc-lexer.l
@@ -102,6 +102,12 @@ static int pop_input_file(void);
 			return DT_REMOVENODE;
 		}
 
+<*>"/undef-prop/"	{
+			DPRINT("Keyword: /undef-prop/\n");
+			BEGIN_DEFAULT();
+			return DT_UNDEFINED;
+		}
+
 <*>{LABEL}:	{
 			DPRINT("Label: %s\n", yytext);
 			yylval.labelref = xstrdup(yytext);
@@ -116,10 +122,10 @@ static int pop_input_file(void);
 		}
 
 
-<*>\&{LABEL}    {       /* label reference without the braces*/
-                        DPRINT("Ref: %s\n", yytext+1);
-                        yylval.labelref = xstrdup(yytext+1);
-                        return DT_REF;
+<*>\&{LABEL}	{       /* label reference without the braces*/
+			DPRINT("Ref: %s\n", yytext+1);
+			yylval.labelref = xstrdup(yytext+1);
+	    		return DT_REF;
 		}
 
 <*>"&{"{LABEL}{PATHCHAR}*\} { /* label and/or path refererence with braces */
diff --git a/dtc-parser.y b/dtc-parser.y
index 0a74c86..ac9cfd7 100644
--- a/dtc-parser.y
+++ b/dtc-parser.y
@@ -56,6 +56,7 @@ static unsigned long long eval_literal(const char *s, int base, int bits);
 %token DT_V1
 %token DT_MEMRESERVE
 %token DT_REMOVENODE
+%token DT_UNDEFINED
 %token <propnodename> DT_PROPNODENAME
 %token <literal> DT_LITERAL
 %token <cbase> DT_BASE
@@ -178,6 +179,11 @@ propdef:
 		{
 			$$ = build_property($1, empty_data);
 		}
+	| DT_PROPNODENAME '=' DT_UNDEFINED ';'
+		{
+			$$ = build_property($1, empty_data);
+			undefine_property($$)
+		}
 	| DT_LABEL propdef
 		{
 			add_label(&$2->labels, $1);
diff --git a/dtc.h b/dtc.h
index a7f3667..b3fca6e 100644
--- a/dtc.h
+++ b/dtc.h
@@ -130,7 +130,12 @@ struct label {
 	struct label *next;
 };
 
+#define PROP_DEFINED (0)
+#define PROP_UNDEFINED (1)
+
 struct property {
+	int undefined;  /* if the property is set to undefined, this feild is
+	                   set to PROP_UNDEFINED */
 	char *name;
 	struct data val;
 
@@ -170,6 +175,7 @@ void add_label(struct label **labels, char *label);
 struct property *build_property(char *name, struct data val);
 struct property *chain_property(struct property *first, struct property *list);
 struct property *reverse_properties(struct property *first);
+void undefine_property(struct property *prop);
 
 struct node *build_node(struct property *proplist, struct node *children);
 struct node *name_node(struct node *node, char *name);
@@ -177,6 +183,7 @@ struct node *chain_node(struct node *first, struct node *list);
 struct node *merge_nodes(struct node *old_node, struct node *new_node);
 
 void add_property(struct node *node, struct property *prop);
+void remove_property(struct node *node, struct property *prop);
 void add_child(struct node *parent, struct node *child);
 void remove_child(struct node *parent, struct node *child);
 
diff --git a/flattree.c b/flattree.c
index ead0332..00439e9 100644
--- a/flattree.c
+++ b/flattree.c
@@ -275,6 +275,9 @@ static void flatten_tree(struct node *tree, struct emitter *emit,
 	for_each_property(tree, prop) {
 		int nameoff;
 
+		if (prop->undefined)
+			continue;
+
 		if (streq(prop->name, "name"))
 			seen_name_prop = 1;
 
diff --git a/livetree.c b/livetree.c
index bf8796b..2ef734d 100644
--- a/livetree.c
+++ b/livetree.c
@@ -51,6 +51,11 @@ struct property *build_property(char *name, struct data val)
 	return new;
 }
 
+void undefine_property(struct property *prop)
+{
+	prop->undefined = 1;
+}
+
 struct property *chain_property(struct property *first, struct property *list)
 {
 	assert(first->next == NULL);
@@ -121,11 +126,15 @@ struct node *merge_nodes(struct node *old_node, struct node *new_node)
 		/* Look for a collision, set new value if there is */
 		for_each_property(old_node, old_prop) {
 			if (streq(old_prop->name, new_prop->name)) {
-				/* Add new labels to old property */
-				for_each_label(new_prop->labels, l)
-					add_label(&old_prop->labels, l->label);
-
-				old_prop->val = new_prop->val;
+				if (new_prop->undefined) {
+					remove_property(old_node, old_prop);
+				} else {
+					/* Add new labels to old property */
+					for_each_label(new_prop->labels, l)
+						add_label(&old_prop->labels, l->label);
+
+					old_prop->val = new_prop->val;
+				}
 				free(new_prop);
 				new_prop = NULL;
 				break;
@@ -188,6 +197,25 @@ void add_property(struct node *node, struct property *prop)
 	*p = prop;
 }
 
+void remove_property(struct node *node, struct property *prop)
+{
+	struct property **p;
+	int found = 0;
+
+	p = &node->proplist;
+	while (*p) {
+		if (*p == prop) {
+			*p = (*p)->next;
+			found = 1;
+			break;
+		}
+		p = &((*p)->next);
+	}
+	/* property not in the node? it's probably an error, so flag it. */
+	assert(found);
+}
+
+
 void add_child(struct node *parent, struct node *child)
 {
 	struct node **p;

Re: [PATCH 4/4] Create a new property value that means 'undefined'.

[Grant Likely]

On Wed, Oct 20, 2010 at 02:45:22PM -0700, John Bonesio wrote:
> When updating existing nodes in a device tree merge operation, properties
> can be removed by setting the value to /undef-prop/.
> 
> if /undef-prop/ is assigned to a property that doesn't exist, the property
> is treated the same as if it had not been declared.
> 
> Signed-off-by: John Bonesio <bones-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>

Implementation looks good.
> ---
> 
>  dtc-lexer.l  |   14 ++++++++++----
>  dtc-parser.y |    6 ++++++
>  dtc.h        |    7 +++++++
>  flattree.c   |    3 +++
>  livetree.c   |   38 +++++++++++++++++++++++++++++++++-----
>  5 files changed, 59 insertions(+), 9 deletions(-)
> 
> diff --git a/dtc-lexer.l b/dtc-lexer.l
> index 216a3d2..efa89b4 100644
> --- a/dtc-lexer.l
> +++ b/dtc-lexer.l
> @@ -102,6 +102,12 @@ static int pop_input_file(void);
>  			return DT_REMOVENODE;
>  		}
>  
> +<*>"/undef-prop/"	{
> +			DPRINT("Keyword: /undef-prop/\n");
> +			BEGIN_DEFAULT();
> +			return DT_UNDEFINED;
> +		}
> +

Does /undef-prop/ really need to be using <*> to match in all start
conditions?

>  <*>{LABEL}:	{
>  			DPRINT("Label: %s\n", yytext);
>  			yylval.labelref = xstrdup(yytext);
> @@ -116,10 +122,10 @@ static int pop_input_file(void);
>  		}
>  
>  
> -<*>\&{LABEL}    {       /* label reference without the braces*/
> -                        DPRINT("Ref: %s\n", yytext+1);
> -                        yylval.labelref = xstrdup(yytext+1);
> -                        return DT_REF;
> +<*>\&{LABEL}	{       /* label reference without the braces*/
> +			DPRINT("Ref: %s\n", yytext+1);
> +			yylval.labelref = xstrdup(yytext+1);
> +	    		return DT_REF;
>  		}

Unrelated whitespace change.  In general, patches should avoid making
unrelated changes in the same patch, even if they are correct, because
they decrease the signal-to-noise ratio for patch reviewers.
Whitespace changes are particularly offensive because the can end up
masking (to a reviewer) functional changes in the same block.

>  
>  <*>"&{"{LABEL}{PATHCHAR}*\} { /* label and/or path refererence with braces */
> diff --git a/dtc-parser.y b/dtc-parser.y
> index 0a74c86..ac9cfd7 100644
> --- a/dtc-parser.y
> +++ b/dtc-parser.y
> @@ -56,6 +56,7 @@ static unsigned long long eval_literal(const char *s, int base, int bits);
>  %token DT_V1
>  %token DT_MEMRESERVE
>  %token DT_REMOVENODE
> +%token DT_UNDEFINED
>  %token <propnodename> DT_PROPNODENAME
>  %token <literal> DT_LITERAL
>  %token <cbase> DT_BASE
> @@ -178,6 +179,11 @@ propdef:
>  		{
>  			$$ = build_property($1, empty_data);
>  		}
> +	| DT_PROPNODENAME '=' DT_UNDEFINED ';'

Hmmm.  I'm going to make this comment once, but I'll shut-up if you
guys disagree with me because the details have already been hashed out
several times, and I've already said I'd be okay with the above form.

The more I look at it, the more I prefer the form 
	/undef-prop/ property;
instead of
	property = /undef-prop/;

The reason being is that while the assignment form does work, it isn't
a very natural construct.  Removal is not logically the same as
assignment.  /undef-prop/ is something that is performed on a
property.  Syntax that shows /undef-prop/ being assigned as a property
value doesn't ring true for me as the right thing to do.

So, my vote is for the "/undef-prop/ property;" form, but I hold my
piece if you both disagree with me.

> +		{
> +			$$ = build_property($1, empty_data);
> +			undefine_property($$)

Don't bother with the undefine_property function.  Just do:
			$$->undefined = 1;

> +		}

>  	| DT_LABEL propdef
>  		{
>  			add_label(&$2->labels, $1);
> diff --git a/dtc.h b/dtc.h
> index a7f3667..b3fca6e 100644
> --- a/dtc.h
> +++ b/dtc.h
> @@ -130,7 +130,12 @@ struct label {
>  	struct label *next;
>  };
>  
> +#define PROP_DEFINED (0)
> +#define PROP_UNDEFINED (1)

No need for the #defines. Just use 0 and 1 in the code when working
with boolean flags.  Besides, these #defines aren't used anywhere
anyway.  :-)

> +
>  struct property {
> +	int undefined;  /* if the property is set to undefined, this feild is
> +	                   set to PROP_UNDEFINED */
>  	char *name;
>  	struct data val;
>  
> @@ -170,6 +175,7 @@ void add_label(struct label **labels, char *label);
>  struct property *build_property(char *name, struct data val);
>  struct property *chain_property(struct property *first, struct property *list);
>  struct property *reverse_properties(struct property *first);
> +void undefine_property(struct property *prop);
>  
>  struct node *build_node(struct property *proplist, struct node *children);
>  struct node *name_node(struct node *node, char *name);
> @@ -177,6 +183,7 @@ struct node *chain_node(struct node *first, struct node *list);
>  struct node *merge_nodes(struct node *old_node, struct node *new_node);
>  
>  void add_property(struct node *node, struct property *prop);
> +void remove_property(struct node *node, struct property *prop);
>  void add_child(struct node *parent, struct node *child);
>  void remove_child(struct node *parent, struct node *child);
>  
> diff --git a/flattree.c b/flattree.c
> index ead0332..00439e9 100644
> --- a/flattree.c
> +++ b/flattree.c
> @@ -275,6 +275,9 @@ static void flatten_tree(struct node *tree, struct emitter *emit,
>  	for_each_property(tree, prop) {
>  		int nameoff;
>  
> +		if (prop->undefined)
> +			continue;
> +
>  		if (streq(prop->name, "name"))
>  			seen_name_prop = 1;
>  
> diff --git a/livetree.c b/livetree.c
> index bf8796b..2ef734d 100644
> --- a/livetree.c
> +++ b/livetree.c
> @@ -51,6 +51,11 @@ struct property *build_property(char *name, struct data val)
>  	return new;
>  }
>  
> +void undefine_property(struct property *prop)
> +{
> +	prop->undefined = 1;
> +}
> +
>  struct property *chain_property(struct property *first, struct property *list)
>  {
>  	assert(first->next == NULL);
> @@ -121,11 +126,15 @@ struct node *merge_nodes(struct node *old_node, struct node *new_node)
>  		/* Look for a collision, set new value if there is */
>  		for_each_property(old_node, old_prop) {
>  			if (streq(old_prop->name, new_prop->name)) {
> -				/* Add new labels to old property */
> -				for_each_label(new_prop->labels, l)
> -					add_label(&old_prop->labels, l->label);
> -
> -				old_prop->val = new_prop->val;
> +				if (new_prop->undefined) {
> +					remove_property(old_node, old_prop);
> +				} else {
> +					/* Add new labels to old property */
> +					for_each_label(new_prop->labels, l)
> +						add_label(&old_prop->labels, l->label);
> +
> +					old_prop->val = new_prop->val;
> +				}
>  				free(new_prop);
>  				new_prop = NULL;
>  				break;
> @@ -188,6 +197,25 @@ void add_property(struct node *node, struct property *prop)
>  	*p = prop;
>  }
>  
> +void remove_property(struct node *node, struct property *prop)
> +{
> +	struct property **p;
> +	int found = 0;
> +
> +	p = &node->proplist;
> +	while (*p) {
> +		if (*p == prop) {
> +			*p = (*p)->next;
> +			found = 1;
> +			break;

You could just return at this point, and assert unconditionally if the
loop exits.  That would be slightly more concise.

> +		}
> +		p = &((*p)->next);
> +	}
> +	/* property not in the node? it's probably an error, so flag it. */
> +	assert(found);
> +}
> +
> +
>  void add_child(struct node *parent, struct node *child)
>  {
>  	struct node **p;
> 

Re: [PATCH 4/4] Create a new property value that means 'undefined'.

[David Gibson]

On Wed, Oct 20, 2010 at 11:20:53PM -0600, Grant Likely wrote:
> On Wed, Oct 20, 2010 at 02:45:22PM -0700, John Bonesio wrote:
> > When updating existing nodes in a device tree merge operation, properties
> > can be removed by setting the value to /undef-prop/.
> > 
> > if /undef-prop/ is assigned to a property that doesn't exist, the property
> > is treated the same as if it had not been declared.
> > 
> > Signed-off-by: John Bonesio <bones-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
> 
> Implementation looks good.
> > ---
> > 
> >  dtc-lexer.l  |   14 ++++++++++----
> >  dtc-parser.y |    6 ++++++
> >  dtc.h        |    7 +++++++
> >  flattree.c   |    3 +++
> >  livetree.c   |   38 +++++++++++++++++++++++++++++++++-----
> >  5 files changed, 59 insertions(+), 9 deletions(-)
> > 
> > diff --git a/dtc-lexer.l b/dtc-lexer.l
> > index 216a3d2..efa89b4 100644
> > --- a/dtc-lexer.l
> > +++ b/dtc-lexer.l
> > @@ -102,6 +102,12 @@ static int pop_input_file(void);
> >  			return DT_REMOVENODE;
> >  		}
> >  
> > +<*>"/undef-prop/"	{
> > +			DPRINT("Keyword: /undef-prop/\n");
> > +			BEGIN_DEFAULT();
> > +			return DT_UNDEFINED;
> > +		}
> > +
> 
> Does /undef-prop/ really need to be using <*> to match in all start
> conditions?

It doesn't need to, but it's a good idea for it to do so, because if
the keyword is lexed as a keyword everywhere, it will lead to more
meaningful error messages if it's put somewhere it shouldn't be.

In fact, something I've learnt writing dtc is that in general you
should make your lexical tokens as wide as they can without colliding
with each other, then check that they have the right contents later.
That way you get a clear error message from the checking code
("such-and-so contained an illegal character"), rather than the lexer
breaking it into different tokens instead and the parser generating
some cryptic error.

[snip]
> > @@ -178,6 +179,11 @@ propdef:
> >  		{
> >  			$$ = build_property($1, empty_data);
> >  		}
> > +	| DT_PROPNODENAME '=' DT_UNDEFINED ';'
> 
> Hmmm.  I'm going to make this comment once, but I'll shut-up if you
> guys disagree with me because the details have already been hashed out
> several times, and I've already said I'd be okay with the above form.
> 
> The more I look at it, the more I prefer the form 
> 	/undef-prop/ property;
> instead of
> 	property = /undef-prop/;
> 
> The reason being is that while the assignment form does work, it isn't
> a very natural construct.  Removal is not logically the same as
> assignment.  /undef-prop/ is something that is performed on a
> property.  Syntax that shows /undef-prop/ being assigned as a property
> value doesn't ring true for me as the right thing to do.
> 
> So, my vote is for the "/undef-prop/ property;" form, but I hold my
> piece if you both disagree with me.

I.. yeah.. I'm not sure.  I was leaning towards prop = /undef-prop/;
before, but you've more-or-less persuaded me here.

-- 
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: [PATCH 4/4] Create a new property value that means 'undefined'.

[John Bonesio]

On Thu, 2010-10-21 at 17:19 +1100, David Gibson wrote:
> On Wed, Oct 20, 2010 at 11:20:53PM -0600, Grant Likely wrote:
> > On Wed, Oct 20, 2010 at 02:45:22PM -0700, John Bonesio wrote:
> > > When updating existing nodes in a device tree merge operation, properties
> > > can be removed by setting the value to /undef-prop/.
> > > 
> > > if /undef-prop/ is assigned to a property that doesn't exist, the property
> > > is treated the same as if it had not been declared.
> > > 
> > > Signed-off-by: John Bonesio <bones-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
> > 
> > Implementation looks good.
> > > ---
> > > 
> > >  dtc-lexer.l  |   14 ++++++++++----
> > >  dtc-parser.y |    6 ++++++
> > >  dtc.h        |    7 +++++++
> > >  flattree.c   |    3 +++
> > >  livetree.c   |   38 +++++++++++++++++++++++++++++++++-----
> > >  5 files changed, 59 insertions(+), 9 deletions(-)
> > > 
> > > diff --git a/dtc-lexer.l b/dtc-lexer.l
> > > index 216a3d2..efa89b4 100644
> > > --- a/dtc-lexer.l
> > > +++ b/dtc-lexer.l
> > > @@ -102,6 +102,12 @@ static int pop_input_file(void);
> > >  			return DT_REMOVENODE;
> > >  		}
> > >  
> > > +<*>"/undef-prop/"	{
> > > +			DPRINT("Keyword: /undef-prop/\n");
> > > +			BEGIN_DEFAULT();
> > > +			return DT_UNDEFINED;
> > > +		}
> > > +
> > 
> > Does /undef-prop/ really need to be using <*> to match in all start
> > conditions?
> 
> It doesn't need to, but it's a good idea for it to do so, because if
> the keyword is lexed as a keyword everywhere, it will lead to more
> meaningful error messages if it's put somewhere it shouldn't be.
> 
> In fact, something I've learnt writing dtc is that in general you
> should make your lexical tokens as wide as they can without colliding
> with each other, then check that they have the right contents later.
> That way you get a clear error message from the checking code
> ("such-and-so contained an illegal character"), rather than the lexer
> breaking it into different tokens instead and the parser generating
> some cryptic error.

This is probably what David is saying. Generally you want the lexer to
be context free - meaning everything gets tokenized the same way
everywhere.

The fact that we're using various start conditions in the dtc, is a
little unsettling to me. It makes me wonder if we've got functionality
in the lexer that really should be in the parser.

Most lexers for the 'C' language have only one start condition to kick
things off.


> [snip]
> > > @@ -178,6 +179,11 @@ propdef:
> > >  		{
> > >  			$$ = build_property($1, empty_data);
> > >  		}
> > > +	| DT_PROPNODENAME '=' DT_UNDEFINED ';'
> > 
> > Hmmm.  I'm going to make this comment once, but I'll shut-up if you
> > guys disagree with me because the details have already been hashed out
> > several times, and I've already said I'd be okay with the above form.
> > 
> > The more I look at it, the more I prefer the form 
> > 	/undef-prop/ property;
> > instead of
> > 	property = /undef-prop/;
> > 
> > The reason being is that while the assignment form does work, it isn't
> > a very natural construct.  Removal is not logically the same as
> > assignment.  /undef-prop/ is something that is performed on a
> > property.  Syntax that shows /undef-prop/ being assigned as a property
> > value doesn't ring true for me as the right thing to do.
> > 
> > So, my vote is for the "/undef-prop/ property;" form, but I hold my
> > piece if you both disagree with me.
> 
> I.. yeah.. I'm not sure.  I was leaning towards prop = /undef-prop/;
> before, but you've more-or-less persuaded me here.
> 

Re: [PATCH 4/4] Create a new property value that means 'undefined'.

[David Gibson]

On Thu, Oct 21, 2010 at 08:20:59AM -0700, John Bonesio wrote:
> On Thu, 2010-10-21 at 17:19 +1100, David Gibson wrote:
> > On Wed, Oct 20, 2010 at 11:20:53PM -0600, Grant Likely wrote:
> > > On Wed, Oct 20, 2010 at 02:45:22PM -0700, John Bonesio wrote:
[snip]
> > > Does /undef-prop/ really need to be using <*> to match in all start
> > > conditions?
> > 
> > It doesn't need to, but it's a good idea for it to do so, because if
> > the keyword is lexed as a keyword everywhere, it will lead to more
> > meaningful error messages if it's put somewhere it shouldn't be.
> > 
> > In fact, something I've learnt writing dtc is that in general you
> > should make your lexical tokens as wide as they can without colliding
> > with each other, then check that they have the right contents later.
> > That way you get a clear error message from the checking code
> > ("such-and-so contained an illegal character"), rather than the lexer
> > breaking it into different tokens instead and the parser generating
> > some cryptic error.
> 
> This is probably what David is saying. Generally you want the lexer to
> be context free - meaning everything gets tokenized the same way
> everywhere.

Part of what I was saying, yes.

> The fact that we're using various start conditions in the dtc, is a
> little unsettling to me. It makes me wonder if we've got functionality
> in the lexer that really should be in the parser.

Yeah, it bothered me when it went in, too.  And I've reworked the way
it was done at least once, because it was confusing me.  But I'm
pretty sure it's necessary.

It's basically all because property and node names are lexically
awkward.  They can and do contain a bunch of characters that would
usually be operators or delimiters in a language that's lexically like
C.  But being able to have those property and node names bare means we
have a syntax that's more readable and concise.  But we want the rest
of the syntax to be lexically like C, so we can use C-ish delimiters
and so forth.  So, start conditions it is.

There's also the BYTESTRING start condition.  That's one's also a
convenience / conciseness feature to allow hex blobs of data to be
entered easily.  It's well localized, so again, a reasonable tradeoff
I think.

I just noticed that the INCLUDE state is no longer used, we should
remove it.  And V1 is a hangover from when we supported both dts-v0
and dts-v1 input, which are lexically different, due to the changed
format for integer literals.  We could probably remove that too,
though it would require a bit more care.

-- 
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: [PATCH 4/4] Create a new property value that means 'undefined'.

[John Bonesio]

On Wed, 2010-10-20 at 23:20 -0600, Grant Likely wrote:
> On Wed, Oct 20, 2010 at 02:45:22PM -0700, John Bonesio wrote:
> > When updating existing nodes in a device tree merge operation, properties
> > can be removed by setting the value to /undef-prop/.
> > 
> > if /undef-prop/ is assigned to a property that doesn't exist, the property
> > is treated the same as if it had not been declared.
> > 
> > Signed-off-by: John Bonesio <bones-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
> 
> Implementation looks good.
> > ---
> > 
> >  dtc-lexer.l  |   14 ++++++++++----
> >  dtc-parser.y |    6 ++++++
> >  dtc.h        |    7 +++++++
> >  flattree.c   |    3 +++
> >  livetree.c   |   38 +++++++++++++++++++++++++++++++++-----
> >  5 files changed, 59 insertions(+), 9 deletions(-)
> > 
> > diff --git a/dtc-lexer.l b/dtc-lexer.l
> > index 216a3d2..efa89b4 100644
> > --- a/dtc-lexer.l
> > +++ b/dtc-lexer.l
> > @@ -102,6 +102,12 @@ static int pop_input_file(void);
> >  			return DT_REMOVENODE;
> >  		}
> >  
> > +<*>"/undef-prop/"	{
> > +			DPRINT("Keyword: /undef-prop/\n");
> > +			BEGIN_DEFAULT();
> > +			return DT_UNDEFINED;
> > +		}
> > +
> 
> Does /undef-prop/ really need to be using <*> to match in all start
> conditions?
> 
> >  <*>{LABEL}:	{
> >  			DPRINT("Label: %s\n", yytext);
> >  			yylval.labelref = xstrdup(yytext);
> > @@ -116,10 +122,10 @@ static int pop_input_file(void);
> >  		}
> >  
> >  
> > -<*>\&{LABEL}    {       /* label reference without the braces*/
> > -                        DPRINT("Ref: %s\n", yytext+1);
> > -                        yylval.labelref = xstrdup(yytext+1);
> > -                        return DT_REF;
> > +<*>\&{LABEL}	{       /* label reference without the braces*/
> > +			DPRINT("Ref: %s\n", yytext+1);
> > +			yylval.labelref = xstrdup(yytext+1);
> > +	    		return DT_REF;
> >  		}
> 
> Unrelated whitespace change.  In general, patches should avoid making
> unrelated changes in the same patch, even if they are correct, because
> they decrease the signal-to-noise ratio for patch reviewers.
> Whitespace changes are particularly offensive because the can end up
> masking (to a reviewer) functional changes in the same block.
> 
> >  
> >  <*>"&{"{LABEL}{PATHCHAR}*\} { /* label and/or path refererence with braces */
> > diff --git a/dtc-parser.y b/dtc-parser.y
> > index 0a74c86..ac9cfd7 100644
> > --- a/dtc-parser.y
> > +++ b/dtc-parser.y
> > @@ -56,6 +56,7 @@ static unsigned long long eval_literal(const char *s, int base, int bits);
> >  %token DT_V1
> >  %token DT_MEMRESERVE
> >  %token DT_REMOVENODE
> > +%token DT_UNDEFINED
> >  %token <propnodename> DT_PROPNODENAME
> >  %token <literal> DT_LITERAL
> >  %token <cbase> DT_BASE
> > @@ -178,6 +179,11 @@ propdef:
> >  		{
> >  			$$ = build_property($1, empty_data);
> >  		}
> > +	| DT_PROPNODENAME '=' DT_UNDEFINED ';'
> 
> Hmmm.  I'm going to make this comment once, but I'll shut-up if you
> guys disagree with me because the details have already been hashed out
> several times, and I've already said I'd be okay with the above form.
> 
> The more I look at it, the more I prefer the form 
> 	/undef-prop/ property;
> instead of
> 	property = /undef-prop/;
> 
> The reason being is that while the assignment form does work, it isn't
> a very natural construct.  Removal is not logically the same as
> assignment.  /undef-prop/ is something that is performed on a
> property.  Syntax that shows /undef-prop/ being assigned as a property
> value doesn't ring true for me as the right thing to do.
> 
> So, my vote is for the "/undef-prop/ property;" form, but I hold my
> piece if you both disagree with me.

I'd be ok with /undef-prop/ property;

This seems to fit with our /remove-node/ node; syntax. If we do this, I
think it would be better to use
	/remove-prop/ property;


> > +		{
> > +			$$ = build_property($1, empty_data);
> > +			undefine_property($$)
> 
> Don't bother with the undefine_property function.  Just do:
> 			$$->undefined = 1;
> 
> > +		}
> 
> >  	| DT_LABEL propdef
> >  		{
> >  			add_label(&$2->labels, $1);
> > diff --git a/dtc.h b/dtc.h
> > index a7f3667..b3fca6e 100644
> > --- a/dtc.h
> > +++ b/dtc.h
> > @@ -130,7 +130,12 @@ struct label {
> >  	struct label *next;
> >  };
> >  
> > +#define PROP_DEFINED (0)
> > +#define PROP_UNDEFINED (1)
> 
> No need for the #defines. Just use 0 and 1 in the code when working
> with boolean flags.  Besides, these #defines aren't used anywhere
> anyway.  :-)
> 
> > +
> >  struct property {
> > +	int undefined;  /* if the property is set to undefined, this feild is
> > +	                   set to PROP_UNDEFINED */
> >  	char *name;
> >  	struct data val;
> >  
> > @@ -170,6 +175,7 @@ void add_label(struct label **labels, char *label);
> >  struct property *build_property(char *name, struct data val);
> >  struct property *chain_property(struct property *first, struct property *list);
> >  struct property *reverse_properties(struct property *first);
> > +void undefine_property(struct property *prop);
> >  
> >  struct node *build_node(struct property *proplist, struct node *children);
> >  struct node *name_node(struct node *node, char *name);
> > @@ -177,6 +183,7 @@ struct node *chain_node(struct node *first, struct node *list);
> >  struct node *merge_nodes(struct node *old_node, struct node *new_node);
> >  
> >  void add_property(struct node *node, struct property *prop);
> > +void remove_property(struct node *node, struct property *prop);
> >  void add_child(struct node *parent, struct node *child);
> >  void remove_child(struct node *parent, struct node *child);
> >  
> > diff --git a/flattree.c b/flattree.c
> > index ead0332..00439e9 100644
> > --- a/flattree.c
> > +++ b/flattree.c
> > @@ -275,6 +275,9 @@ static void flatten_tree(struct node *tree, struct emitter *emit,
> >  	for_each_property(tree, prop) {
> >  		int nameoff;
> >  
> > +		if (prop->undefined)
> > +			continue;
> > +
> >  		if (streq(prop->name, "name"))
> >  			seen_name_prop = 1;
> >  
> > diff --git a/livetree.c b/livetree.c
> > index bf8796b..2ef734d 100644
> > --- a/livetree.c
> > +++ b/livetree.c
> > @@ -51,6 +51,11 @@ struct property *build_property(char *name, struct data val)
> >  	return new;
> >  }
> >  
> > +void undefine_property(struct property *prop)
> > +{
> > +	prop->undefined = 1;
> > +}
> > +
> >  struct property *chain_property(struct property *first, struct property *list)
> >  {
> >  	assert(first->next == NULL);
> > @@ -121,11 +126,15 @@ struct node *merge_nodes(struct node *old_node, struct node *new_node)
> >  		/* Look for a collision, set new value if there is */
> >  		for_each_property(old_node, old_prop) {
> >  			if (streq(old_prop->name, new_prop->name)) {
> > -				/* Add new labels to old property */
> > -				for_each_label(new_prop->labels, l)
> > -					add_label(&old_prop->labels, l->label);
> > -
> > -				old_prop->val = new_prop->val;
> > +				if (new_prop->undefined) {
> > +					remove_property(old_node, old_prop);
> > +				} else {
> > +					/* Add new labels to old property */
> > +					for_each_label(new_prop->labels, l)
> > +						add_label(&old_prop->labels, l->label);
> > +
> > +					old_prop->val = new_prop->val;
> > +				}
> >  				free(new_prop);
> >  				new_prop = NULL;
> >  				break;
> > @@ -188,6 +197,25 @@ void add_property(struct node *node, struct property *prop)
> >  	*p = prop;
> >  }
> >  
> > +void remove_property(struct node *node, struct property *prop)
> > +{
> > +	struct property **p;
> > +	int found = 0;
> > +
> > +	p = &node->proplist;
> > +	while (*p) {
> > +		if (*p == prop) {
> > +			*p = (*p)->next;
> > +			found = 1;
> > +			break;
> 
> You could just return at this point, and assert unconditionally if the
> loop exits.  That would be slightly more concise.
> 
> > +		}
> > +		p = &((*p)->next);
> > +	}
> > +	/* property not in the node? it's probably an error, so flag it. */
> > +	assert(found);
> > +}
> > +
> > +
> >  void add_child(struct node *parent, struct node *child)
> >  {
> >  	struct node **p;
> > 

Re: [PATCH 4/4] Create a new property value that means 'undefined'.

[David Gibson]

On Wed, Oct 20, 2010 at 02:45:22PM -0700, John Bonesio wrote:
> When updating existing nodes in a device tree merge operation, properties
> can be removed by setting the value to /undef-prop/.
> 
> if /undef-prop/ is assigned to a property that doesn't exist, the property
> is treated the same as if it had not been declared.

[snip]
> diff --git a/dtc.h b/dtc.h
> index a7f3667..b3fca6e 100644
> --- a/dtc.h
> +++ b/dtc.h
> @@ -130,7 +130,12 @@ struct label {
>  	struct label *next;
>  };
>  
> +#define PROP_DEFINED (0)
> +#define PROP_UNDEFINED (1)

You never actually use these constants.

> +
>  struct property {
> +	int undefined;  /* if the property is set to undefined, this feild is
> +	                   set to PROP_UNDEFINED */
>  	char *name;
>  	struct data val;
>  
> @@ -170,6 +175,7 @@ void add_label(struct label **labels, char *label);
>  struct property *build_property(char *name, struct data val);
>  struct property *chain_property(struct property *first, struct property *list);
>  struct property *reverse_properties(struct property *first);
> +void undefine_property(struct property *prop);
>  
>  struct node *build_node(struct property *proplist, struct node *children);
>  struct node *name_node(struct node *node, char *name);
> @@ -177,6 +183,7 @@ struct node *chain_node(struct node *first, struct node *list);
>  struct node *merge_nodes(struct node *old_node, struct node *new_node);
>  
>  void add_property(struct node *node, struct property *prop);
> +void remove_property(struct node *node, struct property *prop);
>  void add_child(struct node *parent, struct node *child);
>  void remove_child(struct node *parent, struct node *child);
>  
> diff --git a/flattree.c b/flattree.c
> index ead0332..00439e9 100644
> --- a/flattree.c
> +++ b/flattree.c
> @@ -275,6 +275,9 @@ static void flatten_tree(struct node *tree, struct emitter *emit,
>  	for_each_property(tree, prop) {
>  		int nameoff;
>  
> +		if (prop->undefined)
> +			continue;
> +
>  		if (streq(prop->name, "name"))
>  			seen_name_prop = 1;
>  
> diff --git a/livetree.c b/livetree.c
> index bf8796b..2ef734d 100644
> --- a/livetree.c
> +++ b/livetree.c
> @@ -51,6 +51,11 @@ struct property *build_property(char *name, struct data val)
>  	return new;
>  }
>  
> +void undefine_property(struct property *prop)
> +{
> +	prop->undefined = 1;
> +}
> +
>  struct property *chain_property(struct property *first, struct property *list)
>  {
>  	assert(first->next == NULL);
> @@ -121,11 +126,15 @@ struct node *merge_nodes(struct node *old_node, struct node *new_node)
>  		/* Look for a collision, set new value if there is */
>  		for_each_property(old_node, old_prop) {
>  			if (streq(old_prop->name, new_prop->name)) {
> -				/* Add new labels to old property */
> -				for_each_label(new_prop->labels, l)
> -					add_label(&old_prop->labels, l->label);
> -
> -				old_prop->val = new_prop->val;
> +				if (new_prop->undefined) {
> +					remove_property(old_node,
> old_prop);

This will lose property labels into the ether as your earlier patch
lost node labels.

> +				} else {
> +					/* Add new labels to old property */
> +					for_each_label(new_prop->labels, l)
> +						add_label(&old_prop->labels, l->label);
> +
> +					old_prop->val = new_prop->val;
> +				}
>  				free(new_prop);
>  				new_prop = NULL;
>  				break;
> @@ -188,6 +197,25 @@ void add_property(struct node *node, struct property *prop)
>  	*p = prop;
>  }
>  
> +void remove_property(struct node *node, struct property *prop)
> +{
> +	struct property **p;
> +	int found = 0;
> +
> +	p = &node->proplist;
> +	while (*p) {
> +		if (*p == prop) {
> +			*p = (*p)->next;
> +			found = 1;
> +			break;
> +		}
> +		p = &((*p)->next);
> +	}
> +	/* property not in the node? it's probably an error, so flag it. */
> +	assert(found);

assert()s are for finding bugs in code, not detecting errors in user
input.  AFAICT this *can* be generated by usre input so it should
print an error message and if possible continue, not cause a SIGABRT.

> +}
> +
> +
>  void add_child(struct node *parent, struct node *child)
>  {
>  	struct node **p;
> 

-- 
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: [PATCH 4/4] Create a new property value that means 'undefined'.

[John Bonesio]

On Thu, 2010-10-21 at 17:14 +1100, David Gibson wrote:
> On Wed, Oct 20, 2010 at 02:45:22PM -0700, John Bonesio wrote:
> > When updating existing nodes in a device tree merge operation, properties
> > can be removed by setting the value to /undef-prop/.
> > 
> > if /undef-prop/ is assigned to a property that doesn't exist, the property
> > is treated the same as if it had not been declared.
> 
> [snip]
> > diff --git a/dtc.h b/dtc.h
> > index a7f3667..b3fca6e 100644
> > --- a/dtc.h
> > +++ b/dtc.h
> > @@ -130,7 +130,12 @@ struct label {
> >  	struct label *next;
> >  };
> >  
> > +#define PROP_DEFINED (0)
> > +#define PROP_UNDEFINED (1)
> 
> You never actually use these constants.
> 
> > +
> >  struct property {
> > +	int undefined;  /* if the property is set to undefined, this feild is
> > +	                   set to PROP_UNDEFINED */
> >  	char *name;
> >  	struct data val;
> >  
> > @@ -170,6 +175,7 @@ void add_label(struct label **labels, char *label);
> >  struct property *build_property(char *name, struct data val);
> >  struct property *chain_property(struct property *first, struct property *list);
> >  struct property *reverse_properties(struct property *first);
> > +void undefine_property(struct property *prop);
> >  
> >  struct node *build_node(struct property *proplist, struct node *children);
> >  struct node *name_node(struct node *node, char *name);
> > @@ -177,6 +183,7 @@ struct node *chain_node(struct node *first, struct node *list);
> >  struct node *merge_nodes(struct node *old_node, struct node *new_node);
> >  
> >  void add_property(struct node *node, struct property *prop);
> > +void remove_property(struct node *node, struct property *prop);
> >  void add_child(struct node *parent, struct node *child);
> >  void remove_child(struct node *parent, struct node *child);
> >  
> > diff --git a/flattree.c b/flattree.c
> > index ead0332..00439e9 100644
> > --- a/flattree.c
> > +++ b/flattree.c
> > @@ -275,6 +275,9 @@ static void flatten_tree(struct node *tree, struct emitter *emit,
> >  	for_each_property(tree, prop) {
> >  		int nameoff;
> >  
> > +		if (prop->undefined)
> > +			continue;
> > +
> >  		if (streq(prop->name, "name"))
> >  			seen_name_prop = 1;
> >  
> > diff --git a/livetree.c b/livetree.c
> > index bf8796b..2ef734d 100644
> > --- a/livetree.c
> > +++ b/livetree.c
> > @@ -51,6 +51,11 @@ struct property *build_property(char *name, struct data val)
> >  	return new;
> >  }
> >  
> > +void undefine_property(struct property *prop)
> > +{
> > +	prop->undefined = 1;
> > +}
> > +
> >  struct property *chain_property(struct property *first, struct property *list)
> >  {
> >  	assert(first->next == NULL);
> > @@ -121,11 +126,15 @@ struct node *merge_nodes(struct node *old_node, struct node *new_node)
> >  		/* Look for a collision, set new value if there is */
> >  		for_each_property(old_node, old_prop) {
> >  			if (streq(old_prop->name, new_prop->name)) {
> > -				/* Add new labels to old property */
> > -				for_each_label(new_prop->labels, l)
> > -					add_label(&old_prop->labels, l->label);
> > -
> > -				old_prop->val = new_prop->val;
> > +				if (new_prop->undefined) {
> > +					remove_property(old_node,
> > old_prop);
> 
> This will lose property labels into the ether as your earlier patch
> lost node labels.
> 
> > +				} else {
> > +					/* Add new labels to old property */
> > +					for_each_label(new_prop->labels, l)
> > +						add_label(&old_prop->labels, l->label);
> > +
> > +					old_prop->val = new_prop->val;
> > +				}
> >  				free(new_prop);
> >  				new_prop = NULL;
> >  				break;
> > @@ -188,6 +197,25 @@ void add_property(struct node *node, struct property *prop)
> >  	*p = prop;
> >  }
> >  
> > +void remove_property(struct node *node, struct property *prop)
> > +{
> > +	struct property **p;
> > +	int found = 0;
> > +
> > +	p = &node->proplist;
> > +	while (*p) {
> > +		if (*p == prop) {
> > +			*p = (*p)->next;
> > +			found = 1;
> > +			break;
> > +		}
> > +		p = &((*p)->next);
> > +	}
> > +	/* property not in the node? it's probably an error, so flag it. */
> > +	assert(found);
> 
> assert()s are for finding bugs in code, not detecting errors in user
> input.  AFAICT this *can* be generated by usre input so it should
> print an error message and if possible continue, not cause a SIGABRT.

I agree that assert() shouldn't be used if it can be caused by user
input, but I don't see how the assert can be triggered by user input. In
my mind this is a programming error.

I the only way I see this being triggered by user input is if the user
specifies that a property be deleted but it doesn't exist in the current
in-memory merged tree. I believe the merge_nodes() routine takes care
to avoid problems here by making sure the property already exists in the
in-memory merged tree before calling remove_property() on it.

Maybe there's a case I'm not considering.

> 
> > +}
> > +
> > +
> >  void add_child(struct node *parent, struct node *child)
> >  {
> >  	struct node **p;
> > 
> 

Re: [PATCH 4/4] Create a new property value that means 'undefined'.

[David Gibson]

On Fri, Oct 22, 2010 at 12:50:58PM -0700, John Bonesio wrote:
> On Thu, 2010-10-21 at 17:14 +1100, David Gibson wrote:
> > On Wed, Oct 20, 2010 at 02:45:22PM -0700, John Bonesio wrote:
[snip]
> > > @@ -188,6 +197,25 @@ void add_property(struct node *node, struct property *prop)
> > >  	*p = prop;
> > >  }
> > >  
> > > +void remove_property(struct node *node, struct property *prop)
> > > +{
> > > +	struct property **p;
> > > +	int found = 0;
> > > +
> > > +	p = &node->proplist;
> > > +	while (*p) {
> > > +		if (*p == prop) {
> > > +			*p = (*p)->next;
> > > +			found = 1;
> > > +			break;
> > > +		}
> > > +		p = &((*p)->next);
> > > +	}
> > > +	/* property not in the node? it's probably an error, so flag it. */
> > > +	assert(found);
> > 
> > assert()s are for finding bugs in code, not detecting errors in user
> > input.  AFAICT this *can* be generated by usre input so it should
> > print an error message and if possible continue, not cause a SIGABRT.
> 
> I agree that assert() shouldn't be used if it can be caused by user
> input, but I don't see how the assert can be triggered by user input. In
> my mind this is a programming error.
> 
> I the only way I see this being triggered by user input is if the user
> specifies that a property be deleted but it doesn't exist in the current
> in-memory merged tree. I believe the merge_nodes() routine takes care
> to avoid problems here by making sure the property already exists in the
> in-memory merged tree before calling remove_property() on it.

Well, it does, but remove_property() is called *from* merge_nodes()
and it's called *before* the new property is added to the old node.
So, unless I'm missing something even more subtle, the user attempting
to remove a property which does not exist will indeed trigger this
assert().  This would make a good testcase...

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

[PATCH 0/4] Series short description

[Tony Lindgren]

Hi all,

Here are the changes needed to make hardware.h local for mach-omap2.
These patches are based on v3.6-rc3 and the following patches:

- Arnd's patch "ARM: omap: move platform_data definitions"
- Igor's series "ARM: OMAP: cleanup plat/board.h file"
- Afzal's series "Prepare for GPMC driver conversion (w.r.t MTD)"
- My series "Clean up hardcoded IRQs for mach-omap2, enable SPARSE_IRQ,
  plaform_data/gpio-omap.h"

Regards,

Tony

---

Tony Lindgren (4):
      ARM: OMAP: Remove unused old gpio-switch.h
      ARM: OMAP: Split plat/hardware.h, introduce local hardware.h and soc.h for omap2+
      ARM: OMAP2+ Move SoC specific headers to be local to mach-omap2
      ARM: OMAP1: Move SoC specific headers from plat to mach for omap1


 arch/arm/mach-omap1/board-htcherald.c          |    4 
 arch/arm/mach-omap1/devices.c                  |    2 
 arch/arm/mach-omap1/include/mach/hardware.h    |  285 +++++++++++++++++++++++
 arch/arm/mach-omap1/include/mach/omap1510.h    |    3 
 arch/arm/mach-omap1/include/mach/omap16xx.h    |    3 
 arch/arm/mach-omap1/include/mach/omap7xx.h     |    3 
 arch/arm/mach-omap2/am33xx.h                   |    0 
 arch/arm/mach-omap2/board-2430sdp.c            |    2 
 arch/arm/mach-omap2/board-3430sdp.c            |    9 -
 arch/arm/mach-omap2/board-4430sdp.c            |    9 -
 arch/arm/mach-omap2/board-am3517crane.c        |    1 
 arch/arm/mach-omap2/board-am3517evm.c          |    1 
 arch/arm/mach-omap2/board-apollon.c            |    3 
 arch/arm/mach-omap2/board-cm-t35.c             |    2 
 arch/arm/mach-omap2/board-cm-t3517.c           |    3 
 arch/arm/mach-omap2/board-devkit8000.c         |   22 +-
 arch/arm/mach-omap2/board-flash.c              |    6 
 arch/arm/mach-omap2/board-generic.c            |    1 
 arch/arm/mach-omap2/board-h4.c                 |    3 
 arch/arm/mach-omap2/board-igep0020.c           |    1 
 arch/arm/mach-omap2/board-ldp.c                |   10 -
 arch/arm/mach-omap2/board-n8x0.c               |   10 -
 arch/arm/mach-omap2/board-omap3beagle.c        |   12 -
 arch/arm/mach-omap2/board-omap3evm.c           |   11 -
 arch/arm/mach-omap2/board-omap3logic.c         |   16 -
 arch/arm/mach-omap2/board-omap3pandora.c       |    9 -
 arch/arm/mach-omap2/board-omap3stalker.c       |   20 +-
 arch/arm/mach-omap2/board-omap3touchbook.c     |    9 -
 arch/arm/mach-omap2/board-omap4panda.c         |    7 -
 arch/arm/mach-omap2/board-overo.c              |   14 -
 arch/arm/mach-omap2/board-rm680.c              |    1 
 arch/arm/mach-omap2/board-rx51.c               |    5 
 arch/arm/mach-omap2/board-ti8168evm.c          |    2 
 arch/arm/mach-omap2/board-zoom-debugboard.c    |    4 
 arch/arm/mach-omap2/board-zoom-display.c       |    4 
 arch/arm/mach-omap2/board-zoom-peripherals.c   |    1 
 arch/arm/mach-omap2/clkt2xxx_virt_prcm_set.c   |    2 
 arch/arm/mach-omap2/clkt_dpll.c                |    2 
 arch/arm/mach-omap2/clock.c                    |    8 -
 arch/arm/mach-omap2/clock2420_data.c           |    2 
 arch/arm/mach-omap2/clock2430.c                |    2 
 arch/arm/mach-omap2/clock2430_data.c           |    2 
 arch/arm/mach-omap2/clock2xxx.c                |    2 
 arch/arm/mach-omap2/clock33xx_data.c           |    2 
 arch/arm/mach-omap2/clock3xxx.c                |    2 
 arch/arm/mach-omap2/clock3xxx_data.c           |    3 
 arch/arm/mach-omap2/clock44xx_data.c           |    3 
 arch/arm/mach-omap2/cm2xxx_3xxx.c              |    3 
 arch/arm/mach-omap2/common.c                   |    2 
 arch/arm/mach-omap2/common.h                   |    6 
 arch/arm/mach-omap2/control.c                  |    2 
 arch/arm/mach-omap2/control.h                  |    2 
 arch/arm/mach-omap2/devices.c                  |    1 
 arch/arm/mach-omap2/dpll3xxx.c                 |    2 
 arch/arm/mach-omap2/dpll44xx.c                 |    2 
 arch/arm/mach-omap2/emu.c                      |    2 
 arch/arm/mach-omap2/gpmc-nand.c                |    5 
 arch/arm/mach-omap2/gpmc-onenand.c             |    5 
 arch/arm/mach-omap2/gpmc-smc91x.c              |    3 
 arch/arm/mach-omap2/gpmc.c                     |    4 
 arch/arm/mach-omap2/hardware.h                 |    6 
 arch/arm/mach-omap2/hsmmc.c                    |    2 
 arch/arm/mach-omap2/i2c.c                      |    1 
 arch/arm/mach-omap2/id.c                       |    2 
 arch/arm/mach-omap2/include/mach/hardware.h    |    2 
 arch/arm/mach-omap2/io.c                       |    3 
 arch/arm/mach-omap2/irq.c                      |    3 
 arch/arm/mach-omap2/mailbox.c                  |    3 
 arch/arm/mach-omap2/mcbsp.c                    |    5 
 arch/arm/mach-omap2/omap-mpuss-lowpower.c      |    3 
 arch/arm/mach-omap2/omap-smp.c                 |    2 
 arch/arm/mach-omap2/omap-wakeupgen.c           |    2 
 arch/arm/mach-omap2/omap24xx.h                 |    2 
 arch/arm/mach-omap2/omap34xx.h                 |    2 
 arch/arm/mach-omap2/omap4-common.c             |    2 
 arch/arm/mach-omap2/omap44xx.h                 |    1 
 arch/arm/mach-omap2/omap54xx.h                 |    0 
 arch/arm/mach-omap2/omap_hwmod.c               |    8 -
 arch/arm/mach-omap2/omap_hwmod_2420_data.c     |    5 
 arch/arm/mach-omap2/omap_hwmod_2430_data.c     |    5 
 arch/arm/mach-omap2/omap_hwmod_3xxx_data.c     |    3 
 arch/arm/mach-omap2/omap_hwmod_44xx_data.c     |    1 
 arch/arm/mach-omap2/omap_l3_noc.c              |    3 
 arch/arm/mach-omap2/omap_phy_internal.c        |    3 
 arch/arm/mach-omap2/opp2420_data.c             |    2 
 arch/arm/mach-omap2/opp2430_data.c             |    2 
 arch/arm/mach-omap2/opp3xxx_data.c             |    2 
 arch/arm/mach-omap2/opp4xxx_data.c             |    3 
 arch/arm/mach-omap2/powerdomain.c              |    6 
 arch/arm/mach-omap2/powerdomains3xxx_data.c    |    4 
 arch/arm/mach-omap2/prm2xxx_3xxx.c             |    4 
 arch/arm/mach-omap2/prm44xx.c                  |    4 
 arch/arm/mach-omap2/sdrc2xxx.c                 |    2 
 arch/arm/mach-omap2/sleep24xx.S                |    3 
 arch/arm/mach-omap2/sleep34xx.S                |    2 
 arch/arm/mach-omap2/sleep44xx.S                |    2 
 arch/arm/mach-omap2/soc.h                      |    1 
 arch/arm/mach-omap2/sram242x.S                 |    3 
 arch/arm/mach-omap2/sram243x.S                 |    3 
 arch/arm/mach-omap2/sram34xx.S                 |    3 
 arch/arm/mach-omap2/ti81xx.h                   |    0 
 arch/arm/mach-omap2/timer.c                    |    3 
 arch/arm/mach-omap2/twl-common.c               |    2 
 arch/arm/mach-omap2/usb-host.c                 |    1 
 arch/arm/mach-omap2/usb-musb.c                 |    6 
 arch/arm/mach-omap2/vc.c                       |    3 
 arch/arm/mach-omap2/voltagedomains3xxx_data.c  |    3 
 arch/arm/plat-omap/Makefile                    |    3 
 arch/arm/plat-omap/counter_32k.c               |    2 
 arch/arm/plat-omap/devices.c                   |   91 -------
 arch/arm/plat-omap/dma.c                       |    3 
 arch/arm/plat-omap/include/plat/gpio-switch.h  |   54 ----
 arch/arm/plat-omap/include/plat/hardware.h     |  293 ------------------------
 arch/arm/plat-omap/include/plat/omap4-keypad.h |    2 
 drivers/spi/spi-omap-uwire.c                   |    3 
 drivers/watchdog/omap_wdt.c                    |    5 
 sound/soc/omap/omap-abe-twl6040.c              |    4 
 sound/soc/omap/omap-mcpdm.c                    |    2 
 118 files changed, 475 insertions(+), 687 deletions(-)
 rename arch/arm/{plat-omap/include/plat/omap1510.h => mach-omap1/include/mach/omap1510.h} (97%)
 rename arch/arm/{plat-omap/include/plat/omap16xx.h => mach-omap1/include/mach/omap16xx.h} (99%)
 rename arch/arm/{plat-omap/include/plat/omap7xx.h => mach-omap1/include/mach/omap7xx.h} (98%)
 rename arch/arm/{plat-omap/include/plat/am33xx.h => mach-omap2/am33xx.h} (100%)
 create mode 100644 arch/arm/mach-omap2/hardware.h
 rename arch/arm/{plat-omap/include/plat/omap24xx.h => mach-omap2/omap24xx.h} (98%)
 rename arch/arm/{plat-omap/include/plat/omap34xx.h => mach-omap2/omap34xx.h} (98%)
 rename arch/arm/{plat-omap/include/plat/omap44xx.h => mach-omap2/omap44xx.h} (98%)
 rename arch/arm/{plat-omap/include/plat/omap54xx.h => mach-omap2/omap54xx.h} (100%)
 create mode 100644 arch/arm/mach-omap2/soc.h
 rename arch/arm/{plat-omap/include/plat/ti81xx.h => mach-omap2/ti81xx.h} (100%)
 delete mode 100644 arch/arm/plat-omap/devices.c
 delete mode 100644 arch/arm/plat-omap/include/plat/gpio-switch.h
 delete mode 100644 arch/arm/plat-omap/include/plat/hardware.h

-- 
Signature

[PATCH 0/4] Series short description

[Tony Lindgren]

Hi all,

Here are the changes needed to make hardware.h local for mach-omap2.
These patches are based on v3.6-rc3 and the following patches:

- Arnd's patch "ARM: omap: move platform_data definitions"
- Igor's series "ARM: OMAP: cleanup plat/board.h file"
- Afzal's series "Prepare for GPMC driver conversion (w.r.t MTD)"
- My series "Clean up hardcoded IRQs for mach-omap2, enable SPARSE_IRQ,
  plaform_data/gpio-omap.h"

Regards,

Tony

---

Tony Lindgren (4):
      ARM: OMAP: Remove unused old gpio-switch.h
      ARM: OMAP: Split plat/hardware.h, introduce local hardware.h and soc.h for omap2+
      ARM: OMAP2+ Move SoC specific headers to be local to mach-omap2
      ARM: OMAP1: Move SoC specific headers from plat to mach for omap1


 arch/arm/mach-omap1/board-htcherald.c          |    4 
 arch/arm/mach-omap1/devices.c                  |    2 
 arch/arm/mach-omap1/include/mach/hardware.h    |  285 +++++++++++++++++++++++
 arch/arm/mach-omap1/include/mach/omap1510.h    |    3 
 arch/arm/mach-omap1/include/mach/omap16xx.h    |    3 
 arch/arm/mach-omap1/include/mach/omap7xx.h     |    3 
 arch/arm/mach-omap2/am33xx.h                   |    0 
 arch/arm/mach-omap2/board-2430sdp.c            |    2 
 arch/arm/mach-omap2/board-3430sdp.c            |    9 -
 arch/arm/mach-omap2/board-4430sdp.c            |    9 -
 arch/arm/mach-omap2/board-am3517crane.c        |    1 
 arch/arm/mach-omap2/board-am3517evm.c          |    1 
 arch/arm/mach-omap2/board-apollon.c            |    3 
 arch/arm/mach-omap2/board-cm-t35.c             |    2 
 arch/arm/mach-omap2/board-cm-t3517.c           |    3 
 arch/arm/mach-omap2/board-devkit8000.c         |   22 +-
 arch/arm/mach-omap2/board-flash.c              |    6 
 arch/arm/mach-omap2/board-generic.c            |    1 
 arch/arm/mach-omap2/board-h4.c                 |    3 
 arch/arm/mach-omap2/board-igep0020.c           |    1 
 arch/arm/mach-omap2/board-ldp.c                |   10 -
 arch/arm/mach-omap2/board-n8x0.c               |   10 -
 arch/arm/mach-omap2/board-omap3beagle.c        |   12 -
 arch/arm/mach-omap2/board-omap3evm.c           |   11 -
 arch/arm/mach-omap2/board-omap3logic.c         |   16 -
 arch/arm/mach-omap2/board-omap3pandora.c       |    9 -
 arch/arm/mach-omap2/board-omap3stalker.c       |   20 +-
 arch/arm/mach-omap2/board-omap3touchbook.c     |    9 -
 arch/arm/mach-omap2/board-omap4panda.c         |    7 -
 arch/arm/mach-omap2/board-overo.c              |   14 -
 arch/arm/mach-omap2/board-rm680.c              |    1 
 arch/arm/mach-omap2/board-rx51.c               |    5 
 arch/arm/mach-omap2/board-ti8168evm.c          |    2 
 arch/arm/mach-omap2/board-zoom-debugboard.c    |    4 
 arch/arm/mach-omap2/board-zoom-display.c       |    4 
 arch/arm/mach-omap2/board-zoom-peripherals.c   |    1 
 arch/arm/mach-omap2/clkt2xxx_virt_prcm_set.c   |    2 
 arch/arm/mach-omap2/clkt_dpll.c                |    2 
 arch/arm/mach-omap2/clock.c                    |    8 -
 arch/arm/mach-omap2/clock2420_data.c           |    2 
 arch/arm/mach-omap2/clock2430.c                |    2 
 arch/arm/mach-omap2/clock2430_data.c           |    2 
 arch/arm/mach-omap2/clock2xxx.c                |    2 
 arch/arm/mach-omap2/clock33xx_data.c           |    2 
 arch/arm/mach-omap2/clock3xxx.c                |    2 
 arch/arm/mach-omap2/clock3xxx_data.c           |    3 
 arch/arm/mach-omap2/clock44xx_data.c           |    3 
 arch/arm/mach-omap2/cm2xxx_3xxx.c              |    3 
 arch/arm/mach-omap2/common.c                   |    2 
 arch/arm/mach-omap2/common.h                   |    6 
 arch/arm/mach-omap2/control.c                  |    2 
 arch/arm/mach-omap2/control.h                  |    2 
 arch/arm/mach-omap2/devices.c                  |    1 
 arch/arm/mach-omap2/dpll3xxx.c                 |    2 
 arch/arm/mach-omap2/dpll44xx.c                 |    2 
 arch/arm/mach-omap2/emu.c                      |    2 
 arch/arm/mach-omap2/gpmc-nand.c                |    5 
 arch/arm/mach-omap2/gpmc-onenand.c             |    5 
 arch/arm/mach-omap2/gpmc-smc91x.c              |    3 
 arch/arm/mach-omap2/gpmc.c                     |    4 
 arch/arm/mach-omap2/hardware.h                 |    6 
 arch/arm/mach-omap2/hsmmc.c                    |    2 
 arch/arm/mach-omap2/i2c.c                      |    1 
 arch/arm/mach-omap2/id.c                       |    2 
 arch/arm/mach-omap2/include/mach/hardware.h    |    2 
 arch/arm/mach-omap2/io.c                       |    3 
 arch/arm/mach-omap2/irq.c                      |    3 
 arch/arm/mach-omap2/mailbox.c                  |    3 
 arch/arm/mach-omap2/mcbsp.c                    |    5 
 arch/arm/mach-omap2/omap-mpuss-lowpower.c      |    3 
 arch/arm/mach-omap2/omap-smp.c                 |    2 
 arch/arm/mach-omap2/omap-wakeupgen.c           |    2 
 arch/arm/mach-omap2/omap24xx.h                 |    2 
 arch/arm/mach-omap2/omap34xx.h                 |    2 
 arch/arm/mach-omap2/omap4-common.c             |    2 
 arch/arm/mach-omap2/omap44xx.h                 |    1 
 arch/arm/mach-omap2/omap54xx.h                 |    0 
 arch/arm/mach-omap2/omap_hwmod.c               |    8 -
 arch/arm/mach-omap2/omap_hwmod_2420_data.c     |    5 
 arch/arm/mach-omap2/omap_hwmod_2430_data.c     |    5 
 arch/arm/mach-omap2/omap_hwmod_3xxx_data.c     |    3 
 arch/arm/mach-omap2/omap_hwmod_44xx_data.c     |    1 
 arch/arm/mach-omap2/omap_l3_noc.c              |    3 
 arch/arm/mach-omap2/omap_phy_internal.c        |    3 
 arch/arm/mach-omap2/opp2420_data.c             |    2 
 arch/arm/mach-omap2/opp2430_data.c             |    2 
 arch/arm/mach-omap2/opp3xxx_data.c             |    2 
 arch/arm/mach-omap2/opp4xxx_data.c             |    3 
 arch/arm/mach-omap2/powerdomain.c              |    6 
 arch/arm/mach-omap2/powerdomains3xxx_data.c    |    4 
 arch/arm/mach-omap2/prm2xxx_3xxx.c             |    4 
 arch/arm/mach-omap2/prm44xx.c                  |    4 
 arch/arm/mach-omap2/sdrc2xxx.c                 |    2 
 arch/arm/mach-omap2/sleep24xx.S                |    3 
 arch/arm/mach-omap2/sleep34xx.S                |    2 
 arch/arm/mach-omap2/sleep44xx.S                |    2 
 arch/arm/mach-omap2/soc.h                      |    1 
 arch/arm/mach-omap2/sram242x.S                 |    3 
 arch/arm/mach-omap2/sram243x.S                 |    3 
 arch/arm/mach-omap2/sram34xx.S                 |    3 
 arch/arm/mach-omap2/ti81xx.h                   |    0 
 arch/arm/mach-omap2/timer.c                    |    3 
 arch/arm/mach-omap2/twl-common.c               |    2 
 arch/arm/mach-omap2/usb-host.c                 |    1 
 arch/arm/mach-omap2/usb-musb.c                 |    6 
 arch/arm/mach-omap2/vc.c                       |    3 
 arch/arm/mach-omap2/voltagedomains3xxx_data.c  |    3 
 arch/arm/plat-omap/Makefile                    |    3 
 arch/arm/plat-omap/counter_32k.c               |    2 
 arch/arm/plat-omap/devices.c                   |   91 -------
 arch/arm/plat-omap/dma.c                       |    3 
 arch/arm/plat-omap/include/plat/gpio-switch.h  |   54 ----
 arch/arm/plat-omap/include/plat/hardware.h     |  293 ------------------------
 arch/arm/plat-omap/include/plat/omap4-keypad.h |    2 
 drivers/spi/spi-omap-uwire.c                   |    3 
 drivers/watchdog/omap_wdt.c                    |    5 
 sound/soc/omap/omap-abe-twl6040.c              |    4 
 sound/soc/omap/omap-mcpdm.c                    |    2 
 118 files changed, 475 insertions(+), 687 deletions(-)
 rename arch/arm/{plat-omap/include/plat/omap1510.h => mach-omap1/include/mach/omap1510.h} (97%)
 rename arch/arm/{plat-omap/include/plat/omap16xx.h => mach-omap1/include/mach/omap16xx.h} (99%)
 rename arch/arm/{plat-omap/include/plat/omap7xx.h => mach-omap1/include/mach/omap7xx.h} (98%)
 rename arch/arm/{plat-omap/include/plat/am33xx.h => mach-omap2/am33xx.h} (100%)
 create mode 100644 arch/arm/mach-omap2/hardware.h
 rename arch/arm/{plat-omap/include/plat/omap24xx.h => mach-omap2/omap24xx.h} (98%)
 rename arch/arm/{plat-omap/include/plat/omap34xx.h => mach-omap2/omap34xx.h} (98%)
 rename arch/arm/{plat-omap/include/plat/omap44xx.h => mach-omap2/omap44xx.h} (98%)
 rename arch/arm/{plat-omap/include/plat/omap54xx.h => mach-omap2/omap54xx.h} (100%)
 create mode 100644 arch/arm/mach-omap2/soc.h
 rename arch/arm/{plat-omap/include/plat/ti81xx.h => mach-omap2/ti81xx.h} (100%)
 delete mode 100644 arch/arm/plat-omap/devices.c
 delete mode 100644 arch/arm/plat-omap/include/plat/gpio-switch.h
 delete mode 100644 arch/arm/plat-omap/include/plat/hardware.h

-- 
Signature

[PATCH 0/4] Series short description

[Bernd Schubert]

With the ext3/ext4 directory index implementation hashes are used to specify
offsets for llseek(). For compatibility with NFSv2 and 32-bit user space
on 64-bit systems (kernel space) ext3/ext4 currently only return 32-bit 
hashes and therefore the probability of hash collisions for larger directories
is rather high. As recently reported on the NFS mailing list that theoretical
problem also happens on real systems:
http://comments.gmane.org/gmane.linux.nfs/40863

The following series adds two new f_mode flags to tell ext4
to use 32-bit or 64-bit hash values for llseek() calls.
These flags can then used by network file systems, such as NFS, to
request 32-bit or 64-bit offsets (hashes).

Version 2:
- use f_mode instead of O_* flags and also in a separate patch
- introduce EXT4_HTREE_EOF_32BIT and EXT4_HTREE_EOF_64BIT
- fix SEEK_END in ext4_dir_llseek()
- set f_mode flags in NFS code as early as possible and introduce a new
  NFSD_MAY_64BIT_COOKIE flag for that

--
Bernd Schubert
Fraunhofer ITWM

[PATCH 0/4] Series short description

[Alexey Zaytsev]

Hi.

So, it's time for the second version of the the fanotify
range patches.

The patch adds modification ranges to fsnotify events. Fanotify
is made to pass the range to the users.

This is useful for backup programs that work on huge files,
so that only a part of a modified file needs to be scanned
for changes.

changed. This is particularly useful for backup utilities that
work on huge files, so that only a part of the modified file
needs to be scanned for changes.

The series is split into 4 patches. The first one breaks the
ABI (but not API), and should get in before the .37 release,
or not at all. The last patch extends the ABI to handle event-
specific data ('options') in a backwards-compatible way.

You can also get the patchies from

git://git.zaytsev.su/git/linux-2.6.git branch fsnotify

A modified fanotify-example is available from

git://git.zaytsev.su/git/fanotify-example.git branch range

Changes since the first RFC:

1) Reworked the user interface, see the last patch.

2) Handle event merges properly.

3) Pass the range to fsnotify_parent().

3) Various small cleanups and fixes.

---

Alexey Zaytsev (4):
      fanotify: Shrink struct fanotify_event_metadata by 32 bits
      VFS: Tell fsnotify what part of the file might have changed
      fsnotify: Handle the file change ranges
      fanotify: Expose the file changes to the user


 fs/compat.c                        |    2 -
 fs/nfsd/vfs.c                      |    2 -
 fs/notify/fanotify/fanotify.c      |   19 +++++
 fs/notify/fanotify/fanotify_user.c |  132 +++++++++++++++++++++++++++++++-----
 fs/notify/fsnotify.c               |   24 ++++---
 fs/notify/inode_mark.c             |    2 -
 fs/notify/inotify/inotify_user.c   |    2 -
 fs/notify/notification.c           |   18 ++++-
 fs/open.c                          |    4 +
 fs/read_write.c                    |    4 +
 include/linux/fanotify.h           |   88 +++++++++++++++++++++---
 include/linux/fs.h                 |   14 ++++
 include/linux/fsnotify.h           |   68 +++++++++++--------
 include/linux/fsnotify_backend.h   |   31 +++++++-
 14 files changed, 330 insertions(+), 80 deletions(-)

--

[PATCH 0/4] Series short description

[John Bonesio]

The following series implements various changes for merging and modifiying
device trees.

The last one is a first stab at making labels from aliases. This way we can
have a consistent syntax for paths from alias and label references:
	&{alias/path/from/alias}
	&{label/path/from/label}

There might be a better approach to implementing this. Once of the constraints
I ended up following was to provide line information when there is an attempt
to re-use a label from a deleted or modified node. This constraint (or
requirement) might not be viewed as important like I think. Removing this might
make the changes easier to implemnt but may make it harder for dts writers to
discover the source of errors.

This last patch doesn't include a test case, though I have tested basic
functionality.

---

John Bonesio (4):
      Allow nodes to be refrenced by path at the top level.
      Implements a new feature for deleting existing device tree nodes.
      Implements a new feature for deleting existing properties in device tree nodes.
      Allow nodes at the root to be specified by path as well as by label.


 Makefile.dtc                    |    1 
 checks.c                        |   20 +++++++
 dtc-lexer.l                     |   38 ++++++++++++-
 dtc-parser.y                    |   86 +++++++++++++++++++++++++++-
 dtc.h                           |   10 +++
 flattree.c                      |    3 +
 livetree.c                      |  118 +++++++++++++++++++++++++++++++++++++--
 tests/dtc-checkfails.sh         |    2 -
 tests/run_tests.sh              |    6 ++
 tests/test_tree1_merge_path.dts |   41 ++++++++++++++
 util.h                          |   10 +++
 11 files changed, 324 insertions(+), 11 deletions(-)
 create mode 100644 tests/test_tree1_merge_path.dts

-- 
Signature

[PATCH 0/4] Series short description

[Benjamin Zores]

The following series implements support for Marvell 88F6282 SoC.
It should take into account all previously made comments.

---

Benjamin Zores (4):
      arch/arm/mach-kirkwood: update MPP definition.
      arch/arm/mach-kirkwood: add support for 88F6282-A0 revision
      arch/arm/mach-kirkwood: add support for 2nd PCIe port on 88f6282.
      arch/arm/mach-kirkwood: add support for 2nd TWSI controller on 88f6282.


 arch/arm/mach-kirkwood/addr-map.c                 |   21 +
 arch/arm/mach-kirkwood/common.c                   |   90 +++
 arch/arm/mach-kirkwood/common.h                   |   10 
 arch/arm/mach-kirkwood/db88f6281-bp-setup.c       |    2 
 arch/arm/mach-kirkwood/include/mach/bridge-regs.h |    3 
 arch/arm/mach-kirkwood/include/mach/irqs.h        |    4 
 arch/arm/mach-kirkwood/include/mach/kirkwood.h    |   43 +-
 arch/arm/mach-kirkwood/mpp.c                      |    2 
 arch/arm/mach-kirkwood/mpp.h                      |  595 +++++++++++----------
 arch/arm/mach-kirkwood/mv88f6281gtw_ge-setup.c    |    2 
 arch/arm/mach-kirkwood/netspace_v2-setup.c        |    6 
 arch/arm/mach-kirkwood/netxbig_v2-setup.c         |   34 +
 arch/arm/mach-kirkwood/openrd-setup.c             |    4 
 arch/arm/mach-kirkwood/pcie.c                     |  204 +++++--
 arch/arm/mach-kirkwood/rd88f6192-nas-setup.c      |    2 
 arch/arm/mach-kirkwood/rd88f6281-setup.c          |    2 
 arch/arm/mach-kirkwood/ts219-setup.c              |    8 
 arch/arm/mach-kirkwood/ts41x-setup.c              |   32 +
 18 files changed, 663 insertions(+), 401 deletions(-)

-- 
Signature

[PATCH 0/4] Series short description

[Alan Cox]

The following series implements...

---

Alan Cox (4):
      pata: Update experimental tags
      cmd64x: implement serialization as per notes
      pata_sis: Implement MWDMA for the UDMA 133 capable chips
      pata_via: Blacklist some combinations of Transcend Flash and via


 drivers/ata/Kconfig       |    8 +--
 drivers/ata/pata_cmd64x.c |  132 ++++++++++++++++++++++++++++++++++++++++++---
 drivers/ata/pata_sis.c    |   88 +++++++++++++++++++++++-------
 drivers/ata/pata_via.c    |   27 +++++++++
 4 files changed, 222 insertions(+), 33 deletions(-)

-- 
Signature

[PATCH 0/4] Series short description

[Alan Cox]

A quick batch of PIIX updates
---

Alan Cox (4):
      ata_piix: Remove stale comment
      ata_piix: Turn on hotplugging support for older chips
      ata_piix: The Sony TZ90 needs the cable type hardcoding
      ata_piix: ICH7 does not support correct MWDMA timings


 drivers/ata/ata_piix.c |   28 +++++++++++++++++++---------
 1 files changed, 19 insertions(+), 9 deletions(-)

Re: [PATCH 0/4] Series short description

[Jeff Garzik]

Alan Cox wrote:
> A quick batch of PIIX updates
> ---
> 
> Alan Cox (4):
>       ata_piix: Remove stale comment
>       ata_piix: Turn on hotplugging support for older chips
>       ata_piix: The Sony TZ90 needs the cable type hardcoding
>       ata_piix: ICH7 does not support correct MWDMA timings

I guess 1 & 2 are #upstream-fixes material, and 3 & 4 are #upstream ?

	Jeff

Re: [PATCH 0/4] Series short description

[Alan Cox]

On Mon, 11 May 2009 13:21:43 -0400
Jeff Garzik <jeff@garzik.org> wrote:

> Alan Cox wrote:
> > A quick batch of PIIX updates
> > ---
> > 
> > Alan Cox (4):
> >       ata_piix: Remove stale comment
> >       ata_piix: Turn on hotplugging support for older chips
> >       ata_piix: The Sony TZ90 needs the cable type hardcoding
> >       ata_piix: ICH7 does not support correct MWDMA timings
> 
> I guess 1 & 2 are #upstream-fixes material, and 3 & 4 are #upstream ?

I'd say they are all fixes. In terms of urgency #4 hasn't been reported
by anyone it was found when checking errata, #3 is a real problem
encountered by users #2 affects only fairly obscure hardware and isn't a
regression, #1 is a comment.

I've also now confirmed with the Intel hardware folks that there isn't
any reason we can't turn on parallel probing in ata_piix.

Alan

Re: [PATCH 0/4] Series short description

[Jeff Garzik]

Alan Cox wrote:
> On Mon, 11 May 2009 13:21:43 -0400
> Jeff Garzik <jeff@garzik.org> wrote:
> 
>> Alan Cox wrote:
>>> A quick batch of PIIX updates
>>> ---
>>>
>>> Alan Cox (4):
>>>       ata_piix: Remove stale comment
>>>       ata_piix: Turn on hotplugging support for older chips
>>>       ata_piix: The Sony TZ90 needs the cable type hardcoding
>>>       ata_piix: ICH7 does not support correct MWDMA timings
>> I guess 1 & 2 are #upstream-fixes material, and 3 & 4 are #upstream ?
> 
> I'd say they are all fixes. In terms of urgency #4 hasn't been reported
> by anyone it was found when checking errata, #3 is a real problem
> encountered by users #2 affects only fairly obscure hardware and isn't a
> regression, #1 is a comment.

I was referring to the patch numbering provided by you, in your 
submission, which is the reverse of the order listed above:

patch #1: ICH7 MWDMA timings
patch #2: TZ90
patch #3: turn on hotplugging
patch #4: fix comment

patch #3 is a behavior change IMO very late in -rc cycle, and patch #4 
is not urgent.

	Jeff

Re: [PATCH 0/4] Series short description

[Alan Cox]

> I was referring to the patch numbering provided by you, in your 
> submission, which is the reverse of the order listed above:
> 
> patch #1: ICH7 MWDMA timings
> patch #2: TZ90
> patch #3: turn on hotplugging
> patch #4: fix comment
> 
> patch #3 is a behavior change IMO very late in -rc cycle, and patch #4 
> is not urgent.

Agreed - sorry I'd forgotten that stgit is quite creative with its
ordering.

[PATCH 0/4] Series short description

[Eric Paris]

The following series implements...

---

Eric Paris (4):
      AUDIT: emit new record type showing all capset information
      AUDIT: audit when fcaps increase the permitted or inheritable capabilities
      AUDIT: output permitted and inheritable fcaps in PATH records
      CAPABILITIES: add cpu endian vfs caps structure


 include/linux/audit.h      |   22 +++++
 include/linux/capability.h |   12 +++
 kernel/auditsc.c           |  201 +++++++++++++++++++++++++++++++++++++++++++-
 kernel/capability.c        |    3 +
 security/commoncap.c       |  131 ++++++++++++++++-------------
 5 files changed, 308 insertions(+), 61 deletions(-)

-- 
Signature

[PATCH 0/4] Series short description

[Eric Paris]

The following series implements...

---

Eric Paris (4):
      AUDIT: emit new record type showing all capset information
      AUDIT: audit when fcaps increase the permitted or inheritable capabilities
      AUDIT: output permitted and inheritable fcaps in PATH records
      CAPABILITIES: add cpu endian vfs caps structure


 include/linux/audit.h      |   22 +++++
 include/linux/capability.h |   12 +++
 kernel/auditsc.c           |  201 +++++++++++++++++++++++++++++++++++++++++++-
 kernel/capability.c        |    3 +
 security/commoncap.c       |  131 ++++++++++++++++-------------
 5 files changed, 308 insertions(+), 61 deletions(-)

-- 
Signature

[PATCH 0/4] Series short description

[grant.likely]

The following series makes the mpc8349emitx* cuImage work 'out of the box'
Major changes are adding the linux,network-index property to the Ethernet
nodes and adding a default linux,stdout-path to the .dts files.  With
these changes, a cuImage created by 'make mpc834x_itx_defconfig; make zImage'
should boot without changes.

Cheers,
g.

--
Grant Likely, B.Sc. P.Eng.
Secret Lab Technologies Ltd.

[PATCH 0/4] Series short description

[Yann Dirson]

The following adds a --patch flag to "stg refresh".  I mainly needed it to
edit the log message in patches down the stack, but with some work it could
be used to record changes in another file and come back.

That would require to pop patches with --keep, and if we ask to only record
a subset of the changes, we would need to implement "push --keep" first.

This patch is not perfect, since it reverses a list twice, but I did not
find an elegant way to make it better.  Python gurus, feel free to improve
this :)

While I was at it, I noticed that "stg goto" need not revert the applied list
if we're going to an unapplied patch (sorta compensate my extraneous revert above ;),
and I noticed a DWIM behaviour on the (should-be)-erroneous "refresh --ack --sign".

-- 
Yann Dirson    <ydirson@altern.org> |
Debian-related: <dirson@debian.org> |   Support Debian GNU/Linux:
                                    |  Freedom, Power, Stability, Gratis

Re: [PATCH 0/4] Series short description

[Catalin Marinas]

On 23/11/06, Yann Dirson <ydirson@altern.org> wrote:
> The following adds a --patch flag to "stg refresh".  I mainly needed it to
> edit the log message in patches down the stack, but with some work it could
> be used to record changes in another file and come back.
>
> That would require to pop patches with --keep,

pop_patches(keep=True) allows this (I already added it to your patch).
This option actually applies the reverse of the patch between the
current index and the bottom of the last patch to be popped. If it
fails, it doesn't affect the current tree and reports an error.

> and if we ask to only record
> a subset of the changes, we would need to implement "push --keep" first.

The "push --keep" is a bit more complicated to implement since it
would usually have to generate commits (pop doesn't need to) since
usually the base of the pushed patches might have changed (especially
with the refresh --patch). It is even more complicated if the changes
you want to preserve affect a file modified by the pushed patches.

A solution I see is to generate a temporary diff, push the patches you
want and fold this temporary diff (the diff can be temporarily written
to a local file so that you don't lose them in case of an error or
Ctrl-C). If the push fails (conflicts) or the diff no longer applies,
just undo the push and re-apply the diff. There is no way to simply
check whether all the patches apply (git-apply --check) since you
can't generate a single diff for all the pushed patches (which might
not be in the committed order).

--