lkml.org 
[lkml]   [2017]   [Mar]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] usb-core: Add MS_INTR_BINTERVAL USB quirk
On Sun, 12 Mar 2017, Samuel Thibault wrote:

> Some USB 2.0 devices erroneously report millisecond values in
> bInterval. The generic config code manages to catch most of them,
> but in some cases it's not completely enough.
>
> The case at stake here is a USB 2.0 braille device, which wants to
> announce 10ms and thus sets bInterval to 10, but with the USB 2.0
> computation that yields to 64ms. It happens that one can type fast
> enough to reach this interval and get the device buffers overflown,
> leading to problematic latencies. The generic config code does not
> catch this case because the 64ms is considered a sane enough value.

Interesting. This is a high-speed device that mistakenly uses the
low/full-speed encoding for an interrupt bInterval value?

That's pretty unusual. Most HID devices (which includes the Braille
devices I have heard of) run at low speed, and a few of them run at
full speed. I can't remember any running at high speed.

> This change thus adds a USB_QUIRK_MS_INTR_BINTERVAL quirk to mark
> devices which actually report milliseconds in bInterval, and marks
> Vario Ultra devices as needing it.
>
> Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
>
> --- a/include/linux/usb/quirks.h
> +++ b/include/linux/usb/quirks.h
> @@ -50,4 +50,10 @@
> /* device can't handle Link Power Management */
> #define USB_QUIRK_NO_LPM BIT(10)
>
> +/*
> + * Device reports its bInterval as milliseconds instead of the

This should be described as "linear frames", not "milliseconds", and
its name should be USB_QUIRK_LINEAR_FRAME_INTR_BINTERVAL.

> + * USB 2.0 calculation.
> + */
> +#define USB_QUIRK_MS_INTR_BINTERVAL BIT(11)
> +
> #endif /* __LINUX_USB_QUIRKS_H */
> --- a/drivers/usb/core/config.c
> +++ b/drivers/usb/core/config.c
> @@ -280,6 +280,14 @@ static int usb_parse_endpoint(struct dev
>
> /*
> * Adjust bInterval for quirked devices.
> + */
> + /*
> + * This quirk fixes bIntervals reported in ms.
> + */
> + if (to_usb_device(ddev)->quirks &
> + USB_QUIRK_MS_INTR_BINTERVAL)
> + i = j = n;

You want to use the bInterval value the device intended to provide, not
a default value. This should be like the microframe case, with the
value increased by 3:

n = clamp(fls(d->bInterval) + 3, i, j);
i = j = n;

> + /*
> * This quirk fixes bIntervals reported in
> * linear microframes.
> */
> --- a/drivers/usb/core/quirks.c
> +++ b/drivers/usb/core/quirks.c
> @@ -218,6 +218,14 @@ static const struct usb_device_id usb_qu
> /* INTEL VALUE SSD */
> { USB_DEVICE(0x8086, 0xf1a5), .driver_info = USB_QUIRK_RESET_RESUME },
>
> + /* Baum Vario Ultra */
> + { USB_DEVICE(0x0904, 0x6101), .driver_info =
> + USB_QUIRK_MS_INTR_BINTERVAL },
> + { USB_DEVICE(0x0904, 0x6102), .driver_info =
> + USB_QUIRK_MS_INTR_BINTERVAL },
> + { USB_DEVICE(0x0904, 0x6103), .driver_info =
> + USB_QUIRK_MS_INTR_BINTERVAL },

You didn't read the comment at the start of the file. :-) This list
is supposed to remain sorted by vendor and product ID.

Alan Stern

> +
> { } /* terminating entry must be last */
> };
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>

\
 
 \ /
  Last update: 2017-03-12 22:19    [W:0.056 / U:0.024 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site