On 4/21/22 07:22, Liu Xinpeng wrote:
The timeout should be invalid when it is out of the hardware
timeout range.
ACPI watchdog: Using watchdog_timeout_invalid to check parameter
timeout invalid
Signed-off-by: Liu Xinpeng <liuxp11@xxxxxxxxxxxxxxx>
---
drivers/watchdog/wdat_wdt.c | 3 +--
include/linux/watchdog.h | 17 ++++++++++++-----
2 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/drivers/watchdog/wdat_wdt.c b/drivers/watchdog/wdat_wdt.c
index 195c8c004b69..d166d33ce7ae 100644
--- a/drivers/watchdog/wdat_wdt.c
+++ b/drivers/watchdog/wdat_wdt.c
@@ -450,8 +450,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;
diff --git a/include/linux/watchdog.h b/include/linux/watchdog.h
index 99660197a36c..e82daeef0b26 100644
--- a/include/linux/watchdog.h
+++ b/include/linux/watchdog.h
@@ -167,6 +167,15 @@ static inline void watchdog_stop_ping_on_suspend(struct watchdog_device *wdd)
/* Use the following function to check if a timeout value is invalid */
static inline bool watchdog_timeout_invalid(struct watchdog_device *wdd, unsigned int t)
{
+ /*
+ * If a maximum/minimum hardware timeout is configured,
+ * the timeout is invalid when it is out of the range.
+ */
+ if (wdd->max_hw_heartbeat_ms)
+ return t * 1000 > wdd->max_hw_heartbeat_ms;
+ if (wdd->min_hw_heartbeat_ms)
+ return t * 1000 < wdd->min_hw_heartbeat_ms;
+
I have no idea what problem you are trying to solve, but this is
completely wrong: It defeats the purpose of having separate minimum
and maximum HW timeouts and configured timeout values. The watchdog
core takes care of those situations.
NACK.
Guenter
/*
* The timeout is invalid if
* - the requested value is larger than UINT_MAX / 1000
@@ -174,13 +183,11 @@ static inline bool watchdog_timeout_invalid(struct watchdog_device *wdd, unsigne
* or
* - the requested value is smaller than the configured minimum timeout,
* or
- * - a maximum hardware timeout is not configured, a maximum timeout
- * is configured, and the requested value is larger than the
- * configured maximum timeout.
+ * - maximum timeout is configured, and the requested value is larger than
+ * the configured maximum timeout.
*/
return t > UINT_MAX / 1000 || t < wdd->min_timeout ||
- (!wdd->max_hw_heartbeat_ms && wdd->max_timeout &&
- t > wdd->max_timeout);
+ (wdd->max_timeout && t > wdd->max_timeout);
}
/* Use the following function to check if a pretimeout value is invalid */
