On 01/06/11 07:00, Sarah Sharp wrote: > 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. Indeed, this is on top of your fix-- I was finding an isochronous endpoint (for my USB audio device) was not working. For this type of endpoint, the NEC controller will not accept intervals < 3 (responding with the 0x11 completion code). Cheers, Matt -- 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
