diff mbox series

[v8,1/3] watchdog: wdat_wdt: Using the existing function to check parameter timeout

Message ID 1650984810-6247-2-git-send-email-liuxp11@chinatelecom.cn (mailing list archive)
State Accepted
Headers show
Series Some impovements about acpi hardware watchdog | expand

Commit Message

Liu Xinpeng April 26, 2022, 2:53 p.m. UTC
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>
---
 drivers/watchdog/wdat_wdt.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Guenter Roeck April 26, 2022, 8:15 p.m. UTC | #1
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
>
diff mbox series

Patch

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;