Attention is currently required from: Tarun Tuli, SRIDHAR SIRICILLA, Dinesh Gehlot, Kapil Porwal, Sridhar Siricilla.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74382 )
Change subject: soc/intel/cmn/cse: Move API to get FW partition info into cse_lite.c ......................................................................
Patch Set 3:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/74382/comment/070dfe59_dfc4d5b5 PS1, Line 9: The patch moves API that gets the CSE FW partition information into : CSE Lite specific file aka cse_lite.c because the consumer of this API : is the cse_lite specific ChromeOS devices hence, it's meaningful to : move the cse lite specific implementation inside cse_lite.c file.
cse.c represents library, provides API. I don't see any need to move the GET IMAGE FW INFO command handler into cse_lite.c. I request you to reconsider the option of moving the API into cse_lite.c. Thanks!
cse.c can't be a library because it has other pci ops entries. Today the R&R of cse. c and cse_lite.c are not very clear.
IMO, CSE.c is something which is common between cse consumer/corporate/lite sku where else cse_lite.c is specifically to meet the chromeos devices usecase.
If you wish to represent cse.c as a library then it has to be renamed as cselib.c like cpulib.c file and refactored accordingly.
Coming to the reason why i moved this API into cse_lite.c. seems like this whole patchsets and incremental patch train is very specific to CSE lite implementation and chromeos use case, where we would like to store the cse rw version after CSE sync being done (either in romstage or ramstage). Additionally, get ISH partition version also having the same dependency towards cse sync being done, when to call into it. what is the point of making it public when the other non-CSE lite aka consumer/corporate skus can't utilize the entire implementation. The last patch in this train is eventually making this API as static.
In future, if we felt other non-CSE lite users would like to use this API (which is not likely the case) then it can be declared as public.