MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c | 90 +++++++++++++----------- 1 file changed, 47 insertions(+), 43 deletions(-)
---
MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c | 90 +++++++++++++-----------
1 file changed, 47 insertions(+), 43 deletions(-)
diff --git a/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c b/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
index 7505951c82..37c9388161 100644
--- a/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
+++ b/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
@@ -792,6 +792,19 @@ InitKeyboardLayout (
return EFI_SUCCESS;
}
+static
+BOOLEAN
+UsbQuirkRequireSetIdle (
+ IN EFI_USB_DEVICE_DESCRIPTOR DevDesc
+ )
+{
+ switch (DevDesc.IdVendor) {
+ case 0x0557:
+ return DevDesc.IdProduct == 0x2419;
+ default:
+ return FALSE;
+ }
+}
/**
Initialize USB keyboard device and all private data structures.
@@ -807,10 +820,8 @@ InitUSBKeyboard (
IN OUT USB_KB_DEV *UsbKeyboardDevice
)
{
- UINT16 ConfigValue;
- UINT8 Protocol;
- EFI_STATUS Status;
- UINT32 TransferResult;
+ EFI_STATUS Status;
+ EFI_USB_DEVICE_DESCRIPTOR DevDesc;
REPORT_STATUS_CODE_WITH_DEVICE_PATH (
EFI_PROGRESS_CODE,
@@ -823,55 +834,38 @@ InitUSBKeyboard (
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
+ // Get device descriptor so vendor/device IDs available
+ // for quirk handling
//
- Status = UsbGetConfiguration (
+ Status = UsbKeyboardDevice->UsbIo->UsbGetDeviceDescriptor (
UsbKeyboardDevice->UsbIo,
- &ConfigValue,
- &TransferResult
- );
+ &DevDesc);
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;
- }
+ return EFI_DEVICE_ERROR;
}
- 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;
@@ -896,6 +890,16 @@ InitUSBKeyboard (
UsbKeyboardDevice->CurrentNsKey = NULL;
+ //
+ // Some keyboards don't send interrupt transfers until SetIdle is called
+ //
+ if (UsbQuirkRequireSetIdle(DevDesc)) {
+ UsbSetIdleRequest(UsbKeyboardDevice->UsbIo,
+ UsbKeyboardDevice->InterfaceDescriptor.InterfaceNumber,
+ 0,
+ 0);
+ }
+
//
// Sync the initial state of lights on keyboard.
//
--
2.20.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#52866): https://edk2.groups.io/g/devel/message/52866
Mute This Topic: https://groups.io/mt/69457407/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
The endpoint selected by the driver needs to not
only be an interrupt type, but have direction IN
as required to set up an asynchronous interrupt transfer.
Currently, the driver assumes that the first INT endpoint
will be of type IN, but that is not true of all devices,
and will silently fail on devices which have the OUT endpoint
before the IN. Adjust the endpoint selection loop to explictly
check for direction IN.
Test: detachable keyboard on Google Pixel Slate now works.
Signed-off-by: Matt DeVillier <matt.devillier@gmail.com>
---
MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.c b/MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.c
index 27685995c2..ccb389067a 100644
--- a/MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.c
+++ b/MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.c
@@ -215,7 +215,7 @@ USBKeyboardDriverBindingStart (
EndpointNumber = UsbKeyboardDevice->InterfaceDescriptor.NumEndpoints;
//
- // Traverse endpoints to find interrupt endpoint
+ // Traverse endpoints to find interrupt endpoint IN
//
Found = FALSE;
for (Index = 0; Index < EndpointNumber; Index++) {
@@ -226,7 +226,8 @@ USBKeyboardDriverBindingStart (
&EndpointDescriptor
);
- if ((EndpointDescriptor.Attributes & (BIT0 | BIT1)) == USB_ENDPOINT_INTERRUPT) {
+ if (((EndpointDescriptor.Attributes & (BIT0 | BIT1)) == USB_ENDPOINT_INTERRUPT) &&
+ ((EndpointDescriptor.EndpointAddress & USB_ENDPOINT_DIR_IN) != 0)) {
//
// We only care interrupt endpoint here
//
--
2.20.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#52867): https://edk2.groups.io/g/devel/message/52867
Mute This Topic: https://groups.io/mt/69457409/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> MrChromebox
> Sent: Monday, January 06, 2020 12:12 PM
> To: devel@edk2.groups.io
> Cc: Matt DeVillier
> Subject: [edk2-devel] [PATCH v3 1/3] MdeModulePkg/Usb/EfiKey: Fix
> endpoint selection
>
> The endpoint selected by the driver needs to not
> only be an interrupt type, but have direction IN
> as required to set up an asynchronous interrupt transfer.
>
> Currently, the driver assumes that the first INT endpoint
> will be of type IN, but that is not true of all devices,
> and will silently fail on devices which have the OUT endpoint
> before the IN. Adjust the endpoint selection loop to explictly
> check for direction IN.
>
> Test: detachable keyboard on Google Pixel Slate now works.
>
> Signed-off-by: Matt DeVillier <matt.devillier@gmail.com>
> ---
> MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.c
> b/MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.c
> index 27685995c2..ccb389067a 100644
> --- a/MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.c
> +++ b/MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.c
> @@ -215,7 +215,7 @@ USBKeyboardDriverBindingStart (
> EndpointNumber = UsbKeyboardDevice-
> >InterfaceDescriptor.NumEndpoints;
>
> //
> - // Traverse endpoints to find interrupt endpoint
> + // Traverse endpoints to find interrupt endpoint IN
> //
> Found = FALSE;
> for (Index = 0; Index < EndpointNumber; Index++) {
> @@ -226,7 +226,8 @@ USBKeyboardDriverBindingStart (
> &EndpointDescriptor
> );
>
> - if ((EndpointDescriptor.Attributes & (BIT0 | BIT1)) ==
> USB_ENDPOINT_INTERRUPT) {
> + if (((EndpointDescriptor.Attributes & (BIT0 | BIT1)) ==
> USB_ENDPOINT_INTERRUPT) &&
> + ((EndpointDescriptor.EndpointAddress & USB_ENDPOINT_DIR_IN) != 0))
> {
Reviewed-by: Hao A Wu <hao.a.wu@intel.com>
Since there is only coding style change compared with V1 series, I will keep
the R-b tag by GuoMinJ <newexplorerj@gmail.com> as well.
Best Regards,
Hao Wu
> //
> // We only care interrupt endpoint here
> //
> --
> 2.20.1
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#52871): https://edk2.groups.io/g/devel/message/52871
Mute This Topic: https://groups.io/mt/69457409/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
The endpoint selected by the driver needs to not
only be an interrupt type, but have direction IN
as required to set up an asynchronous interrupt transfer.
Currently, the driver assumes that the first INT endpoint
will be of type IN, but that is not true of all devices,
and will silently fail on devices which have the OUT endpoint
before the IN. Adjust the endpoint selection loop to explictly
check for direction IN.
Signed-off-by: Matt DeVillier <matt.devillier@gmail.com>
---
MdeModulePkg/Bus/Usb/UsbMouseDxe/UsbMouse.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/MdeModulePkg/Bus/Usb/UsbMouseDxe/UsbMouse.c b/MdeModulePkg/Bus/Usb/UsbMouseDxe/UsbMouse.c
index 677815a8ad..143ff15eb0 100644
--- a/MdeModulePkg/Bus/Usb/UsbMouseDxe/UsbMouse.c
+++ b/MdeModulePkg/Bus/Usb/UsbMouseDxe/UsbMouse.c
@@ -203,7 +203,7 @@ USBMouseDriverBindingStart (
EndpointNumber = UsbMouseDevice->InterfaceDescriptor.NumEndpoints;
//
- // Traverse endpoints to find interrupt endpoint
+ // Traverse endpoints to find interrupt endpoint IN
//
Found = FALSE;
for (Index = 0; Index < EndpointNumber; Index++) {
@@ -213,7 +213,8 @@ USBMouseDriverBindingStart (
&EndpointDescriptor
);
- if ((EndpointDescriptor.Attributes & (BIT0 | BIT1)) == USB_ENDPOINT_INTERRUPT) {
+ if (((EndpointDescriptor.Attributes & (BIT0 | BIT1)) == USB_ENDPOINT_INTERRUPT) &&
+ ((EndpointDescriptor.EndpointAddress & USB_ENDPOINT_DIR_IN) != 0)) {
//
// We only care interrupt endpoint here
//
--
2.20.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#52868): https://edk2.groups.io/g/devel/message/52868
Mute This Topic: https://groups.io/mt/69457410/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> MrChromebox
> Sent: Monday, January 06, 2020 12:12 PM
> To: devel@edk2.groups.io
> Cc: Matt DeVillier
> Subject: [edk2-devel] [PATCH v3 2/3] MdeModulePkg/Usb/UsbMouse: Fix
> endpoint selection
>
> The endpoint selected by the driver needs to not
> only be an interrupt type, but have direction IN
> as required to set up an asynchronous interrupt transfer.
>
> Currently, the driver assumes that the first INT endpoint
> will be of type IN, but that is not true of all devices,
> and will silently fail on devices which have the OUT endpoint
> before the IN. Adjust the endpoint selection loop to explictly
> check for direction IN.
>
> Signed-off-by: Matt DeVillier <matt.devillier@gmail.com>
> ---
> MdeModulePkg/Bus/Usb/UsbMouseDxe/UsbMouse.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/MdeModulePkg/Bus/Usb/UsbMouseDxe/UsbMouse.c
> b/MdeModulePkg/Bus/Usb/UsbMouseDxe/UsbMouse.c
> index 677815a8ad..143ff15eb0 100644
> --- a/MdeModulePkg/Bus/Usb/UsbMouseDxe/UsbMouse.c
> +++ b/MdeModulePkg/Bus/Usb/UsbMouseDxe/UsbMouse.c
> @@ -203,7 +203,7 @@ USBMouseDriverBindingStart (
> EndpointNumber = UsbMouseDevice->InterfaceDescriptor.NumEndpoints;
>
> //
> - // Traverse endpoints to find interrupt endpoint
> + // Traverse endpoints to find interrupt endpoint IN
> //
> Found = FALSE;
> for (Index = 0; Index < EndpointNumber; Index++) {
> @@ -213,7 +213,8 @@ USBMouseDriverBindingStart (
> &EndpointDescriptor
> );
>
> - if ((EndpointDescriptor.Attributes & (BIT0 | BIT1)) ==
> USB_ENDPOINT_INTERRUPT) {
> + if (((EndpointDescriptor.Attributes & (BIT0 | BIT1)) ==
> USB_ENDPOINT_INTERRUPT) &&
> + ((EndpointDescriptor.EndpointAddress & USB_ENDPOINT_DIR_IN) != 0))
> {
Reviewed-by: Hao A Wu <hao.a.wu@intel.com>
Since there is only coding style change compared with V1 series, I will keep
the R-b tag by GuoMinJ <newexplorerj@gmail.com> as well.
Best Regards,
Hao Wu
> //
> // We only care interrupt endpoint here
> //
> --
> 2.20.1
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#52872): https://edk2.groups.io/g/devel/message/52872
Mute This Topic: https://groups.io/mt/69457410/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
The endpoint selected by the driver needs to not
only be an interrupt type, but have direction IN
as required to set up an asynchronous interrupt transfer.
Currently, the driver assumes that the first INT endpoint
will be of type IN, but that is not true of all devices,
and will silently fail on devices which have the OUT endpoint
before the IN. Adjust the endpoint selection loop to explictly
check for direction IN.
Signed-off-by: Matt DeVillier <matt.devillier@gmail.com>
---
MdeModulePkg/Bus/Usb/UsbMouseAbsolutePointerDxe/UsbMouseAbsolutePointer.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/MdeModulePkg/Bus/Usb/UsbMouseAbsolutePointerDxe/UsbMouseAbsolutePointer.c b/MdeModulePkg/Bus/Usb/UsbMouseAbsolutePointerDxe/UsbMouseAbsolutePointer.c
index 8953e7031c..9cd0e4cd53 100644
--- a/MdeModulePkg/Bus/Usb/UsbMouseAbsolutePointerDxe/UsbMouseAbsolutePointer.c
+++ b/MdeModulePkg/Bus/Usb/UsbMouseAbsolutePointerDxe/UsbMouseAbsolutePointer.c
@@ -203,7 +203,7 @@ USBMouseAbsolutePointerDriverBindingStart (
EndpointNumber = UsbMouseAbsolutePointerDevice->InterfaceDescriptor.NumEndpoints;
//
- // Traverse endpoints to find interrupt endpoint
+ // Traverse endpoints to find interrupt endpoint IN
//
Found = FALSE;
for (Index = 0; Index < EndpointNumber; Index++) {
@@ -213,7 +213,8 @@ USBMouseAbsolutePointerDriverBindingStart (
&EndpointDescriptor
);
- if ((EndpointDescriptor.Attributes & (BIT0 | BIT1)) == USB_ENDPOINT_INTERRUPT) {
+ if (((EndpointDescriptor.Attributes & (BIT0 | BIT1)) == USB_ENDPOINT_INTERRUPT) &&
+ ((EndpointDescriptor.EndpointAddress & USB_ENDPOINT_DIR_IN) != 0)) {
//
// We only care interrupt endpoint here
//
--
2.20.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#52869): https://edk2.groups.io/g/devel/message/52869
Mute This Topic: https://groups.io/mt/69457411/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Best Regards,
Hao Wu
> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> MrChromebox
> Sent: Monday, January 06, 2020 12:12 PM
> To: devel@edk2.groups.io
> Cc: Matt DeVillier
> Subject: [edk2-devel] [PATCH v3 3/3]
> MdeModulePkg/UsbMouseAbsolutePointer: Fix endpoint selection
>
> The endpoint selected by the driver needs to not
> only be an interrupt type, but have direction IN
> as required to set up an asynchronous interrupt transfer.
>
> Currently, the driver assumes that the first INT endpoint
> will be of type IN, but that is not true of all devices,
> and will silently fail on devices which have the OUT endpoint
> before the IN. Adjust the endpoint selection loop to explictly
> check for direction IN.
>
> Signed-off-by: Matt DeVillier <matt.devillier@gmail.com>
> ---
>
> MdeModulePkg/Bus/Usb/UsbMouseAbsolutePointerDxe/UsbMouseAbsolu
> tePointer.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git
> a/MdeModulePkg/Bus/Usb/UsbMouseAbsolutePointerDxe/UsbMouseAbs
> olutePointer.c
> b/MdeModulePkg/Bus/Usb/UsbMouseAbsolutePointerDxe/UsbMouseAbs
> olutePointer.c
> index 8953e7031c..9cd0e4cd53 100644
> ---
> a/MdeModulePkg/Bus/Usb/UsbMouseAbsolutePointerDxe/UsbMouseAbs
> olutePointer.c
> +++
> b/MdeModulePkg/Bus/Usb/UsbMouseAbsolutePointerDxe/UsbMouseAbs
> olutePointer.c
> @@ -203,7 +203,7 @@ USBMouseAbsolutePointerDriverBindingStart (
> EndpointNumber = UsbMouseAbsolutePointerDevice-
> >InterfaceDescriptor.NumEndpoints;
>
> //
> - // Traverse endpoints to find interrupt endpoint
> + // Traverse endpoints to find interrupt endpoint IN
> //
> Found = FALSE;
> for (Index = 0; Index < EndpointNumber; Index++) {
> @@ -213,7 +213,8 @@ USBMouseAbsolutePointerDriverBindingStart (
> &EndpointDescriptor
> );
>
> - if ((EndpointDescriptor.Attributes & (BIT0 | BIT1)) ==
> USB_ENDPOINT_INTERRUPT) {
> + if (((EndpointDescriptor.Attributes & (BIT0 | BIT1)) ==
> USB_ENDPOINT_INTERRUPT) &&
> + ((EndpointDescriptor.EndpointAddress & USB_ENDPOINT_DIR_IN) != 0))
> {
Reviewed-by: Hao A Wu <hao.a.wu@intel.com>
Best Regards,
Hao Wu
> //
> // We only care interrupt endpoint here
> //
> --
> 2.20.1
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#52873): https://edk2.groups.io/g/devel/message/52873
Mute This Topic: https://groups.io/mt/69457411/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
© 2016 - 2024 Red Hat, Inc.