[PATCH] watchdog: mediatek: Enable pretimeout support

[PATCH] watchdog: mediatek: Enable pretimeout support

[Wanming Gao]

The watchdog pretimeout interrupt needs to be deasserted after the
hardware pretimeout event is received. Add IRQ level control handling
to the MediaTek watchdog interrupt handler so the interrupt is cleared
before notifying the watchdog core.

Serialize all WDT_MODE read-modify-write operations using the existing
watchdog spinlock. Initialize the lock before registering the interrupt
handler and hold it across the complete 70us IRQ level toggle sequence.
This prevents concurrent watchdog start, stop, or pretimeout updates
from overwriting the temporary IRQ level state and disrupting the
deassert pulse.

This avoids leaving the watchdog pretimeout interrupt asserted, prevents
a potential interrupt storm, and allows the kernel to handle the
pretimeout event properly before the final watchdog reset.

Test: Boot ok, trigger watchdog pretimeout case and verify the
      pretimeout interrupt is received and handled before watchdog reset.

Signed-off-by: Wanming Gao <wanming.gao@mediatek.com>
---
 drivers/watchdog/mtk_wdt.c | 59 +++++++++++++++++++++++++++++++++++---
 1 file changed, 55 insertions(+), 4 deletions(-)

diff --git a/drivers/watchdog/mtk_wdt.c b/drivers/watchdog/mtk_wdt.c
index d9c30e4c80e3..17176bee9d38 100644
--- a/drivers/watchdog/mtk_wdt.c
+++ b/drivers/watchdog/mtk_wdt.c
@@ -35,6 +35,7 @@
 #define WDT_MAX_TIMEOUT		31
 #define WDT_MIN_TIMEOUT		2
 #define WDT_LENGTH_TIMEOUT(n)	((n) << 5)
+#define WDT_IRQ_LEVEL_SYNC_US	70
 
 #define WDT_LENGTH		0x04
 #define WDT_LENGTH_KEY		0x8
@@ -49,6 +50,7 @@
 #define WDT_MODE_EXRST_EN	(1 << 2)
 #define WDT_MODE_IRQ_EN		(1 << 3)
 #define WDT_MODE_AUTO_START	(1 << 4)
+#define WDT_MODE_IRQ_LEVEL_EN	(1 << 5)
 #define WDT_MODE_DUAL_EN	(1 << 6)
 #define WDT_MODE_CNT_SEL	(1 << 8)
 #define WDT_MODE_KEY		0x22000000
