Re: [PATCH] watchdog: it8712f_wdt: remove redundant driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Critical] The commit silently removes support for reading the watchdog reset status (`WDIOF_CARDRESET`) and destroys the hardware status on initialization.
- [High] The alias redirecting `it8712f_wdt` to `it87_wdt` causes module loading to fail if legacy parameters ('margin', 'wdt_control_reg') are provided, preventing the watchdog from being serviced.
- [High] Removing `CONFIG_IT8712F_WDT` without providing a transitional fallback config will silently drop the built-in watchdog driver during kernel updates.
--

commit 7c9bf5495dff37192e527a7fb23eeefe02364661
Author: Ethan Nelson-Moore <enelsonmoore@gmail.com>

watchdog: it8712f_wdt: remove redundant driver

This commit removes the it8712f_wdt driver in favor of the it87_wdt driver
which also supports the IT8712 hardware. It introduces a module alias to
redirect requests for it8712f_wdt to it87_wdt.

> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index dc78729ba2a5..a84fbf9d52b8 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
[ ... ]
> -config IT8712F_WDT
> -	tristate "IT8712F (Smart Guardian) Watchdog Timer"
> -	depends on (X86 || COMPILE_TEST) && HAS_IOPORT
> -	help
> -	  This is the driver for the built-in watchdog timer on the IT8712F
> -	  Super I/0 chipset used on many motherboards.
> -
> -	  If the driver does not work, then make sure that the game port in
> -	  the BIOS is enabled.
> -
> -	  To compile this driver as a module, choose M here: the
> -	  module will be called it8712f_wdt.
> -

By completely removing CONFIG_IT8712F_WDT, will users updating their
kernels with make oldconfig silently lose watchdog support since there
is no transitional dummy config that selects IT87_WDT? 

If the hardware timer is already armed by the firmware, could this lack
of driver support lead to uncontrolled reboots?

> diff --git a/drivers/watchdog/it8712f_wdt.c b/drivers/watchdog/it8712f_wdt.c
> deleted file mode 100644
> index b776e6766c9d..000000000000
> --- a/drivers/watchdog/it8712f_wdt.c
> +++ /dev/null
[ ... ]
> -static int it8712f_wdt_get_status(void)
> -{
> -	if (superio_inb(WDT_CONTROL) & 0x01)
> -		return WDIOF_CARDRESET;
> -	else
> -		return 0;
> -}

The old driver reported WDIOF_CARDRESET by reading the WDT_CONTROL register.

The replacement it87_wdt driver does not appear to populate wdt_dev.bootstatus
or report WDIOF_CARDRESET in its capabilities. Will userspace applications
relying on this reset reason fail to detect watchdog reboots? 

Also, since it87_wdt_init() in the new driver unconditionally writes 0x00 to
WDTCTRL during probe, does this permanently clear the hardware reset status
bit before userspace can ever query it?

> diff --git a/drivers/watchdog/it87_wdt.c b/drivers/watchdog/it87_wdt.c
> index 1d9f8591f38d..9da5ec544c8a 100644
> --- a/drivers/watchdog/it87_wdt.c
> +++ b/drivers/watchdog/it87_wdt.c
[ ... ]
> @@ -419,6 +419,11 @@ static void __exit it87_wdt_exit(void)
>  module_init(it87_wdt_init);
>  module_exit(it87_wdt_exit);
>  
> +/*
> + * Load this driver when the separate IT8712F driver, which was removed, is
> + * requested
> + */
> +MODULE_ALIAS("it8712f_wdt");

If existing user configurations in /etc/modprobe.d/ pass legacy parameters
like margin or wdt_control_reg when requesting it8712f_wdt, will the
module load fail completely because it87_wdt only defines timeout, testmode,
and nowayout?

If modprobe aborts with an unknown parameter error and the BIOS previously
armed the watchdog, could the system unexpectedly hard-reset due to the OS
failing to load the driver to service it?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260505032145.66065-1-enelsonmoore@gmail.com?part=1