Hello Wolfram,
> From: Wolfram Sang <[email protected]>
> Sent: 04 December 2018 12:02
> Subject: [RFC] watchdog: renesas_wdt: don't keep timer value during
> suspend/resume
>
> After discussing this mail thread [1] again, we concluded that giving
> userspace enough time to prepare is our favourite option. So, do not
> keep the time value when suspended but reset it when resuming.
>
> [1] https://patchwork.kernel.org/patch/10252209/
>
> Signed-off-by: Wolfram Sang <[email protected]>
> ---
>
> Fabrizio: can you agree to that? The R-Car BSP team and we (the R-Car upstream
> team) would prefer it this way (knowing it is also not perfect).
I am not too sure, as the system relies on the watchdog to fix problems that
won't resolve
on their own, therefore if you have an application made of two threads, one
very critical
for the health of the system (but unfortunately buggy, which means it is
destined to not
ping watchdog on time), and the other thread takes care of putting the system
to sleep,
you may find yourself in a place where the system doesn't work properly (as the
critical
thread won't work as expected) and never restarts (because the other thread
works properly
and putting the system to sleep resets the watchdog).
Sometimes the line between policies and mechanisms is not a clear cut, I think
this patch
looks more like a policy decision, but I may be wrong.
Cheers,
Fab
>
> drivers/watchdog/renesas_wdt.c | 11 ++++-------
> 1 file changed, 4 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/watchdog/renesas_wdt.c b/drivers/watchdog/renesas_wdt.c
> index b570962e84f3..9f2307bf727b 100644
> --- a/drivers/watchdog/renesas_wdt.c
> +++ b/drivers/watchdog/renesas_wdt.c
> @@ -48,7 +48,6 @@ struct rwdt_priv {
> void __iomem *base;
> struct watchdog_device wdev;
> unsigned long clk_rate;
> -u16 time_left;
> u8 cks;
> };
>
> @@ -263,10 +262,9 @@ static int __maybe_unused rwdt_suspend(struct device
> *dev)
> {
> struct rwdt_priv *priv = dev_get_drvdata(dev);
>
> -if (watchdog_active(&priv->wdev)) {
> -priv->time_left = readw(priv->base + RWTCNT);
> +if (watchdog_active(&priv->wdev))
> rwdt_stop(&priv->wdev);
> -}
> +
> return 0;
> }
>
> @@ -274,10 +272,9 @@ static int __maybe_unused rwdt_resume(struct device *dev)
> {
> struct rwdt_priv *priv = dev_get_drvdata(dev);
>
> -if (watchdog_active(&priv->wdev)) {
> +if (watchdog_active(&priv->wdev))
> rwdt_start(&priv->wdev);
> -rwdt_write(priv, priv->time_left, RWTCNT);
> -}
> +
> return 0;
> }
>
> --
> 2.11.0
[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.
