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? Thanks, Simon
