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

["Rafael J. Wysocki" <rjw@sisk.pl>]

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