Re: [PATCH] iio: chemical: sgp30: Convert enum->pointer for data in the match tables

[Andreas Brauchli <a.brauchli@elementarea.net>]

On Mon, 2023-08-28 at 17:13 +0100, Jonathan Cameron wrote:
> On Sat, 12 Aug 2023 17:57:30 +0100
> Biju Das <biju.das.jz@bp.renesas.com> wrote:
> 
> > Convert enum->pointer for data in the match tables, so that
> > device_get_match_data() can do match against OF/ACPI/I2C tables,
> > once i2c
> > bus type match support added to it.
> > 
> > Add product_id variable to struct sgp_device and remove the local
> > variable
> > product_id in probe() and replace enum->struct *sgp_device for data
> > in the
> > match table. Simplify theprobe() by replacing
> > device_get_match_data() and
> > ID lookup for retrieving data by i2c_get_match_data().
> > 
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> +CC Andreas (comments still welcome!)
ACK. Thanks, LGTM.

+CC Pascal at Sensirion

> 
> Applied to the togreg branch of iio.git but only pushed out as
> testing for
> now because I'll be rebasing on rc1 once available.  0-day can poke
> at
> it in the meantime.
> 
> Thanks,
> 
> Jonathan
> 
> > ---
> >  drivers/iio/chemical/sgp30.c | 24 ++++++++++++------------
> >  1 file changed, 12 insertions(+), 12 deletions(-)
> > 
> > diff --git a/drivers/iio/chemical/sgp30.c
> > b/drivers/iio/chemical/sgp30.c
> > index b509cff9ce37..21730d62b5c8 100644
> > --- a/drivers/iio/chemical/sgp30.c
> > +++ b/drivers/iio/chemical/sgp30.c
> > @@ -114,6 +114,7 @@ struct sgp_data {
> >  };
> > 
> >  struct sgp_device {
> > +       unsigned long product_id;
> >         const struct iio_chan_spec *channels;
> >         int num_channels;
> >  };
> > @@ -182,10 +183,12 @@ static const struct iio_chan_spec
> > sgpc3_channels[] = {
> > 
> >  static const struct sgp_device sgp_devices[] = {
> >         [SGP30] = {
> > +               .product_id = SGP30,
> >                 .channels = sgp30_channels,
> >                 .num_channels = ARRAY_SIZE(sgp30_channels),
> >         },
> >         [SGPC3] = {
> > +               .product_id = SGPC3,
> >                 .channels = sgpc3_channels,
> >                 .num_channels = ARRAY_SIZE(sgpc3_channels),
> >         },
> > @@ -491,28 +494,25 @@ static const struct iio_info sgp_info = {
> >  };
> > 
> >  static const struct of_device_id sgp_dt_ids[] = {
> > -       { .compatible = "sensirion,sgp30", .data = (void *)SGP30 },
> > -       { .compatible = "sensirion,sgpc3", .data = (void *)SGPC3 },
> > +       { .compatible = "sensirion,sgp30", .data =
> > &sgp_devices[SGP30] },
> > +       { .compatible = "sensirion,sgpc3", .data =
> > &sgp_devices[SGPC3] },
> >         { }
> >  };
> > 
> >  static int sgp_probe(struct i2c_client *client)
> >  {
> >         const struct i2c_device_id *id =
> > i2c_client_get_device_id(client);
> > +       const struct sgp_device *match_data;
> >         struct device *dev = &client->dev;
> >         struct iio_dev *indio_dev;
> >         struct sgp_data *data;
> > -       unsigned long product_id;
> >         int ret;
> > 
> >         indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
> >         if (!indio_dev)
> >                 return -ENOMEM;
> > 
> > -       if (dev_fwnode(dev))
> > -               product_id = (unsigned
> > long)device_get_match_data(dev);
> > -       else
> > -               product_id = id->driver_data;
> > +       match_data = i2c_get_match_data(client);
> > 
> >         data = iio_priv(indio_dev);
> >         i2c_set_clientdata(client, indio_dev);
> > @@ -528,15 +528,15 @@ static int sgp_probe(struct i2c_client
> > *client)
> > 
> >         data->feature_set = be16_to_cpu(data-
> > >buffer.raw_words[0].value);
> > 
> > -       ret = sgp_check_compat(data, product_id);
> > +       ret = sgp_check_compat(data, match_data->product_id);
> >         if (ret)
> >                 return ret;
> > 
> >         indio_dev->info = &sgp_info;
> >         indio_dev->name = id->name;
> >         indio_dev->modes = INDIO_DIRECT_MODE;
> > -       indio_dev->channels = sgp_devices[product_id].channels;
> > -       indio_dev->num_channels =
> > sgp_devices[product_id].num_channels;
> > +       indio_dev->channels = match_data->channels;
> > +       indio_dev->num_channels = match_data->num_channels;
> > 
> >         sgp_init(data);
> > 
> > @@ -562,8 +562,8 @@ static void sgp_remove(struct i2c_client
> > *client)
> >  }
> > 
> >  static const struct i2c_device_id sgp_id[] = {
> > -       { "sgp30", SGP30 },
> > -       { "sgpc3", SGPC3 },
> > +       { "sgp30", (kernel_ulong_t)&sgp_devices[SGP30] },
> > +       { "sgpc3", (kernel_ulong_t)&sgp_devices[SGPC3] },
> >         { }
> >  };
> > 
>