[U-Boot] [PATCH v2 2/3] efi_loader: enumerate disk devices every time

AKASHI Takahiro
2019-01-25T09:18:18Z

On Fri, Jan 25, 2019 at 09:52:31AM +0100, Alexander Graf wrote:
>
>
> On 25.01.19 09:27, AKASHI Takahiro wrote:
> > Alex,
> >
> > On Wed, Jan 23, 2019 at 10:51:29AM +0100, Alexander Graf wrote:
> >> On 01/22/2019 08:39 PM, Simon Glass wrote:
> >>> Hi Alex,
> >>>
> >>> On Tue, 22 Jan 2019 at 22:08, Alexander Graf <agraf@suse.de> wrote:
> >>>>
> >>>>
> >>>> On 22.01.19 09:29, AKASHI Takahiro wrote:
> >>>>> Alex, Simon,
> >>>>>
> >>>>> Apologies for my slow response on this matter,
> >>>>>
> >>>>> On Fri, Jan 11, 2019 at 08:57:05AM +0100, Alexander Graf wrote:
> >>>>>>
> >>>>>> On 11.01.19 05:29, AKASHI Takahiro wrote:
> >>>>>>> Alex, Heinrich and Simon,
> >>>>>>>
> >>>>>>> Thank you for your comments, they are all valuable but also make me
> >>>>>>> confused as different people have different requirements :)
> >>>>>>> I'm not sure that all of us share the same *ultimate* goal here.
> >>>>>> The shared ultimate goal is to "merge" (as Simon put it) dm and efi objects.
> >>>>> I don't still understand what "merge" means very well.
> >>>> It basically means that "struct efi_object" moves into "struct udevice".
> >>>> Every udevice instance of type UCLASS_BLK would expose the block and
> >>>> device_path protocols.
> >>>>
> >>>> This will be a slightly bigger rework, but eventually allows us to
> >>>> basically get rid of efi_init_obj_list() I think.
> >>> I envisaged something like:
> >>>
> >>> - EFI objects have their own UCLASS_EFI uclass
> >>
> >> ... and then we need to create our own sub object model around the
> >> UCLASS_EFI devices again. I' not convinced that's a great idea yet :). I
> >> really see little reason not to just expose every dm device as EFI handle.
> >> Things would plug in quite naturally I think.
> >
> > You said that the ultimate goal is to remove all efi_object data.
> > Do you think that all the existing efi_object can be mapped to
> > one of existing u-boot uclass devices?
> >
> > If so, what would be an real entity of a UEFI handle?
> > struct udevice *?
> >
> > But Simon seems not to agree to adding any UEFI-specific members
> > in struct udevice.
>
> I think we'll have to experiment with both approaches. I personally
> would like to have struct udevice * be the UEFI handle, yes.
>
> >
> >> But either way, someone would need to sit down and prototype things to be
> >> sure.
> >>
> >
> > The most simplest prototype would include
> > * event mechanism (just registration and execution of hook/handler)
> > event: udevice creation (and deletion)
> > * efi_disk hook for udevice(UCLASS_BLK) creation
> > * modified block device's enumeration code, say, scsi_scan(),
> > to add an event hook at udevice creation
> > * removing efi_disk_register() from efi_init_obj_list()
> > * Optionally(?) UCLASS_PARTITION
> > (Partition udevices would be created in part_init().)
>
> Almost.
>
> The simplest prototype would be to add a struct efi_object into struct
> udevice. Then whenever we're looping over efi_obj_list in the code, we
> additionally loop over all udevices to find the handle.

Ah, yes. You're going further :)

> Then, we could slowly give the uclasses explicit knowledge of uefi
> protocols. So most of the logic of efi_disk_register() would move into
> (or get called by) drivers/block/blk-uclass.c:blk_create_device().

Via event? Otherwise, we cannot decouple u-boot and UEFI world.

> Instead of creating diskobj and adding calling efi_add_handle(), we
> could then just use existing data structure from the udevice (and its
> platdata).

I don't have good confidence that we can remove struct efi_disk_obj,
at least, for the time being as some of its members are quite UEFI-specific.

>
> Does this make sense? Less events, more implicity :).

I'll go for it.

Thanks,
-Takahiro Akashi

> Alex