Re: [PATCH 08/19] dm: add event notification

On 2/2/22 02:08, AKASHI Takahiro wrote:

From: Simon Glass sjg@chromium.org

This is a draft implementation of event notification mechanism from Simon. Under this scheme, any U-Boot subsystem can register some kind of callback function to a particular event (more event types will be added later) and that function will be invoked once the event is fired.

As a first user, UEFI subsystem makes use of PROBE and REMOVE events so that we will be able to automatically create/remove efi_disk objects relying on associated block devices (UCLASS_BLK).

To run the test: ./u-boot -T -c "ut common test_event_probe"

Signed-off-by: Simon Glass sjg@chromium.org [add REMOVE event; fix checkpatch warnings] Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org

---

common/Kconfig | 11 ++++ common/Makefile | 2 + common/board_f.c | 2 + common/event.c | 103 +++++++++++++++++++++++++++++ common/log.c | 1 + drivers/core/device-remove.c | 9 +++ drivers/core/device.c | 9 +++ include/asm-generic/global_data.h | 3 + include/dm/device-internal.h | 10 +++ include/event.h | 105 ++++++++++++++++++++++++++++++ include/event_internal.h | 34 ++++++++++ include/log.h | 2 + test/common/Makefile | 1 + test/common/event.c | 87 +++++++++++++++++++++++++ test/test-main.c | 5 ++ 15 files changed, 384 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 82cd864baf93..d411e5bf7a50 100644 --- a/common/Kconfig +++ b/common/Kconfig @@ -492,6 +492,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 3eff71960160..4e9f59cc07ad 100644 --- a/common/Makefile +++ b/common/Makefile @@ -89,6 +89,8 @@ obj-y += malloc_simple.o endif endif

+obj-$(CONFIG_EVENT) += event.o

• obj-$(CONFIG_$(SPL_TPL_)HASH) += hash.o obj-$(CONFIG_IO_TRACE) += iotrace.o obj-y += memsize.o

diff --git a/common/board_f.c b/common/board_f.c index a68760092ac1..e36bdbc988fa 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -19,6 +19,7 @@ #include <dm.h> #include <env.h> #include <env_internal.h> +#include <event.h> #include <fdtdec.h> #include <fs.h> #include <hang.h> @@ -828,6 +829,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 000000000000..428628da44d6 --- /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 sjg@chromium.org
• */

+#define LOG_CATEGORY LOGC_EVENT

+#include <common.h> +#include <event.h> +#include <event_internal.h> +#include <log.h> +#include <malloc.h> +#include <asm/global_data.h> +#include <linux/list.h>

+DECLARE_GLOBAL_DATA_PTR;

