Re: [PATCH v6 6/8] watchdog: rzv2h: Add syscon support for WDTDCR

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] watchdog: rzv2h: Deadlock in restart handler via syscon regmap locking
--

commit 1e1b26069f70735a537750b22a51257bb422c2e4
Author: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

watchdog: rzv2h: Add syscon support for WDTDCR

The commit adds support for accessing the WDTDCR register via a syscon
regmap interface for RZ/T2H and RZ/N2H SoCs. It looks up the SYSC regmap
using the "renesas,sysc" property or falls back to MMIO access if absent.

> diff --git a/drivers/watchdog/rzv2h_wdt.c b/drivers/watchdog/rzv2h_wdt.c
> index 76c1d315b0e77..30b0b70fecaa9 100644
> --- a/drivers/watchdog/rzv2h_wdt.c
> +++ b/drivers/watchdog/rzv2h_wdt.c

[ ... ]

> @@ -93,18 +104,22 @@ static int rzv2h_wdt_ping(struct watchdog_device *wdev)
>  
>  static int rzt2h_wdt_wdtdcr_count_stop(struct rzv2h_wdt_priv *priv)
>  {
> -	guard(spinlock_irqsave)(&priv->regmap_lock);
> -	return regmap_set_bits(priv->wdtdcr_regmap, WDTDCR, WDTDCR_WDTSTOPCTRL);
> +	struct rzv2h_sysc_wdtdcr *sysc = &priv->sysc;
> +
> +	guard(spinlock_irqsave)(&sysc->regmap_lock);
> +	return regmap_set_bits(sysc->regmap, sysc->offset, WDTDCR_WDTSTOPCTRL);
>  }

[Severity: High]
Is the external sysc->regmap_lock still necessary here?

When using the syscon regmap returned by syscon_regmap_lookup_by_phandle_args(),
the regmap has internal locking enabled by default. Since regmap_set_bits()
already performs an atomic read-modify-write using the syscon's internal lock,
this external wrapper lock appears redundant for the syscon path.

>  
>  static inline int rzt2h_wdt_wdtdcr_count_start_nolock(struct rzv2h_wdt_priv *priv)
>  {
> -	return regmap_clear_bits(priv->wdtdcr_regmap, WDTDCR, WDTDCR_WDTSTOPCTRL);
> +	struct rzv2h_sysc_wdtdcr *sysc = &priv->sysc;
> +
> +	return regmap_clear_bits(sysc->regmap, sysc->offset, WDTDCR_WDTSTOPCTRL);
>  }

[Severity: High]
Could this lead to a deadlock during a system panic or restart?

This function is called by the rzv2h_wdt_restart() handler, which executes in
atomic or panic context and must remain strictly lockless to prevent deadlocks.

While bypassing the driver's local sysc->regmap_lock was safe in the legacy
MMIO fallback case (where rzv2h_wdtdcr_regmap_config disables locking), the
syscon regmap has internal locking enabled.

Calling regmap_clear_bits() unconditionally attempts to acquire the syscon
regmap's internal spinlock. If the system panics while another CPU is accessing
the shared syscon block, the restart handler could hang trying to acquire this
lock, defeating the purpose of the watchdog restart recovery.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260923104054.3324830-1-prabhakar.mahadev-lad.rj@bp.renesas.com?part=6