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

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

[Alisa-Dariana Roman]

From: romandariana <alisa.roman@analog.com>

Dear maintainers,

Thank you all for the feedback!

I am submitting the upgraded series of patches for the ad7192 driver.

Please consider applying in order.

Thank you!

v3 -> v4
        - drop device properties patch, changes already applied to tree
        - change bindings and driver such that for AD7194 there are 16
        differential channels, by default set to AINx - AINCOM, which can be
        configured in devicetree however the user likes
        - corrected mistake regarding positive and negative channel macros:
        subtract 1 from the number corresponding to AIN input

v2 -> v3
	- add precursor patch to simply functions to only pass
	  ad7192_state
	- add patch to replace custom attribute
	- bindings patch: correct use of allOf and some minor changes to
	  the ad7194 example
	- add ad7194 patch:
		- use "ad7192 and similar"
		- ad7194 no longer needs attribute group
		- use callback function in chip_info to parse channels
		- move struct ad7192_chip_info
		- change position of parse functions
	- drop clock bindings patch

v1 -> v2
	- new commit with missing documentation for properties
	- add constraint for channels in binding
	- correct pattern for channels
	- correct commit message by adding "()" to functions
	- use in_range
	- use preferred structure in Kconfig

Kind regards,

Alisa-Dariana Roman (4):
  iio: adc: ad7192: Pass state directly
  iio: adc: ad7192: Use standard attribute
  dt-bindings: iio: adc: ad7192: Add AD7194 support
  iio: adc: ad7192: Add AD7194 support

 .../bindings/iio/adc/adi,ad7192.yaml          |  75 ++++++
 drivers/iio/adc/Kconfig                       |  11 +-
 drivers/iio/adc/ad7192.c                      | 218 ++++++++++++++----
 3 files changed, 250 insertions(+), 54 deletions(-)

-- 
2.34.1

[PATCH v4 1/4] iio: adc: ad7192: Pass state directly

[Alisa-Dariana Roman]

Pass only the ad7192_state structure. There is no need to pass the
iio_dev structure.

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

diff --git a/drivers/iio/adc/ad7192.c b/drivers/iio/adc/ad7192.c
index 7bcc7e2aa2a2..72de9cc6716e 100644
--- a/drivers/iio/adc/ad7192.c
+++ b/drivers/iio/adc/ad7192.c
@@ -387,9 +387,9 @@ static int ad7192_clock_select(struct ad7192_state *st)
 	return clock_sel;
 }
 
-static int ad7192_setup(struct iio_dev *indio_dev, struct device *dev)
+static int ad7192_setup(struct ad7192_state *st)
 {
-	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;
@@ -460,7 +460,7 @@ static int ad7192_setup(struct iio_dev *indio_dev, struct device *dev)
 	/* Populate available ADC input ranges */
 	for (i = 0; i < ARRAY_SIZE(st->scale_avail); i++) {
 		scale_uv = ((u64)st->int_vref_mv * 100000000)
-			>> (indio_dev->channels[0].scan_type.realbits -
+			>> (st->chip_info->channels[0].scan_type.realbits -
 			!FIELD_GET(AD7192_CONF_UNIPOLAR, st->conf));
 		scale_uv >>= i;
 
@@ -1152,7 +1152,7 @@ static int ad7192_probe(struct spi_device *spi)
 		}
 	}
 
-	ret = ad7192_setup(indio_dev, &spi->dev);
+	ret = ad7192_setup(st);
 	if (ret)
 		return ret;
 
-- 
2.34.1

[PATCH v4 2/4] iio: adc: ad7192: Use standard attribute

[Alisa-Dariana Roman]

Replace custom attribute filter_low_pass_3db_frequency_available with
standard attribute.

Store the available values in ad7192_state struct.

The function that used to compute those values replaced by
ad7192_update_filter_freq_avail().

Function ad7192_show_filter_avail() is no longer needed.

Note that the initial available values are hardcoded.

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

diff --git a/drivers/iio/adc/ad7192.c b/drivers/iio/adc/ad7192.c
index 72de9cc6716e..e0f1c9eaf9ae 100644
--- a/drivers/iio/adc/ad7192.c
+++ b/drivers/iio/adc/ad7192.c
@@ -190,6 +190,7 @@ struct ad7192_state {
 	u32				mode;
 	u32				conf;
 	u32				scale_avail[8][2];
+	u32				filter_freq_avail[4][2];
 	u32				oversampling_ratio_avail[4];
 	u8				gpocon;
 	u8				clock_sel;
@@ -473,6 +474,16 @@ static int ad7192_setup(struct ad7192_state *st)
 	st->oversampling_ratio_avail[2] = 8;
 	st->oversampling_ratio_avail[3] = 16;
 
+	st->filter_freq_avail[0][0] = 600;
+	st->filter_freq_avail[1][0] = 800;
+	st->filter_freq_avail[2][0] = 2300;
+	st->filter_freq_avail[3][0] = 2720;
+
+	st->filter_freq_avail[0][1] = 1000;
+	st->filter_freq_avail[1][1] = 1000;
+	st->filter_freq_avail[2][1] = 1000;
+	st->filter_freq_avail[3][1] = 1000;
+
 	return 0;
 }
 
@@ -586,48 +597,24 @@ static int ad7192_get_f_adc(struct ad7192_state *st)
 				 f_order * FIELD_GET(AD7192_MODE_RATE_MASK, st->mode));
 }
 
