Re: [PATCH v5 3/5] watchdog: w83627hf_wdt: Move register offsets into driver data

[Tzung-Bi Shih]

On Tue, Oct 20, 2026 at 10:11:56AM +0200, Paul Louvel wrote:
> Register offsets for the watchdog timer, control, and status registers
> differ across Super I/O chip variants. Store them in the driver data
> instead of global variables.
> 
> Signed-off-by: Paul Louvel <paul.louvel@xxxxxxxxxxx>

With a nit fixed,

Reviewed-by: Tzung-Bi Shih <tzungbi@xxxxxxxxxx>

>  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 */
>  static int wdt_cfg_enter = 0x87;/* key to unlock configuration space */
>  static int wdt_cfg_leave = 0xAA;/* key to lock configuration space */
...
> +struct w83627hf_reg {
> +	int control;
> +	int timeout;
> +	int csr;
> +};
...
> > +static const struct w83627hf_reg w83627hf_default_regs = {
> +	.timeout = W83627HF_WDT_TIMEOUT,
> +	.control = W83627HF_WDT_CONTROL,
> +	.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
> +};

To be consistent and neat, I'd expect the order in the declaration of
struct w83627hf_reg to also be: timeout -> control -> csr.