All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] watchdog: orion_wdt: stop the watchdog during system suspend
@ 2026-10-11 23:16 ` Igor Velkov
  0 siblings, 0 replies; 3+ messages in thread
From: Igor Velkov via B4 Relay @ 2026-10-11 23:16 UTC (permalink / raw)
  To: Wim Van Sebroeck, Guenter Roeck; +Cc: linux-watchdog, linux-kernel, Igor Velkov

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>
---
Tested on a Kobol Helios4 (Armada 388, v7.2.9). Unpatched, the board
resets 60 s into s2idle. With this patch:
- 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.
---
 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 5d56e93f0406..0e3d896fe930 100644
--- a/drivers/watchdog/orion_wdt.c
+++ b/drivers/watchdog/orion_wdt.c
@@ -668,6 +668,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,
@@ -675,6 +698,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),
 	},
 };
 

---
base-commit: 8b5a9f09037e3c372c4e9f2fbda85fcfbf5ea5f0
change-id: 20261012-orion-wdt-suspend-08f276aafa14

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



^ permalink raw reply related	[flat|nested] 3+ messages in thread

* [PATCH] watchdog: orion_wdt: stop the watchdog during system suspend
@ 2026-10-11 23:16 ` Igor Velkov
  0 siblings, 0 replies; 3+ messages in thread
From: Igor Velkov @ 2026-10-11 23:16 UTC (permalink / raw)
  To: Wim Van Sebroeck, Guenter Roeck; +Cc: linux-watchdog, linux-kernel, 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>
---
Tested on a Kobol Helios4 (Armada 388, v7.2.9). Unpatched, the board
resets 60 s into s2idle. With this patch:
- 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.
---
 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 5d56e93f0406..0e3d896fe930 100644
--- a/drivers/watchdog/orion_wdt.c
+++ b/drivers/watchdog/orion_wdt.c
@@ -668,6 +668,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,
@@ -675,6 +698,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),
 	},
 };
 

---
base-commit: 8b5a9f09037e3c372c4e9f2fbda85fcfbf5ea5f0
change-id: 20261012-orion-wdt-suspend-08f276aafa14

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


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] watchdog: orion_wdt: stop the watchdog during system suspend
  2026-10-11 23:16 ` Igor Velkov
  (?)
@ 2026-10-11 23:32 ` sashiko-bot
  -1 siblings, 0 replies; 3+ messages in thread
From: sashiko-bot @ 2026-10-11 23:32 UTC (permalink / raw)
  To: Igor Velkov; +Cc: linux-watchdog

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] watchdog: orion_wdt: uninitialized pretimeout timer on Armada 370 resume
- [Low] pm: dev_pm_ops structure wrapped with pm_sleep_ptr() instead of pm_ptr()
--

commit 56ff9bfe90545f6af378431628185580afe76771
Author: Igor Velkov <iav@iav.lv>

watchdog: orion_wdt: stop the watchdog during system suspend

The watchdog timer keeps counting across system suspend while the
userspace that pings it is frozen, so it expires and resets the system.
Stop the timer on suspend and restart it on resume when it is active or
running in hardware.

> diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c
> index 5d56e93f04064..0e3d896fe930d 100644
> --- a/drivers/watchdog/orion_wdt.c
> +++ b/drivers/watchdog/orion_wdt.c

[ ... ]

> @@ -668,6 +668,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);

[Severity: High]
Can this cause the pretimeout timer to fire prematurely or immediately upon
system resume?

When orion_wdt_resume() calls orion_wdt_start(), it eventually invokes
armada370_start() on Armada 370/XP:

drivers/watchdog/orion_wdt.c:armada370_start() {
    ...
	/* Enable watchdog timer */
	reg = dev->data->wdt_enable_bit;
	if (dev->wdt.info->options & WDIOF_PRETIMEOUT)
		reg |= TIMER1_ENABLE_BIT;
	atomic_io_modify(dev->reg + TIMER_CTRL, reg, reg);
    ...
}

The pretimeout timer (TIMER1) is enabled here without restoring or reloading its
duration (TIMER1_VAL_OFF). If context was lost or the timer resumes from its
residual value before suspend, could this trigger a kernel panic depending on
the pretimeout governor?

> +
> +	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,
> @@ -675,6 +698,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),

[Severity: Low]
Does this violate the PM subsystem guidelines by using pm_sleep_ptr() instead
of pm_ptr() for the dev_pm_ops structure pointer?

>  	},
>  };

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-10-11 23:32 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-11 23:16 [PATCH] watchdog: orion_wdt: stop the watchdog during system suspend Igor Velkov via B4 Relay
2026-10-11 23:16 ` Igor Velkov
2026-10-11 23:32 ` sashiko-bot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.