[PATCH v2 1/4] iio: adc: ad7887: convert dual-channel mode to DT/ACPI

This change converts the configuration of the dual-channel mode from the
old platform-data, to the device_property_present() function, which
supports both device-tree and ACPI configuration setups.

With this change the old platform_data include of the driver can be
removed.

Signed-off-by: Alexandru Ardelean <redacted>
---

I'm wondering if this changeset is what was in mind here:
 https://lore.kernel.org/linux-iio/CA+U=DsqF5tu8Be9KXeyCWD2uHvV688Nc3n=z_Xi2J6H6DFJPRQ@mail.gmail.com/T/#mbe72e4da3acea3899d0d35402ea81e52a9bc34e6
This driver could have been simplified/reduced a whole lot more, but I'm
not sure about it. It's a bit of patch-noise, and later

Changelog v1 -> v2:
* dropped patch 'iio: adc: ad7887: convert driver to full DT probing'
  not adding the device_get_match_data() logic anymore
* added patch 'iio: adc: ad7887: remove matching code from driver'
  hooking the chip info directly to AD7887
* added patch 'iio: adc: ad7887: add OF match table'
  this just adds an OF table for DT and ACPI

 drivers/iio/adc/ad7887.c             | 10 +++++-----
 include/linux/platform_data/ad7887.h | 21 ---------------------
 2 files changed, 5 insertions(+), 26 deletions(-)
 delete mode 100644 include/linux/platform_data/ad7887.h

diff --git a/drivers/iio/adc/ad7887.c b/drivers/iio/adc/ad7887.c
index 4f6f0e0e03ee..06f684c053a0 100644
--- a/drivers/iio/adc/ad7887.c
+++ b/drivers/iio/adc/ad7887.c

@@ -23,8 +23,6 @@
 #include <linux/iio/trigger_consumer.h>
 #include <linux/iio/triggered_buffer.h>
 
-#include <linux/platform_data/ad7887.h>
-
 #define AD7887_REF_DIS		BIT(5)	/* on-chip reference disable */
 #define AD7887_DUAL		BIT(4)	/* dual-channel mode */
 #define AD7887_CH_AIN1		BIT(3)	/* convert on channel 1, DUAL=1 */

