Hi Geert,
Thanks for the feedback.
> Subject: Re: [PATCH v4 4/6] watchdog: rzg2l_wdt: Add error check for
> reset_control_deassert
>
> Hi Biju,
>
> On Wed, Feb 23, 2022 at 5:01 PM Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> wrote:
> > If reset_control_deassert() fails, then we won't be able to access the
> > device registers. Therefore check the return code of
> > reset_control_deassert() and bailout in case of error.
> >
> > While at it change reset_control_assert->reset_control_reset in
> > rzg2l_wdt_stop() and remove unnecessary reset_control_deassert() from
> > rzg2l_wdt_start().
> >
> > Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
>
> Thanks for your patch!
>
> > --- a/drivers/watchdog/rzg2l_wdt.c
> > +++ b/drivers/watchdog/rzg2l_wdt.c
> > @@ -88,7 +88,6 @@ static int rzg2l_wdt_start(struct watchdog_device
> > *wdev) {
> > struct rzg2l_wdt_priv *priv = watchdog_get_drvdata(wdev);
> >
> > - reset_control_deassert(priv->rstc);
>
> So before, we had a reset control imbalance?
> - After probe, reset was deasserted.
> - After start, reset was deasserted twice. Oops.
> You probably want to mention this in the commit description, too.
OK, will add this in the commit description.
>
> > pm_runtime_get_sync(wdev->parent);
> >
> > /* Initialize time out */
> > @@ -108,7 +107,7 @@ static int rzg2l_wdt_stop(struct watchdog_device
> *wdev)
> > struct rzg2l_wdt_priv *priv = watchdog_get_drvdata(wdev);
> >
> > pm_runtime_put(wdev->parent);
> > - reset_control_assert(priv->rstc);
> > + reset_control_reset(priv->rstc);
>
> As the reset is now deasserted after probe(), stop(), and start(), I think
> the reset_control_reset() in .restart() can now be removed?
For Overflow method still we need to reset the module, so that we can update
WDTSET register to change the timeout value from 60sec to 43.69 msec,
so that reboot can occur after 43.69 msec which corresponds to counter value of 1.
Yes, it is removed in patch#5, which use Force reset by parity error
instead of WDT overflow method.
Regards,
Biju
>
> >
> > return 0;
> > }
> > @@ -204,7 +203,10 @@ static int rzg2l_wdt_probe(struct platform_device
> *pdev)
> > return dev_err_probe(&pdev->dev, PTR_ERR(priv->rstc),
> > "failed to get cpg reset");
> >
> > - reset_control_deassert(priv->rstc);
> > + ret = reset_control_deassert(priv->rstc);
> > + if (ret)
> > + return dev_err_probe(dev, ret, "failed to deassert");
> > +
> > pm_runtime_enable(&pdev->dev);
> >
> > priv->wdev.info = &rzg2l_wdt_ident;
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-
> m68k.org
>
> In personal conversations with technical people, I call myself a hacker.
> But when I'm talking to journalists I just say "programmer" or something
> like that.
> -- Linus Torvalds
