[RFC 1/7] xhci: Fix LPM disable/enable during device reset.

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

 



The USB 3.0 specification says that sending a Set Feature or Clear
Feature for U1/U2 Enable is not a valid request when the device is in
the Default or Addressed state.  It is only valid when the device is in
the Configured state.

The original LPM patch attempted to disable LPM after the device had
been reset by hub_port_init(), before it had the configuration
reinstalled.  The TI hub I tested with did not fail the Clear Feature
U1/U2 Enable request that khubd sent while it was in the addressed
state, which is why I didn't catch it.

Move the LPM disable before the device reset, so that we can send the
Clear Feature U1/U2 Enable successfully, and balance the LPM disable
count.

Also delete any calls to usb_enable_lpm() on error paths that lead to
re-enumeration.  The calls will fail because the device isn't
configured, and it's not useful to balance the LPM disable count because
the usb_device is about to be destroyed before re-enumeration.

Fix the early exit path ("done" label) to call usb_enable_lpm() to
balance the LPM disable count.

Note that calling usb_reset_and_verify_device() with an unconfigured
device may fail on the first call to usb_disable_lpm().  That's because
the LPM disable count is initialized to 0 (LPM enabled), and
usb_disable_lpm() will attempt to send a Clear Feature U1/U2 request to
a device in the Addressed state.  The next patch will fix that.

This commit should be backported to kernels as old as 3.5, that contain
the commit 8306095fd2c1100e8244c09bf560f97aca5a311d "USB: Disable USB
3.0 LPM in critical sections."

Signed-off-by: Sarah Sharp <sarah.a.sharp@xxxxxxxxxxxxxxx>
Cc: stable@xxxxxxxxxxxxxxx
---
 drivers/usb/core/hub.c |   26 ++++++++++++--------------
 1 files changed, 12 insertions(+), 14 deletions(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 25a7422..afa2c61 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -4670,6 +4670,16 @@ static int usb_reset_and_verify_device(struct usb_device *udev)
 	}
 	parent_hub = hdev_to_hub(parent_hdev);
 
+	/* Disable LPM while we reset the device and reinstall the alt settings.
+	 * Device-initiated LPM settings, and system exit latency settings are
+	 * cleared when the device is reset, so we have to set them up again.
+	 */
+	ret = usb_unlocked_disable_lpm(udev);
+	if (ret) {
+		dev_err(&udev->dev, "%s Failed to disable LPM\n.", __func__);
+		goto re_enumerate;
+	}
+
 	set_bit(port1, parent_hub->busy_bits);
 	for (i = 0; i < SET_CONFIG_TRIES; ++i) {
 
@@ -4697,22 +4707,11 @@ static int usb_reset_and_verify_device(struct usb_device *udev)
 		goto done;
 
 	mutex_lock(hcd->bandwidth_mutex);
-	/* Disable LPM while we reset the device and reinstall the alt settings.
-	 * Device-initiated LPM settings, and system exit latency settings are
-	 * cleared when the device is reset, so we have to set them up again.
-	 */
-	ret = usb_disable_lpm(udev);
-	if (ret) {
-		dev_err(&udev->dev, "%s Failed to disable LPM\n.", __func__);
-		mutex_unlock(hcd->bandwidth_mutex);
-		goto done;
-	}
 	ret = usb_hcd_alloc_bandwidth(udev, udev->actconfig, NULL, NULL);
 	if (ret < 0) {
 		dev_warn(&udev->dev,
 				"Busted HC?  Not enough HCD resources for "
 				"old configuration.\n");
-		usb_enable_lpm(udev);
 		mutex_unlock(hcd->bandwidth_mutex);
 		goto re_enumerate;
 	}
@@ -4724,7 +4723,6 @@ static int usb_reset_and_verify_device(struct usb_device *udev)
 		dev_err(&udev->dev,
 			"can't restore configuration #%d (error=%d)\n",
 			udev->actconfig->desc.bConfigurationValue, ret);
-		usb_enable_lpm(udev);
 		mutex_unlock(hcd->bandwidth_mutex);
 		goto re_enumerate;
   	}
@@ -4763,17 +4761,17 @@ static int usb_reset_and_verify_device(struct usb_device *udev)
 				desc->bInterfaceNumber,
 				desc->bAlternateSetting,
 				ret);
-			usb_unlocked_enable_lpm(udev);
 			goto re_enumerate;
 		}
 	}
 
+done:
 	/* Now that the alt settings are re-installed, enable LPM. */
 	usb_unlocked_enable_lpm(udev);
-done:
 	return 0;
  
 re_enumerate:
+	/* LPM state doesn't matter when we're about to destroy the device. */
 	hub_port_logical_disconnect(parent_hub, port1);
 	return -ENODEV;
 }
-- 
1.7.9

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