[PATCH 1/2] x86: Fix saying arch_cpu_init

All of lore.kernel.org
 help / color / mirror / Atom feed

* [PATCH 1/2] x86: Fix saying arch_cpu_init_dm in debug/docs
@ 2023-01-14 20:49 Tom Rini
  2023-01-14 20:49 ` [PATCH 2/2] event: Correct dependencies on the EVENT framework Tom Rini
                   ` (3 more replies)
  0 siblings, 4 replies; 20+ messages in thread
From: Tom Rini @ 2023-01-14 20:49 UTC (permalink / raw)
  To: u-boot; +Cc: Simon Glass

The function arch_cpu_init_dm was renamed to fsp_setup_pinctrl in these
cases, so rename debug / docs to match.

Cc: Simon Glass <sjg@chromium.org>
Fixes: 7fe32b3442f0 ("event: Convert arch_cpu_init_dm() to use events")
Signed-off-by: Tom Rini <trini@konsulko.com>
---
 arch/x86/lib/spl.c                    | 2 +-
 doc/board/google/chromebook_coral.rst | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/lib/spl.c b/arch/x86/lib/spl.c
index 34ef68f2bb7d..bdf57ef7b5bd 100644
--- a/arch/x86/lib/spl.c
+++ b/arch/x86/lib/spl.c
@@ -92,7 +92,7 @@ static int x86_spl_init(void)
 #ifndef CONFIG_TPL
 	ret = fsp_setup_pinctrl(NULL, NULL);
 	if (ret) {
-		debug("%s: arch_cpu_init_dm() failed\n", __func__);
+		debug("%s: fsp_setup_pinctrl() failed\n", __func__);
 		return ret;
 	}
 #endif
diff --git a/doc/board/google/chromebook_coral.rst b/doc/board/google/chromebook_coral.rst
index 8edbf0429cd0..23e2db455641 100644
--- a/doc/board/google/chromebook_coral.rst
+++ b/doc/board/google/chromebook_coral.rst
@@ -259,7 +259,7 @@ Boot flow - U-Boot pre-relocation
 
 U-Boot (running from start_from_spl.S) starts running in RAM and uses the same
 stack as SPL. It does various init activities before relocation. Notably
-arch_cpu_init_dm() sets up the pin muxing for the chip using a very large table
+fsp_setup_pinctrl() sets up the pin muxing for the chip using a very large table
 in the device tree.
 
 PCI auto-config is not used before relocation, but CONFIG_PCI of course is
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 2/2] event: Correct dependencies on the EVENT framework
  2023-01-14 20:49 [PATCH 1/2] x86: Fix saying arch_cpu_init_dm in debug/docs Tom Rini
@ 2023-01-14 20:49 ` Tom Rini
  2023-01-15  1:31   ` Simon Glass
                     ` (2 more replies)
  2023-01-15  1:31 ` [PATCH 1/2] x86: Fix saying arch_cpu_init_dm in debug/docs Simon Glass
                   ` (2 subsequent siblings)
  3 siblings, 3 replies; 20+ messages in thread
From: Tom Rini @ 2023-01-14 20:49 UTC (permalink / raw)
  To: u-boot; +Cc: Simon Glass, Oliver Graute, Francesco Dolcini

The event framework is just that, a framework. Enabling it by itself
does nothing, so we shouldn't ask the user about it. Reword (and correct
typos) around this the option and help text. This also applies to
DM_EVENT, so reword as well.

With this, it's time to address the larger problems. When functionality
uses events, typically via EVENT_SPY, the appropriate framework then
must be select'd and NOT imply'd. As the functionality will cease to
work (and so, platforms will fail to boot) this is non-optional and
where select is appropriate. Audit the current users of EVENT_SPY to
have a more fine-grained approach to select'ing the framework where
used.

Cc: Simon Glass <sjg@chromium.org>
Reported-by: Oliver Graute <Oliver.Graute@kococonnector.com>
Reported-by: Francesco Dolcini <francesco.dolcini@toradex.com>
Fixes: 7fe32b3442f0 ("event: Convert arch_cpu_init_dm() to use events")
Fixes: 42fdcebf859f ("event: Convert misc_init_f() to use events")
Signed-off-by: Tom Rini <trini@konsulko.com>
---
 arch/Kconfig                     |  6 +++---
 arch/arm/Kconfig                 |  9 ++++-----
 arch/arm/mach-omap2/Kconfig      |  3 +++
 arch/mips/Kconfig                |  2 +-
 arch/powerpc/cpu/mpc85xx/Kconfig |  1 +
 arch/x86/Kconfig                 |  1 +
 arch/x86/cpu/baytrail/Kconfig    |  1 +
 arch/x86/cpu/broadwell/Kconfig   |  1 +
 arch/x86/cpu/ivybridge/Kconfig   |  1 +
 arch/x86/cpu/quark/Kconfig       |  1 +
 board/google/Kconfig             |  1 +
 board/keymile/Kconfig            |  1 +
 boot/Kconfig                     |  1 +
 cmd/Kconfig                      |  1 +
 common/Kconfig                   | 17 ++++++++---------
 drivers/core/Kconfig             |  9 +++++----
 drivers/cpu/Kconfig              |  1 -
 lib/efi_loader/Kconfig           |  5 ++---
 18 files changed, 36 insertions(+), 26 deletions(-)

diff --git a/arch/Kconfig b/arch/Kconfig
index 8fb87b7d857c..d30676ae817b 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -93,7 +93,7 @@ config NIOS2
 	bool "Nios II architecture"
 	select CPU
 	select DM
-	imply DM_EVENT
+	select DM_EVENT
 	select OF_CONTROL
 	select SUPPORT_OF_CONTROL
 	imply CMD_DM
@@ -111,9 +111,9 @@ config RISCV
 	select SUPPORT_OF_CONTROL
 	select OF_CONTROL
 	select DM
+	select DM_EVENT
 	imply SPL_SEPARATE_BSS if SPL
 	imply DM_SERIAL
-	imply DM_EVENT
 	imply DM_MMC
 	imply DM_SPI
 	imply DM_SPI_FLASH
@@ -136,6 +136,7 @@ config SANDBOX
 	select BZIP2
 	select CMD_POWEROFF
 	select DM
+	select DM_EVENT
 	select DM_FUZZING_ENGINE
 	select DM_GPIO
 	select DM_I2C
@@ -240,7 +241,6 @@ config X86
 	imply CMD_SF
 	imply CMD_SF_TEST
 	imply CMD_ZBOOT
-	imply DM_EVENT
 	imply DM_GPIO
 	imply DM_KEYBOARD
 	imply DM_MMC
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index bbf1d5227b3f..c9a44ebc2213 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -778,7 +778,6 @@ config ARCH_OMAP2PLUS
 	select SUPPORT_SPL
 	imply TI_SYSC if DM && OF_CONTROL
 	imply FIT
-	imply DM_EVENT
 	imply SPL_SEPARATE_BSS
 
 config ARCH_MESON
@@ -823,11 +822,11 @@ config ARCH_IMX8
 	select SYS_FSL_SEC_COMPAT_4
 	select SYS_FSL_SEC_LE
 	select DM
+	select DM_EVENT
 	select GPIO_EXTRA_HEADER
 	select MACH_IMX
 	select OF_CONTROL
 	select ENABLE_ARM_SOC_BOOT0_HOOK
-	imply DM_EVENT
 
 config ARCH_IMX8M
 	bool "NXP i.MX8M platform"
@@ -839,14 +838,15 @@ config ARCH_IMX8M
 	select SYS_FSL_SEC_LE
 	select SYS_I2C_MXC
 	select DM
+	select DM_EVENT if CLK
 	select SUPPORT_SPL
 	imply CMD_DM
-	imply DM_EVENT
 
 config ARCH_IMX8ULP
 	bool "NXP i.MX8ULP platform"
 	select ARM64
 	select DM
+	select DM_EVENT
 	select MACH_IMX
 	select OF_CONTROL
 	select SUPPORT_SPL
@@ -854,19 +854,18 @@ config ARCH_IMX8ULP
 	select MISC
 	select IMX_SENTINEL
 	imply CMD_DM
-	imply DM_EVENT
 
 config ARCH_IMX9
 	bool "NXP i.MX9 platform"
 	select ARM64
 	select DM
+	select DM_EVENT
 	select MACH_IMX
 	select SUPPORT_SPL
 	select GPIO_EXTRA_HEADER
 	select MISC
 	select IMX_SENTINEL
 	imply CMD_DM
-	imply DM_EVENT
 
 config ARCH_IMXRT
 	bool "NXP i.MXRT platform"
diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
index 1db71df27212..309b967b0dd5 100644
--- a/arch/arm/mach-omap2/Kconfig
+++ b/arch/arm/mach-omap2/Kconfig
@@ -31,6 +31,7 @@ config OMAP34XX
 
 config OMAP44XX
 	bool "OMAP44XX SoC"
+	select DM_EVENT
 	select SPL_USE_TINY_PRINTF
 	select SPL_SYS_NO_VECTOR_TABLE if SPL
 	imply NAND_OMAP_ELM
@@ -55,6 +56,7 @@ config OMAP54XX
 	bool "OMAP54XX SoC"
 	select ARM_CORTEX_A15_CVE_2017_5715
 	select ARM_ERRATA_798870
+	select DM_EVENT
 	select SYS_THUMB_BUILD
 	imply NAND_OMAP_ELM
 	imply NAND_OMAP_GPMC
@@ -111,6 +113,7 @@ config AM43XX
 config AM33XX
 	bool "AM33XX SoC"
 	select ARM_CORTEX_A8_CVE_2017_5715
+	select DM_EVENT
 	select SPECIFY_CONSOLE_INDEX
 	imply NAND_OMAP_ELM
 	imply NAND_OMAP_GPMC
diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index 23142bd2700d..569f5f48bc6c 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -121,6 +121,7 @@ config MACH_PIC32
 	bool "Support Microchip PIC32"
 	select HAS_FIXED_TIMER_FREQUENCY
 	select DM
+	select DM_EVENT
 	select OF_CONTROL
 	imply CMD_DM
 
@@ -128,7 +129,6 @@ config TARGET_BOSTON
 	bool "Support Boston"
 	select HAS_FIXED_TIMER_FREQUENCY
 	select DM
-	imply DM_EVENT
 	select DM_SERIAL
 	select MIPS_CM
 	select SYS_CACHE_SHIFT_6
diff --git a/arch/powerpc/cpu/mpc85xx/Kconfig b/arch/powerpc/cpu/mpc85xx/Kconfig
index 1b180481a483..2c54a9e2120f 100644
--- a/arch/powerpc/cpu/mpc85xx/Kconfig
+++ b/arch/powerpc/cpu/mpc85xx/Kconfig
@@ -248,6 +248,7 @@ config TARGET_KMP204X
 config TARGET_KMCENT2
 	bool "Support kmcent2"
 	select VENDOR_KM
+	select EVENT
 	select FSL_CORENET
 	select SYS_DPAA_FMAN
 	select SYS_DPAA_PME
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 93f1c77be3f3..07be5cd05ec0 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -395,6 +395,7 @@ config FSP_VERSION1
 
 config FSP_VERSION2
 	bool "FSP version 2.x"
+	select DM_EVENT
 	help
 	  This covers versions 2.0 and 2.1. See here for details:
 	  https://github.com/IntelFsp/fsp/wiki
diff --git a/arch/x86/cpu/baytrail/Kconfig b/arch/x86/cpu/baytrail/Kconfig
index d2c3473d6abf..a8efea8a3413 100644
--- a/arch/x86/cpu/baytrail/Kconfig
+++ b/arch/x86/cpu/baytrail/Kconfig
@@ -7,6 +7,7 @@ config INTEL_BAYTRAIL
 	select HAVE_FSP
 	select ARCH_MISC_INIT
 	select CPU_INTEL_TURBO_NOT_PACKAGE_SCOPED
+	select DM_EVENT
 	imply HAVE_INTEL_ME
 	imply ENABLE_MRC_CACHE
 	imply AHCI_PCI
diff --git a/arch/x86/cpu/broadwell/Kconfig b/arch/x86/cpu/broadwell/Kconfig
index 5b015c89d950..39deda364479 100644
--- a/arch/x86/cpu/broadwell/Kconfig
+++ b/arch/x86/cpu/broadwell/Kconfig
@@ -6,6 +6,7 @@
 config INTEL_BROADWELL
 	bool
 	select CACHE_MRC_BIN
+	select DM_EVENT
 	select ARCH_EARLY_INIT_R
 	imply HAVE_INTEL_ME
 	imply ENABLE_MRC_CACHE
diff --git a/arch/x86/cpu/ivybridge/Kconfig b/arch/x86/cpu/ivybridge/Kconfig
index be3ef5e5d8f8..704f145adf88 100644
--- a/arch/x86/cpu/ivybridge/Kconfig
+++ b/arch/x86/cpu/ivybridge/Kconfig
@@ -8,6 +8,7 @@
 config NORTHBRIDGE_INTEL_IVYBRIDGE
 	bool
 	select CACHE_MRC_BIN if HAVE_MRC
+	select DM_EVENT
 	imply HAVE_INTEL_ME
 	imply ENABLE_MRC_CACHE
 	imply AHCI_PCI
diff --git a/arch/x86/cpu/quark/Kconfig b/arch/x86/cpu/quark/Kconfig
index 61bb5792c868..0d4008a31f4c 100644
--- a/arch/x86/cpu/quark/Kconfig
+++ b/arch/x86/cpu/quark/Kconfig
@@ -7,6 +7,7 @@ config INTEL_QUARK
 	select HAVE_RMU
 	select ARCH_EARLY_INIT_R
 	select ARCH_MISC_INIT
+	select DM_EVENT
 	imply ENABLE_MRC_CACHE
 	imply ETH_DESIGNWARE
 	imply ICH_SPI
diff --git a/board/google/Kconfig b/board/google/Kconfig
index 0474b4e69384..a0f1a6097641 100644
--- a/board/google/Kconfig
+++ b/board/google/Kconfig
@@ -18,6 +18,7 @@ choice
 config TARGET_CHROMEBOOK_CORAL
 	bool "Chromebook coral"
 	select BIOSEMU
+	select EVENT
 	help
 	  This is a range of Intel-based laptops released in 2018. They use an
 	  Intel Apollo Lake SoC. The design supports WiFi, 4GB to 16GB of
diff --git a/board/keymile/Kconfig b/board/keymile/Kconfig
index e5d7c80a869d..bf899d005c46 100644
--- a/board/keymile/Kconfig
+++ b/board/keymile/Kconfig
@@ -124,6 +124,7 @@ config SYS_IVM_EEPROM_PAGE_LEN
 
 config PG_WCOM_UBOOT_UPDATE_SUPPORTED
 	bool "Enable U-boot Field Fail-Safe Update Functionality"
+	select EVENT
 	default n
 	help
 	  Indicates that field fail-safe u-boot update is supported.
diff --git a/boot/Kconfig b/boot/Kconfig
index 30bc182fcd5c..725afd36a3b9 100644
--- a/boot/Kconfig
+++ b/boot/Kconfig
@@ -474,6 +474,7 @@ config BOOTMETH_VBE
 	depends on FIT
 	default y
 	select BOOTMETH_GLOBAL
+	select EVENT
 	help
 	  Enables support for VBE boot. This is a standard boot method which
 	  supports selection of various firmware components, seleciton of an OS to
diff --git a/cmd/Kconfig b/cmd/Kconfig
index b2aefae9cb70..4fe2c75de256 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -2622,6 +2622,7 @@ config CMD_DIAG
 
 config CMD_EVENT
 	bool "event - Show information about events"
+	depends on EVENT
 	default y if EVENT_DEBUG
 	help
 	  This enables the 'event' command which provides information about
diff --git a/common/Kconfig b/common/Kconfig
index 73e3fe36573d..6f5aa4e53aae 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -604,24 +604,23 @@ config CYCLIC_MAX_CPU_TIME_US
 endif # CYCLIC
 
 config EVENT
-	bool "General-purpose event-handling mechanism"
-	default y if SANDBOX
+	bool
 	help
-	  This enables sending and processing of events, to allow interested
-	  parties to be alerted when something happens. This is an attempt to
-	  stem the flow of weak functions, hooks, functions in board_f.c
-	  and board_r.c and the Kconfig options below.
+	  This adds a framework for general purpose sending and processing of
+	  events, to allow interested parties to be alerted when something
+	  happens. This is an attempt to stem 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
+	bool
 	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
+	  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
diff --git a/drivers/core/Kconfig b/drivers/core/Kconfig
index 8fde77c23ee0..6fc8854b574b 100644
--- a/drivers/core/Kconfig
+++ b/drivers/core/Kconfig
@@ -109,13 +109,14 @@ config DM_DEVICE_REMOVE
 	  causes USB host controllers to not be stopped when booting the OS.
 
 config DM_EVENT
-	bool "Support events with driver model"
-	depends on DM && EVENT
-	default y if SANDBOX
+	bool
+	depends on DM
+	select EVENT
 	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.
+	  register a 'spy' function that is called when the event occurs. Such
+	  subsystems must select this option.
 
 config SPL_DM_DEVICE_REMOVE
 	bool "Support device removal in SPL"
diff --git a/drivers/cpu/Kconfig b/drivers/cpu/Kconfig
index 21874335c873..3bf04105e5e9 100644
--- a/drivers/cpu/Kconfig
+++ b/drivers/cpu/Kconfig
@@ -23,7 +23,6 @@ config CPU_RISCV
 config CPU_MICROBLAZE
 	bool "Enable Microblaze CPU driver"
 	depends on CPU && MICROBLAZE
-	select EVENT
 	select DM_EVENT
 	select XILINX_MICROBLAZE0_PVR
 	help
diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
index b498c72206fd..d04203cddab4 100644
--- a/lib/efi_loader/Kconfig
+++ b/lib/efi_loader/Kconfig
@@ -14,9 +14,8 @@ config EFI_LOADER
 	depends on !EFI_APP
 	default y if !ARM || SYS_CPU = armv7 || SYS_CPU = armv8
 	select CHARSET
-	select DM_EVENT
-	select EVENT
-	select EVENT_DYNAMIC
+	select DM_EVENT if PARTITIONS
+	select EVENT_DYNAMIC if EVENT
 	select LIB_UUID
 	imply PARTITION_UUIDS
 	select REGEX
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/2] event: Correct dependencies on the EVENT framework
  2023-01-14 20:49 ` [PATCH 2/2] event: Correct dependencies on the EVENT framework Tom Rini
@ 2023-01-15  1:31   ` Simon Glass
  2023-01-15 13:45     ` Tom Rini
  2023-01-15 12:29   ` Fabio Estevam
  2023-01-16  3:10   ` AKASHI Takahiro
  2 siblings, 1 reply; 20+ messages in thread
From: Simon Glass @ 2023-01-15  1:31 UTC (permalink / raw)
  To: Tom Rini; +Cc: u-boot, Oliver Graute, Francesco Dolcini

Hi Tom,

On Sat, 14 Jan 2023 at 13:49, Tom Rini <trini@konsulko.com> wrote:
>
> The event framework is just that, a framework. Enabling it by itself
> does nothing, so we shouldn't ask the user about it. Reword (and correct
> typos) around this the option and help text. This also applies to
> DM_EVENT, so reword as well.
>
> With this, it's time to address the larger problems. When functionality
> uses events, typically via EVENT_SPY, the appropriate framework then
> must be select'd and NOT imply'd. As the functionality will cease to
> work (and so, platforms will fail to boot) this is non-optional and
> where select is appropriate. Audit the current users of EVENT_SPY to
> have a more fine-grained approach to select'ing the framework where
> used.
>
> Cc: Simon Glass <sjg@chromium.org>
> Reported-by: Oliver Graute <Oliver.Graute@kococonnector.com>
> Reported-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> Fixes: 7fe32b3442f0 ("event: Convert arch_cpu_init_dm() to use events")
> Fixes: 42fdcebf859f ("event: Convert misc_init_f() to use events")
> Signed-off-by: Tom Rini <trini@konsulko.com>
> ---
>  arch/Kconfig                     |  6 +++---
>  arch/arm/Kconfig                 |  9 ++++-----
>  arch/arm/mach-omap2/Kconfig      |  3 +++
>  arch/mips/Kconfig                |  2 +-
>  arch/powerpc/cpu/mpc85xx/Kconfig |  1 +
>  arch/x86/Kconfig                 |  1 +
>  arch/x86/cpu/baytrail/Kconfig    |  1 +
>  arch/x86/cpu/broadwell/Kconfig   |  1 +
>  arch/x86/cpu/ivybridge/Kconfig   |  1 +
>  arch/x86/cpu/quark/Kconfig       |  1 +
>  board/google/Kconfig             |  1 +
>  board/keymile/Kconfig            |  1 +
>  boot/Kconfig                     |  1 +
>  cmd/Kconfig                      |  1 +
>  common/Kconfig                   | 17 ++++++++---------
>  drivers/core/Kconfig             |  9 +++++----
>  drivers/cpu/Kconfig              |  1 -

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

Tested on chromebook-coral:
Tested-by: Simon Glass <sjg@chromium.org>

I am not quite sure what the effective difference is between select
and imply. Doesn't this suggest that there are some conflicting config
options? The only way imply would be disabled ( I thought) is if a
board does it explicitly?

Regards,
Simon

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] x86: Fix saying arch_cpu_init_dm in debug/docs
  2023-01-14 20:49 [PATCH 1/2] x86: Fix saying arch_cpu_init_dm in debug/docs Tom Rini
  2023-01-14 20:49 ` [PATCH 2/2] event: Correct dependencies on the EVENT framework Tom Rini
@ 2023-01-15  1:31 ` Simon Glass
  2023-01-16 20:46 ` [v3 2/2] event: Correct dependencies on the EVENT framework Tom Rini
  2023-01-19 14:46 ` [PATCH 1/2] x86: Fix saying arch_cpu_init_dm in debug/docs Tom Rini
  3 siblings, 0 replies; 20+ messages in thread
From: Simon Glass @ 2023-01-15  1:31 UTC (permalink / raw)
  To: Tom Rini; +Cc: u-boot

On Sat, 14 Jan 2023 at 13:49, Tom Rini <trini@konsulko.com> wrote:
>
> The function arch_cpu_init_dm was renamed to fsp_setup_pinctrl in these
> cases, so rename debug / docs to match.
>
> Cc: Simon Glass <sjg@chromium.org>
> Fixes: 7fe32b3442f0 ("event: Convert arch_cpu_init_dm() to use events")
> Signed-off-by: Tom Rini <trini@konsulko.com>
> ---
>  arch/x86/lib/spl.c                    | 2 +-
>  doc/board/google/chromebook_coral.rst | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/2] event: Correct dependencies on the EVENT framework
  2023-01-14 20:49 ` [PATCH 2/2] event: Correct dependencies on the EVENT framework Tom Rini
  2023-01-15  1:31   ` Simon Glass
@ 2023-01-15 12:29   ` Fabio Estevam
  2023-01-16  3:10   ` AKASHI Takahiro
  2 siblings, 0 replies; 20+ messages in thread
From: Fabio Estevam @ 2023-01-15 12:29 UTC (permalink / raw)
  To: Tom Rini; +Cc: u-boot, Simon Glass, Oliver Graute, Francesco Dolcini

Hi Tom,

On Sat, Jan 14, 2023 at 5:50 PM Tom Rini <trini@konsulko.com> wrote:
>
> The event framework is just that, a framework. Enabling it by itself
> does nothing, so we shouldn't ask the user about it. Reword (and correct
> typos) around this the option and help text. This also applies to
> DM_EVENT, so reword as well.
>
> With this, it's time to address the larger problems. When functionality
> uses events, typically via EVENT_SPY, the appropriate framework then
> must be select'd and NOT imply'd. As the functionality will cease to
> work (and so, platforms will fail to boot) this is non-optional and
> where select is appropriate. Audit the current users of EVENT_SPY to
> have a more fine-grained approach to select'ing the framework where
> used.
>
> Cc: Simon Glass <sjg@chromium.org>
> Reported-by: Oliver Graute <Oliver.Graute@kococonnector.com>
> Reported-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> Fixes: 7fe32b3442f0 ("event: Convert arch_cpu_init_dm() to use events")
> Fixes: 42fdcebf859f ("event: Convert misc_init_f() to use events")
> Signed-off-by: Tom Rini <trini@konsulko.com>

Thanks for investigating and providing a proper fix:

Reviewed-by: Fabio Estevam <festevam@denx.de>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/2] event: Correct dependencies on the EVENT framework
  2023-01-15  1:31   ` Simon Glass
@ 2023-01-15 13:45     ` Tom Rini
  2023-01-15 21:52       ` Simon Glass
  0 siblings, 1 reply; 20+ messages in thread
From: Tom Rini @ 2023-01-15 13:45 UTC (permalink / raw)
  To: Simon Glass; +Cc: u-boot, Oliver Graute, Francesco Dolcini

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

On Sat, Jan 14, 2023 at 06:31:05PM -0700, Simon Glass wrote:
> Hi Tom,
> 
> On Sat, 14 Jan 2023 at 13:49, Tom Rini <trini@konsulko.com> wrote:
> >
> > The event framework is just that, a framework. Enabling it by itself
> > does nothing, so we shouldn't ask the user about it. Reword (and correct
> > typos) around this the option and help text. This also applies to
> > DM_EVENT, so reword as well.
> >
> > With this, it's time to address the larger problems. When functionality
> > uses events, typically via EVENT_SPY, the appropriate framework then
> > must be select'd and NOT imply'd. As the functionality will cease to
> > work (and so, platforms will fail to boot) this is non-optional and
> > where select is appropriate. Audit the current users of EVENT_SPY to
> > have a more fine-grained approach to select'ing the framework where
> > used.
> >
> > Cc: Simon Glass <sjg@chromium.org>
> > Reported-by: Oliver Graute <Oliver.Graute@kococonnector.com>
> > Reported-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> > Fixes: 7fe32b3442f0 ("event: Convert arch_cpu_init_dm() to use events")
> > Fixes: 42fdcebf859f ("event: Convert misc_init_f() to use events")
> > Signed-off-by: Tom Rini <trini@konsulko.com>
> > ---
> >  arch/Kconfig                     |  6 +++---
> >  arch/arm/Kconfig                 |  9 ++++-----
> >  arch/arm/mach-omap2/Kconfig      |  3 +++
> >  arch/mips/Kconfig                |  2 +-
> >  arch/powerpc/cpu/mpc85xx/Kconfig |  1 +
> >  arch/x86/Kconfig                 |  1 +
> >  arch/x86/cpu/baytrail/Kconfig    |  1 +
> >  arch/x86/cpu/broadwell/Kconfig   |  1 +
> >  arch/x86/cpu/ivybridge/Kconfig   |  1 +
> >  arch/x86/cpu/quark/Kconfig       |  1 +
> >  board/google/Kconfig             |  1 +
> >  board/keymile/Kconfig            |  1 +
> >  boot/Kconfig                     |  1 +
> >  cmd/Kconfig                      |  1 +
> >  common/Kconfig                   | 17 ++++++++---------
> >  drivers/core/Kconfig             |  9 +++++----
> >  drivers/cpu/Kconfig              |  1 -
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>
> 
> Tested on chromebook-coral:
> Tested-by: Simon Glass <sjg@chromium.org>
> 
> I am not quite sure what the effective difference is between select
> and imply. Doesn't this suggest that there are some conflicting config
> options? The only way imply would be disabled ( I thought) is if a
> board does it explicitly?

So there's two parts to it. As a framework, the option needs to be
select'd, not implied. It doesn't make sense to disable the event
framework and then wonder why VBE doesn't work. The next part however is

Fixes: c5ef2025579e ("dm: fix DM_EVENT dependencies")

should have been part of it too. That commit turned all of the platforms
which had imply DM_EVENT (and so from DM_EVENT, imply EVENT) in to
depends on EVENT being set, and if it wasn't set (or select'd, only
EFI_LOADER was select'ing it at the time) in to not enabling DM_EVENT,
and since imply options can be off, no warnings were thrown by the build
system. And since all of the non-dynamic EVENT stuff is linker-based,
no binary size changes happened either.

-- 
Tom

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/2] event: Correct dependencies on the EVENT framework
  2023-01-15 13:45     ` Tom Rini
