Run-time PM idea (was: Re: [RFC][PATCH 0/2] PM: Rearrange core suspend code)

Run-time PM idea (was: Re: [RFC][PATCH 0/2] PM: Rearrange core suspend code)

[Rafael J. Wysocki]

On Sunday 07 June 2009, Alan Stern wrote:
> On Sun, 7 Jun 2009, Rafael J. Wysocki wrote:
> 
> > Hi,
> > 
> > Here's something I wanted to do quite some time ago.
> > 
> > kernel/power/main.c becomes more and more difficult to maintain over time,
> > since it contains both the suspend to RAM core code and some common PM code
> > that is also used for hibernation.  For this reason [1/2] separates the suspend
> > to RAM code from main.c and puts it into two new files (the test facility is,
> > again, separated from the core code for clarity).
> > 
> > [2/2] renames kernel/power/disk.c to kernel/power/hibernate.c, because the role
> > of this file is analogous to kernel/power/suspend.c (introduced by [1/2]).
> > 
> > Comments welcome.
> 
> Looks like a good idea to me.

Great, thanks for your feedback! :-)

BTW, I've been considering the run-time PM a bit recently and the result is
below (on top of this series).

I noticed that since resume can be scheduled while suspend is in progress,
we need two work structures in struct device, one for suspend and one for
resume.  Also, in theory, we may want to resume the device before the suspend
has a chance to run, so there should be some synchronization between them,
which is done with the help of the spinlock in dev_pm_info.

The general idea is that drivers or bus types may use pm_schedule_suspend()
to put a suspend request into the work queue and pm_schedule_resume() to
queue a resume request or cancel a pending suspend request.  There's no
requirement to use these functions, but I think they may be helpful in some
simple cases.

It may be necessary to resume a device synchronously, but I'm still thinking
how to implement that.

Please have a look.

Best,
Rafael

---
 drivers/base/power/Makefile  |    1 
 drivers/base/power/main.c    |    6 +
 drivers/base/power/runtime.c |  163 +++++++++++++++++++++++++++++++++++++++++++
 include/linux/pm.h           |   36 ++++++++-
 include/linux/pm_runtime.h   |   82 +++++++++++++++++++++
 kernel/power/Kconfig         |   14 +++
 kernel/power/main.c          |   17 ++++
 7 files changed, 316 insertions(+), 3 deletions(-)

Index: linux-2.6/kernel/power/Kconfig
===================================================================
--- linux-2.6.orig/kernel/power/Kconfig
+++ linux-2.6/kernel/power/Kconfig
@@ -204,3 +204,17 @@ config APM_EMULATION
 	  random kernel OOPSes or reboots that don't seem to be related to
 	  anything, try disabling/enabling this option (or disabling/enabling
 	  APM in your BIOS).
+
+config PM_RUNTIME
+	bool "Run-time PM core functionality"
+	depends on PM
+	---help---
+	  Enable functionality allowing I/O devices to be put into energy-saving
+	  (low power) states at run time (or autosuspended) after a specified
+	  period of inactivity and woken up in response to a hardware-generated
+	  wake-up event or a driver's request.
+
+	  Hardware support is generally required for this functionality to work
+	  and the bus type drivers of the buses the devices are on are
+	  responsibile for the actual handling of the autosuspend requests and
+	  wake-up events.
Index: linux-2.6/kernel/power/main.c
===================================================================
--- linux-2.6.orig/kernel/power/main.c
+++ linux-2.6/kernel/power/main.c
@@ -11,6 +11,7 @@
 #include <linux/kobject.h>
 #include <linux/string.h>
 #include <linux/resume-trace.h>
+#include <linux/workqueue.h>
 
 #include "power.h"
 
@@ -217,8 +218,24 @@ static struct attribute_group attr_group
 	.attrs = g,
 };
 
+#ifdef CONFIG_PM_RUNTIME
+struct workqueue_struct *pm_wq;
+
+static int __init pm_start_workqueue(void)
+{
+	pm_wq = create_freezeable_workqueue("pm");
+
+	return pm_wq ? 0 : -ENOMEM;
+}
+#else
+static inline int pm_start_workqueue(void) { return 0; }
+#endif
+
 static int __init pm_init(void)
 {
+	int error = pm_start_workqueue();
+	if (error)
+		return error;
 	power_kobj = kobject_create_and_add("power", NULL);
 	if (!power_kobj)
 		return -ENOMEM;
Index: linux-2.6/include/linux/pm.h
===================================================================
--- linux-2.6.orig/include/linux/pm.h
+++ linux-2.6/include/linux/pm.h
@@ -22,6 +22,8 @@
 #define _LINUX_PM_H
 
 #include <linux/list.h>
+#include <linux/workqueue.h>
+#include <linux/spinlock.h>
 
 /*
  * Callbacks for platform drivers to implement.
@@ -165,6 +167,15 @@ typedef struct pm_message {
  * It is allowed to unregister devices while the above callbacks are being
  * executed.  However, it is not allowed to unregister a device from within any
  * of its own callbacks.
+ *
+ * There also are two callbacks related to run-time power management of devices:
+ *
+ * @autosuspend: Save the device registers and put it into an energy-saving (low
+ *	power) state at run-time, enable wake-up events as appropriate.
+ *
+ * @autoresume: Put the device into the full power state and restore its
+ *	registers (if applicable) at run time, in response to a wake-up event
+ *	generated by hardware or at a request of software.
  */
 
 struct dev_pm_ops {
@@ -182,6 +193,10 @@ struct dev_pm_ops {
 	int (*thaw_noirq)(struct device *dev);
 	int (*poweroff_noirq)(struct device *dev);
 	int (*restore_noirq)(struct device *dev);
+#ifdef CONFIG_PM_RUNTIME
+	int (*autosuspend)(struct device *dev);
+	int (*autoresume)(struct device *dev);
+#endif
 };
 
 /**
@@ -315,14 +330,31 @@ enum dpm_state {
 	DPM_OFF_IRQ,
 };
 
+enum rpm_state {
+	RPM_UNKNOWN = -1,
+	RPM_ACTIVE,
+	RPM_IDLE,
+	RPM_SUSPENDING,
+	RPM_SUSPENDED,
+};
+
 struct dev_pm_info {
 	pm_message_t		power_state;
-	unsigned		can_wakeup:1;
-	unsigned		should_wakeup:1;
+	unsigned int		can_wakeup:1;
+	unsigned int		should_wakeup:1;
 	enum dpm_state		status;		/* Owned by the PM core */
 #ifdef	CONFIG_PM_SLEEP
 	struct list_head	entry;
 #endif
+#ifdef	CONFIG_PM_RUNTIME
+	struct delayed_work	suspend_work;
+	struct work_struct	resume_work;
+	unsigned int		suspend_autocancel:1;
+	unsigned int		resume_autocancel:1;
+	unsigned int		suspend_aborted:1;
+	enum rpm_state		runtime_status;
+	spinlock_t		lock;
+#endif
 };
 
 /*
Index: linux-2.6/drivers/base/power/Makefile
===================================================================
--- linux-2.6.orig/drivers/base/power/Makefile
+++ linux-2.6/drivers/base/power/Makefile
@@ -1,5 +1,6 @@
 obj-$(CONFIG_PM)	+= sysfs.o
 obj-$(CONFIG_PM_SLEEP)	+= main.o
+obj-$(CONFIG_PM_RUNTIME)	+= runtime.o
 obj-$(CONFIG_PM_TRACE_RTC)	+= trace.o
 
 ccflags-$(CONFIG_DEBUG_DRIVER) := -DDEBUG
Index: linux-2.6/drivers/base/power/runtime.c
===================================================================
--- /dev/null
+++ linux-2.6/drivers/base/power/runtime.c
@@ -0,0 +1,163 @@
+/*
+ * drivers/base/power/runtime.c - Helper functions for device run-time PM
+ *
+ * Copyright (c) 2009 Rafael J. Wysocki <rjw@sisk.pl>, Novell Inc.
+ *
+ * This file is released under the GPLv2.
+ */
+
+#include <linux/pm_runtime.h>
+
+/**
+ * pm_runtime_reset - Clear all of the device run-time PM flags.
+ * @dev: Device object to clear the flags for.
+ */
+static void pm_runtime_reset(struct device *dev)
+{
+	dev->power.resume_autocancel = false;
+	dev->power.suspend_autocancel = false;
+	dev->power.suspend_aborted = false;
+	dev->power.runtime_status = RPM_ACTIVE;
+}
+
+/**
+ * pm_runtime_init - Initialize run-time PM fields in given device object.
+ * @dev: Device object to handle.
+ */
+void pm_runtime_init(struct device *dev)
+{
+	pm_runtime_reset(dev);
+	spin_lock_init(&dev->power.lock);
+}
+
+/**
+ * pm_autosuspend - Run autosuspend callback of given device object's bus type.
+ * @work: Work structure used for scheduling the execution of this function.
+ *
+ * Use @work to get the device object the suspend has been scheduled for,
+ * check if the suspend request hasn't been cancelled and run the
+ * ->autosuspend() callback from the device's bus type driver.  Update the
+ * run-time PM flags in the device object to reflect the current status of the
+ * device.
+ */
+static void pm_autosuspend(struct work_struct *work)
+{
+	struct delayed_work *dw = to_delayed_work(work);
+	struct device *dev = suspend_work_to_device(dw);
+	int error = 0;
+
+	pm_lock_device(dev);
+	if (dev->power.suspend_aborted) {
+		dev->power.runtime_status = RPM_ACTIVE;
+		goto out;
+	}
+	dev->power.suspend_autocancel = false;
+	dev->power.runtime_status = RPM_SUSPENDING;
+	pm_unlock_device(dev);
+
+	if (dev && dev->bus && dev->bus->pm && dev->bus->pm->autosuspend)
+		error = dev->bus->pm->autosuspend(dev);
+
+	pm_lock_device(dev);
+	dev->power.runtime_status = error ? RPM_UNKNOWN : RPM_SUSPENDED;
+ out:
+	pm_unlock_device(dev);
+}
+
+/**
+ * __pm_schedule_suspend - Schedule run-time suspend of given device.
+ * @dev: Device to suspend.
+ * @delay: Time to wait before attempting to suspend the device.
+ * @autocancel: If set, the request will be cancelled during a resume from a
+ *	system-wide sleep state if it happens before @delay elapses.
+ */
+void __pm_schedule_suspend(struct device *dev, unsigned long delay,
+			   bool autocancel)
+{
+	pm_lock_device(dev);
+	if (dev->power.runtime_status != RPM_ACTIVE)
+		goto out;
+	dev->power.suspend_autocancel = autocancel;
+	dev->power.suspend_aborted = false;
+	dev->power.runtime_status = RPM_IDLE;
+	INIT_DELAYED_WORK(&dev->power.suspend_work, pm_autosuspend);
+	queue_delayed_work(pm_wq, &dev->power.suspend_work, delay);
+ out:
+	pm_unlock_device(dev);
+}
+
+/**
+ * pm_autoresume - Run autoresume callback of given device object's bus type.
+ * @work: Work structure used for scheduling the execution of this function.
+ *
+ * Use @work to get the device object the resume has been scheduled for,
+ * check if the device is really suspended and run the ->autoresume() callback
+ * from the device's bus type driver.  Update the run-time PM flags in the
+ * device object to reflect the current status of the device.
+ */
+static void pm_autoresume(struct work_struct *work)
+{
+	struct device *dev = resume_work_to_device(work);
+	int error = 0;
+
+	pm_lock_device(dev);
+	dev->power.resume_autocancel = false;
+	if (dev->power.runtime_status != RPM_SUSPENDED)
+		goto out;
+	pm_unlock_device(dev);
+
+	if (dev && dev->bus && dev->bus->pm && dev->bus->pm->autoresume)
+		error = dev->bus->pm->autoresume(dev);
+
+	pm_lock_device(dev);
+	dev->power.runtime_status = error ? RPM_UNKNOWN : RPM_ACTIVE;
+ out:
+	pm_unlock_device(dev);
+}
+
+/**
+ * __pm_schedule_resume - Schedule run-time resume of given device.
+ * @dev: Device to resume.
+ * @autocancel: If set, the request will be cancelled during a resume from a
+ *	system-wide sleep state if it happens before pm_autoresume() can be run.
+ */
+void __pm_schedule_resume(struct device *dev, bool autocancel)
+{
+	pm_lock_device(dev);
+	if (dev->power.runtime_status == RPM_IDLE) {
+		dev->power.suspend_autocancel = false;
+		dev->power.suspend_aborted = true;
+		cancel_delayed_work(&dev->power.suspend_work);
+		dev->power.runtime_status = RPM_ACTIVE;
+	} else if (dev->power.runtime_status != RPM_ACTIVE) {
+		dev->power.resume_autocancel = autocancel;
+		INIT_WORK(&dev->power.resume_work, pm_autoresume);
+		queue_work(pm_wq, &dev->power.resume_work);
+	}
+	pm_unlock_device(dev);
+}
+
+/**
+ * pm_runtime_autocancel - Cancel run-time PM requests during system resume.
+ * @dev: Device to handle.
+ *
+ * If dev->power.suspend_autocancel is set during resume from a system sleep
+ * state, there is a run-time suspend request pending that has to be cancelled,
+ * so cancel it, and analogously for pending run-time resume requests.
+ *
+ * This function is only called by the PM core and must not be used by bus types
+ * and device drivers.  Moreover, it is called when the workqueue is frozen, so
+ * it is guaranteed that the autosuspend callbacks are not running at that time.
+ */
+void pm_runtime_autocancel(struct device *dev)
+{
+	pm_lock_device(dev);
+	if (dev->power.suspend_autocancel) {
+		cancel_delayed_work(&dev->power.suspend_work);
+		pm_runtime_reset(dev);
+	} else if (dev->power.resume_autocancel) {
+		work_clear_pending(&dev->power.resume_work);
+		pm_runtime_reset(dev);
+	}
+	pm_unlock_device(dev);
+}
Index: linux-2.6/include/linux/pm_runtime.h
===================================================================
--- /dev/null
+++ linux-2.6/include/linux/pm_runtime.h
@@ -0,0 +1,82 @@
+/*
+ * pm_runtime.h - Device run-time power management helper functions.
+ *
+ * Copyright (C) 2009 Rafael J. Wysocki <rjw@sisk.pl>
+ *
+ * This file is released under the GPLv2.
+ */
+
+#ifndef _LINUX_PM_RUNTIME_H
+#define _LINUX_PM_RUNTIME_H
+
+#include <linux/device.h>
+#include <linux/pm.h>
+
+#ifdef CONFIG_PM_RUNTIME
+extern struct workqueue_struct *pm_wq;
+
+extern void pm_runtime_init(struct device *dev);
+extern void __pm_schedule_suspend(struct device *dev, unsigned long delay,
+				   bool autocancel);
+extern void __pm_schedule_resume(struct device *dev, bool autocancel);
+extern void pm_runtime_autocancel(struct device *dev);
+
+static inline struct device *suspend_work_to_device(struct delayed_work *work)
+{
+	struct dev_pm_info *dpi;
+
+	dpi = container_of(work, struct dev_pm_info, suspend_work);
+	return container_of(dpi, struct device, power);
+}
+
+static inline struct device *resume_work_to_device(struct work_struct *work)
+{
+	struct dev_pm_info *dpi;
+
+	dpi = container_of(work, struct dev_pm_info, resume_work);
+	return container_of(dpi, struct device, power);
+}
+
+static inline void pm_lock_device(struct device *dev)
+{
+	spin_lock(&dev->power.lock);
+}
+
+static inline void pm_unlock_device(struct device *dev)
+{
+	spin_unlock(&dev->power.lock);
+}
+#else /* !CONFIG_PM_RUNTIME */
+static inline void pm_runtime_init(struct device *dev) {}
+static inline void __pm_schedule_suspend(struct device *dev,
+					  unsigned long delay,
+					  bool autocancel) {}
+static inline void __pm_schedule_resume(struct device *dev, bool autocancel) {}
+static inline void pm_runtime_autocancel(struct device *dev) {}
+
+static inline void pm_lock_device(struct device *dev) {}
+static inline void pm_unlock_device(struct device *dev) {}
+#endif /* !CONFIG_PM_RUNTIME */
+
+static inline void pm_schedule_suspend(struct device *dev, unsigned long delay)
+{
+	__pm_schedule_suspend(dev, delay, false);
+}
+
+static inline void pm_schedule_suspend_autocancel(struct device *dev,
+						   unsigned long delay)
+{
+	__pm_schedule_suspend(dev, delay, true);
+}
+
+static inline void pm_schedule_resume(struct device *dev)
+{
+	__pm_schedule_resume(dev, false);
+}
+
+static inline void pm_schedule_resume_autocancel(struct device *dev)
+{
+	__pm_schedule_resume(dev, true);
+}
+
+#endif
Index: linux-2.6/drivers/base/power/main.c
===================================================================
--- linux-2.6.orig/drivers/base/power/main.c
+++ linux-2.6/drivers/base/power/main.c
@@ -21,6 +21,7 @@
 #include <linux/kallsyms.h>
 #include <linux/mutex.h>
 #include <linux/pm.h>
+#include <linux/pm_runtime.h>
 #include <linux/resume-trace.h>
 #include <linux/rwsem.h>
 #include <linux/interrupt.h>
@@ -88,6 +89,7 @@ void device_pm_add(struct device *dev)
 	}
 
 	list_add_tail(&dev->power.entry, &dpm_list);
