Power Button: Wait for power button to be stable when waiting for release

The debounce timer might be too slow to actually update the state of
debounced_power_pressed by the time we do power_button_is_pressed in the S3->S5
state transition. Solution is to move the power_button_wait_for_release function
here and make sure there are no deferreds active.

BUG=chrome-os-partner:35948
TEST=During dev mode screen, press power button, note the device stays off
TEST=Print debounced_power_pressed in power_button_is_pressed(void), note it's
not 0 when power button is actually pressed
BRANCH=veyron

Change-Id: I8258e9e5524bd65d6ea9c77ea5649304d2195bf0
Signed-off-by: Alexandru M Stan <amstan@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/244590
Reviewed-by: Randall Spangler <rspangler@chromium.org>

3 files changed, 52 insertions, 34 deletions

diff --git a/common/power_button.c b/common/power_button.c
index ffa93d1027..993ad40a6c 100644
--- a/common/power_button.c
+++ b/common/power_button.c
@@ -13,6 +13,7 @@
 #include "keyboard_scan.h"
 #include "lid_switch.h"
 #include "power_button.h"
+#include "task.h"
 #include "timer.h"
 #include "util.h"
 
@@ -29,6 +30,7 @@
 
 static int debounced_power_pressed;	/* Debounced power button state */
 static int simulate_power_pressed;
+static volatile int power_button_is_stable = 1;
 
 /**
  * Return non-zero if power button signal asserted at hardware input.
@@ -69,6 +71,36 @@ int power_button_is_pressed(void)
 }
 
 /**
+ * Wait for the power button to be released
+ *
+ * @param timeout_us Timeout in microseconds, or -1 to wait forever
+ * @return EC_SUCCESS if ok, or
+ *         EC_ERROR_TIMEOUT if power button failed to release
+ */
+int power_button_wait_for_release(unsigned int timeout_us)
+{
+	timestamp_t deadline;
+	timestamp_t now = get_time();
+
+	deadline.val = now.val + timeout_us;
+
+	while (!power_button_is_stable || power_button_is_pressed()) {
+		now = get_time();
+		if (timeout_us < 0) {
+			task_wait_event(-1);
+		} else if (timestamp_expired(deadline, &now) ||
+			(task_wait_event(deadline.val - now.val) ==
+			TASK_EVENT_TIMER)) {
+			CPRINTS("power button not released in time");
+			return EC_ERROR_TIMEOUT;
+		}
+	}
+
+	CPRINTS("power button released in time");
+	return EC_SUCCESS;
+}
+
+/**
  * Handle power button initialization.
  */
 static void power_button_init(void)
@@ -93,10 +125,13 @@ static void power_button_change_deferred(void)
 		keyboard_scan_enable(1, KB_SCAN_DISABLE_POWER_BUTTON);
 
 	/* If power button hasn't changed state, nothing to do */
-	if (new_pressed == debounced_power_pressed)
+	if (new_pressed == debounced_power_pressed) {
+		power_button_is_stable = 1;
 		return;
+	}
 
 	debounced_power_pressed = new_pressed;
+	power_button_is_stable = 1;
 
 	CPRINTS("power button %s", new_pressed ? "pressed" : "released");
 
@@ -120,6 +155,7 @@ void power_button_interrupt(enum gpio_signal signal)
 		keyboard_scan_enable(0, KB_SCAN_DISABLE_POWER_BUTTON);
 
 	/* Reset power button debounce time */
+	power_button_is_stable = 0;
 	hook_call_deferred(power_button_change_deferred, PWRBTN_DEBOUNCE_US);
 }
 
@@ -139,12 +175,14 @@ static int command_powerbtn(int argc, char **argv)
 
 	ccprintf("Simulating %d ms power button press.\n", ms);
 	simulate_power_pressed = 1;
+	power_button_is_stable = 0;
 	hook_call_deferred(power_button_change_deferred, 0);
 
 	msleep(ms);
 
 	ccprintf("Simulating power button release.\n");
 	simulate_power_pressed = 0;
+	power_button_is_stable = 0;
 	hook_call_deferred(power_button_change_deferred, 0);
 
 	return EC_SUCCESS;
diff --git a/include/power_button.h b/include/power_button.h
index ce9f63e216..a7ec4612ad 100644
--- a/include/power_button.h
+++ b/include/power_button.h
@@ -18,6 +18,15 @@
 int power_button_is_pressed(void);
 
 /**
+ * Wait for the power button to be released
+ *
+ * @param timeout_us Timeout in microseconds, or -1 to wait forever
+ * @return EC_SUCCESS if ok, or
+ *         EC_ERROR_TIMEOUT if power button failed to release
+ */
+int power_button_wait_for_release(unsigned int timeout_us);
+
+/**
  * Return non-zero if power button signal asserted at hardware input.
  *
  */
diff --git a/power/rockchip.c b/power/rockchip.c
index 272f768615..cbb06fc2af 100644
--- a/power/rockchip.c
+++ b/power/rockchip.c
@@ -358,37 +358,6 @@ static void power_on(void)
 }
 
 /**
- * Wait for the power button to be released
- *
- * @param timeout_us Timeout in microseconds, or -1 to wait forever
- * @return EC_SUCCESS if ok, or
- *         EC_ERROR_TIMEOUT if power button failed to release
- */
-static int wait_for_power_button_release(unsigned int timeout_us)
-{
-	timestamp_t deadline;
-	timestamp_t now = get_time();
-
-	deadline.val = now.val + timeout_us;
-
-	while (power_button_is_pressed()) {
-		now = get_time();
-		if (timeout_us < 0) {
-			task_wait_event(-1);
-		} else if (timestamp_expired(deadline, &now) ||
-			(task_wait_event(deadline.val - now.val) ==
-			TASK_EVENT_TIMER)) {
-			CPRINTS("power button not released in time");
-			return EC_ERROR_TIMEOUT;
-		}
-	}
-
-	CPRINTS("power button released");
-	power_button_was_pressed = 0;
-	return EC_SUCCESS;
-}
-
-/**
  * Power off the AP
  */
 static void power_off(void)
@@ -475,9 +444,10 @@ enum power_state power_handle_state(enum power_state state)
 
 		if (power_wait_signals(IN_POWER_GOOD) == EC_SUCCESS) {
 			CPRINTS("POWER_GOOD seen");
-			if (wait_for_power_button_release(
+			if (power_button_wait_for_release(
 					DELAY_SHUTDOWN_ON_POWER_HOLD) ==
 					EC_SUCCESS) {
+				power_button_was_pressed = 0;
 				set_pmic_pwron(0);
 
 				/* setup misc gpio for S3/S0 functionality */
@@ -538,7 +508,8 @@ enum power_state power_handle_state(enum power_state state)
 		return POWER_S3;
 
 	case POWER_S3S5:
-		wait_for_power_button_release(-1);
+		power_button_wait_for_release(-1);
+		power_button_was_pressed = 0;
 		return POWER_S5;
 
 	case POWER_S5G3: