Runtime PM: Calling Device runtime PM callbacks?

Runtime PM: Calling Device runtime PM callbacks?

[Mahalingam, Nithish]

Hi Rafael,

I was wondering why PM Runtime Core cannot call the device PM callbacks when 
its device's bus does not support runtime PM (if such a scenario is valid)?


Regards,	
Nithish Mahalingam
P.S. Alan, Not sure if you are askin the a similar question in the 
"System sleep vs. runtime PM" mail thread.

Re: Runtime PM: Calling Device runtime PM callbacks?

[Rafael J. Wysocki]

On Sunday 13 December 2009, Mahalingam, Nithish wrote:
> Hi Rafael,
> 
> I was wondering why PM Runtime Core cannot call the device PM callbacks when 
> its device's bus does not support runtime PM (if such a scenario is valid)?

The assumption was it wouldn't be necessary, but the approach can be extended
to device types and device classes.

> Regards,	
> Nithish Mahalingam
> P.S. Alan, Not sure if you are askin the a similar question in the 
> "System sleep vs. runtime PM" mail thread.

I guess so.

Rafael

Re: Runtime PM: Calling Device runtime PM callbacks?

[Alan Stern]

On Sun, 13 Dec 2009, Rafael J. Wysocki wrote:

> On Sunday 13 December 2009, Mahalingam, Nithish wrote:
> > Hi Rafael,
> > 
> > I was wondering why PM Runtime Core cannot call the device PM callbacks when 
> > its device's bus does not support runtime PM (if such a scenario is valid)?
> 
> The assumption was it wouldn't be necessary, but the approach can be extended
> to device types and device classes.
> 
> > Regards,	
> > Nithish Mahalingam
> > P.S. Alan, Not sure if you are askin the a similar question in the 
> > "System sleep vs. runtime PM" mail thread.
> 
> I guess so.

Yes.  I mistakenly used the wrong words, but I meant the device type
and device class.

Alan Stern

Re: Runtime PM: Calling Device runtime PM callbacks?

[Rafael J. Wysocki]

On Sunday 13 December 2009, Alan Stern wrote:
> On Sun, 13 Dec 2009, Rafael J. Wysocki wrote:
> 
> > On Sunday 13 December 2009, Mahalingam, Nithish wrote:
> > > Hi Rafael,
> > > 
> > > I was wondering why PM Runtime Core cannot call the device PM callbacks when 
> > > its device's bus does not support runtime PM (if such a scenario is valid)?
> > 
> > The assumption was it wouldn't be necessary, but the approach can be extended
> > to device types and device classes.
> > 
> > > Regards,	
> > > Nithish Mahalingam
> > > P.S. Alan, Not sure if you are askin the a similar question in the 
> > > "System sleep vs. runtime PM" mail thread.
> > 
> > I guess so.
> 
> Yes.  I mistakenly used the wrong words, but I meant the device type
> and device class.

So, I guess we need something like device_suspend() and device_resume()
for run-time PM, but we also will need to update the documentation.

I'll try to prepare a patch for that, unless you already have one ready or
in the works.

Rafael

Re: Runtime PM: Calling Device runtime PM callbacks?

[Alan Stern]

On Sun, 13 Dec 2009, Rafael J. Wysocki wrote:

> On Sunday 13 December 2009, Alan Stern wrote:
> > On Sun, 13 Dec 2009, Rafael J. Wysocki wrote:
> > 
> > > On Sunday 13 December 2009, Mahalingam, Nithish wrote:
> > > > Hi Rafael,
> > > > 
> > > > I was wondering why PM Runtime Core cannot call the device PM callbacks when 
> > > > its device's bus does not support runtime PM (if such a scenario is valid)?
> > > 
> > > The assumption was it wouldn't be necessary, but the approach can be extended
> > > to device types and device classes.
> > > 
> > > > Regards,	
> > > > Nithish Mahalingam
> > > > P.S. Alan, Not sure if you are askin the a similar question in the 
> > > > "System sleep vs. runtime PM" mail thread.
> > > 
> > > I guess so.
> > 
> > Yes.  I mistakenly used the wrong words, but I meant the device type
> > and device class.
> 
> So, I guess we need something like device_suspend() and device_resume()
> for run-time PM, but we also will need to update the documentation.

Yes.

> I'll try to prepare a patch for that, unless you already have one ready or
> in the works.

I don't.  Go on ahead.

Alan Stern

Re: Runtime PM: Calling Device runtime PM callbacks?

[Rafael J. Wysocki]

On Sunday 13 December 2009, Alan Stern wrote:
> On Sun, 13 Dec 2009, Rafael J. Wysocki wrote:
> 
> > On Sunday 13 December 2009, Alan Stern wrote:
> > > On Sun, 13 Dec 2009, Rafael J. Wysocki wrote:
> > > 
> > > > On Sunday 13 December 2009, Mahalingam, Nithish wrote:
> > > > > Hi Rafael,
> > > > > 
> > > > > I was wondering why PM Runtime Core cannot call the device PM callbacks when 
> > > > > its device's bus does not support runtime PM (if such a scenario is valid)?
> > > > 
> > > > The assumption was it wouldn't be necessary, but the approach can be extended
> > > > to device types and device classes.
> > > > 
> > > > > Regards,	
> > > > > Nithish Mahalingam
> > > > > P.S. Alan, Not sure if you are askin the a similar question in the 
> > > > > "System sleep vs. runtime PM" mail thread.
> > > > 
> > > > I guess so.
> > > 
> > > Yes.  I mistakenly used the wrong words, but I meant the device type
> > > and device class.
> > 
> > So, I guess we need something like device_suspend() and device_resume()
> > for run-time PM, but we also will need to update the documentation.
> 
> Yes.
> 
> > I'll try to prepare a patch for that, unless you already have one ready or
> > in the works.
> 
> I don't.  Go on ahead.

There you go (untested for now).

->runtime_idle() is still only called for the device's bus type, because
otherwise it will be hard to determine the right ordering of the bus type,
device type and device class callbacks.

Comments welcome. :-)

Rafael

---
 Documentation/power/runtime_pm.txt |  204 ++++++++++++++++++++-----------------
 drivers/base/power/runtime.c       |  110 ++++++++++++++++---
 2 files changed, 201 insertions(+), 113 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
