[PATCH v3 0/4] dtc: Check fatal error messages and document deletes

[PATCH v3 0/4] dtc: Check fatal error messages and document deletes

[Simon Glass]

Tests which expect dtc to fail check only its exit status, so they pass
just as well if dtc fails for some other reason. Also, dts-format.txt
does not describe how a node defined more than once is merged, nor
/delete-property/ and /delete-node/, and it wrongly says that dtc
accepts a file without /dts-v1/ as version 0.

I've dropped the 'delete' patches from this version - it's going to be
a bigger job!

Changes in v3:
- Use run_wrap_error_msg_test() instead of adding -m to dtc-fatal.sh
- Check the message in all five tests
- Add new patch to drop dtc-fatal.sh
- Describe only the general rules, leaving out the edge cases
- Drop the four patches which change how deletes are applied
- Add new patch to correct the description of /dts-v1/

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

Simon Glass (4):
  tests: Check the message when dtc fails
  tests: Drop dtc-fatal.sh
  doc: dts-format: Describe later definitions and deletes
  doc: dts-format: Correct what happens without /dts-v1/

 Documentation/dts-format.txt | 40 +++++++++++++++++++++++++++++++++---
 tests/dtc-fatal.sh           | 15 --------------
 tests/run_tests.sh           | 18 +++++++++++-----
 3 files changed, 50 insertions(+), 23 deletions(-)
 delete mode 100755 tests/dtc-fatal.sh

---
base-commit: 6b0371afc5329d7f07cc4f1ff4c5e10b598fadf1
branch: delete-markers3

-- 
2.43.0

[PATCH v3 1/4] tests: Check the message when dtc fails

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

Use run_wrap_error_msg_test() instead, which also checks that dtc's
output contains the expected message.

dtc-fatal.sh also catches a valgrind error, since valgrind exits with
a status other than 1. Make wrap_error_msg() report a valgrind error as
wrap_test() does, so this is not lost.

Suggested-by: Herve Codina <herve.codina@bootlin.com>
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v3:
- Use run_wrap_error_msg_test() instead of adding -m to dtc-fatal.sh
- Check the message in all five tests

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

 tests/run_tests.sh | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/tests/run_tests.sh b/tests/run_tests.sh
index 5bc7817..d3134f2 100755
--- a/tests/run_tests.sh
+++ b/tests/run_tests.sh
@@ -193,6 +193,9 @@ wrap_error_msg () {
 	FAIL_IF_SIGNAL $ret
 	if [ "$ret" -eq 0 ]; then
 	    FAIL "Expected non-zero return code"
+	elif [ "$ret" -eq $VGCODE ]; then
+	    echo "VALGRIND ERROR"
+	    exit $VGCODE
 	fi
 	if ! grep -qF -- "$msg" "$log"; then
 	    FAIL "Expected message '$msg'"
@@ -319,7 +322,8 @@ libfdt_overlay_tests () {
 	run_dtc_test -I dts -O dtb -o $tree.test.dtb "$SRCDIR/$tree.dts"
 	run_test overlay_bad_fixup overlay_base_manual_symbols.test.dtb $tree.test.dtb
     done
-    run_sh_test "$SRCDIR/dtc-fatal.sh" -I dts -O dtb -o /dev/null "$SRCDIR/fixup-ref-to-path.dts"
+    run_wrap_error_msg_test "Can't generate fixup for reference to path" \
+	$VALGRIND $DTC -I dts -O dtb -o /dev/null "$SRCDIR/fixup-ref-to-path.dts"
 }
 
 # Tests to exercise dtc's overlay generation support
@@ -764,7 +768,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_wrap_error_msg_test "Label or path nosuchnode not found" \
+	$VALGRIND $DTC -I dts -O dtb -o /dev/null "$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
@@ -847,9 +852,12 @@ dtc_tests () {
     run_test integer-expressions integer-expressions.test.dtb
 
     # Check for graceful failure in some error conditions
-    run_sh_test "$SRCDIR/dtc-fatal.sh" -I dts -O dtb nosuchfile.dts
-    run_sh_test "$SRCDIR/dtc-fatal.sh" -I dtb -O dtb nosuchfile.dtb
-    run_sh_test "$SRCDIR/dtc-fatal.sh" -I fs -O dtb nosuchfile
+    run_wrap_error_msg_test "Couldn't open \"nosuchfile.dts\"" \
+	$VALGRIND $DTC -I dts -O dtb -o /dev/null nosuchfile.dts
+    run_wrap_error_msg_test "Couldn't open \"nosuchfile.dtb\"" \
+	$VALGRIND $DTC -I dtb -O dtb -o /dev/null nosuchfile.dtb
+    run_wrap_error_msg_test "Couldn't opendir() \"nosuchfile\"" \
+	$VALGRIND $DTC -I fs -O dtb -o /dev/null nosuchfile
 
     # Dependencies
     run_dtc_test -I dts -O dtb -o dependencies.test.dtb -d dependencies.test.d "$SRCDIR/dependencies.dts"
-- 
2.43.0

Re: [PATCH v3 1/4] tests: Check the message when dtc fails

[Herve Codina]

Hi Simon,

On Thu, 29 Oct 2026 03:30:54 -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.
> 
> Use run_wrap_error_msg_test() instead, which also checks that dtc's
> output contains the expected message.
> 
> dtc-fatal.sh also catches a valgrind error, since valgrind exits with
> a status other than 1. Make wrap_error_msg() report a valgrind error as
> wrap_test() does, so this is not lost.
> 
> Suggested-by: Herve Codina <herve.codina@bootlin.com>
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
> Changes in v3:
> - Use run_wrap_error_msg_test() instead of adding -m to dtc-fatal.sh
> - Check the message in all five tests
> 
> Changes in v2:
> - Add new patch to check the error message in dtc-fatal.sh
> 
>  tests/run_tests.sh | 18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/tests/run_tests.sh b/tests/run_tests.sh
> index 5bc7817..d3134f2 100755
> --- a/tests/run_tests.sh
> +++ b/tests/run_tests.sh
> @@ -193,6 +193,9 @@ wrap_error_msg () {
>  	FAIL_IF_SIGNAL $ret
>  	if [ "$ret" -eq 0 ]; then
>  	    FAIL "Expected non-zero return code"
> +	elif [ "$ret" -eq $VGCODE ]; then
> +	    echo "VALGRIND ERROR"
> +	    exit $VGCODE
>  	fi
>  	if ! grep -qF -- "$msg" "$log"; then
>  	    FAIL "Expected message '$msg'"
> @@ -319,7 +322,8 @@ libfdt_overlay_tests () {
>  	run_dtc_test -I dts -O dtb -o $tree.test.dtb "$SRCDIR/$tree.dts"
>  	run_test overlay_bad_fixup overlay_base_manual_symbols.test.dtb $tree.test.dtb
>      done
> -    run_sh_test "$SRCDIR/dtc-fatal.sh" -I dts -O dtb -o /dev/null "$SRCDIR/fixup-ref-to-path.dts"
> +    run_wrap_error_msg_test "Can't generate fixup for reference to path" \
> +	$VALGRIND $DTC -I dts -O dtb -o /dev/null "$SRCDIR/fixup-ref-to-path.dts"

I don't think that the $VALGRIND here is a good idea. Also this is not consistent
with other dtc test calls. Those tests calls (other than using a script) use
run_dtc_test(). run_dtc_test() use $VALGRIND internally.

Could we imagine run_dtc_fatal_test() (or similar name) to call here in order
to be consistent with other calls?

This will look like the following:
--- 8< ---
	run_dtc_fatal_test "Can't generate fixup for reference to path" \
			   -I dts -O dtb -o /dev/null "$SRCDIR/fixup-ref-to-path.dts"
--- 8< ---

At some point, also dtc-checkfails.sh script could be replace by
run_dtc_checkfails_test wrapper and dtc-fails.sh by run_dtc_checkfails_test
wrapper.

Best regards,
Hervé

[PATCH v3 2/4] tests: Drop dtc-fatal.sh

[Simon Glass]

Nothing uses dtc-fatal.sh now that the tests which expect dtc to fail
use run_wrap_error_msg_test() to check its message.

Remove it.

Suggested-by: Herve Codina <herve.codina@bootlin.com>
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v3:
- Add new patch to drop dtc-fatal.sh

 tests/dtc-fatal.sh | 15 ---------------
 1 file changed, 15 deletions(-)
 delete mode 100755 tests/dtc-fatal.sh

diff --git a/tests/dtc-fatal.sh b/tests/dtc-fatal.sh
deleted file mode 100755
index 08a4d29..0000000
--- a/tests/dtc-fatal.sh
+++ /dev/null
@@ -1,15 +0,0 @@
-#! /bin/sh
-
-SRCDIR=`dirname "$0"`
-. "$SRCDIR/testutils.sh"
-
-verbose_run $VALGRIND "$DTC" -o/dev/null "$@"
-ret="$?"
-
-if [ "$ret" -gt 127 ]; then
-    FAIL "dtc killed by signal (ret=$ret)"
-elif [ "$ret" != "1" ]; then
-    FAIL "dtc returned incorrect status $ret instead of 1"
-fi
-
-PASS
-- 
2.43.0

[PATCH v3 3/4] 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 anyone wanting to
use them has to learn from examples or the parser.

Add a section on how later definitions are merged, how to delete a
property or node, and that an overlay cannot delete anything from the
tree it is applied to.

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

Changes in v3:
- Describe only the general rules, leaving out the edge cases

Changes in v2:
- Add new patch to document deletes

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

diff --git a/Documentation/dts-format.txt b/Documentation/dts-format.txt
index 41741df..da0783b 100644
--- a/Documentation/dts-format.txt
+++ b/Documentation/dts-format.txt
@@ -87,6 +87,41 @@ elements, as NUL-terminated strings, as bytestrings or a combination of these.
 	e.g. prop = [ab cd ef byte4: 00 ff fe];
 	e.g. str = start: "string value" end: ;
 
+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 in a later definition removes the property or node from the
+earlier ones. Like child nodes, /delete-node/ must come after any
+properties in a node definition.
+
+An overlay (a file with /plugin/) cannot delete anything from the tree
+it is applied to, since the overlay format has no way to express this.
+
 
 File layout
 -----------
-- 
2.43.0

Re: [PATCH v3 3/4] doc: dts-format: Describe later definitions and deletes

[Herve Codina]

On Thu, 29 Oct 2026 03:30:56 -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 anyone wanting to
> use them has to learn from examples or the parser.
> 
> Add a section on how later definitions are merged, how to delete a
> property or node, and that an overlay cannot delete anything from the
> tree it is applied to.
> 
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
> Changes in v3:
> - Describe only the general rules, leaving out the edge cases
> 
> Changes in v2:
> - Add new patch to document deletes
> 
>  Documentation/dts-format.txt | 35 +++++++++++++++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
> 
> diff --git a/Documentation/dts-format.txt b/Documentation/dts-format.txt
> index 41741df..da0783b 100644
> --- a/Documentation/dts-format.txt
> +++ b/Documentation/dts-format.txt
> @@ -87,6 +87,41 @@ elements, as NUL-terminated strings, as bytestrings or a combination of these.
>  	e.g. prop = [ab cd ef byte4: 00 ff fe];
>  	e.g. str = start: "string value" end: ;
>  
> +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 in a later definition removes the property or node from the
> +earlier ones. Like child nodes, /delete-node/ must come after any
> +properties in a node definition.
> +
> +An overlay (a file with /plugin/) cannot delete anything from the tree
> +it is applied to, since the overlay format has no way to express this.
> +
>  
>  File layout
>  -----------

Sounds good to me.

Reviewed-by: Herve Codina <herve.codina@bootlin.com>

Best regards,
Hervé

[PATCH v3 4/4] doc: dts-format: Correct what happens without /dts-v1/

[Simon Glass]

dts-format.txt says that dtc treats a file without /dts-v1/ as the
obsolete version 0 format. That has not been true since commit
4e1a0a0129b3 ("Remove support for the legacy DTS source file format."),
and dtc now gives a syntax error instead.

Say that dtc does not accept such files.

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

Changes in v3:
- Add new patch to correct the description of /dts-v1/

 Documentation/dts-format.txt | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/Documentation/dts-format.txt b/Documentation/dts-format.txt
index da0783b..87d3d8e 100644
--- a/Documentation/dts-format.txt
+++ b/Documentation/dts-format.txt
@@ -137,9 +137,8 @@ Version 1 DTS files have the overall layout:
 	};
 
 * The "/dts-v1/;" must be present to identify the file as a version 1
-  DTS (dts files without this tag will be treated by dtc as being in
-  the obsolete "version 0", which uses a different format for integers
-  amongst other small but incompatible changes).
+  DTS. dtc does not accept files without it, which would be in the
+  obsolete "version 0" format.
 
 * Memory reservations define an entry for the device tree blob's
   memory reservation table.  They have the form:
-- 
2.43.0

Re: [PATCH v3 4/4] doc: dts-format: Correct what happens without /dts-v1/

[Herve Codina]

Hi Simon,

On Thu, 29 Oct 2026 03:30:57 -0600
Simon Glass <sjg@chromium.org> wrote:

> dts-format.txt says that dtc treats a file without /dts-v1/ as the
> obsolete version 0 format. That has not been true since commit
> 4e1a0a0129b3 ("Remove support for the legacy DTS source file format."),
> and dtc now gives a syntax error instead.
> 
> Say that dtc does not accept such files.
> 
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
> Changes in v3:
> - Add new patch to correct the description of /dts-v1/
> 
>  Documentation/dts-format.txt | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/dts-format.txt b/Documentation/dts-format.txt
> index da0783b..87d3d8e 100644
> --- a/Documentation/dts-format.txt
> +++ b/Documentation/dts-format.txt
> @@ -137,9 +137,8 @@ Version 1 DTS files have the overall layout:
>  	};
>  
>  * The "/dts-v1/;" must be present to identify the file as a version 1
> -  DTS (dts files without this tag will be treated by dtc as being in
> -  the obsolete "version 0", which uses a different format for integers
> -  amongst other small but incompatible changes).
> +  DTS. dtc does not accept files without it, which would be in the
> +  obsolete "version 0" format.

     obsolete and no more supported "version 0" format.

Just to be clear about the "version 0" format.

With that done:
Reviewed-by: Herve Codina <herve.codina@bootlin.com>

>  
>  * Memory reservations define an entry for the device tree blob's
>    memory reservation table.  They have the form:

Best regards,
Hervé

Re: [PATCH v3 0/4] dtc: Check fatal error messages and document deletes

[Herve Codina]

On Thu, 29 Oct 2026 03:30:53 -0600
Simon Glass <sjg@chromium.org> wrote:

> Tests which expect dtc to fail check only its exit status, so they pass
> just as well if dtc fails for some other reason. Also, dts-format.txt
> does not describe how a node defined more than once is merged, nor
> /delete-property/ and /delete-node/, and it wrongly says that dtc
> accepts a file without /dts-v1/ as version 0.

Hum, got the feeling that the two different topics in this series could be
sent in two different series:
 - dts-format.txt update
 - dtc-fatal.sh removal

Except if David applies patches 3 and 4 (dts-format.txt update) from this
iteration.

Best regards,
Hervé