[RFC PATCH 09/77] dtc: Introduce mark_local_phandles()

Herve Codina posted 77 patches 3 weeks, 5 days ago
[RFC PATCH 09/77] dtc: Introduce mark_local_phandles()
Posted by Herve Codina 3 weeks, 5 days ago
In order to have the new FDT_REF_LOCAL tag present in a dtb, the phandle
reference needs to be identify as a local reference.

This is the purpose of mark_local_phandles().

It identifies a phandle reference as a local reference when this
reference points to a local node.

With that node, the related FDT_REF_LOCAL tag is set in the dtb.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
 dtc.c      |  1 +
 dtc.h      |  1 +
 livetree.c | 26 ++++++++++++++++++++++++++
 3 files changed, 28 insertions(+)

diff --git a/dtc.c b/dtc.c
index 88f03ff..d0b4de3 100644
--- a/dtc.c
+++ b/dtc.c
@@ -334,6 +334,7 @@ int main(int argc, char *argv[])
 	}
 
 	update_phandles_ref(dti);
+	mark_local_phandles(dti);
 
 	process_checks(force, dti);
 
diff --git a/dtc.h b/dtc.h
index 351fe41..08c9f07 100644
--- a/dtc.h
+++ b/dtc.h
@@ -346,6 +346,7 @@ void generate_fixups_tree(struct dt_info *dti, const char *name);
 void generate_local_fixups_tree(struct dt_info *dti, const char *name);
 
 void update_phandles_ref(struct dt_info *dti);
+void mark_local_phandles(struct dt_info *dti);
 
 /* Checks */
 
diff --git a/livetree.c b/livetree.c
index 9e30a63..2a0a7ed 100644
--- a/livetree.c
+++ b/livetree.c
@@ -1196,3 +1196,29 @@ void update_phandles_ref(struct dt_info *dti)
 {
 	update_phandles_ref_internal(dti, dti->dt);
 }
+
+static void mark_local_phandles_internal(struct dt_info *dti,
+					 struct node *node)
+{
+	struct node *c;
+	struct property *prop;
+	struct marker *m;
+	struct node *refnode;
+
+	for_each_property(node, prop) {
+		m = prop->val.markers;
+		for_each_marker_of_type(m, REF_PHANDLE) {
+			refnode = get_node_by_ref(dti->dt, m->ref);
+			if (refnode)
+				m->is_local = true;
+		}
+	}
+
+	for_each_child(node, c)
+		mark_local_phandles_internal(dti, c);
+}
+
+void mark_local_phandles(struct dt_info *dti)
+{
+	mark_local_phandles_internal(dti, dti->dt);
+}
-- 
2.52.0
Re: [RFC PATCH 09/77] dtc: Introduce mark_local_phandles()
Posted by David Gibson 3 weeks, 3 days ago
On Mon, Jan 12, 2026 at 03:18:59PM +0100, Herve Codina wrote:
> In order to have the new FDT_REF_LOCAL tag present in a dtb, the phandle
> reference needs to be identify as a local reference.
> 
> This is the purpose of mark_local_phandles().
> 
> It identifies a phandle reference as a local reference when this
> reference points to a local node.
> 
> With that node, the related FDT_REF_LOCAL tag is set in the dtb.

I dislike caching redundant information (whether the ref is local) -
it's an opportunity for them to get out of sync and cause bugs.  Is
there a strong case that you can't just determine whether it's local
only when you actually go to use it?

