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

[Bartosz Golaszewski]

On Fri, May 15, 2026 at 6:27 PM Uwe Kleine-König (The Capable Hub)
<u.kleine-koenig@xxxxxxxxxxxx> 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 and commas.
>
> 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/misc/eeprom/m24lr.c b/drivers/misc/eeprom/m24lr.c
>         index bf2194d7cdbd..e0d7d8739e67 100644
>         --- a/drivers/misc/eeprom/m24lr.c
>         +++ b/drivers/misc/eeprom/m24lr.c
>         @@ -104,9 +104,9 @@ static const struct m24lr_chip m24lr64e_r_chip = {
>          };
>
>          static const struct i2c_device_id m24lr_ids[] = {
>         -       { .name = "m24lr04e-r", .driver_data = (kernel_ulong_t)&m24lr04e_r_chip },
>         -       { .name = "m24lr16e-r", .driver_data = (kernel_ulong_t)&m24lr16e_r_chip },
>         -       { .name = "m24lr64e-r", .driver_data = (kernel_ulong_t)&m24lr64e_r_chip },
>         +       { .name = "m24lr04e-r", .driver_data_ptr = &m24lr04e_r_chip },
>         +       { .name = "m24lr16e-r", .driver_data_ptr = &m24lr16e_r_chip },
>         +       { .name = "m24lr64e-r", .driver_data_ptr = &m24lr64e_r_chip },
>                 { }
>          };
>          MODULE_DEVICE_TABLE(i2c, m24lr_ids);
>         @@ -478,7 +478,7 @@ static const struct m24lr_chip *m24lr_get_chip(struct device *dev)
>                 if (dev->of_node && of_match_device(m24lr_of_match, dev))
>                         ret = of_device_get_match_data(dev);
>                 else if (id)
>         -               ret = (void *)id->driver_data;
>         +               ret = id->driver_data;
>                 else
>                         ret = acpi_device_get_match_data(dev);
>
> 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.
>
> Best regards
> Uwe
>
>  drivers/misc/ad525x_dpot-i2c.c         |  52 ++++++------
>  drivers/misc/amd-sbi/rmi-i2c.c         |   4 +-
>  drivers/misc/apds9802als.c             |   2 +-
>  drivers/misc/apds990x.c                |   4 +-
>  drivers/misc/bh1770glc.c               |   6 +-
>  drivers/misc/ds1682.c                  |   2 +-
>  drivers/misc/eeprom/at24.c             |  62 +++++++-------

Hi!

This drivers lives under drivers/misc/ but I maintain it separately
and send PRs to Wolfram so they go through the i2c tree (historical
reasons really and maybe we should move it under drivers/nvmem/?). Can
you send this separately?

Bart