[PATCH v1] regulator: Use named initializers for arrays of i2c_device_data
Woody Douglass
wdouglass at carnegierobotics.com
Mon May 18 10:07:46 PDT 2026
On 5/15/26 08:28, Laurent Pinchart wrote:
> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender.
>
>
> Hi Uwe,
>
> Thank you for the patch.
>
> On Fri, May 15, 2026 at 12:31:50PM +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 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 at baylibre.com>
>> ---
>> 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.
>>
>> 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
>>
>> [1] https://cheri-alliance.org/discover-cheri/
>> https://lwn.net/Articles/1037974/
>>
>> drivers/regulator/88pg86x.c | 4 +--
>> drivers/regulator/ad5398.c | 4 +--
>> drivers/regulator/da9121-regulator.c | 20 +++++++--------
>> drivers/regulator/da9210-regulator.c | 4 +--
>> drivers/regulator/da9211-regulator.c | 18 +++++++-------
>> drivers/regulator/fan53880.c | 4 +--
>> drivers/regulator/isl9305.c | 4 +--
>> drivers/regulator/lp3971.c | 2 +-
>> drivers/regulator/lp3972.c | 2 +-
>> drivers/regulator/lp872x.c | 34 +++++++++++++-------------
>> drivers/regulator/lp8755.c | 4 +--
>> drivers/regulator/ltc3589.c | 6 ++---
>> drivers/regulator/ltc3676.c | 2 +-
>> drivers/regulator/max1586.c | 2 +-
>> drivers/regulator/max20086-regulator.c | 8 +++---
>> drivers/regulator/max20411-regulator.c | 2 +-
>> drivers/regulator/max77503-regulator.c | 2 +-
>> drivers/regulator/max77675-regulator.c | 2 +-
>> drivers/regulator/max77826-regulator.c | 2 +-
>> drivers/regulator/max77838-regulator.c | 2 +-
>> drivers/regulator/max77857-regulator.c | 8 +++---
>> drivers/regulator/max8649.c | 2 +-
>> drivers/regulator/max8893.c | 2 +-
>> drivers/regulator/max8952.c | 2 +-
>> drivers/regulator/mcp16502.c | 2 +-
>> drivers/regulator/mp5416.c | 6 ++---
>> drivers/regulator/mp8859.c | 4 +--
>> drivers/regulator/mp886x.c | 6 ++---
>> drivers/regulator/mpq7920.c | 4 +--
>> drivers/regulator/mt6311-regulator.c | 4 +--
>> drivers/regulator/pf530x-regulator.c | 8 +++---
>> drivers/regulator/pf8x00-regulator.c | 8 +++---
>> drivers/regulator/pv88060-regulator.c | 4 +--
>> drivers/regulator/pv88080-regulator.c | 8 +++---
>> drivers/regulator/pv88090-regulator.c | 4 +--
>> drivers/regulator/slg51000-regulator.c | 4 +--
>> drivers/regulator/sy8106a-regulator.c | 2 +-
>> drivers/regulator/sy8824x.c | 8 +++---
>> drivers/regulator/sy8827n.c | 4 +--
>> drivers/regulator/tps6286x-regulator.c | 10 ++++----
>> drivers/regulator/tps6287x-regulator.c | 10 ++++----
>> 41 files changed, 119 insertions(+), 119 deletions(-)
> [snip]
>
>> diff --git a/drivers/regulator/pf530x-regulator.c b/drivers/regulator/pf530x-regulator.c
>> index f789c4b6a499..e7b13d60106b 100644
>> --- a/drivers/regulator/pf530x-regulator.c
>> +++ b/drivers/regulator/pf530x-regulator.c
>> @@ -353,10 +353,10 @@ static const struct of_device_id pf530x_dt_ids[] = {
>> MODULE_DEVICE_TABLE(of, pf530x_dt_ids);
>>
>> static const struct i2c_device_id pf530x_i2c_id[] = {
>> - { "pf5300", 0 },
>> - { "pf5301", 0 },
>> - { "pf5302", 0 },
>> - {},
>> + { .name = "pf5300", .driver_data = 0 },
>> + { .name = "pf5301", .driver_data = 0 },
>> + { .name = "pf5302", .driver_data = 0 },
> I think you can drop driver_data here. It doesn't appear to be used by
> the driver.
I can confirm this, the pf530x driver does not use the driver_data
member. This is an improvement, thank you!
Reviewed-by: Woodrow Douglass <wdouglass at carnegierobotics.com>
> I like the result overall. With this small issue addressed,
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas at ideasonboard.com>
>
>> + { }
>> };
>> MODULE_DEVICE_TABLE(i2c, pf530x_i2c_id);
>>
> [snip]
>
> --
> Regards,
>
> Laurent Pinchart
More information about the Linux-mediatek
mailing list