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

[Guenter Roeck <linux@roeck-us.net>]

When the watchdog is stopped via ioctl(WDIOC_SETOPTIONS, WDIOS_DISABLECARD)
while /dev/watchdog remains open, watchdog_stop() clears WDOG_HW_RUNNING
and WDOG_ACTIVE, and starfive_wdt_pm_stop() halts the hardware counter and
drops the runtime PM reference, gating core_clk and apb_clk.

A subsequent ioctl(WDIOC_SETTIMEOUT) invokes starfive_wdt_set_timeout()
regardless of whether the watchdog is running. Because
starfive_wdt_set_timeout() unconditionally accesses MMIO registers and
calls starfive_wdt_enable(), updating the timeout while stopped triggers an
unclocked MMIO bus fault when runtime-suspended, or unexpectedly starts the
hardware counter without WDOG_ACTIVE or WDOG_HW_RUNNING set when runtime PM
is disabled.

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
wdt->reload on resume) so JH7100 loads the configured timeout into the
counter when started from probe or after WDIOS_DISABLECARD.

Fixes: db728ea9c7be ("drivers: watchdog: Add StarFive Watchdog driver")
Cc: Xingyu Wu <xingyu.wu@starfivetech.com>
Cc: Ziv Xu <ziv.xu@starfivetech.com>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v2: New patch

 drivers/watchdog/starfive-wdt.c | 32 +++++++++++++++-----------------
 1 file changed, 15 insertions(+), 17 deletions(-)

diff --git a/drivers/watchdog/starfive-wdt.c b/drivers/watchdog/starfive-wdt.c
index ae2d84ccc76f..758030ad9c65 100644
--- a/drivers/watchdog/starfive-wdt.c
+++ b/drivers/watchdog/starfive-wdt.c
@@ -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;
 }
-- 
2.45.2