Devicetree Compiler
 help / color / mirror / Atom feed
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.

      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