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

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

[David Gibson]

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

On Thu, Oct 29, 2026 at 12:11:52PM +0100, Herve Codina wrote:
> 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.

So, this is reminding me why I think dtc-fatal.sh existed in the first
place:
  1. We want to wrap all dtc invocations in $VALGRIND
  
  2. We don't want to use valgrind on other commands we invoke via
     wrap_error

  3. We want to avoid individual test invocations from dealing with
     $VALGRIND

Hence, two different wrappers: run_wrap_error_test for general
commands, dtc-fatal.sh for errors from dtc specifically.  Why one is a
function and one a separate script I don't recall, although I have a
nasty feeling there might have been a particular reason for having the
script, though I've forgotten what it might have been.

I can think of one more modern reason to prefer scripts over shell
functions:  the current integration of run_tests.sh into meson is
pretty ugly.  Rather than using meson to manage testcases (which it
can do reasonably well), we just invoke each "set" of tests from
run-tests.sh as a single meson testcase.  That means more levels of
logs to figure out exactly what went wrong for a meson test failure.

At some point it might be nice to eliminate run_tests.sh and make the
testsuite meson native.  For that, shell function wrappers are
obviously not ideal.

The split between patches 1 & 2 here isn't ideal, btw.  The VALGRIND
behaviour change is correct, but not logicall connected to the
run_wrap_error_test / dtc-fatal.sh consolidation.  Meanwhile removing
dtc-fatal.sh _does_ belong with the patch that makes it possible
(given that's a single fairly small patch).

So, where next.

I think keep dtc-fatal.sh for the time being, but extend it with the
-m option to check for specific messages.  Consider moving
run_wrap_error_test into its own script (which dtc-fatal.sh could
call).


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

-- 
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 1/4] tests: Check the message when dtc fails

[Herve Codina]

Hi,

On Fri, 30 Oct 2026 14:36:22 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

...

> > > @@ -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.  
> 
> So, this is reminding me why I think dtc-fatal.sh existed in the first
> place:
>   1. We want to wrap all dtc invocations in $VALGRIND
>   
>   2. We don't want to use valgrind on other commands we invoke via
>      wrap_error
> 
>   3. We want to avoid individual test invocations from dealing with
>      $VALGRIND
> 
> Hence, two different wrappers: run_wrap_error_test for general
> commands, dtc-fatal.sh for errors from dtc specifically.  Why one is a
> function and one a separate script I don't recall, although I have a
> nasty feeling there might have been a particular reason for having the
> script, though I've forgotten what it might have been.
> 
> I can think of one more modern reason to prefer scripts over shell
> functions:  the current integration of run_tests.sh into meson is
> pretty ugly.  Rather than using meson to manage testcases (which it
> can do reasonably well), we just invoke each "set" of tests from
> run-tests.sh as a single meson testcase.  That means more levels of
> logs to figure out exactly what went wrong for a meson test failure.
> 
> At some point it might be nice to eliminate run_tests.sh and make the
> testsuite meson native.  For that, shell function wrappers are
> obviously not ideal.

Have you consider switching current tests to TAP (Test Anything Protocol)?

TAP is supported by meson test.

If at some point the plan is to move current tests to a a better meson
integration, I suggest to consider TAP. Indeed, this will allow a finer
grain in test results compared to a meson basic executable test.

We can move tests from current tests to TAP by parts to ease this moving
task.

run_test.sh can provide its output in TAP format.

Cleaner test harnesses are create step by step by extracting parts from
run_test.sh until run_test.sh is empty.

Also all new tests should be TAP tests.

Libraries exist to help building TAP tests. One lib to chose for C test
program (if they are run out of a test shell script) and an other for
Shell test scripts should be enough.

> 
> The split between patches 1 & 2 here isn't ideal, btw.  The VALGRIND
> behaviour change is correct, but not logicall connected to the
> run_wrap_error_test / dtc-fatal.sh consolidation.  Meanwhile removing
> dtc-fatal.sh _does_ belong with the patch that makes it possible
> (given that's a single fairly small patch).
> 
> So, where next.
> 
> I think keep dtc-fatal.sh for the time being, but extend it with the
> -m option to check for specific messages.  Consider moving
> run_wrap_error_test into its own script (which dtc-fatal.sh could
> call).

Ok.

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é

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

[David Gibson]

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

On Thu, Oct 29, 2026 at 03:30:56AM -0600, Simon Glass 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.

This is good information to add, but I think it can be expressed
better than this draft.

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

This is true, but kind of misleading.  Being able to "double define"
isn't really inherent to include files.  The problem here is that this
document never describes source-level overlays / fragments.  The "File
layout" section should be updated to cover that, rather than amending
the description there in a later section.

I think we want to be explicit about how those fragments overlay /
overwrite each other as a way to describing why and when a second node
definition makes sense.  As noted in my comments on the semantic
changes for merges, I think it would be preferable to _only_ allow
those overwrites on fragment merges, not allow them within a single
fragment (though of course either would be better than doing it
inconsistently as we do now).

Describing that source level fragment syntax would also then lead
naturally into extending this document to cover the /plugin/
directive and source for dtbo files.

It might be safer to leave updating this file until we've finalised
the cleanup of the actual semantics of merge, to make sure it remains
accurate.  This document has been wrong (or at least, incomplete) for
a decade, it can wait a little longer.

(Also, btw, converting the doc to Markdown might be a nice idea).


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

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

[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 4/4] doc: dts-format: Correct what happens without /dts-v1/

[David Gibson]

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

On Thu, Oct 29, 2026 at 12:33:59PM +0100, Herve Codina wrote:
> 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>

I'm merging this with a slightly updated version of that change.

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

-- 
David Gibson (he or they)	| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
				| around.
http://www.ozlabs.org/~dgibson

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

Re: [PATCH 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é

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

[David Gibson]

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

On Thu, Oct 29, 2026 at 12:17:38PM +0100, Herve Codina wrote:
> 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

Theoretically maybe, but when both parts are this small it's fine.

> Except if David applies patches 3 and 4 (dts-format.txt update) from this
> iteration.
> 
> Best regards,
> Hervé
> 

-- 
David Gibson (he or they)	| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
				| around.
http://www.ozlabs.org/~dgibson

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