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

["Paul Louvel" <paul.louvel@bootlin.com>]

On Mon Jul 6, 2026 at 4:14 PM CEST, Guenter Roeck wrote:
> On 7/6/26 03:29, Paul Louvel wrote:
>> 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.
>> 
>
> Doesn't that (and the code below) mean that the driver already supports
> the chip, only it identifies it as NCT6116 instead of NCT6126 ?

Yes, because they have the same high-byte chip ID.

>
> This means that the commit message is misleading: It should say that
> the chip is already supported but misidentified, and that the added
> code helps to correctly identify the chip.

Indeed. My bad !

>
> Guenter
>
>> Signed-off-by: Paul Louvel <paul.louvel@bootlin.com>
>> ---
>>   drivers/watchdog/w83627hf_wdt.c | 15 ++++++++++++---
>>   1 file changed, 12 insertions(+), 3 deletions(-)
>> 
>> diff --git a/drivers/watchdog/w83627hf_wdt.c b/drivers/watchdog/w83627hf_wdt.c
>> index bc33b63c5a5d..a6dfa9d01702 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
>> @@ -217,6 +219,7 @@ static int w83627hf_init(struct watchdog_device *wdog, enum chips chip)
>>   	case nct6796:
>>   	case nct6102:
>>   	case nct6116:
>> +	case nct6126:
>>   		/*
>>   		 * These chips have a fixed WDTO# output pin (W83627UHG),
>>   		 * or support more than one WDTO# output pin.
>> @@ -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 */
>> 

Paul.




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