Attention is currently required from: Dinesh Gehlot, Kapil Porwal.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74005 )
Change subject: soc/intel/cmn: Implement HECI commands to get version details of ISHC ......................................................................
Patch Set 11:
(6 comments)
File src/soc/intel/common/block/cse/cse_lite.c:
https://review.coreboot.org/c/coreboot/+/74005/comment/eca608f4_8aa16853 PS11, Line 204: saved_version = cbmem_add(CBMEM_ID_CSE_PARTITION_VERSION, sizeof(fw_partition_version));
ideally this should be cbmem_find() and if find is failing then you should add new entry isn't it.
cbmem_find() will pass for all consecutive boot and cbmem_add() is required only for first boot.
while adding cbmem_add for the first time, u should store the initial value as NULL aka 0 and ishc_version_update_done as false.
https://review.coreboot.org/c/coreboot/+/74005/comment/4d6425ee_1b8854ca PS11, Line 210: ishc_update_required saved_version->ishc_version_update_done = false
https://review.coreboot.org/c/coreboot/+/74005/comment/7db6ceac_48bc613c PS11, Line 212: ishc_update_required assuming ishc_version_update_done is already set to `true` from since last update?
File src/soc/intel/common/block/cse/cse_partition_version.c:
https://review.coreboot.org/c/coreboot/+/74005/comment/bddd0b55_41b72106 PS11, Line 37: printk(BIOS_ERR, "HECI: ISHC version resp Failed: 0x%x\n", resp.hdr.result); while failing, u should mark ishc_version_update_done is false to avoid getting any wrong value
https://review.coreboot.org/c/coreboot/+/74005/comment/30df3ad0_3b037cfd PS11, Line 40: ishc_update_required ishc_version_update_done=true
File src/soc/intel/common/block/include/intelblocks/cse.h:
https://review.coreboot.org/c/coreboot/+/74005/comment/e5b12436_b2bd2112 PS11, Line 140: ishc_update_required ishc_version_update_done