Devicetree
 help / color / mirror / Atom feed
From: David Gibson <david@gibson.dropbear.id.au>
To: Doug Anderson <dianders@chromium.org>
Cc: Devicetree-compiler@vger.kernel.org,
	"Rob Herring" <robh@kernel.org>,
	"Krzysztof Kozlowski" <krzk+dt@kernel.org>,
	"Conor Dooley" <conor+dt@kernel.org>,
	"Konrad Dybcio" <konradybcio@kernel.org>,
	"Chen-Yu Tsai" <wenst@chromium.org>,
	linux-kernel@vger.kernel.org, Devicetree@vger.kernel.org,
	"Brian Norris" <briannorris@chromium.org>,
	"Tudor Ambarus" <tudor.ambarus@linaro.org>,
	"André Draszik" <andre.draszik@linaro.org>,
	"Julius Werner" <jwerner@chromium.org>,
	"Saravana Kannan" <saravanak@kernel.org>,
	"Peter Griffin" <peter.griffin@linaro.org>
Subject: Re: [PATCH 0/4] fdtfactor: Introduce fdtfactor to auto-factor overlays
Date: Thu, 29 Oct 2026 11:27:51 +1100	[thread overview]
Message-ID: <auKS-dqpPfUQ8Vtv@gractus.seuss> (raw)
In-Reply-To: <CAD=FV=XXp6rMfE0fwUOFHQ--eetjRu8N=L1pjZP71Lcu5n-yow@mail.gmail.com>

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

On Wed, Oct 28, 2026 at 02:57:08PM -0700, Doug Anderson wrote:
> Hi David,
> 
> On Thu, Oct 22, 2026 at 10:04 PM David Gibson
> <david@gibson.dropbear.id.au> wrote:
> >
> > On Thu, Oct 15, 2026 at 11:21:33AM -0700, Douglas Anderson wrote:
> > >
> > > Devicetree overlays allow you to have a "base" compiled devicetree and then
> > > to apply an "overlay" to it to add or modify nodes. Devicetree overlays have
> > > at least two purposes:
> > >
> > > 1. Supporting add-on boards (like "hats" on a dev board). In this case the
> > >    main board is described as a base devicetree and the add-on board is an
> > >    overlay. In this case there is a clear separation between the base and the
> > >    add-on board and the base/overlay separation models reality nicely.
> > >
> > > 2. Supporting a "family" of similar boards in a space-efficient way. In
> > >    upstream Linux, there are 47 distinct sc7180-trogdor boards supported.
> > >    These board are all highly similar, but we compile (and ship) a separate
> > >    devicetree for each. Devicetree overlays can make this much more efficient
> > >    by factoring the common bits to the base devicetree and having each
> > >    of the 47 boards represented by a small devicetree overlay.
> > >
> > > Up to this point, the usage of overlays in the upstream devicetree repo has
> > > mostly been for case #1.
> >
> > I don't think it invalidates the concept you're going for, but this
> > description is a bit misleading without some historical context.  In
> > particular this is a bit confused by the fact that there are both
> > source-level (dts) overlays and binary-level (dtb) overlays.
> >
> > Source overlays came first, in conjuction with /include/ support in
> > dtc, and were intended for use case (2) and similar.  A group of
> > boards with the same SoC was the most common case, but families of
> > similar boards and families of similar SoCs also used this mechanism.
> > At the time this was still envisaging that a complete tree was built
> > at boot time, and didn't change.
> >
> > This started to drift into use case (1) since without hotplug (we'll
> > get to that), a board with different hats (or whatever) can be
> > considered variants of the same base board.
> >
> > That still required the complete tree be built at compile time (or at
> > least bootloader / kernel installation time).  That wasn't great for
> > two reasons, roughly matching your two use cases:
> >
> >  1. Compile time is obviously no good for hotplugged hats.  Even for
> >     coldplug hats, it's desirable to be able to complete the tree at
> >     boot time rather than compile / deploy time.
> >
> >  2. Bootloaders often want to support a variety of related boards, not
> >     just a single configuration.  That was already possible by
> >     incorporating a full dtb for every variant, but it gets pretty
> >     bulky and bootloaders are often pretty constrained.
> >
> > dtb overlays were invented to address these, though I don't know that
> > difference between the two motivations was particularly well
> > understood at the time.
> >
> > dtb overlays were pretty much a direct transcription of dts overlay
> > semantics to dtb and runtime.  The use of "in band" encoding (special
> > properties) is kind of inelegant, but it worked well enough to become
> > popular.
> >
> > This works reasonably well for use case (2).  That makes sense: the
> > semantics are based on dts overlays which were designed for use case
> > (2).
> >
> > But, dtb overlays have always been a nasty hack for case (1), even
> > though that's become the predominant one.  Overlays can change
> > anywhere in the tree, whether or not that's physically sensible via
> > the hat connector (or whatever).  The semantics of applying an overlay
> > lose information (by design), which means that removing an overlay for
> > hot unplug is a complete mess.  The way we encode the information
> > necessary to apply an overlay is kind of messy and inconsistent.
> >
> > Still, they worked well enough for (1) that they've become widespread.
> > Hervé Condina is now working on a new scheme (dubbed "addons") which
> > is actually designed for case (1), and should suit it much better.
> > It's a fair way off being ready for use, but it's a good development.
> 
> Interesting. I'd have never thought to call the "dts" vs. "dtsi" split
> "overlays".

