Re: [PATCH 12/18] Ps2KbdCtrller: Make wait for SUCCESS after BAT non-fatal

["Wu, Hao A" <hao.a.wu@intel.com>]

Hello,

(Add Ray as the initial author for this driver.)
Inline comments below:


> -----Original Message-----
> From: Sean Rhodes <sean@starlabs.systems>
> Sent: Friday, February 11, 2022 5:26 AM
> To: devel@edk2.groups.io
> Cc: Wu, Hao A <hao.a.wu@intel.com>; Matt DeVillier
> <matt.devillier@gmail.com>
> Subject: [PATCH 12/18] Ps2KbdCtrller: Make wait for SUCCESS after BAT non-
> fatal
> 
> From: Matt DeVillier <matt.devillier@gmail.com>
> 
> Recent model Chromebooks only return ACK, but not
> BAT_SUCCESS, which causes hanging and failed ps2k init.
> To mitigate this, make the absence of BAT_SUCCESS reply
> non-fatal, and reduce the no-reply timeout from 4s to 1s.
> 
> Tested on google/dracia and purism/librem_14
> 
> Signed-off-by: Matt DeVillier <matt.devillier@gmail.com>
> ---
>  MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdCtrller.c | 8 +-------
>  1 file changed, 1 insertion(+), 7 deletions(-)
> 
> diff --git a/MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdCtrller.c
> b/MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdCtrller.c
> index 77dc226222..981309f8b0 100644
> --- a/MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdCtrller.c
> +++ b/MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdCtrller.c
> @@ -1731,13 +1731,7 @@ InitKeyboard (
>      //
> 
>      // wait for BAT completion code
> 
>      //
> 
> -    mWaitForValueTimeOut = KEYBOARD_BAT_TIMEOUT;
> 
> -
> 
> -    Status = KeyboardWaitForValue (ConsoleIn,
> KEYBOARD_8048_RETURN_8042_BAT_SUCCESS);
> 
> -    if (EFI_ERROR (Status)) {
> 
> -      KeyboardError (ConsoleIn, L"Keyboard self test failed!\n\r");
> 
> -      goto Done;
> 
> -    }
> 
> +    KeyboardWaitForValue (ConsoleIn,
> KEYBOARD_8048_RETURN_8042_BAT_SUCCESS);


Sorry for lacking of expertise in PS2 domain, I will try my best to provide comments based on my knowledge.

I am fine with removing the hard requirement on successful BAT (assuming problematic device will eventually fail on subsequent initialization steps).
I am also fine with reducing the timeout for BAT to 1 second, since I found the below information at link:
http://www-ug.eecg.toronto.edu/msl/nios_devices/datasheets/PS2%20Keyboard%20Protocol.htm
(But I am not sure if the above link is an official spec for PS2 keyboard)
"This BAT completion code must be sent 500~750 milliseconds after power-on."

But please help to update the patch by changing the value of KEYBOARD_BAT_TIMEOUT macro to 1 second instead of removing line:
mWaitForValueTimeOut = KEYBOARD_BAT_TIMEOUT;

@Ni, Ray, could you help to take a look at this patch as well? Thanks in advance.

Best Regards,
Hao Wu


> 
> 
> 
>      mWaitForValueTimeOut = KEYBOARD_WAITFORVALUE_TIMEOUT;
> 
> 
> 
> --
> 2.32.0