Re: [PATCH v2 5/6] watchdog: at91sam9: request the irq with IRQF_NO_SUSPEND

2015-03-11 Thread Nicolas Ferre
Le 11/03/2015 09:38, Boris Brezillon a écrit : > On Sun, 08 Mar 2015 02:11:45 +0100 > "Rafael J. Wysocki" wrote: > >> On Saturday, March 07, 2015 12:06:45 PM Alexandre Belloni wrote: >>> On 07/03/2015 at 11:39:39 +0100, Pavel Machek wrote : > The Atmel watchdog can't be stopped once it's

Re: [PATCH v2 5/6] watchdog: at91sam9: request the irq with IRQF_NO_SUSPEND

2015-03-11 Thread Boris Brezillon
On Sun, 08 Mar 2015 02:11:45 +0100 "Rafael J. Wysocki" wrote: > On Saturday, March 07, 2015 12:06:45 PM Alexandre Belloni wrote: > > On 07/03/2015 at 11:39:39 +0100, Pavel Machek wrote : > > > > The Atmel watchdog can't be stopped once it's started. This is actually > > > > very useful so we

Re: [PATCH v2 5/6] watchdog: at91sam9: request the irq with IRQF_NO_SUSPEND

2015-03-11 Thread Boris Brezillon
Rafael, Alexandre, On Wed, 11 Mar 2015 02:03:08 +0100 "Rafael J. Wysocki" wrote: > On Tuesday, March 10, 2015 11:33:05 PM Alexandre Belloni wrote: > > On 10/03/2015 at 23:31:52 +0100, Rafael J. Wysocki wrote : > > > On Tuesday, March 10, 2015 10:33:17 PM Alexandre Belloni wrote: > > > > Hi, > >

Re: [PATCH v2 5/6] watchdog: at91sam9: request the irq with IRQF_NO_SUSPEND

2015-03-11 Thread Boris Brezillon
On Sun, 08 Mar 2015 02:11:45 +0100 Rafael J. Wysocki r...@rjwysocki.net wrote: On Saturday, March 07, 2015 12:06:45 PM Alexandre Belloni wrote: On 07/03/2015 at 11:39:39 +0100, Pavel Machek wrote : The Atmel watchdog can't be stopped once it's started. This is actually very useful so

Re: [PATCH v2 5/6] watchdog: at91sam9: request the irq with IRQF_NO_SUSPEND

2015-03-11 Thread Boris Brezillon
Rafael, Alexandre, On Wed, 11 Mar 2015 02:03:08 +0100 Rafael J. Wysocki r...@rjwysocki.net wrote: On Tuesday, March 10, 2015 11:33:05 PM Alexandre Belloni wrote: On 10/03/2015 at 23:31:52 +0100, Rafael J. Wysocki wrote : On Tuesday, March 10, 2015 10:33:17 PM Alexandre Belloni wrote:

Re: [PATCH v2 5/6] watchdog: at91sam9: request the irq with IRQF_NO_SUSPEND

2015-03-11 Thread Nicolas Ferre
Le 11/03/2015 09:38, Boris Brezillon a écrit : On Sun, 08 Mar 2015 02:11:45 +0100 Rafael J. Wysocki r...@rjwysocki.net wrote: On Saturday, March 07, 2015 12:06:45 PM Alexandre Belloni wrote: On 07/03/2015 at 11:39:39 +0100, Pavel Machek wrote : The Atmel watchdog can't be stopped once it's

Re: [PATCH v2 5/6] watchdog: at91sam9: request the irq with IRQF_NO_SUSPEND

2015-03-10 Thread Rafael J. Wysocki
On Tuesday, March 10, 2015 11:33:05 PM Alexandre Belloni wrote: > On 10/03/2015 at 23:31:52 +0100, Rafael J. Wysocki wrote : > > On Tuesday, March 10, 2015 10:33:17 PM Alexandre Belloni wrote: > > > Hi, > > > > > > On 09/03/2015 at 15:30:01 +0100, Rafael J. Wysocki wrote : > > > > > > > Actaully,

Re: [PATCH v2 5/6] watchdog: at91sam9: request the irq with IRQF_NO_SUSPEND

2015-03-10 Thread Alexandre Belloni
On 10/03/2015 at 23:31:52 +0100, Rafael J. Wysocki wrote : > On Tuesday, March 10, 2015 10:33:17 PM Alexandre Belloni wrote: > > Hi, > > > > On 09/03/2015 at 15:30:01 +0100, Rafael J. Wysocki wrote : > > > > > > Actaully, your platform should just refuse to enter suspend-to-RAM > > > > > > when

Re: [PATCH v2 5/6] watchdog: at91sam9: request the irq with IRQF_NO_SUSPEND

2015-03-10 Thread Rafael J. Wysocki
On Tuesday, March 10, 2015 10:33:17 PM Alexandre Belloni wrote: > Hi, > > On 09/03/2015 at 15:30:01 +0100, Rafael J. Wysocki wrote : > > > > > Actaully, your platform should just refuse to enter suspend-to-RAM > > > > > when hw watchdog is enabled. > > > > > > > > Quite likely, depending on how

Re: [PATCH v2 5/6] watchdog: at91sam9: request the irq with IRQF_NO_SUSPEND

2015-03-10 Thread Alexandre Belloni
Hi, On 09/03/2015 at 15:30:01 +0100, Rafael J. Wysocki wrote : > > > > Actaully, your platform should just refuse to enter suspend-to-RAM > > > > when hw watchdog is enabled. > > > > > > Quite likely, depending on how exactly the suspend is implemented. > > > > > > > We've had absolutely zero

Re: [PATCH v2 5/6] watchdog: at91sam9: request the irq with IRQF_NO_SUSPEND

2015-03-10 Thread Alexandre Belloni
Hi, On 09/03/2015 at 15:30:01 +0100, Rafael J. Wysocki wrote : Actaully, your platform should just refuse to enter suspend-to-RAM when hw watchdog is enabled. Quite likely, depending on how exactly the suspend is implemented. We've had absolutely zero complain on that. It

Re: [PATCH v2 5/6] watchdog: at91sam9: request the irq with IRQF_NO_SUSPEND

2015-03-10 Thread Rafael J. Wysocki
On Tuesday, March 10, 2015 10:33:17 PM Alexandre Belloni wrote: Hi, On 09/03/2015 at 15:30:01 +0100, Rafael J. Wysocki wrote : Actaully, your platform should just refuse to enter suspend-to-RAM when hw watchdog is enabled. Quite likely, depending on how exactly the suspend

Re: [PATCH v2 5/6] watchdog: at91sam9: request the irq with IRQF_NO_SUSPEND

2015-03-10 Thread Alexandre Belloni
On 10/03/2015 at 23:31:52 +0100, Rafael J. Wysocki wrote : On Tuesday, March 10, 2015 10:33:17 PM Alexandre Belloni wrote: Hi, On 09/03/2015 at 15:30:01 +0100, Rafael J. Wysocki wrote : Actaully, your platform should just refuse to enter suspend-to-RAM when hw watchdog is

Re: [PATCH v2 5/6] watchdog: at91sam9: request the irq with IRQF_NO_SUSPEND

2015-03-10 Thread Rafael J. Wysocki
On Tuesday, March 10, 2015 11:33:05 PM Alexandre Belloni wrote: On 10/03/2015 at 23:31:52 +0100, Rafael J. Wysocki wrote : On Tuesday, March 10, 2015 10:33:17 PM Alexandre Belloni wrote: Hi, On 09/03/2015 at 15:30:01 +0100, Rafael J. Wysocki wrote : Actaully, your platform

Re: [PATCH v2 5/6] watchdog: at91sam9: request the irq with IRQF_NO_SUSPEND

2015-03-09 Thread Rafael J. Wysocki
On Monday, March 09, 2015 08:55:46 AM Alexandre Belloni wrote: > Hi, > > On 08/03/2015 at 02:12:53 +0100, Rafael J. Wysocki wrote : > > > > I think you misunderstood, that is exactly the expected behaviour. This > > > > is hardware defined. Once the watchdog is started, nobody can stop it. > > >

Re: [PATCH v2 5/6] watchdog: at91sam9: request the irq with IRQF_NO_SUSPEND

2015-03-09 Thread Alexandre Belloni
Hi, On 08/03/2015 at 02:12:53 +0100, Rafael J. Wysocki wrote : > > > I think you misunderstood, that is exactly the expected behaviour. This > > > is hardware defined. Once the watchdog is started, nobody can stop it. > > > Trying to change the mode register will result in a reset of the > > >

Re: [PATCH v2 5/6] watchdog: at91sam9: request the irq with IRQF_NO_SUSPEND

2015-03-09 Thread Rafael J. Wysocki
On Monday, March 09, 2015 08:55:46 AM Alexandre Belloni wrote: Hi, On 08/03/2015 at 02:12:53 +0100, Rafael J. Wysocki wrote : I think you misunderstood, that is exactly the expected behaviour. This is hardware defined. Once the watchdog is started, nobody can stop it. Trying to