@@ -111,6 +111,45 @@ int pm_runtime_idle(struct device *dev)
 EXPORT_SYMBOL_GPL(pm_runtime_idle);
 
 /**
+ * device_runtime_suspend - Execute "runtime suspend" callbacks for a device.
+ * @dev: Device to handle.
+ * @error_ptr: Place to store error values returned by the callbacks.
+ */
+static int device_runtime_suspend(struct device *dev, int *error_ptr)
+{
+	int error = -ENOSYS;
+
+	down(&dev->sem);
+
+	if (dev->class && dev->class->pm && dev->class->pm->runtime_suspend) {
+		error = dev->class->pm->runtime_suspend(dev);
+		suspend_report_result(dev->class->pm->runtime_suspend, error);
+		*error_ptr = error;
+		if (error)
+			goto out;
+	}
+
+	if (dev->type && dev->type->pm && dev->type->pm->runtime_suspend) {
+		error = dev->type->pm->runtime_suspend(dev);
+		suspend_report_result(dev->type->pm->runtime_suspend, error);
+		*error_ptr = error;
+		if (error)
+			goto out;
+	}
+
+	if (dev->bus && dev->bus->pm && dev->bus->pm->runtime_suspend) {
+		error = dev->bus->pm->runtime_suspend(dev);
+		suspend_report_result(dev->bus->pm->runtime_suspend, error);
+		*error_ptr = error;
+	}
+
+ out:
+	up(&dev->sem);
+
+	return error;
+}
+
+/**
  * __pm_runtime_suspend - Carry out run-time suspend of given device.
  * @dev: Device to suspend.
  * @from_wq: If set, the function has been called via pm_wq.
@@ -127,7 +166,7 @@ int __pm_runtime_suspend(struct device *
 {
 	struct device *parent = NULL;
 	bool notify = false;
-	int retval = 0;
+	int error, retval = 0;
 
 	dev_dbg(dev, "__pm_runtime_suspend()%s!\n",
 		from_wq ? " from workqueue" : "");
@@ -186,17 +225,13 @@ int __pm_runtime_suspend(struct device *
 
 	dev->power.runtime_status = RPM_SUSPENDING;
 	dev->power.deferred_resume = false;
+	error = dev->power.runtime_error;
+	spin_unlock_irq(&dev->power.lock);
 
-	if (dev->bus && dev->bus->pm && dev->bus->pm->runtime_suspend) {
-		spin_unlock_irq(&dev->power.lock);
-
-		retval = dev->bus->pm->runtime_suspend(dev);
+	retval = device_runtime_suspend(dev, &error);
 
-		spin_lock_irq(&dev->power.lock);
-		dev->power.runtime_error = retval;
-	} else {
-		retval = -ENOSYS;
-	}
+	spin_lock_irq(&dev->power.lock);
+	dev->power.runtime_error = error;
 
 	if (retval) {
 		dev->power.runtime_status = RPM_ACTIVE;
@@ -256,6 +291,45 @@ int pm_runtime_suspend(struct device *de
 EXPORT_SYMBOL_GPL(pm_runtime_suspend);
 
 /**
+ * device_runtime_resume - Execute "runtime resume" callbacks for given device.
+ * @dev: Device to handle.
+ * @error_ptr: Place to store error values returned by the callbacks.
+ */
+static int device_runtime_resume(struct device *dev, int *error_ptr)
+{
+	int error = -ENOSYS;
+
+	down(&dev->sem);
+
+	if (dev->bus && dev->bus->pm && dev->bus->pm->runtime_resume) {
+		error = dev->bus->pm->runtime_resume(dev);
+		suspend_report_result(dev->bus->pm->runtime_resume, error);
+		*error_ptr = error;
+		if (error)
+			goto out;
+	}
+
+	if (dev->type && dev->type->pm && dev->type->pm->runtime_resume) {
+		error = dev->type->pm->runtime_resume(dev);
+		suspend_report_result(dev->type->pm->runtime_resume, error);
+		*error_ptr = error;
+		if (error)
+			goto out;
+	}
+
+	if (dev->class && dev->class->pm && dev->class->pm->runtime_resume) {
+		error = dev->class->pm->runtime_resume(dev);
+		suspend_report_result(dev->class->pm->runtime_resume, error);
+		*error_ptr = error;
+	}
+
+ out:
+	up(&dev->sem);
+
+	return error;
+}
+
+/**
  * __pm_runtime_resume - Carry out run-time resume of given device.
  * @dev: Device to resume.
  * @from_wq: If set, the function has been called via pm_wq.
@@ -272,7 +346,7 @@ int __pm_runtime_resume(struct device *d
 	__releases(&dev->power.lock) __acquires(&dev->power.lock)
 {
 	struct device *parent = NULL;
-	int retval = 0;
+	int error, retval = 0;
 
 	dev_dbg(dev, "__pm_runtime_resume()%s!\n",
 		from_wq ? " from workqueue" : "");
@@ -351,17 +425,13 @@ int __pm_runtime_resume(struct device *d
 	}
 
 	dev->power.runtime_status = RPM_RESUMING;
+	error = dev->power.runtime_error;
+	spin_unlock_irq(&dev->power.lock);
 
-	if (dev->bus && dev->bus->pm && dev->bus->pm->runtime_resume) {
-		spin_unlock_irq(&dev->power.lock);
-
-		retval = dev->bus->pm->runtime_resume(dev);
+	retval = device_runtime_resume(dev, &error);
 
-		spin_lock_irq(&dev->power.lock);
-		dev->power.runtime_error = retval;
-	} else {
-		retval = -ENOSYS;
-	}
+	spin_lock_irq(&dev->power.lock);
+	dev->power.runtime_error = error;
 
 	if (retval) {
 		dev->power.runtime_status = RPM_SUSPENDED;
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
@@ -42,62 +42,79 @@ struct dev_pm_ops {
 	...
 };
 
-The ->runtime_suspend() callback is executed by the PM core for the bus type of
-the device being suspended.  The bus type's callback is then _entirely_
-_responsible_ for handling the device as appropriate, which may, but need not
-include executing the device driver's own ->runtime_suspend() callback (from the
-PM core's point of view it is not necessary to implement a ->runtime_suspend()
-callback in a device driver as long as the bus type's ->runtime_suspend() knows
-what to do to handle the device).
-
-  * Once the bus type's ->runtime_suspend() callback has completed successfully
-    for given device, the PM core regards the device as suspended, which need
-    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 its bus type's
-    ->runtime_resume() callback is executed for it.  The run-time PM status of
-    a device after successful execution of its bus type's ->runtime_suspend()
-    callback is 'suspended'.
-
-  * If the bus type's ->runtime_suspend() callback returns -EBUSY or -EAGAIN,
-    the device's run-time PM status is supposed to be 'active', which means that
-    the device _must_ be fully operational afterwards.
-
-  * If the bus type's ->runtime_suspend() callback returns an error code
-    different from -EBUSY or -EAGAIN, the PM core regards this as a fatal
-    error and will refuse to run the helper functions described in Section 4
-    for the device, until the status of it is directly set either to 'active'
-    or to 'suspended' (the PM core provides special helper functions for this
-    purpose).
+The ->runtime_suspend() callback is executed by the PM core for the bus type,
+device type and device class of the device being suspended.  The bus type,
+device type and device class callbacks are then _entirely_ _responsible_ for
+handling the device as appropriate, which may, but need not include executing
+the device driver's own ->runtime_suspend() callback (from the PM core's point
+of view it is not necessary to implement a ->runtime_suspend() callback in a
+device driver as long as the bus type, device type and device class
+->runtime_suspend() know what to do to handle the device).
+
+  * Once all of the bus type, device type and device class ->runtime_suspend()
+    callbacks defined for given device have completed successfully, the PM core
+    regards the device as suspended, which need 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 bus type, device type and device class ->runtime_resume()
+    callbacks are executed for it.  The run-time PM status of a device after
+    successful execution of the bus type, device type and device class
+    ->runtime_suspend() callbacks is 'suspended'.
+
+  * If the device class ->runtime_suspend() callback returns error code for
+    given device, the bus type and device type ->runtime_suspend() callbacks
+    will not be executed for it.  In turn, if the device type callback returns
+    error code for the device, the bus type callback will not be executed for
+    it.
+
+  * If the bus type, device type or device class ->runtime_suspend() callback
+    returns -EBUSY or -EAGAIN, the device's run-time PM status is supposed to be
+    'active', which means that the device _must_ be fully operational
+    afterwards.
+
+  * If the bus type, device type or device class ->runtime_suspend() callback
+    returns error code different from -EBUSY or -EAGAIN, the PM core regards
+    this as a fatal error and will refuse to run the helper functions described
+    in Section 4 for the device, until the status of it is directly set either
+    to 'active' or to 'suspended' (the PM core provides special helper functions
+    for this purpose).
 
 In particular, if the driver requires remote wakeup capability for proper
 functioning and device_run_wake() returns 'false' for the device, then
 ->runtime_suspend() should return -EBUSY.  On the other hand, if
 device_run_wake() returns 'true' for the device and the device is put
-into a low power state during the execution of its bus type's
-->runtime_suspend(), it is expected that remote wake-up (i.e. hardware mechanism
-allowing the device to request a change of its power state, such as PCI PME)
-will be enabled for the device.  Generally, remote wake-up should be enabled
-for all input devices put into a low power state at run time.
-
-The ->runtime_resume() callback is executed by the PM core for the bus type of
-the device being woken up.  The bus type's callback is then _entirely_
-_responsible_ for handling the device as appropriate, which may, but need not
-include executing the device driver's own ->runtime_resume() callback (from the
-PM core's point of view it is not necessary to implement a ->runtime_resume()
-callback in a device driver as long as the bus type's ->runtime_resume() knows
-what to do to handle the device).
-
-  * Once the bus type's ->runtime_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 of the device is then 'active'.
-
-  * If the bus type's ->runtime_resume() callback returns an error code, the PM
-    core regards this as a fatal error and will refuse to run the helper
-    functions described in Section 4 for the device, until its status is
-    directly set either to 'active' or to 'suspended' (the PM core provides
-    special helper functions for this purpose).
+into a low power state during the execution of its bus type, device type or
+device class ->runtime_suspend(), it is expected that remote wake-up (i.e.
+hardware mechanism allowing the device to request a change of its power state,
+such as PCI PME) will be enabled for the device.  Generally, remote wake-up
+should be enabled for all input devices put into a low power state at run time.
+
+The ->runtime_resume() callback is executed by the PM core for the bus type,
+device type and device class of the device being woken up.  The bus type, device
+type and device class callbacks are then _entirely_ _responsible_ for handling
+the device as appropriate, which may, but need not include executing the device
+driver's own ->runtime_resume() callback (from the PM core's point of view it is
+not necessary to implement a ->runtime_resume() callback in a device driver as
+long as the bus type, device type and device class ->runtime_resume() callbacks
+know what to do to handle the device).
+
+  * Once the bus type, device type and device class ->runtime_resume() callbacks
+    defined for the device have 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 of the device is
+    then 'active'.
+
+  * If the bus type ->runtime_resume() callback returns error code for given
+    device, the device type and device class ->runtime_resume() callbacks will
+    not be executed for it.  In turn, if the device type ->runtime_resume()
+    callback returns error code for the device, the device class callback will
+    not be executed for it.
+
+  * If the bus type, device type or device class ->runtime_resume() callback
+    returns error code, the PM core regards this as a fatal error and will
+    refuse to run the helper functions described in Section 4 for the device,
+    until its status is directly set either to 'active' or to 'suspended' (the
+    PM core provides special helper functions for this purpose).
 
 The ->runtime_idle() callback is executed by the PM core for the bus type of
 given device whenever the device appears to be idle, which is indicated to the
@@ -118,29 +135,28 @@ request for the device in that case.  Th
 ignored by the PM 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
-PM callbacks:
+that the following constraints are met with respect to the bus type, device type
+and device class run-time PM callbacks:
 
-(1) The callbacks are mutually exclusive (e.g. it is forbidden to execute
-    ->runtime_suspend() in parallel with ->runtime_resume() or with another
-    instance of ->runtime_suspend() for the same device) with the exception that
-    ->runtime_suspend() or ->runtime_resume() can be executed in parallel with
-    ->runtime_idle() (although ->runtime_idle() will not be started while any
-    of the other callbacks is being executed for the same device).
+(1) The idle, suspend and resume callbacks are mutually exclusive (e.g. it is
+    forbidden to execute ->runtime_suspend() in parallel with ->runtime_resume()
+    or with another instance of ->runtime_suspend() for the same device) with
+    the exception that ->runtime_suspend() or ->runtime_resume() can be executed
+    in parallel with ->runtime_idle() (although ->runtime_idle() will not be
+    started while any of the other callbacks is being executed for the same
+    device).
 
 (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
-    'active').
+    ->runtime_suspend() for the devices whose run-time PM status is 'active').
 
 (3) ->runtime_idle() and ->runtime_suspend() can only be executed for a device
-    the usage counter of which is equal to zero _and_ either the counter of
-    'active' children of which is equal to zero, or the 'power.ignore_children'
-    flag of which is set.
+    whose usage counter is equal to zero _and_ either the counter of 'active'
+    children is equal to zero, or the 'power.ignore_children' flag 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 status of which is 'suspended').
+    PM core will only execute ->runtime_resume() for the devices whose run-time
+    PM status is 'suspended').
 
 Additionally, the helper functions provided by the PM core obey the following
 rules:
@@ -243,17 +259,18 @@ drivers/base/power/runtime.c and include
       is already being executed
 
   int pm_runtime_suspend(struct device *dev);
-    - execute ->runtime_suspend() for the device's bus type; 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
+    - execute ->runtime_suspend() for the device class, device type and bus
+      type of dev in this order; 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
 
   int pm_runtime_resume(struct device *dev);
-    - execute ->runtime_resume() for the device's bus type; returns 0 on
-      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
+    - execute ->runtime_resume() for the bus type, device type and device class
+      of dev in this order; returns 0 on 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
 
   int pm_request_idle(struct device *dev);
     - submit a request to execute ->runtime_idle() for the device's bus type
@@ -261,20 +278,20 @@ drivers/base/power/runtime.c and include
       or error code if the request has not been queued up
 
   int pm_schedule_suspend(struct device *dev, unsigned int delay);
-    - schedule the execution of ->runtime_suspend() for the device's bus type
-      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
-      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
+    - schedule the execution of ->runtime_suspend() for the device class, device
+      type and bus type of dev 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 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
 
   int pm_request_resume(struct device *dev);
-    - submit a request to execute ->runtime_resume() for the device's bus type
-      (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
-      error code if the request hasn't been queued up
+    - submit a request to execute ->runtime_resume() for the bus type, device
+      type and device class of dev (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 error code if the request hasn't been queued up
 
   void pm_runtime_get_noresume(struct device *dev);
     - increment the device's usage counter
@@ -303,12 +320,13 @@ drivers/base/power/runtime.c and include
       run-time PM callbacks described in Section 2
 
   int pm_runtime_disable(struct device *dev);
-    - prevent the run-time PM helper functions from running the device bus
-      type's run-time PM callbacks, 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
-      ->runtime_resume() for the device's bus type to satisfy that request,
-      otherwise 0 is returned
+    - prevent the run-time PM helper functions from running the device's bus
+      type, device type and device class run-time PM callbacks, 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 ->runtime_resume() for the bus type, device
+      type and device class of dev 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

Re: Runtime PM: Calling Device runtime PM callbacks?

[Mahalingam, Nithish]

> There you go (untested for now).
> 
> ->runtime_idle() is still only called for the device's bus type, because
> otherwise it will be hard to determine the right ordering of the bus type,
> device type and device class callbacks.
> 
> Comments welcome. :-)

Looks good Rafael and exactly what I was looking for.


I had one more question -

+The ->runtime_suspend() callback is executed by the PM core for the bus type,
+device type and device class of the device being suspended.  The bus type,
+device type and device class callbacks are then _entirely_ _responsible_ for
+handling the device as appropriate, which may, but need not include executing
+the device driver's own ->runtime_suspend() callback (from the PM core's point
+of view it is not necessary to implement a ->runtime_suspend() callback in a
+device driver as long as the bus type, device type and device class
+->runtime_suspend() know what to do to handle the device).

Any specific reason why from PM core we should not call the device driver's 
->runtime_suspend() or ->runtime_resume()? I know one of either the bus/
class/type should implement device suspend but what if (worst case) none of 
them are doing it? Is it OK in that case (alone) to call device driver's 
runtime PM directly (if it is implemented) from the runtime PM core?


Regards,
Nithish Mahalingam

Re: Runtime PM: Calling Device runtime PM callbacks?

[Alan Stern]

On Mon, 14 Dec 2009, Mahalingam, Nithish wrote:

> I had one more question -
> 
> +The ->runtime_suspend() callback is executed by the PM core for the bus type,
> +device type and device class of the device being suspended.  The bus type,
> +device type and device class callbacks are then _entirely_ _responsible_ for
> +handling the device as appropriate, which may, but need not include executing
> +the device driver's own ->runtime_suspend() callback (from the PM core's point
> +of view it is not necessary to implement a ->runtime_suspend() callback in a
> +device driver as long as the bus type, device type and device class
> +->runtime_suspend() know what to do to handle the device).
> 
> Any specific reason why from PM core we should not call the device driver's 
> ->runtime_suspend() or ->runtime_resume()? I know one of either the bus/
> class/type should implement device suspend but what if (worst case) none of 
> them are doing it? Is it OK in that case (alone) to call device driver's 
> runtime PM directly (if it is implemented) from the runtime PM core?

The system PM driver_suspend() and driver_resume() routines don't do 
that.  For consistency, the runtime PM routines should behave the same 
way.

Alan Stern

P.S.: Rafael, I just realized that your documentation changes could be
reduced considerably.  All you have to do is explain once how whenever
a method call occurs, the PM core will search for a callback pointer in
the following locations: ...  Then in all those places where you list
all the callback possibilities, just say "the callback routine" or
something like that.

Re: Runtime PM: Calling Device runtime PM callbacks?

[Rafael J. Wysocki]

On Monday 14 December 2009, Alan Stern wrote:
> On Mon, 14 Dec 2009, Mahalingam, Nithish wrote:
> 
> > I had one more question -
> > 
> > +The ->runtime_suspend() callback is executed by the PM core for the bus type,
> > +device type and device class of the device being suspended.  The bus type,
> > +device type and device class callbacks are then _entirely_ _responsible_ for
> > +handling the device as appropriate, which may, but need not include executing
> > +the device driver's own ->runtime_suspend() callback (from the PM core's point
> > +of view it is not necessary to implement a ->runtime_suspend() callback in a
> > +device driver as long as the bus type, device type and device class
> > +->runtime_suspend() know what to do to handle the device).
> > 
> > Any specific reason why from PM core we should not call the device driver's 
> > ->runtime_suspend() or ->runtime_resume()? I know one of either the bus/
> > class/type should implement device suspend but what if (worst case) none of 
> > them are doing it? Is it OK in that case (alone) to call device driver's 
> > runtime PM directly (if it is implemented) from the runtime PM core?
> 
> The system PM driver_suspend() and driver_resume() routines don't do 
> that.  For consistency, the runtime PM routines should behave the same 
> way.
> 
> Alan Stern
> 
> P.S.: Rafael, I just realized that your documentation changes could be
> reduced considerably.  All you have to do is explain once how whenever
> a method call occurs, the PM core will search for a callback pointer in
> the following locations: ...  Then in all those places where you list
> all the callback possibilities, just say "the callback routine" or
> something like that.

Yeah.  It was late yesterday when I was preparing the patch and I just didn't
think about that.

Rafael

Re: Runtime PM: Calling Device runtime PM callbacks?

[Alan Stern]

On Mon, 14 Dec 2009, Rafael J. Wysocki wrote:

> There you go (untested for now).
> 
> ->runtime_idle() is still only called for the device's bus type, because
> otherwise it will be hard to determine the right ordering of the bus type,
> device type and device class callbacks.

Shouldn't it be the same as runtime_suspend and runtime_resume?

>  drivers/base/power/runtime.c       |  110 ++++++++++++++++---
>  2 files changed, 201 insertions(+), 113 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
> @@ -111,6 +111,45 @@ int pm_runtime_idle(struct device *dev)
>  EXPORT_SYMBOL_GPL(pm_runtime_idle);
>  
>  /**
> + * device_runtime_suspend - Execute "runtime suspend" callbacks for a device.
> + * @dev: Device to handle.
> + * @error_ptr: Place to store error values returned by the callbacks.
> + */
> +static int device_runtime_suspend(struct device *dev, int *error_ptr)
> +{
> +	int error = -ENOSYS;
> +
> +	down(&dev->sem);
> +
> +	if (dev->class && dev->class->pm && dev->class->pm->runtime_suspend) {
> +		error = dev->class->pm->runtime_suspend(dev);
> +		suspend_report_result(dev->class->pm->runtime_suspend, error);
> +		*error_ptr = error;
> +		if (error)
> +			goto out;
> +	}
> +
> +	if (dev->type && dev->type->pm && dev->type->pm->runtime_suspend) {
> +		error = dev->type->pm->runtime_suspend(dev);
> +		suspend_report_result(dev->type->pm->runtime_suspend, error);
> +		*error_ptr = error;
> +		if (error)
> +			goto out;
> +	}
> +
> +	if (dev->bus && dev->bus->pm && dev->bus->pm->runtime_suspend) {
> +		error = dev->bus->pm->runtime_suspend(dev);
> +		suspend_report_result(dev->bus->pm->runtime_suspend, error);
> +		*error_ptr = error;
> +	}
> +
> + out:
> +	up(&dev->sem);
> +
> +	return error;
> +}

What's the reason for error_ptr here?  Its value will always be the
same as the return value except in the case where none of the callbacks
are defined.  Why not just use -ENOSYS in that case and eliminate
error_ptr?

Alan Stern

Re: Runtime PM: Calling Device runtime PM callbacks?

[Rafael J. Wysocki]

On Monday 14 December 2009, Alan Stern wrote:
> On Mon, 14 Dec 2009, Rafael J. Wysocki wrote:
> 
> > There you go (untested for now).
> > 
> > ->runtime_idle() is still only called for the device's bus type, because
> > otherwise it will be hard to determine the right ordering of the bus type,
> > device type and device class callbacks.
> 
> Shouldn't it be the same as runtime_suspend and runtime_resume?

Well, the ordering is different in each of them ...

> >  drivers/base/power/runtime.c       |  110 ++++++++++++++++---
> >  2 files changed, 201 insertions(+), 113 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
> > @@ -111,6 +111,45 @@ int pm_runtime_idle(struct device *dev)
> >  EXPORT_SYMBOL_GPL(pm_runtime_idle);
> >  
> >  /**
> > + * device_runtime_suspend - Execute "runtime suspend" callbacks for a device.
> > + * @dev: Device to handle.
> > + * @error_ptr: Place to store error values returned by the callbacks.
> > + */
> > +static int device_runtime_suspend(struct device *dev, int *error_ptr)
> > +{
> > +	int error = -ENOSYS;
> > +
> > +	down(&dev->sem);
> > +
> > +	if (dev->class && dev->class->pm && dev->class->pm->runtime_suspend) {
> > +		error = dev->class->pm->runtime_suspend(dev);
> > +		suspend_report_result(dev->class->pm->runtime_suspend, error);
> > +		*error_ptr = error;
> > +		if (error)
> > +			goto out;
> > +	}
> > +
> > +	if (dev->type && dev->type->pm && dev->type->pm->runtime_suspend) {
> > +		error = dev->type->pm->runtime_suspend(dev);
> > +		suspend_report_result(dev->type->pm->runtime_suspend, error);
> > +		*error_ptr = error;
> > +		if (error)
> > +			goto out;
> > +	}
> > +
> > +	if (dev->bus && dev->bus->pm && dev->bus->pm->runtime_suspend) {
> > +		error = dev->bus->pm->runtime_suspend(dev);
> > +		suspend_report_result(dev->bus->pm->runtime_suspend, error);
> > +		*error_ptr = error;
> > +	}
> > +
> > + out:
> > +	up(&dev->sem);
> > +
> > +	return error;
> > +}
> 
> What's the reason for error_ptr here?  Its value will always be the
> same as the return value except in the case where none of the callbacks
> are defined.  Why not just use -ENOSYS in that case and eliminate
> error_ptr?