+	pm_runtime_init(dev);
 	mutex_unlock(&dpm_list_mtx);
 }
 
@@ -355,7 +357,7 @@ void dpm_resume_noirq(pm_message_t state
 	struct device *dev;
 
 	mutex_lock(&dpm_list_mtx);
-	list_for_each_entry(dev, &dpm_list, power.entry)
+	list_for_each_entry(dev, &dpm_list, power.entry) {
 		if (dev->power.status > DPM_OFF) {
 			int error;
 
@@ -364,6 +366,8 @@ void dpm_resume_noirq(pm_message_t state
 			if (error)
 				pm_dev_err(dev, state, " early", error);
 		}
+		pm_runtime_autocancel(dev);
+	}
 	mutex_unlock(&dpm_list_mtx);
 	resume_device_irqs();
 }

Re: Run-time PM idea (was: Re: [RFC][PATCH 0/2] PM: Rearrange core suspend code)

[Oliver Neukum]

Am Sonntag, 7. Juni 2009 23:46:59 schrieb Rafael J. Wysocki:
> + * Use @work to get the device object the resume has been scheduled for,
> + * check if the device is really suspended and run the ->autoresume()
> callback + * from the device's bus type driver.  Update the run-time PM
> flags in the + * device object to reflect the current status of the device.
> + */
> +static void pm_autoresume(struct work_struct *work)
> +{

Why do you pass it a struct work pointer?

	Regards
		Oliver

Re: Run-time PM idea (was: Re: [RFC][PATCH 0/2] PM: Rearrange core suspend code)

[Oliver Neukum]

Am Sonntag, 7. Juni 2009 23:46:59 schrieb Rafael J. Wysocki:
> It may be necessary to resume a device synchronously, but I'm still
> thinking how to implement that.

This will absolutely be the default. You resume a device because you want
it to do something now. It seems to me that you making your problem worse
by using a spinlock as a lock. A mutex would make it easier.

	Regards
		Oliver

Re: Run-time PM idea (was: Re: [RFC][PATCH 0/2] PM: Rearrange core suspend code)

[Ingo Molnar]

* Rafael J. Wysocki <rjw@sisk.pl> wrote:

> +config PM_RUNTIME
> +	bool "Run-time PM core functionality"
> +	depends on PM
> +	---help---
> +	  Enable functionality allowing I/O devices to be put into energy-saving
> +	  (low power) states at run time (or autosuspended) after a specified
> +	  period of inactivity and woken up in response to a hardware-generated
> +	  wake-up event or a driver's request.
> +
> +	  Hardware support is generally required for this functionality to work
> +	  and the bus type drivers of the buses the devices are on are
> +	  responsibile for the actual handling of the autosuspend requests and
> +	  wake-up events.

Halleluya! :-)

	Ingo

Re: Run-time PM idea (was: Re: [RFC][PATCH 0/2] PM: Rearrange core suspend code)

[Rafael J. Wysocki]

On Monday 08 June 2009, Oliver Neukum wrote:
> Am Sonntag, 7. Juni 2009 23:46:59 schrieb Rafael J. Wysocki:
> > It may be necessary to resume a device synchronously, but I'm still
> > thinking how to implement that.
> 
> This will absolutely be the default. You resume a device because you want
> it to do something now. It seems to me that you making your problem worse
> by using a spinlock as a lock. A mutex would make it easier.

But I need to be able to call __pm_schedule_resume() (at least) from interrupt
context and I can't use a mutex from there.  Otherwise I'd have used a mutex. :-)

Anyway, below is a version with synchronous resume.

Thanks,
Rafael

---
 drivers/base/power/Makefile  |    1 
 drivers/base/power/main.c    |    6 -
 drivers/base/power/runtime.c |  223 +++++++++++++++++++++++++++++++++++++++++++
 include/linux/pm.h           |   36 ++++++
 include/linux/pm_runtime.h   |   82 +++++++++++++++
 kernel/power/Kconfig         |   14 ++
 kernel/power/main.c          |   17 +++
 7 files changed, 376 insertions(+), 3 deletions(-)

Index: linux-2.6/kernel/power/Kconfig
===================================================================
--- linux-2.6.orig/kernel/power/Kconfig
+++ linux-2.6/kernel/power/Kconfig
@@ -204,3 +204,17 @@ config APM_EMULATION
 	  random kernel OOPSes or reboots that don't seem to be related to
 	  anything, try disabling/enabling this option (or disabling/enabling
 	  APM in your BIOS).
+
+config PM_RUNTIME
+	bool "Run-time PM core functionality"
+	depends on PM
+	---help---
+	  Enable functionality allowing I/O devices to be put into energy-saving
+	  (low power) states at run time (or autosuspended) after a specified
+	  period of inactivity and woken up in response to a hardware-generated
+	  wake-up event or a driver's request.
+
+	  Hardware support is generally required for this functionality to work
+	  and the bus type drivers of the buses the devices are on are
+	  responsibile for the actual handling of the autosuspend requests and
+	  wake-up events.
Index: linux-2.6/kernel/power/main.c
===================================================================
--- linux-2.6.orig/kernel/power/main.c
+++ linux-2.6/kernel/power/main.c
@@ -11,6 +11,7 @@
 #include <linux/kobject.h>
 #include <linux/string.h>
 #include <linux/resume-trace.h>
+#include <linux/workqueue.h>
 
 #include "power.h"
 
@@ -217,8 +218,24 @@ static struct attribute_group attr_group
 	.attrs = g,
 };
 
+#ifdef CONFIG_PM_RUNTIME
+struct workqueue_struct *pm_wq;
+
+static int __init pm_start_workqueue(void)
+{
+	pm_wq = create_freezeable_workqueue("pm");
+
+	return pm_wq ? 0 : -ENOMEM;
+}
+#else
+static inline int pm_start_workqueue(void) { return 0; }
+#endif
+
 static int __init pm_init(void)
 {
+	int error = pm_start_workqueue();
+	if (error)
+		return error;
 	power_kobj = kobject_create_and_add("power", NULL);
 	if (!power_kobj)
 		return -ENOMEM;
Index: linux-2.6/include/linux/pm.h
===================================================================
--- linux-2.6.orig/include/linux/pm.h
+++ linux-2.6/include/linux/pm.h
@@ -22,6 +22,8 @@
 #define _LINUX_PM_H
 
 #include <linux/list.h>
+#include <linux/workqueue.h>
+#include <linux/spinlock.h>
 
 /*
  * Callbacks for platform drivers to implement.
@@ -165,6 +167,15 @@ typedef struct pm_message {
  * It is allowed to unregister devices while the above callbacks are being
  * executed.  However, it is not allowed to unregister a device from within any
  * of its own callbacks.
+ *
+ * There also are two callbacks related to run-time power management of devices:
+ *
+ * @autosuspend: Save the device registers and put it into an energy-saving (low
+ *	power) state at run-time, enable wake-up events as appropriate.
+ *
+ * @autoresume: Put the device into the full power state and restore its
+ *	registers (if applicable) at run time, in response to a wake-up event
+ *	generated by hardware or at a request of software.
  */
 
 struct dev_pm_ops {
@@ -182,6 +193,10 @@ struct dev_pm_ops {
 	int (*thaw_noirq)(struct device *dev);
 	int (*poweroff_noirq)(struct device *dev);
 	int (*restore_noirq)(struct device *dev);
+#ifdef CONFIG_PM_RUNTIME
+	int (*autosuspend)(struct device *dev);
+	int (*autoresume)(struct device *dev);
+#endif
 };
 
 /**
@@ -315,14 +330,31 @@ enum dpm_state {
 	DPM_OFF_IRQ,
 };
 
+enum rpm_state {
+	RPM_UNKNOWN = -1,
+	RPM_ACTIVE,
+	RPM_IDLE,
+	RPM_SUSPENDING,
+	RPM_SUSPENDED,
+};
+
 struct dev_pm_info {
 	pm_message_t		power_state;
-	unsigned		can_wakeup:1;
-	unsigned		should_wakeup:1;
+	unsigned int		can_wakeup:1;
+	unsigned int		should_wakeup:1;
 	enum dpm_state		status;		/* Owned by the PM core */
 #ifdef	CONFIG_PM_SLEEP
 	struct list_head	entry;
 #endif
+#ifdef	CONFIG_PM_RUNTIME
+	struct delayed_work	suspend_work;
+	struct work_struct	resume_work;
+	unsigned int		suspend_autocancel:1;
+	unsigned int		resume_autocancel:1;
+	unsigned int		suspend_aborted:1;
+	enum rpm_state		runtime_status;
+	spinlock_t		lock;
+#endif
 };
 
 /*
Index: linux-2.6/drivers/base/power/Makefile
===================================================================
--- linux-2.6.orig/drivers/base/power/Makefile
+++ linux-2.6/drivers/base/power/Makefile
@@ -1,5 +1,6 @@
 obj-$(CONFIG_PM)	+= sysfs.o
 obj-$(CONFIG_PM_SLEEP)	+= main.o
+obj-$(CONFIG_PM_RUNTIME)	+= runtime.o
 obj-$(CONFIG_PM_TRACE_RTC)	+= trace.o
 
 ccflags-$(CONFIG_DEBUG_DRIVER) := -DDEBUG
Index: linux-2.6/drivers/base/power/runtime.c
===================================================================
--- /dev/null
+++ linux-2.6/drivers/base/power/runtime.c
@@ -0,0 +1,223 @@
+/*
+ * drivers/base/power/runtime.c - Helper functions for device run-time PM
+ *
+ * Copyright (c) 2009 Rafael J. Wysocki <rjw@sisk.pl>, Novell Inc.
+ *
+ * This file is released under the GPLv2.
+ */
+
+#include <linux/pm_runtime.h>
+
+/**
+ * pm_runtime_reset - Clear all of the device run-time PM flags.
+ * @dev: Device object to clear the flags for.
+ */
+static void pm_runtime_reset(struct device *dev)
+{
+	dev->power.resume_autocancel = false;
+	dev->power.suspend_autocancel = false;
+	dev->power.suspend_aborted = false;
+	dev->power.runtime_status = RPM_ACTIVE;
+}
+
+/**
+ * pm_autosuspend - Run autosuspend callback of given device object's bus type.
+ * @work: Work structure used for scheduling the execution of this function.
+ *
+ * Use @work to get the device object the suspend has been scheduled for,
+ * check if the suspend request hasn't been cancelled and run the
+ * ->autosuspend() callback from the device's bus type driver.  Update the
+ * run-time PM flags in the device object to reflect the current status of the
+ * device.
+ */
+static void pm_autosuspend(struct work_struct *work)
+{
+	struct delayed_work *dw = to_delayed_work(work);
+	struct device *dev = suspend_work_to_device(dw);
+	int error = 0;
+
+	pm_lock_device(dev);
+	if (dev->power.suspend_aborted) {
+		dev->power.runtime_status = RPM_ACTIVE;
+		goto out;
+	}
+	dev->power.suspend_autocancel = false;
+	dev->power.runtime_status = RPM_SUSPENDING;
+	pm_unlock_device(dev);
+
+	if (dev && dev->bus && dev->bus->pm && dev->bus->pm->autosuspend)
+		error = dev->bus->pm->autosuspend(dev);
+
+	pm_lock_device(dev);
+	dev->power.runtime_status = error ? RPM_UNKNOWN : RPM_SUSPENDED;
+ out:
+	pm_unlock_device(dev);
+}
+
+/**
+ * __pm_schedule_suspend - Schedule run-time suspend of given device.
+ * @dev: Device to suspend.
+ * @delay: Time to wait before attempting to suspend the device.
+ * @autocancel: If set, the request will be cancelled during a resume from a
+ *	system-wide sleep state if it happens before @delay elapses.
+ */
+void __pm_schedule_suspend(struct device *dev, unsigned long delay,
+			   bool autocancel)
+{
+	pm_lock_device(dev);
+	if (dev->power.runtime_status != RPM_ACTIVE)
+		goto out;
+	dev->power.suspend_autocancel = autocancel;
+	dev->power.suspend_aborted = false;
+	dev->power.runtime_status = RPM_IDLE;
+	queue_delayed_work(pm_wq, &dev->power.suspend_work, delay);
+ out:
+	pm_unlock_device(dev);
+}
+
+/**
+ * pm_autoresume - Run autoresume callback of given device object's bus type.
+ * @work: Work structure used for scheduling the execution of this function.
+ *
+ * Use @work to get the device object the resume has been scheduled for,
+ * check if the device is really suspended and run the ->autoresume() callback
+ * from the device's bus type driver.  Update the run-time PM flags in the
+ * device object to reflect the current status of the device.
+ */
+static void pm_autoresume(struct work_struct *work)
+{
+	struct device *dev = resume_work_to_device(work);
+	int error = 0;
+
+	pm_lock_device(dev);
+	dev->power.resume_autocancel = false;
+	if (dev->power.runtime_status != RPM_SUSPENDED)
+		goto out;
+	pm_unlock_device(dev);
+
+	if (dev->bus && dev->bus->pm && dev->bus->pm->autoresume)
+		error = dev->bus->pm->autoresume(dev);
+
+	pm_lock_device(dev);
+	dev->power.runtime_status = error ? RPM_UNKNOWN : RPM_ACTIVE;
+ out:
+	pm_unlock_device(dev);
+}
+
+/**
+ * pm_cancel_suspend - Cancel a pending suspend request for given device.
+ * @dev: Device to cancel the suspend request for.
+ *
+ * Should be called under pm_lock_device() and only if we are sure that the
+ * ->autosuspend() callback hasn't started to yet.
+ */
+static void pm_cancel_suspend(struct device *dev)
+{
+	dev->power.suspend_autocancel = false;
+	dev->power.suspend_aborted = true;
+	cancel_delayed_work(&dev->power.suspend_work);
+	dev->power.runtime_status = RPM_ACTIVE;
+}
+
+/**
+ * __pm_schedule_resume - Schedule run-time resume of given device.
+ * @dev: Device to resume.
+ * @autocancel: If set, the request will be cancelled during a resume from a
+ *	system-wide sleep state if it happens before pm_autoresume() can be run.
+ */
+void __pm_schedule_resume(struct device *dev, bool autocancel)
+{
+	pm_lock_device(dev);
+	if (dev->power.runtime_status == RPM_IDLE) {
+		/* ->autosuspend() hasn't started yet, no need to resume. */
+		pm_cancel_suspend(dev);
+	} else if (dev->power.runtime_status != RPM_ACTIVE) {
+		dev->power.resume_autocancel = autocancel;
+		queue_work(pm_wq, &dev->power.resume_work);
+	}
+	pm_unlock_device(dev);
+}
+
+/**
+ * pm_resume_sync - Resume given device waiting for the operation to complete.
+ * @dev: Device to resume.
+ *
+ * Resume the device synchronously, waiting for the operation to complete.  If
+ * autosuspend is in progress while this function is being run, wait for it to
+ * finish before resuming the device.  If the autosuspend is scheduled, but it
+ * hasn't started yet, cancel it and we're done.
+ */
+int pm_resume_sync(struct device *dev)
+{
+	int error = 0;
+
+	pm_lock_device(dev);
+	if (dev->power.runtime_status == RPM_IDLE) {
+		/* ->autosuspend() hasn't started yet, no need to resume. */
+		pm_cancel_suspend(dev);
+		goto out;
+	}
+
+	if (dev->power.runtime_status == RPM_SUSPENDING) {
+		/*
+		 * The ->autosuspend() callback is being executed right now,
+		 * wait for it to complete.
+		 */
+		pm_unlock_device(dev);
+		cancel_delayed_work_sync(&dev->power.suspend_work);
+		pm_lock_device(dev);
+	}
+
+	if (dev->power.runtime_status != RPM_SUSPENDED) {
+		error = -EINVAL;
+		goto out;
+	}
+	pm_unlock_device(dev);
+
+	if (dev->bus && dev->bus->pm && dev->bus->pm->autoresume)
+		error = dev->bus->pm->autoresume(dev);
+
+	pm_lock_device(dev);
+	dev->power.runtime_status = error ? RPM_UNKNOWN : RPM_ACTIVE;
+ out:
+	pm_unlock_device(dev);
+
+	return error;
+}
+
+/**
+ * pm_runtime_autocancel - Cancel run-time PM requests during system resume.
+ * @dev: Device to handle.
+ *
+ * If dev->power.suspend_autocancel is set during resume from a system sleep
+ * state, there is a run-time suspend request pending that has to be cancelled,
+ * so cancel it, and analogously for pending run-time resume requests.
+ *
+ * This function is only called by the PM core and must not be used by bus types
+ * and device drivers.  Moreover, it is called when the workqueue is frozen, so
+ * it is guaranteed that the autosuspend callbacks are not running at that time.
+ */
+void pm_runtime_autocancel(struct device *dev)
+{
+	pm_lock_device(dev);
+	if (dev->power.suspend_autocancel) {
+		cancel_delayed_work(&dev->power.suspend_work);
+		pm_runtime_reset(dev);
+	} else if (dev->power.resume_autocancel) {
+		work_clear_pending(&dev->power.resume_work);
+		pm_runtime_reset(dev);
+	}
+	pm_unlock_device(dev);
+}
+
+/**
+ * pm_runtime_init - Initialize run-time PM fields in given device object.
+ * @dev: Device object to handle.
+ */
+void pm_runtime_init(struct device *dev)
+{
+	pm_runtime_reset(dev);
+	spin_lock_init(&dev->power.lock);
+	INIT_DELAYED_WORK(&dev->power.suspend_work, pm_autosuspend);
+	INIT_WORK(&dev->power.resume_work, pm_autoresume);
+}
Index: linux-2.6/include/linux/pm_runtime.h
===================================================================
--- /dev/null
+++ linux-2.6/include/linux/pm_runtime.h
@@ -0,0 +1,82 @@
+/*
+ * pm_runtime.h - Device run-time power management helper functions.
+ *
+ * Copyright (C) 2009 Rafael J. Wysocki <rjw@sisk.pl>
+ *
+ * This file is released under the GPLv2.
+ */
+
+#ifndef _LINUX_PM_RUNTIME_H
+#define _LINUX_PM_RUNTIME_H
+
+#include <linux/device.h>
+#include <linux/pm.h>
+
+#ifdef CONFIG_PM_RUNTIME
+extern struct workqueue_struct *pm_wq;
+
+extern void pm_runtime_init(struct device *dev);
+extern void __pm_schedule_suspend(struct device *dev, unsigned long delay,
+				   bool autocancel);
+extern void __pm_schedule_resume(struct device *dev, bool autocancel);
+extern void pm_runtime_autocancel(struct device *dev);
+
+static inline struct device *suspend_work_to_device(struct delayed_work *work)
+{
+	struct dev_pm_info *dpi;
+
+	dpi = container_of(work, struct dev_pm_info, suspend_work);
+	return container_of(dpi, struct device, power);
+}
+
+static inline struct device *resume_work_to_device(struct work_struct *work)
+{
+	struct dev_pm_info *dpi;
+
+	dpi = container_of(work, struct dev_pm_info, resume_work);
+	return container_of(dpi, struct device, power);
+}
+
+static inline void pm_lock_device(struct device *dev)
+{
+	spin_lock(&dev->power.lock);
+}
+
+static inline void pm_unlock_device(struct device *dev)
+{
+	spin_unlock(&dev->power.lock);
+}
+#else /* !CONFIG_PM_RUNTIME */
+static inline void pm_runtime_init(struct device *dev) {}
+static inline void __pm_schedule_suspend(struct device *dev,
+					  unsigned long delay,
+					  bool autocancel) {}
+static inline void __pm_schedule_resume(struct device *dev, bool autocancel) {}
+static inline void pm_runtime_autocancel(struct device *dev) {}
+
+static inline void pm_lock_device(struct device *dev) {}
+static inline void pm_unlock_device(struct device *dev) {}
+#endif /* !CONFIG_PM_RUNTIME */
+
+static inline void pm_schedule_suspend(struct device *dev, unsigned long delay)
+{
+	__pm_schedule_suspend(dev, delay, false);
+}
+
+static inline void pm_schedule_suspend_autocancel(struct device *dev,
+						   unsigned long delay)
+{
+	__pm_schedule_suspend(dev, delay, true);
+}
+
+static inline void pm_schedule_resume(struct device *dev)
+{
+	__pm_schedule_resume(dev, false);
+}
+
+static inline void pm_schedule_resume_autocancel(struct device *dev)
+{
+	__pm_schedule_resume(dev, true);
+}
+
+#endif
Index: linux-2.6/drivers/base/power/main.c
===================================================================
--- linux-2.6.orig/drivers/base/power/main.c
+++ linux-2.6/drivers/base/power/main.c
@@ -21,6 +21,7 @@
 #include <linux/kallsyms.h>
 #include <linux/mutex.h>
 #include <linux/pm.h>
+#include <linux/pm_runtime.h>
 #include <linux/resume-trace.h>
 #include <linux/rwsem.h>
 #include <linux/interrupt.h>
@@ -88,6 +89,7 @@ void device_pm_add(struct device *dev)
 	}
 
 	list_add_tail(&dev->power.entry, &dpm_list);
