[M] Change in coreboot[master]: soc/intel/cmn: Implement HECI commands to get version details of ISHC
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:
(10 comments)
Patchset:
PS11: can you please also create an API as an incremental patch named `get_ishc_version()` which will get called from ramstage alone and if return value version from cbmem if ishc_version_update_done is set to true.
File src/soc/intel/common/block/cse/cse_lite.c:
https://review.coreboot.org/c/coreboot/+/74005/comment/7ca62cc0_9ca3bdfb PS9, Line 204: Two instances of 'fw_version' were allocated to store both the ISHC and CSE versions. : The first instance will store ISHC version and the second instance will store CSE version : */ follow the may char per line rule
File src/soc/intel/common/block/cse/cse_lite.c:
https://review.coreboot.org/c/coreboot/+/74005/comment/6f90aa84_ad742a44 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.
https://review.coreboot.org/c/coreboot/+/74005/comment/121d90df_85c44d92 PS11, Line 206: // Compare if saved CSE is same as current follow /* */
https://review.coreboot.org/c/coreboot/+/74005/comment/e0999b5d_b0692c81 PS11, Line 211: else else {
}
File src/soc/intel/common/block/cse/cse_partition_version.c:
https://review.coreboot.org/c/coreboot/+/74005/comment/d2d9e2a1_2166a2a6 PS11, Line 11: cbmem_entry_start shouldn't this be just cbmem_find()
https://review.coreboot.org/c/coreboot/+/74005/comment/fbc40907_c3d664bd PS11, Line 36: if (ret || resp.hdr.result) if {
}
https://review.coreboot.org/c/coreboot/+/74005/comment/538306d6_e2a8d798 PS11, Line 44: BS_DEV_ENABLE if we want to do this operation at early ramstage then `BS_PRE_DEVICE` is the correct BS
File src/soc/intel/common/block/include/intelblocks/cse.h:
https://review.coreboot.org/c/coreboot/+/74005/comment/f921f6b3_6bac77f7 PS11, Line 139: typedef avoid typedef
https://review.coreboot.org/c/coreboot/+/74005/comment/18a48523_c6b95a23 PS11, Line 143: fw_partition_version cse_fw_ish_version_info
-
Subrata Banik (Code Review)