To preserve the existing logic.

Namely, without the patch dev->power.runtime error is not updated in the
-ENOSYS case and that actually is for a reason (we don't want runtime_error to
be set merely because there's no callbacks to execute).  I could check the
return value, but what if one of the callbacks returns -ENOSYS?

Rafael

Re: Runtime PM: Calling Device runtime PM callbacks?

[Alan Stern]

On Mon, 14 Dec 2009, Rafael J. Wysocki wrote:

> On Monday 14 December 2009, Alan Stern wrote:
> > On Mon, 14 Dec 2009, Rafael J. Wysocki wrote:
> > 
> > > There you go (untested for now).
> > > 
> > > ->runtime_idle() is still only called for the device's bus type, because
> > > otherwise it will be hard to determine the right ordering of the bus type,
> > > device type and device class callbacks.
> > 
> > Shouldn't it be the same as runtime_suspend and runtime_resume?
> 
> Well, the ordering is different in each of them ...

Why not just copy the order used by device_suspend(): class, then type,
then bus?

Actually, I don't know of any cases where the order matters.  But there 
may be some, for system suspend.  Runtime suspend is new enough that 
people will adapt.

> > What's the reason for error_ptr here?  Its value will always be the
> > same as the return value except in the case where none of the callbacks
> > are defined.  Why not just use -ENOSYS in that case and eliminate
> > error_ptr?
> 
> To preserve the existing logic.
> 
> Namely, without the patch dev->power.runtime error is not updated in the
> -ENOSYS case and that actually is for a reason (we don't want runtime_error to
> be set merely because there's no callbacks to execute).  I could check the
> return value, but what if one of the callbacks returns -ENOSYS?

Don't worry about it.  -ENOSYS means the operation isn't implemented.  
So if somebody's method implementation tells you that the method isn't 
implemented, they'll get what they deserve.  :-)

Alan Stern

Re: Runtime PM: Calling Device runtime PM callbacks?

[Rafael J. Wysocki]

On Monday 14 December 2009, Alan Stern wrote:
> On Mon, 14 Dec 2009, Rafael J. Wysocki wrote:
> 
> > On Monday 14 December 2009, Alan Stern wrote:
> > > On Mon, 14 Dec 2009, Rafael J. Wysocki wrote:
> > > 
> > > > There you go (untested for now).
> > > > 
> > > > ->runtime_idle() is still only called for the device's bus type, because
> > > > otherwise it will be hard to determine the right ordering of the bus type,
> > > > device type and device class callbacks.
> > > 
> > > Shouldn't it be the same as runtime_suspend and runtime_resume?
> > 
> > Well, the ordering is different in each of them ...
> 
> Why not just copy the order used by device_suspend(): class, then type,
> then bus?

Do you mean in _idle?

> Actually, I don't know of any cases where the order matters.  But there 
> may be some, for system suspend.  Runtime suspend is new enough that 
> people will adapt.

Still, calling them in the reverse order in resume is kind of logical ...

> > > What's the reason for error_ptr here?  Its value will always be the
> > > same as the return value except in the case where none of the callbacks
> > > are defined.  Why not just use -ENOSYS in that case and eliminate
> > > error_ptr?
> > 
> > To preserve the existing logic.
> > 
> > Namely, without the patch dev->power.runtime error is not updated in the
> > -ENOSYS case and that actually is for a reason (we don't want runtime_error to
> > be set merely because there's no callbacks to execute).  I could check the
> > return value, but what if one of the callbacks returns -ENOSYS?
> 
> Don't worry about it.  -ENOSYS means the operation isn't implemented.  
> So if somebody's method implementation tells you that the method isn't 
> implemented, they'll get what they deserve.  :-)

OK

Rafael

Re: Runtime PM: Calling Device runtime PM callbacks?

[Alan Stern]

On Mon, 14 Dec 2009, Rafael J. Wysocki wrote:

> > > > > ->runtime_idle() is still only called for the device's bus type, because
> > > > > otherwise it will be hard to determine the right ordering of the bus type,
> > > > > device type and device class callbacks.
> > > > 
> > > > Shouldn't it be the same as runtime_suspend and runtime_resume?
> > > 
> > > Well, the ordering is different in each of them ...
> > 
> > Why not just copy the order used by device_suspend(): class, then type,
> > then bus?
> 
> Do you mean in _idle?

Idle is very similar to suspend.  They should use the same order.

I guess there's one extra thing to look out for: If one of the idle
callbacks does pm_runtime_suspend() then there's no point invoking the
later callbacks.

> > Actually, I don't know of any cases where the order matters.  But there 
> > may be some, for system suspend.  Runtime suspend is new enough that 
> > people will adapt.
> 
> Still, calling them in the reverse order in resume is kind of logical ...

Yes.

Alan Stern

Re: Runtime PM: Calling Device runtime PM callbacks?

[Rafael J. Wysocki]

On Tuesday 15 December 2009, Alan Stern wrote:
> On Mon, 14 Dec 2009, Rafael J. Wysocki wrote:
> 
> > > > > > ->runtime_idle() is still only called for the device's bus type, because
> > > > > > otherwise it will be hard to determine the right ordering of the bus type,
> > > > > > device type and device class callbacks.
> > > > > 
> > > > > Shouldn't it be the same as runtime_suspend and runtime_resume?
> > > > 
> > > > Well, the ordering is different in each of them ...
> > > 
> > > Why not just copy the order used by device_suspend(): class, then type,
> > > then bus?
> > 
> > Do you mean in _idle?
> 
> Idle is very similar to suspend.  They should use the same order.
> 
> I guess there's one extra thing to look out for: If one of the idle
> callbacks does pm_runtime_suspend() then there's no point invoking the
> later callbacks.

But how do we know that?

I'm still not sure what actually the point executing _idle for device types and
device classes is.  The only situation I can aticipate if when there's a device
without a bus type.

Hmm.  Actually, are we ever going to call two or more suspend callbacks (ie.
bus type one and device type one or device type one and device class one)
for the same device?  If not, we'll be able to simplify things significantly
by making them mutually exclusive (ie. if there's bus type, call bus type,
or else if there's device type, call device type, or else if there's device
class, call device class).

Rafael

Re: Runtime PM: Calling Device runtime PM callbacks?

[Mahalingam, Nithish]

> Hmm.  Actually, are we ever going to call two or more suspend callbacks (ie.
> bus type one and device type one or device type one and device class one)
> for the same device?  If not, we'll be able to simplify things significantly
> by making them mutually exclusive (ie. if there's bus type, call bus type,
> or else if there's device type, call device type, or else if there's device
> class, call device class).

Was about to ask this question. What is the advantage of calling all of the 
device's bus type, device type and device class runtime suspend callbacks?

Regards,
Nithish Mahalingam

Re: Runtime PM: Calling Device runtime PM callbacks?

[Alan Stern]

On Tue, 15 Dec 2009, Rafael J. Wysocki wrote:

> > Idle is very similar to suspend.  They should use the same order.
> > 
> > I guess there's one extra thing to look out for: If one of the idle
> > callbacks does pm_runtime_suspend() then there's no point invoking the
> > later callbacks.
> 
> But how do we know that?

How do we know whether the callback does pm_runtime_suspend()?  Because 
dev->runtime_status changes to RPM_SUSPENDED.

How do we know there's no point invoking the later callbacks?  Because 
an idle callback is merely supposed to decide whether or not to suspend 
the device.  If the device is already suspended then the callback's job 
is already done.

> I'm still not sure what actually the point executing _idle for device types and
> device classes is.  The only situation I can aticipate if when there's a device
> without a bus type.

In the USB stack we have "devices" and "interfaces", two different
device types (both belonging to the USB bus type).  They need to have
separate callbacks.  The device idle callback will do a bunch of
testing, whereas the interface idle callback will always invoke
pm_runtime_suspend() immediately.

> Hmm.  Actually, are we ever going to call two or more suspend callbacks (ie.
> bus type one and device type one or device type one and device class one)
> for the same device?  If not, we'll be able to simplify things significantly
> by making them mutually exclusive (ie. if there's bus type, call bus type,
> or else if there's device type, call device type, or else if there's device
> class, call device class).

I don't plan ever to have more than one callback.  But I can't speak 
for other parts of the kernel.

The situation should be very much the same as with the DPM callbacks.  
They could make the same assumption.  Or neither should make it -- as
the case may be.

Alan Stern

Re: Runtime PM: Calling Device runtime PM callbacks?

[Rafael J. Wysocki]

On Tuesday 15 December 2009, Alan Stern wrote:
> On Tue, 15 Dec 2009, Rafael J. Wysocki wrote:
> 
> > > Idle is very similar to suspend.  They should use the same order.
> > > 
> > > I guess there's one extra thing to look out for: If one of the idle
> > > callbacks does pm_runtime_suspend() then there's no point invoking the
> > > later callbacks.
> > 
> > But how do we know that?
> 
> How do we know whether the callback does pm_runtime_suspend()?  Because 
> dev->runtime_status changes to RPM_SUSPENDED.
> 
> How do we know there's no point invoking the later callbacks?  Because 
> an idle callback is merely supposed to decide whether or not to suspend 
> the device.  If the device is already suspended then the callback's job 
> is already done.
> 
> > I'm still not sure what actually the point executing _idle for device types and
> > device classes is.  The only situation I can aticipate if when there's a device
> > without a bus type.
> 
> In the USB stack we have "devices" and "interfaces", two different
> device types (both belonging to the USB bus type).  They need to have
> separate callbacks.  The device idle callback will do a bunch of
> testing, whereas the interface idle callback will always invoke
> pm_runtime_suspend() immediately.
> 
> > Hmm.  Actually, are we ever going to call two or more suspend callbacks (ie.
> > bus type one and device type one or device type one and device class one)
> > for the same device?  If not, we'll be able to simplify things significantly
> > by making them mutually exclusive (ie. if there's bus type, call bus type,
> > or else if there's device type, call device type, or else if there's device
> > class, call device class).
> 
> I don't plan ever to have more than one callback.  But I can't speak 
> for other parts of the kernel.
> 
> The situation should be very much the same as with the DPM callbacks.  
> They could make the same assumption.  Or neither should make it -- as
> the case may be.

However, as you said before, the runtime PM interface is new so they can
adapt.

In fact, I'm not very comfortable with the current possibility to call DPM
callbacks of two or three kinds in a row for the same device, because that
means very close connection between the bus type and device type or device
class (or all three of them).  Nobody does that, as far as I can tell, and I
guess nobody will.

Now, if we agree that only one callback will be called for given device
(either bus type, or device type, or device class), the code may be simpler
and there won't be an issue with the ordering in _idle.

Rafael

Re: Runtime PM: Calling Device runtime PM callbacks?

[Alan Stern]

On Tue, 15 Dec 2009, Rafael J. Wysocki wrote:

> In fact, I'm not very comfortable with the current possibility to call DPM
> callbacks of two or three kinds in a row for the same device, because that
> means very close connection between the bus type and device type or device
> class (or all three of them).  Nobody does that, as far as I can tell, and I
> guess nobody will.

How certain are you?  Do you want to add a WARN_ON to the PM core in 
case such a thing turns up?

> Now, if we agree that only one callback will be called for given device
> (either bus type, or device type, or device class), the code may be simpler
> and there won't be an issue with the ordering in _idle.

That would be perfectly fine with me.

Alan Stern

Re: Runtime PM: Calling Device runtime PM callbacks?

[Rafael J. Wysocki]

On Tuesday 15 December 2009, Alan Stern wrote:
> On Tue, 15 Dec 2009, Rafael J. Wysocki wrote:
> 
> > In fact, I'm not very comfortable with the current possibility to call DPM
> > callbacks of two or three kinds in a row for the same device, because that
> > means very close connection between the bus type and device type or device
> > class (or all three of them).  Nobody does that, as far as I can tell, and I
> > guess nobody will.
> 
> How certain are you?  Do you want to add a WARN_ON to the PM core in 
> case such a thing turns up?

That might be a good idea, at least for linux-next testing.

> > Now, if we agree that only one callback will be called for given device
> > (either bus type, or device type, or device class), the code may be simpler
> > and there won't be an issue with the ordering in _idle.
> 
> That would be perfectly fine with me.

OK

Rafael

Re: Runtime PM: Calling Device runtime PM callbacks?

[Rafael J. Wysocki]

On Tuesday 15 December 2009, Rafael J. Wysocki wrote:
> On Tuesday 15 December 2009, Alan Stern wrote:
> > On Tue, 15 Dec 2009, Rafael J. Wysocki wrote:
...
> 
> > > Now, if we agree that only one callback will be called for given device
> > > (either bus type, or device type, or device class), the code may be simpler
> > > and there won't be an issue with the ordering in _idle.
> > 
> > That would be perfectly fine with me.
> 
> OK

Updated patch is appended, please tell me what you think.

Rafael

---
 Documentation/power/runtime_pm.txt |  173 ++++++++++++++++++-------------------
 drivers/base/power/runtime.c       |   45 +++++++++
 2 files changed, 132 insertions(+), 86 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
@@ -85,6 +85,19 @@ static int __pm_runtime_idle(struct devi
 		dev->bus->pm->runtime_idle(dev);
 
 		spin_lock_irq(&dev->power.lock);
+	} else if (dev->type && dev->type->pm && dev->type->pm->runtime_idle) {
+		spin_unlock_irq(&dev->power.lock);
+
+		dev->type->pm->runtime_idle(dev);
+
+		spin_lock_irq(&dev->power.lock);
+	} else if (dev->class && dev->class->pm
+	    && dev->class->pm->runtime_idle) {
+		spin_unlock_irq(&dev->power.lock);
+
+		dev->class->pm->runtime_idle(dev);
+
+		spin_lock_irq(&dev->power.lock);
 	}
 
 	dev->power.idle_notification = false;
@@ -194,6 +207,22 @@ int __pm_runtime_suspend(struct device *
 
 		spin_lock_irq(&dev->power.lock);
 		dev->power.runtime_error = retval;
+	} else if (dev->type && dev->type->pm
+	    && dev->type->pm->runtime_suspend) {
+		spin_unlock_irq(&dev->power.lock);
+
+		retval = dev->type->pm->runtime_suspend(dev);
+
+		spin_lock_irq(&dev->power.lock);
+		dev->power.runtime_error = retval;
+	} else if (dev->class && dev->class->pm
+	    && dev->class->pm->runtime_suspend) {
+		spin_unlock_irq(&dev->power.lock);
+
+		retval = dev->class->pm->runtime_suspend(dev);
+
+		spin_lock_irq(&dev->power.lock);
+		dev->power.runtime_error = retval;
 	} else {
 		retval = -ENOSYS;
 	}
@@ -359,6 +388,22 @@ int __pm_runtime_resume(struct device *d
 
 		spin_lock_irq(&dev->power.lock);
 		dev->power.runtime_error = retval;
+	} else if (dev->type && dev->type->pm
+	    && dev->type->pm->runtime_resume) {
+		spin_unlock_irq(&dev->power.lock);
+
+		retval = dev->type->pm->runtime_resume(dev);
+
+		spin_lock_irq(&dev->power.lock);
+		dev->power.runtime_error = retval;
+	} else if (dev->class && dev->class->pm
+	    && dev->class->pm->runtime_resume) {
+		spin_unlock_irq(&dev->power.lock);
+
+		retval = dev->class->pm->runtime_resume(dev);
+
+		spin_lock_irq(&dev->power.lock);
+		dev->power.runtime_error = retval;
 	} else {
 		retval = -ENOSYS;
 	}
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
@@ -42,80 +42,81 @@ struct dev_pm_ops {
 	...
 };
 
-The ->runtime_suspend() callback is executed by the PM core for the bus type of
-the device being suspended.  The bus type's callback is then _entirely_
-_responsible_ for handling the device as appropriate, which may, but need not
-include executing the device driver's own ->runtime_suspend() callback (from the
+The ->runtime_suspend(), ->runtime_resume() and ->runtime_idle() callbacks are
+executed by the PM core for either the bus type, or device type (if the bus
+type's callback is not defined), or device class (if the bus type's and device
+type's callbacks are not defined) of given device.  The bus type, device type
+and device class callbacks are referred to as subsystem-level callbacks in what
+follows.
+
+The subsystem-level suspend callback is _entirely_ _responsible_ for handling
+the suspend of the device as appropriate, which may, but need not include
+executing the device driver's own ->runtime_suspend() callback (from the
 PM core's point of view it is not necessary to implement a ->runtime_suspend()
-callback in a device driver as long as the bus type's ->runtime_suspend() knows
-what to do to handle the device).
+callback in a device driver as long as the subsystem-level suspend callback
+knows what to do to handle the device).
 
-  * Once the bus type's ->runtime_suspend() callback has completed successfully
+  * Once the subsystem-level suspend callback has completed successfully
     for given device, the PM core regards the device as suspended, which need
     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 its bus type's
-    ->runtime_resume() callback is executed for it.  The run-time PM status of
-    a device after successful execution of its bus type's ->runtime_suspend()
-    callback is 'suspended'.
-
-  * If the bus type's ->runtime_suspend() callback returns -EBUSY or -EAGAIN,
-    the device's run-time PM status is supposed to be 'active', which means that
-    the device _must_ be fully operational afterwards.
-
-  * If the bus type's ->runtime_suspend() callback returns an error code
-    different from -EBUSY or -EAGAIN, the PM core regards this as a fatal
-    error and will refuse to run the helper functions described in Section 4
-    for the device, until the status of it is directly set either to 'active'
-    or to 'suspended' (the PM core provides special helper functions for this
-    purpose).
-
-In particular, if the driver requires remote wakeup capability for proper
-functioning and device_run_wake() returns 'false' for the device, then
-->runtime_suspend() should return -EBUSY.  On the other hand, if
-device_run_wake() returns 'true' for the device and the device is put
-into a low power state during the execution of its bus type's
-->runtime_suspend(), it is expected that remote wake-up (i.e. hardware mechanism
-allowing the device to request a change of its power state, such as PCI PME)
-will be enabled for the device.  Generally, remote wake-up should be enabled
-for all input devices put into a low power state at run time.
-
-The ->runtime_resume() callback is executed by the PM core for the bus type of
-the device being woken up.  The bus type's callback is then _entirely_
-_responsible_ for handling the device as appropriate, which may, but need not
-include executing the device driver's own ->runtime_resume() callback (from the
-PM core's point of view it is not necessary to implement a ->runtime_resume()
-callback in a device driver as long as the bus type's ->runtime_resume() knows
-what to do to handle the device).
-
-  * Once the bus type's ->runtime_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 of the device is then 'active'.
-
-  * If the bus type's ->runtime_resume() callback returns an error code, the PM
-    core regards this as a fatal error and will refuse to run the helper
-    functions described in Section 4 for the device, until its status is
-    directly set either to 'active' or to 'suspended' (the PM core provides
-    special helper functions for this purpose).
-
-The ->runtime_idle() callback is executed by the PM core for the bus type of
-given device whenever the device appears to be idle, which is indicated to the
-PM core by two counters, the device's usage counter and the counter of 'active'
-children of the device.
+    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
+    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
+    _must_ be fully operational afterwards.
+
+  * If the subsystem-level suspend callback returns an error code different
+    from -EBUSY or -EAGAIN, the PM core regards this as a fatal error and will
+    refuse to run the helper functions described in Section 4 for the device,
+    until the status of it is directly set either to 'active', or to 'suspended'
+    (the PM core provides special helper functions for this purpose).
+
+In particular, if the driver requires remote wake-up capability (i.e. hardware
+mechanism allowing the device to request a change of its power state, such as
+PCI PME) for proper functioning and device_run_wake() returns 'false' for the
+device, then ->runtime_suspend() should return -EBUSY.  On the other hand, if
+device_run_wake() returns 'true' for the device and the device is put into a low
+power state during the execution of the subsystem-level suspend callback, it is
+expected that remote wake-up will be enabled for the device.  Generally, remote
+wake-up should be enabled for all input devices put into a low power state at
+run time.
+
+The subsystem-level resume callback is _entirely_ _responsible_ for handling the
+resume of the device as appropriate, which may, but need not include executing
+the device driver's own ->runtime_resume() callback (from the PM core's point of
+view it is not necessary to implement a ->runtime_resume() callback in a device
+driver as long as the subsystem-level resume callback knows what to do to handle
+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
+    of the device is then 'active'.
+
+  * If the subsystem-level resume callback returns an error code, the PM core
+    regards this as a fatal error and will refuse to run the helper functions
+    described in Section 4 for the device, until its status is directly set
+    either to 'active' or to 'suspended' (the PM core provides special helper
+    functions for this purpose).
+
+The subsystem-level idle callback is executed by the PM core whenever the device
+appears to be idle, which is indicated to the PM core by two counters, the
+device's usage counter and the counter of 'active' children of the device.
 
   * If any of these counters is decreased using a helper function provided by
     the PM core and it turns out to be equal to zero, the other counter is
     checked.  If that counter also is equal to zero, the PM core executes the
-    device bus type's ->runtime_idle() callback (with the device as an
-    argument).
+    subsystem-level idle callback with the device as an argument.
 
