[PATCH v2 0/4] xhci: fixes for 3.15-rc usb-linus

[PATCH v2 0/4] xhci: fixes for 3.15-rc usb-linus

[Mathias Nyman]

Hi Greg                                                                                                
                                                                                                       
Second try at this xhci fixes series for 3.15-rc usb-linus.                                                         
Most of them are very small fixes that didn't make                                                     
it to 3.14, sitting and waiting for 3.15-rc1 to come out.                                              
                                                                                                       
Only the "Prefer endpoint context.." patch  by Julius has a bit more content.                          
                                                                                                       
These patches are picked together with Sarah, they are tested on top of                                
3.15-rc1, and apply on your current usb-linus branch                                                   

changes since v1:

Drop "xhci: Use pci_enable_msix_exact() instead of pci_enable_msix()"
from this fixes series, and move to for-usb-next (3.16) instead

Add proper stable tags and information to commit messages    

-Mathias                                   


David Cohen (1):
  usb/xhci: fix compilation warning when !CONFIG_PCI && !CONFIG_PM

Denis Turischev (1):
  xhci: Switch Intel Lynx Point ports to EHCI on shutdown.

Igor Gnatenko (1):
  xhci: extend quirk for Renesas cards

Julius Werner (1):
  usb: xhci: Prefer endpoint context dequeue pointer over stopped_trb

 drivers/usb/host/xhci-pci.c  |  6 ++--
 drivers/usb/host/xhci-ring.c | 67 ++++++++++++++++++++------------------------
 drivers/usb/host/xhci.c      |  7 ++---
 drivers/usb/host/xhci.h      |  2 --
 4 files changed, 37 insertions(+), 45 deletions(-)

-- 
1.8.3.2

[PATCH v2 1/4] usb: xhci: Prefer endpoint context dequeue pointer over stopped_trb

[Mathias Nyman]

From: Julius Werner <jwerner@chromium.org>

We have observed a rare cycle state desync bug after Set TR Dequeue
Pointer commands on Intel LynxPoint xHCs (resulting in an endpoint that
doesn't fetch new TRBs and thus an unresponsive USB device). It always
triggers when a previous Set TR Dequeue Pointer command has set the
pointer to the final Link TRB of a segment, and then another URB gets
enqueued and cancelled again before it can be completed. Further
investigation showed that the xHC had returned the Link TRB in the TRB
Pointer field of the Transfer Event (CC == Stopped -- Length Invalid),
but when xhci_find_new_dequeue_state() later accesses the Endpoint
Context's TR Dequeue Pointer field it is set to the first TRB of the
next segment.

The driver expects those two values to be the same in this situation,
and uses the cycle state of the latter together with the address of the
former. This should be fine according to the XHCI specification, since
the endpoint ring should be stopped when returning the Transfer Event
and thus should not advance over the Link TRB before it gets restarted.
However, real-world XHCI implementations apparently don't really care
that much about these details, so the driver should follow a more
defensive approach to try to work around HC spec violations.

This patch removes the stopped_trb variable that had been used to store
the TRB Pointer from the last Transfer Event of a stopped TRB. Instead,
xhci_find_new_dequeue_state() now relies only on the Endpoint Context,
requiring a small amount of additional processing to find the virtual
address corresponding to the TR Dequeue Pointer. Some other parts of the
function were slightly rearranged to better fit into this model.

This patch should be backported to kernels as old as 2.6.31 that contain
the commit ae636747146ea97efa18e04576acd3416e2514f5 "USB: xhci: URB
cancellation support."

Signed-off-by: Julius Werner <jwerner@chromium.org>
Cc: stable@vger.kernel.org
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
 drivers/usb/host/xhci-ring.c | 67 ++++++++++++++++++++------------------------
 drivers/usb/host/xhci.c      |  1 -
 drivers/usb/host/xhci.h      |  2 --
 3 files changed, 31 insertions(+), 39 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 5f926be..7a0e3c7 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -550,6 +550,7 @@ void xhci_find_new_dequeue_state(struct xhci_hcd *xhci,
 	struct xhci_ring *ep_ring;
 	struct xhci_generic_trb *trb;
 	dma_addr_t addr;
+	u64 hw_dequeue;
 
 	ep_ring = xhci_triad_to_transfer_ring(xhci, slot_id,
 			ep_index, stream_id);
@@ -559,16 +560,6 @@ void xhci_find_new_dequeue_state(struct xhci_hcd *xhci,
 				stream_id);
 		return;
 	}
-	state->new_cycle_state = 0;
-	xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb,
-			"Finding segment containing stopped TRB.");
-	state->new_deq_seg = find_trb_seg(cur_td->start_seg,
-			dev->eps[ep_index].stopped_trb,
-			&state->new_cycle_state);
-	if (!state->new_deq_seg) {
-		WARN_ON(1);
-		return;
-	}
 
 	/* Dig out the cycle state saved by the xHC during the stop ep cmd */
 	xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb,
