On Tue, Sep 01, 2020 at 01:31:18AM -0500, Matt DeVillier wrote:
> From 9e408a5441330b120a477324c017c0525cb5b365 Mon Sep 17 00:00:00 2001
> From: Matt DeVillier <matt.devill...@puri.sm>
> Date: Tue, 1 Sep 2020 01:21:23 -0500
> Subject: [PATCH] usb-hid: Fix keyboards using non-primary interface descriptor
>
> A fair number of USB keyboards use an interface descriptor other than
> the first available, making them non-functional currently.
> To correct this, iterate through all available interface descriptors
> until one with the correct class/subclass is found, then call usb_hid_setup().
>
> Tested on an ultimate hacking keyboard (UHK 60)
>
> Signed-off-by: Matt DeVillier <matt.devill...@puri.sm>
> ---
> src/hw/usb-hid.c | 9 ++-------
> src/hw/usb.c | 17 +++++++++++++++--
> 2 files changed, 17 insertions(+), 9 deletions(-)
>
> diff --git a/src/hw/usb-hid.c b/src/hw/usb-hid.c
> index a22765b..c1ceaba 100644
> --- a/src/hw/usb-hid.c
> +++ b/src/hw/usb-hid.c
> @@ -138,11 +138,6 @@ usb_hid_setup(struct usbdevice_s *usbdev)
> return -1;
> dprintf(2, "usb_hid_setup %p\n", usbdev->defpipe);
>
> - struct usb_interface_descriptor *iface = usbdev->iface;
> - if (iface->bInterfaceSubClass != USB_INTERFACE_SUBCLASS_BOOT)
> - // Doesn't support boot protocol.
> - return -1;
Is this change needed?
> -
> // Find intr in endpoint.
> struct usb_endpoint_descriptor *epdesc = usb_find_desc(
> usbdev, USB_ENDPOINT_XFER_INT, USB_DIR_IN);
> @@ -151,9 +146,9 @@ usb_hid_setup(struct usbdevice_s *usbdev)
> return -1;
> }
>
> - if (iface->bInterfaceProtocol == USB_INTERFACE_PROTOCOL_KEYBOARD)
> + if (usbdev->iface->bInterfaceProtocol == USB_INTERFACE_PROTOCOL_KEYBOARD)
> return usb_kbd_setup(usbdev, epdesc);
> - if (iface->bInterfaceProtocol == USB_INTERFACE_PROTOCOL_MOUSE)
> + if (usbdev->iface->bInterfaceProtocol == USB_INTERFACE_PROTOCOL_MOUSE)
> return usb_mouse_setup(usbdev, epdesc);
> return -1;
> }
> diff --git a/src/hw/usb.c b/src/hw/usb.c
> index 4f9a852..aea70a5 100644
> --- a/src/hw/usb.c
> +++ b/src/hw/usb.c
> @@ -391,8 +391,21 @@ configure_usb_device(struct usbdevice_s *usbdev)
> ret = usb_msc_setup(usbdev);
> if (iface->bInterfaceProtocol == US_PR_UAS)
> ret = usb_uas_setup(usbdev);
> - } else
> - ret = usb_hid_setup(usbdev);
> + } else {
> + // Some keyboards use an interface other than the first one
> + // so iterate through all available
> + for (int i=0; i<config->bNumInterfaces; i++) {
FYI, some older versions of gcc will complain with the above. Declare
the "int i;" outside the for().
> + if (iface->bInterfaceClass == USB_CLASS_HID &&
> + iface->bInterfaceSubClass ==
> USB_INTERFACE_SUBCLASS_BOOT) {
> + usbdev->iface = iface;
> + ret = usb_hid_setup(usbdev);
> + break;
> + } else {
> + iface = (void*)iface + iface->bLength;
> + ret = -1;
> + }
> + }
> + }
If we're going to support multiple interfaces, I think it would be
preferable to expand the loop so that it also works for MASS_STORAGE
and HUB devices.
Also, if an alternate interface is used, I think the usb SET_INTERFACE
command needs to be sent. (That particular keyboard may work without
SET_INTERFACE, but it may not work on another.)
Thanks.
-Kevin
_______________________________________________
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-le...@seabios.org
