[PATCH 0/2] watchdog: w83627hf_wdt: Add NCT6126 support and improve handling of running watchdog

[PATCH 0/2] watchdog: w83627hf_wdt: Add NCT6126 support and improve handling of running watchdog

[Paul Louvel]

This series brings two small improvements to the w83627hf_wdt driver.

The first patch adds support for the Nuvoton NCT6126 Super I/O chip. The
NCT6126 is compatible with the existing driver but requires reading the
full 16-bit chip ID, as it shares its high-byte ID with the NCT6116.

The second patch improves the handling of watchdogs that are already
running when the driver probes. Instead of issuing a single keepalive
using the default timeout, it relies on the watchdog core's
WDOG_HW_RUNNING support to automatically maintain the watchdog until
userspace opens the device.

Signed-off-by: Paul Louvel <paul.louvel@bootlin.com>
---
Paul Louvel (2):
      watchdog: w83627hf_wdt: Add support for Nuvoton NCT6126
      watchdog: w83627hf_wdt: Use WDOG_HW_RUNNING for running chip on boot

 drivers/watchdog/w83627hf_wdt.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)
---
base-commit: 028ef9c96e96197026887c0f092424679298aae8
change-id: 20260705-w83627hf_wdt-nct6126d-9a016bf7c936

Best regards,
--  
Paul Louvel, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[PATCH 1/2] watchdog: w83627hf_wdt: Add support for Nuvoton NCT6126

[Paul Louvel]

Add support for the hardware watchdog integrated in the Nuvoton NCT6126
Super I/O chip. This device is used on a number of x86 single-board
computers and is compatible with the w83627hf_wdt driver.

Unlike most supported chips, the NCT6126 shares the same high-byte chip
ID as the NCT6116. Read the low byte of the chip ID as well to
distinguish between the two devices and identify the NCT6126 correctly.

Signed-off-by: Paul Louvel <paul.louvel@bootlin.com>
---
 drivers/watchdog/w83627hf_wdt.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/watchdog/w83627hf_wdt.c b/drivers/watchdog/w83627hf_wdt.c
index bc33b63c5a5d..9533baa45384 100644
--- a/drivers/watchdog/w83627hf_wdt.c
+++ b/drivers/watchdog/w83627hf_wdt.c
@@ -49,7 +49,7 @@ static int wdt_cfg_leave = 0xAA;/* key to lock configuration space */
 enum chips { w83627hf, w83627s, w83697hf, w83697ug, w83637hf, w83627thf,
 	     w83687thf, w83627ehf, w83627dhg, w83627uhg, w83667hg, w83627dhg_p,
 	     w83667hg_b, nct6775, nct6776, nct6779, nct6791, nct6792, nct6793,
-	     nct6795, nct6796, nct6102, nct6116 };
+	     nct6795, nct6796, nct6102, nct6116, nct6126 };
 
 static int timeout;			/* in seconds */
 module_param(timeout, int, 0);
@@ -95,6 +95,9 @@ MODULE_PARM_DESC(early_disable, "Disable watchdog at boot time (default=0)");
 #define NCT6776_ID		0xc3
 #define NCT6102_ID		0xc4
 #define NCT6116_ID		0xd2
+#define NCT6116_ID		0xd2	/* also NCT6126D */
+#define NCT6126_VER_A_LOW_ID	0x83	/* ... version A */
+#define NCT6126_VER_B_LOW_ID	0x84	/* ... version B */
 #define NCT6779_ID		0xc5
 #define NCT6791_ID		0xc8
 #define NCT6792_ID		0xc9
@@ -428,7 +431,12 @@ static int wdt_find(int addr)
 		cr_wdt_csr = NCT6102D_WDT_CSR;
 		break;
 	case NCT6116_ID:
-		ret = nct6116;
+		val = superio_inb(0x21);
+		if (val == NCT6126_VER_A_LOW_ID || val == NCT6126_VER_B_LOW_ID)
+			ret = nct6126;
+		else
+			ret = nct6116;
+
 		cr_wdt_timeout = NCT6102D_WDT_TIMEOUT;
 		cr_wdt_control = NCT6102D_WDT_CONTROL;
 		cr_wdt_csr = NCT6102D_WDT_CSR;
@@ -499,6 +507,7 @@ static int __init wdt_init(void)
 		"NCT6796",
 		"NCT6102",
 		"NCT6116",
+		"NCT6126"
 	};
 
 	/* Apply system-specific quirks */

-- 
2.55.0