-static void ad7192_get_available_filter_freq(struct ad7192_state *st,
-						    int *freq)
+static void ad7192_update_filter_freq_avail(struct ad7192_state *st)
 {
 	unsigned int fadc;
 
 	/* Formulas for filter at page 25 of the datasheet */
 	fadc = ad7192_compute_f_adc(st, false, true);
-	freq[0] = DIV_ROUND_CLOSEST(fadc * 240, 1024);
+	st->filter_freq_avail[0][0] = DIV_ROUND_CLOSEST(fadc * 240, 1024);
 
 	fadc = ad7192_compute_f_adc(st, true, true);
-	freq[1] = DIV_ROUND_CLOSEST(fadc * 240, 1024);
+	st->filter_freq_avail[1][0] = DIV_ROUND_CLOSEST(fadc * 240, 1024);
 
 	fadc = ad7192_compute_f_adc(st, false, false);
-	freq[2] = DIV_ROUND_CLOSEST(fadc * 230, 1024);
+	st->filter_freq_avail[2][0] = DIV_ROUND_CLOSEST(fadc * 230, 1024);
 
 	fadc = ad7192_compute_f_adc(st, true, false);
-	freq[3] = DIV_ROUND_CLOSEST(fadc * 272, 1024);
+	st->filter_freq_avail[3][0] = DIV_ROUND_CLOSEST(fadc * 272, 1024);
 }
 
-static ssize_t ad7192_show_filter_avail(struct device *dev,
-					struct device_attribute *attr,
-					char *buf)
-{
-	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
-	struct ad7192_state *st = iio_priv(indio_dev);
-	unsigned int freq_avail[4], i;
-	size_t len = 0;
-
-	ad7192_get_available_filter_freq(st, freq_avail);
-
-	for (i = 0; i < ARRAY_SIZE(freq_avail); i++)
-		len += sysfs_emit_at(buf, len, "%d.%03d ", freq_avail[i] / 1000,
-				     freq_avail[i] % 1000);
-
-	buf[len - 1] = '\n';
-
-	return len;
-}
-
-static IIO_DEVICE_ATTR(filter_low_pass_3db_frequency_available,
-		       0444, ad7192_show_filter_avail, NULL, 0);
-
 static IIO_DEVICE_ATTR(bridge_switch_en, 0644,
 		       ad7192_show_bridge_switch, ad7192_set,
 		       AD7192_REG_GPOCON);
@@ -637,7 +624,6 @@ static IIO_DEVICE_ATTR(ac_excitation_en, 0644,
 		       AD7192_REG_CONF);
 
 static struct attribute *ad7192_attributes[] = {
-	&iio_dev_attr_filter_low_pass_3db_frequency_available.dev_attr.attr,
 	&iio_dev_attr_bridge_switch_en.dev_attr.attr,
 	NULL
 };
@@ -647,7 +633,6 @@ static const struct attribute_group ad7192_attribute_group = {
 };
 
 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,
 	&iio_dev_attr_ac_excitation_en.dev_attr.attr,
 	NULL
