Re: [RFC PATCH] RFC: Sketch of dm event notification

Simon Glass
2021-11-25T00:15:53Z

Hi Takahiro,

On Tue, 16 Nov 2021 at 20:03, AKASHI Takahiro
<takahiro.akashi@linaro.org> wrote:
>
> On Tue, Nov 16, 2021 at 07:48:59PM -0700, Simon Glass wrote:
> > Hi Takahiro,
> >
> > On Tue, 16 Nov 2021 at 19:31, AKASHI Takahiro
> > <takahiro.akashi@linaro.org> wrote:
> > >
> > > Simon,
> > >
> > > On Mon, Nov 01, 2021 at 03:41:50PM +0900, AKASHI Takahiro wrote:
> > > > Hello Simon,
> > > >
> > > > On Sun, Oct 31, 2021 at 09:07:01PM -0600, Simon Glass wrote:
> > > > > This is a patch for Takahiro to look at, just for illustration. Not ready
> > > > > for review.
> > > >
> > > > Thank you for posting the draft.
> > > > At a first glance, it looks good and I don't see any specific issue
> > > > with your implementation.
> > >
> > > I said OK and functionally it should work well, but I now have
> > > some concerns:
> > > 1) In my current implementation, I use post_probe/pre_remove hooks
> > > of BLK device to invoke efi callback functions. In this sense,
> > > event(POST_PROBE/ PRE_REMOVE) seems to be a duplicated feature
> > > in some way.
> >
> > We should not be calling EFI functions from a BLK devince, unless it
> > is an EFI block device, i.e. we should provide hooks for EFI or any
> > other interested party to use.
>
> Even for now, part_init() is called in post_probe() (and
> blk_create_partitions(), which is to create another type of
> uclass device, in my patch as well).
> So I'm not sure what functions can and should be called
> and what not in those hooks.
>
> Do you want to convert part_init() to an event subscriber
> along with my efi hooks?

You could do. From my reading of the code it makes sense to do that.

Regards,
Simon

>
> -Takahiro Akashi
>
>
> > > 2) For the rest of uclass devices which don't utilise events yet,
> > > device_notify() is nothing but an overhead as it always tries to
> > > go through a list of event hooks.
> >
> > Yes that's true, but it can be optimised to avoid useless
> > searches/calls, since subscribers need to specify an even they are
> > interested in.
> >
> > >
> > > Event notification can be more than just a dm feature, but ...
> > > What's your thought here?
> >
> > Yes, more than just a DM feature. For example I think it could help
> > with the initcalls we have in board_f and board_r.
> >
> > Regards,
> > Simon
> >
> > >
> > > -Takahiro Akashi
> > >
> > > > Since my code has already added DM_TAG support, I'm looking forward for
> > > > getting your final patch.
> > > >
> > > > The only remaining issue is *modeling* partitions :)
> > > >
> > > > -Takahiro Akashi
> > > >
> > > > > To run the test:
> > > > >
> > > > > ./u-boot -T -c "ut common test_event_probe"
> > > > >
> > > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > > > ---
> >
> > [..]
> >
> > Regards,
> > Simon