Hi Ayush, On Tue, 27 Aug 2024 at 11:55, Ayush Singh <ayush@xxxxxxxxxxxxxxx> wrote: > > Allow appending values to a property instead of overriding the previous > values of property. > > Currently, we have /delete-node/ and /delete-property/, but lack > /append-property/. Hence we end up having to repeat all existing values > when appending to a property (e.g. see [1] appending to clocks from > [2]). > > This functionality is also important for creating a device tree based > implementation to support different types of addon-boards such as > mikroBUS, Grove [3], etc. > > [3] https://lore.kernel.org/linux-arm-kernel/20240702164403.29067-1-afd@xxxxxx/ > [2] https://elixir.bootlin.com/linux/latest/source/arch/arm64/boot/dts/renesas/r8a77951-salvator-xs.dts#L39 > [1] https://elixir.bootlin.com/linux/latest/source/arch/arm64/boot/dts/renesas/r8a77951.dtsi#L3334 > > Signed-off-by: Ayush Singh <ayush@xxxxxxxxxxxxxxx> > --- > dtc-lexer.l | 7 +++++++ > dtc-parser.y | 6 ++++++ > dtc.h | 3 +++ > livetree.c | 30 +++++++++++++++++++++++++----- > 4 files changed, 41 insertions(+), 5 deletions(-) Reviewed-by: Simon Glass <sjg@xxxxxxxxxxxx> nits below > > diff --git a/dtc-lexer.l b/dtc-lexer.l > index de60a70..5da4ca5 100644 > --- a/dtc-lexer.l > +++ b/dtc-lexer.l > @@ -137,6 +137,13 @@ static void PRINTF(1, 2) lexical_error(const char *fmt, ...); > return DT_DEL_NODE; > } > > +<*>"/append-property/" { > + DPRINT("Keyword: /append-property/\n"); > + DPRINT("<PROPNODENAME>\n"); > + BEGIN(PROPNODENAME); > + return DT_APP_PROP; > + } > + > <*>"/omit-if-no-ref/" { > DPRINT("Keyword: /omit-if-no-ref/\n"); > DPRINT("<PROPNODENAME>\n"); > diff --git a/dtc-parser.y b/dtc-parser.y > index 4d5eece..94ce405 100644 > --- a/dtc-parser.y > +++ b/dtc-parser.y > @@ -58,6 +58,7 @@ static bool is_ref_relative(const char *ref) > %token DT_BITS > %token DT_DEL_PROP > %token DT_DEL_NODE > +%token DT_APP_PROP > %token DT_OMIT_NO_REF > %token <propnodename> DT_PROPNODENAME > %token <integer> DT_LITERAL > @@ -296,6 +297,11 @@ propdef: > $$ = build_property_delete($2); > free($2); > } > + | DT_APP_PROP DT_PROPNODENAME '=' propdata ';' > + { > + $$ = build_property_append($2, $4, &@$); > + free($2); > + } > | DT_LABEL propdef > { > add_label(&$2->labels, $1); > diff --git a/dtc.h b/dtc.h > index 4c4aaca..06771c5 100644 > --- a/dtc.h > +++ b/dtc.h > @@ -205,6 +205,7 @@ struct bus_type { > > struct property { > bool deleted; > + bool append; Can you add a comment as to what this means? > char *name; > struct data val; > > @@ -263,6 +264,8 @@ void delete_labels(struct label **labels); > struct property *build_property(const char *name, struct data val, > struct srcpos *srcpos); > struct property *build_property_delete(const char *name); > +struct property *build_property_append(const char *name, struct data val, > + struct srcpos *srcpos); > struct property *chain_property(struct property *first, struct property *list); > struct property *reverse_properties(struct property *first); > > diff --git a/livetree.c b/livetree.c > index 49f7230..74eca1b 100644 > --- a/livetree.c > +++ b/livetree.c > @@ -62,6 +62,20 @@ struct property *build_property_delete(const char *name) > return new; > } > > +struct property *build_property_append(const char *name, struct data val, > + struct srcpos *srcpos) > +{ > + struct property *new = xmalloc(sizeof(*new)); > + > + memset(new, 0, sizeof(*new)); > + new->name = xstrdup(name); > + new->append = true; > + new->val = val; > + new->srcpos = srcpos_copy(srcpos); > + > + return new; > +} > + > struct property *chain_property(struct property *first, struct property *list) > { > assert(first->next == NULL); > @@ -151,8 +165,8 @@ struct node *merge_nodes(struct node *old_node, struct node *new_node) > for_each_label_withdel(new_node->labels, l) > add_label(&old_node->labels, l->label); > > - /* Move properties from the new node to the old node. If there > - * is a collision, replace the old value with the new */ > + /* Move properties from the new node to the old node. If there > + * is a collision, replace/append the old value with the new */ > while (new_node->proplist) { > /* Pop the property off the list */ > new_prop = new_node->proplist; > @@ -165,15 +179,21 @@ struct node *merge_nodes(struct node *old_node, struct node *new_node) > continue; > } > > - /* Look for a collision, set new value if there is */ > + /* Look for a collision, set new value/append if there is */ > for_each_property_withdel(old_node, old_prop) { > if (streq(old_prop->name, new_prop->name)) { > /* Add new labels to old property */ > for_each_label_withdel(new_prop->labels, l) > add_label(&old_prop->labels, l->label); > > - old_prop->val = new_prop->val; > - old_prop->deleted = 0; > + if (new_prop->append) { > + old_prop->val = data_merge(old_prop->val, new_prop->val); > + } else { > + old_prop->val = new_prop->val; > + } Braces are not needed here. > + > + old_prop->deleted = false; > + old_prop->append = false; > free(old_prop->srcpos); > old_prop->srcpos = new_prop->srcpos; > free(new_prop); > > -- > 2.46.0 > > Regards, Simon