+	pm_runtime_init(dev);
 	mutex_unlock(&dpm_list_mtx);
 }
 
@@ -355,7 +357,7 @@ void dpm_resume_noirq(pm_message_t state
 	struct device *dev;
 
 	mutex_lock(&dpm_list_mtx);
-	list_for_each_entry(dev, &dpm_list, power.entry)
+	list_for_each_entry(dev, &dpm_list, power.entry) {
 		if (dev->power.status > DPM_OFF) {
 			int error;
 
@@ -364,6 +366,8 @@ void dpm_resume_noirq(pm_message_t state
 			if (error)
 				pm_dev_err(dev, state, " early", error);
 		}
+		pm_runtime_autocancel(dev);
+	}
 	mutex_unlock(&dpm_list_mtx);
 	resume_device_irqs();
 }

Re: Run-time PM idea (was: Re: [RFC][PATCH 0/2] PM: Rearrange core suspend code)

[Oliver Neukum]

Am Montag, 8. Juni 2009 13:29:26 schrieb Rafael J. Wysocki:

> But I need to be able to call __pm_schedule_resume() (at least) from
> interrupt context and I can't use a mutex from there.  Otherwise I'd have
> used a mutex. :-)

I see.

> Anyway, below is a version with synchronous resume.

You are assuming autosuspend should always be with a delay. Why?

Secondly, you are not using a counter. Therefore only one driver can
control the PM state of a device at a given time. Is that wise?

> + * __pm_schedule_suspend - Schedule run-time suspend of given device.
> + * @dev: Device to suspend.
> + * @delay: Time to wait before attempting to suspend the device.

In which unit of time? If this is to go into kerneldoc that must be specified.

> + * @autocancel: If set, the request will be cancelled during a resume from
> a + *	system-wide sleep state if it happens before @delay elapses.

Why is this needed?

> + */
> +void __pm_schedule_suspend(struct device *dev, unsigned long delay,
> +			   bool autocancel)

[..]


> +
> +/**
> + * __pm_schedule_resume - Schedule run-time resume of given device.
> + * @dev: Device to resume.
> + * @autocancel: If set, the request will be cancelled during a resume from
> a + *	system-wide sleep state if it happens before pm_autoresume() can be
> run. + */

Eeek! This is a bad idea. You never want to a resume to be cancelled.

> +void __pm_schedule_resume(struct device *dev, bool autocancel)

[..]
> +int pm_resume_sync(struct device *dev)
> +{
> +	int error = 0;
> +
> +	pm_lock_device(dev);
> +	if (dev->power.runtime_status == RPM_IDLE) {
> +		/* ->autosuspend() hasn't started yet, no need to resume. */
> +		pm_cancel_suspend(dev);
> +		goto out;
> +	}
> +
> +	if (dev->power.runtime_status == RPM_SUSPENDING) {
> +		/*
> +		 * The ->autosuspend() callback is being executed right now,
> +		 * wait for it to complete.
> +		 */
> +		pm_unlock_device(dev);
> +		cancel_delayed_work_sync(&dev->power.suspend_work);

That is the most glorious abuse of an API I've seen this year :-)

	Regards
		Oliver

Re: Run-time PM idea (was: Re: [RFC][PATCH 0/2] PM: Rearrange core suspend code)

[Rafael J. Wysocki]

On Monday 08 June 2009, Oliver Neukum wrote:
> Am Montag, 8. Juni 2009 13:29:26 schrieb Rafael J. Wysocki:
> 
> > But I need to be able to call __pm_schedule_resume() (at least) from
> > interrupt context and I can't use a mutex from there.  Otherwise I'd have
> > used a mutex. :-)
> 
> I see.
> 
> > Anyway, below is a version with synchronous resume.
> 
> You are assuming autosuspend should always be with a delay. Why?

I couldn't invent a valid case for doing it without a delay.  Perhaps my
imagination is constrained too much. ;-)

> Secondly, you are not using a counter. Therefore only one driver can
> control the PM state of a device at a given time. Is that wise?

I didn't think about it to be honest.  Obviously this patch doesn't cover all
of the possible cases and I'm not even sure it's worth trying to cover them
upfront.

> > + * __pm_schedule_suspend - Schedule run-time suspend of given device.
> > + * @dev: Device to suspend.
> > + * @delay: Time to wait before attempting to suspend the device.
> 
> In which unit of time? If this is to go into kerneldoc that must be specified.

That's in jiifies.  Yes, I should have documented it.

> > + * @autocancel: If set, the request will be cancelled during a resume from
> > a + *	system-wide sleep state if it happens before @delay elapses.
> 
> Why is this needed?

In some subsystems, like PCI, devices will be resumed by the BIOS
unconditionally in the majority of cases and then it's not worth trying to
complete run-time PM requests from before the suspend.

> > + */
> > +void __pm_schedule_suspend(struct device *dev, unsigned long delay,
> > +			   bool autocancel)
> 
> [..]
> 
> 
> > +
> > +/**
> > + * __pm_schedule_resume - Schedule run-time resume of given device.
> > + * @dev: Device to resume.
> > + * @autocancel: If set, the request will be cancelled during a resume from
> > a + *	system-wide sleep state if it happens before pm_autoresume() can be
> > run. + */
> 
> Eeek! This is a bad idea. You never want to a resume to be cancelled.

Sometimes I do (see above).

> > +void __pm_schedule_resume(struct device *dev, bool autocancel)
> 
> [..]
> > +int pm_resume_sync(struct device *dev)
> > +{
> > +	int error = 0;
> > +
> > +	pm_lock_device(dev);
> > +	if (dev->power.runtime_status == RPM_IDLE) {
> > +		/* ->autosuspend() hasn't started yet, no need to resume. */
> > +		pm_cancel_suspend(dev);
> > +		goto out;
> > +	}
> > +
> > +	if (dev->power.runtime_status == RPM_SUSPENDING) {
> > +		/*
> > +		 * The ->autosuspend() callback is being executed right now,
> > +		 * wait for it to complete.
> > +		 */
> > +		pm_unlock_device(dev);
> > +		cancel_delayed_work_sync(&dev->power.suspend_work);
> 
> That is the most glorious abuse of an API I've seen this year :-)

Heh.

OK, what would you do instead?

Rafael

Re: Run-time PM idea (was: Re: [RFC][PATCH 0/2] PM: Rearrange core suspend code)

[Oliver Neukum]

Am Montag, 8. Juni 2009 20:34:30 schrieb Rafael J. Wysocki:
> On Monday 08 June 2009, Oliver Neukum wrote:
> > Am Montag, 8. Juni 2009 13:29:26 schrieb Rafael J. Wysocki:

> > Secondly, you are not using a counter. Therefore only one driver can
> > control the PM state of a device at a given time. Is that wise?
>
> I didn't think about it to be honest.  Obviously this patch doesn't cover
> all of the possible cases and I'm not even sure it's worth trying to cover
> them upfront.

I am thinking of multimedia cards, which have separate drivers for i2c, tuner
and so on.

> > Why is this needed?
>
> In some subsystems, like PCI, devices will be resumed by the BIOS
> unconditionally in the majority of cases and then it's not worth trying to
> complete run-time PM requests from before the suspend.

But why is it worth canceling them? That feature seems to be an unnecessary
complication. As long as you can safely suspend them, why not do it?

> > > +/**
> > > + * __pm_schedule_resume - Schedule run-time resume of given device.
> > > + * @dev: Device to resume.
> > > + * @autocancel: If set, the request will be cancelled during a resume from a 
> > > + *	system-wide sleep state if it happens before pm_autoresume() can be run.
> > > + */
> >
> > Eeek! This is a bad idea. You never want to a resume to be cancelled.
>
> Sometimes I do (see above).

Well no. A driver requests a resume because it has to.
This needs a defined call sequence.

Do you guarantee that autoresume follows autosuspend or not?
If not what sequences can happen? Obviously an autosuspended device
can be unplugged.
But the problem here is STR or STD. How do you notify drivers that the BIOS
has resumed their device instead of autoresume() being called? A driver
has to know that its device has become active without its knowledge.

> > > +		cancel_delayed_work_sync(&dev->power.suspend_work);
> >
> > That is the most glorious abuse of an API I've seen this year :-)
>
> Heh.
>
> OK, what would you do instead?

A waitqueue.

	Regards
		Oliver

Re: Run-time PM idea (was: Re: [RFC][PATCH 0/2] PM: Rearrange core suspend code)

[Alan Stern]

On Tue, 9 Jun 2009, Oliver Neukum wrote:

> But the problem here is STR or STD. How do you notify drivers that the BIOS
> has resumed their device instead of autoresume() being called? A driver
> has to know that its device has become active without its knowledge.

That would be a wonderful contradiction in terms.  :-)

Alan Stern

Re: Run-time PM idea (was: Re: [RFC][PATCH 0/2] PM: Rearrange core suspend code)

[Rafael J. Wysocki]

On Tuesday 09 June 2009, Oliver Neukum wrote:
> Am Montag, 8. Juni 2009 20:34:30 schrieb Rafael J. Wysocki:
> > On Monday 08 June 2009, Oliver Neukum wrote:
> > > Am Montag, 8. Juni 2009 13:29:26 schrieb Rafael J. Wysocki:
> 
> > > Secondly, you are not using a counter. Therefore only one driver can
> > > control the PM state of a device at a given time. Is that wise?
> >
> > I didn't think about it to be honest.  Obviously this patch doesn't cover
> > all of the possible cases and I'm not even sure it's worth trying to cover
> > them upfront.
> 
> I am thinking of multimedia cards, which have separate drivers for i2c, tuner
> and so on.

Hmm, OK there.  But there's only one bus type per device anyway, isn't it?
So I'm not sure how a counter can help in this case.

> > > Why is this needed?
> >
> > In some subsystems, like PCI, devices will be resumed by the BIOS
> > unconditionally in the majority of cases and then it's not worth trying to
> > complete run-time PM requests from before the suspend.
> 
> But why is it worth canceling them? That feature seems to be an unnecessary
> complication. As long as you can safely suspend them, why not do it?

Because that's an operation that need not be necessary.  I'd like to avoid
unnecessary operations, but you're right, it can be done differently.

> > > > +/**
> > > > + * __pm_schedule_resume - Schedule run-time resume of given device.
> > > > + * @dev: Device to resume.
> > > > + * @autocancel: If set, the request will be cancelled during a resume from a 
> > > > + *	system-wide sleep state if it happens before pm_autoresume() can be run.
> > > > + */
> > >
> > > Eeek! This is a bad idea. You never want to a resume to be cancelled.
> >
> > Sometimes I do (see above).
> 
> Well no. A driver requests a resume because it has to.
> This needs a defined call sequence.
> 
> Do you guarantee that autoresume follows autosuspend or not?

Not necessarily.  If there's an autoresume request pending during STR or STD,
the "sleep resume" will do very much the same thing as the autoresume, it will
put the device into the full power state.  IOW, the "sleep resume" can satisfy
an autoresume request, so there should be a mechanism to cancel pending
autoresume requests during 'sleep resume'.  Still, it may be better if the
driver's or bus type's ->resume() does that.

> If not what sequences can happen? Obviously an autosuspended device
> can be unplugged.
> But the problem here is STR or STD. How do you notify drivers that the BIOS
> has resumed their device instead of autoresume() being called? A driver
> has to know that its device has become active without its knowledge.

Actually, the driver will know what happens to the device anyway, because
its ->resume() callback is going to be executed and it has to be synchronized
with the ->auto[suspend|resume]() callbacks.

> > > > +		cancel_delayed_work_sync(&dev->power.suspend_work);
> > >
> > > That is the most glorious abuse of an API I've seen this year :-)
> >
> > Heh.
> >
> > OK, what would you do instead?
> 
> A waitqueue.

Or perhaps a completion?

OK

I tried to address the majority of your comments in the new version of the
patch which I'm going to send in a while in a reply to an Alan's message.

Best,
Rafael

Re: Run-time PM idea (was: Re: [RFC][PATCH 0/2] PM: Rearrange core suspend code)

[Alan Stern]

On Mon, 8 Jun 2009, Rafael J. Wysocki wrote:

> On Monday 08 June 2009, Oliver Neukum wrote:
> > Am Sonntag, 7. Juni 2009 23:46:59 schrieb Rafael J. Wysocki:
> > > It may be necessary to resume a device synchronously, but I'm still
> > > thinking how to implement that.
> > 
> > This will absolutely be the default. You resume a device because you want
> > it to do something now. It seems to me that you making your problem worse
> > by using a spinlock as a lock. A mutex would make it easier.
> 
> But I need to be able to call __pm_schedule_resume() (at least) from interrupt
> context and I can't use a mutex from there.  Otherwise I'd have used a mutex. :-)
> 
> Anyway, below is a version with synchronous resume.

