NVIDIA: SAUCE: watchdog: sbsa_gwdt: stop the watchdog across the whole system-sleep transition

The driver stops a running watchdog in its own device suspend callback
and restarts it in its resume callback. That leaves the watchdog armed,
with nobody refreshing it, for the entire early part of suspend entry:
userspace freeze, kernel thread freeze, and every device suspend
callback that runs before this device's own. The same window exists at
the tail end of resume.

When the watchdog is running from boot (early_enable=1, previously
force_enable=1 downstream; 10 s default timeout) and any device stalls
its suspend callback past the timeout, the watchdog resets the system
in the middle of suspend entry. On N1x (Yukon) this fired on about 7%
of suspend attempts in a randomized stress run (9 resets in 124
suspends; the serial console shows the board dropping into the boot
ROM mid-entry with the watchdog reset status set in NONRST_REG2). Two
elimination runs confirm the mechanism: the identical stress matrix
with the parameter off produced zero resets in 118 suspends, and with
the first version of this change (notifier plus the original device
callbacks) applied and the watchdog force-enabled, zero resets in 198
suspends across four runs (the baseline rate predicts about 14). The
version here keeps that mechanism, removes the device resume callback
and adds the locking described below; it is compile tested.

Stop the watchdog from a PM notifier at the *_PREPARE events, before
tasks are frozen and device callbacks run, and restart it at the
PM_POST_* events, after everything has resumed. The driver state (armed,
stopped for sleep) lives under a lock shared with the watchdog ops, so a
userspace stop or magic close after thaw cannot race the restart, and a
start requested while the transition is in progress is deferred until
PM_POST_* instead of arming hardware nobody can refresh. The notifier is
registered before anything can arm the watchdog and its failure fails
the probe. A suspend-only device callback remains as the final guard for
a device whose probe overlapped the *_PREPARE event; it has no resume
counterpart, so nothing re-arms the watchdog during device resume,
before PM_POST_SUSPEND.

This is also upstream-relevant as a companion to the early_enable
parameter: the armed-during-entry window exists for any system running
the SBSA watchdog from boot.

Fixes: 57d2caa ("Watchdog: introduce ARM SBSA watchdog driver")
Signed-off-by: David Cemin <dcemin@nvidia.com>

Author: dcemin-nv

drivers/watchdog/sbsa_gwdt.c

@@ -47,6 +47,8 @@
 #include <linux/module.h>
 #include <linux/moduleparam.h>
 #include <linux/platform_device.h>
+#include <linux/spinlock.h>
+#include <linux/suspend.h>
 #include <linux/uaccess.h>
 #include <linux/watchdog.h>
 #include <asm/arch_timer.h>
@@ -88,6 +90,13 @@
  *			indicate whether to adjust wdd->timeout to avoid a race with WS0
  * @refresh_base:	Virtual address of the watchdog refresh frame
  * @control_base:	Virtual address of the watchdog control frame
+ * @lock:		Serializes the watchdog ops against the system sleep hooks
+ * @hw_armed:		The watchdog is logically running (started by firmware,
+ *			early_enable or userspace); the hardware follows it except
+ *			while a system sleep transition is in progress
+ * @pm_stopped:		A system sleep transition stopped the hardware and owes
+ *			a restart at PM_POST_*
+ * @pm_nb:		PM notifier stopping the watchdog across system sleep
  */
 struct sbsa_gwdt {
 	struct watchdog_device	wdd;
@@ -96,6 +105,10 @@ struct sbsa_gwdt {
 	bool			need_ws0_race_workaround;
 	void __iomem		*refresh_base;
 	void __iomem		*control_base;
+	spinlock_t		lock; /* hw_armed, pm_stopped */
+	bool			hw_armed;
+	bool			pm_stopped;
+	struct notifier_block	pm_nb;
 };
 
 #define DEFAULT_TIMEOUT		10 /* seconds */
@@ -250,22 +263,48 @@ static void sbsa_gwdt_get_version(struct watchdog_device *wdd)
 		!action && (impl == SBSA_GWDT_IMPL_MEDIATEK);
 }
 
