[PATCH 0/3] PM: Documentation updates

[PATCH 0/3] PM: Documentation updates

[Rafael J. Wysocki]

Hi,

The following patches update the PM documenataion in the kernel tree to
follow the current code.

[1/3] - Document the use of PM domains by the PM core correctly.
[2/3] - Update system suspend/hibernation documentation to follow the code.
[3/3] - Update runtime PM documentation to follow the code.

Thanks,
Rafael

[PATCH 1/3] PM / Domains: Document how PM domains are used by the PM core

[Rafael J. Wysocki]

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

The current power management documentation in Documentation/power/
either doesn't cover PM domains at all, or gives inaccurate
information about them, so update the relevant files in there to
follow the code.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 Documentation/power/devices.txt    |   42 +++++++++++++++++++++++--------------
 Documentation/power/runtime_pm.txt |   29 +++++++++++++++----------
 2 files changed, 45 insertions(+), 26 deletions(-)

Index: linux/Documentation/power/devices.txt
===================================================================
--- linux.orig/Documentation/power/devices.txt
+++ linux/Documentation/power/devices.txt
@@ -123,9 +123,10 @@ please refer directly to the source code
 Subsystem-Level Methods
 -----------------------
 The core methods to suspend and resume devices reside in struct dev_pm_ops
-pointed to 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 buses, like PCI or USB, or device type and device class drivers.
+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.
 
 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
@@ -251,18 +252,29 @@ 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 IRQ_WAKEUP flag).
 
-All phases use bus, type, or class callbacks (that is, methods defined in
-dev->bus->pm, dev->type->pm, or dev->class->pm).  These callbacks are mutually
-exclusive, so if the device type provides a struct dev_pm_ops object pointed to
-by its pm field (i.e. both dev->type and dev->type->pm are defined), the
-callbacks included in that object (i.e. dev->type->pm) will be used.  Otherwise,
-if the class provides a struct dev_pm_ops object pointed to by its pm field
-(i.e. both dev->class and dev->class->pm are defined), the PM core will use the
-callbacks from that object (i.e. dev->class->pm).  Finally, if the pm fields of
-both the device type and class objects are NULL (or those objects do not exist),
-the callbacks provided by the bus (that is, the callbacks from dev->bus->pm)
-will be used (this allows device types to override callbacks provided by bus
-types or classes if necessary).
+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.
+
+    2.	Otherwise, if both dev->type and dev->type->pm are present, the callback
+	included in dev->type->pm will be executed.
+
+    3.	Otherwise, if both dev->class and dev->class->pm are present, the
+	callback included in dev->class->pm will be executed.
+
+    4.	Otherwise, if both dev->bus and dev->bus->pm are present, the callback
+	included in dev->bus->pm will be executed.
+
+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.
Index: linux/Documentation/power/runtime_pm.txt
===================================================================
--- linux.orig/Documentation/power/runtime_pm.txt
+++ linux/Documentation/power/runtime_pm.txt
@@ -44,17 +44,24 @@ struct dev_pm_ops {
 };
 
 The ->runtime_suspend(), ->runtime_resume() and ->runtime_idle() callbacks
-are executed by the PM core for either the power domain, or the device type
-(if the device power domain's struct dev_pm_ops does not exist), or the class
-(if the device power domain's and type's struct dev_pm_ops object does not
-exist), or the bus type (if the device power domain's, type's and class'
-struct dev_pm_ops objects do not exist) of the given device, so the priority
-order of callbacks from high to low is that power domain callbacks, device
-type callbacks, class callbacks and bus type callbacks, and the high priority
-one will take precedence over low priority one. The bus type, device type and
-class callbacks are referred to as subsystem-level callbacks in what follows,
-and generally speaking, the power domain callbacks are used for representing
-power domains within a SoC.
+are executed by the PM core for the device's subsystem that may be either of
+the following:
+
+  1. PM domain of the device, if the device's PM domain object, dev->pm_domain,
+     is present.
+
+  2. Device type of the device, if both dev->type and dev->type->pm are present.
+
+  3. Device class of the device, if both dev->class and dev->class->pm are
+     present.
+
+  4. Bus type of the device, if both dev->bus and dev->bus->pm are 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
+a low-priority one.  The PM domain, bus type, device type and class callbacks
+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

[PATCH 2/3] PM / Sleep: Correct inaccurate information in devices.txt

[Rafael J. Wysocki]

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

The documentation file Documentation/power/devices.txt contains some
information that isn't correct any more due to code modifications
made after that file had been created (or updated last time).  Fix
this.

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

Index: linux/Documentation/power/devices.txt
===================================================================
--- linux.orig/Documentation/power/devices.txt
+++ linux/Documentation/power/devices.txt
@@ -250,7 +250,7 @@ for every device before the next phase b
 support all these callbacks and not all drivers use all the callbacks.  The
 various phases always run after tasks have been frozen and before they are
 unfrozen.  Furthermore, the *_noirq phases run at a time when IRQ handlers have
-been disabled (except for those marked with the IRQ_WAKEUP flag).
+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).
@@ -295,9 +295,8 @@ When the system goes into the standby or
 
 	After the prepare callback method returns, no new children may be
 	registered below the device.  The method may also prepare the device or
-	driver in some way for the upcoming system power transition (for
-	example, by allocating additional memory required for this purpose), but
-	it should not put the device into a low-power state.
+	driver in some way for the upcoming system power transition, but it
+	should not put the device into a low-power state.
 
     2.	The suspend methods should quiesce the device to stop it from performing
 	I/O.  They also may save the device registers and put it into the

[PATCH 3/3] PM / Runtime: Make documentation follow the new behavior of irq_safe

[Rafael J. Wysocki]

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

The runtime PM core code behavior related to the power.irq_safe
device flag has changed recently and the documentation should be
modified to reflect it.

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

Index: linux/Documentation/power/runtime_pm.txt
===================================================================
--- linux.orig/Documentation/power/runtime_pm.txt
+++ linux/Documentation/power/runtime_pm.txt
@@ -65,11 +65,12 @@ are referred to as subsystem-level callb
 
 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 a device's ->runtime_suspend() and ->runtime_resume()
-callbacks should be invoked in atomic context with interrupts disabled.
-This implies that these callback routines must not block or sleep, but it also
-means that the synchronous helper functions listed at the end of Section 4 can
-be used within an interrupt handler or in an atomic context.
+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

[PATCH 4]

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>
Subject: PM / Sleep: Update documentation related to system wakeup

The system wakeup section of Documentation/power/devices.txt is
outdated, so make it agree with the current code.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 Documentation/power/devices.txt |   56 +++++++++++++++++++++++++---------------
 1 file changed, 36 insertions(+), 20 deletions(-)

Index: linux/Documentation/power/devices.txt
===================================================================
--- linux.orig/Documentation/power/devices.txt
+++ linux/Documentation/power/devices.txt
@@ -140,41 +140,57 @@ sequencing in the driver model tree.
 
 /sys/devices/.../power/wakeup files
 -----------------------------------
-All devices in the driver model have two flags to control handling of wakeup
-events (hardware signals that can force the device and/or system out of a low
-power state).  These flags are initialized by bus or device driver code using
+All device objects in the driver model contain fields that control the handling
+of system wakeup events (hardware signals that can force the system out of a
+sleep state).  These fields are initialized by bus or device driver code using
 device_set_wakeup_capable() and device_set_wakeup_enable(), defined in
 include/linux/pm_wakeup.h.
 
-The "can_wakeup" flag just records whether the device (and its driver) can
+The "power.can_wakeup" flag just records whether the device (and its driver) can
 physically support wakeup events.  The device_set_wakeup_capable() routine
-affects this flag.  The "should_wakeup" flag controls whether the device should
-try to use its wakeup mechanism.  device_set_wakeup_enable() affects this flag;
-for the most part drivers should not change its value.  The initial value of
-should_wakeup is supposed to be false for the majority of devices; the major
-exceptions are power buttons, keyboards, and Ethernet adapters whose WoL
-(wake-on-LAN) feature has been set up with ethtool.  It should also default
-to true for devices that don't generate wakeup requests on their own but merely
-forward wakeup requests from one bus to another (like PCI bridges).
+affects this flag.  The "power.wakeup" field is a pointer to an object of type
+struct wakeup_source used for controlling whether or not the device should use
+its system wakeup mechanism and for notifying the PM core of system wakeup
+events signaled by the device.  This object is only present for wakeup-capable
+devices (i.e. devices whose "can_wakeup" flags are set) and is created (or
+removed) by device_set_wakeup_capable().
 
 Whether or not a device is capable of issuing wakeup events is a hardware
 matter, and the kernel is responsible for keeping track of it.  By contrast,
 whether or not a wakeup-capable device should issue wakeup events is a policy
 decision, and it is managed by user space through a sysfs attribute: the
-power/wakeup file.  User space can write the strings "enabled" or "disabled" to
-set or clear the "should_wakeup" flag, respectively.  This file is only present
-for wakeup-capable devices (i.e. devices whose "can_wakeup" flags are set)
-and is created (or removed) by device_set_wakeup_capable().  Reads from the
-file will return the corresponding string.
+"power/wakeup" file.  User space can write the strings "enabled" or "disabled"
+to it to indicate whether or not, respectively, the device is supposed to signal
+system wakeup.  This file is only present if the "power.wakeup" object exists
+for the given device and is created (or removed) along with that object, by
+device_set_wakeup_capable().  Reads from the file will return the corresponding
+string.
+
+The "power/wakeup" file is supposed to contain the "disabled" string initially
+for the majority of devices; the major exceptions are power buttons, keyboards,
+and Ethernet adapters whose WoL (wake-on-LAN) feature has been set up with
+ethtool.  It should also default to "enabled" for devices that don't generate
+wakeup requests on their own but merely forward wakeup requests from one bus to
+another (like PCI Express ports).
 
-The device_may_wakeup() routine returns true only if both flags are set.
+The device_may_wakeup() routine returns true only if the "power.wakeup" object
+exists and the corresponding "power/wakeup" file contains the string "enabled".
 This information is used by subsystems, like the PCI bus type code, to see
 whether or not to enable the devices' wakeup mechanisms.  If device wakeup
 mechanisms are enabled or disabled directly by drivers, they also should use
 device_may_wakeup() to decide what to do during a system sleep transition.
