Linux-IIO Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v3 0/2] iio: adc: Add support for AD7292
@ 2019-11-01 16:23 Marcelo Schmitt
  2019-11-01 16:23 ` [PATCH v3 1/2] dt-bindings: iio: adc: Add dt-schema " Marcelo Schmitt
  2019-11-01 16:24 ` [PATCH v3 2/2] iio: adc: Add driver support " Marcelo Schmitt
  0 siblings, 2 replies; 6+ messages in thread
From: Marcelo Schmitt @ 2019-11-01 16:23 UTC (permalink / raw)
  To: jic23, robh
  Cc: dragos.bogdan, alexandru.ardelean, linux-kernel, linux-iio,
	devicetree, kernel-usp

This patchset adds a basic driver for the AD7292 ADC/DAC system along
with devicetree binding documentation.

Changelog V3:
- dt-binding: changed spi-cpha property
- dt-binding: added spi-cpha to the list of required properties
- dt-binding: renamed example device node
- dt-binding: added ';' to the final curly braces correctly closing dt-schema
- ad7292: fix up a typo in channel macros

Marcelo Schmitt (2):
  dt-bindings: iio: adc: Add dt-schema for AD7292
  iio: adc: Add driver support for AD7292

 .../bindings/iio/adc/adi,ad7292.yaml          | 103 ++++++
 MAINTAINERS                                   |   8 +
 drivers/iio/adc/Kconfig                       |  10 +
 drivers/iio/adc/Makefile                      |   1 +
 drivers/iio/adc/ad7292.c                      | 350 ++++++++++++++++++
 5 files changed, 472 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad7292.yaml
 create mode 100644 drivers/iio/adc/ad7292.c

-- 
2.23.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v3 1/2] dt-bindings: iio: adc: Add dt-schema for AD7292
  2019-11-01 16:23 [PATCH v3 0/2] iio: adc: Add support for AD7292 Marcelo Schmitt
@ 2019-11-01 16:23 ` " Marcelo Schmitt
  2019-11-05 21:09   ` Rob Herring
  2019-11-01 16:24 ` [PATCH v3 2/2] iio: adc: Add driver support " Marcelo Schmitt
  1 sibling, 1 reply; 6+ messages in thread
From: Marcelo Schmitt @ 2019-11-01 16:23 UTC (permalink / raw)
  To: jic23, robh
  Cc: dragos.bogdan, alexandru.ardelean, linux-kernel, linux-iio,
	devicetree, kernel-usp

Add a devicetree schema for AD7292 monitor and control system.

Signed-off-by: Marcelo Schmitt <marcelo.schmitt1@gmail.com>
---
Changelog V2 -> V3:
- changed spi-cpha property
- added spi-cpha to the list of required properties
- renamed example device node
- added ';' to the final curly braces correctly closing dt-schema

 .../bindings/iio/adc/adi,ad7292.yaml          | 103 ++++++++++++++++++
 MAINTAINERS                                   |   7 ++
 2 files changed, 110 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad7292.yaml

diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7292.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7292.yaml
new file mode 100644
index 000000000000..1014eb50a263
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7292.yaml
@@ -0,0 +1,103 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/adc/adi,ad7292.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices AD7292 10-Bit Monitor and Control System
+
+maintainers:
+  - Marcelo Schmitt <marcelo.schmitt1@gmail.com>
+
+description: |
+  Analog Devices AD7292 10-Bit Monitor and Control System with ADC, DACs,
+  Temperature Sensor, and GPIOs
+
+  Specifications about the part can be found at:
+    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7292.pdf
+
+properties:
+  compatible:
+    enum:
+      - adi,ad7292
+
+  reg:
+    maxItems: 1
+
+  vref-supply:
+    description: |
+      The regulator supply for ADC and DAC reference voltage.
+
+  spi-cpha: true
+
+  '#address-cells':
+    const: 1
+
+  '#size-cells':
+    const: 0
+
+required:
+  - compatible
+  - reg
+  - spi-cpha
+
+patternProperties:
+  "^channel@[0-7]$":
+    type: object
+    description: |
+      Represents the external channels which are connected to the ADC.
+      See Documentation/devicetree/bindings/iio/adc/adc.txt.
+
+    properties:
+      reg:
+        description: |
+          The channel number. It can have up to 8 channels numbered from 0 to 7.
+        maxItems: 1
+
+      diff-channels:
+        description: see Documentation/devicetree/bindings/iio/adc/adc.txt
+        maxItems: 1
+
+    required:
+      - reg
+
+examples:
+  - |
+    spi {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      ad7292: adc@0 {
+        compatible = "adi,ad7292";
+        reg = <0>;
+        spi-max-frequency = <25000000>;
+        vref-supply = <&adc_vref>;
+        spi-cpha;
+
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        channel@0 {
+          reg = <0>;
+          diff-channels = <0 1>;
+        };
+        channel@2 {
+          reg = <2>;
+        };
+        channel@3 {
+          reg = <3>;
+        };
+        channel@4 {
+          reg = <4>;
+        };
+        channel@5 {
+          reg = <5>;
+        };
+        channel@6 {
+          reg = <6>;
+        };
+        channel@7 {
+          reg = <7>;
+        };
+      };
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index 32bf5f8116d0..5d00e871c4c6 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -813,6 +813,13 @@ S:	Supported
 F:	drivers/iio/adc/ad7124.c
 F:	Documentation/devicetree/bindings/iio/adc/adi,ad7124.txt
 
+ANALOG DEVICES INC AD7292 DRIVER
+M:	Marcelo Schmitt <marcelo.schmitt1@gmail.com>
+L:	linux-iio@vger.kernel.org
+W:	http://ez.analog.com/community/linux-device-drivers
+S:	Supported
+F:	Documentation/devicetree/bindings/iio/adc/adi,ad7292.yaml
+
 ANALOG DEVICES INC AD7606 DRIVER
 M:	Stefan Popa <stefan.popa@analog.com>
 L:	linux-iio@vger.kernel.org
-- 
2.23.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v3 2/2] iio: adc: Add driver support for AD7292
  2019-11-01 16:23 [PATCH v3 0/2] iio: adc: Add support for AD7292 Marcelo Schmitt
  2019-11-01 16:23 ` [PATCH v3 1/2] dt-bindings: iio: adc: Add dt-schema " Marcelo Schmitt
@ 2019-11-01 16:24 ` " Marcelo Schmitt
  2019-11-03 11:52   ` Jonathan Cameron
  1 sibling, 1 reply; 6+ messages in thread
From: Marcelo Schmitt @ 2019-11-01 16:24 UTC (permalink / raw)
  To: jic23, robh
  Cc: dragos.bogdan, alexandru.ardelean, linux-kernel, linux-iio,
	devicetree, kernel-usp

The AD7292 is a 10-bit monitor and control system with ADC, DACs,
temperature sensor, and GPIOs.

Configure AD7292 devices in direct access mode, enabling single-ended
ADC readings.

Datasheet:
Link: https://www.analog.com/media/en/technical-documentation/data-sheets/ad7292.pdf

Signed-off-by: Marcelo Schmitt <marcelo.schmitt1@gmail.com>
---
Changelog V2 -> V3:
- fix up a typo in channel macros

 MAINTAINERS              |   1 +
 drivers/iio/adc/Kconfig  |  10 ++
 drivers/iio/adc/Makefile |   1 +
 drivers/iio/adc/ad7292.c | 350 +++++++++++++++++++++++++++++++++++++++
 4 files changed, 362 insertions(+)
 create mode 100644 drivers/iio/adc/ad7292.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 5d00e871c4c6..5941cfc0d6f7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -818,6 +818,7 @@ M:	Marcelo Schmitt <marcelo.schmitt1@gmail.com>
 L:	linux-iio@vger.kernel.org
 W:	http://ez.analog.com/community/linux-device-drivers
 S:	Supported
+F:	drivers/iio/adc/ad7292.c
 F:	Documentation/devicetree/bindings/iio/adc/adi,ad7292.yaml
 
 ANALOG DEVICES INC AD7606 DRIVER
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 632b331429c6..02587c990cb5 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -59,6 +59,16 @@ config AD7291
 	  To compile this driver as a module, choose M here: the
 	  module will be called ad7291.
 
+config AD7292
+	tristate "Analog Devices AD7292 ADC driver"
+	depends on SPI
+	help
+	  Say yes here to build support for Analog Devices AD7292
+	  8 Channel ADC with temperature sensor.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called ad7292.
+
 config AD7298
 	tristate "Analog Devices AD7298 ADC driver"
 	depends on SPI
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index 4779ab3ff8fb..1818f2f66566 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -11,6 +11,7 @@ obj-$(CONFIG_AD7124) += ad7124.o
 obj-$(CONFIG_AD7173) += ad7173.o
 obj-$(CONFIG_AD7266) += ad7266.o
 obj-$(CONFIG_AD7291) += ad7291.o
+obj-$(CONFIG_AD7292) += ad7292.o
 obj-$(CONFIG_AD7298) += ad7298.o
 obj-$(CONFIG_AD738X) += ad738x.o
 obj-$(CONFIG_AD7768) += ad7768-1.o
