Refactored keyboard scan enable flag to allow for multiple disable reasons

Refactored keyboard scan enable/disable flag such that it is a mask of
potential disable sources. When all disable sources are off, scanning is
enabled, otherwise scanning is disabled. This fixes a recently introduced
bug in which enabling/disabling keyboard scanning due to lid angle in S3
was interfering with enabling/disabling keyboard scanning due to power
button. This also allows for easy expansion for future causes for disabling
keyboard scanning.

BUG=chrome-os-partner:27851
BRANCH=rambi
TEST=Manual tests with a glimmer. Used the ksstate console command to
check state of keyboard scanning under all permutations of power button
pressed/unpressed, lid switch open/closed, and lid angle in tablet position
vs. laptop positon.

Change-Id: Ied4c5ebb94510b1078cd81d71373c0f1bd0d6678
Signed-off-by: Alec Berg <alecaberg@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/194287
Reviewed-by: Randall Spangler <rspangler@chromium.org>

7 files changed, 77 insertions, 62 deletions

diff --git a/common/keyboard_scan.c b/common/keyboard_scan.c
index da918bbc8b..6214f0934e 100644
--- a/common/keyboard_scan.c
+++ b/common/keyboard_scan.c
@@ -88,20 +88,37 @@ static uint32_t post_scan_clock_us;
  */
 static int print_state_changes;
 
-static int enable_scanning = 1;  /* Must init to 1 for scanning at boot */
+static int disable_scanning_mask;  /* Must init to 0 for scanning at boot */
 
 /* Constantly incrementing counter of the number of times we polled */
 static volatile int kbd_polls;
 
-int keyboard_scan_is_enabled(void)
+static int keyboard_scan_is_enabled(void)
 {
-#ifdef CONFIG_LID_SWITCH
-	/* Scanning is never enabled when lid is closed */
-	if (!lid_is_open())
-		return 0;
-#endif
+	return !disable_scanning_mask;
+}
+
+void keyboard_scan_enable(int enable, enum kb_scan_disable_masks mask)
+{
+	int old_disable_scanning = disable_scanning_mask;
+
+	disable_scanning_mask = enable ? (disable_scanning_mask & ~mask) :
+					 (disable_scanning_mask | mask);
 
-	return enable_scanning;
+	if (disable_scanning_mask != old_disable_scanning)
+		CPRINTF("[%T KB disable_scanning_mask changed: 0x%08x]\n",
+				disable_scanning_mask);
+
+	if (old_disable_scanning && !disable_scanning_mask) {
+		/*
+		 * Scanning is being enabled, so wake up the scanning task to
+		 * unlock the task_wait_event() loop after enable_interrupt().
+		 */
+		task_wake(TASK_ID_KEYSCAN);
+	} else if (disable_scanning_mask && !old_disable_scanning) {
+		keyboard_raw_drive_column(KEYBOARD_COLUMN_NONE);
+		keyboard_clear_buffer();
+	}
 }
 
 /**
@@ -200,11 +217,10 @@ static int read_matrix(uint8_t *state)
 
 	for (c = 0; c < KEYBOARD_COLS; c++) {
 		/*
-		 * Stop if scanning becomes disabled.  Check enable_cscanning
-		 * instead of keyboard_scan_is_enabled() so that we can scan the
-		 * matrix at boot time before the lid switch is readable.
+		 * Stop if scanning becomes disabled. Note, scanning is enabled
+		 * on boot by default.
 		 */
-		if (!enable_scanning)
+		if (!keyboard_scan_is_enabled())
 			break;
 
 		/* Select column, then wait a bit for it to settle */
@@ -617,30 +633,14 @@ void keyboard_scan_task(void)
 	}
 }
 
-void keyboard_scan_enable(int enable)
-{
-	enable_scanning = 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 enable_interrupt().
-		 */
-		task_wake(TASK_ID_KEYSCAN);
-	} else {
-		keyboard_raw_drive_column(KEYBOARD_COLUMN_NONE);
-		keyboard_clear_buffer();
-	}
-}
-
 #ifdef CONFIG_LID_SWITCH
 
 static void keyboard_lid_change(void)
 {
-	/* If lid is open, wake the keyboard task */
 	if (lid_is_open())
-		task_wake(TASK_ID_KEYSCAN);
+		keyboard_scan_enable(1, KB_SCAN_DISABLE_LID_CLOSED);
+	else
+		keyboard_scan_enable(0, KB_SCAN_DISABLE_LID_CLOSED);
 }
 DECLARE_HOOK(HOOK_LID_CHANGE, keyboard_lid_change, HOOK_PRIO_DEFAULT);
 
