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
