Re: [PATCH 1/3] irq / PM: New driver interface for wakeup interruptsn
On Sunday, August 03, 2014 03:42:49 PM Rafael J. Wysocki wrote:
> On Saturday, August 02, 2014 03:31:01 AM Rafael J. Wysocki wrote:
> > On Friday, August 01, 2014 04:29:40 PM Rafael J. Wysocki wrote:
> > > On Friday, August 01, 2014 03:43:21 PM Thomas Gleixner wrote:
> > > > On Fri, 1 Aug 2014, Rafael J. Wysocki wrote:
> > > > > OK, I guess "IRQ_HANDLED from a wakeup interrupt" may be interpreted
> > > > > as
> > > > > IRQ_HANDLED_PMWAKE. On the other hand, if that's going to be handled
> > > > > in
> > > > > handle_irq_event_percpu(), then using a special return code would
> > > > > save us
> > > > > a brach for IRQ_HANDLED interrupts. We could convert it to
> > > > > IRQ_HANDLED
> > > > > immediately then.
> > > >
> > > > We can handle it at the end of the function by calling
> > > > note_interrupt() unconditionally do the following there:
> > > >
> > > > if (suspended) {
> > > > if (ret == IRQ_NONE) {
> > > > if (shared)
> > > >yell_and_abort_or_resume();
> > > > } else {
> > > > abort_or_resume();
> > > > }
> > > > }
> > > > if (noirqdebug)
> > > > return;
> > >
> > > I see.
> > >
> > > > > OK, I'll take a stab at the IRQF_SHARED thing if you don't mind.
> > > >
> > > > Definitely not :)
> > > >
> > > > > Here's my current understanding of what can be done for
> > > > > IRQF_NO_SUSPEND.
> > > > >
> > > > > In suspend_device_irqs():
> > > > >
> > > > > (1) If all actions in the list have the same setting (eg.
> > > > > IRQF_NO_SUSPEND unset),
> > > > > keep the current behavior.
> > > > > (2) If the actions have different settings:
> > > > > - Actions with IRQF_NO_SUSPEND set are not modified.
> > > > > - Actions with IRQF_NO_SUSPEND unset are switched over to a stub
> > > > > handler.
> > > > > - IRQS_SUSPEND_MODE (new flag) is set for the IRQ.
> > > >
> > > > Can we please do that in setup_irq() and let the shared ones always
> > > > run through the stub? That keeps suspend/resume_device_irqs() simple.
> > >
> > > OK
> >
> > I've tried to do that, but encountered a problem. The stub handler is
> > called
> > with irq, dev_id as arguments and for the "interrupts are not suspended"
> > case
> > (common) it has to use irq to find the irq_desc and then it has to use
> > dev_id
> > to find the irqaction to call the original handler from there. That seemed
> > to
> > be a bit too much of a penalty to me. Especially for people who never
> > suspend
> > their machines. :-)
> >
> > For this reason, I went for changing handlers in suspend_device_irqs() and
> > back in resume_device_irqs(). That's not terribly complicated (the
> > restoration
> > in particular is pretty simple) and it should be easily reusable for the
> > wakeup
> > interrupts case. resume_device_irqs() wouldn't need any more changes for
> > that,
> > for example. It minimally affects systems that don't suspend too.
> >
> > I also ended up adding a new interrupt handler return code (IRQ_SUSPENDED).
> > I could add a new irq_desc flag instead, but then the new code in
> > suspend_device_irqs()
> > and the new check in note_interrupt() would need to be slightly more
> > complicated.
>
> Actually, if we have a global irqs_suspended state variable, it will be much
> simpler to handle wakeup interrupts going forward, because in that case we'll
> be able to keep their original interrupt handlers and IRQ_HANDLED from them
> will be interpreted as a wakeup event automatically.
I realized that with this, suspend_device_irqs() will be racy, because it
switches handlers and modifies irqs_suspended at different times then.
I'm not sure how much of a problem that would be, but it's better to
ensure that both the handler and note_interrupt() will be "switched" at
the same time, which means addin a wrapper handler at the __setup_irq()
time, after all.
To avoid the problem mentioned above, I'm using dev_id to store the action's
address and an additional "real dev_id" field to store the original value.
This version seems to work.
Rafael
---
drivers/base/power/main.c |1
drivers/base/power/wakeup.c | 20
include/linux/interrupt.h |4 +++
include/linux/irqdesc.h |1
include/linux/suspend.h |3 ++
kernel/irq/handle.c |3 --
kernel/irq/internals.h |3 ++
kernel/irq/manage.c | 53 ++--
kernel/irq/pm.c | 10
kernel/irq/spurious.c | 13 ++
10 files changed, 103 insertions(+), 8 deletions(-)
Index: linux-pm/kernel/irq/manage.c
===
--- linux-pm.orig/kernel/irq/manage.c
+++ linux-pm/kernel/irq/manage.c
@@ -385,7 +385,7 @@ setup_affinity(unsigned int irq, struct
void __disable_irq(struct irq_desc *desc, unsigned int irq, bool suspend)
{
if (suspend) {
- if (!desc->action ||
Re: [PATCH 1/3] irq / PM: New driver interface for wakeup interruptsn
On Saturday, August 02, 2014 03:31:01 AM Rafael J. Wysocki wrote:
> On Friday, August 01, 2014 04:29:40 PM Rafael J. Wysocki wrote:
> > On Friday, August 01, 2014 03:43:21 PM Thomas Gleixner wrote:
> > > On Fri, 1 Aug 2014, Rafael J. Wysocki wrote:
> > > > OK, I guess "IRQ_HANDLED from a wakeup interrupt" may be interpreted as
> > > > IRQ_HANDLED_PMWAKE. On the other hand, if that's going to be handled in
> > > > handle_irq_event_percpu(), then using a special return code would save
> > > > us
> > > > a brach for IRQ_HANDLED interrupts. We could convert it to IRQ_HANDLED
> > > > immediately then.
> > >
> > > We can handle it at the end of the function by calling
> > > note_interrupt() unconditionally do the following there:
> > >
> > > if (suspended) {
> > >if (ret == IRQ_NONE) {
> > > if (shared)
> > > yell_and_abort_or_resume();
> > > } else {
> > > abort_or_resume();
> > > }
> > > }
> > > if (noirqdebug)
> > >return;
> >
> > I see.
> >
> > > > OK, I'll take a stab at the IRQF_SHARED thing if you don't mind.
> > >
> > > Definitely not :)
> > >
> > > > Here's my current understanding of what can be done for IRQF_NO_SUSPEND.
> > > >
> > > > In suspend_device_irqs():
> > > >
> > > > (1) If all actions in the list have the same setting (eg.
> > > > IRQF_NO_SUSPEND unset),
> > > > keep the current behavior.
> > > > (2) If the actions have different settings:
> > > > - Actions with IRQF_NO_SUSPEND set are not modified.
> > > > - Actions with IRQF_NO_SUSPEND unset are switched over to a stub
> > > > handler.
> > > > - IRQS_SUSPEND_MODE (new flag) is set for the IRQ.
> > >
> > > Can we please do that in setup_irq() and let the shared ones always
> > > run through the stub? That keeps suspend/resume_device_irqs() simple.
> >
> > OK
>
> I've tried to do that, but encountered a problem. The stub handler is called
> with irq, dev_id as arguments and for the "interrupts are not suspended" case
> (common) it has to use irq to find the irq_desc and then it has to use dev_id
> to find the irqaction to call the original handler from there. That seemed to
> be a bit too much of a penalty to me. Especially for people who never suspend
> their machines. :-)
>
> For this reason, I went for changing handlers in suspend_device_irqs() and
> back in resume_device_irqs(). That's not terribly complicated (the
> restoration
> in particular is pretty simple) and it should be easily reusable for the
> wakeup
> interrupts case. resume_device_irqs() wouldn't need any more changes for
> that,
> for example. It minimally affects systems that don't suspend too.
>
> I also ended up adding a new interrupt handler return code (IRQ_SUSPENDED).
> I could add a new irq_desc flag instead, but then the new code in
> suspend_device_irqs()
> and the new check in note_interrupt() would need to be slightly more
> complicated.
Actually, if we have a global irqs_suspended state variable, it will be much
simpler to handle wakeup interrupts going forward, because in that case we'll
be able to keep their original interrupt handlers and IRQ_HANDLED from them
will be interpreted as a wakeup event automatically.
Updated prototype follows.
Rafael
---
drivers/base/power/main.c |1 +
drivers/base/power/wakeup.c | 20
include/linux/interrupt.h |2 ++
include/linux/suspend.h |3 +++
kernel/irq/handle.c |3 +--
kernel/irq/internals.h |2 ++
kernel/irq/manage.c | 35 ++-
kernel/irq/pm.c | 10 ++
kernel/irq/spurious.c | 24
9 files changed, 93 insertions(+), 7 deletions(-)
Index: linux-pm/kernel/irq/manage.c
===
--- linux-pm.orig/kernel/irq/manage.c
+++ linux-pm/kernel/irq/manage.c
@@ -382,11 +382,36 @@ setup_affinity(unsigned int irq, struct
}
#endif
+/*
+ * Dummy handler for shared interrupts to use during system suspend.
+ */
+static irqreturn_t irq_pm_dummy_handler(int irq, void *dev_id)
+{
+ return IRQ_NONE;
+}
+
void __disable_irq(struct irq_desc *desc, unsigned int irq, bool suspend)
{
if (suspend) {
- if (!desc->action || (desc->action->flags & IRQF_NO_SUSPEND))
+ struct irqaction *action;
+
+ if (!desc->action)
return;
+
+ for (action = desc->action; action; action = action->next) {
+ if (action->flags & IRQF_NO_SUSPEND)
+ break;
+ }
+ if (action) {
+ for (action = desc->action; action; action =
action->next) {
+ if (!(action->flags & IRQF_NO_SUSPEND)) {
+ action->saved_handler = action->handler;
+
Re: [PATCH 1/3] irq / PM: New driver interface for wakeup interruptsn
On Saturday, August 02, 2014 03:31:01 AM Rafael J. Wysocki wrote:
On Friday, August 01, 2014 04:29:40 PM Rafael J. Wysocki wrote:
On Friday, August 01, 2014 03:43:21 PM Thomas Gleixner wrote:
On Fri, 1 Aug 2014, Rafael J. Wysocki wrote:
OK, I guess IRQ_HANDLED from a wakeup interrupt may be interpreted as
IRQ_HANDLED_PMWAKE. On the other hand, if that's going to be handled in
handle_irq_event_percpu(), then using a special return code would save
us
a brach for IRQ_HANDLED interrupts. We could convert it to IRQ_HANDLED
immediately then.
We can handle it at the end of the function by calling
note_interrupt() unconditionally do the following there:
if (suspended) {
if (ret == IRQ_NONE) {
if (shared)
yell_and_abort_or_resume();
} else {
abort_or_resume();
}
}
if (noirqdebug)
return;
I see.
OK, I'll take a stab at the IRQF_SHARED thing if you don't mind.
Definitely not :)
Here's my current understanding of what can be done for IRQF_NO_SUSPEND.
In suspend_device_irqs():
(1) If all actions in the list have the same setting (eg.
IRQF_NO_SUSPEND unset),
keep the current behavior.
(2) If the actions have different settings:
- Actions with IRQF_NO_SUSPEND set are not modified.
- Actions with IRQF_NO_SUSPEND unset are switched over to a stub
handler.
- IRQS_SUSPEND_MODE (new flag) is set for the IRQ.
Can we please do that in setup_irq() and let the shared ones always
run through the stub? That keeps suspend/resume_device_irqs() simple.
OK
I've tried to do that, but encountered a problem. The stub handler is called
with irq, dev_id as arguments and for the interrupts are not suspended case
(common) it has to use irq to find the irq_desc and then it has to use dev_id
to find the irqaction to call the original handler from there. That seemed to
be a bit too much of a penalty to me. Especially for people who never suspend
their machines. :-)
For this reason, I went for changing handlers in suspend_device_irqs() and
back in resume_device_irqs(). That's not terribly complicated (the
restoration
in particular is pretty simple) and it should be easily reusable for the
wakeup
interrupts case. resume_device_irqs() wouldn't need any more changes for
that,
for example. It minimally affects systems that don't suspend too.
I also ended up adding a new interrupt handler return code (IRQ_SUSPENDED).
I could add a new irq_desc flag instead, but then the new code in
suspend_device_irqs()
and the new check in note_interrupt() would need to be slightly more
complicated.
Actually, if we have a global irqs_suspended state variable, it will be much
simpler to handle wakeup interrupts going forward, because in that case we'll
be able to keep their original interrupt handlers and IRQ_HANDLED from them
will be interpreted as a wakeup event automatically.
Updated prototype follows.
Rafael
---
drivers/base/power/main.c |1 +
drivers/base/power/wakeup.c | 20
include/linux/interrupt.h |2 ++
include/linux/suspend.h |3 +++
kernel/irq/handle.c |3 +--
kernel/irq/internals.h |2 ++
kernel/irq/manage.c | 35 ++-
kernel/irq/pm.c | 10 ++
kernel/irq/spurious.c | 24
9 files changed, 93 insertions(+), 7 deletions(-)
Index: linux-pm/kernel/irq/manage.c
===
--- linux-pm.orig/kernel/irq/manage.c
+++ linux-pm/kernel/irq/manage.c
@@ -382,11 +382,36 @@ setup_affinity(unsigned int irq, struct
}
#endif
+/*
+ * Dummy handler for shared interrupts to use during system suspend.
+ */
+static irqreturn_t irq_pm_dummy_handler(int irq, void *dev_id)
+{
+ return IRQ_NONE;
+}
+
void __disable_irq(struct irq_desc *desc, unsigned int irq, bool suspend)
{
if (suspend) {
- if (!desc-action || (desc-action-flags IRQF_NO_SUSPEND))
+ struct irqaction *action;
+
+ if (!desc-action)
return;
+
+ for (action = desc-action; action; action = action-next) {
+ if (action-flags IRQF_NO_SUSPEND)
+ break;
+ }
+ if (action) {
+ for (action = desc-action; action; action =
action-next) {
+ if (!(action-flags IRQF_NO_SUSPEND)) {
+ action-saved_handler = action-handler;
+ action-handler = irq_pm_dummy_handler;
+ }
+ }
+ return;
+ }
+
desc-istate |= IRQS_SUSPENDED;
Re: [PATCH 1/3] irq / PM: New driver interface for wakeup interruptsn
On Sunday, August 03, 2014 03:42:49 PM Rafael J. Wysocki wrote:
On Saturday, August 02, 2014 03:31:01 AM Rafael J. Wysocki wrote:
On Friday, August 01, 2014 04:29:40 PM Rafael J. Wysocki wrote:
On Friday, August 01, 2014 03:43:21 PM Thomas Gleixner wrote:
On Fri, 1 Aug 2014, Rafael J. Wysocki wrote:
OK, I guess IRQ_HANDLED from a wakeup interrupt may be interpreted
as
IRQ_HANDLED_PMWAKE. On the other hand, if that's going to be handled
in
handle_irq_event_percpu(), then using a special return code would
save us
a brach for IRQ_HANDLED interrupts. We could convert it to
IRQ_HANDLED
immediately then.
We can handle it at the end of the function by calling
note_interrupt() unconditionally do the following there:
if (suspended) {
if (ret == IRQ_NONE) {
if (shared)
yell_and_abort_or_resume();
} else {
abort_or_resume();
}
}
if (noirqdebug)
return;
I see.
OK, I'll take a stab at the IRQF_SHARED thing if you don't mind.
Definitely not :)
Here's my current understanding of what can be done for
IRQF_NO_SUSPEND.
In suspend_device_irqs():
(1) If all actions in the list have the same setting (eg.
IRQF_NO_SUSPEND unset),
keep the current behavior.
(2) If the actions have different settings:
- Actions with IRQF_NO_SUSPEND set are not modified.
- Actions with IRQF_NO_SUSPEND unset are switched over to a stub
handler.
- IRQS_SUSPEND_MODE (new flag) is set for the IRQ.
Can we please do that in setup_irq() and let the shared ones always
run through the stub? That keeps suspend/resume_device_irqs() simple.
OK
I've tried to do that, but encountered a problem. The stub handler is
called
with irq, dev_id as arguments and for the interrupts are not suspended
case
(common) it has to use irq to find the irq_desc and then it has to use
dev_id
to find the irqaction to call the original handler from there. That seemed
to
be a bit too much of a penalty to me. Especially for people who never
suspend
their machines. :-)
For this reason, I went for changing handlers in suspend_device_irqs() and
back in resume_device_irqs(). That's not terribly complicated (the
restoration
in particular is pretty simple) and it should be easily reusable for the
wakeup
interrupts case. resume_device_irqs() wouldn't need any more changes for
that,
for example. It minimally affects systems that don't suspend too.
I also ended up adding a new interrupt handler return code (IRQ_SUSPENDED).
I could add a new irq_desc flag instead, but then the new code in
suspend_device_irqs()
and the new check in note_interrupt() would need to be slightly more
complicated.
Actually, if we have a global irqs_suspended state variable, it will be much
simpler to handle wakeup interrupts going forward, because in that case we'll
be able to keep their original interrupt handlers and IRQ_HANDLED from them
will be interpreted as a wakeup event automatically.
I realized that with this, suspend_device_irqs() will be racy, because it
switches handlers and modifies irqs_suspended at different times then.
I'm not sure how much of a problem that would be, but it's better to
ensure that both the handler and note_interrupt() will be switched at
the same time, which means addin a wrapper handler at the __setup_irq()
time, after all.
To avoid the problem mentioned above, I'm using dev_id to store the action's
address and an additional real dev_id field to store the original value.
This version seems to work.
Rafael
---
drivers/base/power/main.c |1
drivers/base/power/wakeup.c | 20
include/linux/interrupt.h |4 +++
include/linux/irqdesc.h |1
include/linux/suspend.h |3 ++
kernel/irq/handle.c |3 --
kernel/irq/internals.h |3 ++
kernel/irq/manage.c | 53 ++--
kernel/irq/pm.c | 10
kernel/irq/spurious.c | 13 ++
10 files changed, 103 insertions(+), 8 deletions(-)
Index: linux-pm/kernel/irq/manage.c
===
--- linux-pm.orig/kernel/irq/manage.c
+++ linux-pm/kernel/irq/manage.c
@@ -385,7 +385,7 @@ 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 || desc-skip_suspend)
return;
desc-istate |= IRQS_SUSPENDED;
}
@@ -658,6 +658,44 @@ int irq_set_parent(int irq, int parent_i
}
#endif
+bool irqs_suspended
Re: [PATCH 1/3] irq / PM: New driver interface for wakeup interruptsn
On Friday, August 01, 2014 04:29:40 PM Rafael J. Wysocki wrote:
> On Friday, August 01, 2014 03:43:21 PM Thomas Gleixner wrote:
> > On Fri, 1 Aug 2014, Rafael J. Wysocki wrote:
> > > OK, I guess "IRQ_HANDLED from a wakeup interrupt" may be interpreted as
> > > IRQ_HANDLED_PMWAKE. On the other hand, if that's going to be handled in
> > > handle_irq_event_percpu(), then using a special return code would save us
> > > a brach for IRQ_HANDLED interrupts. We could convert it to IRQ_HANDLED
> > > immediately then.
> >
> > We can handle it at the end of the function by calling
> > note_interrupt() unconditionally do the following there:
> >
> > if (suspended) {
> > if (ret == IRQ_NONE) {
> > if (shared)
> >yell_and_abort_or_resume();
> > } else {
> > abort_or_resume();
> > }
> > }
> > if (noirqdebug)
> > return;
>
> I see.
>
> > > OK, I'll take a stab at the IRQF_SHARED thing if you don't mind.
> >
> > Definitely not :)
> >
> > > Here's my current understanding of what can be done for IRQF_NO_SUSPEND.
> > >
> > > In suspend_device_irqs():
> > >
> > > (1) If all actions in the list have the same setting (eg. IRQF_NO_SUSPEND
> > > unset),
> > > keep the current behavior.
> > > (2) If the actions have different settings:
> > > - Actions with IRQF_NO_SUSPEND set are not modified.
> > > - Actions with IRQF_NO_SUSPEND unset are switched over to a stub
> > > handler.
> > > - IRQS_SUSPEND_MODE (new flag) is set for the IRQ.
> >
> > Can we please do that in setup_irq() and let the shared ones always
> > run through the stub? That keeps suspend/resume_device_irqs() simple.
>
> OK
I've tried to do that, but encountered a problem. The stub handler is called
with irq, dev_id as arguments and for the "interrupts are not suspended" case
(common) it has to use irq to find the irq_desc and then it has to use dev_id
to find the irqaction to call the original handler from there. That seemed to
be a bit too much of a penalty to me. Especially for people who never suspend
their machines. :-)
For this reason, I went for changing handlers in suspend_device_irqs() and
back in resume_device_irqs(). That's not terribly complicated (the restoration
in particular is pretty simple) and it should be easily reusable for the wakeup
interrupts case. resume_device_irqs() wouldn't need any more changes for that,
for example. It minimally affects systems that don't suspend too.
I also ended up adding a new interrupt handler return code (IRQ_SUSPENDED).
I could add a new irq_desc flag instead, but then the new code in
suspend_device_irqs()
and the new check in note_interrupt() would need to be slightly more
complicated.
Below is my current prototype. Please have a look and let me know what you
think.
I gave it a little bit of testing with success.
Of course, I'd prefer to move the IRQ suspend/resume code to pm.c and get rid
of the third argument in __disable_irq() and __enable_irq() before making these
changes for real.
Rafael
---
drivers/base/power/main.c |1 +
drivers/base/power/wakeup.c | 20
include/linux/interrupt.h |2 ++
include/linux/irqreturn.h |2 ++
include/linux/suspend.h |3 +++
kernel/irq/handle.c |3 +--
kernel/irq/manage.c | 35 ++-
kernel/irq/pm.c |7 +--
kernel/irq/spurious.c | 16
9 files changed, 84 insertions(+), 5 deletions(-)
Index: linux-pm/kernel/irq/manage.c
===
--- linux-pm.orig/kernel/irq/manage.c
+++ linux-pm/kernel/irq/manage.c
@@ -382,11 +382,36 @@ setup_affinity(unsigned int irq, struct
}
#endif
+/*
+ * Dummy handler for shared interrupts to use during system suspend.
+ */
+static irqreturn_t irq_pm_dummy_handler(int irq, void *dev_id)
+{
+ return IRQ_SUSPENDED;
+}
+
void __disable_irq(struct irq_desc *desc, unsigned int irq, bool suspend)
{
if (suspend) {
- if (!desc->action || (desc->action->flags & IRQF_NO_SUSPEND))
+ struct irqaction *action;
+
+ if (!desc->action)
return;
+
+ for (action = desc->action; action; action = action->next) {
+ if (action->flags & IRQF_NO_SUSPEND)
+ break;
+ }
+ if (action) {
+ for (action = desc->action; action; action =
action->next) {
+ if (!(action->flags & IRQF_NO_SUSPEND)) {
+ action->saved_handler = action->handler;
+ action->handler = irq_pm_dummy_handler;
+ }
+ }
+ return;
+ }
+
desc->istate |= IRQS_SUSPENDED;
}
@@
Re: [PATCH 1/3] irq / PM: New driver interface for wakeup interruptsn
On Friday, August 01, 2014 03:43:21 PM Thomas Gleixner wrote:
> On Fri, 1 Aug 2014, Rafael J. Wysocki wrote:
> > OK, I guess "IRQ_HANDLED from a wakeup interrupt" may be interpreted as
> > IRQ_HANDLED_PMWAKE. On the other hand, if that's going to be handled in
> > handle_irq_event_percpu(), then using a special return code would save us
> > a brach for IRQ_HANDLED interrupts. We could convert it to IRQ_HANDLED
> > immediately then.
>
> We can handle it at the end of the function by calling
> note_interrupt() unconditionally do the following there:
>
> if (suspended) {
>if (ret == IRQ_NONE) {
> if (shared)
> yell_and_abort_or_resume();
> } else {
> abort_or_resume();
> }
> }
> if (noirqdebug)
>return;
I see.
> > OK, I'll take a stab at the IRQF_SHARED thing if you don't mind.
>
> Definitely not :)
>
> > Here's my current understanding of what can be done for IRQF_NO_SUSPEND.
> >
> > In suspend_device_irqs():
> >
> > (1) If all actions in the list have the same setting (eg. IRQF_NO_SUSPEND
> > unset),
> > keep the current behavior.
> > (2) If the actions have different settings:
> > - Actions with IRQF_NO_SUSPEND set are not modified.
> > - Actions with IRQF_NO_SUSPEND unset are switched over to a stub
> > handler.
> > - IRQS_SUSPEND_MODE (new flag) is set for the IRQ.
>
> Can we please do that in setup_irq() and let the shared ones always
> run through the stub? That keeps suspend/resume_device_irqs() simple.
OK
Rafael
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] irq / PM: New driver interface for wakeup interruptsn
On Fri, 1 Aug 2014, Rafael J. Wysocki wrote:
> OK, I guess "IRQ_HANDLED from a wakeup interrupt" may be interpreted as
> IRQ_HANDLED_PMWAKE. On the other hand, if that's going to be handled in
> handle_irq_event_percpu(), then using a special return code would save us
> a brach for IRQ_HANDLED interrupts. We could convert it to IRQ_HANDLED
> immediately then.
We can handle it at the end of the function by calling
note_interrupt() unconditionally do the following there:
if (suspended) {
if (ret == IRQ_NONE) {
if (shared)
yell_and_abort_or_resume();
} else {
abort_or_resume();
}
}
if (noirqdebug)
return;
> OK, I'll take a stab at the IRQF_SHARED thing if you don't mind.
Definitely not :)
> Here's my current understanding of what can be done for IRQF_NO_SUSPEND.
>
> In suspend_device_irqs():
>
> (1) If all actions in the list have the same setting (eg. IRQF_NO_SUSPEND
> unset),
> keep the current behavior.
> (2) If the actions have different settings:
> - Actions with IRQF_NO_SUSPEND set are not modified.
> - Actions with IRQF_NO_SUSPEND unset are switched over to a stub handler.
> - IRQS_SUSPEND_MODE (new flag) is set for the IRQ.
Can we please do that in setup_irq() and let the shared ones always
run through the stub? That keeps suspend/resume_device_irqs() simple.
Thanks,
tglx
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] irq / PM: New driver interface for wakeup interruptsn
On Friday, August 01, 2014 11:40:55 AM Thomas Gleixner wrote: > On Fri, 1 Aug 2014, Rafael J. Wysocki wrote: > > On Friday, August 01, 2014 12:16:23 AM Thomas Gleixner wrote: [cut] > > > > And now there's one more piece of it which is suspend-to-idle (aka > > > > "freeze"). > > > > That doesn't go all the way to syscore_suspend(), but basically stops > > > > after > > > > finishing the "noirq" phase of suspending devices. Then, it idles the > > > > CPUs > > > > and waits for interrupts that will take them out of idle. Only some of > > > > those > > > > interrupts are wakeup events, so it only resumes when > > > > __pm_wakeup_event() or > > > > __pm_relax() is called in the process of handling the interrupts. > > > > > > > > Of course, it could be implemented differently, but that was the > > > > simplest > > > > way to do that. It still can be changed, but I'd really like it not to > > > > have > > > > to go through all of the disabling nonboot CPUs and sysfore_suspend(), > > > > because > > > > that simply isn't necessary (and it avoids a metric ton of problems > > > > with CPU > > > > offline/online). And of course, it has to work on x86 too. > > > > > > Right. So one thing which would make the situation more clear is that > > > the interrupt handler needs to tell the core code about this by > > > returning something like IRQ_HANDLED_PMWAKE and the core kicks the > > > suspend-to-idle logic back to life. I'm not sure whether the extra > > > return value is actually necessary, we might even map it to > > > IRQ_HANDLED in the core as we know that we are in the suspend > > > state. > > > > I'm not sure I'm following you here. Do you mean that upon receiving > > IRQ_HANDLED_PMWAKE from an interrupt handler the core will know that > > this was a wakeup event and will trigger a resume from suspend-to-idle? > > Correct. Whether we need the extra return code is debatable. But yes, > we want to talk to the PM/suspend/resume thing at core level instead > of letting drivers use random interfaces which happen to be exported > for one reason or the other, but definitely not for the purpose of > random driver. OK, I guess "IRQ_HANDLED from a wakeup interrupt" may be interpreted as IRQ_HANDLED_PMWAKE. On the other hand, if that's going to be handled in handle_irq_event_percpu(), then using a special return code would save us a brach for IRQ_HANDLED interrupts. We could convert it to IRQ_HANDLED immediately then. [cut] > > I'm not sure about the ordering, though. It would be good to have a working > > replacement for the IRQF_NO_SUSPEND things that we'll be removing in 1, for > > example. So since we need to do 3) IRQF_SHARED for both IRQF_NO_SUSPEND and > > wakeup, as you said, would it be practical to start with that one? > > The numbering was not meant as ordering, it was just to seperate the > various issues which we need to look at. OK, I'll take a stab at the IRQF_SHARED thing if you don't mind. Here's my current understanding of what can be done for IRQF_NO_SUSPEND. In suspend_device_irqs(): (1) If all actions in the list have the same setting (eg. IRQF_NO_SUSPEND unset), keep the current behavior. (2) If the actions have different settings: - Actions with IRQF_NO_SUSPEND set are not modified. - Actions with IRQF_NO_SUSPEND unset are switched over to a stub handler. - IRQS_SUSPEND_MODE (new flag) is set for the IRQ. In note_interrupt(): If action_ret is IRQ_NONE and IRQS_SUSPEND_MODE is set for the IRQ, disable the IRQ, set IRQS_SUSPENDED for it and call system_wakeup(BAD_INTERRUPT) (that will abort suspend if still in progress or break the suspend-to-idle loop). In resume_device_irqs(): (1) If IRQS_SUSPEND_MODE is set, switch over actions with IRQF_NO_SUSPEND unset to their original handlers and clear the flag. Fall through. (2) If IRQS_SUSPENDED is set, clear the flag and enable the interrupt. The stub handler only needs to return IRQ_NONE unconditionally in that case. Does that make sense? Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] irq / PM: New driver interface for wakeup interruptsn
On Fri, 1 Aug 2014, Rafael J. Wysocki wrote: > On Friday, August 01, 2014 12:16:23 AM Thomas Gleixner wrote: > > Right, so instead of thinking about a proper solution driver folks just slap > > the next available thing on it w/o thinking about the consequences. But, > > thats partly our own fault due to lack of proper documentation. > > Prototyping with the next available thing is not generally wrong in my view > as long as this doesn't get to the code base without consideration. Prototyping is one thing, but we have real users which are simply wrong AFAICT. > > > And now there's one more piece of it which is suspend-to-idle (aka > > > "freeze"). > > > That doesn't go all the way to syscore_suspend(), but basically stops > > > after > > > finishing the "noirq" phase of suspending devices. Then, it idles the > > > CPUs > > > and waits for interrupts that will take them out of idle. Only some of > > > those > > > interrupts are wakeup events, so it only resumes when __pm_wakeup_event() > > > or > > > __pm_relax() is called in the process of handling the interrupts. > > > > > > Of course, it could be implemented differently, but that was the simplest > > > way to do that. It still can be changed, but I'd really like it not to > > > have > > > to go through all of the disabling nonboot CPUs and sysfore_suspend(), > > > because > > > that simply isn't necessary (and it avoids a metric ton of problems with > > > CPU > > > offline/online). And of course, it has to work on x86 too. > > > > Right. So one thing which would make the situation more clear is that > > the interrupt handler needs to tell the core code about this by > > returning something like IRQ_HANDLED_PMWAKE and the core kicks the > > suspend-to-idle logic back to life. I'm not sure whether the extra > > return value is actually necessary, we might even map it to > > IRQ_HANDLED in the core as we know that we are in the suspend > > state. > > I'm not sure I'm following you here. Do you mean that upon receiving > IRQ_HANDLED_PMWAKE from an interrupt handler the core will know that > this was a wakeup event and will trigger a resume from suspend-to-idle? Correct. Whether we need the extra return code is debatable. But yes, we want to talk to the PM/suspend/resume thing at core level instead of letting drivers use random interfaces which happen to be exported for one reason or the other, but definitely not for the purpose of random driver. > > There is no reason, why we can't flag the affected irq chips with > > IRQCHIP_SKIP_SET_WAKE. That was introduced to handle irq controllers > > which provide wakeup functionality but do not have this special extra > > magic which is implemented in ->irq_set_wake, i.e. talking to some PM > > controller directly. > > That may help, but then it won't prevent wakeup IRQs from being disabled by > suspend_device_irqs(). Do I think correctly that we can re-enable them > in the suspend-to-idle loop, before idling the CPUs? Yes. > > 2) check_wakeup_irqs() > > > >Call it in the suspend-to-idle case and deal with the detected > >suspend abort from there. If we don't do that, we basically force > >people to hack creative workarounds into their driver, be it > >abusing IRQF_NO_SUSPEND or other completely brainwrecked > >modifications. > > > >The approach of doing: > > > > 1) lazy disable > > 2) transition into suspend > > 3) check abort conditions > > > >is the only sane way to keep the real suspend and the > >suspend-to-idle stuff in line. > > Well, suspend-to-idle is a loop of the type > > (1) wait for an interrupt > (2) iterrupt happens -> if wakeup, resume; else goto (1) > > so I'm not sure how to match that with lazy disable? We basically would need > to > do something like: > > (1) enable all wakeup interrupts > (2) wait for an interrupt > (3) interrupt happens > (4) disable all interrupts enabled in (1) > (5) if wakeup, resume (we'll resume interrupts later); else goto (1) > > Is that what you mean? More or less, yes. Whether we do the explicit enable or the implicit leave it enabled thing is an implementation detail. We just need to chose one. > > Though for the PCIe root hub thing which has a shared MSI handler > > !?! we really want to implement that at the device level > > today. Sharing a MSI interrupt is just ass backwards. > > There are more reasons for doing that. A PCIe port is really one device > with multiple things to handle and they really need more coordination with > each other than we have in the current code. In my opinion we should > just use one PCIe port driver doing all of these things together instead of > three separate drivers handling multiple artificially cut out devices on a > virtual bus. > > Problem is, people either have no time or are too scared to do that. Well, if we don't want to end up with a complete unmaintainable mess, we need to find the time and the people who have enough
Re: [PATCH 1/3] irq / PM: New driver interface for wakeup interruptsn
On Fri, 1 Aug 2014, Rafael J. Wysocki wrote: On Friday, August 01, 2014 12:16:23 AM Thomas Gleixner wrote: Right, so instead of thinking about a proper solution driver folks just slap the next available thing on it w/o thinking about the consequences. But, thats partly our own fault due to lack of proper documentation. Prototyping with the next available thing is not generally wrong in my view as long as this doesn't get to the code base without consideration. Prototyping is one thing, but we have real users which are simply wrong AFAICT. And now there's one more piece of it which is suspend-to-idle (aka freeze). That doesn't go all the way to syscore_suspend(), but basically stops after finishing the noirq phase of suspending devices. Then, it idles the CPUs and waits for interrupts that will take them out of idle. Only some of those interrupts are wakeup events, so it only resumes when __pm_wakeup_event() or __pm_relax() is called in the process of handling the interrupts. Of course, it could be implemented differently, but that was the simplest way to do that. It still can be changed, but I'd really like it not to have to go through all of the disabling nonboot CPUs and sysfore_suspend(), because that simply isn't necessary (and it avoids a metric ton of problems with CPU offline/online). And of course, it has to work on x86 too. Right. So one thing which would make the situation more clear is that the interrupt handler needs to tell the core code about this by returning something like IRQ_HANDLED_PMWAKE and the core kicks the suspend-to-idle logic back to life. I'm not sure whether the extra return value is actually necessary, we might even map it to IRQ_HANDLED in the core as we know that we are in the suspend state. I'm not sure I'm following you here. Do you mean that upon receiving IRQ_HANDLED_PMWAKE from an interrupt handler the core will know that this was a wakeup event and will trigger a resume from suspend-to-idle? Correct. Whether we need the extra return code is debatable. But yes, we want to talk to the PM/suspend/resume thing at core level instead of letting drivers use random interfaces which happen to be exported for one reason or the other, but definitely not for the purpose of random driver. There is no reason, why we can't flag the affected irq chips with IRQCHIP_SKIP_SET_WAKE. That was introduced to handle irq controllers which provide wakeup functionality but do not have this special extra magic which is implemented in -irq_set_wake, i.e. talking to some PM controller directly. That may help, but then it won't prevent wakeup IRQs from being disabled by suspend_device_irqs(). Do I think correctly that we can re-enable them in the suspend-to-idle loop, before idling the CPUs? Yes. 2) check_wakeup_irqs() Call it in the suspend-to-idle case and deal with the detected suspend abort from there. If we don't do that, we basically force people to hack creative workarounds into their driver, be it abusing IRQF_NO_SUSPEND or other completely brainwrecked modifications. The approach of doing: 1) lazy disable 2) transition into suspend 3) check abort conditions is the only sane way to keep the real suspend and the suspend-to-idle stuff in line. Well, suspend-to-idle is a loop of the type (1) wait for an interrupt (2) iterrupt happens - if wakeup, resume; else goto (1) so I'm not sure how to match that with lazy disable? We basically would need to do something like: (1) enable all wakeup interrupts (2) wait for an interrupt (3) interrupt happens (4) disable all interrupts enabled in (1) (5) if wakeup, resume (we'll resume interrupts later); else goto (1) Is that what you mean? More or less, yes. Whether we do the explicit enable or the implicit leave it enabled thing is an implementation detail. We just need to chose one. Though for the PCIe root hub thing which has a shared MSI handler !?! we really want to implement that at the device level today. Sharing a MSI interrupt is just ass backwards. There are more reasons for doing that. A PCIe port is really one device with multiple things to handle and they really need more coordination with each other than we have in the current code. In my opinion we should just use one PCIe port driver doing all of these things together instead of three separate drivers handling multiple artificially cut out devices on a virtual bus. Problem is, people either have no time or are too scared to do that. Well, if we don't want to end up with a complete unmaintainable mess, we need to find the time and the people who have enough brains to do so. If there is a spurious interrupt on a shared edge type interrupt, which is crap by definition, but unfortunately reality, we
Re: [PATCH 1/3] irq / PM: New driver interface for wakeup interruptsn
On Friday, August 01, 2014 11:40:55 AM Thomas Gleixner wrote: On Fri, 1 Aug 2014, Rafael J. Wysocki wrote: On Friday, August 01, 2014 12:16:23 AM Thomas Gleixner wrote: [cut] And now there's one more piece of it which is suspend-to-idle (aka freeze). That doesn't go all the way to syscore_suspend(), but basically stops after finishing the noirq phase of suspending devices. Then, it idles the CPUs and waits for interrupts that will take them out of idle. Only some of those interrupts are wakeup events, so it only resumes when __pm_wakeup_event() or __pm_relax() is called in the process of handling the interrupts. Of course, it could be implemented differently, but that was the simplest way to do that. It still can be changed, but I'd really like it not to have to go through all of the disabling nonboot CPUs and sysfore_suspend(), because that simply isn't necessary (and it avoids a metric ton of problems with CPU offline/online). And of course, it has to work on x86 too. Right. So one thing which would make the situation more clear is that the interrupt handler needs to tell the core code about this by returning something like IRQ_HANDLED_PMWAKE and the core kicks the suspend-to-idle logic back to life. I'm not sure whether the extra return value is actually necessary, we might even map it to IRQ_HANDLED in the core as we know that we are in the suspend state. I'm not sure I'm following you here. Do you mean that upon receiving IRQ_HANDLED_PMWAKE from an interrupt handler the core will know that this was a wakeup event and will trigger a resume from suspend-to-idle? Correct. Whether we need the extra return code is debatable. But yes, we want to talk to the PM/suspend/resume thing at core level instead of letting drivers use random interfaces which happen to be exported for one reason or the other, but definitely not for the purpose of random driver. OK, I guess IRQ_HANDLED from a wakeup interrupt may be interpreted as IRQ_HANDLED_PMWAKE. On the other hand, if that's going to be handled in handle_irq_event_percpu(), then using a special return code would save us a brach for IRQ_HANDLED interrupts. We could convert it to IRQ_HANDLED immediately then. [cut] I'm not sure about the ordering, though. It would be good to have a working replacement for the IRQF_NO_SUSPEND things that we'll be removing in 1, for example. So since we need to do 3) IRQF_SHARED for both IRQF_NO_SUSPEND and wakeup, as you said, would it be practical to start with that one? The numbering was not meant as ordering, it was just to seperate the various issues which we need to look at. OK, I'll take a stab at the IRQF_SHARED thing if you don't mind. Here's my current understanding of what can be done for IRQF_NO_SUSPEND. In suspend_device_irqs(): (1) If all actions in the list have the same setting (eg. IRQF_NO_SUSPEND unset), keep the current behavior. (2) If the actions have different settings: - Actions with IRQF_NO_SUSPEND set are not modified. - Actions with IRQF_NO_SUSPEND unset are switched over to a stub handler. - IRQS_SUSPEND_MODE (new flag) is set for the IRQ. In note_interrupt(): If action_ret is IRQ_NONE and IRQS_SUSPEND_MODE is set for the IRQ, disable the IRQ, set IRQS_SUSPENDED for it and call system_wakeup(BAD_INTERRUPT) (that will abort suspend if still in progress or break the suspend-to-idle loop). In resume_device_irqs(): (1) If IRQS_SUSPEND_MODE is set, switch over actions with IRQF_NO_SUSPEND unset to their original handlers and clear the flag. Fall through. (2) If IRQS_SUSPENDED is set, clear the flag and enable the interrupt. The stub handler only needs to return IRQ_NONE unconditionally in that case. Does that make sense? Rafael -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] irq / PM: New driver interface for wakeup interruptsn
On Fri, 1 Aug 2014, Rafael J. Wysocki wrote:
OK, I guess IRQ_HANDLED from a wakeup interrupt may be interpreted as
IRQ_HANDLED_PMWAKE. On the other hand, if that's going to be handled in
handle_irq_event_percpu(), then using a special return code would save us
a brach for IRQ_HANDLED interrupts. We could convert it to IRQ_HANDLED
immediately then.
We can handle it at the end of the function by calling
note_interrupt() unconditionally do the following there:
if (suspended) {
if (ret == IRQ_NONE) {
if (shared)
yell_and_abort_or_resume();
} else {
abort_or_resume();
}
}
if (noirqdebug)
return;
OK, I'll take a stab at the IRQF_SHARED thing if you don't mind.
Definitely not :)
Here's my current understanding of what can be done for IRQF_NO_SUSPEND.
In suspend_device_irqs():
(1) If all actions in the list have the same setting (eg. IRQF_NO_SUSPEND
unset),
keep the current behavior.
(2) If the actions have different settings:
- Actions with IRQF_NO_SUSPEND set are not modified.
- Actions with IRQF_NO_SUSPEND unset are switched over to a stub handler.
- IRQS_SUSPEND_MODE (new flag) is set for the IRQ.
Can we please do that in setup_irq() and let the shared ones always
run through the stub? That keeps suspend/resume_device_irqs() simple.
Thanks,
tglx
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] irq / PM: New driver interface for wakeup interruptsn
On Friday, August 01, 2014 03:43:21 PM Thomas Gleixner wrote:
On Fri, 1 Aug 2014, Rafael J. Wysocki wrote:
OK, I guess IRQ_HANDLED from a wakeup interrupt may be interpreted as
IRQ_HANDLED_PMWAKE. On the other hand, if that's going to be handled in
handle_irq_event_percpu(), then using a special return code would save us
a brach for IRQ_HANDLED interrupts. We could convert it to IRQ_HANDLED
immediately then.
We can handle it at the end of the function by calling
note_interrupt() unconditionally do the following there:
if (suspended) {
if (ret == IRQ_NONE) {
if (shared)
yell_and_abort_or_resume();
} else {
abort_or_resume();
}
}
if (noirqdebug)
return;
I see.
OK, I'll take a stab at the IRQF_SHARED thing if you don't mind.
Definitely not :)
Here's my current understanding of what can be done for IRQF_NO_SUSPEND.
In suspend_device_irqs():
(1) If all actions in the list have the same setting (eg. IRQF_NO_SUSPEND
unset),
keep the current behavior.
(2) If the actions have different settings:
- Actions with IRQF_NO_SUSPEND set are not modified.
- Actions with IRQF_NO_SUSPEND unset are switched over to a stub
handler.
- IRQS_SUSPEND_MODE (new flag) is set for the IRQ.
Can we please do that in setup_irq() and let the shared ones always
run through the stub? That keeps suspend/resume_device_irqs() simple.
OK
Rafael
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] irq / PM: New driver interface for wakeup interruptsn
On Friday, August 01, 2014 04:29:40 PM Rafael J. Wysocki wrote:
On Friday, August 01, 2014 03:43:21 PM Thomas Gleixner wrote:
On Fri, 1 Aug 2014, Rafael J. Wysocki wrote:
OK, I guess IRQ_HANDLED from a wakeup interrupt may be interpreted as
IRQ_HANDLED_PMWAKE. On the other hand, if that's going to be handled in
handle_irq_event_percpu(), then using a special return code would save us
a brach for IRQ_HANDLED interrupts. We could convert it to IRQ_HANDLED
immediately then.
We can handle it at the end of the function by calling
note_interrupt() unconditionally do the following there:
if (suspended) {
if (ret == IRQ_NONE) {
if (shared)
yell_and_abort_or_resume();
} else {
abort_or_resume();
}
}
if (noirqdebug)
return;
I see.
OK, I'll take a stab at the IRQF_SHARED thing if you don't mind.
Definitely not :)
Here's my current understanding of what can be done for IRQF_NO_SUSPEND.
In suspend_device_irqs():
(1) If all actions in the list have the same setting (eg. IRQF_NO_SUSPEND
unset),
keep the current behavior.
(2) If the actions have different settings:
- Actions with IRQF_NO_SUSPEND set are not modified.
- Actions with IRQF_NO_SUSPEND unset are switched over to a stub
handler.
- IRQS_SUSPEND_MODE (new flag) is set for the IRQ.
Can we please do that in setup_irq() and let the shared ones always
run through the stub? That keeps suspend/resume_device_irqs() simple.
OK
I've tried to do that, but encountered a problem. The stub handler is called
with irq, dev_id as arguments and for the interrupts are not suspended case
(common) it has to use irq to find the irq_desc and then it has to use dev_id
to find the irqaction to call the original handler from there. That seemed to
be a bit too much of a penalty to me. Especially for people who never suspend
their machines. :-)
For this reason, I went for changing handlers in suspend_device_irqs() and
back in resume_device_irqs(). That's not terribly complicated (the restoration
in particular is pretty simple) and it should be easily reusable for the wakeup
interrupts case. resume_device_irqs() wouldn't need any more changes for that,
for example. It minimally affects systems that don't suspend too.
I also ended up adding a new interrupt handler return code (IRQ_SUSPENDED).
I could add a new irq_desc flag instead, but then the new code in
suspend_device_irqs()
and the new check in note_interrupt() would need to be slightly more
complicated.
Below is my current prototype. Please have a look and let me know what you
think.
I gave it a little bit of testing with success.
Of course, I'd prefer to move the IRQ suspend/resume code to pm.c and get rid
of the third argument in __disable_irq() and __enable_irq() before making these
changes for real.
Rafael
---
drivers/base/power/main.c |1 +
drivers/base/power/wakeup.c | 20
include/linux/interrupt.h |2 ++
include/linux/irqreturn.h |2 ++
include/linux/suspend.h |3 +++
kernel/irq/handle.c |3 +--
kernel/irq/manage.c | 35 ++-
kernel/irq/pm.c |7 +--
kernel/irq/spurious.c | 16
9 files changed, 84 insertions(+), 5 deletions(-)
Index: linux-pm/kernel/irq/manage.c
===
--- linux-pm.orig/kernel/irq/manage.c
+++ linux-pm/kernel/irq/manage.c
@@ -382,11 +382,36 @@ setup_affinity(unsigned int irq, struct
}
#endif
+/*
+ * Dummy handler for shared interrupts to use during system suspend.
+ */
+static irqreturn_t irq_pm_dummy_handler(int irq, void *dev_id)
+{
+ return IRQ_SUSPENDED;
+}
+
void __disable_irq(struct irq_desc *desc, unsigned int irq, bool suspend)
{
if (suspend) {
- if (!desc-action || (desc-action-flags IRQF_NO_SUSPEND))
+ struct irqaction *action;
+
+ if (!desc-action)
return;
+
+ for (action = desc-action; action; action = action-next) {
+ if (action-flags IRQF_NO_SUSPEND)
+ break;
+ }
+ if (action) {
+ for (action = desc-action; action; action =
action-next) {
+ if (!(action-flags IRQF_NO_SUSPEND)) {
+ action-saved_handler = action-handler;
+ action-handler = irq_pm_dummy_handler;
+ }
+ }
+ return;
+ }
+
desc-istate |= IRQS_SUSPENDED;
}
@@ -445,6 +470,14 @@ EXPORT_SYMBOL(disable_irq);
void __enable_irq(struct irq_desc *desc, unsigned int irq, bool resume)
{
