[U-Boot] [PATCH v2 02/11] efi_loader: return efi_status_t from efi_gop_register

[Heinrich Schuchardt]

On 03/23/2018 03:29 PM, Simon Glass wrote:
> Hi Heinrich,
> 
> On 15 February 2018 at 00:31, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>> All initialization routines should return a status code instead of
>> a boolean.
>>
>> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
>> ---
>> v2
>>         new patch
>> ---
>>  include/efi_loader.h     |  2 +-
>>  lib/efi_loader/efi_gop.c | 34 ++++++++++++++++++++++------------
>>  2 files changed, 23 insertions(+), 13 deletions(-)
>>
>> diff --git a/include/efi_loader.h b/include/efi_loader.h
>> index 72c83fd5033..779b8bde2e3 100644
>> --- a/include/efi_loader.h
>> +++ b/include/efi_loader.h
>> @@ -179,7 +179,7 @@ int efi_disk_create_partitions(efi_handle_t parent, struct blk_desc *desc,
>>                                const char *if_typename, int diskid,
>>                                const char *pdevname);
>>  /* Called by bootefi to make GOP (graphical) interface available */
>> -int efi_gop_register(void);
>> +efi_status_t efi_gop_register(void);
>>  /* Called by bootefi to make the network interface available */
>>  int efi_net_register(void);
>>  /* Called by bootefi to make the watchdog available */
>> diff --git a/lib/efi_loader/efi_gop.c b/lib/efi_loader/efi_gop.c
>> index 3caddd5f844..91b0b6a064b 100644
>> --- a/lib/efi_loader/efi_gop.c
>> +++ b/lib/efi_loader/efi_gop.c
>> @@ -125,8 +125,13 @@ efi_status_t EFIAPI gop_blt(struct efi_gop *this, void *buffer,
>>         return EFI_EXIT(EFI_SUCCESS);
>>  }
>>
>> -/* This gets called from do_bootefi_exec(). */
>> -int efi_gop_register(void)
>> +/*
>> + * Install graphical output protocol.
>> + *
>> + * If no supported video device exists this is not considered as an
>> + * error.
>> + */
> 
> This comment should be in the header file so people can see the API in
> one place.
> 
> It's unfortunate that U-Boot error codes get lost here. Perhaps it
> does not make sense to return them and have the caller report the
> error? I'm not sure what is best, but one symptom of the current
> approach is the use of printf() to report (and suppress) the error.

I understand why using log() is a better choice than printf(). We
already added a log category for the EFI subsystem without using it yet.

Do I get your idea right:

You would return an error code to the caller. And if the caller thinks
that the GOP protocol is not needed he should output a log message and
pass on.

As Alex has already picked this patch I would prefer to put such a
change into an extra patch.

Regards

Heinrich