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>,
"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 v2] iio: accel: adxl345: Convert enum->pointer for data in match data table
Date: Tue, 29 Aug 2023 14:31:46 +0000 [thread overview]
Message-ID: <OS0PR01MB5922770083CB801AD06FC41986E7A@OS0PR01MB5922.jpnprd01.prod.outlook.com> (raw)
In-Reply-To: <20230828124817.2622d85a@jic23-huawei>
Hi Jonathan Cameron,
Thanks for the feedback.
> Subject: Re: [PATCH v2] iio: accel: adxl345: Convert enum->pointer for data
> in match data table
>
> On Fri, 18 Aug 2023 19:12:29 +0100
> Biju Das <biju.das.jz@bp.renesas.com> wrote:
>
> > Convert enum->pointer for data in match data table, so that
> > device_get_match_data() can do match against OF/ACPI/I2C tables, once
> > i2c bus type match support added to it.
> >
> > Add struct adxl3x5_chip_info and replace enum->adxl3x5_chip_info in
> > the match table and simplify adxl345_probe().
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
>
> Hi Biju.
>
> The enum is used for very few purposes in this driver.
> Whilst this patch is a good step in the right direction I'd like to see the
> enum go away entirely by adding the remaining chip specific data to the new
> chip_info structure.
> I think that's just one scale value :)
OK.
>
> Jonathan
>
> > ---
> > v1->v2:
> > * Replaced EINVAL->ENODEV for invaild chip type.
> > * Kept leading commas for adxl345_*_info and adxl375_*_info.
> > * Restored switch statement in adxl345_core_probe()
> > ---
> > drivers/iio/accel/adxl345.h | 5 +++++
> > drivers/iio/accel/adxl345_core.c | 16 ++++++----------
> > drivers/iio/accel/adxl345_i2c.c | 20 +++++++++++++++-----
> > drivers/iio/accel/adxl345_spi.c | 20 +++++++++++++++-----
> > 4 files changed, 41 insertions(+), 20 deletions(-)
> >
> > diff --git a/drivers/iio/accel/adxl345.h b/drivers/iio/accel/adxl345.h
> > index d7e67cb08538..8df1b7f43cb9 100644
> > --- a/drivers/iio/accel/adxl345.h
> > +++ b/drivers/iio/accel/adxl345.h
> > @@ -13,6 +13,11 @@ enum adxl345_device_type {
> > ADXL375 = 2,
> > };
> >
> > +struct adxl3x5_chip_info {
> No wild cards in naming. I almost always ends up being a problem in the
> long run.
> adxl345_chip_info is fine as it's in the adxl345 file.
OK, will change it.
Cheers,
Biju
>
> > + const char *name;
> > + unsigned int type;
> > +};
> > +
> > int adxl345_core_probe(struct device *dev, struct regmap *regmap);
> >
> > #endif /* _ADXL345_H_ */
> > diff --git a/drivers/iio/accel/adxl345_core.c
> > b/drivers/iio/accel/adxl345_core.c
> > index 1919e0089c11..810048099ba9 100644
> > --- a/drivers/iio/accel/adxl345_core.c
> > +++ b/drivers/iio/accel/adxl345_core.c
> > @@ -222,23 +222,19 @@ static void adxl345_powerdown(void *regmap)
> >
> > int adxl345_core_probe(struct device *dev, struct regmap *regmap) {
> > - enum adxl345_device_type type;
> > + const struct adxl3x5_chip_info *info;
> > struct adxl345_data *data;
> > struct iio_dev *indio_dev;
> > - const char *name;
> > u32 regval;
> > int ret;
> >
> > - type = (uintptr_t)device_get_match_data(dev);
> > - switch (type) {
> > + info = device_get_match_data(dev);
> > + switch (info->type) {
> > case ADXL345:
> > - name = "adxl345";
> > - break;
> > case ADXL375:
> > - name = "adxl375";
> > break;
> > default:
> > - return -EINVAL;
> > + return -ENODEV;
> > }
> >
> > ret = regmap_read(regmap, ADXL345_REG_DEVID, ®val); @@ -255,7
> > +251,7 @@ int adxl345_core_probe(struct device *dev, struct regmap
> > *regmap)
> >
> > data = iio_priv(indio_dev);
> > data->regmap = regmap;
> > - data->type = type;
> > + data->type = info->type;
> > /* Enable full-resolution mode */
> > data->data_range = ADXL345_DATA_FORMAT_FULL_RES;
> >
> > @@ -264,7 +260,7 @@ int adxl345_core_probe(struct device *dev, struct
> regmap *regmap)
> > if (ret < 0)
> > return dev_err_probe(dev, ret, "Failed to set data range\n");
> >
> > - indio_dev->name = name;
> > + indio_dev->name = info->name;
> > indio_dev->info = &adxl345_info;
> > indio_dev->modes = INDIO_DIRECT_MODE;
> > indio_dev->channels = adxl345_channels; diff --git
> > a/drivers/iio/accel/adxl345_i2c.c b/drivers/iio/accel/adxl345_i2c.c
> > index e47d12f19602..219de556e81a 100644
> > --- a/drivers/iio/accel/adxl345_i2c.c
> > +++ b/drivers/iio/accel/adxl345_i2c.c
> > @@ -30,22 +30,32 @@ static int adxl345_i2c_probe(struct i2c_client
> *client)
> > return adxl345_core_probe(&client->dev, regmap); }
> >
> > +static const struct adxl3x5_chip_info adxl345_i2c_info = {
> > + .name = "adxl345",
> > + .type = ADXL345,
> > +};
> > +
> > +static const struct adxl3x5_chip_info adxl375_i2c_info = {
> > + .name = "adxl375",
> > + .type = ADXL375,
> > +};
> > +
> > static const struct i2c_device_id adxl345_i2c_id[] = {
> > - { "adxl345", ADXL345 },
> > - { "adxl375", ADXL375 },
> > + { "adxl345", (kernel_ulong_t)&adxl345_i2c_info },
> > + { "adxl375", (kernel_ulong_t)&adxl375_i2c_info },
> > { }
> > };
> > MODULE_DEVICE_TABLE(i2c, adxl345_i2c_id);
> >
> > static const struct of_device_id adxl345_of_match[] = {
> > - { .compatible = "adi,adxl345", .data = (const void *)ADXL345 },
> > - { .compatible = "adi,adxl375", .data = (const void *)ADXL375 },
> > + { .compatible = "adi,adxl345", .data = &adxl345_i2c_info },
> > + { .compatible = "adi,adxl375", .data = &adxl375_i2c_info },
> > { }
> > };
> > MODULE_DEVICE_TABLE(of, adxl345_of_match);
> >
> > static const struct acpi_device_id adxl345_acpi_match[] = {
> > - { "ADS0345", ADXL345 },
> > + { "ADS0345", (kernel_ulong_t)&adxl345_i2c_info },
> > { }
> > };
> > MODULE_DEVICE_TABLE(acpi, adxl345_acpi_match); diff --git
> > a/drivers/iio/accel/adxl345_spi.c b/drivers/iio/accel/adxl345_spi.c
> > index aaade5808657..3acdacc07293 100644
> > --- a/drivers/iio/accel/adxl345_spi.c
> > +++ b/drivers/iio/accel/adxl345_spi.c
> > @@ -36,22 +36,32 @@ static int adxl345_spi_probe(struct spi_device *spi)
> > return adxl345_core_probe(&spi->dev, regmap); }
> >
> > +static const struct adxl3x5_chip_info adxl345_spi_info = {
> > + .name = "adxl345",
> > + .type = ADXL345,
> > +};
> > +
> > +static const struct adxl3x5_chip_info adxl375_spi_info = {
> > + .name = "adxl375",
> > + .type = ADXL375,
> > +};
> > +
> > static const struct spi_device_id adxl345_spi_id[] = {
> > - { "adxl345", ADXL345 },
> > - { "adxl375", ADXL375 },
> > + { "adxl345", (kernel_ulong_t)&adxl345_spi_info },
> > + { "adxl375", (kernel_ulong_t)&adxl375_spi_info },
> > { }
> > };
> > MODULE_DEVICE_TABLE(spi, adxl345_spi_id);
> >
> > static const struct of_device_id adxl345_of_match[] = {
> > - { .compatible = "adi,adxl345", .data = (const void *)ADXL345 },
> > - { .compatible = "adi,adxl375", .data = (const void *)ADXL375 },
> > + { .compatible = "adi,adxl345", .data = &adxl345_spi_info },
> > + { .compatible = "adi,adxl375", .data = &adxl375_spi_info },
> > { }
> > };
> > MODULE_DEVICE_TABLE(of, adxl345_of_match);
> >
> > static const struct acpi_device_id adxl345_acpi_match[] = {
> > - { "ADS0345", ADXL345 },
> > + { "ADS0345", (kernel_ulong_t)&adxl345_spi_info },
> > { }
> > };
> > MODULE_DEVICE_TABLE(acpi, adxl345_acpi_match);
next prev parent reply other threads:[~2023-08-29 14:32 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-08-18 18:12 [PATCH v2] iio: accel: adxl345: Convert enum->pointer for data in match data table Biju Das
2023-08-28 11:48 ` Jonathan Cameron
2023-08-29 14:31 ` Biju Das [this message]
2023-08-28 11:50 ` Jonathan Cameron
2023-08-29 14:32 ` Biju Das
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=OS0PR01MB5922770083CB801AD06FC41986E7A@OS0PR01MB5922.jpnprd01.prod.outlook.com \
--to=biju.das.jz@bp.renesas.com \
--cc=Michael.Hennerich@analog.com \
--cc=geert+renesas@glider.be \
--cc=jic23@kernel.org \
--cc=lars@metafoo.de \
--cc=linux-iio@vger.kernel.org \
--cc=linux-renesas-soc@vger.kernel.org \
--cc=prabhakar.mahadev-lad.rj@bp.renesas.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.