* [PATCH 1/2] MdeModulePkg/Usb/Keyboard.c: remove Get/SetConfig calls
@ 2021-01-20 15:58 Patrick Rudolph
2021-01-20 15:59 ` [PATCH 2/2] MdeModulePkg/Usb/Keyboard.c: don't request protocol before setting Patrick Rudolph
2021-01-22 1:17 ` [edk2-devel] [PATCH 1/2] MdeModulePkg/Usb/Keyboard.c: remove Get/SetConfig calls Wu, Hao A
0 siblings, 2 replies; 6+ messages in thread
From: Patrick Rudolph @ 2021-01-20 15:58 UTC (permalink / raw)
To: devel; +Cc: jian.j.wang, hao.a.wu, ray.ni
From: Matt DeVillier <matt.devillier@gmail.com>
SetConfig is already called during device enumeration,
no need to do it again here.
Signed-off-by: Matt DeVillier <matt.devillier@gmail.com>
---
MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c | 37 --------------------
1 file changed, 37 deletions(-)
diff --git a/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c b/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
index 5faf82ea57..77e20b203f 100644
--- a/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
+++ b/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
@@ -801,10 +801,8 @@ InitUSBKeyboard (
IN OUT USB_KB_DEV *UsbKeyboardDevice
)
{
- UINT16 ConfigValue;
UINT8 Protocol;
EFI_STATUS Status;
- UINT32 TransferResult;
REPORT_STATUS_CODE_WITH_DEVICE_PATH (
EFI_PROGRESS_CODE,
@@ -816,41 +814,6 @@ InitUSBKeyboard (
InitQueue (&UsbKeyboardDevice->EfiKeyQueue, sizeof (EFI_KEY_DATA));
InitQueue (&UsbKeyboardDevice->EfiKeyQueueForNotify, sizeof (EFI_KEY_DATA));
- //
- // Use the config out of the descriptor
- // Assumed the first config is the correct one and this is not always the case
- //
- Status = UsbGetConfiguration (
- UsbKeyboardDevice->UsbIo,
- &ConfigValue,
- &TransferResult
- );
- if (EFI_ERROR (Status)) {
- ConfigValue = 0x01;
- //
- // Uses default configuration to configure the USB Keyboard device.
- //
- Status = UsbSetConfiguration (
- UsbKeyboardDevice->UsbIo,
- ConfigValue,
- &TransferResult
- );
- if (EFI_ERROR (Status)) {
- //
- // If configuration could not be set here, it means
- // the keyboard interface has some errors and could
- // not be initialized
- //
- REPORT_STATUS_CODE_WITH_DEVICE_PATH (
- EFI_ERROR_CODE | EFI_ERROR_MINOR,
- (EFI_PERIPHERAL_KEYBOARD | EFI_P_EC_INTERFACE_ERROR),
- UsbKeyboardDevice->DevicePath
- );
-
- return EFI_DEVICE_ERROR;
- }
- }
-
UsbGetProtocolRequest (
UsbKeyboardDevice->UsbIo,
UsbKeyboardDevice->InterfaceDescriptor.InterfaceNumber,
--
2.26.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] MdeModulePkg/Usb/Keyboard.c: don't request protocol before setting
2021-01-20 15:58 [PATCH 1/2] MdeModulePkg/Usb/Keyboard.c: remove Get/SetConfig calls Patrick Rudolph
@ 2021-01-20 15:59 ` Patrick Rudolph
2021-01-28 2:14 ` Wu, Hao A
2021-01-22 1:17 ` [edk2-devel] [PATCH 1/2] MdeModulePkg/Usb/Keyboard.c: remove Get/SetConfig calls Wu, Hao A
1 sibling, 1 reply; 6+ messages in thread
From: Patrick Rudolph @ 2021-01-20 15:59 UTC (permalink / raw)
To: devel; +Cc: jian.j.wang, hao.a.wu, ray.ni
From: Matt DeVillier <matt.devillier@gmail.com>
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'.
Signed-off-by: Matt DeVillier <matt.devillier@gmail.com>
Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
---
MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c | 28 ++++++++++++--------
1 file changed, 17 insertions(+), 11 deletions(-)
diff --git a/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c b/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
index 77e20b203f..5914174b4d 100644
--- a/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
+++ b/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
@@ -801,7 +801,6 @@ InitUSBKeyboard (
IN OUT USB_KB_DEV *UsbKeyboardDevice
)
{
- UINT8 Protocol;
EFI_STATUS Status;
REPORT_STATUS_CODE_WITH_DEVICE_PATH (
@@ -814,21 +813,28 @@ InitUSBKeyboard (
InitQueue (&UsbKeyboardDevice->EfiKeyQueue, sizeof (EFI_KEY_DATA));
InitQueue (&UsbKeyboardDevice->EfiKeyQueueForNotify, sizeof (EFI_KEY_DATA));
- 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
+ Status = UsbSetProtocolRequest (
+ UsbKeyboardDevice->UsbIo,
+ UsbKeyboardDevice->InterfaceDescriptor.InterfaceNumber,
+ BOOT_PROTOCOL
+ );
+ if (EFI_ERROR (Status)) {
+ //
+ // If protocol could not be set here, it means
+ // the keyboard interface has some errors and could
+ // not be initialized
+ //
+ REPORT_STATUS_CODE_WITH_DEVICE_PATH (
+ EFI_ERROR_CODE | EFI_ERROR_MINOR,
+ (EFI_PERIPHERAL_KEYBOARD | EFI_P_EC_INTERFACE_ERROR),
+ UsbKeyboardDevice->DevicePath
);
+
+ return EFI_DEVICE_ERROR;
}
UsbKeyboardDevice->CtrlOn = FALSE;
--
2.26.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [edk2-devel] [PATCH 1/2] MdeModulePkg/Usb/Keyboard.c: remove Get/SetConfig calls
2021-01-20 15:58 [PATCH 1/2] MdeModulePkg/Usb/Keyboard.c: remove Get/SetConfig calls Patrick Rudolph
2021-01-20 15:59 ` [PATCH 2/2] MdeModulePkg/Usb/Keyboard.c: don't request protocol before setting Patrick Rudolph
@ 2021-01-22 1:17 ` Wu, Hao A
2021-01-28 2:14 ` Wu, Hao A
1 sibling, 1 reply; 6+ messages in thread
From: Wu, Hao A @ 2021-01-22 1:17 UTC (permalink / raw)
To: devel@edk2.groups.io, patrick.rudolph@9elements.com; +Cc: Wang, Jian J, Ni, Ray
Sorry, please grant me some time for this patch series. I will try to provide feedbacks before end of next week.
Meanwhile, could you help to provide the information on what kind of tests have been performed for these 2 patches? Thanks in advance.
Best Regards,
Hao Wu
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Patrick
> Rudolph
> Sent: Wednesday, January 20, 2021 11:59 PM
> To: devel@edk2.groups.io
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>;
> Ni, Ray <ray.ni@intel.com>
> Subject: [edk2-devel] [PATCH 1/2] MdeModulePkg/Usb/Keyboard.c:
> remove Get/SetConfig calls
>
> From: Matt DeVillier <matt.devillier@gmail.com>
>
> SetConfig is already called during device enumeration,
> no need to do it again here.
>
> Signed-off-by: Matt DeVillier <matt.devillier@gmail.com>
> ---
> MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c | 37 --------------------
> 1 file changed, 37 deletions(-)
>
> diff --git a/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
> b/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
> index 5faf82ea57..77e20b203f 100644
> --- a/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
> +++ b/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
> @@ -801,10 +801,8 @@ InitUSBKeyboard (
> IN OUT USB_KB_DEV *UsbKeyboardDevice
>
> )
>
> {
>
> - UINT16 ConfigValue;
>
> UINT8 Protocol;
>
> EFI_STATUS Status;
>
> - UINT32 TransferResult;
>
>
>
> REPORT_STATUS_CODE_WITH_DEVICE_PATH (
>
> EFI_PROGRESS_CODE,
>
> @@ -816,41 +814,6 @@ InitUSBKeyboard (
> InitQueue (&UsbKeyboardDevice->EfiKeyQueue, sizeof (EFI_KEY_DATA));
>
> InitQueue (&UsbKeyboardDevice->EfiKeyQueueForNotify, sizeof
> (EFI_KEY_DATA));
>
>
>
> - //
>
> - // Use the config out of the descriptor
>
> - // Assumed the first config is the correct one and this is not always the case
>
> - //
>
> - Status = UsbGetConfiguration (
>
> - UsbKeyboardDevice->UsbIo,
>
> - &ConfigValue,
>
> - &TransferResult
>
> - );
>
> - if (EFI_ERROR (Status)) {
>
> - ConfigValue = 0x01;
>
> - //
>
> - // Uses default configuration to configure the USB Keyboard device.
>
> - //
>
> - Status = UsbSetConfiguration (
>
> - UsbKeyboardDevice->UsbIo,
>
> - ConfigValue,
>
> - &TransferResult
>
> - );
>
> - if (EFI_ERROR (Status)) {
>
> - //
>
> - // If configuration could not be set here, it means
>
> - // the keyboard interface has some errors and could
>
> - // not be initialized
>
> - //
>
> - REPORT_STATUS_CODE_WITH_DEVICE_PATH (
>
> - EFI_ERROR_CODE | EFI_ERROR_MINOR,
>
> - (EFI_PERIPHERAL_KEYBOARD | EFI_P_EC_INTERFACE_ERROR),
>
> - UsbKeyboardDevice->DevicePath
>
> - );
>
> -
>
> - return EFI_DEVICE_ERROR;
>
> - }
>
> - }
>
> -
>
> UsbGetProtocolRequest (
>
> UsbKeyboardDevice->UsbIo,
>
> UsbKeyboardDevice->InterfaceDescriptor.InterfaceNumber,
>
> --
> 2.26.2
>
>
>
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#70585): https://edk2.groups.io/g/devel/message/70585
> Mute This Topic: https://groups.io/mt/79981643/1768737
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [hao.a.wu@intel.com]
> -=-=-=-=-=-=
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [edk2-devel] [PATCH 1/2] MdeModulePkg/Usb/Keyboard.c: remove Get/SetConfig calls
2021-01-22 1:17 ` [edk2-devel] [PATCH 1/2] MdeModulePkg/Usb/Keyboard.c: remove Get/SetConfig calls Wu, Hao A
@ 2021-01-28 2:14 ` Wu, Hao A
0 siblings, 0 replies; 6+ messages in thread
From: Wu, Hao A @ 2021-01-28 2:14 UTC (permalink / raw)
To: devel@edk2.groups.io, Wu, Hao A, patrick.rudolph@9elements.com,
Matt DeVillier
Cc: Wang, Jian J, Ni, Ray
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Wu, Hao
> A
> Sent: Friday, January 22, 2021 9:17 AM
> To: devel@edk2.groups.io; patrick.rudolph@9elements.com
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Ni, Ray <ray.ni@intel.com>
> Subject: Re: [edk2-devel] [PATCH 1/2] MdeModulePkg/Usb/Keyboard.c:
> remove Get/SetConfig calls
>
> Sorry, please grant me some time for this patch series. I will try to provide
> feedbacks before end of next week.
> Meanwhile, could you help to provide the information on what kind of tests
> have been performed for these 2 patches? Thanks in advance.
>
> Best Regards,
> Hao Wu
>
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Patrick
> > Rudolph
> > Sent: Wednesday, January 20, 2021 11:59 PM
> > To: devel@edk2.groups.io
> > Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A
> > <hao.a.wu@intel.com>; Ni, Ray <ray.ni@intel.com>
> > Subject: [edk2-devel] [PATCH 1/2] MdeModulePkg/Usb/Keyboard.c:
> > remove Get/SetConfig calls
> >
> > From: Matt DeVillier <matt.devillier@gmail.com>
> >
> > SetConfig is already called during device enumeration, no need to do
> > it again here.
I found that the InitUSBKeyboard() function, which includes this Get/SetConfig
calls, is triggered during the keyboard reset flow. Even though the
configuration for the keyboard will be set during enumeration, I think it would
be better to keep this reconfiguration in the reset flow.
Best Regards,
Hao Wu
> >
> > Signed-off-by: Matt DeVillier <matt.devillier@gmail.com>
> > ---
> > MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c | 37 --------------------
> > 1 file changed, 37 deletions(-)
> >
> > diff --git a/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
> > b/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
> > index 5faf82ea57..77e20b203f 100644
> > --- a/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
> > +++ b/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
> > @@ -801,10 +801,8 @@ InitUSBKeyboard (
> > IN OUT USB_KB_DEV *UsbKeyboardDevice
> >
> > )
> >
> > {
> >
> > - UINT16 ConfigValue;
> >
> > UINT8 Protocol;
> >
> > EFI_STATUS Status;
> >
> > - UINT32 TransferResult;
> >
> >
> >
> > REPORT_STATUS_CODE_WITH_DEVICE_PATH (
> >
> > EFI_PROGRESS_CODE,
> >
> > @@ -816,41 +814,6 @@ InitUSBKeyboard (
> > InitQueue (&UsbKeyboardDevice->EfiKeyQueue, sizeof (EFI_KEY_DATA));
> >
> > InitQueue (&UsbKeyboardDevice->EfiKeyQueueForNotify, sizeof
> > (EFI_KEY_DATA));
> >
> >
> >
> > - //
> >
> > - // Use the config out of the descriptor
> >
> > - // Assumed the first config is the correct one and this is not
> > always the case
> >
> > - //
> >
> > - Status = UsbGetConfiguration (
> >
> > - UsbKeyboardDevice->UsbIo,
> >
> > - &ConfigValue,
> >
> > - &TransferResult
> >
> > - );
> >
> > - if (EFI_ERROR (Status)) {
> >
> > - ConfigValue = 0x01;
> >
> > - //
> >
> > - // Uses default configuration to configure the USB Keyboard device.
> >
> > - //
> >
> > - Status = UsbSetConfiguration (
> >
> > - UsbKeyboardDevice->UsbIo,
> >
> > - ConfigValue,
> >
> > - &TransferResult
> >
> > - );
> >
> > - if (EFI_ERROR (Status)) {
> >
> > - //
> >
> > - // If configuration could not be set here, it means
> >
> > - // the keyboard interface has some errors and could
> >
> > - // not be initialized
> >
> > - //
> >
> > - REPORT_STATUS_CODE_WITH_DEVICE_PATH (
> >
> > - EFI_ERROR_CODE | EFI_ERROR_MINOR,
> >
> > - (EFI_PERIPHERAL_KEYBOARD | EFI_P_EC_INTERFACE_ERROR),
> >
> > - UsbKeyboardDevice->DevicePath
> >
> > - );
> >
> > -
> >
> > - return EFI_DEVICE_ERROR;
> >
> > - }
> >
> > - }
> >
> > -
> >
> > UsbGetProtocolRequest (
> >
> > UsbKeyboardDevice->UsbIo,
> >
> > UsbKeyboardDevice->InterfaceDescriptor.InterfaceNumber,
> >
> > --
> > 2.26.2
> >
> >
> >
> > -=-=-=-=-=-=
> > Groups.io Links: You receive all messages sent to this group.
> > View/Reply Online (#70585):
> > https://edk2.groups.io/g/devel/message/70585
> > Mute This Topic: https://groups.io/mt/79981643/1768737
> > Group Owner: devel+owner@edk2.groups.io
> > Unsubscribe: https://edk2.groups.io/g/devel/unsub [hao.a.wu@intel.com]
> > -=-=-=-=-=-=
> >
>
>
>
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] MdeModulePkg/Usb/Keyboard.c: don't request protocol before setting
2021-01-20 15:59 ` [PATCH 2/2] MdeModulePkg/Usb/Keyboard.c: don't request protocol before setting Patrick Rudolph
@ 2021-01-28 2:14 ` Wu, Hao A
2021-01-28 7:31 ` Patrick Rudolph
0 siblings, 1 reply; 6+ messages in thread
From: Wu, Hao A @ 2021-01-28 2:14 UTC (permalink / raw)
To: Patrick Rudolph, devel@edk2.groups.io; +Cc: Wang, Jian J, Ni, Ray
> -----Original Message-----
> From: Patrick Rudolph <patrick.rudolph@9elements.com>
> Sent: Wednesday, January 20, 2021 11:59 PM
> To: devel@edk2.groups.io
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>;
> Ni, Ray <ray.ni@intel.com>
> Subject: [PATCH 2/2] MdeModulePkg/Usb/Keyboard.c: don't request
> protocol before setting
>
> From: Matt DeVillier <matt.devillier@gmail.com>
>
> 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'.
I am okay with the code change.
But since the current logic is here for a long time, I am not sure whether
other USB keyboard device will have functional/performance impact for this
change.
Besides the above mentioned Aspeed BMC HID keyboard device (which has a
performance issue for the Get_Protocol request that the patch is going to
drop), could you at least try one USB keyboard device that works fine with
the origin code logic to see if there is any side effect for this patch?
Thanks in advance.
Best Regards,
Hao Wu
>
> Signed-off-by: Matt DeVillier <matt.devillier@gmail.com>
> Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
> ---
> MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c | 28 ++++++++++++-------
> -
> 1 file changed, 17 insertions(+), 11 deletions(-)
>
> diff --git a/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
> b/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
> index 77e20b203f..5914174b4d 100644
> --- a/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
> +++ b/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
> @@ -801,7 +801,6 @@ InitUSBKeyboard (
> IN OUT USB_KB_DEV *UsbKeyboardDevice ) {- UINT8 Protocol;
> EFI_STATUS Status; REPORT_STATUS_CODE_WITH_DEVICE_PATH (@@
> -814,21 +813,28 @@ InitUSBKeyboard (
> InitQueue (&UsbKeyboardDevice->EfiKeyQueue, sizeof (EFI_KEY_DATA));
> InitQueue (&UsbKeyboardDevice->EfiKeyQueueForNotify, sizeof
> (EFI_KEY_DATA)); - 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+ Status = UsbSetProtocolRequest (+
> UsbKeyboardDevice->UsbIo,+ UsbKeyboardDevice-
> >InterfaceDescriptor.InterfaceNumber,+ BOOT_PROTOCOL+ );+
> if (EFI_ERROR (Status)) {+ //+ // If protocol could not be set here, it
> means+ // the keyboard interface has some errors and could+ // not be
> initialized+ //+ REPORT_STATUS_CODE_WITH_DEVICE_PATH (+
> EFI_ERROR_CODE | EFI_ERROR_MINOR,+ (EFI_PERIPHERAL_KEYBOARD |
> EFI_P_EC_INTERFACE_ERROR),+ UsbKeyboardDevice->DevicePath );++
> return EFI_DEVICE_ERROR; } UsbKeyboardDevice->CtrlOn = FALSE;--
> 2.26.2
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] MdeModulePkg/Usb/Keyboard.c: don't request protocol before setting
2021-01-28 2:14 ` Wu, Hao A
@ 2021-01-28 7:31 ` Patrick Rudolph
0 siblings, 0 replies; 6+ messages in thread
From: Patrick Rudolph @ 2021-01-28 7:31 UTC (permalink / raw)
To: Wu, Hao A; +Cc: devel@edk2.groups.io, Wang, Jian J, Ni, Ray
Hello,
Until now no regressions have been observed.
I'll provide a full test description using a wide variety of platforms soon.
Kind Regards,
Patrick Rudolph
On Thu, Jan 28, 2021 at 3:14 AM Wu, Hao A <hao.a.wu@intel.com> wrote:
>
> > -----Original Message-----
> > From: Patrick Rudolph <patrick.rudolph@9elements.com>
> > Sent: Wednesday, January 20, 2021 11:59 PM
> > To: devel@edk2.groups.io
> > Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>;
> > Ni, Ray <ray.ni@intel.com>
> > Subject: [PATCH 2/2] MdeModulePkg/Usb/Keyboard.c: don't request
> > protocol before setting
> >
> > From: Matt DeVillier <matt.devillier@gmail.com>
> >
> > 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'.
>
>
> I am okay with the code change.
>
> But since the current logic is here for a long time, I am not sure whether
> other USB keyboard device will have functional/performance impact for this
> change.
>
> Besides the above mentioned Aspeed BMC HID keyboard device (which has a
> performance issue for the Get_Protocol request that the patch is going to
> drop), could you at least try one USB keyboard device that works fine with
> the origin code logic to see if there is any side effect for this patch?
>
> Thanks in advance.
>
> Best Regards,
> Hao Wu
>
>
> >
> > Signed-off-by: Matt DeVillier <matt.devillier@gmail.com>
> > Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
> > ---
> > MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c | 28 ++++++++++++-------
> > -
> > 1 file changed, 17 insertions(+), 11 deletions(-)
> >
> > diff --git a/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
> > b/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
> > index 77e20b203f..5914174b4d 100644
> > --- a/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
> > +++ b/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
> > @@ -801,7 +801,6 @@ InitUSBKeyboard (
> > IN OUT USB_KB_DEV *UsbKeyboardDevice ) {- UINT8 Protocol;
> > EFI_STATUS Status; REPORT_STATUS_CODE_WITH_DEVICE_PATH (@@
> > -814,21 +813,28 @@ InitUSBKeyboard (
> > InitQueue (&UsbKeyboardDevice->EfiKeyQueue, sizeof (EFI_KEY_DATA));
> > InitQueue (&UsbKeyboardDevice->EfiKeyQueueForNotify, sizeof
> > (EFI_KEY_DATA)); - 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+ Status = UsbSetProtocolRequest (+
> > UsbKeyboardDevice->UsbIo,+ UsbKeyboardDevice-
> > >InterfaceDescriptor.InterfaceNumber,+ BOOT_PROTOCOL+ );+
> > if (EFI_ERROR (Status)) {+ //+ // If protocol could not be set here, it
> > means+ // the keyboard interface has some errors and could+ // not be
> > initialized+ //+ REPORT_STATUS_CODE_WITH_DEVICE_PATH (+
> > EFI_ERROR_CODE | EFI_ERROR_MINOR,+ (EFI_PERIPHERAL_KEYBOARD |
> > EFI_P_EC_INTERFACE_ERROR),+ UsbKeyboardDevice->DevicePath );++
> > return EFI_DEVICE_ERROR; } UsbKeyboardDevice->CtrlOn = FALSE;--
> > 2.26.2
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-01-28 7:31 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-01-20 15:58 [PATCH 1/2] MdeModulePkg/Usb/Keyboard.c: remove Get/SetConfig calls Patrick Rudolph
2021-01-20 15:59 ` [PATCH 2/2] MdeModulePkg/Usb/Keyboard.c: don't request protocol before setting Patrick Rudolph
2021-01-28 2:14 ` Wu, Hao A
2021-01-28 7:31 ` Patrick Rudolph
2021-01-22 1:17 ` [edk2-devel] [PATCH 1/2] MdeModulePkg/Usb/Keyboard.c: remove Get/SetConfig calls Wu, Hao A
2021-01-28 2:14 ` Wu, Hao A
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox