Messages in this thread | | | From | "Rafael J. Wysocki" <> | Subject | Re: [PATCH 5] PM: Update comments describing device power management callbacks | Date | Mon, 21 Nov 2011 22:23:15 +0100 |
| |
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
| |