On Wed, Jun 26, 2024 at 12:34:05PM +0200, Wolfram Sang wrote:
> Hi Uwe,
>
> > There is an inconsistency in the different *_device_id structures. Some
> > have a kernel_ulong_t for driver private data, others have a void*.
> > Depending on how it's used in the drivers, one or the other is better.
> > To get the best from both the idea is to do
> >
> > diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
> > index 4338b1b4ac44..594c5ace303a 100644
> > --- a/include/linux/mod_devicetable.h
> > +++ b/include/linux/mod_devicetable.h
> > @@ -477,7 +477,10 @@ struct rpmsg_device_id {
> >
> > struct i2c_device_id {
> > char name[I2C_NAME_SIZE];
> > - kernel_ulong_t driver_data; /* Data private to the driver */
> > + union {
> > + kernel_ulong_t driver_data; /* Data private to the driver */
> > + const void *data;
> > + };
> > };
> >
> > /* pci_epf */
> >
> > which then allows to drop quite a few casts, e.g.
>
> I personally could live with the casts, even though the above looks
> clearer, of course. On suggestion, though, can we please have:
>
> + const void *data_ptr;
>
> There is no obvious difference between 'driver_data' and 'data' to
> determine what is the ulong type and what is void*. 'data_ptr' makes
> this instantly recognizable IMO.
I share that concern, the reason for considering "data" is the output of
git grep 'const void *' include/linux/mod_devicetable.h
Getting these all into alignment, maybe even picking a better name than
"driver_data", would be the eventual goal. IMHO "driver_data" vs
"data_ptr" looks strange, maybe I'd prefer "driver_ptr" over
"data" and "data_ptr"?!
Best regards and thanks for your feedback,
Uwe
Attachment:
signature.asc
Description: PGP signature
