On Wed, Aug 26, 2026 at 11:48:38AM +0200, Herve Codina wrote:
> REF_PHANDLE markers lack either the ref string or the is_local flag
> depending on the device-tree input format used.
> 
> When the marker is created from a dtb blob (via FDT_PROPDATA_PHANDLE),
> only the raw phandle value is available. When created from dts source,
> the ref string is present but the is_local flag is not set.
> 
> Add complete_references() to fill in the missing information.
> 
> Call the new function from build_dt_info() in order ensure that a
> dt_info is always fully complete and consistent.
> 
> Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> ---
>  livetree.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 82 insertions(+)
> 
> diff --git a/livetree.c b/livetree.c
> index a9f8c5d2..2ad7fd09 100644
> --- a/livetree.c
> +++ b/livetree.c
> @@ -465,6 +465,82 @@ static void fill_fullpaths(struct node *tree, const char *prefix)
>  		fill_fullpaths(child, tree->fullpath);
>  }
>  
> +static void complete_ref_and_is_local(struct dt_info *dti, char **ref,
> +				      bool *is_local, cell_t phandle)
> +{
> +	struct node *refnode;
> +
> +	/*
> +	 * When created from a dts, the reference is available but the is_local
> +	 * flag is not set.
> +	 *
> +	 * When created from a dtb, the is_local flag is set (and a valid
> +	 * phandle value is available) but not the reference.
> +	 *
> +	 * Complete both ref and is_local based on existing values.

The two cases share essentially zero code in this function, which
suggests to me they'd be better as different functions (each of which
could then have a more natural signature).

> +	 */
> +
> +	if (*ref) {
> +		refnode = get_node_by_ref(dti->dt, *ref);
> +		if (refnode)
> +			*is_local = true;
> +		else
> +			*is_local = false;
> +		return;
> +	}
> +
> +	if (*is_local) {
> +		refnode = get_node_by_phandle(dti->dt, phandle);
> +		if (!refnode)
> +			die("Node not found for phandle 0x%"PRIx32"\n", phandle);
> +
> +		*ref = refnode->fullpath;
> +		return;
> +	}
> +
> +	die("Cannot have a non local reference without a reference\n");
> +}
> +
> +static void complete_references_internal(struct dt_info *dti, struct node *node)
> +{
> +	struct property *prop;
> +	struct marker *m;
> +	struct node *c;
> +	cell_t phandle;
> +
> +	/*
> +	 * In order to have a complete state, a REF_PHANDLE marker needs to
> +	 * have a reference set (either a label or a path) and the is_local
> +	 * flag must be also correctly set.
> +	 *
> +	 * When created from dtb, references are not present in REF_PHANDLE
> +	 * markers created from the FDT_PROPDATA_PHANDLE tag. Indeed, only a
> +	 * phandle value is available with this tag.
> +	 *
> +	 * When created from dts, references are available for REF_PHANDLE
> +	 * markers but 'is_local' flag is not set.
> +	 *
> +	 * Completing a REF_PHANDLE marker consists in setting those fields.
> +	 */
> +	for_each_property(node, prop) {
> +		m = prop->val.markers;
> +		for_each_marker_of_type(m, REF_PHANDLE) {
> +			phandle = propval_cell_n(prop,
> +						 m->offset / sizeof(cell_t));
> +			complete_ref_and_is_local(dti, &m->ref, &m->is_local,
> +						  phandle);
> +		}
> +	}
> +
> +	for_each_child(node, c)
> +		complete_references_internal(dti, c);

Of course the change suggested above would mean that you need two
different cases in the callers as well.  I think that's worth it:
treating this as symmetric when we should know going in what direction
_all_ the fixups will go seems odd to me.  Or more concretely, I'm
concerned that if some bug means we get a marker in bogus state
(e.g. ref valid when coming from dtb) this approach means we'll carry
on sort of working, when it would be preferable to catch the bad state
and die earlier.

> +}
> +
> +static void complete_references(struct dt_info *dti)
> +{
> +	complete_references_internal(dti, dti->dt);
> +}
> +
>  struct dt_info *build_dt_info(unsigned int dtsflags,
>  			      struct reserve_info *reservelist,
>  			      struct node *tree, uint32_t boot_cpuid_phys)
> @@ -479,6 +555,12 @@ struct dt_info *build_dt_info(unsigned int dtsflags,
>  
>  	fill_fullpaths(dti->dt, "");
>  
> +	/*
> +	 * Complete references (REF_PHANDLE).
> +	 * Set ref strings and set is_local flags
> +	 */
> +	complete_references(dti);
> +
>  	return dti;
>  }
>  
> -- 
> 2.55.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