[U-Boot] [PATCH v4 3/9] efi_driver: add name to driver binding protocol

[AKASHI Takahiro]

On Thu, Jan 17, 2019 at 07:58:17AM +0100, Heinrich Schuchardt wrote:
> On 1/17/19 6:33 AM, AKASHI Takahiro wrote:
> > You raised a couple of questions to me.
> > 
> > On Tue, Jan 15, 2019 at 04:41:08AM +0100, Heinrich Schuchardt wrote:
> >> On 1/15/19 3:55 AM, AKASHI Takahiro wrote:
> >>> This new field will be shown as a driver's name in "efitool drivers"
> >>> command.
> >>
> >> We can have drivers supplied by U-Boot
> > 
> > I assume that what you mention here is a UCLASS_EFI driver.
> > 
> > What's the problem is;
> > efi_add_driver() adds EFI_DRIVER_BINDING_PROTOCOL with
> > *efi_driver_binding_extended_protocol* interface, which is NOT compatible
> > with EFI_DRIVER_BINDING_PROTOCOL.
> > On the other hand, for example, in your efi_selftest_controller test
> > a test driver is set up by installing EFI_DRIVER_BINDING_PROTOCOL
> > with EFI_DRIVER_BINDING_PROTOCOL interface.
> > 
> > So we have no way to distinguish the two cases(handles) and cannot
> > deal with them properly.
> 
> Correct. It is allowable to add private fields to a protocol. EDK2 does
> the same. But we cannot make any assumptions about the private fields here.

I misunderstood something here, but anyhow,

> > 
> >> and drivers supplied by an EFI
> >> binary that we recently installed via the bootefi command.
> >>
> >> A driver installed via the bootefi command will not have allocated
> >> memory for the extra fields.
> > 
> > There is no good example of driver of such kind.
> > I don't know how we can retrieve a meaningful "driver name."
> 
> The UEFI spec does not foresee any name field.

Edk2's shell does get a driver's name by using COMPONENT_NAME2_PROTOCOL,
and we'd better do the same way.
So "<NULL>" will be printed with my patch for now.

-Takahiro Akashi

> The interesting information is: on which handle did the driver install
> which protocol.
> 
> This information could be gathered by looping over all protocols on all
> handles and looking for an OpenProtocolInformation where the driver is
> the controller.
> 
> For me it would be fine if we leave that to a later patch.
> 
> Best regards
> 
> Heinrich
> 
> > 
> >> So you cannot use the name field in your "efitool drivers" command.
> > 
> > Any suggestion?
> > 
> > Thanks,
> > -Takahiro Akashi
> > 
> >>
> >> Best regards
> >>
> >> Heinrich
> >>
> >>>
> >>> Signed-off-by: AKASHI Takahiro <takahiro.akashi at linaro.org>
> >>> ---
> >>>  include/efi_driver.h        | 1 +
> >>>  lib/efi_driver/efi_uclass.c | 1 +
> >>>  2 files changed, 2 insertions(+)
> >>>
> >>> diff --git a/include/efi_driver.h b/include/efi_driver.h
> >>> index 840483a416a4..ee8867816094 100644
> >>> --- a/include/efi_driver.h
> >>> +++ b/include/efi_driver.h
> >>> @@ -34,6 +34,7 @@ struct efi_driver_ops {
> >>>   * This structure adds internal fields to the driver binding protocol.
> >>>   */
> >>>  struct efi_driver_binding_extended_protocol {
> >>> +	const char *name;
> >>>  	struct efi_driver_binding_protocol bp;
> >>>  	const struct efi_driver_ops *ops;
> >>>  };
> >>> diff --git a/lib/efi_driver/efi_uclass.c b/lib/efi_driver/efi_uclass.c
> >>> index bb86ffd399c3..8bbaa02d490e 100644
> >>> --- a/lib/efi_driver/efi_uclass.c
> >>> +++ b/lib/efi_driver/efi_uclass.c
> >>> @@ -271,6 +271,7 @@ static efi_status_t efi_add_driver(struct driver *drv)
> >>>  	bp->bp.stop = efi_uc_stop;
> >>>  	bp->bp.version = 0xffffffff;
> >>>  	bp->ops = drv->ops;
> >>> +	bp->name = drv->name;
> >>>  
> >>>  	ret = efi_create_handle(&bp->bp.driver_binding_handle);
> >>>  	if (ret != EFI_SUCCESS) {
> >>>
> >>
> > 
>