diff mbox series

[v2] iio: chemical: vz89x: Convert enum->pointer for data in the match tables

Message ID 20230818190429.338065-1-biju.das.jz@bp.renesas.com (mailing list archive)
State Accepted
Headers show
Series [v2] iio: chemical: vz89x: Convert enum->pointer for data in the match tables | expand

Commit Message

Biju Das Aug. 18, 2023, 7:04 p.m. UTC
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.

Replace enum->struct *vz89x_chip_data for data in the match table. Simplify
the probe() 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>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
v1->v2:
 * Added Rb tag from Andy.
 * Dropped id variable removal from commit description.
---
 drivers/iio/chemical/vz89x.c | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

Comments

Jonathan Cameron Aug. 28, 2023, 12:35 p.m. UTC | #1
On Fri, 18 Aug 2023 20:04:29 +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.
> 
> Replace enum->struct *vz89x_chip_data for data in the match table. Simplify
> the probe() 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>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Biju,

Make sure you cc the driver authors etc.

I'll queue this one up, but Matt feel free to comment if you have time

Thanks,

Jonathan


> ---
> v1->v2:
>  * Added Rb tag from Andy.
>  * Dropped id variable removal from commit description.
> ---
>  drivers/iio/chemical/vz89x.c | 16 +++++-----------
>  1 file changed, 5 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/iio/chemical/vz89x.c b/drivers/iio/chemical/vz89x.c
> index 13555f4f401a..5b358bcd311b 100644
> --- a/drivers/iio/chemical/vz89x.c
> +++ b/drivers/iio/chemical/vz89x.c
> @@ -342,19 +342,17 @@ static const struct vz89x_chip_data vz89x_chips[] = {
>  };
>  
>  static const struct of_device_id vz89x_dt_ids[] = {
> -	{ .compatible = "sgx,vz89x", .data = (void *) VZ89X },
> -	{ .compatible = "sgx,vz89te", .data = (void *) VZ89TE },
> +	{ .compatible = "sgx,vz89x", .data = &vz89x_chips[VZ89X] },
> +	{ .compatible = "sgx,vz89te", .data = &vz89x_chips[VZ89TE] },
>  	{ }
>  };
>  MODULE_DEVICE_TABLE(of, vz89x_dt_ids);
>  
>  static int vz89x_probe(struct i2c_client *client)
>  {
> -	const struct i2c_device_id *id = i2c_client_get_device_id(client);
>  	struct device *dev = &client->dev;
>  	struct iio_dev *indio_dev;
>  	struct vz89x_data *data;
> -	int chip_id;
>  
>  	indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
>  	if (!indio_dev)
> @@ -369,14 +367,10 @@ static int vz89x_probe(struct i2c_client *client)
>  	else
>  		return -EOPNOTSUPP;
>  
> -	if (!dev_fwnode(dev))
> -		chip_id = id->driver_data;
> -	else
> -		chip_id = (unsigned long)device_get_match_data(dev);
> +	data->chip = i2c_get_match_data(client);
>  
>  	i2c_set_clientdata(client, indio_dev);
>  	data->client = client;
> -	data->chip = &vz89x_chips[chip_id];
>  	data->last_update = jiffies - HZ;
>  	mutex_init(&data->lock);
>  
> @@ -391,8 +385,8 @@ static int vz89x_probe(struct i2c_client *client)
>  }
>  
>  static const struct i2c_device_id vz89x_id[] = {
> -	{ "vz89x", VZ89X },
> -	{ "vz89te", VZ89TE },
> +	{ "vz89x", (kernel_ulong_t)&vz89x_chips[VZ89X] },
> +	{ "vz89te", (kernel_ulong_t)&vz89x_chips[VZ89TE] },
>  	{ }
>  };
>  MODULE_DEVICE_TABLE(i2c, vz89x_id);
Biju Das Aug. 28, 2023, 12:43 p.m. UTC | #2
Hi Jonathan Cameron,

> Subject: Re: [PATCH v2] iio: chemical: vz89x: Convert enum->pointer for
> data in the match tables
> 
> On Fri, 18 Aug 2023 20:04:29 +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.
> >
> > Replace enum->struct *vz89x_chip_data for data in the match table.
> > Simplify the probe() 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>
> > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> Biju,
> 
> Make sure you cc the driver authors etc.

Normally, I ran a script against the patch to get details and additionally I add Geert, renesas-soc , Andy in Cc list.
So far the patch worked correctly. Not sure I need to enhance the script to get driver authors name (for eg: Matt in this case)??

