mt8186,mt8188: check the holder of AP_RST_ODL

To distinguish the AP shutdown, and the AP reset held by GSC/Servo,
we use the SYS_RST_ODL as the reference.

If AP_EC_SYSRST_ODL is asserted, but SYS_RST_ODL is not, this is a
normal shutdown.
If AP_EC_SYSRST_ODL is asserted, and so is SYS_RST_ODL, the AP reset
is held by GSC or Servo. In this case, we should let the power state
stay at S0 to prevent from an unexpected shutdown.

BUG=b:276229973
TEST=pass firmware_CorruptMinios
TEST=dut-control warm_reset:on sleep:1 warm:reset:off, it stays at S0
BRANCH=none

Change-Id: I9b69caa0d15e6e58e7f11ad1079536bc1954b5ce
Signed-off-by: Eric Yilun Lin <yllin@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/4402421
Tested-by: Eric Yilun Lin <yllin@google.com>
Reviewed-by: Ting Shen <phoenixshen@chromium.org>
Auto-Submit: Eric Yilun Lin <yllin@google.com>
Commit-Queue: Eric Yilun Lin <yllin@google.com>
Commit-Queue: Ting Shen <phoenixshen@chromium.org>

8 files changed, 42 insertions, 9 deletions

diff --git a/include/chipset.h b/include/chipset.h
index ead2a9680e..c5ba6ffd6f 100644
--- a/include/chipset.h
+++ b/include/chipset.h
@@ -221,7 +221,7 @@ void chipset_ap_rst_interrupt(enum gpio_signal signal);
 /**
  * GPIO interrupt handler of warm reset signal from servo or H1.
  *
- * It is used in Qualcomm chipset power sequence.
+ * It is used in Qualcomm/MediaTek chipset power sequence.
  */
 void chipset_warm_reset_interrupt(enum gpio_signal signal);
 
diff --git a/power/mt8186.c b/power/mt8186.c
index 297d86c317..9550a28c8a 100644
--- a/power/mt8186.c
+++ b/power/mt8186.c
@@ -80,6 +80,8 @@
 
 /* indicate MT8186 is processing a chipset reset. */
 static bool is_resetting;
+/* indicate MT8186 is AP reset is held by servo or GSC. */
+test_export_static bool is_held;
 /* indicate MT8186 is processing a AP forcing shutdown. */
 static bool is_shutdown;
 /* indicate MT8186 has been dropped to S5G3 from the last IN_AP_RST state . */
@@ -125,6 +127,20 @@ static void set_pmic_pwroff(void)
 	GPIO_SET_LEVEL(GPIO_EC_PMIC_EN_ODL, 1);
 }
 
