You-Cheng Syu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31834 )
Change subject: google/chromeos: Support AP watchdog flag from Chrome EC ......................................................................
Patch Set 5:
(2 comments)
https://review.coreboot.org/#/c/31834/4/src/ec/google/chromeec/ec.c File src/ec/google/chromeec/ec.c:
https://review.coreboot.org/#/c/31834/4/src/ec/google/chromeec/ec.c@747 PS4, Line 747: #ifndef __PRE_RAM__
Do the functions above make sense if __PRE_RAM__ is defined?
In patchset 3, I make mtk_wdt_init() call google_chromeec_get_ap_watchdog_flag() (in CL:31843), and mtk_wdt_init should be called before ramstage.
In the current patchset, google_chromeec_get_ap_watchdog_flag() won't be called until ramstage. So yes we can move it back into __PRE_RAM__ scope. But I think there is no reason to forbid AP from asking EC for uptime_info/ap_watchdog_flag before ramstage. For example, google_chromeec_is_uhepi_supported() might be called in verstage. So I think communicating with EC before ramstage is allowed.
https://review.coreboot.org/#/c/31834/4/src/vendorcode/google/chromeos/watch... File src/vendorcode/google/chromeos/watchdog.c:
https://review.coreboot.org/#/c/31834/4/src/vendorcode/google/chromeos/watch... PS4, Line 43: elog_add_event(ELOG_TYPE_ASYNC_HW_TIMER_EXPIRED);
I find this code difficult to parse. […]
Done
The code in patchset 4 actually changed the behavior of always reseting _watchdog_tombstone to zero after reading...