-The action performed by a bus type's ->runtime_idle() callback is totally
-dependent on the bus type in question, but the expected and recommended action
-is to check if the device can be suspended (i.e. if all of the conditions
-necessary for suspending the device are satisfied) and to queue up a suspend
-request for the device in that case.  The value returned by this callback is
-ignored by the PM core.
+The action performed by a subsystem-level idle callback is totally dependent on
+the subsystem in question, but the expected and recommended action is to check
+if the device can be suspended (i.e. if all of the conditions necessary for
+suspending the device are satisfied) and to queue up a suspend request for the
+device in that case.  The value returned by this callback is ignored by the PM
+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
@@ -238,41 +239,41 @@ drivers/base/power/runtime.c and include
       removing the device from device hierarchy
 
   int pm_runtime_idle(struct device *dev);
-    - execute ->runtime_idle() for the device's bus type; returns 0 on success
-      or error code on failure, where -EINPROGRESS means that ->runtime_idle()
-      is already being executed
+    - execute the subsystem-level idle callback for the device; returns 0 on
+      success or error code on failure, where -EINPROGRESS means that
+      ->runtime_idle() is already being executed
 
   int pm_runtime_suspend(struct device *dev);
-    - execute ->runtime_suspend() for the device's bus type; returns 0 on
+    - 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
 
   int pm_runtime_resume(struct device *dev);