Re: [PATCH v2 5/6] watchdog: at91sam9: request the irq with IRQF_NO_SUSPEND

2015-03-09 Thread Alexandre Belloni
Hi, On 08/03/2015 at 02:12:53 +0100, Rafael J. Wysocki wrote : I think you misunderstood, that is exactly the expected behaviour. This is hardware defined. Once the watchdog is started, nobody can stop it. Trying to change the mode register will result in a reset of the SoC.

Re: [PATCH v2 5/6] watchdog: at91sam9: request the irq with IRQF_NO_SUSPEND

2015-03-07 Thread Rafael J. Wysocki
On Saturday, March 07, 2015 12:29:33 PM Pavel Machek wrote: > On Sat 2015-03-07 12:06:45, Alexandre Belloni wrote: > > On 07/03/2015 at 11:39:39 +0100, Pavel Machek wrote : > > > > The Atmel watchdog can't be stopped once it's started. This is actually > > > > very useful so we can reset if

Re: [PATCH v2 5/6] watchdog: at91sam9: request the irq with IRQF_NO_SUSPEND

2015-03-07 Thread Rafael J. Wysocki
On Saturday, March 07, 2015 12:06:45 PM Alexandre Belloni wrote: > On 07/03/2015 at 11:39:39 +0100, Pavel Machek wrote : > > > The Atmel watchdog can't be stopped once it's started. This is actually > > > very useful so we can reset if suspend or resume failed, the only > > > drawback is that

Re: [PATCH v2 5/6] watchdog: at91sam9: request the irq with IRQF_NO_SUSPEND

2015-03-07 Thread Sylvain Rochet
Hello, On Sat, Mar 07, 2015 at 12:29:33PM +0100, Pavel Machek wrote: > On Sat 2015-03-07 12:06:45, Alexandre Belloni wrote: > > On 07/03/2015 at 11:39:39 +0100, Pavel Machek wrote : > > > > The Atmel watchdog can't be stopped once it's started. This is actually > > > > very useful so we can

Re: [PATCH v2 5/6] watchdog: at91sam9: request the irq with IRQF_NO_SUSPEND

2015-03-07 Thread Pavel Machek
On Sat 2015-03-07 12:06:45, Alexandre Belloni wrote: > On 07/03/2015 at 11:39:39 +0100, Pavel Machek wrote : > > > The Atmel watchdog can't be stopped once it's started. This is actually > > > very useful so we can reset if suspend or resume failed, the only > > > drawback is that you have to

Re: [PATCH v2 5/6] watchdog: at91sam9: request the irq with IRQF_NO_SUSPEND

2015-03-07 Thread Alexandre Belloni
On 07/03/2015 at 11:39:39 +0100, Pavel Machek wrote : > > The Atmel watchdog can't be stopped once it's started. This is actually > > very useful so we can reset if suspend or resume failed, the only > > drawback is that you have to wake up from time to time (e.g. by using > > the RTC/RTT) to

Re: [PATCH v2 5/6] watchdog: at91sam9: request the irq with IRQF_NO_SUSPEND

2015-03-07 Thread Sylvain Rochet
Hello, On Sat, Mar 07, 2015 at 11:39:39AM +0100, Pavel Machek wrote: > On Sat 2015-03-07 11:20:56, Sylvain Rochet wrote: > > Hello, > > > > On Sat, Mar 07, 2015 at 10:18:46AM +0100, Peter Zijlstra wrote: > > > On Thu, Mar 05, 2015 at 11:53:08AM +, Mark Rutland wrote: > > > > If everyone

Re: [PATCH v2 5/6] watchdog: at91sam9: request the irq with IRQF_NO_SUSPEND

2015-03-07 Thread Pavel Machek
On Sat 2015-03-07 11:20:56, Sylvain Rochet wrote: > Hello, > > On Sat, Mar 07, 2015 at 10:18:46AM +0100, Peter Zijlstra wrote: > > On Thu, Mar 05, 2015 at 11:53:08AM +, Mark Rutland wrote: > > > If everyone else is happy with this using IRQF_NO_SUSPEND for now then > > > don't let my comments

Re: [PATCH v2 5/6] watchdog: at91sam9: request the irq with IRQF_NO_SUSPEND

