[PATCH 00/13] dtc: Merge the fragments of a dts file after parsing

[PATCH 00/13] dtc: Merge the fragments of a dts file after parsing

[Simon Glass]

The parser merges each top-level fragment of a dts file into the tree as
soon as it is parsed. This means that deletes cannot be dealt with
consistently, nor checks run on each fragment before it is merged. Here
a fragment is a top-level part of the source, such as / { ... }; or
&label { ... };, not an overlay's fragment@N node.

As a first step towards keeping the fragments apart until after parsing,
move the merge logic out of the grammar actions into livetree.c, one
action at a time, then have the parser return a list of fragments and
merge them from main(). The output is unchanged. The only changes to the
errors are the final message when a reference cannot be resolved, that a
syntax error later in the file now stops dtc before any earlier
reference errors are reported, and that reference errors now come after
any other errors from the parser.

Most of the merge errors have no tests, so the series starts by adding a
test for each one, checking the message and its position in the source.
Every line of the new merge code is then covered by the tests. At each
commit, make check passes and dtc gives the same output and messages as
before for all 4162 .dts and .dtso files in Linux
v7.3-12284-g3799eeb906c9

Following this, the plan is to apply deletes while merging, deciding
what a delete in a plugin should do, and then to run some checks on each
fragment.

Link: https://lore.kernel.org/devicetree-compiler/auK0ERvxGCJGSc3p@gractus.seuss/


Simon Glass (13):
  tests: Check each error when merging a dts file
  dtc: Add a list for the top-level fragments of a dts file
  dtc: Move the /delete-node/ handling out of the grammar
  dtc: Move the /omit-if-no-ref/ handling out of the grammar
  dtc: Move is_ref_relative() to livetree.c
  dtc: Move label-reference handling out of the grammar
  dtc: Move path-reference handling out of the grammar
  dtc: Move labelled-reference handling out of the grammar
  dtc: Move first-reference handling out of the grammar
  dtc: Add merge_fragment() to merge any fragment
  dtc: Add helpers for the errors when merging a fragment
  dtc: Merge the fragments of a dts file after parsing
  dtc: Merge the fragments of a dts file in main()

 dtc-parser.y                       | 130 ++++---------
 dtc.c                              |   2 +
 dtc.h                              |  48 +++++
 livetree.c                         | 302 ++++++++++++++++++++++++++++-
 tests/merge-delete-missing.dts     |   9 +
 tests/merge-errors.dts             |  14 ++
 tests/merge-first-ref-relative.dts |  10 +
 tests/merge-first-ref.dts          |   9 +
 tests/merge-labelled-plugin.dts    |  12 ++
 tests/merge-labelled-relative.dts  |  14 ++
 tests/merge-omit-missing.dts       |   9 +
 tests/merge-parse-error.dts        |  17 ++
 tests/merge-path-ref-missing.dts   |  11 ++
 tests/merge-path-relative.dts      |  12 ++
 tests/run_tests.sh                 |  44 ++++-
 treesource.c                       |   5 +-
 16 files changed, 551 insertions(+), 97 deletions(-)
 create mode 100644 tests/merge-delete-missing.dts
 create mode 100644 tests/merge-errors.dts
 create mode 100644 tests/merge-first-ref-relative.dts
 create mode 100644 tests/merge-first-ref.dts
 create mode 100644 tests/merge-labelled-plugin.dts
 create mode 100644 tests/merge-labelled-relative.dts
 create mode 100644 tests/merge-omit-missing.dts
 create mode 100644 tests/merge-parse-error.dts
 create mode 100644 tests/merge-path-ref-missing.dts
 create mode 100644 tests/merge-path-relative.dts

---
base-commit: c22868416b8db8354ce5db29d3494dda688a2ae3
branch: dt-forest

-- 
2.43.0

[PATCH 01/13] tests: Check each error when merging a dts file

[Simon Glass]

When the parser merges the top-level fragments of a dts file, it can
report that a label or path is not found, or that a label-relative
reference is not supported in a plugin. Most of these errors are not
covered by any test, and the two tests which do reach them only check
that dtc fails, not the message or where it is reported. The merge
logic is about to move out of the grammar, so this needs better tests.

Add a test file for each error which is not covered, and check the
message and its position for all of them, including the two existing
ones. Also check that merging carries on after an error, so that later
errors are reported too, that a reference error is still reported when
the parser finds another error, and that a labelled reference within a
plugin adds the label, since nothing covers these either.

Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Simon Glass <sjg@chromium.org>
---

 tests/merge-delete-missing.dts     |  9 +++++++
 tests/merge-errors.dts             | 14 ++++++++++
 tests/merge-first-ref-relative.dts | 10 ++++++++
 tests/merge-first-ref.dts          |  9 +++++++
 tests/merge-labelled-plugin.dts    | 12 +++++++++
 tests/merge-labelled-relative.dts  | 14 ++++++++++
 tests/merge-omit-missing.dts       |  9 +++++++
 tests/merge-parse-error.dts        | 17 +++++++++++++
 tests/merge-path-ref-missing.dts   | 11 ++++++++
 tests/merge-path-relative.dts      | 12 +++++++++
 tests/run_tests.sh                 | 41 ++++++++++++++++++++++++++++++
 11 files changed, 158 insertions(+)
 create mode 100644 tests/merge-delete-missing.dts
 create mode 100644 tests/merge-errors.dts
 create mode 100644 tests/merge-first-ref-relative.dts
 create mode 100644 tests/merge-first-ref.dts
 create mode 100644 tests/merge-labelled-plugin.dts
 create mode 100644 tests/merge-labelled-relative.dts
 create mode 100644 tests/merge-omit-missing.dts
 create mode 100644 tests/merge-parse-error.dts
 create mode 100644 tests/merge-path-ref-missing.dts
 create mode 100644 tests/merge-path-relative.dts

diff --git a/tests/merge-delete-missing.dts b/tests/merge-delete-missing.dts
new file mode 100644
index 0000000..b36e39e
--- /dev/null
+++ b/tests/merge-delete-missing.dts
@@ -0,0 +1,9 @@
+/dts-v1/;
+
+/ {
+	label: node {
+	};
+};
+
+/* Try to delete a node using a non-existent label */
+/delete-node/ &nosuchnode;
diff --git a/tests/merge-errors.dts b/tests/merge-errors.dts
new file mode 100644
index 0000000..6d3969f
--- /dev/null
+++ b/tests/merge-errors.dts
@@ -0,0 +1,14 @@
+/dts-v1/;
+
+/ {
+	label: node {
+	};
+};
+
+/* Each error is reported, not just the first */
+&nosuchnode1 {
+	prop;
+};
+
+/delete-node/ &nosuchnode2;
+/omit-if-no-ref/ &nosuchnode3;
diff --git a/tests/merge-first-ref-relative.dts b/tests/merge-first-ref-relative.dts
new file mode 100644
index 0000000..103cd1f
--- /dev/null
+++ b/tests/merge-first-ref-relative.dts
@@ -0,0 +1,10 @@
+/dts-v1/;
+/plugin/;
+
+/* A plugin cannot start with a label-relative reference */
+&{label/node} {
+	prop;
+};
+
+/* Later fragments are still merged */
+/delete-node/ &nosuchnode;
diff --git a/tests/merge-first-ref.dts b/tests/merge-first-ref.dts
new file mode 100644
index 0000000..8f23a44
--- /dev/null
+++ b/tests/merge-first-ref.dts
@@ -0,0 +1,9 @@
+/dts-v1/;
+
+/* Only a plugin can start with a reference */
+&nosuchnode {
+	prop;
+};
+
+/* Later fragments are still merged */
+/delete-node/ &nosuchnode2;
diff --git a/tests/merge-labelled-plugin.dts b/tests/merge-labelled-plugin.dts
new file mode 100644
index 0000000..3849cc5
--- /dev/null
+++ b/tests/merge-labelled-plugin.dts
@@ -0,0 +1,12 @@
+/dts-v1/;
+/plugin/;
+
+/ {
+	label: node {
+	};
+};
+
+/* Add a label to a node defined in the plugin itself */
+newlabel: &label {
+	prop = "value";
+};
diff --git a/tests/merge-labelled-relative.dts b/tests/merge-labelled-relative.dts
new file mode 100644
index 0000000..00369e3
--- /dev/null
+++ b/tests/merge-labelled-relative.dts
@@ -0,0 +1,14 @@
+/dts-v1/;
+/plugin/;
+
+/ {
+	label: node {
+		subnode {
+		};
+	};
+};
+
+/* A plugin cannot use a label-relative reference, even if it is found */
+newlabel: &{label/subnode} {
+	prop;
+};
diff --git a/tests/merge-omit-missing.dts b/tests/merge-omit-missing.dts
new file mode 100644
index 0000000..0357648
--- /dev/null
+++ b/tests/merge-omit-missing.dts
@@ -0,0 +1,9 @@
+/dts-v1/;
+
+/ {
+	label: node {
+	};
+};
+
+/* Try to mark a node using a non-existent path */
+/omit-if-no-ref/ &{/nosuchnode};
diff --git a/tests/merge-parse-error.dts b/tests/merge-parse-error.dts
new file mode 100644
index 0000000..c39318b
--- /dev/null
+++ b/tests/merge-parse-error.dts
@@ -0,0 +1,17 @@
+/dts-v1/;
+
+/ {
+	label: node {
+	};
+};
+
+/* Reported even though the parser finds another error below */
+&nosuchnode {
+	prop;
+};
+
+/ {
+	node {
+		prop = <(1/0)>;
+	};
+};
diff --git a/tests/merge-path-ref-missing.dts b/tests/merge-path-ref-missing.dts
new file mode 100644
index 0000000..ad2e35e
--- /dev/null
+++ b/tests/merge-path-ref-missing.dts
@@ -0,0 +1,11 @@
+/dts-v1/;
+
+/ {
+	node {
+	};
+};
+
+/* Try to redefine a node using a non-existent path */
+&{/nosuchnode} {
+	prop;
+};
diff --git a/tests/merge-path-relative.dts b/tests/merge-path-relative.dts
new file mode 100644
index 0000000..94a0b38
--- /dev/null
+++ b/tests/merge-path-relative.dts
@@ -0,0 +1,12 @@
+/dts-v1/;
+/plugin/;
+
+/ {
+	label: node {
+	};
+};
+
+/* A plugin cannot use a label-relative path reference */
+&{label/node} {
+	prop;
+};
diff --git a/tests/run_tests.sh b/tests/run_tests.sh
index 5bc7817..e0504c0 100755
--- a/tests/run_tests.sh
+++ b/tests/run_tests.sh
@@ -765,6 +765,47 @@ dtc_tests () {
     run_sh_test "$SRCDIR/dtc-checkfails.sh" phandle_references -- -I dts -O dtb "$SRCDIR/nonexist-node-ref.dts"
     run_sh_test "$SRCDIR/dtc-checkfails.sh" phandle_references -- -I dts -O dtb "$SRCDIR/nonexist-label-ref.dts"
     run_sh_test "$SRCDIR/dtc-fatal.sh" -I dts -O dtb "$SRCDIR/nonexist-node-ref2.dts"
+
+    # Check each error when merging the fragments of a dts file, with its
+    # position
+    run_wrap_error_msg_test "test_label_ref.dts:7.8-15 Label or path handle not found" \
+	$DTC -I dts -O dtb -o /dev/null "$SRCDIR/test_label_ref.dts"
+    run_wrap_error_msg_test "nonexist-node-ref2.dts:9.1-12 Label or path nosuchnode not found" \
+	$DTC -I dts -O dtb -o /dev/null "$SRCDIR/nonexist-node-ref2.dts"
+    run_wrap_error_msg_test "merge-first-ref.dts:4.13-6.3 Label or path nosuchnode not found" \
+	$DTC -I dts -O dtb -o /dev/null "$SRCDIR/merge-first-ref.dts"
+    run_wrap_error_msg_test "merge-first-ref.dts:9.15-27 Label or path nosuchnode2 not found" \
+	$DTC -I dts -O dtb -o /dev/null "$SRCDIR/merge-first-ref.dts"
+    run_wrap_error_msg_test "merge-first-ref-relative.dts:5.15-7.3 Label-relative reference label/node not supported in plugin" \
+	$DTC -I dts -O dtb -o /dev/null "$SRCDIR/merge-first-ref-relative.dts"
+    run_wrap_error_msg_test "merge-first-ref-relative.dts:10.15-26 Label or path nosuchnode not found" \
+	$DTC -I dts -O dtb -o /dev/null "$SRCDIR/merge-first-ref-relative.dts"
+    run_wrap_error_msg_test "merge-labelled-relative.dts:12.1-10 Label-relative reference label/subnode not supported in plugin" \
+	$DTC -I dts -O dtb -o /dev/null "$SRCDIR/merge-labelled-relative.dts"
+    run_wrap_error_msg_test "merge-path-relative.dts:10.1-14 Label-relative reference label/node not supported in plugin" \
+	$DTC -I dts -O dtb -o /dev/null "$SRCDIR/merge-path-relative.dts"
+    run_wrap_error_msg_test "merge-path-ref-missing.dts:9.1-15 Label or path /nosuchnode not found" \
+	$DTC -I dts -O dtb -o /dev/null "$SRCDIR/merge-path-ref-missing.dts"
+    run_wrap_error_msg_test "merge-delete-missing.dts:9.15-26 Label or path nosuchnode not found" \
+	$DTC -I dts -O dtb -o /dev/null "$SRCDIR/merge-delete-missing.dts"
+    run_wrap_error_msg_test "merge-omit-missing.dts:9.18-32 Label or path /nosuchnode not found" \
+	$DTC -I dts -O dtb -o /dev/null "$SRCDIR/merge-omit-missing.dts"
+    run_wrap_error_msg_test "merge-errors.dts:9.1-13 Label or path nosuchnode1 not found" \
+	$DTC -I dts -O dtb -o /dev/null "$SRCDIR/merge-errors.dts"
+    run_wrap_error_msg_test "merge-errors.dts:13.15-27 Label or path nosuchnode2 not found" \
+	$DTC -I dts -O dtb -o /dev/null "$SRCDIR/merge-errors.dts"
+    run_wrap_error_msg_test "merge-errors.dts:14.18-30 Label or path nosuchnode3 not found" \
+	$DTC -I dts -O dtb -o /dev/null "$SRCDIR/merge-errors.dts"
+    run_wrap_error_msg_test "merge-parse-error.dts:9.1-12 Label or path nosuchnode not found" \
+	$DTC -I dts -O dtb -o /dev/null "$SRCDIR/merge-parse-error.dts"
+    run_wrap_error_msg_test "merge-parse-error.dts:15.12-15 Division by zero" \
+	$DTC -I dts -O dtb -o /dev/null "$SRCDIR/merge-parse-error.dts"
+
+    # A labelled reference to a node in a plugin adds the label to it
+    run_dtc_test -@ -I dts -O dtb -o merge-labelled-plugin.test.dtb \
+	"$SRCDIR/merge-labelled-plugin.dts"
+    run_fdtget_test "/node" merge-labelled-plugin.test.dtb /__symbols__ newlabel
+    run_fdtget_test "value" merge-labelled-plugin.test.dtb /node prop
     check_tests "$SRCDIR/bad-name-property.dts" name_properties
 
     check_tests "$SRCDIR/bad-ncells.dts" address_cells_is_cell size_cells_is_cell interrupts_extended_is_cell
-- 
2.43.0

[PATCH 02/13] dtc: Add a list for the top-level fragments of a dts file

[Simon Glass]

A dts file is made up of top-level fragments: the root node, later
definitions of a node by label or path, and /delete-node/ or
/omit-if-no-ref/ with a reference. We would like to deal with each
fragment on its own before it is merged, for example to apply deletes
consistently or to run some checks on each one, but that requires
keeping the fragments apart. At present the parser merges each one into
the tree as soon as it is parsed.

Add a struct fragment to hold one of these, functions to build one, to
append it to a list and to free it, and a list of them in struct dt_info

These are not used yet.

Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Simon Glass <sjg@chromium.org>
---

 dtc.h      | 47 ++++++++++++++++++++++++++++++++++++++++++
 livetree.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 107 insertions(+)

diff --git a/dtc.h b/dtc.h
index 473552e..c0d8e8f 100644
--- a/dtc.h
+++ b/dtc.h
@@ -322,12 +322,59 @@ struct reserve_info *chain_reserve_entry(struct reserve_info *first,
 struct reserve_info *add_reserve_entry(struct reserve_info *list,
 				       struct reserve_info *new);
 
+/*
+ * A fragment is one top-level part of a dts file, after the header and any
+ * memory reservations: the root node, a later definition of a node which
+ * refers to it by label or path, or a /delete-node/ or /omit-if-no-ref/ with
+ * a reference. Fragments are kept apart until they are merged into a single
+ * tree.
+ *
+ * This is not the same as an overlay fragment (a fragment@N node), although
+ * in a plugin a fragment may become one when it is merged.
+ */
+
+enum fragment_type {
+	FRAGMENT_ROOT,		/* / { ... }; */
+	FRAGMENT_REF,		/* [label:] &ref { ... }; */
+	FRAGMENT_DELETE_NODE,	/* /delete-node/ &ref; */
+	FRAGMENT_OMIT_IF_NO_REF, /* /omit-if-no-ref/ &ref; */
+};
+
+/*
+ * Which fields are used depends on the type:
+ *
+ *   FRAGMENT_ROOT            node
+ *   FRAGMENT_REF             ref, path_ref, ref_srcpos and node, plus label
+ *                            and label_srcpos if there is a label
+ *   FRAGMENT_DELETE_NODE     ref, path_ref and ref_srcpos
+ *   FRAGMENT_OMIT_IF_NO_REF  ref, path_ref and ref_srcpos
+ *
+ * Unused fields are NULL or false.
+ */
+struct fragment {
+	enum fragment_type type;
+	char *ref;			/* label or path referred to */
+	bool path_ref;			/* ref is a path, i.e. &{...} */
+	struct srcpos *ref_srcpos;	/* position of ref */
+	char *label;			/* label to add to the target */
+	struct srcpos *label_srcpos;	/* position of label */
+	struct node *node;		/* the node definition */
+
+	struct fragment *next;		/* NULL at the end of the list */
+};
+
+struct fragment *build_fragment(enum fragment_type type, struct node *node);
+struct fragment *build_fragment_ref(char *ref, bool path_ref,
+				    struct srcpos *srcpos);
+struct fragment *append_fragment(struct fragment *list, struct fragment *new);
+void free_fragment(struct fragment *f);
 
 struct dt_info {
 	unsigned int dtsflags;
 	struct reserve_info *reservelist;
 	uint32_t boot_cpuid_phys;
 	struct node *dt;		/* the device tree */
+	struct fragment *fragments;	/* not yet merged into dt */
 	const char *outname;		/* filename being written to, "-" for stdout */
 };
 
diff --git a/livetree.c b/livetree.c
index abe73de..982f0a6 100644
--- a/livetree.c
+++ b/livetree.c
@@ -448,6 +448,65 @@ struct reserve_info *add_reserve_entry(struct reserve_info *list,
 	return list;
 }
 
+/* Create a fragment of the given type, holding an optional node */
+struct fragment *build_fragment(enum fragment_type type, struct node *node)
+{
+	struct fragment *new = xmalloc(sizeof(*new));
+
+	memset(new, '\0', sizeof(*new));
+
+	new->type = type;
+	new->node = node;
+
+	return new;
+}
+
+/*
+ * Create a fragment which refers to a label, or to a path if path_ref. Its
+ * node is NULL, so the caller must either set the node or change the type to
+ * FRAGMENT_DELETE_NODE or FRAGMENT_OMIT_IF_NO_REF
+ */
+struct fragment *build_fragment_ref(char *ref, bool path_ref,
+				    struct srcpos *srcpos)
+{
+	struct fragment *new = build_fragment(FRAGMENT_REF, NULL);
+
+	new->ref = ref;
+	new->path_ref = path_ref;
+	new->ref_srcpos = srcpos_copy(srcpos);
+
+	return new;
+}
+
+/* Add a fragment to the end of a list, returning the new head */
+struct fragment *append_fragment(struct fragment *list, struct fragment *new)
+{
+	struct fragment *last;
+
+	new->next = NULL;
+
+	if (!list)
+		return new;
+
+	for (last = list; last->next; last = last->next)
+		;
+
+	last->next = new;
+
+	return list;
+}
+
+/*
+ * Free a fragment, but not its node, reference or label, since the tree may
+ * still use these
+ */
+void free_fragment(struct fragment *f)
+{
+	srcpos_free(f->ref_srcpos);
+	srcpos_free(f->label_srcpos);
+	free(f);
+}
+
 static void fill_fullpaths(struct node *tree, const char *prefix)
 {
 	struct node *child;
@@ -475,6 +534,7 @@ struct dt_info *build_dt_info(unsigned int dtsflags,
 	dti->dtsflags = dtsflags;
 	dti->reservelist = reservelist;
 	dti->dt = tree;
+	dti->fragments = NULL;
 	dti->boot_cpuid_phys = boot_cpuid_phys;
 
 	fill_fullpaths(dti->dt, "");
-- 
2.43.0

[PATCH 03/13] dtc: Move the /delete-node/ handling out of the grammar

[Simon Glass]

The grammar actions merge each top-level fragment of a dts file into the
tree as it is parsed, so the merge logic is spread over the rules for
each kind of fragment. This makes it hard to follow and to change.

As a first step towards moving this logic into livetree.c, move the
action for a top-level /delete-node/ into a new merge_delete_node(),
passing it a struct fragment for the reference. Add a fragment_ref rule
to build this.

The output does not change.

Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Simon Glass <sjg@chromium.org>
---

 dtc-parser.y | 26 +++++++++++++++++---------
 dtc.h        |  1 +
 livetree.c   | 19 +++++++++++++++++++
 3 files changed, 37 insertions(+), 9 deletions(-)

diff --git a/dtc-parser.y b/dtc-parser.y
index e47f80d..61301a6 100644
--- a/dtc-parser.y
+++ b/dtc-parser.y
@@ -47,6 +47,7 @@ static bool is_ref_relative(const char *ref)
 	struct node *node;
 	struct node *nodelist;
 	struct reserve_info *re;
+	struct fragment *fragment;
 	uint64_t integer;
 	unsigned int flags;
 }
@@ -80,6 +81,7 @@ static bool is_ref_relative(const char *ref)
 %type <prop> propdef
 %type <proplist> proplist
 %type <labelref> dt_ref
+%type <fragment> fragment_ref
 
 %type <node> devicetree
 %type <node> nodedef
@@ -157,6 +159,17 @@ memreserve:
 
 dt_ref: DT_LABEL_REF | DT_PATH_REF;
 
+fragment_ref:
+	  DT_LABEL_REF
+		{
+			$$ = build_fragment_ref($1, false, &@$);
+		}
+	| DT_PATH_REF
+		{
+			$$ = build_fragment_ref($1, true, &@$);
+		}
+	;
+
 devicetree:
 	  '/' nodedef
 		{
@@ -236,16 +249,11 @@ devicetree:
 			}
 			$$ = $1;
 		}
-	| devicetree DT_DEL_NODE dt_ref ';'
+	| devicetree DT_DEL_NODE fragment_ref ';'
 		{
-			struct node *target = get_node_by_ref($1, $3);
-
-			if (target)
-				delete_node(target);
-			else
-				ERROR(&@3, "Label or path %s not found", $3);
-
-
+			$3->type = FRAGMENT_DELETE_NODE;
+			if (merge_delete_node($1, $3))
+				treesource_error = true;
 			$$ = $1;
 		}
 	| devicetree DT_OMIT_NO_REF dt_ref ';'
diff --git a/dtc.h b/dtc.h
index c0d8e8f..9f9076d 100644
--- a/dtc.h
+++ b/dtc.h
@@ -368,6 +368,7 @@ struct fragment *build_fragment_ref(char *ref, bool path_ref,
 				    struct srcpos *srcpos);
 struct fragment *append_fragment(struct fragment *list, struct fragment *new);
 void free_fragment(struct fragment *f);
+bool merge_delete_node(struct node *dt, struct fragment *f);
 
 struct dt_info {
 	unsigned int dtsflags;
diff --git a/livetree.c b/livetree.c
index 982f0a6..5c9ec48 100644
--- a/livetree.c
+++ b/livetree.c
@@ -542,6 +542,25 @@ struct dt_info *build_dt_info(unsigned int dtsflags,
 	return dti;
 }
 
+/*
+ * Delete the node which a /delete-node/ fragment refers to
+ *
+ * Returns true if there is an error, which has been reported.
+ */
+bool merge_delete_node(struct node *dt, struct fragment *f)
+{
+	struct node *target = get_node_by_ref(dt, f->ref);
+
+	if (!target) {
+		srcpos_error(f->ref_srcpos, "Error",
+			     "Label or path %s not found", f->ref);
+		return true;
+	}
+	delete_node(target);
+
+	return false;
+}
+
 /*
  * Tree accessor functions
  */
-- 
2.43.0

[PATCH 04/13] dtc: Move the /omit-if-no-ref/ handling out of the grammar

[Simon Glass]

The grammar action for a top-level /omit-if-no-ref/ still holds its
merge logic.

Move it into a new merge_omit_if_no_ref(), passing it a struct fragment
for the reference.

The output does not change.

Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Simon Glass <sjg@chromium.org>
---

 dtc-parser.y | 13 ++++---------
 dtc.h        |  1 +
 livetree.c   | 20 ++++++++++++++++++++
 3 files changed, 25 insertions(+), 9 deletions(-)

diff --git a/dtc-parser.y b/dtc-parser.y
index 61301a6..089ab55 100644
--- a/dtc-parser.y
+++ b/dtc-parser.y
@@ -256,16 +256,11 @@ devicetree:
 				treesource_error = true;
 			$$ = $1;
 		}
-	| devicetree DT_OMIT_NO_REF dt_ref ';'
+	| devicetree DT_OMIT_NO_REF fragment_ref ';'
 		{
-			struct node *target = get_node_by_ref($1, $3);
-
-			if (target)
-				omit_node_if_unused(target);
-			else
-				ERROR(&@3, "Label or path %s not found", $3);
-
-
+			$3->type = FRAGMENT_OMIT_IF_NO_REF;
+			if (merge_omit_if_no_ref($1, $3))
+				treesource_error = true;
 			$$ = $1;
 		}
 	;
diff --git a/dtc.h b/dtc.h
index 9f9076d..e747dc0 100644
--- a/dtc.h
+++ b/dtc.h
@@ -369,6 +369,7 @@ struct fragment *build_fragment_ref(char *ref, bool path_ref,
 struct fragment *append_fragment(struct fragment *list, struct fragment *new);
 void free_fragment(struct fragment *f);
 bool merge_delete_node(struct node *dt, struct fragment *f);
+bool merge_omit_if_no_ref(struct node *dt, struct fragment *f);
 
 struct dt_info {
 	unsigned int dtsflags;
diff --git a/livetree.c b/livetree.c
index 5c9ec48..5ed2085 100644
--- a/livetree.c
+++ b/livetree.c
@@ -561,6 +561,26 @@ bool merge_delete_node(struct node *dt, struct fragment *f)
 	return false;
 }
 
+/*
+ * Mark the node which an /omit-if-no-ref/ fragment refers to, so that it is
+ * dropped if nothing refers to it
+ *
+ * Returns true if there is an error, which has been reported.
+ */
+bool merge_omit_if_no_ref(struct node *dt, struct fragment *f)
+{
+	struct node *target = get_node_by_ref(dt, f->ref);
+
+	if (!target) {
+		srcpos_error(f->ref_srcpos, "Error",
+			     "Label or path %s not found", f->ref);
+		return true;
+	}
+	omit_node_if_unused(target);
+
+	return false;
+}
+
 /*
  * Tree accessor functions
  */
-- 
2.43.0

[PATCH 05/13] dtc: Move is_ref_relative() to livetree.c

[Simon Glass]

The remaining grammar actions which merge a fragment use
is_ref_relative() to reject a label-relative reference in a plugin.

Move it to livetree.c, so that it is available when these actions move
there too, and add a comment.

Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Simon Glass <sjg@chromium.org>
---

 dtc-parser.y | 5 -----
 dtc.h        | 1 +
 livetree.c   | 6 ++++++
 3 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/dtc-parser.y b/dtc-parser.y
index 089ab55..7545895 100644
--- a/dtc-parser.y
+++ b/dtc-parser.y
@@ -24,11 +24,6 @@ extern void yyerror(char const *s);
 extern struct dt_info *parser_output;
 extern bool treesource_error;
 
-static bool is_ref_relative(const char *ref)
-{
-	return ref[0] != '/' && strchr(&ref[1], '/');
-}
-
 %}
 
 %union {
diff --git a/dtc.h b/dtc.h
index e747dc0..d145b03 100644
--- a/dtc.h
+++ b/dtc.h
@@ -368,6 +368,7 @@ struct fragment *build_fragment_ref(char *ref, bool path_ref,
 				    struct srcpos *srcpos);
 struct fragment *append_fragment(struct fragment *list, struct fragment *new);
 void free_fragment(struct fragment *f);
+bool is_ref_relative(const char *ref);
 bool merge_delete_node(struct node *dt, struct fragment *f);
 bool merge_omit_if_no_ref(struct node *dt, struct fragment *f);
 
diff --git a/livetree.c b/livetree.c
index 5ed2085..027682a 100644
--- a/livetree.c
+++ b/livetree.c
@@ -542,6 +542,12 @@ struct dt_info *build_dt_info(unsigned int dtsflags,
 	return dti;
 }
 
+/* Check whether a reference is relative to a label, e.g. &{label/node} */
+bool is_ref_relative(const char *ref)
+{
+	return ref[0] != '/' && strchr(&ref[1], '/');
+}
+
 /*
  * Delete the node which a /delete-node/ fragment refers to
  *
-- 
2.43.0

[PATCH 06/13] dtc: Move label-reference handling out of the grammar

[Simon Glass]

The grammar action for a top-level &label { ... } holds its merge logic,
including making an overlay fragment in a plugin when the label is not
found.

Move it into a new merge_label_ref(), passing it a struct fragment for
the reference and node. The rule cannot use fragment_ref yet, since a
path reference still has its own rule, so it builds the fragment itself.

The output does not change.

Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Simon Glass <sjg@chromium.org>
---

 dtc-parser.y | 23 +++++++++--------------
 dtc.h        |  1 +
 livetree.c   | 25 +++++++++++++++++++++++++
 3 files changed, 35 insertions(+), 14 deletions(-)

diff --git a/dtc-parser.y b/dtc-parser.y
index 7545895..1a5ad12 100644
--- a/dtc-parser.y
+++ b/dtc-parser.y
@@ -227,21 +227,16 @@ devicetree:
 		}
 	| devicetree DT_LABEL_REF nodedef
 		{
-			struct node *target = get_node_by_ref($1, $2);
+			struct fragment *f = build_fragment_ref($2, false, &@2);
 
-			if (target) {
-				merge_nodes(target, $3);
-			} else {
-				/*
-				 * We rely on the rule being always:
-				 *   versioninfo plugindecl memreserves devicetree
-				 * so $-1 is what we want (plugindecl)
-				 */
-				if ($<flags>-1 & DTSF_PLUGIN)
-					add_orphan_node($1, $3, $2);
-				else
-					ERROR(&@2, "Label or path %s not found", $2);
-			}
+			f->node = $3;
+			/*
+			 * We rely on the rule being always:
+			 *   versioninfo plugindecl memreserves devicetree
+			 * so $-1 is what we want (plugindecl)
+			 */
+			if (merge_label_ref($1, f, $<flags>-1 & DTSF_PLUGIN))
+				treesource_error = true;
 			$$ = $1;
 		}
 	| devicetree DT_DEL_NODE fragment_ref ';'
diff --git a/dtc.h b/dtc.h
index d145b03..256017f 100644
--- a/dtc.h
+++ b/dtc.h
@@ -369,6 +369,7 @@ struct fragment *build_fragment_ref(char *ref, bool path_ref,
 struct fragment *append_fragment(struct fragment *list, struct fragment *new);
 void free_fragment(struct fragment *f);
 bool is_ref_relative(const char *ref);
+bool merge_label_ref(struct node *dt, struct fragment *f, bool plugin);
 bool merge_delete_node(struct node *dt, struct fragment *f);
 bool merge_omit_if_no_ref(struct node *dt, struct fragment *f);
 
diff --git a/livetree.c b/livetree.c
index 027682a..2a01d49 100644
--- a/livetree.c
+++ b/livetree.c
@@ -548,6 +548,31 @@ bool is_ref_relative(const char *ref)
 	return ref[0] != '/' && strchr(&ref[1], '/');
 }
 
+/*
+ * Merge a fragment which refers to a label into the node with that label. In
+ * a plugin, a label which is not found becomes an overlay fragment.
+ *
+ * Returns true if there is an error, which has been reported.
+ */
+bool merge_label_ref(struct node *dt, struct fragment *f, bool plugin)
+{
+	struct node *target = get_node_by_ref(dt, f->ref);
+
+	if (target) {
+		merge_nodes(target, f->node);
+		return false;
+	}
+	if (!plugin) {
+		srcpos_error(f->ref_srcpos, "Error",
+			     "Label or path %s not found", f->ref);
+		return true;
+	}
+	/* The label may be in the base tree, so add a fragment@N node for it */
+	add_orphan_node(dt, f->node, f->ref);
+
+	return false;
+}
+
 /*
  * Delete the node which a /delete-node/ fragment refers to
  *
-- 
2.43.0

[PATCH 07/13] dtc: Move path-reference handling out of the grammar

[Simon Glass]

The grammar action for a top-level &{/path} { ... } holds its merge
logic, including making an overlay fragment in a plugin.

Move it into a new merge_path_ref(), passing it a struct fragment for
the reference and node, built in the same way as for a label reference.

The output does not change.

Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Simon Glass <sjg@chromium.org>
---

 dtc-parser.y | 17 +++++------------
 dtc.h        |  1 +
 livetree.c   | 36 ++++++++++++++++++++++++++++++++++++
 3 files changed, 42 insertions(+), 12 deletions(-)

diff --git a/dtc-parser.y b/dtc-parser.y
index 1a5ad12..cca3ce1 100644
--- a/dtc-parser.y
+++ b/dtc-parser.y
@@ -206,23 +206,16 @@ devicetree:
 		}
 	| devicetree DT_PATH_REF nodedef
 		{
+			struct fragment *f = build_fragment_ref($2, true, &@2);
+
+			f->node = $3;
 			/*
 			 * We rely on the rule being always:
 			 *   versioninfo plugindecl memreserves devicetree
 			 * so $-1 is what we want (plugindecl)
 			 */
-			if ($<flags>-1 & DTSF_PLUGIN) {
-				if (is_ref_relative($2))
-					ERROR(&@2, "Label-relative reference %s not supported in plugin", $2);
-				add_orphan_node($1, $3, $2);
-			} else {
-				struct node *target = get_node_by_ref($1, $2);
-
-				if (target)
-					merge_nodes(target, $3);
-				else
-					ERROR(&@2, "Label or path %s not found", $2);
-			}
+			if (merge_path_ref($1, f, $<flags>-1 & DTSF_PLUGIN))
+				treesource_error = true;
 			$$ = $1;
 		}
 	| devicetree DT_LABEL_REF nodedef
