Re: [PATCH v3 4/6] watchdog: w83627hf_wdt: Store Super I/O configurations in platform data

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

On Tue Oct 6, 2026 at 4:25 PM CEST, Tzung-Bi Shih wrote:
> On Sun, Oct 04, 2026 at 02:12:52PM +0200, Paul Louvel wrote:
>>  static int wdt_probe(struct platform_device *pdev)
>>  {
>>  	const struct platform_device_id *id = platform_get_device_id(pdev);
>> +	const struct wdt_pdata *pdata = pdev->dev.platform_data;
>
> Use dev_get_platdata().
>
>> @@ -522,12 +533,16 @@ static int wdt_probe(struct platform_device *pdev)
>>   */
>>  static int __init wdt_use_alt_key(const struct dmi_system_id *d)
>>  {
>> -	wdt_cfg_enter = 0x88;
>> -	wdt_cfg_leave = 0xBB;
>> +	struct wdt_pdata *pdata = d->driver_data;
>> +
>> +	pdata->siocfg_enter = 0x88;
>> +	pdata->siocfg_leave = 0xBB;
>>  
>>  	return 0;
>>  }
>>  
>> +static struct wdt_pdata pdata;
>> +
>>  static const struct dmi_system_id wdt_dmi_table[] __initconst = {
>>  	{
>>  		.matches = {
>> @@ -537,6 +552,7 @@ static const struct dmi_system_id wdt_dmi_table[] __initconst = {
>>  			DMI_EXACT_MATCH(DMI_BOARD_NAME, "SHARKBAY"),
>>  		},
>>  		.callback = wdt_use_alt_key,
>> +		.driver_data = &pdata,
>>  	},
>>  	{}
>>  };
>> @@ -581,27 +597,33 @@ static struct platform_device *wdt_pdev;
>>  
>>  static int __init wdt_init(void)
>>  {
>> +	int sioaddr;
>>  	int ret;
>>  	int chip;
>>  
>> +	pdata.siocfg_enter = 0x87;
>> +	pdata.siocfg_leave = 0xAA;
>> +
>>  	/* Apply system-specific quirks */
>>  	dmi_check_system(wdt_dmi_table);
>>  
>> -	wdt_io = SIO_REG_CONF_ADDR0;
>> -	chip = wdt_find(SIO_REG_CONF_ADDR0);
>> +	sioaddr = SIO_REG_CONF_ADDR0;
>> +	chip = wdt_find(sioaddr, pdata.siocfg_enter, pdata.siocfg_leave);
>>  	if (chip < 0) {
>> -		wdt_io = SIO_REG_CONF_ADDR1;
>> -		chip = wdt_find(SIO_REG_CONF_ADDR1);
>> +		sioaddr = SIO_REG_CONF_ADDR1;
>> +		chip = wdt_find(sioaddr, pdata.siocfg_enter, pdata.siocfg_leave);
>>  		if (chip < 0)
>>  			return chip;
>>  	}
>>  
>> +	pdata.sioaddr = sioaddr;
>> +
>
> `pdata` is still global.  Given that there are only a few settings, maybe
> we can use the same method I proposed in [3/6] patch, and use
> `dmi_first_match()` to avoid the global variable.  E.g.:
>
>     struct w83627hf_sio_pdata {
>         u16 sioaddr;
>         u8 siocfg_enter;
>         u8 siocfg_leave;
>     };
>
>     static const struct w83627hf_sio_pdata default_pdata = {
>         .siocfg_enter = 0x87,
>         .siocfg_leave = 0xaa,
>     };
>
>     static const struct w83627hf_sio_pdata alt_pdata = {
>         .siocfg_enter = 0x88,
>         .siocfg_leave = 0xbb,
>     };
>
>     static const struct dmi_system_id wdt_dmi_table[] __initconst = {
>         {
>             .matches = {
>                 DMI_EXACT_MATCH(DMI_SYS_VENDOR, "INVENTEC"),
>                 DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "Symphony"),
>                 DMI_EXACT_MATCH(DMI_BOARD_NAME, "SHARKBAY"),
>             },
>             .driver_data = (void *)&alt_pdata,
>         },
>         {}
>     };
>
> Then, in `wdt_init()`, it can define a local `pdata` and initialize it
> based on the DMI match:
>
>     const struct dmi_system_id *match;
>     struct w83627hf_sio_pdata pdata;
>
>     match = dmi_first_match(wdt_dmi_table);
>     if (match)
>         pdata = *(const struct w83627hf_sio_pdata *)match->driver_data;
>     else
>         pdata = default_pdata;
>
>     pdata.sioaddr = SIO_REG_CONF_ADDR0;
>     chip = wdt_find(pdata.sioaddr, pdata.siocfg_enter, pdata.siocfg_leave);

Yes, pdata was still global, but the scope is limited to the init function.
Your proposition looks better tho.



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