On Sat, 26 Jul 2014, James P Michels III wrote:

> This patch adds a usb quirk to support devices with bInterval values
> expressed as microframes. The quirk causes the parse endpoint function
> to modify the reported bInterval to a standards conforming value.

You need to mention that the quirk applies to interrupt endpoints, not 
isochronous endpoints.

> There is currently code in the endpoint parser that checks for
> bIntervals that are outside of the valid range (1-16 for USB 2+).

1-16 is the valid range for high speed and SuperSpeed.  USB-2 also
includes low speed and full speed, for which the valid range is 1-255.  
See Table 9-13 in the USB-2.0 spec.

> In this case, the code assumes the bInterval is being reported in
> 1ms frames. As well, the correction is only applied if the original
> bInterval value is out of the 1-16 range.
> 
> With this quirk applied to the device, the bInterval will be
> accurately adjusted from microframes to an exponent.
> 
> Signed-off-by: James P Michels III <james.p.mich...@gmail.com>
> ---
>  drivers/usb/core/config.c  | 9 +++++++++
>  drivers/usb/core/quirks.c  | 4 ++++
>  include/linux/usb/quirks.h | 9 +++++++++
>  3 files changed, 22 insertions(+)
> 
> diff --git a/drivers/usb/core/config.c b/drivers/usb/core/config.c
> index 1ab4df1..191ce69 100644
> --- a/drivers/usb/core/config.c
> +++ b/drivers/usb/core/config.c
> @@ -199,6 +199,15 @@ static int usb_parse_endpoint(struct device *ddev, int 
> cfgno, int inum,
>                       if (n == 0)
>                               n = 9;  /* 32 ms = 2^(9-1) uframes */
>                       j = 16;
> +
> +                     /* Adjust bInterval for quirked devices.
> +                      * This quirk fixes bIntervals reported in
> +                      * linear microframes. */
> +                     if (to_usb_device(ddev)->quirks &
> +                             USB_QUIRK_LINEAR_UFRAME_INTR_BINTERVAL) {
> +                             n = clamp(fls(d->bInterval), i, j);
> +                             i = j = n;
> +                             }

The closing brace is in the wrong column.  Also, people are likely to 
complain about the style of the multi-line comment.  Although it's the 
same as the rest of the source file, nowadays we prefer all new 
comments to have the form:

                        /*
                         * Blah blah blah
                         * blah blah blah
                         */

> --- a/include/linux/usb/quirks.h
> +++ b/include/linux/usb/quirks.h
> @@ -30,4 +30,13 @@
>     descriptor */
>  #define USB_QUIRK_DELAY_INIT         0x00000040
>  
> +/* The USB 2.0 and USB 3.0 spec require the interval in microframes
> +   (1 microframe = 125 microseconds) to be calculated as
> +   interval = 2 ^ (bInterval -1).

Spacing around the minus sign is wrong.  And again, this applies only
to high-speed and SuperSpeed devices.  The quirk itself applies only to
interrupt endpoints.

> +
> +   Devices with this quirk report their bInterval as the result of
> +   this calculation instead of the exponent variable used in the
> +   calculation */

Wrong style in this multi-line comment.

> +#define USB_QUIRK_LINEAR_UFRAME_INTR_BINTERVAL       0x00000080
> +
>  #endif /* __LINUX_USB_QUIRKS_H */

Aside from those relatively minor issues, this is okay.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to