#!/bin/bash
PROGRAM_DIRECTORY="$(cd "$(dirname "$0")"; pwd; )"

source "${PROGRAM_DIRECTORY}/get_recipients_lib.sh"

MUST_HAVE+=("Geert Uytterhoeven <geert+renesas@glider.be>")
#MUST_HAVE+=("Chris Paterson <chris.paterson2@renesas.com>")
#MUST_HAVE+=("Fabrizio Castro <fabrizio.castro.jz@renesas.com>")
MUST_HAVE+=("Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@bp.renesas.com>")
MUST_HAVE+=("linux-renesas-soc@vger.kernel.org")

MUST_DROP+=("linux-kernel@vger.kernel.org")

populate_to_list
populate_cc_list

the script uses below commands to add relevant to and cc field.
./scripts/get_maintainer.pl ${PATCH_FILES} | grep maintainer

Cheers,
Biju

> 
> I'll queue this one up, but Matt feel free to comment if you have time
> 
> Thanks,
> 
> Jonathan
> 
> 
> > ---
> > v1->v2:
> >  * Added Rb tag from Andy.
> >  * Dropped id variable removal from commit description.
> > ---
> >  drivers/iio/chemical/vz89x.c | 16 +++++-----------
> >  1 file changed, 5 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/iio/chemical/vz89x.c
> > b/drivers/iio/chemical/vz89x.c index 13555f4f401a..5b358bcd311b 100644
> > --- a/drivers/iio/chemical/vz89x.c
> > +++ b/drivers/iio/chemical/vz89x.c
> > @@ -342,19 +342,17 @@ static const struct vz89x_chip_data
> > vz89x_chips[] = {  };
> >
> >  static const struct of_device_id vz89x_dt_ids[] = {
> > -	{ .compatible = "sgx,vz89x", .data = (void *) VZ89X },
> > -	{ .compatible = "sgx,vz89te", .data = (void *) VZ89TE },
> > +	{ .compatible = "sgx,vz89x", .data = &vz89x_chips[VZ89X] },
> > +	{ .compatible = "sgx,vz89te", .data = &vz89x_chips[VZ89TE] },
> >  	{ }
> >  };
> >  MODULE_DEVICE_TABLE(of, vz89x_dt_ids);
> >
> >  static int vz89x_probe(struct i2c_client *client)  {
> > -	const struct i2c_device_id *id = i2c_client_get_device_id(client);
> >  	struct device *dev = &client->dev;
> >  	struct iio_dev *indio_dev;
> >  	struct vz89x_data *data;
> > -	int chip_id;
> >
> >  	indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
> >  	if (!indio_dev)
> > @@ -369,14 +367,10 @@ static int vz89x_probe(struct i2c_client *client)
> >  	else
> >  		return -EOPNOTSUPP;
> >
> > -	if (!dev_fwnode(dev))
> > -		chip_id = id->driver_data;
> > -	else
> > -		chip_id = (unsigned long)device_get_match_data(dev);
> > +	data->chip = i2c_get_match_data(client);
> >
> >  	i2c_set_clientdata(client, indio_dev);
> >  	data->client = client;
> > -	data->chip = &vz89x_chips[chip_id];
> >  	data->last_update = jiffies - HZ;
> >  	mutex_init(&data->lock);
> >
> > @@ -391,8 +385,8 @@ static int vz89x_probe(struct i2c_client *client)
> > }
> >
> >  static const struct i2c_device_id vz89x_id[] = {
> > -	{ "vz89x", VZ89X },
> > -	{ "vz89te", VZ89TE },
> > +	{ "vz89x", (kernel_ulong_t)&vz89x_chips[VZ89X] },
> > +	{ "vz89te", (kernel_ulong_t)&vz89x_chips[VZ89TE] },
> >  	{ }
> >  };
> >  MODULE_DEVICE_TABLE(i2c, vz89x_id);
Andy Shevchenko Aug. 28, 2023, 12:56 p.m. UTC | #3
On Mon, Aug 28, 2023 at 12:43:51PM +0000, Biju Das wrote:
> > On Fri, 18 Aug 2023 20:04:29 +0100
> > Biju Das <biju.das.jz@bp.renesas.com> wrote:

> > Make sure you cc the driver authors etc.
> 
> Normally, I ran a script against the patch to get details and additionally I
> add Geert, renesas-soc , Andy in Cc list.

