Re: [RFC][PATCH 1/3] PM: Introduce new top level suspend and hibernation callbacks

Rafael J. Wysocki
2008-03-19T13:22:00Z

On Wednesday, 19 of March 2008, Greg KH wrote:
> On Mon, Mar 17, 2008 at 12:22:29AM +0100, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rjw@sisk.pl>
> >
> > Introduce 'struct pm_ops' representing a set of suspend and
> > hibernation operations for bus types, device classes and device
> > types.
>
> Ok, I must have missed the thread describing why we need to do this, so,
> why do we need to do this? What is this going to buy us in the end
> after everything is changed?

There were many threads related to that.

To summarize, the first purpose is to separate suspend (aka s2ram and standby)
callbacks from hibernation callbacks in such a way that the new callbacks won't
take arguments and the purpose of each of them will be clearly specified. This
has been requested multiple times by many people, including Linus himself,
and the reason is that within the current scheme if ->resume() is called, for
example, it's difficult to say why it's been called (ie. is it a resume from RAM or
from hibernation or a suspend/hibernation failure etc.?).

The second purpose is to make the suspend/hibernation callbacks more flexible
so that device drivers can handle more than they can within the current scheme.
For example, some drivers may need to prevent new children of the device from
being registered before their ->suspend() callbacks are executed or they may
want to carry out some operations requiring the availability of some other
devices, not directly bound via the parent-child relationship, in order to prepare
for the execution of ->suspend(), etc.

Ultimately, we'd like to stop using the freezing of tasks for suspend and
therefore the drivers' suspend/hibernation code will have to take care of
the handling of the user space during suspend/hibernation which would be
difficult within the current scheme, without the ->prepare() and ->complete()
callbacks.

> > +struct pm_ops {
> > +#ifdef CONFIG_PM_SLEEP
> > + int (*prepare)(struct device *dev);
> > + void (*complete)(struct device *dev);
> > +#endif
> > +#ifdef CONFIG_SUSPEND
> > + int (*suspend)(struct device *dev);
> > + int (*resume)(struct device *dev);
> > +#endif
> > +#ifdef CONFIG_HIBERNATION
> > + int (*freeze)(struct device *dev);
> > + int (*thaw)(struct device *dev);
> > + int (*poweroff)(struct device *dev);
> > + int (*quiesce)(struct device *dev);
> > + int (*restore)(struct device *dev);
> > + int (*recover)(struct device *dev);
> > +#endif
>
>
> Don't ifdef stuff like this, it only causes ifdefs to be needed to the
> .c code as well for all places these structures are defined in
> drivers/busses, right?

Sure, but that code won't be necessary otherwise too.

I'll remove the #ifdefs in the next version of the patch.

Thanks,
Rafael