Hello Felix Singer, Matt Delco, Patrick Georgi, Furquan Shaikh, Julius Werner, Angel Pons, Patrick Rudolph, EricR Lai,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/47084
to review the following change.
Change subject: libpayload/keyboard: Introduce keyboard_drain_input() ......................................................................
libpayload/keyboard: Introduce keyboard_drain_input()
Move the input-buffer draining into a function. It uses the low-level i8042 API directly to avoid conflicts with changes in the high-level keyboard API.
Change-Id: I9427c5b8be4d59c2ee3da12d6168d34590043682 Signed-off-by: Nico Huber nico.h@gmx.de --- M payloads/libpayload/drivers/i8042/keyboard.c 1 file changed, 9 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/84/47084/1
diff --git a/payloads/libpayload/drivers/i8042/keyboard.c b/payloads/libpayload/drivers/i8042/keyboard.c index c26babb..f8b68ab 100644 --- a/payloads/libpayload/drivers/i8042/keyboard.c +++ b/payloads/libpayload/drivers/i8042/keyboard.c @@ -172,6 +172,12 @@ #endif };
+static void keyboard_drain_input(void) +{ + while (i8042_data_ready_ps2()) + (void)i8042_read_data_ps2(); +} + static bool keyboard_cmd(unsigned char cmd) { const uint64_t timeout_us = 1*1000*1000; @@ -379,9 +385,7 @@ if (!i8042_probe() || !i8042_has_ps2()) return;
- /* Empty keyboard buffer */ - while (keyboard_havechar()) - keyboard_getchar(); + keyboard_drain_input();
/* Enable first PS/2 port */ i8042_cmd(I8042_CMD_EN_KB); @@ -413,9 +417,7 @@ if (!i8042_has_ps2()) return;
- /* Empty keyboard buffer */ - while (keyboard_havechar()) - keyboard_getchar(); + keyboard_drain_input();
/* Disable scanning */ keyboard_cmd(I8042_KBCMD_DEFAULT_DIS); @@ -424,8 +426,7 @@ i8042_cmd(I8042_CMD_DIS_KB);
/* Hand off with empty buffer */ - while (keyboard_havechar()) - keyboard_getchar(); + keyboard_drain_input();
/* Release keyboard controller driver */ i8042_close();
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47084 )
Change subject: libpayload/keyboard: Introduce keyboard_drain_input() ......................................................................
Patch Set 1: Code-Review+1
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47084 )
Change subject: libpayload/keyboard: Introduce keyboard_drain_input() ......................................................................
Patch Set 1: Code-Review+1
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47084 )
Change subject: libpayload/keyboard: Introduce keyboard_drain_input() ......................................................................
Patch Set 2: Code-Review+2
Hung-Te Lin has submitted this change. ( https://review.coreboot.org/c/coreboot/+/47084 )
Change subject: libpayload/keyboard: Introduce keyboard_drain_input() ......................................................................
libpayload/keyboard: Introduce keyboard_drain_input()
Move the input-buffer draining into a function. It uses the low-level i8042 API directly to avoid conflicts with changes in the high-level keyboard API.
Change-Id: I9427c5b8be4d59c2ee3da12d6168d34590043682 Signed-off-by: Nico Huber nico.h@gmx.de Reviewed-on: https://review.coreboot.org/c/coreboot/+/47084 Reviewed-by: Angel Pons th3fanbus@gmail.com Reviewed-by: Paul Menzel paulepanter@users.sourceforge.net Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M payloads/libpayload/drivers/i8042/keyboard.c 1 file changed, 9 insertions(+), 8 deletions(-)
Approvals: build bot (Jenkins): Verified Paul Menzel: Looks good to me, but someone else must approve Angel Pons: Looks good to me, approved
diff --git a/payloads/libpayload/drivers/i8042/keyboard.c b/payloads/libpayload/drivers/i8042/keyboard.c index 42d4fdc..46afdc0 100644 --- a/payloads/libpayload/drivers/i8042/keyboard.c +++ b/payloads/libpayload/drivers/i8042/keyboard.c @@ -172,6 +172,12 @@ #endif };
+static void keyboard_drain_input(void) +{ + while (i8042_data_ready_ps2()) + (void)i8042_read_data_ps2(); +} + static bool keyboard_cmd(unsigned char cmd) { const uint64_t timeout_us = cmd == I8042_KBCMD_RESET ? 1*1000*1000 : 200*1000; @@ -368,9 +374,7 @@ if (!i8042_probe() || !i8042_has_ps2()) return;
- /* Empty keyboard buffer */ - while (keyboard_havechar()) - keyboard_getchar(); + keyboard_drain_input();
/* Enable first PS/2 port */ i8042_cmd(I8042_CMD_EN_KB); @@ -400,9 +404,7 @@ if (!i8042_has_ps2()) return;
- /* Empty keyboard buffer */ - while (keyboard_havechar()) - keyboard_getchar(); + keyboard_drain_input();
/* Disable scanning */ keyboard_cmd(I8042_KBCMD_DEFAULT_DIS); @@ -411,8 +413,7 @@ i8042_cmd(I8042_CMD_DIS_KB);
/* Hand off with empty buffer */ - while (keyboard_havechar()) - keyboard_getchar(); + keyboard_drain_input();
/* Release keyboard controller driver */ i8042_close();
-
Angel Pons (Code Review) -
Hung-Te Lin (Code Review)
-
Nico Huber (Code Review)
-
Paul Menzel (Code Review)