Simon,
On Mon, Jan 28, 2019 at 05:46:21PM -0700, Simon Glass wrote:
Hi,
On Mon, 28 Jan 2019 at 01:55, AKASHI Takahiro takahiro.akashi@linaro.org wrote:
On Fri, Jan 25, 2019 at 10:31:20AM +0100, Alexander Graf wrote:
On 25.01.19 10:18, AKASHI Takahiro wrote:
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.
For a prototype, just make it explicit and see how far that gets us.
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.
Maybe we can move them into struct blk_desc? It's a matter of experimenting I guess.
Does this make sense? Less events, more implicity :).
I'll go for it.
Thanks a lot :). Feel free to pick an easier target for starters too if you prefer.
Prototyping is done :) Since it was so easy and simple, now I'm thinking of implementing UCLASS_PARTITION. But it is not so straightforward as I expected, and it won't bring us lots of advantages. (I think that blk_desc should also support a partition in its own.)
blk_desc is in UCLASS_BLK. So we already support partitions within blk_desc. Can you expand a bit on what you mean?
This is partly because "efi_disk_obj" structure is embedded in blk_desc for now. So for UCLASS_PARTITION, we need a dummy blk_desc.
Once it gets working, may I send out a patch?
Yes indeed.
Shortly. We will continue to discuss after my patch is released.
Thanks, -Takahiro Akashi
Regards, Simon
-Takahiro Akashi
Alex