RE: [PATCH 2/4] iio: accel: adxl313: Use i2c_get_match_data

From: Biju Das <biju.das.jz@bp.renesas.com>
To: Jonathan Cameron <jic23@kernel.org>
Cc: Lars-Peter Clausen <lars@metafoo.de>,
	Michael Hennerich <Michael.Hennerich@analog.com>,
	Lucas Stankus <lucas.p.stankus@gmail.com>,
	"linux-iio@vger.kernel.org" <linux-iio@vger.kernel.org>,
	Geert Uytterhoeven <geert+renesas@glider.be>,
	Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@bp.renesas.com>,
	"linux-renesas-soc@vger.kernel.org" 
	<linux-renesas-soc@vger.kernel.org>
Subject: RE: [PATCH 2/4] iio: accel: adxl313: Use i2c_get_match_data
Date: Sat, 22 Jul 2023 17:37:46 +0000	[thread overview]
Message-ID: <OS0PR01MB5922D90733BD82FF6D3F62CB863CA@OS0PR01MB5922.jpnprd01.prod.outlook.com> (raw)
In-Reply-To: <20230722172832.04ad7738@jic23-huawei>

Hi Jonathan Cameron,

> -----Original Message-----
> From: Jonathan Cameron <jic23@kernel.org>
> Sent: Saturday, July 22, 2023 5:29 PM
> To: Biju Das <biju.das.jz@bp.renesas.com>
> Cc: Lars-Peter Clausen <lars@metafoo.de>; Michael Hennerich
> <Michael.Hennerich@analog.com>; Lucas Stankus
> <lucas.p.stankus@gmail.com>; linux-iio@vger.kernel.org; Geert
> Uytterhoeven <geert+renesas@glider.be>; Prabhakar Mahadev Lad
> <prabhakar.mahadev-lad.rj@bp.renesas.com>; linux-renesas-
> soc@vger.kernel.org
> Subject: Re: [PATCH 2/4] iio: accel: adxl313: Use i2c_get_match_data
> 
> On Sun, 16 Jul 2023 18:52:16 +0100
> Biju Das <biju.das.jz@bp.renesas.com> wrote:
> 
> > Replace of_device_get_match_data() and i2c_match_id() by i2c_get_match
> > _data() as we have similar I2C and DT-based matching table.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > ---
> >  drivers/iio/accel/adxl313_i2c.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/iio/accel/adxl313_i2c.c
> > b/drivers/iio/accel/adxl313_i2c.c index 524327ea3663..6d252190207b
> > 100644
> > --- a/drivers/iio/accel/adxl313_i2c.c
> > +++ b/drivers/iio/accel/adxl313_i2c.c
> > @@ -65,9 +65,9 @@ static int adxl313_i2c_probe(struct i2c_client
> *client)
> >  	 * Retrieves device specific data as a pointer to a
> >  	 * adxl313_chip_info structure
> >  	 */
> > -	chip_data = device_get_match_data(&client->dev);
> > +	chip_data = i2c_get_match_data(client);
> >  	if (!chip_data)
> > -		chip_data = (const struct adxl313_chip_info
> *)i2c_match_id(adxl313_i2c_id, client)->driver_data;
> > +		return -ENODEV;
> >
> >  	regmap = devm_regmap_init_i2c(client,
> >  				      &adxl31x_i2c_regmap_config[chip_data-
> >type]);
> 
> This driver looks buggy:
> 
> static const struct i2c_device_id adxl313_i2c_id[] = {
> 	{ .name = "adxl312", .driver_data =
> (kernel_ulong_t)&adxl31x_chip_info[ADXL312] },
> 	{ .name = "adxl313", .driver_data =
> (kernel_ulong_t)&adxl31x_chip_info[ADXL312] },
> 	{ .name = "adxl314", .driver_data =
> (kernel_ulong_t)&adxl31x_chip_info[ADXL312] },
> 	{ }
> };
> 
> MODULE_DEVICE_TABLE(i2c, adxl313_i2c_id);
> 
> static const struct of_device_id adxl313_of_match[] = {
> 	{ .compatible = "adi,adxl312", .data = &adxl31x_chip_info[ADXL312]
> },
> 	{ .compatible = "adi,adxl313", .data = &adxl31x_chip_info[ADXL313]
> },
> 	{ .compatible = "adi,adxl314", .data = &adxl31x_chip_info[ADXL314]
> },
> 	{ }
> };
> 
> Odd that the i2c_device_id table always uses ADXL312
> 
> That would only have previously applied if we failed to match on the of
> one (which only happens in somewhat obscure cases)
> 
> Can we fix that first then apply this cleanup on top?

Sure. Will send V2.

Cheers,
Biju