Re: [RFC 14/22] dm: blk: call efi's device-probe hook

AKASHI Takahiro
2021-10-11T03:15:36Z

Hi Simon,

On Sun, Oct 10, 2021 at 08:14:23AM -0600, Simon Glass wrote:
> Hi Takahiro,
>
> On Thu, 30 Sept 2021 at 23:04, AKASHI Takahiro
> <takahiro.akashi@linaro.org> wrote:
> >
> > Adding this callback function, efi_disk_create() in block devices's
> > post_probe hook will allows for automatically creating efi_disk objects
> > per block device.
> >
> > This will end up not only eliminating efi_disk_register() called in UEFI
> > initialization, but also enabling detections of new block devices even
> > after the initialization.
> >
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > ---
> > drivers/block/blk-uclass.c | 10 ++++++++++
> > 1 file changed, 10 insertions(+)
>
> This is where events come in. We need a way to notify things when
> devices go through different stages, etc.

I favor your idea of event notification to decouple subsystems from
the core block layer.

> I am thinking of:
>
> enum dm_event_t {
> DMEVT_PROBE,
> DMEVT_POST_PROBE,
> ...
> };
>
> struct dm_event {
> enum dm_event_t type;
> union {
> // add data for different event types in here
> } data;
> }
>
> int (*dm_handler_t)(void *ctx, struct dm_event *event);
>
> int dm_register(enum dm_event_t evtype, dm_handler_t func, void *ctx)
>
> int dm_notify(struct udevice *dev, enum dm_event_t type, void *data);
>
> Then the code below becomes:
>
> dm_notify(struct udevice *dev, DMEVT_POST_PROBE, NULL);
>
> the implementation of which will call the handler.
>
> If you like I could create an impl of the above as a separate patch
> for discussion.

Yes, please.
I'm willing to rebase my code on top of your patch.

-Takahiro Akashi

> >
> > diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c
> > index 8fbec8779e1e..ce45cf0a8768 100644
> > --- a/drivers/block/blk-uclass.c
> > +++ b/drivers/block/blk-uclass.c
> > @@ -9,6 +9,7 @@
> > #include <common.h>
> > #include <blk.h>
> > #include <dm.h>
> > +#include <efi_loader.h>
> > #include <log.h>
> > #include <malloc.h>
> > #include <part.h>
> > @@ -827,6 +828,11 @@ int blk_create_partitions(struct udevice *parent)
> >
> > static int blk_post_probe(struct udevice *dev)
> > {
> > + if (CONFIG_IS_ENABLED(EFI_LOADER)) {
> > + if (efi_disk_create(dev))
> > + debug("*** efi_post_probe_device failed\n");
> > + }
> > +
> > if (IS_ENABLED(CONFIG_PARTITIONS) &&
> > IS_ENABLED(CONFIG_HAVE_BLOCK_DEVICE)) {
> > struct blk_desc *desc = dev_get_uclass_plat(dev);
> > @@ -843,6 +849,10 @@ static int blk_post_probe(struct udevice *dev)
> >
> > static int blk_part_post_probe(struct udevice *dev)
> > {
> > + if (CONFIG_IS_ENABLED(EFI_LOADER)) {
> > + if (efi_disk_create(dev))
> > + debug("*** efi_post_probe_device failed\n");
> > + }
> > /*
> > * TODO:
> > * If we call blk_creat_partitions() here, it would allow for
> > --
> > 2.33.0
> >
>
> Regards,
> Simon