@@ -577,46 +568,57 @@ void xhci_find_new_dequeue_state(struct xhci_hcd *xhci,
 	if (ep->ep_state & EP_HAS_STREAMS) {
 		struct xhci_stream_ctx *ctx =
 			&ep->stream_info->stream_ctx_array[stream_id];
-		state->new_cycle_state = 0x1 & le64_to_cpu(ctx->stream_ring);
+		hw_dequeue = le64_to_cpu(ctx->stream_ring);
 	} else {
 		struct xhci_ep_ctx *ep_ctx
 			= xhci_get_ep_ctx(xhci, dev->out_ctx, ep_index);
-		state->new_cycle_state = 0x1 & le64_to_cpu(ep_ctx->deq);
+		hw_dequeue = le64_to_cpu(ep_ctx->deq);
 	}
 
+	/* Find virtual address and segment of hardware dequeue pointer */
+	state->new_deq_seg = ep_ring->deq_seg;
+	state->new_deq_ptr = ep_ring->dequeue;
+	while (xhci_trb_virt_to_dma(state->new_deq_seg, state->new_deq_ptr)
+			!= (dma_addr_t)(hw_dequeue & ~0xf)) {
+		next_trb(xhci, ep_ring, &state->new_deq_seg,
+					&state->new_deq_ptr);
+		if (state->new_deq_ptr == ep_ring->dequeue) {
+			WARN_ON(1);
+			return;
+		}
+	}
+	/*
+	 * Find cycle state for last_trb, starting at old cycle state of
+	 * hw_dequeue. If there is only one segment ring, find_trb_seg() will
+	 * return immediately and cannot toggle the cycle state if this search
+	 * wraps around, so add one more toggle manually in that case.
+	 */
+	state->new_cycle_state = hw_dequeue & 0x1;
+	if (ep_ring->first_seg == ep_ring->first_seg->next &&
+			cur_td->last_trb < state->new_deq_ptr)
+		state->new_cycle_state ^= 0x1;
+
 	state->new_deq_ptr = cur_td->last_trb;
 	xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb,
 			"Finding segment containing last TRB in TD.");
 	state->new_deq_seg = find_trb_seg(state->new_deq_seg,
-			state->new_deq_ptr,
-			&state->new_cycle_state);
+			state->new_deq_ptr, &state->new_cycle_state);
 	if (!state->new_deq_seg) {
 		WARN_ON(1);
 		return;
 	}
 
+	/* Increment to find next TRB after last_trb. Cycle if appropriate. */
 	trb = &state->new_deq_ptr->generic;
 	if (TRB_TYPE_LINK_LE32(trb->field[3]) &&
 	    (trb->field[3] & cpu_to_le32(LINK_TOGGLE)))
 		state->new_cycle_state ^= 0x1;
 	next_trb(xhci, ep_ring, &state->new_deq_seg, &state->new_deq_ptr);
 
-	/*
-	 * If there is only one segment in a ring, find_trb_seg()'s while loop
-	 * will not run, and it will return before it has a chance to see if it
-	 * needs to toggle the cycle bit.  It can't tell if the stalled transfer
-	 * ended just before the link TRB on a one-segment ring, or if the TD
-	 * wrapped around the top of the ring, because it doesn't have the TD in
-	 * question.  Look for the one-segment case where stalled TRB's address
-	 * is greater than the new dequeue pointer address.
-	 */
-	if (ep_ring->first_seg == ep_ring->first_seg->next &&
-			state->new_deq_ptr < dev->eps[ep_index].stopped_trb)
-		state->new_cycle_state ^= 0x1;
+	/* Don't update the ring cycle state for the producer (us). */
 	xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb,
 			"Cycle state = 0x%x", state->new_cycle_state);
 
