Re: [PATCH v2 8/8] watchdog: starfive-wdt: Avoid unclocked MMIO access in starfive_wdt_set_timeout()

[Tzung-Bi Shih <tzungbi@kernel.org>]

On Thu, Nov 05, 2026 at 11:05:29AM -0800, Guenter Roeck wrote:
> Only program the hardware registers in starfive_wdt_set_timeout() when
> watchdog_hw_running() is set. In addition, reload the initial counter value
> via starfive_wdt_set_reload_count() in starfive_wdt_start() (preserving

Is the additional change relevant to this patch (i.e., avoid unclocked
MMIO access)?

> wdt->reload on resume) so JH7100 loads the configured timeout into the
> counter when started from probe or after WDIOS_DISABLECARD.

Is it really JH7100 specific (not for JH7110)?

> @@ -344,7 +344,7 @@ static int starfive_wdt_keepalive(struct watchdog_device *wdd)
>  	return ret;
>  }
>  
> -static int starfive_wdt_start(struct starfive_wdt *wdt)
> +static int starfive_wdt_start(struct starfive_wdt *wdt, u32 count)
>  {
>  	int ret;
>  
> @@ -357,7 +357,9 @@ static int starfive_wdt_start(struct starfive_wdt *wdt)
>  	if (ret)
>  		goto exit;
>  
> -	starfive_wdt_set_count(wdt, wdt->count);
> +	starfive_wdt_set_reload_count(wdt, count);
> +	if (count != wdt->count)
> +		starfive_wdt_set_count(wdt, wdt->count);
>  	starfive_wdt_enable(wdt);
>  
>  exit:
> @@ -381,7 +383,7 @@ static int starfive_wdt_pm_start(struct watchdog_device *wdd)
>  	if (ret < 0)
>  		return ret;
>  
> -	ret = starfive_wdt_start(wdt);
> +	ret = starfive_wdt_start(wdt, wdt->count);
>  	if (ret)
>  		pm_runtime_put_sync(wdd->parent);
>  
> @@ -412,11 +414,13 @@ static int starfive_wdt_set_timeout(struct watchdog_device *wdd,
>  	wdt->count = count;
>  	wdd->timeout = timeout;
>  
> -	starfive_wdt_unlock(wdt);
> -	starfive_wdt_disable(wdt);
> -	starfive_wdt_set_reload_count(wdt, wdt->count);
> -	starfive_wdt_enable(wdt);
> -	starfive_wdt_lock(wdt);
> +	if (watchdog_hw_running(wdd)) {
> +		starfive_wdt_unlock(wdt);
> +		starfive_wdt_disable(wdt);
> +		starfive_wdt_set_reload_count(wdt, wdt->count);
> +		starfive_wdt_enable(wdt);
> +		starfive_wdt_lock(wdt);
> +	}
>  
>  	return 0;
>  }
> @@ -497,7 +501,7 @@ static int starfive_wdt_probe(struct platform_device *pdev)
>  	watchdog_stop_ping_on_suspend(&wdt->wdd);
>  
>  	if (early_enable) {
> -		ret = starfive_wdt_start(wdt);
> +		ret = starfive_wdt_start(wdt, wdt->count);
>  		if (ret)
>  			goto err_exit;
>  		set_bit(WDOG_HW_RUNNING, &wdt->wdd.status);
> @@ -575,14 +579,8 @@ static int starfive_wdt_resume(struct device *dev)
>  	if (ret)
>  		return ret;
>  
> -	if (watchdog_hw_running(&wdt->wdd)) {
> -		starfive_wdt_unlock(wdt);
> -		/* Restore watchdog state. */
> -		starfive_wdt_set_reload_count(wdt, wdt->reload);
> -		starfive_wdt_lock(wdt);
> -
> -		return starfive_wdt_start(wdt);
> -	}
> +	if (watchdog_hw_running(&wdt->wdd))
> +		return starfive_wdt_start(wdt, wdt->reload);
>  
>  	return 0;
>  }