[PATCH v4 13/14] usb: force warm reset to break resume livelock

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

 



Resuming a powered down port sometimes results in the port state being
stuck in USB_SS_PORT_LS_POLLING:

 hub 3-0:1.0: debounce: port 1: total 2000ms stable 0ms status 0x2e0
 port1: can't get reconnection after setting port  power on, status -110
 hub 3-0:1.0: port 1 status 0000.02e0 after resume, -19
 usb 3-1: can't resume, status -19
 hub 3-0:1.0: logical disconnect on port 1

In the case above we wait 2 seconds for the port to reconnect and for
that entire time the port remained in the polling state.  A warm reset
triggers the device to reconnect and resume as normal.  With this patch
we get:

 hub 3-0:1.0: debounce: port 1: total 2000ms stable 0ms status 0x2e0
 usb usb3: port1 usb_port_runtime_resume requires warm reset
 hub 3-0:1.0: port 1 not warm reset yet, waiting 50ms
 usb 3-1: reset SuperSpeed USB device number 2 using xhci_hcd

With this in place we may want to consider reducing the timeout and
relying on warm reset for recovery.  Other xHCs that fail to propagate
warm resets on hub resume may want to trigger this behavior via a quirk.

Cc: Julius Werner <jwerner@xxxxxxxxxxxx>
Cc: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
Cc: Vikas Sajjan <vikas.sajjan@xxxxxxxxxx>
Cc: Kukjin Kim <kgene.kim@xxxxxxxxxxx>
Cc: Vincent Palatin <vpalatin@xxxxxxxxxxxx>
Cc: Lan Tianyu <tianyu.lan@xxxxxxxxx>
Cc: Ksenia Ragiadakou <burzalodowa@xxxxxxxxx>
Cc: Vivek Gautam <gautam.vivek@xxxxxxxxxxx>
Cc: Douglas Anderson <dianders@xxxxxxxxxxxx>
Cc: Felipe Balbi <balbi@xxxxxx>
Cc: Sunil Joshi <joshi@xxxxxxxxxxx>
Signed-off-by: Dan Williams <dan.j.williams@xxxxxxxxx>
---
 drivers/usb/core/hub.c  |   25 +++++++++++++++++--------
 drivers/usb/core/hub.h  |    2 ++
 drivers/usb/core/port.c |   32 ++++++++++++++++++++++----------
 3 files changed, 41 insertions(+), 18 deletions(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 9a505978ab92..243cf5767433 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -2512,10 +2512,12 @@ static int hub_port_reset(struct usb_hub *hub, int port1,
 /* Is a USB 3.0 port in the Inactive or Compliance Mode state?
  * Port worm reset is required to recover
  */
-static bool hub_port_warm_reset_required(struct usb_hub *hub, u16 portstatus)
+static bool hub_port_warm_reset_required(struct usb_hub *hub, int port1,
+					 u16 portstatus)
 {
 	return hub_is_superspeed(hub->hdev) &&
-		(((portstatus & USB_PORT_STAT_LINK_STATE) ==
+		(test_bit(port1, hub->warm_reset_bits) ||
+		((portstatus & USB_PORT_STAT_LINK_STATE) ==
 		  USB_SS_PORT_LS_SS_INACTIVE) ||
 		 ((portstatus & USB_PORT_STAT_LINK_STATE) ==
 		  USB_SS_PORT_LS_COMP_MOD)) ;
@@ -2555,7 +2557,7 @@ static int hub_port_wait_reset(struct usb_hub *hub, int port1,
 	if ((portstatus & USB_PORT_STAT_RESET))
 		return -EBUSY;
 
-	if (hub_port_warm_reset_required(hub, portstatus))
+	if (hub_port_warm_reset_required(hub, port1, portstatus))
 		return -ENOTCONN;
 
 	/* Device went away? */
@@ -2654,8 +2656,9 @@ static int hub_port_reset(struct usb_hub *hub, int port1,
 		if (status < 0)
 			goto done;
 
-		if (hub_port_warm_reset_required(hub, portstatus))
+		if (hub_port_warm_reset_required(hub, port1, portstatus))
 			warm = true;
+		clear_bit(port1, hub->warm_reset_bits);
 	}
 
 	/* Reset the port */
@@ -2693,7 +2696,8 @@ static int hub_port_reset(struct usb_hub *hub, int port1,
 					&portstatus, &portchange) < 0)
 				goto done;
 
-			if (!hub_port_warm_reset_required(hub, portstatus))
+			if (!hub_port_warm_reset_required(hub, port1,
+							  portstatus))
 				goto done;
 
 			/*
@@ -2766,8 +2770,13 @@ static int check_port_resume_type(struct usb_device *udev,
 		struct usb_hub *hub, int port1,
 		int status, unsigned portchange, unsigned portstatus)
 {
+	/* Is a warm reset needed to recover the connection? */
+	if (udev->reset_resume
+	    && hub_port_warm_reset_required(hub, port1, portstatus)) {
+		/* pass */;
+	}
 	/* Is the device still present? */
-	if (status || port_is_suspended(hub, portstatus) ||
+	else if (status || port_is_suspended(hub, portstatus) ||
 			!port_is_power_on(hub, portstatus) ||
 			!(portstatus & USB_PORT_STAT_CONNECTION)) {
 		if (status >= 0)
@@ -3911,7 +3920,7 @@ int hub_port_debounce(struct usb_hub *hub, int port1, bool must_be_connected)
 		"debounce: port %d: total %dms stable %dms status 0x%x\n",
 		port1, total_time, stable_time, portstatus);
 
-	if (stable_time < HUB_DEBOUNCE_STABLE)
+	if (stable_time < HUB_DEBOUNCE_STABLE && !must_be_connected)
 		return -ETIMEDOUT;
 	return portstatus;
 }
@@ -4796,7 +4805,7 @@ static void port_event(struct usb_hub *hub, int port1)
 		/* Warm reset a USB3 protocol port if it's in
 		 * SS.Inactive state.
 		 */
-		if (hub_port_warm_reset_required(hub, portstatus)) {
+		if (hub_port_warm_reset_required(hub, port1, portstatus)) {
 			int status;
 
 			dev_dbg(hub_dev, "port%d: do warm reset\n", port1);
diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h
index b4f397bc6957..70ff9b996fa7 100644
--- a/drivers/usb/core/hub.h
+++ b/drivers/usb/core/hub.h
@@ -51,6 +51,8 @@ struct usb_hub {
 							device present */
 	unsigned long		wakeup_bits[1];	/* ports that have signaled
 							remote wakeup */
+	unsigned long		warm_reset_bits[1]; /* ports that want warm
+						       reset recovery */
 #if USB_MAXCHILDREN > 31 /* 8*sizeof(unsigned long) - 1 */
 #error event_bits[] is too short!
 #endif
diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c
index be1e18355fec..630964a23b72 100644
--- a/drivers/usb/core/port.c
+++ b/drivers/usb/core/port.c
@@ -95,19 +95,31 @@ static int usb_port_runtime_resume(struct device *dev)
 
 	retval = usb_hub_set_port_power(hdev, hub, port1, true);
 	if (port_dev->child && !retval) {
-		/*
-		 * Attempt to wait for usb hub port to be reconnected in order
-		 * to make the resume procedure successful.  The device may have
-		 * disconnected while the port was powered off, so ignore the
-		 * return status.
+		u16 portstatus;
+
+		/* Our preference is to simply wait for the port to reconnect.
+		 * However, there are cases where toggling port power results in
+		 * the host port being stuck in SS.Polling.  This is likely the
+		 * caused by the hub failing to trigger warm reset on exiting
+		 * the logical poweroff state.  We detect it here and flag the
+		 * port as needing warm reset recovery (to be performed later
+		 * by usb_port_resume())
 		 */
-		retval = hub_port_debounce_be_connected(hub, port1);
-		if (retval < 0)
-			dev_dbg(&port_dev->dev, "can't get reconnection after setting port  power on, status %d\n",
-					retval);
+		portstatus = hub_port_debounce_be_connected(hub, port1);
+		if (!(portstatus & USB_PORT_STAT_CONNECTION)
+		    && hub_is_superspeed(hdev)
+		    && (portstatus & USB_SS_PORT_LS_POLLING)) {
+			dev_dbg(&hdev->dev, "port%d %s requires warm reset\n",
+				port1, __func__);
+			set_bit(port1, hub->warm_reset_bits);
+		} else if (!(portstatus & USB_PORT_STAT_CONNECTION)) {
+			dev_dbg(&hdev->dev, "port%d %s failed reconnect\n",
+				port1, __func__);
+		}
 
 		/* keep this port awake until we have had a chance to recover
-		 * the child
+		 * the child.  If warm reset was flagged above it will be
+		 * carried out by the child resume reset
 		 */
 		pm_runtime_get_noresume(&port_dev->dev);
 		port_dev->resume_child = 1;

--
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