Re: [PATCH v2 7/7] iio: Initialize i2c_device_id arrays using member names

[Jonathan Cameron]

On Thu, 21 May 2026 15:20:09 +0200
Uwe Kleine-König (The Capable Hub) <u.kleine-koenig@xxxxxxxxxxxx> wrote:

> Hello Jonathan,
> 
> On Thu, May 21, 2026 at 01:04:44PM +0100, Jonathan Cameron wrote:
> > On Tue, 19 May 2026 21:51:20 +0200
> > Uwe Kleine-König (The Capable Hub) <u.kleine-koenig@xxxxxxxxxxxx> wrote:  
> > > On Tue, May 19, 2026 at 07:39:13PM +0100, Jonathan Cameron wrote:  
> > > > On Tue, 19 May 2026 10:13:09 +0200
> > > > 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.
> > > > > 
> > > > > 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>    
> > > > 
> > > > I'd prefer it split into cases you care about (not just name) and the name only ones.
> > > > That is unless I'm missing some potential change that breaks initializing
> > > > just the first element and hence not the union you plan to add.
> > > > 
> > > > It's a lot of churn and the name one isn't enabling anything new unless
> > > > I'm missing something.    
> > > 
> > > Today all hunks are about using named initializers to improve
> > > readability, so the split into only .name vs. .name+.driver_data feels
> > > very artificial to me. But if you think that's the compromise to use,
> > > I'll adapt.
> > >   
> > > > We also get fixes in these annoyingly often so chances are this will mess
> > > > up backports.  Might even be worth splitting it up into directories
> > > > just to reduce that backport mess.    
> > > 
> > > In my opinion this is the reason to do this kind of cleanup with one
> > > patch per driver. This doesn't only make backports easier, it also
> > > allows to better record who reviewed and acked what, it reduces merge
> > > conflicts (because if one driver is updated in your tree already since
> > > my base, with one subsystem patch you get a merge conflict which makes
> > > the whole patch unapplicable, with one patch per driver only one out of
> > > (here) 208 fails).
> > > 
> > > But that isn't popular with most subsystem maintainers and so I went
> > > with one patch per subsystem. 🤷  
> > 
> > Meh.  This is going to be painful whatever, so to have it as fresh
> > as possible I'll pick it up now as one giant patch.  
> 
> \o/, thanks.
> 
> > Note there are already conflicts... Fixed up:
> > light/tsl2772.c (a couple more entries)
> > light/vcnl4000.c (data is now all pointers, not enum values).
> > 
> > Bunch of line changes in other drivers, but otherwise went in fine.  
> 
> I reproduced the issue, my conflict resolution (on top of next-20260521)
> looks as follows:
> 



> diff --git a/drivers/iio/adc/rtq6056.c b/drivers/iio/adc/rtq6056.c
> index e2b1da13c0d3..ae50fb27bac9 100644
> --- a/drivers/iio/adc/rtq6056.c
> +++ b/drivers/iio/adc/rtq6056.c
> @@ -872,8 +872,8 @@ static const struct richtek_dev_data rtq6059_devdata = {
>  };
>  
>  static const struct i2c_device_id rtq6056_id[] = {
> -	{ "rtq6056", (kernel_ulong_t)&rtq6056_devdata },
> -	{ "rtq6059", (kernel_ulong_t)&rtq6059_devdata },
> +	{ .name = "rtq6056", .driver_data = (kernel_ulong_t)&rtq6056_devdata },
> +	{ .name = "rtq6059", .driver_data = (kernel_ulong_t)&rtq6059_devdata },
>  	{ }
>  };
>  MODULE_DEVICE_TABLE(i2c, rtq6056_id);
> 
> The last hunk was made necessary by commit
> ce80292ead5bb42b50a6b63e44fd95c0edf9d334. And I'm pretty sure that the
> following is possible on top:
> 
>  static const struct of_device_id rtq6056_device_match[] = {
> -	{ .compatible = "richtek,rtq6056", .data = &rtq6056_devdata },
> -	{ .compatible = "richtek,rtq6059", .data = &rtq6059_devdata },
> +	{ .compatible = "richtek,rtq6056" },
> +	{ .compatible = "richtek,rtq6059" },
>  	{ }
>  };
>  MODULE_DEVICE_TABLE(of, rtq6056_device_match);
First two match what I had.

For the rtq one lets do it as a separate patch along with any other stragglers.

> 
> > There are a few new instances in tree, though from a quick glance
> > maybe no i2c ones.  Since you sent the first series I've been looking
> > out for this in reviews, but some stuff was a already queued.
> > 
> > Anyhow, new ones can be dealt with in follow up patches.  
> 
> There will more some more drivers I missed in other subsystems, so I'll
> have to reiterate anyhow. If you catch drivers before going in, that's
> great, but it's not a problem if you miss a few.
> 
> Thanks for your cooperation,
Thanks for doing the hard work ;)

J
> Uwe