diff mbox

[PRESTABLE] xhci: Fix full speed bInterval encoding.

Message ID 1307248992.14280.137.camel@xps-1
State New
Headers show

Commit Message

Steve Conklin June 5, 2011, 4:43 a.m. UTC 
Note, this is a notification of what I've already applied,
with justification for why I did it.

There was a patch that came down in several upstream
stable kernels which broke USB3 for some uses. That
patch was:

USB: xhci - fix math in xhci_get_endpoint_interval()

This patch was applied to Natty, Maverick, and Lucid
as part of recent stable upstream patch sets.

It was later discovered to have a problem:

https://patchwork.kernel.org/patch/837082/

This new patch to fix the problem is:

xhci: Fix full speed bInterval encoding

This patch is in Greg's stable queue for every kernel
except 2.6.35 (Maverick). This is pretty obviously an
oversight. I've emailed Sarah Sharp and I'll follow up
to make sure it is submitted.

Despite the lack of it being in 2.6.35 stable and not
being released in a stable update yet, I decided to treat
it as a prestable patch. It's attached and is very
straightforward.

We're currently prepping for a kernel cycle that I hope
will have new kernels in -proposed within a few days.

This is being tracked in this bug:
https://bugs.launchpad.net/ubuntu/+source/linux/+bug/792959

Here is the history and status for each of our affected
kernels:

=== Natty ===

During -proposed testing of Natty during the last cycle,
the problem was noted, and because we were unable to have
the reporter help bisect the kernel, I reverted the three
xhci patches from Natty which had come in the stable update,
and respun it. This is the kernel that is on deck for the
current cycle. The Natty kernel currently in -proposed will
not be published to -updates.

In the Natty master-next branch, I have reapplied those
three patches, along with the fix patch.

=== Maverick ===

I added the fixer patch to the Maverick kernel which we
had already prepared for release and respun it. The
patch required manual help due to a context
difference, but was trivial.

=== Lucid ===

I added the fixer patch to the Lucid kernel which we
had already prepared for release, and respun it.

--
The fixer was not an ABI bump, so none of the
ABI-dependent packages needed to be respun.

=== Still to be done ===

1) The Lucid ec2 branch which is based off of the Lucid
   master branch will need to be redone.

2) The Lucid lts-backports-maverick branch will have to
   be redone.

3) The Lucid lts-backports-natty branch will have to
   be redone. (apw, talk to me about this Monday)

Everything is tagged and pushed.

If anyone detects any problems with this, speak up soon.

Steve

Comments

Tim Gardner June 6, 2011, 1:27 p.m. UTC | #1 
On 06/04/2011 10:43 PM, Steve Conklin wrote:
> Note, this is a notification of what I've already applied,
> with justification for why I did it.
>
> There was a patch that came down in several upstream
> stable kernels which broke USB3 for some uses. That
> patch was:
>
> USB: xhci - fix math in xhci_get_endpoint_interval()
>
> This patch was applied to Natty, Maverick, and Lucid
> as part of recent stable upstream patch sets.
>
> It was later discovered to have a problem:
>
> https://patchwork.kernel.org/patch/837082/
>
> This new patch to fix the problem is:
>
> xhci: Fix full speed bInterval encoding
>
> This patch is in Greg's stable queue for every kernel
> except 2.6.35 (Maverick). This is pretty obviously an
> oversight. I've emailed Sarah Sharp and I'll follow up
> to make sure it is submitted.
>
> Despite the lack of it being in 2.6.35 stable and not
> being released in a stable update yet, I decided to treat
> it as a prestable patch. It's attached and is very
> straightforward.
>
> We're currently prepping for a kernel cycle that I hope
> will have new kernels in -proposed within a few days.
>
> This is being tracked in this bug:
> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/792959
>
> Here is the history and status for each of our affected
> kernels:
>
> === Natty ===
>
> During -proposed testing of Natty during the last cycle,
> the problem was noted, and because we were unable to have
> the reporter help bisect the kernel, I reverted the three
> xhci patches from Natty which had come in the stable update,
> and respun it. This is the kernel that is on deck for the
> current cycle. The Natty kernel currently in -proposed will
> not be published to -updates.
>
> In the Natty master-next branch, I have reapplied those
> three patches, along with the fix patch.
>
> === Maverick ===
>
> I added the fixer patch to the Maverick kernel which we
> had already prepared for release and respun it. The
> patch required manual help due to a context
> difference, but was trivial.
>
> === Lucid ===
>
> I added the fixer patch to the Lucid kernel which we
> had already prepared for release, and respun it.
>
> --
> The fixer was not an ABI bump, so none of the
> ABI-dependent packages needed to be respun.
>
> === Still to be done ===
>
> 1) The Lucid ec2 branch which is based off of the Lucid
>     master branch will need to be redone.
>
> 2) The Lucid lts-backports-maverick branch will have to
>     be redone.
>
> 3) The Lucid lts-backports-natty branch will have to
>     be redone. (apw, talk to me about this Monday)
>
> Everything is tagged and pushed.
>
> If anyone detects any problems with this, speak up soon.
>
> Steve
>
>
>

