Seeking clarity on IRQCHIP_MASK_ON_SUSPEND

[NeilBrown]

The IRQCHIP_MASK_ON_SUSPEND flag seems to be hard to use correctly, so either
I'm understanding it wrongly, or it could be made easier to use.
If the first case, I'm hoping that some improvement to documentation might
result.  If the second, then maybe we can fix the code.

As I understand it, the need for IRQCHIP_MASK_ON_SUSPEND comes from the fact
that interrupt masking happens lazily.  When an interrupt is disabled the
hardware isn't told about that immediately, only internal data structures are
updated.  If/when the interrupt actually happens, only then is it masked and
ignored.
This cannot really work in suspend as masking after the interrupt fires means
that we have already woken from suspend.

So suspend_device_irqs() (called between the suspend_late handlers and the
suspend_noirq handlers) just disables all interrupts and check_wakeup_irqs()
(which is called very late) is left to talk to the hardware and actually mask
those which should not cause a wake-from-suspend.

The problem is that check_wakeup_irqs() is called so late that it might not be
possible to talk to the hardware.  For example, on the OMAP3 platform,
runtime power management will gate the 'iclk' (interface clock) and possibly
other clocks so that it is no longer possible to talk over an i2c bus or even
directly to the GPIO SoC module to effect the masking.
As runtime PM is disabled at this stage of the suspend cycle it is not even
possible to turn the iclk back on, mask the interrupt, then turn it off
again.

So it seems to me that either:

 1/ irq_chip devices need to mask any non-wakeup interrupts in ->suspend or
    possibly in ->suspend_late before runtimePM has switched the interface
    off, making IRQCHIP_MASK_ON_SUSPEND essentially useless except for some
    rare cases.
or
 2/ IRQCHIP_MASK_ON_SUSPEND needs to happen much earlier, probably before
    the suspend_late callbacks.

It might be tempting to change suspend_device_irqs() to disable interrupts in
such a way that the 'mask' happen synchronously (for non-wakeup interrupts).
However I don't think that would work as it happens after suspend_late and I
think suspend_late is allowed to power_down devices (and iclks).

I *think* the correct answer is '1'.  In that case I would love to know when
IRQCHIP_MASK_ON_SUSPEND can be used correctly (if ever).  I'm hoping someone
who works with interrupts and power management more than I can help me
understand...

There are currently only two drivers that set IRQCHIP_MASK_ON_SUSPEND: 
  arch/arm/mach-omap2/omap-wakeupgen.c
and 
  drivers/mfd/pm8xxx-irq.c

The former is in an 'always-on' power domain so the interaction with runtime
PM presumably doesn't affect it.

The later ... I don't think will compile.  It is only used by pm8921-core.c,
and that requires #include <linux/msm_ssbi.h>, which doesn't exist in
mainline.

Is anyone able to give a definitive answer on this?  Should
IRQCHIP_MASK_ON_SUSPEND be removed?


Thanks,
NeilBrown

Attachment: signature.asc
Description: PGP signature