[PATCH 0/6 v2] irq / PM: Shared IRQs vs IRQF_NO_SUSPEND and suspend-to-idle wakeup interrupts

[PATCH 0/6 v2] irq / PM: Shared IRQs vs IRQF_NO_SUSPEND and suspend-to-idle wakeup interrupts

[Rafael J. Wysocki]

Hi,

I thought I'd just refresh the previous patchset, but in the meantime I found
a way to avoid adding overhead to note_interrupt(), so I decided to change the
approach.

Patch [2/6] fixes the problem with IRQF_NO_SUSPEND and shared interrupts.  The
idea is to use wrapper handlers for all irqactions of the shared IRQs where at
least one of them has IRQF_NO_SUSPEND set.  Since the wrapper handler is now
installed for all irqactions in the given irq_desc, it can track the return
codes from the original handlers and "suspend" the IRQ if the interrupt is going
to be unhandled eventually.  All described in the changelog.  This is independent
of the handling of wakeup interrupts for suspend-to-idle.

Patch [3/6] makes IRQs configured for system wakeup with enable_irq_wake() work
for suspend to idle.  It does that by using a (different) wrapper handler for
all irqactions in the wakeup irq_descs and that wrapper handler takes care of
all the ugly details.  Again, all described in the changelog.

Patches [1/6], [4-5/6] are unmodified and the last one addes documentation.

Please review.

Rafael

[PATCH 1/6 v2] PM / sleep: Mechanism for aborting system suspends unconditionally

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Subject: PM / sleep: Mechanism for aborting system suspends unconditionally

It sometimes may be necessary to abort a system suspend in
progress or wake up the system from suspend-to-idle even if the
pm_wakeup_event()/pm_stay_awake() mechanism is not enabled.

For this purpose, introduce a new global variable abort_suspend and
make pm_wakeup_pending() check its value.  Also add routines for
manipulating that variable.

This is going to be used in subsequent changes.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

This patch is the same as the previous version.

---
 drivers/base/power/wakeup.c |   20 ++++++++++++++++++++
 include/linux/suspend.h     |    4 ++++
 kernel/power/process.c      |    1 +
 3 files changed, 25 insertions(+)

