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

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

[Simon Glass]

(this series is now far too long but I said I will send it - will split
up for the next version)

The parser merges each top-level fragment of a dts file into the tree as
soon as it is parsed, so nothing can look at a fragment on its own. Here
a fragment is a top-level part of the source, such as / { ... }; or
&label { ... };, not an overlay's fragment@N node.

This causes several problems:

- A repeated node or property name gives a check failure in the first
  fragment, but silently overwrites when it is in a later fragment
- Deletes cannot be dealt with consistently
- Checks which need the whole tree give false positives for a plugin,
  yet checks which do not need it cannot run before the merge

The ultimate aim is to run most checks on each fragment before it is
merged, and only those which need the whole tree afterwards. For this to
work, we need the merge to come after dt_from_source() and
build_dt_info() have returned.

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 for
main() to merge. Finally, add a plugin's overlay fragments (fragment@N
nodes) from main() once the merge is complete, rather than during it.

The output only changes for &{label} { ... } in a plugin, where label is
in the plugin itself, which is now merged in the same way as
&label { ... }, and for a plugin with a / { ... } fragment after one
which refers to the base tree, where the fragment@N nodes now come last.
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, that reference errors
now come after any other errors from the parser, and that a plugin can
no longer refer to a fragment@N node by its name, or to a node in one by
its path.

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. Before
the fragment@N nodes move to after the merge, it also adds tests for
referring to one by its name or path. At each commit, make check passes
and dtc gives the same output for every .dts and .dtso file in Linux
v7.3-12284-g3799eeb906c9, with the same messages apart from the changes
above.

This series only moves the merge. It still happens straight after the
input is read, before any checks, so none of the problems above are
fixed yet, and all fragments still come from source rather than a dtbo.

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/

Changes in v3:
- Change the subject, since the tests are not only for errors
- Add merge-overlay-order.dts to check deletes and order with fragments
- Check the final message when the parser finds an error
- Add new patch to add the overlay fragments from merge_fragments()
- Add new patch to check references to a generated overlay fragment
- Add new patch to add the overlay fragments after merging a plugin

Changes in v2:
- Reword four comments in the test files
- Add merge-root-plugin.dts to check / { ... } against &{/} { ... }
- Add merge-label-base.dts to check a label which is not in the plugin
- Add merge-label-braces.dts to check &{label} in a plugin
- Add merge-label-order.dts to check the order of &{label} and &label
- Add new patch for srcpos_string()
- Replace FRAGMENT_ROOT and FRAGMENT_REF with FRAGMENT_NODE
- Use a NULL ref for the root node
- Drop path_ref in favour of checking the ref for a leading '/'
- Add a constructor for each kind of fragment, taking all its fields
- Add the list to struct dt_info later, when it is first used
- Change the subject from 'Add a list' to 'Add a struct'
- Use dt_ref instead of fragment_ref
- Return true on success from merge_delete_node()
- Check the fragment type with assert()
- Use dt_ref instead of fragment_ref
- Return true on success from merge_omit_if_no_ref()
- Check the fragment type with assert()
- Rename merge_label_ref() to merge_ref()
- Build the fragment in one step, with build_fragment_ref()
- Return true on success from merge_ref()
- Check the fragment type with assert()
- Add new patch for &{label} in a plugin
- Combine merge_path_ref() into merge_ref()
- Combine the rules for label and path references, using dt_ref
- Combine merge_labelled_ref() into merge_ref()
- Use dt_ref instead of fragment_ref
- Use dt_ref instead of fragment_ref
- Return true on success from merge_first_ref()
- Use the new constructors and return true on success
- Use dt_ref instead of fragment_ref
- Combine the label and path rules earlier, in the path-reference patch
- Return true from check_relative_ref() if the reference is OK
- Pass the list to build_dt_info(), adding it to struct dt_info here
- Return true on success from merge_fragments()
- Update for merge_fragments() returning true on success
- Explain the aim of the series and what it does not do

Simon Glass (18):
  tests: Check how the fragments of a dts file are merged
  srcpos: Allow srcpos_string() to accept NULL
  dtc: Add a struct 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: Merge &{label} in a plugin as with &label
  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: Add the overlay fragments from merge_fragments()
  tests: Check references to a generated overlay fragment
  dtc: Add the overlay fragments after merging a plugin

 dtc-parser.y                       | 109 ++-------
 dtc.c                              |   3 +
 dtc.h                              |  57 ++++-
 flattree.c                         |   2 +-
 fstree.c                           |   2 +-
 livetree.c                         | 364 ++++++++++++++++++++++++++++-
 srcpos.c                           |   3 +
 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-label-base.dts         |  10 +
 tests/merge-label-braces.dts       |  12 +
 tests/merge-label-order.dts        |  17 ++
 tests/merge-labelled-plugin.dts    |  12 +
 tests/merge-labelled-relative.dts  |  14 ++
 tests/merge-omit-missing.dts       |   9 +
 tests/merge-overlay-name.dts       |  14 ++
 tests/merge-overlay-order.dts      |  19 ++
 tests/merge-overlay-path.dts       |  13 ++
 tests/merge-parse-error.dts        |  17 ++
 tests/merge-path-ref-missing.dts   |  11 +
 tests/merge-path-relative.dts      |  12 +
 tests/merge-root-plugin.dts        |  12 +
 tests/run_tests.sh                 |  89 ++++++-
 treesource.c                       |   5 +-
 26 files changed, 750 insertions(+), 98 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-label-base.dts
 create mode 100644 tests/merge-label-braces.dts
 create mode 100644 tests/merge-label-order.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-overlay-name.dts
 create mode 100644 tests/merge-overlay-order.dts
 create mode 100644 tests/merge-overlay-path.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
 create mode 100644 tests/merge-root-plugin.dts

---
base-commit: 54e547173bf0f2a4cc35eba8b5738caf38b9230f
branch: dt-forest3

-- 
2.43.0

[PATCH v3 01/18] tests: Check how the fragments of a dts file are merged

[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, that a labelled reference within a
plugin adds the label, that / { ... } and &{/} { ... } differ in a
plugin, and that a label in a plugin becomes an overlay fragment if it
is not found or is written as &{label}, which applies after any local
merges, since nothing covers these either. Also check that a node in an
overlay fragment can be deleted by its label, and where the overlay
fragment goes among the plugin's own nodes.

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

Changes in v3:
- Change the subject, since the tests are not only for errors
- Add merge-overlay-order.dts to check deletes and order with fragments

Changes in v2:
- Reword four comments in the test files
- Add merge-root-plugin.dts to check / { ... } against &{/} { ... }
- Add merge-label-base.dts to check a label which is not in the plugin
- Add merge-label-braces.dts to check &{label} in a plugin
- Add merge-label-order.dts to check the order of &{label} and &label

 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-label-base.dts         | 10 ++++
 tests/merge-label-braces.dts       | 12 +++++
 tests/merge-label-order.dts        | 17 +++++++
 tests/merge-labelled-plugin.dts    | 12 +++++
 tests/merge-labelled-relative.dts  | 14 ++++++
 tests/merge-omit-missing.dts       |  9 ++++
 tests/merge-overlay-order.dts      | 19 ++++++++
 tests/merge-parse-error.dts        | 17 +++++++
 tests/merge-path-ref-missing.dts   | 11 +++++
 tests/merge-path-relative.dts      | 12 +++++
 tests/merge-root-plugin.dts        | 12 +++++
 tests/run_tests.sh                 | 74 ++++++++++++++++++++++++++++++
 16 files changed, 261 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-label-base.dts
 create mode 100644 tests/merge-label-braces.dts
 create mode 100644 tests/merge-label-order.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-overlay-order.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
 create mode 100644 tests/merge-root-plugin.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..1e97088
--- /dev/null
+++ b/tests/merge-errors.dts
@@ -0,0 +1,14 @@
+/dts-v1/;
+
+/ {
+	label: node {
+	};
+};
+
+/* All three of the references below are bad, and each one is reported */
+&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..eece7a7
--- /dev/null
+++ b/tests/merge-first-ref-relative.dts
@@ -0,0 +1,10 @@
+/dts-v1/;
+/plugin/;
+
+/* A plugin cannot define a node using 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-label-base.dts b/tests/merge-label-base.dts
new file mode 100644
index 0000000..1a0a8bb
--- /dev/null
+++ b/tests/merge-label-base.dts
@@ -0,0 +1,10 @@
+/dts-v1/;
+/plugin/;
+
+/ {
+};
+
+/* A label which is not in the plugin may be in the base tree */
+&{baselabel} {
+	prop;
+};
diff --git a/tests/merge-label-braces.dts b/tests/merge-label-braces.dts
new file mode 100644
index 0000000..d5f8d0f
--- /dev/null
+++ b/tests/merge-label-braces.dts
@@ -0,0 +1,12 @@
+/dts-v1/;
+/plugin/;
+
+/ {
+	label: node {
+	};
+};
+
+/* This becomes an overlay fragment, even though the label is in the plugin */
+&{label} {
+	prop;
+};
diff --git a/tests/merge-label-order.dts b/tests/merge-label-order.dts
new file mode 100644
index 0000000..f7923a0
--- /dev/null
+++ b/tests/merge-label-order.dts
@@ -0,0 +1,17 @@
+/dts-v1/;
+/plugin/;
+
+&basenode {
+	label: foo {
+		p = "first";
+	};
+};
+
+/* The fragment for &{label} is applied last, so "second" wins */
+&{label} {
+	p = "second";
+};
+
+&label {
+	p = "third";
+};
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..9bedfd9
--- /dev/null
+++ b/tests/merge-labelled-relative.dts
@@ -0,0 +1,14 @@
+/dts-v1/;
+/plugin/;
+
+/ {
+	label: node {
+		subnode {
+		};
+	};
+};
+
+/* Not allowed even though the reference 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-overlay-order.dts b/tests/merge-overlay-order.dts
new file mode 100644
index 0000000..2490157
--- /dev/null
+++ b/tests/merge-overlay-order.dts
@@ -0,0 +1,19 @@
+/dts-v1/;
+/plugin/;
+
+&basenode {
+	label: foo {
+	};
+
+	bar {
+	};
+};
+
+/* A node in the plugin itself, after the overlay fragment */
+/ {
+	node {
+	};
+};
+
+/* A label in an overlay fragment can be referred to */
+/delete-node/ &label;
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..c4e00a4
--- /dev/null
+++ b/tests/merge-path-relative.dts
@@ -0,0 +1,12 @@
+/dts-v1/;
+/plugin/;
+
+/ {
+	label: node {
+	};
+};
+
+/* A plugin cannot define a node using a label-relative path reference */
+&{label/node} {
+	prop;
+};
diff --git a/tests/merge-root-plugin.dts b/tests/merge-root-plugin.dts
new file mode 100644
index 0000000..f67355e
--- /dev/null
+++ b/tests/merge-root-plugin.dts
@@ -0,0 +1,12 @@
+/dts-v1/;
+/plugin/;
+
+/* This is in the root node of the plugin */
+/ {
+	prop1;
+};
+
+/* This is an overlay fragment for the root node of the base tree */
+&{/} {
+	prop2;
+};
diff --git a/tests/run_tests.sh b/tests/run_tests.sh
index f750faa..2d392e2 100755
--- a/tests/run_tests.sh
+++ b/tests/run_tests.sh
@@ -770,6 +770,80 @@ dtc_tests () {
     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" -m "Label or path nosuchnode not found" \
 	-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
+
+    # In a plugin, &{label} becomes an overlay fragment, even if it is found
+    run_dtc_test -I dts -O dtb -o merge-label-braces.test.dtb \
+	"$SRCDIR/merge-label-braces.dts"
+    run_fdtget_test "" merge-label-braces.test.dtb /fragment@0/__overlay__ prop
+    run_fdtget_test "node\nfragment@0\n__local_fixups__" merge-label-braces.test.dtb -l /
+
+    # The fragment for &{label} is applied after any local merges
+    run_dtc_test -I dts -O dtb -o merge-label-order.test.dtb \
+	"$SRCDIR/merge-label-order.dts"
+    run_fdtget_test "third" merge-label-order.test.dtb /fragment@0/__overlay__/foo p
+    run_fdtget_test "second" merge-label-order.test.dtb /fragment@1/__overlay__ p
+
+    # In a plugin, / { ... } and &{/} { ... } are not the same thing
+    run_dtc_test -I dts -O dtb -o merge-root-plugin.test.dtb \
+	"$SRCDIR/merge-root-plugin.dts"
+    run_fdtget_test "" merge-root-plugin.test.dtb / prop1
+    run_fdtget_test "/" merge-root-plugin.test.dtb /fragment@0 target-path
+    run_fdtget_test "" merge-root-plugin.test.dtb /fragment@0/__overlay__ prop2
+
+    # In a plugin, a label which is not found becomes an overlay fragment
+    run_dtc_test -I dts -O dtb -o merge-label-base.test.dtb \
+	"$SRCDIR/merge-label-base.dts"
+    run_fdtget_test "" merge-label-base.test.dtb /fragment@0/__overlay__ prop
+    run_fdtget_test "/fragment@0:target:0" merge-label-base.test.dtb /__fixups__ baselabel
+
+    # In a plugin, a node in an overlay fragment can be deleted by its label,
+    # and the overlay fragment comes before the later node in the plugin
+    run_dtc_test -I dts -O dtb -o merge-overlay-order.test.dtb \
+	"$SRCDIR/merge-overlay-order.dts"
+    run_fdtget_test "fragment@0\nnode\n__fixups__" merge-overlay-order.test.dtb -l /
+    run_fdtget_test "bar" merge-overlay-order.test.dtb -l /fragment@0/__overlay__
+
     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 v3 02/18] srcpos: Allow srcpos_string() to accept NULL

[Simon Glass]

A position is not always available: a node read from a dtb has none, and
top-level fragments may eventually come from a dtbo as well as from
source. At present srcpos_string() crashes if it has no position, so
each caller which might not have one needs to check, as those in
checks.c do.

Return a placeholder in this case, using the same text as
srcpos_string_comment() does, so that srcpos_error() can be used whether
or not there is a position.

No caller passes NULL at present, so this has no test yet.

Suggested-by: David Gibson <david@gibson.dropbear.id.au>
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v2)

Changes in v2:
- Add new patch for srcpos_string()

 srcpos.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/srcpos.c b/srcpos.c
index fef892f..c42eba8 100644
--- a/srcpos.c
+++ b/srcpos.c
@@ -324,6 +324,9 @@ srcpos_string(struct srcpos *pos)
 	const char *fname = "<no-file>";
 	char *pos_str;
 
+	if (!pos)
+		return xstrdup("<no-file>:<no-line>");
+
 	if (pos->file && pos->file->name)
 		fname = pos->file->name;
 
-- 
2.43.0

[PATCH v3 03/18] dtc: Add a struct 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, with a function to build
each kind, and functions to append one to a list and to free it.

These are not used yet.

Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Simon Glass <sjg@chromium.org>
---
David, you asked why the root needs a special type, rather than setting
ref to "/". This drops the type, but uses a NULL ref rather than "/",
since the two differ in a plugin: / { ... } adds to the plugin's own
root, whereas &{/} { ... } becomes a fragment@N node for the root of the
base tree. Using "/" for both would change the output.
merge-root-plugin.dts checks this.

(no changes since v2)

Changes in v2:
- Replace FRAGMENT_ROOT and FRAGMENT_REF with FRAGMENT_NODE
- Use a NULL ref for the root node
- Drop path_ref in favour of checking the ref for a leading '/'
- Add a constructor for each kind of fragment, taking all its fields
- Add the list to struct dt_info later, when it is first used
- Change the subject from 'Add a list' to 'Add a struct'

 dtc.h      | 50 ++++++++++++++++++++++++++++++++
 livetree.c | 85 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 135 insertions(+)

