From 22ceac191211cf6688b1bf6ecd93c8b6bf80ed9b Mon Sep 17 00:00:00 2001 From: Sarah Sharp Date: Mon, 23 Jul 2012 16:06:08 -0700 Subject: xhci: Increase reset timeout for Renesas 720201 host. The NEC/Renesas 720201 xHCI host controller does not complete its reset within 250 milliseconds. In fact, it takes about 9 seconds to reset the host controller, and 1 second for the host to be ready for doorbell rings. Extend the reset and CNR polling timeout to 10 seconds each. This patch should be backported to kernels as old as 2.6.31, that contain the commit 66d4eadd8d067269ea8fead1a50fe87c2979a80d "USB: xhci: BIOS handoff and HW initialization." Signed-off-by: Sarah Sharp Reported-by: Edwin Klein Mentink Cc: stable@vger.kernel.org --- drivers/usb/host/xhci.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index 7648b2d4b268..5c3a3f75e4d5 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -166,7 +166,7 @@ int xhci_reset(struct xhci_hcd *xhci) xhci_writel(xhci, command, &xhci->op_regs->command); ret = handshake(xhci, &xhci->op_regs->command, - CMD_RESET, 0, 250 * 1000); + CMD_RESET, 0, 10 * 1000 * 1000); if (ret) return ret; @@ -175,7 +175,8 @@ int xhci_reset(struct xhci_hcd *xhci) * xHCI cannot write to any doorbells or operational registers other * than status until the "Controller Not Ready" flag is cleared. */ - ret = handshake(xhci, &xhci->op_regs->status, STS_CNR, 0, 250 * 1000); + ret = handshake(xhci, &xhci->op_regs->status, + STS_CNR, 0, 10 * 1000 * 1000); for (i = 0; i < 2; ++i) { xhci->bus_state[i].port_c_suspend = 0; -- cgit v1.2.3 From 5cb7df2b2d3afee7638b3ef23a5bcb89c6f07bd9 Mon Sep 17 00:00:00 2001 From: Sarah Sharp Date: Mon, 2 Jul 2012 13:36:23 -0700 Subject: xhci: Add Etron XHCI_TRUST_TX_LENGTH quirk. Gary reports that with recent kernels, he notices more xHCI driver warnings: xhci_hcd 0000:03:00.0: WARN Successful completion on short TX: needs XHCI_TRUST_TX_LENGTH quirk? We think his Etron xHCI host controller may have the same buggy behavior as the Fresco Logic xHCI host. When a short transfer is received, the host will mark the transfer as successfully completed when it should be marking it with a short completion. Fix this by turning on the XHCI_TRUST_TX_LENGTH quirk when the Etron host is discovered. Note that Gary has revision 1, but if Etron fixes this bug in future revisions, the quirk will have no effect. This patch should be backported to kernels as old as 2.6.36, that contain a backported version of commit 1530bbc6272d9da1e39ef8e06190d42c13a02733 "xhci: Add new short TX quirk for Fresco Logic host." Signed-off-by: Sarah Sharp Reported-by: Gary E. Miller Cc: stable@vger.kernel.org --- drivers/usb/host/xhci-pci.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c index 18b231b0c5d3..92eaff6097c0 100644 --- a/drivers/usb/host/xhci-pci.c +++ b/drivers/usb/host/xhci-pci.c @@ -99,6 +99,7 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci) pdev->device == PCI_DEVICE_ID_ASROCK_P67) { xhci->quirks |= XHCI_RESET_ON_RESUME; xhci_dbg(xhci, "QUIRK: Resetting on resume\n"); + xhci->quirks |= XHCI_TRUST_TX_LENGTH; } if (pdev->vendor == PCI_VENDOR_ID_VIA) xhci->quirks |= XHCI_RESET_ON_RESUME; -- cgit v1.2.3 From 8202ce2e2921941098aa72894ac5d7b486a9d273 Mon Sep 17 00:00:00 2001 From: Sarah Sharp Date: Wed, 25 Jul 2012 10:52:45 -0700 Subject: xhci: Rate-limit XHCI_TRUST_TX_LENGTH quirk warning. When we encounter an xHCI host that needs the XHCI_TRUST_TX_LENGTH quirk, the xHCI driver ends up spewing messages about the quirk into dmesg every time a short packet occurs. Change the xHCI driver to rate-limit such warnings. Signed-off-by: Sarah Sharp Reported-by: Matthew Hall Reported-by: Gary E. Miller --- drivers/usb/host/xhci-ring.c | 4 ++-- drivers/usb/host/xhci.h | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 8275645889da..0c93f5dcea6f 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -2073,8 +2073,8 @@ static int handle_tx_event(struct xhci_hcd *xhci, if (xhci->quirks & XHCI_TRUST_TX_LENGTH) trb_comp_code = COMP_SHORT_TX; else - xhci_warn(xhci, "WARN Successful completion on short TX: " - "needs XHCI_TRUST_TX_LENGTH quirk?\n"); + xhci_warn_ratelimited(xhci, + "WARN Successful completion on short TX: needs XHCI_TRUST_TX_LENGTH quirk?\n"); case COMP_SHORT_TX: break; case COMP_STOP: diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h index 55c0785810c9..96f49dbb50ac 100644 --- a/drivers/usb/host/xhci.h +++ b/drivers/usb/host/xhci.h @@ -1537,6 +1537,8 @@ static inline struct usb_hcd *xhci_to_hcd(struct xhci_hcd *xhci) dev_err(xhci_to_hcd(xhci)->self.controller , fmt , ## args) #define xhci_warn(xhci, fmt, args...) \ dev_warn(xhci_to_hcd(xhci)->self.controller , fmt , ## args) +#define xhci_warn_ratelimited(xhci, fmt, args...) \ + dev_warn_ratelimited(xhci_to_hcd(xhci)->self.controller , fmt , ## args) /* TODO: copied from ehci.h - can be refactored? */ /* xHCI spec says all registers are little endian */ -- cgit v1.2.3 From 50d0206fcaea3e736f912fd5b00ec6233fb4ce44 Mon Sep 17 00:00:00 2001 From: Sarah Sharp Date: Thu, 26 Jul 2012 12:03:59 -0700 Subject: xhci: Fix bug after deq ptr set to link TRB. This patch fixes a particularly nasty bug that was revealed by the ring expansion patches. The bug has been present since the very beginning of the xHCI driver history, and could have caused general protection faults from bad memory accesses. The first thing to note is that a Set TR Dequeue Pointer command can move the dequeue pointer to a link TRB, if the canceled or stalled transfer TD ended just before a link TRB. The function to increment the dequeue pointer, inc_deq, was written before cancellation and stall support was added. It assumed that the dequeue pointer could never point to a link TRB. It would unconditionally increment the dequeue pointer at the start of the function, check if the pointer was now on a link TRB, and move it to the top of the next segment if so. This means that if a Set TR Dequeue Point command moved the dequeue pointer to a link TRB, a subsequent call to inc_deq() would move the pointer off the segment and into la-la-land. It would then read from that memory to determine if it was a link TRB. Other functions would often call inc_deq() until the dequeue pointer matched some other pointer, which means this function would quite happily read all of system memory before wrapping around to the right pointer value. Often, there would be another endpoint segment from a different ring allocated from the same DMA pool, which would be contiguous to the segment inc_deq just stepped off of. inc_deq would eventually find the link TRB in that segment, and blindly move the dequeue pointer back to the top of the correct ring segment. The only reason the original code worked at all is because there was only one ring segment. With the ring expansion patches, the dequeue pointer would eventually wrap into place, but the dequeue segment would be out-of-sync. On the second TD after the dequeue pointer was moved to a link TRB, trb_in_td() would fail (because the dequeue pointer and dequeue segment were out-of-sync), and this message would appear: ERROR Transfer event TRB DMA ptr not part of current TD This fixes bugzilla entry 4333 (option-based modem unhappy on USB 3.0 port: "Transfer event TRB DMA ptr not part of current TD", "rejecting I/O to offline device"), https://bugzilla.kernel.org/show_bug.cgi?id=43333 and possibly other general protection fault bugs as well. This patch should be backported to kernels as old as 2.6.31. A separate patch will be created for kernels older than 3.4, since inc_deq was modified in 3.4 and this patch will not apply. Signed-off-by: Sarah Sharp Tested-by: James Ettle Tested-by: Matthew Hall Cc: stable@vger.kernel.org --- drivers/usb/host/xhci-ring.c | 36 ++++++++++++++++++++++-------------- 1 file changed, 22 insertions(+), 14 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 0c93f5dcea6f..643c2f3f3e73 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -145,29 +145,37 @@ static void next_trb(struct xhci_hcd *xhci, */ static void inc_deq(struct xhci_hcd *xhci, struct xhci_ring *ring) { - union xhci_trb *next; unsigned long long addr; ring->deq_updates++; - /* If this is not event ring, there is one more usable TRB */ + /* + * If this is not event ring, and the dequeue pointer + * is not on a link TRB, there is one more usable TRB + */ if (ring->type != TYPE_EVENT && !last_trb(xhci, ring, ring->deq_seg, ring->dequeue)) ring->num_trbs_free++; - next = ++(ring->dequeue); - /* Update the dequeue pointer further if that was a link TRB or we're at - * the end of an event ring segment (which doesn't have link TRBS) - */ - while (last_trb(xhci, ring, ring->deq_seg, next)) { - if (ring->type == TYPE_EVENT && last_trb_on_last_seg(xhci, - ring, ring->deq_seg, next)) { - ring->cycle_state = (ring->cycle_state ? 0 : 1); + do { + /* + * Update the dequeue pointer further if that was a link TRB or + * we're at the end of an event ring segment (which doesn't have + * link TRBS) + */ + if (last_trb(xhci, ring, ring->deq_seg, ring->dequeue)) { + if (ring->type == TYPE_EVENT && + last_trb_on_last_seg(xhci, ring, + ring->deq_seg, ring->dequeue)) { + ring->cycle_state = (ring->cycle_state ? 0 : 1); + } + ring->deq_seg = ring->deq_seg->next; + ring->dequeue = ring->deq_seg->trbs; + } else { + ring->dequeue++; } - ring->deq_seg = ring->deq_seg->next; - ring->dequeue = ring->deq_seg->trbs; - next = ring->dequeue; - } + } while (last_trb(xhci, ring, ring->deq_seg, ring->dequeue)); + addr = (unsigned long long) xhci_trb_virt_to_dma(ring->deq_seg, ring->dequeue); } -- cgit v1.2.3