@ 2023-01-15 21:52       ` Simon Glass
  2023-01-15 22:04         ` Tom Rini
  0 siblings, 1 reply; 20+ messages in thread
From: Simon Glass @ 2023-01-15 21:52 UTC (permalink / raw)
  To: Tom Rini; +Cc: u-boot, Oliver Graute, Francesco Dolcini

Hi Tom,

On Sun, 15 Jan 2023 at 06:45, Tom Rini <trini@konsulko.com> wrote:
>
> On Sat, Jan 14, 2023 at 06:31:05PM -0700, Simon Glass wrote:
> > Hi Tom,
> >
> > On Sat, 14 Jan 2023 at 13:49, Tom Rini <trini@konsulko.com> wrote:
> > >
> > > The event framework is just that, a framework. Enabling it by itself
> > > does nothing, so we shouldn't ask the user about it. Reword (and correct
> > > typos) around this the option and help text. This also applies to
> > > DM_EVENT, so reword as well.
> > >
> > > With this, it's time to address the larger problems. When functionality
> > > uses events, typically via EVENT_SPY, the appropriate framework then
> > > must be select'd and NOT imply'd. As the functionality will cease to
> > > work (and so, platforms will fail to boot) this is non-optional and
> > > where select is appropriate. Audit the current users of EVENT_SPY to
> > > have a more fine-grained approach to select'ing the framework where
> > > used.
> > >
> > > Cc: Simon Glass <sjg@chromium.org>
> > > Reported-by: Oliver Graute <Oliver.Graute@kococonnector.com>
> > > Reported-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> > > Fixes: 7fe32b3442f0 ("event: Convert arch_cpu_init_dm() to use events")
> > > Fixes: 42fdcebf859f ("event: Convert misc_init_f() to use events")
> > > Signed-off-by: Tom Rini <trini@konsulko.com>
> > > ---
> > >  arch/Kconfig                     |  6 +++---
> > >  arch/arm/Kconfig                 |  9 ++++-----
> > >  arch/arm/mach-omap2/Kconfig      |  3 +++
> > >  arch/mips/Kconfig                |  2 +-
> > >  arch/powerpc/cpu/mpc85xx/Kconfig |  1 +
> > >  arch/x86/Kconfig                 |  1 +
> > >  arch/x86/cpu/baytrail/Kconfig    |  1 +
> > >  arch/x86/cpu/broadwell/Kconfig   |  1 +
> > >  arch/x86/cpu/ivybridge/Kconfig   |  1 +
> > >  arch/x86/cpu/quark/Kconfig       |  1 +
> > >  board/google/Kconfig             |  1 +
> > >  board/keymile/Kconfig            |  1 +
> > >  boot/Kconfig                     |  1 +
> > >  cmd/Kconfig                      |  1 +
> > >  common/Kconfig                   | 17 ++++++++---------
> > >  drivers/core/Kconfig             |  9 +++++----
> > >  drivers/cpu/Kconfig              |  1 -
> >
> > Reviewed-by: Simon Glass <sjg@chromium.org>
> >
> > Tested on chromebook-coral:
> > Tested-by: Simon Glass <sjg@chromium.org>
> >
> > I am not quite sure what the effective difference is between select
> > and imply. Doesn't this suggest that there are some conflicting config
> > options? The only way imply would be disabled ( I thought) is if a
> > board does it explicitly?
>
> So there's two parts to it. As a framework, the option needs to be
> select'd, not implied. It doesn't make sense to disable the event
> framework and then wonder why VBE doesn't work. The next part however is
>
> Fixes: c5ef2025579e ("dm: fix DM_EVENT dependencies")

OK

>
> should have been part of it too. That commit turned all of the platforms
> which had imply DM_EVENT (and so from DM_EVENT, imply EVENT) in to
> depends on EVENT being set, and if it wasn't set (or select'd, only
> EFI_LOADER was select'ing it at the time) in to not enabling DM_EVENT,
> and since imply options can be off, no warnings were thrown by the build
> system. And since all of the non-dynamic EVENT stuff is linker-based,
> no binary size changes happened either.

So does this mean that select is 'forcing' the option to be enabled,
even if it conflicts with something else, or depends on something
which is not set?

I'd quite like to understand this better for the future. Could you
point to a board that was broken and now fixed, so I can have a fiddle
with it?

Thanks,
Simon

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/2] event: Correct dependencies on the EVENT framework
  2023-01-15 21:52       ` Simon Glass
@ 2023-01-15 22:04         ` Tom Rini
  2023-01-21  0:24           ` Simon Glass
  0 siblings, 1 reply; 20+ messages in thread
From: Tom Rini @ 2023-01-15 22:04 UTC (permalink / raw)
  To: Simon Glass; +Cc: u-boot, Oliver Graute, Francesco Dolcini

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

On Sun, Jan 15, 2023 at 02:52:35PM -0700, Simon Glass wrote:
> Hi Tom,
> 
> On Sun, 15 Jan 2023 at 06:45, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Sat, Jan 14, 2023 at 06:31:05PM -0700, Simon Glass wrote:
> > > Hi Tom,
> > >
> > > On Sat, 14 Jan 2023 at 13:49, Tom Rini <trini@konsulko.com> wrote:
> > > >
> > > > The event framework is just that, a framework. Enabling it by itself
> > > > does nothing, so we shouldn't ask the user about it. Reword (and correct
> > > > typos) around this the option and help text. This also applies to
> > > > DM_EVENT, so reword as well.
> > > >
> > > > With this, it's time to address the larger problems. When functionality
> > > > uses events, typically via EVENT_SPY, the appropriate framework then
> > > > must be select'd and NOT imply'd. As the functionality will cease to
> > > > work (and so, platforms will fail to boot) this is non-optional and
> > > > where select is appropriate. Audit the current users of EVENT_SPY to
> > > > have a more fine-grained approach to select'ing the framework where
> > > > used.
> > > >
> > > > Cc: Simon Glass <sjg@chromium.org>
> > > > Reported-by: Oliver Graute <Oliver.Graute@kococonnector.com>
> > > > Reported-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> > > > Fixes: 7fe32b3442f0 ("event: Convert arch_cpu_init_dm() to use events")
> > > > Fixes: 42fdcebf859f ("event: Convert misc_init_f() to use events")
> > > > Signed-off-by: Tom Rini <trini@konsulko.com>
> > > > ---
> > > >  arch/Kconfig                     |  6 +++---
> > > >  arch/arm/Kconfig                 |  9 ++++-----
> > > >  arch/arm/mach-omap2/Kconfig      |  3 +++
> > > >  arch/mips/Kconfig                |  2 +-
> > > >  arch/powerpc/cpu/mpc85xx/Kconfig |  1 +
> > > >  arch/x86/Kconfig                 |  1 +
> > > >  arch/x86/cpu/baytrail/Kconfig    |  1 +
> > > >  arch/x86/cpu/broadwell/Kconfig   |  1 +
> > > >  arch/x86/cpu/ivybridge/Kconfig   |  1 +
> > > >  arch/x86/cpu/quark/Kconfig       |  1 +
> > > >  board/google/Kconfig             |  1 +
> > > >  board/keymile/Kconfig            |  1 +
> > > >  boot/Kconfig                     |  1 +
> > > >  cmd/Kconfig                      |  1 +
> > > >  common/Kconfig                   | 17 ++++++++---------
> > > >  drivers/core/Kconfig             |  9 +++++----
> > > >  drivers/cpu/Kconfig              |  1 -
> > >
> > > Reviewed-by: Simon Glass <sjg@chromium.org>
> > >
> > > Tested on chromebook-coral:
> > > Tested-by: Simon Glass <sjg@chromium.org>
> > >
> > > I am not quite sure what the effective difference is between select
> > > and imply. Doesn't this suggest that there are some conflicting config
> > > options? The only way imply would be disabled ( I thought) is if a
> > > board does it explicitly?
> >
> > So there's two parts to it. As a framework, the option needs to be
> > select'd, not implied. It doesn't make sense to disable the event
> > framework and then wonder why VBE doesn't work. The next part however is
> >
> > Fixes: c5ef2025579e ("dm: fix DM_EVENT dependencies")
> 
> OK
> 
> >
> > should have been part of it too. That commit turned all of the platforms
> > which had imply DM_EVENT (and so from DM_EVENT, imply EVENT) in to
> > depends on EVENT being set, and if it wasn't set (or select'd, only
> > EFI_LOADER was select'ing it at the time) in to not enabling DM_EVENT,
> > and since imply options can be off, no warnings were thrown by the build
> > system. And since all of the non-dynamic EVENT stuff is linker-based,
> > no binary size changes happened either.
> 
> So does this mean that select is 'forcing' the option to be enabled,
> even if it conflicts with something else, or depends on something
> which is not set?
> 
> I'd quite like to understand this better for the future. Could you
> point to a board that was broken and now fixed, so I can have a fiddle
> with it?

While kernel centric at times (of course)
https://www.kernel.org/doc/html/latest/kbuild/kconfig-language.html#menu-attributes
covers things well, especially for the language itself. So yes, select
forces the option to be enabled. If there's an unmet (depends on ...)
dependency, then it will emit a warning to tell you as such. An example
of a previously broken board would be apalis-imx8 as it does not enable
EFI_LOADER so no DM_EVENT / EVENT and so:
arch/arm/mach-imx/imx8/cpu.c:EVENT_SPY(EVT_DM_POST_INIT, imx8_init_mu);
wasn't called and the platform would silently fail to boot.

-- 
Tom

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/2] event: Correct dependencies on the EVENT framework
  2023-01-14 20:49 ` [PATCH 2/2] event: Correct dependencies on the EVENT framework Tom Rini
  2023-01-15  1:31   ` Simon Glass
  2023-01-15 12:29   ` Fabio Estevam
@ 2023-01-16  3:10   ` AKASHI Takahiro
  2023-01-16  3:15     ` Tom Rini
  2 siblings, 1 reply; 20+ messages in thread
From: AKASHI Takahiro @ 2023-01-16  3:10 UTC (permalink / raw)
  To: Tom Rini; +Cc: u-boot, Simon Glass, Oliver Graute, Francesco Dolcini

Hi Tom,

On Sat, Jan 14, 2023 at 03:49:36PM -0500, Tom Rini wrote:
> The event framework is just that, a framework. Enabling it by itself
> does nothing, so we shouldn't ask the user about it. Reword (and correct
> typos) around this the option and help text. This also applies to
> DM_EVENT, so reword as well.
> 
> With this, it's time to address the larger problems. When functionality
> uses events, typically via EVENT_SPY, the appropriate framework then
> must be select'd and NOT imply'd. As the functionality will cease to
> work (and so, platforms will fail to boot) this is non-optional and
> where select is appropriate. Audit the current users of EVENT_SPY to
> have a more fine-grained approach to select'ing the framework where
> used.
> 
> Cc: Simon Glass <sjg@chromium.org>
> Reported-by: Oliver Graute <Oliver.Graute@kococonnector.com>
> Reported-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> Fixes: 7fe32b3442f0 ("event: Convert arch_cpu_init_dm() to use events")
> Fixes: 42fdcebf859f ("event: Convert misc_init_f() to use events")
> Signed-off-by: Tom Rini <trini@konsulko.com>
> ---
>  arch/Kconfig                     |  6 +++---
>  arch/arm/Kconfig                 |  9 ++++-----
>  arch/arm/mach-omap2/Kconfig      |  3 +++
>  arch/mips/Kconfig                |  2 +-
>  arch/powerpc/cpu/mpc85xx/Kconfig |  1 +
>  arch/x86/Kconfig                 |  1 +
>  arch/x86/cpu/baytrail/Kconfig    |  1 +
>  arch/x86/cpu/broadwell/Kconfig   |  1 +
>  arch/x86/cpu/ivybridge/Kconfig   |  1 +
>  arch/x86/cpu/quark/Kconfig       |  1 +
>  board/google/Kconfig             |  1 +
>  board/keymile/Kconfig            |  1 +
>  boot/Kconfig                     |  1 +
>  cmd/Kconfig                      |  1 +
>  common/Kconfig                   | 17 ++++++++---------
>  drivers/core/Kconfig             |  9 +++++----
>  drivers/cpu/Kconfig              |  1 -
>  lib/efi_loader/Kconfig           |  5 ++---
>  18 files changed, 36 insertions(+), 26 deletions(-)
> 
> diff --git a/arch/Kconfig b/arch/Kconfig
> index 8fb87b7d857c..d30676ae817b 100644
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -93,7 +93,7 @@ config NIOS2
>  	bool "Nios II architecture"
>  	select CPU
>  	select DM
> -	imply DM_EVENT
> +	select DM_EVENT
>  	select OF_CONTROL
>  	select SUPPORT_OF_CONTROL
>  	imply CMD_DM
> @@ -111,9 +111,9 @@ config RISCV
>  	select SUPPORT_OF_CONTROL
>  	select OF_CONTROL
>  	select DM
> +	select DM_EVENT
>  	imply SPL_SEPARATE_BSS if SPL
>  	imply DM_SERIAL
> -	imply DM_EVENT
>  	imply DM_MMC
>  	imply DM_SPI
>  	imply DM_SPI_FLASH
> @@ -136,6 +136,7 @@ config SANDBOX
>  	select BZIP2
>  	select CMD_POWEROFF
>  	select DM
> +	select DM_EVENT
>  	select DM_FUZZING_ENGINE
>  	select DM_GPIO
>  	select DM_I2C
> @@ -240,7 +241,6 @@ config X86
>  	imply CMD_SF
>  	imply CMD_SF_TEST
>  	imply CMD_ZBOOT
> -	imply DM_EVENT
>  	imply DM_GPIO
>  	imply DM_KEYBOARD
>  	imply DM_MMC
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index bbf1d5227b3f..c9a44ebc2213 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -778,7 +778,6 @@ config ARCH_OMAP2PLUS
>  	select SUPPORT_SPL
>  	imply TI_SYSC if DM && OF_CONTROL
>  	imply FIT
> -	imply DM_EVENT
>  	imply SPL_SEPARATE_BSS
>  
>  config ARCH_MESON
> @@ -823,11 +822,11 @@ config ARCH_IMX8
>  	select SYS_FSL_SEC_COMPAT_4
>  	select SYS_FSL_SEC_LE
>  	select DM
> +	select DM_EVENT
>  	select GPIO_EXTRA_HEADER
>  	select MACH_IMX
>  	select OF_CONTROL
>  	select ENABLE_ARM_SOC_BOOT0_HOOK
> -	imply DM_EVENT
>  
>  config ARCH_IMX8M
>  	bool "NXP i.MX8M platform"
> @@ -839,14 +838,15 @@ config ARCH_IMX8M
>  	select SYS_FSL_SEC_LE
>  	select SYS_I2C_MXC
>  	select DM
> +	select DM_EVENT if CLK
>  	select SUPPORT_SPL
>  	imply CMD_DM
> -	imply DM_EVENT
>  
>  config ARCH_IMX8ULP
>  	bool "NXP i.MX8ULP platform"
>  	select ARM64
>  	select DM
> +	select DM_EVENT
>  	select MACH_IMX
>  	select OF_CONTROL
>  	select SUPPORT_SPL
> @@ -854,19 +854,18 @@ config ARCH_IMX8ULP
>  	select MISC
>  	select IMX_SENTINEL
>  	imply CMD_DM
> -	imply DM_EVENT
>  
>  config ARCH_IMX9
>  	bool "NXP i.MX9 platform"
>  	select ARM64
>  	select DM
> +	select DM_EVENT
>  	select MACH_IMX
>  	select SUPPORT_SPL
>  	select GPIO_EXTRA_HEADER
>  	select MISC
>  	select IMX_SENTINEL
>  	imply CMD_DM
> -	imply DM_EVENT
>  
>  config ARCH_IMXRT
>  	bool "NXP i.MXRT platform"
> diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
> index 1db71df27212..309b967b0dd5 100644
> --- a/arch/arm/mach-omap2/Kconfig
> +++ b/arch/arm/mach-omap2/Kconfig
> @@ -31,6 +31,7 @@ config OMAP34XX
>  
>  config OMAP44XX
>  	bool "OMAP44XX SoC"
> +	select DM_EVENT
>  	select SPL_USE_TINY_PRINTF
>  	select SPL_SYS_NO_VECTOR_TABLE if SPL
>  	imply NAND_OMAP_ELM
> @@ -55,6 +56,7 @@ config OMAP54XX
>  	bool "OMAP54XX SoC"
>  	select ARM_CORTEX_A15_CVE_2017_5715
>  	select ARM_ERRATA_798870
> +	select DM_EVENT
>  	select SYS_THUMB_BUILD
>  	imply NAND_OMAP_ELM
>  	imply NAND_OMAP_GPMC
> @@ -111,6 +113,7 @@ config AM43XX
>  config AM33XX
>  	bool "AM33XX SoC"
>  	select ARM_CORTEX_A8_CVE_2017_5715
> +	select DM_EVENT
>  	select SPECIFY_CONSOLE_INDEX
>  	imply NAND_OMAP_ELM
>  	imply NAND_OMAP_GPMC
> diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
> index 23142bd2700d..569f5f48bc6c 100644
> --- a/arch/mips/Kconfig
> +++ b/arch/mips/Kconfig
> @@ -121,6 +121,7 @@ config MACH_PIC32
>  	bool "Support Microchip PIC32"
>  	select HAS_FIXED_TIMER_FREQUENCY
>  	select DM
> +	select DM_EVENT
>  	select OF_CONTROL
>  	imply CMD_DM
>  
> @@ -128,7 +129,6 @@ config TARGET_BOSTON
>  	bool "Support Boston"
>  	select HAS_FIXED_TIMER_FREQUENCY
>  	select DM
> -	imply DM_EVENT
>  	select DM_SERIAL
>  	select MIPS_CM
>  	select SYS_CACHE_SHIFT_6
> diff --git a/arch/powerpc/cpu/mpc85xx/Kconfig b/arch/powerpc/cpu/mpc85xx/Kconfig
> index 1b180481a483..2c54a9e2120f 100644
> --- a/arch/powerpc/cpu/mpc85xx/Kconfig
> +++ b/arch/powerpc/cpu/mpc85xx/Kconfig
> @@ -248,6 +248,7 @@ config TARGET_KMP204X
>  config TARGET_KMCENT2
>  	bool "Support kmcent2"
>  	select VENDOR_KM
> +	select EVENT
>  	select FSL_CORENET
>  	select SYS_DPAA_FMAN
>  	select SYS_DPAA_PME
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 93f1c77be3f3..07be5cd05ec0 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -395,6 +395,7 @@ config FSP_VERSION1
>  
>  config FSP_VERSION2
>  	bool "FSP version 2.x"
> +	select DM_EVENT
>  	help
>  	  This covers versions 2.0 and 2.1. See here for details:
>  	  https://github.com/IntelFsp/fsp/wiki
> diff --git a/arch/x86/cpu/baytrail/Kconfig b/arch/x86/cpu/baytrail/Kconfig
> index d2c3473d6abf..a8efea8a3413 100644
> --- a/arch/x86/cpu/baytrail/Kconfig
> +++ b/arch/x86/cpu/baytrail/Kconfig
> @@ -7,6 +7,7 @@ config INTEL_BAYTRAIL
>  	select HAVE_FSP
>  	select ARCH_MISC_INIT
>  	select CPU_INTEL_TURBO_NOT_PACKAGE_SCOPED
> +	select DM_EVENT
>  	imply HAVE_INTEL_ME
>  	imply ENABLE_MRC_CACHE
>  	imply AHCI_PCI
> diff --git a/arch/x86/cpu/broadwell/Kconfig b/arch/x86/cpu/broadwell/Kconfig
> index 5b015c89d950..39deda364479 100644
> --- a/arch/x86/cpu/broadwell/Kconfig
> +++ b/arch/x86/cpu/broadwell/Kconfig
> @@ -6,6 +6,7 @@
>  config INTEL_BROADWELL
>  	bool
>  	select CACHE_MRC_BIN
> +	select DM_EVENT
>  	select ARCH_EARLY_INIT_R
>  	imply HAVE_INTEL_ME
>  	imply ENABLE_MRC_CACHE
> diff --git a/arch/x86/cpu/ivybridge/Kconfig b/arch/x86/cpu/ivybridge/Kconfig
> index be3ef5e5d8f8..704f145adf88 100644
> --- a/arch/x86/cpu/ivybridge/Kconfig
> +++ b/arch/x86/cpu/ivybridge/Kconfig
> @@ -8,6 +8,7 @@
>  config NORTHBRIDGE_INTEL_IVYBRIDGE
>  	bool
>  	select CACHE_MRC_BIN if HAVE_MRC
> +	select DM_EVENT
>  	imply HAVE_INTEL_ME
>  	imply ENABLE_MRC_CACHE
>  	imply AHCI_PCI
> diff --git a/arch/x86/cpu/quark/Kconfig b/arch/x86/cpu/quark/Kconfig
> index 61bb5792c868..0d4008a31f4c 100644
> --- a/arch/x86/cpu/quark/Kconfig
> +++ b/arch/x86/cpu/quark/Kconfig
> @@ -7,6 +7,7 @@ config INTEL_QUARK
>  	select HAVE_RMU
>  	select ARCH_EARLY_INIT_R
>  	select ARCH_MISC_INIT
> +	select DM_EVENT
>  	imply ENABLE_MRC_CACHE
>  	imply ETH_DESIGNWARE
>  	imply ICH_SPI
> diff --git a/board/google/Kconfig b/board/google/Kconfig
> index 0474b4e69384..a0f1a6097641 100644
> --- a/board/google/Kconfig
> +++ b/board/google/Kconfig
> @@ -18,6 +18,7 @@ choice
>  config TARGET_CHROMEBOOK_CORAL
>  	bool "Chromebook coral"
>  	select BIOSEMU
> +	select EVENT
>  	help
>  	  This is a range of Intel-based laptops released in 2018. They use an
>  	  Intel Apollo Lake SoC. The design supports WiFi, 4GB to 16GB of
> diff --git a/board/keymile/Kconfig b/board/keymile/Kconfig
> index e5d7c80a869d..bf899d005c46 100644
> --- a/board/keymile/Kconfig
> +++ b/board/keymile/Kconfig
> @@ -124,6 +124,7 @@ config SYS_IVM_EEPROM_PAGE_LEN
>  
>  config PG_WCOM_UBOOT_UPDATE_SUPPORTED
>  	bool "Enable U-boot Field Fail-Safe Update Functionality"
> +	select EVENT
>  	default n
>  	help
>  	  Indicates that field fail-safe u-boot update is supported.
> diff --git a/boot/Kconfig b/boot/Kconfig
> index 30bc182fcd5c..725afd36a3b9 100644
> --- a/boot/Kconfig
> +++ b/boot/Kconfig
> @@ -474,6 +474,7 @@ config BOOTMETH_VBE
>  	depends on FIT
>  	default y
>  	select BOOTMETH_GLOBAL
> +	select EVENT
>  	help
>  	  Enables support for VBE boot. This is a standard boot method which
>  	  supports selection of various firmware components, seleciton of an OS to
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index b2aefae9cb70..4fe2c75de256 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -2622,6 +2622,7 @@ config CMD_DIAG
>  
>  config CMD_EVENT
>  	bool "event - Show information about events"
> +	depends on EVENT
>  	default y if EVENT_DEBUG
>  	help
>  	  This enables the 'event' command which provides information about
> diff --git a/common/Kconfig b/common/Kconfig
> index 73e3fe36573d..6f5aa4e53aae 100644
> --- a/common/Kconfig
> +++ b/common/Kconfig
> @@ -604,24 +604,23 @@ config CYCLIC_MAX_CPU_TIME_US
>  endif # CYCLIC
>  
>  config EVENT
> -	bool "General-purpose event-handling mechanism"
> -	default y if SANDBOX
> +	bool
>  	help
> -	  This enables sending and processing of events, to allow interested
> -	  parties to be alerted when something happens. This is an attempt to
> -	  stem the flow of weak functions, hooks, functions in board_f.c
> -	  and board_r.c and the Kconfig options below.
> +	  This adds a framework for general purpose sending and processing of
> +	  events, to allow interested parties to be alerted when something
> +	  happens. This is an attempt to stem 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
> +	bool
>  	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
> +	  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
> diff --git a/drivers/core/Kconfig b/drivers/core/Kconfig
> index 8fde77c23ee0..6fc8854b574b 100644
> --- a/drivers/core/Kconfig
> +++ b/drivers/core/Kconfig
> @@ -109,13 +109,14 @@ config DM_DEVICE_REMOVE
>  	  causes USB host controllers to not be stopped when booting the OS.
>  
>  config DM_EVENT
> -	bool "Support events with driver model"
> -	depends on DM && EVENT
> -	default y if SANDBOX
> +	bool
> +	depends on DM
> +	select EVENT
>  	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.
> +	  register a 'spy' function that is called when the event occurs. Such
> +	  subsystems must select this option.
>  
>  config SPL_DM_DEVICE_REMOVE
>  	bool "Support device removal in SPL"
> diff --git a/drivers/cpu/Kconfig b/drivers/cpu/Kconfig
> index 21874335c873..3bf04105e5e9 100644
> --- a/drivers/cpu/Kconfig
> +++ b/drivers/cpu/Kconfig
> @@ -23,7 +23,6 @@ config CPU_RISCV
>  config CPU_MICROBLAZE
>  	bool "Enable Microblaze CPU driver"
>  	depends on CPU && MICROBLAZE
> -	select EVENT
>  	select DM_EVENT
>  	select XILINX_MICROBLAZE0_PVR
>  	help
> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> index b498c72206fd..d04203cddab4 100644
> --- a/lib/efi_loader/Kconfig
> +++ b/lib/efi_loader/Kconfig
> @@ -14,9 +14,8 @@ config EFI_LOADER
>  	depends on !EFI_APP
>  	default y if !ARM || SYS_CPU = armv7 || SYS_CPU = armv8
>  	select CHARSET
> -	select DM_EVENT
> -	select EVENT
> -	select EVENT_DYNAMIC
> +	select DM_EVENT if PARTITIONS
> +	select EVENT_DYNAMIC if EVENT

Since UEFI subsystem, in particular efi_disk, uses event_register() or
EVENT_DYNAMIC directly, adding 'if EVENT' looks strange.
Instead, I suggest that 'select EVENT' be added to CONFIG_EVENT_DYNAMIC
and then we can safely
        select EVENT_DYNAMIC if PARTITIONS
here.

-Takahiro Akashi


