Hi Boris,

On Mon, Mar 02, 2015 at 09:18:14AM +0000, Boris Brezillon wrote:
> The IRQ line used by the RTC device is usually shared with the system timer
> (PIT) on at91 platforms.
> Since timers are registering their handlers with IRQF_NO_SUSPEND, we should
> expect being called in suspended state, and properly wake the system up
> when this is the case.
> 
> Set IRQF_COND_SUSPEND flag when registering the IRQ handler to inform
> irq core that it can safely be called while the system is suspended.
> 
> Signed-off-by: Boris Brezillon <[email protected]>
> ---
>  drivers/rtc/rtc-at91sam9.c | 73 
> ++++++++++++++++++++++++++++++++++++++--------
>  1 file changed, 61 insertions(+), 12 deletions(-)

[...]

> +/*
> + * IRQ handler for the RTC
> + */
> +static irqreturn_t at91_rtc_interrupt(int irq, void *_rtc)
> +{
> +     struct sam9_rtc *rtc = _rtc;
> +     int ret;
> +
> +     spin_lock(&rtc->lock);
> +
> +     ret = at91_rtc_cache_events(rtc);
> +
> +     /* We're called in suspended state */
> +     if (rtc->suspended) {
> +             /* Mask irqs coming from this peripheral */
> +             rtt_writel(rtc, MR,
> +                        rtt_readl(rtc, MR) &
> +                        ~(AT91_RTT_ALMIEN | AT91_RTT_RTTINCIEN));
> +             /* Trigger a system wakeup */
> +             pm_system_wakeup();
> +     } else {
> +             at91_rtc_flush_events(rtc);
> +     }
> +
> +     spin_unlock(&rtc->lock);
> +
> +     return ret;

[...]

> @@ -421,7 +456,8 @@ static int at91_rtc_probe(struct platform_device *pdev)
>  
>       /* register irq handler after we know what name we'll use */
>       ret = devm_request_irq(&pdev->dev, rtc->irq, at91_rtc_interrupt,
> -                             IRQF_SHARED, dev_name(&rtc->rtcdev->dev), rtc);
> +                            IRQF_SHARED | IRQF_COND_SUSPEND,
> +                            dev_name(&rtc->rtcdev->dev), rtc);

To try to avoid thie getting cargo-culted, it's probably worth expanding
the comment to cover the IRQF_COND_SUSPEND usage. Something like:

        /* 
         * Register irq handler after we know what name we'll use.
         *
         * The interrupt line may be shared with a timer (the PIT), so
         * we must use IRQF_COND_SUSPEND and call pm_system_wakeup()
         * when a genuine wakeup event occurs.
         */

Otherwise this looks good to me.

Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to