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 <[email protected]>
> > > > > Reported-by: Oliver Graute <[email protected]>
> > > > > Reported-by: Francesco Dolcini <[email protected]>
> > > > > 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 <[email protected]>
> > > > > ---
> > > > >  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: signature.asc
Description: PGP signature

Reply via email to