keyboard: Add strict debouncer

This CL adds CONFIG_KEYBOARD_STRICT_DEBOUNCE. It makes the keyboard
debouncer register a key stroke after deounce is done.

This CL also adds a unit test.

BUG=b:193505909
BRANCH=Dedede
TEST=make run-kb_scan_strict
TEST=Blipper

Change-Id: Ia380657021035930afab5cafffa8cc2edd7ff475
Signed-off-by: Daisuke Nojiri <dnojiri@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/3044405
Reviewed-by: Jett Rink <jettrink@chromium.org>

4 files changed, 163 insertions, 3 deletions

diff --git a/test/build.mk b/test/build.mk
index 30b7c1ce55..6b156ccaf0 100644
--- a/test/build.mk
+++ b/test/build.mk
@@ -49,6 +49,7 @@ test-list-host += kasa
 test-list-host += kb_8042
 test-list-host += kb_mkbp
 test-list-host += kb_scan
+test-list-host += kb_scan_strict
 test-list-host += lid_sw
 test-list-host += lightbar
 test-list-host += mag_cal
@@ -159,6 +160,7 @@ is_enabled-y=is_enabled.o
 kb_8042-y=kb_8042.o
 kb_mkbp-y=kb_mkbp.o
 kb_scan-y=kb_scan.o
+kb_scan_strict-y=kb_scan.o
 lid_sw-y=lid_sw.o
 lightbar-y=lightbar.o
 mag_cal-y=mag_cal.o
diff --git a/test/kb_scan.c b/test/kb_scan.c
index 8ca81cad18..b3b42813e4 100644
--- a/test/kb_scan.c
+++ b/test/kb_scan.c
@@ -31,7 +31,16 @@
 		old = fifo_add_count; \
 	} while (0)
 
+/* Emulated physical key state */
 static uint8_t mock_state[KEYBOARD_COLS_MAX];
+
+/* Snapshot of last known key state */
+static uint8_t key_state[KEYBOARD_COLS_MAX];
+
+/* Counters for key state changes (UP/DOWN) */
+static int key_state_change[KEYBOARD_COLS_MAX][KEYBOARD_ROWS];
+static int total_key_state_change;
+
 static int column_driven;
 static int fifo_add_count;
 static int lid_open;
@@ -79,7 +88,24 @@ int keyboard_raw_read_rows(void)
 
 int mkbp_keyboard_add(const uint8_t *buffp)
 {
+	int c, r;
+
 	fifo_add_count++;
+
+	for (c = 0; c < KEYBOARD_COLS_MAX; c++) {
+		uint8_t diff = key_state[c] ^ buffp[c];
+
+		for (r = 0; r < KEYBOARD_ROWS; r++) {
+			if (diff & BIT(r)) {
+				key_state_change[c][r]++;
+				total_key_state_change++;
+			}
+		}
+	}
+
+	/* Save a snapshot. */
+	memcpy(key_state, buffp, sizeof(key_state));
+
 	return EC_SUCCESS;
 }
 
@@ -105,13 +131,23 @@ void chipset_reset(void)
 
 static void mock_key(int r, int c, int keydown)
 {
-	ccprintf("%s (%d, %d)\n", keydown ? "Pressing" : "Releasing", r, c);
+	ccprintf("  %s (%d, %d)\n", keydown ? "Pressing" : "Releasing", r, c);
 	if (keydown)
 		mock_state[c] |= (1 << r);
 	else
 		mock_state[c] &= ~(1 << r);
 }
 
