From f4f7e3f42c265bb2d9dfbd2fe442f149d84bd6b0 Mon Sep 17 00:00:00 2001 From: Alec Berg Date: Thu, 10 Apr 2014 12:32:05 -0700 Subject: 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 Reviewed-on: https://chromium-review.googlesource.com/194097 Reviewed-by: Randall Spangler --- common/keyboard_scan.c | 64 +++++++++++++++++++++++++------------------------ common/lid_angle.c | 38 ++++++++++++++--------------- common/power_button.c | 4 ++-- include/keyboard_scan.h | 23 +++++++++++------- power/gaia.c | 2 +- power/tegra.c | 2 +- test/kb_scan.c | 6 +++++ 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 42f57b5023..d6876a516b 100644 --- a/power/tegra.c +++ b/power/tegra.c @@ -220,7 +220,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); -- cgit v1.2.1