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

[PATCH v2 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>
---
Changes in v2:
- Small mistake in the first patch. The NCT6116_ID macro is redefined.
  Fix it.

To: Wim Van Sebroeck <wim@linux-watchdog.org>
To: Guenter Roeck <linux@roeck-us.net>
Cc: linux-watchdog@vger.kernel.org
Cc: linux-kernel@vger.kernel.org

---
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 | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 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 v2 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 | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/watchdog/w83627hf_wdt.c b/drivers/watchdog/w83627hf_wdt.c
index bc33b63c5a5d..1f649fda8a81 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);
@@ -94,7 +94,9 @@ MODULE_PARM_DESC(early_disable, "Disable watchdog at boot time (default=0)");
 #define NCT6775_ID		0xb4
 #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 +430,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 +506,7 @@ static int __init wdt_init(void)
 		"NCT6796",
 		"NCT6102",
 		"NCT6116",
+		"NCT6126"
 	};
 
 	/* Apply system-specific quirks */

-- 
2.55.0

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing hardware reset initialization for the NCT6126 watchdog chip.
--

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

watchdog: w83627hf_wdt: Add support for Nuvoton NCT6126

This commit adds support for the Nuvoton NCT6126 hardware watchdog to the
w83627hf_wdt driver. It differentiates the NCT6126 from the NCT6116 by
checking the low byte of the chip ID.

> diff --git a/drivers/watchdog/w83627hf_wdt.c b/drivers/watchdog/w83627hf_wdt.c
> index bc33b63c5a5df..1f649fda8a813 100644
> --- a/drivers/watchdog/w83627hf_wdt.c
> +++ b/drivers/watchdog/w83627hf_wdt.c

[ ... ]

> @@ -428,7 +430,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;

[Severity: High]
Does this code miss updating the switch statement in w83627hf_init() to
include nct6126?

Looking at w83627hf_init(), the other NCT chips are explicitly handled to
enable the WDTO# output pulse:

drivers/watchdog/w83627hf_wdt.c:w83627hf_init() {
	...
	case nct6102:
	case nct6116:
		/*
		 * These chips have a fixed WDTO# output pin (W83627UHG),
		 * or support more than one WDTO# output pin.
		 * Don't touch its configuration, and hope the BIOS
		 * does the right thing.
		 */
		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;
	...
}

If nct6126 isn't added to this switch statement, it will fall through to the
default case.  Will this cause the chip to skip enabling the reset logic,
resulting in the watchdog timing out without physically resetting the system?

> +		else
> +			ret = nct6116;
> +
>  		cr_wdt_timeout = NCT6102D_WDT_TIMEOUT;
>  		cr_wdt_control = NCT6102D_WDT_CONTROL;
>  		cr_wdt_csr = NCT6102D_WDT_CSR;

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

[PATCH v2 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 1f649fda8a81..5348c0f79de8 100644
--- a/drivers/watchdog/w83627hf_wdt.c
+++ b/drivers/watchdog/w83627hf_wdt.c
@@ -240,9 +240,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 v2 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 manual watchdog timeout reset during driver initialization while continuing to unconditionally force the hardware into 'seconds' mode causes a severe timeout truncation. If the BIOS left the watchdog running with a 'minutes' timebase (e.g., 5 minutes), the raw hardware register value (e.g., 5) is preserved but its unit is instantly reinterpreted as seconds, leaving a window of only 5 seconds before the watchdog fires.
--

commit 41d13e5a4078e3d18e184d179b1d408461a148bd
Author: Paul Louvel <paul.louvel@bootlin.com>

watchdog: w83627hf_wdt: Use WDOG_HW_RUNNING for running chip on boot

This commit replaces the manual watchdog refresh during initialization
with the WDOG_HW_RUNNING flag. It delegates keepalives to the watchdog core
when the device is already running on boot.

> diff --git a/drivers/watchdog/w83627hf_wdt.c b/drivers/watchdog/w83627hf_wdt.c
> index 1f649fda8a813..5348c0f79de8f 100644
> --- a/drivers/watchdog/w83627hf_wdt.c
> +++ b/drivers/watchdog/w83627hf_wdt.c
> @@ -240,9 +240,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 reset cause a timeout truncation issue?

Immediately after this block, w83627hf_init() forces the hardware timebase 
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 previously left the watchdog running with a 'minutes' timebase 
(for example, 5 minutes), the raw hardware register value (5) is preserved.

However, because the timebase is instantly reinterpreted as seconds, the
system is left with a window of only 5 seconds before the watchdog fires.

This can lead to spurious reboots during the OS boot phase before the watchdog 
core or user space can issue the next ping.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260706-w83627hf_wdt-nct6126d-v2-0-77af74af88ae@bootlin.com?part=2