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() ?
That would enable hiding the data field definition completely from drivers.
> and some more casts when the driver data is used can be dropped, too.
> (e.g.
>
> - device_info = (struct pmbus_device_info *)i2c_match_id(pmbus_id, client)->driver_data;
> + device_info = i2c_match_id(pmbus_id, client)->driver_data_ptr;
That code could (should ?) probably use i2c_get_match_data() even today
to avoid the type cast. It would also be nice to have a similar API
function returning ->driver_data as kernel_ulong_t to be able to avoid
dereferencing ->driver_data directly if the value is not used as pointer.
This way ->driver_data and its use could be made internal to the I2C code,
with dereferencing completely handled in the I2C core.
Thanks,
Guenter
