2010-01-18 00:29:16

by Rafael J. Wysocki

[permalink] [raw]
Subject: [RFC][PATCH] PM / Runtime: Add sysfs switch for disabling device run-time PM

From: Rafael J. Wysocki <[email protected]>
Subject: PM / Runtime: Add sysfs switch for disabling device run-time PM

Add new device sysfs attribute, power/runtime, allowing the user
space to block the run-time power management of the device. If this
attribute is set to "disabled", the driver of the device won't be
able to enable run-time power management for it (without breaking the
rules).

Signed-off-by: Rafael J. Wysocki <[email protected]>
---
drivers/base/power/power.h | 4 +
drivers/base/power/runtime.c | 107 +++++++++++++++++++++++++++++++++----------
drivers/base/power/sysfs.c | 46 ++++++++++++++++++
include/linux/pm.h | 1
include/linux/pm_runtime.h | 11 +---
5 files changed, 139 insertions(+), 30 deletions(-)

Index: linux-2.6/drivers/base/power/power.h
===================================================================
--- linux-2.6.orig/drivers/base/power/power.h
+++ linux-2.6/drivers/base/power/power.h
@@ -2,11 +2,15 @@

extern void pm_runtime_init(struct device *dev);
extern void pm_runtime_remove(struct device *dev);
+extern void pm_runtime_allow(struct device *dev);
+extern void pm_runtime_forbid(struct device *dev);

#else /* !CONFIG_PM_RUNTIME */

static inline void pm_runtime_init(struct device *dev) {}
static inline void pm_runtime_remove(struct device *dev) {}
+static inline void pm_runtime_allow(struct device *dev) {}
+static inline void pm_runtime_forbid(struct device *dev) {}

#endif /* !CONFIG_PM_RUNTIME */

Index: linux-2.6/drivers/base/power/sysfs.c
===================================================================
--- linux-2.6.orig/drivers/base/power/sysfs.c
+++ linux-2.6/drivers/base/power/sysfs.c
@@ -7,6 +7,20 @@
#include "power.h"

/*
+ * runtime - Report/change current runtime PM setting of the device
+ *
+ * Runtime power management of a device can be disabled with the help of
+ * this attribute. All devices have one of the following two values for
+ * the power/runtime file:
+ *
+ * + "enabled\n" to allow the device to be power managed at run time;
+ * + "disabled\n" to disable this feature for the device;
+ *
+ * The default for all devices is "enabled\n", but the runtime power
+ * management of the device has to be enabled by its driver to be actually
+ * used. Changing this attribute to "disabled\n" while the device is
+ * suspended causes it to be woken up.
+ *
* wakeup - Report/change current wakeup option for device
*
* Some devices support "wakeup" events, which are hardware signals
@@ -59,6 +73,35 @@
static const char enabled[] = "enabled";
static const char disabled[] = "disabled";

+#ifdef CONFIG_PM_RUNTIME
+static ssize_t runtime_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ return sprintf(buf, "%s\n",
+ dev->power.runtime_forbidden ? disabled : enabled);
+}
+
+static ssize_t runtime_store(struct device * dev, struct device_attribute *attr,
+ const char * buf, size_t n)
+{
+ char *cp;
+ int len = n;
+
+ cp = memchr(buf, '\n', n);
+ if (cp)
+ len = cp - buf;
+ if (len == sizeof enabled - 1 && strncmp(buf, enabled, len) == 0)
+ pm_runtime_allow(dev);
+ else if (len == sizeof disabled - 1 && strncmp(buf, disabled, len) == 0)
+ pm_runtime_forbid(dev);
+ else
+ return -EINVAL;
+ return n;
+}
+
+static DEVICE_ATTR(runtime, 0644, runtime_show, runtime_store);
+#endif
+
static ssize_t
wake_show(struct device * dev, struct device_attribute *attr, char * buf)
{
@@ -123,6 +166,9 @@ static DEVICE_ATTR(async, 0644, async_sh
#endif /* CONFIG_PM_SLEEP_ADVANCED_DEBUG */