+static void sbsa_gwdt_hw_start(struct sbsa_gwdt *gwdt)
+{
+	/* writing WCS will cause an explicit watchdog refresh */
+	writel(SBSA_GWDT_WCS_EN, gwdt->control_base + SBSA_GWDT_WCS);
+}
+
+static void sbsa_gwdt_hw_stop(struct sbsa_gwdt *gwdt)
+{
+	/* Simply write 0 to WCS to clean WCS_EN bit */
+	writel(0, gwdt->control_base + SBSA_GWDT_WCS);
+}
+
 static int sbsa_gwdt_start(struct watchdog_device *wdd)
 {
 	struct sbsa_gwdt *gwdt = watchdog_get_drvdata(wdd);
+	unsigned long flags;
 
-	/* writing WCS will cause an explicit watchdog refresh */
-	writel(SBSA_GWDT_WCS_EN, gwdt->control_base + SBSA_GWDT_WCS);
+	spin_lock_irqsave(&gwdt->lock, flags);
+	gwdt->hw_armed = true;
+	/*
+	 * While a system sleep transition is in progress nobody can refresh
+	 * the watchdog: leave the hardware stopped and let the PM_POST_*
+	 * notifier arm it once everything has resumed.
+	 */
+	if (!gwdt->pm_stopped)
+		sbsa_gwdt_hw_start(gwdt);
+	spin_unlock_irqrestore(&gwdt->lock, flags);
 
 	return 0;
 }
 
 static int sbsa_gwdt_stop(struct watchdog_device *wdd)
 {
 	struct sbsa_gwdt *gwdt = watchdog_get_drvdata(wdd);
+	unsigned long flags;
 
-	/* Simply write 0 to WCS to clean WCS_EN bit */
-	writel(0, gwdt->control_base + SBSA_GWDT_WCS);
+	spin_lock_irqsave(&gwdt->lock, flags);
+	gwdt->hw_armed = false;
+	/* A stop during the transition also cancels the pending restart. */
+	gwdt->pm_stopped = false;
+	sbsa_gwdt_hw_stop(gwdt);
+	spin_unlock_irqrestore(&gwdt->lock, flags);
 
 	return 0;
 }
@@ -294,6 +333,86 @@ static const struct watchdog_ops sbsa_gwdt_ops = {
 	.get_timeleft	= sbsa_gwdt_get_timeleft,
 };
 
+/*
+ * Per-device suspend/resume callbacks alone would stop the watchdog only
+ * once this device itself is suspended, one of the last steps of suspend
+ * entry, and restart it during device resume, before tasks are thawed. A
+ * watchdog running from boot (early_enable) would therefore be armed, with
+ * nobody refreshing it, through task freezing and every other device's
+ * suspend callback on the way down, and again from device resume until
+ * userspace runs on the way up; anything stalling past the timeout in
+ * either window resets the system.
+ *
+ * Own the transition from a PM notifier instead: stop at the *_PREPARE
+ * events, before anything is frozen, and restart at PM_POST_*, after
+ * everything has resumed. The driver state (hw_armed, pm_stopped) is kept
+ * under a lock shared with the watchdog ops so that a userspace stop or
+ * magic close after thaw cannot race the restart, and a start requested
+ * while the transition is in progress is deferred to PM_POST_*. A
+ * suspend-only device callback remains as the final guard for a device
+ * whose probe overlapped the *_PREPARE event; it has no resume
+ * counterpart, so nothing re-arms the hardware before PM_POST_*.
+ */
+static void sbsa_gwdt_sleep_stop(struct sbsa_gwdt *gwdt)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&gwdt->lock, flags);
+	if (gwdt->hw_armed && !gwdt->pm_stopped) {
+		sbsa_gwdt_hw_stop(gwdt);
+		gwdt->pm_stopped = true;
+	}
+	spin_unlock_irqrestore(&gwdt->lock, flags);
+}
+
+static void sbsa_gwdt_sleep_restart(struct sbsa_gwdt *gwdt)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&gwdt->lock, flags);
+	if (gwdt->pm_stopped) {
+		gwdt->pm_stopped = false;
+		if (gwdt->hw_armed)
+			sbsa_gwdt_hw_start(gwdt);
+	}
+	spin_unlock_irqrestore(&gwdt->lock, flags);
+}
+
+static int sbsa_gwdt_pm_notify(struct notifier_block *nb, unsigned long mode,
+			       void *data)
+{
+	struct sbsa_gwdt *gwdt = container_of(nb, struct sbsa_gwdt, pm_nb);
+
+	switch (mode) {
+	case PM_SUSPEND_PREPARE:
+	case PM_HIBERNATION_PREPARE:
+	case PM_RESTORE_PREPARE:
+		sbsa_gwdt_sleep_stop(gwdt);
+		break;
+	case PM_POST_SUSPEND:
+	case PM_POST_HIBERNATION:
+	case PM_POST_RESTORE:
+		sbsa_gwdt_sleep_restart(gwdt);
+		break;
+	}
+
+	return NOTIFY_DONE;
+}
+
+static void sbsa_gwdt_unregister_pm_notifier(void *data)
+{
+	unregister_pm_notifier(data);
+}
+
+static int sbsa_gwdt_suspend(struct device *dev)
+{
+	sbsa_gwdt_sleep_stop(dev_get_drvdata(dev));
+
+	return 0;
+}
+
+static DEFINE_SIMPLE_DEV_PM_OPS(sbsa_gwdt_pm_ops, sbsa_gwdt_suspend, NULL);
+
 static int sbsa_gwdt_probe(struct platform_device *pdev)
 {
 	void __iomem *rf_base, *cf_base;
@@ -325,6 +444,21 @@ static int sbsa_gwdt_probe(struct platform_device *pdev)
 	gwdt->clk = arch_timer_get_cntfrq();
 	gwdt->refresh_base = rf_base;
 	gwdt->control_base = cf_base;
+	spin_lock_init(&gwdt->lock);
+
+	/*
+	 * Register the sleep hook before anything can arm the watchdog, and
+	 * treat its failure as fatal: without it a running watchdog would
+	 * survive into system sleep with nobody refreshing it.
+	 */
+	gwdt->pm_nb.notifier_call = sbsa_gwdt_pm_notify;
+	ret = register_pm_notifier(&gwdt->pm_nb);
+	if (!ret)
+		ret = devm_add_action_or_reset(dev,
+					       sbsa_gwdt_unregister_pm_notifier,
+					       &gwdt->pm_nb);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to register PM notifier\n");
 
 	wdd = &gwdt->wdd;
 	wdd->parent = dev;
@@ -354,8 +488,10 @@ static int sbsa_gwdt_probe(struct platform_device *pdev)
 		dev_warn(dev, "System reset by WDT.\n");
 		wdd->bootstatus |= WDIOF_CARDRESET;
 	}
