[PATCH 0/4] Timestamp and PulSAR support for ad4000

[PATCH 0/4] Timestamp and PulSAR support for ad4000

[Marcelo Schmitt]

Complement the ad4000 driver with a timestamp channel, a minor adjust in
transfer timing, and support for single-channel PulSAR devices.

This series was tested with ADAQ4003, AD7687, and AD7691 on CoraZ7 board.

Marcelo Schmitt (4):
  dt-bindings: iio: adc: adi,ad4000: Add PulSAR
  iio: adc: ad4000: Add timestamp channel
  iio: adc: ad4000: Use device specific timing for SPI transfers
  iio: adc: ad4000: Add support for PulSAR devices

 .../bindings/iio/adc/adi,ad4000.yaml          | 115 ++++++-
 drivers/iio/adc/ad4000.c                      | 311 +++++++++++++++---
 2 files changed, 374 insertions(+), 52 deletions(-)


base-commit: 9dd2270ca0b38ee16094817f4a53e7ba78e31567
-- 
2.45.2

[PATCH 1/4] dt-bindings: iio: adc: adi,ad4000: Add PulSAR

[Marcelo Schmitt]

Extend the AD4000 series device tree documentation to also describe
PulSAR devices.

Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
---
 .../bindings/iio/adc/adi,ad4000.yaml          | 115 +++++++++++++++++-
 1 file changed, 114 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml
index e413a9d8d2a2..35049071a9de 100644
--- a/Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml
+++ b/Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml
@@ -19,6 +19,21 @@ description: |
     https://www.analog.com/media/en/technical-documentation/data-sheets/ad4020-4021-4022.pdf
     https://www.analog.com/media/en/technical-documentation/data-sheets/adaq4001.pdf
     https://www.analog.com/media/en/technical-documentation/data-sheets/adaq4003.pdf
+    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7685.pdf
+    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7686.pdf
+    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7687.pdf
+    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7688.pdf
+    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7690.pdf
+    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7691.pdf
+    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7693.pdf
+    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7694.pdf
+    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7942.pdf
+    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7946.pdf
+    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7980.pdf
+    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7982.pdf
+    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7983.pdf
+    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7984.pdf
+    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7988-1_7988-5.pdf
 
 $ref: /schemas/spi/spi-peripheral-props.yaml#
 
@@ -63,6 +78,38 @@ properties:
 
       - const: adi,adaq4003
 
+      - const: adi,ad7946
+      - items:
+          - enum:
+              - adi,ad7942
+          - const: adi,ad7946
+
+      - const: adi,ad7983
+      - items:
+          - enum:
+              - adi,ad7980
+              - adi,ad7988-5
+              - adi,ad7686
+              - adi,ad7685
+              - adi,ad7694
+              - adi,ad7988-1
+          - const: adi,ad7983
+
+      - const: adi,ad7688
+      - items:
+          - enum:
+              - adi,ad7693
+              - adi,ad7687
+          - const: adi,ad7688
+
+      - const: adi,ad7984
+      - items:
+          - enum:
+              - adi,ad7982
+              - adi,ad7690
+              - adi,ad7691
+          - const: adi,ad7984
+
   reg:
     maxItems: 1
 
@@ -129,10 +176,76 @@ required:
   - compatible
   - reg
   - vdd-supply
-  - vio-supply
   - ref-supply
 
 allOf:
+  # AD7694 doesn't have a VIO pin
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - adi,ad4000
+              - adi,ad4001
+              - adi,ad4002
+              - adi,ad4003
+              - adi,ad4004
+              - adi,ad4005
+              - adi,ad4006
+              - adi,ad4007
+              - adi,ad4008
+              - adi,ad4010
+              - adi,ad4011
+              - adi,ad4020
+              - adi,ad4021
+              - adi,ad4022
+              - adi,adaq4001
+              - adi,adaq4003
+              - adi,ad7685
+              - adi,ad7686
+              - adi,ad7687
+              - adi,ad7688
+              - adi,ad7690
+              - adi,ad7691
+              - adi,ad7693
+              - adi,ad7942
+              - adi,ad7946
+              - adi,ad7980
+              - adi,ad7982
+              - adi,ad7983
+              - adi,ad7984
+              - adi,ad7988-1
+              - adi,ad7988-5
+    then:
+      required:
+        - vio-supply
+  # Single-channel PulSAR devices have SDI either tied to VIO, GND, or host CS.
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - adi,ad7685
+              - adi,ad7686
+              - adi,ad7687
+              - adi,ad7688
+              - adi,ad7690
+              - adi,ad7691
+              - adi,ad7693
+              - adi,ad7694
+              - adi,ad7942
+              - adi,ad7946
+              - adi,ad7980
+              - adi,ad7982
+              - adi,ad7983
+              - adi,ad7984
+              - adi,ad7988-1
+              - adi,ad7988-5
+    then:
+      properties:
+        adi,sdi-pin:
+          enum: [ high, low, cs ]
+          default: high
   # The configuration register can only be accessed if SDI is connected to MOSI
   - if:
       required:
-- 
2.45.2

[PATCH 2/4] iio: adc: ad4000: Add timestamp channel

[Marcelo Schmitt]

The ADC data is pushed to the IIO buffer along with timestamp but no
timestamp channel was provided to retried the time data.
Add a timestamp channel to provide sample capture time.

Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
---
I was about to reply to the patches the other week but waited thinking I
would be able to test them on time.
My initial intent was to provide timestamps for ADC readings, but I didn't
realize an IIO timestamp channel would be needed (silly me).
David, do you want a Suggested-by tag in this one?

 drivers/iio/adc/ad4000.c | 98 +++++++++++++++++++++++-----------------
 1 file changed, 56 insertions(+), 42 deletions(-)

diff --git a/drivers/iio/adc/ad4000.c b/drivers/iio/adc/ad4000.c
index b3b82535f5c1..21731c4d31ee 100644
--- a/drivers/iio/adc/ad4000.c
+++ b/drivers/iio/adc/ad4000.c
@@ -49,6 +49,7 @@
 	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |				\
 			      BIT(IIO_CHAN_INFO_SCALE),				\
 	.info_mask_separate_available = _reg_access ? BIT(IIO_CHAN_INFO_SCALE) : 0,\
