Re: [PATCH v3 1/3] watchdog: rzv2h_wdt: Propagate WDTDCR access errors

Hi Geert,

On Mon, Aug 17, 2026 at 10:46 AM Geert Uytterhoeven
<[email protected]> wrote:
>
> Hi Prabhakar,
>
> On Fri, 14 Aug 2026 at 21:15, Prabhakar <[email protected]> wrote:
> > From: Lad Prabhakar <[email protected]>
> >
> > The WDTDCR helpers currently access the register directly using
> > `readl()`/`writel()` and therefore cannot report failures to their callers.
> > WDTDCR is located in a shared syscon region and will be accessed through
> > regmap in a subsequent change, where register accesses can fail.
> >
> > Make `rzt2h_wdt_wdtdcr_count_start()` and `rzt2h_wdt_wdtdcr_count_stop()`
> > return an error so their callers can propagate failures.
> >
> > Handle these errors in the watchdog start, stop and restart paths and
> > unwind resources acquired before the WDTDCR access. In particular, restore
> > the reset and clock state and release the runtime PM reference as
> > appropriate when an access fails.
> >
> > The helpers still return zero with the current `readl()`/`writel()`
> > implementation, so this change does not alter the driver's behaviour by
> > itself. It prepares the error handling required for the subsequent regmap
> > conversion of WDTDCR access.
> >
> > Signed-off-by: Lad Prabhakar <[email protected]>
> > ---
> > v2->v3:
> > - New patch, split from v2 patch #1 to make thing easier to review.
>
> Thanks for the update!
>
> > --- a/drivers/watchdog/rzv2h_wdt.c
> > +++ b/drivers/watchdog/rzv2h_wdt.c
>
> > @@ -234,8 +250,19 @@ static int rzv2h_wdt_restart(struct watchdog_device *wdev,
> >         rzv2h_wdt_setup(wdev, priv->of_data->cks_min | WDTCR_RPSS_25 |
> >                         WDTCR_RPES_75 | WDTCR_TOPS_1024);
> >
> > -       if (priv->of_data->wdtdcr)
> > -               rzt2h_wdt_wdtdcr_count_start(priv);
> > +       if (priv->of_data->wdtdcr) {
> > +               ret = rzt2h_wdt_wdtdcr_count_start(priv);
> > +               if (ret) {
> > +                       if (!active) {
> > +                               reset_control_assert(priv->rstc);
> > +                               clk_disable(priv->oscclk);
> > +                               clk_disable(priv->pclk);
> > +                       } else {
> > +                               reset_control_reset(priv->rstc);
> > +                       }
>
> FTR, this else-branch is also new in v3.
>
Thanks for pointing that out.

Cheers,
Prabhakar

> > +                       return ret;
> > +               }
> > +       }
> >
> >         rzv2h_wdt_ping(wdev);
> >
>
> Gr{oetje,eeting}s,
>
>                         Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
>
> 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