-	/* Don't update the ring cycle state for the producer (us). */
 	xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb,
 			"New dequeue segment = %p (virtual)",
 			state->new_deq_seg);
@@ -799,7 +801,6 @@ static void xhci_handle_cmd_stop_ep(struct xhci_hcd *xhci, int slot_id,
 	if (list_empty(&ep->cancelled_td_list)) {
 		xhci_stop_watchdog_timer_in_irq(xhci, ep);
 		ep->stopped_td = NULL;
-		ep->stopped_trb = NULL;
 		ring_doorbell_for_active_rings(xhci, slot_id, ep_index);
 		return;
 	}
@@ -867,11 +868,9 @@ remove_finished_td:
 		ring_doorbell_for_active_rings(xhci, slot_id, ep_index);
 	}
 
-	/* Clear stopped_td and stopped_trb if endpoint is not halted */
-	if (!(ep->ep_state & EP_HALTED)) {
+	/* Clear stopped_td if endpoint is not halted */
+	if (!(ep->ep_state & EP_HALTED))
 		ep->stopped_td = NULL;
-		ep->stopped_trb = NULL;
-	}
 
 	/*
 	 * Drop the lock and complete the URBs in the cancelled TD list.
@@ -1941,14 +1940,12 @@ static void xhci_cleanup_halted_endpoint(struct xhci_hcd *xhci,
 	struct xhci_virt_ep *ep = &xhci->devs[slot_id]->eps[ep_index];
 	ep->ep_state |= EP_HALTED;
 	ep->stopped_td = td;
-	ep->stopped_trb = event_trb;
 	ep->stopped_stream = stream_id;
 
 	xhci_queue_reset_ep(xhci, slot_id, ep_index);
 	xhci_cleanup_stalled_ring(xhci, td->urb->dev, ep_index);
 
 	ep->stopped_td = NULL;
-	ep->stopped_trb = NULL;
 	ep->stopped_stream = 0;
 
 	xhci_ring_cmd_db(xhci);
@@ -2030,7 +2027,6 @@ static int finish_td(struct xhci_hcd *xhci, struct xhci_td *td,
 		 * the ring dequeue pointer or take this TD off any lists yet.
 		 */
 		ep->stopped_td = td;