+	.scan_index = 0,							\
 	.scan_type = {								\
 		.sign = _sign,							\
 		.realbits = _real_bits,						\
@@ -62,6 +63,12 @@
 	__AD4000_DIFF_CHANNEL((_sign), (_real_bits),				\
 				     ((_real_bits) > 16 ? 32 : 16), (_reg_access))
 
+#define AD4000_DIFF_CHANNELS(_sign, _real_bits, _reg_access)			\
+{										\
+	AD4000_DIFF_CHANNEL(_sign, _real_bits, _reg_access),			\
+	IIO_CHAN_SOFT_TIMESTAMP(1)						\
+}
+
 #define __AD4000_PSEUDO_DIFF_CHANNEL(_sign, _real_bits, _storage_bits, _reg_access)\
 {										\
 	.type = IIO_VOLTAGE,							\
@@ -71,6 +78,7 @@
 			      BIT(IIO_CHAN_INFO_SCALE) |			\
 			      BIT(IIO_CHAN_INFO_OFFSET),			\
 	.info_mask_separate_available = _reg_access ? BIT(IIO_CHAN_INFO_SCALE) : 0,\
+	.scan_index = 0,							\
 	.scan_type = {								\
 		.sign = _sign,							\
 		.realbits = _real_bits,						\
@@ -84,6 +92,12 @@
 	__AD4000_PSEUDO_DIFF_CHANNEL((_sign), (_real_bits),			\
 				     ((_real_bits) > 16 ? 32 : 16), (_reg_access))
 
+#define AD4000_PSEUDO_DIFF_CHANNELS(_sign, _real_bits, _reg_access)		\
+{										\
+	AD4000_PSEUDO_DIFF_CHANNEL(_sign, _real_bits, _reg_access),		\
+	IIO_CHAN_SOFT_TIMESTAMP(1)						\
+}
+
 static const char * const ad4000_power_supplies[] = {
 	"vdd", "vio"
 };
@@ -110,106 +124,106 @@ static const int ad4000_gains[] = {
 
 struct ad4000_chip_info {
 	const char *dev_name;
-	struct iio_chan_spec chan_spec;
-	struct iio_chan_spec reg_access_chan_spec;
+	struct iio_chan_spec chan_spec[2];
+	struct iio_chan_spec reg_access_chan_spec[2];
 	bool has_hardware_gain;
 };
 
 static const struct ad4000_chip_info ad4000_chip_info = {
 	.dev_name = "ad4000",
-	.chan_spec = AD4000_PSEUDO_DIFF_CHANNEL('u', 16, 0),
-	.reg_access_chan_spec = AD4000_PSEUDO_DIFF_CHANNEL('u', 16, 1),
+	.chan_spec = AD4000_PSEUDO_DIFF_CHANNELS('u', 16, 0),
+	.reg_access_chan_spec = AD4000_PSEUDO_DIFF_CHANNELS('u', 16, 1),
 };
 
 static const struct ad4000_chip_info ad4001_chip_info = {
 	.dev_name = "ad4001",
-	.chan_spec = AD4000_DIFF_CHANNEL('s', 16, 0),
-	.reg_access_chan_spec = AD4000_DIFF_CHANNEL('s', 16, 1),
+	.chan_spec = AD4000_DIFF_CHANNELS('s', 16, 0),
+	.reg_access_chan_spec = AD4000_DIFF_CHANNELS('s', 16, 1),
 };
 
 static const struct ad4000_chip_info ad4002_chip_info = {
 	.dev_name = "ad4002",
-	.chan_spec = AD4000_PSEUDO_DIFF_CHANNEL('u', 18, 0),
-	.reg_access_chan_spec = AD4000_PSEUDO_DIFF_CHANNEL('u', 18, 1),
+	.chan_spec = AD4000_PSEUDO_DIFF_CHANNELS('u', 18, 0),
+	.reg_access_chan_spec = AD4000_PSEUDO_DIFF_CHANNELS('u', 18, 1),
 };
 
 static const struct ad4000_chip_info ad4003_chip_info = {
 	.dev_name = "ad4003",
-	.chan_spec = AD4000_DIFF_CHANNEL('s', 18, 0),
-	.reg_access_chan_spec = AD4000_DIFF_CHANNEL('s', 18, 1),
+	.chan_spec = AD4000_DIFF_CHANNELS('s', 18, 0),
+	.reg_access_chan_spec = AD4000_DIFF_CHANNELS('s', 18, 1),
 };
 
 static const struct ad4000_chip_info ad4004_chip_info = {
 	.dev_name = "ad4004",
-	.chan_spec = AD4000_PSEUDO_DIFF_CHANNEL('u', 16, 0),
-	.reg_access_chan_spec = AD4000_PSEUDO_DIFF_CHANNEL('u', 16, 1),
+	.chan_spec = AD4000_PSEUDO_DIFF_CHANNELS('u', 16, 0),
+	.reg_access_chan_spec = AD4000_PSEUDO_DIFF_CHANNELS('u', 16, 1),
 };
 
 static const struct ad4000_chip_info ad4005_chip_info = {
 	.dev_name = "ad4005",
-	.chan_spec = AD4000_DIFF_CHANNEL('s', 16, 0),
-	.reg_access_chan_spec = AD4000_DIFF_CHANNEL('s', 16, 1),
+	.chan_spec = AD4000_DIFF_CHANNELS('s', 16, 0),
+	.reg_access_chan_spec = AD4000_DIFF_CHANNELS('s', 16, 1),
 };
 
 static const struct ad4000_chip_info ad4006_chip_info = {
 	.dev_name = "ad4006",
-	.chan_spec = AD4000_PSEUDO_DIFF_CHANNEL('u', 18, 0),
-	.reg_access_chan_spec = AD4000_PSEUDO_DIFF_CHANNEL('u', 18, 1),
+	.chan_spec = AD4000_PSEUDO_DIFF_CHANNELS('u', 18, 0),
+	.reg_access_chan_spec = AD4000_PSEUDO_DIFF_CHANNELS('u', 18, 1),
 };
 
 static const struct ad4000_chip_info ad4007_chip_info = {
 	.dev_name = "ad4007",
-	.chan_spec = AD4000_DIFF_CHANNEL('s', 18, 0),
-	.reg_access_chan_spec = AD4000_DIFF_CHANNEL('s', 18, 1),
+	.chan_spec = AD4000_DIFF_CHANNELS('s', 18, 0),
+	.reg_access_chan_spec = AD4000_DIFF_CHANNELS('s', 18, 1),
 };
 
 static const struct ad4000_chip_info ad4008_chip_info = {
 	.dev_name = "ad4008",
-	.chan_spec = AD4000_PSEUDO_DIFF_CHANNEL('u', 16, 0),
-	.reg_access_chan_spec = AD4000_PSEUDO_DIFF_CHANNEL('u', 16, 1),
+	.chan_spec = AD4000_PSEUDO_DIFF_CHANNELS('u', 16, 0),
+	.reg_access_chan_spec = AD4000_PSEUDO_DIFF_CHANNELS('u', 16, 1),
 };
 
 static const struct ad4000_chip_info ad4010_chip_info = {
 	.dev_name = "ad4010",
-	.chan_spec = AD4000_PSEUDO_DIFF_CHANNEL('u', 18, 0),
-	.reg_access_chan_spec = AD4000_PSEUDO_DIFF_CHANNEL('u', 18, 1),
+	.chan_spec = AD4000_PSEUDO_DIFF_CHANNELS('u', 18, 0),
+	.reg_access_chan_spec = AD4000_PSEUDO_DIFF_CHANNELS('u', 18, 1),
 };
 
 static const struct ad4000_chip_info ad4011_chip_info = {
 	.dev_name = "ad4011",
-	.chan_spec = AD4000_DIFF_CHANNEL('s', 18, 0),
-	.reg_access_chan_spec = AD4000_DIFF_CHANNEL('s', 18, 1),
+	.chan_spec = AD4000_DIFF_CHANNELS('s', 18, 0),
+	.reg_access_chan_spec = AD4000_DIFF_CHANNELS('s', 18, 1),
 };
 
 static const struct ad4000_chip_info ad4020_chip_info = {
 	.dev_name = "ad4020",
-	.chan_spec = AD4000_DIFF_CHANNEL('s', 20, 0),
-	.reg_access_chan_spec = AD4000_DIFF_CHANNEL('s', 20, 1),
+	.chan_spec = AD4000_DIFF_CHANNELS('s', 20, 0),
+	.reg_access_chan_spec = AD4000_DIFF_CHANNELS('s', 20, 1),
 };
 
 static const struct ad4000_chip_info ad4021_chip_info = {
 	.dev_name = "ad4021",
-	.chan_spec = AD4000_DIFF_CHANNEL('s', 20, 0),
-	.reg_access_chan_spec = AD4000_DIFF_CHANNEL('s', 20, 1),
+	.chan_spec = AD4000_DIFF_CHANNELS('s', 20, 0),
+	.reg_access_chan_spec = AD4000_DIFF_CHANNELS('s', 20, 1),
 };
 
 static const struct ad4000_chip_info ad4022_chip_info = {
 	.dev_name = "ad4022",
-	.chan_spec = AD4000_DIFF_CHANNEL('s', 20, 0),
-	.reg_access_chan_spec = AD4000_DIFF_CHANNEL('s', 20, 1),
+	.chan_spec = AD4000_DIFF_CHANNELS('s', 20, 0),
+	.reg_access_chan_spec = AD4000_DIFF_CHANNELS('s', 20, 1),
 };
 
 static const struct ad4000_chip_info adaq4001_chip_info = {
 	.dev_name = "adaq4001",
-	.chan_spec = AD4000_DIFF_CHANNEL('s', 16, 0),
-	.reg_access_chan_spec = AD4000_DIFF_CHANNEL('s', 16, 1),
+	.chan_spec = AD4000_DIFF_CHANNELS('s', 16, 0),
+	.reg_access_chan_spec = AD4000_DIFF_CHANNELS('s', 16, 1),
 	.has_hardware_gain = true,
 };
 
 static const struct ad4000_chip_info adaq4003_chip_info = {
 	.dev_name = "adaq4003",
-	.chan_spec = AD4000_DIFF_CHANNEL('s', 18, 0),
-	.reg_access_chan_spec = AD4000_DIFF_CHANNEL('s', 18, 1),
+	.chan_spec = AD4000_DIFF_CHANNELS('s', 18, 0),
+	.reg_access_chan_spec = AD4000_DIFF_CHANNELS('s', 18, 1),
 	.has_hardware_gain = true,
 };
 
@@ -591,7 +605,7 @@ static int ad4000_probe(struct spi_device *spi)
 	switch (st->sdi_pin) {
 	case AD4000_SDI_MOSI:
 		indio_dev->info = &ad4000_reg_access_info;
-		indio_dev->channels = &chip->reg_access_chan_spec;
+		indio_dev->channels = chip->reg_access_chan_spec;
 
 		/*
 		 * In "3-wire mode", the ADC SDI line must be kept high when
@@ -603,7 +617,7 @@ static int ad4000_probe(struct spi_device *spi)
 		if (ret < 0)
 			return ret;
 
-		ret = ad4000_prepare_3wire_mode_message(st, indio_dev->channels);
+		ret = ad4000_prepare_3wire_mode_message(st, &indio_dev->channels[0]);
 		if (ret)
 			return ret;
 
@@ -614,16 +628,16 @@ static int ad4000_probe(struct spi_device *spi)
 		break;
 	case AD4000_SDI_VIO:
 		indio_dev->info = &ad4000_info;
-		indio_dev->channels = &chip->chan_spec;
-		ret = ad4000_prepare_3wire_mode_message(st, indio_dev->channels);
+		indio_dev->channels = chip->chan_spec;
+		ret = ad4000_prepare_3wire_mode_message(st, &indio_dev->channels[0]);
 		if (ret)
 			return ret;
 
 		break;
 	case AD4000_SDI_CS:
 		indio_dev->info = &ad4000_info;
-		indio_dev->channels = &chip->chan_spec;
-		ret = ad4000_prepare_4wire_mode_message(st, indio_dev->channels);
+		indio_dev->channels = chip->chan_spec;
+		ret = ad4000_prepare_4wire_mode_message(st, &indio_dev->channels[0]);
 		if (ret)
 			return ret;
 
@@ -637,7 +651,7 @@ static int ad4000_probe(struct spi_device *spi)
 	}
 
 	indio_dev->name = chip->dev_name;
-	indio_dev->num_channels = 1;
+	indio_dev->num_channels = 2;
 
 	ret = devm_mutex_init(dev, &st->lock);
 	if (ret)
@@ -658,7 +672,7 @@ static int ad4000_probe(struct spi_device *spi)
 		}
 	}
 
-	ad4000_fill_scale_tbl(st, indio_dev->channels);
+	ad4000_fill_scale_tbl(st, &indio_dev->channels[0]);
 
 	ret = devm_iio_triggered_buffer_setup(dev, indio_dev,
 					      &iio_pollfunc_store_time,
-- 
2.45.2

[PATCH 3/4] iio: adc: ad4000: Use device specific timing for SPI transfers

[Marcelo Schmitt]

The SPI transfers for AD4020, AD4021, and AD4022 have slightly different
timing specifications. Use device specific timing constraints to set SPI
transfer parameters.

Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
---
 drivers/iio/adc/ad4000.c | 50 ++++++++++++++++++++++++++++++++--------
 1 file changed, 41 insertions(+), 9 deletions(-)

diff --git a/drivers/iio/adc/ad4000.c b/drivers/iio/adc/ad4000.c
index 21731c4d31ee..68ac77494263 100644
--- a/drivers/iio/adc/ad4000.c
+++ b/drivers/iio/adc/ad4000.c
@@ -35,10 +35,6 @@
 
 #define AD4000_SCALE_OPTIONS		2
 
-#define AD4000_TQUIET1_NS		190
-#define AD4000_TQUIET2_NS		60
-#define AD4000_TCONV_NS			320
-
 #define __AD4000_DIFF_CHANNEL(_sign, _real_bits, _storage_bits, _reg_access)	\
 {										\
 	.type = IIO_VOLTAGE,							\
@@ -122,10 +118,30 @@ static const int ad4000_gains[] = {
 	454, 909, 1000, 1900,
 };
 
+struct ad4000_time_spec {
+	int t_conv_ns;
+	int t_quiet2_ns;
+};
+
+/*
+ * Same timing specifications for all of AD4000, AD4001, ..., AD4008, AD4010,
+ * ADAQ4001, and ADAQ4003.
+ */
+static const struct ad4000_time_spec ad4000_t_spec = {
+	.t_conv_ns = 320,
+	.t_quiet2_ns = 60,
+};
+
+static const struct ad4000_time_spec ad4020_t_spec = {
+	.t_conv_ns = 350,
+	.t_quiet2_ns = 60,
+};
+
 struct ad4000_chip_info {
 	const char *dev_name;
 	struct iio_chan_spec chan_spec[2];
 	struct iio_chan_spec reg_access_chan_spec[2];
+	const struct ad4000_time_spec *time_spec;
 	bool has_hardware_gain;
 };
 
@@ -133,90 +149,105 @@ static const struct ad4000_chip_info ad4000_chip_info = {
 	.dev_name = "ad4000",
 	.chan_spec = AD4000_PSEUDO_DIFF_CHANNELS('u', 16, 0),
 	.reg_access_chan_spec = AD4000_PSEUDO_DIFF_CHANNELS('u', 16, 1),
+	.time_spec = &ad4000_t_spec,
 };
 
 static const struct ad4000_chip_info ad4001_chip_info = {
 	.dev_name = "ad4001",
 	.chan_spec = AD4000_DIFF_CHANNELS('s', 16, 0),
 	.reg_access_chan_spec = AD4000_DIFF_CHANNELS('s', 16, 1),
+	.time_spec = &ad4000_t_spec,
 };
 
 static const struct ad4000_chip_info ad4002_chip_info = {
 	.dev_name = "ad4002",
 	.chan_spec = AD4000_PSEUDO_DIFF_CHANNELS('u', 18, 0),
 	.reg_access_chan_spec = AD4000_PSEUDO_DIFF_CHANNELS('u', 18, 1),
+	.time_spec = &ad4000_t_spec,
 };
 
 static const struct ad4000_chip_info ad4003_chip_info = {
 	.dev_name = "ad4003",
 	.chan_spec = AD4000_DIFF_CHANNELS('s', 18, 0),
 	.reg_access_chan_spec = AD4000_DIFF_CHANNELS('s', 18, 1),
+	.time_spec = &ad4000_t_spec,
 };
 
 static const struct ad4000_chip_info ad4004_chip_info = {
 	.dev_name = "ad4004",
 	.chan_spec = AD4000_PSEUDO_DIFF_CHANNELS('u', 16, 0),
 	.reg_access_chan_spec = AD4000_PSEUDO_DIFF_CHANNELS('u', 16, 1),
+	.time_spec = &ad4000_t_spec,
 };
 
 static const struct ad4000_chip_info ad4005_chip_info = {
 	.dev_name = "ad4005",
 	.chan_spec = AD4000_DIFF_CHANNELS('s', 16, 0),
 	.reg_access_chan_spec = AD4000_DIFF_CHANNELS('s', 16, 1),
+	.time_spec = &ad4000_t_spec,
 };
 
 static const struct ad4000_chip_info ad4006_chip_info = {
 	.dev_name = "ad4006",
 	.chan_spec = AD4000_PSEUDO_DIFF_CHANNELS('u', 18, 0),
 	.reg_access_chan_spec = AD4000_PSEUDO_DIFF_CHANNELS('u', 18, 1),
+	.time_spec = &ad4000_t_spec,
 };
 
 static const struct ad4000_chip_info ad4007_chip_info = {
 	.dev_name = "ad4007",
 	.chan_spec = AD4000_DIFF_CHANNELS('s', 18, 0),
 	.reg_access_chan_spec = AD4000_DIFF_CHANNELS('s', 18, 1),
+	.time_spec = &ad4000_t_spec,
 };
 
 static const struct ad4000_chip_info ad4008_chip_info = {
 	.dev_name = "ad4008",
 	.chan_spec = AD4000_PSEUDO_DIFF_CHANNELS('u', 16, 0),
 	.reg_access_chan_spec = AD4000_PSEUDO_DIFF_CHANNELS('u', 16, 1),
+	.time_spec = &ad4000_t_spec,
 };
 
 static const struct ad4000_chip_info ad4010_chip_info = {
 	.dev_name = "ad4010",
 	.chan_spec = AD4000_PSEUDO_DIFF_CHANNELS('u', 18, 0),
 	.reg_access_chan_spec = AD4000_PSEUDO_DIFF_CHANNELS('u', 18, 1),
+	.time_spec = &ad4000_t_spec,
 };
 
 static const struct ad4000_chip_info ad4011_chip_info = {
 	.dev_name = "ad4011",
 	.chan_spec = AD4000_DIFF_CHANNELS('s', 18, 0),
 	.reg_access_chan_spec = AD4000_DIFF_CHANNELS('s', 18, 1),
+	.time_spec = &ad4000_t_spec,
 };
 
 static const struct ad4000_chip_info ad4020_chip_info = {
 	.dev_name = "ad4020",
 	.chan_spec = AD4000_DIFF_CHANNELS('s', 20, 0),
 	.reg_access_chan_spec = AD4000_DIFF_CHANNELS('s', 20, 1),
+	.time_spec = &ad4020_t_spec,
 };
 
 static const struct ad4000_chip_info ad4021_chip_info = {
 	.dev_name = "ad4021",
 	.chan_spec = AD4000_DIFF_CHANNELS('s', 20, 0),
 	.reg_access_chan_spec = AD4000_DIFF_CHANNELS('s', 20, 1),
+	.time_spec = &ad4020_t_spec,
 };
 
 static const struct ad4000_chip_info ad4022_chip_info = {
 	.dev_name = "ad4022",
 	.chan_spec = AD4000_DIFF_CHANNELS('s', 20, 0),
 	.reg_access_chan_spec = AD4000_DIFF_CHANNELS('s', 20, 1),
+	.time_spec = &ad4020_t_spec,
 };
 
 static const struct ad4000_chip_info adaq4001_chip_info = {
 	.dev_name = "adaq4001",
 	.chan_spec = AD4000_DIFF_CHANNELS('s', 16, 0),
 	.reg_access_chan_spec = AD4000_DIFF_CHANNELS('s', 16, 1),
+	.time_spec = &ad4000_t_spec,
 	.has_hardware_gain = true,
 };
 
@@ -224,6 +255,7 @@ static const struct ad4000_chip_info adaq4003_chip_info = {
 	.dev_name = "adaq4003",
 	.chan_spec = AD4000_DIFF_CHANNELS('s', 18, 0),
 	.reg_access_chan_spec = AD4000_DIFF_CHANNELS('s', 18, 1),
+	.time_spec = &ad4000_t_spec,
 	.has_hardware_gain = true,
 };
 
@@ -238,6 +270,7 @@ struct ad4000_state {
 	bool span_comp;
 	u16 gain_milli;
 	int scale_tbl[AD4000_SCALE_OPTIONS][2];
+	const struct ad4000_time_spec *time_spec;
 
 	/*
 	 * DMA (thus cache coherency maintenance) requires the transfer buffers
@@ -502,16 +535,15 @@ static const struct iio_info ad4000_info = {
 static int ad4000_prepare_3wire_mode_message(struct ad4000_state *st,
 					     const struct iio_chan_spec *chan)
 {
-	unsigned int cnv_pulse_time = AD4000_TCONV_NS;
 	struct spi_transfer *xfers = st->xfers;
 
 	xfers[0].cs_change = 1;
-	xfers[0].cs_change_delay.value = cnv_pulse_time;
+	xfers[0].cs_change_delay.value = st->time_spec->t_conv_ns;
 	xfers[0].cs_change_delay.unit = SPI_DELAY_UNIT_NSECS;
 
 	xfers[1].rx_buf = &st->scan.data;
 	xfers[1].len = BITS_TO_BYTES(chan->scan_type.storagebits);
-	xfers[1].delay.value = AD4000_TQUIET2_NS;
+	xfers[1].delay.value = st->time_spec->t_quiet2_ns;
 	xfers[1].delay.unit = SPI_DELAY_UNIT_NSECS;
 
 	spi_message_init_with_transfers(&st->msg, st->xfers, 2);
@@ -529,7 +561,6 @@ static int ad4000_prepare_3wire_mode_message(struct ad4000_state *st,
 static int ad4000_prepare_4wire_mode_message(struct ad4000_state *st,
 					     const struct iio_chan_spec *chan)
 {
-	unsigned int cnv_to_sdi_time = AD4000_TCONV_NS;
 	struct spi_transfer *xfers = st->xfers;
 
 	/*
@@ -537,7 +568,7 @@ static int ad4000_prepare_4wire_mode_message(struct ad4000_state *st,
 	 * going low.
 	 */
 	xfers[0].cs_off = 1;
-	xfers[0].delay.value = cnv_to_sdi_time;
+	xfers[0].delay.value = st->time_spec->t_conv_ns;
 	xfers[0].delay.unit = SPI_DELAY_UNIT_NSECS;
 
 	xfers[1].rx_buf = &st->scan.data;
@@ -576,6 +607,7 @@ static int ad4000_probe(struct spi_device *spi)
 
 	st = iio_priv(indio_dev);
 	st->spi = spi;
+	st->time_spec = chip->time_spec;
 
 	ret = devm_regulator_bulk_get_enable(dev, ARRAY_SIZE(ad4000_power_supplies),
 					     ad4000_power_supplies);
-- 
2.45.2

[PATCH 4/4] iio: adc: ad4000: Add support for PulSAR devices

[Marcelo Schmitt]

The AD4000 series and the single-channel PulSAR series of devices have
similar SPI transfer specifications and wiring configurations.
Single-channel PulSAR devices are slower than AD4000, and don't have a
configuration register. That taken into account, single-channel PulSARs can
be supported by the ad4000 driver without any increase in code complexity.

Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
---
 drivers/iio/adc/ad4000.c | 163 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 163 insertions(+)

diff --git a/drivers/iio/adc/ad4000.c b/drivers/iio/adc/ad4000.c
index 68ac77494263..8e31b42534f5 100644
--- a/drivers/iio/adc/ad4000.c
+++ b/drivers/iio/adc/ad4000.c
@@ -137,6 +137,41 @@ static const struct ad4000_time_spec ad4020_t_spec = {
 	.t_quiet2_ns = 60,
 };
 
+/* AD7983, AD7984 */
+static const struct ad4000_time_spec ad7983_t_spec = {
+	.t_conv_ns = 500,
+};
+
+/* AD7980, AD7982 */
+static const struct ad4000_time_spec ad7980_t_spec = {
+	.t_conv_ns = 800,
+};
+
+/* AD7946, AD7686, AD7688, AD7988-5, AD7693 */
+static const struct ad4000_time_spec ad7686_t_spec = {
+	.t_conv_ns = 1600,
+};
+
+/* AD7690 */
+static const struct ad4000_time_spec ad7690_t_spec = {
+	.t_conv_ns = 2100,
+};
+
+/* AD7942, AD7685, AD7687, AD7694 */
+static const struct ad4000_time_spec ad7687_t_spec = {
+	.t_conv_ns = 3200,
+};
+
+/* AD7691 */
+static const struct ad4000_time_spec ad7691_t_spec = {
+	.t_conv_ns = 3700,
+};
+
+/* AD7988-1 */
+static const struct ad4000_time_spec ad7988_1_t_spec = {
+	.t_conv_ns = 9500,
+};
+
 struct ad4000_chip_info {
 	const char *dev_name;
 	struct iio_chan_spec chan_spec[2];
@@ -259,6 +294,102 @@ static const struct ad4000_chip_info adaq4003_chip_info = {
 	.has_hardware_gain = true,
 };
 
+static const struct ad4000_chip_info ad7685_chip_info = {
+	.dev_name = "ad7685",
+	.chan_spec = AD4000_PSEUDO_DIFF_CHANNELS('u', 16, 0),
+	.time_spec = &ad7687_t_spec,
+};
+
+static const struct ad4000_chip_info ad7686_chip_info = {
+	.dev_name = "ad7686",
+	.chan_spec = AD4000_PSEUDO_DIFF_CHANNELS('u', 16, 0),
+	.time_spec = &ad7686_t_spec,
+};
+
+static const struct ad4000_chip_info ad7687_chip_info = {
+	.dev_name = "ad7687",
+	.chan_spec = AD4000_DIFF_CHANNELS('s', 16, 0),
+	.time_spec = &ad7687_t_spec,
+};
+
+static const struct ad4000_chip_info ad7688_chip_info = {
+	.dev_name = "ad7688",
+	.chan_spec = AD4000_DIFF_CHANNELS('s', 16, 0),
+	.time_spec = &ad7686_t_spec,
+};
+
+static const struct ad4000_chip_info ad7690_chip_info = {
+	.dev_name = "ad7690",
+	.chan_spec = AD4000_DIFF_CHANNELS('s', 18, 0),
+	.time_spec = &ad7690_t_spec,
+};
+
+static const struct ad4000_chip_info ad7691_chip_info = {
+	.dev_name = "ad7691",
+	.chan_spec = AD4000_DIFF_CHANNELS('s', 18, 0),
+	.time_spec = &ad7691_t_spec,
+};
+
+static const struct ad4000_chip_info ad7693_chip_info = {
+	.dev_name = "ad7693",
+	.chan_spec = AD4000_DIFF_CHANNELS('s', 16, 0),
+	.time_spec = &ad7686_t_spec,
+};
+
+static const struct ad4000_chip_info ad7694_chip_info = {
+	.dev_name = "ad7694",
+	.chan_spec = AD4000_PSEUDO_DIFF_CHANNELS('u', 16, 0),
+	.time_spec = &ad7687_t_spec,
+};
+
+static const struct ad4000_chip_info ad7942_chip_info = {
+	.dev_name = "ad7942",
+	.chan_spec = AD4000_PSEUDO_DIFF_CHANNELS('u', 14, 0),
+	.time_spec = &ad7687_t_spec,
+};
+
+static const struct ad4000_chip_info ad7946_chip_info = {
+	.dev_name = "ad7946",
+	.chan_spec = AD4000_PSEUDO_DIFF_CHANNELS('u', 14, 0),
+	.time_spec = &ad7686_t_spec,
+};
+
+static const struct ad4000_chip_info ad7980_chip_info = {
+	.dev_name = "ad7980",
+	.chan_spec = AD4000_PSEUDO_DIFF_CHANNELS('u', 16, 0),
+	.time_spec = &ad7980_t_spec,
+};
+
+static const struct ad4000_chip_info ad7982_chip_info = {
+	.dev_name = "ad7982",
+	.chan_spec = AD4000_DIFF_CHANNELS('s', 18, 0),
+	.time_spec = &ad7980_t_spec,
+};
+
+static const struct ad4000_chip_info ad7983_chip_info = {
+	.dev_name = "ad7983",
+	.chan_spec = AD4000_PSEUDO_DIFF_CHANNELS('u', 16, 0),
+	.time_spec = &ad7983_t_spec,
+};
+
+static const struct ad4000_chip_info ad7984_chip_info = {
+	.dev_name = "ad7984",
+	.chan_spec = AD4000_DIFF_CHANNELS('s', 18, 0),
+	.time_spec = &ad7983_t_spec,
+};
+
+static const struct ad4000_chip_info ad7988_1_chip_info = {
+	.dev_name = "ad7988-1",
+	.chan_spec = AD4000_PSEUDO_DIFF_CHANNELS('u', 16, 0),
+	.time_spec = &ad7988_1_t_spec,
+};
+
+static const struct ad4000_chip_info ad7988_5_chip_info = {
+	.dev_name = "ad7988-5",
+	.chan_spec = AD4000_PSEUDO_DIFF_CHANNELS('u', 16, 0),
+	.time_spec = &ad7686_t_spec,
+};
+
 struct ad4000_state {
 	struct spi_device *spi;
 	struct gpio_desc *cnv_gpio;
@@ -732,6 +863,22 @@ static const struct spi_device_id ad4000_id[] = {
 	{ "ad4022", (kernel_ulong_t)&ad4022_chip_info },
 	{ "adaq4001", (kernel_ulong_t)&adaq4001_chip_info },
 	{ "adaq4003", (kernel_ulong_t)&adaq4003_chip_info },
+	{ "ad7685", (kernel_ulong_t)&ad7685_chip_info },
+	{ "ad7686", (kernel_ulong_t)&ad7686_chip_info },
+	{ "ad7687", (kernel_ulong_t)&ad7687_chip_info },
+	{ "ad7688", (kernel_ulong_t)&ad7688_chip_info },
+	{ "ad7690", (kernel_ulong_t)&ad7690_chip_info },
+	{ "ad7691", (kernel_ulong_t)&ad7691_chip_info },
+	{ "ad7693", (kernel_ulong_t)&ad7693_chip_info },
+	{ "ad7694", (kernel_ulong_t)&ad7694_chip_info },
+	{ "ad7942", (kernel_ulong_t)&ad7942_chip_info },
+	{ "ad7946", (kernel_ulong_t)&ad7946_chip_info },
+	{ "ad7980", (kernel_ulong_t)&ad7980_chip_info },
+	{ "ad7982", (kernel_ulong_t)&ad7982_chip_info },
+	{ "ad7983", (kernel_ulong_t)&ad7983_chip_info },
+	{ "ad7984", (kernel_ulong_t)&ad7984_chip_info },
+	{ "ad7988-1", (kernel_ulong_t)&ad7988_1_chip_info },
+	{ "ad7988-5", (kernel_ulong_t)&ad7988_5_chip_info },
 	{ }
 };
 MODULE_DEVICE_TABLE(spi, ad4000_id);
@@ -753,6 +900,22 @@ static const struct of_device_id ad4000_of_match[] = {
 	{ .compatible = "adi,ad4022", .data = &ad4022_chip_info },
 	{ .compatible = "adi,adaq4001", .data = &adaq4001_chip_info },
 	{ .compatible = "adi,adaq4003", .data = &adaq4003_chip_info },
+	{ .compatible = "adi,ad7685", .data = &ad7685_chip_info },
+	{ .compatible = "adi,ad7686", .data = &ad7686_chip_info },
+	{ .compatible = "adi,ad7687", .data = &ad7687_chip_info },
+	{ .compatible = "adi,ad7688", .data = &ad7688_chip_info },
+	{ .compatible = "adi,ad7690", .data = &ad7690_chip_info },
+	{ .compatible = "adi,ad7691", .data = &ad7691_chip_info },
+	{ .compatible = "adi,ad7693", .data = &ad7693_chip_info },
+	{ .compatible = "adi,ad7694", .data = &ad7694_chip_info },
+	{ .compatible = "adi,ad7942", .data = &ad7942_chip_info },
+	{ .compatible = "adi,ad7946", .data = &ad7946_chip_info },
+	{ .compatible = "adi,ad7980", .data = &ad7980_chip_info },
+	{ .compatible = "adi,ad7982", .data = &ad7982_chip_info },
+	{ .compatible = "adi,ad7983", .data = &ad7983_chip_info },
+	{ .compatible = "adi,ad7984", .data = &ad7984_chip_info },
+	{ .compatible = "adi,ad7988-1", .data = &ad7988_1_chip_info },
+	{ .compatible = "adi,ad7988-5", .data = &ad7988_5_chip_info },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, ad4000_of_match);
-- 
2.45.2

Re: [PATCH 1/4] dt-bindings: iio: adc: adi,ad4000: Add PulSAR

[David Lechner]

On 11/14/24 5:50 PM, Marcelo Schmitt wrote:
> Extend the AD4000 series device tree documentation to also describe
> PulSAR devices.
> 
> Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
> ---
>  .../bindings/iio/adc/adi,ad4000.yaml          | 115 +++++++++++++++++-
>  1 file changed, 114 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml
> index e413a9d8d2a2..35049071a9de 100644
> --- a/Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml
> @@ -19,6 +19,21 @@ description: |
>      https://www.analog.com/media/en/technical-documentation/data-sheets/ad4020-4021-4022.pdf
>      https://www.analog.com/media/en/technical-documentation/data-sheets/adaq4001.pdf
>      https://www.analog.com/media/en/technical-documentation/data-sheets/adaq4003.pdf
> +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7685.pdf
> +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7686.pdf
> +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7687.pdf
> +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7688.pdf
> +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7690.pdf
> +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7691.pdf
> +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7693.pdf
> +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7694.pdf
> +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7942.pdf
> +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7946.pdf
> +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7980.pdf
> +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7982.pdf
> +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7983.pdf
> +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7984.pdf
> +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7988-1_7988-5.pdf

It would be nice to sort these lowest number first.

>  
>  $ref: /schemas/spi/spi-peripheral-props.yaml#
>  
> @@ -63,6 +78,38 @@ properties:
>  
>        - const: adi,adaq4003
>  
> +      - const: adi,ad7946
> +      - items:
> +          - enum:
> +              - adi,ad7942
> +          - const: adi,ad7946
> +
> +      - const: adi,ad7983
> +      - items:
> +          - enum:
> +              - adi,ad7980
> +              - adi,ad7988-5
> +              - adi,ad7686
> +              - adi,ad7685
> +              - adi,ad7694
> +              - adi,ad7988-1
> +          - const: adi,ad7983
> +
> +      - const: adi,ad7688
> +      - items:
> +          - enum:
> +              - adi,ad7693
> +              - adi,ad7687
> +          - const: adi,ad7688
> +
> +      - const: adi,ad7984
> +      - items:
> +          - enum:
> +              - adi,ad7982
> +              - adi,ad7690
> +              - adi,ad7691
> +          - const: adi,ad7984
> +

IMHO, having fallbacks just makes the bindings harder to use and doesn't
actually provide any useful benefit.

And with this many chips, it can be easy to overlook a small difference
in one chips, like ad7694 not having VIO pin, so is it really fallback
compatible? Easier to just avoid the question and not have fallbacks.

>    reg:
>      maxItems: 1
>  
> @@ -129,10 +176,76 @@ required:
>    - compatible
>    - reg
>    - vdd-supply
> -  - vio-supply
>    - ref-supply
>  
>  allOf:
> +  # AD7694 doesn't have a VIO pin

It sounds like using not: could make this if: a lot shorter.

Also, it looks like ad7983 doesn't have the pin either.

> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - adi,ad4000
> +              - adi,ad4001
> +              - adi,ad4002
> +              - adi,ad4003
> +              - adi,ad4004
> +              - adi,ad4005
> +              - adi,ad4006
> +              - adi,ad4007
> +              - adi,ad4008
> +              - adi,ad4010
> +              - adi,ad4011
> +              - adi,ad4020
> +              - adi,ad4021
> +              - adi,ad4022
> +              - adi,adaq4001
> +              - adi,adaq4003
> +              - adi,ad7685
> +              - adi,ad7686
> +              - adi,ad7687
> +              - adi,ad7688
> +              - adi,ad7690
> +              - adi,ad7691
> +              - adi,ad7693
> +              - adi,ad7942
> +              - adi,ad7946
> +              - adi,ad7980
> +              - adi,ad7982
> +              - adi,ad7983
> +              - adi,ad7984
> +              - adi,ad7988-1
> +              - adi,ad7988-5
> +    then:
> +      required:
> +        - vio-supply
> +  # Single-channel PulSAR devices have SDI either tied to VIO, GND, or host CS.

To me, the more interesting thing to say here is that the sdi
option is omitted because these chips don't have a programmable
register.

> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - adi,ad7685
> +              - adi,ad7686
> +              - adi,ad7687
> +              - adi,ad7688
> +              - adi,ad7690
> +              - adi,ad7691
> +              - adi,ad7693
> +              - adi,ad7694
> +              - adi,ad7942
> +              - adi,ad7946
> +              - adi,ad7980
> +              - adi,ad7982
> +              - adi,ad7983
> +              - adi,ad7984
> +              - adi,ad7988-1
> +              - adi,ad7988-5
> +    then:
> +      properties:
> +        adi,sdi-pin:
> +          enum: [ high, low, cs ]
> +          default: high

For the similar ad7944, Rob suggested that the default should be the equivalent
of "cs" since that is most like "regular" SPI. So I think it makes sense do the
same here. (The adi,spi-mode property in the ad7944 binding is named a bit
different, single = high, chain = low and _property omitted_ (default) = cs)

>    # The configuration register can only be accessed if SDI is connected to MOSI
>    - if:
>        required:

Re: [PATCH 2/4] iio: adc: ad4000: Add timestamp channel

[David Lechner]

On 11/14/24 5:50 PM, Marcelo Schmitt wrote:
> The ADC data is pushed to the IIO buffer along with timestamp but no
> timestamp channel was provided to retried the time data.
> Add a timestamp channel to provide sample capture time.
> 
> Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
> ---
> I was about to reply to the patches the other week but waited thinking I
> would be able to test them on time.
> My initial intent was to provide timestamps for ADC readings, but I didn't
> realize an IIO timestamp channel would be needed (silly me).
> David, do you want a Suggested-by tag in this one?

I supposed that would make more sense than Reported-by: if we are
calling this a feature rather than a bug. Also,

Reviewed-by: David Lechner <dlechner@baylibre.com>

Re: [PATCH 3/4] iio: adc: ad4000: Use device specific timing for SPI transfers

[David Lechner]

On 11/14/24 5:51 PM, Marcelo Schmitt wrote:
> The SPI transfers for AD4020, AD4021, and AD4022 have slightly different
> timing specifications. Use device specific timing constraints to set SPI
> transfer parameters.
> 
> Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
> ---
>  drivers/iio/adc/ad4000.c | 50 ++++++++++++++++++++++++++++++++--------
>  1 file changed, 41 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/iio/adc/ad4000.c b/drivers/iio/adc/ad4000.c
> index 21731c4d31ee..68ac77494263 100644
> --- a/drivers/iio/adc/ad4000.c
> +++ b/drivers/iio/adc/ad4000.c
> @@ -35,10 +35,6 @@
>  
>  #define AD4000_SCALE_OPTIONS		2
>  
> -#define AD4000_TQUIET1_NS		190
> -#define AD4000_TQUIET2_NS		60
> -#define AD4000_TCONV_NS			320

We are removing 3 but only adding 2 in the struct below?

If one of these was unused, best to mention it in the commit message.

> -
>  #define __AD4000_DIFF_CHANNEL(_sign, _real_bits, _storage_bits, _reg_access)	\
>  {										\
>  	.type = IIO_VOLTAGE,							\
> @@ -122,10 +118,30 @@ static const int ad4000_gains[] = {
>  	454, 909, 1000, 1900,
>  };
>  
> +struct ad4000_time_spec {
> +	int t_conv_ns;
> +	int t_quiet2_ns;
> +};
> +
> +/*
> + * Same timing specifications for all of AD4000, AD4001, ..., AD4008, AD4010,
> + * ADAQ4001, and ADAQ4003.
> + */
> +static const struct ad4000_time_spec ad4000_t_spec = {
> +	.t_conv_ns = 320,
> +	.t_quiet2_ns = 60,
> +};
> +
> +static const struct ad4000_time_spec ad4020_t_spec = {
> +	.t_conv_ns = 350,
> +	.t_quiet2_ns = 60,
> +};

t_quiet2_ns is the same in both cases, so do we actually need to
add it here instead of using a common macro? Or if it is for future
differences, mention that in the commit message.

Re: [PATCH 4/4] iio: adc: ad4000: Add support for PulSAR devices

[David Lechner]

On 11/14/24 5:51 PM, Marcelo Schmitt wrote:
> The AD4000 series and the single-channel PulSAR series of devices have
> similar SPI transfer specifications and wiring configurations.
> Single-channel PulSAR devices are slower than AD4000, and don't have a
> configuration register. That taken into account, single-channel PulSARs can
> be supported by the ad4000 driver without any increase in code complexity.
> 
> Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
> ---
>  drivers/iio/adc/ad4000.c | 163 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 163 insertions(+)
> 
> diff --git a/drivers/iio/adc/ad4000.c b/drivers/iio/adc/ad4000.c
> index 68ac77494263..8e31b42534f5 100644
> --- a/drivers/iio/adc/ad4000.c
> +++ b/drivers/iio/adc/ad4000.c
> @@ -137,6 +137,41 @@ static const struct ad4000_time_spec ad4020_t_spec = {
>  	.t_quiet2_ns = 60,
>  };
>  
> +/* AD7983, AD7984 */
> +static const struct ad4000_time_spec ad7983_t_spec = {
> +	.t_conv_ns = 500,

I'm sure there are diffing opinions on this but I would prefer
an explicit .t_quiet2_ns = 0, so we know that it wasn't omitted
on accident. Or a group comment to say that these chips don't need
any quite time.

In any case...

Reviewed-by: David Lechner <dlechner@baylibre.com>

Re: [PATCH 1/4] dt-bindings: iio: adc: adi,ad4000: Add PulSAR

[Marcelo Schmitt]

On 11/15, David Lechner wrote:
> On 11/14/24 5:50 PM, Marcelo Schmitt wrote:
> > Extend the AD4000 series device tree documentation to also describe
> > PulSAR devices.
> > 
> > Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
> > ---
> >  .../bindings/iio/adc/adi,ad4000.yaml          | 115 +++++++++++++++++-
> >  1 file changed, 114 insertions(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml
> > index e413a9d8d2a2..35049071a9de 100644
> > --- a/Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml
> > +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml
> > @@ -19,6 +19,21 @@ description: |
> >      https://www.analog.com/media/en/technical-documentation/data-sheets/ad4020-4021-4022.pdf
> >      https://www.analog.com/media/en/technical-documentation/data-sheets/adaq4001.pdf
> >      https://www.analog.com/media/en/technical-documentation/data-sheets/adaq4003.pdf
> > +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7685.pdf
> > +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7686.pdf
> > +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7687.pdf
> > +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7688.pdf
> > +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7690.pdf
> > +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7691.pdf
> > +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7693.pdf
> > +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7694.pdf
> > +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7942.pdf
> > +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7946.pdf
> > +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7980.pdf
> > +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7982.pdf
> > +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7983.pdf
> > +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7984.pdf
> > +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7988-1_7988-5.pdf
> 
> It would be nice to sort these lowest number first.

Ack

> 
> >  
> >  $ref: /schemas/spi/spi-peripheral-props.yaml#
> >  
> > @@ -63,6 +78,38 @@ properties:
> >  
> >        - const: adi,adaq4003
> >  
> > +      - const: adi,ad7946
> > +      - items:
> > +          - enum:
> > +              - adi,ad7942
> > +          - const: adi,ad7946
> > +
> > +      - const: adi,ad7983
> > +      - items:
> > +          - enum:
> > +              - adi,ad7980
> > +              - adi,ad7988-5
> > +              - adi,ad7686
> > +              - adi,ad7685
> > +              - adi,ad7694
> > +              - adi,ad7988-1
> > +          - const: adi,ad7983
> > +
> > +      - const: adi,ad7688
> > +      - items:
> > +          - enum:
> > +              - adi,ad7693
> > +              - adi,ad7687
> > +          - const: adi,ad7688
> > +
> > +      - const: adi,ad7984
> > +      - items:
> > +          - enum:
> > +              - adi,ad7982
> > +              - adi,ad7690
> > +              - adi,ad7691
> > +          - const: adi,ad7984
> > +
> 
> IMHO, having fallbacks just makes the bindings harder to use and doesn't
> actually provide any useful benefit.
> 
Having fallbacks was a suggestion from a dt maintainer to the ad4000 series.
I assumed they would ask it for PulSAR too. Will wait a comment from a dt
maintainer to change it.

> And with this many chips, it can be easy to overlook a small difference
> in one chips, like ad7694 not having VIO pin, so is it really fallback
> compatible? Easier to just avoid the question and not have fallbacks.
> 
The absence of a VIO pin does not change how the driver handles the devices.
They are compatible from software perspective.

> >    reg:
> >      maxItems: 1
> >  
> > @@ -129,10 +176,76 @@ required:
> >    - compatible
> >    - reg
> >    - vdd-supply
> > -  - vio-supply
> >    - ref-supply
> >  
> >  allOf:
> > +  # AD7694 doesn't have a VIO pin
> 
> It sounds like using not: could make this if: a lot shorter.

Ack

> 
> Also, it looks like ad7983 doesn't have the pin either.

Ack

> 
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            enum:
> > +              - adi,ad4000
> > +              - adi,ad4001
> > +              - adi,ad4002
> > +              - adi,ad4003
> > +              - adi,ad4004
> > +              - adi,ad4005
> > +              - adi,ad4006
> > +              - adi,ad4007
> > +              - adi,ad4008
> > +              - adi,ad4010
> > +              - adi,ad4011
> > +              - adi,ad4020
> > +              - adi,ad4021
> > +              - adi,ad4022
> > +              - adi,adaq4001
> > +              - adi,adaq4003
> > +              - adi,ad7685
> > +              - adi,ad7686
> > +              - adi,ad7687
> > +              - adi,ad7688
> > +              - adi,ad7690
> > +              - adi,ad7691
> > +              - adi,ad7693
> > +              - adi,ad7942
> > +              - adi,ad7946
> > +              - adi,ad7980
> > +              - adi,ad7982
> > +              - adi,ad7983
> > +              - adi,ad7984
> > +              - adi,ad7988-1
> > +              - adi,ad7988-5
> > +    then:
> > +      required:
> > +        - vio-supply
> > +  # Single-channel PulSAR devices have SDI either tied to VIO, GND, or host CS.
> 
> To me, the more interesting thing to say here is that the sdi
> option is omitted because these chips don't have a programmable
> register.

Yes, that's correct. But the adi,sdi-pin property is about what is connected
to the SDI/MOSI pin so I kept the comment about hw connections only.
We could in theory connect SDI to host MOSI and set MOSI idle high (if the
controller supports that), but that is harder to describe.

> 
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            enum:
> > +              - adi,ad7685
> > +              - adi,ad7686
> > +              - adi,ad7687
> > +              - adi,ad7688
> > +              - adi,ad7690
> > +              - adi,ad7691
> > +              - adi,ad7693
> > +              - adi,ad7694
> > +              - adi,ad7942
> > +              - adi,ad7946
> > +              - adi,ad7980
> > +              - adi,ad7982
> > +              - adi,ad7983
> > +              - adi,ad7984
> > +              - adi,ad7988-1
> > +              - adi,ad7988-5
> > +    then:
> > +      properties:
> > +        adi,sdi-pin:
> > +          enum: [ high, low, cs ]
> > +          default: high
> 
> For the similar ad7944, Rob suggested that the default should be the equivalent
> of "cs" since that is most like "regular" SPI. So I think it makes sense do the
> same here. (The adi,spi-mode property in the ad7944 binding is named a bit
> different, single = high, chain = low and _property omitted_ (default) = cs)
Ack

> 
> >    # The configuration register can only be accessed if SDI is connected to MOSI
> >    - if:
> >        required:
> 

Re: [PATCH 3/4] iio: adc: ad4000: Use device specific timing for SPI transfers

[Marcelo Schmitt]

On 11/15, David Lechner wrote:
> On 11/14/24 5:51 PM, Marcelo Schmitt wrote:
> > The SPI transfers for AD4020, AD4021, and AD4022 have slightly different
> > timing specifications. Use device specific timing constraints to set SPI
> > transfer parameters.
> > 
> > Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
> > ---
> >  drivers/iio/adc/ad4000.c | 50 ++++++++++++++++++++++++++++++++--------
> >  1 file changed, 41 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/iio/adc/ad4000.c b/drivers/iio/adc/ad4000.c
> > index 21731c4d31ee..68ac77494263 100644
> > --- a/drivers/iio/adc/ad4000.c
> > +++ b/drivers/iio/adc/ad4000.c
> > @@ -35,10 +35,6 @@
> >  
> >  #define AD4000_SCALE_OPTIONS		2
> >  
> > -#define AD4000_TQUIET1_NS		190
> > -#define AD4000_TQUIET2_NS		60
> > -#define AD4000_TCONV_NS			320
> 
> We are removing 3 but only adding 2 in the struct below?
> 
> If one of these was unused, best to mention it in the commit message.

One of them was unused (AD4000_TQUIET1_NS IRCC).
Sure, will add a comment about it in the commit body.

> 
> > -
> >  #define __AD4000_DIFF_CHANNEL(_sign, _real_bits, _storage_bits, _reg_access)	\
> >  {										\
> >  	.type = IIO_VOLTAGE,							\
> > @@ -122,10 +118,30 @@ static const int ad4000_gains[] = {
> >  	454, 909, 1000, 1900,
> >  };
> >  
> > +struct ad4000_time_spec {
> > +	int t_conv_ns;
> > +	int t_quiet2_ns;
> > +};
> > +
> > +/*
> > + * Same timing specifications for all of AD4000, AD4001, ..., AD4008, AD4010,
> > + * ADAQ4001, and ADAQ4003.
> > + */
> > +static const struct ad4000_time_spec ad4000_t_spec = {
> > +	.t_conv_ns = 320,
> > +	.t_quiet2_ns = 60,
> > +};
> > +
> > +static const struct ad4000_time_spec ad4020_t_spec = {
> > +	.t_conv_ns = 350,
> > +	.t_quiet2_ns = 60,
> > +};
> 
> t_quiet2_ns is the same in both cases, so do we actually need to
> add it here instead of using a common macro? Or if it is for future
> differences, mention that in the commit message.

Okay, will add a macro for setting ad4000_time_spec. My plan is to also add a
t_quiet1_ns filed which will be needed for offloading support.
t_quiet1_ns will also differ between AD4000 and AD4020.

Re: [PATCH 4/4] iio: adc: ad4000: Add support for PulSAR devices

[Marcelo Schmitt]

On 11/15, David Lechner wrote:
> On 11/14/24 5:51 PM, Marcelo Schmitt wrote:
> > The AD4000 series and the single-channel PulSAR series of devices have
> > similar SPI transfer specifications and wiring configurations.
> > Single-channel PulSAR devices are slower than AD4000, and don't have a
> > configuration register. That taken into account, single-channel PulSARs can
> > be supported by the ad4000 driver without any increase in code complexity.
> > 
> > Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
> > ---
> >  drivers/iio/adc/ad4000.c | 163 +++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 163 insertions(+)
> > 
> > diff --git a/drivers/iio/adc/ad4000.c b/drivers/iio/adc/ad4000.c
> > index 68ac77494263..8e31b42534f5 100644
> > --- a/drivers/iio/adc/ad4000.c
> > +++ b/drivers/iio/adc/ad4000.c
> > @@ -137,6 +137,41 @@ static const struct ad4000_time_spec ad4020_t_spec = {
> >  	.t_quiet2_ns = 60,
> >  };
> >  
> > +/* AD7983, AD7984 */
> > +static const struct ad4000_time_spec ad7983_t_spec = {
> > +	.t_conv_ns = 500,
> 
> I'm sure there are diffing opinions on this but I would prefer
> an explicit .t_quiet2_ns = 0, so we know that it wasn't omitted
> on accident. Or a group comment to say that these chips don't need
> any quite time.
Ack, will set it with a macro.

> 
> In any case...
> 
> Reviewed-by: David Lechner <dlechner@baylibre.com>
> 
> 

Re: [PATCH 1/4] dt-bindings: iio: adc: adi,ad4000: Add PulSAR

[David Lechner]

On 11/18/24 5:24 AM, Marcelo Schmitt wrote:
> On 11/15, David Lechner wrote:
>> On 11/14/24 5:50 PM, Marcelo Schmitt wrote:
>>> Extend the AD4000 series device tree documentation to also describe
>>> PulSAR devices.
>>>
>>> Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
>>> ---

...

>>>  
>>>  $ref: /schemas/spi/spi-peripheral-props.yaml#
>>>  
>>> @@ -63,6 +78,38 @@ properties:
>>>  
>>>        - const: adi,adaq4003
>>>  
>>> +      - const: adi,ad7946
>>> +      - items:
>>> +          - enum:
>>> +              - adi,ad7942
>>> +          - const: adi,ad7946
>>> +
>>> +      - const: adi,ad7983
>>> +      - items:
>>> +          - enum:
>>> +              - adi,ad7980
>>> +              - adi,ad7988-5
>>> +              - adi,ad7686
>>> +              - adi,ad7685
>>> +              - adi,ad7694
>>> +              - adi,ad7988-1
>>> +          - const: adi,ad7983
>>> +
>>> +      - const: adi,ad7688
>>> +      - items:
>>> +          - enum:
>>> +              - adi,ad7693
>>> +              - adi,ad7687
>>> +          - const: adi,ad7688
>>> +
>>> +      - const: adi,ad7984
>>> +      - items:
>>> +          - enum:
>>> +              - adi,ad7982
>>> +              - adi,ad7690
>>> +              - adi,ad7691
>>> +          - const: adi,ad7984
>>> +
>>
>> IMHO, having fallbacks just makes the bindings harder to use and doesn't
>> actually provide any useful benefit.
>>
> Having fallbacks was a suggestion from a dt maintainer to the ad4000 series.
> I assumed they would ask it for PulSAR too. Will wait a comment from a dt
> maintainer to change it.
> 
>> And with this many chips, it can be easy to overlook a small difference
>> in one chips, like ad7694 not having VIO pin, so is it really fallback
>> compatible? Easier to just avoid the question and not have fallbacks.
>>
> The absence of a VIO pin does not change how the driver handles the devices.
> They are compatible from software perspective.
> 
OK. Another difference for consideration that I noticed is that on some chips,
the SDO line can generate a BUSY interrupt while others can't. Not sure if
that matters from the point of view of fallbacks or not.

Re: [PATCH 1/4] dt-bindings: iio: adc: adi,ad4000: Add PulSAR

[Marcelo Schmitt]

On 11/18, Marcelo Schmitt wrote:
> On 11/15, David Lechner wrote:
> > On 11/14/24 5:50 PM, Marcelo Schmitt wrote:
> > > Extend the AD4000 series device tree documentation to also describe
> > > PulSAR devices.
> > > 
> > > Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
> > > ---
> > >  .../bindings/iio/adc/adi,ad4000.yaml          | 115 +++++++++++++++++-
> > >  1 file changed, 114 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml
> > > index e413a9d8d2a2..35049071a9de 100644
> > > --- a/Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml
> > > +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml
> > > @@ -19,6 +19,21 @@ description: |
> > >      https://www.analog.com/media/en/technical-documentation/data-sheets/ad4020-4021-4022.pdf
> > >      https://www.analog.com/media/en/technical-documentation/data-sheets/adaq4001.pdf
> > >      https://www.analog.com/media/en/technical-documentation/data-sheets/adaq4003.pdf
> > > +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7685.pdf
> > > +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7686.pdf
> > > +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7687.pdf
> > > +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7688.pdf
> > > +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7690.pdf
> > > +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7691.pdf
> > > +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7693.pdf
> > > +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7694.pdf
> > > +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7942.pdf
> > > +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7946.pdf
> > > +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7980.pdf
> > > +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7982.pdf
> > > +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7983.pdf
> > > +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7984.pdf
> > > +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7988-1_7988-5.pdf
> > 
> > It would be nice to sort these lowest number first.
> 
> Ack
> 
Actually, I didn't get how I'm expected to sort those.
Isn't ad4000 < ad7685?
Or did you mean to put adaq at the end?

ad4000-4004-4008.pdf
...
ad4020-4021-4022.pdf
ad7685.pdf
...
ad7988-1_7988-5.pdf
adaq4001.pdf
adaq4003.pdf


[...]
> 
> > And with this many chips, it can be easy to overlook a small difference
> > in one chips, like ad7694 not having VIO pin, so is it really fallback
> > compatible? Easier to just avoid the question and not have fallbacks.
> > 
> The absence of a VIO pin does not change how the driver handles the devices.
> They are compatible from software perspective.
> 
[...]
> > >  
> > >  allOf:
> > > +  # AD7694 doesn't have a VIO pin
> > 
> > It sounds like using not: could make this if: a lot shorter.
> 
> Ack
> 
> > 
> > Also, it looks like ad7983 doesn't have the pin either.
> 
> Ack

I forgot the ad4000 driver fails if VIO is not provided so I was wrong when I
said AD7694 was software compatible with the other ADCs.
I see now AD7694 also doesn't have SDI pin.
Aside from the VIO and SDI pins, AD7694 is similar to AD7685 both being 16-bit
precision 250kSPS pseudo-differential ADCs.
The AD7683 part you mentioned is similar to AD7988-1, both 16-bit
pseudo-differential 100kSPS.
To avoid complicating things, I'm dropping support for AD7694.
AD7685 and AD7988-1 are the parts with features similar to AD7694 and AD7683,
respectively.

Re: [PATCH 1/4] dt-bindings: iio: adc: adi,ad4000: Add PulSAR

[David Lechner]

On 11/18/24 12:25 PM, Marcelo Schmitt wrote:
> On 11/18, Marcelo Schmitt wrote:
>> On 11/15, David Lechner wrote:
>>> On 11/14/24 5:50 PM, Marcelo Schmitt wrote:
>>>> Extend the AD4000 series device tree documentation to also describe
>>>> PulSAR devices.
>>>>
>>>> Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
>>>> ---
>>>>  .../bindings/iio/adc/adi,ad4000.yaml          | 115 +++++++++++++++++-
>>>>  1 file changed, 114 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml
>>>> index e413a9d8d2a2..35049071a9de 100644
>>>> --- a/Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml
>>>> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml
>>>> @@ -19,6 +19,21 @@ description: |
>>>>      https://www.analog.com/media/en/technical-documentation/data-sheets/ad4020-4021-4022.pdf
>>>>      https://www.analog.com/media/en/technical-documentation/data-sheets/adaq4001.pdf
>>>>      https://www.analog.com/media/en/technical-documentation/data-sheets/adaq4003.pdf
>>>> +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7685.pdf
>>>> +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7686.pdf
>>>> +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7687.pdf
>>>> +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7688.pdf
>>>> +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7690.pdf
>>>> +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7691.pdf
>>>> +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7693.pdf
>>>> +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7694.pdf
>>>> +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7942.pdf
>>>> +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7946.pdf
>>>> +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7980.pdf
>>>> +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7982.pdf
>>>> +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7983.pdf
>>>> +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7984.pdf
>>>> +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7988-1_7988-5.pdf
>>>
>>> It would be nice to sort these lowest number first.
>>
>> Ack
>>
> Actually, I didn't get how I'm expected to sort those.
> Isn't ad4000 < ad7685?
> Or did you mean to put adaq at the end?
> 
> ad4000-4004-4008.pdf
> ...
> ad4020-4021-4022.pdf
> ad7685.pdf
> ...
> ad7988-1_7988-5.pdf
> adaq4001.pdf
> adaq4003.pdf
> 
I think all of the 6s, 8s and 9s were playing tricks on my brain when I wrote that.
Looking again now, it looks fine to me. Sorry for the noise.

Re: [PATCH 1/4] dt-bindings: iio: adc: adi,ad4000: Add PulSAR

[Jonathan Cameron]

On Mon, 18 Nov 2024 15:25:58 -0300
Marcelo Schmitt <marcelo.schmitt1@gmail.com> wrote:

> On 11/18, Marcelo Schmitt wrote:
> > On 11/15, David Lechner wrote:  
> > > On 11/14/24 5:50 PM, Marcelo Schmitt wrote:  
> > > > Extend the AD4000 series device tree documentation to also describe
> > > > PulSAR devices.
> > > > 
> > > > Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
> > > > ---
> > > >  .../bindings/iio/adc/adi,ad4000.yaml          | 115 +++++++++++++++++-
> > > >  1 file changed, 114 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml
> > > > index e413a9d8d2a2..35049071a9de 100644
> > > > --- a/Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml
> > > > +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml
> > > > @@ -19,6 +19,21 @@ description: |
> > > >      https://www.analog.com/media/en/technical-documentation/data-sheets/ad4020-4021-4022.pdf
> > > >      https://www.analog.com/media/en/technical-documentation/data-sheets/adaq4001.pdf
> > > >      https://www.analog.com/media/en/technical-documentation/data-sheets/adaq4003.pdf
> > > > +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7685.pdf
> > > > +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7686.pdf
> > > > +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7687.pdf
> > > > +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7688.pdf
> > > > +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7690.pdf
> > > > +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7691.pdf
> > > > +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7693.pdf
> > > > +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7694.pdf
> > > > +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7942.pdf
> > > > +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7946.pdf
> > > > +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7980.pdf
> > > > +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7982.pdf
> > > > +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7983.pdf
> > > > +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7984.pdf
> > > > +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7988-1_7988-5.pdf  
> > > 
> > > It would be nice to sort these lowest number first.  
> > 
> > Ack
> >   
> Actually, I didn't get how I'm expected to sort those.
> Isn't ad4000 < ad7685?
> Or did you mean to put adaq at the end?
> 
> ad4000-4004-4008.pdf
> ...
> ad4020-4021-4022.pdf
> ad7685.pdf
> ...
> ad7988-1_7988-5.pdf
> adaq4001.pdf
> adaq4003.pdf
> 
> 
> [...]
> >   
> > > And with this many chips, it can be easy to overlook a small difference
> > > in one chips, like ad7694 not having VIO pin, so is it really fallback
> > > compatible? Easier to just avoid the question and not have fallbacks.
> > >   
> > The absence of a VIO pin does not change how the driver handles the devices.
> > They are compatible from software perspective.
> >   
> [...]
> > > >  
> > > >  allOf:
> > > > +  # AD7694 doesn't have a VIO pin  
> > > 
> > > It sounds like using not: could make this if: a lot shorter.  
> > 
> > Ack
> >   
> > > 
> > > Also, it looks like ad7983 doesn't have the pin either.  
> > 
> > Ack  
> 
> I forgot the ad4000 driver fails if VIO is not provided so I was wrong when I
> said AD7694 was software compatible with the other ADCs.
> I see now AD7694 also doesn't have SDI pin.
> Aside from the VIO and SDI pins, AD7694 is similar to AD7685 both being 16-bit
> precision 250kSPS pseudo-differential ADCs.
> The AD7683 part you mentioned is similar to AD7988-1, both 16-bit
> pseudo-differential 100kSPS.
> To avoid complicating things, I'm dropping support for AD7694.
> AD7685 and AD7988-1 are the parts with features similar to AD7694 and AD7683,
> respectively.

General rule is that fallbacks only work if a part is a strict subset
functionality wise or identical.   If we get it wrong, then that's fine
as the more specific compatible allows us to fix things up even for
dt files that are in products, but if we know there are incompatibilities
or things we want to check for in the binding then don't do fallbacks in
the first palce.   E.g. if there is a pin that is only on a fancy device
then even if it will work fine with a driver binding to the simpler one, we
may want to not use a fallback because we want the dt-binding to verify
the pin is only provided for the more sophisticated device.

Gets fuzzy however in some cases!

Usually what DT maintainers are looking for is a clear statement of how
the devices are different.

Jonathan

Re: [PATCH 1/4] dt-bindings: iio: adc: adi,ad4000: Add PulSAR

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 304 bytes --]

On Sun, Nov 24, 2024 at 01:11:47PM +0000, Jonathan Cameron wrote:
> Usually what DT maintainers are looking for is a clear statement of how
> the devices are different.

Ye, tell us rather than make us open the datasheets or have to ask
questions to figure out whether or not what you've got is correct.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]