[PATCH] watchdog: msc313e: Fix premature reset during timeout update

[PATCH] watchdog: msc313e: Fix premature reset during timeout update

[Tzung-Bi Shih]

Updating the 32-bit hardware timeout requires writing to two 16-bit
registers sequentially.  If the watchdog is actively running, this
non-atomic update might trigger a premature system reset.

Clear the watchdog counter before updating the registers to prevent the
timer from timing out prematurely against an intermediate threshold.

Fixes: e9800b799464 ("watchdog: Add Mstar MSC313e WDT driver")
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
This fixes a Sashiko report [1].

[1] https://lore.kernel.org/all/20260828162210.3C2881F000E9@smtp.kernel.org
---
 drivers/watchdog/msc313e_wdt.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/watchdog/msc313e_wdt.c b/drivers/watchdog/msc313e_wdt.c
index dc7bdd5660e6..0a68be7a217b 100644
--- a/drivers/watchdog/msc313e_wdt.c
+++ b/drivers/watchdog/msc313e_wdt.c
@@ -46,6 +46,9 @@ static void msc313e_wdt_set_hw_timeout(struct msc313e_wdt_priv *priv,
 {
 	u32 t = timeout * clk_get_rate(priv->clk);
 
+	/* Clear before to prevent premature reset during non-atomic updates. */
+	writew(1, priv->base + REG_WDT_CLR);
+
 	writew(t & 0xffff, priv->base + REG_WDT_MAX_PRD_L);
 	writew((t >> 16) & 0xffff, priv->base + REG_WDT_MAX_PRD_H);
 	writew(1, priv->base + REG_WDT_CLR);
@@ -76,6 +79,9 @@ static int msc313e_wdt_stop(struct watchdog_device *wdev)
 {
 	struct msc313e_wdt_priv *priv = watchdog_get_drvdata(wdev);
 
+	/* Clear before to prevent premature reset during non-atomic updates. */
+	writew(1, priv->base + REG_WDT_CLR);
+
 	writew(0, priv->base + REG_WDT_MAX_PRD_L);
 	writew(0, priv->base + REG_WDT_MAX_PRD_H);
 	writew(0, priv->base + REG_WDT_CLR);
-- 
2.53.0

Re: [PATCH] watchdog: msc313e: Fix premature reset during timeout update

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Incomplete race condition fix leading to premature system reset during watchdog timeout update.
--

commit 7f35d2e3193543673ce5c7858773cd48582d5c52
Author: Tzung-Bi Shih <tzungbi@kernel.org>

watchdog: msc313e: Fix premature reset during timeout update

Updating the 32-bit hardware timeout requires writing to two 16-bit
registers sequentially. Clear the watchdog counter before updating the
registers to prevent the timer from timing out prematurely.

> diff --git a/drivers/watchdog/msc313e_wdt.c b/drivers/watchdog/msc313e_wdt.c
> index 4a5cce2a16b18..a743087e922d5 100644
> --- a/drivers/watchdog/msc313e_wdt.c
> +++ b/drivers/watchdog/msc313e_wdt.c
> @@ -46,6 +46,9 @@ static void msc313e_wdt_set_hw_timeout(struct msc313e_wdt_priv *priv,
>  {
>  	u32 t = timeout * clk_get_rate(priv->clk);
>  
> +	/* Clear before to prevent premature reset during non-atomic updates. */
> +	writew(1, priv->base + REG_WDT_CLR);
> +
>  	writew(t & 0xffff, priv->base + REG_WDT_MAX_PRD_L);
>  	writew((t >> 16) & 0xffff, priv->base + REG_WDT_MAX_PRD_H);

[Severity: High]
This is a pre-existing issue, but does this non-atomic update still leave a
race condition window?

If a preemption or interrupt occurs between these two writes in
msc313e_wdt_set_hw_timeout(), the active threshold becomes a hybrid of the
old high bits and the new low bits.

Since the counter was just cleared to 0 immediately prior to this, if the
old high bits are 0 and the new low bits are small, the intermediate
threshold will be very low.

Could the counter reach this small intermediate threshold during the
preemption window and still trigger a premature hardware reset?

>  	writew(1, priv->base + REG_WDT_CLR);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260913065126.8350-1-tzungbi@kernel.org?part=1