| Message ID | 1650957029-910-2-git-send-email-liuxp11@chinatelecom.cn (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Some impovements about watchdog | expand |
On 4/26/22 00:10, Liu Xinpeng wrote: > Context: If max_hw_heartbeat_ms is provided, the configured maximum timeout Drop "Context:". Also, in the subject, it should be "existing". > 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 <liuxp11@chinatelecom.cn> > Reviewed-by: Guenter Roeck <linux@roeck-us.net> > Reviewed-by: Tzung-Bi Shih <tzungbi@kernel.org> And you really should not make up Reviewed-by: tags. This is completely inappropriate. Guenter > --- > 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;
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;