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

[Geert Uytterhoeven]

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.

> +                       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