Re: [PATCH 2/4] fdtfactor: Add tool to split DTBs into a base DTB and overlays

[Doug Anderson]

Hi David,

On Wed, Oct 28, 2026 at 6:11 PM David Gibson
<david@xxxxxxxxxxxxxxxxxxxxx> wrote:
>
> > Yeah, I debated this a bit originally, and I agree it's a bit confusing.
> > How about this: I'll keep the option, but force it to be just a filename,
> > not a full path. At least to me that makes it less confusing. Hopefully
> > that's OK?
>
> Hm, the way the path is encoded isn't really what I dislike about it.
> Something about the "base-dtb" name suggests an existing base tree,
> rather than an output desination to me (particularly since in future
> you might want an existing base tree to do recursive factoring).  What
> I'd suggest is dropping the short option and renaming the long option
> to include "output".

OK. When you have a chance, take another quick look in v2 and I'm happy
to change this option however you want in v3.


> > > > +            all_have_prop_fixups = all(
> > > > +                bool(d.sym_props[path][prop_name][1]) for d in dtbs
> > > > +            )
> > >
> > > I'm not sure why you care that all the copies of the property have
> > > fixups.  AFAICT all you care about is that the set of fixups is
> > > identical for all of them, empty or not.
> >
> > Sigh, this is where me not understanding 100% of the details of overlays
> > bites me. Some background. I remember there being some talk a little while
> > ago of (perhaps) shipping device trees compiled with '-L'. That would
> > allow the OS to know for sure which properties represented phandles and
> > that could help the OS figure out device dependencies.
>
> [Not really relevant to this series, but that doesn't sound like a
>  great idea to me]
>
> > I wanted to make
> > sure that running fdtfactor wouldn't break this and the OS would still
> > have all the information it needed even after breaking things up into
> > overlays and re-applying.
> >
> > In order to test this, I wanted to make sure we tested re-combining the
> > base devicetrees and overlays and we could confirm that everything was
> > there. In order to make all that work, my agent added a bunch of extra
> > '/fragment@0/__overlay__/__local_fixups__'. ...but it turns out that was
> > a workaround for the fact that fdt_overlay_apply() didn't preserve
> > __local_fixups__ in the base. :(
>
> Ah, right.  Again, I'm not convinced this is a great idea, but if we
> wanted it the right way to do it is to have fdt_overlay_apply()
> (optionally) regenerate __local_fixups__.  It's not really merely
> "preserving", since applying the overlay might require that some
> external fixups be converted to local fixups.

FWIW, in the v2 series this is what I've done. There are 3 patches
in the series that just fix this.

NOTE: personally, I don't _actually_ have a need to run fdtfactor
without the `-S` option. It just seemed more correct to me to
be able to keep __local_fixups__ correct after merging. It also
tends to make verification of things easier because they you can
nicely "reverse" phandles for comparison...