[PATCH] genirq: allow safe sharing of irqs during suspend

From: Mark Rutland
Date: Wed Feb 11 2015 - 11:44:06 EST


In some cases a physical IRQ line may be shared between devices from
which we expect interrupts during suspend (e.g. timers) and those we do
not (e.g. anything we cut the power to). Where a driver did not request
the interrupt with IRQF_NO_SUSPEND, it's unlikely that it can handle
being called during suspend, and where the IRQ PM code detects a
mismatch it produces a loud warning (via WARN_ON_ONCE).

In a small set of cases the handlers for the devices other than timers
can tolerate being called during suspend time. In these cases the
warning is spurious, and masks other potentially unsafe mismatches as it
is only printed for the first mismatch detected. As the behaviour of the
handlers is an implementation detail, we cannot rely on external data to
decide when it is safe for a given interrupt line to be shared in this
manner.

This patch adds a new flag, IRQF_SHARED_TIMER_OK, which drivers can use
when requesting an IRQ to state that they can cope if the interrupt is
shared with a timer driver (and hence may be raised during suspend). The
PM code is updated to only warn when a mismatch occurs and at least one
irqaction has neither asked to be called during suspend or has stated it
is safe to be called during suspend.

This reduces the set of warnings to those cases where there is a real
problem. While it is possible that this flag may be abused, any such
abuses will be explicit in the kernel source and can be detected.

Cc: Boris Brezillon <boris.brezillon@xxxxxxxxxxxxxxxxxx>
Cc: Jason Cooper <jason@xxxxxxxxxxxxxx>
Cc: Nicolas Ferre <nicolas.ferre@xxxxxxxxx>
Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
Cc: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
Signed-off-by: Mark Rutland <mark.rutland@xxxxxxx>
---
include/linux/interrupt.h | 5 +++++
kernel/irq/pm.c | 44 ++++++++++++++++++++++++++++++++++++++++++--
2 files changed, 47 insertions(+), 2 deletions(-)

diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
index d9b05b5..2b8ff50 100644
--- a/include/linux/interrupt.h
+++ b/include/linux/interrupt.h
@@ -57,6 +57,9 @@
* IRQF_NO_THREAD - Interrupt cannot be threaded
* IRQF_EARLY_RESUME - Resume IRQ early during syscore instead of at device
* resume time.
+ * IRQF_SHARED_TIMER_OK - Interrupt is safe to be shared with a timer. The
+ * handler may be called spuriously during suspend
+ * without issue.
*/
#define IRQF_DISABLED 0x00000020
#define IRQF_SHARED 0x00000080
@@ -70,8 +73,10 @@
#define IRQF_FORCE_RESUME 0x00008000
#define IRQF_NO_THREAD 0x00010000
#define IRQF_EARLY_RESUME 0x00020000
+#define __IRQF_TIMER_SIBLING_OK 0x00040000

#define IRQF_TIMER (__IRQF_TIMER | IRQF_NO_SUSPEND | IRQF_NO_THREAD)
+#define IRQF_SHARED_TIMER_OK (IRQF_SHARED | __IRQF_TIMER_SIBLING_OK)

/*
* These values can be returned by request_any_context_irq() and
diff --git a/kernel/irq/pm.c b/kernel/irq/pm.c
index 3ca5325..e4ec91a 100644
--- a/kernel/irq/pm.c
+++ b/kernel/irq/pm.c
@@ -28,6 +28,47 @@ bool irq_pm_check_wakeup(struct irq_desc *desc)
}

/*
+ * Check whether an interrupt is safe to occur during suspend.
+ *
+ * Physical IRQ lines may be shared between devices which may be expected to
+ * raise interrupts during suspend (e.g. timers) and those which may not (e.g.
+ * anything we cut the power to). Not all handlers will be safe to call during
+ * suspend, so we need to scream if there's the possibility an unsafe handler
+ * will be called.
+ *
+ * A small number of handlers are safe to be shared with timer interrupts, and
+ * we don't want to warn erroneously for these. Such handlers will not poke
+ * hardware that's not powered or call into kernel infrastructure not available
+ * during suspend. These are marked with __IRQF_TIMER_SIBLING_OK.
+ */
+bool irq_safe_during_suspend(struct irq_desc * desc, struct irqaction *action)
+{
+ const unsigned int safe_flags =
+ __IRQF_TIMER_SIBLING_OK | IRQF_NO_SUSPEND;
+
+ /*
+ * If no-one wants to be called during suspend, or if everyone does,
+ * then there's no potential conflict.
+ */
+ if (!desc->no_suspend_depth)
+ return true;
+ if (desc->no_suspend_depth == desc->nr_actions)
+ return true;
+
+ /*
+ * If any action hasn't asked to be called during suspend or is not
+ * happy to be called during suspend, we have a potential problem.
+ */
+ if (!(action->flags & safe_flags))
+ return false;
+ for (action = desc->action; action; action = action->next)
+ if (!(action->flags & safe_flags))
+ return false;
+
+ return true;
+}
+
+/*
* Called from __setup_irq() with desc->lock held after @action has
* been installed in the action chain.
*/
@@ -44,8 +85,7 @@ void irq_pm_install_action(struct irq_desc *desc, struct irqaction *action)
if (action->flags & IRQF_NO_SUSPEND)
desc->no_suspend_depth++;

- WARN_ON_ONCE(desc->no_suspend_depth &&
- desc->no_suspend_depth != desc->nr_actions);
+ WARN_ON_ONCE(!irq_safe_during_suspend(desc, action));
}

/*
--
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/