-	if (status & SBSA_GWDT_WCS_EN)
+	if (status & SBSA_GWDT_WCS_EN) {
 		set_bit(WDOG_HW_RUNNING, &wdd->status);
+		gwdt->hw_armed = true;
+	}
 
 	if (action) {
 		irq = platform_get_irq(pdev, 0);
@@ -414,32 +550,6 @@ static int sbsa_gwdt_probe(struct platform_device *pdev)
 	return 0;
 }
 
-/* Disable watchdog if it is active during suspend */
-static int __maybe_unused sbsa_gwdt_suspend(struct device *dev)
-{
-	struct sbsa_gwdt *gwdt = dev_get_drvdata(dev);
-
-	if (watchdog_hw_running(&gwdt->wdd))
-		sbsa_gwdt_stop(&gwdt->wdd);
-
-	return 0;
-}
-
-/* Enable watchdog if necessary */
-static int __maybe_unused sbsa_gwdt_resume(struct device *dev)
-{
-	struct sbsa_gwdt *gwdt = dev_get_drvdata(dev);
-
-	if (watchdog_hw_running(&gwdt->wdd))
-		sbsa_gwdt_start(&gwdt->wdd);
-
-	return 0;
-}
-
-static const struct dev_pm_ops sbsa_gwdt_pm_ops = {
-	SET_SYSTEM_SLEEP_PM_OPS(sbsa_gwdt_suspend, sbsa_gwdt_resume)
-};
-
 static const struct of_device_id sbsa_gwdt_of_match[] = {
 	{ .compatible = "arm,sbsa-gwdt", },
 	{},
@@ -455,7 +565,7 @@ MODULE_DEVICE_TABLE(platform, sbsa_gwdt_pdev_match);
 static struct platform_driver sbsa_gwdt_driver = {
 	.driver = {
 		.name = DRV_NAME,
-		.pm = &sbsa_gwdt_pm_ops,
+		.pm = pm_sleep_ptr(&sbsa_gwdt_pm_ops),
 		.of_match_table = sbsa_gwdt_of_match,
 	},
 	.probe = sbsa_gwdt_probe,