Hi Steinar,

Do you mind trying attached patch? This is not a fix but to verify
whether LPM really works when only one device is used. You can
follow below steps:

1) apply the attached patch on top the latest kernel.
2) build and install the kernel.
3) boot your machine with the new kernel.
4) insert one  Blackmagic Design device into USB3 root port.
7) wait for a few seconds
6) check below sysfs nodes:

$ cat /sys/bus/usb/devices/<usb_device_path>/power/usb3_hardware_lpm_u1
$ cat /sys/bus/usb/devices/<usb_device_path>/power/usb3_hardware_lpm_u2

Can you please post values of above two sysfs nodes?
A copy of dmesg will also be helpful.

Thanks,
Baolu

On 11/05/2015 02:23 AM, Steinar H. Gunderson wrote:
On Mon, Nov 02, 2015 at 04:00:42PM -0500, Alan Stern wrote:
That commit was included in (approximately) the 4.1.5 or later stable
kernel, and it is included in 4.2.  You should be able to put one of
those on a bootable USB stick.
I tried going down this route. After some back and forth, I realize that...
the machine has only two USB ports, so I can't boot off USB stick and still
test how it behaves with two cards. (I could buy a hub, but that would surely
introduce new potential errors in the mix.)

So I'm afraid I can't help you at this point. My recommendation to include
the patch still stands, though.

/* Steinar */

>From c8cd71a0703cf26cfca9d89a3ad451f79c7232af Mon Sep 17 00:00:00 2001
From: Lu Baolu <baolu...@linux.intel.com>
Date: Thu, 5 Nov 2015 14:08:48 +0800
Subject: [PATCH 1/2] usb: core: LPM: fix usb3_hardware_lpm sysfs node