diff --git a/dtc.h b/dtc.h
index 256017f..5af1d1f 100644
--- a/dtc.h
+++ b/dtc.h
@@ -369,6 +369,7 @@ struct fragment *build_fragment_ref(char *ref, bool path_ref,
 struct fragment *append_fragment(struct fragment *list, struct fragment *new);
 void free_fragment(struct fragment *f);
 bool is_ref_relative(const char *ref);
+bool merge_path_ref(struct node *dt, struct fragment *f, bool plugin);
 bool merge_label_ref(struct node *dt, struct fragment *f, bool plugin);
 bool merge_delete_node(struct node *dt, struct fragment *f);
 bool merge_omit_if_no_ref(struct node *dt, struct fragment *f);
diff --git a/livetree.c b/livetree.c
index 2a01d49..b0362e4 100644
--- a/livetree.c
+++ b/livetree.c
@@ -548,6 +548,42 @@ bool is_ref_relative(const char *ref)
 	return ref[0] != '/' && strchr(&ref[1], '/');
 }
 
+/*
+ * Merge a fragment which refers to a path into the node at that path. In a
+ * plugin, the fragment becomes an overlay fragment instead.
+ *
+ * Returns true if there is an error, which has been reported.
+ */
+bool merge_path_ref(struct node *dt, struct fragment *f, bool plugin)
+{
+	struct node *target;
+
+	if (plugin) {
+		bool error = false;
+
+		if (is_ref_relative(f->ref)) {
+			srcpos_error(f->ref_srcpos, "Error",
+				     "Label-relative reference %s not supported in plugin",
+				     f->ref);
+			error = true;
+		}
+		/* Add a fragment@N node which applies the node at the path */
+		add_orphan_node(dt, f->node, f->ref);
+
+		return error;
+	}
+
+	target = get_node_by_ref(dt, f->ref);
+	if (!target) {
+		srcpos_error(f->ref_srcpos, "Error",
+			     "Label or path %s not found", f->ref);
+		return true;
+	}
+	merge_nodes(target, f->node);
+
+	return false;
+}
+
 /*
  * Merge a fragment which refers to a label into the node with that label. In
  * a plugin, a label which is not found becomes an overlay fragment.
-- 
2.43.0

[PATCH 08/13] dtc: Move labelled-reference handling out of the grammar

[Simon Glass]

The grammar action for a top-level label: &ref { ... } holds its merge
logic, which adds the label to the node referred to.

Move it into a new merge_labelled_ref(), passing it a struct fragment
from fragment_ref, with the label and node added.

The output does not change.

Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Simon Glass <sjg@chromium.org>
---

 dtc-parser.y | 18 +++++++-----------
 dtc.h        |  2 ++
 livetree.c   | 28 ++++++++++++++++++++++++++++
 3 files changed, 37 insertions(+), 11 deletions(-)

diff --git a/dtc-parser.y b/dtc-parser.y
index cca3ce1..0e6c4c2 100644
--- a/dtc-parser.y
+++ b/dtc-parser.y
@@ -190,18 +190,14 @@ devicetree:
 						  ""),
 					$2, $1);
 		}
-	| devicetree DT_LABEL dt_ref nodedef
+	| devicetree DT_LABEL fragment_ref nodedef
 		{
-			struct node *target = get_node_by_ref($1, $3);
-
-			if (($<flags>-1 & DTSF_PLUGIN) && is_ref_relative($3))
-				ERROR(&@2, "Label-relative reference %s not supported in plugin", $3);
-
-			if (target) {
-				add_label(&target->labels, $2);
-				merge_nodes(target, $4);
-			} else
-				ERROR(&@3, "Label or path %s not found", $3);
+			$3->label = $2;
+			$3->label_srcpos = srcpos_copy(&@2);
+			$3->node = $4;
+			if (merge_labelled_ref($1, $3,
+					       $<flags>-1 & DTSF_PLUGIN))
+				treesource_error = true;
 			$$ = $1;
 		}
 	| devicetree DT_PATH_REF nodedef
diff --git a/dtc.h b/dtc.h
index 5af1d1f..75b1c0f 100644
--- a/dtc.h
+++ b/dtc.h
@@ -369,6 +369,8 @@ struct fragment *build_fragment_ref(char *ref, bool path_ref,
 struct fragment *append_fragment(struct fragment *list, struct fragment *new);
 void free_fragment(struct fragment *f);
 bool is_ref_relative(const char *ref);
+bool merge_labelled_ref(struct node *dt, struct fragment *f,
+			bool plugin);
 bool merge_path_ref(struct node *dt, struct fragment *f, bool plugin);
 bool merge_label_ref(struct node *dt, struct fragment *f, bool plugin);
 bool merge_delete_node(struct node *dt, struct fragment *f);
diff --git a/livetree.c b/livetree.c
index b0362e4..76474cd 100644
--- a/livetree.c
+++ b/livetree.c
@@ -548,6 +548,34 @@ bool is_ref_relative(const char *ref)
 	return ref[0] != '/' && strchr(&ref[1], '/');
 }
 
+/*
+ * Merge a fragment of the form 'label: &ref { ... }' into the node which ref
+ * refers to, adding the label to that node
+ *
+ * Returns true if there is an error, which has been reported.
+ */
+bool merge_labelled_ref(struct node *dt, struct fragment *f, bool plugin)
+{
+	struct node *target = get_node_by_ref(dt, f->ref);
+	bool error = false;
+
+	if (plugin && is_ref_relative(f->ref)) {
+		srcpos_error(f->label_srcpos, "Error",
+			     "Label-relative reference %s not supported in plugin",
+			     f->ref);
+		error = true;
+	}
+	if (!target) {
+		srcpos_error(f->ref_srcpos, "Error",
+			     "Label or path %s not found", f->ref);
+		return true;
+	}
+	add_label(&target->labels, f->label);
+	merge_nodes(target, f->node);
+
+	return error;
+}
+
 /*
  * Merge a fragment which refers to a path into the node at that path. In a
  * plugin, the fragment becomes an overlay fragment instead.
-- 
2.43.0

[PATCH 09/13] dtc: Move first-reference handling out of the grammar

[Simon Glass]

In a plugin, the first top-level fragment can refer to a label or path
instead of being the root node. The grammar action for this holds its
merge logic, which makes an overlay fragment in a new tree.

Move it into a new merge_first_ref(), passing it a struct fragment from
fragment_ref, with the node added. This is the last user of
is_ref_relative() in the grammar, so make that function static.

The output does not change.

Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Simon Glass <sjg@chromium.org>
---

 dtc-parser.y | 13 ++++---------
 dtc.h        |  2 +-
 livetree.c   | 30 +++++++++++++++++++++++++++++-
 3 files changed, 34 insertions(+), 11 deletions(-)

diff --git a/dtc-parser.y b/dtc-parser.y
index 0e6c4c2..32f7c26 100644
--- a/dtc-parser.y
+++ b/dtc-parser.y
@@ -174,21 +174,16 @@ devicetree:
 		{
 			$$ = merge_nodes($1, $3);
 		}
-	| dt_ref nodedef
+	| fragment_ref nodedef
 		{
+			$1->node = $2;
 			/*
 			 * We rely on the rule being always:
 			 *   versioninfo plugindecl memreserves devicetree
 			 * so $-1 is what we want (plugindecl)
 			 */