2015-03-07 Thread Sylvain Rochet
Hello, On Sat, Mar 07, 2015 at 10:18:46AM +0100, Peter Zijlstra wrote: > On Thu, Mar 05, 2015 at 11:53:08AM +, Mark Rutland wrote: > > If everyone else is happy with this using IRQF_NO_SUSPEND for now then > > don't let my comments above block this patch. > > Yeah, I'm really not happy with

Re: [PATCH v2 5/6] watchdog: at91sam9: request the irq with IRQF_NO_SUSPEND

2015-03-07 Thread Peter Zijlstra
On Thu, Mar 05, 2015 at 11:53:08AM +, Mark Rutland wrote: > If everyone else is happy with this using IRQF_NO_SUSPEND for now then > don't let my comments above block this patch. Yeah, I'm really not happy with NO_SUSPEND + enable_irq_wake(). I really want that combo to BUG/WARN -- esp.

Re: [PATCH v2 5/6] watchdog: at91sam9: request the irq with IRQF_NO_SUSPEND

2015-03-07 Thread Peter Zijlstra
On Fri, Mar 06, 2015 at 11:06:18AM +, Mark Rutland wrote: > [...] > > > > The request_irq path never results in a call to chip->irq_set_wake(), > > > even with the IRQF_NO_SUSPEND flag. So requesting an irq with > > > IRQF_NO_SUSPEND does not guarantee wakeup; it only guarantees that the > >

Re: [PATCH v2 5/6] watchdog: at91sam9: request the irq with IRQF_NO_SUSPEND

2015-03-07 Thread Peter Zijlstra
On Thu, Mar 05, 2015 at 04:10:16PM +0100, Rafael J. Wysocki wrote: > enable_irq_wake() has no effect on IRQF_NO_SUSPEND interrupts, so if the > driver uses IRQF_NO_SUSPEND, it does not need to use enable_irq_wake() > in addition to that. I still feel we should BUG when someone is calling

Re: [PATCH v2 5/6] watchdog: at91sam9: request the irq with IRQF_NO_SUSPEND

2015-03-07 Thread Pavel Machek
On Sat 2015-03-07 11:20:56, Sylvain Rochet wrote: Hello, On Sat, Mar 07, 2015 at 10:18:46AM +0100, Peter Zijlstra wrote: On Thu, Mar 05, 2015 at 11:53:08AM +, Mark Rutland wrote: If everyone else is happy with this using IRQF_NO_SUSPEND for now then don't let my comments above

Re: [PATCH v2 5/6] watchdog: at91sam9: request the irq with IRQF_NO_SUSPEND

2015-03-07 Thread Sylvain Rochet
Hello, On Sat, Mar 07, 2015 at 12:29:33PM +0100, Pavel Machek wrote: On Sat 2015-03-07 12:06:45, Alexandre Belloni wrote: On 07/03/2015 at 11:39:39 +0100, Pavel Machek wrote : The Atmel watchdog can't be stopped once it's started. This is actually very useful so we can reset if

Re: [PATCH v2 5/6] watchdog: at91sam9: request the irq with IRQF_NO_SUSPEND

2015-03-07 Thread Sylvain Rochet
Hello, On Sat, Mar 07, 2015 at 10:18:46AM +0100, Peter Zijlstra wrote: On Thu, Mar 05, 2015 at 11:53:08AM +, Mark Rutland wrote: If everyone else is happy with this using IRQF_NO_SUSPEND for now then don't let my comments above block this patch. Yeah, I'm really not happy with

Re: [PATCH v2 5/6] watchdog: at91sam9: request the irq with IRQF_NO_SUSPEND

2015-03-07 Thread Alexandre Belloni
On 07/03/2015 at 11:39:39 +0100, Pavel Machek wrote : The Atmel watchdog can't be stopped once it's started. This is actually very useful so we can reset if suspend or resume failed, the only drawback is that you have to wake up from time to time (e.g. by using the RTC/RTT) to clear the

Re: [PATCH v2 5/6] watchdog: at91sam9: request the irq with IRQF_NO_SUSPEND

2015-03-07 Thread Peter Zijlstra
On Thu, Mar 05, 2015 at 04:10:16PM +0100, Rafael J. Wysocki wrote: enable_irq_wake() has no effect on IRQF_NO_SUSPEND interrupts, so if the driver uses IRQF_NO_SUSPEND, it does not need to use enable_irq_wake() in addition to that. I still feel we should BUG when someone is calling

Re: [PATCH v2 5/6] watchdog: at91sam9: request the irq with IRQF_NO_SUSPEND

