[PATCH 0/3] iio: adc: ad7192: Add support for AD7194

[PATCH 0/3] iio: adc: ad7192: Add support for AD7194

[alisadariana]

From: Alisa-Dariana Roman <alisa.roman@analog.com>

Dear maintainers,

I am submitting a series of patches to improve the ad7192 driver by
adding suport for AD7194 also.

Please consider applying these patches in order.

Thank you for your time and attention.

Kind regards,

Alisa-Dariana Roman (3):
  iio: adc: ad7192: Use device api
  dt-bindings: iio: adc: ad7192: Add AD7194 support
  iio: adc: ad7192: Add AD7194 support

 .../bindings/iio/adc/adi,ad7192.yaml          |  69 +++++++
 drivers/iio/adc/Kconfig                       |   4 +-
 drivers/iio/adc/ad7192.c                      | 176 ++++++++++++++++--
 3 files changed, 228 insertions(+), 21 deletions(-)

-- 
2.34.1

[PATCH 1/3] iio: adc: ad7192: Use device api

[alisadariana]

From: Alisa-Dariana Roman <alisa.roman@analog.com>

Replace of.h and corresponding functions with preferred device specific
functions.

Also replace of_device_get_match_data function with
spi_get_device_match_data.

Signed-off-by: Alisa-Dariana Roman <alisa.roman@analog.com>
---
 drivers/iio/adc/ad7192.c | 32 +++++++++++++++-----------------
 1 file changed, 15 insertions(+), 17 deletions(-)

diff --git a/drivers/iio/adc/ad7192.c b/drivers/iio/adc/ad7192.c
index adc3cbe92d6e..48e0357564af 100644
--- a/drivers/iio/adc/ad7192.c
+++ b/drivers/iio/adc/ad7192.c
@@ -17,7 +17,6 @@
 #include <linux/err.h>
 #include <linux/sched.h>
 #include <linux/delay.h>
-#include <linux/of.h>
 
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
@@ -364,19 +363,19 @@ static inline bool ad7192_valid_external_frequency(u32 freq)
 		freq <= AD7192_EXT_FREQ_MHZ_MAX);
 }
 
-static int ad7192_of_clock_select(struct ad7192_state *st)
+static int ad7192_device_clock_select(struct ad7192_state *st)
 {
-	struct device_node *np = st->sd.spi->dev.of_node;
+	struct device *dev = &st->sd.spi->dev;
 	unsigned int clock_sel;
 
 	clock_sel = AD7192_CLK_INT;
 
 	/* use internal clock */
 	if (!st->mclk) {
-		if (of_property_read_bool(np, "adi,int-clock-output-enable"))
+		if (device_property_read_bool(dev, "adi,int-clock-output-enable"))
 			clock_sel = AD7192_CLK_INT_CO;
 	} else {
-		if (of_property_read_bool(np, "adi,clock-xtal"))
+		if (device_property_read_bool(dev, "adi,clock-xtal"))
 			clock_sel = AD7192_CLK_EXT_MCLK1_2;
 		else
 			clock_sel = AD7192_CLK_EXT_MCLK2;
@@ -385,9 +384,10 @@ static int ad7192_of_clock_select(struct ad7192_state *st)
 	return clock_sel;
 }
 
-static int ad7192_setup(struct iio_dev *indio_dev, struct device_node *np)
+static int ad7192_setup(struct iio_dev *indio_dev)
 {
 	struct ad7192_state *st = iio_priv(indio_dev);
+	struct device *dev = &st->sd.spi->dev;
 	bool rej60_en, refin2_en;
 	bool buf_en, bipolar, burnout_curr_en;
 	unsigned long long scale_uv;
@@ -416,26 +416,26 @@ static int ad7192_setup(struct iio_dev *indio_dev, struct device_node *np)
 
 	st->conf = FIELD_PREP(AD7192_CONF_GAIN_MASK, 0);
 
-	rej60_en = of_property_read_bool(np, "adi,rejection-60-Hz-enable");
+	rej60_en = device_property_read_bool(dev, "adi,rejection-60-Hz-enable");
 	if (rej60_en)
 		st->mode |= AD7192_MODE_REJ60;
 
-	refin2_en = of_property_read_bool(np, "adi,refin2-pins-enable");
+	refin2_en = device_property_read_bool(dev, "adi,refin2-pins-enable");
 	if (refin2_en && st->chip_info->chip_id != CHIPID_AD7195)
 		st->conf |= AD7192_CONF_REFSEL;
 
 	st->conf &= ~AD7192_CONF_CHOP;
 
-	buf_en = of_property_read_bool(np, "adi,buffer-enable");
+	buf_en = device_property_read_bool(dev, "adi,buffer-enable");
 	if (buf_en)
 		st->conf |= AD7192_CONF_BUF;
 
-	bipolar = of_property_read_bool(np, "bipolar");
+	bipolar = device_property_read_bool(dev, "bipolar");
 	if (!bipolar)
 		st->conf |= AD7192_CONF_UNIPOLAR;
 
-	burnout_curr_en = of_property_read_bool(np,
-						"adi,burnout-currents-enable");
+	burnout_curr_en =
+		device_property_read_bool(dev, "adi,burnout-currents-enable");
 	if (burnout_curr_en && buf_en) {
 		st->conf |= AD7192_CONF_BURN;
 	} else if (burnout_curr_en) {
@@ -1117,9 +1117,7 @@ static int ad7192_probe(struct spi_device *spi)
 	}
 	st->int_vref_mv = ret / 1000;
 
-	st->chip_info = of_device_get_match_data(&spi->dev);
-	if (!st->chip_info)
-		st->chip_info = (void *)spi_get_device_id(spi)->driver_data;
+	st->chip_info = spi_get_device_match_data(spi);
 	indio_dev->name = st->chip_info->name;
 	indio_dev->modes = INDIO_DIRECT_MODE;
 	indio_dev->channels = st->chip_info->channels;
@@ -1140,7 +1138,7 @@ static int ad7192_probe(struct spi_device *spi)
 	if (IS_ERR(st->mclk))
 		return PTR_ERR(st->mclk);
 
-	st->clock_sel = ad7192_of_clock_select(st);
+	st->clock_sel = ad7192_device_clock_select(st);
 
 	if (st->clock_sel == AD7192_CLK_EXT_MCLK1_2 ||
 	    st->clock_sel == AD7192_CLK_EXT_MCLK2) {
@@ -1152,7 +1150,7 @@ static int ad7192_probe(struct spi_device *spi)
 		}
 	}
 
-	ret = ad7192_setup(indio_dev, spi->dev.of_node);
+	ret = ad7192_setup(indio_dev);
 	if (ret)
 		return ret;
 
-- 
2.34.1

[PATCH 2/3] dt-bindings: iio: adc: ad7192: Add AD7194 support

[alisadariana]

From: Alisa-Dariana Roman <alisa.roman@analog.com>

Unlike the other AD719Xs, AD7194 has configurable differential
channels. The default configuration for these channels can be changed
from the devicetree.

Also add an example for AD7194 devicetree.

Signed-off-by: Alisa-Dariana Roman <alisa.roman@analog.com>
---
 .../bindings/iio/adc/adi,ad7192.yaml          | 69 +++++++++++++++++++
 1 file changed, 69 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7192.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7192.yaml
index 16def2985ab4..b9a9f7b20670 100644
--- a/Documentation/devicetree/bindings/iio/adc/adi,ad7192.yaml
+++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7192.yaml
@@ -21,8 +21,15 @@ properties:
       - adi,ad7190
       - adi,ad7192
       - adi,ad7193
+      - adi,ad7194
       - adi,ad7195
 
+  '#address-cells':
+    const: 1
+
+  '#size-cells':
+    const: 0
+
   reg:
     maxItems: 1
 
@@ -96,6 +103,31 @@ required:
   - spi-cpol
   - spi-cpha
 
+patternProperties:
+  "^channel@([0-9a-f])$":
+    type: object
+    $ref: adc.yaml
+    unevaluatedProperties: false
+
+    properties:
+      reg:
+        description: The channel index.
+        minimum: 0
+        maximum: 7
+
+      diff-channels:
+        description: |
+          The differential channel pair for Ad7194 configurable channels. The
+          first channel is the positive input, the second channel is the
+          negative input.
+        items:
+          minimum: 1
+          maximum: 16
+
+    required:
+      - reg
+      - diff-channels
+
 allOf:
   - $ref: /schemas/spi/spi-peripheral-props.yaml#
 
@@ -127,3 +159,40 @@ examples:
             adi,burnout-currents-enable;
         };
     };
