[U-Boot] [PATCH 2/2] efi_loader: Fall back to fdtfile naming convention

[Alexander Graf]

On 04/14/2016 03:43 PM, Andreas Färber wrote:
> Am 13.04.2016 um 23:22 schrieb Alexander Graf:
>> When there is no $fdtfile variable set, we still have a good chance
>> that on 32bit arm the fdtfile really is just called $soc-$board.dtb.
>>
>> Enable the exports for $soc and $board in our distr defaults and make
>> use of them in the efi boot script.
>>
>> Reported-by: Andreas Faerber <afaerber at suse.de>
>> Reported-by: Stephen Warren <swarren at wwwdotorg.org>
>> Signed-off-by: Alexander Graf <agraf at suse.de>
>> ---
>>   include/config_distro_bootcmd.h  | 24 +++++++++++++++++++++---
>>   include/config_distro_defaults.h |  1 +
>>   2 files changed, 22 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/config_distro_bootcmd.h b/include/config_distro_bootcmd.h
>> index 67eb8f2..eaaf2cc 100644
>> --- a/include/config_distro_bootcmd.h
>> +++ b/include/config_distro_bootcmd.h
>> @@ -99,6 +99,21 @@
>>   #endif
>>   
>>   #ifdef BOOTEFI_NAME
>> +#if defined(CONFIG_ARM) && !defined(CONFIG_ARM64)
>> +/*
>> + * On 32bit ARM systems there is a reasonable number of systems that follow
>> + * the $soc-$board$boardver.dtb name scheme for their device trees. Use that
>> + * scheme if we don't have an explicit fdtfile variable.
>> + */
> Why limit this to 32-bit? If fdtfile is not set and we drop the
> limitation above, then for CONFIG_ARM64 (and theoretically MIPS) we
> could add the vendor subdir to our prefixes. Even without the latter it
> does no harm, your config_distro_defaults.h change below does not seem
> to be limited to ARM anyway.

For ARM64 the pattern doesn't work because of the subdirectories, so 
we'll have to have a separate block that takes the soc vendor name into 
account as well. Does MIPS use dtb these days?

>
>> +#define BOOTENV_EFI_SET_FDTFILE_FALLBACK                                  \
>> +	"if test -z \"${fdtfile}\" -a -n \"${soc}\"; then "               \
>> +	  "setenv efifdtfile ${soc}-${board}${boardver}.dtb; "            \
> Please consistently use efi_ for readability.
>
> The logic looks slightly weird on first sight, being separated from the
> below context. Can't we just set fdtfile here if unset and drop the four
> extra lines below? Once boot has executed, there may be leftover
> variables such as filesize anyway.

The problem is that the boot may fail - and in that case you would have 
a potentially invalid fdtfile variable.


Alex