Fix poweron state machine in the EC

The existing state machine does the following:
- when power button is pressed it
1. powers on the AP
2. sets a timer of 1 sec and then
3. waits for power button to be released

When the timer fires it checks xpshold is set by the AP and if so
it clears the pwron signal (which is used by the AP to detect power
button is pressed).

The problem occurs when the user holds the power button for more than a
second.  The AP turns on xpshold, then notices that pwron is still on
and subsequently powers down because it thinks the power button is
pressed.  When the button is finally released, since it was held down
for more than a second, the timer routine notices that xpshold is not on
and therefore shuts down the system.

Another problem found while analysing this state machine is that loop
checking for poweroff only triggers on the rising edge of xpshold.  This
means that if the AP powers down the EC might miss a possible power
event.

Here is the proposed fix:
When the power button is pressed the EC will:
1. power on the AP
2. Check for xpshold to be asserted with a 1 sec timeout
3. If uboot is healthy xpshold should come on pretty quickly; the EC
then waits for the power button to be released in less than 8 seconds
4. If the power button is released then the EC waits for power off
events.
5. If the power button is not released it waits for upto 8 seconds
before turning off the AP.

The added wrinkle is how to address a borked uboot case.  In the case
where xpshold doesn't come on in < 1 second, the EC will allow the AP to
stay on for upto 16 seconds so that USB boot can finish.  The user must
hold the power button down until uboot boots and sets xpshold.  The
assumption here is that USB boot takes < 16 seconds.

BUG=chrome-os-partner:12748
TEST="follow instructions in bug report"

Change-Id: I5b582a6c3ae3449238e2813e4a581bd8f92dd846
Signed-off-by: Puneet Kumar <puneetster@chromium.org>
Reviewed-on: https://gerrit.chromium.org/gerrit/31291
Reviewed-by: Vincent Palatin <vpalatin@chromium.org>
Reviewed-by: David Hendricks <dhendrix@chromium.org>

2 files changed, 51 insertions, 53 deletions

diff --git a/board/snow/board.c b/board/snow/board.c
index 4575a17a40..b58e8cde86 100644
--- a/board/snow/board.c
+++ b/board/snow/board.c
@@ -40,7 +40,7 @@ const struct gpio_info gpio_list[GPIO_COUNT] = {
 	/* Inputs with interrupt handlers are first for efficiency */
 	{"KB_PWR_ON_L", GPIO_B, (1<<5),  GPIO_INT_BOTH, gaia_power_event},
 	{"PP1800_LDO2", GPIO_A, (1<<1),  GPIO_INT_BOTH, gaia_power_event},
-	{"XPSHOLD",     GPIO_A, (1<<3),  GPIO_INT_RISING, gaia_power_event},
+	{"XPSHOLD",     GPIO_A, (1<<3),  GPIO_INT_BOTH, gaia_power_event},
 	{"CHARGER_INT", GPIO_C, (1<<4),  GPIO_INT_FALLING, pmu_irq_handler},
 	{"LID_OPEN",    GPIO_C, (1<<13), GPIO_INT_RISING, gaia_lid_event},
 	{"SUSPEND_L",   GPIO_A, (1<<7),  GPIO_INT_BOTH, gaia_suspend_event},
diff --git a/common/gaia_power.c b/common/gaia_power.c
index 42b294b234..28e22808c0 100644
--- a/common/gaia_power.c
+++ b/common/gaia_power.c
@@ -16,7 +16,7 @@
  *  it off until pwron is released and pressed again
  *
  *  When powered on:
- *  - The PMIC PWRON signal is released one second after the power button is
+ *  - The PMIC PWRON signal is released <= 1 second after the power button is
  *  released (we expect that U-Boot as asserted XPSHOLD by then)
  *  - Holding pwron for 8s powers off the AP
  *  - Pressing and releasing pwron within that 8s is ignored
@@ -63,13 +63,14 @@
  *    into the inner loop, waiting for next event to occur (power button
  *    press or XPSHOLD == 0).
  *
- * U-Boot updating: User presses and holds power button. EC does not
- *    check XPSHOLD, and waits up to 16sec for an event. If no event occurs
+ * U-Boot updating: User presses and holds power button. If EC does not
+ *    see XPSHOLD, it waits up to 16sec for an event. If no event occurs
  *    within 16sec, EC powers off AP.
  */
-#define DELAY_SHUTDOWN_ON_POWER_HOLD	(16 * 1000000)
+#define DELAY_SHUTDOWN_ON_POWER_HOLD	(8 * 1000000)
+#define DELAY_SHUTDOWN_ON_USB_BOOT      (16 * 1000000)
 
-/* Delay after power button release before we release GPIO_PMIC_PWRON_L */
+/* Maximum delay after power button press before we release GPIO_PMIC_PWRON_L */
 #define DELAY_RELEASE_PWRON	1000000 /* 1s */
 
 /* debounce time to prevent accidental power-on after keyboard power off */
@@ -102,12 +103,6 @@ static char lid_changed;
 /* time where we will power off, if power button still held down */
 static timestamp_t power_off_deadline;
 
-/* 1 if we have released GPIO_PMIC_PWRON_L */
-static int pwron_released;
-
-/* time where we will release GPIO_PMIC_PWRON_L */
-static timestamp_t pwron_deadline;
-
 /* force AP power on (used for recovery keypress) */
 static int auto_power_on;
 
@@ -198,10 +193,11 @@ static int check_for_power_off_event(void)
 		CPUTS("Cancel power off\n");
 		gpio_set_level(GPIO_PMIC_PWRON_L, 1);
 	}
