[PATCH v2 00/13] event: Provide support for events to connect subsystems

[PATCH v2 00/13] event: Provide support for events to connect subsystems

[Simon Glass]

It is a common need in U-Boot to have one subsystem notify another
when something happens. An example is reading a partition table when a
new block device is set up.

It is also common to add weak functions and 'hook' functions to modify
how U-Boot works. See for example ft_board_setup() and the like.

U-Boot would benefit from a generic mechanism to handle these cases,
with the ability to hook into various 'events' in a
subsystem-independent and transparent way.

This series provides a way to create and dispatch events, with a way of
registering a 'spy' which watches for events of different types. This
allows 'hook' functions to be created in a generic way.

It also includes a script to list the hooks in an image, which is a bit
easier to debug than weak functions, as well as an 'event' command to
do the same from within U-Boot.

These 'static' events can be used to replace hooks like misc_init_f(),
for example. Also included is basic support for 'dynamic' events, where
a spy can be registered at runtime. The need for this is still being
figured out.

Changes in v2:
- Add a 'used' attribute to avoid LTO dropping the code
- Update keymile pg-wcom boards also
- Make the sandbox function static
- Convert arch/arm/mach-imx/imx8/cpu.c also
- Add DM_EVENT to ARCH_IMX8 too
- Tidy up galileo defconfig and rename quark_init_dm()
- Make a few more of the functions static
- Add a weak function for spl
- Update for patman snake-case change
- Use a regular expression in the test to avoid dependency on build dir

Simon Glass (12):
  Makefile: Allow LTO to be disabled for a build
  sandbox: start: Sort the header files
  binman: Expand elf support a little
  event: Add basic support for events
  event: Add a simple test
  event: Set up the event system on start-up
  event: Add events for device probe/remove
  event: Convert misc_init_f() to use events
  event: Convert arch_cpu_init_dm() to use events
  event: Add a command
  event: Add a script to decode the event-spy list
  event: Add documentation

Tim Harvey (1):
  phy: nop-phy: Fix phy reset if no reset-gpio defined

 MAINTAINERS                                   |  10 +
 Makefile                                      |  18 +-
 arch/Kconfig                                  |   3 +
 arch/arm/Kconfig                              |   4 +
 arch/arm/config.mk                            |   4 +-
 arch/arm/include/asm/global_data.h            |   2 +-
 arch/arm/mach-imx/imx8/cpu.c                  |   4 +-
 arch/arm/mach-imx/imx8m/soc.c                 |   4 +-
 arch/arm/mach-imx/imx8ulp/soc.c               |   4 +-
 arch/arm/mach-omap2/am33xx/board.c            |   4 +-
 arch/arm/mach-omap2/hwinit-common.c           |   5 +-
 arch/mips/Kconfig                             |   1 +
 arch/mips/mach-pic32/cpu.c                    |   4 +-
 arch/nios2/cpu/cpu.c                          |   4 +-
 arch/riscv/cpu/cpu.c                          |   5 +-
 arch/riscv/include/asm/system.h               |   5 +
 arch/riscv/lib/spl.c                          |   3 +-
 arch/sandbox/cpu/start.c                      |   8 +-
 arch/x86/cpu/baytrail/cpu.c                   |   4 +-
 arch/x86/cpu/broadwell/cpu.c                  |   4 +-
 arch/x86/cpu/ivybridge/cpu.c                  |   4 +-
 arch/x86/cpu/quark/quark.c                    |   4 +-
 arch/x86/include/asm/fsp2/fsp_api.h           |   8 +
 arch/x86/lib/fsp2/fsp_init.c                  |   4 +-
 arch/x86/lib/spl.c                            |   5 +-
 arch/x86/lib/tpl.c                            |  10 -
 board/google/chromebook_coral/coral.c         |   7 +-
 board/keymile/kmcent2/kmcent2.c               |   4 +-
 .../keymile/pg-wcom-ls102xa/pg-wcom-ls102xa.c |   5 +-
 cmd/Kconfig                                   |   8 +
 cmd/Makefile                                  |   1 +
 cmd/event.c                                   |  27 +++
 common/Kconfig                                |  37 ++-
 common/Makefile                               |   2 +
 common/board_f.c                              |  13 +-
 common/board_r.c                              |   1 +
 common/event.c                                | 196 ++++++++++++++++
 common/log.c                                  |   1 +
 configs/chromebook_coral_defconfig            |   1 +
 configs/kmcent2_defconfig                     |   2 +-
 configs/pg_wcom_expu1_defconfig               |   1 +
 configs/pg_wcom_expu1_update_defconfig        |   1 +
 configs/pg_wcom_seli8_defconfig               |   1 +
 configs/pg_wcom_seli8_update_defconfig        |   1 +
 configs/sandbox64_defconfig                   |   1 -
 configs/sandbox_defconfig                     |   1 -
 configs/sandbox_flattree_defconfig            |   1 -
 configs/sandbox_spl_defconfig                 |   1 -
 configs/tools-only_defconfig                  |   1 -
 doc/build/gcc.rst                             |  17 ++
 doc/develop/event.rst                         |  66 ++++++
 doc/develop/index.rst                         |   1 +
 doc/usage/event.rst                           |  49 ++++
 doc/usage/index.rst                           |   1 +
 drivers/core/Kconfig                          |  10 +
 drivers/core/device-remove.c                  |   8 +
 drivers/core/device.c                         |   9 +
 drivers/core/root.c                           |   5 +
 drivers/phy/nop-phy.c                         |  12 +-
 include/asm-generic/global_data.h             |  13 ++
 include/configs/km/pg-wcom-ls102xa.h          |   2 -
 include/dm/device-internal.h                  |  10 +
 include/event.h                               | 210 ++++++++++++++++++
 include/event_internal.h                      |  35 +++
 include/init.h                                |  12 -
 include/log.h                                 |   2 +
 scripts/event_dump.py                         | 115 ++++++++++
 test/common/Makefile                          |   1 +
 test/common/event.c                           |  85 +++++++
 test/py/tests/test_event_dump.py              |  20 ++
 test/test-main.c                              |   4 +
 tools/binman/elf.py                           |  58 ++++-
 72 files changed, 1108 insertions(+), 86 deletions(-)
 create mode 100644 cmd/event.c
 create mode 100644 common/event.c
 create mode 100644 doc/develop/event.rst
 create mode 100644 doc/usage/event.rst
 create mode 100644 include/event.h
 create mode 100644 include/event_internal.h
 create mode 100755 scripts/event_dump.py
 create mode 100644 test/common/event.c
 create mode 100644 test/py/tests/test_event_dump.py

-- 
2.35.1.616.g0bdcbb4464-goog

[PATCH v2 01/13] phy: nop-phy: Fix phy reset if no reset-gpio defined

[Simon Glass]

From: Tim Harvey <tharvey@gateworks.com>

Ensure there is a valid reset-gpio defined before using it.

Fixes: f9852acdce02 ("phy: nop-phy: Fix enabling reset")
Cc: Adam Ford <aford173@gmail.com>
Signed-off-by: Tim Harvey <tharvey@gateworks.com>
Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 drivers/phy/nop-phy.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/phy/nop-phy.c b/drivers/phy/nop-phy.c
index e2ee6e9206..d0904f4f07 100644
--- a/drivers/phy/nop-phy.c
+++ b/drivers/phy/nop-phy.c
@@ -45,11 +45,13 @@ static int nop_phy_init(struct phy *phy)
 
 #if CONFIG_IS_ENABLED(DM_GPIO)
 	/* Take phy out of reset */
