All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] PM: Make the PM core execute driver callbacks if subsystem ones are not present
@ 2011-12-09 23:18 Rafael J. Wysocki
  2011-12-09 23:20 ` [PATCH 1/4] PM / Sleep: Make pm_op() and pm_noirq_op() return callback pointers Rafael J. Wysocki
                   ` (5 more replies)
  0 siblings, 6 replies; 20+ messages in thread
From: Rafael J. Wysocki @ 2011-12-09 23:18 UTC (permalink / raw)
  To: Linux PM list; +Cc: Greg Kroah-Hartman, LKML, Russell King

Hi,

The following patchset changes the PM core so that driver callbacks
are executed automatically when subsystem ones are not present and
uses the new PM core's behavior to simplify the platform and AMBA bus
types (there probably are many more bus types that may be simplified
this way, but those two were the easiest targets for me).

The reasons why to do this are outlined in the changelog of the second patch.

Comments welcome!

Thanks,
Rafael


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 1/4] PM / Sleep: Make pm_op() and pm_noirq_op() return callback pointers
  2011-12-09 23:18 [PATCH 0/4] PM: Make the PM core execute driver callbacks if subsystem ones are not present Rafael J. Wysocki
@ 2011-12-09 23:20 ` Rafael J. Wysocki
  2011-12-12  4:04   ` Namhyung Kim
  2011-12-09 23:20 ` [PATCH 2/4] PM: Run the driver callback directly if the subsystem one is not there Rafael J. Wysocki
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 20+ messages in thread
From: Rafael J. Wysocki @ 2011-12-09 23:20 UTC (permalink / raw)
  To: Linux PM list; +Cc: Greg Kroah-Hartman, LKML, Russell King

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

Make the pm_op() and pm_noirq_op() functions return pointers to
appropriate callbacks instead of executing those callbacks and
returning their results.

This change is required for a subsequent modification that will
execute the corresponding driver callback if the subsystem
callback returned by either pm_op(), or pm_noirq_op() is NULL.

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

Index: linux/drivers/base/power/main.c
===================================================================
--- linux.orig/drivers/base/power/main.c
+++ linux/drivers/base/power/main.c
@@ -211,113 +211,70 @@ static void dpm_wait_for_children(struct
        device_for_each_child(dev, &async, dpm_wait_fn);
 }
 
-static int dpm_run_callback(struct device *dev, int (*cb)(struct device *))
-{
-	ktime_t calltime;
-	int error;
-
-	if (!cb)
-		return 0;
-
-	calltime = initcall_debug_start(dev);
-
-	error = cb(dev);
-	suspend_report_result(cb, error);
-
-	initcall_debug_report(dev, calltime, error);
-
-	return error;
-}
-
 /**
- * pm_op - Execute the PM operation appropriate for given PM event.
- * @dev: Device to handle.
+ * pm_op - Return the PM operation appropriate for given PM event.
  * @ops: PM operations to choose from.
  * @state: PM transition of the system being carried out.
  */
-static int pm_op(struct device *dev,
-		 const struct dev_pm_ops *ops,
-		 pm_message_t state)
+static int (*pm_op(const struct dev_pm_ops *ops, pm_message_t state))(struct device *)
 {
-	int error = 0;
-
 	switch (state.event) {
 #ifdef CONFIG_SUSPEND
 	case PM_EVENT_SUSPEND:
-		error = dpm_run_callback(dev, ops->suspend);
-		break;
+		return ops->suspend;
 	case PM_EVENT_RESUME:
-		error = dpm_run_callback(dev, ops->resume);
-		break;
+		return ops->resume;
 #endif /* CONFIG_SUSPEND */
 #ifdef CONFIG_HIBERNATE_CALLBACKS
 	case PM_EVENT_FREEZE:
 	case PM_EVENT_QUIESCE:
-		error = dpm_run_callback(dev, ops->freeze);
-		break;
+		return ops->freeze;
 	case PM_EVENT_HIBERNATE:
-		error = dpm_run_callback(dev, ops->poweroff);
-		break;
+		return ops->poweroff;
 	case PM_EVENT_THAW:
 	case PM_EVENT_RECOVER:
-		error = dpm_run_callback(dev, ops->thaw);
+		return ops->thaw;
 		break;
 	case PM_EVENT_RESTORE:
-		error = dpm_run_callback(dev, ops->restore);
-		break;
+		return ops->restore;
 #endif /* CONFIG_HIBERNATE_CALLBACKS */
-	default:
-		error = -EINVAL;
 	}
 
-	return error;
+	return NULL;
 }
 
 /**
- * pm_noirq_op - Execute the PM operation appropriate for given PM event.
- * @dev: Device to handle.
+ * pm_noirq_op - Return the PM operation appropriate for given PM event.
  * @ops: PM operations to choose from.
  * @state: PM transition of the system being carried out.
  *
  * The driver of @dev will not receive interrupts while this function is being
  * executed.
  */
-static int pm_noirq_op(struct device *dev,
-			const struct dev_pm_ops *ops,
-			pm_message_t state)
+static int (*pm_noirq_op(const struct dev_pm_ops *ops, pm_message_t state))(struct device *)
 {
-	int error = 0;
-
 	switch (state.event) {
 #ifdef CONFIG_SUSPEND
 	case PM_EVENT_SUSPEND:
-		error = dpm_run_callback(dev, ops->suspend_noirq);
-		break;
+		return ops->suspend_noirq;
 	case PM_EVENT_RESUME:
-		error = dpm_run_callback(dev, ops->resume_noirq);
-		break;
+		return ops->resume_noirq;
 #endif /* CONFIG_SUSPEND */
 #ifdef CONFIG_HIBERNATE_CALLBACKS
 	case PM_EVENT_FREEZE:
 	case PM_EVENT_QUIESCE:
-		error = dpm_run_callback(dev, ops->freeze_noirq);
-		break;
+		return ops->freeze_noirq;
 	case PM_EVENT_HIBERNATE:
-		error = dpm_run_callback(dev, ops->poweroff_noirq);
-		break;
+		return ops->poweroff_noirq;
 	case PM_EVENT_THAW:
 	case PM_EVENT_RECOVER:
-		error = dpm_run_callback(dev, ops->thaw_noirq);
-		break;
+		return ops->thaw_noirq;
 	case PM_EVENT_RESTORE:
-		error = dpm_run_callback(dev, ops->restore_noirq);
-		break;
+		return ops->restore_noirq;
 #endif /* CONFIG_HIBERNATE_CALLBACKS */
-	default:
-		error = -EINVAL;
 	}
 
-	return error;
+	return NULL;
 }
 
 static char *pm_verb(int event)
@@ -375,6 +332,26 @@ static void dpm_show_time(ktime_t startt
 		usecs / USEC_PER_MSEC, usecs % USEC_PER_MSEC);
 }
 
+static int dpm_run_callback(int (*cb)(struct device *), struct device *dev,
+			    pm_message_t state, char *info)
+{
+	ktime_t calltime;
+	int error;
+
+	if (!cb)
+		return 0;
+
+	calltime = initcall_debug_start(dev);
+
+	pm_dev_dbg(dev, state, info);
+	error = cb(dev);
+	suspend_report_result(cb, error);
+
+	initcall_debug_report(dev, calltime, error);
+
+	return error;
+}
+
 /*------------------------- Resume routines -------------------------*/
 
 /**
@@ -387,25 +364,29 @@ static void dpm_show_time(ktime_t startt
  */
 static int device_resume_noirq(struct device *dev, pm_message_t state)
 {
+	int (*callback)(struct device *) = NULL;
+	char *info = NULL;
 	int error = 0;
 
 	TRACE_DEVICE(dev);
 	TRACE_RESUME(0);
 
 	if (dev->pm_domain) {
-		pm_dev_dbg(dev, state, "EARLY power domain ");
-		error = pm_noirq_op(dev, &dev->pm_domain->ops, state);
+		info = "EARLY power domain ";
+		callback = pm_noirq_op(&dev->pm_domain->ops, state);
 	} else if (dev->type && dev->type->pm) {
-		pm_dev_dbg(dev, state, "EARLY type ");
-		error = pm_noirq_op(dev, dev->type->pm, state);
+		info = "EARLY type ";
+		callback = pm_noirq_op(dev->type->pm, state);
 	} else if (dev->class && dev->class->pm) {
-		pm_dev_dbg(dev, state, "EARLY class ");
-		error = pm_noirq_op(dev, dev->class->pm, state);
+		info = "EARLY class ";
+		callback = pm_noirq_op(dev->class->pm, state);
 	} else if (dev->bus && dev->bus->pm) {
-		pm_dev_dbg(dev, state, "EARLY ");
-		error = pm_noirq_op(dev, dev->bus->pm, state);
+		info = "EARLY ";
+		callback = pm_noirq_op(dev->bus->pm, state);
 	}
 
+	error = dpm_run_callback(callback, dev, state, info);
+
 	TRACE_RESUME(error);
 	return error;
 }
@@ -455,6 +436,8 @@ EXPORT_SYMBOL_GPL(dpm_resume_noirq);
  */
 static int device_resume(struct device *dev, pm_message_t state, bool async)
 {
+	int (*callback)(struct device *) = NULL;
+	char *info = NULL;
 	int error = 0;
 	bool put = false;
 
@@ -477,40 +460,41 @@ static int device_resume(struct device *
 	put = true;
 
 	if (dev->pm_domain) {
-		pm_dev_dbg(dev, state, "power domain ");
-		error = pm_op(dev, &dev->pm_domain->ops, state);
+		info = "power domain ";
+		callback = pm_op(&dev->pm_domain->ops, state);
 		goto End;
 	}
 
 	if (dev->type && dev->type->pm) {
-		pm_dev_dbg(dev, state, "type ");
-		error = pm_op(dev, dev->type->pm, state);
+		info = "type ";
+		callback = pm_op(dev->type->pm, state);
 		goto End;
 	}
 
 	if (dev->class) {
 		if (dev->class->pm) {
-			pm_dev_dbg(dev, state, "class ");
-			error = pm_op(dev, dev->class->pm, state);
+			info = "class ";
+			callback = pm_op(dev->class->pm, state);
 			goto End;
 		} else if (dev->class->resume) {
-			pm_dev_dbg(dev, state, "legacy class ");
-			error = dpm_run_callback(dev, dev->class->resume);
+			info = "legacy class ";
+			callback = dev->class->resume;
 			goto End;
 		}
 	}
 
 	if (dev->bus) {
 		if (dev->bus->pm) {
-			pm_dev_dbg(dev, state, "");
-			error = pm_op(dev, dev->bus->pm, state);
+			info = "";
+			callback = pm_op(dev->bus->pm, state);
 		} else if (dev->bus->resume) {
-			pm_dev_dbg(dev, state, "legacy ");
-			error = dpm_run_callback(dev, dev->bus->resume);
+			info = "legacy ";
+			callback = dev->bus->resume;
 		}
 	}
 
  End:
+	error = dpm_run_callback(callback, dev, state, info);
 	dev->power.is_suspended = false;
 
  Unlock:
@@ -705,23 +689,24 @@ static pm_message_t resume_event(pm_mess
  */
 static int device_suspend_noirq(struct device *dev, pm_message_t state)
 {
-	int error = 0;
+	int (*callback)(struct device *) = NULL;
+	char *info = NULL;
 
 	if (dev->pm_domain) {
-		pm_dev_dbg(dev, state, "LATE power domain ");
-		error = pm_noirq_op(dev, &dev->pm_domain->ops, state);
+		info = "LATE power domain ";
+		callback = pm_noirq_op(&dev->pm_domain->ops, state);
 	} else if (dev->type && dev->type->pm) {
-		pm_dev_dbg(dev, state, "LATE type ");
-		error = pm_noirq_op(dev, dev->type->pm, state);
+		info = "LATE type ";
+		callback = pm_noirq_op(dev->type->pm, state);
 	} else if (dev->class && dev->class->pm) {
-		pm_dev_dbg(dev, state, "LATE class ");
-		error = pm_noirq_op(dev, dev->class->pm, state);
+		info = "LATE class ";
+		callback = pm_noirq_op(dev->class->pm, state);
 	} else if (dev->bus && dev->bus->pm) {
-		pm_dev_dbg(dev, state, "LATE ");
-		error = pm_noirq_op(dev, dev->bus->pm, state);
+		info = "LATE ";
+		callback = pm_noirq_op(dev->bus->pm, state);
 	}
 
-	return error;
+	return dpm_run_callback(callback, dev, state, info);
 }
 
 /**
@@ -798,6 +783,8 @@ static int legacy_suspend(struct device
  */
 static int __device_suspend(struct device *dev, pm_message_t state, bool async)
 {
+	int (*callback)(struct device *) = NULL;
+	char *info = NULL;
 	int error = 0;
 
 	dpm_wait_for_children(dev, async);
@@ -818,22 +805,22 @@ static int __device_suspend(struct devic
 	device_lock(dev);
 
 	if (dev->pm_domain) {
-		pm_dev_dbg(dev, state, "power domain ");
-		error = pm_op(dev, &dev->pm_domain->ops, state);
-		goto End;
+		info = "power domain ";
+		callback = pm_op(&dev->pm_domain->ops, state);
+		goto Run;
 	}
 
 	if (dev->type && dev->type->pm) {
-		pm_dev_dbg(dev, state, "type ");
-		error = pm_op(dev, dev->type->pm, state);
-		goto End;
+		info = "type ";
+		callback = pm_op(dev->type->pm, state);
+		goto Run;
 	}
 
 	if (dev->class) {
 		if (dev->class->pm) {
-			pm_dev_dbg(dev, state, "class ");
-			error = pm_op(dev, dev->class->pm, state);
-			goto End;
+			info = "class ";
+			callback = pm_op(dev->class->pm, state);
+			goto Run;
 		} else if (dev->class->suspend) {
 			pm_dev_dbg(dev, state, "legacy class ");
 			error = legacy_suspend(dev, state, dev->class->suspend);
@@ -843,14 +830,18 @@ static int __device_suspend(struct devic
 
 	if (dev->bus) {
 		if (dev->bus->pm) {
-			pm_dev_dbg(dev, state, "");
-			error = pm_op(dev, dev->bus->pm, state);
+			info = "";
+			callback = pm_op(dev->bus->pm, state);
 		} else if (dev->bus->suspend) {
 			pm_dev_dbg(dev, state, "legacy ");
 			error = legacy_suspend(dev, state, dev->bus->suspend);
+			goto End;
 		}
 	}
 
+ Run:
+	error = dpm_run_callback(callback, dev, state, info);
+
  End:
 	if (!error) {
 		dev->power.is_suspended = true;


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 2/4] PM: Run the driver callback directly if the subsystem one is not there
  2011-12-09 23:18 [PATCH 0/4] PM: Make the PM core execute driver callbacks if subsystem ones are not present Rafael J. Wysocki
  2011-12-09 23:20 ` [PATCH 1/4] PM / Sleep: Make pm_op() and pm_noirq_op() return callback pointers Rafael J. Wysocki
