Re: [PATCH 02/13] dtc: Add a list for the top-level fragments of a dts file

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

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

On Wed, Nov 04, 2026 at 09:29:11AM +0100, Herve Codina wrote:
> On Wed, 4 Nov 2026 17:32:35 +1100
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > On Tue, Nov 03, 2026 at 06:56:20PM +0100, Herve Codina wrote:
> > > On Mon, 2 Nov 2026 20:49:48 +1100
> > > David Gibson <david@gibson.dropbear.id.au> wrote:
> > >   
> > > > On Sun, Nov 01, 2026 at 09:36:56AM -0700, Simon Glass wrote:  
> > > > > A dts file is made up of top-level fragments: the root node, later
> > > > > definitions of a node by label or path, and /delete-node/ or
> > > > > /omit-if-no-ref/ with a reference. We would like to deal with each
> > > > > fragment on its own before it is merged, for example to apply deletes
> > > > > consistently or to run some checks on each one, but that requires
> > > > > keeping the fragments apart. At present the parser merges each one into
> > > > > the tree as soon as it is parsed.    
> > > > 
> > > > Btw, I'm extremely open to a better name than "fragment" if one occurs
> > > > to you.  I believe I originally thought of these as "overlays" but
> > > > that's confusing since runtime overlays were invented.  
> > > 
> > > "A dts file is made up of top-level fragments: the root node, later
> > > definitions of a node by label or path"
> > > 
> > > A node reference by a label or a path is not a definition of node, it is
> > > a reference to a node.
> > > 
> > > Once those references are merged (if possible), we only have a root node
> > > and orphan nodes (nodes without a parent node). Indeed, those orphan nodes
> > > reference a node out of the current devicetree (dts or dtb).  
> > 
> > Yes..
> > 
> > > Orphan nodes are just references that cannot be merged in the current
> > > devicetree.
> > > 
> > > Instead of "fragment", "unresolved" ?
> > > 
> > > &label {
> > >    prop;
> > > };
> > > 
> > > Is an unresolved node while &label is not resolved to an existing node.
> > > 
> > > We try to merge those unresolved nodes (label resolution + merge) and any
> > > unresolved nodes that are not merged (label not resolved) are orphan nodes.
> > > 
> > > The exact same occurs when we use paths instead of label. The only difference
> > > is that a path can always be resolved. Having an unresolved path is an error.
> > > 
> > > With that in mind, having a look at orphan nodes in the addon series [0] (patch 44
> > > and following ones) could give some ideas.
> > > 
> > > Does it make sense ?  
> > 
> > That's all true, but I don't see what you're suggesting.  Here I'm
> > looking for a name for the syntactic top-level construct, whether or
> > not it's resolvable within this file or not.
> > 
> 
> struct node {
> 	...
> 	const char *ref;  /* Only orphan nodes doesn't have a name but a ref */
> }

I don't think this is a good change.  Most nodes don't have this a
fragment (or whatever we call it) is a structurally different element
that should have a different representation.

> struct dt_info {
> 	struct node *root;
> 	struct node *orphanlist;
> 	...
> }
> 
> When you parse, you create the orphanlist with orphan nodes (same order in the
> list as the order in the dts) without trying to merge.

"orphan" is a bad name here - the "orphan" implies we don't have a
parent we can attach them to, which we don't know until we attempt to
resolve during merge.

> Once done, build_dt_info(..., root, orphanlist);

That's a good function signature (apart from the "orphan" name).

> Then, in build_dt_info() or later:
> 
> reduce_orphans(struct dt_info *dt_info)
>   - Handle deletes
>     You have the root node and the orphan list with item in the same order as
>     they are found in the dts. Deleting node available before the "/delete-node/"
>     keyword should be doable.
>   - Merge as much nodes as possible from the orphan list. This merge orphan
>     nodes in either the root node or a possible previous orphan.

Sure, but this isn't meaningfully different from the current draft (I
think, I haven't finished reading it).

> After this call:
>  - All possible deletes have been handled
>  - Orphans are reduced to real orphan without duplicates (merge done when
>    possible).
> 
> For dts, this orphan list must be empty.
> For overlay or addons, this orphan list can have items.
> 
> Perform the checks with this structure.
> 
> Generate output:
>   For overlays: Convert to /fragment@N/... then generate the output
>   For addons: no conversion needed, generate the output
> 
> Do we want to handle deletes at runtime ?

No.  Runtime overlays are already a nasty hack, I don't want to add
more complexity to them.

> The only relevant is having an overlay or addon the capabilities to
> delete an orphan node. In that case, at runtime, instead of merging
> a delete is perform. This use case is not handle yet but to handle
> them:
> 
>   - reduce_orphans(struct dt_info *dt_info)
>     Delete what you can delete
>     Keep orphans with delete flag in the orphan list (after reduction)
>   - Generate output
>      for orphan with delete flag, encode the delete information (new node
>      metadata tag). Those orphan with delete flag should be empty. No
>      property to merge, no node to merge. Indeed they are in the output
>      just to tell that the related node in the base tree must be deleted.
> 
> 
> Now checks(). The orphan list has been reduce to the strict minimum. Is that
> ok for checks() you have in mind?
> 
> 
> 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 --]