Attention is currently required from: Bill XIE.
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78288?usp=email )
The change is no longer submittable: All-Comments-Resolved is unsatisfied now.
Change subject: drivers/pc80/rtc/option.c: Stop resetting CMOS during s3 resume ......................................................................
Patch Set 5:
(2 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/78288/comment/acb7ac1e_f6bca475 : PS5, Line 17: "untrained" result restored, so s3 resume will fail. AFAICS, sanitize_cmos() never cleared/wrote bank 1 (bytes 128 to 255) prior to the referenced commit.
With this commit, normal boot path will clear both bank 0 and bank 1 on cmos_error(). To me this sound like the correct think to do, but it is not mentioned in the commit message. Also, a failing cmos_lb_cks_valid() that covers bank 0 data only, will clear bank 1 too on errors now?
https://review.coreboot.org/c/coreboot/+/78288/comment/de84b052_5be6c0d6 : PS5, Line 20: cmos_need_reset will be negated when acpi_is_wakeup_s3() returns true. We should not be allowing get_option() to target CMOS NVRAM, whose checksum was not confirmed.
Would it be possible to define that offsets after LB_CKS_LOC are not to be cleared with STATIC_OPTION_TABLE=y ?