lkml.org 
[lkml]   [2020]   [Mar]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    SubjectRe: [PATCH v2 1/2] iio: adc: ad7291: convert to device tree
    Date
    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@ni.com>
    > ---
    >
    > 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
    \
     
     \ /
      Last update: 2020-03-31 13:11    [W:4.469 / U:0.088 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site