@@ -680,6 +680,8 @@ static int command_ksstate(int argc, char **argv)
 	print_state(prev_state, "prev      ");
 	print_state(debouncing, "debouncing");
 
+	ccprintf("Keyboard scan disable mask: 0x%08x\n",
+		 disable_scanning_mask);
 	ccprintf("Keyboard scan state printing %s\n",
 		 print_state_changes ? "on" : "off");
 	return EC_SUCCESS;
diff --git a/common/lid_angle.c b/common/lid_angle.c
index 02f72f1991..2bee31d8c9 100644
--- a/common/lid_angle.c
+++ b/common/lid_angle.c
@@ -8,6 +8,7 @@
 #include "chipset.h"
 #include "common.h"
 #include "console.h"
+#include "hooks.h"
 #include "keyboard_scan.h"
 #include "lid_angle.h"
 #include "lid_switch.h"
@@ -125,16 +126,11 @@ void lidangle_keyscan_update(float lid_ang)
 	lidangle_buffer[index] = lid_ang;
 	index = (index == KEY_SCAN_LID_ANGLE_BUFFER_SIZE-1) ? 0 : index+1;
 
-#ifdef CONFIG_LID_SWITCH
 	/*
-	 * If lid is closed, don't need to check if keyboard scanning should
-	 * be enabled.
+	 * Any time the chipset is off, manage whether or not keyboard scanning
+	 * is enabled based on lid angle history.
 	 */
-	if (!lid_is_open())
-		return;
-#endif
-
-	if (chipset_in_state(CHIPSET_STATE_SUSPEND)) {
+	if (!chipset_in_state(CHIPSET_STATE_ON)) {
 		for (i = 0; i < KEY_SCAN_LID_ANGLE_BUFFER_SIZE; i++) {
 			/*
 			 * If any lid angle samples are unreliable, then
@@ -154,16 +150,20 @@ void lidangle_keyscan_update(float lid_ang)
 				keys_ignore = 0;
 		}
 
-		/* Enable or disable keyboard scanning if necessary. */
-		if (keys_accept && !keyboard_scan_is_enabled()) {
-			CPRINTF("[%T Enabling keyboard scan, lid ang at %d]\n",
-					(int)lidangle_buffer[index]);
-			keyboard_scan_enable(1);
-		} else if (keys_ignore && !keys_accept &&
-				keyboard_scan_is_enabled()) {
-			CPRINTF("[%T Disabling keyboard scan, lid ang at %d]\n",
-					(int)lidangle_buffer[index]);
-			keyboard_scan_enable(0);
-		}
+		/* Enable or disable keyboard scanning as necessary. */
+		if (keys_accept)
+			keyboard_scan_enable(1, KB_SCAN_DISABLE_LID_ANGLE);
+		else if (keys_ignore && !keys_accept)
+			keyboard_scan_enable(0, KB_SCAN_DISABLE_LID_ANGLE);
 	}
 }
+
+static void enable_keyboard(void)
+{
+	/*
+	 * Make sure lid angle is not disabling keyboard scanning when AP is
+	 * running.
+	 */
+	keyboard_scan_enable(1, KB_SCAN_DISABLE_LID_ANGLE);
+}
+DECLARE_HOOK(HOOK_CHIPSET_RESUME, enable_keyboard, HOOK_PRIO_DEFAULT);
diff --git a/common/power_button.c b/common/power_button.c
index ae20083c52..f4b2af41b1 100644
--- a/common/power_button.c
+++ b/common/power_button.c
@@ -75,7 +75,7 @@ static void power_button_change_deferred(void)
 
 	/* Re-enable keyboard scanning if power button is no longer pressed */
 	if (!new_pressed)
-		keyboard_scan_enable(1);
+		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)
@@ -102,7 +102,7 @@ void power_button_interrupt(enum gpio_signal signal)
 	 * on the same column with refresh key.
 	 */
 	if (raw_power_button_pressed())
-		keyboard_scan_enable(0);
+		keyboard_scan_enable(0, KB_SCAN_DISABLE_POWER_BUTTON);
 
 	/* Reset power button debounce time */
 	hook_call_deferred(power_button_change_deferred, PWRBTN_DEBOUNCE_US);
diff --git a/include/keyboard_scan.h b/include/keyboard_scan.h
index 8359cbd174..41cdbdf611 100644
--- a/include/keyboard_scan.h
+++ b/include/keyboard_scan.h
@@ -75,20 +75,27 @@ static inline enum boot_key keyboard_scan_get_boot_key(void)
  */
 const uint8_t *keyboard_scan_get_state(void);
 