@@ -665,17 +650,15 @@ static unsigned int ad7192_get_temp_scale(bool unipolar)
 static int ad7192_set_3db_filter_freq(struct ad7192_state *st,
 				      int val, int val2)
 {
-	int freq_avail[4], i, ret, freq;
+	int i, ret, freq;
 	unsigned int diff_new, diff_old;
 	int idx = 0;
 
 	diff_old = U32_MAX;
 	freq = val * 1000 + val2;
 
-	ad7192_get_available_filter_freq(st, freq_avail);
-
-	for (i = 0; i < ARRAY_SIZE(freq_avail); i++) {
-		diff_new = abs(freq - freq_avail[i]);
+	for (i = 0; i < ARRAY_SIZE(st->filter_freq_avail); i++) {
+		diff_new = abs(freq - st->filter_freq_avail[i][0]);
 		if (diff_new < diff_old) {
 			diff_old = diff_new;
 			idx = i;
@@ -826,6 +809,7 @@ static int ad7192_write_raw(struct iio_dev *indio_dev,
 		st->mode &= ~AD7192_MODE_RATE_MASK;
 		st->mode |= FIELD_PREP(AD7192_MODE_RATE_MASK, div);
 		ad_sd_write_reg(&st->sd, AD7192_REG_MODE, 3, st->mode);
+		ad7192_update_filter_freq_avail(st);
 		break;
 	case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
 		ret = ad7192_set_3db_filter_freq(st, val, val2 / 1000);
@@ -846,6 +830,7 @@ static int ad7192_write_raw(struct iio_dev *indio_dev,
 				break;
 			}
 		mutex_unlock(&st->lock);
+		ad7192_update_filter_freq_avail(st);
 		break;
 	default:
 		ret = -EINVAL;
@@ -888,6 +873,12 @@ static int ad7192_read_avail(struct iio_dev *indio_dev,
 		/* Values are stored in a 2D matrix  */
 		*length = ARRAY_SIZE(st->scale_avail) * 2;
 
+		return IIO_AVAIL_LIST;
+	case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
+		*vals = (int *)st->filter_freq_avail;
+		*type = IIO_VAL_FRACTIONAL;
+		*length = ARRAY_SIZE(st->filter_freq_avail) * 2;
+
 		return IIO_AVAIL_LIST;
 	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
 		*vals = (int *)st->oversampling_ratio_avail;
@@ -956,7 +947,9 @@ static const struct iio_info ad7195_info = {
 			BIT(IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY) | \
 			(_mask_all), \
 		.info_mask_shared_by_type_available = (_mask_type_av), \
-		.info_mask_shared_by_all_available = (_mask_all_av), \
+		.info_mask_shared_by_all_available = \
+			BIT(IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY) | \
+			(_mask_all_av), \
 		.ext_info = (_ext_info), \
 		.scan_index = (_si), \
 		.scan_type = { \
-- 
2.34.1

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

[Alisa-Dariana Roman]

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>
Signed-off-by: romandariana <alisa.roman@analog.com>
---
 .../bindings/iio/adc/adi,ad7192.yaml          | 75 +++++++++++++++++++
 1 file changed, 75 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7192.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7192.yaml
index 16def2985ab4..c62862760311 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,8 +103,44 @@ required:
   - spi-cpol
   - spi-cpha
 
+patternProperties:
+  "^channel@([0-9]+)$":
+    type: object
+    $ref: adc.yaml
+    unevaluatedProperties: false
+
+    properties:
+      reg:
+        description: The channel index.
+        minimum: 1
+        maximum: 16
+
+      diff-channels:
+        description: |
+          Both inputs can be connected to pins AIN1 to AIN16 by choosing the
+          appropriate value from 1 to 16. The negative input can also be
+          connected to AINCOM by choosing 0.
+        items:
+          minimum: 0
+          maximum: 16
+
+    required:
+      - reg
+      - diff-channels
+
 allOf:
   - $ref: /schemas/spi/spi-peripheral-props.yaml#
+  - if:
+      properties:
+        compatible:
+          enum:
+            - adi,ad7190
+            - adi,ad7192
+            - adi,ad7193
+            - adi,ad7195
+    then:
+      patternProperties:
+        "^channel@([0-9]+)$": false
 
 unevaluatedProperties: false
 
@@ -127,3 +170,35 @@ 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>;
+
+            channel@1 {
+                reg = <1>;
+                diff-channels = <1 6>;
+            };
+
+            channel@16 {
+                reg = <16>;
+                diff-channels = <16 5>;
+            };
+        };
+    };
-- 
2.34.1

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

[Krzysztof Kozlowski]

On 28/02/2024 13:26, Alisa-Dariana Roman wrote:
> 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>
> Signed-off-by: romandariana <alisa.roman@analog.com>

Something is not right here...

> ---
>  .../bindings/iio/adc/adi,ad7192.yaml          | 75 +++++++++++++++++++
>  1 file changed, 75 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7192.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7192.yaml
> index 16def2985ab4..c62862760311 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,8 +103,44 @@ required:
>    - spi-cpol
>    - spi-cpha
>  
> +patternProperties:
> +  "^channel@([0-9]+)$":

No need for ()

> +    type: object
> +    $ref: adc.yaml
> +    unevaluatedProperties: false
> +
> +    properties:
> +      reg:
> +        description: The channel index.
> +        minimum: 1
> +        maximum: 16
> +
> +      diff-channels:
> +        description: |
> +          Both inputs can be connected to pins AIN1 to AIN16 by choosing the
> +          appropriate value from 1 to 16. The negative input can also be
> +          connected to AINCOM by choosing 0.
> +        items:
> +          minimum: 0
> +          maximum: 16
> +
> +    required:
> +      - reg
> +      - diff-channels
> +
>  allOf:
>    - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +  - if:
> +      properties:
> +        compatible:
> +          enum:
> +            - adi,ad7190
> +            - adi,ad7192
> +            - adi,ad7193
> +            - adi,ad7195
> +    then:
> +      patternProperties:
> +        "^channel@([0-9]+)$": false

No need for ()

>  
>  unevaluatedProperties: false
>  
> @@ -127,3 +170,35 @@ examples:


Best regards,
Krzysztof

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

[David Lechner]

On Wed, Feb 28, 2024 at 6:26 AM Alisa-Dariana Roman
<alisadariana@gmail.com> wrote:
>
> 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>
> Signed-off-by: romandariana <alisa.roman@analog.com>

Why two SOBs with the same email?

> ---
>  .../bindings/iio/adc/adi,ad7192.yaml          | 75 +++++++++++++++++++
>  1 file changed, 75 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7192.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7192.yaml
> index 16def2985ab4..c62862760311 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
>

Based on the discussion in v3, I was expecting to see an aincom-supply
property added. (Although that probably belongs in a separate patch
since it applies to all existing chips, not just the one being added
here.)


> @@ -96,8 +103,44 @@ required:
>    - spi-cpol
>    - spi-cpha
>
> +patternProperties:
> +  "^channel@([0-9]+)$":
> +    type: object
> +    $ref: adc.yaml
> +    unevaluatedProperties: false
> +
> +    properties:
> +      reg:
> +        description: The channel index.
> +        minimum: 1
> +        maximum: 16

I guess this is OK, but max of 16 is a bit artificial since there
could be unusual use cases where inputs are reused on multiple
channels. Technically, there are 16 * 17 = 272 possible combinations
that could appear.

[PATCH v4 4/4] iio: adc: ad7192: Add AD7194 support

[Alisa-Dariana Roman]

Unlike the other AD719Xs, AD7194 has configurable differential
channels. The configuration for the 16 differential channels can be
changed in the devicetree.

The default configuration includes 16 differential channels
corresponding to the voltage difference between each AINx input and
AINCOM input.

Also modify config AD7192 description for better scaling.

Moved ad7192_chip_info struct definition to allow use of callback
function parse_channels().

Signed-off-by: Alisa-Dariana Roman <alisa.roman@analog.com>
Reviewed-by: Nuno Sa <nuno.sa@analog.com>
Signed-off-by: romandariana <alisa.roman@analog.com>
---
 drivers/iio/adc/Kconfig  |  11 ++-
 drivers/iio/adc/ad7192.c | 143 ++++++++++++++++++++++++++++++++++++---
 2 files changed, 141 insertions(+), 13 deletions(-)

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 0d9282fa67f5..6d8202d9f501 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -71,12 +71,17 @@ config AD7124
 	  called ad7124.
 
 config AD7192
-	tristate "Analog Devices AD7190 AD7192 AD7193 AD7195 ADC driver"
+	tristate "Analog Devices AD7192 and similar 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).
+	  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").
 
 	  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 e0f1c9eaf9ae..f8ba724bc143 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
+ * AD7192 and similar SPI ADC driver
  *
  * Copyright 2011-2015 Analog Devices Inc.
  */
@@ -128,10 +128,39 @@
 #define AD7193_CH_AIN8		0x480 /* AIN7 - AINCOM */
 #define AD7193_CH_AINCOM	0x600 /* AINCOM - AINCOM */
 
+#define AD7194_CH_POS(x)	(((x) - 1) << 4)
+#define AD7194_CH_NEG(x)	((x) - 1)
+#define AD7194_CH_DIFF(pos, neg) \
+		(((neg) == 0 ? BIT(10) : AD7194_CH_NEG(neg)) | AD7194_CH_POS(pos))
+#define AD7194_CH_AIN1		AD7194_CH_DIFF(1, 0) /* AIN1 - AINCOM */
+#define AD7194_CH_AIN2		AD7194_CH_DIFF(2, 0) /* AIN2 - AINCOM */
+#define AD7194_CH_AIN3		AD7194_CH_DIFF(3, 0) /* AIN3 - AINCOM */
+#define AD7194_CH_AIN4		AD7194_CH_DIFF(4, 0) /* AIN4 - AINCOM */
+#define AD7194_CH_AIN5		AD7194_CH_DIFF(5, 0) /* AIN5 - AINCOM */
+#define AD7194_CH_AIN6		AD7194_CH_DIFF(6, 0) /* AIN6 - AINCOM */
+#define AD7194_CH_AIN7		AD7194_CH_DIFF(7, 0) /* AIN7 - AINCOM */
+#define AD7194_CH_AIN8		AD7194_CH_DIFF(8, 0) /* AIN8 - AINCOM */
+#define AD7194_CH_AIN9		AD7194_CH_DIFF(9, 0) /* AIN9 - AINCOM */
+#define AD7194_CH_AIN10		AD7194_CH_DIFF(10, 0) /* AIN10 - AINCOM */
+#define AD7194_CH_AIN11		AD7194_CH_DIFF(11, 0) /* AIN11 - AINCOM */
+#define AD7194_CH_AIN12		AD7194_CH_DIFF(12, 0) /* AIN12 - AINCOM */
+#define AD7194_CH_AIN13		AD7194_CH_DIFF(13, 0) /* AIN13 - AINCOM */
+#define AD7194_CH_AIN14		AD7194_CH_DIFF(14, 0) /* AIN14 - AINCOM */
+#define AD7194_CH_AIN15		AD7194_CH_DIFF(15, 0) /* AIN15 - AINCOM */
+#define AD7194_CH_AIN16		AD7194_CH_DIFF(16, 0) /* AIN16 - AINCOM */
+#define AD7194_CH_TEMP		0x100 /* Temp sensor */
+#define AD7194_CH_DIFF_START	1
+#define AD7194_CH_DIFF_NR	16
+#define AD7194_CH_AIN0_START	1
+#define AD7194_CH_AIN0_NR	16
+#define AD7194_CH_AIN1_START	0
+#define AD7194_CH_AIN1_NR	17
+
 /* 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)
 
@@ -169,17 +198,10 @@ enum {
 	ID_AD7190,
 	ID_AD7192,
 	ID_AD7193,
+	ID_AD7194,
 	ID_AD7195,
 };
 
-struct ad7192_chip_info {
-	unsigned int			chip_id;
-	const char			*name;
-	const struct iio_chan_spec	*channels;
-	u8				num_channels;
-	const struct iio_info		*info;
-};
-
 struct ad7192_state {
 	const struct ad7192_chip_info	*chip_info;
 	struct regulator		*avdd;
@@ -200,6 +222,15 @@ struct ad7192_state {
 	struct ad_sigma_delta		sd;
 };
 
+struct ad7192_chip_info {
+	unsigned int			chip_id;
+	const char			*name;
+	const struct iio_chan_spec	*channels;
+	u8				num_channels;
+	const struct iio_info		*info;
+	int (*parse_channels)(struct ad7192_state *st);
+};
+
 static const char * const ad7192_syscalib_modes[] = {
 	[AD7192_SYSCALIB_ZERO_SCALE] = "zero_scale",
 	[AD7192_SYSCALIB_FULL_SCALE] = "full_scale",
@@ -921,6 +952,15 @@ 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,
+	.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,
@@ -1012,6 +1052,73 @@ 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, 0, AD7194_CH_AIN1),
+	AD7193_DIFF_CHANNEL(1, 2, 0, AD7194_CH_AIN2),
+	AD7193_DIFF_CHANNEL(2, 3, 0, AD7194_CH_AIN3),
+	AD7193_DIFF_CHANNEL(3, 4, 0, AD7194_CH_AIN4),
+	AD7193_DIFF_CHANNEL(4, 5, 0, AD7194_CH_AIN5),
+	AD7193_DIFF_CHANNEL(5, 6, 0, AD7194_CH_AIN6),
+	AD7193_DIFF_CHANNEL(6, 7, 0, AD7194_CH_AIN7),
+	AD7193_DIFF_CHANNEL(7, 8, 0, AD7194_CH_AIN8),
+	AD7193_DIFF_CHANNEL(8, 9, 0, AD7194_CH_AIN9),
+	AD7193_DIFF_CHANNEL(9, 10, 0, AD7194_CH_AIN10),
+	AD7193_DIFF_CHANNEL(10, 11, 0, AD7194_CH_AIN11),
+	AD7193_DIFF_CHANNEL(11, 12, 0, AD7194_CH_AIN12),
+	AD7193_DIFF_CHANNEL(12, 13, 0, AD7194_CH_AIN13),
+	AD7193_DIFF_CHANNEL(13, 14, 0, AD7194_CH_AIN14),
+	AD7193_DIFF_CHANNEL(14, 15, 0, AD7194_CH_AIN15),
+	AD7193_DIFF_CHANNEL(15, 16, 0, AD7194_CH_AIN16),
+	AD719x_TEMP_CHANNEL(16, AD7194_CH_TEMP),
+	IIO_CHAN_SOFT_TIMESTAMP(17),
+};
+
+static int ad7192_parse_channel(struct fwnode_handle *child)
+{
+	u32 reg, ain[2];
+	int ret;
+
+	ret = fwnode_property_read_u32(child, "reg", &reg);
+	if (ret)
+		return ret;
+
+	if (!in_range(reg, AD7194_CH_DIFF_START, AD7194_CH_DIFF_NR))
+		return -EINVAL;
+
+	ret = fwnode_property_read_u32_array(child, "diff-channels", ain,
+					     ARRAY_SIZE(ain));
+	if (ret)
+		return ret;
+
+	if (!in_range(ain[0], AD7194_CH_AIN0_START, AD7194_CH_AIN0_NR) ||
+	    !in_range(ain[1], AD7194_CH_AIN1_START, AD7194_CH_AIN1_NR))
+		return -EINVAL;
+
+	reg--;
+	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 ad7192_state *st)
+{
+	struct device *dev = &st->sd.spi->dev;
+	struct fwnode_handle *child;
+	int ret;
+
+	device_for_each_child_node(dev, child) {
+		ret = ad7192_parse_channel(child);
+		if (ret) {
+			fwnode_handle_put(child);
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
 static const struct ad7192_chip_info ad7192_chip_info_tbl[] = {
 	[ID_AD7190] = {
 		.chip_id = CHIPID_AD7190,
@@ -1034,6 +1141,14 @@ 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,
+		.parse_channels = ad7192_parse_channels,
+	},
 	[ID_AD7195] = {
 		.chip_id = CHIPID_AD7195,
 		.name = "ad7195",
@@ -1145,6 +1260,12 @@ static int ad7192_probe(struct spi_device *spi)
 		}
 	}
 
+	if (st->chip_info->parse_channels) {
+		ret = st->chip_info->parse_channels(st);
+		if (ret)
+			return ret;
+	}
+
 	ret = ad7192_setup(st);
 	if (ret)
 		return ret;
@@ -1156,6 +1277,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] },
 	{}
 };
@@ -1165,6 +1287,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] },
 	{}
 };
@@ -1181,6 +1304,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 AD7192 and similar ADC");
 MODULE_LICENSE("GPL v2");
 MODULE_IMPORT_NS(IIO_AD_SIGMA_DELTA);
-- 
2.34.1

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

[David Lechner]

On Wed, Feb 28, 2024 at 6:26 AM Alisa-Dariana Roman
<alisadariana@gmail.com> wrote:
>

...

> @@ -1012,6 +1052,73 @@ 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, 0, AD7194_CH_AIN1),
> +       AD7193_DIFF_CHANNEL(1, 2, 0, AD7194_CH_AIN2),
> +       AD7193_DIFF_CHANNEL(2, 3, 0, AD7194_CH_AIN3),
> +       AD7193_DIFF_CHANNEL(3, 4, 0, AD7194_CH_AIN4),
> +       AD7193_DIFF_CHANNEL(4, 5, 0, AD7194_CH_AIN5),
> +       AD7193_DIFF_CHANNEL(5, 6, 0, AD7194_CH_AIN6),
> +       AD7193_DIFF_CHANNEL(6, 7, 0, AD7194_CH_AIN7),
> +       AD7193_DIFF_CHANNEL(7, 8, 0, AD7194_CH_AIN8),
> +       AD7193_DIFF_CHANNEL(8, 9, 0, AD7194_CH_AIN9),
> +       AD7193_DIFF_CHANNEL(9, 10, 0, AD7194_CH_AIN10),
> +       AD7193_DIFF_CHANNEL(10, 11, 0, AD7194_CH_AIN11),
> +       AD7193_DIFF_CHANNEL(11, 12, 0, AD7194_CH_AIN12),
> +       AD7193_DIFF_CHANNEL(12, 13, 0, AD7194_CH_AIN13),
> +       AD7193_DIFF_CHANNEL(13, 14, 0, AD7194_CH_AIN14),
> +       AD7193_DIFF_CHANNEL(14, 15, 0, AD7194_CH_AIN15),
> +       AD7193_DIFF_CHANNEL(15, 16, 0, AD7194_CH_AIN16),
> +       AD719x_TEMP_CHANNEL(16, AD7194_CH_TEMP),
> +       IIO_CHAN_SOFT_TIMESTAMP(17),
> +};

Based on the discussion on the v3 patch I was expecting this to be
fully dynamic rather than having a fixed number of channels since
there are so many possible combinations and the fact that
pseudo-differential channels should be using AD7193_CHANNEL() rather
than AD7193_DIFF_CHANNEL().

> +
> +static int ad7192_parse_channel(struct fwnode_handle *child)
> +{
> +       u32 reg, ain[2];
> +       int ret;
> +
> +       ret = fwnode_property_read_u32(child, "reg", &reg);
> +       if (ret)
> +               return ret;
> +
> +       if (!in_range(reg, AD7194_CH_DIFF_START, AD7194_CH_DIFF_NR))
> +               return -EINVAL;
> +
> +       ret = fwnode_property_read_u32_array(child, "diff-channels", ain,
> +                                            ARRAY_SIZE(ain));
> +       if (ret)
> +               return ret;
> +
> +       if (!in_range(ain[0], AD7194_CH_AIN0_START, AD7194_CH_AIN0_NR) ||
> +           !in_range(ain[1], AD7194_CH_AIN1_START, AD7194_CH_AIN1_NR))
> +               return -EINVAL;
> +
> +       reg--;
> +       ad7194_channels[reg].channel = ain[0];
> +       ad7194_channels[reg].channel2 = ain[1];

Needs to set ad7194_channels[reg].differential = ain[1] != 0.

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

Re: [PATCH v4 0/4] iio: adc: ad7192: Add support for AD7194

[Andy Shevchenko]

On Wed, Feb 28, 2024 at 02:26:13PM +0200, Alisa-Dariana Roman wrote:
> From: romandariana <alisa.roman@analog.com>

> Alisa-Dariana Roman (4):

Please fix and align the following:
- email From line (submitter)
- SoB tag(s)

-- 
With Best Regards,
Andy Shevchenko