On 2/7/23 15:50, Tom Rini wrote:
On Tue, Feb 07, 2023 at 08:39:38AM +0100, Heinrich Schuchardt wrote:
On 2/7/23 01:00, Tom Rini wrote:
On Tue, Feb 07, 2023 at 12:54:03AM +0100, Heinrich Schuchardt wrote:
On 2/6/23 01:53, Simon Glass wrote:
This converts 3 usages of this option to the non-SPL form, since there is
no SPL_CMD_BOOTEFI_BOOTMGR defined in Kconfig
Signed-off-by: Simon Glass <[email protected]>
---
(no changes since v1)
boot/Makefile | 2 +-
cmd/bootmenu.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/boot/Makefile b/boot/Makefile
index 69c31adb77d..73b5b19816b 100644
--- a/boot/Makefile
+++ b/boot/Makefile
@@ -29,7 +29,7 @@ obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_EFILOADER) += bootmeth_efi.o
obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_SANDBOX) += bootmeth_sandbox.o
obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_SCRIPT) += bootmeth_script.o
ifdef CONFIG_$(SPL_TPL_)BOOTSTD_FULL
-obj-$(CONFIG_$(SPL_TPL_)CMD_BOOTEFI_BOOTMGR) += bootmeth_efi_mgr.o
+obj-$(CONFIG_CMD_BOOTEFI_BOOTMGR) += bootmeth_efi_mgr.o
obj-$(CONFIG_$(SPL_TPL_)BOOTSTD) += bootflow_menu.o
endif
diff --git a/cmd/bootmenu.c b/cmd/bootmenu.c
index 3236ca5d799..422ab411252 100644
--- a/cmd/bootmenu.c
+++ b/cmd/bootmenu.c
@@ -223,7 +223,7 @@ static int prepare_bootmenu_entry(struct bootmenu_data
*menu,
return 1;
}
-#if (CONFIG_IS_ENABLED(CMD_BOOTEFI_BOOTMGR)) &&
(CONFIG_IS_ENABLED(CMD_EFICONFIG))
+#if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR)) &&
(CONFIG_IS_ENABLED(CMD_EFICONFIG))
There is no reason whatsoever for using different macros for the two
options.
Here and elsewhere, one CONFIG is being fixed at a time. If at the end
of the series this is not fixed, then that's an issue to address.
This cannot be reviewed easily. I never received the complete series.
This, and the related series, are among the most reviewed we've had in
quite some time. Just FWIW.
CONFIG_IS_ENABLED() is more restrictive than IS_ENABLED(). No motivation is
provided why the condition should be relaxed in the commit message.
The idea of "restrictive" is not how either of those macros should be
evaluated.
Cover-letters are not in the commit history. But anyway the cover-letter
does not provide any motivation for the change either.
NAK to this patch.
It's incorrect to use CONFIG_IS_ENABLED() instead of IS_ENABLED()
outside of:
- CONFIG_FOO, CONFIG_SPL_FOO (etc) exist
- The code in question is compiled in the SPL (etc) context and we do
need to know if the code block in question is required here and the
implicit value of SPL_FOO being false is useful.
This case is why Simon insists that adding def_bool n for
SPL_EFI_LOADER, etc, is correct, but I'm not convinced.
I will be taking most of these patches in the next day or two, but I can
avoid the EFI ones if you insist. They are however I believe correct.
The code changes are ok with me. It were simply the commit messages
which were insufficient.
Acked-by: Heinrich Schuchardt <[email protected]>
