Re: [PATCH v1] Input: Use named initializers for arrays of i2c_device_data

[Dmitry Torokhov]

Hi Uwe,

On Fri, May 15, 2026 at 06:48:47PM +0200, Uwe Kleine-König (The Capable Hub) wrote:
> While being less compact, using named initializers allows to more easily
> see which members of the structs are assigned which value without having
> to lookup the declaration of the struct. And it's also more robust
> against changes to the struct definition.
> 
> The mentioned robustness is relevant for a planned change to struct
> i2c_device_id that replaces .driver_data by an anonymous union.
> 
> This patch doesn't modify the compiled arrays, only their representation
> in source form benefits. The former was confirmed with x86 and arm64
> builds.
> 
> Signed-off-by: Uwe Kleine-König (The Capable Hub) <u.kleine-koenig@xxxxxxxxxxxx>
> ---
> Hello,
> 
> the mentioned change to i2c_device_id is the following:
> 
> 	diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
> 	index 23ff24080dfd..aebd3a5e90af 100644
> 	--- a/include/linux/mod_devicetable.h
> 	+++ b/include/linux/mod_devicetable.h
> 	@@ -477,7 +477,11 @@ struct rpmsg_device_id {
> 	
> 	 struct i2c_device_id {
> 	 	char name[I2C_NAME_SIZE];
> 	-	kernel_ulong_t driver_data;     /* Data private to the driver */
> 	+	union {
> 	+		/* Data private to the driver */
> 	+		kernel_ulong_t driver_data;
> 	+		const void *driver_data_ptr;
> 	+	};
> 	 };
> 	
> 	 /* pci_epf */
> 
> and this requires that .driver_data is assigned via a named initializer
> for static data. This requirement isn't a bad one because named
> initializers are also much better readable than list initializers.
> 
> The union added to struct i2c_device_id enables further cleanups like:
> 
> 	diff --git a/drivers/input/touchscreen/ili210x.c b/drivers/input/touchscreen/ili210x.c
> 	index 66ada7ffbc80..94aa4dc002c5 100644
> 	--- a/drivers/input/touchscreen/ili210x.c
> 	+++ b/drivers/input/touchscreen/ili210x.c
> 	@@ -969,7 +969,7 @@ static int ili210x_i2c_probe(struct i2c_client *client)
> 	 
> 		chip = device_get_match_data(dev);
> 		if (!chip && id)
> 	-		chip = (const struct ili2xxx_chip *)id->driver_data;
> 	+		chip = id->driver_data_ptr;
> 		if (!chip)
> 			return dev_err_probe(&client->dev, -ENODEV, "unknown device model\n");
> 	 
> 	@@ -1049,10 +1049,10 @@ static int ili210x_i2c_probe(struct i2c_client *client)
> 	 }
> 	 
> 	 static const struct i2c_device_id ili210x_i2c_id[] = {
> 	-	{ .name = "ili210x", .driver_data = (long)&ili210x_chip },
> 	-	{ .name = "ili2117", .driver_data = (long)&ili211x_chip },
> 	-	{ .name = "ili2120", .driver_data = (long)&ili212x_chip },
> 	-	{ .name = "ili251x", .driver_data = (long)&ili251x_chip },
> 	+	{ .name = "ili210x", .driver_data_ptr = &ili210x_chip },
> 	+	{ .name = "ili2117", .driver_data_ptr = &ili211x_chip },
> 	+	{ .name = "ili2120", .driver_data_ptr = &ili212x_chip },
> 	+	{ .name = "ili251x", .driver_data_ptr = &ili251x_chip },
> 		{ }
> 	 };
> 	 MODULE_DEVICE_TABLE(i2c, ili210x_i2c_id);
> 
> that are an improvement for readability (again!) and it keeps some
> properties of the pointers (here: being const) without having to pay
> attention for that.
> 
> My additional motivation for this effort is CHERI[1]. This is a hardware
> extension that uses 128 bit pointers but unsigned long is still 64 bit.
> So with CHERI you cannot store pointers in unsigned long variables.

I like the ability to properly set up pointers for driver data, however
I do not think we should use named initializers for name field. As long
as we are not planning on moving its position I like the brevity of just
saying

	{ "ili210x", .driver_data_ptr = &ili210x_chip },

Can we keep the old style for the name field?

Thanks.

-- 
Dmitry