Re: [edk2-devel] [PATCH 1/3] MdeModulePkg: Add ClockRate field to SerialPortInfo

2023-10-11 Thread MrChromebox
hi Pedro,

thanks for the input, I will adjust the patch to append for v2

On Wed, Oct 11, 2023 at 2:16 PM Pedro Falcato 
wrote:

> On Mon, Oct 9, 2023 at 4:54 PM Matt DeVillier 
> wrote:
> >
> > On Thu, Oct 5, 2023 at 12:45 PM Pedro Falcato 
> wrote:
> >>
> >> On Wed, Oct 4, 2023 at 9:01 PM MrChromebox 
> wrote:
> >> >
> >> > Add the ClockRate field to the UNIVERSAL_PAYLOAD_SERIAL_PORT_INFO
> >> > struct, so that the field can be used by UefiPayloadPkg to properly
> >> > set up the serial port on boards using a non-standard clock rate.
> >> >
> >> > Signed-off-by: Matt DeVillier 
> >> > Change-Id: I9bcaf03ab63f6a45d2cf25a580f7a2eba388cbbd
> >>
> >> Series-generic-feedback: Remove Change-Id lines and CC the proper
> >> maintainers for each patch
> >
> >
> > ack
> >
> >>
> >> > ---
> >> >  MdeModulePkg/Include/UniversalPayload/SerialPortInfo.h | 1 +
> >> >  1 file changed, 1 insertion(+)
> >> >
> >> > diff --git a/MdeModulePkg/Include/UniversalPayload/SerialPortInfo.h
> b/MdeModulePkg/Include/UniversalPayload/SerialPortInfo.h
> >> > index 3c4459e2c0..e3c9f93654 100644
> >> > --- a/MdeModulePkg/Include/UniversalPayload/SerialPortInfo.h
> >> > +++ b/MdeModulePkg/Include/UniversalPayload/SerialPortInfo.h
> >> > @@ -19,6 +19,7 @@ typedef struct {
> >> >BOOLEAN UseMmio;
> >> >UINT8   RegisterStride;
> >> >UINT32  BaudRate;
> >> > +  UINT32  ClockRate;
> >>
> >> I don't think you can do this? UNIVERSAL_PAYLOAD_SERIAL_PORT_INFO is
> >> part of a spec (
> https://universalscalablefirmware.github.io/documentation/2_universal_payload.html
> )
> >> and it doesn't even seem to be versioned, so you'd just break ABI. Am
> >> I missing something?
> >
> >
> > The USF spec says that it is currently at version 0.7 and under
> development / not final. I don't see why adding a field to a hob is
> problematic provided it's properly documented. The alternatives are some
> really hacky workarounds to pass the necessary data, or the serial port not
> working.
>
> My 2 cents, as someone that neither maintains nor works on this code:
> 1) This sounds like something that needs to be discussed with the spec
> people (good luck! lol)
> 2) Changing the structure abruptly like this seems like it would break
> many current users. HOWEVER:
> The spec (
> https://universalscalablefirmware.github.io/documentation/2_universal_payload.html#common-payload-header
> )
> says that the common header's Revision (indeed, it's versioned and I
> missed that) should be incremented if current members are renamed or
> reinterpreted, but not if you just append members. So it seems that if
> you add ClockRate like this:
>
> --- a/MdeModulePkg/Include/UniversalPayload/SerialPortInfo.h
> +++ b/MdeModulePkg/Include/UniversalPayload/SerialPortInfo.h
> @@ -19,6 +19,7 @@ typedef struct {
>BOOLEAN UseMmio;
>UINT8   RegisterStride;
>UINT32  BaudRate;
>EFI_PHYSICAL_ADDRESSRegisterBase;
> +  UINT32  ClockRate;
> } UNIVERSAL_PAYLOAD_SERIAL_PORT_INFO;
>
> --
>
> this should be acceptable for the spec people and
> EDK2/coreboot/SlimBootloader/etc people, given all the current
> consuming code takes the generic header's Length into account (as
> specified by the spec).
>
> --
> Pedro
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109535): https://edk2.groups.io/g/devel/message/109535
Mute This Topic: https://groups.io/mt/101763374/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] [PATCH 1/3] MdeModulePkg: Add ClockRate field to SerialPortInfo

