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

[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

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