Attention is currently required from: Tim Wawrzynczak, Patrick Rudolph. Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56617 )
Change subject: soc/intel/alderlake: Clear RTC_BATTERY_DEAD ......................................................................
Patch Set 2:
(1 comment)
File src/soc/intel/alderlake/pmutil.c:
https://review.coreboot.org/c/coreboot/+/56617/comment/edd2c212_baa5c399 PS1, Line 188: soc_get_rtc_failed
Done
This series looks okay to me. I am wondering if there is scope for us to simplify things somewhat.
Currently, there are two paths in coreboot:
1. verstage: Checks if CMOS failed (and clears failed flag). If yes, then it calls cmos_init.
2. ramstage: Checks if CMOS failed (and clears failed flag). If yes, then it calls cmos_check_update_date and cmos_init.
Should this instead be such that:
1. bootblock checks for CMOS failed and takes all appropriate steps to fix that and clears RTC failed flag.
2. verstage wouldn't check for CMOS failure at all. Instead it has its own checksum that it does to see if VBNV contents are valid. If not, it restores from backup (This checksum and restore is already done in `read_vbnv_cmos()`.
3. ramstage doesn't need to do anything special.
With this, bootblock always inits CMOS correctly and this doesn't need to be handled in different stages. Thoughts?
(Also, the same needs to be aligned on AMD platforms as well).