Re: [PATCH v5 1/8] watchdog: rzv2h: Drop enabling clocks in the restart handler

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Unchecked division by zero during probe if the optional 'oscclk' is missing from the device tree.
--

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

watchdog: rzv2h: Drop enabling clocks in the restart handler

The watchdog restart handler runs from do_kernel_restart() with
interrupts disabled and after smp_send_stop() has halted the other CPUs.
Calling clk_enable() from the !watchdog_active() path takes the clock
framework's global enable_lock, which can cause a spin if another CPU holds it.
This commit keeps pclk and oscclk enabled for the lifetime of the watchdog
device by using devm_clk_get_enabled() and devm_clk_get_optional_enabled().

> diff --git a/drivers/watchdog/rzv2h_wdt.c b/drivers/watchdog/rzv2h_wdt.c
> index 3b6abb66a1da0..bd86da71b0eb8 100644
> --- a/drivers/watchdog/rzv2h_wdt.c
> +++ b/drivers/watchdog/rzv2h_wdt.c

[ ... ]

> @@ -290,11 +277,11 @@ static int rzv2h_wdt_probe(struct platform_device *pdev)
>  	if (IS_ERR(priv->base))
>  		return PTR_ERR(priv->base);
>  
> -	priv->pclk = devm_clk_get_prepared(dev, "pclk");
> +	priv->pclk = devm_clk_get_enabled(dev, "pclk");
>  	if (IS_ERR(priv->pclk))
>  		return dev_err_probe(dev, PTR_ERR(priv->pclk), "Failed to get pclk\n");
>  
> -	priv->oscclk = devm_clk_get_optional_prepared(dev, "oscclk");
> +	priv->oscclk = devm_clk_get_optional_enabled(dev, "oscclk");

[Severity: High]
This is a pre-existing issue, but can this optional clock fetch result in a
divide by zero oops?

If the optional "oscclk" is legitimately omitted from the device tree (or the
device tree is malformed), priv->oscclk will be successfully returned as NULL.

Later in rzv2h_wdt_probe(), if the chip variant uses COUNT_SOURCE_LOCO, this
NULL pointer is assigned to count_clk and evaluated:

	switch (priv->of_data->count_source) {
	case COUNT_SOURCE_LOCO:
		count_clk = priv->oscclk;
		break;
	...

	priv->wdev.max_hw_heartbeat_ms = (MILLI * priv->of_data->timeout_cycles *
					  priv->of_data->cks_div) / clk_get_rate(count_clk);

Because clk_get_rate(NULL) safely returns 0, the unchecked division will
cause a divide error exception during device probe. Should this verify
count_clk or its rate before the division?

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