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
