| Message ID | 1650984810-6247-1-git-send-email-liuxp11@chinatelecom.cn |
|---|---|
| Headers | show |
| Series | Some impovements about acpi hardware watchdog | expand |
On Tue, Apr 26, 2022 at 10:53:28PM +0800, Liu Xinpeng wrote: > 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 <liuxp11@chinatelecom.cn> Reviewed-by: Guenter Roeck <linux@roeck-us.net> > --- > drivers/watchdog/wdat_wdt.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/watchdog/wdat_wdt.c b/drivers/watchdog/wdat_wdt.c > index 195c8c004b69..df0865a61a70 100644 > --- a/drivers/watchdog/wdat_wdt.c > +++ b/drivers/watchdog/wdat_wdt.c > @@ -344,6 +344,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 = 1; > wdat->stopped_in_sleep = tbl->flags & ACPI_WDAT_STOPPED; > wdat->wdd.info = &wdat_wdt_info; > wdat->wdd.ops = &wdat_wdt_ops; > @@ -450,8 +451,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 >
On Tue, Apr 26, 2022 at 10:53:30PM +0800, Liu Xinpeng wrote: > Test shows that wachdog still reboots machine after the module > is removed. Use watchdog_stop_on_unregister to stop the watchdog > on removing. > > Signed-off-by: Liu Xinpeng <liuxp11@chinatelecom.cn> Reviewed-by: Guenter Roeck <linux@roeck-us.net> > --- > 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 6f36a653767b..e6f95e99156d 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) > > watchdog_set_nowayout(&wdat->wdd, nowayout); > watchdog_stop_on_reboot(&wdat->wdd); > + watchdog_stop_on_unregister(&wdat->wdd); > return devm_watchdog_register_device(dev, &wdat->wdd); > } > > -- > 2.23.0 >
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". v4->v5 Split iTCO_wdt's commit from this patchset. v5->v6 For patch 1, drop "Context: " and to change "existed" in the subject to "existing". v6->v7 Rename "watchdog: wdat_wdg" in subject to "watchdog: wdat_wdt". v7->v8 Set min_timeout to 1 without using a define. Liu Xinpeng (3): watchdog: wdat_wdt: Using the existing function to check parameter timeout watchdog: wdat_wdt: Stop watchdog when rebooting the system watchdog: wdat_wdt: Stop watchdog when uninstalling module drivers/watchdog/wdat_wdt.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)