+static void reset_key_state(void)
+{
+	memset(mock_state, 0, sizeof(mock_state));
+	memset(key_state, 0, sizeof(key_state));
+	memset(key_state_change, 0, sizeof(key_state_change));
+	task_wake(TASK_ID_KEYSCAN);
+	msleep(NO_KEYDOWN_DELAY_MS);
+	total_key_state_change = 0;
+}
+
 static int expect_keychange(void)
 {
 	int old_count = fifo_add_count;
@@ -164,6 +200,8 @@ static int verify_key_presses(int old, int expected)
 
 static int deghost_test(void)
 {
+	reset_key_state();
+
 	/* Test we can detect a keypress */
 	mock_key(1, 1, 1);
 	TEST_ASSERT(expect_keychange() == EC_SUCCESS);
@@ -205,11 +243,118 @@ static int deghost_test(void)
 	return EC_SUCCESS;
 }
 
+static int strict_debounce_test(void)
+{
+	reset_key_state();
+
+	ccprintf("Test key press & hold.\n");
+	mock_key(1, 1, 1);
+	TEST_EQ(expect_keychange(), EC_SUCCESS, "%d");
+	TEST_EQ(key_state_change[1][1], 1, "%d");
+	TEST_EQ(total_key_state_change, 1, "%d");
+	ccprintf("Pass.\n");
+
+	reset_key_state();
+
+	ccprintf("Test a short stroke.\n");
+	mock_key(1, 1, 1);
+	task_wake_then_sleep_1ms(TASK_ID_KEYSCAN);
+	mock_key(1, 1, 0);
+	task_wake_then_sleep_1ms(TASK_ID_KEYSCAN);
+	TEST_EQ(expect_no_keychange(), EC_SUCCESS, "%d");
+	TEST_EQ(key_state_change[1][1], 0, "%d");
+	ccprintf("Pass.\n");
+
+	reset_key_state();
+
+	ccprintf("Test ripples being suppressed.\n");
+	/* DOWN */
+	mock_key(1, 1, 1);
+	task_wake_then_sleep_1ms(TASK_ID_KEYSCAN);
+	mock_key(1, 1, 0);
+	task_wake_then_sleep_1ms(TASK_ID_KEYSCAN);
+	mock_key(1, 1, 1);
+	task_wake_then_sleep_1ms(TASK_ID_KEYSCAN);
+	TEST_EQ(expect_keychange(), EC_SUCCESS, "%d");
+	TEST_EQ(key_state_change[1][1], 1, "%d");
+	TEST_EQ(total_key_state_change, 1, "%d");
+	/* UP */
+	mock_key(1, 1, 0);
+	task_wake_then_sleep_1ms(TASK_ID_KEYSCAN);
+	mock_key(1, 1, 1);
+	task_wake_then_sleep_1ms(TASK_ID_KEYSCAN);
+	mock_key(1, 1, 0);
+	task_wake_then_sleep_1ms(TASK_ID_KEYSCAN);
+	TEST_EQ(expect_keychange(), EC_SUCCESS, "%d");
+	TEST_EQ(key_state_change[1][1], 2, "%d");
+	TEST_EQ(total_key_state_change, 2, "%d");
+	ccprintf("Pass.\n");
+
+	reset_key_state();
+
+	ccprintf("Test simultaneous strokes.\n");
+	mock_key(1, 1, 1);
+	mock_key(2, 1, 1);
+	task_wake_then_sleep_1ms(TASK_ID_KEYSCAN);
+	TEST_EQ(expect_keychange(), EC_SUCCESS, "%d");
+	TEST_EQ(key_state_change[1][1], 1, "%d");
+	TEST_EQ(key_state_change[1][2], 1, "%d");
+	TEST_EQ(total_key_state_change, 2, "%d");
+	ccprintf("Pass.\n");
+
+	reset_key_state();
+
+	ccprintf("Test simultaneous strokes in two columns.\n");
+	mock_key(1, 1, 1);
+	mock_key(1, 2, 1);
+	task_wake_then_sleep_1ms(TASK_ID_KEYSCAN);
+	TEST_EQ(expect_keychange(), EC_SUCCESS, "%d");
+	TEST_EQ(key_state_change[1][1], 1, "%d");
+	TEST_EQ(key_state_change[2][1], 1, "%d");
+	TEST_EQ(total_key_state_change, 2, "%d");
+	ccprintf("Pass.\n");
+
+	reset_key_state();
+
+	ccprintf("Test normal & short simultaneous strokes.\n");
+	mock_key(1, 1, 1);
+	task_wake_then_sleep_1ms(TASK_ID_KEYSCAN);
+	mock_key(2, 1, 1);
+	task_wake_then_sleep_1ms(TASK_ID_KEYSCAN);
+	mock_key(1, 1, 0);
+	task_wake_then_sleep_1ms(TASK_ID_KEYSCAN);
+	TEST_EQ(expect_keychange(), EC_SUCCESS, "%d");
+	TEST_EQ(key_state_change[1][1], 0, "%d");
+	TEST_EQ(key_state_change[1][2], 1, "%d");
+	TEST_EQ(total_key_state_change, 1, "%d");
+	ccprintf("Pass.\n");
+
+	reset_key_state();
+
+	ccprintf("Test normal & short simultaneous strokes in two columns.\n");
+	reset_key_state();
+	mock_key(1, 1, 1);
+	task_wake(TASK_ID_KEYSCAN);
+	mock_key(1, 2, 1);
+	task_wake(TASK_ID_KEYSCAN);
+	mock_key(1, 1, 0);
+	task_wake(TASK_ID_KEYSCAN);
+	TEST_EQ(expect_keychange(), EC_SUCCESS, "%d");
+	TEST_EQ(key_state_change[1][1], 0, "%d");
+	TEST_EQ(key_state_change[2][1], 1, "%d");
+	TEST_EQ(total_key_state_change, 1, "%d");
+	ccprintf("Pass.\n");
+
+	return EC_SUCCESS;
+}
+
 static int debounce_test(void)
 {
 	int old_count = fifo_add_count;
 	int i;
 
+	reset_key_state();
+
 	/* One brief keypress is detected. */
 	msleep(40);
 	mock_key(1, 1, 1);
@@ -293,6 +438,8 @@ static int simulate_key_test(void)
 {
 	int old_count;
 
+	reset_key_state();
+
 	task_wake(TASK_ID_KEYSCAN);
 	msleep(40); /* Wait for debouncing to settle */
 
@@ -332,6 +479,8 @@ static int verify_variable_not_set(int *var)
 
 static int runtime_key_test(void)
 {
+	reset_key_state();
+
 	/* Alt-VolUp-H triggers system hibernation */
 	mock_defined_key(LEFT_ALT, 1);
 	mock_default_key(VOL_UP, 1);
@@ -372,6 +521,8 @@ static int runtime_key_test(void)
 #ifdef CONFIG_LID_SWITCH
 static int lid_test(void)
 {
+	reset_key_state();
+
 	msleep(40); /* Allow debounce to settle */
 
 	lid_open = 0;
@@ -429,7 +580,10 @@ static void run_test_step1(void)
 
 	RUN_TEST(deghost_test);
 
-	RUN_TEST(debounce_test);
+	if (IS_ENABLED(CONFIG_KEYBOARD_STRICT_DEBOUNCE))
+		RUN_TEST(strict_debounce_test);
+	else
+		RUN_TEST(debounce_test);
 
 	if (0)  /* crbug.com/976974 */
 		RUN_TEST(simulate_key_test);
diff --git a/test/kb_scan_strict.tasklist b/test/kb_scan_strict.tasklist
new file mode 120000
index 0000000000..d4d4fa1efb
--- /dev/null
+++ b/test/kb_scan_strict.tasklist
@@ -0,0 +1 @@
+kb_scan.tasklist
\ No newline at end of file
diff --git a/test/test_config.h b/test/test_config.h
index 9e74a8646b..8b98dc1087 100644
--- a/test/test_config.h
+++ b/test/test_config.h
@@ -59,10 +59,13 @@
 #define CONFIG_MKBP_USE_GPIO
 #endif
 
-#ifdef TEST_KB_SCAN
+#if defined(TEST_KB_SCAN) || defined(TEST_KB_SCAN_STRICT)
 #define CONFIG_KEYBOARD_PROTOCOL_MKBP
 #define CONFIG_MKBP_EVENT
 #define CONFIG_MKBP_USE_GPIO
+#ifdef TEST_KB_SCAN_STRICT
+#define CONFIG_KEYBOARD_STRICT_DEBOUNCE
+#endif
 #endif
 
 #ifdef TEST_MATH_UTIL