Attention is currently required from: Nico Huber, Sridhar Siricilla, Patrick Rudolph. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/61380 )
Change subject: soc/inte/common: Add support to control coreboot and Intel SoC features ......................................................................
Patch Set 2:
(1 comment)
File src/soc/intel/common/block/cse/cse_lite.c:
https://review.coreboot.org/c/coreboot/+/61380/comment/20cc11f0_6b80a93c PS2, Line 666: is_cse_fw_update_enabled
- SOC_INTEL_COMMON_BLOCK_DEBUG_FEATURE & SOC_INTEL_CSE_RW_UPDATE are enabled -> decision is based on debug flag
Do you mean `&` (bitwise) or `&&` (logical) ?
SOC_INTEL_CSE_RW_UPDATE is only enabled -> CSE FW update should be enabled
So, you will always have SOC_INTEL_CSE_RW_UPDATE on platform and if would like to disable RW update then will perform override from strap and select SOC_INTEL_COMMON_BLOCK_DEBUG_FEATURE ?
I felt this is over doing.
IMO, you don't need additional SOC_INTEL_COMMON_BLOCK_DEBUG_FEATURE to guard the softstrap read operation.
The flow should look like below:
1. SOC_INTEL_CSE_RW_UPDATE is enabled and read softstrap to know if CSE update is allowed, if softstrap is set then perform the update. Assume except debug scenarios, you always have this softstrap enabled. 2. On debug scenarios when you don't wish to perform the CSE update even SoC has SOC_INTEL_CSE_RW_UPDATE enabled. is_debug_cse_fw_update_disable() function will return true and coreboot will skip CSE FW update. 3. SoC that doesn't want to perform CSE FW update, won't select SOC_INTEL_CSE_RW_UPDATE config.