Julius Werner 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 7:
(1 comment)
https://review.coreboot.org/#/c/31834/5/src/vendorcode/google/chromeos/watch... File src/vendorcode/google/chromeos/watchdog.c:
https://review.coreboot.org/#/c/31834/5/src/vendorcode/google/chromeos/watch... PS5, Line 51: if (!CONFIG(CHROMEOS_USE_EC_WATCHDOG_FLAG)) {
It seems like currently MTK WDT driver is the only one who calls mark_watchdog_tombstone... […]
Right, I missed the assertion. And your argument makes sense that we may want to leave it in the memlayout since other 8183 board may want to use it. But I'm also not really happy about the dissymmetry of setting but not using the tombstone.
How about making the two things completely independent? So you'd write the function above like this:
bool flag = false;
if (CONFIG(...EC...) flag |= google_chromeec_...();
if (REGION_SIZE(...)) { flag |= read32(...) == ...MAGIC; write32(..., 0); }
if (flag) elog_add_event();
So if a watchdog event was recorded through either of the two mechanism, we log it.