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.
Best regards
Heinrich
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
