archive mirror
 help / color / mirror / Atom feed
* [PATCH v3] watchdog: wdat_wdt: Set the min and max timeout values properly
@ 2022-08-23 13:47 Jean Delvare
  0 siblings, 0 replies; only message in thread
From: Jean Delvare @ 2022-08-23 13:47 UTC (permalink / raw)
  To: linux-watchdog, LKML
  Cc: Mika Westerberg, Guenter Roeck, Wim Van Sebroeck,
	Rafael J. Wysocki, Liu Xinpeng

The wdat_wdt driver is misusing the min_hw_heartbeat_ms field. This
field should only be used when the hardware watchdog device should not
be pinged more frequently than a specific period. The ACPI WDAT
"Minimum Count" field, on the other hand, specifies the minimum
timeout value that can be set. This corresponds to the min_timeout
field in Linux's watchdog infrastructure.

Setting min_hw_heartbeat_ms instead can cause pings to the hardware
to be delayed when there is no reason for that, eventually leading to
unexpected firing of the watchdog timer (and thus unexpected reboot).

Since commit 6d72c7ac9fbe ("watchdog: wdat_wdt: Using the existing
function to check parameter timeout"), min_timeout is being set too,
but to the arbitrary value of 1 second, which doesn't make sense and
allows setting timeout values lower that the ACPI WDAT "Minimum
Count" field.

I'm also changing max_hw_heartbeat_ms to max_timeout for symmetry,
although the use of this one isn't fundamentally wrong, but there is
also no reason to enable the software-driven ping mechanism for the
wdat_wdt driver.

Signed-off-by: Jean Delvare <>
Fixes: 058dfc767008 ("ACPI / watchdog: Add support for WDAT hardware watchdog")
Fixes: 6d72c7ac9fbe ("watchdog: wdat_wdt: Using the existing function to check parameter timeout")
Reviewed-by: Mika Westerberg <>
Reviewed-by: Guenter Roeck <>
Cc: Wim Van Sebroeck <>
Cc: Rafael J. Wysocki <>
Cc: Liu Xinpeng <>
Changes since v2:
 * Rebased on top of v5.19.
Changes since v1:
 * Fix a stupid typo which broke the build.

Mika, Guenter, I chose to preserve your respective Reviewed-by tags, as
the end result is essentially similar to the previous iteration of the
patch. Feel free to complain if you disagree.

 drivers/watchdog/wdat_wdt.c |    5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

--- linux-5.19.orig/drivers/watchdog/wdat_wdt.c	2022-07-31 23:03:01.000000000 +0200
+++ linux-5.19/drivers/watchdog/wdat_wdt.c	2022-08-23 15:01:12.106398116 +0200
@@ -342,9 +342,8 @@ static int wdat_wdt_probe(struct platfor
 		return -EINVAL;
 	wdat->period = tbl->timer_period;
-	wdat->wdd.min_hw_heartbeat_ms = wdat->period * tbl->min_count;
-	wdat->wdd.max_hw_heartbeat_ms = wdat->period * tbl->max_count;
-	wdat->wdd.min_timeout = 1;
+	wdat->wdd.min_timeout = DIV_ROUND_UP(wdat->period * tbl->min_count, 1000);
+	wdat->wdd.max_timeout = wdat->period * tbl->max_count / 1000;
 	wdat->stopped_in_sleep = tbl->flags & ACPI_WDAT_STOPPED;
 	wdat-> = &wdat_wdt_info;
 	wdat->wdd.ops = &wdat_wdt_ops;

Jean Delvare
SUSE L3 Support

^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2022-08-23 17:19 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-23 13:47 [PATCH v3] watchdog: wdat_wdt: Set the min and max timeout values properly Jean Delvare

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).