Re: [PATCH v3 3/6] watchdog: w83627hf_wdt: Move register offsets into driver data

[Tzung-Bi Shih <tzungbi@kernel.org>]

On Sun, Oct 04, 2026 at 02:12:51PM +0200, Paul Louvel wrote:
> @@ -491,6 +481,21 @@ static int wdt_probe(struct platform_device *pdev)
>  	wdd->min_timeout = 1;
>  	wdd->max_timeout = 255;
>  
> +	data->reg.timeout = W83627HF_WDT_TIMEOUT;
> +	data->reg.control = W83627HF_WDT_CONTROL;
> +	data->reg.csr = W836X7HF_WDT_CSR;
> +
> +	if (chip == nct6102 || chip == nct6116 || chip == nct6126) {
> +		data->reg.timeout = NCT6102D_WDT_TIMEOUT;
> +		data->reg.control = NCT6102D_WDT_CONTROL;
> +		data->reg.csr = NCT6102D_WDT_CSR;
> +	}
> +
> +	if (chip == w83697hf || chip == w83697ug) {
> +		data->reg.timeout = W83697HF_WDT_TIMEOUT;
> +		data->reg.control = W83697HF_WDT_CONTROL;
> +	}
> +

A switch statement would be cleaner here.

Also, there are only 3 distinct register configurations: default, nct61xx,
and w83697xx.  Rather than storing and copying mutable integer fields in
every driver data instance, consider defining static const register tables
and holding a pointer to them, e.g.:

    struct w83627hf_regs {
	u8 timeout;
	u8 control;
	u8 csr;
    };

    static const struct w83627hf_regs w83627hf_default_regs = {
	.timeout = W83627HF_WDT_TIMEOUT,
	.control = W83627HF_WDT_CONTROL,
	.csr = W836X7HF_WDT_CSR,
    };

    ...

    struct w83627hf_data {
	...
	const struct w83627hf_regs *regs;
    };