-	ret = dm_gpio_set_value(&priv->reset_gpio, false);
-	if (ret) {
-		if (CONFIG_IS_ENABLED(CLK))
-			clk_disable_bulk(&priv->bulk);
-		return ret;
+	if (dm_gpio_is_valid(&priv->reset_gpio)) {
+		ret = dm_gpio_set_value(&priv->reset_gpio, false);
+		if (ret) {
+			if (CONFIG_IS_ENABLED(CLK))
+				clk_disable_bulk(&priv->bulk);
+			return ret;
+		}
 	}
 #endif
 	return 0;
-- 
2.35.1.616.g0bdcbb4464-goog

[PATCH v2 02/13] Makefile: Allow LTO to be disabled for a build

[Simon Glass]

LTO (Link-Time Optimisation) is an very useful feature which can
significantly reduce the size of U-Boot binaries. So far it has been
made available for selected ARM boards and sandbox.

However, incremental builds are much slower when LTO is used. For example,
an incremental build of sandbox takes 2.1 seconds on my machine, but 6.7
seconds with LTO enabled.

Add a LTO_BUILD=n parameter to the build, so it can be disabled during
development if needed, for faster builds.

Add some documentation about LTO while we are here.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 Makefile                           | 18 +++++++++++++-----
 arch/arm/config.mk                 |  4 ++--
 arch/arm/include/asm/global_data.h |  2 +-
 doc/build/gcc.rst                  | 17 +++++++++++++++++
 4 files changed, 33 insertions(+), 8 deletions(-)

diff --git a/Makefile b/Makefile
index 9ef34ca4b7..716f5e1a08 100644
--- a/Makefile
+++ b/Makefile
@@ -434,6 +434,9 @@ KBUILD_CFLAGS	+= -fshort-wchar -fno-strict-aliasing
 KBUILD_AFLAGS   := -D__ASSEMBLY__
 KBUILD_LDFLAGS  :=
 
+# Set this to "n" use of LTO for this build, e.g. LTO_BUILD=n
+LTO_BUILD	?= y
+
 ifeq ($(cc-name),clang)
 ifneq ($(CROSS_COMPILE),)
 CLANG_TARGET	:= --target=$(notdir $(CROSS_COMPILE:%-=%))
@@ -643,6 +646,11 @@ export CFLAGS_EFI	# Compiler flags to add when building EFI app
 export CFLAGS_NON_EFI	# Compiler flags to remove when building EFI app
 export EFI_TARGET	# binutils target if EFI is natively supported
 
+export LTO_ENABLE
+
+# This is y if LTO is enabled for this build
+LTO_ENABLE=$(if $(CONFIG_LTO),$(LTO_BUILD),)
+
 # If board code explicitly specified LDSCRIPT or CONFIG_SYS_LDSCRIPT, use
 # that (or fail if absent).  Otherwise, search for a linker script in a
 # standard location.
@@ -690,16 +698,16 @@ endif
 LTO_CFLAGS :=
 LTO_FINAL_LDFLAGS :=
 export LTO_CFLAGS LTO_FINAL_LDFLAGS
-ifdef CONFIG_LTO
+ifeq ($(LTO_ENABLE),y)
 	ifeq ($(cc-name),clang)
-		LTO_CFLAGS		+= -flto
+		LTO_CFLAGS		+= -DLTO_ENABLE -flto
 		LTO_FINAL_LDFLAGS	+= -flto
 
 		AR			= $(shell $(CC) -print-prog-name=llvm-ar)
 		NM			= $(shell $(CC) -print-prog-name=llvm-nm)
 	else
 		NPROC			:= $(shell nproc 2>/dev/null || echo 1)
-		LTO_CFLAGS		+= -flto=$(NPROC)
+		LTO_CFLAGS		+= -DLTO_ENABLE -flto=$(NPROC)
 		LTO_FINAL_LDFLAGS	+= -fuse-linker-plugin -flto=$(NPROC)
 
 		# use plugin aware tools
@@ -1740,7 +1748,7 @@ ARCH_POSTLINK := $(wildcard $(srctree)/arch/$(ARCH)/Makefile.postlink)
 
 # Generate linker list symbols references to force compiler to not optimize
 # them away when compiling with LTO
-ifdef CONFIG_LTO
+ifeq ($(LTO_ENABLE),y)
 u-boot-keep-syms-lto := keep-syms-lto.o
 u-boot-keep-syms-lto_c := $(patsubst %.o,%.c,$(u-boot-keep-syms-lto))
 
@@ -1762,7 +1770,7 @@ endif
 
 # Rule to link u-boot
 # May be overridden by arch/$(ARCH)/config.mk
-ifdef CONFIG_LTO
+ifeq ($(LTO_ENABLE),y)
 quiet_cmd_u-boot__ ?= LTO     $@
       cmd_u-boot__ ?=								\
 		$(CC) -nostdlib -nostartfiles					\
diff --git a/arch/arm/config.mk b/arch/arm/config.mk
index b107b1af27..065dbec406 100644
--- a/arch/arm/config.mk
+++ b/arch/arm/config.mk
@@ -15,11 +15,11 @@ CFLAGS_NON_EFI := -fno-pic -ffixed-r9 -ffunction-sections -fdata-sections \
 		  -fstack-protector-strong
 CFLAGS_EFI := -fpic -fshort-wchar
 
-ifneq ($(CONFIG_LTO)$(CONFIG_USE_PRIVATE_LIBGCC),yy)
+ifneq ($(LTO_ENABLE)$(CONFIG_USE_PRIVATE_LIBGCC),yy)
 LDFLAGS_FINAL += --gc-sections
 endif
 
-ifndef CONFIG_LTO
+ifneq ($(LTO_ENABLE),y)
 PLATFORM_RELFLAGS += -ffunction-sections -fdata-sections
 endif
 
diff --git a/arch/arm/include/asm/global_data.h b/arch/arm/include/asm/global_data.h
index 085e12b5d4..b255b195aa 100644
--- a/arch/arm/include/asm/global_data.h
+++ b/arch/arm/include/asm/global_data.h
@@ -98,7 +98,7 @@ struct arch_global_data {
 
 #include <asm-generic/global_data.h>
 
-#if defined(__clang__) || defined(CONFIG_LTO)
+#if defined(__clang__) || defined(LTO_ENABLE)
 
 #define DECLARE_GLOBAL_DATA_PTR
 #define gd	get_gd()
diff --git a/doc/build/gcc.rst b/doc/build/gcc.rst
index b883cf73ee..08c74c4d7b 100644
--- a/doc/build/gcc.rst
+++ b/doc/build/gcc.rst
@@ -151,6 +151,23 @@ of dtc is new enough. It also makes sure that pylibfdt is present, if needed
 Note that the :doc:`tools` are always built with the included version of libfdt
 so it is not possible to build U-Boot tools with a system libfdt, at present.
 
+Link-time optimisation (LTO)
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+U-Boot supports link-time optimisation which can reduce the size of the final
+U-Boot binaries, particularly with SPL.
+
+At present this can be enabled by ARM boards by adding `CONFIG_LTO=y` into the
+defconfig file. Other architectures are not supported. LTO is enabled by default
+for sandbox.
+
+This does incur a link-time penalty of several seconds. For faster incremental
+builds during development, you can disable it by setting `LTO_BUILD` to `n`.
+
+.. code-block:: bash
+
+    LTO_BUILD=n make
+
 Other build targets
 ~~~~~~~~~~~~~~~~~~~
 
-- 
2.35.1.616.g0bdcbb4464-goog

[PATCH v2 03/13] sandbox: start: Sort the header files

[Simon Glass]

These header files don't follow the correct order. Fix this.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 arch/sandbox/cpu/start.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/sandbox/cpu/start.c b/arch/sandbox/cpu/start.c
index 13b0731ec3..12aace9a20 100644
--- a/arch/sandbox/cpu/start.c
+++ b/arch/sandbox/cpu/start.c
@@ -4,14 +4,13 @@
  */
 
 #include <common.h>
+#include <cli.h>
 #include <command.h>
-#include <dm/root.h>
 #include <efi_loader.h>
 #include <errno.h>
 #include <init.h>
 #include <log.h>
 #include <os.h>
-#include <cli.h>
 #include <sort.h>
 #include <asm/getopt.h>
 #include <asm/global_data.h>
@@ -19,6 +18,7 @@
 #include <asm/malloc.h>
 #include <asm/sections.h>
 #include <asm/state.h>
+#include <dm/root.h>
 #include <linux/ctype.h>
 
 DECLARE_GLOBAL_DATA_PTR;
-- 
2.35.1.616.g0bdcbb4464-goog

[PATCH v2 04/13] binman: Expand elf support a little

[Simon Glass]

Allow finding a symbol by its address. Also export the function to get
the file offset of a particular address, so it can be used by a script to
be added.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 tools/binman/elf.py | 58 +++++++++++++++++++++++++++++++++++++++------
 1 file changed, 51 insertions(+), 7 deletions(-)

diff --git a/tools/binman/elf.py b/tools/binman/elf.py
index 5e7d6ae7b9..35971731d0 100644
--- a/tools/binman/elf.py
+++ b/tools/binman/elf.py
@@ -85,6 +85,57 @@ def GetSymbols(fname, patterns):
     # Sort dict by address
     return OrderedDict(sorted(syms.items(), key=lambda x: x[1].address))
 
+def _GetFileOffset(elf, addr):
+    """Get the file offset for an address
+
+    Args:
+        elf (ELFFile): ELF file to check
+        addr (int): Address to search for
+
+    Returns
+        int: Offset of that address in the ELF file, or None if not valid
+    """
+    for seg in elf.iter_segments():
+        seg_end = seg['p_vaddr'] + seg['p_filesz']
+        if seg.header['p_type'] == 'PT_LOAD':
+            if addr >= seg['p_vaddr'] and addr < seg_end:
+                return addr - seg['p_vaddr'] + seg['p_offset']
+
+def GetFileOffset(fname, addr):
+    """Get the file offset for an address
+
+    Args:
+        fname (str): Filename of ELF file to check
+        addr (int): Address to search for
+
+    Returns
+        int: Offset of that address in the ELF file, or None if not valid
+    """
+    if not ELF_TOOLS:
+        raise ValueError('Python elftools package is not available')
+    with open(fname, 'rb') as fd:
+        elf = ELFFile(fd)
+        return _GetFileOffset(elf, addr)
+
+def GetSymbolFromAddress(fname, addr):
+    """Get the symbol at a particular address
+
+    Args:
+        fname (str): Filename of ELF file to check
+        addr (int): Address to search for
+
+    Returns:
+        str: Symbol name, or None if no symbol at that address
+    """
+    if not ELF_TOOLS:
+        raise ValueError('Python elftools package is not available')
+    with open(fname, 'rb') as fd:
+        elf = ELFFile(fd)
+        syms = GetSymbols(fname, None)
+    for name, sym in syms.items():
+        if sym.address == addr:
+            return name
+
 def GetSymbolFileOffset(fname, patterns):
     """Get the symbols from an ELF file
 
@@ -97,13 +148,6 @@ def GetSymbolFileOffset(fname, patterns):
           key: Name of symbol
           value: Hex value of symbol
     """
-    def _GetFileOffset(elf, addr):
-        for seg in elf.iter_segments():
-            seg_end = seg['p_vaddr'] + seg['p_filesz']
-            if seg.header['p_type'] == 'PT_LOAD':
-                if addr >= seg['p_vaddr'] and addr < seg_end:
-                    return addr - seg['p_vaddr'] + seg['p_offset']
-
     if not ELF_TOOLS:
         raise ValueError('Python elftools package is not available')
 
-- 
2.35.1.616.g0bdcbb4464-goog

[PATCH v2 05/13] event: Add basic support for events

[Simon Glass]

Add a way to create and dispatch events without needing to allocate
memory. Also add a way to 'spy' on events, thus allowing 'hooks' to be
created.

Use a linker list for static events, which we can use to replace functions
like arch_cpu_init_f(). Allow an EVENT_DEBUG option which makes it
easier to see what is going on at runtime, but uses more code space.

Dynamic events allow the creation of a spy at runtime. This is not always
necessary, but can be enabled with EVENT_DYNAMIC if needed.

A 'test' event is the only option for now.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2:
- Add a 'used' attribute to avoid LTO dropping the code

 MAINTAINERS                       |   6 +
 common/Kconfig                    |  31 +++++
 common/Makefile                   |   2 +
 common/board_r.c                  |   1 +
 common/event.c                    | 186 +++++++++++++++++++++++++++++
 common/log.c                      |   1 +
 include/asm-generic/global_data.h |  13 +++
 include/event.h                   | 188 ++++++++++++++++++++++++++++++
 include/event_internal.h          |  35 ++++++
 include/log.h                     |   2 +
 10 files changed, 465 insertions(+)
 create mode 100644 common/event.c
 create mode 100644 include/event.h
 create mode 100644 include/event_internal.h

diff --git a/MAINTAINERS b/MAINTAINERS
index fb171e0c68..b534ad66b9 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -809,6 +809,12 @@ S:	Maintained
 F:	doc/usage/environment.rst
 F:	scripts/env2string.awk
 
+EVENTS
+M:	Simon Glass <sjg@chromium.org>
+S:	Maintained
+F:	common/event.c
+F:	include/event.h
+
 FASTBOOT
 S:	Orphaned
 F:	cmd/fastboot.c
diff --git a/common/Kconfig b/common/Kconfig
index 82cd864baf..76c04b2001 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -492,6 +492,37 @@ 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.
+
+if EVENT
+
+config EVENT_DYNAMIC
+	bool "Support event registration at runtime"
+	default y if SANDBOX
+	help
+	  Enable this to support adding an event spy at runtime, without adding
+	  it to the EVENT_SPy() linker list. This increases code size slightly
+	  but provides more flexibility for boards and subsystems that need it.
+
+config EVENT_DEBUG
+	bool "Enable event debugging assistance"
+	default y if SANDBOX
+	help
+	  Enable this get usefui features for seeing what is happening with
+	  events, such as event-type names. This adds to the code size of
+	  U-Boot so can be turned off for production builds.
+
+endif # EVENT
+
 config ARCH_EARLY_INIT_R
 	bool "Call arch-specific init soon after relocation"
 	help
diff --git a/common/Makefile b/common/Makefile
index 3eff719601..cc2ba30c63 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -89,6 +89,8 @@ obj-y += malloc_simple.o
 endif
 endif
 
+obj-$(CONFIG_$(SPL_TPL_)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_r.c b/common/board_r.c
index c24d9b4e22..b92c1bb0be 100644
--- a/common/board_r.c
+++ b/common/board_r.c
@@ -594,6 +594,7 @@ static int run_main_loop(void)
 static init_fnc_t init_sequence_r[] = {
 	initr_trace,
 	initr_reloc,
+	event_init,
 	/* TODO: could x86/PPC have this also perhaps? */
 #if defined(CONFIG_ARM) || defined(CONFIG_RISCV)
 	initr_caches,
diff --git a/common/event.c b/common/event.c
new file mode 100644
index 0000000000..366be24569
--- /dev/null
+++ b/common/event.c
@@ -0,0 +1,186 @@
+// 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 <linker_lists.h>
+#include <malloc.h>
+#include <asm/global_data.h>
+#include <linux/list.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+#if CONFIG_IS_ENABLED(EVENT_DEBUG)
+const char *const type_name[] = {
+	"none",
+	"test",
+};
+
+_Static_assert(ARRAY_SIZE(type_name) == EVT_COUNT, "event type_name size");
+#endif
+
+static const char *event_type_name(enum event_t type)
+{
+#if CONFIG_IS_ENABLED(EVENT_DEBUG)
+	return type_name[type];
+#else
+	return "(unknown)";
+#endif
+}
+
+static int notify_static(struct event *ev)
+{
+	struct evspy_info *start =
+		ll_entry_start(struct evspy_info, evspy_info);
+	const int n_ents = ll_entry_count(struct evspy_info, evspy_info);
+	struct evspy_info *spy;
+
+	for (spy = start; spy != start + n_ents; spy++) {
+		if (spy->type == ev->type) {
+			int ret;
+
+			log_debug("Sending event %x/%s to spy '%s'\n", ev->type,
+				  event_type_name(ev->type), event_spy_id(spy));
+			ret = spy->func(NULL, ev);
+
+			/*
+			 * 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;
+}
+
+static int notify_dynamic(struct event *ev)
+{
+	struct event_state *state = gd_event_state();
+	struct event_spy *spy, *next;
+
+	list_for_each_entry_safe(spy, next, &state->spy_head, sibling_node) {
+		if (spy->type == ev->type) {
+			int ret;
+
+			log_debug("Sending event %x/%s to spy '%s'\n", ev->type,
+				  event_type_name(ev->type), spy->id);
+			ret = spy->func(spy->ctx, ev);
+
+			/*
+			 * 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_notify(enum event_t type, void *data, int size)
+{
+	struct event event;
+	int ret;
+
+	event.type = type;
+	if (size > sizeof(event.data))
+		return log_msg_ret("size", -E2BIG);
+	memcpy(&event.data, data, size);
+
+	ret = notify_static(&event);
+	if (ret)
+		return log_msg_ret("dyn", ret);
+
+	if (CONFIG_IS_ENABLED(EVENT_DYNAMIC)) {
+		ret = notify_dynamic(&event);
+		if (ret)
+			return log_msg_ret("dyn", ret);
+	}
+
+	return 0;
+}
+
+int event_notify_null(enum event_t type)
+{
+	return event_notify(type, NULL, 0);
+}
+
+void event_show_spy_list(void)
+{
+	struct evspy_info *start =
+		ll_entry_start(struct evspy_info, evspy_info);
+	const int n_ents = ll_entry_count(struct evspy_info, evspy_info);
+	struct evspy_info *spy;
+	const int size = sizeof(ulong) * 2;
+
+	printf("Seq  %-24s  %*s  %s\n", "Type", size, "Function", "ID");
+	for (spy = start; spy != start + n_ents; spy++) {
+		printf("%3x  %-3x %-20s  %*p  %s\n", (uint)(spy - start),
+		       spy->type, event_type_name(spy->type), size, spy->func,
+		       event_spy_id(spy));
+	}
+}
+
+#if CONFIG_IS_ENABLED(EVENT_DYNAMIC)
+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;
+
+	if (!CONFIG_IS_ENABLED(EVENT_DYNAMIC))
+		return -ENOSYS;
+	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_uninit(void)
+{
+	struct event_state *state = gd_event_state();
+	struct event_spy *spy, *next;
+
+	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 = gd_event_state();
+
+	INIT_LIST_HEAD(&state->spy_head);
+
+	return 0;
+}
+#endif /* EVENT_DYNAMIC */
diff --git a/common/log.c b/common/log.c
index f7e0c0fbf5..7254aa70bf 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/include/asm-generic/global_data.h b/include/asm-generic/global_data.h
index c2f8fad1cb..e49f5bf2f7 100644
--- a/include/asm-generic/global_data.h
+++ b/include/asm-generic/global_data.h
@@ -20,6 +20,7 @@
  */
 
 #ifndef __ASSEMBLY__
+#include <event_internal.h>
 #include <fdtdec.h>
 #include <membuff.h>
 #include <linux/list.h>
@@ -467,6 +468,12 @@ struct global_data {
 	 */
 	char *smbios_version;
 #endif
+#if CONFIG_IS_ENABLED(EVENT)
+	/**
+	 * @event_state: Points to the current state of events
+	 */
+	struct event_state event_state;
+#endif
 };
 #ifndef DO_DEPS_ONLY
 static_assert(sizeof(struct global_data) == GD_SIZE);
@@ -532,6 +539,12 @@ static_assert(sizeof(struct global_data) == GD_SIZE);
 #define gd_set_multi_dtb_fit(_dtb)
 #endif
 
+#if CONFIG_IS_ENABLED(EVENT_DYNAMIC)
+#define gd_event_state()	((struct event_state *)&gd->event_state)
+#else
+#define gd_event_state()	NULL
+#endif
+
 /**
  * enum gd_flags - global data flags
  *
diff --git a/include/event.h b/include/event.h
new file mode 100644
index 0000000000..effd58c704
--- /dev/null
+++ b/include/event.h
@@ -0,0 +1,188 @@
+/* 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,
+
+	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 - 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);
+
+/**
+ * struct evspy_info - information about an event spy
+ *
+ * @func: Function to call when the event is activated (must be first)
+ * @type: Event type
+ * @id: Event id string
+ */
+struct evspy_info {
+	event_handler_t func;
+	enum event_t type;
+#if CONFIG_IS_ENABLED(EVENT_DEBUG)
+	const char *id;
+#endif
+};
+
+/* Declare a new event spy */
+#if CONFIG_IS_ENABLED(EVENT_DEBUG)
+#define _ESPY_REC(_type, _func)   { _func, _type, #_func, }
+#else
+#define _ESPY_REC(_type, _func)   { _func, _type, }
+#endif
+
+static inline const char *event_spy_id(struct evspy_info *spy)
+{
+#if CONFIG_IS_ENABLED(EVENT_DEBUG)
+	return spy->id;
+#else
+	return "?";
+#endif
+}
+
+/*
+ * It seems that LTO will drop list entries if it decides they are not used,
+ * although the conditions that cause this are unclear.
+ *
+ * The example found is the following:
+ *
+ * static int sandbox_misc_init_f(void *ctx, struct event *event)
+ * {
+ *    return sandbox_early_getopt_check();
+ * }
+ * EVENT_SPY(EVT_MISC_INIT_F, sandbox_misc_init_f);
+ *
+ * where EVENT_SPY uses ll_entry_declare()
+ *
+ * In this case, LTO decides to drop the sandbox_misc_init_f() function
+ * (which is fine) but then drops the linker-list entry too. This means
+ * that the code no longer works, in this case sandbox no-longer checks its
+ * command-line arguments properly.
+ *
+ * Without LTO, the KEEP() command in the .lds file is enough to keep the
+ * entry around. But with LTO it seems that the entry has already been
+ * dropped before the link script is considered.
+ *
+ * The only solution I can think of is to mark linker-list entries as 'used'
+ * using an attribute. This should be safe, since we don't actually want to drop
+ * any of these. However this does slightly limit LTO's optimisation choices.
+ */
+#define EVENT_SPY(_type, _func) \
+	static __attribute__((used)) ll_entry_declare(struct evspy_info, \
+						      _type, evspy_info) = \
+	_ESPY_REC(_type, _func)
+
+/**
+ * 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 error
+ */
+int event_register(const char *id, enum event_t type, event_handler_t func,
+		   void *ctx);
+
+#if CONFIG_IS_ENABLED(EVENT)
+/**
+ * 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
+ * @return 0 if OK, -ve on error
+ */
+int event_notify(enum event_t type, void *data, int size);
+
+/**
+ * event_notify_null() - notify spies about an event
+ *
+ * Data is NULL and the size is 0
+ *
+ * @type: Event type
+ * @return 0 if OK, -ve on error
+ */
+int event_notify_null(enum event_t type);
+#else
+static inline int event_notify(enum event_t type, void *data, int size)
+{
+	return 0;
+}
+
+static inline int event_notify_null(enum event_t type)
+{
+	return 0;
+}
+#endif
+
+#if CONFIG_IS_ENABLED(EVENT_DYNAMIC)
+/**
+ * event_uninit() - Clean up dynamic events
+ *
+ * This removes all dynamic event handlers
+ */
+int event_uninit(void);
+
+/**
+ * event_uninit() - Set up dynamic events
+ *
+ * Init a list of dynamic event handlers, so that these can be added as
+ * needed
+ */
+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 0000000000..8432c6f0e5
--- /dev/null
+++ b/include/event_internal.h
@@ -0,0 +1,35 @@
+/* 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 <event.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 ce48d51446..8f35c10abb 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 */
-- 
2.35.1.616.g0bdcbb4464-goog

[PATCH v2 06/13] event: Add a simple test

[Simon Glass]

Add a test for event registration and activation.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 MAINTAINERS          |  1 +
 test/common/Makefile |  1 +
 test/common/event.c  | 47 ++++++++++++++++++++++++++++++++++++++++++++
 test/test-main.c     |  4 ++++
 4 files changed, 53 insertions(+)
 create mode 100644 test/common/event.c

diff --git a/MAINTAINERS b/MAINTAINERS
index b534ad66b9..2786adad4b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -814,6 +814,7 @@ M:	Simon Glass <sjg@chromium.org>
 S:	Maintained
 F:	common/event.c
 F:	include/event.h
+F:	test/common/event.c
 
 FASTBOOT
 S:	Orphaned
diff --git a/test/common/Makefile b/test/common/Makefile
index 24c9145dcc..9087788ba6 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 0000000000..dfaa66ea49
--- /dev/null
+++ b/test/common/event.c
@@ -0,0 +1,47 @@
+// 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);
diff --git a/test/test-main.c b/test/test-main.c
index 8fcb02ecea..ee38d1faea 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,8 @@ static int dm_test_restore(struct device_node *of_root)
  */
 static int test_pre_run(struct unit_test_state *uts, struct unit_test *test)
 {
+	ut_assertok(event_init());
+
 	if (test->flags & UT_TESTF_DM)
 		ut_assertok(dm_test_pre_run(uts));
 
@@ -260,6 +263,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;
 }
-- 
2.35.1.616.g0bdcbb4464-goog

[PATCH v2 07/13] event: Set up the event system on start-up

[Simon Glass]

Call event_init() before relocation to get the event system running.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 common/board_f.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/common/board_f.c b/common/board_f.c
index a68760092a..e36bdbc988 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
-- 
2.35.1.616.g0bdcbb4464-goog

[PATCH v2 08/13] event: Add events for device probe/remove

[Simon Glass]

Generate events when devices are probed or removed, allowing hooks
to be added for these situations.

This is controlled by the DM_EVENT config option.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 common/event.c               |  6 ++++++
 drivers/core/Kconfig         | 10 ++++++++++
 drivers/core/device-remove.c |  8 ++++++++
 drivers/core/device.c        |  9 +++++++++
 include/dm/device-internal.h | 10 ++++++++++
 include/event.h              | 15 ++++++++++++++
 test/common/event.c          | 38 ++++++++++++++++++++++++++++++++++++
 7 files changed, 96 insertions(+)

diff --git a/common/event.c b/common/event.c
index 366be24569..737d3ac9ea 100644
--- a/common/event.c
+++ b/common/event.c
@@ -24,6 +24,12 @@ DECLARE_GLOBAL_DATA_PTR;
 const char *const type_name[] = {
 	"none",
 	"test",
+
+	/* Events related to driver model */
+	"dm_pre_probe",
+	"dm_post_probe",
+	"dm_pre_remove",
+	"dm_post_remove",
 };
 
 _Static_assert(ARRAY_SIZE(type_name) == EVT_COUNT, "event type_name size");
diff --git a/drivers/core/Kconfig b/drivers/core/Kconfig
index 8f7703c8b5..5c3400417f 100644
--- a/drivers/core/Kconfig
+++ b/drivers/core/Kconfig
@@ -77,6 +77,16 @@ config DM_DEVICE_REMOVE
 	  it causes unplugged devices to linger around in the dm-tree, and it
 	  causes USB host controllers to not be stopped when booting the OS.
 
+config DM_EVENT
+	bool "Support events with driver model"
+	depends on DM
+	imply EVENT
+	default y if SANDBOX
+	help
+	  This enables support for generating events related to driver model
+	  operations, such as prbing or removing a device. Subsystems can
+	  register a 'spy' function that is called when the event occurs.
+
 config SPL_DM_DEVICE_REMOVE
 	bool "Support device removal in SPL"
 	depends on SPL_DM
diff --git a/drivers/core/device-remove.c b/drivers/core/device-remove.c
index e6ec6ff421..73d2e9e420 100644
--- a/drivers/core/device-remove.c
+++ b/drivers/core/device-remove.c
@@ -207,6 +207,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 +260,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 901c1e2f7d..1b356f12dd 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)
+		return ret;
+
 	return 0;
 fail_uclass:
 	if (device_remove(dev, DM_REMOVE_NORMAL)) {
diff --git a/include/dm/device-internal.h b/include/dm/device-internal.h
index 02002acb78..c420726287 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)
+{
+#if CONFIG_IS_ENABLED(DM_EVENT)
+	return event_notify(type, &dev, sizeof(dev));
+#else
+	return 0;
+#endif
+}
 #endif
diff --git a/include/event.h b/include/event.h
index effd58c704..f4c12d768b 100644
--- a/include/event.h
+++ b/include/event.h
@@ -19,6 +19,12 @@ 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
 };
 
@@ -31,6 +37,15 @@ union event_data {
 	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;
 };
 
 /**
diff --git a/test/common/event.c b/test/common/event.c
index dfaa66ea49..6037ae2ce3 100644
--- a/test/common/event.c
+++ b/test/common/event.c
@@ -45,3 +45,41 @@ static int test_event_base(struct unit_test_state *uts)
 	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);
-- 
2.35.1.616.g0bdcbb4464-goog

[PATCH v2 09/13] event: Convert misc_init_f() to use events

[Simon Glass]

This hook can be implmented using events, for the three boards that
actually use it.

Add the event type and event handlers. Drop CONFIG_MISC_INIT_F since we
can just use CONFIG_EVENT to control this. Since sandbox always enables
CONFIG_EVENT, we can drop the defconfig lines there too.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2:
- Update keymile pg-wcom boards also
- Make the sandbox function static

 arch/sandbox/cpu/start.c                        | 4 +++-
 board/google/chromebook_coral/coral.c           | 7 +++++--
 board/keymile/kmcent2/kmcent2.c                 | 4 +++-
 board/keymile/pg-wcom-ls102xa/pg-wcom-ls102xa.c | 5 ++++-
 common/Kconfig                                  | 6 ------
 common/board_f.c                                | 7 +++++--
 common/event.c                                  | 3 +++
 configs/chromebook_coral_defconfig              | 1 +
 configs/kmcent2_defconfig                       | 2 +-
 configs/pg_wcom_expu1_defconfig                 | 1 +
 configs/pg_wcom_expu1_update_defconfig          | 1 +
 configs/pg_wcom_seli8_defconfig                 | 1 +
 configs/pg_wcom_seli8_update_defconfig          | 1 +
 configs/sandbox64_defconfig                     | 1 -
 configs/sandbox_defconfig                       | 1 -
 configs/sandbox_flattree_defconfig              | 1 -
 configs/sandbox_spl_defconfig                   | 1 -
 configs/tools-only_defconfig                    | 1 -
 include/configs/km/pg-wcom-ls102xa.h            | 2 --
 include/event.h                                 | 3 +++
 include/init.h                                  | 1 -
 21 files changed, 32 insertions(+), 22 deletions(-)

diff --git a/arch/sandbox/cpu/start.c b/arch/sandbox/cpu/start.c
index 12aace9a20..0f5a87309d 100644
--- a/arch/sandbox/cpu/start.c
+++ b/arch/sandbox/cpu/start.c
@@ -8,6 +8,7 @@
 #include <command.h>
 #include <efi_loader.h>
 #include <errno.h>
+#include <event.h>
 #include <init.h>
 #include <log.h>
 #include <os.h>
@@ -119,10 +120,11 @@ int sandbox_early_getopt_check(void)
 	os_exit(0);
 }
 
-int misc_init_f(void)
+static int sandbox_misc_init_f(void *ctx, struct event *event)
 {
 	return sandbox_early_getopt_check();
 }
+EVENT_SPY(EVT_MISC_INIT_F, sandbox_misc_init_f);
 
 static int sandbox_cmdline_cb_help(struct sandbox_state *state, const char *arg)
 {
diff --git a/board/google/chromebook_coral/coral.c b/board/google/chromebook_coral/coral.c
index 182cf7517a..9e23f5cd31 100644
--- a/board/google/chromebook_coral/coral.c
+++ b/board/google/chromebook_coral/coral.c
@@ -10,6 +10,7 @@
 #include <command.h>
 #include <cros_ec.h>
 #include <dm.h>
+#include <event.h>
 #include <init.h>
 #include <log.h>
 #include <sysinfo.h>
@@ -32,11 +33,12 @@ struct cros_gpio_info {
 	int flags;
 };
 
-int misc_init_f(void)
+static int coral_check_ll_boot(void *ctx, struct event *event)
 {
 	if (!ll_boot_init()) {
 		printf("Running as secondary loader");
-		if (gd->arch.coreboot_table) {
+		if (CONFIG_IS_ENABLED(COREBOOT_SYSINFO) &&
+		    gd->arch.coreboot_table) {
 			int ret;
 
 			printf(" (found coreboot table at %lx)",
@@ -55,6 +57,7 @@ int misc_init_f(void)
 
 	return 0;
 }
+EVENT_SPY(EVT_MISC_INIT_F, coral_check_ll_boot);
 
 int arch_misc_init(void)
 {
diff --git a/board/keymile/kmcent2/kmcent2.c b/board/keymile/kmcent2/kmcent2.c
index ca24b960c7..44865384f6 100644
--- a/board/keymile/kmcent2/kmcent2.c
+++ b/board/keymile/kmcent2/kmcent2.c
@@ -6,6 +6,7 @@
  * Copyright 2013 Freescale Semiconductor, Inc.
  */
 
+#include <event.h>
 #include <asm/cache.h>
 #include <asm/fsl_fdt.h>
 #include <asm/fsl_law.h>
@@ -181,7 +182,7 @@ unsigned long get_serial_clock(unsigned long dummy)
 	return (gd->bus_clk / 2);
 }
 
-int misc_init_f(void)
+static int kmcent2_misc_init_f(void *ctx, struct event *event)
 {
 	/* configure QRIO pis for i2c deblocking */
 	i2c_deblock_gpio_cfg();
@@ -209,6 +210,7 @@ int misc_init_f(void)
 
 	return 0;
 }
+EVENT_SPY(EVT_MISC_INIT_F, kmcent2_misc_init_f);
 
 #define USED_SRDS_BANK 0
 #define EXPECTED_SRDS_RFCK SRDS_PLLCR0_RFCK_SEL_100
diff --git a/board/keymile/pg-wcom-ls102xa/pg-wcom-ls102xa.c b/board/keymile/pg-wcom-ls102xa/pg-wcom-ls102xa.c
index 467f110951..ed8142d868 100644
--- a/board/keymile/pg-wcom-ls102xa/pg-wcom-ls102xa.c
+++ b/board/keymile/pg-wcom-ls102xa/pg-wcom-ls102xa.c
@@ -4,6 +4,7 @@
  */
 
 #include <common.h>
+#include <event.h>
 #include <i2c.h>
 #include <asm/io.h>
 #include <asm/arch/immap_ls102xa.h>
@@ -109,12 +110,14 @@ int board_early_init_f(void)
 	return 0;
 }
 
-int misc_init_f(void)
+static int pg_wcom_misc_init_f(void *ctx, struct event *event)
 {
 	if (IS_ENABLED(CONFIG_PG_WCOM_UBOOT_UPDATE_SUPPORTED))
 		check_for_uboot_update();
+
 	return 0;
 }
+EVENT_SPY(EVT_MISC_INIT_F, pg_wcom_misc_init_f);
 
 int board_init(void)
 {
diff --git a/common/Kconfig b/common/Kconfig
index 76c04b2001..d8deb0020e 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -583,12 +583,6 @@ config LAST_STAGE_INIT
 	  U-Boot calls last_stage_init() before the command-line interpreter is
 	  started.
 
-config MISC_INIT_F
-	bool "Execute pre-relocation misc init"
-	help
-	  Enabling this option calls the 'misc_init_f' function in the init
-	  sequence just before DRAM is inited.
-
 config MISC_INIT_R
 	bool "Execute Misc Init"
 	default y if ARCH_KEYSTONE || ARCH_SUNXI || MPC85xx
diff --git a/common/board_f.c b/common/board_f.c
index e36bdbc988..0ef34c7575 100644
--- a/common/board_f.c
+++ b/common/board_f.c
@@ -818,6 +818,11 @@ __weak int clear_bss(void)
 	return 0;
 }
 
+static int misc_init_f(void)
+{
+	return event_notify_null(EVT_MISC_INIT_F);
+}
+
 static const init_fnc_t init_sequence_f[] = {
 	setup_mon_len,
 #ifdef CONFIG_OF_CONTROL
@@ -877,9 +882,7 @@ static const init_fnc_t init_sequence_f[] = {
 	show_board_info,
 #endif
 	INIT_FUNC_WATCHDOG_INIT
-#if defined(CONFIG_MISC_INIT_F)
 	misc_init_f,
-#endif
 	INIT_FUNC_WATCHDOG_RESET
 #if CONFIG_IS_ENABLED(SYS_I2C_LEGACY)
 	init_func_i2c,
diff --git a/common/event.c b/common/event.c
index 737d3ac9ea..4270809d49 100644
--- a/common/event.c
+++ b/common/event.c
@@ -30,6 +30,9 @@ const char *const type_name[] = {
 	"dm_post_probe",
 	"dm_pre_remove",
 	"dm_post_remove",
+
+	/* init hooks */
+	"misc_init_f",
 };
 
 _Static_assert(ARRAY_SIZE(type_name) == EVT_COUNT, "event type_name size");
diff --git a/configs/chromebook_coral_defconfig b/configs/chromebook_coral_defconfig
index 0cd8f39aa3..0295acc3b4 100644
--- a/configs/chromebook_coral_defconfig
+++ b/configs/chromebook_coral_defconfig
@@ -35,6 +35,7 @@ CONFIG_USE_BOOTCOMMAND=y
 CONFIG_BOOTCOMMAND="tpm init; tpm startup TPM2_SU_CLEAR; read mmc 0:2 100000 0 80; setexpr loader *001004f0; setexpr size *00100518; setexpr blocks $size / 200; read mmc 0:2 100000 80 $blocks; setexpr setup $loader - 1000; setexpr cmdline_ptr $loader - 2000; setexpr.s cmdline *$cmdline_ptr; setexpr cmdline gsub %U \\\\${uuid}; if part uuid mmc 0:2 uuid; then zboot start 100000 0 0 0 $setup cmdline; zboot load; zboot setup; zboot dump; zboot go;fi"
 CONFIG_SYS_CONSOLE_INFO_QUIET=y
 CONFIG_DISPLAY_BOARDINFO_LATE=y
+CONFIG_EVENT=y
 CONFIG_LAST_STAGE_INIT=y
 CONFIG_BLOBLIST=y
 # CONFIG_TPL_BLOBLIST is not set
diff --git a/configs/kmcent2_defconfig b/configs/kmcent2_defconfig
index cf0748c49e..a3bd12ecb3 100644
--- a/configs/kmcent2_defconfig
+++ b/configs/kmcent2_defconfig
@@ -16,10 +16,10 @@ CONFIG_FIT=y
 CONFIG_FIT_VERBOSE=y
 CONFIG_OF_BOARD_SETUP=y
 CONFIG_OF_STDOUT_VIA_ALIAS=y
+CONFIG_EVENT=y
 CONFIG_BOARD_EARLY_INIT_F=y
 CONFIG_BOARD_EARLY_INIT_R=y
 CONFIG_LAST_STAGE_INIT=y
-CONFIG_MISC_INIT_F=y
 CONFIG_HUSH_PARSER=y
 CONFIG_CMD_DM=y
 CONFIG_CMD_I2C=y
diff --git a/configs/pg_wcom_expu1_defconfig b/configs/pg_wcom_expu1_defconfig
index f92532faec..a534f3370e 100644
--- a/configs/pg_wcom_expu1_defconfig
+++ b/configs/pg_wcom_expu1_defconfig
@@ -35,6 +35,7 @@ CONFIG_AUTOBOOT_STOP_STR=" "
 CONFIG_USE_BOOTARGS=y
 CONFIG_BOOTARGS="console=ttyS0,115200 root=/dev/ram0"
 CONFIG_SILENT_CONSOLE=y
+CONFIG_EVENT=y
 CONFIG_LAST_STAGE_INIT=y
 CONFIG_MISC_INIT_R=y
 CONFIG_CMD_IMLS=y
diff --git a/configs/pg_wcom_expu1_update_defconfig b/configs/pg_wcom_expu1_update_defconfig
index 1020b6883a..6914caa2b9 100644
--- a/configs/pg_wcom_expu1_update_defconfig
+++ b/configs/pg_wcom_expu1_update_defconfig
@@ -33,6 +33,7 @@ CONFIG_AUTOBOOT_STOP_STR=" "
 CONFIG_USE_BOOTARGS=y
 CONFIG_BOOTARGS="console=ttyS0,115200 root=/dev/ram0"
 CONFIG_SILENT_CONSOLE=y
+CONFIG_EVENT=y
 CONFIG_LAST_STAGE_INIT=y
 CONFIG_MISC_INIT_R=y
 CONFIG_CMD_IMLS=y
diff --git a/configs/pg_wcom_seli8_defconfig b/configs/pg_wcom_seli8_defconfig
index 1a2ba8c36c..16a4fe5863 100644
--- a/configs/pg_wcom_seli8_defconfig
+++ b/configs/pg_wcom_seli8_defconfig
@@ -35,6 +35,7 @@ CONFIG_AUTOBOOT_STOP_STR=" "
 CONFIG_USE_BOOTARGS=y
 CONFIG_BOOTARGS="console=ttyS0,115200 root=/dev/ram0"
 CONFIG_SILENT_CONSOLE=y
+CONFIG_EVENT=y
 CONFIG_LAST_STAGE_INIT=y
 CONFIG_MISC_INIT_R=y
 CONFIG_CMD_IMLS=y
diff --git a/configs/pg_wcom_seli8_update_defconfig b/configs/pg_wcom_seli8_update_defconfig
index 3a51d4e96c..81abce8fe3 100644
--- a/configs/pg_wcom_seli8_update_defconfig
+++ b/configs/pg_wcom_seli8_update_defconfig
@@ -33,6 +33,7 @@ CONFIG_AUTOBOOT_STOP_STR=" "
 CONFIG_USE_BOOTARGS=y
 CONFIG_BOOTARGS="console=ttyS0,115200 root=/dev/ram0"
 CONFIG_SILENT_CONSOLE=y
+CONFIG_EVENT=y
 CONFIG_LAST_STAGE_INIT=y
 CONFIG_MISC_INIT_R=y
 CONFIG_CMD_IMLS=y
diff --git a/configs/sandbox64_defconfig b/configs/sandbox64_defconfig
index 7c157a23d0..40d1422a37 100644
--- a/configs/sandbox64_defconfig
+++ b/configs/sandbox64_defconfig
@@ -23,7 +23,6 @@ CONFIG_CONSOLE_RECORD=y
 CONFIG_CONSOLE_RECORD_OUT_SIZE=0x1000
 CONFIG_PRE_CONSOLE_BUFFER=y
 CONFIG_DISPLAY_BOARDINFO_LATE=y
-CONFIG_MISC_INIT_F=y
 CONFIG_CMD_CPU=y
 CONFIG_CMD_LICENSE=y
 CONFIG_CMD_BOOTZ=y
diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
index 7ebeb89264..eaaac6d3fd 100644
--- a/configs/sandbox_defconfig
+++ b/configs/sandbox_defconfig
@@ -32,7 +32,6 @@ CONFIG_CONSOLE_RECORD_OUT_SIZE=0x1000
 CONFIG_PRE_CONSOLE_BUFFER=y
 CONFIG_LOG=y
 CONFIG_DISPLAY_BOARDINFO_LATE=y
-CONFIG_MISC_INIT_F=y
 CONFIG_STACKPROTECTOR=y
 CONFIG_ANDROID_AB=y
 CONFIG_CMD_CPU=y
diff --git a/configs/sandbox_flattree_defconfig b/configs/sandbox_flattree_defconfig
index 217b0647bb..7ccee70f42 100644
--- a/configs/sandbox_flattree_defconfig
+++ b/configs/sandbox_flattree_defconfig
@@ -20,7 +20,6 @@ CONFIG_BOOTSTAGE_STASH_SIZE=0x4096
 CONFIG_CONSOLE_RECORD=y
 CONFIG_CONSOLE_RECORD_OUT_SIZE=0x1000
 CONFIG_DISPLAY_BOARDINFO_LATE=y
-CONFIG_MISC_INIT_F=y
 CONFIG_CMD_CPU=y
 CONFIG_CMD_LICENSE=y
 CONFIG_CMD_BOOTZ=y
diff --git a/configs/sandbox_spl_defconfig b/configs/sandbox_spl_defconfig
index 1687ccf453..31f5aa8502 100644
--- a/configs/sandbox_spl_defconfig
+++ b/configs/sandbox_spl_defconfig
@@ -30,7 +30,6 @@ CONFIG_BOOTSTAGE_STASH_SIZE=0x4096
 CONFIG_CONSOLE_RECORD=y
 CONFIG_CONSOLE_RECORD_OUT_SIZE=0x1000
 CONFIG_DISPLAY_BOARDINFO_LATE=y
-CONFIG_MISC_INIT_F=y
 CONFIG_HANDOFF=y
 CONFIG_SPL_BOARD_INIT=y
 CONFIG_SPL_ENV_SUPPORT=y
diff --git a/configs/tools-only_defconfig b/configs/tools-only_defconfig
index 64eb766515..211acc7774 100644
--- a/configs/tools-only_defconfig
+++ b/configs/tools-only_defconfig
@@ -9,7 +9,6 @@ CONFIG_TIMESTAMP=y
 CONFIG_FIT_SIGNATURE=y
 CONFIG_USE_BOOTCOMMAND=y
 CONFIG_BOOTCOMMAND="run distro_bootcmd"
-CONFIG_MISC_INIT_F=y
 # CONFIG_CMD_BOOTD is not set
 # CONFIG_CMD_BOOTM is not set
 # CONFIG_CMD_ELF is not set
diff --git a/include/configs/km/pg-wcom-ls102xa.h b/include/configs/km/pg-wcom-ls102xa.h
index 8453be8495..9d7a9e18d5 100644
--- a/include/configs/km/pg-wcom-ls102xa.h
+++ b/include/configs/km/pg-wcom-ls102xa.h
@@ -274,6 +274,4 @@
 #define CONFIG_SYS_BOOTM_LEN	(64 << 20) /* Increase max gunzip size */
 #define CONFIG_SYS_BOOTMAPSZ	(256 << 20) /* Increase map for Linux */
 
-#define CONFIG_MISC_INIT_F
-
 #endif
diff --git a/include/event.h b/include/event.h
index f4c12d768b..6b347e92f0 100644
--- a/include/event.h
+++ b/include/event.h
@@ -25,6 +25,9 @@ enum event_t {
 	EVT_DM_PRE_REMOVE,
 	EVT_DM_POST_REMOVE,
 
+	/* Init hooks */
+	EVT_MISC_INIT_F,
+
 	EVT_COUNT
 };
 
diff --git a/include/init.h b/include/init.h
index 20c3976af0..c03b29bb0d 100644
--- a/include/init.h
+++ b/include/init.h
@@ -217,7 +217,6 @@ int init_cache_f_r(void);
 int print_cpuinfo(void);
 #endif
 int timer_init(void);
-int misc_init_f(void);
 
 #if defined(CONFIG_DTB_RESELECT)
 int embedded_dtb_select(void);
-- 
2.35.1.616.g0bdcbb4464-goog

[PATCH v2 10/13] event: Convert arch_cpu_init_dm() to use events

[Simon Glass]

Instead of a special function, send an event after driver model is inited
and adjust the boards which use this function.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2:
- Convert arch/arm/mach-imx/imx8/cpu.c also
- Add DM_EVENT to ARCH_IMX8 too
- Tidy up galileo defconfig and rename quark_init_dm()
- Make a few more of the functions static
- Add a weak function for spl

 arch/Kconfig                        |  3 +++
 arch/arm/Kconfig                    |  4 ++++
 arch/arm/mach-imx/imx8/cpu.c        |  4 +++-
 arch/arm/mach-imx/imx8m/soc.c       |  4 +++-
 arch/arm/mach-imx/imx8ulp/soc.c     |  4 +++-
 arch/arm/mach-omap2/am33xx/board.c  |  4 +++-
 arch/arm/mach-omap2/hwinit-common.c |  5 ++++-
 arch/mips/Kconfig                   |  1 +
 arch/mips/mach-pic32/cpu.c          |  4 +++-
 arch/nios2/cpu/cpu.c                |  4 +++-
 arch/riscv/cpu/cpu.c                |  5 ++++-
 arch/riscv/include/asm/system.h     |  5 +++++
 arch/riscv/lib/spl.c                |  3 ++-
 arch/x86/cpu/baytrail/cpu.c         |  4 +++-
 arch/x86/cpu/broadwell/cpu.c        |  4 +++-
 arch/x86/cpu/ivybridge/cpu.c        |  4 +++-
 arch/x86/cpu/quark/quark.c          |  4 +++-
 arch/x86/include/asm/fsp2/fsp_api.h |  8 ++++++++
 arch/x86/lib/fsp2/fsp_init.c        |  4 +++-
 arch/x86/lib/spl.c                  |  5 +++--
 arch/x86/lib/tpl.c                  | 10 ----------
 common/board_f.c                    |  6 ------
 common/event.c                      |  1 +
 drivers/core/root.c                 |  5 +++++
 include/event.h                     |  1 +
 include/init.h                      | 11 -----------
 26 files changed, 74 insertions(+), 43 deletions(-)

diff --git a/arch/Kconfig b/arch/Kconfig
index e6191446a3..1b35fda64c 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -94,6 +94,7 @@ config NIOS2
 	bool "Nios II architecture"
 	select CPU
 	select DM
+	imply DM_EVENT
 	select OF_CONTROL
 	select SUPPORT_OF_CONTROL
 	imply CMD_DM
@@ -113,6 +114,7 @@ config RISCV
 	select DM
 	imply DM_SERIAL
 	imply DM_ETH
+	imply DM_EVENT
 	imply DM_MMC
 	imply DM_SPI
 	imply DM_SPI_FLASH
@@ -238,6 +240,7 @@ config X86
 	imply CMD_SF_TEST
 	imply CMD_ZBOOT
 	imply DM_ETH
+	imply DM_EVENT
 	imply DM_GPIO
 	imply DM_KEYBOARD
 	imply DM_MMC
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 391a77c2b4..0b04804289 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -767,6 +767,7 @@ config ARCH_OMAP2PLUS
 	select SUPPORT_SPL
 	imply TI_SYSC if DM && OF_CONTROL
 	imply FIT
+	imply DM_EVENT
 
 config ARCH_MESON
 	bool "Amlogic Meson"
@@ -811,6 +812,7 @@ config ARCH_IMX8
 	select MACH_IMX
 	select OF_CONTROL
 	select ENABLE_ARM_SOC_BOOT0_HOOK
+	imply DM_EVENT
 
 config ARCH_IMX8M
 	bool "NXP i.MX8M platform"
@@ -824,6 +826,7 @@ config ARCH_IMX8M
 	select DM
 	select SUPPORT_SPL
 	imply CMD_DM
+	imply DM_EVENT
 
 config ARCH_IMX8ULP
 	bool "NXP i.MX8ULP platform"
@@ -834,6 +837,7 @@ config ARCH_IMX8ULP
 	select SUPPORT_SPL
 	select GPIO_EXTRA_HEADER
 	imply CMD_DM
+	imply DM_EVENT
 
 config ARCH_IMXRT
 	bool "NXP i.MXRT platform"
diff --git a/arch/arm/mach-imx/imx8/cpu.c b/arch/arm/mach-imx/imx8/cpu.c
index ee5cc47903..359f8c796e 100644
--- a/arch/arm/mach-imx/imx8/cpu.c
+++ b/arch/arm/mach-imx/imx8/cpu.c
@@ -8,6 +8,7 @@
 #include <cpu.h>
 #include <cpu_func.h>
 #include <dm.h>
+#include <event.h>
 #include <init.h>
 #include <log.h>
 #include <asm/cache.h>
@@ -66,7 +67,7 @@ int arch_cpu_init(void)
 	return 0;
 }
 
-int arch_cpu_init_dm(void)
+static int imx8_init_mu(void *ctx, struct event *event)
 {
 	struct udevice *devp;
 	int node, ret;
@@ -88,6 +89,7 @@ int arch_cpu_init_dm(void)
 
 	return 0;
 }
+EVENT_SPY(EVT_DM_POST_INIT, imx8_init_mu);
 
 int print_bootinfo(void)
 {
diff --git a/arch/arm/mach-imx/imx8m/soc.c b/arch/arm/mach-imx/imx8m/soc.c
index 1a5a391443..838f0a3749 100644
--- a/arch/arm/mach-imx/imx8m/soc.c
+++ b/arch/arm/mach-imx/imx8m/soc.c
@@ -7,6 +7,7 @@
 
 #include <common.h>
 #include <cpu_func.h>
+#include <event.h>
 #include <init.h>
 #include <log.h>
 #include <asm/arch/imx-regs.h>
@@ -494,7 +495,7 @@ static void imx_set_wdog_powerdown(bool enable)
 	writew(enable, &wdog3->wmcr);
 }
 
-int arch_cpu_init_dm(void)
+static int imx8m_check_clock(void *ctx, struct event *event)
 {
 	struct udevice *dev;
 	int ret;
@@ -511,6 +512,7 @@ int arch_cpu_init_dm(void)
 
 	return 0;
 }
+EVENT_SPY(EVT_DM_POST_INIT, imx8m_check_clock);
 
 int arch_cpu_init(void)
 {
diff --git a/arch/arm/mach-imx/imx8ulp/soc.c b/arch/arm/mach-imx/imx8ulp/soc.c
index 934b0ef038..e6d417ed48 100644
--- a/arch/arm/mach-imx/imx8ulp/soc.c
+++ b/arch/arm/mach-imx/imx8ulp/soc.c
@@ -11,6 +11,7 @@
 #include <asm/mach-imx/boot_mode.h>
 #include <asm/global_data.h>
 #include <efi_loader.h>
+#include <event.h>
 #include <spl.h>
 #include <asm/arch/rdc.h>
 #include <asm/arch/s400_api.h>
@@ -569,7 +570,7 @@ int arch_cpu_init(void)
 	return 0;
 }
 
-int arch_cpu_init_dm(void)
+static int imx8ulp_check_mu(void *ctx, struct event *event)
 {
 	struct udevice *devp;
 	int node, ret;
@@ -584,6 +585,7 @@ int arch_cpu_init_dm(void)
 
 	return 0;
 }
+EVENT_SPY(EVT_DM_POST_INIT, imx8ulp_check_mu);
 
 #if defined(CONFIG_SPL_BUILD)
 __weak void __noreturn jump_to_image_no_args(struct spl_image_info *spl_image)
diff --git a/arch/arm/mach-omap2/am33xx/board.c b/arch/arm/mach-omap2/am33xx/board.c
index c44667668e..bcc907ce36 100644
--- a/arch/arm/mach-omap2/am33xx/board.c
+++ b/arch/arm/mach-omap2/am33xx/board.c
@@ -11,6 +11,7 @@
 #include <dm.h>
 #include <debug_uart.h>
 #include <errno.h>
+#include <event.h>
 #include <init.h>
 #include <net.h>
 #include <ns16550.h>
@@ -596,7 +597,7 @@ void board_init_f(ulong dummy)
 
 #endif
 
-int arch_cpu_init_dm(void)
+static int am33xx_dm_post_init(void *ctx, struct event *event)
 {
 	hw_data_init();
 #if !CONFIG_IS_ENABLED(SKIP_LOWLEVEL_INIT)
@@ -604,3 +605,4 @@ int arch_cpu_init_dm(void)
 #endif
 	return 0;
 }
+EVENT_SPY(EVT_DM_POST_INIT, am33xx_dm_post_init);
diff --git a/arch/arm/mach-omap2/hwinit-common.c b/arch/arm/mach-omap2/hwinit-common.c
index 3da50f974d..c4a8eabc3e 100644
--- a/arch/arm/mach-omap2/hwinit-common.c
+++ b/arch/arm/mach-omap2/hwinit-common.c
@@ -12,6 +12,7 @@
  */
 #include <common.h>
 #include <debug_uart.h>
+#include <event.h>
 #include <fdtdec.h>
 #include <init.h>
 #include <spl.h>
@@ -239,11 +240,13 @@ void board_init_f(ulong dummy)
 }
 #endif
 
-int arch_cpu_init_dm(void)
+static int omap2_system_init(void *ctx, struct event *event)
 {
 	early_system_init();
+
 	return 0;
 }
+EVENT_SPY(EVT_DM_POST_INIT, omap2_system_init);
 
 /*
  * Routine: wait_for_command_complete
diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index 28234aa0bb..06cae68ee5 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -130,6 +130,7 @@ config MACH_PIC32
 config TARGET_BOSTON
 	bool "Support Boston"
 	select DM
+	imply DM_EVENT
 	select DM_SERIAL
 	select MIPS_CM
 	select SYS_CACHE_SHIFT_6
diff --git a/arch/mips/mach-pic32/cpu.c b/arch/mips/mach-pic32/cpu.c
index eac2fe5f8c..de449e3c6a 100644
--- a/arch/mips/mach-pic32/cpu.c
+++ b/arch/mips/mach-pic32/cpu.c
@@ -7,6 +7,7 @@
 #include <common.h>
 #include <clk.h>
 #include <dm.h>
+#include <event.h>
 #include <init.h>
 #include <malloc.h>
 #include <asm/global_data.h>
@@ -95,12 +96,13 @@ static void prefetch_init(void)
 }
 
 /* arch specific CPU init after DM */
-int arch_cpu_init_dm(void)
+static int pic32_flash_prefetch(void *ctx, struct event *event)
 {
 	/* flash prefetch */
 	prefetch_init();
 	return 0;
 }
+EVENT_SPY(EVT_DM_POST_INIT, pic32_flash_prefetch);
 
 /* Un-gate DDR2 modules (gated by default) */
 static void ddr2_pmd_ungate(void)
diff --git a/arch/nios2/cpu/cpu.c b/arch/nios2/cpu/cpu.c
index b55c8fbc58..4dd9c10faa 100644
--- a/arch/nios2/cpu/cpu.c
+++ b/arch/nios2/cpu/cpu.c
@@ -10,6 +10,7 @@
 #include <cpu_func.h>
 #include <dm.h>
 #include <errno.h>
+#include <event.h>
 #include <init.h>
 #include <irq_func.h>
 #include <asm/cache.h>
@@ -63,7 +64,7 @@ static void copy_exception_trampoline(void)
 }
 #endif
 
-int arch_cpu_init_dm(void)
+static int nios_cpu_setup(void *ctx, struct event *event)
 {
 	struct udevice *dev;
 	int ret;
@@ -79,6 +80,7 @@ int arch_cpu_init_dm(void)
 
 	return 0;
 }
+EVENT_SPY(EVT_DM_POST_INIT, nios_cpu_setup);
 
 static int altera_nios2_get_desc(const struct udevice *dev, char *buf,
 				 int size)
diff --git a/arch/riscv/cpu/cpu.c b/arch/riscv/cpu/cpu.c
index 8d90c5e6b8..3ffcbbd23f 100644
--- a/arch/riscv/cpu/cpu.c
+++ b/arch/riscv/cpu/cpu.c
@@ -7,9 +7,11 @@
 #include <cpu.h>
 #include <dm.h>
 #include <dm/lists.h>
+#include <event.h>
 #include <init.h>
 #include <log.h>
 #include <asm/encoding.h>
+#include <asm/system.h>
 #include <dm/uclass-internal.h>
 #include <linux/bitops.h>
 
@@ -81,7 +83,7 @@ static void dummy_pending_ipi_clear(ulong hart, ulong arg0, ulong arg1)
 }
 #endif
 
-int arch_cpu_init_dm(void)
+int riscv_cpu_setup(void *ctx, struct event *event)
 {
 	int ret;
 
@@ -133,6 +135,7 @@ int arch_cpu_init_dm(void)
 
 	return 0;
 }
+EVENT_SPY(EVT_DM_POST_INIT, riscv_cpu_setup);
 
 int arch_early_init_r(void)
 {
diff --git a/arch/riscv/include/asm/system.h b/arch/riscv/include/asm/system.h
index a340475823..9d8e43e394 100644
--- a/arch/riscv/include/asm/system.h
+++ b/arch/riscv/include/asm/system.h
@@ -7,6 +7,8 @@
 #ifndef __ASM_RISCV_SYSTEM_H
 #define __ASM_RISCV_SYSTEM_H
 
+struct event;
+
 /*
  * Interrupt configuring macros.
  *
@@ -14,4 +16,7 @@
  *
  */
 
+/* Hook to set up the CPU (called from SPL too) */
+int riscv_cpu_setup(void *ctx, struct event *event);
+
 #endif	/* __ASM_RISCV_SYSTEM_H */
diff --git a/arch/riscv/lib/spl.c b/arch/riscv/lib/spl.c
index 8baee07bea..f4d3b67e5d 100644
--- a/arch/riscv/lib/spl.c
+++ b/arch/riscv/lib/spl.c
@@ -11,6 +11,7 @@
 #include <spl.h>
 #include <asm/global_data.h>
 #include <asm/smp.h>
+#include <asm/system.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -27,7 +28,7 @@ __weak void board_init_f(ulong dummy)
 	if (ret)
 		panic("spl_early_init() failed: %d\n", ret);
 
-	arch_cpu_init_dm();
+	riscv_cpu_setup(NULL, NULL);
 
 	preloader_console_init();
 
diff --git a/arch/x86/cpu/baytrail/cpu.c b/arch/x86/cpu/baytrail/cpu.c
index 309a50a116..68bf40ba8e 100644
--- a/arch/x86/cpu/baytrail/cpu.c
+++ b/arch/x86/cpu/baytrail/cpu.c
@@ -8,6 +8,7 @@
 #include <common.h>
 #include <cpu.h>
 #include <dm.h>
+#include <event.h>
 #include <init.h>
 #include <log.h>
 #include <pci.h>
@@ -44,7 +45,7 @@ static void hsuart_clock_set(void *base)
  * Configure the internal clock of both SIO HS-UARTs, if they are enabled
  * via FSP
  */
-int arch_cpu_init_dm(void)
+static int baytrail_uart_init(void *ctx, struct event *event)
 {
 	struct udevice *dev;
 	void *base;
@@ -63,6 +64,7 @@ int arch_cpu_init_dm(void)
 
 	return 0;
 }
+EVENT_SPY(EVT_DM_POST_INIT, baytrail_uart_init);
 
 static void set_max_freq(void)
 {
diff --git a/arch/x86/cpu/broadwell/cpu.c b/arch/x86/cpu/broadwell/cpu.c
index 3832a97f2c..2adcf4b242 100644
--- a/arch/x86/cpu/broadwell/cpu.c
+++ b/arch/x86/cpu/broadwell/cpu.c
@@ -8,6 +8,7 @@
 #include <common.h>
 #include <dm.h>
 #include <cpu.h>
+#include <event.h>
 #include <init.h>
 #include <log.h>
 #include <asm/cpu.h>
@@ -24,7 +25,7 @@
 #include <asm/arch/pch.h>
 #include <asm/arch/rcb.h>
 
-int arch_cpu_init_dm(void)
+static int broadwell_init_cpu(void *ctx, struct event *event)
 {
 	struct udevice *dev;
 	int ret;
@@ -41,6 +42,7 @@ int arch_cpu_init_dm(void)
 
 	return 0;
 }
+EVENT_SPY(EVT_DM_POST_INIT, broadwell_init_cpu);
 
 void set_max_freq(void)
 {
diff --git a/arch/x86/cpu/ivybridge/cpu.c b/arch/x86/cpu/ivybridge/cpu.c
index a02f4f9600..cffc5d5b1d 100644
--- a/arch/x86/cpu/ivybridge/cpu.c
+++ b/arch/x86/cpu/ivybridge/cpu.c
@@ -14,6 +14,7 @@
 #include <cpu_func.h>
 #include <dm.h>
 #include <errno.h>
+#include <event.h>
 #include <fdtdec.h>
 #include <init.h>
 #include <log.h>
@@ -53,7 +54,7 @@ int arch_cpu_init(void)
 	return x86_cpu_init_f();
 }
 
-int arch_cpu_init_dm(void)
+static int ivybridge_cpu_init(void *ctx, struct event *ev)
 {
 	struct pci_controller *hose;
 	struct udevice *bus, *dev;
@@ -85,6 +86,7 @@ int arch_cpu_init_dm(void)
 
 	return 0;
 }
+EVENT_SPY(EVT_DM_POST_INIT, ivybridge_cpu_init);
 
 #define PCH_EHCI0_TEMP_BAR0 0xe8000000
 #define PCH_EHCI1_TEMP_BAR0 0xe8000400
diff --git a/arch/x86/cpu/quark/quark.c b/arch/x86/cpu/quark/quark.c
index 30b4711b9a..e016fae04f 100644
--- a/arch/x86/cpu/quark/quark.c
+++ b/arch/x86/cpu/quark/quark.c
@@ -5,6 +5,7 @@
 
 #include <common.h>
 #include <cpu_func.h>
+#include <event.h>
 #include <init.h>
 #include <mmc.h>
 #include <asm/cache.h>
@@ -247,7 +248,7 @@ int arch_cpu_init(void)
 	return 0;
 }
 
-int arch_cpu_init_dm(void)
+static int quark_init_pcie(void *ctx, struct event *event)
 {
 	/*
 	 * Initialize PCIe controller
@@ -262,6 +263,7 @@ int arch_cpu_init_dm(void)
 
 	return 0;
 }
+EVENT_SPY(EVT_DM_POST_INIT, quark_init_pcie);
 
 int checkcpu(void)
 {
diff --git a/arch/x86/include/asm/fsp2/fsp_api.h b/arch/x86/include/asm/fsp2/fsp_api.h
index dccbfa45a1..ca3f6848b6 100644
--- a/arch/x86/include/asm/fsp2/fsp_api.h
+++ b/arch/x86/include/asm/fsp2/fsp_api.h
@@ -60,4 +60,12 @@ int fsp_silicon_init(bool s3wake, bool use_spi_flash);
 
 typedef asmlinkage int (*fsp_silicon_init_func)(struct fsps_upd *params);
 
+/**
+ *  fsp_setup_pinctrl() - Set up the pinctrl for FSP
+ *
+ * @ctx: Event context (not used)
+ * @event: Event information (not used)
+ */
+int fsp_setup_pinctrl(void *ctx, struct event *event);
+
 #endif
diff --git a/arch/x86/lib/fsp2/fsp_init.c b/arch/x86/lib/fsp2/fsp_init.c
index 5afdce1e0d..b15926e824 100644
--- a/arch/x86/lib/fsp2/fsp_init.c
+++ b/arch/x86/lib/fsp2/fsp_init.c
@@ -9,6 +9,7 @@
 #include <bootstage.h>
 #include <cbfs.h>
 #include <dm.h>
+#include <event.h>
 #include <init.h>
 #include <log.h>
 #include <spi.h>
@@ -18,7 +19,7 @@
 #include <dm/uclass-internal.h>
 #include <asm/fsp2/fsp_internal.h>
 
-int arch_cpu_init_dm(void)
+int fsp_setup_pinctrl(void *ctx, struct event *event)
 {
 	struct udevice *dev;
 	ofnode node;
@@ -41,6 +42,7 @@ int arch_cpu_init_dm(void)
 
 	return ret;
 }
+EVENT_SPY(EVT_DM_POST_INIT, fsp_setup_pinctrl);
 
 #if !defined(CONFIG_TPL_BUILD)
 binman_sym_declare(ulong, intel_fsp_m, image_pos);
diff --git a/arch/x86/lib/spl.c b/arch/x86/lib/spl.c
index b18c1cd609..2d50c62964 100644
--- a/arch/x86/lib/spl.c
+++ b/arch/x86/lib/spl.c
@@ -17,6 +17,7 @@
 #include <syscon.h>
 #include <asm/cpu.h>
 #include <asm/cpu_common.h>
+#include <asm/fsp2/fsp_api.h>
 #include <asm/global_data.h>
 #include <asm/mrccache.h>
 #include <asm/mtrr.h>
@@ -27,7 +28,7 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
-__weak int arch_cpu_init_dm(void)
+__weak int fsp_setup_pinctrl(void *ctx, struct event *event)
 {
 	return 0;
 }
@@ -89,7 +90,7 @@ static int x86_spl_init(void)
 		return ret;
 	}
 #ifndef CONFIG_TPL
-	ret = arch_cpu_init_dm();
+	ret = fsp_setup_pinctrl(NULL, NULL);
 	if (ret) {
 		debug("%s: arch_cpu_init_dm() failed\n", __func__);
 		return ret;
diff --git a/arch/x86/lib/tpl.c b/arch/x86/lib/tpl.c
index 5b57e53c2d..18b05b2f67 100644
--- a/arch/x86/lib/tpl.c
+++ b/arch/x86/lib/tpl.c
@@ -19,11 +19,6 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
-__weak int arch_cpu_init_dm(void)
-{
-	return 0;
-}
-
 static int x86_tpl_init(void)
 {
 	int ret;
@@ -44,11 +39,6 @@ static int x86_tpl_init(void)
 		debug("%s: arch_cpu_init() failed\n", __func__);
 		return ret;
 	}
-	ret = arch_cpu_init_dm();
-	if (ret) {
-		debug("%s: arch_cpu_init_dm() failed\n", __func__);
-		return ret;
-	}
 	preloader_console_init();
 
 	return 0;
diff --git a/common/board_f.c b/common/board_f.c
index 0ef34c7575..5b655ad6ef 100644
--- a/common/board_f.c
+++ b/common/board_f.c
@@ -803,11 +803,6 @@ __weak int reserve_arch(void)
 	return 0;
 }
 
-__weak int arch_cpu_init_dm(void)
-{
-	return 0;
-}
-
 __weak int checkcpu(void)
 {
 	return 0;
@@ -848,7 +843,6 @@ static const init_fnc_t init_sequence_f[] = {
 	arch_cpu_init,		/* basic arch cpu dependent setup */
 	mach_cpu_init,		/* SoC/machine dependent CPU setup */
 	initf_dm,
-	arch_cpu_init_dm,
 #if defined(CONFIG_BOARD_EARLY_INIT_F)
 	board_early_init_f,
 #endif
diff --git a/common/event.c b/common/event.c
index 4270809d49..9d67a060a0 100644
--- a/common/event.c
+++ b/common/event.c
@@ -26,6 +26,7 @@ const char *const type_name[] = {
 	"test",
 
 	/* Events related to driver model */
+	"dm_post_init",
 	"dm_pre_probe",
 	"dm_post_probe",
 	"dm_pre_remove",
diff --git a/drivers/core/root.c b/drivers/core/root.c
index e3f87956d8..8efb4256b2 100644
--- a/drivers/core/root.c
+++ b/drivers/core/root.c
@@ -404,6 +404,11 @@ int dm_init_and_scan(bool pre_reloc_only)
 			return ret;
 		}
 	}
+	if (CONFIG_IS_ENABLED(DM_EVENT)) {
+		ret = event_notify_null(EVT_DM_POST_INIT);
+		if (ret)
+			return log_msg_ret("ev", ret);
+	}
 
 	return 0;
 }
diff --git a/include/event.h b/include/event.h
index 6b347e92f0..78a42374ac 100644
--- a/include/event.h
+++ b/include/event.h
@@ -20,6 +20,7 @@ enum event_t {
 	EVT_TEST,
 
 	/* Events related to driver model */
+	EVT_DM_POST_INIT,
 	EVT_DM_PRE_PROBE,
 	EVT_DM_POST_PROBE,
 	EVT_DM_PRE_REMOVE,
diff --git a/include/init.h b/include/init.h
index c03b29bb0d..74496500d2 100644
--- a/include/init.h
+++ b/include/init.h
@@ -45,17 +45,6 @@ void board_init_f(ulong dummy);
  */
 int arch_cpu_init(void);
 
-/**
- * arch_cpu_init_dm() - init CPU after driver model is available
- *
- * This is called immediately after driver model is available before
- * relocation. This is similar to arch_cpu_init() but is able to reference
- * devices
- *
- * Return: 0 if OK, -ve on error
- */
-int arch_cpu_init_dm(void);
-
 /**
  * mach_cpu_init() - SoC/machine dependent CPU setup
  *
-- 
2.35.1.616.g0bdcbb4464-goog

[PATCH v2 11/13] event: Add a command

[Simon Glass]

Add a command to show the available events.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 MAINTAINERS     |  1 +
 cmd/Kconfig     |  8 ++++++++
 cmd/Makefile    |  1 +
 cmd/event.c     | 27 +++++++++++++++++++++++++++
 include/event.h |  3 +++
 5 files changed, 40 insertions(+)
 create mode 100644 cmd/event.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 2786adad4b..6e5c022138 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -812,6 +812,7 @@ F:	scripts/env2string.awk
 EVENTS
 M:	Simon Glass <sjg@chromium.org>
 S:	Maintained
+F:	cmd/event.c
 F:	common/event.c
 F:	include/event.h
 F:	test/common/event.c
diff --git a/cmd/Kconfig b/cmd/Kconfig
index 5e25e45fd2..52bd825d02 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -2342,6 +2342,14 @@ config CMD_DIAG
 	  available tests and running either all the tests, or specific tests
 	  identified by name.
 
+config CMD_EVENT
+	bool "event - Show information about events"
+	default y if EVENT_DEBUG
+	help
+	  This enables the 'event' command which provides information about
+	  events and event-handler routines. This can help to device event
+	  hadling.
+
 config CMD_IRQ
 	bool "irq - Show information about interrupts"
 	depends on !ARM && !MIPS && !RISCV && !SH
diff --git a/cmd/Makefile b/cmd/Makefile
index 166c652d98..0d2b2ee092 100644
--- a/cmd/Makefile
+++ b/cmd/Makefile
@@ -53,6 +53,7 @@ obj-$(CONFIG_CMD_DIAG) += diag.o
 endif
 obj-$(CONFIG_CMD_ADTIMG) += adtimg.o
 obj-$(CONFIG_CMD_ABOOTIMG) += abootimg.o
+obj-$(CONFIG_CMD_EVENT) += event.o
 obj-$(CONFIG_CMD_EXTENSION) += extension_board.o
 obj-$(CONFIG_CMD_ECHO) += echo.o
 obj-$(CONFIG_ENV_IS_IN_EEPROM) += eeprom.o
diff --git a/cmd/event.c b/cmd/event.c
new file mode 100644
index 0000000000..9cac202353
--- /dev/null
+++ b/cmd/event.c
@@ -0,0 +1,27 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Command-line access to events
+ *
+ * Copyright 2021 Google LLC
+ * Written by Simon Glass <sjg@chromium.org>
+ */
+
+#include <common.h>
+#include <command.h>
+#include <event.h>
+
+static int do_event_list(struct cmd_tbl *cmdtp, int flag, int argc,
+			 char *const argv[])
+{
+	event_show_spy_list();
+
+	return 0;
+}
+
+#ifdef CONFIG_SYS_LONGHELP
+static char event_help_text[] =
+	"event list   - list event spies";
+#endif
+
+U_BOOT_CMD_WITH_SUBCMDS(event, "Events", event_help_text,
+	U_BOOT_SUBCMD_MKENT(list, 1, 1, do_event_list));
diff --git a/include/event.h b/include/event.h
index 78a42374ac..62e72a7bd3 100644
--- a/include/event.h
+++ b/include/event.h
@@ -141,6 +141,9 @@ static inline const char *event_spy_id(struct evspy_info *spy)
 int event_register(const char *id, enum event_t type, event_handler_t func,
 		   void *ctx);
 
+/** event_show_spy_list( - Show a list of event spies */
+void event_show_spy_list(void);
+
 #if CONFIG_IS_ENABLED(EVENT)
 /**
  * event_notify() - notify spies about an event
-- 
2.35.1.616.g0bdcbb4464-goog

[PATCH v2 12/13] event: Add a script to decode the event-spy list

[Simon Glass]

For debugging and dicoverability it is useful to be able to see a list of
each event spy in a U-Boot ELF file. Add a script which shows this, along
with the event type and the source location. This makes events a little
easier to use than weak functions, for example.

Add a basic sandbox test as well. We could provide a test for other
boards, but for now, few use events.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2:
- Update for patman snake-case change
- Use a regular expression in the test to avoid dependency on build dir

 MAINTAINERS                      |   2 +
 scripts/event_dump.py            | 115 +++++++++++++++++++++++++++++++
 test/py/tests/test_event_dump.py |  20 ++++++
 3 files changed, 137 insertions(+)
 create mode 100755 scripts/event_dump.py
 create mode 100644 test/py/tests/test_event_dump.py

diff --git a/MAINTAINERS b/MAINTAINERS
index 6e5c022138..7012cc2b86 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -815,7 +815,9 @@ S:	Maintained
 F:	cmd/event.c
 F:	common/event.c
 F:	include/event.h
+F:	scripts/event_dump.py
 F:	test/common/event.c
+F:	test/py/tests/test_event_dump.py
 
 FASTBOOT
 S:	Orphaned
diff --git a/scripts/event_dump.py b/scripts/event_dump.py
new file mode 100755
index 0000000000..751f41b183
--- /dev/null
+++ b/scripts/event_dump.py
@@ -0,0 +1,115 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: GPL-2.0+
+
+"""Decode the evspy_info linker list in a U-Boot ELF image"""
+
+from argparse import ArgumentParser
+import os
+import re
+import struct
+import sys
+
+our_path = os.path.dirname(os.path.realpath(__file__))
+src_path = os.path.dirname(our_path)
+
+sys.path.insert(1, os.path.join(our_path, '../tools'))
+
+from binman import elf
+from patman import tools
+
+PREFIX = '_u_boot_list_2_evspy_info_2_'
+RE_EVTYPE = re.compile('%s(.*)' % PREFIX)
+
+def show_sym(fname, data, endian, evtype, sym):
+    """Show information about an evspy entry
+
+    Args:
+        fname (str): Filename of ELF file
+        data (bytes): Data for this symbol
+        endian (str): Endianness to use ('little', 'big', 'auto')
+        evtype (str): Event type, e.g. 'MISC_INIT_F'
+        sym (elf.Symbol): Symbol to show
+    """
+    def _unpack_val(sym_data, offset):
+        start = offset * func_size
+        val_data = sym_data[start:start + func_size]
+        fmt = '%s%s' % ('>' if endian == 'big' else '<',
+                        'L' if func_size == 4 else 'Q')
+        val = struct.unpack(fmt, val_data)[0]
+        return val
+
+    # Get the data, which is a struct evspy_info
+    sym_data = data[sym.offset:sym.offset + sym.size]
+
+    # Figure out the word size of the struct
+    func_size = 4 if sym.size < 16 else 8
+
+    # Read the function name for evspy_info->func
+    while True:
+        # Switch to big-endian if we see a failure
+        func_addr = _unpack_val(sym_data, 0)
+        func_name = elf.GetSymbolFromAddress(fname, func_addr)
+        if not func_name and endian == 'auto':
+            endian = 'big'
+        else:
+            break
+    has_id = sym.size in [12, 24]
+    if has_id:
+        # Find the address of evspy_info->id in the ELF
+        id_addr = _unpack_val(sym_data, 2)
+
+        # Get the file offset for that address
+        id_ofs = elf.GetFileOffset(fname, id_addr)
+
+        # Read out a nul-terminated string
+        id_data = data[id_ofs:id_ofs + 80]
+        pos = id_data.find(0)
+        if pos:
+            id_data = id_data[:pos]
+        id_str = id_data.decode('utf-8')
+    else:
+        id_str = None
+
+    # Find the file/line for the function
+    cmd = ['addr2line', '-e', fname, '%x' % func_addr]
+    out = tools.run(*cmd).strip()
+
+    # Drop the full path if it is the current directory
+    if out.startswith(src_path):
+        out = out[len(src_path) + 1:]
+    print('%-20s  %-30s  %s' % (evtype, id_str or f'f:{func_name}', out))
+
+def show_event_spy_list(fname, endian):
+    """Show a the event-spy- list from a U-Boot image
+
+    Args:
+        fname (str): Filename of ELF file
+        endian (str): Endianness to use ('little', 'big', 'auto')
+    """
+    syms = elf.GetSymbolFileOffset(fname, [PREFIX])
+    data = tools.read_file(fname)
+    print('%-20s  %-30s  %s' % ('Event type', 'Id', 'Source location'))
+    print('%-20s  %-30s  %s' % ('-' * 20, '-' * 30, '-' * 30))
+    for name, sym in syms.items():
+        m_evtype = RE_EVTYPE.search(name)
+        evtype = m_evtype .group(1)
+        show_sym(fname, data, endian, evtype, sym)
+
+def main(argv):
+    """Main program
+
+    Args:
+        argv (list of str): List of program arguments, excluding arvg[0]
+    """
+    epilog = 'Show a list of even spies in a U-Boot EFL file'
+    parser = ArgumentParser(epilog=epilog)
+    parser.add_argument('elf', type=str, help='ELF file to decode')
+    parser.add_argument('-e', '--endian', type=str, default='auto',
+                        help='Big-endian image')
+    parser.add_argument('-t', '--test', action='store_true',
+                        help='Big-endian image')
+    args = parser.parse_args(argv)
+    show_event_spy_list(args.elf, args.endian)
+
+if __name__ == "__main__":
+    main(sys.argv[1:])
diff --git a/test/py/tests/test_event_dump.py b/test/py/tests/test_event_dump.py
new file mode 100644
index 0000000000..b753e804ac
--- /dev/null
+++ b/test/py/tests/test_event_dump.py
@@ -0,0 +1,20 @@
+# SPDX-License-Identifier: GPL-2.0+
+# Copyright 2021 Google LLC
+# Written by Simon Glass <sjg@chromium.org>
+
+import pytest
+import re
+import u_boot_utils as util
+
+# This is only a partial test - coverting 64-bit sandbox. It does not test
+# big-endian images, nor 32-bit images
+@pytest.mark.boardspec('sandbox')
+def test_event_dump(u_boot_console):
+    """Test that the "help" command can be executed."""
+    cons = u_boot_console
+    sandbox = cons.config.build_dir + '/u-boot'
+    out = util.run_and_log(cons, ['scripts/event_dump.py', sandbox])
+    expect = '''.*Event type            Id                              Source location
+--------------------  ------------------------------  ------------------------------
+EVT_MISC_INIT_F       sandbox_misc_init_f             .*arch/sandbox/cpu/start.c:'''
+    assert re.match(expect, out, re.MULTILINE) is not None
-- 
2.35.1.616.g0bdcbb4464-goog

[PATCH v2 13/13] event: Add documentation

[Simon Glass]

Add documentation for events, including the event command.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 doc/develop/event.rst | 66 +++++++++++++++++++++++++++++++++++++++++++
 doc/develop/index.rst |  1 +
 doc/usage/event.rst   | 49 ++++++++++++++++++++++++++++++++
 doc/usage/index.rst   |  1 +
 4 files changed, 117 insertions(+)
 create mode 100644 doc/develop/event.rst
 create mode 100644 doc/usage/event.rst

diff --git a/doc/develop/event.rst b/doc/develop/event.rst
new file mode 100644
index 0000000000..6e144cfcdd
--- /dev/null
+++ b/doc/develop/event.rst
@@ -0,0 +1,66 @@
+.. SPDX-License-Identifier: GPL-2.0+
+
+Events
+======
+
+U-Boot supports a way for various events to be handled by interested
+subsystems. This provide a generic way to handle 'hooks' like setting up the
+CPUs after driver model is active, or reading a partition table after a new
+block device is probed.
+
+Rather than using weak functions and direct calls across subsystemss, it is
+often easier to use an event.
+
+An event consists of a type (e.g. EVT_DM_POST_INIT) and some optional data,
+in `union event_data`. An event spy can be creasted to watch for events of a
+particular type. When the event is created, it is sent to each spy in turn.
+
+
+Declaring a spy
+---------------
+
+To declare a spy, use something like this::
+
+    static int snow_setup_cpus(void *ctx, struct event *event)
+    {
+        /* do something */
+        return 0;
+    }
+    EVENT_SPY(EVT_DM_POST_INIT, snow_setup_cpus);
+
+Your function is called when EVT_DM_POST_INIT is emitted, i.e. after driver
+model is inited (in SPL, or in U-Boot proper before and after relocation).
+
+
+Debugging
+---------
+
+To assist with debugging events, enable `CONFIG_EVENT_DEBUG` and
+`CONFIG_CMD_EVENT`. The :doc:`../usage/event` command can then be used to
+provide a spy list.
+
+It is also possible to list spy information from the U-Boot executable,, using
+the `event_dump.py` script::
+
+    $ scripts/event_dump.py /tmp/b/sandbox/u-boot
+    Event type            Id                              Source location
+    --------------------  ------------------------------  ------------------------------
+    EVT_MISC_INIT_F       f:sandbox_misc_init_f           arch/sandbox/cpu/start.c:125
+
+This shows each event spy in U-Boot, along with the event type, function name
+(or ID) and source location.
+
+Note that if `CONFIG_EVENT_DEBUG` is not enabled, the event ID is missing, so
+the function is shown instead (with an `f:` prefix as above). Since the ID is
+generally the same as the function name, this does not matter much.
+
+The event type is decoded by the symbol used by U-Boot for the event linker
+list. Symbols have the form::
+
+    _u_boot_list_2_evspy_info_2_EVT_MISC_INIT_F
+
+so the event type can be read from the end. To manually list spy information
+in an image, use $(CROSS_COMPILE)nm::
+
+    nm u-boot |grep evspy |grep list
+    00000000002d6300 D _u_boot_list_2_evspy_info_2_EVT_MISC_INIT_F
diff --git a/doc/develop/index.rst b/doc/develop/index.rst
index 93ebfa485f..2e6d6c302a 100644
--- a/doc/develop/index.rst
+++ b/doc/develop/index.rst
@@ -17,6 +17,7 @@ Implementation
    distro
    driver-model/index
    environment
+   event
    global_data
    logging
    makefiles
diff --git a/doc/usage/event.rst b/doc/usage/event.rst
new file mode 100644
index 0000000000..c0f8acd727
--- /dev/null
+++ b/doc/usage/event.rst
@@ -0,0 +1,49 @@
+.. SPDX-License-Identifier: GPL-2.0+
+
+event command
+=============
+
+Synopsis
+--------
+
+::
+
+    event list
+
+Description
+-----------
+
+The event command provides spy list.
+
+This shows the following information:
+
+Seq
+    Sequence number of the spy, numbered from 0
+
+Type
+    Type of the spy, both as a number and a label. If `CONFIG_EVENT_DEBUG` is
+    not enabled, the label just shows `(unknown)`.
+
+Function
+    Address of the function to call
+
+ID
+    ID string for this event, if `CONFIG_EVENT_DEBUG` is enabled. Otherwise this
+    just shows `?`.
+
+
+See :doc:`../develop/event` for more information on events.
+
+Example
+-------
+
+::
+
+    => event list
+    Seq  Type                              Function  ID
+      0  7   misc_init_f               55a070517c68  ?
+
+Configuration
+-------------
+
+The event command is only available if CONFIG_CMD_EVENT=y.
diff --git a/doc/usage/index.rst b/doc/usage/index.rst
index 0aacf531b2..750102830b 100644
--- a/doc/usage/index.rst
+++ b/doc/usage/index.rst
@@ -29,6 +29,7 @@ Shell commands
    x86/cbsysinfo
    conitrace
    echo
+   event
    exception
    extension
    exit
-- 
2.35.1.616.g0bdcbb4464-goog

Re: [PATCH v2 05/13] event: Add basic support for events

[AKASHI Takahiro]

Hi Simon,

On Fri, Mar 04, 2022 at 08:43:00AM -0700, Simon Glass wrote:
> Add a way to create and dispatch events without needing to allocate
> memory. Also add a way to 'spy' on events, thus allowing 'hooks' to be
> created.
> 
> Use a linker list for static events, which we can use to replace functions
> like arch_cpu_init_f(). Allow an EVENT_DEBUG option which makes it
> easier to see what is going on at runtime, but uses more code space.
> 
> Dynamic events allow the creation of a spy at runtime. This is not always
> necessary, but can be enabled with EVENT_DYNAMIC if needed.
> 
> A 'test' event is the only option for now.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
> Changes in v2:
> - Add a 'used' attribute to avoid LTO dropping the code
> 
>  MAINTAINERS                       |   6 +
>  common/Kconfig                    |  31 +++++
>  common/Makefile                   |   2 +
>  common/board_r.c                  |   1 +
>  common/event.c                    | 186 +++++++++++++++++++++++++++++
>  common/log.c                      |   1 +
>  include/asm-generic/global_data.h |  13 +++
>  include/event.h                   | 188 ++++++++++++++++++++++++++++++
>  include/event_internal.h          |  35 ++++++
>  include/log.h                     |   2 +
>  10 files changed, 465 insertions(+)
>  create mode 100644 common/event.c
>  create mode 100644 include/event.h
>  create mode 100644 include/event_internal.h
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index fb171e0c68..b534ad66b9 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -809,6 +809,12 @@ S:	Maintained
>  F:	doc/usage/environment.rst
>  F:	scripts/env2string.awk
>  
> +EVENTS
> +M:	Simon Glass <sjg@chromium.org>
> +S:	Maintained
> +F:	common/event.c
> +F:	include/event.h
> +
>  FASTBOOT
>  S:	Orphaned
>  F:	cmd/fastboot.c
> diff --git a/common/Kconfig b/common/Kconfig
> index 82cd864baf..76c04b2001 100644
> --- a/common/Kconfig
> +++ b/common/Kconfig
> @@ -492,6 +492,37 @@ config DISPLAY_BOARDINFO_LATE
>  
>  menu "Start-up hooks"
>  
> +config EVENT
> +	bool "General-purpose event-handling mechanism"

I don't think that this config option needs to be visible (or user-selectable).
Instead, any subsystem that needs it should explicitly select it.
I prefer that subsystem can add "select EVENT or DM_EVENT" rather than
"imply" or "depends on".

-Takahiro Akashi

> +	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.
> +
> +if EVENT
> +
> +config EVENT_DYNAMIC
> +	bool "Support event registration at runtime"
> +	default y if SANDBOX
> +	help
> +	  Enable this to support adding an event spy at runtime, without adding
> +	  it to the EVENT_SPy() linker list. This increases code size slightly
> +	  but provides more flexibility for boards and subsystems that need it.
> +
> +config EVENT_DEBUG
> +	bool "Enable event debugging assistance"
> +	default y if SANDBOX
> +	help
> +	  Enable this get usefui features for seeing what is happening with
> +	  events, such as event-type names. This adds to the code size of
> +	  U-Boot so can be turned off for production builds.
> +
> +endif # EVENT
> +
>  config ARCH_EARLY_INIT_R
>  	bool "Call arch-specific init soon after relocation"
>  	help
> diff --git a/common/Makefile b/common/Makefile
> index 3eff719601..cc2ba30c63 100644
> --- a/common/Makefile
> +++ b/common/Makefile
> @@ -89,6 +89,8 @@ obj-y += malloc_simple.o
>  endif
>  endif
>  
> +obj-$(CONFIG_$(SPL_TPL_)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_r.c b/common/board_r.c
> index c24d9b4e22..b92c1bb0be 100644
> --- a/common/board_r.c
> +++ b/common/board_r.c
> @@ -594,6 +594,7 @@ static int run_main_loop(void)
>  static init_fnc_t init_sequence_r[] = {
>  	initr_trace,
>  	initr_reloc,
> +	event_init,
>  	/* TODO: could x86/PPC have this also perhaps? */
>  #if defined(CONFIG_ARM) || defined(CONFIG_RISCV)
>  	initr_caches,
> diff --git a/common/event.c b/common/event.c
> new file mode 100644
> index 0000000000..366be24569
> --- /dev/null
> +++ b/common/event.c
> @@ -0,0 +1,186 @@
> +// 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 <linker_lists.h>
> +#include <malloc.h>
> +#include <asm/global_data.h>
> +#include <linux/list.h>
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +#if CONFIG_IS_ENABLED(EVENT_DEBUG)
> +const char *const type_name[] = {
> +	"none",
> +	"test",
> +};
> +
> +_Static_assert(ARRAY_SIZE(type_name) == EVT_COUNT, "event type_name size");
> +#endif
> +
> +static const char *event_type_name(enum event_t type)
> +{
> +#if CONFIG_IS_ENABLED(EVENT_DEBUG)
> +	return type_name[type];
> +#else
> +	return "(unknown)";
> +#endif
> +}
> +
> +static int notify_static(struct event *ev)
> +{
> +	struct evspy_info *start =
> +		ll_entry_start(struct evspy_info, evspy_info);
> +	const int n_ents = ll_entry_count(struct evspy_info, evspy_info);
> +	struct evspy_info *spy;
> +
> +	for (spy = start; spy != start + n_ents; spy++) {
> +		if (spy->type == ev->type) {
> +			int ret;
> +
> +			log_debug("Sending event %x/%s to spy '%s'\n", ev->type,
> +				  event_type_name(ev->type), event_spy_id(spy));
> +			ret = spy->func(NULL, ev);
> +
> +			/*
> +			 * 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;
> +}
> +
> +static int notify_dynamic(struct event *ev)
> +{
> +	struct event_state *state = gd_event_state();
> +	struct event_spy *spy, *next;
> +
> +	list_for_each_entry_safe(spy, next, &state->spy_head, sibling_node) {
> +		if (spy->type == ev->type) {
> +			int ret;
> +
> +			log_debug("Sending event %x/%s to spy '%s'\n", ev->type,
> +				  event_type_name(ev->type), spy->id);
> +			ret = spy->func(spy->ctx, ev);
> +
> +			/*
> +			 * 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_notify(enum event_t type, void *data, int size)
> +{
> +	struct event event;
> +	int ret;
> +
> +	event.type = type;
> +	if (size > sizeof(event.data))
> +		return log_msg_ret("size", -E2BIG);
> +	memcpy(&event.data, data, size);
> +
> +	ret = notify_static(&event);
> +	if (ret)
> +		return log_msg_ret("dyn", ret);
> +
> +	if (CONFIG_IS_ENABLED(EVENT_DYNAMIC)) {
> +		ret = notify_dynamic(&event);
> +		if (ret)
> +			return log_msg_ret("dyn", ret);
> +	}
> +
> +	return 0;
> +}
> +
> +int event_notify_null(enum event_t type)
> +{
> +	return event_notify(type, NULL, 0);
> +}
> +
> +void event_show_spy_list(void)
> +{
> +	struct evspy_info *start =
> +		ll_entry_start(struct evspy_info, evspy_info);
> +	const int n_ents = ll_entry_count(struct evspy_info, evspy_info);
> +	struct evspy_info *spy;
> +	const int size = sizeof(ulong) * 2;
> +
> +	printf("Seq  %-24s  %*s  %s\n", "Type", size, "Function", "ID");
> +	for (spy = start; spy != start + n_ents; spy++) {
> +		printf("%3x  %-3x %-20s  %*p  %s\n", (uint)(spy - start),
> +		       spy->type, event_type_name(spy->type), size, spy->func,
> +		       event_spy_id(spy));
> +	}
> +}
> +
> +#if CONFIG_IS_ENABLED(EVENT_DYNAMIC)
> +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;
> +
> +	if (!CONFIG_IS_ENABLED(EVENT_DYNAMIC))
> +		return -ENOSYS;
> +	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_uninit(void)
> +{
> +	struct event_state *state = gd_event_state();
> +	struct event_spy *spy, *next;
> +
> +	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 = gd_event_state();
> +
> +	INIT_LIST_HEAD(&state->spy_head);
> +
> +	return 0;
> +}
> +#endif /* EVENT_DYNAMIC */
> diff --git a/common/log.c b/common/log.c
> index f7e0c0fbf5..7254aa70bf 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/include/asm-generic/global_data.h b/include/asm-generic/global_data.h
> index c2f8fad1cb..e49f5bf2f7 100644
> --- a/include/asm-generic/global_data.h
> +++ b/include/asm-generic/global_data.h
> @@ -20,6 +20,7 @@
>   */
>  
>  #ifndef __ASSEMBLY__
> +#include <event_internal.h>
>  #include <fdtdec.h>
>  #include <membuff.h>
>  #include <linux/list.h>
> @@ -467,6 +468,12 @@ struct global_data {
>  	 */
>  	char *smbios_version;
>  #endif
> +#if CONFIG_IS_ENABLED(EVENT)
> +	/**
> +	 * @event_state: Points to the current state of events
> +	 */
> +	struct event_state event_state;
> +#endif
>  };
>  #ifndef DO_DEPS_ONLY
>  static_assert(sizeof(struct global_data) == GD_SIZE);
> @@ -532,6 +539,12 @@ static_assert(sizeof(struct global_data) == GD_SIZE);
>  #define gd_set_multi_dtb_fit(_dtb)
>  #endif
>  
> +#if CONFIG_IS_ENABLED(EVENT_DYNAMIC)
> +#define gd_event_state()	((struct event_state *)&gd->event_state)
> +#else
> +#define gd_event_state()	NULL
> +#endif
> +
>  /**
>   * enum gd_flags - global data flags
>   *
> diff --git a/include/event.h b/include/event.h
> new file mode 100644
> index 0000000000..effd58c704
> --- /dev/null
> +++ b/include/event.h
> @@ -0,0 +1,188 @@
> +/* 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,
> +
> +	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 - 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);
> +
> +/**
> + * struct evspy_info - information about an event spy
> + *
> + * @func: Function to call when the event is activated (must be first)
> + * @type: Event type
> + * @id: Event id string
> + */
> +struct evspy_info {
> +	event_handler_t func;
> +	enum event_t type;
> +#if CONFIG_IS_ENABLED(EVENT_DEBUG)
> +	const char *id;
> +#endif
> +};
> +
> +/* Declare a new event spy */
> +#if CONFIG_IS_ENABLED(EVENT_DEBUG)
> +#define _ESPY_REC(_type, _func)   { _func, _type, #_func, }
> +#else
> +#define _ESPY_REC(_type, _func)   { _func, _type, }
> +#endif
> +
> +static inline const char *event_spy_id(struct evspy_info *spy)
> +{
> +#if CONFIG_IS_ENABLED(EVENT_DEBUG)
> +	return spy->id;
> +#else
> +	return "?";
> +#endif
> +}
> +
> +/*
> + * It seems that LTO will drop list entries if it decides they are not used,
> + * although the conditions that cause this are unclear.
> + *
> + * The example found is the following:
> + *
> + * static int sandbox_misc_init_f(void *ctx, struct event *event)
> + * {
> + *    return sandbox_early_getopt_check();
> + * }
> + * EVENT_SPY(EVT_MISC_INIT_F, sandbox_misc_init_f);
> + *
> + * where EVENT_SPY uses ll_entry_declare()
> + *
> + * In this case, LTO decides to drop the sandbox_misc_init_f() function
> + * (which is fine) but then drops the linker-list entry too. This means
> + * that the code no longer works, in this case sandbox no-longer checks its
> + * command-line arguments properly.
> + *
> + * Without LTO, the KEEP() command in the .lds file is enough to keep the
> + * entry around. But with LTO it seems that the entry has already been
> + * dropped before the link script is considered.
> + *
> + * The only solution I can think of is to mark linker-list entries as 'used'
> + * using an attribute. This should be safe, since we don't actually want to drop
> + * any of these. However this does slightly limit LTO's optimisation choices.
> + */
> +#define EVENT_SPY(_type, _func) \
> +	static __attribute__((used)) ll_entry_declare(struct evspy_info, \
> +						      _type, evspy_info) = \
> +	_ESPY_REC(_type, _func)
> +
> +/**
> + * 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 error
> + */
> +int event_register(const char *id, enum event_t type, event_handler_t func,
> +		   void *ctx);
> +
> +#if CONFIG_IS_ENABLED(EVENT)
> +/**
> + * 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
> + * @return 0 if OK, -ve on error
> + */
> +int event_notify(enum event_t type, void *data, int size);
> +
> +/**
> + * event_notify_null() - notify spies about an event
> + *
> + * Data is NULL and the size is 0
> + *
> + * @type: Event type
> + * @return 0 if OK, -ve on error
> + */
> +int event_notify_null(enum event_t type);
> +#else
> +static inline int event_notify(enum event_t type, void *data, int size)
> +{
> +	return 0;
> +}
> +
> +static inline int event_notify_null(enum event_t type)
> +{
> +	return 0;
> +}
> +#endif
> +
> +#if CONFIG_IS_ENABLED(EVENT_DYNAMIC)
> +/**
> + * event_uninit() - Clean up dynamic events
> + *
> + * This removes all dynamic event handlers
> + */
> +int event_uninit(void);
> +
> +/**
> + * event_uninit() - Set up dynamic events
> + *
> + * Init a list of dynamic event handlers, so that these can be added as
> + * needed
> + */
> +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 0000000000..8432c6f0e5
> --- /dev/null
> +++ b/include/event_internal.h
> @@ -0,0 +1,35 @@
> +/* 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 <event.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 ce48d51446..8f35c10abb 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 */
> -- 
> 2.35.1.616.g0bdcbb4464-goog
> 

Re: [PATCH v2 02/13] Makefile: Allow LTO to be disabled for a build

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 991 bytes --]

On Fri, Mar 04, 2022 at 08:42:57AM -0700, Simon Glass wrote:

> LTO (Link-Time Optimisation) is an very useful feature which can
> significantly reduce the size of U-Boot binaries. So far it has been
> made available for selected ARM boards and sandbox.
> 
> However, incremental builds are much slower when LTO is used. For example,
> an incremental build of sandbox takes 2.1 seconds on my machine, but 6.7
> seconds with LTO enabled.
> 
> Add a LTO_BUILD=n parameter to the build, so it can be disabled during
> development if needed, for faster builds.
> 
> Add some documentation about LTO while we are here.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

We don't need this since you can do:
make EXTRA_CFLAGS="-fno-lto" EXTRA_LDFLAGS="-fno-lto"
to pass -fno-lto to compile/linking and disable lto and per
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=46905 this has been working
for some time.

Not that you need to respin the series for this.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH v2 00/13] event: Provide support for events to connect subsystems

[Heinrich Schuchardt]

On 3/4/22 16:42, Simon Glass wrote:
> It is a common need in U-Boot to have one subsystem notify another
> when something happens. An example is reading a partition table when a
> new block device is set up.
>
> It is also common to add weak functions and 'hook' functions to modify
> how U-Boot works. See for example ft_board_setup() and the like.
>
> U-Boot would benefit from a generic mechanism to handle these cases,
> with the ability to hook into various 'events' in a
> subsystem-independent and transparent way.
>
> This series provides a way to create and dispatch events, with a way of
> registering a 'spy' which watches for events of different types. This
> allows 'hook' functions to be created in a generic way.
>
> It also includes a script to list the hooks in an image, which is a bit
> easier to debug than weak functions, as well as an 'event' command to
> do the same from within U-Boot.
>
> These 'static' events can be used to replace hooks like misc_init_f(),
> for example. Also included is basic support for 'dynamic' events, where
> a spy can be registered at runtime. The need for this is still being
> figured out.

@Simon, Tom:

What is the status of this series? Takahiro's UEFI integration series
builds on it. Is it going to be pushed to origin/next soon?

Best regards

Heinrich

>
> Changes in v2:
> - Add a 'used' attribute to avoid LTO dropping the code
> - Update keymile pg-wcom boards also
> - Make the sandbox function static
> - Convert arch/arm/mach-imx/imx8/cpu.c also
> - Add DM_EVENT to ARCH_IMX8 too
> - Tidy up galileo defconfig and rename quark_init_dm()
> - Make a few more of the functions static
> - Add a weak function for spl
> - Update for patman snake-case change
> - Use a regular expression in the test to avoid dependency on build dir
>
> Simon Glass (12):
>    Makefile: Allow LTO to be disabled for a build
>    sandbox: start: Sort the header files
>    binman: Expand elf support a little
>    event: Add basic support for events
>    event: Add a simple test
>    event: Set up the event system on start-up
>    event: Add events for device probe/remove
>    event: Convert misc_init_f() to use events
>    event: Convert arch_cpu_init_dm() to use events
>    event: Add a command
>    event: Add a script to decode the event-spy list
>    event: Add documentation
>
> Tim Harvey (1):
>    phy: nop-phy: Fix phy reset if no reset-gpio defined
>
>   MAINTAINERS                                   |  10 +
>   Makefile                                      |  18 +-
>   arch/Kconfig                                  |   3 +
>   arch/arm/Kconfig                              |   4 +
>   arch/arm/config.mk                            |   4 +-
>   arch/arm/include/asm/global_data.h            |   2 +-
>   arch/arm/mach-imx/imx8/cpu.c                  |   4 +-
>   arch/arm/mach-imx/imx8m/soc.c                 |   4 +-
>   arch/arm/mach-imx/imx8ulp/soc.c               |   4 +-
>   arch/arm/mach-omap2/am33xx/board.c            |   4 +-
>   arch/arm/mach-omap2/hwinit-common.c           |   5 +-
>   arch/mips/Kconfig                             |   1 +
>   arch/mips/mach-pic32/cpu.c                    |   4 +-
>   arch/nios2/cpu/cpu.c                          |   4 +-
>   arch/riscv/cpu/cpu.c                          |   5 +-
>   arch/riscv/include/asm/system.h               |   5 +
>   arch/riscv/lib/spl.c                          |   3 +-
>   arch/sandbox/cpu/start.c                      |   8 +-
>   arch/x86/cpu/baytrail/cpu.c                   |   4 +-
>   arch/x86/cpu/broadwell/cpu.c                  |   4 +-
>   arch/x86/cpu/ivybridge/cpu.c                  |   4 +-
>   arch/x86/cpu/quark/quark.c                    |   4 +-
>   arch/x86/include/asm/fsp2/fsp_api.h           |   8 +
>   arch/x86/lib/fsp2/fsp_init.c                  |   4 +-
>   arch/x86/lib/spl.c                            |   5 +-
>   arch/x86/lib/tpl.c                            |  10 -
>   board/google/chromebook_coral/coral.c         |   7 +-
>   board/keymile/kmcent2/kmcent2.c               |   4 +-
>   .../keymile/pg-wcom-ls102xa/pg-wcom-ls102xa.c |   5 +-
>   cmd/Kconfig                                   |   8 +
>   cmd/Makefile                                  |   1 +
>   cmd/event.c                                   |  27 +++
>   common/Kconfig                                |  37 ++-
>   common/Makefile                               |   2 +
>   common/board_f.c                              |  13 +-
>   common/board_r.c                              |   1 +
>   common/event.c                                | 196 ++++++++++++++++
>   common/log.c                                  |   1 +
>   configs/chromebook_coral_defconfig            |   1 +
>   configs/kmcent2_defconfig                     |   2 +-
>   configs/pg_wcom_expu1_defconfig               |   1 +
>   configs/pg_wcom_expu1_update_defconfig        |   1 +
>   configs/pg_wcom_seli8_defconfig               |   1 +
>   configs/pg_wcom_seli8_update_defconfig        |   1 +
>   configs/sandbox64_defconfig                   |   1 -
>   configs/sandbox_defconfig                     |   1 -
>   configs/sandbox_flattree_defconfig            |   1 -
>   configs/sandbox_spl_defconfig                 |   1 -
>   configs/tools-only_defconfig                  |   1 -
>   doc/build/gcc.rst                             |  17 ++
>   doc/develop/event.rst                         |  66 ++++++
>   doc/develop/index.rst                         |   1 +
>   doc/usage/event.rst                           |  49 ++++
>   doc/usage/index.rst                           |   1 +
>   drivers/core/Kconfig                          |  10 +
>   drivers/core/device-remove.c                  |   8 +
>   drivers/core/device.c                         |   9 +
>   drivers/core/root.c                           |   5 +
>   drivers/phy/nop-phy.c                         |  12 +-
>   include/asm-generic/global_data.h             |  13 ++
>   include/configs/km/pg-wcom-ls102xa.h          |   2 -
>   include/dm/device-internal.h                  |  10 +
>   include/event.h                               | 210 ++++++++++++++++++
>   include/event_internal.h                      |  35 +++
>   include/init.h                                |  12 -
>   include/log.h                                 |   2 +
>   scripts/event_dump.py                         | 115 ++++++++++
>   test/common/Makefile                          |   1 +
>   test/common/event.c                           |  85 +++++++
>   test/py/tests/test_event_dump.py              |  20 ++
>   test/test-main.c                              |   4 +
>   tools/binman/elf.py                           |  58 ++++-
>   72 files changed, 1108 insertions(+), 86 deletions(-)
>   create mode 100644 cmd/event.c
>   create mode 100644 common/event.c
>   create mode 100644 doc/develop/event.rst
>   create mode 100644 doc/usage/event.rst
>   create mode 100644 include/event.h
>   create mode 100644 include/event_internal.h
>   create mode 100755 scripts/event_dump.py
>   create mode 100644 test/common/event.c
>   create mode 100644 test/py/tests/test_event_dump.py
>

Re: [PATCH v2 01/13] phy: nop-phy: Fix phy reset if no reset-gpio defined

[Heinrich Schuchardt]

On 3/4/22 16:42, Simon Glass wrote:
> From: Tim Harvey<tharvey@gateworks.com>
>
> Ensure there is a valid reset-gpio defined before using it.
>
> Fixes: f9852acdce02 ("phy: nop-phy: Fix enabling reset")
> Cc: Adam Ford<aford173@gmail.com>
> Signed-off-by: Tim Harvey<tharvey@gateworks.com>
> Signed-off-by: Simon Glass<sjg@chromium.org>

This patch is already in origin/master.

Re: [PATCH v2 00/13] event: Provide support for events to connect subsystems

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1544 bytes --]

On Tue, Mar 08, 2022 at 02:11:02PM +0100, Heinrich Schuchardt wrote:
> On 3/4/22 16:42, Simon Glass wrote:
> > It is a common need in U-Boot to have one subsystem notify another
> > when something happens. An example is reading a partition table when a
> > new block device is set up.
> > 
> > It is also common to add weak functions and 'hook' functions to modify
> > how U-Boot works. See for example ft_board_setup() and the like.
> > 
> > U-Boot would benefit from a generic mechanism to handle these cases,
> > with the ability to hook into various 'events' in a
> > subsystem-independent and transparent way.
> > 
> > This series provides a way to create and dispatch events, with a way of
> > registering a 'spy' which watches for events of different types. This
> > allows 'hook' functions to be created in a generic way.
> > 
> > It also includes a script to list the hooks in an image, which is a bit
> > easier to debug than weak functions, as well as an 'event' command to
> > do the same from within U-Boot.
> > 
> > These 'static' events can be used to replace hooks like misc_init_f(),
> > for example. Also included is basic support for 'dynamic' events, where
> > a spy can be registered at runtime. The need for this is still being
> > figured out.
> 
> @Simon, Tom:
> 
> What is the status of this series? Takahiro's UEFI integration series
> builds on it. Is it going to be pushed to origin/next soon?

I'm waiting for Simon to reply to Takahiro's comment on v2 before
applying.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH v2 05/13] event: Add basic support for events

[Simon Glass]

Hi Takahiro,

On Sun, 6 Mar 2022 at 21:26, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote:
>
> Hi Simon,
>
> On Fri, Mar 04, 2022 at 08:43:00AM -0700, Simon Glass wrote:
> > Add a way to create and dispatch events without needing to allocate
> > memory. Also add a way to 'spy' on events, thus allowing 'hooks' to be
> > created.
> >
> > Use a linker list for static events, which we can use to replace functions
> > like arch_cpu_init_f(). Allow an EVENT_DEBUG option which makes it
> > easier to see what is going on at runtime, but uses more code space.
> >
> > Dynamic events allow the creation of a spy at runtime. This is not always
> > necessary, but can be enabled with EVENT_DYNAMIC if needed.
> >
> > A 'test' event is the only option for now.
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> >
> > Changes in v2:
> > - Add a 'used' attribute to avoid LTO dropping the code
> >
> >  MAINTAINERS                       |   6 +
> >  common/Kconfig                    |  31 +++++
> >  common/Makefile                   |   2 +
> >  common/board_r.c                  |   1 +
> >  common/event.c                    | 186 +++++++++++++++++++++++++++++
> >  common/log.c                      |   1 +
> >  include/asm-generic/global_data.h |  13 +++
> >  include/event.h                   | 188 ++++++++++++++++++++++++++++++
> >  include/event_internal.h          |  35 ++++++
> >  include/log.h                     |   2 +
> >  10 files changed, 465 insertions(+)
> >  create mode 100644 common/event.c
> >  create mode 100644 include/event.h
> >  create mode 100644 include/event_internal.h
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index fb171e0c68..b534ad66b9 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -809,6 +809,12 @@ S:       Maintained
> >  F:   doc/usage/environment.rst
> >  F:   scripts/env2string.awk
> >
> > +EVENTS
> > +M:   Simon Glass <sjg@chromium.org>
> > +S:   Maintained
> > +F:   common/event.c
> > +F:   include/event.h
> > +
> >  FASTBOOT
> >  S:   Orphaned
> >  F:   cmd/fastboot.c
> > diff --git a/common/Kconfig b/common/Kconfig
> > index 82cd864baf..76c04b2001 100644
> > --- a/common/Kconfig
> > +++ b/common/Kconfig
> > @@ -492,6 +492,37 @@ config DISPLAY_BOARDINFO_LATE
> >
> >  menu "Start-up hooks"
> >
> > +config EVENT
> > +     bool "General-purpose event-handling mechanism"
>
> I don't think that this config option needs to be visible (or user-selectable).
> Instead, any subsystem that needs it should explicitly select it.
> I prefer that subsystem can add "select EVENT or DM_EVENT" rather than
> "imply" or "depends on".

But events can be added for other reasons. For example a board may
want to detect that a USB controller is about to be probed and enable
power to devices on that USB bus so that the devices are enumerated.
This series is partly about replacing all the various hooks we
currently have but goes further than that.

[..]

Regards,
Simon

Re: [PATCH v2 00/13] event: Provide support for events to connect subsystems

[Simon Glass]

Hi Tom,

On Tue, 8 Mar 2022 at 06:26, Tom Rini <trini@konsulko.com> wrote:
>
> On Tue, Mar 08, 2022 at 02:11:02PM +0100, Heinrich Schuchardt wrote:
> > On 3/4/22 16:42, Simon Glass wrote:
> > > It is a common need in U-Boot to have one subsystem notify another
> > > when something happens. An example is reading a partition table when a
> > > new block device is set up.
> > >
> > > It is also common to add weak functions and 'hook' functions to modify
> > > how U-Boot works. See for example ft_board_setup() and the like.
> > >
> > > U-Boot would benefit from a generic mechanism to handle these cases,
> > > with the ability to hook into various 'events' in a
> > > subsystem-independent and transparent way.
> > >
> > > This series provides a way to create and dispatch events, with a way of
> > > registering a 'spy' which watches for events of different types. This
> > > allows 'hook' functions to be created in a generic way.
> > >
> > > It also includes a script to list the hooks in an image, which is a bit
> > > easier to debug than weak functions, as well as an 'event' command to
> > > do the same from within U-Boot.
> > >
> > > These 'static' events can be used to replace hooks like misc_init_f(),
> > > for example. Also included is basic support for 'dynamic' events, where
> > > a spy can be registered at runtime. The need for this is still being
> > > figured out.
> >
> > @Simon, Tom:
> >
> > What is the status of this series? Takahiro's UEFI integration series
> > builds on it. Is it going to be pushed to origin/next soon?
>
> I'm waiting for Simon to reply to Takahiro's comment on v2 before
> applying.

OK I found it and replied.

Regards,
Simon

Re: [PATCH v2 03/13] sandbox: start: Sort the header files

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 228 bytes --]

On Fri, Mar 04, 2022 at 08:42:58AM -0700, Simon Glass wrote:

> These header files don't follow the correct order. Fix this.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/next, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH v2 04/13] binman: Expand elf support a little

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 328 bytes --]

On Fri, Mar 04, 2022 at 08:42:59AM -0700, Simon Glass wrote:

> Allow finding a symbol by its address. Also export the function to get
> the file offset of a particular address, so it can be used by a script to
> be added.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/next, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH v2 05/13] event: Add basic support for events

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 739 bytes --]

