Re: [PATCH v1 2/2] iio: Initialize i2c_device_id arrays using member names

[Jonathan Cameron <jic23@kernel.org>]

On Tue, 12 May 2026 14:50:35 +0200
Uwe Kleine-König (The Capable Hub) <u.kleine-koenig@baylibre.com> 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@baylibre.com>
Hi Uwe

A couple of these are cases where we can rip the driver_data out like you did in patch 1.

Other minor things inline.

Thanks for doing this.

Jonathan

> diff --git a/drivers/iio/accel/bmc150-accel-i2c.c b/drivers/iio/accel/bmc150-accel-i2c.c
> index b4604f441553..aab3b110faa5 100644
> --- a/drivers/iio/accel/bmc150-accel-i2c.c
> +++ b/drivers/iio/accel/bmc150-accel-i2c.c
> @@ -245,16 +245,16 @@ static const struct acpi_device_id bmc150_accel_acpi_match[] = {
>  MODULE_DEVICE_TABLE(acpi, bmc150_accel_acpi_match);
>  
>  static const struct i2c_device_id bmc150_accel_id[] = {
> -	{"bma222"},
> -	{"bma222e"},
> -	{"bma250e"},
> -	{"bma253"},
> -	{"bma254"},
> -	{"bma255"},
> -	{"bma280"},
> -	{"bmc150_accel"},
> -	{"bmc156_accel", BOSCH_BMC156},
> -	{"bmi055_accel"},
> +	{ .name = "bma222", .driver_data = 0 },
So we do have BOSCH_UKNOWN for the the 0 value.  It doesn't actually mean
it's unknown but rather that ID matching will work so we don't need
to know what it is.  Maybe we should use it anyway rather than 0?

Original approach of relying on the magic 0 is more horrible still so
this is definitely an improvement!

> +	{ .name = "bma222e", .driver_data = 0 },
> +	{ .name = "bma250e", .driver_data = 0 },
> +	{ .name = "bma253", .driver_data = 0 },
> +	{ .name = "bma254", .driver_data = 0 },
> +	{ .name = "bma255", .driver_data = 0 },
> +	{ .name = "bma280", .driver_data = 0 },
> +	{ .name = "bmc150_accel", .driver_data = 0 },
> +	{ .name = "bmc156_accel", .driver_data = BOSCH_BMC156 },
> +	{ .name = "bmi055_accel", .driver_data = 0 },
>  	{ }
>  };
>  

> diff --git a/drivers/iio/adc/ad7091r5.c b/drivers/iio/adc/ad7091r5.c
> index bd4877268689..d4c4b19edc61 100644
> --- a/drivers/iio/adc/ad7091r5.c
> +++ b/drivers/iio/adc/ad7091r5.c
> @@ -117,7 +117,7 @@ static const struct of_device_id ad7091r5_dt_ids[] = {
>  MODULE_DEVICE_TABLE(of, ad7091r5_dt_ids);
>  
>  static const struct i2c_device_id ad7091r5_i2c_ids[] = {
> -	{ "ad7091r5", (kernel_ulong_t)&ad7091r5_init_info },
> +	{ .name = "ad7091r5", .driver_data = (kernel_ulong_t)&ad7091r5_init_info },
>  	{ }
>  };
>  MODULE_DEVICE_TABLE(i2c, ad7091r5_i2c_ids);

Hmm.  I guess another user of the common library actually supports multiple parts.
For this one we could just hard code it in ad7091r5_i2c_probe()


>
> diff --git a/drivers/iio/dac/max5821.c b/drivers/iio/dac/max5821.c
> index e7e29359f8fe..b3ae00431910 100644
> --- a/drivers/iio/dac/max5821.c
> +++ b/drivers/iio/dac/max5821.c
> @@ -332,7 +332,7 @@ static int max5821_probe(struct i2c_client *client)
>  }
>  
>  static const struct i2c_device_id max5821_id[] = {
> -	{ "max5821", ID_MAX5821 },
> +	{ .name = "max5821", .driver_data = ID_MAX5821 },

Used?  I get suspicious of any single entry ones and I can't immediately
see where it is used.

>  	{ }
>  };
>  MODULE_DEVICE_TABLE(i2c, max5821_id);

> diff --git a/drivers/iio/proximity/sx9310.c b/drivers/iio/proximity/sx9310.c
> index fb02eac78ed4..602f7b95c83e 100644
> --- a/drivers/iio/proximity/sx9310.c
> +++ b/drivers/iio/proximity/sx9310.c
Comments below refer to the these 3.

> @@ -1007,8 +1007,8 @@ static const struct of_device_id sx9310_of_match[] = {
>  MODULE_DEVICE_TABLE(of, sx9310_of_match);
>  
>  static const struct i2c_device_id sx9310_id[] = {
> -	{ "sx9310", (kernel_ulong_t)&sx9310_info },
> -	{ "sx9311", (kernel_ulong_t)&sx9311_info },
> +	{ .name = "sx9310", .driver_data = (kernel_ulong_t)&sx9310_info },
> +	{ .name = "sx9311", .driver_data = (kernel_ulong_t)&sx9311_info },

These are used so fine. But the other users of the sx_common library are
less obvious. I'm fairly sure the use here is confined to this sub driver.

>  	{ }
>  };
>  MODULE_DEVICE_TABLE(i2c, sx9310_id);
> diff --git a/drivers/iio/proximity/sx9324.c b/drivers/iio/proximity/sx9324.c
> index f61eff39751d..4831d33cb337 100644
> --- a/drivers/iio/proximity/sx9324.c
> +++ b/drivers/iio/proximity/sx9324.c
> @@ -1135,7 +1135,7 @@ static const struct of_device_id sx9324_of_match[] = {
>  MODULE_DEVICE_TABLE(of, sx9324_of_match);
>  
>  static const struct i2c_device_id sx9324_id[] = {
> -	{ "sx9324", SX9324_WHOAMI_VALUE },
> +	{ .name = "sx9324", .driver_data = SX9324_WHOAMI_VALUE },
Same as below. I think you can rip this out.  Do the same for the other
tables whilst doing so.

>  	{ }
>  };
>  MODULE_DEVICE_TABLE(i2c, sx9324_id);
> diff --git a/drivers/iio/proximity/sx9360.c b/drivers/iio/proximity/sx9360.c
> index 4448988d4e7e..f8fd399dc2be 100644
> --- a/drivers/iio/proximity/sx9360.c
> +++ b/drivers/iio/proximity/sx9360.c
> @@ -845,7 +845,7 @@ static const struct of_device_id sx9360_of_match[] = {
>  MODULE_DEVICE_TABLE(of, sx9360_of_match);
>  
>  static const struct i2c_device_id sx9360_id[] = {
> -	{"sx9360", SX9360_WHOAMI_VALUE },
> +	{ .name = "sx9360", .driver_data = SX9360_WHOAMI_VALUE },

These are only 'sort of used'  I wonder if we should just rip
them out.  They are used locally in e.g. sx9310_check_whoami()
but there is only one value so we could hard coded it there instead.

This smells like cut and paste rather than something that is actually useful.
The name should be hardcoded as well.  Is is for this particular
driver and the sx9324.



>  	{ }
>  };
>  MODULE_DEVICE_TABLE(i2c, sx9360_id);