Re: [PATCH v2 4/9] watchdog: msc313e: Enable clock before accessing hardware registers

[Tzung-Bi Shih]

On Wed, Sep 09, 2026 at 02:14:41PM -0700, Guenter Roeck wrote:
> On Sat, Aug 29, 2026 at 12:13:43AM +0800, Tzung-Bi Shih wrote:
> > v2:
> > - Fix a clock leak issue in the error handling path (Sashiko reported).
> > 
> > v1: https://lore.kernel.org/all/20260827044700.554333-3-tzungbi@xxxxxxxxxx
...
> > @@ -144,7 +157,13 @@ static int msc313e_wdt_probe(struct platform_device *pdev)
> >  	watchdog_stop_on_reboot(&priv->wdev);
> >  	watchdog_stop_on_unregister(&priv->wdev);
> >  
> > -	return devm_watchdog_register_device(dev, &priv->wdev);
> > +	ret = devm_watchdog_register_device(dev, &priv->wdev);
> > +
> > +	/* If the WDT is running and anything goes wrong, disable the clock. */
> > +	if (ret && test_bit(WDOG_HW_RUNNING, &priv->wdev.status))
> > +		clk_disable_unprepare(priv->clk);
> 
> Curious. Does this mean that Sashiko complains either way ?

Correct.

Sashiko complained about:
- "[Medium] Clock reference count is leaked on the probe error path if
   watchdog registration fails." in v1 [1].
- "[High] Probe error path disables the watchdog clock without executing
   the hardware shutdown sequence, neutralizing the fail-safe." in v2
   (current version) [2].

Current version makes more sense to me.  It balances the CCF reference
count correctly in both paths.

Moreover, if a system relies on the "fail-safe" WDT armed by bootloader,
the WDT shouldn't really depend on the prepared and enabled clock.  Note
that the WDT should be already running before the driver gets probed.

[1] https://lore.kernel.org/all/20260827050107.9AD441F000E9@xxxxxxxxxxxxxxx
[2] https://lore.kernel.org/all/20260828162531.7FA3A1F000E9@xxxxxxxxxxxxxxx