diff --git a/drivers/iio/adc/ad7292.c b/drivers/iio/adc/ad7292.c
new file mode 100644
index 000000000000..e2badf6ac10d
--- /dev/null
+++ b/drivers/iio/adc/ad7292.c
@@ -0,0 +1,350 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Analog Devices AD7292 SPI ADC driver
+ *
+ * Copyright 2019 Analog Devices Inc.
+ */
+
+#include <linux/bitfield.h>
+#include <linux/device.h>
+#include <linux/module.h>
+#include <linux/regulator/consumer.h>
+#include <linux/spi/spi.h>
+
+#include <linux/iio/iio.h>
+
+#define ADI_VENDOR_ID 0x0018
+
+/* AD7292 registers definition */
+#define AD7292_REG_VENDOR_ID		0x00
+#define AD7292_REG_CONF_BANK		0x05
+#define AD7292_REG_CONV_COMM		0x0E
+#define AD7292_REG_ADC_CH(x)		(0x10 + (x))
+
+/* AD7292 configuration bank subregisters definition */
+#define AD7292_BANK_REG_VIN_RNG0	0x10
+#define AD7292_BANK_REG_VIN_RNG1	0x11
+#define AD7292_BANK_REG_SAMP_MODE	0x12
+
+#define AD7292_RD_FLAG_MSK(x)		(BIT(7) | ((x) & 0x3F))
+
+/* AD7292_REG_ADC_CONVERSION */
+#define AD7292_ADC_DATA_MASK		GENMASK(15, 6)
+#define AD7292_ADC_DATA(x)		FIELD_GET(AD7292_ADC_DATA_MASK, x)
+
+/* AD7292_CHANNEL_SAMPLING_MODE */
+#define AD7292_CH_SAMP_MODE(reg, ch)	((reg >> 8) & BIT(ch))
+
+/* AD7292_CHANNEL_VIN_RANGE */
+#define AD7292_CH_VIN_RANGE(reg, ch)	(reg & BIT(ch))
+
+#define AD7292_VOLTAGE_CHAN(_chan)					\
+{									\
+	.type = IIO_VOLTAGE,						\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |			\
+			      BIT(IIO_CHAN_INFO_SCALE),			\
+	.indexed = 1,							\
+	.channel = _chan,						\
+}
+
+static const struct iio_chan_spec ad7292_channels[] = {
+	AD7292_VOLTAGE_CHAN(0),
+	AD7292_VOLTAGE_CHAN(1),
+	AD7292_VOLTAGE_CHAN(2),
+	AD7292_VOLTAGE_CHAN(3),
+	AD7292_VOLTAGE_CHAN(4),
+	AD7292_VOLTAGE_CHAN(5),
+	AD7292_VOLTAGE_CHAN(6),
+	AD7292_VOLTAGE_CHAN(7)
+};
+
+static const struct iio_chan_spec ad7292_channels_diff[] = {
+	{
+		.type = IIO_VOLTAGE,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
+		.indexed = 1,
+		.differential = 1,
+		.channel = 0,
+		.channel2 = 1,
+	},
+	AD7292_VOLTAGE_CHAN(2),
+	AD7292_VOLTAGE_CHAN(3),
+	AD7292_VOLTAGE_CHAN(4),
+	AD7292_VOLTAGE_CHAN(5),
+	AD7292_VOLTAGE_CHAN(6),
+	AD7292_VOLTAGE_CHAN(7)
+};
+
+struct ad7292_state {
+	struct spi_device *spi;
+	struct regulator *reg;
+	unsigned short vref_mv;
+
+	__be16 d16 ____cacheline_aligned;
+	u8 d8[2];
+};
+
+static int ad7292_spi_reg_read(struct ad7292_state *st, unsigned int addr)
+{
+	int ret;
+
+	st->d8[0] = AD7292_RD_FLAG_MSK(addr);
+
+	ret = spi_write_then_read(st->spi, st->d8, 1, &st->d16, 2);
+	if (ret < 0)
+		return ret;
+
+	return be16_to_cpu(st->d16);
+}
+
+static int ad7292_spi_subreg_read(struct ad7292_state *st, unsigned int addr,
+				  unsigned int sub_addr, unsigned int len)
+{
+	unsigned int shift = 16 - (8 * len);
+	int ret;
+
+	st->d8[0] = AD7292_RD_FLAG_MSK(addr);
+	st->d8[1] = sub_addr;
+
+	ret = spi_write_then_read(st->spi, st->d8, 2, &st->d16, len);
+	if (ret < 0)
+		return ret;
+
+	return (be16_to_cpu(st->d16) >> shift);
+}
+
+static int ad7292_single_conversion(struct ad7292_state *st,
+				    unsigned int chan_addr)
+{
+	int ret;
+
+	struct spi_transfer t[] = {
+		{
+			.tx_buf = &st->d8,
+			.len = 4,
+			.delay_usecs = 6,
+		}, {
+			.rx_buf = &st->d16,
+			.len = 2,
+		},
+	};
+
+	st->d8[0] = chan_addr;
+	st->d8[1] = AD7292_RD_FLAG_MSK(AD7292_REG_CONV_COMM);
+
+	ret = spi_sync_transfer(st->spi, t, ARRAY_SIZE(t));
+
+	if (ret < 0)
+		return ret;
+
+	return be16_to_cpu(st->d16);
+}
+
+static int ad7292_vin_range_multiplier(struct ad7292_state *st, int channel)
+{
+	int samp_mode, range0, range1, factor = 1;
+
+	/*
+	 * Every AD7292 ADC channel may have its input range adjusted according
+	 * to the settings at the ADC sampling mode and VIN range subregisters.
+	 * For a given channel, the minimum input range is equal to Vref, and it
+	 * may be increased by a multiplier factor of 2 or 4 according to the
+	 * following rule:
+	 * If channel is being sampled with respect to AGND:
+	 *	factor = 4 if VIN range0 and VIN range1 equal 0
+	 *	factor = 2 if only one of VIN ranges equal 1
+	 *	factor = 1 if both VIN range0 and VIN range1 equal 1
+	 * If channel is being sampled with respect to AVDD:
+	 *	factor = 4 if VIN range0 and VIN range1 equal 0
+	 *	Behavior is undefined if any of VIN range doesn't equal 0
+	 */
+
+	samp_mode = ad7292_spi_subreg_read(st, AD7292_REG_CONF_BANK,
+					   AD7292_BANK_REG_SAMP_MODE, 2);
+
+	if (samp_mode < 0)
+		return samp_mode;
+
+	range0 = ad7292_spi_subreg_read(st, AD7292_REG_CONF_BANK,
+					AD7292_BANK_REG_VIN_RNG0, 2);
+
+	if (range0 < 0)
+		return range0;
+
+	range1 = ad7292_spi_subreg_read(st, AD7292_REG_CONF_BANK,
+					AD7292_BANK_REG_VIN_RNG1, 2);
+
+	if (range1 < 0)
+		return range1;
+
+	if (AD7292_CH_SAMP_MODE(samp_mode, channel)) {
+		/* Sampling with respect to AGND */
+		if (!AD7292_CH_VIN_RANGE(range0, channel))
+			factor *= 2;
+
+		if (!AD7292_CH_VIN_RANGE(range1, channel))
+			factor *= 2;
+
+	} else {
+		/* Sampling with respect to AVDD */
+		if (AD7292_CH_VIN_RANGE(range0, channel) ||
+		    AD7292_CH_VIN_RANGE(range1, channel))
+			return -EPERM;
+
+		factor = 4;
+	}
+
+	return factor;
+}
+
+static int ad7292_read_raw(struct iio_dev *indio_dev,
+			   const struct iio_chan_spec *chan,
+			   int *val, int *val2, long info)
+{
+	struct ad7292_state *st = iio_priv(indio_dev);
+	unsigned int ch_addr;
+	int ret;
+
+	switch (info) {
+	case IIO_CHAN_INFO_RAW:
+		ch_addr = AD7292_REG_ADC_CH(chan->channel);
+		ret = ad7292_single_conversion(st, ch_addr);
+		if (ret < 0)
+			return ret;
+
+		*val = AD7292_ADC_DATA(ret);
+
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_SCALE:
+		/*
+		 * To convert a raw value to standard units, the IIO defines
+		 * this formula: Scaled value = (raw + offset) * scale.
+		 * For the scale to be a correct multiplier for (raw + offset),
+		 * it must be calculated as the input range divided by the
+		 * number of possible distinct input values. Given the ADC data
+		 * is 10 bit long, it may assume 2^10 distinct values.
+		 * Hence, scale = range / 2^10. The IIO_VAL_FRACTIONAL_LOG2
+		 * return type indicates to the IIO API to divide *val by 2 to
+		 * the power of *val2 when returning from read_raw.
+		 */
+
+		ret = ad7292_vin_range_multiplier(st, chan->channel);
+		if (ret < 0)
+			return ret;
+
+		*val = st->vref_mv * ret;
+		*val2 = 10;
+		return IIO_VAL_FRACTIONAL_LOG2;
+	default:
+		break;
+	}
+	return -EINVAL;
+}
+
+static const struct iio_info ad7292_info = {
+	.read_raw = ad7292_read_raw,
+};
+
+static void ad7292_regulator_disable(void *data)
+{
+	struct ad7292_state *st = data;
+
+	regulator_disable(st->reg);
+}
+
+static int ad7292_probe(struct spi_device *spi)
+{
+	struct ad7292_state *st;
+	struct iio_dev *indio_dev;
+	struct device_node *child;
+	bool diff_channels = 0;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	st = iio_priv(indio_dev);
+	st->spi = spi;
+
+	ret = ad7292_spi_reg_read(st, AD7292_REG_VENDOR_ID);
+	if (ret != ADI_VENDOR_ID) {
+		dev_err(&spi->dev, "Wrong vendor id 0x%x\n", ret);
+		return -EINVAL;
+	}
+
+	spi_set_drvdata(spi, indio_dev);
+
+	st->reg = devm_regulator_get_optional(&spi->dev, "vref");
+	if (!IS_ERR(st->reg)) {
+		ret = regulator_enable(st->reg);
+		if (ret) {
+			dev_err(&spi->dev,
+				"Failed to enable external vref supply\n");
+			return ret;
+		}
+
+		ret = devm_add_action_or_reset(&spi->dev,
+					       ad7292_regulator_disable, st);
+		if (ret) {
+			regulator_disable(st->reg);
+			return ret;
+		}
+
+		ret = regulator_get_voltage(st->reg);
+		if (ret < 0)
+			return ret;
+
+		st->vref_mv = ret / 1000;
+	} else {
+		/* Use the internal voltage reference. */
+		st->vref_mv = 1250;
+	}
+
+	indio_dev->dev.parent = &spi->dev;
+	indio_dev->name = spi_get_device_id(spi)->name;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->info = &ad7292_info;
+
+	for_each_available_child_of_node(spi->dev.of_node, child) {
+		diff_channels = of_property_read_bool(child, "diff-channels");
+		if (diff_channels)
+			break;
+	}
+
+	if (diff_channels) {
+		indio_dev->num_channels = ARRAY_SIZE(ad7292_channels_diff);
+		indio_dev->channels = ad7292_channels_diff;
+	} else {
+		indio_dev->num_channels = ARRAY_SIZE(ad7292_channels);
+		indio_dev->channels = ad7292_channels;
+	}
+
+	return devm_iio_device_register(&spi->dev, indio_dev);
+}
+
+static const struct spi_device_id ad7292_id_table[] = {
+	{ "ad7292", 0 },
+	{}
+};
+MODULE_DEVICE_TABLE(spi, ad7292_id_table);
+
+static const struct of_device_id ad7292_of_match[] = {
+	{ .compatible = "adi,ad7292" },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, ad7292_of_match);
+
+static struct spi_driver ad7292_driver = {
+	.driver = {
+		.name = "ad7292",
+		.of_match_table = ad7292_of_match,
+	},
+	.probe = ad7292_probe,
+	.id_table = ad7292_id_table,
+};
+module_spi_driver(ad7292_driver);
+
+MODULE_AUTHOR("Marcelo Schmitt <marcelo.schmitt1@gmail.com>");
+MODULE_DESCRIPTION("Analog Devices AD7292 ADC driver");
+MODULE_LICENSE("GPL v2");
-- 
2.23.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 2/2] iio: adc: Add driver support for AD7292
  2019-11-01 16:24 ` [PATCH v3 2/2] iio: adc: Add driver support " Marcelo Schmitt
@ 2019-11-03 11:52   ` Jonathan Cameron
  2019-11-07 16:38     ` Marcelo Schmitt
  0 siblings, 1 reply; 6+ messages in thread
From: Jonathan Cameron @ 2019-11-03 11:52 UTC (permalink / raw)
  To: Marcelo Schmitt
  Cc: robh, dragos.bogdan, alexandru.ardelean, linux-kernel, linux-iio,
	devicetree, kernel-usp

On Fri, 1 Nov 2019 13:24:26 -0300
Marcelo Schmitt <marcelo.schmitt1@gmail.com> wrote:

> The AD7292 is a 10-bit monitor and control system with ADC, DACs,
> temperature sensor, and GPIOs.
> 
> Configure AD7292 devices in direct access mode, enabling single-ended
> ADC readings.
> 
> Datasheet:
> Link: https://www.analog.com/media/en/technical-documentation/data-sheets/ad7292.pdf
> 
> Signed-off-by: Marcelo Schmitt <marcelo.schmitt1@gmail.com>
One totally trivial thing inline if you happen to be respinning.
Otherwise I'll fix up when applying.

I would like to give a bit of time for others to comment, particularly on the dt
bindings as I frequently miss problems in them. Hence I won't be applying it today
but may pick it up later in the week or next weekend.

Thanks,

Jonathan


> ---
> Changelog V2 -> V3:
> - fix up a typo in channel macros
> 
>  MAINTAINERS              |   1 +
>  drivers/iio/adc/Kconfig  |  10 ++
>  drivers/iio/adc/Makefile |   1 +
>  drivers/iio/adc/ad7292.c | 350 +++++++++++++++++++++++++++++++++++++++
>  4 files changed, 362 insertions(+)
>  create mode 100644 drivers/iio/adc/ad7292.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 5d00e871c4c6..5941cfc0d6f7 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -818,6 +818,7 @@ M:	Marcelo Schmitt <marcelo.schmitt1@gmail.com>
>  L:	linux-iio@vger.kernel.org
>  W:	http://ez.analog.com/community/linux-device-drivers
>  S:	Supported
> +F:	drivers/iio/adc/ad7292.c
>  F:	Documentation/devicetree/bindings/iio/adc/adi,ad7292.yaml
>  
>  ANALOG DEVICES INC AD7606 DRIVER
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 632b331429c6..02587c990cb5 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -59,6 +59,16 @@ config AD7291
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called ad7291.
>  
> +config AD7292
> +	tristate "Analog Devices AD7292 ADC driver"
> +	depends on SPI
> +	help
> +	  Say yes here to build support for Analog Devices AD7292
> +	  8 Channel ADC with temperature sensor.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called ad7292.
> +
>  config AD7298
>  	tristate "Analog Devices AD7298 ADC driver"
>  	depends on SPI
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index 4779ab3ff8fb..1818f2f66566 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -11,6 +11,7 @@ obj-$(CONFIG_AD7124) += ad7124.o
>  obj-$(CONFIG_AD7173) += ad7173.o
>  obj-$(CONFIG_AD7266) += ad7266.o
>  obj-$(CONFIG_AD7291) += ad7291.o
> +obj-$(CONFIG_AD7292) += ad7292.o
>  obj-$(CONFIG_AD7298) += ad7298.o
>  obj-$(CONFIG_AD738X) += ad738x.o
>  obj-$(CONFIG_AD7768) += ad7768-1.o
> diff --git a/drivers/iio/adc/ad7292.c b/drivers/iio/adc/ad7292.c
> new file mode 100644
> index 000000000000..e2badf6ac10d
> --- /dev/null
> +++ b/drivers/iio/adc/ad7292.c
> @@ -0,0 +1,350 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Analog Devices AD7292 SPI ADC driver
> + *
> + * Copyright 2019 Analog Devices Inc.
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/device.h>
> +#include <linux/module.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/spi/spi.h>
> +
> +#include <linux/iio/iio.h>
> +
> +#define ADI_VENDOR_ID 0x0018
> +
> +/* AD7292 registers definition */
> +#define AD7292_REG_VENDOR_ID		0x00
> +#define AD7292_REG_CONF_BANK		0x05
> +#define AD7292_REG_CONV_COMM		0x0E
> +#define AD7292_REG_ADC_CH(x)		(0x10 + (x))
> +
> +/* AD7292 configuration bank subregisters definition */
> +#define AD7292_BANK_REG_VIN_RNG0	0x10
> +#define AD7292_BANK_REG_VIN_RNG1	0x11
> +#define AD7292_BANK_REG_SAMP_MODE	0x12
> +
> +#define AD7292_RD_FLAG_MSK(x)		(BIT(7) | ((x) & 0x3F))
> +
> +/* AD7292_REG_ADC_CONVERSION */
> +#define AD7292_ADC_DATA_MASK		GENMASK(15, 6)
> +#define AD7292_ADC_DATA(x)		FIELD_GET(AD7292_ADC_DATA_MASK, x)
> +
> +/* AD7292_CHANNEL_SAMPLING_MODE */
> +#define AD7292_CH_SAMP_MODE(reg, ch)	((reg >> 8) & BIT(ch))
> +
> +/* AD7292_CHANNEL_VIN_RANGE */
> +#define AD7292_CH_VIN_RANGE(reg, ch)	(reg & BIT(ch))

Trivial but all macro parameters should be in brackets to prevent
weirdness if some sticks a parameter with higher precedence in there
so ((reg) & BIT(ch)).

I can fix this up if the rest is fine.

> +
> +#define AD7292_VOLTAGE_CHAN(_chan)					\
> +{									\
> +	.type = IIO_VOLTAGE,						\
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |			\
> +			      BIT(IIO_CHAN_INFO_SCALE),			\
> +	.indexed = 1,							\
> +	.channel = _chan,						\
> +}
> +
> +static const struct iio_chan_spec ad7292_channels[] = {
> +	AD7292_VOLTAGE_CHAN(0),
> +	AD7292_VOLTAGE_CHAN(1),
> +	AD7292_VOLTAGE_CHAN(2),
> +	AD7292_VOLTAGE_CHAN(3),
> +	AD7292_VOLTAGE_CHAN(4),
> +	AD7292_VOLTAGE_CHAN(5),
> +	AD7292_VOLTAGE_CHAN(6),
> +	AD7292_VOLTAGE_CHAN(7)
> +};
> +
> +static const struct iio_chan_spec ad7292_channels_diff[] = {
> +	{
> +		.type = IIO_VOLTAGE,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> +		.indexed = 1,
> +		.differential = 1,
> +		.channel = 0,
> +		.channel2 = 1,
> +	},
> +	AD7292_VOLTAGE_CHAN(2),
> +	AD7292_VOLTAGE_CHAN(3),
> +	AD7292_VOLTAGE_CHAN(4),
> +	AD7292_VOLTAGE_CHAN(5),
> +	AD7292_VOLTAGE_CHAN(6),
> +	AD7292_VOLTAGE_CHAN(7)
> +};
> +
> +struct ad7292_state {
> +	struct spi_device *spi;
> +	struct regulator *reg;
> +	unsigned short vref_mv;
> +
> +	__be16 d16 ____cacheline_aligned;
> +	u8 d8[2];
> +};
> +
> +static int ad7292_spi_reg_read(struct ad7292_state *st, unsigned int addr)
> +{
> +	int ret;
> +
> +	st->d8[0] = AD7292_RD_FLAG_MSK(addr);
> +
> +	ret = spi_write_then_read(st->spi, st->d8, 1, &st->d16, 2);
> +	if (ret < 0)
> +		return ret;
> +
> +	return be16_to_cpu(st->d16);
> +}
> +
> +static int ad7292_spi_subreg_read(struct ad7292_state *st, unsigned int addr,
> +				  unsigned int sub_addr, unsigned int len)
> +{
> +	unsigned int shift = 16 - (8 * len);
> +	int ret;
> +
> +	st->d8[0] = AD7292_RD_FLAG_MSK(addr);
> +	st->d8[1] = sub_addr;
> +
> +	ret = spi_write_then_read(st->spi, st->d8, 2, &st->d16, len);
> +	if (ret < 0)
> +		return ret;
> +
> +	return (be16_to_cpu(st->d16) >> shift);
> +}
> +
> +static int ad7292_single_conversion(struct ad7292_state *st,
> +				    unsigned int chan_addr)
> +{
> +	int ret;
> +
> +	struct spi_transfer t[] = {
> +		{
> +			.tx_buf = &st->d8,
> +			.len = 4,
> +			.delay_usecs = 6,
> +		}, {
> +			.rx_buf = &st->d16,
> +			.len = 2,
> +		},
> +	};
> +
> +	st->d8[0] = chan_addr;
> +	st->d8[1] = AD7292_RD_FLAG_MSK(AD7292_REG_CONV_COMM);
> +
> +	ret = spi_sync_transfer(st->spi, t, ARRAY_SIZE(t));
> +
> +	if (ret < 0)
> +		return ret;
> +
> +	return be16_to_cpu(st->d16);
> +}
> +
> +static int ad7292_vin_range_multiplier(struct ad7292_state *st, int channel)
> +{
> +	int samp_mode, range0, range1, factor = 1;
> +
> +	/*
> +	 * Every AD7292 ADC channel may have its input range adjusted according
> +	 * to the settings at the ADC sampling mode and VIN range subregisters.
> +	 * For a given channel, the minimum input range is equal to Vref, and it
> +	 * may be increased by a multiplier factor of 2 or 4 according to the
> +	 * following rule:
> +	 * If channel is being sampled with respect to AGND:
> +	 *	factor = 4 if VIN range0 and VIN range1 equal 0
> +	 *	factor = 2 if only one of VIN ranges equal 1
> +	 *	factor = 1 if both VIN range0 and VIN range1 equal 1
> +	 * If channel is being sampled with respect to AVDD:
> +	 *	factor = 4 if VIN range0 and VIN range1 equal 0
> +	 *	Behavior is undefined if any of VIN range doesn't equal 0
> +	 */
> +
> +	samp_mode = ad7292_spi_subreg_read(st, AD7292_REG_CONF_BANK,
> +					   AD7292_BANK_REG_SAMP_MODE, 2);
> +
> +	if (samp_mode < 0)
> +		return samp_mode;
> +
> +	range0 = ad7292_spi_subreg_read(st, AD7292_REG_CONF_BANK,
> +					AD7292_BANK_REG_VIN_RNG0, 2);
> +
> +	if (range0 < 0)
> +		return range0;
> +
> +	range1 = ad7292_spi_subreg_read(st, AD7292_REG_CONF_BANK,
> +					AD7292_BANK_REG_VIN_RNG1, 2);
> +
> +	if (range1 < 0)
> +		return range1;
> +
> +	if (AD7292_CH_SAMP_MODE(samp_mode, channel)) {
> +		/* Sampling with respect to AGND */
> +		if (!AD7292_CH_VIN_RANGE(range0, channel))
> +			factor *= 2;
> +
> +		if (!AD7292_CH_VIN_RANGE(range1, channel))
> +			factor *= 2;
> +
> +	} else {
> +		/* Sampling with respect to AVDD */
> +		if (AD7292_CH_VIN_RANGE(range0, channel) ||
> +		    AD7292_CH_VIN_RANGE(range1, channel))
> +			return -EPERM;
> +
> +		factor = 4;
> +	}
> +
> +	return factor;
> +}
> +
> +static int ad7292_read_raw(struct iio_dev *indio_dev,
> +			   const struct iio_chan_spec *chan,
> +			   int *val, int *val2, long info)
> +{
> +	struct ad7292_state *st = iio_priv(indio_dev);
> +	unsigned int ch_addr;
> +	int ret;
> +
> +	switch (info) {
> +	case IIO_CHAN_INFO_RAW:
> +		ch_addr = AD7292_REG_ADC_CH(chan->channel);
> +		ret = ad7292_single_conversion(st, ch_addr);
> +		if (ret < 0)
> +			return ret;
> +
> +		*val = AD7292_ADC_DATA(ret);
> +
> +		return IIO_VAL_INT;
> +	case IIO_CHAN_INFO_SCALE:
> +		/*
> +		 * To convert a raw value to standard units, the IIO defines
> +		 * this formula: Scaled value = (raw + offset) * scale.
> +		 * For the scale to be a correct multiplier for (raw + offset),
> +		 * it must be calculated as the input range divided by the
> +		 * number of possible distinct input values. Given the ADC data
> +		 * is 10 bit long, it may assume 2^10 distinct values.
> +		 * Hence, scale = range / 2^10. The IIO_VAL_FRACTIONAL_LOG2
> +		 * return type indicates to the IIO API to divide *val by 2 to
> +		 * the power of *val2 when returning from read_raw.
> +		 */
> +
> +		ret = ad7292_vin_range_multiplier(st, chan->channel);
> +		if (ret < 0)
> +			return ret;
> +
> +		*val = st->vref_mv * ret;
> +		*val2 = 10;
> +		return IIO_VAL_FRACTIONAL_LOG2;
> +	default:
> +		break;
> +	}
> +	return -EINVAL;
> +}
> +
> +static const struct iio_info ad7292_info = {
> +	.read_raw = ad7292_read_raw,
> +};
> +
> +static void ad7292_regulator_disable(void *data)
> +{
> +	struct ad7292_state *st = data;
> +
> +	regulator_disable(st->reg);
> +}
> +
> +static int ad7292_probe(struct spi_device *spi)
> +{
> +	struct ad7292_state *st;
> +	struct iio_dev *indio_dev;
> +	struct device_node *child;
> +	bool diff_channels = 0;
> +	int ret;
> +
> +	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	st = iio_priv(indio_dev);
> +	st->spi = spi;
> +
> +	ret = ad7292_spi_reg_read(st, AD7292_REG_VENDOR_ID);
> +	if (ret != ADI_VENDOR_ID) {
> +		dev_err(&spi->dev, "Wrong vendor id 0x%x\n", ret);
> +		return -EINVAL;
> +	}
> +
> +	spi_set_drvdata(spi, indio_dev);
> +
> +	st->reg = devm_regulator_get_optional(&spi->dev, "vref");
> +	if (!IS_ERR(st->reg)) {
> +		ret = regulator_enable(st->reg);
> +		if (ret) {
> +			dev_err(&spi->dev,
> +				"Failed to enable external vref supply\n");
> +			return ret;
> +		}
> +
> +		ret = devm_add_action_or_reset(&spi->dev,
> +					       ad7292_regulator_disable, st);
> +		if (ret) {
> +			regulator_disable(st->reg);
> +			return ret;
> +		}
> +
> +		ret = regulator_get_voltage(st->reg);
> +		if (ret < 0)
> +			return ret;
> +
> +		st->vref_mv = ret / 1000;
> +	} else {
> +		/* Use the internal voltage reference. */
> +		st->vref_mv = 1250;
> +	}
> +
> +	indio_dev->dev.parent = &spi->dev;
> +	indio_dev->name = spi_get_device_id(spi)->name;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->info = &ad7292_info;
> +
> +	for_each_available_child_of_node(spi->dev.of_node, child) {
> +		diff_channels = of_property_read_bool(child, "diff-channels");
> +		if (diff_channels)
> +			break;
> +	}
> +
> +	if (diff_channels) {
> +		indio_dev->num_channels = ARRAY_SIZE(ad7292_channels_diff);
> +		indio_dev->channels = ad7292_channels_diff;
> +	} else {
> +		indio_dev->num_channels = ARRAY_SIZE(ad7292_channels);
> +		indio_dev->channels = ad7292_channels;
> +	}
> +
> +	return devm_iio_device_register(&spi->dev, indio_dev);
> +}
> +
> +static const struct spi_device_id ad7292_id_table[] = {
> +	{ "ad7292", 0 },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(spi, ad7292_id_table);
> +
> +static const struct of_device_id ad7292_of_match[] = {
> +	{ .compatible = "adi,ad7292" },
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(of, ad7292_of_match);
> +
> +static struct spi_driver ad7292_driver = {
> +	.driver = {
> +		.name = "ad7292",
> +		.of_match_table = ad7292_of_match,
> +	},
> +	.probe = ad7292_probe,
> +	.id_table = ad7292_id_table,
> +};
> +module_spi_driver(ad7292_driver);
> +
> +MODULE_AUTHOR("Marcelo Schmitt <marcelo.schmitt1@gmail.com>");
> +MODULE_DESCRIPTION("Analog Devices AD7292 ADC driver");
> +MODULE_LICENSE("GPL v2");


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 1/2] dt-bindings: iio: adc: Add dt-schema for AD7292
  2019-11-01 16:23 ` [PATCH v3 1/2] dt-bindings: iio: adc: Add dt-schema " Marcelo Schmitt