static struct attribute * power_attrs[] = {
+#ifdef CONFIG_PM_RUNTIME
+ &dev_attr_runtime.attr,
+#endif
&dev_attr_wakeup.attr,
#ifdef CONFIG_PM_SLEEP_ADVANCED_DEBUG
&dev_attr_async.attr,
Index: linux-2.6/include/linux/pm.h
===================================================================
--- linux-2.6.orig/include/linux/pm.h
+++ linux-2.6/include/linux/pm.h
@@ -433,6 +433,7 @@ struct dev_pm_info {
unsigned int request_pending:1;
unsigned int deferred_resume:1;
unsigned int run_wake:1;
+ unsigned int runtime_forbidden:1;
enum rpm_request request;
enum rpm_status runtime_status;
int runtime_error;
Index: linux-2.6/drivers/base/power/runtime.c
===================================================================
--- linux-2.6.orig/drivers/base/power/runtime.c
+++ linux-2.6/drivers/base/power/runtime.c
@@ -943,35 +943,26 @@ int pm_runtime_barrier(struct device *de
EXPORT_SYMBOL_GPL(pm_runtime_barrier);

/**
- * __pm_runtime_disable - Disable run-time PM of a device.
+ * __pm_runtime_disable - Disable run-time PM of a device, no locking.
* @dev: Device to handle.
- * @check_resume: If set, check if there's a resume request for the device.
+ * @resume: If set, resume the device before disabling its run-time PM.
*
* Increment power.disable_depth for the device and if was zero previously,
* cancel all pending run-time PM requests for the device and wait for all
* operations in progress to complete. The device can be either active or
* suspended after its run-time PM has been disabled.
*
- * If @check_resume is set and there's a resume request pending when
- * __pm_runtime_disable() is called and power.disable_depth is zero, the
- * function will wake up the device before disabling its run-time PM.
+ * If @resume is set, the function will wake up the device before disabling its
+ * run-time PM.
*/
-void __pm_runtime_disable(struct device *dev, bool check_resume)
+static void __pm_runtime_disable(struct device *dev, bool resume)
{
- spin_lock_irq(&dev->power.lock);
-
if (dev->power.disable_depth > 0) {
dev->power.disable_depth++;
- goto out;
+ return;
}

- /*
- * Wake up the device if there's a resume request pending, because that
- * means there probably is some I/O to process and disabling run-time PM
- * shouldn't prevent the device from processing the I/O.
- */
- if (check_resume && dev->power.request_pending
- && dev->power.request == RPM_REQ_RESUME) {
+ if (resume) {
/*
* Prevent suspends and idle notifications from being carried
* out after we have woken up the device.
@@ -985,32 +976,101 @@ void __pm_runtime_disable(struct device

if (!dev->power.disable_depth++)
__pm_runtime_barrier(dev);
+}

- out:
+/**
+ * pm_runtime_disable_resume - Disable run-time PM of a device and resume it.
+ * @dev: Device to handle.
+ *
+ * Execute __pm_raw_runtime_disable() for the device in such a way that the
+ * device will be woken up if there's a resume request pending for it.
+ */
+void pm_runtime_disable_resume(struct device *dev)
+{
+ spin_lock_irq(&dev->power.lock);
+ /*
+ * Wake up the device if there's a resume request pending, because that
+ * means there probably is some I/O to process and disabling run-time PM
+ * shouldn't prevent the device from processing the I/O.
+ */
+ __pm_runtime_disable(dev, dev->power.request_pending
+ && dev->power.request == RPM_REQ_RESUME);
spin_unlock_irq(&dev->power.lock);
}
-EXPORT_SYMBOL_GPL(__pm_runtime_disable);
+EXPORT_SYMBOL_GPL(pm_runtime_disable_resume);

/**
- * pm_runtime_enable - Enable run-time PM of a device.
+ * pm_runtime_disable - Disable run-time PM of a device.
* @dev: Device to handle.
*/
-void pm_runtime_enable(struct device *dev)
+void pm_runtime_disable(struct device *dev)
{
- unsigned long flags;
-
- spin_lock_irqsave(&dev->power.lock, flags);
+ spin_lock_irq(&dev->power.lock);
+ __pm_runtime_disable(dev, false);
+ spin_unlock_irq(&dev->power.lock);
+}
+EXPORT_SYMBOL_GPL(pm_runtime_disable);

+/**
+ * __pm_runtime_enable - Enable run-time PM of a device, no locking.
+ * @dev: Device to handle.
+ */
+static void __pm_runtime_enable(struct device *dev)
+{
if (dev->power.disable_depth > 0)
dev->power.disable_depth--;
else
dev_warn(dev, "Unbalanced %s!\n", __func__);
+}
+
+/**
+ * pm_runtime_enable - Enable run-time PM of a device.
+ * @dev: Device to handle.
+ */
+void pm_runtime_enable(struct device *dev)
+{
+ unsigned long flags;

+ spin_lock_irqsave(&dev->power.lock, flags);
+ __pm_runtime_enable(dev);
spin_unlock_irqrestore(&dev->power.lock, flags);
}
EXPORT_SYMBOL_GPL(pm_runtime_enable);

/**
+ * pm_runtime_forbid - Block run-time PM of a device.
+ * @dev: Device to handle.
+ *
+ * Increase the device's disable count and set its power.runtime_forbidden flag,
+ * so that it's not enabled until pm_runtime_allow() is called for @dev.
+ */
+void pm_runtime_forbid(struct device *dev)
+{
+ spin_lock_irq(&dev->power.lock);
+ if (!dev->power.runtime_forbidden) {
+ dev->power.runtime_forbidden = true;
+ __pm_runtime_disable(dev, true);
+ }
+ spin_unlock_irq(&dev->power.lock);
+}
+
+/**
+ * pm_runtime_allow - Unblock run-time PM of a device.
+ * @dev: Device to handle.
+ *
+ * Decrease the device's disable count and set its power.runtime_forbidden flag.
+ */
+void pm_runtime_allow(struct device *dev)
+{
+ spin_lock_irq(&dev->power.lock);
+ if (dev->power.runtime_forbidden) {
+ dev->power.runtime_forbidden = false;
+ __pm_runtime_enable(dev);
+ }
+ spin_unlock_irq(&dev->power.lock);
+}
+
+/**
* pm_runtime_init - Initialize run-time PM fields in given device object.
* @dev: Device object to initialize.
*/
@@ -1023,6 +1083,7 @@ void pm_runtime_init(struct device *dev)

dev->power.disable_depth = 1;
atomic_set(&dev->power.usage_count, 0);
+ dev->power.runtime_forbidden = false;

dev->power.runtime_error = 0;

Index: linux-2.6/include/linux/pm_runtime.h
===================================================================
--- linux-2.6.orig/include/linux/pm_runtime.h
+++ linux-2.6/include/linux/pm_runtime.h
@@ -27,7 +27,8 @@ extern int __pm_runtime_put(struct devic
extern int __pm_runtime_set_status(struct device *dev, unsigned int status);
extern int pm_runtime_barrier(struct device *dev);
extern void pm_runtime_enable(struct device *dev);
-extern void __pm_runtime_disable(struct device *dev, bool check_resume);
+extern void pm_runtime_disable_resume(struct device *dev);
+extern void pm_runtime_disable(struct device *dev);

static inline bool pm_children_suspended(struct device *dev)
{
@@ -77,7 +78,8 @@ static inline int __pm_runtime_set_statu
unsigned int status) { return 0; }
static inline int pm_runtime_barrier(struct device *dev) { return 0; }
static inline void pm_runtime_enable(struct device *dev) {}
-static inline void __pm_runtime_disable(struct device *dev, bool c) {}
+static inline void pm_runtime_disable_resume(struct device *dev) {}
+static inline void pm_runtime_disable(struct device *dev) {}

static inline bool pm_children_suspended(struct device *dev) { return false; }
static inline void pm_suspend_ignore_children(struct device *dev, bool en) {}
@@ -118,9 +120,4 @@ static inline void pm_runtime_set_suspen
__pm_runtime_set_status(dev, RPM_SUSPENDED);
}

-static inline void pm_runtime_disable(struct device *dev)
-{
- __pm_runtime_disable(dev, true);
-}
-
#endif


2010-01-18 16:10:23

by Alan Stern

[permalink] [raw]
Subject: Re: [RFC][PATCH] PM / Runtime: Add sysfs switch for disabling device run-time PM

On Mon, 18 Jan 2010, Rafael J. Wysocki wrote:

> From: Rafael J. Wysocki <[email protected]>
> Subject: PM / Runtime: Add sysfs switch for disabling device run-time PM
>
> Add new device sysfs attribute, power/runtime, allowing the user
> space to block the run-time power management of the device. If this
> attribute is set to "disabled", the driver of the device won't be
> able to enable run-time power management for it (without breaking the
> rules).

Ouch. This does nearly the same thing as the power/level attribute in
the USB subsystem, but in an incompatible and more complicated way.

The power/level attribute can contain either "on" or "auto", meaning
that the device is always on or that it is subject to automatic runtime
power management (autosuspend). Changing the setting from "auto" to
"on" merely does sets a flag and does pm_runtime_get_sync(); changing
it from "on" to "auto" clears the flag and does pm_runtime_put_sync().

Is there any reason this same mechanism shouldn't be used for all
devices?

Alan Stern

2010-01-18 22:25:10

by Rafael J. Wysocki

[permalink] [raw]
Subject: [Update][RFC][PATCH] PM / Runtime: Add sysfs switch for disabling device run-time PM

On Monday 18 January 2010, Alan Stern wrote:
> On Mon, 18 Jan 2010, Rafael J. Wysocki wrote:
>
> > From: Rafael J. Wysocki <[email protected]>
> > Subject: PM / Runtime: Add sysfs switch for disabling device run-time PM
> >
> > Add new device sysfs attribute, power/runtime, allowing the user
> > space to block the run-time power management of the device. If this
> > attribute is set to "disabled", the driver of the device won't be
> > able to enable run-time power management for it (without breaking the
> > rules).
>
> Ouch. This does nearly the same thing as the power/level attribute in
> the USB subsystem, but in an incompatible and more complicated way.
>
> The power/level attribute can contain either "on" or "auto", meaning
> that the device is always on or that it is subject to automatic runtime
> power management (autosuspend).

It looks like my "disable" is similar to "on", while my "enable" is similar to
"auto". I can use "auto" and "on" just fine.

> Changing the setting from "auto" to "on" merely does sets a flag and does
> pm_runtime_get_sync(); changing it from "on" to "auto" clears the flag and
> does pm_runtime_put_sync().

We can do it almost this way in general, although I think the flag should be
changed under the power.lock.

Updated patch is appended.

Rafael

---
From: Rafael J. Wysocki <[email protected]>
Subject: PM / Runtime: Add sysfs switch for disabling device run-time PM

Add new device sysfs attribute, power/runtime, allowing the user
space to block the run-time power management of the devices. If this
attribute is set to "on", the driver of the device won't be able to power
manage it at run time (without breaking the rules).

Signed-off-by: Rafael J. Wysocki <[email protected]>
---
drivers/base/power/power.h | 4 +++
drivers/base/power/runtime.c | 43 ++++++++++++++++++++++++++++++++++++
drivers/base/power/sysfs.c | 50 +++++++++++++++++++++++++++++++++++++++++++
include/linux/pm.h | 1
4 files changed, 98 insertions(+)

Index: linux-2.6/drivers/base/power/power.h
===================================================================
--- linux-2.6.orig/drivers/base/power/power.h
+++ linux-2.6/drivers/base/power/power.h
@@ -2,11 +2,15 @@

extern void pm_runtime_init(struct device *dev);
extern void pm_runtime_remove(struct device *dev);
+extern void pm_runtime_allow(struct device *dev);
+extern void pm_runtime_forbid(struct device *dev);

#else /* !CONFIG_PM_RUNTIME */

static inline void pm_runtime_init(struct device *dev) {}
static inline void pm_runtime_remove(struct device *dev) {}
+static inline void pm_runtime_allow(struct device *dev) {}
+static inline void pm_runtime_forbid(struct device *dev) {}

#endif /* !CONFIG_PM_RUNTIME */

Index: linux-2.6/drivers/base/power/sysfs.c
===================================================================
--- linux-2.6.orig/drivers/base/power/sysfs.c
+++ linux-2.6/drivers/base/power/sysfs.c
@@ -7,6 +7,21 @@
#include "power.h"

/*
+ * runtime - Report/change current runtime PM setting of the device
+ *
+ * Runtime power management of a device can be blocked with the help of
+ * this attribute. All devices have one of the following two values for
+ * the power/runtime file:
+ *
+ * + "auto\n" to allow the device to be power managed at run time;
+ * + "on\n" to prevent the device from being power managemed at run time;
+ *
+ * The default for all devices is "auto", which means that devices may be
+ * subject to automatic power management, depending on their drivers.
+ * Changing this attribute to "on" prevents the driver from power managing
+ * the device at run time. Doing that while the device is suspended causes
+ * it to be woken up.
+ *
* wakeup - Report/change current wakeup option for device
*
* Some devices support "wakeup" events, which are hardware signals
@@ -59,6 +74,38 @@
static const char enabled[] = "enabled";
static const char disabled[] = "disabled";

+#ifdef CONFIG_PM_RUNTIME
+static const char str_auto[] = "auto";
+static const char str_on[] = "on";
+
+static ssize_t runtime_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ return sprintf(buf, "%s\n",
+ dev->power.runtime_auto ? str_auto : str_on);
+}
+
+static ssize_t runtime_store(struct device * dev, struct device_attribute *attr,
+ const char * buf, size_t n)
+{
+ char *cp;
+ int len = n;
+
+ cp = memchr(buf, '\n', n);
+ if (cp)
+ len = cp - buf;
+ if (len == sizeof str_auto - 1 && strncmp(buf, str_auto, len) == 0)
+ pm_runtime_allow(dev);
+ else if (len == sizeof str_on - 1 && strncmp(buf, str_on, len) == 0)
+ pm_runtime_forbid(dev);
+ else
+ return -EINVAL;
+ return n;
+}
+
+static DEVICE_ATTR(runtime, 0644, runtime_show, runtime_store);
+#endif
+
static ssize_t
wake_show(struct device * dev, struct device_attribute *attr, char * buf)
{
@@ -123,6 +170,9 @@ static DEVICE_ATTR(async, 0644, async_sh
#endif /* CONFIG_PM_SLEEP_ADVANCED_DEBUG */

static struct attribute * power_attrs[] = {
+#ifdef CONFIG_PM_RUNTIME
+ &dev_attr_runtime.attr,
+#endif
&dev_attr_wakeup.attr,
#ifdef CONFIG_PM_SLEEP_ADVANCED_DEBUG
&dev_attr_async.attr,
Index: linux-2.6/include/linux/pm.h
===================================================================
--- linux-2.6.orig/include/linux/pm.h
+++ linux-2.6/include/linux/pm.h
@@ -433,6 +433,7 @@ struct dev_pm_info {
unsigned int request_pending:1;
unsigned int deferred_resume:1;
unsigned int run_wake:1;
+ unsigned int runtime_auto:1;
enum rpm_request request;
enum rpm_status runtime_status;
int runtime_error;
Index: linux-2.6/drivers/base/power/runtime.c
===================================================================
--- linux-2.6.orig/drivers/base/power/runtime.c
+++ linux-2.6/drivers/base/power/runtime.c
@@ -1011,6 +1011,48 @@ void pm_runtime_enable(struct device *de
EXPORT_SYMBOL_GPL(pm_runtime_enable);

/**
+ * pm_runtime_forbid - Block run-time PM of a device.
+ * @dev: Device to handle.
+ *
+ * Increase the device's usage count and clear its power.runtime_auto flag,
+ * so that it cannot be suspended at run time until pm_runtime_allow() is called
+ * for it.
+ */
+void pm_runtime_forbid(struct device *dev)
+{
+ spin_lock_irq(&dev->power.lock);
+ if (!dev->power.runtime_auto)
+ goto out;
+
+ dev->power.runtime_auto = false;
+ atomic_inc(&dev->power.usage_count);
+ __pm_runtime_resume(dev, false);
+
+ out:
+ spin_unlock_irq(&dev->power.lock);
+}
+
+/**
+ * pm_runtime_allow - Unblock run-time PM of a device.
+ * @dev: Device to handle.
+ *
+ * Decrease the device's usage count and set its power.runtime_auto flag.
+ */
+void pm_runtime_allow(struct device *dev)
+{
+ spin_lock_irq(&dev->power.lock);
+ if (dev->power.runtime_auto)
+ goto out;
+
+ dev->power.runtime_auto = true;
+ if (atomic_dec_and_test(&dev->power.usage_count))
+ __pm_runtime_idle(dev);
+
+ out:
+ spin_unlock_irq(&dev->power.lock);
+}
+
+/**
* pm_runtime_init - Initialize run-time PM fields in given device object.
* @dev: Device object to initialize.
*/
@@ -1028,6 +1070,7 @@ void pm_runtime_init(struct device *dev)

atomic_set(&dev->power.child_count, 0);
pm_suspend_ignore_children(dev, false);
+ dev->power.runtime_auto = true;

dev->power.request_pending = false;
dev->power.request = RPM_REQ_NONE;

2010-01-19 15:40:34

by Alan Stern

[permalink] [raw]
Subject: Re: [Update][RFC][PATCH] PM / Runtime: Add sysfs switch for disabling device run-time PM

On Mon, 18 Jan 2010, Rafael J. Wysocki wrote:

> > Ouch. This does nearly the same thing as the power/level attribute in
> > the USB subsystem, but in an incompatible and more complicated way.
> >
> > The power/level attribute can contain either "on" or "auto", meaning
> > that the device is always on or that it is subject to automatic runtime
> > power management (autosuspend).
>
> It looks like my "disable" is similar to "on", while my "enable" is similar to
> "auto". I can use "auto" and "on" just fine.

Good.

> > Changing the setting from "auto" to "on" merely does sets a flag and does
> > pm_runtime_get_sync(); changing it from "on" to "auto" clears the flag and
> > does pm_runtime_put_sync().
>
> We can do it almost this way in general, although I think the flag should be
> changed under the power.lock.

Yes. I was using the device semaphore, but the power.lock is more
appropriate here.

> Updated patch is appended.

Why change the name from "level" to "runtime"?

> /*
> + * runtime - Report/change current runtime PM setting of the device
> + *
> + * Runtime power management of a device can be blocked with the help of
> + * this attribute. All devices have one of the following two values for
> + * the power/runtime file:
> + *
> + * + "auto\n" to allow the device to be power managed at run time;
> + * + "on\n" to prevent the device from being power managemed at run time;

---------------------------------------------------------------^^ typo

Don't forget to add an entry to Documentation/ABI/testing/.

Alan Stern

2010-01-19 20:32:08

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [Update][RFC][PATCH] PM / Runtime: Add sysfs switch for disabling device run-time PM

On Tuesday 19 January 2010, Alan Stern wrote:
> On Mon, 18 Jan 2010, Rafael J. Wysocki wrote:
>
> > > Ouch. This does nearly the same thing as the power/level attribute in
> > > the USB subsystem, but in an incompatible and more complicated way.
> > >
> > > The power/level attribute can contain either "on" or "auto", meaning
> > > that the device is always on or that it is subject to automatic runtime
> > > power management (autosuspend).
> >
> > It looks like my "disable" is similar to "on", while my "enable" is similar to
> > "auto". I can use "auto" and "on" just fine.
>
> Good.
>
> > > Changing the setting from "auto" to "on" merely does sets a flag and does
> > > pm_runtime_get_sync(); changing it from "on" to "auto" clears the flag and
> > > does pm_runtime_put_sync().
> >
> > We can do it almost this way in general, although I think the flag should be
> > changed under the power.lock.
>
> Yes. I was using the device semaphore, but the power.lock is more
> appropriate here.
>
> > Updated patch is appended.
>
> Why change the name from "level" to "runtime"?

Because it wouldn't be really obvious what "level" applied to. For example,
some people might expect the values of "level" to correspond to PCI power
states for PCI devices and so on, so it might turn out to be confusing.

> > /*
> > + * runtime - Report/change current runtime PM setting of the device
> > + *
> > + * Runtime power management of a device can be blocked with the help of
> > + * this attribute. All devices have one of the following two values for
> > + * the power/runtime file:
> > + *
> > + * + "auto\n" to allow the device to be power managed at run time;
> > + * + "on\n" to prevent the device from being power managemed at run time;
>
> ---------------------------------------------------------------^^ typo

Ah, thanks

> Don't forget to add an entry to Documentation/ABI/testing/.

OK

Rafael

2010-01-19 23:26:55

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [Update][RFC][PATCH] PM / Runtime: Add sysfs switch for disabling device run-time PM

On Tuesday 19 January 2010, Rafael J. Wysocki wrote:
> On Tuesday 19 January 2010, Alan Stern wrote:
> > On Mon, 18 Jan 2010, Rafael J. Wysocki wrote:
...
> > Don't forget to add an entry to Documentation/ABI/testing/.
>
> OK

Actually, not in this patch.

There doesn't seem to be room for the /sys/devices/.../power/* attributes at
all right now and I don't think this particular patch is the right one for
adding them all.

Rafael

2010-01-20 03:18:20

by Greg KH

[permalink] [raw]
Subject: Re: [RFC][PATCH] PM / Runtime: Add sysfs switch for disabling device run-time PM

On Mon, Jan 18, 2010 at 01:29:55AM +0100, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <[email protected]>
> Subject: PM / Runtime: Add sysfs switch for disabling device run-time PM
>
> Add new device sysfs attribute, power/runtime, allowing the user
> space to block the run-time power management of the device. If this
> attribute is set to "disabled", the driver of the device won't be
> able to enable run-time power management for it (without breaking the
> rules).
>
> Signed-off-by: Rafael J. Wysocki <[email protected]>
> ---
> drivers/base/power/power.h | 4 +
> drivers/base/power/runtime.c | 107 +++++++++++++++++++++++++++++++++----------
> drivers/base/power/sysfs.c | 46 ++++++++++++++++++
> include/linux/pm.h | 1
> include/linux/pm_runtime.h | 11 +---
> 5 files changed, 139 insertions(+), 30 deletions(-)

You forgot to also add an entry into Documentation/ABI/ for when you
add/change a sysfs file :)

thanks,

greg k-h

2010-01-20 15:09:09

by Alan Stern

[permalink] [raw]
Subject: Re: [Update][RFC][PATCH] PM / Runtime: Add sysfs switch for disabling device run-time PM

On Tue, 19 Jan 2010, Rafael J. Wysocki wrote:

> > Why change the name from "level" to "runtime"?
>
> Because it wouldn't be really obvious what "level" applied to. For example,
> some people might expect the values of "level" to correspond to PCI power
> states for PCI devices and so on, so it might turn out to be confusing.

But is the name "runtime" any clearer? Especially to people who have
never come across the phrase "runtime power management"?

Does anybody have a suggestion for a better attribute name?

Alan Stern

2010-01-20 20:55:15

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [RFC][PATCH] PM / Runtime: Add sysfs switch for disabling device run-time PM

On Wednesday 20 January 2010, Greg KH wrote:
> On Mon, Jan 18, 2010 at 01:29:55AM +0100, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <[email protected]>
> > Subject: PM / Runtime: Add sysfs switch for disabling device run-time PM
> >
> > Add new device sysfs attribute, power/runtime, allowing the user
> > space to block the run-time power management of the device. If this
> > attribute is set to "disabled", the driver of the device won't be
> > able to enable run-time power management for it (without breaking the
> > rules).
> >
> > Signed-off-by: Rafael J. Wysocki <[email protected]>
> > ---
> > drivers/base/power/power.h | 4 +
> > drivers/base/power/runtime.c | 107 +++++++++++++++++++++++++++++++++----------
> > drivers/base/power/sysfs.c | 46 ++++++++++++++++++
> > include/linux/pm.h | 1
> > include/linux/pm_runtime.h | 11 +---
> > 5 files changed, 139 insertions(+), 30 deletions(-)
>
> You forgot to also add an entry into Documentation/ABI/ for when you
> add/change a sysfs file :)

There's no description for /sys/devices/.../power/wakeup in there too and
that one has been present for quite some time already.

I'd prefer adding them both in a separate patch if you don't mind.

Rafael

2010-01-20 21:09:08

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [Update][RFC][PATCH] PM / Runtime: Add sysfs switch for disabling device run-time PM

On Wednesday 20 January 2010, Alan Stern wrote:
> On Tue, 19 Jan 2010, Rafael J. Wysocki wrote:
>
> > > Why change the name from "level" to "runtime"?
> >
> > Because it wouldn't be really obvious what "level" applied to. For example,
> > some people might expect the values of "level" to correspond to PCI power
> > states for PCI devices and so on, so it might turn out to be confusing.
>
> But is the name "runtime" any clearer? Especially to people who have
> never come across the phrase "runtime power management"?
>
> Does anybody have a suggestion for a better attribute name?

Apparently not, but I can try. ;-)

* status

* power (power/power looks weird, but the values would make sense)

* control

No more ideas for now.

Rafael

2010-01-20 21:37:39

by Alan Stern

[permalink] [raw]
Subject: Re: [Update][RFC][PATCH] PM / Runtime: Add sysfs switch for disabling device run-time PM

On Wed, 20 Jan 2010, Rafael J. Wysocki wrote:

> > Does anybody have a suggestion for a better attribute name?
>
> Apparently not, but I can try. ;-)
>
> * status
>
> * power (power/power looks weird, but the values would make sense)
>
> * control
>
> No more ideas for now.

I vote for "control".

Alan Stern

2010-01-20 22:17:22

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [Update][RFC][PATCH] PM / Runtime: Add sysfs switch for disabling device run-time PM

On Wednesday 20 January 2010, Alan Stern wrote:
> On Wed, 20 Jan 2010, Rafael J. Wysocki wrote:
>
> > > Does anybody have a suggestion for a better attribute name?
> >
> > Apparently not, but I can try. ;-)
> >
> > * status
> >
> > * power (power/power looks weird, but the values would make sense)
> >
> > * control
> >
> > No more ideas for now.
>
> I vote for "control".

OK, if no one objects, I'll use that in the next "release" of the patch.

Rafael

2010-01-20 22:56:32

by Greg KH

[permalink] [raw]
Subject: Re: [RFC][PATCH] PM / Runtime: Add sysfs switch for disabling device run-time PM

On Wed, Jan 20, 2010 at 09:55:36PM +0100, Rafael J. Wysocki wrote:
> On Wednesday 20 January 2010, Greg KH wrote:
> > On Mon, Jan 18, 2010 at 01:29:55AM +0100, Rafael J. Wysocki wrote:
> > > From: Rafael J. Wysocki <[email protected]>
> > > Subject: PM / Runtime: Add sysfs switch for disabling device run-time PM
> > >
> > > Add new device sysfs attribute, power/runtime, allowing the user
> > > space to block the run-time power management of the device. If this
> > > attribute is set to "disabled", the driver of the device won't be
> > > able to enable run-time power management for it (without breaking the
> > > rules).
> > >
> > > Signed-off-by: Rafael J. Wysocki <[email protected]>
> > > ---
> > > drivers/base/power/power.h | 4 +
> > > drivers/base/power/runtime.c | 107 +++++++++++++++++++++++++++++++++----------
> > > drivers/base/power/sysfs.c | 46 ++++++++++++++++++
> > > include/linux/pm.h | 1
> > > include/linux/pm_runtime.h | 11 +---
> > > 5 files changed, 139 insertions(+), 30 deletions(-)
> >
> > You forgot to also add an entry into Documentation/ABI/ for when you
> > add/change a sysfs file :)
>
> There's no description for /sys/devices/.../power/wakeup in there too and
> that one has been present for quite some time already.
>
> I'd prefer adding them both in a separate patch if you don't mind.

That's fine with me, as long as you add them :)

thanks,

greg k-h

2010-01-21 23:01:24

by Rafael J. Wysocki

[permalink] [raw]
Subject: [PATCH 0/2] sysfs switch for disabling device run-time PM

Hi,

The first of the following two patches adds a sysfs switch for controlling the
run-time power management of devices and the second one documents the sysfs
attributes in /sys/devices/.../power/ (including the new one).

Thanks,
Rafael

2010-01-21 23:01:22

by Rafael J. Wysocki

[permalink] [raw]
Subject: [PATCH 2/2] PM: Document device power attributes in sysfs

From: Rafael J. Wysocki <[email protected]>

There are sysfs attributes in /sys/devices/.../power/ that haven't
been documented yet in Documentation/ABI/. Document them as
appropriate.

Signed-off-by: Rafael J. Wysocki <[email protected]>
---
Documentation/ABI/testing/sysfs-devices-power | 53 ++++++++++++++++++++++++++
1 file changed, 53 insertions(+)

Index: linux-2.6/Documentation/ABI/testing/sysfs-devices-power
===================================================================
--- /dev/null
+++ linux-2.6/Documentation/ABI/testing/sysfs-devices-power
@@ -0,0 +1,53 @@
+What: /sys/devices/.../power/
+Date: January 2009
+Contact: Rafael J. Wysocki <[email protected]>
+Description:
+ The /sys/devices/.../power directory contains attributes
+ allowing the user space to check and modify some power
+ management related properties of given device.
+
+What: /sys/devices/.../power/wakeup
+Date: January 2009
+Contact: Rafael J. Wysocki <[email protected]>
+Description:
+ The /sys/devices/.../power/wakeup attribute allows the user
+ space to check if the device is enabled to wake up the system
+ from sleep states, such as the memory sleep state (suspend to
+ RAM) and hibernation (suspend to disk), and to enable or disable
+ it to do that as desired.
+
+ Some devices support "wakeup" events, which are hardware signals
+ used to activate the system from a sleep state. Such devices
+ have one of the following two values for the sysfs power/wakeup
+ file:
+
+ + "enabled\n" to issue the events;
+ + "disabled\n" not to do so;
+
+ In that cases the user space can change the setting represented
+ by the contents of this file by writing either "enabled", or
+ "disabled" to it.
+
+ For the devices that are not capable of generating system wakeup
+ events this file contains "\n". In that cases the user space
+ cannot modify the contents of this file and the device cannot be
+ enabled to wake up the system.
+
+What: /sys/devices/.../power/control
+Date: January 2009
+Contact: Rafael J. Wysocki <[email protected]>
+Description:
+ The /sys/devices/.../power/control attribute allows the user
+ space to control the run-time power management of the device.
+
+ All devices have one of the following two values for the
+ power/control file:
+
+ + "auto\n" to allow the device to be power managed at run time;
+ + "on\n" to prevent the device from being power managed;
+
+ The default for all devices is "auto", which means that they may
+ be subject to automatic power management, depending on their
+ drivers. Changing this attribute to "on" prevents the driver
+ from power managing the device at run time. Doing that while
+ the device is suspended causes it to be woken up.

2010-01-21 23:01:21

by Rafael J. Wysocki

[permalink] [raw]
Subject: [PATCH 1/2] PM / Runtime: Add sysfs switch for disabling device run-time PM (rev. 2)

From: Rafael J. Wysocki <[email protected]>

Add new device sysfs attribute, power/control, allowing the user
space to block the run-time power management of devices. If this
attribute is set to "on", the driver of the device won't be able to power
manage it at run time (without breaking the rules) and the device will
always be in the full power state (except when the entire system goes
into a sleep state).

Signed-off-by: Rafael J. Wysocki <[email protected]>
---
drivers/base/power/power.h | 4 +++
drivers/base/power/runtime.c | 43 ++++++++++++++++++++++++++++++++++++
drivers/base/power/sysfs.c | 50 +++++++++++++++++++++++++++++++++++++++++++
include/linux/pm.h | 1
4 files changed, 98 insertions(+)

Index: linux-2.6/drivers/base/power/power.h
===================================================================
--- linux-2.6.orig/drivers/base/power/power.h
+++ linux-2.6/drivers/base/power/power.h
@@ -2,11 +2,15 @@

extern void pm_runtime_init(struct device *dev);
extern void pm_runtime_remove(struct device *dev);
+extern void pm_runtime_allow(struct device *dev);
+extern void pm_runtime_forbid(struct device *dev);

#else /* !CONFIG_PM_RUNTIME */

static inline void pm_runtime_init(struct device *dev) {}
static inline void pm_runtime_remove(struct device *dev) {}
+static inline void pm_runtime_allow(struct device *dev) {}
+static inline void pm_runtime_forbid(struct device *dev) {}

#endif /* !CONFIG_PM_RUNTIME */

Index: linux-2.6/drivers/base/power/sysfs.c
===================================================================
--- linux-2.6.orig/drivers/base/power/sysfs.c
+++ linux-2.6/drivers/base/power/sysfs.c
@@ -7,6 +7,21 @@
#include "power.h"

/*
+ * control - Report/change current runtime PM setting of the device
+ *
+ * Runtime power management of a device can be blocked with the help of
+ * this attribute. All devices have one of the following two values for
+ * the power/control file:
+ *
+ * + "auto\n" to allow the device to be power managed at run time;
+ * + "on\n" to prevent the device from being power managed at run time;
+ *
+ * The default for all devices is "auto", which means that devices may be
+ * subject to automatic power management, depending on their drivers.
+ * Changing this attribute to "on" prevents the driver from power managing
+ * the device at run time. Doing that while the device is suspended causes
+ * it to be woken up.
+ *
* wakeup - Report/change current wakeup option for device
*
* Some devices support "wakeup" events, which are hardware signals
@@ -59,6 +74,38 @@
static const char enabled[] = "enabled";
static const char disabled[] = "disabled";

+#ifdef CONFIG_PM_RUNTIME
+static const char ctrl_auto[] = "auto";
+static const char ctrl_on[] = "on";
+
+static ssize_t control_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ return sprintf(buf, "%s\n",
+ dev->power.runtime_auto ? ctrl_auto : ctrl_on);
+}
+
+static ssize_t control_store(struct device * dev, struct device_attribute *attr,
+ const char * buf, size_t n)
+{
+ char *cp;
+ int len = n;
+
+ cp = memchr(buf, '\n', n);
+ if (cp)
+ len = cp - buf;
+ if (len == sizeof ctrl_auto - 1 && strncmp(buf, ctrl_auto, len) == 0)
+ pm_runtime_allow(dev);
+ else if (len == sizeof ctrl_on - 1 && strncmp(buf, ctrl_on, len) == 0)
+ pm_runtime_forbid(dev);
+ else
+ return -EINVAL;
+ return n;
+}
+
+static DEVICE_ATTR(control, 0644, control_show, control_store);
+#endif
+
static ssize_t
wake_show(struct device * dev, struct device_attribute *attr, char * buf)
{
@@ -123,6 +170,9 @@ static DEVICE_ATTR(async, 0644, async_sh
#endif /* CONFIG_PM_SLEEP_ADVANCED_DEBUG */

static struct attribute * power_attrs[] = {
+#ifdef CONFIG_PM_RUNTIME
+ &dev_attr_control.attr,
+#endif
&dev_attr_wakeup.attr,
#ifdef CONFIG_PM_SLEEP_ADVANCED_DEBUG
&dev_attr_async.attr,
Index: linux-2.6/include/linux/pm.h
===================================================================
--- linux-2.6.orig/include/linux/pm.h
+++ linux-2.6/include/linux/pm.h
@@ -433,6 +433,7 @@ struct dev_pm_info {
unsigned int request_pending:1;
unsigned int deferred_resume:1;
unsigned int run_wake:1;
+ unsigned int runtime_auto:1;
enum rpm_request request;
enum rpm_status runtime_status;
int runtime_error;
Index: linux-2.6/drivers/base/power/runtime.c
===================================================================
--- linux-2.6.orig/drivers/base/power/runtime.c
+++ linux-2.6/drivers/base/power/runtime.c
@@ -1011,6 +1011,48 @@ void pm_runtime_enable(struct device *de
EXPORT_SYMBOL_GPL(pm_runtime_enable);

/**
+ * pm_runtime_forbid - Block run-time PM of a device.
+ * @dev: Device to handle.
+ *
+ * Increase the device's usage count and clear its power.runtime_auto flag,
+ * so that it cannot be suspended at run time until pm_runtime_allow() is called
+ * for it.
+ */
+void pm_runtime_forbid(struct device *dev)
+{
+ spin_lock_irq(&dev->power.lock);
+ if (!dev->power.runtime_auto)
+ goto out;
+
+ dev->power.runtime_auto = false;
+ atomic_inc(&dev->power.usage_count);
+ __pm_runtime_resume(dev, false);
+
+ out:
+ spin_unlock_irq(&dev->power.lock);
+}
+
+/**
+ * pm_runtime_allow - Unblock run-time PM of a device.
+ * @dev: Device to handle.
+ *
+ * Decrease the device's usage count and set its power.runtime_auto flag.
+ */
+void pm_runtime_allow(struct device *dev)
+{
+ spin_lock_irq(&dev->power.lock);
+ if (dev->power.runtime_auto)
+ goto out;
+
+ dev->power.runtime_auto = true;
+ if (atomic_dec_and_test(&dev->power.usage_count))
+ __pm_runtime_idle(dev);
+
+ out:
+ spin_unlock_irq(&dev->power.lock);
+}
+
+/**
* pm_runtime_init - Initialize run-time PM fields in given device object.
* @dev: Device object to initialize.
*/
@@ -1028,6 +1070,7 @@ void pm_runtime_init(struct device *dev)

atomic_set(&dev->power.child_count, 0);
pm_suspend_ignore_children(dev, false);
+ dev->power.runtime_auto = true;

dev->power.request_pending = false;
dev->power.request = RPM_REQ_NONE;

2010-01-22 03:02:30

by Alan Stern

[permalink] [raw]
Subject: Re: [PATCH 1/2] PM / Runtime: Add sysfs switch for disabling device run-time PM (rev. 2)

On Thu, 21 Jan 2010, Rafael J. Wysocki wrote:

> From: Rafael J. Wysocki <[email protected]>
>
> Add new device sysfs attribute, power/control, allowing the user
> space to block the run-time power management of devices. If this
> attribute is set to "on", the driver of the device won't be able to power
> manage it at run time (without breaking the rules) and the device will
> always be in the full power state (except when the entire system goes
> into a sleep state).

...

> --- linux-2.6.orig/drivers/base/power/power.h
> +++ linux-2.6/drivers/base/power/power.h
> @@ -2,11 +2,15 @@
>
> extern void pm_runtime_init(struct device *dev);
> extern void pm_runtime_remove(struct device *dev);
> +extern void pm_runtime_allow(struct device *dev);
> +extern void pm_runtime_forbid(struct device *dev);

Can you write a follow-up patch that moves these declarations to a
public header file like include/linux/pm_runtime.h and EXPORTs the two
new routines? It is sometimes useful for drivers to be able to call
the routines directly.

For example, we disable runtime PM for most USB devices by default.
But a few drivers may know that their devices are able to handle it, so
they want to change the default setting when they are bound.

Alan Stern

2010-01-22 20:55:40

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH 1/2] PM / Runtime: Add sysfs switch for disabling device run-time PM (rev. 2)

On Friday 22 January 2010, Alan Stern wrote:
> On Thu, 21 Jan 2010, Rafael J. Wysocki wrote:
>
> > From: Rafael J. Wysocki <[email protected]>
> >
> > Add new device sysfs attribute, power/control, allowing the user
> > space to block the run-time power management of devices. If this
> > attribute is set to "on", the driver of the device won't be able to power
> > manage it at run time (without breaking the rules) and the device will
> > always be in the full power state (except when the entire system goes
> > into a sleep state).
>
> ...
>
> > --- linux-2.6.orig/drivers/base/power/power.h
> > +++ linux-2.6/drivers/base/power/power.h
> > @@ -2,11 +2,15 @@
> >
> > extern void pm_runtime_init(struct device *dev);
> > extern void pm_runtime_remove(struct device *dev);
> > +extern void pm_runtime_allow(struct device *dev);
> > +extern void pm_runtime_forbid(struct device *dev);
>
> Can you write a follow-up patch that moves these declarations to a
> public header file like include/linux/pm_runtime.h and EXPORTs the two
> new routines? It is sometimes useful for drivers to be able to call
> the routines directly.

I can put them in there in this patch just as well, but I'm not sure it's the
right thing to do.

> For example, we disable runtime PM for most USB devices by default.
> But a few drivers may know that their devices are able to handle it, so
> they want to change the default setting when they are bound.

Runtime PM is disabled for all devices by default unless the driver enables
it. The setting in power/control is to override the driver's choice, so that
it can be disabled even if the driver tries to enable it.

IOW, the user space is the owner of the power.runtime_auto flag and I don't
think we should allow drivers to modify it.

Rafael

2010-01-22 22:45:59

by Alan Stern

[permalink] [raw]
Subject: Re: [PATCH 1/2] PM / Runtime: Add sysfs switch for disabling device run-time PM (rev. 2)

On Fri, 22 Jan 2010, Rafael J. Wysocki wrote:

> > For example, we disable runtime PM for most USB devices by default.
> > But a few drivers may know that their devices are able to handle it, so
> > they want to change the default setting when they are bound.
>
> Runtime PM is disabled for all devices by default unless the driver enables
> it. The setting in power/control is to override the driver's choice, so that
> it can be disabled even if the driver tries to enable it.
>
> IOW, the user space is the owner of the power.runtime_auto flag and I don't
> think we should allow drivers to modify it.

Okay, I understand your point of view. It means I'll have to make some
changes to the USB code. No problem.

Alan Stern

2010-01-22 22:55:28

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH 1/2] PM / Runtime: Add sysfs switch for disabling device run-time PM (rev. 2)

On Friday 22 January 2010, Alan Stern wrote:
> On Fri, 22 Jan 2010, Rafael J. Wysocki wrote:
>
> > > For example, we disable runtime PM for most USB devices by default.
> > > But a few drivers may know that their devices are able to handle it, so
> > > they want to change the default setting when they are bound.
> >
> > Runtime PM is disabled for all devices by default unless the driver enables
> > it. The setting in power/control is to override the driver's choice, so that
> > it can be disabled even if the driver tries to enable it.
> >
> > IOW, the user space is the owner of the power.runtime_auto flag and I don't
> > think we should allow drivers to modify it.
>
> Okay, I understand your point of view. It means I'll have to make some
> changes to the USB code. No problem.

Great, thanks a lot.

Rafael

2010-01-23 03:22:13

by Alan Stern

[permalink] [raw]
Subject: Re: [PATCH 1/2] PM / Runtime: Add sysfs switch for disabling device run-time PM (rev. 2)

On Fri, 22 Jan 2010, Rafael J. Wysocki wrote:

> > For example, we disable runtime PM for most USB devices by default.
> > But a few drivers may know that their devices are able to handle it, so
> > they want to change the default setting when they are bound.
>
> Runtime PM is disabled for all devices by default unless the driver enables
> it. The setting in power/control is to override the driver's choice, so that
> it can be disabled even if the driver tries to enable it.
>
> IOW, the user space is the owner of the power.runtime_auto flag and I don't
> think we should allow drivers to modify it.

After more thought, I changed my mind about this. The ability to
override the driver's choice means that the user should be able to
enable runtime PM even if the driver tries to disable it, as well as
the other way 'round.

In other words, it's not productive to say the user is the owner of the
runtime_auto flag. What we _really_ want is to put the user in charge
of whether or not a device is subject to runtime PM.

First of all, note that if the driver doesn't support runtime PM then
it makes no difference what value runtime_auto has. The issue is moot.

So suppose the driver does support runtime PM. In this case, it's
irrelevant that runtime PM starts out disabled by default when the
device structure is initialized. The bus subsystem and driver will
reinitialize the settings in the way they think best.

But if the driver decides to disable runtime PM and pm_runtime_forbid()
isn't EXPORTed, then the driver will be forced to implement its
decision by leaving disable_depth > 0 -- which means there's nothing
the user can do. The power/control attribute won't help. On the other
hand, if the driver decides to disable runtime PM and it can do so by
calling pm_runtime_forbid(), then the user can override the decision.

Alan Stern

2010-01-23 12:14:30

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH 1/2] PM / Runtime: Add sysfs switch for disabling device run-time PM (rev. 2)

On Saturday 23 January 2010, Alan Stern wrote:
> On Fri, 22 Jan 2010, Rafael J. Wysocki wrote:
>
> > > For example, we disable runtime PM for most USB devices by default.
> > > But a few drivers may know that their devices are able to handle it, so
> > > they want to change the default setting when they are bound.
> >
> > Runtime PM is disabled for all devices by default unless the driver enables
> > it. The setting in power/control is to override the driver's choice, so that
> > it can be disabled even if the driver tries to enable it.
> >
> > IOW, the user space is the owner of the power.runtime_auto flag and I don't
> > think we should allow drivers to modify it.
>
> After more thought, I changed my mind about this. The ability to
> override the driver's choice means that the user should be able to
> enable runtime PM even if the driver tries to disable it, as well as
> the other way 'round.
>
> In other words, it's not productive to say the user is the owner of the
> runtime_auto flag. What we _really_ want is to put the user in charge
> of whether or not a device is subject to runtime PM.
>
> First of all, note that if the driver doesn't support runtime PM then
> it makes no difference what value runtime_auto has. The issue is moot.
>
> So suppose the driver does support runtime PM. In this case, it's
> irrelevant that runtime PM starts out disabled by default when the
> device structure is initialized. The bus subsystem and driver will
> reinitialize the settings in the way they think best.
>
> But if the driver decides to disable runtime PM and pm_runtime_forbid()
> isn't EXPORTed, then the driver will be forced to implement its
> decision by leaving disable_depth > 0 -- which means there's nothing
> the user can do. The power/control attribute won't help. On the other
> hand, if the driver decides to disable runtime PM and it can do so by
> calling pm_runtime_forbid(), then the user can override the decision.

Hmm, OK.

Of course, if the driver decides to leave disable_depth > 0, the user still
won't be able to do anything about that, but if I understand correctly, you
want drivers to have the option to use pm_runtime_forbid() initially, right
after pm_runtime_enable(), so that the user space can turn the runtime PM
on if desired via sysfs.

That makes sense. I have a usage case for that myself. :-)

Updated patch is appended.

Rafael

---
From: Rafael J. Wysocki <[email protected]>
Subject: PM / Runtime: Add sysfs switch for disabling device run-time PM

Add new device sysfs attribute, power/control, allowing the user
space to block the run-time power management of the devices. If this
attribute is set to "on", the driver of the device won't be able to power
manage it at run time (without breaking the rules) and the device will
always be in the full power state (except when the entire system goes
into a sleep state).

Signed-off-by: Rafael J. Wysocki <[email protected]>
---
drivers/base/power/runtime.c | 45 +++++++++++++++++++++++++++++++++++++
drivers/base/power/sysfs.c | 51 +++++++++++++++++++++++++++++++++++++++++++
include/linux/pm.h | 1
include/linux/pm_runtime.h | 4 +++
4 files changed, 101 insertions(+)

Index: linux-2.6/drivers/base/power/sysfs.c
===================================================================
--- linux-2.6.orig/drivers/base/power/sysfs.c
+++ linux-2.6/drivers/base/power/sysfs.c
@@ -4,9 +4,25 @@

#include <linux/device.h>
#include <linux/string.h>
+#include <linux/pm_runtime.h>
#include "power.h"

/*
+ * control - Report/change current runtime PM setting of the device
+ *
+ * Runtime power management of a device can be blocked with the help of
+ * this attribute. All devices have one of the following two values for
+ * the power/control file:
+ *
+ * + "auto\n" to allow the device to be power managed at run time;
+ * + "on\n" to prevent the device from being power managed at run time;
+ *
+ * The default for all devices is "auto", which means that devices may be
+ * subject to automatic power management, depending on their drivers.
+ * Changing this attribute to "on" prevents the driver from power managing
+ * the device at run time. Doing that while the device is suspended causes
+ * it to be woken up.
+ *
* wakeup - Report/change current wakeup option for device
*
* Some devices support "wakeup" events, which are hardware signals
@@ -43,6 +59,38 @@
static const char enabled[] = "enabled";
static const char disabled[] = "disabled";

+#ifdef CONFIG_PM_RUNTIME
+static const char ctrl_auto[] = "auto";
+static const char ctrl_on[] = "on";
+
+static ssize_t control_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ return sprintf(buf, "%s\n",
+ dev->power.runtime_auto ? ctrl_auto : ctrl_on);
+}
+
+static ssize_t control_store(struct device * dev, struct device_attribute *attr,
+ const char * buf, size_t n)
+{
+ char *cp;
+ int len = n;
+
+ cp = memchr(buf, '\n', n);
+ if (cp)
+ len = cp - buf;
+ if (len == sizeof ctrl_auto - 1 && strncmp(buf, ctrl_auto, len) == 0)
+ pm_runtime_allow(dev);
+ else if (len == sizeof ctrl_on - 1 && strncmp(buf, ctrl_on, len) == 0)
+ pm_runtime_forbid(dev);
+ else
+ return -EINVAL;
+ return n;
+}
+
+static DEVICE_ATTR(control, 0644, control_show, control_store);
+#endif
+
static ssize_t
wake_show(struct device * dev, struct device_attribute *attr, char * buf)
{
@@ -79,6 +127,9 @@ static DEVICE_ATTR(wakeup, 0644, wake_sh


static struct attribute * power_attrs[] = {
+#ifdef CONFIG_PM_RUNTIME
+ &dev_attr_control.attr,
+#endif
&dev_attr_wakeup.attr,
NULL,
};
Index: linux-2.6/include/linux/pm.h
===================================================================
--- linux-2.6.orig/include/linux/pm.h
+++ linux-2.6/include/linux/pm.h
@@ -430,6 +430,7 @@ struct dev_pm_info {
unsigned int request_pending:1;
unsigned int deferred_resume:1;
unsigned int run_wake:1;
+ unsigned int runtime_auto:1;
enum rpm_request request;
enum rpm_status runtime_status;
int runtime_error;
Index: linux-2.6/drivers/base/power/runtime.c
===================================================================
--- linux-2.6.orig/drivers/base/power/runtime.c
+++ linux-2.6/drivers/base/power/runtime.c
@@ -1011,6 +1011,50 @@ void pm_runtime_enable(struct device *de
EXPORT_SYMBOL_GPL(pm_runtime_enable);

/**
+ * pm_runtime_forbid - Block run-time PM of a device.
+ * @dev: Device to handle.
+ *
+ * Increase the device's usage count and clear its power.runtime_auto flag,
+ * so that it cannot be suspended at run time until pm_runtime_allow() is called
+ * for it.
+ */
+void pm_runtime_forbid(struct device *dev)
+{
+ spin_lock_irq(&dev->power.lock);
+ if (!dev->power.runtime_auto)
+ goto out;
+
+ dev->power.runtime_auto = false;
+ atomic_inc(&dev->power.usage_count);
+ __pm_runtime_resume(dev, false);
+
+ out:
+ spin_unlock_irq(&dev->power.lock);
+}
+EXPORT_SYMBOL_GPL(pm_runtime_forbid);
+
+/**
+ * pm_runtime_allow - Unblock run-time PM of a device.
+ * @dev: Device to handle.
+ *
+ * Decrease the device's usage count and set its power.runtime_auto flag.
+ */
+void pm_runtime_allow(struct device *dev)
+{
+ spin_lock_irq(&dev->power.lock);
+ if (dev->power.runtime_auto)
+ goto out;
+
+ dev->power.runtime_auto = true;
+ if (atomic_dec_and_test(&dev->power.usage_count))
+ __pm_runtime_idle(dev);
+
+ out:
+ spin_unlock_irq(&dev->power.lock);
+}
+EXPORT_SYMBOL_GPL(pm_runtime_allow);
+
+/**
* pm_runtime_init - Initialize run-time PM fields in given device object.
* @dev: Device object to initialize.
*/
@@ -1028,6 +1072,7 @@ void pm_runtime_init(struct device *dev)

atomic_set(&dev->power.child_count, 0);
pm_suspend_ignore_children(dev, false);
+ dev->power.runtime_auto = true;

dev->power.request_pending = false;
dev->power.request = RPM_REQ_NONE;
Index: linux-2.6/include/linux/pm_runtime.h
===================================================================
--- linux-2.6.orig/include/linux/pm_runtime.h
+++ linux-2.6/include/linux/pm_runtime.h
@@ -28,6 +28,8 @@ extern int __pm_runtime_set_status(struc
extern int pm_runtime_barrier(struct device *dev);
extern void pm_runtime_enable(struct device *dev);
extern void __pm_runtime_disable(struct device *dev, bool check_resume);
+extern void pm_runtime_allow(struct device *dev);
+extern void pm_runtime_forbid(struct device *dev);

static inline bool pm_children_suspended(struct device *dev)
{
@@ -78,6 +80,8 @@ static inline int __pm_runtime_set_statu
static inline int pm_runtime_barrier(struct device *dev) { return 0; }
static inline void pm_runtime_enable(struct device *dev) {}
static inline void __pm_runtime_disable(struct device *dev, bool c) {}
+static inline void pm_runtime_allow(struct device *dev) {}
+static inline void pm_runtime_forbid(struct device *dev) {}

static inline bool pm_children_suspended(struct device *dev) { return false; }
static inline void pm_suspend_ignore_children(struct device *dev, bool en) {}

2010-01-23 15:20:20

by Alan Stern

[permalink] [raw]
Subject: Re: [PATCH 1/2] PM / Runtime: Add sysfs switch for disabling device run-time PM (rev. 2)

On Sat, 23 Jan 2010, Rafael J. Wysocki wrote:

> Hmm, OK.
>
> Of course, if the driver decides to leave disable_depth > 0, the user still
> won't be able to do anything about that, but if I understand correctly, you
> want drivers to have the option to use pm_runtime_forbid() initially, right
> after pm_runtime_enable(), so that the user space can turn the runtime PM
> on if desired via sysfs.
>
> That makes sense. I have a usage case for that myself. :-)
>
> Updated patch is appended.

It looks good.

Acked-by: Alan Stern <[email protected]>

Alan Stern

2010-01-23 20:07:48

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH 1/2] PM / Runtime: Add sysfs switch for disabling device run-time PM (rev. 2)

On Saturday 23 January 2010, Alan Stern wrote:
> On Sat, 23 Jan 2010, Rafael J. Wysocki wrote:
>
> > Hmm, OK.
> >
> > Of course, if the driver decides to leave disable_depth > 0, the user still
> > won't be able to do anything about that, but if I understand correctly, you
> > want drivers to have the option to use pm_runtime_forbid() initially, right
> > after pm_runtime_enable(), so that the user space can turn the runtime PM
> > on if desired via sysfs.
> >
> > That makes sense. I have a usage case for that myself. :-)
> >
> > Updated patch is appended.
>
> It looks good.
>
> Acked-by: Alan Stern <[email protected]>

Thanks!

Rafael

2010-01-24 15:56:15

by Jesse Barnes

[permalink] [raw]
Subject: Re: [PATCH 1/2] PM / Runtime: Add sysfs switch for disabling device run-time PM (rev. 2)



"Rafael J. Wysocki" <[email protected]> wrote:
>On Saturday 23 January 2010, Alan Stern wrote:>> On Sat, 23 Jan 2010, Rafael J. Wysocki wrote:>> >> > Hmm, OK.>> > >> > Of course, if the driver decides to leave disable_depth > 0, the user still>> > won't be able to do anything about that, but if I understand correctly, you>> > want drivers to have the option to use pm_runtime_forbid() initially, right>> > after pm_runtime_enable(), so that the user space can turn the runtime PM>> > on if desired via sysfs.>> > >> > That makes sense. I have a usage case for that myself. :-)>> > >> > Updated patch is appended.>> >> It looks good.>> >> Acked-by: Alan Stern <[email protected]>>>Thanks!>>Rafael>????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2010-02-04 22:46:35

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH 1/2] PM / Runtime: Add sysfs switch for disabling device run-time PM (rev. 2)

Hi!

> /*
> + * control - Report/change current runtime PM setting of the device
> + *
> + * Runtime power management of a device can be blocked with the help of
> + * this attribute. All devices have one of the following two values for
> + * the power/control file:
> + *
> + * + "auto\n" to allow the device to be power managed at run time;
> + * + "on\n" to prevent the device from being power managed at run time;

I find interface with mandatory \n quite 'interesting'.

Plus english is strange here. All devices have ... "auto" to allow...?
Strange formulation. List the values first, then add "write ... to allow"?

> + * The default for all devices is "auto", which means that devices may be
> + * subject to automatic power management, depending on their drivers.

Is it wise to specify 'auto' default value for devices without runtime
pm?


> +static ssize_t control_store(struct device * dev, struct device_attribute *attr,
> + const char * buf, size_t n)
> +{
> + char *cp;
> + int len = n;
> +
> + cp = memchr(buf, '\n', n);
> + if (cp)
> + len = cp - buf;
> + if (len == sizeof ctrl_auto - 1 && strncmp(buf, ctrl_auto, len) == 0)
> + pm_runtime_allow(dev);

parenthesis after sizeof?

Do I read it correctly that all of

"auto"
"auto\n"
"auto\non\nIm confused"

will switch to auto?

(and sorry for late reply...)
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2010-02-04 23:07:58

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH 1/2] PM / Runtime: Add sysfs switch for disabling device run-time PM (rev. 2)

On Thursday 04 February 2010, Pavel Machek wrote:
> Hi!
>
> > /*
> > + * control - Report/change current runtime PM setting of the device
> > + *
> > + * Runtime power management of a device can be blocked with the help of
> > + * this attribute. All devices have one of the following two values for
> > + * the power/control file:
> > + *
> > + * + "auto\n" to allow the device to be power managed at run time;
> > + * + "on\n" to prevent the device from being power managed at run time;
>
> I find interface with mandatory \n quite 'interesting'.

This simply follows the other descriptions in this file.

> Plus english is strange here. All devices have ... "auto" to allow...?
> Strange formulation. List the values first, then add "write ... to allow"?

Ditto.

> > + * The default for all devices is "auto", which means that devices may be
> > + * subject to automatic power management, depending on their drivers.
>
> Is it wise to specify 'auto' default value for devices without runtime
> pm?

Yes, it is. It means the user space doesn't care whether or not the device is
power managed at run-time.

> > +static ssize_t control_store(struct device * dev, struct device_attribute *attr,
> > + const char * buf, size_t n)
> > +{
> > + char *cp;
> > + int len = n;
> > +
> > + cp = memchr(buf, '\n', n);
> > + if (cp)
> > + len = cp - buf;
> > + if (len == sizeof ctrl_auto - 1 && strncmp(buf, ctrl_auto, len) == 0)
> > + pm_runtime_allow(dev);
>
> parenthesis after sizeof?

This notation is used throughout this file too.

> Do I read it correctly that all of
>
> "auto"
> "auto\n"
> "auto\non\nIm confused"
>
> will switch to auto?

Perhaps it would, but what exactly is the problem with that?

Rafael

2010-02-05 06:28:11

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH 1/2] PM / Runtime: Add sysfs switch for disabling device run-time PM (rev. 2)


> > > + * The default for all devices is "auto", which means that devices may be
> > > + * subject to automatic power management, depending on their drivers.
> >
> > Is it wise to specify 'auto' default value for devices without runtime
> > pm?
>
> Yes, it is. It means the user space doesn't care whether or not the device is
> power managed at run-time.

Well, defaulting to 'on' for drivers when runtime pm is experimental
seems logical. and there will be such drivers...

> > > + len = cp - buf;
> > > + if (len == sizeof ctrl_auto - 1 && strncmp(buf, ctrl_auto, len) == 0)
> > > + pm_runtime_allow(dev);
> >
> > parenthesis after sizeof?
>
> This notation is used throughout this file too.

...but is inconsistent with rest of kernel.

> > Do I read it correctly that all of
> >
> > "auto"
> > "auto\n"
> > "auto\non\nIm confused"
> >
> > will switch to auto?
>
> Perhaps it would, but what exactly is the problem with that?

That apps will start depending on such broken behaviour?
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2010-02-05 08:01:24

by Nigel Cunningham

[permalink] [raw]
Subject: Re: [linux-pm] [PATCH 1/2] PM / Runtime: Add sysfs switch for disabling device run-time PM (rev. 2)

Hi Pavel et al.

Pavel Machek wrote:
> Hi!
>
>> /*
>> + * control - Report/change current runtime PM setting of the device
>> + *
>> + * Runtime power management of a device can be blocked with the help of
>> + * this attribute. All devices have one of the following two values for
>> + * the power/control file:
>> + *
>> + * + "auto\n" to allow the device to be power managed at run time;
>> + * + "on\n" to prevent the device from being power managed at run time;
>
> I find interface with mandatory \n quite 'interesting'.

Agreed.

> Plus english is strange here. All devices have ... "auto" to allow...?
> Strange formulation. List the values first, then add "write ... to allow"?

It seems okay to me because of the preceding sentence ending in a colon.

Regards,

Nigel

2010-02-05 15:19:21

by Alan Stern

[permalink] [raw]
Subject: Re: [PATCH 1/2] PM / Runtime: Add sysfs switch for disabling device run-time PM (rev. 2)

On Fri, 5 Feb 2010, Pavel Machek wrote:

>
> > > > + * The default for all devices is "auto", which means that devices may be
> > > > + * subject to automatic power management, depending on their drivers.
> > >
> > > Is it wise to specify 'auto' default value for devices without runtime
> > > pm?
> >
> > Yes, it is. It means the user space doesn't care whether or not the device is
> > power managed at run-time.
>
> Well, defaulting to 'on' for drivers when runtime pm is experimental
> seems logical. and there will be such drivers...

It's really not very important what the default setting is. Runtime PM
is always initially disabled, regardless of the "auto"/"on" setting,
until the driver explicitly enables it. At that time the driver also
has the option of changing the setting from "auto" to "on".

So if a driver writer wants to support runtime PM but thinks that his
implementation (or the device!) may be buggy, he can easily arrange
things so the user has to change the sysfs setting to make anything
happen.

Alan Stern