[PATCH v2 0/2] watchdog: orion_wdt: stop the watchdog during system suspend

[PATCH v2 0/2] watchdog: orion_wdt: stop the watchdog during system suspend

[Igor Velkov via B4 Relay]

The watchdog keeps counting across system suspend while the userspace
that pings it is frozen, so a Kobol Helios4 (Armada 388) resets 60 s
into s2idle. Patch 2 stops the timer on suspend and restarts it on
resume. Patch 1 fixes the Armada XP pretimeout that Sashiko spotted on
the restart path of v1; the same bug hits every start since
7a6b3d8a432d.

Tested on a Helios4 (v7.2.9). Unpatched, the board resets 60 s into
s2idle. With patch 2:
- watchdog daemon: three 120 s s2idle cycles, no reset;
- systemd RuntimeWatchdogSec=60s: one 120 s s2idle and one 120 s
  standby cycle, no reset;
- the watchdog still fires after resume: freezing the daemon (SIGSTOP)
  after the s2idle cycles reset the board in 61 s, a panic after
  standby in 54 s.
Patch 1 is compile-tested only; I have no Armada XP.

Signed-off-by: Igor Velkov <iav@iav.lv>
---
Changes in v2:
- Add patch 1, the Armada XP pretimeout reload (Sashiko).
- Keep pm_sleep_ptr(): it is the pairing for DEFINE_SIMPLE_DEV_PM_OPS()
  (1a3c7bb08826).
- Link to v1: https://lore.kernel.org/r/20261012-orion-wdt-suspend-v1-1-1676cb79e29e@iav.lv

---
Igor Velkov (2):
      watchdog: orion_wdt: load the pretimeout counter on Armada 370/XP start
      watchdog: orion_wdt: stop the watchdog during system suspend

 drivers/watchdog/orion_wdt.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)
---
base-commit: 8b5a9f09037e3c372c4e9f2fbda85fcfbf5ea5f0
change-id: 20261012-orion-wdt-suspend-08f276aafa14

Best regards,
-- 
Igor Velkov <iav@iav.lv>

[PATCH v2 0/2] watchdog: orion_wdt: stop the watchdog during system suspend

[Igor Velkov]

The watchdog keeps counting across system suspend while the userspace
that pings it is frozen, so a Kobol Helios4 (Armada 388) resets 60 s
into s2idle. Patch 2 stops the timer on suspend and restarts it on
resume. Patch 1 fixes the Armada XP pretimeout that Sashiko spotted on
the restart path of v1; the same bug hits every start since
7a6b3d8a432d.

Tested on a Helios4 (v7.2.9). Unpatched, the board resets 60 s into
s2idle. With patch 2:
- watchdog daemon: three 120 s s2idle cycles, no reset;
- systemd RuntimeWatchdogSec=60s: one 120 s s2idle and one 120 s
  standby cycle, no reset;
- the watchdog still fires after resume: freezing the daemon (SIGSTOP)
  after the s2idle cycles reset the board in 61 s, a panic after
  standby in 54 s.
Patch 1 is compile-tested only; I have no Armada XP.

Signed-off-by: Igor Velkov <iav@iav.lv>
---
Changes in v2:
- Add patch 1, the Armada XP pretimeout reload (Sashiko).
- Keep pm_sleep_ptr(): it is the pairing for DEFINE_SIMPLE_DEV_PM_OPS()
  (1a3c7bb08826).
- Link to v1: https://lore.kernel.org/r/20261012-orion-wdt-suspend-v1-1-1676cb79e29e@iav.lv

---
Igor Velkov (2):
      watchdog: orion_wdt: load the pretimeout counter on Armada 370/XP start
      watchdog: orion_wdt: stop the watchdog during system suspend

 drivers/watchdog/orion_wdt.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)
---
base-commit: 8b5a9f09037e3c372c4e9f2fbda85fcfbf5ea5f0
change-id: 20261012-orion-wdt-suspend-08f276aafa14

Best regards,
-- 
Igor Velkov <iav@iav.lv>

[PATCH v2 1/2] watchdog: orion_wdt: load the pretimeout counter on Armada 370/XP start

[Igor Velkov via B4 Relay]

From: Igor Velkov <iav@iav.lv>