On Fri, Mar 04, 2022 at 08:43:00AM -0700, Simon Glass wrote:

> Add a way to create and dispatch events without needing to allocate
> memory. Also add a way to 'spy' on events, thus allowing 'hooks' to be
> created.
> 
> Use a linker list for static events, which we can use to replace functions
> like arch_cpu_init_f(). Allow an EVENT_DEBUG option which makes it
> easier to see what is going on at runtime, but uses more code space.
> 
> Dynamic events allow the creation of a spy at runtime. This is not always
> necessary, but can be enabled with EVENT_DYNAMIC if needed.
> 
> A 'test' event is the only option for now.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/next, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH v2 06/13] event: Add a simple test

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 217 bytes --]

On Fri, Mar 04, 2022 at 08:43:01AM -0700, Simon Glass wrote:

> Add a test for event registration and activation.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/next, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH v2 07/13] event: Set up the event system on start-up

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 236 bytes --]

On Fri, Mar 04, 2022 at 08:43:02AM -0700, Simon Glass wrote:

> Call event_init() before relocation to get the event system running.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/next, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH v2 08/13] event: Add events for device probe/remove

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 328 bytes --]

On Fri, Mar 04, 2022 at 08:43:03AM -0700, Simon Glass wrote:

> Generate events when devices are probed or removed, allowing hooks
> to be added for these situations.
> 
> This is controlled by the DM_EVENT config option.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/next, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH v2 09/13] event: Convert misc_init_f() to use events

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 469 bytes --]

