[coreboot-gerrit] [M] Change in coreboot[master]: soc/intel/cmd/blk/cse: Implement APIs to access FW versions in CMOS

[Dinesh Gehlot (Code Review)]

Attention is currently required from: Kapil Porwal, Sridhar Siricilla, Subrata Banik.

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

(9 comments)

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

https://review.coreboot.org/c/coreboot/+/74995/comment/de355601_d73c0aa5 :
PS38, Line 49: read_cmos_partition_version
> cmos_read_parition_version

Acknowledged

https://review.coreboot.org/c/coreboot/+/74995/comment/f6689672_4860f7b2 :
PS38, Line 75: write_cmos_partition_version
> start with cmos

Acknowledged

https://review.coreboot.org/c/coreboot/+/74995/comment/c8fa4722_2d56de8e :
PS38, Line 89: 	u8 i, *p, offset = PARTITION_FW_CMOS_OFFSET;
> please give one line break

Acknowledged

https://review.coreboot.org/c/coreboot/+/74995/comment/7a6f62e0_e19c6d2b :
PS38, Line 102: 	struct cse_fw_table version;
> same

Acknowledged

https://review.coreboot.org/c/coreboot/+/74995/comment/3c7655cd_8bd2bbab :
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? […]

Acknowledged

https://review.coreboot.org/c/coreboot/+/74995/comment/324e52c6_f10533c2 :
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

Acknowledged

https://review.coreboot.org/c/coreboot/+/74995/comment/617a0e4f_4b9a1d5c :
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

Acknowledged

https://review.coreboot.org/c/coreboot/+/74995/comment/606069a2_5df9e690 :
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

Acknowledged

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

https://review.coreboot.org/c/coreboot/+/74995/comment/c356c580_3b3ea331 :
PS39, Line 49:
> tab

Acknowledged