From mboxrd@z Thu Jan 1 00:00:00 1970 From: Heinrich Schuchardt Date: Fri, 15 Sep 2017 08:48:34 +0200 Subject: [U-Boot] [PATCH 15/23] efi_loader: implement ConnectController In-Reply-To: References: <20170826225110.7381-1-xypron.glpk@gmx.de> <20170826225328.7550-1-xypron.glpk@gmx.de> <20170826225328.7550-6-xypron.glpk@gmx.de> Message-ID: <15a8b074-4287-e97a-d977-835b3da93445@gmx.de> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 08/31/2017 02:52 PM, Simon Glass wrote: > On 27 August 2017 at 06:53, Heinrich Schuchardt wrote: >> Signed-off-by: Heinrich Schuchardt >> --- >> 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 > > 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. This is what checkpatch wants to see. Your suggestion leads to ERROR: need consistent spacing around '*' (ctx:WxV) #25: FILE: include/efi_api.h:616: + efi_status_t (EFIAPI *supported)( So I prefer not change this before checkpatch.pl is not giving a different result. > >> + 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? The GUIDs are defined in the UEFI spec. So maybe we should just put a comment above all of these. Regards Heinrich > >> 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? >