Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47430 )
Change subject: soc/intel/common: Add Kconfig for CSE RW firmware version ......................................................................
Patch Set 4:
(2 comments)
https://review.coreboot.org/c/coreboot/+/47430/4/src/soc/intel/common/block/... File src/soc/intel/common/block/cse/Kconfig:
https://review.coreboot.org/c/coreboot/+/47430/4/src/soc/intel/common/block/... PS4, Line 22: What do you think about adding a Kconfig `SOC_INTEL_CSE_RW_UPDATE` which enables support for RW update. I know previously we decided not to add a separate Kconfig and only use the presence of SOC_INTEL_CSE_RW_FILE as an indication. But, now that we have multiple Kconfigs for update, maybe it makes sense to have that switch?
It will also allow us to add compile-time checks in Makefile to ensure all required configs are provided by mainboard.
i.e. if SOC_INTEL_CSE_RW_UPDATE is selected, but SOC_INTEL_CSE_RW_FILE or SOC_INTEL_CSE_RW_VERSION is not provided, then throw an error asking the user to set both configs.
Also, you can use the switch to decide if the configs below need to be made user-visible.
i.e. if SOC_INTEL_CSE_RW_UPDATE
config SOC_INTEL_CSE_FMAP_NAME ... ... ...
endif
https://review.coreboot.org/c/coreboot/+/47430/4/src/soc/intel/common/block/... PS4, Line 49: firmware nit: Rest of the Kconfigs refer to this as blob. Maybe use the same here?
Also, it would be good to add text indicating that this is really the version for the file provided by SOC_INTEL_CSE_RW_FILE.