>  	select LIB_UUID
>  	imply PARTITION_UUIDS
>  	select REGEX
> -- 
> 2.25.1
> 

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/2] event: Correct dependencies on the EVENT framework
  2023-01-16  3:10   ` AKASHI Takahiro
@ 2023-01-16  3:15     ` Tom Rini
  2023-01-16  4:17       ` AKASHI Takahiro
  0 siblings, 1 reply; 20+ messages in thread
From: Tom Rini @ 2023-01-16  3:15 UTC (permalink / raw)
  To: AKASHI Takahiro, u-boot, Simon Glass, Oliver Graute,
	Francesco Dolcini

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

On Mon, Jan 16, 2023 at 12:10:50PM +0900, AKASHI Takahiro wrote:
> Hi Tom,
> 
> On Sat, Jan 14, 2023 at 03:49:36PM -0500, Tom Rini wrote:
> > The event framework is just that, a framework. Enabling it by itself
> > does nothing, so we shouldn't ask the user about it. Reword (and correct
> > typos) around this the option and help text. This also applies to
> > DM_EVENT, so reword as well.
> > 
> > With this, it's time to address the larger problems. When functionality
> > uses events, typically via EVENT_SPY, the appropriate framework then
> > must be select'd and NOT imply'd. As the functionality will cease to
> > work (and so, platforms will fail to boot) this is non-optional and
> > where select is appropriate. Audit the current users of EVENT_SPY to
> > have a more fine-grained approach to select'ing the framework where
> > used.
> > 
> > Cc: Simon Glass <sjg@chromium.org>
> > Reported-by: Oliver Graute <Oliver.Graute@kococonnector.com>
> > Reported-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> > Fixes: 7fe32b3442f0 ("event: Convert arch_cpu_init_dm() to use events")
> > Fixes: 42fdcebf859f ("event: Convert misc_init_f() to use events")
> > Signed-off-by: Tom Rini <trini@konsulko.com>
> > ---
> >  arch/Kconfig                     |  6 +++---
> >  arch/arm/Kconfig                 |  9 ++++-----
> >  arch/arm/mach-omap2/Kconfig      |  3 +++
> >  arch/mips/Kconfig                |  2 +-
> >  arch/powerpc/cpu/mpc85xx/Kconfig |  1 +
> >  arch/x86/Kconfig                 |  1 +
> >  arch/x86/cpu/baytrail/Kconfig    |  1 +
> >  arch/x86/cpu/broadwell/Kconfig   |  1 +
> >  arch/x86/cpu/ivybridge/Kconfig   |  1 +
> >  arch/x86/cpu/quark/Kconfig       |  1 +
> >  board/google/Kconfig             |  1 +
> >  board/keymile/Kconfig            |  1 +
> >  boot/Kconfig                     |  1 +
> >  cmd/Kconfig                      |  1 +
> >  common/Kconfig                   | 17 ++++++++---------
> >  drivers/core/Kconfig             |  9 +++++----
> >  drivers/cpu/Kconfig              |  1 -
> >  lib/efi_loader/Kconfig           |  5 ++---
> >  18 files changed, 36 insertions(+), 26 deletions(-)
> > 
> > diff --git a/arch/Kconfig b/arch/Kconfig
> > index 8fb87b7d857c..d30676ae817b 100644
> > --- a/arch/Kconfig
> > +++ b/arch/Kconfig
> > @@ -93,7 +93,7 @@ config NIOS2
> >  	bool "Nios II architecture"
> >  	select CPU
> >  	select DM
> > -	imply DM_EVENT
> > +	select DM_EVENT
> >  	select OF_CONTROL
> >  	select SUPPORT_OF_CONTROL
> >  	imply CMD_DM
> > @@ -111,9 +111,9 @@ config RISCV
> >  	select SUPPORT_OF_CONTROL
> >  	select OF_CONTROL
> >  	select DM
> > +	select DM_EVENT
> >  	imply SPL_SEPARATE_BSS if SPL
> >  	imply DM_SERIAL
> > -	imply DM_EVENT
> >  	imply DM_MMC
> >  	imply DM_SPI
> >  	imply DM_SPI_FLASH
> > @@ -136,6 +136,7 @@ config SANDBOX
> >  	select BZIP2
> >  	select CMD_POWEROFF
> >  	select DM
> > +	select DM_EVENT
> >  	select DM_FUZZING_ENGINE
> >  	select DM_GPIO
> >  	select DM_I2C
> > @@ -240,7 +241,6 @@ config X86
> >  	imply CMD_SF
> >  	imply CMD_SF_TEST
> >  	imply CMD_ZBOOT
> > -	imply DM_EVENT
> >  	imply DM_GPIO
> >  	imply DM_KEYBOARD
> >  	imply DM_MMC
> > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> > index bbf1d5227b3f..c9a44ebc2213 100644
> > --- a/arch/arm/Kconfig
> > +++ b/arch/arm/Kconfig
> > @@ -778,7 +778,6 @@ config ARCH_OMAP2PLUS
> >  	select SUPPORT_SPL
> >  	imply TI_SYSC if DM && OF_CONTROL
> >  	imply FIT
> > -	imply DM_EVENT
> >  	imply SPL_SEPARATE_BSS
> >  
> >  config ARCH_MESON
> > @@ -823,11 +822,11 @@ config ARCH_IMX8
> >  	select SYS_FSL_SEC_COMPAT_4
> >  	select SYS_FSL_SEC_LE
> >  	select DM
> > +	select DM_EVENT
> >  	select GPIO_EXTRA_HEADER
> >  	select MACH_IMX
> >  	select OF_CONTROL
> >  	select ENABLE_ARM_SOC_BOOT0_HOOK
> > -	imply DM_EVENT
> >  
> >  config ARCH_IMX8M
> >  	bool "NXP i.MX8M platform"
> > @@ -839,14 +838,15 @@ config ARCH_IMX8M
> >  	select SYS_FSL_SEC_LE
> >  	select SYS_I2C_MXC
> >  	select DM
> > +	select DM_EVENT if CLK
> >  	select SUPPORT_SPL
> >  	imply CMD_DM
> > -	imply DM_EVENT
> >  
> >  config ARCH_IMX8ULP
> >  	bool "NXP i.MX8ULP platform"
> >  	select ARM64
> >  	select DM
> > +	select DM_EVENT
> >  	select MACH_IMX
> >  	select OF_CONTROL
> >  	select SUPPORT_SPL
> > @@ -854,19 +854,18 @@ config ARCH_IMX8ULP
> >  	select MISC
> >  	select IMX_SENTINEL
> >  	imply CMD_DM
> > -	imply DM_EVENT
> >  
> >  config ARCH_IMX9
> >  	bool "NXP i.MX9 platform"
> >  	select ARM64
> >  	select DM
> > +	select DM_EVENT
> >  	select MACH_IMX
> >  	select SUPPORT_SPL
> >  	select GPIO_EXTRA_HEADER
> >  	select MISC
> >  	select IMX_SENTINEL
> >  	imply CMD_DM
> > -	imply DM_EVENT
> >  
> >  config ARCH_IMXRT
> >  	bool "NXP i.MXRT platform"
> > diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
> > index 1db71df27212..309b967b0dd5 100644
> > --- a/arch/arm/mach-omap2/Kconfig
> > +++ b/arch/arm/mach-omap2/Kconfig
> > @@ -31,6 +31,7 @@ config OMAP34XX
> >  
> >  config OMAP44XX
> >  	bool "OMAP44XX SoC"
> > +	select DM_EVENT
> >  	select SPL_USE_TINY_PRINTF
> >  	select SPL_SYS_NO_VECTOR_TABLE if SPL
> >  	imply NAND_OMAP_ELM
> > @@ -55,6 +56,7 @@ config OMAP54XX
> >  	bool "OMAP54XX SoC"
> >  	select ARM_CORTEX_A15_CVE_2017_5715
> >  	select ARM_ERRATA_798870
> > +	select DM_EVENT
> >  	select SYS_THUMB_BUILD
> >  	imply NAND_OMAP_ELM
> >  	imply NAND_OMAP_GPMC
> > @@ -111,6 +113,7 @@ config AM43XX
> >  config AM33XX
> >  	bool "AM33XX SoC"
> >  	select ARM_CORTEX_A8_CVE_2017_5715
> > +	select DM_EVENT
> >  	select SPECIFY_CONSOLE_INDEX
> >  	imply NAND_OMAP_ELM
> >  	imply NAND_OMAP_GPMC
> > diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
> > index 23142bd2700d..569f5f48bc6c 100644
> > --- a/arch/mips/Kconfig
> > +++ b/arch/mips/Kconfig
> > @@ -121,6 +121,7 @@ config MACH_PIC32
> >  	bool "Support Microchip PIC32"
> >  	select HAS_FIXED_TIMER_FREQUENCY
> >  	select DM
> > +	select DM_EVENT
> >  	select OF_CONTROL
> >  	imply CMD_DM
> >  
> > @@ -128,7 +129,6 @@ config TARGET_BOSTON
> >  	bool "Support Boston"
> >  	select HAS_FIXED_TIMER_FREQUENCY
> >  	select DM
> > -	imply DM_EVENT
> >  	select DM_SERIAL
> >  	select MIPS_CM
> >  	select SYS_CACHE_SHIFT_6
> > diff --git a/arch/powerpc/cpu/mpc85xx/Kconfig b/arch/powerpc/cpu/mpc85xx/Kconfig
> > index 1b180481a483..2c54a9e2120f 100644
> > --- a/arch/powerpc/cpu/mpc85xx/Kconfig
> > +++ b/arch/powerpc/cpu/mpc85xx/Kconfig
> > @@ -248,6 +248,7 @@ config TARGET_KMP204X
> >  config TARGET_KMCENT2
> >  	bool "Support kmcent2"
> >  	select VENDOR_KM
> > +	select EVENT
> >  	select FSL_CORENET
> >  	select SYS_DPAA_FMAN
> >  	select SYS_DPAA_PME
> > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> > index 93f1c77be3f3..07be5cd05ec0 100644
> > --- a/arch/x86/Kconfig
> > +++ b/arch/x86/Kconfig
> > @@ -395,6 +395,7 @@ config FSP_VERSION1
> >  
> >  config FSP_VERSION2
> >  	bool "FSP version 2.x"
> > +	select DM_EVENT
> >  	help
> >  	  This covers versions 2.0 and 2.1. See here for details:
> >  	  https://github.com/IntelFsp/fsp/wiki
> > diff --git a/arch/x86/cpu/baytrail/Kconfig b/arch/x86/cpu/baytrail/Kconfig
> > index d2c3473d6abf..a8efea8a3413 100644
> > --- a/arch/x86/cpu/baytrail/Kconfig
> > +++ b/arch/x86/cpu/baytrail/Kconfig
> > @@ -7,6 +7,7 @@ config INTEL_BAYTRAIL
> >  	select HAVE_FSP
> >  	select ARCH_MISC_INIT
> >  	select CPU_INTEL_TURBO_NOT_PACKAGE_SCOPED
> > +	select DM_EVENT
> >  	imply HAVE_INTEL_ME
> >  	imply ENABLE_MRC_CACHE
> >  	imply AHCI_PCI
> > diff --git a/arch/x86/cpu/broadwell/Kconfig b/arch/x86/cpu/broadwell/Kconfig
> > index 5b015c89d950..39deda364479 100644
> > --- a/arch/x86/cpu/broadwell/Kconfig
> > +++ b/arch/x86/cpu/broadwell/Kconfig
> > @@ -6,6 +6,7 @@
> >  config INTEL_BROADWELL
> >  	bool
> >  	select CACHE_MRC_BIN
> > +	select DM_EVENT
> >  	select ARCH_EARLY_INIT_R
> >  	imply HAVE_INTEL_ME
> >  	imply ENABLE_MRC_CACHE
> > diff --git a/arch/x86/cpu/ivybridge/Kconfig b/arch/x86/cpu/ivybridge/Kconfig
> > index be3ef5e5d8f8..704f145adf88 100644
> > --- a/arch/x86/cpu/ivybridge/Kconfig
> > +++ b/arch/x86/cpu/ivybridge/Kconfig
> > @@ -8,6 +8,7 @@
> >  config NORTHBRIDGE_INTEL_IVYBRIDGE
> >  	bool
> >  	select CACHE_MRC_BIN if HAVE_MRC
> > +	select DM_EVENT
> >  	imply HAVE_INTEL_ME
> >  	imply ENABLE_MRC_CACHE
> >  	imply AHCI_PCI
> > diff --git a/arch/x86/cpu/quark/Kconfig b/arch/x86/cpu/quark/Kconfig
> > index 61bb5792c868..0d4008a31f4c 100644
> > --- a/arch/x86/cpu/quark/Kconfig
> > +++ b/arch/x86/cpu/quark/Kconfig
> > @@ -7,6 +7,7 @@ config INTEL_QUARK
> >  	select HAVE_RMU
> >  	select ARCH_EARLY_INIT_R
> >  	select ARCH_MISC_INIT
> > +	select DM_EVENT
> >  	imply ENABLE_MRC_CACHE
> >  	imply ETH_DESIGNWARE
> >  	imply ICH_SPI
> > diff --git a/board/google/Kconfig b/board/google/Kconfig
> > index 0474b4e69384..a0f1a6097641 100644
> > --- a/board/google/Kconfig
> > +++ b/board/google/Kconfig
> > @@ -18,6 +18,7 @@ choice
> >  config TARGET_CHROMEBOOK_CORAL
> >  	bool "Chromebook coral"
> >  	select BIOSEMU
> > +	select EVENT
> >  	help
> >  	  This is a range of Intel-based laptops released in 2018. They use an
> >  	  Intel Apollo Lake SoC. The design supports WiFi, 4GB to 16GB of
> > diff --git a/board/keymile/Kconfig b/board/keymile/Kconfig
> > index e5d7c80a869d..bf899d005c46 100644
> > --- a/board/keymile/Kconfig
> > +++ b/board/keymile/Kconfig
> > @@ -124,6 +124,7 @@ config SYS_IVM_EEPROM_PAGE_LEN
> >  
> >  config PG_WCOM_UBOOT_UPDATE_SUPPORTED
> >  	bool "Enable U-boot Field Fail-Safe Update Functionality"
> > +	select EVENT
> >  	default n
> >  	help
> >  	  Indicates that field fail-safe u-boot update is supported.
> > diff --git a/boot/Kconfig b/boot/Kconfig
> > index 30bc182fcd5c..725afd36a3b9 100644
> > --- a/boot/Kconfig
> > +++ b/boot/Kconfig
> > @@ -474,6 +474,7 @@ config BOOTMETH_VBE
> >  	depends on FIT
> >  	default y
> >  	select BOOTMETH_GLOBAL
> > +	select EVENT
> >  	help
> >  	  Enables support for VBE boot. This is a standard boot method which
> >  	  supports selection of various firmware components, seleciton of an OS to
> > diff --git a/cmd/Kconfig b/cmd/Kconfig
> > index b2aefae9cb70..4fe2c75de256 100644
> > --- a/cmd/Kconfig
> > +++ b/cmd/Kconfig
> > @@ -2622,6 +2622,7 @@ config CMD_DIAG
> >  
> >  config CMD_EVENT
> >  	bool "event - Show information about events"
> > +	depends on EVENT
> >  	default y if EVENT_DEBUG
> >  	help
> >  	  This enables the 'event' command which provides information about
> > diff --git a/common/Kconfig b/common/Kconfig
> > index 73e3fe36573d..6f5aa4e53aae 100644
> > --- a/common/Kconfig
> > +++ b/common/Kconfig
> > @@ -604,24 +604,23 @@ config CYCLIC_MAX_CPU_TIME_US
> >  endif # CYCLIC
> >  
> >  config EVENT
> > -	bool "General-purpose event-handling mechanism"
> > -	default y if SANDBOX
> > +	bool
> >  	help
> > -	  This enables sending and processing of events, to allow interested
> > -	  parties to be alerted when something happens. This is an attempt to
> > -	  stem the flow of weak functions, hooks, functions in board_f.c
> > -	  and board_r.c and the Kconfig options below.
> > +	  This adds a framework for general purpose sending and processing of
> > +	  events, to allow interested parties to be alerted when something
> > +	  happens. This is an attempt to stem 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
> > +	bool
> >  	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
> > +	  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
> > diff --git a/drivers/core/Kconfig b/drivers/core/Kconfig
> > index 8fde77c23ee0..6fc8854b574b 100644
> > --- a/drivers/core/Kconfig
> > +++ b/drivers/core/Kconfig
> > @@ -109,13 +109,14 @@ config DM_DEVICE_REMOVE
> >  	  causes USB host controllers to not be stopped when booting the OS.
> >  
> >  config DM_EVENT
> > -	bool "Support events with driver model"
> > -	depends on DM && EVENT
> > -	default y if SANDBOX
> > +	bool
> > +	depends on DM
> > +	select EVENT
> >  	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.
> > +	  register a 'spy' function that is called when the event occurs. Such
> > +	  subsystems must select this option.
> >  
> >  config SPL_DM_DEVICE_REMOVE
> >  	bool "Support device removal in SPL"
> > diff --git a/drivers/cpu/Kconfig b/drivers/cpu/Kconfig
> > index 21874335c873..3bf04105e5e9 100644
> > --- a/drivers/cpu/Kconfig
> > +++ b/drivers/cpu/Kconfig
> > @@ -23,7 +23,6 @@ config CPU_RISCV
> >  config CPU_MICROBLAZE
> >  	bool "Enable Microblaze CPU driver"
> >  	depends on CPU && MICROBLAZE
> > -	select EVENT
> >  	select DM_EVENT
> >  	select XILINX_MICROBLAZE0_PVR
> >  	help
> > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> > index b498c72206fd..d04203cddab4 100644
> > --- a/lib/efi_loader/Kconfig
> > +++ b/lib/efi_loader/Kconfig
> > @@ -14,9 +14,8 @@ config EFI_LOADER
> >  	depends on !EFI_APP
> >  	default y if !ARM || SYS_CPU = armv7 || SYS_CPU = armv8
> >  	select CHARSET
> > -	select DM_EVENT
> > -	select EVENT
> > -	select EVENT_DYNAMIC
> > +	select DM_EVENT if PARTITIONS
> > +	select EVENT_DYNAMIC if EVENT
> 
> Since UEFI subsystem, in particular efi_disk, uses event_register() or
> EVENT_DYNAMIC directly, adding 'if EVENT' looks strange.
> Instead, I suggest that 'select EVENT' be added to CONFIG_EVENT_DYNAMIC
> and then we can safely
>         select EVENT_DYNAMIC if PARTITIONS
> here.

How about "if PARTITIONS" for both? Dynamic events can be for any of the
types, so the EFI_LOADER part should be select'ing DM_EVENT and
EVENT_DYNAMIC. And the dynamic event code is a subset of the regular
event code.

-- 
Tom

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/2] event: Correct dependencies on the EVENT framework
  2023-01-16  3:15     ` Tom Rini
@ 2023-01-16  4:17       ` AKASHI Takahiro
  2023-01-16  6:32         ` AKASHI Takahiro
  0 siblings, 1 reply; 20+ messages in thread
From: AKASHI Takahiro @ 2023-01-16  4:17 UTC (permalink / raw)
  To: Tom Rini; +Cc: u-boot, Simon Glass, Oliver Graute, Francesco Dolcini

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

