Re: [PATCH] powerpc/prom_init: Fixup missing powermac #size-cells

[Michael Ellerman <mpe@ellerman.id.au>]

Rob Herring <robh@kernel.org> writes:
> On Mon, Nov 25, 2024 at 8:57 PM Michael Ellerman <mpe@ellerman.id.au> wrote:
>>
>> On some powermacs `escc` nodes are missing `#size-cells` properties,
>> which is deprecated and now triggers a warning at boot since commit
>> 045b14ca5c36 ("of: WARN on deprecated #address-cells/#size-cells
>> handling").
>>
>> For example:
>>
>>   Missing '#size-cells' in /pci@f2000000/mac-io@c/escc@13000
>>   WARNING: CPU: 0 PID: 0 at drivers/of/base.c:133 of_bus_n_size_cells+0x98/0x108
>>   Hardware name: PowerMac3,1 7400 0xc0209 PowerMac
>>   ...
>>   Call Trace:
>>     of_bus_n_size_cells+0x98/0x108 (unreliable)
>>     of_bus_default_count_cells+0x40/0x60
>>     __of_get_address+0xc8/0x21c
>>     __of_address_to_resource+0x5c/0x228
>>     pmz_init_port+0x5c/0x2ec
>>     pmz_probe.isra.0+0x144/0x1e4
>>     pmz_console_init+0x10/0x48
>>     console_init+0xcc/0x138
>>     start_kernel+0x5c4/0x694
>>
>> As powermacs boot via prom_init it's possible to add the missing
>> properties to the device tree during boot, avoiding the warning. Note
>> that `escc-legacy` nodes are also missing `#size-cells` properties, but
>> they are skipped by the macio driver, so leave them alone.
>>
>> Depends-on: 045b14ca5c36 ("of: WARN on deprecated #address-cells/#size-cells handling")
>> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
>> ---
>>  arch/powerpc/kernel/prom_init.c | 29 +++++++++++++++++++++++++++--
>>  1 file changed, 27 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
>> index 73210e5bcfa7..8e776ba39497 100644
>> --- a/arch/powerpc/kernel/prom_init.c
>> +++ b/arch/powerpc/kernel/prom_init.c
>> @@ -2848,7 +2848,7 @@ static void __init fixup_device_tree_chrp(void)
>>  #endif
>>
>>  #if defined(CONFIG_PPC64) && defined(CONFIG_PPC_PMAC)
>> -static void __init fixup_device_tree_pmac(void)
>> +static void __init fixup_device_tree_pmac64(void)
>>  {
>>         phandle u3, i2c, mpic;
>>         u32 u3_rev;
>> @@ -2888,7 +2888,31 @@ static void __init fixup_device_tree_pmac(void)
>>                      &parent, sizeof(parent));
>>  }
>>  #else
>> -#define fixup_device_tree_pmac()
>> +#define fixup_device_tree_pmac64()
>> +#endif
>> +
>> +#ifdef CONFIG_PPC_PMAC
>> +static void __init fixup_device_tree_pmac(void)
>> +{
>> +       __be32 val = 1;
>> +       char type[8];
>> +       phandle node;
>
> I suppose you are keeping the existing style, but you could use
> IS_ENABLED() here instead of #ifdef.

Yeah true. I'll do that as a follow-up and convert them all.

> Either way,
>
> Reviewed-by: Rob Herring <robh@kernel.org>

Thanks.

cheers