get_node_by_phandle() allows to get a node based on its phandle value.
It checks the phandle value against value available in internal node
structure.

This internal phandle value is updated during process_check() and so,
get_node_by_phandle() cannot give correct results before the
process_check() call.

Improve get_node_by_phandle() to look at node phandle properties when
the internal phandle value is not valid.

This allows to return a correct matching node even if process_check()
was not called yet.

With the recently introduced FDT_REF_LOCAL dtb tag, this will be needed
to update internal phandle references before the call to process_check().
Indeed, this tag allows to identify phandles and internal references
need to be updated based on the phandle value before the
process_check() call.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
 livetree.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/livetree.c b/livetree.c
index f328824..9b67934 100644
--- a/livetree.c
+++ b/livetree.c
@@ -609,16 +609,35 @@ struct node *get_node_by_label(struct node *tree, const char *label)
 	return NULL;
 }
 
+static cell_t get_node_phandle_existing(struct node *node)
+{
+	struct property *prop;
+
+	if (phandle_is_valid(node->phandle))
+		return node->phandle;
+
+	prop = get_property(node, "phandle");
+	if (!prop) {
+		prop = get_property(node, "linux,phandle");
+		if (!prop)
+			return 0;
+	}
+
+	return propval_cell(prop);
+}
+
 struct node *get_node_by_phandle(struct node *tree, cell_t phandle)
 {
 	struct node *child, *node;
+	cell_t tree_phandle;
 
 	if (!phandle_is_valid(phandle)) {
 		assert(generate_fixups);
 		return NULL;
 	}
 
-	if (tree->phandle == phandle) {
+	tree_phandle = get_node_phandle_existing(tree);
+	if (phandle_is_valid(tree_phandle) && tree_phandle == phandle) {
 		if (tree->deleted)
 			return NULL;
 		return tree;
-- 
2.52.0

On Mon, Jan 12, 2026 at 03:18:57PM +0100, Herve Codina wrote:
> get_node_by_phandle() allows to get a node based on its phandle value.
> It checks the phandle value against value available in internal node
> structure.
> 
> This internal phandle value is updated during process_check() and so,
> get_node_by_phandle() cannot give correct results before the
> process_check() call.
> 
> Improve get_node_by_phandle() to look at node phandle properties when
> the internal phandle value is not valid.
> 
> This allows to return a correct matching node even if process_check()
> was not called yet.
> 
> With the recently introduced FDT_REF_LOCAL dtb tag, this will be needed
> to update internal phandle references before the call to process_check().
> Indeed, this tag allows to identify phandles and internal references
> need to be updated based on the phandle value before the
> process_check() call.

Having two entirely different paths for get_node_by_phandle() is
really ugly.

I suspect a better approach would be to special case updates to the
internal phandle field as we parse the phandle properties, rather than
doing it as a batch during the checks.

> Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> ---
>  livetree.c | 21 ++++++++++++++++++++-
>  1 file changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/livetree.c b/livetree.c
> index f328824..9b67934 100644
> --- a/livetree.c
> +++ b/livetree.c
> @@ -609,16 +609,35 @@ struct node *get_node_by_label(struct node *tree, const char *label)
>  	return NULL;
>  }
>  
> +static cell_t get_node_phandle_existing(struct node *node)
> +{
> +	struct property *prop;
> +
> +	if (phandle_is_valid(node->phandle))
g> +		return node->phandle;
> +
> +	prop = get_property(node, "phandle");
> +	if (!prop) {
> +		prop = get_property(node, "linux,phandle");
> +		if (!prop)
> +			return 0;
> +	}
> +
> +	return propval_cell(prop);
> +}
> +
>  struct node *get_node_by_phandle(struct node *tree, cell_t phandle)
>  {
>  	struct node *child, *node;
> +	cell_t tree_phandle;
>  
>  	if (!phandle_is_valid(phandle)) {
>  		assert(generate_fixups);
>  		return NULL;
>  	}
>  
> -	if (tree->phandle == phandle) {
> +	tree_phandle = get_node_phandle_existing(tree);

It's especially nasty that we call the expensive version first, then
fall back to the cheap version...

> +	if (phandle_is_valid(tree_phandle) && tree_phandle == phandle) {
>  		if (tree->deleted)
>  			return NULL;
>  		return tree;
> -- 
> 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

Hi David,

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

> On Mon, Jan 12, 2026 at 03:18:57PM +0100, Herve Codina wrote:
> > get_node_by_phandle() allows to get a node based on its phandle value.
> > It checks the phandle value against value available in internal node
> > structure.
> > 
> > This internal phandle value is updated during process_check() and so,
> > get_node_by_phandle() cannot give correct results before the
> > process_check() call.
> > 
> > Improve get_node_by_phandle() to look at node phandle properties when
> > the internal phandle value is not valid.
> > 
> > This allows to return a correct matching node even if process_check()
> > was not called yet.
> > 
> > With the recently introduced FDT_REF_LOCAL dtb tag, this will be needed
> > to update internal phandle references before the call to process_check().
> > Indeed, this tag allows to identify phandles and internal references
> > need to be updated based on the phandle value before the
> > process_check() call.  
> 
> Having two entirely different paths for get_node_by_phandle() is
> really ugly.
> 
> I suspect a better approach would be to special case updates to the
> internal phandle field as we parse the phandle properties, rather than
> doing it as a batch during the checks.

Doing that when we parse the property will be quite complex. Indeed,
when we parse a dts, the node internal object is not yet created when
the property is parsed.

What I think could be done is to set the phandle field just after the
parsing of input (dts or dtb). In current implementation this is done by
process_check() when fixup_phandle_references() is called.

This fixup_phandle_references() call should be removed from process_check()
and called right after the input parsing.

I you are ok with that, I can propose something in the next iteration.

Best regards,
Hervé

On Fri, Jan 16, 2026 at 11:52:54AM +0100, Herve Codina wrote:
> Hi David,
> 
> On Thu, 15 Jan 2026 11:41:32 +1100
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > On Mon, Jan 12, 2026 at 03:18:57PM +0100, Herve Codina wrote:
> > > get_node_by_phandle() allows to get a node based on its phandle value.
> > > It checks the phandle value against value available in internal node
> > > structure.
> > > 
> > > This internal phandle value is updated during process_check() and so,
> > > get_node_by_phandle() cannot give correct results before the
> > > process_check() call.
> > > 
> > > Improve get_node_by_phandle() to look at node phandle properties when
> > > the internal phandle value is not valid.
> > > 
> > > This allows to return a correct matching node even if process_check()
> > > was not called yet.
> > > 
> > > With the recently introduced FDT_REF_LOCAL dtb tag, this will be needed
> > > to update internal phandle references before the call to process_check().
> > > Indeed, this tag allows to identify phandles and internal references
> > > need to be updated based on the phandle value before the
> > > process_check() call.  
> > 
> > Having two entirely different paths for get_node_by_phandle() is
> > really ugly.
> > 
> > I suspect a better approach would be to special case updates to the
> > internal phandle field as we parse the phandle properties, rather than
> > doing it as a batch during the checks.
> 
> Doing that when we parse the property will be quite complex. Indeed,
> when we parse a dts, the node internal object is not yet created when
> the property is parsed.

Ah, good point.

> What I think could be done is to set the phandle field just after the
> parsing of input (dts or dtb). In current implementation this is done by
> process_check() when fixup_phandle_references() is called.

Oof.  So, on the one hand, doing this indexing in the "checks" is kind
of weird - I implemented it that way because it just seemed a
convenient place that already scanned the full tree and had mechanisms
for dependencies between different steps.

However... these dependencies are a bit subtle.  Part of the trick
here is that the indexing has dependencies on the really basic checks
that we don't have invalid or duplicate node/property names.  If we
moved the phandle indexing before that, we'd have to explicitly deal
with the possibility of multiple "phandle" properties and other
weirdness.

Maybe we want to split the "checks" into two different phases.  One
for "structural" checks, which check the basic required properties:
characters in names, no duplicate names, no duplicate phandles.  That
phase could also handle fixups and indexing that aren't really checks.

A later phase could do the semantic checks (does the interrupts tree
make sense etc.).   We could do processing between those two phases if
it makes sense to do so.

> This fixup_phandle_references() call should be removed from process_check()
> and called right after the input parsing.
> 
> I you are ok with that, I can propose something in the next 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