1. Patchew
  2. EDK2
  3. View series

[edk2-devel] [PATCH] MdeModulePkg/Usb/Keyboard.c: Don't request protocol before setting

Sean Rhodes posted 1 patch 2 years, 2 months ago
Failed in applying to current master (apply log)
MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c | 18 +++++-------------
1 file changed, 5 insertions(+), 13 deletions(-)
[edk2-devel] [PATCH] MdeModulePkg/Usb/Keyboard.c: Don't request protocol before setting
Posted by Sean Rhodes 2 years, 2 months ago 
No need to check the interface protocol then conditionally setting,
just set it to BOOT_PROTOCOL and check for error.

This is what Linux does for HID devices as some don't follow the USB spec.
One example is the Aspeed BMC HID keyboard device, which adds a massive
boot delay without this patch as it doesn't respond to 'GetProtocolRequest'.

Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Signed-off-by: Matt DeVillier <matt.devillier@gmail.com>
Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
Signed-off-by: Sean Rhodes <sean@starlabs.systems>
---
 MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c | 18 +++++-------------
 1 file changed, 5 insertions(+), 13 deletions(-)

diff --git a/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c b/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
index 5a94a4dda7..b5a6459a35 100644
--- a/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
+++ b/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
@@ -805,7 +805,6 @@ InitUSBKeyboard (
   )
 {
   UINT16      ConfigValue;
-  UINT8       Protocol;
   EFI_STATUS  Status;
   UINT32      TransferResult;
 
@@ -854,22 +853,15 @@ InitUSBKeyboard (
     }
   }
 
-  UsbGetProtocolRequest (
-    UsbKeyboardDevice->UsbIo,
-    UsbKeyboardDevice->InterfaceDescriptor.InterfaceNumber,
-    &Protocol
-    );
   //
   // Set boot protocol for the USB Keyboard.
   // This driver only supports boot protocol.
   //
-  if (Protocol != BOOT_PROTOCOL) {
-    UsbSetProtocolRequest (
-      UsbKeyboardDevice->UsbIo,
-      UsbKeyboardDevice->InterfaceDescriptor.InterfaceNumber,
-      BOOT_PROTOCOL
-      );
-  }
+  UsbSetProtocolRequest (
+    UsbKeyboardDevice->UsbIo,
+    UsbKeyboardDevice->InterfaceDescriptor.InterfaceNumber,
+    BOOT_PROTOCOL
+    );
 
   UsbKeyboardDevice->CtrlOn    = FALSE;
   UsbKeyboardDevice->AltOn     = FALSE;
-- 
2.32.0



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#86964): https://edk2.groups.io/g/devel/message/86964
Mute This Topic: https://groups.io/mt/89228276/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] MdeModulePkg/Usb/Keyboard.c: Don't request protocol before setting
Posted by Wu, Hao A 2 years, 2 months ago 
Reviewed-by: Hao A Wu <hao.a.wu@intel.com>

Best Regards,
Hao Wu

> -----Original Message-----
> From: Sean Rhodes <sean@starlabs.systems>
> Sent: Thursday, February 24, 2022 7:38 PM
> To: devel@edk2.groups.io
> Cc: Dong, Guo <guo.dong@intel.com>; Rhodes, Sean
> <sean@starlabs.systems>; Wu, Hao A <hao.a.wu@intel.com>; Ni, Ray
> <ray.ni@intel.com>; Matt DeVillier <matt.devillier@gmail.com>; Patrick
> Rudolph <patrick.rudolph@9elements.com>
> Subject: [PATCH] MdeModulePkg/Usb/Keyboard.c: Don't request protocol
> before setting
> 
> No need to check the interface protocol then conditionally setting,
> just set it to BOOT_PROTOCOL and check for error.
> 
> This is what Linux does for HID devices as some don't follow the USB spec.
> One example is the Aspeed BMC HID keyboard device, which adds a massive
> boot delay without this patch as it doesn't respond to 'GetProtocolRequest'.
> 
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Signed-off-by: Matt DeVillier <matt.devillier@gmail.com>
> Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
> Signed-off-by: Sean Rhodes <sean@starlabs.systems>
> ---
>  MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c | 18 +++++-------------
>  1 file changed, 5 insertions(+), 13 deletions(-)
> 
> diff --git a/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
> b/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
> index 5a94a4dda7..b5a6459a35 100644
> --- a/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
> +++ b/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
> @@ -805,7 +805,6 @@ InitUSBKeyboard (
>    )
> 
>  {
> 
>    UINT16      ConfigValue;
> 
> -  UINT8       Protocol;
> 
>    EFI_STATUS  Status;
> 
>    UINT32      TransferResult;
> 
> 
> 
> @@ -854,22 +853,15 @@ InitUSBKeyboard (
>      }
> 
>    }
> 
> 
> 
> -  UsbGetProtocolRequest (
> 
> -    UsbKeyboardDevice->UsbIo,
> 
> -    UsbKeyboardDevice->InterfaceDescriptor.InterfaceNumber,
> 
> -    &Protocol
> 
> -    );
> 
>    //
> 
>    // Set boot protocol for the USB Keyboard.
> 
>    // This driver only supports boot protocol.
> 
>    //
> 
> -  if (Protocol != BOOT_PROTOCOL) {
> 
> -    UsbSetProtocolRequest (
> 
> -      UsbKeyboardDevice->UsbIo,
> 
> -      UsbKeyboardDevice->InterfaceDescriptor.InterfaceNumber,
> 
> -      BOOT_PROTOCOL
> 
> -      );
> 
> -  }
> 
> +  UsbSetProtocolRequest (
> 
> +    UsbKeyboardDevice->UsbIo,
> 
> +    UsbKeyboardDevice->InterfaceDescriptor.InterfaceNumber,
> 
> +    BOOT_PROTOCOL
> 
> +    );
> 
> 
> 
>    UsbKeyboardDevice->CtrlOn    = FALSE;
> 
>    UsbKeyboardDevice->AltOn     = FALSE;
> 
> --
> 2.32.0



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#87184): https://edk2.groups.io/g/devel/message/87184
Mute This Topic: https://groups.io/mt/89228276/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] MdeModulePkg/Usb/Keyboard.c: Don't request protocol before setting
Posted by Wu, Hao A 2 years, 2 months ago 
Pushed via:
PR - https://github.com/tianocore/edk2/pull/2574
Commit - https://github.com/tianocore/edk2/commit/589d51df260465e2561979b8a988e77b0f32a6e8

Best Regards,
Hao Wu

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Wu, Hao
> A
> Sent: Wednesday, March 2, 2022 10:10 AM
> To: Rhodes, Sean <sean@starlabs.systems>; devel@edk2.groups.io
> Cc: Dong, Guo <guo.dong@intel.com>; Rhodes, Sean
> <sean@starlabs.systems>; Ni, Ray <ray.ni@intel.com>; Matt DeVillier
> <matt.devillier@gmail.com>; Patrick Rudolph
> <patrick.rudolph@9elements.com>
> Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/Usb/Keyboard.c: Don't
> request protocol before setting
> 
> Reviewed-by: Hao A Wu <hao.a.wu@intel.com>
> 
> Best Regards,
> Hao Wu
> 
> > -----Original Message-----
> > From: Sean Rhodes <sean@starlabs.systems>
> > Sent: Thursday, February 24, 2022 7:38 PM
> > To: devel@edk2.groups.io
> > Cc: Dong, Guo <guo.dong@intel.com>; Rhodes, Sean
> > <sean@starlabs.systems>; Wu, Hao A <hao.a.wu@intel.com>; Ni, Ray
> > <ray.ni@intel.com>; Matt DeVillier <matt.devillier@gmail.com>; Patrick
> > Rudolph <patrick.rudolph@9elements.com>
> > Subject: [PATCH] MdeModulePkg/Usb/Keyboard.c: Don't request protocol
> > before setting
> >
> > No need to check the interface protocol then conditionally setting,
> > just set it to BOOT_PROTOCOL and check for error.
> >
> > This is what Linux does for HID devices as some don't follow the USB spec.
> > One example is the Aspeed BMC HID keyboard device, which adds a
> > massive boot delay without this patch as it doesn't respond to
> 'GetProtocolRequest'.
> >
> > Cc: Hao A Wu <hao.a.wu@intel.com>
> > Cc: Ray Ni <ray.ni@intel.com>
> > Signed-off-by: Matt DeVillier <matt.devillier@gmail.com>
> > Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
> > Signed-off-by: Sean Rhodes <sean@starlabs.systems>
> > ---
> >  MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c | 18 +++++-------------
> >  1 file changed, 5 insertions(+), 13 deletions(-)
> >
> > diff --git a/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
> > b/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
> > index 5a94a4dda7..b5a6459a35 100644
> > --- a/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
> > +++ b/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
> > @@ -805,7 +805,6 @@ InitUSBKeyboard (
> >    )
> >
> >  {
> >
> >    UINT16      ConfigValue;
> >
> > -  UINT8       Protocol;
> >
> >    EFI_STATUS  Status;
> >
> >    UINT32      TransferResult;
> >
> >
> >
> > @@ -854,22 +853,15 @@ InitUSBKeyboard (
> >      }
> >
> >    }
> >
> >
> >
> > -  UsbGetProtocolRequest (
> >
> > -    UsbKeyboardDevice->UsbIo,
> >
> > -    UsbKeyboardDevice->InterfaceDescriptor.InterfaceNumber,
> >
> > -    &Protocol
> >
> > -    );
> >
> >    //
> >
> >    // Set boot protocol for the USB Keyboard.
> >
> >    // This driver only supports boot protocol.
> >
> >    //
> >
> > -  if (Protocol != BOOT_PROTOCOL) {
> >
> > -    UsbSetProtocolRequest (
> >
> > -      UsbKeyboardDevice->UsbIo,
> >
> > -      UsbKeyboardDevice->InterfaceDescriptor.InterfaceNumber,
> >
> > -      BOOT_PROTOCOL
> >
> > -      );
> >
> > -  }
> >
> > +  UsbSetProtocolRequest (
> >
> > +    UsbKeyboardDevice->UsbIo,
> >
> > +    UsbKeyboardDevice->InterfaceDescriptor.InterfaceNumber,
> >
> > +    BOOT_PROTOCOL
> >
> > +    );
> >
> >
> >
> >    UsbKeyboardDevice->CtrlOn    = FALSE;
> >
> >    UsbKeyboardDevice->AltOn     = FALSE;
> >
> > --
> > 2.32.0
> 
> 
> 
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#87250): https://edk2.groups.io/g/devel/message/87250
Mute This Topic: https://groups.io/mt/89228276/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.