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...) > Signed-off-by: Felipe Balbi <[email protected]> > --- > > Tested against dwc3. Here's tracepoints showing values changing when > connecting straight to roothub port or through one USB3 hub: > > irq/13-dwc3-1577 [000] d..1 497.671262: dwc3_ctrl_req: Set Isochronous > Delay(Delay = 40 ns) > irq/13-dwc3-1577 [000] d..1 766.912097: dwc3_ctrl_req: Set Isochronous > Delay(Delay = 108 ns) > > drivers/usb/core/hub.c | 24 ++++++++++++++++++++++++ > drivers/usb/core/message.c | 24 ++++++++++++++++++++++++ > drivers/usb/core/usb.h | 1 + > include/linux/usb.h | 6 ++++++ > 4 files changed, 55 insertions(+) > > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c > index e9ce6bb0b22d..7feef558f788 100644 > --- a/drivers/usb/core/hub.c > +++ b/drivers/usb/core/hub.c > @@ -38,6 +38,9 @@ > #define USB_VENDOR_GENESYS_LOGIC 0x05e3 > #define HUB_QUIRK_CHECK_PORT_AUTOSUSPEND 0x01 > > +#define USB_TP_TRANSMISSION_DELAY 40 /* ns */ > +#define USB_TP_TRANSMISSION_DELAY_MAX 65535 /* ns */ > + > /* Protect struct usb_device->state and ->children members > * Note: Both are also protected by ->dev.sem, except that ->state can > * change to USB_STATE_NOTATTACHED even when the semaphore isn't held. */ > @@ -1352,6 +1355,20 @@ static int hub_configure(struct usb_hub *hub, > goto fail; > } > > + /* > + * Accumulate wHubDelay + 40ns for every hub in the tree of devices. > + * The resulting value will be used for SetIsochDelay() request. > + */ > + if (hub_is_superspeed(hdev) || hub_is_superspeedplus(hdev)) { > + u32 delay = __le16_to_cpu(hub->descriptor->u.ss.wHubDelay); > + > + if (hdev->parent) > + delay += hdev->parent->hub_delay; > + > + delay += USB_TP_TRANSMISSION_DELAY; > + hdev->hub_delay = min_t(u32, delay, > USB_TP_TRANSMISSION_DELAY_MAX); > + } > + > maxchild = hub->descriptor->bNbrPorts; > dev_info(hub_dev, "%d port%s detected\n", maxchild, > (maxchild == 1) ? "" : "s"); > @@ -4586,7 +4603,14 @@ hub_port_init(struct usb_hub *hub, struct usb_device > *udev, int port1, > if (retval >= 0) > retval = -EMSGSIZE; > } else { > + u32 delay; > + > retval = 0; > + > + delay = udev->parent->hub_delay; > + udev->hub_delay = min_t(u32, delay, > + USB_TP_TRANSMISSION_DELAY_MAX); > + usb_set_isoch_delay(udev); > break; > } > } > 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 this request going to have problems? > + */ > +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? thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