-		ep->stopped_trb = event_trb;
 		return 0;
 	} else {
 		if (trb_comp_code == COMP_STALL) {
@@ -2042,7 +2038,6 @@ static int finish_td(struct xhci_hcd *xhci, struct xhci_td *td,
 			 * USB class driver clear the stall later.
 			 */
 			ep->stopped_td = td;
-			ep->stopped_trb = event_trb;
 			ep->stopped_stream = ep_ring->stream_id;
 		} else if (xhci_requires_manual_halt_cleanup(xhci,
 					ep_ctx, trb_comp_code)) {
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 8fe4e12..988ed5f 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -2954,7 +2954,6 @@ void xhci_endpoint_reset(struct usb_hcd *hcd,
 		xhci_ring_cmd_db(xhci);
 	}
 	virt_ep->stopped_td = NULL;
-	virt_ep->stopped_trb = NULL;
 	virt_ep->stopped_stream = 0;
 	spin_unlock_irqrestore(&xhci->lock, flags);
 
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index d280e92..4746816 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -865,8 +865,6 @@ struct xhci_virt_ep {
 #define EP_GETTING_NO_STREAMS	(1 << 5)
 	/* ----  Related to URB cancellation ---- */
 	struct list_head	cancelled_td_list;
-	/* The TRB that was last reported in a stopped endpoint ring */
-	union xhci_trb		*stopped_trb;
 	struct xhci_td		*stopped_td;
 	unsigned int		stopped_stream;
 	/* Watchdog timer for stop endpoint command to cancel URBs */
-- 
1.8.3.2

[PATCH v2 2/4] xhci: Switch Intel Lynx Point ports to EHCI on shutdown.

[Mathias Nyman]

From: Denis Turischev <denis.turischev@compulab.co.il>

The same issue like with Panther Point chipsets. If the USB ports are
switched to xHCI on shutdown, the xHCI host will send a spurious interrupt,
which will wake the system. Some BIOS have work around for this, but not all.
One example is Compulab's mini-desktop, the Intense-PC2.

The bug can be avoided if the USB ports are switched back to EHCI on
shutdown.

This patch should be backported to stable kernels as old as 3.12,
that contain the commit 638298dc66ea36623dbc2757a24fc2c4ab41b016
"xhci: Fix spurious wakeups after S5 on Haswell"

Signed-off-by: Denis Turischev <denis@compulab.co.il>
Cc: stable@vger.kernel.org
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
 drivers/usb/host/xhci-pci.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index 47390e3..1715063 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -134,6 +134,8 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci)
 		 */
 		if (pdev->subsystem_vendor == PCI_VENDOR_ID_HP)
 			xhci->quirks |= XHCI_SPURIOUS_WAKEUP;
+
+		xhci->quirks |= XHCI_SPURIOUS_REBOOT;
 	}
 	if (pdev->vendor == PCI_VENDOR_ID_ETRON &&
 			pdev->device == PCI_DEVICE_ID_ASROCK_P67) {
-- 
1.8.3.2

[PATCH v2 3/4] xhci: extend quirk for Renesas cards

[Mathias Nyman]

From: Igor Gnatenko <i.gnatenko.brain@gmail.com>

After suspend another Renesas PCI-X USB 3.0 card doesn't work.
[root@fedora-20 ~]# lspci -vmnnd 1912:
Device:	03:00.0
Class:	USB controller [0c03]
Vendor:	Renesas Technology Corp. [1912]
Device:	uPD720202 USB 3.0 Host Controller [0015]
SVendor:	Renesas Technology Corp. [1912]
SDevice:	uPD720202 USB 3.0 Host Controller [0015]
Rev:	02
ProgIf:	30

This patch should be applied to stable kernel 3.14 that contain
the commit 1aa9578c1a9450fb21501c4f549f5b1edb557e6d
"xhci: Fix resume issues on Renesas chips in Samsung laptops"

Reported-and-tested-by: Anatoly Kharchenko <rfr-bugs@yandex.ru>
Reference: http://redmine.russianfedora.pro/issues/1315
Signed-off-by: Igor Gnatenko <i.gnatenko.brain@gmail.com>
Cc: stable@vger.kernel.org # 3.14
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
 drivers/usb/host/xhci-pci.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index 1715063..35d4477 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -145,9 +145,7 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci)
 		xhci->quirks |= XHCI_TRUST_TX_LENGTH;
 	}
 	if (pdev->vendor == PCI_VENDOR_ID_RENESAS &&
-			pdev->device == 0x0015 &&
-			pdev->subsystem_vendor == PCI_VENDOR_ID_SAMSUNG &&
-			pdev->subsystem_device == 0xc0cd)
+			pdev->device == 0x0015)
 		xhci->quirks |= XHCI_RESET_ON_RESUME;
 	if (pdev->vendor == PCI_VENDOR_ID_VIA)
 		xhci->quirks |= XHCI_RESET_ON_RESUME;
-- 
1.8.3.2

[PATCH v2 4/4] usb/xhci: fix compilation warning when !CONFIG_PCI && !CONFIG_PM

[Mathias Nyman]

From: David Cohen <david.a.cohen@linux.intel.com>

When CONFIG_PCI and CONFIG_PM are not selected, xhci.c gets this
warning:
drivers/usb/host/xhci.c:409:13: warning: ‘xhci_msix_sync_irqs’ defined
but not used [-Wunused-function]

Instead of creating nested #ifdefs, this patch fixes it by defining the
xHCI PCI stubs as inline.

This warning has been in since 3.2 kernel and was
caused by commit 421aa841a134f6a743111cf44d0c6d3b45e3cf8c
"usb/xhci: hide MSI code behind PCI bars", but wasn't noticed
until 3.13 when a configuration with these options was tried

Signed-off-by: David Cohen <david.a.cohen@linux.intel.com>
Cc: stable@vger.kernel.org # 3.2
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
 drivers/usb/host/xhci.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 988ed5f..3008369 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -408,16 +408,16 @@ static int xhci_try_enable_msi(struct usb_hcd *hcd)
 
 #else
 
