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 <felipe.balbi@xxxxxxxxxxxxxxx>
> ---
>
> 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
