Attention is currently required from: Andrey Petrov, Arthur Heymans, Kapil Porwal, Ronak Kanabar, Tarun Tuli.
Subrata Banik 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/cd4deb3a_cc9801a1 : 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). 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.
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.
-1 gets removed on updating a patch iirc.
thanks for highlighting this.