diff mbox

genirq: better describe IRQF_NO_SUSPEND semantics

Message ID 1424444026-31231-1-git-send-email-mark.rutland@arm.com (mailing list archive)
State New, archived
Headers show

Commit Message

Mark Rutland Feb. 20, 2015, 2:53 p.m. UTC
The IRQF_NO_SUSPEND flag is intended to be used for interrupts required
to be enabled during the suspend-resume cycle. This mostly consists of
IPIs and timer interrupts, potentially including chained irqchip
interrupts if these are necessary to handle timers or IPIs. If an
interrupt does not fall into one of the aforementioned categories,
requesting it with IRQF_NO_SUSPEND is likely incorrect.

Using IRQF_NO_SUSPEND does not guarantee that the interrupt can wake the
system from a suspended state. For an interrupt to be able to trigger a
wakeup, it may be necessary to program various components of the system.
In these cases it is necessary to use {enable,disabled}_irq_wake.

Unfortunately, several drivers assume that IRQF_NO_SUSPEND ensures that
an IRQ can wake up the system, and the documentation can be read
ambiguously w.r.t. this property.

This patch updates the documentation regarding IRQF_NO_SUSPEND to make
this caveat explicit, hopefully making future misuse rarer. Cleanup of
existing misuse will occur as part of later patch series.

Cc: Boris Brezillon <boris.brezillon@free-electrons.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
---
 Documentation/power/suspend-and-interrupts.txt | 6 ++++--
 include/linux/interrupt.h                      | 4 +++-
 2 files changed, 7 insertions(+), 3 deletions(-)

Comments

Rafael J. Wysocki Feb. 24, 2015, 12:26 a.m. UTC | #1
On Friday, February 20, 2015 02:53:46 PM Mark Rutland wrote:
> The IRQF_NO_SUSPEND flag is intended to be used for interrupts required
> to be enabled during the suspend-resume cycle. This mostly consists of
> IPIs and timer interrupts, potentially including chained irqchip
> interrupts if these are necessary to handle timers or IPIs. If an
> interrupt does not fall into one of the aforementioned categories,
> requesting it with IRQF_NO_SUSPEND is likely incorrect.
> 
> Using IRQF_NO_SUSPEND does not guarantee that the interrupt can wake the
> system from a suspended state. For an interrupt to be able to trigger a
> wakeup, it may be necessary to program various components of the system.
> In these cases it is necessary to use {enable,disabled}_irq_wake.
> 
> Unfortunately, several drivers assume that IRQF_NO_SUSPEND ensures that
> an IRQ can wake up the system, and the documentation can be read
> ambiguously w.r.t. this property.
> 
> This patch updates the documentation regarding IRQF_NO_SUSPEND to make
> this caveat explicit, hopefully making future misuse rarer. Cleanup of
> existing misuse will occur as part of later patch series.
> 
> Cc: Boris Brezillon <boris.brezillon@free-electrons.com>
> Cc: Ian Campbell <ian.campbell@citrix.com>
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>

Applied to linux-pm.git/linux-next, thanks!

Peter, please let me know if you don't want this to go it.

Rafael
Peter Zijlstra Feb. 25, 2015, 3:19 p.m. UTC | #2
On Tue, Feb 24, 2015 at 01:26:05AM +0100, Rafael J. Wysocki wrote:
> On Friday, February 20, 2015 02:53:46 PM Mark Rutland wrote:
> > The IRQF_NO_SUSPEND flag is intended to be used for interrupts required
> > to be enabled during the suspend-resume cycle. This mostly consists of
> > IPIs and timer interrupts, potentially including chained irqchip
> > interrupts if these are necessary to handle timers or IPIs. If an
> > interrupt does not fall into one of the aforementioned categories,
> > requesting it with IRQF_NO_SUSPEND is likely incorrect.
> > 
> > Using IRQF_NO_SUSPEND does not guarantee that the interrupt can wake the
> > system from a suspended state. For an interrupt to be able to trigger a
> > wakeup, it may be necessary to program various components of the system.
> > In these cases it is necessary to use {enable,disabled}_irq_wake.
> > 
> > Unfortunately, several drivers assume that IRQF_NO_SUSPEND ensures that
> > an IRQ can wake up the system, and the documentation can be read
> > ambiguously w.r.t. this property.
> > 
> > This patch updates the documentation regarding IRQF_NO_SUSPEND to make
> > this caveat explicit, hopefully making future misuse rarer. Cleanup of
> > existing misuse will occur as part of later patch series.
> > 
> > Cc: Boris Brezillon <boris.brezillon@free-electrons.com>
> > Cc: Ian Campbell <ian.campbell@citrix.com>
> > Cc: Jonathan Corbet <corbet@lwn.net>
> > Cc: Peter Zijlstra <peterz@infradead.org>
> > Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> 
> Applied to linux-pm.git/linux-next, thanks!
> 
> Peter, please let me know if you don't want this to go it.

