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

[AKASHI Takahiro <takahiro.akashi@linaro.org>]

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