Attention is currently required from: Bill XIE, Kyösti Mälkki.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78881?usp=email )
Change subject: pc80/rtc: Clear CMOS on errors on S3 resume too ......................................................................
Patch Set 1:
(1 comment)
File src/drivers/pc80/rtc/option.c:
https://review.coreboot.org/c/coreboot/+/78881/comment/03ba4dc7_e2fac200 : PS1, Line 204: (CONFIG(STATIC_OPTION_TABLE) && !acpi_is_wakeup_s3())
During CB:78288 I Have considered this, and decided to defer CMOS reset on error to the next boot, for CMOS reset during s3 on GM45 platforms will erase their DRAM training data, making s3 resume fail.
Yes, but what are the chances that GM45 could continue booting without resetting CMOS? And multiply that with the chance that an error occurs in the first place. I don't think it matters more than returning to a known state, on error.
Do we really need to reset CMOS on error during s3 resume?
We could as well ask "Do we really need to reset CMOS ever?". CMOS is read during resume as it is on any boot. So it boils down to the question if we prefer a known default state over a potentially corrupted one. I don't see why we should make an exception for S3 resume.
-
Nico Huber (Code Review)