diff --git a/dtc.h b/dtc.h
index 473552e..f15a162 100644
--- a/dtc.h
+++ b/dtc.h
@@ -322,6 +322,56 @@ 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_NODE,		/* / { ... }; or [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_NODE            node, plus ref and ref_srcpos unless it is the
+ *                            root node, plus label and label_srcpos if there
+ *                            is a label
+ *   FRAGMENT_DELETE_NODE     ref and ref_srcpos
+ *   FRAGMENT_OMIT_IF_NO_REF  ref and ref_srcpos
+ *
+ * Unused fields are NULL. For / { ... }; the ref is NULL rather than "/", since
+ * in a plugin &{/} { ... }; refers to the root of the base tree instead.
+ */
+struct fragment {
+	enum fragment_type type;
+	char *ref;			/* label or path referred to */
+	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_root(struct node *node);
+struct fragment *build_fragment_ref(char *label, struct srcpos *label_srcpos,
+				    char *ref, struct srcpos *ref_srcpos,
+				    struct node *node);
+struct fragment *build_fragment_delete_node(char *ref,
+					    struct srcpos *ref_srcpos);
+struct fragment *build_fragment_omit_if_no_ref(char *ref,
+					       struct srcpos *ref_srcpos);
+struct fragment *append_fragment(struct fragment *list, struct fragment *new);
+void free_fragment(struct fragment *f);
 
 struct dt_info {
 	unsigned int dtsflags;
diff --git a/livetree.c b/livetree.c
index abe73de..85725d3 100644
--- a/livetree.c
+++ b/livetree.c
@@ -448,6 +448,91 @@ struct reserve_info *add_reserve_entry(struct reserve_info *list,
 	return list;
 }
 
+/* Create a fragment of the given type, with its other fields NULL */
+static struct fragment *new_fragment(enum fragment_type type, char *ref,
+				     struct srcpos *ref_srcpos)
+{
+	struct fragment *new = xmalloc(sizeof(*new));
+
+	memset(new, '\0', sizeof(*new));
+
+	new->type = type;
+	new->ref = ref;
+	new->ref_srcpos = srcpos_copy(ref_srcpos);
+
+	return new;
+}
+
+/* Create a fragment for / { ... }; */
+struct fragment *build_fragment_root(struct node *node)
+{
+	struct fragment *new = new_fragment(FRAGMENT_NODE, NULL, NULL);
+
+	new->node = node;
+
+	return new;
+}
+
+/*
+ * Create a fragment for [label:] &ref { ... }; where label and label_srcpos are
+ * NULL if there is no label
+ */
+struct fragment *build_fragment_ref(char *label, struct srcpos *label_srcpos,
+				    char *ref, struct srcpos *ref_srcpos,
+				    struct node *node)
+{
+	struct fragment *new = new_fragment(FRAGMENT_NODE, ref, ref_srcpos);
+
+	new->label = label;
+	new->label_srcpos = srcpos_copy(label_srcpos);
+	new->node = node;
+
+	return new;
+}
+
+/* Create a fragment for /delete-node/ &ref; */
+struct fragment *build_fragment_delete_node(char *ref,
+					    struct srcpos *ref_srcpos)
+{
+	return new_fragment(FRAGMENT_DELETE_NODE, ref, ref_srcpos);
+}
+
+/* Create a fragment for /omit-if-no-ref/ &ref; */
+struct fragment *build_fragment_omit_if_no_ref(char *ref,
+					       struct srcpos *ref_srcpos)
+{
+	return new_fragment(FRAGMENT_OMIT_IF_NO_REF, ref, ref_srcpos);
+}
+
+/* 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;
-- 
2.43.0

[PATCH v3 04/18] 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.

The output does not change.

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

(no changes since v2)

Changes in v2:
- Use dt_ref instead of fragment_ref
- Return true on success from merge_delete_node()
- Check the fragment type with assert()

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

diff --git a/dtc-parser.y b/dtc-parser.y
index e47f80d..bba98b6 100644
--- a/dtc-parser.y
+++ b/dtc-parser.y
@@ -238,14 +238,11 @@ devicetree:
 		}
 	| devicetree DT_DEL_NODE dt_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);
-
+			struct fragment *f;
 
+			f = build_fragment_delete_node($3, &@3);
+			if (!merge_delete_node($1, f))
+				treesource_error = true;
 			$$ = $1;
 		}
 	| devicetree DT_OMIT_NO_REF dt_ref ';'
diff --git a/dtc.h b/dtc.h
index f15a162..5dca8bc 100644
--- a/dtc.h
+++ b/dtc.h
@@ -372,6 +372,7 @@ struct fragment *build_fragment_omit_if_no_ref(char *ref,
 					       struct srcpos *ref_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 85725d3..35beee0 100644
--- a/livetree.c
+++ b/livetree.c
@@ -567,6 +567,26 @@ struct dt_info *build_dt_info(unsigned int dtsflags,
 	return dti;
 }
 
+/*
+ * Delete the node which a /delete-node/ fragment refers to
+ *
+ * Returns true on success, false on 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(f->type == FRAGMENT_DELETE_NODE);
+	if (!target) {
+		srcpos_error(f->ref_srcpos, "Error",
+			     "Label or path %s not found", f->ref);
+		return false;
+	}
+	delete_node(target);
+
+	return true;
+}
+
 /*
  * Tree accessor functions
  */
-- 
2.43.0

[PATCH v3 05/18] 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>
---

(no changes since v2)

Changes in v2:
- Use dt_ref instead of fragment_ref
- Return true on success from merge_omit_if_no_ref()
- Check the fragment type with assert()

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

diff --git a/dtc-parser.y b/dtc-parser.y
index bba98b6..1aaff42 100644
--- a/dtc-parser.y
+++ b/dtc-parser.y
@@ -247,14 +247,11 @@ devicetree:
 		}
 	| devicetree DT_OMIT_NO_REF dt_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);
-
+			struct fragment *f;
 
+			f = build_fragment_omit_if_no_ref($3, &@3);
+			if (!merge_omit_if_no_ref($1, f))
+				treesource_error = true;
 			$$ = $1;
 		}
 	;
diff --git a/dtc.h b/dtc.h
index 5dca8bc..22a050a 100644
--- a/dtc.h
+++ b/dtc.h
@@ -373,6 +373,7 @@ struct fragment *build_fragment_omit_if_no_ref(char *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 35beee0..a54009c 100644
--- a/livetree.c
+++ b/livetree.c
@@ -587,6 +587,27 @@ bool merge_delete_node(struct node *dt, struct fragment *f)
 	return true;
 }
 
+/*
+ * Mark the node which an /omit-if-no-ref/ fragment refers to, so that it is
+ * dropped if nothing refers to it
+ *
+ * Returns true on success, false on 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);
+
+	assert(f->type == FRAGMENT_OMIT_IF_NO_REF);
+	if (!target) {
+		srcpos_error(f->ref_srcpos, "Error",
+			     "Label or path %s not found", f->ref);
+		return false;
+	}
+	omit_node_if_unused(target);
+
+	return true;
+}
+
 /*
  * Tree accessor functions
  */
-- 
2.43.0

[PATCH v3 06/18] 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>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
---

(no changes since v1)

 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 1aaff42..9b9b6db 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 22a050a..8c4d7bf 100644
--- a/dtc.h
+++ b/dtc.h
@@ -372,6 +372,7 @@ struct fragment *build_fragment_omit_if_no_ref(char *ref,
 					       struct srcpos *ref_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 a54009c..3ff4e14 100644
--- a/livetree.c
+++ b/livetree.c
@@ -567,6 +567,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 v3 07/18] 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_ref(), passing it a struct fragment for the
reference and node.

The output does not change.

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

(no changes since v2)

Changes in v2:
- Rename merge_label_ref() to merge_ref()
- Build the fragment in one step, with build_fragment_ref()
- Return true on success from merge_ref()
- Check the fragment type with assert()

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

diff --git a/dtc-parser.y b/dtc-parser.y
index 9b9b6db..c58a66d 100644
--- a/dtc-parser.y
+++ b/dtc-parser.y
@@ -214,21 +214,16 @@ devicetree:
 		}
 	| devicetree DT_LABEL_REF nodedef
 		{
-			struct node *target = get_node_by_ref($1, $2);
+			struct fragment *f;
 
-			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 = build_fragment_ref(NULL, NULL, $2, &@2, $3);
+			/*
+			 * We rely on the rule being always:
+			 *   versioninfo plugindecl memreserves devicetree
+			 * so $-1 is what we want (plugindecl)
+			 */
+			if (!merge_ref($1, f, $<flags>-1 & DTSF_PLUGIN))
+				treesource_error = true;
 			$$ = $1;
 		}
 	| devicetree DT_DEL_NODE dt_ref ';'
diff --git a/dtc.h b/dtc.h
index 8c4d7bf..9b4389b 100644
--- a/dtc.h
+++ b/dtc.h
@@ -373,6 +373,7 @@ struct fragment *build_fragment_omit_if_no_ref(char *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_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 3ff4e14..179866d 100644
--- a/livetree.c
+++ b/livetree.c
@@ -573,6 +573,32 @@ 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 on success, false on an error, which has been reported.
+ */
+bool merge_ref(struct node *dt, struct fragment *f, bool plugin)
+{
+	struct node *target = get_node_by_ref(dt, f->ref);
+
+	assert(f->type == FRAGMENT_NODE);
+	if (target) {
+		merge_nodes(target, f->node);
+		return true;
+	}
+	if (!plugin) {
+		srcpos_error(f->ref_srcpos, "Error",
+			     "Label or path %s not found", f->ref);
+		return false;
+	}
+	/* 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 true;
+}
+
 /*
  * Delete the node which a /delete-node/ fragment refers to
  *
-- 
2.43.0

[PATCH v3 08/18] dtc: Merge &{label} in a plugin as with &label

[Simon Glass]

In a plugin, &label { ... } is merged into the labelled node if the
label is in the plugin, as of commit 0fcffda15e9f ("Merge nodes with
local target label references"). But &{label} { ... } always becomes a
separate fragment@N node, since anything in braces is lexed as a path
reference. This adds a fragment which is not needed, which that commit
aims to avoid, and fails with FDT_ERR_NOTFOUND if the label is not
within a fragment.

Treat a reference as a path only if it starts with '/', so that both
forms are merged into the node. This changes the output for such a file.
Applying an overlay which already works gives the same result, apart
from the order of the properties and that a later &label { ... } now
overrides an earlier &{label} { ... }, as the source order suggests.

Other references are not affected. An absolute path still becomes a
fragment@N node, as does a label which is not in the plugin, since it
may be in the base tree.

This also means that dtc does not need to record whether a reference is
written in braces. The merge code, which is about to move out of the
grammar, can tell a path from a label by looking at the reference.

Update the merge-label-braces.dts and merge-label-order.dts tests to
match.

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

(no changes since v2)

Changes in v2:
- Add new patch for &{label} in a plugin

 dtc-parser.y                 | 10 +++++++---
 tests/merge-label-braces.dts |  2 +-
 tests/merge-label-order.dts  |  2 +-
 tests/run_tests.sh           | 10 +++++-----
 4 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/dtc-parser.y b/dtc-parser.y
index c58a66d..3614d71 100644
--- a/dtc-parser.y
+++ b/dtc-parser.y
@@ -193,6 +193,8 @@ devicetree:
 		}
 	| devicetree DT_PATH_REF nodedef
 		{
+			struct node *target = get_node_by_ref($1, $2);
+
 			/*
 			 * We rely on the rule being always:
 			 *   versioninfo plugindecl memreserves devicetree
@@ -201,10 +203,12 @@ devicetree:
 			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);
+				/* A path is in the base tree and a label may be */
+				if ($2[0] == '/' || !target)
+					add_orphan_node($1, $3, $2);
+				else
+					merge_nodes(target, $3);
 			} else {
-				struct node *target = get_node_by_ref($1, $2);
-
 				if (target)
 					merge_nodes(target, $3);
 				else
diff --git a/tests/merge-label-braces.dts b/tests/merge-label-braces.dts
index d5f8d0f..569e7f8 100644
--- a/tests/merge-label-braces.dts
+++ b/tests/merge-label-braces.dts
@@ -6,7 +6,7 @@
 	};
 };
 
-/* This becomes an overlay fragment, even though the label is in the plugin */
+/* A label in the plugin itself is merged, however it is written */
 &{label} {
 	prop;
 };
diff --git a/tests/merge-label-order.dts b/tests/merge-label-order.dts
index f7923a0..40fa668 100644
--- a/tests/merge-label-order.dts
+++ b/tests/merge-label-order.dts
@@ -7,7 +7,7 @@
 	};
 };
 
-/* The fragment for &{label} is applied last, so "second" wins */
+/* The last definition wins */
 &{label} {
 	p = "second";
 };
diff --git a/tests/run_tests.sh b/tests/run_tests.sh
index 2d392e2..0aaa29c 100755
--- a/tests/run_tests.sh
+++ b/tests/run_tests.sh
@@ -812,17 +812,17 @@ dtc_tests () {
     run_fdtget_test "/node" merge-labelled-plugin.test.dtb /__symbols__ newlabel
     run_fdtget_test "value" merge-labelled-plugin.test.dtb /node prop
 
-    # In a plugin, &{label} becomes an overlay fragment, even if it is found
+    # In a plugin, &{label} is the same as &label
     run_dtc_test -I dts -O dtb -o merge-label-braces.test.dtb \
 	"$SRCDIR/merge-label-braces.dts"
-    run_fdtget_test "" merge-label-braces.test.dtb /fragment@0/__overlay__ prop
-    run_fdtget_test "node\nfragment@0\n__local_fixups__" merge-label-braces.test.dtb -l /
+    run_fdtget_test "" merge-label-braces.test.dtb /node prop
+    run_fdtget_test "node" merge-label-braces.test.dtb -l /
 
-    # The fragment for &{label} is applied after any local merges
+    # The last definition wins, with no fragment for &{label}
     run_dtc_test -I dts -O dtb -o merge-label-order.test.dtb \
 	"$SRCDIR/merge-label-order.dts"
     run_fdtget_test "third" merge-label-order.test.dtb /fragment@0/__overlay__/foo p
-    run_fdtget_test "second" merge-label-order.test.dtb /fragment@1/__overlay__ p
+    run_fdtget_test "fragment@0\n__fixups__" merge-label-order.test.dtb -l /
 
     # In a plugin, / { ... } and &{/} { ... } are not the same thing
     run_dtc_test -I dts -O dtb -o merge-root-plugin.test.dtb \
-- 
2.43.0

[PATCH v3 09/18] 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 merge_ref(), which can tell a path from a label by looking
at the reference, and use a single rule for both kinds of reference.

The output does not change.

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

(no changes since v2)

Changes in v2:
- Combine merge_path_ref() into merge_ref()
- Combine the rules for label and path references, using dt_ref

 dtc-parser.y | 27 +--------------------------
 livetree.c   | 33 +++++++++++++++++++++++----------
 2 files changed, 24 insertions(+), 36 deletions(-)

diff --git a/dtc-parser.y b/dtc-parser.y
index 3614d71..3ec9254 100644
--- a/dtc-parser.y
+++ b/dtc-parser.y
@@ -191,32 +191,7 @@ devicetree:
 				ERROR(&@3, "Label or path %s not found", $3);
 			$$ = $1;
 		}
