Refactor boot key detection

Keyboard scan module now owns the recovery key state on all platforms.

And clean up a few comments to linux kernel style

BUG=chrome-os-partner:10890
TEST=manual
- Power on system.  Should boot normally.
- Power+Refresh+D.  Should turn dev switch on.
- Power+Refresh+F.  Should turn dev switch off.
- Power+Esc.  Should reboot system.  Power button should power on normally.
- Power+Refresh+Esc.  Should power on into recovery mode.
- Then press power to shut system down.
- Power button should power on normally (not back into recovery mode).

Change-Id: I4d16e1e8b039efeacbd41e8acec115844bc8457d
Signed-off-by: Randall Spangler <rspangler@chromium.org>
Reviewed-on: https://gerrit.chromium.org/gerrit/26147
Reviewed-by: Simon Glass <sjg@chromium.org>

5 files changed, 143 insertions, 101 deletions

diff --git a/chip/lm4/keyboard_scan.c b/chip/lm4/keyboard_scan.c
index a74dd952c4..4abb8c7622 100644
--- a/chip/lm4/keyboard_scan.c
+++ b/chip/lm4/keyboard_scan.c
@@ -28,9 +28,21 @@
 
 #define KB_COLS 13
 
+/* Boot key list.  Must be in same order as enum boot_key. */
+struct boot_key_entry {
+	uint8_t mask_index;
+	uint8_t mask_value;
+};
+const struct boot_key_entry boot_key_list[] = {
+	{0, 0x00},
+	{1, 0x02},
+	{2, 0x10},
+	{3, 0x10},
+	{11, 0x40},
+};
 
 static uint8_t raw_state[KB_COLS];
-static uint8_t raw_state_at_boot[KB_COLS];
+enum boot_key boot_key_value = BOOT_KEY_OTHER;
 
 /* Mask with 1 bits only for keys that actually exist */
 static const uint8_t *actual_key_mask;
@@ -45,15 +57,8 @@ static const uint8_t actual_key_masks[4][KB_COLS] = {
 	};
 
 /* Key masks for special boot keys */
-#define MASK_INDEX_ESC     1
-#define MASK_VALUE_ESC     0x02
 #define MASK_INDEX_REFRESH 2
 #define MASK_VALUE_REFRESH 0x04
-#define MASK_INDEX_D       2
-#define MASK_VALUE_D       0x10
-#define MASK_INDEX_F       3
-#define MASK_VALUE_F       0x10
-
 
 static void wait_for_interrupt(void)
 {
@@ -67,7 +72,6 @@ static void wait_for_interrupt(void)
 	lm4_enable_matrix_interrupt();
 }
 
-
 static void enter_polling_mode(void)
 {
 	CPUTS("[KB poll]\n");
@@ -75,10 +79,11 @@ static void enter_polling_mode(void)
 	lm4_select_column(COLUMN_TRI_STATE_ALL);
 }
 
-
-/* Update the raw key state without sending messages.  Used in pre-init, so
+/*
+ * Update the raw key state without sending messages.  Used in pre-init, so
  * must not make task-switching-dependent calls; udelay() is ok because it's a
- * spin-loop. */
+ * spin-loop.
+ */
 static void update_key_state(void)
 {
 	int c;
@@ -99,7 +104,6 @@ static void update_key_state(void)
 	lm4_select_column(COLUMN_TRI_STATE_ALL);
 }
 
-
 /* Print the raw keyboard state. */
 static void print_raw_state(const char *msg)
 {
@@ -115,7 +119,6 @@ static void print_raw_state(const char *msg)
 	CPUTS("]\n");
 }
 
