Re: [PATCH] genirq / PM: Add flag for shared NO_SUSPEND interrupt lines

[Peter Zijlstra <peterz@infradead.org>]

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?