-	| devicetree DT_PATH_REF nodedef
-		{
-			struct node *target = get_node_by_ref($1, $2);
-
-			/*
-			 * 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);
-				/* A path is in the base tree and a label may be */
-				if ($2[0] == '/' || !target)
-					add_orphan_node($1, $3, $2);
-				else
-					merge_nodes(target, $3);
-			} else {
-				if (target)
-					merge_nodes(target, $3);
-				else
-					ERROR(&@2, "Label or path %s not found", $2);
-			}
-			$$ = $1;
-		}
-	| devicetree DT_LABEL_REF nodedef
+	| devicetree dt_ref nodedef
 		{
 			struct fragment *f;
 
diff --git a/livetree.c b/livetree.c
index 179866d..2e9a3b2 100644
--- a/livetree.c
+++ b/livetree.c
@@ -574,29 +574,42 @@ bool is_ref_relative(const char *ref)
 }
 
 /*
- * 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.
+ * Merge a fragment which refers to a label or path into the node it refers to.
+ *
+ * In a plugin, a path always refers to the base tree and a label may do so, if
+ * it is not found. In both cases, the fragment becomes an overlay fragment.
  *
  * Returns true on success, false on an error, which has been reported.
  */
 bool merge_ref(struct node *dt, struct fragment *f, bool plugin)
 {
-	struct node *target = get_node_by_ref(dt, f->ref);
+	struct node *target;
+	bool ok = true;
 
 	assert(f->type == FRAGMENT_NODE);
-	if (target) {
-		merge_nodes(target, f->node);
-		return true;
+	if (plugin) {
+		if (is_ref_relative(f->ref)) {
+			srcpos_error(f->ref_srcpos, "Error",
+				     "Label-relative reference %s not supported in plugin",
+				     f->ref);
+			ok = false;
+		}
+		if (f->ref[0] == '/' || !get_node_by_ref(dt, f->ref)) {
+			/* Add a fragment@N node for the reference */
+			add_orphan_node(dt, f->node, f->ref);
+			return ok;
+		}
 	}
-	if (!plugin) {
+
+	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 false;
 	}
-	/* The label may be in the base tree, so add a fragment@N node for it */
-	add_orphan_node(dt, f->node, f->ref);
+	merge_nodes(target, f->node);
 
-	return true;
+	return ok;
 }
 
 /*
-- 
2.43.0

[PATCH v3 10/18] 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 merge_ref(), passing it a struct fragment with the label.

The output does not change.

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

(no changes since v2)

Changes in v2:
- Combine merge_labelled_ref() into merge_ref()
- Use dt_ref instead of fragment_ref

 dtc-parser.y | 13 ++++---------
 livetree.c   | 14 ++++++++++----
 2 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/dtc-parser.y b/dtc-parser.y
index 3ec9254..8d505a1 100644
--- a/dtc-parser.y
+++ b/dtc-parser.y
@@ -179,16 +179,11 @@ devicetree:
 		}
 	| devicetree DT_LABEL dt_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);
+			struct fragment *f;
 
-			if (target) {
-				add_label(&target->labels, $2);
-				merge_nodes(target, $4);
-			} else
-				ERROR(&@3, "Label or path %s not found", $3);
+			f = build_fragment_ref($2, &@2, $3, &@3, $4);
+			if (!merge_ref($1, f, $<flags>-1 & DTSF_PLUGIN))
+				treesource_error = true;
 			$$ = $1;
 		}
 	| devicetree dt_ref nodedef
diff --git a/livetree.c b/livetree.c
index 2e9a3b2..7ab9437 100644
--- a/livetree.c
+++ b/livetree.c
@@ -574,10 +574,12 @@ bool is_ref_relative(const char *ref)
 }
 
 /*
- * Merge a fragment which refers to a label or path into the node it refers to.
+ * Merge a fragment which refers to a label or path into the node it refers to,
+ * adding any label in the fragment to that node.
  *
  * In a plugin, a path always refers to the base tree and a label may do so, if
- * it is not found. In both cases, the fragment becomes an overlay fragment.
+ * it is not found. In both cases, the fragment becomes an overlay fragment,
+ * unless it adds a label, which needs a node in the plugin itself.
  *
  * Returns true on success, false on an error, which has been reported.
  */
@@ -589,12 +591,14 @@ bool merge_ref(struct node *dt, struct fragment *f, bool plugin)
 	assert(f->type == FRAGMENT_NODE);
 	if (plugin) {
 		if (is_ref_relative(f->ref)) {
-			srcpos_error(f->ref_srcpos, "Error",
+			srcpos_error(f->label ? f->label_srcpos : f->ref_srcpos,
+				     "Error",
 				     "Label-relative reference %s not supported in plugin",
 				     f->ref);
 			ok = false;
 		}
-		if (f->ref[0] == '/' || !get_node_by_ref(dt, f->ref)) {
+		if (!f->label &&
+		    (f->ref[0] == '/' || !get_node_by_ref(dt, f->ref))) {
 			/* Add a fragment@N node for the reference */
 			add_orphan_node(dt, f->node, f->ref);
 			return ok;
@@ -607,6 +611,8 @@ bool merge_ref(struct node *dt, struct fragment *f, bool plugin)
 			     "Label or path %s not found", f->ref);
 		return false;
 	}
+	if (f->label)
+		add_label(&target->labels, f->label);
 	merge_nodes(target, f->node);
 
 	return ok;
-- 
2.43.0

[PATCH v3 11/18] 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 for
the reference and node. 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>
---

(no changes since v2)

Changes in v2:
- Use dt_ref instead of fragment_ref
- Return true on success from merge_first_ref()

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

diff --git a/dtc-parser.y b/dtc-parser.y
index 8d505a1..abeee0d 100644
--- a/dtc-parser.y
+++ b/dtc-parser.y
@@ -163,19 +163,16 @@ devicetree:
 		}
 	| dt_ref nodedef
 		{
+			struct fragment *f;
+
+			f = build_fragment_ref(NULL, NULL, $1, &@1, $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(&$$, f, $<flags>-1 & DTSF_PLUGIN))
+				treesource_error = true;
 		}
 	| devicetree DT_LABEL dt_ref nodedef
 		{
diff --git a/dtc.h b/dtc.h
index 9b4389b..1befd5c 100644
--- a/dtc.h
+++ b/dtc.h
@@ -372,7 +372,7 @@ struct fragment *build_fragment_omit_if_no_ref(char *ref,
 					       struct srcpos *ref_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_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 7ab9437..26d39df 100644
--- a/livetree.c
+++ b/livetree.c
@@ -568,11 +568,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 on success, false on an error, which has been reported.
+ */
+bool merge_first_ref(struct node **dtp, struct fragment *f, bool plugin)
+{
+	bool ok = true;
+
+	if (!plugin) {
+		srcpos_error(f->node->srcpos, "Error",
+			     "Label or path %s not found", f->ref);
+		ok = false;
+	} else if (is_ref_relative(f->ref)) {
+		srcpos_error(f->node->srcpos, "Error",
+			     "Label-relative reference %s not supported in plugin",
+			     f->ref);
+		ok = false;
+	}
+	/* 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 ok;
+}
+
 /*
  * Merge a fragment which refers to a label or path into the node it refers to,
  * adding any label in the fragment to that node.
-- 
2.43.0

[PATCH v3 12/18] 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 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>
---

(no changes since v2)

Changes in v2:
- Use the new constructors and return true on success
- Use dt_ref instead of fragment_ref
- Combine the label and path rules earlier, in the path-reference patch

 dtc-parser.y | 53 ++++++++++++++++++++++++++--------------------------
 dtc.h        |  5 +----
 livetree.c   | 42 +++++++++++++++++++++++++++++++++++++----
 3 files changed, 66 insertions(+), 34 deletions(-)

diff --git a/dtc-parser.y b/dtc-parser.y
index abeee0d..9cda518 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 {
@@ -155,65 +171,50 @@ dt_ref: DT_LABEL_REF | DT_PATH_REF;
 devicetree:
 	  '/' nodedef
 		{
-			$$ = name_node($2, "");
+			struct fragment *f = build_fragment_root($2);
+
+			$$ = merge_parsed_fragment(NULL, f, $<flags>-1);
 		}
 	| devicetree '/' nodedef
 		{
-			$$ = merge_nodes($1, $3);
+			struct fragment *f = build_fragment_root($3);
+
+			$$ = merge_parsed_fragment($1, f, $<flags>-1);
 		}
 	| dt_ref nodedef
 		{
 			struct fragment *f;
 
 			f = build_fragment_ref(NULL, NULL, $1, &@1, $2);
-			/*
-			 * We rely on the rule being always:
-			 *   versioninfo plugindecl memreserves devicetree
-			 * so $-1 is what we want (plugindecl)
-			 */
-			if (!merge_first_ref(&$$, f, $<flags>-1 & DTSF_PLUGIN))
-				treesource_error = true;
+			$$ = merge_parsed_fragment(NULL, f, $<flags>-1);
 		}
 	| devicetree DT_LABEL dt_ref nodedef
 		{
 			struct fragment *f;
 
 			f = build_fragment_ref($2, &@2, $3, &@3, $4);
-			if (!merge_ref($1, f, $<flags>-1 & DTSF_PLUGIN))
-				treesource_error = true;
-			$$ = $1;
+			$$ = merge_parsed_fragment($1, f, $<flags>-1);
 		}
 	| devicetree dt_ref nodedef
 		{
 			struct fragment *f;
 
 			f = build_fragment_ref(NULL, NULL, $2, &@2, $3);
-			/*
-			 * We rely on the rule being always:
-			 *   versioninfo plugindecl memreserves devicetree
-			 * so $-1 is what we want (plugindecl)
-			 */
-			if (!merge_ref($1, f, $<flags>-1 & DTSF_PLUGIN))
-				treesource_error = true;
-			$$ = $1;
+			$$ = merge_parsed_fragment($1, f, $<flags>-1);
 		}
 	| devicetree DT_DEL_NODE dt_ref ';'
 		{
 			struct fragment *f;
 
 			f = build_fragment_delete_node($3, &@3);
-			if (!merge_delete_node($1, f))
-				treesource_error = true;
-			$$ = $1;
+			$$ = merge_parsed_fragment($1, f, $<flags>-1);
 		}
 	| devicetree DT_OMIT_NO_REF dt_ref ';'
 		{
 			struct fragment *f;
 
 			f = build_fragment_omit_if_no_ref($3, &@3);
-			if (!merge_omit_if_no_ref($1, f))
-				treesource_error = true;
-			$$ = $1;
+			$$ = merge_parsed_fragment($1, f, $<flags>-1);
 		}
 	;
 
diff --git a/dtc.h b/dtc.h
index 1befd5c..e223bc2 100644
--- a/dtc.h
+++ b/dtc.h
@@ -372,10 +372,7 @@ struct fragment *build_fragment_omit_if_no_ref(char *ref,
 					       struct srcpos *ref_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_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 26d39df..a1d5d71 100644
--- a/livetree.c
+++ b/livetree.c
@@ -580,7 +580,7 @@ static bool is_ref_relative(const char *ref)
  *
  * Returns true on success, false on 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 ok = true;
 
@@ -611,7 +611,7 @@ bool merge_first_ref(struct node **dtp, struct fragment *f, bool plugin)
  *
  * Returns true on success, false on an error, which has been reported.
  */
-bool merge_ref(struct node *dt, struct fragment *f, bool plugin)
+static bool merge_ref(struct node *dt, struct fragment *f, bool plugin)
 {
 	struct node *target;
 	bool ok = true;
@@ -651,7 +651,7 @@ bool merge_ref(struct node *dt, struct fragment *f, bool plugin)
  *
  * Returns true on success, false on 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);
 
@@ -672,7 +672,7 @@ bool merge_delete_node(struct node *dt, struct fragment *f)
  *
  * Returns true on success, false on 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);
 
@@ -687,6 +687,40 @@ bool merge_omit_if_no_ref(struct node *dt, struct fragment *f)
 	return true;
 }
 
+/*
+ * 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 on success, false on 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->ref) {
+			*dtp = name_node(f->node, "");
+			return true;
+		}
+		return merge_first_ref(dtp, f, plugin);
+	}
+
+	switch (f->type) {
+	case FRAGMENT_NODE:
+		if (f->ref)
+			return merge_ref(dt, f, plugin);
+		*dtp = merge_nodes(dt, f->node);
+		break;
+	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 true;
+}
+
 /*
  * Tree accessor functions
  */
-- 
2.43.0

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

[Simon Glass]

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

(no changes since v2)

Changes in v2:
- Return true from check_relative_ref() if the reference is OK

 livetree.c | 75 +++++++++++++++++++++++++++++++++---------------------
 1 file changed, 46 insertions(+), 29 deletions(-)

diff --git a/livetree.c b/livetree.c
index a1d5d71..a70a7ba 100644
--- a/livetree.c
+++ b/livetree.c
@@ -573,6 +573,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 the reference is OK, false if there is an error
+ */
+static bool check_relative_ref(struct fragment *f, struct srcpos *pos)
+{
+	if (!is_ref_relative(f->ref))
+		return true;
+	srcpos_error(pos, "Error",
+		     "Label-relative reference %s not supported in plugin",
+		     f->ref);
+
+	return false;
+}
+
+/*
+ * 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
@@ -582,17 +616,14 @@ static bool is_ref_relative(const char *ref)
  */
 static bool merge_first_ref(struct node **dtp, struct fragment *f, bool plugin)
 {
-	bool ok = true;
+	bool ok;
 
-	if (!plugin) {
+	if (plugin) {
+		ok = check_relative_ref(f, f->node->srcpos);
+	} else {
 		srcpos_error(f->node->srcpos, "Error",
 			     "Label or path %s not found", f->ref);
 		ok = false;
-	} else if (is_ref_relative(f->ref)) {
-		srcpos_error(f->node->srcpos, "Error",
-			     "Label-relative reference %s not supported in plugin",
-			     f->ref);
-		ok = false;
 	}
 	/* Make a new, empty tree with a fragment@N node for the reference */
 	*dtp = add_orphan_node(name_node(build_node(NULL, NULL, NULL), ""),
@@ -618,13 +649,8 @@ static bool merge_ref(struct node *dt, struct fragment *f, bool plugin)
 
 	assert(f->type == FRAGMENT_NODE);
 	if (plugin) {
-		if (is_ref_relative(f->ref)) {
-			srcpos_error(f->label ? f->label_srcpos : f->ref_srcpos,
-				     "Error",
-				     "Label-relative reference %s not supported in plugin",
-				     f->ref);
-			ok = false;
-		}
+		ok = check_relative_ref(f, f->label ? f->label_srcpos :
+					f->ref_srcpos);
 		if (!f->label &&
 		    (f->ref[0] == '/' || !get_node_by_ref(dt, f->ref))) {
 			/* Add a fragment@N node for the reference */
@@ -633,12 +659,9 @@ static bool merge_ref(struct node *dt, struct fragment *f, bool plugin)
 		}
 	}
 
-	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 false;
-	}
 	if (f->label)
 		add_label(&target->labels, f->label);
 	merge_nodes(target, f->node);
@@ -653,14 +676,11 @@ static bool merge_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);
 
 	assert(f->type == FRAGMENT_DELETE_NODE);
-	if (!target) {
-		srcpos_error(f->ref_srcpos, "Error",
-			     "Label or path %s not found", f->ref);
+	if (!target)
 		return false;
-	}
 	delete_node(target);
 
 	return true;
@@ -674,14 +694,11 @@ 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);
 
 	assert(f->type == FRAGMENT_OMIT_IF_NO_REF);
-	if (!target) {
-		srcpos_error(f->ref_srcpos, "Error",
-			     "Label or path %s not found", f->ref);
+	if (!target)
 		return false;
-	}
 	omit_node_if_unused(target);
 
 	return true;
-- 
2.43.0

[PATCH v3 14/18] 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,
passing it to build_dt_info() to store in struct dt_info, 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.

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>
---
Apart from the two differences in the errors, dtc gives the same output
and messages as before for every .dts and .dtso file in Linux
v7.3-12284-g3799eeb906c9, and for the dts files in tests/

(no changes since v2)

Changes in v2:
- Pass the list to build_dt_info(), adding it to struct dt_info here
- Return true on success from merge_fragments()

 dtc-parser.y | 65 ++++++++++++++++------------------------------------
 dtc.h        |  6 +++--
 flattree.c   |  2 +-
 fstree.c     |  2 +-
 livetree.c   | 40 +++++++++++++++++++++++++++++---
 5 files changed, 63 insertions(+), 52 deletions(-)

diff --git a/dtc-parser.y b/dtc-parser.y
index 9cda518..0ff04c7 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 {
@@ -58,6 +42,7 @@ static struct node *merge_parsed_fragment(struct node *dt, struct fragment *f,
 	struct node *node;
 	struct node *nodelist;
 	struct reserve_info *re;
+	struct fragment *fragment;
 	uint64_t integer;
 	unsigned int flags;
 }
@@ -92,7 +77,7 @@ static struct node *merge_parsed_fragment(struct node *dt, struct fragment *f,
 %type <proplist> proplist
 %type <labelref> dt_ref
 
-%type <node> devicetree
+%type <fragment> devicetree
 %type <node> nodedef
 %type <node> subnode
 %type <nodelist> subnodes
@@ -117,8 +102,9 @@ 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, $3, 0);
+			if (!merge_fragments(parser_output))
+				treesource_error = true;
 		}
 	;
 
@@ -171,50 +157,39 @@ dt_ref: DT_LABEL_REF | DT_PATH_REF;
 devicetree:
 	  '/' nodedef
 		{
-			struct fragment *f = build_fragment_root($2);
-
-			$$ = merge_parsed_fragment(NULL, f, $<flags>-1);
+			$$ = build_fragment_root($2);
 		}
 	| devicetree '/' nodedef
 		{
-			struct fragment *f = build_fragment_root($3);
-
-			$$ = merge_parsed_fragment($1, f, $<flags>-1);
+			$$ = append_fragment($1, build_fragment_root($3));
 		}
 	| dt_ref nodedef
 		{
-			struct fragment *f;
-
-			f = build_fragment_ref(NULL, NULL, $1, &@1, $2);
-			$$ = merge_parsed_fragment(NULL, f, $<flags>-1);
+			$$ = build_fragment_ref(NULL, NULL, $1, &@1, $2);
 		}
 	| devicetree DT_LABEL dt_ref nodedef
 		{
-			struct fragment *f;
-
-			f = build_fragment_ref($2, &@2, $3, &@3, $4);
-			$$ = merge_parsed_fragment($1, f, $<flags>-1);
+			$$ = append_fragment($1, build_fragment_ref($2, &@2,
+								    $3, &@3,
+								    $4));
 		}
 	| devicetree dt_ref nodedef
 		{
-			struct fragment *f;
-
-			f = build_fragment_ref(NULL, NULL, $2, &@2, $3);
-			$$ = merge_parsed_fragment($1, f, $<flags>-1);
+			$$ = append_fragment($1, build_fragment_ref(NULL, NULL,
+								    $2, &@2,
+								    $3));
 		}
 	| devicetree DT_DEL_NODE dt_ref ';'
 		{
-			struct fragment *f;
-
-			f = build_fragment_delete_node($3, &@3);
-			$$ = merge_parsed_fragment($1, f, $<flags>-1);
+			$$ = append_fragment($1,
+					     build_fragment_delete_node($3,
+									&@3));
 		}
 	| devicetree DT_OMIT_NO_REF dt_ref ';'
 		{
-			struct fragment *f;
-
-			f = build_fragment_omit_if_no_ref($3, &@3);
-			$$ = merge_parsed_fragment($1, f, $<flags>-1);
+			$$ = append_fragment($1,
+					     build_fragment_omit_if_no_ref($3,
+									   &@3));
 		}
 	;
 
diff --git a/dtc.h b/dtc.h
index e223bc2..437efbc 100644
--- a/dtc.h
+++ b/dtc.h
@@ -372,13 +372,13 @@ struct fragment *build_fragment_omit_if_no_ref(char *ref,
 					       struct srcpos *ref_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;
 	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 */
 };
 
@@ -388,7 +388,9 @@ struct dt_info {
 
 struct dt_info *build_dt_info(unsigned int dtsflags,
 			      struct reserve_info *reservelist,
-			      struct node *tree, uint32_t boot_cpuid_phys);
+			      struct node *tree, struct fragment *fragments,
+			      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/flattree.c b/flattree.c
index f3b698c..aa183a0 100644
--- a/flattree.c
+++ b/flattree.c
@@ -923,5 +923,5 @@ struct dt_info *dt_from_blob(const char *fname)
 	if (get_subnode(tree, "__fixups__") || get_subnode(tree, "__local_fixups__"))
 		dtsflags |= DTSF_PLUGIN;
 
-	return build_dt_info(dtsflags, reservelist, tree, boot_cpuid_phys);
+	return build_dt_info(dtsflags, reservelist, tree, NULL, boot_cpuid_phys);
 }
diff --git a/fstree.c b/fstree.c
index 0f9a534..88ff4ba 100644
--- a/fstree.c
+++ b/fstree.c
@@ -72,5 +72,5 @@ struct dt_info *dt_from_fs(const char *dirname)
 	tree = read_fstree(dirname);
 	tree = name_node(tree, "");
 
-	return build_dt_info(DTSF_V1, NULL, tree, guess_boot_cpuid(tree));
+	return build_dt_info(DTSF_V1, NULL, tree, NULL, guess_boot_cpuid(tree));
 }
diff --git a/livetree.c b/livetree.c
index a70a7ba..f2c7703 100644
--- a/livetree.c
+++ b/livetree.c
@@ -552,7 +552,8 @@ static void fill_fullpaths(struct node *tree, const char *prefix)
 
 struct dt_info *build_dt_info(unsigned int dtsflags,
 			      struct reserve_info *reservelist,
-			      struct node *tree, uint32_t boot_cpuid_phys)
+			      struct node *tree, struct fragment *fragments,
+			      uint32_t boot_cpuid_phys)
 {
 	struct dt_info *dti;
 
@@ -560,9 +561,11 @@ struct dt_info *build_dt_info(unsigned int dtsflags,
 	dti->dtsflags = dtsflags;
 	dti->reservelist = reservelist;
 	dti->dt = tree;
+	dti->fragments = fragments;
 	dti->boot_cpuid_phys = boot_cpuid_phys;
 
-	fill_fullpaths(dti->dt, "");
+	if (tree)
+		fill_fullpaths(dti->dt, "");
 
 	return dti;
 }
@@ -710,7 +713,8 @@ static bool merge_omit_if_no_ref(struct node *dt, struct fragment *f)
  *
  * Returns true on success, false on 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;
 
@@ -738,6 +742,36 @@ bool merge_fragment(struct node **dtp, struct fragment *f, bool plugin)
 	return true;
 }
 
+/*
+ * Merge the fragments of a dts file into a single tree, in source order.
+ *
+ * Returns true on success, false on 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 ok = true;
+
+	if (!dti->fragments)
+		return true;
+
+	for (f = dti->fragments; f; f = next) {
+		next = f->next;
+		if (!merge_fragment(&dt, f, plugin))
+			ok = false;
+		free_fragment(f);
+	}
+
+	dti->dt = dt;
+	dti->fragments = NULL;
+	dti->boot_cpuid_phys = guess_boot_cpuid(dt);
+	fill_fullpaths(dti->dt, "");
+
+	return ok;
+}
+
 /*
  * Tree accessor functions
  */
-- 
2.43.0

[PATCH v3 15/18] 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. The merge-parse-error.dts
test checks this, with a fragment which refers to a missing node,
followed by a division by zero. Also check that dtc still gives up with
'Syntax error parsing input tree' in this case.

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

Changes in v3:
- Check the final message when the parser finds an error

Changes in v2:
- Update for merge_fragments() returning true on success

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

diff --git a/dtc-parser.y b/dtc-parser.y
index 0ff04c7..33a0078 100644
--- a/dtc-parser.y
+++ b/dtc-parser.y
@@ -103,8 +103,6 @@ sourcefile:
 	  headers memreserves devicetree
 		{
 			parser_output = build_dt_info($1, $2, NULL, $3, 0);
-			if (!merge_fragments(parser_output))
-				treesource_error = true;
 		}
 	;
 
diff --git a/dtc.c b/dtc.c
index e506837..73e5d76 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 0aaa29c..8391717 100755
--- a/tests/run_tests.sh
+++ b/tests/run_tests.sh
@@ -741,7 +741,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
@@ -805,6 +806,8 @@ dtc_tests () {
 	$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"
+    run_wrap_error_msg_test "Syntax error parsing input tree" \
+	$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 \
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

[PATCH v3 16/18] dtc: Add the overlay fragments from merge_fragments()

[Simon Glass]

In a plugin, merge_first_ref() and merge_ref() call add_orphan_node() to
turn a fragment which refers to the base tree into an overlay fragment
(a fragment@N node). The grammar actions have always done this, but it
mixes converting the plugin to the overlay format into the functions
which merge each fragment.

Have merge_fragment() tell its caller when a fragment is for the base
tree, and call add_orphan_node() from merge_fragments() instead, so that
none of the merge_*() functions does the conversion. Add a comment for
add_orphan_node() too.

This does not change the output.

Suggested-by: Herve Codina <herve.codina@bootlin.com>
Link: https://lore.kernel.org/devicetree-compiler/20261105100714.47bf0434@bootlin.com/
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Simon Glass <sjg@chromium.org>
---
dtc gives the same output and messages as before for every .dts and
.dtso file in Linux v7.3-12284-g3799eeb906c9

Changes in v3:
- Add new patch to add the overlay fragments from merge_fragments()

 livetree.c | 47 +++++++++++++++++++++++++++++++----------------
 1 file changed, 31 insertions(+), 16 deletions(-)

diff --git a/livetree.c b/livetree.c
index f2c7703..c5fec49 100644
--- a/livetree.c
+++ b/livetree.c
@@ -225,6 +225,15 @@ struct node *merge_nodes(struct node *old_node, struct node *new_node)
 	return old_node;
 }
 
+/*
+ * Add an overlay fragment (a fragment@N node) to the root of a plugin, for a
+ * node which refers to the base tree. The node becomes the fragment's
+ * __overlay__ subnode and ref gives its target, either a path (target-path)
+ * or a label (target, as a phandle reference). Fragments are numbered in the
+ * order they are added.
+ *
+ * Returns the root node, dt
+ */
 struct node * add_orphan_node(struct node *dt, struct node *new_node, char *ref)
 {
 	static unsigned int next_orphan_fragment = 0;
@@ -611,9 +620,9 @@ static struct node *lookup_ref(struct node *dt, struct fragment *f)
 }
 
 /*
- * 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.
+ * Make an empty tree at *dtp for 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 on success, false on an error, which has been reported.
  */
@@ -628,9 +637,7 @@ static bool merge_first_ref(struct node **dtp, struct fragment *f, bool plugin)
 			     "Label or path %s not found", f->ref);
 		ok = false;
 	}
