[v3,1/2] USB: quirks: Check device interface LPM capability

Commit Message

Kyle Williams Dec. 8, 2018, 1:47 a.m. UTC

From: Kyle Williams <[e-mail]>

Description: enable the ability to disable LPM for all devices matched
by interface information

Signed-off-by: Kyle Williams <[e-mail]>
---
 drivers/usb/core/hub.c | 87 +++++++++++++++++++++++-------------------
 1 file changed, 48 insertions(+), 39 deletions(-)

Comments

Greg KH Dec. 12, 2018, 11:37 a.m. UTC | #1

On Fri, Dec 07, 2018 at 08:47:21PM -0500, Kyle Williams wrote:
> From: Kyle Williams <[e-mail]>
> 
> Description: enable the ability to disable LPM for all devices matched
> by interface information

Why have "Description:" in here?  Of course this is the description :)

And the subject doesn't make much sense, what does this have to do with
quirks?

Also, you need more information here in the description about why you
are doing all of this, as it is, I have no idea why this patch is needed
at all.

thanks,

greg k-h

Kyle Williams Jan. 10, 2019, 3:49 p.m. UTC | #2

On Wed, Dec 12, 2018 at 12:37:09PM +0100, Greg Kroah-Hartman wrote:
> On Fri, Dec 07, 2018 at 08:47:21PM -0500, Kyle Williams wrote:
> > From: Kyle Williams <[e-mail]>
> > 
> > Description: enable the ability to disable LPM for all devices matched
> > by interface information
> 
> Why have "Description:" in here?  Of course this is the description :)
> 
> And the subject doesn't make much sense, what does this have to do with
> quirks?
> 
> Also, you need more information here in the description about why you
> are doing all of this, as it is, I have no idea why this patch is needed
> at all.
> 
> thanks,
> 
> greg k-h

It seems the underlying issue has been solved in
0472bf0 xhci: Prevent U1/U2 link pm states if exit latency is too long

However, the patch to fix the issue when matching the USB_QUIRK_NO_LPM
quirk by iterface could be useful in the future. Currently nothing will
happen that quirk is matched by interface instead of by vid/pid pair.
Is their anything that would need to be changed besides removing the
Description tag?

-Kyle

Greg KH Jan. 11, 2019, 9:09 a.m. UTC | #3

On Thu, Jan 10, 2019 at 10:49:09AM -0500, Kyle Williams wrote:
> On Wed, Dec 12, 2018 at 12:37:09PM +0100, Greg Kroah-Hartman wrote:
> > On Fri, Dec 07, 2018 at 08:47:21PM -0500, Kyle Williams wrote:
> > > From: Kyle Williams <[e-mail]>
> > > 
> > > Description: enable the ability to disable LPM for all devices matched
> > > by interface information
> > 
> > Why have "Description:" in here?  Of course this is the description :)
> > 
> > And the subject doesn't make much sense, what does this have to do with
> > quirks?
> > 
> > Also, you need more information here in the description about why you
> > are doing all of this, as it is, I have no idea why this patch is needed
> > at all.
> > 
> > thanks,
> > 
> > greg k-h
> 
> It seems the underlying issue has been solved in
> 0472bf0 xhci: Prevent U1/U2 link pm states if exit latency is too long
> 
> However, the patch to fix the issue when matching the USB_QUIRK_NO_LPM
> quirk by iterface could be useful in the future. Currently nothing will
> happen that quirk is matched by interface instead of by vid/pid pair.
> Is their anything that would need to be changed besides removing the
> Description tag?

Don't add code to the kernel that is not used now.  If in the future
someone needs to add this type of functionality, worry about it then...

thanks,

greg k-h

Patch

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 0f9381b69a3b..8f366ec4d21b 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -365,6 +365,51 @@  static void usb_set_lpm_parameters(struct usb_device *udev)
 	usb_set_lpm_sel(udev, &udev->u2_params);
 }
 
