Re: [RFC PATCH 07/77] livetree: Improve get_node_by_phandle()

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

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

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]