On Tue, May 31, 2011 at 12:08:14PM -0700, Dmitry Torokhov wrote: > On Tue, May 31, 2011 at 11:34:58AM -0700, Sarah Sharp wrote: > > On Tue, May 31, 2011 at 09:49:41AM -0700, Dmitry Torokhov wrote: > > > Hi Matt, > > > > > > On Tue, May 31, 2011 at 03:52:16PM +1000, Matt Evans wrote: > > > > Commit dfa49c4ad120a784ef1ff0717168aa79f55a483a > > > > "USB: xhci - fix math in xhci_get_endpoint_interval()" > > > > expands the clamping of the interval value to 0-15, which is only legal > > > > for HS/SS endpoints. For FS, it must be 3-11. When testing with FS > > > > isochronous endpoints (an audio device), values outside this range caused > > > > the following error 0x11 (again) on an NEC xHCI controller: > > > > > > > > xhci_hcd 0000:01:00.0: ERROR: unexpected command completion code 0x11. > > > > usb 1-1: Not enough bandwidth for altsetting 1 > > > > > > > > > > Hmm, however USB2.0 spec (to whcih high/full/low-speed devices should conform > > > states: > > > > > > "For full-/high-speed isochronous endpoints, this value must be in the > > > range from 1 to 16. The bInterval value bInterval-1 is used as the > > > exponent for a 2 value; e.g., a 4-1 bInterval of 4 means a period of 8 > > > (2 )." > > > > It doesn't really matter what the USB 2.0 specification says. If the > > xHCI specification says that xHCI hosts can only handle FS intervals of > > 3-11, and host controllers are going to reject Configure Endpoint > > commands based on what the spec says, then the xHCI driver needs to > > limit the interval in the same way. > > > > It could be that the xHCI spec architect chose to limit the FS intervals > > for a reason, much the same way they chose to limit the interval to > > powers of two. Or it could have just been a typo. > > > > If the xHCI spec is wrong, then you should email xhcispec@xxxxxxxxx and > > try to get an errata put in for it. But even then, we won't be able to > > tell which hosts implement the errata, so we'll probably have to stick > > with the lower interval limit. > > xHCI spec seems to be confused indeed. Compare: > > Table 56: > "The legal range of values is 3 to 10 for Full- or Low-speed Interrupt > endpoints (1 to 256 ms.) and 0 to 15 for all other endpoint types (125 > us to 4096 us)." > > This suggests that full- and low-speed interrupt endpoints specify > interval in exponent-based form. > > However 6.2.3.6 states: > > For low-speed Interrupt and full-speed Interrupt and Isoch endpoints the > bInterval field declared by a Full-or Low-speed device is computed as > bInterval * 1ms., where bInterval = 1 to 255." My updated version of the 1.0 spec (with all the errata up to Jan. 11, 2011) has a nice table in that section instead. I'm just going to attach a screenshot of it. Basically, it says to clamp FS devices at 10, with the exception of FS isochronous devices, which can go up to 18. Of course, it's possible that the NEC host controller is doing some internal checking of the interval, and won't accept FS isochronous intervals greater than 10. I guess we would need to clamp to 18 only for xHCI 1.0 hosts. > This suggests that for these endpoints devices should use frame-based > interval encoding and this is consistent with USB2.0 specs. > Incidentially if you convert frame-based to exponent interval you'll get > 3-11 (or more precisely 3-10) range. > > I think Matt might be stubling onto the bug I introduced (and you fixed) > when we used wrong encoding for full speed interrupt endpoints. No, Matt told me (off list) that he was working on the 3.0 kernel. That bug should be fixed there, even if it isn't fixed in the 2.6.39 stable tree yet. Sarah Sharp
Attachment:
xhci-interval-as-of-2011-01-17.jpeg
Description: JPEG image
