Re: [PATCH] arm64: make sure fdt has #address-cells and #size-cells properties

[Javier Martinez Canillas <javierm@redhat.com>]

On 10/31/19 12:56 PM, Vladimir 'phcoder' Serbinenko wrote:
> This patch looks incomplete. What if "chosen" is there but does not contain
> the relevant fields?
>

My understanding is that if there's a chosen node already defined in the
Device Tree but without #address-cells and #size-cells properties, then
is a malformed DT and is not up to GRUB to correct it.

But if GRUB is adding a child node as is the case of the chosen node, then
is up to GRUB to make sure that there are #address-cells and #size-cells
properties also defined so the child nodes are addressed correctly.

But maybe I'm wrong on this, so I'll let Mark comment since he authored
the patch and knows better what's the problem that's fixing.

> On Thu, 31 Oct 2019, 11:39 Javier Martinez Canillas, <javierm@redhat.com>
> wrote:
> 
>> From: Mark Salter <msalter@redhat.com>
>>
>> Recent upstream changes to kexec-tools relies on #address-cells
>> and #size-cells properties in the FDT. If grub2 needs to create
>> a chosen node, it is likely because firmware did not provide one.
>> In that case, set #address-cells and #size-cells properties to
>> make sure they exist.
>>
>> Signed-off-by: Mark Salter <msalter@redhat.com>
>> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
>> ---
>>
>>  grub-core/loader/arm64/linux.c | 16 +++++++++++++++-
>>  1 file changed, 15 insertions(+), 1 deletion(-)
>>
>> diff --git grub-core/loader/arm64/linux.c grub-core/loader/arm64/linux.c
>> index ef3e9f9444c..24d73732d89 100644
>> --- grub-core/loader/arm64/linux.c
>> +++ grub-core/loader/arm64/linux.c
>> @@ -78,7 +78,21 @@ finalize_params_linux (void)
>>
>>    node = grub_fdt_find_subnode (fdt, 0, "chosen");
>>    if (node < 0)
>> -    node = grub_fdt_add_subnode (fdt, 0, "chosen");
>> +    {
>> +      /*
>> +       * If we have to create a chosen node, Make sure we
>> +       * have #address-cells and #size-cells properties.
>> +       */
>> +      retval = grub_fdt_set_prop32(fdt, 0, "#address-cells", 2);
>> +      if (retval)
>> +        goto failure;
>> +
>> +      retval = grub_fdt_set_prop32(fdt, 0, "#size-cells", 2);
>> +      if (retval)
>> +        goto failure;
>> +
>> +      node = grub_fdt_add_subnode (fdt, 0, "chosen");
>> +    }
>>
>>    if (node < 1)
>>      goto failure;
>> --
>> 2.21.0
>>
>>
 
Best regards,
-- 
Javier Martinez Canillas
Software Engineer - Desktop Hardware Enablement
Red Hat