Re: [PATCH v4 4/6] watchdog: w83627hf_wdt: Move register offsets into driver data

[Paul Louvel]

On Mon Oct 19, 2026 at 5:23 PM CEST, Tzung-Bi Shih wrote:
> On Tue, Oct 13, 2026 at 03:58:48PM +0200, Paul Louvel wrote:
>> @@ -42,9 +42,6 @@
>>  #define WATCHDOG_TIMEOUT 60		/* 60 sec default timeout */
>>  
>>  static int wdt_io;
>> -static int cr_wdt_timeout;	/* WDT timeout register */
>> -static int cr_wdt_control;	/* WDT control register */
>> -static int cr_wdt_csr;		/* WDT control & status register */
> ...
>> +struct w83627hf_reg {
>> +	int control;
>> +	int timeout;
>> +	int csr;
>> +};
>> +
> ...
>> +static const struct w83627hf_reg w83627hf_default_regs = {
>> +	.control = W83627HF_WDT_CONTROL,
>> +	.timeout = W83627HF_WDT_TIMEOUT,
>> +	.csr = W836X7HF_WDT_CSR
>> +};
>> +
>> +static const struct w83627hf_reg w83627hf_nct61xx_regs = {
>> +	.timeout = NCT6102D_WDT_TIMEOUT,
>> +	.control = NCT6102D_WDT_CONTROL,
>> +	.csr = NCT6102D_WDT_CSR
>> +};
>> +
>> +static const struct w83627hf_reg w83627hf_w83697xx_regs = {
>> +	.timeout = W83697HF_WDT_TIMEOUT,
>> +	.control = W83697HF_WDT_CONTROL,
>> +	.csr = W836X7HF_WDT_CSR
>> +};
>
> Nit: the order is inconsistent.  It would be better to stick with
> the order timeout -> control -> csr.
>
>> @@ -489,6 +499,21 @@ static int wdt_probe(struct platform_device *pdev)
>>  	wdd->min_timeout = 1;
>>  	wdd->max_timeout = 255;
>>  
>> +	switch (chip) {
>> +	case nct6102:
>> +	case nct6116:
>> +	case nct6126:
>> +		data->reg = &w83627hf_nct61xx_regs;
>> +		break;
>> +	case w83697hf:
>> +	case w83697ug:
>> +		data->reg = &w83627hf_w83697xx_regs;
>> +		break;
>> +	default:
>> +		data->reg = &w83627hf_default_regs;
>> +		break;
>> +	};
>          ^
> Please remove the extra ';'.

ACK.

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