2015-03-07 Thread Pavel Machek
On Sat 2015-03-07 12:06:45, Alexandre Belloni wrote: On 07/03/2015 at 11:39:39 +0100, Pavel Machek wrote : The Atmel watchdog can't be stopped once it's started. This is actually very useful so we can reset if suspend or resume failed, the only drawback is that you have to wake up from

Re: [PATCH v2 5/6] watchdog: at91sam9: request the irq with IRQF_NO_SUSPEND

2015-03-07 Thread Peter Zijlstra
On Fri, Mar 06, 2015 at 11:06:18AM +, Mark Rutland wrote: [...] The request_irq path never results in a call to chip-irq_set_wake(), even with the IRQF_NO_SUSPEND flag. So requesting an irq with IRQF_NO_SUSPEND does not guarantee wakeup; it only guarantees that the CPU can take

Re: [PATCH v2 5/6] watchdog: at91sam9: request the irq with IRQF_NO_SUSPEND

2015-03-07 Thread Peter Zijlstra
On Thu, Mar 05, 2015 at 11:53:08AM +, Mark Rutland wrote: If everyone else is happy with this using IRQF_NO_SUSPEND for now then don't let my comments above block this patch. Yeah, I'm really not happy with NO_SUSPEND + enable_irq_wake(). I really want that combo to BUG/WARN -- esp. since

Re: [PATCH v2 5/6] watchdog: at91sam9: request the irq with IRQF_NO_SUSPEND

2015-03-07 Thread Sylvain Rochet
Hello, On Sat, Mar 07, 2015 at 11:39:39AM +0100, Pavel Machek wrote: On Sat 2015-03-07 11:20:56, Sylvain Rochet wrote: Hello, On Sat, Mar 07, 2015 at 10:18:46AM +0100, Peter Zijlstra wrote: On Thu, Mar 05, 2015 at 11:53:08AM +, Mark Rutland wrote: If everyone else is happy

Re: [PATCH v2 5/6] watchdog: at91sam9: request the irq with IRQF_NO_SUSPEND

2015-03-07 Thread Rafael J. Wysocki
On Saturday, March 07, 2015 12:06:45 PM Alexandre Belloni wrote: On 07/03/2015 at 11:39:39 +0100, Pavel Machek wrote : The Atmel watchdog can't be stopped once it's started. This is actually very useful so we can reset if suspend or resume failed, the only drawback is that you have to

Re: [PATCH v2 5/6] watchdog: at91sam9: request the irq with IRQF_NO_SUSPEND

2015-03-07 Thread Rafael J. Wysocki
On Saturday, March 07, 2015 12:29:33 PM Pavel Machek wrote: On Sat 2015-03-07 12:06:45, Alexandre Belloni wrote: On 07/03/2015 at 11:39:39 +0100, Pavel Machek wrote : The Atmel watchdog can't be stopped once it's started. This is actually very useful so we can reset if suspend or

Re: [PATCH v2 5/6] watchdog: at91sam9: request the irq with IRQF_NO_SUSPEND

2015-03-06 Thread Mark Rutland
> >> > We seem to be conflating some related properties: > >> > > >> > [a] The IRQ will be left unmasked. > >> > [b] The IRQ will be handled immediately when taken. > >> > [c] The IRQ will wake the system from suspend. [...] > > Considering that the use-case of a watchdog is to alert us to

Re: [PATCH v2 5/6] watchdog: at91sam9: request the irq with IRQF_NO_SUSPEND

2015-03-06 Thread Rafael J. Wysocki
On Fri, Mar 6, 2015 at 12:06 PM, Mark Rutland wrote: > [...] > >> > The request_irq path never results in a call to chip->irq_set_wake(), >> > even with the IRQF_NO_SUSPEND flag. So requesting an irq with >> > IRQF_NO_SUSPEND does not guarantee wakeup; it only guarantees that the >> > CPU can

Re: [PATCH v2 5/6] watchdog: at91sam9: request the irq with IRQF_NO_SUSPEND

2015-03-06 Thread Mark Rutland
[...] > > The request_irq path never results in a call to chip->irq_set_wake(), > > even with the IRQF_NO_SUSPEND flag. So requesting an irq with > > IRQF_NO_SUSPEND does not guarantee wakeup; it only guarantees that the > > CPU can take the interrupt _around_ the suspended state, not necessarily

Re: [PATCH v2 5/6] watchdog: at91sam9: request the irq with IRQF_NO_SUSPEND

