Re: [PATCH] genirq: ensure IRQs are lazy disabled before suspend



On Thu, May 7, 2009 at 9:19 AM, Kevin Hilman
<khilman@xxxxxxxxxxxxxxxxxxx> wrote:
From a3f359c66bd0ae1bb2603e7cf120d9d4d68591b7 Mon Sep 17 00:00:00 2001
From: Kevin Hilman <khilman@xxxxxxxxxxxxxxxxxxx>
Date: Wed, 6 May 2009 16:00:07 -0700
Subject: [PATCH] genirq: ensure IRQs are lazy disabled before suspend

In commit 76d2160147f43f982dfe881404cfde9fd0a9da21, the default
behavior of disable_irq() was changed to delay the disable until it is
next handled.

However, this leaves open the possibility that the system can go into
suspend with an interrupt enabled.  For example, if a driver calls
disable_irq() in its suspend_hook there's now a possibility that the
system is suspended before the lazy disable happens.

The result is an unwanted wakeup from suspend if the IRQ is capable of
waking the system (common on embedded SoCs.)

If the interrupt contoller uses the same enable register for wakeup
and interrupts, I think it is the responsibility of the platform code,
not individual drivers, to disable the interrupts that are not marked
for wakeup before entering suspend.

This patch ensures that the lazy disable is done, and masked by
the irq_chip before the system goes into suspend.

This will create a window where wakeup interrupts can be lost if the
driver has masked the interrupt (by calling disable_irq). If the
hardware does not allow edge detection on disabled interrupts (the msm
platform has this limitation) then this change will turn off the edge
detection. If suspend_ops->enter does not turn the interrupt (and edge
detection) back on (without this change it may never need to turn on
any interrupt) it will not wakeup at all.

Note that even though __disable_irq() also calls irq_chip->disable, it
is quite common for the irq_chip not to provide a disable hook in
which case the IRQ is never masked/disabled in hardware before going
into suspend.

Signed-off-by: Kevin Hilman <khilman@xxxxxxxxxxxxxxxxxxx>
---
 kernel/irq/manage.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
index 2734eca..c786820 100644
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -190,6 +190,13 @@ void __disable_irq(struct irq_desc *desc, unsigned int irq, bool suspend)
               if (!desc->action || (desc->action->flags & IRQF_TIMER))
                       return;
               desc->status |= IRQ_SUSPENDED;
+
+               /*
+                * Lazy disable: handles case where lazy disable in
+                * handler doesn't happen before suspend.
+                */
+               if (desc->status & IRQ_DISABLED)
+                       desc->chip->mask(irq);
       }

       if (!desc->depth++) {
--
1.6.2.2

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/




--
Arve Hjønnevåg
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/



Relevant Pages

  • Re: [PATCH] PM: suspend_device_irqs(): dont disable wakeup IRQs
    ... If it's a wakeup IRQ, I assume you want it to prevent suspend. ... allowing the CPU to actually receive the interrupt ... your suspend_device_irqsfunction disables all IRQs at the ...
    (Linux-Kernel)
  • Re: [patch] ohci1394: dont leave interrupts enabled during suspend/resume
    ... irq 19: nobody cared ... There also seems to be an interrupt storm during suspend/resume when this ... This patch gets rid of both issues and makes the resume as a whole ... Likely the driver should be setting some register on the device to disable it from generating interrupts before suspend and it's not doing this. ...
    (Linux-Kernel)
  • Re: [PATCH] PM: suspend_device_irqs(): dont disable wakeup IRQs
    ... should not be disabled for suspend. ... I'd be interested in hearing why wakeup interrupts should be disabled ... What do you do if the interrupt triggers right after your driver has ... If it's a wakeup IRQ, I assume you want it to prevent suspend. ...
    (Linux-Kernel)
  • Re: [PATCH] PM: suspend_device_irqs(): dont disable wakeup IRQs
    ... should not be disabled for suspend. ... I'd be interested in hearing why wakeup interrupts should be disabled ... What do you do if the interrupt triggers right after your driver has ... If it's a wakeup IRQ, I assume you want it to prevent suspend. ...
    (Linux-Kernel)
  • Re: [PATCH] PM: suspend_device_irqs(): dont disable wakeup IRQs
    ... should not be disabled for suspend. ... I'd be interested in hearing why wakeup interrupts should be disabled ... What do you do if the interrupt triggers right after your driver has ... If it's a wakeup IRQ, I assume you want it to prevent suspend. ...
    (Linux-Kernel)