Hi, Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> writes: > On Thu, Nov 09, 2017 at 03:41:54PM +0200, Felipe Balbi wrote: >> USB SS and SSP hubs provide wHubDelay values on their hub descriptor >> which we should inform the USB Device about. >> >> The USB Specification 3.0 explains, on section 9.4.11, how to >> calculate the value and how to issue the request. Note that a >> USB_REQ_SET_ISOCH_DELAY is valid on all device states (Default, >> Address, Configured), we just *chose* to issue it from Address state >> right after successfully fetching the USB Device Descriptor. > > I missed that this was in the 3.0 spec, I had heard about it a long time > ago. Do we have any devices out there that actually set this value? Or > on the other hand, can anyone use it yet (I know audio wanted it...) Well, USB 3.0 devices are required to accept this request, so my comment that it's "optional", isn't entirely correct. It's part of Chapter9 testing on USBCV. Those failing this test, wouldn't get a USB sticker. DWC3, for example, receives the number, but can't do much about it so far. We could, eventually, pass it to gadget drivers and let them use the value to figure out how many requests they should queue ahead of time for isoc endpoints. >> diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c >> index 371a07d874a3..c1db751163d5 100644 >> --- a/drivers/usb/core/message.c >> +++ b/drivers/usb/core/message.c >> @@ -915,6 +915,30 @@ int usb_get_device_descriptor(struct usb_device *dev, unsigned int size) >> return ret; >> } >> >> +/* >> + * usb_set_isoch_delay - informs the device of the packet transmit delay >> + * @dev: the device whose delay is to be informed >> + * Context: !in_interrupt() >> + * >> + * Since this is an optional request, we don't bother if it fails. > > But we should return an error, right? Are devices that don't expect Right, I'm not sure :-) Do we care if we get a stall on this one? > this request going to have problems? No idea. I assume we won't. I can add the error and handle it from the calling site. >> + */ >> +void usb_set_isoch_delay(struct usb_device *dev) >> +{ >> + /* skip hub devices */ >> + if (dev->descriptor.bDeviceClass == USB_CLASS_HUB) >> + return; >> + >> + /* skip non-SS/non-SSP devices */ >> + if (dev->speed < USB_SPEED_SUPER) >> + return; >> + >> + (void) usb_control_msg(dev, usb_sndctrlpipe(dev, 0), >> + USB_REQ_SET_ISOCH_DELAY, >> + USB_DIR_OUT | USB_TYPE_STANDARD | USB_RECIP_DEVICE, >> + cpu_to_le16(dev->hub_delay), 0, NULL, 0, >> + USB_CTRL_SET_TIMEOUT); >> +} > > No need to export this yet, but we probably will, right? you mean EXPORT_SYMBOL_GPL()? I don't think we need. The value won't change without reenumeration, at which point we will recalculate the value. -- balbi -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html