-	/* 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);
+	*dtp = name_node(build_node(NULL, NULL, NULL), "");
 
 	return ok;
 }
@@ -640,12 +647,14 @@ static bool merge_first_ref(struct node **dtp, struct fragment *f, bool plugin)
  * adding any label in the fragment to that node.
  *
  * In a plugin, a path always refers to the base tree and a label may do so, if
- * it is not found. In both cases, the fragment becomes an overlay fragment,
- * unless it adds a label, which needs a node in the plugin itself.
+ * it is not found. In both cases, *for_basep is set to true so that the
+ * fragment becomes an overlay fragment, unless it adds a label, which needs a
+ * node in the plugin itself.
  *
  * Returns true on success, false on an error, which has been reported.
  */
-static bool merge_ref(struct node *dt, struct fragment *f, bool plugin)
+static bool merge_ref(struct node *dt, struct fragment *f, bool plugin,
+		      bool *for_basep)
 {
 	struct node *target;
 	bool ok = true;
@@ -656,8 +665,7 @@ static bool merge_ref(struct node *dt, struct fragment *f, bool plugin)
 					f->ref_srcpos);
 		if (!f->label &&
 		    (f->ref[0] == '/' || !get_node_by_ref(dt, f->ref))) {
-			/* Add a fragment@N node for the reference */
-			add_orphan_node(dt, f->node, f->ref);
+			*for_basep = true;
 			return ok;
 		}
 	}
