[PATCH 3/5] watchdog: starfive-wdt: Avoid unclocked register access in remove and PM paths

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

When runtime PM is enabled and the watchdog is inactive, the device enters
runtime suspend and starfive_wdt_runtime_suspend() disables core_clk and
apb_clk.

However, starfive_wdt_remove(), starfive_wdt_shutdown(),
starfive_wdt_suspend(), and starfive_wdt_resume() unconditionally read and
write watchdog MMIO registers without checking whether the watchdog is
running or clocks are enabled, causing a bus fault when the device is
runtime-suspended:

- In starfive_wdt_remove(), starfive_wdt_stop() is called before
  watchdog_unregister_device() while clocks may be gated off. Unregister
  the watchdog device first (which stops an active watchdog via
  watchdog_stop_on_unregister while its runtime PM reference keeps clocks
  enabled), and only call starfive_wdt_stop() if watchdog_hw_running() is
  still set.
- In starfive_wdt_pm_stop(), ignore non-zero idle return codes from
  pm_runtime_put_sync() so watchdog_stop() does not falsely treat a
  stopped watchdog as still running.
- In starfive_wdt_shutdown(), skip starfive_wdt_pm_stop() if the device is
  already runtime-suspended.
- In starfive_wdt_suspend() and starfive_wdt_resume(), only save/stop and
  restore/restart the hardware watchdog when watchdog_hw_running() is set
  (in which case an active runtime PM reference ensures clocks are enabled
  before suspend and re-enabled by pm_runtime_force_resume()).

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>
---
 drivers/watchdog/starfive-wdt.c | 33 +++++++++++++++++++++------------
 1 file changed, 21 insertions(+), 12 deletions(-)

diff --git a/drivers/watchdog/starfive-wdt.c b/drivers/watchdog/starfive-wdt.c
index 4724fccbfe63..18301f5d1144 100644
--- a/drivers/watchdog/starfive-wdt.c
+++ b/drivers/watchdog/starfive-wdt.c
@@ -390,7 +390,10 @@ static int starfive_wdt_pm_stop(struct watchdog_device *wdd)
 	struct starfive_wdt *wdt = watchdog_get_drvdata(wdd);
 
 	starfive_wdt_stop(wdt);
-	return pm_runtime_put_sync(wdd->parent);
+	if (pm_runtime_enabled(wdd->parent))
+		pm_runtime_put_sync(wdd->parent);
+
+	return 0;
 }
 
 static int starfive_wdt_set_timeout(struct watchdog_device *wdd,
@@ -526,9 +529,11 @@ static void starfive_wdt_remove(struct platform_device *pdev)
 {
 	struct starfive_wdt *wdt = platform_get_drvdata(pdev);
 
-	starfive_wdt_stop(wdt);
 	watchdog_unregister_device(&wdt->wdd);
 
+	if (watchdog_hw_running(&wdt->wdd))
+		starfive_wdt_stop(wdt);
+
 	if (pm_runtime_enabled(&pdev->dev))
 		pm_runtime_disable(&pdev->dev);
 	else
@@ -540,18 +545,21 @@ static void starfive_wdt_shutdown(struct platform_device *pdev)
 {
 	struct starfive_wdt *wdt = platform_get_drvdata(pdev);
 
-	starfive_wdt_pm_stop(&wdt->wdd);
+	if (!pm_runtime_suspended(&pdev->dev))
+		starfive_wdt_pm_stop(&wdt->wdd);
 }
 
 static int starfive_wdt_suspend(struct device *dev)
 {
 	struct starfive_wdt *wdt = dev_get_drvdata(dev);
 
-	/* Save watchdog state, and turn it off. */
-	wdt->reload = starfive_wdt_get_count(wdt);
+	if (watchdog_hw_running(&wdt->wdd)) {
+		/* Save watchdog state, and turn it off. */
+		wdt->reload = starfive_wdt_get_count(wdt);
 
-	/* Note that WTCNT doesn't need to be saved. */
-	starfive_wdt_stop(wdt);
+		/* Note that WTCNT doesn't need to be saved. */
+		starfive_wdt_stop(wdt);
+	}
 
 	return pm_runtime_force_suspend(dev);
 }
@@ -565,13 +573,14 @@ static int starfive_wdt_resume(struct device *dev)
 	if (ret)
 		return ret;
 
-	starfive_wdt_unlock(wdt);
-	/* Restore watchdog state. */
-	starfive_wdt_set_reload_count(wdt, wdt->reload);
-	starfive_wdt_lock(wdt);
+	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);
 
-	if (watchdog_active(&wdt->wdd))
 		return starfive_wdt_start(wdt);
+	}
 
 	return 0;
 }
-- 
2.45.2