| Message ID | 20211001050228.55183-28-takahiro.akashi@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | efi_loader: more tightly integrate UEFI disks to device model | expand |
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 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. > > 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
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
Hi Takahiro, On Sun, 10 Oct 2021 at 21:15, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote: > > 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. OK I will give it a crack, hopefully around the end of the week. Regards, Simon
Hi Takahiro, On Mon, 11 Oct 2021 at 08:54, Simon Glass <sjg@chromium.org> wrote: > > Hi Takahiro, > > On Sun, 10 Oct 2021 at 21:15, AKASHI Takahiro > <takahiro.akashi@linaro.org> wrote: > > > > 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. > > OK I will give it a crack, hopefully around the end of the week. Well that didn't happen as the two things ahead of it expanded. I'll send a patch with a sketch soon, for you to look at. Regards, Simon
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
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(+) -- 2.33.0