@@ -709,12 +717,13 @@ static bool merge_omit_if_no_ref(struct node *dt, struct fragment *f)
 
 /*
  * 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.
+ * first fragment. A reference is looked up in the tree as merged so far. If
+ * the fragment is for the base tree, *for_basep is set to true.
  *
  * Returns true on success, false on an error, which has been reported.
  */
 static bool merge_fragment(struct node **dtp, struct fragment *f,
-			   bool plugin)
+			   bool plugin, bool *for_basep)
 {
 	struct node *dt = *dtp;
 
@@ -724,13 +733,14 @@ static bool merge_fragment(struct node **dtp, struct fragment *f,
 			*dtp = name_node(f->node, "");
 			return true;
 		}
+		*for_basep = true;
 		return merge_first_ref(dtp, f, plugin);
 	}
 
 	switch (f->type) {
 	case FRAGMENT_NODE:
 		if (f->ref)
-			return merge_ref(dt, f, plugin);
+			return merge_ref(dt, f, plugin, for_basep);
 		*dtp = merge_nodes(dt, f->node);
 		break;
 	case FRAGMENT_DELETE_NODE:
@@ -743,7 +753,8 @@ static bool merge_fragment(struct node **dtp, struct fragment *f,
 }
 
 /*
- * Merge the fragments of a dts file into a single tree, in source order.
+ * Merge the fragments of a dts file into a single tree, in source order. In a
+ * plugin, each fragment for the base tree becomes an overlay fragment.
  *
  * Returns true on success, false on errors, which have been reported.
  */
@@ -758,9 +769,13 @@ bool merge_fragments(struct dt_info *dti)
 		return true;
 
 	for (f = dti->fragments; f; f = next) {
+		bool for_base = false;
+
 		next = f->next;
-		if (!merge_fragment(&dt, f, plugin))
+		if (!merge_fragment(&dt, f, plugin, &for_base))
 			ok = false;
+		if (for_base)
+			add_orphan_node(dt, f->node, f->ref);
 		free_fragment(f);
 	}
 
-- 
2.43.0

[PATCH v3 17/18] tests: Check references to a generated overlay fragment

[Simon Glass]

In a plugin, a fragment can refer to an overlay fragment (a fragment@N
node) which dtc generates for an earlier fragment, either by giving its
name in a / { ... } fragment or by giving a path through it to
/delete-node/. No test covers either of these.

Add a test for each, checking that the / { ... } fragment adds to the
generated fragment@N node and that the path finds the node in it.
Neither is really desirable, since dtc generates these names and a
plugin should not rely on them, but the tests show what changes when the
fragment@N nodes are added after the merge.

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

Changes in v3:
- Add new patch to check references to a generated overlay fragment

 tests/merge-overlay-name.dts | 14 ++++++++++++++
 tests/merge-overlay-path.dts | 13 +++++++++++++
 tests/run_tests.sh           | 10 ++++++++++
 3 files changed, 37 insertions(+)
 create mode 100644 tests/merge-overlay-name.dts
 create mode 100644 tests/merge-overlay-path.dts

diff --git a/tests/merge-overlay-name.dts b/tests/merge-overlay-name.dts
new file mode 100644
index 0000000..be08268
--- /dev/null
+++ b/tests/merge-overlay-name.dts
@@ -0,0 +1,14 @@
+/dts-v1/;
+/plugin/;
+
+&basenode {
+	node {
+	};
+};
+
+/* Refers to the overlay fragment above by the name which dtc gives it */
+/ {
+	fragment@0 {
+		prop = <1>;
+	};
+};
diff --git a/tests/merge-overlay-path.dts b/tests/merge-overlay-path.dts
new file mode 100644
index 0000000..2f4c5fc
--- /dev/null
+++ b/tests/merge-overlay-path.dts
@@ -0,0 +1,13 @@
+/dts-v1/;
+/plugin/;
+
+&basenode {
+	node1 {
+	};
+
+	node2 {
+	};
+};
+
+/* Refers to a node in the overlay fragment above by its path */
+/delete-node/ &{/fragment@0/__overlay__/node1};
diff --git a/tests/run_tests.sh b/tests/run_tests.sh
index 8391717..92e4d2c 100755
--- a/tests/run_tests.sh
+++ b/tests/run_tests.sh
@@ -847,6 +847,16 @@ dtc_tests () {
     run_fdtget_test "fragment@0\nnode\n__fixups__" merge-overlay-order.test.dtb -l /
     run_fdtget_test "bar" merge-overlay-order.test.dtb -l /fragment@0/__overlay__
 
+    # In a plugin, a / { ... } fragment can add to an overlay fragment by
+    # giving the name which dtc generates for it, and a node in an overlay
+    # fragment can be deleted by its path
+    run_dtc_test -I dts -O dtb -o merge-overlay-name.test.dtb \
+	"$SRCDIR/merge-overlay-name.dts"
+    run_fdtget_test 1 merge-overlay-name.test.dtb /fragment@0 prop
+    run_dtc_test -I dts -O dtb -o merge-overlay-path.test.dtb \
+	"$SRCDIR/merge-overlay-path.dts"
+    run_fdtget_test "node2" merge-overlay-path.test.dtb -l /fragment@0/__overlay__
+
     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 v3 18/18] dtc: Add the overlay fragments after merging a plugin

[Simon Glass]

In a plugin, merge_fragments() turns each fragment which refers to the
base tree into an overlay fragment (a fragment@N node) as soon as it is
merged. This puts the fragment@N nodes into the tree which later
fragments are merged into, so converting the plugin to the overlay
format is still mixed with the merge.

Keep these fragments in dti->fragments instead and add the fragment@N
nodes from main() once the merge is complete. A later fragment can still
refer to a label in one of them, so look up a label in these fragments
as well as in the tree. Make add_orphan_node() return the fragment@N
node, so that its full path can be filled in.

This changes the output in three cases.

Firstly, where a / { ... } fragment comes after one which refers to the
base tree, the fragment@N nodes now come after the nodes which it adds,
rather than before them.

Secondly, a / { ... } fragment can no longer add to a fragment@N node by
giving its name, which now gives a duplicate-node-name error. dtc
generates these nodes and numbers them in order, so a plugin should not
rely on their names, and an error is better than silently changing a
generated fragment.

Finally, /delete-node/ and /omit-if-no-ref/ can no longer refer to a
node in an overlay fragment by its path, such as
&{/fragment@0/__overlay__/node}, since that path does not exist until
after the merge. The path also relies on a generated name, whereas a
label, which still works, refers to the node as written in the source.

Update the tests for these three cases.

Suggested-by: Herve Codina <herve.codina@bootlin.com>
Link: https://lore.kernel.org/devicetree-compiler/20261105100714.47bf0434@bootlin.com/
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Simon Glass <sjg@chromium.org>
---
None of these changes affects any .dts or .dtso file in Linux
v7.3-12284-g3799eeb906c9

Changes in v3:
- Add new patch to add the overlay fragments after merging a plugin

 dtc.c              |   1 +
 dtc.h              |   6 ++-
 livetree.c         | 105 ++++++++++++++++++++++++++++++++++-----------
 tests/run_tests.sh |  20 ++++-----
 4 files changed, 94 insertions(+), 38 deletions(-)

diff --git a/dtc.c b/dtc.c
index 73e5d76..13a9963 100644
--- a/dtc.c
+++ b/dtc.c
@@ -301,6 +301,7 @@ int main(int argc, char *argv[])
 
 	if (!merge_fragments(dti))
 		die("Unable to merge input tree\n");
+	add_overlay_fragments(dti);
 	dti->outname = outname;
 
 	if (depfile) {
diff --git a/dtc.h b/dtc.h
index 437efbc..0062e3d 100644
--- a/dtc.h
+++ b/dtc.h
@@ -330,7 +330,8 @@ struct reserve_info *add_reserve_entry(struct reserve_info *list,
  * 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.
+ * in a plugin a fragment which refers to the base tree becomes one after the
+ * others are merged.
  */
 
 enum fragment_type {
@@ -378,7 +379,7 @@ struct dt_info {
 	struct reserve_info *reservelist;
 	uint32_t boot_cpuid_phys;
 	struct node *dt;		/* the device tree */
-	struct fragment *fragments;	/* not yet merged into dt */
+	struct fragment *fragments;	/* not merged into dt */
 	const char *outname;		/* filename being written to, "-" for stdout */
 };
 
@@ -391,6 +392,7 @@ struct dt_info *build_dt_info(unsigned int dtsflags,
 			      struct node *tree, struct fragment *fragments,
 			      uint32_t boot_cpuid_phys);
 bool merge_fragments(struct dt_info *dti);
+void add_overlay_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 c5fec49..3413a26 100644
--- a/livetree.c
+++ b/livetree.c
@@ -232,7 +232,7 @@ struct node *merge_nodes(struct node *old_node, struct node *new_node)
  * or a label (target, as a phandle reference). Fragments are numbered in the
  * order they are added.
  *
- * Returns the root node, dt
+ * Returns the fragment@N node
  */
 struct node * add_orphan_node(struct node *dt, struct node *new_node, char *ref)
 {
@@ -262,7 +262,7 @@ struct node * add_orphan_node(struct node *dt, struct node *new_node, char *ref)
 	free(name);
 
 	add_child(dt, node);
-	return dt;
+	return node;
 }
 
 struct node *chain_node(struct node *first, struct node *list)
@@ -602,15 +602,37 @@ static bool check_relative_ref(struct fragment *f, struct srcpos *pos)
 	return false;
 }
 
+/*
+ * Look up a reference in the tree merged so far or, for a label, in base_frags,
+ * the fragments for the base tree so far, since a plugin may refer to a node in
+ * one of these
+ *
+ * Returns the node, or NULL if it is not found
+ */
+static struct node *find_ref(struct node *dt, struct fragment *base_frags,
+			     const char *ref)
+{
+	struct node *target = get_node_by_ref(dt, ref);
+	struct fragment *f;
+
+	if (ref[0] == '/')
+		return target;
+	for (f = base_frags; f && !target; f = f->next)
+		target = get_node_by_ref(f->node, ref);
+
+	return target;
+}
+
 /*
  * 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)
+static struct node *lookup_ref(struct node *dt, struct fragment *base_frags,
+			       struct fragment *f)
 {
-	struct node *target = get_node_by_ref(dt, f->ref);
+	struct node *target = find_ref(dt, base_frags, f->ref);
 
 	if (!target)
 		srcpos_error(f->ref_srcpos, "Error",
@@ -622,7 +644,7 @@ static struct node *lookup_ref(struct node *dt, struct fragment *f)
 /*
  * Make an empty tree at *dtp for 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.
+ * the fragment is kept to become an overlay fragment.
  *
  * Returns true on success, false on an error, which has been reported.
  */
@@ -653,8 +675,8 @@ static bool merge_first_ref(struct node **dtp, struct fragment *f, bool plugin)
  *
  * Returns true on success, false on an error, which has been reported.
  */
-static bool merge_ref(struct node *dt, struct fragment *f, bool plugin,
-		      bool *for_basep)
+static bool merge_ref(struct node *dt, struct fragment *base_frags,
+		      struct fragment *f, bool plugin, bool *for_basep)
 {
 	struct node *target;
 	bool ok = true;
@@ -664,13 +686,13 @@ static bool merge_ref(struct node *dt, struct fragment *f, bool plugin,
 		ok = check_relative_ref(f, f->label ? f->label_srcpos :
 					f->ref_srcpos);
 		if (!f->label &&
-		    (f->ref[0] == '/' || !get_node_by_ref(dt, f->ref))) {
+		    (f->ref[0] == '/' || !find_ref(dt, base_frags, f->ref))) {
 			*for_basep = true;
 			return ok;
 		}
 	}
 
-	target = lookup_ref(dt, f);
+	target = lookup_ref(dt, base_frags, f);
 	if (!target)
 		return false;
 	if (f->label)
@@ -685,9 +707,10 @@ static bool merge_ref(struct node *dt, struct fragment *f, bool plugin,
  *
  * Returns true on success, false on an error, which has been reported.
  */
-static bool merge_delete_node(struct node *dt, struct fragment *f)
+static bool merge_delete_node(struct node *dt, struct fragment *base_frags,
+			      struct fragment *f)
 {
-	struct node *target = lookup_ref(dt, f);
+	struct node *target = lookup_ref(dt, base_frags, f);
 
 	assert(f->type == FRAGMENT_DELETE_NODE);
 	if (!target)
@@ -703,9 +726,10 @@ static bool merge_delete_node(struct node *dt, struct fragment *f)
  *
  * Returns true on success, false on an error, which has been reported.
  */
-static bool merge_omit_if_no_ref(struct node *dt, struct fragment *f)
+static bool merge_omit_if_no_ref(struct node *dt, struct fragment *base_frags,
+				 struct fragment *f)
 {
-	struct node *target = lookup_ref(dt, f);
+	struct node *target = lookup_ref(dt, base_frags, f);
 
 	assert(f->type == FRAGMENT_OMIT_IF_NO_REF);
 	if (!target)
@@ -717,13 +741,14 @@ static bool merge_omit_if_no_ref(struct node *dt, struct fragment *f)
 
 /*
  * 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. If
- * the fragment is for the base tree, *for_basep is set to true.
+ * first fragment. A reference is looked up in the tree as merged so far and in
+ * base_frags, the fragments for the base tree so far. If this fragment is for
+ * the base tree too, *for_basep is set to true.
  *
  * Returns true on success, false on an error, which has been reported.
  */
-static bool merge_fragment(struct node **dtp, struct fragment *f,
-			   bool plugin, bool *for_basep)
+static bool merge_fragment(struct node **dtp, struct fragment *base_frags,
+			   struct fragment *f, bool plugin, bool *for_basep)
 {
 	struct node *dt = *dtp;
 
@@ -740,13 +765,13 @@ static bool merge_fragment(struct node **dtp, struct fragment *f,
 	switch (f->type) {
 	case FRAGMENT_NODE:
 		if (f->ref)
-			return merge_ref(dt, f, plugin, for_basep);
+			return merge_ref(dt, base_frags, f, plugin, for_basep);
 		*dtp = merge_nodes(dt, f->node);
 		break;
 	case FRAGMENT_DELETE_NODE:
-		return merge_delete_node(dt, f);
+		return merge_delete_node(dt, base_frags, f);
 	case FRAGMENT_OMIT_IF_NO_REF:
-		return merge_omit_if_no_ref(dt, f);
+		return merge_omit_if_no_ref(dt, base_frags, f);
 	}
 
 	return true;
@@ -754,39 +779,67 @@ static bool merge_fragment(struct node **dtp, struct fragment *f,
 
 /*
  * Merge the fragments of a dts file into a single tree, in source order. In a
- * plugin, each fragment for the base tree becomes an overlay fragment.
+ * plugin, the fragments for the base tree are left in dti->fragments, for
+ * add_overlay_fragments() to deal with.
  *
  * Returns true on success, false on errors, which have been reported.
  */
 bool merge_fragments(struct dt_info *dti)
 {
 	bool plugin = dti->dtsflags & DTSF_PLUGIN;
-	struct fragment *f, *next;
+	struct fragment *f, *next, *base_frags = NULL;
 	struct node *dt = NULL;
 	bool ok = true;
 
 	if (!dti->fragments)
 		return true;
 
+	/*
+	 * At the start, dti->fragments holds all the fragments in source order,
+	 * dt is NULL and base_frags is empty. Merge each fragment into dt and
+	 * free it, unless it is for the base tree, in which case move it to
+	 * base_frags so that later fragments can refer to its labels. At the
+	 * end, each fragment has been freed or moved to base_frags, and
+	 * dti->fragments is set to base_frags below.
+	 */
 	for (f = dti->fragments; f; f = next) {
 		bool for_base = false;
 
 		next = f->next;
-		if (!merge_fragment(&dt, f, plugin, &for_base))
+		if (!merge_fragment(&dt, base_frags, f, plugin, &for_base))
 			ok = false;
 		if (for_base)
-			add_orphan_node(dt, f->node, f->ref);
-		free_fragment(f);
+			base_frags = append_fragment(base_frags, f);
+		else
+			free_fragment(f);
 	}
 
 	dti->dt = dt;
-	dti->fragments = NULL;
+	dti->fragments = base_frags;
 	dti->boot_cpuid_phys = guess_boot_cpuid(dt);
 	fill_fullpaths(dti->dt, "");
 
 	return ok;
 }
 
+/*
+ * Add an overlay fragment (a fragment@N node) to the tree for each fragment of
+ * a plugin which is left after merging, i.e. which refers to the base tree
+ */
+void add_overlay_fragments(struct dt_info *dti)
+{
+	struct fragment *f, *next;
+	struct node *node;
+
+	for (f = dti->fragments; f; f = next) {
+		next = f->next;
+		node = add_orphan_node(dti->dt, f->node, f->ref);
+		fill_fullpaths(node, dti->dt->fullpath);
+		free_fragment(f);
+	}
+	dti->fragments = NULL;
+}
+
 /*
  * Tree accessor functions
  */
diff --git a/tests/run_tests.sh b/tests/run_tests.sh
index 92e4d2c..73ab5f8 100755
--- a/tests/run_tests.sh
+++ b/tests/run_tests.sh
@@ -841,21 +841,21 @@ dtc_tests () {
     run_fdtget_test "/fragment@0:target:0" merge-label-base.test.dtb /__fixups__ baselabel
 
     # In a plugin, a node in an overlay fragment can be deleted by its label,
-    # and the overlay fragment comes before the later node in the plugin
+    # and the overlay fragment comes after the nodes in the plugin, since it
+    # is added after the merge
     run_dtc_test -I dts -O dtb -o merge-overlay-order.test.dtb \
 	"$SRCDIR/merge-overlay-order.dts"
-    run_fdtget_test "fragment@0\nnode\n__fixups__" merge-overlay-order.test.dtb -l /
+    run_fdtget_test "node\nfragment@0\n__fixups__" merge-overlay-order.test.dtb -l /
     run_fdtget_test "bar" merge-overlay-order.test.dtb -l /fragment@0/__overlay__
 
-    # In a plugin, a / { ... } fragment can add to an overlay fragment by
+    # In a plugin, a / { ... } fragment cannot add to an overlay fragment by
     # giving the name which dtc generates for it, and a node in an overlay
-    # fragment can be deleted by its path
-    run_dtc_test -I dts -O dtb -o merge-overlay-name.test.dtb \
-	"$SRCDIR/merge-overlay-name.dts"
-    run_fdtget_test 1 merge-overlay-name.test.dtb /fragment@0 prop
-    run_dtc_test -I dts -O dtb -o merge-overlay-path.test.dtb \
-	"$SRCDIR/merge-overlay-path.dts"
-    run_fdtget_test "node2" merge-overlay-path.test.dtb -l /fragment@0/__overlay__
+    # fragment cannot be referred to by its path, since the overlay fragments
+    # are only added after the merge
+    run_sh_test "$SRCDIR/dtc-checkfails.sh" duplicate_node_names -- \
+	-I dts -O dtb "$SRCDIR/merge-overlay-name.dts"
+    run_wrap_error_msg_test "merge-overlay-path.dts:13.15-47 Label or path /fragment@0/__overlay__/node1 not found" \
+	$DTC -I dts -O dtb -o /dev/null "$SRCDIR/merge-overlay-path.dts"
 
     check_tests "$SRCDIR/bad-name-property.dts" name_properties
 
-- 
2.43.0

Re: [PATCH v3 02/18] srcpos: Allow srcpos_string() to accept NULL

[David Gibson]

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

On Fri, Nov 06, 2026 at 12:51:29PM -0700, Simon Glass wrote:
> A position is not always available: a node read from a dtb has none, and
> top-level fragments may eventually come from a dtbo as well as from
> source. At present srcpos_string() crashes if it has no position, so
> each caller which might not have one needs to check, as those in
> checks.c do.
> 
> Return a placeholder in this case, using the same text as
> srcpos_string_comment() does, so that srcpos_error() can be used whether
> or not there is a position.
> 
> No caller passes NULL at present, so this has no test yet.
> 
> Suggested-by: David Gibson <david@gibson.dropbear.id.au>
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Simon Glass <sjg@chromium.org>

Sensible independent of the rest of the series; applied.

-- 
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 v3 01/18] tests: Check how the fragments of a dts file are merged

[David Gibson]

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

On Fri, Nov 06, 2026 at 12:51:28PM -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, that a labelled reference within a
> plugin adds the label, that / { ... } and &{/} { ... } differ in a
> plugin, and that a label in a plugin becomes an overlay fragment if it
> is not found or is written as &{label}, which applies after any local
> merges, since nothing covers these either. Also check that a node in an
> overlay fragment can be deleted by its label, and where the overlay
> fragment goes among the plugin's own nodes.
> 
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Simon Glass <sjg@chromium.org>

Adding tests for this is very valuable.  Few concerns about some of
the specific things tested, see below.


[snip]
> diff --git a/tests/merge-errors.dts b/tests/merge-errors.dts
> new file mode 100644
> index 0000000..1e97088
> --- /dev/null
> +++ b/tests/merge-errors.dts
> @@ -0,0 +1,14 @@
> +/dts-v1/;
> +
> +/ {
> +	label: node {
> +	};
> +};
> +
> +/* All three of the references below are bad, and each one is reported */

Which three references?  I only see one.

> +&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..eece7a7
> --- /dev/null
> +++ b/tests/merge-first-ref-relative.dts
> @@ -0,0 +1,10 @@
> +/dts-v1/;
> +/plugin/;
> +
> +/* A plugin cannot define a node using a label-relative reference */
> +&{label/node} {
> +	prop;
> +};
> +
> +/* Later fragments are still merged */

Not sure this tests anything useful, since the compile will fail
overall.

> +/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 */

Again, not sure what this is really testing.

> +/delete-node/ &nosuchnode2;
> diff --git a/tests/merge-label-base.dts b/tests/merge-label-base.dts
> new file mode 100644
> index 0000000..1a0a8bb
> --- /dev/null
> +++ b/tests/merge-label-base.dts
> @@ -0,0 +1,10 @@
> +/dts-v1/;
> +/plugin/;
> +
> +/ {
> +};
> +
> +/* A label which is not in the plugin may be in the base tree */
> +&{baselabel} {
> +	prop;
> +};
> diff --git a/tests/merge-label-braces.dts b/tests/merge-label-braces.dts
> new file mode 100644
> index 0000000..d5f8d0f
> --- /dev/null
> +++ b/tests/merge-label-braces.dts
> @@ -0,0 +1,12 @@
> +/dts-v1/;
> +/plugin/;
> +
> +/ {
> +	label: node {
> +	};
> +};
> +
> +/* This becomes an overlay fragment, even though the label is in the plugin */
> +&{label} {

I'm not sure if this is testing behaviour we actually want to
enshrine.  My first thought was that &{label} should be illegal (the
path must be absolute.  But then I realised this does need to be
valid, as a degenerate case of a label-relative path.

That may currently have the side effect of forcing this to be an
orphan node, but I think that's an accident of implementation rather
than an intentional policy.

> +	prop;
> +};
> diff --git a/tests/merge-label-order.dts b/tests/merge-label-order.dts
> new file mode 100644
> index 0000000..f7923a0
> --- /dev/null
> +++ b/tests/merge-label-order.dts
> @@ -0,0 +1,17 @@
> +/dts-v1/;
> +/plugin/;
> +
> +&basenode {
> +	label: foo {
> +		p = "first";
> +	};
> +};
> +
> +/* The fragment for &{label} is applied last, so "second" wins */
> +&{label} {
> +	p = "second";
> +};
> +
> +&label {
> +	p = "third";
> +};

In this derived case, that existing behaviour just seems like a bug to
me.

> 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..9bedfd9
> --- /dev/null
> +++ b/tests/merge-labelled-relative.dts
> @@ -0,0 +1,14 @@
> +/dts-v1/;
> +/plugin/;
> +
> +/ {
> +	label: node {
> +		subnode {
> +		};
> +	};
> +};
> +
> +/* Not allowed even though the reference is found */

Again, I'm not sure this is behaviour we want to enshrine in a test.
Since we can resolve this locally it would be reasonable to extend the
code so that it does.

> +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-overlay-order.dts b/tests/merge-overlay-order.dts
> new file mode 100644
> index 0000000..2490157
> --- /dev/null
> +++ b/tests/merge-overlay-order.dts
> @@ -0,0 +1,19 @@
> +/dts-v1/;
> +/plugin/;
> +
> +&basenode {
> +	label: foo {
> +	};
> +
> +	bar {
> +	};
> +};
> +
> +/* A node in the plugin itself, after the overlay fragment */

As discussed on another thread, I'm not sure we really want to allow
this, although we might need to at least in the short to medium term,
for compatibility.

> +/ {
> +	node {
> +	};
> +};
> +
> +/* A label in an overlay fragment can be referred to */
> +/delete-node/ &label;
> 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 */

Reporting multiple errors is, in general, desirable behaviour, but..


This look like something that might be broken by the proposed changes:
the division by zero will caught during parse (since that's when we
resolve arithmetic) but we're going to defer the discovery of the
unknown node until the merge pass.  While reporting both errors would
be nice, it's not worth complexifying the implementation for it.
Therefore, I'm not sure we want to enshrine that behaviour in a test.

> +&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..c4e00a4
> --- /dev/null
> +++ b/tests/merge-path-relative.dts
> @@ -0,0 +1,12 @@
> +/dts-v1/;
> +/plugin/;
> +
> +/ {
> +	label: node {
> +	};
> +};
> +
> +/* A plugin cannot define a node using a label-relative path reference */

It's possible in future we could allow this in some cases, if the
label can be resolved locally in the plugin.  What we definitely can't
do is use a label-relative path reference with an external label, so
that would be the more useful case to test.

> +&{label/node} {
> +	prop;
> +};
> diff --git a/tests/merge-root-plugin.dts b/tests/merge-root-plugin.dts
> new file mode 100644
> index 0000000..f67355e
> --- /dev/null
> +++ b/tests/merge-root-plugin.dts
> @@ -0,0 +1,12 @@
> +/dts-v1/;
> +/plugin/;
> +
> +/* This is in the root node of the plugin */
> +/ {
> +	prop1;
> +};
> +
> +/* This is an overlay fragment for the root node of the base tree */
> +&{/} {
> +	prop2;
> +};
> diff --git a/tests/run_tests.sh b/tests/run_tests.sh
> index f750faa..2d392e2 100755
> --- a/tests/run_tests.sh
> +++ b/tests/run_tests.sh
> @@ -770,6 +770,80 @@ dtc_tests () {
>      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" -m "Label or path nosuchnode not found" \
>  	-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
> +
> +    # In a plugin, &{label} becomes an overlay fragment, even if it is found
> +    run_dtc_test -I dts -O dtb -o merge-label-braces.test.dtb \
> +	"$SRCDIR/merge-label-braces.dts"
> +    run_fdtget_test "" merge-label-braces.test.dtb /fragment@0/__overlay__ prop
> +    run_fdtget_test "node\nfragment@0\n__local_fixups__" merge-label-braces.test.dtb -l /
> +
> +    # The fragment for &{label} is applied after any local merges
> +    run_dtc_test -I dts -O dtb -o merge-label-order.test.dtb \
> +	"$SRCDIR/merge-label-order.dts"
> +    run_fdtget_test "third" merge-label-order.test.dtb /fragment@0/__overlay__/foo p
> +    run_fdtget_test "second" merge-label-order.test.dtb /fragment@1/__overlay__ p
> +
> +    # In a plugin, / { ... } and &{/} { ... } are not the same thing
> +    run_dtc_test -I dts -O dtb -o merge-root-plugin.test.dtb \
> +	"$SRCDIR/merge-root-plugin.dts"
> +    run_fdtget_test "" merge-root-plugin.test.dtb / prop1
> +    run_fdtget_test "/" merge-root-plugin.test.dtb /fragment@0 target-path
> +    run_fdtget_test "" merge-root-plugin.test.dtb /fragment@0/__overlay__ prop2
> +
> +    # In a plugin, a label which is not found becomes an overlay fragment
> +    run_dtc_test -I dts -O dtb -o merge-label-base.test.dtb \
> +	"$SRCDIR/merge-label-base.dts"
> +    run_fdtget_test "" merge-label-base.test.dtb /fragment@0/__overlay__ prop
> +    run_fdtget_test "/fragment@0:target:0" merge-label-base.test.dtb /__fixups__ baselabel
> +
> +    # In a plugin, a node in an overlay fragment can be deleted by its label,
> +    # and the overlay fragment comes before the later node in the plugin
> +    run_dtc_test -I dts -O dtb -o merge-overlay-order.test.dtb \
> +	"$SRCDIR/merge-overlay-order.dts"
> +    run_fdtget_test "fragment@0\nnode\n__fixups__" merge-overlay-order.test.dtb -l /
> +    run_fdtget_test "bar" merge-overlay-order.test.dtb -l /fragment@0/__overlay__
> +
>      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 v3 03/18] dtc: Add a struct for the top-level fragments of a dts file

[David Gibson]

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

On Fri, Nov 06, 2026 at 12:51:30PM -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.
> 
> Add a struct fragment to hold one of these, with a function to build
> each kind, and functions to append one to a list and to free it.
> 
> These are not used yet.
> 
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> David, you asked why the root needs a special type, rather than setting
> ref to "/". This drops the type, but uses a NULL ref rather than "/",
> since the two differ in a plugin: / { ... } adds to the plugin's own
> root, whereas &{/} { ... } becomes a fragment@N node for the root of the
> base tree. Using "/" for both would change the output.
> merge-root-plugin.dts checks this.
> 
> (no changes since v2)
> 
> Changes in v2:
> - Replace FRAGMENT_ROOT and FRAGMENT_REF with FRAGMENT_NODE
> - Use a NULL ref for the root node
> - Drop path_ref in favour of checking the ref for a leading '/'
> - Add a constructor for each kind of fragment, taking all its fields
> - Add the list to struct dt_info later, when it is first used
> - Change the subject from 'Add a list' to 'Add a struct'
> 
>  dtc.h      | 50 ++++++++++++++++++++++++++++++++
>  livetree.c | 85 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 135 insertions(+)
> 
> diff --git a/dtc.h b/dtc.h
> index 473552e..f15a162 100644
> --- a/dtc.h
> +++ b/dtc.h
> @@ -322,6 +322,56 @@ 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_NODE,		/* / { ... }; or [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_NODE            node, plus ref and ref_srcpos unless it is the
> + *                            root node, plus label and label_srcpos if there
> + *                            is a label
> + *   FRAGMENT_DELETE_NODE     ref and ref_srcpos
> + *   FRAGMENT_OMIT_IF_NO_REF  ref and ref_srcpos
> + *
> + * Unused fields are NULL. For / { ... }; the ref is NULL rather than "/", since
> + * in a plugin &{/} { ... }; refers to the root of the base tree instead.

With this version, I think these fields are clearer, and there's less
need for describing this.  All fields are always "valid" in the sense
that they're initialized to something sensible, though they might be
NULL in some cases (and must be for certain types).

> + */
> +struct fragment {
> +	enum fragment_type type;
> +	char *ref;			/* label or path referred to */
> +	struct srcpos *ref_srcpos;	/* position of ref */
> +	char *label;			/* label to add to the target */
> +	struct srcpos *label_srcpos;	/* position of label */

What's label_srcpos used for, distinct from ref_srcpos?  The'll almost
always be in adjacent places on the same line.

> +	struct node *node;		/* the node definition */
> +
> +	struct fragment *next;		/* NULL at the end of the list */
> +};
> +
> +struct fragment *build_fragment_root(struct node *node);
> +struct fragment *build_fragment_ref(char *label, struct srcpos *label_srcpos,
> +				    char *ref, struct srcpos *ref_srcpos,
> +				    struct node *node);
> +struct fragment *build_fragment_delete_node(char *ref,
> +					    struct srcpos *ref_srcpos);
> +struct fragment *build_fragment_omit_if_no_ref(char *ref,
> +					       struct srcpos *ref_srcpos);
> +struct fragment *append_fragment(struct fragment *list, struct fragment *new);
> +void free_fragment(struct fragment *f);
>  
>  struct dt_info {
>  	unsigned int dtsflags;
> diff --git a/livetree.c b/livetree.c
> index abe73de..85725d3 100644
> --- a/livetree.c
> +++ b/livetree.c
> @@ -448,6 +448,91 @@ struct reserve_info *add_reserve_entry(struct reserve_info *list,
>  	return list;
>  }
>  
> +/* Create a fragment of the given type, with its other fields NULL */
> +static struct fragment *new_fragment(enum fragment_type type, char *ref,
> +				     struct srcpos *ref_srcpos)
> +{
> +	struct fragment *new = xmalloc(sizeof(*new));
> +
> +	memset(new, '\0', sizeof(*new));

I'd use 0 rather than '\0' here - I'd only use the latter when the
thing we're zeroing is a string buffer.

> +
> +	new->type = type;
> +	new->ref = ref;
> +	new->ref_srcpos = srcpos_copy(ref_srcpos);
> +
> +	return new;
> +}
> +
> +/* Create a fragment for / { ... }; */
> +struct fragment *build_fragment_root(struct node *node)
> +{
> +	struct fragment *new = new_fragment(FRAGMENT_NODE, NULL, NULL);

I think this should also take a srcpos: after all the fragment itself
(and the literal "/") does have a position in the source, even if it
isn't a ref token.  I'm ok if the caller passes NULL for now, if it's
awkward to extract the srcpos at present.

In fact, I think the root fragment could be treated as a special case
of a ref fragment (with ref == NULL, for now), so having this call
build_fragment_ref would make sense.

> +
> +	new->node = node;
> +
> +	return new;
> +}
> +
> +/*
> + * Create a fragment for [label:] &ref { ... }; where label and label_srcpos are
> + * NULL if there is no label
> + */
> +struct fragment *build_fragment_ref(char *label, struct srcpos *label_srcpos,
> +				    char *ref, struct srcpos *ref_srcpos,
> +				    struct node *node)
> +{
> +	struct fragment *new = new_fragment(FRAGMENT_NODE, ref, ref_srcpos);
> +
> +	new->label = label;
> +	new->label_srcpos = srcpos_copy(label_srcpos);
> +	new->node = node;
> +
> +	return new;
> +}
> +
> +/* Create a fragment for /delete-node/ &ref; */
> +struct fragment *build_fragment_delete_node(char *ref,
> +					    struct srcpos *ref_srcpos)
> +{
> +	return new_fragment(FRAGMENT_DELETE_NODE, ref, ref_srcpos);
> +}
> +
> +/* Create a fragment for /omit-if-no-ref/ &ref; */
> +struct fragment *build_fragment_omit_if_no_ref(char *ref,
> +					       struct srcpos *ref_srcpos)
> +{
> +	return new_fragment(FRAGMENT_OMIT_IF_NO_REF, ref, ref_srcpos);
> +}
> +
> +/* Add a fragment to the end of a list, returning the new head */
> +struct fragment *append_fragment(struct fragment *list, struct fragment *new)

Since this is used from the parser, I think we can make this a bit
more similar to existing code as a quasi-functional chain_fragment(),
similar to the existing chain_node().  Technically that also removes
an O(n^2) behaviour, although not with an n that's likely to matter.

> +{
> +	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;
> -- 
> 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 v3 04/18] dtc: Move the /delete-node/ handling out of the grammar

[David Gibson]

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

On Fri, Nov 06, 2026 at 12:51:31PM -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.
> 
> The output does not change.
> 
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Simon Glass <sjg@chromium.org>

LGTM, except for any rework needed to accomodate changes suggested in
earlier patches.


> ---
> 
> (no changes since v2)
> 
> Changes in v2:
> - Use dt_ref instead of fragment_ref
> - Return true on success from merge_delete_node()
> - Check the fragment type with assert()
> 
>  dtc-parser.y | 11 ++++-------
>  dtc.h        |  1 +
>  livetree.c   | 20 ++++++++++++++++++++
>  3 files changed, 25 insertions(+), 7 deletions(-)
> 
> diff --git a/dtc-parser.y b/dtc-parser.y
> index e47f80d..bba98b6 100644
> --- a/dtc-parser.y
> +++ b/dtc-parser.y
> @@ -238,14 +238,11 @@ devicetree:
>  		}
>  	| devicetree DT_DEL_NODE dt_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);
> -
> +			struct fragment *f;
>  
> +			f = build_fragment_delete_node($3, &@3);
> +			if (!merge_delete_node($1, f))
> +				treesource_error = true;
>  			$$ = $1;
>  		}
>  	| devicetree DT_OMIT_NO_REF dt_ref ';'
> diff --git a/dtc.h b/dtc.h
> index f15a162..5dca8bc 100644
> --- a/dtc.h
> +++ b/dtc.h
> @@ -372,6 +372,7 @@ struct fragment *build_fragment_omit_if_no_ref(char *ref,
>  					       struct srcpos *ref_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 85725d3..35beee0 100644
> --- a/livetree.c
> +++ b/livetree.c
> @@ -567,6 +567,26 @@ struct dt_info *build_dt_info(unsigned int dtsflags,
>  	return dti;
>  }
>  
> +/*
> + * Delete the node which a /delete-node/ fragment refers to
> + *
> + * Returns true on success, false on 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(f->type == FRAGMENT_DELETE_NODE);
> +	if (!target) {
> +		srcpos_error(f->ref_srcpos, "Error",
> +			     "Label or path %s not found", f->ref);
> +		return false;
> +	}
> +	delete_node(target);
> +
> +	return true;
> +}
> +
>  /*
>   * 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 v3 05/18] dtc: Move the /omit-if-no-ref/ handling out of the grammar

[David Gibson]

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

On Fri, Nov 06, 2026 at 12:51:32PM -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>

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

> ---
> 
> (no changes since v2)
> 
> Changes in v2:
> - Use dt_ref instead of fragment_ref
> - Return true on success from merge_omit_if_no_ref()
> - Check the fragment type with assert()
> 
>  dtc-parser.y | 11 ++++-------
>  dtc.h        |  1 +
>  livetree.c   | 21 +++++++++++++++++++++
>  3 files changed, 26 insertions(+), 7 deletions(-)
> 
> diff --git a/dtc-parser.y b/dtc-parser.y
> index bba98b6..1aaff42 100644
> --- a/dtc-parser.y
> +++ b/dtc-parser.y
> @@ -247,14 +247,11 @@ devicetree:
>  		}
>  	| devicetree DT_OMIT_NO_REF dt_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);
> -
> +			struct fragment *f;
>  
> +			f = build_fragment_omit_if_no_ref($3, &@3);
> +			if (!merge_omit_if_no_ref($1, f))
> +				treesource_error = true;
>  			$$ = $1;
>  		}
>  	;
> diff --git a/dtc.h b/dtc.h
> index 5dca8bc..22a050a 100644
> --- a/dtc.h
> +++ b/dtc.h
> @@ -373,6 +373,7 @@ struct fragment *build_fragment_omit_if_no_ref(char *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 35beee0..a54009c 100644
> --- a/livetree.c
> +++ b/livetree.c
> @@ -587,6 +587,27 @@ bool merge_delete_node(struct node *dt, struct fragment *f)
>  	return true;
>  }
>  
> +/*
> + * Mark the node which an /omit-if-no-ref/ fragment refers to, so that it is
> + * dropped if nothing refers to it
> + *
> + * Returns true on success, false on 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);
> +
> +	assert(f->type == FRAGMENT_OMIT_IF_NO_REF);
> +	if (!target) {
> +		srcpos_error(f->ref_srcpos, "Error",
> +			     "Label or path %s not found", f->ref);
> +		return false;
> +	}
> +	omit_node_if_unused(target);
> +
> +	return true;
> +}
> +
>  /*
>   * 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 v3 06/18] dtc: Move is_ref_relative() to livetree.c

[David Gibson]

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

On Fri, Nov 06, 2026 at 12:51:33PM -0700, Simon Glass wrote:
> 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>

This makes sense independent of the reset of the series.  Merging.

> ---
> 
> (no changes since v1)
> 
>  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 1aaff42..9b9b6db 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 22a050a..8c4d7bf 100644
> --- a/dtc.h
> +++ b/dtc.h
> @@ -372,6 +372,7 @@ struct fragment *build_fragment_omit_if_no_ref(char *ref,
>  					       struct srcpos *ref_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 a54009c..3ff4e14 100644
> --- a/livetree.c
> +++ b/livetree.c
> @@ -567,6 +567,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 v3 07/18] dtc: Move label-reference handling out of the grammar

[David Gibson]

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

On Fri, Nov 06, 2026 at 12:51:34PM -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_ref(), passing it a struct fragment for the
> reference and node.
> 
> The output does not change.
> 
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Simon Glass <sjg@chromium.org>

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

> ---
> 
> (no changes since v2)
> 
> Changes in v2:
> - Rename merge_label_ref() to merge_ref()
> - Build the fragment in one step, with build_fragment_ref()
> - Return true on success from merge_ref()
> - Check the fragment type with assert()
> 
>  dtc-parser.y | 23 +++++++++--------------
>  dtc.h        |  1 +
>  livetree.c   | 26 ++++++++++++++++++++++++++
>  3 files changed, 36 insertions(+), 14 deletions(-)
> 
> diff --git a/dtc-parser.y b/dtc-parser.y
> index 9b9b6db..c58a66d 100644
> --- a/dtc-parser.y
> +++ b/dtc-parser.y
> @@ -214,21 +214,16 @@ devicetree:
>  		}
>  	| devicetree DT_LABEL_REF nodedef
>  		{
> -			struct node *target = get_node_by_ref($1, $2);
> +			struct fragment *f;
>  
> -			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 = build_fragment_ref(NULL, NULL, $2, &@2, $3);
> +			/*
> +			 * We rely on the rule being always:
> +			 *   versioninfo plugindecl memreserves devicetree
> +			 * so $-1 is what we want (plugindecl)
> +			 */
> +			if (!merge_ref($1, f, $<flags>-1 & DTSF_PLUGIN))
> +				treesource_error = true;
>  			$$ = $1;
>  		}
>  	| devicetree DT_DEL_NODE dt_ref ';'
> diff --git a/dtc.h b/dtc.h
> index 8c4d7bf..9b4389b 100644
> --- a/dtc.h
> +++ b/dtc.h
> @@ -373,6 +373,7 @@ struct fragment *build_fragment_omit_if_no_ref(char *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_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 3ff4e14..179866d 100644
> --- a/livetree.c
> +++ b/livetree.c
> @@ -573,6 +573,32 @@ 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 on success, false on an error, which has been reported.
> + */
> +bool merge_ref(struct node *dt, struct fragment *f, bool plugin)
> +{
> +	struct node *target = get_node_by_ref(dt, f->ref);
> +
> +	assert(f->type == FRAGMENT_NODE);
> +	if (target) {
> +		merge_nodes(target, f->node);
> +		return true;
> +	}
> +	if (!plugin) {
> +		srcpos_error(f->ref_srcpos, "Error",
> +			     "Label or path %s not found", f->ref);
> +		return false;
> +	}
> +	/* 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 true;
> +}
> +
>  /*
>   * 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 v3 08/18] dtc: Merge &{label} in a plugin as with &label

[David Gibson]

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

On Fri, Nov 06, 2026 at 12:51:35PM -0700, Simon Glass wrote:
> In a plugin, &label { ... } is merged into the labelled node if the
> label is in the plugin, as of commit 0fcffda15e9f ("Merge nodes with
> local target label references"). But &{label} { ... } always becomes a
> separate fragment@N node, since anything in braces is lexed as a path
> reference. This adds a fragment which is not needed, which that commit
> aims to avoid, and fails with FDT_ERR_NOTFOUND if the label is not
> within a fragment.
> 
> Treat a reference as a path only if it starts with '/', so that both
> forms are merged into the node. This changes the output for such a file.
> Applying an overlay which already works gives the same result, apart
> from the order of the properties and that a later &label { ... } now
> overrides an earlier &{label} { ... }, as the source order suggests.
> 
> Other references are not affected. An absolute path still becomes a
> fragment@N node, as does a label which is not in the plugin, since it
> may be in the base tree.
> 
> This also means that dtc does not need to record whether a reference is
> written in braces. The merge code, which is about to move out of the
> grammar, can tell a path from a label by looking at the reference.
> 
> Update the merge-label-braces.dts and merge-label-order.dts tests to
> match.
> 
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Simon Glass <sjg@chromium.org>

LGTM, I think this behavioural change is the correct call.

> ---
> 
> (no changes since v2)
> 
> Changes in v2:
> - Add new patch for &{label} in a plugin
> 
>  dtc-parser.y                 | 10 +++++++---
>  tests/merge-label-braces.dts |  2 +-
>  tests/merge-label-order.dts  |  2 +-
>  tests/run_tests.sh           | 10 +++++-----
>  4 files changed, 14 insertions(+), 10 deletions(-)
> 
> diff --git a/dtc-parser.y b/dtc-parser.y
> index c58a66d..3614d71 100644
> --- a/dtc-parser.y
> +++ b/dtc-parser.y
> @@ -193,6 +193,8 @@ devicetree:
>  		}
>  	| devicetree DT_PATH_REF nodedef
>  		{
> +			struct node *target = get_node_by_ref($1, $2);
> +
>  			/*
>  			 * We rely on the rule being always:
>  			 *   versioninfo plugindecl memreserves devicetree
> @@ -201,10 +203,12 @@ devicetree:
>  			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);
> +				/* A path is in the base tree and a label may be */
> +				if ($2[0] == '/' || !target)
> +					add_orphan_node($1, $3, $2);
> +				else
> +					merge_nodes(target, $3);
>  			} else {
> -				struct node *target = get_node_by_ref($1, $2);
> -
>  				if (target)
>  					merge_nodes(target, $3);
>  				else
> diff --git a/tests/merge-label-braces.dts b/tests/merge-label-braces.dts
> index d5f8d0f..569e7f8 100644
> --- a/tests/merge-label-braces.dts
> +++ b/tests/merge-label-braces.dts
> @@ -6,7 +6,7 @@
>  	};
>  };
>  
> -/* This becomes an overlay fragment, even though the label is in the plugin */
> +/* A label in the plugin itself is merged, however it is written */
>  &{label} {
>  	prop;
>  };
> diff --git a/tests/merge-label-order.dts b/tests/merge-label-order.dts
> index f7923a0..40fa668 100644
> --- a/tests/merge-label-order.dts
> +++ b/tests/merge-label-order.dts
> @@ -7,7 +7,7 @@
>  	};
>  };
>  
> -/* The fragment for &{label} is applied last, so "second" wins */
> +/* The last definition wins */
>  &{label} {
>  	p = "second";
>  };
> diff --git a/tests/run_tests.sh b/tests/run_tests.sh
> index 2d392e2..0aaa29c 100755
> --- a/tests/run_tests.sh
> +++ b/tests/run_tests.sh
> @@ -812,17 +812,17 @@ dtc_tests () {
>      run_fdtget_test "/node" merge-labelled-plugin.test.dtb /__symbols__ newlabel
>      run_fdtget_test "value" merge-labelled-plugin.test.dtb /node prop
>  
> -    # In a plugin, &{label} becomes an overlay fragment, even if it is found
> +    # In a plugin, &{label} is the same as &label
>      run_dtc_test -I dts -O dtb -o merge-label-braces.test.dtb \
>  	"$SRCDIR/merge-label-braces.dts"
> -    run_fdtget_test "" merge-label-braces.test.dtb /fragment@0/__overlay__ prop
> -    run_fdtget_test "node\nfragment@0\n__local_fixups__" merge-label-braces.test.dtb -l /
> +    run_fdtget_test "" merge-label-braces.test.dtb /node prop
> +    run_fdtget_test "node" merge-label-braces.test.dtb -l /
>  
> -    # The fragment for &{label} is applied after any local merges
> +    # The last definition wins, with no fragment for &{label}
>      run_dtc_test -I dts -O dtb -o merge-label-order.test.dtb \
>  	"$SRCDIR/merge-label-order.dts"
>      run_fdtget_test "third" merge-label-order.test.dtb /fragment@0/__overlay__/foo p
> -    run_fdtget_test "second" merge-label-order.test.dtb /fragment@1/__overlay__ p
> +    run_fdtget_test "fragment@0\n__fixups__" merge-label-order.test.dtb -l /
>  
>      # In a plugin, / { ... } and &{/} { ... } are not the same thing
>      run_dtc_test -I dts -O dtb -o merge-root-plugin.test.dtb \
> -- 
> 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 v3 09/18] dtc: Move path-reference handling out of the grammar

[David Gibson]

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

On Fri, Nov 06, 2026 at 12:51:36PM -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 merge_ref(), which can tell a path from a label by looking
> at the reference, and use a single rule for both kinds of reference.
> 
> The output does not change.
> 
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
> (no changes since v2)
> 
> Changes in v2:
> - Combine merge_path_ref() into merge_ref()
> - Combine the rules for label and path references, using dt_ref
> 
>  dtc-parser.y | 27 +--------------------------
>  livetree.c   | 33 +++++++++++++++++++++++----------
>  2 files changed, 24 insertions(+), 36 deletions(-)
> 
> diff --git a/dtc-parser.y b/dtc-parser.y
> index 3614d71..3ec9254 100644
> --- a/dtc-parser.y
> +++ b/dtc-parser.y
> @@ -191,32 +191,7 @@ devicetree:
>  				ERROR(&@3, "Label or path %s not found", $3);
>  			$$ = $1;
>  		}
> -	| devicetree DT_PATH_REF nodedef
> -		{
> -			struct node *target = get_node_by_ref($1, $2);
> -
> -			/*
> -			 * 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);
> -				/* A path is in the base tree and a label may be */
> -				if ($2[0] == '/' || !target)
> -					add_orphan_node($1, $3, $2);
> -				else
> -					merge_nodes(target, $3);
> -			} else {
> -				if (target)
> -					merge_nodes(target, $3);
> -				else
> -					ERROR(&@2, "Label or path %s not found", $2);
> -			}
> -			$$ = $1;
> -		}
> -	| devicetree DT_LABEL_REF nodedef
> +	| devicetree dt_ref nodedef
>  		{
>  			struct fragment *f;
>  
> diff --git a/livetree.c b/livetree.c
> index 179866d..2e9a3b2 100644
> --- a/livetree.c
> +++ b/livetree.c
> @@ -574,29 +574,42 @@ bool is_ref_relative(const char *ref)
>  }
>  
>  /*
> - * 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.
> + * Merge a fragment which refers to a label or path into the node it refers to.
> + *
> + * In a plugin, a path always refers to the base tree and a label may do so, if
> + * it is not found. In both cases, the fragment becomes an overlay fragment.
>   *
>   * Returns true on success, false on an error, which has been reported.
>   */
>  bool merge_ref(struct node *dt, struct fragment *f, bool plugin)
>  {
> -	struct node *target = get_node_by_ref(dt, f->ref);
> +	struct node *target;
> +	bool ok = true;
>  
>  	assert(f->type == FRAGMENT_NODE);
> -	if (target) {
> -		merge_nodes(target, f->node);
> -		return true;
> +	if (plugin) {
> +		if (is_ref_relative(f->ref)) {
> +			srcpos_error(f->ref_srcpos, "Error",
> +				     "Label-relative reference %s not supported in plugin",
> +				     f->ref);

AFAICT, the fact that we can't encode label-relative references in
.dtbo is only relevant in the case that we can't resolve it locally.
So I think this should go within the next if.

> +			ok = false;
> +		}
> +		if (f->ref[0] == '/' || !get_node_by_ref(dt, f->ref)) {
> +			/* Add a fragment@N node for the reference */
> +			add_orphan_node(dt, f->node, f->ref);

Pre-existing, but what will add_orphan_node() actually *do* if given a
ref it has no way to encode in a dtbo?

> +			return ok;
> +		}
>  	}
> -	if (!plugin) {
> +
> +	target = get_node_by_ref(dt, f->ref);

You can leave the get_node_by_ref() at the top, since it's used in
both branches.

> +	if (!target) {
>  		srcpos_error(f->ref_srcpos, "Error",
>  			     "Label or path %s not found", f->ref);
>  		return false;
>  	}
> -	/* The label may be in the base tree, so add a fragment@N node for it */
> -	add_orphan_node(dt, f->node, f->ref);
> +	merge_nodes(target, f->node);

> -	return true;
> +	return ok;
>  }
>  
>  /*
> -- 
> 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 v3 10/18] dtc: Move labelled-reference handling out of the grammar

[David Gibson]

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

On Fri, Nov 06, 2026 at 12:51:37PM -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 merge_ref(), passing it a struct fragment with the label.
> 
> The output does not change.
> 
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
> (no changes since v2)
> 
> Changes in v2:
> - Combine merge_labelled_ref() into merge_ref()
> - Use dt_ref instead of fragment_ref
> 
>  dtc-parser.y | 13 ++++---------
>  livetree.c   | 14 ++++++++++----
>  2 files changed, 14 insertions(+), 13 deletions(-)
> 
> diff --git a/dtc-parser.y b/dtc-parser.y
> index 3ec9254..8d505a1 100644
> --- a/dtc-parser.y
> +++ b/dtc-parser.y
> @@ -179,16 +179,11 @@ devicetree:
>  		}
>  	| devicetree DT_LABEL dt_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);
> +			struct fragment *f;
>  
> -			if (target) {
> -				add_label(&target->labels, $2);
> -				merge_nodes(target, $4);
> -			} else
> -				ERROR(&@3, "Label or path %s not found", $3);
> +			f = build_fragment_ref($2, &@2, $3, &@3, $4);
> +			if (!merge_ref($1, f, $<flags>-1 & DTSF_PLUGIN))
> +				treesource_error = true;
>  			$$ = $1;
>  		}
>  	| devicetree dt_ref nodedef
> diff --git a/livetree.c b/livetree.c
> index 2e9a3b2..7ab9437 100644
> --- a/livetree.c
> +++ b/livetree.c
> @@ -574,10 +574,12 @@ bool is_ref_relative(const char *ref)
>  }
>  
>  /*
> - * Merge a fragment which refers to a label or path into the node it refers to.
> + * Merge a fragment which refers to a label or path into the node it refers to,
> + * adding any label in the fragment to that node.
>   *
>   * In a plugin, a path always refers to the base tree and a label may do so, if
> - * it is not found. In both cases, the fragment becomes an overlay fragment.
> + * it is not found. In both cases, the fragment becomes an overlay fragment,
> + * unless it adds a label, which needs a node in the plugin itself.
>   *
>   * Returns true on success, false on an error, which has been reported.
>   */
> @@ -589,12 +591,14 @@ bool merge_ref(struct node *dt, struct fragment *f, bool plugin)
>  	assert(f->type == FRAGMENT_NODE);
>  	if (plugin) {
>  		if (is_ref_relative(f->ref)) {
> -			srcpos_error(f->ref_srcpos, "Error",
> +			srcpos_error(f->label ? f->label_srcpos : f->ref_srcpos,
> +				     "Error",

This change doesn't make sense to me - the problem is with the ref,
not the label, whether or not the label exists.

>  				     "Label-relative reference %s not supported in plugin",
>  				     f->ref);
>  			ok = false;
>  		}
> -		if (f->ref[0] == '/' || !get_node_by_ref(dt, f->ref)) {
> +		if (!f->label &&

Why does having a label preclude an orphan merge? IIRC, we can encode
this by putting label = "/fragment@XX/__overlay__" into the dtbo's
__symbols__.

> +		    (f->ref[0] == '/' || !get_node_by_ref(dt, f->ref))) {
>  			/* Add a fragment@N node for the reference */
>  			add_orphan_node(dt, f->node, f->ref);
>  			return ok;
> @@ -607,6 +611,8 @@ bool merge_ref(struct node *dt, struct fragment *f, bool plugin)
>  			     "Label or path %s not found", f->ref);
>  		return false;
>  	}
> +	if (f->label)
> +		add_label(&target->labels, f->label);
>  	merge_nodes(target, f->node);
>  
>  	return ok;
> -- 
> 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 v3 11/18] dtc: Move first-reference handling out of the grammar

[David Gibson]

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

On Fri, Nov 06, 2026 at 12:51:38PM -0700, Simon Glass wrote:
> 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 for
> the reference and node. 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>

I hope the first fragment will be less special cased than this in the
end, but this looks fine as an interim step.

> ---
> 
> (no changes since v2)
> 
> Changes in v2:
> - Use dt_ref instead of fragment_ref
> - Return true on success from merge_first_ref()
> 
>  dtc-parser.y | 13 +++++--------
>  dtc.h        |  2 +-
>  livetree.c   | 30 +++++++++++++++++++++++++++++-
>  3 files changed, 35 insertions(+), 10 deletions(-)
> 
> diff --git a/dtc-parser.y b/dtc-parser.y
> index 8d505a1..abeee0d 100644
> --- a/dtc-parser.y
> +++ b/dtc-parser.y
> @@ -163,19 +163,16 @@ devicetree:
>  		}
>  	| dt_ref nodedef
>  		{
> +			struct fragment *f;
> +
> +			f = build_fragment_ref(NULL, NULL, $1, &@1, $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(&$$, f, $<flags>-1 & DTSF_PLUGIN))
> +				treesource_error = true;
>  		}
>  	| devicetree DT_LABEL dt_ref nodedef
>  		{
> diff --git a/dtc.h b/dtc.h
> index 9b4389b..1befd5c 100644
> --- a/dtc.h
> +++ b/dtc.h
> @@ -372,7 +372,7 @@ struct fragment *build_fragment_omit_if_no_ref(char *ref,
>  					       struct srcpos *ref_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_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 7ab9437..26d39df 100644
> --- a/livetree.c
> +++ b/livetree.c
> @@ -568,11 +568,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 on success, false on an error, which has been reported.
> + */
> +bool merge_first_ref(struct node **dtp, struct fragment *f, bool plugin)
> +{
> +	bool ok = true;
> +
> +	if (!plugin) {
> +		srcpos_error(f->node->srcpos, "Error",
> +			     "Label or path %s not found", f->ref);
> +		ok = false;
> +	} else if (is_ref_relative(f->ref)) {
> +		srcpos_error(f->node->srcpos, "Error",
> +			     "Label-relative reference %s not supported in plugin",
> +			     f->ref);
> +		ok = false;
> +	}
> +	/* 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 ok;
> +}
> +
>  /*
>   * Merge a fragment which refers to a label or path into the node it refers to,
>   * adding any label in the fragment to that node.
> -- 
> 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 v3 12/18] dtc: Add merge_fragment() to merge any fragment

[David Gibson]

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

On Fri, Nov 06, 2026 at 12:51:39PM -0700, Simon Glass wrote:
> 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 functions used by merge_fragment() become static.
> 
> The output does not change.

LGTM.  Again, the logic is a bit more redundant than I'd like, but
it's a reasonable interim step.

> 
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
> (no changes since v2)
> 
> Changes in v2:
> - Use the new constructors and return true on success
> - Use dt_ref instead of fragment_ref
> - Combine the label and path rules earlier, in the path-reference patch
> 
>  dtc-parser.y | 53 ++++++++++++++++++++++++++--------------------------
>  dtc.h        |  5 +----
>  livetree.c   | 42 +++++++++++++++++++++++++++++++++++++----
>  3 files changed, 66 insertions(+), 34 deletions(-)
> 
> diff --git a/dtc-parser.y b/dtc-parser.y
> index abeee0d..9cda518 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 {
> @@ -155,65 +171,50 @@ dt_ref: DT_LABEL_REF | DT_PATH_REF;
>  devicetree:
>  	  '/' nodedef
>  		{
> -			$$ = name_node($2, "");
> +			struct fragment *f = build_fragment_root($2);
> +
> +			$$ = merge_parsed_fragment(NULL, f, $<flags>-1);
>  		}
>  	| devicetree '/' nodedef
>  		{
> -			$$ = merge_nodes($1, $3);
> +			struct fragment *f = build_fragment_root($3);
> +
> +			$$ = merge_parsed_fragment($1, f, $<flags>-1);
>  		}
>  	| dt_ref nodedef
>  		{
>  			struct fragment *f;
>  
>  			f = build_fragment_ref(NULL, NULL, $1, &@1, $2);
> -			/*
> -			 * We rely on the rule being always:
> -			 *   versioninfo plugindecl memreserves devicetree
> -			 * so $-1 is what we want (plugindecl)
> -			 */
> -			if (!merge_first_ref(&$$, f, $<flags>-1 & DTSF_PLUGIN))
> -				treesource_error = true;
> +			$$ = merge_parsed_fragment(NULL, f, $<flags>-1);
>  		}
>  	| devicetree DT_LABEL dt_ref nodedef
>  		{
>  			struct fragment *f;
>  
>  			f = build_fragment_ref($2, &@2, $3, &@3, $4);
> -			if (!merge_ref($1, f, $<flags>-1 & DTSF_PLUGIN))
> -				treesource_error = true;
> -			$$ = $1;
> +			$$ = merge_parsed_fragment($1, f, $<flags>-1);
>  		}
>  	| devicetree dt_ref nodedef
>  		{
>  			struct fragment *f;
>  
>  			f = build_fragment_ref(NULL, NULL, $2, &@2, $3);
> -			/*
> -			 * We rely on the rule being always:
> -			 *   versioninfo plugindecl memreserves devicetree
> -			 * so $-1 is what we want (plugindecl)
> -			 */
> -			if (!merge_ref($1, f, $<flags>-1 & DTSF_PLUGIN))
> -				treesource_error = true;
> -			$$ = $1;
> +			$$ = merge_parsed_fragment($1, f, $<flags>-1);
>  		}
>  	| devicetree DT_DEL_NODE dt_ref ';'
>  		{
>  			struct fragment *f;
>  
>  			f = build_fragment_delete_node($3, &@3);
> -			if (!merge_delete_node($1, f))
> -				treesource_error = true;
> -			$$ = $1;
> +			$$ = merge_parsed_fragment($1, f, $<flags>-1);
>  		}
>  	| devicetree DT_OMIT_NO_REF dt_ref ';'
>  		{
>  			struct fragment *f;
>  
>  			f = build_fragment_omit_if_no_ref($3, &@3);
> -			if (!merge_omit_if_no_ref($1, f))
> -				treesource_error = true;
> -			$$ = $1;
> +			$$ = merge_parsed_fragment($1, f, $<flags>-1);
>  		}
>  	;
>  
> diff --git a/dtc.h b/dtc.h
> index 1befd5c..e223bc2 100644
> --- a/dtc.h
> +++ b/dtc.h
> @@ -372,10 +372,7 @@ struct fragment *build_fragment_omit_if_no_ref(char *ref,
>  					       struct srcpos *ref_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_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 26d39df..a1d5d71 100644
> --- a/livetree.c
> +++ b/livetree.c
> @@ -580,7 +580,7 @@ static bool is_ref_relative(const char *ref)
>   *
>   * Returns true on success, false on 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 ok = true;
>  
> @@ -611,7 +611,7 @@ bool merge_first_ref(struct node **dtp, struct fragment *f, bool plugin)
>   *
>   * Returns true on success, false on an error, which has been reported.
>   */
> -bool merge_ref(struct node *dt, struct fragment *f, bool plugin)
> +static bool merge_ref(struct node *dt, struct fragment *f, bool plugin)
>  {
>  	struct node *target;
>  	bool ok = true;
> @@ -651,7 +651,7 @@ bool merge_ref(struct node *dt, struct fragment *f, bool plugin)
>   *
>   * Returns true on success, false on 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);
>  
> @@ -672,7 +672,7 @@ bool merge_delete_node(struct node *dt, struct fragment *f)
>   *
>   * Returns true on success, false on 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);
>  
> @@ -687,6 +687,40 @@ bool merge_omit_if_no_ref(struct node *dt, struct fragment *f)
>  	return true;
>  }
>  
> +/*
> + * 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 on success, false on 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->ref) {
> +			*dtp = name_node(f->node, "");
> +			return true;
> +		}
> +		return merge_first_ref(dtp, f, plugin);
> +	}
> +
> +	switch (f->type) {
> +	case FRAGMENT_NODE:
> +		if (f->ref)
> +			return merge_ref(dt, f, plugin);
> +		*dtp = merge_nodes(dt, f->node);
> +		break;
> +	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 true;
> +}
> +
>  /*
>   * 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 --]