[PATCH v4 0/2] iio: adc: Add support for AD7292

[PATCH v4 0/2] iio: adc: Add support for AD7292

[Marcelo Schmitt]

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

Changelog V4:
- dt-binding: updated SPDX identifier to GPL-2.0-only
- dt-binding: changed maxitems constraint on channel property
- ad7292: added brackets to shield macro parameters

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          | 104 ++++++
 MAINTAINERS                                   |   8 +
 drivers/iio/adc/Kconfig                       |  10 +
 drivers/iio/adc/Makefile                      |   1 +
 drivers/iio/adc/ad7292.c                      | 350 ++++++++++++++++++
 5 files changed, 473 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad7292.yaml
 create mode 100644 drivers/iio/adc/ad7292.c

-- 
2.23.0

[PATCH v4 1/2] dt-bindings: iio: adc: Add dt-schema for AD7292

[Marcelo Schmitt]

Add a devicetree schema for AD7292 monitor and control system.

Signed-off-by: Marcelo Schmitt <marcelo.schmitt1@gmail.com>
---
Changelog V3 -> V4:
- updated SPDX identifier to GPL-2.0-only
- changed maxitems constraint on channel property

 .../bindings/iio/adc/adi,ad7292.yaml          | 104 ++++++++++++++++++
 MAINTAINERS                                   |   7 ++
 2 files changed, 111 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..b68be3aaf587
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7292.yaml
@@ -0,0 +1,104 @@
+# SPDX-License-Identifier: GPL-2.0-only
+%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.
+        items:
+          maximum: 7
+
+      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

[PATCH v4 2/2] iio: adc: Add driver support for AD7292

[Marcelo Schmitt]

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 V3 -> V4:
- added brackets to shield macro parameters

 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..a6798f7dfdb8
--- /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

Re: [PATCH v4 1/2] dt-bindings: iio: adc: Add dt-schema for AD7292

[Jonathan Cameron]

On Fri, 8 Nov 2019 10:56:09 -0300
Marcelo Schmitt <marcelo.schmitt1@gmail.com> wrote:

> Add a devicetree schema for AD7292 monitor and control system.
> 
> Signed-off-by: Marcelo Schmitt <marcelo.schmitt1@gmail.com>
Seems like you have addressed everything Rob raised, so I'll apply this
and it can go out for build testing.  Still some time for Rob to add
a tag if he wants to!

Thanks,

Jonathan

> ---
> Changelog V3 -> V4:
> - updated SPDX identifier to GPL-2.0-only
> - changed maxitems constraint on channel property
> 
>  .../bindings/iio/adc/adi,ad7292.yaml          | 104 ++++++++++++++++++
>  MAINTAINERS                                   |   7 ++
>  2 files changed, 111 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..b68be3aaf587
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7292.yaml
> @@ -0,0 +1,104 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +%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.
> +        items:
> +          maximum: 7
> +
> +      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

Re: [PATCH v4 2/2] iio: adc: Add driver support for AD7292

[Jonathan Cameron]

On Fri, 8 Nov 2019 10:56:50 -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>
Looks good to me.

Applied to the togreg branch of iio.git and pushed out as testing for the
autobuilders to play with it.

Thanks,

Jonathan

> ---
> Changelog V3 -> V4:
> - added brackets to shield macro parameters
> 
>  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..a6798f7dfdb8
> --- /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");

Re: [PATCH v4 1/2] dt-bindings: iio: adc: Add dt-schema for AD7292

[Rob Herring]

On Fri, Nov 08, 2019 at 10:56:09AM -0300, Marcelo Schmitt wrote:
> Add a devicetree schema for AD7292 monitor and control system.
> 
> Signed-off-by: Marcelo Schmitt <marcelo.schmitt1@gmail.com>
> ---
> Changelog V3 -> V4:
> - updated SPDX identifier to GPL-2.0-only
> - changed maxitems constraint on channel property
> 
>  .../bindings/iio/adc/adi,ad7292.yaml          | 104 ++++++++++++++++++
>  MAINTAINERS                                   |   7 ++
>  2 files changed, 111 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..b68be3aaf587
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7292.yaml
> @@ -0,0 +1,104 @@
> +# SPDX-License-Identifier: GPL-2.0-only

Sigh, I gave you the exact line to use:

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

I've said to dual license with (GPL-2.0-only OR BSD-2-Clause) and people 
think I mean to pick one. So now I just give the whole line. I don't 
know how to be clearer.

> +%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.
> +        items:
> +          maximum: 7

Not what I said either. A slight but important difference in that you 
are missing a '-' to make 'items' a list rather than a schema/dict.