+  - |
+    spi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        adc@0 {
+            #address-cells = <1>;
+            #size-cells = <0>;
+            compatible = "adi,ad7194";
+            reg = <0>;
+            spi-max-frequency = <1000000>;
+            spi-cpol;
+            spi-cpha;
+            clocks = <&ad7192_mclk>;
+            clock-names = "mclk";
+            interrupts = <25 0x2>;
+            interrupt-parent = <&gpio>;
+            dvdd-supply = <&dvdd>;
+            avdd-supply = <&avdd>;
+            vref-supply = <&vref>;
+
+            adi,refin2-pins-enable;
+            adi,rejection-60-Hz-enable;
+            adi,buffer-enable;
+            adi,burnout-currents-enable;
+
+            channel@0 {
+                reg = <0>;
+                diff-channels = <1 6>;
+            };
+
+            channel@1 {
+                reg = <1>;
+                diff-channels = <2 3>;
+            };
+        };
+    };
-- 
2.34.1

[PATCH 3/3] iio: adc: ad7192: Add AD7194 support

[alisadariana]

From: Alisa-Dariana Roman <alisa.roman@analog.com>

Unlike the other AD719Xs, AD7194 has configurable differential
channels. The default configuration for these channels can be changed
from the devicetree.

The default configuration is hardcoded in order to have a stable number
of channels.

Signed-off-by: Alisa-Dariana Roman <alisa.roman@analog.com>
---
 drivers/iio/adc/Kconfig  |   4 +-
 drivers/iio/adc/ad7192.c | 144 ++++++++++++++++++++++++++++++++++++++-
 2 files changed, 144 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 1e2b7a2c67c6..020d37457d81 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -55,12 +55,12 @@ config AD7124
 	  called ad7124.
 
 config AD7192
-	tristate "Analog Devices AD7190 AD7192 AD7193 AD7195 ADC driver"
+	tristate "Analog Devices AD7190 AD7192 AD7193 AD7194 AD7195 ADC driver"
 	depends on SPI
 	select AD_SIGMA_DELTA
 	help
 	  Say yes here to build support for Analog Devices AD7190,
-	  AD7192, AD7193 or AD7195 SPI analog to digital converters (ADC).
+	  AD7192, AD7193, AD7194 or AD7195 SPI analog to digital converters (ADC).
 	  If unsure, say N (but it's safe to say "Y").
 
 	  To compile this driver as a module, choose M here: the
diff --git a/drivers/iio/adc/ad7192.c b/drivers/iio/adc/ad7192.c
index 48e0357564af..9c716c5c18b3 100644
--- a/drivers/iio/adc/ad7192.c
+++ b/drivers/iio/adc/ad7192.c
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0
 /*
- * AD7190 AD7192 AD7193 AD7195 SPI ADC driver
+ * AD7190 AD7192 AD7193 AD7194 AD7195 SPI ADC driver
  *
  * Copyright 2011-2015 Analog Devices Inc.
  */
@@ -125,10 +125,39 @@
 #define AD7193_CH_AIN8		0x480 /* AIN7 - AINCOM */
 #define AD7193_CH_AINCOM	0x600 /* AINCOM - AINCOM */
 
+#define AD7194_CH_TEMP		0x100 /* Temp sensor */
+#define AD7194_CH_AIN1		0x400 /* AIN1 - AINCOM */
+#define AD7194_CH_AIN2		0x410 /* AIN2 - AINCOM */
+#define AD7194_CH_AIN3		0x420 /* AIN3 - AINCOM */
+#define AD7194_CH_AIN4		0x430 /* AIN4 - AINCOM */
+#define AD7194_CH_AIN5		0x440 /* AIN5 - AINCOM */
+#define AD7194_CH_AIN6		0x450 /* AIN6 - AINCOM */
+#define AD7194_CH_AIN7		0x460 /* AIN7 - AINCOM */
+#define AD7194_CH_AIN8		0x470 /* AIN8 - AINCOM */
+#define AD7194_CH_AIN9		0x480 /* AIN9 - AINCOM */
+#define AD7194_CH_AIN10		0x490 /* AIN10 - AINCOM */
+#define AD7194_CH_AIN11		0x4A0 /* AIN11 - AINCOM */
+#define AD7194_CH_AIN12		0x4B0 /* AIN12 - AINCOM */
+#define AD7194_CH_AIN13		0x4C0 /* AIN13 - AINCOM */
+#define AD7194_CH_AIN14		0x4D0 /* AIN14 - AINCOM */
+#define AD7194_CH_AIN15		0x4E0 /* AIN15 - AINCOM */
+#define AD7194_CH_AIN16		0x4F0 /* AIN16 - AINCOM */
+#define AD7194_CH_POS_MASK	GENMASK(7, 4)
+#define AD7194_CH_POS(x)	FIELD_PREP(AD7194_CH_POS_MASK, (x))
+#define AD7194_CH_NEG_MASK	GENMASK(3, 0)
+#define AD7194_CH_NEG(x)	FIELD_PREP(AD7194_CH_NEG_MASK, (x))
+#define AD7194_CH_DIFF(pos, neg) \
+		(AD7194_CH_POS(pos) | AD7194_CH_NEG(neg))
+#define AD7194_CH_DIFF_NR_MIN	0
+#define AD7194_CH_DIFF_NR_MAX	7
+#define AD7194_CH_AIN_MIN	1
+#define AD7194_CH_AIN_MAX	16
+
 /* ID Register Bit Designations (AD7192_REG_ID) */
 #define CHIPID_AD7190		0x4
 #define CHIPID_AD7192		0x0
 #define CHIPID_AD7193		0x2
+#define CHIPID_AD7194		0x3
 #define CHIPID_AD7195		0x6
 #define AD7192_ID_MASK		GENMASK(3, 0)
 
@@ -166,6 +195,7 @@ enum {
 	ID_AD7190,
 	ID_AD7192,
 	ID_AD7193,
+	ID_AD7194,
 	ID_AD7195,
 };
 
@@ -644,6 +674,15 @@ static const struct attribute_group ad7192_attribute_group = {
 	.attrs = ad7192_attributes,
 };
 
+static struct attribute *ad7194_attributes[] = {
+	&iio_dev_attr_filter_low_pass_3db_frequency_available.dev_attr.attr,
+	NULL
+};
+
+static const struct attribute_group ad7194_attribute_group = {
+	.attrs = ad7194_attributes,
+};
+
 static struct attribute *ad7195_attributes[] = {
 	&iio_dev_attr_filter_low_pass_3db_frequency_available.dev_attr.attr,
 	&iio_dev_attr_bridge_switch_en.dev_attr.attr,
@@ -928,6 +967,16 @@ static const struct iio_info ad7192_info = {
 	.update_scan_mode = ad7192_update_scan_mode,
 };
 
+static const struct iio_info ad7194_info = {
+	.read_raw = ad7192_read_raw,
+	.write_raw = ad7192_write_raw,
+	.write_raw_get_fmt = ad7192_write_raw_get_fmt,
+	.read_avail = ad7192_read_avail,
+	.attrs = &ad7194_attribute_group,
+	.validate_trigger = ad_sd_validate_trigger,
+	.update_scan_mode = ad7192_update_scan_mode,
+};
+
 static const struct iio_info ad7195_info = {
 	.read_raw = ad7192_read_raw,
 	.write_raw = ad7192_write_raw,
@@ -1017,6 +1066,35 @@ static const struct iio_chan_spec ad7193_channels[] = {
 	IIO_CHAN_SOFT_TIMESTAMP(14),
 };
 
+static struct iio_chan_spec ad7194_channels[] = {
+	AD7193_DIFF_CHANNEL(0, 1, 2, 0x001),
+	AD7193_DIFF_CHANNEL(1, 3, 4, 0x023),
+	AD7193_DIFF_CHANNEL(2, 5, 6, 0x045),
+	AD7193_DIFF_CHANNEL(3, 7, 8, 0x067),
+	AD7193_DIFF_CHANNEL(4, 9, 10, 0x089),
+	AD7193_DIFF_CHANNEL(5, 11, 12, 0x0AB),
+	AD7193_DIFF_CHANNEL(6, 13, 14, 0x0CD),
+	AD7193_DIFF_CHANNEL(7, 15, 16, 0x0EF),
+	AD719x_TEMP_CHANNEL(8, AD7194_CH_TEMP),
+	AD7193_CHANNEL(9, 1, AD7194_CH_AIN1),
+	AD7193_CHANNEL(10, 2, AD7194_CH_AIN2),
+	AD7193_CHANNEL(11, 3, AD7194_CH_AIN3),
+	AD7193_CHANNEL(12, 4, AD7194_CH_AIN4),
+	AD7193_CHANNEL(13, 5, AD7194_CH_AIN5),
+	AD7193_CHANNEL(14, 6, AD7194_CH_AIN6),
+	AD7193_CHANNEL(15, 7, AD7194_CH_AIN7),
+	AD7193_CHANNEL(16, 8, AD7194_CH_AIN8),
+	AD7193_CHANNEL(17, 9, AD7194_CH_AIN9),
+	AD7193_CHANNEL(18, 10, AD7194_CH_AIN10),
+	AD7193_CHANNEL(19, 11, AD7194_CH_AIN11),
+	AD7193_CHANNEL(20, 12, AD7194_CH_AIN12),
+	AD7193_CHANNEL(21, 13, AD7194_CH_AIN13),
+	AD7193_CHANNEL(22, 14, AD7194_CH_AIN14),
+	AD7193_CHANNEL(23, 15, AD7194_CH_AIN15),
+	AD7193_CHANNEL(24, 16, AD7194_CH_AIN16),
+	IIO_CHAN_SOFT_TIMESTAMP(25),
+};
+
 static const struct ad7192_chip_info ad7192_chip_info_tbl[] = {
 	[ID_AD7190] = {
 		.chip_id = CHIPID_AD7190,
@@ -1039,6 +1117,13 @@ static const struct ad7192_chip_info ad7192_chip_info_tbl[] = {
 		.num_channels = ARRAY_SIZE(ad7193_channels),
 		.info = &ad7192_info,
 	},
+	[ID_AD7194] = {
+		.chip_id = CHIPID_AD7194,
+		.name = "ad7194",
+		.channels = ad7194_channels,
+		.num_channels = ARRAY_SIZE(ad7194_channels),
+		.info = &ad7194_info,
+	},
 	[ID_AD7195] = {
 		.chip_id = CHIPID_AD7195,
 		.name = "ad7195",
@@ -1053,6 +1138,53 @@ static void ad7192_reg_disable(void *reg)
 	regulator_disable(reg);
 }
 
+static int ad7192_parse_channel(struct iio_dev *indio_dev,
+				struct fwnode_handle *child)
+{
+	u32 reg, ain[2];
+	int ret;
+
+	ret = fwnode_property_read_u32(child, "reg", &reg);
+	if (ret)
+		return ret;
+
+	if (reg < AD7194_CH_DIFF_NR_MIN || reg > AD7194_CH_DIFF_NR_MAX)
+		return -EINVAL;
+
+	ret = fwnode_property_read_u32_array(child, "diff-channels", ain,
+					     ARRAY_SIZE(ain));
+	if (ret)
+		return ret;
+
+	if (ain[0] < AD7194_CH_AIN_MIN || ain[0] > AD7194_CH_AIN_MAX ||
+	    ain[1] < AD7194_CH_AIN_MIN || ain[1] > AD7194_CH_AIN_MAX)
+		return -EINVAL;
+
+	ad7194_channels[reg].channel = ain[0];
+	ad7194_channels[reg].channel2 = ain[1];
+	ad7194_channels[reg].address = AD7194_CH_DIFF(ain[0], ain[1]);
+
+	return 0;
+}
+
+static int ad7192_parse_channels(struct iio_dev *indio_dev)
+{
+	struct ad7192_state *st = iio_priv(indio_dev);
+	struct device *dev = &st->sd.spi->dev;
+	struct fwnode_handle *child;
+	int ret;
+
+	device_for_each_child_node(dev, child) {
+		ret = ad7192_parse_channel(indio_dev, child);
+		if (ret) {
+			fwnode_handle_put(child);
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
 static int ad7192_probe(struct spi_device *spi)
 {
 	struct ad7192_state *st;
@@ -1150,6 +1282,12 @@ static int ad7192_probe(struct spi_device *spi)
 		}
 	}
 
+	if (st->chip_info->chip_id == CHIPID_AD7194) {
+		ret = ad7192_parse_channels(indio_dev);
+		if (ret)
+			return ret;
+	}
+
 	ret = ad7192_setup(indio_dev);
 	if (ret)
 		return ret;
@@ -1161,6 +1299,7 @@ static const struct of_device_id ad7192_of_match[] = {
 	{ .compatible = "adi,ad7190", .data = &ad7192_chip_info_tbl[ID_AD7190] },
 	{ .compatible = "adi,ad7192", .data = &ad7192_chip_info_tbl[ID_AD7192] },
 	{ .compatible = "adi,ad7193", .data = &ad7192_chip_info_tbl[ID_AD7193] },
+	{ .compatible = "adi,ad7194", .data = &ad7192_chip_info_tbl[ID_AD7194] },
 	{ .compatible = "adi,ad7195", .data = &ad7192_chip_info_tbl[ID_AD7195] },
 	{}
 };
@@ -1170,6 +1309,7 @@ static const struct spi_device_id ad7192_ids[] = {
 	{ "ad7190", (kernel_ulong_t)&ad7192_chip_info_tbl[ID_AD7190] },
 	{ "ad7192", (kernel_ulong_t)&ad7192_chip_info_tbl[ID_AD7192] },
 	{ "ad7193", (kernel_ulong_t)&ad7192_chip_info_tbl[ID_AD7193] },
+	{ "ad7194", (kernel_ulong_t)&ad7192_chip_info_tbl[ID_AD7194] },
 	{ "ad7195", (kernel_ulong_t)&ad7192_chip_info_tbl[ID_AD7195] },
 	{}
 };
@@ -1186,6 +1326,6 @@ static struct spi_driver ad7192_driver = {
 module_spi_driver(ad7192_driver);
 
 MODULE_AUTHOR("Michael Hennerich <michael.hennerich@analog.com>");
-MODULE_DESCRIPTION("Analog Devices AD7190, AD7192, AD7193, AD7195 ADC");
+MODULE_DESCRIPTION("Analog Devices AD7190, AD7192, AD7193, AD7194, AD7195 ADC");
 MODULE_LICENSE("GPL v2");
 MODULE_IMPORT_NS(IIO_AD_SIGMA_DELTA);
-- 
2.34.1

Re: [PATCH 2/3] dt-bindings: iio: adc: ad7192: Add AD7194 support

[Krzysztof Kozlowski]

On 05/11/2023 20:31, alisadariana@gmail.com wrote:
> From: Alisa-Dariana Roman <alisa.roman@analog.com>
> 
> Unlike the other AD719Xs, AD7194 has configurable differential
> channels. The default configuration for these channels can be changed
> from the devicetree.
> 
> Also add an example for AD7194 devicetree.
> 
> Signed-off-by: Alisa-Dariana Roman <alisa.roman@analog.com>
> ---
>  .../bindings/iio/adc/adi,ad7192.yaml          | 69 +++++++++++++++++++
>  1 file changed, 69 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7192.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7192.yaml
> index 16def2985ab4..b9a9f7b20670 100644
> --- a/Documentation/devicetree/bindings/iio/adc/adi,ad7192.yaml
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7192.yaml
> @@ -21,8 +21,15 @@ properties:
>        - adi,ad7190
>        - adi,ad7192
>        - adi,ad7193
> +      - adi,ad7194
>        - adi,ad7195
>  
> +  '#address-cells':
> +    const: 1
> +
> +  '#size-cells':
> +    const: 0
> +
>    reg:
>      maxItems: 1
>  
> @@ -96,6 +103,31 @@ required:
>    - spi-cpol
>    - spi-cpha
>  
> +patternProperties:
> +  "^channel@([0-9a-f])$":
> +    type: object
> +    $ref: adc.yaml
> +    unevaluatedProperties: false
> +
> +    properties:
> +      reg:
> +        description: The channel index.
> +        minimum: 0
> +        maximum: 7

Your pattern a bit above is not correct then: [0-7]

> +
> +      diff-channels:
> +        description: |
> +          The differential channel pair for Ad7194 configurable channels. The
> +          first channel is the positive input, the second channel is the
> +          negative input.
> +        items:
> +          minimum: 1
> +          maximum: 16
> +
> +    required:
> +      - reg
> +      - diff-channels
> +
>  allOf:
>    - $ref: /schemas/spi/spi-peripheral-props.yaml#
>  
> @@ -127,3 +159,40 @@ examples:
>              adi,burnout-currents-enable;
>          };
>      };
> +  - |
> +    spi {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        adc@0 {
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +            compatible = "adi,ad7194";
> +            reg = <0>;
> +            spi-max-frequency = <1000000>;
> +            spi-cpol;
> +            spi-cpha;
> +            clocks = <&ad7192_mclk>;
> +            clock-names = "mclk";
> +            interrupts = <25 0x2>;
> +            interrupt-parent = <&gpio>;
> +            dvdd-supply = <&dvdd>;
> +            avdd-supply = <&avdd>;
> +            vref-supply = <&vref>;
> +
> +            adi,refin2-pins-enable;
> +            adi,rejection-60-Hz-enable;
> +            adi,buffer-enable;
> +            adi,burnout-currents-enable;
> +
> +            channel@0 {

Why cannot you add this to the existing example?



Best regards,
Krzysztof

Re: [PATCH 1/3] iio: adc: ad7192: Use device api

[Krzysztof Kozlowski]

On 05/11/2023 20:31, alisadariana@gmail.com wrote:
> From: Alisa-Dariana Roman <alisa.roman@analog.com>
> 
> Replace of.h and corresponding functions with preferred device specific
> functions.
> 
> Also replace of_device_get_match_data function with
> spi_get_device_match_data.
> 
> Signed-off-by: Alisa-Dariana Roman <alisa.roman@analog.com>
> ---
>  drivers/iio/adc/ad7192.c | 32 +++++++++++++++-----------------
>  1 file changed, 15 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/iio/adc/ad7192.c b/drivers/iio/adc/ad7192.c
> index adc3cbe92d6e..48e0357564af 100644
> --- a/drivers/iio/adc/ad7192.c
> +++ b/drivers/iio/adc/ad7192.c
> @@ -17,7 +17,6 @@
>  #include <linux/err.h>
>  #include <linux/sched.h>
>  #include <linux/delay.h>
> -#include <linux/of.h>
>  
>  #include <linux/iio/iio.h>
>  #include <linux/iio/sysfs.h>
> @@ -364,19 +363,19 @@ static inline bool ad7192_valid_external_frequency(u32 freq)
>  		freq <= AD7192_EXT_FREQ_MHZ_MAX);
>  }
>  
> -static int ad7192_of_clock_select(struct ad7192_state *st)
> +static int ad7192_device_clock_select(struct ad7192_state *st)
>  {
> -	struct device_node *np = st->sd.spi->dev.of_node;
> +	struct device *dev = &st->sd.spi->dev;
>  	unsigned int clock_sel;
>  
>  	clock_sel = AD7192_CLK_INT;
>  
>  	/* use internal clock */
>  	if (!st->mclk) {
> -		if (of_property_read_bool(np, "adi,int-clock-output-enable"))
> +		if (device_property_read_bool(dev, "adi,int-clock-output-enable"))
>  			clock_sel = AD7192_CLK_INT_CO;
>  	} else {
> -		if (of_property_read_bool(np, "adi,clock-xtal"))
> +		if (device_property_read_bool(dev, "adi,clock-xtal"))
>  			clock_sel = AD7192_CLK_EXT_MCLK1_2;
>  		else
>  			clock_sel = AD7192_CLK_EXT_MCLK2;
> @@ -385,9 +384,10 @@ static int ad7192_of_clock_select(struct ad7192_state *st)
>  	return clock_sel;
>  }
>  
> -static int ad7192_setup(struct iio_dev *indio_dev, struct device_node *np)
> +static int ad7192_setup(struct iio_dev *indio_dev)
>  {
>  	struct ad7192_state *st = iio_priv(indio_dev);
> +	struct device *dev = &st->sd.spi->dev;
>  	bool rej60_en, refin2_en;
>  	bool buf_en, bipolar, burnout_curr_en;
>  	unsigned long long scale_uv;
> @@ -416,26 +416,26 @@ static int ad7192_setup(struct iio_dev *indio_dev, struct device_node *np)
>  
>  	st->conf = FIELD_PREP(AD7192_CONF_GAIN_MASK, 0);
>  
> -	rej60_en = of_property_read_bool(np, "adi,rejection-60-Hz-enable");
> +	rej60_en = device_property_read_bool(dev, "adi,rejection-60-Hz-enable");

Not strictly related to your patch, but where are these properties
documented?


Best regards,
Krzysztof

Re: [PATCH 1/3] iio: adc: ad7192: Use device api

[Andy Shevchenko]

On Sun, Nov 05, 2023 at 09:31:29PM +0200, alisadariana@gmail.com wrote:
> From: Alisa-Dariana Roman <alisa.roman@analog.com>
> 
> Replace of.h and corresponding functions with preferred device specific
> functions.
> 
> Also replace of_device_get_match_data function with

of_device_get_match_data()

> spi_get_device_match_data.

spi_get_device_match_data()

With the above fixed,
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

The first patch should be documentation of the properties as Krzysztof noted.

P.S. Also consider using or taking an ideas from the "smart" script [1] I wrote
to send patches, it will put the better list of people into the proper places,
including maintainers and mailing lists.

[1]: https://github.com/andy-shev/home-bin-tools/blob/master/ge2maintainer.sh

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 3/3] iio: adc: ad7192: Add AD7194 support

[Andy Shevchenko]

On Sun, Nov 05, 2023 at 09:31:31PM +0200, alisadariana@gmail.com wrote:
> From: Alisa-Dariana Roman <alisa.roman@analog.com>
> 
> Unlike the other AD719Xs, AD7194 has configurable differential
> channels. The default configuration for these channels can be changed
> from the devicetree.
> 
> The default configuration is hardcoded in order to have a stable number
> of channels.

...

>  config AD7192
> -	tristate "Analog Devices AD7190 AD7192 AD7193 AD7195 ADC driver"
> +	tristate "Analog Devices AD7190 AD7192 AD7193 AD7194 AD7195 ADC driver"

This doesn't scale. Please change this and below like:

	tristate "Analog Devices AD719x ADC driver"

>  	depends on SPI
>  	select AD_SIGMA_DELTA
>  	help
>  	  Say yes here to build support for Analog Devices AD7190,
> -	  AD7192, AD7193 or AD7195 SPI analog to digital converters (ADC).
> +	  AD7192, AD7193, AD7194 or AD7195 SPI analog to digital converters (ADC).

	  Say yes here to build support for Analog Devices SPI analog to
	  digital converters (ADC):
	  - AD7190
	  - AD7192
	  - AD7193
	  - AD7194
	  - AD7195

>  	  If unsure, say N (but it's safe to say "Y").

With above change adding a new one will be just a mater of adding a single
line.

...

> +static int ad7192_parse_channel(struct iio_dev *indio_dev,
> +				struct fwnode_handle *child)
> +{
> +	u32 reg, ain[2];
> +	int ret;
> +
> +	ret = fwnode_property_read_u32(child, "reg", &reg);
> +	if (ret)
> +		return ret;

> +	if (reg < AD7194_CH_DIFF_NR_MIN || reg > AD7194_CH_DIFF_NR_MAX)
> +		return -EINVAL;

in_range()


> +	ret = fwnode_property_read_u32_array(child, "diff-channels", ain,
> +					     ARRAY_SIZE(ain));
> +	if (ret)
> +		return ret;
> +
> +	if (ain[0] < AD7194_CH_AIN_MIN || ain[0] > AD7194_CH_AIN_MAX ||
> +	    ain[1] < AD7194_CH_AIN_MIN || ain[1] > AD7194_CH_AIN_MAX)

Ditto.

> +		return -EINVAL;
> +
> +	ad7194_channels[reg].channel = ain[0];
> +	ad7194_channels[reg].channel2 = ain[1];
> +	ad7194_channels[reg].address = AD7194_CH_DIFF(ain[0], ain[1]);
> +
> +	return 0;
> +}

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 3/3] iio: adc: ad7192: Add AD7194 support

[Jonathan Cameron]

On Mon, 6 Nov 2023 12:27:29 +0200
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> On Sun, Nov 05, 2023 at 09:31:31PM +0200, alisadariana@gmail.com wrote:
> > From: Alisa-Dariana Roman <alisa.roman@analog.com>
> > 
> > Unlike the other AD719Xs, AD7194 has configurable differential
> > channels. The default configuration for these channels can be changed
> > from the devicetree.
> > 
> > The default configuration is hardcoded in order to have a stable number
> > of channels.  
> 
> ...
> 
> >  config AD7192
> > -	tristate "Analog Devices AD7190 AD7192 AD7193 AD7195 ADC driver"
> > +	tristate "Analog Devices AD7190 AD7192 AD7193 AD7194 AD7195 ADC driver"  
> 
> This doesn't scale. Please change this and below like:
> 
> 	tristate "Analog Devices AD719x ADC driver"
Also tends to cause trouble given habit of manufacturers to not have consistent
naming.

"AD7194 and similar ADC driver"
would be my preference.

Re: [PATCH 1/3] iio: adc: ad7192: Use device api

[Alisa-Dariana Roman]

On 06.11.2023 11:24, Krzysztof Kozlowski wrote:
> On 05/11/2023 20:31, alisadariana@gmail.com wrote:
>> From: Alisa-Dariana Roman <alisa.roman@analog.com>
>>
>> Replace of.h and corresponding functions with preferred device specific
>> functions.
>>
>> Also replace of_device_get_match_data function with
>> spi_get_device_match_data.
>>
>> Signed-off-by: Alisa-Dariana Roman <alisa.roman@analog.com>
>> ---
>>   drivers/iio/adc/ad7192.c | 32 +++++++++++++++-----------------
>>   1 file changed, 15 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/iio/adc/ad7192.c b/drivers/iio/adc/ad7192.c
>> index adc3cbe92d6e..48e0357564af 100644
>> --- a/drivers/iio/adc/ad7192.c
>> +++ b/drivers/iio/adc/ad7192.c
>> @@ -17,7 +17,6 @@
>>   #include <linux/err.h>
>>   #include <linux/sched.h>
>>   #include <linux/delay.h>
>> -#include <linux/of.h>
>>   
>>   #include <linux/iio/iio.h>
>>   #include <linux/iio/sysfs.h>
>> @@ -364,19 +363,19 @@ static inline bool ad7192_valid_external_frequency(u32 freq)
>>   		freq <= AD7192_EXT_FREQ_MHZ_MAX);
>>   }
>>   
>> -static int ad7192_of_clock_select(struct ad7192_state *st)
>> +static int ad7192_device_clock_select(struct ad7192_state *st)
>>   {
>> -	struct device_node *np = st->sd.spi->dev.of_node;
>> +	struct device *dev = &st->sd.spi->dev;
>>   	unsigned int clock_sel;
>>   
>>   	clock_sel = AD7192_CLK_INT;
>>   
>>   	/* use internal clock */
>>   	if (!st->mclk) {
>> -		if (of_property_read_bool(np, "adi,int-clock-output-enable"))
>> +		if (device_property_read_bool(dev, "adi,int-clock-output-enable"))
>>   			clock_sel = AD7192_CLK_INT_CO;
>>   	} else {
>> -		if (of_property_read_bool(np, "adi,clock-xtal"))
>> +		if (device_property_read_bool(dev, "adi,clock-xtal"))
>>   			clock_sel = AD7192_CLK_EXT_MCLK1_2;
>>   		else
>>   			clock_sel = AD7192_CLK_EXT_MCLK2;
>> @@ -385,9 +384,10 @@ static int ad7192_of_clock_select(struct ad7192_state *st)
>>   	return clock_sel;
>>   }
>>   
>> -static int ad7192_setup(struct iio_dev *indio_dev, struct device_node *np)
>> +static int ad7192_setup(struct iio_dev *indio_dev)
>>   {
>>   	struct ad7192_state *st = iio_priv(indio_dev);
>> +	struct device *dev = &st->sd.spi->dev;
>>   	bool rej60_en, refin2_en;
>>   	bool buf_en, bipolar, burnout_curr_en;
>>   	unsigned long long scale_uv;
>> @@ -416,26 +416,26 @@ static int ad7192_setup(struct iio_dev *indio_dev, struct device_node *np)
>>   
>>   	st->conf = FIELD_PREP(AD7192_CONF_GAIN_MASK, 0);
>>   
>> -	rej60_en = of_property_read_bool(np, "adi,rejection-60-Hz-enable");
>> +	rej60_en = device_property_read_bool(dev, "adi,rejection-60-Hz-enable");
> 
> Not strictly related to your patch, but where are these properties
> documented?
> 
> 
> Best regards,
> Krzysztof
> 
Thank you for the feedback! The properties are documented in
Documentation/devicetree/bindings/iio/adc/adi,ad7192.yaml. But the
properties related to the clock configuration are indeed missing. I will
add them.

Kind regards,
Alisa-Dariana Roman

Re: [PATCH 2/3] dt-bindings: iio: adc: ad7192: Add AD7194 support

[Alisa-Dariana Roman]

On 06.11.2023 10:56, Krzysztof Kozlowski wrote:
> On 05/11/2023 20:31, alisadariana@gmail.com wrote:
>> From: Alisa-Dariana Roman <alisa.roman@analog.com>
>>
>> Unlike the other AD719Xs, AD7194 has configurable differential
>> channels. The default configuration for these channels can be changed
>> from the devicetree.
>>
>> Also add an example for AD7194 devicetree.
>>
>> Signed-off-by: Alisa-Dariana Roman <alisa.roman@analog.com>
>> ---
>>   .../bindings/iio/adc/adi,ad7192.yaml          | 69 +++++++++++++++++++
>>   1 file changed, 69 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7192.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7192.yaml
>> index 16def2985ab4..b9a9f7b20670 100644
>> --- a/Documentation/devicetree/bindings/iio/adc/adi,ad7192.yaml
>> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7192.yaml
>> @@ -21,8 +21,15 @@ properties:
>>         - adi,ad7190
>>         - adi,ad7192
>>         - adi,ad7193
>> +      - adi,ad7194
>>         - adi,ad7195
>>   
>> +  '#address-cells':
>> +    const: 1
>> +
>> +  '#size-cells':
>> +    const: 0
>> +
>>     reg:
>>       maxItems: 1
>>   
>> @@ -96,6 +103,31 @@ required:
>>     - spi-cpol
>>     - spi-cpha
>>   
>> +patternProperties:
>> +  "^channel@([0-9a-f])$":
>> +    type: object
>> +    $ref: adc.yaml
>> +    unevaluatedProperties: false
>> +
>> +    properties:
>> +      reg:
>> +        description: The channel index.
>> +        minimum: 0
>> +        maximum: 7
> 
> Your pattern a bit above is not correct then: [0-7]
> 
>> +
>> +      diff-channels:
>> +        description: |
>> +          The differential channel pair for Ad7194 configurable channels. The
>> +          first channel is the positive input, the second channel is the
>> +          negative input.
>> +        items:
>> +          minimum: 1
>> +          maximum: 16
>> +
>> +    required:
>> +      - reg
>> +      - diff-channels
>> +
>>   allOf:
>>     - $ref: /schemas/spi/spi-peripheral-props.yaml#
>>   
>> @@ -127,3 +159,40 @@ examples:
>>               adi,burnout-currents-enable;
>>           };
>>       };
>> +  - |
>> +    spi {
>> +        #address-cells = <1>;
>> +        #size-cells = <0>;
>> +
>> +        adc@0 {
>> +            #address-cells = <1>;
>> +            #size-cells = <0>;
>> +            compatible = "adi,ad7194";
>> +            reg = <0>;
>> +            spi-max-frequency = <1000000>;
>> +            spi-cpol;
>> +            spi-cpha;
>> +            clocks = <&ad7192_mclk>;
>> +            clock-names = "mclk";
>> +            interrupts = <25 0x2>;
>> +            interrupt-parent = <&gpio>;
>> +            dvdd-supply = <&dvdd>;
>> +            avdd-supply = <&avdd>;
>> +            vref-supply = <&vref>;
>> +
>> +            adi,refin2-pins-enable;
>> +            adi,rejection-60-Hz-enable;
>> +            adi,buffer-enable;
>> +            adi,burnout-currents-enable;
>> +
>> +            channel@0 {
> 
> Why cannot you add this to the existing example?
> 
> 
> 
> Best regards,
> Krzysztof
> 
I added another example to highlight the fact that only AD7194 supports 
configurable channels. How should I proceed?

Kind regards,
Alisa-Dariana Roman

Re: [PATCH 2/3] dt-bindings: iio: adc: ad7192: Add AD7194 support

[Krzysztof Kozlowski]

On 14/11/2023 17:02, Alisa-Dariana Roman wrote:
> On 06.11.2023 10:56, Krzysztof Kozlowski wrote:
>> On 05/11/2023 20:31, alisadariana@gmail.com wrote:
>>> From: Alisa-Dariana Roman <alisa.roman@analog.com>
>>>
>>> Unlike the other AD719Xs, AD7194 has configurable differential
>>> channels. The default configuration for these channels can be changed
>>> from the devicetree.
>>>
>>> Also add an example for AD7194 devicetree.
>>>
>>> Signed-off-by: Alisa-Dariana Roman <alisa.roman@analog.com>
>>> ---
>>>   .../bindings/iio/adc/adi,ad7192.yaml          | 69 +++++++++++++++++++
>>>   1 file changed, 69 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7192.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7192.yaml
>>> index 16def2985ab4..b9a9f7b20670 100644
>>> --- a/Documentation/devicetree/bindings/iio/adc/adi,ad7192.yaml
>>> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7192.yaml
>>> @@ -21,8 +21,15 @@ properties:
>>>         - adi,ad7190
>>>         - adi,ad7192
>>>         - adi,ad7193
>>> +      - adi,ad7194
>>>         - adi,ad7195
>>>   
>>> +  '#address-cells':
>>> +    const: 1
>>> +
>>> +  '#size-cells':
>>> +    const: 0
>>> +
>>>     reg:
>>>       maxItems: 1
>>>   
>>> @@ -96,6 +103,31 @@ required:
>>>     - spi-cpol
>>>     - spi-cpha
>>>   
>>> +patternProperties:
>>> +  "^channel@([0-9a-f])$":
>>> +    type: object
>>> +    $ref: adc.yaml
>>> +    unevaluatedProperties: false
>>> +
>>> +    properties:
>>> +      reg:
>>> +        description: The channel index.
>>> +        minimum: 0
>>> +        maximum: 7
>>
>> Your pattern a bit above is not correct then: [0-7]
>>
>>> +
>>> +      diff-channels:
>>> +        description: |
>>> +          The differential channel pair for Ad7194 configurable channels. The
>>> +          first channel is the positive input, the second channel is the
>>> +          negative input.
>>> +        items:
>>> +          minimum: 1
>>> +          maximum: 16
>>> +
>>> +    required:
>>> +      - reg
>>> +      - diff-channels
>>> +
>>>   allOf:
>>>     - $ref: /schemas/spi/spi-peripheral-props.yaml#
>>>   
>>> @@ -127,3 +159,40 @@ examples:
>>>               adi,burnout-currents-enable;
>>>           };
>>>       };
>>> +  - |
>>> +    spi {
>>> +        #address-cells = <1>;
>>> +        #size-cells = <0>;
>>> +
>>> +        adc@0 {
>>> +            #address-cells = <1>;
>>> +            #size-cells = <0>;
>>> +            compatible = "adi,ad7194";
>>> +            reg = <0>;
>>> +            spi-max-frequency = <1000000>;
>>> +            spi-cpol;
>>> +            spi-cpha;
>>> +            clocks = <&ad7192_mclk>;
>>> +            clock-names = "mclk";
>>> +            interrupts = <25 0x2>;
>>> +            interrupt-parent = <&gpio>;
>>> +            dvdd-supply = <&dvdd>;
>>> +            avdd-supply = <&avdd>;
>>> +            vref-supply = <&vref>;
>>> +
>>> +            adi,refin2-pins-enable;
>>> +            adi,rejection-60-Hz-enable;
>>> +            adi,buffer-enable;
>>> +            adi,burnout-currents-enable;
>>> +
>>> +            channel@0 {
>>
>> Why cannot you add this to the existing example?
>>
>>
>>
>> Best regards,
>> Krzysztof
>>
> I added another example to highlight the fact that only AD7194 supports 
> configurable channels. How should I proceed?

Bindings did not tell that, so it seems you miss that part - allOf
constraining channels per variant.

Best regards,
Krzysztof

Re: [PATCH 2/3] dt-bindings: iio: adc: ad7192: Add AD7194 support

[Alisa-Dariana Roman]

On 14.11.2023 19:39, Krzysztof Kozlowski wrote:
> On 14/11/2023 17:02, Alisa-Dariana Roman wrote:
>> On 06.11.2023 10:56, Krzysztof Kozlowski wrote:
>>> On 05/11/2023 20:31, alisadariana@gmail.com wrote:
>>>> From: Alisa-Dariana Roman <alisa.roman@analog.com>
>>>>
>>>> Unlike the other AD719Xs, AD7194 has configurable differential
>>>> channels. The default configuration for these channels can be changed
>>>> from the devicetree.
>>>>
>>>> Also add an example for AD7194 devicetree.
>>>>
>>>> Signed-off-by: Alisa-Dariana Roman <alisa.roman@analog.com>
>>>> ---
>>>>    .../bindings/iio/adc/adi,ad7192.yaml          | 69 +++++++++++++++++++
>>>>    1 file changed, 69 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7192.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7192.yaml
>>>> index 16def2985ab4..b9a9f7b20670 100644
>>>> --- a/Documentation/devicetree/bindings/iio/adc/adi,ad7192.yaml
>>>> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7192.yaml
>>>> @@ -21,8 +21,15 @@ properties:
>>>>          - adi,ad7190
>>>>          - adi,ad7192
>>>>          - adi,ad7193
>>>> +      - adi,ad7194
>>>>          - adi,ad7195
>>>>    
>>>> +  '#address-cells':
>>>> +    const: 1
>>>> +
>>>> +  '#size-cells':
>>>> +    const: 0
>>>> +
>>>>      reg:
>>>>        maxItems: 1
>>>>    
>>>> @@ -96,6 +103,31 @@ required:
>>>>      - spi-cpol
>>>>      - spi-cpha
>>>>    
>>>> +patternProperties:
>>>> +  "^channel@([0-9a-f])$":
>>>> +    type: object
>>>> +    $ref: adc.yaml
>>>> +    unevaluatedProperties: false
>>>> +
>>>> +    properties:
>>>> +      reg:
>>>> +        description: The channel index.
>>>> +        minimum: 0
>>>> +        maximum: 7
>>>
>>> Your pattern a bit above is not correct then: [0-7]
>>>
>>>> +
>>>> +      diff-channels:
>>>> +        description: |
>>>> +          The differential channel pair for Ad7194 configurable channels. The
>>>> +          first channel is the positive input, the second channel is the
>>>> +          negative input.
>>>> +        items:
>>>> +          minimum: 1
>>>> +          maximum: 16
>>>> +
>>>> +    required:
>>>> +      - reg
>>>> +      - diff-channels
>>>> +
>>>>    allOf:
>>>>      - $ref: /schemas/spi/spi-peripheral-props.yaml#
>>>>    
>>>> @@ -127,3 +159,40 @@ examples:
>>>>                adi,burnout-currents-enable;
>>>>            };
>>>>        };
>>>> +  - |
>>>> +    spi {
>>>> +        #address-cells = <1>;
>>>> +        #size-cells = <0>;
>>>> +
>>>> +        adc@0 {
>>>> +            #address-cells = <1>;
>>>> +            #size-cells = <0>;
>>>> +            compatible = "adi,ad7194";
>>>> +            reg = <0>;
>>>> +            spi-max-frequency = <1000000>;
>>>> +            spi-cpol;
>>>> +            spi-cpha;
>>>> +            clocks = <&ad7192_mclk>;
>>>> +            clock-names = "mclk";
>>>> +            interrupts = <25 0x2>;
>>>> +            interrupt-parent = <&gpio>;
>>>> +            dvdd-supply = <&dvdd>;
>>>> +            avdd-supply = <&avdd>;
>>>> +            vref-supply = <&vref>;
>>>> +
>>>> +            adi,refin2-pins-enable;
>>>> +            adi,rejection-60-Hz-enable;
>>>> +            adi,buffer-enable;
>>>> +            adi,burnout-currents-enable;
>>>> +
>>>> +            channel@0 {
>>>
>>> Why cannot you add this to the existing example?
>>>
>>>
>>>
>>> Best regards,
>>> Krzysztof
>>>
>> I added another example to highlight the fact that only AD7194 supports
>> configurable channels. How should I proceed?
> 
> Bindings did not tell that, so it seems you miss that part - allOf
> constraining channels per variant.
> 
> Best regards,
> Krzysztof
> 
And should I remove the AD7194 example?

Kind regards,
Alisa-Dariana Roman

Re: [PATCH 2/3] dt-bindings: iio: adc: ad7192: Add AD7194 support

[Krzysztof Kozlowski]

On 14/11/2023 19:27, Alisa-Dariana Roman wrote:

>>>>
>>>> Best regards,
>>>> Krzysztof
>>>>
>>> I added another example to highlight the fact that only AD7194 supports
>>> configurable channels. How should I proceed?
>>
>> Bindings did not tell that, so it seems you miss that part - allOf
>> constraining channels per variant.
>>
>> Best regards,
>> Krzysztof
>>
> And should I remove the AD7194 example?

You can add new example in such case (and keep old).

Best regards,
Krzysztof