| Message ID | 20150304200040.GA12126@leverpostej (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Delegated to: | Rafael Wysocki |
| Headers | show |
On Wednesday, March 04, 2015 08:00:40 PM Mark Rutland wrote: > With certain restrictions it is possible for a wakeup device to share > and IRQ with an IRQF_NO_SUSPEND user, and the warnings introduced by > commit cab303be91dc47942bc25de33dc1140123540800 are spurious. The new > IRQF_COND_SUSPEND flag allows drivers to tell the core when these > restrictions are met, allowing spurious warnings to be silenced. > > This patch documents how IRQF_COND_SUSPEND is expected to be used, > updating some of the text now made invalid by its addition. > > Signed-off-by: Mark Rutland <mark.rutland@arm.com> > Cc: Boris Brezillon <boris.brezillon@free-electrons.com> > Cc: Peter Zijlstra <peterz@infradead.org> > Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > --- > Documentation/power/suspend-and-interrupts.txt | 16 +++++++++++++--- > 1 file changed, 13 insertions(+), 3 deletions(-) > > As promised previously, take IRQF_COND_SUSPEND into account in the > documentation. > > Rafael, does this look OK to you? Yes, it does, thanks! I'll queue it up along with the rest of the IRQF_COND_SUSPEND patches. > diff --git a/Documentation/power/suspend-and-interrupts.txt b/Documentation/power/suspend-and-interrupts.txt > index 50493c9..8afb29a 100644 > --- a/Documentation/power/suspend-and-interrupts.txt > +++ b/Documentation/power/suspend-and-interrupts.txt > @@ -112,8 +112,9 @@ 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. > +There are very few valid reasons to use both enable_irq_wake() and the > +IRQF_NO_SUSPEND flag on the same IRQ, and it is never valid to use both for the > +same device. > > 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 > @@ -122,4 +123,13 @@ 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. > +interrupt source and an IRQF_NO_SUSPEND interrupt source does not generally > +make sense. > + > +In rare cases an IRQ can be shared between a wakeup device driver and an > +IRQF_NO_SUSPEND user. In order for this to be safe, the wakeup device driver > +must be able to discern spurious IRQs from genuine wakeup events (signalling > +the latter to the core with pm_system_wakeup()), must use enable_irq_wake() to > +ensure that the IRQ will function as a wakeup source, and must request the IRQ > +with IRQF_COND_SUSPEND to tell the core that it meets these requirements. If > +these requirements are not met, it is not valid to use IRQF_COND_SUSPEND. >
tiny tiny nitpick: On 04/03/2015 at 20:00:40 +0000, Mark Rutland wrote : > With certain restrictions it is possible for a wakeup device to share > and IRQ with an IRQF_NO_SUSPEND user, and the warnings introduced by ^ an > +In rare cases an IRQ can be shared between a wakeup device driver and an > +IRQF_NO_SUSPEND user. In order for this to be safe, the wakeup device driver > +must be able to discern spurious IRQs from genuine wakeup events (signalling And genuine question, should we use British English or American English or we don't care ?
On Wednesday 04 March 2015 23:17:29 Alexandre Belloni wrote: > On 04/03/2015 at 20:00:40 +0000, Mark Rutland wrote : > > With certain restrictions it is possible for a wakeup device to share > > and IRQ with an IRQF_NO_SUSPEND user, and the warnings introduced by > ^ an > > > > +In rare cases an IRQ can be shared between a wakeup device driver and an > > +IRQF_NO_SUSPEND user. In order for this to be safe, the wakeup device driver > > +must be able to discern spurious IRQs from genuine wakeup events (signalling > > And genuine question, should we use British English or American English > or we don't care ? I believe most developers use Incorrect English, the second most common is American English, followed by Indian English and British English. ;-) More seriously, just try to be consistent within one file or subsystem. Arnd -- 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
On Wed, Mar 04, 2015 at 10:17:29PM +0000, Alexandre Belloni wrote: > tiny tiny nitpick: > > On 04/03/2015 at 20:00:40 +0000, Mark Rutland wrote : > > With certain restrictions it is possible for a wakeup device to share > > and IRQ with an IRQF_NO_SUSPEND user, and the warnings introduced by > ^ an Whoops. Rafael, are you happy to fix that up, or would you like me to resend? > > +In rare cases an IRQ can be shared between a wakeup device driver and an > > +IRQF_NO_SUSPEND user. In order for this to be safe, the wakeup device driver > > +must be able to discern spurious IRQs from genuine wakeup events (signalling > > And genuine question, should we use British English or American English > or we don't care ? Have I written something that isn't valid American English there? I read over this a few times and failed to spot anything obvious. I'm happy to change for consistency, I generally assume that's the most important thing. Mark. -- 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
On 05/03/2015 at 11:04:11 +0000, Mark Rutland wrote : > > > +In rare cases an IRQ can be shared between a wakeup device driver and an > > > +IRQF_NO_SUSPEND user. In order for this to be safe, the wakeup device driver > > > +must be able to discern spurious IRQs from genuine wakeup events (signalling > > > > And genuine question, should we use British English or American English > > or we don't care ? > > Have I written something that isn't valid American English there? I read > over this a few times and failed to spot anything obvious. > > I'm happy to change for consistency, I generally assume that's the most > important thing. I'd say signalling vs signaling. I actually had to look up which one was correct. I'm personally using Incorrect/Broken English so I'm definitely not here to give lessons.
On Thu, Mar 05, 2015 at 11:33:06AM +0000, Alexandre Belloni wrote: > On 05/03/2015 at 11:04:11 +0000, Mark Rutland wrote : > > > > +In rare cases an IRQ can be shared between a wakeup device driver and an > > > > +IRQF_NO_SUSPEND user. In order for this to be safe, the wakeup device driver > > > > +must be able to discern spurious IRQs from genuine wakeup events (signalling > > > > > > And genuine question, should we use British English or American English > > > or we don't care ? > > > > Have I written something that isn't valid American English there? I read > > over this a few times and failed to spot anything obvious. > > > > I'm happy to change for consistency, I generally assume that's the most > > important thing. > > I'd say signalling vs signaling. I actually had to look up which one was > correct. I'm personally using Incorrect/Broken English so I'm definitely > not here to give lessons. Easy option to keep everyone happy: s/signalling/indicating/ That should be valid for the English variants I'm aware of, and it has the same number of characters so we don't need to reflow the text. Mark. -- 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
On Thursday, March 05, 2015 11:04:11 AM Mark Rutland wrote: > On Wed, Mar 04, 2015 at 10:17:29PM +0000, Alexandre Belloni wrote: > > tiny tiny nitpick: > > > > On 04/03/2015 at 20:00:40 +0000, Mark Rutland wrote : > > > With certain restrictions it is possible for a wakeup device to share > > > and IRQ with an IRQF_NO_SUSPEND user, and the warnings introduced by > > ^ an > > Whoops. > > Rafael, are you happy to fix that up, or would you like me to resend? Fixed, thanks!
diff --git a/Documentation/power/suspend-and-interrupts.txt b/Documentation/power/suspend-and-interrupts.txt index 50493c9..8afb29a 100644 --- a/Documentation/power/suspend-and-interrupts.txt +++ b/Documentation/power/suspend-and-interrupts.txt @@ -112,8 +112,9 @@ 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. +There are very few valid reasons to use both enable_irq_wake() and the +IRQF_NO_SUSPEND flag on the same IRQ, and it is never valid to use both for the +same device. 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 @@ -122,4 +123,13 @@ 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. +interrupt source and an IRQF_NO_SUSPEND interrupt source does not generally +make sense. + +In rare cases an IRQ can be shared between a wakeup device driver and an +IRQF_NO_SUSPEND user. In order for this to be safe, the wakeup device driver +must be able to discern spurious IRQs from genuine wakeup events (signalling +the latter to the core with pm_system_wakeup()), must use enable_irq_wake() to +ensure that the IRQ will function as a wakeup source, and must request the IRQ +with IRQF_COND_SUSPEND to tell the core that it meets these requirements. If +these requirements are not met, it is not valid to use IRQF_COND_SUSPEND.
With certain restrictions it is possible for a wakeup device to share and IRQ with an IRQF_NO_SUSPEND user, and the warnings introduced by commit cab303be91dc47942bc25de33dc1140123540800 are spurious. The new IRQF_COND_SUSPEND flag allows drivers to tell the core when these restrictions are met, allowing spurious warnings to be silenced. This patch documents how IRQF_COND_SUSPEND is expected to be used, updating some of the text now made invalid by its addition. Signed-off-by: Mark Rutland <mark.rutland@arm.com> Cc: Boris Brezillon <boris.brezillon@free-electrons.com> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com> --- Documentation/power/suspend-and-interrupts.txt | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) As promised previously, take IRQF_COND_SUSPEND into account in the documentation. Rafael, does this look OK to you? Thanks, Mark.