On 15/09/15 03:52, Dmitry Torokhov wrote:

On Mon, Sep 14, 2015 at 4:16 AM, Sudeep Holla [off-list ref] wrote:

[...]

quoted

This is wrong assumption in the driver. enable_irq_wake doesn't
implicitly enable the IRQ. So the disable_irq should be moved to else.
And the resume patch also needs to be fixed accordingly, otherwise you
may get unbalanced irq. But this should not be the reason for fixing the
pinctrl suspend/resume.

Elan driver does not want to enable servicing IRQs, it just wants to
configure them as wakeup sources. Hence the current elan_suspend() is
fine. When system wakes up and the device is resumed and the driver is
ready to service interrupts it will enable IRQ again.

Fair enough. But I am struggling to understand how this fits into
existing IRQ infrastructure. Few controllers that don't have wakeup
source configuration facility can set IRQCHIP_SKIP_SET_WAKE and just
leave the interrupts enabled in suspend path to wake it up. So IMO,
the above strategy might not work on such controllers.

IOW enable_irq_wake() and enable_irq() are 2 completely different
calls and it is perfectly fine to disable IRQ and then ebale it as a
wakeup source.

I agree that they are entirely different APIs, I am not sure if we can
support different interrupt controller with such strategy.

Since the irq/pm core handle disabling device IRQs and section "System
Wakeup Interrupts, enable_irq_wake() and disable_irq_wake()" in
Documentation/power/suspend-and-interrupts.txt gives me different
understanding, we can check with tglx on how to handle this.

Regards,
Sudeep