[RFC PATCH 08/77] dtc: Introduce update_phandles_ref()

Herve Codina posted 77 patches 3 weeks, 5 days ago
[RFC PATCH 08/77] dtc: Introduce update_phandles_ref()
Posted by Herve Codina 3 weeks, 5 days ago
With the introduction of FDT_REF_LOCAL dtb tag, a local phandle used
by a property is identify when a dtb is parsed.

In order to have consistent internal data, the reference related to this
phandle usage needs to be updated based on the phandle value.

This is done by update_phandles_ref().

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

diff --git a/dtc.c b/dtc.c
index b3445b7..88f03ff 100644
--- a/dtc.c
+++ b/dtc.c
@@ -333,6 +333,8 @@ int main(int argc, char *argv[])
 		generate_fixups = 1;
 	}
 
+	update_phandles_ref(dti);
+
 	process_checks(force, dti);
 
 	if (auto_label_aliases)
diff --git a/dtc.h b/dtc.h
index 965321c..351fe41 100644
--- a/dtc.h
+++ b/dtc.h
@@ -345,6 +345,8 @@ void generate_label_tree(struct dt_info *dti, const char *name, bool allocph);
 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);
+
 /* Checks */
 
 void parse_checks_option(bool warn, bool error, const char *arg);
diff --git a/livetree.c b/livetree.c
index 9b67934..9e30a63 100644
--- a/livetree.c
+++ b/livetree.c
@@ -1158,3 +1158,41 @@ void generate_local_fixups_tree(struct dt_info *dti, const char *name)
 			"Warning: Preexisting data in %s malformed, some content could not be added.\n",
 			name);
 }
+
+static void update_phandles_ref_internal(struct dt_info *dti, struct node *node)
+{
+	struct node *c;
+	struct property *prop;
+	struct marker *m;
+	struct node *refnode;
+	cell_t phandle;
+
+	for_each_property(node, prop) {
+		m = prop->val.markers;
+		for_each_marker_of_type(m, REF_PHANDLE) {
+			if (m->ref)
+				continue;
+
+			if (m->is_local) {
+				phandle = propval_cell_n(prop,
+							 m->offset / sizeof(cell_t));
+				refnode = get_node_by_phandle(dti->dt, phandle);
+				if (!refnode)
+					die("Node not found for phandle 0x%"PRIx32"\n", phandle);
+
+				m->ref = refnode->fullpath;
+				continue;
+			} else {
+				die("Found a non local phandle without a reference\n");
+			}
+		}
+	}
+
+	for_each_child(node, c)
+		update_phandles_ref_internal(dti, c);
+}
+
+void update_phandles_ref(struct dt_info *dti)
+{
+	update_phandles_ref_internal(dti, dti->dt);
+}
-- 
2.52.0
Re: [RFC PATCH 08/77] dtc: Introduce update_phandles_ref()
Posted by David Gibson 3 weeks, 3 days ago
On Mon, Jan 12, 2026 at 03:18:58PM +0100, Herve Codina wrote:
> With the introduction of FDT_REF_LOCAL dtb tag, a local phandle used
> by a property is identify when a dtb is parsed.
> 
> In order to have consistent internal data, the reference related to this
> phandle usage needs to be updated based on the phandle value.
> 
> This is done by update_phandles_ref().
> 
> Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> ---
>  dtc.c      |  2 ++
>  dtc.h      |  2 ++
>  livetree.c | 38 ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 42 insertions(+)
> 
> diff --git a/dtc.c b/dtc.c
> index b3445b7..88f03ff 100644
> --- a/dtc.c
> +++ b/dtc.c
> @@ -333,6 +333,8 @@ int main(int argc, char *argv[])
>  		generate_fixups = 1;
>  	}
>  
> +	update_phandles_ref(dti);
> +
>  	process_checks(force, dti);
>  
>  	if (auto_label_aliases)
> diff --git a/dtc.h b/dtc.h
> index 965321c..351fe41 100644
> --- a/dtc.h
> +++ b/dtc.h
> @@ -345,6 +345,8 @@ void generate_label_tree(struct dt_info *dti, const char *name, bool allocph);
>  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);
> +
>  /* Checks */
>  
>  void parse_checks_option(bool warn, bool error, const char *arg);
> diff --git a/livetree.c b/livetree.c
> index 9b67934..9e30a63 100644
> --- a/livetree.c
> +++ b/livetree.c
> @@ -1158,3 +1158,41 @@ void generate_local_fixups_tree(struct dt_info *dti, const char *name)
>  			"Warning: Preexisting data in %s malformed, some content could not be added.\n",
>  			name);
>  }
> +
> +static void update_phandles_ref_internal(struct dt_info *dti, struct node *node)
> +{
> +	struct node *c;
> +	struct property *prop;
> +	struct marker *m;
> +	struct node *refnode;
> +	cell_t phandle;
> +
> +	for_each_property(node, prop) {
> +		m = prop->val.markers;
> +		for_each_marker_of_type(m, REF_PHANDLE) {
> +			if (m->ref)
> +				continue;

IIUC this means that REF_PHANDLE markers can be missing their ref.
Allowing the markers to be in an incomplete state is a footgun.  If
possible it would be better to fully generate the markers when we
create them.  If not, we should use a different marker type when it's
introduced, and convert it later.

I think what's going on here is one type is saying "you have a
reference, fill in the phandle", the other is saying "you have a
phandle, fill in the reference".  Those seem like they should be
different types to me, even if they can be converted once all the
fixups are applied.

> +
> +			if (m->is_local) {
> +				phandle = propval_cell_n(prop,
> +							 m->offset / sizeof(cell_t));
> +				refnode = get_node_by_phandle(dti->dt, phandle);
> +				if (!refnode)
> +					die("Node not found for phandle 0x%"PRIx32"\n", phandle);
> +
> +				m->ref = refnode->fullpath;
> +				continue;
> +			} else {
> +				die("Found a non local phandle without a reference\n");
> +			}
> +		}
> +	}
> +
> +	for_each_child(node, c)
> +		update_phandles_ref_internal(dti, c);
> +}
> +
> +void update_phandles_ref(struct dt_info *dti)
> +{
> +	update_phandles_ref_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 08/77] dtc: Introduce update_phandles_ref()
Posted by Herve Codina 3 weeks, 1 day ago
On Thu, 15 Jan 2026 11:46:52 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Mon, Jan 12, 2026 at 03:18:58PM +0100, Herve Codina wrote:
> > With the introduction of FDT_REF_LOCAL dtb tag, a local phandle used
> > by a property is identify when a dtb is parsed.
> > 
> > In order to have consistent internal data, the reference related to this
> > phandle usage needs to be updated based on the phandle value.
> > 
> > This is done by update_phandles_ref().
> > 
> > Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> > ---
> >  dtc.c      |  2 ++
> >  dtc.h      |  2 ++
> >  livetree.c | 38 ++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 42 insertions(+)
> > 
> > diff --git a/dtc.c b/dtc.c
> > index b3445b7..88f03ff 100644
> > --- a/dtc.c
> > +++ b/dtc.c
> > @@ -333,6 +333,8 @@ int main(int argc, char *argv[])
> >  		generate_fixups = 1;
> >  	}
> >  
> > +	update_phandles_ref(dti);
> > +
> >  	process_checks(force, dti);
> >  
> >  	if (auto_label_aliases)
> > diff --git a/dtc.h b/dtc.h
> > index 965321c..351fe41 100644
> > --- a/dtc.h
> > +++ b/dtc.h
> > @@ -345,6 +345,8 @@ void generate_label_tree(struct dt_info *dti, const char *name, bool allocph);
> >  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);
> > +
> >  /* Checks */
> >  
> >  void parse_checks_option(bool warn, bool error, const char *arg);
> > diff --git a/livetree.c b/livetree.c
> > index 9b67934..9e30a63 100644
> > --- a/livetree.c
> > +++ b/livetree.c
> > @@ -1158,3 +1158,41 @@ void generate_local_fixups_tree(struct dt_info *dti, const char *name)
> >  			"Warning: Preexisting data in %s malformed, some content could not be added.\n",
> >  			name);
> >  }
> > +
> > +static void update_phandles_ref_internal(struct dt_info *dti, struct node *node)
> > +{
> > +	struct node *c;
> > +	struct property *prop;
> > +	struct marker *m;
> > +	struct node *refnode;
> > +	cell_t phandle;
> > +
> > +	for_each_property(node, prop) {
> > +		m = prop->val.markers;
> > +		for_each_marker_of_type(m, REF_PHANDLE) {
> > +			if (m->ref)
> > +				continue;  
> 
> IIUC this means that REF_PHANDLE markers can be missing their ref.

Yes, no ref are present in dtb for phandle pointing to local node, only
the phandle value is present. We need at some point to set the ref from
this phandle value.

> Allowing the markers to be in an incomplete state is a footgun.  If
> possible it would be better to fully generate the markers when we
> create them.  If not, we should use a different marker type when it's
> introduced, and convert it later.
> 
> I think what's going on here is one type is saying "you have a
> reference, fill in the phandle", the other is saying "you have a
> phandle, fill in the reference".  Those seem like they should be
> different types to me, even if they can be converted once all the
> fixups are applied.

Right, I can introduce
- REF_PHANDLE_FILL_PHANDLE: "you have a phandle, fill in the reference"

- REF_PHANDLE_FILL_REF: "you have a reference, fill the phandle"
  Well, fill the phandle or not... Indeed, this reference can be external and
  so this can be an unresolved symbol.

But anyway those new marker types will exist only between the parsing step and
the step where we convert them to REF_PHANDLE markers.

Will see what I can propose in the next series iteration.

Best regards,
Hervé


Re: [RFC PATCH 08/77] dtc: Introduce update_phandles_ref()
Posted by David Gibson 2 weeks, 6 days ago
On Fri, Jan 16, 2026 at 12:26:30PM +0100, Herve Codina wrote:
> On Thu, 15 Jan 2026 11:46:52 +1100
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > On Mon, Jan 12, 2026 at 03:18:58PM +0100, Herve Codina wrote:
> > > With the introduction of FDT_REF_LOCAL dtb tag, a local phandle used
> > > by a property is identify when a dtb is parsed.
> > > 
> > > In order to have consistent internal data, the reference related to this
> > > phandle usage needs to be updated based on the phandle value.
> > > 
> > > This is done by update_phandles_ref().
> > > 
> > > Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> > > ---
> > >  dtc.c      |  2 ++
> > >  dtc.h      |  2 ++
> > >  livetree.c | 38 ++++++++++++++++++++++++++++++++++++++
> > >  3 files changed, 42 insertions(+)
> > > 
> > > diff --git a/dtc.c b/dtc.c
> > > index b3445b7..88f03ff 100644
> > > --- a/dtc.c
> > > +++ b/dtc.c
> > > @@ -333,6 +333,8 @@ int main(int argc, char *argv[])
> > >  		generate_fixups = 1;
> > >  	}
> > >  
> > > +	update_phandles_ref(dti);
> > > +
> > >  	process_checks(force, dti);
> > >  
> > >  	if (auto_label_aliases)
> > > diff --git a/dtc.h b/dtc.h
> > > index 965321c..351fe41 100644
> > > --- a/dtc.h
> > > +++ b/dtc.h
> > > @@ -345,6 +345,8 @@ void generate_label_tree(struct dt_info *dti, const char *name, bool allocph);
> > >  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);
> > > +
> > >  /* Checks */
> > >  
> > >  void parse_checks_option(bool warn, bool error, const char *arg);
> > > diff --git a/livetree.c b/livetree.c
> > > index 9b67934..9e30a63 100644
> > > --- a/livetree.c
> > > +++ b/livetree.c
> > > @@ -1158,3 +1158,41 @@ void generate_local_fixups_tree(struct dt_info *dti, const char *name)
> > >  			"Warning: Preexisting data in %s malformed, some content could not be added.\n",
> > >  			name);
> > >  }
> > > +
> > > +static void update_phandles_ref_internal(struct dt_info *dti, struct node *node)
> > > +{
> > > +	struct node *c;
> > > +	struct property *prop;
> > > +	struct marker *m;
> > > +	struct node *refnode;
> > > +	cell_t phandle;
> > > +
> > > +	for_each_property(node, prop) {
> > > +		m = prop->val.markers;
> > > +		for_each_marker_of_type(m, REF_PHANDLE) {
> > > +			if (m->ref)
> > > +				continue;  
> > 
> > IIUC this means that REF_PHANDLE markers can be missing their ref.
> 
> Yes, no ref are present in dtb for phandle pointing to local node, only
> the phandle value is present. We need at some point to set the ref from
> this phandle value.
> 
> > Allowing the markers to be in an incomplete state is a footgun.  If
> > possible it would be better to fully generate the markers when we
> > create them.  If not, we should use a different marker type when it's
> > introduced, and convert it later.
> > 
> > I think what's going on here is one type is saying "you have a
> > reference, fill in the phandle", the other is saying "you have a
> > phandle, fill in the reference".  Those seem like they should be
> > different types to me, even if they can be converted once all the
> > fixups are applied.
> 
> Right, I can introduce
> - REF_PHANDLE_FILL_PHANDLE: "you have a phandle, fill in the reference"
> 
> - REF_PHANDLE_FILL_REF: "you have a reference, fill the phandle"
>   Well, fill the phandle or not... Indeed, this reference can be external and
>   so this can be an unresolved symbol.
> 
> But anyway those new marker types will exist only between the parsing step and
> the step where we convert them to REF_PHANDLE markers.

So REF_PHANDLE always has a complete reference, and a
complete-as-possible phandle value (-1 if it's an external reference)?
I think that makes sense - I don't love the proposed names for the
earlier markers, but better ones aren't immediately occurring to me.

> 
> Will see what I can propose in the next series iteration.
> 
> 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