[U-Boot] [PATCH 1/1] efi_loader: pass address to efi_install_fdt()

[Alexander Graf]

On 03.12.19 09:08, Heinrich Schuchardt wrote:
> On 12/3/19 7:37 AM, Alexander Graf wrote:
>>
>> On 03.12.19 08:27, Heinrich Schuchardt wrote:
>>> As part of moving the parsing of command line arguments to do_bootefi()
>>> call efi_install_fdt() with the address of the device tree instead of a
>>> string.
>>>
>>> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
>>> ---
>>>   cmd/bootefi.c | 30 ++++++++++++++++--------------
>>>   1 file changed, 16 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
>>> index f613cce7e2..3cf190889e 100644
>>> --- a/cmd/bootefi.c
>>> +++ b/cmd/bootefi.c
>>> @@ -196,40 +196,37 @@ static void *get_config_table(const efi_guid_t
>>> *guid)
>>>   #endif /* !CONFIG_IS_ENABLED(GENERATE_ACPI_TABLE) */
>>>
>>>   /**
>>> - * efi_install_fdt() - install fdt passed by a command argument
>>> + * efi_install_fdt() - install device tree
>>>    *
>>> - * If fdt_opt is available, the device tree located at that memory
>>> address will
>>> + * If fdt_addr is available, the device tree located at that memory
>>> address will
>>>    * will be installed as configuration table, otherwise the device
>>> tree located
>>>    * at the address indicated by environment variable fdtcontroladdr
>>> will be used.
>>>    *
>>> - * On architectures (x86) using ACPI tables device trees shall not be
>>> installed
>>> - * as configuration table.
>>> + * On architectures using ACPI tables device trees shall not be
>>> installed as
>>> + * configuration table.
>>>    *
>>> - * @fdt_opt:    pointer to argument
>>> + * @fdt_addr:    address of device tree
>>>    * Return:    status code
>>>    */
>>> -static efi_status_t efi_install_fdt(const char *fdt_opt)
>>> +static efi_status_t efi_install_fdt(uintptr_t fdt_addr)
>>>   {
>>>       /*
>>>        * The EBBR spec requires that we have either an FDT or an ACPI
>>> table
>>>        * but not both.
>>>        */
>>>   #if CONFIG_IS_ENABLED(GENERATE_ACPI_TABLE)
>>> -    if (fdt_opt) {
>>> +    if (fdt_addr) {
>>
>>
>> Why check for fdt_addr != 0 here? Since you do the parsing outside of
>> this function now, just make 0 a valid pointer and check for the
>> validity outside of this function.
>
> fdt_addr == 0 signals that U-Boot's internal device tree shall be used
> for the UEFI sub-system.
>
> Your suggested change would drop the capability to use the internal
> device tree. Why would you want to do so?


Mostly because I didn't look at the bigger picture, sorry :).
Looking at all the options, your patch is probably the best solution. 
Sorry for the fuss.


Reviewed-by: Alexander Graf <agraf at csgraf.de>


Alex