@@ -241,9 +239,9 @@ static void ad7887_reg_disable(void *data)
 
 static int ad7887_probe(struct spi_device *spi)
 {
-	struct ad7887_platform_data *pdata = spi->dev.platform_data;
 	struct ad7887_state *st;
 	struct iio_dev *indio_dev;
+	bool dual_mode;
 	uint8_t mode;
 	int ret;
 

@@ -286,7 +284,9 @@ static int ad7887_probe(struct spi_device *spi)
 	mode = AD7887_PM_MODE4;
 	if (!st->reg)
 		mode |= AD7887_REF_DIS;
-	if (pdata && pdata->en_dual)
+
+	dual_mode = device_property_present(&spi->dev, "adi,dual-channel-mode");
+	if (dual_mode)
 		mode |= AD7887_DUAL;
 
 	st->tx_cmd_buf[0] = AD7887_CH_AIN0 | mode;

@@ -298,7 +298,7 @@ static int ad7887_probe(struct spi_device *spi)
 	spi_message_init(&st->msg[AD7887_CH0]);
 	spi_message_add_tail(&st->xfer[0], &st->msg[AD7887_CH0]);
 
-	if (pdata && pdata->en_dual) {
+	if (dual_mode) {
 		st->tx_cmd_buf[2] = AD7887_CH_AIN1 | mode;
 
 		st->xfer[1].rx_buf = &st->data[0];

diff --git a/include/linux/platform_data/ad7887.h b/include/linux/platform_data/ad7887.h
deleted file mode 100644
index 9b4dca6ae70b..000000000000
--- a/include/linux/platform_data/ad7887.h
+++ /dev/null

@@ -1,21 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-or-later */
-/*
- * AD7887 SPI ADC driver
- *
- * Copyright 2010 Analog Devices Inc.
- */
-#ifndef IIO_ADC_AD7887_H_
-#define IIO_ADC_AD7887_H_
-
-/**
- * struct ad7887_platform_data - AD7887 ADC driver platform data
- * @en_dual: Whether to use dual channel mode. If set to true AIN1 becomes the
- *	second input channel, and Vref is internally connected to Vdd. If set to
- *	false the device is used in single channel mode and AIN1/Vref is used as
- *	VREF input.
- */
-struct ad7887_platform_data {
-	bool en_dual;
-};
-
-#endif /* IIO_ADC_AD7887_H_ */

-- 
2.17.1

This change adds a simple device-tree binding for thhe Analog Devices
AD7887 ADC.

Signed-off-by: Alexandru Ardelean <redacted>
---
 .../bindings/iio/adc/adi,ad7887.yaml          | 70 +++++++++++++++++++
 1 file changed, 70 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad7887.yaml

diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7887.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7887.yaml
new file mode 100644
index 000000000000..9b30f4569b4e
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7887.yaml

@@ -0,0 +1,70 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/adc/adi,ad7887.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices AD7887 low power, 12-bit ADC
+
+maintainers:
+  - Michael Hennerich <michael.hennerich@analog.com>
+
+description: |
+  Analog Devices AD7887 low power, 12-bit analog-to-digital converter (ADC)
+  that operates from a single 2.7 V to 5.25 V power supply.
+
+properties:
+  compatible:
+    enum:
+      - adi,ad7887
+
+  reg:
+    maxItems: 1
+
+  spi-cpha: true
+
+  spi-cpol: true
+
+  avcc-supply: true
+
+  spi-max-frequency: true
+
+  vref-supply:
+    description:
+      ADC reference voltage supply
+
+  adi,dual-channel-mode:
+    description:
+      Configures dual-channel mode for the ADC. In dual-channel operation,
+      the AIN1/VREF pin assumes its AIN1 function, providing a second analog
+      input channel. In this case, he reference voltage for the part is provided
+      via the VDD pin. As a result, the input voltage range on both the AIN0 and
+      AIN1 inputs is 0 to VDD.
+    type: boolean
+
+additionalProperties: false
+
+required:
+  - compatible
+  - reg
+  - spi-cpha
+  - spi-cpol
+
+examples:
+  - |
+    spi0 {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        adc@0 {
+                compatible = "adi,ad7887";
+                reg = <0>;
+                spi-max-frequency = <1000000>;
+                spi-cpol;
+                spi-cpha;
+
+                avcc-supply = <&adc_supply>;
+                vref-supply = <&adc_vref>;
+        };
+    };
+...

-- 
2.17.1

The driver currently supports a single part. So we don't need to do any
extra matching based on what the SPI framework has found during probe.

This should make the driver probe-able via other mechanisms like greybus.

Signed-off-by: Alexandru Ardelean <redacted>
---
 drivers/iio/adc/ad7887.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/adc/ad7887.c b/drivers/iio/adc/ad7887.c
index 06f684c053a0..c28f704301d9 100644
--- a/drivers/iio/adc/ad7887.c
+++ b/drivers/iio/adc/ad7887.c

@@ -269,13 +269,12 @@ static int ad7887_probe(struct spi_device *spi)
 			return ret;
 	}
 
-	st->chip_info =
-		&ad7887_chip_info_tbl[spi_get_device_id(spi)->driver_data];
+	st->chip_info = &ad7887_chip_info_tbl[ID_AD7887];
 
 	spi_set_drvdata(spi, indio_dev);
 	st->spi = spi;
 
-	indio_dev->name = spi_get_device_id(spi)->name;
+	indio_dev->name = "ad7887";
 	indio_dev->info = &ad7887_info;
 	indio_dev->modes = INDIO_DIRECT_MODE;
 

-- 
2.17.1

This will make the driver probe-able via device-tree and ACPI via PRP0001.
While the SPI match table may be sufficient, this should extend the cover
of this driver being probed by other methods.

Signed-off-by: Alexandru Ardelean <redacted>
---
 drivers/iio/adc/ad7887.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/iio/adc/ad7887.c b/drivers/iio/adc/ad7887.c
index c28f704301d9..eb3a5b0080ee 100644
--- a/drivers/iio/adc/ad7887.c
+++ b/drivers/iio/adc/ad7887.c

@@ -341,9 +341,16 @@ static const struct spi_device_id ad7887_id[] = {
 };
 MODULE_DEVICE_TABLE(spi, ad7887_id);
 
+static const struct of_device_id ad7887_of_match[] = {
+	{ .compatible = "adi,ad7887" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, ad7887_of_match);
+
 static struct spi_driver ad7887_driver = {
 	.driver = {
 		.name	= "ad7887",
+		.of_match_table	= ad7887_of_match,
 	},
 	.probe		= ad7887_probe,
 	.id_table	= ad7887_id,

-- 
2.17.1

On Thu, Nov 19, 2020 at 12:02 PM Alexandru Ardelean
[off-list ref] wrote:

This change converts the configuration of the dual-channel mode from the
old platform-data, to the device_property_present() function, which
supports both device-tree and ACPI configuration setups.

With this change the old platform_data include of the driver can be
removed.

I mostly like the part of getting rid of legacy platform data.
Reviewed-by: Andy Shevchenko <redacted>
(for patches 1-3 only)

-- 
With Best Regards,
Andy Shevchenko

On Thu, 19 Nov 2020 12:07:45 +0200
Alexandru Ardelean [off-list ref] wrote:

This change converts the configuration of the dual-channel mode from the
old platform-data, to the device_property_present() function, which
supports both device-tree and ACPI configuration setups.

With this change the old platform_data include of the driver can be
removed.

Signed-off-by: Alexandru Ardelean <redacted>

Hi Alexandru,

The set looks good in general, but there is a an oddity in the driver.
If we don't provide the vref regulator, then the scale is set to reflect
the internal 2.5V reference.  Fiven the external reference only
works by overdriving the 2.5V (must be higher than that to have
an effect) I guess we could in theory clamp it to a minimum of
2.5V but anyone wiring up less than that would have built a crazy board
so we can probably ignore it.

However, as I read the datasheet in dual channel mode it should be set
to VDD not 2.5V.  Right now you could make it work in a DT file
by setting VREF==VDD regulator but that's inelegant.

If you agree with my logic, perhaps a follow up patch?

Jonathan

On Thu, 19 Nov 2020 12:07:48 +0200
Alexandru Ardelean [off-list ref] wrote:

This change adds a simple device-tree binding for thhe Analog Devices
AD7887 ADC.

Signed-off-by: Alexandru Ardelean <redacted>

Hi Alexandru

Few things inline.

Jonathan

On datasheet this seems to be vdd-supply
As driver doesn't currently use it I assume ti would be safe to make them
match?

Perhaps worth mentioning that not supplying this will result in a 2.5V internal
reference being used unless we are in dual-channel-mode in which case it
will be VDD.  (I originally started looking at datasheet when I wondered
if we could just use the absence of this regulator to configure to single / dual
channel mode - but we can't because of the 2.5V internal reference).

On Sat, Nov 21, 2020 at 5:38 PM Jonathan Cameron [off-list ref] wrote:

On Thu, 19 Nov 2020 12:07:45 +0200
Alexandru Ardelean [off-list ref] wrote:

quoted

This change converts the configuration of the dual-channel mode from the
old platform-data, to the device_property_present() function, which
supports both device-tree and ACPI configuration setups.

With this change the old platform_data include of the driver can be
removed.

Signed-off-by: Alexandru Ardelean <redacted>

Hi Alexandru,

The set looks good in general, but there is a an oddity in the driver.
If we don't provide the vref regulator, then the scale is set to reflect
the internal 2.5V reference.  Fiven the external reference only
works by overdriving the 2.5V (must be higher than that to have
an effect) I guess we could in theory clamp it to a minimum of
2.5V but anyone wiring up less than that would have built a crazy board
so we can probably ignore it.

However, as I read the datasheet in dual channel mode it should be set
to VDD not 2.5V.  Right now you could make it work in a DT file
by setting VREF==VDD regulator but that's inelegant.

If you agree with my logic, perhaps a follow up patch?

I haven't managed to get around to this one.
Will try next week.

Jonathan

quoted

---

I'm wondering if this changeset is what was in mind here:
 https://lore.kernel.org/linux-iio/CA+U=DsqF5tu8Be9KXeyCWD2uHvV688Nc3n=z_Xi2J6H6DFJPRQ@mail.gmail.com/T/#mbe72e4da3acea3899d0d35402ea81e52a9bc34e6
This driver could have been simplified/reduced a whole lot more, but I'm
not sure about it. It's a bit of patch-noise, and later

Changelog v1 -> v2:
* dropped patch 'iio: adc: ad7887: convert driver to full DT probing'
  not adding the device_get_match_data() logic anymore
* added patch 'iio: adc: ad7887: remove matching code from driver'
  hooking the chip info directly to AD7887
* added patch 'iio: adc: ad7887: add OF match table'
  this just adds an OF table for DT and ACPI

 drivers/iio/adc/ad7887.c             | 10 +++++-----
 include/linux/platform_data/ad7887.h | 21 ---------------------
 2 files changed, 5 insertions(+), 26 deletions(-)
 delete mode 100644 include/linux/platform_data/ad7887.h

diff --git a/drivers/iio/adc/ad7887.c b/drivers/iio/adc/ad7887.c
index 4f6f0e0e03ee..06f684c053a0 100644
--- a/drivers/iio/adc/ad7887.c
+++ b/drivers/iio/adc/ad7887.c

@@ -23,8 +23,6 @@
 #include <linux/iio/trigger_consumer.h>
 #include <linux/iio/triggered_buffer.h>

-#include <linux/platform_data/ad7887.h>
-
 #define AD7887_REF_DIS               BIT(5)  /* on-chip reference disable */
 #define AD7887_DUAL          BIT(4)  /* dual-channel mode */
 #define AD7887_CH_AIN1               BIT(3)  /* convert on channel 1, DUAL=1 */

@@ -241,9 +239,9 @@ static void ad7887_reg_disable(void *data)

 static int ad7887_probe(struct spi_device *spi)
 {
-     struct ad7887_platform_data *pdata = spi->dev.platform_data;
      struct ad7887_state *st;
      struct iio_dev *indio_dev;
+     bool dual_mode;
      uint8_t mode;
      int ret;

@@ -286,7 +284,9 @@ static int ad7887_probe(struct spi_device *spi)
      mode = AD7887_PM_MODE4;
      if (!st->reg)
              mode |= AD7887_REF_DIS;
-     if (pdata && pdata->en_dual)
+
+     dual_mode = device_property_present(&spi->dev, "adi,dual-channel-mode");
+     if (dual_mode)
              mode |= AD7887_DUAL;

      st->tx_cmd_buf[0] = AD7887_CH_AIN0 | mode;

@@ -298,7 +298,7 @@ static int ad7887_probe(struct spi_device *spi)
      spi_message_init(&st->msg[AD7887_CH0]);
      spi_message_add_tail(&st->xfer[0], &st->msg[AD7887_CH0]);

-     if (pdata && pdata->en_dual) {
+     if (dual_mode) {
              st->tx_cmd_buf[2] = AD7887_CH_AIN1 | mode;

              st->xfer[1].rx_buf = &st->data[0];

diff --git a/include/linux/platform_data/ad7887.h b/include/linux/platform_data/ad7887.h
deleted file mode 100644
index 9b4dca6ae70b..000000000000
--- a/include/linux/platform_data/ad7887.h
+++ /dev/null

@@ -1,21 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-or-later */
-/*
- * AD7887 SPI ADC driver
- *
- * Copyright 2010 Analog Devices Inc.
- */
-#ifndef IIO_ADC_AD7887_H_
-#define IIO_ADC_AD7887_H_
-
-/**
- * struct ad7887_platform_data - AD7887 ADC driver platform data
- * @en_dual: Whether to use dual channel mode. If set to true AIN1 becomes the
- *   second input channel, and Vref is internally connected to Vdd. If set to
- *   false the device is used in single channel mode and AIN1/Vref is used as
- *   VREF input.
- */
-struct ad7887_platform_data {
-     bool en_dual;
-};
-
-#endif /* IIO_ADC_AD7887_H_ */

On Thu, Nov 19, 2020 at 12:07:48PM +0200, Alexandru Ardelean wrote:

Dual license new bindings. checkpatch.pl will tell you this.

+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/adc/adi,ad7887.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices AD7887 low power, 12-bit ADC
+
+maintainers:
+  - Michael Hennerich [off-list ref]
+
+description: |
+  Analog Devices AD7887 low power, 12-bit analog-to-digital converter (ADC)
+  that operates from a single 2.7 V to 5.25 V power supply.
+
+properties:
+  compatible:
+    enum:
+      - adi,ad7887
+
+  reg:
+    maxItems: 1
+
+  spi-cpha: true
+
+  spi-cpol: true
+
+  avcc-supply: true
+
+  spi-max-frequency: true
+
+  vref-supply:
+    description:
+      ADC reference voltage supply
+
+  adi,dual-channel-mode:
+    description:
+      Configures dual-channel mode for the ADC. In dual-channel operation,
+      the AIN1/VREF pin assumes its AIN1 function, providing a second analog
+      input channel. In this case, he reference voltage for the part is provided
+      via the VDD pin. As a result, the input voltage range on both the AIN0 and
+      AIN1 inputs is 0 to VDD.
+    type: boolean
+
+additionalProperties: false
+
+required:
+  - compatible
+  - reg
+  - spi-cpha
+  - spi-cpol
+
+examples:
+  - |
+    spi0 {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        adc@0 {
+                compatible = "adi,ad7887";
+                reg = <0>;
+                spi-max-frequency = <1000000>;
+                spi-cpol;
+                spi-cpha;
+
+                avcc-supply = <&adc_supply>;
+                vref-supply = <&adc_vref>;
+        };
+    };
+...
-- 
2.17.1