Re: [PATCH] hwmon: Initialize i2c_device_id structures by name

[Guenter Roeck <linux@roeck-us.net>]

On 12/7/24 01:13, Uwe Kleine-König wrote:
> Hello Guenter,
> 
> [dropped Jose Ramon San Buenaventura from Cc: who's address bounced]
> 
> On Thu, Dec 05, 2024 at 08:27:15AM -0800, Guenter Roeck wrote:
>> On Thu, Dec 05, 2024 at 04:28:33PM +0100, Uwe Kleine-König wrote:
>>> I intend to change the definition of struct i2c_device_id to look as
>>> follows:
>>>
>>>          struct i2c_device_id {
>>>                 char name[I2C_NAME_SIZE];
>>>                 /* Data private to the driver */
>>>                 union {
>>>                         kernel_ulong_t driver_data;
>>>                         const void *driver_data_ptr;
>>>                 };
>>>          };
>>>
>>> That the initializers for these structures also work with this new
>>> definition, they must use named members.
>>>
>>> The motivation for that change is to be able to drop many casts from
>>> pointer to kernel_ulong_t. So once the definition is updated,
>>> initializers that today use:
>>>
>>> 	{"adp4000", (kernel_ulong_t)&pmbus_info_one},
>>>
>>> can be changed to
>>>
>>>          { .name = "adp4000", .driver_data_ptr = &pmbus_info_one },
>>>
>>
>> How about introducing a macro for that instead, similar to I3C_DEVICE() ?
> 
> OK, for now we'd have then:
> 
> 	#define I2C_DEVICE_ID_PTR(_name, _driver_data_ptr)	\
> 		{ .name = (_name), .driver_data = (kernel_ulong_t)_driver_data_ptr }
> 
> 	#define I2C_DEVICE_ID_ULONG(_name, _driver_data)	\
> 		{ .name = (_name), .driver_data = _driver_data }
> 
> plus maybe:
> 
> 	#define I2C_DEVICE_ID(_name)	\
> 		{ .name = (_name) }
> 
> for the drivers that don't need driver data?
> 

Something like that, though I'd even hide the parameter type and just have

	#define I2C_DEVICE_ID_DATA(_name, _data)	\
		{ .name = (_name), .driver_data = (kernel_ulong_t)_data }

	#define I2C_DEVICE_ID(_name)	\
		{ .name = (_name) }

where I2C_DEVICE_ID_DATA() would accept any type.

> When all drivers are converted accordingly, we could change the
> definition of i2c_device_id in a commit that only touches i2c core
> things to introduce the stronger type checking.

The stronger type checking would not be possible with that, though.
Does that really add value ? I personally prefer the opaque style where
driver users can provide (almost) any type they want/need.

Thanks,
Guenter