-    - execute ->runtime_resume() for the device's bus type; returns 0 on
+    - execute the subsystem-leve resume callback for the device; returns 0 on
       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
 
   int pm_request_idle(struct device *dev);
-    - submit a request to execute ->runtime_idle() for the device's bus type
-      (the request is represented by a work item in pm_wq); returns 0 on success
-      or error code if the request has not been queued up
+    - submit a request to execute the subsystem-level idle callback for the
+      device (the request is represented by a work item in pm_wq); returns 0 on
+      success or error code if the request has not been queued up
 
   int pm_schedule_suspend(struct device *dev, unsigned int delay);
-    - schedule the execution of ->runtime_suspend() for the device's bus type
-      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
+    - schedule the execution of the subsystem-level suspend callback for the
+      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
       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
 
   int pm_request_resume(struct device *dev);
-    - submit a request to execute ->runtime_resume() for the device's bus type
-      (the request is represented by a work item in pm_wq); returns 0 on
+    - 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
       error code if the request hasn't been queued up
 
@@ -303,12 +304,12 @@ drivers/base/power/runtime.c and include
       run-time PM callbacks described in Section 2
 
   int pm_runtime_disable(struct device *dev);
-    - prevent the run-time PM helper functions from running the device bus
-      type's run-time PM callbacks, 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
-      ->runtime_resume() for the device's bus type to satisfy that request,
-      otherwise 0 is returned
+    - 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
+      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
 
   void pm_suspend_ignore_children(struct device *dev, bool enable);
     - set/unset the power.ignore_children flag of the device
