On Mon, May 18, 2026 at 03:40:35PM +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. > > 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..dfb0b07500a7 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; > > 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 tpm driver 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 don't understand why any of this should be merged to be honest, and why I should care about CHERI when reviewing mainline patches. Clean up can be side-effect but not a purpose. > > Best regards > Uwe > > [1] https://cheri-alliance.org/discover-cheri/ > https://lwn.net/Articles/1037974/ > --- > drivers/char/tpm/st33zp24/i2c.c | 4 ++-- > drivers/char/tpm/tpm_i2c_atmel.c | 4 ++-- > drivers/char/tpm/tpm_i2c_infineon.c | 8 ++++---- > drivers/char/tpm/tpm_i2c_nuvoton.c | 6 +++--- > drivers/char/tpm/tpm_tis_i2c.c | 4 ++-- > 5 files changed, 13 insertions(+), 13 deletions(-) > > diff --git a/drivers/char/tpm/st33zp24/i2c.c b/drivers/char/tpm/st33zp24/i2c.c > index 81348487c125..74b25af5ce79 100644 > --- a/drivers/char/tpm/st33zp24/i2c.c > +++ b/drivers/char/tpm/st33zp24/i2c.c > @@ -133,8 +133,8 @@ static void st33zp24_i2c_remove(struct i2c_client *client) > } > > static const struct i2c_device_id st33zp24_i2c_id[] = { > - { TPM_ST33_I2C }, > - {} > + { .name = TPM_ST33_I2C }, > + { } > }; > MODULE_DEVICE_TABLE(i2c, st33zp24_i2c_id); > > diff --git a/drivers/char/tpm/tpm_i2c_atmel.c > b/drivers/char/tpm/tpm_i2c_atmel.c > index 9fd73049821f..6891642a7f51 100644 > --- a/drivers/char/tpm/tpm_i2c_atmel.c > +++ b/drivers/char/tpm/tpm_i2c_atmel.c > @@ -199,8 +199,8 @@ static void i2c_atmel_remove(struct i2c_client *client) > } > > static const struct i2c_device_id i2c_atmel_id[] = { > - { I2C_DRIVER_NAME }, > - {} > + { .name = I2C_DRIVER_NAME }, > + { } > }; > MODULE_DEVICE_TABLE(i2c, i2c_atmel_id); > > diff --git a/drivers/char/tpm/tpm_i2c_infineon.c > b/drivers/char/tpm/tpm_i2c_infineon.c > index 8b7d32de0b2e..29cf2f998405 100644 > --- a/drivers/char/tpm/tpm_i2c_infineon.c > +++ b/drivers/char/tpm/tpm_i2c_infineon.c > @@ -664,10 +664,10 @@ static int tpm_tis_i2c_init(struct device *dev) > } > > static const struct i2c_device_id tpm_tis_i2c_table[] = { > - {"tpm_i2c_infineon"}, > - {"slb9635tt"}, > - {"slb9645tt"}, > - {}, > + { .name = "tpm_i2c_infineon" }, > + { .name = "slb9635tt" }, > + { .name = "slb9645tt" }, > + { } > }; > > MODULE_DEVICE_TABLE(i2c, tpm_tis_i2c_table); > diff --git a/drivers/char/tpm/tpm_i2c_nuvoton.c > b/drivers/char/tpm/tpm_i2c_nuvoton.c > index d44903b29929..71c59eeaccab 100644 > --- a/drivers/char/tpm/tpm_i2c_nuvoton.c > +++ b/drivers/char/tpm/tpm_i2c_nuvoton.c > @@ -624,9 +624,9 @@ static void i2c_nuvoton_remove(struct i2c_client *client) > } > > static const struct i2c_device_id i2c_nuvoton_id[] = { > - {"tpm_i2c_nuvoton"}, > - {"tpm2_i2c_nuvoton", .driver_data = I2C_IS_TPM2}, > - {} > + { .name = "tpm_i2c_nuvoton" }, > + { .name = "tpm2_i2c_nuvoton", .driver_data = I2C_IS_TPM2}, > + { } > }; > MODULE_DEVICE_TABLE(i2c, i2c_nuvoton_id); > > diff --git a/drivers/char/tpm/tpm_tis_i2c.c b/drivers/char/tpm/tpm_tis_i2c.c > index 6cd07dd34507..21d66bfba6a8 100644 > --- a/drivers/char/tpm/tpm_tis_i2c.c > +++ b/drivers/char/tpm/tpm_tis_i2c.c > @@ -375,8 +375,8 @@ static void tpm_tis_i2c_remove(struct i2c_client *client) > } > > static const struct i2c_device_id tpm_tis_i2c_id[] = { > - { "tpm_tis_i2c" }, > - {} > + { .name = "tpm_tis_i2c" }, > + { } > }; > MODULE_DEVICE_TABLE(i2c, tpm_tis_i2c_id); > > -- > 2.47.3 > BR, Jarkko
