On Sat, Dec 07, 2024 at 07:32:43AM -0800, Guenter Roeck wrote: > 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) } It's better, if we go this way, to keep this in sync with above by changing as #define I2C_DEVICE_ID(_name) I2C_DEVICE_ID_DATA(_name, 0) > 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. -- With Best Regards, Andy Shevchenko