keyboard_scan: Send kb events at start of debouncing period.

Current behavior is to start a timer upon first detecting a transition of
any one cell of the keyboard matrix, and then generate a keyboard event
(press or release) only after that timer has expired.  Due to the fact that
the release timer has a longer period than the press timer, in some cases,
e.g. releasing the shift key right before depressing another key, events
could end up getting re-ordered.

BUG=chromium:547131
BRANCH=master
TEST=make run-kb_scan
Signed-off-by: Jes Klinke <jbk@google.com>
Change-Id: If3de2e629dc9df4325d8c17590d6624a41e27187
Bug: 547131
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/1579905
Tested-by: Jes Klinke <jbk@chromium.org>
Reviewed-by: Jett Rink <jettrink@chromium.org>
Commit-Queue: Jes Klinke <jbk@chromium.org>

1 files changed, 27 insertions, 40 deletions

diff --git a/common/keyboard_scan.c b/common/keyboard_scan.c
index a8f843e0a2..1c0f9759d1 100644
--- a/common/keyboard_scan.c
+++ b/common/keyboard_scan.c
@@ -86,8 +86,6 @@ uint8_t keyboard_cols = KEYBOARD_COLS_MAX;
 
 /* Debounced key matrix */
 static uint8_t __bss_slow debounced_state[KEYBOARD_COLS_MAX];
-/* Matrix from previous scan */
-static uint8_t __bss_slow prev_state[KEYBOARD_COLS_MAX];
 /* Mask of keys being debounced */
 static uint8_t __bss_slow debouncing[KEYBOARD_COLS_MAX];
 /* Keys simulated-pressed */
@@ -475,46 +473,27 @@ static int check_keys_changed(uint8_t *state)
 
 	/* Check for changes between previous scan and this one */
 	for (c = 0; c < keyboard_cols; c++) {
-		int diff = new_state[c] ^ prev_state[c];
+		int diff;
 
-		if (!diff)
-			continue;
-
-		for (i = 0; i < KEYBOARD_ROWS; i++) {
-			if (diff & BIT(i))
-				scan_edge_index[c][i] = scan_time_index;
-		}
-
-		debouncing[c] |= diff;
-		prev_state[c] = new_state[c];
-	}
-
-	/* Check for keys which are done debouncing */
-	for (c = 0; c < keyboard_cols; c++) {
-		int debc = debouncing[c];
-
-		if (!debc)
-			continue;
-
-		for (i = 0; i < KEYBOARD_ROWS; i++) {
-			int mask = 1 << i;
-			int new_mask = new_state[c] & mask;
-
-			/* Are we done debouncing this key? */
-			if (!(debc & mask))
-				continue;  /* Not debouncing this key */
+		/* Clear debouncing flag, if sufficient time has elapsed. */
+		for (i = 0; i < KEYBOARD_ROWS && debouncing[c]; i++) {
+			if (!(debouncing[c] & BIT(i)))
+				continue;
 			if (tnow - scan_time[scan_edge_index[c][i]] <
-			    (new_mask ? keyscan_config.debounce_down_us :
+			    (state[c] ? keyscan_config.debounce_down_us :
 					keyscan_config.debounce_up_us))
 				continue;  /* Not done debouncing */
+			debouncing[c] &= ~BIT(i);
+		}
 
-			debouncing[c] &= ~mask;
-
-			/* Did the key change from its previous state? */
-			if ((state[c] & mask) == new_mask)
-				continue;  /* No */
-
-			state[c] ^= mask;
+		/* Recognize change in state, unless debounce in effect. */
+		diff = (new_state[c] ^ state[c]) & ~debouncing[c];
+		if (!diff)
+			continue;
+		for (i = 0; i < KEYBOARD_ROWS; i++) {
+			if (!(diff & BIT(i)))
+				continue;
+			scan_edge_index[c][i] = scan_time_index;
 			any_change = 1;
 
 			/* Inform keyboard module if scanning is enabled */
@@ -522,9 +501,19 @@ static int check_keys_changed(uint8_t *state)
 				/* This is no-op for protocols that require a
 				 * full keyboard matrix (e.g., MKBP).
 				 */
-				keyboard_state_changed(i, c, new_mask ? 1 : 0);
+				keyboard_state_changed(
+					i, c, !!(new_state[c] & BIT(i)));
 			}
 		}
+
+		/* For any keyboard events just sent, turn on debouncing. */
+		debouncing[c] |= diff;
+		/*
+		 * Note: In order to "remember" what was last reported
+		 * (up or down), the state bits are only updated if the
+		 * edge was not suppressed due to debouncing.
+		 */
+		state[c] ^= diff;
 	}
 
 	if (any_change) {
@@ -681,7 +670,6 @@ void keyboard_scan_init(void)
 
 	/* Initialize raw state */
 	read_matrix(debounced_state);
-	memcpy(prev_state, debounced_state, sizeof(prev_state));
 
 #ifdef CONFIG_KEYBOARD_LANGUAGE_ID
 	/* Check keyboard ID state */
@@ -982,7 +970,6 @@ static int command_ksstate(int argc, char **argv)
 	}
 
 	print_state(debounced_state, "debounced ");
-	print_state(prev_state, "prev      ");
 	print_state(debouncing, "debouncing");
 
 	ccprintf("Keyboard scan disable mask: 0x%08x\n",