[PATCH 2/2] USB: prepare for changover to Runtime PM framework

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

 



This patch (as1303) revises the USB Power Management infrastructure to
make it compatible with the new driver-model Runtime PM framework:

	Drivers are no longer allowed to access intf->pm_usage_cnt
	directly; the PM framework manages its own usage counters.

	usb_autopm_set_interface() is eliminated, because it directly
	sets intf->pm_usage_cnt.

	usb_autopm_enable() and usb_autopm_disable() are eliminated,
	because they call usb_autopm_set_interface().

	usb_autopm_get_interface_no_resume() and
	usb_autopm_put_interface_no_suspend() are added.  They
	correspond to pm_runtime_get_noresume() and
	pm_runtime_put_noidle() in the PM framework.

	The power/level attribute no longer accepts "suspend", only
	"on" and "auto".  The PM framework doesn't allow devices to be
	forced into a suspended mode.

The hub driver contains the only code that violates the new
guidelines.  It is updated to use the new interface routines instead.

Signed-off-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>

---

Index: usb-2.6/Documentation/usb/power-management.txt
===================================================================
--- usb-2.6.orig/Documentation/usb/power-management.txt
+++ usb-2.6/Documentation/usb/power-management.txt
@@ -2,7 +2,7 @@
 
 		 Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
 
-			    October 5, 2007
+			    November 10, 2009
 
 
 
@@ -123,9 +123,9 @@ relevant attribute files are: wakeup, le
 
 	power/level
 
-		This file contains one of three words: "on", "auto",
-		or "suspend".  You can write those words to the file
-		to change the device's setting.
+		This file contains one of two words: "on" or "auto".
+		You can write those words to the file to change the
+		device's setting.
 
 		"on" means that the device should be resumed and
 		autosuspend is not allowed.  (Of course, system
@@ -134,10 +134,10 @@ relevant attribute files are: wakeup, le
 		"auto" is the normal state in which the kernel is
 		allowed to autosuspend and autoresume the device.
 
-		"suspend" means that the device should remain
-		suspended, and autoresume is not allowed.  (But remote
-		wakeup may still be allowed, since it is controlled
-		separately by the power/wakeup attribute.)
+		(In kernels up to 2.6.32, you could also specify
+		"suspend", meaning that the device should remain
+		suspended and autoresume was not allowed.  This
+		setting is no longer supported.)
 
 	power/autosuspend
 
@@ -313,13 +313,14 @@ three of the methods listed above.  In a
 that it supports autosuspend by setting the .supports_autosuspend flag
 in its usb_driver structure.  It is then responsible for informing the
 USB core whenever one of its interfaces becomes busy or idle.  The
-driver does so by calling these five functions:
+driver does so by calling these six functions:
 
 	int  usb_autopm_get_interface(struct usb_interface *intf);
 	void usb_autopm_put_interface(struct usb_interface *intf);
-	int  usb_autopm_set_interface(struct usb_interface *intf);
 	int  usb_autopm_get_interface_async(struct usb_interface *intf);
 	void usb_autopm_put_interface_async(struct usb_interface *intf);
+	void usb_autopm_get_interface_no_resume(struct usb_interface *intf);
+	void usb_autopm_put_interface_no_suspend(struct usb_interface *intf);
 
 The functions work by maintaining a counter in the usb_interface
 structure.  When intf->pm_usage_count is > 0 then the interface is
@@ -331,11 +332,13 @@ considered to be idle, and the kernel ma
 associated with the device itself rather than any of its interfaces.
 This field is used only by the USB core.)
 
-The driver owns intf->pm_usage_count; it can modify the value however
-and whenever it likes.  A nice aspect of the non-async usb_autopm_*
-routines is that the changes they make are protected by the usb_device
-structure's PM mutex (udev->pm_mutex); however drivers may change
-pm_usage_count without holding the mutex.  Drivers using the async
+Drivers must not modify intf->pm_usage_count directly; its value
+should be changed only be using the functions listed above.  Drivers
+are responsible for insuring that the overall change to pm_usage_count
+during their lifetime balances out to 0 (it may be necessary for the
+disconnect method to call usb_autopm_put_interface() one or more times
+to fulfill this requirement).  The first two routines use the PM mutex
+in struct usb_device for mutual exclusion; drivers using the async
 routines are responsible for their own synchronization and mutual
 exclusion.
 