There are a few things here which need further thought:

The implementation of pm_lock_device() assumes it will never be called 
with interrupts disabled.  This is a bad assumption.

Use of the RPM_UNKNOWN state isn't good.  A bus may have valid reasons 
of its own for not carrying out an autosuspend.  When this happens the 
device's state isn't unknown.

The scheme doesn't include any mechanism for communicating runtime
power information up the device tree.  When a device is autosuspended,
its parent's driver should be told so that the driver can consider
autosuspending the parent.  Likewise, if we want to autoresume a device 
below an autosuspended parent, the parent should be autoresumed first.  
Did you want to make the bus subsystem responsible for all of this?  
What about device's whose parent belongs to a different subsystem?

There should be a sysfs interface (like the one in USB) to allow
userspace to prevent a device from being autosuspended -- and perhaps
also to force it to be suspended.

What about devices that have more than two runtime power states?  For
example, you can't squeeze PCI's {D0,D1,D2,D3hot} range into {running,
suspended}.

That's what I come up with on a first reading.  There may be more later 
on...  :-)

Alan Stern

Re: Run-time PM idea (was: Re: [RFC][PATCH 0/2] PM: Rearrange core suspend code)

[Rafael J. Wysocki]

On Monday 08 June 2009, Ingo Molnar wrote:
> 
> * Rafael J. Wysocki <rjw@sisk.pl> wrote:
> 
> > +config PM_RUNTIME
> > +	bool "Run-time PM core functionality"
> > +	depends on PM
> > +	---help---
> > +	  Enable functionality allowing I/O devices to be put into energy-saving
> > +	  (low power) states at run time (or autosuspended) after a specified
> > +	  period of inactivity and woken up in response to a hardware-generated
> > +	  wake-up event or a driver's request.
> > +
> > +	  Hardware support is generally required for this functionality to work
> > +	  and the bus type drivers of the buses the devices are on are
> > +	  responsibile for the actual handling of the autosuspend requests and
> > +	  wake-up events.
> 
> Halleluya! :-)

I guess this means you like the general idea. ;-)

Well, we've been discussing it for quite a while and since more and more people
are interested, I'm giving it a high priority.

Best,
Rafael

Re: Run-time PM idea (was: Re: [RFC][PATCH 0/2] PM: Rearrange core suspend code)

[Ingo Molnar]

* Rafael J. Wysocki <rjw@sisk.pl> wrote:

> On Monday 08 June 2009, Ingo Molnar wrote:
> > 
> > * Rafael J. Wysocki <rjw@sisk.pl> wrote:
> > 
> > > +config PM_RUNTIME
> > > +	bool "Run-time PM core functionality"
> > > +	depends on PM
> > > +	---help---
> > > +	  Enable functionality allowing I/O devices to be put into energy-saving
> > > +	  (low power) states at run time (or autosuspended) after a specified
> > > +	  period of inactivity and woken up in response to a hardware-generated
> > > +	  wake-up event or a driver's request.
> > > +
> > > +	  Hardware support is generally required for this functionality to work
> > > +	  and the bus type drivers of the buses the devices are on are
> > > +	  responsibile for the actual handling of the autosuspend requests and
> > > +	  wake-up events.
> > 
> > Halleluya! :-)
> 
> I guess this means you like the general idea. ;-)
> 
> Well, we've been discussing it for quite a while and since more 
> and more people are interested, I'm giving it a high priority.

Cool. I think that if within a few years we could achieve that every 
default distro (both on desktops and on servers) triggers PM 
functionality runtime on common hardware, we'd both have lower power 
consumption in general, and we'd have more robust suspend-resume 
code as well.

It would also be far more debuggable if the various suspend/resume 
bits were triggered and used independently and runtime, allowing 
bugs to be 'spread out'. Right now if they fail they fail in a very 
hard to debug spot (in the s2ram/s2disk codepaths), which makes 
their hacking rather challenging. (which i'm sure you are well aware 
of ;-)

So yes, i like the idea, a lot.

	Ingo

Re: Run-time PM idea (was: Re: [RFC][PATCH 0/2] PM: Rearrange core suspend code)

[Matthew Garrett]

On Mon, Jun 08, 2009 at 03:05:09PM +0200, Ingo Molnar wrote:
> 
> * Rafael J. Wysocki <rjw@sisk.pl> wrote:
> > Well, we've been discussing it for quite a while and since more 
> > and more people are interested, I'm giving it a high priority.
> 
> Cool. I think that if within a few years we could achieve that every 
> default distro (both on desktops and on servers) triggers PM 
> functionality runtime on common hardware, we'd both have lower power 
> consumption in general, and we'd have more robust suspend-resume 
> code as well.

The difficulty is in determining when it's viable to autosuspend a given 
device. There's a limit to how much we can determine purely from kernel 
state (for instance, we could suspend ahci when there's no pending disk 
access, but we'd lose hotplug notifications) so there's going to have to 
be some level of userspace policy determination. Having the 
infrastructure in the kernel is an important part of this, but there'll 
be some distance to go after that.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: Run-time PM idea (was: Re: [RFC][PATCH 0/2] PM: Rearrange core suspend code)

[Ingo Molnar]

* Matthew Garrett <mjg59@srcf.ucam.org> wrote:

> On Mon, Jun 08, 2009 at 03:05:09PM +0200, Ingo Molnar wrote:
> > 
> > * Rafael J. Wysocki <rjw@sisk.pl> wrote:
> > > Well, we've been discussing it for quite a while and since more 
> > > and more people are interested, I'm giving it a high priority.
> > 
> > Cool. I think that if within a few years we could achieve that every 
> > default distro (both on desktops and on servers) triggers PM 
> > functionality runtime on common hardware, we'd both have lower power 
> > consumption in general, and we'd have more robust suspend-resume 
> > code as well.
> 
> The difficulty is in determining when it's viable to autosuspend a 
> given device. There's a limit to how much we can determine purely 
> from kernel state (for instance, we could suspend ahci when 
> there's no pending disk access, but we'd lose hotplug 
> notifications) so there's going to have to be some level of 
> userspace policy determination. Having the infrastructure in the 
> kernel is an important part of this, but there'll be some distance 
> to go after that.

What will the 'user space policy' bit do what the kernel cannot?

If you mean the user has to configure something manually - that wont 
really happen in practice. We are happy if they know where to put 
those USB sticks in ;-)

	Ingo

Re: Run-time PM idea (was: Re: [RFC][PATCH 0/2] PM: Rearrange core suspend code)

[Matthew Garrett]

On Mon, Jun 08, 2009 at 03:22:35PM +0200, Ingo Molnar wrote:
> 
> * Matthew Garrett <mjg59@srcf.ucam.org> wrote:
> > The difficulty is in determining when it's viable to autosuspend a 
> > given device. There's a limit to how much we can determine purely 
> > from kernel state (for instance, we could suspend ahci when 
> > there's no pending disk access, but we'd lose hotplug 
> > notifications) so there's going to have to be some level of 
> > userspace policy determination. Having the infrastructure in the 
> > kernel is an important part of this, but there'll be some distance 
> > to go after that.
> 
> What will the 'user space policy' bit do what the kernel cannot?

How does the kernel know whether the user cares about SATA hotplug or 
not?

> If you mean the user has to configure something manually - that wont 
> really happen in practice. We are happy if they know where to put 
> those USB sticks in ;-)

It'll be up to the distributions to provide sane defaults and let them 
be reconfigured as necessary, depending on the information we have from 
the user and maybe platform-specific knowledge. But this is a difficult 
problem - we need to be smart about all the potential sources of 
information in order to pick an appropriate policy, and the kernel's not 
the right layer to do some of this information collection.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: Run-time PM idea (was: Re: [RFC][PATCH 0/2] PM: Rearrange core suspend code)

[Oliver Neukum]

Am Montag, 8. Juni 2009 15:22:35 schrieb Ingo Molnar:

> What will the 'user space policy' bit do what the kernel cannot?
>
> If you mean the user has to configure something manually - that wont
> really happen in practice. We are happy if they know where to put
> those USB sticks in ;-)

User space need not be the user. Currently user space doesn't tell
the kernel how much functionality it needs. open/close give a binary
opposition which badly maps onto the graduated capabilities devices
have.

For example do you really need every key pressed while the screen saver
is running or is it enough for the keyboard to be able to generate a wakeup
event?

	Regards
		Oliver

Re: Run-time PM idea (was: Re: [RFC][PATCH 0/2] PM: Rearrange core suspend code)

[Ingo Molnar]

* Matthew Garrett <mjg59@srcf.ucam.org> wrote:

> On Mon, Jun 08, 2009 at 03:22:35PM +0200, Ingo Molnar wrote:
> > 
> > * Matthew Garrett <mjg59@srcf.ucam.org> wrote:
> > > The difficulty is in determining when it's viable to autosuspend a 
> > > given device. There's a limit to how much we can determine purely 
> > > from kernel state (for instance, we could suspend ahci when 
> > > there's no pending disk access, but we'd lose hotplug 
> > > notifications) so there's going to have to be some level of 
> > > userspace policy determination. Having the infrastructure in the 
> > > kernel is an important part of this, but there'll be some distance 
> > > to go after that.
> > 
> > What will the 'user space policy' bit do what the kernel cannot?
> 
> How does the kernel know whether the user cares about SATA hotplug 
> or not?

The typical user probably doesnt know what 'SATA' means, and 
probably has very vague concepts about 'hotplug' as well.

The kernel default should be: 'yes, if the kernel feature is enabled 
and if the hardware can support it' (it's not on a blacklist of some 
sort, etc., etc.).

> > If you mean the user has to configure something manually - that 
> > wont really happen in practice. We are happy if they know where 
> > to put those USB sticks in ;-)
> 
> It'll be up to the distributions to provide sane defaults and let 
> them be reconfigured as necessary, depending on the information we 
> have from the user and maybe platform-specific knowledge. But this 
> is a difficult problem - we need to be smart about all the 
> potential sources of information in order to pick an appropriate 
> policy, and the kernel's not the right layer to do some of this 
> information collection.

What sources of information exactly? Again, the user wont enter 
anything, in 95% of the cases - in the remaining 3% of cases what is 
entered is wrong and only in another 2% of cases is it correct ;-)

Sane kernel defaults are important and the kernel sure knows what 
kind of hardware it runs on. This 'let the user decide policy' for 
something as fundamental (and also as arcane) as power saving mode 
is really a disease that has caused a lot of unnecessary pain in 
Linux in the past 15 years.

Sure, there might be tradeoffs when a piece of hardware cannot be 
turned off sanely: obviously the monitor might not know it 
(currently) whether someone is watching it, and 
wake-on-packet-for-me is not commonly implemented in wireless and 
wired networking cards so turning off an active networking card 
might not be possible without the user asking allowing that 
imperfect mode of power saving.

But there are plenty of cases where turning off hardware is fine, 
and the broken special cases will go away as technology advances, 
and we should not design based on broken concepts.

( Providing a way to _override_ those defaults is of course natural,
  via /sysfs, should the user express an interest in tweaking it, or
  should the kernel get it so wrong that a distro wants to work it
  around. But your argument seems to be "push configuration and
  handling into user-space" which is really backwards. )

	Ingo

Re: Run-time PM idea (was: Re: [RFC][PATCH 0/2] PM: Rearrange core suspend code)

[Matthew Garrett]

On Mon, Jun 08, 2009 at 03:46:47PM +0200, Ingo Molnar wrote:
> 
> * Matthew Garrett <mjg59@srcf.ucam.org> wrote:
> > How does the kernel know whether the user cares about SATA hotplug 
> > or not?
> 
> The typical user probably doesnt know what 'SATA' means, and 
> probably has very vague concepts about 'hotplug' as well.

eSATA is pretty common now.

> The kernel default should be: 'yes, if the kernel feature is enabled 
> and if the hardware can support it' (it's not on a blacklist of some 
> sort, etc., etc.).

The problem with this kind of default is that you get people who are 
confused that their hardware doesn't work. If the kernel doesn't have 
enough information to make a decision it should err on the side of 
functionality - we're talking about fairly low-level power savings, but 
potentially several years of aggregate confusion on the part of users.

> > It'll be up to the distributions to provide sane defaults and let 
> > them be reconfigured as necessary, depending on the information we 
> > have from the user and maybe platform-specific knowledge. But this 
> > is a difficult problem - we need to be smart about all the 
> > potential sources of information in order to pick an appropriate 
> > policy, and the kernel's not the right layer to do some of this 
> > information collection.
> 
> What sources of information exactly? Again, the user wont enter 
> anything, in 95% of the cases - in the remaining 3% of cases what is 
> entered is wrong and only in another 2% of cases is it correct ;-)

