On Friday 08 June 2007, Alan Stern wrote: > This patch (as904) adds code to check for endpoint descriptor bInterval > values outside the legal limits. Illegal values are set to 32 ms, which > seems like a reasonable default. > > This fixes Bugzilla #8432. > > Signed-off-by: Alan Stern <[EMAIL PROTECTED]>
>From my experience, a common cause of bogus bInterval values is developers forgetting that the value has a different meaning for high-speed and low-speed/full-speed devices. I've come across a device reporting interrupt bInterval set to 32 instead of 9. By sheer luck your patch fixes this properly, but it would set a wrong bInterval value for devices setting bInterval to 16 instead of 8 for instance. The USB developers FAQ (available at http://www.lvr.com/usbfaq.htm) states that "Reports are that many device manufacturers are using full-speed bInterval values in high-speed descriptors. Reports also say that under Windows, a device may receive its intended polling rate (rather than its requested polling rate) in spite of this error. Other operating systems are likely to attempt to provide the requested polling rate, or may report an illegal bInterval value." For high-speed interrupt endpoints, wouldn't it be better to set bInterval to fls(bInterval*8) if the requested value is higher than 16 ? Laurent Pinchart > Index: usb-2.6/drivers/usb/core/config.c > =================================================================== > --- usb-2.6.orig/drivers/usb/core/config.c > +++ usb-2.6/drivers/usb/core/config.c > @@ -1,4 +1,5 @@ > #include <linux/usb.h> > +#include <linux/usb/ch9.h> > #include <linux/module.h> > #include <linux/init.h> > #include <linux/slab.h> > @@ -49,7 +50,7 @@ static int usb_parse_endpoint(struct dev > unsigned char *buffer0 = buffer; > struct usb_endpoint_descriptor *d; > struct usb_host_endpoint *endpoint; > - int n, i; > + int n, i, j; > > d = (struct usb_endpoint_descriptor *) buffer; > buffer += d->bLength; > @@ -84,6 +85,45 @@ static int usb_parse_endpoint(struct dev > memcpy(&endpoint->desc, d, n); > INIT_LIST_HEAD(&endpoint->urb_list); > > + /* If the bInterval value is outside the legal range, > + * set it to a default value: 32 ms */ > + i = 0; /* i = min, j = max, n = default */ > + j = 255; > + if (usb_endpoint_xfer_int(d)) { > + i = 1; > + switch (to_usb_device(ddev)->speed) { > + case USB_SPEED_HIGH: > + n = 9; /* 32 ms = 2^(9-1) uframes */ > + j = 16; > + break; > + default: /* USB_SPEED_FULL or _LOW */ > + /* For low-speed, 10 ms is the official minimum. > + * But some "overclocked" devices might want faster > + * polling so we'll allow it. */ > + n = 32; > + break; > + } > + } else if (usb_endpoint_xfer_isoc(d)) { > + i = 1; > + j = 16; > + switch (to_usb_device(ddev)->speed) { > + case USB_SPEED_HIGH: > + n = 9; /* 32 ms = 2^(9-1) uframes */ > + break; > + default: /* USB_SPEED_FULL */ > + n = 6; /* 32 ms = 2^(6-1) frames */ > + break; > + } > + } > + if (d->bInterval < i || d->bInterval > j) { > + dev_warn(ddev, "config %d interface %d altsetting %d " > + "endpoint 0x%X has an invalid bInterval %d, " > + "changing to %d\n", > + cfgno, inum, asnum, > + d->bEndpointAddress, d->bInterval, n); > + endpoint->desc.bInterval = n; > + } > + > /* Skip over any Class Specific or Vendor Specific descriptors; > * find the next endpoint or interface descriptor */ > endpoint->extra = buffer; ------------------------------------------------------------------------- This SF.net email is sponsored by DB2 Express Download DB2 Express C - the FREE version of DB2 express and take control of your XML. No limits. Just data. Click to get it now. http://sourceforge.net/powerbar/db2/ _______________________________________________ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