armada370_start() enables TIMER1 for the pretimeout but, unlike
armada375_start(), does not load its counter, so TIMER1 runs down from
whatever value it holds and the pretimeout can fire early. Of the boards
that use armada370_start(), only Armada XP and 98DX3236 describe the
pretimeout interrupt in their device tree.

Load TIMER1 on start the same way armada375_start() does.

Fixes: 7a6b3d8a432d ("watchdog: orion_wdt: support pretimeout on Armada-XP")
Link: https://sashiko.dev/#/patchset/20261012-orion-wdt-suspend-v1-1-1676cb79e29e@iav.lv?part=1
Assisted-by: LLM
Signed-off-by: Igor Velkov <iav@iav.lv>
---
 drivers/watchdog/orion_wdt.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c
index 5d56e93f0406..ef64e3730001 100644
--- a/drivers/watchdog/orion_wdt.c
+++ b/drivers/watchdog/orion_wdt.c
@@ -232,6 +232,9 @@ static int armada370_start(struct watchdog_device *wdt_dev)
 	/* Set watchdog duration */
 	writel(dev->clk_rate * wdt_dev->timeout,
 	       dev->reg + dev->data->wdt_counter_offset);
+	if (dev->wdt.info->options & WDIOF_PRETIMEOUT)
+		writel(dev->clk_rate * (wdt_dev->timeout - wdt_dev->pretimeout),
+		       dev->reg + TIMER1_VAL_OFF);
 
 	/* Clear the watchdog expiration bit */
 	atomic_io_modify(dev->reg + TIMER_A370_STATUS, WDT_A370_EXPIRED, 0);

-- 
2.43.0

[PATCH v2 1/2] watchdog: orion_wdt: load the pretimeout counter on Armada 370/XP start

[Igor Velkov]

armada370_start() enables TIMER1 for the pretimeout but, unlike
armada375_start(), does not load its counter, so TIMER1 runs down from
whatever value it holds and the pretimeout can fire early. Of the boards
that use armada370_start(), only Armada XP and 98DX3236 describe the
pretimeout interrupt in their device tree.

Load TIMER1 on start the same way armada375_start() does.

Fixes: 7a6b3d8a432d ("watchdog: orion_wdt: support pretimeout on Armada-XP")
Link: https://sashiko.dev/#/patchset/20261012-orion-wdt-suspend-v1-1-1676cb79e29e@iav.lv?part=1
Assisted-by: LLM
Signed-off-by: Igor Velkov <iav@iav.lv>
---
 drivers/watchdog/orion_wdt.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c
index 5d56e93f0406..ef64e3730001 100644
--- a/drivers/watchdog/orion_wdt.c
+++ b/drivers/watchdog/orion_wdt.c
@@ -232,6 +232,9 @@ static int armada370_start(struct watchdog_device *wdt_dev)
 	/* Set watchdog duration */
 	writel(dev->clk_rate * wdt_dev->timeout,
 	       dev->reg + dev->data->wdt_counter_offset);
+	if (dev->wdt.info->options & WDIOF_PRETIMEOUT)
+		writel(dev->clk_rate * (wdt_dev->timeout - wdt_dev->pretimeout),
+		       dev->reg + TIMER1_VAL_OFF);
 
 	/* Clear the watchdog expiration bit */
 	atomic_io_modify(dev->reg + TIMER_A370_STATUS, WDT_A370_EXPIRED, 0);

-- 
2.43.0

[PATCH v2 2/2] watchdog: orion_wdt: stop the watchdog during system suspend

[Igor Velkov via B4 Relay]

From: Igor Velkov <iav@iav.lv>

The watchdog timer keeps counting across system suspend while the
userspace that pings it is frozen, so it expires and resets the system.
On a Kobol Helios4 (Armada 388) with a 60 s timeout the board resets
47-61 s into s2idle, whether the watchdog daemon or systemd's
RuntimeWatchdogSec holds the device.

Stop the timer on suspend and restart it on resume when it is active or
running in hardware, as sp805_wdt does. orion_wdt_start() reloads the
counter, so userspace gets a full timeout after resume.

Assisted-by: LLM
Signed-off-by: Igor Velkov <iav@iav.lv>
---
 drivers/watchdog/orion_wdt.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c
index ef64e3730001..c65042dc2329 100644
--- a/drivers/watchdog/orion_wdt.c
+++ b/drivers/watchdog/orion_wdt.c
@@ -671,6 +671,29 @@ static void orion_wdt_shutdown(struct platform_device *pdev)
 	orion_wdt_stop(wdt_dev);
 }
 
+static int orion_wdt_suspend(struct device *dev)
+{
+	struct watchdog_device *wdt_dev = dev_get_drvdata(dev);
+
+	if (watchdog_active(wdt_dev) || watchdog_hw_running(wdt_dev))
+		return orion_wdt_stop(wdt_dev);
+
+	return 0;
+}
+
+static int orion_wdt_resume(struct device *dev)
+{
+	struct watchdog_device *wdt_dev = dev_get_drvdata(dev);
+
+	if (watchdog_active(wdt_dev) || watchdog_hw_running(wdt_dev))
+		return orion_wdt_start(wdt_dev);
+
+	return 0;
+}
+
+static DEFINE_SIMPLE_DEV_PM_OPS(orion_wdt_pm_ops, orion_wdt_suspend,
+				orion_wdt_resume);
+
 static struct platform_driver orion_wdt_driver = {
 	.probe		= orion_wdt_probe,
 	.remove		= orion_wdt_remove,
@@ -678,6 +701,7 @@ static struct platform_driver orion_wdt_driver = {
 	.driver		= {
 		.name	= "orion_wdt",
 		.of_match_table = orion_wdt_of_match_table,
+		.pm	= pm_sleep_ptr(&orion_wdt_pm_ops),
 	},
 };
 

-- 
2.43.0

[PATCH v2 2/2] watchdog: orion_wdt: stop the watchdog during system suspend

[Igor Velkov]

The watchdog timer keeps counting across system suspend while the
userspace that pings it is frozen, so it expires and resets the system.
On a Kobol Helios4 (Armada 388) with a 60 s timeout the board resets
47-61 s into s2idle, whether the watchdog daemon or systemd's
RuntimeWatchdogSec holds the device.

Stop the timer on suspend and restart it on resume when it is active or
running in hardware, as sp805_wdt does. orion_wdt_start() reloads the
counter, so userspace gets a full timeout after resume.

Assisted-by: LLM
Signed-off-by: Igor Velkov <iav@iav.lv>
---
 drivers/watchdog/orion_wdt.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c
index ef64e3730001..c65042dc2329 100644
--- a/drivers/watchdog/orion_wdt.c
+++ b/drivers/watchdog/orion_wdt.c
@@ -671,6 +671,29 @@ static void orion_wdt_shutdown(struct platform_device *pdev)
 	orion_wdt_stop(wdt_dev);
 }
 
+static int orion_wdt_suspend(struct device *dev)
+{
+	struct watchdog_device *wdt_dev = dev_get_drvdata(dev);
+
+	if (watchdog_active(wdt_dev) || watchdog_hw_running(wdt_dev))
+		return orion_wdt_stop(wdt_dev);
+
+	return 0;
+}
+
+static int orion_wdt_resume(struct device *dev)
+{
+	struct watchdog_device *wdt_dev = dev_get_drvdata(dev);
+
+	if (watchdog_active(wdt_dev) || watchdog_hw_running(wdt_dev))
+		return orion_wdt_start(wdt_dev);
+
+	return 0;
+}
+
+static DEFINE_SIMPLE_DEV_PM_OPS(orion_wdt_pm_ops, orion_wdt_suspend,
+				orion_wdt_resume);
+
 static struct platform_driver orion_wdt_driver = {
 	.probe		= orion_wdt_probe,
 	.remove		= orion_wdt_remove,
@@ -678,6 +701,7 @@ static struct platform_driver orion_wdt_driver = {
 	.driver		= {
 		.name	= "orion_wdt",
 		.of_match_table = orion_wdt_of_match_table,
+		.pm	= pm_sleep_ptr(&orion_wdt_pm_ops),
 	},
 };
 

-- 
2.43.0

Re: [PATCH v2 1/2] watchdog: orion_wdt: load the pretimeout counter on Armada 370/XP start

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] watchdog: orion_wdt: global orion_wdt_info struct contamination
- [Low] watchdog: orion_wdt: TIMER1 clock rate mismatch for pretimeout on Armada 370/375
--

commit dbf1fe3fb63d91004e0ddd635005322182202b47
Author: Igor Velkov <iav@iav.lv>

watchdog: orion_wdt: load the pretimeout counter on Armada 370/XP start

