* [PATCH v2] genirq: Clarify that irq wake state is orthogonal to enable/disable
@ 2020-02-06 19:15 Stephen Boyd
2020-02-06 19:32 ` Doug Anderson
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Stephen Boyd @ 2020-02-06 19:15 UTC (permalink / raw)
To: Thomas Gleixner
Cc: linux-kernel, Marc Zyngier, Douglas Anderson, Lina Iyer, Maulik Shah
There's some confusion around if an irq that's disabled with
disable_irq() can still wake the system from sleep states such as
"suspend to RAM". Let's clarify this in the kernel documentation for
irq_set_irq_wake() so that it's clear that an irq can be disabled and
still wake the system if it has been marked for wakeup.
Cc: Marc Zyngier <maz@kernel.org>
Cc: Douglas Anderson <dianders@chromium.org>
Cc: Lina Iyer <ilina@codeaurora.org>
Cc: Maulik Shah <mkshah@codeaurora.org>
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---
Changes from v1:
* Added the last sentence from tglx
kernel/irq/manage.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
index 818b2802d3e7..e1e217d7778c 100644
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -731,6 +731,13 @@ static int set_irq_wake_real(unsigned int irq, unsigned int on)
*
* Wakeup mode lets this IRQ wake the system from sleep
* states like "suspend to RAM".
+ *
+ * Note: irq enable/disable state is completely orthogonal
+ * to the enable/disable state of irq wake. An irq can be
+ * disabled with disable_irq() and still wake the system as
+ * long as the irq has wake enabled. If this does not hold,
+ * then either the underlying irq chip and the related driver
+ * need to be investigated.
*/
int irq_set_irq_wake(unsigned int irq, unsigned int on)
{
--
Sent by a computer, using git, on the internet
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2] genirq: Clarify that irq wake state is orthogonal to enable/disable
2020-02-06 19:15 [PATCH v2] genirq: Clarify that irq wake state is orthogonal to enable/disable Stephen Boyd
@ 2020-02-06 19:32 ` Doug Anderson
2020-02-06 19:57 ` Lina Iyer
2020-02-07 20:40 ` [tip: irq/urgent] " tip-bot2 for Stephen Boyd
2 siblings, 0 replies; 6+ messages in thread
From: Doug Anderson @ 2020-02-06 19:32 UTC (permalink / raw)
To: Stephen Boyd; +Cc: Thomas Gleixner, LKML, Marc Zyngier, Lina Iyer, Maulik Shah
Hi,
On Thu, Feb 6, 2020 at 11:15 AM Stephen Boyd <swboyd@chromium.org> wrote:
>
> There's some confusion around if an irq that's disabled with
> disable_irq() can still wake the system from sleep states such as
> "suspend to RAM". Let's clarify this in the kernel documentation for
> irq_set_irq_wake() so that it's clear that an irq can be disabled and
> still wake the system if it has been marked for wakeup.
>
> Cc: Marc Zyngier <maz@kernel.org>
> Cc: Douglas Anderson <dianders@chromium.org>
> Cc: Lina Iyer <ilina@codeaurora.org>
> Cc: Maulik Shah <mkshah@codeaurora.org>
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> ---
>
> Changes from v1:
> * Added the last sentence from tglx
>
> kernel/irq/manage.c | 7 +++++++
> 1 file changed, 7 insertions(+)
FWIW this mathes my understanding and in the past I've done work to
'drivers/pinctrl/pinctrl-rockchip.c' making it support this exact
thing. Thanks for documenting.
Reviewed-by: Douglas Anderson <dianders@chromium.org>
-Doug
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] genirq: Clarify that irq wake state is orthogonal to enable/disable
2020-02-06 19:15 [PATCH v2] genirq: Clarify that irq wake state is orthogonal to enable/disable Stephen Boyd
2020-02-06 19:32 ` Doug Anderson
@ 2020-02-06 19:57 ` Lina Iyer
2020-02-07 18:59 ` Doug Anderson
2020-02-07 20:40 ` [tip: irq/urgent] " tip-bot2 for Stephen Boyd
2 siblings, 1 reply; 6+ messages in thread
From: Lina Iyer @ 2020-02-06 19:57 UTC (permalink / raw)
To: Stephen Boyd
Cc: Thomas Gleixner, linux-kernel, Marc Zyngier, Douglas Anderson,
Maulik Shah
On Thu, Feb 06 2020 at 12:15 -0700, Stephen Boyd wrote:
>There's some confusion around if an irq that's disabled with
>disable_irq() can still wake the system from sleep states such as
>"suspend to RAM". Let's clarify this in the kernel documentation for
>irq_set_irq_wake() so that it's clear that an irq can be disabled and
>still wake the system if it has been marked for wakeup.
>
Thomas also mentioned that hardware could work either way and probably
should not be assumed to work one way or the other.
>Cc: Marc Zyngier <maz@kernel.org>
>Cc: Douglas Anderson <dianders@chromium.org>
>Cc: Lina Iyer <ilina@codeaurora.org>
>Cc: Maulik Shah <mkshah@codeaurora.org>
>Signed-off-by: Stephen Boyd <swboyd@chromium.org>
>---
>
>Changes from v1:
> * Added the last sentence from tglx
>
> kernel/irq/manage.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
>diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
>index 818b2802d3e7..e1e217d7778c 100644
>--- a/kernel/irq/manage.c
>+++ b/kernel/irq/manage.c
>@@ -731,6 +731,13 @@ static int set_irq_wake_real(unsigned int irq, unsigned int on)
> *
> * Wakeup mode lets this IRQ wake the system from sleep
> * states like "suspend to RAM".
>+ *
>+ * Note: irq enable/disable state is completely orthogonal
>+ * to the enable/disable state of irq wake. An irq can be
>+ * disabled with disable_irq() and still wake the system as
>+ * long as the irq has wake enabled. If this does not hold,
>+ * then either the underlying irq chip and the related driver
>+ * need to be investigated.
> */
> int irq_set_irq_wake(unsigned int irq, unsigned int on)
> {
>--
>Sent by a computer, using git, on the internet
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] genirq: Clarify that irq wake state is orthogonal to enable/disable
2020-02-06 19:57 ` Lina Iyer
@ 2020-02-07 18:59 ` Doug Anderson
2020-02-07 20:33 ` Thomas Gleixner
0 siblings, 1 reply; 6+ messages in thread
From: Doug Anderson @ 2020-02-07 18:59 UTC (permalink / raw)
To: Lina Iyer; +Cc: Stephen Boyd, Thomas Gleixner, LKML, Marc Zyngier, Maulik Shah
Hi,
On Thu, Feb 6, 2020 at 11:57 AM Lina Iyer <ilina@codeaurora.org> wrote:
>
> On Thu, Feb 06 2020 at 12:15 -0700, Stephen Boyd wrote:
> >There's some confusion around if an irq that's disabled with
> >disable_irq() can still wake the system from sleep states such as
> >"suspend to RAM". Let's clarify this in the kernel documentation for
> >irq_set_irq_wake() so that it's clear that an irq can be disabled and
> >still wake the system if it has been marked for wakeup.
> >
> Thomas also mentioned that hardware could work either way and probably
> should not be assumed to work one way or the other.
Right...
...and then (paraphrasing) Stephen pointed out that policy makes it
really hard for clients of the API to work properly.
...and then (paraphrasing) Thomas said "Good point. As long as you
document that not all drivers _actually_ behave the way you describe,
it's fine to add a comment saying that drivers _should_ behave the way
you describe".
Or, said another way: if a driver doesn't behave the way Stephen
describes then it should be fixed unless there is some reason why
there is no possible way to make it happen.
(Thomas / Stephen: please correct if I got my paraphrasing incorrect).
-Doug
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] genirq: Clarify that irq wake state is orthogonal to enable/disable
2020-02-07 18:59 ` Doug Anderson
@ 2020-02-07 20:33 ` Thomas Gleixner
0 siblings, 0 replies; 6+ messages in thread
From: Thomas Gleixner @ 2020-02-07 20:33 UTC (permalink / raw)
To: Doug Anderson, Lina Iyer; +Cc: Stephen Boyd, LKML, Marc Zyngier, Maulik Shah
Doug Anderson <dianders@chromium.org> writes:
>> Thomas also mentioned that hardware could work either way and probably
>> should not be assumed to work one way or the other.
>
> Right...
>
> ...and then (paraphrasing) Stephen pointed out that policy makes it
> really hard for clients of the API to work properly.
>
> ...and then (paraphrasing) Thomas said "Good point. As long as you
> document that not all drivers _actually_ behave the way you describe,
> it's fine to add a comment saying that drivers _should_ behave the way
> you describe".
>
> Or, said another way: if a driver doesn't behave the way Stephen
> describes then it should be fixed unless there is some reason why
> there is no possible way to make it happen.
Yes, that's right.
Thanks,
tglx
^ permalink raw reply [flat|nested] 6+ messages in thread
* [tip: irq/urgent] genirq: Clarify that irq wake state is orthogonal to enable/disable
2020-02-06 19:15 [PATCH v2] genirq: Clarify that irq wake state is orthogonal to enable/disable Stephen Boyd
2020-02-06 19:32 ` Doug Anderson
2020-02-06 19:57 ` Lina Iyer
@ 2020-02-07 20:40 ` tip-bot2 for Stephen Boyd
2 siblings, 0 replies; 6+ messages in thread
From: tip-bot2 for Stephen Boyd @ 2020-02-07 20:40 UTC (permalink / raw)
To: linux-tip-commits
Cc: Stephen Boyd, Thomas Gleixner, Douglas Anderson, x86, LKML
The following commit has been merged into the irq/urgent branch of tip:
Commit-ID: f9f21cea311340f38074ff93a8d89b4a9cae6bcc
Gitweb: https://git.kernel.org/tip/f9f21cea311340f38074ff93a8d89b4a9cae6bcc
Author: Stephen Boyd <swboyd@chromium.org>
AuthorDate: Thu, 06 Feb 2020 11:15:21 -08:00
Committer: Thomas Gleixner <tglx@linutronix.de>
CommitterDate: Fri, 07 Feb 2020 21:37:08 +01:00
genirq: Clarify that irq wake state is orthogonal to enable/disable
There's some confusion around if an irq that's disabled with disable_irq()
can still wake the system from sleep states such as "suspend to RAM".
Clarify this in the kernel documentation for irq_set_irq_wake() so that
it's clear that an irq can be disabled and still wake the system if it has
been marked for wakeup.
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Douglas Anderson <dianders@chromium.org>
Link: https://lkml.kernel.org/r/20200206191521.94559-1-swboyd@chromium.org
---
kernel/irq/manage.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
index 818b280..3089a60 100644
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -731,6 +731,13 @@ static int set_irq_wake_real(unsigned int irq, unsigned int on)
*
* Wakeup mode lets this IRQ wake the system from sleep
* states like "suspend to RAM".
+ *
+ * Note: irq enable/disable state is completely orthogonal
+ * to the enable/disable state of irq wake. An irq can be
+ * disabled with disable_irq() and still wake the system as
+ * long as the irq has wake enabled. If this does not hold,
+ * then the underlying irq chip and the related driver need
+ * to be investigated.
*/
int irq_set_irq_wake(unsigned int irq, unsigned int on)
{
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-02-07 20:40 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-06 19:15 [PATCH v2] genirq: Clarify that irq wake state is orthogonal to enable/disable Stephen Boyd
2020-02-06 19:32 ` Doug Anderson
2020-02-06 19:57 ` Lina Iyer
2020-02-07 18:59 ` Doug Anderson
2020-02-07 20:33 ` Thomas Gleixner
2020-02-07 20:40 ` [tip: irq/urgent] " tip-bot2 for Stephen Boyd
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).