Mathias Nyman <mathias.nyman@...> writes:
>
> On 09/15/2014 10:50 PM, Alan Stern wrote:
> > On Mon, 15 Sep 2014, Mathias Nyman wrote:
> >
> >> I haven't had time to dig into the usbmon traces, but I just got
another report from Gunter K�ningsmann
> about similar scanner problem, and I just noticed
> >> that in both cases we round the interval for high speed bulk _IN_
endpoint, which should not have interval
> set at all.
> >> The interval value should be ignored by host, but maybe setting it
still could cause some issues.
> >>
> >> I don't have high hopes for this patch, but it's worth a shot.
> >> Can you try this out and see if it makes any difference?
> >>
> >> -Mathias
> >>
> >> diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
> >> index 8936211..7f21b77 100644
> >> --- a/drivers/usb/host/xhci-mem.c
> >> +++ b/drivers/usb/host/xhci-mem.c
> >> <at> <at> -1291,7 +1291,8 <at> <at> static unsigned int
xhci_get_endpoint_interval(struct usb_device *udev,
> >> /* Max NAK rate */
> >> if (usb_endpoint_xfer_control(&ep->desc) ||
> >> usb_endpoint_xfer_bulk(&ep->desc)) {
> >> - interval = xhci_parse_microframe_interval(udev,
ep);
> >> + if (!usb_endpoint_dir_in(&ep->desc))
> >> + interval =
xhci_parse_microframe_interval(udev, ep);
> >
> > It's not a good idea to test the direction of control endpoints, since
> > they are bi-directional.
>
> Thats right, thanks, I only wanted the bulk endpoints checked here, I need
to change that.
>
> >
> > Also, xhci_parse_microframe_interval assumes the value is stored with
> > an exponential encoding, but the NAK rate value for control and
> > bulk-OUT endpoints is stored in the endpoint descriptor directly as a
> > number of microframes (not exponential).
> >
> > It's not fully clear how xHCI controllers use the Interval field in the
> > Endpoint Context. Table 65 notably fails to include a line for HS
> > bulk-OUT/control endpoints, and the text isn't clear as to whether the
> > field is interpreted using exponential coding for non-periodic
> > endpoints. As near as I can tell, it is -- which means you should call
> > xhci_microframes_to_exponent() here, not
> > xhci_parse_microframe_interval().
> >
> > Alan Stern
> >
>
> I think this is correct now as it is, assuming xhci Interval field for HS
bulk-OUT/control endpoint context
> uses the same exponential coding as for all other endpoints.
>
> flow goes:
> if (HS bulk or control ep) //endpoints bInterval is
in number of microframes here,
> xhci_parse_microframe_interval()
> xhci_microframes_to_exponent(bInterval, ...)
> interval = fls(bInterval) - 1; // interval is now in
exponent form
Any progress on this one?
Quite a bunch of people, that try to use a scanner on a xhci port gets
bitten by this issue. Given, that modern haswell mainboards tend to only
support xhci ports anymore, this is a real showstopper and kind of
regression from users POV.
https://bugzilla.novell.com/show_bug.cgi?id=856794
https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1102797
Would be nice to cc me on answers in order to relay results to other
suffering people.
Thanks in advance,
Pete
