diff options
author | Aseda Aboagye <aaboagye@google.com> | 2016-07-06 19:37:00 -0700 |
---|---|---|
committer | chrome-bot <chrome-bot@chromium.org> | 2016-07-19 18:33:36 -0700 |
commit | 87a071941b89e3f7fd3eb329b682e60b3fbd6c73 (patch) | |
tree | d82c7b76924dea16c7c9d4de69b591be3a9ddfe6 | |
parent | c0f6ac5e02cdb7a25f593af6dce7b0eea71d996d (diff) | |
download | chrome-ec-87a071941b89e3f7fd3eb329b682e60b3fbd6c73.tar.gz |
mkbp: Add support for buttons and switches.
Currently, the matrix keyboard protocol does not have support for
handling non-matrixed keys. This commit adds support for buttons which
do not appear in the keyboard matrix as well as switches.
Additionally, the keyboard FIFO is now just a general MKBP events FIFO
which MKBP events are free to use. Now, buttons and switches wil join
the key matrix event.
BUG=chrome-os-partner:54988
BUG=chrome-os-partner:54976
BUG=chromium:626863
BRANCH=None
TEST=Flash kevin, and verify that keyboard is still functional.
TEST=make -j buildall
CQ-DEPEND=CL:358926
Change-Id: If4ada904cbd5d77823a0710d4671484b198c9d91
Signed-off-by: Aseda Aboagye <aaboagye@google.com>
Reviewed-on: https://chromium-review.googlesource.com/358633
Commit-Ready: Aseda Aboagye <aaboagye@chromium.org>
Tested-by: Aseda Aboagye <aaboagye@chromium.org>
Reviewed-by: Randall Spangler <rspangler@chromium.org>
-rw-r--r-- | common/build.mk | 2 | ||||
-rw-r--r-- | common/keyboard_mkbp.c | 229 | ||||
-rw-r--r-- | common/mkbp_event.c | 63 | ||||
-rw-r--r-- | core/host/host_exe.lds | 5 | ||||
-rw-r--r-- | include/config.h | 6 | ||||
-rw-r--r-- | include/ec_commands.h | 19 | ||||
-rw-r--r-- | include/keyboard_mkbp.h | 23 | ||||
-rw-r--r-- | test/kb_mkbp.c | 54 | ||||
-rw-r--r-- | test/test_config.h | 2 |
9 files changed, 283 insertions, 120 deletions
diff --git a/common/build.mk b/common/build.mk index 1ebc48e63b..4cedd5b82f 100644 --- a/common/build.mk +++ b/common/build.mk @@ -106,4 +106,4 @@ ifeq ($(CTS_MODULE),) common-$(TEST_BUILD)+=test_util.o else common-y+=test_util.o -endif
\ No newline at end of file +endif diff --git a/common/keyboard_mkbp.c b/common/keyboard_mkbp.c index 0e5f766a08..b461227300 100644 --- a/common/keyboard_mkbp.c +++ b/common/keyboard_mkbp.c @@ -7,15 +7,21 @@ #include "atomic.h" #include "chipset.h" +#include "common.h" #include "console.h" +#include "ec_commands.h" #include "gpio.h" +#include "hooks.h" #include "host_command.h" #include "keyboard_config.h" +#include "keyboard_mkbp.h" #include "keyboard_protocol.h" #include "keyboard_raw.h" #include "keyboard_scan.h" #include "keyboard_test.h" +#include "lid_switch.h" #include "mkbp_event.h" +#include "power_button.h" #include "system.h" #include "task.h" #include "timer.h" @@ -26,14 +32,15 @@ #define CPRINTS(format, args...) cprints(CC_KEYBOARD, format, ## args) /* - * Keyboard FIFO depth. This needs to be big enough not to overflow if a + * Common FIFO depth. This needs to be big enough not to overflow if a * series of keys is pressed in rapid succession and the kernel is too busy * to read them out right away. * - * RAM usage is (depth * #cols); see kb_fifo[][] below. A 16-entry FIFO will - * consume 16x13=208 bytes, which is non-trivial but not horrible. + * RAM usage is (depth * #cols); A 16-entry FIFO will consume 16x13=208 bytes, + * which is non-trivial but not horrible. */ -#define KB_FIFO_DEPTH 16 + +#define FIFO_DEPTH 16 /* Changes to col,row here need to also be reflected in kernel. * drivers/input/mkbp.c ... see KEY_BATTERY. @@ -42,12 +49,16 @@ #define BATTERY_KEY_ROW 7 #define BATTERY_KEY_ROW_MASK (1 << BATTERY_KEY_ROW) -static uint32_t kb_fifo_start; /* first entry */ -static uint32_t kb_fifo_end; /* last entry */ -static uint32_t kb_fifo_entries; /* number of existing entries */ -static uint8_t kb_fifo[KB_FIFO_DEPTH][KEYBOARD_COLS]; +static uint32_t fifo_start; /* first entry */ +static uint32_t fifo_end; /* last entry */ +static uint32_t fifo_entries; /* number of existing entries */ +static struct ec_response_get_next_event fifo[FIFO_DEPTH]; static struct mutex fifo_mutex; +/* Button and switch state. */ +static uint32_t mkbp_button_state; +static uint32_t mkbp_switch_state; + /* Config for mkbp protocol; does not include fields from scan config */ struct ec_mkbp_protocol_config { uint32_t valid_mask; /* valid fields */ @@ -65,20 +76,41 @@ static struct ec_mkbp_protocol_config config = { EC_MKBP_VALID_DEBOUNCE_UP | EC_MKBP_VALID_FIFO_MAX_DEPTH, .valid_flags = EC_MKBP_FLAGS_ENABLE, .flags = EC_MKBP_FLAGS_ENABLE, - .fifo_max_depth = KB_FIFO_DEPTH, + .fifo_max_depth = FIFO_DEPTH, }; +static int get_data_size(enum ec_mkbp_event e) +{ + switch (e) { + case EC_MKBP_EVENT_KEY_MATRIX: + return KEYBOARD_COLS; + + case EC_MKBP_EVENT_HOST_EVENT: + case EC_MKBP_EVENT_BUTTON: + case EC_MKBP_EVENT_SWITCH: + return sizeof(uint32_t); + default: + /* For unknown types, say it's 0. */ + return 0; + } +} + /** - * Pop keyboard state from FIFO + * Pop MKBP event data from FIFO * * @return EC_SUCCESS if entry popped, EC_ERROR_UNKNOWN if FIFO is empty */ -static int kb_fifo_remove(uint8_t *buffp) +static int fifo_remove(uint8_t *buffp) { - if (!kb_fifo_entries) { + int size; + + if (!fifo_entries) { /* no entry remaining in FIFO : return last known state */ - int last = (kb_fifo_start + KB_FIFO_DEPTH - 1) % KB_FIFO_DEPTH; - memcpy(buffp, kb_fifo[last], KEYBOARD_COLS); + int last = (fifo_start + FIFO_DEPTH - 1) % FIFO_DEPTH; + + size = get_data_size(fifo[last].event_type); + + memcpy(buffp, &fifo[last].data, size); /* * Bail out without changing any FIFO indices and let the @@ -87,22 +119,16 @@ static int kb_fifo_remove(uint8_t *buffp) */ return EC_ERROR_UNKNOWN; } - memcpy(buffp, kb_fifo[kb_fifo_start], KEYBOARD_COLS); - kb_fifo_start = (kb_fifo_start + 1) % KB_FIFO_DEPTH; + /* Return just the event data. */ + size = get_data_size(fifo[fifo_start].event_type); + memcpy(buffp, &fifo[fifo_start].data, size); /* skip over event_type. */ - atomic_sub(&kb_fifo_entries, 1); + fifo_start = (fifo_start + 1) % FIFO_DEPTH; - return EC_SUCCESS; -} + atomic_sub(&fifo_entries, 1); -/** - * Assert host keyboard interrupt line. - */ -static void set_host_interrupt(int active) -{ - /* interrupt host by using active low EC_INT signal */ - gpio_set_level(GPIO_EC_INT_L, !active); + return EC_SUCCESS; } /*****************************************************************************/ @@ -110,70 +136,136 @@ static void set_host_interrupt(int active) void keyboard_clear_buffer(void) { + mkbp_clear_fifo(); +} + +void mkbp_clear_fifo(void) +{ int i; - CPRINTS("clearing keyboard fifo"); + CPRINTS("clearing MKBP common fifo"); - kb_fifo_start = 0; - kb_fifo_end = 0; - kb_fifo_entries = 0; - for (i = 0; i < KB_FIFO_DEPTH; i++) - memset(kb_fifo[i], 0, KEYBOARD_COLS); + fifo_start = 0; + fifo_end = 0; + fifo_entries = 0; + for (i = 0; i < FIFO_DEPTH; i++) + memset(&fifo[i], 0, sizeof(struct ec_response_get_next_event)); } test_mockable int keyboard_fifo_add(const uint8_t *buffp) { + return mkbp_fifo_add((uint8_t)EC_MKBP_EVENT_KEY_MATRIX, buffp); +} + +test_mockable int mkbp_fifo_add(uint8_t event_type, const uint8_t *buffp) +{ int ret = EC_SUCCESS; + uint8_t size; /* - * If keyboard protocol is not enabled, don't save the state to the - * FIFO or trigger an interrupt. + * If the data is a keyboard matrix and the keyboard protocol is not + * enabled, don't save the state to the FIFO or trigger an interrupt. */ - if (!(config.flags & EC_MKBP_FLAGS_ENABLE)) + if (!(config.flags & EC_MKBP_FLAGS_ENABLE) && + (event_type == EC_MKBP_EVENT_KEY_MATRIX)) return EC_SUCCESS; - if (kb_fifo_entries >= config.fifo_max_depth) { - CPRINTS("KB FIFO depth %d reached", + if (fifo_entries >= config.fifo_max_depth) { + CPRINTS("MKBP common FIFO depth %d reached", config.fifo_max_depth); ret = EC_ERROR_OVERFLOW; - goto kb_fifo_push_done; + goto fifo_push_done; } + size = get_data_size(event_type); mutex_lock(&fifo_mutex); - memcpy(kb_fifo[kb_fifo_end], buffp, KEYBOARD_COLS); - kb_fifo_end = (kb_fifo_end + 1) % KB_FIFO_DEPTH; - atomic_add(&kb_fifo_entries, 1); + fifo[fifo_end].event_type = event_type; + memcpy(&fifo[fifo_end].data, buffp, size); + fifo_end = (fifo_end + 1) % FIFO_DEPTH; + atomic_add(&fifo_entries, 1); mutex_unlock(&fifo_mutex); -kb_fifo_push_done: +fifo_push_done: - if (ret == EC_SUCCESS) { -#ifdef CONFIG_MKBP_EVENT - mkbp_send_event(EC_MKBP_EVENT_KEY_MATRIX); -#else - set_host_interrupt(1); -#endif - } + if (ret == EC_SUCCESS) + mkbp_send_event(event_type); return ret; } -#ifdef CONFIG_MKBP_EVENT -static int keyboard_get_next_event(uint8_t *out) +void mkbp_update_switches(uint32_t sw, int state) +{ + + mkbp_switch_state &= ~(1 << sw); + mkbp_switch_state |= (!!state << sw); + + mkbp_fifo_add(EC_MKBP_EVENT_SWITCH, + (const uint8_t *)&mkbp_switch_state); +} + +/** + * Handle lid changing state. + */ +static void lid_change(void) +{ + mkbp_update_switches(EC_MKBP_LID_OPEN, lid_is_open()); +} +DECLARE_HOOK(HOOK_LID_CHANGE, lid_change, HOOK_PRIO_LAST); +DECLARE_HOOK(HOOK_INIT, lid_change, HOOK_PRIO_INIT_LID+1); + +static int get_next_event(uint8_t *out, enum ec_mkbp_event evt) { - if (!kb_fifo_entries) + uint8_t t = fifo[fifo_start].event_type; + uint8_t size; + + if (!fifo_entries) return -1; - kb_fifo_remove(out); + /* + * We need to peek at the next event to check that we were called with + * the correct event. + */ + if (t != (uint8_t)evt) { + /* + * We were called with the wrong event. The next element in the + * FIFO's event type doesn't match with what we were called + * with. Return an error that we're busy. The caller will need + * to call us with the correct event first. + */ + return -EC_ERROR_BUSY; + } + + fifo_remove(out); /* Keep sending events if FIFO is not empty */ - if (kb_fifo_entries) - mkbp_send_event(EC_MKBP_EVENT_KEY_MATRIX); + if (fifo_entries) + mkbp_send_event(fifo[fifo_start].event_type); + + /* Return the correct size of the data. */ + size = get_data_size(t); + if (size) + return size; + else + return -EC_ERROR_UNKNOWN; +} - return KEYBOARD_COLS; +static int keyboard_get_next_event(uint8_t *out) +{ + return get_next_event(out, EC_MKBP_EVENT_KEY_MATRIX); } DECLARE_EVENT_SOURCE(EC_MKBP_EVENT_KEY_MATRIX, keyboard_get_next_event); -#endif + +static int button_get_next_event(uint8_t *out) +{ + return get_next_event(out, EC_MKBP_EVENT_BUTTON); +} +DECLARE_EVENT_SOURCE(EC_MKBP_EVENT_BUTTON, button_get_next_event); + +static int switch_get_next_event(uint8_t *out) +{ + return get_next_event(out, EC_MKBP_EVENT_SWITCH); +} +DECLARE_EVENT_SOURCE(EC_MKBP_EVENT_SWITCH, switch_get_next_event); void keyboard_send_battery_key(void) { @@ -190,27 +282,6 @@ void keyboard_send_battery_key(void) /*****************************************************************************/ /* Host commands */ - -static int keyboard_get_scan(struct host_cmd_handler_args *args) -{ - kb_fifo_remove(args->response); - /* if CONFIG_MKBP_EVENT is enabled, we still reset the interrupt - * (even if there is an another pending event) - * to be backward compatible with firmware using the old API to poll - * the keyboard, other software should use EC_CMD_GET_NEXT_EVENT - * instead of this command - */ - if (!kb_fifo_entries) - set_host_interrupt(0); - - args->response_size = KEYBOARD_COLS; - - return EC_RES_SUCCESS; -} -DECLARE_HOST_COMMAND(EC_CMD_MKBP_STATE, - keyboard_get_scan, - EC_VER_MASK(0)); - static int keyboard_get_info(struct host_cmd_handler_args *args) { struct ec_response_mkbp_info *r = args->response; @@ -303,7 +374,7 @@ static void keyscan_copy_config(const struct ec_mkbp_config *src, if (valid_mask & EC_MKBP_VALID_FIFO_MAX_DEPTH) { /* Sanity check for fifo depth */ dst->fifo_max_depth = MIN(src->fifo_max_depth, - KB_FIFO_DEPTH); + FIFO_DEPTH); } new_flags = dst->flags & ~valid_flags; diff --git a/common/mkbp_event.c b/common/mkbp_event.c index 60ca19d645..844aaf01f1 100644 --- a/common/mkbp_event.c +++ b/common/mkbp_event.c @@ -75,35 +75,49 @@ static int mkbp_get_next_event(struct host_cmd_handler_args *args) uint8_t *resp = args->response; const struct mkbp_event_source *src; - /* - * Find the next event to service. We do this in a round-robin - * way to make sure no event gets starved. - */ - for (i = 0; i < EC_MKBP_EVENT_COUNT; ++i) - if (event_is_set((last + i) % EC_MKBP_EVENT_COUNT)) - break; - - if (i == EC_MKBP_EVENT_COUNT) - return EC_RES_ERROR; + do { + /* + * Find the next event to service. We do this in a round-robin + * way to make sure no event gets starved. + */ + for (i = 0; i < EC_MKBP_EVENT_COUNT; ++i) + if (event_is_set((last + i) % EC_MKBP_EVENT_COUNT)) + break; - evt = (i + last) % EC_MKBP_EVENT_COUNT; - last = evt + 1; + if (i == EC_MKBP_EVENT_COUNT) + return EC_RES_UNAVAILABLE; - /* - * Clear the event before retrieving the event data in case the - * event source wants to send the same event. - */ - clear_event(evt); + evt = (i + last) % EC_MKBP_EVENT_COUNT; + last = evt + 1; - for (src = __mkbp_evt_srcs; src < __mkbp_evt_srcs_end; ++src) - if (src->event_type == evt) - break; + /* + * Clear the event before retrieving the event data in case the + * event source wants to send the same event. + */ + clear_event(evt); - if (src == __mkbp_evt_srcs_end) - return EC_RES_ERROR; + for (src = __mkbp_evt_srcs; src < __mkbp_evt_srcs_end; ++src) + if (src->event_type == evt) + break; + + if (src == __mkbp_evt_srcs_end) + return EC_RES_ERROR; + + resp[0] = evt; /* Event type */ + + /* + * get_data() can return -EC_ERROR_BUSY which indicates that the + * next element in the keyboard FIFO does not match what we were + * called with. For example, get_data is expecting a keyboard + * matrix, however the next element in the FIFO is a button + * event instead. Therefore, we have to service that button + * event first. + */ + data_size = src->get_data(resp + 1); + if (data_size == -EC_ERROR_BUSY) + set_event(evt); + } while (data_size == -EC_ERROR_BUSY); - resp[0] = evt; /* Event type */ - data_size = src->get_data(resp + 1); if (data_size < 0) return EC_RES_ERROR; args->response_size = 1 + data_size; @@ -116,4 +130,3 @@ static int mkbp_get_next_event(struct host_cmd_handler_args *args) DECLARE_HOST_COMMAND(EC_CMD_GET_NEXT_EVENT, mkbp_get_next_event, EC_VER_MASK(0)); - diff --git a/core/host/host_exe.lds b/core/host/host_exe.lds index 3ea0a706e4..6cdb172045 100644 --- a/core/host/host_exe.lds +++ b/core/host/host_exe.lds @@ -19,6 +19,11 @@ SECTIONS { *(.rodata.hcmds) __hcmds_end = .; + . = ALIGN(4); + __mkbp_evt_srcs = .; + KEEP(*(.rodata.evtsrcs)) + __mkbp_evt_srcs_end = .; + . = ALIGN(8); __hooks_init = .; *(.rodata.HOOK_INIT) diff --git a/include/config.h b/include/config.h index d1de4f5c79..b8da925fff 100644 --- a/include/config.h +++ b/include/config.h @@ -2232,6 +2232,12 @@ #define CONFIG_TEMP_SENSOR #endif +/******************************************************************************/ +/* The Matrix Keyboard Protocol depends on MKBP events. */ +#ifdef CONFIG_KEYBOARD_PROTOCOL_MKBP +#define CONFIG_MKBP_EVENT +#endif + /*****************************************************************************/ /* * Handle task-dependent configs. diff --git a/include/ec_commands.h b/include/ec_commands.h index 0d0f686e7e..f724a56458 100644 --- a/include/ec_commands.h +++ b/include/ec_commands.h @@ -2526,6 +2526,12 @@ enum ec_mkbp_event { /* New Sensor FIFO data. The event data is fifo_info structure. */ EC_MKBP_EVENT_SENSOR_FIFO = 2, + /* The state of the non-matrixed buttons have changed. */ + EC_MKBP_EVENT_BUTTON = 3, + + /* The state of the switches have changed. */ + EC_MKBP_EVENT_SWITCH = 4, + /* Number of MKBP events */ EC_MKBP_EVENT_COUNT, }; @@ -2541,6 +2547,10 @@ union ec_response_get_next_data { uint8_t rsvd[3]; struct ec_response_motion_sense_fifo_info info; } sensor_fifo; + + uint32_t buttons; + + uint32_t switches; } __packed; struct ec_response_get_next_event { @@ -2549,6 +2559,15 @@ struct ec_response_get_next_event { union ec_response_get_next_data data; } __packed; +/* Bit definitions for buttons and switches.*/ +/* Buttons */ +#define EC_MKBP_POWER_BUTTON 0 +#define EC_MKBP_VOL_UP 1 +#define EC_MKBP_VOL_DOWN 2 + +/* Switches */ +#define EC_MKBP_LID_OPEN 0 + /* Run keyboard factory test scanning */ #define EC_CMD_KEYBOARD_FACTORY_TEST 0x68 diff --git a/include/keyboard_mkbp.h b/include/keyboard_mkbp.h index 5ea3c54593..054593d1b4 100644 --- a/include/keyboard_mkbp.h +++ b/include/keyboard_mkbp.h @@ -9,6 +9,7 @@ #define __CROS_EC_KEYBOARD_MKBP_H #include "common.h" +#include "keyboard_config.h" /** * Add keyboard state into FIFO @@ -18,6 +19,20 @@ int keyboard_fifo_add(const uint8_t *buffp); /** + * Add an element to the common MKBP FIFO. + * + * @param event_type The MKBP event type. + * @param buffp Pointer to the event data to enqueue. + * @return EC_SUCCESS if entry added, EC_ERROR_OVERFLOW if FIFO is full. + */ +int mkbp_fifo_add(uint8_t event_type, const uint8_t *buffp); + +/** + * Clear the MKBP common FIFO. + */ +void mkbp_clear_fifo(void); + +/** * Send KEY_BATTERY keystroke. */ #ifdef CONFIG_KEYBOARD_PROTOCOL_MKBP @@ -26,4 +41,12 @@ void keyboard_send_battery_key(void); static inline void keyboard_send_battery_key(void) { } #endif +/** + * Update the state of the switches. + * + * @param sw The switch that changed. + * @param state The state of the switch. + */ +void mkbp_update_switches(uint32_t sw, int state); + #endif /* __CROS_EC_KEYBOARD_MKBP_H */ diff --git a/test/kb_mkbp.c b/test/kb_mkbp.c index e1abcc3adb..f838a0c411 100644 --- a/test/kb_mkbp.c +++ b/test/kb_mkbp.c @@ -71,31 +71,33 @@ int press_key(int c, int r, int pressed) int verify_key(int c, int r, int pressed) { struct host_cmd_handler_args args; - uint8_t mkbp_out[KEYBOARD_COLS]; + struct ec_response_get_next_event event; int i; - if (c >= 0 && r >= 0) { - ccprintf("Verify %s (%d, %d)\n", action[pressed], c, r); - set_state(c, r, pressed); - } else { - ccprintf("Verify last state\n"); - } - args.version = 0; - args.command = EC_CMD_MKBP_STATE; + args.command = EC_CMD_GET_NEXT_EVENT; args.params = NULL; args.params_size = 0; - args.response = mkbp_out; - args.response_max = sizeof(mkbp_out); + args.response = &event; + args.response_max = sizeof(event); args.response_size = 0; - if (host_command_process(&args) != EC_RES_SUCCESS) - return 0; + if (c >= 0 && r >= 0) { + ccprintf("Verify %s (%d, %d)\n", action[pressed], c, r); + set_state(c, r, pressed); - for (i = 0; i < KEYBOARD_COLS; ++i) - if (mkbp_out[i] != state[i]) + if (host_command_process(&args) != EC_RES_SUCCESS) return 0; + for (i = 0; i < KEYBOARD_COLS; ++i) + if (event.data.key_matrix[i] != state[i]) + return 0; + } else { + ccprintf("Verify no events available\n"); + if (host_command_process(&args) != EC_RES_UNAVAILABLE) + return 0; + } + return 1; } @@ -136,6 +138,26 @@ int set_kb_scan_enabled(int enabled) return mkbp_config(params); } +void clear_mkbp_events(void) +{ + struct host_cmd_handler_args args; + struct ec_response_get_next_event event; + + args.version = 0; + args.command = EC_CMD_GET_NEXT_EVENT; + args.params = NULL; + args.params_size = 0; + args.response = &event; + args.response_max = sizeof(event); + args.response_size = 0; + + /* + * We should return EC_RES_UNAVAILABLE if there are no MKBP events left. + */ + while (host_command_process(&args) != EC_RES_UNAVAILABLE) + ; +} + /*****************************************************************************/ /* Tests */ @@ -211,6 +233,8 @@ void run_test(void) ec_int_level = 1; test_reset(); + /* Clear any pending events such as lid open. */ + clear_mkbp_events(); RUN_TEST(single_key_press); RUN_TEST(test_fifo_size); RUN_TEST(test_enable); diff --git a/test/test_config.h b/test/test_config.h index 7e0a9d073d..3750015bc3 100644 --- a/test/test_config.h +++ b/test/test_config.h @@ -31,10 +31,12 @@ #ifdef TEST_KB_MKBP #define CONFIG_KEYBOARD_PROTOCOL_MKBP +#define CONFIG_MKBP_EVENT #endif #ifdef TEST_KB_SCAN #define CONFIG_KEYBOARD_PROTOCOL_MKBP +#define CONFIG_MKBP_EVENT #endif #ifdef TEST_MATH_UTIL |