lkml.org 
[lkml]   [2011]   [Nov]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [PATCH 5] PM: Update comments describing device power management callbacks
    Date
    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


    \
     
     \ /
      Last update: 2011-11-21 22:23    [W:0.034 / U:90.568 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site