> 
> Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> ---
>  dtc.c      |  1 +
>  dtc.h      |  1 +
>  livetree.c | 26 ++++++++++++++++++++++++++
>  3 files changed, 28 insertions(+)
> 
> diff --git a/dtc.c b/dtc.c
> index 88f03ff..d0b4de3 100644
> --- a/dtc.c
> +++ b/dtc.c
> @@ -334,6 +334,7 @@ int main(int argc, char *argv[])
>  	}
>  
>  	update_phandles_ref(dti);
> +	mark_local_phandles(dti);
>  
>  	process_checks(force, dti);
>  
> diff --git a/dtc.h b/dtc.h
> index 351fe41..08c9f07 100644
> --- a/dtc.h
> +++ b/dtc.h
> @@ -346,6 +346,7 @@ void generate_fixups_tree(struct dt_info *dti, const char *name);
>  void generate_local_fixups_tree(struct dt_info *dti, const char *name);
>  
>  void update_phandles_ref(struct dt_info *dti);
> +void mark_local_phandles(struct dt_info *dti);
>  
>  /* Checks */
>  
> diff --git a/livetree.c b/livetree.c
> index 9e30a63..2a0a7ed 100644
> --- a/livetree.c
> +++ b/livetree.c
> @@ -1196,3 +1196,29 @@ void update_phandles_ref(struct dt_info *dti)
>  {
>  	update_phandles_ref_internal(dti, dti->dt);
>  }
> +
> +static void mark_local_phandles_internal(struct dt_info *dti,
> +					 struct node *node)
> +{
> +	struct node *c;
> +	struct property *prop;
> +	struct marker *m;
> +	struct node *refnode;
> +
> +	for_each_property(node, prop) {
> +		m = prop->val.markers;
> +		for_each_marker_of_type(m, REF_PHANDLE) {
> +			refnode = get_node_by_ref(dti->dt, m->ref);
> +			if (refnode)
> +				m->is_local = true;
> +		}
> +	}
> +
> +	for_each_child(node, c)
> +		mark_local_phandles_internal(dti, c);
> +}
> +
> +void mark_local_phandles(struct dt_info *dti)
> +{
> +	mark_local_phandles_internal(dti, dti->dt);
> +}
> -- 
> 2.52.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
Re: [RFC PATCH 09/77] dtc: Introduce mark_local_phandles()
Posted by Herve Codina 3 weeks, 1 day ago
Hi David,

On Thu, 15 Jan 2026 11:48:44 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Mon, Jan 12, 2026 at 03:18:59PM +0100, Herve Codina wrote:
> > In order to have the new FDT_REF_LOCAL tag present in a dtb, the phandle
> > reference needs to be identify as a local reference.
> > 
> > This is the purpose of mark_local_phandles().
> > 
> > It identifies a phandle reference as a local reference when this
> > reference points to a local node.
> > 
> > With that node, the related FDT_REF_LOCAL tag is set in the dtb.  
> 
> I dislike caching redundant information (whether the ref is local) -
> it's an opportunity for them to get out of sync and cause bugs.  Is
> there a strong case that you can't just determine whether it's local
> only when you actually go to use it?

Well, I can't find any strong case.

I would like to avoid passing the full dti (struct dt_info) to flatten_tree()
in order to determine if the ref is local or not to set a FDT_REF_LOCAL or
a FDT_REF_PHANDLE tag.

Also, this flag, set when a FDT_REF_LOCAL tag is parsed from a dtb, is
useful later when the ref has to be found based on the phandle value.

Indeed, because the is_local flag is set, the phandle value available in the
property *must* reference an existing node in the dtb.