-However for runtime power management, wakeup events should be enabled whenever
-the device and driver both support them, regardless of the should_wakeup flag.
+Device drivers, however, are not supposed to call device_set_wakeup_enable()
+directly in any case.
 
+It ought to be noted that system wakeup is conceptually different from "remote
+wakeup" used by runtime power management, although it may be supported by the
+same physical mechanism.  Remote wakeup is a feature allowing devices in
+low-power states to trigger specific interrupts to signal conditions in which
+they should be put into the full-power state.  Those interrupts may or may not
+be used to signal system wakeup events, depending on the hardware design.  On
+some systems it is impossible to trigger them from system sleep states.  In any
+case, remote wakeup should always be enabled for runtime power management for
+all devices and drivers that support it.
 
 /sys/devices/.../power/control files
 ------------------------------------

[PATCH 5] PM: Update comments describing device power management callbacks

[Rafael J. Wysocki]

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

The comments describing device power management callbacks in
include/pm.h are outdated and somewhat confusing, so make them
reflect the reality more accurately.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 include/linux/pm.h |  193 ++++++++++++++++++++++++++++++-----------------------
 1 file changed, 110 insertions(+), 83 deletions(-)

Index: linux/include/linux/pm.h
===================================================================
--- linux.orig/include/linux/pm.h
+++ linux/include/linux/pm.h
@@ -54,15 +54,22 @@ typedef struct pm_message {
 /**
  * struct dev_pm_ops - device PM callbacks
  *
- * Several driver power state transitions are externally visible, affecting
+ * Several device power state transitions are externally visible, affecting
  * the state of pending I/O queues and (for drivers that touch hardware)
  * interrupts, wakeups, DMA, and other hardware state.  There may also be
- * internal transitions to various low power modes, which are transparent
+ * internal transitions to various low-power modes which are transparent
  * to the rest of the driver stack (such as a driver that's ON gating off
  * clocks which are not in active use).
  *
- * The externally visible transitions are handled with the help of the following
- * callbacks included in this structure:
+ * The externally visible transitions are handled with the help of callbacks
+ * included in this structure in such a way that two levels of callbacks are
+ * involved.  First, the PM core executes callbacks provided by PM domains,
+ * device types, classes and bus types.  They are the subsystem-level callbacks
+ * supposed to execute callbacks provided by device drivers, although they may
+ * choose not to do that.  If the driver callbacks are executed, they have to
+ * collaborate with the subsystem-level callbacks to achieve the goals
+ * appropriate for the given system transition, given transition phase and the
+ * subsystem the device belongs to.
  *
  * @prepare: Prepare the device for the upcoming transition, but do NOT change
  *	its hardware state.  Prevent new children of the device from being
@@ -71,101 +78,118 @@ typedef struct pm_message {
  *	probe method from being made too once @prepare() has succeeded).  If
  *	@prepare() detects a situation it cannot handle (e.g. registration of a
  *	child already in progress), it may return -EAGAIN, so that the PM core
- *	can execute it once again (e.g. after the new child has been registered)
+ *	can execute it once again (e.g. after a new child has been registered)
  *	to recover from the race condition.  This method is executed for all
  *	kinds of suspend transitions and is followed by one of the suspend
  *	callbacks: @suspend(), @freeze(), or @poweroff().
- *	The PM core executes @prepare() for all devices before starting to
- *	execute suspend callbacks for any of them, so drivers may assume all of
- *	the other devices to be present and functional while @prepare() is being
- *	executed.  In particular, it is safe to make GFP_KERNEL memory
- *	allocations from within @prepare().  However, drivers may NOT assume
- *	anything about the availability of the user space at that time and it
- *	is not correct to request firmware from within @prepare() (it's too
- *	late to do that).  [To work around this limitation, drivers may
- *	register suspend and hibernation notifiers that are executed before the
+ *	The PM core executes subsystem-level @prepare() for all devices before
+ *	starting to execute suspend callbacks for any of them, so all devices
+ *	may be assumed to be present and functional while @prepare() is being
+ *	executed.  However, device drivers may NOT assume anything about the
+ *	availability of user space at that time and it is NOT valid to request
+ *	firmware from within @prepare() (it's too late to do that).  It also is
+ *	NOT valid to allocate substantial amounts of memory from @prepare() in
+ *	the GFP_KERNEL mode.  [To work around these limitations, drivers may
+ *	register suspend and hibernation notifiers to be executed before the
  *	freezing of tasks.]
  *
  * @complete: Undo the changes made by @prepare().  This method is executed for
  *	all kinds of resume transitions, following one of the resume callbacks:
  *	@resume(), @thaw(), @restore().  Also called if the state transition
- *	fails before the driver's suspend callback (@suspend(), @freeze(),
- *	@poweroff()) can be executed (e.g. if the suspend callback fails for one
+ *	fails before the driver's suspend callback: @suspend(), @freeze() or
+ *	@poweroff(), can be executed (e.g. if the suspend callback fails for one
  *	of the other devices that the PM core has unsuccessfully attempted to
  *	suspend earlier).
- *	The PM core executes @complete() after it has executed the appropriate
- *	resume callback for all devices.
+ *	The PM core executes subsystem-level @complete() after it has executed
+ *	the appropriate resume callbacks for all devices.
  *
  * @suspend: Executed before putting the system into a sleep state in which the
- *	contents of main memory are preserved.  Quiesce the device, put it into
- *	a low power state appropriate for the upcoming system state (such as
- *	PCI_D3hot), and enable wakeup events as appropriate.
+ *	contents of main memory are preserved.  The exact action to perform
+ *	depends on the device's subsystem (PM domain, device type, class or bus
+ *	type), but generally the device must be quiescent after @suspend() has
+ *	returned, so that it doesn't do any I/O or DMA.
+ *	Subsystem-level @suspend() is executed for all devices after invoking
+ *	subsystem-level @prepare() for all of them.
  *
  * @resume: Executed after waking the system up from a sleep state in which the
- *	contents of main memory were preserved.  Put the device into the
- *	appropriate state, according to the information saved in memory by the
- *	preceding @suspend().  The driver starts working again, responding to
- *	hardware events and software requests.  The hardware may have gone
- *	through a power-off reset, or it may have maintained state from the
- *	previous suspend() which the driver may rely on while resuming.  On most
- *	platforms, there are no restrictions on availability of resources like
- *	clocks during @resume().
+ *	contents of main memory were preserved.  Undo the changes made by
+ *	the preceding @suspend() and cause the device to become operational
+ *	(the exact action to perform depends on the device's subsystem).
+ *	The driver starts working again, responding to hardware events and
+ *	software requests.  The state of the device at the time its driver's
+ *	@resume() callback is run depends on the platform and subsystem the
+ *	device belongs to.  On most platforms, there are no restrictions on
+ *	availability of resources like clocks during @resume().
+ *	Subsystem-level @resume() is executed for all devices after invoking
+ *	subsystem-level @resume_noirq() for all of them.
  *
  * @freeze: Hibernation-specific, executed before creating a hibernation image.
- *	Quiesce operations so that a consistent image can be created, but do NOT
- *	otherwise put the device into a low power device state and do NOT emit
- *	system wakeup events.  Save in main memory the device settings to be
- *	used by @restore() during the subsequent resume from hibernation or by
- *	the subsequent @thaw(), if the creation of the image or the restoration
- *	of main memory contents from it fails.
+ *	Analogous to @suspend(), but it should not enable the device to signal
+ *	wakeup events.  The majority of subsystems (with the notable exception
+ *	of the PCI bus type) expect the driver-level @freeze() to save the
+ *	device settings in memory to be used by @restore() during the subsequent
+ *	resume from hibernation.
+ *	Subsystem-level @freeze() is executed for all devices after invoking
+ *	subsystem-level @prepare() for all of them.
  *
  * @thaw: Hibernation-specific, executed after creating a hibernation image OR
- *	if the creation of the image fails.  Also executed after a failing
+ *	if the creation of an image has failed.  Also executed after a failing
  *	attempt to restore the contents of main memory from such an image.
  *	Undo the changes made by the preceding @freeze(), so the device can be
  *	operated in the same way as immediately before the call to @freeze().
+ *	Subsystem-level @thaw() is executed for all devices after invoking
+ *	subsystem-level @thaw_noirq() for all of them.  It also may be executed
+ *	directly after @freeze() in case of a transition error.
  *
  * @poweroff: Hibernation-specific, executed after saving a hibernation image.
- *	Quiesce the device, put it into a low power state appropriate for the
- *	upcoming system state (such as PCI_D3hot), and enable wakeup events as
- *	appropriate.
+ *	Analogous to @suspend(), but it need not save the the device settings in
+ *	memory.
+ *	Subsystem-level @poweroff() is executed for all devices after invoking
+ *	subsystem-level @prepare() for all of them.
  *
  * @restore: Hibernation-specific, executed after restoring the contents of main
- *	memory from a hibernation image.  Driver starts working again,
- *	responding to hardware events and software requests.  Drivers may NOT
- *	make ANY assumptions about the hardware state right prior to @restore().
- *	On most platforms, there are no restrictions on availability of
- *	resources like clocks during @restore().
- *
- * @suspend_noirq: Complete the operations of ->suspend() by carrying out any
- *	actions required for suspending the device that need interrupts to be
- *	disabled
- *
- * @resume_noirq: Prepare for the execution of ->resume() by carrying out any
- *	actions required for resuming the device that need interrupts to be
- *	disabled
- *
- * @freeze_noirq: Complete the operations of ->freeze() by carrying out any
- *	actions required for freezing the device that need interrupts to be
- *	disabled
- *
- * @thaw_noirq: Prepare for the execution of ->thaw() by carrying out any
- *	actions required for thawing the device that need interrupts to be
- *	disabled
- *
- * @poweroff_noirq: Complete the operations of ->poweroff() by carrying out any
- *	actions required for handling the device that need interrupts to be
- *	disabled
- *
- * @restore_noirq: Prepare for the execution of ->restore() by carrying out any
- *	actions required for restoring the operations of the device that need
- *	interrupts to be disabled
+ *	memory from a hibernation image.  The state of the device at the time
+ *	its driver's @restore() callback is run depends on the platform and
+ *	subsystem the device belongs to.  On most platforms, there are no
+ *	restrictions on availability of resources like clocks during @restore().
+ *	Subsystem-level @restore() is executed for all devices after invoking
+ *	subsystem-level @restore_noirq() for all of them.
+ *
+ * @suspend_noirq: Complete the actions started by @suspend().  Carry out any
+ *	additional operations required for suspending the device that might be
+ *	racing with its driver's interrupt handler, which is guaranteed not to
+ *	run while @suspend_noirq() is being executed.
+ *
+ * @resume_noirq: Prepare for the execution of @resume() by carrying out any
+ *	operations required for resuming the device that might be racing with
+ *	its driver's interrupt handler, which is guaranteed not to run while
+ *	@resume_noirq() is being executed.
+ *
+ * @freeze_noirq: Complete the actions started by @freeze().  Carry out any
+ *	additional operations required for freezing the device that might be
+ *	racing with its driver's interrupt handler, which is guaranteed not to
+ *	run while @freeze_noirq() is being executed.
+ *
+ * @thaw_noirq: Prepare for the execution of @thaw() by carrying out any
+ *	operations required for thawing the device that might be racing with its
+ *	driver's interrupt handler, which is guaranteed not to run while
+ *	@thaw_noirq() is being executed.
+ *
+ * @poweroff_noirq: Complete the actions started by @poweroff().  Carry out any
+ *	additional operations required for powering off the device that might be
+ *	racing with its driver's interrupt handler, which is guaranteed not to
+ *	run while @poweroff_noirq() is being executed.
+ *
+ * @restore_noirq: Prepare for the execution of @restore() by carrying out any
+ *	operations required for thawing the device that might be racing with its
+ *	driver's interrupt handler, which is guaranteed not to run while
+ *	@restore_noirq() is being executed.
  *
  * All of the above callbacks, except for @complete(), return error codes.
  * However, the error codes returned by the resume operations, @resume(),
- * @thaw(), @restore(), @resume_noirq(), @thaw_noirq(), and @restore_noirq() do
+ * @thaw(), @restore(), @resume_noirq(), @thaw_noirq(), and @restore_noirq(), do
  * not cause the PM core to abort the resume transition during which they are
- * returned.  The error codes returned in that cases are only printed by the PM
+ * returned.  The error codes returned in those cases are only printed by the PM
  * core to the system logs for debugging purposes.  Still, it is recommended
  * that drivers only return error codes from their resume methods in case of an
  * unrecoverable failure (i.e. when the device being handled refuses to resume
@@ -174,29 +198,32 @@ typedef struct pm_message {
  * their children.
  *
  * It is allowed to unregister devices while the above callbacks are being
- * executed.  However, it is not allowed to unregister a device from within any
- * of its own callbacks.
+ * executed.  However, it is NOT allowed to unregister a device from within any
+ * of its driver's callbacks.
  *
- * There also are the following callbacks related to run-time power management
- * of devices:
+ * There also are callbacks related to runtime power management of devices.
+ * Again, these callbacks are executed by the PM core only for subsystems
+ * (PM domains, device types, classes and bus types) and the subsystem-level
+ * callbacks are supposed to invoke the driver callbacks.  Moreover, the exact
+ * actions to be performed by a device driver's callbacks generally depend on
+ * the platform and subsystem the device belongs to.
  *
  * @runtime_suspend: Prepare the device for a condition in which it won't be
  *	able to communicate with the CPU(s) and RAM due to power management.
- *	This need not mean that the device should be put into a low power state.
+ *	This need not mean that the device should be put into a low-power state.
  *	For example, if the device is behind a link which is about to be turned
  *	off, the device may remain at full power.  If the device does go to low
- *	power and is capable of generating run-time wake-up events, remote
- *	wake-up (i.e., a hardware mechanism allowing the device to request a
- *	change of its power state via a wake-up event, such as PCI PME) should
- *	be enabled for it.
+ *	power and is capable of generating runtime wakeup events, remote wakeup
+ *	(i.e., a hardware mechanism allowing the device to request a change of
+ *	its power state via an interrupt) should be enabled for it.
  *
  * @runtime_resume: Put the device into the fully active state in response to a
- *	wake-up event generated by hardware or at the request of software.  If
- *	necessary, put the device into the full power state and restore its
+ *	wakeup event generated by hardware or at the request of software.  If
+ *	necessary, put the device into the full-power state and restore its
  *	registers, so that it is fully operational.
  *
- * @runtime_idle: Device appears to be inactive and it might be put into a low
- *	power state if all of the necessary conditions are satisfied.  Check
+ * @runtime_idle: Device appears to be inactive and it might be put into a
+ *	low-power state if all of the necessary conditions are satisfied.  Check
  *	these conditions and handle the device as appropriate, possibly queueing
  *	a suspend request for it.  The return value is ignored by the PM core.
  */

Re: [PATCH 5] PM: Update comments describing device power management callbacks

[Alan Stern]

On Mon, 21 Nov 2011, Rafael J. Wysocki wrote:

> From: Rafael J. Wysocki <rjw@sisk.pl>
> 
> The comments describing device power management callbacks in
> include/pm.h are outdated and somewhat confusing, so make them
> reflect the reality more accurately.

A few comments below...

>   * @freeze: Hibernation-specific, executed before creating a hibernation image.
> - *	Quiesce operations so that a consistent image can be created, but do NOT
> - *	otherwise put the device into a low power device state and do NOT emit
> - *	system wakeup events.  Save in main memory the device settings to be
> - *	used by @restore() during the subsequent resume from hibernation or by
> - *	the subsequent @thaw(), if the creation of the image or the restoration
> - *	of main memory contents from it fails.
> + *	Analogous to @suspend(), but it should not enable the device to signal
> + *	wakeup events.  The majority of subsystems (with the notable exception
> + *	of the PCI bus type) expect the driver-level @freeze() to save the
> + *	device settings in memory to be used by @restore() during the subsequent
> + *	resume from hibernation.
> + *	Subsystem-level @freeze() is executed for all devices after invoking
> + *	subsystem-level @prepare() for all of them.

The first three lines you removed contain some important points which I
think should be retained.

> @@ -174,29 +198,32 @@ typedef struct pm_message {
>   * their children.
>   *
>   * It is allowed to unregister devices while the above callbacks are being
> - * executed.  However, it is not allowed to unregister a device from within any
> - * of its own callbacks.
> + * executed.  However, it is NOT allowed to unregister a device from within any
> + * of its driver's callbacks.

Please be a little more precise here.  If driver D manages two devices,
A and B, then D _is_ allowed to unregister A from within a callback for
B (except if A is an ancestor of B or something like that).  It's just
not allowed to unregister B from within a callback for B.

> - * There also are the following callbacks related to run-time power management
> - * of devices:
> + * There also are callbacks related to runtime power management of devices.
> + * Again, these callbacks are executed by the PM core only for subsystems
> + * (PM domains, device types, classes and bus types) and the subsystem-level
> + * callbacks are supposed to invoke the driver callbacks.  Moreover, the exact
> + * actions to be performed by a device driver's callbacks generally depend on
> + * the platform and subsystem the device belongs to.
>   *
>   * @runtime_suspend: Prepare the device for a condition in which it won't be
>   *	able to communicate with the CPU(s) and RAM due to power management.
> - *	This need not mean that the device should be put into a low power state.
> + *	This need not mean that the device should be put into a low-power state.
>   *	For example, if the device is behind a link which is about to be turned
>   *	off, the device may remain at full power.  If the device does go to low
> - *	power and is capable of generating run-time wake-up events, remote
> - *	wake-up (i.e., a hardware mechanism allowing the device to request a
> - *	change of its power state via a wake-up event, such as PCI PME) should
> - *	be enabled for it.
> + *	power and is capable of generating runtime wakeup events, remote wakeup
> + *	(i.e., a hardware mechanism allowing the device to request a change of
> + *	its power state via an interrupt) should be enabled for it.
>   *
>   * @runtime_resume: Put the device into the fully active state in response to a
> - *	wake-up event generated by hardware or at the request of software.  If
> - *	necessary, put the device into the full power state and restore its
> + *	wakeup event generated by hardware or at the request of software.  If
> + *	necessary, put the device into the full-power state and restore its
>   *	registers, so that it is fully operational.
>   *
> - * @runtime_idle: Device appears to be inactive and it might be put into a low
> - *	power state if all of the necessary conditions are satisfied.  Check
> + * @runtime_idle: Device appears to be inactive and it might be put into a
> + *	low-power state if all of the necessary conditions are satisfied.  Check
>   *	these conditions and handle the device as appropriate, possibly queueing
>   *	a suspend request for it.  The return value is ignored by the PM core.
>   */

Somewhere in here the text should tell people to find more information 
in Documentation/power/runtime_pm.txt.  Likewise for .../devices.txt, 
although that should be added at the appropriate spot higher up.

Alan Stern

Re: [PATCH 5] PM: Update comments describing device power management callbacks

[Rafael J. Wysocki]

On Monday, November 21, 2011, Alan Stern wrote:
> On Mon, 21 Nov 2011, Rafael J. Wysocki wrote:
> 
> > From: Rafael J. Wysocki <rjw@sisk.pl>
> > 
> > The comments describing device power management callbacks in
> > include/pm.h are outdated and somewhat confusing, so make them
> > reflect the reality more accurately.
> 
> A few comments below...
> 
> >   * @freeze: Hibernation-specific, executed before creating a hibernation image.
> > - *	Quiesce operations so that a consistent image can be created, but do NOT
> > - *	otherwise put the device into a low power device state and do NOT emit
> > - *	system wakeup events.  Save in main memory the device settings to be
> > - *	used by @restore() during the subsequent resume from hibernation or by
> > - *	the subsequent @thaw(), if the creation of the image or the restoration
> > - *	of main memory contents from it fails.
> > + *	Analogous to @suspend(), but it should not enable the device to signal
> > + *	wakeup events.  The majority of subsystems (with the notable exception
> > + *	of the PCI bus type) expect the driver-level @freeze() to save the
> > + *	device settings in memory to be used by @restore() during the subsequent
> > + *	resume from hibernation.
> > + *	Subsystem-level @freeze() is executed for all devices after invoking
> > + *	subsystem-level @prepare() for all of them.
> 
> The first three lines you removed contain some important points which I
> think should be retained.

Well, not really, because in fact what the callback is supposed to do depends
on the subsystem.  For example, on PCI freeze is not supposed to save the
the device state even and generally those routines don't emit any events.

> > @@ -174,29 +198,32 @@ typedef struct pm_message {
> >   * their children.
> >   *
> >   * It is allowed to unregister devices while the above callbacks are being
> > - * executed.  However, it is not allowed to unregister a device from within any
> > - * of its own callbacks.
> > + * executed.  However, it is NOT allowed to unregister a device from within any
> > + * of its driver's callbacks.
> 
> Please be a little more precise here.  If driver D manages two devices,
> A and B, then D _is_ allowed to unregister A from within a callback for
> B (except if A is an ancestor of B or something like that).  It's just
> not allowed to unregister B from within a callback for B.

I can leave that as is, but devices (or device objects to be precise) don't
have any callbacks.  Their drivers do.

I can add "(unless they are being executed for a different device)" or
something like this, but I'm not sure if that makes things any clearer.

Do you know any situation in which that matters?

> > - * There also are the following callbacks related to run-time power management
> > - * of devices:
> > + * There also are callbacks related to runtime power management of devices.
> > + * Again, these callbacks are executed by the PM core only for subsystems
> > + * (PM domains, device types, classes and bus types) and the subsystem-level
> > + * callbacks are supposed to invoke the driver callbacks.  Moreover, the exact
> > + * actions to be performed by a device driver's callbacks generally depend on
> > + * the platform and subsystem the device belongs to.
> >   *
> >   * @runtime_suspend: Prepare the device for a condition in which it won't be
> >   *	able to communicate with the CPU(s) and RAM due to power management.
> > - *	This need not mean that the device should be put into a low power state.
> > + *	This need not mean that the device should be put into a low-power state.
> >   *	For example, if the device is behind a link which is about to be turned
> >   *	off, the device may remain at full power.  If the device does go to low
> > - *	power and is capable of generating run-time wake-up events, remote
> > - *	wake-up (i.e., a hardware mechanism allowing the device to request a
> > - *	change of its power state via a wake-up event, such as PCI PME) should
> > - *	be enabled for it.
> > + *	power and is capable of generating runtime wakeup events, remote wakeup
> > + *	(i.e., a hardware mechanism allowing the device to request a change of
> > + *	its power state via an interrupt) should be enabled for it.
> >   *
> >   * @runtime_resume: Put the device into the fully active state in response to a
> > - *	wake-up event generated by hardware or at the request of software.  If
> > - *	necessary, put the device into the full power state and restore its
> > + *	wakeup event generated by hardware or at the request of software.  If
> > + *	necessary, put the device into the full-power state and restore its
> >   *	registers, so that it is fully operational.
> >   *
> > - * @runtime_idle: Device appears to be inactive and it might be put into a low
> > - *	power state if all of the necessary conditions are satisfied.  Check
> > + * @runtime_idle: Device appears to be inactive and it might be put into a
> > + *	low-power state if all of the necessary conditions are satisfied.  Check
> >   *	these conditions and handle the device as appropriate, possibly queueing
> >   *	a suspend request for it.  The return value is ignored by the PM core.
> >   */
> 
> Somewhere in here the text should tell people to find more information 
> in Documentation/power/runtime_pm.txt.  Likewise for .../devices.txt, 
> although that should be added at the appropriate spot higher up.

OK

Thanks,
Rafael

Re: [PATCH 5] PM: Update comments describing device power management callbacks

[Alan Stern]

On Mon, 21 Nov 2011, Rafael J. Wysocki wrote:

> > >   * @freeze: Hibernation-specific, executed before creating a hibernation image.
> > > - *	Quiesce operations so that a consistent image can be created, but do NOT
> > > - *	otherwise put the device into a low power device state and do NOT emit
> > > - *	system wakeup events.  Save in main memory the device settings to be
> > > - *	used by @restore() during the subsequent resume from hibernation or by
> > > - *	the subsequent @thaw(), if the creation of the image or the restoration
> > > - *	of main memory contents from it fails.
> > > + *	Analogous to @suspend(), but it should not enable the device to signal
> > > + *	wakeup events.  The majority of subsystems (with the notable exception
> > > + *	of the PCI bus type) expect the driver-level @freeze() to save the
> > > + *	device settings in memory to be used by @restore() during the subsequent
> > > + *	resume from hibernation.
> > > + *	Subsystem-level @freeze() is executed for all devices after invoking
> > > + *	subsystem-level @prepare() for all of them.
> > 
> > The first three lines you removed contain some important points which I
> > think should be retained.
> 
> Well, not really, because in fact what the callback is supposed to do depends
> on the subsystem.  For example, on PCI freeze is not supposed to save the
> the device state even and generally those routines don't emit any events.

But you removed the part saying that the freeze callback should quiesce
the device but doesn't necessarily have to put the device into a
low-power state (in fact, it should avoid changing the power state if
possible).  And you removed the explanation that this is needed in
order to guarantee a consistent memory image.  These two points are
true for all subsystems, including PCI.


> > > @@ -174,29 +198,32 @@ typedef struct pm_message {
> > >   * their children.
> > >   *
> > >   * It is allowed to unregister devices while the above callbacks are being
> > > - * executed.  However, it is not allowed to unregister a device from within any
> > > - * of its own callbacks.
> > > + * executed.  However, it is NOT allowed to unregister a device from within any
> > > + * of its driver's callbacks.
> > 
> > Please be a little more precise here.  If driver D manages two devices,
> > A and B, then D _is_ allowed to unregister A from within a callback for
> > B (except if A is an ancestor of B or something like that).  It's just
> > not allowed to unregister B from within a callback for B.
> 
> I can leave that as is, but devices (or device objects to be precise) don't
> have any callbacks.  Their drivers do.
> 
> I can add "(unless they are being executed for a different device)" or
> something like this, but I'm not sure if that makes things any clearer.

"A callback routine must NOT try to unregister the device for which it
was called, however it may unregister children of that device (for
example, if it detects that a child was hot-unplugged while the system
was asleep)."

> Do you know any situation in which that matters?

Not offhand, but I'm not familiar with too many subsystems.

Alan Stern

Re: [PATCH 5] PM: Update comments describing device power management callbacks

[Rafael J. Wysocki]

On Monday, November 21, 2011, Alan Stern wrote:
> On Mon, 21 Nov 2011, Rafael J. Wysocki wrote:
> 
> > > >   * @freeze: Hibernation-specific, executed before creating a hibernation image.
> > > > - *	Quiesce operations so that a consistent image can be created, but do NOT
> > > > - *	otherwise put the device into a low power device state and do NOT emit
> > > > - *	system wakeup events.  Save in main memory the device settings to be
> > > > - *	used by @restore() during the subsequent resume from hibernation or by
> > > > - *	the subsequent @thaw(), if the creation of the image or the restoration
> > > > - *	of main memory contents from it fails.
> > > > + *	Analogous to @suspend(), but it should not enable the device to signal
> > > > + *	wakeup events.  The majority of subsystems (with the notable exception
> > > > + *	of the PCI bus type) expect the driver-level @freeze() to save the
> > > > + *	device settings in memory to be used by @restore() during the subsequent
> > > > + *	resume from hibernation.
> > > > + *	Subsystem-level @freeze() is executed for all devices after invoking
> > > > + *	subsystem-level @prepare() for all of them.
> > > 
> > > The first three lines you removed contain some important points which I
> > > think should be retained.
> > 
> > Well, not really, because in fact what the callback is supposed to do depends
> > on the subsystem.  For example, on PCI freeze is not supposed to save the
> > the device state even and generally those routines don't emit any events.
> 
> But you removed the part saying that the freeze callback should quiesce
> the device but doesn't necessarily have to put the device into a
> low-power state (in fact, it should avoid changing the power state if
> possible).  And you removed the explanation that this is needed in
> order to guarantee a consistent memory image.  These two points are
> true for all subsystems, including PCI.

I said "Analogous to @suspend()" instead.  I'm not sure why this is not
sufficient?

> > > > @@ -174,29 +198,32 @@ typedef struct pm_message {
> > > >   * their children.
> > > >   *
> > > >   * It is allowed to unregister devices while the above callbacks are being
> > > > - * executed.  However, it is not allowed to unregister a device from within any
> > > > - * of its own callbacks.
> > > > + * executed.  However, it is NOT allowed to unregister a device from within any
> > > > + * of its driver's callbacks.
> > > 
> > > Please be a little more precise here.  If driver D manages two devices,
> > > A and B, then D _is_ allowed to unregister A from within a callback for
> > > B (except if A is an ancestor of B or something like that).  It's just
> > > not allowed to unregister B from within a callback for B.
> > 
> > I can leave that as is, but devices (or device objects to be precise) don't
> > have any callbacks.  Their drivers do.
> > 
> > I can add "(unless they are being executed for a different device)" or
> > something like this, but I'm not sure if that makes things any clearer.
> 
> "A callback routine must NOT try to unregister the device for which it
> was called, however it may unregister children of that device (for
> example, if it detects that a child was hot-unplugged while the system
> was asleep)."

That sounds good, thanks!
 
> > Do you know any situation in which that matters?
> 
> Not offhand, but I'm not familiar with too many subsystems.

Well, it seems quite extreme to me to be honest. :-)

Thanks,
Rafael

Re: [PATCH 5] PM: Update comments describing device power management callbacks

[Alan Stern]

On Mon, 21 Nov 2011, Rafael J. Wysocki wrote:

> On Monday, November 21, 2011, Alan Stern wrote:
> > On Mon, 21 Nov 2011, Rafael J. Wysocki wrote:
> > 
> > > > >   * @freeze: Hibernation-specific, executed before creating a hibernation image.
> > > > > - *	Quiesce operations so that a consistent image can be created, but do NOT
> > > > > - *	otherwise put the device into a low power device state and do NOT emit
> > > > > - *	system wakeup events.  Save in main memory the device settings to be
> > > > > - *	used by @restore() during the subsequent resume from hibernation or by
> > > > > - *	the subsequent @thaw(), if the creation of the image or the restoration
> > > > > - *	of main memory contents from it fails.
> > > > > + *	Analogous to @suspend(), but it should not enable the device to signal
> > > > > + *	wakeup events.  The majority of subsystems (with the notable exception
> > > > > + *	of the PCI bus type) expect the driver-level @freeze() to save the
> > > > > + *	device settings in memory to be used by @restore() during the subsequent
> > > > > + *	resume from hibernation.
> > > > > + *	Subsystem-level @freeze() is executed for all devices after invoking
> > > > > + *	subsystem-level @prepare() for all of them.
> > > > 
> > > > The first three lines you removed contain some important points which I
> > > > think should be retained.
> > > 
> > > Well, not really, because in fact what the callback is supposed to do depends
> > > on the subsystem.  For example, on PCI freeze is not supposed to save the
> > > the device state even and generally those routines don't emit any events.
> > 
> > But you removed the part saying that the freeze callback should quiesce
> > the device but doesn't necessarily have to put the device into a
> > low-power state (in fact, it should avoid changing the power state if
> > possible).  And you removed the explanation that this is needed in
> > order to guarantee a consistent memory image.  These two points are
> > true for all subsystems, including PCI.
> 
> I said "Analogous to @suspend()" instead.  I'm not sure why this is not
> sufficient?

Because @suspend() is very different!  Its description basically says 
to do three things:

	Quiesce the device,

	Put it into a low-power state,

	And enable wakeup events.

@freeze() is supposed to do the first but not the second or third.  
This makes it only 33% similar to @suspend().  :-)

Also, the description of @suspend() says nothing about having a
consistent memory image.


> > "A callback routine must NOT try to unregister the device for which it
> > was called, however it may unregister children of that device (for
> > example, if it detects that a child was hot-unplugged while the system
> > was asleep)."
> 
> That sounds good, thanks!
>  
> > > Do you know any situation in which that matters?
> > 
> > Not offhand, but I'm not familiar with too many subsystems.
> 
> Well, it seems quite extreme to me to be honest. :-)

It would matter for the USB subsystem, except that USB registers and
unregisters all its devices from a freezable kernel thread.  It's not
hard to imagine that other subsystems might want to unregister devices
as soon as they are found to be missing, which means during the
parent's resume routine.

Alan Stern

Re: [PATCH 5] PM: Update comments describing device power management callbacks

[Rafael J. Wysocki]

On Monday, November 21, 2011, Alan Stern wrote:
> On Mon, 21 Nov 2011, Rafael J. Wysocki wrote:
> 
> > On Monday, November 21, 2011, Alan Stern wrote:
> > > On Mon, 21 Nov 2011, Rafael J. Wysocki wrote:
> > > 
> > > > > >   * @freeze: Hibernation-specific, executed before creating a hibernation image.
> > > > > > - *	Quiesce operations so that a consistent image can be created, but do NOT
> > > > > > - *	otherwise put the device into a low power device state and do NOT emit
> > > > > > - *	system wakeup events.  Save in main memory the device settings to be
> > > > > > - *	used by @restore() during the subsequent resume from hibernation or by
> > > > > > - *	the subsequent @thaw(), if the creation of the image or the restoration
> > > > > > - *	of main memory contents from it fails.
> > > > > > + *	Analogous to @suspend(), but it should not enable the device to signal
> > > > > > + *	wakeup events.  The majority of subsystems (with the notable exception
> > > > > > + *	of the PCI bus type) expect the driver-level @freeze() to save the
> > > > > > + *	device settings in memory to be used by @restore() during the subsequent
> > > > > > + *	resume from hibernation.
> > > > > > + *	Subsystem-level @freeze() is executed for all devices after invoking
> > > > > > + *	subsystem-level @prepare() for all of them.
> > > > > 
> > > > > The first three lines you removed contain some important points which I
> > > > > think should be retained.
> > > > 
> > > > Well, not really, because in fact what the callback is supposed to do depends
> > > > on the subsystem.  For example, on PCI freeze is not supposed to save the
> > > > the device state even and generally those routines don't emit any events.
> > > 
> > > But you removed the part saying that the freeze callback should quiesce
> > > the device but doesn't necessarily have to put the device into a
> > > low-power state (in fact, it should avoid changing the power state if
> > > possible).  And you removed the explanation that this is needed in
> > > order to guarantee a consistent memory image.  These two points are
> > > true for all subsystems, including PCI.
> > 
> > I said "Analogous to @suspend()" instead.  I'm not sure why this is not
> > sufficient?
> 
> Because @suspend() is very different!  Its description basically says 
> to do three things:
> 
> 	Quiesce the device,
> 
> 	Put it into a low-power state,
> 
> 	And enable wakeup events.

No, it doesn't any more.  It's being changed by the proposed patch too. :-)

> @freeze() is supposed to do the first but not the second or third.  
> This makes it only 33% similar to @suspend().  :-)
> 
> Also, the description of @suspend() says nothing about having a
> consistent memory image.