Update dt-schema. This should give a warning now.

> +
> +      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
> 

Re: [PATCH v4 1/2] dt-bindings: iio: adc: Add dt-schema for AD7292

[Marcelo Schmitt]

Hi Rob,

Thanks for reviewing the binding doc again.
Aparently, this patch was added to Greg KH's staging tree.
What is the right procedure in this case? Should I send a v5 patchset or
just send a patch for this doc?

In any case, I still have some doubts about the maximum constraint of
the channel property. Comments inline.


Thanks

Marcelo

On 11/12, Rob Herring wrote:
> On Fri, Nov 08, 2019 at 10:56:09AM -0300, Marcelo Schmitt wrote:
> > Add a devicetree schema for AD7292 monitor and control system.
> > 
> > Signed-off-by: Marcelo Schmitt <marcelo.schmitt1@gmail.com>
> > ---
> > Changelog V3 -> V4:
> > - updated SPDX identifier to GPL-2.0-only
> > - changed maxitems constraint on channel property
> > 
> >  .../bindings/iio/adc/adi,ad7292.yaml          | 104 ++++++++++++++++++
> >  MAINTAINERS                                   |   7 ++
> >  2 files changed, 111 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..b68be3aaf587
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7292.yaml
> > @@ -0,0 +1,104 @@
> > +# SPDX-License-Identifier: GPL-2.0-only
> 
> Sigh, I gave you the exact line to use:
> 
> # SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> 
> I've said to dual license with (GPL-2.0-only OR BSD-2-Clause) and people 
> think I mean to pick one. So now I just give the whole line. I don't 
> know how to be clearer.

I thought I could use just GPL-2.0 since the driver code is GPL-2.0.
Anyway, I'll use the above line to specify the dt-binding license.

> 
> > +%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.
> > +        items:
> > +          maximum: 7
> 
> Not what I said either. A slight but important difference in that you 
> are missing a '-' to make 'items' a list rather than a schema/dict.
> 
> Update dt-schema. This should give a warning now.

I'm confused, I don't know how to make this doc the way you want.
I pulled the updates from the master branch of dt-schema repo and
reinstalled it.
Then I tried
        items:
          - maximum: 7
I've tried
        - items:
            maximum: 7
I also tried
        - items:
          maximum: 7
all gave me parsing errors when processing the ad7292 schema with 
'make dt_binding_check' and also with 'make -k dt_binding_check'.
Am I using the right branch? Should I pull from a branch other than the
master?
I was first inspired by the adi,ad7124.yaml doc which has a similar
channel declaration. Curiously, processing the ad7292 schema the way it
was in v4 gave me no errors so, I must be missing something.

> 
> > +
> > +      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
> > 

Re: [PATCH v4 1/2] dt-bindings: iio: adc: Add dt-schema for AD7292

[Rob Herring]

On Wed, Nov 13, 2019 at 8:52 AM Marcelo Schmitt
<marcelo.schmitt1@gmail.com> wrote:
>
> Hi Rob,
>
> Thanks for reviewing the binding doc again.
> Aparently, this patch was added to Greg KH's staging tree.
> What is the right procedure in this case? Should I send a v5 patchset or
> just send a patch for this doc?

You need to do an incremental patch. Greg doesn't rebase.

> In any case, I still have some doubts about the maximum constraint of
> the channel property. Comments inline.
>
>
> Thanks
>
> Marcelo
>
> On 11/12, Rob Herring wrote:
> > On Fri, Nov 08, 2019 at 10:56:09AM -0300, Marcelo Schmitt wrote:
> > > Add a devicetree schema for AD7292 monitor and control system.
> > >
> > > Signed-off-by: Marcelo Schmitt <marcelo.schmitt1@gmail.com>
> > > ---
> > > Changelog V3 -> V4:
> > > - updated SPDX identifier to GPL-2.0-only
> > > - changed maxitems constraint on channel property
> > >
> > >  .../bindings/iio/adc/adi,ad7292.yaml          | 104 ++++++++++++++++++
> > >  MAINTAINERS                                   |   7 ++
> > >  2 files changed, 111 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..b68be3aaf587
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7292.yaml
> > > @@ -0,0 +1,104 @@
> > > +# SPDX-License-Identifier: GPL-2.0-only
> >
> > Sigh, I gave you the exact line to use:
> >
> > # SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> >
> > I've said to dual license with (GPL-2.0-only OR BSD-2-Clause) and people
> > think I mean to pick one. So now I just give the whole line. I don't
> > know how to be clearer.
>
> I thought I could use just GPL-2.0 since the driver code is GPL-2.0.
> Anyway, I'll use the above line to specify the dt-binding license.
>
> >
> > > +%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.
> > > +        items:
> > > +          maximum: 7
> >
> > Not what I said either. A slight but important difference in that you
> > are missing a '-' to make 'items' a list rather than a schema/dict.
> >
> > Update dt-schema. This should give a warning now.
>
> I'm confused, I don't know how to make this doc the way you want.
> I pulled the updates from the master branch of dt-schema repo and
> reinstalled it.
> Then I tried
>         items:
>           - maximum: 7
> I've tried
>         - items:
>             maximum: 7
> I also tried
>         - items:
>           maximum: 7
> all gave me parsing errors when processing the ad7292 schema with
> 'make dt_binding_check' and also with 'make -k dt_binding_check'.
> Am I using the right branch? Should I pull from a branch other than the
> master?

Sorry, my fault there. The meta-schema requires 'minimum' if you give
'maximum'. So:

        items:
          - minimum: 0
            maximum: 7

The error message was less than useful, but I think I have a fix for that too.

Rob

Re: [PATCH v4 1/2] dt-bindings: iio: adc: Add dt-schema for AD7292

[Jonathan Cameron]

On Wed, 13 Nov 2019 11:06:45 -0600
Rob Herring <robh@kernel.org> wrote:

> On Wed, Nov 13, 2019 at 8:52 AM Marcelo Schmitt
> <marcelo.schmitt1@gmail.com> wrote:
> >
> > Hi Rob,
> >
> > Thanks for reviewing the binding doc again.
> > Aparently, this patch was added to Greg KH's staging tree.
> > What is the right procedure in this case? Should I send a v5 patchset or
> > just send a patch for this doc?  
> 
> You need to do an incremental patch. Greg doesn't rebase.
Sorry. I jumped the gun a bit here.  Send me an incremental patch and
we'll get it queued up.  As it can be considered 'a license fix'
we can get it lined up for straight after the merge window.

Thanks,

Jonathan


> 
> > In any case, I still have some doubts about the maximum constraint of
> > the channel property. Comments inline.
> >
> >
> > Thanks
> >
> > Marcelo
> >
> > On 11/12, Rob Herring wrote:  
> > > On Fri, Nov 08, 2019 at 10:56:09AM -0300, Marcelo Schmitt wrote:  
> > > > Add a devicetree schema for AD7292 monitor and control system.
> > > >
> > > > Signed-off-by: Marcelo Schmitt <marcelo.schmitt1@gmail.com>
> > > > ---
> > > > Changelog V3 -> V4:
> > > > - updated SPDX identifier to GPL-2.0-only
> > > > - changed maxitems constraint on channel property
> > > >
> > > >  .../bindings/iio/adc/adi,ad7292.yaml          | 104 ++++++++++++++++++
> > > >  MAINTAINERS                                   |   7 ++
> > > >  2 files changed, 111 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..b68be3aaf587
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7292.yaml
> > > > @@ -0,0 +1,104 @@
> > > > +# SPDX-License-Identifier: GPL-2.0-only  
> > >
> > > Sigh, I gave you the exact line to use:
> > >
> > > # SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > >
> > > I've said to dual license with (GPL-2.0-only OR BSD-2-Clause) and people
> > > think I mean to pick one. So now I just give the whole line. I don't
> > > know how to be clearer.  
> >
> > I thought I could use just GPL-2.0 since the driver code is GPL-2.0.
> > Anyway, I'll use the above line to specify the dt-binding license.
> >  
> > >  
> > > > +%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.
> > > > +        items:
> > > > +          maximum: 7  
> > >
> > > Not what I said either. A slight but important difference in that you
> > > are missing a '-' to make 'items' a list rather than a schema/dict.
> > >
> > > Update dt-schema. This should give a warning now.  
> >
> > I'm confused, I don't know how to make this doc the way you want.
> > I pulled the updates from the master branch of dt-schema repo and
> > reinstalled it.
> > Then I tried
> >         items:
> >           - maximum: 7
> > I've tried
> >         - items:
> >             maximum: 7
> > I also tried
> >         - items:
> >           maximum: 7
> > all gave me parsing errors when processing the ad7292 schema with
> > 'make dt_binding_check' and also with 'make -k dt_binding_check'.
> > Am I using the right branch? Should I pull from a branch other than the
> > master?  
> 
> Sorry, my fault there. The meta-schema requires 'minimum' if you give
> 'maximum'. So:
> 
>         items:
>           - minimum: 0
>             maximum: 7
> 
> The error message was less than useful, but I think I have a fix for that too.
> 
> Rob