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

2021-12-29 Thread Simon Glass
Hi Takahiro,

On Tue, 7 Dec 2021 at 08:07, Simon Glass  wrote:
>
> Hi Tahahiro,
>
> On Wed, 24 Nov 2021 at 19:57, AKASHI Takahiro
>  wrote:
> >
> > Hi Simon,
> >
> > On Wed, Nov 24, 2021 at 05:12:45PM -0700, Simon Glass wrote:
> > > Hi Takahiro,
> > >
> > > On Tue, 16 Nov 2021 at 20:03, AKASHI Takahiro
> > >  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
> > > > >  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.
> >
> > Ok, but I will put this change on hold for now.
> >
> > One of problems with the proposed framework of event notification
> > is that we cannot deterministically specify the order of callback
> > functions when we want to add callbacks for a number of subsystems
> > to a particular event.
> > For instance, we have callbacks for efi_disk creations as well as
> > partitions (part_init() & blk_create_partitions()) at POST_PROBE of
> > BLK devices, they should (logically) be called in this order but
> > currently no definite guarantee.
> > If we utilise a callback to create efi_disk's for "partition" devices
> > as well, this situation can get more complicated.
> > # Why? If the order of invocations is the other way around, efi_disks
> > # for partitions might be created before an efi_disk for a whole disk.
>
> Yes I was wondering if that would cause problems.
>
> We also need to think about discoverability. It needs to be *very
> easy* to see a list of callbacks for an event a priori and we don't
> want debugging to be a pain.
>
> I'll take a look at adding priority and some commands to list the callbacks.

I added the commands but not the priority, yet. I will see if I get
some comments first.

Regards,
Simon


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

2021-12-07 Thread Simon Glass
Hi Tahahiro,

On Wed, 24 Nov 2021 at 19:57, AKASHI Takahiro
 wrote:
>
> Hi Simon,
>
> On Wed, Nov 24, 2021 at 05:12:45PM -0700, Simon Glass wrote:
> > Hi Takahiro,
> >
> > On Tue, 16 Nov 2021 at 20:03, AKASHI Takahiro
> >  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
> > > >  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.
>
> Ok, but I will put this change on hold for now.
>
> One of problems with the proposed framework of event notification
> is that we cannot deterministically specify the order of callback
> functions when we want to add callbacks for a number of subsystems
> to a particular event.
> For instance, we have callbacks for efi_disk creations as well as
> partitions (part_init() & blk_create_partitions()) at POST_PROBE of
> BLK devices, they should (logically) be called in this order but
> currently no definite guarantee.
> If we utilise a callback to create efi_disk's for "partition" devices
> as well, this situation can get more complicated.
> # Why? If the order of invocations is the other way around, efi_disks
> # for partitions might be created before an efi_disk for a whole disk.

Yes I was wondering if that would cause problems.

We also need to think about discoverability. It needs to be *very
easy* to see a list of callbacks for an event a priori and we don't
want debugging to be a pain.

I'll take a look at adding priority and some commands to list the callbacks.

Regards,
Simon


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

2021-11-24 Thread AKASHI Takahiro
Hi Simon,

On Wed, Nov 24, 2021 at 05:12:45PM -0700, Simon Glass wrote:
> Hi Takahiro,
> 
> On Tue, 16 Nov 2021 at 20:03, AKASHI Takahiro
>  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
> > >  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.

Ok, but I will put this change on hold for now.

One of problems with the proposed framework of event notification
is that we cannot deterministically specify the order of callback
functions when we want to add callbacks for a number of subsystems
to a particular event.
For instance, we have callbacks for efi_disk creations as well as
partitions (part_init() & blk_create_partitions()) at POST_PROBE of
BLK devices, they should (logically) be called in this order but
currently no definite guarantee.
If we utilise a callback to create efi_disk's for "partition" devices
as well, this situation can get more complicated.
# Why? If the order of invocations is the other way around, efi_disks
# for partitions might be created before an efi_disk for a whole disk.

-Takahiro Akashi

> 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 
> > > > > > ---
> > >
> > > [..]
> > >
> > > Regards,
> > > Simon


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

2021-11-24 Thread Simon Glass
Hi Takahiro,

On Tue, 16 Nov 2021 at 20:03, AKASHI Takahiro
 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
> >  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 
> > > > > ---
> >
> > [..]
> >
> > Regards,
> > Simon


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

2021-11-16 Thread AKASHI Takahiro
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
>  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?

-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 
> > > > ---
> 
> [..]
> 
> Regards,
> Simon


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