Users are generally ok at realising correlation between a setting change 
and something no longer working, so as long as you provide that they'll 
be happy. I agree that this sucks. What we actually want is some means 
of reliably identifying whether a port is hotplug or not, but eSATA 
makes this very difficult.

> Sure, there might be tradeoffs when a piece of hardware cannot be 
> turned off sanely: obviously the monitor might not know it 
> (currently) whether someone is watching it, and 
> wake-on-packet-for-me is not commonly implemented in wireless and 
> wired networking cards so turning off an active networking card 
> might not be possible without the user asking allowing that 
> imperfect mode of power saving.

These cases can all be handled with sufficiently intelligent userland, 
so I'm not worried about them.

> ( Providing a way to _override_ those defaults is of course natural,
>   via /sysfs, should the user express an interest in tweaking it, or
>   should the kernel get it so wrong that a distro wants to work it
>   around. But your argument seems to be "push configuration and
>   handling into user-space" which is really backwards. )

My argument is "Hardware should work, and if the kernel default is for 
it to be broken then the default is wrong". We went through this for USB 
autosuspend. Userspace simply has more available information than the 
kernel, and it's not just a matter of static configuration (though that 
may be part of it). For instance, Oliver's example of screensavers and 
USB keyboards. If nothing's paying attention to volume keys (or if the 
keyboard doesn't have any) then you can enable remote wakeup and suspend 
the keyboard. If something /is/ paying attention to volume keys, you 
can't do that. That's the kind of case I'm discussing.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: Run-time PM idea (was: Re: [RFC][PATCH 0/2] PM: Rearrange core suspend code)

[Oliver Neukum]

Am Montag, 8. Juni 2009 15:46:47 schrieb Ingo Molnar:
> ( Providing a way to _override_ those defaults is of course natural,
>   via /sysfs, should the user express an interest in tweaking it, or
>   should the kernel get it so wrong that a distro wants to work it
>   around. But your argument seems to be "push configuration and
>   handling into user-space" which is really backwards. )

If we agree that the default shall be that the kernel doesn't switch
off features of the hardware for power saving by default, does this
make a practical difference to keeping the configuration in user space?

	Regards
		Oliver

Re: Run-time PM idea (was: Re: [RFC][PATCH 0/2] PM: Rearrange core suspend code)

[Ingo Molnar]

* Matthew Garrett <mjg59@srcf.ucam.org> wrote:

> On Mon, Jun 08, 2009 at 03:46:47PM +0200, Ingo Molnar wrote:
> > 
> > * Matthew Garrett <mjg59@srcf.ucam.org> wrote:
>
> > > How does the kernel know whether the user cares about SATA 
> > > hotplug or not?
> > 
> > The typical user probably doesnt know what 'SATA' means, and 
> > probably has very vague concepts about 'hotplug' as well.
> 
> eSATA is pretty common now.

[ And 99% of the CPUs have an IDT still 99.9% of the users dont know 
  what it is :) ]

> > The kernel default should be: 'yes, if the kernel feature is 
> > enabled and if the hardware can support it' (it's not on a 
> > blacklist of some sort, etc., etc.).
> 
> The problem with this kind of default is that you get people who 
> are confused that their hardware doesn't work.

If the hardware 'doesnt work' that is a kernel bug. Hardware that 
_cannot be suspended_ safely (physically) should not be 
auto-suspended, of course.

> If the kernel doesn't have enough information to make a decision 
> it should err on the side of functionality - we're talking about 
> fairly low-level power savings, but potentially several years of 
> aggregate confusion on the part of users.

the difference between a 10W and a 1W footprint is a long series of 
'low-level power savings'.

If users are getting confused and if hardware gets broken then tha's 
a plain bug and the wrong path is being walked.

> > What sources of information exactly? Again, the user wont enter 
> > anything, in 95% of the cases - in the remaining 3% of cases 
> > what is entered is wrong and only in another 2% of cases is it 
> > correct ;-)
> 
> Users are generally ok at realising correlation between a setting 
> change and something no longer working, so as long as you provide 
> that they'll be happy. I agree that this sucks. What we actually 
> want is some means of reliably identifying whether a port is 
> hotplug or not, but eSATA makes this very difficult.

Is it impossible?

> > Sure, there might be tradeoffs when a piece of hardware cannot 
> > be turned off sanely: obviously the monitor might not know it 
> > (currently) whether someone is watching it, and 
> > wake-on-packet-for-me is not commonly implemented in wireless 
> > and wired networking cards so turning off an active networking 
> > card might not be possible without the user asking allowing that 
> > imperfect mode of power saving.
> 
> These cases can all be handled with sufficiently intelligent 
> userland, so I'm not worried about them.
> 
> > ( Providing a way to _override_ those defaults is of course natural,
> >   via /sysfs, should the user express an interest in tweaking it, or
> >   should the kernel get it so wrong that a distro wants to work it
> >   around. But your argument seems to be "push configuration and
> >   handling into user-space" which is really backwards. )
> 
> My argument is "Hardware should work, and if the kernel default is 
> for it to be broken then the default is wrong". We went through 
> this for USB autosuspend. Userspace simply has more available 
> information than the kernel, and it's not just a matter of static 
> configuration (though that may be part of it). For instance, 
> Oliver's example of screensavers and USB keyboards. If nothing's 
> paying attention to volume keys (or if the keyboard doesn't have 
> any) then you can enable remote wakeup and suspend the keyboard. 
> If something /is/ paying attention to volume keys, you can't do 
> that. That's the kind of case I'm discussing.

See my reply to Oliver. This is really advocating a broken model of 
device usage. That volume key usage dependency is being hidden from 
the kernel, and then you want to kludge it around by pushing suspend 
functionality to user-space? That way lies madness. The proper way 
is to close the device if it's not used by anything. Then the kernel 
can auto-suspend it just like it could auto-suspend network 
interfaces that are not in use, or like it could auto-suspend a 
dislay port that has no monitor or other output device attached.

	Ingo

Re: Run-time PM idea (was: Re: [RFC][PATCH 0/2] PM: Rearrange core suspend code)

[Matthew Garrett]

On Mon, Jun 08, 2009 at 04:24:50PM +0200, Ingo Molnar wrote:
> * Matthew Garrett <mjg59@srcf.ucam.org> wrote:
> > eSATA is pretty common now.
> 
> [ And 99% of the CPUs have an IDT still 99.9% of the users dont know 
>   what it is :) ]

Users know that there's a socket on the front of their computer that 
they can plug a hard drive into, and if that doesn't work then they're 
going to be upset.

> > The problem with this kind of default is that you get people who 
> > are confused that their hardware doesn't work.
> 
> If the hardware 'doesnt work' that is a kernel bug. Hardware that 
> _cannot be suspended_ safely (physically) should not be 
> auto-suspended, of course.

So, like I said, the kernel can't automatically suspend AHCI unless it's 
received some information from elsewhere that tells it it's ok to. The 
kernel can't know if there's an eSATA port or not.

> > If the kernel doesn't have enough information to make a decision 
> > it should err on the side of functionality - we're talking about 
> > fairly low-level power savings, but potentially several years of 
> > aggregate confusion on the part of users.
> 
> the difference between a 10W and a 1W footprint is a long series of 
> 'low-level power savings'.
> 
> If users are getting confused and if hardware gets broken then tha's 
> a plain bug and the wrong path is being walked.

Yes. And powersaving is a tradeoff between functionality and power 
consumption. The kernel doesn't know what level of functionality a given 
user requires. It *can't* know that itself.

> > Users are generally ok at realising correlation between a setting 
> > change and something no longer working, so as long as you provide 
> > that they'll be happy. I agree that this sucks. What we actually 
> > want is some means of reliably identifying whether a port is 
> > hotplug or not, but eSATA makes this very difficult.
> 
> Is it impossible?

To the best of my knowledge, yes.

> > My argument is "Hardware should work, and if the kernel default is 
> > for it to be broken then the default is wrong". We went through 
> > this for USB autosuspend. Userspace simply has more available 
> > information than the kernel, and it's not just a matter of static 
> > configuration (though that may be part of it). For instance, 
> > Oliver's example of screensavers and USB keyboards. If nothing's 
> > paying attention to volume keys (or if the keyboard doesn't have 
> > any) then you can enable remote wakeup and suspend the keyboard. 
> > If something /is/ paying attention to volume keys, you can't do 
> > that. That's the kind of case I'm discussing.
> 
> See my reply to Oliver. This is really advocating a broken model of 
> device usage. That volume key usage dependency is being hidden from 
> the kernel, and then you want to kludge it around by pushing suspend 
> functionality to user-space? That way lies madness. The proper way 
> is to close the device if it's not used by anything. Then the kernel 
> can auto-suspend it just like it could auto-suspend network 
> interfaces that are not in use, or like it could auto-suspend a 
> dislay port that has no monitor or other output device attached.

No, we can't just close it - then we won't get notification that a key's 
been hit in order to unlock the screensaver. Yes, we can greatly expand 
the userland-visible interface to every piece of hardware in order to 
make this work, but that's a huge amount of effort to avoid a model 
where userspace sets some tunables appropriately.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: Run-time PM idea (was: Re: [RFC][PATCH 0/2] PM: Rearrange core suspend code)

[Ingo Molnar]

* Matthew Garrett <mjg59@srcf.ucam.org> wrote:

> On Mon, Jun 08, 2009 at 04:24:50PM +0200, Ingo Molnar wrote:
> > * Matthew Garrett <mjg59@srcf.ucam.org> wrote:
> > > eSATA is pretty common now.
> > 
> > [ And 99% of the CPUs have an IDT still 99.9% of the users dont know 
> >   what it is :) ]
> 
> Users know that there's a socket on the front of their computer that 
> they can plug a hard drive into, and if that doesn't work then they're 
> going to be upset.
> 
> > > The problem with this kind of default is that you get people who 
> > > are confused that their hardware doesn't work.
> > 
> > If the hardware 'doesnt work' that is a kernel bug. Hardware that 
> > _cannot be suspended_ safely (physically) should not be 
> > auto-suspended, of course.
> 
> So, like I said, the kernel can't automatically suspend AHCI unless it's 
> received some information from elsewhere that tells it it's ok to. The 
> kernel can't know if there's an eSATA port or not.
> 
> > > If the kernel doesn't have enough information to make a decision 
> > > it should err on the side of functionality - we're talking about 
> > > fairly low-level power savings, but potentially several years of 
> > > aggregate confusion on the part of users.
> > 
> > the difference between a 10W and a 1W footprint is a long series of 
> > 'low-level power savings'.
> > 
> > If users are getting confused and if hardware gets broken then tha's 
> > a plain bug and the wrong path is being walked.
> 
> Yes. And powersaving is a tradeoff between functionality and power 
> consumption. The kernel doesn't know what level of functionality a 
> given user requires. It *can't* know that itself.
> 
> > > Users are generally ok at realising correlation between a setting 
> > > change and something no longer working, so as long as you provide 
> > > that they'll be happy. I agree that this sucks. What we actually 
> > > want is some means of reliably identifying whether a port is 
> > > hotplug or not, but eSATA makes this very difficult.
> > 
> > Is it impossible?
> 
> To the best of my knowledge, yes.
> 
> > > My argument is "Hardware should work, and if the kernel default is 
> > > for it to be broken then the default is wrong". We went through 
> > > this for USB autosuspend. Userspace simply has more available 
> > > information than the kernel, and it's not just a matter of static 
> > > configuration (though that may be part of it). For instance, 
> > > Oliver's example of screensavers and USB keyboards. If nothing's 
> > > paying attention to volume keys (or if the keyboard doesn't have 
> > > any) then you can enable remote wakeup and suspend the keyboard. 
> > > If something /is/ paying attention to volume keys, you can't do 
> > > that. That's the kind of case I'm discussing.
> > 
> > See my reply to Oliver. This is really advocating a broken model 
> > of device usage. That volume key usage dependency is being 
> > hidden from the kernel, and then you want to kludge it around by 
> > pushing suspend functionality to user-space? That way lies 
> > madness. The proper way is to close the device if it's not used 
> > by anything. Then the kernel can auto-suspend it just like it 
> > could auto-suspend network interfaces that are not in use, or 
> > like it could auto-suspend a dislay port that has no monitor or 
> > other output device attached.
> 
> No, we can't just close it - then we won't get notification that a 
> key's been hit in order to unlock the screensaver. [...]

Looks like a broken notification model.

> [...] Yes, we can greatly expand the userland-visible interface to 
> every piece of hardware in order to make this work, but that's a 
> huge amount of effort to avoid a model where userspace sets some 
> tunables appropriately.

What huge amount of effort? All you are doing is to track the "is 
the device really used" state in user-space - and, if the current 
desktop experience is any measure, highly imperfectly so.

What i'm suggesting is to track it properly in the kernel. It's not 
like the kernel doesnt need to know whether a piece of hardware is 
under use or not ...

	Ingo

Re: Run-time PM idea (was: Re: [RFC][PATCH 0/2] PM: Rearrange core suspend code)

[Matthew Garrett]

On Mon, Jun 08, 2009 at 04:44:55PM +0200, Ingo Molnar wrote:
> 
> * Matthew Garrett <mjg59@srcf.ucam.org> wrote:
> > No, we can't just close it - then we won't get notification that a 
> > key's been hit in order to unlock the screensaver. [...]
> 
> Looks like a broken notification model.

We've closed the input device. Where are we supposed to get the input 
event from?

> > [...] Yes, we can greatly expand the userland-visible interface to 
> > every piece of hardware in order to make this work, but that's a 
> > huge amount of effort to avoid a model where userspace sets some 
> > tunables appropriately.
> 
> What huge amount of effort? All you are doing is to track the "is 
> the device really used" state in user-space - and, if the current 
> desktop experience is any measure, highly imperfectly so.
> 
> What i'm suggesting is to track it properly in the kernel. It's not 
> like the kernel doesnt need to know whether a piece of hardware is 
> under use or not ...

So, for instance, we need to add interfaces like "I care about hotplug 
events on this SATA port" and "I'm listening for these keys so please 
don't suspend the device" and "The service bound to this port needs to 
maintain network connectivity and the one bound to this port doesn't, so 
only put the wireless card into deep powersave if the first exits", and 
then we need to wait for userspace to adopt these interfaces before we 
can enable any of the functionality because otherwise old userspace will 
be broken with new kernels.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: Run-time PM idea (was: Re: [RFC][PATCH 0/2] PM: Rearrange core suspend code)

[Pavel Machek]

> > > [...] Yes, we can greatly expand the userland-visible interface to 
> > > every piece of hardware in order to make this work, but that's a 
> > > huge amount of effort to avoid a model where userspace sets some 
> > > tunables appropriately.
> > 
> > What huge amount of effort? All you are doing is to track the "is 
> > the device really used" state in user-space - and, if the current 
> > desktop experience is any measure, highly imperfectly so.
> > 
> > What i'm suggesting is to track it properly in the kernel. It's not 
> > like the kernel doesnt need to know whether a piece of hardware is 
> > under use or not ...
> 
> So, for instance, we need to add interfaces like "I care about hotplug 
> events on this SATA port" and "I'm listening for these keys so please 
> don't suspend the device" and "The service bound to this port needs to 
> maintain network connectivity and the one bound to this port doesn't, so 
> only put the wireless card into deep powersave if the first exits", and 
> then we need to wait for userspace to adopt these interfaces before we 
> can enable any of the functionality because otherwise old userspace will 
> be broken with new kernels.

Yes, that's the way to go. It is not particulary easy way, but at
least such userspace will work with upcoming hardware and kernel will
be able to get features such as 'system autosuspend'...
								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: Run-time PM idea (was: Re: [RFC][PATCH 0/2] PM: Rearrange core suspend code)

[Robert Hancock]

