[regression] USB power management failure to suspend / high CPU usage

[Mathias Nyman <mathias.nyman@linux.intel.com>]

On 14.2.2019 20.04, Eric Blau wrote:
> On Thu, Feb 14, 2019 at 9:56 AM Mathias Nyman
> <mathias.nyman@linux.intel.com> wrote:
>>
>>> Thanks for looking into this, Mathias. Now that you point this out, on
>>> older kernels where suspend and resume works, I noticed the following
>>> log messages emitted when resuming from suspend:
>>>
>>> Feb 06 18:58:05 eric-macbookpro kernel: usb usb2-port3: Cannot enable.
>>> Maybe the USB cable is bad?
>>
>> Attached a testpatch that should react to ports stuck in polling state,
>> and warm reset them.
>>
>> It doesn't limit the numbers of reset tries, or allow system suspend with ports
>> stuck in polling state, but I'd like to know how the MacBook reacts to it
>>
>> Can you test it with debugging enabled?
> 
> Hi Mathias,
> 
> Thanks for the patch. I tested it against 4.20.8 and got a couple
> successful suspends but on the third attempt I got the same failure
> again. I noticed after the first suspend/resume, the card reader
> showed as "Link:Compliance" but on later attempts it showed stuck in
> Polling again.
> 
> I've attached the logs with debugging enabled.
> 

Thanks, logs show that several resets won't recover the card reader.

I'm taking a different approach, USB3 ports in polling state should
automatically move to connected/enabled. So instead of preventing
suspend if a port is found in polling I'll let it try to finish link
training for some time, and then just continue with suspend if it fails.

Patch attached.

This won't fix the broken card reader, but should allow your MacBook to suspend.
After this we can start looking at fixing the card reader, Ivan Miranov sent some
proposal for this.

-Mathias

From 444cab4f41c79c5a04d1fc8939b450c89dc64768 Mon Sep 17 00:00:00 2001
From: Mathias Nyman <mathias.nyman@linux.intel.com>
Date: Wed, 20 Feb 2019 16:10:54 +0200
Subject: [PATCH 1/2] xhci: Don't let USB3 ports stuck in polling state prevent
 suspend

Commit 2f31a67f01a8 ("usb: xhci: Prevent bus suspend if a port connect
change or polling state is detected") was intended to prevent ports that
were still link training from being forced to U3 suspend state mid
enumeration.
This solved enumeration issues for devices with slow link training.

Turns out some devices are stuck in the link training/polling state,
and thus that patch will prevent suspend completely for these devices.
This is seen with USB3 card readers in some MacBooks.

Instead of preventing suspend, give some time to complete the link
training. On successful training the port will end up as connected
and enabled.
If port instead is stuck in link training the bus suspend will continue
suspending after 360ms (10 * 36ms) timeout (tPollingLFPSTimeout).

Original patch was sent to stable, this one should go there as well

Fixes: 2f31a67f01a8 ("usb: xhci: Prevent bus suspend if a port connect change or polling state is detected")
Cc: stable@vger.kernel.org
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
 drivers/usb/host/xhci-hub.c | 19 ++++++++++++-------
 drivers/usb/host/xhci.h     |  8 ++++++++
 2 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index e2eece6..96a7405 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -1545,20 +1545,25 @@ int xhci_bus_suspend(struct usb_hcd *hcd)
 	port_index = max_ports;
 	while (port_index--) {
 		u32 t1, t2;
-
+		int retries = 10;
+retry:
 		t1 = readl(ports[port_index]->addr);
 		t2 = xhci_port_state_to_neutral(t1);
 		portsc_buf[port_index] = 0;
 
-		/* Bail out if a USB3 port has a new device in link training */
-		if ((hcd->speed >= HCD_USB3) &&
+		/*
+		 * Give a USB3 port in link training time to finish, but don't
+		 * prevent suspend as port might be stuck
+		 */
+		if ((hcd->speed >= HCD_USB3) && retries-- &&
 		    (t1 & PORT_PLS_MASK) == XDEV_POLLING) {
-			bus_state->bus_suspended = 0;
 			spin_unlock_irqrestore(&xhci->lock, flags);
-			xhci_dbg(xhci, "Bus suspend bailout, port in polling\n");
-			return -EBUSY;
+			msleep(XHCI_PORT_POLLING_LFPS_TIME);
+			spin_lock_irqsave(&xhci->lock, flags);
+			xhci_dbg(xhci, "port %d polling in bus suspend, waiting\n",
+				 port_index);
+			goto retry;
 		}
-
 		/* suspend ports in U0, or bail out for new connect changes */
 		if ((t1 & PORT_PE) && (t1 & PORT_PLS_MASK) == XDEV_U0) {
 			if ((t1 & PORT_CSC) && wake_enabled) {
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 652dc36..9334cde 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -452,6 +452,14 @@ struct xhci_op_regs {
  */
 #define XHCI_DEFAULT_BESL	4
 
+/*
+ * USB3 specification define a 360ms tPollingLFPSTiemout for USB3 ports
+ * to complete link training. usually link trainig completes much faster
+ * so check status 10 times with 36ms sleep in places we need to wait for
+ * polling to complete.
+ */
+#define XHCI_PORT_POLLING_LFPS_TIME  36
+
 /**
  * struct xhci_intr_reg - Interrupt Register Set
  * @irq_pending:	IMAN - Interrupt Management Register.  Used to enable
-- 
2.7.4