| Message ID | 20200212145941.32914-4-mika.westerberg@linux.intel.com (mailing list archive) |
|---|---|
| State | Mainlined, archived |
| Headers | show |
| Series | ACPI / watchdog: Fix two reported issues | expand |
On Wed, 12 Feb 2020 17:59:41 +0300, Mika Westerberg wrote: > If the BIOS default timeout for the watchdog is too small userspace may > not have enough time to configure new timeout after opening the device > before the system is already reset. For this reason program default > timeout of 30 seconds in the driver probe and allow userspace to change > this from command line or through module parameter (wdat_wdt.timeout). > > Reported-by: Jean Delvare <jdelvare@suse.de> > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> > --- > drivers/watchdog/wdat_wdt.c | 23 +++++++++++++++++++++++ > 1 file changed, 23 insertions(+) > > diff --git a/drivers/watchdog/wdat_wdt.c b/drivers/watchdog/wdat_wdt.c > index e1b1fcfc02af..3065dd670a18 100644 > --- a/drivers/watchdog/wdat_wdt.c > +++ b/drivers/watchdog/wdat_wdt.c > @@ -54,6 +54,13 @@ module_param(nowayout, bool, 0); > MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default=" > __MODULE_STRING(WATCHDOG_NOWAYOUT) ")"); > > +#define WDAT_DEFAULT_TIMEOUT 30 > + > +static int timeout = WDAT_DEFAULT_TIMEOUT; > +module_param(timeout, int, 0); > +MODULE_PARM_DESC(timeout, "Watchdog timeout in seconds (default=" > + __MODULE_STRING(WDAT_DEFAULT_TIMEOUT) ")"); > + > static int wdat_wdt_read(struct wdat_wdt *wdat, > const struct wdat_instruction *instr, u32 *value) > { > @@ -438,6 +445,22 @@ static int wdat_wdt_probe(struct platform_device *pdev) > > platform_set_drvdata(pdev, wdat); > > + /* > + * Set initial timeout so that userspace has time to configure the > + * 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) { > + dev_warn(dev, "Invalid timeout %d given, using %d\n", > + timeout, WDAT_DEFAULT_TIMEOUT); > + timeout = WDAT_DEFAULT_TIMEOUT; > + } > + > + ret = wdat_wdt_set_timeout(&wdat->wdd, timeout); > + if (ret) > + return ret; > + > watchdog_set_nowayout(&wdat->wdd, nowayout); > return devm_watchdog_register_device(dev, &wdat->wdd); > } Reviewed-by: Jean Delvare <jdelvare@suse.de> Thanks Mika!
diff --git a/drivers/watchdog/wdat_wdt.c b/drivers/watchdog/wdat_wdt.c index e1b1fcfc02af..3065dd670a18 100644 --- a/drivers/watchdog/wdat_wdt.c +++ b/drivers/watchdog/wdat_wdt.c @@ -54,6 +54,13 @@ module_param(nowayout, bool, 0); MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default=" __MODULE_STRING(WATCHDOG_NOWAYOUT) ")"); +#define WDAT_DEFAULT_TIMEOUT 30 + +static int timeout = WDAT_DEFAULT_TIMEOUT; +module_param(timeout, int, 0); +MODULE_PARM_DESC(timeout, "Watchdog timeout in seconds (default=" + __MODULE_STRING(WDAT_DEFAULT_TIMEOUT) ")"); + static int wdat_wdt_read(struct wdat_wdt *wdat, const struct wdat_instruction *instr, u32 *value) { @@ -438,6 +445,22 @@ static int wdat_wdt_probe(struct platform_device *pdev) platform_set_drvdata(pdev, wdat); + /* + * Set initial timeout so that userspace has time to configure the + * 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) { + dev_warn(dev, "Invalid timeout %d given, using %d\n", + timeout, WDAT_DEFAULT_TIMEOUT); + timeout = WDAT_DEFAULT_TIMEOUT; + } + + ret = wdat_wdt_set_timeout(&wdat->wdd, timeout); + if (ret) + return ret; + watchdog_set_nowayout(&wdat->wdd, nowayout); return devm_watchdog_register_device(dev, &wdat->wdd); }
If the BIOS default timeout for the watchdog is too small userspace may not have enough time to configure new timeout after opening the device before the system is already reset. For this reason program default timeout of 30 seconds in the driver probe and allow userspace to change this from command line or through module parameter (wdat_wdt.timeout). Reported-by: Jean Delvare <jdelvare@suse.de> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> --- drivers/watchdog/wdat_wdt.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+)