On Sun, Jan 15, 2023 at 10:15:11PM -0500, Tom Rini wrote:
> On Mon, Jan 16, 2023 at 12:10:50PM +0900, AKASHI Takahiro wrote:
> > Hi Tom,
> > 
> > On Sat, Jan 14, 2023 at 03:49:36PM -0500, Tom Rini wrote:
> > > The event framework is just that, a framework. Enabling it by itself
> > > does nothing, so we shouldn't ask the user about it. Reword (and correct
> > > typos) around this the option and help text. This also applies to
> > > DM_EVENT, so reword as well.
> > > 
> > > With this, it's time to address the larger problems. When functionality
> > > uses events, typically via EVENT_SPY, the appropriate framework then
> > > must be select'd and NOT imply'd. As the functionality will cease to
> > > work (and so, platforms will fail to boot) this is non-optional and
> > > where select is appropriate. Audit the current users of EVENT_SPY to
> > > have a more fine-grained approach to select'ing the framework where
> > > used.
> > > 
> > > Cc: Simon Glass <sjg@chromium.org>
> > > Reported-by: Oliver Graute <Oliver.Graute@kococonnector.com>
> > > Reported-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> > > Fixes: 7fe32b3442f0 ("event: Convert arch_cpu_init_dm() to use events")
> > > Fixes: 42fdcebf859f ("event: Convert misc_init_f() to use events")
> > > Signed-off-by: Tom Rini <trini@konsulko.com>
> > > ---
> > >  arch/Kconfig                     |  6 +++---
> > >  arch/arm/Kconfig                 |  9 ++++-----
> > >  arch/arm/mach-omap2/Kconfig      |  3 +++
> > >  arch/mips/Kconfig                |  2 +-
> > >  arch/powerpc/cpu/mpc85xx/Kconfig |  1 +
> > >  arch/x86/Kconfig                 |  1 +
> > >  arch/x86/cpu/baytrail/Kconfig    |  1 +
> > >  arch/x86/cpu/broadwell/Kconfig   |  1 +
> > >  arch/x86/cpu/ivybridge/Kconfig   |  1 +
> > >  arch/x86/cpu/quark/Kconfig       |  1 +
> > >  board/google/Kconfig             |  1 +
> > >  board/keymile/Kconfig            |  1 +
> > >  boot/Kconfig                     |  1 +
> > >  cmd/Kconfig                      |  1 +
> > >  common/Kconfig                   | 17 ++++++++---------
> > >  drivers/core/Kconfig             |  9 +++++----
> > >  drivers/cpu/Kconfig              |  1 -
> > >  lib/efi_loader/Kconfig           |  5 ++---
> > >  18 files changed, 36 insertions(+), 26 deletions(-)
> > > 
> > > diff --git a/arch/Kconfig b/arch/Kconfig
> > > index 8fb87b7d857c..d30676ae817b 100644
> > > --- a/arch/Kconfig
> > > +++ b/arch/Kconfig
> > > @@ -93,7 +93,7 @@ config NIOS2
> > >  	bool "Nios II architecture"
> > >  	select CPU
> > >  	select DM
> > > -	imply DM_EVENT
> > > +	select DM_EVENT
> > >  	select OF_CONTROL
> > >  	select SUPPORT_OF_CONTROL
> > >  	imply CMD_DM
> > > @@ -111,9 +111,9 @@ config RISCV
> > >  	select SUPPORT_OF_CONTROL
> > >  	select OF_CONTROL
> > >  	select DM
> > > +	select DM_EVENT
> > >  	imply SPL_SEPARATE_BSS if SPL
> > >  	imply DM_SERIAL
> > > -	imply DM_EVENT
> > >  	imply DM_MMC
> > >  	imply DM_SPI
> > >  	imply DM_SPI_FLASH
> > > @@ -136,6 +136,7 @@ config SANDBOX
> > >  	select BZIP2
> > >  	select CMD_POWEROFF
> > >  	select DM
> > > +	select DM_EVENT
> > >  	select DM_FUZZING_ENGINE
> > >  	select DM_GPIO
> > >  	select DM_I2C
> > > @@ -240,7 +241,6 @@ config X86
> > >  	imply CMD_SF
> > >  	imply CMD_SF_TEST
> > >  	imply CMD_ZBOOT
> > > -	imply DM_EVENT
> > >  	imply DM_GPIO
> > >  	imply DM_KEYBOARD
> > >  	imply DM_MMC
> > > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> > > index bbf1d5227b3f..c9a44ebc2213 100644
> > > --- a/arch/arm/Kconfig
> > > +++ b/arch/arm/Kconfig
> > > @@ -778,7 +778,6 @@ config ARCH_OMAP2PLUS
> > >  	select SUPPORT_SPL
> > >  	imply TI_SYSC if DM && OF_CONTROL
> > >  	imply FIT
> > > -	imply DM_EVENT
> > >  	imply SPL_SEPARATE_BSS
> > >  
> > >  config ARCH_MESON
> > > @@ -823,11 +822,11 @@ config ARCH_IMX8
> > >  	select SYS_FSL_SEC_COMPAT_4
> > >  	select SYS_FSL_SEC_LE
> > >  	select DM
> > > +	select DM_EVENT
> > >  	select GPIO_EXTRA_HEADER
> > >  	select MACH_IMX
> > >  	select OF_CONTROL
> > >  	select ENABLE_ARM_SOC_BOOT0_HOOK
> > > -	imply DM_EVENT
> > >  
> > >  config ARCH_IMX8M
> > >  	bool "NXP i.MX8M platform"
> > > @@ -839,14 +838,15 @@ config ARCH_IMX8M
> > >  	select SYS_FSL_SEC_LE
> > >  	select SYS_I2C_MXC
> > >  	select DM
> > > +	select DM_EVENT if CLK
> > >  	select SUPPORT_SPL
> > >  	imply CMD_DM
> > > -	imply DM_EVENT
> > >  
> > >  config ARCH_IMX8ULP
> > >  	bool "NXP i.MX8ULP platform"
> > >  	select ARM64
> > >  	select DM
> > > +	select DM_EVENT
> > >  	select MACH_IMX
> > >  	select OF_CONTROL
> > >  	select SUPPORT_SPL
> > > @@ -854,19 +854,18 @@ config ARCH_IMX8ULP
> > >  	select MISC
> > >  	select IMX_SENTINEL
> > >  	imply CMD_DM
> > > -	imply DM_EVENT
> > >  
> > >  config ARCH_IMX9
> > >  	bool "NXP i.MX9 platform"
> > >  	select ARM64
> > >  	select DM
> > > +	select DM_EVENT
> > >  	select MACH_IMX
> > >  	select SUPPORT_SPL
> > >  	select GPIO_EXTRA_HEADER
> > >  	select MISC
> > >  	select IMX_SENTINEL
> > >  	imply CMD_DM
> > > -	imply DM_EVENT
> > >  
> > >  config ARCH_IMXRT
> > >  	bool "NXP i.MXRT platform"
> > > diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
> > > index 1db71df27212..309b967b0dd5 100644
> > > --- a/arch/arm/mach-omap2/Kconfig
> > > +++ b/arch/arm/mach-omap2/Kconfig
> > > @@ -31,6 +31,7 @@ config OMAP34XX
> > >  
> > >  config OMAP44XX
> > >  	bool "OMAP44XX SoC"
> > > +	select DM_EVENT
> > >  	select SPL_USE_TINY_PRINTF
> > >  	select SPL_SYS_NO_VECTOR_TABLE if SPL
> > >  	imply NAND_OMAP_ELM
> > > @@ -55,6 +56,7 @@ config OMAP54XX
> > >  	bool "OMAP54XX SoC"
> > >  	select ARM_CORTEX_A15_CVE_2017_5715
> > >  	select ARM_ERRATA_798870
> > > +	select DM_EVENT
> > >  	select SYS_THUMB_BUILD
> > >  	imply NAND_OMAP_ELM
> > >  	imply NAND_OMAP_GPMC
> > > @@ -111,6 +113,7 @@ config AM43XX
> > >  config AM33XX
> > >  	bool "AM33XX SoC"
> > >  	select ARM_CORTEX_A8_CVE_2017_5715
> > > +	select DM_EVENT
> > >  	select SPECIFY_CONSOLE_INDEX
> > >  	imply NAND_OMAP_ELM
> > >  	imply NAND_OMAP_GPMC
> > > diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
> > > index 23142bd2700d..569f5f48bc6c 100644
> > > --- a/arch/mips/Kconfig
> > > +++ b/arch/mips/Kconfig
> > > @@ -121,6 +121,7 @@ config MACH_PIC32
> > >  	bool "Support Microchip PIC32"
> > >  	select HAS_FIXED_TIMER_FREQUENCY
> > >  	select DM
> > > +	select DM_EVENT
> > >  	select OF_CONTROL
> > >  	imply CMD_DM
> > >  
> > > @@ -128,7 +129,6 @@ config TARGET_BOSTON
> > >  	bool "Support Boston"
> > >  	select HAS_FIXED_TIMER_FREQUENCY
> > >  	select DM
> > > -	imply DM_EVENT
> > >  	select DM_SERIAL
> > >  	select MIPS_CM
> > >  	select SYS_CACHE_SHIFT_6
> > > diff --git a/arch/powerpc/cpu/mpc85xx/Kconfig b/arch/powerpc/cpu/mpc85xx/Kconfig
> > > index 1b180481a483..2c54a9e2120f 100644
> > > --- a/arch/powerpc/cpu/mpc85xx/Kconfig
> > > +++ b/arch/powerpc/cpu/mpc85xx/Kconfig
> > > @@ -248,6 +248,7 @@ config TARGET_KMP204X
> > >  config TARGET_KMCENT2
> > >  	bool "Support kmcent2"
> > >  	select VENDOR_KM
> > > +	select EVENT
> > >  	select FSL_CORENET
> > >  	select SYS_DPAA_FMAN
> > >  	select SYS_DPAA_PME
> > > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> > > index 93f1c77be3f3..07be5cd05ec0 100644
> > > --- a/arch/x86/Kconfig
> > > +++ b/arch/x86/Kconfig
> > > @@ -395,6 +395,7 @@ config FSP_VERSION1
> > >  
> > >  config FSP_VERSION2
> > >  	bool "FSP version 2.x"
> > > +	select DM_EVENT
> > >  	help
> > >  	  This covers versions 2.0 and 2.1. See here for details:
> > >  	  https://github.com/IntelFsp/fsp/wiki
> > > diff --git a/arch/x86/cpu/baytrail/Kconfig b/arch/x86/cpu/baytrail/Kconfig
> > > index d2c3473d6abf..a8efea8a3413 100644
> > > --- a/arch/x86/cpu/baytrail/Kconfig
> > > +++ b/arch/x86/cpu/baytrail/Kconfig
> > > @@ -7,6 +7,7 @@ config INTEL_BAYTRAIL
> > >  	select HAVE_FSP
> > >  	select ARCH_MISC_INIT
> > >  	select CPU_INTEL_TURBO_NOT_PACKAGE_SCOPED
> > > +	select DM_EVENT
> > >  	imply HAVE_INTEL_ME
> > >  	imply ENABLE_MRC_CACHE
> > >  	imply AHCI_PCI
> > > diff --git a/arch/x86/cpu/broadwell/Kconfig b/arch/x86/cpu/broadwell/Kconfig
> > > index 5b015c89d950..39deda364479 100644
> > > --- a/arch/x86/cpu/broadwell/Kconfig
> > > +++ b/arch/x86/cpu/broadwell/Kconfig
> > > @@ -6,6 +6,7 @@
> > >  config INTEL_BROADWELL
> > >  	bool
> > >  	select CACHE_MRC_BIN
> > > +	select DM_EVENT
> > >  	select ARCH_EARLY_INIT_R
> > >  	imply HAVE_INTEL_ME
> > >  	imply ENABLE_MRC_CACHE
> > > diff --git a/arch/x86/cpu/ivybridge/Kconfig b/arch/x86/cpu/ivybridge/Kconfig
> > > index be3ef5e5d8f8..704f145adf88 100644
> > > --- a/arch/x86/cpu/ivybridge/Kconfig
> > > +++ b/arch/x86/cpu/ivybridge/Kconfig
> > > @@ -8,6 +8,7 @@
> > >  config NORTHBRIDGE_INTEL_IVYBRIDGE
> > >  	bool
> > >  	select CACHE_MRC_BIN if HAVE_MRC
> > > +	select DM_EVENT
> > >  	imply HAVE_INTEL_ME
> > >  	imply ENABLE_MRC_CACHE
> > >  	imply AHCI_PCI
> > > diff --git a/arch/x86/cpu/quark/Kconfig b/arch/x86/cpu/quark/Kconfig
> > > index 61bb5792c868..0d4008a31f4c 100644
> > > --- a/arch/x86/cpu/quark/Kconfig
> > > +++ b/arch/x86/cpu/quark/Kconfig
> > > @@ -7,6 +7,7 @@ config INTEL_QUARK
> > >  	select HAVE_RMU
> > >  	select ARCH_EARLY_INIT_R
> > >  	select ARCH_MISC_INIT
> > > +	select DM_EVENT
> > >  	imply ENABLE_MRC_CACHE
> > >  	imply ETH_DESIGNWARE
> > >  	imply ICH_SPI
> > > diff --git a/board/google/Kconfig b/board/google/Kconfig
> > > index 0474b4e69384..a0f1a6097641 100644
> > > --- a/board/google/Kconfig
> > > +++ b/board/google/Kconfig
> > > @@ -18,6 +18,7 @@ choice
> > >  config TARGET_CHROMEBOOK_CORAL
> > >  	bool "Chromebook coral"
> > >  	select BIOSEMU
> > > +	select EVENT
> > >  	help
> > >  	  This is a range of Intel-based laptops released in 2018. They use an
> > >  	  Intel Apollo Lake SoC. The design supports WiFi, 4GB to 16GB of
> > > diff --git a/board/keymile/Kconfig b/board/keymile/Kconfig
> > > index e5d7c80a869d..bf899d005c46 100644
> > > --- a/board/keymile/Kconfig
> > > +++ b/board/keymile/Kconfig
> > > @@ -124,6 +124,7 @@ config SYS_IVM_EEPROM_PAGE_LEN
> > >  
> > >  config PG_WCOM_UBOOT_UPDATE_SUPPORTED
> > >  	bool "Enable U-boot Field Fail-Safe Update Functionality"
> > > +	select EVENT
> > >  	default n
> > >  	help
> > >  	  Indicates that field fail-safe u-boot update is supported.
> > > diff --git a/boot/Kconfig b/boot/Kconfig
> > > index 30bc182fcd5c..725afd36a3b9 100644
> > > --- a/boot/Kconfig
> > > +++ b/boot/Kconfig
> > > @@ -474,6 +474,7 @@ config BOOTMETH_VBE
> > >  	depends on FIT
> > >  	default y
> > >  	select BOOTMETH_GLOBAL
> > > +	select EVENT
> > >  	help
> > >  	  Enables support for VBE boot. This is a standard boot method which
> > >  	  supports selection of various firmware components, seleciton of an OS to
> > > diff --git a/cmd/Kconfig b/cmd/Kconfig
> > > index b2aefae9cb70..4fe2c75de256 100644
> > > --- a/cmd/Kconfig
> > > +++ b/cmd/Kconfig
> > > @@ -2622,6 +2622,7 @@ config CMD_DIAG
> > >  
> > >  config CMD_EVENT
> > >  	bool "event - Show information about events"
> > > +	depends on EVENT
> > >  	default y if EVENT_DEBUG
> > >  	help
> > >  	  This enables the 'event' command which provides information about
> > > diff --git a/common/Kconfig b/common/Kconfig
> > > index 73e3fe36573d..6f5aa4e53aae 100644
> > > --- a/common/Kconfig
> > > +++ b/common/Kconfig
> > > @@ -604,24 +604,23 @@ config CYCLIC_MAX_CPU_TIME_US
> > >  endif # CYCLIC
> > >  
> > >  config EVENT
> > > -	bool "General-purpose event-handling mechanism"
> > > -	default y if SANDBOX
> > > +	bool
> > >  	help
> > > -	  This enables sending and processing of events, to allow interested
> > > -	  parties to be alerted when something happens. This is an attempt to
> > > -	  stem the flow of weak functions, hooks, functions in board_f.c
> > > -	  and board_r.c and the Kconfig options below.
> > > +	  This adds a framework for general purpose sending and processing of
> > > +	  events, to allow interested parties to be alerted when something
> > > +	  happens. This is an attempt to stem 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
> > > +	bool
> > >  	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
> > > +	  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
> > > diff --git a/drivers/core/Kconfig b/drivers/core/Kconfig
> > > index 8fde77c23ee0..6fc8854b574b 100644
> > > --- a/drivers/core/Kconfig
> > > +++ b/drivers/core/Kconfig
> > > @@ -109,13 +109,14 @@ config DM_DEVICE_REMOVE
> > >  	  causes USB host controllers to not be stopped when booting the OS.
> > >  
> > >  config DM_EVENT
> > > -	bool "Support events with driver model"
> > > -	depends on DM && EVENT
> > > -	default y if SANDBOX
> > > +	bool
> > > +	depends on DM
> > > +	select EVENT
> > >  	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.
> > > +	  register a 'spy' function that is called when the event occurs. Such
> > > +	  subsystems must select this option.
> > >  
> > >  config SPL_DM_DEVICE_REMOVE
> > >  	bool "Support device removal in SPL"
> > > diff --git a/drivers/cpu/Kconfig b/drivers/cpu/Kconfig
> > > index 21874335c873..3bf04105e5e9 100644
> > > --- a/drivers/cpu/Kconfig
> > > +++ b/drivers/cpu/Kconfig
> > > @@ -23,7 +23,6 @@ config CPU_RISCV
> > >  config CPU_MICROBLAZE
> > >  	bool "Enable Microblaze CPU driver"
> > >  	depends on CPU && MICROBLAZE
> > > -	select EVENT
> > >  	select DM_EVENT
> > >  	select XILINX_MICROBLAZE0_PVR
> > >  	help
> > > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> > > index b498c72206fd..d04203cddab4 100644
> > > --- a/lib/efi_loader/Kconfig
> > > +++ b/lib/efi_loader/Kconfig
> > > @@ -14,9 +14,8 @@ config EFI_LOADER
> > >  	depends on !EFI_APP
> > >  	default y if !ARM || SYS_CPU = armv7 || SYS_CPU = armv8
> > >  	select CHARSET
> > > -	select DM_EVENT
> > > -	select EVENT
> > > -	select EVENT_DYNAMIC
> > > +	select DM_EVENT if PARTITIONS
> > > +	select EVENT_DYNAMIC if EVENT
> > 
> > Since UEFI subsystem, in particular efi_disk, uses event_register() or
> > EVENT_DYNAMIC directly, adding 'if EVENT' looks strange.
> > Instead, I suggest that 'select EVENT' be added to CONFIG_EVENT_DYNAMIC
> > and then we can safely
> >         select EVENT_DYNAMIC if PARTITIONS
> > here.
> 
> How about "if PARTITIONS" for both? Dynamic events can be for any of the
> types, so the EFI_LOADER part should be select'ing DM_EVENT and
> EVENT_DYNAMIC. And the dynamic event code is a subset of the regular
> event code.

Okay, that is what I meant.

-Takahiro Akashi

> 
> -- 
> Tom



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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/2] event: Correct dependencies on the EVENT framework
  2023-01-16  4:17       ` AKASHI Takahiro
@ 2023-01-16  6:32         ` AKASHI Takahiro
  2023-01-16 14:19           ` Tom Rini
  0 siblings, 1 reply; 20+ messages in thread
From: AKASHI Takahiro @ 2023-01-16  6:32 UTC (permalink / raw)
  To: Tom Rini, u-boot, Simon Glass, Oliver Graute, Francesco Dolcini

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

On Mon, Jan 16, 2023 at 01:17:53PM +0900, AKASHI Takahiro wrote:
> On Sun, Jan 15, 2023 at 10:15:11PM -0500, Tom Rini wrote:
> > On Mon, Jan 16, 2023 at 12:10:50PM +0900, AKASHI Takahiro wrote:
> > > Hi Tom,
> > > 
> > > On Sat, Jan 14, 2023 at 03:49:36PM -0500, Tom Rini wrote:
> > > > The event framework is just that, a framework. Enabling it by itself
> > > > does nothing, so we shouldn't ask the user about it. Reword (and correct
> > > > typos) around this the option and help text. This also applies to
> > > > DM_EVENT, so reword as well.
> > > > 
> > > > With this, it's time to address the larger problems. When functionality
> > > > uses events, typically via EVENT_SPY, the appropriate framework then
> > > > must be select'd and NOT imply'd. As the functionality will cease to
> > > > work (and so, platforms will fail to boot) this is non-optional and
> > > > where select is appropriate. Audit the current users of EVENT_SPY to
> > > > have a more fine-grained approach to select'ing the framework where
> > > > used.
> > > > 
> > > > Cc: Simon Glass <sjg@chromium.org>
> > > > Reported-by: Oliver Graute <Oliver.Graute@kococonnector.com>
> > > > Reported-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> > > > Fixes: 7fe32b3442f0 ("event: Convert arch_cpu_init_dm() to use events")
> > > > Fixes: 42fdcebf859f ("event: Convert misc_init_f() to use events")
> > > > Signed-off-by: Tom Rini <trini@konsulko.com>
> > > > ---
> > > >  arch/Kconfig                     |  6 +++---
> > > >  arch/arm/Kconfig                 |  9 ++++-----
> > > >  arch/arm/mach-omap2/Kconfig      |  3 +++
> > > >  arch/mips/Kconfig                |  2 +-
> > > >  arch/powerpc/cpu/mpc85xx/Kconfig |  1 +
> > > >  arch/x86/Kconfig                 |  1 +
> > > >  arch/x86/cpu/baytrail/Kconfig    |  1 +
> > > >  arch/x86/cpu/broadwell/Kconfig   |  1 +
> > > >  arch/x86/cpu/ivybridge/Kconfig   |  1 +
> > > >  arch/x86/cpu/quark/Kconfig       |  1 +
> > > >  board/google/Kconfig             |  1 +
> > > >  board/keymile/Kconfig            |  1 +
> > > >  boot/Kconfig                     |  1 +
> > > >  cmd/Kconfig                      |  1 +
> > > >  common/Kconfig                   | 17 ++++++++---------
> > > >  drivers/core/Kconfig             |  9 +++++----
> > > >  drivers/cpu/Kconfig              |  1 -
> > > >  lib/efi_loader/Kconfig           |  5 ++---
> > > >  18 files changed, 36 insertions(+), 26 deletions(-)
> > > > 
> > > > diff --git a/arch/Kconfig b/arch/Kconfig
> > > > index 8fb87b7d857c..d30676ae817b 100644
> > > > --- a/arch/Kconfig
> > > > +++ b/arch/Kconfig
> > > > @@ -93,7 +93,7 @@ config NIOS2
> > > >  	bool "Nios II architecture"
> > > >  	select CPU
> > > >  	select DM
> > > > -	imply DM_EVENT
> > > > +	select DM_EVENT
> > > >  	select OF_CONTROL
> > > >  	select SUPPORT_OF_CONTROL
> > > >  	imply CMD_DM
> > > > @@ -111,9 +111,9 @@ config RISCV
> > > >  	select SUPPORT_OF_CONTROL
> > > >  	select OF_CONTROL
> > > >  	select DM
> > > > +	select DM_EVENT
> > > >  	imply SPL_SEPARATE_BSS if SPL
> > > >  	imply DM_SERIAL
> > > > -	imply DM_EVENT
> > > >  	imply DM_MMC
> > > >  	imply DM_SPI
> > > >  	imply DM_SPI_FLASH
> > > > @@ -136,6 +136,7 @@ config SANDBOX
> > > >  	select BZIP2
> > > >  	select CMD_POWEROFF
> > > >  	select DM
> > > > +	select DM_EVENT
> > > >  	select DM_FUZZING_ENGINE
> > > >  	select DM_GPIO
> > > >  	select DM_I2C
> > > > @@ -240,7 +241,6 @@ config X86
> > > >  	imply CMD_SF
> > > >  	imply CMD_SF_TEST
> > > >  	imply CMD_ZBOOT
> > > > -	imply DM_EVENT
> > > >  	imply DM_GPIO
> > > >  	imply DM_KEYBOARD
> > > >  	imply DM_MMC
> > > > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> > > > index bbf1d5227b3f..c9a44ebc2213 100644
> > > > --- a/arch/arm/Kconfig
> > > > +++ b/arch/arm/Kconfig
> > > > @@ -778,7 +778,6 @@ config ARCH_OMAP2PLUS
> > > >  	select SUPPORT_SPL
> > > >  	imply TI_SYSC if DM && OF_CONTROL
> > > >  	imply FIT
> > > > -	imply DM_EVENT
> > > >  	imply SPL_SEPARATE_BSS
> > > >  
> > > >  config ARCH_MESON
> > > > @@ -823,11 +822,11 @@ config ARCH_IMX8
> > > >  	select SYS_FSL_SEC_COMPAT_4
> > > >  	select SYS_FSL_SEC_LE
> > > >  	select DM
> > > > +	select DM_EVENT
> > > >  	select GPIO_EXTRA_HEADER
> > > >  	select MACH_IMX
> > > >  	select OF_CONTROL
> > > >  	select ENABLE_ARM_SOC_BOOT0_HOOK
> > > > -	imply DM_EVENT
> > > >  
> > > >  config ARCH_IMX8M
> > > >  	bool "NXP i.MX8M platform"
> > > > @@ -839,14 +838,15 @@ config ARCH_IMX8M
> > > >  	select SYS_FSL_SEC_LE
> > > >  	select SYS_I2C_MXC
> > > >  	select DM
> > > > +	select DM_EVENT if CLK
> > > >  	select SUPPORT_SPL
> > > >  	imply CMD_DM
> > > > -	imply DM_EVENT
> > > >  
> > > >  config ARCH_IMX8ULP
> > > >  	bool "NXP i.MX8ULP platform"
> > > >  	select ARM64
> > > >  	select DM
> > > > +	select DM_EVENT
> > > >  	select MACH_IMX
> > > >  	select OF_CONTROL
> > > >  	select SUPPORT_SPL
> > > > @@ -854,19 +854,18 @@ config ARCH_IMX8ULP
> > > >  	select MISC
> > > >  	select IMX_SENTINEL
> > > >  	imply CMD_DM
> > > > -	imply DM_EVENT
> > > >  
> > > >  config ARCH_IMX9
> > > >  	bool "NXP i.MX9 platform"
> > > >  	select ARM64
> > > >  	select DM
> > > > +	select DM_EVENT
> > > >  	select MACH_IMX
> > > >  	select SUPPORT_SPL
> > > >  	select GPIO_EXTRA_HEADER
> > > >  	select MISC
> > > >  	select IMX_SENTINEL
> > > >  	imply CMD_DM
> > > > -	imply DM_EVENT
> > > >  
> > > >  config ARCH_IMXRT
> > > >  	bool "NXP i.MXRT platform"
> > > > diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
> > > > index 1db71df27212..309b967b0dd5 100644
> > > > --- a/arch/arm/mach-omap2/Kconfig
> > > > +++ b/arch/arm/mach-omap2/Kconfig
> > > > @@ -31,6 +31,7 @@ config OMAP34XX
> > > >  
> > > >  config OMAP44XX
> > > >  	bool "OMAP44XX SoC"
> > > > +	select DM_EVENT
> > > >  	select SPL_USE_TINY_PRINTF
> > > >  	select SPL_SYS_NO_VECTOR_TABLE if SPL
> > > >  	imply NAND_OMAP_ELM
> > > > @@ -55,6 +56,7 @@ config OMAP54XX
> > > >  	bool "OMAP54XX SoC"
> > > >  	select ARM_CORTEX_A15_CVE_2017_5715
> > > >  	select ARM_ERRATA_798870
> > > > +	select DM_EVENT
> > > >  	select SYS_THUMB_BUILD
> > > >  	imply NAND_OMAP_ELM
> > > >  	imply NAND_OMAP_GPMC
> > > > @@ -111,6 +113,7 @@ config AM43XX
> > > >  config AM33XX
> > > >  	bool "AM33XX SoC"
> > > >  	select ARM_CORTEX_A8_CVE_2017_5715
> > > > +	select DM_EVENT
> > > >  	select SPECIFY_CONSOLE_INDEX
> > > >  	imply NAND_OMAP_ELM
> > > >  	imply NAND_OMAP_GPMC
> > > > diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
> > > > index 23142bd2700d..569f5f48bc6c 100644
> > > > --- a/arch/mips/Kconfig
> > > > +++ b/arch/mips/Kconfig
> > > > @@ -121,6 +121,7 @@ config MACH_PIC32
> > > >  	bool "Support Microchip PIC32"
> > > >  	select HAS_FIXED_TIMER_FREQUENCY
> > > >  	select DM
> > > > +	select DM_EVENT
> > > >  	select OF_CONTROL
> > > >  	imply CMD_DM
> > > >  
> > > > @@ -128,7 +129,6 @@ config TARGET_BOSTON
> > > >  	bool "Support Boston"
> > > >  	select HAS_FIXED_TIMER_FREQUENCY
> > > >  	select DM
> > > > -	imply DM_EVENT
> > > >  	select DM_SERIAL
> > > >  	select MIPS_CM
> > > >  	select SYS_CACHE_SHIFT_6
> > > > diff --git a/arch/powerpc/cpu/mpc85xx/Kconfig b/arch/powerpc/cpu/mpc85xx/Kconfig
> > > > index 1b180481a483..2c54a9e2120f 100644
> > > > --- a/arch/powerpc/cpu/mpc85xx/Kconfig
> > > > +++ b/arch/powerpc/cpu/mpc85xx/Kconfig
> > > > @@ -248,6 +248,7 @@ config TARGET_KMP204X
> > > >  config TARGET_KMCENT2
> > > >  	bool "Support kmcent2"
> > > >  	select VENDOR_KM
> > > > +	select EVENT
> > > >  	select FSL_CORENET
> > > >  	select SYS_DPAA_FMAN
> > > >  	select SYS_DPAA_PME
> > > > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> > > > index 93f1c77be3f3..07be5cd05ec0 100644
> > > > --- a/arch/x86/Kconfig
> > > > +++ b/arch/x86/Kconfig
> > > > @@ -395,6 +395,7 @@ config FSP_VERSION1
> > > >  
> > > >  config FSP_VERSION2
> > > >  	bool "FSP version 2.x"
> > > > +	select DM_EVENT
> > > >  	help
> > > >  	  This covers versions 2.0 and 2.1. See here for details:
> > > >  	  https://github.com/IntelFsp/fsp/wiki
> > > > diff --git a/arch/x86/cpu/baytrail/Kconfig b/arch/x86/cpu/baytrail/Kconfig
> > > > index d2c3473d6abf..a8efea8a3413 100644
> > > > --- a/arch/x86/cpu/baytrail/Kconfig
> > > > +++ b/arch/x86/cpu/baytrail/Kconfig
> > > > @@ -7,6 +7,7 @@ config INTEL_BAYTRAIL
> > > >  	select HAVE_FSP
> > > >  	select ARCH_MISC_INIT
> > > >  	select CPU_INTEL_TURBO_NOT_PACKAGE_SCOPED
> > > > +	select DM_EVENT
> > > >  	imply HAVE_INTEL_ME
> > > >  	imply ENABLE_MRC_CACHE
> > > >  	imply AHCI_PCI
> > > > diff --git a/arch/x86/cpu/broadwell/Kconfig b/arch/x86/cpu/broadwell/Kconfig
> > > > index 5b015c89d950..39deda364479 100644
> > > > --- a/arch/x86/cpu/broadwell/Kconfig
> > > > +++ b/arch/x86/cpu/broadwell/Kconfig
> > > > @@ -6,6 +6,7 @@
> > > >  config INTEL_BROADWELL
> > > >  	bool
> > > >  	select CACHE_MRC_BIN
> > > > +	select DM_EVENT
> > > >  	select ARCH_EARLY_INIT_R
> > > >  	imply HAVE_INTEL_ME
> > > >  	imply ENABLE_MRC_CACHE
> > > > diff --git a/arch/x86/cpu/ivybridge/Kconfig b/arch/x86/cpu/ivybridge/Kconfig
> > > > index be3ef5e5d8f8..704f145adf88 100644
> > > > --- a/arch/x86/cpu/ivybridge/Kconfig
> > > > +++ b/arch/x86/cpu/ivybridge/Kconfig
> > > > @@ -8,6 +8,7 @@
> > > >  config NORTHBRIDGE_INTEL_IVYBRIDGE
> > > >  	bool
> > > >  	select CACHE_MRC_BIN if HAVE_MRC
> > > > +	select DM_EVENT
> > > >  	imply HAVE_INTEL_ME
> > > >  	imply ENABLE_MRC_CACHE
> > > >  	imply AHCI_PCI
> > > > diff --git a/arch/x86/cpu/quark/Kconfig b/arch/x86/cpu/quark/Kconfig
> > > > index 61bb5792c868..0d4008a31f4c 100644
> > > > --- a/arch/x86/cpu/quark/Kconfig
> > > > +++ b/arch/x86/cpu/quark/Kconfig
> > > > @@ -7,6 +7,7 @@ config INTEL_QUARK
> > > >  	select HAVE_RMU
> > > >  	select ARCH_EARLY_INIT_R
> > > >  	select ARCH_MISC_INIT
> > > > +	select DM_EVENT
> > > >  	imply ENABLE_MRC_CACHE
> > > >  	imply ETH_DESIGNWARE
> > > >  	imply ICH_SPI
> > > > diff --git a/board/google/Kconfig b/board/google/Kconfig
> > > > index 0474b4e69384..a0f1a6097641 100644
> > > > --- a/board/google/Kconfig
> > > > +++ b/board/google/Kconfig
> > > > @@ -18,6 +18,7 @@ choice
> > > >  config TARGET_CHROMEBOOK_CORAL
> > > >  	bool "Chromebook coral"
> > > >  	select BIOSEMU
> > > > +	select EVENT
> > > >  	help
> > > >  	  This is a range of Intel-based laptops released in 2018. They use an
> > > >  	  Intel Apollo Lake SoC. The design supports WiFi, 4GB to 16GB of
> > > > diff --git a/board/keymile/Kconfig b/board/keymile/Kconfig
> > > > index e5d7c80a869d..bf899d005c46 100644
> > > > --- a/board/keymile/Kconfig
> > > > +++ b/board/keymile/Kconfig
> > > > @@ -124,6 +124,7 @@ config SYS_IVM_EEPROM_PAGE_LEN
> > > >  
> > > >  config PG_WCOM_UBOOT_UPDATE_SUPPORTED
> > > >  	bool "Enable U-boot Field Fail-Safe Update Functionality"
> > > > +	select EVENT
> > > >  	default n
> > > >  	help
> > > >  	  Indicates that field fail-safe u-boot update is supported.
> > > > diff --git a/boot/Kconfig b/boot/Kconfig
> > > > index 30bc182fcd5c..725afd36a3b9 100644
> > > > --- a/boot/Kconfig
> > > > +++ b/boot/Kconfig
> > > > @@ -474,6 +474,7 @@ config BOOTMETH_VBE
> > > >  	depends on FIT
> > > >  	default y
> > > >  	select BOOTMETH_GLOBAL
> > > > +	select EVENT
> > > >  	help
> > > >  	  Enables support for VBE boot. This is a standard boot method which
> > > >  	  supports selection of various firmware components, seleciton of an OS to
> > > > diff --git a/cmd/Kconfig b/cmd/Kconfig
> > > > index b2aefae9cb70..4fe2c75de256 100644
> > > > --- a/cmd/Kconfig
> > > > +++ b/cmd/Kconfig
> > > > @@ -2622,6 +2622,7 @@ config CMD_DIAG
> > > >  
> > > >  config CMD_EVENT
> > > >  	bool "event - Show information about events"
> > > > +	depends on EVENT
> > > >  	default y if EVENT_DEBUG
> > > >  	help
> > > >  	  This enables the 'event' command which provides information about
> > > > diff --git a/common/Kconfig b/common/Kconfig
> > > > index 73e3fe36573d..6f5aa4e53aae 100644
> > > > --- a/common/Kconfig
> > > > +++ b/common/Kconfig
> > > > @@ -604,24 +604,23 @@ config CYCLIC_MAX_CPU_TIME_US
> > > >  endif # CYCLIC
> > > >  
> > > >  config EVENT
> > > > -	bool "General-purpose event-handling mechanism"
> > > > -	default y if SANDBOX
> > > > +	bool
> > > >  	help
> > > > -	  This enables sending and processing of events, to allow interested
> > > > -	  parties to be alerted when something happens. This is an attempt to
> > > > -	  stem the flow of weak functions, hooks, functions in board_f.c
> > > > -	  and board_r.c and the Kconfig options below.
> > > > +	  This adds a framework for general purpose sending and processing of
> > > > +	  events, to allow interested parties to be alerted when something
> > > > +	  happens. This is an attempt to stem 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
> > > > +	bool
> > > >  	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
> > > > +	  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
> > > > diff --git a/drivers/core/Kconfig b/drivers/core/Kconfig
> > > > index 8fde77c23ee0..6fc8854b574b 100644
> > > > --- a/drivers/core/Kconfig
> > > > +++ b/drivers/core/Kconfig
> > > > @@ -109,13 +109,14 @@ config DM_DEVICE_REMOVE
> > > >  	  causes USB host controllers to not be stopped when booting the OS.
> > > >  
> > > >  config DM_EVENT
> > > > -	bool "Support events with driver model"
> > > > -	depends on DM && EVENT
> > > > -	default y if SANDBOX
> > > > +	bool
> > > > +	depends on DM
> > > > +	select EVENT
> > > >  	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.
> > > > +	  register a 'spy' function that is called when the event occurs. Such
> > > > +	  subsystems must select this option.
> > > >  
> > > >  config SPL_DM_DEVICE_REMOVE
> > > >  	bool "Support device removal in SPL"
> > > > diff --git a/drivers/cpu/Kconfig b/drivers/cpu/Kconfig
> > > > index 21874335c873..3bf04105e5e9 100644
> > > > --- a/drivers/cpu/Kconfig
> > > > +++ b/drivers/cpu/Kconfig
> > > > @@ -23,7 +23,6 @@ config CPU_RISCV
> > > >  config CPU_MICROBLAZE
> > > >  	bool "Enable Microblaze CPU driver"
> > > >  	depends on CPU && MICROBLAZE
> > > > -	select EVENT
> > > >  	select DM_EVENT
> > > >  	select XILINX_MICROBLAZE0_PVR
> > > >  	help
> > > > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> > > > index b498c72206fd..d04203cddab4 100644
> > > > --- a/lib/efi_loader/Kconfig
> > > > +++ b/lib/efi_loader/Kconfig
> > > > @@ -14,9 +14,8 @@ config EFI_LOADER
> > > >  	depends on !EFI_APP
> > > >  	default y if !ARM || SYS_CPU = armv7 || SYS_CPU = armv8
> > > >  	select CHARSET
> > > > -	select DM_EVENT
> > > > -	select EVENT
> > > > -	select EVENT_DYNAMIC
> > > > +	select DM_EVENT if PARTITIONS
> > > > +	select EVENT_DYNAMIC if EVENT
> > > 
> > > Since UEFI subsystem, in particular efi_disk, uses event_register() or
> > > EVENT_DYNAMIC directly, adding 'if EVENT' looks strange.
> > > Instead, I suggest that 'select EVENT' be added to CONFIG_EVENT_DYNAMIC
> > > and then we can safely
> > >         select EVENT_DYNAMIC if PARTITIONS
> > > here.
> > 
> > How about "if PARTITIONS" for both? Dynamic events can be for any of the
> > types, so the EFI_LOADER part should be select'ing DM_EVENT and
> > EVENT_DYNAMIC. And the dynamic event code is a subset of the regular
> > event code.
> 
> Okay, that is what I meant.

I have to correct myself.
The fact is that event mechanism is used to detect not only partitions but also
raw block devices, then DM_EVENT & EVENT_DYNAMIC must always be selected even if
CONFIG_PARTITIONS is disabled (it's unlikely, though).
Please note that EFI_LOADER itself has a dependency on BLK.

So the correct fix would be:
1. remove 'select EVENT' from EFI_LOADER
2. add 'select EVENT' to EVENT_DYNAMIC

-Takahiro Akashi

> -Takahiro Akashi
> 
> > 
> > -- 
> > Tom
> 
> 



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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/2] event: Correct dependencies on the EVENT framework
  2023-01-16  6:32         ` AKASHI Takahiro
