On 2026-04-28 12:01 -0600, Simon Glass wrote: > Hi Harsimran, > > On 2026-04-24T17:31:50, Harsimran Singh Tungal > <[email protected]> wrote: > > corstone1000: enable bootefi selftest > > > > Enable UEFI selftest command in Corstone-1000 defconfig > > > > Turn on CONFIG_CMD_BOOTEFI_SELFTEST in the Corstone-1000 defconfig > > so the board can run the built-in UEFI self-test suite during > > development and validation. > > > > Signed-off-by: Harsimran Singh Tungal <[email protected]> > > > > configs/corstone1000_defconfig | 3 +++ > > 1 file changed, 3 insertions(+) > > > diff --git a/configs/corstone1000_defconfig b/configs/corstone1000_defconfig > > @@ -63,6 +65,7 @@ CONFIG_SYSRESET=y > > CONFIG_SYSRESET_PSCI=y > > CONFIG_TEE=y > > CONFIG_OPTEE=y > > +# CONFIG_CMD_POWEROFF is not set > > CONFIG_USB=y > > Why is this disabled? It's not mentioned in the commit message and > looks unrelated to enabling the bootefi selftest. If it's a stray > byproduct of 'make savedefconfig' please drop it; if intentional it > should be a separate patch with its own justification. >
Thanks, these two lines are intentional rather than stray `savedefconfig` fallout. On Corstone-1000, enabling `CMD_BOOTEFI_SELFTEST` causes `CMD_BOOTEFI_HELLO` to default to `y`, and `CMD_POWEROFF` is also pulled in on this PSCI-based defconfig. I wanted to enable only the selftest command here, without broadening the rest of the board command set, so both are kept explicitly disabled. I can make that explicit in the commit message in v2. Does this sound reasonable to you? > > diff --git a/configs/corstone1000_defconfig b/configs/corstone1000_defconfig > > @@ -32,6 +32,8 @@ CONFIG_SYS_PROMPT="corstone1000# " > > # CONFIG_CMD_CONSOLE is not set > > CONFIG_CMD_FWU_METADATA=y > > CONFIG_CMD_BOOTZ=y > > +# CONFIG_CMD_BOOTEFI_HELLO is not set > > +CONFIG_CMD_BOOTEFI_SELFTEST=y > > The commit message says only that selftest is being enabled, but > you're also explicitly disabling CMD_BOOTEFI_HELLO (which would > otherwise default to y once SELFTEST is set). Please mention this, and > ideally state the real motivation - that this is needed so patch 7 of > the series can run the runtime variable selftest on this board, right? > Thanks, agreed. The explicit `CMD_BOOTEFI_HELLO=n` is intentional. On this board, enabling `CMD_BOOTEFI_SELFTEST` would otherwise make `CMD_BOOTEFI_HELLO` default to `y`. The goal here is to enable the selftest command so the later runtime-variable selftest patch in this series can be exercised on Corstone-1000, without broadening the rest of the board command set at the same time. I'll make that motivation explicit in the commit message in v2. Regards Harsimran Singh Tungal > > Enable UEFI selftest command in Corstone-1000 defconfig > > > > > Turn on CONFIG_CMD_BOOTEFI_SELFTEST in the Corstone-1000 defconfig > > so the board can run the built-in UEFI self-test suite during > > development and validation. > > The first paragraph just restates the subject. Please drop it and keep > the second paragraph, which actually explains the change. Referencing > the runtime-variable selftest added later in the series would also > help, since that is the real reason for enabling it now. > > Regards, > Simon >
