Re: [PATCH v2 0/6] dtc: Fix deletes within a section and before a redefinition

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

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

On Sun, Nov 01, 2026 at 04:35:50PM +0000, Simon Glass wrote:
> Hi David,
> 
> On Wed, 28 Oct 2026 at 20:52, David Gibson <david@gibson.dropbear.id.au> wrote:
> >
> > On Wed, Oct 28, 2026 at 03:36:07AM -0600, Simon Glass wrote:
> > > A /delete-property/ or /delete-node/ in the first section of a dts, or
> > > in a node which a later section adds, is ignored and leaves its
> > > placeholder in the tree, where it can show up as a bogus duplicate. A
> > > node which is deleted and then defined again also returns to its old
> > > place, with its old subnodes in their old order.
> > >
> > > The first patch applies each delete within its own section and keeps
> > > the placeholders out of the tree. The second removes deleted entries
> > > from the tree, so that a node defined again comes after the existing
> > > ones. The third clarifies some comments. The fourth lets dtc-fatal.sh
> > > check the error message, which the fifth uses to test an error for a
> > > delete in an overlay fragment which has nothing to delete there, since
> > > an overlay cannot delete from the base tree. The sixth documents how
> > > deletes behave.
> > >
> > > Note that the second and fifth patches change existing behaviour. The
> > > second changes the output of existing dts files which delete and then
> > > redefine a node or property. The fifth breaks the build of two overlays
> > > in Linux, whose deletes have never taken effect. Each patch explains
> > > why this seems the better behaviour.
> >
> > Sorry, I didn't catch this in v1: I think this does improve behaviour,
> > but I don't think it's the right approach.
> >
> > At the moment we handle the fragment merges right within the parser,
> > meaning we have a single unified tree as soon as we return from
> > yyparse().  That's already a bit confusing, and means behaviour isn't
> > quite the same between the first fragment listed and subsequent ones.
> >
> > This series improves that consistency, but it does so with kind of
> > ad-hoc alterations which are pretty hard to keep track of.  It also
> > moves towards the model where later things can overwrite earlier
> > things, even within the same fragment, not just between fragments.
> > Although it is a way of making the semantics more consistent, I think
> > going that way makes it easier to have uncaught typos in the dt
> > source.
> >
> > Here's the approach I think we should take instead.  I've had this in
> > mind for a while, but never got around to it.  Seeing this series kind
> > of crystalized the rationale for me.  It is more work, but I think it
> > will be worth it:
> >
> > 1. Have the parser treat each fragment / overlay separately, returning
> >    a forest of those pieces, rather than a single tree
> >
> >   1a. We'll also want dt_from_blob() to parse dtbo /fragment@ pieces
> >       and convert those to a similar forest.  That can be implemented
> >       later on, though.
> >
> > 2a. For base trees, call a function from ain() which merges the forest
> >     into a single tree, with similar logic to what we have now.
> >
> > 2b. For /plugin/ trees, call a function from main() which wraps each
> >     parsed fragment in /frament@NN/__overlay__ and combines those
> >     into a single tree
> >
> > 3. Start splitting the checks into two sets.  One set runs *before*
> >    the merge on each individual fragment.  The other runs on the
> >    merged tree.
> >
> > Under this scheme /delete/ placeholders will only exist in the
> > forest-of-fragments stage.  They'll either be resolved by actually
> > deleting the node/property in (2a), or trip an error in (2b).
> >
> > This can be more gradual, but by moving things like the
> > duplicate_node_names checks to the pre-merge phase we can catch more
> > errors within fragments, without breaking either compile time or
> > runtime overlay semantics.
> 
> Thanks for the info, agreed. I've dug into the code a bit and am
> sending a series for the first step: the parser returns a list of
> fragments, which main() then merges.

Excellent!

> Re your plan, should deleting something defined earlier in the same
> fragment be an error?

Yes, that was my intention, though we can revisit it if that breakgs
things for people in practice.  This was the way it was before
source-level overlays were implemented, and still is for the "base"
tree fragment in a file.

> In an overlay, a top-level /delete-node/ &label can remove a node from
> another fragment of the same overlay, as Hervé pointed out. I assume
> that should keep working, with only deletes of something in the base
> tree being an error. Is that right?

Uh.. yes.  In a sense this is a question of whether we try to
partially resolve the overlays in a /plugin/ dts or not.  i.e. Do we
attempt to merge those fragments we can, or do we translate every
fragment directly into the dtbo to be resolved at runtime.  I'm not
sure there's a clear correct answer here, so I'd err on the side of
maintaining existing behaviour.

-- 
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 --]