Re: [PATCH V5 12/15] Add efi_arch_use_config_file() function to control use of config file

[Roy Franz <roy.franz@linaro.org>]

On Mon, Sep 22, 2014 at 5:48 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 19.09.14 at 00:50, <roy.franz@linaro.org> wrote:
>> The x86 EFI build of Xen always uses a configuration file to load modules,
>> but
>> the ARM version can either use a config file to specify the modules, or be
>> loaded by GRUB in which case GRUB loads the modules and adds them to the DTB
>> that is passed to Xen.  Add the efi_arch_use_config_file() to indicate if a
>> configuration file is required.  For x86, this will always be true.  ARM
>> will
>> examine the DTB passed via EFI configuration table (if any), and if it
>> contains
>> module information will use that that not use the configuration file at all.
>> Add Emacs footer to efi-boot.h and boot.c
>>
>> Signed-off-by: Roy Franz <roy.franz@linaro.org>
>
> Acked-by: Jan Beulich <jbeulich@suse.com>
> with two formatting change requests:
>
>> @@ -809,109 +805,119 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
>>      if ( EFI_ERROR(status) )
>>          gop = NULL;
>>
>> -    /* Read and parse the config file. */
>> -    if ( !cfg_file_name.w )
>> +    if ( efi_arch_use_config_file(SystemTable) )
>>      {
>> -        CHAR16 *tail;
>> +        EFI_FILE_HANDLE dir_handle;
>> +        /* Get the file system interface. */
>> +        dir_handle = get_parent_handle(loaded_image, &file_name.w);
>
> Blank line between declarations and statements please.
I reviewed all the functions - forgot to check all the other blocks....

>
>> --- a/xen/include/asm-x86/efi-boot.h
>> +++ b/xen/include/asm-x86/efi-boot.h
>> @@ -628,3 +628,17 @@ static void __init efi_arch_load_addr_check(EFI_LOADED_IMAGE *loaded_image)
>>          blexit(L"Xen must be loaded at a 2Mb boundary.");
>>      trampoline_xen_phys_start = xen_phys_start;
>>  }
>> +
>> +static __init bool_t efi_arch_use_config_file(EFI_SYSTEM_TABLE *SystemTable)
>
> static <type> <attributes> <name>
>
> please (i.e. __init and bool_t ought to switch places).
>
I'll fix this..

Thanks,
Roy