Re: [PATCH 1/2] watchdog: w83627hf_wdt: Add support for Nuvoton NCT6126

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The newly added `nct6126` chip is missing from the `switch (chip)` statement in `w83627hf_init()`, causing critical hardware watchdog initialization (WDTO# output routing) to be skipped.
- [Low] The macro `NCT6116_ID` is redundantly defined twice in a row with the identical value.
--

commit 0f99926a9f6fc9f6f9e5246a88211f0c8ad46b5b
Author: Paul Louvel <paul.louvel@bootlin.com>

watchdog: w83627hf_wdt: Add support for Nuvoton NCT6126

Add support for the hardware watchdog integrated in the Nuvoton NCT6126
Super I/O chip. This distinguishes the NCT6126 from the NCT6116 by reading
the low byte of the chip ID.

> diff --git a/drivers/watchdog/w83627hf_wdt.c b/drivers/watchdog/w83627hf_wdt.c
> index bc33b63c5a5df..9533baa453841 100644
> --- a/drivers/watchdog/w83627hf_wdt.c
> +++ b/drivers/watchdog/w83627hf_wdt.c
> @@ -49,7 +49,7 @@ static int wdt_cfg_leave = 0xAA;/* key to lock configuration space */
>  enum chips { w83627hf, w83627s, w83697hf, w83697ug, w83637hf, w83627thf,
>  	     w83687thf, w83627ehf, w83627dhg, w83627uhg, w83667hg, w83627dhg_p,
>  	     w83667hg_b, nct6775, nct6776, nct6779, nct6791, nct6792, nct6793,
> -	     nct6795, nct6796, nct6102, nct6116 };
> +	     nct6795, nct6796, nct6102, nct6116, nct6126 };
>  

[Severity: High]
Does this code miss handling the new nct6126 chip in w83627hf_init()?

When w83627hf_init() initializes the watchdog device, it uses a switch
statement on the chip type. Because nct6126 is omitted from the switch cases
in w83627hf_init(), it falls through to the default case:

drivers/watchdog/w83627hf_wdt.c:w83627hf_init() {
        ...
        switch (chip) {
        ...
        case nct6116:
                ...
                t = superio_inb(cr_wdt_control);
                t |= 0x02;      /* enable the WDTO# output low pulse
                                 * to the KBRST# pin */
                superio_outb(cr_wdt_control, t);
                break;
        default:
                break;
        }
}

Without nct6126 being added alongside nct6116, the WDTO# output initialization
is skipped. Will this cause the watchdog timer to count down and expire
without successfully triggering a physical system reset?

[ ... ]

> @@ -95,6 +95,9 @@ MODULE_PARM_DESC(early_disable, "Disable watchdog at boot time (default=0)");
>  #define NCT6776_ID		0xc3
>  #define NCT6102_ID		0xc4
>  #define NCT6116_ID		0xd2
> +#define NCT6116_ID		0xd2	/* also NCT6126D */

[Severity: Low]
This isn't a bug, but NCT6116_ID is now defined twice with the exact same
value. Was the intention to just add a comment to the existing definition
instead of redefining it entirely?

> +#define NCT6126_VER_A_LOW_ID	0x83	/* ... version A */
> +#define NCT6126_VER_B_LOW_ID	0x84	/* ... version B */
>  #define NCT6779_ID		0xc5

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260706-w83627hf_wdt-nct6126d-v1-0-c5b0d51691c6@bootlin.com?part=1

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

[Paul Louvel]

If the watchdog is already running when the driver is initialized, the
driver currently refreshes it once using the default timeout of 60
seconds.

Since commit ee142889e32f ("watchdog: Introduce WDOG_HW_RUNNING flag"),
the watchdog core can keep an already running watchdog alive even when
the watchdog device is not open. This is more robust than issuing a
single manual keepalive during initialization.

Replace the manual watchdog refresh with the WDOG_HW_RUNNING flag and
let the watchdog core handle periodic keepalives until user space takes
over.

Signed-off-by: Paul Louvel <paul.louvel@bootlin.com>
---
 drivers/watchdog/w83627hf_wdt.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/watchdog/w83627hf_wdt.c b/drivers/watchdog/w83627hf_wdt.c
index 9533baa45384..067831caa964 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);
 		}
 	}
 

-- 
2.55.0

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

[sashiko-bot]

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 <paul.louvel@bootlin.com>

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/20260706-w83627hf_wdt-nct6126d-v1-0-c5b0d51691c6@bootlin.com?part=2

Re: [PATCH 0/2] watchdog: w83627hf_wdt: Add NCT6126 support and improve handling of running watchdog

[Paul Louvel]

Hi,

The first patch contains a small mistake.
Please discard this series and wait for V2, I will send it immediately.
Sorry for the inconvenience.

Paul.

On 7/6/26 12:14 PM, Paul Louvel wrote:
> This series brings two small improvements to the w83627hf_wdt driver.
>
> The first patch adds support for the Nuvoton NCT6126 Super I/O chip. The
> NCT6126 is compatible with the existing driver but requires reading the
> full 16-bit chip ID, as it shares its high-byte ID with the NCT6116.
>
> The second patch improves the handling of watchdogs that are already
> running when the driver probes. Instead of issuing a single keepalive
> using the default timeout, it relies on the watchdog core's
> WDOG_HW_RUNNING support to automatically maintain the watchdog until
> userspace opens the device.
>
> Signed-off-by: Paul Louvel <paul.louvel@bootlin.com>
> ---
> Paul Louvel (2):
>        watchdog: w83627hf_wdt: Add support for Nuvoton NCT6126
>        watchdog: w83627hf_wdt: Use WDOG_HW_RUNNING for running chip on boot
>
>   drivers/watchdog/w83627hf_wdt.c | 18 +++++++++++++-----
>   1 file changed, 13 insertions(+), 5 deletions(-)
> ---
> base-commit: 028ef9c96e96197026887c0f092424679298aae8
> change-id: 20260705-w83627hf_wdt-nct6126d-9a016bf7c936
>
> Best regards,
> --
> Paul Louvel, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
>
-- 
Paul Louvel, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com