Date:	Sun, 20 Nov 2011 21:56:30 -0500 (EST)
From:	Alan Stern <stern@...land.harvard.edu>
To:	"Rafael J. Wysocki" <rjw@...k.pl>
cc:	Linux PM list <linux-pm@...r.kernel.org>,
	LKML <linux-kernel@...r.kernel.org>,
	Randy Dunlap <rdunlap@...otime.net>
Subject: Re: [PATCH 5] PM: Update comments describing device power management
 callbacks

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

> From: Rafael J. Wysocki <rjw@...k.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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists