On Tue, 2020-03-17 at 09:51 -0500, Michael Auchter wrote:
> [External]
>
> There are no in-tree users of the platform data for this driver, so
> remove it and convert the driver to use device tree instead.
>
A few comments inline for this.
Sorry if this is a bit late, but since there will be a V3 based on the DT
bindings patch, it's still not too late.
> Signed-off-by: Michael Auchter <michael.auchter@xxxxxx>
> ---
>
> Changes since v1:
> - Fix regulator handling as suggested by Lars
>
> drivers/iio/adc/ad7291.c | 33 ++++++++++++++++------------
> include/linux/platform_data/ad7291.h | 13 -----------
> 2 files changed, 19 insertions(+), 27 deletions(-)
> delete mode 100644 include/linux/platform_data/ad7291.h
>
> diff --git a/drivers/iio/adc/ad7291.c b/drivers/iio/adc/ad7291.c
> index b2b137fed246..43a201fb4f34 100644
> --- a/drivers/iio/adc/ad7291.c
> +++ b/drivers/iio/adc/ad7291.c
> @@ -20,8 +20,6 @@
> #include <linux/iio/sysfs.h>
> #include <linux/iio/events.h>
>
> -#include <linux/platform_data/ad7291.h>
> -
> /*
> * Simplified handling
> *
> @@ -465,7 +463,6 @@ static const struct iio_info ad7291_info = {
> static int ad7291_probe(struct i2c_client *client,
> const struct i2c_device_id *id)
> {
> - struct ad7291_platform_data *pdata = client->dev.platform_data;
> struct ad7291_chip_info *chip;
> struct iio_dev *indio_dev;
> int ret;
> @@ -475,16 +472,6 @@ static int ad7291_probe(struct i2c_client *client,
> return -ENOMEM;
> chip = iio_priv(indio_dev);
>
> - if (pdata && pdata->use_external_ref) {
> - chip->reg = devm_regulator_get(&client->dev, "vref");
> - if (IS_ERR(chip->reg))
> - return PTR_ERR(chip->reg);
> -
> - ret = regulator_enable(chip->reg);
> - if (ret)
> - return ret;
> - }
> -
> mutex_init(&chip->state_lock);
> /* this is only used for device removal purposes */
> i2c_set_clientdata(client, indio_dev);
> @@ -495,8 +482,19 @@ static int ad7291_probe(struct i2c_client *client,
> AD7291_T_SENSE_MASK | /* Tsense always enabled */
> AD7291_ALERT_POLARITY; /* set irq polarity low level */
>
> - if (pdata && pdata->use_external_ref)
> + chip->reg = devm_regulator_get_optional(&client->dev, "vref");
> + if (!IS_ERR(chip->reg)) {
> + ret = regulator_enable(chip->reg);
> + if (ret)
> + return ret;
> +
> chip->command |= AD7291_EXT_REF;
> + } else {
> + if (PTR_ERR(chip->reg) != -ENODEV)
> + return PTR_ERR(chip->reg);
> +
> + chip->reg = NULL;
> + }
>
> indio_dev->name = id->name;
> indio_dev->channels = ad7291_channels;
> @@ -569,9 +567,16 @@ static const struct i2c_device_id ad7291_id[] = {
>
> MODULE_DEVICE_TABLE(i2c, ad7291_id);
>
> +static const struct of_device_id ad7291_of_match[] = {
> + { .compatible = "adi,ad7291", },
> + {},
[2]
if updating [1], maybe remove the trailing comma for the null-terminator;
so, {}, -> {}
not a biggy, but there are chances that someone else might complain about it
before Jonathan gets a chance to look over this;
> +};
> +MODULE_DEVICE_TABLE(of, ad7291_of_match);
> +
> static struct i2c_driver ad7291_driver = {
> .driver = {
> .name = KBUILD_MODNAME,
> + .of_match_table = of_match_ptr(ad7291_of_match),
[1]
not sure if there was a comment about this, but I'd remove the of_match_ptr()
and just assign this directly;
i.e. .of_match_table = ad7297_of_match,
there is some code that can also handle this OF-table for ACPI as well; I don't
know if this is sufficient for ACPI [with this patch], but it's a good idea to
prepare for that;
> },
> .probe = ad7291_probe,
> .remove = ad7291_remove,
> diff --git a/include/linux/platform_data/ad7291.h
> b/include/linux/platform_data/ad7291.h
> deleted file mode 100644
> index b1fd1530c9a5..000000000000
> --- a/include/linux/platform_data/ad7291.h
> +++ /dev/null
> @@ -1,13 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0 */
> -#ifndef __IIO_AD7291_H__
> -#define __IIO_AD7291_H__
> -
> -/**
> - * struct ad7291_platform_data - AD7291 platform data
> - * @use_external_ref: Whether to use an external or internal reference
> voltage
> - */
> -struct ad7291_platform_data {
> - bool use_external_ref;
> -};
> -
> -#endif