No seems fine, Thanks!

Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Jonathan Corbet Feb. 27, 2015, 10:08 p.m. UTC | #3
On Fri, 20 Feb 2015 14:53:46 +0000
Mark Rutland <mark.rutland@arm.com> wrote:

> This patch updates the documentation regarding IRQF_NO_SUSPEND to make
> this caveat explicit, hopefully making future misuse rarer. Cleanup of
> existing misuse will occur as part of later patch series.

Applied (with Peter's ack) to the docs tree.

Thanks,

jon
Jonathan Corbet Feb. 27, 2015, 10:23 p.m. UTC | #4
On Fri, 27 Feb 2015 23:41:05 +0100
"Rafael J. Wysocki" <rjw@rjwysocki.net> wrote:

> > Applied (with Peter's ack) to the docs tree.  
> 
> Well, I've applied it too in the meantime. :-)
> 
> Also I'll have more commits on top of it, so it's better if it goes through
> my tree I think, if you don't mind.

Gee, I feel like I've been here before :)

In the future, would you like me to avoid taking patches to
Documentation/power?

I'll drop it out of my tree.

jon
Rafael J. Wysocki Feb. 27, 2015, 10:41 p.m. UTC | #5
On Friday, February 27, 2015 03:08:10 PM Jonathan Corbet wrote:
> On Fri, 20 Feb 2015 14:53:46 +0000
> Mark Rutland <mark.rutland@arm.com> wrote:
> 
> > This patch updates the documentation regarding IRQF_NO_SUSPEND to make
> > this caveat explicit, hopefully making future misuse rarer. Cleanup of
> > existing misuse will occur as part of later patch series.
> 
> Applied (with Peter's ack) to the docs tree.

Well, I've applied it too in the meantime. :-)

Also I'll have more commits on top of it, so it's better if it goes through
my tree I think, if you don't mind.

Rafael
Rafael J. Wysocki Feb. 27, 2015, 11:04 p.m. UTC | #6
On Friday, February 27, 2015 03:23:13 PM Jonathan Corbet wrote:
> On Fri, 27 Feb 2015 23:41:05 +0100
> "Rafael J. Wysocki" <rjw@rjwysocki.net> wrote:
> 
> > > Applied (with Peter's ack) to the docs tree.  
> > 
> > Well, I've applied it too in the meantime. :-)
> > 
> > Also I'll have more commits on top of it, so it's better if it goes through
> > my tree I think, if you don't mind.
> 
> Gee, I feel like I've been here before :)
> 
> In the future, would you like me to avoid taking patches to
> Documentation/power?

Well, I tend to apply them as a rule, unless I overlook them.  That usually
happens if they aren't CCed to me directly or to linux-pm.

But yeah, you can safely avoid patches touching Documentation/power/ or
Documentation/acpi/, I'll be taking care of them.

> I'll drop it out of my tree.

Thanks!
diff mbox

Patch

diff --git a/Documentation/power/suspend-and-interrupts.txt b/Documentation/power/suspend-and-interrupts.txt
index 2f9c5a5..50493c9 100644
--- a/Documentation/power/suspend-and-interrupts.txt
+++ b/Documentation/power/suspend-and-interrupts.txt
@@ -40,8 +40,10 @@  but also to IPIs and to some other special-purpose interrupts.
 
 The IRQF_NO_SUSPEND flag is used to indicate that to the IRQ subsystem when
 requesting a special-purpose interrupt.  It causes suspend_device_irqs() to
-leave the corresponding IRQ enabled so as to allow the interrupt to work all
-the time as expected.
+leave the corresponding IRQ enabled so as to allow the interrupt to work as
+expected during the suspend-resume cycle, but does not guarantee that the
+interrupt will wake the system from a suspended state -- for such cases it is
+necessary to use enable_irq_wake().
 
 Note that the IRQF_NO_SUSPEND flag affects the entire IRQ and not just one
 user of it.  Thus, if the IRQ is shared, all of the interrupt handlers installed
diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
index d9b05b5..606771c 100644
--- a/include/linux/interrupt.h
+++ b/include/linux/interrupt.h
@@ -52,7 +52,9 @@ 
  * IRQF_ONESHOT - Interrupt is not reenabled after the hardirq handler finished.
  *                Used by threaded interrupts which need to keep the
  *                irq line disabled until the threaded handler has been run.
- * IRQF_NO_SUSPEND - Do not disable this IRQ during suspend
+ * IRQF_NO_SUSPEND - Do not disable this IRQ during suspend.  Does not guarantee
+ *                   that this interrupt will wake the system from a suspended
+ *                   state.  See Documentation/power/suspend-and-interrupts.txt
  * IRQF_FORCE_RESUME - Force enable it on resume even if IRQF_NO_SUSPEND is set
  * IRQF_NO_THREAD - Interrupt cannot be threaded
  * IRQF_EARLY_RESUME - Resume IRQ early during syscore instead of at device