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 <[email protected]> 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 <[email protected]> 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 - > > > > > > Reviewed-by: Simon Glass <[email protected]> > > > > > > Tested on chromebook-coral: > > > Tested-by: Simon Glass <[email protected]> > > > > > > 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
signature.asc
Description: PGP signature
