Hi Boris, On Thu, 5 Mar 2015 10:53:08 +0000 Mark Rutland <mark.rutland@xxxxxxx> 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, > > > > - IRQF_SHARED | IRQF_IRQPOLL, > > > > + IRQF_SHARED | IRQF_IRQPOLL | > > > > + IRQF_NO_SUSPEND, > > > > > > I'm a little confused by this. What happens if the watchdog fires when > > > we're actually in the suspended state (when IRQF_NO_SUSPEND interrupts > > > aren't guaranteed to be delivered). > > > > It reboot the system. > > Is the timer we use to ping the watchdog guaranted to result in a wakeup > before an interrupt will be triggered? If so, then I think we're ok. It should be (I don't recall exactly what the logic is, but it's at least half the watchdog time limit). > > If not, then don't we need to clear a potentially pending watchdog irq > at resume time so at to not immediately reboot the machine? I couldn't > see any logic to that effect in the driver. That depends on what we want. If we want the watchdog to be inactive when entering suspend, then we shouldn't reboot the machine when receiving a watchdog irq while the system is suspended. ITOH, with the hardware mode (reset handled by the watchdog IP) you can't disable the watchdog when entering suspend, so I would expect the same behavior for the SW mode. > > Regardless, if the only reason we care about taking the interrupt during > the suspend/resume phases is due to the timer sharing the IRQ, then > shouldn't we be using IRQF_COND_SUSPEND? I'm not sure, but IMO this interrupt should be flagged as NO_SUSPEND, because it's here to reset the system (even if it is suspended). If you flag the irq line as COND_SUSPEND, and atmel decide to give this peripheral its own IRQ line (on new SoCs), then your watchdog will not reboot the system when it is suspended. Another solution would be to support wakeup for this peripheral and delay the system reboot until it has resumed. Anyway, if we decide to go for the wakeup approach, I'd prefer to post another patch on top of this one. Best Regards, Boris -- Boris Brezillon, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com -- To unsubscribe from this list: send the line "unsubscribe linux-serial" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