@ 2023-01-16 14:19           ` Tom Rini
  0 siblings, 0 replies; 20+ messages in thread
From: Tom Rini @ 2023-01-16 14:19 UTC (permalink / raw)
  To: AKASHI Takahiro, u-boot, Simon Glass, Oliver Graute,
	Francesco Dolcini

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

On Mon, Jan 16, 2023 at 03:32:06PM +0900, AKASHI Takahiro wrote:
> On Mon, Jan 16, 2023 at 01:17:53PM +0900, AKASHI Takahiro wrote:
> > On Sun, Jan 15, 2023 at 10:15:11PM -0500, Tom Rini wrote:
> > > On Mon, Jan 16, 2023 at 12:10:50PM +0900, AKASHI Takahiro wrote:
> > > > Hi Tom,
> > > > 
> > > > On Sat, Jan 14, 2023 at 03:49:36PM -0500, Tom Rini wrote:
> > > > > The event framework is just that, a framework. Enabling it by itself
> > > > > does nothing, so we shouldn't ask the user about it. Reword (and correct
> > > > > typos) around this the option and help text. This also applies to
> > > > > DM_EVENT, so reword as well.
> > > > > 
> > > > > With this, it's time to address the larger problems. When functionality
> > > > > uses events, typically via EVENT_SPY, the appropriate framework then
> > > > > must be select'd and NOT imply'd. As the functionality will cease to
> > > > > work (and so, platforms will fail to boot) this is non-optional and
> > > > > where select is appropriate. Audit the current users of EVENT_SPY to
> > > > > have a more fine-grained approach to select'ing the framework where
> > > > > used.
> > > > > 
> > > > > Cc: Simon Glass <sjg@chromium.org>
> > > > > Reported-by: Oliver Graute <Oliver.Graute@kococonnector.com>
> > > > > Reported-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> > > > > Fixes: 7fe32b3442f0 ("event: Convert arch_cpu_init_dm() to use events")
> > > > > Fixes: 42fdcebf859f ("event: Convert misc_init_f() to use events")
> > > > > Signed-off-by: Tom Rini <trini@konsulko.com>
> > > > > ---
> > > > >  arch/Kconfig                     |  6 +++---
> > > > >  arch/arm/Kconfig                 |  9 ++++-----
> > > > >  arch/arm/mach-omap2/Kconfig      |  3 +++
> > > > >  arch/mips/Kconfig                |  2 +-
> > > > >  arch/powerpc/cpu/mpc85xx/Kconfig |  1 +
> > > > >  arch/x86/Kconfig                 |  1 +
> > > > >  arch/x86/cpu/baytrail/Kconfig    |  1 +
> > > > >  arch/x86/cpu/broadwell/Kconfig   |  1 +
> > > > >  arch/x86/cpu/ivybridge/Kconfig   |  1 +
> > > > >  arch/x86/cpu/quark/Kconfig       |  1 +
> > > > >  board/google/Kconfig             |  1 +
> > > > >  board/keymile/Kconfig            |  1 +
> > > > >  boot/Kconfig                     |  1 +
> > > > >  cmd/Kconfig                      |  1 +
> > > > >  common/Kconfig                   | 17 ++++++++---------
> > > > >  drivers/core/Kconfig             |  9 +++++----
> > > > >  drivers/cpu/Kconfig              |  1 -
> > > > >  lib/efi_loader/Kconfig           |  5 ++---
> > > > >  18 files changed, 36 insertions(+), 26 deletions(-)
> > > > > 
> > > > > diff --git a/arch/Kconfig b/arch/Kconfig
> > > > > index 8fb87b7d857c..d30676ae817b 100644
> > > > > --- a/arch/Kconfig
> > > > > +++ b/arch/Kconfig
> > > > > @@ -93,7 +93,7 @@ config NIOS2
> > > > >  	bool "Nios II architecture"
> > > > >  	select CPU
> > > > >  	select DM
> > > > > -	imply DM_EVENT
> > > > > +	select DM_EVENT
> > > > >  	select OF_CONTROL
> > > > >  	select SUPPORT_OF_CONTROL
> > > > >  	imply CMD_DM
> > > > > @@ -111,9 +111,9 @@ config RISCV
> > > > >  	select SUPPORT_OF_CONTROL
> > > > >  	select OF_CONTROL
> > > > >  	select DM
> > > > > +	select DM_EVENT
> > > > >  	imply SPL_SEPARATE_BSS if SPL
> > > > >  	imply DM_SERIAL
> > > > > -	imply DM_EVENT
> > > > >  	imply DM_MMC
> > > > >  	imply DM_SPI
> > > > >  	imply DM_SPI_FLASH
> > > > > @@ -136,6 +136,7 @@ config SANDBOX
> > > > >  	select BZIP2
> > > > >  	select CMD_POWEROFF
> > > > >  	select DM
> > > > > +	select DM_EVENT
> > > > >  	select DM_FUZZING_ENGINE
> > > > >  	select DM_GPIO
> > > > >  	select DM_I2C
> > > > > @@ -240,7 +241,6 @@ config X86
> > > > >  	imply CMD_SF
> > > > >  	imply CMD_SF_TEST
> > > > >  	imply CMD_ZBOOT
> > > > > -	imply DM_EVENT
> > > > >  	imply DM_GPIO
> > > > >  	imply DM_KEYBOARD
> > > > >  	imply DM_MMC
> > > > > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> > > > > index bbf1d5227b3f..c9a44ebc2213 100644
> > > > > --- a/arch/arm/Kconfig
> > > > > +++ b/arch/arm/Kconfig
> > > > > @@ -778,7 +778,6 @@ config ARCH_OMAP2PLUS
> > > > >  	select SUPPORT_SPL
> > > > >  	imply TI_SYSC if DM && OF_CONTROL
> > > > >  	imply FIT
> > > > > -	imply DM_EVENT
> > > > >  	imply SPL_SEPARATE_BSS
> > > > >  
> > > > >  config ARCH_MESON
> > > > > @@ -823,11 +822,11 @@ config ARCH_IMX8
> > > > >  	select SYS_FSL_SEC_COMPAT_4
> > > > >  	select SYS_FSL_SEC_LE
> > > > >  	select DM
> > > > > +	select DM_EVENT
> > > > >  	select GPIO_EXTRA_HEADER
> > > > >  	select MACH_IMX
> > > > >  	select OF_CONTROL
> > > > >  	select ENABLE_ARM_SOC_BOOT0_HOOK
> > > > > -	imply DM_EVENT
> > > > >  
> > > > >  config ARCH_IMX8M
> > > > >  	bool "NXP i.MX8M platform"
> > > > > @@ -839,14 +838,15 @@ config ARCH_IMX8M
> > > > >  	select SYS_FSL_SEC_LE
> > > > >  	select SYS_I2C_MXC
> > > > >  	select DM
> > > > > +	select DM_EVENT if CLK
> > > > >  	select SUPPORT_SPL
> > > > >  	imply CMD_DM
> > > > > -	imply DM_EVENT
> > > > >  
> > > > >  config ARCH_IMX8ULP
> > > > >  	bool "NXP i.MX8ULP platform"
> > > > >  	select ARM64
> > > > >  	select DM
> > > > > +	select DM_EVENT
> > > > >  	select MACH_IMX
> > > > >  	select OF_CONTROL
> > > > >  	select SUPPORT_SPL
> > > > > @@ -854,19 +854,18 @@ config ARCH_IMX8ULP
> > > > >  	select MISC
> > > > >  	select IMX_SENTINEL
> > > > >  	imply CMD_DM
> > > > > -	imply DM_EVENT
> > > > >  
> > > > >  config ARCH_IMX9
> > > > >  	bool "NXP i.MX9 platform"
> > > > >  	select ARM64
> > > > >  	select DM
> > > > > +	select DM_EVENT
> > > > >  	select MACH_IMX
> > > > >  	select SUPPORT_SPL
> > > > >  	select GPIO_EXTRA_HEADER
> > > > >  	select MISC
> > > > >  	select IMX_SENTINEL
> > > > >  	imply CMD_DM
> > > > > -	imply DM_EVENT
> > > > >  
> > > > >  config ARCH_IMXRT
> > > > >  	bool "NXP i.MXRT platform"
> > > > > diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
> > > > > index 1db71df27212..309b967b0dd5 100644
> > > > > --- a/arch/arm/mach-omap2/Kconfig
> > > > > +++ b/arch/arm/mach-omap2/Kconfig
> > > > > @@ -31,6 +31,7 @@ config OMAP34XX
> > > > >  
> > > > >  config OMAP44XX
> > > > >  	bool "OMAP44XX SoC"
> > > > > +	select DM_EVENT
> > > > >  	select SPL_USE_TINY_PRINTF
> > > > >  	select SPL_SYS_NO_VECTOR_TABLE if SPL
> > > > >  	imply NAND_OMAP_ELM
> > > > > @@ -55,6 +56,7 @@ config OMAP54XX
> > > > >  	bool "OMAP54XX SoC"
> > > > >  	select ARM_CORTEX_A15_CVE_2017_5715
> > > > >  	select ARM_ERRATA_798870
> > > > > +	select DM_EVENT
> > > > >  	select SYS_THUMB_BUILD
> > > > >  	imply NAND_OMAP_ELM
> > > > >  	imply NAND_OMAP_GPMC
> > > > > @@ -111,6 +113,7 @@ config AM43XX
> > > > >  config AM33XX
> > > > >  	bool "AM33XX SoC"
> > > > >  	select ARM_CORTEX_A8_CVE_2017_5715
> > > > > +	select DM_EVENT
> > > > >  	select SPECIFY_CONSOLE_INDEX
> > > > >  	imply NAND_OMAP_ELM
> > > > >  	imply NAND_OMAP_GPMC
> > > > > diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
> > > > > index 23142bd2700d..569f5f48bc6c 100644
> > > > > --- a/arch/mips/Kconfig
> > > > > +++ b/arch/mips/Kconfig
> > > > > @@ -121,6 +121,7 @@ config MACH_PIC32
> > > > >  	bool "Support Microchip PIC32"
> > > > >  	select HAS_FIXED_TIMER_FREQUENCY
> > > > >  	select DM
> > > > > +	select DM_EVENT
> > > > >  	select OF_CONTROL
> > > > >  	imply CMD_DM
> > > > >  
> > > > > @@ -128,7 +129,6 @@ config TARGET_BOSTON
> > > > >  	bool "Support Boston"
> > > > >  	select HAS_FIXED_TIMER_FREQUENCY
> > > > >  	select DM
> > > > > -	imply DM_EVENT
> > > > >  	select DM_SERIAL
> > > > >  	select MIPS_CM
> > > > >  	select SYS_CACHE_SHIFT_6
> > > > > diff --git a/arch/powerpc/cpu/mpc85xx/Kconfig b/arch/powerpc/cpu/mpc85xx/Kconfig
> > > > > index 1b180481a483..2c54a9e2120f 100644
> > > > > --- a/arch/powerpc/cpu/mpc85xx/Kconfig
> > > > > +++ b/arch/powerpc/cpu/mpc85xx/Kconfig
> > > > > @@ -248,6 +248,7 @@ config TARGET_KMP204X
> > > > >  config TARGET_KMCENT2
> > > > >  	bool "Support kmcent2"
> > > > >  	select VENDOR_KM
> > > > > +	select EVENT
> > > > >  	select FSL_CORENET
> > > > >  	select SYS_DPAA_FMAN
> > > > >  	select SYS_DPAA_PME
> > > > > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> > > > > index 93f1c77be3f3..07be5cd05ec0 100644
> > > > > --- a/arch/x86/Kconfig
> > > > > +++ b/arch/x86/Kconfig
> > > > > @@ -395,6 +395,7 @@ config FSP_VERSION1
> > > > >  
> > > > >  config FSP_VERSION2
> > > > >  	bool "FSP version 2.x"
> > > > > +	select DM_EVENT
> > > > >  	help
> > > > >  	  This covers versions 2.0 and 2.1. See here for details:
> > > > >  	  https://github.com/IntelFsp/fsp/wiki
> > > > > diff --git a/arch/x86/cpu/baytrail/Kconfig b/arch/x86/cpu/baytrail/Kconfig
> > > > > index d2c3473d6abf..a8efea8a3413 100644
> > > > > --- a/arch/x86/cpu/baytrail/Kconfig
> > > > > +++ b/arch/x86/cpu/baytrail/Kconfig
> > > > > @@ -7,6 +7,7 @@ config INTEL_BAYTRAIL
> > > > >  	select HAVE_FSP
> > > > >  	select ARCH_MISC_INIT
> > > > >  	select CPU_INTEL_TURBO_NOT_PACKAGE_SCOPED
> > > > > +	select DM_EVENT
> > > > >  	imply HAVE_INTEL_ME
> > > > >  	imply ENABLE_MRC_CACHE
> > > > >  	imply AHCI_PCI
> > > > > diff --git a/arch/x86/cpu/broadwell/Kconfig b/arch/x86/cpu/broadwell/Kconfig
> > > > > index 5b015c89d950..39deda364479 100644
> > > > > --- a/arch/x86/cpu/broadwell/Kconfig
> > > > > +++ b/arch/x86/cpu/broadwell/Kconfig
> > > > > @@ -6,6 +6,7 @@
> > > > >  config INTEL_BROADWELL
> > > > >  	bool
> > > > >  	select CACHE_MRC_BIN
> > > > > +	select DM_EVENT
> > > > >  	select ARCH_EARLY_INIT_R
> > > > >  	imply HAVE_INTEL_ME
> > > > >  	imply ENABLE_MRC_CACHE
> > > > > diff --git a/arch/x86/cpu/ivybridge/Kconfig b/arch/x86/cpu/ivybridge/Kconfig
> > > > > index be3ef5e5d8f8..704f145adf88 100644
> > > > > --- a/arch/x86/cpu/ivybridge/Kconfig
> > > > > +++ b/arch/x86/cpu/ivybridge/Kconfig
> > > > > @@ -8,6 +8,7 @@
> > > > >  config NORTHBRIDGE_INTEL_IVYBRIDGE
> > > > >  	bool
> > > > >  	select CACHE_MRC_BIN if HAVE_MRC
> > > > > +	select DM_EVENT
> > > > >  	imply HAVE_INTEL_ME
> > > > >  	imply ENABLE_MRC_CACHE
> > > > >  	imply AHCI_PCI
> > > > > diff --git a/arch/x86/cpu/quark/Kconfig b/arch/x86/cpu/quark/Kconfig
> > > > > index 61bb5792c868..0d4008a31f4c 100644
> > > > > --- a/arch/x86/cpu/quark/Kconfig
> > > > > +++ b/arch/x86/cpu/quark/Kconfig
> > > > > @@ -7,6 +7,7 @@ config INTEL_QUARK
> > > > >  	select HAVE_RMU
> > > > >  	select ARCH_EARLY_INIT_R
> > > > >  	select ARCH_MISC_INIT
> > > > > +	select DM_EVENT
> > > > >  	imply ENABLE_MRC_CACHE
> > > > >  	imply ETH_DESIGNWARE
> > > > >  	imply ICH_SPI
> > > > > diff --git a/board/google/Kconfig b/board/google/Kconfig
> > > > > index 0474b4e69384..a0f1a6097641 100644
> > > > > --- a/board/google/Kconfig
> > > > > +++ b/board/google/Kconfig
> > > > > @@ -18,6 +18,7 @@ choice
> > > > >  config TARGET_CHROMEBOOK_CORAL
> > > > >  	bool "Chromebook coral"
> > > > >  	select BIOSEMU
> > > > > +	select EVENT
> > > > >  	help
> > > > >  	  This is a range of Intel-based laptops released in 2018. They use an
> > > > >  	  Intel Apollo Lake SoC. The design supports WiFi, 4GB to 16GB of
> > > > > diff --git a/board/keymile/Kconfig b/board/keymile/Kconfig
> > > > > index e5d7c80a869d..bf899d005c46 100644
> > > > > --- a/board/keymile/Kconfig
> > > > > +++ b/board/keymile/Kconfig
> > > > > @@ -124,6 +124,7 @@ config SYS_IVM_EEPROM_PAGE_LEN
> > > > >  
> > > > >  config PG_WCOM_UBOOT_UPDATE_SUPPORTED
> > > > >  	bool "Enable U-boot Field Fail-Safe Update Functionality"
> > > > > +	select EVENT
> > > > >  	default n
> > > > >  	help
> > > > >  	  Indicates that field fail-safe u-boot update is supported.
> > > > > diff --git a/boot/Kconfig b/boot/Kconfig
> > > > > index 30bc182fcd5c..725afd36a3b9 100644
> > > > > --- a/boot/Kconfig
> > > > > +++ b/boot/Kconfig
> > > > > @@ -474,6 +474,7 @@ config BOOTMETH_VBE
> > > > >  	depends on FIT
> > > > >  	default y
> > > > >  	select BOOTMETH_GLOBAL
> > > > > +	select EVENT
> > > > >  	help
> > > > >  	  Enables support for VBE boot. This is a standard boot method which
> > > > >  	  supports selection of various firmware components, seleciton of an OS to
> > > > > diff --git a/cmd/Kconfig b/cmd/Kconfig
> > > > > index b2aefae9cb70..4fe2c75de256 100644
> > > > > --- a/cmd/Kconfig
> > > > > +++ b/cmd/Kconfig
> > > > > @@ -2622,6 +2622,7 @@ config CMD_DIAG
> > > > >  
> > > > >  config CMD_EVENT
> > > > >  	bool "event - Show information about events"
> > > > > +	depends on EVENT
> > > > >  	default y if EVENT_DEBUG
> > > > >  	help
> > > > >  	  This enables the 'event' command which provides information about
> > > > > diff --git a/common/Kconfig b/common/Kconfig
> > > > > index 73e3fe36573d..6f5aa4e53aae 100644
> > > > > --- a/common/Kconfig
> > > > > +++ b/common/Kconfig
> > > > > @@ -604,24 +604,23 @@ config CYCLIC_MAX_CPU_TIME_US
> > > > >  endif # CYCLIC
> > > > >  
> > > > >  config EVENT
> > > > > -	bool "General-purpose event-handling mechanism"
> > > > > -	default y if SANDBOX
> > > > > +	bool
> > > > >  	help
> > > > > -	  This enables sending and processing of events, to allow interested
> > > > > -	  parties to be alerted when something happens. This is an attempt to
> > > > > -	  stem the flow of weak functions, hooks, functions in board_f.c
> > > > > -	  and board_r.c and the Kconfig options below.
> > > > > +	  This adds a framework for general purpose sending and processing of
> > > > > +	  events, to allow interested parties to be alerted when something
> > > > > +	  happens. This is an attempt to stem 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
> > > > > +	bool
> > > > >  	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
> > > > > +	  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
> > > > > diff --git a/drivers/core/Kconfig b/drivers/core/Kconfig
> > > > > index 8fde77c23ee0..6fc8854b574b 100644
> > > > > --- a/drivers/core/Kconfig
> > > > > +++ b/drivers/core/Kconfig
> > > > > @@ -109,13 +109,14 @@ config DM_DEVICE_REMOVE
> > > > >  	  causes USB host controllers to not be stopped when booting the OS.
> > > > >  
> > > > >  config DM_EVENT
> > > > > -	bool "Support events with driver model"
> > > > > -	depends on DM && EVENT
> > > > > -	default y if SANDBOX
> > > > > +	bool
> > > > > +	depends on DM
> > > > > +	select EVENT
> > > > >  	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.
> > > > > +	  register a 'spy' function that is called when the event occurs. Such
> > > > > +	  subsystems must select this option.
> > > > >  
> > > > >  config SPL_DM_DEVICE_REMOVE
> > > > >  	bool "Support device removal in SPL"
> > > > > diff --git a/drivers/cpu/Kconfig b/drivers/cpu/Kconfig
> > > > > index 21874335c873..3bf04105e5e9 100644
> > > > > --- a/drivers/cpu/Kconfig
> > > > > +++ b/drivers/cpu/Kconfig
> > > > > @@ -23,7 +23,6 @@ config CPU_RISCV
> > > > >  config CPU_MICROBLAZE
> > > > >  	bool "Enable Microblaze CPU driver"
> > > > >  	depends on CPU && MICROBLAZE
> > > > > -	select EVENT
> > > > >  	select DM_EVENT
> > > > >  	select XILINX_MICROBLAZE0_PVR
> > > > >  	help
> > > > > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> > > > > index b498c72206fd..d04203cddab4 100644
> > > > > --- a/lib/efi_loader/Kconfig
> > > > > +++ b/lib/efi_loader/Kconfig
> > > > > @@ -14,9 +14,8 @@ config EFI_LOADER
> > > > >  	depends on !EFI_APP
> > > > >  	default y if !ARM || SYS_CPU = armv7 || SYS_CPU = armv8
> > > > >  	select CHARSET
> > > > > -	select DM_EVENT
> > > > > -	select EVENT
> > > > > -	select EVENT_DYNAMIC
> > > > > +	select DM_EVENT if PARTITIONS
> > > > > +	select EVENT_DYNAMIC if EVENT
> > > > 
> > > > Since UEFI subsystem, in particular efi_disk, uses event_register() or
> > > > EVENT_DYNAMIC directly, adding 'if EVENT' looks strange.
> > > > Instead, I suggest that 'select EVENT' be added to CONFIG_EVENT_DYNAMIC
> > > > and then we can safely
> > > >         select EVENT_DYNAMIC if PARTITIONS
> > > > here.
> > > 
> > > How about "if PARTITIONS" for both? Dynamic events can be for any of the
> > > types, so the EFI_LOADER part should be select'ing DM_EVENT and
> > > EVENT_DYNAMIC. And the dynamic event code is a subset of the regular
> > > event code.
> > 
> > Okay, that is what I meant.
> 
> I have to correct myself.
> The fact is that event mechanism is used to detect not only partitions but also
> raw block devices, then DM_EVENT & EVENT_DYNAMIC must always be selected even if
> CONFIG_PARTITIONS is disabled (it's unlikely, though).
> Please note that EFI_LOADER itself has a dependency on BLK.
> 
> So the correct fix would be:
> 1. remove 'select EVENT' from EFI_LOADER
> 2. add 'select EVENT' to EVENT_DYNAMIC

Unless I'm missing something, lib/efi_driver/efi_block_device.c is only
built with PARTITIONS=y, and that's where the event_notify calls are.
And again, EVENT_DYNAMIC is a feature of the event framework, it does
not make sense to enable it by itself.

-- 
Tom

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [v3 2/2] event: Correct dependencies on the EVENT framework
  2023-01-14 20:49 [PATCH 1/2] x86: Fix saying arch_cpu_init_dm in debug/docs Tom Rini
  2023-01-14 20:49 ` [PATCH 2/2] event: Correct dependencies on the EVENT framework Tom Rini
  2023-01-15  1:31 ` [PATCH 1/2] x86: Fix saying arch_cpu_init_dm in debug/docs Simon Glass
@ 2023-01-16 20:46 ` Tom Rini
  2023-01-16 21:01   ` Heinrich Schuchardt
  2023-01-19 14:46   ` Tom Rini
  2023-01-19 14:46 ` [PATCH 1/2] x86: Fix saying arch_cpu_init_dm in debug/docs Tom Rini
  3 siblings, 2 replies; 20+ messages in thread
From: Tom Rini @ 2023-01-16 20:46 UTC (permalink / raw)
  To: u-boot
  Cc: AKASHI Takahiro, Heinrich Schuchardt, Oliver Graute,
	Francesco Dolcini, Simon Glass, Fabio Estevam

The event framework is just that, a framework. Enabling it by itself
does nothing, so we shouldn't ask the user about it. Reword (and correct
typos) around this the option and help text. This also applies to
DM_EVENT and EVENT_DYNAMIC. Only EVENT_DEBUG and CMD_EVENT should be
visible to the user to select, when EVENT is selected.

With this, it's time to address the larger problems. When functionality
uses events, typically via EVENT_SPY, the appropriate framework then
must be select'd and NOT imply'd. As the functionality will cease to
work (and so, platforms will fail to boot) this is non-optional and
where select is appropriate. Audit the current users of EVENT_SPY to
have a more fine-grained approach to select'ing the framework where
used. Also ensure the current users of event_register and also select
EVENT_DYNAMIC.

Cc: AKASHI Takahiro <takahiro.akashi@linaro.org>
Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
Reported-by: Oliver Graute <Oliver.Graute@kococonnector.com>
Reported-by: Francesco Dolcini <francesco.dolcini@toradex.com>
Fixes: 7fe32b3442f0 ("event: Convert arch_cpu_init_dm() to use events")
Fixes: 42fdcebf859f ("event: Convert misc_init_f() to use events")
Fixes: c5ef2025579e ("dm: fix DM_EVENT dependencies")
Signed-off-by: Tom Rini <trini@konsulko.com>
Tested-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Fabio Estevam <festevam@denx.de>
---
Changes in v3:
- Drop guards around when EFI_LOADER selects DM_EVENT / EVENT_DYNAMIC,
  keep comment.

Changes in v2:
- Collect tags
- Reword the commit message a little, reword and comment on the
  EFI_LOADER case.
- Add the rest of the VBE cases I had forgotten.
---
 arch/Kconfig                     |  6 +++---
 arch/arm/Kconfig                 |  9 ++++-----
 arch/arm/mach-omap2/Kconfig      |  3 +++
 arch/mips/Kconfig                |  2 +-
 arch/powerpc/cpu/mpc85xx/Kconfig |  1 +
 arch/x86/Kconfig                 |  1 +
 arch/x86/cpu/baytrail/Kconfig    |  1 +
 arch/x86/cpu/broadwell/Kconfig   |  1 +
 arch/x86/cpu/ivybridge/Kconfig   |  1 +
 arch/x86/cpu/quark/Kconfig       |  1 +
 board/google/Kconfig             |  1 +
 board/keymile/Kconfig            |  1 +
 boot/Kconfig                     |  3 +++
 cmd/Kconfig                      |  1 +
 common/Kconfig                   | 17 ++++++++---------
 drivers/core/Kconfig             |  9 +++++----
 drivers/cpu/Kconfig              |  1 -
 lib/efi_loader/Kconfig           |  2 +-
 18 files changed, 37 insertions(+), 24 deletions(-)

diff --git a/arch/Kconfig b/arch/Kconfig
index 8fb87b7d857c..d30676ae817b 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -93,7 +93,7 @@ config NIOS2
 	bool "Nios II architecture"
 	select CPU
 	select DM
-	imply DM_EVENT
+	select DM_EVENT
 	select OF_CONTROL
 	select SUPPORT_OF_CONTROL
 	imply CMD_DM
@@ -111,9 +111,9 @@ config RISCV
 	select SUPPORT_OF_CONTROL
 	select OF_CONTROL
 	select DM
+	select DM_EVENT
 	imply SPL_SEPARATE_BSS if SPL
 	imply DM_SERIAL
-	imply DM_EVENT
 	imply DM_MMC
 	imply DM_SPI
 	imply DM_SPI_FLASH
@@ -136,6 +136,7 @@ config SANDBOX
 	select BZIP2
 	select CMD_POWEROFF
 	select DM
+	select DM_EVENT
 	select DM_FUZZING_ENGINE
 	select DM_GPIO
 	select DM_I2C
@@ -240,7 +241,6 @@ config X86
 	imply CMD_SF
 	imply CMD_SF_TEST
 	imply CMD_ZBOOT
-	imply DM_EVENT
 	imply DM_GPIO
 	imply DM_KEYBOARD
 	imply DM_MMC
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index bbf1d5227b3f..c9a44ebc2213 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -778,7 +778,6 @@ config ARCH_OMAP2PLUS
 	select SUPPORT_SPL
 	imply TI_SYSC if DM && OF_CONTROL
 	imply FIT
-	imply DM_EVENT
 	imply SPL_SEPARATE_BSS
 
 config ARCH_MESON
@@ -823,11 +822,11 @@ config ARCH_IMX8
 	select SYS_FSL_SEC_COMPAT_4
 	select SYS_FSL_SEC_LE
 	select DM
+	select DM_EVENT
 	select GPIO_EXTRA_HEADER
 	select MACH_IMX
 	select OF_CONTROL
 	select ENABLE_ARM_SOC_BOOT0_HOOK
-	imply DM_EVENT
 
 config ARCH_IMX8M
 	bool "NXP i.MX8M platform"
@@ -839,14 +838,15 @@ config ARCH_IMX8M
 	select SYS_FSL_SEC_LE
 	select SYS_I2C_MXC
 	select DM
+	select DM_EVENT if CLK
 	select SUPPORT_SPL
 	imply CMD_DM
-	imply DM_EVENT
 
 config ARCH_IMX8ULP
 	bool "NXP i.MX8ULP platform"
 	select ARM64
 	select DM
+	select DM_EVENT
 	select MACH_IMX
 	select OF_CONTROL
 	select SUPPORT_SPL
@@ -854,19 +854,18 @@ config ARCH_IMX8ULP
 	select MISC
 	select IMX_SENTINEL
 	imply CMD_DM
-	imply DM_EVENT
 
 config ARCH_IMX9
 	bool "NXP i.MX9 platform"
 	select ARM64
 	select DM
+	select DM_EVENT
 	select MACH_IMX
 	select SUPPORT_SPL
 	select GPIO_EXTRA_HEADER
 	select MISC
 	select IMX_SENTINEL
 	imply CMD_DM
-	imply DM_EVENT
 
 config ARCH_IMXRT
 	bool "NXP i.MXRT platform"
diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
index 1db71df27212..309b967b0dd5 100644
--- a/arch/arm/mach-omap2/Kconfig
+++ b/arch/arm/mach-omap2/Kconfig
@@ -31,6 +31,7 @@ config OMAP34XX
 
 config OMAP44XX
 	bool "OMAP44XX SoC"
+	select DM_EVENT
 	select SPL_USE_TINY_PRINTF
 	select SPL_SYS_NO_VECTOR_TABLE if SPL
 	imply NAND_OMAP_ELM
@@ -55,6 +56,7 @@ config OMAP54XX
 	bool "OMAP54XX SoC"
 	select ARM_CORTEX_A15_CVE_2017_5715
 	select ARM_ERRATA_798870
+	select DM_EVENT
 	select SYS_THUMB_BUILD
 	imply NAND_OMAP_ELM
 	imply NAND_OMAP_GPMC
@@ -111,6 +113,7 @@ config AM43XX
 config AM33XX
 	bool "AM33XX SoC"
 	select ARM_CORTEX_A8_CVE_2017_5715
+	select DM_EVENT
 	select SPECIFY_CONSOLE_INDEX
 	imply NAND_OMAP_ELM
 	imply NAND_OMAP_GPMC
diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index 23142bd2700d..569f5f48bc6c 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -121,6 +121,7 @@ config MACH_PIC32
 	bool "Support Microchip PIC32"
 	select HAS_FIXED_TIMER_FREQUENCY
 	select DM
+	select DM_EVENT
 	select OF_CONTROL
 	imply CMD_DM
 
@@ -128,7 +129,6 @@ config TARGET_BOSTON
 	bool "Support Boston"
 	select HAS_FIXED_TIMER_FREQUENCY
 	select DM
-	imply DM_EVENT
 	select DM_SERIAL
 	select MIPS_CM
 	select SYS_CACHE_SHIFT_6
diff --git a/arch/powerpc/cpu/mpc85xx/Kconfig b/arch/powerpc/cpu/mpc85xx/Kconfig
index 1b180481a483..2c54a9e2120f 100644
--- a/arch/powerpc/cpu/mpc85xx/Kconfig
+++ b/arch/powerpc/cpu/mpc85xx/Kconfig
@@ -248,6 +248,7 @@ config TARGET_KMP204X
 config TARGET_KMCENT2
 	bool "Support kmcent2"
 	select VENDOR_KM
+	select EVENT
 	select FSL_CORENET
 	select SYS_DPAA_FMAN
 	select SYS_DPAA_PME
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 93f1c77be3f3..07be5cd05ec0 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -395,6 +395,7 @@ config FSP_VERSION1
 
 config FSP_VERSION2
 	bool "FSP version 2.x"
+	select DM_EVENT
 	help
 	  This covers versions 2.0 and 2.1. See here for details:
 	  https://github.com/IntelFsp/fsp/wiki
diff --git a/arch/x86/cpu/baytrail/Kconfig b/arch/x86/cpu/baytrail/Kconfig
index d2c3473d6abf..a8efea8a3413 100644
--- a/arch/x86/cpu/baytrail/Kconfig
+++ b/arch/x86/cpu/baytrail/Kconfig
@@ -7,6 +7,7 @@ config INTEL_BAYTRAIL
 	select HAVE_FSP
 	select ARCH_MISC_INIT
 	select CPU_INTEL_TURBO_NOT_PACKAGE_SCOPED
+	select DM_EVENT
 	imply HAVE_INTEL_ME
 	imply ENABLE_MRC_CACHE
 	imply AHCI_PCI
diff --git a/arch/x86/cpu/broadwell/Kconfig b/arch/x86/cpu/broadwell/Kconfig
index 5b015c89d950..39deda364479 100644
--- a/arch/x86/cpu/broadwell/Kconfig
+++ b/arch/x86/cpu/broadwell/Kconfig
@@ -6,6 +6,7 @@
 config INTEL_BROADWELL
 	bool
 	select CACHE_MRC_BIN
+	select DM_EVENT
 	select ARCH_EARLY_INIT_R
 	imply HAVE_INTEL_ME
 	imply ENABLE_MRC_CACHE
diff --git a/arch/x86/cpu/ivybridge/Kconfig b/arch/x86/cpu/ivybridge/Kconfig
index be3ef5e5d8f8..704f145adf88 100644
--- a/arch/x86/cpu/ivybridge/Kconfig
+++ b/arch/x86/cpu/ivybridge/Kconfig
@@ -8,6 +8,7 @@
 config NORTHBRIDGE_INTEL_IVYBRIDGE
 	bool
 	select CACHE_MRC_BIN if HAVE_MRC
+	select DM_EVENT
 	imply HAVE_INTEL_ME
 	imply ENABLE_MRC_CACHE
 	imply AHCI_PCI
diff --git a/arch/x86/cpu/quark/Kconfig b/arch/x86/cpu/quark/Kconfig
index 61bb5792c868..0d4008a31f4c 100644
--- a/arch/x86/cpu/quark/Kconfig
+++ b/arch/x86/cpu/quark/Kconfig
@@ -7,6 +7,7 @@ config INTEL_QUARK
 	select HAVE_RMU
 	select ARCH_EARLY_INIT_R
 	select ARCH_MISC_INIT
+	select DM_EVENT
 	imply ENABLE_MRC_CACHE
 	imply ETH_DESIGNWARE
 	imply ICH_SPI
diff --git a/board/google/Kconfig b/board/google/Kconfig
index 0474b4e69384..a0f1a6097641 100644
--- a/board/google/Kconfig
+++ b/board/google/Kconfig
@@ -18,6 +18,7 @@ choice
 config TARGET_CHROMEBOOK_CORAL
 	bool "Chromebook coral"
 	select BIOSEMU
+	select EVENT
 	help
 	  This is a range of Intel-based laptops released in 2018. They use an
 	  Intel Apollo Lake SoC. The design supports WiFi, 4GB to 16GB of
diff --git a/board/keymile/Kconfig b/board/keymile/Kconfig
index e5d7c80a869d..bf899d005c46 100644
--- a/board/keymile/Kconfig
+++ b/board/keymile/Kconfig
@@ -124,6 +124,7 @@ config SYS_IVM_EEPROM_PAGE_LEN
 
 config PG_WCOM_UBOOT_UPDATE_SUPPORTED
 	bool "Enable U-boot Field Fail-Safe Update Functionality"
+	select EVENT
 	default n
 	help
 	  Indicates that field fail-safe u-boot update is supported.
diff --git a/boot/Kconfig b/boot/Kconfig
index 30bc182fcd5c..daa01a10ebdc 100644
--- a/boot/Kconfig
+++ b/boot/Kconfig
@@ -474,6 +474,7 @@ config BOOTMETH_VBE
 	depends on FIT
 	default y
 	select BOOTMETH_GLOBAL
+	select EVENT
 	help
 	  Enables support for VBE boot. This is a standard boot method which
 	  supports selection of various firmware components, seleciton of an OS to
@@ -482,6 +483,7 @@ config BOOTMETH_VBE
 config SPL_BOOTMETH_VBE
 	bool "Bootdev support for Verified Boot for Embedded (SPL)"
 	depends on SPL && FIT
+	select EVENT
 	default y if VPL
 	help
 	  Enables support for VBE boot. This is a standard boot method which
@@ -491,6 +493,7 @@ config SPL_BOOTMETH_VBE
 config VPL_BOOTMETH_VBE
 	bool "Bootdev support for Verified Boot for Embedded (VPL)"
 	depends on VPL && FIT
+	select EVENT
 	default y
 	help
 	  Enables support for VBE boot. This is a standard boot method which
diff --git a/cmd/Kconfig b/cmd/Kconfig
index b2aefae9cb70..4fe2c75de256 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -2622,6 +2622,7 @@ config CMD_DIAG
 
 config CMD_EVENT
 	bool "event - Show information about events"
+	depends on EVENT
 	default y if EVENT_DEBUG
 	help
 	  This enables the 'event' command which provides information about
diff --git a/common/Kconfig b/common/Kconfig
index 439b2198f605..1c9f4774ba7a 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -604,24 +604,23 @@ config CYCLIC_MAX_CPU_TIME_US
 endif # CYCLIC
 
 config EVENT
-	bool "General-purpose event-handling mechanism"
-	default y if SANDBOX
+	bool
 	help
-	  This enables sending and processing of events, to allow interested
-	  parties to be alerted when something happens. This is an attempt to
-	  stem the flow of weak functions, hooks, functions in board_f.c
-	  and board_r.c and the Kconfig options below.
+	  This adds a framework for general purpose sending and processing of
+	  events, to allow interested parties to be alerted when something
+	  happens. This is an attempt to stem 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
+	bool
 	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
+	  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
diff --git a/drivers/core/Kconfig b/drivers/core/Kconfig
index 8fde77c23ee0..6fc8854b574b 100644
--- a/drivers/core/Kconfig
+++ b/drivers/core/Kconfig
@@ -109,13 +109,14 @@ config DM_DEVICE_REMOVE
 	  causes USB host controllers to not be stopped when booting the OS.
 
 config DM_EVENT
-	bool "Support events with driver model"
-	depends on DM && EVENT
-	default y if SANDBOX
+	bool
+	depends on DM
+	select EVENT
 	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.
+	  register a 'spy' function that is called when the event occurs. Such
+	  subsystems must select this option.
 
 config SPL_DM_DEVICE_REMOVE
 	bool "Support device removal in SPL"
diff --git a/drivers/cpu/Kconfig b/drivers/cpu/Kconfig
index 21874335c873..3bf04105e5e9 100644
--- a/drivers/cpu/Kconfig
+++ b/drivers/cpu/Kconfig
@@ -23,7 +23,6 @@ config CPU_RISCV
 config CPU_MICROBLAZE
 	bool "Enable Microblaze CPU driver"
 	depends on CPU && MICROBLAZE
-	select EVENT
 	select DM_EVENT
 	select XILINX_MICROBLAZE0_PVR
 	help
diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
index b498c72206fd..b630d88ef9e2 100644
--- a/lib/efi_loader/Kconfig
+++ b/lib/efi_loader/Kconfig
@@ -14,8 +14,8 @@ config EFI_LOADER
 	depends on !EFI_APP
 	default y if !ARM || SYS_CPU = armv7 || SYS_CPU = armv8
 	select CHARSET
+	# We need to send DM events, dynamically, in the EFI block driver
 	select DM_EVENT
-	select EVENT
 	select EVENT_DYNAMIC
 	select LIB_UUID
 	imply PARTITION_UUIDS
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [v3 2/2] event: Correct dependencies on the EVENT framework
  2023-01-16 20:46 ` [v3 2/2] event: Correct dependencies on the EVENT framework Tom Rini