Because that is irrelevant.  The state of the device after the resume
has to be consistent, regardless of whether the resume is from RAM or
from an on-disk image.

> > > "A callback routine must NOT try to unregister the device for which it
> > > was called, however it may unregister children of that device (for
> > > example, if it detects that a child was hot-unplugged while the system
> > > was asleep)."
> > 
> > That sounds good, thanks!
> >  
> > > > Do you know any situation in which that matters?
> > > 
> > > Not offhand, but I'm not familiar with too many subsystems.
> > 
> > Well, it seems quite extreme to me to be honest. :-)
> 
> It would matter for the USB subsystem, except that USB registers and
> unregisters all its devices from a freezable kernel thread.  It's not
> hard to imagine that other subsystems might want to unregister devices
> as soon as they are found to be missing, which means during the
> parent's resume routine.

I see.

Thanks,
Rafael

Re: [PATCH 5] PM: Update comments describing device power management callbacks

[Alan Stern]

On Mon, 21 Nov 2011, Rafael J. Wysocki wrote:

> > > I said "Analogous to @suspend()" instead.  I'm not sure why this is not
> > > sufficient?
> > 
> > Because @suspend() is very different!  Its description basically says 
> > to do three things:
> > 
> > 	Quiesce the device,
> > 
> > 	Put it into a low-power state,
> > 
> > 	And enable wakeup events.
> 
> No, it doesn't any more.  It's being changed by the proposed patch too. :-)

