Re: [PATCH v4] pm_ops: add system quiesce/activate hooks

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

On Friday, 13 April 2007 23:33, Pavel Machek wrote:
> On Fri 2007-04-13 23:18:05, Johannes Berg wrote:
> > On Fri, 2007-04-13 at 23:12 +0200, Pavel Machek wrote:
> > 
> > 
> > > > Whoops, I had that in an earlier version, it's not supposed to be called
> > > > for (u)swsusp because pm_ops involvement there is a hack anyway. I'll
> > > > fix the description.
> > > 
> > > But (u)swsusp needs to shut down the devices, and it does both suspend
> > > and powerdown, too...?
> > 
> > Yes, but the special S4 state is just hacked onto pm_ops, nothing but
> > ACPI S4 will ever sanely use pm_ops. If I'd invented pm_ops I'd made
> > pm_ops solely for suspend to ram/standby/... and added a *separate* hook
> > for pm_disk_mode and S4 because logically that really doesn't belong to
> > pm_ops which does almost exclusively things for suspend to ram/standby.
> 
> Still, swsusp shuts down the devices:
> 
>         if ((error = arch_prepare_suspend()))
>                 return error;
> 
>         local_irq_disable();
>         /* At this point, device_suspend() has been called, but *not*
>          * device_power_down(). We *must* device_power_down() now.
>          * Otherwise, drivers for some devices (e.g. interrupt
> controllers)
>          * become desynchronized with the actual state of the hardware
>          * at resume time, and evil weirdness ensues.
>          */
>         if ((error = device_power_down(PMSG_FREEZE))) {
>                 printk(KERN_ERR "Some devices failed to power down, aborting suspend\n");
>                 goto Enable_irqs;
>         }
> 
> This local_irq_disable() is very similar to the one you are doing in
> suspend-to-RAM. According to your description, it needs to play with
> the decrementor, too. But I'd really prefer not to have pm_ops call
> here -- exactly because swsusp has nothing to do with pm_ops in
> shutdown mode.

Hmm, I missed that. :-(

I think we need to make things clear: Either we add the additional hooks to
pm_ops in which case they should be taken into account in the (u)swsusp code
too, or we don't add them at all.

Well, there also is one more solution.  Namely, we can add the hooks to pm_ops
and make (u)swsusp use something else instead of pm_ops, but that would require
some more consideration.

Greetings,
Rafael