2015-03-06 Thread Mark Rutland
We seem to be conflating some related properties: [a] The IRQ will be left unmasked. [b] The IRQ will be handled immediately when taken. [c] The IRQ will wake the system from suspend. [...] Considering that the use-case of a watchdog is to alert us to something going hideously

Re: [PATCH v2 5/6] watchdog: at91sam9: request the irq with IRQF_NO_SUSPEND

2015-03-06 Thread Rafael J. Wysocki
On Fri, Mar 6, 2015 at 12:06 PM, Mark Rutland mark.rutl...@arm.com wrote: [...] The request_irq path never results in a call to chip-irq_set_wake(), even with the IRQF_NO_SUSPEND flag. So requesting an irq with IRQF_NO_SUSPEND does not guarantee wakeup; it only guarantees that the CPU

Re: [PATCH v2 5/6] watchdog: at91sam9: request the irq with IRQF_NO_SUSPEND

2015-03-06 Thread Mark Rutland
[...] The request_irq path never results in a call to chip-irq_set_wake(), even with the IRQF_NO_SUSPEND flag. So requesting an irq with IRQF_NO_SUSPEND does not guarantee wakeup; it only guarantees that the CPU can take the interrupt _around_ the suspended state, not necessarily while

Re: [PATCH v2 5/6] watchdog: at91sam9: request the irq with IRQF_NO_SUSPEND

2015-03-05 Thread Rafael J. Wysocki
On Thursday, March 05, 2015 04:32:27 PM Mark Rutland wrote: > Hi Rafael, > > > enable_irq_wake() has no effect on IRQF_NO_SUSPEND interrupts, so if the > > driver uses IRQF_NO_SUSPEND, it does not need to use enable_irq_wake() > > in addition to that. > > That's not generally true -- certainly

Re: [PATCH v2 5/6] watchdog: at91sam9: request the irq with IRQF_NO_SUSPEND

2015-03-05 Thread Mark Rutland
Hi Rafael, > enable_irq_wake() has no effect on IRQF_NO_SUSPEND interrupts, so if the > driver uses IRQF_NO_SUSPEND, it does not need to use enable_irq_wake() > in addition to that. That's not generally true -- certainly not for irq_chips without the IRQCHIP_SKIP_SET_WAKE flag. Consider systems

Re: [PATCH v2 5/6] watchdog: at91sam9: request the irq with IRQF_NO_SUSPEND