I use [1] for the submissions and seems in 99.9% it works just as expected.
Can you compare the difference in the heuristics?

[1]: https://github.com/andy-shev/home-bin-tools/blob/master/ge2maintainer.sh
Geert Uytterhoeven Aug. 28, 2023, 1:43 p.m. UTC | #4
Hi Biju,

On Mon, Aug 28, 2023 at 2:43 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > Subject: Re: [PATCH v2] iio: chemical: vz89x: Convert enum->pointer for
> > data in the match tables
> >
> > On Fri, 18 Aug 2023 20:04:29 +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.
> > >
> > > Replace enum->struct *vz89x_chip_data for data in the match table.
> > > Simplify the probe() 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>
> > > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> >
> > Biju,
> >
> > Make sure you cc the driver authors etc.
>
> Normally, I ran a script against the patch to get details and additionally I add Geert, renesas-soc , Andy in Cc list.

I don't think there is a need to add renesas-soc (or me, FWIW ;-), unless
the specific device is used on Renesas platforms.

Gr{oetje,eeting}s,

                        Geert
Biju Das Aug. 28, 2023, 2:05 p.m. UTC | #5
Hi Geert Uytterhoeven,

> Subject: Re: [PATCH v2] iio: chemical: vz89x: Convert enum->pointer for
> data in the match tables
> 
> Hi Biju,
> 
> On Mon, Aug 28, 2023 at 2:43 PM Biju Das <biju.das.jz@bp.renesas.com>
> wrote:
> > > Subject: Re: [PATCH v2] iio: chemical: vz89x: Convert enum->pointer
> > > for data in the match tables
> > >
> > > On Fri, 18 Aug 2023 20:04:29 +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.
> > > >
> > > > Replace enum->struct *vz89x_chip_data for data in the match table.
> > > > Simplify the probe() 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>
> > > > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > >
> > > Biju,
> > >
> > > Make sure you cc the driver authors etc.
> >
> > Normally, I ran a script against the patch to get details and
> additionally I add Geert, renesas-soc , Andy in Cc list.
> 
> I don't think there is a need to add renesas-soc (or me, FWIW ;-), unless
> the specific device is used on Renesas platforms.

Thanks for letting me know. Sorry for the noise.

Cheers,
Biju
diff mbox series

Patch

diff --git a/drivers/iio/chemical/vz89x.c b/drivers/iio/chemical/vz89x.c
index 13555f4f401a..5b358bcd311b 100644
--- a/drivers/iio/chemical/vz89x.c
+++ b/drivers/iio/chemical/vz89x.c
@@ -342,19 +342,17 @@  static const struct vz89x_chip_data vz89x_chips[] = {
 };
 
 static const struct of_device_id vz89x_dt_ids[] = {
-	{ .compatible = "sgx,vz89x", .data = (void *) VZ89X },
-	{ .compatible = "sgx,vz89te", .data = (void *) VZ89TE },
+	{ .compatible = "sgx,vz89x", .data = &vz89x_chips[VZ89X] },
+	{ .compatible = "sgx,vz89te", .data = &vz89x_chips[VZ89TE] },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, vz89x_dt_ids);
 
 static int vz89x_probe(struct i2c_client *client)
 {
-	const struct i2c_device_id *id = i2c_client_get_device_id(client);
 	struct device *dev = &client->dev;
 	struct iio_dev *indio_dev;
 	struct vz89x_data *data;
-	int chip_id;
 
 	indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
 	if (!indio_dev)
@@ -369,14 +367,10 @@  static int vz89x_probe(struct i2c_client *client)
 	else
 		return -EOPNOTSUPP;
 
-	if (!dev_fwnode(dev))
-		chip_id = id->driver_data;
-	else
-		chip_id = (unsigned long)device_get_match_data(dev);
+	data->chip = i2c_get_match_data(client);
 
 	i2c_set_clientdata(client, indio_dev);
 	data->client = client;
-	data->chip = &vz89x_chips[chip_id];
 	data->last_update = jiffies - HZ;
 	mutex_init(&data->lock);
 
@@ -391,8 +385,8 @@  static int vz89x_probe(struct i2c_client *client)
 }
 
 static const struct i2c_device_id vz89x_id[] = {
-	{ "vz89x", VZ89X },
-	{ "vz89te", VZ89TE },
+	{ "vz89x", (kernel_ulong_t)&vz89x_chips[VZ89X] },
+	{ "vz89te", (kernel_ulong_t)&vz89x_chips[VZ89TE] },
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, vz89x_id);