On Mon, May 18, 2026 at 12:04:01PM +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. > > While touching all these arrays, unify usage of whitespace in the list > terminator and drop trailing commas there. > > 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) > <[email protected]> > --- > 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/regulator/ad5398.c b/drivers/regulator/ad5398.c > index 0123ca8157a8..84272ba65d08 100644 > --- a/drivers/regulator/ad5398.c > +++ b/drivers/regulator/ad5398.c > @@ -207,8 +207,8 @@ struct ad5398_current_data_format { > static const struct ad5398_current_data_format df_10_4_120 = {10, 4, > 0, 120000}; > > static const struct i2c_device_id ad5398_id[] = { > - { .name = "ad5398", .driver_data = (kernel_ulong_t)&df_10_4_120 > }, > - { .name = "ad5821", .driver_data = (kernel_ulong_t)&df_10_4_120 > }, > + { .name = "ad5398", .driver_data_ptr = &df_10_4_120 }, > + { .name = "ad5821", .driver_data_ptr = &df_10_4_120 }, > { } > }; > MODULE_DEVICE_TABLE(i2c, ad5398_id); > @@ -219,8 +219,7 @@ static int ad5398_probe(struct i2c_client *client) > struct regulator_init_data *init_data = > dev_get_platdata(&client->dev); > struct regulator_config config = { }; > struct ad5398_chip_info *chip; > - const struct ad5398_current_data_format *df = > - (struct ad5398_current_data_format > *)id->driver_data; > + const struct ad5398_current_data_format *df = > id->driver_data_ptr; > > chip = devm_kzalloc(&client->dev, sizeof(*chip), GFP_KERNEL); > if (!chip) > > 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. (I didn't find a nice driver below drivers/gpu that > benefits, so this is "only" a regulator driver example.) > > 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 was unsure if I should split the patch. I guess doing all of > drivers/gpu/drm/bridge together is fine, please tell if I should split > off drivers/gpu/drm/nouveau/ and/or drivers/gpu/drm/sitronix/. > > Best regards > Uwe > > [1] https://cheri-alliance.org/discover-cheri/ > https://lwn.net/Articles/1037974/ > > drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 10 +++++----- > drivers/gpu/drm/bridge/analogix/analogix-anx6345.c | 2 +- > drivers/gpu/drm/bridge/analogix/anx7625.c | 4 ++-- > drivers/gpu/drm/bridge/chipone-icn6211.c | 4 ++-- > drivers/gpu/drm/bridge/chrontel-ch7033.c | 2 +- > drivers/gpu/drm/bridge/ite-it6263.c | 2 +-
Reviewed-by: Liu Ying <[email protected]> # ite-it6263.c -- Regards, Liu Ying