+void chipset_warm_reset_interrupt(enum gpio_signal signal)
+{
+	/* If this is not a chipset_reset, the ap_rst must be held by gsc or
+	 * servo.
+	 */
+	if (!is_resetting && !gpio_get_level(GPIO_SYS_RST_ODL) &&
+	    !gpio_get_level(signal))
+		is_held = true;
+	else
+		is_held = false;
+
+	power_signal_interrupt(signal);
+}
+
 static void reset_request_interrupt_deferred(void)
 {
 	chipset_reset(CHIPSET_RESET_AP_REQ);
@@ -237,23 +253,27 @@ static void power_reset_host_sleep_state(void)
  *  G3 |         1 |                   x |               1|
  *
  * S5 is a temp stage, which will be put into G3 after s5_inactivity_timeout.
- * is_resetting flag indicate it's resetting chipset, and it's always S0.
+ * is_resetting flag indicates it's resetting chipset, and always S0.
+ * is_held flag indicates the AP reset is held by servo or GSC, and always S0.
  * is_shutdown flag indicates it's shutting down the AP, it goes for S5.
  * is_s5g3_passed flag indicates it has shutdown from S5 to G3 since last
  * shutdown.
  */
 static enum power_state power_get_signal_state(void)
 {
+	if (is_shutdown)
+		return POWER_S5;
 	/*
-	 * We are processing a chipset reset(S0->S0), so we don't check the
+	 * - We are processing a chipset reset(S0->S0), so we don't check the
 	 * power signals until the reset is finished. This is because
 	 * while the chipset is resetting, the intermediate power signal state
 	 * is not reflecting the current power state.
+	 *
+	 * - GSC or Servo is holding the SYS_RST, in this case, we should stay
+	 * at S0.
 	 */
-	if (is_resetting)
+	if (is_resetting || is_held)
 		return POWER_S0;
-	if (is_shutdown)
-		return POWER_S5;
 	if (power_get_signals() & IN_AP_RST) {
 		/* If it has been put to G3 from S5 idle, then stay at G3.*/
 		if (is_s5g3_passed)
@@ -470,6 +490,10 @@ enum power_state power_handle_state(enum power_state state)
 		hook_notify(HOOK_CHIPSET_SHUTDOWN_COMPLETE);
 
 		is_shutdown = false;
+		/* AP down and PMIC off, the servo and GSC is unable to hold the
+		 * AP for S0.
+		 */
+		is_held = false;
 		return POWER_S5;
 
 	case POWER_S5G3:
diff --git a/zephyr/program/corsola/ite_interrupts.dtsi b/zephyr/program/corsola/ite_interrupts.dtsi
index b99effb9eb..d3f718ec8c 100644
--- a/zephyr/program/corsola/ite_interrupts.dtsi
+++ b/zephyr/program/corsola/ite_interrupts.dtsi
@@ -44,7 +44,7 @@
 		int_ap_in_rst: ap_in_rst {
 			irq-pin = <&ap_sysrst_odl_r>;
 			flags = <GPIO_INT_EDGE_BOTH>;
-			handler = "power_signal_interrupt";
+			handler = "chipset_warm_reset_interrupt";
 		};
 		int_ap_wdtrst: ap_wdtrst {
 			irq-pin = <&ap_ec_wdtrst_l>;
diff --git a/zephyr/program/corsola/npcx_interrupts.dtsi b/zephyr/program/corsola/npcx_interrupts.dtsi
index 130f4501dd..c59d4b257e 100644
--- a/zephyr/program/corsola/npcx_interrupts.dtsi
+++ b/zephyr/program/corsola/npcx_interrupts.dtsi
@@ -48,7 +48,7 @@
 		int_ap_in_rst: ap_in_rst {
 			irq-pin = <&ap_sysrst_odl_r>;
 			flags = <GPIO_INT_EDGE_BOTH>;
-			handler = "power_signal_interrupt";
+			handler = "chipset_warm_reset_interrupt";
 		};
 		int_ap_wdtrst: ap_wdtrst {
 			irq-pin = <&ap_ec_wdtrst_l>;
diff --git a/zephyr/test/kingler/src/fakes.c b/zephyr/test/kingler/src/fakes.c
index 52befc59c4..fd06ab2eb1 100644
--- a/zephyr/test/kingler/src/fakes.c
+++ b/zephyr/test/kingler/src/fakes.c
@@ -19,6 +19,7 @@ FAKE_VOID_FUNC(switch_interrupt, enum gpio_signal);
 FAKE_VOID_FUNC(tcpc_alert_event, enum gpio_signal);
 FAKE_VOID_FUNC(ppc_interrupt, enum gpio_signal);
 FAKE_VOID_FUNC(bc12_interrupt, enum gpio_signal);
+FAKE_VOID_FUNC(chipset_warm_reset_interrupt, enum gpio_signal);
 #ifndef CONFIG_TEST_KINGLER_CCD
 FAKE_VOID_FUNC(ccd_interrupt, enum gpio_signal);
 #endif
diff --git a/zephyr/test/krabby/prj.conf b/zephyr/test/krabby/prj.conf
index 3de743c076..58b208ef7d 100644
--- a/zephyr/test/krabby/prj.conf
+++ b/zephyr/test/krabby/prj.conf
@@ -38,3 +38,5 @@ CONFIG_PLATFORM_EC_USB_POWER_DELIVERY=y
 CONFIG_PLATFORM_EC_VBOOT_HASH=n
 
 CONFIG_PLATFORM_EC_USBC_PPC=y
+
+CONFIG_NATIVE_UART_0_ON_STDINOUT=y
diff --git a/zephyr/test/krabby/src/fake.c b/zephyr/test/krabby/src/fake.c
index 9baff6f74d..5c83e1cb88 100644
--- a/zephyr/test/krabby/src/fake.c
+++ b/zephyr/test/krabby/src/fake.c
@@ -12,6 +12,7 @@
 FAKE_VOID_FUNC(button_interrupt, enum gpio_signal);
 FAKE_VOID_FUNC(chipset_reset_request_interrupt, enum gpio_signal);
 FAKE_VOID_FUNC(power_signal_interrupt, enum gpio_signal);
+FAKE_VOID_FUNC(chipset_warm_reset_interrupt, enum gpio_signal);
 FAKE_VOID_FUNC(chipset_watchdog_interrupt, enum gpio_signal);
 FAKE_VOID_FUNC(extpower_interrupt, enum gpio_signal);
 FAKE_VOID_FUNC(usb_a0_interrupt, enum gpio_signal);
diff --git a/zephyr/test/krabby/src/power_seq.c b/zephyr/test/krabby/src/power_seq.c
index d01fbad035..de88342831 100644
--- a/zephyr/test/krabby/src/power_seq.c
+++ b/zephyr/test/krabby/src/power_seq.c
@@ -32,6 +32,9 @@ FAKE_VALUE_FUNC(int, system_jumped_late);
 
 #define S5_INACTIVE_SEC 11
 
+/* mt8186 is_held flag */
+extern bool is_held;
+
 static void set_signal_state(enum power_state state)
 {
 	const struct gpio_dt_spec *ap_ec_sysrst_odl =
@@ -59,6 +62,8 @@ static void set_signal_state(enum power_state state)
 		zassert_unreachable("state %d not supported", state);
 	}
 
+	/* reset is_held flag */
+	is_held = false;
 	task_wake(TASK_ID_CHIPSET);
 	k_sleep(K_SECONDS(1));
 }
@@ -69,8 +74,8 @@ static void power_seq_before(void *f)
 	set_test_runner_tid();
 
 	/* Start from G3 */
-	set_signal_state(POWER_G3);
 	power_set_state(POWER_G3);
+	set_signal_state(POWER_G3);
 	k_sleep(K_SECONDS(S5_INACTIVE_SEC));
 
 	RESET_FAKE(chipset_pre_init_hook);