@ 2011-12-09 23:20 ` Rafael J. Wysocki
  2011-12-10 14:59   ` Alan Stern
  2011-12-09 23:21 ` [PATCH 3/4] PM / Sleep: Remove forward-only callbacks from platform bus type Rafael J. Wysocki
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 20+ messages in thread
From: Rafael J. Wysocki @ 2011-12-09 23:20 UTC (permalink / raw)
  To: Linux PM list; +Cc: Greg Kroah-Hartman, LKML, Russell King

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

Make the PM core execute driver PM callbacks directly if the
corresponding subsystem callbacks are not present.

There are three reasons for doing that.  First, it reflects the
behavior of drivers/base/dd.c:really_probe() that runs the driver's
.probe() callback directly if the bus type's one is not defined, so
this change will remove one arbitrary difference between the PM core
and the remaining parts of the driver core.  Second, it will allow
some subsystems, whose PM callbacks don't do anything except for
executing driver callbacks, to be simplified quite a bit by removing
those "forward-only" callbacks.  Finally, it will allow us to remove
one level of indirection in the system suspend and resume code paths
where it is not necessary, which is going to lead to less debug noise
with initcall_debug passed in the kernel command line (messages won't
be printed for driverless devices whose subsystems don't provide
PM callbacks among other things).

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 Documentation/power/devices.txt    |   39 ++++++-----
 Documentation/power/runtime_pm.txt |  128 +++++++++++++++++++------------------
 drivers/base/power/main.c          |  109 ++++++++++++++++++++-----------
 drivers/base/power/runtime.c       |    9 ++
 4 files changed, 170 insertions(+), 115 deletions(-)

Index: linux/drivers/base/power/runtime.c
===================================================================
--- linux.orig/drivers/base/power/runtime.c
+++ linux/drivers/base/power/runtime.c
@@ -250,6 +250,9 @@ static int rpm_idle(struct device *dev,
 	else
 		callback = NULL;
 
+	if (!callback && dev->driver && dev->driver->pm)
+		callback = dev->driver->pm->runtime_idle;
+
 	if (callback)
 		__rpm_callback(callback, dev);
 
@@ -713,6 +716,12 @@ static int rpm_resume(struct device *dev
 	else
 		callback = NULL;
 
+	if (!callback && dev->driver && dev->driver->pm)
+		callback = dev->driver->pm->runtime_suspend;
+
+	if (!callback && dev->driver && dev->driver->pm)
+		callback = dev->driver->pm->runtime_resume;
+
 	retval = rpm_callback(callback, dev);
 	if (retval) {
 		__update_runtime_status(dev, RPM_SUSPENDED);
Index: linux/Documentation/power/devices.txt
===================================================================
--- linux.orig/Documentation/power/devices.txt
+++ linux/Documentation/power/devices.txt
@@ -126,7 +126,9 @@ The core methods to suspend and resume d
 pointed to by the ops member of struct dev_pm_domain, or by the pm member of
 struct bus_type, struct device_type and struct class.  They are mostly of
 interest to the people writing infrastructure for platforms and buses, like PCI
-or USB, or device type and device class drivers.
+or USB, or device type and device class drivers.  They also are relevant to the
+writers of device drivers whose subsystems (PM domains, device types, device
+classes and bus types) don't provide all power management methods.
 
 Bus drivers implement these methods as appropriate for the hardware and the
 drivers using it; PCI works differently from USB, and so on.  Not many people
@@ -268,32 +270,35 @@ various phases always run after tasks ha
 unfrozen.  Furthermore, the *_noirq phases run at a time when IRQ handlers have
 been disabled (except for those marked with the IRQF_NO_SUSPEND flag).
 
-All phases use PM domain, bus, type, or class callbacks (that is, methods
-defined in dev->pm_domain->ops, dev->bus->pm, dev->type->pm, or dev->class->pm).
-These callbacks are regarded by the PM core as mutually exclusive.  Moreover,
-PM domain callbacks always take precedence over bus, type and class callbacks,
-while type callbacks take precedence over bus and class callbacks, and class
-callbacks take precedence over bus callbacks.  To be precise, the following
-rules are used to determine which callback to execute in the given phase:
-
-    1.	If dev->pm_domain is present, the PM core will attempt to execute the
-	callback included in dev->pm_domain->ops.  If that callback is not
-	present, no action will be carried out for the given device.
+All phases use PM domain, bus, type, class or driver callbacks (that is, methods
+defined in dev->pm_domain->ops, dev->bus->pm, dev->type->pm, dev->class->pm or
+dev->driver->pm).  These callbacks are regarded by the PM core as mutually
+exclusive.  Moreover, PM domain callbacks always take precedence over all of the
+other callbacks and, for example, type callbacks take precedence over bus, class
+and driver callbacks.  To be precise, the following rules are used to determine
+which callback to execute in the given phase:
+
+    1.	If dev->pm_domain is present, the PM core will choose the callback
+	included in dev->pm_domain->ops for execution
 
     2.	Otherwise, if both dev->type and dev->type->pm are present, the callback
-	included in dev->type->pm will be executed.
+	included in dev->type->pm will be chosen for execution.
 
     3.	Otherwise, if both dev->class and dev->class->pm are present, the
-	callback included in dev->class->pm will be executed.
+	callback included in dev->class->pm will be chosen for execution.
 
     4.	Otherwise, if both dev->bus and dev->bus->pm are present, the callback
-	included in dev->bus->pm will be executed.
+	included in dev->bus->pm will be chosen for execution.
 
 This allows PM domains and device types to override callbacks provided by bus
 types or device classes if necessary.
 
-These callbacks may in turn invoke device- or driver-specific methods stored in
-dev->driver->pm, but they don't have to.
+The PM domain, type, class and bus callbacks may in turn invoke device- or
+driver-specific methods stored in dev->driver->pm, but they don't have to do
+that.
+
+If the subsystem callback chosen for execution is not present, the PM core will
+execute the corresponding method from dev->driver->pm instead if there is one.
 
 
 Entering System Suspend
Index: linux/Documentation/power/runtime_pm.txt
===================================================================
--- linux.orig/Documentation/power/runtime_pm.txt
+++ linux/Documentation/power/runtime_pm.txt
@@ -57,6 +57,10 @@ the following:
 
   4. Bus type of the device, if both dev->bus and dev->bus->pm are present.
 
+If the subsystem chosen by applying the above rules doesn't provide the relevant
+callback, the PM core will invoke the corresponding driver callback stored in
+dev->driver->pm directly (if present).
+
 The PM core always checks which callback to use in the order given above, so the
 priority order of callbacks from high to low is: PM domain, device type, class
 and bus type.  Moreover, the high-priority one will always take precedence over
@@ -64,86 +68,88 @@ a low-priority one.  The PM domain, bus
 are referred to as subsystem-level callbacks in what follows.
 
 By default, the callbacks are always invoked in process context with interrupts
-enabled.  However, subsystems can use the pm_runtime_irq_safe() helper function
-to tell the PM core that their ->runtime_suspend(), ->runtime_resume() and
-->runtime_idle() callbacks may be invoked in atomic context with interrupts
-disabled for a given device.  This implies that the callback routines in
-question must not block or sleep, but it also means that the synchronous helper
-functions listed at the end of Section 4 may be used for that device within an
-interrupt handler or generally in an atomic context.
-
-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
+enabled.  However, the pm_runtime_irq_safe() helper function can be used to tell
+the PM core that it is safe to run the ->runtime_suspend(), ->runtime_resume()
+and ->runtime_idle() callbacks for the given device in atomic context with
+interrupts disabled.  This implies that the callback routines in question must
+not block or sleep, but it also means that the synchronous helper functions
+listed at the end of Section 4 may be used for that device within an interrupt
+handler or generally in an atomic context.
+
+The subsystem-level suspend callback, if present, 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 subsystem-level suspend callback
 knows what to do to handle the device).
 
-  * 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 the subsystem-level resume
-    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 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
-    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).
+  * Once the subsystem-level suspend callback (or the driver suspend callback,
+    if invoked directly) has completed successfully for the given device, the PM
+    core regards the device as suspended, which need not mean that it 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 appropriate resume callback is executed for it.  The runtime
+    PM status of a device after successful execution of the suspend callback is
+    'suspended'.
+
+  * If the suspend callback returns -EBUSY or -EAGAIN, the device's runtime PM
+    status remains 'active', which means that the device _must_ be fully
+    operational afterwards.
+
+  * If the suspend callback returns an error code different from -EBUSY and
+    -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 its status
+    is directly set to  either'active', or 'suspended' (the PM core provides
+    special helper functions for this purpose).
 
-In particular, if the driver requires remote wake-up capability (i.e. hardware
+In particular, if the driver requires remote wakeup 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 runtime 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.
+device_run_wake() returns 'true' for the device and the device is put into a
+low-power state during the execution of the suspend callback, it is expected
+that remote wakeup will be enabled for the device.  Generally, remote wakeup
+should be enabled for all input devices put into low-power states at run time.
+
+The subsystem-level resume callback, if present, 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 (or the driver resume callback, if
+    invoked directly) 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 runtime PM status of the device is then
+    'active'.
+
+  * If the 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 to either 'active', or
+    'suspended' (by means of special helper functions provided by the PM core
+    for this purpose).
+
+The idle callback (a subsystem-level one, if present, or the driver one) 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
-    subsystem-level idle callback with the device as an argument.
+    idle callback with the device as its argument.
 
-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
+The action performed by the idle callback is totally dependent on the subsystem
+(or driver) 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 runtime
-PM callbacks:
+that the following constraints are met with respect to runtime PM callbacks for
+one device:
 
 (1) The callbacks are mutually exclusive (e.g. it is forbidden to execute
     ->runtime_suspend() in parallel with ->runtime_resume() or with another
Index: linux/drivers/base/power/main.c
===================================================================
--- linux.orig/drivers/base/power/main.c
+++ linux/drivers/base/power/main.c
@@ -381,10 +381,15 @@ static int device_resume_noirq(struct de
 		info = "EARLY class ";
 		callback = pm_noirq_op(dev->class->pm, state);
 	} else if (dev->bus && dev->bus->pm) {
-		info = "EARLY ";
+		info = "EARLY bus ";
 		callback = pm_noirq_op(dev->bus->pm, state);
 	}
 
+	if (!callback && dev->driver && dev->driver->pm) {
+		info = "EARLY driver ";
+		callback = pm_noirq_op(dev->driver->pm, state);
+	}
+
 	error = dpm_run_callback(callback, dev, state, info);
 
 	TRACE_RESUME(error);
@@ -462,20 +467,20 @@ static int device_resume(struct device *
 	if (dev->pm_domain) {
 		info = "power domain ";
 		callback = pm_op(&dev->pm_domain->ops, state);
-		goto End;
+		goto Driver;
 	}
 
 	if (dev->type && dev->type->pm) {
 		info = "type ";
 		callback = pm_op(dev->type->pm, state);
-		goto End;
+		goto Driver;
 	}
 
 	if (dev->class) {
 		if (dev->class->pm) {
 			info = "class ";
 			callback = pm_op(dev->class->pm, state);
-			goto End;
+			goto Driver;
 		} else if (dev->class->resume) {
 			info = "legacy class ";
 			callback = dev->class->resume;
@@ -485,14 +490,21 @@ static int device_resume(struct device *
 
 	if (dev->bus) {
 		if (dev->bus->pm) {
-			info = "";
+			info = "bus ";
 			callback = pm_op(dev->bus->pm, state);
 		} else if (dev->bus->resume) {
-			info = "legacy ";
+			info = "legacy bus ";
 			callback = dev->bus->resume;
+			goto End;
 		}
 	}
 
+ Driver:
+	if (!callback && dev->driver && dev->driver->pm) {
+		info = "driver ";
+		callback = pm_op(dev->driver->pm, state);
+	}
+
  End:
 	error = dpm_run_callback(callback, dev, state, info);
 	dev->power.is_suspended = false;
@@ -586,24 +598,33 @@ void dpm_resume(pm_message_t state)
  */
 static void device_complete(struct device *dev, pm_message_t state)
 {
+	void (*callback)(struct device *) = NULL;
+	char *info = NULL;
+
 	device_lock(dev);
 
 	if (dev->pm_domain) {
-		pm_dev_dbg(dev, state, "completing power domain ");
-		if (dev->pm_domain->ops.complete)
-			dev->pm_domain->ops.complete(dev);
+		info = "completing power domain ";
+		callback = dev->pm_domain->ops.complete;
 	} else if (dev->type && dev->type->pm) {
-		pm_dev_dbg(dev, state, "completing type ");
-		if (dev->type->pm->complete)
-			dev->type->pm->complete(dev);
+		info = "completing type ";
+		callback = dev->type->pm->complete;
 	} else if (dev->class && dev->class->pm) {
-		pm_dev_dbg(dev, state, "completing class ");
-		if (dev->class->pm->complete)
-			dev->class->pm->complete(dev);
+		info = "completing class ";
+		callback = dev->class->pm->complete;
 	} else if (dev->bus && dev->bus->pm) {
-		pm_dev_dbg(dev, state, "completing ");
-		if (dev->bus->pm->complete)
-			dev->bus->pm->complete(dev);
+		info = "completing bus ";
+		callback = dev->bus->pm->complete;
+	}
+
+	if (!callback && dev->driver && dev->driver->pm) {
+		info = "completing driver ";
+		callback = dev->driver->pm->complete;
+	}
+
+	if (callback) {
+		pm_dev_dbg(dev, state, info);
+		callback(dev);
 	}
 
 	device_unlock(dev);
@@ -702,10 +723,15 @@ static int device_suspend_noirq(struct d
 		info = "LATE class ";
 		callback = pm_noirq_op(dev->class->pm, state);
 	} else if (dev->bus && dev->bus->pm) {
-		info = "LATE ";
+		info = "LATE bus ";
 		callback = pm_noirq_op(dev->bus->pm, state);
 	}
 
+	if (!callback && dev->driver && dev->driver->pm) {
+		info = "LATE driver ";
+		callback = pm_noirq_op(dev->driver->pm, state);
+	}
+
 	return dpm_run_callback(callback, dev, state, info);
 }
 
@@ -830,16 +856,21 @@ static int __device_suspend(struct devic
 
 	if (dev->bus) {
 		if (dev->bus->pm) {
-			info = "";
+			info = "bus ";
 			callback = pm_op(dev->bus->pm, state);
 		} else if (dev->bus->suspend) {
-			pm_dev_dbg(dev, state, "legacy ");
+			pm_dev_dbg(dev, state, "legacy bus ");
 			error = legacy_suspend(dev, state, dev->bus->suspend);
 			goto End;
 		}
 	}
 
  Run:
+	if (!callback && dev->driver && dev->driver->pm) {
+		info = "driver ";
+		callback = pm_op(dev->driver->pm, state);
+	}
+
 	error = dpm_run_callback(callback, dev, state, info);
 
  End:
@@ -947,6 +978,8 @@ int dpm_suspend(pm_message_t state)
  */
 static int device_prepare(struct device *dev, pm_message_t state)
 {
+	int (*callback)(struct device *) = NULL;
+	char *info = NULL;
 	int error = 0;
 
 	device_lock(dev);
@@ -954,25 +987,27 @@ static int device_prepare(struct device
 	dev->power.wakeup_path = device_may_wakeup(dev);
 
 	if (dev->pm_domain) {
-		pm_dev_dbg(dev, state, "preparing power domain ");
-		if (dev->pm_domain->ops.prepare)
-			error = dev->pm_domain->ops.prepare(dev);
-		suspend_report_result(dev->pm_domain->ops.prepare, error);
+		info = "preparing power domain ";
+		callback = dev->pm_domain->ops.prepare;
 	} else if (dev->type && dev->type->pm) {
-		pm_dev_dbg(dev, state, "preparing type ");
-		if (dev->type->pm->prepare)
-			error = dev->type->pm->prepare(dev);
-		suspend_report_result(dev->type->pm->prepare, error);
+		info = "preparing type ";
+		callback = dev->type->pm->prepare;
 	} else if (dev->class && dev->class->pm) {
-		pm_dev_dbg(dev, state, "preparing class ");
-		if (dev->class->pm->prepare)
-			error = dev->class->pm->prepare(dev);
-		suspend_report_result(dev->class->pm->prepare, error);
+		info = "preparing class ";
+		callback = dev->class->pm->prepare;
 	} else if (dev->bus && dev->bus->pm) {
-		pm_dev_dbg(dev, state, "preparing ");
-		if (dev->bus->pm->prepare)
-			error = dev->bus->pm->prepare(dev);
-		suspend_report_result(dev->bus->pm->prepare, error);
+		info = "preparing bus ";
+		callback = dev->bus->pm->prepare;
+	}
+
+	if (!callback && dev->driver && dev->driver->pm) {
+		info = "preparing driver ";
+		callback = dev->driver->pm->prepare;
+	}
+
+	if (callback) {
+		error = callback(dev);
+		suspend_report_result(callback, error);
 	}
 
 	device_unlock(dev);


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 3/4] PM / Sleep: Remove forward-only callbacks from platform bus type
  2011-12-09 23:18 [PATCH 0/4] PM: Make the PM core execute driver callbacks if subsystem ones are not present Rafael J. Wysocki
  2011-12-09 23:20 ` [PATCH 1/4] PM / Sleep: Make pm_op() and pm_noirq_op() return callback pointers Rafael J. Wysocki
  2011-12-09 23:20 ` [PATCH 2/4] PM: Run the driver callback directly if the subsystem one is not there Rafael J. Wysocki
@ 2011-12-09 23:21 ` Rafael J. Wysocki
  2011-12-09 23:23 ` [PATCH 4/4] PM / Sleep: Remove forward-only callbacks from AMBA " Rafael J. Wysocki
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 20+ messages in thread
From: Rafael J. Wysocki @ 2011-12-09 23:21 UTC (permalink / raw)
  To: Linux PM list; +Cc: Greg Kroah-Hartman, LKML, Russell King

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

The forward-only PM callbacks provided by the platform bus type are
not necessary any more, because the PM core executes driver callbacks
when the corresponding subsystem callbacks are not present, so drop
them.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/base/platform.c         |  115 ----------------------------------------
 include/linux/platform_device.h |   30 ----------
 2 files changed, 1 insertion(+), 144 deletions(-)

Index: linux/drivers/base/platform.c
===================================================================
--- linux.orig/drivers/base/platform.c
+++ linux/drivers/base/platform.c
@@ -700,25 +700,6 @@ static int platform_legacy_resume(struct
 	return ret;
 }
 
-int platform_pm_prepare(struct device *dev)
-{
-	struct device_driver *drv = dev->driver;
-	int ret = 0;
-
-	if (drv && drv->pm && drv->pm->prepare)
-		ret = drv->pm->prepare(dev);
-
-	return ret;
-}
-
-void platform_pm_complete(struct device *dev)
-{
-	struct device_driver *drv = dev->driver;
-
-	if (drv && drv->pm && drv->pm->complete)
-		drv->pm->complete(dev);
-}
-
 #endif /* CONFIG_PM_SLEEP */
 
 #ifdef CONFIG_SUSPEND
@@ -741,22 +722,6 @@ int platform_pm_suspend(struct device *d
 	return ret;
 }
 
-int platform_pm_suspend_noirq(struct device *dev)
-{
-	struct device_driver *drv = dev->driver;
-	int ret = 0;
-
-	if (!drv)
-		return 0;
-
-	if (drv->pm) {
-		if (drv->pm->suspend_noirq)
-			ret = drv->pm->suspend_noirq(dev);
-	}
-
-	return ret;
-}
-
 int platform_pm_resume(struct device *dev)
 {
 	struct device_driver *drv = dev->driver;
@@ -775,22 +740,6 @@ int platform_pm_resume(struct device *de
 	return ret;
 }
 
-int platform_pm_resume_noirq(struct device *dev)
-{
-	struct device_driver *drv = dev->driver;
-	int ret = 0;
-
-	if (!drv)
-		return 0;
-
-	if (drv->pm) {
-		if (drv->pm->resume_noirq)
-			ret = drv->pm->resume_noirq(dev);
-	}
-
-	return ret;
-}
-
 #endif /* CONFIG_SUSPEND */
 
 #ifdef CONFIG_HIBERNATE_CALLBACKS
@@ -813,22 +762,6 @@ int platform_pm_freeze(struct device *de
 	return ret;
 }
 
-int platform_pm_freeze_noirq(struct device *dev)
-{
-	struct device_driver *drv = dev->driver;
-	int ret = 0;
-
-	if (!drv)
-		return 0;
-
-	if (drv->pm) {
-		if (drv->pm->freeze_noirq)
-			ret = drv->pm->freeze_noirq(dev);
-	}
-
-	return ret;
-}
-
 int platform_pm_thaw(struct device *dev)
 {
 	struct device_driver *drv = dev->driver;
@@ -847,22 +780,6 @@ int platform_pm_thaw(struct device *dev)
 	return ret;
 }
 
-int platform_pm_thaw_noirq(struct device *dev)
-{
-	struct device_driver *drv = dev->driver;
-	int ret = 0;
-
-	if (!drv)
-		return 0;
-
-	if (drv->pm) {
-		if (drv->pm->thaw_noirq)
-			ret = drv->pm->thaw_noirq(dev);
-	}
-
-	return ret;
-}
-
 int platform_pm_poweroff(struct device *dev)
 {
 	struct device_driver *drv = dev->driver;
@@ -881,22 +798,6 @@ int platform_pm_poweroff(struct device *
 	return ret;
 }
 
-int platform_pm_poweroff_noirq(struct device *dev)
-{
-	struct device_driver *drv = dev->driver;
-	int ret = 0;
-
-	if (!drv)
-		return 0;
-
-	if (drv->pm) {
-		if (drv->pm->poweroff_noirq)
-			ret = drv->pm->poweroff_noirq(dev);
-	}
-
-	return ret;
-}
-
 int platform_pm_restore(struct device *dev)
 {
 	struct device_driver *drv = dev->driver;
@@ -913,22 +814,6 @@ int platform_pm_restore(struct device *d
 	}
 
 	return ret;
-}
-
-int platform_pm_restore_noirq(struct device *dev)
-{
-	struct device_driver *drv = dev->driver;
-	int ret = 0;
-
-	if (!drv)
-		return 0;
-
-	if (drv->pm) {
-		if (drv->pm->restore_noirq)
-			ret = drv->pm->restore_noirq(dev);
-	}
-
-	return ret;
 }
 
 #endif /* CONFIG_HIBERNATE_CALLBACKS */
Index: linux/include/linux/platform_device.h
===================================================================
--- linux.orig/include/linux/platform_device.h
+++ linux/include/linux/platform_device.h
@@ -264,62 +264,34 @@ static inline char *early_platform_drive
 }
 #endif /* MODULE */
 
-#ifdef CONFIG_PM_SLEEP
-extern int platform_pm_prepare(struct device *dev);
-extern void platform_pm_complete(struct device *dev);
-#else
-#define platform_pm_prepare	NULL
-#define platform_pm_complete	NULL
-#endif
-
 #ifdef CONFIG_SUSPEND
 extern int platform_pm_suspend(struct device *dev);
-extern int platform_pm_suspend_noirq(struct device *dev);
 extern int platform_pm_resume(struct device *dev);
-extern int platform_pm_resume_noirq(struct device *dev);
 #else
 #define platform_pm_suspend		NULL
 #define platform_pm_resume		NULL
-#define platform_pm_suspend_noirq	NULL
-#define platform_pm_resume_noirq	NULL
 #endif
 
 #ifdef CONFIG_HIBERNATE_CALLBACKS
 extern int platform_pm_freeze(struct device *dev);
-extern int platform_pm_freeze_noirq(struct device *dev);
 extern int platform_pm_thaw(struct device *dev);
-extern int platform_pm_thaw_noirq(struct device *dev);
 extern int platform_pm_poweroff(struct device *dev);
-extern int platform_pm_poweroff_noirq(struct device *dev);
 extern int platform_pm_restore(struct device *dev);
-extern int platform_pm_restore_noirq(struct device *dev);
 #else
 #define platform_pm_freeze		NULL
 #define platform_pm_thaw		NULL
 #define platform_pm_poweroff		NULL
 #define platform_pm_restore		NULL
-#define platform_pm_freeze_noirq	NULL
-#define platform_pm_thaw_noirq		NULL
-#define platform_pm_poweroff_noirq	NULL
-#define platform_pm_restore_noirq	NULL
 #endif
 
 #ifdef CONFIG_PM_SLEEP
 #define USE_PLATFORM_PM_SLEEP_OPS \
-	.prepare = platform_pm_prepare, \
-	.complete = platform_pm_complete, \
 	.suspend = platform_pm_suspend, \
 	.resume = platform_pm_resume, \
 	.freeze = platform_pm_freeze, \
 	.thaw = platform_pm_thaw, \
 	.poweroff = platform_pm_poweroff, \
-	.restore = platform_pm_restore, \
-	.suspend_noirq = platform_pm_suspend_noirq, \
-	.resume_noirq = platform_pm_resume_noirq, \
-	.freeze_noirq = platform_pm_freeze_noirq, \
-	.thaw_noirq = platform_pm_thaw_noirq, \
-	.poweroff_noirq = platform_pm_poweroff_noirq, \
-	.restore_noirq = platform_pm_restore_noirq,
+	.restore = platform_pm_restore,
 #else
 #define USE_PLATFORM_PM_SLEEP_OPS
 #endif


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 4/4] PM / Sleep: Remove forward-only callbacks from AMBA bus type
  2011-12-09 23:18 [PATCH 0/4] PM: Make the PM core execute driver callbacks if subsystem ones are not present Rafael J. Wysocki
                   ` (2 preceding siblings ...)
  2011-12-09 23:21 ` [PATCH 3/4] PM / Sleep: Remove forward-only callbacks from platform bus type Rafael J. Wysocki
@ 2011-12-09 23:23 ` Rafael J. Wysocki
  2011-12-09 23:47 ` [PATCH 0/4] PM: Make the PM core execute driver callbacks if subsystem ones are not present Greg KH
  2011-12-15 22:34 ` [Update][PATCH 0/5] " Rafael J. Wysocki
  5 siblings, 0 replies; 20+ messages in thread
