Re: [PATCH 10/23] iio: accel: kxcjk-1013: Convert to i2c's .probe_new

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

On Mon, Oct 24, 2022 at 09:05:18AM +0200, Uwe Kleine-König wrote:
> On Sun, Oct 23, 2022 at 10:06:11PM +0300, Andy Shevchenko wrote:
> > On Sun, Oct 23, 2022 at 03:22:49PM +0200, Uwe Kleine-König wrote:

...

> > > +static const struct i2c_device_id kxcjk1013_id[] = {
> > > +	{"kxcjk1013", KXCJK1013},
> > > +	{"kxcj91008", KXCJ91008},
> > > +	{"kxtj21009", KXTJ21009},
> > > +	{"kxtf9",     KXTF9},
> > > +	{"kx023-1025", KX0231025},
> > > +	{"SMO8500",   KXCJ91008},
> > > +	{}
> > > +};
> > > +MODULE_DEVICE_TABLE(i2c, kxcjk1013_id);
> > 
> > I don't like this part. Can we, please, find a way how to dereference this
> > table via struct i2c_client, please?
> 
> It would be possible to do (on top of my patch here as PoC):
> 
> diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
> index e043dd698747..00269b25af99 100644
> --- a/drivers/iio/accel/kxcjk-1013.c
> +++ b/drivers/iio/accel/kxcjk-1013.c
> @@ -1445,7 +1445,7 @@ MODULE_DEVICE_TABLE(i2c, kxcjk1013_id);
>  
>  static int kxcjk1013_probe(struct i2c_client *client)
>  {
> -	const struct i2c_device_id *id = i2c_match_id(kxcjk1013_id, client);
> +	const struct i2c_device_id *id = i2c_match_id(to_i2c_driver(client->dev.driver)->id_table, client);
>  	struct kxcjk1013_data *data;
>  	struct iio_dev *indio_dev;
>  	struct kxcjk_1013_platform_data *pdata;
> 
> (only compile tested), you could even create a function or macro to make
> this a bit prettier on the source level. For the compiler loading the
> address from a local symbol instead of from two pointer dereferences is
> (I guess) a bit more effective and IMHO more natural.
> 
> *shrug*, I don't care much, but I don't like to have to rework this
> series just because you don't like this part. You even didn't give a
> rationale, I can imagine several different ones:

And I don't want to have ping-ponging the pieces of code (ID tables) because
some API has to be fixes or so.

>  [ ] it makes the patch bigger
>  [ ] it results in an unnatural order of symbols in the driver
>  [ ] it's some kind of duplication
>  [ ] something else
>      please elaborate: ________________________________

It adds a burden to the future work with no good justification along with
a churn in _this_ series.

While I like the rest of the series, these things I would rather postpone
or rework.

-- 
With Best Regards,
Andy Shevchenko