I must have missed reading that part.  Okay...  but it seems weird that
none of the new descriptions says anything about changing the power
state.  Shouldn't the description of @suspend say something like "For
many platforms and subsystems, the device should be put in a low-power
state"?

> > @freeze() is supposed to do the first but not the second or third.  
> > This makes it only 33% similar to @suspend().  :-)
> > 
> > Also, the description of @suspend() says nothing about having a
> > consistent memory image.
> 
> Because that is irrelevant.  The state of the device after the resume
> has to be consistent, regardless of whether the resume is from RAM or
> from an on-disk image.

Sure, the device's state will be consistent.  But will the contents of
memory image be consistent?  Not if the device was doing DMA writes
during the time when the image was created.

Alan Stern

Re: [PATCH 5] PM: Update comments describing device power management callbacks

[Rafael J. Wysocki]

On Monday, November 21, 2011, Alan Stern wrote:
> On Mon, 21 Nov 2011, Rafael J. Wysocki wrote:
> 
> > > > I said "Analogous to @suspend()" instead.  I'm not sure why this is not
> > > > sufficient?
> > > 
> > > Because @suspend() is very different!  Its description basically says 
> > > to do three things:
> > > 
> > > 	Quiesce the device,
> > > 
> > > 	Put it into a low-power state,
> > > 
> > > 	And enable wakeup events.
> > 
> > No, it doesn't any more.  It's being changed by the proposed patch too. :-)
> 
> I must have missed reading that part.  Okay...  but it seems weird that
> none of the new descriptions says anything about changing the power
> state.  Shouldn't the description of @suspend say something like "For
> many platforms and subsystems, the device should be put in a low-power
> state"?

