2022-04-26 09:04:59

by Liu Xinpeng

[permalink] [raw]
Subject: [PATCH v3 0/4] Some impovements about watchdog

Changelog:
v1->v2 Update the commit messages
v2->v3 - Add the context about why using watchdog_timeout_invalid.
- Using SET_NOIRQ_SYSTEM_SLEEP_PM_OPS reduces redundant code for
iTCO watchdog.
v3->v4 - For patch 1, update commit message, rename WDAT_TIMEOUT_MIN
to WDAT_MIN_TIMEOUT, keeps consistent with WDAT_DEFAULT_TIMEOUT.
- For patch 4, iTCO_wdt_suspend_noirq and iTCO_wdt_resume_noirq
are possible unused, so keep "ifdef CONFIG_PM_SLEEP ... #endif".

Liu Xinpeng (4):
watchdog: wdat_wdg: Using the existed function to check parameter
timeout
watchdog: wdat_wdg: Stop watchdog when rebooting the system
watchdog: wdat_wdg: Stop watchdog when uninstalling module
watchdog: iTCO_wdg: Make code more clearly with macro definition

drivers/watchdog/iTCO_wdt.c | 12 +++---------
drivers/watchdog/wdat_wdt.c | 7 +++++--
2 files changed, 8 insertions(+), 11 deletions(-)

--
2.23.0


2022-04-26 10:29:08

by Liu Xinpeng

[permalink] [raw]
Subject: [PATCH v4 2/4] watchdog: wdat_wdg: Stop watchdog when rebooting the system

Executing reboot command several times on the machine "Dell
PowerEdge R740", UEFI security detection stopped machine
with the following prompt:

UEFI0082: The system was reset due to a timeout from the watchdog
timer. Check the System Event Log (SEL) or crash dumps from
Operating Sysstem to identify the source that triggered the
watchdog timer reset. Update the firmware or driver for the
identified device.

iDRAC has warning event: "The watchdog timer reset the system".

This patch fixes this issue by adding the reboot notifier.

Signed-off-by: Liu Xinpeng <[email protected]>
---
drivers/watchdog/wdat_wdt.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/watchdog/wdat_wdt.c b/drivers/watchdog/wdat_wdt.c
index 9db01d165310..0ef2b918364a 100644
--- a/drivers/watchdog/wdat_wdt.c
+++ b/drivers/watchdog/wdat_wdt.c
@@ -463,6 +463,7 @@ static int wdat_wdt_probe(struct platform_device *pdev)
return ret;

watchdog_set_nowayout(&wdat->wdd, nowayout);
+ watchdog_stop_on_reboot(&wdat->wdd);
return devm_watchdog_register_device(dev, &wdat->wdd);
}

--
2.23.0

2022-04-26 19:07:40

by Liu Xinpeng

[permalink] [raw]
Subject: [PATCH v4 1/4] watchdog: wdat_wdg: Using the existed function to check parameter timeout

Context: If max_hw_heartbeat_ms is provided, the configured maximum timeout
is not limited by it. The limit check in this driver therefore doesn't make
much sense. Similar, the watchdog core ensures that minimum timeout limits
are met if min_hw_heartbeat_ms is set. Using watchdog_timeout_invalid()
makes more sense because it takes this into account.

Signed-off-by: Liu Xinpeng <[email protected]>
Reviewed-by: Guenter Roeck <[email protected]>
Reviewed-by: Tzung-Bi Shih <[email protected]>
---
drivers/watchdog/wdat_wdt.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/watchdog/wdat_wdt.c b/drivers/watchdog/wdat_wdt.c
index 195c8c004b69..9db01d165310 100644
--- a/drivers/watchdog/wdat_wdt.c
+++ b/drivers/watchdog/wdat_wdt.c
@@ -55,6 +55,7 @@ MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default="
__MODULE_STRING(WATCHDOG_NOWAYOUT) ")");

#define WDAT_DEFAULT_TIMEOUT 30
+#define WDAT_MIN_TIMEOUT 1

static int timeout = WDAT_DEFAULT_TIMEOUT;
module_param(timeout, int, 0);
@@ -344,6 +345,7 @@ static int wdat_wdt_probe(struct platform_device *pdev)
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 = WDAT_MIN_TIMEOUT;
wdat->stopped_in_sleep = tbl->flags & ACPI_WDAT_STOPPED;
wdat->wdd.info = &wdat_wdt_info;
wdat->wdd.ops = &wdat_wdt_ops;
@@ -450,8 +452,7 @@ static int wdat_wdt_probe(struct platform_device *pdev)
* watchdog properly after it has opened the device. In some cases
* the BIOS default is too short and causes immediate reboot.
*/
- if (timeout * 1000 < wdat->wdd.min_hw_heartbeat_ms ||
- timeout * 1000 > wdat->wdd.max_hw_heartbeat_ms) {
+ if (watchdog_timeout_invalid(&wdat->wdd, timeout)) {
dev_warn(dev, "Invalid timeout %d given, using %d\n",
timeout, WDAT_DEFAULT_TIMEOUT);
timeout = WDAT_DEFAULT_TIMEOUT;
--
2.23.0