From: "Wu, Hao A" <hao.a.wu@intel.com>
To: "Rhodes, Sean" <sean@starlabs.systems>,
Matt DeVillier <matt.devillier@gmail.com>,
Patrick Rudolph <patrick.rudolph@9elements.com>,
"devel@edk2.groups.io" <devel@edk2.groups.io>
Cc: "Wang, Jian J" <jian.j.wang@intel.com>,
"Gao, Liming" <gaoliming@byosoft.com.cn>,
"Ni, Ray" <ray.ni@intel.com>
Subject: Re: [PATCH 13/18] MdeModulePkg/Usb/Keyboard.c: don't request protocol before setting
Date: Fri, 11 Feb 2022 03:32:44 +0000 [thread overview]
Message-ID: <DM6PR11MB4025950A432A906A6D1A6B64CA309@DM6PR11MB4025.namprd11.prod.outlook.com> (raw)
In-Reply-To: <5527aab4cdea820aa58de36fd8d58ef88f24da82.1644527848.git.sean@starlabs.systems>
Hello,
Inline comments below:
> -----Original Message-----
> From: Sean Rhodes <sean@starlabs.systems>
> Sent: Friday, February 11, 2022 5:27 AM
> To: devel@edk2.groups.io
> Cc: Wu, Hao A <hao.a.wu@intel.com>; Matt DeVillier
> <matt.devillier@gmail.com>; Patrick Rudolph
> <patrick.rudolph@9elements.com>
> Subject: [PATCH 13/18] 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'.
>
> Signed-off-by: Matt DeVillier <matt.devillier@gmail.com>
> Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
> ---
> MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c | 22
> +++++++++++++++++-----
> 1 file changed, 17 insertions(+), 5 deletions(-)
>
> diff --git a/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
> b/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
> index 5a94a4dda7..56d249f937 100644
> --- a/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
> +++ b/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
> @@ -863,12 +863,24 @@ InitUSBKeyboard (
> // 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;
>
> }
Sorry for a question, I do not understand why the proposed change will eliminate the boot delay.
For device that does not respond to GetProtocolRequest(), the origin flow is like:
a.1 Timeout occurs for GetProtocolRequest()
a.2 Conditionally execute UsbSetProtocolRequest()
The proposed new flow is like:
b.1 Timeout occurs for GetProtocolRequest()
b.2 Always execute UsbSetProtocolRequest()
Could you help to elaborate on the change? Thanks in advance.
Best Regards,
Hao Wu
>
>
>
> UsbKeyboardDevice->CtrlOn = FALSE;
>
> --
> 2.32.0
next prev parent reply other threads:[~2022-02-11 3:32 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <df21ef7edddfef8074ba94514bf73d7a5d0d6046.1644527848.git.sean@starlabs.systems>
2022-02-10 21:20 ` [PATCH 02/18] UefiPayloadPkg: Provide option to use Boot Splash Sean Rhodes
2022-02-10 21:23 ` [PATCH 03/18] MdeModulePackage: Add option to follow BGRT spec Sean Rhodes
2022-02-10 21:23 ` [PATCH 04/18] UefiPayloadPkg: Hookup BGRT build option Sean Rhodes
2022-02-10 21:23 ` [PATCH 05/18] UefiPayloadPkg: Make Boot Manager Key configurable Sean Rhodes
2022-02-10 21:24 ` [PATCH 06/18] UefiPayloadPkg: Add build option for Above 4G Memory Sean Rhodes
2022-02-10 21:24 ` [PATCH 08/18] UefiPayloadPkg: Fix case of protocol Sean Rhodes
2022-02-20 18:11 ` [edk2-devel] " Patrick Rudolph
2022-02-10 21:25 ` [PATCH 09/18] SdMmcPciDxe: Reduce timeout for SD card reset Sean Rhodes
2022-02-11 3:15 ` Wu, Hao A
2022-02-10 21:25 ` [PATCH 10/18] BlSupportSmm: fix definition of SetSmrr() Sean Rhodes
2022-02-20 18:09 ` [edk2-devel] " Patrick Rudolph
2022-02-10 21:25 ` [PATCH 11/18] UefiPayloadPkg: Make Boot Timeout configurable Sean Rhodes
2022-02-10 21:26 ` [PATCH 12/18] Ps2KbdCtrller: Make wait for SUCCESS after BAT non-fatal Sean Rhodes
2022-02-11 3:12 ` Wu, Hao A
2022-02-10 21:27 ` [PATCH 13/18] MdeModulePkg/Usb/Keyboard.c: don't request protocol before setting Sean Rhodes
2022-02-11 3:32 ` Wu, Hao A [this message]
2022-02-11 22:45 ` MrChromebox
2022-02-18 2:17 ` [edk2-devel] " Wu, Hao A
2022-02-10 21:27 ` [PATCH 14/18] UefiPayloadPkg: Add RNG support Sean Rhodes
2022-02-16 23:57 ` Guo Dong
2022-02-10 21:27 ` [PATCH 15/18] UefiPayloadPkg/Library/PlatformBootManagerLib: Remove broken VGA detection Sean Rhodes
2022-02-16 23:51 ` Guo Dong
2022-03-21 9:48 ` [edk2-devel] " Sean Rhodes
2022-03-29 17:32 ` Guo Dong
2022-03-29 17:34 ` Sean Rhodes
2022-03-29 18:01 ` Guo Dong
2022-03-29 18:04 ` Sean Rhodes
2022-03-29 18:22 ` Guo Dong
2022-02-10 21:28 ` [PATCH 16/18] UefiCpuPkg: Disable MTRR programming for UefiPayloadPkg Sean Rhodes
2022-02-10 22:06 ` [edk2-devel] " Michael D Kinney
2022-02-11 7:12 ` Patrick Rudolph
2022-02-10 21:29 ` [PATCH 17/18] BmBoot: prioritize internal boot devices over external Sean Rhodes
2022-02-10 21:29 ` [PATCH 18/18] UefiPayloadPkg: Hookup Prioritise Internal build option Sean Rhodes
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=DM6PR11MB4025950A432A906A6D1A6B64CA309@DM6PR11MB4025.namprd11.prod.outlook.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox