Hello, Inline comments below:
> -----Original Message----- > From: Sean Rhodes <[email protected]> > Sent: Friday, February 11, 2022 5:27 AM > To: [email protected] > Cc: Wu, Hao A <[email protected]>; Matt DeVillier > <[email protected]>; Patrick Rudolph > <[email protected]> > Subject: [PATCH 13/18] MdeModulePkg/Usb/Keyboard.c: don't request > protocol before setting > > From: Matt DeVillier <[email protected]> > > 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 <[email protected]> > Signed-off-by: Patrick Rudolph <[email protected]> > --- > 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 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#86605): https://edk2.groups.io/g/devel/message/86605 Mute This Topic: https://groups.io/mt/89056141/21656 Group Owner: [email protected] Unsubscribe: https://edk2.groups.io/g/devel/unsub [[email protected]] -=-=-=-=-=-=-=-=-=-=-=-
