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

[David Gibson <david@gibson.dropbear.id.au>]

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

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

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

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

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

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