Attention is currently required from: Haribalaraman Ramasubramanian, Dinesh Gehlot, Rizwan Qureshi, Reka Norman.
Sridhar Siricilla has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74005 )
Change subject: soc/intel/cmd/block: Implement an API to get firmware partition details ......................................................................
Patch Set 28:
(5 comments)
File src/soc/intel/common/block/cse/cse.c:
https://review.coreboot.org/c/coreboot/+/74005/comment/64263eee_dd0e1fd7 PS28, Line 1242: fw partition request Get Image Firmware Version command?
https://review.coreboot.org/c/coreboot/+/74005/comment/406aaba2_515d6230 PS28, Line 1250: partition partition information for?
https://review.coreboot.org/c/coreboot/+/74005/comment/ac26878a_ed545d38 PS28, Line 1259: if (vboot_recovery_mode_enabled()) { : printk(BIOS_INFO, "CSE: Info request denied, request during recovery mode\n"); : return false; : } I see you added the condition in the higher layer, so please remove it from here. this has nothing to do with the command execution.
https://review.coreboot.org/c/coreboot/+/74005/comment/59715f54_aaabb6ab PS28, Line 1264: SOC_INTEL_CSE_LITE_SKU Please note cse_get_fpt_partition_info() is also applicable for non-CSE lite SKU and this file refers CSE API library. I don't see need for this check as pre-requisites already handling this.
https://review.coreboot.org/c/coreboot/+/74005/comment/ca89e53c_903a6710 PS28, Line 1269: if (id == FPT_PARTITION_NAME_ISHC && !CONFIG(DRIVERS_INTEL_ISH)) { : printk(BIOS_INFO, "CSE: Info request denied, no ISH partition\n"); : return false; : } : Can this moved to higher layer since this file contains CSE API Lib?