| Message ID | 20231016222835.596572-22-sjg@chromium.org |
|---|---|
| State | Changes Requested |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | Tidy up use of CONFIG_CMDLINE | expand |
Hi Simon, Thank you for taking my idea here. On Mon, Oct 16, 2023 at 04:28:12PM -0600, Simon Glass wrote: > The command should not be used to enable library functionality. Add a > new BOOTEFI_BOOTMGR Kconfig for that. Adjust the conditions so that the > same code is built. > > Signed-off-by: Simon Glass <sjg@chromium.org> > Suggested-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > --- > > Changes in v3: > - Add new patch to rearrange the Kconfig for CMD_BOOTEFI_BOOTMGR > > cmd/Kconfig | 9 +++++++++ > lib/efi_loader/Kconfig | 7 +++---- > lib/efi_loader/Makefile | 2 +- > 3 files changed, 13 insertions(+), 5 deletions(-) > > diff --git a/cmd/Kconfig b/cmd/Kconfig > index 46e484fc08b6..3b4112d9f319 100644 > --- a/cmd/Kconfig > +++ b/cmd/Kconfig > @@ -306,6 +306,15 @@ config CMD_BOOTI > help > Boot an AArch64 Linux Kernel image from memory. > > +config CMD_BOOTEFI_BOOTMGR > + bool "UEFI Boot Manager command" > + depends on BOOTEFI_BOOTMGR && CMD_BOOTEFI > + default y > + help > + Select this option if you want to select the UEFI binary to be booted > + via UEFI variables Boot####, BootOrder, and BootNext. This enables the > + 'bootefi bootmgr' command. > + > config BOOTM_LINUX Do you have any intention to put this configuration here? Otherwise, it should be placed just after CMD_BOOTEFI. In addition, CMD_EFICONFIG should have a dependency on BOOTEFI_BOOTMGR rather than CMD_BOOTEFI_BOOTMGR as it is a separate command. Thanks, -Takahiro Akashi > bool "Support booting Linux OS images" > depends on CMD_BOOTM || CMD_BOOTZ || CMD_BOOTI > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig > index 621ed5e5b0fb..13cad6342c36 100644 > --- a/lib/efi_loader/Kconfig > +++ b/lib/efi_loader/Kconfig > @@ -32,15 +32,14 @@ config EFI_LOADER > > if EFI_LOADER > > -config CMD_BOOTEFI_BOOTMGR > +config BOOTEFI_BOOTMGR > bool "UEFI Boot Manager" > - depends on CMDLINE > default y > select BOOTMETH_GLOBAL if BOOTSTD > help > Select this option if you want to select the UEFI binary to be booted > - via UEFI variables Boot####, BootOrder, and BootNext. This enables the > - 'bootefi bootmgr' command. > + via UEFI variables Boot####, BootOrder, and BootNext. You should also > + normally enable CMD_BOOTEFI_BOOTMGR so that the command is available. > > choice > prompt "Store for non-volatile UEFI variables" > diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile > index 8d31fc61c601..0a2cb6e3c476 100644 > --- a/lib/efi_loader/Makefile > +++ b/lib/efi_loader/Makefile > @@ -42,7 +42,7 @@ targets += initrddump.o > endif > > obj-$(CONFIG_CMD_BOOTEFI_HELLO) += helloworld_efi.o > -obj-$(CONFIG_CMD_BOOTEFI_BOOTMGR) += efi_bootmgr.o > +obj-$(CONFIG_BOOTEFI_BOOTMGR) += efi_bootmgr.o > obj-y += efi_boottime.o > obj-y += efi_helper.o > obj-$(CONFIG_EFI_HAVE_CAPSULE_SUPPORT) += efi_capsule.o > -- > 2.42.0.655.g421f12c284-goog >
On Mon, Oct 16, 2023 at 04:28:12PM -0600, Simon Glass wrote: > The command should not be used to enable library functionality. Add a > new BOOTEFI_BOOTMGR Kconfig for that. Adjust the conditions so that the > same code is built. > > Signed-off-by: Simon Glass <sjg@chromium.org> > Suggested-by: AKASHI Takahiro <takahiro.akashi@linaro.org> [snip] > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig > index 621ed5e5b0fb..13cad6342c36 100644 > --- a/lib/efi_loader/Kconfig > +++ b/lib/efi_loader/Kconfig > @@ -32,15 +32,14 @@ config EFI_LOADER > > if EFI_LOADER > > -config CMD_BOOTEFI_BOOTMGR > +config BOOTEFI_BOOTMGR > bool "UEFI Boot Manager" > - depends on CMDLINE This is another example of why I'm asking for re-ordering things so that first you clean / re-order things then you make all of CMD depend on CMDLINE. This patch, aside from other feedback, is standalone, if you do that.
Hi Tom, On Tue, Oct 17, 2023, 07:13 Tom Rini <trini@konsulko.com> wrote: > > On Mon, Oct 16, 2023 at 04:28:12PM -0600, Simon Glass wrote: > > The command should not be used to enable library functionality. Add a > > new BOOTEFI_BOOTMGR Kconfig for that. Adjust the conditions so that the > > same code is built. > > > > Signed-off-by: Simon Glass <sjg@chromium.org> > > Suggested-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > [snip] > > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig > > index 621ed5e5b0fb..13cad6342c36 100644 > > --- a/lib/efi_loader/Kconfig > > +++ b/lib/efi_loader/Kconfig > > @@ -32,15 +32,14 @@ config EFI_LOADER > > > > if EFI_LOADER > > > > -config CMD_BOOTEFI_BOOTMGR > > +config BOOTEFI_BOOTMGR > > bool "UEFI Boot Manager" > > - depends on CMDLINE > > This is another example of why I'm asking for re-ordering things so that > first you clean / re-order things then you make all of CMD depend on > CMDLINE. This patch, aside from other feedback, is standalone, if you > do that. This patch is before the one that makes all of CMD depend on CMDLINE. Regards, Simon
diff --git a/cmd/Kconfig b/cmd/Kconfig index 46e484fc08b6..3b4112d9f319 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -306,6 +306,15 @@ config CMD_BOOTI help Boot an AArch64 Linux Kernel image from memory. +config CMD_BOOTEFI_BOOTMGR + bool "UEFI Boot Manager command" + depends on BOOTEFI_BOOTMGR && CMD_BOOTEFI + default y + help + Select this option if you want to select the UEFI binary to be booted + via UEFI variables Boot####, BootOrder, and BootNext. This enables the + 'bootefi bootmgr' command. + config BOOTM_LINUX bool "Support booting Linux OS images" depends on CMD_BOOTM || CMD_BOOTZ || CMD_BOOTI diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig index 621ed5e5b0fb..13cad6342c36 100644 --- a/lib/efi_loader/Kconfig +++ b/lib/efi_loader/Kconfig @@ -32,15 +32,14 @@ config EFI_LOADER if EFI_LOADER -config CMD_BOOTEFI_BOOTMGR +config BOOTEFI_BOOTMGR bool "UEFI Boot Manager" - depends on CMDLINE default y select BOOTMETH_GLOBAL if BOOTSTD help Select this option if you want to select the UEFI binary to be booted - via UEFI variables Boot####, BootOrder, and BootNext. This enables the - 'bootefi bootmgr' command. + via UEFI variables Boot####, BootOrder, and BootNext. You should also + normally enable CMD_BOOTEFI_BOOTMGR so that the command is available. choice prompt "Store for non-volatile UEFI variables" diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile index 8d31fc61c601..0a2cb6e3c476 100644 --- a/lib/efi_loader/Makefile +++ b/lib/efi_loader/Makefile @@ -42,7 +42,7 @@ targets += initrddump.o endif obj-$(CONFIG_CMD_BOOTEFI_HELLO) += helloworld_efi.o -obj-$(CONFIG_CMD_BOOTEFI_BOOTMGR) += efi_bootmgr.o +obj-$(CONFIG_BOOTEFI_BOOTMGR) += efi_bootmgr.o obj-y += efi_boottime.o obj-y += efi_helper.o obj-$(CONFIG_EFI_HAVE_CAPSULE_SUPPORT) += efi_capsule.o
The command should not be used to enable library functionality. Add a new BOOTEFI_BOOTMGR Kconfig for that. Adjust the conditions so that the same code is built. Signed-off-by: Simon Glass <sjg@chromium.org> Suggested-by: AKASHI Takahiro <takahiro.akashi@linaro.org> --- Changes in v3: - Add new patch to rearrange the Kconfig for CMD_BOOTEFI_BOOTMGR cmd/Kconfig | 9 +++++++++ lib/efi_loader/Kconfig | 7 +++---- lib/efi_loader/Makefile | 2 +- 3 files changed, 13 insertions(+), 5 deletions(-)