-
 /* Return 1 if any key is still pressed, 0 if no key is pressed. */
 static int check_keys_changed(void)
 {
@@ -132,13 +135,17 @@ static int check_keys_changed(void)
 		r = lm4_read_raw_row_state();
 		/* Invert it so 0=not pressed, 1=pressed */
 		r ^= 0xff;
-		/* Mask off keys that don't exist so they never show
-		 * as pressed */
+		/*
+		 * Mask off keys that don't exist so they never show as
+		 * pressed.
+		 */
 		r &= actual_key_mask[c];
 
 #ifdef OR_WITH_CURRENT_STATE_FOR_TESTING
-		/* KLUDGE - or current state in, so we can make sure
-		 * all the lines are hooked up */
+		/*
+		 * KLUDGE - or current state in, so we can make sure
+		 * all the lines are hooked up.
+		 */
 		r |= raw_state[c];
 #endif
 
@@ -151,11 +158,13 @@ static int check_keys_changed(void)
 		if (!keys[c])
 			continue;
 		for (c2 = c + 1; c2 < KB_COLS; c2++) {
-			/* A little bit of cleverness here.  Ghosting happens
+			/*
+			 * A little bit of cleverness here.  Ghosting happens
 			 * if 2 columns share at least 2 keys.  So we OR the
 			 * columns together and then see if more than one bit
-			 * is set.  x&(x-1) is non-zero only if x has more
-			 * than one bit set. */
+			 * is set.  x&(x-1) is non-zero only if x has more than
+			 * one bit set.
+			 */
 			uint8_t common = keys[c] & keys[c2];
 
 			if (common & (common - 1))
@@ -191,32 +200,49 @@ out:
 	return 0;
 }
 
-
-/* Returns non-zero if the specified key is pressed, and only other allowed
- * keys are pressed. */
-static int check_boot_key(int index, int mask)
+/*
+ * Return non-zero if the specified key is pressed, with at most the keys used
+ * for keyboard-controlled reset also pressed.
+ */
+static int check_key(int index, int mask)
 {
 	uint8_t allowed_mask[KB_COLS] = {0};
 	int c;
 
 	/* Check for the key */
-	if (!(raw_state_at_boot[index] & mask))
+	if (mask && !(raw_state[index] & mask))
 		return 0;
 
-	/* Make sure only other allowed keys are pressed.  This protects
-	 * against accidentally triggering the special key when a cat sits on
-	 * your keyboard.  Currently, only the requested key and the keys used
-	 * for the Silego reset are allowed. */
+	/* Check for other allowed keys */
 	allowed_mask[index] |= mask;
 	allowed_mask[MASK_INDEX_REFRESH] |= MASK_VALUE_REFRESH;
 
 	for (c = 0; c < KB_COLS; c++) {
-		if (raw_state_at_boot[c] & ~allowed_mask[c])
-			return 0;  /* Additional disallowed key pressed */
+		if (raw_state[c] & ~allowed_mask[c])
+			return 0;  /* Disallowed key pressed */
 	}
 	return 1;
 }
 
+enum boot_key keyboard_scan_get_boot_key(void)
+{
+	return boot_key_value;
+}
+
+void keyboard_scan_clear_boot_key(void)
+{
+	boot_key_value = BOOT_KEY_OTHER;
+
+#ifdef CONFIG_TASK_POWERBTN
+	/* Wake the power button task to update the recovery switch state */
+	task_wake(TASK_ID_POWERBTN);
+#endif
+}
+
+int keyboard_scan_recovery_pressed(void)
+{
+	return boot_key_value == BOOT_KEY_ESC ? 1 : 0;
+}
 
 int keyboard_scan_init(void)
 {
@@ -226,28 +252,37 @@ int keyboard_scan_init(void)
 	/* Tri-state the columns */
 	lm4_select_column(COLUMN_TRI_STATE_ALL);
 
-	/* TODO: method to set which keyboard we have, so we set the actual
-	 * key mask properly */
+	/*
+	 * TODO: method to set which keyboard we have, so we set the actual
+	 * key mask properly.
+	 */
 	actual_key_mask = actual_key_masks[0];
 
 	/* Initialize raw state */
 	update_key_state();
 
-	/* Copy to the state at boot */
-	memcpy(raw_state_at_boot, raw_state, sizeof(raw_state_at_boot));
-
-	/* If we're booting due to a reset-pin-caused reset, check if the
-	 * recovery key is pressed. */
+	/*
+	 * If we're booting due to a reset-pin-caused reset, check what other
+	 * single key is held down (if any).
+	 */
 	if (system_get_reset_cause() == SYSTEM_RESET_RESET_PIN) {
-		power_set_recovery_pressed(check_boot_key(MASK_INDEX_ESC,
-							  MASK_VALUE_ESC));
+		const struct boot_key_entry *k = boot_key_list;
+		int i;
+
+		for (i = 0; i < ARRAY_SIZE(boot_key_list); i++, k++) {
+			if (check_key(k->mask_index, k->mask_value)) {
+				CPRINTF("[KB boot key %d]\n", i);
+				boot_key_value = i;
+				break;
+			}
+		}
 
 #ifdef CONFIG_FAKE_DEV_SWITCH
 		/* Turn fake dev switch on if D pressed, off if F pressed. */
-		if (check_boot_key(MASK_INDEX_D, MASK_VALUE_D)) {
+		if (boot_key_value == BOOT_KEY_D) {
 			eoption_set_bool(EOPTION_BOOL_FAKE_DEV, 1);
 			CPUTS("[Enabling fake dev-mode]\n");
-		} else if (check_boot_key(MASK_INDEX_F, MASK_VALUE_F)) {
+		} else if (boot_key_value == BOOT_KEY_F) {
 			eoption_set_bool(EOPTION_BOOL_FAKE_DEV, 0);
 			CPUTS("[Disabling fake dev-mode]\n");
 		}
@@ -257,7 +292,6 @@ int keyboard_scan_init(void)
 	return EC_SUCCESS;
 }
 
-
 void keyboard_scan_task(void)
 {
 	int key_press_timer = 0;
@@ -295,7 +329,6 @@ void keyboard_scan_task(void)
 	}
 }
 
-
 static void matrix_interrupt(void)
 {
 	uint32_t ris = lm4_clear_matrix_interrupt_status();
@@ -305,16 +338,19 @@ static void matrix_interrupt(void)
 }
 DECLARE_IRQ(KB_SCAN_ROW_IRQ, matrix_interrupt, 3);
 
-
-/* The actual implementation is controlling the enable_scanning variable, then
- * that controls whether lm4_select_column() can pull-down columns or not. */
+/*
+ * The actual implementation is controlling the enable_scanning variable, then
+ * that controls whether lm4_select_column() can pull-down columns or not.
+ */
 void keyboard_enable_scanning(int enable)
 {
 	lm4_set_scanning_enabled(enable);
 	if (enable) {
-		/* A power button press had tri-stated all columns (see the
-		 * 'else' statement below), we need a wake-up to unlock
-		 * the task_wait_event() loop after wait_for_interrupt(). */
+		/*
+		 * A power button press had tri-stated all columns (see the
+		 * 'else' statement below), we need a wake-up to unlock the
+		 * task_wait_event() loop after wait_for_interrupt().
+		 */
 		task_wake(TASK_ID_KEYSCAN);
 	} else {
 		lm4_select_column(COLUMN_TRI_STATE_ALL);
diff --git a/chip/lm4/power_button.c b/chip/lm4/power_button.c
index 1ef91ead6c..aa69d2c231 100644
--- a/chip/lm4/power_button.c
+++ b/chip/lm4/power_button.c
@@ -98,8 +98,6 @@ static int debounced_lid_open;
 static int debounced_power_pressed;
 static int ac_changed;
 static int simulate_power_pressed;
-static int keyboard_recovery_pressed;
-
 
 /* Update status of non-debounced switches */
 static void update_other_switches(void)
@@ -113,7 +111,7 @@ static void update_other_switches(void)
 	else
 		*memmap_switches &= ~EC_SWITCH_WRITE_PROTECT_DISABLED;
 
-	if (keyboard_recovery_pressed)
+	if (keyboard_scan_recovery_pressed())
 		*memmap_switches |= EC_SWITCH_KEYBOARD_RECOVERY;
 	else
 		*memmap_switches &= ~EC_SWITCH_KEYBOARD_RECOVERY;
@@ -224,7 +222,7 @@ static void lid_switch_open(uint64_t tnow)
 	/* If the chipset is off, clear keyboard recovery and send a power
 	 * button pulse to wake up the chipset. */
 	if (chipset_in_state(CHIPSET_STATE_ANY_OFF)) {
-		power_set_recovery_pressed(0);
+		keyboard_scan_clear_boot_key();
 		chipset_exit_hard_off();
 		set_pwrbtn_to_pch(0);
 		pwrbtn_state = PWRBTN_STATE_LID_OPEN;
@@ -299,7 +297,7 @@ static void set_initial_pwrbtn_state(void)
 	if (system_get_reset_cause() == SYSTEM_RESET_RESET_PIN) {
 		/* Reset triggered by keyboard-controlled reset, so override
 		 * the power button signal to the PCH. */
-		if (keyboard_recovery_pressed) {
+		if (keyboard_scan_recovery_pressed()) {
 			/* In recovery mode, so send a power button pulse to
 			 * the PCH so it powers on. */
 			CPRINTF("[%T PB init-recovery]\n");
@@ -356,24 +354,6 @@ int power_lid_open_debounced(void)
 }
 
 
-int power_recovery_pressed(void)
-{
-	return keyboard_recovery_pressed;
-}
-
-
-void power_set_recovery_pressed(int pressed)
-{
-	if (keyboard_recovery_pressed == pressed)
-		return;
-
-	CPRINTF("[%T PB recovery button %s]\n",
-		pressed ? "pressed" : "released");
-
-	keyboard_recovery_pressed = pressed;
-	update_other_switches();
-}
-
 /*****************************************************************************/
 /* Task / state machine */
 
@@ -391,13 +371,15 @@ static void state_machine(uint64_t tnow)
 	case PWRBTN_STATE_PRESSED:
 		if (chipset_in_state(CHIPSET_STATE_ANY_OFF)) {
 			/* Clear keyboard recovery */
-			power_set_recovery_pressed(0);
-			/* Chipset is off, so wake the chipset and send it a
+			keyboard_scan_clear_boot_key();
+			/*
+			 * Chipset is off, so wake the chipset and send it a
 			 * long enough pulse to wake up.  After that we'll
 			 * reflect the true power button state.  If we don't
 			 * stretch the pulse here, the user may release the
 			 * power button before the chipset finishes waking from
-			 * hard off state. */
+			 * hard off state.
+			 */
 			chipset_exit_hard_off();
 			tnext_state = tnow + PWRBTN_INITIAL_US;
 			pwrbtn_state = PWRBTN_STATE_WAS_OFF;
@@ -414,9 +396,11 @@ static void state_machine(uint64_t tnow)
 		set_pwrbtn_to_pch(1);
 		break;
 	case PWRBTN_STATE_T1:
-		/* If the chipset is already off, don't tell it the power
+		/*
+		 * If the chipset is already off, don't tell it the power
 		 * button is down; it'll just cause the chipset to turn on
-		 * again. */
+		 * again.
+		 */
 		if (chipset_in_state(CHIPSET_STATE_ANY_OFF))
 			CPRINTF("[%T PB chipset already off]\n");
 		else
diff --git a/common/vboot_sig.c b/common/vboot_sig.c
index 855a3334a2..6df42e7bf5 100644
--- a/common/vboot_sig.c
+++ b/common/vboot_sig.c
@@ -5,10 +5,11 @@
 
 /* Verified boot module for Chrome EC */
 
+#include "board.h"
+#include "config.h"
 #include "console.h"
 #include "cryptolib.h"
 #include "gpio.h"
-#include "power_button.h"
 #include "system.h"
 #include "timer.h"
 #include "util.h"
@@ -88,23 +89,27 @@ static int maybe_jump_to_other_image(void)
 	if (system_get_image_copy() != SYSTEM_IMAGE_RO)
 		return 0;
 
-#ifdef CONFIG_TASK_POWERBTN
+#ifdef CONFIG_TASK_KEYSCAN
 	/* Don't jump if recovery requested */
-	if (power_recovery_pressed()) {
+	if (keyboard_scan_recovery_pressed()) {
 		CPUTS("[Vboot staying in RO because recovery key pressed]\n");
 		return 0;
 	}
 #endif
 
-	/* Don't jump if we're in RO becuase we jumped there (this keeps us
-	 * from jumping to RO only to jump right back). */
+	/*
+	 * Don't jump if we're in RO becuase we jumped there (this keeps us
+	 * from jumping to RO only to jump right back).
+	 */
 	if (system_jumped_to_this_image())
 		return 0;
 
 #if !defined(CHIP_stm32)
-	/* TODO: (crosbug.com/p/8572) Daisy and Snow don't define a GPIO
-	 * for the recovery signal from servo, so we can't check it.
-	 * BDS uses the DOWN button. */
+	/*
+	 * TODO: (crosbug.com/p/8572) Daisy and Snow don't define a GPIO for
+	 * the recovery signal from servo, so we can't check it.  BDS uses the
+	 * DOWN button.
+	 */
 	if (gpio_get_level(GPIO_RECOVERYn) == 0) {
 		CPUTS("[Vboot staying in RO due to recovery signal]\n");
 		return 0;
diff --git a/include/keyboard_scan.h b/include/keyboard_scan.h
index 590f482317..98ff5db826 100644
--- a/include/keyboard_scan.h
+++ b/include/keyboard_scan.h
@@ -13,11 +13,31 @@
 /* Initializes the module. */
 int keyboard_scan_init(void);
 
-/* Returns non-zero if recovery key was pressed at boot.  Used by st32m-based
- * boards only; lm4-based boards use power_recovery_pressed(). */
+/* Key held down at keyboard-controlled reset boot time. */
+enum boot_key {
+	BOOT_KEY_NONE,  /* No keys other than keyboard-controlled reset keys */
+	BOOT_KEY_ESC,
+	BOOT_KEY_D,
+	BOOT_KEY_F,
+	BOOT_KEY_DOWN_ARROW,
+	BOOT_KEY_OTHER = -1,  /* None of the above */
+};
+
+/*
+ * Return the key held down at boot time in addition to the keyboard-controlled
+ * reset keys.  Returns BOOT_KEY_OTHER if none of the keys specifically checked
+ * was pressed, or reset was not caused by a keyboard-controlled reset, or if
+ * the state has been cleared by keyboard_scan_clear_boot_key().
+ */
+enum boot_key keyboard_scan_get_boot_key(void);
+
+/* Clear the boot key state. */
+void keyboard_scan_clear_boot_key(void);
+
+/* Return non-zero if recovery key was pressed at boot. */
 int keyboard_scan_recovery_pressed(void);
 
-/* clear any saved keyboard state (empty FIFO, etc) */
+/* Clear any saved keyboard state (empty FIFO, etc) */
 void keyboard_clear_state(void);
 
 /* Enables/disables keyboard matrix scan. */
diff --git a/include/power_button.h b/include/power_button.h
index a0bb85635e..7e99690185 100644
--- a/include/power_button.h
+++ b/include/power_button.h
@@ -11,8 +11,10 @@
 #include "common.h"
 #include "gpio.h"
 
-/* Interrupt handler for the power button and lid switch.  Passed the signal
- * which triggered the interrupt. */
+/*
+ * Interrupt handler for the power button and lid switch.  Passed the signal
+ * which triggered the interrupt.
+ */
 void power_button_interrupt(enum gpio_signal signal);
 
 /* Power button task */
@@ -21,15 +23,10 @@ void power_button_task(void);
 /* Return non-zero if AC power is present. */
 int power_ac_present(void);
 
-/* Return non-zero if lid is open.  Uses the debounced lid state, not the raw
- * signal from the GPIO. */
+/*
+ * Return non-zero if lid is open.  Uses the debounced lid state, not the raw
+ * signal from the GPIO.
+ */
 int power_lid_open_debounced(void);
 
-/* Return non-zero if the recovery button is pressed. */
-int power_recovery_pressed(void);
-
-/* Set the state of the recovery button.  Called by the keyboard scanner at
- * init if the keyboard recovery combo was pressed. */
-void power_set_recovery_pressed(int pressed);
-
 #endif  /* __CROS_EC_POWER_BUTTON_H */