Re: [PATCH] rtc: ds1374: wdt:support suspend/resume for watchdog

[Guenter Roeck]

On 09/26/2017 03:22 AM, Alexandre Belloni wrote:
> (+Cc wdt maintainers)
>
> On 26/09/2017 at 09:56:32 +0800, 18502523564 wrote:
> > Hi Alexandre,
> >
> > Thanks for your reply.
> > Do you think is this a issue when using as a watchdog in suspend?
> > I takes the drivers/watchdog/ subsystem for reference, some drivers in suspend
> > also call disable_watchdog.Or do you have any  better suggestion?
>
> I guess this is a question for the watchdog subsystem maintainers.
> However, I think that everything can be handled properly from userspace
> as I know some atmel based devices have their watchdog enabled while
> the platform is suspended.

Depends on the driver, really. There is one thing wrong below, though:
It appears that on resume, the watchdog is unconditionally enabled.

Guenter

> > Thank you.
> >
> >
> >
> >
> > ------------------ Original ------------------
> > From: Alexandre Belloni <alexandre.belloni@xxxxxxxxxxxxxxxxxx>
> > Date: åä,9æ 25,2017 20:07
> > To: winton.liu <18502523564@xxxxxxx>
> > Cc: a.zummo <a.zummo@xxxxxxxxxxxx>, linux-rtc <linux-rtc@xxxxxxxxxxxxxxx>,
> > linux-kernel <linux-kernel@xxxxxxxxxxxxxxx>
> > Subject: Re: [PATCH] rtc: ds1374: wdt:support suspend/resume for watchdog
> >
> >
> > Hi,
> >
> > On 25/09/2017 at 19:58:44 +0800, winton.liu wrote:
> > > When enable CONFIG_RTC_DRV_DS1374_WDT use as watchdog,
> > > in suspend mode, watchdog is still working but no daemon
> > > patting the watchdog. The system will reboot if timeout.
> > > So disable watchdog in suspend  and recover it in resume.
> >
> > That is definitively not what we want. Many people will want to still
> > have the watchdog running when the platform is suspended. Your options
> > are to either disable the watchdog before going to suspend or wake up
> > the platform just in time to ping the watchdog.
> >
> > > Signed-off-by: winton.liu <18502523564@xxxxxxx>
> > > ---
> > >   drivers/rtc/rtc-ds1374.c | 8 ++++++++
> > >   1 file changed, 8 insertions(+)
> > >
> > > diff --git a/drivers/rtc/rtc-ds1374.c b/drivers/rtc/rtc-ds1374.c
> > > index 38a2e9e..e990773 100644
> > > --- a/drivers/rtc/rtc-ds1374.c
> > > +++ b/drivers/rtc/rtc-ds1374.c
> > > @@ -690,6 +690,10 @@ static int ds1374_suspend(struct device *dev)
> > >   {
> > >        struct i2c_client *client = to_i2c_client(dev);
> > >
> > > +#ifdef CONFIG_RTC_DRV_DS1374_WDT
> > > +     ds1374_wdt_disable();
> > > +#endif
> > > +
> > >        if (client->irq > 0 && device_may_wakeup(&client->dev))
> > >                enable_irq_wake(client->irq);
> > >        return 0;
> > > @@ -699,6 +703,10 @@ static int ds1374_resume(struct device *dev)
> > >   {
> > >        struct i2c_client *client = to_i2c_client(dev);
> > >
> > > +#ifdef CONFIG_RTC_DRV_DS1374_WDT
> > > +     ds1374_wdt_settimeout(131072);
> > > +#endif
> > > +
> > >        if (client->irq > 0 && device_may_wakeup(&client->dev))
> > >                disable_irq_wake(client->irq);
> > >        return 0;
> > > --
> > > 1.9.1
> >
> > --
> > Alexandre Belloni, Free Electrons
> > Embedded Linux and Kernel engineering
> > http://free-electrons.com