+static void spy_free(struct event_spy *spy) +{

• list_del(&spy->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(&spy->sibling_node, &state->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(&event.data, data, size);
• list_for_each_entry_safe(spy, next, &state->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, &event);
  

• 	/*
  

• 	 * 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, &state->spy_head, sibling_node)

• spy_free(spy);
  

• return 0;

+}

+int event_init(void) +{

• struct event_state *state;
• state = malloc(sizeof(struct event_state));
• if (!state)

• return log_msg_ret("alloc", -ENOMEM);
  

• INIT_LIST_HEAD(&state->spy_head);
• gd->event_state = state;
• return 0;

+} diff --git a/common/log.c b/common/log.c index f7e0c0fbf556..7254aa70bfdf 100644 --- a/common/log.c +++ b/common/log.c @@ -28,6 +28,7 @@ static const char *const log_cat_name[] = { "devres", "acpi", "boot",

• "event", };

  _Static_assert(ARRAY_SIZE(log_cat_name) == LOGC_COUNT - LOGC_NONE,

diff --git a/drivers/core/device-remove.c b/drivers/core/device-remove.c index e6ec6ff42121..c3f8ed3e5721 100644 --- a/drivers/core/device-remove.c +++ b/drivers/core/device-remove.c @@ -12,6 +12,7 @@

#include <common.h> #include <errno.h> +#include <event.h> #include <log.h> #include <malloc.h> #include <dm/device.h> @@ -207,6 +208,10 @@ int device_remove(struct udevice *dev, uint flags) if (!(dev_get_flags(dev) & DM_FLAG_ACTIVATED)) return 0;

• ret = device_notify(dev, EVT_DM_PRE_REMOVE);
• if (ret)

• return ret;
  

• /*
  • If the child returns EKEYREJECTED, continue. It just means that it
  • didn't match the flags.

@@ -256,6 +261,10 @@ int device_remove(struct udevice *dev, uint flags)

dev_bic_flags(dev, DM_FLAG_ACTIVATED);

• ret = device_notify(dev, EVT_DM_POST_REMOVE);

• if (ret)

• goto err_remove;
  

• return 0;

  err_remove:

diff --git a/drivers/core/device.c b/drivers/core/device.c index 901c1e2f7db3..6f69b234a182 100644 --- a/drivers/core/device.c +++ b/drivers/core/device.c @@ -10,6 +10,7 @@

#include <common.h> #include <cpu_func.h> +#include <event.h> #include <log.h> #include <asm/global_data.h> #include <asm/io.h> @@ -493,6 +494,10 @@ int device_probe(struct udevice *dev) if (dev_get_flags(dev) & DM_FLAG_ACTIVATED) return 0;

• ret = device_notify(dev, EVT_DM_PRE_PROBE);
• if (ret)

• return ret;
  

• drv = dev->driver; assert(drv);

@@ -597,6 +602,10 @@ int device_probe(struct udevice *dev) dev->name, ret, errno_str(ret)); }

• ret = device_notify(dev, EVT_DM_POST_PROBE);
• if (ret)

• goto fail;
  

• return 0; fail_uclass: if (device_remove(dev, DM_REMOVE_NORMAL)) {

diff --git a/include/asm-generic/global_data.h b/include/asm-generic/global_data.h index c2f8fad1cb92..9f4598f09ee5 100644 --- a/include/asm-generic/global_data.h +++ b/include/asm-generic/global_data.h @@ -467,6 +467,9 @@ struct global_data { */ char *smbios_version; #endif +#if CONFIG_IS_ENABLED(EVENT)

• struct event_state *event_state;

This breaks 'make htmldocs'.

./include/asm-generic/global_data.h:474: warning: Function parameter or member 'event_state' not described in 'global_data'

Best regards

Heinrich

+#endif }; #ifndef DO_DEPS_ONLY static_assert(sizeof(struct global_data) == GD_SIZE); diff --git a/include/dm/device-internal.h b/include/dm/device-internal.h index 02002acb787c..63074e1de509 100644 --- a/include/dm/device-internal.h +++ b/include/dm/device-internal.h @@ -10,6 +10,7 @@ #ifndef _DM_DEVICE_INTERNAL_H #define _DM_DEVICE_INTERNAL_H

+#include <event.h> #include <linker_lists.h> #include <dm/ofnode.h>

@@ -426,4 +427,13 @@ static inline void devres_release_all(struct udevice *dev) }

#endif /* ! CONFIG_DEVRES */

+static inline int device_notify(const struct udevice *dev, enum event_t type) +{ +#ifdef CONFIG_EVENT

• return event_notify(type, &dev, sizeof(dev));

+#else

• return 0;

+#endif +} #endif diff --git a/include/event.h b/include/event.h new file mode 100644 index 000000000000..e2b74e6e62f0 --- /dev/null +++ b/include/event.h @@ -0,0 +1,105 @@ +/* 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 sjg@chromium.org
• */

+#ifndef __event_h +#define __event_h

+/**

• • enum event_t - Types of events supported by U-Boot
• • @EVT_DM_PRE_PROBE: Device is about to be probed
• */

+enum event_t {

• EVT_NONE,
• EVT_TEST,
• /* Events related to driver model */
• EVT_DM_PRE_PROBE,
• EVT_DM_POST_PROBE,
• EVT_DM_PRE_REMOVE,
• EVT_DM_POST_REMOVE,
• EVT_COUNT

+};

+union event_data {

• /**

• * struct event_data_test  - test data
  

• *
  

• * @signal: A value to update the state with
  

• */
  

• struct event_data_test {

• int signal;
  

• } test;
• /**

• * struct event_dm - driver model event
  

• *
  

• * @dev: Device this event relates to
  

• */
  

• struct event_dm {

• struct udevice *dev;
  

• } dm;

+};

+/**

• • struct event - an event that can be sent and received
• • @type: Event type
• • @data: Data for this particular event
• */

+struct event {

• enum event_t type;
• union event_data data;

+};

+/** Function type for event handlers */ +typedef int (*event_handler_t)(void *ctx, struct event *event);

+/**

• • event_register - register a new spy
• • @id: Spy ID
• • @type: Event type to subscribe to
• • @func: Function to call when the event is sent
• • @ctx: Context to pass to the function
• • @return 0 if OK, -ve on erropr
• */

+int event_register(const char *id, enum event_t type, event_handler_t func,

•    void *ctx);
  

+/**

• • event_notify() - notify spies about an event
• • It is possible to pass in union event_data here but that may not be
• • convenient if the data is elsewhere, or is one of the members of the union.
• • So this uses a void * for @data, with a separate @size.
• • @type: Event type
• • @data: Event data to be sent (e.g. union_event_data)
• • @size: Size of data in bytes
• */

+int event_notify(enum event_t type, void *data, int size);

+#if CONFIG_IS_ENABLED(EVENT) +int event_uninit(void); +int event_init(void); +#else +static inline int event_uninit(void) +{

• return 0;

+}

+static inline int event_init(void) +{

• return 0;

+} +#endif

+#endif diff --git a/include/event_internal.h b/include/event_internal.h new file mode 100644 index 000000000000..19308453f7b2 --- /dev/null +++ b/include/event_internal.h @@ -0,0 +1,34 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/*

• • Internal definitions for events
• • Copyright 2021 Google LLC
• • Written by Simon Glass sjg@chromium.org
• */

+#ifndef __event_internal_h +#define __event_internal_h

+#include <linux/list.h>

+/**

• • struct event_spy - a spy that watches for an event of a particular type
• • @id: Spy ID
• • @type: Event type to subscribe to
• • @func: Function to call when the event is sent
• • @ctx: Context to pass to the function
• */

+struct event_spy {

• struct list_head sibling_node;
• const char *id;
• enum event_t type;
• event_handler_t func;
• void *ctx;

+};

+struct event_state {

• struct list_head spy_head;

+};

+#endif diff --git a/include/log.h b/include/log.h index ce48d51446f5..8f35c10abb5e 100644 --- a/include/log.h +++ b/include/log.h @@ -98,6 +98,8 @@ enum log_category_t { LOGC_ACPI, /** @LOGC_BOOT: Related to boot process / boot image processing */ LOGC_BOOT,

• /** @LOGC_EVENT: Related to event and event handling */
• LOGC_EVENT, /** @LOGC_COUNT: Number of log categories */ LOGC_COUNT, /** @LOGC_END: Sentinel value for lists of log categories */

diff --git a/test/common/Makefile b/test/common/Makefile index 24c9145dccc8..9087788ba6a8 100644 --- a/test/common/Makefile +++ b/test/common/Makefile @@ -1,3 +1,4 @@ # SPDX-License-Identifier: GPL-2.0+ obj-y += cmd_ut_common.o obj-$(CONFIG_AUTOBOOT) += test_autoboot.o +obj-$(CONFIG_EVENT) += event.o diff --git a/test/common/event.c b/test/common/event.c new file mode 100644 index 000000000000..ddce7400f269 --- /dev/null +++ b/test/common/event.c @@ -0,0 +1,87 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*

• • Unit tests for event handling
• • Copyright 2021 Google LLC
• • Written by Simon Glass sjg@chromium.org
• */

+#include <common.h> +#include <dm.h> +#include <event.h> +#include <test/common.h> +#include <test/test.h> +#include <test/ut.h>

+struct test_state {

• struct udevice *dev;
• int val;

+};

+static int h_adder(void *ctx, struct event *event) +{

• struct event_data_test *data = &event->data.test;
• struct test_state *test_state = ctx;
• test_state->val += data->signal;
• return 0;

+}

+static int test_event_base(struct unit_test_state *uts) +{

• struct test_state state;
• int signal;
• state.val = 12;
• ut_assertok(event_register("wibble", EVT_TEST, h_adder, &state));
• signal = 17;
• /* Check that the handler is called */
• ut_assertok(event_notify(EVT_TEST, &signal, sizeof(signal)));
• ut_asserteq(12 + 17, state.val);
• return 0;

+}

+COMMON_TEST(test_event_base, 0);

+static int h_probe(void *ctx, struct event *event) +{

• struct test_state *test_state = ctx;
• test_state->dev = event->data.dm.dev;
• switch (event->type) {
• case EVT_DM_PRE_PROBE:

• test_state->val |= 1;
  

• break;
  

• case EVT_DM_POST_PROBE:

• test_state->val |= 2;
  

• break;
  

• default:

• break;
  

• }
• return 0;

+}

+static int test_event_probe(struct unit_test_state *uts) +{

• struct test_state state;
• struct udevice *dev;
• state.val = 0;
• ut_assertok(event_register("pre", EVT_DM_PRE_PROBE, h_probe, &state));
• ut_assertok(event_register("post", EVT_DM_POST_PROBE, h_probe, &state));
• /* Probe a device */
• ut_assertok(uclass_first_device_err(UCLASS_TEST_FDT, &dev));
• /* Check that the handler is called */
• ut_asserteq(3, state.val);
• return 0;

+}

+COMMON_TEST(test_event_probe, UT_TESTF_DM | UT_TESTF_SCAN_FDT); diff --git a/test/test-main.c b/test/test-main.c index 8fcb02ecea5c..fd89ff558a0e 100644 --- a/test/test-main.c +++ b/test/test-main.c @@ -7,6 +7,7 @@ #include <common.h> #include <console.h> #include <dm.h> +#include <event.h> #include <dm/root.h> #include <dm/test.h> #include <dm/uclass-internal.h> @@ -218,6 +219,9 @@ static int dm_test_restore(struct device_node *of_root) */ static int test_pre_run(struct unit_test_state *uts, struct unit_test *test) {

• gd->event_state = NULL;
• ut_assertok(event_init());
• if (test->flags & UT_TESTF_DM) ut_assertok(dm_test_pre_run(uts));

@@ -260,6 +264,7 @@ static int test_post_run(struct unit_test_state *uts, struct unit_test *test) ut_unsilence_console(uts); if (test->flags & UT_TESTF_DM) ut_assertok(dm_test_post_run(uts));

• ut_assertok(event_uninit());

  return 0; }