Patch that fixes an 'at_keyboard' module issue (unreliable key presses)

[Michael Bideau]

Hi all,

This patch fixes an issue that prevented the 'at_keyboard' module to
work (for me).

The cause is a bad/wrong return value in the function
'grub_at_keyboard_getkey()' in file 
'grub-core/term/at_keyboard.c' at line 234.


///////// patch /////////
diff --git a/grub-core/term/at_keyboard.c b/grub-core/term/at_keyboard.c
index f0a986eb1..597111077 100644
--- a/grub-core/term/at_keyboard.c
+++ b/grub-core/term/at_keyboard.c
@@ -234,7 +234,7 @@ grub_at_keyboard_getkey (struct grub_term_input *term __attribute__ ((unused)))
     return GRUB_TERM_NO_KEY;
 
   if (! KEYBOARD_ISREADY (grub_inb (KEYBOARD_REG_STATUS)))
-    return -1;
+    return GRUB_TERM_NO_KEY;
   at_key = grub_inb (KEYBOARD_REG_DATA);
   old_led = ps2_state.led_status;
///////// end of patch /////////


My symptoms were to have an unresponsive keyboard: keys needed to be
pressed 10x and more to
effectively be printed, sometimes generating multiple key presses
(after 1 or 2 sec of no printing).
Very problematic for typing passphrase in early stage (with
GRUB_ENABLE_CRYPTODISK).
When switching to 'console' terminal input, keyboard works perfectly.
It also worked great with grub 2.02 packaged by Debian (2.02+dfsg1-20).
It was not an output issue, but an input one.


I've managed to analyse the issue and found where it came from: the
following commit:
///////// commit /////////
Commit: 216950a4eea1a1ead1c28eaca94e34ea2ef2ad19
Author: Vladimir Serbinenko <address@hidden>
Date:   Mon May 8 21:41:22 2017 +0200

    at_keyboard: Split protocol from controller code.
    
    On vexpress controller is different but protocol is the same, so reuse the
    code.
///////// end of commit /////////


3 lines where moved from the function 'fetch_key()' in file 'grub-
core/term/at_keyboard.c', to the
begining of function 'grub_at_keyboard_getkey()' (same file).
But returning '-1' made sense when in function 'fetch_key()' but not
anymore in function
'grub_at_keyboard_getkey()', which should return 'GRUB_TERM_NO_KEY'.


See by yourself:

///////// git diff /////////
~> git diff ac6b41b89fbb82a3ba843a7ed19115d81432b604 216950a4eea1a1ead1c28eaca94e34ea2ef2ad19

..truncated..

diff --git a/grub-core/term/at_keyboard.c b/grub-core/term/at_keyboard.c
index b4ea9ff7e..3ab4e205f 100644
--- a/grub-core/term/at_keyboard.c
+++ b/grub-core/term/at_keyboard.c

..truncated..

@@ -386,164 +195,10 @@ keyboard_controller_led (grub_uint8_t leds)
   grub_outb (leds & 0x7, KEYBOARD_REG_DATA);
 }
 
-static int
-fetch_key (int *is_break)
-{
-  int was_ext = 0;
-  grub_uint8_t at_key;
-  int ret = 0;
-
-  if (! KEYBOARD_ISREADY (grub_inb (KEYBOARD_REG_STATUS)))
-    return -1;
-  at_key = grub_inb (KEYBOARD_REG_DATA);

..truncated..

@@ -566,51 +221,28 @@ grub_at_keyboard_is_alive (void)
 static int
 grub_at_keyboard_getkey (struct grub_term_input *term __attribute__ ((unused)))
 {
-  int code;
+  grub_uint8_t at_key;

..truncated..

   if (!grub_at_keyboard_is_alive ())
     return GRUB_TERM_NO_KEY;

..truncated..

+  if (! KEYBOARD_ISREADY (grub_inb (KEYBOARD_REG_STATUS)))
+    return -1;
+  at_key = grub_inb (KEYBOARD_REG_DATA);

..truncated..
///////// end of git diff /////////


If you want me to fill a bug, just tell me and I'll do it.


PS: this is my first contribution to grub, to "low level" code, and in
C, so please be kind ;-)

Best regards, and thank you all, for your great work on grub.
Michael Bideau [FR|France].