Hello Guenter, > From: Guenter Roeck <groeck7@xxxxxxxxx> On Behalf Of Guenter Roeck > Sent: 09 December 2018 18:13 > Subject: Re: [RFC] watchdog: renesas_wdt: don't keep timer value during suspend/resume > > On 12/9/18 8:36 AM, Wolfram Sang wrote: > > Hi Guenter, > > > >>> I can relate to the policy argument, though. Regardless of this patch, I > >>> wonder if we can make it configurable from userspace. A draft: > >>> > >>> #defineWDIOF_RESUME_OPTS0x0800 > >>> > >>> #defineWDIOS_RESUME_KEEP0x0008 > >>> #defineWDIOS_RESUME_RESET0x0010 > >>> > >>> and then in watchdog_ioctl() under WDIOC_SETOPTIONS: > >>> > >>> if (!(wdd->info->options & WDIOF_RESUME_OPTS)) > >>> err = -EOPNOTSUPP; > >>> goto break; > >>> > >>> if (val & WDIOS_RESUME_KEEP) > >>> wdd->status |= WDOG_KEEP_TIMER_WHEN_RESUME; > >>> > >>> if (val & WDIOS_RESUME_RESET) > >>> wdd->status ~= ~WDOG_KEEP_TIMER_WHEN_RESUME; > >>> > >>> So, drivers with WDIOF_RESUME_OPTS could act on the > >>> WDOG_KEEP_TIMER_WHEN_RESUME flag. > >>> > >>> Opinions? > >>> > >> > >> Not entirely sure I understand the use case. > > > > Well, as I mentioned before, I can understand the "isn't this policy?" > > question from Fabrizio. Would be good to hear his opinion on this. > > > I understand, but what is the use case behind it ? If the watchdog Is there a documented use case for resetting the counter at resume? I don't think the documentation is clear about this, therefore we need to think ahead > was close to expire on suspend, we want it to expire for good on resume ? That decision is up to user space, isn't it? The decision to go to sleep comes from user space, therefore user space should consider the time left on the counter before going to sleep (or ask the system to ping the watchdog at resume), and do what's best for the health of the system, from kernel space we don't know if the user application is behaving as expected or not, therefore we don't know what's best for the system. Why don't we let user space decide? I guess Wolfram proposal goes in the right direction? > Make the watchdog during a suspend/resume cycle more stringent that during > normal operation, effectively letting it expire early (or earlier) ? As the decision to go to sleep comes from user space, I don't think we can say that letting the watchdog expire on resume is more stringent (or unfair) than during normal operation, if the system is healthy user space should consider the delay introduced by going to sleep and waking up and it should make sure that there is enough time left on the watchdog timer before asking the system to go to sleep. > > I'd rather clarify in the documentation that watchdog drivers are expected > to ping the watchdog after resume, ie that restarting the watchdog after > resume should be handled like starting the watchdog. Let me understand this a little bit better, if you have a use case where you don't want to automatically ping the watchdog at resume you can't go to sleep? > > >> Having said that, if we were to add this option, I think only a single > >> flag would be needed - WDIOF_RESUME_KEEP. All we need to do is declare > >> that a ping on resume shall be the default. Anything else would result > >> in undefined per-driver default behavior. > > > > I would very much love that. To be honest, I thought we already are in > > the undefined per-driver behaviour; this is why I added two flags, to > > not cause regressions. Declaring a default would be a great first step I agree with Wolfram, to me it looks like this is undefined per-driver behaviour already I hope this helps. Fab > > How would adding two flag bits (or one, for that matter) change the > current (undefined) behavior ? > Also, what exactly would be the regression ? Existing drivers would > not change their behavior, neither with one or two or three flag bits. > > Thanks, > Guenter > > > IMHO, and then we can build the WDIOF_RESUME_KEEP option on top of it, > > if needed. But for clarity, the first step seems to be a good idea in > > any case, I'd say. > > > Thanks, > > > > Wolfram > > > > [https://www2.renesas.eu/media/email/unicef.jpg] This Christmas, instead of sending out cards, Renesas Electronics Europe have decided to support Unicef with a donation. For further details click here<https://www.unicef.org/> to find out about the valuable work they do, helping children all over the world. We would like to take this opportunity to wish you a Merry Christmas and a prosperous New Year. Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne End, Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under Registered No. 04586709.