Technically they're not quite the same thing, you can have an
overlay within a single dts file:

	/ {
		prop = "somthing";
	}
	/ {
		prop = "else";
	}

But yes, they're not very useful without includes and the dts/dtsi
split tends to match the overlay pieces.

This was where the term "overlay" originated (I believe I coined it
myself), referring to the fact that later fragments overwrite / cover
/ "overlay" the earlier fragments.

> I'll try to reword this in the cover letter.
> 
> 
> > > I've used this tool to reduce sc7180-trogdor device tree files (which
> > > are stored lz4 compressed) from 1.71 MB to 213 KB, a 87.5% savings.
> >
> > Nice.  I suspect this gap would be substantially smaller with a large
> > window compressor.  The trick here is that the factoring is finding
> > long distance redundancies which lz4 won't be able to see.
> 
> FWIW, I don't think the large window compressor would have helped
> here. Each .dtb file is compressed on its own, right? This means that
> the bootloader doesn't need to read/decompress all of the devicetree
> files--just the ones actually cares about.

Oh, good point, the large window compressor would only help
substantially if all the overlays were compressed together.

> > > I'll freely admit that the patches in this series were pretty heavily vibe
> > > coded. I'll not pretend that I have a deep understanding of the intracicies
> > > of how overlays are implemented. That being said:
> > > * I've spent quite a bit of time reviewing the code from a high-level
> > >   perspective: confirming that functions are well documented, well structured,
> > >   and make sense.
> > > * I believe that what the tool is doing and how the tool is used make sense.
> > > * I have had quite a number of internal "the AI reviews the AI" rounds to
> > >   the point of diminishing returns.
> > > At this point, I'm not embarassed to put my name behind this. While I won't
> > > promise that there is zero cases of the AI doing something really stupid,
> > > I'd be willing to bet that the end result is better (and cleaner / better
> > > structured) than what I would have coded up if I had spend a few weeks doing
> > > this on my own.
> >
> > Thanks for that clear and helpful context.  My views on AI are
> > complicated, leaning negative, but I do think it can be a useful tool
> > and have no plans to reject patches purely because they're AI
> > assisted.  A statement like that goes a long way to convincing me that
> > I'm not about to waste my effort reviewing slop.  c.f. my rather
> > unfriendly response[2] to a recent very sloppish bug report which
> > vastly overestimated the impact of the bugs in question.
> 
> Sorry if it still wasn't super amazing. FWIW, it was definitely much
> worse when I first started with it. :-P The quick prototype was done
> in 30 minutes, but then I spent a big chunk of time trying to whip it
> into something sensible. ...but obviously (as you found in reviewing
> it) there were a bunch of things that looked reasonable to me but
> weren't ideal. FWIW, there were at least some things that I simply
> made the wrong choice on. I wasn't sure how much you would want
> fdtfactor to stay out of the way of the rest of "dtc" (AKA keep my
> changes self-contained) vs. actually improving / fixing the dtc source
> base. I chose to stay out of the way in a few cases. That's changed in
> v2 which makes fdtfactor smaller.

