[PATCH 1/3] PM / Runtime: Update documentation of interactions with system sleep

[PATCH 1/3] PM / Runtime: Update documentation of interactions with system sleep

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

The documents describing the interactions between runtime PM and
system sleep generally refer to the model in which the system sleep
state is entered through a global firmware or hardware operation.
As a result, some recommendations given in there are not entirely
suitable for systems in which this is not the case.  Update the
documentation take the existence of those systems into accout.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
Reviewed-by: Kevin Hilman <khilman@ti.com>
---
 Documentation/power/devices.txt    |    6 +++---
 Documentation/power/runtime_pm.txt |   27 +++++++++++++++++++--------
 2 files changed, 22 insertions(+), 11 deletions(-)

Index: linux-2.6/Documentation/power/runtime_pm.txt
===================================================================
--- linux-2.6.orig/Documentation/power/runtime_pm.txt
+++ linux-2.6/Documentation/power/runtime_pm.txt
@@ -537,9 +537,9 @@ suspend routine).  It may be necessary t
 in order to do so.  The same is true if the driver uses different power levels
 or other settings for run-time suspend and system sleep.
 
-During system resume, devices generally should be brought back to full power,
-even if they were suspended before the system sleep began.  There are several
-reasons for this, including:
+During system resume, the simplest approach is to bring all devices back to full
+power, even if they had been suspended before the system suspend began.  There
+are several reasons for this, including:
 
   * The device might need to switch power levels, wake-up settings, etc.
 
@@ -556,16 +556,27 @@ reasons for this, including:
   * Even though the device was suspended, if its usage counter was > 0 then most
     likely it would need a run-time resume in the near future anyway.
 
-  * Always going back to full power is simplest.
-
-If the device was suspended before the sleep began, then its run-time PM status
-will have to be updated to reflect the actual post-system sleep status.  The way
-to do this is:
+If the device had been suspended before the system suspend began and it's
+brought back to full power during resume, then its run-time PM status will have
+to be updated to reflect the actual post-system sleep status.  The way to do
+this is:
 
 	pm_runtime_disable(dev);
 	pm_runtime_set_active(dev);
 	pm_runtime_enable(dev);
 
+On some systems, however, system sleep is not entered through a global firmware
+or hardware operation.  Instead, all hardware components are put into low-power
+states directly by the kernel in a coordinated way.  Then, the system sleep
+state effectively follows from the states the hardware components end up in
+and the system is woken up from that state by a hardware interrupt or a similar
+mechanism entirely under the kernel's control.  As a result, the kernel never
+gives control away and the states of all devices during resume are precisely
+known to it.  If that is the case and none of the situations listed above takes
+place (in particular, if the system is not waking up from hibernation), it may
+be more efficient to leave the devices that had been suspended before the system
+suspend began in the suspended state.
+
 7. Generic subsystem callbacks
 
 Subsystems may wish to conserve code space by using the set of generic power
Index: linux-2.6/Documentation/power/devices.txt
===================================================================
--- linux-2.6.orig/Documentation/power/devices.txt
+++ linux-2.6/Documentation/power/devices.txt
@@ -604,7 +604,7 @@ state temporarily, for example so that i
 disabled.  This all depends on the hardware and the design of the subsystem and
 device driver in question.
 
-During system-wide resume from a sleep state it's best to put devices into the
-full-power state, as explained in Documentation/power/runtime_pm.txt.  Refer to
-that document for more information regarding this particular issue as well as
+During system-wide resume from a sleep state it's easiest to put devices into
+the full-power state, as explained in Documentation/power/runtime_pm.txt.  Refer
+to that document for more information regarding this particular issue as well as
 for information on the device runtime power management framework in general.

[PATCH 2/3] PM / Runtime: Return special error code if runtime PM is disabled

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

