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

[Alexander Graf]

On 03.12.19 11:26, Alexander Graf wrote:
>
> 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.


Actually, thinking about it again, can we make the "Please fall back to 
internal device tree" token explicit instead of the implicit 0? It might 
be enough to just do something as simple as

   #define EFI_FDT_USE_INTERNAL 0

and then use that in the if(!fdt_addr) line and in do_bootefi() instead. 
Then document it in the function header. That way we can make it an 
actual supported API that FIT image support can rely on.


Alex



Alex