Attention is currently required from: Subrata Banik, Tim Wawrzynczak, Werner Zeh, Patrick Rudolph, EricR Lai. Sridhar Siricilla has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/61520 )
Change subject: soc/intel/common/cse: Add function to perform global reset lock ......................................................................
Patch Set 9: Code-Review+1
(2 comments)
Patchset:
PS9: Looks Good To Me.
File src/soc/intel/common/block/cse/cse.c:
https://review.coreboot.org/c/coreboot/+/61520/comment/ff4c4f74_553d2a43 PS8, Line 1022: if (cse_is_hfs1_spi_protected())
Also, isn't that you said, CSE update doesn't bother about global reset using PMC rather use MEI […]
Any pointer to about who shares this recommendation and in what form ? I mean any document as such.
You may refer the CSE Lite User Guide (or cse/cse_lite.c) to understand the CSE Lite update flow. The flow gives details what is the method coreboot use to update the CSE Lite firmware. From this, we can infer locking global reset always doesn't make any issue.
Note, `if clause` is about locking the CF9Lock and `else` is about keep the CF9 unlock to allow global reset. What you have asked originally at starting of this thread as below is against what you just said here.
You have asked to skip locking the CF9Lock in case SoC user selects CSE LITE SKU.
if (cse_cse_is_hfs1_spi_protected() && !CONFIG(CONFIG_SOC_INTEL_CSE_LITE_SKU)
And, in above comment you said, the recommendation is global reset remain locked for CSE lite? then why to leave the CF9Lock unlock for CSE Lite SKU?
OMG, my code interpretation is wrong here.
nit: if (cse_cse_is_hfs1_spi_protected() || CONFIG(CONFIG_SOC_INTEL_CSE_LITE_SKU))