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

On 03/03/2018 03:28 PM, Heinrich Schuchardt wrote:

All initialization routines should return a status code instead of a boolean.

Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de

v3 no change 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 72c83fd503..779b8bde2e 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 3caddd5f84..91b0b6a064 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.
• */

+efi_status_t efi_gop_register(void) { struct efi_gop_obj *gopobj; u32 bpix, col, row; @@ -136,12 +141,15 @@ int efi_gop_register(void)

#ifdef CONFIG_DM_VIDEO struct udevice *vdev;

• struct video_priv *priv;

  /* We only support a single video output device for now */

• if (uclass_first_device(UCLASS_VIDEO, &vdev) || !vdev)

• return -1;
  

• if (uclass_first_device(UCLASS_VIDEO, &vdev) || !vdev) {

• printf("WARNING: No video device\n");
  

I don't think we should emit a warning, just because someone enabled support for DM_VIDEO but doesn't have any device backing it. Imagine for example a cross-board U-Boot binary that adapts its device tree based on the board it finds. On some, it may use video output, on others it might not. Then we would emit a warning here for no good reason.

• return EFI_SUCCESS;
  

• }

• struct video_priv *priv = dev_get_uclass_priv(vdev);

• priv = dev_get_uclass_priv(vdev); bpix = priv->bpix; col = video_get_xsize(vdev); row = video_get_ysize(vdev);

@@ -170,13 +178,14 @@ int efi_gop_register(void) break; default: /* So far, we only work in 16 or 32 bit mode */

• return -1;
  

• printf("WARNING: Unsupported video mode\n");
  

Same here. Maybe convert into debug() prints?

Alex

• return EFI_SUCCESS;
  

  }

  gopobj = calloc(1, sizeof(*gopobj)); if (!gopobj) { printf("ERROR: Out of memory\n");

• return 1;
  

• return EFI_OUT_OF_RESOURCES;
  

  }

  /* Hook up to the device list */

@@ -186,8 +195,8 @@ int efi_gop_register(void) ret = efi_add_protocol(gopobj->parent.handle, &efi_gop_guid, &gopobj->ops); if (ret != EFI_SUCCESS) {

• printf("ERROR: Out of memory\n");
  

• return 1;
  

• printf("ERROR: Failure adding gop protocol\n");
  

• return ret;
  

  } gopobj->ops.query_mode = gop_query_mode; gopobj->ops.set_mode = gop_set_mode;

@@ -199,10 +208,11 @@ int efi_gop_register(void) gopobj->mode.info_size = sizeof(gopobj->info);

#ifdef CONFIG_DM_VIDEO

• if (bpix == VIDEO_BPP32) {

• if (bpix == VIDEO_BPP32) #else

• if (bpix == LCD_COLOR32) {

• if (bpix == LCD_COLOR32) #endif
• { /* With 32bit color space we can directly expose the fb */ gopobj->mode.fb_base = fb_base; gopobj->mode.fb_size = fb_size;

@@ -217,5 +227,5 @@ int efi_gop_register(void) gopobj->bpix = bpix; gopobj->fb = fb;

• return 0;

• return EFI_SUCCESS; }