[RFC][PATCH 0/3] PM: Asynchronous suspend and resume

[RFC][PATCH 0/3] PM: Asynchronous suspend and resume

[Rafael J. Wysocki]

Hi,

The following patches introduce a mechanism allowing us to execute device
drivers' suspend and resume callbacks asynchronously during system sleep
transitions, such as suspend to RAM.  The idea is explained in the [1/1] patch
message.

Comments welcome.

Thanks,
Rafael

[RFC][PATCH 1/3] PM: Asynchronous resume of devices

[Rafael J. Wysocki]

Theoretically, the total time of system sleep transitions (suspend
to RAM, hibernation) can be reduced by running suspend and resume
callbacks of device drivers in parallel with each other.  However,
there are dependencies between devices such that, for example, we may
not be allowed to put one device into a low power state before
anohter one has been suspended (e.g. we cannot suspend a bridge
before suspending all devices behind it).  In particular, we're not
allowed to suspend the parent of a device before suspending the
device itself.  Analogously, we're not allowed to resume a device
before resuming its parent.

Thus, to make it possible to execute suspend and resume callbacks
provided by device drivers in parallel with each other, we need to
provide a synchronization mechanism preventing the dependencies
between devices from being violated.

The patch below introduces a mechanism allowing some devices to be
resumed asynchronously, using completions with the following rules:
(1) There is a completion, dev->power.comp, for each device object.
(2) All of these completions are reset before suspend as well as
    each resume stage (dpm_resume_noirq(), dpm_resume()).
(3) If dev->power.async_suspend is set for dev or for its parent, the
    PM core waits for the parent's completion before attempting to
    run the resume callbacks, appropriate for this particular stage
    of resume, for dev.
(4) dev->power.comp is completed for each device after running its
    resume callbacks.

