Attention is currently required from: Andrey Petrov, Arthur Heymans, Kapil Porwal, Ronak Kanabar.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/75921?usp=email )
Change subject: {commonlib/drivers}: Have option to store MRC version inside CBMEM ......................................................................
Patch Set 3:
(2 comments)
File src/commonlib/bsd/include/commonlib/bsd/cbmem_id.h:
https://review.coreboot.org/c/coreboot/+/75921/comment/bc75bdf7_fd61ceb0 : PS3, Line 116: CBMEM_ID_MRC_VERSION
Why is a new TAG needed?
This tag is need to pass the information between romstage and ramstage. as in romstage creates the data and ramstage consumers it. Hence, ramstage uses either the MRC version or FSP-M version (depending on the config) when updating the MRC cache.
File src/drivers/intel/fsp2_0/memory_init.c:
https://review.coreboot.org/c/coreboot/+/75921/comment/292d178d_2b7c6b0c : PS3, Line 75: if (CONFIG(MRC_CACHE_USING_MRC_VERSION)) : do_cbmem_version_entry(CBMEM_ID_MRC_VERSION, version); : else : do_cbmem_version_entry(CBMEM_ID_FSPM_VERSION, version);
Given that the creation of MRC and FSPM version cbmem is mutually exclusive. Does it even make sense to have a different ID? Can't you rename the existing one to CBMEM_ID_FSPM_OR_MRC_VERSION?
its meaningful to have different IDs as they are associated with different string