watchdog: sbsa: Adjust keepalive timeout to avoid MediaTek WS0 race condition

The MediaTek implementation of the sbsa_gwdt watchdog has a race
condition where a write to SBSA_GWDT_WRR is ignored if it occurs while
the hardware is processing a timeout refresh that asserts WS0.

Detect this based on the hardware implementer and adjust
wdd->min_hw_heartbeat_ms to avoid the race by forcing the keepalive ping
to be one second later.

Signed-off-by: Aaron Plattner <aplattner@nvidia.com>
Acked-by: Timur Tabi <ttabi@nvidia.com>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
Link: https://lore.kernel.org/r/20250721230640.2244915-1-aplattner@nvidia.com
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Wim Van Sebroeck <wim@linux-watchdog.org>

diff --git a/drivers/watchdog/sbsa_gwdt.c b/drivers/watchdog/sbsa_gwdt.c
index 5f23913ce3b49c4d054eaebf9057e99b4689f8d7..6ce1bfb39064133c005bb48ccce8625ae1bc16c1 100644 (file)
--- a/drivers/watchdog/sbsa_gwdt.c
+++ b/drivers/watchdog/sbsa_gwdt.c
@@ -75,11 +75,17 @@
 #define SBSA_GWDT_VERSION_MASK  0xF
 #define SBSA_GWDT_VERSION_SHIFT 16
 
+#define SBSA_GWDT_IMPL_MASK    0x7FF
+#define SBSA_GWDT_IMPL_SHIFT   0
+#define SBSA_GWDT_IMPL_MEDIATEK        0x426
+
 /**
  * struct sbsa_gwdt - Internal representation of the SBSA GWDT
  * @wdd:               kernel watchdog_device structure
  * @clk:               store the System Counter clock frequency, in Hz.
  * @version:            store the architecture version
+ * @need_ws0_race_workaround:
+ *                     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
  */
@@ -87,6 +93,7 @@ struct sbsa_gwdt {
        struct watchdog_device  wdd;
        u32                     clk;
        int                     version;
+       bool                    need_ws0_race_workaround;
        void __iomem            *refresh_base;
        void __iomem            *control_base;
 };
@@ -161,6 +168,31 @@ static int sbsa_gwdt_set_timeout(struct watchdog_device *wdd,
                 */
                sbsa_gwdt_reg_write(((u64)gwdt->clk / 2) * timeout, gwdt);
 
+       /*
+        * Some watchdog hardware has a race condition where it will ignore
+        * sbsa_gwdt_keepalive() if it is called at the exact moment that a
+        * timeout occurs and WS0 is being asserted. Unfortunately, the default
+        * behavior of the watchdog core is very likely to trigger this race
+        * when action=0 because it programs WOR to be half of the desired
+        * timeout, and watchdog_next_keepalive() chooses the exact same time to
+        * send keepalive pings.
+        *
+        * This triggers a race where sbsa_gwdt_keepalive() can be called right
+        * as WS0 is being asserted, and affected hardware will ignore that
+        * write and continue to assert WS0. After another (timeout / 2)
+        * seconds, the same race happens again. If the driver wins then the
+        * explicit refresh will reset WS0 to false but if the hardware wins,
+        * then WS1 is asserted and the system resets.
+        *
+        * Avoid the problem by scheduling keepalive heartbeats one second later
+        * than the WOR timeout.
+        *
+        * This workaround might not be needed in a future revision of the
+        * hardware.
+        */
+       if (gwdt->need_ws0_race_workaround)
+               wdd->min_hw_heartbeat_ms = timeout * 500 + 1000;
+
        return 0;
 }
 
@@ -202,12 +234,15 @@ static int sbsa_gwdt_keepalive(struct watchdog_device *wdd)
 static void sbsa_gwdt_get_version(struct watchdog_device *wdd)
 {
        struct sbsa_gwdt *gwdt = watchdog_get_drvdata(wdd);
-       int ver;
+       int iidr, ver, impl;
 
-       ver = readl(gwdt->control_base + SBSA_GWDT_W_IIDR);
-       ver = (ver >> SBSA_GWDT_VERSION_SHIFT) & SBSA_GWDT_VERSION_MASK;
+       iidr = readl(gwdt->control_base + SBSA_GWDT_W_IIDR);
+       ver = (iidr >> SBSA_GWDT_VERSION_SHIFT) & SBSA_GWDT_VERSION_MASK;
+       impl = (iidr >> SBSA_GWDT_IMPL_SHIFT) & SBSA_GWDT_IMPL_MASK;
 
        gwdt->version = ver;
+       gwdt->need_ws0_race_workaround =
+               !action && (impl == SBSA_GWDT_IMPL_MEDIATEK);
 }
 
 static int sbsa_gwdt_start(struct watchdog_device *wdd)
@@ -299,6 +334,15 @@ static int sbsa_gwdt_probe(struct platform_device *pdev)
        else
                wdd->max_hw_heartbeat_ms = GENMASK_ULL(47, 0) / gwdt->clk * 1000;
 
+       if (gwdt->need_ws0_race_workaround) {
+               /*
+                * A timeout of 3 seconds means that WOR will be set to 1.5
+                * seconds and the heartbeat will be scheduled every 2.5
+                * seconds.
+                */
+               wdd->min_timeout = 3;
+       }
+
        status = readl(cf_base + SBSA_GWDT_WCS);
        if (status & SBSA_GWDT_WCS_WS1) {
                dev_warn(dev, "System reset by WDT.\n");