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.
Signed-off-by: Michael Auchter <[email protected]>
---
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..536e31862309 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)) {
+ if (PTR_ERR(chip->reg) == -EPROBE_DEFER)
+ return -EPROBE_DEFER;
+
+ chip->reg = NULL;
+ } else {
+ ret = regulator_enable(chip->reg);
+ if (ret)
+ return ret;
+
chip->command |= AD7291_EXT_REF;
+ }
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", },
+ {},
+};
+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),
},
.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
--
2.24.1
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.
Signed-off-by: Michael Auchter <[email protected]>
---
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", },
+ {},
+};
+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),
},
.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
--
2.24.1
Add device-tree binding for ADI AD7291 ADC.
Signed-off-by: Michael Auchter <[email protected]>
---
.../bindings/iio/adc/adi,ad7291.yaml | 50 +++++++++++++++++++
1 file changed, 50 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad7291.yaml
diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7291.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7291.yaml
new file mode 100644
index 000000000000..93aa29413049
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7291.yaml
@@ -0,0 +1,50 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/adc/adi,ad7291.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: AD7291 8-Channel, I2C, 12-Bit SAR ADC with Temperature Sensor
+
+maintainers:
+ - Michael Auchter <[email protected]>
+
+description: |
+ Analog Devices AD7291 8-Channel I2C 12-Bit SAR ADC with Temperature Sensor
+ https://www.analog.com/media/en/technical-documentation/data-sheets/ad7291.pdf
+
+properties:
+ compatible:
+ enum:
+ - adi,ad7291
+
+ reg:
+ maxItems: 1
+
+ vref-supply:
+ description: |
+ The regulator supply for ADC reference voltage.
+
+ '#address-cells':
+ const: 1
+
+ '#size-cells':
+ const: 0
+
+required:
+ - compatible
+ - reg
+
+examples:
+ - |
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ ad7291: adc@0 {
+ compatible = "adi,ad7291";
+ reg = <0>;
+ vref-supply = <&adc_vref>;
+ };
+ };
+
--
2.24.1
Add device-tree binding for ADI AD7291 ADC.
Signed-off-by: Michael Auchter <[email protected]>
---
.../bindings/iio/adc/adi,ad7291.yaml | 50 +++++++++++++++++++
1 file changed, 50 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad7291.yaml
diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7291.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7291.yaml
new file mode 100644
index 000000000000..93aa29413049
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7291.yaml
@@ -0,0 +1,50 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/adc/adi,ad7291.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: AD7291 8-Channel, I2C, 12-Bit SAR ADC with Temperature Sensor
+
+maintainers:
+ - Michael Auchter <[email protected]>
+
+description: |
+ Analog Devices AD7291 8-Channel I2C 12-Bit SAR ADC with Temperature Sensor
+ https://www.analog.com/media/en/technical-documentation/data-sheets/ad7291.pdf
+
+properties:
+ compatible:
+ enum:
+ - adi,ad7291
+
+ reg:
+ maxItems: 1
+
+ vref-supply:
+ description: |
+ The regulator supply for ADC reference voltage.
+
+ '#address-cells':
+ const: 1
+
+ '#size-cells':
+ const: 0
+
+required:
+ - compatible
+ - reg
+
+examples:
+ - |
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ ad7291: adc@0 {
+ compatible = "adi,ad7291";
+ reg = <0>;
+ vref-supply = <&adc_vref>;
+ };
+ };
+
--
2.24.1
On 3/17/20 3:51 PM, Michael Auchter wrote:
> 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.
>
> Signed-off-by: Michael Auchter <[email protected]>
Reviewed-by: Lars-Peter Clausen <[email protected]>
On Tue, Mar 17, 2020 at 4:53 PM Michael Auchter <[email protected]> wrote:
>
> 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.
...
> + chip->reg = devm_regulator_get_optional(&client->dev, "vref");
> + if (!IS_ERR(chip->reg)) {
Why not to go with usual positive conditional?
> + 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;
> + }
...
> +static const struct of_device_id ad7291_of_match[] = {
> + { .compatible = "adi,ad7291", },
> + {},
No need for comma.
> +};
...
> + .of_match_table = of_match_ptr(ad7291_of_match),
No need to use of_match_ptr(). Haven't you got a compiler warning in !OF case?
--
With Best Regards,
Andy Shevchenko
Hello Andy,
Thanks for the review!
On Sun, Mar 22, 2020 at 01:46:21AM +0200, Andy Shevchenko wrote:
> On Tue, Mar 17, 2020 at 4:53 PM Michael Auchter <[email protected]> wrote:
> >
> > 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.
>
> ...
>
> > + chip->reg = devm_regulator_get_optional(&client->dev, "vref");
> > + if (!IS_ERR(chip->reg)) {
>
> Why not to go with usual positive conditional?
I took this pattern from ad7266.c which Lars pointed me to. I agree that
a positive conditional here would probably be more natural. I'll change
that if you'd prefer.
> > + 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;
> > + }
>
> ...
>
> > +static const struct of_device_id ad7291_of_match[] = {
> > + { .compatible = "adi,ad7291", },
>
> > + {},
>
> No need for comma.
Indeed, I'll drop it.
>
> > +};
>
> ...
>
> > + .of_match_table = of_match_ptr(ad7291_of_match),
>
> No need to use of_match_ptr(). Haven't you got a compiler warning in !OF case?
Hm, no warning as far as I can see with !OF... but agreed that this
doesn't make much sense as-is.
Is dropping of_match_ptr() the preferred route here? The driver doesn't
depend on OF, so it seems like keeping of_match_ptr and instead guarding
the ad7291_of_match table with #ifdef CONFIG_OF would be preferred. Of
course, maybe that's not worth it for saving some bytes from the final
image.
Let me know which route would be preferred.
Thanks again,
Michael
On Tue, 17 Mar 2020 09:51:13 -0500, Michael Auchter wrote:
> Add device-tree binding for ADI AD7291 ADC.
>
> Signed-off-by: Michael Auchter <[email protected]>
> ---
> .../bindings/iio/adc/adi,ad7291.yaml | 50 +++++++++++++++++++
> 1 file changed, 50 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad7291.yaml
>
Reviewed-by: Rob Herring <[email protected]>
On Mon, Mar 30, 2020 at 11:12 PM Michael Auchter <[email protected]> wrote:
> On Sun, Mar 22, 2020 at 01:46:21AM +0200, Andy Shevchenko wrote:
> > On Tue, Mar 17, 2020 at 4:53 PM Michael Auchter <[email protected]> wrote:
...
> > > + chip->reg = devm_regulator_get_optional(&client->dev, "vref");
> > > + if (!IS_ERR(chip->reg)) {
> >
> > Why not to go with usual positive conditional?
>
> I took this pattern from ad7266.c which Lars pointed me to. I agree that
> a positive conditional here would probably be more natural. I'll change
> that if you'd prefer.
Yes, please do.
...
> > > + .of_match_table = of_match_ptr(ad7291_of_match),
> >
> > No need to use of_match_ptr(). Haven't you got a compiler warning in !OF case?
>
> Hm, no warning as far as I can see with !OF...
Have you used `make W=1 ...`? With it you should get a warning that
table defined but not used.
> but agreed that this
> doesn't make much sense as-is.
>
> Is dropping of_match_ptr() the preferred route here? The driver doesn't
> depend on OF, so it seems like keeping of_match_ptr and instead guarding
> the ad7291_of_match table with #ifdef CONFIG_OF would be preferred. Of
> course, maybe that's not worth it for saving some bytes from the final
> image.
You need either both (of_match_ptr() _and_ ugly ifdeffery, and note
you will need of.h for that) or none (mod_devicetable.h maybe needed,
though).
> Let me know which route would be preferred.
If we would like to use this in non-DT environment, then drop all that
OF-specific stuff.
--
With Best Regards,
Andy Shevchenko
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 <[email protected]>
> ---
>
> 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