power: rk3399: Debounce PGOOD_AP signal

PGOOD_AP may go low for a period < 100ms during regulator output voltage
transitions, so ignore such pulses.

BRANCH=None
BUG=chrome-os-partner:54814
TEST=On kevin, verify suspend / resume succeeds for 10 cycles.

Change-Id: I5b6240a570472e1ea74de6e5f2341472ea7afe6b
Reviewed-on: https://chromium-review.googlesource.com/374524
Commit-Ready: Shawn N <shawnn@chromium.org>
Tested-by: Douglas Anderson <dianders@chromium.org>
Tested-by: Shunqian Zheng <zhengsq@rock-chips.com>
Tested-by: Shawn N <shawnn@chromium.org>
Reviewed-by: Aseda Aboagye <aaboagye@chromium.org>

3 files changed, 52 insertions, 8 deletions

diff --git a/include/power.h b/include/power.h
index ab8d7ac46a..b81a952156 100644
--- a/include/power.h
+++ b/include/power.h
@@ -68,7 +68,7 @@ uint32_t power_get_signals(void);
 int power_has_signals(uint32_t want);
 
 /**
- * Wait for power input signals to be present
+ * Wait for power input signals to be present using default timeout
  *
  * @param want		Mask of signals which must be present (one or more
  *			POWER_SIGNAL_MASK()s).  If want=0, stops waiting for
@@ -79,6 +79,18 @@ int power_has_signals(uint32_t want);
 int power_wait_signals(uint32_t want);
 
 /**
+ * Wait for power input signals to be present
+ *
+ * @param want		Mask of signals which must be present (one or more
+ *			POWER_SIGNAL_MASK()s).  If want=0, stops waiting for
+ *			signals.
+ * @param timeout       Timeout in usec to wait for signals to be present.
+ * @return EC_SUCCESS when all inputs are present, or ERROR_TIMEOUT if timeout
+ * before reaching the desired state.
+ */
+int power_wait_signals_timeout(uint32_t want, int timeout);
+
+/**
  * Set the low-level power chipset state.
  *
  * @param new_state New chipset state.
diff --git a/power/common.c b/power/common.c
index 1916758482..b0d474831d 100644
--- a/power/common.c
+++ b/power/common.c
@@ -134,16 +134,23 @@ int power_has_signals(uint32_t want)
 
 int power_wait_signals(uint32_t want)
 {
+	int ret = power_wait_signals_timeout(want, DEFAULT_TIMEOUT);
+
+	if (ret == EC_ERROR_TIMEOUT)
+		CPRINTS("power timeout on input; wanted 0x%04x, got 0x%04x",
+			want, in_signals & want);
+	return ret;
+}
+
+int power_wait_signals_timeout(uint32_t want, int timeout)
+{
 	in_want = want;
 	if (!want)
 		return EC_SUCCESS;
 
 	while ((in_signals & in_want) != in_want) {
-		if (task_wait_event(DEFAULT_TIMEOUT) == TASK_EVENT_TIMER) {
+		if (task_wait_event(timeout) == TASK_EVENT_TIMER) {
 			power_update_signals();
-			CPRINTS("power timeout on input; "
-				"wanted 0x%04x, got 0x%04x",
-				in_want, in_signals & in_want);
 			return EC_ERROR_TIMEOUT;
 		}
 		/*
diff --git a/power/rk3399.c b/power/rk3399.c
index 76b012c548..58056b98dc 100644
--- a/power/rk3399.c
+++ b/power/rk3399.c
@@ -91,6 +91,12 @@ static void force_shutdown(void)
 }
 DECLARE_DEFERRED(force_shutdown);
 
+/*
+ * Debounce PGOOD_AP if we lose it suddenly during S0, since output voltage
+ * transitions may cause spurious pulses.
+ */
+#define PGOOD_AP_DEBOUNCE_TIMEOUT (100 * MSEC)
+
 enum power_state power_handle_state(enum power_state state)
 {
 	static int sys_reset_needed;
@@ -110,15 +116,34 @@ enum power_state power_handle_state(enum power_state state)
 	case POWER_S3:
 		if (!power_has_signals(IN_PGOOD_S3) || forcing_shutdown)
 			return POWER_S3S5;
-		else if (!gpio_get_level(GPIO_AP_EC_S3_S0_L))
+		else if (power_get_signals() & IN_SUSPEND_DEASSERTED)
 			return POWER_S3S0;
 		break;
 
 	case POWER_S0:
-		if (!power_has_signals(IN_PGOOD_S0) ||
+		if (!power_has_signals(IN_PGOOD_S3) ||
+		    forcing_shutdown ||
+		    !(power_get_signals() & IN_SUSPEND_DEASSERTED))
+			return POWER_S0S3;
+
+		/*
+		 * Wait up to PGOOD_AP_DEBOUNCE_TIMEOUT for IN_PGOOD_AP to
+		 * come back before transitioning back to S3.
+		 */
+		if (power_wait_signals_timeout(IN_PGOOD_AP,
+					       PGOOD_AP_DEBOUNCE_TIMEOUT)
+					       == EC_ERROR_TIMEOUT)
+			return POWER_S0S3;
+
+		/*
+		 * power_wait_signals_timeout() can block and consume task
+		 * wake events, so re-verify the state of the world.
+		 */
+		if (!power_has_signals(IN_PGOOD_S3) ||
 		    forcing_shutdown ||
-		    gpio_get_level(GPIO_AP_EC_S3_S0_L))
+		    !(power_get_signals() & IN_SUSPEND_DEASSERTED))
 			return POWER_S0S3;
+
 		break;
 
 	case POWER_G3S5: