Re: [PATCH RFC] i2c: Add a void pointer to i2c_device_id

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello,

On Mon, Apr 29, 2024 at 11:54:29AM +0300, Andy Shevchenko wrote:
> On Fri, Apr 26, 2024 at 11:38:33PM +0200, Uwe Kleine-König wrote:
> > Traditionally a struct i2c_device_id has a kernel_ulong_t member to
> > store some chip variant data. Some drivers use it to store an enum,
> > others to store a pointer. In the latter case there is some ugly(?)
> > casting involved. To improve that, add a void pointer in an anonymous
> > union together with the integer driver_data.
> > 
> > This way a i2c_device_id requires usage of a designated initializer when
> > the driver_data or data member should be initialized, but IMHO that's
> > fine and you might even consider that an advantage.
> 
> ...
> 
> >  static const struct i2c_device_id wlf_gf_module_id[] = {
> > -	{ "wlf-gf-module", 0 },
> > +	{ "wlf-gf-module", },
> 
> In such cases the inner comma is redundant as well.

I would tend to keep the comma, but no strong opinion on my side.
If another member init is added later, the line has to be touched
anyhow, but in the layout:

	... = {
		{
			"wlf-gf-module",
		},
		{ }
	}

I'd keep it for sure.


> >  	{ }
> >  };
> 
> ...
> 
> In general idea might be okay, but I always have the same Q (do we have it
> being clarified in the documentation, btw): is an ID table the ABI or not?
> In another word, how should we treat the changes there, because ID tables
> are being used by the user space tools.

Note that the layout doesn't change and the traditional interpretation
of the data still works fine. Or do you see something that I miss?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Linux GPIO]     [Linux SPI]     [Linux Hardward Monitoring]     [LM Sensors]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux