[PATCH v2 0/6] dtc: Fix deletes within a section and before a redefinition

[PATCH v2 0/6] dtc: Fix deletes within a section and before a redefinition

[Simon Glass]

A /delete-property/ or /delete-node/ in the first section of a dts, or
in a node which a later section adds, is ignored and leaves its
placeholder in the tree, where it can show up as a bogus duplicate. A
node which is deleted and then defined again also returns to its old
place, with its old subnodes in their old order.

The first patch applies each delete within its own section and keeps
the placeholders out of the tree. The second removes deleted entries
from the tree, so that a node defined again comes after the existing
ones. The third clarifies some comments. The fourth lets dtc-fatal.sh
check the error message, which the fifth uses to test an error for a
delete in an overlay fragment which has nothing to delete there, since
an overlay cannot delete from the base tree. The sixth documents how
deletes behave.

Note that the second and fifth patches change existing behaviour. The
second changes the output of existing dts files which delete and then
redefine a node or property. The fifth breaks the build of two overlays
in Linux, whose deletes have never taken effect. Each patch explains
why this seems the better behaviour.

Changes in v2:
- Add new patch to clarify the comments
- Add new patch to check the error message in dtc-fatal.sh
- Add new patch to report an error for a delete in an overlay fragment
- Add new patch to document deletes

Simon Glass (6):
  dtc: Apply deletes in the section which has them
  dtc: Remove a deleted node or property from the tree
  dtc: Clarify when delete placeholders are dropped
  tests: Allow dtc-fatal.sh to check the error message
  dtc: Report an error for a delete in an overlay fragment
  doc: dts-format: Describe later definitions and deletes

 Documentation/dts-format.txt                  |  44 +++++
 dtc-parser.y                                  |  28 ++-
 dtc.h                                         |   4 +
 livetree.c                                    | 159 ++++++++++++++++--
 tests/delete_in_section.dts                   | 125 ++++++++++++++
 tests/delete_in_section_badref.dts            |  11 ++
 tests/delete_in_section_labels.dts            |  28 +++
 tests/delete_in_section_labels_ref.dts        |   9 +
 tests/delete_in_section_overlay.dts           |  22 +++
 tests/delete_in_section_overlay_ref.dts       |   9 +
 tests/delete_in_section_overlay_unused.dts    |   8 +
 ...delete_in_section_overlay_unused_label.dts |  14 ++
 .../delete_in_section_overlay_unused_path.dts |  13 ++
 tests/delete_in_section_ref.dts               |  44 +++++
 tests/delete_readd_order.dts                  |  57 +++++++
 tests/delete_readd_order_ref.dts              |  28 +++
 tests/dtc-fatal.sh                            |  18 +-
 tests/run_tests.sh                            |  39 ++++-
 18 files changed, 642 insertions(+), 18 deletions(-)
 create mode 100644 tests/delete_in_section.dts
 create mode 100644 tests/delete_in_section_badref.dts
 create mode 100644 tests/delete_in_section_labels.dts
 create mode 100644 tests/delete_in_section_labels_ref.dts
 create mode 100644 tests/delete_in_section_overlay.dts
 create mode 100644 tests/delete_in_section_overlay_ref.dts
 create mode 100644 tests/delete_in_section_overlay_unused.dts
 create mode 100644 tests/delete_in_section_overlay_unused_label.dts
 create mode 100644 tests/delete_in_section_overlay_unused_path.dts
 create mode 100644 tests/delete_in_section_ref.dts
 create mode 100644 tests/delete_readd_order.dts
 create mode 100644 tests/delete_readd_order_ref.dts

---
base-commit: 1d50f3f8fa8bf9429eebb0769dafa5df37381ae1
branch: delete-markers2

-- 
2.43.0

[PATCH v2 1/6] dtc: Apply deletes in the section which has them

[Simon Glass]

The parser holds each /delete-property/ and /delete-node/ as a
placeholder entry, flagged as deleted, among the node's properties or
children. It only takes effect when merge_nodes() merges its section
into an earlier one. In the first section of a dts it is ignored, so
'a; /delete-property/ a;' leaves 'a' in the tree. The same happens in
any node which a later section adds, since merge_nodes() attaches a new
child as it is. An ignored /delete-node/ also leaves its placeholder in
the tree, which then shows up as '(null): Duplicate node name'

Apply each delete when its node is built, by dropping the earlier
entries of the same name from that node. Nothing can refer to the node
yet, so they can be unlinked rather than flagged as deleted. Keep the
placeholder, so that merging the node into an earlier definition still
deletes there too.

When a node joins the tree without being merged, as the first section,
as a new child in merge_nodes() or as an overlay fragment, drop its
placeholders, since there is nothing earlier for them to delete. This
keeps placeholders and entries deleted within a section out of the tree,
so that nothing which walks the tree has to skip them.

Duplicate entries within a section are still reported as errors, since
nothing is merged.

Closes: https://github.com/dgibson/dtc/issues/37
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Simon Glass <sjg@chromium.org>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Herve Codina <herve.codina@bootlin.com>
---

(no changes since v1)

 dtc-parser.y                            |   1 +
 dtc.h                                   |   1 +
 livetree.c                              |  69 ++++++++++++-
 tests/delete_in_section.dts             | 125 ++++++++++++++++++++++++
 tests/delete_in_section_badref.dts      |  11 +++
 tests/delete_in_section_labels.dts      |  28 ++++++
 tests/delete_in_section_labels_ref.dts  |   9 ++
 tests/delete_in_section_overlay.dts     |  21 ++++
 tests/delete_in_section_overlay_ref.dts |   9 ++
 tests/delete_in_section_ref.dts         |  44 +++++++++
 tests/run_tests.sh                      |  21 ++++
 11 files changed, 338 insertions(+), 1 deletion(-)
 create mode 100644 tests/delete_in_section.dts
 create mode 100644 tests/delete_in_section_badref.dts
 create mode 100644 tests/delete_in_section_labels.dts
 create mode 100644 tests/delete_in_section_labels_ref.dts
 create mode 100644 tests/delete_in_section_overlay.dts
 create mode 100644 tests/delete_in_section_overlay_ref.dts
 create mode 100644 tests/delete_in_section_ref.dts

diff --git a/dtc-parser.y b/dtc-parser.y
index e47f80d..0282f7b 100644
--- a/dtc-parser.y
+++ b/dtc-parser.y
@@ -160,6 +160,7 @@ dt_ref: DT_LABEL_REF | DT_PATH_REF;
 devicetree:
 	  '/' nodedef
 		{
+			drop_delete_placeholders($2);
 			$$ = name_node($2, "");
 		}
 	| devicetree '/' nodedef
diff --git a/dtc.h b/dtc.h
index 473552e..3e5109d 100644
--- a/dtc.h
+++ b/dtc.h
@@ -277,6 +277,7 @@ struct node *omit_node_if_unused(struct node *node);
 struct node *reference_node(struct node *node);
 struct node *chain_node(struct node *first, struct node *list);
 struct node *merge_nodes(struct node *old_node, struct node *new_node);
+void drop_delete_placeholders(struct node *node);
 struct node *add_orphan_node(struct node *old_node, struct node *new_node, char *ref);
 
 void add_property(struct node *node, struct property *prop);
diff --git a/livetree.c b/livetree.c
index abe73de..b474a05 100644
--- a/livetree.c
+++ b/livetree.c
@@ -85,6 +85,68 @@ struct property *reverse_properties(struct property *first)
 	return head;
 }
 
+/*
+ * The parser holds each /delete-property/ and /delete-node/ as a placeholder
+ * entry, flagged as deleted, among the node's properties or children. Apply
+ * each one by dropping the earlier entries of the same name in the node. The
+ * placeholders stay, so that merging the node into an earlier definition
+ * deletes there too
+ */
+static void apply_deletes(struct node *node)
+{
+	struct property *prop, **pp;
+	struct node *child, **np;
+
+	for (prop = node->proplist; prop; prop = prop->next) {
+		if (!prop->deleted)
+			continue;
+		for (pp = &node->proplist; *pp != prop;) {
+			if (streq((*pp)->name, prop->name))
+				*pp = (*pp)->next;
+			else
+				pp = &(*pp)->next;
+		}
+	}
+
+	for (child = node->children; child; child = child->next_sibling) {
+		if (!child->deleted)
+			continue;
+		for (np = &node->children; *np != child;) {
+			if (streq((*np)->name, child->name))
+				*np = (*np)->next_sibling;
+			else
+				np = &(*np)->next_sibling;
+		}
+	}
+}
+
+/*
+ * Drop the /delete-property/ and /delete-node/ placeholders from a subtree
+ * which joins the tree without being merged, since there is nothing earlier
+ * for them to delete
+ */
+void drop_delete_placeholders(struct node *node)
+{
+	struct property **pp = &node->proplist;
+	struct node **np = &node->children;
+
+	while (*pp) {
+		if ((*pp)->deleted)
+			*pp = (*pp)->next;
+		else
+			pp = &(*pp)->next;
+	}
+
+	while (*np) {
+		if ((*np)->deleted) {
+			*np = (*np)->next_sibling;
+		} else {
+			drop_delete_placeholders(*np);
+			np = &(*np)->next_sibling;
+		}
+	}
+}
+
 struct node *build_node(struct property *proplist, struct node *children,
 			struct srcpos *srcpos)
 {
@@ -100,6 +162,7 @@ struct node *build_node(struct property *proplist, struct node *children,
 	for_each_child(new, child) {
 		child->parent = new;
 	}
+	apply_deletes(new);
 
 	return new;
 }
@@ -212,8 +275,10 @@ struct node *merge_nodes(struct node *old_node, struct node *new_node)
 		}
 
 		/* if no collision occurred, add child to the old node. */
-		if (new_child)
+		if (new_child) {
+			drop_delete_placeholders(new_child);
 			add_child(old_node, new_child);
+		}
 	}
 
 	old_node->srcpos = srcpos_extend(old_node->srcpos, new_node->srcpos);
@@ -245,6 +310,8 @@ struct node * add_orphan_node(struct node *dt, struct node *new_node, char *ref)
 		p = build_property("target", d, NULL);
 	}
 
+	drop_delete_placeholders(new_node);
+
 	xasprintf(&name, "fragment@%u",
 			next_orphan_fragment++);
 	name_node(new_node, "__overlay__");
