[PATCH v2] of: WARN on deprecated #address-cells/#size-cells handling

[Rob Herring]

On Fri, Nov 8, 2024 at 8:33 AM Steven Price <steven.price at arm.com> wrote:
>
> On 08/11/2024 14:04, Rob Herring wrote:
> > On Fri, Nov 8, 2024 at 7:26 AM Steven Price <steven.price at arm.com> wrote:
> >>
> >> On 08/11/2024 11:04, Marek Szyprowski wrote:
> >>> Hi Rob,
> >>>
> >>> On 06.11.2024 18:10, Rob Herring (Arm) wrote:
> >>>> While OpenFirmware originally allowed walking parent nodes and default
> >>>> root values for #address-cells and #size-cells, FDT has long required
> >>>> explicit values. It's been a warning in dtc for the root node since the
> >>>> beginning (2005) and for any parent node since 2007. Of course, not all
> >>>> FDT uses dtc, but that should be the majority by far. The various
> >>>> extracted OF devicetrees I have dating back to the 1990s (various
> >>>> PowerMac, OLPC, PASemi Nemo) all have explicit root node properties. The
> >>>> warning is disabled for Sparc as there are known systems relying on
> >>>> default root node values.
> >>>>
> >>>> Signed-off-by: Rob Herring (Arm) <robh at kernel.org>
> >>>> ---
> >>>> v2:
> >>>>   - Add a define for excluded platforms to help clarify the intent
> >>>>     is to have an exclude list and make adding platforms easier.
> >>>>   - Also warn when walking parent nodes.
> >>>> ---
> >>>>   drivers/of/base.c | 28 ++++++++++++++++++++++------
> >>>>   drivers/of/fdt.c  |  4 ++--
> >>>>   2 files changed, 24 insertions(+), 8 deletions(-)
> >>>
> >>> This patch landed in today's linux-next as commit 4b28a0dec185 ("of:
> >>> WARN on deprecated #address-cells/#size-cells handling"). In my tests I
> >>> found that it introduces warnings on almost all of my test systems. I
> >>> took a look at the first one I got in my logs (Samsung Exynos Rinato
> >>> board: arch/arm/boot/dts/samsung/exynos3250-rinato.dts):
> >>
> >> Just a "me too" for rk3288-firefly.dtb:
> >>
> >> [    0.138735] WARNING: CPU: 0 PID: 1 at drivers/of/base.c:106 of_bus_n_addr_cells+0x9c/0xd8
> >> [    0.138776] Missing '#address-cells' in /power-management at ff730000
> >>
> >> I'm sure it's easy to fix up the DTB, but we shouldn't be breaking long existing DTBs.
> >
> > What broke?
>
> Nothing 'broke' as such (the board continued booting) but the WARN
> shouldn't be happening. My CI treats the WARN as a failure as these
> shouldn't occur unless there's a programming error.
>
> > The intent here is to exclude any platforms/arch which actually need
> > the deprecated behavior, not change DTBs. That's spelled out at the
> > WARN which I assume people would read before fixing "Missing
> > '#address-cells' in /power-management at ff730000". I tried to make the
> > warn message indicate that on v1 with:
> >
> > WARN_ONCE(!IS_ENABLED(CONFIG_SPARC), "Only listed platforms should
> > rely on default '#address-cells'\n");
>
> So one possibility is to include this platform in the exclusion list -
> but I'm not sure how to do that, I assume including CONFIG_ARM in the
> list would rather defeat the point of the patch. But my feeling is that
> it would involve a lot of playing whack-a-mole to identify individual
> platforms.

Please see my posted fix in this thread. Things "broke" quite a bit
more widely than anticipated.

> One obvious idea would be to look at the DTBs in the kernel tree and see
> which are affected by this currently, that might be a good place to
> start with an exclusion list.

It's been a dtc warning since 2007, so I can say all of the in tree
dts's are fine. The problem for these reported platforms is the
kernel, not the DT.

> You could also downgrade the warning to a pr_warn() or similar.

I find that pr_warn() may or may not get noticed, but WARN for sure
will which is what I want here.

Rob