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. > > This commit makes xhci_parse_exponent_interval() check the EP speed and > > clamp the interval appropriately. Also, xhci_parse_frame_interval() clamped > > interval to 3-10; this has been changed to 3-11, as is valid for LS/HS. > > > > With this patch, full-speed isoch devices (e.g. audio) work again on xHCI. > > > > This should be queued for all stable kernels that contain > > dfa49c4ad120a784ef1ff0717168aa79f55a483a > > > > Signed-off-by: Matt Evans <matt@xxxxxxxxxx> > > --- > > > > A review of the xhci_parse_frame_interval() change (max 10 -> 11) would be > > appreciated. The value 10 comes from Sarah's original xhci_get_endpoint_interval() > > implementation; was there a reason for this being 10 rather than (as per the spec) > > 11? > > The allowed interval is 1-255 ms or 8-1024 uframes, which gives 3-10 > exponent interval. I originally implemented the max interval for FS devices as 10 because the version of the xHCI 0.96 spec I have says to. Table 56 says that "The legal range of values is 3 to 10 for Full- or Low-speed Interrupt endpoints (1 to 256 ms.)" I think maybe that got changed in an errata to the 0.96 spec, but I'm not sure. The xHCI 1.0 spec says something different in the same table "The legal range of values is 3 to 11 for Full- or Low-speed Interrupt endpoints (1 to 256 ms.)" So maybe we need to have a different clamping interval based on whether the host is 0.96 or 1.0 based? You can check xhci->hci_version, and clamp it to 10 for hosts with xhci->hci_version < 0x100, and clamp it to 11 otherwise. Sarah Sharp -- 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
