* [PATCH 0/2] Add capability to append to property
@ 2024-08-27 17:54 Ayush Singh
2024-08-27 17:54 ` [PATCH 1/2] dtc: Add /append-property/ Ayush Singh
2024-08-27 17:54 ` [PATCH 2/2] tests: Add test for append-property Ayush Singh
0 siblings, 2 replies; 7+ messages in thread
From: Ayush Singh @ 2024-08-27 17:54 UTC (permalink / raw)
To: lorforlinux, jkridner, robertcnelson, nenad.marinkovic,
Andrew Davis, Geert Uytterhoeven, Robert Nelson
Cc: devicetree-compiler, Ayush Singh
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.
Open items
- Better tests: I only started the work today, so well, I still don't
have a good picture of how to properly write tests.
- Appending to non-existent property: Currently am just adding a new
property if not found. Not sure if this is the desired behaviour.
- Incompatible values: Not sure how to handle when trying to append
incompatible values.
[3] https://lore.kernel.org/linux-arm-kernel/20240702164403.29067-1-afd@ti.com/
[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@beagleboard.org>
---
Ayush Singh (2):
dtc: Add /append-property/
tests: Add test for append-property
dtc-lexer.l | 7 +++++++
dtc-parser.y | 6 ++++++
dtc.h | 3 +++
livetree.c | 30 +++++++++++++++++++++++++-----
tests/run_tests.sh | 3 +++
tests/test_tree1_append.dts | 21 +++++++++++++++++++++
6 files changed, 65 insertions(+), 5 deletions(-)
---
base-commit: 99031e3a4a6e479466ae795790b44727434ca27d
change-id: 20240827-append-f614fe3261f9
Best regards,
--
Ayush Singh <ayush@beagleboard.org>
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH 1/2] dtc: Add /append-property/ 2024-08-27 17:54 [PATCH 0/2] Add capability to append to property Ayush Singh @ 2024-08-27 17:54 ` Ayush Singh 2024-08-29 14:04 ` Simon Glass 2024-08-27 17:54 ` [PATCH 2/2] tests: Add test for append-property Ayush Singh 1 sibling, 1 reply; 7+ messages in thread From: Ayush Singh @ 2024-08-27 17:54 UTC (permalink / raw) To: lorforlinux, jkridner, robertcnelson, nenad.marinkovic, Andrew Davis, Geert Uytterhoeven, Robert Nelson Cc: devicetree-compiler, Ayush Singh 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@ti.com/ [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@beagleboard.org> --- dtc-lexer.l | 7 +++++++ dtc-parser.y | 6 ++++++ dtc.h | 3 +++ livetree.c | 30 +++++++++++++++++++++++++----- 4 files changed, 41 insertions(+), 5 deletions(-) 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; 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; + } + + old_prop->deleted = false; + old_prop->append = false; free(old_prop->srcpos); old_prop->srcpos = new_prop->srcpos; free(new_prop); -- 2.46.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] dtc: Add /append-property/ 2024-08-27 17:54 ` [PATCH 1/2] dtc: Add /append-property/ Ayush Singh @ 2024-08-29 14:04 ` Simon Glass 0 siblings, 0 replies; 7+ messages in thread From: Simon Glass @ 2024-08-29 14:04 UTC (permalink / raw) To: Ayush Singh Cc: lorforlinux, jkridner, robertcnelson, nenad.marinkovic, Andrew Davis, Geert Uytterhoeven, Robert Nelson, devicetree-compiler Hi Ayush, On Tue, 27 Aug 2024 at 11:55, Ayush Singh <ayush@beagleboard.org> 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@ti.com/ > [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@beagleboard.org> > --- > 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@chromium.org> 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2] tests: Add test for append-property 2024-08-27 17:54 [PATCH 0/2] Add capability to append to property Ayush Singh 2024-08-27 17:54 ` [PATCH 1/2] dtc: Add /append-property/ Ayush Singh @ 2024-08-27 17:54 ` Ayush Singh 2024-08-29 14:04 ` Simon Glass 1 sibling, 1 reply; 7+ messages in thread From: Ayush Singh @ 2024-08-27 17:54 UTC (permalink / raw) To: lorforlinux, jkridner, robertcnelson, nenad.marinkovic, Andrew Davis, Geert Uytterhoeven, Robert Nelson Cc: devicetree-compiler, Ayush Singh Simple test to build a device tree with /append-property/. Signed-off-by: Ayush Singh <ayush@beagleboard.org> --- tests/run_tests.sh | 3 +++ tests/test_tree1_append.dts | 21 +++++++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/tests/run_tests.sh b/tests/run_tests.sh index 937b128..ac0dc05 100755 --- a/tests/run_tests.sh +++ b/tests/run_tests.sh @@ -708,6 +708,9 @@ dtc_tests () { run_dtc_test -I dts -O dtb -o dtc_tree1_delete.test.dtb "$SRCDIR/test_tree1_delete.dts" tree1_tests dtc_tree1_delete.test.dtb + # Check prop append functionality + run_dtc_test -I dts -O dtb -o dtc_tree1_append.test.dtb "$SRCDIR/test_tree1_append.dts" + # Check omit-if-no-ref functionality run_dtc_test -I dts -O dtb -o omit-no-ref.test.dtb "$SRCDIR/omit-no-ref.dts" run_test check_path omit-no-ref.test.dtb not-exists "/node1" diff --git a/tests/test_tree1_append.dts b/tests/test_tree1_append.dts new file mode 100644 index 0000000..953095d --- /dev/null +++ b/tests/test_tree1_append.dts @@ -0,0 +1,21 @@ +/dts-v1/; + +/include/ "test_tree1.dts" + +/ { + nonexistant-property = <0x00000000>; + + subnode@1 { + sub-prop = <0>; + str-prop = "0"; + }; +}; + +/ { + /append-property/ nonexistant-property = <0x00000001 0x00000002>; + + subnode@1 { + /append-property/ sub-prop = <1 2>; + /append-property/ str-prop = "1", "2"; + }; +}; -- 2.46.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] tests: Add test for append-property 2024-08-27 17:54 ` [PATCH 2/2] tests: Add test for append-property Ayush Singh @ 2024-08-29 14:04 ` Simon Glass 2024-08-29 14:17 ` Ayush Singh 0 siblings, 1 reply; 7+ messages in thread From: Simon Glass @ 2024-08-29 14:04 UTC (permalink / raw) To: Ayush Singh Cc: lorforlinux, jkridner, robertcnelson, nenad.marinkovic, Andrew Davis, Geert Uytterhoeven, Robert Nelson, devicetree-compiler Hi Ayush, On Tue, 27 Aug 2024 at 11:55, Ayush Singh <ayush@beagleboard.org> wrote: > > Simple test to build a device tree with /append-property/. > > Signed-off-by: Ayush Singh <ayush@beagleboard.org> > --- > tests/run_tests.sh | 3 +++ > tests/test_tree1_append.dts | 21 +++++++++++++++++++++ > 2 files changed, 24 insertions(+) > > diff --git a/tests/run_tests.sh b/tests/run_tests.sh > index 937b128..ac0dc05 100755 > --- a/tests/run_tests.sh > +++ b/tests/run_tests.sh > @@ -708,6 +708,9 @@ dtc_tests () { > run_dtc_test -I dts -O dtb -o dtc_tree1_delete.test.dtb "$SRCDIR/test_tree1_delete.dts" > tree1_tests dtc_tree1_delete.test.dtb > > + # Check prop append functionality > + run_dtc_test -I dts -O dtb -o dtc_tree1_append.test.dtb "$SRCDIR/test_tree1_append.dts" > + How does this check that something actually happened? From your cover letter, it seems that you might send a v2, but I'm just checking? > # Check omit-if-no-ref functionality > run_dtc_test -I dts -O dtb -o omit-no-ref.test.dtb "$SRCDIR/omit-no-ref.dts" > run_test check_path omit-no-ref.test.dtb not-exists "/node1" > diff --git a/tests/test_tree1_append.dts b/tests/test_tree1_append.dts > new file mode 100644 > index 0000000..953095d > --- /dev/null > +++ b/tests/test_tree1_append.dts > @@ -0,0 +1,21 @@ > +/dts-v1/; > + > +/include/ "test_tree1.dts" > + > +/ { > + nonexistant-property = <0x00000000>; > + > + subnode@1 { > + sub-prop = <0>; > + str-prop = "0"; > + }; > +}; > + > +/ { > + /append-property/ nonexistant-property = <0x00000001 0x00000002>; > + > + subnode@1 { > + /append-property/ sub-prop = <1 2>; > + /append-property/ str-prop = "1", "2"; > + }; > +}; > > -- > 2.46.0 > > Regards, Simon ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] tests: Add test for append-property 2024-08-29 14:04 ` Simon Glass @ 2024-08-29 14:17 ` Ayush Singh 2024-08-29 15:00 ` Simon Glass 0 siblings, 1 reply; 7+ messages in thread From: Ayush Singh @ 2024-08-29 14:17 UTC (permalink / raw) To: Simon Glass Cc: lorforlinux, jkridner, robertcnelson, nenad.marinkovic, Andrew Davis, Geert Uytterhoeven, Robert Nelson, devicetree-compiler On 8/29/24 19:34, Simon Glass wrote: > Hi Ayush, > > On Tue, 27 Aug 2024 at 11:55, Ayush Singh <ayush@beagleboard.org> wrote: >> Simple test to build a device tree with /append-property/. >> >> Signed-off-by: Ayush Singh <ayush@beagleboard.org> >> --- >> tests/run_tests.sh | 3 +++ >> tests/test_tree1_append.dts | 21 +++++++++++++++++++++ >> 2 files changed, 24 insertions(+) >> >> diff --git a/tests/run_tests.sh b/tests/run_tests.sh >> index 937b128..ac0dc05 100755 >> --- a/tests/run_tests.sh >> +++ b/tests/run_tests.sh >> @@ -708,6 +708,9 @@ dtc_tests () { >> run_dtc_test -I dts -O dtb -o dtc_tree1_delete.test.dtb "$SRCDIR/test_tree1_delete.dts" >> tree1_tests dtc_tree1_delete.test.dtb >> >> + # Check prop append functionality >> + run_dtc_test -I dts -O dtb -o dtc_tree1_append.test.dtb "$SRCDIR/test_tree1_append.dts" >> + > How does this check that something actually happened? From your cover > letter, it seems that you might send a v2, but I'm just checking? Yeah, it tries to build it and that's all. I am not sure if `run_dtc_test` fails if the command fails. I can see there are helpers like `check_path` but is there any helper to check the value of a property? If not, I can try to write one. Ayush Singh ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] tests: Add test for append-property 2024-08-29 14:17 ` Ayush Singh @ 2024-08-29 15:00 ` Simon Glass 0 siblings, 0 replies; 7+ messages in thread From: Simon Glass @ 2024-08-29 15:00 UTC (permalink / raw) To: Ayush Singh Cc: lorforlinux, jkridner, robertcnelson, nenad.marinkovic, Andrew Davis, Geert Uytterhoeven, Robert Nelson, devicetree-compiler Hi Ayush, On Thu, 29 Aug 2024 at 08:17, Ayush Singh <ayush@beagleboard.org> wrote: > > On 8/29/24 19:34, Simon Glass wrote: > > > Hi Ayush, > > > > On Tue, 27 Aug 2024 at 11:55, Ayush Singh <ayush@beagleboard.org> wrote: > >> Simple test to build a device tree with /append-property/. > >> > >> Signed-off-by: Ayush Singh <ayush@beagleboard.org> > >> --- > >> tests/run_tests.sh | 3 +++ > >> tests/test_tree1_append.dts | 21 +++++++++++++++++++++ > >> 2 files changed, 24 insertions(+) > >> > >> diff --git a/tests/run_tests.sh b/tests/run_tests.sh > >> index 937b128..ac0dc05 100755 > >> --- a/tests/run_tests.sh > >> +++ b/tests/run_tests.sh > >> @@ -708,6 +708,9 @@ dtc_tests () { > >> run_dtc_test -I dts -O dtb -o dtc_tree1_delete.test.dtb "$SRCDIR/test_tree1_delete.dts" > >> tree1_tests dtc_tree1_delete.test.dtb > >> > >> + # Check prop append functionality > >> + run_dtc_test -I dts -O dtb -o dtc_tree1_append.test.dtb "$SRCDIR/test_tree1_append.dts" > >> + > > How does this check that something actually happened? From your cover > > letter, it seems that you might send a v2, but I'm just checking? > > Yeah, it tries to build it and that's all. I am not sure if > `run_dtc_test` fails if the command fails. > > > I can see there are helpers like `check_path` but is there any helper to > check the value of a property? If not, I can try to write one. The fdtget tests do some of that. Regards, Simon ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2024-08-29 15:00 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-08-27 17:54 [PATCH 0/2] Add capability to append to property Ayush Singh 2024-08-27 17:54 ` [PATCH 1/2] dtc: Add /append-property/ Ayush Singh 2024-08-29 14:04 ` Simon Glass 2024-08-27 17:54 ` [PATCH 2/2] tests: Add test for append-property Ayush Singh 2024-08-29 14:04 ` Simon Glass 2024-08-29 14:17 ` Ayush Singh 2024-08-29 15:00 ` Simon Glass
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).