Some callers of pm_runtime_get_sync() and other runtime PM helper
functions, scsi_autopm_get_host() and scsi_autopm_get_device() in
particular, need to distinguish error codes returned when runtime PM
is disabled (i.e. power.disable_depth is nonzero for the given
device) from error codes returned in other situations.  For this
reason, make the runtime PM helper functions return -EACCES when
power.disable_depth is nonzero and ensure that this error code
won't be returned by them in any other circumstances.  Modify
scsi_autopm_get_host() and scsi_autopm_get_device() to check the
error code returned by pm_runtime_get_sync() and ignore -EACCES.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 Documentation/power/runtime_pm.txt |    6 ++++--
 drivers/base/power/runtime.c       |    9 +++++----
 drivers/scsi/scsi_pm.c             |    8 ++++----
 3 files changed, 13 insertions(+), 10 deletions(-)

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
@@ -135,8 +135,9 @@ static int rpm_check_suspend_allowed(str
 
 	if (dev->power.runtime_error)
 		retval = -EINVAL;
-	else if (atomic_read(&dev->power.usage_count) > 0
-	    || dev->power.disable_depth > 0)
+	else if (dev->power.disable_depth > 0)
+		retval = -EACCES;
+	else if (atomic_read(&dev->power.usage_count) > 0)
 		retval = -EAGAIN;
 	else if (!pm_children_suspended(dev))
 		retval = -EBUSY;
@@ -262,7 +263,7 @@ static int rpm_callback(int (*cb)(struct
 		spin_lock_irq(&dev->power.lock);
 	}
 	dev->power.runtime_error = retval;
-	return retval;
+	return retval != -EACCES ? retval : -EIO;
 }
 
 /**
@@ -458,7 +459,7 @@ static int rpm_resume(struct device *dev
 	if (dev->power.runtime_error)
 		retval = -EINVAL;
 	else if (dev->power.disable_depth > 0)
-		retval = -EAGAIN;
+		retval = -EACCES;
 	if (retval)
 		goto out;
 
Index: linux-2.6/drivers/scsi/scsi_pm.c
===================================================================
--- linux-2.6.orig/drivers/scsi/scsi_pm.c
+++ linux-2.6/drivers/scsi/scsi_pm.c
@@ -144,9 +144,9 @@ int scsi_autopm_get_device(struct scsi_d
 	int	err;
 
 	err = pm_runtime_get_sync(&sdev->sdev_gendev);
-	if (err < 0)
+	if (err < 0 && err !=-EACCES)
 		pm_runtime_put_sync(&sdev->sdev_gendev);
-	else if (err > 0)
+	else
 		err = 0;
 	return err;
 }
@@ -173,9 +173,9 @@ int scsi_autopm_get_host(struct Scsi_Hos
 	int	err;
 
 	err = pm_runtime_get_sync(&shost->shost_gendev);
-	if (err < 0)
+	if (err < 0 && err !=-EACCES)
 		pm_runtime_put_sync(&shost->shost_gendev);
-	else if (err > 0)
+	else
 		err = 0;
 	return err;
 }
Index: linux-2.6/Documentation/power/runtime_pm.txt
===================================================================
--- linux-2.6.orig/Documentation/power/runtime_pm.txt
+++ linux-2.6/Documentation/power/runtime_pm.txt
@@ -291,7 +291,8 @@ drivers/base/power/runtime.c and include
     - execute the subsystem-level suspend callback for the device; returns 0 on
       success, 1 if the device's run-time PM status was already 'suspended', or
       error code on failure, where -EAGAIN or -EBUSY means it is safe to attempt
-      to suspend the device again in future
+      to suspend the device again in future and -EACCES means that disable_depth
+      is different from 0
 
   int pm_runtime_autosuspend(struct device *dev);
     - same as pm_runtime_suspend() except that the autosuspend delay is taken
@@ -304,7 +305,8 @@ drivers/base/power/runtime.c and include
       success, 1 if the device's run-time PM status was already 'active' or
       error code on failure, where -EAGAIN means it may be safe to attempt to
       resume the device again in future, but 'power.runtime_error' should be
-      checked additionally
+      checked additionally, and -EACCES means that disable_depth is different
+      from 0
 
   int pm_request_idle(struct device *dev);
     - submit a request to execute the subsystem-level idle callback for the

[PATCH 3/3] PM: Limit race conditions between runtime PM and system sleep

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

One of the roles of the PM core is to prevent different PM callbacks
executed for the same device object from racing with each other.
Unfortunately, after commit e8665002477f0278f84f898145b1f141ba26ee26
(PM: Allow pm_runtime_suspend() to succeed during system suspend)
runtime PM callbacks may be executed concurrently with system
suspend/resume callbacks for the same device.

The main reason for commit e8665002477f0278f84f898145b1f141ba26ee26
was that some subsystems and device drivers wanted to use runtime PM
helpers, pm_runtime_suspend() and pm_runtime_put_sync() in
particular, for carrying out the suspend of devices in their
.suspend() callbacks.  However, as it's been determined recently,
there are multiple reasons not to do so, inlcuding:

 * The caller really doesn't control the runtime PM usage counters,
   because user space can access them through sysfs and effectively
   block runtime PM.  That means using pm_runtime_suspend() or
   pm_runtime_get_sync() to suspend devices during system suspend
   may or may not work.

 * If a driver calls pm_runtime_suspend() from its .suspend()
   callback, it causes the subsystem's .runtime_suspend() callback to
   be executed, which leads to the call sequence:

   subsys->suspend(dev)
      driver->suspend(dev)
         pm_runtime_suspend(dev)
            subsys->runtime_suspend(dev)

   recursive from the subsystem's point of view.  For some subsystems
   that may actually work (e.g. the platform bus type), but for some
   it will fail in a rather spectacular fashion (e.g. PCI).  In each
   case it means a layering violation.

 * Both the subsystem and the driver can provide .suspend_noirq()
   callbacks for system suspend that can do whatever the
   .runtime_suspend() callbacks do just fine, so it really isn't
   necessary to call pm_runtime_suspend() during system suspend.

 * The runtime PM's handling of wakeup devices is usually different
   from the system suspend's one, so .runtime_suspend() may simply be
   inappropriate for system suspend.

 * System suspend is supposed to work even if CONFIG_PM_RUNTIME is
   unset.

 * The runtime PM workqueue is frozen before system suspend, so if
   whatever the driver is going to do during system suspend depends
   on it, that simply won't work.

Still, there is a good reason to allow pm_runtime_resume() to
succeed during system suspend and resume (for instance, some
subsystems and device drivers may legitimately use it to ensure that
their devices are in full-power states before suspending them).
Moreover, there is no reason to prevent runtime PM callbacks from
being executed in parallel with the system suspend/resume .prepare()
and .complete() callbacks and the code removed by commit
e8665002477f0278f84f898145b1f141ba26ee26 went too far in this
respect.  On the other hand, runtime PM callbacks, including
.runtime_resume(), must not be executed during system suspend's
"late" stage of suspending devices and during system resume's "early"
device resume stage.

Taking all of the above into consideration, make the PM core
acquire a runtime PM reference to every device and resume it if
there's a runtime PM resume request pending right before executing
the subsystem-level .suspend() callback for it.  Make the PM core
drop references to all devices right after executing the
subsystem-level .resume() callbacks for them.  Additionally,
make the PM core disable the runtime PM framework for all devices
during system suspend, after executing the subsystem-level .suspend()
callbacks for them, and enable the runtime PM framework for all
devices during system resume, right before executing the
subsystem-level .resume() callbacks for them.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 Documentation/power/runtime_pm.txt |   20 ++++++++++++++++++++
 drivers/base/power/main.c          |   27 ++++++++++++++++++++++-----
 2 files changed, 42 insertions(+), 5 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
@@ -505,6 +505,7 @@ static int legacy_resume(struct device *
 static int device_resume(struct device *dev, pm_message_t state, bool async)
 {
 	int error = 0;
+	bool put = false;
 
 	TRACE_DEVICE(dev);
 	TRACE_RESUME(0);
@@ -521,6 +522,9 @@ static int device_resume(struct device *
 	if (!dev->power.is_suspended)
 		goto Unlock;
 
+	pm_runtime_enable(dev);
+	put = true;
+
 	if (dev->pm_domain) {
 		pm_dev_dbg(dev, state, "power domain ");
 		error = pm_op(dev, &dev->pm_domain->ops, state);
@@ -563,6 +567,10 @@ static int device_resume(struct device *
 	complete_all(&dev->power.completion);
 
 	TRACE_RESUME(error);
+
+	if (put)
+		pm_runtime_put_sync(dev);
+
 	return error;
 }
 
@@ -843,16 +851,22 @@ static int __device_suspend(struct devic
 	int error = 0;
 
 	dpm_wait_for_children(dev, async);
-	device_lock(dev);
 
 	if (async_error)
-		goto Unlock;
+		return 0;
+
+	pm_runtime_get_noresume(dev);
+	if (pm_runtime_barrier(dev) && device_may_wakeup(dev))
+		pm_wakeup_event(dev, 0);
 
 	if (pm_wakeup_pending()) {
+		pm_runtime_put_sync(dev);
 		async_error = -EBUSY;
-		goto Unlock;
+		return 0;
 	}
 
+	device_lock(dev);
+
 	if (dev->pm_domain) {
 		pm_dev_dbg(dev, state, "power domain ");
 		error = pm_op(dev, &dev->pm_domain->ops, state);
@@ -890,12 +904,15 @@ static int __device_suspend(struct devic
  End:
 	dev->power.is_suspended = !error;
 
- Unlock:
 	device_unlock(dev);
 	complete_all(&dev->power.completion);
 
-	if (error)
+	if (error) {
+		pm_runtime_put_sync(dev);
 		async_error = error;
+	} else if (dev->power.is_suspended) {
+		__pm_runtime_disable(dev, false);
+	}
 
 	return error;
 }
Index: linux-2.6/Documentation/power/runtime_pm.txt
===================================================================
--- linux-2.6.orig/Documentation/power/runtime_pm.txt
+++ linux-2.6/Documentation/power/runtime_pm.txt
@@ -567,6 +567,11 @@ this is:
 	pm_runtime_set_active(dev);
 	pm_runtime_enable(dev);
 
+The PM core always increments the run-time usage counter before calling the
+->suspend() callback and decrements it after calling the ->resume() callback.
+Hence disabling run-time PM temporarily like this will not cause any run-time
+suspend callbacks to be lost.
+
 On some systems, however, system sleep is not entered through a global firmware
 or hardware operation.  Instead, all hardware components are put into low-power
 states directly by the kernel in a coordinated way.  Then, the system sleep
@@ -579,6 +584,21 @@ place (in particular, if the system is n
 be more efficient to leave the devices that had been suspended before the system
 suspend began in the suspended state.
 
+The PM core does its best to reduce the probability of race conditions between
+the runtime PM and system suspend/resume (and hibernation) callbacks by carrying
+out the following operations:
+
+  * During system suspend it acquires a runtime PM reference to every device
+    and resume it if there's a runtime PM resume request pending right before
+    executing the subsystem-level .suspend() callback for it.  In addition to
+    that it disables the runtime PM framework for every device right after
+    executing the subsystem-level .suspend() callback for it.
+
+  * During system resume it enables the runtime PM framework for all devices
+    right before executing the subsystem-level .resume() callbacks for them.
+    Additionally, it drops references to all devices right after executing the
+    subsystem-level .resume() callbacks for them.
+
 7. Generic subsystem callbacks
 
 Subsystems may wish to conserve code space by using the set of generic power

Re: [PATCH 1/3] PM / Runtime: Update documentation of interactions with system sleep

[Jesper Juhl]

On Sun, 26 Jun 2011, Rafael J. Wysocki wrote:

> From: Rafael J. Wysocki <rjw@sisk.pl>
> 

A few tiny nit-picks..

> The documents describing the interactions between runtime PM and
> system sleep generally refer to the model in which the system sleep
> state is entered through a global firmware or hardware operation.
> As a result, some recommendations given in there are not entirely
> suitable for systems in which this is not the case.  Update the
> documentation take the existence of those systems into accout.
> 

I believe this should read "... documentation to take the existence of 
those systems ..."


<...>
>  
> +On some systems, however, system sleep is not entered through a global firmware
> +or hardware operation.  Instead, all hardware components are put into low-power
> +states directly by the kernel in a coordinated way.  Then, the system sleep
> +state effectively follows from the states the hardware components end up in
> +and the system is woken up from that state by a hardware interrupt or a similar
> +mechanism entirely under the kernel's control.  As a result, the kernel never
> +gives control away and the states of all devices during resume are precisely
> +known to it.  If that is the case and none of the situations listed above takes
> +place (in particular, if the system is not waking up from hibernation), it may
> +be more efficient to leave the devices that had been suspended before the system
> +suspend began in the suspended state.
> +

You are refering to device*s*, so I believe this last bit should be "... 
in the suspended states".

-- 
Jesper Juhl <jj@chaosbits.net>       http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.

Re: [PATCH 3/3] PM: Limit race conditions between runtime PM and system sleep

[Alan Stern]

On Sun, 26 Jun 2011, Rafael J. Wysocki wrote:

> From: Rafael J. Wysocki <rjw@sisk.pl>
> 
> One of the roles of the PM core is to prevent different PM callbacks
> executed for the same device object from racing with each other.
> Unfortunately, after commit e8665002477f0278f84f898145b1f141ba26ee26
> (PM: Allow pm_runtime_suspend() to succeed during system suspend)
> runtime PM callbacks may be executed concurrently with system
> suspend/resume callbacks for the same device.

...

> Index: linux-2.6/Documentation/power/runtime_pm.txt
> ===================================================================
> --- linux-2.6.orig/Documentation/power/runtime_pm.txt
> +++ linux-2.6/Documentation/power/runtime_pm.txt

> +The PM core does its best to reduce the probability of race conditions between
> +the runtime PM and system suspend/resume (and hibernation) callbacks by carrying
> +out the following operations:
> +
> +  * During system suspend it acquires a runtime PM reference to every device
> +    and resume it if there's a runtime PM resume request pending right before
> +    executing the subsystem-level .suspend() callback for it.  In addition to
> +    that it disables the runtime PM framework for every device right after
> +    executing the subsystem-level .suspend() callback for it.
> +
> +  * During system resume it enables the runtime PM framework for all devices
> +    right before executing the subsystem-level .resume() callbacks for them.
> +    Additionally, it drops references to all devices right after executing the
> +    subsystem-level .resume() callbacks for them.

I think it would be better to be a little more specific here.  Instead 
of "acquires a runtime PM reference", say "calls 
pm_runtime_get_noresume()".  Or at least, say "increments the 
run-time usage counter".

Likewise, instead of "disables the runtime PM framework", say "calls 
pm_runtime_disable()" or at least "increments power.disable_depth".

Hmmm, come to think of it...  The documentation for pm_runtime_enable() 
and pm_runtime_disable() fails to mention power.disable_depth, which is 
a surprising omission.  In particular, the description of 
pm_runtime_enable() is wrong because it ignores the possibility of 
nested disables.

Alan Stern

Re: [PATCH 3/3] PM: Limit race conditions between runtime PM and system sleep

[Ming Lei]

Hi Rafael,

On Sun, 26 Jun 2011 00:56:31 +0200
"Rafael J. Wysocki" <rjw@sisk.pl> wrote:

> Index: linux-2.6/Documentation/power/runtime_pm.txt
> ===================================================================
> --- linux-2.6.orig/Documentation/power/runtime_pm.txt
> +++ linux-2.6/Documentation/power/runtime_pm.txt
> @@ -567,6 +567,11 @@ this is:
>  	pm_runtime_set_active(dev);
>  	pm_runtime_enable(dev);
>  
> +The PM core always increments the run-time usage counter before calling the
> +->suspend() callback and decrements it after calling the ->resume() callback.
> +Hence disabling run-time PM temporarily like this will not cause any run-time
> +suspend callbacks to be lost.

Could you explain why the above is that "this will not cause any run-time suspend
callbacks to be lost"? 

Looks like it should be "this will not cause any run-time suspend callbacks to
be called", but not sure.


thanks,
-- 
Ming Lei

Re: [PATCH 3/3] PM: Limit race conditions between runtime PM and system sleep

[Rafael J. Wysocki]

On Tuesday, June 28, 2011, Ming Lei wrote:
> 
> Hi Rafael,
> 
> On Sun, 26 Jun 2011 00:56:31 +0200
> "Rafael J. Wysocki" <rjw@sisk.pl> wrote:
> 
> > Index: linux-2.6/Documentation/power/runtime_pm.txt
> > ===================================================================
> > --- linux-2.6.orig/Documentation/power/runtime_pm.txt
> > +++ linux-2.6/Documentation/power/runtime_pm.txt
> > @@ -567,6 +567,11 @@ this is:
> >  	pm_runtime_set_active(dev);
> >  	pm_runtime_enable(dev);
> >  
> > +The PM core always increments the run-time usage counter before calling the
> > +->suspend() callback and decrements it after calling the ->resume() callback.
> > +Hence disabling run-time PM temporarily like this will not cause any run-time
> > +suspend callbacks to be lost.
> 
> Could you explain why the above is that "this will not cause any run-time suspend
> callbacks to be lost"? 
> 
> Looks like it should be "this will not cause any run-time suspend callbacks to
> be called", but not sure.

You're right the wording is not perfect.  The problem is that if it's done
this way without incrementing the usage counter beforehand, the status may
change to "suspended" right before the pm_runtime_set_active() and then the
new status will not reflect the actual state of the device.

So, it may be better to say "Hence disabling runtime PM temporarily like this
will not cause the runtime PM status of the device to conflict with the actual
device state".

Alan, what do you think?

Rafael

Re: [PATCH 3/3] PM: Limit race conditions between runtime PM and system sleep

[Kevin Hilman]

"Rafael J. Wysocki" <rjw@sisk.pl> writes:

> From: Rafael J. Wysocki <rjw@sisk.pl>
>
> One of the roles of the PM core is to prevent different PM callbacks
> executed for the same device object from racing with each other.
> Unfortunately, after commit e8665002477f0278f84f898145b1f141ba26ee26
> (PM: Allow pm_runtime_suspend() to succeed during system suspend)
> runtime PM callbacks may be executed concurrently with system
> suspend/resume callbacks for the same device.
>
> The main reason for commit e8665002477f0278f84f898145b1f141ba26ee26
> was that some subsystems and device drivers wanted to use runtime PM
> helpers, pm_runtime_suspend() and pm_runtime_put_sync() in
> particular, for carrying out the suspend of devices in their
> .suspend() callbacks.  However, as it's been determined recently,
> there are multiple reasons not to do so, inlcuding:
>
>  * The caller really doesn't control the runtime PM usage counters,
>    because user space can access them through sysfs and effectively
>    block runtime PM.  That means using pm_runtime_suspend() or
>    pm_runtime_get_sync() to suspend devices during system suspend
>    may or may not work.
>
>  * If a driver calls pm_runtime_suspend() from its .suspend()
>    callback, it causes the subsystem's .runtime_suspend() callback to
>    be executed, which leads to the call sequence:
>
>    subsys->suspend(dev)
>       driver->suspend(dev)
>          pm_runtime_suspend(dev)
>             subsys->runtime_suspend(dev)
>
>    recursive from the subsystem's point of view.  For some subsystems
>    that may actually work (e.g. the platform bus type), but for some
>    it will fail in a rather spectacular fashion (e.g. PCI).  In each
>    case it means a layering violation.
>
>  * Both the subsystem and the driver can provide .suspend_noirq()
>    callbacks for system suspend that can do whatever the
>    .runtime_suspend() callbacks do just fine, so it really isn't
>    necessary to call pm_runtime_suspend() during system suspend.
>
>  * The runtime PM's handling of wakeup devices is usually different
>    from the system suspend's one, so .runtime_suspend() may simply be
>    inappropriate for system suspend.
>
>  * System suspend is supposed to work even if CONFIG_PM_RUNTIME is
>    unset.
>
>  * The runtime PM workqueue is frozen before system suspend, so if
>    whatever the driver is going to do during system suspend depends
>    on it, that simply won't work.

Thanks for the thorough description of all these reasons.

> Still, there is a good reason to allow pm_runtime_resume() to
> succeed during system suspend and resume (for instance, some
> subsystems and device drivers may legitimately use it to ensure that
> their devices are in full-power states before suspending them).
> Moreover, there is no reason to prevent runtime PM callbacks from
> being executed in parallel with the system suspend/resume .prepare()
> and .complete() callbacks and the code removed by commit
> e8665002477f0278f84f898145b1f141ba26ee26 went too far in this
> respect.  On the other hand, runtime PM callbacks, including
> .runtime_resume(), must not be executed during system suspend's
> "late" stage of suspending devices and during system resume's "early"
> device resume stage.
>
> Taking all of the above into consideration, make the PM core
> acquire a runtime PM reference to every device and resume it if
> there's a runtime PM resume request pending right before executing
> the subsystem-level .suspend() callback for it.  Make the PM core
> drop references to all devices right after executing the
> subsystem-level .resume() callbacks for them.  Additionally,
> make the PM core disable the runtime PM framework for all devices
> during system suspend, after executing the subsystem-level .suspend()
> callbacks for them, and enable the runtime PM framework for all
> devices during system resume, right before executing the
> subsystem-level .resume() callbacks for them.
>
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>

OK, I'm convinced.  :)

Acked-by: Kevin Hilman <khilman@ti.com>

Minor documentation comment below...

>  Documentation/power/runtime_pm.txt |   20 ++++++++++++++++++++
>  drivers/base/power/main.c          |   27 ++++++++++++++++++++++-----
>  2 files changed, 42 insertions(+), 5 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
> @@ -505,6 +505,7 @@ static int legacy_resume(struct device *
>  static int device_resume(struct device *dev, pm_message_t state, bool async)
>  {
>  	int error = 0;
> +	bool put = false;
>  
>  	TRACE_DEVICE(dev);
>  	TRACE_RESUME(0);
> @@ -521,6 +522,9 @@ static int device_resume(struct device *
>  	if (!dev->power.is_suspended)
>  		goto Unlock;
>  
> +	pm_runtime_enable(dev);
> +	put = true;
> +
>  	if (dev->pm_domain) {
>  		pm_dev_dbg(dev, state, "power domain ");
>  		error = pm_op(dev, &dev->pm_domain->ops, state);
> @@ -563,6 +567,10 @@ static int device_resume(struct device *
>  	complete_all(&dev->power.completion);
>  
>  	TRACE_RESUME(error);
> +
> +	if (put)
> +		pm_runtime_put_sync(dev);
> +
>  	return error;
>  }
>  
> @@ -843,16 +851,22 @@ static int __device_suspend(struct devic
>  	int error = 0;
>  
>  	dpm_wait_for_children(dev, async);
> -	device_lock(dev);
>  
>  	if (async_error)
> -		goto Unlock;
> +		return 0;
> +
> +	pm_runtime_get_noresume(dev);
> +	if (pm_runtime_barrier(dev) && device_may_wakeup(dev))
> +		pm_wakeup_event(dev, 0);
>  	if (pm_wakeup_pending()) {
> +		pm_runtime_put_sync(dev);
>  		async_error = -EBUSY;
> -		goto Unlock;
> +		return 0;
>  	}
>  
> +	device_lock(dev);
> +
>  	if (dev->pm_domain) {
>  		pm_dev_dbg(dev, state, "power domain ");
>  		error = pm_op(dev, &dev->pm_domain->ops, state);
> @@ -890,12 +904,15 @@ static int __device_suspend(struct devic
>   End:
>  	dev->power.is_suspended = !error;
>  
> - Unlock:
>  	device_unlock(dev);
>  	complete_all(&dev->power.completion);
>  
> -	if (error)
> +	if (error) {
> +		pm_runtime_put_sync(dev);
>  		async_error = error;
> +	} else if (dev->power.is_suspended) {
> +		__pm_runtime_disable(dev, false);
> +	}
>  
>  	return error;
>  }
> Index: linux-2.6/Documentation/power/runtime_pm.txt
> ===================================================================
> --- linux-2.6.orig/Documentation/power/runtime_pm.txt
> +++ linux-2.6/Documentation/power/runtime_pm.txt
> @@ -567,6 +567,11 @@ this is:
>  	pm_runtime_set_active(dev);
>  	pm_runtime_enable(dev);
>  
> +The PM core always increments the run-time usage counter before calling the
> +->suspend() callback and decrements it after calling the ->resume() callback.
> +Hence disabling run-time PM temporarily like this will not cause any run-time
> +suspend callbacks to be lost.
> +
>  On some systems, however, system sleep is not entered through a global firmware
>  or hardware operation.  Instead, all hardware components are put into low-power
>  states directly by the kernel in a coordinated way.  Then, the system sleep
> @@ -579,6 +584,21 @@ place (in particular, if the system is n
>  be more efficient to leave the devices that had been suspended before the system
>  suspend began in the suspended state.
>  
> +The PM core does its best to reduce the probability of race conditions between
> +the runtime PM and system suspend/resume (and hibernation) callbacks by carrying
> +out the following operations:
> +
> +  * During system suspend it acquires a runtime PM reference to every device
> +    and resume it if there's a runtime PM resume request pending right before

minor: s/resume/resumes/

> +    executing the subsystem-level .suspend() callback for it.  In addition to
> +    that it disables the runtime PM framework for every device right after
> +    executing the subsystem-level .suspend() callback for it.
> +
> +  * During system resume it enables the runtime PM framework for all devices
> +    right before executing the subsystem-level .resume() callbacks for them.
> +    Additionally, it drops references to all devices right after executing the
> +    subsystem-level .resume() callbacks for them.
> +
>  7. Generic subsystem callbacks
>  
>  Subsystems may wish to conserve code space by using the set of generic power

Kevin

Re: [PATCH 3/3] PM: Limit race conditions between runtime PM and system sleep

[Rafael J. Wysocki]

Hi,

On Friday, July 01, 2011, Kevin Hilman wrote:
> "Rafael J. Wysocki" <rjw@sisk.pl> writes:
> 
> > From: Rafael J. Wysocki <rjw@sisk.pl>
> >
> > One of the roles of the PM core is to prevent different PM callbacks
> > executed for the same device object from racing with each other.
> > Unfortunately, after commit e8665002477f0278f84f898145b1f141ba26ee26
> > (PM: Allow pm_runtime_suspend() to succeed during system suspend)
> > runtime PM callbacks may be executed concurrently with system
> > suspend/resume callbacks for the same device.
> >
> > The main reason for commit e8665002477f0278f84f898145b1f141ba26ee26
> > was that some subsystems and device drivers wanted to use runtime PM
> > helpers, pm_runtime_suspend() and pm_runtime_put_sync() in
> > particular, for carrying out the suspend of devices in their
> > .suspend() callbacks.  However, as it's been determined recently,
> > there are multiple reasons not to do so, inlcuding:
> >
> >  * The caller really doesn't control the runtime PM usage counters,
> >    because user space can access them through sysfs and effectively
> >    block runtime PM.  That means using pm_runtime_suspend() or
> >    pm_runtime_get_sync() to suspend devices during system suspend
> >    may or may not work.
> >
> >  * If a driver calls pm_runtime_suspend() from its .suspend()
> >    callback, it causes the subsystem's .runtime_suspend() callback to
> >    be executed, which leads to the call sequence:
> >
> >    subsys->suspend(dev)
> >       driver->suspend(dev)
> >          pm_runtime_suspend(dev)
> >             subsys->runtime_suspend(dev)
> >
> >    recursive from the subsystem's point of view.  For some subsystems
> >    that may actually work (e.g. the platform bus type), but for some
> >    it will fail in a rather spectacular fashion (e.g. PCI).  In each
> >    case it means a layering violation.
> >
> >  * Both the subsystem and the driver can provide .suspend_noirq()
> >    callbacks for system suspend that can do whatever the
> >    .runtime_suspend() callbacks do just fine, so it really isn't
> >    necessary to call pm_runtime_suspend() during system suspend.
> >
> >  * The runtime PM's handling of wakeup devices is usually different
> >    from the system suspend's one, so .runtime_suspend() may simply be
> >    inappropriate for system suspend.
> >
> >  * System suspend is supposed to work even if CONFIG_PM_RUNTIME is
> >    unset.
> >
> >  * The runtime PM workqueue is frozen before system suspend, so if
> >    whatever the driver is going to do during system suspend depends
> >    on it, that simply won't work.
> 
> Thanks for the thorough description of all these reasons.

Well, I thought I had to document them before I would forget again.

> > Still, there is a good reason to allow pm_runtime_resume() to
> > succeed during system suspend and resume (for instance, some
> > subsystems and device drivers may legitimately use it to ensure that
> > their devices are in full-power states before suspending them).
> > Moreover, there is no reason to prevent runtime PM callbacks from
> > being executed in parallel with the system suspend/resume .prepare()
> > and .complete() callbacks and the code removed by commit
> > e8665002477f0278f84f898145b1f141ba26ee26 went too far in this
> > respect.  On the other hand, runtime PM callbacks, including
> > .runtime_resume(), must not be executed during system suspend's
> > "late" stage of suspending devices and during system resume's "early"
> > device resume stage.
> >
> > Taking all of the above into consideration, make the PM core
> > acquire a runtime PM reference to every device and resume it if
> > there's a runtime PM resume request pending right before executing
> > the subsystem-level .suspend() callback for it.  Make the PM core
> > drop references to all devices right after executing the
> > subsystem-level .resume() callbacks for them.  Additionally,
> > make the PM core disable the runtime PM framework for all devices
> > during system suspend, after executing the subsystem-level .suspend()
> > callbacks for them, and enable the runtime PM framework for all
> > devices during system resume, right before executing the
> > subsystem-level .resume() callbacks for them.
> >
> > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> 
> OK, I'm convinced.  :)
> 
> Acked-by: Kevin Hilman <khilman@ti.com>

Cool, thanks. :-)

> Minor documentation comment below...
> 
> >  Documentation/power/runtime_pm.txt |   20 ++++++++++++++++++++
> >  drivers/base/power/main.c          |   27 ++++++++++++++++++++++-----
> >  2 files changed, 42 insertions(+), 5 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
> > @@ -505,6 +505,7 @@ static int legacy_resume(struct device *
> >  static int device_resume(struct device *dev, pm_message_t state, bool async)
> >  {
> >  	int error = 0;
> > +	bool put = false;
> >  
> >  	TRACE_DEVICE(dev);
> >  	TRACE_RESUME(0);
> > @@ -521,6 +522,9 @@ static int device_resume(struct device *
> >  	if (!dev->power.is_suspended)
> >  		goto Unlock;
> >  
> > +	pm_runtime_enable(dev);
> > +	put = true;
> > +
> >  	if (dev->pm_domain) {
> >  		pm_dev_dbg(dev, state, "power domain ");
> >  		error = pm_op(dev, &dev->pm_domain->ops, state);
> > @@ -563,6 +567,10 @@ static int device_resume(struct device *
> >  	complete_all(&dev->power.completion);
> >  
> >  	TRACE_RESUME(error);
> > +
> > +	if (put)
> > +		pm_runtime_put_sync(dev);
> > +
> >  	return error;
> >  }
> >  
> > @@ -843,16 +851,22 @@ static int __device_suspend(struct devic
> >  	int error = 0;
> >  
> >  	dpm_wait_for_children(dev, async);
> > -	device_lock(dev);
> >  
> >  	if (async_error)
> > -		goto Unlock;
> > +		return 0;
> > +
> > +	pm_runtime_get_noresume(dev);
> > +	if (pm_runtime_barrier(dev) && device_may_wakeup(dev))
> > +		pm_wakeup_event(dev, 0);
> >  	if (pm_wakeup_pending()) {
> > +		pm_runtime_put_sync(dev);
> >  		async_error = -EBUSY;
> > -		goto Unlock;
> > +		return 0;
> >  	}
> >  
> > +	device_lock(dev);
> > +
> >  	if (dev->pm_domain) {
> >  		pm_dev_dbg(dev, state, "power domain ");
> >  		error = pm_op(dev, &dev->pm_domain->ops, state);
> > @@ -890,12 +904,15 @@ static int __device_suspend(struct devic
> >   End:
> >  	dev->power.is_suspended = !error;
> >  
> > - Unlock:
> >  	device_unlock(dev);
> >  	complete_all(&dev->power.completion);
> >  
> > -	if (error)
> > +	if (error) {
> > +		pm_runtime_put_sync(dev);
> >  		async_error = error;
> > +	} else if (dev->power.is_suspended) {
> > +		__pm_runtime_disable(dev, false);
> > +	}
> >  
> >  	return error;
> >  }
> > Index: linux-2.6/Documentation/power/runtime_pm.txt
> > ===================================================================
> > --- linux-2.6.orig/Documentation/power/runtime_pm.txt
> > +++ linux-2.6/Documentation/power/runtime_pm.txt
> > @@ -567,6 +567,11 @@ this is:
> >  	pm_runtime_set_active(dev);
> >  	pm_runtime_enable(dev);
> >  
> > +The PM core always increments the run-time usage counter before calling the
> > +->suspend() callback and decrements it after calling the ->resume() callback.
> > +Hence disabling run-time PM temporarily like this will not cause any run-time
> > +suspend callbacks to be lost.
> > +
> >  On some systems, however, system sleep is not entered through a global firmware
> >  or hardware operation.  Instead, all hardware components are put into low-power
> >  states directly by the kernel in a coordinated way.  Then, the system sleep
> > @@ -579,6 +584,21 @@ place (in particular, if the system is n
> >  be more efficient to leave the devices that had been suspended before the system
> >  suspend began in the suspended state.
> >  
> > +The PM core does its best to reduce the probability of race conditions between
> > +the runtime PM and system suspend/resume (and hibernation) callbacks by carrying
> > +out the following operations:
> > +
> > +  * During system suspend it acquires a runtime PM reference to every device
> > +    and resume it if there's a runtime PM resume request pending right before
> 
> minor: s/resume/resumes/

That has been changed in the latest version:
https://patchwork.kernel.org/patch/919622/

Thanks,
Rafael

[PATCH 0/6 v2] PM: Fix up interactions between system suspend/resume and runtime PM

[Rafael J. Wysocki]

Hi,

The majority of the patches in the series have been updated and I prepared
two documentation-only patches that are related to this series (posted
previously) and a PCI patch that I thought would be needed for 3.0, but is
only relevant along with [4/6].

On Sunday, June 26, 2011, Rafael J. Wysocki wrote:
> Hi,
> 
> The patches in this series are related to the interactions between system
> suspend/resume and the I/O runtime PM framework.  We've had quite some
> confusion and uncertainty in that area recently, so I'd like to clean it
> up once and for all, so that everyone knows what the rules are and why
> they are what they are.
 
[1/6] - Update documentation describing interactions between runtime PM and
        system suspend/resume.

[2/6] - Return special error code from runtime PM helers if runtime PM is
        disabled and make the SCSI error handling code take it into account.

[3/6] - Add early wakeup events detection to the PCI bus type.

[4/6] - Limit race conditions between runtime PM and system suspend/resume.

[5/6] - Fix descriptions of pm_runtime_enable/disable() and add a description
        of pm_runtime_barrier() to the runtime PM documentation.

[6/6] - Replace "run-time" with "runtime" everywhere in the runtime PM
        documentation and kerneldoc comments.

The patches are on top of the PM domains patchset I've just posted, which is
available from the git branch at:

git://git.kernel.org/pub/scm/linux/kernel/git/rafael/suspend-2.6.git pm-domains

They are regarded as 3.1 material.

Thanks,
Rafael

[PATCH 1/6 v2] PM / Runtime: Update documentation of interactions with system sleep

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

The documents describing the interactions between runtime PM and
system sleep generally refer to the model in which the system sleep
state is entered through a global firmware or hardware operation.
As a result, some recommendations given in there are not entirely
suitable for systems in which this is not the case.  Update the
documentation to take the existence of those systems into account.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
Reviewed-by: Kevin Hilman <khilman@ti.com>
---
 Documentation/power/devices.txt    |    6 +++---
 Documentation/power/runtime_pm.txt |   27 +++++++++++++++++++--------
 2 files changed, 22 insertions(+), 11 deletions(-)

Index: linux-2.6/Documentation/power/runtime_pm.txt
===================================================================
--- linux-2.6.orig/Documentation/power/runtime_pm.txt
+++ linux-2.6/Documentation/power/runtime_pm.txt
@@ -537,9 +537,9 @@ suspend routine).  It may be necessary t
 in order to do so.  The same is true if the driver uses different power levels
 or other settings for run-time suspend and system sleep.
 
-During system resume, devices generally should be brought back to full power,
-even if they were suspended before the system sleep began.  There are several
-reasons for this, including:
+During system resume, the simplest approach is to bring all devices back to full
+power, even if they had been suspended before the system suspend began.  There
+are several reasons for this, including:
 
   * The device might need to switch power levels, wake-up settings, etc.
 
@@ -556,16 +556,27 @@ reasons for this, including:
   * Even though the device was suspended, if its usage counter was > 0 then most
     likely it would need a run-time resume in the near future anyway.
 
-  * Always going back to full power is simplest.
-
-If the device was suspended before the sleep began, then its run-time PM status
-will have to be updated to reflect the actual post-system sleep status.  The way
-to do this is:
+If the device had been suspended before the system suspend began and it's
+brought back to full power during resume, then its run-time PM status will have
+to be updated to reflect the actual post-system sleep status.  The way to do
+this is:
 
 	pm_runtime_disable(dev);
 	pm_runtime_set_active(dev);
 	pm_runtime_enable(dev);
 
+On some systems, however, system sleep is not entered through a global firmware
+or hardware operation.  Instead, all hardware components are put into low-power
+states directly by the kernel in a coordinated way.  Then, the system sleep
+state effectively follows from the states the hardware components end up in
+and the system is woken up from that state by a hardware interrupt or a similar
+mechanism entirely under the kernel's control.  As a result, the kernel never
+gives control away and the states of all devices during resume are precisely
+known to it.  If that is the case and none of the situations listed above takes
+place (in particular, if the system is not waking up from hibernation), it may
+be more efficient to leave the devices that had been suspended before the system
+suspend began in the suspended state.
+
 7. Generic subsystem callbacks
 
 Subsystems may wish to conserve code space by using the set of generic power
Index: linux-2.6/Documentation/power/devices.txt
===================================================================
--- linux-2.6.orig/Documentation/power/devices.txt
+++ linux-2.6/Documentation/power/devices.txt
@@ -604,7 +604,7 @@ state temporarily, for example so that i
 disabled.  This all depends on the hardware and the design of the subsystem and
 device driver in question.
 
-During system-wide resume from a sleep state it's best to put devices into the
-full-power state, as explained in Documentation/power/runtime_pm.txt.  Refer to
-that document for more information regarding this particular issue as well as
+During system-wide resume from a sleep state it's easiest to put devices into
+the full-power state, as explained in Documentation/power/runtime_pm.txt.  Refer
+to that document for more information regarding this particular issue as well as
 for information on the device runtime power management framework in general.

[PATCH 2/6 v2] PM / Runtime: Return special error code if runtime PM is disabled

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

Some callers of pm_runtime_get_sync() and other runtime PM helper
functions, scsi_autopm_get_host() and scsi_autopm_get_device() in
particular, need to distinguish error codes returned when runtime PM
is disabled (i.e. power.disable_depth is nonzero for the given
device) from error codes returned in other situations.  For this
reason, make the runtime PM helper functions return -EACCES when
power.disable_depth is nonzero and ensure that this error code
won't be returned by them in any other circumstances.  Modify
scsi_autopm_get_host() and scsi_autopm_get_device() to check the
error code returned by pm_runtime_get_sync() and ignore -EACCES.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 Documentation/power/runtime_pm.txt |    6 ++++--
 drivers/base/power/runtime.c       |    9 +++++----
 drivers/scsi/scsi_pm.c             |    8 ++++----
 3 files changed, 13 insertions(+), 10 deletions(-)

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
@@ -135,8 +135,9 @@ static int rpm_check_suspend_allowed(str
 
 	if (dev->power.runtime_error)
 		retval = -EINVAL;
-	else if (atomic_read(&dev->power.usage_count) > 0
-	    || dev->power.disable_depth > 0)
+	else if (dev->power.disable_depth > 0)
+		retval = -EACCES;
+	else if (atomic_read(&dev->power.usage_count) > 0)
 		retval = -EAGAIN;
 	else if (!pm_children_suspended(dev))
 		retval = -EBUSY;
@@ -262,7 +263,7 @@ static int rpm_callback(int (*cb)(struct
 		spin_lock_irq(&dev->power.lock);
 	}
 	dev->power.runtime_error = retval;
-	return retval;
+	return retval != -EACCES ? retval : -EIO;
 }
 
 /**
@@ -458,7 +459,7 @@ static int rpm_resume(struct device *dev
 	if (dev->power.runtime_error)
 		retval = -EINVAL;
 	else if (dev->power.disable_depth > 0)
-		retval = -EAGAIN;
+		retval = -EACCES;
 	if (retval)
 		goto out;
 
Index: linux-2.6/drivers/scsi/scsi_pm.c
===================================================================
--- linux-2.6.orig/drivers/scsi/scsi_pm.c
+++ linux-2.6/drivers/scsi/scsi_pm.c
@@ -144,9 +144,9 @@ int scsi_autopm_get_device(struct scsi_d
 	int	err;
 
 	err = pm_runtime_get_sync(&sdev->sdev_gendev);
-	if (err < 0)
+	if (err < 0 && err !=-EACCES)
 		pm_runtime_put_sync(&sdev->sdev_gendev);
-	else if (err > 0)
+	else
 		err = 0;
 	return err;
 }
@@ -173,9 +173,9 @@ int scsi_autopm_get_host(struct Scsi_Hos
 	int	err;
 
 	err = pm_runtime_get_sync(&shost->shost_gendev);
-	if (err < 0)
+	if (err < 0 && err !=-EACCES)
 		pm_runtime_put_sync(&shost->shost_gendev);
-	else if (err > 0)
+	else
 		err = 0;
 	return err;
 }
Index: linux-2.6/Documentation/power/runtime_pm.txt
===================================================================
--- linux-2.6.orig/Documentation/power/runtime_pm.txt
+++ linux-2.6/Documentation/power/runtime_pm.txt
@@ -291,7 +291,8 @@ drivers/base/power/runtime.c and include
     - execute the subsystem-level suspend callback for the device; returns 0 on
       success, 1 if the device's run-time PM status was already 'suspended', or
       error code on failure, where -EAGAIN or -EBUSY means it is safe to attempt
-      to suspend the device again in future
+      to suspend the device again in future and -EACCES means that
+      'power.disable_depth' is different from 0
 
   int pm_runtime_autosuspend(struct device *dev);
     - same as pm_runtime_suspend() except that the autosuspend delay is taken
@@ -304,7 +305,8 @@ drivers/base/power/runtime.c and include
       success, 1 if the device's run-time PM status was already 'active' or
       error code on failure, where -EAGAIN means it may be safe to attempt to
       resume the device again in future, but 'power.runtime_error' should be
-      checked additionally
+      checked additionally, and -EACCES means that 'power.disable_depth' is
+      different from 0
 
   int pm_request_idle(struct device *dev);
     - submit a request to execute the subsystem-level idle callback for the

[PATCH 3/6 v2] PCI / PM: Detect early wakeup in pci_pm_prepare()

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

A subsequent patch is going to move the invocation of
pm_runtime_barrier() from dpm_prepare() to __device_suspend().
Consequently, early wakeup events resulting from runtime resume
requests for wakeup devices queued up right before system suspend
will only be detected after all of the subsystem-level .prepare()
callbacks have run.  However, the PCI bus type calls
pm_runtime_get_sync() from its pci_pm_prepare() callback routine,
so it would destroy the early wakeup events information regarding PCI
devices.  To prevent this from happening add an early wakeup
detection mechanism, analogous to the one currently in dpm_prepare(),
to pci_pm_prepare().

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/pci/pci-driver.c |   16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

Index: linux-2.6/drivers/pci/pci-driver.c
===================================================================
--- linux-2.6.orig/drivers/pci/pci-driver.c
+++ linux-2.6/drivers/pci/pci-driver.c
@@ -18,6 +18,7 @@
 #include <linux/sched.h>
 #include <linux/cpu.h>
 #include <linux/pm_runtime.h>
+#include <linux/suspend.h>
 #include "pci.h"
 
 struct pci_dynid {
@@ -616,6 +617,19 @@ static int pci_pm_prepare(struct device
 	int error = 0;
 
 	/*
+	 * If a PCI device configured to wake up the system from sleep states
+	 * has been suspended at run time and there's a resume request pending
+	 * for it, this is equivalent to the device signaling wakeup, so the
+	 * system suspend operation should be aborted.
+	 */
+	pm_runtime_get_noresume(dev);
+	if (pm_runtime_barrier(dev) && device_may_wakeup(dev))
+		pm_wakeup_event(dev, 0);
+
+	if (pm_wakeup_pending())
+		return -EBUSY;
+
+	/*
 	 * PCI devices suspended at run time need to be resumed at this
 	 * point, because in general it is necessary to reconfigure them for
 	 * system suspend.  Namely, if the device is supposed to wake up the
@@ -624,7 +638,7 @@ static int pci_pm_prepare(struct device
 	 * system from the sleep state, we'll have to prevent it from signaling
 	 * wake-up.
 	 */
-	pm_runtime_get_sync(dev);
+	pm_runtime_resume(dev);
 
 	if (drv && drv->pm && drv->pm->prepare)
 		error = drv->pm->prepare(dev);

[PATCH 4/6 v2] PM: Limit race conditions between runtime PM and system sleep (v2)

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

One of the roles of the PM core is to prevent different PM callbacks
executed for the same device object from racing with each other.
Unfortunately, after commit e8665002477f0278f84f898145b1f141ba26ee26
(PM: Allow pm_runtime_suspend() to succeed during system suspend)
runtime PM callbacks may be executed concurrently with system
suspend/resume callbacks for the same device.

The main reason for commit e8665002477f0278f84f898145b1f141ba26ee26
was that some subsystems and device drivers wanted to use runtime PM
helpers, pm_runtime_suspend() and pm_runtime_put_sync() in
particular, for carrying out the suspend of devices in their
.suspend() callbacks.  However, as it's been determined recently,
there are multiple reasons not to do so, inlcuding:

 * The caller really doesn't control the runtime PM usage counters,
   because user space can access them through sysfs and effectively
   block runtime PM.  That means using pm_runtime_suspend() or
   pm_runtime_get_sync() to suspend devices during system suspend
   may or may not work.

 * If a driver calls pm_runtime_suspend() from its .suspend()
   callback, it causes the subsystem's .runtime_suspend() callback to
   be executed, which leads to the call sequence:

   subsys->suspend(dev)
      driver->suspend(dev)
         pm_runtime_suspend(dev)
            subsys->runtime_suspend(dev)

   recursive from the subsystem's point of view.  For some subsystems
   that may actually work (e.g. the platform bus type), but for some
   it will fail in a rather spectacular fashion (e.g. PCI).  In each
   case it means a layering violation.

 * Both the subsystem and the driver can provide .suspend_noirq()
   callbacks for system suspend that can do whatever the
   .runtime_suspend() callbacks do just fine, so it really isn't
   necessary to call pm_runtime_suspend() during system suspend.

 * The runtime PM's handling of wakeup devices is usually different
   from the system suspend's one, so .runtime_suspend() may simply be
   inappropriate for system suspend.

 * System suspend is supposed to work even if CONFIG_PM_RUNTIME is
   unset.

 * The runtime PM workqueue is frozen before system suspend, so if
   whatever the driver is going to do during system suspend depends
   on it, that simply won't work.

Still, there is a good reason to allow pm_runtime_resume() to
succeed during system suspend and resume (for instance, some
subsystems and device drivers may legitimately use it to ensure that
their devices are in full-power states before suspending them).
Moreover, there is no reason to prevent runtime PM callbacks from
being executed in parallel with the system suspend/resume .prepare()
and .complete() callbacks and the code removed by commit
e8665002477f0278f84f898145b1f141ba26ee26 went too far in this
respect.  On the other hand, runtime PM callbacks, including
.runtime_resume(), must not be executed during system suspend's
"late" stage of suspending devices and during system resume's "early"
device resume stage.

Taking all of the above into consideration, make the PM core
acquire a runtime PM reference to every device and resume it if
there's a runtime PM resume request pending right before executing
the subsystem-level .suspend() callback for it.  Make the PM core
drop references to all devices right after executing the
subsystem-level .resume() callbacks for them.  Additionally,
make the PM core disable the runtime PM framework for all devices
during system suspend, after executing the subsystem-level .suspend()
callbacks for them, and enable the runtime PM framework for all
devices during system resume, right before executing the
subsystem-level .resume() callbacks for them.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 Documentation/power/runtime_pm.txt |   21 +++++++++++++++++++++
 drivers/base/power/main.c          |   35 +++++++++++++++++++++++------------
 2 files changed, 44 insertions(+), 12 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
@@ -505,6 +505,7 @@ static int legacy_resume(struct device *
 static int device_resume(struct device *dev, pm_message_t state, bool async)
 {
 	int error = 0;
+	bool put = false;
 
 	TRACE_DEVICE(dev);
 	TRACE_RESUME(0);
@@ -521,6 +522,9 @@ static int device_resume(struct device *
 	if (!dev->power.is_suspended)
 		goto Unlock;
 
+	pm_runtime_enable(dev);
+	put = true;
+
 	if (dev->pm_domain) {
 		pm_dev_dbg(dev, state, "power domain ");
 		error = pm_op(dev, &dev->pm_domain->ops, state);
@@ -563,6 +567,10 @@ static int device_resume(struct device *
 	complete_all(&dev->power.completion);
 
 	TRACE_RESUME(error);
+
+	if (put)
+		pm_runtime_put_sync(dev);
+
 	return error;
 }
 
@@ -843,16 +851,22 @@ static int __device_suspend(struct devic
 	int error = 0;
 
 	dpm_wait_for_children(dev, async);
-	device_lock(dev);
 
 	if (async_error)
-		goto Unlock;
+		return 0;
+
+	pm_runtime_get_noresume(dev);
+	if (pm_runtime_barrier(dev) && device_may_wakeup(dev))
+		pm_wakeup_event(dev, 0);
 
 	if (pm_wakeup_pending()) {
+		pm_runtime_put_sync(dev);
 		async_error = -EBUSY;
-		goto Unlock;
+		return 0;
 	}
 
+	device_lock(dev);
+
 	if (dev->pm_domain) {
 		pm_dev_dbg(dev, state, "power domain ");
 		error = pm_op(dev, &dev->pm_domain->ops, state);
@@ -890,12 +904,15 @@ static int __device_suspend(struct devic
  End:
 	dev->power.is_suspended = !error;
 
- Unlock:
 	device_unlock(dev);
 	complete_all(&dev->power.completion);
 
-	if (error)
+	if (error) {
+		pm_runtime_put_sync(dev);
 		async_error = error;
+	} else if (dev->power.is_suspended) {
+		__pm_runtime_disable(dev, false);
+	}
 
 	return error;
 }
@@ -1035,13 +1052,7 @@ int dpm_prepare(pm_message_t state)
 		get_device(dev);
 		mutex_unlock(&dpm_list_mtx);
 
-		pm_runtime_get_noresume(dev);
-		if (pm_runtime_barrier(dev) && device_may_wakeup(dev))
-			pm_wakeup_event(dev, 0);
-
-		pm_runtime_put_sync(dev);
-		error = pm_wakeup_pending() ?
-				-EBUSY : device_prepare(dev, state);
+		error = device_prepare(dev, state);
 
 		mutex_lock(&dpm_list_mtx);
 		if (error) {
Index: linux-2.6/Documentation/power/runtime_pm.txt
===================================================================
--- linux-2.6.orig/Documentation/power/runtime_pm.txt
+++ linux-2.6/Documentation/power/runtime_pm.txt
@@ -567,6 +567,13 @@ this is:
 	pm_runtime_set_active(dev);
 	pm_runtime_enable(dev);
 
+The PM core always increments the run-time usage counter before calling the
+->suspend() callback and decrements it after calling the ->resume() callback.
+Hence disabling run-time PM temporarily like this will not cause any runtime
+suspend attempts to be permanently lost.  If the usage count goes to zero
+following the return of the ->resume() callback, the ->runtime_idle() callback
+will be invoked as usual.
+
 On some systems, however, system sleep is not entered through a global firmware
 or hardware operation.  Instead, all hardware components are put into low-power
 states directly by the kernel in a coordinated way.  Then, the system sleep
@@ -579,6 +586,20 @@ place (in particular, if the system is n
 be more efficient to leave the devices that had been suspended before the system
 suspend began in the suspended state.
 
+The PM core does its best to reduce the probability of race conditions between
+the runtime PM and system suspend/resume (and hibernation) callbacks by carrying
+out the following operations:
+
+  * During system suspend it calls pm_runtime_get_noresume() and
+    pm_runtime_barrier() for every device right before executing the
+    subsystem-level .suspend() callback for it.  In addition to that it calls
+    pm_runtime_disable() for every device right after executing the
+    subsystem-level .suspend() callback for it.
+
+  * During system resume it calls pm_runtime_enable() and pm_runtime_put_sync()
+    for every device right before and right after executing the subsystem-level
+    .resume() callback for it, respectively.
+
 7. Generic subsystem callbacks
 
 Subsystems may wish to conserve code space by using the set of generic power

[PATCH 5/6 v2] PM / Runtime: Improve documentation of enable, disable and barrier

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

The runtime PM documentation in Documentation/power/runtime_pm.txt
doesn't say that pm_runtime_enable() and pm_runtime_disable() work by
operating on power.disable_depth, which is wrong, because the
possibility of nesting disables doesn't follow from the description
of these functions.  Also, there is no description of
pm_runtime_barrier() at all in the document, which is confusing.
Improve the documentation by fixing those issues.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 Documentation/power/runtime_pm.txt |   18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

Index: linux-2.6/Documentation/power/runtime_pm.txt
===================================================================
--- linux-2.6.orig/Documentation/power/runtime_pm.txt
+++ linux-2.6/Documentation/power/runtime_pm.txt
@@ -369,17 +369,27 @@ drivers/base/power/runtime.c and include
       pm_runtime_autosuspend(dev) and return its result
 
   void pm_runtime_enable(struct device *dev);
-    - enable the run-time PM helper functions to run the device bus type's
-      run-time PM callbacks described in Section 2
+    - decrement the device's 'power.disable_depth' field; if that field is equal
+      to zero, the run-time PM helper functions can execute subsystem-level
+      callbacks described in Section 2 for the device
 
   int pm_runtime_disable(struct device *dev);
-    - prevent the run-time PM helper functions from running subsystem-level
-      run-time PM callbacks for the device, make sure that all of the pending
+    - increment the device's 'power.disable_depth' field (if the value of that
+      field was previously zero, this prevents subsystem-level runtime PM
+      callbacks from being run for the device), make sure that all of the pending
       run-time PM operations on the device are either completed or canceled;
       returns 1 if there was a resume request pending and it was necessary to
       execute the subsystem-level resume callback for the device to satisfy that
       request, otherwise 0 is returned
 
+  int pm_runtime_barrier(struct device *dev);
+    - check if there's a resume request pending for the device and resume it
+      (synchronously) in that case, cancel any other pending runtime PM requests
+      regarding it and wait for all runtime PM operations on it in progress to
+      complete; returns 1 if there was a resume request pending and it was
+      necessary to execute the subsystem-level resume callback for the device to
+      satisfy that request, otherwise 0 is returned
+
   void pm_suspend_ignore_children(struct device *dev, bool enable);
     - set/unset the power.ignore_children flag of the device
 

[PATCH 6/6 v2] PM / Runtime: Replace "run-time" with "runtime" in documentation

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

The runtime PM documentation and kerneldoc comments sometimes spell
"runtime" with a dash (i.e. "run-time").  Replace all of those
instances with "runtime" to make the naming consistent.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 Documentation/power/runtime_pm.txt |  130 ++++++++++++++++++-------------------
 drivers/base/power/runtime.c       |   66 +++++++++---------
 2 files changed, 98 insertions(+), 98 deletions(-)

Index: linux-2.6/Documentation/power/runtime_pm.txt
===================================================================
--- linux-2.6.orig/Documentation/power/runtime_pm.txt
+++ linux-2.6/Documentation/power/runtime_pm.txt
@@ -1,39 +1,39 @@
-Run-time Power Management Framework for I/O Devices
+Runtime Power Management Framework for I/O Devices
 
 (C) 2009-2011 Rafael J. Wysocki <rjw@sisk.pl>, Novell Inc.
 (C) 2010 Alan Stern <stern@rowland.harvard.edu>
 
 1. Introduction
 
-Support for run-time power management (run-time PM) of I/O devices is provided
+Support for runtime power management (runtime PM) of I/O devices is provided
 at the power management core (PM core) level by means of:
 
 * The power management workqueue pm_wq in which bus types and device drivers can
   put their PM-related work items.  It is strongly recommended that pm_wq be
-  used for queuing all work items related to run-time PM, because this allows
+  used for queuing all work items related to runtime PM, because this allows
   them to be synchronized with system-wide power transitions (suspend to RAM,
   hibernation and resume from system sleep states).  pm_wq is declared in
   include/linux/pm_runtime.h and defined in kernel/power/main.c.
 
-* A number of run-time PM fields in the 'power' member of 'struct device' (which
+* A number of runtime PM fields in the 'power' member of 'struct device' (which
   is of the type 'struct dev_pm_info', defined in include/linux/pm.h) that can
-  be used for synchronizing run-time PM operations with one another.
+  be used for synchronizing runtime PM operations with one another.
 
-* Three device run-time PM callbacks in 'struct dev_pm_ops' (defined in
+* Three device runtime PM callbacks in 'struct dev_pm_ops' (defined in
   include/linux/pm.h).
 
 * A set of helper functions defined in drivers/base/power/runtime.c that can be
-  used for carrying out run-time PM operations in such a way that the
+  used for carrying out runtime PM operations in such a way that the
   synchronization between them is taken care of by the PM core.  Bus types and
   device drivers are encouraged to use these functions.
 
-The run-time PM callbacks present in 'struct dev_pm_ops', the device run-time PM
+The runtime PM callbacks present in 'struct dev_pm_ops', the device runtime PM
 fields of 'struct dev_pm_info' and the core helper functions provided for
-run-time PM are described below.
+runtime PM are described below.
 
-2. Device Run-time PM Callbacks
+2. Device Runtime PM Callbacks
 
-There are three device run-time PM callbacks defined in 'struct dev_pm_ops':
+There are three device runtime PM callbacks defined in 'struct dev_pm_ops':
 
 struct dev_pm_ops {
 	...
@@ -72,11 +72,11 @@ knows what to do to handle the device).
     not mean that the device has been put into a low power state.  It is
     supposed to mean, however, that the device will not process data and will
     not communicate with the CPU(s) and RAM until the subsystem-level resume
-    callback is executed for it.  The run-time PM status of a device after
+    callback is executed for it.  The runtime PM status of a device after
     successful execution of the subsystem-level suspend callback is 'suspended'.
 
   * If the subsystem-level suspend callback returns -EBUSY or -EAGAIN,
-    the device's run-time PM status is 'active', which means that the device
+    the device's runtime PM status is 'active', which means that the device
     _must_ be fully operational afterwards.
 
   * If the subsystem-level suspend callback returns an error code different
@@ -104,7 +104,7 @@ the device).
 
   * Once the subsystem-level resume callback has completed successfully, the PM
     core regards the device as fully operational, which means that the device
-    _must_ be able to complete I/O operations as needed.  The run-time PM status
+    _must_ be able to complete I/O operations as needed.  The runtime PM status
     of the device is then 'active'.
 
   * If the subsystem-level resume callback returns an error code, the PM core
@@ -130,7 +130,7 @@ device in that case.  The value returned
 core.
 
 The helper functions provided by the PM core, described in Section 4, guarantee
-that the following constraints are met with respect to the bus type's run-time
+that the following constraints are met with respect to the bus type's runtime
 PM callbacks:
 
 (1) The callbacks are mutually exclusive (e.g. it is forbidden to execute
@@ -142,7 +142,7 @@ PM callbacks:
 
 (2) ->runtime_idle() and ->runtime_suspend() can only be executed for 'active'
     devices (i.e. the PM core will only execute ->runtime_idle() or
-    ->runtime_suspend() for the devices the run-time PM status of which is
+    ->runtime_suspend() for the devices the runtime PM status of which is
     'active').
 
 (3) ->runtime_idle() and ->runtime_suspend() can only be executed for a device
@@ -151,7 +151,7 @@ PM callbacks:
     flag of which is set.
 
 (4) ->runtime_resume() can only be executed for 'suspended' devices  (i.e. the
-    PM core will only execute ->runtime_resume() for the devices the run-time
+    PM core will only execute ->runtime_resume() for the devices the runtime
     PM status of which is 'suspended').
 
 Additionally, the helper functions provided by the PM core obey the following
@@ -171,9 +171,9 @@ rules:
     scheduled requests to execute the other callbacks for the same device,
     except for scheduled autosuspends.
 
-3. Run-time PM Device Fields
+3. Runtime PM Device Fields
 
-The following device run-time PM fields are present in 'struct dev_pm_info', as
+The following device runtime PM fields are present in 'struct dev_pm_info', as
 defined in include/linux/pm.h:
 
   struct timer_list suspend_timer;
@@ -205,7 +205,7 @@ defined in include/linux/pm.h:
 
   unsigned int disable_depth;
     - used for disabling the helper funcions (they work normally if this is
-      equal to zero); the initial value of it is 1 (i.e. run-time PM is
+      equal to zero); the initial value of it is 1 (i.e. runtime PM is
       initially disabled for all devices)
 
   unsigned int runtime_error;
@@ -229,10 +229,10 @@ defined in include/linux/pm.h:
       suspend to complete; means "start a resume as soon as you've suspended"
 
   unsigned int run_wake;
-    - set if the device is capable of generating run-time wake-up events
+    - set if the device is capable of generating runtime wake-up events
 
   enum rpm_status runtime_status;
-    - the run-time PM status of the device; this field's initial value is
+    - the runtime PM status of the device; this field's initial value is
       RPM_SUSPENDED, which means that each device is initially regarded by the
       PM core as 'suspended', regardless of its real hardware status
 
@@ -243,7 +243,7 @@ defined in include/linux/pm.h:
       and pm_runtime_forbid() helper functions
 
   unsigned int no_callbacks;
-    - indicates that the device does not use the run-time PM callbacks (see
+    - indicates that the device does not use the runtime PM callbacks (see
       Section 8); it may be modified only by the pm_runtime_no_callbacks()
       helper function
 
@@ -270,16 +270,16 @@ defined in include/linux/pm.h:
 
 All of the above fields are members of the 'power' member of 'struct device'.
 
-4. Run-time PM Device Helper Functions
+4. Runtime PM Device Helper Functions
 
-The following run-time PM helper functions are defined in
+The following runtime PM helper functions are defined in
 drivers/base/power/runtime.c and include/linux/pm_runtime.h:
 
   void pm_runtime_init(struct device *dev);
-    - initialize the device run-time PM fields in 'struct dev_pm_info'
+    - initialize the device runtime PM fields in 'struct dev_pm_info'
 
   void pm_runtime_remove(struct device *dev);
-    - make sure that the run-time PM of the device will be disabled after
+    - make sure that the runtime PM of the device will be disabled after
       removing the device from device hierarchy
 
   int pm_runtime_idle(struct device *dev);
@@ -289,7 +289,7 @@ drivers/base/power/runtime.c and include
 
   int pm_runtime_suspend(struct device *dev);
     - execute the subsystem-level suspend callback for the device; returns 0 on
-      success, 1 if the device's run-time PM status was already 'suspended', or
+      success, 1 if the device's runtime PM status was already 'suspended', or
       error code on failure, where -EAGAIN or -EBUSY means it is safe to attempt
       to suspend the device again in future and -EACCES means that
       'power.disable_depth' is different from 0
@@ -302,7 +302,7 @@ drivers/base/power/runtime.c and include
 
   int pm_runtime_resume(struct device *dev);
     - execute the subsystem-level resume callback for the device; returns 0 on
-      success, 1 if the device's run-time PM status was already 'active' or
+      success, 1 if the device's runtime PM status was already 'active' or
       error code on failure, where -EAGAIN means it may be safe to attempt to
       resume the device again in future, but 'power.runtime_error' should be
       checked additionally, and -EACCES means that 'power.disable_depth' is
@@ -323,7 +323,7 @@ drivers/base/power/runtime.c and include
       device in future, where 'delay' is the time to wait before queuing up a
       suspend work item in pm_wq, in milliseconds (if 'delay' is zero, the work
       item is queued up immediately); returns 0 on success, 1 if the device's PM
-      run-time status was already 'suspended', or error code if the request
+      runtime status was already 'suspended', or error code if the request
       hasn't been scheduled (or queued up if 'delay' is 0); if the execution of
       ->runtime_suspend() is already scheduled and not yet expired, the new
       value of 'delay' will be used as the time to wait
@@ -331,7 +331,7 @@ drivers/base/power/runtime.c and include
   int pm_request_resume(struct device *dev);
     - submit a request to execute the subsystem-level resume callback for the
       device (the request is represented by a work item in pm_wq); returns 0 on
-      success, 1 if the device's run-time PM status was already 'active', or
+      success, 1 if the device's runtime PM status was already 'active', or
       error code if the request hasn't been queued up
 
   void pm_runtime_get_noresume(struct device *dev);
@@ -370,14 +370,14 @@ drivers/base/power/runtime.c and include
 
   void pm_runtime_enable(struct device *dev);
     - decrement the device's 'power.disable_depth' field; if that field is equal
-      to zero, the run-time PM helper functions can execute subsystem-level
+      to zero, the runtime PM helper functions can execute subsystem-level
       callbacks described in Section 2 for the device
 
   int pm_runtime_disable(struct device *dev);
     - increment the device's 'power.disable_depth' field (if the value of that
       field was previously zero, this prevents subsystem-level runtime PM
       callbacks from being run for the device), make sure that all of the pending
-      run-time PM operations on the device are either completed or canceled;
+      runtime PM operations on the device are either completed or canceled;
       returns 1 if there was a resume request pending and it was necessary to
       execute the subsystem-level resume callback for the device to satisfy that
       request, otherwise 0 is returned
@@ -394,7 +394,7 @@ drivers/base/power/runtime.c and include
     - set/unset the power.ignore_children flag of the device
 
   int pm_runtime_set_active(struct device *dev);
-    - clear the device's 'power.runtime_error' flag, set the device's run-time
+    - clear the device's 'power.runtime_error' flag, set the device's runtime
       PM status to 'active' and update its parent's counter of 'active'
       children as appropriate (it is only valid to use this function if
       'power.runtime_error' is set or 'power.disable_depth' is greater than
@@ -402,7 +402,7 @@ drivers/base/power/runtime.c and include
       which is not active and the 'power.ignore_children' flag of which is unset
 
   void pm_runtime_set_suspended(struct device *dev);
-    - clear the device's 'power.runtime_error' flag, set the device's run-time
+    - clear the device's 'power.runtime_error' flag, set the device's runtime
       PM status to 'suspended' and update its parent's counter of 'active'
       children as appropriate (it is only valid to use this function if
       'power.runtime_error' is set or 'power.disable_depth' is greater than
@@ -423,7 +423,7 @@ drivers/base/power/runtime.c and include
       effectively prevent the device from being power managed at run time)
 
   void pm_runtime_no_callbacks(struct device *dev);
-    - set the power.no_callbacks flag for the device and remove the run-time
+    - set the power.no_callbacks flag for the device and remove the runtime
       PM attributes from /sys/devices/.../power (or prevent them from being
       added when the device is registered)
 
@@ -443,7 +443,7 @@ drivers/base/power/runtime.c and include
 
   void pm_runtime_set_autosuspend_delay(struct device *dev, int delay);
     - set the power.autosuspend_delay value to 'delay' (expressed in
-      milliseconds); if 'delay' is negative then run-time suspends are
+      milliseconds); if 'delay' is negative then runtime suspends are
       prevented
 
   unsigned long pm_runtime_autosuspend_expiration(struct device *dev);
@@ -482,35 +482,35 @@ pm_runtime_resume()
 pm_runtime_get_sync()
 pm_runtime_put_sync_suspend()
 
-5. Run-time PM Initialization, Device Probing and Removal
+5. Runtime PM Initialization, Device Probing and Removal
 
-Initially, the run-time PM is disabled for all devices, which means that the
-majority of the run-time PM helper funtions described in Section 4 will return
+Initially, the runtime PM is disabled for all devices, which means that the
+majority of the runtime PM helper funtions described in Section 4 will return
 -EAGAIN until pm_runtime_enable() is called for the device.
 
-In addition to that, the initial run-time PM status of all devices is
+In addition to that, the initial runtime PM status of all devices is
 'suspended', but it need not reflect the actual physical state of the device.
 Thus, if the device is initially active (i.e. it is able to process I/O), its
-run-time PM status must be changed to 'active', with the help of
+runtime PM status must be changed to 'active', with the help of
 pm_runtime_set_active(), before pm_runtime_enable() is called for the device.
 
-However, if the device has a parent and the parent's run-time PM is enabled,
+However, if the device has a parent and the parent's runtime PM is enabled,
 calling pm_runtime_set_active() for the device will affect the parent, unless
 the parent's 'power.ignore_children' flag is set.  Namely, in that case the
 parent won't be able to suspend at run time, using the PM core's helper
 functions, as long as the child's status is 'active', even if the child's
-run-time PM is still disabled (i.e. pm_runtime_enable() hasn't been called for
+runtime PM is still disabled (i.e. pm_runtime_enable() hasn't been called for
 the child yet or pm_runtime_disable() has been called for it).  For this reason,
 once pm_runtime_set_active() has been called for the device, pm_runtime_enable()
-should be called for it too as soon as reasonably possible or its run-time PM
+should be called for it too as soon as reasonably possible or its runtime PM
 status should be changed back to 'suspended' with the help of
 pm_runtime_set_suspended().
 
-If the default initial run-time PM status of the device (i.e. 'suspended')
+If the default initial runtime PM status of the device (i.e. 'suspended')
 reflects the actual state of the device, its bus type's or its driver's
 ->probe() callback will likely need to wake it up using one of the PM core's
 helper functions described in Section 4.  In that case, pm_runtime_resume()
-should be used.  Of course, for this purpose the device's run-time PM has to be
+should be used.  Of course, for this purpose the device's runtime PM has to be
 enabled earlier by calling pm_runtime_enable().
 
 If the device bus type's or driver's ->probe() or ->remove() callback runs
@@ -525,29 +525,29 @@ The user space can effectively disallow
 it at run time by changing the value of its /sys/devices/.../power/control
 attribute to "on", which causes pm_runtime_forbid() to be called.  In principle,
 this mechanism may also be used by the driver to effectively turn off the
-run-time power management of the device until the user space turns it on.
-Namely, during the initialization the driver can make sure that the run-time PM
+runtime power management of the device until the user space turns it on.
+Namely, during the initialization the driver can make sure that the runtime PM
 status of the device is 'active' and call pm_runtime_forbid().  It should be
 noted, however, that if the user space has already intentionally changed the
 value of /sys/devices/.../power/control to "auto" to allow the driver to power
 manage the device at run time, the driver may confuse it by using
 pm_runtime_forbid() this way.
 
-6. Run-time PM and System Sleep
+6. Runtime PM and System Sleep
 
-Run-time PM and system sleep (i.e., system suspend and hibernation, also known
+Runtime PM and system sleep (i.e., system suspend and hibernation, also known
 as suspend-to-RAM and suspend-to-disk) interact with each other in a couple of
 ways.  If a device is active when a system sleep starts, everything is
 straightforward.  But what should happen if the device is already suspended?
 
-The device may have different wake-up settings for run-time PM and system sleep.
-For example, remote wake-up may be enabled for run-time suspend but disallowed
+The device may have different wake-up settings for runtime PM and system sleep.
+For example, remote wake-up may be enabled for runtime suspend but disallowed
 for system sleep (device_may_wakeup(dev) returns 'false').  When this happens,
 the subsystem-level system suspend callback is responsible for changing the
 device's wake-up setting (it may leave that to the device driver's system
 suspend routine).  It may be necessary to resume the device and suspend it again
 in order to do so.  The same is true if the driver uses different power levels
-or other settings for run-time suspend and system sleep.
+or other settings for runtime suspend and system sleep.
 
 During system resume, the simplest approach is to bring all devices back to full
 power, even if they had been suspended before the system suspend began.  There
@@ -566,10 +566,10 @@ are several reasons for this, including:
   * The device might need to be reset.
 
   * Even though the device was suspended, if its usage counter was > 0 then most
-    likely it would need a run-time resume in the near future anyway.
+    likely it would need a runtime resume in the near future anyway.
 
 If the device had been suspended before the system suspend began and it's
-brought back to full power during resume, then its run-time PM status will have
+brought back to full power during resume, then its runtime PM status will have
 to be updated to reflect the actual post-system sleep status.  The way to do
 this is:
 
@@ -577,9 +577,9 @@ this is:
 	pm_runtime_set_active(dev);
 	pm_runtime_enable(dev);
 
-The PM core always increments the run-time usage counter before calling the
+The PM core always increments the runtime usage counter before calling the
 ->suspend() callback and decrements it after calling the ->resume() callback.
-Hence disabling run-time PM temporarily like this will not cause any runtime
+Hence disabling runtime PM temporarily like this will not cause any runtime
 suspend attempts to be permanently lost.  If the usage count goes to zero
 following the return of the ->resume() callback, the ->runtime_idle() callback
 will be invoked as usual.
@@ -694,8 +694,8 @@ the GENERIC_SUBSYS_PM_OPS macro, defined
 dev_pm_ops structure pointer.
 
 Device drivers that wish to use the same function as a system suspend, freeze,
-poweroff and run-time suspend callback, and similarly for system resume, thaw,
-restore, and run-time resume, can achieve this with the help of the
+poweroff and runtime suspend callback, and similarly for system resume, thaw,
+restore, and runtime resume, can achieve this with the help of the
 UNIVERSAL_DEV_PM_OPS macro defined in include/linux/pm.h (possibly setting its
 last argument to NULL).
 
@@ -705,7 +705,7 @@ Some "devices" are only logical sub-devi
 power-managed on their own.  (The prototype example is a USB interface.  Entire
 USB devices can go into low-power mode or send wake-up requests, but neither is
 possible for individual interfaces.)  The drivers for these devices have no
-need of run-time PM callbacks; if the callbacks did exist, ->runtime_suspend()
+need of runtime PM callbacks; if the callbacks did exist, ->runtime_suspend()
 and ->runtime_resume() would always return 0 without doing anything else and
 ->runtime_idle() would always call pm_runtime_suspend().
 
@@ -713,7 +713,7 @@ Subsystems can tell the PM core about th
 pm_runtime_no_callbacks().  This should be done after the device structure is
 initialized and before it is registered (although after device registration is
 also okay).  The routine will set the device's power.no_callbacks flag and
-prevent the non-debugging run-time PM sysfs attributes from being created.
+prevent the non-debugging runtime PM sysfs attributes from being created.
 
 When power.no_callbacks is set, the PM core will not invoke the
 ->runtime_idle(), ->runtime_suspend(), or ->runtime_resume() callbacks.
@@ -721,7 +721,7 @@ Instead it will assume that suspends and
 devices should be suspended.
 
 As a consequence, the PM core will never directly inform the device's subsystem
-or driver about run-time power changes.  Instead, the driver for the device's
+or driver about runtime power changes.  Instead, the driver for the device's
 parent must take responsibility for telling the device's driver when the
 parent's power state changes.
 
@@ -732,13 +732,13 @@ A device should be put in a low-power st
 think it will remain in that state for a substantial time.  A common heuristic
 says that a device which hasn't been used for a while is liable to remain
 unused; following this advice, drivers should not allow devices to be suspended
-at run-time until they have been inactive for some minimum period.  Even when
+at runtime until they have been inactive for some minimum period.  Even when
 the heuristic ends up being non-optimal, it will still prevent devices from
 "bouncing" too rapidly between low-power and full-power states.
 
 The term "autosuspend" is an historical remnant.  It doesn't mean that the
 device is automatically suspended (the subsystem or driver still has to call
-the appropriate PM routines); rather it means that run-time suspends will
+the appropriate PM routines); rather it means that runtime suspends will
 automatically be delayed until the desired period of inactivity has elapsed.
 
 Inactivity is determined based on the power.last_busy field.  Drivers should
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
@@ -1,5 +1,5 @@
 /*
- * drivers/base/power/runtime.c - Helper functions for device run-time PM
+ * drivers/base/power/runtime.c - Helper functions for device runtime PM
  *
  * Copyright (c) 2009 Rafael J. Wysocki <rjw@sisk.pl>, Novell Inc.
  * Copyright (C) 2010 Alan Stern <stern@rowland.harvard.edu>
@@ -159,7 +159,7 @@ static int rpm_check_suspend_allowed(str
  * @dev: Device to notify the bus type about.
  * @rpmflags: Flag bits.
  *
- * Check if the device's run-time PM status allows it to be suspended.  If
+ * Check if the device's runtime PM status allows it to be suspended.  If
  * another idle notification has been started earlier, return immediately.  If
  * the RPM_ASYNC flag is set then queue an idle-notification request; otherwise
  * run the ->runtime_idle() callback directly.
@@ -267,11 +267,11 @@ static int rpm_callback(int (*cb)(struct
 }
 
 /**
- * rpm_suspend - Carry out run-time suspend of given device.
+ * rpm_suspend - Carry out runtime suspend of given device.
  * @dev: Device to suspend.
  * @rpmflags: Flag bits.
  *
- * Check if the device's run-time PM status allows it to be suspended.  If
+ * Check if the device's runtime PM status allows it to be suspended.  If
  * another suspend has been started earlier, either return immediately or wait
  * for it to finish, depending on the RPM_NOWAIT and RPM_ASYNC flags.  Cancel a
  * pending idle notification.  If the RPM_ASYNC flag is set then queue a
@@ -430,11 +430,11 @@ static int rpm_suspend(struct device *de
 }
 
 /**
- * rpm_resume - Carry out run-time resume of given device.
+ * rpm_resume - Carry out runtime resume of given device.
  * @dev: Device to resume.
  * @rpmflags: Flag bits.
  *
- * Check if the device's run-time PM status allows it to be resumed.  Cancel
+ * Check if the device's runtime PM status allows it to be resumed.  Cancel
  * any scheduled or pending requests.  If another resume has been started
  * earlier, either return immediately or wait for it to finish, depending on the
  * RPM_NOWAIT and RPM_ASYNC flags.  Similarly, if there's a suspend running in
@@ -551,7 +551,7 @@ static int rpm_resume(struct device *dev
 
 		spin_lock(&parent->power.lock);
 		/*
-		 * We can resume if the parent's run-time PM is disabled or it
+		 * We can resume if the parent's runtime PM is disabled or it
 		 * is set to ignore children.
 		 */
 		if (!parent->power.disable_depth
@@ -615,11 +615,11 @@ static int rpm_resume(struct device *dev
 }
 
 /**
- * pm_runtime_work - Universal run-time PM work function.
+ * pm_runtime_work - Universal runtime PM work function.
  * @work: Work structure used for scheduling the execution of this function.
  *
  * Use @work to get the device object the work is to be done for, determine what
- * is to be done and execute the appropriate run-time PM function.
+ * is to be done and execute the appropriate runtime PM function.
  */
 static void pm_runtime_work(struct work_struct *work)
 {
@@ -718,7 +718,7 @@ int pm_schedule_suspend(struct device *d
 EXPORT_SYMBOL_GPL(pm_schedule_suspend);
 
 /**
- * __pm_runtime_idle - Entry point for run-time idle operations.
+ * __pm_runtime_idle - Entry point for runtime idle operations.
  * @dev: Device to send idle notification for.
  * @rpmflags: Flag bits.
  *
@@ -747,7 +747,7 @@ int __pm_runtime_idle(struct device *dev
 EXPORT_SYMBOL_GPL(__pm_runtime_idle);
 
 /**
- * __pm_runtime_suspend - Entry point for run-time put/suspend operations.
+ * __pm_runtime_suspend - Entry point for runtime put/suspend operations.
  * @dev: Device to suspend.
  * @rpmflags: Flag bits.
  *
@@ -776,7 +776,7 @@ int __pm_runtime_suspend(struct device *
 EXPORT_SYMBOL_GPL(__pm_runtime_suspend);
 
 /**
- * __pm_runtime_resume - Entry point for run-time resume operations.
+ * __pm_runtime_resume - Entry point for runtime resume operations.
  * @dev: Device to resume.
  * @rpmflags: Flag bits.
  *
@@ -802,11 +802,11 @@ int __pm_runtime_resume(struct device *d
 EXPORT_SYMBOL_GPL(__pm_runtime_resume);
 
 /**
- * __pm_runtime_set_status - Set run-time PM status of a device.
+ * __pm_runtime_set_status - Set runtime PM status of a device.
  * @dev: Device to handle.
- * @status: New run-time PM status of the device.
+ * @status: New runtime PM status of the device.
  *
- * If run-time PM of the device is disabled or its power.runtime_error field is
+ * If runtime PM of the device is disabled or its power.runtime_error field is
  * different from zero, the status may be changed either to RPM_ACTIVE, or to
  * RPM_SUSPENDED, as long as that reflects the actual state of the device.
  * However, if the device has a parent and the parent is not active, and the
@@ -852,7 +852,7 @@ int __pm_runtime_set_status(struct devic
 
 		/*
 		 * It is invalid to put an active child under a parent that is
-		 * not active, has run-time PM enabled and the
+		 * not active, has runtime PM enabled and the
 		 * 'power.ignore_children' flag unset.
 		 */
 		if (!parent->power.disable_depth
@@ -886,7 +886,7 @@ EXPORT_SYMBOL_GPL(__pm_runtime_set_statu
  * @dev: Device to handle.
  *
  * Flush all pending requests for the device from pm_wq and wait for all
- * run-time PM operations involving the device in progress to complete.
+ * runtime PM operations involving the device in progress to complete.
  *
  * Should be called under dev->power.lock with interrupts disabled.
  */
@@ -934,7 +934,7 @@ static void __pm_runtime_barrier(struct
  * Prevent the device from being suspended by incrementing its usage counter and
  * if there's a pending resume request for the device, wake the device up.
  * Next, make sure that all pending requests for the device have been flushed
- * from pm_wq and wait for all run-time PM operations involving the device in
+ * from pm_wq and wait for all runtime PM operations involving the device in
  * progress to complete.
  *
  * Return value:
@@ -964,18 +964,18 @@ int pm_runtime_barrier(struct device *de
 EXPORT_SYMBOL_GPL(pm_runtime_barrier);
 
 /**
- * __pm_runtime_disable - Disable run-time PM of a device.
+ * __pm_runtime_disable - Disable runtime PM of a device.
  * @dev: Device to handle.
  * @check_resume: If set, check if there's a resume request for the device.
  *
  * Increment power.disable_depth for the device and if was zero previously,
- * cancel all pending run-time PM requests for the device and wait for all
+ * cancel all pending runtime PM requests for the device and wait for all
  * operations in progress to complete.  The device can be either active or
- * suspended after its run-time PM has been disabled.
+ * suspended after its runtime PM has been disabled.
  *
  * If @check_resume is set and there's a resume request pending when
  * __pm_runtime_disable() is called and power.disable_depth is zero, the
- * function will wake up the device before disabling its run-time PM.
+ * function will wake up the device before disabling its runtime PM.
  */
 void __pm_runtime_disable(struct device *dev, bool check_resume)
 {
@@ -988,7 +988,7 @@ void __pm_runtime_disable(struct device
 
 	/*
 	 * Wake up the device if there's a resume request pending, because that
-	 * means there probably is some I/O to process and disabling run-time PM
+	 * means there probably is some I/O to process and disabling runtime PM
 	 * shouldn't prevent the device from processing the I/O.
 	 */
 	if (check_resume && dev->power.request_pending
@@ -1013,7 +1013,7 @@ void __pm_runtime_disable(struct device
 EXPORT_SYMBOL_GPL(__pm_runtime_disable);
 
 /**
- * pm_runtime_enable - Enable run-time PM of a device.
+ * pm_runtime_enable - Enable runtime PM of a device.
  * @dev: Device to handle.
  */
 void pm_runtime_enable(struct device *dev)
@@ -1032,7 +1032,7 @@ void pm_runtime_enable(struct device *de
 EXPORT_SYMBOL_GPL(pm_runtime_enable);
 
 /**
- * pm_runtime_forbid - Block run-time PM of a device.
+ * pm_runtime_forbid - Block runtime PM of a device.
  * @dev: Device to handle.
  *
  * Increase the device's usage count and clear its power.runtime_auto flag,
@@ -1055,7 +1055,7 @@ void pm_runtime_forbid(struct device *de
 EXPORT_SYMBOL_GPL(pm_runtime_forbid);
 
 /**
- * pm_runtime_allow - Unblock run-time PM of a device.
+ * pm_runtime_allow - Unblock runtime PM of a device.
  * @dev: Device to handle.
  *
  * Decrease the device's usage count and set its power.runtime_auto flag.
@@ -1076,12 +1076,12 @@ void pm_runtime_allow(struct device *dev
 EXPORT_SYMBOL_GPL(pm_runtime_allow);
 
 /**
- * pm_runtime_no_callbacks - Ignore run-time PM callbacks for a device.
+ * pm_runtime_no_callbacks - Ignore runtime PM callbacks for a device.
  * @dev: Device to handle.
  *
  * Set the power.no_callbacks flag, which tells the PM core that this
- * device is power-managed through its parent and has no run-time PM
- * callbacks of its own.  The run-time sysfs attributes will be removed.
+ * device is power-managed through its parent and has no runtime PM
+ * callbacks of its own.  The runtime sysfs attributes will be removed.
  */
 void pm_runtime_no_callbacks(struct device *dev)
 {
@@ -1157,8 +1157,8 @@ static void update_autosuspend(struct de
  * @delay: Value of the new delay in milliseconds.
  *
  * Set the device's power.autosuspend_delay value.  If it changes to negative
- * and the power.use_autosuspend flag is set, prevent run-time suspends.  If it
- * changes the other way, allow run-time suspends.
+ * and the power.use_autosuspend flag is set, prevent runtime suspends.  If it
+ * changes the other way, allow runtime suspends.
  */
 void pm_runtime_set_autosuspend_delay(struct device *dev, int delay)
 {
@@ -1178,7 +1178,7 @@ EXPORT_SYMBOL_GPL(pm_runtime_set_autosus
  * @dev: Device to handle.
  * @use: New value for use_autosuspend.
  *
- * Set the device's power.use_autosuspend flag, and allow or prevent run-time
+ * Set the device's power.use_autosuspend flag, and allow or prevent runtime
  * suspends as needed.
  */
 void __pm_runtime_use_autosuspend(struct device *dev, bool use)
@@ -1195,7 +1195,7 @@ void __pm_runtime_use_autosuspend(struct
 EXPORT_SYMBOL_GPL(__pm_runtime_use_autosuspend);
 
 /**
- * pm_runtime_init - Initialize run-time PM fields in given device object.
+ * pm_runtime_init - Initialize runtime PM fields in given device object.
  * @dev: Device object to initialize.
  */
 void pm_runtime_init(struct device *dev)

Re: [PATCH 4/6 v2] PM: Limit race conditions between runtime PM and system sleep (v2)

[Linus Walleij]

2011/6/29 Rafael J. Wysocki <rjw@sisk.pl>:

> One of the roles of the PM core is to prevent different PM callbacks
> executed for the same device object from racing with each other.
> Unfortunately, after commit e8665002477f0278f84f898145b1f141ba26ee26
> (PM: Allow pm_runtime_suspend() to succeed during system suspend)
> runtime PM callbacks may be executed concurrently with system
> suspend/resume callbacks for the same device.
(...)
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>

A quick question: is there some specific reason why this patch should
not go into the 3.0.y stable releases? We are trying to produce
a runtime PM system of product quality based on 3.0.y and we've
already had to backport this patch ourselves to get things stable.

We have also backported:
PM: Introduce generic "noirq" callback routines for subsystems (v2)
PM / Runtime: Update documentation of interactions with system sleep
PM / Runtime: Add new helper function: pm_runtime_status_suspended()

And now it seems to be sufficient to get this thing going.

Yours,
Linus Walleij

Re: [PATCH 4/6 v2] PM: Limit race conditions between runtime PM and system sleep (v2)

[Rafael J. Wysocki]

On Thursday, October 27, 2011, Linus Walleij wrote:
> 2011/6/29 Rafael J. Wysocki <rjw@sisk.pl>:
> 
> > One of the roles of the PM core is to prevent different PM callbacks
> > executed for the same device object from racing with each other.
> > Unfortunately, after commit e8665002477f0278f84f898145b1f141ba26ee26
> > (PM: Allow pm_runtime_suspend() to succeed during system suspend)
> > runtime PM callbacks may be executed concurrently with system
> > suspend/resume callbacks for the same device.
> (...)
> > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> 
> A quick question: is there some specific reason why this patch should
> not go into the 3.0.y stable releases?
>
> We are trying to produce
> a runtime PM system of product quality based on 3.0.y and we've
> already had to backport this patch ourselves to get things stable.
> 
> We have also backported:
> PM: Introduce generic "noirq" callback routines for subsystems (v2)
> PM / Runtime: Update documentation of interactions with system sleep
> PM / Runtime: Add new helper function: pm_runtime_status_suspended()
> 
> And now it seems to be sufficient to get this thing going.

Well, it isn't a simple fix and it changes the code's behavior quite
significantly, so I thought it might not be a good idea to risk problems
with -stable because of it.  Perhaps let's see how it works out in 3.1
and backport it later if there are no problem reports related to it?

Rafael

Re: [PATCH 4/6 v2] PM: Limit race conditions between runtime PM and system sleep (v2)

[Linus Walleij]

On Thu, Oct 27, 2011 at 10:06 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:

>> We are trying to produce
>> a runtime PM system of product quality based on 3.0.y and we've
>> already had to backport this patch ourselves to get things stable.
>>
>> We have also backported:
>> PM: Introduce generic "noirq" callback routines for subsystems (v2)
>> PM / Runtime: Update documentation of interactions with system sleep
>> PM / Runtime: Add new helper function: pm_runtime_status_suspended()
>>
>> And now it seems to be sufficient to get this thing going.
>
> Well, it isn't a simple fix and it changes the code's behavior quite
> significantly, so I thought it might not be a good idea to risk problems
> with -stable because of it.  Perhaps let's see how it works out in 3.1
> and backport it later if there are no problem reports related to it?

OK no hurries, let's give it a spin in 3.1 for some weeks or so.

Yours,
Linus Walleij

Re: [PATCH 4/6 v2] PM: Limit race conditions between runtime PM and system sleep (v2)

[Greg KH]

On Thu, Oct 27, 2011 at 03:54:20PM +0200, Linus Walleij wrote:
> 2011/6/29 Rafael J. Wysocki <rjw@sisk.pl>:
> 
> > One of the roles of the PM core is to prevent different PM callbacks
> > executed for the same device object from racing with each other.
> > Unfortunately, after commit e8665002477f0278f84f898145b1f141ba26ee26
> > (PM: Allow pm_runtime_suspend() to succeed during system suspend)
> > runtime PM callbacks may be executed concurrently with system
> > suspend/resume callbacks for the same device.
> (...)
> > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> 
> A quick question: is there some specific reason why this patch should
> not go into the 3.0.y stable releases? We are trying to produce
> a runtime PM system of product quality based on 3.0.y and we've
> already had to backport this patch ourselves to get things stable.
> 
> We have also backported:
> PM: Introduce generic "noirq" callback routines for subsystems (v2)
> PM / Runtime: Update documentation of interactions with system sleep
> PM / Runtime: Add new helper function: pm_runtime_status_suspended()
> 
> And now it seems to be sufficient to get this thing going.

So, what specific git commits do you want to see in the 3.0-stable
tree, and in what order should they be applied in?

thanks,

greg k-h

Re: [PATCH 4/6 v2] PM: Limit race conditions between runtime PM and system sleep (v2)

[Linus Walleij]

On Wed, Nov 2, 2011 at 8:14 PM, Greg KH <greg@kroah.com> wrote:

>> We have also backported:
>> PM: Introduce generic "noirq" callback routines for subsystems (v2)
>> PM / Runtime: Update documentation of interactions with system sleep
>> PM / Runtime: Add new helper function: pm_runtime_status_suspended()
>>
>> And now it seems to be sufficient to get this thing going.
>
> So, what specific git commits do you want to see in the 3.0-stable
> tree, and in what order should they be applied in?

So to my untrained eye it looks like it should be applied like this (top to
bottom) using the reverse commit order from the mainline kernel:

e529192 PM: Introduce generic "noirq" callback routines for subsystems (v2)
455716e PM / Runtime: Update documentation of interactions with system sleep
1e2ef05 PM: Limit race conditions between runtime PM and system sleep (v2)
f3393b6 PM / Runtime: Add new helper function: pm_runtime_status_suspended()

So (2) documents the problem, (3) fixes it, whereas (1) and (4) makes
it possible
to write proper _noirq() code that does not race, IIRC.

Yours,
Linus Walleij

Re: [PATCH 4/6 v2] PM: Limit race conditions between runtime PM and system sleep (v2)

[Greg KH]

On Wed, Nov 02, 2011 at 09:06:02PM +0100, Linus Walleij wrote:
> On Wed, Nov 2, 2011 at 8:14 PM, Greg KH <greg@kroah.com> wrote:
> 
> >> We have also backported:
> >> PM: Introduce generic "noirq" callback routines for subsystems (v2)
> >> PM / Runtime: Update documentation of interactions with system sleep
> >> PM / Runtime: Add new helper function: pm_runtime_status_suspended()
> >>
> >> And now it seems to be sufficient to get this thing going.
> >
> > So, what specific git commits do you want to see in the 3.0-stable
> > tree, and in what order should they be applied in?
> 
> So to my untrained eye it looks like it should be applied like this (top to
> bottom) using the reverse commit order from the mainline kernel:
> 
> e529192 PM: Introduce generic "noirq" callback routines for subsystems (v2)
> 455716e PM / Runtime: Update documentation of interactions with system sleep
> 1e2ef05 PM: Limit race conditions between runtime PM and system sleep (v2)
> f3393b6 PM / Runtime: Add new helper function: pm_runtime_status_suspended()
> 
> So (2) documents the problem, (3) fixes it, whereas (1) and (4) makes
> it possible
> to write proper _noirq() code that does not race, IIRC.

But, after this, it's just adding new infrastructure that drivers will
then be able to use.  As I'm not adding new drivers to 3.0, there will
not be any users of this code, so why add it in the first place?  It
doesn't look like this follows the rules of the stable kernel tree at
all, does it?

confused,

greg k-h

Re: [PATCH 4/6 v2] PM: Limit race conditions between runtime PM and system sleep (v2)

[Linus Walleij]

On Wed, Nov 2, 2011 at 9:11 PM, Greg KH <greg@kroah.com> wrote:
> On Wed, Nov 02, 2011 at 09:06:02PM +0100, Linus Walleij wrote:
>> On Wed, Nov 2, 2011 at 8:14 PM, Greg KH <greg@kroah.com> wrote:
>> > So, what specific git commits do you want to see in the 3.0-stable
>> > tree, and in what order should they be applied in?
>>
>> So to my untrained eye it looks like it should be applied like this (top to
>> bottom) using the reverse commit order from the mainline kernel:
>>
>> e529192 PM: Introduce generic "noirq" callback routines for subsystems (v2)
>> 455716e PM / Runtime: Update documentation of interactions with system sleep
>> 1e2ef05 PM: Limit race conditions between runtime PM and system sleep (v2)
>> f3393b6 PM / Runtime: Add new helper function: pm_runtime_status_suspended()
>>
>> So (2) documents the problem, (3) fixes it, whereas (1) and (4) makes
>> it possible
>> to write proper _noirq() code that does not race, IIRC.
>
> But, after this, it's just adding new infrastructure that drivers will
> then be able to use.  As I'm not adding new drivers to 3.0, there will
> not be any users of this code, so why add it in the first place?  It
> doesn't look like this follows the rules of the stable kernel tree at
> all, does it?

We'll have to get some feedback from Rafael, but the way I percieved
it, the patch (2) fixes a very real race, then fixing that makes it
necessary for drivers using runtime_pm to do things differently to
be on the safe side. But none were really fixed when first merging
it.

So on second thought you probably only want patch (2)
"PM: Limit race conditions between runtime PM and system sleep (v2)"
for stable.

Yours,
Linus Walleij

Re: [PATCH 4/6 v2] PM: Limit race conditions between runtime PM and system sleep (v2)

[Greg KH]

On Wed, Nov 02, 2011 at 10:00:39PM +0100, Linus Walleij wrote:
> On Wed, Nov 2, 2011 at 9:11 PM, Greg KH <greg@kroah.com> wrote:
> > On Wed, Nov 02, 2011 at 09:06:02PM +0100, Linus Walleij wrote:
> >> On Wed, Nov 2, 2011 at 8:14 PM, Greg KH <greg@kroah.com> wrote:
> >> > So, what specific git commits do you want to see in the 3.0-stable
> >> > tree, and in what order should they be applied in?
> >>
> >> So to my untrained eye it looks like it should be applied like this (top to
> >> bottom) using the reverse commit order from the mainline kernel:
> >>
> >> e529192 PM: Introduce generic "noirq" callback routines for subsystems (v2)
> >> 455716e PM / Runtime: Update documentation of interactions with system sleep
> >> 1e2ef05 PM: Limit race conditions between runtime PM and system sleep (v2)
> >> f3393b6 PM / Runtime: Add new helper function: pm_runtime_status_suspended()
> >>
> >> So (2) documents the problem, (3) fixes it, whereas (1) and (4) makes
> >> it possible
> >> to write proper _noirq() code that does not race, IIRC.
> >
> > But, after this, it's just adding new infrastructure that drivers will
> > then be able to use.  As I'm not adding new drivers to 3.0, there will
> > not be any users of this code, so why add it in the first place?  It
> > doesn't look like this follows the rules of the stable kernel tree at
> > all, does it?
> 
> We'll have to get some feedback from Rafael, but the way I percieved
> it, the patch (2) fixes a very real race, then fixing that makes it
> necessary for drivers using runtime_pm to do things differently to
> be on the safe side. But none were really fixed when first merging
> it.
> 
> So on second thought you probably only want patch (2)
> "PM: Limit race conditions between runtime PM and system sleep (v2)"
> for stable.

Ok, Rafael, any objection to me applying just this one?

Linus, have you tested the 3.0 kernel with just this patch applied?  If
not, please do so and let me know.  I'll wait for that before doing
anything.

thanks,

greg k-h

Re: [PATCH 4/6 v2] PM: Limit race conditions between runtime PM and system sleep (v2)

[Rafael J. Wysocki]

On Wednesday, November 02, 2011, Greg KH wrote:
> On Wed, Nov 02, 2011 at 10:00:39PM +0100, Linus Walleij wrote:
> > On Wed, Nov 2, 2011 at 9:11 PM, Greg KH <greg@kroah.com> wrote:
> > > On Wed, Nov 02, 2011 at 09:06:02PM +0100, Linus Walleij wrote:
> > >> On Wed, Nov 2, 2011 at 8:14 PM, Greg KH <greg@kroah.com> wrote:
> > >> > So, what specific git commits do you want to see in the 3.0-stable
> > >> > tree, and in what order should they be applied in?
> > >>
> > >> So to my untrained eye it looks like it should be applied like this (top to
> > >> bottom) using the reverse commit order from the mainline kernel:
> > >>
> > >> e529192 PM: Introduce generic "noirq" callback routines for subsystems (v2)
> > >> 455716e PM / Runtime: Update documentation of interactions with system sleep
> > >> 1e2ef05 PM: Limit race conditions between runtime PM and system sleep (v2)
> > >> f3393b6 PM / Runtime: Add new helper function: pm_runtime_status_suspended()
> > >>
> > >> So (2) documents the problem, (3) fixes it, whereas (1) and (4) makes
> > >> it possible
> > >> to write proper _noirq() code that does not race, IIRC.
> > >
> > > But, after this, it's just adding new infrastructure that drivers will
> > > then be able to use.  As I'm not adding new drivers to 3.0, there will
> > > not be any users of this code, so why add it in the first place?  It
> > > doesn't look like this follows the rules of the stable kernel tree at
> > > all, does it?
> > 
> > We'll have to get some feedback from Rafael, but the way I percieved
> > it, the patch (2) fixes a very real race, then fixing that makes it
> > necessary for drivers using runtime_pm to do things differently to
> > be on the safe side. But none were really fixed when first merging
> > it.
> > 
> > So on second thought you probably only want patch (2)
> > "PM: Limit race conditions between runtime PM and system sleep (v2)"
> > for stable.
> 
> Ok, Rafael, any objection to me applying just this one?

Well, I'd prefer not to unless there are reports confirming user problems
being fixed specifically by this patch.  It changes the PM core's behavior
quite significantly and I'm a little afraid of possible regressions that may
result from it.

Thanks,
Rafael

Re: [PATCH 4/6 v2] PM: Limit race conditions between runtime PM and system sleep (v2)

[Greg KH]

On Thu, Nov 03, 2011 at 01:28:52AM +0100, Rafael J. Wysocki wrote:
> On Wednesday, November 02, 2011, Greg KH wrote:
> > On Wed, Nov 02, 2011 at 10:00:39PM +0100, Linus Walleij wrote:
> > > On Wed, Nov 2, 2011 at 9:11 PM, Greg KH <greg@kroah.com> wrote:
> > > > On Wed, Nov 02, 2011 at 09:06:02PM +0100, Linus Walleij wrote:
> > > >> On Wed, Nov 2, 2011 at 8:14 PM, Greg KH <greg@kroah.com> wrote:
> > > >> > So, what specific git commits do you want to see in the 3.0-stable
> > > >> > tree, and in what order should they be applied in?
> > > >>
> > > >> So to my untrained eye it looks like it should be applied like this (top to
> > > >> bottom) using the reverse commit order from the mainline kernel:
> > > >>
> > > >> e529192 PM: Introduce generic "noirq" callback routines for subsystems (v2)
> > > >> 455716e PM / Runtime: Update documentation of interactions with system sleep
> > > >> 1e2ef05 PM: Limit race conditions between runtime PM and system sleep (v2)
> > > >> f3393b6 PM / Runtime: Add new helper function: pm_runtime_status_suspended()
> > > >>
> > > >> So (2) documents the problem, (3) fixes it, whereas (1) and (4) makes
> > > >> it possible
> > > >> to write proper _noirq() code that does not race, IIRC.
> > > >
> > > > But, after this, it's just adding new infrastructure that drivers will
> > > > then be able to use.  As I'm not adding new drivers to 3.0, there will
> > > > not be any users of this code, so why add it in the first place?  It
> > > > doesn't look like this follows the rules of the stable kernel tree at
> > > > all, does it?
> > > 
> > > We'll have to get some feedback from Rafael, but the way I percieved
> > > it, the patch (2) fixes a very real race, then fixing that makes it
> > > necessary for drivers using runtime_pm to do things differently to
> > > be on the safe side. But none were really fixed when first merging
> > > it.
> > > 
> > > So on second thought you probably only want patch (2)
> > > "PM: Limit race conditions between runtime PM and system sleep (v2)"
> > > for stable.
> > 
> > Ok, Rafael, any objection to me applying just this one?
> 
> Well, I'd prefer not to unless there are reports confirming user problems
> being fixed specifically by this patch.  It changes the PM core's behavior
> quite significantly and I'm a little afraid of possible regressions that may
> result from it.

Ok, good reason to not apply it, consider it dropped :)

greg k-h

Re: [PATCH 3/3] PM: Limit race conditions between runtime PM and system sleep

[Rafael J. Wysocki]

On Sunday, June 26, 2011, Alan Stern wrote:
> On Sun, 26 Jun 2011, Rafael J. Wysocki wrote:
> 
> > From: Rafael J. Wysocki <rjw@sisk.pl>
> > 
> > One of the roles of the PM core is to prevent different PM callbacks
> > executed for the same device object from racing with each other.
> > Unfortunately, after commit e8665002477f0278f84f898145b1f141ba26ee26
> > (PM: Allow pm_runtime_suspend() to succeed during system suspend)
> > runtime PM callbacks may be executed concurrently with system
> > suspend/resume callbacks for the same device.
> 
> ...
> 
> > Index: linux-2.6/Documentation/power/runtime_pm.txt
> > ===================================================================
> > --- linux-2.6.orig/Documentation/power/runtime_pm.txt
> > +++ linux-2.6/Documentation/power/runtime_pm.txt
> 
> > +The PM core does its best to reduce the probability of race conditions between
> > +the runtime PM and system suspend/resume (and hibernation) callbacks by carrying
> > +out the following operations:
> > +
> > +  * During system suspend it acquires a runtime PM reference to every device
> > +    and resume it if there's a runtime PM resume request pending right before
> > +    executing the subsystem-level .suspend() callback for it.  In addition to
> > +    that it disables the runtime PM framework for every device right after
> > +    executing the subsystem-level .suspend() callback for it.
> > +
> > +  * During system resume it enables the runtime PM framework for all devices
> > +    right before executing the subsystem-level .resume() callbacks for them.
> > +    Additionally, it drops references to all devices right after executing the
> > +    subsystem-level .resume() callbacks for them.
> 
> I think it would be better to be a little more specific here.  Instead 
> of "acquires a runtime PM reference", say "calls 
> pm_runtime_get_noresume()".  Or at least, say "increments the 
> run-time usage counter".
> 
> Likewise, instead of "disables the runtime PM framework", say "calls 
> pm_runtime_disable()" or at least "increments power.disable_depth".

OK, updated patch is appended.

> Hmmm, come to think of it...  The documentation for pm_runtime_enable() 
> and pm_runtime_disable() fails to mention power.disable_depth, which is 
> a surprising omission.  In particular, the description of 
> pm_runtime_enable() is wrong because it ignores the possibility of 
> nested disables.

That's a good point.  There seems to be no description of pm_runtime_barrier()
too.  I'll prepare a separate patch for those issues.

Thanks,
Rafael

---
From: Rafael J. Wysocki <rjw@sisk.pl>
Subject: PM: Limit race conditions between runtime PM and system sleep

One of the roles of the PM core is to prevent different PM callbacks
executed for the same device object from racing with each other.
Unfortunately, after commit e8665002477f0278f84f898145b1f141ba26ee26
(PM: Allow pm_runtime_suspend() to succeed during system suspend)
runtime PM callbacks may be executed concurrently with system
suspend/resume callbacks for the same device.

The main reason for commit e8665002477f0278f84f898145b1f141ba26ee26
was that some subsystems and device drivers wanted to use runtime PM
helpers, pm_runtime_suspend() and pm_runtime_put_sync() in
particular, for carrying out the suspend of devices in their
.suspend() callbacks.  However, as it's been determined recently,
there are multiple reasons not to do so, inlcuding:

 * The caller really doesn't control the runtime PM usage counters,
   because user space can access them through sysfs and effectively
   block runtime PM.  That means using pm_runtime_suspend() or
   pm_runtime_get_sync() to suspend devices during system suspend
   may or may not work.

 * If a driver calls pm_runtime_suspend() from its .suspend()
   callback, it causes the subsystem's .runtime_suspend() callback to
   be executed, which leads to the call sequence:

   subsys->suspend(dev)
      driver->suspend(dev)
         pm_runtime_suspend(dev)
            subsys->runtime_suspend(dev)

   recursive from the subsystem's point of view.  For some subsystems
   that may actually work (e.g. the platform bus type), but for some
   it will fail in a rather spectacular fashion (e.g. PCI).  In each
   case it means a layering violation.

 * Both the subsystem and the driver can provide .suspend_noirq()
   callbacks for system suspend that can do whatever the
   .runtime_suspend() callbacks do just fine, so it really isn't
   necessary to call pm_runtime_suspend() during system suspend.

 * The runtime PM's handling of wakeup devices is usually different
   from the system suspend's one, so .runtime_suspend() may simply be
   inappropriate for system suspend.

 * System suspend is supposed to work even if CONFIG_PM_RUNTIME is
   unset.

 * The runtime PM workqueue is frozen before system suspend, so if
   whatever the driver is going to do during system suspend depends
   on it, that simply won't work.

Still, there is a good reason to allow pm_runtime_resume() to
succeed during system suspend and resume (for instance, some
subsystems and device drivers may legitimately use it to ensure that
their devices are in full-power states before suspending them).
Moreover, there is no reason to prevent runtime PM callbacks from
being executed in parallel with the system suspend/resume .prepare()
and .complete() callbacks and the code removed by commit
e8665002477f0278f84f898145b1f141ba26ee26 went too far in this
respect.  On the other hand, runtime PM callbacks, including
.runtime_resume(), must not be executed during system suspend's
"late" stage of suspending devices and during system resume's "early"
device resume stage.

Taking all of the above into consideration, make the PM core
acquire a runtime PM reference to every device and resume it if
there's a runtime PM resume request pending right before executing
the subsystem-level .suspend() callback for it.  Make the PM core
drop references to all devices right after executing the
subsystem-level .resume() callbacks for them.  Additionally,
make the PM core disable the runtime PM framework for all devices
during system suspend, after executing the subsystem-level .suspend()
callbacks for them, and enable the runtime PM framework for all
devices during system resume, right before executing the
subsystem-level .resume() callbacks for them.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 Documentation/power/runtime_pm.txt |   19 +++++++++++++++++++
 drivers/base/power/main.c          |   27 ++++++++++++++++++++++-----
 2 files changed, 41 insertions(+), 5 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
@@ -505,6 +505,7 @@ static int legacy_resume(struct device *
 static int device_resume(struct device *dev, pm_message_t state, bool async)
 {
 	int error = 0;
+	bool put = false;
 
 	TRACE_DEVICE(dev);
 	TRACE_RESUME(0);
@@ -521,6 +522,9 @@ static int device_resume(struct device *
 	if (!dev->power.is_suspended)
 		goto Unlock;
 
+	pm_runtime_enable(dev);
+	put = true;
+
 	if (dev->pm_domain) {
 		pm_dev_dbg(dev, state, "power domain ");
 		error = pm_op(dev, &dev->pm_domain->ops, state);
@@ -563,6 +567,10 @@ static int device_resume(struct device *
 	complete_all(&dev->power.completion);
 
 	TRACE_RESUME(error);
+
+	if (put)
+		pm_runtime_put_sync(dev);
+
 	return error;
 }
 
@@ -843,16 +851,22 @@ static int __device_suspend(struct devic
 	int error = 0;
 
 	dpm_wait_for_children(dev, async);
-	device_lock(dev);
 
 	if (async_error)
-		goto Unlock;
+		return 0;
+
+	pm_runtime_get_noresume(dev);
+	if (pm_runtime_barrier(dev) && device_may_wakeup(dev))
+		pm_wakeup_event(dev, 0);
 
 	if (pm_wakeup_pending()) {
+		pm_runtime_put_sync(dev);
 		async_error = -EBUSY;
-		goto Unlock;
+		return 0;
 	}
 
+	device_lock(dev);
+
 	if (dev->pm_domain) {
 		pm_dev_dbg(dev, state, "power domain ");
 		error = pm_op(dev, &dev->pm_domain->ops, state);
@@ -890,12 +904,15 @@ static int __device_suspend(struct devic
  End:
 	dev->power.is_suspended = !error;
 
- Unlock:
 	device_unlock(dev);
 	complete_all(&dev->power.completion);
 
-	if (error)
+	if (error) {
+		pm_runtime_put_sync(dev);
 		async_error = error;
+	} else if (dev->power.is_suspended) {
+		__pm_runtime_disable(dev, false);
+	}
 
 	return error;
 }
Index: linux-2.6/Documentation/power/runtime_pm.txt
===================================================================
--- linux-2.6.orig/Documentation/power/runtime_pm.txt
+++ linux-2.6/Documentation/power/runtime_pm.txt
@@ -567,6 +567,11 @@ this is:
 	pm_runtime_set_active(dev);
 	pm_runtime_enable(dev);
 
+The PM core always increments the run-time usage counter before calling the
+->suspend() callback and decrements it after calling the ->resume() callback.
+Hence disabling run-time PM temporarily like this will not cause any run-time
+suspend callbacks to be lost.
+
 On some systems, however, system sleep is not entered through a global firmware
 or hardware operation.  Instead, all hardware components are put into low-power
 states directly by the kernel in a coordinated way.  Then, the system sleep
@@ -579,6 +584,20 @@ place (in particular, if the system is n
 be more efficient to leave the devices that had been suspended before the system
 suspend began in the suspended state.
 
+The PM core does its best to reduce the probability of race conditions between
+the runtime PM and system suspend/resume (and hibernation) callbacks by carrying
+out the following operations:
+
+  * During system suspend it calls pm_runtime_get_noresume() and
+    pm_runtime_barrier() for every device right before executing the
+    subsystem-level .suspend() callback for it.  In addition to that it calls
+    pm_runtime_disable() for every device right after executing the
+    subsystem-level .suspend() callback for it.
+
+  * During system resume it calls pm_runtime_enable() and pm_runtime_put_sync()
+    for every device right before and right after executing the subsystem-level
+    .resume() callback for it, respectively.
+
 7. Generic subsystem callbacks
 
 Subsystems may wish to conserve code space by using the set of generic power

Re: [PATCH 3/3] PM: Limit race conditions between runtime PM and system sleep

[Alan Stern]

On Tue, 28 Jun 2011, Rafael J. Wysocki wrote:

> On Tuesday, June 28, 2011, Ming Lei wrote:
> > 
> > Hi Rafael,
> > 
> > On Sun, 26 Jun 2011 00:56:31 +0200
> > "Rafael J. Wysocki" <rjw@sisk.pl> wrote:
> > 
> > > Index: linux-2.6/Documentation/power/runtime_pm.txt
> > > ===================================================================
> > > --- linux-2.6.orig/Documentation/power/runtime_pm.txt
> > > +++ linux-2.6/Documentation/power/runtime_pm.txt
> > > @@ -567,6 +567,11 @@ this is:
> > >  	pm_runtime_set_active(dev);
> > >  	pm_runtime_enable(dev);
> > >  
> > > +The PM core always increments the run-time usage counter before calling the
> > > +->suspend() callback and decrements it after calling the ->resume() callback.
> > > +Hence disabling run-time PM temporarily like this will not cause any run-time
> > > +suspend callbacks to be lost.
> > 
> > Could you explain why the above is that "this will not cause any run-time suspend
> > callbacks to be lost"? 
> > 
> > Looks like it should be "this will not cause any run-time suspend callbacks to
> > be called", but not sure.
> 
> You're right the wording is not perfect.  The problem is that if it's done
> this way without incrementing the usage counter beforehand, the status may
> change to "suspended" right before the pm_runtime_set_active() and then the
> new status will not reflect the actual state of the device.
> 
> So, it may be better to say "Hence disabling runtime PM temporarily like this
> will not cause the runtime PM status of the device to conflict with the actual
> device state".
> 
> Alan, what do you think?

How about: "... will not cause any runtime suspend attempts to be 
permanently lost.  If the usage count goes to zero following the return 
of the ->resume() callback, the ->runtime_idle() callback will be 
invoked as usual."

Alan Stern

Re: [PATCH 3/3] PM: Limit race conditions between runtime PM and system sleep

[Rafael J. Wysocki]

On Wednesday, June 29, 2011, Alan Stern wrote:
> On Tue, 28 Jun 2011, Rafael J. Wysocki wrote:
> 
> > On Tuesday, June 28, 2011, Ming Lei wrote:
> > > 
> > > Hi Rafael,
> > > 
> > > On Sun, 26 Jun 2011 00:56:31 +0200
> > > "Rafael J. Wysocki" <rjw@sisk.pl> wrote:
> > > 
> > > > Index: linux-2.6/Documentation/power/runtime_pm.txt
> > > > ===================================================================
> > > > --- linux-2.6.orig/Documentation/power/runtime_pm.txt
> > > > +++ linux-2.6/Documentation/power/runtime_pm.txt
> > > > @@ -567,6 +567,11 @@ this is:
> > > >  	pm_runtime_set_active(dev);
> > > >  	pm_runtime_enable(dev);
> > > >  
> > > > +The PM core always increments the run-time usage counter before calling the
> > > > +->suspend() callback and decrements it after calling the ->resume() callback.
> > > > +Hence disabling run-time PM temporarily like this will not cause any run-time
> > > > +suspend callbacks to be lost.
> > > 
> > > Could you explain why the above is that "this will not cause any run-time suspend
> > > callbacks to be lost"? 
> > > 
> > > Looks like it should be "this will not cause any run-time suspend callbacks to
> > > be called", but not sure.
> > 
> > You're right the wording is not perfect.  The problem is that if it's done
> > this way without incrementing the usage counter beforehand, the status may
> > change to "suspended" right before the pm_runtime_set_active() and then the
> > new status will not reflect the actual state of the device.
> > 
> > So, it may be better to say "Hence disabling runtime PM temporarily like this
> > will not cause the runtime PM status of the device to conflict with the actual
> > device state".
> > 
> > Alan, what do you think?
> 
> How about: "... will not cause any runtime suspend attempts to be 
> permanently lost.  If the usage count goes to zero following the return 
> of the ->resume() callback, the ->runtime_idle() callback will be 
> invoked as usual."

That's better, thanks!

Rafael