| Message ID | 20230716175218.130557-3-biju.das.jz@bp.renesas.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Use i2c_get_match_data() | expand |
On Sun, Jul 16, 2023 at 7:52 PM 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> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> Gr{oetje,eeting}s, Geert
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? Thanks, Jonathan
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
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]);
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(-)