2015-03-05 Thread Rafael J. Wysocki
On Thu, Mar 5, 2015 at 11:57 AM, Mark Rutland wrote: > [...] > >> > > err = request_irq(wdt->irq, wdt_interrupt, >> > > - IRQF_SHARED | IRQF_IRQPOLL, >> > > + IRQF_SHARED | IRQF_IRQPOLL | >> > > +

Re: [PATCH v2 5/6] watchdog: at91sam9: request the irq with IRQF_NO_SUSPEND

2015-03-05 Thread Mark Rutland
Hi Boris, TL;DR - I guess using IRQF_NO_SUSPEND for now is OK a a best-effort approach for now, so don't let my comments block this patch. However, there are still some potential issues in what would already be a failure case: your usual wakeup mechanism not waking the system up in time to poke

Re: [PATCH v2 5/6] watchdog: at91sam9: request the irq with IRQF_NO_SUSPEND

2015-03-05 Thread Boris Brezillon
On Thu, 5 Mar 2015 12:17:23 +0100 Boris Brezillon wrote: > Hi Boris, ^ Mark, I'm suffering from a dual personality disorder :-) > > On Thu, 5 Mar 2015 10:53:08 + > Mark Rutland wrote: > > > Hi Boris, > > > > I'd missed the fact that this was for SW watchdog as opposed to HW > >

Re: [PATCH v2 5/6] watchdog: at91sam9: request the irq with IRQF_NO_SUSPEND

2015-03-05 Thread Boris Brezillon
Hi Boris, On Thu, 5 Mar 2015 10:53:08 + Mark Rutland wrote: > Hi Boris, > > I'd missed the fact that this was for SW watchdog as opposed to HW > watchdog, which may explain my confusion. > > [...] > > > > > err = request_irq(wdt->irq, wdt_interrupt, > > > > -

Re: [PATCH v2 5/6] watchdog: at91sam9: request the irq with IRQF_NO_SUSPEND

2015-03-05 Thread Mark Rutland
[...] > > > err = request_irq(wdt->irq, wdt_interrupt, > > > - IRQF_SHARED | IRQF_IRQPOLL, > > > + IRQF_SHARED | IRQF_IRQPOLL | > > > + IRQF_NO_SUSPEND, > > > > I'm a little confused by this. What happens if

Re: [PATCH v2 5/6] watchdog: at91sam9: request the irq with IRQF_NO_SUSPEND

2015-03-05 Thread Mark Rutland
Hi Boris, I'd missed the fact that this was for SW watchdog as opposed to HW watchdog, which may explain my confusion. [...] > > > err = request_irq(wdt->irq, wdt_interrupt, > > > - IRQF_SHARED | IRQF_IRQPOLL, > > > + IRQF_SHARED |

Re: [PATCH v2 5/6] watchdog: at91sam9: request the irq with IRQF_NO_SUSPEND

2015-03-05 Thread Boris Brezillon
Hi Mark, On Wed, 4 Mar 2015 18:38:09 + Mark Rutland wrote: > Hi Boris, > > On Mon, Mar 02, 2015 at 09:18:17AM +, Boris Brezillon wrote: > > The watchdog interrupt (only used when activating software watchdog) > > shouldn't be suspended when entering suspend mode, because it is shared >

Re: [PATCH v2 5/6] watchdog: at91sam9: request the irq with IRQF_NO_SUSPEND

2015-03-05 Thread Rafael J. Wysocki
On Thursday, March 05, 2015 04:32:27 PM Mark Rutland wrote: Hi Rafael, enable_irq_wake() has no effect on IRQF_NO_SUSPEND interrupts, so if the driver uses IRQF_NO_SUSPEND, it does not need to use enable_irq_wake() in addition to that. That's not generally true -- certainly not for

Re: [PATCH v2 5/6] watchdog: at91sam9: request the irq with IRQF_NO_SUSPEND

2015-03-05 Thread Mark Rutland
Hi Rafael, enable_irq_wake() has no effect on IRQF_NO_SUSPEND interrupts, so if the driver uses IRQF_NO_SUSPEND, it does not need to use enable_irq_wake() in addition to that. That's not generally true -- certainly not for irq_chips without the IRQCHIP_SKIP_SET_WAKE flag. Consider systems

Re: [PATCH v2 5/6] watchdog: at91sam9: request the irq with IRQF_NO_SUSPEND

2015-03-05 Thread Boris Brezillon
Hi Mark, On Wed, 4 Mar 2015 18:38:09 + Mark Rutland mark.rutl...@arm.com wrote: Hi Boris, On Mon, Mar 02, 2015 at 09:18:17AM +, Boris Brezillon wrote: The watchdog interrupt (only used when activating software watchdog) shouldn't be suspended when entering suspend mode, because

Re: [PATCH v2 5/6] watchdog: at91sam9: request the irq with IRQF_NO_SUSPEND

2015-03-05 Thread Mark Rutland
Hi Boris, I'd missed the fact that this was for SW watchdog as opposed to HW watchdog, which may explain my confusion. [...] err = request_irq(wdt-irq, wdt_interrupt, - IRQF_SHARED | IRQF_IRQPOLL, + IRQF_SHARED |

Re: [PATCH v2 5/6] watchdog: at91sam9: request the irq with IRQF_NO_SUSPEND

2015-03-05 Thread Mark Rutland
[...] err = request_irq(wdt-irq, wdt_interrupt, - IRQF_SHARED | IRQF_IRQPOLL, + IRQF_SHARED | IRQF_IRQPOLL | + IRQF_NO_SUSPEND, I'm a little confused by this. What happens if the watchdog

Re: [PATCH v2 5/6] watchdog: at91sam9: request the irq with IRQF_NO_SUSPEND

2015-03-05 Thread Boris Brezillon
Hi Boris, On Thu, 5 Mar 2015 10:53:08 + Mark Rutland mark.rutl...@arm.com wrote: Hi Boris, I'd missed the fact that this was for SW watchdog as opposed to HW watchdog, which may explain my confusion. [...] err = request_irq(wdt-irq, wdt_interrupt, -

Re: [PATCH v2 5/6] watchdog: at91sam9: request the irq with IRQF_NO_SUSPEND

2015-03-05 Thread Boris Brezillon
On Thu, 5 Mar 2015 12:17:23 +0100 Boris Brezillon boris.brezil...@free-electrons.com wrote: Hi Boris, ^ Mark, I'm suffering from a dual personality disorder :-) On Thu, 5 Mar 2015 10:53:08 + Mark Rutland mark.rutl...@arm.com wrote: Hi Boris, I'd missed the fact that this

Re: [PATCH v2 5/6] watchdog: at91sam9: request the irq with IRQF_NO_SUSPEND

2015-03-05 Thread Mark Rutland
Hi Boris, TL;DR - I guess using IRQF_NO_SUSPEND for now is OK a a best-effort approach for now, so don't let my comments block this patch. However, there are still some potential issues in what would already be a failure case: your usual wakeup mechanism not waking the system up in time to poke

Re: [PATCH v2 5/6] watchdog: at91sam9: request the irq with IRQF_NO_SUSPEND

2015-03-05 Thread Rafael J. Wysocki
On Thu, Mar 5, 2015 at 11:57 AM, Mark Rutland mark.rutl...@arm.com wrote: [...] err = request_irq(wdt-irq, wdt_interrupt, - IRQF_SHARED | IRQF_IRQPOLL, + IRQF_SHARED | IRQF_IRQPOLL | +

Re: [PATCH v2 5/6] watchdog: at91sam9: request the irq with IRQF_NO_SUSPEND

2015-03-04 Thread Rafael J. Wysocki
On Wednesday, March 04, 2015 06:38:09 PM Mark Rutland wrote: > Hi Boris, > > On Mon, Mar 02, 2015 at 09:18:17AM +, Boris Brezillon wrote: > > The watchdog interrupt (only used when activating software watchdog) > > shouldn't be suspended when entering suspend mode, because it is shared > >

Re: [PATCH v2 5/6] watchdog: at91sam9: request the irq with IRQF_NO_SUSPEND

2015-03-04 Thread Mark Rutland
Hi Boris, On Mon, Mar 02, 2015 at 09:18:17AM +, Boris Brezillon wrote: > The watchdog interrupt (only used when activating software watchdog) > shouldn't be suspended when entering suspend mode, because it is shared > with a timer device (which request the line with IRQF_NO_SUSPEND) and once

Re: [PATCH v2 5/6] watchdog: at91sam9: request the irq with IRQF_NO_SUSPEND

2015-03-04 Thread Mark Rutland
Hi Boris, On Mon, Mar 02, 2015 at 09:18:17AM +, Boris Brezillon wrote: The watchdog interrupt (only used when activating software watchdog) shouldn't be suspended when entering suspend mode, because it is shared with a timer device (which request the line with IRQF_NO_SUSPEND) and once

Re: [PATCH v2 5/6] watchdog: at91sam9: request the irq with IRQF_NO_SUSPEND

2015-03-04 Thread Rafael J. Wysocki
On Wednesday, March 04, 2015 06:38:09 PM Mark Rutland wrote: Hi Boris, On Mon, Mar 02, 2015 at 09:18:17AM +, Boris Brezillon wrote: The watchdog interrupt (only used when activating software watchdog) shouldn't be suspended when entering suspend mode, because it is shared with a

Re: [PATCH v2 5/6] watchdog: at91sam9: request the irq with IRQF_NO_SUSPEND

2015-03-02 Thread Guenter Roeck
On 03/02/2015 01:18 AM, Boris Brezillon wrote: The watchdog interrupt (only used when activating software watchdog) shouldn't be suspended when entering suspend mode, because it is shared with a timer device (which request the line with IRQF_NO_SUSPEND) and once the watchdog "Mode Register" has

[PATCH v2 5/6] watchdog: at91sam9: request the irq with IRQF_NO_SUSPEND

2015-03-02 Thread Boris Brezillon
The watchdog interrupt (only used when activating software watchdog) shouldn't be suspended when entering suspend mode, because it is shared with a timer device (which request the line with IRQF_NO_SUSPEND) and once the watchdog "Mode Register" has been written, it cannot be changed (which means

Re: [PATCH v2 5/6] watchdog: at91sam9: request the irq with IRQF_NO_SUSPEND

2015-03-02 Thread Guenter Roeck
On 03/02/2015 01:18 AM, Boris Brezillon wrote: The watchdog interrupt (only used when activating software watchdog) shouldn't be suspended when entering suspend mode, because it is shared with a timer device (which request the line with IRQF_NO_SUSPEND) and once the watchdog Mode Register has

[PATCH v2 5/6] watchdog: at91sam9: request the irq with IRQF_NO_SUSPEND

2015-03-02 Thread Boris Brezillon
The watchdog interrupt (only used when activating software watchdog) shouldn't be suspended when entering suspend mode, because it is shared with a timer device (which request the line with IRQF_NO_SUSPEND) and once the watchdog Mode Register has been written, it cannot be changed (which means we