Re: [PATCH v3 09/18] dtc: Move path-reference handling out of the grammar

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

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

On Fri, Nov 06, 2026 at 12:51:36PM -0700, Simon Glass wrote:
> The grammar action for a top-level &{/path} { ... } holds its merge
> logic, including making an overlay fragment in a plugin.
> 
> Move it into merge_ref(), which can tell a path from a label by looking
> at the reference, and use a single rule for both kinds of reference.
> 
> The output does not change.
> 
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
> (no changes since v2)
> 
> Changes in v2:
> - Combine merge_path_ref() into merge_ref()
> - Combine the rules for label and path references, using dt_ref
> 
>  dtc-parser.y | 27 +--------------------------
>  livetree.c   | 33 +++++++++++++++++++++++----------
>  2 files changed, 24 insertions(+), 36 deletions(-)
> 
> diff --git a/dtc-parser.y b/dtc-parser.y
> index 3614d71..3ec9254 100644
> --- a/dtc-parser.y
> +++ b/dtc-parser.y
> @@ -191,32 +191,7 @@ devicetree:
>  				ERROR(&@3, "Label or path %s not found", $3);
>  			$$ = $1;
>  		}
> -	| 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
> -			 * so $-1 is what we want (plugindecl)
> -			 */
> -			if ($<flags>-1 & DTSF_PLUGIN) {
> -				if (is_ref_relative($2))
> -					ERROR(&@2, "Label-relative reference %s not supported in plugin", $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 {
> -				if (target)
> -					merge_nodes(target, $3);
> -				else
> -					ERROR(&@2, "Label or path %s not found", $2);
> -			}
> -			$$ = $1;
> -		}
> -	| devicetree DT_LABEL_REF nodedef
> +	| devicetree dt_ref nodedef
>  		{
>  			struct fragment *f;
>  
> diff --git a/livetree.c b/livetree.c
> index 179866d..2e9a3b2 100644
> --- a/livetree.c
> +++ b/livetree.c
> @@ -574,29 +574,42 @@ bool is_ref_relative(const char *ref)
>  }
>  
>  /*
> - * Merge a fragment which refers to a label into the node with that label. In
> - * a plugin, a label which is not found becomes an overlay fragment.
> + * Merge a fragment which refers to a label or path into the node it refers to.
> + *
> + * In a plugin, a path always refers to the base tree and a label may do so, if
> + * it is not found. In both cases, the fragment becomes an overlay fragment.
>   *
>   * Returns true on success, false on an error, which has been reported.
>   */
>  bool merge_ref(struct node *dt, struct fragment *f, bool plugin)
>  {
> -	struct node *target = get_node_by_ref(dt, f->ref);
> +	struct node *target;
> +	bool ok = true;
>  
>  	assert(f->type == FRAGMENT_NODE);
> -	if (target) {
> -		merge_nodes(target, f->node);
> -		return true;
> +	if (plugin) {
> +		if (is_ref_relative(f->ref)) {
> +			srcpos_error(f->ref_srcpos, "Error",
> +				     "Label-relative reference %s not supported in plugin",
> +				     f->ref);

AFAICT, the fact that we can't encode label-relative references in
.dtbo is only relevant in the case that we can't resolve it locally.
So I think this should go within the next if.

> +			ok = false;
> +		}
> +		if (f->ref[0] == '/' || !get_node_by_ref(dt, f->ref)) {
> +			/* Add a fragment@N node for the reference */
> +			add_orphan_node(dt, f->node, f->ref);

Pre-existing, but what will add_orphan_node() actually *do* if given a
ref it has no way to encode in a dtbo?

> +			return ok;
> +		}
>  	}
> -	if (!plugin) {
> +
> +	target = get_node_by_ref(dt, f->ref);

You can leave the get_node_by_ref() at the top, since it's used in
both branches.

> +	if (!target) {
>  		srcpos_error(f->ref_srcpos, "Error",
>  			     "Label or path %s not found", f->ref);
>  		return false;
>  	}
> -	/* The label may be in the base tree, so add a fragment@N node for it */
> -	add_orphan_node(dt, f->node, f->ref);
> +	merge_nodes(target, f->node);

> -	return true;
> +	return ok;
>  }
>  
>  /*
> -- 
> 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 --]