@@ -72,7 +74,7 @@ static unsigned int timeout;
 struct mtk_wdt_dev {
 	struct watchdog_device wdt_dev;
 	void __iomem *wdt_base;
-	spinlock_t lock; /* protects WDT_SWSYSRST reg */
+	spinlock_t lock; /* protects WDT_MODE and WDT_SWSYSRST reg */
 	struct reset_controller_dev rcdev;
 	bool disable_wdt_extrst;
 	bool reset_by_toprgu;
@@ -212,8 +214,6 @@ static int toprgu_register_reset_controller(struct platform_device *pdev,
 	int ret;
 	struct mtk_wdt_dev *mtk_wdt = platform_get_drvdata(pdev);
 
-	spin_lock_init(&mtk_wdt->lock);
-
 	mtk_wdt->rcdev.owner = THIS_MODULE;
 	mtk_wdt->rcdev.nr_resets = rst_num;
 	mtk_wdt->rcdev.ops = &toprgu_reset_ops;
@@ -230,14 +230,17 @@ static int mtk_wdt_restart(struct watchdog_device *wdt_dev,
 {
 	struct mtk_wdt_dev *mtk_wdt = watchdog_get_drvdata(wdt_dev);
 	void __iomem *wdt_base;
+	unsigned long flags;
 	u32 reg;
 
 	wdt_base = mtk_wdt->wdt_base;
 
 	/* Enable reset in order to issue a system reset instead of an IRQ */
+	spin_lock_irqsave(&mtk_wdt->lock, flags);
 	reg = readl(wdt_base + WDT_MODE);
 	reg &= ~WDT_MODE_IRQ_EN;
 	writel(reg | WDT_MODE_KEY, wdt_base + WDT_MODE);
+	spin_unlock_irqrestore(&mtk_wdt->lock, flags);
 
 	while (1) {
 		writel(WDT_SWRST_KEY, wdt_base + WDT_SWRST);
@@ -302,12 +305,15 @@ static int mtk_wdt_stop(struct watchdog_device *wdt_dev)
 {
 	struct mtk_wdt_dev *mtk_wdt = watchdog_get_drvdata(wdt_dev);
 	void __iomem *wdt_base = mtk_wdt->wdt_base;
+	unsigned long flags;
 	u32 reg;
 
+	spin_lock_irqsave(&mtk_wdt->lock, flags);
 	reg = readl(wdt_base + WDT_MODE);
 	reg &= ~WDT_MODE_EN;
 	reg |= WDT_MODE_KEY;
 	iowrite32(reg, wdt_base + WDT_MODE);
+	spin_unlock_irqrestore(&mtk_wdt->lock, flags);
 
 	return 0;
 }
@@ -317,12 +323,14 @@ static int mtk_wdt_start(struct watchdog_device *wdt_dev)
 	u32 reg;
 	struct mtk_wdt_dev *mtk_wdt = watchdog_get_drvdata(wdt_dev);
 	void __iomem *wdt_base = mtk_wdt->wdt_base;
+	unsigned long flags;
 	int ret;
 
 	ret = mtk_wdt_set_timeout(wdt_dev, wdt_dev->timeout);
 	if (ret < 0)
 		return ret;
 
+	spin_lock_irqsave(&mtk_wdt->lock, flags);
 	reg = ioread32(wdt_base + WDT_MODE);
 	if (wdt_dev->pretimeout)
 		reg |= (WDT_MODE_IRQ_EN | WDT_MODE_DUAL_EN);
@@ -334,6 +342,7 @@ static int mtk_wdt_start(struct watchdog_device *wdt_dev)
 		reg |= WDT_MODE_CNT_SEL;
 	reg |= (WDT_MODE_EN | WDT_MODE_KEY);
 	iowrite32(reg, wdt_base + WDT_MODE);
+	spin_unlock_irqrestore(&mtk_wdt->lock, flags);
 
 	return 0;
 }
@@ -343,7 +352,11 @@ static int mtk_wdt_set_pretimeout(struct watchdog_device *wdd,
 {
 	struct mtk_wdt_dev *mtk_wdt = watchdog_get_drvdata(wdd);
 	void __iomem *wdt_base = mtk_wdt->wdt_base;
-	u32 reg = ioread32(wdt_base + WDT_MODE);
+	unsigned long flags;
+	u32 reg;
+
+	spin_lock_irqsave(&mtk_wdt->lock, flags);
+	reg = ioread32(wdt_base + WDT_MODE);
 
 	if (timeout && !wdd->pretimeout) {
 		wdd->pretimeout = wdd->timeout / 2;
@@ -352,19 +365,55 @@ static int mtk_wdt_set_pretimeout(struct watchdog_device *wdd,
 		wdd->pretimeout = 0;
 		reg &= ~(WDT_MODE_IRQ_EN | WDT_MODE_DUAL_EN);
 	} else {
+		spin_unlock_irqrestore(&mtk_wdt->lock, flags);
 		return 0;
 	}
 
 	reg |= WDT_MODE_KEY;
 	iowrite32(reg, wdt_base + WDT_MODE);
+	spin_unlock_irqrestore(&mtk_wdt->lock, flags);
 
 	return mtk_wdt_set_timeout(wdd, wdd->timeout);
 }
 
+static void mtk_wdt_deassert_irq(struct watchdog_device *wdd)
+{
+	struct mtk_wdt_dev *mtk_wdt = watchdog_get_drvdata(wdd);
+	void __iomem *wdt_base = mtk_wdt->wdt_base;
+	unsigned long flags;
+	u32 reg;
+
+	spin_lock_irqsave(&mtk_wdt->lock, flags);
+	reg = ioread32(wdt_base + WDT_MODE);
+
+	if (reg & WDT_MODE_IRQ_LEVEL_EN) {
+		reg &= ~WDT_MODE_IRQ_LEVEL_EN;
+		iowrite32(reg | WDT_MODE_KEY, wdt_base + WDT_MODE);
+		/*
+		 * Wait for two 32KHz watchdog clock cycles so the
+		 * hardware can latch the IRQ level change across the clock
+		 * domain.
+		 */
+		udelay(WDT_IRQ_LEVEL_SYNC_US);
+		reg = ioread32(wdt_base + WDT_MODE);
+		reg |= WDT_MODE_IRQ_LEVEL_EN;
+	} else {
+		reg |= WDT_MODE_IRQ_LEVEL_EN;
+		iowrite32(reg | WDT_MODE_KEY, wdt_base + WDT_MODE);
+		udelay(WDT_IRQ_LEVEL_SYNC_US);
+		reg = ioread32(wdt_base + WDT_MODE);
+		reg &= ~WDT_MODE_IRQ_LEVEL_EN;
+	}
+	iowrite32(reg | WDT_MODE_KEY, wdt_base + WDT_MODE);
+	ioread32(wdt_base + WDT_MODE);
+	spin_unlock_irqrestore(&mtk_wdt->lock, flags);
+}
+
 static irqreturn_t mtk_wdt_isr(int irq, void *arg)
 {
 	struct watchdog_device *wdd = arg;
 
+	mtk_wdt_deassert_irq(wdd);
 	watchdog_notify_pretimeout(wdd);
 
 	return IRQ_HANDLED;
@@ -406,6 +455,8 @@ static int mtk_wdt_probe(struct platform_device *pdev)
 	if (!mtk_wdt)
 		return -ENOMEM;
 
+	spin_lock_init(&mtk_wdt->lock);
+
 	platform_set_drvdata(pdev, mtk_wdt);
 
 	mtk_wdt->wdt_base = devm_platform_ioremap_resource(pdev, 0);
-- 
2.45.2

Re: [PATCH] watchdog: mediatek: Enable pretimeout support

[Tzung-Bi Shih]

On Mon, Aug 24, 2026 at 09:29:12PM +0800, Wanming Gao wrote:
> The watchdog pretimeout interrupt needs to be deasserted after the
> hardware pretimeout event is received. Add IRQ level control handling
> to the MediaTek watchdog interrupt handler so the interrupt is cleared
> before notifying the watchdog core.

This could be clearer.  Please merge this with the third paragraph to
explain the why rather than just the what (e.g., explain that the hardware
uses level triggering and requires an edge transition to clear the
interrupt, if that is the case).

> Serialize all WDT_MODE read-modify-write operations using the existing
> watchdog spinlock. Initialize the lock before registering the interrupt
> handler and hold it across the complete 70us IRQ level toggle sequence.
> This prevents concurrent watchdog start, stop, or pretimeout updates
> from overwriting the temporary IRQ level state and disrupting the
> deassert pulse.

Please expand on the why here as well.  The 70us delay specifically needs
to be explained in the commit message (e.g., waiting for the clock domain
crossing to the 32KHz clock).

> 
> This avoids leaving the watchdog pretimeout interrupt asserted, prevents
> a potential interrupt storm, and allows the kernel to handle the
> pretimeout event properly before the final watchdog reset.

As mentioned above, please merge this with the first paragraph.

> 
> Test: Boot ok, trigger watchdog pretimeout case and verify the
>       pretimeout interrupt is received and handled before watchdog reset.

It is expected that patches are tested before submission.  Unless you are
specifying the exact board/platform you tested against, please drop this
section.

> diff --git a/drivers/watchdog/mtk_wdt.c b/drivers/watchdog/mtk_wdt.c
...
> +static void mtk_wdt_deassert_irq(struct watchdog_device *wdd)
> +{
> +	struct mtk_wdt_dev *mtk_wdt = watchdog_get_drvdata(wdd);
> +	void __iomem *wdt_base = mtk_wdt->wdt_base;
> +	unsigned long flags;
> +	u32 reg;
> +
> +	spin_lock_irqsave(&mtk_wdt->lock, flags);
> +	reg = ioread32(wdt_base + WDT_MODE);
> +
> +	if (reg & WDT_MODE_IRQ_LEVEL_EN) {
> +		reg &= ~WDT_MODE_IRQ_LEVEL_EN;
> +		iowrite32(reg | WDT_MODE_KEY, wdt_base + WDT_MODE);
> +		/*
> +		 * Wait for two 32KHz watchdog clock cycles so the
> +		 * hardware can latch the IRQ level change across the clock
> +		 * domain.
> +		 */
> +		udelay(WDT_IRQ_LEVEL_SYNC_US);
> +		reg = ioread32(wdt_base + WDT_MODE);
> +		reg |= WDT_MODE_IRQ_LEVEL_EN;
> +	} else {
> +		reg |= WDT_MODE_IRQ_LEVEL_EN;
> +		iowrite32(reg | WDT_MODE_KEY, wdt_base + WDT_MODE);
> +		udelay(WDT_IRQ_LEVEL_SYNC_US);
> +		reg = ioread32(wdt_base + WDT_MODE);
> +		reg &= ~WDT_MODE_IRQ_LEVEL_EN;
> +	}

Is the main purpose of toggling WDT_MODE_IRQ_LEVEL_EN simply to clear the
interrupt?  Since the value of the bit doesn't seem to matter.

> +	iowrite32(reg | WDT_MODE_KEY, wdt_base + WDT_MODE);
> +	ioread32(wdt_base + WDT_MODE);

This read deserves a comment.  Otherwise, it looks like a NOP and might be
accidentally removed by someone in the future.

> +	spin_unlock_irqrestore(&mtk_wdt->lock, flags);
> +}

The function can be simplified a bit, e.g.:

    /* Trigger a state change */
    reg ^= WDT_MODE_IRQ_LEVEL_EN;
    iowrite32(...);

    /*
     * Wait for two 32KHz watchdog clock cycles ...
     */
    udelay(...);

    /* Restore to its original state */
    reg = ioread32(...)
    reg ^= WDT_MODE_IRQ_LEVEL_EN;
    iowrite32(...)