Messages in this thread | | | Date | Fri, 27 Feb 2015 09:38:59 +0100 | From | Peter Zijlstra <> | Subject | Re: [PATCH] genirq / PM: Add flag for shared NO_SUSPEND interrupt lines |
| |
On Fri, Feb 27, 2015 at 12:07:55AM +0100, Rafael J. Wysocki wrote: > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > It currently is required that all users of NO_SUSPEND interrupt > lines pass the IRQF_NO_SUSPEND flag when requesting the IRQ or the > WARN_ON_ONCE() in irq_pm_install_action() will trigger. That is > done to warn about situations in which unprepared interrupt handlers > may be run unnecessarily for suspended devices and may attempt to > access those devices by mistake. However, it may cause drivers > that have no technical reasons for using IRQF_NO_SUSPEND to set > that flag just because they happen to share the interrupt line > with something like a timer. > > Moreover, the generic handling of wakeup interrupts introduced by > commit 9ce7a25849e8 (genirq: Simplify wakeup mechanism) only works > for IRQs without any NO_SUSPEND users, so the drivers of wakeup > devices needing to use shared NO_SUSPEND interrupt lines for > signaling system wakeup generally have to detect wakeup in their > interrupt handlers. Thus if they happen to share an interrupt line > with a NO_SUSPEND user, they also need to request that their > interrupt handlers be run after suspend_device_irqs(). > > In both cases the reason for using IRQF_NO_SUSPEND is not because > the driver in question has a genuine need to run its interrupt > handler after suspend_device_irqs(), but because it happens to > share the line with some other NO_SUSPEND user. Otherwise, the > driver would do without IRQF_NO_SUSPEND just fine. > > To make it possible to specify that condition explicitly, introduce > a new IRQ action handler flag for shared IRQs, IRQF_COND_SUSPEND, > that, when set, will indicate to the IRQ core that the interrupt > user is generally fine with suspending the IRQ, but it also can > tolerate handler invocations after suspend_device_irqs() and, in > particular, it is capable of detecting system wakeup and triggering > it as appropriate from its interrupt handler. > > That will allow us to work around a problem with a shared timer > interrupt line on at91 platforms. > > Link: http://marc.info/?l=linux-kernel&m=142252777602084&w=2 > Link: http://marc.info/?t=142252775300011&r=1&w=2 > Linx: https://lkml.org/lkml/2014/12/15/552 > Reported-by: Boris Brezillon <boris.brezillon@free-electrons.com> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Seems good to me. Should I take this through tip/irq ?
Also, should we warn if people use enable_irq_wake() where there is only a single descriptor with NO_SUSPEND?
| |