Re: PCI PM: Restore standard config registers of all devices early

Linus Torvalds
2009-02-02T20:51:35Z

On Tue, 3 Feb 2009, Benjamin Herrenschmidt wrote:
>
> And won't you have a potential problem here if ACPI is doing clock
> gating or turning off the whole power plane ? IE. If that happens, your
> pci_restore_state() done early with interrupts off will fail as well...

Well, it won't fail any worse than it does anyway.

> Since the root of that problem seems to be related to interrupts, maybe
> the right approach is to have drivers disable it on suspend (ie,
> disable_irq -> disabled at PIC level)

No.

We went through this, you apparently didn't follow it.

You cannot disable_irq() at suspend/resume time, because that means that
when you have shared interrupts the device can no longer use interrupts in
those paths - because they may be disabled by totally different devices.

And a lot of devices WILL NOT WORK without interrupts. Including
suspend/resume events. Think something as common as USB.

> It's not a trivial problem...

That's the understatement of the year.

The whole reason we want to do the two-phase commit thing where
"suspend()" starts the thing and "suspend_late()" finalizes things is
exactly all about these inter-connections and interrupts in particular.

I suspect that we could possibly make ACPI happy by actually leaving
interrupts "enabled" in the suspend-late (and early-resume) paths, but
with all hardware interrupts actually turned off. But that's really just a
"let's fool people by turning off interrupts a different way" thing - it
in no way really changes any fundamental issues.

Whether you use "disable_irq() over all interrupts" or "local_irq_save ->
local_irq_restore" really doesn't change anything. You cannot do this in a
single phase, because that means that you randomly disable interrupts too
early (or enable them too late) when drivers still _require_ them.

So the only workable way to handle interrupts is one of two:

- the two-phase thing we do. Do a first phase with interrupts enabled,
then the actual low-level "turn off" with interrupts disabled (and the
reverse on resume), so that device drivers never have to see the case
of "interrupt happens with dead device", while still having the
_guarantee_ that interrupts work for part of their suspend/resume
cycle.

- expecting all drivers to be perfect and handle interrupts correcly in
the driver.

Quite frankly, I don't think the second one is workable. It may be the
optimal one in theory, but it's never worked for us in practice.

Linus