Skip to content

Commit

Permalink
Add SetProtocol command to force devices to Report Mode (#361)
Browse files Browse the repository at this point in the history
## Description

USB HID specification 1.11
(https://www.usb.org/document-library/device-class-definition-hid-111)
section 7.2.6 states:
```
When initialized, all devices default to report protocol. However the host should 
not make any assumptions about the device’s state and should set the desired 
protocol whenever initializing a device. 
```

In testing actual devices, it has been observed that some actual
endpoint HID devices come up in "boot protocol" rather than "report
protocol." This PR implements the recommendation that the host (in this
case, the UsbHidDxe driver) not make any assumptions about device state,
and explicitly sets report protocol for devices that implement the
"boot" interface subclass.

The PR also makes a minor adjustment to debug verbosity. 

- [ ] Impacts functionality?
- [ ] Impacts security?
- [ ] Breaking change?
- [ ] Includes tests?
- [ ] Includes documentation?

## How This Was Tested

Verified with QEMU that command is sent; verified with hardware that has
this issue that SetProtocol resolves it.

## Integration Instructions

N/A
  • Loading branch information
joschock authored Nov 16, 2023
1 parent 1edf727 commit 14c187b
Showing 1 changed file with 22 additions and 3 deletions.
25 changes: 22 additions & 3 deletions HidPkg/UsbHidDxe/UsbHidDxe.c
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,12 @@

#include <IndustryStandard/Usb.h>

#define CLASS_HID 3
#define CLASS_HID 3
#define SUBCLASS_BOOT 1

// Refer to USB HID 1.11, section 7.2.6.
#define BOOT_PROTOCOL 0
#define REPORT_PROTOCOL 1

//
// A common header for usb standard descriptor.
Expand Down Expand Up @@ -731,7 +736,7 @@ ReadDescriptors (
UINT8 Index;
EFI_USB_ENDPOINT_DESCRIPTOR EndpointDescriptor;

DEBUG ((DEBUG_INFO, "[%a:%d] getting descriptors.\n", __FUNCTION__, __LINE__));
DEBUG ((DEBUG_VERBOSE, "[%a:%d] getting descriptors.\n", __FUNCTION__, __LINE__));

ZeroMem (&UsbHidDevice->IntInEndpointDescriptor, sizeof (UsbHidDevice->IntInEndpointDescriptor));

Expand All @@ -742,7 +747,7 @@ ReadDescriptors (
}

DEBUG ((
DEBUG_INFO,
DEBUG_VERBOSE,
"[%a:%d] interface class: 0x%x, subclass: 0x%x, protocol: 0x%x.\n",
__FUNCTION__,
__LINE__,
Expand Down Expand Up @@ -948,6 +953,20 @@ UsbHidDriverBindingStart (
goto ErrorExit;
}

// Some boot devices send a report descriptor for the "non-boot" reports they support
// but then send boot reports unless they are explicitly configured for report mode.
// So explicitly set the device to report mode if it is a boot device.
if (UsbHidDevice->InterfaceDescriptor.InterfaceSubClass == SUBCLASS_BOOT) {
Status = UsbSetProtocolRequest (
UsbHidDevice->UsbIo,
UsbHidDevice->InterfaceDescriptor.InterfaceNumber,
REPORT_PROTOCOL
);
if (EFI_ERROR (Status)) {
DEBUG ((DEBUG_WARN, "[%a] failed to set report protocol on device: %r\n", __FUNCTION__, Status));
}
}

Status = gBS->InstallProtocolInterface (
&Controller,
&gHidIoProtocolGuid,
Expand Down

0 comments on commit 14c187b

Please sign in to comment.