From: Rafael J. Wysocki @ 2011-12-09 23:23 UTC (permalink / raw)
  To: Linux PM list; +Cc: Greg Kroah-Hartman, LKML, Russell King

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

The forward-only PM callbacks provided by the AMBA bus type are not
necessary any more, because the PM core executes driver callbacks
when the corresponding subsystem callbacks are not present, so drop
them.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/amba/bus.c |  136 -----------------------------------------------------
 1 file changed, 1 insertion(+), 135 deletions(-)

Index: linux/drivers/amba/bus.c
===================================================================
--- linux.orig/drivers/amba/bus.c
+++ linux/drivers/amba/bus.c
@@ -109,31 +109,7 @@ static int amba_legacy_resume(struct dev
 	return ret;
 }
 
-static int amba_pm_prepare(struct device *dev)
-{
-	struct device_driver *drv = dev->driver;
-	int ret = 0;
-
-	if (drv && drv->pm && drv->pm->prepare)
-		ret = drv->pm->prepare(dev);
-
-	return ret;
-}
-
-static void amba_pm_complete(struct device *dev)
-{
-	struct device_driver *drv = dev->driver;
-
-	if (drv && drv->pm && drv->pm->complete)
-		drv->pm->complete(dev);
-}
-
-#else /* !CONFIG_PM_SLEEP */
-
-#define amba_pm_prepare		NULL
-#define amba_pm_complete		NULL
-
-#endif /* !CONFIG_PM_SLEEP */
+#endif /* CONFIG_PM_SLEEP */
 
 #ifdef CONFIG_SUSPEND
 
@@ -155,22 +131,6 @@ static int amba_pm_suspend(struct device
 	return ret;
 }
 
