Hi Rob,
On 08.11.2024 14:25, Rob Herring wrote:
> On Fri, Nov 8, 2024 at 5:04 AM Marek Szyprowski
> <m.szyprowski@xxxxxxxxxxx> wrote:
>> 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@xxxxxxxxxx>
>>> ---
>>> 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):
> Thanks for the report. Let me know if any others have a different
> backtrace. Also, since it's a WARN_ONCE, fixing one case could expose
> others.
> >...
> I'm going to fold in the following fix which should fix the warning:
This fixes all the warnings I've observed.
Reported-by: Marek Szyprowski <m.szyprowski@xxxxxxxxxxx>
Tested-by: Marek Szyprowski <m.szyprowski@xxxxxxxxxxx>
> diff --git a/drivers/of/address.c b/drivers/of/address.c
> index 824bb449e007..f21f4699df7a 100644
> --- a/drivers/of/address.c
> +++ b/drivers/of/address.c
> @@ -333,7 +333,8 @@ static unsigned int of_bus_isa_get_flags(const __be32 *addr)
>
> static int of_bus_default_flags_match(struct device_node *np)
> {
> - return of_bus_n_addr_cells(np) == 3;
> + /* Check for presence first since of_bus_n_addr_cells() will
> walk parents */
> + return of_property_present(np, "#address-cells") &&
> (of_bus_n_addr_cells(np) == 3);
> }
>
> /*
> @@ -701,16 +702,16 @@ const __be32 *__of_get_address(struct
> device_node *dev, int index, int bar_no,
> if (strcmp(bus->name, "pci") && (bar_no >= 0))
> return NULL;
>
> - bus->count_cells(dev, &na, &ns);
> - if (!OF_CHECK_ADDR_COUNT(na))
> - return NULL;
> -
> /* Get "reg" or "assigned-addresses" property */
> prop = of_get_property(dev, bus->addresses, &psize);
> if (prop == NULL)
> return NULL;
> psize /= 4;
>
> + bus->count_cells(dev, &na, &ns);
> + if (!OF_CHECK_ADDR_COUNT(na))
> + return NULL;
> +
> onesize = na + ns;
> for (i = 0; psize >= onesize; psize -= onesize, prop += onesize, i++) {
> u32 val = be32_to_cpu(prop[0]);
>
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