@ 2023-01-16 21:01   ` Heinrich Schuchardt
  2023-01-16 21:04     ` Tom Rini
  2023-01-19 14:46   ` Tom Rini
  1 sibling, 1 reply; 20+ messages in thread
From: Heinrich Schuchardt @ 2023-01-16 21:01 UTC (permalink / raw)
  To: Tom Rini
  Cc: AKASHI Takahiro, Oliver Graute, Francesco Dolcini, Simon Glass,
	Fabio Estevam, u-boot

On 1/16/23 21:46, Tom Rini wrote:
> The event framework is just that, a framework. Enabling it by itself
> does nothing, so we shouldn't ask the user about it. Reword (and correct
> typos) around this the option and help text. This also applies to
> DM_EVENT and EVENT_DYNAMIC. Only EVENT_DEBUG and CMD_EVENT should be
> visible to the user to select, when EVENT is selected.
>
> With this, it's time to address the larger problems. When functionality
> uses events, typically via EVENT_SPY, the appropriate framework then
> must be select'd and NOT imply'd. As the functionality will cease to
> work (and so, platforms will fail to boot) this is non-optional and
> where select is appropriate. Audit the current users of EVENT_SPY to
> have a more fine-grained approach to select'ing the framework where
> used. Also ensure the current users of event_register and also select
> EVENT_DYNAMIC.
>
> Cc: AKASHI Takahiro <takahiro.akashi@linaro.org>
> Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Reported-by: Oliver Graute <Oliver.Graute@kococonnector.com>
> Reported-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> Fixes: 7fe32b3442f0 ("event: Convert arch_cpu_init_dm() to use events")
> Fixes: 42fdcebf859f ("event: Convert misc_init_f() to use events")
> Fixes: c5ef2025579e ("dm: fix DM_EVENT dependencies")
> Signed-off-by: Tom Rini <trini@konsulko.com>
> Tested-by: Simon Glass <sjg@chromium.org>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> Reviewed-by: Fabio Estevam <festevam@denx.de>
> ---
> Changes in v3:
> - Drop guards around when EFI_LOADER selects DM_EVENT / EVENT_DYNAMIC,
>    keep comment.
>
> Changes in v2:
> - Collect tags
> - Reword the commit message a little, reword and comment on the
>    EFI_LOADER case.
> - Add the rest of the VBE cases I had forgotten.
> ---
>   arch/Kconfig                     |  6 +++---
>   arch/arm/Kconfig                 |  9 ++++-----
>   arch/arm/mach-omap2/Kconfig      |  3 +++
>   arch/mips/Kconfig                |  2 +-
>   arch/powerpc/cpu/mpc85xx/Kconfig |  1 +
>   arch/x86/Kconfig                 |  1 +
>   arch/x86/cpu/baytrail/Kconfig    |  1 +
>   arch/x86/cpu/broadwell/Kconfig   |  1 +
>   arch/x86/cpu/ivybridge/Kconfig   |  1 +
>   arch/x86/cpu/quark/Kconfig       |  1 +
>   board/google/Kconfig             |  1 +
>   board/keymile/Kconfig            |  1 +
>   boot/Kconfig                     |  3 +++
>   cmd/Kconfig                      |  1 +
>   common/Kconfig                   | 17 ++++++++---------
>   drivers/core/Kconfig             |  9 +++++----
>   drivers/cpu/Kconfig              |  1 -
>   lib/efi_loader/Kconfig           |  2 +-
>   18 files changed, 37 insertions(+), 24 deletions(-)
>
> diff --git a/arch/Kconfig b/arch/Kconfig
> index 8fb87b7d857c..d30676ae817b 100644
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -93,7 +93,7 @@ config NIOS2
>   	bool "Nios II architecture"
>   	select CPU
>   	select DM
> -	imply DM_EVENT
> +	select DM_EVENT
>   	select OF_CONTROL
>   	select SUPPORT_OF_CONTROL
>   	imply CMD_DM
> @@ -111,9 +111,9 @@ config RISCV
>   	select SUPPORT_OF_CONTROL
>   	select OF_CONTROL
>   	select DM
> +	select DM_EVENT
>   	imply SPL_SEPARATE_BSS if SPL
>   	imply DM_SERIAL
> -	imply DM_EVENT
>   	imply DM_MMC
>   	imply DM_SPI
>   	imply DM_SPI_FLASH
> @@ -136,6 +136,7 @@ config SANDBOX
>   	select BZIP2
>   	select CMD_POWEROFF
>   	select DM
> +	select DM_EVENT
>   	select DM_FUZZING_ENGINE
>   	select DM_GPIO
>   	select DM_I2C
> @@ -240,7 +241,6 @@ config X86
>   	imply CMD_SF
>   	imply CMD_SF_TEST
>   	imply CMD_ZBOOT
> -	imply DM_EVENT
>   	imply DM_GPIO
>   	imply DM_KEYBOARD
>   	imply DM_MMC
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index bbf1d5227b3f..c9a44ebc2213 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -778,7 +778,6 @@ config ARCH_OMAP2PLUS
>   	select SUPPORT_SPL
>   	imply TI_SYSC if DM && OF_CONTROL
>   	imply FIT
> -	imply DM_EVENT
>   	imply SPL_SEPARATE_BSS
>
>   config ARCH_MESON
> @@ -823,11 +822,11 @@ config ARCH_IMX8
>   	select SYS_FSL_SEC_COMPAT_4
>   	select SYS_FSL_SEC_LE
>   	select DM
> +	select DM_EVENT
>   	select GPIO_EXTRA_HEADER
>   	select MACH_IMX
>   	select OF_CONTROL
>   	select ENABLE_ARM_SOC_BOOT0_HOOK
> -	imply DM_EVENT
>
>   config ARCH_IMX8M
>   	bool "NXP i.MX8M platform"
> @@ -839,14 +838,15 @@ config ARCH_IMX8M
>   	select SYS_FSL_SEC_LE
>   	select SYS_I2C_MXC
>   	select DM
> +	select DM_EVENT if CLK
>   	select SUPPORT_SPL
>   	imply CMD_DM
> -	imply DM_EVENT
>
>   config ARCH_IMX8ULP
>   	bool "NXP i.MX8ULP platform"
>   	select ARM64
>   	select DM
> +	select DM_EVENT
>   	select MACH_IMX
>   	select OF_CONTROL
>   	select SUPPORT_SPL
> @@ -854,19 +854,18 @@ config ARCH_IMX8ULP
>   	select MISC
>   	select IMX_SENTINEL
>   	imply CMD_DM
> -	imply DM_EVENT
>
>   config ARCH_IMX9
>   	bool "NXP i.MX9 platform"
>   	select ARM64
>   	select DM
> +	select DM_EVENT
>   	select MACH_IMX
>   	select SUPPORT_SPL
>   	select GPIO_EXTRA_HEADER
>   	select MISC
>   	select IMX_SENTINEL
>   	imply CMD_DM
> -	imply DM_EVENT
>
>   config ARCH_IMXRT
>   	bool "NXP i.MXRT platform"
> diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
> index 1db71df27212..309b967b0dd5 100644
> --- a/arch/arm/mach-omap2/Kconfig
> +++ b/arch/arm/mach-omap2/Kconfig
> @@ -31,6 +31,7 @@ config OMAP34XX
>
>   config OMAP44XX
>   	bool "OMAP44XX SoC"
> +	select DM_EVENT
>   	select SPL_USE_TINY_PRINTF
>   	select SPL_SYS_NO_VECTOR_TABLE if SPL
>   	imply NAND_OMAP_ELM
> @@ -55,6 +56,7 @@ config OMAP54XX
>   	bool "OMAP54XX SoC"
>   	select ARM_CORTEX_A15_CVE_2017_5715
>   	select ARM_ERRATA_798870
> +	select DM_EVENT
>   	select SYS_THUMB_BUILD
>   	imply NAND_OMAP_ELM
>   	imply NAND_OMAP_GPMC
> @@ -111,6 +113,7 @@ config AM43XX
>   config AM33XX
>   	bool "AM33XX SoC"
>   	select ARM_CORTEX_A8_CVE_2017_5715
> +	select DM_EVENT
>   	select SPECIFY_CONSOLE_INDEX
>   	imply NAND_OMAP_ELM
>   	imply NAND_OMAP_GPMC
> diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
> index 23142bd2700d..569f5f48bc6c 100644
> --- a/arch/mips/Kconfig
> +++ b/arch/mips/Kconfig
> @@ -121,6 +121,7 @@ config MACH_PIC32
>   	bool "Support Microchip PIC32"
>   	select HAS_FIXED_TIMER_FREQUENCY
>   	select DM
> +	select DM_EVENT
>   	select OF_CONTROL
>   	imply CMD_DM
>
> @@ -128,7 +129,6 @@ config TARGET_BOSTON
>   	bool "Support Boston"
>   	select HAS_FIXED_TIMER_FREQUENCY
>   	select DM
> -	imply DM_EVENT
>   	select DM_SERIAL
>   	select MIPS_CM
>   	select SYS_CACHE_SHIFT_6
> diff --git a/arch/powerpc/cpu/mpc85xx/Kconfig b/arch/powerpc/cpu/mpc85xx/Kconfig
> index 1b180481a483..2c54a9e2120f 100644
> --- a/arch/powerpc/cpu/mpc85xx/Kconfig
> +++ b/arch/powerpc/cpu/mpc85xx/Kconfig
> @@ -248,6 +248,7 @@ config TARGET_KMP204X
>   config TARGET_KMCENT2
>   	bool "Support kmcent2"
>   	select VENDOR_KM
> +	select EVENT
>   	select FSL_CORENET
>   	select SYS_DPAA_FMAN
>   	select SYS_DPAA_PME
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 93f1c77be3f3..07be5cd05ec0 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -395,6 +395,7 @@ config FSP_VERSION1
>
>   config FSP_VERSION2
>   	bool "FSP version 2.x"
> +	select DM_EVENT
>   	help
>   	  This covers versions 2.0 and 2.1. See here for details:
>   	  https://github.com/IntelFsp/fsp/wiki
> diff --git a/arch/x86/cpu/baytrail/Kconfig b/arch/x86/cpu/baytrail/Kconfig
> index d2c3473d6abf..a8efea8a3413 100644
> --- a/arch/x86/cpu/baytrail/Kconfig
> +++ b/arch/x86/cpu/baytrail/Kconfig
> @@ -7,6 +7,7 @@ config INTEL_BAYTRAIL
>   	select HAVE_FSP
>   	select ARCH_MISC_INIT
>   	select CPU_INTEL_TURBO_NOT_PACKAGE_SCOPED
> +	select DM_EVENT
>   	imply HAVE_INTEL_ME
>   	imply ENABLE_MRC_CACHE
>   	imply AHCI_PCI
> diff --git a/arch/x86/cpu/broadwell/Kconfig b/arch/x86/cpu/broadwell/Kconfig
> index 5b015c89d950..39deda364479 100644
> --- a/arch/x86/cpu/broadwell/Kconfig
> +++ b/arch/x86/cpu/broadwell/Kconfig
> @@ -6,6 +6,7 @@
>   config INTEL_BROADWELL
>   	bool
>   	select CACHE_MRC_BIN
> +	select DM_EVENT
>   	select ARCH_EARLY_INIT_R
>   	imply HAVE_INTEL_ME
>   	imply ENABLE_MRC_CACHE
> diff --git a/arch/x86/cpu/ivybridge/Kconfig b/arch/x86/cpu/ivybridge/Kconfig
> index be3ef5e5d8f8..704f145adf88 100644
> --- a/arch/x86/cpu/ivybridge/Kconfig
> +++ b/arch/x86/cpu/ivybridge/Kconfig
> @@ -8,6 +8,7 @@
>   config NORTHBRIDGE_INTEL_IVYBRIDGE
>   	bool
>   	select CACHE_MRC_BIN if HAVE_MRC
> +	select DM_EVENT
>   	imply HAVE_INTEL_ME
>   	imply ENABLE_MRC_CACHE
>   	imply AHCI_PCI
> diff --git a/arch/x86/cpu/quark/Kconfig b/arch/x86/cpu/quark/Kconfig
> index 61bb5792c868..0d4008a31f4c 100644
> --- a/arch/x86/cpu/quark/Kconfig
> +++ b/arch/x86/cpu/quark/Kconfig
> @@ -7,6 +7,7 @@ config INTEL_QUARK
>   	select HAVE_RMU
>   	select ARCH_EARLY_INIT_R
>   	select ARCH_MISC_INIT
> +	select DM_EVENT
>   	imply ENABLE_MRC_CACHE
>   	imply ETH_DESIGNWARE
>   	imply ICH_SPI
> diff --git a/board/google/Kconfig b/board/google/Kconfig
> index 0474b4e69384..a0f1a6097641 100644
> --- a/board/google/Kconfig
> +++ b/board/google/Kconfig
> @@ -18,6 +18,7 @@ choice
>   config TARGET_CHROMEBOOK_CORAL
>   	bool "Chromebook coral"
>   	select BIOSEMU
> +	select EVENT
>   	help
>   	  This is a range of Intel-based laptops released in 2018. They use an
>   	  Intel Apollo Lake SoC. The design supports WiFi, 4GB to 16GB of
> diff --git a/board/keymile/Kconfig b/board/keymile/Kconfig
> index e5d7c80a869d..bf899d005c46 100644
> --- a/board/keymile/Kconfig
> +++ b/board/keymile/Kconfig
> @@ -124,6 +124,7 @@ config SYS_IVM_EEPROM_PAGE_LEN
>
>   config PG_WCOM_UBOOT_UPDATE_SUPPORTED
>   	bool "Enable U-boot Field Fail-Safe Update Functionality"
> +	select EVENT
>   	default n
>   	help
>   	  Indicates that field fail-safe u-boot update is supported.
> diff --git a/boot/Kconfig b/boot/Kconfig
> index 30bc182fcd5c..daa01a10ebdc 100644
> --- a/boot/Kconfig
> +++ b/boot/Kconfig
> @@ -474,6 +474,7 @@ config BOOTMETH_VBE
>   	depends on FIT
>   	default y
>   	select BOOTMETH_GLOBAL
> +	select EVENT
>   	help
>   	  Enables support for VBE boot. This is a standard boot method which
>   	  supports selection of various firmware components, seleciton of an OS to
> @@ -482,6 +483,7 @@ config BOOTMETH_VBE
>   config SPL_BOOTMETH_VBE
>   	bool "Bootdev support for Verified Boot for Embedded (SPL)"
>   	depends on SPL && FIT
> +	select EVENT
>   	default y if VPL
>   	help
>   	  Enables support for VBE boot. This is a standard boot method which
> @@ -491,6 +493,7 @@ config SPL_BOOTMETH_VBE
>   config VPL_BOOTMETH_VBE
>   	bool "Bootdev support for Verified Boot for Embedded (VPL)"
>   	depends on VPL && FIT
> +	select EVENT
>   	default y
>   	help
>   	  Enables support for VBE boot. This is a standard boot method which
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index b2aefae9cb70..4fe2c75de256 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -2622,6 +2622,7 @@ config CMD_DIAG
>
>   config CMD_EVENT
>   	bool "event - Show information about events"
> +	depends on EVENT
>   	default y if EVENT_DEBUG
>   	help
>   	  This enables the 'event' command which provides information about
> diff --git a/common/Kconfig b/common/Kconfig
> index 439b2198f605..1c9f4774ba7a 100644
> --- a/common/Kconfig
> +++ b/common/Kconfig
> @@ -604,24 +604,23 @@ config CYCLIC_MAX_CPU_TIME_US
>   endif # CYCLIC
>
>   config EVENT
> -	bool "General-purpose event-handling mechanism"
> -	default y if SANDBOX
> +	bool
>   	help
> -	  This enables sending and processing of events, to allow interested
> -	  parties to be alerted when something happens. This is an attempt to
> -	  stem the flow of weak functions, hooks, functions in board_f.c
> -	  and board_r.c and the Kconfig options below.
> +	  This adds a framework for general purpose sending and processing of
> +	  events, to allow interested parties to be alerted when something
> +	  happens. This is an attempt to stem 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
> +	bool
>   	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
> +	  it to the EVENT_SPY() linker list. This increases code size slightly
>   	  but provides more flexibility for boards and subsystems that need it.

The long text does not cover register_event() which is also a reason to
select this setting.

Best regards

Heinrich

>
>   config EVENT_DEBUG
> diff --git a/drivers/core/Kconfig b/drivers/core/Kconfig
> index 8fde77c23ee0..6fc8854b574b 100644
> --- a/drivers/core/Kconfig
> +++ b/drivers/core/Kconfig
> @@ -109,13 +109,14 @@ config DM_DEVICE_REMOVE
>   	  causes USB host controllers to not be stopped when booting the OS.
>
>   config DM_EVENT
> -	bool "Support events with driver model"
> -	depends on DM && EVENT
> -	default y if SANDBOX
> +	bool
> +	depends on DM
> +	select EVENT
>   	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.
> +	  register a 'spy' function that is called when the event occurs. Such
> +	  subsystems must select this option.
>
>   config SPL_DM_DEVICE_REMOVE
>   	bool "Support device removal in SPL"
> diff --git a/drivers/cpu/Kconfig b/drivers/cpu/Kconfig
> index 21874335c873..3bf04105e5e9 100644
> --- a/drivers/cpu/Kconfig
> +++ b/drivers/cpu/Kconfig
> @@ -23,7 +23,6 @@ config CPU_RISCV
>   config CPU_MICROBLAZE
>   	bool "Enable Microblaze CPU driver"
>   	depends on CPU && MICROBLAZE
> -	select EVENT
>   	select DM_EVENT
>   	select XILINX_MICROBLAZE0_PVR
>   	help
> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> index b498c72206fd..b630d88ef9e2 100644
> --- a/lib/efi_loader/Kconfig
> +++ b/lib/efi_loader/Kconfig
> @@ -14,8 +14,8 @@ config EFI_LOADER
>   	depends on !EFI_APP
>   	default y if !ARM || SYS_CPU = armv7 || SYS_CPU = armv8
>   	select CHARSET
> +	# We need to send DM events, dynamically, in the EFI block driver
>   	select DM_EVENT
> -	select EVENT
>   	select EVENT_DYNAMIC
>   	select LIB_UUID
>   	imply PARTITION_UUIDS


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [v3 2/2] event: Correct dependencies on the EVENT framework
  2023-01-16 21:01   ` Heinrich Schuchardt
