Re: [PATCH 2/2] watchdog: w83627hf_wdt: Use WDOG_HW_RUNNING for running chip on boot

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Removing the explicit watchdog timeout reset while retaining an unconditional switch to a seconds-based timebase severely truncates the remaining watchdog timeout, causing spurious system reboots.
--

commit 8d12d1780388b6b4befd25bff1a9eac5e3d4130d
Author: Paul Louvel <[email protected]>

watchdog: w83627hf_wdt: Use WDOG_HW_RUNNING for running chip on boot

This commit removes the manual watchdog timeout reset during initialization
for an already running watchdog. It instead sets the WDOG_HW_RUNNING flag to
let the watchdog core handle periodic keepalives.

> diff --git a/drivers/watchdog/w83627hf_wdt.c b/drivers/watchdog/w83627hf_wdt.c
> index 9533baa453841..067831caa9648 100644
> --- a/drivers/watchdog/w83627hf_wdt.c
> +++ b/drivers/watchdog/w83627hf_wdt.c
> @@ -241,9 +241,8 @@ static int w83627hf_init(struct watchdog_device *wdog, enum chips chip)
>  			pr_warn("Stopping previously enabled watchdog until userland kicks in\n");
>  			superio_outb(cr_wdt_timeout, 0);
>  		} else {
> -			pr_info("Watchdog already running. Resetting timeout to %d sec\n",
> -				wdog->timeout);
> -			superio_outb(cr_wdt_timeout, wdog->timeout);
> +			pr_info("Watchdog already running.\n");
> +			set_bit(WDOG_HW_RUNNING, &wdog->status);

[Severity: High]
Does removing this timeout refresh introduce a risk of spurious reboots
if the watchdog was originally started in minutes mode by the BIOS?

Just after this block in w83627hf_init(), the driver unconditionally clears
bit 3 of cr_wdt_control to force the hardware timer into seconds mode:

	/* set second mode & disable keyboard turning off watchdog */
	t = superio_inb(cr_wdt_control) & ~0x0C;
	superio_outb(cr_wdt_control, t);

If the BIOS originally configured the watchdog in minutes and the unscaled
counter value is small (e.g., 2 for 2 minutes), skipping the timeout reset
here means that small value remains in the register.

When the timebase is immediately switched to seconds, the remaining timeout
will drastically shrink from 2 minutes to 2 seconds.

Could this very short timeout cause the hardware timer to expire before the
watchdog core's ping worker has a chance to execute during a heavy boot
load?

Additionally, if CONFIG_WATCHDOG_HANDLE_BOOT_ENABLED is disabled or 
overridden by a module parameter, wouldn't the watchdog core deliberately
avoid pinging the hardware, leading to a guaranteed spurious reboot in a 
few seconds?

>  		}
>  	}
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2