On Thu, Sep 03, 2020 at 09:03:50PM -0500, Matt DeVillier wrote: > On Thu, Sep 3, 2020 at 7:28 PM Kevin O'Connor <ke...@koconnor.net> wrote: > > 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.) > > how about something like: > > int i; > ret = -1; > for (i=0; i<config->bNumInterfaces; i++) { > if (iface->bInterfaceClass == USB_CLASS_HUB) > ret = usb_hub_setup(usbdev); > else if (iface->bInterfaceClass == USB_CLASS_MASS_STORAGE) { > if (iface->bInterfaceProtocol == US_PR_BULK) > ret = usb_msc_setup(usbdev); > else if (iface->bInterfaceProtocol == US_PR_UAS) > ret = usb_uas_setup(usbdev); > } else > ret = usb_hid_setup(usbdev); > > if (ret == 0) > break; > > iface = (void*)iface + iface->bLength; > usb_set_interface(iface); //need to create this > usbdev->iface = iface; > }
Wouldn't it be better to run the check before calling set_configure()? Perhaps something like the below (totally untested). -Kevin diff --git a/src/hw/usb.c b/src/hw/usb.c index 4f9a852..732d4cd 100644 --- a/src/hw/usb.c +++ b/src/hw/usb.c @@ -248,14 +248,14 @@ get_device_config(struct usb_pipe *pipe) if (ret) return NULL; - void *config = malloc_tmphigh(cfg.wTotalLength); + struct usb_config_descriptor *config = malloc_tmphigh(cfg.wTotalLength); if (!config) { warn_noalloc(); return NULL; } req.wLength = cfg.wTotalLength; ret = usb_send_default_control(pipe, &req, config); - if (ret) { + if (ret || config->wTotalLength != cfg.wTotalLength) { free(config); return NULL; } @@ -367,19 +367,33 @@ configure_usb_device(struct usbdevice_s *usbdev) return 0; // Determine if a driver exists for this device - only look at the - // first interface of the first configuration. + // interfaces of the first configuration. + int num_iface = config->bNumInterfaces; + void *config_end = (void*)config + config->wTotalLength; struct usb_interface_descriptor *iface = (void*)(&config[1]); - if (iface->bInterfaceClass != USB_CLASS_HID - && iface->bInterfaceClass != USB_CLASS_MASS_STORAGE - && iface->bInterfaceClass != USB_CLASS_HUB) - // Not a supported device. - goto fail; + for (;;) { + if (!num_iface-- || (void*)iface + iface->bLength >= config_end) + // Not a supported device. + goto fail; + if (iface->bInterfaceClass == USB_CLASS_HUB + || (iface->bInterfaceClass == USB_CLASS_MASS_STORAGE + && (iface->bInterfaceProtocol == US_PR_BULK + || iface->bInterfaceProtocol == US_PR_UAS)) + || (iface->bInterfaceClass == USB_CLASS_HID + && iface->bInterfaceSubClass == USB_INTERFACE_SUBCLASS_BOOT)) + break; + iface = (void*)iface + iface->bLength; + } // Set the configuration. ret = set_configuration(usbdev->defpipe, config->bConfigurationValue); if (ret) goto fail; + if (iface->bAlternateSetting) + // XXX + set_interface(iface); + // Configure driver. usbdev->config = config; usbdev->iface = iface; _______________________________________________ SeaBIOS mailing list -- seabios@seabios.org To unsubscribe send an email to seabios-le...@seabios.org