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

[Simon Glass]

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.

Regards,
Simon