On Fri, Mar 04, 2022 at 08:43:04AM -0700, Simon Glass wrote:

> This hook can be implmented using events, for the three boards that
> actually use it.
> 
> Add the event type and event handlers. Drop CONFIG_MISC_INIT_F since we
> can just use CONFIG_EVENT to control this. Since sandbox always enables
> CONFIG_EVENT, we can drop the defconfig lines there too.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/next, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH v2 10/13] event: Convert arch_cpu_init_dm() to use events

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 291 bytes --]

On Fri, Mar 04, 2022 at 08:43:05AM -0700, Simon Glass wrote:

> Instead of a special function, send an event after driver model is inited
> and adjust the boards which use this function.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/next, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH v2 11/13] event: Add a command

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 211 bytes --]

On Fri, Mar 04, 2022 at 08:43:06AM -0700, Simon Glass wrote:

> Add a command to show the available events.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/next, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH v2 12/13] event: Add a script to decode the event-spy list

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 559 bytes --]

On Fri, Mar 04, 2022 at 08:43:07AM -0700, Simon Glass wrote:

> For debugging and dicoverability it is useful to be able to see a list of
> each event spy in a U-Boot ELF file. Add a script which shows this, along
> with the event type and the source location. This makes events a little
> easier to use than weak functions, for example.
> 
> Add a basic sandbox test as well. We could provide a test for other
> boards, but for now, few use events.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/next, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH v2 13/13] event: Add documentation

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 226 bytes --]

