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

[Alexander Graf]

On 03/12/2018 01:07 PM, Alexander Graf wrote:
> 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 at 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?

I've just applied it with those prints converted to debug() calls now.


Alex