Patch "USB: global suspend and remote wakeup don't mix" has been added to the 3.10-stable tree

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

 



This is a note to let you know that I've just added the patch titled

    USB: global suspend and remote wakeup don't mix

to the 3.10-stable tree which can be found at:
    http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
     usb-global-suspend-and-remote-wakeup-don-t-mix.patch
and it can be found in the queue-3.10 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@xxxxxxxxxxxxxxx> know about it.


>From e583d9db9960cf40e0bc8afee4946baa9d71596e Mon Sep 17 00:00:00 2001
From: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
Date: Thu, 11 Jul 2013 14:58:04 -0400
Subject: USB: global suspend and remote wakeup don't mix

From: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>

commit e583d9db9960cf40e0bc8afee4946baa9d71596e upstream.

The hub driver was recently changed to use "global" suspend for system
suspend transitions on non-SuperSpeed buses.  This means that we don't
suspend devices individually by setting the suspend feature on the
upstream hub port; instead devices all go into suspend automatically
when the root hub stops transmitting packets.  The idea was to save
time and to avoid certain kinds of wakeup races.

Now it turns out that many hubs are buggy; they don't relay wakeup
requests from a downstream port to their upstream port if the
downstream port's suspend feature is not set (depending on the speed
of the downstream port, whether or not the hub is enabled for remote
wakeup, and possibly other factors).

We can't have hubs dropping wakeup requests.  Therefore this patch
goes partway back to the old policy: It sets the suspend feature for a
port if the device attached to that port or any of its descendants is
enabled for wakeup.  People will still be able to benefit from the
time savings if they don't care about wakeup and leave it disabled on
all their devices.

In order to accomplish this, the patch adds a new field to the usb_hub
structure: wakeup_enabled_descendants is a count of how many devices
below a suspended hub are enabled for remote wakeup.  A corresponding
new subroutine determines the number of wakeup-enabled devices at or
below an arbitrary suspended USB device.

This should be applied to the 3.10 stable kernel.

Signed-off-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
Reported-and-tested-by: Toralf Förster <toralf.foerster@xxxxxx>
Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>

---
 drivers/usb/core/hub.c |   39 +++++++++++++++++++++++++++++++--------
 drivers/usb/core/hub.h |    3 +++
 2 files changed, 34 insertions(+), 8 deletions(-)

--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -2855,6 +2855,15 @@ static int usb_disable_function_remotewa
 				USB_CTRL_SET_TIMEOUT);
 }
 
+/* Count of wakeup-enabled devices at or below udev */
+static unsigned wakeup_enabled_descendants(struct usb_device *udev)
+{
+	struct usb_hub *hub = usb_hub_to_struct_hub(udev);
+
+	return udev->do_remote_wakeup +
+			(hub ? hub->wakeup_enabled_descendants : 0);
+}
+
 /*
  * usb_port_suspend - suspend a usb device's upstream port
  * @udev: device that's no longer in active use, not a root hub
@@ -2895,8 +2904,8 @@ static int usb_disable_function_remotewa
  * Linux (2.6) currently has NO mechanisms to initiate that:  no khubd
  * timer, no SRP, no requests through sysfs.
  *
- * If Runtime PM isn't enabled or used, non-SuperSpeed devices really get
- * suspended only when their bus goes into global suspend (i.e., the root
+ * If Runtime PM isn't enabled or used, non-SuperSpeed devices may not get
+ * suspended until their bus goes into global suspend (i.e., the root
  * hub is suspended).  Nevertheless, we change @udev->state to
  * USB_STATE_SUSPENDED as this is the device's "logical" state.  The actual
  * upstream port setting is stored in @udev->port_is_suspended.
@@ -2967,15 +2976,21 @@ int usb_port_suspend(struct usb_device *
 	/* see 7.1.7.6 */
 	if (hub_is_superspeed(hub->hdev))
 		status = hub_set_port_link_state(hub, port1, USB_SS_PORT_LS_U3);
-	else if (PMSG_IS_AUTO(msg))
-		status = set_port_feature(hub->hdev, port1,
-						USB_PORT_FEAT_SUSPEND);
+
 	/*
 	 * For system suspend, we do not need to enable the suspend feature
 	 * on individual USB-2 ports.  The devices will automatically go
 	 * into suspend a few ms after the root hub stops sending packets.
 	 * The USB 2.0 spec calls this "global suspend".
+	 *
+	 * However, many USB hubs have a bug: They don't relay wakeup requests
+	 * from a downstream port if the port's suspend feature isn't on.
+	 * Therefore we will turn on the suspend feature if udev or any of its
+	 * descendants is enabled for remote wakeup.
 	 */
+	else if (PMSG_IS_AUTO(msg) || wakeup_enabled_descendants(udev) > 0)
+		status = set_port_feature(hub->hdev, port1,
+				USB_PORT_FEAT_SUSPEND);
 	else {
 		really_suspend = false;
 		status = 0;
@@ -3010,15 +3025,16 @@ int usb_port_suspend(struct usb_device *
 		if (!PMSG_IS_AUTO(msg))
 			status = 0;
 	} else {
-		/* device has up to 10 msec to fully suspend */
 		dev_dbg(&udev->dev, "usb %ssuspend, wakeup %d\n",
 				(PMSG_IS_AUTO(msg) ? "auto-" : ""),
 				udev->do_remote_wakeup);
-		usb_set_device_state(udev, USB_STATE_SUSPENDED);
 		if (really_suspend) {
 			udev->port_is_suspended = 1;
+
+			/* device has up to 10 msec to fully suspend */
 			msleep(10);
 		}
+		usb_set_device_state(udev, USB_STATE_SUSPENDED);
 	}
 
 	/*
@@ -3300,7 +3316,11 @@ static int hub_suspend(struct usb_interf
 	unsigned		port1;
 	int			status;
 
-	/* Warn if children aren't already suspended */
+	/*
+	 * Warn if children aren't already suspended.
+	 * Also, add up the number of wakeup-enabled descendants.
+	 */
+	hub->wakeup_enabled_descendants = 0;
 	for (port1 = 1; port1 <= hdev->maxchild; port1++) {
 		struct usb_device	*udev;
 
@@ -3310,6 +3330,9 @@ static int hub_suspend(struct usb_interf
 			if (PMSG_IS_AUTO(msg))
 				return -EBUSY;
 		}
+		if (udev)
+			hub->wakeup_enabled_descendants +=
+					wakeup_enabled_descendants(udev);
 	}
 
 	if (hdev->do_remote_wakeup && hub->quirk_check_port_auto_suspend) {
--- a/drivers/usb/core/hub.h
+++ b/drivers/usb/core/hub.h
@@ -59,6 +59,9 @@ struct usb_hub {
 	struct usb_tt		tt;		/* Transaction Translator */
 
 	unsigned		mA_per_port;	/* current for each child */
+#ifdef	CONFIG_PM
+	unsigned		wakeup_enabled_descendants;
+#endif
 
 	unsigned		limited_power:1;
 	unsigned		quiescing:1;


Patches currently in stable-queue which might be from stern@xxxxxxxxxxxxxxxxxxx are

queue-3.10/usb-clear-both-buffers-when-clearing-a-control-transfer-tt-buffer.patch
queue-3.10/usb-global-suspend-and-remote-wakeup-don-t-mix.patch
queue-3.10/usb-ehci-fix-resume-signalling-on-remote-wakeup.patch
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]