-static int amba_pm_suspend_noirq(struct device *dev)
-{
-	struct device_driver *drv = dev->driver;
-	int ret = 0;
-
-	if (!drv)
-		return 0;
-
-	if (drv->pm) {
-		if (drv->pm->suspend_noirq)
-			ret = drv->pm->suspend_noirq(dev);
-	}
-
-	return ret;
-}
-
 static int amba_pm_resume(struct device *dev)
 {
 	struct device_driver *drv = dev->driver;
@@ -189,28 +149,10 @@ static int amba_pm_resume(struct device
 	return ret;
 }
 
-static int amba_pm_resume_noirq(struct device *dev)
-{
-	struct device_driver *drv = dev->driver;
-	int ret = 0;
-
-	if (!drv)
-		return 0;
-
-	if (drv->pm) {
-		if (drv->pm->resume_noirq)
-			ret = drv->pm->resume_noirq(dev);
-	}
-
-	return ret;
-}
-
 #else /* !CONFIG_SUSPEND */
 
 #define amba_pm_suspend		NULL
 #define amba_pm_resume		NULL
-#define amba_pm_suspend_noirq	NULL
-#define amba_pm_resume_noirq	NULL
 
 #endif /* !CONFIG_SUSPEND */
 
@@ -234,22 +176,6 @@ static int amba_pm_freeze(struct device
 	return ret;
 }
 
-static int amba_pm_freeze_noirq(struct device *dev)
-{
-	struct device_driver *drv = dev->driver;
-	int ret = 0;
-
-	if (!drv)
-		return 0;
-
-	if (drv->pm) {
-		if (drv->pm->freeze_noirq)
-			ret = drv->pm->freeze_noirq(dev);
-	}
-
-	return ret;
-}
-
 static int amba_pm_thaw(struct device *dev)
 {
 	struct device_driver *drv = dev->driver;
@@ -268,22 +194,6 @@ static int amba_pm_thaw(struct device *d
 	return ret;
 }
 
-static int amba_pm_thaw_noirq(struct device *dev)
-{
-	struct device_driver *drv = dev->driver;
-	int ret = 0;
-
-	if (!drv)
-		return 0;
-
-	if (drv->pm) {
-		if (drv->pm->thaw_noirq)
-			ret = drv->pm->thaw_noirq(dev);
-	}
-
-	return ret;
-}
-
 static int amba_pm_poweroff(struct device *dev)
 {
 	struct device_driver *drv = dev->driver;
@@ -302,22 +212,6 @@ static int amba_pm_poweroff(struct devic
 	return ret;
 }
 
-static int amba_pm_poweroff_noirq(struct device *dev)
-{
-	struct device_driver *drv = dev->driver;
-	int ret = 0;
-
-	if (!drv)
-		return 0;
-
-	if (drv->pm) {
-		if (drv->pm->poweroff_noirq)
-			ret = drv->pm->poweroff_noirq(dev);
-	}
-
-	return ret;
-}
-
 static int amba_pm_restore(struct device *dev)
 {
 	struct device_driver *drv = dev->driver;
@@ -336,32 +230,12 @@ static int amba_pm_restore(struct device
 	return ret;
 }
 
-static int amba_pm_restore_noirq(struct device *dev)
-{
-	struct device_driver *drv = dev->driver;
-	int ret = 0;
-
-	if (!drv)
-		return 0;
-
-	if (drv->pm) {
-		if (drv->pm->restore_noirq)
-			ret = drv->pm->restore_noirq(dev);
-	}
-
-	return ret;
-}
-
 #else /* !CONFIG_HIBERNATE_CALLBACKS */
 
 #define amba_pm_freeze		NULL
 #define amba_pm_thaw		NULL
 #define amba_pm_poweroff		NULL
 #define amba_pm_restore		NULL
-#define amba_pm_freeze_noirq	NULL
-#define amba_pm_thaw_noirq		NULL
-#define amba_pm_poweroff_noirq	NULL
-#define amba_pm_restore_noirq	NULL
 
 #endif /* !CONFIG_HIBERNATE_CALLBACKS */
 
@@ -402,20 +276,12 @@ static int amba_pm_runtime_resume(struct
 #ifdef CONFIG_PM
 
 static const struct dev_pm_ops amba_pm = {
-	.prepare	= amba_pm_prepare,
-	.complete	= amba_pm_complete,
 	.suspend	= amba_pm_suspend,
 	.resume		= amba_pm_resume,
 	.freeze		= amba_pm_freeze,
 	.thaw		= amba_pm_thaw,
 	.poweroff	= amba_pm_poweroff,
 	.restore	= amba_pm_restore,
-	.suspend_noirq	= amba_pm_suspend_noirq,
-	.resume_noirq	= amba_pm_resume_noirq,
-	.freeze_noirq	= amba_pm_freeze_noirq,
-	.thaw_noirq	= amba_pm_thaw_noirq,
-	.poweroff_noirq	= amba_pm_poweroff_noirq,
-	.restore_noirq	= amba_pm_restore_noirq,
 	SET_RUNTIME_PM_OPS(
 		amba_pm_runtime_suspend,
 		amba_pm_runtime_resume,


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 0/4] PM: Make the PM core execute driver callbacks if subsystem ones are not present
  2011-12-09 23:18 [PATCH 0/4] PM: Make the PM core execute driver callbacks if subsystem ones are not present Rafael J. Wysocki
                   ` (3 preceding siblings ...)
  2011-12-09 23:23 ` [PATCH 4/4] PM / Sleep: Remove forward-only callbacks from AMBA " Rafael J. Wysocki
@ 2011-12-09 23:47 ` Greg KH
  2011-12-09 23:52   ` Rafael J. Wysocki
  2011-12-15 22:34 ` [Update][PATCH 0/5] " Rafael J. Wysocki
  5 siblings, 1 reply; 20+ messages in thread
From: Greg KH @ 2011-12-09 23:47 UTC (permalink / raw)
  To: Rafael J. Wysocki; +Cc: Linux PM list, LKML, Russell King

On Sat, Dec 10, 2011 at 12:18:38AM +0100, Rafael J. Wysocki wrote:
> Hi,
> 
> The following patchset changes the PM core so that driver callbacks
> are executed automatically when subsystem ones are not present and
> uses the new PM core's behavior to simplify the platform and AMBA bus
> types (there probably are many more bus types that may be simplified
> this way, but those two were the easiest targets for me).
> 
> The reasons why to do this are outlined in the changelog of the second patch.
> 
> Comments welcome!

Looks sane to me, nice work.

greg k-h

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 0/4] PM: Make the PM core execute driver callbacks if subsystem ones are not present
  2011-12-09 23:47 ` [PATCH 0/4] PM: Make the PM core execute driver callbacks if subsystem ones are not present Greg KH
@ 2011-12-09 23:52   ` Rafael J. Wysocki
  0 siblings, 0 replies; 20+ messages in thread
From: Rafael J. Wysocki @ 2011-12-09 23:52 UTC (permalink / raw)
  To: Greg KH; +Cc: Linux PM list, LKML, Russell King

On Saturday, December 10, 2011, Greg KH wrote:
> On Sat, Dec 10, 2011 at 12:18:38AM +0100, Rafael J. Wysocki wrote:
> > Hi,
> > 
> > The following patchset changes the PM core so that driver callbacks
> > are executed automatically when subsystem ones are not present and
> > uses the new PM core's behavior to simplify the platform and AMBA bus
> > types (there probably are many more bus types that may be simplified
> > this way, but those two were the easiest targets for me).
> > 
> > The reasons why to do this are outlined in the changelog of the second patch.
> > 
> > Comments welcome!
> 
> Looks sane to me, nice work.

Thanks!

Rafael

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/4] PM: Run the driver callback directly if the subsystem one is not there
  2011-12-09 23:20 ` [PATCH 2/4] PM: Run the driver callback directly if the subsystem one is not there Rafael J. Wysocki
@ 2011-12-10 14:59   ` Alan Stern
  2011-12-10 18:21     ` Rafael J. Wysocki
  0 siblings, 1 reply; 20+ messages in thread
From: Alan Stern @ 2011-12-10 14:59 UTC (permalink / raw)
  To: Rafael J. Wysocki; +Cc: Linux PM list, Greg Kroah-Hartman, LKML, Russell King

On Sat, 10 Dec 2011, Rafael J. Wysocki wrote:

> From: Rafael J. Wysocki <rjw@sisk.pl>
> 
> Make the PM core execute driver PM callbacks directly if the
> corresponding subsystem callbacks are not present.
> --- linux.orig/drivers/base/power/runtime.c
> +++ linux/drivers/base/power/runtime.c
> @@ -250,6 +250,9 @@ static int rpm_idle(struct device *dev,
>  	else
>  		callback = NULL;
>  
> +	if (!callback && dev->driver && dev->driver->pm)
> +		callback = dev->driver->pm->runtime_idle;
> +
>  	if (callback)
>  		__rpm_callback(callback, dev);
>  
> @@ -713,6 +716,12 @@ static int rpm_resume(struct device *dev
>  	else
>  		callback = NULL;
>  
> +	if (!callback && dev->driver && dev->driver->pm)
> +		callback = dev->driver->pm->runtime_suspend;
> +
> +	if (!callback && dev->driver && dev->driver->pm)
> +		callback = dev->driver->pm->runtime_resume;
> +
>  	retval = rpm_callback(callback, dev);
>  	if (retval) {
>  		__update_runtime_status(dev, RPM_SUSPENDED);

Looks like part of that got added in the wrong place.

Alan Stern


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/4] PM: Run the driver callback directly if the subsystem one is not there
  2011-12-10 14:59   ` Alan Stern
@ 2011-12-10 18:21     ` Rafael J. Wysocki
  0 siblings, 0 replies; 20+ messages in thread
From: Rafael J. Wysocki @ 2011-12-10 18:21 UTC (permalink / raw)
  To: Alan Stern; +Cc: Linux PM list, Greg Kroah-Hartman, LKML, Russell King

On Saturday, December 10, 2011, Alan Stern wrote:
> On Sat, 10 Dec 2011, Rafael J. Wysocki wrote:
> 
> > From: Rafael J. Wysocki <rjw@sisk.pl>
> > 
> > Make the PM core execute driver PM callbacks directly if the
> > corresponding subsystem callbacks are not present.
> > --- linux.orig/drivers/base/power/runtime.c
> > +++ linux/drivers/base/power/runtime.c
> > @@ -250,6 +250,9 @@ static int rpm_idle(struct device *dev,
> >  	else
> >  		callback = NULL;
> >  
> > +	if (!callback && dev->driver && dev->driver->pm)
> > +		callback = dev->driver->pm->runtime_idle;
> > +
> >  	if (callback)
> >  		__rpm_callback(callback, dev);
> >  
> > @@ -713,6 +716,12 @@ static int rpm_resume(struct device *dev
> >  	else
> >  		callback = NULL;
> >  
> > +	if (!callback && dev->driver && dev->driver->pm)
> > +		callback = dev->driver->pm->runtime_suspend;
> > +
> > +	if (!callback && dev->driver && dev->driver->pm)
> > +		callback = dev->driver->pm->runtime_resume;
> > +
> >  	retval = rpm_callback(callback, dev);
> >  	if (retval) {
> >  		__update_runtime_status(dev, RPM_SUSPENDED);
> 
> Looks like part of that got added in the wrong place.

Indeed, thanks for spotting this!

Updated patch is appended.

---
From: Rafael J. Wysocki <rjw@sisk.pl>
Subject: PM: Run the driver callback directly if the subsystem one is not there

Make the PM core execute driver PM callbacks directly if the
corresponding subsystem callbacks are not present.

There are three reasons for doing that.  First, it reflects the
behavior of drivers/base/dd.c:really_probe() that runs the driver's
.probe() callback directly if the bus type's one is not defined, so
this change will remove one arbitrary difference between the PM core
and the remaining parts of the driver core.  Second, it will allow
some subsystems, whose PM callbacks don't do anything except for
executing driver callbacks, to be simplified quite a bit by removing
those "forward-only" callbacks.  Finally, it will allow us to remove
one level of indirection in the system suspend and resume code paths
where it is not necessary, which is going to lead to less debug noise
with initcall_debug passed in the kernel command line (messages won't
be printed for driverless devices whose subsystems don't provide
PM callbacks among other things).

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 Documentation/power/devices.txt    |   39 ++++++-----
 Documentation/power/runtime_pm.txt |  128 +++++++++++++++++++------------------
 drivers/base/power/main.c          |  109 ++++++++++++++++++++-----------
 drivers/base/power/runtime.c       |    9 ++
 4 files changed, 170 insertions(+), 115 deletions(-)

Index: linux/drivers/base/power/runtime.c
===================================================================
--- linux.orig/drivers/base/power/runtime.c
+++ linux/drivers/base/power/runtime.c
@@ -250,6 +250,9 @@ static int rpm_idle(struct device *dev,
 	else
 		callback = NULL;
 
+	if (!callback && dev->driver && dev->driver->pm)
+		callback = dev->driver->pm->runtime_idle;
+
 	if (callback)
 		__rpm_callback(callback, dev);
 
@@ -485,6 +488,9 @@ static int rpm_suspend(struct device *de
 	else
 		callback = NULL;
 
+	if (!callback && dev->driver && dev->driver->pm)
+		callback = dev->driver->pm->runtime_suspend;
+
 	retval = rpm_callback(callback, dev);
 	if (retval)
 		goto fail;
@@ -713,6 +719,9 @@ static int rpm_resume(struct device *dev
 	else
 		callback = NULL;
 
+	if (!callback && dev->driver && dev->driver->pm)
+		callback = dev->driver->pm->runtime_resume;
+
 	retval = rpm_callback(callback, dev);
 	if (retval) {
 		__update_runtime_status(dev, RPM_SUSPENDED);
Index: linux/Documentation/power/devices.txt
===================================================================
--- linux.orig/Documentation/power/devices.txt
+++ linux/Documentation/power/devices.txt
@@ -126,7 +126,9 @@ The core methods to suspend and resume d
 pointed to by the ops member of struct dev_pm_domain, or by the pm member of
 struct bus_type, struct device_type and struct class.  They are mostly of
 interest to the people writing infrastructure for platforms and buses, like PCI
-or USB, or device type and device class drivers.
+or USB, or device type and device class drivers.  They also are relevant to the
+writers of device drivers whose subsystems (PM domains, device types, device
+classes and bus types) don't provide all power management methods.
 
 Bus drivers implement these methods as appropriate for the hardware and the
 drivers using it; PCI works differently from USB, and so on.  Not many people
@@ -268,32 +270,35 @@ various phases always run after tasks ha
 unfrozen.  Furthermore, the *_noirq phases run at a time when IRQ handlers have
 been disabled (except for those marked with the IRQF_NO_SUSPEND flag).
 
-All phases use PM domain, bus, type, or class callbacks (that is, methods
-defined in dev->pm_domain->ops, dev->bus->pm, dev->type->pm, or dev->class->pm).
-These callbacks are regarded by the PM core as mutually exclusive.  Moreover,
-PM domain callbacks always take precedence over bus, type and class callbacks,
-while type callbacks take precedence over bus and class callbacks, and class
-callbacks take precedence over bus callbacks.  To be precise, the following
-rules are used to determine which callback to execute in the given phase:
-
-    1.	If dev->pm_domain is present, the PM core will attempt to execute the
-	callback included in dev->pm_domain->ops.  If that callback is not
-	present, no action will be carried out for the given device.
+All phases use PM domain, bus, type, class or driver callbacks (that is, methods
+defined in dev->pm_domain->ops, dev->bus->pm, dev->type->pm, dev->class->pm or
+dev->driver->pm).  These callbacks are regarded by the PM core as mutually
+exclusive.  Moreover, PM domain callbacks always take precedence over all of the
+other callbacks and, for example, type callbacks take precedence over bus, class
+and driver callbacks.  To be precise, the following rules are used to determine
+which callback to execute in the given phase:
+
+    1.	If dev->pm_domain is present, the PM core will choose the callback
+	included in dev->pm_domain->ops for execution
 
     2.	Otherwise, if both dev->type and dev->type->pm are present, the callback
-	included in dev->type->pm will be executed.
+	included in dev->type->pm will be chosen for execution.
 
     3.	Otherwise, if both dev->class and dev->class->pm are present, the
-	callback included in dev->class->pm will be executed.
+	callback included in dev->class->pm will be chosen for execution.
 
     4.	Otherwise, if both dev->bus and dev->bus->pm are present, the callback
-	included in dev->bus->pm will be executed.
+	included in dev->bus->pm will be chosen for execution.
 
 This allows PM domains and device types to override callbacks provided by bus
 types or device classes if necessary.
 
-These callbacks may in turn invoke device- or driver-specific methods stored in
-dev->driver->pm, but they don't have to.
+The PM domain, type, class and bus callbacks may in turn invoke device- or
+driver-specific methods stored in dev->driver->pm, but they don't have to do
+that.
+
+If the subsystem callback chosen for execution is not present, the PM core will
+execute the corresponding method from dev->driver->pm instead if there is one.
 
 
 Entering System Suspend
Index: linux/Documentation/power/runtime_pm.txt
===================================================================
--- linux.orig/Documentation/power/runtime_pm.txt
+++ linux/Documentation/power/runtime_pm.txt
@@ -57,6 +57,10 @@ the following:
 
   4. Bus type of the device, if both dev->bus and dev->bus->pm are present.
 
+If the subsystem chosen by applying the above rules doesn't provide the relevant
+callback, the PM core will invoke the corresponding driver callback stored in
+dev->driver->pm directly (if present).
+
 The PM core always checks which callback to use in the order given above, so the
 priority order of callbacks from high to low is: PM domain, device type, class
 and bus type.  Moreover, the high-priority one will always take precedence over
@@ -64,86 +68,88 @@ a low-priority one.  The PM domain, bus
 are referred to as subsystem-level callbacks in what follows.
 
 By default, the callbacks are always invoked in process context with interrupts
-enabled.  However, subsystems can use the pm_runtime_irq_safe() helper function
-to tell the PM core that their ->runtime_suspend(), ->runtime_resume() and
-->runtime_idle() callbacks may be invoked in atomic context with interrupts
-disabled for a given device.  This implies that the callback routines in
-question must not block or sleep, but it also means that the synchronous helper
-functions listed at the end of Section 4 may be used for that device within an
-interrupt handler or generally in an atomic context.
-
-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
+enabled.  However, the pm_runtime_irq_safe() helper function can be used to tell
+the PM core that it is safe to run the ->runtime_suspend(), ->runtime_resume()
+and ->runtime_idle() callbacks for the given device in atomic context with
+interrupts disabled.  This implies that the callback routines in question must
+not block or sleep, but it also means that the synchronous helper functions
+listed at the end of Section 4 may be used for that device within an interrupt
+handler or generally in an atomic context.
+
+The subsystem-level suspend callback, if present, 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 subsystem-level suspend callback
 knows what to do to handle the device).
 
-  * 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 the subsystem-level resume
-    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 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
-    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).
+  * Once the subsystem-level suspend callback (or the driver suspend callback,
+    if invoked directly) has completed successfully for the given device, the PM
+    core regards the device as suspended, which need not mean that it 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 appropriate resume callback is executed for it.  The runtime
+    PM status of a device after successful execution of the suspend callback is
+    'suspended'.
+
+  * If the suspend callback returns -EBUSY or -EAGAIN, the device's runtime PM
+    status remains 'active', which means that the device _must_ be fully
+    operational afterwards.
+
+  * If the suspend callback returns an error code different from -EBUSY and
+    -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 its status
+    is directly set to  either'active', or 'suspended' (the PM core provides
+    special helper functions for this purpose).
 
-In particular, if the driver requires remote wake-up capability (i.e. hardware
+In particular, if the driver requires remote wakeup 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 runtime 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.
+device_run_wake() returns 'true' for the device and the device is put into a
+low-power state during the execution of the suspend callback, it is expected
+that remote wakeup will be enabled for the device.  Generally, remote wakeup
+should be enabled for all input devices put into low-power states at run time.
+
+The subsystem-level resume callback, if present, 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 (or the driver resume callback, if
+    invoked directly) 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 runtime PM status of the device is then
+    'active'.
+
+  * If the 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 to either 'active', or
+    'suspended' (by means of special helper functions provided by the PM core
+    for this purpose).
+
+The idle callback (a subsystem-level one, if present, or the driver one) 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
-    subsystem-level idle callback with the device as an argument.
+    idle callback with the device as its argument.
 
-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
+The action performed by the idle callback is totally dependent on the subsystem
+(or driver) 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 runtime
-PM callbacks:
+that the following constraints are met with respect to runtime PM callbacks for
+one device:
 
 (1) The callbacks are mutually exclusive (e.g. it is forbidden to execute
     ->runtime_suspend() in parallel with ->runtime_resume() or with another
Index: linux/drivers/base/power/main.c
===================================================================
--- linux.orig/drivers/base/power/main.c
+++ linux/drivers/base/power/main.c
@@ -381,10 +381,15 @@ static int device_resume_noirq(struct de
 		info = "EARLY class ";
 		callback = pm_noirq_op(dev->class->pm, state);
 	} else if (dev->bus && dev->bus->pm) {
-		info = "EARLY ";
+		info = "EARLY bus ";
 		callback = pm_noirq_op(dev->bus->pm, state);
 	}
 
+	if (!callback && dev->driver && dev->driver->pm) {
+		info = "EARLY driver ";
+		callback = pm_noirq_op(dev->driver->pm, state);
+	}
+
 	error = dpm_run_callback(callback, dev, state, info);
 
 	TRACE_RESUME(error);
@@ -462,20 +467,20 @@ static int device_resume(struct device *
 	if (dev->pm_domain) {
 		info = "power domain ";
 		callback = pm_op(&dev->pm_domain->ops, state);
-		goto End;
+		goto Driver;
 	}
 
 	if (dev->type && dev->type->pm) {
 		info = "type ";
 		callback = pm_op(dev->type->pm, state);
-		goto End;
+		goto Driver;
 	}
 
 	if (dev->class) {
 		if (dev->class->pm) {
 			info = "class ";
 			callback = pm_op(dev->class->pm, state);
-			goto End;
+			goto Driver;
 		} else if (dev->class->resume) {
 			info = "legacy class ";
 			callback = dev->class->resume;
@@ -485,14 +490,21 @@ static int device_resume(struct device *
 
 	if (dev->bus) {
 		if (dev->bus->pm) {
-			info = "";
+			info = "bus ";
 			callback = pm_op(dev->bus->pm, state);
 		} else if (dev->bus->resume) {
-			info = "legacy ";
+			info = "legacy bus ";
 			callback = dev->bus->resume;
+			goto End;
 		}
 	}
 
+ Driver:
+	if (!callback && dev->driver && dev->driver->pm) {
+		info = "driver ";
+		callback = pm_op(dev->driver->pm, state);
+	}
+
  End:
 	error = dpm_run_callback(callback, dev, state, info);
 	dev->power.is_suspended = false;
@@ -586,24 +598,33 @@ void dpm_resume(pm_message_t state)
  */
 static void device_complete(struct device *dev, pm_message_t state)
 {
+	void (*callback)(struct device *) = NULL;
+	char *info = NULL;
+
 	device_lock(dev);
 
 	if (dev->pm_domain) {
-		pm_dev_dbg(dev, state, "completing power domain ");
-		if (dev->pm_domain->ops.complete)
-			dev->pm_domain->ops.complete(dev);
+		info = "completing power domain ";
+		callback = dev->pm_domain->ops.complete;
 	} else if (dev->type && dev->type->pm) {
-		pm_dev_dbg(dev, state, "completing type ");
-		if (dev->type->pm->complete)
-			dev->type->pm->complete(dev);
+		info = "completing type ";
+		callback = dev->type->pm->complete;
 	} else if (dev->class && dev->class->pm) {
-		pm_dev_dbg(dev, state, "completing class ");
-		if (dev->class->pm->complete)
-			dev->class->pm->complete(dev);
+		info = "completing class ";
+		callback = dev->class->pm->complete;
 	} else if (dev->bus && dev->bus->pm) {
-		pm_dev_dbg(dev, state, "completing ");
-		if (dev->bus->pm->complete)
-			dev->bus->pm->complete(dev);
+		info = "completing bus ";
+		callback = dev->bus->pm->complete;
+	}
+
+	if (!callback && dev->driver && dev->driver->pm) {
+		info = "completing driver ";
+		callback = dev->driver->pm->complete;
+	}
+
+	if (callback) {
+		pm_dev_dbg(dev, state, info);
+		callback(dev);
 	}
 
 	device_unlock(dev);
@@ -702,10 +723,15 @@ static int device_suspend_noirq(struct d
 		info = "LATE class ";
 		callback = pm_noirq_op(dev->class->pm, state);
 	} else if (dev->bus && dev->bus->pm) {
-		info = "LATE ";
+		info = "LATE bus ";
 		callback = pm_noirq_op(dev->bus->pm, state);
 	}
 
+	if (!callback && dev->driver && dev->driver->pm) {
+		info = "LATE driver ";
+		callback = pm_noirq_op(dev->driver->pm, state);
+	}
+
 	return dpm_run_callback(callback, dev, state, info);
 }
 
@@ -830,16 +856,21 @@ static int __device_suspend(struct devic
 
 	if (dev->bus) {
 		if (dev->bus->pm) {
-			info = "";
+			info = "bus ";
 			callback = pm_op(dev->bus->pm, state);
 		} else if (dev->bus->suspend) {
-			pm_dev_dbg(dev, state, "legacy ");
+			pm_dev_dbg(dev, state, "legacy bus ");
 			error = legacy_suspend(dev, state, dev->bus->suspend);
 			goto End;
 		}
 	}
 
  Run:
+	if (!callback && dev->driver && dev->driver->pm) {
+		info = "driver ";
+		callback = pm_op(dev->driver->pm, state);
+	}
+
 	error = dpm_run_callback(callback, dev, state, info);
 
  End:
@@ -947,6 +978,8 @@ int dpm_suspend(pm_message_t state)
  */
 static int device_prepare(struct device *dev, pm_message_t state)
 {
+	int (*callback)(struct device *) = NULL;
+	char *info = NULL;
 	int error = 0;
 
 	device_lock(dev);
@@ -954,25 +987,27 @@ static int device_prepare(struct device
 	dev->power.wakeup_path = device_may_wakeup(dev);
 
 	if (dev->pm_domain) {
-		pm_dev_dbg(dev, state, "preparing power domain ");
-		if (dev->pm_domain->ops.prepare)
-			error = dev->pm_domain->ops.prepare(dev);
-		suspend_report_result(dev->pm_domain->ops.prepare, error);
+		info = "preparing power domain ";
+		callback = dev->pm_domain->ops.prepare;
 	} else if (dev->type && dev->type->pm) {
-		pm_dev_dbg(dev, state, "preparing type ");
-		if (dev->type->pm->prepare)
-			error = dev->type->pm->prepare(dev);
-		suspend_report_result(dev->type->pm->prepare, error);
+		info = "preparing type ";
+		callback = dev->type->pm->prepare;
 	} else if (dev->class && dev->class->pm) {
-		pm_dev_dbg(dev, state, "preparing class ");
-		if (dev->class->pm->prepare)
-			error = dev->class->pm->prepare(dev);
-		suspend_report_result(dev->class->pm->prepare, error);
+		info = "preparing class ";
+		callback = dev->class->pm->prepare;
 	} else if (dev->bus && dev->bus->pm) {
-		pm_dev_dbg(dev, state, "preparing ");
-		if (dev->bus->pm->prepare)
-			error = dev->bus->pm->prepare(dev);
-		suspend_report_result(dev->bus->pm->prepare, error);
+		info = "preparing bus ";
+		callback = dev->bus->pm->prepare;
+	}
+
+	if (!callback && dev->driver && dev->driver->pm) {
+		info = "preparing driver ";
+		callback = dev->driver->pm->prepare;
+	}
+
+	if (callback) {
+		error = callback(dev);
+		suspend_report_result(callback, error);
 	}
 
 	device_unlock(dev);

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/4] PM / Sleep: Make pm_op() and pm_noirq_op() return callback pointers
  2011-12-09 23:20 ` [PATCH 1/4] PM / Sleep: Make pm_op() and pm_noirq_op() return callback pointers Rafael J. Wysocki
@ 2011-12-12  4:04   ` Namhyung Kim
  2011-12-12 23:53     ` Rafael J. Wysocki
  0 siblings, 1 reply; 20+ messages in thread
From: Namhyung Kim @ 2011-12-12  4:04 UTC (permalink / raw)
  To: linux-kernel

Rafael J. Wysocki <rjw <at> sisk.pl> writes:

> 
> From: Rafael J. Wysocki <rjw <at> sisk.pl>
> 
> Make the pm_op() and pm_noirq_op() functions return pointers to
> appropriate callbacks instead of executing those callbacks and
> returning their results.
> 
> This change is required for a subsequent modification that will
> execute the corresponding driver callback if the subsystem
> callback returned by either pm_op(), or pm_noirq_op() is NULL.
> 

Hello Rafael,

How about typedef'ing something like pm_callback_t for readability?

   typedef int (*pm_callback_t)(struct device *);

This way, the code will be easier to read.
(Dropped inline changes below due to the grumpiness of the GMANE's
web interface)

Thanks.
Namhyung Kim



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/4] PM / Sleep: Make pm_op() and pm_noirq_op() return callback pointers
  2011-12-12  4:04   ` Namhyung Kim
@ 2011-12-12 23:53     ` Rafael J. Wysocki
  2011-12-13 13:55       ` Namhyung Kim
  0 siblings, 1 reply; 20+ messages in thread
From: Rafael J. Wysocki @ 2011-12-12 23:53 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: linux-kernel, Linux PM list, Greg Kroah-Hartman, Russell King,
	Alan Stern

On Monday, December 12, 2011, Namhyung Kim wrote:
> Rafael J. Wysocki <rjw <at> sisk.pl> writes:
> 
> > 
> > From: Rafael J. Wysocki <rjw <at> sisk.pl>
> > 
> > Make the pm_op() and pm_noirq_op() functions return pointers to
> > appropriate callbacks instead of executing those callbacks and
> > returning their results.
> > 
> > This change is required for a subsequent modification that will
> > execute the corresponding driver callback if the subsystem
> > callback returned by either pm_op(), or pm_noirq_op() is NULL.
> > 
> 
> Hello Rafael,
> 
> How about typedef'ing something like pm_callback_t for readability?
> 
>    typedef int (*pm_callback_t)(struct device *);
> 
> This way, the code will be easier to read.

Do you mean something like in the patch below?  It does look a bit simpler.

Thanks,
Rafael

---
From: Rafael J. Wysocki <rjw@sisk.pl>
Subject: PM / Sleep: Make pm_op() and pm_noirq_op() return callback pointers

Make the pm_op() and pm_noirq_op() functions return pointers to
appropriate callbacks instead of executing those callbacks and
returning their results.

This change is required for a subsequent modification that will
execute the corresponding driver callback if the subsystem
callback returned by either pm_op(), or pm_noirq_op() is NULL.

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

Index: linux/drivers/base/power/main.c
===================================================================
--- linux.orig/drivers/base/power/main.c
+++ linux/drivers/base/power/main.c
@@ -32,6 +32,8 @@
 #include "../base.h"
 #include "power.h"
 
+typedef int (*pm_callback_t)(struct device *);
+
 /*
  * The entries in the dpm_list list are in a depth first order, simply
  * because children are guaranteed to be discovered after parents, and
@@ -211,113 +213,70 @@ static void dpm_wait_for_children(struct
        device_for_each_child(dev, &async, dpm_wait_fn);
 }
 
-static int dpm_run_callback(struct device *dev, int (*cb)(struct device *))
-{
-	ktime_t calltime;
-	int error;
-
-	if (!cb)
-		return 0;
-
-	calltime = initcall_debug_start(dev);
-
-	error = cb(dev);
-	suspend_report_result(cb, error);
-
-	initcall_debug_report(dev, calltime, error);
-
-	return error;
-}
-
 /**
- * pm_op - Execute the PM operation appropriate for given PM event.
- * @dev: Device to handle.
+ * pm_op - Return the PM operation appropriate for given PM event.
  * @ops: PM operations to choose from.
  * @state: PM transition of the system being carried out.
  */
-static int pm_op(struct device *dev,
-		 const struct dev_pm_ops *ops,
-		 pm_message_t state)
+static pm_callback_t pm_op(const struct dev_pm_ops *ops, pm_message_t state)
 {
-	int error = 0;
-
 	switch (state.event) {
 #ifdef CONFIG_SUSPEND
 	case PM_EVENT_SUSPEND:
-		error = dpm_run_callback(dev, ops->suspend);
-		break;
+		return ops->suspend;
 	case PM_EVENT_RESUME:
-		error = dpm_run_callback(dev, ops->resume);
-		break;
+		return ops->resume;
 #endif /* CONFIG_SUSPEND */
 #ifdef CONFIG_HIBERNATE_CALLBACKS
 	case PM_EVENT_FREEZE:
 	case PM_EVENT_QUIESCE:
-		error = dpm_run_callback(dev, ops->freeze);
-		break;
+		return ops->freeze;
 	case PM_EVENT_HIBERNATE:
-		error = dpm_run_callback(dev, ops->poweroff);
-		break;
+		return ops->poweroff;
 	case PM_EVENT_THAW:
 	case PM_EVENT_RECOVER:
-		error = dpm_run_callback(dev, ops->thaw);
+		return ops->thaw;
 		break;
 	case PM_EVENT_RESTORE:
-		error = dpm_run_callback(dev, ops->restore);
-		break;
+		return ops->restore;
 #endif /* CONFIG_HIBERNATE_CALLBACKS */
-	default:
-		error = -EINVAL;
 	}
 
-	return error;
+	return NULL;
 }
 
 /**
- * pm_noirq_op - Execute the PM operation appropriate for given PM event.
- * @dev: Device to handle.
+ * pm_noirq_op - Return the PM operation appropriate for given PM event.
  * @ops: PM operations to choose from.
  * @state: PM transition of the system being carried out.
  *
  * The driver of @dev will not receive interrupts while this function is being
  * executed.
  */
-static int pm_noirq_op(struct device *dev,
-			const struct dev_pm_ops *ops,
-			pm_message_t state)
+static pm_callback_t pm_noirq_op(const struct dev_pm_ops *ops, pm_message_t state)
 {
-	int error = 0;
-
 	switch (state.event) {
 #ifdef CONFIG_SUSPEND
 	case PM_EVENT_SUSPEND:
-		error = dpm_run_callback(dev, ops->suspend_noirq);
-		break;
+		return ops->suspend_noirq;
 	case PM_EVENT_RESUME:
-		error = dpm_run_callback(dev, ops->resume_noirq);
-		break;
+		return ops->resume_noirq;
 #endif /* CONFIG_SUSPEND */
 #ifdef CONFIG_HIBERNATE_CALLBACKS
 	case PM_EVENT_FREEZE:
 	case PM_EVENT_QUIESCE:
-		error = dpm_run_callback(dev, ops->freeze_noirq);
-		break;
+		return ops->freeze_noirq;
 	case PM_EVENT_HIBERNATE:
-		error = dpm_run_callback(dev, ops->poweroff_noirq);
-		break;
+		return ops->poweroff_noirq;
 	case PM_EVENT_THAW:
 	case PM_EVENT_RECOVER:
-		error = dpm_run_callback(dev, ops->thaw_noirq);
-		break;
+		return ops->thaw_noirq;
 	case PM_EVENT_RESTORE:
-		error = dpm_run_callback(dev, ops->restore_noirq);
-		break;
+		return ops->restore_noirq;
 #endif /* CONFIG_HIBERNATE_CALLBACKS */
-	default:
-		error = -EINVAL;
 	}
 
-	return error;
+	return NULL;
 }
 
 static char *pm_verb(int event)
@@ -375,6 +334,26 @@ static void dpm_show_time(ktime_t startt
 		usecs / USEC_PER_MSEC, usecs % USEC_PER_MSEC);
 }
 
+static int dpm_run_callback(pm_callback_t cb, struct device *dev,
+			    pm_message_t state, char *info)
+{
+	ktime_t calltime;
+	int error;
+
+	if (!cb)
+		return 0;
+
+	calltime = initcall_debug_start(dev);
+
+	pm_dev_dbg(dev, state, info);
+	error = cb(dev);
+	suspend_report_result(cb, error);
+
+	initcall_debug_report(dev, calltime, error);
+
+	return error;
+}
+
 /*------------------------- Resume routines -------------------------*/
 
 /**
@@ -387,25 +366,29 @@ static void dpm_show_time(ktime_t startt
  */
 static int device_resume_noirq(struct device *dev, pm_message_t state)
 {
+	pm_callback_t callback = NULL;
+	char *info = NULL;
 	int error = 0;
 
 	TRACE_DEVICE(dev);
 	TRACE_RESUME(0);
 
 	if (dev->pm_domain) {
-		pm_dev_dbg(dev, state, "EARLY power domain ");
-		error = pm_noirq_op(dev, &dev->pm_domain->ops, state);
+		info = "EARLY power domain ";
+		callback = pm_noirq_op(&dev->pm_domain->ops, state);
 	} else if (dev->type && dev->type->pm) {
-		pm_dev_dbg(dev, state, "EARLY type ");
-		error = pm_noirq_op(dev, dev->type->pm, state);
+		info = "EARLY type ";
+		callback = pm_noirq_op(dev->type->pm, state);
 	} else if (dev->class && dev->class->pm) {
-		pm_dev_dbg(dev, state, "EARLY class ");
-		error = pm_noirq_op(dev, dev->class->pm, state);
+		info = "EARLY class ";
+		callback = pm_noirq_op(dev->class->pm, state);
 	} else if (dev->bus && dev->bus->pm) {
-		pm_dev_dbg(dev, state, "EARLY ");
-		error = pm_noirq_op(dev, dev->bus->pm, state);
+		info = "EARLY ";
+		callback = pm_noirq_op(dev->bus->pm, state);
 	}
 
+	error = dpm_run_callback(callback, dev, state, info);
+
 	TRACE_RESUME(error);
 	return error;
 }
@@ -455,6 +438,8 @@ EXPORT_SYMBOL_GPL(dpm_resume_noirq);
  */
 static int device_resume(struct device *dev, pm_message_t state, bool async)
 {
+	pm_callback_t callback = NULL;
+	char *info = NULL;
 	int error = 0;
 	bool put = false;
 
@@ -477,40 +462,41 @@ static int device_resume(struct device *
 	put = true;
 
 	if (dev->pm_domain) {
-		pm_dev_dbg(dev, state, "power domain ");
-		error = pm_op(dev, &dev->pm_domain->ops, state);
+		info = "power domain ";
+		callback = pm_op(&dev->pm_domain->ops, state);
 		goto End;
 	}
 
 	if (dev->type && dev->type->pm) {
-		pm_dev_dbg(dev, state, "type ");
-		error = pm_op(dev, dev->type->pm, state);
+		info = "type ";
+		callback = pm_op(dev->type->pm, state);
 		goto End;
 	}
 
 	if (dev->class) {
 		if (dev->class->pm) {
-			pm_dev_dbg(dev, state, "class ");
-			error = pm_op(dev, dev->class->pm, state);
+			info = "class ";
+			callback = pm_op(dev->class->pm, state);
 			goto End;
 		} else if (dev->class->resume) {
-			pm_dev_dbg(dev, state, "legacy class ");
-			error = dpm_run_callback(dev, dev->class->resume);
+			info = "legacy class ";
+			callback = dev->class->resume;
 			goto End;
 		}
 	}
 
 	if (dev->bus) {
 		if (dev->bus->pm) {
-			pm_dev_dbg(dev, state, "");
-			error = pm_op(dev, dev->bus->pm, state);
+			info = "";
+			callback = pm_op(dev->bus->pm, state);
 		} else if (dev->bus->resume) {
-			pm_dev_dbg(dev, state, "legacy ");
-			error = dpm_run_callback(dev, dev->bus->resume);
+			info = "legacy ";
+			callback = dev->bus->resume;
 		}
 	}
 
  End:
+	error = dpm_run_callback(callback, dev, state, info);
 	dev->power.is_suspended = false;
 
  Unlock:
@@ -705,23 +691,24 @@ static pm_message_t resume_event(pm_mess
  */
 static int device_suspend_noirq(struct device *dev, pm_message_t state)
 {
-	int error = 0;
+	pm_callback_t callback = NULL;
+	char *info = NULL;
 
 	if (dev->pm_domain) {
-		pm_dev_dbg(dev, state, "LATE power domain ");
-		error = pm_noirq_op(dev, &dev->pm_domain->ops, state);
+		info = "LATE power domain ";
+		callback = pm_noirq_op(&dev->pm_domain->ops, state);
 	} else if (dev->type && dev->type->pm) {
-		pm_dev_dbg(dev, state, "LATE type ");
-		error = pm_noirq_op(dev, dev->type->pm, state);
+		info = "LATE type ";
+		callback = pm_noirq_op(dev->type->pm, state);
 	} else if (dev->class && dev->class->pm) {
-		pm_dev_dbg(dev, state, "LATE class ");
-		error = pm_noirq_op(dev, dev->class->pm, state);
+		info = "LATE class ";
+		callback = pm_noirq_op(dev->class->pm, state);
 	} else if (dev->bus && dev->bus->pm) {
-		pm_dev_dbg(dev, state, "LATE ");
-		error = pm_noirq_op(dev, dev->bus->pm, state);
+		info = "LATE ";
+		callback = pm_noirq_op(dev->bus->pm, state);
 	}
 
-	return error;
+	return dpm_run_callback(callback, dev, state, info);
 }
 
 /**
@@ -798,6 +785,8 @@ static int legacy_suspend(struct device
  */
 static int __device_suspend(struct device *dev, pm_message_t state, bool async)
 {
+	pm_callback_t callback = NULL;
+	char *info = NULL;
 	int error = 0;
 
 	dpm_wait_for_children(dev, async);
@@ -818,22 +807,22 @@ static int __device_suspend(struct devic
 	device_lock(dev);
 
 	if (dev->pm_domain) {
-		pm_dev_dbg(dev, state, "power domain ");
-		error = pm_op(dev, &dev->pm_domain->ops, state);
-		goto End;
+		info = "power domain ";
+		callback = pm_op(&dev->pm_domain->ops, state);
+		goto Run;
 	}
 
 	if (dev->type && dev->type->pm) {
-		pm_dev_dbg(dev, state, "type ");
-		error = pm_op(dev, dev->type->pm, state);
-		goto End;
+		info = "type ";
+		callback = pm_op(dev->type->pm, state);
+		goto Run;
 	}
 
 	if (dev->class) {
 		if (dev->class->pm) {
-			pm_dev_dbg(dev, state, "class ");
-			error = pm_op(dev, dev->class->pm, state);
-			goto End;
+			info = "class ";
+			callback = pm_op(dev->class->pm, state);
+			goto Run;
 		} else if (dev->class->suspend) {
 			pm_dev_dbg(dev, state, "legacy class ");
 			error = legacy_suspend(dev, state, dev->class->suspend);
@@ -843,14 +832,18 @@ static int __device_suspend(struct devic
 
 	if (dev->bus) {
 		if (dev->bus->pm) {
-			pm_dev_dbg(dev, state, "");
-			error = pm_op(dev, dev->bus->pm, state);
+			info = "";
+			callback = pm_op(dev->bus->pm, state);
 		} else if (dev->bus->suspend) {
 			pm_dev_dbg(dev, state, "legacy ");
 			error = legacy_suspend(dev, state, dev->bus->suspend);
+			goto End;
 		}
 	}
 
+ Run:
+	error = dpm_run_callback(callback, dev, state, info);
+
  End:
 	if (!error) {
 		dev->power.is_suspended = true;

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/4] PM / Sleep: Make pm_op() and pm_noirq_op() return callback pointers
  2011-12-12 23:53     ` Rafael J. Wysocki
@ 2011-12-13 13:55       ` Namhyung Kim
  0 siblings, 0 replies; 20+ messages in thread
From: Namhyung Kim @ 2011-12-13 13:55 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: linux-kernel, Linux PM list, Greg Kroah-Hartman, Russell King,
	Alan Stern

2011-12-13 (화), 00:53 +0100, Rafael J. Wysocki:
> On Monday, December 12, 2011, Namhyung Kim wrote:
> > Rafael J. Wysocki <rjw <at> sisk.pl> writes:
> > 
> > > 
> > > From: Rafael J. Wysocki <rjw <at> sisk.pl>
> > > 
> > > Make the pm_op() and pm_noirq_op() functions return pointers to
> > > appropriate callbacks instead of executing those callbacks and
> > > returning their results.
> > > 
> > > This change is required for a subsequent modification that will
> > > execute the corresponding driver callback if the subsystem
> > > callback returned by either pm_op(), or pm_noirq_op() is NULL.
> > > 
> > 
> > Hello Rafael,
> > 
> > How about typedef'ing something like pm_callback_t for readability?
> > 
> >    typedef int (*pm_callback_t)(struct device *);
> > 
> > This way, the code will be easier to read.
> 
> Do you mean something like in the patch below?  It does look a bit simpler.
> 

Yes, indeed. The patch looks better to me. :)

Thanks,
Namhyung Kim




^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Update][PATCH 0/5] PM: Make the PM core execute driver callbacks if subsystem ones are not present
  2011-12-09 23:18 [PATCH 0/4] PM: Make the PM core execute driver callbacks if subsystem ones are not present Rafael J. Wysocki
                   ` (4 preceding siblings ...)
  2011-12-09 23:47 ` [PATCH 0/4] PM: Make the PM core execute driver callbacks if subsystem ones are not present Greg KH
@ 2011-12-15 22:34 ` Rafael J. Wysocki
  2011-12-15 22:35   ` [Update][PATCH 1/5] PM / Sleep: Make pm_op() and pm_noirq_op() return callback pointers Rafael J. Wysocki
                     ` (5 more replies)
  5 siblings, 6 replies; 20+ messages in thread
From: Rafael J. Wysocki @ 2011-12-15 22:34 UTC (permalink / raw)
  To: Linux PM list
  Cc: Greg Kroah-Hartman, LKML, Russell King, Namhyung Kim, Alan Stern

Hi,

I have one more patch for this series and a couple of fixes.

On Saturday, December 10, 2011, Rafael J. Wysocki wrote:
> 
> The following patchset changes the PM core so that driver callbacks
> are executed automatically when subsystem ones are not present and
> uses the new PM core's behavior to simplify the platform and AMBA bus
> types (there probably are many more bus types that may be simplified
> this way, but those two were the easiest targets for me).
> 
> The reasons why to do this are outlined in the changelog of the second patch.

[1/5] - Make pm_op and pm_noirq_op return callbacks
[2/5] - Make PM core call driver callbacks directly if subsystem ones are not present
[3/5] - Remove unnecessary bus type PM callback from platform bus type
[4/5] - Remove unnecessary bus type PM callback from AMBA bus type
[5/5] - Drop generic_subsys_pm_ops

If there are no objections, I'd like to take these patches for 3.3.

Thanks,
Rafael

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Update][PATCH 1/5] PM / Sleep: Make pm_op() and pm_noirq_op() return callback pointers
  2011-12-15 22:34 ` [Update][PATCH 0/5] " Rafael J. Wysocki
@ 2011-12-15 22:35   ` Rafael J. Wysocki
  2011-12-15 22:36   ` [Update][PATCH 2/5] PM: Run the driver callback directly if the subsystem one is not there Rafael J. Wysocki
                     ` (4 subsequent siblings)
  5 siblings, 0 replies; 20+ messages in thread
From: Rafael J. Wysocki @ 2011-12-15 22:35 UTC (permalink / raw)
  To: Linux PM list
  Cc: Greg Kroah-Hartman, LKML, Russell King, Namhyung Kim, Alan Stern

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

Make the pm_op() and pm_noirq_op() functions return pointers to
appropriate callbacks instead of executing those callbacks and
returning their results.

This change is required for a subsequent modification that will
execute the corresponding driver callback if the subsystem
callback returned by either pm_op(), or pm_noirq_op() is NULL.

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

Index: linux/drivers/base/power/main.c
===================================================================
--- linux.orig/drivers/base/power/main.c
+++ linux/drivers/base/power/main.c
@@ -32,6 +32,8 @@
 #include "../base.h"
 #include "power.h"
 
+typedef int (*pm_callback_t)(struct device *);
+
 /*
  * The entries in the dpm_list list are in a depth first order, simply
  * because children are guaranteed to be discovered after parents, and
@@ -211,113 +213,70 @@ static void dpm_wait_for_children(struct
        device_for_each_child(dev, &async, dpm_wait_fn);
 }
 
-static int dpm_run_callback(struct device *dev, int (*cb)(struct device *))
-{
-	ktime_t calltime;
-	int error;
-
-	if (!cb)
-		return 0;
-
-	calltime = initcall_debug_start(dev);
-
-	error = cb(dev);
-	suspend_report_result(cb, error);
-
-	initcall_debug_report(dev, calltime, error);
-
-	return error;
-}
-
 /**
- * pm_op - Execute the PM operation appropriate for given PM event.
- * @dev: Device to handle.
+ * pm_op - Return the PM operation appropriate for given PM event.
  * @ops: PM operations to choose from.
  * @state: PM transition of the system being carried out.
  */
-static int pm_op(struct device *dev,
-		 const struct dev_pm_ops *ops,
-		 pm_message_t state)
+static pm_callback_t pm_op(const struct dev_pm_ops *ops, pm_message_t state)
 {
-	int error = 0;
-
 	switch (state.event) {
 #ifdef CONFIG_SUSPEND
 	case PM_EVENT_SUSPEND:
-		error = dpm_run_callback(dev, ops->suspend);
-		break;
+		return ops->suspend;
 	case PM_EVENT_RESUME:
-		error = dpm_run_callback(dev, ops->resume);
-		break;
+		return ops->resume;
 #endif /* CONFIG_SUSPEND */
 #ifdef CONFIG_HIBERNATE_CALLBACKS
 	case PM_EVENT_FREEZE:
 	case PM_EVENT_QUIESCE:
-		error = dpm_run_callback(dev, ops->freeze);
-		break;
+		return ops->freeze;
 	case PM_EVENT_HIBERNATE:
-		error = dpm_run_callback(dev, ops->poweroff);
-		break;
+		return ops->poweroff;
 	case PM_EVENT_THAW:
 	case PM_EVENT_RECOVER:
-		error = dpm_run_callback(dev, ops->thaw);
+		return ops->thaw;
 		break;
 	case PM_EVENT_RESTORE:
-		error = dpm_run_callback(dev, ops->restore);
-		break;
+		return ops->restore;
 #endif /* CONFIG_HIBERNATE_CALLBACKS */
-	default:
-		error = -EINVAL;
 	}
 
-	return error;
+	return NULL;
 }
 
 /**
- * pm_noirq_op - Execute the PM operation appropriate for given PM event.
- * @dev: Device to handle.
+ * pm_noirq_op - Return the PM operation appropriate for given PM event.
  * @ops: PM operations to choose from.
  * @state: PM transition of the system being carried out.
  *
  * The driver of @dev will not receive interrupts while this function is being
  * executed.
  */
-static int pm_noirq_op(struct device *dev,
-			const struct dev_pm_ops *ops,
-			pm_message_t state)
+static pm_callback_t pm_noirq_op(const struct dev_pm_ops *ops, pm_message_t state)
 {
-	int error = 0;
-
 	switch (state.event) {
 #ifdef CONFIG_SUSPEND
 	case PM_EVENT_SUSPEND:
-		error = dpm_run_callback(dev, ops->suspend_noirq);
-		break;
+		return ops->suspend_noirq;
 	case PM_EVENT_RESUME:
-		error = dpm_run_callback(dev, ops->resume_noirq);
-		break;
+		return ops->resume_noirq;
 #endif /* CONFIG_SUSPEND */
 #ifdef CONFIG_HIBERNATE_CALLBACKS
 	case PM_EVENT_FREEZE:
 	case PM_EVENT_QUIESCE:
-		error = dpm_run_callback(dev, ops->freeze_noirq);
-		break;
+		return ops->freeze_noirq;
 	case PM_EVENT_HIBERNATE:
-		error = dpm_run_callback(dev, ops->poweroff_noirq);
-		break;
+		return ops->poweroff_noirq;
 	case PM_EVENT_THAW:
 	case PM_EVENT_RECOVER:
-		error = dpm_run_callback(dev, ops->thaw_noirq);
-		break;
+		return ops->thaw_noirq;
 	case PM_EVENT_RESTORE:
-		error = dpm_run_callback(dev, ops->restore_noirq);
-		break;
+		return ops->restore_noirq;
 #endif /* CONFIG_HIBERNATE_CALLBACKS */
-	default:
-		error = -EINVAL;
 	}
 
-	return error;
+	return NULL;
 }
 
 static char *pm_verb(int event)
@@ -375,6 +334,26 @@ static void dpm_show_time(ktime_t startt
 		usecs / USEC_PER_MSEC, usecs % USEC_PER_MSEC);
 }
 
+static int dpm_run_callback(pm_callback_t cb, struct device *dev,
+			    pm_message_t state, char *info)
+{
+	ktime_t calltime;
+	int error;
+
+	if (!cb)
+		return 0;
+
+	calltime = initcall_debug_start(dev);
+
+	pm_dev_dbg(dev, state, info);
+	error = cb(dev);
+	suspend_report_result(cb, error);
+
+	initcall_debug_report(dev, calltime, error);
+
+	return error;
+}
+
 /*------------------------- Resume routines -------------------------*/
 
 /**
@@ -387,25 +366,29 @@ static void dpm_show_time(ktime_t startt
  */
 static int device_resume_noirq(struct device *dev, pm_message_t state)
 {
+	pm_callback_t callback = NULL;
+	char *info = NULL;
 	int error = 0;
 
 	TRACE_DEVICE(dev);
 	TRACE_RESUME(0);
 
 	if (dev->pm_domain) {
-		pm_dev_dbg(dev, state, "EARLY power domain ");
-		error = pm_noirq_op(dev, &dev->pm_domain->ops, state);
+		info = "EARLY power domain ";
+		callback = pm_noirq_op(&dev->pm_domain->ops, state);
 	} else if (dev->type && dev->type->pm) {
-		pm_dev_dbg(dev, state, "EARLY type ");
-		error = pm_noirq_op(dev, dev->type->pm, state);
+		info = "EARLY type ";
+		callback = pm_noirq_op(dev->type->pm, state);
 	} else if (dev->class && dev->class->pm) {
-		pm_dev_dbg(dev, state, "EARLY class ");
-		error = pm_noirq_op(dev, dev->class->pm, state);
+		info = "EARLY class ";
+		callback = pm_noirq_op(dev->class->pm, state);
 	} else if (dev->bus && dev->bus->pm) {
-		pm_dev_dbg(dev, state, "EARLY ");
-		error = pm_noirq_op(dev, dev->bus->pm, state);
+		info = "EARLY ";
+		callback = pm_noirq_op(dev->bus->pm, state);
 	}
 
+	error = dpm_run_callback(callback, dev, state, info);
+
 	TRACE_RESUME(error);
 	return error;
 }
@@ -455,6 +438,8 @@ EXPORT_SYMBOL_GPL(dpm_resume_noirq);
  */
 static int device_resume(struct device *dev, pm_message_t state, bool async)
 {
+	pm_callback_t callback = NULL;
+	char *info = NULL;
 	int error = 0;
 	bool put = false;
 
@@ -477,40 +462,41 @@ static int device_resume(struct device *
 	put = true;
 
 	if (dev->pm_domain) {
-		pm_dev_dbg(dev, state, "power domain ");
-		error = pm_op(dev, &dev->pm_domain->ops, state);
+		info = "power domain ";
+		callback = pm_op(&dev->pm_domain->ops, state);
 		goto End;
 	}
 
 	if (dev->type && dev->type->pm) {
-		pm_dev_dbg(dev, state, "type ");
-		error = pm_op(dev, dev->type->pm, state);
+		info = "type ";
+		callback = pm_op(dev->type->pm, state);
 		goto End;
 	}
 
 	if (dev->class) {
 		if (dev->class->pm) {
-			pm_dev_dbg(dev, state, "class ");
-			error = pm_op(dev, dev->class->pm, state);
+			info = "class ";
+			callback = pm_op(dev->class->pm, state);
 			goto End;
 		} else if (dev->class->resume) {
-			pm_dev_dbg(dev, state, "legacy class ");
-			error = dpm_run_callback(dev, dev->class->resume);
+			info = "legacy class ";
+			callback = dev->class->resume;
 			goto End;
 		}
 	}
 
 	if (dev->bus) {
 		if (dev->bus->pm) {
-			pm_dev_dbg(dev, state, "");
-			error = pm_op(dev, dev->bus->pm, state);
+			info = "";
+			callback = pm_op(dev->bus->pm, state);
 		} else if (dev->bus->resume) {
-			pm_dev_dbg(dev, state, "legacy ");
-			error = dpm_run_callback(dev, dev->bus->resume);
+			info = "legacy ";
+			callback = dev->bus->resume;
 		}
 	}
 
  End:
+	error = dpm_run_callback(callback, dev, state, info);
 	dev->power.is_suspended = false;
 
  Unlock:
@@ -705,23 +691,24 @@ static pm_message_t resume_event(pm_mess
  */
 static int device_suspend_noirq(struct device *dev, pm_message_t state)
 {
-	int error = 0;
+	pm_callback_t callback = NULL;
+	char *info = NULL;
 
 	if (dev->pm_domain) {
-		pm_dev_dbg(dev, state, "LATE power domain ");
-		error = pm_noirq_op(dev, &dev->pm_domain->ops, state);
+		info = "LATE power domain ";
+		callback = pm_noirq_op(&dev->pm_domain->ops, state);
 	} else if (dev->type && dev->type->pm) {
-		pm_dev_dbg(dev, state, "LATE type ");
-		error = pm_noirq_op(dev, dev->type->pm, state);
+		info = "LATE type ";
+		callback = pm_noirq_op(dev->type->pm, state);
 	} else if (dev->class && dev->class->pm) {
-		pm_dev_dbg(dev, state, "LATE class ");
-		error = pm_noirq_op(dev, dev->class->pm, state);
+		info = "LATE class ";
+		callback = pm_noirq_op(dev->class->pm, state);
 	} else if (dev->bus && dev->bus->pm) {
-		pm_dev_dbg(dev, state, "LATE ");
-		error = pm_noirq_op(dev, dev->bus->pm, state);
+		info = "LATE ";
+		callback = pm_noirq_op(dev->bus->pm, state);
 	}
 
-	return error;
+	return dpm_run_callback(callback, dev, state, info);
 }
 
 /**
@@ -798,6 +785,8 @@ static int legacy_suspend(struct device
  */
 static int __device_suspend(struct device *dev, pm_message_t state, bool async)
 {
+	pm_callback_t callback = NULL;
+	char *info = NULL;
 	int error = 0;
 
 	dpm_wait_for_children(dev, async);
@@ -818,22 +807,22 @@ static int __device_suspend(struct devic
 	device_lock(dev);
 
 	if (dev->pm_domain) {
-		pm_dev_dbg(dev, state, "power domain ");
-		error = pm_op(dev, &dev->pm_domain->ops, state);
-		goto End;
+		info = "power domain ";
+		callback = pm_op(&dev->pm_domain->ops, state);
+		goto Run;
 	}
 
 	if (dev->type && dev->type->pm) {
-		pm_dev_dbg(dev, state, "type ");
-		error = pm_op(dev, dev->type->pm, state);
-		goto End;
+		info = "type ";
+		callback = pm_op(dev->type->pm, state);
+		goto Run;
 	}
 
 	if (dev->class) {
 		if (dev->class->pm) {
-			pm_dev_dbg(dev, state, "class ");
-			error = pm_op(dev, dev->class->pm, state);
-			goto End;
+			info = "class ";
+			callback = pm_op(dev->class->pm, state);
+			goto Run;
 		} else if (dev->class->suspend) {
 			pm_dev_dbg(dev, state, "legacy class ");
 			error = legacy_suspend(dev, state, dev->class->suspend);
@@ -843,14 +832,18 @@ static int __device_suspend(struct devic
 
 	if (dev->bus) {
 		if (dev->bus->pm) {
-			pm_dev_dbg(dev, state, "");
-			error = pm_op(dev, dev->bus->pm, state);
+			info = "";
+			callback = pm_op(dev->bus->pm, state);
 		} else if (dev->bus->suspend) {
 			pm_dev_dbg(dev, state, "legacy ");
 			error = legacy_suspend(dev, state, dev->bus->suspend);
+			goto End;
 		}
 	}
 
+ Run:
+	error = dpm_run_callback(callback, dev, state, info);
+
  End:
 	if (!error) {
 		dev->power.is_suspended = true;


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Update][PATCH 2/5] PM: Run the driver callback directly if the subsystem one is not there
  2011-12-15 22:34 ` [Update][PATCH 0/5] " Rafael J. Wysocki
  2011-12-15 22:35   ` [Update][PATCH 1/5] PM / Sleep: Make pm_op() and pm_noirq_op() return callback pointers Rafael J. Wysocki
@ 2011-12-15 22:36   ` Rafael J. Wysocki
  2011-12-15 22:37   ` [Update][PATCH 3/5] PM / Sleep: Remove forward-only callbacks from platform bus type Rafael J. Wysocki
                     ` (3 subsequent siblings)
  5 siblings, 0 replies; 20+ messages in thread
From: Rafael J. Wysocki @ 2011-12-15 22:36 UTC (permalink / raw)
  To: Linux PM list
  Cc: Greg Kroah-Hartman, LKML, Russell King, Namhyung Kim, Alan Stern

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

Make the PM core execute driver PM callbacks directly if the
corresponding subsystem callbacks are not present.

There are three reasons for doing that.  First, it reflects the
behavior of drivers/base/dd.c:really_probe() that runs the driver's
.probe() callback directly if the bus type's one is not defined, so
this change will remove one arbitrary difference between the PM core
and the remaining parts of the driver core.  Second, it will allow
some subsystems, whose PM callbacks don't do anything except for
executing driver callbacks, to be simplified quite a bit by removing
those "forward-only" callbacks.  Finally, it will allow us to remove
one level of indirection in the system suspend and resume code paths
where it is not necessary, which is going to lead to less debug noise
with initcall_debug passed in the kernel command line (messages won't
be printed for driverless devices whose subsystems don't provide
PM callbacks among other things).

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 Documentation/power/devices.txt    |   39 ++++++-----
 Documentation/power/runtime_pm.txt |  128 +++++++++++++++++++------------------
 drivers/base/power/main.c          |  109 ++++++++++++++++++++-----------
 drivers/base/power/runtime.c       |    9 ++
 4 files changed, 170 insertions(+), 115 deletions(-)

Index: linux/drivers/base/power/runtime.c
===================================================================
--- linux.orig/drivers/base/power/runtime.c
+++ linux/drivers/base/power/runtime.c
@@ -250,6 +250,9 @@ static int rpm_idle(struct device *dev,
 	else
 		callback = NULL;
 
+	if (!callback && dev->driver && dev->driver->pm)
+		callback = dev->driver->pm->runtime_idle;
+
 	if (callback)
 		__rpm_callback(callback, dev);
 
@@ -485,6 +488,9 @@ static int rpm_suspend(struct device *de
 	else
 		callback = NULL;
 
+	if (!callback && dev->driver && dev->driver->pm)
+		callback = dev->driver->pm->runtime_suspend;
+
 	retval = rpm_callback(callback, dev);
 	if (retval)
 		goto fail;
@@ -713,6 +719,9 @@ static int rpm_resume(struct device *dev
 	else
 		callback = NULL;
 
+	if (!callback && dev->driver && dev->driver->pm)
+		callback = dev->driver->pm->runtime_resume;
+
 	retval = rpm_callback(callback, dev);
 	if (retval) {
 		__update_runtime_status(dev, RPM_SUSPENDED);
Index: linux/Documentation/power/devices.txt
===================================================================
--- linux.orig/Documentation/power/devices.txt
+++ linux/Documentation/power/devices.txt
@@ -126,7 +126,9 @@ The core methods to suspend and resume d
 pointed to by the ops member of struct dev_pm_domain, or by the pm member of
 struct bus_type, struct device_type and struct class.  They are mostly of
 interest to the people writing infrastructure for platforms and buses, like PCI
-or USB, or device type and device class drivers.
+or USB, or device type and device class drivers.  They also are relevant to the
+writers of device drivers whose subsystems (PM domains, device types, device
+classes and bus types) don't provide all power management methods.
 
 Bus drivers implement these methods as appropriate for the hardware and the
 drivers using it; PCI works differently from USB, and so on.  Not many people
@@ -268,32 +270,35 @@ various phases always run after tasks ha
 unfrozen.  Furthermore, the *_noirq phases run at a time when IRQ handlers have
 been disabled (except for those marked with the IRQF_NO_SUSPEND flag).
 
-All phases use PM domain, bus, type, or class callbacks (that is, methods
-defined in dev->pm_domain->ops, dev->bus->pm, dev->type->pm, or dev->class->pm).
-These callbacks are regarded by the PM core as mutually exclusive.  Moreover,
-PM domain callbacks always take precedence over bus, type and class callbacks,
-while type callbacks take precedence over bus and class callbacks, and class
-callbacks take precedence over bus callbacks.  To be precise, the following
-rules are used to determine which callback to execute in the given phase:
-
-    1.	If dev->pm_domain is present, the PM core will attempt to execute the
-	callback included in dev->pm_domain->ops.  If that callback is not
-	present, no action will be carried out for the given device.
+All phases use PM domain, bus, type, class or driver callbacks (that is, methods
+defined in dev->pm_domain->ops, dev->bus->pm, dev->type->pm, dev->class->pm or
+dev->driver->pm).  These callbacks are regarded by the PM core as mutually
+exclusive.  Moreover, PM domain callbacks always take precedence over all of the
+other callbacks and, for example, type callbacks take precedence over bus, class
+and driver callbacks.  To be precise, the following rules are used to determine
+which callback to execute in the given phase:
+
+    1.	If dev->pm_domain is present, the PM core will choose the callback
+	included in dev->pm_domain->ops for execution
 
     2.	Otherwise, if both dev->type and dev->type->pm are present, the callback
-	included in dev->type->pm will be executed.
+	included in dev->type->pm will be chosen for execution.
 
     3.	Otherwise, if both dev->class and dev->class->pm are present, the
-	callback included in dev->class->pm will be executed.
+	callback included in dev->class->pm will be chosen for execution.
 
     4.	Otherwise, if both dev->bus and dev->bus->pm are present, the callback
-	included in dev->bus->pm will be executed.
+	included in dev->bus->pm will be chosen for execution.
 
 This allows PM domains and device types to override callbacks provided by bus
 types or device classes if necessary.
 
-These callbacks may in turn invoke device- or driver-specific methods stored in
-dev->driver->pm, but they don't have to.
+The PM domain, type, class and bus callbacks may in turn invoke device- or
+driver-specific methods stored in dev->driver->pm, but they don't have to do
+that.
+
+If the subsystem callback chosen for execution is not present, the PM core will
+execute the corresponding method from dev->driver->pm instead if there is one.
 
 
 Entering System Suspend
Index: linux/Documentation/power/runtime_pm.txt
===================================================================
--- linux.orig/Documentation/power/runtime_pm.txt
+++ linux/Documentation/power/runtime_pm.txt
@@ -57,6 +57,10 @@ the following:
 
   4. Bus type of the device, if both dev->bus and dev->bus->pm are present.
 
+If the subsystem chosen by applying the above rules doesn't provide the relevant
+callback, the PM core will invoke the corresponding driver callback stored in
+dev->driver->pm directly (if present).
+
 The PM core always checks which callback to use in the order given above, so the
 priority order of callbacks from high to low is: PM domain, device type, class
 and bus type.  Moreover, the high-priority one will always take precedence over
@@ -64,86 +68,88 @@ a low-priority one.  The PM domain, bus
 are referred to as subsystem-level callbacks in what follows.
 
 By default, the callbacks are always invoked in process context with interrupts
-enabled.  However, subsystems can use the pm_runtime_irq_safe() helper function
-to tell the PM core that their ->runtime_suspend(), ->runtime_resume() and
-->runtime_idle() callbacks may be invoked in atomic context with interrupts
-disabled for a given device.  This implies that the callback routines in
-question must not block or sleep, but it also means that the synchronous helper
-functions listed at the end of Section 4 may be used for that device within an
-interrupt handler or generally in an atomic context.
-
-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
+enabled.  However, the pm_runtime_irq_safe() helper function can be used to tell
+the PM core that it is safe to run the ->runtime_suspend(), ->runtime_resume()
+and ->runtime_idle() callbacks for the given device in atomic context with
+interrupts disabled.  This implies that the callback routines in question must
+not block or sleep, but it also means that the synchronous helper functions
+listed at the end of Section 4 may be used for that device within an interrupt
+handler or generally in an atomic context.
+
+The subsystem-level suspend callback, if present, 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 subsystem-level suspend callback
 knows what to do to handle the device).
 
-  * 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 the subsystem-level resume
-    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 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
-    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).
+  * Once the subsystem-level suspend callback (or the driver suspend callback,
+    if invoked directly) has completed successfully for the given device, the PM
+    core regards the device as suspended, which need not mean that it 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 appropriate resume callback is executed for it.  The runtime
+    PM status of a device after successful execution of the suspend callback is
+    'suspended'.
+
+  * If the suspend callback returns -EBUSY or -EAGAIN, the device's runtime PM
+    status remains 'active', which means that the device _must_ be fully
+    operational afterwards.
+
+  * If the suspend callback returns an error code different from -EBUSY and
+    -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 its status
+    is directly set to  either'active', or 'suspended' (the PM core provides
+    special helper functions for this purpose).
 
-In particular, if the driver requires remote wake-up capability (i.e. hardware
+In particular, if the driver requires remote wakeup 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 runtime 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.
+device_run_wake() returns 'true' for the device and the device is put into a
+low-power state during the execution of the suspend callback, it is expected
+that remote wakeup will be enabled for the device.  Generally, remote wakeup
+should be enabled for all input devices put into low-power states at run time.
+
+The subsystem-level resume callback, if present, 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 (or the driver resume callback, if
+    invoked directly) 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 runtime PM status of the device is then
+    'active'.
+
+  * If the 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 to either 'active', or
+    'suspended' (by means of special helper functions provided by the PM core
+    for this purpose).
+
+The idle callback (a subsystem-level one, if present, or the driver one) 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
-    subsystem-level idle callback with the device as an argument.
+    idle callback with the device as its argument.
 
-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
+The action performed by the idle callback is totally dependent on the subsystem
+(or driver) 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 runtime
-PM callbacks:
+that the following constraints are met with respect to runtime PM callbacks for
+one device:
 
 (1) The callbacks are mutually exclusive (e.g. it is forbidden to execute
     ->runtime_suspend() in parallel with ->runtime_resume() or with another
Index: linux/drivers/base/power/main.c
===================================================================
--- linux.orig/drivers/base/power/main.c
+++ linux/drivers/base/power/main.c
@@ -383,10 +383,15 @@ static int device_resume_noirq(struct de
 		info = "EARLY class ";
 		callback = pm_noirq_op(dev->class->pm, state);
 	} else if (dev->bus && dev->bus->pm) {
-		info = "EARLY ";
+		info = "EARLY bus ";
 		callback = pm_noirq_op(dev->bus->pm, state);
 	}
 
+	if (!callback && dev->driver && dev->driver->pm) {
+		info = "EARLY driver ";
+		callback = pm_noirq_op(dev->driver->pm, state);
+	}
+
 	error = dpm_run_callback(callback, dev, state, info);
 
 	TRACE_RESUME(error);
@@ -464,20 +469,20 @@ static int device_resume(struct device *
 	if (dev->pm_domain) {
 		info = "power domain ";
 		callback = pm_op(&dev->pm_domain->ops, state);
-		goto End;
+		goto Driver;
 	}
 
 	if (dev->type && dev->type->pm) {
 		info = "type ";
 		callback = pm_op(dev->type->pm, state);
-		goto End;
+		goto Driver;
 	}
 
 	if (dev->class) {
 		if (dev->class->pm) {
 			info = "class ";
 			callback = pm_op(dev->class->pm, state);
-			goto End;
+			goto Driver;
 		} else if (dev->class->resume) {
 			info = "legacy class ";
 			callback = dev->class->resume;
@@ -487,14 +492,21 @@ static int device_resume(struct device *
 
 	if (dev->bus) {
 		if (dev->bus->pm) {
-			info = "";
+			info = "bus ";
 			callback = pm_op(dev->bus->pm, state);
 		} else if (dev->bus->resume) {
-			info = "legacy ";
+			info = "legacy bus ";
 			callback = dev->bus->resume;
+			goto End;
 		}
 	}
 
+ Driver:
+	if (!callback && dev->driver && dev->driver->pm) {
+		info = "driver ";
+		callback = pm_op(dev->driver->pm, state);
+	}
+
  End:
 	error = dpm_run_callback(callback, dev, state, info);
 	dev->power.is_suspended = false;
@@ -588,24 +600,33 @@ void dpm_resume(pm_message_t state)
  */
 static void device_complete(struct device *dev, pm_message_t state)
 {
+	void (*callback)(struct device *) = NULL;
+	char *info = NULL;
+
 	device_lock(dev);
 
 	if (dev->pm_domain) {
-		pm_dev_dbg(dev, state, "completing power domain ");
-		if (dev->pm_domain->ops.complete)
-			dev->pm_domain->ops.complete(dev);
+		info = "completing power domain ";
+		callback = dev->pm_domain->ops.complete;
 	} else if (dev->type && dev->type->pm) {
-		pm_dev_dbg(dev, state, "completing type ");
-		if (dev->type->pm->complete)
-			dev->type->pm->complete(dev);
+		info = "completing type ";
+		callback = dev->type->pm->complete;
 	} else if (dev->class && dev->class->pm) {
-		pm_dev_dbg(dev, state, "completing class ");
-		if (dev->class->pm->complete)
-			dev->class->pm->complete(dev);
+		info = "completing class ";
+		callback = dev->class->pm->complete;
 	} else if (dev->bus && dev->bus->pm) {
-		pm_dev_dbg(dev, state, "completing ");
-		if (dev->bus->pm->complete)
-			dev->bus->pm->complete(dev);
+		info = "completing bus ";
+		callback = dev->bus->pm->complete;
+	}
+
+	if (!callback && dev->driver && dev->driver->pm) {
+		info = "completing driver ";
+		callback = dev->driver->pm->complete;
+	}
+
+	if (callback) {
+		pm_dev_dbg(dev, state, info);
+		callback(dev);
 	}
 
 	device_unlock(dev);
@@ -704,10 +725,15 @@ static int device_suspend_noirq(struct d
 		info = "LATE class ";
 		callback = pm_noirq_op(dev->class->pm, state);
 	} else if (dev->bus && dev->bus->pm) {
-		info = "LATE ";
+		info = "LATE bus ";
 		callback = pm_noirq_op(dev->bus->pm, state);
 	}
 
+	if (!callback && dev->driver && dev->driver->pm) {
+		info = "LATE driver ";
+		callback = pm_noirq_op(dev->driver->pm, state);
+	}
+
 	return dpm_run_callback(callback, dev, state, info);
 }
 
@@ -832,16 +858,21 @@ static int __device_suspend(struct devic
 
 	if (dev->bus) {
 		if (dev->bus->pm) {
-			info = "";
+			info = "bus ";
 			callback = pm_op(dev->bus->pm, state);
 		} else if (dev->bus->suspend) {
-			pm_dev_dbg(dev, state, "legacy ");
+			pm_dev_dbg(dev, state, "legacy bus ");
 			error = legacy_suspend(dev, state, dev->bus->suspend);
 			goto End;
 		}
 	}
 
  Run:
+	if (!callback && dev->driver && dev->driver->pm) {
+		info = "driver ";
+		callback = pm_op(dev->driver->pm, state);
+	}
+
 	error = dpm_run_callback(callback, dev, state, info);
 
  End:
@@ -949,6 +980,8 @@ int dpm_suspend(pm_message_t state)
  */
 static int device_prepare(struct device *dev, pm_message_t state)
 {
+	int (*callback)(struct device *) = NULL;
+	char *info = NULL;
 	int error = 0;
 
 	device_lock(dev);
@@ -956,25 +989,27 @@ static int device_prepare(struct device
 	dev->power.wakeup_path = device_may_wakeup(dev);
 
 	if (dev->pm_domain) {
-		pm_dev_dbg(dev, state, "preparing power domain ");
-		if (dev->pm_domain->ops.prepare)
-			error = dev->pm_domain->ops.prepare(dev);
-		suspend_report_result(dev->pm_domain->ops.prepare, error);
+		info = "preparing power domain ";
+		callback = dev->pm_domain->ops.prepare;
 	} else if (dev->type && dev->type->pm) {
-		pm_dev_dbg(dev, state, "preparing type ");
-		if (dev->type->pm->prepare)
-			error = dev->type->pm->prepare(dev);
-		suspend_report_result(dev->type->pm->prepare, error);
+		info = "preparing type ";
+		callback = dev->type->pm->prepare;
 	} else if (dev->class && dev->class->pm) {
-		pm_dev_dbg(dev, state, "preparing class ");
-		if (dev->class->pm->prepare)
-			error = dev->class->pm->prepare(dev);
-		suspend_report_result(dev->class->pm->prepare, error);
+		info = "preparing class ";
+		callback = dev->class->pm->prepare;
 	} else if (dev->bus && dev->bus->pm) {
-		pm_dev_dbg(dev, state, "preparing ");
-		if (dev->bus->pm->prepare)
-			error = dev->bus->pm->prepare(dev);
-		suspend_report_result(dev->bus->pm->prepare, error);
+		info = "preparing bus ";
+		callback = dev->bus->pm->prepare;
+	}
+
+	if (!callback && dev->driver && dev->driver->pm) {
+		info = "preparing driver ";
+		callback = dev->driver->pm->prepare;
+	}
+
+	if (callback) {
+		error = callback(dev);
+		suspend_report_result(callback, error);
 	}
 
 	device_unlock(dev);


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Update][PATCH 3/5] PM / Sleep: Remove forward-only callbacks from platform bus type
  2011-12-15 22:34 ` [Update][PATCH 0/5] " Rafael J. Wysocki
  2011-12-15 22:35   ` [Update][PATCH 1/5] PM / Sleep: Make pm_op() and pm_noirq_op() return callback pointers Rafael J. Wysocki
  2011-12-15 22:36   ` [Update][PATCH 2/5] PM: Run the driver callback directly if the subsystem one is not there Rafael J. Wysocki
@ 2011-12-15 22:37   ` Rafael J. Wysocki
  2011-12-15 22:38   ` [Update][PATCH 4/5] PM / Sleep: Remove forward-only callbacks from AMBA " Rafael J. Wysocki
                     ` (2 subsequent siblings)
  5 siblings, 0 replies; 20+ messages in thread
From: Rafael J. Wysocki @ 2011-12-15 22:37 UTC (permalink / raw)
  To: Linux PM list
  Cc: Greg Kroah-Hartman, LKML, Russell King, Namhyung Kim, Alan Stern

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

The forward-only PM callbacks provided by the platform bus type are
not necessary any more, because the PM core executes driver callbacks
when the corresponding subsystem callbacks are not present, so drop
them.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/base/platform.c         |  115 ----------------------------------------
 include/linux/platform_device.h |   30 ----------
 2 files changed, 1 insertion(+), 144 deletions(-)

Index: linux/drivers/base/platform.c
===================================================================
--- linux.orig/drivers/base/platform.c
+++ linux/drivers/base/platform.c
@@ -700,25 +700,6 @@ static int platform_legacy_resume(struct
 	return ret;
 }
 
-int platform_pm_prepare(struct device *dev)
-{
-	struct device_driver *drv = dev->driver;
-	int ret = 0;
-
-	if (drv && drv->pm && drv->pm->prepare)
-		ret = drv->pm->prepare(dev);
-
-	return ret;
-}
-
-void platform_pm_complete(struct device *dev)
-{
-	struct device_driver *drv = dev->driver;
-
-	if (drv && drv->pm && drv->pm->complete)
-		drv->pm->complete(dev);
-}
-
 #endif /* CONFIG_PM_SLEEP */
 
 #ifdef CONFIG_SUSPEND
@@ -741,22 +722,6 @@ int platform_pm_suspend(struct device *d
 	return ret;
 }
 
-int platform_pm_suspend_noirq(struct device *dev)
-{
-	struct device_driver *drv = dev->driver;
-	int ret = 0;
-
-	if (!drv)
-		return 0;
-
-	if (drv->pm) {
-		if (drv->pm->suspend_noirq)
-			ret = drv->pm->suspend_noirq(dev);
-	}
-
-	return ret;
-}
-
 int platform_pm_resume(struct device *dev)
 {
 	struct device_driver *drv = dev->driver;
@@ -775,22 +740,6 @@ int platform_pm_resume(struct device *de
 	return ret;
 }
 
-int platform_pm_resume_noirq(struct device *dev)
-{
-	struct device_driver *drv = dev->driver;
-	int ret = 0;
-
-	if (!drv)
-		return 0;
-
-	if (drv->pm) {
-		if (drv->pm->resume_noirq)
-			ret = drv->pm->resume_noirq(dev);
-	}
-
-	return ret;
-}
-
 #endif /* CONFIG_SUSPEND */
 
 #ifdef CONFIG_HIBERNATE_CALLBACKS
@@ -813,22 +762,6 @@ int platform_pm_freeze(struct device *de
 	return ret;
 }
 
-int platform_pm_freeze_noirq(struct device *dev)
-{
-	struct device_driver *drv = dev->driver;
-	int ret = 0;
-
-	if (!drv)
-		return 0;
-
-	if (drv->pm) {
-		if (drv->pm->freeze_noirq)
-			ret = drv->pm->freeze_noirq(dev);
-	}
-
-	return ret;
-}
-
 int platform_pm_thaw(struct device *dev)
 {
 	struct device_driver *drv = dev->driver;
@@ -847,22 +780,6 @@ int platform_pm_thaw(struct device *dev)
 	return ret;
 }
 
-int platform_pm_thaw_noirq(struct device *dev)
-{
-	struct device_driver *drv = dev->driver;
-	int ret = 0;
-
-	if (!drv)
-		return 0;
-
-	if (drv->pm) {
-		if (drv->pm->thaw_noirq)
-			ret = drv->pm->thaw_noirq(dev);
-	}
-
-	return ret;
-}
-
 int platform_pm_poweroff(struct device *dev)
 {
 	struct device_driver *drv = dev->driver;
@@ -881,22 +798,6 @@ int platform_pm_poweroff(struct device *
 	return ret;
 }
 
-int platform_pm_poweroff_noirq(struct device *dev)
-{
-	struct device_driver *drv = dev->driver;
-	int ret = 0;
-
-	if (!drv)
-		return 0;
-
-	if (drv->pm) {
-		if (drv->pm->poweroff_noirq)
-			ret = drv->pm->poweroff_noirq(dev);
-	}
-
-	return ret;
-}
-
 int platform_pm_restore(struct device *dev)
 {
 	struct device_driver *drv = dev->driver;
@@ -913,22 +814,6 @@ int platform_pm_restore(struct device *d
 	}
 
 	return ret;
-}
-
-int platform_pm_restore_noirq(struct device *dev)
-{
-	struct device_driver *drv = dev->driver;
-	int ret = 0;
-
-	if (!drv)
-		return 0;
-
-	if (drv->pm) {
-		if (drv->pm->restore_noirq)
-			ret = drv->pm->restore_noirq(dev);
-	}
-
-	return ret;
 }
 
 #endif /* CONFIG_HIBERNATE_CALLBACKS */
Index: linux/include/linux/platform_device.h
===================================================================
--- linux.orig/include/linux/platform_device.h
+++ linux/include/linux/platform_device.h
@@ -264,62 +264,34 @@ static inline char *early_platform_drive
 }
 #endif /* MODULE */
 
-#ifdef CONFIG_PM_SLEEP
-extern int platform_pm_prepare(struct device *dev);
-extern void platform_pm_complete(struct device *dev);
-#else
-#define platform_pm_prepare	NULL
-#define platform_pm_complete	NULL
-#endif
-
 #ifdef CONFIG_SUSPEND
 extern int platform_pm_suspend(struct device *dev);
-extern int platform_pm_suspend_noirq(struct device *dev);
 extern int platform_pm_resume(struct device *dev);
-extern int platform_pm_resume_noirq(struct device *dev);
 #else
 #define platform_pm_suspend		NULL
 #define platform_pm_resume		NULL
-#define platform_pm_suspend_noirq	NULL
-#define platform_pm_resume_noirq	NULL
 #endif
 
 #ifdef CONFIG_HIBERNATE_CALLBACKS
 extern int platform_pm_freeze(struct device *dev);
-extern int platform_pm_freeze_noirq(struct device *dev);
 extern int platform_pm_thaw(struct device *dev);
-extern int platform_pm_thaw_noirq(struct device *dev);
 extern int platform_pm_poweroff(struct device *dev);
-extern int platform_pm_poweroff_noirq(struct device *dev);
 extern int platform_pm_restore(struct device *dev);
-extern int platform_pm_restore_noirq(struct device *dev);
 #else
 #define platform_pm_freeze		NULL
 #define platform_pm_thaw		NULL
 #define platform_pm_poweroff		NULL
 #define platform_pm_restore		NULL
-#define platform_pm_freeze_noirq	NULL
-#define platform_pm_thaw_noirq		NULL
-#define platform_pm_poweroff_noirq	NULL
-#define platform_pm_restore_noirq	NULL
 #endif
 
 #ifdef CONFIG_PM_SLEEP
 #define USE_PLATFORM_PM_SLEEP_OPS \
-	.prepare = platform_pm_prepare, \
-	.complete = platform_pm_complete, \
 	.suspend = platform_pm_suspend, \
 	.resume = platform_pm_resume, \
 	.freeze = platform_pm_freeze, \
 	.thaw = platform_pm_thaw, \
 	.poweroff = platform_pm_poweroff, \
-	.restore = platform_pm_restore, \
-	.suspend_noirq = platform_pm_suspend_noirq, \
-	.resume_noirq = platform_pm_resume_noirq, \
-	.freeze_noirq = platform_pm_freeze_noirq, \
-	.thaw_noirq = platform_pm_thaw_noirq, \
-	.poweroff_noirq = platform_pm_poweroff_noirq, \
-	.restore_noirq = platform_pm_restore_noirq,
+	.restore = platform_pm_restore,
 #else
 #define USE_PLATFORM_PM_SLEEP_OPS
 #endif


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Update][PATCH 4/5] PM / Sleep: Remove forward-only callbacks from AMBA bus type
  2011-12-15 22:34 ` [Update][PATCH 0/5] " Rafael J. Wysocki
                     ` (2 preceding siblings ...)
  2011-12-15 22:37   ` [Update][PATCH 3/5] PM / Sleep: Remove forward-only callbacks from platform bus type Rafael J. Wysocki
@ 2011-12-15 22:38   ` Rafael J. Wysocki
  2011-12-15 22:39   ` [Update][PATCH 5/5] PM: Drop generic_subsys_pm_ops Rafael J. Wysocki
  2011-12-15 23:08   ` [Update][PATCH 0/5] PM: Make the PM core execute driver callbacks if subsystem ones are not present Greg KH
  5 siblings, 0 replies; 20+ messages in thread
From: Rafael J. Wysocki @ 2011-12-15 22:38 UTC (permalink / raw)
  To: Linux PM list
  Cc: Greg Kroah-Hartman, LKML, Russell King, Namhyung Kim, Alan Stern

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

The forward-only PM callbacks provided by the AMBA bus type are not
necessary any more, because the PM core executes driver callbacks
when the corresponding subsystem callbacks are not present, so drop
them.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/amba/bus.c |  136 -----------------------------------------------------
 1 file changed, 1 insertion(+), 135 deletions(-)

Index: linux/drivers/amba/bus.c
===================================================================
--- linux.orig/drivers/amba/bus.c
+++ linux/drivers/amba/bus.c
@@ -109,31 +109,7 @@ static int amba_legacy_resume(struct dev
 	return ret;
 }
 
-static int amba_pm_prepare(struct device *dev)
-{
-	struct device_driver *drv = dev->driver;
-	int ret = 0;
-
-	if (drv && drv->pm && drv->pm->prepare)
-		ret = drv->pm->prepare(dev);
-
-	return ret;
-}
-
-static void amba_pm_complete(struct device *dev)
-{
-	struct device_driver *drv = dev->driver;
-
-	if (drv && drv->pm && drv->pm->complete)
-		drv->pm->complete(dev);
-}
-
-#else /* !CONFIG_PM_SLEEP */
-
-#define amba_pm_prepare		NULL
-#define amba_pm_complete		NULL
-
-#endif /* !CONFIG_PM_SLEEP */
+#endif /* CONFIG_PM_SLEEP */
 
 #ifdef CONFIG_SUSPEND
 
@@ -155,22 +131,6 @@ static int amba_pm_suspend(struct device
 	return ret;
 }
 
-static int amba_pm_suspend_noirq(struct device *dev)
-{
-	struct device_driver *drv = dev->driver;
-	int ret = 0;
-
-	if (!drv)
-		return 0;
-
-	if (drv->pm) {
-		if (drv->pm->suspend_noirq)
-			ret = drv->pm->suspend_noirq(dev);
-	}
-
-	return ret;
-}
-
 static int amba_pm_resume(struct device *dev)
 {
 	struct device_driver *drv = dev->driver;
@@ -189,28 +149,10 @@ static int amba_pm_resume(struct device
 	return ret;
 }
 
-static int amba_pm_resume_noirq(struct device *dev)
-{
-	struct device_driver *drv = dev->driver;
-	int ret = 0;
-
-	if (!drv)
-		return 0;
-
-	if (drv->pm) {
-		if (drv->pm->resume_noirq)
-			ret = drv->pm->resume_noirq(dev);
-	}
-
-	return ret;
-}
-
 #else /* !CONFIG_SUSPEND */
 
 #define amba_pm_suspend		NULL
 #define amba_pm_resume		NULL
-#define amba_pm_suspend_noirq	NULL
-#define amba_pm_resume_noirq	NULL
 
 #endif /* !CONFIG_SUSPEND */
 
@@ -234,22 +176,6 @@ static int amba_pm_freeze(struct device
 	return ret;
 }
 
-static int amba_pm_freeze_noirq(struct device *dev)
-{
-	struct device_driver *drv = dev->driver;
-	int ret = 0;
-
-	if (!drv)
-		return 0;
-
-	if (drv->pm) {
-		if (drv->pm->freeze_noirq)
-			ret = drv->pm->freeze_noirq(dev);
-	}
-
-	return ret;
-}
-
 static int amba_pm_thaw(struct device *dev)
 {
 	struct device_driver *drv = dev->driver;
@@ -268,22 +194,6 @@ static int amba_pm_thaw(struct device *d
 	return ret;
 }
 
-static int amba_pm_thaw_noirq(struct device *dev)
-{
-	struct device_driver *drv = dev->driver;
-	int ret = 0;
-
-	if (!drv)
-		return 0;
-
-	if (drv->pm) {
-		if (drv->pm->thaw_noirq)
-			ret = drv->pm->thaw_noirq(dev);
-	}
-
-	return ret;
-}
-
 static int amba_pm_poweroff(struct device *dev)
 {
 	struct device_driver *drv = dev->driver;
@@ -302,22 +212,6 @@ static int amba_pm_poweroff(struct devic
 	return ret;
 }
 
-static int amba_pm_poweroff_noirq(struct device *dev)
-{
-	struct device_driver *drv = dev->driver;
-	int ret = 0;
-
-	if (!drv)
-		return 0;
-
-	if (drv->pm) {
-		if (drv->pm->poweroff_noirq)
-			ret = drv->pm->poweroff_noirq(dev);
-	}
-
-	return ret;
-}
-
 static int amba_pm_restore(struct device *dev)
 {
 	struct device_driver *drv = dev->driver;
@@ -336,32 +230,12 @@ static int amba_pm_restore(struct device
 	return ret;
 }
 
-static int amba_pm_restore_noirq(struct device *dev)
-{
-	struct device_driver *drv = dev->driver;
-	int ret = 0;
-
-	if (!drv)
-		return 0;
-
-	if (drv->pm) {
-		if (drv->pm->restore_noirq)
-			ret = drv->pm->restore_noirq(dev);
-	}
-
-	return ret;
-}
-
 #else /* !CONFIG_HIBERNATE_CALLBACKS */
 
 #define amba_pm_freeze		NULL
 #define amba_pm_thaw		NULL
 #define amba_pm_poweroff		NULL
 #define amba_pm_restore		NULL
-#define amba_pm_freeze_noirq	NULL
-#define amba_pm_thaw_noirq		NULL
-#define amba_pm_poweroff_noirq	NULL
-#define amba_pm_restore_noirq	NULL
 
 #endif /* !CONFIG_HIBERNATE_CALLBACKS */
 
@@ -402,20 +276,12 @@ static int amba_pm_runtime_resume(struct
 #ifdef CONFIG_PM
 
 static const struct dev_pm_ops amba_pm = {
-	.prepare	= amba_pm_prepare,
-	.complete	= amba_pm_complete,
 	.suspend	= amba_pm_suspend,
 	.resume		= amba_pm_resume,
 	.freeze		= amba_pm_freeze,
 	.thaw		= amba_pm_thaw,
 	.poweroff	= amba_pm_poweroff,
 	.restore	= amba_pm_restore,
-	.suspend_noirq	= amba_pm_suspend_noirq,
-	.resume_noirq	= amba_pm_resume_noirq,
-	.freeze_noirq	= amba_pm_freeze_noirq,
-	.thaw_noirq	= amba_pm_thaw_noirq,
-	.poweroff_noirq	= amba_pm_poweroff_noirq,
-	.restore_noirq	= amba_pm_restore_noirq,
 	SET_RUNTIME_PM_OPS(
 		amba_pm_runtime_suspend,
 		amba_pm_runtime_resume,


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Update][PATCH 5/5] PM: Drop generic_subsys_pm_ops
  2011-12-15 22:34 ` [Update][PATCH 0/5] " Rafael J. Wysocki
                     ` (3 preceding siblings ...)
  2011-12-15 22:38   ` [Update][PATCH 4/5] PM / Sleep: Remove forward-only callbacks from AMBA " Rafael J. Wysocki
@ 2011-12-15 22:39   ` Rafael J. Wysocki
  2011-12-15 22:47     ` Rafael J. Wysocki
  2011-12-15 23:08   ` [Update][PATCH 0/5] PM: Make the PM core execute driver callbacks if subsystem ones are not present Greg KH
  5 siblings, 1 reply; 20+ messages in thread
From: Rafael J. Wysocki @ 2011-12-15 22:39 UTC (permalink / raw)
  To: Linux PM list
  Cc: Greg Kroah-Hartman, LKML, Russell King, Namhyung Kim, Alan Stern

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

Since the PM core is now going to execute driver callbacks directly
if the corresponding subsystem callbacks are not present,
forward-only subsystem callbacks (i.e. such that only execute the
corresponding driver callbacks) are not necessary any more.  Thus
it is possible to remove generic_subsys_pm_ops, because the only
callback in there that is not forward-only, .runtime_idle, is not
really used by the only user of generic_subsys_pm_ops, which is
vio_bus_type.

However, the generic callback routines themselves cannot be removed
from generic_ops.c, because they are used individually by a number
of subsystems.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 arch/powerpc/kernel/vio.c        |    1 -
 drivers/base/power/generic_ops.c |   25 -------------------------
 include/linux/pm.h               |   13 -------------
 3 files changed, 39 deletions(-)

Index: linux/drivers/base/power/generic_ops.c
===================================================================
--- linux.orig/drivers/base/power/generic_ops.c
+++ linux/drivers/base/power/generic_ops.c
@@ -302,28 +302,3 @@ void pm_generic_complete(struct device *
 	pm_runtime_idle(dev);
 }
 #endif /* CONFIG_PM_SLEEP */
-
-struct dev_pm_ops generic_subsys_pm_ops = {
-#ifdef CONFIG_PM_SLEEP
-	.prepare = pm_generic_prepare,
-	.suspend = pm_generic_suspend,
-	.suspend_noirq = pm_generic_suspend_noirq,
-	.resume = pm_generic_resume,
-	.resume_noirq = pm_generic_resume_noirq,
-	.freeze = pm_generic_freeze,
-	.freeze_noirq = pm_generic_freeze_noirq,
-	.thaw = pm_generic_thaw,
-	.thaw_noirq = pm_generic_thaw_noirq,
-	.poweroff = pm_generic_poweroff,
-	.poweroff_noirq = pm_generic_poweroff_noirq,
-	.restore = pm_generic_restore,
-	.restore_noirq = pm_generic_restore_noirq,
-	.complete = pm_generic_complete,
-#endif
-#ifdef CONFIG_PM_RUNTIME
-	.runtime_suspend = pm_generic_runtime_suspend,
-	.runtime_resume = pm_generic_runtime_resume,
-	.runtime_idle = pm_generic_runtime_idle,
-#endif
-};
-EXPORT_SYMBOL_GPL(generic_subsys_pm_ops);
Index: linux/include/linux/pm.h
===================================================================
--- linux.orig/include/linux/pm.h
+++ linux/include/linux/pm.h
@@ -300,19 +300,6 @@ const struct dev_pm_ops name = { \
 	SET_RUNTIME_PM_OPS(suspend_fn, resume_fn, idle_fn) \
 }
 
-/*
- * Use this for subsystems (bus types, device types, device classes) that don't
- * need any special suspend/resume handling in addition to invoking the PM
- * callbacks provided by device drivers supporting both the system sleep PM and
- * runtime PM, make the pm member point to generic_subsys_pm_ops.
- */
-#ifdef CONFIG_PM
-extern struct dev_pm_ops generic_subsys_pm_ops;
-#define GENERIC_SUBSYS_PM_OPS	(&generic_subsys_pm_ops)
-#else
-#define GENERIC_SUBSYS_PM_OPS	NULL
-#endif
-
 /**
  * PM_EVENT_ messages
  *
Index: linux/arch/powerpc/kernel/vio.c
===================================================================
--- linux.orig/arch/powerpc/kernel/vio.c
+++ linux/arch/powerpc/kernel/vio.c
@@ -1406,7 +1406,6 @@ static struct bus_type vio_bus_type = {
 	.match = vio_bus_match,
 	.probe = vio_bus_probe,
 	.remove = vio_bus_remove,
-	.pm = GENERIC_SUBSYS_PM_OPS,
 };
 
 /**


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Update][PATCH 5/5] PM: Drop generic_subsys_pm_ops
  2011-12-15 22:39   ` [Update][PATCH 5/5] PM: Drop generic_subsys_pm_ops Rafael J. Wysocki
@ 2011-12-15 22:47     ` Rafael J. Wysocki
  0 siblings, 0 replies; 20+ messages in thread
From: Rafael J. Wysocki @ 2011-12-15 22:47 UTC (permalink / raw)
  To: Linux PM list
  Cc: Greg Kroah-Hartman, LKML, Russell King, Namhyung Kim, Alan Stern,
	Benjamin Herrenschmidt

On Thursday, December 15, 2011, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rjw@sisk.pl>
> 
> Since the PM core is now going to execute driver callbacks directly
> if the corresponding subsystem callbacks are not present,
> forward-only subsystem callbacks (i.e. such that only execute the
> corresponding driver callbacks) are not necessary any more.  Thus
> it is possible to remove generic_subsys_pm_ops, because the only
> callback in there that is not forward-only, .runtime_idle, is not
> really used by the only user of generic_subsys_pm_ops, which is
> vio_bus_type.
> 
> However, the generic callback routines themselves cannot be removed
> from generic_ops.c, because they are used individually by a number
> of subsystems.
> 
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>

Well, Ben should have a look at this one too.  Now CCed.

Thanks,
Rafael


> ---
>  arch/powerpc/kernel/vio.c        |    1 -
>  drivers/base/power/generic_ops.c |   25 -------------------------
>  include/linux/pm.h               |   13 -------------
>  3 files changed, 39 deletions(-)
> 
> Index: linux/drivers/base/power/generic_ops.c
> ===================================================================
> --- linux.orig/drivers/base/power/generic_ops.c
> +++ linux/drivers/base/power/generic_ops.c
> @@ -302,28 +302,3 @@ void pm_generic_complete(struct device *
>  	pm_runtime_idle(dev);
>  }
>  #endif /* CONFIG_PM_SLEEP */
> -
> -struct dev_pm_ops generic_subsys_pm_ops = {
> -#ifdef CONFIG_PM_SLEEP
> -	.prepare = pm_generic_prepare,
> -	.suspend = pm_generic_suspend,
> -	.suspend_noirq = pm_generic_suspend_noirq,
> -	.resume = pm_generic_resume,
> -	.resume_noirq = pm_generic_resume_noirq,
> -	.freeze = pm_generic_freeze,
> -	.freeze_noirq = pm_generic_freeze_noirq,
> -	.thaw = pm_generic_thaw,
> -	.thaw_noirq = pm_generic_thaw_noirq,
> -	.poweroff = pm_generic_poweroff,
> -	.poweroff_noirq = pm_generic_poweroff_noirq,
> -	.restore = pm_generic_restore,
> -	.restore_noirq = pm_generic_restore_noirq,
> -	.complete = pm_generic_complete,
> -#endif
> -#ifdef CONFIG_PM_RUNTIME
> -	.runtime_suspend = pm_generic_runtime_suspend,
> -	.runtime_resume = pm_generic_runtime_resume,
> -	.runtime_idle = pm_generic_runtime_idle,
> -#endif
> -};
> -EXPORT_SYMBOL_GPL(generic_subsys_pm_ops);
> Index: linux/include/linux/pm.h
> ===================================================================
> --- linux.orig/include/linux/pm.h
> +++ linux/include/linux/pm.h
> @@ -300,19 +300,6 @@ const struct dev_pm_ops name = { \
>  	SET_RUNTIME_PM_OPS(suspend_fn, resume_fn, idle_fn) \
>  }
>  
> -/*
> - * Use this for subsystems (bus types, device types, device classes) that don't
> - * need any special suspend/resume handling in addition to invoking the PM
> - * callbacks provided by device drivers supporting both the system sleep PM and
> - * runtime PM, make the pm member point to generic_subsys_pm_ops.
> - */
> -#ifdef CONFIG_PM
> -extern struct dev_pm_ops generic_subsys_pm_ops;
> -#define GENERIC_SUBSYS_PM_OPS	(&generic_subsys_pm_ops)
> -#else
> -#define GENERIC_SUBSYS_PM_OPS	NULL
> -#endif
> -
>  /**
>   * PM_EVENT_ messages
>   *
> Index: linux/arch/powerpc/kernel/vio.c
> ===================================================================
> --- linux.orig/arch/powerpc/kernel/vio.c
> +++ linux/arch/powerpc/kernel/vio.c
> @@ -1406,7 +1406,6 @@ static struct bus_type vio_bus_type = {
>  	.match = vio_bus_match,
>  	.probe = vio_bus_probe,
>  	.remove = vio_bus_remove,
> -	.pm = GENERIC_SUBSYS_PM_OPS,
>  };
>  
>  /**
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 
> 


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Update][PATCH 0/5] PM: Make the PM core execute driver callbacks if subsystem ones are not present
  2011-12-15 22:34 ` [Update][PATCH 0/5] " Rafael J. Wysocki
                     ` (4 preceding siblings ...)
  2011-12-15 22:39   ` [Update][PATCH 5/5] PM: Drop generic_subsys_pm_ops Rafael J. Wysocki
@ 2011-12-15 23:08   ` Greg KH
  5 siblings, 0 replies; 20+ messages in thread
From: Greg KH @ 2011-12-15 23:08 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Linux PM list, LKML, Russell King, Namhyung Kim, Alan Stern

On Thu, Dec 15, 2011 at 11:34:16PM +0100, Rafael J. Wysocki wrote:
> Hi,
> 
> I have one more patch for this series and a couple of fixes.
> 
> On Saturday, December 10, 2011, Rafael J. Wysocki wrote:
> > 
> > The following patchset changes the PM core so that driver callbacks
> > are executed automatically when subsystem ones are not present and
> > uses the new PM core's behavior to simplify the platform and AMBA bus
> > types (there probably are many more bus types that may be simplified
> > this way, but those two were the easiest targets for me).
> > 
> > The reasons why to do this are outlined in the changelog of the second patch.
> 
> [1/5] - Make pm_op and pm_noirq_op return callbacks
> [2/5] - Make PM core call driver callbacks directly if subsystem ones are not present
> [3/5] - Remove unnecessary bus type PM callback from platform bus type
> [4/5] - Remove unnecessary bus type PM callback from AMBA bus type
> [5/5] - Drop generic_subsys_pm_ops
> 
> If there are no objections, I'd like to take these patches for 3.3.

No objection from me, thanks for doing this.

greg k-h

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2011-12-15 23:11 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-12-09 23:18 [PATCH 0/4] PM: Make the PM core execute driver callbacks if subsystem ones are not present Rafael J. Wysocki
2011-12-09 23:20 ` [PATCH 1/4] PM / Sleep: Make pm_op() and pm_noirq_op() return callback pointers Rafael J. Wysocki
2011-12-12  4:04   ` Namhyung Kim
2011-12-12 23:53     ` Rafael J. Wysocki
2011-12-13 13:55       ` Namhyung Kim
2011-12-09 23:20 ` [PATCH 2/4] PM: Run the driver callback directly if the subsystem one is not there Rafael J. Wysocki
2011-12-10 14:59   ` Alan Stern
2011-12-10 18:21     ` Rafael J. Wysocki
2011-12-09 23:21 ` [PATCH 3/4] PM / Sleep: Remove forward-only callbacks from platform bus type Rafael J. Wysocki
2011-12-09 23:23 ` [PATCH 4/4] PM / Sleep: Remove forward-only callbacks from AMBA " Rafael J. Wysocki
2011-12-09 23:47 ` [PATCH 0/4] PM: Make the PM core execute driver callbacks if subsystem ones are not present Greg KH
2011-12-09 23:52   ` Rafael J. Wysocki
2011-12-15 22:34 ` [Update][PATCH 0/5] " Rafael J. Wysocki
2011-12-15 22:35   ` [Update][PATCH 1/5] PM / Sleep: Make pm_op() and pm_noirq_op() return callback pointers Rafael J. Wysocki
2011-12-15 22:36   ` [Update][PATCH 2/5] PM: Run the driver callback directly if the subsystem one is not there Rafael J. Wysocki
2011-12-15 22:37   ` [Update][PATCH 3/5] PM / Sleep: Remove forward-only callbacks from platform bus type Rafael J. Wysocki
2011-12-15 22:38   ` [Update][PATCH 4/5] PM / Sleep: Remove forward-only callbacks from AMBA " Rafael J. Wysocki
2011-12-15 22:39   ` [Update][PATCH 5/5] PM: Drop generic_subsys_pm_ops Rafael J. Wysocki
2011-12-15 22:47     ` Rafael J. Wysocki
2011-12-15 23:08   ` [Update][PATCH 0/5] PM: Make the PM core execute driver callbacks if subsystem ones are not present Greg KH

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.