diff --git a/tests/delete_in_section.dts b/tests/delete_in_section.dts
new file mode 100644
index 0000000..e742e79
--- /dev/null
+++ b/tests/delete_in_section.dts
@@ -0,0 +1,125 @@
+/dts-v1/;
+
+/*
+ * Delete properties and nodes defined earlier in the same section. This is
+ * the first section, so nothing is merged into an earlier one
+ */
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <0>;
+
+	gone-prop;
+	/delete-property/ gone-prop;
+	kept-prop;
+	again-prop = "old";
+	/delete-property/ again-prop;
+	again-prop = "new";
+	both-prop;
+
+	/* Deleted and added again here, then changed by a later section */
+	redel-prop = "old";
+	/delete-property/ redel-prop;
+	redel-prop = "new";
+	reset-prop = "old";
+	/delete-property/ reset-prop;
+	reset-prop = "new";
+
+	gone-node {
+	};
+	/delete-node/ gone-node;
+
+	again-node {
+		old;
+	};
+	/delete-node/ again-node;
+	again-node {
+		new;
+	};
+
+	lbl: node {
+		inner-prop;
+		/delete-property/ inner-prop;
+
+		inner-node {
+		};
+		/delete-node/ inner-node;
+	};
+
+	redel-node {
+		old;
+	};
+	/delete-node/ redel-node;
+	redel-node {
+		new;
+	};
+
+	remerge-node {
+		old;
+	};
+	/delete-node/ remerge-node;
+	remerge-node {
+		new;
+	};
+
+	/* Checks must not see what is deleted here */
+	named-node {
+		name = "other";
+		/delete-property/ name;
+	};
+
+	bus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		child@1 {
+			reg = <1>;
+		};
+		/delete-node/ child@1;
+	};
+
+	/* Deleted here, then added again by a later section */
+	readd@1 {
+		reg = <1>;
+		old;
+	};
+	/delete-node/ readd@1;
+	/delete-node/ placeholder@2;
+};
+
+/* Delete within nodes which a later section adds, and within the merge */
+
+/ {
+	both-prop = "new";
+	/delete-property/ both-prop;
+	/delete-property/ redel-prop;
+	reset-prop = "later";
+
+	new-node {
+		new-prop;
+		/delete-property/ new-prop;
+		kept;
+	};
+
+	/delete-node/ redel-node;
+
+	remerge-node {
+		later;
+	};
+
+	readd@1 {
+		reg = <1>;
+		later;
+	};
+
+	placeholder@2 {
+		reg = <2>;
+	};
+};
+
+&lbl {
+	child {
+		child-prop;
+		/delete-property/ child-prop;
+	};
+};
diff --git a/tests/delete_in_section_badref.dts b/tests/delete_in_section_badref.dts
new file mode 100644
index 0000000..79a40e4
--- /dev/null
+++ b/tests/delete_in_section_badref.dts
@@ -0,0 +1,11 @@
+/dts-v1/;
+
+/* A reference to a node deleted in the same section has nothing to refer to */
+
+/ {
+	ref = <&gone>;
+
+	gone: node {
+	};
+	/delete-node/ node;
+};
diff --git a/tests/delete_in_section_labels.dts b/tests/delete_in_section_labels.dts
new file mode 100644
index 0000000..fbde838
--- /dev/null
+++ b/tests/delete_in_section_labels.dts
@@ -0,0 +1,28 @@
+/dts-v1/;
+
+/*
+ * A label moves to the new definition of a node deleted in the same
+ * section, and labels on deleted nodes stay out of __symbols__
+ */
+
+/ {
+	ref = <&moved>;
+
+	moved: node {
+		old;
+	};
+	/delete-node/ node;
+	moved: node {
+		new;
+	};
+
+	gone: gone-node {
+	};
+	/delete-node/ gone-node;
+};
+
+/ {
+	later: later-node {
+	};
+	/delete-node/ later-node;
+};
diff --git a/tests/delete_in_section_labels_ref.dts b/tests/delete_in_section_labels_ref.dts
new file mode 100644
index 0000000..5d8cf38
--- /dev/null
+++ b/tests/delete_in_section_labels_ref.dts
@@ -0,0 +1,9 @@
+/dts-v1/;
+
+/ {
+	ref = <&moved>;
+
+	moved: node {
+		new;
+	};
+};
diff --git a/tests/delete_in_section_overlay.dts b/tests/delete_in_section_overlay.dts
new file mode 100644
index 0000000..e3e8f71
--- /dev/null
+++ b/tests/delete_in_section_overlay.dts
@@ -0,0 +1,21 @@
+/dts-v1/;
+/plugin/;
+
+/* Delete within the fragments of an overlay */
+
+&target {
+	a;
+	/delete-property/ a;
+	b;
+	/delete-property/ c;
+
+	gone {
+	};
+	/delete-node/ gone;
+	/delete-node/ d;
+};
+
+&{/path} {
+	p;
+	/delete-property/ p;
+};
diff --git a/tests/delete_in_section_overlay_ref.dts b/tests/delete_in_section_overlay_ref.dts
new file mode 100644
index 0000000..48f36b3
--- /dev/null
+++ b/tests/delete_in_section_overlay_ref.dts
@@ -0,0 +1,9 @@
+/dts-v1/;
+/plugin/;
+
+&target {
+	b;
+};
+
+&{/path} {
+};
diff --git a/tests/delete_in_section_ref.dts b/tests/delete_in_section_ref.dts
new file mode 100644
index 0000000..20d459a
--- /dev/null
+++ b/tests/delete_in_section_ref.dts
@@ -0,0 +1,44 @@
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <0>;
+	kept-prop;
+	again-prop = "new";
+	reset-prop = "later";
+
+	again-node {
+		new;
+	};
+
+	lbl: node {
+		child {
+		};
+	};
+
+	remerge-node {
+		new;
+		later;
+	};
+
+	named-node {
+	};
+
+	bus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+	};
+
+	new-node {
+		kept;
+	};
+
+	readd@1 {
+		reg = <1>;
+		later;
+	};
+
+	placeholder@2 {
+		reg = <2>;
+	};
+};
diff --git a/tests/run_tests.sh b/tests/run_tests.sh
index 842b543..2650df6 100755
--- a/tests/run_tests.sh
+++ b/tests/run_tests.sh
@@ -725,6 +725,27 @@ dtc_tests () {
     run_dtc_test -I dts -O dts -o delete_reinstate_multilabel.dts.test.dts "$SRCDIR/delete_reinstate_multilabel.dts"
     run_wrap_test cmp delete_reinstate_multilabel.dts.test.dts "$SRCDIR/delete_reinstate_multilabel_ref.dts"
 
+    # Check deletes within the section which has them
+    run_dtc_test -E reg_format -E avoid_unnecessary_addr_size -I dts -O dtb \
+	-o delete_in_section.test.dtb "$SRCDIR/delete_in_section.dts"
+    run_dtc_test -I dts -O dtb -o delete_in_section_ref.test.dtb \
+	"$SRCDIR/delete_in_section_ref.dts"
+    run_test dtbs_equal_ordered delete_in_section.test.dtb delete_in_section_ref.test.dtb
+    run_dtc_test -@ -I dts -O dtb -o delete_in_section_labels.test.dtb \
+	"$SRCDIR/delete_in_section_labels.dts"
+    run_dtc_test -@ -I dts -O dtb -o delete_in_section_labels_ref.test.dtb \
+	"$SRCDIR/delete_in_section_labels_ref.dts"
+    run_test dtbs_equal_ordered delete_in_section_labels.test.dtb \
+	delete_in_section_labels_ref.test.dtb
+    run_dtc_test -I dts -O dtb -o delete_in_section_overlay.test.dtb \
+	"$SRCDIR/delete_in_section_overlay.dts"
+    run_dtc_test -I dts -O dtb -o delete_in_section_overlay_ref.test.dtb \
+	"$SRCDIR/delete_in_section_overlay_ref.dts"
+    run_test dtbs_equal_ordered delete_in_section_overlay.test.dtb \
+	delete_in_section_overlay_ref.test.dtb
+    run_sh_test "$SRCDIR/dtc-checkfails.sh" phandle_references \
+	-n duplicate_node_names -- -I dts -O dtb "$SRCDIR/delete_in_section_badref.dts"
+
     # Check some checks
     check_tests "$SRCDIR/dup-nodename.dts" duplicate_node_names
     check_tests "$SRCDIR/dup-propname.dts" duplicate_property_names
-- 
2.43.0

[PATCH v2 2/6] dtc: Remove a deleted node or property from the tree

[Simon Glass]

Deleting a node or property only flags it as deleted, leaving it in
place. If a later section adds it again, merge_nodes() finds the flagged
entry and revives it where it was, along with its subnodes, which come
back in their old order before any new ones. So deleting a node in
order to redefine it does not give the order which the source shows.

Unlink a node or property from its parent when deleting it, so that
adding it again creates a new entry after the existing ones. Nothing
deleted then stays in the tree while parsing, so merge_nodes() has
nothing to revive and can skip deleted entries like everything else.

Leave the node's next_sibling alone, since fixup_omit_unused_nodes()
deletes nodes while walking over their siblings.

Note that this changes the output of existing dts files. When a dts
deletes a node or property and then defines it again, the new definition
now comes after its siblings instead of taking the old one's place, and
its subnodes come in the order of the new definition alone.

This is the better behaviour because:

- Deleting is how a dts says to start again. A dts which wants to keep a
  node's place can override the node without deleting it, since merging
  already updates a node in place
- Node order can matter. In issue 174, U-Boot's binman builds an image
  from a node which is deleted and defined again, yet the image comes
  out in the old order. The only workaround is to define the nodes in
  the wanted order before including the file which defines them
- It matches the previous patch, in which a node deleted and defined
  again within a section, or deleted in the first section and defined
  by a later one, already comes out where the new definition is

Closes: https://github.com/dgibson/dtc/issues/174
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Herve Codina <herve.codina@bootlin.com>
---

(no changes since v1)

 livetree.c                       | 43 ++++++++++++++++++------
 tests/delete_readd_order.dts     | 57 ++++++++++++++++++++++++++++++++
 tests/delete_readd_order_ref.dts | 28 ++++++++++++++++
 tests/run_tests.sh               |  6 ++++
 4 files changed, 123 insertions(+), 11 deletions(-)
 create mode 100644 tests/delete_readd_order.dts
 create mode 100644 tests/delete_readd_order_ref.dts

diff --git a/livetree.c b/livetree.c
index b474a05..6609e4e 100644
--- a/livetree.c
+++ b/livetree.c
@@ -208,8 +208,6 @@ struct node *merge_nodes(struct node *old_node, struct node *new_node)
 	struct node *new_child, *old_child;
 	struct label *l;
 
-	old_node->deleted = 0;
-
 	/* Add new node labels to old node */
 	for_each_label_withdel(new_node->labels, l)
 		add_label(&old_node->labels, l->label);
@@ -229,14 +227,13 @@ struct node *merge_nodes(struct node *old_node, struct node *new_node)
 		}
 
 		/* Look for a collision, set new value if there is */