-			if (!($<flags>-1 & DTSF_PLUGIN))
-				ERROR(&@2, "Label or path %s not found", $1);
-			else if (is_ref_relative($1))
-				ERROR(&@2, "Label-relative reference %s not supported in plugin", $1);
-			$$ = add_orphan_node(
-					name_node(build_node(NULL, NULL, NULL),
-						  ""),
-					$2, $1);
+			if (merge_first_ref(&$$, $1, $<flags>-1 & DTSF_PLUGIN))
+				treesource_error = true;
 		}
 	| devicetree DT_LABEL fragment_ref nodedef
 		{
diff --git a/dtc.h b/dtc.h
index 75b1c0f..32e4394 100644
--- a/dtc.h
+++ b/dtc.h
@@ -368,7 +368,7 @@ struct fragment *build_fragment_ref(char *ref, bool path_ref,
 				    struct srcpos *srcpos);
 struct fragment *append_fragment(struct fragment *list, struct fragment *new);
 void free_fragment(struct fragment *f);
-bool is_ref_relative(const char *ref);
+bool merge_first_ref(struct node **dtp, struct fragment *f, bool plugin);
 bool merge_labelled_ref(struct node *dt, struct fragment *f,
 			bool plugin);
 bool merge_path_ref(struct node *dt, struct fragment *f, bool plugin);
diff --git a/livetree.c b/livetree.c
index 76474cd..f2244e2 100644
--- a/livetree.c
+++ b/livetree.c
@@ -543,11 +543,39 @@ struct dt_info *build_dt_info(unsigned int dtsflags,
 }
 
 /* Check whether a reference is relative to a label, e.g. &{label/node} */
-bool is_ref_relative(const char *ref)
+static bool is_ref_relative(const char *ref)
 {
 	return ref[0] != '/' && strchr(&ref[1], '/');
 }
 
+/*
+ * Make the tree at *dtp from a first fragment which refers to a label or path,
+ * rather than being the root node. This is only valid in a plugin, where the
+ * fragment becomes an overlay fragment.
+ *
+ * Returns true if there is an error, which has been reported.
+ */
+bool merge_first_ref(struct node **dtp, struct fragment *f, bool plugin)
+{
+	bool error = false;
+
+	if (!plugin) {
+		srcpos_error(f->node->srcpos, "Error",
+			     "Label or path %s not found", f->ref);
+		error = true;
+	} else if (is_ref_relative(f->ref)) {
+		srcpos_error(f->node->srcpos, "Error",
+			     "Label-relative reference %s not supported in plugin",
+			     f->ref);
+		error = true;
+	}
+	/* Make a new, empty tree with a fragment@N node for the reference */
+	*dtp = add_orphan_node(name_node(build_node(NULL, NULL, NULL), ""),
+			       f->node, f->ref);
+
+	return error;
+}
+
 /*
  * Merge a fragment of the form 'label: &ref { ... }' into the node which ref
  * refers to, adding the label to that node
-- 
2.43.0

[PATCH 10/13] dtc: Add merge_fragment() to merge any fragment

[Simon Glass]

Each grammar action which merges a fragment calls the function for its
own kind of fragment, sets treesource_error if that fails, and finds out
whether the file is a plugin using $<flags>-1

Add merge_fragment() to merge any fragment, choosing the function by the
type of fragment and whether it is the first. Have each action build a
struct fragment and pass it to a new merge_parsed_fragment() helper,
which calls merge_fragment(), sets treesource_error and frees the
fragment. The two rules for a plain label or path reference become one,
using fragment_ref, and the functions used by merge_fragment() become
static.

The output does not change.

Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Simon Glass <sjg@chromium.org>
---

 dtc-parser.y | 74 ++++++++++++++++++++--------------------------------
 dtc.h        |  8 +-----
 livetree.c   | 50 ++++++++++++++++++++++++++++++-----
 3 files changed, 74 insertions(+), 58 deletions(-)

diff --git a/dtc-parser.y b/dtc-parser.y
index 32f7c26..1ed9731 100644
--- a/dtc-parser.y
+++ b/dtc-parser.y
@@ -24,6 +24,22 @@ extern void yyerror(char const *s);
 extern struct dt_info *parser_output;
 extern bool treesource_error;
 
+/*
+ * Merge a fragment into the tree as soon as it is parsed. The flags come
+ * from the actions using $<flags>-1, which relies on the rule being always:
+ *   headers memreserves devicetree
+ * so $-1 is the value of headers
+ */
+static struct node *merge_parsed_fragment(struct node *dt, struct fragment *f,
+					  unsigned int flags)
+{
+	if (merge_fragment(&dt, f, flags & DTSF_PLUGIN))
+		treesource_error = true;
+	free_fragment(f);
+
+	return dt;
+}
+
 %}
 
 %union {
@@ -168,74 +184,42 @@ fragment_ref:
 devicetree:
 	  '/' nodedef
 		{
-			$$ = name_node($2, "");
+			struct fragment *f = build_fragment(FRAGMENT_ROOT, $2);
+
+			$$ = merge_parsed_fragment(NULL, f, $<flags>-1);
 		}
 	| devicetree '/' nodedef
 		{
-			$$ = merge_nodes($1, $3);
+			struct fragment *f = build_fragment(FRAGMENT_ROOT, $3);
+
+			$$ = merge_parsed_fragment($1, f, $<flags>-1);
 		}
 	| fragment_ref nodedef
 		{
 			$1->node = $2;
-			/*
-			 * We rely on the rule being always:
-			 *   versioninfo plugindecl memreserves devicetree
-			 * so $-1 is what we want (plugindecl)
-			 */
-			if (merge_first_ref(&$$, $1, $<flags>-1 & DTSF_PLUGIN))
-				treesource_error = true;
+			$$ = merge_parsed_fragment(NULL, $1, $<flags>-1);
 		}
 	| devicetree DT_LABEL fragment_ref nodedef
 		{
 			$3->label = $2;
 			$3->label_srcpos = srcpos_copy(&@2);
 			$3->node = $4;
-			if (merge_labelled_ref($1, $3,
-					       $<flags>-1 & DTSF_PLUGIN))
-				treesource_error = true;
-			$$ = $1;
+			$$ = merge_parsed_fragment($1, $3, $<flags>-1);
 		}
-	| devicetree DT_PATH_REF nodedef
+	| devicetree fragment_ref nodedef
 		{
-			struct fragment *f = build_fragment_ref($2, true, &@2);
-
-			f->node = $3;
-			/*
-			 * We rely on the rule being always:
-			 *   versioninfo plugindecl memreserves devicetree
-			 * so $-1 is what we want (plugindecl)
-			 */
-			if (merge_path_ref($1, f, $<flags>-1 & DTSF_PLUGIN))
-				treesource_error = true;
-			$$ = $1;
-		}
-	| devicetree DT_LABEL_REF nodedef
-		{
-			struct fragment *f = build_fragment_ref($2, false, &@2);
-
-			f->node = $3;
-			/*
-			 * We rely on the rule being always:
-			 *   versioninfo plugindecl memreserves devicetree
-			 * so $-1 is what we want (plugindecl)
-			 */
-			if (merge_label_ref($1, f, $<flags>-1 & DTSF_PLUGIN))
-				treesource_error = true;
-			$$ = $1;
+			$2->node = $3;
+			$$ = merge_parsed_fragment($1, $2, $<flags>-1);
 		}
 	| devicetree DT_DEL_NODE fragment_ref ';'
 		{
 			$3->type = FRAGMENT_DELETE_NODE;
-			if (merge_delete_node($1, $3))
-				treesource_error = true;
-			$$ = $1;
+			$$ = merge_parsed_fragment($1, $3, $<flags>-1);
 		}
 	| devicetree DT_OMIT_NO_REF fragment_ref ';'
 		{
 			$3->type = FRAGMENT_OMIT_IF_NO_REF;
-			if (merge_omit_if_no_ref($1, $3))
-				treesource_error = true;
-			$$ = $1;
+			$$ = merge_parsed_fragment($1, $3, $<flags>-1);
 		}
 	;
 
diff --git a/dtc.h b/dtc.h
index 32e4394..41013f5 100644
--- a/dtc.h
+++ b/dtc.h
@@ -368,13 +368,7 @@ struct fragment *build_fragment_ref(char *ref, bool path_ref,
 				    struct srcpos *srcpos);
 struct fragment *append_fragment(struct fragment *list, struct fragment *new);
 void free_fragment(struct fragment *f);
-bool merge_first_ref(struct node **dtp, struct fragment *f, bool plugin);
-bool merge_labelled_ref(struct node *dt, struct fragment *f,
-			bool plugin);
-bool merge_path_ref(struct node *dt, struct fragment *f, bool plugin);
-bool merge_label_ref(struct node *dt, struct fragment *f, bool plugin);
-bool merge_delete_node(struct node *dt, struct fragment *f);
-bool merge_omit_if_no_ref(struct node *dt, struct fragment *f);
+bool merge_fragment(struct node **dtp, struct fragment *f, bool plugin);
 
 struct dt_info {
 	unsigned int dtsflags;
diff --git a/livetree.c b/livetree.c
index f2244e2..6a290fb 100644
--- a/livetree.c
+++ b/livetree.c
@@ -555,7 +555,7 @@ static bool is_ref_relative(const char *ref)
  *
  * Returns true if there is an error, which has been reported.
  */
-bool merge_first_ref(struct node **dtp, struct fragment *f, bool plugin)
+static bool merge_first_ref(struct node **dtp, struct fragment *f, bool plugin)
 {
 	bool error = false;
 
@@ -582,7 +582,7 @@ bool merge_first_ref(struct node **dtp, struct fragment *f, bool plugin)
  *
  * Returns true if there is an error, which has been reported.
  */
-bool merge_labelled_ref(struct node *dt, struct fragment *f, bool plugin)
+static bool merge_labelled_ref(struct node *dt, struct fragment *f, bool plugin)
 {
 	struct node *target = get_node_by_ref(dt, f->ref);
 	bool error = false;
@@ -610,7 +610,7 @@ bool merge_labelled_ref(struct node *dt, struct fragment *f, bool plugin)
  *
  * Returns true if there is an error, which has been reported.
  */
-bool merge_path_ref(struct node *dt, struct fragment *f, bool plugin)
+static bool merge_path_ref(struct node *dt, struct fragment *f, bool plugin)
 {
 	struct node *target;
 
@@ -646,7 +646,7 @@ bool merge_path_ref(struct node *dt, struct fragment *f, bool plugin)
  *
  * Returns true if there is an error, which has been reported.
  */
-bool merge_label_ref(struct node *dt, struct fragment *f, bool plugin)
+static bool merge_label_ref(struct node *dt, struct fragment *f, bool plugin)
 {
 	struct node *target = get_node_by_ref(dt, f->ref);
 
@@ -670,7 +670,7 @@ bool merge_label_ref(struct node *dt, struct fragment *f, bool plugin)
  *
  * Returns true if there is an error, which has been reported.
  */
-bool merge_delete_node(struct node *dt, struct fragment *f)
+static bool merge_delete_node(struct node *dt, struct fragment *f)
 {
 	struct node *target = get_node_by_ref(dt, f->ref);
 
@@ -690,7 +690,7 @@ bool merge_delete_node(struct node *dt, struct fragment *f)
  *
  * Returns true if there is an error, which has been reported.
  */
-bool merge_omit_if_no_ref(struct node *dt, struct fragment *f)
+static bool merge_omit_if_no_ref(struct node *dt, struct fragment *f)
 {
 	struct node *target = get_node_by_ref(dt, f->ref);
 
@@ -704,6 +704,44 @@ bool merge_omit_if_no_ref(struct node *dt, struct fragment *f)
 	return false;
 }
 
+/*
+ * Merge a fragment of a dts file into the tree at *dtp, which is NULL for the
+ * first fragment. A reference is looked up in the tree as merged so far.
+ *
+ * Returns true if there is an error, which has been reported.
+ */
+bool merge_fragment(struct node **dtp, struct fragment *f, bool plugin)
+{
+	struct node *dt = *dtp;
+
+	if (!dt) {
+		/* The first fragment is the root or, in a plugin, a ref */
+		if (f->type == FRAGMENT_ROOT) {
+			*dtp = name_node(f->node, "");
+			return false;
+		}
+		return merge_first_ref(dtp, f, plugin);
+	}
+
+	switch (f->type) {
+	case FRAGMENT_ROOT:
+		*dtp = merge_nodes(dt, f->node);
+		break;
+	case FRAGMENT_REF:
+		if (f->label)
+			return merge_labelled_ref(dt, f, plugin);
+		if (f->path_ref)
+			return merge_path_ref(dt, f, plugin);
+		return merge_label_ref(dt, f, plugin);
+	case FRAGMENT_DELETE_NODE:
+		return merge_delete_node(dt, f);
+	case FRAGMENT_OMIT_IF_NO_REF:
+		return merge_omit_if_no_ref(dt, f);
+	}
+
+	return false;
+}
+
 /*
  * Tree accessor functions
  */
-- 
2.43.0

[PATCH 11/13] dtc: Add helpers for the errors when merging a fragment

[Simon Glass]

Four of the merge functions look up the node which a fragment refers to
and report 'Label or path not found' if it is missing, and three check
for a label-relative reference in a plugin. Each of these has its own
copy of the error, so a change to a message or its position must be made
in several places.

Add lookup_ref() to look up the node, reporting an error if it is not
found, and check_relative_ref() to report a label-relative reference.
Use these in the merge functions.

The output does not change.

Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Simon Glass <sjg@chromium.org>
---

 livetree.c | 90 +++++++++++++++++++++++++++++-------------------------
 1 file changed, 49 insertions(+), 41 deletions(-)

diff --git a/livetree.c b/livetree.c
index 6a290fb..1e012d4 100644
--- a/livetree.c
+++ b/livetree.c
@@ -548,6 +548,40 @@ static bool is_ref_relative(const char *ref)
 	return ref[0] != '/' && strchr(&ref[1], '/');
 }
 
+/*
+ * Report an error at pos if a fragment uses a label-relative reference, which
+ * a plugin does not support
+ *
+ * Returns true if there is an error
+ */
+static bool check_relative_ref(struct fragment *f, struct srcpos *pos)
+{
+	if (!is_ref_relative(f->ref))
+		return false;
+	srcpos_error(pos, "Error",
+		     "Label-relative reference %s not supported in plugin",
+		     f->ref);
+
+	return true;
+}
+
+/*
+ * Look up the node which a fragment refers to, reporting an error if it is
+ * not found
+ *
+ * Returns the node, or NULL if it is not found
+ */
+static struct node *lookup_ref(struct node *dt, struct fragment *f)
+{
+	struct node *target = get_node_by_ref(dt, f->ref);
+
+	if (!target)
+		srcpos_error(f->ref_srcpos, "Error",
+			     "Label or path %s not found", f->ref);
+
+	return target;
+}
+
 /*
  * Make the tree at *dtp from a first fragment which refers to a label or path,
  * rather than being the root node. This is only valid in a plugin, where the
@@ -557,17 +591,14 @@ static bool is_ref_relative(const char *ref)
  */
 static bool merge_first_ref(struct node **dtp, struct fragment *f, bool plugin)
 {
-	bool error = false;
+	bool error;
 
-	if (!plugin) {
+	if (plugin) {
+		error = check_relative_ref(f, f->node->srcpos);
+	} else {
 		srcpos_error(f->node->srcpos, "Error",
 			     "Label or path %s not found", f->ref);
 		error = true;
-	} else if (is_ref_relative(f->ref)) {
-		srcpos_error(f->node->srcpos, "Error",
-			     "Label-relative reference %s not supported in plugin",
-			     f->ref);
-		error = true;
 	}
 	/* Make a new, empty tree with a fragment@N node for the reference */
 	*dtp = add_orphan_node(name_node(build_node(NULL, NULL, NULL), ""),
@@ -584,20 +615,12 @@ static bool merge_first_ref(struct node **dtp, struct fragment *f, bool plugin)
  */
 static bool merge_labelled_ref(struct node *dt, struct fragment *f, bool plugin)
 {
-	struct node *target = get_node_by_ref(dt, f->ref);
-	bool error = false;
+	struct node *target;
+	bool error = plugin && check_relative_ref(f, f->label_srcpos);
 
-	if (plugin && is_ref_relative(f->ref)) {
-		srcpos_error(f->label_srcpos, "Error",
-			     "Label-relative reference %s not supported in plugin",
-			     f->ref);
-		error = true;
-	}
-	if (!target) {
-		srcpos_error(f->ref_srcpos, "Error",
-			     "Label or path %s not found", f->ref);
+	target = lookup_ref(dt, f);
+	if (!target)
 		return true;
-	}
 	add_label(&target->labels, f->label);
 	merge_nodes(target, f->node);
 
@@ -615,26 +638,17 @@ static bool merge_path_ref(struct node *dt, struct fragment *f, bool plugin)
 	struct node *target;
 
 	if (plugin) {
-		bool error = false;
+		bool error = check_relative_ref(f, f->ref_srcpos);
 
-		if (is_ref_relative(f->ref)) {
-			srcpos_error(f->ref_srcpos, "Error",
-				     "Label-relative reference %s not supported in plugin",
-				     f->ref);
-			error = true;
-		}
 		/* Add a fragment@N node which applies the node at the path */
 		add_orphan_node(dt, f->node, f->ref);
 
 		return error;
 	}
 
-	target = get_node_by_ref(dt, f->ref);
-	if (!target) {
-		srcpos_error(f->ref_srcpos, "Error",
-			     "Label or path %s not found", f->ref);
+	target = lookup_ref(dt, f);
+	if (!target)
 		return true;
-	}
 	merge_nodes(target, f->node);
 
 	return false;
@@ -672,13 +686,10 @@ static bool merge_label_ref(struct node *dt, struct fragment *f, bool plugin)
  */
 static bool merge_delete_node(struct node *dt, struct fragment *f)
 {
-	struct node *target = get_node_by_ref(dt, f->ref);
+	struct node *target = lookup_ref(dt, f);
 
-	if (!target) {
-		srcpos_error(f->ref_srcpos, "Error",
-			     "Label or path %s not found", f->ref);
+	if (!target)
 		return true;
-	}
 	delete_node(target);
 
 	return false;
@@ -692,13 +703,10 @@ static bool merge_delete_node(struct node *dt, struct fragment *f)
  */
 static bool merge_omit_if_no_ref(struct node *dt, struct fragment *f)
 {
-	struct node *target = get_node_by_ref(dt, f->ref);
+	struct node *target = lookup_ref(dt, f);
 
-	if (!target) {
-		srcpos_error(f->ref_srcpos, "Error",
-			     "Label or path %s not found", f->ref);
+	if (!target)
 		return true;
-	}
 	omit_node_if_unused(target);
 
 	return false;
-- 
2.43.0

[PATCH 12/13] dtc: Merge the fragments of a dts file after parsing

[Simon Glass]

We would like to deal with each top-level fragment of a dts file before
it is merged into the tree, but that requires the fragments to be kept
apart until the whole file is parsed. At present the parser merges each
one as soon as it is parsed. The grammar actions also need to know
whether the file is a plugin, which they find using $<flags>-1

Have the grammar build a list of the fragments instead, in source order,
and merge them with a new merge_fragments() once the whole file is
parsed. This calls merge_fragment() for each one, so the output does not
change, and frees each fragment once it is merged.

The $<flags>-1 term is no longer needed. Within a devicetree action it
reaches back up the parser stack to the value of headers, i.e. the flags
from /dts-v1/ and /plugin/. The sourcefile action passes the same value
to build_dt_info(), which stores it in dti->dtsflags, so
merge_fragments() can use that instead.

There are two differences in the errors. A syntax error later in the
file now stops dtc before any earlier reference errors are reported, and
reference errors now come after any other errors which the parser
reports, such as a division by zero, rather than in source order.

dtc gives the same output and messages as before for all 4162 .dts and
.dtso files in Linux v7.3-12284-g3799eeb906c9, and for the dts files in
tests/

Suggested-by: David Gibson <david@gibson.dropbear.id.au>
Link: https://lore.kernel.org/devicetree-compiler/auK0ERvxGCJGSc3p@gractus.seuss/
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Simon Glass <sjg@chromium.org>
---

 dtc-parser.y | 43 +++++++++++++------------------------------
 dtc.h        |  2 +-
 livetree.c   | 36 ++++++++++++++++++++++++++++++++++--
 3 files changed, 48 insertions(+), 33 deletions(-)

diff --git a/dtc-parser.y b/dtc-parser.y
index 1ed9731..c1aa4ad 100644
--- a/dtc-parser.y
+++ b/dtc-parser.y
@@ -24,22 +24,6 @@ extern void yyerror(char const *s);
 extern struct dt_info *parser_output;
 extern bool treesource_error;
 
-/*
- * Merge a fragment into the tree as soon as it is parsed. The flags come
- * from the actions using $<flags>-1, which relies on the rule being always:
- *   headers memreserves devicetree
- * so $-1 is the value of headers
- */
-static struct node *merge_parsed_fragment(struct node *dt, struct fragment *f,
-					  unsigned int flags)
-{
-	if (merge_fragment(&dt, f, flags & DTSF_PLUGIN))
-		treesource_error = true;
-	free_fragment(f);
-
-	return dt;
-}
-
 %}
 
 %union {
@@ -94,7 +78,7 @@ static struct node *merge_parsed_fragment(struct node *dt, struct fragment *f,
 %type <labelref> dt_ref
 %type <fragment> fragment_ref
 
-%type <node> devicetree
+%type <fragment> devicetree
 %type <node> nodedef
 %type <node> subnode
 %type <nodelist> subnodes
@@ -119,8 +103,10 @@ static struct node *merge_parsed_fragment(struct node *dt, struct fragment *f,
 sourcefile:
 	  headers memreserves devicetree
 		{
-			parser_output = build_dt_info($1, $2, $3,
-			                              guess_boot_cpuid($3));
+			parser_output = build_dt_info($1, $2, NULL, 0);
+			parser_output->fragments = $3;
+			if (merge_fragments(parser_output))
+				treesource_error = true;
 		}
 	;
 
@@ -184,42 +170,39 @@ fragment_ref:
 devicetree:
 	  '/' nodedef
 		{
-			struct fragment *f = build_fragment(FRAGMENT_ROOT, $2);
-
-			$$ = merge_parsed_fragment(NULL, f, $<flags>-1);
+			$$ = build_fragment(FRAGMENT_ROOT, $2);
 		}
 	| devicetree '/' nodedef
 		{
-			struct fragment *f = build_fragment(FRAGMENT_ROOT, $3);
-
-			$$ = merge_parsed_fragment($1, f, $<flags>-1);
+			$$ = append_fragment($1, build_fragment(FRAGMENT_ROOT,
+								$3));
 		}
 	| fragment_ref nodedef
 		{
 			$1->node = $2;
-			$$ = merge_parsed_fragment(NULL, $1, $<flags>-1);
+			$$ = $1;
 		}
 	| devicetree DT_LABEL fragment_ref nodedef
 		{
 			$3->label = $2;
 			$3->label_srcpos = srcpos_copy(&@2);
 			$3->node = $4;
-			$$ = merge_parsed_fragment($1, $3, $<flags>-1);
+			$$ = append_fragment($1, $3);
 		}
 	| devicetree fragment_ref nodedef
 		{
 			$2->node = $3;
-			$$ = merge_parsed_fragment($1, $2, $<flags>-1);
+			$$ = append_fragment($1, $2);
 		}
 	| devicetree DT_DEL_NODE fragment_ref ';'
 		{
 			$3->type = FRAGMENT_DELETE_NODE;
-			$$ = merge_parsed_fragment($1, $3, $<flags>-1);
+			$$ = append_fragment($1, $3);
 		}
 	| devicetree DT_OMIT_NO_REF fragment_ref ';'
 		{
 			$3->type = FRAGMENT_OMIT_IF_NO_REF;
-			$$ = merge_parsed_fragment($1, $3, $<flags>-1);
+			$$ = append_fragment($1, $3);
 		}
 	;
 
diff --git a/dtc.h b/dtc.h
index 41013f5..fdd0d75 100644
--- a/dtc.h
+++ b/dtc.h
@@ -368,7 +368,6 @@ struct fragment *build_fragment_ref(char *ref, bool path_ref,
 				    struct srcpos *srcpos);
 struct fragment *append_fragment(struct fragment *list, struct fragment *new);
 void free_fragment(struct fragment *f);
-bool merge_fragment(struct node **dtp, struct fragment *f, bool plugin);
 
 struct dt_info {
 	unsigned int dtsflags;
@@ -386,6 +385,7 @@ struct dt_info {
 struct dt_info *build_dt_info(unsigned int dtsflags,
 			      struct reserve_info *reservelist,
 			      struct node *tree, uint32_t boot_cpuid_phys);
+bool merge_fragments(struct dt_info *dti);
 void sort_tree(struct dt_info *dti);
 void generate_labels_from_tree(struct dt_info *dti, const char *name);
 void generate_label_tree(struct dt_info *dti, const char *name, bool allocph);
diff --git a/livetree.c b/livetree.c
index 1e012d4..68ad3c3 100644
--- a/livetree.c
+++ b/livetree.c
@@ -537,7 +537,8 @@ struct dt_info *build_dt_info(unsigned int dtsflags,
 	dti->fragments = NULL;
 	dti->boot_cpuid_phys = boot_cpuid_phys;
 
-	fill_fullpaths(dti->dt, "");
+	if (tree)
+		fill_fullpaths(dti->dt, "");
 
 	return dti;
 }
@@ -718,7 +719,8 @@ static bool merge_omit_if_no_ref(struct node *dt, struct fragment *f)
  *
  * Returns true if there is an error, which has been reported.
  */
-bool merge_fragment(struct node **dtp, struct fragment *f, bool plugin)
+static bool merge_fragment(struct node **dtp, struct fragment *f,
+			   bool plugin)
 {
 	struct node *dt = *dtp;
 
@@ -750,6 +752,36 @@ bool merge_fragment(struct node **dtp, struct fragment *f, bool plugin)
 	return false;
 }
 
+/*
+ * Merge the fragments of a dts file into a single tree, in source order.
+ *
+ * Returns true if there are errors, which have been reported.
+ */
+bool merge_fragments(struct dt_info *dti)
+{
+	bool plugin = dti->dtsflags & DTSF_PLUGIN;
+	struct fragment *f, *next;
+	struct node *dt = NULL;
+	bool error = false;
+
+	if (!dti->fragments)
+		return false;
+
+	for (f = dti->fragments; f; f = next) {
+		next = f->next;
+		if (merge_fragment(&dt, f, plugin))
+			error = true;
+		free_fragment(f);
+	}
+
+	dti->dt = dt;
+	dti->fragments = NULL;
+	dti->boot_cpuid_phys = guess_boot_cpuid(dt);
+	fill_fullpaths(dti->dt, "");
+
+	return error;
+}
+
 /*
  * Tree accessor functions
  */
-- 
2.43.0

[PATCH 13/13] dtc: Merge the fragments of a dts file in main()

[Simon Glass]

We would like to run checks on each fragment of a dts file before it is
merged, but that requires dt_from_source() to return the fragments
unmerged, so that main() can decide what to do with them. At present
they are merged at the end of the parse.

Call merge_fragments() from main() instead, once the input is read. It
does nothing for dtb or fs input, which has no fragments.

When a reference cannot be resolved, dtc now reports 'Unable to merge
input tree' rather than 'Syntax error parsing input tree', since the
file was parsed successfully. Check for this message with
test_label_ref.dts, since no test checks it at present.

If the parser reports an error, dt_from_source() gives up before main()
can merge the fragments. Merge them first in that case, so that any
errors in the fragments are still reported.

Suggested-by: David Gibson <david@gibson.dropbear.id.au>
Link: https://lore.kernel.org/devicetree-compiler/auK0ERvxGCJGSc3p@gractus.seuss/
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Simon Glass <sjg@chromium.org>
---

 dtc-parser.y       | 2 --
 dtc.c              | 2 ++
 tests/run_tests.sh | 3 ++-
 treesource.c       | 5 ++++-
 4 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/dtc-parser.y b/dtc-parser.y
index c1aa4ad..f547a62 100644
--- a/dtc-parser.y
+++ b/dtc-parser.y
@@ -105,8 +105,6 @@ sourcefile:
 		{
 			parser_output = build_dt_info($1, $2, NULL, 0);
 			parser_output->fragments = $3;
-			if (merge_fragments(parser_output))
-				treesource_error = true;
 		}
 	;
 
diff --git a/dtc.c b/dtc.c
index e506837..787c724 100644
--- a/dtc.c
+++ b/dtc.c
@@ -299,6 +299,8 @@ int main(int argc, char *argv[])
 	else
 		die("Unknown input format \"%s\"\n", inform);
 
+	if (merge_fragments(dti))
+		die("Unable to merge input tree\n");
 	dti->outname = outname;
 
 	if (depfile) {
diff --git a/tests/run_tests.sh b/tests/run_tests.sh
index e0504c0..4ffd919 100755
--- a/tests/run_tests.sh
+++ b/tests/run_tests.sh
@@ -737,7 +737,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 "$SRCDIR/test_tree1_merge_path.dts"
     tree1_tests dtc_tree1_merge_path.test.dtb test_tree1.dtb
-    run_wrap_error_test $DTC -I dts -O dtb -o /dev/null "$SRCDIR/test_label_ref.dts"
+    run_wrap_error_msg_test "Unable to merge input tree" \
+	$DTC -I dts -O dtb -o /dev/null "$SRCDIR/test_label_ref.dts"
 
     run_dtc_test -I dts -O dtb -o dtc_relref_merge.test.dtb "$SRCDIR/relref_merge.dts"
     run_test relref_merge dtc_relref_merge.test.dtb
diff --git a/treesource.c b/treesource.c
index bf648bf..3fb9d83 100644
--- a/treesource.c
+++ b/treesource.c
@@ -25,8 +25,11 @@ struct dt_info *dt_from_source(const char *fname)
 	if (yyparse() != 0)
 		die("Unable to parse input tree\n");
 
-	if (treesource_error)
+	if (treesource_error) {
+		/* Report any errors in the fragments as well, before giving up */
+		merge_fragments(parser_output);
 		die("Syntax error parsing input tree\n");
+	}
 
 	return parser_output;
 }
-- 
2.43.0

Re: [PATCH 01/13] tests: Check each error when merging a dts file

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 10616 bytes --]

On Sun, Nov 01, 2026 at 09:36:55AM -0700, Simon Glass wrote:
> When the parser merges the top-level fragments of a dts file, it can
> report that a label or path is not found, or that a label-relative
> reference is not supported in a plugin. Most of these errors are not
> covered by any test, and the two tests which do reach them only check
> that dtc fails, not the message or where it is reported. The merge
> logic is about to move out of the grammar, so this needs better tests.
> 
> Add a test file for each error which is not covered, and check the
> message and its position for all of them, including the two existing
> ones. Also check that merging carries on after an error, so that later
> errors are reported too, that a reference error is still reported when
> the parser finds another error, and that a labelled reference within a
> plugin adds the label, since nothing covers these either.
> 
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Simon Glass <sjg@chromium.org>

Couple of nits, otherwise LGTM.

> ---
> 
>  tests/merge-delete-missing.dts     |  9 +++++++
>  tests/merge-errors.dts             | 14 ++++++++++
>  tests/merge-first-ref-relative.dts | 10 ++++++++
>  tests/merge-first-ref.dts          |  9 +++++++
>  tests/merge-labelled-plugin.dts    | 12 +++++++++
>  tests/merge-labelled-relative.dts  | 14 ++++++++++
>  tests/merge-omit-missing.dts       |  9 +++++++
>  tests/merge-parse-error.dts        | 17 +++++++++++++
>  tests/merge-path-ref-missing.dts   | 11 ++++++++
>  tests/merge-path-relative.dts      | 12 +++++++++
>  tests/run_tests.sh                 | 41 ++++++++++++++++++++++++++++++
>  11 files changed, 158 insertions(+)
>  create mode 100644 tests/merge-delete-missing.dts
>  create mode 100644 tests/merge-errors.dts
>  create mode 100644 tests/merge-first-ref-relative.dts
>  create mode 100644 tests/merge-first-ref.dts
>  create mode 100644 tests/merge-labelled-plugin.dts
>  create mode 100644 tests/merge-labelled-relative.dts
>  create mode 100644 tests/merge-omit-missing.dts
>  create mode 100644 tests/merge-parse-error.dts
>  create mode 100644 tests/merge-path-ref-missing.dts
>  create mode 100644 tests/merge-path-relative.dts
> 
> diff --git a/tests/merge-delete-missing.dts b/tests/merge-delete-missing.dts
> new file mode 100644
> index 0000000..b36e39e
> --- /dev/null
> +++ b/tests/merge-delete-missing.dts
> @@ -0,0 +1,9 @@
> +/dts-v1/;
> +
> +/ {
> +	label: node {
> +	};
> +};
> +
> +/* Try to delete a node using a non-existent label */
> +/delete-node/ &nosuchnode;
> diff --git a/tests/merge-errors.dts b/tests/merge-errors.dts
> new file mode 100644
> index 0000000..6d3969f
> --- /dev/null
> +++ b/tests/merge-errors.dts
> @@ -0,0 +1,14 @@
> +/dts-v1/;
> +
> +/ {
> +	label: node {
> +	};
> +};
> +
> +/* Each error is reported, not just the first */

I'm a bit confused by this comment - I only see one error in this
file.

> +&nosuchnode1 {
> +	prop;
> +};
> +
> +/delete-node/ &nosuchnode2;
> +/omit-if-no-ref/ &nosuchnode3;
> diff --git a/tests/merge-first-ref-relative.dts b/tests/merge-first-ref-relative.dts
> new file mode 100644
> index 0000000..103cd1f
> --- /dev/null
> +++ b/tests/merge-first-ref-relative.dts
> @@ -0,0 +1,10 @@
> +/dts-v1/;
> +/plugin/;
> +
> +/* A plugin cannot start with a label-relative reference */

That's slightly misleading, insn't it?  A plugin can't have _any_
label relative references unless they resolve within the plugin.

> +&{label/node} {
> +	prop;
> +};
> +
> +/* Later fragments are still merged */
> +/delete-node/ &nosuchnode;
> diff --git a/tests/merge-first-ref.dts b/tests/merge-first-ref.dts
> new file mode 100644
> index 0000000..8f23a44
> --- /dev/null
> +++ b/tests/merge-first-ref.dts
> @@ -0,0 +1,9 @@
> +/dts-v1/;
> +
> +/* Only a plugin can start with a reference */
> +&nosuchnode {
> +	prop;
> +};
> +
> +/* Later fragments are still merged */
> +/delete-node/ &nosuchnode2;
> diff --git a/tests/merge-labelled-plugin.dts b/tests/merge-labelled-plugin.dts
> new file mode 100644
> index 0000000..3849cc5
> --- /dev/null
> +++ b/tests/merge-labelled-plugin.dts
> @@ -0,0 +1,12 @@
> +/dts-v1/;
> +/plugin/;
> +
> +/ {
> +	label: node {
> +	};
> +};
> +
> +/* Add a label to a node defined in the plugin itself */
> +newlabel: &label {
> +	prop = "value";
> +};
> diff --git a/tests/merge-labelled-relative.dts b/tests/merge-labelled-relative.dts
> new file mode 100644
> index 0000000..00369e3
> --- /dev/null
> +++ b/tests/merge-labelled-relative.dts
> @@ -0,0 +1,14 @@
> +/dts-v1/;
> +/plugin/;
> +
> +/ {
> +	label: node {
> +		subnode {
> +		};
> +	};
> +};
> +
> +/* A plugin cannot use a label-relative reference, even if it is found */
> +newlabel: &{label/subnode} {
> +	prop;
> +};
> diff --git a/tests/merge-omit-missing.dts b/tests/merge-omit-missing.dts
> new file mode 100644
> index 0000000..0357648
> --- /dev/null
> +++ b/tests/merge-omit-missing.dts
> @@ -0,0 +1,9 @@
> +/dts-v1/;
> +
> +/ {
> +	label: node {
> +	};
> +};
> +
> +/* Try to mark a node using a non-existent path */
> +/omit-if-no-ref/ &{/nosuchnode};
> diff --git a/tests/merge-parse-error.dts b/tests/merge-parse-error.dts
> new file mode 100644
> index 0000000..c39318b
> --- /dev/null
> +++ b/tests/merge-parse-error.dts
> @@ -0,0 +1,17 @@
> +/dts-v1/;
> +
> +/ {
> +	label: node {
> +	};
> +};
> +
> +/* Reported even though the parser finds another error below */
> +&nosuchnode {
> +	prop;
> +};
> +
> +/ {
> +	node {
> +		prop = <(1/0)>;
> +	};
> +};
> diff --git a/tests/merge-path-ref-missing.dts b/tests/merge-path-ref-missing.dts
> new file mode 100644
> index 0000000..ad2e35e
> --- /dev/null
> +++ b/tests/merge-path-ref-missing.dts
> @@ -0,0 +1,11 @@
> +/dts-v1/;
> +
> +/ {
> +	node {
> +	};
> +};
> +
> +/* Try to redefine a node using a non-existent path */
> +&{/nosuchnode} {
> +	prop;
> +};
> diff --git a/tests/merge-path-relative.dts b/tests/merge-path-relative.dts
> new file mode 100644
> index 0000000..94a0b38
> --- /dev/null
> +++ b/tests/merge-path-relative.dts
> @@ -0,0 +1,12 @@
> +/dts-v1/;
> +/plugin/;
> +
> +/ {
> +	label: node {
> +	};
> +};
> +
> +/* A plugin cannot use a label-relative path reference */
> +&{label/node} {
> +	prop;
> +};
> diff --git a/tests/run_tests.sh b/tests/run_tests.sh
> index 5bc7817..e0504c0 100755
> --- a/tests/run_tests.sh
> +++ b/tests/run_tests.sh
> @@ -765,6 +765,47 @@ dtc_tests () {
>      run_sh_test "$SRCDIR/dtc-checkfails.sh" phandle_references -- -I dts -O dtb "$SRCDIR/nonexist-node-ref.dts"
>      run_sh_test "$SRCDIR/dtc-checkfails.sh" phandle_references -- -I dts -O dtb "$SRCDIR/nonexist-label-ref.dts"
>      run_sh_test "$SRCDIR/dtc-fatal.sh" -I dts -O dtb "$SRCDIR/nonexist-node-ref2.dts"
> +
> +    # Check each error when merging the fragments of a dts file, with its
> +    # position
> +    run_wrap_error_msg_test "test_label_ref.dts:7.8-15 Label or path handle not found" \
> +	$DTC -I dts -O dtb -o /dev/null "$SRCDIR/test_label_ref.dts"
> +    run_wrap_error_msg_test "nonexist-node-ref2.dts:9.1-12 Label or path nosuchnode not found" \
> +	$DTC -I dts -O dtb -o /dev/null "$SRCDIR/nonexist-node-ref2.dts"
> +    run_wrap_error_msg_test "merge-first-ref.dts:4.13-6.3 Label or path nosuchnode not found" \
> +	$DTC -I dts -O dtb -o /dev/null "$SRCDIR/merge-first-ref.dts"
> +    run_wrap_error_msg_test "merge-first-ref.dts:9.15-27 Label or path nosuchnode2 not found" \
> +	$DTC -I dts -O dtb -o /dev/null "$SRCDIR/merge-first-ref.dts"
> +    run_wrap_error_msg_test "merge-first-ref-relative.dts:5.15-7.3 Label-relative reference label/node not supported in plugin" \
> +	$DTC -I dts -O dtb -o /dev/null "$SRCDIR/merge-first-ref-relative.dts"
> +    run_wrap_error_msg_test "merge-first-ref-relative.dts:10.15-26 Label or path nosuchnode not found" \
> +	$DTC -I dts -O dtb -o /dev/null "$SRCDIR/merge-first-ref-relative.dts"
> +    run_wrap_error_msg_test "merge-labelled-relative.dts:12.1-10 Label-relative reference label/subnode not supported in plugin" \
> +	$DTC -I dts -O dtb -o /dev/null "$SRCDIR/merge-labelled-relative.dts"
> +    run_wrap_error_msg_test "merge-path-relative.dts:10.1-14 Label-relative reference label/node not supported in plugin" \
> +	$DTC -I dts -O dtb -o /dev/null "$SRCDIR/merge-path-relative.dts"
> +    run_wrap_error_msg_test "merge-path-ref-missing.dts:9.1-15 Label or path /nosuchnode not found" \
> +	$DTC -I dts -O dtb -o /dev/null "$SRCDIR/merge-path-ref-missing.dts"
> +    run_wrap_error_msg_test "merge-delete-missing.dts:9.15-26 Label or path nosuchnode not found" \
> +	$DTC -I dts -O dtb -o /dev/null "$SRCDIR/merge-delete-missing.dts"
> +    run_wrap_error_msg_test "merge-omit-missing.dts:9.18-32 Label or path /nosuchnode not found" \
> +	$DTC -I dts -O dtb -o /dev/null "$SRCDIR/merge-omit-missing.dts"
> +    run_wrap_error_msg_test "merge-errors.dts:9.1-13 Label or path nosuchnode1 not found" \
> +	$DTC -I dts -O dtb -o /dev/null "$SRCDIR/merge-errors.dts"
> +    run_wrap_error_msg_test "merge-errors.dts:13.15-27 Label or path nosuchnode2 not found" \
> +	$DTC -I dts -O dtb -o /dev/null "$SRCDIR/merge-errors.dts"
> +    run_wrap_error_msg_test "merge-errors.dts:14.18-30 Label or path nosuchnode3 not found" \
> +	$DTC -I dts -O dtb -o /dev/null "$SRCDIR/merge-errors.dts"
> +    run_wrap_error_msg_test "merge-parse-error.dts:9.1-12 Label or path nosuchnode not found" \
> +	$DTC -I dts -O dtb -o /dev/null "$SRCDIR/merge-parse-error.dts"
> +    run_wrap_error_msg_test "merge-parse-error.dts:15.12-15 Division by zero" \
> +	$DTC -I dts -O dtb -o /dev/null "$SRCDIR/merge-parse-error.dts"
> +
> +    # A labelled reference to a node in a plugin adds the label to it
> +    run_dtc_test -@ -I dts -O dtb -o merge-labelled-plugin.test.dtb \
> +	"$SRCDIR/merge-labelled-plugin.dts"
> +    run_fdtget_test "/node" merge-labelled-plugin.test.dtb /__symbols__ newlabel
> +    run_fdtget_test "value" merge-labelled-plugin.test.dtb /node prop
>      check_tests "$SRCDIR/bad-name-property.dts" name_properties
>  
>      check_tests "$SRCDIR/bad-ncells.dts" address_cells_is_cell size_cells_is_cell interrupts_extended_is_cell
> -- 
> 2.43.0
> 
> 

-- 
David Gibson (he or they)	| 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 02/13] dtc: Add a list for the top-level fragments of a dts file

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 6542 bytes --]

On Sun, Nov 01, 2026 at 09:36:56AM -0700, Simon Glass wrote:
> A dts file is made up of top-level fragments: the root node, later
> definitions of a node by label or path, and /delete-node/ or
> /omit-if-no-ref/ with a reference. We would like to deal with each
> fragment on its own before it is merged, for example to apply deletes
> consistently or to run some checks on each one, but that requires
> keeping the fragments apart. At present the parser merges each one into
> the tree as soon as it is parsed.

Btw, I'm extremely open to a better name than "fragment" if one occurs
to you.  I believe I originally thought of these as "overlays" but
that's confusing since runtime overlays were invented.

> Add a struct fragment to hold one of these, functions to build one, to
> append it to a list and to free it, and a list of them in struct dt_info
> 
> These are not used yet.
> 
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
>  dtc.h      | 47 ++++++++++++++++++++++++++++++++++++++++++
>  livetree.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 107 insertions(+)
> 
> diff --git a/dtc.h b/dtc.h
> index 473552e..c0d8e8f 100644
> --- a/dtc.h
> +++ b/dtc.h
> @@ -322,12 +322,59 @@ struct reserve_info *chain_reserve_entry(struct reserve_info *first,
>  struct reserve_info *add_reserve_entry(struct reserve_info *list,
>  				       struct reserve_info *new);
>  
> +/*
> + * A fragment is one top-level part of a dts file, after the header and any
> + * memory reservations: the root node, a later definition of a node which
> + * refers to it by label or path, or a /delete-node/ or /omit-if-no-ref/ with
> + * a reference. Fragments are kept apart until they are merged into a single
> + * tree.
> + *
> + * This is not the same as an overlay fragment (a fragment@N node), although
> + * in a plugin a fragment may become one when it is merged.
> + */
> +
> +enum fragment_type {
> +	FRAGMENT_ROOT,		/* / { ... }; */

Why do we need a special type for ROOT, rather than setting ref == "/"?

> +	FRAGMENT_REF,		/* [label:] &ref { ... }; */
> +	FRAGMENT_DELETE_NODE,	/* /delete-node/ &ref; */
> +	FRAGMENT_OMIT_IF_NO_REF, /* /omit-if-no-ref/ &ref; */
> +};
> +
> +/*
> + * Which fields are used depends on the type:
> + *
> + *   FRAGMENT_ROOT            node
> + *   FRAGMENT_REF             ref, path_ref, ref_srcpos and node, plus label
> + *                            and label_srcpos if there is a label
> + *   FRAGMENT_DELETE_NODE     ref, path_ref and ref_srcpos
> + *   FRAGMENT_OMIT_IF_NO_REF  ref, path_ref and ref_srcpos
> + *
> + * Unused fields are NULL or false.
> + */
> +struct fragment {
> +	enum fragment_type type;
> +	char *ref;			/* label or path referred to */
> +	bool path_ref;			/* ref is a path, i.e. &{...} */

This can be derived from ref, so I don't think we need to store it.

> +	struct srcpos *ref_srcpos;	/* position of ref */
> +	char *label;			/* label to add to the target */
> +	struct srcpos *label_srcpos;	/* position of label */
> +	struct node *node;		/* the node definition */
> +
> +	struct fragment *next;		/* NULL at the end of the list */
> +};
> +
> +struct fragment *build_fragment(enum fragment_type type, struct node *node);
> +struct fragment *build_fragment_ref(char *ref, bool path_ref,
> +				    struct srcpos *srcpos);
> +struct fragment *append_fragment(struct fragment *list, struct fragment *new);
> +void free_fragment(struct fragment *f);
>  
>  struct dt_info {
>  	unsigned int dtsflags;
>  	struct reserve_info *reservelist;
>  	uint32_t boot_cpuid_phys;
>  	struct node *dt;		/* the device tree */
> +	struct fragment *fragments;	/* not yet merged into dt */
>  	const char *outname;		/* filename being written to, "-" for stdout */
>  };
>  
> diff --git a/livetree.c b/livetree.c
> index abe73de..982f0a6 100644
> --- a/livetree.c
> +++ b/livetree.c
> @@ -448,6 +448,65 @@ struct reserve_info *add_reserve_entry(struct reserve_info *list,
>  	return list;
>  }
>  
> +/* Create a fragment of the given type, holding an optional node */
> +struct fragment *build_fragment(enum fragment_type type, struct node *node)

Since there's no ref provided, presumably this must of be
FRAGMENT_ROOT, so why a type parameter?

> +{
> +	struct fragment *new = xmalloc(sizeof(*new));
> +
> +	memset(new, '\0', sizeof(*new));
> +
> +	new->type = type;
> +	new->node = node;
> +
> +	return new;
> +}
> +
> +/*
> + * Create a fragment which refers to a label, or to a path if path_ref. Its
> + * node is NULL, so the caller must either set the node or change the type to
> + * FRAGMENT_DELETE_NODE or FRAGMENT_OMIT_IF_NO_REF

That seems a clumsy interface.  Maybe different constructor functions
for the different types, that take all the necessary pieces?

> + */
> +struct fragment *build_fragment_ref(char *ref, bool path_ref,
> +				    struct srcpos *srcpos)

Here too, path_ref should be derivable from ref.

> +{
> +	struct fragment *new = build_fragment(FRAGMENT_REF, NULL);
> +
> +	new->ref = ref;
> +	new->path_ref = path_ref;
> +	new->ref_srcpos = srcpos_copy(srcpos);
> +
> +	return new;
> +}
> +
> +/* Add a fragment to the end of a list, returning the new head */
> +struct fragment *append_fragment(struct fragment *list, struct fragment *new)
> +{
> +	struct fragment *last;
> +
> +	new->next = NULL;
> +
> +	if (!list)
> +		return new;
> +
> +	for (last = list; last->next; last = last->next)
> +		;
> +
> +	last->next = new;
> +
> +	return list;
> +}
> +
> +/*
> + * Free a fragment, but not its node, reference or label, since the tree may
> + * still use these
> + */
> +void free_fragment(struct fragment *f)
> +{
> +	srcpos_free(f->ref_srcpos);
> +	srcpos_free(f->label_srcpos);
> +	free(f);
> +}
> +
>  static void fill_fullpaths(struct node *tree, const char *prefix)
>  {
>  	struct node *child;
> @@ -475,6 +534,7 @@ struct dt_info *build_dt_info(unsigned int dtsflags,
>  	dti->dtsflags = dtsflags;
>  	dti->reservelist = reservelist;
>  	dti->dt = tree;
> +	dti->fragments = NULL;
>  	dti->boot_cpuid_phys = boot_cpuid_phys;
>  
>  	fill_fullpaths(dti->dt, "");
> -- 
> 2.43.0
> 
> 

-- 
David Gibson (he or they)	| 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 03/13] dtc: Move the /delete-node/ handling out of the grammar

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 4032 bytes --]

On Sun, Nov 01, 2026 at 09:36:57AM -0700, Simon Glass wrote:
> The grammar actions merge each top-level fragment of a dts file into the
> tree as it is parsed, so the merge logic is spread over the rules for
> each kind of fragment. This makes it hard to follow and to change.
> 
> As a first step towards moving this logic into livetree.c, move the
> action for a top-level /delete-node/ into a new merge_delete_node(),
> passing it a struct fragment for the reference. Add a fragment_ref rule
> to build this.
> 
> The output does not change.
> 
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
>  dtc-parser.y | 26 +++++++++++++++++---------
>  dtc.h        |  1 +
>  livetree.c   | 19 +++++++++++++++++++
>  3 files changed, 37 insertions(+), 9 deletions(-)
> 
> diff --git a/dtc-parser.y b/dtc-parser.y
> index e47f80d..61301a6 100644
> --- a/dtc-parser.y
> +++ b/dtc-parser.y
> @@ -47,6 +47,7 @@ static bool is_ref_relative(const char *ref)
>  	struct node *node;
>  	struct node *nodelist;
>  	struct reserve_info *re;
> +	struct fragment *fragment;
>  	uint64_t integer;
>  	unsigned int flags;
>  }
> @@ -80,6 +81,7 @@ static bool is_ref_relative(const char *ref)
>  %type <prop> propdef
>  %type <proplist> proplist
>  %type <labelref> dt_ref
> +%type <fragment> fragment_ref
>  
>  %type <node> devicetree
>  %type <node> nodedef
> @@ -157,6 +159,17 @@ memreserve:
>  
>  dt_ref: DT_LABEL_REF | DT_PATH_REF;
>  
> +fragment_ref:
> +	  DT_LABEL_REF
> +		{
> +			$$ = build_fragment_ref($1, false, &@$);
> +		}
> +	| DT_PATH_REF
> +		{
> +			$$ = build_fragment_ref($1, true, &@$);
> +		}
> +	;
> +

This seems weird.  If the point is to allow either kind of ref, we
already have the dt_ref nonterminal for that.  If the point is to
build fragments, rather than just refs, then why do it here before you
have the information you need to properly build the fragment?

>  devicetree:
>  	  '/' nodedef
>  		{
> @@ -236,16 +249,11 @@ devicetree:
>  			}
>  			$$ = $1;
>  		}
> -	| devicetree DT_DEL_NODE dt_ref ';'
> +	| devicetree DT_DEL_NODE fragment_ref ';'
>  		{
> -			struct node *target = get_node_by_ref($1, $3);
> -
> -			if (target)
> -				delete_node(target);
> -			else
> -				ERROR(&@3, "Label or path %s not found", $3);
> -
> -
> +			$3->type = FRAGMENT_DELETE_NODE;
> +			if (merge_delete_node($1, $3))
> +				treesource_error = true;
>  			$$ = $1;
>  		}
>  	| devicetree DT_OMIT_NO_REF dt_ref ';'
> diff --git a/dtc.h b/dtc.h
> index c0d8e8f..9f9076d 100644
> --- a/dtc.h
> +++ b/dtc.h
> @@ -368,6 +368,7 @@ struct fragment *build_fragment_ref(char *ref, bool path_ref,
>  				    struct srcpos *srcpos);
>  struct fragment *append_fragment(struct fragment *list, struct fragment *new);
>  void free_fragment(struct fragment *f);
> +bool merge_delete_node(struct node *dt, struct fragment *f);
>  
>  struct dt_info {
>  	unsigned int dtsflags;
> diff --git a/livetree.c b/livetree.c
> index 982f0a6..5c9ec48 100644
> --- a/livetree.c
> +++ b/livetree.c
> @@ -542,6 +542,25 @@ struct dt_info *build_dt_info(unsigned int dtsflags,
>  	return dti;
>  }
>  
> +/*
> + * Delete the node which a /delete-node/ fragment refers to
> + *
> + * Returns true if there is an error, which has been reported.
> + */
> +bool merge_delete_node(struct node *dt, struct fragment *f)
> +{
> +	struct node *target = get_node_by_ref(dt, f->ref);

assert() that f->type == DELETE_NODE, maybe?

> +	if (!target) {
> +		srcpos_error(f->ref_srcpos, "Error",
> +			     "Label or path %s not found", f->ref);
> +		return true;
> +	}
> +	delete_node(target);
> +
> +	return false;
> +}
> +
>  /*
>   * Tree accessor functions
>   */
> -- 
> 2.43.0
> 
> 

-- 
David Gibson (he or they)	| 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 04/13] dtc: Move the /omit-if-no-ref/ handling out of the grammar

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 2837 bytes --]

On Sun, Nov 01, 2026 at 09:36:58AM -0700, Simon Glass wrote:
> The grammar action for a top-level /omit-if-no-ref/ still holds its
> merge logic.
> 
> Move it into a new merge_omit_if_no_ref(), passing it a struct fragment
> for the reference.
> 
> The output does not change.
> 
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
>  dtc-parser.y | 13 ++++---------
>  dtc.h        |  1 +
>  livetree.c   | 20 ++++++++++++++++++++
>  3 files changed, 25 insertions(+), 9 deletions(-)
> 
> diff --git a/dtc-parser.y b/dtc-parser.y
> index 61301a6..089ab55 100644
> --- a/dtc-parser.y
> +++ b/dtc-parser.y
> @@ -256,16 +256,11 @@ devicetree:
>  				treesource_error = true;
>  			$$ = $1;
>  		}
> -	| devicetree DT_OMIT_NO_REF dt_ref ';'
> +	| devicetree DT_OMIT_NO_REF fragment_ref ';'

As in the previous patch, the fragment_ref nonterminal doesn't make
much sense to me.

>  		{
> -			struct node *target = get_node_by_ref($1, $3);
> -
> -			if (target)
> -				omit_node_if_unused(target);
> -			else
> -				ERROR(&@3, "Label or path %s not found", $3);
> -
> -
> +			$3->type = FRAGMENT_OMIT_IF_NO_REF;
> +			if (merge_omit_if_no_ref($1, $3))
> +				treesource_error = true;
>  			$$ = $1;
>  		}
>  	;
> diff --git a/dtc.h b/dtc.h
> index 9f9076d..e747dc0 100644
> --- a/dtc.h
> +++ b/dtc.h
> @@ -369,6 +369,7 @@ struct fragment *build_fragment_ref(char *ref, bool path_ref,
>  struct fragment *append_fragment(struct fragment *list, struct fragment *new);
>  void free_fragment(struct fragment *f);
>  bool merge_delete_node(struct node *dt, struct fragment *f);
> +bool merge_omit_if_no_ref(struct node *dt, struct fragment *f);
>  
>  struct dt_info {
>  	unsigned int dtsflags;
> diff --git a/livetree.c b/livetree.c
> index 5c9ec48..5ed2085 100644
> --- a/livetree.c
> +++ b/livetree.c
> @@ -561,6 +561,26 @@ bool merge_delete_node(struct node *dt, struct fragment *f)
>  	return false;
>  }
>  
> +/*
> + * Mark the node which an /omit-if-no-ref/ fragment refers to, so that it is
> + * dropped if nothing refers to it
> + *
> + * Returns true if there is an error, which has been reported.
> + */
> +bool merge_omit_if_no_ref(struct node *dt, struct fragment *f)
> +{
> +	struct node *target = get_node_by_ref(dt, f->ref);
> +
> +	if (!target) {
> +		srcpos_error(f->ref_srcpos, "Error",
> +			     "Label or path %s not found", f->ref);
> +		return true;
> +	}
> +	omit_node_if_unused(target);
> +
> +	return false;
> +}
> +
>  /*
>   * Tree accessor functions
>   */
> -- 
> 2.43.0
> 
> 

-- 
David Gibson (he or they)	| 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 05/13] dtc: Move is_ref_relative() to livetree.c

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 2245 bytes --]

On Sun, Nov 01, 2026 at 09:36:59AM -0700, Simon Glass wrote:
61;8401;1c> The remaining grammar actions which merge a fragment use
> is_ref_relative() to reject a label-relative reference in a plugin.
> 
> Move it to livetree.c, so that it is available when these actions move
> there too, and add a comment.
> 
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Simon Glass <sjg@chromium.org>

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

> ---
> 
>  dtc-parser.y | 5 -----
>  dtc.h        | 1 +
>  livetree.c   | 6 ++++++
>  3 files changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/dtc-parser.y b/dtc-parser.y
> index 089ab55..7545895 100644
> --- a/dtc-parser.y
> +++ b/dtc-parser.y
> @@ -24,11 +24,6 @@ extern void yyerror(char const *s);
>  extern struct dt_info *parser_output;
>  extern bool treesource_error;
>  
> -static bool is_ref_relative(const char *ref)
> -{
> -	return ref[0] != '/' && strchr(&ref[1], '/');
> -}
> -
>  %}
>  
>  %union {
> diff --git a/dtc.h b/dtc.h
> index e747dc0..d145b03 100644
> --- a/dtc.h
> +++ b/dtc.h
> @@ -368,6 +368,7 @@ struct fragment *build_fragment_ref(char *ref, bool path_ref,
>  				    struct srcpos *srcpos);
>  struct fragment *append_fragment(struct fragment *list, struct fragment *new);
>  void free_fragment(struct fragment *f);
> +bool is_ref_relative(const char *ref);
>  bool merge_delete_node(struct node *dt, struct fragment *f);
>  bool merge_omit_if_no_ref(struct node *dt, struct fragment *f);
>  
> diff --git a/livetree.c b/livetree.c
> index 5ed2085..027682a 100644
> --- a/livetree.c
> +++ b/livetree.c
> @@ -542,6 +542,12 @@ struct dt_info *build_dt_info(unsigned int dtsflags,
>  	return dti;
>  }
>  
> +/* Check whether a reference is relative to a label, e.g. &{label/node} */
> +bool is_ref_relative(const char *ref)
> +{
> +	return ref[0] != '/' && strchr(&ref[1], '/');
> +}
> +
>  /*
>   * Delete the node which a /delete-node/ fragment refers to
>   *
> -- 
> 2.43.0
> 
> 

-- 
David Gibson (he or they)	| 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 06/13] dtc: Move label-reference handling out of the grammar

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 4022 bytes --]

On Sun, Nov 01, 2026 at 09:37:00AM -0700, Simon Glass wrote:
> The grammar action for a top-level &label { ... } holds its merge logic,
> including making an overlay fragment in a plugin when the label is not
> found.
> 
> Move it into a new merge_label_ref(), passing it a struct fragment for
> the reference and node. The rule cannot use fragment_ref yet, since a
> path reference still has its own rule, so it builds the fragment itself.
> 
> The output does not change.
> 
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
>  dtc-parser.y | 23 +++++++++--------------
>  dtc.h        |  1 +
>  livetree.c   | 25 +++++++++++++++++++++++++
>  3 files changed, 35 insertions(+), 14 deletions(-)
> 
> diff --git a/dtc-parser.y b/dtc-parser.y
> index 7545895..1a5ad12 100644
> --- a/dtc-parser.y
> +++ b/dtc-parser.y
> @@ -227,21 +227,16 @@ devicetree:
>  		}
>  	| devicetree DT_LABEL_REF nodedef
>  		{
> -			struct node *target = get_node_by_ref($1, $2);
> +			struct fragment *f = build_fragment_ref($2, false, &@2);

As noted previously, having this build *some* of the fragment, but
then requiring it to be completed...

> -			if (target) {
> -				merge_nodes(target, $3);
> -			} else {
> -				/*
> -				 * We rely on the rule being always:
> -				 *   versioninfo plugindecl memreserves devicetree
> -				 * so $-1 is what we want (plugindecl)
> -				 */
> -				if ($<flags>-1 & DTSF_PLUGIN)
> -					add_orphan_node($1, $3, $2);
> -				else
> -					ERROR(&@2, "Label or path %s not found", $2);
> -			}
> +			f->node = $3;

...here is pretty ugly.

> +			/*
> +			 * We rely on the rule being always:
> +			 *   versioninfo plugindecl memreserves devicetree
> +			 * so $-1 is what we want (plugindecl)
> +			 */
> +			if (merge_label_ref($1, f, $<flags>-1 & DTSF_PLUGIN))
> +				treesource_error = true;
>  			$$ = $1;
>  		}
>  	| devicetree DT_DEL_NODE fragment_ref ';'
> diff --git a/dtc.h b/dtc.h
> index d145b03..256017f 100644
> --- a/dtc.h
> +++ b/dtc.h
> @@ -369,6 +369,7 @@ struct fragment *build_fragment_ref(char *ref, bool path_ref,
>  struct fragment *append_fragment(struct fragment *list, struct fragment *new);
>  void free_fragment(struct fragment *f);
>  bool is_ref_relative(const char *ref);
> +bool merge_label_ref(struct node *dt, struct fragment *f, bool plugin);
>  bool merge_delete_node(struct node *dt, struct fragment *f);
>  bool merge_omit_if_no_ref(struct node *dt, struct fragment *f);
>  
> diff --git a/livetree.c b/livetree.c
> index 027682a..2a01d49 100644
> --- a/livetree.c
> +++ b/livetree.c
> @@ -548,6 +548,31 @@ bool is_ref_relative(const char *ref)
>  	return ref[0] != '/' && strchr(&ref[1], '/');
>  }
>  
> +/*
> + * Merge a fragment which refers to a label into the node with that label. In
> + * a plugin, a label which is not found becomes an overlay fragment.
> + *
> + * Returns true if there is an error, which has been reported.

I generally prefer bool-typed functions to return true on success,
false on error, whereas for int-typed functions 0 for success, -ve for
failure is the convention.

> + */
> +bool merge_label_ref(struct node *dt, struct fragment *f, bool plugin)
> +{
> +	struct node *target = get_node_by_ref(dt, f->ref);
> +
> +	if (target) {
> +		merge_nodes(target, f->node);
> +		return false;
> +	}
> +	if (!plugin) {
> +		srcpos_error(f->ref_srcpos, "Error",
> +			     "Label or path %s not found", f->ref);
> +		return true;
> +	}
> +	/* The label may be in the base tree, so add a fragment@N node for it */
> +	add_orphan_node(dt, f->node, f->ref);
> +
> +	return false;
> +}
> +
>  /*
>   * Delete the node which a /delete-node/ fragment refers to
>   *
> -- 
> 2.43.0
> 
> 

-- 
David Gibson (he or they)	| 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 07/13] dtc: Move path-reference handling out of the grammar

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 4194 bytes --]

On Sun, Nov 01, 2026 at 09:37:01AM -0700, Simon Glass wrote:
> The grammar action for a top-level &{/path} { ... } holds its merge
> logic, including making an overlay fragment in a plugin.
> 
> Move it into a new merge_path_ref(), passing it a struct fragment for
> the reference and node, built in the same way as for a label reference.
> 
> The output does not change.
> 
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
>  dtc-parser.y | 17 +++++------------
>  dtc.h        |  1 +
>  livetree.c   | 36 ++++++++++++++++++++++++++++++++++++
>  3 files changed, 42 insertions(+), 12 deletions(-)
> 
> diff --git a/dtc-parser.y b/dtc-parser.y
> index 1a5ad12..cca3ce1 100644
> --- a/dtc-parser.y
> +++ b/dtc-parser.y
> @@ -206,23 +206,16 @@ devicetree:
>  		}
>  	| devicetree DT_PATH_REF nodedef
>  		{
> +			struct fragment *f = build_fragment_ref($2, true, &@2);
> +
> +			f->node = $3;
>  			/*
>  			 * We rely on the rule being always:
>  			 *   versioninfo plugindecl memreserves devicetree
>  			 * so $-1 is what we want (plugindecl)
>  			 */
> -			if ($<flags>-1 & DTSF_PLUGIN) {
> -				if (is_ref_relative($2))
> -					ERROR(&@2, "Label-relative reference %s not supported in plugin", $2);
> -				add_orphan_node($1, $3, $2);
> -			} else {
> -				struct node *target = get_node_by_ref($1, $2);
> -
> -				if (target)
> -					merge_nodes(target, $3);
> -				else
> -					ERROR(&@2, "Label or path %s not found", $2);
> -			}
> +			if (merge_path_ref($1, f, $<flags>-1 & DTSF_PLUGIN))
> +				treesource_error = true;
>  			$$ = $1;
>  		}
>  	| devicetree DT_LABEL_REF nodedef
> diff --git a/dtc.h b/dtc.h
> index 256017f..5af1d1f 100644
> --- a/dtc.h
> +++ b/dtc.h
> @@ -369,6 +369,7 @@ struct fragment *build_fragment_ref(char *ref, bool path_ref,
>  struct fragment *append_fragment(struct fragment *list, struct fragment *new);
>  void free_fragment(struct fragment *f);
>  bool is_ref_relative(const char *ref);
> +bool merge_path_ref(struct node *dt, struct fragment *f, bool plugin);
>  bool merge_label_ref(struct node *dt, struct fragment *f, bool plugin);
>  bool merge_delete_node(struct node *dt, struct fragment *f);
>  bool merge_omit_if_no_ref(struct node *dt, struct fragment *f);
> diff --git a/livetree.c b/livetree.c
> index 2a01d49..b0362e4 100644
> --- a/livetree.c
> +++ b/livetree.c
> @@ -548,6 +548,42 @@ bool is_ref_relative(const char *ref)
>  	return ref[0] != '/' && strchr(&ref[1], '/');
>  }
>  
> +/*
> + * Merge a fragment which refers to a path into the node at that path. In a
> + * plugin, the fragment becomes an overlay fragment instead.
> + *
> + * Returns true if there is an error, which has been reported.
> + */
> +bool merge_path_ref(struct node *dt, struct fragment *f, bool plugin)

It would make more sense to me for merge_path_ref() and
merge_label_ref() to be combined, working out which they need to do
from the contents of the ref itself.

I suspect that will also allow the grammar rules to be combined for
the two cases.

> +{
> +	struct node *target;
> +
> +	if (plugin) {
> +		bool error = false;
> +
> +		if (is_ref_relative(f->ref)) {
> +			srcpos_error(f->ref_srcpos, "Error",
> +				     "Label-relative reference %s not supported in plugin",
> +				     f->ref);
> +			error = true;
> +		}
> +		/* Add a fragment@N node which applies the node at the path */
> +		add_orphan_node(dt, f->node, f->ref);
> +
> +		return error;
> +	}
> +
> +	target = get_node_by_ref(dt, f->ref);
> +	if (!target) {
> +		srcpos_error(f->ref_srcpos, "Error",
> +			     "Label or path %s not found", f->ref);
> +		return true;
> +	}
> +	merge_nodes(target, f->node);
> +
> +	return false;
> +}
> +
>  /*
>   * Merge a fragment which refers to a label into the node with that label. In
>   * a plugin, a label which is not found becomes an overlay fragment.
> -- 
> 2.43.0
> 
> 

-- 
David Gibson (he or they)	| 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 08/13] dtc: Move labelled-reference handling out of the grammar

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 3901 bytes --]

On Sun, Nov 01, 2026 at 09:37:02AM -0700, Simon Glass wrote:
> The grammar action for a top-level label: &ref { ... } holds its merge
> logic, which adds the label to the node referred to.
> 
> Move it into a new merge_labelled_ref(), passing it a struct fragment
> from fragment_ref, with the label and node added.
> 
> The output does not change.
> 
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
>  dtc-parser.y | 18 +++++++-----------
>  dtc.h        |  2 ++
>  livetree.c   | 28 ++++++++++++++++++++++++++++
>  3 files changed, 37 insertions(+), 11 deletions(-)
> 
> diff --git a/dtc-parser.y b/dtc-parser.y
> index cca3ce1..0e6c4c2 100644
> --- a/dtc-parser.y
> +++ b/dtc-parser.y
> @@ -190,18 +190,14 @@ devicetree:
>  						  ""),
>  					$2, $1);
>  		}
> -	| devicetree DT_LABEL dt_ref nodedef
> +	| devicetree DT_LABEL fragment_ref nodedef

Usual comments on fragment_ref.

>  		{
> -			struct node *target = get_node_by_ref($1, $3);
> -
> -			if (($<flags>-1 & DTSF_PLUGIN) && is_ref_relative($3))
> -				ERROR(&@2, "Label-relative reference %s not supported in plugin", $3);
> -
> -			if (target) {
> -				add_label(&target->labels, $2);
> -				merge_nodes(target, $4);
> -			} else
> -				ERROR(&@3, "Label or path %s not found", $3);
> +			$3->label = $2;
> +			$3->label_srcpos = srcpos_copy(&@2);
> +			$3->node = $4;
> +			if (merge_labelled_ref($1, $3,
> +					       $<flags>-1 & DTSF_PLUGIN))
> +				treesource_error = true;
>  			$$ = $1;
>  		}
>  	| devicetree DT_PATH_REF nodedef
> diff --git a/dtc.h b/dtc.h
> index 5af1d1f..75b1c0f 100644
> --- a/dtc.h
> +++ b/dtc.h
> @@ -369,6 +369,8 @@ struct fragment *build_fragment_ref(char *ref, bool path_ref,
>  struct fragment *append_fragment(struct fragment *list, struct fragment *new);
>  void free_fragment(struct fragment *f);
>  bool is_ref_relative(const char *ref);
> +bool merge_labelled_ref(struct node *dt, struct fragment *f,
> +			bool plugin);
>  bool merge_path_ref(struct node *dt, struct fragment *f, bool plugin);
>  bool merge_label_ref(struct node *dt, struct fragment *f, bool plugin);
>  bool merge_delete_node(struct node *dt, struct fragment *f);
> diff --git a/livetree.c b/livetree.c
> index b0362e4..76474cd 100644
> --- a/livetree.c
> +++ b/livetree.c
> @@ -548,6 +548,34 @@ bool is_ref_relative(const char *ref)
>  	return ref[0] != '/' && strchr(&ref[1], '/');
>  }
>  
> +/*
> + * Merge a fragment of the form 'label: &ref { ... }' into the node which ref
> + * refers to, adding the label to that node
> + *
> + * Returns true if there is an error, which has been reported.
> + */
> +bool merge_labelled_ref(struct node *dt, struct fragment *f, bool plugin)

This again seems like the fragment itself contains the information to
do the right thing for path/label or labelled fragments, rather than
having separate functions for each case.

> +{
> +	struct node *target = get_node_by_ref(dt, f->ref);
> +	bool error = false;
> +
> +	if (plugin && is_ref_relative(f->ref)) {
> +		srcpos_error(f->label_srcpos, "Error",
> +			     "Label-relative reference %s not supported in plugin",
> +			     f->ref);
> +		error = true;
> +	}
> +	if (!target) {
> +		srcpos_error(f->ref_srcpos, "Error",
> +			     "Label or path %s not found", f->ref);
> +		return true;
> +	}
> +	add_label(&target->labels, f->label);
> +	merge_nodes(target, f->node);
> +
> +	return error;
> +}
> +
>  /*
>   * Merge a fragment which refers to a path into the node at that path. In a
>   * plugin, the fragment becomes an overlay fragment instead.
> -- 
> 2.43.0
> 
> 

-- 
David Gibson (he or they)	| 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 02/13] dtc: Add a list for the top-level fragments of a dts file

[Herve Codina]

On Sun,  1 Nov 2026 09:36:56 -0700
Simon Glass <sjg@chromium.org> wrote:

> A dts file is made up of top-level fragments: the root node, later
> definitions of a node by label or path, and /delete-node/ or
> /omit-if-no-ref/ with a reference. We would like to deal with each
> fragment on its own before it is merged, for example to apply deletes
> consistently or to run some checks on each one, but that requires
> keeping the fragments apart. At present the parser merges each one into
> the tree as soon as it is parsed.
> 
> Add a struct fragment to hold one of these, functions to build one, to
> append it to a list and to free it, and a list of them in struct dt_info
> 
> These are not used yet.
> 
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
>  dtc.h      | 47 ++++++++++++++++++++++++++++++++++++++++++
>  livetree.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 107 insertions(+)
> 
> diff --git a/dtc.h b/dtc.h
> index 473552e..c0d8e8f 100644
> --- a/dtc.h
> +++ b/dtc.h
> @@ -322,12 +322,59 @@ struct reserve_info *chain_reserve_entry(struct reserve_info *first,
>  struct reserve_info *add_reserve_entry(struct reserve_info *list,
>  				       struct reserve_info *new);
>  
> +/*
> + * A fragment is one top-level part of a dts file, after the header and any
> + * memory reservations: the root node, a later definition of a node which
> + * refers to it by label or path, or a /delete-node/ or /omit-if-no-ref/ with
> + * a reference. Fragments are kept apart until they are merged into a single
> + * tree.
> + *
> + * This is not the same as an overlay fragment (a fragment@N node), although
> + * in a plugin a fragment may become one when it is merged.
> + */
> +
> +enum fragment_type {
> +	FRAGMENT_ROOT,		/* / { ... }; */
> +	FRAGMENT_REF,		/* [label:] &ref { ... }; */
> +	FRAGMENT_DELETE_NODE,	/* /delete-node/ &ref; */
> +	FRAGMENT_OMIT_IF_NO_REF, /* /omit-if-no-ref/ &ref; */
> +};
> +
> +/*
> + * Which fields are used depends on the type:
> + *
> + *   FRAGMENT_ROOT            node
> + *   FRAGMENT_REF             ref, path_ref, ref_srcpos and node, plus label
> + *                            and label_srcpos if there is a label
> + *   FRAGMENT_DELETE_NODE     ref, path_ref and ref_srcpos
> + *   FRAGMENT_OMIT_IF_NO_REF  ref, path_ref and ref_srcpos
> + *
> + * Unused fields are NULL or false.
> + */
> +struct fragment {
> +	enum fragment_type type;
> +	char *ref;			/* label or path referred to */
> +	bool path_ref;			/* ref is a path, i.e. &{...} */
> +	struct srcpos *ref_srcpos;	/* position of ref */
> +	char *label;			/* label to add to the target */
> +	struct srcpos *label_srcpos;	/* position of label */
> +	struct node *node;		/* the node definition */
> +
> +	struct fragment *next;		/* NULL at the end of the list */
> +};
> +
> +struct fragment *build_fragment(enum fragment_type type, struct node *node);
> +struct fragment *build_fragment_ref(char *ref, bool path_ref,
> +				    struct srcpos *srcpos);
> +struct fragment *append_fragment(struct fragment *list, struct fragment *new);
> +void free_fragment(struct fragment *f);
>  
>  struct dt_info {
>  	unsigned int dtsflags;
>  	struct reserve_info *reservelist;
>  	uint32_t boot_cpuid_phys;
>  	struct node *dt;		/* the device tree */
> +	struct fragment *fragments;	/* not yet merged into dt */

Why to we need the fragment list in struct dt_info ?

This list is a temporary list built_dt_info() could have the list as parameter and
handle it leading to the struct node *dt fully updated.

Other function using the struct dt_info should never see the temporary fragment 
list and so, IMHO, there is no reason to store it in the struct dt_info.

Best regards,
Hervé

Re: [PATCH 12/13] dtc: Merge the fragments of a dts file after parsing

[Herve Codina]

On Sun,  1 Nov 2026 09:37:06 -0700
Simon Glass <sjg@chromium.org> wrote:

> We would like to deal with each top-level fragment of a dts file before
> it is merged into the tree, but that requires the fragments to be kept
> apart until the whole file is parsed. At present the parser merges each
> one as soon as it is parsed. The grammar actions also need to know
> whether the file is a plugin, which they find using $<flags>-1
> 
> Have the grammar build a list of the fragments instead, in source order,
> and merge them with a new merge_fragments() once the whole file is
> parsed. This calls merge_fragment() for each one, so the output does not
> change, and frees each fragment once it is merged.
> 
> The $<flags>-1 term is no longer needed. Within a devicetree action it
> reaches back up the parser stack to the value of headers, i.e. the flags
> from /dts-v1/ and /plugin/. The sourcefile action passes the same value
> to build_dt_info(), which stores it in dti->dtsflags, so
> merge_fragments() can use that instead.
> 
> There are two differences in the errors. A syntax error later in the
> file now stops dtc before any earlier reference errors are reported, and
> reference errors now come after any other errors which the parser
> reports, such as a division by zero, rather than in source order.
> 
> dtc gives the same output and messages as before for all 4162 .dts and
> .dtso files in Linux v7.3-12284-g3799eeb906c9, and for the dts files in
> tests/
> 
> Suggested-by: David Gibson <david@gibson.dropbear.id.au>
> Link: https://lore.kernel.org/devicetree-compiler/auK0ERvxGCJGSc3p@gractus.seuss/
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
>  dtc-parser.y | 43 +++++++++++++------------------------------
>  dtc.h        |  2 +-
>  livetree.c   | 36 ++++++++++++++++++++++++++++++++++--
>  3 files changed, 48 insertions(+), 33 deletions(-)
> 
> diff --git a/dtc-parser.y b/dtc-parser.y
> index 1ed9731..c1aa4ad 100644
> --- a/dtc-parser.y
> +++ b/dtc-parser.y
> @@ -24,22 +24,6 @@ extern void yyerror(char const *s);
>  extern struct dt_info *parser_output;
>  extern bool treesource_error;
>  
> -/*
> - * Merge a fragment into the tree as soon as it is parsed. The flags come
> - * from the actions using $<flags>-1, which relies on the rule being always:
> - *   headers memreserves devicetree
> - * so $-1 is the value of headers
> - */
> -static struct node *merge_parsed_fragment(struct node *dt, struct fragment *f,
> -					  unsigned int flags)
> -{
> -	if (merge_fragment(&dt, f, flags & DTSF_PLUGIN))
> -		treesource_error = true;
> -	free_fragment(f);
> -
> -	return dt;
> -}
> -
>  %}
>  
>  %union {
> @@ -94,7 +78,7 @@ static struct node *merge_parsed_fragment(struct node *dt, struct fragment *f,
>  %type <labelref> dt_ref
>  %type <fragment> fragment_ref
>  
> -%type <node> devicetree
> +%type <fragment> devicetree
>  %type <node> nodedef
>  %type <node> subnode
>  %type <nodelist> subnodes
> @@ -119,8 +103,10 @@ static struct node *merge_parsed_fragment(struct node *dt, struct fragment *f,
>  sourcefile:
>  	  headers memreserves devicetree
>  		{
> -			parser_output = build_dt_info($1, $2, $3,
> -			                              guess_boot_cpuid($3));
> +			parser_output = build_dt_info($1, $2, NULL, 0);
> +			parser_output->fragments = $3;
> +			if (merge_fragments(parser_output))
> +				treesource_error = true;

Why not just pass the fragment list to build_dt_info() instead of using the 
parser_output->fragments and later in merge_fragments() setting this same field
to NULL once the merge is done?

IHMO, the struct dt_info doesn't need the fragments fiels (already mentionned
in patch 2).

If build_dt_info() has too much parameters, a struct dt_info_def could be
created to group all those parameters in one structure.

>  		}
>  	;
>  
> @@ -184,42 +170,39 @@ fragment_ref:
>  devicetree:
>  	  '/' nodedef
>  		{
> -			struct fragment *f = build_fragment(FRAGMENT_ROOT, $2);
> -
> -			$$ = merge_parsed_fragment(NULL, f, $<flags>-1);
> +			$$ = build_fragment(FRAGMENT_ROOT, $2);
>  		}
>  	| devicetree '/' nodedef
>  		{
> -			struct fragment *f = build_fragment(FRAGMENT_ROOT, $3);
> -
> -			$$ = merge_parsed_fragment($1, f, $<flags>-1);
> +			$$ = append_fragment($1, build_fragment(FRAGMENT_ROOT,
> +								$3));
>  		}
>  	| fragment_ref nodedef
>  		{
>  			$1->node = $2;
> -			$$ = merge_parsed_fragment(NULL, $1, $<flags>-1);
> +			$$ = $1;
>  		}
>  	| devicetree DT_LABEL fragment_ref nodedef
>  		{
>  			$3->label = $2;
>  			$3->label_srcpos = srcpos_copy(&@2);
>  			$3->node = $4;
> -			$$ = merge_parsed_fragment($1, $3, $<flags>-1);
> +			$$ = append_fragment($1, $3);
>  		}
>  	| devicetree fragment_ref nodedef
>  		{
>  			$2->node = $3;
> -			$$ = merge_parsed_fragment($1, $2, $<flags>-1);
> +			$$ = append_fragment($1, $2);
>  		}
>  	| devicetree DT_DEL_NODE fragment_ref ';'
>  		{
>  			$3->type = FRAGMENT_DELETE_NODE;
> -			$$ = merge_parsed_fragment($1, $3, $<flags>-1);
> +			$$ = append_fragment($1, $3);
>  		}
>  	| devicetree DT_OMIT_NO_REF fragment_ref ';'
>  		{
>  			$3->type = FRAGMENT_OMIT_IF_NO_REF;
> -			$$ = merge_parsed_fragment($1, $3, $<flags>-1);
> +			$$ = append_fragment($1, $3);
>  		}
>  	;
>  
> diff --git a/dtc.h b/dtc.h
> index 41013f5..fdd0d75 100644
> --- a/dtc.h
> +++ b/dtc.h
> @@ -368,7 +368,6 @@ struct fragment *build_fragment_ref(char *ref, bool path_ref,
>  				    struct srcpos *srcpos);
>  struct fragment *append_fragment(struct fragment *list, struct fragment *new);
>  void free_fragment(struct fragment *f);
> -bool merge_fragment(struct node **dtp, struct fragment *f, bool plugin);
>  
>  struct dt_info {
>  	unsigned int dtsflags;
> @@ -386,6 +385,7 @@ struct dt_info {
>  struct dt_info *build_dt_info(unsigned int dtsflags,
>  			      struct reserve_info *reservelist,
>  			      struct node *tree, uint32_t boot_cpuid_phys);
> +bool merge_fragments(struct dt_info *dti);
>  void sort_tree(struct dt_info *dti);
>  void generate_labels_from_tree(struct dt_info *dti, const char *name);
>  void generate_label_tree(struct dt_info *dti, const char *name, bool allocph);
> diff --git a/livetree.c b/livetree.c
> index 1e012d4..68ad3c3 100644
> --- a/livetree.c
> +++ b/livetree.c
> @@ -537,7 +537,8 @@ struct dt_info *build_dt_info(unsigned int dtsflags,
>  	dti->fragments = NULL;
>  	dti->boot_cpuid_phys = boot_cpuid_phys;
>  
> -	fill_fullpaths(dti->dt, "");
> +	if (tree)
> +		fill_fullpaths(dti->dt, "");
>  
>  	return dti;
>  }
> @@ -718,7 +719,8 @@ static bool merge_omit_if_no_ref(struct node *dt, struct fragment *f)
>   *
>   * Returns true if there is an error, which has been reported.
>   */
> -bool merge_fragment(struct node **dtp, struct fragment *f, bool plugin)
> +static bool merge_fragment(struct node **dtp, struct fragment *f,
> +			   bool plugin)
>  {
>  	struct node *dt = *dtp;
>  
> @@ -750,6 +752,36 @@ bool merge_fragment(struct node **dtp, struct fragment *f, bool plugin)
>  	return false;
>  }
>  
> +/*
> + * Merge the fragments of a dts file into a single tree, in source order.
> + *
> + * Returns true if there are errors, which have been reported.
> + */
> +bool merge_fragments(struct dt_info *dti)
> +{
> +	bool plugin = dti->dtsflags & DTSF_PLUGIN;
> +	struct fragment *f, *next;
> +	struct node *dt = NULL;
> +	bool error = false;
> +
> +	if (!dti->fragments)
> +		return false;
> +
> +	for (f = dti->fragments; f; f = next) {
> +		next = f->next;
> +		if (merge_fragment(&dt, f, plugin))
> +			error = true;
> +		free_fragment(f);
> +	}
> +
> +	dti->dt = dt;
> +	dti->fragments = NULL;

This field is really a temporary field. Should be removed from the
struct dt_info.


> +	dti->boot_cpuid_phys = guess_boot_cpuid(dt);
> +	fill_fullpaths(dti->dt, "");

fill_fullpaths() should be called only once in build_dt_info() as well as
setting dti->boot_cpuid_phys.

They have nothing to do with fragments.

> +
> +	return error;
> +}
> +
>  /*
>   * Tree accessor functions
>   */

Best regards,
Hervé

Re: [PATCH 13/13] dtc: Merge the fragments of a dts file in main()

[Herve Codina]

On Sun,  1 Nov 2026 09:37:07 -0700
Simon Glass <sjg@chromium.org> wrote:

> We would like to run checks on each fragment of a dts file before it is
> merged, but that requires dt_from_source() to return the fragments
> unmerged, so that main() can decide what to do with them. At present
> they are merged at the end of the parse.

Why running checks on each fragment instead of the dt where fragments are
merged ?

> 
> Call merge_fragments() from main() instead, once the input is read. It
> does nothing for dtb or fs input, which has no fragments.
> 
> When a reference cannot be resolved, dtc now reports 'Unable to merge
> input tree' rather than 'Syntax error parsing input tree', since the
> file was parsed successfully. Check for this message with
> test_label_ref.dts, since no test checks it at present.
> 
> If the parser reports an error, dt_from_source() gives up before main()
> can merge the fragments. Merge them first in that case, so that any
> errors in the fragments are still reported.

Merge fragments in build_dt_info() to avoid two calls two calls to
merge_fragments(), one in main() and one in dt_from_source().

Best regards,
Hervé

Re: [PATCH 11/13] dtc: Add helpers for the errors when merging a fragment

[Herve Codina]

On Sun,  1 Nov 2026 09:37:05 -0700
Simon Glass <sjg@chromium.org> wrote:

> Four of the merge functions look up the node which a fragment refers to
> and report 'Label or path not found' if it is missing, and three check
> for a label-relative reference in a plugin. Each of these has its own
> copy of the error, so a change to a message or its position must be made
> in several places.
> 
> Add lookup_ref() to look up the node, reporting an error if it is not
> found, and check_relative_ref() to report a label-relative reference.
> Use these in the merge functions.
> 
> The output does not change.
> 
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
>  livetree.c | 90 +++++++++++++++++++++++++++++-------------------------
>  1 file changed, 49 insertions(+), 41 deletions(-)
> 
> diff --git a/livetree.c b/livetree.c
> index 6a290fb..1e012d4 100644
> --- a/livetree.c
> +++ b/livetree.c
> @@ -548,6 +548,40 @@ static bool is_ref_relative(const char *ref)
>  	return ref[0] != '/' && strchr(&ref[1], '/');
>  }
>  
> +/*
> + * Report an error at pos if a fragment uses a label-relative reference, which
> + * a plugin does not support
> + *
> + * Returns true if there is an error
> + */
> +static bool check_relative_ref(struct fragment *f, struct srcpos *pos)
> +{
> +	if (!is_ref_relative(f->ref))
> +		return false;
> +	srcpos_error(pos, "Error",
> +		     "Label-relative reference %s not supported in plugin",
> +		     f->ref);

You suppose that srcpos is valid and so this function and remaining ones are
be called only from dt_from_source() with the parsing done by dtc-parser.y.

Not sure that this is a good assumption for the future.

IHMO, it will be better to decouple those functions from the type of parsing
or just add a simple 'if (pos)' before calling srcpos_error().

Best regards,
Hervé

> +
> +	return true;
> +}
> +
> +/*
> + * Look up the node which a fragment refers to, reporting an error if it is
> + * not found
> + *
> + * Returns the node, or NULL if it is not found
> + */
> +static struct node *lookup_ref(struct node *dt, struct fragment *f)
> +{
> +	struct node *target = get_node_by_ref(dt, f->ref);
> +
> +	if (!target)
> +		srcpos_error(f->ref_srcpos, "Error",
> +			     "Label or path %s not found", f->ref);
> +
> +	return target;
> +}
> +
>  /*
>   * Make the tree at *dtp from a first fragment which refers to a label or path,
>   * rather than being the root node. This is only valid in a plugin, where the
> @@ -557,17 +591,14 @@ static bool is_ref_relative(const char *ref)
>   */
>  static bool merge_first_ref(struct node **dtp, struct fragment *f, bool plugin)
>  {
> -	bool error = false;
> +	bool error;
>  
> -	if (!plugin) {
> +	if (plugin) {
> +		error = check_relative_ref(f, f->node->srcpos);
> +	} else {
>  		srcpos_error(f->node->srcpos, "Error",
>  			     "Label or path %s not found", f->ref);
>  		error = true;
> -	} else if (is_ref_relative(f->ref)) {
> -		srcpos_error(f->node->srcpos, "Error",
> -			     "Label-relative reference %s not supported in plugin",
> -			     f->ref);
> -		error = true;
>  	}
>  	/* Make a new, empty tree with a fragment@N node for the reference */
>  	*dtp = add_orphan_node(name_node(build_node(NULL, NULL, NULL), ""),
> @@ -584,20 +615,12 @@ static bool merge_first_ref(struct node **dtp, struct fragment *f, bool plugin)
>   */
>  static bool merge_labelled_ref(struct node *dt, struct fragment *f, bool plugin)
>  {
> -	struct node *target = get_node_by_ref(dt, f->ref);
> -	bool error = false;
> +	struct node *target;
> +	bool error = plugin && check_relative_ref(f, f->label_srcpos);
>  
> -	if (plugin && is_ref_relative(f->ref)) {
> -		srcpos_error(f->label_srcpos, "Error",
> -			     "Label-relative reference %s not supported in plugin",
> -			     f->ref);
> -		error = true;
> -	}
> -	if (!target) {
> -		srcpos_error(f->ref_srcpos, "Error",
> -			     "Label or path %s not found", f->ref);
> +	target = lookup_ref(dt, f);
> +	if (!target)
>  		return true;
> -	}
>  	add_label(&target->labels, f->label);
>  	merge_nodes(target, f->node);
>  
> @@ -615,26 +638,17 @@ static bool merge_path_ref(struct node *dt, struct fragment *f, bool plugin)
>  	struct node *target;
>  
>  	if (plugin) {
> -		bool error = false;
> +		bool error = check_relative_ref(f, f->ref_srcpos);
>  
> -		if (is_ref_relative(f->ref)) {
> -			srcpos_error(f->ref_srcpos, "Error",
> -				     "Label-relative reference %s not supported in plugin",
> -				     f->ref);
> -			error = true;
> -		}
>  		/* Add a fragment@N node which applies the node at the path */
>  		add_orphan_node(dt, f->node, f->ref);
>  
>  		return error;
>  	}
>  
> -	target = get_node_by_ref(dt, f->ref);
> -	if (!target) {
> -		srcpos_error(f->ref_srcpos, "Error",
> -			     "Label or path %s not found", f->ref);
> +	target = lookup_ref(dt, f);
> +	if (!target)
>  		return true;
> -	}
>  	merge_nodes(target, f->node);
>  
>  	return false;
> @@ -672,13 +686,10 @@ static bool merge_label_ref(struct node *dt, struct fragment *f, bool plugin)
>   */
>  static bool merge_delete_node(struct node *dt, struct fragment *f)
>  {
> -	struct node *target = get_node_by_ref(dt, f->ref);
> +	struct node *target = lookup_ref(dt, f);
>  
> -	if (!target) {
> -		srcpos_error(f->ref_srcpos, "Error",
> -			     "Label or path %s not found", f->ref);
> +	if (!target)
>  		return true;
> -	}
>  	delete_node(target);
>  
>  	return false;
> @@ -692,13 +703,10 @@ static bool merge_delete_node(struct node *dt, struct fragment *f)
>   */
>  static bool merge_omit_if_no_ref(struct node *dt, struct fragment *f)
>  {
> -	struct node *target = get_node_by_ref(dt, f->ref);
> +	struct node *target = lookup_ref(dt, f);
>  
> -	if (!target) {
> -		srcpos_error(f->ref_srcpos, "Error",
> -			     "Label or path %s not found", f->ref);
> +	if (!target)
>  		return true;
> -	}
>  	omit_node_if_unused(target);
>  
>  	return false;



-- 
Hervé Codina, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH 02/13] dtc: Add a list for the top-level fragments of a dts file

[Herve Codina]

On Mon, 2 Nov 2026 20:49:48 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Sun, Nov 01, 2026 at 09:36:56AM -0700, Simon Glass wrote:
> > A dts file is made up of top-level fragments: the root node, later
> > definitions of a node by label or path, and /delete-node/ or
> > /omit-if-no-ref/ with a reference. We would like to deal with each
> > fragment on its own before it is merged, for example to apply deletes
> > consistently or to run some checks on each one, but that requires
> > keeping the fragments apart. At present the parser merges each one into
> > the tree as soon as it is parsed.  
> 
> Btw, I'm extremely open to a better name than "fragment" if one occurs
> to you.  I believe I originally thought of these as "overlays" but
> that's confusing since runtime overlays were invented.

"A dts file is made up of top-level fragments: the root node, later
definitions of a node by label or path"

A node reference by a label or a path is not a definition of node, it is
a reference to a node.

Once those references are merged (if possible), we only have a root node
and orphan nodes (nodes without a parent node). Indeed, those orphan nodes
reference a node out of the current devicetree (dts or dtb).

Orphan nodes are just references that cannot be merged in the current
devicetree.

Instead of "fragment", "unresolved" ?

&label {
   prop;
};

Is an unresolved node while &label is not resolved to an existing node.

We try to merge those unresolved nodes (label resolution + merge) and any
unresolved nodes that are not merged (label not resolved) are orphan nodes.

The exact same occurs when we use paths instead of label. The only difference
is that a path can always be resolved. Having an unresolved path is an error.

With that in mind, having a look at orphan nodes in the addon series [0] (patch 44
and following ones) could give some ideas.

Does it make sense ?

[0]: https://lore.kernel.org/devicetree-compiler/20260826094950.1088288-1-herve.codina@bootlin.com/

Best regards,
Hervé

Re: [PATCH 02/13] dtc: Add a list for the top-level fragments of a dts file

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 4768 bytes --]

On Tue, Nov 03, 2026 at 06:07:14PM +0100, Herve Codina wrote:
> On Sun,  1 Nov 2026 09:36:56 -0700
> Simon Glass <sjg@chromium.org> wrote:
> 
> > A dts file is made up of top-level fragments: the root node, later
> > definitions of a node by label or path, and /delete-node/ or
> > /omit-if-no-ref/ with a reference. We would like to deal with each
> > fragment on its own before it is merged, for example to apply deletes
> > consistently or to run some checks on each one, but that requires
> > keeping the fragments apart. At present the parser merges each one into
> > the tree as soon as it is parsed.
> > 
> > Add a struct fragment to hold one of these, functions to build one, to
> > append it to a list and to free it, and a list of them in struct dt_info
> > 
> > These are not used yet.
> > 
> > Assisted-by: Claude:claude-opus-5-5
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> > 
> >  dtc.h      | 47 ++++++++++++++++++++++++++++++++++++++++++
> >  livetree.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 107 insertions(+)
> > 
> > diff --git a/dtc.h b/dtc.h
> > index 473552e..c0d8e8f 100644
> > --- a/dtc.h
> > +++ b/dtc.h
> > @@ -322,12 +322,59 @@ struct reserve_info *chain_reserve_entry(struct reserve_info *first,
> >  struct reserve_info *add_reserve_entry(struct reserve_info *list,
> >  				       struct reserve_info *new);
> >  
> > +/*
> > + * A fragment is one top-level part of a dts file, after the header and any
> > + * memory reservations: the root node, a later definition of a node which
> > + * refers to it by label or path, or a /delete-node/ or /omit-if-no-ref/ with
> > + * a reference. Fragments are kept apart until they are merged into a single
> > + * tree.
> > + *
> > + * This is not the same as an overlay fragment (a fragment@N node), although
> > + * in a plugin a fragment may become one when it is merged.
> > + */
> > +
> > +enum fragment_type {
> > +	FRAGMENT_ROOT,		/* / { ... }; */
> > +	FRAGMENT_REF,		/* [label:] &ref { ... }; */
> > +	FRAGMENT_DELETE_NODE,	/* /delete-node/ &ref; */
> > +	FRAGMENT_OMIT_IF_NO_REF, /* /omit-if-no-ref/ &ref; */
> > +};
> > +
> > +/*
> > + * Which fields are used depends on the type:
> > + *
> > + *   FRAGMENT_ROOT            node
> > + *   FRAGMENT_REF             ref, path_ref, ref_srcpos and node, plus label
> > + *                            and label_srcpos if there is a label
> > + *   FRAGMENT_DELETE_NODE     ref, path_ref and ref_srcpos
> > + *   FRAGMENT_OMIT_IF_NO_REF  ref, path_ref and ref_srcpos
> > + *
> > + * Unused fields are NULL or false.
> > + */
> > +struct fragment {
> > +	enum fragment_type type;
> > +	char *ref;			/* label or path referred to */
> > +	bool path_ref;			/* ref is a path, i.e. &{...} */
> > +	struct srcpos *ref_srcpos;	/* position of ref */
> > +	char *label;			/* label to add to the target */
> > +	struct srcpos *label_srcpos;	/* position of label */
> > +	struct node *node;		/* the node definition */
> > +
> > +	struct fragment *next;		/* NULL at the end of the list */
> > +};
> > +
> > +struct fragment *build_fragment(enum fragment_type type, struct node *node);
> > +struct fragment *build_fragment_ref(char *ref, bool path_ref,
> > +				    struct srcpos *srcpos);
> > +struct fragment *append_fragment(struct fragment *list, struct fragment *new);
> > +void free_fragment(struct fragment *f);
> >  
> >  struct dt_info {
> >  	unsigned int dtsflags;
> >  	struct reserve_info *reservelist;
> >  	uint32_t boot_cpuid_phys;
> >  	struct node *dt;		/* the device tree */
> > +	struct fragment *fragments;	/* not yet merged into dt */
> 
> Why to we need the fragment list in struct dt_info ?
> 
> This list is a temporary list built_dt_info() could have the list as parameter and
> handle it leading to the struct node *dt fully updated.
> 
> Other function using the struct dt_info should never see the temporary fragment 
> list and so, IMHO, there is no reason to store it in the struct
> dt_info.

That's true for now, but the aim of this change is to delay the merge
until later.  We want to be able to do things (like some checks)
before the merge, but after dt_from_*() and therefore after
build_dt_info().

I'll agree that having the redundant dt and fragments list side by
side in the structure is a little ugly, but not enough that I think it
requires a rework at this stage.  I'm open to better names than
"dt_info", but nothing is immediately occurring to me.

-- 
David Gibson (he or they)	| 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 02/13] dtc: Add a list for the top-level fragments of a dts file

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 2502 bytes --]

On Tue, Nov 03, 2026 at 06:56:20PM +0100, Herve Codina wrote:
> On Mon, 2 Nov 2026 20:49:48 +1100
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > On Sun, Nov 01, 2026 at 09:36:56AM -0700, Simon Glass wrote:
> > > A dts file is made up of top-level fragments: the root node, later
> > > definitions of a node by label or path, and /delete-node/ or
> > > /omit-if-no-ref/ with a reference. We would like to deal with each
> > > fragment on its own before it is merged, for example to apply deletes
> > > consistently or to run some checks on each one, but that requires
> > > keeping the fragments apart. At present the parser merges each one into
> > > the tree as soon as it is parsed.  
> > 
> > Btw, I'm extremely open to a better name than "fragment" if one occurs
> > to you.  I believe I originally thought of these as "overlays" but
> > that's confusing since runtime overlays were invented.
> 
> "A dts file is made up of top-level fragments: the root node, later
> definitions of a node by label or path"
> 
> A node reference by a label or a path is not a definition of node, it is
> a reference to a node.
> 
> Once those references are merged (if possible), we only have a root node
> and orphan nodes (nodes without a parent node). Indeed, those orphan nodes
> reference a node out of the current devicetree (dts or dtb).

Yes..

> Orphan nodes are just references that cannot be merged in the current
> devicetree.
> 
> Instead of "fragment", "unresolved" ?
> 
> &label {
>    prop;
> };
> 
> Is an unresolved node while &label is not resolved to an existing node.
> 
> We try to merge those unresolved nodes (label resolution + merge) and any
> unresolved nodes that are not merged (label not resolved) are orphan nodes.
> 
> The exact same occurs when we use paths instead of label. The only difference
> is that a path can always be resolved. Having an unresolved path is an error.
> 
> With that in mind, having a look at orphan nodes in the addon series [0] (patch 44
> and following ones) could give some ideas.
> 
> Does it make sense ?

That's all true, but I don't see what you're suggesting.  Here I'm
looking for a name for the syntactic top-level construct, whether or
not it's resolvable within this file or not.

-- 
David Gibson (he or they)	| 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 11/13] dtc: Add helpers for the errors when merging a fragment

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 7347 bytes --]

On Tue, Nov 03, 2026 at 06:23:44PM +0100, Herve Codina wrote:
> On Sun,  1 Nov 2026 09:37:05 -0700
> Simon Glass <sjg@chromium.org> wrote:
> 
> > Four of the merge functions look up the node which a fragment refers to
> > and report 'Label or path not found' if it is missing, and three check
> > for a label-relative reference in a plugin. Each of these has its own
> > copy of the error, so a change to a message or its position must be made
> > in several places.
> > 
> > Add lookup_ref() to look up the node, reporting an error if it is not
> > found, and check_relative_ref() to report a label-relative reference.
> > Use these in the merge functions.
> > 
> > The output does not change.
> > 
> > Assisted-by: Claude:claude-opus-5-5
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> > 
> >  livetree.c | 90 +++++++++++++++++++++++++++++-------------------------
> >  1 file changed, 49 insertions(+), 41 deletions(-)
> > 
> > diff --git a/livetree.c b/livetree.c
> > index 6a290fb..1e012d4 100644
> > --- a/livetree.c
> > +++ b/livetree.c
> > @@ -548,6 +548,40 @@ static bool is_ref_relative(const char *ref)
> >  	return ref[0] != '/' && strchr(&ref[1], '/');
> >  }
> >  
> > +/*
> > + * Report an error at pos if a fragment uses a label-relative reference, which
> > + * a plugin does not support
> > + *
> > + * Returns true if there is an error
> > + */
> > +static bool check_relative_ref(struct fragment *f, struct srcpos *pos)
> > +{
> > +	if (!is_ref_relative(f->ref))
> > +		return false;
> > +	srcpos_error(pos, "Error",
> > +		     "Label-relative reference %s not supported in plugin",
> > +		     f->ref);
> 
> You suppose that srcpos is valid and so this function and remaining ones are
> be called only from dt_from_source() with the parsing done by dtc-parser.y.
> 
> Not sure that this is a good assumption for the future.
> 
> IHMO, it will be better to decouple those functions from the type of parsing
> or just add a simple 'if (pos)' before calling srcpos_error().

It's a good point that eventually we'll want to get fragments from
dtbos as well as from source, in which case we won't have srcpos
information.  But when we are coming from source, the position
information is valuable.

This case of maybe-present srcpos information is common enough, that
the way to address it might be to alter srcpos_string() to accept
NULL, returning "<unknown>" or some other suitable placeholder when
srcpos information isn't available.

> 
> Best regards,
> Hervé
> 
> > +
> > +	return true;
> > +}
> > +
> > +/*
> > + * Look up the node which a fragment refers to, reporting an error if it is
> > + * not found
> > + *
> > + * Returns the node, or NULL if it is not found
> > + */
> > +static struct node *lookup_ref(struct node *dt, struct fragment *f)
> > +{
> > +	struct node *target = get_node_by_ref(dt, f->ref);
> > +
> > +	if (!target)
> > +		srcpos_error(f->ref_srcpos, "Error",
> > +			     "Label or path %s not found", f->ref);
> > +
> > +	return target;
> > +}
> > +
> >  /*
> >   * Make the tree at *dtp from a first fragment which refers to a label or path,
> >   * rather than being the root node. This is only valid in a plugin, where the
> > @@ -557,17 +591,14 @@ static bool is_ref_relative(const char *ref)
> >   */
> >  static bool merge_first_ref(struct node **dtp, struct fragment *f, bool plugin)
> >  {
> > -	bool error = false;
> > +	bool error;
> >  
> > -	if (!plugin) {
> > +	if (plugin) {
> > +		error = check_relative_ref(f, f->node->srcpos);
> > +	} else {
> >  		srcpos_error(f->node->srcpos, "Error",
> >  			     "Label or path %s not found", f->ref);
> >  		error = true;
> > -	} else if (is_ref_relative(f->ref)) {
> > -		srcpos_error(f->node->srcpos, "Error",
> > -			     "Label-relative reference %s not supported in plugin",
> > -			     f->ref);
> > -		error = true;
> >  	}
> >  	/* Make a new, empty tree with a fragment@N node for the reference */
> >  	*dtp = add_orphan_node(name_node(build_node(NULL, NULL, NULL), ""),
> > @@ -584,20 +615,12 @@ static bool merge_first_ref(struct node **dtp, struct fragment *f, bool plugin)
> >   */
> >  static bool merge_labelled_ref(struct node *dt, struct fragment *f, bool plugin)
> >  {
> > -	struct node *target = get_node_by_ref(dt, f->ref);
> > -	bool error = false;
> > +	struct node *target;
> > +	bool error = plugin && check_relative_ref(f, f->label_srcpos);
> >  
> > -	if (plugin && is_ref_relative(f->ref)) {
> > -		srcpos_error(f->label_srcpos, "Error",
> > -			     "Label-relative reference %s not supported in plugin",
> > -			     f->ref);
> > -		error = true;
> > -	}
> > -	if (!target) {
> > -		srcpos_error(f->ref_srcpos, "Error",
> > -			     "Label or path %s not found", f->ref);
> > +	target = lookup_ref(dt, f);
> > +	if (!target)
> >  		return true;
> > -	}
> >  	add_label(&target->labels, f->label);
> >  	merge_nodes(target, f->node);
> >  
> > @@ -615,26 +638,17 @@ static bool merge_path_ref(struct node *dt, struct fragment *f, bool plugin)
> >  	struct node *target;
> >  
> >  	if (plugin) {
> > -		bool error = false;
> > +		bool error = check_relative_ref(f, f->ref_srcpos);
> >  
> > -		if (is_ref_relative(f->ref)) {
> > -			srcpos_error(f->ref_srcpos, "Error",
> > -				     "Label-relative reference %s not supported in plugin",
> > -				     f->ref);
> > -			error = true;
> > -		}
> >  		/* Add a fragment@N node which applies the node at the path */
> >  		add_orphan_node(dt, f->node, f->ref);
> >  
> >  		return error;
> >  	}
> >  
> > -	target = get_node_by_ref(dt, f->ref);
> > -	if (!target) {
> > -		srcpos_error(f->ref_srcpos, "Error",
> > -			     "Label or path %s not found", f->ref);
> > +	target = lookup_ref(dt, f);
> > +	if (!target)
> >  		return true;
> > -	}
> >  	merge_nodes(target, f->node);
> >  
> >  	return false;
> > @@ -672,13 +686,10 @@ static bool merge_label_ref(struct node *dt, struct fragment *f, bool plugin)
> >   */
> >  static bool merge_delete_node(struct node *dt, struct fragment *f)
> >  {
> > -	struct node *target = get_node_by_ref(dt, f->ref);
> > +	struct node *target = lookup_ref(dt, f);
> >  
> > -	if (!target) {
> > -		srcpos_error(f->ref_srcpos, "Error",
> > -			     "Label or path %s not found", f->ref);
> > +	if (!target)
> >  		return true;
> > -	}
> >  	delete_node(target);
> >  
> >  	return false;
> > @@ -692,13 +703,10 @@ static bool merge_delete_node(struct node *dt, struct fragment *f)
> >   */
> >  static bool merge_omit_if_no_ref(struct node *dt, struct fragment *f)
> >  {
> > -	struct node *target = get_node_by_ref(dt, f->ref);
> > +	struct node *target = lookup_ref(dt, f);
> >  
> > -	if (!target) {
> > -		srcpos_error(f->ref_srcpos, "Error",
> > -			     "Label or path %s not found", f->ref);
> > +	if (!target)
> >  		return true;
> > -	}
> >  	omit_node_if_unused(target);
> >  
> >  	return false;
> 
> 
> 
> -- 
> Hervé Codina, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
> 

-- 
David Gibson (he or they)	| 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 12/13] dtc: Merge the fragments of a dts file after parsing

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 4279 bytes --]

On Tue, Nov 03, 2026 at 06:07:53PM +0100, Herve Codina wrote:
> On Sun,  1 Nov 2026 09:37:06 -0700
> Simon Glass <sjg@chromium.org> wrote:
> 
> > We would like to deal with each top-level fragment of a dts file before
> > it is merged into the tree, but that requires the fragments to be kept
> > apart until the whole file is parsed. At present the parser merges each
> > one as soon as it is parsed. The grammar actions also need to know
> > whether the file is a plugin, which they find using $<flags>-1
> > 
> > Have the grammar build a list of the fragments instead, in source order,
> > and merge them with a new merge_fragments() once the whole file is
> > parsed. This calls merge_fragment() for each one, so the output does not
> > change, and frees each fragment once it is merged.
> > 
> > The $<flags>-1 term is no longer needed. Within a devicetree action it
> > reaches back up the parser stack to the value of headers, i.e. the flags
> > from /dts-v1/ and /plugin/. The sourcefile action passes the same value
> > to build_dt_info(), which stores it in dti->dtsflags, so
> > merge_fragments() can use that instead.
> > 
> > There are two differences in the errors. A syntax error later in the
> > file now stops dtc before any earlier reference errors are reported, and
> > reference errors now come after any other errors which the parser
> > reports, such as a division by zero, rather than in source order.
> > 
> > dtc gives the same output and messages as before for all 4162 .dts and
> > .dtso files in Linux v7.3-12284-g3799eeb906c9, and for the dts files in
> > tests/
> > 
> > Suggested-by: David Gibson <david@gibson.dropbear.id.au>
> > Link: https://lore.kernel.org/devicetree-compiler/auK0ERvxGCJGSc3p@gractus.seuss/
> > Assisted-by: Claude:claude-opus-5-5
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> > 
> >  dtc-parser.y | 43 +++++++++++++------------------------------
> >  dtc.h        |  2 +-
> >  livetree.c   | 36 ++++++++++++++++++++++++++++++++++--
> >  3 files changed, 48 insertions(+), 33 deletions(-)
> > 
> > diff --git a/dtc-parser.y b/dtc-parser.y
> > index 1ed9731..c1aa4ad 100644
> > --- a/dtc-parser.y
> > +++ b/dtc-parser.y
> > @@ -24,22 +24,6 @@ extern void yyerror(char const *s);
> >  extern struct dt_info *parser_output;
> >  extern bool treesource_error;
> >  
> > -/*
> > - * Merge a fragment into the tree as soon as it is parsed. The flags come
> > - * from the actions using $<flags>-1, which relies on the rule being always:
> > - *   headers memreserves devicetree
> > - * so $-1 is the value of headers
> > - */
> > -static struct node *merge_parsed_fragment(struct node *dt, struct fragment *f,
> > -					  unsigned int flags)
> > -{
> > -	if (merge_fragment(&dt, f, flags & DTSF_PLUGIN))
> > -		treesource_error = true;
> > -	free_fragment(f);
> > -
> > -	return dt;
> > -}
> > -
> >  %}
> >  
> >  %union {
> > @@ -94,7 +78,7 @@ static struct node *merge_parsed_fragment(struct node *dt, struct fragment *f,
> >  %type <labelref> dt_ref
> >  %type <fragment> fragment_ref
> >  
> > -%type <node> devicetree
> > +%type <fragment> devicetree
> >  %type <node> nodedef
> >  %type <node> subnode
> >  %type <nodelist> subnodes
> > @@ -119,8 +103,10 @@ static struct node *merge_parsed_fragment(struct node *dt, struct fragment *f,
> >  sourcefile:
> >  	  headers memreserves devicetree
> >  		{
> > -			parser_output = build_dt_info($1, $2, $3,
> > -			                              guess_boot_cpuid($3));
> > +			parser_output = build_dt_info($1, $2, NULL, 0);
> > +			parser_output->fragments = $3;
> > +			if (merge_fragments(parser_output))
> > +				treesource_error = true;
> 
> Why not just pass the fragment list to build_dt_info() instead of using the 
> parser_output->fragments and later in merge_fragments() setting this same field
> to NULL once the merge is done?

As I noted elswhere, the point of this series is to delay the merge,
and it will need to be after build_dt_info() to do what we want.

-- 
David Gibson (he or they)	| 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 13/13] dtc: Merge the fragments of a dts file in main()

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 885 bytes --]

On Tue, Nov 03, 2026 at 06:08:03PM +0100, Herve Codina wrote:
> On Sun,  1 Nov 2026 09:37:07 -0700
> Simon Glass <sjg@chromium.org> wrote:
> 
> > We would like to run checks on each fragment of a dts file before it is
> > merged, but that requires dt_from_source() to return the fragments
> > unmerged, so that main() can decide what to do with them. At present
> > they are merged at the end of the parse.
> 
> Why running checks on each fragment instead of the dt where fragments are
> merged ?

This is part of the aim of the series.  Some checks make sense on an
individual fragment, others don't.  Allowing both kinds lets us
address several longstanding problems.


-- 
David Gibson (he or they)	| 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 02/13] dtc: Add a list for the top-level fragments of a dts file

[Herve Codina]

On Wed, 4 Nov 2026 17:32:35 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Tue, Nov 03, 2026 at 06:56:20PM +0100, Herve Codina wrote:
> > On Mon, 2 Nov 2026 20:49:48 +1100
> > David Gibson <david@gibson.dropbear.id.au> wrote:
> >   
> > > On Sun, Nov 01, 2026 at 09:36:56AM -0700, Simon Glass wrote:  
> > > > A dts file is made up of top-level fragments: the root node, later
> > > > definitions of a node by label or path, and /delete-node/ or
> > > > /omit-if-no-ref/ with a reference. We would like to deal with each
> > > > fragment on its own before it is merged, for example to apply deletes
> > > > consistently or to run some checks on each one, but that requires
> > > > keeping the fragments apart. At present the parser merges each one into
> > > > the tree as soon as it is parsed.    
> > > 
> > > Btw, I'm extremely open to a better name than "fragment" if one occurs
> > > to you.  I believe I originally thought of these as "overlays" but
> > > that's confusing since runtime overlays were invented.  
> > 
> > "A dts file is made up of top-level fragments: the root node, later
> > definitions of a node by label or path"
> > 
> > A node reference by a label or a path is not a definition of node, it is
> > a reference to a node.
> > 
> > Once those references are merged (if possible), we only have a root node
> > and orphan nodes (nodes without a parent node). Indeed, those orphan nodes
> > reference a node out of the current devicetree (dts or dtb).  
> 
> Yes..
> 
> > Orphan nodes are just references that cannot be merged in the current
> > devicetree.
> > 
> > Instead of "fragment", "unresolved" ?
> > 
> > &label {
> >    prop;
> > };
> > 
> > Is an unresolved node while &label is not resolved to an existing node.
> > 
> > We try to merge those unresolved nodes (label resolution + merge) and any
> > unresolved nodes that are not merged (label not resolved) are orphan nodes.
> > 
> > The exact same occurs when we use paths instead of label. The only difference
> > is that a path can always be resolved. Having an unresolved path is an error.
> > 
> > With that in mind, having a look at orphan nodes in the addon series [0] (patch 44
> > and following ones) could give some ideas.
> > 
> > Does it make sense ?  
> 
> That's all true, but I don't see what you're suggesting.  Here I'm
> looking for a name for the syntactic top-level construct, whether or
> not it's resolvable within this file or not.
> 

struct node {
	...
	const char *ref;  /* Only orphan nodes doesn't have a name but a ref */
}

struct dt_info {
	struct node *root;
	struct node *orphanlist;
	...
}

When you parse, you create the orphanlist with orphan nodes (same order in the
list as the order in the dts) without trying to merge.

Once done, build_dt_info(..., root, orphanlist);

Then, in build_dt_info() or later:

reduce_orphans(struct dt_info *dt_info)
  - Handle deletes
    You have the root node and the orphan list with item in the same order as
    they are found in the dts. Deleting node available before the "/delete-node/"
    keyword should be doable.
  - Merge as much nodes as possible from the orphan list. This merge orphan
    nodes in either the root node or a possible previous orphan.

After this call:
 - All possible deletes have been handled
 - Orphans are reduced to real orphan without duplicates (merge done when
   possible).

For dts, this orphan list must be empty.
For overlay or addons, this orphan list can have items.

Perform the checks with this structure.

Generate output:
  For overlays: Convert to /fragment@N/... then generate the output
  For addons: no conversion needed, generate the output

Do we want to handle deletes at runtime ?
The only relevant is having an overlay or addon the capabilities to
delete an orphan node. In that case, at runtime, instead of merging
a delete is perform. This use case is not handle yet but to handle
them:

  - reduce_orphans(struct dt_info *dt_info)
    Delete what you can delete
    Keep orphans with delete flag in the orphan list (after reduction)
  - Generate output
     for orphan with delete flag, encode the delete information (new node
     metadata tag). Those orphan with delete flag should be empty. No
     property to merge, no node to merge. Indeed they are in the output
     just to tell that the related node in the base tree must be deleted.


Now checks(). The orphan list has been reduce to the strict minimum. Is that
ok for checks() you have in mind?


Best regards,
Hervé

Re: [PATCH 13/13] dtc: Merge the fragments of a dts file in main()

[Herve Codina]

Hi David,

On Wed, 4 Nov 2026 17:51:41 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Tue, Nov 03, 2026 at 06:08:03PM +0100, Herve Codina wrote:
> > On Sun,  1 Nov 2026 09:37:07 -0700
> > Simon Glass <sjg@chromium.org> wrote:
> >   
> > > We would like to run checks on each fragment of a dts file before it is
> > > merged, but that requires dt_from_source() to return the fragments
> > > unmerged, so that main() can decide what to do with them. At present
> > > they are merged at the end of the parse.  
> > 
> > Why running checks on each fragment instead of the dt where fragments are
> > merged ?  
> 
> This is part of the aim of the series.  Some checks make sense on an
> individual fragment, others don't.  Allowing both kinds lets us
> address several longstanding problems.
> 

To clarify this point, what kind of checks for individual fragment?
What kind of longstanding problems?

Can you give me details about them?

Maybe I've faced some of them in the support for addons implementation.

Best regards,
Hervé

Re: [PATCH 11/13] dtc: Add helpers for the errors when merging a fragment

[Herve Codina]

On Wed, 4 Nov 2026 17:49:09 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Tue, Nov 03, 2026 at 06:23:44PM +0100, Herve Codina wrote:
> > On Sun,  1 Nov 2026 09:37:05 -0700
> > Simon Glass <sjg@chromium.org> wrote:
> >   
> > > Four of the merge functions look up the node which a fragment refers to
> > > and report 'Label or path not found' if it is missing, and three check
> > > for a label-relative reference in a plugin. Each of these has its own
> > > copy of the error, so a change to a message or its position must be made
> > > in several places.
> > > 
> > > Add lookup_ref() to look up the node, reporting an error if it is not
> > > found, and check_relative_ref() to report a label-relative reference.
> > > Use these in the merge functions.
> > > 
> > > The output does not change.
> > > 
> > > Assisted-by: Claude:claude-opus-5-5
> > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > ---
> > > 
> > >  livetree.c | 90 +++++++++++++++++++++++++++++-------------------------
> > >  1 file changed, 49 insertions(+), 41 deletions(-)
> > > 
> > > diff --git a/livetree.c b/livetree.c
> > > index 6a290fb..1e012d4 100644
> > > --- a/livetree.c
> > > +++ b/livetree.c
> > > @@ -548,6 +548,40 @@ static bool is_ref_relative(const char *ref)
> > >  	return ref[0] != '/' && strchr(&ref[1], '/');
> > >  }
> > >  
> > > +/*
> > > + * Report an error at pos if a fragment uses a label-relative reference, which
> > > + * a plugin does not support
> > > + *
> > > + * Returns true if there is an error
> > > + */
> > > +static bool check_relative_ref(struct fragment *f, struct srcpos *pos)
> > > +{
> > > +	if (!is_ref_relative(f->ref))
> > > +		return false;
> > > +	srcpos_error(pos, "Error",
> > > +		     "Label-relative reference %s not supported in plugin",
> > > +		     f->ref);  
> > 
> > You suppose that srcpos is valid and so this function and remaining ones are
> > be called only from dt_from_source() with the parsing done by dtc-parser.y.
> > 
> > Not sure that this is a good assumption for the future.
> > 
> > IHMO, it will be better to decouple those functions from the type of parsing
> > or just add a simple 'if (pos)' before calling srcpos_error().  
> 
> It's a good point that eventually we'll want to get fragments from
> dtbos as well as from source, in which case we won't have srcpos
> information.  But when we are coming from source, the position
> information is valuable.
> 
> This case of maybe-present srcpos information is common enough, that
> the way to address it might be to alter srcpos_string() to accept
> NULL, returning "<unknown>" or some other suitable placeholder when
> srcpos information isn't available.

"<unknown>" or just an empty string "" to avoid any user confusion.
That's a detail.

Yes, checking pos at the low level srcpos_string() is a good things to do.

Best regards
Hervé

Re: [PATCH 02/13] dtc: Add a list for the top-level fragments of a dts file

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 5760 bytes --]

On Wed, Nov 04, 2026 at 09:29:11AM +0100, Herve Codina wrote:
> On Wed, 4 Nov 2026 17:32:35 +1100
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > On Tue, Nov 03, 2026 at 06:56:20PM +0100, Herve Codina wrote:
> > > On Mon, 2 Nov 2026 20:49:48 +1100
> > > David Gibson <david@gibson.dropbear.id.au> wrote:
> > >   
> > > > On Sun, Nov 01, 2026 at 09:36:56AM -0700, Simon Glass wrote:  
> > > > > A dts file is made up of top-level fragments: the root node, later
> > > > > definitions of a node by label or path, and /delete-node/ or
> > > > > /omit-if-no-ref/ with a reference. We would like to deal with each
> > > > > fragment on its own before it is merged, for example to apply deletes
> > > > > consistently or to run some checks on each one, but that requires
> > > > > keeping the fragments apart. At present the parser merges each one into
> > > > > the tree as soon as it is parsed.    
> > > > 
> > > > Btw, I'm extremely open to a better name than "fragment" if one occurs
> > > > to you.  I believe I originally thought of these as "overlays" but
> > > > that's confusing since runtime overlays were invented.  
> > > 
> > > "A dts file is made up of top-level fragments: the root node, later
> > > definitions of a node by label or path"
> > > 
> > > A node reference by a label or a path is not a definition of node, it is
> > > a reference to a node.
> > > 
> > > Once those references are merged (if possible), we only have a root node
> > > and orphan nodes (nodes without a parent node). Indeed, those orphan nodes
> > > reference a node out of the current devicetree (dts or dtb).  
> > 
> > Yes..
> > 
> > > Orphan nodes are just references that cannot be merged in the current
> > > devicetree.
> > > 
> > > Instead of "fragment", "unresolved" ?
> > > 
> > > &label {
> > >    prop;
> > > };
> > > 
> > > Is an unresolved node while &label is not resolved to an existing node.
> > > 
> > > We try to merge those unresolved nodes (label resolution + merge) and any
> > > unresolved nodes that are not merged (label not resolved) are orphan nodes.
> > > 
> > > The exact same occurs when we use paths instead of label. The only difference
> > > is that a path can always be resolved. Having an unresolved path is an error.
> > > 
> > > With that in mind, having a look at orphan nodes in the addon series [0] (patch 44
> > > and following ones) could give some ideas.
> > > 
> > > Does it make sense ?  
> > 
> > That's all true, but I don't see what you're suggesting.  Here I'm
> > looking for a name for the syntactic top-level construct, whether or
> > not it's resolvable within this file or not.
> > 
> 
> struct node {
> 	...
> 	const char *ref;  /* Only orphan nodes doesn't have a name but a ref */
> }

I don't think this is a good change.  Most nodes don't have this a
fragment (or whatever we call it) is a structurally different element
that should have a different representation.

> struct dt_info {
> 	struct node *root;
> 	struct node *orphanlist;
> 	...
> }
> 
> When you parse, you create the orphanlist with orphan nodes (same order in the
> list as the order in the dts) without trying to merge.

"orphan" is a bad name here - the "orphan" implies we don't have a
parent we can attach them to, which we don't know until we attempt to
resolve during merge.

> Once done, build_dt_info(..., root, orphanlist);

That's a good function signature (apart from the "orphan" name).

> Then, in build_dt_info() or later:
> 
> reduce_orphans(struct dt_info *dt_info)
>   - Handle deletes
>     You have the root node and the orphan list with item in the same order as
>     they are found in the dts. Deleting node available before the "/delete-node/"
>     keyword should be doable.
>   - Merge as much nodes as possible from the orphan list. This merge orphan
>     nodes in either the root node or a possible previous orphan.

Sure, but this isn't meaningfully different from the current draft (I
think, I haven't finished reading it).

> After this call:
>  - All possible deletes have been handled
>  - Orphans are reduced to real orphan without duplicates (merge done when
>    possible).
> 
> For dts, this orphan list must be empty.
> For overlay or addons, this orphan list can have items.
> 
> Perform the checks with this structure.
> 
> Generate output:
>   For overlays: Convert to /fragment@N/... then generate the output
>   For addons: no conversion needed, generate the output
> 
> Do we want to handle deletes at runtime ?

No.  Runtime overlays are already a nasty hack, I don't want to add
more complexity to them.

> The only relevant is having an overlay or addon the capabilities to
> delete an orphan node. In that case, at runtime, instead of merging
> a delete is perform. This use case is not handle yet but to handle
> them:
> 
>   - reduce_orphans(struct dt_info *dt_info)
>     Delete what you can delete
>     Keep orphans with delete flag in the orphan list (after reduction)
>   - Generate output
>      for orphan with delete flag, encode the delete information (new node
>      metadata tag). Those orphan with delete flag should be empty. No
>      property to merge, no node to merge. Indeed they are in the output
>      just to tell that the related node in the base tree must be deleted.
> 
> 
> Now checks(). The orphan list has been reduce to the strict minimum. Is that
> ok for checks() you have in mind?
> 
> 
> Best regards,
> Hervé
> 

-- 
David Gibson (he or they)	| 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 13/13] dtc: Merge the fragments of a dts file in main()

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 3251 bytes --]

On Wed, Nov 04, 2026 at 09:40:55AM +0100, Herve Codina wrote:
> Hi David,
> 
> On Wed, 4 Nov 2026 17:51:41 +1100
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > On Tue, Nov 03, 2026 at 06:08:03PM +0100, Herve Codina wrote:
> > > On Sun,  1 Nov 2026 09:37:07 -0700
> > > Simon Glass <sjg@chromium.org> wrote:
> > >   
> > > > We would like to run checks on each fragment of a dts file before it is
> > > > merged, but that requires dt_from_source() to return the fragments
> > > > unmerged, so that main() can decide what to do with them. At present
> > > > they are merged at the end of the parse.  
> > > 
> > > Why running checks on each fragment instead of the dt where fragments are
> > > merged ?  
> > 
> > This is part of the aim of the series.  Some checks make sense on an
> > individual fragment, others don't.  Allowing both kinds lets us
> > address several longstanding problems.
> > 
> 
> To clarify this point, what kind of checks for individual fragment?

As many as we can, eventually.  Anything that doesn't rely on whole
tree context is a candidate.  This means
  a) they'll be checked sensibly for dtbo output where we don't have
     all the information to perform checks that rely on the broader
     tree context
  b) they'll catch logically incorrect things, even if they're later
     overwritten by something

Initially the most important ones to run before merge are the
structural ones like duplicate_*_names, node_name_not_empty,
node_name_format.  The duplicate ones in particular means we enable
consistent behaviour w.r.t. overwriting nodes between the first
fragment and later fragments in the file.


In fact, thinking about it, before the merge should eventually be the
default place to run tests.  So the question is more, which checks
should be deferred until after the merge (and therefore not be run at
all for dtbo output).  Obviously anything that needs to look up an
absolute path has to go after the merge.  Things that look up phanles
are a tricky intermediate case.  Potentially those can work before
merge, but only if they search for labels rather than actual phandles.

> What kind of longstanding problems?

The biggest one, that I'm hoping this series will address is the fact
that we don't have consistent behaviour if a node or property name is
repeated between the first fragment (it generates a check failure) and
later fragments (it silently overwrites).  But there have also been
various reports about false positives from checks when they run on
/plugin/ files, because the partial tree just doesn't have the
information needed to properly complete the check.  Having different
sets of checks before and after merge is the clearerst way I can see
to properly handle those false positives, while still checking as much
as we can for a dtbo.
 
> Can you give me details about them?
> 
> Maybe I've faced some of them in the support for addons implementation.

That's possible, yes.  Or you might yet face them in addons.

-- 
David Gibson (he or they)	| 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 13/13] dtc: Merge the fragments of a dts file in main()

[Herve Codina]

Hi David,

On Wed, 4 Nov 2026 20:47:45 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Wed, Nov 04, 2026 at 09:40:55AM +0100, Herve Codina wrote:
> > Hi David,
> > 
> > On Wed, 4 Nov 2026 17:51:41 +1100
> > David Gibson <david@gibson.dropbear.id.au> wrote:
> >   
> > > On Tue, Nov 03, 2026 at 06:08:03PM +0100, Herve Codina wrote:  
> > > > On Sun,  1 Nov 2026 09:37:07 -0700
> > > > Simon Glass <sjg@chromium.org> wrote:
> > > >     
> > > > > We would like to run checks on each fragment of a dts file before it is
> > > > > merged, but that requires dt_from_source() to return the fragments
> > > > > unmerged, so that main() can decide what to do with them. At present
> > > > > they are merged at the end of the parse.    
> > > > 
> > > > Why running checks on each fragment instead of the dt where fragments are
> > > > merged ?    
> > > 
> > > This is part of the aim of the series.  Some checks make sense on an
> > > individual fragment, others don't.  Allowing both kinds lets us
> > > address several longstanding problems.
> > >   
> > 
> > To clarify this point, what kind of checks for individual fragment?  
> 
> As many as we can, eventually.  Anything that doesn't rely on whole
> tree context is a candidate.  This means
>   a) they'll be checked sensibly for dtbo output where we don't have
>      all the information to perform checks that rely on the broader
>      tree context
>   b) they'll catch logically incorrect things, even if they're later
>      overwritten by something
> 
> Initially the most important ones to run before merge are the
> structural ones like duplicate_*_names, node_name_not_empty,
> node_name_format.  The duplicate ones in particular means we enable
> consistent behaviour w.r.t. overwriting nodes between the first
> fragment and later fragments in the file.
> 
> 
> In fact, thinking about it, before the merge should eventually be the
> default place to run tests.  So the question is more, which checks
> should be deferred until after the merge (and therefore not be run at
> all for dtbo output).  Obviously anything that needs to look up an
> absolute path has to go after the merge.  Things that look up phanles
> are a tricky intermediate case.  Potentially those can work before
> merge, but only if they search for labels rather than actual phandles.
> 
> > What kind of longstanding problems?  
> 
> The biggest one, that I'm hoping this series will address is the fact
> that we don't have consistent behaviour if a node or property name is
> repeated between the first fragment (it generates a check failure) and
> later fragments (it silently overwrites).  But there have also been
> various reports about false positives from checks when they run on
> /plugin/ files, because the partial tree just doesn't have the
> information needed to properly complete the check.  Having different
> sets of checks before and after merge is the clearerst way I can see
> to properly handle those false positives, while still checking as much
> as we can for a dtbo.
>  
> > Can you give me details about them?
> > 
> > Maybe I've faced some of them in the support for addons implementation.  
> 
> That's possible, yes.  Or you might yet face them in addons.
> 

Hum, I see. I think the problem is not before / after merge for checks but
with / without orphans.

In a dts, we can have:
--- 8< ---
/ {
	node: node-a {
		prop1;
	};
};

&node {
	prop2;
	subnode: subnode {
		prop1;
	}
};

&node {
	prop3;
}

&subnode {
	prop2;
};
--- 8< --

In the end no orphans are present. Everything is merged in the root node.

In a dtso we can have:
--- 8< ---
&node {
	prop2;
	subnode: subnode {
		prop1;
	}
};

&node {
	prop3;
}

&subnode {
	prop2;
};
--- 8< ---

In this case, everything is merged to the orphan node &node.
This merged orphan node has to be checked.

I make a distinction between merge and format conversion performed for orphan
handling in dtso.

For dtso, the orphan node &node will be converted (format conversion) to
--- 8< ---
/ {
	fragment@0 {
		target = <&node>;
		__overlay__ {
			...
		};
	};

	__fixup__ {
		node = "/fragment@0:target:0";
	};
};
--- 8< ---

Best regards,
Hervé

Re: [PATCH 01/13] tests: Check each error when merging a dts file

[Simon Glass]

Hi David,

On Mon, 2 Nov 2026 at 01:16, David Gibson <david@gibson.dropbear.id.au> wrote:
>
> On Sun, Nov 01, 2026 at 09:36:55AM -0700, Simon Glass wrote:
> > When the parser merges the top-level fragments of a dts file, it can
> > report that a label or path is not found, or that a label-relative
> > reference is not supported in a plugin. Most of these errors are not
> > covered by any test, and the two tests which do reach them only check
> > that dtc fails, not the message or where it is reported. The merge
> > logic is about to move out of the grammar, so this needs better tests.
> >
> > Add a test file for each error which is not covered, and check the
> > message and its position for all of them, including the two existing
> > ones. Also check that merging carries on after an error, so that later
> > errors are reported too, that a reference error is still reported when
> > the parser finds another error, and that a labelled reference within a
> > plugin adds the label, since nothing covers these either.
> >
> > Assisted-by: Claude:claude-opus-5-5
> > Signed-off-by: Simon Glass <sjg@chromium.org>
>
> Couple of nits, otherwise LGTM.
>
> > ---
> >
> >  tests/merge-delete-missing.dts     |  9 +++++++
> >  tests/merge-errors.dts             | 14 ++++++++++
> >  tests/merge-first-ref-relative.dts | 10 ++++++++
> >  tests/merge-first-ref.dts          |  9 +++++++
> >  tests/merge-labelled-plugin.dts    | 12 +++++++++
> >  tests/merge-labelled-relative.dts  | 14 ++++++++++
> >  tests/merge-omit-missing.dts       |  9 +++++++
> >  tests/merge-parse-error.dts        | 17 +++++++++++++
> >  tests/merge-path-ref-missing.dts   | 11 ++++++++
> >  tests/merge-path-relative.dts      | 12 +++++++++
> >  tests/run_tests.sh                 | 41 ++++++++++++++++++++++++++++++
> >  11 files changed, 158 insertions(+)
> >  create mode 100644 tests/merge-delete-missing.dts
> >  create mode 100644 tests/merge-errors.dts
> >  create mode 100644 tests/merge-first-ref-relative.dts
> >  create mode 100644 tests/merge-first-ref.dts
> >  create mode 100644 tests/merge-labelled-plugin.dts
> >  create mode 100644 tests/merge-labelled-relative.dts
> >  create mode 100644 tests/merge-omit-missing.dts
> >  create mode 100644 tests/merge-parse-error.dts
> >  create mode 100644 tests/merge-path-ref-missing.dts
> >  create mode 100644 tests/merge-path-relative.dts
> >
> > diff --git a/tests/merge-delete-missing.dts b/tests/merge-delete-missing.dts
> > new file mode 100644
> > index 0000000..b36e39e
> > --- /dev/null
> > +++ b/tests/merge-delete-missing.dts
> > @@ -0,0 +1,9 @@
> > +/dts-v1/;
> > +
> > +/ {
> > +     label: node {
> > +     };
> > +};
> > +
> > +/* Try to delete a node using a non-existent label */
> > +/delete-node/ &nosuchnode;
> > diff --git a/tests/merge-errors.dts b/tests/merge-errors.dts
> > new file mode 100644
> > index 0000000..6d3969f
> > --- /dev/null
> > +++ b/tests/merge-errors.dts
> > @@ -0,0 +1,14 @@
> > +/dts-v1/;
> > +
> > +/ {
> > +     label: node {
> > +     };
> > +};
> > +
> > +/* Each error is reported, not just the first */
>
> I'm a bit confused by this comment - I only see one error in this
> file.

I mean including the /delete-node/ and /omit-if-no-ref/ below. I'll
update the comment to be more specific.

>
> > +&nosuchnode1 {
> > +     prop;
> > +};
> > +
> > +/delete-node/ &nosuchnode2;
> > +/omit-if-no-ref/ &nosuchnode3;
> > diff --git a/tests/merge-first-ref-relative.dts b/tests/merge-first-ref-relative.dts
> > new file mode 100644
> > index 0000000..103cd1f
> > --- /dev/null
> > +++ b/tests/merge-first-ref-relative.dts
> > @@ -0,0 +1,10 @@
> > +/dts-v1/;
> > +/plugin/;
> > +
> > +/* A plugin cannot start with a label-relative reference */
>
> That's slightly misleading, insn't it?  A plugin can't have _any_
> label relative references unless they resolve within the plugin.

Yes, I'll reword that to say you can't *define* a node. dtc does allow
'/delete-node/ &{label/subnode}' in a plugin, for example, as long as
the reference resolves within the plugin. Property references work the
same way. Perhaps a node definition could too, since it can merge once
the reference resolved? Anyway, let's worry about that later.

>
> > +&{label/node} {
> > +     prop;
> > +};
> > +
> > +/* Later fragments are still merged */
> > +/delete-node/ &nosuchnode;

[..]

Regards,
Simon

Re: [PATCH 03/13] dtc: Move the /delete-node/ handling out of the grammar

[Simon Glass]

Hi David,

On Mon, 2 Nov 2026 at 02:55, David Gibson <david@gibson.dropbear.id.au> wrote:
>
> On Sun, Nov 01, 2026 at 09:36:57AM -0700, Simon Glass wrote:
> > The grammar actions merge each top-level fragment of a dts file into the
> > tree as it is parsed, so the merge logic is spread over the rules for
> > each kind of fragment. This makes it hard to follow and to change.
> >
> > As a first step towards moving this logic into livetree.c, move the
> > action for a top-level /delete-node/ into a new merge_delete_node(),
> > passing it a struct fragment for the reference. Add a fragment_ref rule
> > to build this.
> >
> > The output does not change.
> >
> > Assisted-by: Claude:claude-opus-5-5
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> >
> >  dtc-parser.y | 26 +++++++++++++++++---------
> >  dtc.h        |  1 +
> >  livetree.c   | 19 +++++++++++++++++++
> >  3 files changed, 37 insertions(+), 9 deletions(-)
> >
> > diff --git a/dtc-parser.y b/dtc-parser.y
> > index e47f80d..61301a6 100644
> > --- a/dtc-parser.y
> > +++ b/dtc-parser.y
> > @@ -47,6 +47,7 @@ static bool is_ref_relative(const char *ref)
> >       struct node *node;
> >       struct node *nodelist;
> >       struct reserve_info *re;
> > +     struct fragment *fragment;
> >       uint64_t integer;
> >       unsigned int flags;
> >  }
> > @@ -80,6 +81,7 @@ static bool is_ref_relative(const char *ref)
> >  %type <prop> propdef
> >  %type <proplist> proplist
> >  %type <labelref> dt_ref
> > +%type <fragment> fragment_ref
> >
> >  %type <node> devicetree
> >  %type <node> nodedef
> > @@ -157,6 +159,17 @@ memreserve:
> >
> >  dt_ref: DT_LABEL_REF | DT_PATH_REF;
> >
> > +fragment_ref:
> > +       DT_LABEL_REF
> > +             {
> > +                     $$ = build_fragment_ref($1, false, &@$);
> > +             }
> > +     | DT_PATH_REF
> > +             {
> > +                     $$ = build_fragment_ref($1, true, &@$);
> > +             }
> > +     ;
> > +
>
> This seems weird.  If the point is to allow either kind of ref, we
> already have the dt_ref nonterminal for that.  If the point is to
> build fragments, rather than just refs, then why do it here before you
> have the information you need to properly build the fragment?

Oh yes, will fix.

>
> >  devicetree:
> >         '/' nodedef
> >               {
> > @@ -236,16 +249,11 @@ devicetree:
> >                       }
> >                       $$ = $1;
> >               }
> > -     | devicetree DT_DEL_NODE dt_ref ';'
> > +     | devicetree DT_DEL_NODE fragment_ref ';'
> >               {
> > -                     struct node *target = get_node_by_ref($1, $3);
> > -
> > -                     if (target)
> > -                             delete_node(target);
> > -                     else
> > -                             ERROR(&@3, "Label or path %s not found", $3);
> > -
> > -
> > +                     $3->type = FRAGMENT_DELETE_NODE;
> > +                     if (merge_delete_node($1, $3))
> > +                             treesource_error = true;
> >                       $$ = $1;
> >               }
> >       | devicetree DT_OMIT_NO_REF dt_ref ';'
> > diff --git a/dtc.h b/dtc.h
> > index c0d8e8f..9f9076d 100644
> > --- a/dtc.h
> > +++ b/dtc.h
> > @@ -368,6 +368,7 @@ struct fragment *build_fragment_ref(char *ref, bool path_ref,
> >                                   struct srcpos *srcpos);
> >  struct fragment *append_fragment(struct fragment *list, struct fragment *new);
> >  void free_fragment(struct fragment *f);
> > +bool merge_delete_node(struct node *dt, struct fragment *f);
> >
> >  struct dt_info {
> >       unsigned int dtsflags;
> > diff --git a/livetree.c b/livetree.c
> > index 982f0a6..5c9ec48 100644
> > --- a/livetree.c
> > +++ b/livetree.c
> > @@ -542,6 +542,25 @@ struct dt_info *build_dt_info(unsigned int dtsflags,
> >       return dti;
> >  }
> >
> > +/*
> > + * Delete the node which a /delete-node/ fragment refers to
> > + *
> > + * Returns true if there is an error, which has been reported.
> > + */
> > +bool merge_delete_node(struct node *dt, struct fragment *f)
> > +{
> > +     struct node *target = get_node_by_ref(dt, f->ref);
>
> assert() that f->type == DELETE_NODE, maybe?

Yes, added

>
> > +     if (!target) {
> > +             srcpos_error(f->ref_srcpos, "Error",
> > +                          "Label or path %s not found", f->ref);
> > +             return true;
> > +     }
> > +     delete_node(target);
> > +
> > +     return false;
> > +}
> > +
> >  /*
> >   * Tree accessor functions
> >   */
> > --
> > 2.43.0
> >
> >
>

[..]

Regards,
Simon

Re: [PATCH 07/13] dtc: Move path-reference handling out of the grammar

[Simon Glass]

Hi David,

On Tue, 3 Nov 2026 at 00:34, David Gibson <david@gibson.dropbear.id.au> wrote:
>
> On Sun, Nov 01, 2026 at 09:37:01AM -0700, Simon Glass wrote:
> > The grammar action for a top-level &{/path} { ... } holds its merge
> > logic, including making an overlay fragment in a plugin.
> >
> > Move it into a new merge_path_ref(), passing it a struct fragment for
> > the reference and node, built in the same way as for a label reference.
> >
> > The output does not change.
> >
> > Assisted-by: Claude:claude-opus-5-5
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> >
> >  dtc-parser.y | 17 +++++------------
> >  dtc.h        |  1 +
> >  livetree.c   | 36 ++++++++++++++++++++++++++++++++++++
> >  3 files changed, 42 insertions(+), 12 deletions(-)
> >
> > diff --git a/dtc-parser.y b/dtc-parser.y
> > index 1a5ad12..cca3ce1 100644
> > --- a/dtc-parser.y
> > +++ b/dtc-parser.y
> > @@ -206,23 +206,16 @@ devicetree:
> >               }
> >       | devicetree DT_PATH_REF nodedef
> >               {
> > +                     struct fragment *f = build_fragment_ref($2, true, &@2);
> > +
> > +                     f->node = $3;
> >                       /*
> >                        * We rely on the rule being always:
> >                        *   versioninfo plugindecl memreserves devicetree
> >                        * so $-1 is what we want (plugindecl)
> >                        */
> > -                     if ($<flags>-1 & DTSF_PLUGIN) {
> > -                             if (is_ref_relative($2))
> > -                                     ERROR(&@2, "Label-relative reference %s not supported in plugin", $2);
> > -                             add_orphan_node($1, $3, $2);
> > -                     } else {
> > -                             struct node *target = get_node_by_ref($1, $2);
> > -
> > -                             if (target)
> > -                                     merge_nodes(target, $3);
> > -                             else
> > -                                     ERROR(&@2, "Label or path %s not found", $2);
> > -                     }
> > +                     if (merge_path_ref($1, f, $<flags>-1 & DTSF_PLUGIN))
> > +                             treesource_error = true;
> >                       $$ = $1;
> >               }
> >       | devicetree DT_LABEL_REF nodedef
> > diff --git a/dtc.h b/dtc.h
> > index 256017f..5af1d1f 100644
> > --- a/dtc.h
> > +++ b/dtc.h
> > @@ -369,6 +369,7 @@ struct fragment *build_fragment_ref(char *ref, bool path_ref,
> >  struct fragment *append_fragment(struct fragment *list, struct fragment *new);
> >  void free_fragment(struct fragment *f);
> >  bool is_ref_relative(const char *ref);
> > +bool merge_path_ref(struct node *dt, struct fragment *f, bool plugin);
> >  bool merge_label_ref(struct node *dt, struct fragment *f, bool plugin);
> >  bool merge_delete_node(struct node *dt, struct fragment *f);
> >  bool merge_omit_if_no_ref(struct node *dt, struct fragment *f);
> > diff --git a/livetree.c b/livetree.c
> > index 2a01d49..b0362e4 100644
> > --- a/livetree.c
> > +++ b/livetree.c
> > @@ -548,6 +548,42 @@ bool is_ref_relative(const char *ref)
> >       return ref[0] != '/' && strchr(&ref[1], '/');
> >  }
> >
> > +/*
> > + * Merge a fragment which refers to a path into the node at that path. In a
> > + * plugin, the fragment becomes an overlay fragment instead.
> > + *
> > + * Returns true if there is an error, which has been reported.
> > + */
> > +bool merge_path_ref(struct node *dt, struct fragment *f, bool plugin)
>
> It would make more sense to me for merge_path_ref() and
> merge_label_ref() to be combined, working out which they need to do
> from the contents of the ref itself.
>
> I suspect that will also allow the grammar rules to be combined for
> the two cases.

Yes, I see, a bit more refactoring so I will split this up into a few
patches. That also highlights another missing test and case...

>
> > +{
> > +     struct node *target;
> > +
> > +     if (plugin) {
> > +             bool error = false;
> > +
> > +             if (is_ref_relative(f->ref)) {
> > +                     srcpos_error(f->ref_srcpos, "Error",
> > +                                  "Label-relative reference %s not supported in plugin",
> > +                                  f->ref);
> > +                     error = true;
> > +             }
> > +             /* Add a fragment@N node which applies the node at the path */
> > +             add_orphan_node(dt, f->node, f->ref);
> > +
> > +             return error;
> > +     }
> > +
> > +     target = get_node_by_ref(dt, f->ref);
> > +     if (!target) {
> > +             srcpos_error(f->ref_srcpos, "Error",
> > +                          "Label or path %s not found", f->ref);
> > +             return true;
> > +     }
> > +     merge_nodes(target, f->node);
> > +
> > +     return false;
> > +}
> > +
> >  /*
> >   * Merge a fragment which refers to a label into the node with that label. In
> >   * a plugin, a label which is not found becomes an overlay fragment.
> > --
> > 2.43.0
> >
> >

Regards,
SImon

Re: [PATCH 08/13] dtc: Move labelled-reference handling out of the grammar

[Simon Glass]

Hi David,

On Tue, 3 Nov 2026 at 00:34, David Gibson <david@gibson.dropbear.id.au> wrote:
>
> On Sun, Nov 01, 2026 at 09:37:02AM -0700, Simon Glass wrote:
> > The grammar action for a top-level label: &ref { ... } holds its merge
> > logic, which adds the label to the node referred to.
> >
> > Move it into a new merge_labelled_ref(), passing it a struct fragment
> > from fragment_ref, with the label and node added.
> >
> > The output does not change.
> >
> > Assisted-by: Claude:claude-opus-5-5
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> >
> >  dtc-parser.y | 18 +++++++-----------
> >  dtc.h        |  2 ++
> >  livetree.c   | 28 ++++++++++++++++++++++++++++
> >  3 files changed, 37 insertions(+), 11 deletions(-)
> >
> > diff --git a/dtc-parser.y b/dtc-parser.y
> > index cca3ce1..0e6c4c2 100644
> > --- a/dtc-parser.y
> > +++ b/dtc-parser.y
> > @@ -190,18 +190,14 @@ devicetree:
> >                                                 ""),
> >                                       $2, $1);
> >               }
> > -     | devicetree DT_LABEL dt_ref nodedef
> > +     | devicetree DT_LABEL fragment_ref nodedef
>
> Usual comments on fragment_ref.

Yes

>
> >               {
> > -                     struct node *target = get_node_by_ref($1, $3);
> > -
> > -                     if (($<flags>-1 & DTSF_PLUGIN) && is_ref_relative($3))
> > -                             ERROR(&@2, "Label-relative reference %s not supported in plugin", $3);
> > -
> > -                     if (target) {
> > -                             add_label(&target->labels, $2);
> > -                             merge_nodes(target, $4);
> > -                     } else
> > -                             ERROR(&@3, "Label or path %s not found", $3);
> > +                     $3->label = $2;
> > +                     $3->label_srcpos = srcpos_copy(&@2);
> > +                     $3->node = $4;
> > +                     if (merge_labelled_ref($1, $3,
> > +                                            $<flags>-1 & DTSF_PLUGIN))
> > +                             treesource_error = true;
> >                       $$ = $1;
> >               }
> >       | devicetree DT_PATH_REF nodedef
> > diff --git a/dtc.h b/dtc.h
> > index 5af1d1f..75b1c0f 100644
> > --- a/dtc.h
> > +++ b/dtc.h
> > @@ -369,6 +369,8 @@ struct fragment *build_fragment_ref(char *ref, bool path_ref,
> >  struct fragment *append_fragment(struct fragment *list, struct fragment *new);
> >  void free_fragment(struct fragment *f);
> >  bool is_ref_relative(const char *ref);
> > +bool merge_labelled_ref(struct node *dt, struct fragment *f,
> > +                     bool plugin);
> >  bool merge_path_ref(struct node *dt, struct fragment *f, bool plugin);
> >  bool merge_label_ref(struct node *dt, struct fragment *f, bool plugin);
> >  bool merge_delete_node(struct node *dt, struct fragment *f);
> > diff --git a/livetree.c b/livetree.c
> > index b0362e4..76474cd 100644
> > --- a/livetree.c
> > +++ b/livetree.c
> > @@ -548,6 +548,34 @@ bool is_ref_relative(const char *ref)
> >       return ref[0] != '/' && strchr(&ref[1], '/');
> >  }
> >
> > +/*
> > + * Merge a fragment of the form 'label: &ref { ... }' into the node which ref
> > + * refers to, adding the label to that node
> > + *
> > + * Returns true if there is an error, which has been reported.
> > + */
> > +bool merge_labelled_ref(struct node *dt, struct fragment *f, bool plugin)
>
> This again seems like the fragment itself contains the information to
> do the right thing for path/label or labelled fragments, rather than
> having separate functions for each case.

Yes I'm following that pattern now.

>
> > +{
> > +     struct node *target = get_node_by_ref(dt, f->ref);
> > +     bool error = false;
> > +
> > +     if (plugin && is_ref_relative(f->ref)) {
> > +             srcpos_error(f->label_srcpos, "Error",
> > +                          "Label-relative reference %s not supported in plugin",
> > +                          f->ref);
> > +             error = true;
> > +     }
> > +     if (!target) {
> > +             srcpos_error(f->ref_srcpos, "Error",
> > +                          "Label or path %s not found", f->ref);
> > +             return true;
> > +     }
> > +     add_label(&target->labels, f->label);
> > +     merge_nodes(target, f->node);
> > +
> > +     return error;
> > +}
> > +
> >  /*
> >   * Merge a fragment which refers to a path into the node at that path. In a
> >   * plugin, the fragment becomes an overlay fragment instead.
> > --
> > 2.43.0
> >
> >
>

Regards,
Simon

Re: [PATCH 02/13] dtc: Add a list for the top-level fragments of a dts file

[Simon Glass]

Hi David,

On Mon, 2 Nov 2026 at 02:55, David Gibson <david@gibson.dropbear.id.au> wrote:
>
> On Sun, Nov 01, 2026 at 09:36:56AM -0700, Simon Glass wrote:
> > A dts file is made up of top-level fragments: the root node, later
> > definitions of a node by label or path, and /delete-node/ or
> > /omit-if-no-ref/ with a reference. We would like to deal with each
> > fragment on its own before it is merged, for example to apply deletes
> > consistently or to run some checks on each one, but that requires
> > keeping the fragments apart. At present the parser merges each one into
> > the tree as soon as it is parsed.
>
> Btw, I'm extremely open to a better name than "fragment" if one occurs
> to you.  I believe I originally thought of these as "overlays" but
> that's confusing since runtime overlays were invented.

I did look at this as I had the same thought, but ended up going with
fragment as I didn't come up with any great idesa in the two minutes I
thought about it. Here is my new attempt:

section - doesn't really match Elf's use of the term, since it isn't
referring to a part of the output.
statement - matches the syntax in a sense (statements end in ';') but
it is too generic and I would find it confusing
block - so, so generic
stanza - a bit out there, but at least it would not be confusing
layer - too confusing with overlay (since the things you overlay are
layers), even though it does fit the process that the fragments go
through (including the idea that order matters)

If any of those seem appealing let me know :-)

>
> > Add a struct fragment to hold one of these, functions to build one, to
> > append it to a list and to free it, and a list of them in struct dt_info
> >
> > These are not used yet.
> >
> > Assisted-by: Claude:claude-opus-5-5
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> >
> >  dtc.h      | 47 ++++++++++++++++++++++++++++++++++++++++++
> >  livetree.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 107 insertions(+)
> >
> > diff --git a/dtc.h b/dtc.h
> > index 473552e..c0d8e8f 100644
> > --- a/dtc.h
> > +++ b/dtc.h
> > @@ -322,12 +322,59 @@ struct reserve_info *chain_reserve_entry(struct reserve_info *first,
> >  struct reserve_info *add_reserve_entry(struct reserve_info *list,
> >                                      struct reserve_info *new);
> >
> > +/*
> > + * A fragment is one top-level part of a dts file, after the header and any
> > + * memory reservations: the root node, a later definition of a node which
> > + * refers to it by label or path, or a /delete-node/ or /omit-if-no-ref/ with
> > + * a reference. Fragments are kept apart until they are merged into a single
> > + * tree.
> > + *
> > + * This is not the same as an overlay fragment (a fragment@N node), although
> > + * in a plugin a fragment may become one when it is merged.
> > + */
> > +
> > +enum fragment_type {
> > +     FRAGMENT_ROOT,          /* / { ... }; */
>
> Why do we need a special type for ROOT, rather than setting ref == "/"?

I'll change this,. I was just separating the case but the code doesn't
really gain much.

Note that we need to handle &{/} in a plugin, so I think this we need
ref == NULL rather than "/".

>
> > +     FRAGMENT_REF,           /* [label:] &ref { ... }; */
> > +     FRAGMENT_DELETE_NODE,   /* /delete-node/ &ref; */
> > +     FRAGMENT_OMIT_IF_NO_REF, /* /omit-if-no-ref/ &ref; */
> > +};
> > +
> > +/*
> > + * Which fields are used depends on the type:
> > + *
> > + *   FRAGMENT_ROOT            node
> > + *   FRAGMENT_REF             ref, path_ref, ref_srcpos and node, plus label
> > + *                            and label_srcpos if there is a label
> > + *   FRAGMENT_DELETE_NODE     ref, path_ref and ref_srcpos
> > + *   FRAGMENT_OMIT_IF_NO_REF  ref, path_ref and ref_srcpos
> > + *
> > + * Unused fields are NULL or false.
> > + */
> > +struct fragment {
> > +     enum fragment_type type;
> > +     char *ref;                      /* label or path referred to */
> > +     bool path_ref;                  /* ref is a path, i.e. &{...} */
>
> This can be derived from ref, so I don't think we need to store it.

Right, we can use ref[0] == '/' to get the bits we need.

>
> > +     struct srcpos *ref_srcpos;      /* position of ref */
> > +     char *label;                    /* label to add to the target */
> > +     struct srcpos *label_srcpos;    /* position of label */
> > +     struct node *node;              /* the node definition */
> > +
> > +     struct fragment *next;          /* NULL at the end of the list */
> > +};
> > +
> > +struct fragment *build_fragment(enum fragment_type type, struct node *node);
> > +struct fragment *build_fragment_ref(char *ref, bool path_ref,
> > +                                 struct srcpos *srcpos);
> > +struct fragment *append_fragment(struct fragment *list, struct fragment *new);
> > +void free_fragment(struct fragment *f);
> >
> >  struct dt_info {
> >       unsigned int dtsflags;
> >       struct reserve_info *reservelist;
> >       uint32_t boot_cpuid_phys;
> >       struct node *dt;                /* the device tree */
> > +     struct fragment *fragments;     /* not yet merged into dt */
> >       const char *outname;            /* filename being written to, "-" for stdout */
> >  };
> >
> > diff --git a/livetree.c b/livetree.c
> > index abe73de..982f0a6 100644
> > --- a/livetree.c
> > +++ b/livetree.c
> > @@ -448,6 +448,65 @@ struct reserve_info *add_reserve_entry(struct reserve_info *list,
> >       return list;
> >  }
> >
> > +/* Create a fragment of the given type, holding an optional node */
> > +struct fragment *build_fragment(enum fragment_type type, struct node *node)
>
> Since there's no ref provided, presumably this must of be
> FRAGMENT_ROOT, so why a type parameter?\


>
> > +{
> > +     struct fragment *new = xmalloc(sizeof(*new));
> > +
> > +     memset(new, '\0', sizeof(*new));
> > +
> > +     new->type = type;
> > +     new->node = node;
> > +
> > +     return new;
> > +}
> > +
> > +/*
> > + * Create a fragment which refers to a label, or to a path if path_ref. Its
> > + * node is NULL, so the caller must either set the node or change the type to
> > + * FRAGMENT_DELETE_NODE or FRAGMENT_OMIT_IF_NO_REF
>
> That seems a clumsy interface.  Maybe different constructor functions
> for the different types, that take all the necessary pieces?

Yes, that's better, thanks.

>
> > + */
> > +struct fragment *build_fragment_ref(char *ref, bool path_ref,
> > +                                 struct srcpos *srcpos)
>
> Here too, path_ref should be derivable from ref.

Right.

[..]

Regards,
Simon

Re: [PATCH 12/13] dtc: Merge the fragments of a dts file after parsing

[Simon Glass]

Hi,

On Tue, 3 Nov 2026 at 23:51, David Gibson <david@gibson.dropbear.id.au> wrote:
>
> On Tue, Nov 03, 2026 at 06:07:53PM +0100, Herve Codina wrote:
> > On Sun,  1 Nov 2026 09:37:06 -0700
> > Simon Glass <sjg@chromium.org> wrote:
> >
> > > We would like to deal with each top-level fragment of a dts file before
> > > it is merged into the tree, but that requires the fragments to be kept
> > > apart until the whole file is parsed. At present the parser merges each
> > > one as soon as it is parsed. The grammar actions also need to know
> > > whether the file is a plugin, which they find using $<flags>-1
> > >
> > > Have the grammar build a list of the fragments instead, in source order,
> > > and merge them with a new merge_fragments() once the whole file is
> > > parsed. This calls merge_fragment() for each one, so the output does not
> > > change, and frees each fragment once it is merged.
> > >
> > > The $<flags>-1 term is no longer needed. Within a devicetree action it
> > > reaches back up the parser stack to the value of headers, i.e. the flags
> > > from /dts-v1/ and /plugin/. The sourcefile action passes the same value
> > > to build_dt_info(), which stores it in dti->dtsflags, so
> > > merge_fragments() can use that instead.
> > >
> > > There are two differences in the errors. A syntax error later in the
> > > file now stops dtc before any earlier reference errors are reported, and
> > > reference errors now come after any other errors which the parser
> > > reports, such as a division by zero, rather than in source order.
> > >
> > > dtc gives the same output and messages as before for all 4162 .dts and
> > > .dtso files in Linux v7.3-12284-g3799eeb906c9, and for the dts files in
> > > tests/
> > >
> > > Suggested-by: David Gibson <david@gibson.dropbear.id.au>
> > > Link: https://lore.kernel.org/devicetree-compiler/auK0ERvxGCJGSc3p@gractus.seuss/
> > > Assisted-by: Claude:claude-opus-5-5
> > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > ---
> > >
> > >  dtc-parser.y | 43 +++++++++++++------------------------------
> > >  dtc.h        |  2 +-
> > >  livetree.c   | 36 ++++++++++++++++++++++++++++++++++--
> > >  3 files changed, 48 insertions(+), 33 deletions(-)
> > >
> > > diff --git a/dtc-parser.y b/dtc-parser.y
> > > index 1ed9731..c1aa4ad 100644
> > > --- a/dtc-parser.y
> > > +++ b/dtc-parser.y
> > > @@ -24,22 +24,6 @@ extern void yyerror(char const *s);
> > >  extern struct dt_info *parser_output;
> > >  extern bool treesource_error;
> > >
> > > -/*
> > > - * Merge a fragment into the tree as soon as it is parsed. The flags come
> > > - * from the actions using $<flags>-1, which relies on the rule being always:
> > > - *   headers memreserves devicetree
> > > - * so $-1 is the value of headers
> > > - */
> > > -static struct node *merge_parsed_fragment(struct node *dt, struct fragment *f,
> > > -                                     unsigned int flags)
> > > -{
> > > -   if (merge_fragment(&dt, f, flags & DTSF_PLUGIN))
> > > -           treesource_error = true;
> > > -   free_fragment(f);
> > > -
> > > -   return dt;
> > > -}
> > > -
> > >  %}
> > >
> > >  %union {
> > > @@ -94,7 +78,7 @@ static struct node *merge_parsed_fragment(struct node *dt, struct fragment *f,
> > >  %type <labelref> dt_ref
> > >  %type <fragment> fragment_ref
> > >
> > > -%type <node> devicetree
> > > +%type <fragment> devicetree
> > >  %type <node> nodedef
> > >  %type <node> subnode
> > >  %type <nodelist> subnodes
> > > @@ -119,8 +103,10 @@ static struct node *merge_parsed_fragment(struct node *dt, struct fragment *f,
> > >  sourcefile:
> > >       headers memreserves devicetree
> > >             {
> > > -                   parser_output = build_dt_info($1, $2, $3,
> > > -                                                 guess_boot_cpuid($3));
> > > +                   parser_output = build_dt_info($1, $2, NULL, 0);
> > > +                   parser_output->fragments = $3;
> > > +                   if (merge_fragments(parser_output))
> > > +                           treesource_error = true;
> >
> > Why not just pass the fragment list to build_dt_info() instead of using the
> > parser_output->fragments and later in merge_fragments() setting this same field
> > to NULL once the merge is done?
>
> As I noted elswhere, the point of this series is to delay the merge,
> and it will need to be after build_dt_info() to do what we want.

I'm going to try to expand the cover letter as I didn't really cover this well.

Regards,
Simon