[U-Boot] [PATCH 15/23] efi_loader: implement ConnectController

[Simon Glass]

On 27 August 2017 at 06:53, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
> ---
>  include/efi_api.h             |  22 ++++++++
>  include/efi_loader.h          |   1 +
>  lib/efi_loader/efi_boottime.c | 119 +++++++++++++++++++++++++++++++++++++++++-
>  3 files changed, 141 insertions(+), 1 deletion(-)
>

Reviewed-by: Simon Glass <sjg at chromium.org>

Please see below.

> diff --git a/include/efi_api.h b/include/efi_api.h
> index 8efc8dfab8..b2838125d7 100644
> --- a/include/efi_api.h
> +++ b/include/efi_api.h
> @@ -609,4 +609,26 @@ struct efi_pxe {
>         struct efi_pxe_mode *mode;
>  };
>
> +#define EFI_DRIVER_BINDING_PROTOCOL_GUID \
> +       EFI_GUID(0x18a031ab, 0xb443, 0x4d1a,\
> +                0xa5, 0xc0, 0x0c, 0x09, 0x26, 0x1e, 0x9f, 0x71)
> +struct efi_driver_binding_protocol {
> +       efi_status_t (EFIAPI * supported)(

I think the * should be up against 'supported'. Similarly below.

> +                       struct efi_driver_binding_protocol *this,
> +                       efi_handle_t controller_handle,
> +                       struct efi_device_path *remaining_device_path);
> +       efi_status_t (EFIAPI * start)(
> +                       struct efi_driver_binding_protocol *this,
> +                       efi_handle_t controller_handle,
> +                       struct efi_device_path *remaining_device_path);
> +       efi_status_t (EFIAPI * stop)(
> +                       struct efi_driver_binding_protocol *this,
> +                       efi_handle_t controller_handle,
> +                       UINTN number_of_children,
> +                       efi_handle_t child_handle_buffer);

These should have function comments.

> +       u32 version;
> +       efi_handle_t image_handle;
> +       efi_handle_t driver_binding_handle;
> +};
> +
>  #endif
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index 9c68246c7c..f9f33e1d01 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -74,6 +74,7 @@ extern const struct efi_device_path_to_text_protocol efi_device_path_to_text;
>
>  extern const efi_guid_t efi_guid_console_control;
>  extern const efi_guid_t efi_guid_device_path;
> +extern const efi_guid_t efi_guid_driver_binding_protocol;

comment for this?

>  extern const efi_guid_t efi_guid_loaded_image;
>  extern const efi_guid_t efi_guid_device_path_to_text_protocol;
>
> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
> index 5a73ea5cd0..1069da7d79 100644
> --- a/lib/efi_loader/efi_boottime.c
> +++ b/lib/efi_loader/efi_boottime.c
> @@ -18,6 +18,14 @@
>
>  DECLARE_GLOBAL_DATA_PTR;
>
> +static efi_status_t EFIAPI efi_locate_protocol(const efi_guid_t *protocol,
> +                                              void *registration,
> +                                              void **protocol_interface);
> +static efi_status_t EFIAPI efi_locate_handle_buffer(
> +                       enum efi_locate_search_type search_type,
> +                       const efi_guid_t *protocol, void *search_key,
> +                       unsigned long *no_handles, efi_handle_t **buffer);
> +

Is this a forward decl? Can you reorder (in a separate patch) to avoid it?