Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39221 )
Change subject: vboot: Clean up pre-RAM use of vboot_recovery_mode_enabled() ......................................................................
Patch Set 2: Code-Review+1
(4 comments)
https://review.coreboot.org/c/coreboot/+/39221/2/src/drivers/intel/fsp1_1/Kc... File src/drivers/intel/fsp1_1/Kconfig:
https://review.coreboot.org/c/coreboot/+/39221/2/src/drivers/intel/fsp1_1/Kc... PS2, Line 16: depends on !VBOOT_STARTS_IN_BOOTBLOCK
There seems to be no FSP1. […]
AFAIK, the only SoC that uses FSP 1.1 is Braswell, and none of the Braswell boards select VBOOT_STARTS_IN_BOOTBLOCK
https://review.coreboot.org/c/coreboot/+/39221/2/src/drivers/mrc_cache/mrc_c... File src/drivers/mrc_cache/mrc_cache.c:
https://review.coreboot.org/c/coreboot/+/39221/2/src/drivers/mrc_cache/mrc_c... PS2, Line 522: if (!CONFIG(VBOOT_STARTS_IN_BOOTBLOCK))
This isn't the same thing, this is CONFIG(MRC_CLEAR_NORMAL_CACHE_ON_RECOVERY_RETRAIN). […]
About this problem: could we please explicitly mention the calls in raminit in the commit message?
https://review.coreboot.org/c/coreboot/+/39221/2/src/northbridge/intel/sandy... File src/northbridge/intel/sandybridge/raminit_mrc.c:
https://review.coreboot.org/c/coreboot/+/39221/2/src/northbridge/intel/sandy... PS2, Line 210: if (!(CONFIG(SANDYBRIDGE_VBOOT_IN_BOOTBLOCK) && vboot_recovery_mode_enabled()) ||
Would using VBOOT_STARTS_IN_BOOTBLOCK on all chipset be an unified solution?
Well, if the symbols already exist, I'd reuse them
https://review.coreboot.org/c/coreboot/+/39221/2/src/southbridge/intel/bd82x... File src/southbridge/intel/bd82x6x/me_8.x.c:
https://review.coreboot.org/c/coreboot/+/39221/2/src/southbridge/intel/bd82x... PS2, Line 419: #if CONFIG(CHROMEOS) && 0 /* DISABLED */ I am not sure if this could be useful in the future. I'm leaving a resolved comment here just to remind myself that this patch got rid of it.