Ok, good.

> FWIW, I've attempted to get my AI to improve the later patches in the
> series based on your feedback in the cover letter and patch #2.
> Hopefully this makes things look better to you in v2...
> 
> 
> > [2] https://lore.kernel.org/devicetree-compiler/as8RYmIp0pDCplpu@gractus.seuss/
> >
> > Understand that all dtc maintainership is unpaid volunteer time for
> > me, so I'm less responsive about reviews than would be ideal.  I am
> > gradually chiping away at the backlog, though.
> 
> I really appreciate the time you took on this. I've made it a priority
> to go through all of your feedback and ensure everything is addressed.
> 
> I'm sure v2 won't be perfect, but hopefully it's a bunch better.

Excellent.

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

  reply	other threads:[~2026-10-29  1:11 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-15 18:21 [PATCH 0/4] fdtfactor: Introduce fdtfactor to auto-factor overlays Douglas Anderson
2026-10-15 18:21 ` [PATCH 1/4] tests: Fix Python test environment setup in meson.build Douglas Anderson
2026-10-23  4:51   ` David Gibson
2026-10-15 18:21 ` [PATCH 2/4] fdtfactor: Add tool to split DTBs into a base DTB and overlays Douglas Anderson
2026-10-26  4:50   ` David Gibson
2026-10-28 21:57     ` Doug Anderson
2026-10-29  1:00       ` David Gibson
2026-10-29  1:28         ` Doug Anderson
2026-10-26 15:30   ` Simon Glass
2026-10-28 21:57     ` Doug Anderson
2026-10-15 18:21 ` [PATCH 3/4] fdtfactor: Support compressed DTB inputs and outputs Douglas Anderson
2026-10-26  4:51   ` David Gibson
2026-10-26 15:31   ` Simon Glass
2026-10-28 21:57     ` Doug Anderson
2026-10-15 18:21 ` [PATCH 4/4] fdtfactor: Add compiled FIT image factoring support Douglas Anderson
2026-10-26 15:31   ` Simon Glass
2026-10-28 21:58     ` Doug Anderson
2026-10-23  4:49 ` [PATCH 0/4] fdtfactor: Introduce fdtfactor to auto-factor overlays David Gibson
2026-10-26  5:48   ` David Gibson
2026-10-28 21:57   ` Doug Anderson
2026-10-29  0:27     ` David Gibson [this message]
2026-10-26 15:30 ` Simon Glass
2026-10-28 21:57   ` Doug Anderson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=auKS-dqpPfUQ8Vtv@gractus.seuss \
    --to=david@gibson.dropbear.id.au \
    --cc=Devicetree-compiler@vger.kernel.org \
    --cc=Devicetree@vger.kernel.org \
    --cc=andre.draszik@linaro.org \
    --cc=briannorris@chromium.org \
    --cc=conor+dt@kernel.org \
    --cc=dianders@chromium.org \
    --cc=jwerner@chromium.org \
    --cc=konradybcio@kernel.org \
    --cc=krzk+dt@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=peter.griffin@linaro.org \
    --cc=robh@kernel.org \
    --cc=saravanak@kernel.org \
    --cc=tudor.ambarus@linaro.org \
    --cc=wenst@chromium.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox