Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39343 )
Change subject: vboot: remove extraneous vboot_recovery_mode_memory_retrain ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39343/5/src/soc/mediatek/mt8183/mem... File src/soc/mediatek/mt8183/memory.c:
https://review.coreboot.org/c/coreboot/+/39343/5/src/soc/mediatek/mt8183/mem... PS5, Line 179: vboot_check_recovery_request() == VB2_RECOVERY_TRAIN_AND_REBOOT) { This is a somewhat tangential but still related question... why do we only handle this recovery reason in MT8183? Shouldn't this also work on Intel? I thought this was originally introduced for Intel anyway (CL:407656)?
I was thinking we should maybe have a helper that combines checking the manual switch and the recovery reason (which might then be called something like vboot_recovery_mode_memory_retrain(), so maybe worth keeping the helper and just changing its meaning), but then I noticed that all the other code doesn't actually handle this reason. Is that intentional or an oversight?