Attention is currently required from: Shelley Chen, Furquan Shaikh, Paul Menzel, Julius Werner. Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52291 )
Change subject: drivers/mrc_cache: Return -1 on stash data failure ......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
I believe the basic idea is just that there's not really anything useful to do when the stashing fai […]
I didn't see any error on asurada. It's all about showing the right messages.
For asurada (without recovery mrc cache), mrc_cache_stash_data() always returns success, but in recovery mode there is actually nothing being written to flash. This makes the current log "DRAM-K: Calibration params saved to flash" misleading.
In my first version of CL (not showing here), update_mrc_cache_by_type() would return -1 when lookup_region() failed. Then I realized that was inconsistent with the MRC_STASH_TO_CBMEM variant, so I changed to return value to 0 (as in PS1 here), while completely forgetting about my original purpose.
It looks like we have 2 options to avoid the misleading log:
(a) Ignore the return value of mrc_cache_stash_data() from platform code. That is, let mrc_cache print all the logs. BTW, I wonder if we can make mrc_cache_stash_data() return void. For MRC_STASH_TO_CBMEM, does the return value matter? It looks like some platform (soc/amd/common/block/s3/s3_resume.c) will check the return value.
(b) Change the meaning of return value for mrc_cache_stash_data(): return 0 if and only if the data is successfully written to flash/cbmem. This is basically my "first version of CL" described above.