-		for_each_property_withdel(old_node, old_prop) {
+		for_each_property(old_node, old_prop) {
 			if (streq(old_prop->name, new_prop->name)) {
 				/* Add new labels to old property */
 				for_each_label_withdel(new_prop->labels, l)
 					add_label(&old_prop->labels, l->label);
 
 				old_prop->val = new_prop->val;
-				old_prop->deleted = 0;
 				srcpos_free(old_prop->srcpos);
 				old_prop->srcpos = new_prop->srcpos;
 				free(new_prop);
@@ -266,7 +263,7 @@ struct node *merge_nodes(struct node *old_node, struct node *new_node)
 		}
 
 		/* Search for a collision.  Merge if there is */
-		for_each_child_withdel(old_node, old_child) {
+		for_each_child(old_node, old_child) {
 			if (streq(old_child->name, new_child->name)) {
 				merge_nodes(old_child, new_child);
 				new_child = NULL;
@@ -346,14 +343,18 @@ void add_property(struct node *node, struct property *prop)
 
 void delete_property_by_name(struct node *node, char *name)
 {
-	struct property *prop = node->proplist;
+	struct property **pp = &node->proplist;
+
+	while (*pp) {
+		if (streq((*pp)->name, name)) {
+			struct property *prop = *pp;
 
-	while (prop) {
-		if (streq(prop->name, name)) {
+			/* Unlink it, so that adding it again starts afresh */
+			*pp = prop->next;
 			delete_property(prop);
 			return;
 		}
-		prop = prop->next;
+		pp = &(*pp)->next;
 	}
 }
 
@@ -390,19 +391,39 @@ void delete_node_by_name(struct node *parent, char *name)
 	}
 }
 
-void delete_node(struct node *node)
+static void delete_subtree(struct node *node)
 {
 	struct property *prop;
 	struct node *child;
 
 	node->deleted = 1;
 	for_each_child(node, child)
-		delete_node(child);
+		delete_subtree(child);
 	for_each_property(node, prop)
 		delete_property(prop);
 	delete_labels(&node->labels);
 }
 
+void delete_node(struct node *node)
+{
+	struct node **np;
+
+	/*
+	 * Unlink the node, so that adding it again starts afresh. Leave its
+	 * next_sibling alone, since a walk over its siblings may be at it
+	 */
+	if (node->parent) {
+		for (np = &node->parent->children; *np;
+		     np = &(*np)->next_sibling) {
+			if (*np == node) {
+				*np = node->next_sibling;
+				break;
+			}
+		}
+	}
+	delete_subtree(node);
+}
+
 void append_to_property(struct node *node,
 			char *name, const void *data, int len,
 			enum markertype type)
diff --git a/tests/delete_readd_order.dts b/tests/delete_readd_order.dts
new file mode 100644
index 0000000..91a5fa7
--- /dev/null
+++ b/tests/delete_readd_order.dts
@@ -0,0 +1,57 @@
+/dts-v1/;
+
+/*
+ * Delete properties and nodes in a later section and add them again. They
+ * come back as new entries after the existing ones, rather than returning
+ * to their old places along with their old subnodes
+ */
+
+/ {
+	first-prop;
+	moved-prop;
+	last-prop;
+
+	parent {
+		images {
+			old-image {
+			};
+			kept-image {
+				old;
+			};
+		};
+
+		other {
+		};
+	};
+
+	gone: gone-node {
+		old;
+	};
+
+	last-node {
+	};
+};
+
+/ {
+	/delete-property/ moved-prop;
+	moved-prop;
+
+	parent {
+		/delete-node/ images;
+		images {
+			new-image {
+			};
+			kept-image {
+				new;
+			};
+		};
+	};
+};
+
+/delete-node/ &gone;
+
+/ {
+	gone-node {
+		new;
+	};
+};
diff --git a/tests/delete_readd_order_ref.dts b/tests/delete_readd_order_ref.dts
new file mode 100644
index 0000000..7c86d0f
--- /dev/null
+++ b/tests/delete_readd_order_ref.dts
@@ -0,0 +1,28 @@
+/dts-v1/;
+
+/ {
+	first-prop;
+	last-prop;
+	moved-prop;
+
+	parent {
+		other {
+		};
+
+		images {
+			new-image {
+			};
+
+			kept-image {
+				new;
+			};
+		};
+	};
+
+	last-node {
+	};
+
+	gone-node {
+		new;
+	};
+};
diff --git a/tests/run_tests.sh b/tests/run_tests.sh
index 2650df6..8813846 100755
--- a/tests/run_tests.sh
+++ b/tests/run_tests.sh
@@ -746,6 +746,12 @@ dtc_tests () {
     run_sh_test "$SRCDIR/dtc-checkfails.sh" phandle_references \
 	-n duplicate_node_names -- -I dts -O dtb "$SRCDIR/delete_in_section_badref.dts"
 
+    run_dtc_test -I dts -O dtb -o delete_readd_order.test.dtb \
+	"$SRCDIR/delete_readd_order.dts"
+    run_dtc_test -I dts -O dtb -o delete_readd_order_ref.test.dtb \
+	"$SRCDIR/delete_readd_order_ref.dts"
+    run_test dtbs_equal_ordered delete_readd_order.test.dtb delete_readd_order_ref.test.dtb
+
     # Check some checks
     check_tests "$SRCDIR/dup-nodename.dts" duplicate_node_names
     check_tests "$SRCDIR/dup-propname.dts" duplicate_property_names
-- 
2.43.0

[PATCH v2 3/6] dtc: Clarify when delete placeholders are dropped

[Simon Glass]

The comment on drop_delete_placeholders() does not make clear which
subtrees it is for. The call in merge_nodes() also relies on sections
being merged in source order, since it drops the child's placeholders on
the basis that nothing earlier remains to be merged, but nothing there
says so.

Say that drop_delete_placeholders() is for the first definition of a
node, i.e. the first in the list of definitions which are merged
together. Explain at the call in merge_nodes() why the child is the
first definition, and that merging is not associative.

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

Changes in v2:
- Add new patch to clarify the comments

 livetree.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/livetree.c b/livetree.c
index 6609e4e..b7a5161 100644
--- a/livetree.c
+++ b/livetree.c
@@ -122,8 +122,10 @@ static void apply_deletes(struct node *node)
 
 /*
  * Drop the /delete-property/ and /delete-node/ placeholders from a subtree
- * which joins the tree without being merged, since there is nothing earlier
- * for them to delete
+ * which is the first definition of its node, i.e. the first in the list of
+ * definitions which are merged together. apply_deletes() has already dropped
+ * what they delete within the subtree, and there is nothing earlier for them
+ * to delete
  */
 void drop_delete_placeholders(struct node *node)
 {
@@ -273,6 +275,14 @@ struct node *merge_nodes(struct node *old_node, struct node *new_node)
 
 		/* if no collision occurred, add child to the old node. */
 		if (new_child) {
+			/*
+			 * This is the first definition of the child, since
+			 * sections are merged in source order, so its
+			 * placeholders have nothing to delete. This is
+			 * important since dropping them makes merging not
+			 * associative: merging later sections together first
+			 * would lose their deletes
+			 */
 			drop_delete_placeholders(new_child);
 			add_child(old_node, new_child);
 		}
-- 
2.43.0

[PATCH v2 4/6] tests: Allow dtc-fatal.sh to check the error message

[Simon Glass]

dtc-fatal.sh only checks that dtc exits with status 1, so a test which
expects a particular error passes just as well if dtc fails for some
other reason, such as a mistake in the test's dts file.

Add a -m option which also checks that dtc's output contains a given
message. Use it for the nonexist-node-ref2 test.

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

Changes in v2:
- Add new patch to check the error message in dtc-fatal.sh

 tests/dtc-fatal.sh | 18 +++++++++++++++++-
 tests/run_tests.sh |  3 ++-
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/tests/dtc-fatal.sh b/tests/dtc-fatal.sh
index 08a4d29..2fa96ef 100755
--- a/tests/dtc-fatal.sh
+++ b/tests/dtc-fatal.sh
@@ -1,9 +1,21 @@
 #! /bin/sh
 
+# Usage: dtc-fatal.sh [-m <message>] <dtc args>
+# Check that dtc fails, and with -m that its output contains <message>
+
 SRCDIR=`dirname "$0"`
 . "$SRCDIR/testutils.sh"
 
-verbose_run $VALGRIND "$DTC" -o/dev/null "$@"
+if [ "$1" = "-m" ]; then
+    msg="$2"
+    shift 2
+fi
+
+LOG=tmp.log.$$
+rm -f $LOG
+trap "rm -f $LOG" 0
+
+verbose_run_log "$LOG" $VALGRIND "$DTC" -o/dev/null "$@"
 ret="$?"
 
 if [ "$ret" -gt 127 ]; then
@@ -12,4 +24,8 @@ elif [ "$ret" != "1" ]; then
     FAIL "dtc returned incorrect status $ret instead of 1"
 fi
 
+if [ -n "$msg" ] && ! grep -qF -- "$msg" "$LOG"; then
+    FAIL "Expected message '$msg'"
+fi
+
 PASS
diff --git a/tests/run_tests.sh b/tests/run_tests.sh
index 8813846..1018a6b 100755
--- a/tests/run_tests.sh
+++ b/tests/run_tests.sh
@@ -760,7 +760,8 @@ dtc_tests () {
     check_tests "$SRCDIR/minusone-phandle.dts" explicit_phandles
     run_sh_test "$SRCDIR/dtc-checkfails.sh" phandle_references -- -I dts -O dtb "$SRCDIR/nonexist-node-ref.dts"
     run_sh_test "$SRCDIR/dtc-checkfails.sh" phandle_references -- -I dts -O dtb "$SRCDIR/nonexist-label-ref.dts"
-    run_sh_test "$SRCDIR/dtc-fatal.sh" -I dts -O dtb "$SRCDIR/nonexist-node-ref2.dts"
+    run_sh_test "$SRCDIR/dtc-fatal.sh" -m "Label or path nosuchnode not found" \
+	-I dts -O dtb "$SRCDIR/nonexist-node-ref2.dts"
     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 v2 5/6] dtc: Report an error for a delete in an overlay fragment

[Simon Glass]

An overlay cannot encode a /delete-property/ or /delete-node/, since
applying an overlay only adds or updates nodes and properties. So a
delete in an overlay fragment has no effect on the base tree, yet dtc
drops it without complaint, which suggests that it works. A delete which
removes something earlier in the same fragment is fine, since the
fragment is compiled without it.

Report an error for a /delete-property/ or /delete-node/ in a fragment
which does not delete anything within the fragment, naming it. Record in
apply_deletes() which placeholders delete something, and add
find_unapplied_delete() to look for one which does not. Do this for each
of the three ways in which a plugin creates a fragment.

Drop the deletes with nothing to delete from the overlay test, and add
tests which check for the error, one for each way of creating a
fragment.

Note: This breaks the build of two overlays in Linux v7.3-rc7, whose
deletes have never taken effect:

   imx8mn-tqma8mqnl-mba8mx-usbotg.dtso
   imx8mp-tqma8mpql-mba8mp-ras314-imx219.dtso

Linux needs to remove these deletes. Where the result matters, the base
dts must change instead, e.g. by moving the property or node to the
boards which need it, so that the overlay only adds to the tree.

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

Changes in v2:
- Add new patch to report an error for a delete in an overlay fragment

 dtc-parser.y                                  | 27 +++++++++--
 dtc.h                                         |  3 ++
 livetree.c                                    | 45 +++++++++++++++++--
 tests/delete_in_section_overlay.dts           |  7 +--
 tests/delete_in_section_overlay_unused.dts    |  8 ++++
 ...delete_in_section_overlay_unused_label.dts | 14 ++++++
 .../delete_in_section_overlay_unused_path.dts | 13 ++++++
 tests/run_tests.sh                            |  9 ++++
 8 files changed, 115 insertions(+), 11 deletions(-)
 create mode 100644 tests/delete_in_section_overlay_unused.dts
 create mode 100644 tests/delete_in_section_overlay_unused_label.dts
 create mode 100644 tests/delete_in_section_overlay_unused_path.dts

diff --git a/dtc-parser.y b/dtc-parser.y
index 0282f7b..e1b8368 100644
--- a/dtc-parser.y
+++ b/dtc-parser.y
@@ -29,6 +29,25 @@ static bool is_ref_relative(const char *ref)
 	return ref[0] != '/' && strchr(&ref[1], '/');
 }
 
+/*
+ * Add a node from a plugin as an overlay fragment. An overlay cannot encode a
+ * delete, so a /delete-property/ or /delete-node/ which does not delete
+ * anything within the fragment would have no effect when the overlay is applied
+ */
+static struct node *add_fragment(struct node *dt, struct node *new_node,
+				 char *ref, struct srcpos *loc)
+{
+	const char *name;
+	bool is_node;
+
+	name = find_unapplied_delete(new_node, &is_node);
+	if (name)
+		ERROR(loc, "/delete-%s/ %s in %s has no effect in plugin",
+		      is_node ? "node" : "property", name, ref);
+
+	return add_orphan_node(dt, new_node, ref);
+}
+
 %}
 
 %union {
@@ -178,10 +197,10 @@ devicetree:
 				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(
+			$$ = add_fragment(
 					name_node(build_node(NULL, NULL, NULL),
 						  ""),
-					$2, $1);
+					$2, $1, &@2);
 		}
 	| devicetree DT_LABEL dt_ref nodedef
 		{
@@ -207,7 +226,7 @@ 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);
+				add_fragment($1, $3, $2, &@3);
 			} else {
 				struct node *target = get_node_by_ref($1, $2);
 
@@ -231,7 +250,7 @@ devicetree:
 				 * so $-1 is what we want (plugindecl)
 				 */
 				if ($<flags>-1 & DTSF_PLUGIN)
-					add_orphan_node($1, $3, $2);
+					add_fragment($1, $3, $2, &@3);
 				else
 					ERROR(&@2, "Label or path %s not found", $2);
 			}
diff --git a/dtc.h b/dtc.h
index 3e5109d..fc632bd 100644
--- a/dtc.h
+++ b/dtc.h
@@ -208,6 +208,7 @@ struct bus_type {
 
 struct property {
 	bool deleted;
+	bool delete_applied;
 	char *name;
 	struct data val;
 
@@ -219,6 +220,7 @@ struct property {
 
 struct node {
 	bool deleted;
+	bool delete_applied;
 	char *name;
 	struct property *proplist;
 	struct node *children;
@@ -277,6 +279,7 @@ struct node *omit_node_if_unused(struct node *node);
 struct node *reference_node(struct node *node);
 struct node *chain_node(struct node *first, struct node *list);
 struct node *merge_nodes(struct node *old_node, struct node *new_node);
+const char *find_unapplied_delete(struct node *node, bool *is_node);
 void drop_delete_placeholders(struct node *node);
 struct node *add_orphan_node(struct node *old_node, struct node *new_node, char *ref);
 
diff --git a/livetree.c b/livetree.c
index b7a5161..37e37c6 100644
--- a/livetree.c
+++ b/livetree.c
@@ -101,10 +101,13 @@ static void apply_deletes(struct node *node)
 		if (!prop->deleted)
 			continue;
 		for (pp = &node->proplist; *pp != prop;) {
-			if (streq((*pp)->name, prop->name))
+			if (streq((*pp)->name, prop->name)) {
+				if (!(*pp)->deleted)
+					prop->delete_applied = true;
 				*pp = (*pp)->next;
-			else
+			} else {
 				pp = &(*pp)->next;
+			}
 		}
 	}
 
@@ -112,14 +115,48 @@ static void apply_deletes(struct node *node)
 		if (!child->deleted)
 			continue;
 		for (np = &node->children; *np != child;) {
-			if (streq((*np)->name, child->name))
+			if (streq((*np)->name, child->name)) {
+				if (!(*np)->deleted)
+					child->delete_applied = true;
 				*np = (*np)->next_sibling;
-			else
+			} else {
 				np = &(*np)->next_sibling;
+			}
 		}
 	}
 }
 
+/*
+ * Find a /delete-property/ or /delete-node/ in a subtree which did not delete
+ * anything within it. Return its name, setting *is_node to say which it is, or
+ * NULL if there is none
+ */
+const char *find_unapplied_delete(struct node *node, bool *is_node)
+{
+	struct property *prop;
+	struct node *child;
+	const char *name;
+
+	for (prop = node->proplist; prop; prop = prop->next) {
+		if (prop->deleted && !prop->delete_applied) {
+			*is_node = false;
+			return prop->name;
+		}
+	}
+
+	for (child = node->children; child; child = child->next_sibling) {
+		if (child->deleted && !child->delete_applied) {
+			*is_node = true;
+			return child->name;
+		}
+		name = find_unapplied_delete(child, is_node);
+		if (name)
+			return name;
+	}
+
+	return NULL;
+}
+
 /*
  * Drop the /delete-property/ and /delete-node/ placeholders from a subtree
  * which is the first definition of its node, i.e. the first in the list of
diff --git a/tests/delete_in_section_overlay.dts b/tests/delete_in_section_overlay.dts
index e3e8f71..f7c8f64 100644
--- a/tests/delete_in_section_overlay.dts
+++ b/tests/delete_in_section_overlay.dts
@@ -1,18 +1,19 @@
 /dts-v1/;
 /plugin/;
 
-/* Delete within the fragments of an overlay */
+/*
+ * Delete within the fragments of an overlay. Each delete must delete something
+ * in its fragment, since an overlay cannot delete from the base tree
+ */
 
 &target {
 	a;
 	/delete-property/ a;
 	b;
-	/delete-property/ c;
 
 	gone {
 	};
 	/delete-node/ gone;
-	/delete-node/ d;
 };
 
 &{/path} {
diff --git a/tests/delete_in_section_overlay_unused.dts b/tests/delete_in_section_overlay_unused.dts
new file mode 100644
index 0000000..4956689
--- /dev/null
+++ b/tests/delete_in_section_overlay_unused.dts
@@ -0,0 +1,8 @@
+/dts-v1/;
+/plugin/;
+
+/* An overlay cannot delete from the base tree */
+
+&target {
+	/delete-property/ a;
+};
diff --git a/tests/delete_in_section_overlay_unused_label.dts b/tests/delete_in_section_overlay_unused_label.dts
new file mode 100644
index 0000000..b80f3b2
--- /dev/null
+++ b/tests/delete_in_section_overlay_unused_label.dts
@@ -0,0 +1,14 @@
+/dts-v1/;
+/plugin/;
+
+/* A delete with nothing to delete in a subnode of a later fragment */
+
+&target {
+	a;
+};
+
+&other {
+	sub {
+		/delete-node/ gone;
+	};
+};
diff --git a/tests/delete_in_section_overlay_unused_path.dts b/tests/delete_in_section_overlay_unused_path.dts
new file mode 100644
index 0000000..5186b1b
--- /dev/null
+++ b/tests/delete_in_section_overlay_unused_path.dts
@@ -0,0 +1,13 @@
+/dts-v1/;
+/plugin/;
+
+/* A delete before the definition in a later fragment which uses a path */
+
+&target {
+	a;
+};
+
+&{/path} {
+	/delete-property/ p;
+	p;
+};
diff --git a/tests/run_tests.sh b/tests/run_tests.sh
index 1018a6b..ed93f86 100755
--- a/tests/run_tests.sh
+++ b/tests/run_tests.sh
@@ -743,6 +743,15 @@ dtc_tests () {
 	"$SRCDIR/delete_in_section_overlay_ref.dts"
     run_test dtbs_equal_ordered delete_in_section_overlay.test.dtb \
 	delete_in_section_overlay_ref.test.dtb
+    run_sh_test "$SRCDIR/dtc-fatal.sh" \
+	-m "/delete-property/ a in target has no effect in plugin" \
+	-I dts -O dtb "$SRCDIR/delete_in_section_overlay_unused.dts"
+    run_sh_test "$SRCDIR/dtc-fatal.sh" \
+	-m "/delete-node/ gone in other has no effect in plugin" \
+	-I dts -O dtb "$SRCDIR/delete_in_section_overlay_unused_label.dts"
+    run_sh_test "$SRCDIR/dtc-fatal.sh" \
+	-m "/delete-property/ p in /path has no effect in plugin" \
+	-I dts -O dtb "$SRCDIR/delete_in_section_overlay_unused_path.dts"
     run_sh_test "$SRCDIR/dtc-checkfails.sh" phandle_references \
 	-n duplicate_node_names -- -I dts -O dtb "$SRCDIR/delete_in_section_badref.dts"
 
-- 
2.43.0

[PATCH v2 6/6] doc: dts-format: Describe later definitions and deletes

[Simon Glass]

dts-format.txt does not say that a node can be defined more than once,
nor describe /delete-property/ and /delete-node/, so the rules for
deletes in the earlier patches are not written down anywhere.

Add a section on how later definitions are merged and how deletes
behave: within a definition, before a node or property is defined
again, and in an overlay.

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

Changes in v2:
- Add new patch to document deletes

 Documentation/dts-format.txt | 44 ++++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/Documentation/dts-format.txt b/Documentation/dts-format.txt
index 41741df..d594822 100644
--- a/Documentation/dts-format.txt
+++ b/Documentation/dts-format.txt
@@ -116,6 +116,50 @@ Version 1 DTS files have the overall layout:
 * C style (/* ... */) and C++ style (// ...) comments are supported.
 
 
+Later definitions and deletes
+-----------------------------
+
+A node may be defined more than once, for example in an included file
+and again in the file which includes it. A later definition refers to
+the node by '&' followed by its label or full path in braces, or
+repeats the path from the root. The definitions are merged in source
+order: a property which is defined again takes the new value, and new
+properties and child nodes are added.
+
+	e.g. / {
+		serial: serial@1000 {
+			status = "disabled";
+			clock-frequency = <100000000>;
+		};
+	     };
+
+	     &serial {
+		status = "okay";
+		/delete-property/ clock-frequency;
+	     };
+
+Within a node definition, a property or child node can be removed with:
+	/delete-property/ property-name;
+	/delete-node/ node-name;
+and outside any node, a node can be removed with:
+	/delete-node/ &label;
+
+* A delete removes the property or node as defined so far, including
+  earlier in the same node definition, but not one defined after it.
+  Outside an overlay, a delete within a node definition with nothing to
+  remove is ignored.
+
+* A property or node which is deleted and then defined again comes
+  after the existing ones, like any new property or node. A node defined
+  again has only the properties and child nodes of its new definition.
+
+* In an overlay (a file with /plugin/), a delete can only remove
+  something defined earlier in the same fragment, i.e. the same
+  &label { ... } or &{/path} { ... }. An overlay cannot delete anything
+  from the tree it is applied to, so dtc reports an error for a delete
+  with nothing to remove in its fragment.
+
+
 
 	-- David Gibson <david@gibson.dropbear.id.au>
 	-- Yoder Stuart <stuart.yoder@freescale.com>
-- 
2.43.0

Re: [PATCH v2 3/6] dtc: Clarify when delete placeholders are dropped

[Herve Codina]

Hi Simon,

On Wed, 28 Oct 2026 03:36:10 -0600
Simon Glass <sjg@chromium.org> wrote:

> The comment on drop_delete_placeholders() does not make clear which
> subtrees it is for. The call in merge_nodes() also relies on sections
> being merged in source order, since it drops the child's placeholders on
> the basis that nothing earlier remains to be merged, but nothing there
> says so.
> 
> Say that drop_delete_placeholders() is for the first definition of a
> node, i.e. the first in the list of definitions which are merged
> together. Explain at the call in merge_nodes() why the child is the
> first definition, and that merging is not associative.
> 
> Suggested-by: David Gibson <david@gibson.dropbear.id.au>
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
> Changes in v2:
> - Add new patch to clarify the comments
> 
>  livetree.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/livetree.c b/livetree.c
> index 6609e4e..b7a5161 100644
> --- a/livetree.c
> +++ b/livetree.c
> @@ -122,8 +122,10 @@ static void apply_deletes(struct node *node)
>  
>  /*
>   * Drop the /delete-property/ and /delete-node/ placeholders from a subtree
> - * which joins the tree without being merged, since there is nothing earlier
> - * for them to delete
> + * which is the first definition of its node, i.e. the first in the list of
> + * definitions which are merged together. apply_deletes() has already dropped
> + * what they delete within the subtree, and there is nothing earlier for them
> + * to delete
>   */
>  void drop_delete_placeholders(struct node *node)
>  {
> @@ -273,6 +275,14 @@ struct node *merge_nodes(struct node *old_node, struct node *new_node)
>  
>  		/* if no collision occurred, add child to the old node. */
>  		if (new_child) {
> +			/*
> +			 * This is the first definition of the child, since
> +			 * sections are merged in source order, so its
> +			 * placeholders have nothing to delete. This is
> +			 * important since dropping them makes merging not
> +			 * associative: merging later sections together first
> +			 * would lose their deletes
> +			 */
>  			drop_delete_placeholders(new_child);
>  			add_child(old_node, new_child);
>  		}

drop_delete_placeholders() has been added in patch 1.

Why those both modification related to comment here in patch 3
Worth noting that patch 2 is only related to tests.

IHMO, this patch should be squashed in patch 1.

Best regards,
Hervé 

Re: [PATCH v2 4/6] tests: Allow dtc-fatal.sh to check the error message

[Herve Codina]

Hi Simon,

On Wed, 28 Oct 2026 03:36:11 -0600
Simon Glass <sjg@chromium.org> wrote:

> dtc-fatal.sh only checks that dtc exits with status 1, so a test which
> expects a particular error passes just as well if dtc fails for some
> other reason, such as a mistake in the test's dts file.
> 
> Add a -m option which also checks that dtc's output contains a given
> message. Use it for the nonexist-node-ref2 test.
> 
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
> Changes in v2:
> - Add new patch to check the error message in dtc-fatal.sh
> 
>  tests/dtc-fatal.sh | 18 +++++++++++++++++-
>  tests/run_tests.sh |  3 ++-
>  2 files changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/dtc-fatal.sh b/tests/dtc-fatal.sh
> index 08a4d29..2fa96ef 100755
> --- a/tests/dtc-fatal.sh
> +++ b/tests/dtc-fatal.sh
> @@ -1,9 +1,21 @@
>  #! /bin/sh
>  
> +# Usage: dtc-fatal.sh [-m <message>] <dtc args>
> +# Check that dtc fails, and with -m that its output contains <message>
> +
>  SRCDIR=`dirname "$0"`
>  . "$SRCDIR/testutils.sh"
>  
> -verbose_run $VALGRIND "$DTC" -o/dev/null "$@"
> +if [ "$1" = "-m" ]; then
> +    msg="$2"
> +    shift 2
> +fi
> +
> +LOG=tmp.log.$$
> +rm -f $LOG
> +trap "rm -f $LOG" 0
> +
> +verbose_run_log "$LOG" $VALGRIND "$DTC" -o/dev/null "$@"
>  ret="$?"
>  
>  if [ "$ret" -gt 127 ]; then
> @@ -12,4 +24,8 @@ elif [ "$ret" != "1" ]; then
>      FAIL "dtc returned incorrect status $ret instead of 1"
>  fi
>  
> +if [ -n "$msg" ] && ! grep -qF -- "$msg" "$LOG"; then
> +    FAIL "Expected message '$msg'"
> +fi
> +
>  PASS
> diff --git a/tests/run_tests.sh b/tests/run_tests.sh
> index 8813846..1018a6b 100755
> --- a/tests/run_tests.sh
> +++ b/tests/run_tests.sh
> @@ -760,7 +760,8 @@ dtc_tests () {
>      check_tests "$SRCDIR/minusone-phandle.dts" explicit_phandles
>      run_sh_test "$SRCDIR/dtc-checkfails.sh" phandle_references -- -I dts -O dtb "$SRCDIR/nonexist-node-ref.dts"
>      run_sh_test "$SRCDIR/dtc-checkfails.sh" phandle_references -- -I dts -O dtb "$SRCDIR/nonexist-label-ref.dts"
> -    run_sh_test "$SRCDIR/dtc-fatal.sh" -I dts -O dtb "$SRCDIR/nonexist-node-ref2.dts"
> +    run_sh_test "$SRCDIR/dtc-fatal.sh" -m "Label or path nosuchnode not found" \
> +	-I dts -O dtb "$SRCDIR/nonexist-node-ref2.dts"
>      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

Nice!

Can we imagine to add the '-m' option in other dtc-fatal.sh calls.

Best regards,
Hervé

Re: [PATCH v2 5/6] dtc: Report an error for a delete in an overlay fragment

[Herve Codina]

Hi Simon,

On Wed, 28 Oct 2026 03:36:12 -0600
Simon Glass <sjg@chromium.org> wrote:

> An overlay cannot encode a /delete-property/ or /delete-node/, since
> applying an overlay only adds or updates nodes and properties. So a
> delete in an overlay fragment has no effect on the base tree, yet dtc
> drops it without complaint, which suggests that it works. A delete which
> removes something earlier in the same fragment is fine, since the
> fragment is compiled without it.
> 
> Report an error for a /delete-property/ or /delete-node/ in a fragment
> which does not delete anything within the fragment, naming it. Record in
> apply_deletes() which placeholders delete something, and add
> find_unapplied_delete() to look for one which does not. Do this for each
> of the three ways in which a plugin creates a fragment.
> 
> Drop the deletes with nothing to delete from the overlay test, and add
> tests which check for the error, one for each way of creating a
> fragment.
> 
> Note: This breaks the build of two overlays in Linux v7.3-rc7, whose
> deletes have never taken effect:
> 
>    imx8mn-tqma8mqnl-mba8mx-usbotg.dtso
>    imx8mp-tqma8mpql-mba8mp-ras314-imx219.dtso
> 
> Linux needs to remove these deletes. Where the result matters, the base
> dts must change instead, e.g. by moving the property or node to the
> boards which need it, so that the overlay only adds to the tree.
> 
> Suggested-by: David Gibson <david@gibson.dropbear.id.au>
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 

/delete-node/ &label;

We can probably delete a node defined in the overlay but in an other
fragment.

Is than handled ?

Best regards,
Hervé

Re: [PATCH v2 6/6] doc: dts-format: Describe later definitions and deletes

[Herve Codina]

On Wed, 28 Oct 2026 03:36:13 -0600
Simon Glass <sjg@chromium.org> wrote:

> dts-format.txt does not say that a node can be defined more than once,
> nor describe /delete-property/ and /delete-node/, so the rules for
> deletes in the earlier patches are not written down anywhere.
> 
> Add a section on how later definitions are merged and how deletes
> behave: within a definition, before a node or property is defined
> again, and in an overlay.
> 
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
> Changes in v2:
> - Add new patch to document deletes
> 
>  Documentation/dts-format.txt | 44 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 44 insertions(+)
> 
> diff --git a/Documentation/dts-format.txt b/Documentation/dts-format.txt
> index 41741df..d594822 100644
> --- a/Documentation/dts-format.txt
> +++ b/Documentation/dts-format.txt
> @@ -116,6 +116,50 @@ Version 1 DTS files have the overall layout:
>  * C style (/* ... */) and C++ style (// ...) comments are supported.
>  
>  
> +Later definitions and deletes
> +-----------------------------
> +
> +A node may be defined more than once, for example in an included file
> +and again in the file which includes it. A later definition refers to
> +the node by '&' followed by its label or full path in braces, or
> +repeats the path from the root. The definitions are merged in source
> +order: a property which is defined again takes the new value, and new
> +properties and child nodes are added.
> +
> +	e.g. / {
> +		serial: serial@1000 {
> +			status = "disabled";
> +			clock-frequency = <100000000>;
> +		};
> +	     };
> +
> +	     &serial {
> +		status = "okay";
> +		/delete-property/ clock-frequency;
> +	     };
> +
> +Within a node definition, a property or child node can be removed with:
> +	/delete-property/ property-name;
> +	/delete-node/ node-name;
> +and outside any node, a node can be removed with:
> +	/delete-node/ &label;
> +
> +* A delete removes the property or node as defined so far, including
> +  earlier in the same node definition, but not one defined after it.
> +  Outside an overlay, a delete within a node definition with nothing to
> +  remove is ignored.
> +
> +* A property or node which is deleted and then defined again comes
> +  after the existing ones, like any new property or node. A node defined
> +  again has only the properties and child nodes of its new definition.
> +
> +* In an overlay (a file with /plugin/), a delete can only remove
> +  something defined earlier in the same fragment, i.e. the same
> +  &label { ... } or &{/path} { ... }. An overlay cannot delete anything
> +  from the tree it is applied to, so dtc reports an error for a delete
> +  with nothing to remove in its fragment.
> +
> +

Not so sure:
--- 8< ---
/plugin/

&target1 {
	node: node1 {
		...
	};
};

/delete-node/ &node;  <--- It is not defined in the *same* fragment
--- 8< ---

Agree that an overlay cannot delete something in the tree it is applied to.
But /delete-node/ can be outside any fragments and delete something in a
fragment.

Best regards,
Hervé

Re: [PATCH v2 0/6] dtc: Fix deletes within a section and before a redefinition

[David Gibson]

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

On Wed, Oct 28, 2026 at 03:36:07AM -0600, Simon Glass wrote:
> A /delete-property/ or /delete-node/ in the first section of a dts, or
> in a node which a later section adds, is ignored and leaves its
> placeholder in the tree, where it can show up as a bogus duplicate. A
> node which is deleted and then defined again also returns to its old
> place, with its old subnodes in their old order.
> 
> The first patch applies each delete within its own section and keeps
> the placeholders out of the tree. The second removes deleted entries
> from the tree, so that a node defined again comes after the existing
> ones. The third clarifies some comments. The fourth lets dtc-fatal.sh
> check the error message, which the fifth uses to test an error for a
> delete in an overlay fragment which has nothing to delete there, since
> an overlay cannot delete from the base tree. The sixth documents how
> deletes behave.
> 
> Note that the second and fifth patches change existing behaviour. The
> second changes the output of existing dts files which delete and then
> redefine a node or property. The fifth breaks the build of two overlays
> in Linux, whose deletes have never taken effect. Each patch explains
> why this seems the better behaviour.

Sorry, I didn't catch this in v1: I think this does improve behaviour,
but I don't think it's the right approach.

At the moment we handle the fragment merges right within the parser,
meaning we have a single unified tree as soon as we return from
yyparse().  That's already a bit confusing, and means behaviour isn't
quite the same between the first fragment listed and subsequent ones.

This series improves that consistency, but it does so with kind of
ad-hoc alterations which are pretty hard to keep track of.  It also
moves towards the model where later things can overwrite earlier
things, even within the same fragment, not just between fragments.
Although it is a way of making the semantics more consistent, I think
going that way makes it easier to have uncaught typos in the dt
source.

Here's the approach I think we should take instead.  I've had this in
mind for a while, but never got around to it.  Seeing this series kind
of crystalized the rationale for me.  It is more work, but I think it
will be worth it:

1. Have the parser treat each fragment / overlay separately, returning
   a forest of those pieces, rather than a single tree

  1a. We'll also want dt_from_blob() to parse dtbo /fragment@ pieces
      and convert those to a similar forest.  That can be implemented
      later on, though.

2a. For base trees, call a function from ain() which merges the forest
    into a single tree, with similar logic to what we have now.

2b. For /plugin/ trees, call a function from main() which wraps each
    parsed fragment in /frament@NN/__overlay__ and combines those
    into a single tree

3. Start splitting the checks into two sets.  One set runs *before*
   the merge on each individual fragment.  The other runs on the
   merged tree.

Under this scheme /delete/ placeholders will only exist in the
forest-of-fragments stage.  They'll either be resolved by actually
deleting the node/property in (2a), or trip an error in (2b).

This can be more gradual, but by moving things like the
duplicate_node_names checks to the pre-merge phase we can catch more
errors within fragments, without breaking either compile time or
runtime overlay semantics.

-- 
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 v2 4/6] tests: Allow dtc-fatal.sh to check the error message

[Herve Codina]

Hi Simon,

On Wed, 28 Oct 2026 03:36:11 -0600
Simon Glass <sjg@chromium.org> wrote:

> dtc-fatal.sh only checks that dtc exits with status 1, so a test which
> expects a particular error passes just as well if dtc fails for some
> other reason, such as a mistake in the test's dts file.
> 
> Add a -m option which also checks that dtc's output contains a given
> message. Use it for the nonexist-node-ref2 test.
> 
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 

On an other series which has been applied [1], new wrappers wrap_error_msg()
and run_wrap_error_msg_test() [2].

Those wrappers allow to check error messages.

It would be nice to have dtc-fatal.sh feature benefit of those wrappers
probably even removing the dtc-fatal.sh script itself.

[1]: https://lore.kernel.org/devicetree-compiler/auKqGJdeg-SZS_tp@gractus.seuss/
[2]: https://lore.kernel.org/devicetree-compiler/auKpLvtbaWufFMSr@gractus.seuss/

Best regards,
Hervé