2023-10-09 Thread MrChromebox
On Thu, Oct 5, 2023 at 12:45 PM Pedro Falcato 
wrote:

> On Wed, Oct 4, 2023 at 9:01 PM MrChromebox 
> wrote:
> >
> > Add the ClockRate field to the UNIVERSAL_PAYLOAD_SERIAL_PORT_INFO
> > struct, so that the field can be used by UefiPayloadPkg to properly
> > set up the serial port on boards using a non-standard clock rate.
> >
> > Signed-off-by: Matt DeVillier 
> > Change-Id: I9bcaf03ab63f6a45d2cf25a580f7a2eba388cbbd
>
> Series-generic-feedback: Remove Change-Id lines and CC the proper
> maintainers for each patch
>

ack


> > ---
> >  MdeModulePkg/Include/UniversalPayload/SerialPortInfo.h | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/MdeModulePkg/Include/UniversalPayload/SerialPortInfo.h
> b/MdeModulePkg/Include/UniversalPayload/SerialPortInfo.h
> > index 3c4459e2c0..e3c9f93654 100644
> > --- a/MdeModulePkg/Include/UniversalPayload/SerialPortInfo.h
> > +++ b/MdeModulePkg/Include/UniversalPayload/SerialPortInfo.h
> > @@ -19,6 +19,7 @@ typedef struct {
> >BOOLEAN UseMmio;
> >UINT8   RegisterStride;
> >UINT32  BaudRate;
> > +  UINT32  ClockRate;
>
> I don't think you can do this? UNIVERSAL_PAYLOAD_SERIAL_PORT_INFO is
> part of a spec (
> https://universalscalablefirmware.github.io/documentation/2_universal_payload.html
> )
> and it doesn't even seem to be versioned, so you'd just break ABI. Am
> I missing something?
>

The USF spec says that it is currently at version 0.7 and under development
/ not final. I don't see why adding a field to a hob is problematic
provided it's properly documented. The alternatives are some really hacky
workarounds to pass the necessary data, or the serial port not working.


>
> --
> Pedro
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109461): https://edk2.groups.io/g/devel/message/109461
Mute This Topic: https://groups.io/mt/101763374/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] [PATCH 3/3] PlatformHookLib: Set PcdSerialClockRate using HOB data

2023-10-04 Thread MrChromebox
Fixes serial output on platforms using coreboot and a non-default
clock rate such as AMD Picasso and newer Zen-based platforms.

Signed-off-by: Matt DeVillier 
Change-Id: I91290397852176754e9a34ec6e5829044f41d15a
---
 UefiPayloadPkg/Library/PlatformHookLib/PlatformHookLib.c   | 5 +
 UefiPayloadPkg/Library/PlatformHookLib/PlatformHookLib.inf | 1 +
 2 files changed, 6 insertions(+)

diff --git a/UefiPayloadPkg/Library/PlatformHookLib/PlatformHookLib.c 
b/UefiPayloadPkg/Library/PlatformHookLib/PlatformHookLib.c
index 60a17b8fc2..e3d47ac2fa 100644
--- a/UefiPayloadPkg/Library/PlatformHookLib/PlatformHookLib.c
+++ b/UefiPayloadPkg/Library/PlatformHookLib/PlatformHookLib.c
@@ -90,6 +90,11 @@ PlatformHookSerialPortInitialize (
   return Status;
 }
 
+Status = PcdSet32S (PcdSerialClockRate, SerialPortInfo->ClockRate);
+if (RETURN_ERROR (Status)) {
+  return Status;
+}
+
 return RETURN_SUCCESS;
   }
 
diff --git a/UefiPayloadPkg/Library/PlatformHookLib/PlatformHookLib.inf 
b/UefiPayloadPkg/Library/PlatformHookLib/PlatformHookLib.inf
index 7ac6bfa1b1..e2908cfbca 100644
--- a/UefiPayloadPkg/Library/PlatformHookLib/PlatformHookLib.inf
+++ b/UefiPayloadPkg/Library/PlatformHookLib/PlatformHookLib.inf
@@ -38,3 +38,4 @@
   gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase## PRODUCES
   gEfiMdeModulePkgTokenSpaceGuid.PcdSerialBaudRate## PRODUCES
   gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterStride  ## PRODUCES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialClockRate   ## PRODUCES
-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109337): https://edk2.groups.io/g/devel/message/109337
Mute This Topic: https://groups.io/mt/101763377/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] [PATCH 2/3] UefiPayloadEntry: Set serial ClockRate from parsed SerialInfo

2023-10-04 Thread MrChromebox
Extract and use the serial port clock rate provided by coreboot via
the InputHertz field.

Signed-off-by: Matt DeVillier 
Change-Id: If764bd7c0b691cf887205471d0343fdf62372141
---
 UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c 
b/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c
index 030a5baed9..91dfac0d76 100644
--- a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c
+++ b/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c
@@ -431,6 +431,7 @@ _ModuleEntryPoint (
 UniversalSerialPort->UseMmio = (SerialPortInfo.Type == 1) ? FALSE 
: TRUE;
 UniversalSerialPort->RegisterBase= SerialPortInfo.BaseAddr;
 UniversalSerialPort->BaudRate= SerialPortInfo.Baud;
+UniversalSerialPort->ClockRate   = SerialPortInfo.InputHertz;
 UniversalSerialPort->RegisterStride  = (UINT8)SerialPortInfo.RegWidth;
   }
 
-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109336): https://edk2.groups.io/g/devel/message/109336
Mute This Topic: https://groups.io/mt/101763376/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] [PATCH 1/3] MdeModulePkg: Add ClockRate field to SerialPortInfo

2023-10-04 Thread MrChromebox
Add the ClockRate field to the UNIVERSAL_PAYLOAD_SERIAL_PORT_INFO
struct, so that the field can be used by UefiPayloadPkg to properly
set up the serial port on boards using a non-standard clock rate.

Signed-off-by: Matt DeVillier 
Change-Id: I9bcaf03ab63f6a45d2cf25a580f7a2eba388cbbd
---
 MdeModulePkg/Include/UniversalPayload/SerialPortInfo.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MdeModulePkg/Include/UniversalPayload/SerialPortInfo.h 
b/MdeModulePkg/Include/UniversalPayload/SerialPortInfo.h
index 3c4459e2c0..e3c9f93654 100644
--- a/MdeModulePkg/Include/UniversalPayload/SerialPortInfo.h
+++ b/MdeModulePkg/Include/UniversalPayload/SerialPortInfo.h
@@ -19,6 +19,7 @@ typedef struct {
   BOOLEAN UseMmio;
   UINT8   RegisterStride;
   UINT32  BaudRate;
+  UINT32  ClockRate;
   EFI_PHYSICAL_ADDRESSRegisterBase;
 } UNIVERSAL_PAYLOAD_SERIAL_PORT_INFO;
 #pragma pack()
-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109335): https://edk2.groups.io/g/devel/message/109335
Mute This Topic: https://groups.io/mt/101763374/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] [PATCH 13/18] MdeModulePkg/Usb/Keyboard.c: don't request protocol before setting

2022-02-11 Thread MrChromebox
seems this patch got truncated during a rebase, here is the correct version.

For device that does not respond to GetProtocolRequest(), the original flow is:
a.1 Execute GetProtocolRequest()
a.2 Timeout occurs for GetProtocolRequest()
a.3 Conditionally execute UsbSetProtocolRequest()

The proposed new flow is:
b.1 Always execute UsbSetProtocolRequest()

The timeout delay for keyboards which do not support
GetProtocolRequest() that we've seen is significant (>5s, IIRC)

 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 5a94a4dda7..ad55fa3330 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,21 +853,28 @@ 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
+  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.32.0

