Re: [PATCH] s2ram: add arch irq disable/enable hooks

[Johannes Berg <johannes@sipsolutions.net>]

[-- Attachment #1.1: Type: text/plain, Size: 1401 bytes --]

On Fri, 2007-04-20 at 22:36 +0200, Rafael J. Wysocki wrote:

> Well, I can't say why exactly suspend_enter() usues irq_save/restore, but it
> looks like that's not needed.

powerpc works fine just with local_irq_disable/enable, never did
anything else and my new patches just ignore the *flags argument.

> For the suspend to disk we call
> local_irq_disable/enable() in the corresponding places and, for example,
> acpi_pm_enter apparently uses irq_save/restore() itself too.

Why would acpi_pm_enter do that? It can always assume irqs are disabled.
Or does it need to enable them for something (hey, that would mean Ben
is right in that it probably wants to use these new hooks...)

> > The only users of this function are in the power management core code (main.c
> > and user.c) and they certainly always call it with interrupts enabled, the
> > entry point for other users is pm_suspend() and that surely cannot be called 
> > with interrupts disabled. Rafael?
> 
> That's correct, plus pm_suspend() also uses enter_state().

Sure, but it can't be called with interrupts disabled.

> In fact suspend_enter() has been made extern so that we can call it from
> kernel/power/user.c and no one else is supposed to use it.

Right.

So, any argument against re-spinning this patch, removing the unsigned
long *flags stuff and using local_irq_disable/enable?

johannes

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 190 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]