armada370_start() enables TIMER1 for the pretimeout but, unlike
armada375_start(), does not load its counter, so TIMER1 runs down from
whatever value it holds and the pretimeout can fire early. Load TIMER1 on
start the same way armada375_start() does.

Link: https://sashiko.dev/#/patchset/20261012-orion-wdt-suspend-v1-1-1676cb79e29e@iav.lv?part=1

> diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c
> index 5d56e93f04064..ef64e37300012 100644
> --- a/drivers/watchdog/orion_wdt.c
> +++ b/drivers/watchdog/orion_wdt.c
> @@ -232,6 +232,9 @@ static int armada370_start(struct watchdog_device *wdt_dev)
>  	/* Set watchdog duration */
>  	writel(dev->clk_rate * wdt_dev->timeout,
>  	       dev->reg + dev->data->wdt_counter_offset);
> +	if (dev->wdt.info->options & WDIOF_PRETIMEOUT)
[Severity: Medium]
Is it possible for WDIOF_PRETIMEOUT to be set spuriously due to global
state contamination?

In drivers/watchdog/orion_wdt.c:orion_wdt_probe(), the global static
structure orion_wdt_info is unconditionally mutated if a pretimeout IRQ
is found:

static struct watchdog_info orion_wdt_info = {
    ...
};

orion_wdt_probe() {
    ...
    if (irq > 0) {
        orion_wdt_info.options |= WDIOF_PRETIMEOUT;
...
}

If a prior probe on a device or platform had the IRQ, a subsequent device
that does not have the IRQ will still inherit the flag across unbind/rebind
cycles or multiple instances.

Does this cause the newly added code here to erroneously write to
TIMER1_VAL_OFF and mistakenly enable the TIMER1 pretimeout interrupt, risking
an unhandled interrupt storm?

> +		writel(dev->clk_rate * (wdt_dev->timeout - wdt_dev->pretimeout),
> +		       dev->reg + TIMER1_VAL_OFF);
[Severity: Low]
Does this calculation incorrectly assume TIMER1 counts down at the
divided/fixed clock rate dev->clk_rate?

On Armada 370, drivers/watchdog/orion_wdt.c:armada370_wdt_clock_init() sets
WDT_A370_RATIO_MASK for the watchdog, but fails to set the corresponding
TIMER1_DIV:

static int armada370_wdt_clock_init(...)
{
    ...
    /* Setup watchdog input clock */
    atomic_io_modify(dev->reg + TIMER_CTRL,
            WDT_A370_RATIO_MASK(WDT_A370_RATIO_SHIFT),
            WDT_A370_RATIO_MASK(WDT_A370_RATIO_SHIFT));
    
    dev->clk_rate = clk_get_rate(dev->clk) / WDT_A370_RATIO;
    ...
}

Similarly on Armada 375, the initialization in armada375_wdt_clock_init()
sets WDT_AXP_FIXED_ENABLE_BIT but omits TIMER1_FIXED_ENABLE_BIT, unlike
Armada XP.

If a user boots Armada 370 or Armada 375 with a device tree that defines the
pretimeout interrupt, won't the pretimeout fire prematurely because TIMER1
counts down at the undivided/unfixed clock rate, while the initial counter
value is calculated using the slower dev->clk_rate?

>  
>  	/* Clear the watchdog expiration bit */
>  	atomic_io_modify(dev->reg + TIMER_A370_STATUS, WDT_A370_EXPIRED, 0);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261012-orion-wdt-suspend-v2-0-f13cf71443ba@iav.lv?part=1

Re: [PATCH v2 1/2] watchdog: orion_wdt: load the pretimeout counter on Armada 370/XP start

[Igor Velkov]

> - [Medium] watchdog: orion_wdt: global orion_wdt_info struct contamination
> - [Low] watchdog: orion_wdt: TIMER1 clock rate mismatch for pretimeout on Armada 370/375

Both are pre-existing. orion_wdt_info gets WDIOF_PRETIMEOUT in probe
since e07a4c79ca75, and armada370_start() already enabled TIMER1 on
that flag, so this patch does not widen it. On the TIMER1 rate: no
in-tree Armada 370 or 375 DT has the pretimeout interrupt, and XP/38x
set TIMER1_FIXED_ENABLE_BIT in armadaxp_wdt_clock_init(). I'll leave
both outside this series.

Igor