Re: [PATCH] checks: Avoid warnings for reg override in __overlay__
Brian Norris <[email protected]> Mon, 22 Jun 2026 14:14:48 -0700
| Newsgroups | org.kernel.vger.devicetree-compiler |
|---|---|
| Message-ID | <[email protected]> |
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 <[email protected]>
> > ---
> > 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/[email protected]/
[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.