@ 2023-01-16 21:04     ` Tom Rini
  2023-01-20 22:11       ` Simon Glass
  0 siblings, 1 reply; 20+ messages in thread
From: Tom Rini @ 2023-01-16 21:04 UTC (permalink / raw)
  To: Heinrich Schuchardt
  Cc: AKASHI Takahiro, Oliver Graute, Francesco Dolcini, Simon Glass,
	Fabio Estevam, u-boot

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

On Mon, Jan 16, 2023 at 10:01:47PM +0100, Heinrich Schuchardt wrote:
> On 1/16/23 21:46, Tom Rini wrote:
> > The event framework is just that, a framework. Enabling it by itself
> > does nothing, so we shouldn't ask the user about it. Reword (and correct
> > typos) around this the option and help text. This also applies to
> > DM_EVENT and EVENT_DYNAMIC. Only EVENT_DEBUG and CMD_EVENT should be
> > visible to the user to select, when EVENT is selected.
> > 
> > With this, it's time to address the larger problems. When functionality
> > uses events, typically via EVENT_SPY, the appropriate framework then
> > must be select'd and NOT imply'd. As the functionality will cease to
> > work (and so, platforms will fail to boot) this is non-optional and
> > where select is appropriate. Audit the current users of EVENT_SPY to
> > have a more fine-grained approach to select'ing the framework where
> > used. Also ensure the current users of event_register and also select
> > EVENT_DYNAMIC.
> > 
> > Cc: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> > Reported-by: Oliver Graute <Oliver.Graute@kococonnector.com>
> > Reported-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> > Fixes: 7fe32b3442f0 ("event: Convert arch_cpu_init_dm() to use events")
> > Fixes: 42fdcebf859f ("event: Convert misc_init_f() to use events")
> > Fixes: c5ef2025579e ("dm: fix DM_EVENT dependencies")
> > Signed-off-by: Tom Rini <trini@konsulko.com>
> > Tested-by: Simon Glass <sjg@chromium.org>
> > Reviewed-by: Simon Glass <sjg@chromium.org>
> > Reviewed-by: Fabio Estevam <festevam@denx.de>
> > ---
> > Changes in v3:
> > - Drop guards around when EFI_LOADER selects DM_EVENT / EVENT_DYNAMIC,
> >    keep comment.
> > 
> > Changes in v2:
> > - Collect tags
> > - Reword the commit message a little, reword and comment on the
> >    EFI_LOADER case.
> > - Add the rest of the VBE cases I had forgotten.
> > ---
> >   arch/Kconfig                     |  6 +++---
> >   arch/arm/Kconfig                 |  9 ++++-----
> >   arch/arm/mach-omap2/Kconfig      |  3 +++
> >   arch/mips/Kconfig                |  2 +-
> >   arch/powerpc/cpu/mpc85xx/Kconfig |  1 +
> >   arch/x86/Kconfig                 |  1 +
> >   arch/x86/cpu/baytrail/Kconfig    |  1 +
> >   arch/x86/cpu/broadwell/Kconfig   |  1 +
> >   arch/x86/cpu/ivybridge/Kconfig   |  1 +
> >   arch/x86/cpu/quark/Kconfig       |  1 +
> >   board/google/Kconfig             |  1 +
> >   board/keymile/Kconfig            |  1 +
> >   boot/Kconfig                     |  3 +++
> >   cmd/Kconfig                      |  1 +
> >   common/Kconfig                   | 17 ++++++++---------
> >   drivers/core/Kconfig             |  9 +++++----
> >   drivers/cpu/Kconfig              |  1 -
> >   lib/efi_loader/Kconfig           |  2 +-
> >   18 files changed, 37 insertions(+), 24 deletions(-)
> > 
> > diff --git a/arch/Kconfig b/arch/Kconfig
> > index 8fb87b7d857c..d30676ae817b 100644
> > --- a/arch/Kconfig
> > +++ b/arch/Kconfig
> > @@ -93,7 +93,7 @@ config NIOS2
> >   	bool "Nios II architecture"
> >   	select CPU
> >   	select DM
> > -	imply DM_EVENT
> > +	select DM_EVENT
> >   	select OF_CONTROL
> >   	select SUPPORT_OF_CONTROL
> >   	imply CMD_DM
> > @@ -111,9 +111,9 @@ config RISCV
> >   	select SUPPORT_OF_CONTROL
> >   	select OF_CONTROL
> >   	select DM
> > +	select DM_EVENT
> >   	imply SPL_SEPARATE_BSS if SPL
> >   	imply DM_SERIAL
> > -	imply DM_EVENT
> >   	imply DM_MMC
> >   	imply DM_SPI
> >   	imply DM_SPI_FLASH
> > @@ -136,6 +136,7 @@ config SANDBOX
> >   	select BZIP2
> >   	select CMD_POWEROFF
> >   	select DM
> > +	select DM_EVENT
> >   	select DM_FUZZING_ENGINE
> >   	select DM_GPIO
> >   	select DM_I2C
> > @@ -240,7 +241,6 @@ config X86
> >   	imply CMD_SF
> >   	imply CMD_SF_TEST
> >   	imply CMD_ZBOOT
> > -	imply DM_EVENT
> >   	imply DM_GPIO
> >   	imply DM_KEYBOARD
> >   	imply DM_MMC
> > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> > index bbf1d5227b3f..c9a44ebc2213 100644
> > --- a/arch/arm/Kconfig
> > +++ b/arch/arm/Kconfig
> > @@ -778,7 +778,6 @@ config ARCH_OMAP2PLUS
> >   	select SUPPORT_SPL
> >   	imply TI_SYSC if DM && OF_CONTROL
> >   	imply FIT
> > -	imply DM_EVENT
> >   	imply SPL_SEPARATE_BSS
> > 
> >   config ARCH_MESON
> > @@ -823,11 +822,11 @@ config ARCH_IMX8
> >   	select SYS_FSL_SEC_COMPAT_4
> >   	select SYS_FSL_SEC_LE
> >   	select DM
> > +	select DM_EVENT
> >   	select GPIO_EXTRA_HEADER
> >   	select MACH_IMX
> >   	select OF_CONTROL
> >   	select ENABLE_ARM_SOC_BOOT0_HOOK
> > -	imply DM_EVENT
> > 
> >   config ARCH_IMX8M
> >   	bool "NXP i.MX8M platform"
> > @@ -839,14 +838,15 @@ config ARCH_IMX8M
> >   	select SYS_FSL_SEC_LE
> >   	select SYS_I2C_MXC
> >   	select DM
> > +	select DM_EVENT if CLK
> >   	select SUPPORT_SPL
> >   	imply CMD_DM
> > -	imply DM_EVENT
> > 
> >   config ARCH_IMX8ULP
> >   	bool "NXP i.MX8ULP platform"
> >   	select ARM64
> >   	select DM
> > +	select DM_EVENT
> >   	select MACH_IMX
> >   	select OF_CONTROL
> >   	select SUPPORT_SPL
> > @@ -854,19 +854,18 @@ config ARCH_IMX8ULP
> >   	select MISC
> >   	select IMX_SENTINEL
> >   	imply CMD_DM
> > -	imply DM_EVENT
> > 
> >   config ARCH_IMX9
> >   	bool "NXP i.MX9 platform"
> >   	select ARM64
> >   	select DM
> > +	select DM_EVENT
> >   	select MACH_IMX
> >   	select SUPPORT_SPL
> >   	select GPIO_EXTRA_HEADER
> >   	select MISC
> >   	select IMX_SENTINEL
> >   	imply CMD_DM
> > -	imply DM_EVENT
> > 
> >   config ARCH_IMXRT
> >   	bool "NXP i.MXRT platform"
> > diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
> > index 1db71df27212..309b967b0dd5 100644
> > --- a/arch/arm/mach-omap2/Kconfig
> > +++ b/arch/arm/mach-omap2/Kconfig
> > @@ -31,6 +31,7 @@ config OMAP34XX
> > 
> >   config OMAP44XX
> >   	bool "OMAP44XX SoC"
> > +	select DM_EVENT
> >   	select SPL_USE_TINY_PRINTF
> >   	select SPL_SYS_NO_VECTOR_TABLE if SPL
> >   	imply NAND_OMAP_ELM
> > @@ -55,6 +56,7 @@ config OMAP54XX
> >   	bool "OMAP54XX SoC"
> >   	select ARM_CORTEX_A15_CVE_2017_5715
> >   	select ARM_ERRATA_798870
> > +	select DM_EVENT
> >   	select SYS_THUMB_BUILD
> >   	imply NAND_OMAP_ELM
> >   	imply NAND_OMAP_GPMC
> > @@ -111,6 +113,7 @@ config AM43XX
> >   config AM33XX
> >   	bool "AM33XX SoC"
> >   	select ARM_CORTEX_A8_CVE_2017_5715
> > +	select DM_EVENT
> >   	select SPECIFY_CONSOLE_INDEX
> >   	imply NAND_OMAP_ELM
> >   	imply NAND_OMAP_GPMC
> > diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
> > index 23142bd2700d..569f5f48bc6c 100644
> > --- a/arch/mips/Kconfig
> > +++ b/arch/mips/Kconfig
> > @@ -121,6 +121,7 @@ config MACH_PIC32
> >   	bool "Support Microchip PIC32"
> >   	select HAS_FIXED_TIMER_FREQUENCY
> >   	select DM
> > +	select DM_EVENT
> >   	select OF_CONTROL
> >   	imply CMD_DM
> > 
> > @@ -128,7 +129,6 @@ config TARGET_BOSTON
> >   	bool "Support Boston"
> >   	select HAS_FIXED_TIMER_FREQUENCY
> >   	select DM
> > -	imply DM_EVENT
> >   	select DM_SERIAL
> >   	select MIPS_CM
> >   	select SYS_CACHE_SHIFT_6
> > diff --git a/arch/powerpc/cpu/mpc85xx/Kconfig b/arch/powerpc/cpu/mpc85xx/Kconfig
> > index 1b180481a483..2c54a9e2120f 100644
> > --- a/arch/powerpc/cpu/mpc85xx/Kconfig
> > +++ b/arch/powerpc/cpu/mpc85xx/Kconfig
> > @@ -248,6 +248,7 @@ config TARGET_KMP204X
> >   config TARGET_KMCENT2
> >   	bool "Support kmcent2"
> >   	select VENDOR_KM
> > +	select EVENT
> >   	select FSL_CORENET
> >   	select SYS_DPAA_FMAN
> >   	select SYS_DPAA_PME
> > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> > index 93f1c77be3f3..07be5cd05ec0 100644
> > --- a/arch/x86/Kconfig
> > +++ b/arch/x86/Kconfig
> > @@ -395,6 +395,7 @@ config FSP_VERSION1
> > 
> >   config FSP_VERSION2
> >   	bool "FSP version 2.x"
> > +	select DM_EVENT
> >   	help
> >   	  This covers versions 2.0 and 2.1. See here for details:
> >   	  https://github.com/IntelFsp/fsp/wiki
> > diff --git a/arch/x86/cpu/baytrail/Kconfig b/arch/x86/cpu/baytrail/Kconfig
> > index d2c3473d6abf..a8efea8a3413 100644
> > --- a/arch/x86/cpu/baytrail/Kconfig
> > +++ b/arch/x86/cpu/baytrail/Kconfig
> > @@ -7,6 +7,7 @@ config INTEL_BAYTRAIL
> >   	select HAVE_FSP
> >   	select ARCH_MISC_INIT
> >   	select CPU_INTEL_TURBO_NOT_PACKAGE_SCOPED
> > +	select DM_EVENT
> >   	imply HAVE_INTEL_ME
> >   	imply ENABLE_MRC_CACHE
> >   	imply AHCI_PCI
> > diff --git a/arch/x86/cpu/broadwell/Kconfig b/arch/x86/cpu/broadwell/Kconfig
> > index 5b015c89d950..39deda364479 100644
> > --- a/arch/x86/cpu/broadwell/Kconfig
> > +++ b/arch/x86/cpu/broadwell/Kconfig
> > @@ -6,6 +6,7 @@
> >   config INTEL_BROADWELL
> >   	bool
> >   	select CACHE_MRC_BIN
> > +	select DM_EVENT
> >   	select ARCH_EARLY_INIT_R
> >   	imply HAVE_INTEL_ME
> >   	imply ENABLE_MRC_CACHE
> > diff --git a/arch/x86/cpu/ivybridge/Kconfig b/arch/x86/cpu/ivybridge/Kconfig
> > index be3ef5e5d8f8..704f145adf88 100644
> > --- a/arch/x86/cpu/ivybridge/Kconfig
> > +++ b/arch/x86/cpu/ivybridge/Kconfig
> > @@ -8,6 +8,7 @@
> >   config NORTHBRIDGE_INTEL_IVYBRIDGE
> >   	bool
> >   	select CACHE_MRC_BIN if HAVE_MRC
> > +	select DM_EVENT
> >   	imply HAVE_INTEL_ME
> >   	imply ENABLE_MRC_CACHE
> >   	imply AHCI_PCI
> > diff --git a/arch/x86/cpu/quark/Kconfig b/arch/x86/cpu/quark/Kconfig
> > index 61bb5792c868..0d4008a31f4c 100644
> > --- a/arch/x86/cpu/quark/Kconfig
> > +++ b/arch/x86/cpu/quark/Kconfig
> > @@ -7,6 +7,7 @@ config INTEL_QUARK
> >   	select HAVE_RMU
> >   	select ARCH_EARLY_INIT_R
> >   	select ARCH_MISC_INIT
> > +	select DM_EVENT
> >   	imply ENABLE_MRC_CACHE
> >   	imply ETH_DESIGNWARE
> >   	imply ICH_SPI
> > diff --git a/board/google/Kconfig b/board/google/Kconfig
> > index 0474b4e69384..a0f1a6097641 100644
> > --- a/board/google/Kconfig
> > +++ b/board/google/Kconfig
> > @@ -18,6 +18,7 @@ choice
> >   config TARGET_CHROMEBOOK_CORAL
> >   	bool "Chromebook coral"
> >   	select BIOSEMU
> > +	select EVENT
> >   	help
> >   	  This is a range of Intel-based laptops released in 2018. They use an
> >   	  Intel Apollo Lake SoC. The design supports WiFi, 4GB to 16GB of
> > diff --git a/board/keymile/Kconfig b/board/keymile/Kconfig
> > index e5d7c80a869d..bf899d005c46 100644
> > --- a/board/keymile/Kconfig
> > +++ b/board/keymile/Kconfig
> > @@ -124,6 +124,7 @@ config SYS_IVM_EEPROM_PAGE_LEN
> > 
> >   config PG_WCOM_UBOOT_UPDATE_SUPPORTED
> >   	bool "Enable U-boot Field Fail-Safe Update Functionality"
> > +	select EVENT
> >   	default n
> >   	help
> >   	  Indicates that field fail-safe u-boot update is supported.
> > diff --git a/boot/Kconfig b/boot/Kconfig
> > index 30bc182fcd5c..daa01a10ebdc 100644
> > --- a/boot/Kconfig
> > +++ b/boot/Kconfig
> > @@ -474,6 +474,7 @@ config BOOTMETH_VBE
> >   	depends on FIT
> >   	default y
> >   	select BOOTMETH_GLOBAL
> > +	select EVENT
> >   	help
> >   	  Enables support for VBE boot. This is a standard boot method which
> >   	  supports selection of various firmware components, seleciton of an OS to
> > @@ -482,6 +483,7 @@ config BOOTMETH_VBE
> >   config SPL_BOOTMETH_VBE
> >   	bool "Bootdev support for Verified Boot for Embedded (SPL)"
> >   	depends on SPL && FIT
> > +	select EVENT
> >   	default y if VPL
> >   	help
> >   	  Enables support for VBE boot. This is a standard boot method which
> > @@ -491,6 +493,7 @@ config SPL_BOOTMETH_VBE
> >   config VPL_BOOTMETH_VBE
> >   	bool "Bootdev support for Verified Boot for Embedded (VPL)"
> >   	depends on VPL && FIT
> > +	select EVENT
> >   	default y
> >   	help
> >   	  Enables support for VBE boot. This is a standard boot method which
> > diff --git a/cmd/Kconfig b/cmd/Kconfig
> > index b2aefae9cb70..4fe2c75de256 100644
> > --- a/cmd/Kconfig
> > +++ b/cmd/Kconfig
> > @@ -2622,6 +2622,7 @@ config CMD_DIAG
> > 
> >   config CMD_EVENT
> >   	bool "event - Show information about events"
> > +	depends on EVENT
> >   	default y if EVENT_DEBUG
> >   	help
> >   	  This enables the 'event' command which provides information about
> > diff --git a/common/Kconfig b/common/Kconfig
> > index 439b2198f605..1c9f4774ba7a 100644
> > --- a/common/Kconfig
> > +++ b/common/Kconfig
> > @@ -604,24 +604,23 @@ config CYCLIC_MAX_CPU_TIME_US
> >   endif # CYCLIC
> > 
> >   config EVENT
> > -	bool "General-purpose event-handling mechanism"
> > -	default y if SANDBOX
> > +	bool
> >   	help
> > -	  This enables sending and processing of events, to allow interested
> > -	  parties to be alerted when something happens. This is an attempt to
> > -	  stem the flow of weak functions, hooks, functions in board_f.c
> > -	  and board_r.c and the Kconfig options below.
> > +	  This adds a framework for general purpose sending and processing of
> > +	  events, to allow interested parties to be alerted when something
> > +	  happens. This is an attempt to stem 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
> > +	bool
> >   	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
> > +	  it to the EVENT_SPY() linker list. This increases code size slightly
> >   	  but provides more flexibility for boards and subsystems that need it.
> 
> The long text does not cover register_event() which is also a reason to
> select this setting.

event_register also isn't in doc/develop/event.rst so a follow-up to
expand on this would be good, yes, as the help text should generally
point towards the rST docs, especially for non-user options.

-- 
Tom

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] x86: Fix saying arch_cpu_init_dm in debug/docs
  2023-01-14 20:49 [PATCH 1/2] x86: Fix saying arch_cpu_init_dm in debug/docs Tom Rini
                   ` (2 preceding siblings ...)
  2023-01-16 20:46 ` [v3 2/2] event: Correct dependencies on the EVENT framework Tom Rini
@ 2023-01-19 14:46 ` Tom Rini
  3 siblings, 0 replies; 20+ messages in thread
From: Tom Rini @ 2023-01-19 14:46 UTC (permalink / raw)
  To: u-boot; +Cc: Simon Glass

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

On Sat, Jan 14, 2023 at 03:49:35PM -0500, Tom Rini wrote:

> The function arch_cpu_init_dm was renamed to fsp_setup_pinctrl in these
> cases, so rename debug / docs to match.
> 
> Cc: Simon Glass <sjg@chromium.org>
> Fixes: 7fe32b3442f0 ("event: Convert arch_cpu_init_dm() to use events")
> Signed-off-by: Tom Rini <trini@konsulko.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [v3 2/2] event: Correct dependencies on the EVENT framework
  2023-01-16 20:46 ` [v3 2/2] event: Correct dependencies on the EVENT framework Tom Rini
  2023-01-16 21:01   ` Heinrich Schuchardt
@ 2023-01-19 14:46   ` Tom Rini
  1 sibling, 0 replies; 20+ messages in thread
From: Tom Rini @ 2023-01-19 14:46 UTC (permalink / raw)
  To: u-boot
  Cc: AKASHI Takahiro, Heinrich Schuchardt, Oliver Graute,
	Francesco Dolcini, Simon Glass, Fabio Estevam

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