Hmm.  I'm not really sure, actually, because I'd recommend that subsystems
rather than drivers change power states of devices and this description
is targeted at driver writers mostly.

> > > @freeze() is supposed to do the first but not the second or third.  
> > > This makes it only 33% similar to @suspend().  :-)
> > > 
> > > Also, the description of @suspend() says nothing about having a
> > > consistent memory image.
> > 
> > Because that is irrelevant.  The state of the device after the resume
> > has to be consistent, regardless of whether the resume is from RAM or
> > from an on-disk image.
> 
> Sure, the device's state will be consistent.  But will the contents of
> memory image be consistent?  Not if the device was doing DMA writes
> during the time when the image was created.

Well, since .suspend() is also expected to stop DMA, that's rather moot.

Thanks,
Rafael

Re: [PATCH 5] PM: Update comments describing device power management callbacks

[Ming Lei]

Hi,

On Mon, Nov 21, 2011 at 7:38 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> From: Rafael J. Wysocki <rjw@sisk.pl>

>  * @prepare: Prepare the device for the upcoming transition, but do NOT change
>  *     its hardware state.  Prevent new children of the device from being
> @@ -71,101 +78,118 @@ typedef struct pm_message {
>  *     probe method from being made too once @prepare() has succeeded).  If
>  *     @prepare() detects a situation it cannot handle (e.g. registration of a
>  *     child already in progress), it may return -EAGAIN, so that the PM core
> - *     can execute it once again (e.g. after the new child has been registered)
> + *     can execute it once again (e.g. after a new child has been registered)
>  *     to recover from the race condition.  This method is executed for all
>  *     kinds of suspend transitions and is followed by one of the suspend
>  *     callbacks: @suspend(), @freeze(), or @poweroff().
> - *     The PM core executes @prepare() for all devices before starting to
> - *     execute suspend callbacks for any of them, so drivers may assume all of
> - *     the other devices to be present and functional while @prepare() is being
> - *     executed.  In particular, it is safe to make GFP_KERNEL memory
> - *     allocations from within @prepare().  However, drivers may NOT assume
> - *     anything about the availability of the user space at that time and it
> - *     is not correct to request firmware from within @prepare() (it's too
> - *     late to do that).  [To work around this limitation, drivers may
> - *     register suspend and hibernation notifiers that are executed before the
> + *     The PM core executes subsystem-level @prepare() for all devices before
> + *     starting to execute suspend callbacks for any of them, so all devices
> + *     may be assumed to be present and functional while @prepare() is being

Devices aren't functional in runtime suspend state, so maybe the word of
'functional' should be removed.

> + *     executed.  However, device drivers may NOT assume anything about the
> + *     availability of user space at that time and it is NOT valid to request
> + *     firmware from within @prepare() (it's too late to do that).  It also is
> + *     NOT valid to allocate substantial amounts of memory from @prepare() in
> + *     the GFP_KERNEL mode.  [To work around these limitations, drivers may
> + *     register suspend and hibernation notifiers to be executed before the
>  *     freezing of tasks.]


>  * @resume: Executed after waking the system up from a sleep state in which the
> - *     contents of main memory were preserved.  Put the device into the
> - *     appropriate state, according to the information saved in memory by the
> - *     preceding @suspend().  The driver starts working again, responding to
> - *     hardware events and software requests.  The hardware may have gone
> - *     through a power-off reset, or it may have maintained state from the
> - *     previous suspend() which the driver may rely on while resuming.  On most
> - *     platforms, there are no restrictions on availability of resources like
> - *     clocks during @resume().
> + *     contents of main memory were preserved.  Undo the changes made by
> + *     the preceding @suspend() and cause the device to become operational

The device may still not be operational if it was runtime suspended
before running
@suspend().

> + *     (the exact action to perform depends on the device's subsystem).
> + *     The driver starts working again, responding to hardware events and
> + *     software requests.  The state of the device at the time its driver's
> + *     @resume() callback is run depends on the platform and subsystem the
> + *     device belongs to.  On most platforms, there are no restrictions on
> + *     availability of resources like clocks during @resume().
> + *     Subsystem-level @resume() is executed for all devices after invoking
> + *     subsystem-level @resume_noirq() for all of them.


thanks,
-- 
Ming Lei

Re: [PATCH 5] PM: Update comments describing device power management callbacks

[Alan Stern]

On Mon, 21 Nov 2011, Rafael J. Wysocki wrote:

> On Monday, November 21, 2011, Alan Stern wrote:
> > On Mon, 21 Nov 2011, Rafael J. Wysocki wrote:
> > 
> > > > > I said "Analogous to @suspend()" instead.  I'm not sure why this is not
> > > > > sufficient?
> > > > 
> > > > Because @suspend() is very different!  Its description basically says 
> > > > to do three things:
> > > > 
> > > > 	Quiesce the device,
> > > > 
> > > > 	Put it into a low-power state,
> > > > 
> > > > 	And enable wakeup events.
> > > 
> > > No, it doesn't any more.  It's being changed by the proposed patch too. :-)
> > 
> > I must have missed reading that part.  Okay...  but it seems weird that
> > none of the new descriptions says anything about changing the power
> > state.  Shouldn't the description of @suspend say something like "For
> > many platforms and subsystems, the device should be put in a low-power
> > state"?
> 
> Hmm.  I'm not really sure, actually, because I'd recommend that subsystems
> rather than drivers change power states of devices and this description
> is targeted at driver writers mostly.

The same data structure (dev_pm_ops) is used for both drivers and
subsystems.  Therefore the comments should be directed toward both
driver writers and subsystem writers.

You could say: "For many platforms, the subsystem @suspend() or
@suspend_noirq() callback should put the device in a low-power state.  
Some subsystems may require the driver to do this instead."

> > > > @freeze() is supposed to do the first but not the second or third.  
> > > > This makes it only 33% similar to @suspend().  :-)
> > > > 
> > > > Also, the description of @suspend() says nothing about having a
> > > > consistent memory image.
> > > 
> > > Because that is irrelevant.  The state of the device after the resume
> > > has to be consistent, regardless of whether the resume is from RAM or
> > > from an on-disk image.
> > 
> > Sure, the device's state will be consistent.  But will the contents of
> > memory image be consistent?  Not if the device was doing DMA writes
> > during the time when the image was created.
> 
> Well, since .suspend() is also expected to stop DMA, that's rather moot.

It won't be moot if you add the sentences I recommend above.  You could
add an additional sentence: "Either way, the @freeze() and
@freeze_noirq() callbacks (both the subsystem's and the driver's)  
should always avoid changing the device's state."

Alan Stern

Re: [PATCH 5] PM: Update comments describing device power management callbacks

[Rafael J. Wysocki]

On Tuesday, November 22, 2011, Ming Lei wrote:
> Hi,
> 
> On Mon, Nov 21, 2011 at 7:38 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> > From: Rafael J. Wysocki <rjw@sisk.pl>
> 
> >  * @prepare: Prepare the device for the upcoming transition, but do NOT change
> >  *     its hardware state.  Prevent new children of the device from being
> > @@ -71,101 +78,118 @@ typedef struct pm_message {
> >  *     probe method from being made too once @prepare() has succeeded).  If
> >  *     @prepare() detects a situation it cannot handle (e.g. registration of a
> >  *     child already in progress), it may return -EAGAIN, so that the PM core
> > - *     can execute it once again (e.g. after the new child has been registered)
> > + *     can execute it once again (e.g. after a new child has been registered)
> >  *     to recover from the race condition.  This method is executed for all
> >  *     kinds of suspend transitions and is followed by one of the suspend
> >  *     callbacks: @suspend(), @freeze(), or @poweroff().
> > - *     The PM core executes @prepare() for all devices before starting to
> > - *     execute suspend callbacks for any of them, so drivers may assume all of
> > - *     the other devices to be present and functional while @prepare() is being
> > - *     executed.  In particular, it is safe to make GFP_KERNEL memory
> > - *     allocations from within @prepare().  However, drivers may NOT assume
> > - *     anything about the availability of the user space at that time and it
> > - *     is not correct to request firmware from within @prepare() (it's too
> > - *     late to do that).  [To work around this limitation, drivers may
> > - *     register suspend and hibernation notifiers that are executed before the
> > + *     The PM core executes subsystem-level @prepare() for all devices before
> > + *     starting to execute suspend callbacks for any of them, so all devices
> > + *     may be assumed to be present and functional while @prepare() is being
> 
> Devices aren't functional in runtime suspend state, so maybe the word of
> 'functional' should be removed.

That's kind of complicated, see below.

> > + *     executed.  However, device drivers may NOT assume anything about the
> > + *     availability of user space at that time and it is NOT valid to request
> > + *     firmware from within @prepare() (it's too late to do that).  It also is
> > + *     NOT valid to allocate substantial amounts of memory from @prepare() in
> > + *     the GFP_KERNEL mode.  [To work around these limitations, drivers may
> > + *     register suspend and hibernation notifiers to be executed before the
> >  *     freezing of tasks.]
> 
> 
> >  * @resume: Executed after waking the system up from a sleep state in which the
> > - *     contents of main memory were preserved.  Put the device into the
> > - *     appropriate state, according to the information saved in memory by the
> > - *     preceding @suspend().  The driver starts working again, responding to
> > - *     hardware events and software requests.  The hardware may have gone
> > - *     through a power-off reset, or it may have maintained state from the
> > - *     previous suspend() which the driver may rely on while resuming.  On most
> > - *     platforms, there are no restrictions on availability of resources like
> > - *     clocks during @resume().
> > + *     contents of main memory were preserved.  Undo the changes made by
> > + *     the preceding @suspend() and cause the device to become operational
> 
> The device may still not be operational if it was runtime suspended
> before running @suspend().

That's correct, but at the same time it's not 100% clear what @resume should
do with devices that have been runtime-suspended before system suspend.

For example, it may depend on what power configuration the device is in
(it may be a member of a power domain that was off before the system suspend or
something like this).

I'm starting to think that it might be better to simply remove those comments
altogether. :-)

Thanks,
Rafael

Re: [PATCH 5] PM: Update comments describing device power management callbacks

[Rafael J. Wysocki]

On Tuesday, November 22, 2011, Alan Stern wrote:
> On Mon, 21 Nov 2011, Rafael J. Wysocki wrote:
> 
> > On Monday, November 21, 2011, Alan Stern wrote:
> > > On Mon, 21 Nov 2011, Rafael J. Wysocki wrote:
> > > 
> > > > > > I said "Analogous to @suspend()" instead.  I'm not sure why this is not
> > > > > > sufficient?
> > > > > 
> > > > > Because @suspend() is very different!  Its description basically says 
> > > > > to do three things:
> > > > > 
> > > > > 	Quiesce the device,
> > > > > 
> > > > > 	Put it into a low-power state,
> > > > > 
> > > > > 	And enable wakeup events.
> > > > 
> > > > No, it doesn't any more.  It's being changed by the proposed patch too. :-)
> > > 
> > > I must have missed reading that part.  Okay...  but it seems weird that
> > > none of the new descriptions says anything about changing the power
> > > state.  Shouldn't the description of @suspend say something like "For
> > > many platforms and subsystems, the device should be put in a low-power
> > > state"?
> > 
> > Hmm.  I'm not really sure, actually, because I'd recommend that subsystems
> > rather than drivers change power states of devices and this description
> > is targeted at driver writers mostly.
> 
> The same data structure (dev_pm_ops) is used for both drivers and
> subsystems.  Therefore the comments should be directed toward both
> driver writers and subsystem writers.

I don't quite agree.  Trying to do so would turn that comment into a lengthy
document and there's no room for anything like that in the header file.

Perhaps I'll just modify the comment to say when the callbacks are executed
without specifying what the specific callbacks are supposed to do (because
that may vary from one subsystem to another and even between different
drivers belonging to the same subsystem).  If PM domains are taken into
account, it gets even more complicated.

> You could say: "For many platforms, the subsystem @suspend() or
> @suspend_noirq() callback should put the device in a low-power state.  
> Some subsystems may require the driver to do this instead."

Well, that information is not really useful for someone wanting to learn
what the callbacks are supposed to do, because he still has to know what
the subsystem in question expects him to do.

> > > > > @freeze() is supposed to do the first but not the second or third.  
> > > > > This makes it only 33% similar to @suspend().  :-)
> > > > > 
> > > > > Also, the description of @suspend() says nothing about having a
> > > > > consistent memory image.
> > > > 
> > > > Because that is irrelevant.  The state of the device after the resume
> > > > has to be consistent, regardless of whether the resume is from RAM or
> > > > from an on-disk image.
> > > 
> > > Sure, the device's state will be consistent.  But will the contents of
> > > memory image be consistent?  Not if the device was doing DMA writes
> > > during the time when the image was created.
> > 
> > Well, since .suspend() is also expected to stop DMA, that's rather moot.
> 
> It won't be moot if you add the sentences I recommend above.  You could
> add an additional sentence: "Either way, the @freeze() and
> @freeze_noirq() callbacks (both the subsystem's and the driver's)  
> should always avoid changing the device's state."

That sounds good.

Thanks,
Rafael

Re: [PATCH 5] PM: Update comments describing device power management callbacks

[Alan Stern]

On Tue, 22 Nov 2011, Rafael J. Wysocki wrote:

> > The same data structure (dev_pm_ops) is used for both drivers and
> > subsystems.  Therefore the comments should be directed toward both
> > driver writers and subsystem writers.
> 
> I don't quite agree.  Trying to do so would turn that comment into a lengthy
> document and there's no room for anything like that in the header file.

That's a good point.

> Perhaps I'll just modify the comment to say when the callbacks are executed
> without specifying what the specific callbacks are supposed to do (because
> that may vary from one subsystem to another and even between different
> drivers belonging to the same subsystem).  If PM domains are taken into
> account, it gets even more complicated.

Yes, a minimal description would be good, along with a pointer to the 
Documentation/power files.

> > It won't be moot if you add the sentences I recommend above.  You could
> > add an additional sentence: "Either way, the @freeze() and
> > @freeze_noirq() callbacks (both the subsystem's and the driver's)  
> > should always avoid changing the device's state."
> 
> That sounds good.

Actually I meant to write "avoid changing the device's power state".

Alan Stern

Re: [PATCH 5] PM: Update comments describing device power management callbacks

[Rafael J. Wysocki]

On Tuesday, November 22, 2011, Alan Stern wrote:
> On Tue, 22 Nov 2011, Rafael J. Wysocki wrote:
> 
> > > The same data structure (dev_pm_ops) is used for both drivers and
> > > subsystems.  Therefore the comments should be directed toward both
> > > driver writers and subsystem writers.
> > 
> > I don't quite agree.  Trying to do so would turn that comment into a lengthy
> > document and there's no room for anything like that in the header file.
> 
> That's a good point.
> 
> > Perhaps I'll just modify the comment to say when the callbacks are executed
> > without specifying what the specific callbacks are supposed to do (because
> > that may vary from one subsystem to another and even between different
> > drivers belonging to the same subsystem).  If PM domains are taken into
> > account, it gets even more complicated.
> 
> Yes, a minimal description would be good, along with a pointer to the 
> Documentation/power files.

Well, the one below is not exactly minimal, but I think it doesn't go too
far at least.

Thanks,
Rafael

---
From: Rafael J. Wysocki <rjw@sisk.pl>
Subject: PM: Update comments describing device power management callbacks (v2)

The comments describing device power management callbacks in
include/pm.h are outdated and somewhat confusing, so make them
reflect the reality more accurately.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 include/linux/pm.h |  231 ++++++++++++++++++++++++++++++-----------------------
 1 file changed, 135 insertions(+), 96 deletions(-)

Index: linux/include/linux/pm.h
===================================================================
--- linux.orig/include/linux/pm.h
+++ linux/include/linux/pm.h
@@ -54,118 +54,145 @@ typedef struct pm_message {
 /**
  * struct dev_pm_ops - device PM callbacks
  *
- * Several driver power state transitions are externally visible, affecting
+ * Several device power state transitions are externally visible, affecting
  * the state of pending I/O queues and (for drivers that touch hardware)
  * interrupts, wakeups, DMA, and other hardware state.  There may also be
- * internal transitions to various low power modes, which are transparent
+ * internal transitions to various low-power modes which are transparent
  * to the rest of the driver stack (such as a driver that's ON gating off
  * clocks which are not in active use).
  *
- * The externally visible transitions are handled with the help of the following
- * callbacks included in this structure:
- *
- * @prepare: Prepare the device for the upcoming transition, but do NOT change
- *	its hardware state.  Prevent new children of the device from being
- *	registered after @prepare() returns (the driver's subsystem and
- *	generally the rest of the kernel is supposed to prevent new calls to the
- *	probe method from being made too once @prepare() has succeeded).  If
- *	@prepare() detects a situation it cannot handle (e.g. registration of a
- *	child already in progress), it may return -EAGAIN, so that the PM core
- *	can execute it once again (e.g. after the new child has been registered)
- *	to recover from the race condition.  This method is executed for all
- *	kinds of suspend transitions and is followed by one of the suspend
- *	callbacks: @suspend(), @freeze(), or @poweroff().
- *	The PM core executes @prepare() for all devices before starting to
- *	execute suspend callbacks for any of them, so drivers may assume all of
- *	the other devices to be present and functional while @prepare() is being
- *	executed.  In particular, it is safe to make GFP_KERNEL memory
- *	allocations from within @prepare().  However, drivers may NOT assume
- *	anything about the availability of the user space at that time and it
- *	is not correct to request firmware from within @prepare() (it's too
- *	late to do that).  [To work around this limitation, drivers may
- *	register suspend and hibernation notifiers that are executed before the
- *	freezing of tasks.]
+ * The externally visible transitions are handled with the help of callbacks
+ * included in this structure in such a way that two levels of callbacks are
+ * involved.  First, the PM core executes callbacks provided by PM domains,
+ * device types, classes and bus types.  They are the subsystem-level callbacks
+ * supposed to execute callbacks provided by device drivers, although they may
+ * choose not to do that.  If the driver callbacks are executed, they have to
+ * collaborate with the subsystem-level callbacks to achieve the goals
+ * appropriate for the given system transition, given transition phase and the
+ * subsystem the device belongs to.
+ *
+ * @prepare: The principal role of this callback is to prevent new children of
+ *	the device from being registered after it has returned (the driver's
+ *	subsystem and generally the rest of the kernel is supposed to prevent
+ *	new calls to the probe method from being made too once @prepare() has
+ *	succeeded).  If @prepare() detects a situation it cannot handle (e.g.
+ *	registration of a child already in progress), it may return -EAGAIN, so
+ *	that the PM core can execute it once again (e.g. after a new child has
+ *	been registered) to recover from the race condition.
+ *	This method is executed for all kinds of suspend transitions and is
+ *	followed by one of the suspend callbacks: @suspend(), @freeze(), or
+ *	@poweroff().  The PM core executes subsystem-level @prepare() for all
+ *	devices before starting to invoke suspend callbacks for any of them, so
+ *	generally devices may be assumed to be functional or to respond to
+ *	runtime resume requests while @prepare() is being executed.  However,
+ *	device drivers may NOT assume anything about the availability of user
+ *	space at that time and it is NOT valid to request firmware from within
+ *	@prepare() (it's too late to do that).  It also is NOT valid to allocate
+ *	substantial amounts of memory from @prepare() in the GFP_KERNEL mode.
+ *	[To work around these limitations, drivers may register suspend and
+ *	hibernation notifiers to be executed before the freezing of tasks.]
  *
  * @complete: Undo the changes made by @prepare().  This method is executed for
  *	all kinds of resume transitions, following one of the resume callbacks:
  *	@resume(), @thaw(), @restore().  Also called if the state transition
- *	fails before the driver's suspend callback (@suspend(), @freeze(),
- *	@poweroff()) can be executed (e.g. if the suspend callback fails for one
+ *	fails before the driver's suspend callback: @suspend(), @freeze() or
+ *	@poweroff(), can be executed (e.g. if the suspend callback fails for one
  *	of the other devices that the PM core has unsuccessfully attempted to
  *	suspend earlier).
- *	The PM core executes @complete() after it has executed the appropriate
- *	resume callback for all devices.
+ *	The PM core executes subsystem-level @complete() after it has executed
+ *	the appropriate resume callbacks for all devices.
  *
  * @suspend: Executed before putting the system into a sleep state in which the
- *	contents of main memory are preserved.  Quiesce the device, put it into
- *	a low power state appropriate for the upcoming system state (such as
- *	PCI_D3hot), and enable wakeup events as appropriate.
+ *	contents of main memory are preserved.  The exact action to perform
+ *	depends on the device's subsystem (PM domain, device type, class or bus
+ *	type), but generally the device must be quiescent after subsystem-level
+ *	@suspend() has returned, so that it doesn't do any I/O or DMA.
+ *	Subsystem-level @suspend() is executed for all devices after invoking
+ *	subsystem-level @prepare() for all of them.
  *
  * @resume: Executed after waking the system up from a sleep state in which the
- *	contents of main memory were preserved.  Put the device into the
- *	appropriate state, according to the information saved in memory by the
- *	preceding @suspend().  The driver starts working again, responding to
- *	hardware events and software requests.  The hardware may have gone
- *	through a power-off reset, or it may have maintained state from the
- *	previous suspend() which the driver may rely on while resuming.  On most
- *	platforms, there are no restrictions on availability of resources like
- *	clocks during @resume().
+ *	contents of main memory were preserved.  The exact action to perform
+ *	depends on the device's subsystem, but generally the driver is expected
+ *	to start working again, responding to hardware events and software
+ *	requests (the device itself may be left in a low-power state, waiting
+ *	for a runtime resume to occur).  The state of the device at the time its
+ *	driver's @resume() callback is run depends on the platform and subsystem
+ *	the device belongs to.  On most platforms, there are no restrictions on
+ *	availability of resources like clocks during @resume().
+ *	Subsystem-level @resume() is executed for all devices after invoking
+ *	subsystem-level @resume_noirq() for all of them.
  *
  * @freeze: Hibernation-specific, executed before creating a hibernation image.
- *	Quiesce operations so that a consistent image can be created, but do NOT
- *	otherwise put the device into a low power device state and do NOT emit
- *	system wakeup events.  Save in main memory the device settings to be
- *	used by @restore() during the subsequent resume from hibernation or by
- *	the subsequent @thaw(), if the creation of the image or the restoration
- *	of main memory contents from it fails.
+ *	Analogous to @suspend(), but it should not enable the device to signal
+ *	wakeup events or change its power state.  The majority of subsystems
+ *	(with the notable exception of the PCI bus type) expect the driver-level
+ *	@freeze() to save the device settings in memory to be used by @restore()
+ *	during the subsequent resume from hibernation.
+ *	Subsystem-level @freeze() is executed for all devices after invoking
+ *	subsystem-level @prepare() for all of them.
  *
  * @thaw: Hibernation-specific, executed after creating a hibernation image OR
- *	if the creation of the image fails.  Also executed after a failing
+ *	if the creation of an image has failed.  Also executed after a failing
  *	attempt to restore the contents of main memory from such an image.
  *	Undo the changes made by the preceding @freeze(), so the device can be
  *	operated in the same way as immediately before the call to @freeze().
+ *	Subsystem-level @thaw() is executed for all devices after invoking
+ *	subsystem-level @thaw_noirq() for all of them.  It also may be executed
+ *	directly after @freeze() in case of a transition error.
  *
  * @poweroff: Hibernation-specific, executed after saving a hibernation image.
- *	Quiesce the device, put it into a low power state appropriate for the
- *	upcoming system state (such as PCI_D3hot), and enable wakeup events as
- *	appropriate.
+ *	Analogous to @suspend(), but it need not save the device's settings in
+ *	memory.
+ *	Subsystem-level @poweroff() is executed for all devices after invoking
+ *	subsystem-level @prepare() for all of them.
  *
  * @restore: Hibernation-specific, executed after restoring the contents of main
- *	memory from a hibernation image.  Driver starts working again,
- *	responding to hardware events and software requests.  Drivers may NOT
- *	make ANY assumptions about the hardware state right prior to @restore().
- *	On most platforms, there are no restrictions on availability of
- *	resources like clocks during @restore().
- *
- * @suspend_noirq: Complete the operations of ->suspend() by carrying out any
- *	actions required for suspending the device that need interrupts to be
- *	disabled
- *
- * @resume_noirq: Prepare for the execution of ->resume() by carrying out any
- *	actions required for resuming the device that need interrupts to be
- *	disabled
- *
- * @freeze_noirq: Complete the operations of ->freeze() by carrying out any
- *	actions required for freezing the device that need interrupts to be
- *	disabled
- *
- * @thaw_noirq: Prepare for the execution of ->thaw() by carrying out any
- *	actions required for thawing the device that need interrupts to be
- *	disabled
- *
- * @poweroff_noirq: Complete the operations of ->poweroff() by carrying out any
- *	actions required for handling the device that need interrupts to be
- *	disabled
- *
- * @restore_noirq: Prepare for the execution of ->restore() by carrying out any
- *	actions required for restoring the operations of the device that need
- *	interrupts to be disabled
+ *	memory from a hibernation image, analogous to @resume().
+ *
+ * @suspend_noirq: Complete the actions started by @suspend().  Carry out any
+ *	additional operations required for suspending the device that might be
+ *	racing with its driver's interrupt handler, which is guaranteed not to
+ *	run while @suspend_noirq() is being executed.
+ *	It generally is expected that the device will be in a low-power state
+ *	(appropriate for the target system sleep state) after subsystem-level
+ *	@suspend_noirq() has returned successfully.  If the device can generate
+ *	system wakeup signals and is enabled to wake up the system, it should be
+ *	configured to do so at that time.  However, depending on the platform
+ *	and device's subsystem, @suspend() may be allowed to put the device into
+ *	the low-power state and configure it to generate wakeup signals, in
+ *	which case it generally is not necessary to define @suspend_noirq().
+ *
+ * @resume_noirq: Prepare for the execution of @resume() by carrying out any
+ *	operations required for resuming the device that might be racing with
+ *	its driver's interrupt handler, which is guaranteed not to run while
+ *	@resume_noirq() is being executed.
+ *
+ * @freeze_noirq: Complete the actions started by @freeze().  Carry out any
+ *	additional operations required for freezing the device that might be
+ *	racing with its driver's interrupt handler, which is guaranteed not to
+ *	run while @freeze_noirq() is being executed.
+ *	The power state of the device should not be changed by either @freeze()
+ *	or @freeze_noirq() and it should not be configured to signal system
+ *	wakeup by any of these callbacks.
+ *
+ * @thaw_noirq: Prepare for the execution of @thaw() by carrying out any
+ *	operations required for thawing the device that might be racing with its
+ *	driver's interrupt handler, which is guaranteed not to run while
+ *	@thaw_noirq() is being executed.
+ *
+ * @poweroff_noirq: Complete the actions started by @poweroff().  Analogous to
+ *	@suspend_noirq(), but it need not save the device's settings in memory.
+ *
+ * @restore_noirq: Prepare for the execution of @restore() by carrying out any
+ *	operations required for thawing the device that might be racing with its
+ *	driver's interrupt handler, which is guaranteed not to run while
+ *	@restore_noirq() is being executed.  Analogous to @resume_noirq().
  *
  * All of the above callbacks, except for @complete(), return error codes.
  * However, the error codes returned by the resume operations, @resume(),
- * @thaw(), @restore(), @resume_noirq(), @thaw_noirq(), and @restore_noirq() do
+ * @thaw(), @restore(), @resume_noirq(), @thaw_noirq(), and @restore_noirq(), do
  * not cause the PM core to abort the resume transition during which they are
- * returned.  The error codes returned in that cases are only printed by the PM
+ * returned.  The error codes returned in those cases are only printed by the PM
  * core to the system logs for debugging purposes.  Still, it is recommended
  * that drivers only return error codes from their resume methods in case of an
  * unrecoverable failure (i.e. when the device being handled refuses to resume
@@ -174,31 +201,43 @@ typedef struct pm_message {
  * their children.
  *
  * It is allowed to unregister devices while the above callbacks are being
- * executed.  However, it is not allowed to unregister a device from within any
- * of its own callbacks.
- *
- * There also are the following callbacks related to run-time power management
- * of devices:
+ * executed.  However, a callback routine must NOT try to unregister the device
+ * it was called for, although it may unregister children of that device (for
+ * example, if it detects that a child was unplugged while the system was
+ * asleep).
+ *
+ * Refer to Documentation/power/devices.txt for more information about the role
+ * of the above callbacks in the system suspend process.
+ *
+ * There also are callbacks related to runtime power management of devices.
+ * Again, these callbacks are executed by the PM core only for subsystems
+ * (PM domains, device types, classes and bus types) and the subsystem-level
+ * callbacks are supposed to invoke the driver callbacks.  Moreover, the exact
+ * actions to be performed by a device driver's callbacks generally depend on
+ * the platform and subsystem the device belongs to.
  *
  * @runtime_suspend: Prepare the device for a condition in which it won't be
  *	able to communicate with the CPU(s) and RAM due to power management.
- *	This need not mean that the device should be put into a low power state.
+ *	This need not mean that the device should be put into a low-power state.
  *	For example, if the device is behind a link which is about to be turned
  *	off, the device may remain at full power.  If the device does go to low
- *	power and is capable of generating run-time wake-up events, remote
- *	wake-up (i.e., a hardware mechanism allowing the device to request a
- *	change of its power state via a wake-up event, such as PCI PME) should
- *	be enabled for it.
+ *	power and is capable of generating runtime wakeup events, remote wakeup
+ *	(i.e., a hardware mechanism allowing the device to request a change of
+ *	its power state via an interrupt) should be enabled for it.
  *
  * @runtime_resume: Put the device into the fully active state in response to a
- *	wake-up event generated by hardware or at the request of software.  If
- *	necessary, put the device into the full power state and restore its
+ *	wakeup event generated by hardware or at the request of software.  If
+ *	necessary, put the device into the full-power state and restore its
  *	registers, so that it is fully operational.
  *
- * @runtime_idle: Device appears to be inactive and it might be put into a low
- *	power state if all of the necessary conditions are satisfied.  Check
+ * @runtime_idle: Device appears to be inactive and it might be put into a
+ *	low-power state if all of the necessary conditions are satisfied.  Check
  *	these conditions and handle the device as appropriate, possibly queueing
  *	a suspend request for it.  The return value is ignored by the PM core.
+ *
+ * Refer to Documentation/power/runtime_pm.txt for more information about the
+ * role of the above callbacks in device runtime power management.
+ *
  */
 
 struct dev_pm_ops {

Re: [PATCH 5] PM: Update comments describing device power management callbacks

[Alan Stern]

On Wed, 23 Nov 2011, Rafael J. Wysocki wrote:

> On Tuesday, November 22, 2011, Alan Stern wrote:
> > On Tue, 22 Nov 2011, Rafael J. Wysocki wrote:
> > 
> > > > The same data structure (dev_pm_ops) is used for both drivers and
> > > > subsystems.  Therefore the comments should be directed toward both
> > > > driver writers and subsystem writers.
> > > 
> > > I don't quite agree.  Trying to do so would turn that comment into a lengthy
> > > document and there's no room for anything like that in the header file.
> > 
> > That's a good point.
> > 
> > > Perhaps I'll just modify the comment to say when the callbacks are executed
> > > without specifying what the specific callbacks are supposed to do (because
> > > that may vary from one subsystem to another and even between different
> > > drivers belonging to the same subsystem).  If PM domains are taken into
> > > account, it gets even more complicated.
> > 
> > Yes, a minimal description would be good, along with a pointer to the 
> > Documentation/power files.
> 
> Well, the one below is not exactly minimal, but I think it doesn't go too
> far at least.

Yes, I like this version better.

Alan Stern

Re: [PATCH 5] PM: Update comments describing device power management callbacks

[Rafael J. Wysocki]

On Wednesday, November 23, 2011, Alan Stern wrote:
> On Wed, 23 Nov 2011, Rafael J. Wysocki wrote:
> 
> > On Tuesday, November 22, 2011, Alan Stern wrote:
> > > On Tue, 22 Nov 2011, Rafael J. Wysocki wrote:
> > > 
> > > > > The same data structure (dev_pm_ops) is used for both drivers and
> > > > > subsystems.  Therefore the comments should be directed toward both
> > > > > driver writers and subsystem writers.
> > > > 
> > > > I don't quite agree.  Trying to do so would turn that comment into a lengthy
> > > > document and there's no room for anything like that in the header file.
> > > 
> > > That's a good point.
> > > 
> > > > Perhaps I'll just modify the comment to say when the callbacks are executed
> > > > without specifying what the specific callbacks are supposed to do (because
> > > > that may vary from one subsystem to another and even between different
> > > > drivers belonging to the same subsystem).  If PM domains are taken into
> > > > account, it gets even more complicated.
> > > 
> > > Yes, a minimal description would be good, along with a pointer to the 
> > > Documentation/power files.
> > 
> > Well, the one below is not exactly minimal, but I think it doesn't go too
> > far at least.
> 
> Yes, I like this version better.

Great, thanks!

Rafael