Commit 655fe4effe0f ("usbcore: add sysfs support to xHCI usb3
hardware LPM") introduced usb3_hardware_lpm sysfs node. This
doesn't show the correct status of USB3 U1 and U2 LPM status.

This patch fixes this by replacing usb3_hardware_lpm with two
nodes, usb3_hardware_lpm_u1 (for U1) and usb3_hardware_lpm_u2
(for U2), and recording the U1/U2 LPM status in right places.

Signed-off-by: Lu Baolu <baolu...@linux.intel.com>
---
 Documentation/ABI/testing/sysfs-bus-usb | 16 +++++++++-------
 Documentation/usb/power-management.txt  | 11 ++++++-----
 drivers/usb/core/hub.c                  | 29 +++++++++++++++++++++--------
 drivers/usb/core/sysfs.c                | 28 ++++++++++++++++++++++++----
 include/linux/usb.h                     |  2 ++
 5 files changed, 62 insertions(+), 24 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-bus-usb b/Documentation/ABI/testing/sysfs-bus-usb
index 3a4abfc..136ba17 100644
--- a/Documentation/ABI/testing/sysfs-bus-usb
+++ b/Documentation/ABI/testing/sysfs-bus-usb
@@ -134,19 +134,21 @@ Description:
 		enabled for the device. Developer can write y/Y/1 or n/N/0 to
 		the file to enable/disable the feature.
 
-What:		/sys/bus/usb/devices/.../power/usb3_hardware_lpm
-Date:		June 2015
+What:		/sys/bus/usb/devices/.../power/usb3_hardware_lpm_u1
+		/sys/bus/usb/devices/.../power/usb3_hardware_lpm_u2
+Date:		November 2015
 Contact:	Kevin Strasser <kevin.stras...@linux.intel.com>
+		Lu Baolu <baolu...@linux.intel.com>
 Description:
 		If CONFIG_PM is set and a USB 3.0 lpm-capable device is plugged
 		in to a xHCI host which supports link PM, it will check if U1
 		and U2 exit latencies have been set in the BOS descriptor; if
-		the check is is passed and the host supports USB3 hardware LPM,
+		the check is passed and the host supports USB3 hardware LPM,
 		USB3 hardware LPM will be enabled for the device and the USB
-		device directory will contain a file named
-		power/usb3_hardware_lpm. The file holds a string value (enable
-		or disable) indicating whether or not USB3 hardware LPM is
-		enabled for the device.
+		device directory will contain two files named
+		power/usb3_hardware_lpm_u1 and power/usb3_hardware_lpm_u2. These
+		files hold a string value (enable or disable) indicating whether
+		or not USB3 hardware LPM U1 or U2 is enabled for the device.
 
 What:		/sys/bus/usb/devices/.../removable
 Date:		February 2012
diff --git a/Documentation/usb/power-management.txt b/Documentation/usb/power-management.txt
index 4a15c90..0a94ffe 100644
--- a/Documentation/usb/power-management.txt
+++ b/Documentation/usb/power-management.txt
@@ -537,17 +537,18 @@ relevant attribute files are usb2_hardware_lpm and usb3_hardware_lpm.
 		can write y/Y/1 or n/N/0 to the file to	enable/disable
 		USB2 hardware LPM manually. This is for	test purpose mainly.
 
-	power/usb3_hardware_lpm
+	power/usb3_hardware_lpm_u1
+	power/usb3_hardware_lpm_u2
 
 		When a USB 3.0 lpm-capable device is plugged in to a
 		xHCI host which supports link PM, it will check if U1
 		and U2 exit latencies have been set in the BOS
 		descriptor; if the check is is passed and the host
 		supports USB3 hardware LPM, USB3 hardware LPM will be
-		enabled for the device and this file will be created.
-		The file holds a string value (enable or disable)
-		indicating whether or not USB3 hardware LPM is
-		enabled for the device.
+		enabled for the device and these files will be created.
+		The files hold a string value (enable or disable)
+		indicating whether or not USB3 hardware LPM U1 or U2
+		is enabled for the device.
 
 	USB Port Power Control
 	----------------------
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index bdeadc1..94a10e0 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -3875,17 +3875,28 @@ static void usb_enable_link_state(struct usb_hcd *hcd, struct usb_device *udev,
 		return;
 	}
 
-	if (usb_set_lpm_timeout(udev, state, timeout))
+	ret = usb_set_lpm_timeout(udev, state, timeout);
+	if (ret) {
 		/* If we can't set the parent hub U1/U2 timeout,
 		 * device-initiated LPM won't be allowed either, so let the xHCI
 		 * host know that this link state won't be enabled.
 		 */
-		hcd->driver->disable_usb3_lpm_timeout(hcd, udev, state);
-
+		if (hcd->driver->disable_usb3_lpm_timeout(hcd, udev, state))
+			dev_warn(&udev->dev,
+				"Could not disable xHCI %s timeout, "
+				"bus schedule bandwidth may be impacted.\n",
+				usb3_lpm_names[state]);
 	/* Only a configured device will accept the Set Feature U1/U2_ENABLE */
-	else if (udev->actconfig)
+	} else if (udev->actconfig) {
 		usb_set_device_initiated_lpm(udev, state, true);
+	}
 
+	if (!ret) {
+		if (state == USB3_LPM_U1)
+			udev->usb3_lpm_u1_enabled = 1;
+		else if (state == USB3_LPM_U2)
+			udev->usb3_lpm_u2_enabled = 1;
+	}
 }
 
 /*
@@ -3925,6 +3936,12 @@ static int usb_disable_link_state(struct usb_hcd *hcd, struct usb_device *udev,
 		dev_warn(&udev->dev, "Could not disable xHCI %s timeout, "
 				"bus schedule bandwidth may be impacted.\n",
 				usb3_lpm_names[state]);
+
+	if (state == USB3_LPM_U1)
+		udev->usb3_lpm_u1_enabled = 0;
+	else if (state == USB3_LPM_U2)
+		udev->usb3_lpm_u2_enabled = 0;
+
 	return 0;
 }
 
@@ -3959,8 +3976,6 @@ int usb_disable_lpm(struct usb_device *udev)
 	if (usb_disable_link_state(hcd, udev, USB3_LPM_U2))
 		goto enable_lpm;
 
-	udev->usb3_lpm_enabled = 0;
-
 	return 0;
 
 enable_lpm:
@@ -4018,8 +4033,6 @@ void usb_enable_lpm(struct usb_device *udev)
 
 	usb_enable_link_state(hcd, udev, USB3_LPM_U1);
 	usb_enable_link_state(hcd, udev, USB3_LPM_U2);
-
-	udev->usb3_lpm_enabled = 1;
 }
 EXPORT_SYMBOL_GPL(usb_enable_lpm);
 
diff --git a/drivers/usb/core/sysfs.c b/drivers/usb/core/sysfs.c
index d9ec2de..26a6a01 100644
--- a/drivers/usb/core/sysfs.c
+++ b/drivers/usb/core/sysfs.c
@@ -531,7 +531,7 @@ static ssize_t usb2_lpm_besl_store(struct device *dev,
 }
 static DEVICE_ATTR_RW(usb2_lpm_besl);
 
-static ssize_t usb3_hardware_lpm_show(struct device *dev,
+static ssize_t usb3_hardware_lpm_u1_show(struct device *dev,
 				      struct device_attribute *attr, char *buf)
 {
 	struct usb_device *udev = to_usb_device(dev);
@@ -539,7 +539,7 @@ static ssize_t usb3_hardware_lpm_show(struct device *dev,
 
 	usb_lock_device(udev);
 
-	if (udev->usb3_lpm_enabled)
+	if (udev->usb3_lpm_u1_enabled)
 		p = "enabled";
 	else
 		p = "disabled";
@@ -548,7 +548,26 @@ static ssize_t usb3_hardware_lpm_show(struct device *dev,
 
 	return sprintf(buf, "%s\n", p);
 }
-static DEVICE_ATTR_RO(usb3_hardware_lpm);
+static DEVICE_ATTR_RO(usb3_hardware_lpm_u1);
+
+static ssize_t usb3_hardware_lpm_u2_show(struct device *dev,
+				      struct device_attribute *attr, char *buf)
+{
+	struct usb_device *udev = to_usb_device(dev);
+	const char *p;
+
+	usb_lock_device(udev);
+
+	if (udev->usb3_lpm_u2_enabled)
+		p = "enabled";
+	else
+		p = "disabled";
+
+	usb_unlock_device(udev);
+
+	return sprintf(buf, "%s\n", p);
+}
+static DEVICE_ATTR_RO(usb3_hardware_lpm_u2);
 
 static struct attribute *usb2_hardware_lpm_attr[] = {
 	&dev_attr_usb2_hardware_lpm.attr,
@@ -562,7 +581,8 @@ static struct attribute_group usb2_hardware_lpm_attr_group = {
 };
 
 static struct attribute *usb3_hardware_lpm_attr[] = {
-	&dev_attr_usb3_hardware_lpm.attr,
+	&dev_attr_usb3_hardware_lpm_u1.attr,
+	&dev_attr_usb3_hardware_lpm_u2.attr,
 	NULL,
 };
 static struct attribute_group usb3_hardware_lpm_attr_group = {
diff --git a/include/linux/usb.h b/include/linux/usb.h
index b9a2807..211664f 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -584,6 +584,8 @@ struct usb_device {
 	unsigned usb2_hw_lpm_enabled:1;
 	unsigned usb2_hw_lpm_allowed:1;
 	unsigned usb3_lpm_enabled:1;
+	unsigned usb3_lpm_u1_enabled:1;
+	unsigned usb3_lpm_u2_enabled:1;
 	int string_langid;
 
 	/* static strings from the device */
-- 
2.1.4

Reply via email to