Attention is currently required from: Andrey Petrov, Kapil Porwal, Ronak Kanabar, Subrata Banik, Tarun Tuli.
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/75919?usp=email )
Change subject: driver/{intel,mrc_cache}: Have MRC cache ver. based on FSP-M/MRC ver ......................................................................
Patch Set 3:
(1 comment)
File src/drivers/mrc_cache/Kconfig:
https://review.coreboot.org/c/coreboot/+/75919/comment/28ccefda_dacd8e4a : PS3, Line 14: help
This choice is user visible on ALL users of CACHE_MRC_SETTINGS, however not all users use FSP and the choice is not even meaningfull on all FSP platforms.
Is a user visible option even meaningful? The FSP either exposes extended header or it does not. The user should not have to figure this out. The platform should select the right option.
isn't the help msg is self explanatory ? i got your point but there is no need to give -1 for such reason (an unsolved comment itself is enough as in the latest patchset is not even +2'ed)
Yes but why should the user even have to read the help msg in the menu if it can be avoided.
I believe here the definition of the `user` is still a developer and not the real end user (i hope u agrees to this fact).
No I don't agree. For me the user is someone possibly not very knowledgeable or even someone less knowledgeable with the platform trying to navigate through the menu to get something working for a board. Every 'visible' option added is one more way to fail. Therefore less visible options is desirable.
And for the platform developer, we should have knowledge about what is the decision logic is used to store the MRC cache. For example: since last so many FSP generations of Intel based devices, we were using FSP version to associate with MRC cache because there was no way FSP would have provided the MRC version. What is the downside of this logic? the real end users are seeing some unnecessary black screen while we are updating the FW in-field devices.
No problem with new logic for MRC version. It just not something that someone building a rom should know about. It should be hidden and the soc should select the proper logic, depending on whether the associated FSP implements the logic.
The time we actually discovered that (since the mrc_cache being implemented till date we were not concerned about this), it took few months to land a proper solution (w/ help from Intel) which would help to improve the device experience so, my point is its still good to give some msgs/notifications to the developer to know what are the hidden underlying assumption while building the platform. And if develop is not sure, should leave this as is.
The FSP in use either implements the logic or not. Should the one building and configuring coreboot really have to investigate whether that is the case for his platforms. Can that decision not be made instead for him rather than having to ask it?