it8xxx2: don't set system reset flags directly

The get_reset_cause method of the cros_system driver is meant to only
return the cause. Setting the system reset flags directly there (rather
than allowing the caller to adjust them) is incorrect and causes
confusing behavior like reporting multiple mutually-incompatible reset
causes in `sysinfo` command output.

The tests for causes are reordered to return the "most fundamental"
reset event that could have occurred, preferring power-on then reset pin
then watchdog. It's not generally possible to determine which reset
actually occurred because the registers appear to only be cleared when
written by software, so this assumes that a set bit indicates a reset of
that form occurred and hasn't yet been noted by the firmware.

The twiddling of the watchdog bit is removed from the function
completely, since it didn't do anything: system_set_reset_flags will
only set bits in the flags, so attempting to clear
EC_RESET_FLAG_WATCHDOG had no effect.

BUG=b:227972919
TEST=Reset causes on Nereid now correctly reflect the actual last reset
     in sysinfo command output and do not indicate multiple causes.
BRANCH=none

Change-Id: I28fd125dfb476ba451c07622478345dd3b61f3cd
Signed-off-by: Peter Marheine <pmarheine@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/3606030
Reviewed-by: Dino Li <dino.li@ite.corp-partner.google.com>
Reviewed-by: Andrew McRae <amcrae@google.com>

1 files changed, 15 insertions, 26 deletions

diff --git a/zephyr/drivers/cros_system/cros_system_it8xxx2.c b/zephyr/drivers/cros_system/cros_system_it8xxx2.c
index 4e49ab85b2..668caa9987 100644
--- a/zephyr/drivers/cros_system/cros_system_it8xxx2.c
+++ b/zephyr/drivers/cros_system/cros_system_it8xxx2.c
@@ -80,41 +80,30 @@ static int cros_system_it8xxx2_get_reset_cause(const struct device *dev)
 {
 	ARG_UNUSED(dev);
 	struct gctrl_it8xxx2_regs *const gctrl_base = GCTRL_IT8XXX2_REG_BASE;
-	/* system reset flag */
-	uint32_t system_flags = chip_read_reset_flags();
-	int chip_reset_cause = 0;
-	uint8_t raw_reset_cause = gctrl_base->GCTRL_RSTS & IT8XXX2_GCTRL_LRS;
+	uint8_t last_reset_source = gctrl_base->GCTRL_RSTS & IT8XXX2_GCTRL_LRS;
 	uint8_t raw_reset_cause2 = gctrl_base->GCTRL_SPCTRL4 &
 		(IT8XXX2_GCTRL_LRSIWR | IT8XXX2_GCTRL_LRSIPWRSWTR |
 		IT8XXX2_GCTRL_LRSIPGWR);
 
 	/* Clear reset cause. */
 	gctrl_base->GCTRL_RSTS |= IT8XXX2_GCTRL_LRS;
-	gctrl_base->GCTRL_SPCTRL4 |= (IT8XXX2_GCTRL_LRSIWR |
-		IT8XXX2_GCTRL_LRSIPWRSWTR | IT8XXX2_GCTRL_LRSIPGWR);
-
-	/* Determine if watchdog reset or power on reset. */
-	if (raw_reset_cause & IT8XXX2_GCTRL_IWDTR) {
-		system_flags |= EC_RESET_FLAG_WATCHDOG;
-		chip_reset_cause = WATCHDOG_RST;
-	} else if (raw_reset_cause < 2) {
-		system_flags |= EC_RESET_FLAG_POWER_ON;
-		chip_reset_cause = POWERUP;
+	gctrl_base->GCTRL_SPCTRL4 |=
+		(IT8XXX2_GCTRL_LRSIWR | IT8XXX2_GCTRL_LRSIPWRSWTR |
+		 IT8XXX2_GCTRL_LRSIPGWR);
+
+	if (last_reset_source & IT8XXX2_GCTRL_IWDTR) {
+		return WATCHDOG_RST;
 	}
-	/* Determine reset-pin reset. */
 	if (raw_reset_cause2 & IT8XXX2_GCTRL_LRSIWR) {
-		system_flags |= EC_RESET_FLAG_RESET_PIN;
-		chip_reset_cause = VCC1_RST_PIN;
+		/*
+		 * We can't differentiate between power-on and reset pin because
+		 * LRSIWR is set on both ~WRST assertion and power-on, and LRS
+		 * is either 0 or 1 in both cases. Conservatively treat both as
+		 * power-on.
+		 */
+		return POWERUP;
 	}
-
-	/* watchdog module triggers these reset */
-	if (system_flags & (EC_RESET_FLAG_HARD | EC_RESET_FLAG_SOFT))
-		system_flags &= ~EC_RESET_FLAG_WATCHDOG;
-
-	/* Set the system reset flags. */
-	system_set_reset_flags(system_flags);
-
-	return chip_reset_cause;
+	return UNKNOWN_RST;
 }
 
 static int cros_system_it8xxx2_init(const struct device *dev)