Index: linux-pm/include/linux/suspend.h
===================================================================
--- linux-pm.orig/include/linux/suspend.h
+++ linux-pm/include/linux/suspend.h
@@ -371,6 +371,8 @@ extern int unregister_pm_notifier(struct
 extern bool events_check_enabled;
 
 extern bool pm_wakeup_pending(void);
+extern void pm_system_wakeup(void);
+extern void pm_wakeup_clear(void);
 extern bool pm_get_wakeup_count(unsigned int *count, bool block);
 extern bool pm_save_wakeup_count(unsigned int count);
 extern void pm_wakep_autosleep_enabled(bool set);
@@ -418,6 +420,8 @@ static inline int unregister_pm_notifier
 #define pm_notifier(fn, pri)	do { (void)(fn); } while (0)
 
 static inline bool pm_wakeup_pending(void) { return false; }
+static inline void pm_system_wakeup(void) {}
+static inline void pm_wakeup_clear(void) {}
 
 static inline void lock_system_sleep(void) {}
 static inline void unlock_system_sleep(void) {}
Index: linux-pm/drivers/base/power/wakeup.c
===================================================================
--- linux-pm.orig/drivers/base/power/wakeup.c
+++ linux-pm/drivers/base/power/wakeup.c
@@ -691,6 +691,8 @@ void pm_print_active_wakeup_sources(void
 }
 EXPORT_SYMBOL_GPL(pm_print_active_wakeup_sources);
 
+static bool abort_suspend;
+
 /**
  * pm_wakeup_pending - Check if power transition in progress should be aborted.
  *
@@ -712,6 +714,7 @@ bool pm_wakeup_pending(void)
 		ret = (cnt != saved_count || inpr > 0);
 		events_check_enabled = !ret;
 	}
+	ret = ret || abort_suspend;
 	spin_unlock_irqrestore(&events_lock, flags);
 
 	if (ret) {
@@ -722,6 +725,23 @@ bool pm_wakeup_pending(void)
 	return ret;
 }
 
+void pm_system_wakeup(void)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&events_lock, flags);
+	abort_suspend = true;
+	spin_unlock_irqrestore(&events_lock, flags);
+	freeze_wake();
+}
+
+void pm_wakeup_clear(void)
+{
+	spin_lock_irq(&events_lock);
+	abort_suspend = false;
+	spin_unlock_irq(&events_lock);
+}
+
 /**
  * pm_get_wakeup_count - Read the number of registered wakeup events.
  * @count: Address to store the value at.
Index: linux-pm/kernel/power/process.c
===================================================================
--- linux-pm.orig/kernel/power/process.c
+++ linux-pm/kernel/power/process.c
@@ -129,6 +129,7 @@ int freeze_processes(void)
 	if (!pm_freezing)
 		atomic_inc(&system_freezing_cnt);
 
+	pm_wakeup_clear();
 	printk("Freezing user space processes ... ");
 	pm_freezing = true;
 	error = try_to_freeze_tasks(true);

[PATCH 2/6 v2] irq / PM: Make IRQF_NO_SUSPEND work with shared interrupts

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Since __disable_irq() only checks IRQF_NO_SUSPEND for the first
irqaction in a given irq_desc, that value of that bit for the
first irqaction affects all of the other irqactions sharing the
interrupt with it.  This is problematic in two cases.

First, if IRQF_NO_SUSPEND is set in the first irqaction and unset
in at least one of the other irqactions sharing the same IRQ, the
interrupt handlers of the irqactions with IRQF_NO_SUSPEND unset
will be invoked after suspend_device_irqs(), even though they are
not supposed to be invoked at that time.  That shouldn't be a problem
if those interrupt handlers are implemented correctly and the
corresponding devices are properly suspended, but it may lead to
functional issues otherwise.

Second, if IRQF_NO_SUSPEND is unset in the first irqaction and set
in at least one of the other irqactions sharing the same IRQ, the
interrupt handlers of the irqactions with IRQF_NO_SUSPEND set will
not be invoked after suspend_device_irqs(), although they are
supposed to be invoked at that time.  That may be a problem if
interrupts from the corresponding sources have to be properly
handled during system suspend/resume too, which is the case for
timer interrupts or the ACPI SCI, for example.

Both the situations described above have to be avoided, but the
fix should only impact the handling of interrupts during system
suspend and resume.  That means no changes to the code in
kernel/irq/handle.c and to the code called from there.  Moreover,
kernels build with CONFIG_PM_SLEEP unset should not be affected
at all.

Another thing to consider is what to do if there's an unhandled
interrupt during system suspend after suspend_device_irqs() and
the consensus here seems to be to abort the suspend it that case
(or trigger a wakeup from suspend-to-idle if already there).

Taking the above into account leads to the following approach.

During suspend_device_irqs() all of the shared IRQs that are not to
be suspended (that is, shared IRQs with at least one IRQF_NO_SUSPEND
irqaction) are switched over to a special "suspend mode" by replacing
the original interrupt handlers in their irqactions by a wrapper
handler.

That wrapper handler executes the original interrupt handlers for
the IRQF_NO_SUSPEND irqactions only and tracks their return codes.
It does that by combining the return code of the current irqaction's
handler with the return codes from all of the previous irqactions
in the chain and propagating that value to the next irqaction in the
chain with the help of the (new) prev_ret field in struct irqaction.
For the last irqaction in the chain, the combined return code is thus
equal to the final value of retval in handle_irq_event_percpu().  If
that value is IRQ_NONE, the wrapper handler regards the interrupt as
unhandled, disables the IRQ, marks it as "suspended" and triggers
system wakeup to allow it to recover from that potentially dangerous
situation.

During resume_device_irqs() all IRQs that were previously in the
"suspend mode" described above are switched back to their original
configuration.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 include/linux/interrupt.h |    8 ++++
 include/linux/irqdesc.h   |    3 +
 kernel/irq/internals.h    |   20 ++++++++++
 kernel/irq/manage.c       |   11 ++++-
 kernel/irq/pm.c           |   89 ++++++++++++++++++++++++++++++++++++++++++++--
 5 files changed, 127 insertions(+), 4 deletions(-)

Index: linux-pm/include/linux/interrupt.h
===================================================================
--- linux-pm.orig/include/linux/interrupt.h
+++ linux-pm/include/linux/interrupt.h
@@ -101,6 +101,9 @@ typedef irqreturn_t (*irq_handler_t)(int
  * @thread_flags:	flags related to @thread
  * @thread_mask:	bitmask for keeping track of @thread activity
  * @dir:	pointer to the proc/irq/NN/name entry
+ * @s_handler:	original interrupt handler for suspend mode interrupts
+ * @s_dev_id:	original device identification cookie for suspend mode
+ * @prev_ret:	suspend mode return code from the previous action
  */
 struct irqaction {
 	irq_handler_t		handler;
@@ -115,6 +118,11 @@ struct irqaction {
 	unsigned long		thread_mask;
 	const char		*name;
 	struct proc_dir_entry	*dir;
+#ifdef CONFIG_PM_SLEEP
+	irq_handler_t		s_handler;
+	void			*s_dev_id;
+	irqreturn_t		prev_ret;
+#endif
 } ____cacheline_internodealigned_in_smp;
 
 extern irqreturn_t no_action(int cpl, void *dev_id);
Index: linux-pm/include/linux/irqdesc.h
===================================================================
--- linux-pm.orig/include/linux/irqdesc.h
+++ linux-pm/include/linux/irqdesc.h
@@ -71,6 +71,9 @@ struct irq_desc {
 #ifdef CONFIG_PROC_FS
 	struct proc_dir_entry	*dir;
 #endif
+#ifdef CONFIG_PM_SLEEP
+	unsigned int		skip_suspend_depth;
+#endif
 	int			parent_irq;
 	struct module		*owner;
 	const char		*name;
Index: linux-pm/kernel/irq/manage.c
===================================================================
--- linux-pm.orig/kernel/irq/manage.c
+++ linux-pm/kernel/irq/manage.c
@@ -385,7 +385,9 @@ setup_affinity(unsigned int irq, struct
 void __disable_irq(struct irq_desc *desc, unsigned int irq, bool suspend)
 {
 	if (suspend) {
-		if (!desc->action || (desc->action->flags & IRQF_NO_SUSPEND))
+		if (!desc->action)
+			return;
+		if (irq_pm_suspend_mode(desc))
 			return;
 		desc->istate |= IRQS_SUSPENDED;
 	}
@@ -445,6 +447,7 @@ EXPORT_SYMBOL(disable_irq);
 void __enable_irq(struct irq_desc *desc, unsigned int irq, bool resume)
 {
 	if (resume) {
+		irq_pm_normal_mode(desc);
 		if (!(desc->istate & IRQS_SUSPENDED)) {
 			if (!desc->action)
 				return;
@@ -1222,6 +1225,8 @@ __setup_irq(unsigned int irq, struct irq
 	desc->irq_count = 0;
 	desc->irqs_unhandled = 0;
 
+	irq_pm_setup(desc, new);
+
 	/*
 	 * Check whether we disabled the irq via the spurious handler
 	 * before. Reenable it and give it another chance.
@@ -1328,7 +1333,7 @@ static struct irqaction *__free_irq(unsi
 			return NULL;
 		}
 
-		if (action->dev_id == dev_id)
+		if (action->dev_id == dev_id || irq_pm_saved_id(action, dev_id))
 			break;
 		action_ptr = &action->next;
 	}
@@ -1336,6 +1341,8 @@ static struct irqaction *__free_irq(unsi
 	/* Found it - now remove it from the list of entries: */
 	*action_ptr = action->next;
 
+	irq_pm_cleanup(desc, action);
+
 	/* If this was the last handler, shut down the IRQ line: */
 	if (!desc->action) {
 		irq_shutdown(desc);
Index: linux-pm/kernel/irq/pm.c
===================================================================
--- linux-pm.orig/kernel/irq/pm.c
+++ linux-pm/kernel/irq/pm.c
@@ -9,10 +9,96 @@
 #include <linux/irq.h>
 #include <linux/module.h>
 #include <linux/interrupt.h>
+#include <linux/suspend.h>
 #include <linux/syscore_ops.h>
 
 #include "internals.h"
 
+static void irq_pm_restore_handler(struct irqaction *action)
+{
+	if (action->s_handler) {
+		action->handler = action->s_handler;
+		action->s_handler = NULL;
+		action->dev_id = action->s_dev_id;
+		action->s_dev_id = NULL;
+	}
+}
+
+static void irq_pm_disable_and_wakeup(int irq)
+{
+	struct irq_desc *desc = irq_to_desc(irq);
+
+	desc->istate |= IRQS_SUSPENDED;
+	desc->depth++;
+	irq_disable(desc);
+	pm_system_wakeup();
+}
+
+static irqreturn_t irq_suspend_mode_handler(int irq, void *dev_id)
+{
+	struct irqaction *action = dev_id;
+	struct irqaction *next = action->next;
+	irqreturn_t ret = (action->flags & IRQF_NO_SUSPEND) ?
+		action->s_handler(irq, action->s_dev_id) : IRQ_NONE;
+
+	if (next) {
+		/* Propagate the return code. */
+		next->prev_ret = ret | action->prev_ret;
+	} else if ((ret | action->prev_ret) == IRQ_NONE) {
+		/*
+		 * This is the last action is this chain, and all of the
+		 * handlers returned IRQ_NONE.  This means an unhandled
+		 * interrupt during system suspend, so disable the IRQ and
+		 * trigger wakeup.
+		 */
+		pr_err("IRQ %d: Unhandled while suspended\n", irq);
+		irq_pm_disable_and_wakeup(irq);
+	}
+	return ret;
+}
+
+bool irq_pm_suspend_mode(struct irq_desc *desc)
+{
+	struct irqaction *action;
+
+	if (!desc->skip_suspend_depth)
+		return false;
+
+	/* No need to replace the handler if the IRQ is not shared. */
+	if (!desc->action->next)
+		return true;
+
+	for (action = desc->action; action; action = action->next) {
+		action->s_handler = action->handler;
+		action->handler = irq_suspend_mode_handler;
+		action->s_dev_id = action->dev_id;
+		action->dev_id = action;
+		action->prev_ret = IRQ_NONE;
+	}
+	return true;
+}
+
+void irq_pm_normal_mode(struct irq_desc *desc)
+{
+	struct irqaction *action;
+
+	for (action = desc->action; action; action = action->next)
+		irq_pm_restore_handler(action);
+}
+
+void irq_pm_setup(struct irq_desc *desc, struct irqaction *action)
+{
+	if (action->flags & IRQF_NO_SUSPEND)
+		desc->skip_suspend_depth++;
+}
+
+void irq_pm_cleanup(struct irq_desc *desc, struct irqaction *action)
+{
+	irq_pm_restore_handler(action);
+	if (action->flags & IRQF_NO_SUSPEND)
+		desc->skip_suspend_depth--;
+}
+
 /**
  * suspend_device_irqs - disable all currently enabled interrupt lines
  *
@@ -35,8 +121,7 @@ void suspend_device_irqs(void)
 	}
 
 	for_each_irq_desc(irq, desc)
-		if (desc->istate & IRQS_SUSPENDED)
-			synchronize_irq(irq);
+		synchronize_irq(irq);
 }
 EXPORT_SYMBOL_GPL(suspend_device_irqs);
 
Index: linux-pm/kernel/irq/internals.h
===================================================================
--- linux-pm.orig/kernel/irq/internals.h
+++ linux-pm/kernel/irq/internals.h
@@ -194,3 +194,23 @@ static inline void kstat_incr_irqs_this_
 	__this_cpu_inc(*desc->kstat_irqs);
 	__this_cpu_inc(kstat.irqs_sum);
 }
+
+#ifdef CONFIG_PM_SLEEP
+static inline bool irq_pm_saved_id(struct irqaction *action, void *dev_id)
+{
+	return action->s_dev_id == dev_id;
+}
+extern void irq_pm_setup(struct irq_desc *desc, struct irqaction *action);
+extern void irq_pm_cleanup(struct irq_desc *desc, struct irqaction *action);
+extern bool irq_pm_suspend_mode(struct irq_desc *desc);
+extern void irq_pm_normal_mode(struct irq_desc *desc);
+#else
+static inline bool irq_pm_saved_id(struct irqaction *action, void *dev_id)
+{
+	return false;
+}
+static inline void irq_pm_setup(struct irq_desc *desc, struct irqaction *action) {}
+static inline void irq_pm_cleanup(struct irq_desc *desc, struct irqaction *action) {}
+static inline bool irq_pm_suspend_mode(struct irq_desc *desc) { return false; }
+static inline void irq_pm_normal_mode(struct irq_desc *desc) {}
+#endif

[PATCH 3/6 v2] irq / PM: Make wakeup interrupts work with suspend-to-idle

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Make IRQs enabled for system wakeup via enable_irq_wake() wake up
the system from suspend-to-idle.

For this purpose, introduce a new routine, wakeup_mode_for_irqs(),
for switching wakeup IRQs into a special "wakeup mode" and back from
it and make freeze_enter() call it to turn the "wakeup mode" on
before starting the suspend-to-idle loop and to turn it off after
that loop has been terminated.

The "wakeup mode" switch works by replaceing interrupt handlers for
all wakeup IRQs and all their irqactions with a special "wakeup mode"
handler and enabling all wakeup IRQs, previously disabled by
suspend_device_irqs().  The "wakeup mode" interrupt handler returns
IRQ_NONE for all irqactions except for the last one in the given chain
and for that one it disables the IRQ, marks it as "suspended" and
pending and triggers system wakeup.

The switch back from the "wakeup mode" restores the original
interrupt handlers for wakeup IRQs and disables them so that they
are in the state that they were put into by suspend_device_irqs().

As a result, when in suspend-to-idle, every wakeup interrupt will
trigger a system wakeup, but the original interrupt handlers will not
be invoked for wakeup interrupts.

The line of reasoning leading to that is as follows.

The way suspend_device_irqs() works and the existing code in
check_wakeup_irqs(), called by syscore_suspend(), imply that:

  (1) Interrupt handlers are not invoked for wakeup interrupts
      after suspend_device_irqs().

  (2) System wakeup interrups after suspend_device_irqs() cause
      full system suspends to be aborted.

In addition to the above, there is the requirement that

  (3) System wakeup interrupts should wake up the system from
      suspend-to-idle.

It immediately follows from (1) and (2) that no effort is made to
distinguish "genuine" wakeup interrupts from "spurious" ones.  They
all are treated in the same way.  Since (3) means that "genuine"
wakeup interrupts are supposed to wake up the system from
suspend-to-idle too, consistency with (1) and (2) requires that
"spurious" wakeup interrupts should do the same thing.  That also is
consistent with the rule

  (4) Unhandled interrupts after suspend_device_irqs() abort system
      suspends in progress (or wake up the system from suspend-to-idle
      if already there).

introduced by the previous changeset and implies that there is no
reason to invoke interrupt handlers for wakeup interrups after
suspend_device_irqs() in the suspend-to-idle case.  Moreover, doing
so would go against rule (1).

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 include/linux/interrupt.h |    5 +-
 kernel/irq/pm.c           |   84 +++++++++++++++++++++++++++++++++++++++-------
 kernel/power/suspend.c    |    3 +
 3 files changed, 79 insertions(+), 13 deletions(-)

Index: linux-pm/include/linux/interrupt.h
===================================================================
--- linux-pm.orig/include/linux/interrupt.h
+++ linux-pm/include/linux/interrupt.h
@@ -101,8 +101,8 @@ typedef irqreturn_t (*irq_handler_t)(int
  * @thread_flags:	flags related to @thread
  * @thread_mask:	bitmask for keeping track of @thread activity
  * @dir:	pointer to the proc/irq/NN/name entry
- * @s_handler:	original interrupt handler for suspend mode interrupts
- * @s_dev_id:	original device identification cookie for suspend mode
+ * @s_handler:	original interrupt handler for suspend/wakeup mode interrupts
+ * @s_dev_id:	original device identification cookie for suspend/wakeup mode
  * @prev_ret:	suspend mode return code from the previous action
  */
 struct irqaction {
@@ -201,6 +201,7 @@ extern void irq_wake_thread(unsigned int
 /* The following three functions are for the core kernel use only. */
 extern void suspend_device_irqs(void);
 extern void resume_device_irqs(void);
+extern void wakeup_mode_for_irqs(bool enable);
 #ifdef CONFIG_PM_SLEEP
 extern int check_wakeup_irqs(void);
 #else
Index: linux-pm/kernel/power/suspend.c
===================================================================
--- linux-pm.orig/kernel/power/suspend.c
+++ linux-pm/kernel/power/suspend.c
@@ -28,6 +28,7 @@
 #include <linux/ftrace.h>
 #include <trace/events/power.h>
 #include <linux/compiler.h>
+#include <linux/interrupt.h>
 
 #include "power.h"
 
@@ -55,7 +56,9 @@ static void freeze_enter(void)
 {
 	cpuidle_use_deepest_state(true);
 	cpuidle_resume();
+	wakeup_mode_for_irqs(true);
 	wait_event(suspend_freeze_wait_head, suspend_freeze_wake);
+	wakeup_mode_for_irqs(false);
 	cpuidle_pause();
 	cpuidle_use_deepest_state(false);
 }
Index: linux-pm/kernel/irq/pm.c
===================================================================
--- linux-pm.orig/kernel/irq/pm.c
+++ linux-pm/kernel/irq/pm.c
@@ -14,6 +14,17 @@
 
 #include "internals.h"
 
+static void irq_pm_replace_handler(struct irqaction *action,
+				   irq_handler_t new_handler)
+{
+	if (!action->s_handler) {
+		action->s_handler = action->handler;
+		action->handler = new_handler;
+		action->s_dev_id = action->dev_id;
+		action->dev_id = action;
+	}
+}
+
 static void irq_pm_restore_handler(struct irqaction *action)
 {
 	if (action->s_handler) {
@@ -24,14 +35,36 @@ static void irq_pm_restore_handler(struc
 	}
 }
 
-static void irq_pm_disable_and_wakeup(int irq)
+static void irq_pm_disable_and_wakeup(struct irq_desc *desc)
 {
-	struct irq_desc *desc = irq_to_desc(irq);
+	if (!(desc->istate & IRQS_SUSPENDED)) {
+		desc->istate |= IRQS_SUSPENDED;
+		desc->depth++;
+		irq_disable(desc);
+		pm_system_wakeup();
+	}
+}
+
+static irqreturn_t irq_wakeup_mode_handler(int irq, void *dev_id)
+{
+	struct irqaction *action = dev_id;
+	struct irq_desc *desc;
+
+	if (action->next)
+		return IRQ_NONE;
+
+	desc = irq_to_desc(irq);
+	desc->istate |= IRQS_PENDING;
+	irq_pm_disable_and_wakeup(desc);
+	return IRQ_HANDLED;
+}
+
+static void irq_pm_wakeup_mode(struct irq_desc *desc)
+{
+	struct irqaction *action;
 
-	desc->istate |= IRQS_SUSPENDED;
-	desc->depth++;
-	irq_disable(desc);
-	pm_system_wakeup();
+	for (action = desc->action; action; action = action->next)
+		irq_pm_replace_handler(action, irq_wakeup_mode_handler);
 }
 
 static irqreturn_t irq_suspend_mode_handler(int irq, void *dev_id)
@@ -52,7 +85,7 @@ static irqreturn_t irq_suspend_mode_hand
 		 * trigger wakeup.
 		 */
 		pr_err("IRQ %d: Unhandled while suspended\n", irq);
-		irq_pm_disable_and_wakeup(irq);
+		irq_pm_disable_and_wakeup(irq_to_desc(irq));
 	}
 	return ret;
 }
@@ -69,10 +102,7 @@ bool irq_pm_suspend_mode(struct irq_desc
 		return true;
 
 	for (action = desc->action; action; action = action->next) {
-		action->s_handler = action->handler;
-		action->handler = irq_suspend_mode_handler;
-		action->s_dev_id = action->dev_id;
-		action->dev_id = action;
+		irq_pm_replace_handler(action, irq_suspend_mode_handler);
 		action->prev_ret = IRQ_NONE;
 	}
 	return true;
@@ -213,3 +243,35 @@ int check_wakeup_irqs(void)
 
 	return 0;
 }
+
+void wakeup_mode_for_irqs(bool enable)
+{
+	struct irq_desc *desc;
+	int irq;
+
+	for_each_irq_desc(irq, desc) {
+		struct irqaction *action = desc->action;
+		unsigned long flags;
+
+		raw_spin_lock_irqsave(&desc->lock, flags);
+
+		if (action && irqd_is_wakeup_set(&desc->irq_data) &&
+		    !desc->skip_suspend_depth) {
+			if (enable) {
+				irq_pm_wakeup_mode(desc);
+				if (desc->istate & IRQS_SUSPENDED) {
+					desc->istate &= ~IRQS_SUSPENDED;
+					__enable_irq(desc, irq, false);
+				}
+			} else {
+				if (!(desc->istate & IRQS_SUSPENDED)) {
+					__disable_irq(desc, irq, false);
+					desc->istate |= IRQS_SUSPENDED;
+				}
+				irq_pm_normal_mode(desc);
+			}
+		}
+
+		raw_spin_unlock_irqrestore(&desc->lock, flags);
+	}
+}

[PATCH 4/6 v2] x86 / PM: Set IRQCHIP_SKIP_SET_WAKE for IOAPIC IRQ chip objects

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Set the IRQCHIP_SKIP_SET_WAKE for IOAPIC IRQ chip objects so that
interrupts from them can work as wakeup interrupts for suspend-to-idle.

After this change, running enable_irq_wake() on one of the IRQs in
question will succeed and IRQD_WAKEUP_STATE will be set for it, so
all of the suspend-to-idle wakeup mechanics introduced previously
will work for it automatically.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

This patch is the same as the previous version.

---
 arch/x86/kernel/apic/io_apic.c |    5 +++++
 1 file changed, 5 insertions(+)

Index: linux-pm/arch/x86/kernel/apic/io_apic.c
===================================================================
--- linux-pm.orig/arch/x86/kernel/apic/io_apic.c
+++ linux-pm/arch/x86/kernel/apic/io_apic.c
@@ -2507,6 +2507,7 @@ static struct irq_chip ioapic_chip __rea
 	.irq_eoi		= ack_apic_level,
 	.irq_set_affinity	= native_ioapic_set_affinity,
 	.irq_retrigger		= ioapic_retrigger_irq,
+	.flags			= IRQCHIP_SKIP_SET_WAKE,
 };
 
 static inline void init_IO_APIC_traps(void)
@@ -3030,6 +3031,7 @@ static struct irq_chip msi_chip = {
 	.irq_ack		= ack_apic_edge,
 	.irq_set_affinity	= msi_set_affinity,
 	.irq_retrigger		= ioapic_retrigger_irq,
+	.flags			= IRQCHIP_SKIP_SET_WAKE,
 };
 
 int setup_msi_irq(struct pci_dev *dev, struct msi_desc *msidesc,
@@ -3128,6 +3130,7 @@ static struct irq_chip dmar_msi_type = {
 	.irq_ack		= ack_apic_edge,
 	.irq_set_affinity	= dmar_msi_set_affinity,
 	.irq_retrigger		= ioapic_retrigger_irq,
+	.flags			= IRQCHIP_SKIP_SET_WAKE,
 };
 
 int arch_setup_dmar_msi(unsigned int irq)
@@ -3178,6 +3181,7 @@ static struct irq_chip hpet_msi_type = {
 	.irq_ack = ack_apic_edge,
 	.irq_set_affinity = hpet_msi_set_affinity,
 	.irq_retrigger = ioapic_retrigger_irq,
+	.flags = IRQCHIP_SKIP_SET_WAKE,
 };
 
 int default_setup_hpet_msi(unsigned int irq, unsigned int id)
@@ -3241,6 +3245,7 @@ static struct irq_chip ht_irq_chip = {
 	.irq_ack		= ack_apic_edge,
 	.irq_set_affinity	= ht_set_affinity,
 	.irq_retrigger		= ioapic_retrigger_irq,
+	.flags			= IRQCHIP_SKIP_SET_WAKE,
 };
 
 int arch_setup_ht_irq(unsigned int irq, struct pci_dev *dev)

[PATCH 5/6 v2] PCI / PM: Make PCIe PME interrupts wake up from suspend-to-idle

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

To make PCIe PME interrupts wake up the system from suspend to idle,
make the PME driver use enable_irq_wake() on the IRQ during system
suspend (if there are any wakeup devices below the given PCIe port)
without disabling PME interrupts.  This way, an interrupt will still
trigger if a wakeup event happens and the system will be woken up (or
system suspend in progress will be aborted) by means of the new
mechanics introduced previously.

This change allows Wake-on-LAN to be used for wakeup from
suspend-to-idle on my MSI Wind tesbed netbook.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

This patch is the same as the previous version.

---
 drivers/pci/pcie/pme.c |   61 ++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 51 insertions(+), 10 deletions(-)


--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Index: linux-pm/drivers/pci/pcie/pme.c
===================================================================
--- linux-pm.orig/drivers/pci/pcie/pme.c
+++ linux-pm/drivers/pci/pcie/pme.c
@@ -41,11 +41,17 @@ static int __init pcie_pme_setup(char *s
 }
 __setup("pcie_pme=", pcie_pme_setup);
 
+enum pme_suspend_level {
+	PME_SUSPEND_NONE = 0,
+	PME_SUSPEND_WAKEUP,
+	PME_SUSPEND_NOIRQ,
+};
+
 struct pcie_pme_service_data {
 	spinlock_t lock;
 	struct pcie_device *srv;
 	struct work_struct work;
-	bool noirq; /* Don't enable the PME interrupt used by this service. */
+	enum pme_suspend_level suspend_level;
 };
 
 /**
@@ -223,7 +229,7 @@ static void pcie_pme_work_fn(struct work
 	spin_lock_irq(&data->lock);
 
 	for (;;) {
-		if (data->noirq)
+		if (data->suspend_level != PME_SUSPEND_NONE)
 			break;
 
 		pcie_capability_read_dword(port, PCI_EXP_RTSTA, &rtsta);
@@ -250,7 +256,7 @@ static void pcie_pme_work_fn(struct work
 		spin_lock_irq(&data->lock);
 	}
 
-	if (!data->noirq)
+	if (data->suspend_level == PME_SUSPEND_NONE)
 		pcie_pme_interrupt_enable(port, true);
 
 	spin_unlock_irq(&data->lock);
@@ -367,6 +373,21 @@ static int pcie_pme_probe(struct pcie_de
 	return ret;
 }
 
+static bool pcie_pme_check_wakeup(struct pci_bus *bus)
+{
+	struct pci_dev *dev;
+
+	if (!bus)
+		return false;
+
+	list_for_each_entry(dev, &bus->devices, bus_list)
+		if (device_may_wakeup(&dev->dev)
+		    || pcie_pme_check_wakeup(dev->subordinate))
+			return true;
+
+	return false;
+}
+
 /**
  * pcie_pme_suspend - Suspend PCIe PME service device.
  * @srv: PCIe service device to suspend.
@@ -375,11 +396,26 @@ static int pcie_pme_suspend(struct pcie_
 {
 	struct pcie_pme_service_data *data = get_service_data(srv);
 	struct pci_dev *port = srv->port;
+	bool wakeup;
 
+	if (device_may_wakeup(&port->dev)) {
+		wakeup = true;
+	} else {
+		down_read(&pci_bus_sem);
+		wakeup = pcie_pme_check_wakeup(port->subordinate);
+		up_read(&pci_bus_sem);
+	}
 	spin_lock_irq(&data->lock);
-	pcie_pme_interrupt_enable(port, false);
-	pcie_clear_root_pme_status(port);
-	data->noirq = true;
+	if (wakeup) {
+		enable_irq_wake(srv->irq);
+		data->suspend_level = PME_SUSPEND_WAKEUP;
+	} else {
+		struct pci_dev *port = srv->port;
+
+		pcie_pme_interrupt_enable(port, false);
+		pcie_clear_root_pme_status(port);
+		data->suspend_level = PME_SUSPEND_NOIRQ;
+	}
 	spin_unlock_irq(&data->lock);
 
 	synchronize_irq(srv->irq);
@@ -394,12 +430,17 @@ static int pcie_pme_suspend(struct pcie_
 static int pcie_pme_resume(struct pcie_device *srv)
 {
 	struct pcie_pme_service_data *data = get_service_data(srv);
-	struct pci_dev *port = srv->port;
 
 	spin_lock_irq(&data->lock);
-	data->noirq = false;
-	pcie_clear_root_pme_status(port);
-	pcie_pme_interrupt_enable(port, true);
+	if (data->suspend_level == PME_SUSPEND_NOIRQ) {
+		struct pci_dev *port = srv->port;
+
+		pcie_clear_root_pme_status(port);
+		pcie_pme_interrupt_enable(port, true);
+	} else {
+		disable_irq_wake(srv->irq);
+	}
+	data->suspend_level = PME_SUSPEND_NONE;
 	spin_unlock_irq(&data->lock);
 
 	return 0;

[PATCH 6/6 v2] irq / PM: Document rules related to system suspend and interrupts

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Add a document describing how IRQs are managed during system suspend
and resume, how to set up a wakeup interrupt and what the IRQF_NO_SUSPEND
flag is supposed to be used for.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 Documentation/power/suspend-and-interrupts.txt |  156 +++++++++++++++++++++++++
 1 file changed, 156 insertions(+)

Index: linux-pm/Documentation/power/suspend-and-interrupts.txt
===================================================================
--- /dev/null
+++ linux-pm/Documentation/power/suspend-and-interrupts.txt
@@ -0,0 +1,156 @@
+System Suspend and Device Interrupts
+
+Copyright (C) 2014 Intel Corp.
+Author: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
+
+
+Suspending and Resuming Device IRQs
+-----------------------------------
+
+Device interrupt request lines (IRQs) are generally disabled during system
+suspend after the "late" phase of suspending devices (that is, after all of the
+->prepare, ->suspend and ->suspend_late callbacks have been executed for all
+devices).  That is done by suspend_device_irqs().
+
+The rationale for doing so is that after the "late" phase of device suspend
+there is no legitimate reason why any interrupts from suspended devices should
+trigger and if any devices have not been suspended properly yet, it is better to
+block interrupts from them anyway.  Also, in the past we had problems with
+interrupt handlers for shared IRQs that device drivers using them were not
+prepared for interrupts triggering after their devices had been suspended.
+In some cases they would attempt to access, for example, memory address spaces
+of suspended devices and cause unpredictable behavior to ensue as a result.
+Unfortunately, such problems are very difficult to debug and the introduction
+of suspend_device_irqs(), along with the "noirq" phase of device suspend and
+resume, was the only practical way to prevent them from happening.
+
+Device IRQs are re-enabled during system resume, right before the "early" phase
+of resuming devices (that is, before starting to execute ->resume_early
+callbacks for devices).  The function doing that is resume_device_irqs().
+
+
+The IRQF_NO_SUSPEND Flag
+------------------------
+
+There are interrupts that can legitimately trigger during the entire system
+suspend-resume cycle, including the "noirq" phases of suspending and resuming
+devices as well as during the time when nonboot CPUs are taken offline and
+brought back online.  That applies to timer interrupts in the first place,
+but also to IPIs and to some other special-purpose interrupts, such as the ACPI
+SCI that isn't associated with any specific device and is used for signaling
+many different types of events.
+
+The IRQF_NO_SUSPEND flag is used to indicate that to the IRQ subsystem when
+requesting a special-purpose interrupt.  It causes suspend_device_irqs() to
+leave the corresponding IRQ enabled so as to allow the interrupt to work all
+the time as expected.
+
+If that IRQ is shared, it will still be left enabled by suspend_device_irqs(),
+but the interrupt handlers that were installed for it without IRQF_NO_SUSPEND
+set will not be executed after suspend_device_irqs() has returned.  Then, the
+IRQ subsystem will only execute interrupt handlers for which IRQF_NO_SUSPEND is
+set.  In that case, if the final result of handing an interrupt is IRQ_NONE, the
+IRQ subsystem will assume that the interrupt came from one of the apparently
+suspended devices that share the IRQ with an IRQF_NO_SUSPEND interrupt source
+(or more of them).  As a result, the IRQ will be disabled and marked as
+"suspended" to prevent spurious interrupts from triggering going forward.  Next
+(1) if a system suspend is in progress, it will be aborted or
+(2) if the system is in the "freeze" sleep state (suspend-to-idle), it will be
+    woken up,
+to allow the system to recover from that potentially unstable condition
+gracefully.
+
+The IRQF_NO_SUSPEND flag should not be used when requesting interrupts that
+subsequently will be configured for waking up the system from sleep states
+via enable_irq_wake() (that is, system wakeup interrupts).
+
+
+System Wakeup Interrupts, enable_irq_wake() and disable_irq_wake()
+------------------------------------------------------------------
+
+System wakeup interrupts generally need to be configured to wake up the system
+from sleep states, especially if they are used for different purposes (e.g. as
+I/O interrupts) in the working state.
+
+That may involve turning on a special signal handling logic within the platform
+(such as an SoC) so that signals from a given line are routed in a different way
+during system sleep so as to trigger a system wakeup when needed.  For example,
+the platform may include a dedicated interrupt controller used specifically for
+handling system wakeup events.  Then, if a given interrupt line is supposed to
+wake up the system from sleep sates, the corresponding input of that interrupt
+controller needs to be enabled to handle signals from the line in question.
+After wakeup, it generally is better to disable that input to prevent the
+dedicated controller from triggering interrupts unnecessarily.
+
+The IRQ subsystem provides two helper functions to be used by device drivers for
+those purposes.  Namely, enable_irq_wake() turns on the platform's logic for
+handling the given IRQ as a system wakeup interrupt line and disable_irq_wake()
+turns that logic off.
+
+Calling enable_irq_wake() doesn't prevent the working-state logic for handling
+the given IRQ from being disabled by suspend_device_irqs(), so after the "late"
+phase of suspending devices the IRQ can only be expected to work as a system
+wakeup interrupt line.  The IRQ subsystem checks if there are any pending
+interrupts on those lines by calling check_wakeup_irqs() at the last (syscore)
+stage of full system suspend.  If there are any, it aborts the system suspend
+by returning -EBUSY from that function.  It does not, however, invoke any
+interrupt handlers for system wakeup IRQs after suspend_device_irqs().
+
+
+System Wakeup Interrupts and Suspend-to-Idle
+--------------------------------------------
+
+Suspend-to-idle (also known as the "freeze" sleep state) is a relatively new
+system sleep state that works by idling all of the processors and waiting for
+interrupts right after the "noirq" phase of suspending devices.
+
+Of course, this means that all of the interrupts with the IRQF_NO_SUSPEND flag
+set will bring CPUs out of idle while in that state, but they will not cause the
+IRQ subsystem to trigger a system wakeup, unless the final result of handling an
+interrupt for a shared IRQ is IRQ_NONE.
+
+System wakeup interrupts, in turn, are generally expected to trigger wakeup from
+system sleep states, including suspend-to-idle.  For this reason, all of the
+IRQs configured for system wakeup with enable_irq_wake(), previously disabled
+by suspend_device_irqs(), are re-enabled right before starting the final "go to
+an idle state and wait for an interrupt" loop of suspend-to-idle.  However, they
+are also reconfigured so that the original handlers associated with them will
+not be invoked at that time.  Those interrupt handlers are all temporarily
+replaced with a special "wakeup mode" interrupt handler that disables the IRQ,
+marks it as "suspended" and pending and triggers wakeup from suspend-to-idle.
+
+The rationale here is to keep the behavior consistent with the previously
+existing way of handling wakeup interrupts during full system suspends, which
+is that (a) interrupt handlers are not executed for them after
+suspend_device_irqs() and (b) full system suspends are aborted if any of them
+are received after suspend_device_irqs().  Combined with the requirement that
+system wakeup interrupts wake up the system from suspend-to-idle, (a) and (b)
+imply that interrupt handlers should not be invoked for wakeup interrupts in
+suspend-to-idle, although those interrupts should still wake up the system from
+that state.  The IRQs that triggered wakeup also need to be marked as pending
+to avoid losing wakeup events.
+
+When wakeup from suspend-to-idle is triggered, the wakeup IRQs are disabled
+again and their original behavior is restored.  They are subsequently re-enabled
+by resume_device_irqs(), as usual.
+
+
+IRQF_NO_SUSPEND and enable_irq_wake()
+-------------------------------------
+
+There are no valid reasons to use both enable_irq_wake() and the IRQF_NO_SUSPEND
+flag on the same IRQ.
+
+First of all, if the IRQ is not shared, the rules for handling IRQF_NO_SUSPEND
+interrupts (interrupt handlers are invoked after suspend_device_irqs()) are
+directly at odds with the rules for handling system wakeup interrupts (interrupt
+handlers are not invoked after suspend_device_irqs()).
+
+Secondly, enable_irq_wake() applies to entire IRQs and not to individual
+interrupt handlers, so sharing an IRQ between a system wakeup interrupt source
+and an IRQF_NO_SUSPEND interrupt source does not make sense in principle.  If
+attempted, it leads to situations in which genuine wakeup interrups are lost,
+because they are regarded as interrupts from the IRQF_NO_SUSPEND interrupt
+source, or are reported as unhandled interrupts, depending on the implementation
+of the interrupt handler for that source, the timing of events and other
+factors.

[PATCH 0/5 v3] irq / PM: Suspend-to-idle wakeup interrupts

[Rafael J. Wysocki]

Hi,

On Monday, August 11, 2014 03:56:46 PM Rafael J. Wysocki wrote:
> Hi,
> 
> I thought I'd just refresh the previous patchset, but in the meantime I found
> a way to avoid adding overhead to note_interrupt(), so I decided to change the
> approach.
> 
> Patch [2/6] fixes the problem with IRQF_NO_SUSPEND and shared interrupts.  The
> idea is to use wrapper handlers for all irqactions of the shared IRQs where at
> least one of them has IRQF_NO_SUSPEND set.  Since the wrapper handler is now
> installed for all irqactions in the given irq_desc, it can track the return
> codes from the original handlers and "suspend" the IRQ if the interrupt is going
> to be unhandled eventually.  All described in the changelog.  This is independent
> of the handling of wakeup interrupts for suspend-to-idle.
> 
> Patch [3/6] makes IRQs configured for system wakeup with enable_irq_wake() work
> for suspend to idle.  It does that by using a (different) wrapper handler for
> all irqactions in the wakeup irq_descs and that wrapper handler takes care of
> all the ugly details.  Again, all described in the changelog.
> 
> Patches [1/6], [4-5/6] are unmodified and the last one addes documentation.

It has occured to me recently that trying to address two different issues at
once in one patchset may be more confusing than it has to be, so the following
series has one and only one purpose which is to make enable_irq_wake() interrupts
wake up from suspend-to-idle while avoiding the shared interrupts pain.

The way it works is that for each IRQD_WAKEUP_STATE IRQ (a) all of the interrupt
handlers are substituted for by a special "wakeup mode" handler and (b) the
IRQ is re-enabled right before entering the suspend-to-idle loop.  After a wakeup
interrupt all is restored to the previous state left by suspend_device_irqs().

The "wakeup mode" handler triggers a system wakeup, disables the IRQ and marks
it as "suspended" and "pending".

To me, all of this is relatively straightforward and the handling of
IRQF_NO_SUSPEND for shared interrupts, which is a separate problem, can be
addressed on top of it later (make no mistake, I still think that it should be
addressed).

The patches are as follows:

[1/5] Mechanism to wake up the system or abort suspend in progress automatically.
[2/5] Wakeup interrupts support for suspend-to-idle.
[3/5] Set IRQCHIP_SKIP_SET_WAKE for x86 IOAPIC IRQ chips.
[4/5] Make PCIe PME wake up from suspend to idle.
[5/5] Documentation.

Patches [1/5] and [3-4/5] have not been changed since I posted them previously
and patch [5/5] has been updated to reflect the focus on wakeup (I removed the
paragraph on IRQF_NO_SUSPEND vs shared interrupts from it).

All tested on MSI Wind.

Rafael

[PATCH 1/5 v3] PM / sleep: Mechanism for aborting system suspends unconditionally

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

It sometimes may be necessary to abort a system suspend in
progress or wake up the system from suspend-to-idle even if the
pm_wakeup_event()/pm_stay_awake() mechanism is not enabled.

For this purpose, introduce a new global variable abort_suspend and
make pm_wakeup_pending() check its value.  Also add routines for
manipulating that variable.

This is going to be used in subsequent changes.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/base/power/wakeup.c |   20 ++++++++++++++++++++
 include/linux/suspend.h     |    4 ++++
 kernel/power/process.c      |    1 +
 3 files changed, 25 insertions(+)

Index: linux-pm/include/linux/suspend.h
===================================================================
--- linux-pm.orig/include/linux/suspend.h
+++ linux-pm/include/linux/suspend.h
@@ -371,6 +371,8 @@ extern int unregister_pm_notifier(struct
 extern bool events_check_enabled;
 
 extern bool pm_wakeup_pending(void);
+extern void pm_system_wakeup(void);
+extern void pm_wakeup_clear(void);
 extern bool pm_get_wakeup_count(unsigned int *count, bool block);
 extern bool pm_save_wakeup_count(unsigned int count);
 extern void pm_wakep_autosleep_enabled(bool set);
@@ -418,6 +420,8 @@ static inline int unregister_pm_notifier
 #define pm_notifier(fn, pri)	do { (void)(fn); } while (0)
 
 static inline bool pm_wakeup_pending(void) { return false; }
+static inline void pm_system_wakeup(void) {}
+static inline void pm_wakeup_clear(void) {}
 
 static inline void lock_system_sleep(void) {}
 static inline void unlock_system_sleep(void) {}
Index: linux-pm/drivers/base/power/wakeup.c
===================================================================
--- linux-pm.orig/drivers/base/power/wakeup.c
+++ linux-pm/drivers/base/power/wakeup.c
@@ -691,6 +691,8 @@ void pm_print_active_wakeup_sources(void
 }
 EXPORT_SYMBOL_GPL(pm_print_active_wakeup_sources);
 
+static bool abort_suspend;
+
 /**
  * pm_wakeup_pending - Check if power transition in progress should be aborted.
  *
@@ -712,6 +714,7 @@ bool pm_wakeup_pending(void)
 		ret = (cnt != saved_count || inpr > 0);
 		events_check_enabled = !ret;
 	}
+	ret = ret || abort_suspend;
 	spin_unlock_irqrestore(&events_lock, flags);
 
 	if (ret) {
@@ -722,6 +725,23 @@ bool pm_wakeup_pending(void)
 	return ret;
 }
 
+void pm_system_wakeup(void)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&events_lock, flags);
+	abort_suspend = true;
+	spin_unlock_irqrestore(&events_lock, flags);
+	freeze_wake();
+}
+
+void pm_wakeup_clear(void)
+{
+	spin_lock_irq(&events_lock);
+	abort_suspend = false;
+	spin_unlock_irq(&events_lock);
+}
+
 /**
  * pm_get_wakeup_count - Read the number of registered wakeup events.
  * @count: Address to store the value at.
Index: linux-pm/kernel/power/process.c
===================================================================
--- linux-pm.orig/kernel/power/process.c
+++ linux-pm/kernel/power/process.c
@@ -129,6 +129,7 @@ int freeze_processes(void)
 	if (!pm_freezing)
 		atomic_inc(&system_freezing_cnt);
 
+	pm_wakeup_clear();
 	printk("Freezing user space processes ... ");
 	pm_freezing = true;
 	error = try_to_freeze_tasks(true);

[PATCH 2/5 v3] irq / PM: Make wakeup interrupts work with suspend-to-idle

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Make IRQs enabled for system wakeup via enable_irq_wake() wake up
the system from suspend-to-idle.

For this purpose, introduce a new routine, wakeup_mode_for_irqs(),
for switching wakeup IRQs into a special "wakeup mode" and back from
it and make freeze_enter() call it to turn the "wakeup mode" on
before starting the suspend-to-idle loop and to turn it off after
that loop has been terminated.

The "wakeup mode" switch works by substituting a special "wakeup mode"
interrupt handler for all interrupt handlers in all irqactions of all
wakeup IRQs and enabling those IRQs, previously disabled by
suspend_device_irqs().  The "wakeup mode" interrupt handler returns
IRQ_NONE for all irqactions except for the last one in the given chain
and for that one it disables the IRQ, marks it as "suspended" and
pending and triggers a system wakeup.

The switch back from the "wakeup mode" restores the original
interrupt handlers for wakeup IRQs and disables them so that they
are in the state that they were put into by suspend_device_irqs().

As a result, when in suspend-to-idle, every wakeup interrupt will
trigger a system wakeup, but the original interrupt handlers will not
be invoked for those interrupts.

The line of reasoning leading to that is as follows.

The way suspend_device_irqs() works and the existing code in
check_wakeup_irqs(), called by syscore_suspend(), imply that:

  (1) Interrupt handlers are not invoked for wakeup interrupts
      after suspend_device_irqs().

  (2) All interrups from system wakeup IRQs received after\
      suspend_device_irqs() cause full system suspends to be aborted.

In addition to the above, there is the requirement that

  (3) System wakeup interrupts should wake up the system from
      suspend-to-idle.

It immediately follows from (1) and (2) that no effort is made to
distinguish "genuine" wakeup interrupts from "spurious" ones.  They
all are treated in the same way.  Since (3) means that "genuine"
wakeup interrupts are supposed to wake up the system from
suspend-to-idle too, consistency with (1) and (2) requires that
"spurious" wakeup interrupts should do the same thing.  Thus there is
no reason to invoke interrupt handlers for wakeup interrups after
suspend_device_irqs() in the suspend-to-idle case.  Moreover, doing
so would go against rule (1).

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 include/linux/interrupt.h |    7 +++
 kernel/irq/internals.h    |   14 +++++++
 kernel/irq/manage.c       |    4 +-
 kernel/irq/pm.c           |   85 ++++++++++++++++++++++++++++++++++++++++++++++
 kernel/power/suspend.c    |    3 +
 5 files changed, 112 insertions(+), 1 deletion(-)

Index: linux-pm/include/linux/interrupt.h
===================================================================
--- linux-pm.orig/include/linux/interrupt.h
+++ linux-pm/include/linux/interrupt.h
@@ -101,6 +101,8 @@ typedef irqreturn_t (*irq_handler_t)(int
  * @thread_flags:	flags related to @thread
  * @thread_mask:	bitmask for keeping track of @thread activity
  * @dir:	pointer to the proc/irq/NN/name entry
+ * @s_handler:	original interrupt handler for wakeup mode interrupts
+ * @s_dev_id:	original device identification cookie for wakeup mode
  */
 struct irqaction {
 	irq_handler_t		handler;
@@ -115,6 +117,10 @@ struct irqaction {
 	unsigned long		thread_mask;
 	const char		*name;
 	struct proc_dir_entry	*dir;
+#ifdef CONFIG_PM_SLEEP
+	irq_handler_t		s_handler;
+	void			*s_dev_id;
+#endif
 } ____cacheline_internodealigned_in_smp;
 
 extern irqreturn_t no_action(int cpl, void *dev_id);
@@ -193,6 +199,7 @@ extern void irq_wake_thread(unsigned int
 /* The following three functions are for the core kernel use only. */
 extern void suspend_device_irqs(void);
 extern void resume_device_irqs(void);
+extern void wakeup_mode_for_irqs(bool enable);
 #ifdef CONFIG_PM_SLEEP
 extern int check_wakeup_irqs(void);
 #else
Index: linux-pm/kernel/irq/internals.h
===================================================================
--- linux-pm.orig/kernel/irq/internals.h
+++ linux-pm/kernel/irq/internals.h
@@ -194,3 +194,17 @@ static inline void kstat_incr_irqs_this_
 	__this_cpu_inc(*desc->kstat_irqs);
 	__this_cpu_inc(kstat.irqs_sum);
 }
+
+#ifdef CONFIG_PM_SLEEP
+static inline bool irq_pm_saved_id(struct irqaction *action, void *dev_id)
+{
+	return action->s_dev_id == dev_id;
+}
+extern void irq_pm_restore_handler(struct irqaction *action);
+#else
+static inline bool irq_pm_saved_id(struct irqaction *action, void *dev_id)
+{
+	return false;
+}
+static inline void irq_pm_restore_handler(struct irqaction *action) {}
+#endif
Index: linux-pm/kernel/irq/manage.c
===================================================================
--- linux-pm.orig/kernel/irq/manage.c
+++ linux-pm/kernel/irq/manage.c
@@ -1328,7 +1328,7 @@ static struct irqaction *__free_irq(unsi
 			return NULL;
 		}
 
-		if (action->dev_id == dev_id)
+		if (action->dev_id == dev_id || irq_pm_saved_id(action, dev_id))
 			break;
 		action_ptr = &action->next;
 	}
@@ -1336,6 +1336,8 @@ static struct irqaction *__free_irq(unsi
 	/* Found it - now remove it from the list of entries: */
 	*action_ptr = action->next;
 
+	irq_pm_restore_handler(action);
+
 	/* If this was the last handler, shut down the IRQ line: */
 	if (!desc->action) {
 		irq_shutdown(desc);
Index: linux-pm/kernel/irq/pm.c
===================================================================
--- linux-pm.orig/kernel/irq/pm.c
+++ linux-pm/kernel/irq/pm.c
@@ -9,10 +9,95 @@
 #include <linux/irq.h>
 #include <linux/module.h>
 #include <linux/interrupt.h>
+#include <linux/suspend.h>
 #include <linux/syscore_ops.h>
 
 #include "internals.h"
 
+void irq_pm_restore_handler(struct irqaction *action)
+{
+	if (action->s_handler) {
+		action->handler = action->s_handler;
+		action->s_handler = NULL;
+		action->dev_id = action->s_dev_id;
+		action->s_dev_id = NULL;
+	}
+}
+
+static void irq_pm_substitute_handler(struct irqaction *action,
+				      irq_handler_t new_handler)
+{
+	if (!action->s_handler) {
+		action->s_handler = action->handler;
+		action->handler = new_handler;
+		action->s_dev_id = action->dev_id;
+		action->dev_id = action;
+	}
+}
+
+static irqreturn_t irq_wakeup_mode_handler(int irq, void *dev_id)
+{
+	struct irqaction *action = dev_id;
+	struct irq_desc *desc;
+
+	if (action->next)
+		return IRQ_NONE;
+
+	desc = irq_to_desc(irq);
+	desc->istate |= IRQS_SUSPENDED | IRQS_PENDING;
+	desc->depth++;
+	irq_disable(desc);
+	pm_system_wakeup();
+	return IRQ_HANDLED;
+}
+
+static void irq_pm_wakeup_mode(struct irq_desc *desc)
+{
+	struct irqaction *action;
+
+	for (action = desc->action; action; action = action->next)
+		irq_pm_substitute_handler(action, irq_wakeup_mode_handler);
+}
+
+static void irq_pm_normal_mode(struct irq_desc *desc)
+{
+	struct irqaction *action;
+
+	for (action = desc->action; action; action = action->next)
+		irq_pm_restore_handler(action);
+}
+
+void wakeup_mode_for_irqs(bool enable)
+{
+	struct irq_desc *desc;
+	int irq;
+
+	for_each_irq_desc(irq, desc) {
+		struct irqaction *action = desc->action;
+		unsigned long flags;
+
+		raw_spin_lock_irqsave(&desc->lock, flags);
+
+		if (action && irqd_is_wakeup_set(&desc->irq_data)) {
+			if (enable) {
+				if (desc->istate & IRQS_SUSPENDED) {
+					irq_pm_wakeup_mode(desc);
+					desc->istate &= ~IRQS_SUSPENDED;
+					__enable_irq(desc, irq, false);
+				}
+			} else {
+				if (!(desc->istate & IRQS_SUSPENDED)) {
+					__disable_irq(desc, irq, false);
+					desc->istate |= IRQS_SUSPENDED;
+				}
+				irq_pm_normal_mode(desc);
+			}
+		}
+
+		raw_spin_unlock_irqrestore(&desc->lock, flags);
+	}
+}
+
 /**
  * suspend_device_irqs - disable all currently enabled interrupt lines
  *
Index: linux-pm/kernel/power/suspend.c
===================================================================
--- linux-pm.orig/kernel/power/suspend.c
+++ linux-pm/kernel/power/suspend.c
@@ -28,6 +28,7 @@
 #include <linux/ftrace.h>
 #include <trace/events/power.h>
 #include <linux/compiler.h>
+#include <linux/interrupt.h>
 
 #include "power.h"
 
@@ -55,7 +56,9 @@ static void freeze_enter(void)
 {
 	cpuidle_use_deepest_state(true);
 	cpuidle_resume();
+	wakeup_mode_for_irqs(true);
 	wait_event(suspend_freeze_wait_head, suspend_freeze_wake);
+	wakeup_mode_for_irqs(false);
 	cpuidle_pause();
 	cpuidle_use_deepest_state(false);
 }

Re: [PATCH 2/5 v3] irq / PM: Make wakeup interrupts work with suspend-to-idle

[Thomas Gleixner]

On Wed, 27 Aug 2014, Rafael J. Wysocki wrote:
> The line of reasoning leading to that is as follows.
> 
> The way suspend_device_irqs() works and the existing code in
> check_wakeup_irqs(), called by syscore_suspend(), imply that:
> 
>   (1) Interrupt handlers are not invoked for wakeup interrupts
>       after suspend_device_irqs().
> 
>   (2) All interrups from system wakeup IRQs received after\
>       suspend_device_irqs() cause full system suspends to be aborted.
> 
> In addition to the above, there is the requirement that
> 
>   (3) System wakeup interrupts should wake up the system from
>       suspend-to-idle.
> 
> It immediately follows from (1) and (2) that no effort is made to
> distinguish "genuine" wakeup interrupts from "spurious" ones.  They
> all are treated in the same way.  Since (3) means that "genuine"
> wakeup interrupts are supposed to wake up the system from
> suspend-to-idle too, consistency with (1) and (2) requires that
> "spurious" wakeup interrupts should do the same thing.  Thus there is
> no reason to invoke interrupt handlers for wakeup interrups after
> suspend_device_irqs() in the suspend-to-idle case.  Moreover, doing
> so would go against rule (1).

I agree with that, but I disagree with the implementation.

We now have two separate mechanisms to abort suspend:

1) The existing suspend_device_irqs() / check_wakeup_irqs() 

2) The new suspend_device_irqs() /
   reenable_stuff_and_fiddle_with_irq_action()

So why do we need those two mechanisms in the first place?

AFAICT there is no reason why we cant use the abort_suspend mechanics
to replace the suspend_device_irqs() / check_wakeup_irqs() pair.

All it needs is to do the handler substitution in
suspend_device_irqs() right away and replace the loop in
check_wakeup_irqs() with a check for abort_suspend == true. The roll
back of the handler substitution can happen in resume_device_irqs()
for both scenarios.

Aside of that the whole irqaction based substitution is silly. What's
wrong with doing it at the real interrupt handler level?

static void handle_wakeup_irq(unsigned int irq, struct irq_desc *desc)
{
	raw_spin_lock(&desc->lock);

	desc->istate |= IRQS_SUSPENDED | IRQS_PENDING;
	desc->depth++;
	irq_disable(desc);
	pm_system_wakeup();

	raw_spin_unlock(&desc->lock);
}

void suspend_device_irqs(void)
{
	for_each_irq_desc(irq, desc) {
		/* Disable the interrupt unconditionally */	       
		disable_irq(irq);

		/* Is the irq a wakeup source? */
		if (!irqd_is_wakeup_set(&desc->irq_data))
			continue;

		/* Replace the handler */
		raw_spin_lock_irqsave(&desc->lock, flags);
	     	desc->saved_handler = desc->handler;
		desc->handler = handle_wakeup_irq;
		raw_spin_unlock_irqrestore(&desc->lock, flags);

		/* Reenable the wakeup irq */
		enable_irq(irq);
	}
}

/* Move that into the pm core code */
bool check_wakeup_irqs(void)
{
	return abort_suspend;
}

void resume_device_irqs(void)
{
	for_each_irq_desc(irq, desc) {

		/* Prevent the wakeup handler from running */
		disable_irq();

		raw_spin_lock_irqsave(&desc->lock, flags);

		/* Do we need to restore the handler? */
		if (desc->handler == handle_wakeup_irq)
		   	desc->handler = desc->saved_handler;

		/* Is the irq a wakeup source? */
		if (!irqd_is_wakeup_set(&desc->irq_data))
		   	__enable_irq(irq, desc);

		/* Did it get disabled in the wakeup handler? */
		else if (desc->istate & IRQS_SUSPENDED)
		   	__enable_irq(irq, desc);

		raw_spin_unlock_irqrestore(&desc->lock, flags);

		enable_irq();
	}
}

Hmm?

One thing we might think about is having flow specific
handle_wakeup_irq variants as some hardware might require an ack or
eoi, but that's a simple to solve problem and way simpler than
fiddling with the irqaction chain and avoids the whole mess of
sprinkling irq_pm_saved_id() and irq_pm_restore_handler() calls all
over the place. I wonder why you added them to __free_irq() at all,
but no, we dont want that.

Thanks,

	tglx

Re: [PATCH 2/5 v3] irq / PM: Make wakeup interrupts work with suspend-to-idle

[Rafael J. Wysocki]

On Wednesday, August 27, 2014 10:32:23 PM Thomas Gleixner wrote:
> On Wed, 27 Aug 2014, Rafael J. Wysocki wrote:
> > The line of reasoning leading to that is as follows.
> > 
> > The way suspend_device_irqs() works and the existing code in
> > check_wakeup_irqs(), called by syscore_suspend(), imply that:
> > 
> >   (1) Interrupt handlers are not invoked for wakeup interrupts
> >       after suspend_device_irqs().
> > 
> >   (2) All interrups from system wakeup IRQs received after\
> >       suspend_device_irqs() cause full system suspends to be aborted.
> > 
> > In addition to the above, there is the requirement that
> > 
> >   (3) System wakeup interrupts should wake up the system from
> >       suspend-to-idle.
> > 
> > It immediately follows from (1) and (2) that no effort is made to
> > distinguish "genuine" wakeup interrupts from "spurious" ones.  They
> > all are treated in the same way.  Since (3) means that "genuine"
> > wakeup interrupts are supposed to wake up the system from
> > suspend-to-idle too, consistency with (1) and (2) requires that
> > "spurious" wakeup interrupts should do the same thing.  Thus there is
> > no reason to invoke interrupt handlers for wakeup interrups after
> > suspend_device_irqs() in the suspend-to-idle case.  Moreover, doing
> > so would go against rule (1).
> 
> I agree with that, but I disagree with the implementation.
> 
> We now have two separate mechanisms to abort suspend:
> 
> 1) The existing suspend_device_irqs() / check_wakeup_irqs() 
> 
> 2) The new suspend_device_irqs() /
>    reenable_stuff_and_fiddle_with_irq_action()
> 
> So why do we need those two mechanisms in the first place?
> 
> AFAICT there is no reason why we cant use the abort_suspend mechanics
> to replace the suspend_device_irqs() / check_wakeup_irqs() pair.
> 
> All it needs is to do the handler substitution in
> suspend_device_irqs() right away and replace the loop in
> check_wakeup_irqs() with a check for abort_suspend == true. The roll
> back of the handler substitution can happen in resume_device_irqs()
> for both scenarios.

We can do that of course.

> Aside of that the whole irqaction based substitution is silly. What's
> wrong with doing it at the real interrupt handler level?

Nothing I suppose. :-)

> static void handle_wakeup_irq(unsigned int irq, struct irq_desc *desc)
> {
> 	raw_spin_lock(&desc->lock);
> 
> 	desc->istate |= IRQS_SUSPENDED | IRQS_PENDING;
> 	desc->depth++;
> 	irq_disable(desc);
> 	pm_system_wakeup();
> 
> 	raw_spin_unlock(&desc->lock);
> }
> 
> void suspend_device_irqs(void)
> {
> 	for_each_irq_desc(irq, desc) {
> 		/* Disable the interrupt unconditionally */	       
> 		disable_irq(irq);

We still need to skip the IRQF_NO_SUSPEND stuff (eg. timers), so I guess
everything left disabled here needs to be IRQS_SUSPENDED, so we know which
ones to re-enable in resume_device_irqs().

> 
> 		/* Is the irq a wakeup source? */
> 		if (!irqd_is_wakeup_set(&desc->irq_data))
> 			continue;
> 
> 		/* Replace the handler */
> 		raw_spin_lock_irqsave(&desc->lock, flags);
> 	     	desc->saved_handler = desc->handler;
> 		desc->handler = handle_wakeup_irq;

Hmm.  There's no handler field in struct irq_desc (/me is puzzled).

Did you mean handle_irq (I think you did)?

> 		raw_spin_unlock_irqrestore(&desc->lock, flags);
> 
> 		/* Reenable the wakeup irq */
> 		enable_irq(irq);
> 	}
> }
> 
> /* Move that into the pm core code */
> bool check_wakeup_irqs(void)
> {
> 	return abort_suspend;
> }
> 
> void resume_device_irqs(void)
> {
> 	for_each_irq_desc(irq, desc) {
> 
> 		/* Prevent the wakeup handler from running */
> 		disable_irq();
> 
> 		raw_spin_lock_irqsave(&desc->lock, flags);
> 
> 		/* Do we need to restore the handler? */
> 		if (desc->handler == handle_wakeup_irq)
> 		   	desc->handler = desc->saved_handler;
> 
> 		/* Is the irq a wakeup source? */
> 		if (!irqd_is_wakeup_set(&desc->irq_data))
> 		   	__enable_irq(irq, desc);
> 
> 		/* Did it get disabled in the wakeup handler? */
> 		else if (desc->istate & IRQS_SUSPENDED)
> 		   	__enable_irq(irq, desc);
> 
> 		raw_spin_unlock_irqrestore(&desc->lock, flags);
> 
> 		enable_irq();
> 	}
> }
> 
> Hmm?

OK

There is quite some ugliness related to resume_irqs(), the want_early thing
and IRQF_EARLY_RESUME / IRQF_FORCE_RESUME.  I guess that needs to be preserved?

> One thing we might think about is having flow specific
> handle_wakeup_irq variants as some hardware might require an ack or
> eoi, but that's a simple to solve problem and way simpler than
> fiddling with the irqaction chain and avoids the whole mess of
> sprinkling irq_pm_saved_id() and irq_pm_restore_handler() calls all
> over the place. I wonder why you added them to __free_irq() at all,
> but no, we dont want that.

I was concerned about the (unlikely) possibility of freeing an interrupt
having a temporary handler.  Never mind.

Rafael

Re: [PATCH 2/5 v3] irq / PM: Make wakeup interrupts work with suspend-to-idle

[Thomas Gleixner]

On Thu, 28 Aug 2014, Rafael J. Wysocki wrote:
> On Wednesday, August 27, 2014 10:32:23 PM Thomas Gleixner wrote:
> > void suspend_device_irqs(void)
> > {
> > 	for_each_irq_desc(irq, desc) {
> > 		/* Disable the interrupt unconditionally */	       
> > 		disable_irq(irq);
> 
> We still need to skip the IRQF_NO_SUSPEND stuff (eg. timers), so I guess
> everything left disabled here needs to be IRQS_SUSPENDED, so we know which
> ones to re-enable in resume_device_irqs().

Right. I skipped that one for simplicity. I wanted to look into the
whole maze today again with brain awake. I think it's simple to
integrate the no suspend magic here and have a separate handler for
it.

> > 
> > 		/* Is the irq a wakeup source? */
> > 		if (!irqd_is_wakeup_set(&desc->irq_data))
> > 			continue;
> > 
> > 		/* Replace the handler */
> > 		raw_spin_lock_irqsave(&desc->lock, flags);
> > 	     	desc->saved_handler = desc->handler;
> > 		desc->handler = handle_wakeup_irq;
> 
> Hmm.  There's no handler field in struct irq_desc (/me is puzzled).
> 
> Did you mean handle_irq (I think you did)?

Yup.
 
> There is quite some ugliness related to resume_irqs(), the want_early thing
> and IRQF_EARLY_RESUME / IRQF_FORCE_RESUME.  I guess that needs to be preserved?

Probably. Did not look into the madness of that yet.

Thanks,

	tglx

Re: [PATCH 2/5 v3] irq / PM: Make wakeup interrupts work with suspend-to-idle

[Rafael J. Wysocki]

On Thursday, August 28, 2014 11:23:11 AM Thomas Gleixner wrote:
> On Thu, 28 Aug 2014, Rafael J. Wysocki wrote:
> > On Wednesday, August 27, 2014 10:32:23 PM Thomas Gleixner wrote:
> > > void suspend_device_irqs(void)
> > > {
> > > 	for_each_irq_desc(irq, desc) {
> > > 		/* Disable the interrupt unconditionally */	       
> > > 		disable_irq(irq);
> > 
> > We still need to skip the IRQF_NO_SUSPEND stuff (eg. timers), so I guess
> > everything left disabled here needs to be IRQS_SUSPENDED, so we know which
> > ones to re-enable in resume_device_irqs().
> 
> Right. I skipped that one for simplicity. I wanted to look into the
> whole maze today again with brain awake. I think it's simple to
> integrate the no suspend magic here and have a separate handler for
> it.

Well, I've already read your message about this particular thing. :-)

Before that I was about to say that I'd rather leave the no suspend stuff as
is for now until we have working wakeup IRQs for suspend-to-idle at least.

> > > 
> > > 		/* Is the irq a wakeup source? */
> > > 		if (!irqd_is_wakeup_set(&desc->irq_data))
> > > 			continue;
> > > 
> > > 		/* Replace the handler */
> > > 		raw_spin_lock_irqsave(&desc->lock, flags);
> > > 	     	desc->saved_handler = desc->handler;
> > > 		desc->handler = handle_wakeup_irq;
> > 
> > Hmm.  There's no handler field in struct irq_desc (/me is puzzled).
> > 
> > Did you mean handle_irq (I think you did)?
> 
> Yup.

So I got that to work earlier today, but with some more stuff in the handler
which doesn't look particularly generic to me.  Namely, my (working) wakeup
handler looks like this at the moment:

static void handle_wakeup_irq(unsigned int irq, struct irq_desc *desc)
{
	struct irq_chip *chip = irq_desc_get_chip(desc);

	raw_spin_lock(&desc->lock);

	desc->istate &= ~(IRQS_REPLAY | IRQS_WAITING);
	kstat_incr_irqs_this_cpu(irq, desc);

	if (irqd_irq_disabled(&desc->irq_data)) {
		desc->istate |= IRQS_PENDING;
		mask_irq(desc);
	} else {
		desc->istate |= IRQS_SUSPENDED | IRQS_PENDING;
		desc->depth++;
		irq_disable(desc);
		pm_system_wakeup();
	}

	if (chip->irq_eoi && !(chip->flags & IRQCHIP_EOI_IF_HANDLED))
		chip->irq_eoi(&desc->irq_data);

	raw_spin_unlock(&desc->lock);
}

but it only works on a particular machine with a particular wakeup interrupt.

I had to add the

	if (chip->irq_eoi && !(chip->flags & IRQCHIP_EOI_IF_HANDLED))
		chip->irq_eoi(&desc->irq_data);

check, because otherwise it hanged the system solid once the interrupt happened.

I also had to add this:


	if (irqd_irq_disabled(&desc->irq_data)) {
		desc->istate |= IRQS_PENDING;
		mask_irq(desc);
	} ...

because otherwise the IRQ didn't work correctly after resume.

However, both those things appear to be needed just because that's a fastEOI
interrupt and some other manipulations would need to be done for edge interrupts
etc.

I'm not sure, then, if we really can come up with a perfectly generic handle_irq
wakeup interrupt handler that would work with all kinds of IRQs.  It looks like
the part that could be replaced in all of them in principle is the handle_irq_event()
call, but the rest seems to be too specific to me.

So I'm not sure what to do at this point.

It is tempting to do the following:
(1) Add a handle_event callback to struct irq_desc.
(2) Rename the existing handle_irq_event() to handle_irq_event_common().
(3) Point desc->handle_event to handle_irq_event_common() for every desc at init.
(4) Make (new, possibly static inline) handle_irq_event() invoke desc->handle_event().
(5) During suspend_device_irqs() point desc->handle_event to handle_irq_event_wakeup()
    for all wakeup IRQs.
(6) During resume_device_irqs() point desc->handle_event back to handle_irq_event_common()
    for everybody.
Then, for CONFIG_PM_SLEEP unset, handle_irq_event() can be defined as
handle_irq_event_common().

Of course, that adds a function pointer dereference overhead to every interrupt
event, so I'm not sure if it's acceptable.

One alternative may be to add a handle_wakeup_irq pointer to struct irq_desc and
check in handle_irq_event() if that is present and call it instead of the usual
stuff if that's the case.

Another way may be to introduce an irq_desc flag that will be checked by
handle_irq_event() and will indicate "wakeup mode" to it, in which it will do
the simplified wakeup handling instead of the usual stuff.  This, however, like
the previous one, will add a branch to handle_irq_event() that will be checked
every time even though it only is really necessary during system suspend.

And handlers can be replaced at the irqaction level in a couple of ways too.

Rafael

[PATCH 3/5 v3] x86 / PM: Set IRQCHIP_SKIP_SET_WAKE for IOAPIC IRQ chip objects

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Set the IRQCHIP_SKIP_SET_WAKE for IOAPIC IRQ chip objects so that
interrupts from them can work as wakeup interrupts for suspend-to-idle.

After this change, running enable_irq_wake() on one of the IRQs in
question will succeed and IRQD_WAKEUP_STATE will be set for it, so
all of the suspend-to-idle wakeup mechanics introduced previously
will work for it automatically.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 arch/x86/kernel/apic/io_apic.c |    5 +++++
 1 file changed, 5 insertions(+)

Index: linux-pm/arch/x86/kernel/apic/io_apic.c
===================================================================
--- linux-pm.orig/arch/x86/kernel/apic/io_apic.c
+++ linux-pm/arch/x86/kernel/apic/io_apic.c
@@ -2618,6 +2618,7 @@ static struct irq_chip ioapic_chip __rea
 	.irq_eoi		= ack_apic_level,
 	.irq_set_affinity	= native_ioapic_set_affinity,
 	.irq_retrigger		= ioapic_retrigger_irq,
+	.flags			= IRQCHIP_SKIP_SET_WAKE,
 };
 
 static inline void init_IO_APIC_traps(void)
@@ -3168,6 +3169,7 @@ static struct irq_chip msi_chip = {
 	.irq_ack		= ack_apic_edge,
 	.irq_set_affinity	= msi_set_affinity,
 	.irq_retrigger		= ioapic_retrigger_irq,
+	.flags			= IRQCHIP_SKIP_SET_WAKE,
 };
 
 int setup_msi_irq(struct pci_dev *dev, struct msi_desc *msidesc,
@@ -3266,6 +3268,7 @@ static struct irq_chip dmar_msi_type = {
 	.irq_ack		= ack_apic_edge,
 	.irq_set_affinity	= dmar_msi_set_affinity,
 	.irq_retrigger		= ioapic_retrigger_irq,
+	.flags			= IRQCHIP_SKIP_SET_WAKE,
 };
 
 int arch_setup_dmar_msi(unsigned int irq)
@@ -3316,6 +3319,7 @@ static struct irq_chip hpet_msi_type = {
 	.irq_ack = ack_apic_edge,
 	.irq_set_affinity = hpet_msi_set_affinity,
 	.irq_retrigger = ioapic_retrigger_irq,
+	.flags = IRQCHIP_SKIP_SET_WAKE,
 };
 
 int default_setup_hpet_msi(unsigned int irq, unsigned int id)
@@ -3379,6 +3383,7 @@ static struct irq_chip ht_irq_chip = {
 	.irq_ack		= ack_apic_edge,
 	.irq_set_affinity	= ht_set_affinity,
 	.irq_retrigger		= ioapic_retrigger_irq,
+	.flags			= IRQCHIP_SKIP_SET_WAKE,
 };
 
 int arch_setup_ht_irq(unsigned int irq, struct pci_dev *dev)

[PATCH 4/5 v3] PCI / PM: Make PCIe PME interrupts wake up from suspend-to-idle

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

To make PCIe PME interrupts wake up the system from suspend to idle,
make the PME driver use enable_irq_wake() on the IRQ during system
suspend (if there are any wakeup devices below the given PCIe port)
without disabling PME interrupts.  This way, an interrupt will still
trigger if a wakeup event happens and the system will be woken up (or
system suspend in progress will be aborted) by means of the new
mechanics introduced previously.

This change allows Wake-on-LAN to be used for wakeup from
suspend-to-idle on my MSI Wind tesbed netbook.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/pci/pcie/pme.c |   61 ++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 51 insertions(+), 10 deletions(-)


--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Index: linux-pm/drivers/pci/pcie/pme.c
===================================================================
--- linux-pm.orig/drivers/pci/pcie/pme.c
+++ linux-pm/drivers/pci/pcie/pme.c
@@ -41,11 +41,17 @@ static int __init pcie_pme_setup(char *s
 }
 __setup("pcie_pme=", pcie_pme_setup);
 
+enum pme_suspend_level {
+	PME_SUSPEND_NONE = 0,
+	PME_SUSPEND_WAKEUP,
+	PME_SUSPEND_NOIRQ,
+};
+
 struct pcie_pme_service_data {
 	spinlock_t lock;
 	struct pcie_device *srv;
 	struct work_struct work;
-	bool noirq; /* Don't enable the PME interrupt used by this service. */
+	enum pme_suspend_level suspend_level;
 };
 
 /**
@@ -223,7 +229,7 @@ static void pcie_pme_work_fn(struct work
 	spin_lock_irq(&data->lock);
 
 	for (;;) {
-		if (data->noirq)
+		if (data->suspend_level != PME_SUSPEND_NONE)
 			break;
 
 		pcie_capability_read_dword(port, PCI_EXP_RTSTA, &rtsta);
@@ -250,7 +256,7 @@ static void pcie_pme_work_fn(struct work
 		spin_lock_irq(&data->lock);
 	}
 
-	if (!data->noirq)
+	if (data->suspend_level == PME_SUSPEND_NONE)
 		pcie_pme_interrupt_enable(port, true);
 
 	spin_unlock_irq(&data->lock);
@@ -367,6 +373,21 @@ static int pcie_pme_probe(struct pcie_de
 	return ret;
 }
 
+static bool pcie_pme_check_wakeup(struct pci_bus *bus)
+{
+	struct pci_dev *dev;
+
+	if (!bus)
+		return false;
+
+	list_for_each_entry(dev, &bus->devices, bus_list)
+		if (device_may_wakeup(&dev->dev)
+		    || pcie_pme_check_wakeup(dev->subordinate))
+			return true;
+
+	return false;
+}
+
 /**
  * pcie_pme_suspend - Suspend PCIe PME service device.
  * @srv: PCIe service device to suspend.
@@ -375,11 +396,26 @@ static int pcie_pme_suspend(struct pcie_
 {
 	struct pcie_pme_service_data *data = get_service_data(srv);
 	struct pci_dev *port = srv->port;
+	bool wakeup;
 
+	if (device_may_wakeup(&port->dev)) {
+		wakeup = true;
+	} else {
+		down_read(&pci_bus_sem);
+		wakeup = pcie_pme_check_wakeup(port->subordinate);
+		up_read(&pci_bus_sem);
+	}
 	spin_lock_irq(&data->lock);
-	pcie_pme_interrupt_enable(port, false);
-	pcie_clear_root_pme_status(port);
-	data->noirq = true;
+	if (wakeup) {
+		enable_irq_wake(srv->irq);
+		data->suspend_level = PME_SUSPEND_WAKEUP;
+	} else {
+		struct pci_dev *port = srv->port;
+
+		pcie_pme_interrupt_enable(port, false);
+		pcie_clear_root_pme_status(port);
+		data->suspend_level = PME_SUSPEND_NOIRQ;
+	}
 	spin_unlock_irq(&data->lock);
 
 	synchronize_irq(srv->irq);
@@ -394,12 +430,17 @@ static int pcie_pme_suspend(struct pcie_
 static int pcie_pme_resume(struct pcie_device *srv)
 {
 	struct pcie_pme_service_data *data = get_service_data(srv);
-	struct pci_dev *port = srv->port;
 
 	spin_lock_irq(&data->lock);
-	data->noirq = false;
-	pcie_clear_root_pme_status(port);
-	pcie_pme_interrupt_enable(port, true);
+	if (data->suspend_level == PME_SUSPEND_NOIRQ) {
+		struct pci_dev *port = srv->port;
+
+		pcie_clear_root_pme_status(port);
+		pcie_pme_interrupt_enable(port, true);
+	} else {
+		disable_irq_wake(srv->irq);
+	}
+	data->suspend_level = PME_SUSPEND_NONE;
 	spin_unlock_irq(&data->lock);
 
 	return 0;

[PATCH 5/5 v3] irq / PM: Document rules related to system suspend and interrupts

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Add a document describing how IRQs are managed during system suspend
and resume, how to set up a wakeup interrupt and what the IRQF_NO_SUSPEND
flag is supposed to be used for.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 Documentation/power/suspend-and-interrupts.txt |  146 +++++++++++++++++++++++++
 1 file changed, 146 insertions(+)

Index: linux-pm/Documentation/power/suspend-and-interrupts.txt
===================================================================
--- /dev/null
+++ linux-pm/Documentation/power/suspend-and-interrupts.txt
@@ -0,0 +1,146 @@
+System Suspend and Device Interrupts
+
+Copyright (C) 2014 Intel Corp.
+Author: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
+
+
+Suspending and Resuming Device IRQs
+-----------------------------------
+
+Device interrupt request lines (IRQs) are generally disabled during system
+suspend after the "late" phase of suspending devices (that is, after all of the
+->prepare, ->suspend and ->suspend_late callbacks have been executed for all
+devices).  That is done by suspend_device_irqs().
+
+The rationale for doing so is that after the "late" phase of device suspend
+there is no legitimate reason why any interrupts from suspended devices should
+trigger and if any devices have not been suspended properly yet, it is better to
+block interrupts from them anyway.  Also, in the past we had problems with
+interrupt handlers for shared IRQs that device drivers implementing them were
+not prepared for interrupts triggering after their devices had been suspended.
+In some cases they would attempt to access, for example, memory address spaces
+of suspended devices and cause unpredictable behavior to ensue as a result.
+Unfortunately, such problems are very difficult to debug and the introduction
+of suspend_device_irqs(), along with the "noirq" phase of device suspend and
+resume, was the only practical way to mitigate them.
+
+Device IRQs are re-enabled during system resume, right before the "early" phase
+of resuming devices (that is, before starting to execute ->resume_early
+callbacks for devices).  The function doing that is resume_device_irqs().
+
+
+The IRQF_NO_SUSPEND Flag
+------------------------
+
+There are interrupts that can legitimately trigger during the entire system
+suspend-resume cycle, including the "noirq" phases of suspending and resuming
+devices as well as during the time when nonboot CPUs are taken offline and
+brought back online.  That applies to timer interrupts in the first place,
+but also to IPIs and to some other special-purpose interrupts, such as the ACPI
+SCI that isn't associated with any specific device and is used for signaling
+many different types of events.
+
+The IRQF_NO_SUSPEND flag is used to indicate that to the IRQ subsystem when
+requesting a special-purpose interrupt.  It causes suspend_device_irqs() to
+leave the corresponding IRQ enabled so as to allow the interrupt to work all
+the time as expected.
+
+The IRQF_NO_SUSPEND flag should not be used when requesting interrupts that
+subsequently will be configured for waking up the system from sleep states
+via enable_irq_wake() (that is, system wakeup interrupts).
+
+
+System Wakeup Interrupts, enable_irq_wake() and disable_irq_wake()
+------------------------------------------------------------------
+
+System wakeup interrupts generally need to be configured to wake up the system
+from sleep states, especially if they are used for different purposes (e.g. as
+I/O interrupts) in the working state.
+
+That may involve turning on a special signal handling logic within the platform
+(such as an SoC) so that signals from a given line are routed in a different way
+during system sleep so as to trigger a system wakeup when needed.  For example,
+the platform may include a dedicated interrupt controller used specifically for
+handling system wakeup events.  Then, if a given interrupt line is supposed to
+wake up the system from sleep sates, the corresponding input of that interrupt
+controller needs to be enabled to receive signals from the line in question.
+After wakeup, it generally is better to disable that input to prevent the
+dedicated controller from triggering interrupts unnecessarily.
+
+The IRQ subsystem provides two helper functions to be used by device drivers for
+those purposes.  Namely, enable_irq_wake() turns on the platform's logic for
+handling the given IRQ as a system wakeup interrupt line and disable_irq_wake()
+turns that logic off.
+
+Calling enable_irq_wake() doesn't prevent the working-state logic for handling
+the given IRQ from being disabled by suspend_device_irqs(), so after the "late"
+phase of suspending devices the IRQ can only be expected to work as a system
+wakeup interrupt line.  The IRQ subsystem checks if there are any pending
+interrupts on those lines by calling check_wakeup_irqs() at the last (syscore)
+stage of full system suspend.  If there are any, it aborts the system suspend
+by returning -EBUSY from that function.  It does not, however, invoke any
+interrupt handlers for system wakeup IRQs after suspend_device_irqs().
+
+
+System Wakeup Interrupts and Suspend-to-Idle
+--------------------------------------------
+
+Suspend-to-idle (also known as the "freeze" sleep state) is a relatively new
+system sleep state that works by idling all of the processors and waiting for
+interrupts right after the "noirq" phase of suspending devices.
+
+Of course, this means that all of the interrupts with the IRQF_NO_SUSPEND flag
+set will bring CPUs out of idle while in that state, but they will not cause the
+IRQ subsystem to trigger a system wakeup.
+
+System wakeup interrupts, in turn, are generally expected to trigger wakeup from
+system sleep states, including suspend-to-idle.  For this reason, all of the
+IRQs configured for system wakeup with enable_irq_wake(), previously disabled
+by suspend_device_irqs(), are re-enabled right before starting the final "go to
+an idle state and wait for an interrupt" loop of suspend-to-idle.  However, they
+are also reconfigured so that the original handlers associated with them will
+not be invoked at that time.  Those interrupt handlers are all temporarily
+substituted for by a special "wakeup mode" interrupt handler that disables the
+IRQ, marks it as "suspended" and pending and triggers a suspend-to-idle wakeup
+event.
+
+The rationale here is to keep the behavior consistent with the existing way of
+handling wakeup interrupts during full system suspends, which is that
+
+ (a) interrupt handlers are not executed for them after suspend_device_irqs()
+
+and
+
+ (b) full system suspends are aborted if an interrupt from a wakeup IRQ line is
+     received after suspend_device_irqs().
+
+Combined with the requirement that system wakeup interrupts wake up the system
+from suspend-to-idle, (a) and (b) imply that interrupt handlers should not be
+invoked for wakeup interrupts in suspend-to-idle, although those interrupts
+should still wake up the system from that state.  The IRQs that triggered wakeup
+also need to be marked as pending to avoid losing wakeup events.
+
+When wakeup from suspend-to-idle is triggered, the wakeup IRQs are disabled
+again and their original interrupt handlers are restored.  They are subsequently
+re-enabled by resume_device_irqs(), as usual.
+
+
+IRQF_NO_SUSPEND and enable_irq_wake()
+-------------------------------------
+
+There are no valid reasons to use both enable_irq_wake() and the IRQF_NO_SUSPEND
+flag on the same IRQ.
+
+First of all, if the IRQ is not shared, the rules for handling IRQF_NO_SUSPEND
+interrupts (interrupt handlers are invoked after suspend_device_irqs()) are
+directly at odds with the rules for handling system wakeup interrupts (interrupt
+handlers are not invoked after suspend_device_irqs()).
+
+Secondly, enable_irq_wake() applies to entire IRQs and not to individual
+interrupt handlers, so sharing an IRQ between a system wakeup interrupt source
+and an IRQF_NO_SUSPEND interrupt source does not make sense in principle.  If
+attempted, it leads to situations in which genuine wakeup interrups are lost,
+because they are regarded as interrupts from the IRQF_NO_SUSPEND interrupt
+source, or are reported as unhandled interrupts, depending on the implementation
+of the interrupt handler for that source, the timing of events and other
+factors.

Re: [PATCH 0/5 v3] irq / PM: Suspend-to-idle wakeup interrupts

[Thomas Gleixner]

On Wed, 27 Aug 2014, Rafael J. Wysocki wrote:
> To me, all of this is relatively straightforward and the handling of
> IRQF_NO_SUSPEND for shared interrupts, which is a separate problem, can be
> addressed on top of it later (make no mistake, I still think that it should be
> addressed).

Why? Just because there is enough idiotic code using it?

Let's look at the usage sites:

sound/soc/codecs/twl6040.c

  Introduced in commit 8d61f4901f83461e1f04df7743777e9db5f541aa

    ASoC: twl6040: Convert PLUGINT to no-suspend irq
    
    Convert headset PLUGINT interrupt to NO_SUSPEND type in order to
    allow handling of insertion/removal events while device is
    suspended.

  So why does this need to be a NO_SUSPEND type interrupt? Just because
  the flag is sexy? What's wrong with using the wake mechanism?

drivers/xen/events/events_base.c

  A genuine usecase which makes sense and is not shared

drivers/watchdog/intel-mid_wdt.c

  MUHAHAHAHAHAHA

  static irqreturn_t mid_wdt_irq(int irq, void *dev_id)
  {
      panic("Kernel Watchdog");
      /* This code should not be reached */
      return IRQ_HANDLED;
  }

  So why does this need to be NO_SUSPEND? Because we forgot to trigger
  the watchdog during suspend? Brilliant!

drivers/usb/phy/phy-ab8500-usb.c

  Of course no explanation WHY this uses NO_SUSPEND plus SHARED and
  there is no fcking reason to do so.

So really, I'm too lazy to walk through that mess further. I bet NONE
of the usage sites except those for which this has been introduced in
the first place has a real good reason to do so.

Unless someone comes up with a use case where a shared NO_SUSPEND
handler is absolutely required, there is nothing which needs to be
addressed. You've proven yourself, that the wake mechanism is
sufficient to solve the problem which led to this discussion.

So we rather go and fix the ABUSE instead of making it legitimate by
fugly workarounds in the core code.

The same applies to the IRQF_RESUME_EARLY flag.

Just look at:

  commit 8b41669ceba0c2d4c09d69ccb9a3458953dae784

    mfd: twl4030: Fix chained irq handling on resume from suspend
    
    The irqs are enabled one-by-one in pm core resume_noirq phase.
    This leads to situation where the twl4030 primary interrupt
    handler (PIH) is enabled before the chained secondary handlers
    (SIH). As the PIH cannot clear the pending interrupt, and
    SIHs have not been enabled yet, a flood of interrupts hangs
    the device.
    
    Fixed the issue by setting the SIH irqs with IRQF_EARLY_RESUME
    flags, so they get enabled before the PIH.

So we solve an ordering problem which has a completely different root
cause by slapping random flags on it which paper over the issue?

The solution to the problem is completely wrong and of course the
"fix" is only applied to the one instance which might be affected by
that issue, i.e. the one which caused the patch submitter trouble.

Now I don't blame the author, I blame the maintainer who happily
applied that "fix".

That's unfortunately a very common pattern in the kernel which will
cause serious maintainability issues in the long run, but of course
that's just the opinion of a grumpy old greybeard.

The sad thing is that neither the author nor the maintainer who
applied it is going to be around and responsive when the shit hits the
fan. That commit is a perfect example for this.

Thanks,

	tglx

Re: [PATCH 0/5 v3] irq / PM: Suspend-to-idle wakeup interrupts

[Rafael J. Wysocki]

On Friday, August 29, 2014 12:44:11 AM Thomas Gleixner wrote:
> On Wed, 27 Aug 2014, Rafael J. Wysocki wrote:
> > To me, all of this is relatively straightforward and the handling of
> > IRQF_NO_SUSPEND for shared interrupts, which is a separate problem, can be
> > addressed on top of it later (make no mistake, I still think that it should be
> > addressed).
> 
> Why? Just because there is enough idiotic code using it?
> 
> Let's look at the usage sites:
> 
> sound/soc/codecs/twl6040.c
> 
>   Introduced in commit 8d61f4901f83461e1f04df7743777e9db5f541aa
> 
>     ASoC: twl6040: Convert PLUGINT to no-suspend irq
>     
>     Convert headset PLUGINT interrupt to NO_SUSPEND type in order to
>     allow handling of insertion/removal events while device is
>     suspended.
> 
>   So why does this need to be a NO_SUSPEND type interrupt? Just because
>   the flag is sexy? What's wrong with using the wake mechanism?
> 
> drivers/xen/events/events_base.c
> 
>   A genuine usecase which makes sense and is not shared
> 
> drivers/watchdog/intel-mid_wdt.c
> 
>   MUHAHAHAHAHAHA
> 
>   static irqreturn_t mid_wdt_irq(int irq, void *dev_id)
>   {
>       panic("Kernel Watchdog");
>       /* This code should not be reached */
>       return IRQ_HANDLED;
>   }
> 
>   So why does this need to be NO_SUSPEND? Because we forgot to trigger
>   the watchdog during suspend? Brilliant!
> 
> drivers/usb/phy/phy-ab8500-usb.c
> 
>   Of course no explanation WHY this uses NO_SUSPEND plus SHARED and
>   there is no fcking reason to do so.
> 
> So really, I'm too lazy to walk through that mess further. I bet NONE
> of the usage sites except those for which this has been introduced in
> the first place has a real good reason to do so.

A quite similar conclusion occured to me earlier today independently,
so we seem to be in agreement here. :-)

> Unless someone comes up with a use case where a shared NO_SUSPEND
> handler is absolutely required, there is nothing which needs to be
> addressed. You've proven yourself, that the wake mechanism is
> sufficient to solve the problem which led to this discussion.

Yeah, that particular thing can be perfectly addressed without using
IRQF_NO_SUSPEND.

There seems to be quite some confusion about that in the ARM world,
though, as IRQF_NO_SUSPEND things *happen* to wake the system up if WFI is
used to emulate "suspend" and that appears to make people believe that using
them for this purpose is actually fine (because that appears to work).

> So we rather go and fix the ABUSE instead of making it legitimate by
> fugly workarounds in the core code.

Fine by me.

> The same applies to the IRQF_RESUME_EARLY flag.
> 
> Just look at:
> 
>   commit 8b41669ceba0c2d4c09d69ccb9a3458953dae784
> 
>     mfd: twl4030: Fix chained irq handling on resume from suspend
>     
>     The irqs are enabled one-by-one in pm core resume_noirq phase.
>     This leads to situation where the twl4030 primary interrupt
>     handler (PIH) is enabled before the chained secondary handlers
>     (SIH). As the PIH cannot clear the pending interrupt, and
>     SIHs have not been enabled yet, a flood of interrupts hangs
>     the device.
>     
>     Fixed the issue by setting the SIH irqs with IRQF_EARLY_RESUME
>     flags, so they get enabled before the PIH.
> 
> So we solve an ordering problem which has a completely different root
> cause by slapping random flags on it which paper over the issue?
> 
> The solution to the problem is completely wrong and of course the
> "fix" is only applied to the one instance which might be affected by
> that issue, i.e. the one which caused the patch submitter trouble.
> 
> Now I don't blame the author, I blame the maintainer who happily
> applied that "fix".
> 
> That's unfortunately a very common pattern in the kernel which will
> cause serious maintainability issues in the long run, but of course
> that's just the opinion of a grumpy old greybeard.
> 
> The sad thing is that neither the author nor the maintainer who
> applied it is going to be around and responsive when the shit hits the
> fan.
>
> That commit is a perfect example for this.

Oh, I can reach that particular maintainer if need be.

That said I was not involved in the introduction of IRQF_EARLY_RESUME (or
at least I can't recall being involved) and I also can't recall having used
it for anything, so I can't really say what the original motivation was.

Rafael

Re: [PATCH 0/5 v3] irq / PM: Suspend-to-idle wakeup interrupts

[Thomas Gleixner]

On Fri, 29 Aug 2014, Rafael J. Wysocki wrote:
> On Friday, August 29, 2014 12:44:11 AM Thomas Gleixner wrote:
> > So really, I'm too lazy to walk through that mess further. I bet NONE
> > of the usage sites except those for which this has been introduced in
> > the first place has a real good reason to do so.
> 
> A quite similar conclusion occured to me earlier today independently,
> so we seem to be in agreement here. :-)

I appreciate that!
 
> > Unless someone comes up with a use case where a shared NO_SUSPEND
> > handler is absolutely required, there is nothing which needs to be
> > addressed. You've proven yourself, that the wake mechanism is
> > sufficient to solve the problem which led to this discussion.
> 
> Yeah, that particular thing can be perfectly addressed without using
> IRQF_NO_SUSPEND.
> 
> There seems to be quite some confusion about that in the ARM world,
> though, as IRQF_NO_SUSPEND things *happen* to wake the system up if WFI is
> used to emulate "suspend" and that appears to make people believe that using
> them for this purpose is actually fine (because that appears to work).

Well, the problem in the ARM world is still the same as we had 10
years ago, just a bit different. While 10 years ago, the "make it work
for me" attitude was hidden in some vendor tree and stayed there
forever, today its still the same "make it work for me" thing which
gets applied to the vendor tree (they still sport 1000+ patches
despite all efforts) and then trickles into the mainline w/o much
review to fulfil the "we need to be mainline" mantra.
 
> That said I was not involved in the introduction of IRQF_EARLY_RESUME (or
> at least I can't recall being involved) and I also can't recall having used
> it for anything, so I can't really say what the original motivation was.

It was solely introduced to solve an oddball issue with the XEN "IPIs"
and I take the blame for not documenting it proper. Though I really
have a hard time to understand why random drivers use this w/o comment
and for no obvious reason. "Works for me" looks like a reasonable
explanation.

I've uploaded an incomplete and completely untested patch series to
handle this stuff to: https://tglx.de/~tglx/patches.tar

Feel free to pick it up and work from there.

Thanks,

	tglx

[PATCH 00/13] genirq / PM: Wakeup interrupts handling rework (related to suspend-to-idle)

[Rafael J. Wysocki]

Hi,

This set of patches reworks the handling of system wakeup interrupts so that
suspend-to-idle is covered in the same way as full system suspend.

This time we have several new patches from Thomas and hopefully we're nailed
down that thing at last.

[01/13] Mechanism to wake up the system or abort suspend in progress
        automatically (this is different from the previous versions, because
        I eventually convinced myself that pm_abort_suspend didn't really
        require any locking).
[02/13] Move PM-related IRQ code to kernel/irq/pm.c.
[03/13] Add accounting for IRQF_NO_SUSPEND and IRQF_RESUME_EARLY.
[04/13] Use the accoundint added by [03/13].
[05/13] Make suspend_device_irqs() mask non-wakeup interruts if
        IRQCHIP_MASK_ON_SUSPEND is set.
[06/13] Synchronize IRQs in the same loop in which they are reconfigured
        in suspend_device_irqs().
[07/13] Separate irq_check_poll() checks from disabled/no action checks in
        kernel/irq/chip.c.
[08/13] Add a helper for checks made by many IRQ flow handlers in the same way.
[09/13] Mark wakeup sources as "armed" on suspend (using a new flag).
[10/13] Make suspend_device_irqs() leave wakeup interrupts enabled, but handle
        them so that the first interrupt triggers system wakeup.
[11/13] Set IRQCHIP_SKIP_SET_WAKE for x86 IOAPIC IRQ chips (same as before).
[12/13] Make PCIe PME wake up from suspend to idle (same as before).
[13/13] Document rules related to system suspend and interrupts (updated).

The series have been tested on MSI Wind, but more testing is welcome.

There is a pm-genirq branch in the linux-pm.git tree containing these patches:

git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git pm-genirq

that can be pulled from.

Kind regards,
Rafael

[PATCH 01/13] PM / sleep: Mechanism for aborting system suspends unconditionally

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

It sometimes may be necessary to abort a system suspend in
progress or wake up the system from suspend-to-idle even if the
pm_wakeup_event()/pm_stay_awake() mechanism is not enabled.

For this purpose, introduce a new global variable pm_abort_suspend
and make pm_wakeup_pending() check its value.  Also add routines
for manipulating that variable.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/base/power/wakeup.c |   16 +++++++++++++++-
 include/linux/suspend.h     |    4 ++++
 kernel/power/process.c      |    1 +
 3 files changed, 20 insertions(+), 1 deletion(-)

Index: linux-pm/include/linux/suspend.h
===================================================================
--- linux-pm.orig/include/linux/suspend.h
+++ linux-pm/include/linux/suspend.h
@@ -371,6 +371,8 @@ extern int unregister_pm_notifier(struct
 extern bool events_check_enabled;
 
 extern bool pm_wakeup_pending(void);
+extern void pm_system_wakeup(void);
+extern void pm_wakeup_clear(void);
 extern bool pm_get_wakeup_count(unsigned int *count, bool block);
 extern bool pm_save_wakeup_count(unsigned int count);
 extern void pm_wakep_autosleep_enabled(bool set);
@@ -418,6 +420,8 @@ static inline int unregister_pm_notifier
 #define pm_notifier(fn, pri)	do { (void)(fn); } while (0)
 
 static inline bool pm_wakeup_pending(void) { return false; }
+static inline void pm_system_wakeup(void) {}
+static inline void pm_wakeup_clear(void) {}
 
 static inline void lock_system_sleep(void) {}
 static inline void unlock_system_sleep(void) {}
Index: linux-pm/drivers/base/power/wakeup.c
===================================================================
--- linux-pm.orig/drivers/base/power/wakeup.c
+++ linux-pm/drivers/base/power/wakeup.c
@@ -24,6 +24,9 @@
  */
 bool events_check_enabled __read_mostly;
 
+/* If set and the system is suspending, terminate the suspend. */
+static bool pm_abort_suspend __read_mostly;
+
 /*
  * Combined counters of registered wakeup events and wakeup events in progress.
  * They need to be modified together atomically, so it's better to use one
@@ -719,7 +722,18 @@ bool pm_wakeup_pending(void)
 		pm_print_active_wakeup_sources();
 	}
 
-	return ret;
+	return ret || pm_abort_suspend;
+}
+
+void pm_system_wakeup(void)
+{
+	pm_abort_suspend = true;
+	freeze_wake();
+}
+
+void pm_wakeup_clear(void)
+{
+	pm_abort_suspend = false;
 }
 
 /**
Index: linux-pm/kernel/power/process.c
===================================================================
--- linux-pm.orig/kernel/power/process.c
+++ linux-pm/kernel/power/process.c
@@ -129,6 +129,7 @@ int freeze_processes(void)
 	if (!pm_freezing)
 		atomic_inc(&system_freezing_cnt);
 
+	pm_wakeup_clear();
 	printk("Freezing user space processes ... ");
 	pm_freezing = true;
 	error = try_to_freeze_tasks(true);

[PATCH 02/13] genirq: Move suspend/resume logic into irq/pm code

[Rafael J. Wysocki]

From: Thomas Gleixner <tglx@linutronix.de>

No functional change. Preparatory patch for cleaning up the suspend
abort functionality. Update the comments while at it.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 kernel/irq/internals.h |    4 ++--
 kernel/irq/manage.c    |   28 +++++-----------------------
 kernel/irq/pm.c        |   44 ++++++++++++++++++++++++++++++++++++++------
 3 files changed, 45 insertions(+), 31 deletions(-)

Index: linux/kernel/irq/internals.h
===================================================================
--- linux.orig/kernel/irq/internals.h
+++ linux/kernel/irq/internals.h
@@ -63,8 +63,8 @@ enum {
 
 extern int __irq_set_trigger(struct irq_desc *desc, unsigned int irq,
 		unsigned long flags);
-extern void __disable_irq(struct irq_desc *desc, unsigned int irq, bool susp);
-extern void __enable_irq(struct irq_desc *desc, unsigned int irq, bool resume);
+extern void __disable_irq(struct irq_desc *desc, unsigned int irq);
+extern void __enable_irq(struct irq_desc *desc, unsigned int irq);
 
 extern int irq_startup(struct irq_desc *desc, bool resend);
 extern void irq_shutdown(struct irq_desc *desc);
Index: linux/kernel/irq/manage.c
===================================================================
--- linux.orig/kernel/irq/manage.c
+++ linux/kernel/irq/manage.c
@@ -382,14 +382,8 @@ setup_affinity(unsigned int irq, struct
 }
 #endif
 
-void __disable_irq(struct irq_desc *desc, unsigned int irq, bool suspend)
+void __disable_irq(struct irq_desc *desc, unsigned int irq)
 {
-	if (suspend) {
-		if (!desc->action || (desc->action->flags & IRQF_NO_SUSPEND))
-			return;
-		desc->istate |= IRQS_SUSPENDED;
-	}
-
 	if (!desc->depth++)
 		irq_disable(desc);
 }
@@ -401,7 +395,7 @@ static int __disable_irq_nosync(unsigned
 
 	if (!desc)
 		return -EINVAL;
-	__disable_irq(desc, irq, false);
+	__disable_irq(desc, irq);
 	irq_put_desc_busunlock(desc, flags);
 	return 0;
 }
@@ -442,20 +436,8 @@ void disable_irq(unsigned int irq)
 }
 EXPORT_SYMBOL(disable_irq);
 
-void __enable_irq(struct irq_desc *desc, unsigned int irq, bool resume)
+void __enable_irq(struct irq_desc *desc, unsigned int irq)
 {
-	if (resume) {
-		if (!(desc->istate & IRQS_SUSPENDED)) {
-			if (!desc->action)
-				return;
-			if (!(desc->action->flags & IRQF_FORCE_RESUME))
-				return;
-			/* Pretend that it got disabled ! */
-			desc->depth++;
-		}
-		desc->istate &= ~IRQS_SUSPENDED;
-	}
-
 	switch (desc->depth) {
 	case 0:
  err_out:
@@ -497,7 +479,7 @@ void enable_irq(unsigned int irq)
 		 KERN_ERR "enable_irq before setup/request_irq: irq %u\n", irq))
 		goto out;
 
-	__enable_irq(desc, irq, false);
+	__enable_irq(desc, irq);
 out:
 	irq_put_desc_busunlock(desc, flags);
 }
@@ -1228,7 +1210,7 @@ __setup_irq(unsigned int irq, struct irq
 	 */
 	if (shared && (desc->istate & IRQS_SPURIOUS_DISABLED)) {
 		desc->istate &= ~IRQS_SPURIOUS_DISABLED;
-		__enable_irq(desc, irq, false);
+		__enable_irq(desc, irq);
 	}
 
 	raw_spin_unlock_irqrestore(&desc->lock, flags);
Index: linux/kernel/irq/pm.c
===================================================================
--- linux.orig/kernel/irq/pm.c
+++ linux/kernel/irq/pm.c
@@ -13,13 +13,26 @@
 
 #include "internals.h"
 
+static void suspend_device_irq(struct irq_desc *desc, int irq)
+{
+	if (!desc->action || (desc->action->flags & IRQF_NO_SUSPEND))
+		return;
+
+	desc->istate |= IRQS_SUSPENDED;
+	__disable_irq(desc, irq);
+}
+
 /**
  * suspend_device_irqs - disable all currently enabled interrupt lines
  *
- * During system-wide suspend or hibernation device drivers need to be prevented
- * from receiving interrupts and this function is provided for this purpose.
- * It marks all interrupt lines in use, except for the timer ones, as disabled
- * and sets the IRQS_SUSPENDED flag for each of them.
+ * During system-wide suspend or hibernation device drivers need to be
+ * prevented from receiving interrupts and this function is provided
+ * for this purpose.
+ *
+ * So we disable all interrupts and mark them IRQS_SUSPENDED except
+ * for those which are unused and those which are marked as not
+ * suspendable via an interrupt request with the flag IRQF_NO_SUSPEND
+ * set.
  */
 void suspend_device_irqs(void)
 {
@@ -30,7 +43,7 @@ void suspend_device_irqs(void)
 		unsigned long flags;
 
 		raw_spin_lock_irqsave(&desc->lock, flags);
-		__disable_irq(desc, irq, true);
+		suspend_device_irq(desc, irq);
 		raw_spin_unlock_irqrestore(&desc->lock, flags);
 	}
 
@@ -40,6 +53,25 @@ void suspend_device_irqs(void)
 }
 EXPORT_SYMBOL_GPL(suspend_device_irqs);
 
+static void resume_irq(struct irq_desc *desc, int irq)
+{
+	if (desc->istate & IRQS_SUSPENDED)
+		goto resume;
+
+	if (!desc->action)
+		return;
+
+	/* Interrupts marked with that flag are force reenabled */
+	if (!(desc->action->flags & IRQF_FORCE_RESUME))
+		return;
+
+	/* Pretend that it got disabled ! */
+	desc->depth++;
+resume:
+	desc->istate &= ~IRQS_SUSPENDED;
+	__enable_irq(desc, irq);
+}
+
 static void resume_irqs(bool want_early)
 {
 	struct irq_desc *desc;
@@ -54,7 +86,7 @@ static void resume_irqs(bool want_early)
 			continue;
 
 		raw_spin_lock_irqsave(&desc->lock, flags);
-		__enable_irq(desc, irq, true);
+		resume_irq(desc, irq);
 		raw_spin_unlock_irqrestore(&desc->lock, flags);
 	}
 }

[PATCH 03/13] genirq: Add sanity checks for PM options on shared interrupt lines

[Rafael J. Wysocki]

From: Thomas Gleixner <tglx@linutronix.de>

Account the IRQF_NO_SUSPEND and IRQF_RESUME_EARLY actions on shared
interrupt lines and yell loudly if there is a mismatch.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 include/linux/irqdesc.h |   10 ++++++++++
 kernel/irq/internals.h  |   10 ++++++++++
 kernel/irq/manage.c     |    4 ++++
 kernel/irq/pm.c         |   36 ++++++++++++++++++++++++++++++++++++
 4 files changed, 60 insertions(+)

Index: linux/include/linux/irqdesc.h
===================================================================
--- linux.orig/include/linux/irqdesc.h
+++ linux/include/linux/irqdesc.h
@@ -36,6 +36,11 @@ struct irq_desc;
  * @threads_oneshot:	bitfield to handle shared oneshot threads
  * @threads_active:	number of irqaction threads currently running
  * @wait_for_threads:	wait queue for sync_irq to wait for threaded handlers
+ * @nr_actions:		number of installed actions on this descriptor
+ * @no_suspend_depth:	number of irqactions on a irq descriptor with
+ *			IRQF_NO_SUSPEND set
+ * @force_resume_depth:	number of irqactions on a irq descriptor with
+ *			IRQF_FORCE_RESUME set
  * @dir:		/proc/irq/ procfs entry
  * @name:		flow handler name for /proc/interrupts output
  */
@@ -68,6 +73,11 @@ struct irq_desc {
 	unsigned long		threads_oneshot;
 	atomic_t		threads_active;
 	wait_queue_head_t       wait_for_threads;
+#ifdef CONFIG_PM_SLEEP
+	unsigned int		nr_actions;
+	unsigned int		no_suspend_depth;
+	unsigned int		force_resume_depth;
+#endif
 #ifdef CONFIG_PROC_FS
 	struct proc_dir_entry	*dir;
 #endif
Index: linux/kernel/irq/internals.h
===================================================================
--- linux.orig/kernel/irq/internals.h
+++ linux/kernel/irq/internals.h
@@ -194,3 +194,13 @@ static inline void kstat_incr_irqs_this_
 	__this_cpu_inc(*desc->kstat_irqs);
 	__this_cpu_inc(kstat.irqs_sum);
 }
+
+#ifdef CONFIG_PM_SLEEP
+void irq_pm_install_action(struct irq_desc *desc, struct irqaction *action);
+void irq_pm_remove_action(struct irq_desc *desc, struct irqaction *action);
+#else
+static inline void
+irq_pm_install_action(struct irq_desc *desc, struct irqaction *action) { }
+static inline void
+irq_pm_remove_action(struct irq_desc *desc, struct irqaction *action) { }
+#endif
Index: linux/kernel/irq/manage.c
===================================================================
--- linux.orig/kernel/irq/manage.c
+++ linux/kernel/irq/manage.c
@@ -1200,6 +1200,8 @@ __setup_irq(unsigned int irq, struct irq
 	new->irq = irq;
 	*old_ptr = new;
 
+	irq_pm_install_action(desc, new);
+
 	/* Reset broken irq detection when installing new handler */
 	desc->irq_count = 0;
 	desc->irqs_unhandled = 0;
@@ -1318,6 +1320,8 @@ static struct irqaction *__free_irq(unsi
 	/* Found it - now remove it from the list of entries: */
 	*action_ptr = action->next;
 
+	irq_pm_remove_action(desc, action);
+
 	/* If this was the last handler, shut down the IRQ line: */
 	if (!desc->action) {
 		irq_shutdown(desc);
Index: linux/kernel/irq/pm.c
===================================================================
--- linux.orig/kernel/irq/pm.c
+++ linux/kernel/irq/pm.c
@@ -13,6 +13,42 @@
 
 #include "internals.h"
 
+/*
+ * Called from __setup_irq() with desc->lock held after @action has
+ * been installed in the action chain.
+ */
+void irq_pm_install_action(struct irq_desc *desc, struct irqaction *action)
+{
+	desc->nr_actions++;
+
+	if (action->flags & IRQF_FORCE_RESUME)
+		desc->force_resume_depth++;
+
+	WARN_ON_ONCE(desc->force_resume_depth &&
+		     desc->force_resume_depth != desc->nr_actions);
+
+	if (action->flags & IRQF_NO_SUSPEND)
+		desc->no_suspend_depth++;
+
+	WARN_ON_ONCE(desc->no_suspend_depth &&
+		     desc->no_suspend_depth != desc->nr_actions);
+}
+
+/*
+ * Called from __free_irq() with desc->lock held after @action has
+ * been removed from the action chain.
+ */
+void irq_pm_remove_action(struct irq_desc *desc, struct irqaction *action)
+{
+	desc->nr_actions--;
+
+	if (action->flags & IRQF_FORCE_RESUME)
+		desc->force_resume_depth--;
+
+	if (action->flags & IRQF_NO_SUSPEND)
+		desc->no_suspend_depth--;
+}
+
 static void suspend_device_irq(struct irq_desc *desc, int irq)
 {
 	if (!desc->action || (desc->action->flags & IRQF_NO_SUSPEND))

[PATCH 04/13] genirq: Make use of pm misfeature accounting

[Rafael J. Wysocki]

From: Thomas Gleixner <tglx@linutronix.de>

Use the accounting fields which got introduced for snity checking for
the various PM options.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 kernel/irq/pm.c |    9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

Index: linux/kernel/irq/pm.c
===================================================================
--- linux.orig/kernel/irq/pm.c
+++ linux/kernel/irq/pm.c
@@ -51,7 +51,7 @@ void irq_pm_remove_action(struct irq_des
 
 static void suspend_device_irq(struct irq_desc *desc, int irq)
 {
-	if (!desc->action || (desc->action->flags & IRQF_NO_SUSPEND))
+	if (!desc->action || desc->no_suspend_depth)
 		return;
 
 	desc->istate |= IRQS_SUSPENDED;
@@ -94,11 +94,8 @@ static void resume_irq(struct irq_desc *
 	if (desc->istate & IRQS_SUSPENDED)
 		goto resume;
 
-	if (!desc->action)
-		return;
-
-	/* Interrupts marked with that flag are force reenabled */
-	if (!(desc->action->flags & IRQF_FORCE_RESUME))
+	/* Force resume the interrupt? */
+	if (!desc->force_resume_depth)
 		return;
 
 	/* Pretend that it got disabled ! */

[PATCH 05/13] genirq: Move MASK_ON_SUSPEND handling into suspend_device_irqs()

[Rafael J. Wysocki]

From: Thomas Gleixner <tglx@linutronix.de>

There is no reason why we should delay the masking of interrupts whose
interrupt chip requests MASK_ON_SUSPEND to the point where we check
the wakeup interrupts. We can do it right at the point where we mark
the interrupt as suspended.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 kernel/irq/pm.c |   21 +++++++++------------
 1 file changed, 9 insertions(+), 12 deletions(-)

Index: linux/kernel/irq/pm.c
===================================================================
--- linux.orig/kernel/irq/pm.c
+++ linux/kernel/irq/pm.c
@@ -56,6 +56,15 @@ static void suspend_device_irq(struct ir
 
 	desc->istate |= IRQS_SUSPENDED;
 	__disable_irq(desc, irq);
+
+	/*
+	 * Hardware which has no wakeup source configuration facility
+	 * requires that the non wakeup interrupts are masked at the
+	 * chip level. The chip implementation indicates that with
+	 * IRQCHIP_MASK_ON_SUSPEND.
+	 */
+	if (irq_desc_get_chip(desc)->flags & IRQCHIP_MASK_ON_SUSPEND)
+		mask_irq(desc);
 }
 
 /**
@@ -176,19 +185,7 @@ int check_wakeup_irqs(void)
 		if (irqd_is_wakeup_set(&desc->irq_data)) {
 			if (desc->depth == 1 && desc->istate & IRQS_PENDING)
 				return -EBUSY;
-			continue;
 		}
-		/*
-		 * Check the non wakeup interrupts whether they need
-		 * to be masked before finally going into suspend
-		 * state. That's for hardware which has no wakeup
-		 * source configuration facility. The chip
-		 * implementation indicates that with
-		 * IRQCHIP_MASK_ON_SUSPEND.
-		 */
-		if (desc->istate & IRQS_SUSPENDED &&
-		    irq_desc_get_chip(desc)->flags & IRQCHIP_MASK_ON_SUSPEND)
-			mask_irq(desc);
 	}
 
 	return 0;

[PATCH 06/13] genirq: Avoid double loop on suspend

[Rafael J. Wysocki]

From: Thomas Gleixner <tglx@linutronix.de>

We can synchronize the suspended interrupts right away. No need for an
extra loop.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 kernel/irq/pm.c |   13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

Index: linux/kernel/irq/pm.c
===================================================================
--- linux.orig/kernel/irq/pm.c
+++ linux/kernel/irq/pm.c
@@ -49,10 +49,10 @@ void irq_pm_remove_action(struct irq_des
 		desc->no_suspend_depth--;
 }
 
-static void suspend_device_irq(struct irq_desc *desc, int irq)
+static bool suspend_device_irq(struct irq_desc *desc, int irq)
 {
 	if (!desc->action || desc->no_suspend_depth)
-		return;
+		return false;
 
 	desc->istate |= IRQS_SUSPENDED;
 	__disable_irq(desc, irq);
@@ -65,6 +65,7 @@ static void suspend_device_irq(struct ir
 	 */
 	if (irq_desc_get_chip(desc)->flags & IRQCHIP_MASK_ON_SUSPEND)
 		mask_irq(desc);
+	return true;
 }
 
 /**
@@ -86,15 +87,15 @@ void suspend_device_irqs(void)
 
 	for_each_irq_desc(irq, desc) {
 		unsigned long flags;
+		bool sync;
 
 		raw_spin_lock_irqsave(&desc->lock, flags);
-		suspend_device_irq(desc, irq);
+		sync = suspend_device_irq(desc, irq);
 		raw_spin_unlock_irqrestore(&desc->lock, flags);
-	}
 
-	for_each_irq_desc(irq, desc)
-		if (desc->istate & IRQS_SUSPENDED)
+		if (sync)
 			synchronize_irq(irq);
+	}
 }
 EXPORT_SYMBOL_GPL(suspend_device_irqs);
 

[PATCH 07/13] genirq: Distangle edge handler entry

[Rafael J. Wysocki]

From: Thomas Gleixner <tglx@linutronix.de>

If the interrupt is disabled or has no action, then we should not call
the poll check. Separate the checks.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 kernel/irq/chip.c |   39 +++++++++++++++++++++++++++++----------
 1 file changed, 29 insertions(+), 10 deletions(-)

Index: linux/kernel/irq/chip.c
===================================================================
--- linux.orig/kernel/irq/chip.c
+++ linux/kernel/irq/chip.c
@@ -540,19 +540,29 @@ handle_edge_irq(unsigned int irq, struct
 	raw_spin_lock(&desc->lock);
 
 	desc->istate &= ~(IRQS_REPLAY | IRQS_WAITING);
+
 	/*
-	 * If we're currently running this IRQ, or its disabled,
-	 * we shouldn't process the IRQ. Mark it pending, handle
-	 * the necessary masking and go out
+	 * If the handler is currently running, mark it pending,
+	 * handle the necessary masking and go out
 	 */
-	if (unlikely(irqd_irq_disabled(&desc->irq_data) ||
-		     irqd_irq_inprogress(&desc->irq_data) || !desc->action)) {
+	if (unlikely(irqd_irq_inprogress(&desc->irq_data))) {
 		if (!irq_check_poll(desc)) {
 			desc->istate |= IRQS_PENDING;
 			mask_ack_irq(desc);
 			goto out_unlock;
 		}
 	}
+
+	/*
+	 * If its disabled or no action available then mask it and get
+	 * out of here.
+	 */
+	if (irqd_irq_disabled(&desc->irq_data) || !desc->action) {
+		desc->istate |= IRQS_PENDING;
+		mask_ack_irq(desc);
+		goto out_unlock;
+	}
+
 	kstat_incr_irqs_this_cpu(irq, desc);
 
 	/* Start handling the irq */
@@ -601,18 +611,27 @@ void handle_edge_eoi_irq(unsigned int ir
 	raw_spin_lock(&desc->lock);
 
 	desc->istate &= ~(IRQS_REPLAY | IRQS_WAITING);
+
 	/*
-	 * If we're currently running this IRQ, or its disabled,
-	 * we shouldn't process the IRQ. Mark it pending, handle
-	 * the necessary masking and go out
+	 * If the handler is currently running, mark it pending,
+	 * handle the necessary masking and go out
 	 */
-	if (unlikely(irqd_irq_disabled(&desc->irq_data) ||
-		     irqd_irq_inprogress(&desc->irq_data) || !desc->action)) {
+	if (unlikely(irqd_irq_inprogress(&desc->irq_data))) {
 		if (!irq_check_poll(desc)) {
 			desc->istate |= IRQS_PENDING;
 			goto out_eoi;
 		}
 	}
+
+	/*
+	 * If its disabled or no action available then mask it and get
+	 * out of here.
+	 */
+	if (irqd_irq_disabled(&desc->irq_data) || !desc->action) {
+		desc->istate |= IRQS_PENDING;
+		goto out_eoi;
+	}
+
 	kstat_incr_irqs_this_cpu(irq, desc);
 
 	do {

[PATCH 08/13] genirq: Create helper for flow handler entry check

[Rafael J. Wysocki]

From: Thomas Gleixner <tglx@linutronix.de>

All flow handlers - except the per cpu ones - check for an interrupt
in progress and an eventual concurrent polling on another cpu.

Create a helper function for the repeated code pattern.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 kernel/irq/chip.c |   48 ++++++++++++++++++++----------------------------
 1 file changed, 20 insertions(+), 28 deletions(-)

Index: linux/kernel/irq/chip.c
===================================================================
--- linux.orig/kernel/irq/chip.c
+++ linux/kernel/irq/chip.c
@@ -342,6 +342,13 @@ static bool irq_check_poll(struct irq_de
 	return irq_wait_for_poll(desc);
 }
 
+static bool irq_may_run(struct irq_desc *desc)
+{
+	if (!irqd_irq_inprogress(&desc->irq_data))
+		return true;
+	return irq_check_poll(desc);
+}
+
 /**
  *	handle_simple_irq - Simple and software-decoded IRQs.
  *	@irq:	the interrupt number
@@ -359,9 +366,8 @@ handle_simple_irq(unsigned int irq, stru
 {
 	raw_spin_lock(&desc->lock);
 
-	if (unlikely(irqd_irq_inprogress(&desc->irq_data)))
-		if (!irq_check_poll(desc))
-			goto out_unlock;
+	if (!irq_may_run(desc))
+		goto out_unlock;
 
 	desc->istate &= ~(IRQS_REPLAY | IRQS_WAITING);
 	kstat_incr_irqs_this_cpu(irq, desc);
@@ -412,9 +418,8 @@ handle_level_irq(unsigned int irq, struc
 	raw_spin_lock(&desc->lock);
 	mask_ack_irq(desc);
 
-	if (unlikely(irqd_irq_inprogress(&desc->irq_data)))
-		if (!irq_check_poll(desc))
-			goto out_unlock;
+	if (!irq_may_run(desc))
+		goto out_unlock;
 
 	desc->istate &= ~(IRQS_REPLAY | IRQS_WAITING);
 	kstat_incr_irqs_this_cpu(irq, desc);
@@ -485,9 +490,8 @@ handle_fasteoi_irq(unsigned int irq, str
 
 	raw_spin_lock(&desc->lock);
 
-	if (unlikely(irqd_irq_inprogress(&desc->irq_data)))
-		if (!irq_check_poll(desc))
-			goto out;
+	if (!irq_may_run(desc))
+		goto out;
 
 	desc->istate &= ~(IRQS_REPLAY | IRQS_WAITING);
 	kstat_incr_irqs_this_cpu(irq, desc);
@@ -541,16 +545,10 @@ handle_edge_irq(unsigned int irq, struct
 
 	desc->istate &= ~(IRQS_REPLAY | IRQS_WAITING);
 
-	/*
-	 * If the handler is currently running, mark it pending,
-	 * handle the necessary masking and go out
-	 */
-	if (unlikely(irqd_irq_inprogress(&desc->irq_data))) {
-		if (!irq_check_poll(desc)) {
-			desc->istate |= IRQS_PENDING;
-			mask_ack_irq(desc);
-			goto out_unlock;
-		}
+	if (!irq_may_run(desc)) {
+		desc->istate |= IRQS_PENDING;
+		mask_ack_irq(desc);
+		goto out_unlock;
 	}
 
 	/*
@@ -612,15 +610,9 @@ void handle_edge_eoi_irq(unsigned int ir
 
 	desc->istate &= ~(IRQS_REPLAY | IRQS_WAITING);
 
-	/*
-	 * If the handler is currently running, mark it pending,
-	 * handle the necessary masking and go out
-	 */
-	if (unlikely(irqd_irq_inprogress(&desc->irq_data))) {
-		if (!irq_check_poll(desc)) {
-			desc->istate |= IRQS_PENDING;
-			goto out_eoi;
-		}
+	if (!irq_may_run(desc)) {
+		desc->istate |= IRQS_PENDING;
+		goto out_eoi;
 	}
 
 	/*

[PATCH 09/13] genirq: Mark wakeup sources as armed on suspend

[Rafael J. Wysocki]

From: Thomas Gleixner <tglx@linutronix.de>

This allows us to utilize this information in the irq_may_run() check
without adding another conditional to the fast path.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 include/linux/irq.h |    8 ++++++++
 kernel/irq/pm.c     |    5 +++++
 2 files changed, 13 insertions(+)

Index: linux/include/linux/irq.h
===================================================================
--- linux.orig/include/linux/irq.h
+++ linux/include/linux/irq.h
@@ -173,6 +173,7 @@ struct irq_data {
  * IRQD_IRQ_DISABLED		- Disabled state of the interrupt
  * IRQD_IRQ_MASKED		- Masked state of the interrupt
  * IRQD_IRQ_INPROGRESS		- In progress state of the interrupt
+ * IRQD_WAKEUP_ARMED		- Wakeup mode armed
  */
 enum {
 	IRQD_TRIGGER_MASK		= 0xf,
@@ -186,6 +187,7 @@ enum {
 	IRQD_IRQ_DISABLED		= (1 << 16),
 	IRQD_IRQ_MASKED			= (1 << 17),
 	IRQD_IRQ_INPROGRESS		= (1 << 18),
+	IRQD_WAKEUP_ARMED		= (1 << 19),
 };
 
 static inline bool irqd_is_setaffinity_pending(struct irq_data *d)
@@ -257,6 +259,12 @@ static inline bool irqd_irq_inprogress(s
 	return d->state_use_accessors & IRQD_IRQ_INPROGRESS;
 }
 
+static inline bool irqd_is_wakeup_armed(struct irq_data *d)
+{
+	return d->state_use_accessors & IRQD_WAKEUP_ARMED;
+}
+
+
 /*
  * Functions for chained handlers which can be enabled/disabled by the
  * standard disable_irq/enable_irq calls. Must be called with
Index: linux/kernel/irq/pm.c
===================================================================
--- linux.orig/kernel/irq/pm.c
+++ linux/kernel/irq/pm.c
@@ -54,6 +54,9 @@ static bool suspend_device_irq(struct ir
 	if (!desc->action || desc->no_suspend_depth)
 		return false;
 
+	if (irqd_is_wakeup_set(&desc->irq_data))
+		irqd_set(&desc->irq_data, IRQD_WAKEUP_ARMED);
+
 	desc->istate |= IRQS_SUSPENDED;
 	__disable_irq(desc, irq);
 
@@ -101,6 +104,8 @@ EXPORT_SYMBOL_GPL(suspend_device_irqs);
 
 static void resume_irq(struct irq_desc *desc, int irq)
 {
+	irqd_clear(&desc->irq_data, IRQD_WAKEUP_ARMED);
+
 	if (desc->istate & IRQS_SUSPENDED)
 		goto resume;
 

[PATCH 10/13] genirq: Simplify wakeup mechanism

[Rafael J. Wysocki]

From: Thomas Gleixner <tglx@linutronix.de>

Currently we suspend wakeup interrupts by lazy disabling them and
check later whether the interrupt has fired, but that's not sufficient
for suspend to idle as there is no way to check that once we
transitioned into the CPU idle state.

So we change the mechanism in the following way:

1) Leave the wakeup interrupts enabled across suspend

2) Add a check to irq_may_run() which is called at the beginning of
   each flow handler whether the interrupt is an armed wakeup source.

   This check is basically free as it just extends the existing check
   for IRQD_IRQ_INPROGRESS. So no new conditional in the hot path.

   If the IRQD_WAKEUP_ARMED flag is set, then the interrupt is
   disabled, marked as pending/suspended and the pm core is notified
   about the wakeup event.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
[ rjw: syscore.c and irq_pm_check_wakeup() in pm.c ]
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/base/syscore.c    |    7 ++---
 include/linux/interrupt.h |    5 ----
 kernel/irq/chip.c         |   20 +++++++++++++++-
 kernel/irq/internals.h    |    2 +
 kernel/irq/pm.c           |   55 ++++++++++++++++++++++++----------------------
 5 files changed, 53 insertions(+), 36 deletions(-)

Index: linux-pm/drivers/base/syscore.c
===================================================================
--- linux-pm.orig/drivers/base/syscore.c
+++ linux-pm/drivers/base/syscore.c
@@ -9,7 +9,7 @@
 #include <linux/syscore_ops.h>
 #include <linux/mutex.h>
 #include <linux/module.h>
-#include <linux/interrupt.h>
+#include <linux/suspend.h>
 #include <trace/events/power.h>
 
 static LIST_HEAD(syscore_ops_list);
@@ -54,9 +54,8 @@ int syscore_suspend(void)
 	pr_debug("Checking wakeup interrupts\n");
 
 	/* Return error code if there are any wakeup interrupts pending. */
-	ret = check_wakeup_irqs();
-	if (ret)
-		return ret;
+	if (pm_wakeup_pending())
+		return -EBUSY;
 
 	WARN_ONCE(!irqs_disabled(),
 		"Interrupts enabled before system core suspend.\n");
Index: linux-pm/include/linux/interrupt.h
===================================================================
--- linux-pm.orig/include/linux/interrupt.h
+++ linux-pm/include/linux/interrupt.h
@@ -193,11 +193,6 @@ extern void irq_wake_thread(unsigned int
 /* The following three functions are for the core kernel use only. */
 extern void suspend_device_irqs(void);
 extern void resume_device_irqs(void);
-#ifdef CONFIG_PM_SLEEP
-extern int check_wakeup_irqs(void);
-#else
-static inline int check_wakeup_irqs(void) { return 0; }
-#endif
 
 /**
  * struct irq_affinity_notify - context for notification of IRQ affinity changes
Index: linux-pm/kernel/irq/chip.c
===================================================================
--- linux-pm.orig/kernel/irq/chip.c
+++ linux-pm/kernel/irq/chip.c
@@ -344,8 +344,26 @@ static bool irq_check_poll(struct irq_de
 
 static bool irq_may_run(struct irq_desc *desc)
 {
-	if (!irqd_irq_inprogress(&desc->irq_data))
+	unsigned int mask = IRQD_IRQ_INPROGRESS | IRQD_WAKEUP_ARMED;
+
+	/*
+	 * If the interrupt is not in progress and is not an armed
+	 * wakeup interrupt, proceed.
+	 */
+	if (!irqd_has_set(&desc->irq_data, mask))
 		return true;
+
+	/*
+	 * If the interrupt is an armed wakeup source, mark it pending
+	 * and suspended, disable it and notify the pm core about the
+	 * event.
+	 */
+	if (irq_pm_check_wakeup(desc))
+		return false;
+
+	/*
+	 * Handle a potential concurrent poll on a different core.
+	 */
 	return irq_check_poll(desc);
 }
 
Index: linux-pm/kernel/irq/pm.c
===================================================================
--- linux-pm.orig/kernel/irq/pm.c
+++ linux-pm/kernel/irq/pm.c
@@ -9,10 +9,24 @@
 #include <linux/irq.h>
 #include <linux/module.h>
 #include <linux/interrupt.h>
+#include <linux/suspend.h>
 #include <linux/syscore_ops.h>
 
 #include "internals.h"
 
+bool irq_pm_check_wakeup(struct irq_desc *desc)
+{
+	if (irqd_is_wakeup_armed(&desc->irq_data)) {
+		irqd_clear(&desc->irq_data, IRQD_WAKEUP_ARMED);
+		desc->istate |= IRQS_SUSPENDED | IRQS_PENDING;
+		desc->depth++;
+		irq_disable(desc);
+		pm_system_wakeup();
+		return true;
+	}
+	return false;
+}
+
 /*
  * Called from __setup_irq() with desc->lock held after @action has
  * been installed in the action chain.
@@ -54,8 +68,16 @@ static bool suspend_device_irq(struct ir
 	if (!desc->action || desc->no_suspend_depth)
 		return false;
 
-	if (irqd_is_wakeup_set(&desc->irq_data))
+	if (irqd_is_wakeup_set(&desc->irq_data)) {
 		irqd_set(&desc->irq_data, IRQD_WAKEUP_ARMED);
+		/*
+		 * We return true here to force the caller to issue
+		 * synchronize_irq(). We need to make sure that the
+		 * IRQD_WAKEUP_ARMED is visible before we return from
+		 * suspend_device_irqs().
+		 */
+		return true;
+	}
 
 	desc->istate |= IRQS_SUSPENDED;
 	__disable_irq(desc, irq);
@@ -79,9 +101,13 @@ static bool suspend_device_irq(struct ir
  * for this purpose.
  *
  * So we disable all interrupts and mark them IRQS_SUSPENDED except
- * for those which are unused and those which are marked as not
+ * for those which are unused, those which are marked as not
  * suspendable via an interrupt request with the flag IRQF_NO_SUSPEND
- * set.
+ * set and those which are marked as active wakeup sources.
+ *
+ * The active wakeup sources are handled by the flow handler entry
+ * code which checks for the IRQD_WAKEUP_ARMED flag, suspends the
+ * interrupt and notifies the pm core about the wakeup.
  */
 void suspend_device_irqs(void)
 {
@@ -173,26 +199,3 @@ void resume_device_irqs(void)
 	resume_irqs(false);
 }
 EXPORT_SYMBOL_GPL(resume_device_irqs);
-
-/**
- * check_wakeup_irqs - check if any wake-up interrupts are pending
- */
-int check_wakeup_irqs(void)
-{
-	struct irq_desc *desc;
-	int irq;
-
-	for_each_irq_desc(irq, desc) {
-		/*
-		 * Only interrupts which are marked as wakeup source
-		 * and have not been disabled before the suspend check
-		 * can abort suspend.
-		 */
-		if (irqd_is_wakeup_set(&desc->irq_data)) {
-			if (desc->depth == 1 && desc->istate & IRQS_PENDING)
-				return -EBUSY;
-		}
-	}
-
-	return 0;
-}
Index: linux-pm/kernel/irq/internals.h
===================================================================
--- linux-pm.orig/kernel/irq/internals.h
+++ linux-pm/kernel/irq/internals.h
@@ -196,9 +196,11 @@ static inline void kstat_incr_irqs_this_
 }
 
 #ifdef CONFIG_PM_SLEEP
+bool irq_pm_check_wakeup(struct irq_desc *desc);
 void irq_pm_install_action(struct irq_desc *desc, struct irqaction *action);
 void irq_pm_remove_action(struct irq_desc *desc, struct irqaction *action);
 #else
+static inline bool irq_pm_check_wakeup(struct irq_desc *desc) { return false; }
 static inline void
 irq_pm_install_action(struct irq_desc *desc, struct irqaction *action) { }
 static inline void

[PATCH 11/13] x86 / PM: Set IRQCHIP_SKIP_SET_WAKE for IOAPIC IRQ chip objects

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Set the IRQCHIP_SKIP_SET_WAKE for IOAPIC IRQ chip objects so that
interrupts from them can work as wakeup interrupts for suspend-to-idle.

After this change, running enable_irq_wake() on one of the IRQs in
question will succeed and IRQD_WAKEUP_STATE will be set for it, so
all of the suspend-to-idle wakeup mechanics introduced previously
will work for it automatically.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 arch/x86/kernel/apic/io_apic.c |    5 +++++
 1 file changed, 5 insertions(+)

Index: linux-pm/arch/x86/kernel/apic/io_apic.c
===================================================================
--- linux-pm.orig/arch/x86/kernel/apic/io_apic.c
+++ linux-pm/arch/x86/kernel/apic/io_apic.c
@@ -2618,6 +2618,7 @@ static struct irq_chip ioapic_chip __rea
 	.irq_eoi		= ack_apic_level,
 	.irq_set_affinity	= native_ioapic_set_affinity,
 	.irq_retrigger		= ioapic_retrigger_irq,
+	.flags			= IRQCHIP_SKIP_SET_WAKE,
 };
 
 static inline void init_IO_APIC_traps(void)
@@ -3168,6 +3169,7 @@ static struct irq_chip msi_chip = {
 	.irq_ack		= ack_apic_edge,
 	.irq_set_affinity	= msi_set_affinity,
 	.irq_retrigger		= ioapic_retrigger_irq,
+	.flags			= IRQCHIP_SKIP_SET_WAKE,
 };
 
 int setup_msi_irq(struct pci_dev *dev, struct msi_desc *msidesc,
@@ -3266,6 +3268,7 @@ static struct irq_chip dmar_msi_type = {
 	.irq_ack		= ack_apic_edge,
 	.irq_set_affinity	= dmar_msi_set_affinity,
 	.irq_retrigger		= ioapic_retrigger_irq,
+	.flags			= IRQCHIP_SKIP_SET_WAKE,
 };
 
 int arch_setup_dmar_msi(unsigned int irq)
@@ -3316,6 +3319,7 @@ static struct irq_chip hpet_msi_type = {
 	.irq_ack = ack_apic_edge,
 	.irq_set_affinity = hpet_msi_set_affinity,
 	.irq_retrigger = ioapic_retrigger_irq,
+	.flags = IRQCHIP_SKIP_SET_WAKE,
 };
 
 int default_setup_hpet_msi(unsigned int irq, unsigned int id)
@@ -3379,6 +3383,7 @@ static struct irq_chip ht_irq_chip = {
 	.irq_ack		= ack_apic_edge,
 	.irq_set_affinity	= ht_set_affinity,
 	.irq_retrigger		= ioapic_retrigger_irq,
+	.flags			= IRQCHIP_SKIP_SET_WAKE,
 };
 
 int arch_setup_ht_irq(unsigned int irq, struct pci_dev *dev)

[PATCH 12/13] PCI / PM: Make PCIe PME interrupts wake up from suspend-to-idle

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

To make PCIe PME interrupts wake up the system from suspend to idle,
make the PME driver use enable_irq_wake() on the IRQ during system
suspend (if there are any wakeup devices below the given PCIe port)
without disabling PME interrupts.  This way, an interrupt will still
trigger if a wakeup event happens and the system will be woken up (or
system suspend in progress will be aborted) by means of the new
mechanics introduced previously.

This change allows Wake-on-LAN to be used for wakeup from
suspend-to-idle on my MSI Wind tesbed netbook.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/pci/pcie/pme.c |   61 ++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 51 insertions(+), 10 deletions(-)

Index: linux-pm/drivers/pci/pcie/pme.c
===================================================================
--- linux-pm.orig/drivers/pci/pcie/pme.c
+++ linux-pm/drivers/pci/pcie/pme.c
@@ -41,11 +41,17 @@ static int __init pcie_pme_setup(char *s
 }
 __setup("pcie_pme=", pcie_pme_setup);
 
+enum pme_suspend_level {
+	PME_SUSPEND_NONE = 0,
+	PME_SUSPEND_WAKEUP,
+	PME_SUSPEND_NOIRQ,
+};
+
 struct pcie_pme_service_data {
 	spinlock_t lock;
 	struct pcie_device *srv;
 	struct work_struct work;
-	bool noirq; /* Don't enable the PME interrupt used by this service. */
+	enum pme_suspend_level suspend_level;
 };
 
 /**
@@ -223,7 +229,7 @@ static void pcie_pme_work_fn(struct work
 	spin_lock_irq(&data->lock);
 
 	for (;;) {
-		if (data->noirq)
+		if (data->suspend_level != PME_SUSPEND_NONE)
 			break;
 
 		pcie_capability_read_dword(port, PCI_EXP_RTSTA, &rtsta);
@@ -250,7 +256,7 @@ static void pcie_pme_work_fn(struct work
 		spin_lock_irq(&data->lock);
 	}
 
-	if (!data->noirq)
+	if (data->suspend_level == PME_SUSPEND_NONE)
 		pcie_pme_interrupt_enable(port, true);
 
 	spin_unlock_irq(&data->lock);
@@ -367,6 +373,21 @@ static int pcie_pme_probe(struct pcie_de
 	return ret;
 }
 
+static bool pcie_pme_check_wakeup(struct pci_bus *bus)
+{
+	struct pci_dev *dev;
+
+	if (!bus)
+		return false;
+
+	list_for_each_entry(dev, &bus->devices, bus_list)
+		if (device_may_wakeup(&dev->dev)
+		    || pcie_pme_check_wakeup(dev->subordinate))
+			return true;
+
+	return false;
+}
+
 /**
  * pcie_pme_suspend - Suspend PCIe PME service device.
  * @srv: PCIe service device to suspend.
@@ -375,11 +396,26 @@ static int pcie_pme_suspend(struct pcie_
 {
 	struct pcie_pme_service_data *data = get_service_data(srv);
 	struct pci_dev *port = srv->port;
+	bool wakeup;
 
+	if (device_may_wakeup(&port->dev)) {
+		wakeup = true;
+	} else {
+		down_read(&pci_bus_sem);
+		wakeup = pcie_pme_check_wakeup(port->subordinate);
+		up_read(&pci_bus_sem);
+	}
 	spin_lock_irq(&data->lock);
-	pcie_pme_interrupt_enable(port, false);
-	pcie_clear_root_pme_status(port);
-	data->noirq = true;
+	if (wakeup) {
+		enable_irq_wake(srv->irq);
+		data->suspend_level = PME_SUSPEND_WAKEUP;
+	} else {
+		struct pci_dev *port = srv->port;
+
+		pcie_pme_interrupt_enable(port, false);
+		pcie_clear_root_pme_status(port);
+		data->suspend_level = PME_SUSPEND_NOIRQ;
+	}
 	spin_unlock_irq(&data->lock);
 
 	synchronize_irq(srv->irq);
@@ -394,12 +430,17 @@ static int pcie_pme_suspend(struct pcie_
 static int pcie_pme_resume(struct pcie_device *srv)
 {
 	struct pcie_pme_service_data *data = get_service_data(srv);
-	struct pci_dev *port = srv->port;
 
 	spin_lock_irq(&data->lock);
-	data->noirq = false;
-	pcie_clear_root_pme_status(port);
-	pcie_pme_interrupt_enable(port, true);
+	if (data->suspend_level == PME_SUSPEND_NOIRQ) {
+		struct pci_dev *port = srv->port;
+
+		pcie_clear_root_pme_status(port);
+		pcie_pme_interrupt_enable(port, true);
+	} else {
+		disable_irq_wake(srv->irq);
+	}
+	data->suspend_level = PME_SUSPEND_NONE;
 	spin_unlock_irq(&data->lock);
 
 	return 0;

[PATCH 13/13] PM / genirq: Document rules related to system suspend and interrupts

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Add a document describing how IRQs are managed during system suspend
and resume, how wakeup interrupts work and what the IRQF_NO_SUSPEND
flag is supposed to be used for.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 Documentation/power/suspend-and-interrupts.txt |  123 +++++++++++++++++++++++++
 1 file changed, 123 insertions(+)

Index: linux-pm/Documentation/power/suspend-and-interrupts.txt
===================================================================
--- /dev/null
+++ linux-pm/Documentation/power/suspend-and-interrupts.txt
@@ -0,0 +1,123 @@
+System Suspend and Device Interrupts
+
+Copyright (C) 2014 Intel Corp.
+Author: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
+
+
+Suspending and Resuming Device IRQs
+-----------------------------------
+
+Device interrupt request lines (IRQs) are generally disabled during system
+suspend after the "late" phase of suspending devices (that is, after all of the
+->prepare, ->suspend and ->suspend_late callbacks have been executed for all
+devices).  That is done by suspend_device_irqs().
+
+The rationale for doing so is that after the "late" phase of device suspend
+there is no legitimate reason why any interrupts from suspended devices should
+trigger and if any devices have not been suspended properly yet, it is better to
+block interrupts from them anyway.  Also, in the past we had problems with
+interrupt handlers for shared IRQs that device drivers implementing them were
+not prepared for interrupts triggering after their devices had been suspended.
+In some cases they would attempt to access, for example, memory address spaces
+of suspended devices and cause unpredictable behavior to ensue as a result.
+Unfortunately, such problems are very difficult to debug and the introduction
+of suspend_device_irqs(), along with the "noirq" phase of device suspend and
+resume, was the only practical way to mitigate them.
+
+Device IRQs are re-enabled during system resume, right before the "early" phase
+of resuming devices (that is, before starting to execute ->resume_early
+callbacks for devices).  The function doing that is resume_device_irqs().
+
+
+The IRQF_NO_SUSPEND Flag
+------------------------
+
+There are interrupts that can legitimately trigger during the entire system
+suspend-resume cycle, including the "noirq" phases of suspending and resuming
+devices as well as during the time when nonboot CPUs are taken offline and
+brought back online.  That applies to timer interrupts in the first place,
+but also to IPIs and to some other special-purpose interrupts.
+
+The IRQF_NO_SUSPEND flag is used to indicate that to the IRQ subsystem when
+requesting a special-purpose interrupt.  It causes suspend_device_irqs() to
+leave the corresponding IRQ enabled so as to allow the interrupt to work all
+the time as expected.
+
+Note that the IRQF_NO_SUSPEND flag affects the entire IRQ and not just one
+user of it.  Thus, if the IRQ is shared, all of the interrupt handlers installed
+for it will be executed as usual after suspend_device_irqs(), even if the
+IRQF_NO_SUSPEND flag was not passed to request_irq() (or equivalent) by some of
+the IRQ's users.  For this reason, using IRQF_NO_SUSPEND and IRQF_SHARED at the
+same time should be avoided.
+
+
+System Wakeup Interrupts, enable_irq_wake() and disable_irq_wake()
+------------------------------------------------------------------
+
+System wakeup interrupts generally need to be configured to wake up the system
+from sleep states, especially if they are used for different purposes (e.g. as
+I/O interrupts) in the working state.
+
+That may involve turning on a special signal handling logic within the platform
+(such as an SoC) so that signals from a given line are routed in a different way
+during system sleep so as to trigger a system wakeup when needed.  For example,
+the platform may include a dedicated interrupt controller used specifically for
+handling system wakeup events.  Then, if a given interrupt line is supposed to
+wake up the system from sleep sates, the corresponding input of that interrupt
+controller needs to be enabled to receive signals from the line in question.
+After wakeup, it generally is better to disable that input to prevent the
+dedicated controller from triggering interrupts unnecessarily.
+
+The IRQ subsystem provides two helper functions to be used by device drivers for
+those purposes.  Namely, enable_irq_wake() turns on the platform's logic for
+handling the given IRQ as a system wakeup interrupt line and disable_irq_wake()
+turns that logic off.
+
+Calling enable_irq_wake() causes suspend_device_irqs() to treat the given IRQ
+in a special way.  Namely, the IRQ remains enabled, by on the first interrupt
+it will be disabled, marked as pending and "suspended" so that it will be
+re-enabled by resume_device_irqs() during the subsequent system resume.  Also
+the PM core is notified about the event which casues the system suspend in
+progress to be aborted (that doesn't have to happen immediately, but at one
+of the points where the suspend thread looks for pending wakeup events).
+
+This way every interrupt from a wakeup interrupt source will either cause the
+system suspend currently in progress to be aborted or wake up the system if
+already suspended.  However, after suspend_device_irqs() interrupt handlers are
+not executed for system wakeup IRQs.  They are only executed for IRQF_NO_SUSPEND
+IRQs at that time, but those IRQs should not be configured for system wakeup
+using enable_irq_wake().
+
+
+Interrupts and Suspend-to-Idle
+------------------------------
+
+Suspend-to-idle (also known as the "freeze" sleep state) is a relatively new
+system sleep state that works by idling all of the processors and waiting for
+interrupts right after the "noirq" phase of suspending devices.
+
+Of course, this means that all of the interrupts with the IRQF_NO_SUSPEND flag
+set will bring CPUs out of idle while in that state, but they will not cause the
+IRQ subsystem to trigger a system wakeup.
+
+System wakeup interrupts, in turn, will trigger wakeup from suspend-to-idle in
+analogy with what they do in the full system suspend case.  The only difference
+is that the wakeup from suspend-to-idle is signaled using the usual working
+state interrupt delivery mechanisms and doesn't require the platform to use
+any special interrupt handling logic for it to work.
+
+
+IRQF_NO_SUSPEND and enable_irq_wake()
+-------------------------------------
+
+There are no valid reasons to use both enable_irq_wake() and the IRQF_NO_SUSPEND
+flag on the same IRQ.
+
+First of all, if the IRQ is not shared, the rules for handling IRQF_NO_SUSPEND
+interrupts (interrupt handlers are invoked after suspend_device_irqs()) are
+directly at odds with the rules for handling system wakeup interrupts (interrupt
+handlers are not invoked after suspend_device_irqs()).
+
+Second, both enable_irq_wake() and IRQF_NO_SUSPEND apply to entire IRQs and not
+to individual interrupt handlers, so sharing an IRQ between a system wakeup
+interrupt source and an IRQF_NO_SUSPEND interrupt source does not make sense.