+enum kb_scan_disable_masks {
+	/* Reasons why keyboard scanning should be disabled */
+	KB_SCAN_DISABLE_LID_CLOSED   = (1<<0),
+	KB_SCAN_DISABLE_POWER_BUTTON = (1<<1),
+	KB_SCAN_DISABLE_LID_ANGLE    = (1<<2),
+};
+
 #ifdef HAS_TASK_KEYSCAN
 /**
- * Enables/disables keyboard matrix scan.
+ * Enable/disable keyboard scanning. Scanning will be disabled if any disable
+ * reason bit is set. Scanning is enabled only if no disable reasons are set.
+ *
+ * @param enable Clear(=1) or set(=0) disable-bits from the mask.
+ * @param mask Disable reasons from kb_scan_disable_masks
  */
-void keyboard_scan_enable(int enable);
+void keyboard_scan_enable(int enable, enum kb_scan_disable_masks mask);
 #else
-static inline void keyboard_scan_enable(int enable) { }
+static inline void keyboard_scan_enable(int enable,
+		enum kb_scan_disable_masks mask) { }
 #endif
 
-/**
- * Returns if keyboard matrix scanning is enabled/disabled.
- */
-int keyboard_scan_is_enabled(void);
-
 #ifdef CONFIG_KEYBOARD_SUPPRESS_NOISE
 /**
  * Indicate to audio codec that a key has been pressed.
diff --git a/power/gaia.c b/power/gaia.c
index 523c20ed70..b1d4f9a645 100644
--- a/power/gaia.c
+++ b/power/gaia.c
@@ -201,7 +201,7 @@ static int check_for_power_off_event(void)
 #ifdef HAS_TASK_KEYSCAN
 	/* Dis/Enable keyboard scanning when the power button state changes */
 	if (!pressed || pressed != power_button_was_pressed)
-		keyboard_scan_enable(!pressed);
+		keyboard_scan_enable(!pressed, KB_SCAN_DISABLE_POWER_BUTTON);
 #endif
 
 
diff --git a/power/tegra.c b/power/tegra.c
index 4ca3a172f3..58f46ad8ab 100644
--- a/power/tegra.c
+++ b/power/tegra.c
@@ -180,7 +180,7 @@ static int check_for_power_off_event(void)
 #ifdef HAS_TASK_KEYSCAN
 	/* Dis/Enable keyboard scanning when the power button state changes */
 	if (!pressed || pressed != power_button_was_pressed)
-		keyboard_scan_enable(!pressed);
+		keyboard_scan_enable(!pressed, KB_SCAN_DISABLE_POWER_BUTTON);
 #endif
 
 	now = get_time();
diff --git a/test/kb_scan.c b/test/kb_scan.c
index e0e73d17f5..926b0a27df 100644
--- a/test/kb_scan.c
+++ b/test/kb_scan.c
@@ -9,6 +9,7 @@
 #include "common.h"
 #include "console.h"
 #include "gpio.h"
+#include "hooks.h"
 #include "host_command.h"
 #include "keyboard_raw.h"
 #include "keyboard_scan.h"
@@ -330,12 +331,14 @@ static int runtime_key_test(void)
 static int lid_test(void)
 {
 	lid_open = 0;
+	hook_notify(HOOK_LID_CHANGE);
 	mock_key(1, 1, 1);
 	TEST_ASSERT(expect_no_keychange() == EC_SUCCESS);
 	mock_key(1, 1, 0);
 	TEST_ASSERT(expect_no_keychange() == EC_SUCCESS);
 
 	lid_open = 1;
+	hook_notify(HOOK_LID_CHANGE);
 	mock_key(1, 1, 1);
 	TEST_ASSERT(expect_keychange() == EC_SUCCESS);
 	mock_key(1, 1, 0);
@@ -375,6 +378,7 @@ void test_init(void)
 static void run_test_step1(void)
 {
 	lid_open = 1;
+	hook_notify(HOOK_LID_CHANGE);
 	test_reset();
 
 	RUN_TEST(deghost_test);
@@ -396,6 +400,7 @@ static void run_test_step1(void)
 static void run_test_step2(void)
 {
 	lid_open = 1;
+	hook_notify(HOOK_LID_CHANGE);
 	test_reset();
 
 	RUN_TEST(test_check_boot_esc);
@@ -409,6 +414,7 @@ static void run_test_step2(void)
 static void run_test_step3(void)
 {
 	lid_open = 1;
+	hook_notify(HOOK_LID_CHANGE);
 	test_reset();
 
 	RUN_TEST(test_check_boot_down);