linux-kernel - Re: [PATCH RFC] i2c: Add a void pointer to i2c_device

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Mon, 29 Apr 2024 12:21:05 +0200
From: Uwe Kleine-König <u.kleine-koenig@...gutronix.de>
To: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Cc: Arnd Bergmann <arnd@...db.de>, 
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>, linux-kernel@...r.kernel.org, 
	Krzysztof Kozlowski <krzk@...nel.org>, Wolfram Sang <wsa+renesas@...g-engineering.com>, 
	linux-i2c@...r.kernel.org, kernel@...gutronix.de
Subject: Re: [PATCH RFC] i2c: Add a void pointer to i2c_device_id

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/ |

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)