On Fri, Mar 04, 2022 at 08:43:08AM -0700, Simon Glass wrote:

> Add documentation for events, including the event command.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/next, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH v2 02/13] Makefile: Allow LTO to be disabled for a build

[Simon Glass]

Hi Tom,

On Mon, 7 Mar 2022 at 07:33, Tom Rini <trini@konsulko.com> wrote:
>
> On Fri, Mar 04, 2022 at 08:42:57AM -0700, Simon Glass wrote:
>
> > LTO (Link-Time Optimisation) is an very useful feature which can
> > significantly reduce the size of U-Boot binaries. So far it has been
> > made available for selected ARM boards and sandbox.
> >
> > However, incremental builds are much slower when LTO is used. For example,
> > an incremental build of sandbox takes 2.1 seconds on my machine, but 6.7
> > seconds with LTO enabled.
> >
> > Add a LTO_BUILD=n parameter to the build, so it can be disabled during
> > development if needed, for faster builds.
> >
> > Add some documentation about LTO while we are here.
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
>
> We don't need this since you can do:
> make EXTRA_CFLAGS="-fno-lto" EXTRA_LDFLAGS="-fno-lto"
> to pass -fno-lto to compile/linking and disable lto and per
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=46905 this has been working
> for some time.

Thanks for that, it is a big pain point for me, picking up this patch
for every series I write. The incremental build time for sandbox goes
from 3 seconds to 27 seconds on my laptop with LTO, which is
intolerable.