@@ -347,11 +350,6 @@ exclusion.
 	attempts an autosuspend if the new value is <= 0 and the
 	device isn't suspended.
 
-	usb_autopm_set_interface() leaves pm_usage_count alone.
-	It attempts an autoresume if the value is > 0 and the device
-	is suspended, and it attempts an autosuspend if the value is
-	<= 0 and the device isn't suspended.
-
 	usb_autopm_get_interface_async() and
 	usb_autopm_put_interface_async() do almost the same things as
 	their non-async counterparts.  The differences are: they do
@@ -360,13 +358,11 @@ exclusion.
 	such as an URB's completion handler, but when they return the
 	device will not generally not yet be in the desired state.
 
-There also are a couple of utility routines drivers can use:
-
-	usb_autopm_enable() sets pm_usage_cnt to 0 and then calls
-	usb_autopm_set_interface(), which will attempt an autosuspend.
-
-	usb_autopm_disable() sets pm_usage_cnt to 1 and then calls
-	usb_autopm_set_interface(), which will attempt an autoresume.
+	usb_autopm_get_interface_no_resume() and
+	usb_autopm_put_interface_no_suspend() merely increment or
+	decrement the pm_usage_count value; they do not attempt to
+	carry out an autoresume or an autosuspend.  Hence they can be
+	called in an atomic context.
 
 The conventional usage pattern is that a driver calls
 usb_autopm_get_interface() in its open routine and
@@ -400,11 +396,11 @@ though, setting this flag won't cause th
 Normally a driver would set this flag in its probe method, at which
 time the device is guaranteed not to be autosuspended.)
 
-The usb_autopm_* routines have to run in a sleepable process context;
-they must not be called from an interrupt handler or while holding a
-spinlock.  In fact, the entire autosuspend mechanism is not well geared
-toward interrupt-driven operation.  However there is one thing a
-driver can do in an interrupt handler:
+The synchronous usb_autopm_* routines have to run in a sleepable
+process context; they must not be called from an interrupt handler or
+while holding a spinlock.  In fact, the entire autosuspend mechanism
+is not well geared toward interrupt-driven operation.  However there
+is one thing a driver can do in an interrupt handler:
 
 	usb_mark_last_busy(struct usb_device *udev);
 
Index: usb-2.6/include/linux/usb.h
===================================================================
--- usb-2.6.orig/include/linux/usb.h
+++ usb-2.6/include/linux/usb.h
@@ -432,7 +432,6 @@ struct usb_tt;
  * @do_remote_wakeup:  remote wakeup should be enabled
  * @reset_resume: needs reset instead of resume
  * @autosuspend_disabled: autosuspend disabled by the user
- * @autoresume_disabled: autoresume disabled by the user
  * @skip_sys_resume: skip the next system resume
  * @wusb_dev: if this is a Wireless USB device, link to the WUSB
  *	specific data for the device.