On 06/08/2009 08:35 AM, Matthew Garrett wrote:
> On Mon, Jun 08, 2009 at 04:24:50PM +0200, Ingo Molnar wrote:
>> * Matthew Garrett<mjg59@srcf.ucam.org>  wrote:
>>> eSATA is pretty common now.
>> [ And 99% of the CPUs have an IDT still 99.9% of the users dont know
>>    what it is :) ]
>
> Users know that there's a socket on the front of their computer that
> they can plug a hard drive into, and if that doesn't work then they're
> going to be upset.
>
>>> The problem with this kind of default is that you get people who
>>> are confused that their hardware doesn't work.
>> If the hardware 'doesnt work' that is a kernel bug. Hardware that
>> _cannot be suspended_ safely (physically) should not be
>> auto-suspended, of course.
>
> So, like I said, the kernel can't automatically suspend AHCI unless it's
> received some information from elsewhere that tells it it's ok to. The
> kernel can't know if there's an eSATA port or not.
>
>>> If the kernel doesn't have enough information to make a decision
>>> it should err on the side of functionality - we're talking about
>>> fairly low-level power savings, but potentially several years of
>>> aggregate confusion on the part of users.
>> the difference between a 10W and a 1W footprint is a long series of
>> 'low-level power savings'.
>>
>> If users are getting confused and if hardware gets broken then tha's
>> a plain bug and the wrong path is being walked.
>
> Yes. And powersaving is a tradeoff between functionality and power
> consumption. The kernel doesn't know what level of functionality a given
> user requires. It *can't* know that itself.
>
>>> Users are generally ok at realising correlation between a setting
>>> change and something no longer working, so as long as you provide
>>> that they'll be happy. I agree that this sucks. What we actually
>>> want is some means of reliably identifying whether a port is
>>> hotplug or not, but eSATA makes this very difficult.
>> Is it impossible?
>
> To the best of my knowledge, yes.

Well, in some cases we can get an idea - the current AHCI spec has bits 
in the PxCMD register (External SATA Port and Hot Plug Capable Port) 
which can indicate which ports are externally accessible and thus are 
likely to receive hotplug events. Of course, these are supposed to be 
programmed by the BIOS based on the particular motherboard/machine, and 
we all know how accurate BIOS-reported information can be..

Re: Run-time PM idea (was: Re: [RFC][PATCH 0/2] PM: Rearrange core suspend code)

[Matthew Garrett]

On Mon, Jun 08, 2009 at 03:39:19PM +0200, Oliver Neukum wrote:

> For example do you really need every key pressed while the screen saver
> is running or is it enough for the keyboard to be able to generate a wakeup
> event?

Depends whether you have a media player running or not...

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: Run-time PM idea (was: Re: [RFC][PATCH 0/2] PM: Rearrange core suspend code)

[Ingo Molnar]

* Oliver Neukum <oliver@neukum.org> wrote:

> Am Montag, 8. Juni 2009 15:22:35 schrieb Ingo Molnar:
> 
> > What will the 'user space policy' bit do what the kernel cannot?
> >
> > If you mean the user has to configure something manually - that 
> > wont really happen in practice. We are happy if they know where 
> > to put those USB sticks in ;-)
> 
> User space need not be the user. Currently user space doesn't tell 
> the kernel how much functionality it needs. open/close give a 
> binary opposition which badly maps onto the graduated capabilities 
> devices have.

If the kernel isnt told what capabilities are used that's buggy code 
then.

> For example do you really need every key pressed while the screen 
> saver is running or is it enough for the keyboard to be able to 
> generate a wakeup event?

The sane default here is to suspend the keyboard, except if an audio 
app is running that binds to the volume keys of the keyboard.

If the 'keyboard' is properly abstracted in the kernel and the 
kernel driver _knows_ that the volume keys are in use, this is not a 
problem.

Arguing otherwise is just saying the equivalent of: "we have a 
broken model to utilize hardware, and instead of fixing it properly, 
introduce an _even more broken_ model, because in the current model 
things cannot be made to work".

The kernel _needs_ to have precise information about whether a piece 
of hardware is in use or not.

	Ingo

Re: Run-time PM idea (was: Re: [RFC][PATCH 0/2] PM: Rearrange core suspend code)

[Matthew Garrett]

On Mon, Jun 08, 2009 at 04:21:54PM +0200, Ingo Molnar wrote:

> The kernel _needs_ to have precise information about whether a piece 
> of hardware is in use or not.

The kernel can only have that information if userspace tells it. What 
we're quibbling over is whether the kernel should be explicitly told 
about the requirement (ie, every time an app makes a key grab in X the 
kernel gets told about it) or whether it should be implicit (userspace 
knows that a key grab has been made and so requests that the keyboard 
not be suspended).

We *can* put all of that complexity in the kernel. The question is 
whether it buys us anything. We'd have to modify huge chunks of 
userspace and in the process we'd end up limited to whatever policy 
happens to exist in the version of the kernel the user is running.

I'd like the kernel to expose this functionality but leave the policy 
decisions to userland.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: Run-time PM idea (was: Re: [RFC][PATCH 0/2] PM: Rearrange core suspend code)

[Ingo Molnar]

* Matthew Garrett <mjg59@srcf.ucam.org> wrote:

> On Mon, Jun 08, 2009 at 04:21:54PM +0200, Ingo Molnar wrote:
> 
> > The kernel _needs_ to have precise information about whether a piece 
> > of hardware is in use or not.
> 
> The kernel can only have that information if userspace tells it. 
> What we're quibbling over is whether the kernel should be 
> explicitly told about the requirement (ie, every time an app makes 
> a key grab in X the kernel gets told about it) or whether it 
> should be implicit (userspace knows that a key grab has been made 
> and so requests that the keyboard not be suspended).
> 
> We *can* put all of that complexity in the kernel. The question is 
> whether it buys us anything. We'd have to modify huge chunks of 
> userspace and in the process we'd end up limited to whatever 
> policy happens to exist in the version of the kernel the user is 
> running.
> 
> I'd like the kernel to expose this functionality but leave the 
> policy decisions to userland.

The thing is, suspending something that is being used and relied on 
by an app is a _bug_. This is rather fundamental: hardware state and 
usage tracking is _NOT POLICY_.

Having a global override of "dont ever suspend anything here, 
because i say so" _is_ policy.

Providing _essential_ functionality to not suspend a keyboard while 
an app relies on it is _NOT_ policy.

I will even buy the argument that most current hardware cannot be 
auto-suspended safely.

But if you think that tracking the usage state of the hardware is 
'complexity', then you very much dont know what you are talking 
about. The main task of the kernel is to track hardware usage and to 
abstract away the fact that the same hardware is used by multiple 
tasks, and to do it safely. It's what the kernel does all day.

	Ingo

Re: Run-time PM idea (was: Re: [RFC][PATCH 0/2] PM: Rearrange core suspend code)

[Matthew Garrett]

On Mon, Jun 08, 2009 at 05:06:03PM +0200, Ingo Molnar wrote:

> But if you think that tracking the usage state of the hardware is 
> 'complexity', then you very much dont know what you are talking 
> about. The main task of the kernel is to track hardware usage and to 
> abstract away the fact that the same hardware is used by multiple 
> tasks, and to do it safely. It's what the kernel does all day.

What I'm saying is that you don't *know* what the usage state of the 
hardware is, and in many cases you can't know. A given user may be happy 
to sacrifice their SATA hotplug support. Another with identical hardware 
may not. A given network application may be mission critical and 
intolerant of the network interface being shut down. The same 
application in a different context may not. We'd need to provide a 
bewildering array of interfaces to distinguish between these situations, 
and we'd be unable to turn on autosuspend until the entirity of 
userspace had been ported to them.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: Run-time PM idea (was: Re: [RFC][PATCH 0/2] PM: Rearrange core suspend code)

[Ray Lee]

On Mon, Jun 8, 2009 at 8:06 AM, Ingo Molnar<mingo@elte.hu> wrote:
>
> * Matthew Garrett <mjg59@srcf.ucam.org> wrote:

>> I'd like the kernel to expose this functionality but leave the
>> policy decisions to userland.
>
> The thing is, suspending something that is being used and relied on
> by an app is a _bug_. This is rather fundamental: hardware state and
> usage tracking is _NOT POLICY_.

Yes. But, actual example time: what about the case where completely
turning off a laptop's DVD drive saves extra power, but then also
turns off kernel and userspace notification of a disc being inserted?

One of the Other OS's handles this by having a popup when the battery
gets low, asking if it's okay to turn the drive off. This is part of
what Matthew is talking about here.

Re: Run-time PM idea (was: Re: [RFC][PATCH 0/2] PM: Rearrange core suspend code)

[Jiri Kosina]

On Mon, 8 Jun 2009, Ingo Molnar wrote:

> > For example do you really need every key pressed while the screen 
> > saver is running or is it enough for the keyboard to be able to 
> > generate a wakeup event?
> The sane default here is to suspend the keyboard, except if an audio 
> app is running that binds to the volume keys of the keyboard.
> If the 'keyboard' is properly abstracted in the kernel and the 
> kernel driver _knows_ that the volume keys are in use, this is not a 
> problem.

So, if you want to abstract this properly, you are proposing that the 
application should in some sense "bind to keyboard keys"?

That has several drawbacks:

- applications in the current universe don't do that
- it's awful overhead: 
  + it apparently wouldn't have any other use than for waking up from 
    autosuspended mode (possibly while screensaver is running)
  + I believe that application writers will find it a little boring to 
    have to start all their main() functions with explicit eunumeration of 
    the keys the application is expecting :)
- even if we require applications to do so, there will be ones violating 
  this rule (i.e. kernel only knows what userspace tells him, in this 
  situation ... is this reliable enough?)

To sum it up -- I don't think that what you are proposing will work.

Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: Run-time PM idea (was: Re: [RFC][PATCH 0/2] PM: Rearrange core suspend code)

[Rafael J. Wysocki]

On Monday 08 June 2009, Alan Stern wrote:
> On Mon, 8 Jun 2009, Rafael J. Wysocki wrote:
> 
> > On Monday 08 June 2009, Oliver Neukum wrote:
> > > Am Sonntag, 7. Juni 2009 23:46:59 schrieb Rafael J. Wysocki:
> > > > It may be necessary to resume a device synchronously, but I'm still
> > > > thinking how to implement that.
> > > 
> > > This will absolutely be the default. You resume a device because you want
> > > it to do something now. It seems to me that you making your problem worse
> > > by using a spinlock as a lock. A mutex would make it easier.
> > 
> > But I need to be able to call __pm_schedule_resume() (at least) from interrupt
> > context and I can't use a mutex from there.  Otherwise I'd have used a mutex. :-)
> > 
> > Anyway, below is a version with synchronous resume.
> 
> There are a few things here which need further thought:
> 
> The implementation of pm_lock_device() assumes it will never be called 
> with interrupts disabled.  This is a bad assumption.

Indeed.

> Use of the RPM_UNKNOWN state isn't good.  A bus may have valid reasons 
> of its own for not carrying out an autosuspend.  When this happens the 
> device's state isn't unknown.

I'm not sure what you mean exactly.

If ->autosuspend() fails, the device power state may be known, but the core
can't be sure if the device is active.  This information is available to the
driver and/or the bus type, which should change the status to whatever is
appropriate.

The name of this constant may be confusing, but I didn't have any better ideas.

> The scheme doesn't include any mechanism for communicating runtime
> power information up the device tree.  When a device is autosuspended,
> its parent's driver should be told so that the driver can consider
> autosuspending the parent.

I thought the bus type's ->autosuspend() callback could take care of this.

> Likewise, if we want to autoresume a device below an autosuspended parent,
> the parent should be autoresumed first.  Did you want to make the bus
> subsystem responsible for all of this?

Yes, that was the idea.
 
> What about device's whose parent belongs to a different subsystem?

Good question. :-)

I think that requires some research.  Probably a USB device attached to a PCI
USB controller is a good example here, but we first need to have a prototype
implementation for PCI to carry out some testing.

In fact I'd like to avoid the complexity for now and consider one bus type at a
time.  Especially that, for example, for PCI we won't autosuspend bridges
initially, so this case is going to be really simple.

> There should be a sysfs interface (like the one in USB) to allow
> userspace to prevent a device from being autosuspended -- and perhaps
> also to force it to be suspended.

To prevent a device from being suspended - yes.  To force it to stay suspended
- I'm not sure.

Anyway, that will be the next step.

> What about devices that have more than two runtime power states?  For
> example, you can't squeeze PCI's {D0,D1,D2,D3hot} range into {running,
> suspended}.

That has to be bus type-specific.

In the case of PCI all of the low power states (D1-D3) are in fact substates of
"suspended", because we generally need to quiesce the device before putting
it into any of these states.

I'm not sure if we can introduce more "levels of suspension", so to speak, at
the core level, but in any case we can easily distinguish between "device
quiesced and in a low power state" and "device fully active".

So, in this picture the device is "suspended" from the core's point of view
once it's bus type's ->autosuspend() callback has been successfully executed. 

> That's what I come up with on a first reading.  There may be more later 
> on...  :-)

Sure.

Thanks for your comments! :-)

Best,
Rafael

Re: Run-time PM idea (was: Re: [RFC][PATCH 0/2] PM: Rearrange core suspend code)

[Alan Stern]

On Mon, 8 Jun 2009, Rafael J. Wysocki wrote:

> > Use of the RPM_UNKNOWN state isn't good.  A bus may have valid reasons 
> > of its own for not carrying out an autosuspend.  When this happens the 
> > device's state isn't unknown.
> 
> I'm not sure what you mean exactly.
> 
> If ->autosuspend() fails, the device power state may be known, but the core
> can't be sure if the device is active.  This information is available to the
> driver and/or the bus type, which should change the status to whatever is
> appropriate.

But no matter what the driver or bus type sets the state to, your 
pm_autosuspend() will change it to one of RPM_UNKNOWN or RPM_SUSPENDED.  
Neither might be right.

> The name of this constant may be confusing, but I didn't have any better ideas.

It's not clear what RPM_ACTIVE, RPM_IDLE, and RPM_SUSPENDED are 
supposed to mean; this should be documented in the code.  Also, why 
isn't there RPM_RESUMING?

By the way, a legitimate reason for aborting an autosuspend is if the
device's driver requires remote wakeup to be enabled during suspend but
the user has disabled it.

> > The scheme doesn't include any mechanism for communicating runtime
> > power information up the device tree.  When a device is autosuspended,
> > its parent's driver should be told so that the driver can consider
> > autosuspending the parent.
> 
> I thought the bus type's ->autosuspend() callback could take care of this.

Shouldn't this happen after the device's state has changed to 
RPM_SUSPENDED?  That's not until after the callback returns.

> > There should be a sysfs interface (like the one in USB) to allow
> > userspace to prevent a device from being autosuspended -- and perhaps
> > also to force it to be suspended.
> 
> To prevent a device from being suspended - yes.  To force it to stay suspended
> - I'm not sure.

I'm not sure either.  Oliver Neukum requested it originally and it has
been useful for debugging, but I haven't seen many places where it
would come in useful in practice.

> > What about devices that have more than two runtime power states?  For
> > example, you can't squeeze PCI's {D0,D1,D2,D3hot} range into {running,
> > suspended}.
> 
> That has to be bus type-specific.
> 
> In the case of PCI all of the low power states (D1-D3) are in fact substates of
> "suspended", because we generally need to quiesce the device before putting
> it into any of these states.
> 
> I'm not sure if we can introduce more "levels of suspension", so to speak, at
> the core level, but in any case we can easily distinguish between "device
> quiesced and in a low power state" and "device fully active".
> 
> So, in this picture the device is "suspended" from the core's point of view
> once it's bus type's ->autosuspend() callback has been successfully executed. 

This too should be documented in the code.  Or in a Documentation file.

Alan Stern

Re: Run-time PM idea (was: Re: [RFC][PATCH 0/2] PM: Rearrange core suspend code)

[Oliver Neukum]

Am Montag, 8. Juni 2009 23:31:58 schrieb Rafael J. Wysocki:
> If ->autosuspend() fails, the device power state may be known, but the core
> can't be sure if the device is active.  This information is available to
> the driver and/or the bus type, which should change the status to whatever
> is appropriate.

That is quite confusing. You'd better define error returns.
One that would mean that the suspension has failed but the device is
unaffected, and another one that means that the device is in an
undefined state now.

> > The scheme doesn't include any mechanism for communicating runtime
> > power information up the device tree.  When a device is autosuspended,
> > its parent's driver should be told so that the driver can consider
> > autosuspending the parent.
>
> I thought the bus type's ->autosuspend() callback could take care of this.

That can't work because you have to operate between busses.

> > Likewise, if we want to autoresume a device below an autosuspended
> > parent, the parent should be autoresumed first.  Did you want to make the
> > bus subsystem responsible for all of this?
>
> Yes, that was the idea.

That is an important point. Can some subsytems operate with a parent still
suspended?

	Regards
		Oliver

Re: Run-time PM idea (was: Re: [RFC][PATCH 0/2] PM: Rearrange core suspend code)

[Rafael J. Wysocki]

On Tuesday 09 June 2009, Oliver Neukum wrote:
> Am Montag, 8. Juni 2009 23:31:58 schrieb Rafael J. Wysocki:
> > If ->autosuspend() fails, the device power state may be known, but the core
> > can't be sure if the device is active.  This information is available to
> > the driver and/or the bus type, which should change the status to whatever
> > is appropriate.
> 
> That is quite confusing. You'd better define error returns.

That might work too, but the information need not be available to the driver
immediately.  It may need to schedule a reset of the device to recover from
the error condition, for example.

> One that would mean that the suspension has failed but the device is
> unaffected, and another one that means that the device is in an
> undefined state now.
>
> > > The scheme doesn't include any mechanism for communicating runtime
> > > power information up the device tree.  When a device is autosuspended,
> > > its parent's driver should be told so that the driver can consider
> > > autosuspending the parent.
> >
> > I thought the bus type's ->autosuspend() callback could take care of this.
> 
> That can't work because you have to operate between busses.

OK, point taken.

> > > Likewise, if we want to autoresume a device below an autosuspended
> > > parent, the parent should be autoresumed first.  Did you want to make the
> > > bus subsystem responsible for all of this?
> >
> > Yes, that was the idea.
> 
> That is an important point. Can some subsytems operate with a parent still
> suspended?

OK, I see the value of doing that at the core level.

I tried to address this in the new version of the patch, which has been sent
in my last reply to Alan.

Thanks,
Rafael

Re: Run-time PM idea (was: Re: [RFC][PATCH 0/2] PM: Rearrange core suspend code)

[Oliver Neukum]

Am Dienstag, 9. Juni 2009 16:33:12 schrieb Alan Stern:
> On Tue, 9 Jun 2009, Oliver Neukum wrote:
> > But the problem here is STR or STD. How do you notify drivers that the
> > BIOS has resumed their device instead of autoresume() being called? A
> > driver has to know that its device has become active without its
> > knowledge.
>
> That would be a wonderful contradiction in terms.  :-)

A practical application of the uncertainty principle in quantum computing.

But you have to notify a driver if you notice that a device's power state
has been changed without the knowledge of its driver.

	Regards
		Oliver

Re: Run-time PM idea (was: Re: [RFC][PATCH 0/2] PM: Rearrange core suspend code)

[Rafael J. Wysocki]

On Tuesday 09 June 2009, Alan Stern wrote:
> On Mon, 8 Jun 2009, Rafael J. Wysocki wrote:
> 
> > > Use of the RPM_UNKNOWN state isn't good.  A bus may have valid reasons 
> > > of its own for not carrying out an autosuspend.  When this happens the 
> > > device's state isn't unknown.
> > 
> > I'm not sure what you mean exactly.
> > 
> > If ->autosuspend() fails, the device power state may be known, but the core
> > can't be sure if the device is active.  This information is available to the
> > driver and/or the bus type, which should change the status to whatever is
> > appropriate.
> 
> But no matter what the driver or bus type sets the state to, your 
> pm_autosuspend() will change it to one of RPM_UNKNOWN or RPM_SUSPENDED.  
> Neither might be right.

The idea is that if ->autosuspend() or ->autoresume() returns an error code,
this is a situation the PM core cannot recover from by itself, so it shouldn't
pretend it knows what's happened.  Instead, it marks the device as "I don't
know if it is safe to touch this" and won't handle it until the device driver
or bus type clears the status.

> > The name of this constant may be confusing, but I didn't have any better ideas.
> 
> It's not clear what RPM_ACTIVE, RPM_IDLE, and RPM_SUSPENDED are 
> supposed to mean; this should be documented in the code.  Also, why 
> isn't there RPM_RESUMING?

Yes, there should be.  In fact it's in the current version of the patch, which
is appended.  Also, there's a comment explaining the meaning of the RPM_*
constants in pm.h .

> By the way, a legitimate reason for aborting an autosuspend is if the
> device's driver requires remote wakeup to be enabled during suspend but
> the user has disabled it.

Do you mean the user has disabled the remote wakeup?

> > > The scheme doesn't include any mechanism for communicating runtime
> > > power information up the device tree.  When a device is autosuspended,
> > > its parent's driver should be told so that the driver can consider
> > > autosuspending the parent.
> > 
> > I thought the bus type's ->autosuspend() callback could take care of this.
> 
> Shouldn't this happen after the device's state has changed to 
> RPM_SUSPENDED?  That's not until after the callback returns.

OK, I tried to address the issue of parent suspend/resume in the new
version of the patch below (I'm not sure if I did the nesting of spinlocks in
pm_request_resume() correctly).

> > > There should be a sysfs interface (like the one in USB) to allow
> > > userspace to prevent a device from being autosuspended -- and perhaps
> > > also to force it to be suspended.
> > 
> > To prevent a device from being suspended - yes.  To force it to stay suspended
> > - I'm not sure.
> 
> I'm not sure either.  Oliver Neukum requested it originally and it has
> been useful for debugging, but I haven't seen many places where it
> would come in useful in practice.

The problem with it is that the user space may not know if it is safe to keep
a device suspended and if it is not, the kernel will have to ignore the setting
anyway, so I'm not sure what's the point (except for debugging).

> > > What about devices that have more than two runtime power states?  For
> > > example, you can't squeeze PCI's {D0,D1,D2,D3hot} range into {running,
> > > suspended}.
> > 
> > That has to be bus type-specific.
> > 
> > In the case of PCI all of the low power states (D1-D3) are in fact substates of
> > "suspended", because we generally need to quiesce the device before putting
> > it into any of these states.
> > 
> > I'm not sure if we can introduce more "levels of suspension", so to speak, at
> > the core level, but in any case we can easily distinguish between "device
> > quiesced and in a low power state" and "device fully active".
> > 
> > So, in this picture the device is "suspended" from the core's point of view
> > once it's bus type's ->autosuspend() callback has been successfully executed. 
> 
> This too should be documented in the code.  Or in a Documentation file.

OK

I tried to address your comments and the Oliver's comments too in the new
version of the patch below.  Please have a look and tell me what you think.

Best,
Rafael

---
 drivers/base/power/Makefile  |    1 
 drivers/base/power/main.c    |    2 
 drivers/base/power/runtime.c |  318 +++++++++++++++++++++++++++++++++++++++++++
 include/linux/pm.h           |   76 ++++++++++
 include/linux/pm_runtime.h   |   50 ++++++
 kernel/power/Kconfig         |   14 +
 kernel/power/main.c          |   17 ++
 7 files changed, 476 insertions(+), 2 deletions(-)

Index: linux-2.6/kernel/power/Kconfig
===================================================================
--- linux-2.6.orig/kernel/power/Kconfig
+++ linux-2.6/kernel/power/Kconfig
@@ -208,3 +208,17 @@ config APM_EMULATION
 	  random kernel OOPSes or reboots that don't seem to be related to
 	  anything, try disabling/enabling this option (or disabling/enabling
 	  APM in your BIOS).
+
+config PM_RUNTIME
+	bool "Run-time PM core functionality"
+	depends on PM
+	---help---
+	  Enable functionality allowing I/O devices to be put into energy-saving
+	  (low power) states at run time (or autosuspended) after a specified
+	  period of inactivity and woken up in response to a hardware-generated
+	  wake-up event or a driver's request.
+
+	  Hardware support is generally required for this functionality to work
+	  and the bus type drivers of the buses the devices are on are
+	  responsibile for the actual handling of the autosuspend requests and
+	  wake-up events.
Index: linux-2.6/kernel/power/main.c
===================================================================
--- linux-2.6.orig/kernel/power/main.c
+++ linux-2.6/kernel/power/main.c
@@ -11,6 +11,7 @@
 #include <linux/kobject.h>
 #include <linux/string.h>
 #include <linux/resume-trace.h>
+#include <linux/workqueue.h>
 
 #include "power.h"
 
@@ -217,8 +218,24 @@ static struct attribute_group attr_group
 	.attrs = g,
 };
 
+#ifdef CONFIG_PM_RUNTIME
+struct workqueue_struct *pm_wq;
+
+static int __init pm_start_workqueue(void)
+{
+	pm_wq = create_freezeable_workqueue("pm");
+
+	return pm_wq ? 0 : -ENOMEM;
+}
+#else
+static inline int pm_start_workqueue(void) { return 0; }
+#endif
+
 static int __init pm_init(void)
 {
+	int error = pm_start_workqueue();
+	if (error)
+		return error;
 	power_kobj = kobject_create_and_add("power", NULL);
 	if (!power_kobj)
 		return -ENOMEM;
Index: linux-2.6/include/linux/pm.h
===================================================================
--- linux-2.6.orig/include/linux/pm.h
+++ linux-2.6/include/linux/pm.h
@@ -22,6 +22,9 @@
 #define _LINUX_PM_H
 
 #include <linux/list.h>
+#include <linux/workqueue.h>
+#include <linux/spinlock.h>
+#include <linux/completion.h>
 
 /*
  * Callbacks for platform drivers to implement.
@@ -165,6 +168,15 @@ typedef struct pm_message {
  * It is allowed to unregister devices while the above callbacks are being
  * executed.  However, it is not allowed to unregister a device from within any
  * of its own callbacks.
+ *
+ * There also are two callbacks related to run-time power management of devices:
+ *
+ * @autosuspend: Save the device registers and put it into an energy-saving (low
+ *	power) state at run-time, enable wake-up events as appropriate.
+ *
+ * @autoresume: Put the device into the full power state and restore its
+ *	registers (if applicable) at run time, in response to a wake-up event
+ *	generated by hardware or at a request of software.
  */
 
 struct dev_pm_ops {
@@ -182,6 +194,10 @@ struct dev_pm_ops {
 	int (*thaw_noirq)(struct device *dev);
 	int (*poweroff_noirq)(struct device *dev);
 	int (*restore_noirq)(struct device *dev);
+#ifdef CONFIG_PM_RUNTIME
+	int (*autosuspend)(struct device *dev);
+	int (*autoresume)(struct device *dev);
+#endif
 };
 
 /**
@@ -315,14 +331,70 @@ enum dpm_state {
 	DPM_OFF_IRQ,
 };
 
+/**
+ * Device run-time power management state.
+ *
+ * These state labels are used internally by the PM core to indicate the current
+ * status of a device with respect to the PM core operations.  They do not
+ * reflect the actual power state of the device or its status as seen by the
+ * driver.
+ *
+ * RPM_ACTIVE		Device is fully operational, no run-time PM requests are
+ *			pending for it.
+ *
+ * RPM_IDLE		It has been requested that the device be suspended.
+ *			Suspend request has been put into the run-time PM
+ *			workqueue and it's pending execution.
+ *
+ * RPM_SUSPENDING	Device bus type's ->autosuspend() callback is being
+ *			executed.
+ *
+ * RPM_SUSPENDED	Device bus type's ->autosuspend() callback has completed
+ *			successfully.  The device is regarded as suspended.
+ *
+ * RPM_WAKE		It has been requested that the device be woken up.
+ *			Resume request has been put into the run-time PM
+ *			workqueue and it's pending execution.
+ *
+ * RPM_RESUMING		Device bus type's ->autoresume() callback is being
+ *			executed.
+ *
+ * RPM_ERROR		Represents a condition from which the PM core cannot
+ *			recover by itself.  If the device's run-time PM status
+ *			field has this value, all of the run-time PM operations
+ *			carried out for the device by the core will fail, until
+ *			the status field is changed to either RPM_ACTIVE or
+ *			RPM_SUSPENDED (it is not valid to use the other values
+ *			in such a situation) by the device's driver or bus type.
+ *			This happens when the device bus type's ->autosuspend()
+ *			or ->autoresume() callback returns error code.
+ */
+enum rpm_state {
+	RPM_ERROR = -1,
+	RPM_ACTIVE,
+	RPM_IDLE,
+	RPM_SUSPENDING,
+	RPM_SUSPENDED,
+	RPM_WAKE,
+	RPM_RESUMING,
+};
+
 struct dev_pm_info {
 	pm_message_t		power_state;
-	unsigned		can_wakeup:1;
-	unsigned		should_wakeup:1;
+	unsigned int		can_wakeup:1;
+	unsigned int		should_wakeup:1;
 	enum dpm_state		status;		/* Owned by the PM core */
 #ifdef	CONFIG_PM_SLEEP
 	struct list_head	entry;
 #endif
+#ifdef	CONFIG_PM_RUNTIME
+	struct delayed_work	suspend_work;
+	struct completion	suspend_done;
+	unsigned int		suspend_aborted:1;
+	struct work_struct	resume_work;
+	enum rpm_state		runtime_status;
+	spinlock_t		lock;
+#endif
 };
 
 /*
Index: linux-2.6/drivers/base/power/Makefile
===================================================================
--- linux-2.6.orig/drivers/base/power/Makefile
+++ linux-2.6/drivers/base/power/Makefile
@@ -1,5 +1,6 @@
 obj-$(CONFIG_PM)	+= sysfs.o
 obj-$(CONFIG_PM_SLEEP)	+= main.o
+obj-$(CONFIG_PM_RUNTIME)	+= runtime.o
 obj-$(CONFIG_PM_TRACE_RTC)	+= trace.o
 
 ccflags-$(CONFIG_DEBUG_DRIVER) := -DDEBUG
Index: linux-2.6/drivers/base/power/runtime.c
===================================================================
--- /dev/null
+++ linux-2.6/drivers/base/power/runtime.c
@@ -0,0 +1,318 @@
+/*
+ * drivers/base/power/runtime.c - Helper functions for device run-time PM
+ *
+ * Copyright (c) 2009 Rafael J. Wysocki <rjw@sisk.pl>, Novell Inc.
+ *
+ * This file is released under the GPLv2.
+ */
+
+#include <linux/pm_runtime.h>
+
+/**
+ * pm_runtime_reset - Clear all of the device run-time PM flags.
+ * @dev: Device object to clear the flags for.
+ */
+static void pm_runtime_reset(struct device *dev)
+{
+	dev->power.suspend_aborted = false;
+	dev->power.runtime_status = RPM_ACTIVE;
+}
+
+/**
+ * pm_device_suspended - Check if given device has been suspended at run time.
+ * @dev: Device to check.
+ * @data: Ignored.
+ *
+ * Returns 0 if the device has been suspended or -EBUSY otherwise.
+ */
+static int pm_device_suspended(struct device *dev, void *data)
+{
+	int ret;
+
+	spin_lock(&dev->power.lock);
+
+	ret = dev->power.runtime_status == RPM_SUSPENDED ? 0 : -EBUSY;
+
+	spin_unlock(&dev->power.lock);
+
+	return ret;
+}
+
+/**
+ * pm_check_children - Check if all children of a device have been suspended.
+ * @dev: Device to check.
+ *
+ * Returns 0 if all children of the device have been suspended or -EBUSY
+ * otherwise.
+ */
+static int pm_check_children(struct device *dev)
+{
+	return device_for_each_child(dev, NULL, pm_device_suspended);
+}
+
+/**
+ * pm_autosuspend - Run autosuspend callback of given device object's bus type.
+ * @work: Work structure used for scheduling the execution of this function.
+ *
+ * Use @work to get the device object the suspend has been scheduled for,
+ * check if the suspend request hasn't been cancelled and run the
+ * ->autosuspend() callback from the device's bus type driver.  Update the
+ * run-time PM flags in the device object to reflect the current status of the
+ * device.
+ */
+static void pm_autosuspend(struct work_struct *work)
+{
+	struct delayed_work *dw = to_delayed_work(work);
+	struct device *dev = suspend_work_to_device(dw);
+	int error = 0;
+
+	spin_lock(&dev->power.lock);
+
+	if (dev->power.suspend_aborted) {
+		dev->power.runtime_status = RPM_ACTIVE;
+		goto out;
+	} else if (dev->power.runtime_status != RPM_IDLE) {
+		goto out;
+	} else if (pm_check_children(dev)) {
+		/*
+		 * We can only suspend the device if all of its children have
+		 * been suspended.
+		 */
+		goto out;
+	}
+
+	dev->power.runtime_status = RPM_SUSPENDING;
+	init_completion(&dev->power.suspend_done);
+
+	spin_unlock(&dev->power.lock);
+
+	if (dev && dev->bus && dev->bus->pm && dev->bus->pm->autosuspend)
+		error = dev->bus->pm->autosuspend(dev);
+
+	spin_lock(&dev->power.lock);
+
+	dev->power.runtime_status = error ? RPM_ERROR : RPM_SUSPENDED;
+	complete(&dev->power.suspend_done);
+
+ out:
+	spin_unlock(&dev->power.lock);
+}
+
+/**
+ * pm_request_suspend - Schedule run-time suspend of given device.
+ * @dev: Device to suspend.
+ * @delay: Time to wait before attempting to suspend the device.
+ */
+void pm_request_suspend(struct device *dev, unsigned long delay)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&dev->power.lock, flags);
+
+	if (dev->power.runtime_status != RPM_ACTIVE)
+		goto out;
+
+	dev->power.runtime_status = RPM_IDLE;
+	dev->power.suspend_aborted = false;
+	queue_delayed_work(pm_wq, &dev->power.suspend_work, delay);
+
+ out:
+	spin_unlock_irqrestore(&dev->power.lock, flags);
+}
+
+/**
+ * pm_cancel_suspend - Cancel a pending suspend request for given device.
+ * @dev: Device to cancel the suspend request for.
+ *
+ * Should be called under pm_lock_device() and only if we are sure that the
+ * ->autosuspend() callback hasn't started to yet.
+ */
+static void pm_cancel_suspend(struct device *dev)
+{
+	dev->power.suspend_aborted = true;
+	cancel_delayed_work(&dev->power.suspend_work);
+	dev->power.runtime_status = RPM_ACTIVE;
+}
+
+/**
+ * pm_autoresume - Run autoresume callback of given device object's bus type.
+ * @work: Work structure used for scheduling the execution of this function.
+ *
+ * Use @work to get the device object the resume has been scheduled for,
+ * check if the device is really suspended and run the ->autoresume() callback
+ * from the device's bus type driver.  Update the run-time PM flags in the
+ * device object to reflect the current status of the device.
+ */
+static void pm_autoresume(struct work_struct *work)
+{
+	struct device *dev = resume_work_to_device(work);
+	int error = 0;
+
+	spin_lock(&dev->power.lock);
+
+	if (dev->power.runtime_status != RPM_WAKE)
+		goto out;
+
+	dev->power.runtime_status = RPM_RESUMING;
+
+	spin_unlock(&dev->power.lock);
+
+	if (dev->bus && dev->bus->pm && dev->bus->pm->autoresume)
+		error = dev->bus->pm->autoresume(dev);
+
+	spin_lock(&dev->power.lock);
+
+	dev->power.runtime_status = error ? RPM_ERROR : RPM_ACTIVE;
+
+ out:
+	spin_unlock(&dev->power.lock);
+}
+
+/**
+ * pm_request_resume - Schedule run-time resume of given device.
+ * @dev: Device to resume.
+ */
+void pm_request_resume(struct device *dev)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&dev->parent->power.lock, flags);
+	spin_lock(&dev->power.lock);
+
+	if (dev->power.runtime_status == RPM_IDLE) {
+		/* ->autosuspend() hasn't started yet, no need to resume. */
+		pm_cancel_suspend(dev);
+		goto out;
+	} else if (dev->power.runtime_status != RPM_SUSPENDING
+	    && dev->power.runtime_status != RPM_SUSPENDED) {
+		goto out;
+	}
+
+	dev->power.runtime_status = RPM_WAKE;
+	queue_work(pm_wq, &dev->power.resume_work);
+
+ out:
+	spin_unlock(&dev->power.lock);
+	spin_unlock_irqrestore(&dev->parent->power.lock, flags);
+}
+
+/**
+ * pm_resume_sync - Resume given device waiting for the operation to complete.
+ * @dev: Device to resume.
+ *
+ * Resume the device synchronously, waiting for the operation to complete.  If
+ * autosuspend is in progress while this function is being run, wait for it to
+ * finish before resuming the device.  If the autosuspend is scheduled, but it
+ * hasn't started yet, cancel it and we're done.
+ */
+int pm_resume_sync(struct device *dev)
+{
+	int error = 0;
+
+	spin_lock(&dev->power.lock);
+
+	if (dev->power.runtime_status == RPM_ACTIVE) {
+		goto out;
+	} if (dev->power.runtime_status == RPM_IDLE) {
+		/* ->autosuspend() hasn't started yet, no need to resume. */
+		pm_cancel_suspend(dev);
+		goto out;
+	}
+
+	if (dev->power.runtime_status == RPM_SUSPENDING) {
+		spin_unlock(&dev->power.lock);
+
+		/*
+		 * The ->autosuspend() callback is being executed right now,
+		 * wait for it to complete.
+		 */
+		wait_for_completion(&dev->power.suspend_done);
+	} else if (dev->power.runtime_status == RPM_SUSPENDED) {
+		spin_unlock(&dev->power.lock);
+
+		/* The device's parent may also be suspended.  Resume it. */
+		error = pm_resume_sync(dev->parent);
+		if (error)
+			return error;
+	} else {
+		spin_unlock(&dev->power.lock);
+	}
+
+	spin_lock(&dev->parent->power.lock);
+	spin_lock(&dev->power.lock);
+
+	if (dev->power.runtime_status == RPM_RESUMING)
+		/* There's another resume running in parallel with us. */
+		error = -EAGAIN;
+	else if (dev->power.runtime_status != RPM_SUSPENDED)
+		error = -EINVAL;
+	if (error) {
+		spin_unlock(&dev->parent->power.lock);
+		goto out;
+	}
+
+	dev->power.runtime_status = RPM_RESUMING;
+
+	spin_unlock(&dev->power.lock);
+	spin_unlock(&dev->parent->power.lock);
+
+	if (dev->bus && dev->bus->pm && dev->bus->pm->autoresume)
+		error = dev->bus->pm->autoresume(dev);
+
+	spin_lock(&dev->power.lock);
+
+	dev->power.runtime_status = error ? RPM_ERROR : RPM_ACTIVE;
+
+ out:
+	spin_unlock(&dev->power.lock);
+
+	return error;
+}
+
+/**
+ * pm_cancel_autosuspend - Cancel a pending autosuspend request for given device
+ * @dev: Device to handle.
+ *
+ * This routine is only supposed to be called when the run-time PM workqueue is
+ * frozen (i.e. during system-wide suspend or hibernation) when it is guaranteed
+ * that no work items are being executed.
+ */
+void pm_cancel_autosuspend(struct device *dev)
+{
+	spin_lock(&dev->power.lock);
+
+	cancel_delayed_work(&dev->power.suspend_work);
+	pm_runtime_reset(dev);
+
+	spin_unlock(&dev->power.lock);
+}
+
+/**
+ * pm_cancel_autoresume - Cancel a pending autoresume request for given device
+ * @dev: Device to handle.
+ *
+ * This routine is only supposed to be called when the run-time PM workqueue is
+ * frozen (i.e. during system-wide suspend or hibernation) when it is guaranteed
+ * that no work items are being executed.
+ */
+void pm_cancel_autoresume(struct device *dev)
+{
+	spin_lock(&dev->power.lock);
+
+	work_clear_pending(&dev->power.resume_work);
+	pm_runtime_reset(dev);
+
+	spin_unlock(&dev->power.lock);
+}
+
+/**
+ * pm_runtime_init - Initialize run-time PM fields in given device object.
+ * @dev: Device object to handle.
+ */
+void pm_runtime_init(struct device *dev)
+{
+	pm_runtime_reset(dev);
+	spin_lock_init(&dev->power.lock);
+	INIT_DELAYED_WORK(&dev->power.suspend_work, pm_autosuspend);
+	INIT_WORK(&dev->power.resume_work, pm_autoresume);
+}
Index: linux-2.6/include/linux/pm_runtime.h
===================================================================
--- /dev/null
+++ linux-2.6/include/linux/pm_runtime.h
@@ -0,0 +1,50 @@
+/*
+ * pm_runtime.h - Device run-time power management helper functions.
+ *
+ * Copyright (C) 2009 Rafael J. Wysocki <rjw@sisk.pl>
+ *
+ * This file is released under the GPLv2.
+ */
+
+#ifndef _LINUX_PM_RUNTIME_H
+#define _LINUX_PM_RUNTIME_H
+
+#include <linux/device.h>
+#include <linux/pm.h>
+
+#ifdef CONFIG_PM_RUNTIME
+extern struct workqueue_struct *pm_wq;
+
+extern void pm_runtime_init(struct device *dev);
+extern void pm_request_suspend(struct device *dev, unsigned long delay);
+extern void pm_request_resume(struct device *dev);
+extern int pm_resume_sync(struct device *dev);
+extern void pm_cancel_autosuspend(struct device *dev);
+extern void pm_cancel_autoresume(struct device *dev);
+
+static inline struct device *suspend_work_to_device(struct delayed_work *work)
+{
+	struct dev_pm_info *dpi;
+
+	dpi = container_of(work, struct dev_pm_info, suspend_work);
+	return container_of(dpi, struct device, power);
+}
+
+static inline struct device *resume_work_to_device(struct work_struct *work)
+{
+	struct dev_pm_info *dpi;
+
+	dpi = container_of(work, struct dev_pm_info, resume_work);
+	return container_of(dpi, struct device, power);
+}
+
+#else /* !CONFIG_PM_RUNTIME */
+static inline void pm_runtime_init(struct device *dev) {}
+static inline void pm_request_suspend(struct device *dev, unsigned long delay);
+static inline void pm_request_resume(struct device *dev) {}
+static inline int pm_resume_sync(struct device *dev) { return -ENOSYS; }
+static inline void pm_cancel_autosuspend(struct device *dev) {}
+static inline void pm_cancel_autoresume(struct device *dev) {}
+#endif /* !CONFIG_PM_RUNTIME */
+
+#endif
Index: linux-2.6/drivers/base/power/main.c
===================================================================
--- linux-2.6.orig/drivers/base/power/main.c
+++ linux-2.6/drivers/base/power/main.c
@@ -21,6 +21,7 @@
 #include <linux/kallsyms.h>
 #include <linux/mutex.h>
 #include <linux/pm.h>
+#include <linux/pm_runtime.h>
 #include <linux/resume-trace.h>
 #include <linux/rwsem.h>
 #include <linux/interrupt.h>
@@ -88,6 +89,7 @@ void device_pm_add(struct device *dev)
 	}
 
 	list_add_tail(&dev->power.entry, &dpm_list);
+	pm_runtime_init(dev);
 	mutex_unlock(&dpm_list_mtx);
 }
 

Re: Run-time PM idea (was: Re: [RFC][PATCH 0/2] PM: Rearrange core suspend code)

[Alan Stern]

On Wed, 10 Jun 2009, Rafael J. Wysocki wrote:

> > By the way, a legitimate reason for aborting an autosuspend is if the
> > device's driver requires remote wakeup to be enabled during suspend but
> > the user has disabled it.
> 
> Do you mean the user has disabled the remote wakeup?

Yes, by writing to the power/wakeup attribute.


> > > > There should be a sysfs interface (like the one in USB) to allow
> > > > userspace to prevent a device from being autosuspended -- and perhaps
> > > > also to force it to be suspended.
> > > 
> > > To prevent a device from being suspended - yes.  To force it to stay suspended
> > > - I'm not sure.
> > 
> > I'm not sure either.  Oliver Neukum requested it originally and it has
> > been useful for debugging, but I haven't seen many places where it
> > would come in useful in practice.

I did think of one use for this feature.  It's unique to USB,
however...

In Windows, you're not supposed to unplug a hot-unpluggable device
without first telling the OS -- there's a "Safely Remove Hardware"  
applet.  When you tell the applet you want to remove a USB device, the
system disables the device's port and then says it's okay to unplug the
device.  Now Linux doesn't have any user API for disabling USB ports,
but suspending a port has the same effect (the device can't distinguish
a disable from a suspend).

It turns out that some devices (MP3 players, for instance) have
incorporated this into their design.  They display a "Safe to unplug"  
message when their port is disabled or suspended.  People like to see
this message -- it makes them feel good about unplugging the device --
and the only way to get it under Linux is by forcing the device to be
suspended.  :-)

> The problem with it is that the user space may not know if it is safe to keep
> a device suspended and if it is not, the kernel will have to ignore the setting
> anyway, so I'm not sure what's the point (except for debugging).

This falls into the category of "The user knows better".  If the user
specifically tells the kernel to suspend a device (rather than just
letting it autosuspend), and this causes a problem, then it's the
user's own fault.

After all, who's really the master?  Us or the kernel?

Alan Stern

Re: Run-time PM idea (was: Re: [RFC][PATCH 0/2] PM: Rearrange core suspend code)

[Rafael J. Wysocki]

On Wednesday 10 June 2009, Alan Stern wrote:
> On Wed, 10 Jun 2009, Rafael J. Wysocki wrote:
> 
> > > By the way, a legitimate reason for aborting an autosuspend is if the
> > > device's driver requires remote wakeup to be enabled during suspend but
> > > the user has disabled it.
> > 
> > Do you mean the user has disabled the remote wakeup?
> 
> Yes, by writing to the power/wakeup attribute.
> 
> 
> > > > > There should be a sysfs interface (like the one in USB) to allow
> > > > > userspace to prevent a device from being autosuspended -- and perhaps
> > > > > also to force it to be suspended.
> > > > 
> > > > To prevent a device from being suspended - yes.  To force it to stay suspended
> > > > - I'm not sure.
> > > 
> > > I'm not sure either.  Oliver Neukum requested it originally and it has
> > > been useful for debugging, but I haven't seen many places where it
> > > would come in useful in practice.
> 
> I did think of one use for this feature.  It's unique to USB,
> however...
> 
> In Windows, you're not supposed to unplug a hot-unpluggable device
> without first telling the OS -- there's a "Safely Remove Hardware"  
> applet.  When you tell the applet you want to remove a USB device, the
> system disables the device's port and then says it's okay to unplug the
> device.  Now Linux doesn't have any user API for disabling USB ports,
> but suspending a port has the same effect (the device can't distinguish
> a disable from a suspend).
> 
> It turns out that some devices (MP3 players, for instance) have
> incorporated this into their design.  They display a "Safe to unplug"  
> message when their port is disabled or suspended.  People like to see
> this message -- it makes them feel good about unplugging the device --
> and the only way to get it under Linux is by forcing the device to be
> suspended.  :-)

Well, I'd very much prefer to have a separate mechanism for that.

> > The problem with it is that the user space may not know if it is safe to keep
> > a device suspended and if it is not, the kernel will have to ignore the setting
> > anyway, so I'm not sure what's the point (except for debugging).
> 
> This falls into the category of "The user knows better".  If the user
> specifically tells the kernel to suspend a device (rather than just
> letting it autosuspend), and this causes a problem, then it's the
> user's own fault.
> 
> After all, who's really the master?  Us or the kernel?

Oh, that depends on who the user is.  If I'm the user, I'm the master, but in
case of a typical Windows user I'm afraid the kernel has to know better. ;-)

Best,
Rafael