On 4/3/2011 1:17 AM, Colin Cross wrote:
On Sat, Apr 2, 2011 at 2:40 AM, Santosh Shilimkar
<santosh.shilimkar@xxxxxx> wrote:
On 4/2/2011 11:40 AM, Colin Cross wrote:
[....]
+#ifdef CONFIG_PM
+/*
+ * Architecture specific set_wake extension
+ */
+static int wakeupgen_set_wake(struct irq_data *d, unsigned int on)
+{
+ spin_lock(&wakeupgen_lock);
+ if (on)
+ _wakeupgen_set(d->irq);
+ else
+ _wakeupgen_clear(d->irq);
+ spin_unlock(&wakeupgen_lock);
+
+ return 0;
+}
+
+#else
+#define wakeupgen_set_wake NULL
+#endif
I don't think these are correct, and it probably only works at all due
to lazy disabling of interrupts during suspend.
First, unless I'm missing some code somewhere, all interrupts are
still enabled during suspend. Any interrupt that has had enable_irq
on it resulted in a call to wakeupgen_unmask, but when disable_irq is
called on all the interrupts during suspend, masking is delayed until
an interrupt is delivered. If no interrupt is delivered, all enabled
irqs will still be enabled in the wakeupgen module in suspend, and
they will all wake the device out of suspend.
During suspend it's expected that the drivers disables there interrupts
as part of suspend hooks. One can used "set_wake" API's to
enable/disable wakeups from suspend.
But because of lazy masking, disabling an interrupt is not the same as
masking. None of the interrupts will get masked, and they will all
act as wakeup interrupts, even if enable_irq_wake was never called on
it.
Second, it is possible for a wake interrupt that should be enabled to
end up disabled in suspend. Consider the following calls that occur
in a driver during its suspend handler:
enable_irq_wake
...
wakeupgen_unmask (irq is now unmasked)
disable_irq (lazy disable, wakeupgen_mask is not called, irq is still
unmasked)
<irq occurs>
handle_level_irq
...
wakeupgen_mask (irq is now masked)
The irq will never get unmasked because it is marked disabled, and the
irq will not wake the device from suspend.
wakeupgen_set_wake needs to set or clear bits in memory, and then
those suspend masks need to be copied into the wakeupgen registers
very late in suspend, after interrupts have been disabled at the cpu.
I think syscore_ops is the right place.
This is a good point about lazy disabling. Copy to memory happens
already as part of save in SAR layout.
Will think over this one. Thanks for bringing this point here.
The key is that mask/unmask and set_wake must act on different mask
data - mask/unmask must take effect immediately, so they must write to
the mask registers, but set_wake takes effect only during suspend, so
it must write to a copy of the mask registers that gets switched in
during suspend.
Right. I shall fix the set_wake() API to take care of lazy
disabling of IRQs.
Regards
Santosh
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