Acked-by: Tim Gardner <tim.gardner@canonical.com>
diff mbox

Patch

From b8f653f794ec459132a903c7b7d6aa40bbe5c2a1 Mon Sep 17 00:00:00 2001
From: Sarah Sharp <sarah.a.sharp@linux.intel.com>
Date: Fri, 13 May 2011 13:10:01 -0700
Subject: [PRESTABLE][PATCH] xhci: Fix full speed bInterval encoding.

BugLink: http://bugs.launchpad.net/bugs/792959

commit b513d44751bfb609a3c20463f764c8ce822d63e9 upstream.

Dmitry's patch

dfa49c4ad120a784ef1ff0717168aa79f55a483a USB: xhci - fix math in xhci_get_endpoint_interval()

introduced a bug.  The USB 2.0 spec says that full speed isochronous endpoints'
bInterval must be decoded as an exponent to a power of two (e.g. interval =
2^(bInterval - 1)).  Full speed interrupt endpoints, on the other hand, don't
use exponents, and the interval in frames is encoded straight into bInterval.

Dmitry's patch was supposed to fix up the full speed isochronous to parse
bInterval as an exponent, but instead it changed the *interrupt* endpoint
bInterval decoding.  The isochronous endpoint encoding was the same.

This caused full speed devices with interrupt endpoints (including mice, hubs,
and USB to ethernet devices) to fail under NEC 0.96 xHCI host controllers:

[  100.909818] xhci_hcd 0000:06:00.0: add ep 0x83, slot id 1, new drop flags = 0x0, new add flags = 0x99, new slot info = 0x38100000
[  100.909821] xhci_hcd 0000:06:00.0: xhci_check_bandwidth called for udev ffff88011f0ea000
...
[  100.910187] xhci_hcd 0000:06:00.0: ERROR: unexpected command completion code 0x11.
[  100.910190] xhci_hcd 0000:06:00.0: xhci_reset_bandwidth called for udev ffff88011f0ea000

When the interrupt endpoint was added and a Configure Endpoint command was
issued to the host, the host controller would return a very odd error message
(0x11 means "Slot Not Enabled", which isn't true because the slot was enabled).
Probably the host controller was getting very confused with the bad encoding.

Signed-off-by: Sarah Sharp <sarah.a.sharp@linux.intel.com>
Cc: Dmitry Torokhov <dtor@vmware.com>
Reported-by: Thomas Lindroth <thomas.lindroth@gmail.com>
Tested-by: Thomas Lindroth <thomas.lindroth@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
Signed-off-by: Steve Conklin <sconklin@canonical.com>
---
 drivers/usb/host/xhci-mem.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 0de3100..2172998 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -992,12 +992,12 @@  static inline unsigned int xhci_get_endpoint_interval(struct usb_device *udev,
 		break;
 
 	case USB_SPEED_FULL:
-		if (usb_endpoint_xfer_int(&ep->desc)) {
+		if (usb_endpoint_xfer_isoc(&ep->desc)) {
 			interval = xhci_parse_exponent_interval(udev, ep);
 			break;
 		}
 		/*
-		 * Fall through for isochronous endpoint interval decoding
+		 * Fall through for interrupt endpoint interval decoding
 		 * since it uses the same rules as low speed interrupt
 		 * endpoints.
 		 */
-- 
1.7.0.4