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-att... 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