On Mon, Jul 24, 2017 at 1:11 PM, Heinrich Schuchardt <xypron.g...@gmx.de> wrote: > On 07/24/2017 11:48 AM, Rob Clark wrote: >> On Tue, Jul 11, 2017 at 4:06 PM, Heinrich Schuchardt <xypron.g...@gmx.de> >> wrote: >>> efi_open_protocol was implemented to call a protocol specific open >>> function to retrieve the protocol interface. >>> >>> The UEFI specification does not know of such a function. >>> >>> It is not possible to implement InstallProtocolInterface with the >>> current design. >>> >>> With the patch the protocol interface itself is stored in the list >>> of installed protocols of an efi_object instead of an open function. >>> >>> Signed-off-by: Heinrich Schuchardt <xypron.g...@gmx.de> >>> --- >>> v2 >>> Remove implementation of efi_return_handle. >>> --- >>> cmd/bootefi.c | 14 +++----------- >>> include/efi_loader.h | 17 ++--------------- >>> lib/efi_loader/efi_boottime.c | 18 ++++++++++++++---- >>> lib/efi_loader/efi_disk.c | 29 +++-------------------------- >>> lib/efi_loader/efi_gop.c | 2 +- >>> lib/efi_loader/efi_image_loader.c | 8 -------- >>> lib/efi_loader/efi_net.c | 30 +++--------------------------- >>> 7 files changed, 26 insertions(+), 92 deletions(-) >>> >>> diff --git a/cmd/bootefi.c b/cmd/bootefi.c >>> index 7ddeead671..7cf0129a18 100644 >>> --- a/cmd/bootefi.c >>> +++ b/cmd/bootefi.c >>> @@ -54,14 +54,6 @@ static struct efi_device_path_file_path >>> bootefi_device_path[] = { >>> } >>> }; >>> >>> -static efi_status_t EFIAPI bootefi_open_dp(void *handle, efi_guid_t >>> *protocol, >>> - void **protocol_interface, void *agent_handle, >>> - void *controller_handle, uint32_t attributes) >>> -{ >>> - *protocol_interface = bootefi_device_path; >>> - return EFI_SUCCESS; >>> -} >>> - >>> /* The EFI loaded_image interface for the image executed via "bootefi" */ >>> static struct efi_loaded_image loaded_image_info = { >>> .device_handle = bootefi_device_path, >>> @@ -78,7 +70,7 @@ static struct efi_object loaded_image_info_obj = { >>> * return handle which points to loaded_image_info >>> */ >>> .guid = &efi_guid_loaded_image, >>> - .open = &efi_return_handle, >>> + .protocol_interface = &loaded_image_info, >> >> btw, I probably should have looked at this patchset earlier.. in >> general, I like it, since it removes a lot of boilerplate. But there >> are some cases where ->open() allows deferred allocation. For >> example, I'm working on efi_file and simple-file-system-protocol >> implementation. A file object can have a device-path accessed by >> opening device-path protocol. 99% of the time this isn't used, so the >> ->open() approach lets me defer constructing a file's device-path. >> >> How would you feel if I re-introduced ->open() as an optional thing >> (where the normal case if open is NULL would be to just return >> protocol_interface)? >> >> BR, >> -R >> > > > Hello Rob, > > a big gap we currently have in the EFI implementation is the missing > handling of lock entries in OpenProtocol which have to be stored per > protocol as a list of EFI_OPEN_PROTOCOL_INFORMATION_ENTRYs. > > Could you, please, elaborate how OpenProtocolInformation, > ConnectController, and DisconnectController shall work together with > your suggested open() approach. > > We need be able to iterate efficiently over the > EFI_OPEN_PROTOCOL_INFORMATION_ENTRYs in these functions. > > I fear your proposal will make the code overly complicated here. >
I guess I'm not too sure what you see the issue would be, but I'm also unfamiliar with what needs to be implemented. The patch I sent just (on first OpenProtocol()) calls ->open() to resolve the actual protocol interface. Afaiu, ->OpenProtocol() always have to be called first. And ofc it is completely optional. It avoids having to up-front create the simple-file-system, and avoids having to construct a device-path for each efi_file object (when it is almost never accessed). BR, -R _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot