[M] Change in coreboot[master]: soc/intel/cmd/block: Implement an API to get firmware partition details

[Dinesh Gehlot (Code Review)]

Attention is currently required from: Haribalaraman Ramasubramanian, Paul Menzel, Rizwan Qureshi, Reka Norman, Sridhar Siricilla.

Dinesh Gehlot 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 29:

(7 comments)

File src/soc/intel/common/block/cse/cse.c:

https://review.coreboot.org/c/coreboot/+/74005/comment/ab16bc66_720e49bb 
PS28, Line 1218: bool
> Pleas use enum cb_err.

Ack

https://review.coreboot.org/c/coreboot/+/74005/comment/f0c023ef_956bd5a5 
PS28, Line 1242: fw partition request
> Get Image Firmware Version command?

Ack

https://review.coreboot.org/c/coreboot/+/74005/comment/f3496822_70c55685 
PS28, Line 1250: partition
> partition information for?

Ack

https://review.coreboot.org/c/coreboot/+/74005/comment/0abfc06c_58309f33 
PS28, Line 1260: BIOS_INFO
> Should it be level warning or error?

Ack

https://review.coreboot.org/c/coreboot/+/74005/comment/18ecba90_e68657ca 
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. […]

The API is standalone and exposed for generic use. Therefore, it is possible that the user may or may not have performed a recovery mode check before calling this API.

https://review.coreboot.org/c/coreboot/+/74005/comment/c22e4943_667dfc48 
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 refer […]

Ack

https://review.coreboot.org/c/coreboot/+/74005/comment/66e4a639_934e3697 
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?

We could move the check to another location, but I believe it is best to perform all checks before calling the send_get_fpt_partition_info_cmd() function. Scattered checks throughout the call flow will make the code less readable.