From 1cb328dc0c429233b7731d4e9eb1a8f03d4f6f8d Mon Sep 17 00:00:00 2001 From: Louis Yung-Chieh Lo Date: Wed, 9 May 2012 01:05:15 +0800 Subject: [PATCH] Fix some behaviors of keyboard command handlers. The phenomenon is that there is a char on-hold in port 0x60 and the kernel never picks it up. Hence the keyboard cannnot be recognized after resume. It comes from multiple reasons: 1. The command I8042_CMD_RESET_BAT(0xff) and I8042_CMD_ENABLE(0xf4) didn't clean the buffer. 2. clean_underlying_buffer() has clean the queue, but forgot to clean the TOH (TO Host). Add keyboard_clean_buffer() to clean the TOH (To Host). 3. When KB interrupt is just enabled, the IRQ didn't sent if there is a char queued in buffer already. keyboard_resume_interrupt() solves this. 4. Not all keyboard reset should reset the buffer. Only the enable/disble of controller RAM should NOT reset buffer. Other enable/disable should clean the buffer. 5. i8042 commands (those commands to port 0x64) should NOT return ACK even the parameter byte(s) goes to port 0x60. 6. Keyboard was disabled by kernel, but key stroke still sent to host (this needs the BIOS to fix). Also fix the minor issues: 1. I8042_CMD_RESEND should not return I8042_RET_ACK. 2. I8042_DIS_KB/I8042_ENA_KB should effect the controller RAM content. 3. only send out the scan code when keyboard is enabled. 4. add kblog command for future debug (disabled by default because it neeeds 1KB of memory). Signed-off-by: Louis Yung-Chieh Lo BUG=chrome-os-partner:9525 TEST=tested on link. Start from S0. 1. Run powerd_suspend. 2. Expect system is in S3. 3. Press any key to wake up system. 4. Expect system is up and keyboard is working. 5. repeat for 20+ times. Change-Id: I1c48822687d7c1f7ef0e8d8bca54bf9b05fd785f --- chip/lm4/keyboard_scan.c | 18 +++++++ chip/lm4/lpc.c | 38 ++++++++++++-- common/i8042.c | 14 ++++- common/keyboard.c | 109 +++++++++++++++++++++++++++++++-------- include/keyboard.h | 15 +++++- include/lpc.h | 6 +++ 6 files changed, 172 insertions(+), 28 deletions(-) diff --git a/chip/lm4/keyboard_scan.c b/chip/lm4/keyboard_scan.c index 39618ba70f..8e00deca38 100644 --- a/chip/lm4/keyboard_scan.c +++ b/chip/lm4/keyboard_scan.c @@ -470,6 +470,24 @@ void keyboard_put_char(uint8_t chr, int send_irq) #endif } +void keyboard_clear_buffer(void) +{ +#if defined(HOST_KB_BUS_LPC) + lpc_keyboard_clear_buffer(); +#else +#error "keyboard_scan needs to know what bus to use for keyboard interface" +#endif +} + +void keyboard_resume_interrupt(void) +{ +#if defined(HOST_KB_BUS_LPC) + lpc_keyboard_resume_irq(); +#else +#error "keyboard_scan needs to know what bus to use for keyboard interface" +#endif +} + /* We don't support this API yet, just return -1 */ int keyboard_get_scan(uint8_t **buffp, int max_bytes) { diff --git a/chip/lm4/lpc.c b/chip/lm4/lpc.c index cc32fbcc7f..e0b5a40ca8 100644 --- a/chip/lm4/lpc.c +++ b/chip/lm4/lpc.c @@ -27,6 +27,9 @@ #define LPC_SYSJUMP_TAG 0x4c50 /* "LP" */ +/* TO Host bit in LPCCH?ST) */ +#define TOH (1 << 0) + static uint32_t host_events; /* Currently pending SCI/SMI events */ static uint32_t event_mask[3]; /* Event masks for each type */ @@ -51,9 +54,8 @@ static void configure_gpio(void) } -static void wait_send_serirq(uint32_t lpcirqctl) { - LM4_LPC_LPCIRQCTL = lpcirqctl; - +static void wait_irq_sent(void) +{ /* TODO: udelay() is not graceful. Since the SIRQRIS is almost not * cleared in continuous mode and EC has problem to file * more than 1 frame in the quiet mode, this is the best way @@ -62,6 +64,12 @@ static void wait_send_serirq(uint32_t lpcirqctl) { * enough to guarantee the IRQ has been sent out. */ } +static void wait_send_serirq(uint32_t lpcirqctl) +{ + LM4_LPC_LPCIRQCTL = lpcirqctl; + wait_irq_sent(); +} + /* Manually generates an IRQ to host (edge-trigger). * * For SERIRQ quite mode, we need to set LM4_LPC_LPCIRQCTL twice. @@ -163,10 +171,11 @@ void host_send_response(int slot, const uint8_t *data, int size) /* Return true if the TOH is still set */ int lpc_keyboard_has_char(void) { - return (LM4_LPC_ST(LPC_CH_KEYBOARD) & (1 << 0 /* TOH */)) ? 1 : 0; + return (LM4_LPC_ST(LPC_CH_KEYBOARD) & TOH) ? 1 : 0; } +/* Put a char to host buffer and send IRQ if specified. */ void lpc_keyboard_put_char(uint8_t chr, int send_irq) { LPC_POOL_KEYBOARD[1] = chr; if (send_irq) { @@ -175,6 +184,27 @@ void lpc_keyboard_put_char(uint8_t chr, int send_irq) { } +/* Clear the keyboard buffer. */ +void lpc_keyboard_clear_buffer(void) +{ + /* Make sure the previous TOH and IRQ has been sent out. */ + wait_irq_sent(); + + LM4_LPC_ST(LPC_CH_KEYBOARD) &= ~TOH; + + /* Ensure there is no TOH set in this period. */ + wait_irq_sent(); +} + +/* Send an IRQ to host if there is a byte in buffer already. */ +void lpc_keyboard_resume_irq(void) +{ + if (lpc_keyboard_has_char()) { + lpc_manual_irq(1); /* IRQ#1 */ + } +} + + int lpc_comx_has_char(void) { return LM4_LPC_ST(LPC_CH_COMX) & 0x02; diff --git a/common/i8042.c b/common/i8042.c index fa5a7bc512..35e1f2b094 100644 --- a/common/i8042.c +++ b/common/i8042.c @@ -49,6 +49,7 @@ static int i8042_irq_enabled = 0; void i8042_init() { head_to_buffer = tail_to_buffer = 0; + keyboard_clear_buffer(); } @@ -86,6 +87,8 @@ static void enq_to_host(int len, uint8_t *to_host) /* Check if the buffer has enough space, then copy them to buffer. */ if ((tail_to_buffer + len) <= (head_to_buffer + HOST_BUFFER_SIZE - 1)) { for (from = 0, to = tail_to_buffer; from < len;) { + kblog_put('t', to); + kblog_put('T', to_host[from]); to_host_buffer[to++] = to_host[from++]; to %= HOST_BUFFER_SIZE; } @@ -99,6 +102,7 @@ static void enq_to_host(int len, uint8_t *to_host) */ void i8042_enable_keyboard_irq(void) { i8042_irq_enabled = 1; + keyboard_resume_interrupt(); } void i8042_disable_keyboard_irq(void) { @@ -133,6 +137,8 @@ void i8042_command_task(void) /* Get a char from buffer. */ chr = to_host_buffer[head_to_buffer]; + kblog_put('k', head_to_buffer); + kblog_put('K', chr); head_to_buffer = (head_to_buffer + 1) % HOST_BUFFER_SIZE; @@ -147,9 +153,15 @@ void i8042_command_task(void) enum ec_error_list i8042_send_to_host(int len, uint8_t *to_host) { + int i; + + for (i = 0; i < len; i++) + kblog_put('s', to_host[i]); + + /* Put to queue in memory */ enq_to_host(len, to_host); - /* Wake up the task to handle the command */ + /* Wake up the task to move from queue to the buffer to host. */ task_wake(TASK_ID_I8042CMD); return EC_SUCCESS; diff --git a/common/keyboard.c b/common/keyboard.c index 2163de3e00..4d6670866c 100644 --- a/common/keyboard.c +++ b/common/keyboard.c @@ -15,6 +15,7 @@ #include "lpc.h" #include "ec_commands.h" #include "registers.h" +#include "shared_mem.h" #include "system.h" #include "task.h" #include "timer.h" @@ -142,6 +143,15 @@ static uint16_t scancode_set2[CROS_ROW_NUM][CROS_COL_NUM] = { static uint8_t simulated_key[CROS_COL_NUM]; +/* Log the traffic between EC and host -- for debug only */ +struct kblog_t { + uint8_t type; + uint8_t byte; +}; +static struct kblog_t *kblog; +static int kblog_len; + + /* Change to set 1 if the I8042_XLATE flag is set. */ static enum scancode_set_list acting_code_set(enum scancode_set_list set) { @@ -238,7 +248,7 @@ static void reset_rate_and_delay(void) } -static void clean_underlying_buffer(void) +static void clear_underlying_buffer(void) { i8042_init(); } @@ -272,7 +282,8 @@ void keyboard_state_changed(int row, int col, int is_pressed) &len); if (ret == EC_SUCCESS) { ASSERT(len > 0); - i8042_send_to_host(len, scan_code); + if (keyboard_enabled) + i8042_send_to_host(len, scan_code); } if (is_pressed) { @@ -295,7 +306,7 @@ void keyboard_enable(int enable) } else if (keyboard_enabled && !enable) { /* disable */ reset_rate_and_delay(); - clean_underlying_buffer(); + typematic_len = 0; /* stop typematic */ } keyboard_enabled = enable; } @@ -325,13 +336,16 @@ void update_ctl_ram(uint8_t addr, uint8_t data) addr, data, orig); if (addr == 0x00) { /* the controller RAM */ + /* Enable IRQ before enable keyboard (queue chars to host) */ + if (!(orig & I8042_ENIRQ1) && (data & I8042_ENIRQ1)) + i8042_enable_keyboard_irq(); + /* Handle the I8042_KBD_DIS bit */ keyboard_enable(!(data & I8042_KBD_DIS)); - /* Handle the I8042_ENIRQ1 bit */ - if (!(orig & I8042_ENIRQ1) && (data & I8042_ENIRQ1)) - i8042_enable_keyboard_irq(); - else if ((orig & I8042_ENIRQ1) && !(data & I8042_ENIRQ1)) + /* Disable IRQ after disable keyboard so that every char + * must have informed the host. */ + if ((orig & I8042_ENIRQ1) && !(data & I8042_ENIRQ1)) i8042_disable_keyboard_irq(); } } @@ -357,6 +371,7 @@ int handle_keyboard_data(uint8_t data, uint8_t *output) int i; CPRINTF5("[KB recv data: 0x%02x]\n", data); + kblog_put('d', data); switch (data_port_state) { case STATE_SCANCODE: @@ -393,13 +408,11 @@ int handle_keyboard_data(uint8_t data, uint8_t *output) case STATE_WRITE_CMD_BYTE: CPRINTF5("[Eaten by STATE_WRITE_CMD_BYTE: 0x%02x]\n", data); update_ctl_ram(controller_ram_address, data); - output[out_len++] = I8042_RET_ACK; data_port_state = STATE_NORMAL; break; case STATE_ECHO_MOUSE: CPRINTF5("[Eaten by STATE_ECHO_MOUSE: 0x%02x]\n", data); - output[out_len++] = I8042_RET_ACK; output[out_len++] = data; data_port_state = STATE_NORMAL; break; @@ -462,30 +475,31 @@ int handle_keyboard_data(uint8_t data, uint8_t *output) case I8042_CMD_ENABLE: output[out_len++] = I8042_RET_ACK; keyboard_enable(1); + clear_underlying_buffer(); break; case I8042_CMD_RESET_DIS: output[out_len++] = I8042_RET_ACK; keyboard_enable(0); reset_rate_and_delay(); - clean_underlying_buffer(); + clear_underlying_buffer(); break; case I8042_CMD_RESET_DEF: output[out_len++] = I8042_RET_ACK; reset_rate_and_delay(); - clean_underlying_buffer(); + clear_underlying_buffer(); break; case I8042_CMD_RESET_BAT: + reset_rate_and_delay(); + clear_underlying_buffer(); output[out_len++] = I8042_RET_ACK; - keyboard_enable(0); output[out_len++] = I8042_RET_BAT; output[out_len++] = I8042_RET_BAT; break; case I8042_CMD_RESEND: - output[out_len++] = I8042_RET_ACK; save_for_resend = 0; for (i = 0; i < resend_command_len; ++i) output[out_len++] = resend_command[i]; @@ -525,6 +539,8 @@ int handle_keyboard_command(uint8_t command, uint8_t *output) int out_len = 0; CPRINTF5("[KB recv cmd: 0x%02x]\n", command); + kblog_put('c', command); + switch (command) { case I8042_READ_CMD_BYTE: output[out_len++] = read_ctl_ram(0); @@ -536,11 +552,11 @@ int handle_keyboard_command(uint8_t command, uint8_t *output) break; case I8042_DIS_KB: - keyboard_enable(0); + update_ctl_ram(0, read_ctl_ram(0) | I8042_KBD_DIS); break; case I8042_ENA_KB: - keyboard_enable(1); + update_ctl_ram(0, read_ctl_ram(0) & ~I8042_KBD_DIS); break; case I8042_RESET_SELF_TEST: @@ -590,7 +606,7 @@ int handle_keyboard_command(uint8_t command, uint8_t *output) } else { CPRINTF("[Unsupported cmd: 0x%02x]\n", command); reset_rate_and_delay(); - clean_underlying_buffer(); + clear_underlying_buffer(); output[out_len++] = I8042_RET_NAK; data_port_state = STATE_NORMAL; } @@ -644,10 +660,22 @@ void keyboard_set_power_button(int pressed) return; code_set = acting_code_set(scancode_set); - ret = i8042_send_to_host( - (code_set == SCANCODE_SET_2 && !pressed) ? 3 : 2, - code[code_set - SCANCODE_SET_1][pressed]); - ASSERT(ret == EC_SUCCESS); + if (keyboard_enabled) { + ret = i8042_send_to_host( + (code_set == SCANCODE_SET_2 && !pressed) ? 3 : 2, + code[code_set - SCANCODE_SET_1][pressed]); + ASSERT(ret == EC_SUCCESS); + } +} + + +void kblog_put(char type, uint8_t byte) +{ + if (kblog && kblog_len < MAX_KBLOG) { + kblog[kblog_len].type = type; + kblog[kblog_len].byte = byte; + kblog_len++; + } } @@ -662,8 +690,9 @@ void keyboard_typematic_task(void) if (typematic_delay <= 0) { /* re-send to host */ - i8042_send_to_host(typematic_len, - typematic_scan_code); + if (keyboard_enabled) + i8042_send_to_host(typematic_len, + typematic_scan_code); typematic_delay = refill_inter_delay * 1000; } } @@ -716,6 +745,7 @@ static int command_codeset(int argc, char **argv) ccprintf("Current scancode set: %d\n", scancode_set); ccprintf("I8042_XLATE: %d\n", controller_ram[0] & I8042_XLATE ? 1 : 0); + } else if (argc == 2) { set = strtoi(argv[1], NULL, 0); switch (set) { @@ -819,6 +849,41 @@ static int command_keyboard_press(int argc, char **argv) DECLARE_CONSOLE_COMMAND(kbpress, command_keyboard_press); +static int command_keyboard_log(int argc, char **argv) +{ + int i; + + if (argc == 1) { + ccprintf("KBC log (len=%d):\n", kblog_len); + for (i = 0; kblog && i < kblog_len; ++i) { + ccprintf("%c.%02x ", kblog[i].type, kblog[i].byte); + if ((i & 15) == 15) { + ccputs("\n"); + cflush(); + } + } + ccputs("\n"); + } else if (argc == 2 && !strcasecmp("on", argv[1])) { + if (!kblog) { + ASSERT(EC_SUCCESS == + shared_mem_acquire(sizeof(*kblog) * MAX_KBLOG, + 1, (char **)&kblog)); + kblog_len = 0; + } + } else if (argc == 2 && !strcasecmp("off", argv[1])) { + kblog_len = 0; + shared_mem_release(kblog); + kblog = NULL; + } else { + ccputs("Usage: kblog [on/off]\n"); + return EC_ERROR_UNKNOWN; + } + + return EC_SUCCESS; +} +DECLARE_CONSOLE_COMMAND(kblog, command_keyboard_log); + + /* Preserves the states of keyboard controller to keep the initialized states * between reboot_ec commands. Saving info include: * diff --git a/include/keyboard.h b/include/keyboard.h index af581e494c..6b71f1e93f 100644 --- a/include/keyboard.h +++ b/include/keyboard.h @@ -16,6 +16,8 @@ #define MAX_SCAN_CODE_LEN 4 +#define MAX_KBLOG 512 + enum scancode_set_list { SCANCODE_GET_SET = 0, SCANCODE_SET_1, @@ -51,6 +53,10 @@ int handle_keyboard_command(uint8_t command, uint8_t *output); void keyboard_set_power_button(int pressed); +/* Log the keyboard-related information */ +void kblog_put(char type, uint8_t byte); + + /* Register the board-specific keyboard matrix translation function. * The callback function accepts col/row and returns the scan code. * @@ -91,10 +97,17 @@ enum ec_error_list keyboard_register_callback(keyboard_callback cb); */ enum ec_error_list keyboard_get_state(uint8_t *bit_array); -/* Return true if the TOH is still set */ +/* Returns true if the to-host-buffer is non-empty. */ int keyboard_has_char(void); +/* Sends a char to host and triggers IRQ if specified. */ void keyboard_put_char(uint8_t chr, int send_irq); +/* Clears the keyboard buffer to host. */ +void keyboard_clear_buffer(void); + +/* Host just resumes the interrupt. Sends an interrupt if buffer is non-empty. + */ +void keyboard_resume_interrupt(void); #endif /* __INCLUDE_KEYBOARD_H */ diff --git a/include/lpc.h b/include/lpc.h index 45e777689a..e9f5cad690 100644 --- a/include/lpc.h +++ b/include/lpc.h @@ -25,6 +25,12 @@ int lpc_keyboard_has_char(void); /* Send a byte to host via port 0x60 and asserts IRQ if specified. */ void lpc_keyboard_put_char(uint8_t chr, int send_irq); +/* Clear the keyboard buffer. */ +void lpc_keyboard_clear_buffer(void); + +/* Send an IRQ to host if there is a byte in buffer already. */ +void lpc_keyboard_resume_irq(void); + /* Return non-zero if the COMx interface has received a character. */ int lpc_comx_has_char(void);