@@ -378,5 +379,5 @@ pm_runtime_suspend() or pm_runtime_idle(
 they will fail returning -EAGAIN, because the device's usage counter is
 incremented by the core before executing ->probe() and ->remove().  Still, it
 may be desirable to suspend the device as soon as ->probe() or ->remove() has
-finished, so the PM core uses pm_runtime_idle_sync() to invoke the device bus
-type's ->runtime_idle() callback at that time.
+finished, so the PM core uses pm_runtime_idle_sync() to invoke the
+subsystem-level idle callback for the device at that time.

Re: Runtime PM: Calling Device runtime PM callbacks?

[Alan Stern]

On Sat, 19 Dec 2009, Rafael J. Wysocki wrote:

> On Tuesday 15 December 2009, Rafael J. Wysocki wrote:
> > On Tuesday 15 December 2009, Alan Stern wrote:
> > > On Tue, 15 Dec 2009, Rafael J. Wysocki wrote:
> ...
> > 
> > > > Now, if we agree that only one callback will be called for given device
> > > > (either bus type, or device type, or device class), the code may be simpler
> > > > and there won't be an issue with the ordering in _idle.
> > > 
> > > That would be perfectly fine with me.
> > 
> > OK
> 
> Updated patch is appended, please tell me what you think.

Yes, this is the sort of thing I had in mind.  Although it might be
nice to avoid all the repeated code.  For example, you might add a
helper routine:

	static int invoke_callback(struct device *dev,
			int (*func)(struct device *))
	{
		int retval;

		spin_unlock_irq(&dev->power.lock);
		retval = func(dev);
		spin_lock_irq(&dev->power.lock);
		return retval;
	}

Alan Stern

Re: Runtime PM: Calling Device runtime PM callbacks?

[Rafael J. Wysocki]

On Saturday 19 December 2009, Alan Stern wrote:
> On Sat, 19 Dec 2009, Rafael J. Wysocki wrote:
> 
> > On Tuesday 15 December 2009, Rafael J. Wysocki wrote:
> > > On Tuesday 15 December 2009, Alan Stern wrote:
> > > > On Tue, 15 Dec 2009, Rafael J. Wysocki wrote:
> > ...
> > > 
> > > > > Now, if we agree that only one callback will be called for given device
> > > > > (either bus type, or device type, or device class), the code may be simpler
> > > > > and there won't be an issue with the ordering in _idle.
> > > > 
> > > > That would be perfectly fine with me.
> > > 
> > > OK
> > 
> > Updated patch is appended, please tell me what you think.
> 
> Yes, this is the sort of thing I had in mind.  Although it might be
> nice to avoid all the repeated code.  For example, you might add a
> helper routine:
> 
> 	static int invoke_callback(struct device *dev,
> 			int (*func)(struct device *))
> 	{
> 		int retval;
> 
> 		spin_unlock_irq(&dev->power.lock);
> 		retval = func(dev);
> 		spin_lock_irq(&dev->power.lock);
> 		return retval;
> 	}

The problem with dev->power.runtime_error would reappear in
that case and I think the "unlock, do something, lock" structure looks odd. :-)

The only things repeated are unlocking, locking and
"dev->power.runtime_error = retval", which I think is not too much ...

Rafael