+
 	power_button_was_pressed = pressed;
 
 	/* XPSHOLD released by AP : shutdown immediately */
-	if (pwron_released && gpio_get_level(GPIO_SOC1V8_XPSHOLD) == 0)
+	if (gpio_get_level(GPIO_SOC1V8_XPSHOLD) == 0)
 		return 3;
 
 	if (power_request == POWER_REQ_OFF) {
@@ -415,6 +411,26 @@ static int wait_for_power_button_release(unsigned int timeout_us)
 }
 
 /**
+ * Wait for the XPSHOLD signal from the AP to be asserted within timeout_us
+ * and if asserted clear the PMIC_PWRON signal
+ *
+ * @return 0 if ok, -1 if power button failed to release
+ */
+static int react_to_xpshold(unsigned int timeout_us)
+{
+	/* wait for Power button release */
+	wait_in_signal(GPIO_SOC1V8_XPSHOLD, 1, timeout_us);
+
+	if (gpio_get_level(GPIO_SOC1V8_XPSHOLD) == 0) {
+		CPUTS("XPSHOLD not seen in time\n");
+		return -1;
+	}
+	CPRINTF("%T XPSHOLD seen\n");
+	gpio_set_level(GPIO_PMIC_PWRON_L, 1);
+	return 0;
+}
+
+/**
  * Power off the AP
  */
 static void power_off(void)
@@ -433,26 +449,6 @@ static void power_off(void)
 	CPUTS("Shutdown complete.\n");
 }
 
-/**
- * Set a timer to release GPIO_PMIC_PWRON_L in the future
- */
-static void set_pwron_timer(void)
-{
-	pwron_deadline = get_time();
-	pwron_deadline.val += DELAY_RELEASE_PWRON;
-	CPRINTF("Setting pwron timer %d\n", pwron_deadline.val);
-	pwron_released = 0;
-}
-
-static void check_pwron_release(void)
-{
-	if (!pwron_released && timestamp_expired(pwron_deadline, NULL)) {
-		pwron_deadline.val = 0;
-		pwron_released = 1;
-		gpio_set_level(GPIO_PMIC_PWRON_L, 1);
-		CPRINTF("Releasing pwron\n");
-	}
-}
 
 /*
  * Calculates the delay in microseconds to the next time we have to check
@@ -462,15 +458,10 @@ static void check_pwron_release(void)
  */
 static int next_pwr_event(void)
 {
-	uint64_t next;
-
-	if (!pwron_deadline.val && !power_off_deadline.val)
+	if (!power_off_deadline.val)
 		return -1;
 
-	/* We know that pwron_deadline will be earlier, if it exists */
-	next = pwron_deadline.val ? pwron_deadline.val
-			: power_off_deadline.val;
-	return next - get_time().val;
+	return power_off_deadline.val - get_time().val;
 }
 
 
@@ -488,20 +479,27 @@ void gaia_power_task(void)
 		while (!check_for_power_on_event())
 			task_wait_event(-1);
 
-		/*
-		 * If we can power on, and the power button is released,
-		 * start running!
-		 */
-		if (!power_on() && !wait_for_power_button_release(
-					DELAY_SHUTDOWN_ON_POWER_HOLD)) {
-			/* Wait until we need to power off, then power off */
-			power_button_was_pressed = 0;
-			set_pwron_timer();
-			while (value = check_for_power_off_event(), !value) {
-				check_pwron_release();
-				task_wait_event(next_pwr_event());
+		if (!power_on()) {
+			int continue_power = 0;
+
+			if (!react_to_xpshold(DELAY_RELEASE_PWRON)) {
+				/* AP looks good */
+				if (!wait_for_power_button_release(
+					DELAY_SHUTDOWN_ON_POWER_HOLD))
+					continue_power = 1;
+			} else {
+				/* AP is possibly in bad shape */
+				/* allow USB boot in 16 secs */
+				if (!wait_for_power_button_release(
+					DELAY_SHUTDOWN_ON_USB_BOOT))
+					continue_power = 1;
+			}
+			if (continue_power) {
+				power_button_was_pressed = 0;
+				while (!(value = check_for_power_off_event()))
+					task_wait_event(next_pwr_event());
+				CPRINTF("%T ending loop %d\n", value);
 			}
-			CPRINTF("ending loop %d\n", value);
 		}
 		power_off();
 		wait_for_power_button_release(-1);