With this mechanism in place, the drivers wanting their resume
callbacks to be executed asynchronously and knowing that their
devices are not dependent on any other devices indirectly (i.e. in
any way that's not reflected by the structure of the device tree)
can set dev->power.async_suspend for them, with the help of
device_enable_async_suspend().

Since the only dependencies between devices known to the PM core
are the ones reflected by the structure of the device tree, we don't
seem to be able to do any better than this at the core level.

---
 drivers/base/power/main.c |  115 ++++++++++++++++++++++++++++++++++++++++------
 include/linux/device.h    |    6 ++
 include/linux/pm.h        |    4 +
 3 files changed, 111 insertions(+), 14 deletions(-)

Index: linux-2.6/include/linux/pm.h
===================================================================
--- linux-2.6.orig/include/linux/pm.h
+++ linux-2.6/include/linux/pm.h
@@ -26,6 +26,7 @@
 #include <linux/spinlock.h>
 #include <linux/wait.h>
 #include <linux/timer.h>
+#include <linux/completion.h>
 
 /*
  * Callbacks for platform drivers to implement.
@@ -411,9 +412,12 @@ struct dev_pm_info {
 	pm_message_t		power_state;
 	unsigned int		can_wakeup:1;
 	unsigned int		should_wakeup:1;
+	unsigned		async_suspend:1;
 	enum dpm_state		status;		/* Owned by the PM core */
 #ifdef CONFIG_PM_SLEEP
 	struct list_head	entry;
+	struct completion	comp;
+	pm_message_t		async_state;
 #endif
 #ifdef CONFIG_PM_RUNTIME
 	struct timer_list	suspend_timer;
Index: linux-2.6/include/linux/device.h
===================================================================
--- linux-2.6.orig/include/linux/device.h
+++ linux-2.6/include/linux/device.h
@@ -472,6 +472,12 @@ static inline int device_is_registered(s
 	return dev->kobj.state_in_sysfs;
 }
 
+static inline void device_enable_async_suspend(struct device *dev, bool enable)
+{
+	if (dev->power.status == DPM_ON)
+		dev->power.async_suspend = enable;
+}
+
 void driver_init(void);
 
 /*
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
@@ -25,6 +25,8 @@
 #include <linux/resume-trace.h>
 #include <linux/rwsem.h>
 #include <linux/interrupt.h>
+#include <linux/async.h>
+#include <linux/completion.h>
 
 #include "../base.h"
 #include "power.h"
@@ -56,6 +58,7 @@ static bool transition_started;
 void device_pm_init(struct device *dev)
 {
 	dev->power.status = DPM_ON;
+	init_completion(&dev->power.comp);
 	pm_runtime_init(dev);
 }
 
@@ -163,6 +166,34 @@ void device_pm_move_last(struct device *
 	list_move_tail(&dev->power.entry, &dpm_list);
 }
 
+static void dpm_synchronize_noirq(void)
+{
+	struct device *dev;
+
+	async_synchronize_full();
+
+	list_for_each_entry(dev, &dpm_list, power.entry)
+		INIT_COMPLETION(dev->power.comp);
+}
+
+static void dpm_synchronize(void)
+{
+	struct device *dev;
+
+	async_synchronize_full();
+
+	mutex_lock(&dpm_list_mtx);
+	list_for_each_entry(dev, &dpm_list, power.entry)
+		INIT_COMPLETION(dev->power.comp);
+	mutex_unlock(&dpm_list_mtx);
+}
+
+static void device_pm_wait(struct device *dev, struct device *master)
+{
+	if (dev->power.async_suspend || (master && master->power.async_suspend))
+		wait_for_completion(&master->power.comp);
+}
+
 /**
  *	pm_op - execute the PM operation appropiate for given PM event
  *	@dev:	Device.
@@ -329,31 +360,57 @@ static void pm_dev_err(struct device *de
 /*------------------------- Resume routines -------------------------*/
 
 /**
- *	device_resume_noirq - Power on one device (early resume).
- *	@dev:	Device.
- *	@state: PM transition of the system being carried out.
+ * __device_resume_noirq - Execute an "early resume" callback for given device.
+ * @dev: Device to resume.
+ * @state: PM transition of the system being carried out.
  *
- *	Must be called with interrupts disabled.
+ * The driver of the device won't receive interrupts while this function is
+ * being executed.
  */
-static int device_resume_noirq(struct device *dev, pm_message_t state)
+static int __device_resume_noirq(struct device *dev, pm_message_t state)
 {
 	int error = 0;
 
 	TRACE_DEVICE(dev);
 	TRACE_RESUME(0);
 
-	if (!dev->bus)
-		goto End;
+	device_pm_wait(dev, dev->parent);
 
-	if (dev->bus->pm) {
+	if (dev->bus && dev->bus->pm) {
 		pm_dev_dbg(dev, state, "EARLY ");
 		error = pm_noirq_op(dev, dev->bus->pm, state);
 	}
- End:
+
+	complete_all(&dev->power.comp);
+
 	TRACE_RESUME(error);
 	return error;
 }
 
+static void async_resume_noirq(void *data, async_cookie_t cookie)
+{
+	struct device *dev = (struct device *)data;
+	int error;
+
+	pm_dev_dbg(dev, dev->power.async_state, "async EARLY ");
+	error = __device_resume_noirq(dev, dev->power.async_state);
+	if (error)
+		pm_dev_err(dev, dev->power.async_state, " async EARLY", error);
+	put_device(dev);
+}
+
+static int device_resume_noirq(struct device *dev, pm_message_t state)
+{
+	if (dev->power.async_suspend) {
+		get_device(dev);
+		dev->power.async_state = state;
+		async_schedule(async_resume_noirq, dev);
+		return 0;
+	}
+
+	return __device_resume_noirq(dev, state);
+}
+
 /**
  *	dpm_resume_noirq - Power on all regular (non-sysdev) devices.
  *	@state: PM transition of the system being carried out.
@@ -378,23 +435,25 @@ void dpm_resume_noirq(pm_message_t state
 			if (error)
 				pm_dev_err(dev, state, " early", error);
 		}
+	dpm_synchronize_noirq();
 	mutex_unlock(&dpm_list_mtx);
 	resume_device_irqs();
 }
 EXPORT_SYMBOL_GPL(dpm_resume_noirq);
 
 /**
- *	device_resume - Restore state for one device.
- *	@dev:	Device.
- *	@state: PM transition of the system being carried out.
+ * __device_resume - Execute "resume" callbacks for given device.
+ * @dev: Device to resume.
+ * @state: PM transition of the system being carried out.
  */
-static int device_resume(struct device *dev, pm_message_t state)
+static int __device_resume(struct device *dev, pm_message_t state)
 {
 	int error = 0;
 
 	TRACE_DEVICE(dev);
 	TRACE_RESUME(0);
 
+	device_pm_wait(dev, dev->parent);
 	down(&dev->sem);
 
 	if (dev->bus) {
@@ -429,11 +488,36 @@ static int device_resume(struct device *
 	}
  End:
 	up(&dev->sem);
+	complete_all(&dev->power.comp);
 
 	TRACE_RESUME(error);
 	return error;
 }
 
+static void async_resume(void *data, async_cookie_t cookie)
+{
+	struct device *dev = (struct device *)data;
+	int error;
+
+	pm_dev_dbg(dev, dev->power.async_state, "async ");
+	error = __device_resume(dev, dev->power.async_state);
+	if (error)
+		pm_dev_err(dev, dev->power.async_state, " async", error);
+	put_device(dev);
+}
+
+static int device_resume(struct device *dev, pm_message_t state)
+{
+	if (dev->power.async_suspend) {
+		get_device(dev);
+		dev->power.async_state = state;
+		async_schedule(async_resume, dev);
+		return 0;
+	}
+
+	return __device_resume(dev, state);
+}
+
 /**
  *	dpm_resume - Resume every device.
  *	@state: PM transition of the system being carried out.
@@ -473,6 +557,7 @@ static void dpm_resume(pm_message_t stat
 	}
 	list_splice(&list, &dpm_list);
 	mutex_unlock(&dpm_list_mtx);
+	dpm_synchronize();
 }
 
 /**
@@ -794,8 +879,10 @@ static int dpm_prepare(pm_message_t stat
 			break;
 		}
 		dev->power.status = DPM_SUSPENDING;
-		if (!list_empty(&dev->power.entry))
+		if (!list_empty(&dev->power.entry)) {
 			list_move_tail(&dev->power.entry, &list);
+			INIT_COMPLETION(dev->power.comp);
+		}
 		put_device(dev);
 	}
 	list_splice(&list, &dpm_list);

Re: [RFC][PATCH 1/3] PM: Asynchronous resume of devices

[Pavel Machek]

> The patch below introduces a mechanism allowing some devices to be
> resumed asynchronously, using completions with the following rules:
> (1) There is a completion, dev->power.comp, for each device object.
> (2) All of these completions are reset before suspend as well as
>     each resume stage (dpm_resume_noirq(), dpm_resume()).
> (3) If dev->power.async_suspend is set for dev or for its parent, the
>     PM core waits for the parent's completion before attempting to
>     run the resume callbacks, appropriate for this particular stage
>     of resume, for dev.

at least this needs to go in as a comment.

> (4) dev->power.comp is completed for each device after running its
> @@ -411,9 +412,12 @@ struct dev_pm_info {
>  	pm_message_t		power_state;
>  	unsigned int		can_wakeup:1;
>  	unsigned int		should_wakeup:1;
> +	unsigned		async_suspend:1;
>  	enum dpm_state		status;		/* Owned by the PM core */

unsigned int? Or bool?

Should it go under config_pm_sleep?

>  #ifdef CONFIG_PM_SLEEP
>  	struct list_head	entry;
> +	struct completion	comp;
> +	pm_message_t		async_state;
>  #endif
>  }
>  
> +static inline void device_enable_async_suspend(struct device *dev, bool enable)
> +{
> +	if (dev->power.status == DPM_ON)
> +		dev->power.async_suspend = enable;
> +}
> +
> @@ -163,6 +166,34 @@ void device_pm_move_last(struct device *
>  	list_move_tail(&dev->power.entry, &dpm_list);
>  }
>  
> +static void dpm_synchronize_noirq(void)
> +{
> +	struct device *dev;
> +
> +	async_synchronize_full();
> +
> +	list_for_each_entry(dev, &dpm_list, power.entry)
> +		INIT_COMPLETION(dev->power.comp);
> +}
> +
> +static void dpm_synchronize(void)
> +{
> +	struct device *dev;
> +
> +	async_synchronize_full();
> +
> +	mutex_lock(&dpm_list_mtx);
> +	list_for_each_entry(dev, &dpm_list, power.entry)
> +		INIT_COMPLETION(dev->power.comp);
> +	mutex_unlock(&dpm_list_mtx);
> +}

Why is it ok to avoid locking in noirq case? Do we really need async for
noirq handlers?


>  /**
> - *	device_resume_noirq - Power on one device (early resume).
> - *	@dev:	Device.
> - *	@state: PM transition of the system being carried out.
> + * __device_resume_noirq - Execute an "early resume" callback for given device.
> + * @dev: Device to resume.
> + * @state: PM transition of the system being carried out.
>   *
> - *	Must be called with interrupts disabled.
> + * The driver of the device won't receive interrupts while this function is
> + * being executed.
>   */

You still want it called with interrupts disabled, right?


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

Re: [RFC][PATCH 1/3] PM: Asynchronous resume of devices

[Rafael J. Wysocki]

On Friday 14 August 2009, Pavel Machek wrote:
> > The patch below introduces a mechanism allowing some devices to be
> > resumed asynchronously, using completions with the following rules:
> > (1) There is a completion, dev->power.comp, for each device object.
> > (2) All of these completions are reset before suspend as well as
> >     each resume stage (dpm_resume_noirq(), dpm_resume()).
> > (3) If dev->power.async_suspend is set for dev or for its parent, the
> >     PM core waits for the parent's completion before attempting to
> >     run the resume callbacks, appropriate for this particular stage
> >     of resume, for dev.
> 
> at least this needs to go in as a comment.

OK, this is a prototype patch, still under discussion.

> > (4) dev->power.comp is completed for each device after running its
> > @@ -411,9 +412,12 @@ struct dev_pm_info {
> >  	pm_message_t		power_state;
> >  	unsigned int		can_wakeup:1;
> >  	unsigned int		should_wakeup:1;
> > +	unsigned		async_suspend:1;
> >  	enum dpm_state		status;		/* Owned by the PM core */
> 
> unsigned int? Or bool?

unsigned means 'unsigned int'.  I should have added 'int', but again, this is
a prototype patch.

> Should it go under config_pm_sleep?

Not necessaily.  'status' is not there as well.

> >  #ifdef CONFIG_PM_SLEEP
> >  	struct list_head	entry;
> > +	struct completion	comp;
> > +	pm_message_t		async_state;
> >  #endif
> >  }
> >  
> > +static inline void device_enable_async_suspend(struct device *dev, bool enable)
> > +{
> > +	if (dev->power.status == DPM_ON)
> > +		dev->power.async_suspend = enable;
> > +}
> > +
> > @@ -163,6 +166,34 @@ void device_pm_move_last(struct device *
> >  	list_move_tail(&dev->power.entry, &dpm_list);
> >  }
> >  
> > +static void dpm_synchronize_noirq(void)
> > +{
> > +	struct device *dev;
> > +
> > +	async_synchronize_full();
> > +
> > +	list_for_each_entry(dev, &dpm_list, power.entry)
> > +		INIT_COMPLETION(dev->power.comp);
> > +}
> > +
> > +static void dpm_synchronize(void)
> > +{
> > +	struct device *dev;
> > +
> > +	async_synchronize_full();
> > +
> > +	mutex_lock(&dpm_list_mtx);
> > +	list_for_each_entry(dev, &dpm_list, power.entry)
> > +		INIT_COMPLETION(dev->power.comp);
> > +	mutex_unlock(&dpm_list_mtx);
> > +}
> 
> Why is it ok to avoid locking in noirq case?

It's not, but we hold dpm_list_mtx throughout the entire noirq suspend.

> Do we really need async for noirq handlers?

Yes, we do.  Specifically, for PCI.

> >  /**
> > - *	device_resume_noirq - Power on one device (early resume).
> > - *	@dev:	Device.
> > - *	@state: PM transition of the system being carried out.
> > + * __device_resume_noirq - Execute an "early resume" callback for given device.
> > + * @dev: Device to resume.
> > + * @state: PM transition of the system being carried out.
> >   *
> > - *	Must be called with interrupts disabled.
> > + * The driver of the device won't receive interrupts while this function is
> > + * being executed.
> >   */
> 
> You still want it called with interrupts disabled, right?

No.  It's not called with interrupts off now.

Thanks,
Rafael

Re: [RFC][PATCH 1/3] PM: Asynchronous resume of devices

[Pavel Machek]

Hi!

> > Do we really need async for noirq handlers?
> 
> Yes, we do.  Specifically, for PCI.

Do noirq parts of PCI handling really so long that we need to make
them async?

Async in normal paths looks pretty much okay to me, but in noirq
parts, locking is tricky, etc...
								Pavel

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

Re: [RFC][PATCH 1/3] PM: Asynchronous resume of devices

[Rafael J. Wysocki]

On Saturday 22 August 2009, Pavel Machek wrote:
> Hi!
> 
> > > Do we really need async for noirq handlers?
> > 
> > Yes, we do.  Specifically, for PCI.
> 
> Do noirq parts of PCI handling really so long that we need to make
> them async?

I think so.  It may get 10 ms to handle a PCI device at this stage usually
and this is all waiting.  If you multiply that by the number of PCI devices ...

> Async in normal paths looks pretty much okay to me, but in noirq
> parts, locking is tricky, etc...

The locking is actually simpler in that case.

Thanks,
Rafael

[RFC][PATCH 2/3] PM: Asynchronous suspend of devices

[Rafael J. Wysocki]

The patch below extends the mechanism introduced by the previous
patch to the suspend part of the PM core.

Asynchronous suspend is slightly more complicated, because in this
case child devices need to be waited for by their parents, so each
parent has to wait on power.comp for each of its children.  Apart
from this, if any of the suspend callbacks executed asynchronously
returns error code, the entire suspend has to be terminated and
rolled back.
---
 drivers/base/power/main.c |  133 +++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 119 insertions(+), 14 deletions(-)

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
@@ -194,6 +194,19 @@ static void device_pm_wait(struct device
 		wait_for_completion(&master->power.comp);
 }
 
+static int device_pm_wait_wrapper(struct device *dev, void *data)
+{
+	struct device *parent = (struct device *)data;
+
+	device_pm_wait(parent, dev);
+	return 0;
+}
+
+static void device_pm_wait_for_children(struct device *parent)
+{
+	device_for_each_child(parent, parent, device_pm_wait_wrapper);
+}
+
 /**
  *	pm_op - execute the PM operation appropiate for given PM event
  *	@dev:	Device.
@@ -639,6 +652,8 @@ EXPORT_SYMBOL_GPL(dpm_resume_end);
 
 /*------------------------- Suspend routines -------------------------*/
 
+static int async_error;
+
 /**
  *	resume_event - return a PM message representing the resume event
  *	               corresponding to given sleep state.
@@ -659,26 +674,61 @@ static pm_message_t resume_event(pm_mess
 }
 
 /**
- *	device_suspend_noirq - Shut down one device (late suspend).
- *	@dev:	Device.
- *	@state: PM transition of the system being carried out.
+ * __device_suspend_noirq - Execute a "late" suspend callback for given device.
+ * @dev: Device to suspend.
+ * @state: PM transition of the system being carried out.
  *
- *	This is called with interrupts off and only a single CPU running.
+ * The driver of the device won't receive interrupts while this function is
+ * being executed.
  */
-static int device_suspend_noirq(struct device *dev, pm_message_t state)
+static int __device_suspend_noirq(struct device *dev, pm_message_t state)
 {
 	int error = 0;
 
-	if (!dev->bus)
-		return 0;
+	device_pm_wait_for_children(dev);
 
-	if (dev->bus->pm) {
+	if (dev->bus && dev->bus->pm) {
 		pm_dev_dbg(dev, state, "LATE ");
 		error = pm_noirq_op(dev, dev->bus->pm, state);
 	}
+
+	complete_all(&dev->power.comp);
+
 	return error;
 }
 
+static void async_suspend_noirq(void *data, async_cookie_t cookie)
+{
+	struct device *dev = (struct device *)data;
+	int error = async_error;
+
+	if (error)
+		return;
+
+	pm_dev_dbg(dev, dev->power.async_state, "async LATE ");
+	error = __device_suspend_noirq(dev, dev->power.async_state);
+	if (error) {
+		pm_dev_err(dev, dev->power.async_state, " async LATE", error);
+		dev->power.status = DPM_OFF;
+	}
+	put_device(dev);
+
+	if (error && !async_error)
+		async_error = error;
+}
+
+static int device_suspend_noirq(struct device *dev, pm_message_t state)
+{
+	if (dev->power.async_suspend) {
+		get_device(dev);
+		dev->power.async_state = state;
+		async_schedule(async_suspend_noirq, dev);
+		return 0;
+	}
+
+	return __device_suspend_noirq(dev, state);
+}
+
 /**
  *	dpm_suspend_noirq - Power down all regular (non-sysdev) devices.
  *	@state: PM transition of the system being carried out.
@@ -696,13 +746,19 @@ int dpm_suspend_noirq(pm_message_t state
 	suspend_device_irqs();
 	mutex_lock(&dpm_list_mtx);
 	list_for_each_entry_reverse(dev, &dpm_list, power.entry) {
+		dev->power.status = DPM_OFF_IRQ;
 		error = device_suspend_noirq(dev, state);
 		if (error) {
 			pm_dev_err(dev, state, " late", error);
+			dev->power.status = DPM_OFF;
+			break;
+		}
+		if (async_error) {
+			error = async_error;
 			break;
 		}
-		dev->power.status = DPM_OFF_IRQ;
 	}
+	dpm_synchronize_noirq();
 	mutex_unlock(&dpm_list_mtx);
 	if (error)
 		dpm_resume_noirq(resume_event(state));
@@ -711,14 +767,15 @@ int dpm_suspend_noirq(pm_message_t state
 EXPORT_SYMBOL_GPL(dpm_suspend_noirq);
 
 /**
- *	device_suspend - Save state of one device.
- *	@dev:	Device.
- *	@state: PM transition of the system being carried out.
+ * __device_suspend - Execute suspend callbacks for given device.
+ * @dev: Device to suspend.
+ * @state: PM transition of the system being carried out.
  */
-static int device_suspend(struct device *dev, pm_message_t state)
+static int __device_suspend(struct device *dev, pm_message_t state)
 {
 	int error = 0;
 
+	device_pm_wait_for_children(dev);
 	down(&dev->sem);
 
 	if (dev->class) {
@@ -755,10 +812,51 @@ static int device_suspend(struct device 
 	}
  End:
 	up(&dev->sem);
+	complete_all(&dev->power.comp);
 
 	return error;
 }
 
+static void async_suspend(void *data, async_cookie_t cookie)
+{
+	struct device *dev = (struct device *)data;
+	int error;
+
+	pm_dev_dbg(dev, dev->power.async_state, "async ");
+
+	mutex_lock(&dpm_list_mtx);
+	error = async_error;
+	mutex_unlock(&dpm_list_mtx);
+	if (error)
+		goto End;
+
+	error = __device_suspend(dev, dev->power.async_state);
+	if (error) {
+		pm_dev_err(dev, dev->power.async_state, " async", error);
+
+		mutex_lock(&dpm_list_mtx);
+		dev->power.status = DPM_SUSPENDING;
+		if (!async_error)
+			async_error = error;
+		mutex_unlock(&dpm_list_mtx);
+	}
+
+ End:
+	put_device(dev);
+}
+
+static int device_suspend(struct device *dev, pm_message_t state)
+{
+	if (dev->power.async_suspend) {
+		get_device(dev);
+		dev->power.async_state = state;
+		async_schedule(async_suspend, dev);
+		return 0;
+	}
+
+	return __device_suspend(dev, state);
+}
+
 /**
  *	dpm_suspend - Suspend every device.
  *	@state: PM transition of the system being carried out.
@@ -776,6 +874,7 @@ static int dpm_suspend(pm_message_t stat
 		struct device *dev = to_device(dpm_list.prev);
 
 		get_device(dev);
+		dev->power.status = DPM_OFF;
 		mutex_unlock(&dpm_list_mtx);
 
 		error = device_suspend(dev, state);
@@ -783,16 +882,21 @@ static int dpm_suspend(pm_message_t stat
 		mutex_lock(&dpm_list_mtx);
 		if (error) {
 			pm_dev_err(dev, state, "", error);
+			dev->power.status = DPM_SUSPENDING;
 			put_device(dev);
 			break;
 		}
-		dev->power.status = DPM_OFF;
 		if (!list_empty(&dev->power.entry))
 			list_move(&dev->power.entry, &list);
 		put_device(dev);
+		if (async_error) {
+			error = async_error;
+			break;
+		}
 	}
 	list_splice(&list, dpm_list.prev);
 	mutex_unlock(&dpm_list_mtx);
+	dpm_synchronize();
 	return error;
 }
 
@@ -848,6 +952,7 @@ static int dpm_prepare(pm_message_t stat
 	INIT_LIST_HEAD(&list);
 	mutex_lock(&dpm_list_mtx);
 	transition_started = true;
+	async_error = 0;
 	while (!list_empty(&dpm_list)) {
 		struct device *dev = to_device(dpm_list.next);
 

Re: [RFC][PATCH 2/3] PM: Asynchronous suspend of devices

[Pavel Machek]

> @@ -659,26 +674,61 @@ static pm_message_t resume_event(pm_mess
>  }
>  
>  /**
> - *	device_suspend_noirq - Shut down one device (late suspend).
> - *	@dev:	Device.
> - *	@state: PM transition of the system being carried out.
> + * __device_suspend_noirq - Execute a "late" suspend callback for given device.
> + * @dev: Device to suspend.
> + * @state: PM transition of the system being carried out.

>   *
> - *	This is called with interrupts off and only a single CPU running.

that still looks like useful comment... why delete it?

> + * The driver of the device won't receive interrupts while this function is
> + * being executed.
>   */
> @@ -696,13 +746,19 @@ int dpm_suspend_noirq(pm_message_t state
>  	suspend_device_irqs();
>  	mutex_lock(&dpm_list_mtx);
>  	list_for_each_entry_reverse(dev, &dpm_list, power.entry) {
> +		dev->power.status = DPM_OFF_IRQ;
>  		error = device_suspend_noirq(dev, state);
>  		if (error) {
>  			pm_dev_err(dev, state, " late", error);
> +			dev->power.status = DPM_OFF;
> +			break;
> +		}
> +		if (async_error) {
> +			error = async_error;
>  			break;

async_error is 'interesting'. How does locking work in noirq case?

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

Re: [RFC][PATCH 2/3] PM: Asynchronous suspend of devices

[Rafael J. Wysocki]

On Friday 14 August 2009, Pavel Machek wrote:
> > @@ -659,26 +674,61 @@ static pm_message_t resume_event(pm_mess
> >  }
> >  
> >  /**
> > - *	device_suspend_noirq - Shut down one device (late suspend).
> > - *	@dev:	Device.
> > - *	@state: PM transition of the system being carried out.
> > + * __device_suspend_noirq - Execute a "late" suspend callback for given device.
> > + * @dev: Device to suspend.
> > + * @state: PM transition of the system being carried out.
> 
> >   *
> > - *	This is called with interrupts off and only a single CPU running.
> 
> that still looks like useful comment... why delete it?

Because it's not valid any more.

> > + * The driver of the device won't receive interrupts while this function is
> > + * being executed.
> >   */
> > @@ -696,13 +746,19 @@ int dpm_suspend_noirq(pm_message_t state
> >  	suspend_device_irqs();
> >  	mutex_lock(&dpm_list_mtx);
> >  	list_for_each_entry_reverse(dev, &dpm_list, power.entry) {
> > +		dev->power.status = DPM_OFF_IRQ;
> >  		error = device_suspend_noirq(dev, state);
> >  		if (error) {
> >  			pm_dev_err(dev, state, " late", error);
> > +			dev->power.status = DPM_OFF;
> > +			break;
> > +		}
> > +		if (async_error) {
> > +			error = async_error;
> >  			break;
> 
> async_error is 'interesting'. How does locking work in noirq case?

It's racy, a little bit. :-)

If two async drivers return errors exactly at the same time, one of them will
win the race, but it doesn't really matter which one wins as long as
async_error is different from zero as a result.  And it will be, since it's
an 'int' and the integrity of these is guaranteed.

Thanks,
Rafael

Re: [RFC][PATCH 2/3] PM: Asynchronous suspend of devices

[Pavel Machek]

> > > + * The driver of the device won't receive interrupts while this function is
> > > + * being executed.
> > >   */
> > > @@ -696,13 +746,19 @@ int dpm_suspend_noirq(pm_message_t state
> > >  	suspend_device_irqs();
> > >  	mutex_lock(&dpm_list_mtx);
> > >  	list_for_each_entry_reverse(dev, &dpm_list, power.entry) {
> > > +		dev->power.status = DPM_OFF_IRQ;
> > >  		error = device_suspend_noirq(dev, state);
> > >  		if (error) {
> > >  			pm_dev_err(dev, state, " late", error);
> > > +			dev->power.status = DPM_OFF;
> > > +			break;
> > > +		}
> > > +		if (async_error) {
> > > +			error = async_error;
> > >  			break;
> > 
> > async_error is 'interesting'. How does locking work in noirq case?
> 
> It's racy, a little bit. :-)
> 
> If two async drivers return errors exactly at the same time, one of them will
> win the race, but it doesn't really matter which one wins as long as
> async_error is different from zero as a result.  And it will be, since it's
> an 'int' and the integrity of these is guaranteed.

Rather than relying on atomicity of 'int' (where half of kernel
hackers says it is and second half says it is not), can we just use
atomic_t? It compiles to same code on sane architectures, and serves
as documentation/warning...

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

Re: [RFC][PATCH 2/3] PM: Asynchronous suspend of devices

[Rafael J. Wysocki]

On Saturday 22 August 2009, Pavel Machek wrote:
> > > > + * The driver of the device won't receive interrupts while this function is
> > > > + * being executed.
> > > >   */
> > > > @@ -696,13 +746,19 @@ int dpm_suspend_noirq(pm_message_t state
> > > >  	suspend_device_irqs();
> > > >  	mutex_lock(&dpm_list_mtx);
> > > >  	list_for_each_entry_reverse(dev, &dpm_list, power.entry) {
> > > > +		dev->power.status = DPM_OFF_IRQ;
> > > >  		error = device_suspend_noirq(dev, state);
> > > >  		if (error) {
> > > >  			pm_dev_err(dev, state, " late", error);
> > > > +			dev->power.status = DPM_OFF;
> > > > +			break;
> > > > +		}
> > > > +		if (async_error) {
> > > > +			error = async_error;
> > > >  			break;
> > > 
> > > async_error is 'interesting'. How does locking work in noirq case?
> > 
> > It's racy, a little bit. :-)
> > 
> > If two async drivers return errors exactly at the same time, one of them will
> > win the race, but it doesn't really matter which one wins as long as
> > async_error is different from zero as a result.  And it will be, since it's
> > an 'int' and the integrity of these is guaranteed.
> 
> Rather than relying on atomicity of 'int' (where half of kernel
> hackers says it is and second half says it is not), can we just use
> atomic_t? It compiles to same code on sane architectures, and serves
> as documentation/warning...

I used atomic_t for that in the updated patches, already sent a few days ago.
Please refer to that code.

Thanks,
Rafael

[RFC][PATCH 3/3] PM: Asynchronous suspend and resume for ACPI battery

[Rafael J. Wysocki]

Allow the ACPI battery device to be resumed and suspended
asynchronously.

This only means that the driver's suspend and resume callbacks may
be executed in parallel with the callbacks of devices that don't
directly depend on the battery device (i.e. that are not its parent
or children).

Index: linux-2.6/drivers/acpi/battery.c
===================================================================
--- linux-2.6.orig/drivers/acpi/battery.c
+++ linux-2.6/drivers/acpi/battery.c
@@ -837,6 +837,7 @@ static int acpi_battery_add(struct acpi_
 		printk(KERN_INFO PREFIX "%s Slot [%s] (battery %s)\n",
 			ACPI_BATTERY_DEVICE_NAME, acpi_device_bid(device),
 			device->status.battery_present ? "present" : "absent");
+		device_enable_async_suspend(&device->dev, true);
 	} else {
 #ifdef CONFIG_ACPI_PROCFS_POWER
 		acpi_battery_remove_fs(device);

Re: [RFC][PATCH 0/3] PM: Asynchronous suspend and resume

[Alan Stern]

On Wed, 12 Aug 2009, Rafael J. Wysocki wrote:

> Hi,
> 
> The following patches introduce a mechanism allowing us to execute device
> drivers' suspend and resume callbacks asynchronously during system sleep
> transitions, such as suspend to RAM.  The idea is explained in the [1/1] patch
> message.
> 
> Comments welcome.

I get the idea.  Not bad.  Have you tried it in a serious way?  For 
example, turning on the async_suspend flag for every device?

In one way it isn't as efficient as it could be.  You fire off a bunch
of async threads and then make many of them wait for parent or child
devices.  They could be doing useful work instead.

It would be interesting to invent a way of representing explicitly the 
non-tree dependencies -- assuming there aren't too many of them!  (I 
can just hear the TI guys hollering about power and timer domains...)

Alan Stern

Re: [RFC][PATCH 0/3] PM: Asynchronous suspend and resume

[Rafael J. Wysocki]

On Wednesday 12 August 2009, Alan Stern wrote:
> On Wed, 12 Aug 2009, Rafael J. Wysocki wrote:
> 
> > Hi,
> > 
> > The following patches introduce a mechanism allowing us to execute device
> > drivers' suspend and resume callbacks asynchronously during system sleep
> > transitions, such as suspend to RAM.  The idea is explained in the [1/1] patch
> > message.
> > 
> > Comments welcome.
> 
> I get the idea.  Not bad.

Thanks!

> Have you tried it in a serious way?  For example, turning on the
> async_suspend flag for every device?

No, I've only tested it with a few selected drivers.  I'm going to try the
"async everyone" scenario, though.

> In one way it isn't as efficient as it could be.  You fire off a bunch
> of async threads and then make many of them wait for parent or child
> devices.  They could be doing useful work instead.

I kind of agree, but then the patches would be more complicated.

> It would be interesting to invent a way of representing explicitly the 
> non-tree dependencies -- assuming there aren't too many of them!  (I 
> can just hear the TI guys hollering about power and timer domains...)

I have an idea.

Every such dependency involves two devices, one of which is a "master"
and the second of which is a "slave", meaning that the "slave" have to be
suspended before the "master" and cannot be resumed before it.  In principle
we could give each device two lists of "dependency objects", one containing
"dependency objects" where the device is the "master" and the other containing
"dependency objects" where the device is the "slave".  Then, each "dependency
object" could be represented as

struct pm_connection {
    struct device *master;
    struct list_head master_hook;
    struct device *slave;
    struct list_head slave_hook;
};

Add some locking, helpers for adding / removing "dependency objects" etc.
and it should work.  Instead of checking the parent, walk the list of
"masters", instead of walking the list of children, walk the list of "slaves".

The core could create those objects for parent-child relationships
automatically, the other ones would have to be added by platforms / bus types /
drivers etc.

This approach has a problem that it's prone to adding circular dependencies by
mistake, but then I think it would apply to any other approach just as well.

Best,
Rafael

Re: [RFC][PATCH 0/3] PM: Asynchronous suspend and resume

[Zhang Rui]

On Thu, 2009-08-13 at 05:43 +0800, Rafael J. Wysocki wrote:
> On Wednesday 12 August 2009, Alan Stern wrote:
> > On Wed, 12 Aug 2009, Rafael J. Wysocki wrote:
> > 
> > > Hi,
> > > 
> > > The following patches introduce a mechanism allowing us to execute device
> > > drivers' suspend and resume callbacks asynchronously during system sleep
> > > transitions, such as suspend to RAM.  The idea is explained in the [1/1] patch
> > > message.
> > > 
> > > Comments welcome.
> > 
> > I get the idea.  Not bad.
> 
> Thanks!
> 
> > Have you tried it in a serious way?  For example, turning on the
> > async_suspend flag for every device?
> 
> No, I've only tested it with a few selected drivers.  I'm going to try the
> "async everyone" scenario, though.
> 
> > In one way it isn't as efficient as it could be.  You fire off a bunch
> > of async threads and then make many of them wait for parent or child
> > devices.  They could be doing useful work instead.
> 
are you talking about this scenario, or I find another problem of this
approach:
there is a part of dpm_list, dev1->dev_aaa->...->dev_bbb->dev2

dev2 is dev1's first child.
dev1 resume takes 1s
dev_aaa~dev_bbb resume takes 0.1s.

if we call device_enable_async_suspend(dev1, true) in order to resume
device1 asynchronously, the real asynchronous resume only happens
between dev1 and dev_aaa to dev_bbb because dev2 needs to wait until
dev1 resume finished.

so kernel schedules dev1 resume in an async thread first, and then takes
0.1s to finish the dev_aaa to dev_bbb resume, and then sleep 0.9s

> I kind of agree, but then the patches would be more complicated.
> 
The problem is that we need to invoke device_resume for every device
synchronously.
I wonder if we can make the child devices inherit the
parent's dev->power.async_suspend flag, so that devices that need to
wait are resumed asynchronously, i.e. we never wait/sleep when parsing
the dpm_list.

this doesn't bring too much benefit in suspend case but it can speed up
the resume process a lot.

Of cause, this is not a problem if we turn on the async_suspend flag for
every device.

> > It would be interesting to invent a way of representing explicitly the 
> > non-tree dependencies -- assuming there aren't too many of them!  (I 
> > can just hear the TI guys hollering about power and timer domains...)
> 
> I have an idea.
> 
> Every such dependency involves two devices, one of which is a "master"
> and the second of which is a "slave", meaning that the "slave" have to be
> suspended before the "master" and cannot be resumed before it.  In principle
> we could give each device two lists of "dependency objects", one containing
> "dependency objects" where the device is the "master" and the other containing
> "dependency objects" where the device is the "slave".  Then, each "dependency
> object" could be represented as
> 
> struct pm_connection {
>     struct device *master;
>     struct list_head master_hook;
>     struct device *slave;
>     struct list_head slave_hook;
> };
> 
> Add some locking, helpers for adding / removing "dependency objects" etc.
> and it should work.  Instead of checking the parent, walk the list of
> "masters", instead of walking the list of children, walk the list of "slaves".
> 
> The core could create those objects for parent-child relationships
> automatically, the other ones would have to be added by platforms / bus types /
> drivers etc.
> 
this sounds great. :)

thanks,
rui

> This approach has a problem that it's prone to adding circular dependencies by
> mistake, but then I think it would apply to any other approach just as well.
> 
> Best,
> Rafael

Re: [RFC][PATCH 0/3] PM: Asynchronous suspend and resume

[Rafael J. Wysocki]

On Thursday 13 August 2009, Zhang Rui wrote:
> On Thu, 2009-08-13 at 05:43 +0800, Rafael J. Wysocki wrote:
> > On Wednesday 12 August 2009, Alan Stern wrote:
> > > On Wed, 12 Aug 2009, Rafael J. Wysocki wrote:
> > > 
> > > > Hi,
> > > > 
> > > > The following patches introduce a mechanism allowing us to execute device
> > > > drivers' suspend and resume callbacks asynchronously during system sleep
> > > > transitions, such as suspend to RAM.  The idea is explained in the [1/1] patch
> > > > message.
> > > > 
> > > > Comments welcome.
> > > 
> > > I get the idea.  Not bad.
> > 
> > Thanks!
> > 
> > > Have you tried it in a serious way?  For example, turning on the
> > > async_suspend flag for every device?
> > 
> > No, I've only tested it with a few selected drivers.  I'm going to try the
> > "async everyone" scenario, though.
> > 
> > > In one way it isn't as efficient as it could be.  You fire off a bunch
> > > of async threads and then make many of them wait for parent or child
> > > devices.  They could be doing useful work instead.
> > 
> are you talking about this scenario, or I find another problem of this
> approach:
> there is a part of dpm_list, dev1->dev_aaa->...->dev_bbb->dev2
> 
> dev2 is dev1's first child.
> dev1 resume takes 1s
> dev_aaa~dev_bbb resume takes 0.1s.
> 
> if we call device_enable_async_suspend(dev1, true) in order to resume
> device1 asynchronously, the real asynchronous resume only happens
> between dev1 and dev_aaa to dev_bbb because dev2 needs to wait until
> dev1 resume finished.

Yes, that's how it works, but I would call it a limitation rather than a
problem.  It partially stems from the fact that __async_schedule() executes
ptr() synchronously in some circumstances (e.g. async_enabled unset), so the
suspend and resume callbacks have to be scheduled in the same order, in which
they would have been called synchronously.

> so kernel schedules dev1 resume in an async thread first, and then takes
> 0.1s to finish the dev_aaa to dev_bbb resume, and then sleep 0.9s
> 
> > I kind of agree, but then the patches would be more complicated.
> > 
> The problem is that we need to invoke device_resume for every device
> synchronously.

Yes, we do.

> I wonder if we can make the child devices inherit the
> parent's dev->power.async_suspend flag, so that devices that need to
> wait are resumed asynchronously, i.e. we never wait/sleep when parsing
> the dpm_list.
> 
> this doesn't bring too much benefit in suspend case but it can speed up
> the resume process a lot.

We can do that at the core level, because there may be dependencies between
the children the core doesn't know about.  Subsystems are free to set
async_suspend for the entire branches of device hierarchy if they are known
not to contain any off-tree dependencies, but the core has no information
about that.

> Of cause, this is not a problem if we turn on the async_suspend flag for
> every device.

Yes, but we cannot do that at this point.

> > > It would be interesting to invent a way of representing explicitly the 
> > > non-tree dependencies -- assuming there aren't too many of them!  (I 
> > > can just hear the TI guys hollering about power and timer domains...)
> > 
> > I have an idea.
> > 
> > Every such dependency involves two devices, one of which is a "master"
> > and the second of which is a "slave", meaning that the "slave" have to be
> > suspended before the "master" and cannot be resumed before it.  In principle
> > we could give each device two lists of "dependency objects", one containing
> > "dependency objects" where the device is the "master" and the other containing
> > "dependency objects" where the device is the "slave".  Then, each "dependency
> > object" could be represented as
> > 
> > struct pm_connection {
> >     struct device *master;
> >     struct list_head master_hook;
> >     struct device *slave;
> >     struct list_head slave_hook;
> > };
> > 
> > Add some locking, helpers for adding / removing "dependency objects" etc.
> > and it should work.  Instead of checking the parent, walk the list of
> > "masters", instead of walking the list of children, walk the list of "slaves".
> > 
> > The core could create those objects for parent-child relationships
> > automatically, the other ones would have to be added by platforms / bus types /
> > drivers etc.
> > 
> this sounds great. :)

It can only be the next step, though, because it will affect the runtime PM as
well,  among other things.

Thanks,
Rafael

Re: [RFC][PATCH 0/3] PM: Asynchronous suspend and resume

[Zhang Rui]

On Fri, 2009-08-14 at 02:08 +0800, Rafael J. Wysocki wrote:
> On Thursday 13 August 2009, Zhang Rui wrote:
> > On Thu, 2009-08-13 at 05:43 +0800, Rafael J. Wysocki wrote:
> > > On Wednesday 12 August 2009, Alan Stern wrote:
> > > > On Wed, 12 Aug 2009, Rafael J. Wysocki wrote:
> > > > 
> > > > > Hi,
> > > > > 
> > > > > The following patches introduce a mechanism allowing us to execute device
> > > > > drivers' suspend and resume callbacks asynchronously during system sleep
> > > > > transitions, such as suspend to RAM.  The idea is explained in the [1/1] patch
> > > > > message.
> > > > > 
> > > > > Comments welcome.
> > > > 
> > > > I get the idea.  Not bad.
> > > 
> > > Thanks!
> > > 
> > > > Have you tried it in a serious way?  For example, turning on the
> > > > async_suspend flag for every device?
> > > 
> > > No, I've only tested it with a few selected drivers.  I'm going to try the
> > > "async everyone" scenario, though.
> > > 
> > > > In one way it isn't as efficient as it could be.  You fire off a bunch
> > > > of async threads and then make many of them wait for parent or child
> > > > devices.  They could be doing useful work instead.
> > > 
> > are you talking about this scenario, or I find another problem of this
> > approach:
> > there is a part of dpm_list, dev1->dev_aaa->...->dev_bbb->dev2
> > 
> > dev2 is dev1's first child.
> > dev1 resume takes 1s
> > dev_aaa~dev_bbb resume takes 0.1s.
> > 
> > if we call device_enable_async_suspend(dev1, true) in order to resume
> > device1 asynchronously, the real asynchronous resume only happens
> > between dev1 and dev_aaa to dev_bbb because dev2 needs to wait until
> > dev1 resume finished.
> 
> Yes, that's how it works, but I would call it a limitation rather than a
> problem.  It partially stems from the fact that __async_schedule() executes
> ptr() synchronously in some circumstances (e.g. async_enabled unset), so the
> suspend and resume callbacks have to be scheduled in the same order, in which
> they would have been called synchronously.
> 
> > so kernel schedules dev1 resume in an async thread first, and then takes
> > 0.1s to finish the dev_aaa to dev_bbb resume, and then sleep 0.9s
> > 
> > > I kind of agree, but then the patches would be more complicated.
> > > 
> > The problem is that we need to invoke device_resume for every device
> > synchronously.
> 
> Yes, we do.
> 
> > I wonder if we can make the child devices inherit the
> > parent's dev->power.async_suspend flag, so that devices that need to
> > wait are resumed asynchronously, i.e. we never wait/sleep when parsing
> > the dpm_list.
> > 
> > this doesn't bring too much benefit in suspend case but it can speed up
> > the resume process a lot.
> 
> We can do that at the core level,

I think you mean we can't do that at the core level.

>  because there may be dependencies between
> the children the core doesn't know about.  Subsystems are free to set
> async_suspend for the entire branches of device hierarchy if they are known
> not to contain any off-tree dependencies, but the core has no information
> about that.
> 
hmm, but the current patch doesn't handle the off-tree dependencies
neither.
e.g.
dev1, dev2, dev3
dev2 depends on dev1, dev3 is dev1's first child,
we only promise that dev1 has been resumed before resuming dev3 in the
current proposal.

anyway, this is not a problem after the pm_connection stuff is
implemented. :)

thanks,
rui
> > Of cause, this is not a problem if we turn on the async_suspend flag for
> > every device.
> 
> Yes, but we cannot do that at this point.
> 
> > > > It would be interesting to invent a way of representing explicitly the 
> > > > non-tree dependencies -- assuming there aren't too many of them!  (I 
> > > > can just hear the TI guys hollering about power and timer domains...)
> > > 
> > > I have an idea.
> > > 
> > > Every such dependency involves two devices, one of which is a "master"
> > > and the second of which is a "slave", meaning that the "slave" have to be
> > > suspended before the "master" and cannot be resumed before it.  In principle
> > > we could give each device two lists of "dependency objects", one containing
> > > "dependency objects" where the device is the "master" and the other containing
> > > "dependency objects" where the device is the "slave".  Then, each "dependency
> > > object" could be represented as
> > > 
> > > struct pm_connection {
> > >     struct device *master;
> > >     struct list_head master_hook;
> > >     struct device *slave;
> > >     struct list_head slave_hook;
> > > };
> > > 
> > > Add some locking, helpers for adding / removing "dependency objects" etc.
> > > and it should work.  Instead of checking the parent, walk the list of
> > > "masters", instead of walking the list of children, walk the list of "slaves".
> > > 
> > > The core could create those objects for parent-child relationships
> > > automatically, the other ones would have to be added by platforms / bus types /
> > > drivers etc.
> > > 
> > this sounds great. :)
> 
> It can only be the next step, though, because it will affect the runtime PM as
> well,  among other things.
> 



> Thanks,
> Rafael

Re: [RFC][PATCH 0/3] PM: Asynchronous suspend and resume

[Rafael J. Wysocki]

On Friday 14 August 2009, Zhang Rui wrote:
> On Fri, 2009-08-14 at 02:08 +0800, Rafael J. Wysocki wrote:
> > On Thursday 13 August 2009, Zhang Rui wrote:
> > > On Thu, 2009-08-13 at 05:43 +0800, Rafael J. Wysocki wrote:
> > > > On Wednesday 12 August 2009, Alan Stern wrote:
> > > > > On Wed, 12 Aug 2009, Rafael J. Wysocki wrote:
> > > > > 
> > > > > > Hi,
> > > > > > 
> > > > > > The following patches introduce a mechanism allowing us to execute device
> > > > > > drivers' suspend and resume callbacks asynchronously during system sleep
> > > > > > transitions, such as suspend to RAM.  The idea is explained in the [1/1] patch
> > > > > > message.
> > > > > > 
> > > > > > Comments welcome.
> > > > > 
> > > > > I get the idea.  Not bad.
> > > > 
> > > > Thanks!
> > > > 
> > > > > Have you tried it in a serious way?  For example, turning on the
> > > > > async_suspend flag for every device?
> > > > 
> > > > No, I've only tested it with a few selected drivers.  I'm going to try the
> > > > "async everyone" scenario, though.
> > > > 
> > > > > In one way it isn't as efficient as it could be.  You fire off a bunch
> > > > > of async threads and then make many of them wait for parent or child
> > > > > devices.  They could be doing useful work instead.
> > > > 
> > > are you talking about this scenario, or I find another problem of this
> > > approach:
> > > there is a part of dpm_list, dev1->dev_aaa->...->dev_bbb->dev2
> > > 
> > > dev2 is dev1's first child.
> > > dev1 resume takes 1s
> > > dev_aaa~dev_bbb resume takes 0.1s.
> > > 
> > > if we call device_enable_async_suspend(dev1, true) in order to resume
> > > device1 asynchronously, the real asynchronous resume only happens
> > > between dev1 and dev_aaa to dev_bbb because dev2 needs to wait until
> > > dev1 resume finished.
> > 
> > Yes, that's how it works, but I would call it a limitation rather than a
> > problem.  It partially stems from the fact that __async_schedule() executes
> > ptr() synchronously in some circumstances (e.g. async_enabled unset), so the
> > suspend and resume callbacks have to be scheduled in the same order, in which
> > they would have been called synchronously.
> > 
> > > so kernel schedules dev1 resume in an async thread first, and then takes
> > > 0.1s to finish the dev_aaa to dev_bbb resume, and then sleep 0.9s
> > > 
> > > > I kind of agree, but then the patches would be more complicated.
> > > > 
> > > The problem is that we need to invoke device_resume for every device
> > > synchronously.
> > 
> > Yes, we do.
> > 
> > > I wonder if we can make the child devices inherit the
> > > parent's dev->power.async_suspend flag, so that devices that need to
> > > wait are resumed asynchronously, i.e. we never wait/sleep when parsing
> > > the dpm_list.
> > > 
> > > this doesn't bring too much benefit in suspend case but it can speed up
> > > the resume process a lot.
> > 
> > We can do that at the core level,
> 
> I think you mean we can't do that at the core level.

That's correct, sorry for the mistake.

> >  because there may be dependencies between
> > the children the core doesn't know about.  Subsystems are free to set
> > async_suspend for the entire branches of device hierarchy if they are known
> > not to contain any off-tree dependencies, but the core has no information
> > about that.
> > 
> hmm, but the current patch doesn't handle the off-tree dependencies
> neither.
> e.g.
> dev1, dev2, dev3
> dev2 depends on dev1, dev3 is dev1's first child,
> we only promise that dev1 has been resumed before resuming dev3 in the
> current proposal.

Sure, and that's why I said "if they are known not to contain any off-tree
dependencies".  IOW, if a subsystem (e.g. platform) knows that none of its
devices have any off-tree dependencies, it can safely set async_suspend for
all of them.

> anyway, this is not a problem after the pm_connection stuff is
> implemented. :)

Well, that's going to take some time I guess.

Thanks,
Rafael

Re: [RFC][PATCH 0/3] PM: Asynchronous suspend and resume

[Alan Stern]

On Wed, 12 Aug 2009, Rafael J. Wysocki wrote:

> I have an idea.
> 
> Every such dependency involves two devices, one of which is a "master"
> and the second of which is a "slave", meaning that the "slave" have to be
> suspended before the "master" and cannot be resumed before it.  In principle
> we could give each device two lists of "dependency objects", one containing
> "dependency objects" where the device is the "master" and the other containing
> "dependency objects" where the device is the "slave".  Then, each "dependency
> object" could be represented as
> 
> struct pm_connection {
>     struct device *master;
>     struct list_head master_hook;
>     struct device *slave;
>     struct list_head slave_hook;
> };
> 
> Add some locking, helpers for adding / removing "dependency objects" etc.
> and it should work.  Instead of checking the parent, walk the list of
> "masters", instead of walking the list of children, walk the list of "slaves".

If the set of pm_connection objects is reasonably small then the
master_hook and slave_hook aren't needed; you can just read through the
entire list.  Leaving out parent-child connections, which we already
know, would help shrink the set.

The layout of the pm_connection objects could then be improved
slightly.  Each object could contain a variable-sized array of device
pointers together with two integers, M and S.  The first M pointers
would be masters and the remaining S pointers would be slaves.  This
could be useful when, for example, a large number of devices all depend
on one particular power device.

> The core could create those objects for parent-child relationships
> automatically, the other ones would have to be added by platforms / bus types /
> drivers etc.
> 
> This approach has a problem that it's prone to adding circular dependencies by
> mistake, but then I think it would apply to any other approach just as well.

Yes.  In theory we could detect such cycles at runtime, but it's 
probably not worth the effort.

Alan Stern

Re: [RFC][PATCH 0/3] PM: Asynchronous suspend and resume

[Rafael J. Wysocki]

On Thursday 13 August 2009, Alan Stern wrote:
> On Wed, 12 Aug 2009, Rafael J. Wysocki wrote:
> 
> > I have an idea.
> > 
> > Every such dependency involves two devices, one of which is a "master"
> > and the second of which is a "slave", meaning that the "slave" have to be
> > suspended before the "master" and cannot be resumed before it.  In principle
> > we could give each device two lists of "dependency objects", one containing
> > "dependency objects" where the device is the "master" and the other containing
> > "dependency objects" where the device is the "slave".  Then, each "dependency
> > object" could be represented as
> > 
> > struct pm_connection {
> >     struct device *master;
> >     struct list_head master_hook;
> >     struct device *slave;
> >     struct list_head slave_hook;
> > };
> > 
> > Add some locking, helpers for adding / removing "dependency objects" etc.
> > and it should work.  Instead of checking the parent, walk the list of
> > "masters", instead of walking the list of children, walk the list of "slaves".
> 
> If the set of pm_connection objects is reasonably small then the
> master_hook and slave_hook aren't needed; you can just read through the
> entire list.  Leaving out parent-child connections, which we already
> know, would help shrink the set.

Do you mean a global list for the entire system?  I'm not sure if that wouldn't
hurt performance (please note that it also affects runtime PM).

We could use counters of "suspended slaves" and "active masters" to optimize
things, but still updating these counters could take time (one has to find all
devices dependent on given one and update a counter for each of them).

> The layout of the pm_connection objects could then be improved
> slightly.  Each object could contain a variable-sized array of device
> pointers together with two integers, M and S.  The first M pointers
> would be masters and the remaining S pointers would be slaves.  This
> could be useful when, for example, a large number of devices all depend
> on one particular power device.

I'm not sure.  If we use the coutners, this case will be quite easy to handle.

Thanks,
Rafael

Re: [RFC][PATCH 0/3] PM: Asynchronous suspend and resume

[Alan Stern]

On Thu, 13 Aug 2009, Rafael J. Wysocki wrote:

> > If the set of pm_connection objects is reasonably small then the
> > master_hook and slave_hook aren't needed; you can just read through the
> > entire list.  Leaving out parent-child connections, which we already
> > know, would help shrink the set.
> 
> Do you mean a global list for the entire system?  I'm not sure if that wouldn't
> hurt performance (please note that it also affects runtime PM).

It doesn't have to affect runtime PM.  That is, for runtime PM we can 
make the bus subsystems responsible for enforcing the dependencies.

> We could use counters of "suspended slaves" and "active masters" to optimize
> things, but still updating these counters could take time (one has to find all
> devices dependent on given one and update a counter for each of them).

The problem with counters is that devices can be removed in the middle 
of a suspend or resume.  That would mess up the counter values.

Alan Stern

Re: [RFC][PATCH 0/3] PM: Asynchronous suspend and resume

[Rafael J. Wysocki]

On Thursday 13 August 2009, Alan Stern wrote:
> On Thu, 13 Aug 2009, Rafael J. Wysocki wrote:
> 
> > > If the set of pm_connection objects is reasonably small then the
> > > master_hook and slave_hook aren't needed; you can just read through the
> > > entire list.  Leaving out parent-child connections, which we already
> > > know, would help shrink the set.
> > 
> > Do you mean a global list for the entire system?  I'm not sure if that wouldn't
> > hurt performance (please note that it also affects runtime PM).
> 
> It doesn't have to affect runtime PM.  That is, for runtime PM we can 
> make the bus subsystems responsible for enforcing the dependencies.

Since the runtime PM framework takes the parent-child relationships into
accout, I think it should also follow the "master"-"slave" dependencies if they
are represented at the core level.

Still, at this point the question really is how many off-tree dependencies
there are, because the best way to represent them will follow from that.

Thanks,
Rafael

Re: [RFC][PATCH 0/3] PM: Asynchronous suspend and resume

[Rafael J. Wysocki]

On Wednesday 12 August 2009, Rafael J. Wysocki wrote:
> On Wednesday 12 August 2009, Alan Stern wrote:
> > On Wed, 12 Aug 2009, Rafael J. Wysocki wrote:
> > 
> > > Hi,
> > > 
> > > The following patches introduce a mechanism allowing us to execute device
> > > drivers' suspend and resume callbacks asynchronously during system sleep
> > > transitions, such as suspend to RAM.  The idea is explained in the [1/1] patch
> > > message.
> > > 
> > > Comments welcome.
> > 
> > I get the idea.  Not bad.
> 
> Thanks!
> 
> > Have you tried it in a serious way?  For example, turning on the
> > async_suspend flag for every device?
> 
> No, I've only tested it with a few selected drivers.  I'm going to try the
> "async everyone" scenario, though.

On HP nx6325 booted with init=/bin/bash it doesn't pass the
'echo devices > /sys/power/pm_test && echo mem > /sys/power/state' test.

The suspend part actually seems to work, but the resume part crashes
miserably.

Thanks,
Rafael

Re: [RFC][PATCH 0/3] PM: Asynchronous suspend and resume

[Alan Stern]

On Thu, 13 Aug 2009, Rafael J. Wysocki wrote:

> > No, I've only tested it with a few selected drivers.  I'm going to try the
> > "async everyone" scenario, though.
> 
> On HP nx6325 booted with init=/bin/bash it doesn't pass the
> 'echo devices > /sys/power/pm_test && echo mem > /sys/power/state' test.
> 
> The suspend part actually seems to work, but the resume part crashes
> miserably.

Any details?  Can you tell where it crashes?

Maybe I'll try it here just to see what happens...

Alan Stern

Re: [RFC][PATCH 0/3] PM: Asynchronous suspend and resume

[Rafael J. Wysocki]

On Friday 14 August 2009, Alan Stern wrote:
> On Thu, 13 Aug 2009, Rafael J. Wysocki wrote:
> 
> > > No, I've only tested it with a few selected drivers.  I'm going to try the
> > > "async everyone" scenario, though.
> > 
> > On HP nx6325 booted with init=/bin/bash it doesn't pass the
> > 'echo devices > /sys/power/pm_test && echo mem > /sys/power/state' test.
> > 
> > The suspend part actually seems to work, but the resume part crashes
> > miserably.
> 
> Any details?  Can you tell where it crashes?

Unfortunately it ends up in a continuous flood of backtraces, so I can't
say much.

However, if I set async_suspend for all PCI devices, as well as for ACPI battery
and i8042, everything apparently works, even with real suspend-resume.

Thanks,
Rafael

Re: [RFC][PATCH 0/3] PM: Asynchronous suspend and resume

[Alan Stern]

On Fri, 14 Aug 2009, Rafael J. Wysocki wrote:

> On Friday 14 August 2009, Alan Stern wrote:
> > On Thu, 13 Aug 2009, Rafael J. Wysocki wrote:
> > 
> > > > No, I've only tested it with a few selected drivers.  I'm going to try the
> > > > "async everyone" scenario, though.
> > > 
> > > On HP nx6325 booted with init=/bin/bash it doesn't pass the
> > > 'echo devices > /sys/power/pm_test && echo mem > /sys/power/state' test.
> > > 
> > > The suspend part actually seems to work, but the resume part crashes
> > > miserably.
> > 
> > Any details?  Can you tell where it crashes?
> 
> Unfortunately it ends up in a continuous flood of backtraces, so I can't
> say much.

Same sort of thing happened with me, but it appeared to happen during 
suspend.  (I say "appeared" because the system froze for a while at the 
start of the suspend until I hit a key.  This may be related to all the 
recent churn in the TTY layer, combined with the fact that I was using 
a serial console.  It didn't help -- none of the backtraces got sent 
out the serial port.)

> However, if I set async_suspend for all PCI devices, as well as for ACPI battery
> and i8042, everything apparently works, even with real suspend-resume.

I think it would be worthwhile to track down these unknown problems and 
dependencies.  It won't be easy, though...

Alan Stern

Re: [RFC][PATCH 0/3] PM: Asynchronous suspend and resume

[Rafael J. Wysocki]

On Friday 14 August 2009, Alan Stern wrote:
> On Fri, 14 Aug 2009, Rafael J. Wysocki wrote:
> 
> > On Friday 14 August 2009, Alan Stern wrote:
> > > On Thu, 13 Aug 2009, Rafael J. Wysocki wrote:
> > > 
> > > > > No, I've only tested it with a few selected drivers.  I'm going to try the
> > > > > "async everyone" scenario, though.
> > > > 
> > > > On HP nx6325 booted with init=/bin/bash it doesn't pass the
> > > > 'echo devices > /sys/power/pm_test && echo mem > /sys/power/state' test.
> > > > 
> > > > The suspend part actually seems to work, but the resume part crashes
> > > > miserably.
> > > 
> > > Any details?  Can you tell where it crashes?
> > 
> > Unfortunately it ends up in a continuous flood of backtraces, so I can't
> > say much.
> 
> Same sort of thing happened with me, but it appeared to happen during 
> suspend.  (I say "appeared" because the system froze for a while at the 
> start of the suspend until I hit a key.  This may be related to all the 
> recent churn in the TTY layer, combined with the fact that I was using 
> a serial console.  It didn't help -- none of the backtraces got sent 
> out the serial port.)
> 
> > However, if I set async_suspend for all PCI devices, as well as for ACPI battery
> > and i8042, everything apparently works, even with real suspend-resume.
> 
> I think it would be worthwhile to track down these unknown problems and 
> dependencies.  It won't be easy, though...

Agreed.

Hm.  Perhaps it's a good idea to get this code in, so that people can play
with it.  The more people will look into this, the greater the chance to
identify problematic drivers.

Rafael

Re: [RFC][PATCH 0/3] PM: Asynchronous suspend and resume

[Alan Stern]

On Fri, 14 Aug 2009, Rafael J. Wysocki wrote:

> > I think it would be worthwhile to track down these unknown problems and 
> > dependencies.  It won't be easy, though...
> 
> Agreed.
> 
> Hm.  Perhaps it's a good idea to get this code in, so that people can play
> with it.  The more people will look into this, the greater the chance to
> identify problematic drivers.

We don't know if the final form will resemble this at all.  So yes, it 
should be made available for testing purposes (especially if there's 
some mechanism for listing dependencies), but it shouldn't go into 
linux-next or anything like that.

Alan Stern

Re: [linux-pm] [RFC][PATCH 0/3] PM: Asynchronous suspend and resume

[Rafael J. Wysocki]

On Friday 14 August 2009, Rafael J. Wysocki wrote:
> On Friday 14 August 2009, Alan Stern wrote:
> > On Fri, 14 Aug 2009, Rafael J. Wysocki wrote:
> > 
> > > On Friday 14 August 2009, Alan Stern wrote:
> > > > On Thu, 13 Aug 2009, Rafael J. Wysocki wrote:
> > > > 
> > > > > > No, I've only tested it with a few selected drivers.  I'm going to try the
> > > > > > "async everyone" scenario, though.
> > > > > 
> > > > > On HP nx6325 booted with init=/bin/bash it doesn't pass the
> > > > > 'echo devices > /sys/power/pm_test && echo mem > /sys/power/state' test.
> > > > > 
> > > > > The suspend part actually seems to work, but the resume part crashes
> > > > > miserably.
> > > > 
> > > > Any details?  Can you tell where it crashes?
> > > 
> > > Unfortunately it ends up in a continuous flood of backtraces, so I can't
> > > say much.
> > 
> > Same sort of thing happened with me, but it appeared to happen during 
> > suspend.  (I say "appeared" because the system froze for a while at the 
> > start of the suspend until I hit a key.  This may be related to all the 
> > recent churn in the TTY layer, combined with the fact that I was using 
> > a serial console.  It didn't help -- none of the backtraces got sent 
> > out the serial port.)
> > 
> > > However, if I set async_suspend for all PCI devices, as well as for ACPI battery
> > > and i8042, everything apparently works, even with real suspend-resume.
> > 
> > I think it would be worthwhile to track down these unknown problems and 
> > dependencies.  It won't be easy, though...
> 
> Agreed.

One data point: I set async_syspend for all ACPI devices in addition to PCI
devices and that broke things.

Which isn't really strange given that PCI devices generally have ACPI
counterparts linked to them that I guess should be suspended later.

Thanks,
Rafael

Re: [RFC][PATCH 0/3] PM: Asynchronous suspend and resume

[Pavel Machek]

> The following patches introduce a mechanism allowing us to execute device
> drivers' suspend and resume callbacks asynchronously during system sleep
> transitions, such as suspend to RAM.  The idea is explained in the [1/1] patch
> message.

it looks sane to me... Not too long and nice code :-)
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [RFC][PATCH 0/3] PM: Asynchronous suspend and resume

[Rafael J. Wysocki]

On Friday 14 August 2009, Pavel Machek wrote:
> > The following patches introduce a mechanism allowing us to execute device
> > drivers' suspend and resume callbacks asynchronously during system sleep
> > transitions, such as suspend to RAM.  The idea is explained in the [1/1] patch
> > message.
> 
> it looks sane to me... Not too long and nice code :-)

Well, thanks.

Alan seems to have some reservations, though.

Rafael

[RFC][PATCH 0/7] PM: Asynchronous suspend and resume (updated)

[Rafael J. Wysocki]

On Wednesday 12 August 2009, Rafael J. Wysocki wrote:
> Hi,
> 
> The following patches introduce a mechanism allowing us to execute device
> drivers' suspend and resume callbacks asynchronously during system sleep
> transitions, such as suspend to RAM.  The idea is explained in the [1/1] patch
> message.

Changes:

* Added [1/7] that fixes kerneldoc comments in drivers/base/power/main.c
  (this is a 2.6.32 candidate).

* Added [2/7] adding a framework for representing PM link (idea described
  in the patch message).

* [3/7] is the async resume patch (idea described in the patch message).

* [4/7] is the async suspend patch.

* [5/7] - [7/7] set async_suspend for devices in a few selected subsystems.

The patches have been tested on HP nx6325.

Comments welcome.

Thanks,
Rafael

[RFC][PATCH 1/7] PM: Update kerneldoc comments in drivers/base/power/main.c

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>
Subject: PM: Update kerneldoc comments in drivers/base/power/main.c

The kerneldoc comments in drivers/base/power/main.c are generally
outdated and some of them don't describe the functions very
accurately.  Update them and standardize the format to use spaces
instead of tabs.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/base/power/main.c |  169 ++++++++++++++++++++++------------------------
 1 file changed, 84 insertions(+), 85 deletions(-)

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
@@ -50,7 +50,7 @@ static DEFINE_MUTEX(dpm_list_mtx);
 static bool transition_started;
 
 /**
- * device_pm_init - Initialize the PM-related part of a device object
+ * device_pm_init - Initialize the PM-related part of a device object.
  * @dev: Device object being initialized.
  */
 void device_pm_init(struct device *dev)
@@ -61,7 +61,7 @@ void device_pm_init(struct device *dev)
 }
 
 /**
- *	device_pm_lock - lock the list of active devices used by the PM core
+ * device_pm_lock - Lock the list of active devices used by the PM core.
  */
 void device_pm_lock(void)
 {
@@ -69,7 +69,7 @@ void device_pm_lock(void)
 }
 
 /**
- *	device_pm_unlock - unlock the list of active devices used by the PM core
+ * device_pm_unlock - Unlock the list of active devices used by the PM core.
  */
 void device_pm_unlock(void)
 {
@@ -77,8 +77,8 @@ void device_pm_unlock(void)
 }
 
 /**
- *	device_pm_add - add a device to the list of active devices
- *	@dev:	Device to be added to the list
+ * device_pm_add - Add a device to the PM core's list of active devices.
+ * @dev: Device to add to the list.
  */
 void device_pm_add(struct device *dev)
 {
@@ -104,10 +104,8 @@ void device_pm_add(struct device *dev)
 }
 
 /**
- *	device_pm_remove - remove a device from the list of active devices
- *	@dev:	Device to be removed from the list
- *
- *	This function also removes the device's PM-related sysfs attributes.
+ * device_pm_remove - Remove a device from the PM core's list of active devices.
+ * @dev: Device to be removed from the list.
  */
 void device_pm_remove(struct device *dev)
 {
@@ -121,9 +119,9 @@ void device_pm_remove(struct device *dev
 }
 
 /**
- *	device_pm_move_before - move device in dpm_list
- *	@deva:  Device to move in dpm_list
- *	@devb:  Device @deva should come before
+ * device_pm_move_before - Move device in the PM core's list of active devices.
+ * @deva: Device to move in dpm_list.
+ * @devb: Device @deva should come before.
  */
 void device_pm_move_before(struct device *deva, struct device *devb)
 {
@@ -137,9 +135,9 @@ void device_pm_move_before(struct device
 }
 
 /**
- *	device_pm_move_after - move device in dpm_list
- *	@deva:  Device to move in dpm_list
- *	@devb:  Device @deva should come after
+ * device_pm_move_after - Move device in the PM core's list of active devices.
+ * @deva: Device to move in dpm_list.
+ * @devb: Device @deva should come after.
  */
 void device_pm_move_after(struct device *deva, struct device *devb)
 {
@@ -153,8 +151,8 @@ void device_pm_move_after(struct device 
 }
 
 /**
- * 	device_pm_move_last - move device to end of dpm_list
- * 	@dev:   Device to move in dpm_list
+ * device_pm_move_last - Move device to end of the PM core's list of devices.
+ * @dev: Device to move in dpm_list.
  */
 void device_pm_move_last(struct device *dev)
 {
@@ -165,10 +163,10 @@ void device_pm_move_last(struct device *
 }
 
 /**
- *	pm_op - execute the PM operation appropiate for given PM event
- *	@dev:	Device.
- *	@ops:	PM operations to choose from.
- *	@state:	PM transition of the system being carried out.
+ * pm_op - Execute the PM operation appropiate for given PM event.
+ * @dev: Device to handle.
+ * @ops: PM operations to choose from.
+ * @state: PM transition of the system being carried out.
  */
 static int pm_op(struct device *dev,
 		 const struct dev_pm_ops *ops,
@@ -226,13 +224,13 @@ static int pm_op(struct device *dev,
 }
 
 /**
- *	pm_noirq_op - execute the PM operation appropiate for given PM event
- *	@dev:	Device.
- *	@ops:	PM operations to choose from.
- *	@state: PM transition of the system being carried out.
+ * pm_noirq_op - Execute the PM operation appropiate for given PM event.
+ * @dev: Device to handle.
+ * @ops: PM operations to choose from.
+ * @state: PM transition of the system being carried out.
  *
- *	The operation is executed with interrupts disabled by the only remaining
- *	functional CPU in the system.
+ * The driver of @dev will not receive interrupts while this fuction is being
+ * executed.
  */
 static int pm_noirq_op(struct device *dev,
 			const struct dev_pm_ops *ops,
@@ -330,11 +328,12 @@ static void pm_dev_err(struct device *de
 /*------------------------- Resume routines -------------------------*/
 
 /**
- *	device_resume_noirq - Power on one device (early resume).
- *	@dev:	Device.
- *	@state: PM transition of the system being carried out.
+ * device_resume_noirq - Execute an "early resume" callback for given device.
+ * @dev: Device to handle.
+ * @state: PM transition of the system being carried out.
  *
- *	Must be called with interrupts disabled.
+ * The driver of @dev will not receive interrupts while this fuction is being
+ * executed.
  */
 static int device_resume_noirq(struct device *dev, pm_message_t state)
 {
@@ -356,14 +355,11 @@ static int device_resume_noirq(struct de
 }
 
 /**
- *	dpm_resume_noirq - Power on all regular (non-sysdev) devices.
- *	@state: PM transition of the system being carried out.
- *
- *	Call the "noirq" resume handlers for all devices marked as
- *	DPM_OFF_IRQ and enable device drivers to receive interrupts.
+ * dpm_resume_noirq - Execute "early resume" callbacks for non-sysdev devices.
+ * @state: PM transition of the system being carried out.
  *
- *	Must be called under dpm_list_mtx.  Device drivers should not receive
- *	interrupts while it's being executed.
+ * Call the "noirq" resume handlers for all devices marked as DPM_OFF_IRQ and
+ * enable device drivers to receive interrupts.
  */
 void dpm_resume_noirq(pm_message_t state)
 {
@@ -385,9 +381,9 @@ void dpm_resume_noirq(pm_message_t state
 EXPORT_SYMBOL_GPL(dpm_resume_noirq);
 
 /**
- *	device_resume - Restore state for one device.
- *	@dev:	Device.
- *	@state: PM transition of the system being carried out.
+ * device_resume - Execute "resume" callbacks for given device.
+ * @dev: Device to handle.
+ * @state: PM transition of the system being carried out.
  */
 static int device_resume(struct device *dev, pm_message_t state)
 {
@@ -436,11 +432,11 @@ static int device_resume(struct device *
 }
 
 /**
- *	dpm_resume - Resume every device.
- *	@state: PM transition of the system being carried out.
+ * dpm_resume - Execute "resume" callbacks for non-sysdev devices.
+ * @state: PM transition of the system being carried out.
  *
- *	Execute the appropriate "resume" callback for all devices the status of
- *	which indicates that they are inactive.
+ * Execute the appropriate "resume" callback for all devices the status of which
+ * indicates that they are suspended.
  */
 static void dpm_resume(pm_message_t state)
 {
@@ -477,9 +473,9 @@ static void dpm_resume(pm_message_t stat
 }
 
 /**
- *	device_complete - Complete a PM transition for given device
- *	@dev:	Device.
- *	@state: PM transition of the system being carried out.
+ * device_complete - Complete a PM transition for given device.
+ * @dev: Device to handle.
+ * @state: PM transition of the system being carried out.
  */
 static void device_complete(struct device *dev, pm_message_t state)
 {
@@ -504,11 +500,11 @@ static void device_complete(struct devic
 }
 
 /**
- *	dpm_complete - Complete a PM transition for all devices.
- *	@state: PM transition of the system being carried out.
+ * dpm_complete - Complete a PM transition for all non-sysdev devices.
+ * @state: PM transition of the system being carried out.
  *
- *	Execute the ->complete() callbacks for all devices that are not marked
- *	as DPM_ON.
+ * Execute the ->complete() callbacks for all devices the PM status of which is
+ * not DPM_ON (this allows new devices to be registered).
  */
 static void dpm_complete(pm_message_t state)
 {
@@ -538,11 +534,11 @@ static void dpm_complete(pm_message_t st
 }
 
 /**
- *	dpm_resume_end - Restore state of each device in system.
- *	@state: PM transition of the system being carried out.
+ * dpm_resume_end - Execute "resume" callbacks and complete system transition.
+ * @state: PM transition of the system being carried out.
  *
- *	Resume all the devices, unlock them all, and allow new
- *	devices to be registered once again.
+ * Resume all the devices, unlock them all, and complete the PM transition of
+ * the system.
  */
 void dpm_resume_end(pm_message_t state)
 {
@@ -556,9 +552,11 @@ EXPORT_SYMBOL_GPL(dpm_resume_end);
 /*------------------------- Suspend routines -------------------------*/
 
 /**
- *	resume_event - return a PM message representing the resume event
- *	               corresponding to given sleep state.
- *	@sleep_state: PM message representing a sleep state.
+ * resume_event - Return a "resume" message for given "suspend" sleep state.
+ * @sleep_state: PM message representing a sleep state.
+ *
+ * Return a PM message representing the resume event corresponding to given
+ * sleep state.
  */
 static pm_message_t resume_event(pm_message_t sleep_state)
 {
@@ -575,11 +573,12 @@ static pm_message_t resume_event(pm_mess
 }
 
 /**
- *	device_suspend_noirq - Shut down one device (late suspend).
- *	@dev:	Device.
- *	@state: PM transition of the system being carried out.
+ * device_suspend_noirq - Execute a "late suspend" callback for given device.
+ * @dev: Device to handle.
+ * @state: PM transition of the system being carried out.
  *
- *	This is called with interrupts off and only a single CPU running.
+ * The driver of @dev will not receive interrupts while this fuction is being
+ * executed.
  */
 static int device_suspend_noirq(struct device *dev, pm_message_t state)
 {
@@ -596,13 +595,11 @@ static int device_suspend_noirq(struct d
 }
 
 /**
- *	dpm_suspend_noirq - Power down all regular (non-sysdev) devices.
- *	@state: PM transition of the system being carried out.
+ * dpm_suspend_noirq - Execute "late suspend" callbacks for non-sysdev devices.
+ * @state: PM transition of the system being carried out.
  *
- *	Prevent device drivers from receiving interrupts and call the "noirq"
- *	suspend handlers.
- *
- *	Must be called under dpm_list_mtx.
+ * Prevent device drivers from receiving interrupts and call the "noirq" suspend
+ * handlers for all non-sysdev devices.
  */
 int dpm_suspend_noirq(pm_message_t state)
 {
@@ -627,9 +624,9 @@ int dpm_suspend_noirq(pm_message_t state
 EXPORT_SYMBOL_GPL(dpm_suspend_noirq);
 
 /**
- *	device_suspend - Save state of one device.
- *	@dev:	Device.
- *	@state: PM transition of the system being carried out.
+ * device_suspend - Execute "suspend" callbacks for given device.
+ * @dev: Device to handle.
+ * @state: PM transition of the system being carried out.
  */
 static int device_suspend(struct device *dev, pm_message_t state)
 {
@@ -676,10 +673,8 @@ static int device_suspend(struct device 
 }
 
 /**
- *	dpm_suspend - Suspend every device.
- *	@state: PM transition of the system being carried out.
- *
- *	Execute the appropriate "suspend" callbacks for all devices.
+ * dpm_suspend - Execute "suspend" callbacks for all non-sysdev devices.
+ * @state: PM transition of the system being carried out.
  */
 static int dpm_suspend(pm_message_t state)
 {
@@ -713,9 +708,12 @@ static int dpm_suspend(pm_message_t stat
 }
 
 /**
- *	device_prepare - Execute the ->prepare() callback(s) for given device.
- *	@dev:	Device.
- *	@state: PM transition of the system being carried out.
+ * device_prepare - Prepare a device for system power transition.
+ * @dev: Device to handle.
+ * @state: PM transition of the system being carried out.
+ *
+ * Execute the ->prepare() callback(s) for given device.  No new children of the
+ * device may be registered after this function has returned.
  */
 static int device_prepare(struct device *dev, pm_message_t state)
 {
@@ -751,10 +749,10 @@ static int device_prepare(struct device 
 }
 
 /**
- *	dpm_prepare - Prepare all devices for a PM transition.
- *	@state: PM transition of the system being carried out.
+ * dpm_prepare - Prepare all non-sysdev devices for a system PM transition.
+ * @state: PM transition of the system being carried out.
  *
- *	Execute the ->prepare() callback for all devices.
+ * Execute the ->prepare() callback(s) for all devices.
  */
 static int dpm_prepare(pm_message_t state)
 {
@@ -805,10 +803,11 @@ static int dpm_prepare(pm_message_t stat
 }
 
 /**
- *	dpm_suspend_start - Save state and stop all devices in system.
- *	@state: PM transition of the system being carried out.
+ * dpm_suspend_start - Prepare devices for PM transition and suspend them.
+ * @state: PM transition of the system being carried out.
  *
- *	Prepare and suspend all devices.
+ * Prepare all non-sysdev devices for system PM transition and execute "suspend"
+ * callbacks for them.
  */
 int dpm_suspend_start(pm_message_t state)
 {

Re: [RFC][PATCH 1/7] PM: Update kerneldoc comments in drivers/base/power/main.c

[Rafael J. Wysocki]

On Monday 17 August 2009, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rjw@sisk.pl>
> Subject: PM: Update kerneldoc comments in drivers/base/power/main.c
> 
> The kerneldoc comments in drivers/base/power/main.c are generally
> outdated and some of them don't describe the functions very
> accurately.  Update them and standardize the format to use spaces
> instead of tabs.
> 
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>

Any objections to this patch from anyone?  Greg, Alan, Randy?

Best,
Rafael


> ---
>  drivers/base/power/main.c |  169 ++++++++++++++++++++++------------------------
>  1 file changed, 84 insertions(+), 85 deletions(-)
> 
> 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
> @@ -50,7 +50,7 @@ static DEFINE_MUTEX(dpm_list_mtx);
>  static bool transition_started;
>  
>  /**
> - * device_pm_init - Initialize the PM-related part of a device object
> + * device_pm_init - Initialize the PM-related part of a device object.
>   * @dev: Device object being initialized.
>   */
>  void device_pm_init(struct device *dev)
> @@ -61,7 +61,7 @@ void device_pm_init(struct device *dev)
>  }
>  
>  /**
> - *	device_pm_lock - lock the list of active devices used by the PM core
> + * device_pm_lock - Lock the list of active devices used by the PM core.
>   */
>  void device_pm_lock(void)
>  {
> @@ -69,7 +69,7 @@ void device_pm_lock(void)
>  }
>  
>  /**
> - *	device_pm_unlock - unlock the list of active devices used by the PM core
> + * device_pm_unlock - Unlock the list of active devices used by the PM core.
>   */
>  void device_pm_unlock(void)
>  {
> @@ -77,8 +77,8 @@ void device_pm_unlock(void)
>  }
>  
>  /**
> - *	device_pm_add - add a device to the list of active devices
> - *	@dev:	Device to be added to the list
> + * device_pm_add - Add a device to the PM core's list of active devices.
> + * @dev: Device to add to the list.
>   */
>  void device_pm_add(struct device *dev)
>  {
> @@ -104,10 +104,8 @@ void device_pm_add(struct device *dev)
>  }
>  
>  /**
> - *	device_pm_remove - remove a device from the list of active devices
> - *	@dev:	Device to be removed from the list
> - *
> - *	This function also removes the device's PM-related sysfs attributes.
> + * device_pm_remove - Remove a device from the PM core's list of active devices.
> + * @dev: Device to be removed from the list.
>   */
>  void device_pm_remove(struct device *dev)
>  {
> @@ -121,9 +119,9 @@ void device_pm_remove(struct device *dev
>  }
>  
>  /**
> - *	device_pm_move_before - move device in dpm_list
> - *	@deva:  Device to move in dpm_list
> - *	@devb:  Device @deva should come before
> + * device_pm_move_before - Move device in the PM core's list of active devices.
> + * @deva: Device to move in dpm_list.
> + * @devb: Device @deva should come before.
>   */
>  void device_pm_move_before(struct device *deva, struct device *devb)
>  {
> @@ -137,9 +135,9 @@ void device_pm_move_before(struct device
>  }
>  
>  /**
> - *	device_pm_move_after - move device in dpm_list
> - *	@deva:  Device to move in dpm_list
> - *	@devb:  Device @deva should come after
> + * device_pm_move_after - Move device in the PM core's list of active devices.
> + * @deva: Device to move in dpm_list.
> + * @devb: Device @deva should come after.
>   */
>  void device_pm_move_after(struct device *deva, struct device *devb)
>  {
> @@ -153,8 +151,8 @@ void device_pm_move_after(struct device 
>  }
>  
>  /**
> - * 	device_pm_move_last - move device to end of dpm_list
> - * 	@dev:   Device to move in dpm_list
> + * device_pm_move_last - Move device to end of the PM core's list of devices.
> + * @dev: Device to move in dpm_list.
>   */
>  void device_pm_move_last(struct device *dev)
>  {
> @@ -165,10 +163,10 @@ void device_pm_move_last(struct device *
>  }
>  
>  /**
> - *	pm_op - execute the PM operation appropiate for given PM event
> - *	@dev:	Device.
> - *	@ops:	PM operations to choose from.
> - *	@state:	PM transition of the system being carried out.
> + * pm_op - Execute the PM operation appropiate for given PM event.
> + * @dev: Device to handle.
> + * @ops: PM operations to choose from.
> + * @state: PM transition of the system being carried out.
>   */
>  static int pm_op(struct device *dev,
>  		 const struct dev_pm_ops *ops,
> @@ -226,13 +224,13 @@ static int pm_op(struct device *dev,
>  }
>  
>  /**
> - *	pm_noirq_op - execute the PM operation appropiate for given PM event
> - *	@dev:	Device.
> - *	@ops:	PM operations to choose from.
> - *	@state: PM transition of the system being carried out.
> + * pm_noirq_op - Execute the PM operation appropiate for given PM event.
> + * @dev: Device to handle.
> + * @ops: PM operations to choose from.
> + * @state: PM transition of the system being carried out.
>   *
> - *	The operation is executed with interrupts disabled by the only remaining
> - *	functional CPU in the system.
> + * The driver of @dev will not receive interrupts while this fuction is being
> + * executed.
>   */
>  static int pm_noirq_op(struct device *dev,
>  			const struct dev_pm_ops *ops,
> @@ -330,11 +328,12 @@ static void pm_dev_err(struct device *de
>  /*------------------------- Resume routines -------------------------*/
>  
>  /**
> - *	device_resume_noirq - Power on one device (early resume).
> - *	@dev:	Device.
> - *	@state: PM transition of the system being carried out.
> + * device_resume_noirq - Execute an "early resume" callback for given device.
> + * @dev: Device to handle.
> + * @state: PM transition of the system being carried out.
>   *
> - *	Must be called with interrupts disabled.
> + * The driver of @dev will not receive interrupts while this fuction is being
> + * executed.
>   */
>  static int device_resume_noirq(struct device *dev, pm_message_t state)
>  {
> @@ -356,14 +355,11 @@ static int device_resume_noirq(struct de
>  }
>  
>  /**
> - *	dpm_resume_noirq - Power on all regular (non-sysdev) devices.
> - *	@state: PM transition of the system being carried out.
> - *
> - *	Call the "noirq" resume handlers for all devices marked as
> - *	DPM_OFF_IRQ and enable device drivers to receive interrupts.
> + * dpm_resume_noirq - Execute "early resume" callbacks for non-sysdev devices.
> + * @state: PM transition of the system being carried out.
>   *
> - *	Must be called under dpm_list_mtx.  Device drivers should not receive
> - *	interrupts while it's being executed.
> + * Call the "noirq" resume handlers for all devices marked as DPM_OFF_IRQ and
> + * enable device drivers to receive interrupts.
>   */
>  void dpm_resume_noirq(pm_message_t state)
>  {
> @@ -385,9 +381,9 @@ void dpm_resume_noirq(pm_message_t state
>  EXPORT_SYMBOL_GPL(dpm_resume_noirq);
>  
>  /**
> - *	device_resume - Restore state for one device.
> - *	@dev:	Device.
> - *	@state: PM transition of the system being carried out.
> + * device_resume - Execute "resume" callbacks for given device.
> + * @dev: Device to handle.
> + * @state: PM transition of the system being carried out.
>   */
>  static int device_resume(struct device *dev, pm_message_t state)
>  {
> @@ -436,11 +432,11 @@ static int device_resume(struct device *
>  }
>  
>  /**
> - *	dpm_resume - Resume every device.
> - *	@state: PM transition of the system being carried out.
> + * dpm_resume - Execute "resume" callbacks for non-sysdev devices.
> + * @state: PM transition of the system being carried out.
>   *
> - *	Execute the appropriate "resume" callback for all devices the status of
> - *	which indicates that they are inactive.
> + * Execute the appropriate "resume" callback for all devices the status of which
> + * indicates that they are suspended.
>   */
>  static void dpm_resume(pm_message_t state)
>  {
> @@ -477,9 +473,9 @@ static void dpm_resume(pm_message_t stat
>  }
>  
>  /**
> - *	device_complete - Complete a PM transition for given device
> - *	@dev:	Device.
> - *	@state: PM transition of the system being carried out.
> + * device_complete - Complete a PM transition for given device.
> + * @dev: Device to handle.
> + * @state: PM transition of the system being carried out.
>   */
>  static void device_complete(struct device *dev, pm_message_t state)
>  {
> @@ -504,11 +500,11 @@ static void device_complete(struct devic
>  }
>  
>  /**
> - *	dpm_complete - Complete a PM transition for all devices.
> - *	@state: PM transition of the system being carried out.
> + * dpm_complete - Complete a PM transition for all non-sysdev devices.
> + * @state: PM transition of the system being carried out.
>   *
> - *	Execute the ->complete() callbacks for all devices that are not marked
> - *	as DPM_ON.
> + * Execute the ->complete() callbacks for all devices the PM status of which is
> + * not DPM_ON (this allows new devices to be registered).
>   */
>  static void dpm_complete(pm_message_t state)
>  {
> @@ -538,11 +534,11 @@ static void dpm_complete(pm_message_t st
>  }
>  
>  /**
> - *	dpm_resume_end - Restore state of each device in system.
> - *	@state: PM transition of the system being carried out.
> + * dpm_resume_end - Execute "resume" callbacks and complete system transition.
> + * @state: PM transition of the system being carried out.
>   *
> - *	Resume all the devices, unlock them all, and allow new
> - *	devices to be registered once again.
> + * Resume all the devices, unlock them all, and complete the PM transition of
> + * the system.
>   */
>  void dpm_resume_end(pm_message_t state)
>  {
> @@ -556,9 +552,11 @@ EXPORT_SYMBOL_GPL(dpm_resume_end);
>  /*------------------------- Suspend routines -------------------------*/
>  
>  /**
> - *	resume_event - return a PM message representing the resume event
> - *	               corresponding to given sleep state.
> - *	@sleep_state: PM message representing a sleep state.
> + * resume_event - Return a "resume" message for given "suspend" sleep state.
> + * @sleep_state: PM message representing a sleep state.
> + *
> + * Return a PM message representing the resume event corresponding to given
> + * sleep state.
>   */
>  static pm_message_t resume_event(pm_message_t sleep_state)
>  {
> @@ -575,11 +573,12 @@ static pm_message_t resume_event(pm_mess
>  }
>  
>  /**
> - *	device_suspend_noirq - Shut down one device (late suspend).
> - *	@dev:	Device.
> - *	@state: PM transition of the system being carried out.
> + * device_suspend_noirq - Execute a "late suspend" callback for given device.
> + * @dev: Device to handle.
> + * @state: PM transition of the system being carried out.
>   *
> - *	This is called with interrupts off and only a single CPU running.
> + * The driver of @dev will not receive interrupts while this fuction is being
> + * executed.
>   */
>  static int device_suspend_noirq(struct device *dev, pm_message_t state)
>  {
> @@ -596,13 +595,11 @@ static int device_suspend_noirq(struct d
>  }
>  
>  /**
> - *	dpm_suspend_noirq - Power down all regular (non-sysdev) devices.
> - *	@state: PM transition of the system being carried out.
> + * dpm_suspend_noirq - Execute "late suspend" callbacks for non-sysdev devices.
> + * @state: PM transition of the system being carried out.
>   *
> - *	Prevent device drivers from receiving interrupts and call the "noirq"
> - *	suspend handlers.
> - *
> - *	Must be called under dpm_list_mtx.
> + * Prevent device drivers from receiving interrupts and call the "noirq" suspend
> + * handlers for all non-sysdev devices.
>   */
>  int dpm_suspend_noirq(pm_message_t state)
>  {
> @@ -627,9 +624,9 @@ int dpm_suspend_noirq(pm_message_t state
>  EXPORT_SYMBOL_GPL(dpm_suspend_noirq);
>  
>  /**
> - *	device_suspend - Save state of one device.
> - *	@dev:	Device.
> - *	@state: PM transition of the system being carried out.
> + * device_suspend - Execute "suspend" callbacks for given device.
> + * @dev: Device to handle.
> + * @state: PM transition of the system being carried out.
>   */
>  static int device_suspend(struct device *dev, pm_message_t state)
>  {
> @@ -676,10 +673,8 @@ static int device_suspend(struct device 
>  }
>  
>  /**
> - *	dpm_suspend - Suspend every device.
> - *	@state: PM transition of the system being carried out.
> - *
> - *	Execute the appropriate "suspend" callbacks for all devices.
> + * dpm_suspend - Execute "suspend" callbacks for all non-sysdev devices.
> + * @state: PM transition of the system being carried out.
>   */
>  static int dpm_suspend(pm_message_t state)
>  {
> @@ -713,9 +708,12 @@ static int dpm_suspend(pm_message_t stat
>  }
>  
>  /**
> - *	device_prepare - Execute the ->prepare() callback(s) for given device.
> - *	@dev:	Device.
> - *	@state: PM transition of the system being carried out.
> + * device_prepare - Prepare a device for system power transition.
> + * @dev: Device to handle.
> + * @state: PM transition of the system being carried out.
> + *
> + * Execute the ->prepare() callback(s) for given device.  No new children of the
> + * device may be registered after this function has returned.
>   */
>  static int device_prepare(struct device *dev, pm_message_t state)
>  {
> @@ -751,10 +749,10 @@ static int device_prepare(struct device 
>  }
>  
>  /**
> - *	dpm_prepare - Prepare all devices for a PM transition.
> - *	@state: PM transition of the system being carried out.
> + * dpm_prepare - Prepare all non-sysdev devices for a system PM transition.
> + * @state: PM transition of the system being carried out.
>   *
> - *	Execute the ->prepare() callback for all devices.
> + * Execute the ->prepare() callback(s) for all devices.
>   */
>  static int dpm_prepare(pm_message_t state)
>  {
> @@ -805,10 +803,11 @@ static int dpm_prepare(pm_message_t stat
>  }
>  
>  /**
> - *	dpm_suspend_start - Save state and stop all devices in system.
> - *	@state: PM transition of the system being carried out.
> + * dpm_suspend_start - Prepare devices for PM transition and suspend them.
> + * @state: PM transition of the system being carried out.
>   *
> - *	Prepare and suspend all devices.
> + * Prepare all non-sysdev devices for system PM transition and execute "suspend"
> + * callbacks for them.
>   */
>  int dpm_suspend_start(pm_message_t state)
>  {
> --

Re: [RFC][PATCH 1/7] PM: Update kerneldoc comments in drivers/base/power/main.c

[Randy Dunlap]

Rafael J. Wysocki wrote:
> On Monday 17 August 2009, Rafael J. Wysocki wrote:
>> From: Rafael J. Wysocki <rjw@sisk.pl>
>> Subject: PM: Update kerneldoc comments in drivers/base/power/main.c
>>
>> The kerneldoc comments in drivers/base/power/main.c are generally
>> outdated and some of them don't describe the functions very
>> accurately.  Update them and standardize the format to use spaces
>> instead of tabs.
>>
>> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> 
> Any objections to this patch from anyone?  Greg, Alan, Randy?

Nope, it's good.  Thanks.
Acked-by: Randy Dunlap <randy.dunlap@oracle.com>


> Best,
> Rafael
> 
> 
>> ---
>>  drivers/base/power/main.c |  169 ++++++++++++++++++++++------------------------
>>  1 file changed, 84 insertions(+), 85 deletions(-)

Re: [RFC][PATCH 1/7] PM: Update kerneldoc comments in drivers/base/power/main.c

[Greg KH]

On Wed, Aug 19, 2009 at 11:57:31PM +0200, Rafael J. Wysocki wrote:
> On Monday 17 August 2009, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rjw@sisk.pl>
> > Subject: PM: Update kerneldoc comments in drivers/base/power/main.c
> > 
> > The kerneldoc comments in drivers/base/power/main.c are generally
> > outdated and some of them don't describe the functions very
> > accurately.  Update them and standardize the format to use spaces
> > instead of tabs.
> > 
> > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> 
> Any objections to this patch from anyone?  Greg, Alan, Randy?

Looks good to me:
	Acked-by: Greg Kroah-Hartman <gregkh@suse.de>

Re: [RFC][PATCH 1/7] PM: Update kerneldoc comments in drivers/base/power/main.c

[Alan Stern]

On Wed, 19 Aug 2009, Rafael J. Wysocki wrote:

> On Monday 17 August 2009, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rjw@sisk.pl>
> > Subject: PM: Update kerneldoc comments in drivers/base/power/main.c
> > 
> > The kerneldoc comments in drivers/base/power/main.c are generally
> > outdated and some of them don't describe the functions very
> > accurately.  Update them and standardize the format to use spaces
> > instead of tabs.
> > 
> > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> 
> Any objections to this patch from anyone?  Greg, Alan, Randy?

Just some very minor changes.

> >  /**
> > - *	pm_op - execute the PM operation appropiate for given PM event
> > - *	@dev:	Device.
> > - *	@ops:	PM operations to choose from.
> > - *	@state:	PM transition of the system being carried out.
> > + * pm_op - Execute the PM operation appropiate for given PM event.
> > + * @dev: Device to handle.
> > + * @ops: PM operations to choose from.
> > + * @state: PM transition of the system being carried out.
> >   */

Here and in several places below, "appropriate" is missing an "r".

> >  /**
> > - *	pm_noirq_op - execute the PM operation appropiate for given PM event
> > - *	@dev:	Device.
> > - *	@ops:	PM operations to choose from.
> > - *	@state: PM transition of the system being carried out.
> > + * pm_noirq_op - Execute the PM operation appropiate for given PM event.
> > + * @dev: Device to handle.
> > + * @ops: PM operations to choose from.
> > + * @state: PM transition of the system being carried out.
> >   *
> > - *	The operation is executed with interrupts disabled by the only remaining
> > - *	functional CPU in the system.
> > + * The driver of @dev will not receive interrupts while this fuction is being
> > + * executed.
> >   */

Here and in several places below, "function" is missing an "n".

> >  /**
> > - *	dpm_resume - Resume every device.
> > - *	@state: PM transition of the system being carried out.
> > + * dpm_resume - Execute "resume" callbacks for non-sysdev devices.
> > + * @state: PM transition of the system being carried out.
> >   *
> > - *	Execute the appropriate "resume" callback for all devices the status of
> > - *	which indicates that they are inactive.
> > + * Execute the appropriate "resume" callback for all devices the status of which
> > + * indicates that they are suspended.
> >   */

The phrasing is slightly awkward.  "... all devices whose status
indicates..." would be better.  Below as well.

> >  /**
> > - *	dpm_resume_end - Restore state of each device in system.
> > - *	@state: PM transition of the system being carried out.
> > + * dpm_resume_end - Execute "resume" callbacks and complete system transition.
> > + * @state: PM transition of the system being carried out.
> >   *
> > - *	Resume all the devices, unlock them all, and allow new
> > - *	devices to be registered once again.
> > + * Resume all the devices, unlock them all, and complete the PM transition of
> > + * the system.
> >   */

The "unlock them all" part is a fossil.  It should be removed.

Alan Stern

Re: [RFC][PATCH 1/7] PM: Update kerneldoc comments in drivers/base/power/main.c

[Rafael J. Wysocki]

On Thursday 20 August 2009, Alan Stern wrote:
> On Wed, 19 Aug 2009, Rafael J. Wysocki wrote:
> 
> > On Monday 17 August 2009, Rafael J. Wysocki wrote:
> > > From: Rafael J. Wysocki <rjw@sisk.pl>
> > > Subject: PM: Update kerneldoc comments in drivers/base/power/main.c
> > > 
> > > The kerneldoc comments in drivers/base/power/main.c are generally
> > > outdated and some of them don't describe the functions very
> > > accurately.  Update them and standardize the format to use spaces
> > > instead of tabs.
> > > 
> > > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> > 
> > Any objections to this patch from anyone?  Greg, Alan, Randy?
> 
> Just some very minor changes.

Thanks for spotting the mistakes.

> > >  /**
> > > - *	pm_op - execute the PM operation appropiate for given PM event
> > > - *	@dev:	Device.
> > > - *	@ops:	PM operations to choose from.
> > > - *	@state:	PM transition of the system being carried out.
> > > + * pm_op - Execute the PM operation appropiate for given PM event.
> > > + * @dev: Device to handle.
> > > + * @ops: PM operations to choose from.
> > > + * @state: PM transition of the system being carried out.
> > >   */
> 
> Here and in several places below, "appropriate" is missing an "r".
> 
> > >  /**
> > > - *	pm_noirq_op - execute the PM operation appropiate for given PM event
> > > - *	@dev:	Device.
> > > - *	@ops:	PM operations to choose from.
> > > - *	@state: PM transition of the system being carried out.
> > > + * pm_noirq_op - Execute the PM operation appropiate for given PM event.
> > > + * @dev: Device to handle.
> > > + * @ops: PM operations to choose from.
> > > + * @state: PM transition of the system being carried out.
> > >   *
> > > - *	The operation is executed with interrupts disabled by the only remaining
> > > - *	functional CPU in the system.
> > > + * The driver of @dev will not receive interrupts while this fuction is being
> > > + * executed.
> > >   */
> 
> Here and in several places below, "function" is missing an "n".
> 
> > >  /**
> > > - *	dpm_resume - Resume every device.
> > > - *	@state: PM transition of the system being carried out.
> > > + * dpm_resume - Execute "resume" callbacks for non-sysdev devices.
> > > + * @state: PM transition of the system being carried out.
> > >   *
> > > - *	Execute the appropriate "resume" callback for all devices the status of
> > > - *	which indicates that they are inactive.
> > > + * Execute the appropriate "resume" callback for all devices the status of which
> > > + * indicates that they are suspended.
> > >   */
> 
> The phrasing is slightly awkward.  "... all devices whose status
> indicates..." would be better.  Below as well.
> 
> > >  /**
> > > - *	dpm_resume_end - Restore state of each device in system.
> > > - *	@state: PM transition of the system being carried out.
> > > + * dpm_resume_end - Execute "resume" callbacks and complete system transition.
> > > + * @state: PM transition of the system being carried out.
> > >   *
> > > - *	Resume all the devices, unlock them all, and allow new
> > > - *	devices to be registered once again.
> > > + * Resume all the devices, unlock them all, and complete the PM transition of
> > > + * the system.
> > >   */
> 
> The "unlock them all" part is a fossil.  It should be removed.

Agreed.

Updated patch is appended.

Thanks,
Rafael

---
From: Rafael J. Wysocki <rjw@sisk.pl>
Subject: PM: Update kerneldoc comments in drivers/base/power/main.c

The kerneldoc comments in drivers/base/power/main.c are generally
outdated and some of them don't describe the functions very
accurately.  Update them and standardize the format to use spaces
instead of tabs.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
Acked-by: Randy Dunlap <randy.dunlap@oracle.com>
Acked-by: Greg Kroah-Hartman <gregkh@suse.de>
---
 drivers/base/power/main.c |  169 ++++++++++++++++++++++------------------------
 1 file changed, 84 insertions(+), 85 deletions(-)

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
@@ -50,7 +50,7 @@ static DEFINE_MUTEX(dpm_list_mtx);
 static bool transition_started;
 
 /**
- * device_pm_init - Initialize the PM-related part of a device object
+ * device_pm_init - Initialize the PM-related part of a device object.
  * @dev: Device object being initialized.
  */
 void device_pm_init(struct device *dev)
@@ -60,7 +60,7 @@ void device_pm_init(struct device *dev)
 }
 
 /**
- *	device_pm_lock - lock the list of active devices used by the PM core
+ * device_pm_lock - Lock the list of active devices used by the PM core.
  */
 void device_pm_lock(void)
 {
@@ -68,7 +68,7 @@ void device_pm_lock(void)
 }
 
 /**
- *	device_pm_unlock - unlock the list of active devices used by the PM core
+ * device_pm_unlock - Unlock the list of active devices used by the PM core.
  */
 void device_pm_unlock(void)
 {
@@ -76,8 +76,8 @@ void device_pm_unlock(void)
 }
 
 /**
- *	device_pm_add - add a device to the list of active devices
- *	@dev:	Device to be added to the list
+ * device_pm_add - Add a device to the PM core's list of active devices.
+ * @dev: Device to add to the list.
  */
 void device_pm_add(struct device *dev)
 {
@@ -103,10 +103,8 @@ void device_pm_add(struct device *dev)
 }
 
 /**
- *	device_pm_remove - remove a device from the list of active devices
- *	@dev:	Device to be removed from the list
- *
- *	This function also removes the device's PM-related sysfs attributes.
+ * device_pm_remove - Remove a device from the PM core's list of active devices.
+ * @dev: Device to be removed from the list.
  */
 void device_pm_remove(struct device *dev)
 {
@@ -120,9 +118,9 @@ void device_pm_remove(struct device *dev
 }
 
 /**
- *	device_pm_move_before - move device in dpm_list
- *	@deva:  Device to move in dpm_list
- *	@devb:  Device @deva should come before
+ * device_pm_move_before - Move device in the PM core's list of active devices.
+ * @deva: Device to move in dpm_list.
+ * @devb: Device @deva should come before.
  */
 void device_pm_move_before(struct device *deva, struct device *devb)
 {
@@ -136,9 +134,9 @@ void device_pm_move_before(struct device
 }
 
 /**
- *	device_pm_move_after - move device in dpm_list
- *	@deva:  Device to move in dpm_list
- *	@devb:  Device @deva should come after
+ * device_pm_move_after - Move device in the PM core's list of active devices.
+ * @deva: Device to move in dpm_list.
+ * @devb: Device @deva should come after.
  */
 void device_pm_move_after(struct device *deva, struct device *devb)
 {
@@ -152,8 +150,8 @@ void device_pm_move_after(struct device 
 }
 
 /**
- * 	device_pm_move_last - move device to end of dpm_list
- * 	@dev:   Device to move in dpm_list
+ * device_pm_move_last - Move device to end of the PM core's list of devices.
+ * @dev: Device to move in dpm_list.
  */
 void device_pm_move_last(struct device *dev)
 {
@@ -164,10 +162,10 @@ void device_pm_move_last(struct device *
 }
 
 /**
- *	pm_op - execute the PM operation appropiate for given PM event
- *	@dev:	Device.
- *	@ops:	PM operations to choose from.
- *	@state:	PM transition of the system being carried out.
+ * pm_op - Execute the PM operation appropriate for given PM event.
+ * @dev: Device to handle.
+ * @ops: PM operations to choose from.
+ * @state: PM transition of the system being carried out.
  */
 static int pm_op(struct device *dev,
 		 const struct dev_pm_ops *ops,
@@ -225,13 +223,13 @@ static int pm_op(struct device *dev,
 }
 
 /**
- *	pm_noirq_op - execute the PM operation appropiate for given PM event
- *	@dev:	Device.
- *	@ops:	PM operations to choose from.
- *	@state: PM transition of the system being carried out.
+ * pm_noirq_op - Execute the PM operation appropriate for given PM event.
+ * @dev: Device to handle.
+ * @ops: PM operations to choose from.
+ * @state: PM transition of the system being carried out.
  *
- *	The operation is executed with interrupts disabled by the only remaining
- *	functional CPU in the system.
+ * The driver of @dev will not receive interrupts while this function is being
+ * executed.
  */
 static int pm_noirq_op(struct device *dev,
 			const struct dev_pm_ops *ops,
@@ -329,11 +327,12 @@ static void pm_dev_err(struct device *de
 /*------------------------- Resume routines -------------------------*/
 
 /**
- *	device_resume_noirq - Power on one device (early resume).
- *	@dev:	Device.
- *	@state: PM transition of the system being carried out.
+ * device_resume_noirq - Execute an "early resume" callback for given device.
+ * @dev: Device to handle.
+ * @state: PM transition of the system being carried out.
  *
- *	Must be called with interrupts disabled.
+ * The driver of @dev will not receive interrupts while this function is being
+ * executed.
  */
 static int device_resume_noirq(struct device *dev, pm_message_t state)
 {
@@ -355,14 +354,11 @@ static int device_resume_noirq(struct de
 }
 
 /**
- *	dpm_resume_noirq - Power on all regular (non-sysdev) devices.
- *	@state: PM transition of the system being carried out.
- *
- *	Call the "noirq" resume handlers for all devices marked as
- *	DPM_OFF_IRQ and enable device drivers to receive interrupts.
+ * dpm_resume_noirq - Execute "early resume" callbacks for non-sysdev devices.
+ * @state: PM transition of the system being carried out.
  *
- *	Must be called under dpm_list_mtx.  Device drivers should not receive
- *	interrupts while it's being executed.
+ * Call the "noirq" resume handlers for all devices marked as DPM_OFF_IRQ and
+ * enable device drivers to receive interrupts.
  */
 void dpm_resume_noirq(pm_message_t state)
 {
@@ -384,9 +380,9 @@ void dpm_resume_noirq(pm_message_t state
 EXPORT_SYMBOL_GPL(dpm_resume_noirq);
 
 /**
- *	device_resume - Restore state for one device.
- *	@dev:	Device.
- *	@state: PM transition of the system being carried out.
+ * device_resume - Execute "resume" callbacks for given device.
+ * @dev: Device to handle.
+ * @state: PM transition of the system being carried out.
  */
 static int device_resume(struct device *dev, pm_message_t state)
 {
@@ -435,11 +431,11 @@ static int device_resume(struct device *
 }
 
 /**
- *	dpm_resume - Resume every device.
- *	@state: PM transition of the system being carried out.
+ * dpm_resume - Execute "resume" callbacks for non-sysdev devices.
+ * @state: PM transition of the system being carried out.
  *
- *	Execute the appropriate "resume" callback for all devices the status of
- *	which indicates that they are inactive.
+ * Execute the appropriate "resume" callback for all devices whose status
+ * indicates that they are suspended.
  */
 static void dpm_resume(pm_message_t state)
 {
@@ -476,9 +472,9 @@ static void dpm_resume(pm_message_t stat
 }
 
 /**
- *	device_complete - Complete a PM transition for given device
- *	@dev:	Device.
- *	@state: PM transition of the system being carried out.
+ * device_complete - Complete a PM transition for given device.
+ * @dev: Device to handle.
+ * @state: PM transition of the system being carried out.
  */
 static void device_complete(struct device *dev, pm_message_t state)
 {
@@ -503,11 +499,11 @@ static void device_complete(struct devic
 }
 
 /**
- *	dpm_complete - Complete a PM transition for all devices.
- *	@state: PM transition of the system being carried out.
+ * dpm_complete - Complete a PM transition for all non-sysdev devices.
+ * @state: PM transition of the system being carried out.
  *
- *	Execute the ->complete() callbacks for all devices that are not marked
- *	as DPM_ON.
+ * Execute the ->complete() callbacks for all devices whose PM status is not
+ * DPM_ON (this allows new devices to be registered).
  */
 static void dpm_complete(pm_message_t state)
 {
@@ -537,11 +533,11 @@ static void dpm_complete(pm_message_t st
 }
 
 /**
- *	dpm_resume_end - Restore state of each device in system.
- *	@state: PM transition of the system being carried out.
+ * dpm_resume_end - Execute "resume" callbacks and complete system transition.
+ * @state: PM transition of the system being carried out.
  *
- *	Resume all the devices, unlock them all, and allow new
- *	devices to be registered once again.
+ * Execute "resume" callbacks for all devices and complete the PM transition of
+ * the system.
  */
 void dpm_resume_end(pm_message_t state)
 {
@@ -555,9 +551,11 @@ EXPORT_SYMBOL_GPL(dpm_resume_end);
 /*------------------------- Suspend routines -------------------------*/
 
 /**
- *	resume_event - return a PM message representing the resume event
- *	               corresponding to given sleep state.
- *	@sleep_state: PM message representing a sleep state.
+ * resume_event - Return a "resume" message for given "suspend" sleep state.
+ * @sleep_state: PM message representing a sleep state.
+ *
+ * Return a PM message representing the resume event corresponding to given
+ * sleep state.
  */
 static pm_message_t resume_event(pm_message_t sleep_state)
 {
@@ -574,11 +572,12 @@ static pm_message_t resume_event(pm_mess
 }
 
 /**
- *	device_suspend_noirq - Shut down one device (late suspend).
- *	@dev:	Device.
- *	@state: PM transition of the system being carried out.
+ * device_suspend_noirq - Execute a "late suspend" callback for given device.
+ * @dev: Device to handle.
+ * @state: PM transition of the system being carried out.
  *
- *	This is called with interrupts off and only a single CPU running.
+ * The driver of @dev will not receive interrupts while this function is being
+ * executed.
  */
 static int device_suspend_noirq(struct device *dev, pm_message_t state)
 {
@@ -595,13 +594,11 @@ static int device_suspend_noirq(struct d
 }
 
 /**
- *	dpm_suspend_noirq - Power down all regular (non-sysdev) devices.
- *	@state: PM transition of the system being carried out.
+ * dpm_suspend_noirq - Execute "late suspend" callbacks for non-sysdev devices.
+ * @state: PM transition of the system being carried out.
  *
- *	Prevent device drivers from receiving interrupts and call the "noirq"
- *	suspend handlers.
- *
- *	Must be called under dpm_list_mtx.
+ * Prevent device drivers from receiving interrupts and call the "noirq" suspend
+ * handlers for all non-sysdev devices.
  */
 int dpm_suspend_noirq(pm_message_t state)
 {
@@ -626,9 +623,9 @@ int dpm_suspend_noirq(pm_message_t state
 EXPORT_SYMBOL_GPL(dpm_suspend_noirq);
 
 /**
- *	device_suspend - Save state of one device.
- *	@dev:	Device.
- *	@state: PM transition of the system being carried out.
+ * device_suspend - Execute "suspend" callbacks for given device.
+ * @dev: Device to handle.
+ * @state: PM transition of the system being carried out.
  */
 static int device_suspend(struct device *dev, pm_message_t state)
 {
@@ -675,10 +672,8 @@ static int device_suspend(struct device 
 }
 
 /**
- *	dpm_suspend - Suspend every device.
- *	@state: PM transition of the system being carried out.
- *
- *	Execute the appropriate "suspend" callbacks for all devices.
+ * dpm_suspend - Execute "suspend" callbacks for all non-sysdev devices.
+ * @state: PM transition of the system being carried out.
  */
 static int dpm_suspend(pm_message_t state)
 {
@@ -712,9 +707,12 @@ static int dpm_suspend(pm_message_t stat
 }
 
 /**
- *	device_prepare - Execute the ->prepare() callback(s) for given device.
- *	@dev:	Device.
- *	@state: PM transition of the system being carried out.
+ * device_prepare - Prepare a device for system power transition.
+ * @dev: Device to handle.
+ * @state: PM transition of the system being carried out.
+ *
+ * Execute the ->prepare() callback(s) for given device.  No new children of the
+ * device may be registered after this function has returned.
  */
 static int device_prepare(struct device *dev, pm_message_t state)
 {
@@ -750,10 +748,10 @@ static int device_prepare(struct device 
 }
 
 /**
- *	dpm_prepare - Prepare all devices for a PM transition.
- *	@state: PM transition of the system being carried out.
+ * dpm_prepare - Prepare all non-sysdev devices for a system PM transition.
+ * @state: PM transition of the system being carried out.
  *
- *	Execute the ->prepare() callback for all devices.
+ * Execute the ->prepare() callback(s) for all devices.
  */
 static int dpm_prepare(pm_message_t state)
 {
@@ -804,10 +802,11 @@ static int dpm_prepare(pm_message_t stat
 }
 
 /**
- *	dpm_suspend_start - Save state and stop all devices in system.
- *	@state: PM transition of the system being carried out.
+ * dpm_suspend_start - Prepare devices for PM transition and suspend them.
+ * @state: PM transition of the system being carried out.
  *
- *	Prepare and suspend all devices.
+ * Prepare all non-sysdev devices for system PM transition and execute "suspend"
+ * callbacks for them.
  */
 int dpm_suspend_start(pm_message_t state)
 {

Re: [RFC][PATCH 1/7] PM: Update kerneldoc comments in drivers/base/power/main.c

[Alan Stern]

On Thu, 20 Aug 2009, Rafael J. Wysocki wrote:

> On Thursday 20 August 2009, Alan Stern wrote:
> > On Wed, 19 Aug 2009, Rafael J. Wysocki wrote:
> > 
> > > On Monday 17 August 2009, Rafael J. Wysocki wrote:
> > > > From: Rafael J. Wysocki <rjw@sisk.pl>
> > > > Subject: PM: Update kerneldoc comments in drivers/base/power/main.c
> > > > 
> > > > The kerneldoc comments in drivers/base/power/main.c are generally
> > > > outdated and some of them don't describe the functions very
> > > > accurately.  Update them and standardize the format to use spaces
> > > > instead of tabs.
> > > > 
> > > > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> > > 
> > > Any objections to this patch from anyone?  Greg, Alan, Randy?
> > 
> > Just some very minor changes.
> 
> Thanks for spotting the mistakes.
> Updated patch is appended.
> 
> Thanks,
> Rafael
> 
> ---
> From: Rafael J. Wysocki <rjw@sisk.pl>
> Subject: PM: Update kerneldoc comments in drivers/base/power/main.c
> 
> The kerneldoc comments in drivers/base/power/main.c are generally
> outdated and some of them don't describe the functions very
> accurately.  Update them and standardize the format to use spaces
> instead of tabs.
> 
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> Acked-by: Randy Dunlap <randy.dunlap@oracle.com>
> Acked-by: Greg Kroah-Hartman <gregkh@suse.de>

Acked-by: Alan Stern <stern@rowland.harvard.edu>

Re: [RFC][PATCH 1/7] PM: Update kerneldoc comments in drivers/base/power/main.c

[Pavel Machek]

On Mon 2009-08-17 02:16:38, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rjw@sisk.pl>
> Subject: PM: Update kerneldoc comments in drivers/base/power/main.c
> 
> The kerneldoc comments in drivers/base/power/main.c are generally
> outdated and some of them don't describe the functions very
> accurately.  Update them and standardize the format to use spaces
> instead of tabs.
> 
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>

Acked-by: Pavel Machek <pavel@ucw.cz>

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

[RFC][PATCH 2/7] PM: Framework for representing PM links between devices

[Rafael J. Wysocki]

The patch below introduces a framework for representing PM
dependencies between devices.

Every such dependency involves two devices, one of which is a "master"
and the second of which is a "slave", meaning that the "slave" have to
be suspended before the "master" and cannot be resumed before it.  In
principle we could give each device two lists of "dependency
objects", one for the dependencies where the device is the "master"
and the other for the dependencies where the device is the "slave".
Then, each "dependency object" can be represented as

struct pm_link {
    struct device *master;
    struct list_head master_hook;
    struct device *slave;
    struct list_head slave_hook;
};

Add some synchronization, helpers for adding / removing "dependency
objects" etc. and it works.  Instead of checking a device's parent,
walk the list of its "masters", instead of walking the list of a
device's children, walk the list of its "slaves".

The PM core creates these objects for parent-child relationships
automatically, they are also created automatically for ACPI devices
and "regular" devices associated with them.  Other ones will have to
be added by platforms / bus types / drivers etc.

---
 drivers/acpi/glue.c          |    3 
 drivers/base/core.c          |    4 
 drivers/base/power/Makefile  |    2 
 drivers/base/power/common.c  |  201 +++++++++++++++++++++++++++++++++++++++++++
 drivers/base/power/main.c    |   28 +----
 drivers/base/power/power.h   |   33 ++++---
 drivers/base/power/runtime.c |    2 
 include/linux/pm.h           |    4 
 include/linux/pm_link.h      |   30 ++++++
 9 files changed, 266 insertions(+), 41 deletions(-)

Index: linux-2.6/include/linux/pm.h
===================================================================
--- linux-2.6.orig/include/linux/pm.h
+++ linux-2.6/include/linux/pm.h
@@ -408,6 +408,9 @@ enum rpm_request {
 };
 
 struct dev_pm_info {
+	spinlock_t		lock;
+	struct list_head	master_links;
+	struct list_head	slave_links;
 	pm_message_t		power_state;
 	unsigned int		can_wakeup:1;
 	unsigned int		should_wakeup:1;
@@ -420,7 +423,6 @@ struct dev_pm_info {
 	unsigned long		timer_expires;
 	struct work_struct	work;
 	wait_queue_head_t	wait_queue;
-	spinlock_t		lock;
 	atomic_t		usage_count;
 	atomic_t		child_count;
 	unsigned int		disable_depth:3;
Index: linux-2.6/drivers/base/power/power.h
===================================================================
--- linux-2.6.orig/drivers/base/power/power.h
+++ linux-2.6/drivers/base/power/power.h
@@ -1,3 +1,7 @@
+extern void device_pm_init(struct device *dev);
+extern void device_pm_add(struct device *dev);
+extern void device_pm_remove(struct device *dev);
+
 #ifdef CONFIG_PM_RUNTIME
 
 extern void pm_runtime_init(struct device *dev);
@@ -23,7 +27,9 @@ static inline struct device *to_device(s
 	return container_of(entry, struct device, power.entry);
 }
 
-extern void device_pm_init(struct device *dev);
+extern void device_pm_sleep_init(struct device *dev);
+extern void device_pm_list_add(struct device *dev);
+extern void device_pm_list_remove(struct device *dev);
 extern void device_pm_add(struct device *);
 extern void device_pm_remove(struct device *);
 extern void device_pm_move_before(struct device *, struct device *);
@@ -32,17 +38,9 @@ extern void device_pm_move_last(struct d
 
 #else /* !CONFIG_PM_SLEEP */
 
-static inline void device_pm_init(struct device *dev)
-{
-	pm_runtime_init(dev);
-}
-
-static inline void device_pm_remove(struct device *dev)
-{
-	pm_runtime_remove(dev);
-}
-
-static inline void device_pm_add(struct device *dev) {}
+static inline void device_pm_sleep_init(struct device *dev) {}
+static inline void device_pm_list_add(struct device *dev) {}
+static inline void device_pm_list_remove(struct device *dev) {}
 static inline void device_pm_move_before(struct device *deva,
 					 struct device *devb) {}
 static inline void device_pm_move_after(struct device *deva,
@@ -60,7 +58,11 @@ static inline void device_pm_move_last(s
 extern int dpm_sysfs_add(struct device *);
 extern void dpm_sysfs_remove(struct device *);
 
-#else /* CONFIG_PM */
+/* drivers/base/power/link.c */
+extern int pm_link_init(void);
+extern void pm_link_remove_all(struct device *dev);
+
+#else /* !CONFIG_PM */
 
 static inline int dpm_sysfs_add(struct device *dev)
 {
@@ -71,4 +73,7 @@ static inline void dpm_sysfs_remove(stru
 {
 }
 
-#endif
+static inline int pm_link_init(void) { return 0; }
+static inline void pm_link_remove_all(struct device *dev) {}
+
+#endif  /* !CONFIG_PM */
Index: linux-2.6/drivers/base/core.c
===================================================================
--- linux-2.6.orig/drivers/base/core.c
+++ linux-2.6/drivers/base/core.c
@@ -1252,9 +1252,13 @@ int __init devices_init(void)
 	sysfs_dev_char_kobj = kobject_create_and_add("char", dev_kobj);
 	if (!sysfs_dev_char_kobj)
 		goto char_kobj_err;
+	if (pm_link_init())
+		goto pm_link_err;
 
 	return 0;
 
+ pm_link_err:
+	kobject_put(sysfs_dev_char_kobj);
  char_kobj_err:
 	kobject_put(sysfs_dev_block_kobj);
  block_kobj_err:
Index: linux-2.6/include/linux/pm_link.h
===================================================================
--- /dev/null
+++ linux-2.6/include/linux/pm_link.h
@@ -0,0 +1,30 @@
+/*
+ * include/linux/pm_link.h - PM links manipulation core.
+ *
+ * Copyright (c) 2009 Rafael J. Wysocki <rjw@sisk.pl>, Novell Inc.
+ *
+ * This file is released under the GPLv2.
+ */
+
+#ifndef _LINUX_PM_LINK_H
+#define _LINUX_PM_LINK_H
+
+#include <linux/list.h>
+
+struct device;
+
+struct pm_link {
+	struct device *master;
+	struct list_head master_hook;
+	struct device *slave;
+	struct list_head slave_hook;
+};
+
+extern int pm_link_add(struct device *slave, struct device *master);
+extern void pm_link_remove(struct device *dev, struct device *master);
+extern int device_for_each_master(struct device *slave, void *data,
+			   int (*fn)(struct device *dev, void *data));
+extern int device_for_each_slave(struct device *master, void *data,
+			  int (*fn)(struct device *dev, void *data));
+
+#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,4 +1,4 @@
-obj-$(CONFIG_PM)	+= sysfs.o
+obj-$(CONFIG_PM)	+= sysfs.o common.o
 obj-$(CONFIG_PM_SLEEP)	+= main.o
 obj-$(CONFIG_PM_RUNTIME)	+= runtime.o
 obj-$(CONFIG_PM_TRACE_RTC)	+= trace.o
Index: linux-2.6/drivers/base/power/runtime.c
===================================================================
--- linux-2.6.orig/drivers/base/power/runtime.c
+++ linux-2.6/drivers/base/power/runtime.c
@@ -972,8 +972,6 @@ EXPORT_SYMBOL_GPL(pm_runtime_enable);
  */
 void pm_runtime_init(struct device *dev)
 {
-	spin_lock_init(&dev->power.lock);
-
 	dev->power.runtime_status = RPM_SUSPENDED;
 	dev->power.idle_notification = false;
 
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_link.h>
 #include <linux/pm_runtime.h>
 #include <linux/resume-trace.h>
 #include <linux/rwsem.h>
@@ -50,17 +51,6 @@ static DEFINE_MUTEX(dpm_list_mtx);
 static bool transition_started;
 
 /**
- * device_pm_init - Initialize the PM-related part of a device object.
- * @dev: Device object being initialized.
- */
-void device_pm_init(struct device *dev)
-{
-	dev->power.status = DPM_ON;
-	spin_lock_init(&dev->power.lock);
-	pm_runtime_init(dev);
-}
-
-/**
  * device_pm_lock - Lock the list of active devices used by the PM core.
  */
 void device_pm_lock(void)
@@ -77,14 +67,11 @@ void device_pm_unlock(void)
 }
 
 /**
- * device_pm_add - Add a device to the PM core's list of active devices.
+ * device_pm_list_add - Add a device to the PM core's list of active devices.
  * @dev: Device to add to the list.
  */
-void device_pm_add(struct device *dev)
+void device_pm_list_add(struct device *dev)
 {
-	pr_debug("PM: Adding info for %s:%s\n",
-		 dev->bus ? dev->bus->name : "No Bus",
-		 kobject_name(&dev->kobj));
 	mutex_lock(&dpm_list_mtx);
 	if (dev->parent) {
 		if (dev->parent->power.status >= DPM_SUSPENDING)
@@ -98,24 +85,19 @@ void device_pm_add(struct device *dev)
 		 */
 		dev_WARN(dev, "Parentless device registered during a PM transaction\n");
 	}
-
 	list_add_tail(&dev->power.entry, &dpm_list);
 	mutex_unlock(&dpm_list_mtx);
 }
 
 /**
- * device_pm_remove - Remove a device from the PM core's list of active devices.
+ * device_pm_list_remove - Remove a device from the PM core's list of devices.
  * @dev: Device to be removed from the list.
  */
-void device_pm_remove(struct device *dev)
+void device_pm_list_remove(struct device *dev)
 {
-	pr_debug("PM: Removing info for %s:%s\n",
-		 dev->bus ? dev->bus->name : "No Bus",
-		 kobject_name(&dev->kobj));
 	mutex_lock(&dpm_list_mtx);
 	list_del_init(&dev->power.entry);
 	mutex_unlock(&dpm_list_mtx);
-	pm_runtime_remove(dev);
 }
 
 /**
Index: linux-2.6/drivers/base/power/common.c
===================================================================
--- /dev/null
+++ linux-2.6/drivers/base/power/common.c
@@ -0,0 +1,201 @@
+/*
+ * drivers/base/power/common.c - device PM common functions.
+ *
+ * Copyright (c) 2009 Rafael J. Wysocki <rjw@sisk.pl>, Novell Inc.
+ *
+ * This file is released under the GPLv2.
+ */
+
+#include <linux/rculist.h>
+#include <linux/device.h>
+#include <linux/srcu.h>
+#include <linux/pm_link.h>
+
+#include "power.h"
+
+/**
+ * device_pm_init - Initialize the PM-related part of a device object
+ * @dev: Device object being initialized.
+ */
+void device_pm_init(struct device *dev)
+{
+	dev->power.status = DPM_ON;
+	spin_lock_init(&dev->power.lock);
+	INIT_LIST_HEAD(&dev->power.master_links);
+	INIT_LIST_HEAD(&dev->power.slave_links);
+	pm_runtime_init(dev);
+}
+
+void device_pm_add(struct device *dev)
+{
+	pr_debug("PM: Adding info for %s:%s\n",
+		 dev->bus ? dev->bus->name : "No Bus",
+		 kobject_name(&dev->kobj));
+	pm_link_add(dev, dev->parent);
+	device_pm_list_add(dev);
+}
+
+void device_pm_remove(struct device *dev)
+{
+	pr_debug("PM: Removing info for %s:%s\n",
+		 dev->bus ? dev->bus->name : "No Bus",
+		 kobject_name(&dev->kobj));
+	device_pm_list_remove(dev);
+	pm_runtime_remove(dev);
+	pm_link_remove_all(dev);
+}
+
+static struct srcu_struct pm_link_ss;
+static DEFINE_MUTEX(pm_link_mtx);
+
+int pm_link_add(struct device *slave, struct device *master)
+{
+	struct pm_link *link;
+	int error = -ENODEV;
+
+	if (!get_device(master))
+		return error;
+
+	if (!get_device(slave))
+		goto err_slave;
+
+	link = kzalloc(sizeof(*link), GFP_KERNEL);
+	if (!link)
+		goto err_link;
+
+	dev_dbg(slave, "PM: Creating PM link to (master) %s %s\n",
+		dev_driver_string(master), dev_name(master));
+
+	link->master = master;
+	INIT_LIST_HEAD(&link->master_hook);
+	link->slave = slave;
+	INIT_LIST_HEAD(&link->slave_hook);
+
+	spin_lock_irq(&master->power.lock);
+	list_add_tail_rcu(&link->master_hook, &master->power.master_links);
+	spin_unlock_irq(&master->power.lock);
+
+	spin_lock_irq(&slave->power.lock);
+	list_add_tail_rcu(&link->slave_hook, &slave->power.slave_links);
+	spin_unlock_irq(&slave->power.lock);
+
+	return 0;
+
+ err_link:
+	error = -ENOMEM;
+	put_device(slave);
+
+ err_slave:
+	put_device(master);
+
+	return error;
+}
+EXPORT_SYMBOL_GPL(pm_link_add);
+
+static void __pm_link_remove(struct pm_link *link)
+{
+	struct device *master = link->master;
+	struct device *slave = link->slave;
+
+	dev_dbg(slave, "PM: Removing PM link to (master) %s %s\n",
+		dev_driver_string(master), dev_name(master));
+
+	spin_lock_irq(&master->power.lock);
+	list_del_rcu(&link->master_hook);
+	spin_unlock_irq(&master->power.lock);
+
+	spin_lock_irq(&slave->power.lock);
+	list_del_rcu(&link->slave_hook);
+	spin_unlock_irq(&slave->power.lock);
+
+	synchronize_srcu(&pm_link_ss);
+
+	kfree(link);
+
+	put_device(master);
+	put_device(slave);
+}
+
+void pm_link_remove_all(struct device *dev)
+{
+	struct pm_link *link, *n;
+
+	mutex_lock(&pm_link_mtx);
+
+	list_for_each_entry_safe(link, n, &dev->power.master_links, master_hook)
+		__pm_link_remove(link);
+
+	list_for_each_entry_safe(link, n, &dev->power.slave_links, slave_hook)
+		__pm_link_remove(link);
+
+	mutex_unlock(&pm_link_mtx);
+}
+
+void pm_link_remove(struct device *dev, struct device *master)
+{
+	struct pm_link *link, *n;
+
+	mutex_lock(&pm_link_mtx);
+
+	list_for_each_entry_safe(link, n, &dev->power.slave_links, slave_hook) {
+		if (link->master != master)
+			continue;
+
+		__pm_link_remove(link);
+		break;
+	}
+
+	mutex_unlock(&pm_link_mtx);
+}
+EXPORT_SYMBOL_GPL(pm_link_remove);
+
+int device_for_each_master(struct device *slave, void *data,
+			   int (*fn)(struct device *dev, void *data))
+{
+	struct pm_link *link;
+	int idx;
+	int error = 0;
+
+	idx = srcu_read_lock(&pm_link_ss);
+
+	list_for_each_entry(link, &slave->power.slave_links, slave_hook) {
+		struct device *master = link->master;
+
+		error = fn(master, data);
+		if (error)
+			break;
+	}
+
+	srcu_read_unlock(&pm_link_ss, idx);
+
+	return error;
+}
+EXPORT_SYMBOL_GPL(device_for_each_master);
+
+int device_for_each_slave(struct device *master, void *data,
+			  int (*fn)(struct device *dev, void *data))
+{
+	struct pm_link *link;
+	int idx;
+	int error = 0;
+
+	idx = srcu_read_lock(&pm_link_ss);
+
+	list_for_each_entry(link, &master->power.master_links, master_hook) {
+		struct device *slave = link->slave;
+
+		error = fn(slave, data);
+		if (error)
+			break;
+	}
+
+	srcu_read_unlock(&pm_link_ss, idx);
+
+	return error;
+}
+EXPORT_SYMBOL_GPL(device_for_each_slave);
+
+int __init pm_link_init(void)
+{
+	return init_srcu_struct(&pm_link_ss);
+}
Index: linux-2.6/drivers/acpi/glue.c
===================================================================
--- linux-2.6.orig/drivers/acpi/glue.c
+++ linux-2.6/drivers/acpi/glue.c
@@ -11,6 +11,7 @@
 #include <linux/device.h>
 #include <linux/rwsem.h>
 #include <linux/acpi.h>
+#include <linux/pm_link.h>
 
 #define ACPI_GLUE_DEBUG	0
 #if ACPI_GLUE_DEBUG
@@ -170,6 +171,7 @@ static int acpi_bind_one(struct device *
 			device_set_wakeup_enable(dev,
 						acpi_dev->wakeup.state.enabled);
 		}
+		pm_link_add(dev, &acpi_dev->dev);
 	}
 
 	return 0;
@@ -189,6 +191,7 @@ static int acpi_unbind_one(struct device
 					&acpi_dev)) {
 			sysfs_remove_link(&dev->kobj, "firmware_node");
 			sysfs_remove_link(&acpi_dev->dev.kobj, "physical_node");
+			pm_link_remove(dev, &acpi_dev->dev);
 		}
 
 		acpi_detach_data(dev->archdata.acpi_handle,

[RFC][PATCH 2/7 update] PM: Framework for representing PM links between devices

[Rafael J. Wysocki]

On Monday 17 August 2009, Rafael J. Wysocki wrote:
> The patch below introduces a framework for representing PM
> dependencies between devices.
> 
> Every such dependency involves two devices, one of which is a "master"
> and the second of which is a "slave", meaning that the "slave" have to
> be suspended before the "master" and cannot be resumed before it.  In
> principle we could give each device two lists of "dependency
> objects", one for the dependencies where the device is the "master"
> and the other for the dependencies where the device is the "slave".
> Then, each "dependency object" can be represented as
> 
> struct pm_link {
>     struct device *master;
>     struct list_head master_hook;
>     struct device *slave;
>     struct list_head slave_hook;
> };
> 
> Add some synchronization, helpers for adding / removing "dependency
> objects" etc. and it works.  Instead of checking a device's parent,
> walk the list of its "masters", instead of walking the list of a
> device's children, walk the list of its "slaves".

Below is a slightly optimized version of this patch in which the core doesn't
create PM links for parent-child relationships and checks them directly
instead.

Thanks,
Rafael

---
 drivers/acpi/glue.c          |    3 
 drivers/base/core.c          |    4 
 drivers/base/power/Makefile  |    2 
 drivers/base/power/common.c  |  210 +++++++++++++++++++++++++++++++++++++++++++
 drivers/base/power/main.c    |   27 +----
 drivers/base/power/power.h   |   33 +++---
 drivers/base/power/runtime.c |    2 
 include/linux/pm.h           |    4 
 include/linux/pm_link.h      |   30 ++++++
 9 files changed, 275 insertions(+), 40 deletions(-)

Index: linux-2.6/include/linux/pm.h
===================================================================
--- linux-2.6.orig/include/linux/pm.h
+++ linux-2.6/include/linux/pm.h
@@ -408,6 +408,9 @@ enum rpm_request {
 };
 
 struct dev_pm_info {
+	spinlock_t		lock;
+	struct list_head	master_links;
+	struct list_head	slave_links;
 	pm_message_t		power_state;
 	unsigned int		can_wakeup:1;
 	unsigned int		should_wakeup:1;
@@ -420,7 +423,6 @@ struct dev_pm_info {
 	unsigned long		timer_expires;
 	struct work_struct	work;
 	wait_queue_head_t	wait_queue;
-	spinlock_t		lock;
 	atomic_t		usage_count;
 	atomic_t		child_count;
 	unsigned int		disable_depth:3;
Index: linux-2.6/drivers/base/power/power.h
===================================================================
--- linux-2.6.orig/drivers/base/power/power.h
+++ linux-2.6/drivers/base/power/power.h
@@ -1,3 +1,7 @@
+extern void device_pm_init(struct device *dev);
+extern void device_pm_add(struct device *dev);
+extern void device_pm_remove(struct device *dev);
+
 #ifdef CONFIG_PM_RUNTIME
 
 extern void pm_runtime_init(struct device *dev);
@@ -23,7 +27,9 @@ static inline struct device *to_device(s
 	return container_of(entry, struct device, power.entry);
 }
 
-extern void device_pm_init(struct device *dev);
+extern void device_pm_sleep_init(struct device *dev);
+extern void device_pm_list_add(struct device *dev);
+extern void device_pm_list_remove(struct device *dev);
 extern void device_pm_add(struct device *);
 extern void device_pm_remove(struct device *);
 extern void device_pm_move_before(struct device *, struct device *);
@@ -32,17 +38,9 @@ extern void device_pm_move_last(struct d
 
 #else /* !CONFIG_PM_SLEEP */
 
-static inline void device_pm_init(struct device *dev)
-{
-	pm_runtime_init(dev);
-}
-
-static inline void device_pm_remove(struct device *dev)
-{
-	pm_runtime_remove(dev);
-}
-
-static inline void device_pm_add(struct device *dev) {}
+static inline void device_pm_sleep_init(struct device *dev) {}
+static inline void device_pm_list_add(struct device *dev) {}
+static inline void device_pm_list_remove(struct device *dev) {}
 static inline void device_pm_move_before(struct device *deva,
 					 struct device *devb) {}
 static inline void device_pm_move_after(struct device *deva,
@@ -60,7 +58,11 @@ static inline void device_pm_move_last(s
 extern int dpm_sysfs_add(struct device *);
 extern void dpm_sysfs_remove(struct device *);
 
-#else /* CONFIG_PM */
+/* drivers/base/power/link.c */
+extern int pm_link_init(void);
+extern void pm_link_remove_all(struct device *dev);
+
+#else /* !CONFIG_PM */
 
 static inline int dpm_sysfs_add(struct device *dev)
 {
@@ -71,4 +73,7 @@ static inline void dpm_sysfs_remove(stru
 {
 }
 
-#endif
+static inline int pm_link_init(void) { return 0; }
+static inline void pm_link_remove_all(struct device *dev) {}
+
+#endif  /* !CONFIG_PM */
Index: linux-2.6/drivers/base/core.c
===================================================================
--- linux-2.6.orig/drivers/base/core.c
+++ linux-2.6/drivers/base/core.c
@@ -1252,9 +1252,13 @@ int __init devices_init(void)
 	sysfs_dev_char_kobj = kobject_create_and_add("char", dev_kobj);
 	if (!sysfs_dev_char_kobj)
 		goto char_kobj_err;
+	if (pm_link_init())
+		goto pm_link_err;
 
 	return 0;
 
+ pm_link_err:
+	kobject_put(sysfs_dev_char_kobj);
  char_kobj_err:
 	kobject_put(sysfs_dev_block_kobj);
  block_kobj_err:
Index: linux-2.6/include/linux/pm_link.h
===================================================================
--- /dev/null
+++ linux-2.6/include/linux/pm_link.h
@@ -0,0 +1,30 @@
+/*
+ * include/linux/pm_link.h - PM links manipulation core.
+ *
+ * Copyright (c) 2009 Rafael J. Wysocki <rjw@sisk.pl>, Novell Inc.
+ *
+ * This file is released under the GPLv2.
+ */
+
+#ifndef _LINUX_PM_LINK_H
+#define _LINUX_PM_LINK_H
+
+#include <linux/list.h>
+
+struct device;
+
+struct pm_link {
+	struct device *master;
+	struct list_head master_hook;
+	struct device *slave;
+	struct list_head slave_hook;
+};
+
+extern int pm_link_add(struct device *slave, struct device *master);
+extern void pm_link_remove(struct device *dev, struct device *master);
+extern int device_for_each_master(struct device *slave, void *data,
+			   int (*fn)(struct device *dev, void *data));
+extern int device_for_each_slave(struct device *master, void *data,
+			  int (*fn)(struct device *dev, void *data));
+
+#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,4 +1,4 @@
-obj-$(CONFIG_PM)	+= sysfs.o
+obj-$(CONFIG_PM)	+= sysfs.o common.o
 obj-$(CONFIG_PM_SLEEP)	+= main.o
 obj-$(CONFIG_PM_RUNTIME)	+= runtime.o
 obj-$(CONFIG_PM_TRACE_RTC)	+= trace.o
Index: linux-2.6/drivers/base/power/runtime.c
===================================================================
--- linux-2.6.orig/drivers/base/power/runtime.c
+++ linux-2.6/drivers/base/power/runtime.c
@@ -972,8 +972,6 @@ EXPORT_SYMBOL_GPL(pm_runtime_enable);
  */
 void pm_runtime_init(struct device *dev)
 {
-	spin_lock_init(&dev->power.lock);
-
 	dev->power.runtime_status = RPM_SUSPENDED;
 	dev->power.idle_notification = false;
 
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_link.h>
 #include <linux/pm_runtime.h>
 #include <linux/resume-trace.h>
 #include <linux/rwsem.h>
@@ -50,16 +51,6 @@ static DEFINE_MUTEX(dpm_list_mtx);
 static bool transition_started;
 
 /**
- * device_pm_init - Initialize the PM-related part of a device object.
- * @dev: Device object being initialized.
- */
-void device_pm_init(struct device *dev)
-{
-	dev->power.status = DPM_ON;
-	pm_runtime_init(dev);
-}
-
-/**
  * device_pm_lock - Lock the list of active devices used by the PM core.
  */
 void device_pm_lock(void)
@@ -76,14 +67,11 @@ void device_pm_unlock(void)
 }
 
 /**
- * device_pm_add - Add a device to the PM core's list of active devices.
+ * device_pm_list_add - Add a device to the PM core's list of active devices.
  * @dev: Device to add to the list.
  */
-void device_pm_add(struct device *dev)
+void device_pm_list_add(struct device *dev)
 {
-	pr_debug("PM: Adding info for %s:%s\n",
-		 dev->bus ? dev->bus->name : "No Bus",
-		 kobject_name(&dev->kobj));
 	mutex_lock(&dpm_list_mtx);
 	if (dev->parent) {
 		if (dev->parent->power.status >= DPM_SUSPENDING)
@@ -97,24 +85,19 @@ void device_pm_add(struct device *dev)
 		 */
 		dev_WARN(dev, "Parentless device registered during a PM transaction\n");
 	}
-
 	list_add_tail(&dev->power.entry, &dpm_list);
 	mutex_unlock(&dpm_list_mtx);
 }
 
 /**
- * device_pm_remove - Remove a device from the PM core's list of active devices.
+ * device_pm_list_remove - Remove a device from the PM core's list of devices.
  * @dev: Device to be removed from the list.
  */
-void device_pm_remove(struct device *dev)
+void device_pm_list_remove(struct device *dev)
 {
-	pr_debug("PM: Removing info for %s:%s\n",
-		 dev->bus ? dev->bus->name : "No Bus",
-		 kobject_name(&dev->kobj));
 	mutex_lock(&dpm_list_mtx);
 	list_del_init(&dev->power.entry);
 	mutex_unlock(&dpm_list_mtx);
-	pm_runtime_remove(dev);
 }
 
 /**
Index: linux-2.6/drivers/base/power/common.c
===================================================================
--- /dev/null
+++ linux-2.6/drivers/base/power/common.c
@@ -0,0 +1,210 @@
+/*
+ * drivers/base/power/common.c - device PM common functions.
+ *
+ * Copyright (c) 2009 Rafael J. Wysocki <rjw@sisk.pl>, Novell Inc.
+ *
+ * This file is released under the GPLv2.
+ */
+
+#include <linux/rculist.h>
+#include <linux/device.h>
+#include <linux/srcu.h>
+#include <linux/pm_link.h>
+
+#include "power.h"
+
+/**
+ * device_pm_init - Initialize the PM-related part of a device object
+ * @dev: Device object being initialized.
+ */
+void device_pm_init(struct device *dev)
+{
+	dev->power.status = DPM_ON;
+	spin_lock_init(&dev->power.lock);
+	INIT_LIST_HEAD(&dev->power.master_links);
+	INIT_LIST_HEAD(&dev->power.slave_links);
+	pm_runtime_init(dev);
+}
+
+void device_pm_add(struct device *dev)
+{
+	pr_debug("PM: Adding info for %s:%s\n",
+		 dev->bus ? dev->bus->name : "No Bus",
+		 kobject_name(&dev->kobj));
+	device_pm_list_add(dev);
+}
+
+void device_pm_remove(struct device *dev)
+{
+	pr_debug("PM: Removing info for %s:%s\n",
+		 dev->bus ? dev->bus->name : "No Bus",
+		 kobject_name(&dev->kobj));
+	device_pm_list_remove(dev);
+	pm_runtime_remove(dev);
+	pm_link_remove_all(dev);
+}
+
+static struct srcu_struct pm_link_ss;
+static DEFINE_MUTEX(pm_link_mtx);
+
+int pm_link_add(struct device *slave, struct device *master)
+{
+	struct pm_link *link;
+	int error = -ENODEV;
+
+	if (!get_device(master))
+		return error;
+
+	if (!get_device(slave))
+		goto err_slave;
+
+	link = kzalloc(sizeof(*link), GFP_KERNEL);
+	if (!link)
+		goto err_link;
+
+	dev_dbg(slave, "PM: Creating PM link to (master) %s %s\n",
+		dev_driver_string(master), dev_name(master));
+
+	link->master = master;
+	INIT_LIST_HEAD(&link->master_hook);
+	link->slave = slave;
+	INIT_LIST_HEAD(&link->slave_hook);
+
+	spin_lock_irq(&master->power.lock);
+	list_add_tail_rcu(&link->master_hook, &master->power.master_links);
+	spin_unlock_irq(&master->power.lock);
+
+	spin_lock_irq(&slave->power.lock);
+	list_add_tail_rcu(&link->slave_hook, &slave->power.slave_links);
+	spin_unlock_irq(&slave->power.lock);
+
+	return 0;
+
+ err_link:
+	error = -ENOMEM;
+	put_device(slave);
+
+ err_slave:
+	put_device(master);
+
+	return error;
+}
+EXPORT_SYMBOL_GPL(pm_link_add);
+
+static void __pm_link_remove(struct pm_link *link)
+{
+	struct device *master = link->master;
+	struct device *slave = link->slave;
+
+	dev_dbg(slave, "PM: Removing PM link to (master) %s %s\n",
+		dev_driver_string(master), dev_name(master));
+
+	spin_lock_irq(&master->power.lock);
+	list_del_rcu(&link->master_hook);
+	spin_unlock_irq(&master->power.lock);
+
+	spin_lock_irq(&slave->power.lock);
+	list_del_rcu(&link->slave_hook);
+	spin_unlock_irq(&slave->power.lock);
+
+	synchronize_srcu(&pm_link_ss);
+
+	kfree(link);
+
+	put_device(master);
+	put_device(slave);
+}
+
+void pm_link_remove_all(struct device *dev)
+{
+	struct pm_link *link, *n;
+
+	mutex_lock(&pm_link_mtx);
+
+	list_for_each_entry_safe(link, n, &dev->power.master_links, master_hook)
+		__pm_link_remove(link);
+
+	list_for_each_entry_safe(link, n, &dev->power.slave_links, slave_hook)
+		__pm_link_remove(link);
+
+	mutex_unlock(&pm_link_mtx);
+}
+
+void pm_link_remove(struct device *dev, struct device *master)
+{
+	struct pm_link *link, *n;
+
+	mutex_lock(&pm_link_mtx);
+
+	list_for_each_entry_safe(link, n, &dev->power.slave_links, slave_hook) {
+		if (link->master != master)
+			continue;
+
+		__pm_link_remove(link);
+		break;
+	}
+
+	mutex_unlock(&pm_link_mtx);
+}
+EXPORT_SYMBOL_GPL(pm_link_remove);
+
+int device_for_each_master(struct device *slave, void *data,
+			   int (*fn)(struct device *dev, void *data))
+{
+	struct pm_link *link;
+	int idx;
+	int error = 0;
+
+	if (slave->parent) {
+		error = fn(slave->parent, data);
+		if (error)
+			return error;
+	}
+
+	idx = srcu_read_lock(&pm_link_ss);
+
+	list_for_each_entry(link, &slave->power.slave_links, slave_hook) {
+		struct device *master = link->master;
+
+		error = fn(master, data);
+		if (error)
+			break;
+	}
+
+	srcu_read_unlock(&pm_link_ss, idx);
+
+	return error;
+}
+EXPORT_SYMBOL_GPL(device_for_each_master);
+
+int device_for_each_slave(struct device *master, void *data,
+			  int (*fn)(struct device *dev, void *data))
+{
+	struct pm_link *link;
+	int idx;
+	int error;
+
+	error = device_for_each_child(master, data, fn);
+	if (error)
+		return error;
+
+	idx = srcu_read_lock(&pm_link_ss);
+
+	list_for_each_entry(link, &master->power.master_links, master_hook) {
+		struct device *slave = link->slave;
+
+		error = fn(slave, data);
+		if (error)
+			break;
+	}
+
+	srcu_read_unlock(&pm_link_ss, idx);
+
+	return error;
+}
+EXPORT_SYMBOL_GPL(device_for_each_slave);
+
+int __init pm_link_init(void)
+{
+	return init_srcu_struct(&pm_link_ss);
+}
Index: linux-2.6/drivers/acpi/glue.c
===================================================================
--- linux-2.6.orig/drivers/acpi/glue.c
+++ linux-2.6/drivers/acpi/glue.c
@@ -11,6 +11,7 @@
 #include <linux/device.h>
 #include <linux/rwsem.h>
 #include <linux/acpi.h>
+#include <linux/pm_link.h>
 
 #define ACPI_GLUE_DEBUG	0
 #if ACPI_GLUE_DEBUG
@@ -170,6 +171,7 @@ static int acpi_bind_one(struct device *
 			device_set_wakeup_enable(dev,
 						acpi_dev->wakeup.state.enabled);
 		}
+		pm_link_add(dev, &acpi_dev->dev);
 	}
 
 	return 0;
@@ -189,6 +191,7 @@ static int acpi_unbind_one(struct device
 					&acpi_dev)) {
 			sysfs_remove_link(&dev->kobj, "firmware_node");
 			sysfs_remove_link(&acpi_dev->dev.kobj, "physical_node");
+			pm_link_remove(dev, &acpi_dev->dev);
 		}
 
 		acpi_detach_data(dev->archdata.acpi_handle,

[RFC][PATCH 3/7] PM: Asynchronous resume of I/O devices

[Rafael J. Wysocki]

Theoretically, the total time of system sleep transitions (suspend
to RAM, hibernation) can be reduced by running suspend and resume
callbacks of device drivers in parallel with each other.  However,
there are dependencies between devices such that, for example, we may
not be allowed to put one device into a low power state before
anohter one has been suspended (e.g. we cannot suspend a bridge
before suspending all devices behind it).  In particular, we're not
allowed to suspend the parent of a device before suspending the
device itself.  Analogously, we're not allowed to resume a device
before resuming its parent.

Thus, to make it possible to execute suspend and resume callbacks
provided by device drivers in parallel with each other, we need to
provide a synchronization mechanism preventing the dependencies
between devices from being violated.

The patch below introduces a mechanism allowing some devices to be
resumed asynchronously, using completions with the following rules:
(1) There is a completion, dev->power.comp, for each device object.
(2) All of these completions are reset before suspend as well as
    each resume stage (dpm_resume_noirq(), dpm_resume()).
(3) If dev->power.async_suspend is set for dev or for one of devices
    it depends on, the PM core waits for the "master" device's
    completion before attempting to run the resume callbacks,
    appropriate for this particular stage of resume, for dev.
(4) dev->power.comp is completed for each device after running its
    resume callbacks.

With this mechanism in place, the drivers wanting their resume
callbacks to be executed asynchronously can set
dev->power.async_suspend for them, with the help of
device_enable_async_suspend().  In addition to that, the PM
dependencies between devices have to be represented by
'struct pm_link' objects introduced by the previous patch.

---
 drivers/base/power/common.c |    5 +
 drivers/base/power/main.c   |  124 ++++++++++++++++++++++++++++++++++++++++----
 include/linux/device.h      |    6 ++
 include/linux/pm.h          |    4 +
 4 files changed, 129 insertions(+), 10 deletions(-)

Index: linux-2.6/include/linux/pm.h
===================================================================
--- linux-2.6.orig/include/linux/pm.h
+++ linux-2.6/include/linux/pm.h
@@ -26,6 +26,7 @@
 #include <linux/spinlock.h>
 #include <linux/wait.h>
 #include <linux/timer.h>
+#include <linux/completion.h>
 
 /*
  * Callbacks for platform drivers to implement.
@@ -414,9 +415,12 @@ struct dev_pm_info {
 	pm_message_t		power_state;
 	unsigned int		can_wakeup:1;
 	unsigned int		should_wakeup:1;
+	unsigned int		async_suspend:1;
 	enum dpm_state		status;		/* Owned by the PM core */
 #ifdef CONFIG_PM_SLEEP
 	struct list_head	entry;
+	struct completion	comp;
+	pm_message_t		async_state;
 #endif
 #ifdef CONFIG_PM_RUNTIME
 	struct timer_list	suspend_timer;
Index: linux-2.6/include/linux/device.h
===================================================================
--- linux-2.6.orig/include/linux/device.h
+++ linux-2.6/include/linux/device.h
@@ -472,6 +472,12 @@ static inline int device_is_registered(s
 	return dev->kobj.state_in_sysfs;
 }
 
+static inline void device_enable_async_suspend(struct device *dev, bool enable)
+{
+	if (dev->power.status == DPM_ON)
+		dev->power.async_suspend = enable;
+}
+
 void driver_init(void);
 
 /*
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
@@ -26,6 +26,8 @@
 #include <linux/resume-trace.h>
 #include <linux/rwsem.h>
 #include <linux/interrupt.h>
+#include <linux/async.h>
+#include <linux/completion.h>
 
 #include "../base.h"
 #include "power.h"
@@ -50,6 +52,11 @@ static DEFINE_MUTEX(dpm_list_mtx);
  */
 static bool transition_started;
 
+void device_pm_sleep_init(struct device *dev)
+{
+	init_completion(&dev->power.comp);
+}
+
 /**
  * device_pm_lock - Lock the list of active devices used by the PM core.
  */
@@ -144,6 +151,50 @@ void device_pm_move_last(struct device *
 	list_move_tail(&dev->power.entry, &dpm_list);
 }
 
+static void dpm_synchronize_noirq(void)
+{
+	struct device *dev;
+
+	async_synchronize_full();
+
+	list_for_each_entry(dev, &dpm_list, power.entry)
+		INIT_COMPLETION(dev->power.comp);
+}
+
+static void dpm_synchronize(void)
+{
+	struct device *dev;
+
+	async_synchronize_full();
+
+	mutex_lock(&dpm_list_mtx);
+	list_for_each_entry(dev, &dpm_list, power.entry)
+		INIT_COMPLETION(dev->power.comp);
+	mutex_unlock(&dpm_list_mtx);
+}
+
+static void device_pm_wait(struct device *sub, struct device *dev)
+{
+	if (sub->power.async_suspend || (dev && dev->power.async_suspend)) {
+		dev_dbg(sub, "PM: Waiting for %s %s\n", dev_driver_string(dev),
+			dev_name(dev));
+		wait_for_completion(&dev->power.comp);
+	}
+}
+
+static int device_pm_wait_fn(struct device *dev, void *data)
+{
+	struct device *sub = (struct device *)data;
+
+	device_pm_wait(sub, dev);
+	return 0;
+}
+
+static void device_pm_wait_for_masters(struct device *slave)
+{
+	device_for_each_master(slave, slave, device_pm_wait_fn);
+}
+
 /**
  * pm_op - Execute the PM operation appropiate for given PM event.
  * @dev: Device to handle.
@@ -310,32 +361,57 @@ static void pm_dev_err(struct device *de
 /*------------------------- Resume routines -------------------------*/
 
 /**
- * device_resume_noirq - Execute an "early resume" callback for given device.
+ * __device_resume_noirq - Execute an "early resume" callback for given device.
  * @dev: Device to handle.
  * @state: PM transition of the system being carried out.
  *
  * The driver of @dev will not receive interrupts while this fuction is being
  * executed.
  */
-static int device_resume_noirq(struct device *dev, pm_message_t state)
+static int __device_resume_noirq(struct device *dev, pm_message_t state)
 {
 	int error = 0;
 
 	TRACE_DEVICE(dev);
 	TRACE_RESUME(0);
 
-	if (!dev->bus)
-		goto End;
+	device_pm_wait_for_masters(dev);
 
-	if (dev->bus->pm) {
+	if (dev->bus && dev->bus->pm) {
 		pm_dev_dbg(dev, state, "EARLY ");
 		error = pm_noirq_op(dev, dev->bus->pm, state);
 	}
- End:
+
+	complete_all(&dev->power.comp);
+
 	TRACE_RESUME(error);
 	return error;
 }
 
+static void async_resume_noirq(void *data, async_cookie_t cookie)
+{
+	struct device *dev = (struct device *)data;
+	int error;
+
+	pm_dev_dbg(dev, dev->power.async_state, "async EARLY ");
+	error = __device_resume_noirq(dev, dev->power.async_state);
+	if (error)
+		pm_dev_err(dev, dev->power.async_state, " async EARLY", error);
+	put_device(dev);
+}
+
+static int device_resume_noirq(struct device *dev, pm_message_t state)
+{
+	if (dev->power.async_suspend) {
+		get_device(dev);
+		dev->power.async_state = state;
+		async_schedule(async_resume_noirq, dev);
+		return 0;
+	}
+
+	return __device_resume_noirq(dev, state);
+}
+
 /**
  * dpm_resume_noirq - Execute "early resume" callbacks for non-sysdev devices.
  * @state: PM transition of the system being carried out.
@@ -357,23 +433,25 @@ void dpm_resume_noirq(pm_message_t state
 			if (error)
 				pm_dev_err(dev, state, " early", error);
 		}
+	dpm_synchronize_noirq();
 	mutex_unlock(&dpm_list_mtx);
 	resume_device_irqs();
 }
 EXPORT_SYMBOL_GPL(dpm_resume_noirq);
 
 /**
- * device_resume - Execute "resume" callbacks for given device.
+ * __device_resume - Execute "resume" callbacks for given device.
  * @dev: Device to handle.
  * @state: PM transition of the system being carried out.
  */
-static int device_resume(struct device *dev, pm_message_t state)
+static int __device_resume(struct device *dev, pm_message_t state)
 {
 	int error = 0;
 
 	TRACE_DEVICE(dev);
 	TRACE_RESUME(0);
 
+	device_pm_wait_for_masters(dev);
 	down(&dev->sem);
 
 	if (dev->bus) {
@@ -408,11 +486,36 @@ static int device_resume(struct device *
 	}
  End:
 	up(&dev->sem);
+	complete_all(&dev->power.comp);
 
 	TRACE_RESUME(error);
 	return error;
 }
 
+static void async_resume(void *data, async_cookie_t cookie)
+{
+	struct device *dev = (struct device *)data;
+	int error;
+
+	pm_dev_dbg(dev, dev->power.async_state, "async ");
+	error = __device_resume(dev, dev->power.async_state);
+	if (error)
+		pm_dev_err(dev, dev->power.async_state, " async", error);
+	put_device(dev);
+}
+
+static int device_resume(struct device *dev, pm_message_t state)
+{
+	if (dev->power.async_suspend) {
+		get_device(dev);
+		dev->power.async_state = state;
+		async_schedule(async_resume, dev);
+		return 0;
+	}
+
+	return __device_resume(dev, state);
+}
+
 /**
  * dpm_resume - Execute "resume" callbacks for non-sysdev devices.
  * @state: PM transition of the system being carried out.
@@ -452,6 +555,7 @@ static void dpm_resume(pm_message_t stat
 	}
 	list_splice(&list, &dpm_list);
 	mutex_unlock(&dpm_list_mtx);
+	dpm_synchronize();
 }
 
 /**
@@ -775,8 +879,10 @@ static int dpm_prepare(pm_message_t stat
 			break;
 		}
 		dev->power.status = DPM_SUSPENDING;
-		if (!list_empty(&dev->power.entry))
+		if (!list_empty(&dev->power.entry)) {
 			list_move_tail(&dev->power.entry, &list);
+			INIT_COMPLETION(dev->power.comp);
+		}
 		put_device(dev);
 	}
 	list_splice(&list, &dpm_list);
Index: linux-2.6/drivers/base/power/common.c
===================================================================
--- linux-2.6.orig/drivers/base/power/common.c
+++ linux-2.6/drivers/base/power/common.c
@@ -19,10 +19,11 @@
  */
 void device_pm_init(struct device *dev)
 {
-	dev->power.status = DPM_ON;
 	spin_lock_init(&dev->power.lock);
 	INIT_LIST_HEAD(&dev->power.master_links);
 	INIT_LIST_HEAD(&dev->power.slave_links);
+	dev->power.status = DPM_ON;
+	device_pm_sleep_init(dev);
 	pm_runtime_init(dev);
 }
 
@@ -82,6 +83,8 @@ int pm_link_add(struct device *slave, st
 	return 0;
 
  err_link:
+	master->power.async_suspend = false;
+	slave->power.async_suspend = false;
 	error = -ENOMEM;
 	put_device(slave);
 

[RFC][PATCH 3/7 updated] PM: Asynchronous resume of I/O devices

[Rafael J. Wysocki]

On Monday 17 August 2009, Rafael J. Wysocki wrote:
> Theoretically, the total time of system sleep transitions (suspend
> to RAM, hibernation) can be reduced by running suspend and resume
> callbacks of device drivers in parallel with each other.  However,
> there are dependencies between devices such that, for example, we may
> not be allowed to put one device into a low power state before
> anohter one has been suspended (e.g. we cannot suspend a bridge
> before suspending all devices behind it).  In particular, we're not
> allowed to suspend the parent of a device before suspending the
> device itself.  Analogously, we're not allowed to resume a device
> before resuming its parent.
> 
> Thus, to make it possible to execute suspend and resume callbacks
> provided by device drivers in parallel with each other, we need to
> provide a synchronization mechanism preventing the dependencies
> between devices from being violated.
> 
> The patch below introduces a mechanism allowing some devices to be
> resumed asynchronously, using completions with the following rules:
> (1) There is a completion, dev->power.comp, for each device object.
> (2) All of these completions are reset before suspend as well as
>     each resume stage (dpm_resume_noirq(), dpm_resume()).
> (3) If dev->power.async_suspend is set for dev or for one of devices
>     it depends on, the PM core waits for the "master" device's
>     completion before attempting to run the resume callbacks,
>     appropriate for this particular stage of resume, for dev.
> (4) dev->power.comp is completed for each device after running its
>     resume callbacks.

I have an update here as well.  The patch below doesn't use completions
any more, but it uses the wait_queue that we already have in struct device for
runtime PM.  It also tries to do more things in parallel.

Thanks,
Rafael

---
Theoretically, the total time of system sleep transitions (suspend
to RAM, hibernation) can be reduced by running suspend and resume
callbacks of device drivers in parallel with each other.  However,
there are dependencies between devices such that, for example, we may
not be allowed to put one device into a low power state before
anohter one has been suspended (e.g. we cannot suspend a bridge
before suspending all devices behind it).  In particular, we're not
allowed to suspend the parent of a device before suspending the
device itself.  Analogously, we're not allowed to resume a device
before resuming its parent.

Thus, to make it possible to execute suspend and resume callbacks
provided by device drivers in parallel with each other, we need to
provide a synchronization mechanism preventing the dependencies
between devices from being violated.

The patch below introduces a mechanism allowing some devices to be
resumed asynchronously, with the following rules:
(1) There is a wait queue head, dev->power.wait_queue, and an
    "operation complete" flag, dev->power.op_complete for each device
    object.
(2) All of the power.op_complete flags are reset before suspend as
    well as after each resume stage (dpm_resume_noirq(),
    dpm_resume()).
(3) If power.async_suspend is set for dev or for one of devices it
    depends on, the PM core waits for the "master" device's
    power.op_complete flag to be set before attempting to run the
    resume callbacks, appropriate for this particular stage of
    resume, for dev.
(4) dev->power.op_complete is set for each device after running its
    resume callbacks at each stage of resume (dpm_resume_noirq(),
    dpm_resume()).

With this mechanism in place, the drivers wanting their resume
callbacks to be executed asynchronously can set
dev->power.async_suspend for them, with the help of
device_enable_async_suspend().  In addition to that, the PM off-tree
dependencies between devices have to be represented by
'struct pm_link' objects introduced by the previous patch.

In this version of the patch the async threads started to execute
the resume callbacks of specific device don't exit immediately having
done that, but search dpm_list for devices whose PM dependencies have
already been satisfied and execute their callbacks without waiting.

---
 drivers/base/power/common.c |    5 
 drivers/base/power/main.c   |  226 +++++++++++++++++++++++++++++++++++++++++---
 include/linux/device.h      |    6 +
 include/linux/pm.h          |    6 -
 4 files changed, 228 insertions(+), 15 deletions(-)

Index: linux-2.6/include/linux/pm.h
===================================================================
--- linux-2.6.orig/include/linux/pm.h
+++ linux-2.6/include/linux/pm.h
@@ -26,6 +26,7 @@
 #include <linux/spinlock.h>
 #include <linux/wait.h>
 #include <linux/timer.h>
+#include <linux/completion.h>
 
 /*
  * Callbacks for platform drivers to implement.
@@ -409,20 +410,23 @@ enum rpm_request {
 
 struct dev_pm_info {
 	spinlock_t		lock;
+	wait_queue_head_t	wait_queue;
 	struct list_head	master_links;
 	struct list_head	slave_links;
 	pm_message_t		power_state;
 	unsigned int		can_wakeup:1;
 	unsigned int		should_wakeup:1;
+	unsigned int		async_suspend:1;
 	enum dpm_state		status;		/* Owned by the PM core */
 #ifdef CONFIG_PM_SLEEP
 	struct list_head	entry;
+	unsigned int		op_started:1;
+	unsigned int		op_complete:1;
 #endif
 #ifdef CONFIG_PM_RUNTIME
 	struct timer_list	suspend_timer;
 	unsigned long		timer_expires;
 	struct work_struct	work;
-	wait_queue_head_t	wait_queue;
 	atomic_t		usage_count;
 	atomic_t		child_count;
 	unsigned int		disable_depth:3;
Index: linux-2.6/include/linux/device.h
===================================================================
--- linux-2.6.orig/include/linux/device.h
+++ linux-2.6/include/linux/device.h
@@ -472,6 +472,12 @@ static inline int device_is_registered(s
 	return dev->kobj.state_in_sysfs;
 }
 
+static inline void device_enable_async_suspend(struct device *dev, bool enable)
+{
+	if (dev->power.status == DPM_ON)
+		dev->power.async_suspend = enable;
+}
+
 void driver_init(void);
 
 /*
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
@@ -26,6 +26,8 @@
 #include <linux/resume-trace.h>
 #include <linux/rwsem.h>
 #include <linux/interrupt.h>
+#include <linux/async.h>
+#include <linux/completion.h>
 
 #include "../base.h"
 #include "power.h"
@@ -43,6 +45,7 @@
 LIST_HEAD(dpm_list);
 
 static DEFINE_MUTEX(dpm_list_mtx);
+static pm_message_t pm_transition;
 
 /*
  * Set once the preparation of devices for a PM transition has started, reset
@@ -144,6 +147,81 @@ void device_pm_move_last(struct device *
 	list_move_tail(&dev->power.entry, &dpm_list);
 }
 
+static void dpm_reset(struct device *dev)
+{
+	dev->power.op_started = false;
+	dev->power.op_complete = false;
+}
+
+static void dpm_reset_all(void)
+{
+	struct device *dev;
+
+	list_for_each_entry(dev, &dpm_list, power.entry)
+		dpm_reset(dev);
+}
+
+static void dpm_synchronize_noirq(void)
+{
+	async_synchronize_full();
+	dpm_reset_all();
+}
+
+static void dpm_synchronize(void)
+{
+	async_synchronize_full();
+	mutex_lock(&dpm_list_mtx);
+	dpm_reset_all();
+	mutex_unlock(&dpm_list_mtx);
+}
+
+static void device_pm_wait(struct device *sub, struct device *dev)
+{
+	if (!dev)
+		return;
+
+	if (!(sub->power.async_suspend || dev->power.async_suspend))
+		return;
+
+	if (!dev->power.op_complete) {
+		dev_dbg(sub, "PM: Waiting for %s %s\n", dev_driver_string(dev),
+			dev_name(dev));
+		wait_event(dev->power.wait_queue, !!dev->power.op_complete);
+	}
+}
+
+static int device_pm_wait_fn(struct device *dev, void *data)
+{
+	device_pm_wait((struct device *)data, dev);
+	return 0;
+}
+
+static void device_pm_wait_for_masters(struct device *slave)
+{
+	if (!pm_trace_enabled)
+		device_for_each_master(slave, slave, device_pm_wait_fn);
+}
+
+static bool device_pm_check(struct device *dev)
+{
+	int ret = 0;
+
+	if (dev)
+		ret = !dev->power.op_complete;
+
+	return ret;
+}
+
+static int device_pm_check_fn(struct device *dev, void *data)
+{
+	return device_pm_check(dev);
+}
+
+static int device_pm_check_masters(struct device *slave)
+{
+	return device_for_each_master(slave, NULL, device_pm_check_fn);
+}
+
 /**
  * pm_op - Execute the PM operation appropriate for given PM event.
  * @dev: Device to handle.
@@ -269,6 +347,20 @@ static int pm_noirq_op(struct device *de
 	return error;
 }
 
+static bool pm_op_started(struct device *dev)
+{
+	bool ret = false;
+
+	spin_lock_irq(&dev->power.lock);
+	if (dev->power.op_started)
+		ret = true;
+	else
+		dev->power.op_started = true;
+	spin_unlock_irq(&dev->power.lock);
+
+	return ret;
+}
+
 static char *pm_verb(int event)
 {
 	switch (event) {
@@ -310,32 +402,78 @@ static void pm_dev_err(struct device *de
 /*------------------------- Resume routines -------------------------*/
 
 /**
- * device_resume_noirq - Execute an "early resume" callback for given device.
+ * __device_resume_noirq - Execute an "early resume" callback for given device.
  * @dev: Device to handle.
  * @state: PM transition of the system being carried out.
  *
  * The driver of @dev will not receive interrupts while this function is being
  * executed.
  */
-static int device_resume_noirq(struct device *dev, pm_message_t state)
+static int __device_resume_noirq(struct device *dev, pm_message_t state)
 {
 	int error = 0;
 
 	TRACE_DEVICE(dev);
 	TRACE_RESUME(0);
 
-	if (!dev->bus)
-		goto End;
-
-	if (dev->bus->pm) {
+	if (dev->bus && dev->bus->pm) {
 		pm_dev_dbg(dev, state, "EARLY ");
 		error = pm_noirq_op(dev, dev->bus->pm, state);
 	}
- End:
+
+	dev->power.op_complete = true;
+	wake_up_all(&dev->power.wait_queue);
+
 	TRACE_RESUME(error);
 	return error;
 }
 
+static void async_device_resume_noirq(struct device *dev)
+{
+	int error;
+
+	pm_dev_dbg(dev, pm_transition, "async EARLY ");
+	error = __device_resume_noirq(dev, pm_transition);
+	if (error)
+		pm_dev_err(dev, pm_transition, " async EARLY", error);
+}
+
+static void async_resume_noirq(void *data, async_cookie_t cookie)
+{
+	struct device *dev = (struct device *)data;
+
+	device_pm_wait_for_masters(dev);
+	async_device_resume_noirq(dev);
+
+	list_for_each_entry_continue(dev, &dpm_list, power.entry) {
+		if (!dev->power.async_suspend || dev->power.status <= DPM_OFF)
+			continue;
+
+		if (device_pm_check_masters(dev))
+			continue;
+
+		if (pm_op_started(dev))
+			continue;
+
+		pm_dev_dbg(dev, pm_transition, "out of order EARLY ");
+		async_device_resume_noirq(dev);
+	}
+}
+
+static int device_resume_noirq(struct device *dev)
+{
+	if (pm_op_started(dev))
+		return 0;
+
+	if (dev->power.async_suspend && !pm_trace_enabled) {
+		async_schedule(async_resume_noirq, dev);
+		return 0;
+	}
+
+	device_pm_wait_for_masters(dev);
+	return __device_resume_noirq(dev, pm_transition);
+}
+
 /**
  * dpm_resume_noirq - Execute "early resume" callbacks for non-sysdev devices.
  * @state: PM transition of the system being carried out.
@@ -349,26 +487,28 @@ void dpm_resume_noirq(pm_message_t state
 
 	mutex_lock(&dpm_list_mtx);
 	transition_started = false;
+	pm_transition = state;
 	list_for_each_entry(dev, &dpm_list, power.entry)
 		if (dev->power.status > DPM_OFF) {
 			int error;
 
 			dev->power.status = DPM_OFF;
-			error = device_resume_noirq(dev, state);
+			error = device_resume_noirq(dev);
 			if (error)
-				pm_dev_err(dev, state, " early", error);
+				pm_dev_err(dev, state, " EARLY", error);
 		}
+	dpm_synchronize_noirq();
 	mutex_unlock(&dpm_list_mtx);
 	resume_device_irqs();
 }
 EXPORT_SYMBOL_GPL(dpm_resume_noirq);
 
 /**
- * device_resume - Execute "resume" callbacks for given device.
+ * __device_resume - Execute "resume" callbacks for given device.
  * @dev: Device to handle.
  * @state: PM transition of the system being carried out.
  */
-static int device_resume(struct device *dev, pm_message_t state)
+static int __device_resume(struct device *dev, pm_message_t state)
 {
 	int error = 0;
 
@@ -409,11 +549,67 @@ static int device_resume(struct device *
 	}
  End:
 	up(&dev->sem);
+	dev->power.op_complete = true;
+	wake_up_all(&dev->power.wait_queue);
 
 	TRACE_RESUME(error);
 	return error;
 }
 
+static void async_device_resume(struct device *dev)
+{
+	int error;
+
+	pm_dev_dbg(dev, pm_transition, "async ");
+	error = __device_resume(dev, pm_transition);
+	if (error)
+		pm_dev_err(dev, pm_transition, " async", error);
+}
+
+static void async_resume(void *data, async_cookie_t cookie)
+{
+	struct device *dev = (struct device *)data;
+
+	device_pm_wait_for_masters(dev);
+
+ repeat:
+	async_device_resume(dev);
+	put_device(dev);
+
+	mutex_lock(&dpm_list_mtx);
+	list_for_each_entry(dev, &dpm_list, power.entry) {
+		if (!dev->power.async_suspend || dev->power.status < DPM_OFF)
+			continue;
+
+		if (device_pm_check_masters(dev))
+			continue;
+
+		if (pm_op_started(dev))
+			continue;
+
+		get_device(dev);
+		mutex_unlock(&dpm_list_mtx);
+		pm_dev_dbg(dev, pm_transition, "out of order ");
+		goto repeat;
+	}
+	mutex_unlock(&dpm_list_mtx);
+}
+
+static int device_resume(struct device *dev)
+{
+	if (pm_op_started(dev))
+		return 0;
+
+	if (dev->power.async_suspend && !pm_trace_enabled) {
+		get_device(dev);
+		async_schedule(async_resume, dev);
+		return 0;
+	}
+
+	device_pm_wait_for_masters(dev);
+	return __device_resume(dev, pm_transition);
+}
+
 /**
  * dpm_resume - Execute "resume" callbacks for non-sysdev devices.
  * @state: PM transition of the system being carried out.
@@ -427,6 +623,7 @@ static void dpm_resume(pm_message_t stat
 
 	INIT_LIST_HEAD(&list);
 	mutex_lock(&dpm_list_mtx);
+	pm_transition = state;
 	while (!list_empty(&dpm_list)) {
 		struct device *dev = to_device(dpm_list.next);
 
@@ -437,7 +634,7 @@ static void dpm_resume(pm_message_t stat
 			dev->power.status = DPM_RESUMING;
 			mutex_unlock(&dpm_list_mtx);
 
-			error = device_resume(dev, state);
+			error = device_resume(dev);
 
 			mutex_lock(&dpm_list_mtx);
 			if (error)
@@ -452,6 +649,7 @@ static void dpm_resume(pm_message_t stat
 	}
 	list_splice(&list, &dpm_list);
 	mutex_unlock(&dpm_list_mtx);
+	dpm_synchronize();
 }
 
 /**
@@ -775,8 +973,10 @@ static int dpm_prepare(pm_message_t stat
 			break;
 		}
 		dev->power.status = DPM_SUSPENDING;
-		if (!list_empty(&dev->power.entry))
+		if (!list_empty(&dev->power.entry)) {
 			list_move_tail(&dev->power.entry, &list);
+			dpm_reset(dev);
+		}
 		put_device(dev);
 	}
 	list_splice(&list, &dpm_list);
Index: linux-2.6/drivers/base/power/common.c
===================================================================
--- linux-2.6.orig/drivers/base/power/common.c
+++ linux-2.6/drivers/base/power/common.c
@@ -19,10 +19,11 @@
  */
 void device_pm_init(struct device *dev)
 {
-	dev->power.status = DPM_ON;
 	spin_lock_init(&dev->power.lock);
+	init_waitqueue_head(&dev->power.wait_queue);
 	INIT_LIST_HEAD(&dev->power.master_links);
 	INIT_LIST_HEAD(&dev->power.slave_links);
+	dev->power.status = DPM_ON;
 	pm_runtime_init(dev);
 }
 
@@ -81,6 +82,8 @@ int pm_link_add(struct device *slave, st
 	return 0;
 
  err_link:
+	master->power.async_suspend = false;
+	slave->power.async_suspend = false;
 	error = -ENOMEM;
 	put_device(slave);
 

[RFC][PATCH 4/7] PM: Asynchronous suspend of I/O devices

[Rafael J. Wysocki]

The patch below extends the mechanism introduced by the previous
patch to the suspend part of the PM core.

Asynchronous suspend is slightly more complicated, because if any of
the suspend callbacks executed asynchronously returns error code, the
entire suspend has to be terminated and rolled back.
---
 drivers/base/power/main.c |   99 ++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 92 insertions(+), 7 deletions(-)

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
@@ -195,6 +195,11 @@ static void device_pm_wait_for_masters(s
 	device_for_each_master(slave, slave, device_pm_wait_fn);
 }
 
+static void device_pm_wait_for_slaves(struct device *master)
+{
+	device_for_each_slave(master, master, device_pm_wait_fn);
+}
+
 /**
  * pm_op - Execute the PM operation appropiate for given PM event.
  * @dev: Device to handle.
@@ -637,6 +642,8 @@ EXPORT_SYMBOL_GPL(dpm_resume_end);
 
 /*------------------------- Suspend routines -------------------------*/
 
+static atomic_t async_error;
+
 /**
  * resume_event - Return a "resume" message for given "suspend" sleep state.
  * @sleep_state: PM message representing a sleep state.
@@ -666,20 +673,52 @@ static pm_message_t resume_event(pm_mess
  * The driver of @dev will not receive interrupts while this fuction is being
  * executed.
  */
-static int device_suspend_noirq(struct device *dev, pm_message_t state)
+static int __device_suspend_noirq(struct device *dev, pm_message_t state)
 {
 	int error = 0;
 
-	if (!dev->bus)
-		return 0;
+	device_pm_wait_for_slaves(dev);
 
-	if (dev->bus->pm) {
+	if (dev->bus && dev->bus->pm) {
 		pm_dev_dbg(dev, state, "LATE ");
 		error = pm_noirq_op(dev, dev->bus->pm, state);
 	}
+
+	complete_all(&dev->power.comp);
+
 	return error;
 }
 
+static void async_suspend_noirq(void *data, async_cookie_t cookie)
+{
+	struct device *dev = (struct device *)data;
+	int error = atomic_read(&async_error);
+
+	if (error)
+		return;
+
+	pm_dev_dbg(dev, dev->power.async_state, "async LATE ");
+	error = __device_suspend_noirq(dev, dev->power.async_state);
+	if (error) {
+		pm_dev_err(dev, dev->power.async_state, " async LATE", error);
+		dev->power.status = DPM_OFF;
+		atomic_set(&async_error, error);
+	}
+	put_device(dev);
+}
+
+static int device_suspend_noirq(struct device *dev, pm_message_t state)
+{
+	if (dev->power.async_suspend) {
+		get_device(dev);
+		dev->power.async_state = state;
+		async_schedule(async_suspend_noirq, dev);
+		return 0;
+	}
+
+	return __device_suspend_noirq(dev, state);
+}
+
 /**
  * dpm_suspend_noirq - Execute "late suspend" callbacks for non-sysdev devices.
  * @state: PM transition of the system being carried out.
@@ -695,13 +734,18 @@ int dpm_suspend_noirq(pm_message_t state
 	suspend_device_irqs();
 	mutex_lock(&dpm_list_mtx);
 	list_for_each_entry_reverse(dev, &dpm_list, power.entry) {
+		dev->power.status = DPM_OFF_IRQ;
 		error = device_suspend_noirq(dev, state);
 		if (error) {
 			pm_dev_err(dev, state, " late", error);
+			dev->power.status = DPM_OFF;
 			break;
 		}
-		dev->power.status = DPM_OFF_IRQ;
+		error = atomic_read(&async_error);
+		if (error)
+			break;
 	}
+	dpm_synchronize_noirq();
 	mutex_unlock(&dpm_list_mtx);
 	if (error)
 		dpm_resume_noirq(resume_event(state));
@@ -714,10 +758,11 @@ EXPORT_SYMBOL_GPL(dpm_suspend_noirq);
  * @dev: Device to handle.
  * @state: PM transition of the system being carried out.
  */
-static int device_suspend(struct device *dev, pm_message_t state)
+static int __device_suspend(struct device *dev, pm_message_t state)
 {
 	int error = 0;
 
+	device_pm_wait_for_slaves(dev);
 	down(&dev->sem);
 
 	if (dev->class) {
@@ -754,10 +799,44 @@ static int device_suspend(struct device 
 	}
  End:
 	up(&dev->sem);
+	complete_all(&dev->power.comp);
 
 	return error;
 }
 
+static void async_suspend(void *data, async_cookie_t cookie)
+{
+	struct device *dev = (struct device *)data;
+	int error = atomic_read(&async_error);
+
+	if (error)
+		return;
+
+	pm_dev_dbg(dev, dev->power.async_state, "async ");
+
+	error = __device_suspend(dev, dev->power.async_state);
+	if (error) {
+		pm_dev_err(dev, dev->power.async_state, " async", error);
+		mutex_lock(&dpm_list_mtx);
+		dev->power.status = DPM_SUSPENDING;
+		mutex_unlock(&dpm_list_mtx);
+		atomic_set(&async_error, error);
+	}
+	put_device(dev);
+}
+
+static int device_suspend(struct device *dev, pm_message_t state)
+{
+	if (dev->power.async_suspend) {
+		get_device(dev);
+		dev->power.async_state = state;
+		async_schedule(async_suspend, dev);
+		return 0;
+	}
+
+	return __device_suspend(dev, state);
+}
+
 /**
  * dpm_suspend - Execute "suspend" callbacks for all non-sysdev devices.
  * @state: PM transition of the system being carried out.
@@ -773,6 +852,7 @@ static int dpm_suspend(pm_message_t stat
 		struct device *dev = to_device(dpm_list.prev);
 
 		get_device(dev);
+		dev->power.status = DPM_OFF;
 		mutex_unlock(&dpm_list_mtx);
 
 		error = device_suspend(dev, state);
@@ -780,16 +860,20 @@ static int dpm_suspend(pm_message_t stat
 		mutex_lock(&dpm_list_mtx);
 		if (error) {
 			pm_dev_err(dev, state, "", error);
+			dev->power.status = DPM_SUSPENDING;
 			put_device(dev);
 			break;
 		}
-		dev->power.status = DPM_OFF;
 		if (!list_empty(&dev->power.entry))
 			list_move(&dev->power.entry, &list);
 		put_device(dev);
+		error = atomic_read(&async_error);
+		if (error)
+			break;
 	}
 	list_splice(&list, dpm_list.prev);
 	mutex_unlock(&dpm_list_mtx);
+	dpm_synchronize();
 	return error;
 }
 
@@ -848,6 +932,7 @@ static int dpm_prepare(pm_message_t stat
 	INIT_LIST_HEAD(&list);
 	mutex_lock(&dpm_list_mtx);
 	transition_started = true;
+	atomic_set(&async_error, 0);
 	while (!list_empty(&dpm_list)) {
 		struct device *dev = to_device(dpm_list.next);
 

[RFC][PATCH 5/7] PM: Asynchronous suspend and resume of PCI devices

[Rafael J. Wysocki]

Set async_suspend for all PCI devices and PCIe port services.

---
 drivers/input/serio/i8042.c     |    2 ++
 drivers/pci/pci.c               |    2 ++
 drivers/pci/pcie/portdrv_core.c |    1 +
 3 files changed, 5 insertions(+)

Index: linux-2.6/drivers/pci/pci.c
===================================================================
--- linux-2.6.orig/drivers/pci/pci.c
+++ linux-2.6/drivers/pci/pci.c
@@ -1409,6 +1409,8 @@ void pci_pm_init(struct pci_dev *dev)
 	} else {
 		dev->pme_support = 0;
 	}
+
+	device_enable_async_suspend(&dev->dev, true);
 }
 
 /**
Index: linux-2.6/drivers/pci/pcie/portdrv_core.c
===================================================================
--- linux-2.6.orig/drivers/pci/pcie/portdrv_core.c
+++ linux-2.6/drivers/pci/pcie/portdrv_core.c
@@ -280,6 +280,7 @@ static void pcie_device_init(struct pci_
 	dev_set_name(device, "%s:pcie%02x",
 		 pci_name(parent), get_descriptor_id(port_type, service_type));
 	device->parent = &parent->dev;
+	device_enable_async_suspend(device, true);
 }
 
 /**

Re: [RFC][PATCH 5/7] PM: Asynchronous suspend and resume of PCI devices

[Zhang Rui]

On Mon, 2009-08-17 at 08:20 +0800, Rafael J. Wysocki wrote:
> Set async_suspend for all PCI devices and PCIe port services.
> 
Alan said that we can not break the resume order of the uhci/ehci host
controllers on some platforms.

For example,
00:1a.0 USB Controller: Intel Corporation 82801H (ICH8 Family) USB UHCI
Controller #4 (rev 02)
00:1a.1 USB Controller: Intel Corporation 82801H (ICH8 Family) USB UHCI
Controller #5 (rev 02)
00:1a.7 USB Controller: Intel Corporation 82801H (ICH8 Family) USB2 EHCI
Controller #2 (rev 02)
00:1a.7 must be resumed after 00:1a.0 and 00:1a.1

please refer to this thread:
http://marc.info/?l=linux-acpi&m=122996117918188&w=2

So I'm afraid we can not suspend/resume the PCI devices in parallel,
unless we add this off-tree dependency at the same time.

thanks,
rui
> ---
>  drivers/input/serio/i8042.c     |    2 ++
>  drivers/pci/pci.c               |    2 ++
>  drivers/pci/pcie/portdrv_core.c |    1 +
>  3 files changed, 5 insertions(+)
> 
> Index: linux-2.6/drivers/pci/pci.c
> ===================================================================
> --- linux-2.6.orig/drivers/pci/pci.c
> +++ linux-2.6/drivers/pci/pci.c
> @@ -1409,6 +1409,8 @@ void pci_pm_init(struct pci_dev *dev)
>  	} else {
>  		dev->pme_support = 0;
>  	}
> +
> +	device_enable_async_suspend(&dev->dev, true);
>  }
>  
>  /**
> Index: linux-2.6/drivers/pci/pcie/portdrv_core.c
> ===================================================================
> --- linux-2.6.orig/drivers/pci/pcie/portdrv_core.c
> +++ linux-2.6/drivers/pci/pcie/portdrv_core.c
> @@ -280,6 +280,7 @@ static void pcie_device_init(struct pci_
>  	dev_set_name(device, "%s:pcie%02x",
>  		 pci_name(parent), get_descriptor_id(port_type, service_type));
>  	device->parent = &parent->dev;
> +	device_enable_async_suspend(device, true);
>  }
>  
>  /**

Re: [RFC][PATCH 5/7] PM: Asynchronous suspend and resume of PCI devices

[Alan Stern]

On Tue, 18 Aug 2009, Zhang Rui wrote:

> On Mon, 2009-08-17 at 08:20 +0800, Rafael J. Wysocki wrote:
> > Set async_suspend for all PCI devices and PCIe port services.
> > 
> Alan said that we can not break the resume order of the uhci/ehci host
> controllers on some platforms.
> 
> For example,
> 00:1a.0 USB Controller: Intel Corporation 82801H (ICH8 Family) USB UHCI
> Controller #4 (rev 02)
> 00:1a.1 USB Controller: Intel Corporation 82801H (ICH8 Family) USB UHCI
> Controller #5 (rev 02)
> 00:1a.7 USB Controller: Intel Corporation 82801H (ICH8 Family) USB2 EHCI
> Controller #2 (rev 02)
> 00:1a.7 must be resumed after 00:1a.0 and 00:1a.1
> 
> please refer to this thread:
> http://marc.info/?l=linux-acpi&m=122996117918188&w=2
> 
> So I'm afraid we can not suspend/resume the PCI devices in parallel,
> unless we add this off-tree dependency at the same time.

It's okay for testing.  The dependency is needed mostly for 
resume-from-hibernation.

Alan Stern

[RFC][PATCH 6/7] PM: Asynchronous suspend and resume of ACPI devices

[Rafael J. Wysocki]

Set async_suspend for all ACPI devices.

---
 drivers/acpi/scan.c |    1 +
 1 file changed, 1 insertion(+)

Index: linux-2.6/drivers/acpi/scan.c
===================================================================
--- linux-2.6.orig/drivers/acpi/scan.c
+++ linux-2.6/drivers/acpi/scan.c
@@ -539,6 +539,7 @@ static int acpi_device_register(struct a
 		       dev_name(&device->dev));
 
 	device->removal_type = ACPI_BUS_REMOVAL_NORMAL;
+	device_enable_async_suspend(&device->dev, true);
 	return 0;
 end:
 	mutex_lock(&acpi_device_lock);

[RFC][PATCH 7/7] PM: Asynchronous suspend and resume of i8042

[Rafael J. Wysocki]

Set async_suspend for i8042.

---
 drivers/input/serio/i8042.c |    2 ++
 1 file changed, 2 insertions(+)

Index: linux-2.6/drivers/input/serio/i8042.c
===================================================================
--- linux-2.6.orig/drivers/input/serio/i8042.c
+++ linux-2.6/drivers/input/serio/i8042.c
@@ -1289,6 +1289,8 @@ static int __init i8042_init(void)
 	if (err)
 		goto err_free_device;
 
+	device_enable_async_suspend(&i8042_platform_device->dev, true);
+
 	panic_blink = i8042_panic_blink;
 
 	return 0;

Re: [RFC][PATCH 7/7] PM: Asynchronous suspend and resume of i8042

[Zhang Rui]

On Mon, 2009-08-17 at 08:21 +0800, Rafael J. Wysocki wrote:
> Set async_suspend for i8042.
> 
it's the psmouse reset that takes the 0.4 seconds during suspend.
so we should call device_enable_async_suspend for the psmouse serio
device in psmouse-base.c

Or invoking device_enable_async_suspend for every serio device in
serio.c, as the keyboard also takes about 0.2s to suspend.

thanks,
rui
> ---
>  drivers/input/serio/i8042.c |    2 ++
>  1 file changed, 2 insertions(+)
> 
> Index: linux-2.6/drivers/input/serio/i8042.c
> ===================================================================
> --- linux-2.6.orig/drivers/input/serio/i8042.c
> +++ linux-2.6/drivers/input/serio/i8042.c
> @@ -1289,6 +1289,8 @@ static int __init i8042_init(void)
>  	if (err)
>  		goto err_free_device;
>  
> +	device_enable_async_suspend(&i8042_platform_device->dev, true);
> +
>  	panic_blink = i8042_panic_blink;
>  
>  	return 0;

Re: [RFC][PATCH 7/7] PM: Asynchronous suspend and resume of i8042

[Rafael J. Wysocki]

On Tuesday 18 August 2009, Zhang Rui wrote:
> On Mon, 2009-08-17 at 08:21 +0800, Rafael J. Wysocki wrote:
> > Set async_suspend for i8042.
> > 
> it's the psmouse reset that takes the 0.4 seconds during suspend.
> so we should call device_enable_async_suspend for the psmouse serio
> device in psmouse-base.c
> 
> Or invoking device_enable_async_suspend for every serio device in
> serio.c, as the keyboard also takes about 0.2s to suspend.

Yes we can do that.  I'll test that later today.

Thanks,
Rafael

Re: [RFC][PATCH 7/7] PM: Asynchronous suspend and resume of i8042

[Rafael J. Wysocki]

On Tuesday 18 August 2009, Rafael J. Wysocki wrote:
> On Tuesday 18 August 2009, Zhang Rui wrote:
> > On Mon, 2009-08-17 at 08:21 +0800, Rafael J. Wysocki wrote:
> > > Set async_suspend for i8042.
> > > 
> > it's the psmouse reset that takes the 0.4 seconds during suspend.
> > so we should call device_enable_async_suspend for the psmouse serio
> > device in psmouse-base.c
> > 
> > Or invoking device_enable_async_suspend for every serio device in
> > serio.c, as the keyboard also takes about 0.2s to suspend.
> 
> Yes we can do that.  I'll test that later today.

The appended patch appears to work on my test box.

Thanks,
Rafael

---
Set async_suspend for serio input devices.

---
 drivers/input/serio/serio.c |    1 +
 1 file changed, 1 insertion(+)

Index: linux-2.6/drivers/input/serio/serio.c
===================================================================
--- linux-2.6.orig/drivers/input/serio/serio.c
+++ linux-2.6/drivers/input/serio/serio.c
@@ -576,6 +576,7 @@ static void serio_add_port(struct serio 
 			printk(KERN_ERR
 				"serio: sysfs_create_group() failed for %s (%s), error: %d\n",
 				serio->phys, serio->name, error);
+		device_enable_async_suspend(&serio->dev, true);
 	}
 }
 

Re: [RFC][PATCH 7/7] PM: Asynchronous suspend and resume of i8042

[Pavel Machek]

On Tue 2009-08-18 15:03:30, Zhang Rui wrote:
> On Mon, 2009-08-17 at 08:21 +0800, Rafael J. Wysocki wrote:
> > Set async_suspend for i8042.
> > 
> it's the psmouse reset that takes the 0.4 seconds during suspend.
> ???so we should call device_enable_async_suspend for the psmouse serio
> device in psmouse-base.c

Do we really need to reset it during suspend?
								Pavel

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

Re: [RFC][PATCH 0/7] PM: Asynchronous suspend and resume (updated)

[Zhang Rui]

hmm,
what are these patches based on?
I tried an 2.6.31-rc6 kernel together with your run-time PM patch (rev.
17) but still failed.

thanks,
rui

On Mon, 2009-08-17 at 08:15 +0800, Rafael J. Wysocki wrote:
> On Wednesday 12 August 2009, Rafael J. Wysocki wrote:
> > Hi,
> > 
> > The following patches introduce a mechanism allowing us to execute device
> > drivers' suspend and resume callbacks asynchronously during system sleep
> > transitions, such as suspend to RAM.  The idea is explained in the [1/1] patch
> > message.
> 
> Changes:
> 
> * Added [1/7] that fixes kerneldoc comments in drivers/base/power/main.c
>   (this is a 2.6.32 candidate).
> 
> * Added [2/7] adding a framework for representing PM link (idea described
>   in the patch message).
> 
> * [3/7] is the async resume patch (idea described in the patch message).
> 
> * [4/7] is the async suspend patch.
> 
> * [5/7] - [7/7] set async_suspend for devices in a few selected subsystems.
> 
> The patches have been tested on HP nx6325.
> 
> Comments welcome.
> 
> Thanks,
> Rafael
> 

Re: [RFC][PATCH 0/7] PM: Asynchronous suspend and resume (updated)

[Zhang Rui]

On Mon, 2009-08-17 at 08:15 +0800, Rafael J. Wysocki wrote:
> On Wednesday 12 August 2009, Rafael J. Wysocki wrote:
> > Hi,
> > 
> > The following patches introduce a mechanism allowing us to execute device
> > drivers' suspend and resume callbacks asynchronously during system sleep
> > transitions, such as suspend to RAM.  The idea is explained in the [1/1] patch
> > message.
> 
> Changes:
> 
> * Added [1/7] that fixes kerneldoc comments in drivers/base/power/main.c
>   (this is a 2.6.32 candidate).
> 
> * Added [2/7] adding a framework for representing PM link (idea described
>   in the patch message).
> 
> * [3/7] is the async resume patch (idea described in the patch message).
> 
> * [4/7] is the async suspend patch.
> 
> * [5/7] - [7/7] set async_suspend for devices in a few selected subsystems.
> 
> The patches have been tested on HP nx6325.
> 
I tried this patch set and it does work. :)
But unfortunately it doesn't save too much time.

I still think that the child device should inherit its parent's
async_suspend flag to do the asynchronous resume more efficiently.

or at least we should provide such an interface
in drivers/base/power/common.c, so that device can tell the device core
to inherit this flag if there is no off-tree dependency.

thanks,
rui

> Thanks,
> Rafael
> 

Re: [RFC][PATCH 0/7] PM: Asynchronous suspend and resume (updated)

[Rafael J. Wysocki]

On Tuesday 18 August 2009, Zhang Rui wrote:
> On Mon, 2009-08-17 at 08:15 +0800, Rafael J. Wysocki wrote:
> > On Wednesday 12 August 2009, Rafael J. Wysocki wrote:
> > > Hi,
> > > 
> > > The following patches introduce a mechanism allowing us to execute device
> > > drivers' suspend and resume callbacks asynchronously during system sleep
> > > transitions, such as suspend to RAM.  The idea is explained in the [1/1] patch
> > > message.
> > 
> > Changes:
> > 
> > * Added [1/7] that fixes kerneldoc comments in drivers/base/power/main.c
> >   (this is a 2.6.32 candidate).
> > 
> > * Added [2/7] adding a framework for representing PM link (idea described
> >   in the patch message).
> > 
> > * [3/7] is the async resume patch (idea described in the patch message).
> > 
> > * [4/7] is the async suspend patch.
> > 
> > * [5/7] - [7/7] set async_suspend for devices in a few selected subsystems.
> > 
> > The patches have been tested on HP nx6325.
> > 
> I tried this patch set and it does work. :)
> But unfortunately it doesn't save too much time.
> 
> I still think that the child device should inherit its parent's
> async_suspend flag to do the asynchronous resume more efficiently.
> 
> or at least we should provide such an interface
> in drivers/base/power/common.c, so that device can tell the device core
> to inherit this flag if there is no off-tree dependency.

Well, I'd prefer to identify all of the off-tree dependencies that have to be
taken into account and handle all devices asynchronously.

Thanks,
Rafael

Re: [RFC][PATCH 0/7] PM: Asynchronous suspend and resume (updated)

[Rafael J. Wysocki]

On Tuesday 18 August 2009, Rafael J. Wysocki wrote:
> On Tuesday 18 August 2009, Zhang Rui wrote:
> > On Mon, 2009-08-17 at 08:15 +0800, Rafael J. Wysocki wrote:
> > > On Wednesday 12 August 2009, Rafael J. Wysocki wrote:
> > > > Hi,
> > > > 
> > > > The following patches introduce a mechanism allowing us to execute device
> > > > drivers' suspend and resume callbacks asynchronously during system sleep
> > > > transitions, such as suspend to RAM.  The idea is explained in the [1/1] patch
> > > > message.
> > > 
> > > Changes:
> > > 
> > > * Added [1/7] that fixes kerneldoc comments in drivers/base/power/main.c
> > >   (this is a 2.6.32 candidate).
> > > 
> > > * Added [2/7] adding a framework for representing PM link (idea described
> > >   in the patch message).
> > > 
> > > * [3/7] is the async resume patch (idea described in the patch message).
> > > 
> > > * [4/7] is the async suspend patch.
> > > 
> > > * [5/7] - [7/7] set async_suspend for devices in a few selected subsystems.
> > > 
> > > The patches have been tested on HP nx6325.
> > > 
> > I tried this patch set and it does work. :)
> > But unfortunately it doesn't save too much time.
> > 
> > I still think that the child device should inherit its parent's
> > async_suspend flag to do the asynchronous resume more efficiently.
> > 
> > or at least we should provide such an interface
> > in drivers/base/power/common.c, so that device can tell the device core
> > to inherit this flag if there is no off-tree dependency.
> 
> Well, I'd prefer to identify all of the off-tree dependencies that have to be
> taken into account and handle all devices asynchronously.

Anyway, I have tested the appended patch on top of [1/7]-[7/7] and my test box
appears to work fine with it, although it doesn't work in the "async for all"
case.

I guess the next step will be to see which devices are not handled
asynchronously with the patch below and try to figure out which of them
break(s) things.

Thanks,
Rafael

---
 drivers/base/power/common.c |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Index: linux-2.6/drivers/base/power/common.c
===================================================================
--- linux-2.6.orig/drivers/base/power/common.c
+++ linux-2.6/drivers/base/power/common.c
@@ -32,7 +32,11 @@ void device_pm_add(struct device *dev)
 	pr_debug("PM: Adding info for %s:%s\n",
 		 dev->bus ? dev->bus->name : "No Bus",
 		 kobject_name(&dev->kobj));
-	pm_link_add(dev, dev->parent);
+	if (dev->parent) {
+		pm_link_add(dev, dev->parent);
+		if (dev->parent->power.async_suspend)
+			dev->power.async_suspend = true;
+	}
 	device_pm_list_add(dev);
 }
 

Re: [RFC][PATCH 0/7] PM: Asynchronous suspend and resume (updated)

[Zhang Rui]

On Wed, 2009-08-19 at 07:58 +0800, Rafael J. Wysocki wrote:
> On Tuesday 18 August 2009, Rafael J. Wysocki wrote:
> > On Tuesday 18 August 2009, Zhang Rui wrote:
> > > On Mon, 2009-08-17 at 08:15 +0800, Rafael J. Wysocki wrote:
> > > > On Wednesday 12 August 2009, Rafael J. Wysocki wrote:
> > > > > Hi,
> > > > > 
> > > > > The following patches introduce a mechanism allowing us to execute device
> > > > > drivers' suspend and resume callbacks asynchronously during system sleep
> > > > > transitions, such as suspend to RAM.  The idea is explained in the [1/1] patch
> > > > > message.
> > > > 
> > > > Changes:
> > > > 
> > > > * Added [1/7] that fixes kerneldoc comments in drivers/base/power/main.c
> > > >   (this is a 2.6.32 candidate).
> > > > 
> > > > * Added [2/7] adding a framework for representing PM link (idea described
> > > >   in the patch message).
> > > > 
> > > > * [3/7] is the async resume patch (idea described in the patch message).
> > > > 
> > > > * [4/7] is the async suspend patch.
> > > > 
> > > > * [5/7] - [7/7] set async_suspend for devices in a few selected subsystems.
> > > > 
> > > > The patches have been tested on HP nx6325.
> > > > 
> > > I tried this patch set and it does work. :)
> > > But unfortunately it doesn't save too much time.
> > > 
> > > I still think that the child device should inherit its parent's
> > > async_suspend flag to do the asynchronous resume more efficiently.
> > > 
> > > or at least we should provide such an interface
> > > in drivers/base/power/common.c, so that device can tell the device core
> > > to inherit this flag if there is no off-tree dependency.
> > 
> > Well, I'd prefer to identify all of the off-tree dependencies that have to be
> > taken into account and handle all devices asynchronously.
> 
> Anyway, I have tested the appended patch on top of [1/7]-[7/7] and my test box
> appears to work fine with it, although it doesn't work in the "async for all"
> case.
> 
> I guess the next step will be to see which devices are not handled
> asynchronously with the patch below and try to figure out which of them
> break(s) things.
> 
> Thanks,
> Rafael
> 
> ---
>  drivers/base/power/common.c |    6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> Index: linux-2.6/drivers/base/power/common.c
> ===================================================================
> --- linux-2.6.orig/drivers/base/power/common.c
> +++ linux-2.6/drivers/base/power/common.c
> @@ -32,7 +32,11 @@ void device_pm_add(struct device *dev)
>  	pr_debug("PM: Adding info for %s:%s\n",
>  		 dev->bus ? dev->bus->name : "No Bus",
>  		 kobject_name(&dev->kobj));
> -	pm_link_add(dev, dev->parent);
> +	if (dev->parent) {
> +		pm_link_add(dev, dev->parent);
> +		if (dev->parent->power.async_suspend)
> +			dev->power.async_suspend = true;
> +	}

to use this, we must make sure that device_enable_async_suspend is
called before any of its child device being registered, right?
should we check this in device_enable_async_suspend?
or at least we should add the comments stating this issue.

thanks,
rui

>  	device_pm_list_add(dev);
>  }
>  

Re: [RFC][PATCH 0/7] PM: Asynchronous suspend and resume (updated)

[Rafael J. Wysocki]

On Wednesday 19 August 2009, Zhang Rui wrote:
> On Wed, 2009-08-19 at 07:58 +0800, Rafael J. Wysocki wrote:
> > On Tuesday 18 August 2009, Rafael J. Wysocki wrote:
> > > On Tuesday 18 August 2009, Zhang Rui wrote:
> > > > On Mon, 2009-08-17 at 08:15 +0800, Rafael J. Wysocki wrote:
> > > > > On Wednesday 12 August 2009, Rafael J. Wysocki wrote:
> > > > > > Hi,
> > > > > > 
> > > > > > The following patches introduce a mechanism allowing us to execute device
> > > > > > drivers' suspend and resume callbacks asynchronously during system sleep
> > > > > > transitions, such as suspend to RAM.  The idea is explained in the [1/1] patch
> > > > > > message.
> > > > > 
> > > > > Changes:
> > > > > 
> > > > > * Added [1/7] that fixes kerneldoc comments in drivers/base/power/main.c
> > > > >   (this is a 2.6.32 candidate).
> > > > > 
> > > > > * Added [2/7] adding a framework for representing PM link (idea described
> > > > >   in the patch message).
> > > > > 
> > > > > * [3/7] is the async resume patch (idea described in the patch message).
> > > > > 
> > > > > * [4/7] is the async suspend patch.
> > > > > 
> > > > > * [5/7] - [7/7] set async_suspend for devices in a few selected subsystems.
> > > > > 
> > > > > The patches have been tested on HP nx6325.
> > > > > 
> > > > I tried this patch set and it does work. :)
> > > > But unfortunately it doesn't save too much time.
> > > > 
> > > > I still think that the child device should inherit its parent's
> > > > async_suspend flag to do the asynchronous resume more efficiently.
> > > > 
> > > > or at least we should provide such an interface
> > > > in drivers/base/power/common.c, so that device can tell the device core
> > > > to inherit this flag if there is no off-tree dependency.
> > > 
> > > Well, I'd prefer to identify all of the off-tree dependencies that have to be
> > > taken into account and handle all devices asynchronously.
> > 
> > Anyway, I have tested the appended patch on top of [1/7]-[7/7] and my test box
> > appears to work fine with it, although it doesn't work in the "async for all"
> > case.
> > 
> > I guess the next step will be to see which devices are not handled
> > asynchronously with the patch below and try to figure out which of them
> > break(s) things.
> > 
> > Thanks,
> > Rafael
> > 
> > ---
> >  drivers/base/power/common.c |    6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> > 
> > Index: linux-2.6/drivers/base/power/common.c
> > ===================================================================
> > --- linux-2.6.orig/drivers/base/power/common.c
> > +++ linux-2.6/drivers/base/power/common.c
> > @@ -32,7 +32,11 @@ void device_pm_add(struct device *dev)
> >  	pr_debug("PM: Adding info for %s:%s\n",
> >  		 dev->bus ? dev->bus->name : "No Bus",
> >  		 kobject_name(&dev->kobj));
> > -	pm_link_add(dev, dev->parent);
> > +	if (dev->parent) {
> > +		pm_link_add(dev, dev->parent);
> > +		if (dev->parent->power.async_suspend)
> > +			dev->power.async_suspend = true;
> > +	}
> 
> to use this, we must make sure that device_enable_async_suspend is
> called before any of its child device being registered, right?
> should we check this in device_enable_async_suspend?
> or at least we should add the comments stating this issue.

That's correct in general, but I added the patch for testing purposes only.

The goal still is to identify all of the dependencies that need to be taken
care of and to represent them appropriately, so that we can safely set
async_suspend for all devices.

I wonder if you get any improvement with this patch applied?

Thanks,
Rafael

Re: [RFC][PATCH 0/7] PM: Asynchronous suspend and resume (updated)

[Zhang Rui]

On Thu, 2009-08-20 at 05:02 +0800, Rafael J. Wysocki wrote:
> On Wednesday 19 August 2009, Zhang Rui wrote:
> > On Wed, 2009-08-19 at 07:58 +0800, Rafael J. Wysocki wrote:
> > > On Tuesday 18 August 2009, Rafael J. Wysocki wrote:
> > > > On Tuesday 18 August 2009, Zhang Rui wrote:
> > > > > On Mon, 2009-08-17 at 08:15 +0800, Rafael J. Wysocki wrote:
> > > > > > On Wednesday 12 August 2009, Rafael J. Wysocki wrote:
> > > > > > > Hi,
> > > > > > > 
> > > > > > > The following patches introduce a mechanism allowing us to execute device
> > > > > > > drivers' suspend and resume callbacks asynchronously during system sleep
> > > > > > > transitions, such as suspend to RAM.  The idea is explained in the [1/1] patch
> > > > > > > message.
> > > > > > 
> > > > > > Changes:
> > > > > > 
> > > > > > * Added [1/7] that fixes kerneldoc comments in drivers/base/power/main.c
> > > > > >   (this is a 2.6.32 candidate).
> > > > > > 
> > > > > > * Added [2/7] adding a framework for representing PM link (idea described
> > > > > >   in the patch message).
> > > > > > 
> > > > > > * [3/7] is the async resume patch (idea described in the patch message).
> > > > > > 
> > > > > > * [4/7] is the async suspend patch.
> > > > > > 
> > > > > > * [5/7] - [7/7] set async_suspend for devices in a few selected subsystems.
> > > > > > 
> > > > > > The patches have been tested on HP nx6325.
> > > > > > 
> > > > > I tried this patch set and it does work. :)
> > > > > But unfortunately it doesn't save too much time.
> > > > > 
> > > > > I still think that the child device should inherit its parent's
> > > > > async_suspend flag to do the asynchronous resume more efficiently.
> > > > > 
> > > > > or at least we should provide such an interface
> > > > > in drivers/base/power/common.c, so that device can tell the device core
> > > > > to inherit this flag if there is no off-tree dependency.
> > > > 
> > > > Well, I'd prefer to identify all of the off-tree dependencies that have to be
> > > > taken into account and handle all devices asynchronously.
> > > 
> > > Anyway, I have tested the appended patch on top of [1/7]-[7/7] and my test box
> > > appears to work fine with it, although it doesn't work in the "async for all"
> > > case.
> > > 
> > > I guess the next step will be to see which devices are not handled
> > > asynchronously with the patch below and try to figure out which of them
> > > break(s) things.
> > > 
> > > Thanks,
> > > Rafael
> > > 
> > > ---
> > >  drivers/base/power/common.c |    6 +++++-
> > >  1 file changed, 5 insertions(+), 1 deletion(-)
> > > 
> > > Index: linux-2.6/drivers/base/power/common.c
> > > ===================================================================
> > > --- linux-2.6.orig/drivers/base/power/common.c
> > > +++ linux-2.6/drivers/base/power/common.c
> > > @@ -32,7 +32,11 @@ void device_pm_add(struct device *dev)
> > >  	pr_debug("PM: Adding info for %s:%s\n",
> > >  		 dev->bus ? dev->bus->name : "No Bus",
> > >  		 kobject_name(&dev->kobj));
> > > -	pm_link_add(dev, dev->parent);
> > > +	if (dev->parent) {
> > > +		pm_link_add(dev, dev->parent);
> > > +		if (dev->parent->power.async_suspend)
> > > +			dev->power.async_suspend = true;
> > > +	}
> > 
> > to use this, we must make sure that device_enable_async_suspend is
> > called before any of its child device being registered, right?
> > should we check this in device_enable_async_suspend?
> > or at least we should add the comments stating this issue.
> 
> That's correct in general, but I added the patch for testing purposes only.
> 
> The goal still is to identify all of the dependencies that need to be taken
> care of and to represent them appropriately, so that we can safely set
> async_suspend for all devices.
> 
> I wonder if you get any improvement with this patch applied?
> 
No, it doesn't work.
the system hangs during suspend.
I have not figured out the root cause.

thanks,
rui

Re: [RFC][PATCH 0/7] PM: Asynchronous suspend and resume (updated)

[Pavel Machek]

Hi!

> I still think that the child device should inherit its parent's
> async_suspend flag to do the asynchronous resume more efficiently.

I really think this should be opt-in. Even if seemingly simple cases
like usb devices... there's stuff like olpc where you have usb device
and sideband channel....

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

Re: [RFC][PATCH 0/7] PM: Asynchronous suspend and resume (updated)

[Alan Stern]

On Mon, 17 Aug 2009, Rafael J. Wysocki wrote:

> On Wednesday 12 August 2009, Rafael J. Wysocki wrote:
> > Hi,
> > 
> > The following patches introduce a mechanism allowing us to execute device
> > drivers' suspend and resume callbacks asynchronously during system sleep
> > transitions, such as suspend to RAM.  The idea is explained in the [1/1] patch
> > message.
> 
> Changes:
> 
> * Added [1/7] that fixes kerneldoc comments in drivers/base/power/main.c
>   (this is a 2.6.32 candidate).
> 
> * Added [2/7] adding a framework for representing PM link (idea described
>   in the patch message).
> 
> * [3/7] is the async resume patch (idea described in the patch message).
> 
> * [4/7] is the async suspend patch.
> 
> * [5/7] - [7/7] set async_suspend for devices in a few selected subsystems.
> 
> The patches have been tested on HP nx6325.
> 
> Comments welcome.

I'm not sure about the design of these things.  How much do we care 
about wasting memory?  Your scheme allocates six pointers for every 
dependency, plus four pointers for every device.  It's possible to 
reduce this considerably, especially if the parent-child dependencies 
aren't stored explicitly.

If you decide to keep this scheme, you should make pm_link_add() check 
for duplicate dependencies before adding them.

Also, I think a better approach to the async execution would not
require adding a struct completion to each device and making each async
thread wait for the completion to be signalled.  Instead, have a single
master thread (i.e., the thread doing the suspend) monitor the
dependencies and have it farm the devices out to async threads as they
become ready to be suspended or resumed.

Finally, devices that don't have async_suspend set should implicitly 
depend on everything that comes after them (for suspend) or before them 
(for resume) in the device list.

Alan Stern

Re: [RFC][PATCH 0/7] PM: Asynchronous suspend and resume (updated)

[Rafael J. Wysocki]

On Tuesday 18 August 2009, Alan Stern wrote:
> On Mon, 17 Aug 2009, Rafael J. Wysocki wrote:
> 
> > On Wednesday 12 August 2009, Rafael J. Wysocki wrote:
> > > Hi,
> > > 
> > > The following patches introduce a mechanism allowing us to execute device
> > > drivers' suspend and resume callbacks asynchronously during system sleep
> > > transitions, such as suspend to RAM.  The idea is explained in the [1/1] patch
> > > message.
> > 
> > Changes:
> > 
> > * Added [1/7] that fixes kerneldoc comments in drivers/base/power/main.c
> >   (this is a 2.6.32 candidate).
> > 
> > * Added [2/7] adding a framework for representing PM link (idea described
> >   in the patch message).
> > 
> > * [3/7] is the async resume patch (idea described in the patch message).
> > 
> > * [4/7] is the async suspend patch.
> > 
> > * [5/7] - [7/7] set async_suspend for devices in a few selected subsystems.
> > 
> > The patches have been tested on HP nx6325.
> > 
> > Comments welcome.
> 
> I'm not sure about the design of these things.  How much do we care 
> about wasting memory?

Not much.

> Your scheme allocates six pointers for every dependency, plus four pointers
> for every device.

Yes, it does.

> It's possible to reduce this considerably, especially if the parent-child
> dependencies aren't stored explicitly.

Yes, at the expense of increased complexity and reduced performance.

> If you decide to keep this scheme, you should make pm_link_add() check 
> for duplicate dependencies before adding them.

That's correct.

> Also, I think a better approach to the async execution would not
> require adding a struct completion to each device and making each async
> thread wait for the completion to be signalled.  Instead, have a single
> master thread (i.e., the thread doing the suspend) monitor the
> dependencies and have it farm the devices out to async threads as they
> become ready to be suspended or resumed.

Do you mean that the master thread should check the dependencies
_before_ executing, for example, __device_resume() and execute it
asynchronously only if they are already satisfied?  In that case we might lose
the opportunity to save some time.

For example, assume devices A and B depend on C. Say that normally, A would be
handled before B, so if C hasn't finished yet, the A's callback will be
executed synchronously.  Now, if both A and B take time T to complete the
callback and C finishes dT after we've called A synchronously, we'll lose the
chance to save T - dT by handling A and B in parallel.

The master thread might chose another device for asynchronous execution, but
then it should revisit A and B and that still is going to be suboptimal
time-wise in some specific situations (eg. A and B are the last two devices to
handle).

> Finally, devices that don't have async_suspend set should implicitly 
> depend on everything that comes after them (for suspend) or before them 
> (for resume) in the device list.

They do, through dpm_list.

Thanks,
Rafael

Re: [RFC][PATCH 0/7] PM: Asynchronous suspend and resume (updated)

[Alan Stern]

On Tue, 18 Aug 2009, Rafael J. Wysocki wrote:

> > Also, I think a better approach to the async execution would not
> > require adding a struct completion to each device and making each async
> > thread wait for the completion to be signalled.  Instead, have a single
> > master thread (i.e., the thread doing the suspend) monitor the
> > dependencies and have it farm the devices out to async threads as they
> > become ready to be suspended or resumed.
> 
> Do you mean that the master thread should check the dependencies
> _before_ executing, for example, __device_resume() and execute it
> asynchronously only if they are already satisfied?  In that case we might lose
> the opportunity to save some time.

That's almost what I mean.  The master thread should keep track of the 
state of all the devices.  Each time a suspend or resume completes, the 
master thread should determine which devices now have all their 
dependencies satisfied as a result, and should asynchronously execute 
__device_resume() for each one of them.

> For example, assume devices A and B depend on C. Say that normally, A would be
> handled before B, so if C hasn't finished yet, the A's callback will be
> executed synchronously.  Now, if both A and B take time T to complete the
> callback and C finishes dT after we've called A synchronously, we'll lose the
> chance to save T - dT by handling A and B in parallel.

No, that's not what I mean.  Until C is finished, the master thread
will sleep.  When C finishes the master thread will wake up, note that
A and B can now be resumed, fire off two async threads to resume them, 
and go back to sleep.

> The master thread might chose another device for asynchronous execution, but
> then it should revisit A and B and that still is going to be suboptimal
> time-wise in some specific situations (eg. A and B are the last two devices to
> handle).
> 
> > Finally, devices that don't have async_suspend set should implicitly 
> > depend on everything that comes after them (for suspend) or before them 
> > (for resume) in the device list.
> 
> They do, through dpm_list.

Do they?  I didn't read the code closely enough to tell.  This
requirement should of course be met by whichever scheme we end up
using.  I mentioned it because it provides a simple way of including 
synchronous operations in an async framework.

Alan Stern

Re: [RFC][PATCH 0/7] PM: Asynchronous suspend and resume (updated)

[Rafael J. Wysocki]

On Tuesday 18 August 2009, Alan Stern wrote:
> On Tue, 18 Aug 2009, Rafael J. Wysocki wrote:
> 
> > > Also, I think a better approach to the async execution would not
> > > require adding a struct completion to each device and making each async
> > > thread wait for the completion to be signalled.  Instead, have a single
> > > master thread (i.e., the thread doing the suspend) monitor the
> > > dependencies and have it farm the devices out to async threads as they
> > > become ready to be suspended or resumed.
> > 
> > Do you mean that the master thread should check the dependencies
> > _before_ executing, for example, __device_resume() and execute it
> > asynchronously only if they are already satisfied?  In that case we might lose
> > the opportunity to save some time.
> 
> That's almost what I mean.  The master thread should keep track of the 
> state of all the devices.  Each time a suspend or resume completes, the 
> master thread should determine which devices now have all their 
> dependencies satisfied as a result, and should asynchronously execute 
> __device_resume() for each one of them.
> 
> > For example, assume devices A and B depend on C. Say that normally, A would be
> > handled before B, so if C hasn't finished yet, the A's callback will be
> > executed synchronously.  Now, if both A and B take time T to complete the
> > callback and C finishes dT after we've called A synchronously, we'll lose the
> > chance to save T - dT by handling A and B in parallel.
> 
> No, that's not what I mean.  Until C is finished, the master thread
> will sleep.  When C finishes the master thread will wake up, note that
> A and B can now be resumed, fire off two async threads to resume them, 
> and go back to sleep.

There's a problem that for safety reasons I maintain the ordering of dpm_list
and the callbacks are scheduled for async execution in the same order in
which they would have been executed synchronously.  If were to change this,
we'd have to be _very_ careful.

Thanks,
Rafael

Re: [RFC][PATCH 0/7] PM: Asynchronous suspend and resume (updated)

[Alan Stern]

On Wed, 19 Aug 2009, Rafael J. Wysocki wrote:

> There's a problem that for safety reasons I maintain the ordering of dpm_list
> and the callbacks are scheduled for async execution in the same order in
> which they would have been executed synchronously.  If were to change this,
> we'd have to be _very_ careful.

Why?  The order in which jobs are scheduled for async execution doesn't 
bear any particular relation to the order in which they get run.

Alan Stern

Re: [RFC][PATCH 0/7] PM: Asynchronous suspend and resume (updated)

[Rafael J. Wysocki]

On Wednesday 19 August 2009, Alan Stern wrote:
> On Wed, 19 Aug 2009, Rafael J. Wysocki wrote:
> 
> > There's a problem that for safety reasons I maintain the ordering of dpm_list
> > and the callbacks are scheduled for async execution in the same order in
> > which they would have been executed synchronously.  If were to change this,
> > we'd have to be _very_ careful.
> 
> Why?  The order in which jobs are scheduled for async execution doesn't 
> bear any particular relation to the order in which they get run.

Yes, it does, if all of the async threads are busy and we add more async jobs
to the queue.  We must ensure that none of the jobs being executed will wait
for any jobs in the queue.

Also, if any devices are handled synchronously, they must not wait for any
"async" devices that haven't been scheduled yet.

With a master thread that would do all the waiting that wouldn't be a problem
any more, but I'm not sure how to implement such a thread efficiently.  The
problem is that each device may depend on multiple other devices, so even
if one callback finishes, there's no guarantee there will be any device with
satisfied dependencies, so it looks like the master thread would have to
browse dpm_list continuously searching it for devices that are ready for
suspending.

Also I don't think we can change the ordering of dpm_list as a result of
asynchronous execution.

Thanks,
Rafael

Re: [RFC][PATCH 0/7] PM: Asynchronous suspend and resume (updated)

[Alan Stern]

On Wed, 19 Aug 2009, Rafael J. Wysocki wrote:

> On Wednesday 19 August 2009, Alan Stern wrote:
> > On Wed, 19 Aug 2009, Rafael J. Wysocki wrote:
> > 
> > > There's a problem that for safety reasons I maintain the ordering of dpm_list
> > > and the callbacks are scheduled for async execution in the same order in
> > > which they would have been executed synchronously.  If were to change this,
> > > we'd have to be _very_ careful.
> > 
> > Why?  The order in which jobs are scheduled for async execution doesn't 
> > bear any particular relation to the order in which they get run.
> 
> Yes, it does, if all of the async threads are busy and we add more async jobs
> to the queue.  We must ensure that none of the jobs being executed will wait
> for any jobs in the queue.

That's part of the reason for the dependencies: to make sure that where
it matters, things don't run in the wrong order.

> Also, if any devices are handled synchronously, they must not wait for any
> "async" devices that haven't been scheduled yet.

I'm not sure what you mean.  If a device is handled synchronously, it 
must wait for all the devices preceding it on the list, regardless of 
whether those devices are handled asynchronously.

> With a master thread that would do all the waiting that wouldn't be a problem
> any more, but I'm not sure how to implement such a thread efficiently.  The
> problem is that each device may depend on multiple other devices, so even
> if one callback finishes, there's no guarantee there will be any device with
> satisfied dependencies, so it looks like the master thread would have to
> browse dpm_list continuously searching it for devices that are ready for
> suspending.
> 
> Also I don't think we can change the ordering of dpm_list as a result of
> asynchronous execution.

Okay, so I'll try to write something.  We'll see how it comes out.

Alan Stern

Re: [RFC][PATCH 0/7] PM: Asynchronous suspend and resume (updated)

[Rafael J. Wysocki]

On Thursday 20 August 2009, Alan Stern wrote:
> On Wed, 19 Aug 2009, Rafael J. Wysocki wrote:
> 
> > On Wednesday 19 August 2009, Alan Stern wrote:
> > > On Wed, 19 Aug 2009, Rafael J. Wysocki wrote:
> > > 
> > > > There's a problem that for safety reasons I maintain the ordering of dpm_list
> > > > and the callbacks are scheduled for async execution in the same order in
> > > > which they would have been executed synchronously.  If were to change this,
> > > > we'd have to be _very_ careful.
> > > 
> > > Why?  The order in which jobs are scheduled for async execution doesn't 
> > > bear any particular relation to the order in which they get run.
> > 
> > Yes, it does, if all of the async threads are busy and we add more async jobs
> > to the queue.  We must ensure that none of the jobs being executed will wait
> > for any jobs in the queue.
> 
> That's part of the reason for the dependencies: to make sure that where
> it matters, things don't run in the wrong order.
> 
> > Also, if any devices are handled synchronously, they must not wait for any
> > "async" devices that haven't been scheduled yet.
> 
> I'm not sure what you mean.  If a device is handled synchronously, it 
> must wait for all the devices preceding it on the list, regardless of 
> whether those devices are handled asynchronously.

Not really, IMO.  Namely, setting async_suspend for a device is a declaration
that all of its dependencies are known, whether they are on "async" or on
"sync" devices.  Hence, "sync" devices don't need to wait for the "async"
devices they don't depend on in a known way, because otherwise it would be
invalid to set async_suspend for those "async" devices.

Nevertheless, "sync" devices have to wait for all of the other "sync" devices
that are "in front" of them in dpm_list, because dependencies between different
"sync" devices need not be known (the direct dependencies betwee them are
known, but there may be other unknown dependencies satisfied only because the
the ordering of dpm_list is preserved).

Obviously "sync" devices have to wait for the "async" devices they depend on
in a known way.  In turn, "async" devices have to wait for all of the devices
they depend on in a known way, "sync" as well as "async".

Thanks,
Rafael

Re: [RFC][PATCH 0/7] PM: Asynchronous suspend and resume (updated)

[Pavel Machek]

Hi!

> > * Added [2/7] adding a framework for representing PM link (idea described
> >   in the patch message).
> > 
> > * [3/7] is the async resume patch (idea described in the patch message).
> > 
> > * [4/7] is the async suspend patch.
> > 
> > * [5/7] - [7/7] set async_suspend for devices in a few selected subsystems.
> > 
> > The patches have been tested on HP nx6325.
> > 
> > Comments welcome.
> 
> I'm not sure about the design of these things.  How much do we care 
> about wasting memory?  Your scheme allocates six pointers for every 
> dependency, plus four pointers for every device.  It's possible to 
> reduce this considerably, especially if the parent-child dependencies 
> aren't stored explicitly.

I'd say that this level of memory waste is perfectly acceptable...

							Pavel


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