@@ -516,7 +515,6 @@ struct usb_device {
 	unsigned do_remote_wakeup:1;
 	unsigned reset_resume:1;
 	unsigned autosuspend_disabled:1;
-	unsigned autoresume_disabled:1;
 	unsigned skip_sys_resume:1;
 #endif
 	struct wusb_dev *wusb_dev;
@@ -542,22 +540,20 @@ extern struct usb_device *usb_find_devic
 
 /* USB autosuspend and autoresume */
 #ifdef CONFIG_USB_SUSPEND
-extern int usb_autopm_set_interface(struct usb_interface *intf);
 extern int usb_autopm_get_interface(struct usb_interface *intf);
 extern void usb_autopm_put_interface(struct usb_interface *intf);
 extern int usb_autopm_get_interface_async(struct usb_interface *intf);
 extern void usb_autopm_put_interface_async(struct usb_interface *intf);
 
-static inline void usb_autopm_enable(struct usb_interface *intf)
+static inline void usb_autopm_get_interface_no_resume(
+		struct usb_interface *intf)
 {
-	atomic_set(&intf->pm_usage_cnt, 0);
-	usb_autopm_set_interface(intf);
+	atomic_inc(&intf->pm_usage_cnt);
 }
-
-static inline void usb_autopm_disable(struct usb_interface *intf)
+static inline void usb_autopm_put_interface_no_suspend(
+		struct usb_interface *intf)
 {
-	atomic_set(&intf->pm_usage_cnt, 1);
-	usb_autopm_set_interface(intf);
+	atomic_dec(&intf->pm_usage_cnt);
 }
 
 static inline void usb_mark_last_busy(struct usb_device *udev)
@@ -567,12 +563,8 @@ static inline void usb_mark_last_busy(st
 
 #else
 
-static inline int usb_autopm_set_interface(struct usb_interface *intf)
-{ return 0; }
-
 static inline int usb_autopm_get_interface(struct usb_interface *intf)
 { return 0; }
-
 static inline int usb_autopm_get_interface_async(struct usb_interface *intf)
 { return 0; }
 
@@ -580,9 +572,11 @@ static inline void usb_autopm_put_interf
 { }
 static inline void usb_autopm_put_interface_async(struct usb_interface *intf)
 { }
-static inline void usb_autopm_enable(struct usb_interface *intf)
+static inline void usb_autopm_get_interface_no_resume(
+		struct usb_interface *intf)
 { }
-static inline void usb_autopm_disable(struct usb_interface *intf)
+static inline void usb_autopm_put_interface_no_suspend(
+		struct usb_interface *intf)
 { }
 static inline void usb_mark_last_busy(struct usb_device *udev)
 { }
Index: usb-2.6/drivers/usb/core/driver.c
===================================================================
--- usb-2.6.orig/drivers/usb/core/driver.c
+++ usb-2.6/drivers/usb/core/driver.c
@@ -948,8 +948,6 @@ static int usb_resume_device(struct usb_
 
  done:
 	dev_vdbg(&udev->dev, "%s: status %d\n", __func__, status);
-	if (status == 0)
-		udev->autoresume_disabled = 0;
 	return status;
 }
 
@@ -1280,11 +1278,6 @@ static int usb_resume_both(struct usb_de
 
 	/* Propagate the resume up the tree, if necessary */
 	if (udev->state == USB_STATE_SUSPENDED) {
-		if ((msg.event & PM_EVENT_AUTO) &&
-				udev->autoresume_disabled) {
-			status = -EPERM;
-			goto done;
-		}
 		if (parent) {
 			status = usb_autoresume_device(parent);
 			if (status == 0) {
@@ -1638,8 +1631,6 @@ int usb_autopm_get_interface_async(struc
 
 	if (intf->condition == USB_INTERFACE_UNBOUND)
 		status = -ENODEV;
-	else if (udev->autoresume_disabled)
-		status = -EPERM;
 	else {
 		atomic_inc(&intf->pm_usage_cnt);
 		if (atomic_read(&intf->pm_usage_cnt) > 0 &&
@@ -1652,28 +1643,6 @@ int usb_autopm_get_interface_async(struc
 }
 EXPORT_SYMBOL_GPL(usb_autopm_get_interface_async);
 
-/**
- * usb_autopm_set_interface - set a USB interface's autosuspend state
- * @intf: the usb_interface whose state should be set
- *
- * This routine sets the autosuspend state of @intf's device according
- * to @intf's usage counter, which the caller must have set previously.
- * If the counter is <= 0, the device is autosuspended (if it isn't
- * already suspended and if nothing else prevents the autosuspend).  If
- * the counter is > 0, the device is autoresumed (if it isn't already
- * awake).
- */
-int usb_autopm_set_interface(struct usb_interface *intf)
-{
-	int	status;
-
-	status = usb_autopm_do_interface(intf, 0);
-	dev_vdbg(&intf->dev, "%s: status %d cnt %d\n",
-			__func__, status, atomic_read(&intf->pm_usage_cnt));
-	return status;
-}
-EXPORT_SYMBOL_GPL(usb_autopm_set_interface);
-
 #else
 
 void usb_autosuspend_work(struct work_struct *work)
Index: usb-2.6/drivers/usb/core/sysfs.c
===================================================================
--- usb-2.6.orig/drivers/usb/core/sysfs.c
+++ usb-2.6/drivers/usb/core/sysfs.c
@@ -317,7 +317,6 @@ static DEVICE_ATTR(autosuspend, S_IRUGO 
 
 static const char on_string[] = "on";
 static const char auto_string[] = "auto";
-static const char suspend_string[] = "suspend";
 
 static ssize_t
 show_level(struct device *dev, struct device_attribute *attr, char *buf)
@@ -325,13 +324,8 @@ show_level(struct device *dev, struct de
 	struct usb_device *udev = to_usb_device(dev);
 	const char *p = auto_string;
 
-	if (udev->state == USB_STATE_SUSPENDED) {
-		if (udev->autoresume_disabled)
-			p = suspend_string;
-	} else {
-		if (udev->autosuspend_disabled)
-			p = on_string;
-	}
+	if (udev->state != USB_STATE_SUSPENDED && udev->autosuspend_disabled)
+		p = on_string;
 	return sprintf(buf, "%s\n", p);
 }
 
@@ -343,7 +337,7 @@ set_level(struct device *dev, struct dev
 	int len = count;
 	char *cp;
 	int rc = 0;
-	int old_autosuspend_disabled, old_autoresume_disabled;
+	int old_autosuspend_disabled;
 
 	cp = memchr(buf, '\n', count);
 	if (cp)
@@ -351,7 +345,6 @@ set_level(struct device *dev, struct dev
 
 	usb_lock_device(udev);
 	old_autosuspend_disabled = udev->autosuspend_disabled;
-	old_autoresume_disabled = udev->autoresume_disabled;
 
 	/* Setting the flags without calling usb_pm_lock is a subject to
 	 * races, but who cares...
@@ -359,28 +352,18 @@ set_level(struct device *dev, struct dev
 	if (len == sizeof on_string - 1 &&
 			strncmp(buf, on_string, len) == 0) {
 		udev->autosuspend_disabled = 1;
-		udev->autoresume_disabled = 0;
 		rc = usb_external_resume_device(udev, PMSG_USER_RESUME);
 
 	} else if (len == sizeof auto_string - 1 &&
 			strncmp(buf, auto_string, len) == 0) {
 		udev->autosuspend_disabled = 0;
-		udev->autoresume_disabled = 0;
 		rc = usb_external_resume_device(udev, PMSG_USER_RESUME);
 
-	} else if (len == sizeof suspend_string - 1 &&
-			strncmp(buf, suspend_string, len) == 0) {
-		udev->autosuspend_disabled = 0;
-		udev->autoresume_disabled = 1;
-		rc = usb_external_suspend_device(udev, PMSG_USER_SUSPEND);
-
 	} else
 		rc = -EINVAL;
 
-	if (rc) {
+	if (rc)
 		udev->autosuspend_disabled = old_autosuspend_disabled;
-		udev->autoresume_disabled = old_autoresume_disabled;
-	}
 	usb_unlock_device(udev);
 	return (rc < 0 ? rc : count);
 }
Index: usb-2.6/drivers/usb/core/hub.c
===================================================================
--- usb-2.6.orig/drivers/usb/core/hub.c
+++ usb-2.6/drivers/usb/core/hub.c
@@ -71,6 +71,7 @@ struct usb_hub {
 
 	unsigned		mA_per_port;	/* current for each child */
 
+	unsigned		init_done:1;
 	unsigned		limited_power:1;
 	unsigned		quiescing:1;
 	unsigned		disconnected:1;
@@ -375,12 +376,13 @@ static void kick_khubd(struct usb_hub *h
 {
 	unsigned long	flags;
 
-	/* Suppress autosuspend until khubd runs */
-	atomic_set(&to_usb_interface(hub->intfdev)->pm_usage_cnt, 1);
-
 	spin_lock_irqsave(&hub_event_lock, flags);
 	if (!hub->disconnected && list_empty(&hub->event_list)) {
 		list_add_tail(&hub->event_list, &hub_event_list);
+
+		/* Suppress autosuspend until khubd runs */
+		usb_autopm_get_interface_no_resume(
+				to_usb_interface(hub->intfdev));
 		wake_up(&khubd_wait);
 	}
 	spin_unlock_irqrestore(&hub_event_lock, flags);
@@ -665,7 +667,7 @@ int usb_remove_device(struct usb_device 
 }
 
 enum hub_activation_type {
-	HUB_INIT, HUB_INIT2, HUB_INIT3,
+	HUB_INIT, HUB_INIT2, HUB_INIT3,		/* INITs must come first */
 	HUB_POST_RESET, HUB_RESUME, HUB_RESET_RESUME,
 };
 
@@ -710,8 +712,8 @@ static void hub_activate(struct usb_hub 
 					msecs_to_jiffies(delay));
 
 			/* Suppress autosuspend until init is done */
-			atomic_set(&to_usb_interface(hub->intfdev)->
-					pm_usage_cnt, 1);
+			usb_autopm_get_interface_no_resume(
+					to_usb_interface(hub->intfdev));
 			return;		/* Continues at init2: below */
 		} else {
 			hub_power_on(hub, true);
@@ -818,6 +820,7 @@ static void hub_activate(struct usb_hub 
 	}
  init3:
 	hub->quiescing = 0;
+	hub->init_done = 1;
 
 	status = usb_submit_urb(hub->urb, GFP_NOIO);
 	if (status < 0)
@@ -827,6 +830,10 @@ static void hub_activate(struct usb_hub 
 
 	/* Scan all ports that need attention */
 	kick_khubd(hub);
+
+	/* Allow autosuspend if it was suppressed */
+	if (type <= HUB_INIT3)
+		usb_autopm_put_interface_async(to_usb_interface(hub->intfdev));
 }
 
 /* Implement the continuations for the delays above */
@@ -854,6 +861,11 @@ static void hub_quiesce(struct usb_hub *
 	int i;
 
 	cancel_delayed_work_sync(&hub->init_work);
+	if (!hub->init_done) {
+		hub->init_done = 1;
+		usb_autopm_put_interface_no_suspend(
+				to_usb_interface(hub->intfdev));
+	}
 
 	/* khubd and related activity won't re-trigger */
 	hub->quiescing = 1;
@@ -1176,7 +1188,10 @@ static void hub_disconnect(struct usb_in
 
 	/* Take the hub off the event list and don't let it be added again */
 	spin_lock_irq(&hub_event_lock);
-	list_del_init(&hub->event_list);
+	if (!list_empty(&hub->event_list)) {
+		list_del_init(&hub->event_list);
+		usb_autopm_put_interface_no_suspend(intf);
+	}
 	hub->disconnected = 1;
 	spin_unlock_irq(&hub_event_lock);
 
@@ -3230,7 +3245,7 @@ static void hub_events(void)
 		 * disconnected while waiting for the lock to succeed. */
 		usb_lock_device(hdev);
 		if (unlikely(hub->disconnected))
-			goto loop;
+			goto loop2;
 
 		/* If the hub has died, clean up after it */
 		if (hdev->state == USB_STATE_NOTATTACHED) {
@@ -3379,11 +3394,15 @@ static void hub_events(void)
 			}
 		}
 
-loop_autopm:
-		/* Allow autosuspend if we're not going to run again */
-		if (list_empty(&hub->event_list))
-			usb_autopm_enable(intf);
-loop:
+ loop_autopm:
+		/* Balance the usb_autopm_get_interface() above */
+		usb_autopm_put_interface_no_suspend(intf);
+ loop:
+		/* Balance the usb_autopm_get_interface_no_resume() in
+		 * kick_khubd() and allow autosuspend.
+		 */
+		usb_autopm_put_interface(intf);
+ loop2:
 		usb_unlock_device(hdev);
 		kref_put(&hub->kref, hub_release);
 

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