Attention is currently required from: Dinesh Gehlot, Sridhar Siricilla.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74995?usp=email )
Change subject: soc/intel/cmd/blk/cse: Implement APIs to access FW versions in CMOS ......................................................................
Patch Set 38:
(8 comments)
File src/soc/intel/common/block/cse/cse_lite_cmos.c:
https://review.coreboot.org/c/coreboot/+/74995/comment/be1773f9_0bb236c2 : PS38, Line 49: read_cmos_partition_version cmos_read_parition_version
https://review.coreboot.org/c/coreboot/+/74995/comment/3d7a86e3_f52d9c74 : PS38, Line 75: write_cmos_partition_version start with cmos
https://review.coreboot.org/c/coreboot/+/74995/comment/f24e0d02_70d74978 : PS38, Line 89: u8 i, *p, offset = PARTITION_FW_CMOS_OFFSET; please give one line break
https://review.coreboot.org/c/coreboot/+/74995/comment/188c1add_fc672785 : PS38, Line 102: struct cse_fw_table version; same
https://review.coreboot.org/c/coreboot/+/74995/comment/da50c202_26486cf4 : PS38, Line 103: if (read_cmos_partition_version(&version)) { : /* : * CMOS failed to read the CSE version. This may be because the firmware version at : * cmos has not yet been initialized. : */ : init_cmos_partition_version(&version); : } can we create a helper function?
``` static bool cmos_get_cse_fpt_data(struct cse_fw_table *data) { if (data == NULL) return false;
/* * CMOS failed to read the CSE version. This may be because the firmware version at * cmos has not yet been initialized. */ if (read_cmos_partition_version(&data)) init_cmos_partition_version(&data);
return true; } ```
``` enum cse_fpt_fw_type { CSE_RW_FW, ISH_FW, MAX_SUPPORTED_TYPE, };
/* Helper function that allows users to read fw version stored from CMOS memory depending on the cse_fw_type */ static void cmos_read_cse_fpt_fw_version(enum cse_fpt_fw_type fw_type, struct fw_version *fw_version) { struct cse_fw_table data; if (!cmos_get_cse_fpt_data(&data)) return;
switch (fw_type) { case CSE_RW_FW: memcpy(fw_version, &data.cse_version, sizeof(struct fw_version)); break; case ISH_FW: memcpy(fw_version, &data.ish_version, sizeof(struct fw_version)); break; default: // add some error msg as unsupported type } } ```
``` /* Helper function that allows users to write fw version stored into CMOS memory depending on the cse_fw_type */ static void cmos_write_cse_fpt_fw_version(enum cse_fpt_fw_type fw_type, struct fw_version *fw_version) { struct cse_fw_table data; if (!cmos_get_cse_fpt_data(&data)) return;
switch (fw_type) { case CSE_RW_FW: memcpy(&data.cse_version, fw_version, sizeof(struct fw_version)); break; case ISH_FW: memcpy(&data.ish_version, fw_version, sizeof(struct fw_version)); break; default: // add some error msg as unsupported type } write_cmos_partition_version(&data); } ```
``` /* API that allows users to read CSE version stored in CMOS memory. */ void cmos_get_cse_rw_fw_version(struct fw_version *cse_version) { return cmos_read_cse_fpt_fw_version(CSE_RW_FW, cse_version); } ```
``` /* API that allows users to write CSE version stored from CMOS memory. */ void cmos_set_cse_rw_fw_version(struct fw_version *cse_version) { return cmos_write_cse_fpt_fw_version(CSE_RW_FW, cse_version); } ```
Similarly for ISH ?
https://review.coreboot.org/c/coreboot/+/74995/comment/7e653a60_ee8cb6fd : PS38, Line 116: struct cse_fw_table version; : if (read_cmos_partition_version(&version)) { : /* : * CMOS failed to read the CSE version. This may be because the firmware version at : * cmos has not yet been initialized. : */ : init_cmos_partition_version(&version); : } reuse the helper function
https://review.coreboot.org/c/coreboot/+/74995/comment/66052068_26e760a8 : PS38, Line 132: if (read_cmos_partition_version(&version)) { : /* CMOS ISH read fail, possibly firmware version has not yet initialized. */ : init_cmos_partition_version(&version); : } same
https://review.coreboot.org/c/coreboot/+/74995/comment/4764f31e_dcba4e4a : PS38, Line 143: if (read_cmos_partition_version(&version)) { : /* CMOS ISH read fail, possibly firmware version has not yet initialized. */ : init_cmos_partition_version(&version); : } same