-static int xhci_try_enable_msi(struct usb_hcd *hcd)
+static inline int xhci_try_enable_msi(struct usb_hcd *hcd)
 {
 	return 0;
 }
 
-static void xhci_cleanup_msix(struct xhci_hcd *xhci)
+static inline void xhci_cleanup_msix(struct xhci_hcd *xhci)
 {
 }
 
-static void xhci_msix_sync_irqs(struct xhci_hcd *xhci)
+static inline void xhci_msix_sync_irqs(struct xhci_hcd *xhci)
 {
 }
 
-- 
1.8.3.2

Re: [PATCH v2 0/4] xhci: fixes for 3.15-rc usb-linus

[Greg KH]

On Fri, Apr 25, 2014 at 07:20:12PM +0300, Mathias Nyman wrote:
> Hi Greg                                                                                                
>                                                                                                        
> Second try at this xhci fixes series for 3.15-rc usb-linus.                                                         
> Most of them are very small fixes that didn't make                                                     
> it to 3.14, sitting and waiting for 3.15-rc1 to come out.                                              
>                                                                                                        
> Only the "Prefer endpoint context.." patch  by Julius has a bit more content.                          
>                                                                                                        
> These patches are picked together with Sarah, they are tested on top of                                
> 3.15-rc1, and apply on your current usb-linus branch                                                   

Much better, all now applied.

What's with all that trailing whitespace in your email text?

greg k-h

Re: [PATCH v2 0/4] xhci: fixes for 3.15-rc usb-linus

[Sarah Sharp]

On Fri, Apr 25, 2014 at 09:35:05AM -0700, Greg KH wrote:
> On Fri, Apr 25, 2014 at 07:20:12PM +0300, Mathias Nyman wrote:
> > Hi Greg                                                                                                
> >                                                                                                        
> > Second try at this xhci fixes series for 3.15-rc usb-linus.                                                         
> > Most of them are very small fixes that didn't make                                                     
> > it to 3.14, sitting and waiting for 3.15-rc1 to come out.                                              
> >                                                                                                        
> > Only the "Prefer endpoint context.." patch  by Julius has a bit more content.                          
> >                                                                                                        
> > These patches are picked together with Sarah, they are tested on top of                                
> > 3.15-rc1, and apply on your current usb-linus branch                                                   
> 
> Much better, all now applied.
> 
> What's with all that trailing whitespace in your email text?

I use the following lines in my .vimrc to highlight trailing whitespace:

hi link localWhitespaceError Error
au Syntax * syn match localWhitespaceError /\(\zs\%#\|\s\)\+$/ display
au Syntax * syn match localWhitespaceError / \+\ze\t/ display

I also have a macro to add stable tags and which commit IDs introduced
the issue the bug fixes:

iab backporthis
\<CR>Fixes: commitID ("commitDescription")
\<CR>Cc: stable@vger.kernel.org # kernelVersion

Then when I'm amending someone's commit to add my Signed-off-by line, I
type backporthis, and vim expands it to the longer version.  (I've
deliberately misspelled it here, so it doesn't get expanded.) Sometimes
it takes a couple of round of `git blame` to figure out when the bug was
introduced.

Sarah Sharp

Re: [PATCH v2 0/4] xhci: fixes for 3.15-rc usb-linus

[Mathias Nyman]

On 04/25/2014 07:35 PM, Greg KH wrote:
> On Fri, Apr 25, 2014 at 07:20:12PM +0300, Mathias Nyman wrote:
>> Hi Greg
>>
>> Second try at this xhci fixes series for 3.15-rc usb-linus.
>> Most of them are very small fixes that didn't make
>> it to 3.14, sitting and waiting for 3.15-rc1 to come out.
>>
>> Only the "Prefer endpoint context.." patch  by Julius has a bit more content.
>>
>> These patches are picked together with Sarah, they are tested on top of
>> 3.15-rc1, and apply on your current usb-linus branch
>
> Much better, all now applied.
>
> What's with all that trailing whitespace in your email text?
>

Thanks for applying, and sorry about the whitespace errors.
Did some copy-pasting between editors without proper whitespace 
detection set on all file types.

I'll be more careful with these in the future

-Mathias