On Mon, Jan 16, 2023 at 03:46:49PM -0500, Tom Rini wrote:

> The event framework is just that, a framework. Enabling it by itself
> does nothing, so we shouldn't ask the user about it. Reword (and correct
> typos) around this the option and help text. This also applies to
> DM_EVENT and EVENT_DYNAMIC. Only EVENT_DEBUG and CMD_EVENT should be
> visible to the user to select, when EVENT is selected.
> 
> With this, it's time to address the larger problems. When functionality
> uses events, typically via EVENT_SPY, the appropriate framework then
> must be select'd and NOT imply'd. As the functionality will cease to
> work (and so, platforms will fail to boot) this is non-optional and
> where select is appropriate. Audit the current users of EVENT_SPY to
> have a more fine-grained approach to select'ing the framework where
> used. Also ensure the current users of event_register and also select
> EVENT_DYNAMIC.
> 
> Cc: AKASHI Takahiro <takahiro.akashi@linaro.org>
> Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Reported-by: Oliver Graute <Oliver.Graute@kococonnector.com>
> Reported-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> Fixes: 7fe32b3442f0 ("event: Convert arch_cpu_init_dm() to use events")
> Fixes: 42fdcebf859f ("event: Convert misc_init_f() to use events")
> Fixes: c5ef2025579e ("dm: fix DM_EVENT dependencies")
> Signed-off-by: Tom Rini <trini@konsulko.com>
> Tested-by: Simon Glass <sjg@chromium.org>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> Reviewed-by: Fabio Estevam <festevam@denx.de>

Applied to u-boot/master, thanks!

-- 
Tom

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [v3 2/2] event: Correct dependencies on the EVENT framework
  2023-01-16 21:04     ` Tom Rini
@ 2023-01-20 22:11       ` Simon Glass
  0 siblings, 0 replies; 20+ messages in thread
From: Simon Glass @ 2023-01-20 22:11 UTC (permalink / raw)
  To: Tom Rini
  Cc: Heinrich Schuchardt, AKASHI Takahiro, Oliver Graute,
	Francesco Dolcini, Fabio Estevam, u-boot

Hi Tom,

On Mon, 16 Jan 2023 at 14:04, Tom Rini <trini@konsulko.com> wrote:
>
> On Mon, Jan 16, 2023 at 10:01:47PM +0100, Heinrich Schuchardt wrote:
> > On 1/16/23 21:46, Tom Rini wrote:
> > > The event framework is just that, a framework. Enabling it by itself
> > > does nothing, so we shouldn't ask the user about it. Reword (and correct
> > > typos) around this the option and help text. This also applies to
> > > DM_EVENT and EVENT_DYNAMIC. Only EVENT_DEBUG and CMD_EVENT should be
> > > visible to the user to select, when EVENT is selected.
> > >
> > > With this, it's time to address the larger problems. When functionality
> > > uses events, typically via EVENT_SPY, the appropriate framework then
> > > must be select'd and NOT imply'd. As the functionality will cease to
> > > work (and so, platforms will fail to boot) this is non-optional and
> > > where select is appropriate. Audit the current users of EVENT_SPY to
> > > have a more fine-grained approach to select'ing the framework where
> > > used. Also ensure the current users of event_register and also select
> > > EVENT_DYNAMIC.
> > >
> > > Cc: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > > Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> > > Reported-by: Oliver Graute <Oliver.Graute@kococonnector.com>
> > > Reported-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> > > Fixes: 7fe32b3442f0 ("event: Convert arch_cpu_init_dm() to use events")
> > > Fixes: 42fdcebf859f ("event: Convert misc_init_f() to use events")
> > > Fixes: c5ef2025579e ("dm: fix DM_EVENT dependencies")
> > > Signed-off-by: Tom Rini <trini@konsulko.com>
> > > Tested-by: Simon Glass <sjg@chromium.org>
> > > Reviewed-by: Simon Glass <sjg@chromium.org>
> > > Reviewed-by: Fabio Estevam <festevam@denx.de>
> > > ---
> > > Changes in v3:
> > > - Drop guards around when EFI_LOADER selects DM_EVENT / EVENT_DYNAMIC,
> > >    keep comment.
> > >
> > > Changes in v2:
> > > - Collect tags
> > > - Reword the commit message a little, reword and comment on the
> > >    EFI_LOADER case.
> > > - Add the rest of the VBE cases I had forgotten.
> > > ---
> > >   arch/Kconfig                     |  6 +++---
> > >   arch/arm/Kconfig                 |  9 ++++-----
> > >   arch/arm/mach-omap2/Kconfig      |  3 +++
> > >   arch/mips/Kconfig                |  2 +-
> > >   arch/powerpc/cpu/mpc85xx/Kconfig |  1 +
> > >   arch/x86/Kconfig                 |  1 +
> > >   arch/x86/cpu/baytrail/Kconfig    |  1 +
> > >   arch/x86/cpu/broadwell/Kconfig   |  1 +
> > >   arch/x86/cpu/ivybridge/Kconfig   |  1 +
> > >   arch/x86/cpu/quark/Kconfig       |  1 +
> > >   board/google/Kconfig             |  1 +
> > >   board/keymile/Kconfig            |  1 +
> > >   boot/Kconfig                     |  3 +++
> > >   cmd/Kconfig                      |  1 +
> > >   common/Kconfig                   | 17 ++++++++---------
> > >   drivers/core/Kconfig             |  9 +++++----
> > >   drivers/cpu/Kconfig              |  1 -
> > >   lib/efi_loader/Kconfig           |  2 +-
> > >   18 files changed, 37 insertions(+), 24 deletions(-)
> > >
> > > diff --git a/arch/Kconfig b/arch/Kconfig
> > > index 8fb87b7d857c..d30676ae817b 100644
> > > --- a/arch/Kconfig
> > > +++ b/arch/Kconfig
> > > @@ -93,7 +93,7 @@ config NIOS2
> > >     bool "Nios II architecture"
> > >     select CPU
> > >     select DM
> > > -   imply DM_EVENT
> > > +   select DM_EVENT
> > >     select OF_CONTROL
> > >     select SUPPORT_OF_CONTROL
> > >     imply CMD_DM
> > > @@ -111,9 +111,9 @@ config RISCV
> > >     select SUPPORT_OF_CONTROL
> > >     select OF_CONTROL
> > >     select DM
> > > +   select DM_EVENT
> > >     imply SPL_SEPARATE_BSS if SPL
> > >     imply DM_SERIAL
> > > -   imply DM_EVENT
> > >     imply DM_MMC
> > >     imply DM_SPI
> > >     imply DM_SPI_FLASH
> > > @@ -136,6 +136,7 @@ config SANDBOX
> > >     select BZIP2
> > >     select CMD_POWEROFF
> > >     select DM
> > > +   select DM_EVENT
> > >     select DM_FUZZING_ENGINE
> > >     select DM_GPIO
> > >     select DM_I2C
> > > @@ -240,7 +241,6 @@ config X86
> > >     imply CMD_SF
> > >     imply CMD_SF_TEST
> > >     imply CMD_ZBOOT
> > > -   imply DM_EVENT
> > >     imply DM_GPIO
> > >     imply DM_KEYBOARD
> > >     imply DM_MMC
> > > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> > > index bbf1d5227b3f..c9a44ebc2213 100644
> > > --- a/arch/arm/Kconfig
> > > +++ b/arch/arm/Kconfig
> > > @@ -778,7 +778,6 @@ config ARCH_OMAP2PLUS
> > >     select SUPPORT_SPL
> > >     imply TI_SYSC if DM && OF_CONTROL
> > >     imply FIT
> > > -   imply DM_EVENT
> > >     imply SPL_SEPARATE_BSS
> > >
> > >   config ARCH_MESON
> > > @@ -823,11 +822,11 @@ config ARCH_IMX8
> > >     select SYS_FSL_SEC_COMPAT_4
> > >     select SYS_FSL_SEC_LE
> > >     select DM
> > > +   select DM_EVENT
> > >     select GPIO_EXTRA_HEADER
> > >     select MACH_IMX
> > >     select OF_CONTROL
> > >     select ENABLE_ARM_SOC_BOOT0_HOOK
> > > -   imply DM_EVENT
> > >
> > >   config ARCH_IMX8M
> > >     bool "NXP i.MX8M platform"
> > > @@ -839,14 +838,15 @@ config ARCH_IMX8M
> > >     select SYS_FSL_SEC_LE
> > >     select SYS_I2C_MXC
> > >     select DM
> > > +   select DM_EVENT if CLK
> > >     select SUPPORT_SPL
> > >     imply CMD_DM
> > > -   imply DM_EVENT
> > >
> > >   config ARCH_IMX8ULP
> > >     bool "NXP i.MX8ULP platform"
> > >     select ARM64
> > >     select DM
> > > +   select DM_EVENT
> > >     select MACH_IMX
> > >     select OF_CONTROL
> > >     select SUPPORT_SPL
> > > @@ -854,19 +854,18 @@ config ARCH_IMX8ULP
> > >     select MISC
> > >     select IMX_SENTINEL
> > >     imply CMD_DM
> > > -   imply DM_EVENT
> > >
> > >   config ARCH_IMX9
> > >     bool "NXP i.MX9 platform"
> > >     select ARM64
> > >     select DM
> > > +   select DM_EVENT
> > >     select MACH_IMX
> > >     select SUPPORT_SPL
> > >     select GPIO_EXTRA_HEADER
> > >     select MISC
> > >     select IMX_SENTINEL
> > >     imply CMD_DM
> > > -   imply DM_EVENT
> > >
> > >   config ARCH_IMXRT
> > >     bool "NXP i.MXRT platform"
> > > diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
> > > index 1db71df27212..309b967b0dd5 100644
> > > --- a/arch/arm/mach-omap2/Kconfig
> > > +++ b/arch/arm/mach-omap2/Kconfig
> > > @@ -31,6 +31,7 @@ config OMAP34XX
> > >
> > >   config OMAP44XX
> > >     bool "OMAP44XX SoC"
> > > +   select DM_EVENT
> > >     select SPL_USE_TINY_PRINTF
> > >     select SPL_SYS_NO_VECTOR_TABLE if SPL
> > >     imply NAND_OMAP_ELM
> > > @@ -55,6 +56,7 @@ config OMAP54XX
> > >     bool "OMAP54XX SoC"
> > >     select ARM_CORTEX_A15_CVE_2017_5715
> > >     select ARM_ERRATA_798870
> > > +   select DM_EVENT
> > >     select SYS_THUMB_BUILD
> > >     imply NAND_OMAP_ELM
> > >     imply NAND_OMAP_GPMC
> > > @@ -111,6 +113,7 @@ config AM43XX
> > >   config AM33XX
> > >     bool "AM33XX SoC"
> > >     select ARM_CORTEX_A8_CVE_2017_5715
> > > +   select DM_EVENT
> > >     select SPECIFY_CONSOLE_INDEX
> > >     imply NAND_OMAP_ELM
> > >     imply NAND_OMAP_GPMC
> > > diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
> > > index 23142bd2700d..569f5f48bc6c 100644
> > > --- a/arch/mips/Kconfig
> > > +++ b/arch/mips/Kconfig
> > > @@ -121,6 +121,7 @@ config MACH_PIC32
> > >     bool "Support Microchip PIC32"
> > >     select HAS_FIXED_TIMER_FREQUENCY
> > >     select DM
> > > +   select DM_EVENT
> > >     select OF_CONTROL
> > >     imply CMD_DM
> > >
> > > @@ -128,7 +129,6 @@ config TARGET_BOSTON
> > >     bool "Support Boston"
> > >     select HAS_FIXED_TIMER_FREQUENCY
> > >     select DM
> > > -   imply DM_EVENT
> > >     select DM_SERIAL
> > >     select MIPS_CM
> > >     select SYS_CACHE_SHIFT_6
> > > diff --git a/arch/powerpc/cpu/mpc85xx/Kconfig b/arch/powerpc/cpu/mpc85xx/Kconfig
> > > index 1b180481a483..2c54a9e2120f 100644
> > > --- a/arch/powerpc/cpu/mpc85xx/Kconfig
> > > +++ b/arch/powerpc/cpu/mpc85xx/Kconfig
> > > @@ -248,6 +248,7 @@ config TARGET_KMP204X
> > >   config TARGET_KMCENT2
> > >     bool "Support kmcent2"
> > >     select VENDOR_KM
> > > +   select EVENT
> > >     select FSL_CORENET
> > >     select SYS_DPAA_FMAN
> > >     select SYS_DPAA_PME
> > > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> > > index 93f1c77be3f3..07be5cd05ec0 100644
> > > --- a/arch/x86/Kconfig
> > > +++ b/arch/x86/Kconfig
> > > @@ -395,6 +395,7 @@ config FSP_VERSION1
> > >
> > >   config FSP_VERSION2
> > >     bool "FSP version 2.x"
> > > +   select DM_EVENT
> > >     help
> > >       This covers versions 2.0 and 2.1. See here for details:
> > >       https://github.com/IntelFsp/fsp/wiki
> > > diff --git a/arch/x86/cpu/baytrail/Kconfig b/arch/x86/cpu/baytrail/Kconfig
> > > index d2c3473d6abf..a8efea8a3413 100644
> > > --- a/arch/x86/cpu/baytrail/Kconfig
> > > +++ b/arch/x86/cpu/baytrail/Kconfig
> > > @@ -7,6 +7,7 @@ config INTEL_BAYTRAIL
> > >     select HAVE_FSP
> > >     select ARCH_MISC_INIT
> > >     select CPU_INTEL_TURBO_NOT_PACKAGE_SCOPED
> > > +   select DM_EVENT
> > >     imply HAVE_INTEL_ME
> > >     imply ENABLE_MRC_CACHE
> > >     imply AHCI_PCI
> > > diff --git a/arch/x86/cpu/broadwell/Kconfig b/arch/x86/cpu/broadwell/Kconfig
> > > index 5b015c89d950..39deda364479 100644
> > > --- a/arch/x86/cpu/broadwell/Kconfig
> > > +++ b/arch/x86/cpu/broadwell/Kconfig
> > > @@ -6,6 +6,7 @@
> > >   config INTEL_BROADWELL
> > >     bool
> > >     select CACHE_MRC_BIN
> > > +   select DM_EVENT
> > >     select ARCH_EARLY_INIT_R
> > >     imply HAVE_INTEL_ME
> > >     imply ENABLE_MRC_CACHE
> > > diff --git a/arch/x86/cpu/ivybridge/Kconfig b/arch/x86/cpu/ivybridge/Kconfig
> > > index be3ef5e5d8f8..704f145adf88 100644
> > > --- a/arch/x86/cpu/ivybridge/Kconfig
> > > +++ b/arch/x86/cpu/ivybridge/Kconfig
> > > @@ -8,6 +8,7 @@
> > >   config NORTHBRIDGE_INTEL_IVYBRIDGE
> > >     bool
> > >     select CACHE_MRC_BIN if HAVE_MRC
> > > +   select DM_EVENT
> > >     imply HAVE_INTEL_ME
> > >     imply ENABLE_MRC_CACHE
> > >     imply AHCI_PCI
> > > diff --git a/arch/x86/cpu/quark/Kconfig b/arch/x86/cpu/quark/Kconfig
> > > index 61bb5792c868..0d4008a31f4c 100644
> > > --- a/arch/x86/cpu/quark/Kconfig
> > > +++ b/arch/x86/cpu/quark/Kconfig
> > > @@ -7,6 +7,7 @@ config INTEL_QUARK
> > >     select HAVE_RMU
> > >     select ARCH_EARLY_INIT_R
> > >     select ARCH_MISC_INIT
> > > +   select DM_EVENT
> > >     imply ENABLE_MRC_CACHE
> > >     imply ETH_DESIGNWARE
> > >     imply ICH_SPI
> > > diff --git a/board/google/Kconfig b/board/google/Kconfig
> > > index 0474b4e69384..a0f1a6097641 100644
> > > --- a/board/google/Kconfig
> > > +++ b/board/google/Kconfig
> > > @@ -18,6 +18,7 @@ choice
> > >   config TARGET_CHROMEBOOK_CORAL
> > >     bool "Chromebook coral"
> > >     select BIOSEMU
> > > +   select EVENT
> > >     help
> > >       This is a range of Intel-based laptops released in 2018. They use an
> > >       Intel Apollo Lake SoC. The design supports WiFi, 4GB to 16GB of
> > > diff --git a/board/keymile/Kconfig b/board/keymile/Kconfig
> > > index e5d7c80a869d..bf899d005c46 100644
> > > --- a/board/keymile/Kconfig
> > > +++ b/board/keymile/Kconfig
> > > @@ -124,6 +124,7 @@ config SYS_IVM_EEPROM_PAGE_LEN
> > >
> > >   config PG_WCOM_UBOOT_UPDATE_SUPPORTED
> > >     bool "Enable U-boot Field Fail-Safe Update Functionality"
> > > +   select EVENT
> > >     default n
> > >     help
> > >       Indicates that field fail-safe u-boot update is supported.
> > > diff --git a/boot/Kconfig b/boot/Kconfig
> > > index 30bc182fcd5c..daa01a10ebdc 100644
> > > --- a/boot/Kconfig
> > > +++ b/boot/Kconfig
> > > @@ -474,6 +474,7 @@ config BOOTMETH_VBE
> > >     depends on FIT
> > >     default y
> > >     select BOOTMETH_GLOBAL
> > > +   select EVENT
> > >     help
> > >       Enables support for VBE boot. This is a standard boot method which
> > >       supports selection of various firmware components, seleciton of an OS to
> > > @@ -482,6 +483,7 @@ config BOOTMETH_VBE
> > >   config SPL_BOOTMETH_VBE
> > >     bool "Bootdev support for Verified Boot for Embedded (SPL)"
> > >     depends on SPL && FIT
> > > +   select EVENT
> > >     default y if VPL
> > >     help
> > >       Enables support for VBE boot. This is a standard boot method which
> > > @@ -491,6 +493,7 @@ config SPL_BOOTMETH_VBE
> > >   config VPL_BOOTMETH_VBE
> > >     bool "Bootdev support for Verified Boot for Embedded (VPL)"
> > >     depends on VPL && FIT
> > > +   select EVENT
> > >     default y
> > >     help
> > >       Enables support for VBE boot. This is a standard boot method which
> > > diff --git a/cmd/Kconfig b/cmd/Kconfig
> > > index b2aefae9cb70..4fe2c75de256 100644
> > > --- a/cmd/Kconfig
> > > +++ b/cmd/Kconfig
> > > @@ -2622,6 +2622,7 @@ config CMD_DIAG
> > >
> > >   config CMD_EVENT
> > >     bool "event - Show information about events"
> > > +   depends on EVENT
> > >     default y if EVENT_DEBUG
> > >     help
> > >       This enables the 'event' command which provides information about
> > > diff --git a/common/Kconfig b/common/Kconfig
> > > index 439b2198f605..1c9f4774ba7a 100644
> > > --- a/common/Kconfig
> > > +++ b/common/Kconfig
> > > @@ -604,24 +604,23 @@ config CYCLIC_MAX_CPU_TIME_US
> > >   endif # CYCLIC
> > >
> > >   config EVENT
> > > -   bool "General-purpose event-handling mechanism"
> > > -   default y if SANDBOX
> > > +   bool
> > >     help
> > > -     This enables sending and processing of events, to allow interested
> > > -     parties to be alerted when something happens. This is an attempt to
> > > -     stem the flow of weak functions, hooks, functions in board_f.c
> > > -     and board_r.c and the Kconfig options below.
> > > +     This adds a framework for general purpose sending and processing of
> > > +     events, to allow interested parties to be alerted when something
> > > +     happens. This is an attempt to stem 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
> > > +   bool
> > >     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
> > > +     it to the EVENT_SPY() linker list. This increases code size slightly
> > >       but provides more flexibility for boards and subsystems that need it.
> >
> > The long text does not cover register_event() which is also a reason to
> > select this setting.
>
> event_register also isn't in doc/develop/event.rst so a follow-up to
> expand on this would be good, yes, as the help text should generally
> point towards the rST docs, especially for non-user options.

https://patchwork.ozlabs.org/project/uboot/patch/20230120214605.2725139-1-sjg@chromium.org/

Regards,
Simon

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/2] event: Correct dependencies on the EVENT framework
  2023-01-15 22:04         ` Tom Rini
@ 2023-01-21  0:24           ` Simon Glass
  0 siblings, 0 replies; 20+ messages in thread
From: Simon Glass @ 2023-01-21  0:24 UTC (permalink / raw)
  To: Tom Rini; +Cc: u-boot, Oliver Graute, Francesco Dolcini

Hi Tom,

On Sun, 15 Jan 2023 at 15:04, Tom Rini <trini@konsulko.com> wrote:
>
> On Sun, Jan 15, 2023 at 02:52:35PM -0700, Simon Glass wrote:
> > Hi Tom,
> >
> > On Sun, 15 Jan 2023 at 06:45, Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Sat, Jan 14, 2023 at 06:31:05PM -0700, Simon Glass wrote:
> > > > Hi Tom,
> > > >
> > > > On Sat, 14 Jan 2023 at 13:49, Tom Rini <trini@konsulko.com> wrote:
> > > > >
> > > > > The event framework is just that, a framework. Enabling it by itself
> > > > > does nothing, so we shouldn't ask the user about it. Reword (and correct
> > > > > typos) around this the option and help text. This also applies to
> > > > > DM_EVENT, so reword as well.
> > > > >
> > > > > With this, it's time to address the larger problems. When functionality
> > > > > uses events, typically via EVENT_SPY, the appropriate framework then
> > > > > must be select'd and NOT imply'd. As the functionality will cease to
> > > > > work (and so, platforms will fail to boot) this is non-optional and
> > > > > where select is appropriate. Audit the current users of EVENT_SPY to
> > > > > have a more fine-grained approach to select'ing the framework where
> > > > > used.
> > > > >
> > > > > Cc: Simon Glass <sjg@chromium.org>
> > > > > Reported-by: Oliver Graute <Oliver.Graute@kococonnector.com>
> > > > > Reported-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> > > > > Fixes: 7fe32b3442f0 ("event: Convert arch_cpu_init_dm() to use events")
> > > > > Fixes: 42fdcebf859f ("event: Convert misc_init_f() to use events")
> > > > > Signed-off-by: Tom Rini <trini@konsulko.com>
> > > > > ---
> > > > >  arch/Kconfig                     |  6 +++---
> > > > >  arch/arm/Kconfig                 |  9 ++++-----
> > > > >  arch/arm/mach-omap2/Kconfig      |  3 +++
> > > > >  arch/mips/Kconfig                |  2 +-
> > > > >  arch/powerpc/cpu/mpc85xx/Kconfig |  1 +
> > > > >  arch/x86/Kconfig                 |  1 +
> > > > >  arch/x86/cpu/baytrail/Kconfig    |  1 +
> > > > >  arch/x86/cpu/broadwell/Kconfig   |  1 +
> > > > >  arch/x86/cpu/ivybridge/Kconfig   |  1 +
> > > > >  arch/x86/cpu/quark/Kconfig       |  1 +
> > > > >  board/google/Kconfig             |  1 +
> > > > >  board/keymile/Kconfig            |  1 +
> > > > >  boot/Kconfig                     |  1 +
> > > > >  cmd/Kconfig                      |  1 +
> > > > >  common/Kconfig                   | 17 ++++++++---------
> > > > >  drivers/core/Kconfig             |  9 +++++----
> > > > >  drivers/cpu/Kconfig              |  1 -
> > > >
> > > > Reviewed-by: Simon Glass <sjg@chromium.org>
> > > >
> > > > Tested on chromebook-coral:
> > > > Tested-by: Simon Glass <sjg@chromium.org>
> > > >
> > > > I am not quite sure what the effective difference is between select
> > > > and imply. Doesn't this suggest that there are some conflicting config
> > > > options? The only way imply would be disabled ( I thought) is if a
> > > > board does it explicitly?
> > >
> > > So there's two parts to it. As a framework, the option needs to be
> > > select'd, not implied. It doesn't make sense to disable the event
> > > framework and then wonder why VBE doesn't work. The next part however is
> > >
> > > Fixes: c5ef2025579e ("dm: fix DM_EVENT dependencies")
> >
> > OK
> >
> > >
> > > should have been part of it too. That commit turned all of the platforms
> > > which had imply DM_EVENT (and so from DM_EVENT, imply EVENT) in to
> > > depends on EVENT being set, and if it wasn't set (or select'd, only
> > > EFI_LOADER was select'ing it at the time) in to not enabling DM_EVENT,
> > > and since imply options can be off, no warnings were thrown by the build
> > > system. And since all of the non-dynamic EVENT stuff is linker-based,
> > > no binary size changes happened either.
> >
> > So does this mean that select is 'forcing' the option to be enabled,
> > even if it conflicts with something else, or depends on something
> > which is not set?
> >
> > I'd quite like to understand this better for the future. Could you
> > point to a board that was broken and now fixed, so I can have a fiddle
> > with it?
>
> While kernel centric at times (of course)
> https://www.kernel.org/doc/html/latest/kbuild/kconfig-language.html#menu-attributes
> covers things well, especially for the language itself. So yes, select
> forces the option to be enabled. If there's an unmet (depends on ...)
> dependency, then it will emit a warning to tell you as such. An example
> of a previously broken board would be apalis-imx8 as it does not enable
> EFI_LOADER so no DM_EVENT / EVENT and so:
> arch/arm/mach-imx/imx8/cpu.c:EVENT_SPY(EVT_DM_POST_INIT, imx8_init_mu);
> wasn't called and the platform would silently fail to boot.

Thanks for the info. So in that case, since DM_EVENT depended on EVENT
and DM_EVENT was only implied (but EVENT was not enabled), it didn't
work.

Yes I agree 'select' is better here, to force it. But in this case the
'select EVENT' in 'DM_EVENT' is enough (as would be an 'imply EVENT').
I am quite happy with how it has turned out, but I do want to use
'select' sparingly as it limits what boards can modify.

Regards,
Simon

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2023-01-21  0:25 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-01-14 20:49 [PATCH 1/2] x86: Fix saying arch_cpu_init_dm in debug/docs Tom Rini
2023-01-14 20:49 ` [PATCH 2/2] event: Correct dependencies on the EVENT framework Tom Rini
2023-01-15  1:31   ` Simon Glass
2023-01-15 13:45     ` Tom Rini
2023-01-15 21:52       ` Simon Glass
2023-01-15 22:04         ` Tom Rini
2023-01-21  0:24           ` Simon Glass
2023-01-15 12:29   ` Fabio Estevam
2023-01-16  3:10   ` AKASHI Takahiro
2023-01-16  3:15     ` Tom Rini
2023-01-16  4:17       ` AKASHI Takahiro
2023-01-16  6:32         ` AKASHI Takahiro
2023-01-16 14:19           ` Tom Rini
2023-01-15  1:31 ` [PATCH 1/2] x86: Fix saying arch_cpu_init_dm in debug/docs Simon Glass
2023-01-16 20:46 ` [v3 2/2] event: Correct dependencies on the EVENT framework Tom Rini
2023-01-16 21:01   ` Heinrich Schuchardt
2023-01-16 21:04     ` Tom Rini
2023-01-20 22:11       ` Simon Glass
2023-01-19 14:46   ` Tom Rini
2023-01-19 14:46 ` [PATCH 1/2] x86: Fix saying arch_cpu_init_dm in debug/docs Tom Rini

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.