5 comments:
File src/drivers/intel/fsp1_1/Kconfig:
Patch Set #2, Line 16: depends on !VBOOT_STARTS_IN_BOOTBLOCK
> AFAIK, the only SoC that uses FSP 1. […]
Like I said, this is just supposed to document that we're relying on that fact now. Let me know if you prefer I take it out, I don't care particularly.
File src/drivers/mrc_cache/mrc_cache.c:
Patch Set #2, Line 522: if (!CONFIG(VBOOT_STARTS_IN_BOOTBLOCK))
About this problem: could we please explicitly mention the calls in raminit in the commit message?
So I'm going to add the dependency Joel mentioned so that I can take one of these checks back out. For the other one, I still prefer to write it like this due to the reasons above. Please let me know if this works for everyone or there are still concerns about the general approach.
(I'll also try to put more details into the commit message although I'm not 100% sure what you want me to write, Angel.)
File src/northbridge/intel/haswell/raminit.c:
Patch Set #2, Line 130: if (!(CONFIG(HASWELL_VBOOT_IN_BOOTBLOCK) && vboot_recovery_mode_enabled())
Would using VBOOT_STARTS_IN_BOOTBLOCK on all chipset be an unified solution?
Do you mean using it on all Haswell boards? I guess so... I don't really know why for some of these chipsets some boards start in bootblock and some in romstage. Usually, if start in bootblock is supported for that chipset you'd want to use that for all boards. But I don't know any details about these chipsets so there may be good reasons for this (maybe some boards have larger bootblocks and then it wouldn't fit or something), so I'd rather not try to change that. With what this patch is doing at least I'm pretty confident that it shouldn't break anything.
File src/northbridge/intel/sandybridge/raminit_mrc.c:
Patch Set #2, Line 210: if (!(CONFIG(SANDYBRIDGE_VBOOT_IN_BOOTBLOCK) && vboot_recovery_mode_enabled()) ||
Well, if the symbols already exist, I'd reuse them
Yeah, I could use the generic symbol here as well I just thought this one fits a bit better.
File src/northbridge/intel/sandybridge/raminit_mrc.c:
Patch Set #3, Line 142: SANDYBRIDGE_VBOOT_IN_BOOTBLOCK
nit: for consistency over platforms, offering also a VBOOT_STARTS_IN_ROMSTAGE option for legacy reas […]
Seems like there are different opinions about which one to prefer (see Angel's other comment), so I'm not sure what to pick. Personally, I think putting the chipset-specific symbol here helps let the reader know that it exists (e.g. that they shouldn't try to set VBOOT_STARTS_IN_BOOTBLOCK directly, or that they should maybe go read the chipset Kconfig to figure out why a separate symbol was made for this). I'm happy to do it either way though depending on what the majority prefers.
To view, visit change 39221. To unsubscribe, or for help writing mail filters, visit settings.