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

[Jonathan Cameron]

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

> [Dropped one recipient who's email address isn't valid any more]
> 
> Hello Jonathan,
> 
> 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.

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.

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.

Thanks,

Jonathan


> 
> > Also, have you done a checkpatch check for this?  Nice to not being fighting
> > it for ever.  
> 
> No, but that's on my agenda.
> 
> Best regards
> Uwe