Hi Harsimran, On Wed, 6 May 2026 at 06:21, Harsimran Singh Tungal <[email protected]> wrote: > > 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?
Yes that makes sense, thanks. > > > > 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. Rgards, Simon
