All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Gibson <david@gibson.dropbear.id.au>
To: Simon Glass <sjg@chromium.org>
Cc: Devicetree Compiler <devicetree-compiler@vger.kernel.org>
Subject: Re: [PATCH 01/13] tests: Check each error when merging a dts file
Date: Mon, 2 Nov 2026 17:32:10 +1100	[thread overview]
Message-ID: <auguYCmZz3fSo3eB@gractus.seuss> (raw)
In-Reply-To: <20261101163721.2458982-2-sjg@chromium.org>

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

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

Couple of nits, otherwise LGTM.

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

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

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

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

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

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

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

  reply	other threads:[~2026-11-02  8:16 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-01 16:36 [PATCH 00/13] dtc: Merge the fragments of a dts file after parsing Simon Glass
2026-11-01 16:36 ` [PATCH 01/13] tests: Check each error when merging a dts file Simon Glass
2026-11-02  6:32   ` David Gibson [this message]
2026-11-01 16:36 ` [PATCH 02/13] dtc: Add a list for the top-level fragments of " Simon Glass
2026-11-02  9:49   ` David Gibson
2026-11-03 17:56     ` Herve Codina
2026-11-03 17:07   ` Herve Codina
2026-11-01 16:36 ` [PATCH 03/13] dtc: Move the /delete-node/ handling out of the grammar Simon Glass
2026-11-02  9:55   ` David Gibson
2026-11-01 16:36 ` [PATCH 04/13] dtc: Move the /omit-if-no-ref/ " Simon Glass
2026-11-02 10:03   ` David Gibson
2026-11-01 16:36 ` [PATCH 05/13] dtc: Move is_ref_relative() to livetree.c Simon Glass
2026-11-02 10:04   ` David Gibson
2026-11-01 16:37 ` [PATCH 06/13] dtc: Move label-reference handling out of the grammar Simon Glass
2026-11-03  7:25   ` David Gibson
2026-11-01 16:37 ` [PATCH 07/13] dtc: Move path-reference " Simon Glass
2026-11-03  7:27   ` David Gibson
2026-11-01 16:37 ` [PATCH 08/13] dtc: Move labelled-reference " Simon Glass
2026-11-03  7:29   ` David Gibson
2026-11-01 16:37 ` [PATCH 09/13] dtc: Move first-reference " Simon Glass
2026-11-01 16:37 ` [PATCH 10/13] dtc: Add merge_fragment() to merge any fragment Simon Glass
2026-11-01 16:37 ` [PATCH 11/13] dtc: Add helpers for the errors when merging a fragment Simon Glass
2026-11-03 17:23   ` Herve Codina
2026-11-01 16:37 ` [PATCH 12/13] dtc: Merge the fragments of a dts file after parsing Simon Glass
2026-11-03 17:07   ` Herve Codina
2026-11-01 16:37 ` [PATCH 13/13] dtc: Merge the fragments of a dts file in main() Simon Glass
2026-11-03 17:08   ` Herve Codina

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=auguYCmZz3fSo3eB@gractus.seuss \
    --to=david@gibson.dropbear.id.au \
    --cc=devicetree-compiler@vger.kernel.org \
    --cc=sjg@chromium.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.