2021-11-16 Thread Simon Glass
Hi Takahiro,

On Tue, 16 Nov 2021 at 19:31, AKASHI Takahiro
 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.

> 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 
> > > ---

[..]

Regards,
Simon


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

2021-11-16 Thread AKASHI Takahiro
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.
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.

Event notification can be more than just a dm feature, but ...
What's your thought here?

-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 
> > ---
> > 
> >  common/Kconfig|  11 
> >  common/Makefile   |   2 +
> >  common/board_f.c  |   2 +
> >  common/event.c| 103 ++
> >  common/log.c  |   1 +
> >  drivers/core/device.c |  14 
> >  include/asm-generic/global_data.h |   3 +
> >  include/event.h   | 103 ++
> >  include/event_internal.h  |  34 ++
> >  include/log.h |   2 +
> >  test/common/Makefile  |   1 +
> >  test/common/event.c   |  85 
> >  test/test-main.c  |   5 ++
> >  13 files changed, 366 insertions(+)
> >  create mode 100644 common/event.c
> >  create mode 100644 include/event.h
> >  create mode 100644 include/event_internal.h
> >  create mode 100644 test/common/event.c
> > 
> > diff --git a/common/Kconfig b/common/Kconfig
> > index d6f77ab7b9c..54d0adaa8d5 100644
> > --- a/common/Kconfig
> > +++ b/common/Kconfig
> > @@ -484,6 +484,17 @@ config DISPLAY_BOARDINFO_LATE
> >  
> >  menu "Start-up hooks"
> >  
> > +config EVENT
> > +   bool "General-purpose event-handling mechanism"
> > +   default y if SANDBOX
> > +   help
> > + This enables sending and processing of events, to allow interested
> > + parties to be alerted when something happens. This is an attempt to
> > + step the flow of weak functions, hooks, functions in board_f.c
> > + and board_r.c and the Kconfig options below.
> > +
> > + See doc/develop/event.rst for more information.
> > +
> >  config ARCH_EARLY_INIT_R
> > bool "Call arch-specific init soon after relocation"
> > help
> > diff --git a/common/Makefile b/common/Makefile
> > index e7839027b6c..1b4601ac13f 100644
> > --- a/common/Makefile
> > +++ b/common/Makefile
> > @@ -101,6 +101,8 @@ obj-y += malloc_simple.o
> >  endif
> >  endif
> >  
> > +obj-$(CONFIG_EVENT) += event.o
> > +
> >  obj-y += image.o image-board.o
> >  obj-$(CONFIG_$(SPL_TPL_)HASH) += hash.o
> >  obj-$(CONFIG_ANDROID_AB) += android_ab.o
> > diff --git a/common/board_f.c b/common/board_f.c
> > index 3dc0eaa59c5..faf262d564c 100644
> > --- a/common/board_f.c
> > +++ b/common/board_f.c
> > @@ -19,6 +19,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >  #include 
> >  #include 
> >  #include 
> > @@ -820,6 +821,7 @@ static const init_fnc_t init_sequence_f[] = {
> > initf_malloc,
> > log_init,
> > initf_bootstage,/* uses its own timer, so does not need DM */
> > +   event_init,
> >  #ifdef CONFIG_BLOBLIST
> > bloblist_init,
> >  #endif
> > diff --git a/common/event.c b/common/event.c
> > new file mode 100644
> > index 000..428628da44d
> > --- /dev/null
> > +++ b/common/event.c
> > @@ -0,0 +1,103 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Events provide a general-purpose way to react to / subscribe to changes
> > + * within U-Boot
> > + *
> > + * Copyright 2021 Google LLC
> > + * Written by Simon Glass 
> > + */
> > +
> > +#define LOG_CATEGORY   LOGC_EVENT
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +DECLARE_GLOBAL_DATA_PTR;
> > +
> > +static void spy_free(struct event_spy *spy)
> > +{
> > +   list_del(>sibling_node);
> > +}
> > +
> > +int event_register(const char *id, enum event_t type, event_handler_t 
> > func, void *ctx)
> > +{
> > +   struct event_state *state = gd->event_state;
> > +   struct event_spy *spy;
> > +
> > +   spy = malloc(sizeof(*spy));
> > +   if (!spy)
> > +   return log_msg_ret("alloc", -ENOMEM);
> > +
> > +   spy->id = id;
> > +   

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

2021-11-01 Thread AKASHI Takahiro
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.
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 
> ---
> 
>  common/Kconfig|  11 
>  common/Makefile   |   2 +
>  common/board_f.c  |   2 +
>  common/event.c| 103 ++
>  common/log.c  |   1 +
>  drivers/core/device.c |  14 
>  include/asm-generic/global_data.h |   3 +
>  include/event.h   | 103 ++
>  include/event_internal.h  |  34 ++
>  include/log.h |   2 +
>  test/common/Makefile  |   1 +
>  test/common/event.c   |  85 
>  test/test-main.c  |   5 ++
>  13 files changed, 366 insertions(+)
>  create mode 100644 common/event.c
>  create mode 100644 include/event.h
>  create mode 100644 include/event_internal.h
>  create mode 100644 test/common/event.c
> 
> diff --git a/common/Kconfig b/common/Kconfig
> index d6f77ab7b9c..54d0adaa8d5 100644
> --- a/common/Kconfig
> +++ b/common/Kconfig
> @@ -484,6 +484,17 @@ config DISPLAY_BOARDINFO_LATE
>  
>  menu "Start-up hooks"
>  
> +config EVENT
> + bool "General-purpose event-handling mechanism"
> + default y if SANDBOX
> + help
> +   This enables sending and processing of events, to allow interested
> +   parties to be alerted when something happens. This is an attempt to
> +   step the flow of weak functions, hooks, functions in board_f.c
> +   and board_r.c and the Kconfig options below.
> +
> +   See doc/develop/event.rst for more information.
> +
>  config ARCH_EARLY_INIT_R
>   bool "Call arch-specific init soon after relocation"
>   help
> diff --git a/common/Makefile b/common/Makefile
> index e7839027b6c..1b4601ac13f 100644
> --- a/common/Makefile
> +++ b/common/Makefile
> @@ -101,6 +101,8 @@ obj-y += malloc_simple.o
>  endif
>  endif
>  
> +obj-$(CONFIG_EVENT) += event.o
> +
>  obj-y += image.o image-board.o
>  obj-$(CONFIG_$(SPL_TPL_)HASH) += hash.o
>  obj-$(CONFIG_ANDROID_AB) += android_ab.o
> diff --git a/common/board_f.c b/common/board_f.c
> index 3dc0eaa59c5..faf262d564c 100644
> --- a/common/board_f.c
> +++ b/common/board_f.c
> @@ -19,6 +19,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -820,6 +821,7 @@ static const init_fnc_t init_sequence_f[] = {
>   initf_malloc,
>   log_init,
>   initf_bootstage,/* uses its own timer, so does not need DM */
> + event_init,
>  #ifdef CONFIG_BLOBLIST
>   bloblist_init,
>  #endif
> diff --git a/common/event.c b/common/event.c
> new file mode 100644
> index 000..428628da44d
> --- /dev/null
> +++ b/common/event.c
> @@ -0,0 +1,103 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Events provide a general-purpose way to react to / subscribe to changes
> + * within U-Boot
> + *
> + * Copyright 2021 Google LLC
> + * Written by Simon Glass 
> + */
> +
> +#define LOG_CATEGORY LOGC_EVENT
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +static void spy_free(struct event_spy *spy)
> +{
> + list_del(>sibling_node);
> +}
> +
> +int event_register(const char *id, enum event_t type, event_handler_t func, 
> void *ctx)
> +{
> + struct event_state *state = gd->event_state;
> + struct event_spy *spy;
> +
> + spy = malloc(sizeof(*spy));
> + if (!spy)
> + return log_msg_ret("alloc", -ENOMEM);
> +
> + spy->id = id;
> + spy->type = type;
> + spy->func = func;
> + spy->ctx = ctx;
> + list_add_tail(>sibling_node, >spy_head);
> +
> + return 0;
> +}
> +
> +int event_notify(enum event_t type, void *data, int size)
> +{
> + struct event_state *state = gd->event_state;
> + struct event_spy *spy, *next;
> + struct event event;
> +
> + event.type = type;
> + if (size > sizeof(event.data))
> + return log_msg_ret("size", -E2BIG);
> + memcpy(, data, size);
> + list_for_each_entry_safe(spy, next, >spy_head, sibling_node) {
> + if (spy->type == type) {
> + int ret;
> +
> + log_debug("Sending event %x to spy '%s'\n", type,
> +   spy->id);
> + ret = spy->func(spy->ctx, );
> +
> + /*
> +  * TODO: Handle various return codes 

[RFC PATCH] RFC: Sketch of dm event notification

2021-10-31 Thread Simon Glass
This is a patch for Takahiro to look at, just for illustration. Not ready
for review.

To run the test:

./u-boot -T -c "ut common test_event_probe"

Signed-off-by: Simon Glass 
---

 common/Kconfig|  11 
 common/Makefile   |   2 +
 common/board_f.c  |   2 +
 common/event.c| 103 ++
 common/log.c  |   1 +
 drivers/core/device.c |  14 
 include/asm-generic/global_data.h |   3 +
 include/event.h   | 103 ++
 include/event_internal.h  |  34 ++
 include/log.h |   2 +
 test/common/Makefile  |   1 +
 test/common/event.c   |  85 
 test/test-main.c  |   5 ++
 13 files changed, 366 insertions(+)
 create mode 100644 common/event.c
 create mode 100644 include/event.h
 create mode 100644 include/event_internal.h
 create mode 100644 test/common/event.c

diff --git a/common/Kconfig b/common/Kconfig
index d6f77ab7b9c..54d0adaa8d5 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -484,6 +484,17 @@ config DISPLAY_BOARDINFO_LATE
 
 menu "Start-up hooks"
 
+config EVENT
+   bool "General-purpose event-handling mechanism"
+   default y if SANDBOX
+   help
+ This enables sending and processing of events, to allow interested
+ parties to be alerted when something happens. This is an attempt to
+ step the flow of weak functions, hooks, functions in board_f.c
+ and board_r.c and the Kconfig options below.
+
+ See doc/develop/event.rst for more information.
+
 config ARCH_EARLY_INIT_R
bool "Call arch-specific init soon after relocation"
help
diff --git a/common/Makefile b/common/Makefile
index e7839027b6c..1b4601ac13f 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -101,6 +101,8 @@ obj-y += malloc_simple.o
 endif
 endif
 
+obj-$(CONFIG_EVENT) += event.o
+
 obj-y += image.o image-board.o
 obj-$(CONFIG_$(SPL_TPL_)HASH) += hash.o
 obj-$(CONFIG_ANDROID_AB) += android_ab.o
diff --git a/common/board_f.c b/common/board_f.c
index 3dc0eaa59c5..faf262d564c 100644
--- a/common/board_f.c
+++ b/common/board_f.c
@@ -19,6 +19,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -820,6 +821,7 @@ static const init_fnc_t init_sequence_f[] = {
initf_malloc,
log_init,
initf_bootstage,/* uses its own timer, so does not need DM */
+   event_init,
 #ifdef CONFIG_BLOBLIST
bloblist_init,
 #endif
diff --git a/common/event.c b/common/event.c
new file mode 100644
index 000..428628da44d
--- /dev/null
+++ b/common/event.c
@@ -0,0 +1,103 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Events provide a general-purpose way to react to / subscribe to changes
+ * within U-Boot
+ *
+ * Copyright 2021 Google LLC
+ * Written by Simon Glass 
+ */
+
+#define LOG_CATEGORY   LOGC_EVENT
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+DECLARE_GLOBAL_DATA_PTR;
+
+static void spy_free(struct event_spy *spy)
+{
+   list_del(>sibling_node);
+}
+
+int event_register(const char *id, enum event_t type, event_handler_t func, 
void *ctx)
+{
+   struct event_state *state = gd->event_state;
+   struct event_spy *spy;
+
+   spy = malloc(sizeof(*spy));
+   if (!spy)
+   return log_msg_ret("alloc", -ENOMEM);
+
+   spy->id = id;
+   spy->type = type;
+   spy->func = func;
+   spy->ctx = ctx;
+   list_add_tail(>sibling_node, >spy_head);
+
+   return 0;
+}
+
+int event_notify(enum event_t type, void *data, int size)
+{
+   struct event_state *state = gd->event_state;
+   struct event_spy *spy, *next;
+   struct event event;
+
+   event.type = type;
+   if (size > sizeof(event.data))
+   return log_msg_ret("size", -E2BIG);
+   memcpy(, data, size);
+   list_for_each_entry_safe(spy, next, >spy_head, sibling_node) {
+   if (spy->type == type) {
+   int ret;
+
+   log_debug("Sending event %x to spy '%s'\n", type,
+ spy->id);
+   ret = spy->func(spy->ctx, );
+
+   /*
+* TODO: Handle various return codes to
+*
+* - claim an event (no others will see it)
+* - return an error from the event
+*/
+   if (ret)
+   return log_msg_ret("spy", ret);
+   }
+   }
+
+   return 0;
+}
+
+int event_uninit(void)
+{
+   struct event_state *state = gd->event_state;
+   struct event_spy *spy, *next;
+
+   if (!state)
+   return 0;
+   list_for_each_entry_safe(spy, next, >spy_head, sibling_node)
+