[regression] of: mis-parsing Depthcharge's /firmware

[regression] of: mis-parsing Depthcharge's /firmware

[Brian Norris]

Hi all,

(New subject; was "Re: [GIT PULL] Devicetree updates for v6.13")

On Mon, Dec 09, 2024 at 05:28:09PM +0800, Chen-Yu Tsai wrote:
> steelix.dtb is the same, plus the firmware now inserts #address-cells
> and #size-cells under /firmware. This fix has landed for all future
> ChromeOS devices via our main firmware branch [1].
> 
> AFAIK they also have a bad FDT END symbol. This was only recently
> discovered and fixed for future devices [2].
> 
> 
> ChenYu
> 
> [1] Gerrit: https://crrev.com/c/6051580
> [2] Gerrit: https://review.coreboot.org/c/coreboot/+/85462

This all comes back to bite us, since nobody went back to patch the
existing Chromebook device trees, and now we've added a true regression
on top:

In commit 6e5773d52f4a ("of/address: Fix WARN when attempting
translating non-translatable addresses") we now reject devices without
'#address-cells', and this breaks the DTs generated by bootloaders
without Chen-Yu's https://crrev.com/c/6051580 fix (this is ... pretty
much all Chromebooks). Specifically, Linux now refuses to add 'reg'
resources to the /firmware/coreboot device, and we fail with:

[   11.886271] coreboot_table firmware:coreboot: probe with driver coreboot_table failed with error -22

This is almost certainly a DTB ABI regression.

This was noticed here (OpenWrt supports some Chromium-based WiFi routers
that use Depthcharge-based bootloaders from many years ago):

https://github.com/openwrt/openwrt/issues/21243

For now, I just patched up the OpenWrt DTS files like so:
https://github.com/openwrt/openwrt/pull/22951

But what should we do going forward? I note that Rob says "We may
revisit this later and address with a fixup to the DT itself" in commit
8600058ba28a ("of: Add coreboot firmware to excluded default cells
list").

That never happened, and a ton of Chromium devices are still broken.
(They don't have WARNINGs, but /sys/firmware/vpd, etc., is still
missing.)

Can we patch of_bus_default_match() to accept an empty 'ranges' [1]? Or
should I go patch every Chromium-device DTS file I can find? So far, I
think I can get that done in 17 files in the upstream tree...

Brian

[1] From ePAPR:

  "If the [ranges] property is defined with an <empty> value, it
  specifies that the parent and child address 28 space is identical, and
  no address translation is required."

And:

  "An ePAPR-compliant boot program shall supply #address-cells and
  #size-cells on all nodes 16 that have children.

  If missing, a client program should assume a default value of 2 for
  #address-cells, and a value of 1 for #size-cells."

So far, this does the trick, but I didn't review all the ramifications
here.

diff --git a/drivers/of/address.c b/drivers/of/address.c
index 4034d798c55a..f86386c407d4 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -328,7 +328,15 @@ static int of_bus_default_flags_match(struct device_node *np)
 
 static int of_bus_default_match(struct device_node *np)
 {
-	return of_property_present(np, "#address-cells");
+	int len;
+
+	if (of_property_present(np, "#address-cells"))
+		return true;
+
+	if (of_find_property(np, "ranges", &len) && len == 0)
+		return true;
+
+	return false;
 }
 
 /*

Re: [regression] of: mis-parsing Depthcharge's /firmware

[Rob Herring]

On Fri, Apr 17, 2026 at 4:26 PM Brian Norris <briannorris@chromium.org> wrote:
>
> Hi all,
>
> (New subject; was "Re: [GIT PULL] Devicetree updates for v6.13")
>
> On Mon, Dec 09, 2024 at 05:28:09PM +0800, Chen-Yu Tsai wrote:
> > steelix.dtb is the same, plus the firmware now inserts #address-cells
> > and #size-cells under /firmware. This fix has landed for all future
> > ChromeOS devices via our main firmware branch [1].
> >
> > AFAIK they also have a bad FDT END symbol. This was only recently
> > discovered and fixed for future devices [2].
> >
> >
> > ChenYu
> >
> > [1] Gerrit: https://crrev.com/c/6051580
> > [2] Gerrit: https://review.coreboot.org/c/coreboot/+/85462
>
> This all comes back to bite us, since nobody went back to patch the
> existing Chromebook device trees, and now we've added a true regression
> on top:
>
> In commit 6e5773d52f4a ("of/address: Fix WARN when attempting
> translating non-translatable addresses") we now reject devices without
> '#address-cells', and this breaks the DTs generated by bootloaders
> without Chen-Yu's https://crrev.com/c/6051580 fix (this is ... pretty
> much all Chromebooks). Specifically, Linux now refuses to add 'reg'
> resources to the /firmware/coreboot device, and we fail with:
>
> [   11.886271] coreboot_table firmware:coreboot: probe with driver coreboot_table failed with error -22
>
> This is almost certainly a DTB ABI regression.
>
> This was noticed here (OpenWrt supports some Chromium-based WiFi routers
> that use Depthcharge-based bootloaders from many years ago):
>
> https://github.com/openwrt/openwrt/issues/21243
>
> For now, I just patched up the OpenWrt DTS files like so:
> https://github.com/openwrt/openwrt/pull/22951
>
> But what should we do going forward? I note that Rob says "We may
> revisit this later and address with a fixup to the DT itself" in commit
> 8600058ba28a ("of: Add coreboot firmware to excluded default cells
> list").
>
> That never happened, and a ton of Chromium devices are still broken.

The above just silenced the warning. If they are broken, then
something else broke them.

> (They don't have WARNINGs, but /sys/firmware/vpd, etc., is still
> missing.)
>
> Can we patch of_bus_default_match() to accept an empty 'ranges' [1]? Or
> should I go patch every Chromium-device DTS file I can find? So far, I
> think I can get that done in 17 files in the upstream tree...

Both.

Though I'd rather the kernel fixup the DT rather than relax the
parsing code for everyone. Then we know what platforms need this and
don't let new ones in.

>
> Brian
>
> [1] From ePAPR:
>
>   "If the [ranges] property is defined with an <empty> value, it
>   specifies that the parent and child address 28 space is identical, and
>   no address translation is required."
>
> And:
>
>   "An ePAPR-compliant boot program shall supply #address-cells and
>   #size-cells on all nodes 16 that have children.
>
>   If missing, a client program should assume a default value of 2 for
>   #address-cells, and a value of 1 for #size-cells."

ePAPR may say that, but that's not what the kernel implements,
defaulting to 1 address cell (on !SPARC). dtc however does default to
2 cells. Relying on defaults has been a warning in dtc essentially
forever. I'd like to get rid of defaults in the kernel

Rob

Re: [regression] of: mis-parsing Depthcharge's /firmware

[Brian Norris]

Hi Rob,

On Mon, Apr 20, 2026 at 07:57:40AM -0500, Rob Herring wrote:
> On Fri, Apr 17, 2026 at 4:26 PM Brian Norris <briannorris@chromium.org> wrote:
> >
> > Hi all,
> >
> > (New subject; was "Re: [GIT PULL] Devicetree updates for v6.13")
> >
> > On Mon, Dec 09, 2024 at 05:28:09PM +0800, Chen-Yu Tsai wrote:
> > > steelix.dtb is the same, plus the firmware now inserts #address-cells
> > > and #size-cells under /firmware. This fix has landed for all future
> > > ChromeOS devices via our main firmware branch [1].
> > >
> > > AFAIK they also have a bad FDT END symbol. This was only recently
> > > discovered and fixed for future devices [2].
> > >
> > >
> > > ChenYu
> > >
> > > [1] Gerrit: https://crrev.com/c/6051580
> > > [2] Gerrit: https://review.coreboot.org/c/coreboot/+/85462
> >
> > This all comes back to bite us, since nobody went back to patch the
> > existing Chromebook device trees, and now we've added a true regression
> > on top:
> >
> > In commit 6e5773d52f4a ("of/address: Fix WARN when attempting
> > translating non-translatable addresses") we now reject devices without
> > '#address-cells', and this breaks the DTs generated by bootloaders
> > without Chen-Yu's https://crrev.com/c/6051580 fix (this is ... pretty
> > much all Chromebooks). Specifically, Linux now refuses to add 'reg'
> > resources to the /firmware/coreboot device, and we fail with:
> >
> > [   11.886271] coreboot_table firmware:coreboot: probe with driver coreboot_table failed with error -22
> >
> > This is almost certainly a DTB ABI regression.
> >
> > This was noticed here (OpenWrt supports some Chromium-based WiFi routers
> > that use Depthcharge-based bootloaders from many years ago):
> >
> > https://github.com/openwrt/openwrt/issues/21243
> >
> > For now, I just patched up the OpenWrt DTS files like so:
> > https://github.com/openwrt/openwrt/pull/22951
> >
> > But what should we do going forward? I note that Rob says "We may
> > revisit this later and address with a fixup to the DT itself" in commit
> > 8600058ba28a ("of: Add coreboot firmware to excluded default cells
> > list").
> >
> > That never happened, and a ton of Chromium devices are still broken.
> 
> The above just silenced the warning. If they are broken, then
> something else broke them.

Right.

To be clear, the regression is in commit 6e5773d52f4a, not 8600058ba28a.
But 8600058ba28a (and this thread I'm replying to):

(a) started the precedent of treating this known-problemtatic DT pattern
    specially;

(b) started to consider "fixing" those old DTs (but notably, not
    reliably/proactively -- even if Google updates official bootloaders,
    many devices are far out of Google support; or even if supported,
    don't have a systematic way of receiving Google-provided updates
    because they run non-Google software); and

(c) because (a)/(b) hid the problem partially, it was less noticeable
    that commit 6e5773d52f4a *really* broke things a month later, in the
    last days of the v6.13 cycle. (Official Google testing probably
    didn't notice, because they only tested devices with the latest
    Google bootloaders. Only people with old bootloaders / non-Google
    software noticed.)

> > (They don't have WARNINGs, but /sys/firmware/vpd, etc., is still
> > missing.)
> >
> > Can we patch of_bus_default_match() to accept an empty 'ranges' [1]? Or
> > should I go patch every Chromium-device DTS file I can find? So far, I
> > think I can get that done in 17 files in the upstream tree...
> 
> Both.

To be clear, my options were:

1. fix up kernel parsing to accept these /firmware/coreboot node
   structures (with empty ranges / no #{address,size}-cells)
2. add #{address,size}-cells into the kernel-included dts(i) files (this
   will merge safely with the DTB modifications patched in by old
   bootloaders).

I wouldn't call #2 "kernel fixup the DT", personally. I'd call it "fix
up the DT source that happens to be provided by the kernel." This
assumes no one is using device trees that are exclusively maintained
outside the kernel. (I believe that's generally true, except for
OpenWrt. And even there, it's still acceptable to patch the DT source,
and I've already done so.)

> Though I'd rather the kernel fixup the DT rather than relax the
> parsing code for everyone. Then we know what platforms need this and
> don't let new ones in.

I'm not sure how to parse this. This paragraph sounds like a 3rd option:

3. "kernel fixup the DT" -- sound like you want the kernel to identify
   these specific /firmware/coreboot structures, and activtly
   modify/patch the FDT at runtime

Is that an accurate interpretation? If so, that sounds rather novel, and
nothing like "both" (#1 + #2 above). It's certainly possible, but seems
like a large lift for this particular incompatibility.

So I assume you actually meant something else, possibly a clarification
or narrowing of #1 or #2. Can you help un-confuse me on what you think
the best route or routes are?

Brian

Re: [regression] of: mis-parsing Depthcharge's /firmware

[Rob Herring]

On Mon, Apr 20, 2026 at 3:57 PM Brian Norris <briannorris@chromium.org> wrote:
>
> Hi Rob,
>
> On Mon, Apr 20, 2026 at 07:57:40AM -0500, Rob Herring wrote:
> > On Fri, Apr 17, 2026 at 4:26 PM Brian Norris <briannorris@chromium.org> wrote:
> > >
> > > Hi all,
> > >
> > > (New subject; was "Re: [GIT PULL] Devicetree updates for v6.13")
> > >
> > > On Mon, Dec 09, 2024 at 05:28:09PM +0800, Chen-Yu Tsai wrote:
> > > > steelix.dtb is the same, plus the firmware now inserts #address-cells
> > > > and #size-cells under /firmware. This fix has landed for all future
> > > > ChromeOS devices via our main firmware branch [1].
> > > >
> > > > AFAIK they also have a bad FDT END symbol. This was only recently
> > > > discovered and fixed for future devices [2].
> > > >
> > > >
> > > > ChenYu
> > > >
> > > > [1] Gerrit: https://crrev.com/c/6051580
> > > > [2] Gerrit: https://review.coreboot.org/c/coreboot/+/85462
> > >
> > > This all comes back to bite us, since nobody went back to patch the
> > > existing Chromebook device trees, and now we've added a true regression
> > > on top:
> > >
> > > In commit 6e5773d52f4a ("of/address: Fix WARN when attempting
> > > translating non-translatable addresses") we now reject devices without
> > > '#address-cells', and this breaks the DTs generated by bootloaders
> > > without Chen-Yu's https://crrev.com/c/6051580 fix (this is ... pretty
> > > much all Chromebooks). Specifically, Linux now refuses to add 'reg'
> > > resources to the /firmware/coreboot device, and we fail with:
> > >
> > > [   11.886271] coreboot_table firmware:coreboot: probe with driver coreboot_table failed with error -22
> > >
> > > This is almost certainly a DTB ABI regression.
> > >
> > > This was noticed here (OpenWrt supports some Chromium-based WiFi routers
> > > that use Depthcharge-based bootloaders from many years ago):
> > >
> > > https://github.com/openwrt/openwrt/issues/21243
> > >
> > > For now, I just patched up the OpenWrt DTS files like so:
> > > https://github.com/openwrt/openwrt/pull/22951
> > >
> > > But what should we do going forward? I note that Rob says "We may
> > > revisit this later and address with a fixup to the DT itself" in commit
> > > 8600058ba28a ("of: Add coreboot firmware to excluded default cells
> > > list").
> > >
> > > That never happened, and a ton of Chromium devices are still broken.
> >
> > The above just silenced the warning. If they are broken, then
> > something else broke them.
>
> Right.
>
> To be clear, the regression is in commit 6e5773d52f4a, not 8600058ba28a.
> But 8600058ba28a (and this thread I'm replying to):
>
> (a) started the precedent of treating this known-problemtatic DT pattern
>     specially;
>
> (b) started to consider "fixing" those old DTs (but notably, not
>     reliably/proactively -- even if Google updates official bootloaders,
>     many devices are far out of Google support; or even if supported,
>     don't have a systematic way of receiving Google-provided updates
>     because they run non-Google software); and
>
> (c) because (a)/(b) hid the problem partially, it was less noticeable
>     that commit 6e5773d52f4a *really* broke things a month later, in the
>     last days of the v6.13 cycle. (Official Google testing probably
>     didn't notice, because they only tested devices with the latest
>     Google bootloaders. Only people with old bootloaders / non-Google
>     software noticed.)
>
> > > (They don't have WARNINGs, but /sys/firmware/vpd, etc., is still
> > > missing.)
> > >
> > > Can we patch of_bus_default_match() to accept an empty 'ranges' [1]? Or
> > > should I go patch every Chromium-device DTS file I can find? So far, I
> > > think I can get that done in 17 files in the upstream tree...
> >
> > Both.
>
> To be clear, my options were:
>
> 1. fix up kernel parsing to accept these /firmware/coreboot node
>    structures (with empty ranges / no #{address,size}-cells)
> 2. add #{address,size}-cells into the kernel-included dts(i) files (this
>    will merge safely with the DTB modifications patched in by old
>    bootloaders).
>
> I wouldn't call #2 "kernel fixup the DT", personally. I'd call it "fix
> up the DT source that happens to be provided by the kernel." This
> assumes no one is using device trees that are exclusively maintained
> outside the kernel. (I believe that's generally true, except for
> OpenWrt. And even there, it's still acceptable to patch the DT source,
> and I've already done so.)
>
> > Though I'd rather the kernel fixup the DT rather than relax the
> > parsing code for everyone. Then we know what platforms need this and
> > don't let new ones in.
>
> I'm not sure how to parse this. This paragraph sounds like a 3rd option:

Well, not in the sense of pick one of 3 options. It's another option
in how to fix the kernel. I think we should fix any .dts files we can
in addition to fixing the kernel.

> 3. "kernel fixup the DT" -- sound like you want the kernel to identify
>    these specific /firmware/coreboot structures, and activtly
>    modify/patch the FDT at runtime
>
> Is that an accurate interpretation? If so, that sounds rather novel, and
> nothing like "both" (#1 + #2 above). It's certainly possible, but seems
> like a large lift for this particular incompatibility.

Yes. It's not novel though. The powerpc code is littered with such
things. Some of them due to the commit in question here. Look at
commits from me in arch/powerpc.

I started some common infrastructure to apply fixups, but the case in
particular that needed it ended up not needing it. So it's something I
have on a branch somewhere. Also it worked on the unflattened tree as
not all things need to be fixed up early.

Rob

Re: [regression] of: mis-parsing Depthcharge's /firmware

[Brian Norris]

On Mon, Apr 20, 2026 at 05:19:06PM -0500, Rob Herring wrote:
> On Mon, Apr 20, 2026 at 3:57 PM Brian Norris <briannorris@chromium.org> wrote:
> > On Mon, Apr 20, 2026 at 07:57:40AM -0500, Rob Herring wrote:
> > > On Fri, Apr 17, 2026 at 4:26 PM Brian Norris <briannorris@chromium.org> wrote:
> > > > Can we patch of_bus_default_match() to accept an empty 'ranges' [1]? Or
> > > > should I go patch every Chromium-device DTS file I can find? So far, I
> > > > think I can get that done in 17 files in the upstream tree...
> > >
> > > Both.
> >
> > To be clear, my options were:
> >
> > 1. fix up kernel parsing to accept these /firmware/coreboot node
> >    structures (with empty ranges / no #{address,size}-cells)
> > 2. add #{address,size}-cells into the kernel-included dts(i) files (this
> >    will merge safely with the DTB modifications patched in by old
> >    bootloaders).
> >
> > I wouldn't call #2 "kernel fixup the DT", personally. I'd call it "fix
> > up the DT source that happens to be provided by the kernel." This
> > assumes no one is using device trees that are exclusively maintained
> > outside the kernel. (I believe that's generally true, except for
> > OpenWrt. And even there, it's still acceptable to patch the DT source,
> > and I've already done so.)
> >
> > > Though I'd rather the kernel fixup the DT rather than relax the
> > > parsing code for everyone. Then we know what platforms need this and
> > > don't let new ones in.
> >
> > I'm not sure how to parse this. This paragraph sounds like a 3rd option:
> 
> Well, not in the sense of pick one of 3 options. It's another option
> in how to fix the kernel.

Ah, got it. So it's an alternative to #1.

> I think we should fix any .dts files we can
> in addition to fixing the kernel.

OK. I have patches for both, and I'll see about sending them out in the
next day or two.

> > 3. "kernel fixup the DT" -- sound like you want the kernel to identify
> >    these specific /firmware/coreboot structures, and activtly
> >    modify/patch the FDT at runtime
> >
> > Is that an accurate interpretation? If so, that sounds rather novel, and
> > nothing like "both" (#1 + #2 above). It's certainly possible, but seems
> > like a large lift for this particular incompatibility.
> 
> Yes. It's not novel though. The powerpc code is littered with such
> things. Some of them due to the commit in question here. Look at
> commits from me in arch/powerpc.
> 
> I started some common infrastructure to apply fixups, but the case in
> particular that needed it ended up not needing it. So it's something I
> have on a branch somewhere. Also it worked on the unflattened tree as
> not all things need to be fixed up early.

You say it's not novel, but then you say the only existing code is
either:

1) completely different, and only applicable to powerpc or
2) only on your local tree.

That sounds novel to me :)

Anyway, I'm more inclined to lean on my #1 and/or #2 than to write a
whole new fixup layer. But maybe #1 can be replaced in the future if we
come to really want/need a generic fixup layer in the future.

(Frankly, if we do #2, #1 and #3 will probably both be redundant and
unnecessary. I don't know of any case here where we're relying on strict
DTB ABI compatibility with no opportunity to update some of the DTS
sources.)

Brian

Re: [regression] of: mis-parsing Depthcharge's /firmware

[Rob Herring]

On Mon, Apr 20, 2026 at 03:54:42PM -0700, Brian Norris wrote:
> On Mon, Apr 20, 2026 at 05:19:06PM -0500, Rob Herring wrote:
> > On Mon, Apr 20, 2026 at 3:57 PM Brian Norris <briannorris@chromium.org> wrote:
> > > On Mon, Apr 20, 2026 at 07:57:40AM -0500, Rob Herring wrote:
> > > > On Fri, Apr 17, 2026 at 4:26 PM Brian Norris <briannorris@chromium.org> wrote:
> > > > > Can we patch of_bus_default_match() to accept an empty 'ranges' [1]? Or
> > > > > should I go patch every Chromium-device DTS file I can find? So far, I
> > > > > think I can get that done in 17 files in the upstream tree...
> > > >
> > > > Both.
> > >
> > > To be clear, my options were:
> > >
> > > 1. fix up kernel parsing to accept these /firmware/coreboot node
> > >    structures (with empty ranges / no #{address,size}-cells)
> > > 2. add #{address,size}-cells into the kernel-included dts(i) files (this
> > >    will merge safely with the DTB modifications patched in by old
> > >    bootloaders).
> > >
> > > I wouldn't call #2 "kernel fixup the DT", personally. I'd call it "fix
> > > up the DT source that happens to be provided by the kernel." This
> > > assumes no one is using device trees that are exclusively maintained
> > > outside the kernel. (I believe that's generally true, except for
> > > OpenWrt. And even there, it's still acceptable to patch the DT source,
> > > and I've already done so.)
> > >
> > > > Though I'd rather the kernel fixup the DT rather than relax the
> > > > parsing code for everyone. Then we know what platforms need this and
> > > > don't let new ones in.
> > >
> > > I'm not sure how to parse this. This paragraph sounds like a 3rd option:
> > 
> > Well, not in the sense of pick one of 3 options. It's another option
> > in how to fix the kernel.
> 
> Ah, got it. So it's an alternative to #1.
> 
> > I think we should fix any .dts files we can
> > in addition to fixing the kernel.
> 
> OK. I have patches for both, and I'll see about sending them out in the
> next day or two.
> 
> > > 3. "kernel fixup the DT" -- sound like you want the kernel to identify
> > >    these specific /firmware/coreboot structures, and activtly
> > >    modify/patch the FDT at runtime
> > >
> > > Is that an accurate interpretation? If so, that sounds rather novel, and
> > > nothing like "both" (#1 + #2 above). It's certainly possible, but seems
> > > like a large lift for this particular incompatibility.
> > 
> > Yes. It's not novel though. The powerpc code is littered with such
> > things. Some of them due to the commit in question here. Look at
> > commits from me in arch/powerpc.
> > 
> > I started some common infrastructure to apply fixups, but the case in
> > particular that needed it ended up not needing it. So it's something I
> > have on a branch somewhere. Also it worked on the unflattened tree as
> > not all things need to be fixed up early.
> 
> You say it's not novel, but then you say the only existing code is
> either:
> 
> 1) completely different, and only applicable to powerpc or
> 2) only on your local tree.
> 
> That sounds novel to me :)
> 
> Anyway, I'm more inclined to lean on my #1 and/or #2 than to write a
> whole new fixup layer. But maybe #1 can be replaced in the future if we
> come to really want/need a generic fixup layer in the future.

The problem with #1 is a) new platforms can then repeat the same mistake 
and b) we'll forget what platforms needed some work-around and whether 
we still need to maintain such a work-around. Well, I might not forget, 
but the next DT maintainer (applications welcome!) even won't know. For 
example, I have know clue if we still need to carry some of the 
work-arounds embedded into the interrupt parsing code. That all 
predates me. The only way I find out is breaking them (I'll never 
understand why people still run PowerMacs from the 1990s).

Calling the fixup code a layer is an exageration. It's on my kernel.org 
tree in the dt/fixup-infrastruct branch. And look, guess what issue it 
was that it has a fixup for.

> (Frankly, if we do #2, #1 and #3 will probably both be redundant and
> unnecessary. I don't know of any case here where we're relying on strict
> DTB ABI compatibility with no opportunity to update some of the DTS
> sources.)

Shrug. I thought the ABI was a concern here. It's ultimately up to the 
maintainers and users of a given platform whether or not they care about 
the ABI.

Rob

Re: [regression] of: mis-parsing Depthcharge's /firmware

[Brian Norris]

Hi Rob,

Thanks for your thoughts. I delayed a bit since I wasn't really sure
what the right conclusion was and wanted to give it some thought.

On Tue, Apr 21, 2026 at 02:30:38PM -0500, Rob Herring wrote:
> On Mon, Apr 20, 2026 at 03:54:42PM -0700, Brian Norris wrote:
> > You say it's not novel, but then you say the only existing code is
> > either:
> > 
> > 1) completely different, and only applicable to powerpc or
> > 2) only on your local tree.
> > 
> > That sounds novel to me :)
> > 
> > Anyway, I'm more inclined to lean on my #1 and/or #2 than to write a
> > whole new fixup layer. But maybe #1 can be replaced in the future if we
> > come to really want/need a generic fixup layer in the future.
> 
> The problem with #1 is a) new platforms can then repeat the same mistake 
> and b) we'll forget what platforms needed some work-around and whether 
> we still need to maintain such a work-around. Well, I might not forget, 
> but the next DT maintainer (applications welcome!) even won't know. For 
> example, I have know clue if we still need to carry some of the 
> work-arounds embedded into the interrupt parsing code. That all 
> predates me. The only way I find out is breaking them (I'll never 
> understand why people still run PowerMacs from the 1990s).

Ack to most of this. I'll note that it's possible to write unit tests
for this, if we go for the in-kernel route though, so hopefully that'd
give maintainers a bit more visibility.

> Calling the fixup code a layer is an exageration. It's on my kernel.org 
> tree in the dt/fixup-infrastruct branch. And look, guess what issue it 
> was that it has a fixup for.

OK! I spoke in ignorance then. I really haven't explored the device tree
construction / unflattening logic, so it was new to me.

If it really comes to patching the kernel itself, I'll consider pulling
in your patch. On first glance, it looks good, and not that complex even
to an outsider like me.

> > (Frankly, if we do #2, #1 and #3 will probably both be redundant and
> > unnecessary. I don't know of any case here where we're relying on strict
> > DTB ABI compatibility with no opportunity to update some of the DTS
> > sources.)
> 
> Shrug. I thought the ABI was a concern here. It's ultimately up to the 
> maintainers and users of a given platform whether or not they care about 
> the ABI.

I'm usually on the receiving end of people complaining about ABI. It
seems like we bend over backwards in a lot of places (drivers, driver
frameworks) to maintain some idea of DTB ABI, while in practice, the ABI
is almost never a strict concern for anything I've dealt with -- 98% of
the DTB is generated from in-kernel DTS sources that match the kernel.

($subject case is actually the closest we get to DTB ABI concerns,
because it involves the small part of the DTB that is generated by a
program that is independent from the kernel tree. But even there, it's
possible to fix the issue in the kernel-provided source.)

So it seems to me like maybe it's best to just ignore the ABI concern,
patch the DTS and call it a day. I've done that here:

  https://lore.kernel.org/all/20260428200712.2660635-1-briannorris@chromium.org/
  [PATCH 0/7] dts: Add /firmware/#{address,size}-cells to Chromium-based DTs

If someone finds reason we should still go back to fix the kernel
itself, I can resurrect the fixup logic in addition.

Thanks,
Brian