@ 2019-11-05 21:09   ` Rob Herring
  0 siblings, 0 replies; 6+ messages in thread
From: Rob Herring @ 2019-11-05 21:09 UTC (permalink / raw)
  To: Marcelo Schmitt
  Cc: jic23, dragos.bogdan, alexandru.ardelean, linux-kernel,
	linux-iio, devicetree, kernel-usp

On Fri, Nov 01, 2019 at 01:23:59PM -0300, Marcelo Schmitt wrote:
> Add a devicetree schema for AD7292 monitor and control system.
> 
> Signed-off-by: Marcelo Schmitt <marcelo.schmitt1@gmail.com>
> ---
> Changelog V2 -> V3:
> - changed spi-cpha property
> - added spi-cpha to the list of required properties
> - renamed example device node
> - added ';' to the final curly braces correctly closing dt-schema
> 
>  .../bindings/iio/adc/adi,ad7292.yaml          | 103 ++++++++++++++++++
>  MAINTAINERS                                   |   7 ++
>  2 files changed, 110 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad7292.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7292.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7292.yaml
> new file mode 100644
> index 000000000000..1014eb50a263
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7292.yaml
> @@ -0,0 +1,103 @@
> +# SPDX-License-Identifier: GPL-2.0

For new bindings:

# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)

> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/adc/adi,ad7292.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Analog Devices AD7292 10-Bit Monitor and Control System
> +
> +maintainers:
> +  - Marcelo Schmitt <marcelo.schmitt1@gmail.com>
> +
> +description: |
> +  Analog Devices AD7292 10-Bit Monitor and Control System with ADC, DACs,
> +  Temperature Sensor, and GPIOs
> +
> +  Specifications about the part can be found at:
> +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7292.pdf
> +
> +properties:
> +  compatible:
> +    enum:
> +      - adi,ad7292
> +
> +  reg:
> +    maxItems: 1
> +
> +  vref-supply:
> +    description: |
> +      The regulator supply for ADC and DAC reference voltage.
> +
> +  spi-cpha: true
> +
> +  '#address-cells':
> +    const: 1
> +
> +  '#size-cells':
> +    const: 0
> +
> +required:
> +  - compatible
> +  - reg
> +  - spi-cpha
> +
> +patternProperties:
> +  "^channel@[0-7]$":
> +    type: object
> +    description: |
> +      Represents the external channels which are connected to the ADC.
> +      See Documentation/devicetree/bindings/iio/adc/adc.txt.
> +
> +    properties:
> +      reg:
> +        description: |
> +          The channel number. It can have up to 8 channels numbered from 0 to 7.

Can be a constraint:

items:
  - maximum: 7

> +        maxItems: 1

And then drop this because it's implied by the above.

> +
> +      diff-channels:
> +        description: see Documentation/devicetree/bindings/iio/adc/adc.txt
> +        maxItems: 1
> +
> +    required:
> +      - reg
> +
> +examples:
> +  - |
> +    spi {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +
> +      ad7292: adc@0 {
> +        compatible = "adi,ad7292";
> +        reg = <0>;
> +        spi-max-frequency = <25000000>;
> +        vref-supply = <&adc_vref>;
> +        spi-cpha;
> +
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        channel@0 {
> +          reg = <0>;
> +          diff-channels = <0 1>;
> +        };
> +        channel@2 {
> +          reg = <2>;
> +        };
> +        channel@3 {
> +          reg = <3>;
> +        };
> +        channel@4 {
> +          reg = <4>;
> +        };
> +        channel@5 {
> +          reg = <5>;
> +        };
> +        channel@6 {
> +          reg = <6>;
> +        };
> +        channel@7 {
> +          reg = <7>;
> +        };
> +      };
> +    };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 32bf5f8116d0..5d00e871c4c6 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -813,6 +813,13 @@ S:	Supported
>  F:	drivers/iio/adc/ad7124.c
>  F:	Documentation/devicetree/bindings/iio/adc/adi,ad7124.txt
>  
> +ANALOG DEVICES INC AD7292 DRIVER
> +M:	Marcelo Schmitt <marcelo.schmitt1@gmail.com>
> +L:	linux-iio@vger.kernel.org
> +W:	http://ez.analog.com/community/linux-device-drivers
> +S:	Supported
> +F:	Documentation/devicetree/bindings/iio/adc/adi,ad7292.yaml
> +
>  ANALOG DEVICES INC AD7606 DRIVER
>  M:	Stefan Popa <stefan.popa@analog.com>
>  L:	linux-iio@vger.kernel.org
> -- 
> 2.23.0
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 2/2] iio: adc: Add driver support for AD7292
  2019-11-03 11:52   ` Jonathan Cameron
@ 2019-11-07 16:38     ` Marcelo Schmitt
  0 siblings, 0 replies; 6+ messages in thread
From: Marcelo Schmitt @ 2019-11-07 16:38 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: robh, dragos.bogdan, alexandru.ardelean, linux-kernel, linux-iio,
	devicetree, kernel-usp

Hi,

I'll respin for the dt-schema. Thus, I can add brackets to macro
parameters. I think I found another couple of brackets missing.
Comments inline.

Thanks,

Marcelo

On 11/03, Jonathan Cameron wrote:
> On Fri, 1 Nov 2019 13:24:26 -0300
> Marcelo Schmitt <marcelo.schmitt1@gmail.com> wrote:
> 
> > The AD7292 is a 10-bit monitor and control system with ADC, DACs,
> > temperature sensor, and GPIOs.
> > 
> > Configure AD7292 devices in direct access mode, enabling single-ended
> > ADC readings.
> > 
> > Datasheet:
> > Link: https://www.analog.com/media/en/technical-documentation/data-sheets/ad7292.pdf
> > 
> > Signed-off-by: Marcelo Schmitt <marcelo.schmitt1@gmail.com>
> One totally trivial thing inline if you happen to be respinning.
> Otherwise I'll fix up when applying.
> 
> I would like to give a bit of time for others to comment, particularly on the dt
> bindings as I frequently miss problems in them. Hence I won't be applying it today
> but may pick it up later in the week or next weekend.
> 
> Thanks,
> 
> Jonathan
> 
> 
> > ---
> > Changelog V2 -> V3:
> > - fix up a typo in channel macros
> > 
> >  MAINTAINERS              |   1 +
> >  drivers/iio/adc/Kconfig  |  10 ++
> >  drivers/iio/adc/Makefile |   1 +
> >  drivers/iio/adc/ad7292.c | 350 +++++++++++++++++++++++++++++++++++++++
> >  4 files changed, 362 insertions(+)
> >  create mode 100644 drivers/iio/adc/ad7292.c
> > 
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 5d00e871c4c6..5941cfc0d6f7 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -818,6 +818,7 @@ M:	Marcelo Schmitt <marcelo.schmitt1@gmail.com>
> >  L:	linux-iio@vger.kernel.org
> >  W:	http://ez.analog.com/community/linux-device-drivers
> >  S:	Supported
> > +F:	drivers/iio/adc/ad7292.c
> >  F:	Documentation/devicetree/bindings/iio/adc/adi,ad7292.yaml
> >  
> >  ANALOG DEVICES INC AD7606 DRIVER
> > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> > index 632b331429c6..02587c990cb5 100644
> > --- a/drivers/iio/adc/Kconfig
> > +++ b/drivers/iio/adc/Kconfig
> > @@ -59,6 +59,16 @@ config AD7291
> >  	  To compile this driver as a module, choose M here: the
> >  	  module will be called ad7291.
> >  
> > +config AD7292
> > +	tristate "Analog Devices AD7292 ADC driver"
> > +	depends on SPI
> > +	help
> > +	  Say yes here to build support for Analog Devices AD7292
> > +	  8 Channel ADC with temperature sensor.
> > +
> > +	  To compile this driver as a module, choose M here: the
> > +	  module will be called ad7292.
> > +
> >  config AD7298
> >  	tristate "Analog Devices AD7298 ADC driver"
> >  	depends on SPI
> > diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> > index 4779ab3ff8fb..1818f2f66566 100644
> > --- a/drivers/iio/adc/Makefile
> > +++ b/drivers/iio/adc/Makefile
> > @@ -11,6 +11,7 @@ obj-$(CONFIG_AD7124) += ad7124.o
> >  obj-$(CONFIG_AD7173) += ad7173.o
> >  obj-$(CONFIG_AD7266) += ad7266.o
> >  obj-$(CONFIG_AD7291) += ad7291.o
> > +obj-$(CONFIG_AD7292) += ad7292.o
> >  obj-$(CONFIG_AD7298) += ad7298.o
> >  obj-$(CONFIG_AD738X) += ad738x.o
> >  obj-$(CONFIG_AD7768) += ad7768-1.o
> > diff --git a/drivers/iio/adc/ad7292.c b/drivers/iio/adc/ad7292.c
> > new file mode 100644
> > index 000000000000..e2badf6ac10d
> > --- /dev/null
> > +++ b/drivers/iio/adc/ad7292.c
> > @@ -0,0 +1,350 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Analog Devices AD7292 SPI ADC driver
> > + *
> > + * Copyright 2019 Analog Devices Inc.
> > + */
> > +
> > +#include <linux/bitfield.h>
> > +#include <linux/device.h>
> > +#include <linux/module.h>
> > +#include <linux/regulator/consumer.h>
> > +#include <linux/spi/spi.h>
> > +
> > +#include <linux/iio/iio.h>
> > +
> > +#define ADI_VENDOR_ID 0x0018
> > +
> > +/* AD7292 registers definition */
> > +#define AD7292_REG_VENDOR_ID		0x00
> > +#define AD7292_REG_CONF_BANK		0x05
> > +#define AD7292_REG_CONV_COMM		0x0E
> > +#define AD7292_REG_ADC_CH(x)		(0x10 + (x))
> > +
> > +/* AD7292 configuration bank subregisters definition */
> > +#define AD7292_BANK_REG_VIN_RNG0	0x10
> > +#define AD7292_BANK_REG_VIN_RNG1	0x11
> > +#define AD7292_BANK_REG_SAMP_MODE	0x12
> > +
> > +#define AD7292_RD_FLAG_MSK(x)		(BIT(7) | ((x) & 0x3F))
> > +
> > +/* AD7292_REG_ADC_CONVERSION */
> > +#define AD7292_ADC_DATA_MASK		GENMASK(15, 6)
> > +#define AD7292_ADC_DATA(x)		FIELD_GET(AD7292_ADC_DATA_MASK, x)
> > +
> > +/* AD7292_CHANNEL_SAMPLING_MODE */
> > +#define AD7292_CH_SAMP_MODE(reg, ch)	((reg >> 8) & BIT(ch))

Maybe this one would also need to be changed for the same reason: 
(((reg) >> 8) & BIT(ch)).
 
> > +
> > +/* AD7292_CHANNEL_VIN_RANGE */
> > +#define AD7292_CH_VIN_RANGE(reg, ch)	(reg & BIT(ch))
> 
> Trivial but all macro parameters should be in brackets to prevent
> weirdness if some sticks a parameter with higher precedence in there
> so ((reg) & BIT(ch)).
> 
> I can fix this up if the rest is fine.

Since I'll be respinning, I can add the brackets.

> 
> > +
> > +#define AD7292_VOLTAGE_CHAN(_chan)					\
> > +{									\
> > +	.type = IIO_VOLTAGE,						\
> > +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |			\
> > +			      BIT(IIO_CHAN_INFO_SCALE),			\
> > +	.indexed = 1,							\
> > +	.channel = _chan,						\
> > +}
> > +
> > +static const struct iio_chan_spec ad7292_channels[] = {
> > +	AD7292_VOLTAGE_CHAN(0),
> > +	AD7292_VOLTAGE_CHAN(1),
> > +	AD7292_VOLTAGE_CHAN(2),
> > +	AD7292_VOLTAGE_CHAN(3),
> > +	AD7292_VOLTAGE_CHAN(4),
> > +	AD7292_VOLTAGE_CHAN(5),
> > +	AD7292_VOLTAGE_CHAN(6),
> > +	AD7292_VOLTAGE_CHAN(7)
> > +};
> > +
> > +static const struct iio_chan_spec ad7292_channels_diff[] = {
> > +	{
> > +		.type = IIO_VOLTAGE,
> > +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> > +		.indexed = 1,
> > +		.differential = 1,
> > +		.channel = 0,
> > +		.channel2 = 1,
> > +	},
> > +	AD7292_VOLTAGE_CHAN(2),
> > +	AD7292_VOLTAGE_CHAN(3),
> > +	AD7292_VOLTAGE_CHAN(4),
> > +	AD7292_VOLTAGE_CHAN(5),
> > +	AD7292_VOLTAGE_CHAN(6),
> > +	AD7292_VOLTAGE_CHAN(7)
> > +};
> > +
> > +struct ad7292_state {
> > +	struct spi_device *spi;
> > +	struct regulator *reg;
> > +	unsigned short vref_mv;
> > +
> > +	__be16 d16 ____cacheline_aligned;
> > +	u8 d8[2];
> > +};
> > +
> > +static int ad7292_spi_reg_read(struct ad7292_state *st, unsigned int addr)
> > +{
> > +	int ret;
> > +
> > +	st->d8[0] = AD7292_RD_FLAG_MSK(addr);
> > +
> > +	ret = spi_write_then_read(st->spi, st->d8, 1, &st->d16, 2);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	return be16_to_cpu(st->d16);
> > +}
> > +
> > +static int ad7292_spi_subreg_read(struct ad7292_state *st, unsigned int addr,
> > +				  unsigned int sub_addr, unsigned int len)
> > +{
> > +	unsigned int shift = 16 - (8 * len);
> > +	int ret;
> > +
> > +	st->d8[0] = AD7292_RD_FLAG_MSK(addr);
> > +	st->d8[1] = sub_addr;
> > +
> > +	ret = spi_write_then_read(st->spi, st->d8, 2, &st->d16, len);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	return (be16_to_cpu(st->d16) >> shift);
> > +}
> > +
> > +static int ad7292_single_conversion(struct ad7292_state *st,
> > +				    unsigned int chan_addr)
> > +{
> > +	int ret;
> > +
> > +	struct spi_transfer t[] = {
> > +		{
> > +			.tx_buf = &st->d8,
> > +			.len = 4,
> > +			.delay_usecs = 6,
> > +		}, {
> > +			.rx_buf = &st->d16,
> > +			.len = 2,
> > +		},
> > +	};
> > +
> > +	st->d8[0] = chan_addr;
> > +	st->d8[1] = AD7292_RD_FLAG_MSK(AD7292_REG_CONV_COMM);
> > +
> > +	ret = spi_sync_transfer(st->spi, t, ARRAY_SIZE(t));
> > +
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	return be16_to_cpu(st->d16);
> > +}
> > +
> > +static int ad7292_vin_range_multiplier(struct ad7292_state *st, int channel)
> > +{
> > +	int samp_mode, range0, range1, factor = 1;
> > +
> > +	/*
> > +	 * Every AD7292 ADC channel may have its input range adjusted according
> > +	 * to the settings at the ADC sampling mode and VIN range subregisters.
> > +	 * For a given channel, the minimum input range is equal to Vref, and it
> > +	 * may be increased by a multiplier factor of 2 or 4 according to the
> > +	 * following rule:
> > +	 * If channel is being sampled with respect to AGND:
> > +	 *	factor = 4 if VIN range0 and VIN range1 equal 0
> > +	 *	factor = 2 if only one of VIN ranges equal 1
> > +	 *	factor = 1 if both VIN range0 and VIN range1 equal 1
> > +	 * If channel is being sampled with respect to AVDD:
> > +	 *	factor = 4 if VIN range0 and VIN range1 equal 0
> > +	 *	Behavior is undefined if any of VIN range doesn't equal 0
> > +	 */
> > +
> > +	samp_mode = ad7292_spi_subreg_read(st, AD7292_REG_CONF_BANK,
> > +					   AD7292_BANK_REG_SAMP_MODE, 2);
> > +
> > +	if (samp_mode < 0)
> > +		return samp_mode;
> > +
> > +	range0 = ad7292_spi_subreg_read(st, AD7292_REG_CONF_BANK,
> > +					AD7292_BANK_REG_VIN_RNG0, 2);
> > +
> > +	if (range0 < 0)
> > +		return range0;
> > +
> > +	range1 = ad7292_spi_subreg_read(st, AD7292_REG_CONF_BANK,
> > +					AD7292_BANK_REG_VIN_RNG1, 2);
> > +
> > +	if (range1 < 0)
> > +		return range1;
> > +
> > +	if (AD7292_CH_SAMP_MODE(samp_mode, channel)) {
> > +		/* Sampling with respect to AGND */
> > +		if (!AD7292_CH_VIN_RANGE(range0, channel))
> > +			factor *= 2;
> > +
> > +		if (!AD7292_CH_VIN_RANGE(range1, channel))
> > +			factor *= 2;
> > +
> > +	} else {
> > +		/* Sampling with respect to AVDD */
> > +		if (AD7292_CH_VIN_RANGE(range0, channel) ||
> > +		    AD7292_CH_VIN_RANGE(range1, channel))
> > +			return -EPERM;
> > +
> > +		factor = 4;
> > +	}
> > +
> > +	return factor;
> > +}
> > +
> > +static int ad7292_read_raw(struct iio_dev *indio_dev,
> > +			   const struct iio_chan_spec *chan,
> > +			   int *val, int *val2, long info)
> > +{
> > +	struct ad7292_state *st = iio_priv(indio_dev);
> > +	unsigned int ch_addr;
> > +	int ret;
> > +
> > +	switch (info) {
> > +	case IIO_CHAN_INFO_RAW:
> > +		ch_addr = AD7292_REG_ADC_CH(chan->channel);
> > +		ret = ad7292_single_conversion(st, ch_addr);
> > +		if (ret < 0)
> > +			return ret;
> > +
> > +		*val = AD7292_ADC_DATA(ret);
> > +
> > +		return IIO_VAL_INT;
> > +	case IIO_CHAN_INFO_SCALE:
> > +		/*
> > +		 * To convert a raw value to standard units, the IIO defines
> > +		 * this formula: Scaled value = (raw + offset) * scale.
> > +		 * For the scale to be a correct multiplier for (raw + offset),
> > +		 * it must be calculated as the input range divided by the
> > +		 * number of possible distinct input values. Given the ADC data
> > +		 * is 10 bit long, it may assume 2^10 distinct values.
> > +		 * Hence, scale = range / 2^10. The IIO_VAL_FRACTIONAL_LOG2
> > +		 * return type indicates to the IIO API to divide *val by 2 to
> > +		 * the power of *val2 when returning from read_raw.
> > +		 */
> > +
> > +		ret = ad7292_vin_range_multiplier(st, chan->channel);
> > +		if (ret < 0)
> > +			return ret;
> > +
> > +		*val = st->vref_mv * ret;
> > +		*val2 = 10;
> > +		return IIO_VAL_FRACTIONAL_LOG2;
> > +	default:
> > +		break;
> > +	}
> > +	return -EINVAL;
> > +}
> > +
> > +static const struct iio_info ad7292_info = {
> > +	.read_raw = ad7292_read_raw,
> > +};
> > +
> > +static void ad7292_regulator_disable(void *data)
> > +{
> > +	struct ad7292_state *st = data;
> > +
> > +	regulator_disable(st->reg);
> > +}
> > +
> > +static int ad7292_probe(struct spi_device *spi)
> > +{
> > +	struct ad7292_state *st;
> > +	struct iio_dev *indio_dev;
> > +	struct device_node *child;
> > +	bool diff_channels = 0;
> > +	int ret;
> > +
> > +	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
> > +	if (!indio_dev)
> > +		return -ENOMEM;
> > +
> > +	st = iio_priv(indio_dev);
> > +	st->spi = spi;
> > +
> > +	ret = ad7292_spi_reg_read(st, AD7292_REG_VENDOR_ID);
> > +	if (ret != ADI_VENDOR_ID) {
> > +		dev_err(&spi->dev, "Wrong vendor id 0x%x\n", ret);
> > +		return -EINVAL;
> > +	}
> > +
> > +	spi_set_drvdata(spi, indio_dev);
> > +
> > +	st->reg = devm_regulator_get_optional(&spi->dev, "vref");
> > +	if (!IS_ERR(st->reg)) {
> > +		ret = regulator_enable(st->reg);
> > +		if (ret) {
> > +			dev_err(&spi->dev,
> > +				"Failed to enable external vref supply\n");
> > +			return ret;
> > +		}
> > +
> > +		ret = devm_add_action_or_reset(&spi->dev,
> > +					       ad7292_regulator_disable, st);
> > +		if (ret) {
> > +			regulator_disable(st->reg);
> > +			return ret;
> > +		}
> > +
> > +		ret = regulator_get_voltage(st->reg);
> > +		if (ret < 0)
> > +			return ret;
> > +
> > +		st->vref_mv = ret / 1000;
> > +	} else {
> > +		/* Use the internal voltage reference. */
> > +		st->vref_mv = 1250;
> > +	}
> > +
> > +	indio_dev->dev.parent = &spi->dev;
> > +	indio_dev->name = spi_get_device_id(spi)->name;
> > +	indio_dev->modes = INDIO_DIRECT_MODE;
> > +	indio_dev->info = &ad7292_info;
> > +
> > +	for_each_available_child_of_node(spi->dev.of_node, child) {
> > +		diff_channels = of_property_read_bool(child, "diff-channels");
> > +		if (diff_channels)
> > +			break;
> > +	}
> > +
> > +	if (diff_channels) {
> > +		indio_dev->num_channels = ARRAY_SIZE(ad7292_channels_diff);
> > +		indio_dev->channels = ad7292_channels_diff;
> > +	} else {
> > +		indio_dev->num_channels = ARRAY_SIZE(ad7292_channels);
> > +		indio_dev->channels = ad7292_channels;
> > +	}
> > +
> > +	return devm_iio_device_register(&spi->dev, indio_dev);
> > +}
> > +
> > +static const struct spi_device_id ad7292_id_table[] = {
> > +	{ "ad7292", 0 },
> > +	{}
> > +};
> > +MODULE_DEVICE_TABLE(spi, ad7292_id_table);
> > +
> > +static const struct of_device_id ad7292_of_match[] = {
> > +	{ .compatible = "adi,ad7292" },
> > +	{ },
> > +};
> > +MODULE_DEVICE_TABLE(of, ad7292_of_match);
> > +
> > +static struct spi_driver ad7292_driver = {
> > +	.driver = {
> > +		.name = "ad7292",
> > +		.of_match_table = ad7292_of_match,
> > +	},
> > +	.probe = ad7292_probe,
> > +	.id_table = ad7292_id_table,
> > +};
> > +module_spi_driver(ad7292_driver);
> > +
> > +MODULE_AUTHOR("Marcelo Schmitt <marcelo.schmitt1@gmail.com>");
> > +MODULE_DESCRIPTION("Analog Devices AD7292 ADC driver");
> > +MODULE_LICENSE("GPL v2");
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, back to index

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-01 16:23 [PATCH v3 0/2] iio: adc: Add support for AD7292 Marcelo Schmitt
2019-11-01 16:23 ` [PATCH v3 1/2] dt-bindings: iio: adc: Add dt-schema " Marcelo Schmitt
2019-11-05 21:09   ` Rob Herring
2019-11-01 16:24 ` [PATCH v3 2/2] iio: adc: Add driver support " Marcelo Schmitt
2019-11-03 11:52   ` Jonathan Cameron
2019-11-07 16:38     ` Marcelo Schmitt

Linux-IIO Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-iio/0 linux-iio/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-iio linux-iio/ https://lore.kernel.org/linux-iio \
		linux-iio@vger.kernel.org
	public-inbox-index linux-iio

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-iio


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git