Jamie Ryu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42282 )
Change subject: soc/intel/tigerlake: Disable hybrid storage mode in CSE Lite RO boot ......................................................................
Patch Set 14:
(2 comments)
https://review.coreboot.org/c/coreboot/+/42282/14/src/soc/intel/tigerlake/fs... File src/soc/intel/tigerlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/42282/14/src/soc/intel/tigerlake/fs... PS14, Line 260: (CONFIG(SOC_INTEL_CSE_LITE_SKU) && cse_is_hfs3_fw_sku_lite()
it looks it's same check, why do we need to check both case?
There is a chance CONFIG_SOC_INTEL_CSE_LITE_SKU is enabled in coreboot while CSE Consumer SKU is integrated in ME blob. Current implementation checks CSE SKU information before runs any CSE Lite specific code flow, so that CSE Lite specific code does not affect running CSE Consumer SKU. This code is added by the same assumption and to be fail-safe.
https://review.coreboot.org/c/coreboot/+/42282/14/src/soc/intel/tigerlake/fs... PS14, Line 261: !cse_is_hfs1_com_normal()
Could you explain why this checking is added?
Current FSP implementation checks CSE state to see which HECI commands are supported in current CSE state, and the HECI commands to override PCIe straps to support hybrid storage dynamic configuration are only supported in CSE Normal state; hence, we are checking CSE Normal state to make sure CSE is in state that cannot support the strap override. Please let me know if more description is needed.