The EXTRA_CFLAGS says it is for 'Backward compatibility' and it still
does the various LTO things (i.e. it changes the build logic). It
seems odd to me to enable the option and then disable it later in the
command line. It is therefore not quite equivalent. But it seems to
work well enough for me fom a small amount of testing. If you are
really set on not having a special option for it, I can live with it
for now. I'm also not convinced that my patch entirely removes the LTO
stuff in a consistent way.

> Not that you need to respin the series for this.

Regards,
Simon

Re: [PATCH v2 02/13] Makefile: Allow LTO to be disabled for a build

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 2396 bytes --]

On Sat, Mar 12, 2022 at 10:58:44AM -0700, Simon Glass wrote:
> Hi Tom,
> 
> On Mon, 7 Mar 2022 at 07:33, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Fri, Mar 04, 2022 at 08:42:57AM -0700, Simon Glass wrote:
> >
> > > LTO (Link-Time Optimisation) is an very useful feature which can
> > > significantly reduce the size of U-Boot binaries. So far it has been
> > > made available for selected ARM boards and sandbox.
> > >
> > > However, incremental builds are much slower when LTO is used. For example,
> > > an incremental build of sandbox takes 2.1 seconds on my machine, but 6.7
> > > seconds with LTO enabled.
> > >
> > > Add a LTO_BUILD=n parameter to the build, so it can be disabled during
> > > development if needed, for faster builds.
> > >
> > > Add some documentation about LTO while we are here.
> > >
> > > Signed-off-by: Simon Glass <sjg@chromium.org>
> >
> > We don't need this since you can do:
> > make EXTRA_CFLAGS="-fno-lto" EXTRA_LDFLAGS="-fno-lto"
> > to pass -fno-lto to compile/linking and disable lto and per
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=46905 this has been working
> > for some time.
> 
> Thanks for that, it is a big pain point for me, picking up this patch
> for every series I write. The incremental build time for sandbox goes
> from 3 seconds to 27 seconds on my laptop with LTO, which is
> intolerable.

Yeah, I noticed it's visible on my laptop, but not at all on my desktop
(i7 vs Ryzen 7).

> The EXTRA_CFLAGS says it is for 'Backward compatibility' and it still
> does the various LTO things (i.e. it changes the build logic). It

We're unlikely to move to newer Linux kernel kbuild logic so this isn't
going away, and there's not much in the way of logic that's changed for
LTO that I see.

> seems odd to me to enable the option and then disable it later in the
> command line. It is therefore not quite equivalent. But it seems to
> work well enough for me fom a small amount of testing. If you are
> really set on not having a special option for it, I can live with it
> for now. I'm also not convinced that my patch entirely removes the LTO
> stuff in a consistent way.

Yeah, I really don't want to go down the path of overriding CONFIG
options via make/environment logic.  I'm also open to turning off LTO on
sandbox and on with qemu-* so it gets wider CI testing.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH v2 02/13] Makefile: Allow LTO to be disabled for a build

[Simon Glass]

Hi Tom,

On Mon, 14 Mar 2022 at 06:49, Tom Rini <trini@konsulko.com> wrote:
>
> On Sat, Mar 12, 2022 at 10:58:44AM -0700, Simon Glass wrote:
> > Hi Tom,
> >
> > On Mon, 7 Mar 2022 at 07:33, Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Fri, Mar 04, 2022 at 08:42:57AM -0700, Simon Glass wrote:
> > >
> > > > LTO (Link-Time Optimisation) is an very useful feature which can
> > > > significantly reduce the size of U-Boot binaries. So far it has been
> > > > made available for selected ARM boards and sandbox.
> > > >
> > > > However, incremental builds are much slower when LTO is used. For example,
> > > > an incremental build of sandbox takes 2.1 seconds on my machine, but 6.7
> > > > seconds with LTO enabled.
> > > >
> > > > Add a LTO_BUILD=n parameter to the build, so it can be disabled during
> > > > development if needed, for faster builds.
> > > >
> > > > Add some documentation about LTO while we are here.
> > > >
> > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > >
> > > We don't need this since you can do:
> > > make EXTRA_CFLAGS="-fno-lto" EXTRA_LDFLAGS="-fno-lto"
> > > to pass -fno-lto to compile/linking and disable lto and per
> > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=46905 this has been working
> > > for some time.
> >
> > Thanks for that, it is a big pain point for me, picking up this patch
> > for every series I write. The incremental build time for sandbox goes
> > from 3 seconds to 27 seconds on my laptop with LTO, which is
> > intolerable.
>
> Yeah, I noticed it's visible on my laptop, but not at all on my desktop
> (i7 vs Ryzen 7).
>
> > The EXTRA_CFLAGS says it is for 'Backward compatibility' and it still
> > does the various LTO things (i.e. it changes the build logic). It
>
> We're unlikely to move to newer Linux kernel kbuild logic so this isn't
> going away, and there's not much in the way of logic that's changed for
> LTO that I see.
>
> > seems odd to me to enable the option and then disable it later in the
> > command line. It is therefore not quite equivalent. But it seems to
> > work well enough for me fom a small amount of testing. If you are
> > really set on not having a special option for it, I can live with it
> > for now. I'm also not convinced that my patch entirely removes the LTO
> > stuff in a consistent way.
>
> Yeah, I really don't want to go down the path of overriding CONFIG
> options via make/environment logic.  I'm also open to turning off LTO on
> sandbox and on with qemu-* so it gets wider CI testing.

Yes you did mention that, but the problem is that LTO is very handy
with sandbox, to test the strange things that happen. For example, I
found the bug where LTO was dropping a linker-list item, using
sandbox. We could perhaps make one of the sandbox builds not use LTO,
e.g. sandbox_flattree ?

Regards,
Simon

Re: [PATCH v2 02/13] Makefile: Allow LTO to be disabled for a build

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 3213 bytes --]

On Mon, Mar 14, 2022 at 12:24:42PM -0600, Simon Glass wrote:
> Hi Tom,
> 
> On Mon, 14 Mar 2022 at 06:49, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Sat, Mar 12, 2022 at 10:58:44AM -0700, Simon Glass wrote:
> > > Hi Tom,
> > >
> > > On Mon, 7 Mar 2022 at 07:33, Tom Rini <trini@konsulko.com> wrote:
> > > >
> > > > On Fri, Mar 04, 2022 at 08:42:57AM -0700, Simon Glass wrote:
> > > >
> > > > > LTO (Link-Time Optimisation) is an very useful feature which can
> > > > > significantly reduce the size of U-Boot binaries. So far it has been
> > > > > made available for selected ARM boards and sandbox.
> > > > >
> > > > > However, incremental builds are much slower when LTO is used. For example,
> > > > > an incremental build of sandbox takes 2.1 seconds on my machine, but 6.7
> > > > > seconds with LTO enabled.
> > > > >
> > > > > Add a LTO_BUILD=n parameter to the build, so it can be disabled during
> > > > > development if needed, for faster builds.
> > > > >
> > > > > Add some documentation about LTO while we are here.
> > > > >
> > > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > >
> > > > We don't need this since you can do:
> > > > make EXTRA_CFLAGS="-fno-lto" EXTRA_LDFLAGS="-fno-lto"
> > > > to pass -fno-lto to compile/linking and disable lto and per
> > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=46905 this has been working
> > > > for some time.
> > >
> > > Thanks for that, it is a big pain point for me, picking up this patch
> > > for every series I write. The incremental build time for sandbox goes
> > > from 3 seconds to 27 seconds on my laptop with LTO, which is
> > > intolerable.
> >
> > Yeah, I noticed it's visible on my laptop, but not at all on my desktop
> > (i7 vs Ryzen 7).
> >
> > > The EXTRA_CFLAGS says it is for 'Backward compatibility' and it still
> > > does the various LTO things (i.e. it changes the build logic). It
> >
> > We're unlikely to move to newer Linux kernel kbuild logic so this isn't
> > going away, and there's not much in the way of logic that's changed for
> > LTO that I see.
> >
> > > seems odd to me to enable the option and then disable it later in the
> > > command line. It is therefore not quite equivalent. But it seems to
> > > work well enough for me fom a small amount of testing. If you are
> > > really set on not having a special option for it, I can live with it
> > > for now. I'm also not convinced that my patch entirely removes the LTO
> > > stuff in a consistent way.
> >
> > Yeah, I really don't want to go down the path of overriding CONFIG
> > options via make/environment logic.  I'm also open to turning off LTO on
> > sandbox and on with qemu-* so it gets wider CI testing.
> 
> Yes you did mention that, but the problem is that LTO is very handy
> with sandbox, to test the strange things that happen. For example, I
> found the bug where LTO was dropping a linker-list item, using
> sandbox. We could perhaps make one of the sandbox builds not use LTO,
> e.g. sandbox_flattree ?

Well, the big issue with LTO+sandbox is that it slows down your
workflow, so I would think you want the inverse, one platform does
enable it?

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH v2 02/13] Makefile: Allow LTO to be disabled for a build

[Simon Glass]

Hi Tom,

On Mon, 14 Mar 2022 at 12:29, Tom Rini <trini@konsulko.com> wrote:
>
> On Mon, Mar 14, 2022 at 12:24:42PM -0600, Simon Glass wrote:
> > Hi Tom,
> >
> > On Mon, 14 Mar 2022 at 06:49, Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Sat, Mar 12, 2022 at 10:58:44AM -0700, Simon Glass wrote:
> > > > Hi Tom,
> > > >
> > > > On Mon, 7 Mar 2022 at 07:33, Tom Rini <trini@konsulko.com> wrote:
> > > > >
> > > > > On Fri, Mar 04, 2022 at 08:42:57AM -0700, Simon Glass wrote:
> > > > >
> > > > > > LTO (Link-Time Optimisation) is an very useful feature which can
> > > > > > significantly reduce the size of U-Boot binaries. So far it has been
> > > > > > made available for selected ARM boards and sandbox.
> > > > > >
> > > > > > However, incremental builds are much slower when LTO is used. For example,
> > > > > > an incremental build of sandbox takes 2.1 seconds on my machine, but 6.7
> > > > > > seconds with LTO enabled.
> > > > > >
> > > > > > Add a LTO_BUILD=n parameter to the build, so it can be disabled during
> > > > > > development if needed, for faster builds.
> > > > > >
> > > > > > Add some documentation about LTO while we are here.
> > > > > >
> > > > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > > >
> > > > > We don't need this since you can do:
> > > > > make EXTRA_CFLAGS="-fno-lto" EXTRA_LDFLAGS="-fno-lto"
> > > > > to pass -fno-lto to compile/linking and disable lto and per
> > > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=46905 this has been working
> > > > > for some time.
> > > >
> > > > Thanks for that, it is a big pain point for me, picking up this patch
> > > > for every series I write. The incremental build time for sandbox goes
> > > > from 3 seconds to 27 seconds on my laptop with LTO, which is
> > > > intolerable.
> > >
> > > Yeah, I noticed it's visible on my laptop, but not at all on my desktop
> > > (i7 vs Ryzen 7).
> > >
> > > > The EXTRA_CFLAGS says it is for 'Backward compatibility' and it still
> > > > does the various LTO things (i.e. it changes the build logic). It
> > >
> > > We're unlikely to move to newer Linux kernel kbuild logic so this isn't
> > > going away, and there's not much in the way of logic that's changed for
> > > LTO that I see.
> > >
> > > > seems odd to me to enable the option and then disable it later in the
> > > > command line. It is therefore not quite equivalent. But it seems to
> > > > work well enough for me fom a small amount of testing. If you are
> > > > really set on not having a special option for it, I can live with it
> > > > for now. I'm also not convinced that my patch entirely removes the LTO
> > > > stuff in a consistent way.
> > >
> > > Yeah, I really don't want to go down the path of overriding CONFIG
> > > options via make/environment logic.  I'm also open to turning off LTO on
> > > sandbox and on with qemu-* so it gets wider CI testing.
> >
> > Yes you did mention that, but the problem is that LTO is very handy
> > with sandbox, to test the strange things that happen. For example, I
> > found the bug where LTO was dropping a linker-list item, using
> > sandbox. We could perhaps make one of the sandbox builds not use LTO,
> > e.g. sandbox_flattree ?
>
> Well, the big issue with LTO+sandbox is that it slows down your
> workflow, so I would think you want the inverse, one platform does
> enable it?

It slows down all boards that use it, actually, so I normally don't
want it enabled for my IDE. This is not sandbox-specific.

Regards,
Simon

Re: [PATCH v2 02/13] Makefile: Allow LTO to be disabled for a build

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 4002 bytes --]

On Mon, Mar 14, 2022 at 01:21:02PM -0600, Simon Glass wrote:
> Hi Tom,
> 
> On Mon, 14 Mar 2022 at 12:29, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Mon, Mar 14, 2022 at 12:24:42PM -0600, Simon Glass wrote:
> > > Hi Tom,
> > >
> > > On Mon, 14 Mar 2022 at 06:49, Tom Rini <trini@konsulko.com> wrote:
> > > >
> > > > On Sat, Mar 12, 2022 at 10:58:44AM -0700, Simon Glass wrote:
> > > > > Hi Tom,
> > > > >
> > > > > On Mon, 7 Mar 2022 at 07:33, Tom Rini <trini@konsulko.com> wrote:
> > > > > >
> > > > > > On Fri, Mar 04, 2022 at 08:42:57AM -0700, Simon Glass wrote:
> > > > > >
> > > > > > > LTO (Link-Time Optimisation) is an very useful feature which can
> > > > > > > significantly reduce the size of U-Boot binaries. So far it has been
> > > > > > > made available for selected ARM boards and sandbox.
> > > > > > >
> > > > > > > However, incremental builds are much slower when LTO is used. For example,
> > > > > > > an incremental build of sandbox takes 2.1 seconds on my machine, but 6.7
> > > > > > > seconds with LTO enabled.
> > > > > > >
> > > > > > > Add a LTO_BUILD=n parameter to the build, so it can be disabled during
> > > > > > > development if needed, for faster builds.
> > > > > > >
> > > > > > > Add some documentation about LTO while we are here.
> > > > > > >
> > > > > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > > > >
> > > > > > We don't need this since you can do:
> > > > > > make EXTRA_CFLAGS="-fno-lto" EXTRA_LDFLAGS="-fno-lto"
> > > > > > to pass -fno-lto to compile/linking and disable lto and per
> > > > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=46905 this has been working
> > > > > > for some time.
> > > > >
> > > > > Thanks for that, it is a big pain point for me, picking up this patch
> > > > > for every series I write. The incremental build time for sandbox goes
> > > > > from 3 seconds to 27 seconds on my laptop with LTO, which is
> > > > > intolerable.
> > > >
> > > > Yeah, I noticed it's visible on my laptop, but not at all on my desktop
> > > > (i7 vs Ryzen 7).
> > > >
> > > > > The EXTRA_CFLAGS says it is for 'Backward compatibility' and it still
> > > > > does the various LTO things (i.e. it changes the build logic). It
> > > >
> > > > We're unlikely to move to newer Linux kernel kbuild logic so this isn't
> > > > going away, and there's not much in the way of logic that's changed for
> > > > LTO that I see.
> > > >
> > > > > seems odd to me to enable the option and then disable it later in the
> > > > > command line. It is therefore not quite equivalent. But it seems to
> > > > > work well enough for me fom a small amount of testing. If you are
> > > > > really set on not having a special option for it, I can live with it
> > > > > for now. I'm also not convinced that my patch entirely removes the LTO
> > > > > stuff in a consistent way.
> > > >
> > > > Yeah, I really don't want to go down the path of overriding CONFIG
> > > > options via make/environment logic.  I'm also open to turning off LTO on
> > > > sandbox and on with qemu-* so it gets wider CI testing.
> > >
> > > Yes you did mention that, but the problem is that LTO is very handy
> > > with sandbox, to test the strange things that happen. For example, I
> > > found the bug where LTO was dropping a linker-list item, using
> > > sandbox. We could perhaps make one of the sandbox builds not use LTO,
> > > e.g. sandbox_flattree ?
> >
> > Well, the big issue with LTO+sandbox is that it slows down your
> > workflow, so I would think you want the inverse, one platform does
> > enable it?
> 
> It slows down all boards that use it, actually, so I normally don't
> want it enabled for my IDE. This is not sandbox-specific.

The degree of slowdown depends on what you're building on.  I see
several seconds of link time on my laptop and nothing on my desktop
(which isn't that high end either, it's not an EPYC or anything) when
LTO is/isn't enabled.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH v2 02/13] Makefile: Allow LTO to be disabled for a build

[Simon Glass]

Hi Tom,

On Mon, 14 Mar 2022 at 13:45, Tom Rini <trini@konsulko.com> wrote:
>
> On Mon, Mar 14, 2022 at 01:21:02PM -0600, Simon Glass wrote:
> > Hi Tom,
> >
> > On Mon, 14 Mar 2022 at 12:29, Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Mon, Mar 14, 2022 at 12:24:42PM -0600, Simon Glass wrote:
> > > > Hi Tom,
> > > >
> > > > On Mon, 14 Mar 2022 at 06:49, Tom Rini <trini@konsulko.com> wrote:
> > > > >
> > > > > On Sat, Mar 12, 2022 at 10:58:44AM -0700, Simon Glass wrote:
> > > > > > Hi Tom,
> > > > > >
> > > > > > On Mon, 7 Mar 2022 at 07:33, Tom Rini <trini@konsulko.com> wrote:
> > > > > > >
> > > > > > > On Fri, Mar 04, 2022 at 08:42:57AM -0700, Simon Glass wrote:
> > > > > > >
> > > > > > > > LTO (Link-Time Optimisation) is an very useful feature which can
> > > > > > > > significantly reduce the size of U-Boot binaries. So far it has been
> > > > > > > > made available for selected ARM boards and sandbox.
> > > > > > > >
> > > > > > > > However, incremental builds are much slower when LTO is used. For example,
> > > > > > > > an incremental build of sandbox takes 2.1 seconds on my machine, but 6.7
> > > > > > > > seconds with LTO enabled.
> > > > > > > >
> > > > > > > > Add a LTO_BUILD=n parameter to the build, so it can be disabled during
> > > > > > > > development if needed, for faster builds.
> > > > > > > >
> > > > > > > > Add some documentation about LTO while we are here.
> > > > > > > >
> > > > > > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > > > > >
> > > > > > > We don't need this since you can do:
> > > > > > > make EXTRA_CFLAGS="-fno-lto" EXTRA_LDFLAGS="-fno-lto"
> > > > > > > to pass -fno-lto to compile/linking and disable lto and per
> > > > > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=46905 this has been working
> > > > > > > for some time.
> > > > > >
> > > > > > Thanks for that, it is a big pain point for me, picking up this patch
> > > > > > for every series I write. The incremental build time for sandbox goes
> > > > > > from 3 seconds to 27 seconds on my laptop with LTO, which is
> > > > > > intolerable.
> > > > >
> > > > > Yeah, I noticed it's visible on my laptop, but not at all on my desktop
> > > > > (i7 vs Ryzen 7).
> > > > >
> > > > > > The EXTRA_CFLAGS says it is for 'Backward compatibility' and it still
> > > > > > does the various LTO things (i.e. it changes the build logic). It
> > > > >
> > > > > We're unlikely to move to newer Linux kernel kbuild logic so this isn't
> > > > > going away, and there's not much in the way of logic that's changed for
> > > > > LTO that I see.
> > > > >
> > > > > > seems odd to me to enable the option and then disable it later in the
> > > > > > command line. It is therefore not quite equivalent. But it seems to
> > > > > > work well enough for me fom a small amount of testing. If you are
> > > > > > really set on not having a special option for it, I can live with it
> > > > > > for now. I'm also not convinced that my patch entirely removes the LTO
> > > > > > stuff in a consistent way.
> > > > >
> > > > > Yeah, I really don't want to go down the path of overriding CONFIG
> > > > > options via make/environment logic.  I'm also open to turning off LTO on
> > > > > sandbox and on with qemu-* so it gets wider CI testing.
> > > >
> > > > Yes you did mention that, but the problem is that LTO is very handy
> > > > with sandbox, to test the strange things that happen. For example, I
> > > > found the bug where LTO was dropping a linker-list item, using
> > > > sandbox. We could perhaps make one of the sandbox builds not use LTO,
> > > > e.g. sandbox_flattree ?
> > >
> > > Well, the big issue with LTO+sandbox is that it slows down your
> > > workflow, so I would think you want the inverse, one platform does
> > > enable it?
> >
> > It slows down all boards that use it, actually, so I normally don't
> > want it enabled for my IDE. This is not sandbox-specific.
>
> The degree of slowdown depends on what you're building on.  I see
> several seconds of link time on my laptop and nothing on my desktop
> (which isn't that high end either, it's not an EPYC or anything) when
> LTO is/isn't enabled.

For me, on 16-core AMD desktop, incremental build

LTO: real 0m7.744s
No LTO: real 0m3.172s

On 4-core laptop:

No LTO: real 0m2.429s
LTO: real 0m15.650s

Perhaps the disconnect here is that you just don't see any difference?
What times are you seeing?

Regards,
SImon

Re: [PATCH v2 02/13] Makefile: Allow LTO to be disabled for a build

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 4984 bytes --]

On Mon, Mar 14, 2022 at 02:18:14PM -0600, Simon Glass wrote:
> Hi Tom,
> 
> On Mon, 14 Mar 2022 at 13:45, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Mon, Mar 14, 2022 at 01:21:02PM -0600, Simon Glass wrote:
> > > Hi Tom,
> > >
> > > On Mon, 14 Mar 2022 at 12:29, Tom Rini <trini@konsulko.com> wrote:
> > > >
> > > > On Mon, Mar 14, 2022 at 12:24:42PM -0600, Simon Glass wrote:
> > > > > Hi Tom,
> > > > >
> > > > > On Mon, 14 Mar 2022 at 06:49, Tom Rini <trini@konsulko.com> wrote:
> > > > > >
> > > > > > On Sat, Mar 12, 2022 at 10:58:44AM -0700, Simon Glass wrote:
> > > > > > > Hi Tom,
> > > > > > >
> > > > > > > On Mon, 7 Mar 2022 at 07:33, Tom Rini <trini@konsulko.com> wrote:
> > > > > > > >
> > > > > > > > On Fri, Mar 04, 2022 at 08:42:57AM -0700, Simon Glass wrote:
> > > > > > > >
> > > > > > > > > LTO (Link-Time Optimisation) is an very useful feature which can
> > > > > > > > > significantly reduce the size of U-Boot binaries. So far it has been
> > > > > > > > > made available for selected ARM boards and sandbox.
> > > > > > > > >
> > > > > > > > > However, incremental builds are much slower when LTO is used. For example,
> > > > > > > > > an incremental build of sandbox takes 2.1 seconds on my machine, but 6.7
> > > > > > > > > seconds with LTO enabled.
> > > > > > > > >
> > > > > > > > > Add a LTO_BUILD=n parameter to the build, so it can be disabled during
> > > > > > > > > development if needed, for faster builds.
> > > > > > > > >
> > > > > > > > > Add some documentation about LTO while we are here.
> > > > > > > > >
> > > > > > > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > > > > > >
> > > > > > > > We don't need this since you can do:
> > > > > > > > make EXTRA_CFLAGS="-fno-lto" EXTRA_LDFLAGS="-fno-lto"
> > > > > > > > to pass -fno-lto to compile/linking and disable lto and per
> > > > > > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=46905 this has been working
> > > > > > > > for some time.
> > > > > > >
> > > > > > > Thanks for that, it is a big pain point for me, picking up this patch
> > > > > > > for every series I write. The incremental build time for sandbox goes
> > > > > > > from 3 seconds to 27 seconds on my laptop with LTO, which is
> > > > > > > intolerable.
> > > > > >
> > > > > > Yeah, I noticed it's visible on my laptop, but not at all on my desktop
> > > > > > (i7 vs Ryzen 7).
> > > > > >
> > > > > > > The EXTRA_CFLAGS says it is for 'Backward compatibility' and it still
> > > > > > > does the various LTO things (i.e. it changes the build logic). It
> > > > > >
> > > > > > We're unlikely to move to newer Linux kernel kbuild logic so this isn't
> > > > > > going away, and there's not much in the way of logic that's changed for
> > > > > > LTO that I see.
> > > > > >
> > > > > > > seems odd to me to enable the option and then disable it later in the
> > > > > > > command line. It is therefore not quite equivalent. But it seems to
> > > > > > > work well enough for me fom a small amount of testing. If you are
> > > > > > > really set on not having a special option for it, I can live with it
> > > > > > > for now. I'm also not convinced that my patch entirely removes the LTO
> > > > > > > stuff in a consistent way.
> > > > > >
> > > > > > Yeah, I really don't want to go down the path of overriding CONFIG
> > > > > > options via make/environment logic.  I'm also open to turning off LTO on
> > > > > > sandbox and on with qemu-* so it gets wider CI testing.
> > > > >
> > > > > Yes you did mention that, but the problem is that LTO is very handy
> > > > > with sandbox, to test the strange things that happen. For example, I
> > > > > found the bug where LTO was dropping a linker-list item, using
> > > > > sandbox. We could perhaps make one of the sandbox builds not use LTO,
> > > > > e.g. sandbox_flattree ?
> > > >
> > > > Well, the big issue with LTO+sandbox is that it slows down your
> > > > workflow, so I would think you want the inverse, one platform does
> > > > enable it?
> > >
> > > It slows down all boards that use it, actually, so I normally don't
> > > want it enabled for my IDE. This is not sandbox-specific.
> >
> > The degree of slowdown depends on what you're building on.  I see
> > several seconds of link time on my laptop and nothing on my desktop
> > (which isn't that high end either, it's not an EPYC or anything) when
> > LTO is/isn't enabled.
> 
> For me, on 16-core AMD desktop, incremental build
> 
> LTO: real 0m7.744s
> No LTO: real 0m3.172s
> 
> On 4-core laptop:
> 
> No LTO: real 0m2.429s
> LTO: real 0m15.650s
> 
> Perhaps the disconnect here is that you just don't see any difference?
> What times are you seeing?

My 16 core desktop is 14 seconds without, 15 seconds with (full build,
not just the link time) and I don't recall what my laptop was but since
it's longer than the desktop I'm always just ssh'd in there anyhow.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH v2 02/13] Makefile: Allow LTO to be disabled for a build

[Simon Glass]

Hi Tom,

On Mon, 14 Mar 2022 at 14:23, Tom Rini <trini@konsulko.com> wrote:
>
> On Mon, Mar 14, 2022 at 02:18:14PM -0600, Simon Glass wrote:
> > Hi Tom,
> >
> > On Mon, 14 Mar 2022 at 13:45, Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Mon, Mar 14, 2022 at 01:21:02PM -0600, Simon Glass wrote:
> > > > Hi Tom,
> > > >
> > > > On Mon, 14 Mar 2022 at 12:29, Tom Rini <trini@konsulko.com> wrote:
> > > > >
> > > > > On Mon, Mar 14, 2022 at 12:24:42PM -0600, Simon Glass wrote:
> > > > > > Hi Tom,
> > > > > >
> > > > > > On Mon, 14 Mar 2022 at 06:49, Tom Rini <trini@konsulko.com> wrote:
> > > > > > >
> > > > > > > On Sat, Mar 12, 2022 at 10:58:44AM -0700, Simon Glass wrote:
> > > > > > > > Hi Tom,
> > > > > > > >
> > > > > > > > On Mon, 7 Mar 2022 at 07:33, Tom Rini <trini@konsulko.com> wrote:
> > > > > > > > >
> > > > > > > > > On Fri, Mar 04, 2022 at 08:42:57AM -0700, Simon Glass wrote:
> > > > > > > > >
> > > > > > > > > > LTO (Link-Time Optimisation) is an very useful feature which can
> > > > > > > > > > significantly reduce the size of U-Boot binaries. So far it has been
> > > > > > > > > > made available for selected ARM boards and sandbox.
> > > > > > > > > >
> > > > > > > > > > However, incremental builds are much slower when LTO is used. For example,
> > > > > > > > > > an incremental build of sandbox takes 2.1 seconds on my machine, but 6.7
> > > > > > > > > > seconds with LTO enabled.
> > > > > > > > > >
> > > > > > > > > > Add a LTO_BUILD=n parameter to the build, so it can be disabled during
> > > > > > > > > > development if needed, for faster builds.
> > > > > > > > > >
> > > > > > > > > > Add some documentation about LTO while we are here.
> > > > > > > > > >
> > > > > > > > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > > > > > > >
> > > > > > > > > We don't need this since you can do:
> > > > > > > > > make EXTRA_CFLAGS="-fno-lto" EXTRA_LDFLAGS="-fno-lto"
> > > > > > > > > to pass -fno-lto to compile/linking and disable lto and per
> > > > > > > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=46905 this has been working
> > > > > > > > > for some time.
> > > > > > > >
> > > > > > > > Thanks for that, it is a big pain point for me, picking up this patch
> > > > > > > > for every series I write. The incremental build time for sandbox goes
> > > > > > > > from 3 seconds to 27 seconds on my laptop with LTO, which is
> > > > > > > > intolerable.
> > > > > > >
> > > > > > > Yeah, I noticed it's visible on my laptop, but not at all on my desktop
> > > > > > > (i7 vs Ryzen 7).
> > > > > > >
> > > > > > > > The EXTRA_CFLAGS says it is for 'Backward compatibility' and it still
> > > > > > > > does the various LTO things (i.e. it changes the build logic). It
> > > > > > >
> > > > > > > We're unlikely to move to newer Linux kernel kbuild logic so this isn't
> > > > > > > going away, and there's not much in the way of logic that's changed for
> > > > > > > LTO that I see.
> > > > > > >
> > > > > > > > seems odd to me to enable the option and then disable it later in the
> > > > > > > > command line. It is therefore not quite equivalent. But it seems to
> > > > > > > > work well enough for me fom a small amount of testing. If you are
> > > > > > > > really set on not having a special option for it, I can live with it
> > > > > > > > for now. I'm also not convinced that my patch entirely removes the LTO
> > > > > > > > stuff in a consistent way.
> > > > > > >
> > > > > > > Yeah, I really don't want to go down the path of overriding CONFIG
> > > > > > > options via make/environment logic.  I'm also open to turning off LTO on
> > > > > > > sandbox and on with qemu-* so it gets wider CI testing.
> > > > > >
> > > > > > Yes you did mention that, but the problem is that LTO is very handy
> > > > > > with sandbox, to test the strange things that happen. For example, I
> > > > > > found the bug where LTO was dropping a linker-list item, using
> > > > > > sandbox. We could perhaps make one of the sandbox builds not use LTO,
> > > > > > e.g. sandbox_flattree ?
> > > > >
> > > > > Well, the big issue with LTO+sandbox is that it slows down your
> > > > > workflow, so I would think you want the inverse, one platform does
> > > > > enable it?
> > > >
> > > > It slows down all boards that use it, actually, so I normally don't
> > > > want it enabled for my IDE. This is not sandbox-specific.
> > >
> > > The degree of slowdown depends on what you're building on.  I see
> > > several seconds of link time on my laptop and nothing on my desktop
> > > (which isn't that high end either, it's not an EPYC or anything) when
> > > LTO is/isn't enabled.
> >
> > For me, on 16-core AMD desktop, incremental build
> >
> > LTO: real 0m7.744s
> > No LTO: real 0m3.172s
> >
> > On 4-core laptop:
> >
> > No LTO: real 0m2.429s
> > LTO: real 0m15.650s
> >
> > Perhaps the disconnect here is that you just don't see any difference?
> > What times are you seeing?
>
> My 16 core desktop is 14 seconds without, 15 seconds with (full build,
> not just the link time) and I don't recall what my laptop was but since
> it's longer than the desktop I'm always just ssh'd in there anyhow.

