2 comments:
File src/drivers/intel/fsp1_1/Kconfig:
Patch Set #2, Line 16: depends on !VBOOT_STARTS_IN_BOOTBLOCK
Is it required adding this dependency?
There seems to be no FSP1.1 board using STARTS_IN_BOOTBLOCK atm, so I didn't account for that case in the code and adding this here just documents that. I assume there are good reasons for the existing SoCs not offering this, and we won't be adding any new FSP1.1 SoCs (right?), so I thought this should be fine?
File src/drivers/mrc_cache/mrc_cache.c:
Patch Set #2, Line 522: if (!CONFIG(VBOOT_STARTS_IN_BOOTBLOCK))
Ya. I was following along in the other CL where this was brought up. […]
Well, yeah, I could hide it all in vboot_recovery_mode_enabled(). I feel like that's hiding important details and risking mistakes due to incorrect assumptions, though. Writing the 'CONFIG(VBOOT_STARTS_IN_BOOTBLOCK) && ' in front of it makes it clear that this check can only return true when that option is enabled, and leaving the implementation of vboot_recovery_mode_enabled() as it is right now (so it will assert() when called too early) makes sure people will notice very quickly if they're using it wrong. When I make it silently return 0 instead it becomes very unobvious that the MRC cache behavior is different between STARTS_IN_BOOTBLOCK and STARTS_IN_ROMSTAGE platforms, for example.
I can change it to that if you want but I feel like my version is safer and more readable (in the sense of helping people who read it actually understand what happens).
To view, visit change 39221. To unsubscribe, or for help writing mail filters, visit settings.