+/*
+ * There are reports of USB 3.0 devices that say they support USB 2.0 Link PM
+ * when they're plugged into a USB 2.0 port, but they don't work when LPM is
+ * enabled.
+ *
+ * Only enable USB 2.0 Link PM if the port is internal (hardwired), or the
+ * device says it supports the new USB 2.0 Link PM errata by setting the BESL
+ * support bit in the BOS descriptor.
+ */
+static void hub_set_initial_usb2_lpm_policy(struct usb_device *udev)
+{
+	struct usb_hub *hub = usb_hub_to_struct_hub(udev->parent);
+	int connect_type = USB_PORT_CONNECT_TYPE_UNKNOWN;
+
+	if (!udev->usb2_hw_lpm_capable || !udev->bos)
+		return;
+
+	if (hub)
+		connect_type = hub->ports[udev->portnum - 1]->connect_type;
+
+	if ((udev->bos->ext_cap->bmAttributes & cpu_to_le32(USB_BESL_SUPPORT))
+			|| connect_type == USB_PORT_CONNECT_TYPE_HARD_WIRED) {
+		udev->usb2_hw_lpm_allowed = 1;
+		usb_set_usb2_hardware_lpm(udev, 1);
+	}
+}
+
+void usb_update_device_lpm(struct usb_hcd *hcd, struct usb_device *udev)
+{
+	int retval;
+
+	if (udev->wusb == 0 && le16_to_cpu(udev->descriptor.bcdUSB) >= 0x0201) {
+		retval = usb_get_bos_descriptor(udev);
+		if (!retval) {
+			udev->lpm_capable = usb_device_supports_lpm(udev);
+			usb_set_lpm_parameters(udev);
+		}
+	}
+
+	/* notify HCD that we have a device connected and addressed */
+	if (hcd->driver->update_device)
+		hcd->driver->update_device(hcd, udev);
+	hub_set_initial_usb2_lpm_policy(udev);
+}
+
 /* USB 2.0 spec Section 11.24.4.5 */
 static int get_hub_descriptor(struct usb_device *hdev,
 		struct usb_hub_descriptor *desc)
@@ -2295,7 +2340,6 @@  static int usb_enumerate_device_otg(struct usb_device *udev)
 	return err;
 }
 
-
 /**
  * usb_enumerate_device - Read device configs/intfs/otg (usbcore-internal)
  * @udev: newly addressed device (in ADDRESS state)
@@ -2351,6 +2395,8 @@  static int usb_enumerate_device(struct usb_device *udev)
 
 	usb_detect_interface_quirks(udev);
 
+	usb_update_device_lpm(hcd, udev);
+
 	return 0;
 }
 
@@ -4402,33 +4448,6 @@  static int hub_set_address(struct usb_device *udev, int devnum)
 	return retval;
 }
 
-/*
- * There are reports of USB 3.0 devices that say they support USB 2.0 Link PM
- * when they're plugged into a USB 2.0 port, but they don't work when LPM is
- * enabled.
- *
- * Only enable USB 2.0 Link PM if the port is internal (hardwired), or the
- * device says it supports the new USB 2.0 Link PM errata by setting the BESL
- * support bit in the BOS descriptor.
- */
-static void hub_set_initial_usb2_lpm_policy(struct usb_device *udev)
-{
-	struct usb_hub *hub = usb_hub_to_struct_hub(udev->parent);
-	int connect_type = USB_PORT_CONNECT_TYPE_UNKNOWN;
-
-	if (!udev->usb2_hw_lpm_capable || !udev->bos)
-		return;
-
-	if (hub)
-		connect_type = hub->ports[udev->portnum - 1]->connect_type;
-
-	if ((udev->bos->ext_cap->bmAttributes & cpu_to_le32(USB_BESL_SUPPORT)) ||
-			connect_type == USB_PORT_CONNECT_TYPE_HARD_WIRED) {
-		udev->usb2_hw_lpm_allowed = 1;
-		usb_set_usb2_hardware_lpm(udev, 1);
-	}
-}
-
 static int hub_enable_device(struct usb_device *udev)
 {
 	struct usb_hcd *hcd = bus_to_hcd(udev->bus);
@@ -4779,19 +4798,9 @@  hub_port_init(struct usb_hub *hub, struct usb_device *udev, int port1,
 
 	usb_detect_quirks(udev);
 
-	if (udev->wusb == 0 && le16_to_cpu(udev->descriptor.bcdUSB) >= 0x0201) {
-		retval = usb_get_bos_descriptor(udev);
-		if (!retval) {
-			udev->lpm_capable = usb_device_supports_lpm(udev);
-			usb_set_lpm_parameters(udev);
-		}
-	}
+	usb_update_device_lpm(hcd, udev);
 
 	retval = 0;
-	/* notify HCD that we have a device connected and addressed */
-	if (hcd->driver->update_device)
-		hcd->driver->update_device(hcd, udev);
-	hub_set_initial_usb2_lpm_policy(udev);
 fail:
 	if (retval) {
 		hub_port_disable(hub, port1, 0);