Re: [PATCH v3 1/3] watchdog: rzv2h_wdt: Propagate WDTDCR access errors
From: Lad, Prabhakar
Date: Mon Aug 17 2026 - 11:45:18 EST
Hi Geert,
On Mon, Aug 17, 2026 at 10:46 AM Geert Uytterhoeven
<geert@xxxxxxxxxxxxxx> wrote:
>
> Hi Prabhakar,
>
> On Fri, 14 Aug 2026 at 21:15, Prabhakar <prabhakar.csengg@xxxxxxxxx> wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
> >
> > 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 <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
> > ---
> > 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 -- geert@xxxxxxxxxxxxxx
>
> 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