Hi Wolfram,
thank you for your feedback!
> Subject: Re: [PATCH v6 1/3] watchdog: renesas_wdt: Add suspend/resume support
>
>
> > +unsigned int timeleft;
>
> What about using
> u16 time_left;
>
> ...
>
> > +static void rwdt_set_timeleft(struct rwdt_priv *priv, unsigned int timeleft)
> > +{
> > +rwdt_write(priv, 65536 - MUL_BY_CLKS_PER_SEC(priv, timeleft), RWTCNT);
> > +}
> > +
>
> ... skipping this ...
>
> > static int rwdt_init_timeout(struct watchdog_device *wdev)
> > {
> > struct rwdt_priv *priv = watchdog_get_drvdata(wdev);
> >
> > -rwdt_write(priv, 65536 - MUL_BY_CLKS_PER_SEC(priv, wdev->timeout), RWTCNT);
> > -
> > +rwdt_set_timeleft(priv, wdev->timeout);
>
> ... and this ...
>
> > return 0;
> > }
> >
> > @@ -203,6 +208,30 @@ static int rwdt_remove(struct platform_device *pdev)
> > return 0;
> > }
> >
> > +static int __maybe_unused rwdt_suspend(struct device *dev)
> > +{
> > +struct rwdt_priv *priv = dev_get_drvdata(dev);
> > +
> > +if (watchdog_active(&priv->wdev)) {
> > +priv->timeleft = rwdt_get_timeleft(&priv->wdev);
>
> ...and read the register value directly here...
>
> > +rwdt_stop(&priv->wdev);
> > +}
> > +return 0;
> > +}
> > +
> > +static int __maybe_unused rwdt_resume(struct device *dev)
> > +{
> > +struct rwdt_priv *priv = dev_get_drvdata(dev);
> > +
> > +if (watchdog_active(&priv->wdev)) {
> > +rwdt_start(&priv->wdev);
> > +rwdt_set_timeleft(priv, priv->timeleft);
>
> ... and put it back here?
It works for me, I'll send a new version with your comments applied.
Thanks,
Fab
>
> That would save calling the conversion macros which may introduce
> rounding errors. It is also a tad faster and smaller. And less lines of
> code.
Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne End, Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under Registered No. 04586709.