In other word, in update_phandles_ref_internal(), 
--- 8< ---
	if (m->is_local) {
		phandle = propval_cell_n(prop,
					 m->offset / sizeof(cell_t));
		refnode = dti_get_node_by_phandle(dti, phandle);
		if (!refnode)
			die("Node not found for phandle 0x%"PRIx32"\n", phandle);

		m->ref = refnode->fullpath;
		continue;
	} else {
		...
--- 8< ---

Best regards,
Hervé
Re: [RFC PATCH 09/77] dtc: Introduce mark_local_phandles()
Posted by David Gibson 2 weeks, 6 days ago
On Fri, Jan 16, 2026 at 02:09:12PM +0100, Herve Codina wrote:
> Hi David,
> 
> On Thu, 15 Jan 2026 11:48:44 +1100
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > On Mon, Jan 12, 2026 at 03:18:59PM +0100, Herve Codina wrote:
> > > In order to have the new FDT_REF_LOCAL tag present in a dtb, the phandle
> > > reference needs to be identify as a local reference.
> > > 
> > > This is the purpose of mark_local_phandles().
> > > 
> > > It identifies a phandle reference as a local reference when this
> > > reference points to a local node.
> > > 
> > > With that node, the related FDT_REF_LOCAL tag is set in the dtb.  
> > 
> > I dislike caching redundant information (whether the ref is local) -
> > it's an opportunity for them to get out of sync and cause bugs.  Is
> > there a strong case that you can't just determine whether it's local
> > only when you actually go to use it?
> 
> Well, I can't find any strong case.
> 
> I would like to avoid passing the full dti (struct dt_info) to flatten_tree()
> in order to determine if the ref is local or not to set a FDT_REF_LOCAL or
> a FDT_REF_PHANDLE tag.
> 
> Also, this flag, set when a FDT_REF_LOCAL tag is parsed from a dtb, is
> useful later when the ref has to be found based on the phandle value.
> 
> Indeed, because the is_local flag is set, the phandle value available in the
> property *must* reference an existing node in the dtb.
> 
> In other word, in update_phandles_ref_internal(),

Hmm, I see.  As with the "known phandle, unknown ref" vs. "known ref,
unknown phandle" cases, I think this might be clearer with different
marker types.  During parse we make everything "known ref, unknown
phandle", then later change to either "known ref, resolved to local
phandle" or "known ref, needs external resolution" markers.



> --- 8< ---
> 	if (m->is_local) {
> 		phandle = propval_cell_n(prop,
> 					 m->offset / sizeof(cell_t));
> 		refnode = dti_get_node_by_phandle(dti, phandle);
> 		if (!refnode)
> 			die("Node not found for phandle 0x%"PRIx32"\n", phandle);
> 
> 		m->ref = refnode->fullpath;
> 		continue;
> 	} else {
> 		...
> --- 8< ---
> 
> 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
Re: [RFC PATCH 09/77] dtc: Introduce mark_local_phandles()
Posted by Herve Codina 2 weeks, 5 days ago
On Mon, 19 Jan 2026 16:46:37 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Fri, Jan 16, 2026 at 02:09:12PM +0100, Herve Codina wrote:
> > Hi David,
> > 
> > On Thu, 15 Jan 2026 11:48:44 +1100
> > David Gibson <david@gibson.dropbear.id.au> wrote:
> >   
> > > On Mon, Jan 12, 2026 at 03:18:59PM +0100, Herve Codina wrote:  
> > > > In order to have the new FDT_REF_LOCAL tag present in a dtb, the phandle
> > > > reference needs to be identify as a local reference.
> > > > 
> > > > This is the purpose of mark_local_phandles().
> > > > 
> > > > It identifies a phandle reference as a local reference when this
> > > > reference points to a local node.
> > > > 
> > > > With that node, the related FDT_REF_LOCAL tag is set in the dtb.    
> > > 
> > > I dislike caching redundant information (whether the ref is local) -
> > > it's an opportunity for them to get out of sync and cause bugs.  Is
> > > there a strong case that you can't just determine whether it's local
> > > only when you actually go to use it?  
> > 
> > Well, I can't find any strong case.
> > 
> > I would like to avoid passing the full dti (struct dt_info) to flatten_tree()
> > in order to determine if the ref is local or not to set a FDT_REF_LOCAL or
> > a FDT_REF_PHANDLE tag.
> > 
> > Also, this flag, set when a FDT_REF_LOCAL tag is parsed from a dtb, is
> > useful later when the ref has to be found based on the phandle value.
> > 
> > Indeed, because the is_local flag is set, the phandle value available in the
> > property *must* reference an existing node in the dtb.
> > 
> > In other word, in update_phandles_ref_internal(),  
> 
> Hmm, I see.  As with the "known phandle, unknown ref" vs. "known ref,
> unknown phandle" cases, I think this might be clearer with different
> marker types.  During parse we make everything "known ref, unknown
> phandle", then later change to either "known ref, resolved to local
> phandle" or "known ref, needs external resolution" markers.

When we parse a dtb, we have "Unknown ref, known phandle". FDT_REF_LOCAL
identify a local phandle value without any 'ref' involved.

The ref is set after parsing and it is set to the full path of the node
matching the phandle value.

Will see what I can do with new markers to identify those different cases.

> 
> 
> 
> > --- 8< ---
> > 	if (m->is_local) {
> > 		phandle = propval_cell_n(prop,
> > 					 m->offset / sizeof(cell_t));
> > 		refnode = dti_get_node_by_phandle(dti, phandle);
> > 		if (!refnode)
> > 			die("Node not found for phandle 0x%"PRIx32"\n", phandle);
> > 
> > 		m->ref = refnode->fullpath;
> > 		continue;
> > 	} else {
> > 		...
> > --- 8< ---
> > 
> > Best regards,
> > Hervé
> >   
> 

Best regards,
Hervé