Seems very slow. Are you sure that is an incremental build?

Regards,
Simon

Re: [PATCH v2 02/13] Makefile: Allow LTO to be disabled for a build

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 5683 bytes --]

On Mon, Mar 14, 2022 at 03:43:05PM -0600, Simon Glass wrote:
> Hi Tom,
> 
> On Mon, 14 Mar 2022 at 14:23, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Mon, Mar 14, 2022 at 02:18:14PM -0600, Simon Glass wrote:
> > > Hi Tom,
> > >
> > > On Mon, 14 Mar 2022 at 13:45, Tom Rini <trini@konsulko.com> wrote:
> > > >
> > > > On Mon, Mar 14, 2022 at 01:21:02PM -0600, Simon Glass wrote:
> > > > > Hi Tom,
> > > > >
> > > > > On Mon, 14 Mar 2022 at 12:29, Tom Rini <trini@konsulko.com> wrote:
> > > > > >
> > > > > > On Mon, Mar 14, 2022 at 12:24:42PM -0600, Simon Glass wrote:
> > > > > > > Hi Tom,
> > > > > > >
> > > > > > > On Mon, 14 Mar 2022 at 06:49, Tom Rini <trini@konsulko.com> wrote:
> > > > > > > >
> > > > > > > > On Sat, Mar 12, 2022 at 10:58:44AM -0700, Simon Glass wrote:
> > > > > > > > > Hi Tom,
> > > > > > > > >
> > > > > > > > > On Mon, 7 Mar 2022 at 07:33, Tom Rini <trini@konsulko.com> wrote:
> > > > > > > > > >
> > > > > > > > > > On Fri, Mar 04, 2022 at 08:42:57AM -0700, Simon Glass wrote:
> > > > > > > > > >
> > > > > > > > > > > LTO (Link-Time Optimisation) is an very useful feature which can
> > > > > > > > > > > significantly reduce the size of U-Boot binaries. So far it has been
> > > > > > > > > > > made available for selected ARM boards and sandbox.
> > > > > > > > > > >
> > > > > > > > > > > However, incremental builds are much slower when LTO is used. For example,
> > > > > > > > > > > an incremental build of sandbox takes 2.1 seconds on my machine, but 6.7
> > > > > > > > > > > seconds with LTO enabled.
> > > > > > > > > > >
> > > > > > > > > > > Add a LTO_BUILD=n parameter to the build, so it can be disabled during
> > > > > > > > > > > development if needed, for faster builds.
> > > > > > > > > > >
> > > > > > > > > > > Add some documentation about LTO while we are here.
> > > > > > > > > > >
> > > > > > > > > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > > > > > > > >
> > > > > > > > > > We don't need this since you can do:
> > > > > > > > > > make EXTRA_CFLAGS="-fno-lto" EXTRA_LDFLAGS="-fno-lto"
> > > > > > > > > > to pass -fno-lto to compile/linking and disable lto and per
> > > > > > > > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=46905 this has been working
> > > > > > > > > > for some time.
> > > > > > > > >
> > > > > > > > > Thanks for that, it is a big pain point for me, picking up this patch
> > > > > > > > > for every series I write. The incremental build time for sandbox goes
> > > > > > > > > from 3 seconds to 27 seconds on my laptop with LTO, which is
> > > > > > > > > intolerable.
> > > > > > > >
> > > > > > > > Yeah, I noticed it's visible on my laptop, but not at all on my desktop
> > > > > > > > (i7 vs Ryzen 7).
> > > > > > > >
> > > > > > > > > The EXTRA_CFLAGS says it is for 'Backward compatibility' and it still
> > > > > > > > > does the various LTO things (i.e. it changes the build logic). It
> > > > > > > >
> > > > > > > > We're unlikely to move to newer Linux kernel kbuild logic so this isn't
> > > > > > > > going away, and there's not much in the way of logic that's changed for
> > > > > > > > LTO that I see.
> > > > > > > >
> > > > > > > > > seems odd to me to enable the option and then disable it later in the
> > > > > > > > > command line. It is therefore not quite equivalent. But it seems to
> > > > > > > > > work well enough for me fom a small amount of testing. If you are
> > > > > > > > > really set on not having a special option for it, I can live with it
> > > > > > > > > for now. I'm also not convinced that my patch entirely removes the LTO
> > > > > > > > > stuff in a consistent way.
> > > > > > > >
> > > > > > > > Yeah, I really don't want to go down the path of overriding CONFIG
> > > > > > > > options via make/environment logic.  I'm also open to turning off LTO on
> > > > > > > > sandbox and on with qemu-* so it gets wider CI testing.
> > > > > > >
> > > > > > > Yes you did mention that, but the problem is that LTO is very handy
> > > > > > > with sandbox, to test the strange things that happen. For example, I
> > > > > > > found the bug where LTO was dropping a linker-list item, using
> > > > > > > sandbox. We could perhaps make one of the sandbox builds not use LTO,
> > > > > > > e.g. sandbox_flattree ?
> > > > > >
> > > > > > Well, the big issue with LTO+sandbox is that it slows down your
> > > > > > workflow, so I would think you want the inverse, one platform does
> > > > > > enable it?
> > > > >
> > > > > It slows down all boards that use it, actually, so I normally don't
> > > > > want it enabled for my IDE. This is not sandbox-specific.
> > > >
> > > > The degree of slowdown depends on what you're building on.  I see
> > > > several seconds of link time on my laptop and nothing on my desktop
> > > > (which isn't that high end either, it's not an EPYC or anything) when
> > > > LTO is/isn't enabled.
> > >
> > > For me, on 16-core AMD desktop, incremental build
> > >
> > > LTO: real 0m7.744s
> > > No LTO: real 0m3.172s
> > >
> > > On 4-core laptop:
> > >
> > > No LTO: real 0m2.429s
> > > LTO: real 0m15.650s
> > >
> > > Perhaps the disconnect here is that you just don't see any difference?
> > > What times are you seeing?
> >
> > My 16 core desktop is 14 seconds without, 15 seconds with (full build,
> > not just the link time) and I don't recall what my laptop was but since
> > it's longer than the desktop I'm always just ssh'd in there anyhow.
> 
> Seems very slow. Are you sure that is an incremental build?

No, it's a from-scratch build, which should be the worst case for this.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH v2 02/13] Makefile: Allow LTO to be disabled for a build

[Simon Glass]

Hi Tom,

On Mon, 14 Mar 2022 at 16:42, Tom Rini <trini@konsulko.com> wrote:
>
> On Mon, Mar 14, 2022 at 03:43:05PM -0600, Simon Glass wrote:
> > Hi Tom,
> >
> > On Mon, 14 Mar 2022 at 14:23, Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Mon, Mar 14, 2022 at 02:18:14PM -0600, Simon Glass wrote:
> > > > Hi Tom,
> > > >
> > > > On Mon, 14 Mar 2022 at 13:45, Tom Rini <trini@konsulko.com> wrote:
> > > > >
> > > > > On Mon, Mar 14, 2022 at 01:21:02PM -0600, Simon Glass wrote:
> > > > > > Hi Tom,
> > > > > >
> > > > > > On Mon, 14 Mar 2022 at 12:29, Tom Rini <trini@konsulko.com> wrote:
> > > > > > >
> > > > > > > On Mon, Mar 14, 2022 at 12:24:42PM -0600, Simon Glass wrote:
> > > > > > > > Hi Tom,
> > > > > > > >
> > > > > > > > On Mon, 14 Mar 2022 at 06:49, Tom Rini <trini@konsulko.com> wrote:
> > > > > > > > >
> > > > > > > > > On Sat, Mar 12, 2022 at 10:58:44AM -0700, Simon Glass wrote:
> > > > > > > > > > Hi Tom,
> > > > > > > > > >
> > > > > > > > > > On Mon, 7 Mar 2022 at 07:33, Tom Rini <trini@konsulko.com> wrote:
> > > > > > > > > > >
> > > > > > > > > > > On Fri, Mar 04, 2022 at 08:42:57AM -0700, Simon Glass wrote:
> > > > > > > > > > >
> > > > > > > > > > > > LTO (Link-Time Optimisation) is an very useful feature which can
> > > > > > > > > > > > significantly reduce the size of U-Boot binaries. So far it has been
> > > > > > > > > > > > made available for selected ARM boards and sandbox.
> > > > > > > > > > > >
> > > > > > > > > > > > However, incremental builds are much slower when LTO is used. For example,
> > > > > > > > > > > > an incremental build of sandbox takes 2.1 seconds on my machine, but 6.7
> > > > > > > > > > > > seconds with LTO enabled.
> > > > > > > > > > > >
> > > > > > > > > > > > Add a LTO_BUILD=n parameter to the build, so it can be disabled during
> > > > > > > > > > > > development if needed, for faster builds.
> > > > > > > > > > > >
> > > > > > > > > > > > Add some documentation about LTO while we are here.
> > > > > > > > > > > >
> > > > > > > > > > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > > > > > > > > >
> > > > > > > > > > > We don't need this since you can do:
> > > > > > > > > > > make EXTRA_CFLAGS="-fno-lto" EXTRA_LDFLAGS="-fno-lto"
> > > > > > > > > > > to pass -fno-lto to compile/linking and disable lto and per
> > > > > > > > > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=46905 this has been working
> > > > > > > > > > > for some time.
> > > > > > > > > >
> > > > > > > > > > Thanks for that, it is a big pain point for me, picking up this patch
> > > > > > > > > > for every series I write. The incremental build time for sandbox goes
> > > > > > > > > > from 3 seconds to 27 seconds on my laptop with LTO, which is
> > > > > > > > > > intolerable.
> > > > > > > > >
> > > > > > > > > Yeah, I noticed it's visible on my laptop, but not at all on my desktop
> > > > > > > > > (i7 vs Ryzen 7).
> > > > > > > > >
> > > > > > > > > > The EXTRA_CFLAGS says it is for 'Backward compatibility' and it still
> > > > > > > > > > does the various LTO things (i.e. it changes the build logic). It
> > > > > > > > >
> > > > > > > > > We're unlikely to move to newer Linux kernel kbuild logic so this isn't
> > > > > > > > > going away, and there's not much in the way of logic that's changed for
> > > > > > > > > LTO that I see.
> > > > > > > > >
> > > > > > > > > > seems odd to me to enable the option and then disable it later in the
> > > > > > > > > > command line. It is therefore not quite equivalent. But it seems to
> > > > > > > > > > work well enough for me fom a small amount of testing. If you are
> > > > > > > > > > really set on not having a special option for it, I can live with it
> > > > > > > > > > for now. I'm also not convinced that my patch entirely removes the LTO
> > > > > > > > > > stuff in a consistent way.
> > > > > > > > >
> > > > > > > > > Yeah, I really don't want to go down the path of overriding CONFIG
> > > > > > > > > options via make/environment logic.  I'm also open to turning off LTO on
> > > > > > > > > sandbox and on with qemu-* so it gets wider CI testing.
> > > > > > > >
> > > > > > > > Yes you did mention that, but the problem is that LTO is very handy
> > > > > > > > with sandbox, to test the strange things that happen. For example, I
> > > > > > > > found the bug where LTO was dropping a linker-list item, using
> > > > > > > > sandbox. We could perhaps make one of the sandbox builds not use LTO,
> > > > > > > > e.g. sandbox_flattree ?
> > > > > > >
> > > > > > > Well, the big issue with LTO+sandbox is that it slows down your
> > > > > > > workflow, so I would think you want the inverse, one platform does
> > > > > > > enable it?
> > > > > >
> > > > > > It slows down all boards that use it, actually, so I normally don't
> > > > > > want it enabled for my IDE. This is not sandbox-specific.
> > > > >
> > > > > The degree of slowdown depends on what you're building on.  I see
> > > > > several seconds of link time on my laptop and nothing on my desktop
> > > > > (which isn't that high end either, it's not an EPYC or anything) when
> > > > > LTO is/isn't enabled.
> > > >
> > > > For me, on 16-core AMD desktop, incremental build
> > > >
> > > > LTO: real 0m7.744s
> > > > No LTO: real 0m3.172s
> > > >
> > > > On 4-core laptop:
> > > >
> > > > No LTO: real 0m2.429s
> > > > LTO: real 0m15.650s
> > > >
> > > > Perhaps the disconnect here is that you just don't see any difference?
> > > > What times are you seeing?
> > >
> > > My 16 core desktop is 14 seconds without, 15 seconds with (full build,
> > > not just the link time) and I don't recall what my laptop was but since
> > > it's longer than the desktop I'm always just ssh'd in there anyhow.
> >
> > Seems very slow. Are you sure that is an incremental build?
>
> No, it's a from-scratch build, which should be the worst case for this.

OK. So I don't do fresh builds very often, mostly they are incremental
(for obvious reasons...I am developing things incrementally). So the
difference is quite dramatic, as you can see.

Does my problem make more sense now?

Regards,
Simon

Re: [PATCH v2 02/13] Makefile: Allow LTO to be disabled for a build

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 6833 bytes --]

On Wed, Mar 16, 2022 at 10:15:17AM +1300, Simon Glass wrote:
> Hi Tom,
> 
> On Mon, 14 Mar 2022 at 16:42, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Mon, Mar 14, 2022 at 03:43:05PM -0600, Simon Glass wrote:
> > > Hi Tom,
> > >
> > > On Mon, 14 Mar 2022 at 14:23, Tom Rini <trini@konsulko.com> wrote:
> > > >
> > > > On Mon, Mar 14, 2022 at 02:18:14PM -0600, Simon Glass wrote:
> > > > > Hi Tom,
> > > > >
> > > > > On Mon, 14 Mar 2022 at 13:45, Tom Rini <trini@konsulko.com> wrote:
> > > > > >
> > > > > > On Mon, Mar 14, 2022 at 01:21:02PM -0600, Simon Glass wrote:
> > > > > > > Hi Tom,
> > > > > > >
> > > > > > > On Mon, 14 Mar 2022 at 12:29, Tom Rini <trini@konsulko.com> wrote:
> > > > > > > >
> > > > > > > > On Mon, Mar 14, 2022 at 12:24:42PM -0600, Simon Glass wrote:
> > > > > > > > > Hi Tom,
> > > > > > > > >
> > > > > > > > > On Mon, 14 Mar 2022 at 06:49, Tom Rini <trini@konsulko.com> wrote:
> > > > > > > > > >
> > > > > > > > > > On Sat, Mar 12, 2022 at 10:58:44AM -0700, Simon Glass wrote:
> > > > > > > > > > > Hi Tom,
> > > > > > > > > > >
> > > > > > > > > > > On Mon, 7 Mar 2022 at 07:33, Tom Rini <trini@konsulko.com> wrote:
> > > > > > > > > > > >
> > > > > > > > > > > > On Fri, Mar 04, 2022 at 08:42:57AM -0700, Simon Glass wrote:
> > > > > > > > > > > >
> > > > > > > > > > > > > LTO (Link-Time Optimisation) is an very useful feature which can
> > > > > > > > > > > > > significantly reduce the size of U-Boot binaries. So far it has been
> > > > > > > > > > > > > made available for selected ARM boards and sandbox.
> > > > > > > > > > > > >
> > > > > > > > > > > > > However, incremental builds are much slower when LTO is used. For example,
> > > > > > > > > > > > > an incremental build of sandbox takes 2.1 seconds on my machine, but 6.7
> > > > > > > > > > > > > seconds with LTO enabled.
> > > > > > > > > > > > >
> > > > > > > > > > > > > Add a LTO_BUILD=n parameter to the build, so it can be disabled during
> > > > > > > > > > > > > development if needed, for faster builds.
> > > > > > > > > > > > >
> > > > > > > > > > > > > Add some documentation about LTO while we are here.
> > > > > > > > > > > > >
> > > > > > > > > > > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > > > > > > > > > >
> > > > > > > > > > > > We don't need this since you can do:
> > > > > > > > > > > > make EXTRA_CFLAGS="-fno-lto" EXTRA_LDFLAGS="-fno-lto"
> > > > > > > > > > > > to pass -fno-lto to compile/linking and disable lto and per
> > > > > > > > > > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=46905 this has been working
> > > > > > > > > > > > for some time.
> > > > > > > > > > >
> > > > > > > > > > > Thanks for that, it is a big pain point for me, picking up this patch
> > > > > > > > > > > for every series I write. The incremental build time for sandbox goes
> > > > > > > > > > > from 3 seconds to 27 seconds on my laptop with LTO, which is
> > > > > > > > > > > intolerable.
> > > > > > > > > >
> > > > > > > > > > Yeah, I noticed it's visible on my laptop, but not at all on my desktop
> > > > > > > > > > (i7 vs Ryzen 7).
> > > > > > > > > >
> > > > > > > > > > > The EXTRA_CFLAGS says it is for 'Backward compatibility' and it still
> > > > > > > > > > > does the various LTO things (i.e. it changes the build logic). It
> > > > > > > > > >
> > > > > > > > > > We're unlikely to move to newer Linux kernel kbuild logic so this isn't
> > > > > > > > > > going away, and there's not much in the way of logic that's changed for
> > > > > > > > > > LTO that I see.
> > > > > > > > > >
> > > > > > > > > > > seems odd to me to enable the option and then disable it later in the
> > > > > > > > > > > command line. It is therefore not quite equivalent. But it seems to
> > > > > > > > > > > work well enough for me fom a small amount of testing. If you are
> > > > > > > > > > > really set on not having a special option for it, I can live with it
> > > > > > > > > > > for now. I'm also not convinced that my patch entirely removes the LTO
> > > > > > > > > > > stuff in a consistent way.
> > > > > > > > > >
> > > > > > > > > > Yeah, I really don't want to go down the path of overriding CONFIG
> > > > > > > > > > options via make/environment logic.  I'm also open to turning off LTO on
> > > > > > > > > > sandbox and on with qemu-* so it gets wider CI testing.
> > > > > > > > >
> > > > > > > > > Yes you did mention that, but the problem is that LTO is very handy
> > > > > > > > > with sandbox, to test the strange things that happen. For example, I
> > > > > > > > > found the bug where LTO was dropping a linker-list item, using
> > > > > > > > > sandbox. We could perhaps make one of the sandbox builds not use LTO,
> > > > > > > > > e.g. sandbox_flattree ?
> > > > > > > >
> > > > > > > > Well, the big issue with LTO+sandbox is that it slows down your
> > > > > > > > workflow, so I would think you want the inverse, one platform does
> > > > > > > > enable it?
> > > > > > >
> > > > > > > It slows down all boards that use it, actually, so I normally don't
> > > > > > > want it enabled for my IDE. This is not sandbox-specific.
> > > > > >
> > > > > > The degree of slowdown depends on what you're building on.  I see
> > > > > > several seconds of link time on my laptop and nothing on my desktop
> > > > > > (which isn't that high end either, it's not an EPYC or anything) when
> > > > > > LTO is/isn't enabled.
> > > > >
> > > > > For me, on 16-core AMD desktop, incremental build
> > > > >
> > > > > LTO: real 0m7.744s
> > > > > No LTO: real 0m3.172s
> > > > >
> > > > > On 4-core laptop:
> > > > >
> > > > > No LTO: real 0m2.429s
> > > > > LTO: real 0m15.650s
> > > > >
> > > > > Perhaps the disconnect here is that you just don't see any difference?
> > > > > What times are you seeing?
> > > >
> > > > My 16 core desktop is 14 seconds without, 15 seconds with (full build,
> > > > not just the link time) and I don't recall what my laptop was but since
> > > > it's longer than the desktop I'm always just ssh'd in there anyhow.
> > >
> > > Seems very slow. Are you sure that is an incremental build?
> >
> > No, it's a from-scratch build, which should be the worst case for this.
> 
> OK. So I don't do fresh builds very often, mostly they are incremental
> (for obvious reasons...I am developing things incrementally). So the
> difference is quite dramatic, as you can see.
> 
> Does my problem make more sense now?

Yes, I didn't realize incremental non-LTO link was so quick.  But no, I
don't want to open the door here to other "make iterative build quicker,
override the config" options.  Maybe just another note to the
development docs and see if that leads to a chorus of people asking for
this feature after all.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]