On 08/11/2024 14:58, Rob Herring wrote:

On Fri, Nov 8, 2024 at 8:33 AM Steven Price [off-list ref] wrote:

quoted

On 08/11/2024 14:04, Rob Herring wrote:

quoted

On Fri, Nov 8, 2024 at 7:26 AM Steven Price [off-list ref] wrote:

quoted

On 08/11/2024 11:04, Marek Szyprowski wrote:

quoted

Hi Rob,

On 06.11.2024 18:10, Rob Herring (Arm) wrote:

quoted

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@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@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.

quoted

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@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.

Thanks for the pointer. Yes that fix seems to work for my board!

Thanks,
Steve

quoted

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.

quoted

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