From: Brian Norris <briannorris@chromium.org>
To: David Gibson <david@gibson.dropbear.id.au>
Cc: devicetree-compiler@vger.kernel.org
Subject: Re: [PATCH] checks: Avoid warnings for reg override in __overlay__
Date: Mon, 22 Jun 2026 14:14:48 -0700 [thread overview]
Message-ID: <ajmlyJ-JWc9MFyMO@google.com> (raw)
In-Reply-To: <ajOrX76nVn7mN5am@zatzit>
Hi David,
On Thu, Jun 18, 2026 at 06:25:03PM +1000, David Gibson wrote:
> On Wed, Jun 17, 2026 at 01:44:01PM -0700, Brian Norris wrote:
> > A simple overlay case can hit a number of false warnings just because it
> > provides a 'reg' property [1]:
> >
> > * avoid_default_addr_size: the generated fragment@0 node is an
> > artificial node, where #address-cells/#size-cells can't exist.
> >
> > * reg_format: we're assuming the wrong/default #address-cells here, so
> > the analysis is wrong. (We don't know how many cells should be in
> > this 'reg', in isolation.)
> >
> > * unit_address_vs_reg: we don't know the real node name here. (It's not
> > actually going to be "__overlay__".)
> >
> > All these cannot be checked by the overlay compiler in isolation.
> > Suppress them when we identify we're running in plugin mode, and we're
> > likely looking at a generated __overlay__ hierarchy that can't be
> > analyzed in this way.
>
> Yeah, the check system was designed before runtime overlays and
> doesn't work well with them. Specifically there's not distinction
> between checks that can be applied locally (just looking at one node),
> checks that can be applied to a subtree (without looking outside the
> subtree) and checks which require a full tree including a root node.
>
> Just suppressing checks ad-hoc like this is not a great solution.
>
> I had a loose plan for tackling this, amongst some other problems, by
> making the assembly of a complete tree an explicitly separate step in
> dtc from parsing each fragment (here referring to compile time dts
> fragments).
>
> The idea was that each of the (compile time) overlays would be
> separately parsed into a temporary tree, rather than assembling them
> together into a single tree as we go. Then we'd have a pass applying
> those checks that make sense for subtrees separately to each
> subtree/overlay.
>
> Then there would be a separate assembly stage. For full trees that
> would essentially apply each overlay. For overlay output it would add
> the fragment@XX/__overlay__ encoding. Afterwards there would be
> another checks pass for the assembled tree - that would have a
> different set of checks from the individual fragment tests, and
> different ones for complete trees versus overlay output.
>
> Unfortunately, I will not realistically ever get to this.
I'll admit, I don't think I understood some of that "loose plan", but it
sounds like to get any benefit, it would require running the compiler
with a fuller context of both base and overlay. So does that imply it
only works when invoked with a pairing of dtb + potentially multiple
dtbo? That sounds like a totally new execution mode, and much outside
the scope of what most DTC users are likely to take on, when they simply
ran into false compiler warnings.
> >
> > [1]
> > $ dtc -I dts -O dts tests/overlay-reg-override.dtso
> > tests/overlay-reg-override.dtso:5.2-14: Warning (reg_format): /fragment@0/__overlay__:reg: property has invalid length (4 bytes) (#address-cells == 2, #size-cells == 1)
> > tests/overlay-reg-override.dtso:4.6-6.3: Warning (unit_address_vs_reg): /fragment@0/__overlay__: node has a reg or ranges property, but no unit name
> > <stdout>: Warning (pci_device_reg): Failed prerequisite 'reg_format'
> > <stdout>: Warning (pci_device_bus_num): Failed prerequisite 'reg_format'
> > <stdout>: Warning (simple_bus_reg): Failed prerequisite 'reg_format'
> > <stdout>: Warning (i2c_bus_reg): Failed prerequisite 'reg_format'
> > <stdout>: Warning (spi_bus_reg): Failed prerequisite 'reg_format'
> > tests/overlay-reg-override.dtso:4.6-6.3: Warning (avoid_default_addr_size): /fragment@0/__overlay__: Relying on default #address-cells value
> > tests/overlay-reg-override.dtso:4.6-6.3: Warning (avoid_default_addr_size): /fragment@0/__overlay__: Relying on default #size-cells value
> > <stdout>: Warning (avoid_unnecessary_addr_size): Failed prerequisite 'avoid_default_addr_size'
> > <stdout>: Warning (unique_unit_address): Failed prerequisite 'avoid_default_addr_size'
> > /dts-v1/;
> >
> > / {
> >
> > fragment@0 {
> > target = <&foo>;
> >
> > __overlay__ {
> > reg = <0x00>;
> > };
> > };
> >
> > __fixups__ {
> > foo = "/fragment@0:target:0";
> > };
> > };
> >
> > Signed-off-by: Brian Norris <briannorris@chromium.org>
> > ---
> > checks.c | 17 ++++++++++++++---
> > tests/overlay-reg-override.dtso | 6 ++++++
> > tests/run_tests.sh | 1 +
> > 3 files changed, 21 insertions(+), 3 deletions(-)
> > create mode 100644 tests/overlay-reg-override.dtso
> >
> > diff --git a/checks.c b/checks.c
> > index a6037ed08000..dd6824dce6ea 100644
> > --- a/checks.c
> > +++ b/checks.c
> > @@ -368,9 +368,12 @@ static void check_unit_address_vs_reg(struct check *c, struct dt_info *dti,
> > const char *unitname = get_unitname(node);
> > struct property *prop = get_property(node, "reg");
> >
> > - if (get_subnode(node, "__overlay__")) {
> > - /* HACK: Overlay fragments are a special case */
> > - return;
> > + /* HACK: Overlay fragments are a special case */
>
> Yeah... it's definitely a hack.
I'm just maintaining the comment you wrote. While it might be considered
a hack in some sense, it solves real problems that you admit and several
users have reported. What's the downside?
Or, what do you propose? That users rewrite the entire warning/checks
system [0]? That they use dubiously-compliant workarounds to suppress
the warnings [1]? That they use non-ergonomic syntax [2]? Or that people
just learn to ignore warnings (the status quo) [3]?
Brian
[0] I seriously doubt you'll get takers for a rewrite.
[1] (Herve's suggestion:)
https://lore.kernel.org/all/20260618222438.1423c6c0@bootlin.com/
[2] That you agreed is "not a good solution"?
https://lore.kernel.org/all/ajOpRVdPadsDi-KT@zatzit/
[3] Or if you're lucky, selective filtering:
dtc \
-Wno-reg_format \
-Wno-unit_address_vs_reg \
-Wno-avoid_default_addr_size [...]
But this might have even more collateral damage, if people start
applying that to *all* compilation steps, and not just
overlay-compilation.
prev parent reply other threads:[~2026-06-22 21:14 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-06-17 20:44 [PATCH] checks: Avoid warnings for reg override in __overlay__ Brian Norris
2026-06-17 22:54 ` Rob Herring
2026-06-17 23:35 ` Brian Norris
2026-06-18 2:58 ` Rob Herring
2026-06-18 8:16 ` David Gibson
2026-06-18 20:24 ` Herve Codina
2026-06-19 4:15 ` David Gibson
2026-06-18 8:25 ` David Gibson
2026-06-22 21:14 ` Brian Norris [this message]
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=ajmlyJ-JWc9MFyMO@google.com \
--to=briannorris@chromium.org \
--cc=david@gibson.dropbear.id.au \
--cc=devicetree-compiler@vger.kernel.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