On Thu, Feb 10, 2022 at 9:32 PM Wu, Hao A  wrote:
>
> Hello,
>
> Inline comments below:
>
>
> > -Original Message-
> > From: Sean Rhodes 
> > Sent: Friday, February 11, 2022 5:27 AM
> > To: devel@edk2.groups.io
> > Cc: Wu, Hao A ; Matt DeVillier
> > ; Patrick Rudolph
> > 
> > Subject: [PATCH 13/18] MdeModulePkg/Usb/Keyboard.c: don't request
> > protocol before setting
> >
> > From: Matt DeVillier 
> >
> > 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 
> > Signed-off-by: Patrick Rudolph 
> > ---
> >  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

Re: [edk2-devel] [PATCH 17/22] .mailmap: add entry for Matt DeVillier

2020-09-07 Thread MrChromebox
Acked-by Matt DeVillier 

On Mon, Sep 7, 2020 at 2:31 PM Laszlo Ersek  wrote:
>
> ... for git-shortlog purposes.
>
> Cc: Matt DeVillier 
> Cc: Philippe Mathieu-Daudé 
> Signed-off-by: Laszlo Ersek 
> ---
>  .mailmap | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/.mailmap b/.mailmap
> index ad593704a1d4..33652b8f0b46 100644
> --- a/.mailmap
> +++ b/.mailmap
> @@ -58,6 +58,7 @@ Marc W Chen 
>  Marvin Häuser 
>  Marvin Häuser  edk2-devel 
> 
>  Marvin Häuser 
> +Matt DeVillier 
>  Maurice Ma 
>  Michael D Kinney 
>  Michael Kubacki 
> --
> 2.19.1.3.g30247aa5d201
>
>

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#65113): https://edk2.groups.io/g/devel/message/65113
Mute This Topic: https://groups.io/mt/76694067/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



[edk2-devel] [PATCH v3 2/3] MdeModulePkg/Usb/UsbMouse: Fix endpoint selection

2020-01-05 Thread MrChromebox
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 
---
 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/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



[edk2-devel] [PATCH] debug

2020-01-05 Thread MrChromebox
---
 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/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



[edk2-devel] [PATCH v3 3/3] MdeModulePkg/UsbMouseAbsolutePointer: Fix endpoint selection

2020-01-05 Thread MrChromebox
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 
---
 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/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



[edk2-devel] [PATCH v3 1/3] MdeModulePkg/Usb/EfiKey: Fix endpoint selection

2020-01-05 Thread MrChromebox
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 
---
 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/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



[edk2-devel] [PATCH 3/3] MdeModulePkg/UsbMouseAbsolutePointer: Fix endpoint selection

2020-01-03 Thread MrChromebox
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 
---
 .../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 (#52835): https://edk2.groups.io/g/devel/message/52835
Mute This Topic: https://groups.io/mt/69401333/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



[edk2-devel] [PATCH v2 2/2] MdeModulePkg/Usb/UsbMouse: Fix endpoint selection

2020-01-02 Thread MrChromebox
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 
---
 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 (#52724): https://edk2.groups.io/g/devel/message/52724
Mute This Topic: https://groups.io/mt/69392851/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



[edk2-devel] [PATCH v2 0/1] MdeModulePkg/Usb/EfiKey: Fix endpoint selection

2020-01-02 Thread MrChromebox
USB keyboard initialization fails if the OUT interrupt
endpoint precedes the IN endpoint. Fix that.

Matt DeVillier (1):
  MdeModulePkg/Usb/EfiKey: Fix endpoint selection

 MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

-- 
2.20.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#52722): https://edk2.groups.io/g/devel/message/52722
Mute This Topic: https://groups.io/mt/69392824/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



[edk2-devel] [PATCH v2 1/1] MdeModulePkg/Usb/EfiKey: Fix endpoint selection

2020-01-02 Thread MrChromebox
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 
---
 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 (#52723): https://edk2.groups.io/g/devel/message/52723
Mute This Topic: https://groups.io/mt/69392825/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



[edk2-devel] [edk2] [PATCH 1/2] MdeModulePkg/Usb/EfiKey: Fix endpoint selection

2019-12-12 Thread MrChromebox
>From e7648a8c3efae0540586ad497a5366ffd0c87fb7 Mon Sep 17 00:00:00 2001
From: Matt DeVillier 
Date: Thu, 12 Dec 2019 12:46:47 -0600
Subject: [PATCH 1/2] 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 
---
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 40f8399942..f0544269de 100644
--- a/MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.c
+++ b/MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.c
@@ -221,7 +221,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++) {
@@ -232,7 +232,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) ) {
//
// We only care interrupt endpoint here
//
--
2.20.1

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#52175): https://edk2.groups.io/g/devel/message/52175
Mute This Topic: https://groups.io/mt/68284872/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



[edk2-devel] [edk2] [PATCH 2/2] MdeModulePkg/Usb/UsbMouse: Fix endpoint selection

2019-12-12 Thread MrChromebox
>From 2a8e343f8a5848825d3d12ba76df0fb4212ab5b8 Mon Sep 17 00:00:00 2001
From: Matt DeVillier 
Date: Thu, 12 Dec 2019 12:54:14 -0600
Subject: [PATCH 2/2] 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 
---
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 f4448bffe6..e9f74cec60 100644
--- a/MdeModulePkg/Bus/Usb/UsbMouseDxe/UsbMouse.c
+++ b/MdeModulePkg/Bus/Usb/UsbMouseDxe/UsbMouse.c
@@ -209,7 +209,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++) {
@@ -219,7 +219,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) ) {
//
// We only care interrupt endpoint here
//
--
2.20.1

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#52174): https://edk2.groups.io/g/devel/message/52174
Mute This Topic: https://groups.io/mt/68284836/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



[edk2-devel] Detachable USB keyboard failing to init

2019-12-09 Thread MrChromebox
Greetings!

I have a Chromebook here which has a detachable USB keyboard/touchpad which is 
failing to initialize in the edk2 environment (regardless if connected before 
boot or after), but works fine once an OS is booted (both Windows and Linux). 
I've compiled a debug build, but having a hard time making sense of the XHCI 
init and comparing it to the working Linux xhci code.  Attached is the output 
from both; any help is appreciated. I did try increasing the 
XHC_RESET_RECOVERY_DELAY (to 250ms) as some searching turned up that as a 
possible cause, but no luck. Both logs below start when the device is attached.

thanks in advance!

edk2 debug output:

XhcClearRootHubPortFeature: status Success
UsbEnumeratePort: port 6 state - 01, change - 01 on 7A249C98
UsbEnumeratePort: Device Connect/Disconnect Normally
UsbEnumeratePort: new device connected at port 6
XhcUsbPortReset!
XhcSetRootHubPortFeature: status Success
XhcClearRootHubPortFeature: status Success
XhcClearRootHubPortFeature: status Success
Enable Slot Successfully, The Slot ID = 0x2
Address 2 assigned successfully
UsbEnumerateNewDev: hub port 6 is reset
UsbEnumerateNewDev: device is of 0 speed
UsbEnumerateNewDev: device uses translator (0, 0)
UsbEnumerateNewDev: device is now ADDRESSED at 2
Evaluate context
UsbEnumerateNewDev: max packet size for EP 0 is 64
Evaluate context
UsbBuildDescTable: device has 1 configures
UsbGetOneConfig: total length is 32
UsbParseConfigDesc: config 1 has 1 interfaces
UsbParseInterfaceDesc: interface 0(setting 0) has 2 endpoints
Endpoint[81]: Created BULK ring [7A277300~7A278300)
Endpoint[1]: Created BULK ring [7A278300~7A279300)
Configure Endpoint
UsbEnumerateNewDev: device 2 is now in CONFIGED state
UsbSelectConfig: config 1 selected for device 2
UsbSelectSetting: setting 0 selected for interface 0
InstallProtocolInterface: 09576E91-6D3F-11D2-8E39-00A0C969723B 791A7398
InstallProtocolInterface: 2B2F68D6-0CD2-44CF-8E8B-BBA20B1B5B75 791E3DC0
UsbConnectDriver: TPL before connect is 8, 791A7B98
UsbConnectDriver: TPL after connect is 8
UsbSelectConfig: failed to connect driver Not Found, ignored
PROGRESS CODE: V02020006 I0
XhcClearRootHubPortFeature: status Disable device slot 2!
Success
UsbEnumeratePort: port 6 state - 01, change - 01 on 7A249C98
UsbEnumeratePort: Device Connect/Disconnect Normally
UsbEnumeratePort: device at port 6 removed from root hub 7A249C98
UsbRemoveDevice: device 2 removed
UsbEnumeratePort: new device connected at port 6
XhcUsbPortReset!
XhcSetRootHubPortFeature: status Success
XhcClearRootHubPortFeature: status Success
XhcClearRootHubPortFeature: status Success
Enable Slot Successfully, The Slot ID = 0x3
Address 3 assigned successfully
UsbEnumerateNewDev: hub port 6 is reset
UsbEnumerateNewDev: device is of 0 speed
UsbEnumerateNewDev: device uses translator (0, 0)
UsbEnumerateNewDev: device is now ADDRESSED at 2
Evaluate context
UsbEnumerateNewDev: max packet size for EP 0 is 64
Evaluate context
UsbBuildDescTable: device has 1 configures
UsbGetOneConfig: total length is 144
UsbParseConfigDesc: config 1 has 5 interfaces
UsbParseInterfaceDesc: interface 0(setting 0) has 2 endpoints
UsbParseInterfaceDesc: interface 1(setting 0) has 2 endpoints
UsbParseInterfaceDesc: interface 2(setting 0) has 1 endpoints
UsbParseInterfaceDesc: interface 3(setting 0) has 2 endpoints
UsbParseInterfaceDesc: interface 4(setting 0) has 0 endpoints
UsbParseInterfaceDesc: interface 4(setting 1) has 2 endpoints
Endpoint[2]: Created INT ring [7A277300~7A278300)
Endpoint[82]: Created INT ring [7A278300~7A279300)
Endpoint[81]: Created BULK ring [7A279300~7A27A300)
Endpoint[1]: Created BULK ring [7A27A300~7A27B300)
Endpoint[83]: Created INT ring [7A27B300~7A27C300)
Endpoint[84]: Created BULK ring [7A27C300~7A27D300)
Endpoint[4]: Created BULK ring [7921C000~7921D000)
Configure Endpoint
UsbEnumerateNewDev: device 2 is now in CONFIGED state
UsbSelectConfig: config 1 selected for device 2
UsbSelectSetting: setting 0 selected for interface 0
InstallProtocolInterface: 09576E91-6D3F-11D2-8E39-00A0C969723B 791DF218
InstallProtocolInterface: 2B2F68D6-0CD2-44CF-8E8B-BBA20B1B5B75 791A7EC0
UsbConnectDriver: TPL before connect is 8, 791DF598
PROGRESS CODE: V01010004 I0
PROGRESS CODE: V01010003 I0
PROGRESS CODE: V01010006 I0
InstallProtocolInterface: 387477C1-69C7-11D2-8E39-00A0C969723B 7921B038
InstallProtocolInterface: DD9E7534-7762-4698-8C14-F58517A625AA 7921B050
PROGRESS CODE: V01010001 I0
PROGRESS CODE: V01011001 I0
XhcCheckUrbResult: STALL_ERROR! Completecode = 6
Recovery Halted Slot = 3,Dci = 1
XhcResetEndpoint: Slot = 0x3, Dci = 0x1
XhcSetTrDequeuePointer: Slot = 0x3, Dci = 0x1, Urb = 0x79218E98
XhcControlTransfer: error - Device Error, transfer - 2
Configure Endpoint
XhcCheckUrbResult: STALL_ERROR! Completecode = 6
Recovery Halted Slot = 3,Dci = 1
XhcResetEndpoint: Slot = 0x3, Dci = 0x1
XhcSetTrDequeuePointer: Slot = 0x3, Dci = 0x1, Urb = 0x79218E98
XhcControlTransfer: error - Device Error, transfer - 2
UsbConnectDriver: TPL after