Re: PROBLEM: Mouse connected to USB-3 stopped working 2.6.38->39 regression

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, May 13, 2011 at 09:35:11AM -0700, Sarah Sharp wrote:
> Hi Thomas,
> 
> I'd like to figure out why your mouse doesn't work under the USB 3.0
> port.
> 
> You said this commit caused your issue:
> 
> commit 926008c9386dde09b015753b6681c502177baa30
> Author: Dmitry Torokhov <dtor@xxxxxxxxxx>
> Date:   Wed Mar 23 20:47:05 2011 -0700
> 
>     USB: xhci: simplify logic of skipping missed isoc TDs

Ok, I checked with some full speed devices I have, and I also see your
problem on my system with an NEC add-in card.  This commit causes the
issue:

commit dfa49c4ad120a784ef1ff0717168aa79f55a483a
Author: Dmitry Torokhov <dtor@xxxxxxxxxx>
Date:   Wed Mar 23 22:41:23 2011 -0700

    USB: xhci - fix math in xhci_get_endpoint_interval()
    
    When parsing exponent-expressed intervals we subtract 1 from the
    value and then expect it to match with original + 1, which is
    highly unlikely, and we end with frequent spew:
    
        usb 3-4: ep 0x83 - rounding interval to 512 microframes
    
    Also, parsing interval for fullspeed isochronous endpoints was
    incorrect - according to USB spec they use exponent-based
    intervals (but xHCI spec claims frame-based intervals). I trust
    USB spec more, especially since USB core agrees with it.

    This should be queued for stable kernels back to 2.6.31.
    
    Reviewed-by: Micah Elizabeth Scott <micah@xxxxxxxxxx>
    Signed-off-by: Dmitry Torokhov <dtor@xxxxxxxxxx>
    Signed-off-by: Sarah Sharp <sarah.a.sharp@xxxxxxxxxxxxxxx>
    Cc: stable@xxxxxxxxxx


Greg, this was marked as being needed to be queued for stable.  Do you
know if it has already made it into the stable trees?  We might need to
revert it or make a bug fix patch on top of it.


On the current Linus tree, both a FS hub and a FS USB to ethernet device
do not work under an NEC 0.96 host controller.  When I revert the patch,
they do work.

Dmitry, I'm pretty sure your patch isn't correct, now that I look at it
more closely.  The USB 2.0 spec says in table 9-13 about bInterval:

"Expressed in frames or microframes depending on the device operating
speed (i.e., either 1 millisecond or 125 Îs units)."

I assume that means frames for LS/FS devices and microframes for HS
devices.

"For full-/high-speed isochronous endpoints, this value must be in the
range from 1 to 16. The bInterval value is used as the exponent for a 2
value; e.g., a bInterval of 4 means a period of 8 = 2^(4-1)."

"For full-/low-speed interrupt endpoints, the value of this field may be
from 1 to 255."

But your patch thinks full speed interrupt endpoints should encoded in
exponent style:

        case USB_SPEED_FULL:
+               if (usb_endpoint_xfer_int(&ep->desc)) {
+                       interval = xhci_parse_exponent_interval(udev, ep);
+                       break;
+               }
+               /*
+                * Fall through for isochronous endpoint interval decoding
+                * since it uses the same rules as low speed interrupt
+                * endpoints.
+                */
+
        case USB_SPEED_LOW:
                if (usb_endpoint_xfer_int(&ep->desc) ||
-                               usb_endpoint_xfer_isoc(&ep->desc)) {
-                       interval = fls(8*ep->desc.bInterval) - 1;
-                       if (interval > 10)
-                               interval = 10;
-                       if (interval < 3)
-                               interval = 3;
-                       if ((1 << interval) != 8*ep->desc.bInterval)
-                               dev_warn(&udev->dev,
-                                               "ep %#x - rounding interval"
-                                               " to %d microframes, "
-                                               "ep desc says %d microframes\n",
-                                               ep->desc.bEndpointAddress,
-                                               1 << interval,
-                                               8*ep->desc.bInterval);
+                   usb_endpoint_xfer_isoc(&ep->desc)) {
+
+                       interval = xhci_parse_frame_interval(udev, ep);
                }


Note xfer_int, not xfer_isoc in that first if statement.  The comment is
also wrong, since full speed isochronous endpoints use the same interval
decoding as high speed isochronous endpoints.  I think you meant that
full speed *interrupt* endpoints use the same rules as low speed
interrupt endpoints.

Greg, what do you want to do in this case?  Revert Dmitry's patch from
Linus' tree and have him try again?  Make a bug fix patch on top of
Dmitry's patch?  The worst that can happen if Dmitry's patch is reverted
is that there will be more messages in dmesg about interval mismatch,
and full speed isochronous endpoints will be polled less frequently than
they should be.

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


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux