i8042: Send command response from priority queue

This patch also makes KEYPROTO task hold sending a scancode when it
receives SETLEDS command until it returns ACK to the second byte (and
leaves STATE_ATKBD_SETLEDS).

This patch also removes and repositions some kblog_put calls because
checking OBF and writing to DBBOUT must be done as atomically as
possible to minimize the race condition.

BUG=b:237981131,b:247795316
BRANCH=None
TEST=Taniks. Press search+alt then 'k' 100 times.

Signed-off-by: Daisuke Nojiri <dnojiri@chromium.org>
Change-Id: I7ccfae99b3657ead5fa9e3c337db623aaffdb0bc
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/3901253
Reviewed-by: Aseda Aboagye <aaboagye@chromium.org>
Commit-Queue: Aseda Aboagye <aaboagye@chromium.org>
Code-Coverage: Zoss <zoss-cl-coverage@prod.google.com>

2 files changed, 139 insertions, 73 deletions

diff --git a/common/keyboard_8042.c b/common/keyboard_8042.c
index e0371ddd5c..ce27828e1d 100644
--- a/common/keyboard_8042.c
+++ b/common/keyboard_8042.c
@@ -78,6 +78,15 @@ enum scancode_set_list {
 #define KB_TO_HOST_RETRIES 3
 
 /*
+ * Timeout for SETLEDS command. Kernel is supposed to send the second byte
+ * within this period. When timeout occurs, the second byte is received as
+ * 'Unsupported AT keyboard command 0x00' (or 0x04). You can evaluate your
+ * timeout is too long or too short by calculating the duration between 'KB
+ * SETLEDS' and 'Unsupported AT...'.
+ */
+#define SETLEDS_TIMEOUT (30 * MSEC)
+
+/*
  * Mutex to control write access to the to-host buffer head.  Don't need to
  * mutex the tail because reads are only done in one place.
  */
@@ -87,6 +96,7 @@ static mutex_t to_host_mutex;
 enum {
 	CHAN_KBD = 0,
 	CHAN_AUX,
+	CHAN_CMD,
 };
 struct data_byte {
 	uint8_t chan;
@@ -94,6 +104,7 @@ struct data_byte {
 };
 
 static struct queue const to_host = QUEUE_NULL(16, struct data_byte);
+static struct queue const to_host_cmd = QUEUE_NULL(16, struct data_byte);
 
 /* Queue command/data from the host */
 enum {
@@ -162,6 +173,7 @@ static int typematic_inter_delay;
 static int typematic_len; /* length of typematic_scan_code */
 static uint8_t typematic_scan_code[MAX_SCAN_CODE_LEN];
 static timestamp_t typematic_deadline;
+static timestamp_t setleds_deadline;
 
 #define KB_SYSJUMP_TAG 0x4b42 /* "KB" */
 #define KB_HOOK_VERSION 2
@@ -182,18 +194,16 @@ struct kblog_t {
 	/*
 	 * Type:
 	 *
-	 * s = byte enqueued to send to host
 	 * a = aux byte enqueued to send to host
-	 * t = to-host queue tail pointer before type='s' bytes enqueued
-	 *
-	 * d = data byte from host
 	 * c = command byte from host
-	 *
-	 * k = to-host queue head pointer before byte dequeued
-	 * K = byte actually sent to host via LPC
-	 * A = byte actually sent to host via LPC as AUX
-	 *
+	 * d = data byte from host
+	 * r = typematic
+	 * s = byte enqueued to send to host
+	 * t = to-host queue tail pointer before type='s' bytes enqueued
+	 * u = byte enqueued to send to host with priority
 	 * x = to_host queue was cleared
+	 * A = byte actually sent to host via LPC as AUX
+	 * K = byte actually sent to host via LPC
 	 *
 	 * The to-host head and tail pointers are logged pre-wrapping to the
 	 * queue size.  This means that they continually increment as units
@@ -265,7 +275,7 @@ static void aux_enable_irq(int enable)
  * host cannot read the previous byte away in time.
  *
  * @param len		Number of bytes to send to the host
- * @param to_host	Data to send
+ * @param bytes		Data to send
  * @param chan		Channel to send data on
  */
 static void i8042_send_to_host(int len, const uint8_t *bytes, uint8_t chan,
@@ -281,15 +291,29 @@ static void i8042_send_to_host(int len, const uint8_t *bytes, uint8_t chan,
 		for (i = 0; i < len; i++)
 			kblog_put('r', bytes[i]);
 	} else {
-		for (i = 0; i < len; i++)
-			kblog_put(chan == CHAN_AUX ? 'a' : 's', bytes[i]);
+		struct queue const *queue = &to_host;
+
+		if (chan == CHAN_CMD)
+			queue = &to_host_cmd;
 
-		if (queue_space(&to_host) >= len) {
-			kblog_put('t', to_host.state->tail);
+		for (i = 0; i < len; i++) {
+			char type;
+
+			if (chan == CHAN_AUX)
+				type = 'a';
+			else if (chan == CHAN_CMD)
+				type = 'u';
+			else
+				type = 's';
+			kblog_put(type, bytes[i]);
+		}
+
+		if (queue_space(queue) >= len) {
+			kblog_put('t', queue->state->tail);
 			for (i = 0; i < len; i++) {
 				data.chan = chan;
 				data.byte = bytes[i];
-				queue_add_unit(&to_host, &data);
+				queue_add_unit(queue, &data);
 			}
 		}
 	}
@@ -417,6 +441,7 @@ void keyboard_clear_buffer(void)
 	mutex_lock(&to_host_mutex);
 	kblog_put('x', queue_count(&to_host));
 	queue_init(&to_host);
+	queue_init(&to_host_cmd);
 	mutex_unlock(&to_host_mutex);
 	lpc_keyboard_clear_buffer();
 }
@@ -661,6 +686,8 @@ static int handle_keyboard_data(uint8_t data, uint8_t *output)
 			/* Chrome OS doesn't have keyboard LEDs, so ignore */
 			output[out_len++] = ATKBD_RET_ACK;
 			data_port_state = STATE_ATKBD_SETLEDS;
+			setleds_deadline.val = get_time().val + SETLEDS_TIMEOUT;
+			CPRINTS5("KB SETLEDS");
 			break;
 
 		case ATKBD_CMD_EX_SETLEDS:
@@ -868,20 +895,23 @@ static void i8042_handle_from_host(void)
 	struct host_byte h;
 	int ret_len;
 	uint8_t output[MAX_SCAN_CODE_LEN];
-	uint8_t chan = CHAN_KBD;
+	uint8_t chan;
 
 	while (queue_remove_unit(&from_host, &h)) {
 		if (h.type == HOST_COMMAND) {
 			ret_len = handle_keyboard_command(h.byte, output);
+			chan = CHAN_KBD;
 		} else {
 			CPRINTS5("KB recv data: 0x%02x", h.byte);
 			kblog_put('d', h.byte);
 
 			if (IS_ENABLED(CONFIG_8042_AUX) &&
-			    handle_mouse_data(h.byte, output, &ret_len))
+			    handle_mouse_data(h.byte, output, &ret_len)) {
 				chan = CHAN_AUX;
-			else
+			} else {
 				ret_len = handle_keyboard_data(h.byte, output);
+				chan = CHAN_CMD;
+			}
 		}
 
 		i8042_send_to_host(ret_len, output, chan, 0);
@@ -925,10 +955,14 @@ void keyboard_protocol_task(void *u)
 			i8042_handle_from_host();
 
 			/* Check if we have data to send to host */
-			if (queue_is_empty(&to_host))
+			if (queue_is_empty(&to_host) &&
+			    queue_is_empty(&to_host_cmd))
 				break;
 
-			/* Handle data waiting for host */
+			/*
+			 * Check if the output buffer is full. We can't proceed
+			 * until the host read the data.
+			 */
 			if (lpc_keyboard_has_char()) {
 				/* If interrupts disabled, nothing we can do */
 				if (!i8042_keyboard_irq_enabled &&
@@ -946,26 +980,53 @@ void keyboard_protocol_task(void *u)
 				 * data?  Send it another interrupt in case it
 				 * somehow missed the first one.
 				 */
-				CPRINTS("KB extra IRQ");
+				CPRINTS("KB host not responding");
 				lpc_keyboard_resume_irq();
 				retries = 0;
 				break;
 			}
 
+			/*
+			 * We know DBBOUT is empty but we need act quickly as
+			 * the host might be sending a byte to DBBIN.
+			 *
+			 * So be cautious if you're adding any code below up to
+			 * lpc_keyboard_put_char since that'll increase the race
+			 * condition. For example, you don't want to add CPRINTS
+			 * or kblog_put.
+			 *
+			 * We should claim OBF=1 atomically to prevent the host
+			 * from writing to DBBIN (i.e. set-ibf-if-not-obf). It's
+			 * not possible for NPCX because NPCX's HIKMST-IBF is
+			 * read-only.
+			 */
+
 			/* Get a char from buffer. */
-			kblog_put('k', to_host.state->head);
-			queue_remove_unit(&to_host, &entry);
+			if (queue_count(&to_host_cmd)) {
+				queue_remove_unit(&to_host_cmd, &entry);
+			} else if (data_port_state == STATE_ATKBD_SETLEDS) {
+				/* to_host_cmd is empty but in SETLEDS */
+				if (!timestamp_expired(setleds_deadline, &t))
+					/* Let's wait for the 2nd byte. */
+					break;
+				/* Didn't receive 2nd byte. Go back to CMD. */
+				CPRINTS("KB SETLEDS timeout");
+				data_port_state = STATE_ATKBD_CMD;
+			} else {
+				/* to_host isn't empty && not in SETLEDS */
+				queue_remove_unit(&to_host, &entry);
+			}
 
 			/* Write to host. */
 			if (entry.chan == CHAN_AUX &&
 			    IS_ENABLED(CONFIG_8042_AUX)) {
-				kblog_put('A', entry.byte);
 				lpc_aux_put_char(entry.byte,
 						 i8042_aux_irq_enabled);
+				kblog_put('A', entry.byte);
 			} else {
-				kblog_put('K', entry.byte);
 				lpc_keyboard_put_char(
 					entry.byte, i8042_keyboard_irq_enabled);
+				kblog_put('K', entry.byte);
 			}
 			retries = 0;
 		}
diff --git a/test/kb_8042.c b/test/kb_8042.c
index e7d7690cff..eb043d5f82 100644
--- a/test/kb_8042.c
+++ b/test/kb_8042.c
@@ -261,6 +261,7 @@ static int _read_cmd_byte(uint8_t *cmd)
 void before_test(void)
 {
 	/* Make sure all tests start with the controller in the same state */
+	keyboard_clear_buffer();
 	_write_cmd_byte(I8042_XLATE | I8042_AUX_DIS | I8042_KBD_DIS);
 }
 
@@ -274,7 +275,7 @@ void after_test(void)
 	}
 }
 
-static int test_8042_aux_loopback(void)
+test_static int test_8042_aux_loopback(void)
 {
 	/* Disable all IRQs */
 	WRITE_CMD_BYTE(0);
@@ -293,7 +294,7 @@ static int test_8042_aux_loopback(void)
 	return EC_SUCCESS;
 }
 
-static int test_8042_aux_two_way_communication(void)
+test_static int test_8042_aux_two_way_communication(void)
 {
 	/* Enable AUX IRQ */
 	WRITE_CMD_BYTE(I8042_ENIRQ12);
@@ -311,7 +312,7 @@ static int test_8042_aux_two_way_communication(void)
 	return EC_SUCCESS;
 }
 
-static int test_8042_aux_inhibit(void)
+test_static int test_8042_aux_inhibit(void)
 {
 	/* Enable AUX IRQ, but inhibit the AUX device from sending data. */
 	WRITE_CMD_BYTE(I8042_ENIRQ12 | I8042_AUX_DIS);
@@ -339,7 +340,7 @@ static int test_8042_aux_inhibit(void)
 	return EC_SUCCESS;
 }
 
-static int test_8042_aux_controller_commands(void)
+test_static int test_8042_aux_controller_commands(void)
 {
 	uint8_t ctrl;
 
@@ -359,7 +360,7 @@ static int test_8042_aux_controller_commands(void)
 	return EC_SUCCESS;
 }
 
-static int test_8042_aux_test_command(void)
+test_static int test_8042_aux_test_command(void)
 {
 	i8042_write_cmd(I8042_TEST_MOUSE);
 
@@ -368,7 +369,7 @@ static int test_8042_aux_test_command(void)
 	return EC_SUCCESS;
 }
 
-static int test_8042_self_test(void)
+test_static int test_8042_self_test(void)
 {
 	i8042_write_cmd(I8042_RESET_SELF_TEST);
 	VERIFY_LPC_CHAR("\x55");
@@ -376,7 +377,7 @@ static int test_8042_self_test(void)
 	return EC_SUCCESS;
 }
 
-static int test_8042_keyboard_test_command(void)
+test_static int test_8042_keyboard_test_command(void)
 {
 	i8042_write_cmd(I8042_TEST_KB_PORT);
 	VERIFY_LPC_CHAR("\x00"); /* Data and Clock are not stuck */
@@ -384,7 +385,7 @@ static int test_8042_keyboard_test_command(void)
 	return EC_SUCCESS;
 }
 
-static int test_8042_keyboard_controller_commands(void)
+test_static int test_8042_keyboard_controller_commands(void)
 {
 	uint8_t ctrl;
 
@@ -404,7 +405,7 @@ static int test_8042_keyboard_controller_commands(void)
 	return EC_SUCCESS;
 }
 
-static int test_8042_keyboard_key_pressed_while_inhibited(void)
+test_static int test_8042_keyboard_key_pressed_while_inhibited(void)
 {
 	ENABLE_KEYSTROKE(1);
 
@@ -440,7 +441,8 @@ static int test_8042_keyboard_key_pressed_while_inhibited(void)
 	return EC_SUCCESS;
 }
 
-static int test_8042_keyboard_key_pressed_before_inhibit_using_cmd_byte(void)
+test_static int
+test_8042_keyboard_key_pressed_before_inhibit_using_cmd_byte(void)
 {
 	ENABLE_KEYSTROKE(1);
 	/* Simulate a keypress on the keyboard */
@@ -472,7 +474,7 @@ static int test_8042_keyboard_key_pressed_before_inhibit_using_cmd_byte(void)
 	return EC_SUCCESS;
 }
 
-static int
+test_static int
 test_8042_keyboard_key_pressed_before_inhibit_using_cmd_byte_with_read(void)
 {
 	uint8_t cmd;
@@ -524,7 +526,7 @@ test_8042_keyboard_key_pressed_before_inhibit_using_cmd_byte_with_read(void)
 	return EC_SUCCESS;
 }
 
-static int test_8042_keyboard_key_pressed_before_inhibit_using_cmd(void)
+test_static int test_8042_keyboard_key_pressed_before_inhibit_using_cmd(void)
 {
 	ENABLE_KEYSTROKE(1);
 	/* Simulate a keypress on the keyboard */
@@ -556,7 +558,7 @@ static int test_8042_keyboard_key_pressed_before_inhibit_using_cmd(void)
 	return EC_SUCCESS;
 }
 
-static int test_single_key_press(void)
+test_static int test_single_key_press(void)
 {
 	ENABLE_KEYSTROKE(1);
 	press_key(1, 1, 1);
@@ -572,7 +574,7 @@ static int test_single_key_press(void)
 	return EC_SUCCESS;
 }
 
-static int test_disable_keystroke(void)
+test_static int test_disable_keystroke(void)
 {
 	ENABLE_KEYSTROKE(0);
 	press_key(1, 1, 1);
@@ -583,7 +585,7 @@ static int test_disable_keystroke(void)
 	return EC_SUCCESS;
 }
 
-static int test_typematic(void)
+test_static int test_typematic(void)
 {
 	ENABLE_KEYSTROKE(1);
 
@@ -610,7 +612,7 @@ static int test_typematic(void)
 	return EC_SUCCESS;
 }
 
-static int test_atkbd_get_scancode(void)
+test_static int test_atkbd_get_scancode(void)
 {
 	SET_SCANCODE(1);
 
@@ -635,7 +637,7 @@ static int test_atkbd_get_scancode(void)
 	return EC_SUCCESS;
 }
 
-static int test_atkbd_set_scancode_with_keystroke_disabled(void)
+test_static int test_atkbd_set_scancode_with_keystroke_disabled(void)
 {
 	ENABLE_KEYSTROKE(0);
 
@@ -647,7 +649,7 @@ static int test_atkbd_set_scancode_with_keystroke_disabled(void)
 	return EC_SUCCESS;
 }
 
-static int test_atkbd_set_scancode_with_key_press_before_set(void)
+test_static int test_atkbd_set_scancode_with_key_press_before_set(void)
 {
 	ENABLE_KEYSTROKE(0);
 	ENABLE_KEYSTROKE(1);
@@ -660,26 +662,17 @@ static int test_atkbd_set_scancode_with_key_press_before_set(void)
 	 * ATKBD_CMD_SSCANSET should cause the keyboard to stop scanning, flush
 	 * the keyboards output queue, and reset the typematic key.
 	 */
-	keyboard_host_write(ATKBD_CMD_SSCANSET, 0);
-	/* Read out the scan code that got pushed into the output buffer before
-	 * the command was sent.
-	 */
-	VERIFY_LPC_CHAR("\x01");
+	i8042_write_data(ATKBD_CMD_SSCANSET);
+	VERIFY_ATKBD_ACK();
 
 	/*
 	 * FIXME: This is wrong. The keyboard's output queue should have been
 	 * flushed when it received the `ATKBD_CMD_SSCANSET` command.
 	 */
-	VERIFY_LPC_CHAR("\x81");
-
-	/* This is the ACK for `ATKBD_CMD_SSCANSET`. */
-	VERIFY_ATKBD_ACK();
-
-	/* The keyboard has flushed the buffer so no more keys. */
-	VERIFY_NO_CHAR();
+	VERIFY_LPC_CHAR("\x01\x81");
 
 	/* Finish setting scan code 1 */
-	keyboard_host_write(1, 0);
+	i8042_write_data(1);
 	VERIFY_ATKBD_ACK();
 
 	/* Key scanning should be restored. */
@@ -690,7 +683,7 @@ static int test_atkbd_set_scancode_with_key_press_before_set(void)
 	return EC_SUCCESS;
 }
 
-static int test_atkbd_set_scancode_with_key_press_during_set(void)
+test_static int test_atkbd_set_scancode_with_key_press_during_set(void)
 {
 	ENABLE_KEYSTROKE(1);
 
@@ -698,7 +691,7 @@ static int test_atkbd_set_scancode_with_key_press_during_set(void)
 	 * ATKBD_CMD_SSCANSET should cause the keyboard to stop scanning, flush
 	 * the keyboards output queue, and reset the typematic key.
 	 */
-	keyboard_host_write(ATKBD_CMD_SSCANSET, 0);
+	i8042_write_data(ATKBD_CMD_SSCANSET);
 	VERIFY_ATKBD_ACK();
 
 	/* These keypresses should be dropped. */
@@ -711,7 +704,7 @@ static int test_atkbd_set_scancode_with_key_press_during_set(void)
 	VERIFY_LPC_CHAR("\x01\x81");
 
 	/* Finish setting scan code 1 */
-	keyboard_host_write(1, 0);
+	i8042_write_data(1);
 	VERIFY_ATKBD_ACK();
 
 	/* Key scanning should be restored. */
@@ -722,7 +715,7 @@ static int test_atkbd_set_scancode_with_key_press_during_set(void)
 	return EC_SUCCESS;
 }
 
-static int test_atkbd_echo(void)
+test_static int test_atkbd_echo(void)
 {
 	i8042_write_data(ATKBD_CMD_DIAG_ECHO);
 	VERIFY_ATKBD_ACK();
@@ -732,7 +725,7 @@ static int test_atkbd_echo(void)
 	return EC_SUCCESS;
 }
 
-static int test_atkbd_get_id(void)
+test_static int test_atkbd_get_id(void)
 {
 	i8042_write_data(ATKBD_CMD_GETID);
 	VERIFY_ATKBD_ACK();
@@ -747,8 +740,10 @@ static int test_atkbd_get_id(void)
 	return EC_SUCCESS;
 }
 
-static int test_atkbd_set_leds_keypress_during(void)
+test_static int test_atkbd_set_leds_keypress_during(void)
 {
+	ENABLE_KEYSTROKE(1);
+
 	/* This should pause scanning. */
 	i8042_write_data(ATKBD_CMD_SETLEDS);
 	VERIFY_ATKBD_ACK();
@@ -756,16 +751,20 @@ static int test_atkbd_set_leds_keypress_during(void)
 	/* Simulate keypress while keyboard is waiting for option byte */
 	press_key(1, 1, 1);
 	press_key(1, 1, 0);
-	/* FIXME: This is wrong, we shouldn't have any key strokes */
-	VERIFY_LPC_CHAR("\x01\x81");
+
+	/* Scancode is kept in queue during SETLEDS. */
+	VERIFY_NO_CHAR();
 
 	i8042_write_data(0x01);
 	VERIFY_ATKBD_ACK();
 
+	/* Scancode previously queued should be sent now. */
+	VERIFY_LPC_CHAR("\x01\x81");
+
 	return EC_SUCCESS;
 }
 
-static int test_atkbd_set_leds_abort_set(void)
+test_static int test_atkbd_set_leds_abort_set(void)
 {
 	i8042_write_data(ATKBD_CMD_SETLEDS);
 	VERIFY_ATKBD_ACK();
@@ -785,7 +784,7 @@ static int test_atkbd_set_leds_abort_set(void)
 	return EC_SUCCESS;
 }
 
-static int test_atkbd_set_ex_leds(void)
+test_static int test_atkbd_set_ex_leds(void)
 {
 	i8042_write_data(ATKBD_CMD_EX_SETLEDS);
 	VERIFY_ATKBD_ACK();
@@ -801,9 +800,10 @@ static int test_atkbd_set_ex_leds(void)
 	return EC_SUCCESS;
 }
 
-static int test_scancode_set2(void)
+test_static int test_scancode_set2(void)
 {
 	SET_SCANCODE(2);
+	ENABLE_KEYSTROKE(1);
 
 	WRITE_CMD_BYTE(READ_CMD_BYTE() | I8042_XLATE);
 	press_key(1, 1, 1);
@@ -820,7 +820,7 @@ static int test_scancode_set2(void)
 	return EC_SUCCESS;
 }
 
-static int test_power_button(void)
+test_static int test_power_button(void)
 {
 	ENABLE_KEYSTROKE(0);
 
@@ -857,7 +857,7 @@ static int test_power_button(void)
 	return EC_SUCCESS;
 }
 
-static int test_sysjump(void)
+test_static int test_sysjump(void)
 {
 	SET_SCANCODE(2);
 	ENABLE_KEYSTROKE(1);
@@ -868,15 +868,17 @@ static int test_sysjump(void)
 	return EC_ERROR_UNKNOWN;
 }
 
-static int test_sysjump_cont(void)
+test_static int test_sysjump_cont(void)
 {
 	WRITE_CMD_BYTE(READ_CMD_BYTE() | I8042_XLATE);
+
 	press_key(1, 1, 1);
 	VERIFY_LPC_CHAR("\x01");
 	press_key(1, 1, 0);
 	VERIFY_LPC_CHAR("\x81");
 
 	WRITE_CMD_BYTE(READ_CMD_BYTE() & ~I8042_XLATE);
+
 	press_key(1, 1, 1);
 	VERIFY_LPC_CHAR("\x76");
 	press_key(1, 1, 0);
@@ -885,7 +887,7 @@ static int test_sysjump_cont(void)
 	return EC_SUCCESS;
 }
 
-static const struct ec_response_keybd_config keybd_config = {
+test_static const struct ec_response_keybd_config keybd_config = {
 	.num_top_row_keys = 13,
 	.action_keys = {
 		TK_BACK,		/* T1 */
@@ -910,7 +912,7 @@ board_vivaldi_keybd_config(void)
 	return &keybd_config;
 }
 
-static int test_ec_cmd_get_keybd_config(void)
+test_static int test_ec_cmd_get_keybd_config(void)
 {
 	struct ec_response_keybd_config resp;
 	int rv;
@@ -931,22 +933,25 @@ static int test_ec_cmd_get_keybd_config(void)
 	return EC_SUCCESS;
 }
 
-static int test_vivaldi_top_keys(void)
+test_static int test_vivaldi_top_keys(void)
 {
 	SET_SCANCODE(2);
 
 	/* Test REFRESH key */
 	WRITE_CMD_BYTE(READ_CMD_BYTE() | I8042_XLATE);
+
 	press_key(2, 3, 1); /* Press T2 */
 	VERIFY_LPC_CHAR("\xe0\x67"); /* Check REFRESH scancode in set-1 */
 
 	/* Test SNAPSHOT key */
 	WRITE_CMD_BYTE(READ_CMD_BYTE() | I8042_XLATE);
+
 	press_key(4, 3, 1); /* Press T2 */
 	VERIFY_LPC_CHAR("\xe0\x13"); /* Check SNAPSHOT scancode in set-1 */
 
 	/* Test VOL_UP key */
 	WRITE_CMD_BYTE(READ_CMD_BYTE() | I8042_XLATE);
+
 	press_key(5, 3, 1); /* Press T2 */
 	VERIFY_LPC_CHAR("\xe0\x30"); /* Check VOL_UP scancode in set-1 */