Series short description
cover Series short description
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 · 1 line
Signature
PATCH 1/4 Allow nodes to be refrenced by path at the top level.
When nodes are modified by merging device trees, nodes to be updated/merged can be specified by a label. Specifying nodes by full path (instead of label) doesn't quite work. This patch fixes that.
---
dtc-lexer.l | 2 +- dtc-parser.y | 5 ++--- tests/run_tests.sh | 2 ++ tests/test_tree1_merge_path.dts | 41 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 46 insertions(+), 4 deletions(-) create mode 100644 tests/test_tree1_merge_path.dts
dtc-lexer.l +1 -1
@@ -115,7 +115,7 @@ static int pop_input_file(void); return DT_REF; } -"&{/"{PATHCHAR}+\} { /* new-style path reference */ +<*>"&{/"{PATHCHAR}+\} { /* new-style path reference */ yytext[yyleng-1] = '\0'; DPRINT("Ref: %s\n", yytext+2); yylval.labelref = xstrdup(yytext+2);
dtc-parser.y +2 -3
@@ -131,13 +131,12 @@ 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; } ;
tests/run_tests.sh +2 -0
@@ -305,6 +305,8 @@ dtc_tests () { run_dtc_test -I dts -O dtb -o multilabel_merge.test.dtb multilabel_merge.dts run_test references multilabel.test.dtb run_test dtbs_equal_ordered multilabel.test.dtb multilabel_merge.test.dtb + run_dtc_test -I dts -O dtb -o dtc_tree1_merge_path.test.dtb test_tree1_merge_path.dts + tree1_tests dtc_tree1_merge_path.test.dtb test_tree1.dtb # Check some checks check_tests dup-nodename.dts duplicate_node_names
tests/test_tree1_merge_path.dts new file +41 -0
41 lines not on this page · raw message
Re: [PATCH 1/4] Allow nodes to be refrenced by path at the top level.
Quoted 5 lines · John Bonesio
When nodes are modified by merging device trees, nodes to be updated/merged can be specified by a label. Specifying nodes by full path (instead of label) doesn't quite work. This patch fixes that.
Signed-off-by John Bonesio <bones-s3s/[email protected]>
If we go where I'm beginning to suspect we want to with deprecating some existing syntax, this may become moot. However as long as we're using reference like syntax for the merging, we should accept all the references that are valid elsewhere.
Signature · 4 lines
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 1/4] Allow nodes to be refrenced by path at the top level.
Quoted 6 lines · John Bonesio
Quoted 1 line
When nodes are modified by merging device trees, nodes to be updated/merged
can
Quoted 3 lines
be specified by a label. Specifying nodes by full path (instead of label) doesn't quite work. This patch fixes that.
Quoted 8 lines
Quoted 1 line
Signed-off-by John Bonesio <bones-s3s/[email protected]>Acked-by David Gibson <[email protected]>If we go where I'm beginning to suspect we want to with deprecating some existing syntax, this may become moot. However as long as we're using reference like syntax for the merging, we should accept all the references that are valid elsewhere.
Are we read for just this patch, or the series?
I didn't see any more commentary about the remaining 3 patches.
Thanks,
jdl
A message that is not in the archive (sent off-list or not yet received): E1PFr8Q-0003CF-52-CYoMK+44s/[email protected]
Re: [PATCH 1/4] Allow nodes to be refrenced by path at the top level.
Quoted 10 lines · Jon Loeliger
Quoted 10 lines · John Bonesio
Quoted 1 line
When nodes are modified by merging device trees, nodes to be updated/merged
can
Quoted 4 lines
be specified by a label. Specifying nodes by full path (instead of label) doesn't quite work. This patch fixes that.
Signed-off-by John Bonesio <bones-s3s/[email protected]>Acked-by David Gibson <[email protected]>
Quoted 7 lines
Quoted 4 lines
If we go where I'm beginning to suspect we want to with deprecating some existing syntax, this may become moot. However as long as we're using reference like syntax for the merging, we should accept all the references that are valid elsewhere.
Are we read for just this patch, or the series?
I didn't see any more commentary about the remaining 3 patches.
Hold off on the remaining three. Also, this patch is based on the commits currently in my tree which have also been acked:
git://git.secretlab.ca/git/dtc.git master
g.
A message that is not in the archive (sent off-list or not yet received): AANLkTim3TopcXUcdJ1kGBwHYTNjL=SdhBaaZgnrWQG-a-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org
Re: [PATCH 1/4] Allow nodes to be refrenced by path at the top level.
Quoted 16 lines · Grant Likely
Quoted 15 lines · Jon Loeliger
Quoted 14 lines · John Bonesio
Quoted 1 line
When nodes are modified by merging device trees, nodes to be updated/merged
can
Quoted 4 lines
be specified by a label. Specifying nodes by full path (instead of label) doesn't quite work. This patch fixes that.
Signed-off-by John Bonesio <bones-s3s/[email protected]>Acked-by David Gibson <[email protected]>If we go where I'm beginning to suspect we want to with deprecating some existing syntax, this may become moot. However as long as we're using reference like syntax for the merging, we should accept all the references that are valid elsewhere.
Quoted 7 lines
Quoted 2 lines
Are we read for just this patch, or the series?
I didn't see any more commentary about the remaining 3 patches.Hold off on the remaining three. Also, this patch is based on the commits currently in my tree which have also been acked:
git://git.secretlab.ca/git/dtc.gitmaster
Yeah, what Grant said. I'm happy with patch 1/4, but the others need more discussion / work.
Signature · 4 lines
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 1/4] Allow nodes to be refrenced by path at the top level.
In reply to a message not in the archiveQuoted 21 lines · Jon Loeliger
Quoted 2 lines
Quoted 2 lines · John Bonesio
Quoted 1 line
When nodes are modified by merging device trees, nodes to be updated/m=
erged
Quoted 2 lines
Quoted 2 lines
=A0can
Quoted 1 line
be specified by a label. Specifying nodes by full path (instead of lab=
el)
Quoted 6 lines
Quoted 6 lines
Quoted 3 lines
doesn't quite work. This patch fixes that.
Signed-off-by John Bonesio <bones-s3s/[email protected]>Acked-by David Gibson <[email protected]>Quoted 7 lines
Quoted 4 lines
If we go where I'm beginning to suspect we want to with deprecating some existing syntax, this may become moot. =A0However as long as we're using reference like syntax for the merging, we should accept all the references that are valid elsewhere.
Are we read for just this patch, or the series?
I didn't see any more commentary about the remaining 3 patches.
Quoted 6 lines
Hold off on the remaining three. Also, this patch is based on the commits currently in my tree which have also been acked:
git://git.secretlab.ca/git/dtc.gitmasterg.
Patch 1/4 in this series applied and pushed out
along with Patch 1/4 from the previous series:
Create new and use new print_error that uses printf style formatting.
Thanks,
jdl
PATCH 2/4 Implements a new feature for deleting existing device tree nodes.
In reply to John BonesioThis 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.
---
Makefile.dtc | 1 + checks.c | 20 ++++++++++++++++++++ dtc-lexer.l | 6 ++++++ dtc-parser.y | 15 +++++++++++++++ dtc.h | 5 +++++ livetree.c | 39 +++++++++++++++++++++++++++++++++++++++ tests/run_tests.sh | 2 ++ util.h | 10 ++++++++++ 8 files changed, 98 insertions(+), 0 deletions(-)
Makefile.dtc +1 -0
@@ -12,6 +12,7 @@ DTC_SRCS = \ livetree.c \ srcpos.c \ treesource.c \ + strtbl.c \ util.c DTC_GEN_SRCS = dtc-lexer.lex.c dtc-parser.tab.c
checks.c +20 -0
@@ -19,6 +19,7 @@*/ #include "dtc.h" +#include "srcpos.h" #ifdef TRACE_CHECKS #define TRACE (c, ...) \
@@ -292,6 +293,25 @@ static void check_duplicate_label(struct check *c, struct node *dt,struct node *othernode =NULL ;struct property *otherprop =NULL ;struct marker *othermark =NULL ; +struct srcpos *pos; + + pos =strtbl_str_data (removed_labels, label); + +if (pos) { +/* Then the label did exist, but the node it's in was removed */ +if (pos->file) +FAIL (c,"Duplicate label '%s' on " DESCLABEL_FMT +" and removed node defined at line %d in file %s" , + label,DESCLABEL_ARGS (node, prop, mark), + pos->first_line, pos->file->name); +else +FAIL (c,"Duplicate label '%s' on " DESCLABEL_FMT +" and removed node defined at line %d" , + label,DESCLABEL_ARGS (node, prop, mark), + pos->first_line); + }else { +/* Check to see if the label is already in another node */ + } othernode =get_node_by_label (dt, label);
dtc-lexer.l +6 -0
@@ -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);
dtc-parser.y +15 -0
@@ -33,6 +33,8 @@ extern void yyerror(char const *s); extern struct boot_info *the_boot_info; extern int treesource_error; +strtbl label_table = NULL; + static unsigned long long eval_literal(const char *s, int base, int bits); %}
@@ -55,6 +57,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
@@ -108,6 +111,7 @@ memreserve: } | DT_LABEL memreserve { + strtbl_insert(&label_table, $1, srcpos_copy(&yylloc)); add_label(&$2->labels, $1); $$ = $2; }
@@ -139,6 +143,15 @@ devicetree: print_error("label or path, '%s', not found", $2); $$ = $1; } + | devicetree DT_REMOVENODE DT_REF ';' + { + struct node *target = get_node_by_ref($1, $3); + + if (target) + remove_child(target->parent, target); + else + print_error("label or path, '%s', not found", $3); + } ; nodedef:
@@ -170,6 +183,7 @@ propdef: } | DT_LABEL propdef { + strtbl_insert(&label_table, $1, srcpos_copy(&yylloc)); add_label(&$2->labels, $1); $$ = $2; }
@@ -305,6 +319,7 @@ subnode: } | DT_LABEL subnode { + strtbl_insert(&label_table, $1, srcpos_copy(&yylloc)); add_label(&$2->labels, $1); $$ = $2; }
dtc.h +5 -0
@@ -35,6 +35,7 @@#include <fdt.h> #include "util.h" +#include "strtbl.h" #ifdef DEBUG #define debug(fmt,args...) printf(fmt, ##args)
@@ -178,6 +179,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);
@@ -221,6 +223,9 @@ struct boot_info {struct boot_info *build_boot_info (struct reserve_info *reservelist,struct node *tree,uint32_t boot_cpuid_phys); +extern strtbl label_table; +extern strtbl removed_labels; +/* Checks */ void process_checks (int force,struct boot_info *bi);
livetree.c +39 -0
@@ -20,6 +20,8 @@#include "dtc.h" +strtbl removed_labels =NULL ; +/* * Tree building functions */
@@ -202,6 +204,43 @@ void add_child(struct node *parent, struct node *child) *p = child; } +void remove_child (struct node *parent,struct node *child) +{ +struct node **p; +struct property *prop; +struct label *l; +void *data; + +/* Make sure we've got a consistent tree here */ +assert (child->parent == parent); + +/* Keep track of removed node labels, maintaining the original +* data associated with the label collected by the parser */ +for_each_label (child->labels, l) { + data =strtbl_str_data (label_table, l->label); +strtbl_insert (&removed_labels, l->label, data); + } + +/* Keep track of removed property labels, maintaining the original +* data associated with the label collected by the parser */ +for_each_property (child, prop) { +for_each_label (prop->labels, l) { + data =strtbl_str_data (label_table, l->label); +strtbl_insert (&removed_labels, l->label, data); + } + } + + 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));
tests/run_tests.sh +2 -0
@@ -307,6 +307,8 @@ dtc_tests () { run_test dtbs_equal_ordered multilabel.test.dtb multilabel_merge.test.dtb run_dtc_test -I dts -O dtb -o dtc_tree1_merge_path.test.dtb test_tree1_merge_path.dts tree1_tests dtc_tree1_merge_path.test.dtb test_tree1.dtb + run_dtc_test -I dts -O dtb -o dtc_tree1_merge_remove.test.dtb test_tree1_merge_remove.dts + tree1_tests dtc_tree1_merge_remove.test.dtb test_tree1.dtb # Check some checks check_tests dup-nodename.dts duplicate_node_names
util.h +10 -0
@@ -40,6 +40,16 @@ static inline void *xmalloc(size_t len)return new; } +static inline void *xcalloc (size_t nmemb,size_t len) { +void *new =calloc (nmemb, len); + +if (!new) +die ("calloc() failed\n" ); + +return new; +} + +static inline void *xrealloc (void *p,size_t len) {void *new =realloc (p, len);
PATCH 3/4 Implements a new feature for deleting existing properties in device tree nodes.
In reply to John BonesioWhen updating existing nodes in a device tree merge operation, properties can be removed by using
/remove-prop/ <property>;
If a property has been removed, it is treated the same as if it had not been declared.
---
dtc-lexer.l | 6 ++++++ dtc-parser.y | 6 ++++++ dtc.h | 3 +++ flattree.c | 3 +++ livetree.c | 31 ++++++++++++++++++++++++++----- tests/dtc-checkfails.sh | 2 +- tests/run_tests.sh | 2 ++ 7 files changed, 47 insertions(+), 6 deletions(-)
dtc-lexer.l +6 -0
@@ -102,6 +102,12 @@ static int pop_input_file(void); return DT_REMOVENODE; } +<*>"/remove-prop/" { + DPRINT("Keyword: /undef-prop/\n"); + BEGIN(PROPNODENAME); + return DT_REMOVEPROP; + } + <*>{LABEL}: { DPRINT("Label: %s\n", yytext); yylval.labelref = xstrdup(yytext);
dtc-parser.y +6 -0
@@ -58,6 +58,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_REMOVEPROP %token <propnodename> DT_PROPNODENAME %token <literal> DT_LITERAL %token <cbase> DT_BASE
@@ -181,6 +182,11 @@ propdef: { $$ = build_property($1, empty_data); } + | DT_REMOVEPROP propdef + { + $2->undefined = 1; + $$ = $2; + } | DT_LABEL propdef { strtbl_insert(&label_table, $1, srcpos_copy(&yylloc));
dtc.h +3 -0
@@ -132,6 +132,8 @@ struct label { };struct property { +int undefined;/* when this feild is set to 1, the property is to be +treated as undefined */ char *name;struct data val;
@@ -178,6 +180,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);
flattree.c +3 -0
@@ -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 ;
livetree.c +26 -5
@@ -123,11 +123,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 ;
@@ -190,6 +194,23 @@ void add_property(struct node *node, struct property *prop) *p = prop; } +void remove_property (struct node *node,struct property *prop) +{ +struct property **p; + + p = &node->proplist; +while (*p) { +if (*p == prop) { + *p = (*p)->next; +return ; + } + p = &((*p)->next); + } +/* property not in the node? it's probably an error, so flag it. */ +assert (0 ); +} + +void add_child (struct node *parent,struct node *child) {struct node **p;
tests/dtc-checkfails.sh +1 -1
@@ -23,7 +23,7 @@ if [ "$ret" -gt 127 ]; then fi for c in $CHECKS; do - if ! grep -E "^(ERROR)|(Warning) \($c\):" $LOG > /dev/null; then + if ! grep -E "^(ERROR)|(Error)|(Warning) \($c\):" $LOG > /dev/null; then FAIL "Failed to trigger check \"$c\"" fi done
tests/run_tests.sh +2 -0
@@ -309,6 +309,8 @@ dtc_tests () { tree1_tests dtc_tree1_merge_path.test.dtb test_tree1.dtb run_dtc_test -I dts -O dtb -o dtc_tree1_merge_remove.test.dtb test_tree1_merge_remove.dts tree1_tests dtc_tree1_merge_remove.test.dtb test_tree1.dtb + run_dtc_test -I dts -O dtb -o dtc_tree1_merge_remove_prop.test.dtb test_tree1_merge_remove_prop.dts + tree1_tests dtc_tree1_merge_remove_prop.test.dtb test_tree1.dtb # Check some checks check_tests dup-nodename.dts duplicate_node_names
PATCH 4/4 Allow nodes at the root to be specified by path as well as by label.
In reply to John BonesioChanges 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.
---
dtc-lexer.l | 24 +++++++++++++++++++++++ dtc-parser.y | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ dtc.h | 2 ++ livetree.c | 48 +++++++++++++++++++++++++++++++++++++++++++++- 4 files changed, 133 insertions(+), 1 deletions(-)
dtc-lexer.l +24 -0
@@ -127,6 +127,23 @@ static int pop_input_file(void); 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 */ yytext[yyleng-1] = '\0'; DPRINT("Ref: %s\n", yytext+2);
@@ -146,6 +163,13 @@ static int pop_input_file(void); return ']'; } +<PROPNODENAME>"aliases" { + DPRINT("Aliases: %s\n", yytext); + yylval.propnodename = xstrdup(yytext); + BEGIN_DEFAULT(); + return DT_ALIASES; + } + <PROPNODENAME>{PROPNODECHAR}+ { DPRINT("PropNodeName: %s\n", yytext); yylval.propnodename = xstrdup(yytext);
dtc-parser.y +60 -0
111 lines not on this page · raw message
dtc.h +2 -0
@@ -179,6 +179,8 @@ struct node *name_node(struct node *node, char *name);struct node *chain_node (struct node *first,struct node *list);struct node *merge_nodes (struct node *old_node,struct node *new_node); +void make_aliases_labels (struct node *root,struct node *aliases); +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);
livetree.c +47 -1
55 lines not on this page · raw message