[PATCH v4 00/17] iio: adc: ad7768-1: Add features, improvements, and fixes

[PATCH v4 00/17] iio: adc: ad7768-1: Add features, improvements, and fixes

[Jonathan Santos]

This patch series introduces some new features, improvements,
and fixes for the AD7768-1 ADC driver. 

The goal is to support all key functionalities listed in the device
datasheet, including filter mode selection, common mode voltage output
configuration and GPIO support. Additionally, this includes fixes 
for SPI communication and for IIO interface, and also code improvements
to enhance maintainability and readability.

---
Changes in v4:
* Added missing `select REGMAP_SPI` and `select REGULATOR` to the device's Kconfig.
* VCM output regulator property renamed.
* Added direct mode conditional locks to regulator controller callbacks.
* Renamed regulator controller.
* Created helper function to precalculate the sampling frequency table and avoid
  race conditions.
* Link to v3: https://lore.kernel.org/linux-iio/cover.1739368121.git.Jonathan.Santos@analog.com/T/#t

Changes in v3:
* Fixed irregular or missing SoBs.
* Moved MOSI idle state patch to the start of the patch, as the other fix.
* fixed dt-binding errors.
* Trigger-sources is handled in a different way, as an alternative to sync-in-gpio.
  (this way we avoid breaking old applications).
* VCM output is controlled by the regulator framework.
* Added a second regmap for 24-bit register values.
* Add new preparatory patch replacing the manual attribute declarations for
  the read_avail from struct iio_info.
* included sinc3+rej60 filter type.
* Addressed review comments, see individual pacthes.
* Link to v2: https://lore.kernel.org/linux-iio/cover.1737985435.git.Jonathan.Santos@analog.com/T/#u

Changes in v2:
* Removed synchronization over SPI property and replaced it for trigger-sources.
* Added GPIO controller documentation.
* VCM output control changed from an IIO attribute to a devicetree property (static value).
* Converted driver to use regmap and dropped spi_read_reg and spi_write_reg pacthes.
* replaced decimation_rate attribute for oversampling_ratio and dropped device specific documentation patch.
* Added low pass -3dB cutoff attribute.
* Addressed review comments, see individual pacthes.
* Link to v1: https://lore.kernel.org/linux-iio/cover.1736201898.git.Jonathan.Santos@analog.com/T/#t
---

Jonathan Santos (13):
  iio: adc: ad7768-1: set MOSI idle state to prevent accidental reset
  dt-bindings: iio: adc: ad7768-1: add trigger-sources property
  dt-bindings: iio: adc: ad7768-1: Document GPIO controller
  dt-bindings: iio: adc: ad7768-1: document regulator provider property
  Documentation: ABI: add wideband filter type to  sysfs-bus-iio
  iio: adc: ad7768-1: remove unnecessary locking
  iio: adc: ad7768-1: convert driver to use regmap
  iio: adc: ad7768-1: add regulator to control VCM output
  iio: adc: ad7768-1: add multiple scan types to support 16-bits mode
  iio: adc: ad7768-1: add support for Synchronization over SPI
  iio: adc: ad7768-1: replace manual attribute declaration
  iio: adc: ad7768-1: add filter type and oversampling ratio attributes
  iio: adc: ad7768-1: add low pass -3dB cutoff attribute

Sergiu Cuciurean (4):
  iio: adc: ad7768-1: Fix conversion result sign
  iio: adc: ad7768-1: Add reset gpio
  iio: adc: ad7768-1: Move buffer allocation to a separate function
  iio: adc: ad7768-1: Add GPIO controller support

 Documentation/ABI/testing/sysfs-bus-iio       |    2 +
 .../bindings/iio/adc/adi,ad7768-1.yaml        |   59 +-
 drivers/iio/adc/Kconfig                       |    2 +
 drivers/iio/adc/ad7768-1.c                    | 1116 ++++++++++++++---
 4 files changed, 999 insertions(+), 180 deletions(-)


base-commit: 5de07b8a24cf44cdb78adeab790704bf577c2c1d
prerequisite-patch-id: 8b531bca46f7c7ea1c0f6d232d162fd05fda52f7
-- 
2.34.1

[PATCH v4 01/17] iio: adc: ad7768-1: Fix conversion result sign

[Jonathan Santos]

From: Sergiu Cuciurean <sergiu.cuciurean@analog.com>

The ad7768-1 ADC output code is two's complement, meaning that the voltage
conversion result is a signed value.. Since the value is a 24 bit one,
stored in a 32 bit variable, the sign should be extended in order to get
the correct representation.

Also the channel description has been updated to signed representation,
to match the ADC specifications.

Fixes: a5f8c7da3dbe ("iio: adc: Add AD7768-1 ADC basic support")
Reviewed-by: David Lechner <dlechner@baylibre.com>
Reviewed-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
Signed-off-by: Sergiu Cuciurean <sergiu.cuciurean@analog.com>
Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com>
---
v4 Changes:
* none.

v3 Changes:
* Added missing SoB.

v2 Changes:
* Patch moved to the start of the patch series.
---
 drivers/iio/adc/ad7768-1.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/adc/ad7768-1.c b/drivers/iio/adc/ad7768-1.c
index 113703fb7245..c3cf04311c40 100644
--- a/drivers/iio/adc/ad7768-1.c
+++ b/drivers/iio/adc/ad7768-1.c
@@ -142,7 +142,7 @@ static const struct iio_chan_spec ad7768_channels[] = {
 		.channel = 0,
 		.scan_index = 0,
 		.scan_type = {
-			.sign = 'u',
+			.sign = 's',
 			.realbits = 24,
 			.storagebits = 32,
 			.shift = 8,
@@ -371,7 +371,7 @@ static int ad7768_read_raw(struct iio_dev *indio_dev,
 
 		ret = ad7768_scan_direct(indio_dev);
 		if (ret >= 0)
-			*val = ret;
+			*val = sign_extend32(ret, chan->scan_type.realbits - 1);
 
 		iio_device_release_direct_mode(indio_dev);
 		if (ret < 0)
-- 
2.34.1

[PATCH v4 02/17] iio: adc: ad7768-1: set MOSI idle state to prevent accidental reset

[Jonathan Santos]

Datasheet recommends Setting the MOSI idle state to high in order to
prevent accidental reset of the device when SCLK is free running.
This happens when the controller clocks out a 1 followed by 63 zeros
while the CS is held low.

Check if SPI controller supports SPI_MOSI_IDLE_HIGH flag and set it.

Fixes: a5f8c7da3dbe ("iio: adc: Add AD7768-1 ADC basic support")
Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com>
---
v4 Changes:
* None.

v3 Changes:
* Patch moved closer to the start of the patch set.

v2 Changes:
* Only setup SPI_MOSI_IDLE_HIGH flag if the controller supports it, otherwise the driver
  continues the same. I realized that using bits_per_word does not avoid the problem that
  MOSI idle state is trying to solve. If the controller drives the MOSI low between bytes
  during a transfer, nothing happens.
---
 drivers/iio/adc/ad7768-1.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/iio/adc/ad7768-1.c b/drivers/iio/adc/ad7768-1.c
index c3cf04311c40..2e2d50ccb744 100644
--- a/drivers/iio/adc/ad7768-1.c
+++ b/drivers/iio/adc/ad7768-1.c
@@ -574,6 +574,21 @@ static int ad7768_probe(struct spi_device *spi)
 		return -ENOMEM;
 
 	st = iio_priv(indio_dev);
+	/*
+	 * Datasheet recommends SDI line to be kept high when data is not being
+	 * clocked out of the controller and the spi clock is free running,
+	 * to prevent accidental reset.
+	 * Since many controllers do not support the SPI_MOSI_IDLE_HIGH flag
+	 * yet, only request the MOSI idle state to enable if the controller
+	 * supports it.
+	 */
+	if (spi->controller->mode_bits & SPI_MOSI_IDLE_HIGH) {
+		spi->mode |= SPI_MOSI_IDLE_HIGH;
+		ret = spi_setup(spi);
+		if (ret < 0)
+			return ret;
+	}
+
 	st->spi = spi;
 
 	st->vref = devm_regulator_get(&spi->dev, "vref");
-- 
2.34.1

[PATCH v4 03/17] dt-bindings: iio: adc: ad7768-1: add trigger-sources property

[Jonathan Santos]

In addition to GPIO synchronization, The AD7768-1 also supports
synchronization over SPI, which use is recommended when the GPIO
cannot provide a pulse synchronous with the base MCLK signal. It
consists of looping back the SYNC_OUT to the SYNC_IN pin and send
a command via SPI to trigger the synchronization.

Add a new trigger-sources property to enable synchronization over SPI
and future multiple devices support. This property references the
main device (or trigger provider) responsible for generating the
SYNC_OUT pulse to drive the SYNC_IN of device.

While at it, add description to the interrupts property.

Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com>
---
v4 Changes:
* none

v3 Changes:
* Fixed dt-bindings errors.
* Trigger-source is set as an alternative to sync-in-gpios, so we
  don't break the previous ABI.
* increased maxItems from trigger-sources to 2.

v2 Changes:
* Patch added as replacement for adi,sync-in-spi patch.
* addressed the request for a description to interrupts property.
---
 .../bindings/iio/adc/adi,ad7768-1.yaml        | 28 +++++++++++++++++--
 1 file changed, 25 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml
index 3ce59d4d065f..4bcc9e20fab9 100644
--- a/Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml
+++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml
@@ -26,7 +26,19 @@ properties:
   clock-names:
     const: mclk
 
+  trigger-sources:
+    description:
+      Specifies the device responsible for driving the synchronization pin,
+      as an alternative to adi,sync-in-gpios. If the own device node is
+      referenced, The synchronization over SPI is enabled and the SYNC_OUT
+      output will drive the SYNC_IN pin.
+    maxItems: 2
+
   interrupts:
+    description:
+      Specifies the interrupt line associated with the ADC. This refers
+      to the DRDY (Data Ready) pin, which signals when conversion results are
+      available.
     maxItems: 1
 
   '#address-cells':
@@ -57,6 +69,9 @@ properties:
   "#io-channel-cells":
     const: 1
 
+  "#trigger-source-cells":
+    const: 0
+
 required:
   - compatible
   - reg
@@ -65,7 +80,6 @@ required:
   - vref-supply
   - spi-cpol
   - spi-cpha
-  - adi,sync-in-gpios
 
 patternProperties:
   "^channel@([0-9]|1[0-5])$":
@@ -89,6 +103,13 @@ patternProperties:
 allOf:
   - $ref: /schemas/spi/spi-peripheral-props.yaml#
 
+  - oneOf:
+      - required:
+          - trigger-sources
+          - "#trigger-source-cells"
+      - required:
+          - adi,sync-in-gpios
+
 unevaluatedProperties: false
 
 examples:
@@ -99,7 +120,7 @@ examples:
         #address-cells = <1>;
         #size-cells = <0>;
 
-        adc@0 {
+        adc0: adc@0 {
             compatible = "adi,ad7768-1";
             reg = <0>;
             spi-max-frequency = <2000000>;
@@ -108,7 +129,8 @@ examples:
             vref-supply = <&adc_vref>;
             interrupts = <25 IRQ_TYPE_EDGE_RISING>;
             interrupt-parent = <&gpio>;
-            adi,sync-in-gpios = <&gpio 22 GPIO_ACTIVE_LOW>;
+            trigger-sources = <&adc0 0>;
+            #trigger-source-cells = <0>;
             reset-gpios = <&gpio 27 GPIO_ACTIVE_LOW>;
             clocks = <&ad7768_mclk>;
             clock-names = "mclk";
-- 
2.34.1

[PATCH v4 04/17] dt-bindings: iio: adc: ad7768-1: Document GPIO controller

[Jonathan Santos]

The AD7768-1 ADC exports four bidirectional GPIOs accessible
via register map.

Document GPIO properties necessary to enable GPIO controller for this
device.

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com>
---
v4 Changes:
* none.

v3 Changes:
* none.

v2 Changes:
* New patch in v2.
---
 .../devicetree/bindings/iio/adc/adi,ad7768-1.yaml      | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml
index 4bcc9e20fab9..e2f9782b5fc8 100644
--- a/Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml
+++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml
@@ -72,6 +72,14 @@ properties:
   "#trigger-source-cells":
     const: 0
 
+  gpio-controller: true
+
+  "#gpio-cells":
+    const: 2
+    description: |
+      The first cell is for the GPIO number: 0 to 3.
+      The second cell takes standard GPIO flags.
+
 required:
   - compatible
   - reg
@@ -126,6 +134,8 @@ examples:
             spi-max-frequency = <2000000>;
             spi-cpol;
             spi-cpha;
+            gpio-controller;
+            #gpio-cells = <2>;
             vref-supply = <&adc_vref>;
             interrupts = <25 IRQ_TYPE_EDGE_RISING>;
             interrupt-parent = <&gpio>;
-- 
2.34.1

[PATCH v4 05/17] dt-bindings: iio: adc: ad7768-1: document regulator provider property

[Jonathan Santos]

The AD7768-1 provides a buffered common-mode voltage output
on the VCM pin that can be used to bias analog input signals.

Add regulators property to enable the use of the VCM output,
referenced here as vcm-output, by any other device.

Acked-by: Conor Dooley <conor.dooley@microchip.com>
Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com>
---
v4 Changes:
* replace "vcm_output" property name for "vcm-output". 

v3 Changes:
* VCM is now provided as a regulator within the device, instead of a 
  custom property.

v2 Changes:
* New patch in v2.
---
 .../bindings/iio/adc/adi,ad7768-1.yaml        | 21 +++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml
index e2f9782b5fc8..12358ea9138a 100644
--- a/Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml
+++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml
@@ -59,6 +59,19 @@ properties:
       in any way, for example if the filter decimation rate changes.
       As the line is active low, it should be marked GPIO_ACTIVE_LOW.
 
+  regulators:
+    type: object
+    description:
+      list of regulators provided by this controller.
+
+    properties:
+      vcm-output:
+        $ref: /schemas/regulator/regulator.yaml#
+        type: object
+        unevaluatedProperties: false
+
+    additionalProperties: false
+
   reset-gpios:
     maxItems: 1
 
@@ -152,6 +165,14 @@ examples:
                 reg = <0>;
                 label = "channel_0";
             };
+
+            regulators {
+              vcm_reg: vcm-output {
+                regulator-name = "ad7768-1-vcm";
+                regulator-min-microvolt = <900000>;
+                regulator-max-microvolt = <2500000>;
+              };
+            };
         };
     };
 ...
-- 
2.34.1

[PATCH v4 06/17] Documentation: ABI: add wideband filter type to sysfs-bus-iio

[Jonathan Santos]

The Wideband Low Ripple filter is used for AD7768-1 Driver.
Document wideband filter option into filter_type_available
attribute.

Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com>
---
v4 Changes:
* None.

v3 Changes:
* None, since we still did not agree on a better name for this filter type.

v2 Changes:
* Removed FIR mentions.
---
 Documentation/ABI/testing/sysfs-bus-iio | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
index f83bd6829285..9b879e7732cd 100644
--- a/Documentation/ABI/testing/sysfs-bus-iio
+++ b/Documentation/ABI/testing/sysfs-bus-iio
@@ -2291,6 +2291,8 @@ Description:
 		* "sinc3+pf2" - Sinc3 + device specific Post Filter 2.
 		* "sinc3+pf3" - Sinc3 + device specific Post Filter 3.
 		* "sinc3+pf4" - Sinc3 + device specific Post Filter 4.
+		* "wideband" - filter with wideband low ripple passband
+		  and sharp transition band.
 
 What:		/sys/.../events/in_proximity_thresh_either_runningperiod
 KernelVersion:	6.6
-- 
2.34.1

[PATCH v4 07/17] iio: adc: ad7768-1: remove unnecessary locking

[Jonathan Santos]

The current locking is only preventing a triggered buffer Transfer and a
debugfs register access from happening at the same time. If a register
access happens during a buffered read, the action is doomed to fail anyway,
since we need to write a magic value to exit continuous read mode.

Remove locking from the trigger handler and use
iio_device_claim_direct_mode() instead in the register access function.

Reviewed-by: David Lechner <dlechner@baylibre.com>
Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com>
---
v4 Changes:
* None.

v3 Changes:
* Also removed the mutex_init and lock variable.

v2 Changes:
* New patch in v2. It replaces the guard(mutex) patch. 
---
 drivers/iio/adc/ad7768-1.c | 21 +++++++++------------
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/drivers/iio/adc/ad7768-1.c b/drivers/iio/adc/ad7768-1.c
index 2e2d50ccb744..f5509a0a36ab 100644
--- a/drivers/iio/adc/ad7768-1.c
+++ b/drivers/iio/adc/ad7768-1.c
@@ -154,7 +154,6 @@ static const struct iio_chan_spec ad7768_channels[] = {
 struct ad7768_state {
 	struct spi_device *spi;
 	struct regulator *vref;
-	struct mutex lock;
 	struct clk *mclk;
 	unsigned int mclk_freq;
 	unsigned int samp_freq;
@@ -256,18 +255,21 @@ static int ad7768_reg_access(struct iio_dev *indio_dev,
 	struct ad7768_state *st = iio_priv(indio_dev);
 	int ret;
 
-	mutex_lock(&st->lock);
+	ret = iio_device_claim_direct_mode(indio_dev);
+	if (ret)
+		return ret;
+
 	if (readval) {
 		ret = ad7768_spi_reg_read(st, reg, 1);
 		if (ret < 0)
-			goto err_unlock;
+			goto err_release;
 		*readval = ret;
 		ret = 0;
 	} else {
 		ret = ad7768_spi_reg_write(st, reg, writeval);
 	}
-err_unlock:
-	mutex_unlock(&st->lock);
+err_release:
+	iio_device_release_direct_mode(indio_dev);
 
 	return ret;
 }
@@ -471,18 +473,15 @@ static irqreturn_t ad7768_trigger_handler(int irq, void *p)
 	struct ad7768_state *st = iio_priv(indio_dev);
 	int ret;
 
-	mutex_lock(&st->lock);
-
 	ret = spi_read(st->spi, &st->data.scan.chan, 3);
 	if (ret < 0)
-		goto err_unlock;
+		goto out;
 
 	iio_push_to_buffers_with_timestamp(indio_dev, &st->data.scan,
 					   iio_get_time_ns(indio_dev));
 
-err_unlock:
+out:
 	iio_trigger_notify_done(indio_dev->trig);
-	mutex_unlock(&st->lock);
 
 	return IRQ_HANDLED;
 }
@@ -611,8 +610,6 @@ static int ad7768_probe(struct spi_device *spi)
 
 	st->mclk_freq = clk_get_rate(st->mclk);
 
-	mutex_init(&st->lock);
-
 	indio_dev->channels = ad7768_channels;
 	indio_dev->num_channels = ARRAY_SIZE(ad7768_channels);
 	indio_dev->name = spi_get_device_id(spi)->name;
-- 
2.34.1

[PATCH v4 08/17] iio: adc: ad7768-1: convert driver to use regmap

[Jonathan Santos]

Convert the AD7768-1 driver to use the regmap API for register
access. This change simplifies and standardizes register interactions,
reducing code duplication and improving maintainability.

Create two regmap configurations, one for 8-bit register values and
other for 24-bit register values.

Since we are using regmap now, define the remaining registers from 0x32
to 0x34.

Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com>
---
v4 Changes:
* Add `REGMAP24` to the register macros with 24-bit value.
* Add `select REGMAP_SPI` line to the Kconfig.

v3 Changes:
* Included a second register map for the 24-bit register values.
* Added register tables to separate the 24-bit from the 8-bit values.

v2 Changes:
* New patch in v2.
---
 drivers/iio/adc/Kconfig    |   1 +
 drivers/iio/adc/ad7768-1.c | 151 +++++++++++++++++++++++++------------
 2 files changed, 104 insertions(+), 48 deletions(-)

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 849c90203071..a2fdb7e03a66 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -277,6 +277,7 @@ config AD7766
 config AD7768_1
 	tristate "Analog Devices AD7768-1 ADC driver"
 	depends on SPI
+	select REGMAP_SPI
 	select IIO_BUFFER
 	select IIO_TRIGGER
 	select IIO_TRIGGERED_BUFFER
diff --git a/drivers/iio/adc/ad7768-1.c b/drivers/iio/adc/ad7768-1.c
index f5509a0a36ab..04a26e5b7d5c 100644
--- a/drivers/iio/adc/ad7768-1.c
+++ b/drivers/iio/adc/ad7768-1.c
@@ -12,6 +12,7 @@
 #include <linux/gpio/consumer.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
 #include <linux/sysfs.h>
 #include <linux/spi/spi.h>
@@ -53,12 +54,15 @@
 #define AD7768_REG_SPI_DIAG_ENABLE	0x28
 #define AD7768_REG_ADC_DIAG_ENABLE	0x29
 #define AD7768_REG_DIG_DIAG_ENABLE	0x2A
-#define AD7768_REG_ADC_DATA		0x2C
+#define AD7768_REG24_ADC_DATA		0x2C
 #define AD7768_REG_MASTER_STATUS	0x2D
 #define AD7768_REG_SPI_DIAG_STATUS	0x2E
 #define AD7768_REG_ADC_DIAG_STATUS	0x2F
 #define AD7768_REG_DIG_DIAG_STATUS	0x30
 #define AD7768_REG_MCLK_COUNTER		0x31
+#define AD7768_REG_COEFF_CONTROL	0x32
+#define AD7768_REG24_COEFF_DATA		0x33
+#define AD7768_REG_ACCESS_KEY		0x34
 
 /* AD7768_REG_POWER_CLOCK */
 #define AD7768_PWR_MCLK_DIV_MSK		GENMASK(5, 4)
@@ -153,6 +157,8 @@ static const struct iio_chan_spec ad7768_channels[] = {
 
 struct ad7768_state {
 	struct spi_device *spi;
+	struct regmap *regmap;
+	struct regmap *regmap24;
 	struct regulator *vref;
 	struct clk *mclk;
 	unsigned int mclk_freq;
@@ -175,46 +181,76 @@ struct ad7768_state {
 	} data __aligned(IIO_DMA_MINALIGN);
 };
 
-static int ad7768_spi_reg_read(struct ad7768_state *st, unsigned int addr,
-			       unsigned int len)
-{
-	unsigned int shift;
-	int ret;
+static const struct regmap_range ad7768_regmap_rd_ranges[] = {
+	regmap_reg_range(AD7768_REG_CHIP_TYPE, AD7768_REG_DIG_DIAG_ENABLE),
+	regmap_reg_range(AD7768_REG_MASTER_STATUS, AD7768_REG_COEFF_CONTROL),
+	regmap_reg_range(AD7768_REG_ACCESS_KEY, AD7768_REG_ACCESS_KEY),
+};
 
-	shift = 32 - (8 * len);
-	st->data.d8[0] = AD7768_RD_FLAG_MSK(addr);
+static const struct regmap_access_table ad7768_regmap_rd_table = {
+	.yes_ranges = ad7768_regmap_rd_ranges,
+	.n_yes_ranges = ARRAY_SIZE(ad7768_regmap_rd_ranges),
+};
 
-	ret = spi_write_then_read(st->spi, st->data.d8, 1,
-				  &st->data.d32, len);
-	if (ret < 0)
-		return ret;
+static const struct regmap_range ad7768_regmap_wr_ranges[] = {
+	regmap_reg_range(AD7768_REG_SCRATCH_PAD, AD7768_REG_SCRATCH_PAD),
+	regmap_reg_range(AD7768_REG_INTERFACE_FORMAT, AD7768_REG_GPIO_WRITE),
+	regmap_reg_range(AD7768_REG_OFFSET_HI, AD7768_REG_DIG_DIAG_ENABLE),
+	regmap_reg_range(AD7768_REG_SPI_DIAG_STATUS, AD7768_REG_SPI_DIAG_STATUS),
+	regmap_reg_range(AD7768_REG_COEFF_CONTROL, AD7768_REG_COEFF_CONTROL),
+};
 
-	return (be32_to_cpu(st->data.d32) >> shift);
-}
+static const struct regmap_access_table ad7768_regmap_wr_table = {
+	.yes_ranges = ad7768_regmap_wr_ranges,
+	.n_yes_ranges = ARRAY_SIZE(ad7768_regmap_wr_ranges),
+};
 
-static int ad7768_spi_reg_write(struct ad7768_state *st,
-				unsigned int addr,
-				unsigned int val)
-{
-	st->data.d8[0] = AD7768_WR_FLAG_MSK(addr);
-	st->data.d8[1] = val & 0xFF;
+static const struct regmap_config ad7768_regmap_config = {
+	.name = "ad7768-1-8",
+	.reg_bits = 8,
+	.val_bits = 8,
+	.read_flag_mask = BIT(6),
+	.rd_table = &ad7768_regmap_rd_table,
+	.wr_table = &ad7768_regmap_wr_table,
+	.max_register = AD7768_REG_ACCESS_KEY,
+	.use_single_write = true,
+	.use_single_read = true,
+};
 
-	return spi_write(st->spi, st->data.d8, 2);
-}
+static const struct regmap_range ad7768_regmap24_rd_ranges[] = {
+	regmap_reg_range(AD7768_REG24_ADC_DATA, AD7768_REG24_ADC_DATA),
+	regmap_reg_range(AD7768_REG24_COEFF_DATA, AD7768_REG24_COEFF_DATA),
+};
 
-static int ad7768_set_mode(struct ad7768_state *st,
-			   enum ad7768_conv_mode mode)
-{
-	int regval;
+static const struct regmap_access_table ad7768_regmap24_rd_table = {
+	.yes_ranges = ad7768_regmap24_rd_ranges,
+	.n_yes_ranges = ARRAY_SIZE(ad7768_regmap24_rd_ranges),
+};
 
-	regval = ad7768_spi_reg_read(st, AD7768_REG_CONVERSION, 1);
-	if (regval < 0)
-		return regval;
+static const struct regmap_range ad7768_regmap24_wr_ranges[] = {
+	regmap_reg_range(AD7768_REG24_COEFF_DATA, AD7768_REG24_COEFF_DATA),
+};
 
-	regval &= ~AD7768_CONV_MODE_MSK;
-	regval |= AD7768_CONV_MODE(mode);
+static const struct regmap_access_table ad7768_regmap24_wr_table = {
+	.yes_ranges = ad7768_regmap24_wr_ranges,
+	.n_yes_ranges = ARRAY_SIZE(ad7768_regmap24_wr_ranges),
+};
+
+static const struct regmap_config ad7768_regmap24_config = {
+	.name = "ad7768-1-24",
+	.reg_bits = 8,
+	.val_bits = 24,
+	.read_flag_mask = BIT(6),
+	.rd_table = &ad7768_regmap24_rd_table,
+	.wr_table = &ad7768_regmap24_wr_table,
+	.max_register = AD7768_REG24_COEFF_DATA,
+};
 
-	return ad7768_spi_reg_write(st, AD7768_REG_CONVERSION, regval);
+static int ad7768_set_mode(struct ad7768_state *st,
+			   enum ad7768_conv_mode mode)
+{
+	return regmap_update_bits(st->regmap, AD7768_REG_CONVERSION,
+				 AD7768_CONV_MODE_MSK, AD7768_CONV_MODE(mode));
 }
 
 static int ad7768_scan_direct(struct iio_dev *indio_dev)
@@ -233,9 +269,10 @@ static int ad7768_scan_direct(struct iio_dev *indio_dev)
 	if (!ret)
 		return -ETIMEDOUT;
 
-	readval = ad7768_spi_reg_read(st, AD7768_REG_ADC_DATA, 3);
-	if (readval < 0)
-		return readval;
+	ret = regmap_read(st->regmap24, AD7768_REG24_ADC_DATA, &readval);
+	if (ret)
+		return ret;
+
 	/*
 	 * Any SPI configuration of the AD7768-1 can only be
 	 * performed in continuous conversion mode.
@@ -259,16 +296,23 @@ static int ad7768_reg_access(struct iio_dev *indio_dev,
 	if (ret)
 		return ret;
 
+	ret = -EINVAL;
 	if (readval) {
-		ret = ad7768_spi_reg_read(st, reg, 1);
-		if (ret < 0)
-			goto err_release;
-		*readval = ret;
-		ret = 0;
+		if (regmap_check_range_table(st->regmap, reg, &ad7768_regmap_rd_table))
+			ret = regmap_read(st->regmap, reg, readval);
+
+		if (regmap_check_range_table(st->regmap24, reg, &ad7768_regmap24_rd_table))
+			ret = regmap_read(st->regmap24, reg, readval);
+
 	} else {
-		ret = ad7768_spi_reg_write(st, reg, writeval);
+		if (regmap_check_range_table(st->regmap, reg, &ad7768_regmap_wr_table))
+			ret = regmap_write(st->regmap, reg, writeval);
+
+		if (regmap_check_range_table(st->regmap24, reg, &ad7768_regmap24_wr_table))
+			ret = regmap_write(st->regmap24, reg, writeval);
+
 	}
-err_release:
+
 	iio_device_release_direct_mode(indio_dev);
 
 	return ret;
@@ -285,7 +329,7 @@ static int ad7768_set_dig_fil(struct ad7768_state *st,
 	else
 		mode = AD7768_DIG_FIL_DEC_RATE(dec_rate);
 
-	ret = ad7768_spi_reg_write(st, AD7768_REG_DIGITAL_FILTER, mode);
+	ret = regmap_write(st->regmap, AD7768_REG_DIGITAL_FILTER, mode);
 	if (ret < 0)
 		return ret;
 
@@ -322,7 +366,7 @@ static int ad7768_set_freq(struct ad7768_state *st,
 	 */
 	pwr_mode = AD7768_PWR_MCLK_DIV(ad7768_clk_config[idx].mclk_div) |
 		   AD7768_PWR_PWRMODE(ad7768_clk_config[idx].pwrmode);
-	ret = ad7768_spi_reg_write(st, AD7768_REG_POWER_CLOCK, pwr_mode);
+	ret = regmap_write(st->regmap, AD7768_REG_POWER_CLOCK, pwr_mode);
 	if (ret < 0)
 		return ret;
 
@@ -449,11 +493,11 @@ static int ad7768_setup(struct ad7768_state *st)
 	 * to 10. When the sequence is detected, the reset occurs.
 	 * See the datasheet, page 70.
 	 */
-	ret = ad7768_spi_reg_write(st, AD7768_REG_SYNC_RESET, 0x3);
+	ret = regmap_write(st->regmap, AD7768_REG_SYNC_RESET, 0x3);
 	if (ret)
 		return ret;
 
-	ret = ad7768_spi_reg_write(st, AD7768_REG_SYNC_RESET, 0x2);
+	ret = regmap_write(st->regmap, AD7768_REG_SYNC_RESET, 0x2);
 	if (ret)
 		return ret;
 
@@ -508,18 +552,19 @@ static int ad7768_buffer_postenable(struct iio_dev *indio_dev)
 	 * continuous read mode. Subsequent data reads do not require an
 	 * initial 8-bit write to query the ADC_DATA register.
 	 */
-	return ad7768_spi_reg_write(st, AD7768_REG_INTERFACE_FORMAT, 0x01);
+	return regmap_write(st->regmap, AD7768_REG_INTERFACE_FORMAT, 0x01);
 }
 
 static int ad7768_buffer_predisable(struct iio_dev *indio_dev)
 {
 	struct ad7768_state *st = iio_priv(indio_dev);
+	unsigned int unused;
 
 	/*
 	 * To exit continuous read mode, perform a single read of the ADC_DATA
 	 * reg (0x2C), which allows further configuration of the device.
 	 */
-	return ad7768_spi_reg_read(st, AD7768_REG_ADC_DATA, 3);
+	return regmap_read(st->regmap24, AD7768_REG24_ADC_DATA, &unused);
 }
 
 static const struct iio_buffer_setup_ops ad7768_buffer_ops = {
@@ -590,6 +635,16 @@ static int ad7768_probe(struct spi_device *spi)
 
 	st->spi = spi;
 
+	st->regmap = devm_regmap_init_spi(spi, &ad7768_regmap_config);
+	if (IS_ERR(st->regmap))
+		return dev_err_probe(&spi->dev, PTR_ERR(st->regmap),
+				     "Failed to initialize regmap");
+
+	st->regmap24 = devm_regmap_init_spi(spi, &ad7768_regmap24_config);
+	if (IS_ERR(st->regmap24))
+		return dev_err_probe(&spi->dev, PTR_ERR(st->regmap24),
+				     "Failed to initialize regmap24");
+
 	st->vref = devm_regulator_get(&spi->dev, "vref");
 	if (IS_ERR(st->vref))
 		return PTR_ERR(st->vref);
-- 
2.34.1

[PATCH v4 09/17] iio: adc: ad7768-1: Add reset gpio

[Jonathan Santos]

From: Sergiu Cuciurean <sergiu.cuciurean@analog.com>

Depending on the controller, the default state of a gpio can vary. This
change excludes the probability that the dafult state of the ADC reset
gpio will be HIGH if it will be passed as reference in the devicetree.

Reviewed-by: David Lechner <dlechner@baylibre.com>
Reviewed-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
Signed-off-by: Sergiu Cuciurean <sergiu.cuciurean@analog.com>
Co-developed-by: Jonathan Santos <Jonathan.Santos@analog.com>
Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com>
---
v4 Changes:
* None.

v3 Changes:
* fixed SoB order.
* increased delay after finishing the reset action to 200us, as the
  datasheet recommends.

v2 Changes:
* Replaced usleep_range() for fsleep() and gpiod_direction_output() for 
  gpiod_set_value_cansleep().
* Reset via SPI register is performed if the Reset GPIO is not defined. 
---
 drivers/iio/adc/ad7768-1.c | 36 ++++++++++++++++++++++++------------
 1 file changed, 24 insertions(+), 12 deletions(-)

diff --git a/drivers/iio/adc/ad7768-1.c b/drivers/iio/adc/ad7768-1.c
index 04a26e5b7d5c..86f44d28c478 100644
--- a/drivers/iio/adc/ad7768-1.c
+++ b/drivers/iio/adc/ad7768-1.c
@@ -166,6 +166,7 @@ struct ad7768_state {
 	struct completion completion;
 	struct iio_trigger *trig;
 	struct gpio_desc *gpio_sync_in;
+	struct gpio_desc *gpio_reset;
 	const char *labels[ARRAY_SIZE(ad7768_channels)];
 	/*
 	 * DMA (thus cache coherency maintenance) may require the
@@ -487,19 +488,30 @@ static int ad7768_setup(struct ad7768_state *st)
 {
 	int ret;
 
-	/*
-	 * Two writes to the SPI_RESET[1:0] bits are required to initiate
-	 * a software reset. The bits must first be set to 11, and then
-	 * to 10. When the sequence is detected, the reset occurs.
-	 * See the datasheet, page 70.
-	 */
-	ret = regmap_write(st->regmap, AD7768_REG_SYNC_RESET, 0x3);
-	if (ret)
-		return ret;
+	st->gpio_reset = devm_gpiod_get_optional(&st->spi->dev, "reset",
+						 GPIOD_OUT_HIGH);
+	if (IS_ERR(st->gpio_reset))
+		return PTR_ERR(st->gpio_reset);
 
-	ret = regmap_write(st->regmap, AD7768_REG_SYNC_RESET, 0x2);
-	if (ret)
-		return ret;
+	if (st->gpio_reset) {
+		fsleep(10);
+		gpiod_set_value_cansleep(st->gpio_reset, 0);
+		fsleep(200);
+	} else {
+		/*
+		 * Two writes to the SPI_RESET[1:0] bits are required to initiate
+		 * a software reset. The bits must first be set to 11, and then
+		 * to 10. When the sequence is detected, the reset occurs.
+		 * See the datasheet, page 70.
+		 */
+		ret = regmap_write(st->regmap, AD7768_REG_SYNC_RESET, 0x3);
+		if (ret)
+			return ret;
+
+		ret = regmap_write(st->regmap, AD7768_REG_SYNC_RESET, 0x2);
+		if (ret)
+			return ret;
+	}
 
 	st->gpio_sync_in = devm_gpiod_get(&st->spi->dev, "adi,sync-in",
 					  GPIOD_OUT_LOW);
-- 
2.34.1

[PATCH v4 10/17] iio: adc: ad7768-1: Move buffer allocation to a separate function

[Jonathan Santos]

From: Sergiu Cuciurean <sergiu.cuciurean@analog.com>

This change moves the buffer allocation in a separate function, making
space for adding another type of iio buffer if needed.

Reviewed-by: David Lechner <dlechner@baylibre.com>
Signed-off-by: Sergiu Cuciurean <sergiu.cuciurean@analog.com>
Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com>
---
v4 Changes:
* None.

v3 Changes:
* Added missing SoB.

v2 Changes:
* Interrupt and completion moved out from ad7768_triggered_buffer_alloc(). 
---
 drivers/iio/adc/ad7768-1.c | 44 ++++++++++++++++++++++----------------
 1 file changed, 26 insertions(+), 18 deletions(-)

diff --git a/drivers/iio/adc/ad7768-1.c b/drivers/iio/adc/ad7768-1.c
index 86f44d28c478..e88e9431bb7a 100644
--- a/drivers/iio/adc/ad7768-1.c
+++ b/drivers/iio/adc/ad7768-1.c
@@ -619,6 +619,31 @@ static int ad7768_set_channel_label(struct iio_dev *indio_dev,
 	return 0;
 }
 
+static int ad7768_triggered_buffer_alloc(struct iio_dev *indio_dev)
+{
+	struct ad7768_state *st = iio_priv(indio_dev);
+	int ret;
+
+	st->trig = devm_iio_trigger_alloc(indio_dev->dev.parent, "%s-dev%d",
+					  indio_dev->name,
+					  iio_device_id(indio_dev));
+	if (!st->trig)
+		return -ENOMEM;
+
+	st->trig->ops = &ad7768_trigger_ops;
+	iio_trigger_set_drvdata(st->trig, indio_dev);
+	ret = devm_iio_trigger_register(indio_dev->dev.parent, st->trig);
+	if (ret)
+		return ret;
+
+	indio_dev->trig = iio_trigger_get(st->trig);
+
+	return devm_iio_triggered_buffer_setup(indio_dev->dev.parent, indio_dev,
+					       &iio_pollfunc_store_time,
+					       &ad7768_trigger_handler,
+					       &ad7768_buffer_ops);
+}
+
 static int ad7768_probe(struct spi_device *spi)
 {
 	struct ad7768_state *st;
@@ -689,20 +714,6 @@ static int ad7768_probe(struct spi_device *spi)
 		return ret;
 	}
 
-	st->trig = devm_iio_trigger_alloc(&spi->dev, "%s-dev%d",
-					  indio_dev->name,
-					  iio_device_id(indio_dev));
-	if (!st->trig)
-		return -ENOMEM;
-
-	st->trig->ops = &ad7768_trigger_ops;
-	iio_trigger_set_drvdata(st->trig, indio_dev);
-	ret = devm_iio_trigger_register(&spi->dev, st->trig);
-	if (ret)
-		return ret;
-
-	indio_dev->trig = iio_trigger_get(st->trig);
-
 	init_completion(&st->completion);
 
 	ret = ad7768_set_channel_label(indio_dev, ARRAY_SIZE(ad7768_channels));
@@ -716,10 +727,7 @@ static int ad7768_probe(struct spi_device *spi)
 	if (ret)
 		return ret;
 
-	ret = devm_iio_triggered_buffer_setup(&spi->dev, indio_dev,
-					      &iio_pollfunc_store_time,
-					      &ad7768_trigger_handler,
-					      &ad7768_buffer_ops);
+	ret = ad7768_triggered_buffer_alloc(indio_dev);
 	if (ret)
 		return ret;
 
-- 
2.34.1

[PATCH v4 11/17] iio: adc: ad7768-1: add regulator to control VCM output

[Jonathan Santos]

The VCM output voltage can be used as a common-mode voltage within the
amplifier preconditioning circuits external to the AD7768-1.

This change allows the user to configure VCM output using the regulator
framework.

Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com>
---
v4 Changes:
* Added iio_device_claim_direct_mode() to regulator callbacks to avoid register access
  while in buffered mode.
* Changed regulator name to "ad7768-1-vcm".
* When regulator enable is called, it will set the last voltage selector configured.
* Disabled regulator before configuring it.
* Adressed other nits.

v3 Changes:
* Register VCM output via the regulator framework for improved flexibility
  and external integration.

v2 Changes:
* VCM output support is now defined by a devicetree property, instead of 
  and IIO attribute.
---
 drivers/iio/adc/Kconfig    |   1 +
 drivers/iio/adc/ad7768-1.c | 181 +++++++++++++++++++++++++++++++++++++
 2 files changed, 182 insertions(+)

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index a2fdb7e03a66..d8f2ed477ba7 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -277,6 +277,7 @@ config AD7766
 config AD7768_1
 	tristate "Analog Devices AD7768-1 ADC driver"
 	depends on SPI
+	select REGULATOR
 	select REGMAP_SPI
 	select IIO_BUFFER
 	select IIO_TRIGGER
diff --git a/drivers/iio/adc/ad7768-1.c b/drivers/iio/adc/ad7768-1.c
index e88e9431bb7a..2a6317f5b582 100644
--- a/drivers/iio/adc/ad7768-1.c
+++ b/drivers/iio/adc/ad7768-1.c
@@ -12,8 +12,10 @@
 #include <linux/gpio/consumer.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/of.h>
 #include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
+#include <linux/regulator/driver.h>
 #include <linux/sysfs.h>
 #include <linux/spi/spi.h>
 
@@ -80,9 +82,15 @@
 #define AD7768_CONV_MODE_MSK		GENMASK(2, 0)
 #define AD7768_CONV_MODE(x)		FIELD_PREP(AD7768_CONV_MODE_MSK, x)
 
+/* AD7768_REG_ANALOG2 */
+#define AD7768_REG_ANALOG2_VCM_MSK	GENMASK(2, 0)
+#define AD7768_REG_ANALOG2_VCM(x)	FIELD_PREP(AD7768_REG_ANALOG2_VCM_MSK, x)
+
 #define AD7768_RD_FLAG_MSK(x)		(BIT(6) | ((x) & 0x3F))
 #define AD7768_WR_FLAG_MSK(x)		((x) & 0x3F)
 
+#define AD7768_VCM_OFF			0x07
+
 enum ad7768_conv_mode {
 	AD7768_CONTINUOUS,
 	AD7768_ONE_SHOT,
@@ -160,6 +168,8 @@ struct ad7768_state {
 	struct regmap *regmap;
 	struct regmap *regmap24;
 	struct regulator *vref;
+	struct regulator_dev *vcm_rdev;
+	unsigned int vcm_output_sel;
 	struct clk *mclk;
 	unsigned int mclk_freq;
 	unsigned int samp_freq;
@@ -644,6 +654,172 @@ static int ad7768_triggered_buffer_alloc(struct iio_dev *indio_dev)
 					       &ad7768_buffer_ops);
 }
 
+static int ad7768_vcm_enable(struct regulator_dev *rdev)
+{
+	struct iio_dev *indio_dev = rdev_get_drvdata(rdev);
+	struct ad7768_state *st = iio_priv(indio_dev);
+	int ret, regval;
+
+	if (!indio_dev)
+		return -EINVAL;
+
+	ret = iio_device_claim_direct_mode(indio_dev);
+	if (ret)
+		return ret;
+
+	/* To enable, set the last selected output */
+	regval = AD7768_REG_ANALOG2_VCM(st->vcm_output_sel + 1);
+	ret = regmap_update_bits(st->regmap, AD7768_REG_ANALOG2,
+				 AD7768_REG_ANALOG2_VCM_MSK, regval);
+	iio_device_release_direct_mode(indio_dev);
+
+	return ret;
+}
+
+static int ad7768_vcm_disable(struct regulator_dev *rdev)
+{
+	struct iio_dev *indio_dev = rdev_get_drvdata(rdev);
+	struct ad7768_state *st = iio_priv(indio_dev);
+	int ret;
+
+	if (!indio_dev)
+		return -EINVAL;
+
+	ret = iio_device_claim_direct_mode(indio_dev);
+	if (ret)
+		return ret;
+
+	ret = regmap_update_bits(st->regmap, AD7768_REG_ANALOG2,
+				 AD7768_REG_ANALOG2_VCM_MSK, AD7768_VCM_OFF);
+	iio_device_release_direct_mode(indio_dev);
+
+	return ret;
+}
+
+static int ad7768_vcm_is_enabled(struct regulator_dev *rdev)
+{
+	struct iio_dev *indio_dev = rdev_get_drvdata(rdev);
+	struct ad7768_state *st = iio_priv(indio_dev);
+	int ret, val;
+
+	if (!indio_dev)
+		return -EINVAL;
+
+	ret = iio_device_claim_direct_mode(indio_dev);
+	if (ret)
+		return ret;
+
+	ret = regmap_read(st->regmap, AD7768_REG_ANALOG2, &val);
+	if (ret)
+		goto err_release;
+
+	ret = FIELD_GET(AD7768_REG_ANALOG2_VCM_MSK, val) != AD7768_VCM_OFF;
+err_release:
+	iio_device_release_direct_mode(indio_dev);
+
+	return ret;
+}
+
+static int ad7768_set_voltage_sel(struct regulator_dev *rdev,
+				  unsigned int selector)
+{
+	unsigned int regval = AD7768_REG_ANALOG2_VCM(selector + 1);
+	struct iio_dev *indio_dev = rdev_get_drvdata(rdev);
+	struct ad7768_state *st = iio_priv(indio_dev);
+	int ret;
+
+	if (!indio_dev)
+		return -EINVAL;
+
+	ret = iio_device_claim_direct_mode(indio_dev);
+	if (ret)
+		return ret;
+
+	ret = regmap_update_bits(st->regmap, AD7768_REG_ANALOG2,
+				 AD7768_REG_ANALOG2_VCM_MSK, regval);
+	iio_device_release_direct_mode(indio_dev);
+	st->vcm_output_sel = selector;
+
+	return ret;
+}
+
+static int ad7768_get_voltage_sel(struct regulator_dev *rdev)
+{
+	struct iio_dev *indio_dev = rdev_get_drvdata(rdev);
+	struct ad7768_state *st = iio_priv(indio_dev);
+	int ret, val;
+
+	if (!indio_dev)
+		return -EINVAL;
+
+	ret = iio_device_claim_direct_mode(indio_dev);
+	if (ret)
+		return ret;
+
+	ret = regmap_read(st->regmap, AD7768_REG_ANALOG2, &val);
+	if (ret)
+		goto err_release;
+
+	val = FIELD_GET(AD7768_REG_ANALOG2_VCM_MSK, val);
+	ret = clamp(val, 1, (int)rdev->desc->n_voltages) - 1;
+err_release:
+	iio_device_release_direct_mode(indio_dev);
+
+	return ret;
+}
+
+static const struct regulator_ops vcm_regulator_ops = {
+	.enable = ad7768_vcm_enable,
+	.disable = ad7768_vcm_disable,
+	.is_enabled = ad7768_vcm_is_enabled,
+	.list_voltage = regulator_list_voltage_table,
+	.set_voltage_sel = ad7768_set_voltage_sel,
+	.get_voltage_sel = ad7768_get_voltage_sel,
+};
+
+static const unsigned int vcm_voltage_table[] = {
+	2500000,
+	2050000,
+	1650000,
+	1900000,
+	1100000,
+	900000,
+};
+
+static const struct regulator_desc vcm_desc = {
+	.name = "ad7768-1-vcm",
+	.of_match = of_match_ptr("vcm-output"),
+	.regulators_node = of_match_ptr("regulators"),
+	.n_voltages = ARRAY_SIZE(vcm_voltage_table),
+	.volt_table = vcm_voltage_table,
+	.ops = &vcm_regulator_ops,
+	.type = REGULATOR_VOLTAGE,
+	.owner = THIS_MODULE,
+};
+
+static int ad7768_register_regulators(struct device *dev, struct ad7768_state *st,
+				      struct iio_dev *indio_dev)
+{
+	struct regulator_config config = {
+		.dev = dev,
+		.driver_data = indio_dev,
+	};
+	int ret;
+
+	/* Disable the regulator before registering it */
+	ret = regmap_update_bits(st->regmap, AD7768_REG_ANALOG2,
+				 AD7768_REG_ANALOG2_VCM_MSK, AD7768_VCM_OFF);
+	if (ret)
+		return -EINVAL;
+
+	st->vcm_rdev = devm_regulator_register(dev, &vcm_desc, &config);
+	if (IS_ERR(st->vcm_rdev))
+		return dev_err_probe(dev, PTR_ERR(st->vcm_rdev),
+				     "failed to register VCM regulator\n");
+
+	return 0;
+}
+
 static int ad7768_probe(struct spi_device *spi)
 {
 	struct ad7768_state *st;
@@ -708,6 +884,11 @@ static int ad7768_probe(struct spi_device *spi)
 	indio_dev->info = &ad7768_info;
 	indio_dev->modes = INDIO_DIRECT_MODE;
 
+	/* Register VCM output regulator */
+	ret = ad7768_register_regulators(&spi->dev, st, indio_dev);
+	if (ret)
+		return ret;
+
 	ret = ad7768_setup(st);
 	if (ret < 0) {
 		dev_err(&spi->dev, "AD7768 setup failed\n");
-- 
2.34.1

[PATCH v4 12/17] iio: adc: ad7768-1: Add GPIO controller support

[Jonathan Santos]

From: Sergiu Cuciurean <sergiu.cuciurean@analog.com>

The AD7768-1 has the ability to control other local hardware (such as gain
stages),to power down other blocks in the signal chain, or read local
status signals over the SPI interface.

Add direct mode conditional locks in the gpio callbacks to prevent register
access when the device is in buffered mode.

This change exports the AD7768-1's four gpios and makes them accessible
at an upper layer.

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Sergiu Cuciurean <sergiu.cuciurean@analog.com>
Co-developed-by: Jonathan Santos <Jonathan.Santos@analog.com>
Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com>
---
v4 Changes:
* Mentioned in the commit message that we cannot tweak the GPIO controller
  when the device is not in direct mode.

v3 Changes:
* Fixed SoB order.
* Added mising iio_device_release_direct_mode().
* Simplified some regmap writes.
* Removed ad7768_gpio_request() callback.
* Fixed line wrapping.

v2 Changes:
* Replaced mutex for iio_device_claim_direct_mode().
* Use gpio-controller property to conditionally enable the
  GPIO support.
* OBS: when the GPIO is configured as output, we should read 
  the current state value from AD7768_REG_GPIO_WRITE.
---
 drivers/iio/adc/ad7768-1.c | 143 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 141 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/adc/ad7768-1.c b/drivers/iio/adc/ad7768-1.c
index 2a6317f5b582..d1c3bf4f0f45 100644
--- a/drivers/iio/adc/ad7768-1.c
+++ b/drivers/iio/adc/ad7768-1.c
@@ -9,6 +9,8 @@
 #include <linux/delay.h>
 #include <linux/device.h>
 #include <linux/err.h>
+#include <linux/gpio.h>
+#include <linux/gpio/driver.h>
 #include <linux/gpio/consumer.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
@@ -86,6 +88,16 @@
 #define AD7768_REG_ANALOG2_VCM_MSK	GENMASK(2, 0)
 #define AD7768_REG_ANALOG2_VCM(x)	FIELD_PREP(AD7768_REG_ANALOG2_VCM_MSK, x)
 
+/* AD7768_REG_GPIO_CONTROL */
+#define AD7768_GPIO_UNIVERSAL_EN	BIT(7)
+#define AD7768_GPIO_CONTROL_MSK		GENMASK(3, 0)
+
+/* AD7768_REG_GPIO_WRITE */
+#define AD7768_GPIO_WRITE_MSK		GENMASK(3, 0)
+
+/* AD7768_REG_GPIO_READ */
+#define AD7768_GPIO_READ_MSK		GENMASK(3, 0)
+
 #define AD7768_RD_FLAG_MSK(x)		(BIT(6) | ((x) & 0x3F))
 #define AD7768_WR_FLAG_MSK(x)		((x) & 0x3F)
 
@@ -171,6 +183,7 @@ struct ad7768_state {
 	struct regulator_dev *vcm_rdev;
 	unsigned int vcm_output_sel;
 	struct clk *mclk;
+	struct gpio_chip gpiochip;
 	unsigned int mclk_freq;
 	unsigned int samp_freq;
 	struct completion completion;
@@ -351,6 +364,124 @@ static int ad7768_set_dig_fil(struct ad7768_state *st,
 	return 0;
 }
 
+static int ad7768_gpio_direction_input(struct gpio_chip *chip, unsigned int offset)
+{
+	struct iio_dev *indio_dev = gpiochip_get_data(chip);
+	struct ad7768_state *st = iio_priv(indio_dev);
+	int ret;
+
+	ret = iio_device_claim_direct_mode(indio_dev);
+	if (ret)
+		return ret;
+
+	ret = regmap_clear_bits(st->regmap, AD7768_REG_GPIO_CONTROL,
+				BIT(offset));
+	iio_device_release_direct_mode(indio_dev);
+
+	return ret;
+}
+
+static int ad7768_gpio_direction_output(struct gpio_chip *chip,
+					unsigned int offset, int value)
+{
+	struct iio_dev *indio_dev = gpiochip_get_data(chip);
+	struct ad7768_state *st = iio_priv(indio_dev);
+	int ret;
+
+	ret = iio_device_claim_direct_mode(indio_dev);
+	if (ret)
+		return ret;
+
+	ret = regmap_set_bits(st->regmap, AD7768_REG_GPIO_CONTROL,
+			      BIT(offset));
+	iio_device_release_direct_mode(indio_dev);
+
+	return ret;
+}
+
+static int ad7768_gpio_get(struct gpio_chip *chip, unsigned int offset)
+{
+	struct iio_dev *indio_dev = gpiochip_get_data(chip);
+	struct ad7768_state *st = iio_priv(indio_dev);
+	unsigned int val;
+	int ret;
+
+	ret = iio_device_claim_direct_mode(indio_dev);
+	if (ret)
+		return ret;
+
+	ret = regmap_read(st->regmap, AD7768_REG_GPIO_CONTROL, &val);
+	if (ret)
+		goto err_release;
+
+	/*
+	 * If the GPIO is configured as an output, read the current value from
+	 * AD7768_REG_GPIO_WRITE. Otherwise, read the input value from
+	 * AD7768_REG_GPIO_READ.
+	 */
+	if (val & BIT(offset))
+		ret = regmap_read(st->regmap, AD7768_REG_GPIO_WRITE, &val);
+	else
+		ret = regmap_read(st->regmap, AD7768_REG_GPIO_READ, &val);
+	if (ret)
+		goto err_release;
+
+	ret = !!(val & BIT(offset));
+err_release:
+	iio_device_release_direct_mode(indio_dev);
+
+	return ret;
+}
+
+static void ad7768_gpio_set(struct gpio_chip *chip, unsigned int offset, int value)
+{
+	struct iio_dev *indio_dev = gpiochip_get_data(chip);
+	struct ad7768_state *st = iio_priv(indio_dev);
+	unsigned int val;
+	int ret;
+
+	ret = iio_device_claim_direct_mode(indio_dev);
+	if (ret)
+		return;
+
+	ret = regmap_read(st->regmap, AD7768_REG_GPIO_CONTROL, &val);
+	if (ret)
+		goto err_release;
+
+	if (val & BIT(offset))
+		regmap_update_bits(st->regmap, AD7768_REG_GPIO_WRITE,
+				   BIT(offset), value << offset);
+
+err_release:
+	iio_device_release_direct_mode(indio_dev);
+}
+
+static int ad7768_gpio_init(struct iio_dev *indio_dev)
+{
+	struct ad7768_state *st = iio_priv(indio_dev);
+	int ret;
+
+	ret = regmap_write(st->regmap, AD7768_REG_GPIO_CONTROL,
+			   AD7768_GPIO_UNIVERSAL_EN);
+	if (ret)
+		return ret;
+
+	st->gpiochip = (struct gpio_chip) {
+		.label = "ad7768_1_gpios",
+		.base = -1,
+		.ngpio = 4,
+		.parent = &st->spi->dev,
+		.can_sleep = true,
+		.direction_input = ad7768_gpio_direction_input,
+		.direction_output = ad7768_gpio_direction_output,
+		.get = ad7768_gpio_get,
+		.set = ad7768_gpio_set,
+		.owner = THIS_MODULE,
+	};
+
+	return gpiochip_add_data(&st->gpiochip, indio_dev);
+}
+
 static int ad7768_set_freq(struct ad7768_state *st,
 			   unsigned int freq)
 {
@@ -494,8 +625,9 @@ static const struct iio_info ad7768_info = {
 	.debugfs_reg_access = &ad7768_reg_access,
 };
 
-static int ad7768_setup(struct ad7768_state *st)
+static int ad7768_setup(struct iio_dev *indio_dev)
 {
+	struct ad7768_state *st = iio_priv(indio_dev);
 	int ret;
 
 	st->gpio_reset = devm_gpiod_get_optional(&st->spi->dev, "reset",
@@ -528,6 +660,13 @@ static int ad7768_setup(struct ad7768_state *st)
 	if (IS_ERR(st->gpio_sync_in))
 		return PTR_ERR(st->gpio_sync_in);
 
+	/* Only create a Chip GPIO if flagged for it */
+	if (device_property_read_bool(&st->spi->dev, "gpio-controller")) {
+		ret = ad7768_gpio_init(indio_dev);
+		if (ret < 0)
+			return ret;
+	}
+
 	/* Set the default sampling frequency to 32000 kSPS */
 	return ad7768_set_freq(st, 32000);
 }
@@ -889,7 +1028,7 @@ static int ad7768_probe(struct spi_device *spi)
 	if (ret)
 		return ret;
 
-	ret = ad7768_setup(st);
+	ret = ad7768_setup(indio_dev);
 	if (ret < 0) {
 		dev_err(&spi->dev, "AD7768 setup failed\n");
 		return ret;
-- 
2.34.1

[PATCH v4 13/17] iio: adc: ad7768-1: add multiple scan types to support 16-bits mode

[Jonathan Santos]

When the device is configured to decimation x8, only possible in the
sinc5 filter, output data is reduced to 16-bits in order to support
1 MHz of sampling frequency due to clock limitation.

Use multiple scan types feature to enable the driver to switch
scan type in runtime, making possible to support both 24-bit and
16-bit resolution.

Reviewed-by: David Lechner <dlechner@baylire.com>
Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com>
---
v4 Changes:
* None.

v3 Changes:
* Decreased storagebits to 16 for AD7768_SCAN_TYPE_HIGH_SPEED
  scan type.

v2 Changes:
* Included the ".shift" value back to scan_type.
* Changed the number of bytes from regmap_read instead of shifting the 
  ADC sample value when the word size is lower (16-bits).
---
 drivers/iio/adc/ad7768-1.c | 74 ++++++++++++++++++++++++++++++++------
 1 file changed, 64 insertions(+), 10 deletions(-)

diff --git a/drivers/iio/adc/ad7768-1.c b/drivers/iio/adc/ad7768-1.c
index d1c3bf4f0f45..a51279fa6447 100644
--- a/drivers/iio/adc/ad7768-1.c
+++ b/drivers/iio/adc/ad7768-1.c
@@ -142,6 +142,15 @@ struct ad7768_clk_configuration {
 	enum ad7768_pwrmode pwrmode;
 };
 
+enum ad7768_scan_type {
+	AD7768_SCAN_TYPE_NORMAL,
+	AD7768_SCAN_TYPE_HIGH_SPEED,
+};
+
+static const int ad7768_mclk_div_rates[4] = {
+	16, 8, 4, 2,
+};
+
 static const struct ad7768_clk_configuration ad7768_clk_config[] = {
 	{ AD7768_MCLK_DIV_2, AD7768_DEC_RATE_8, 16,  AD7768_FAST_MODE },
 	{ AD7768_MCLK_DIV_2, AD7768_DEC_RATE_16, 32,  AD7768_FAST_MODE },
@@ -156,6 +165,22 @@ static const struct ad7768_clk_configuration ad7768_clk_config[] = {
 	{ AD7768_MCLK_DIV_16, AD7768_DEC_RATE_1024, 16384, AD7768_ECO_MODE },
 };
 
+static const struct iio_scan_type ad7768_scan_type[] = {
+	[AD7768_SCAN_TYPE_NORMAL] = {
+		.sign = 's',
+		.realbits = 24,
+		.storagebits = 32,
+		.shift = 8,
+		.endianness = IIO_BE,
+	},
+	[AD7768_SCAN_TYPE_HIGH_SPEED] = {
+		.sign = 's',
+		.realbits = 16,
+		.storagebits = 16,
+		.endianness = IIO_BE,
+	},
+};
+
 static const struct iio_chan_spec ad7768_channels[] = {
 	{
 		.type = IIO_VOLTAGE,
@@ -165,13 +190,9 @@ static const struct iio_chan_spec ad7768_channels[] = {
 		.indexed = 1,
 		.channel = 0,
 		.scan_index = 0,
-		.scan_type = {
-			.sign = 's',
-			.realbits = 24,
-			.storagebits = 32,
-			.shift = 8,
-			.endianness = IIO_BE,
-		},
+		.has_ext_scan_type = 1,
+		.ext_scan_type = ad7768_scan_type,
+		.num_ext_scan_type = ARRAY_SIZE(ad7768_scan_type),
 	},
 };
 
@@ -185,6 +206,7 @@ struct ad7768_state {
 	struct clk *mclk;
 	struct gpio_chip gpiochip;
 	unsigned int mclk_freq;
+	unsigned int dec_rate;
 	unsigned int samp_freq;
 	struct completion completion;
 	struct iio_trigger *trig;
@@ -297,6 +319,15 @@ static int ad7768_scan_direct(struct iio_dev *indio_dev)
 	if (ret)
 		return ret;
 
+	/*
+	 * When the decimation rate is set to x8, the ADC data precision is
+	 * reduced from 24 bits to 16 bits. Since the AD7768_REG_ADC_DATA
+	 * register provides 24-bit data, the precision is reduced by
+	 * right-shifting the read value by 8 bits.
+	 */
+	if (st->dec_rate == 8)
+		readval >>= 8;
+
 	/*
 	 * Any SPI configuration of the AD7768-1 can only be
 	 * performed in continuous conversion mode.
@@ -516,6 +547,8 @@ static int ad7768_set_freq(struct ad7768_state *st,
 	if (ret < 0)
 		return ret;
 
+	st->dec_rate = ad7768_clk_config[idx].clk_div /
+		       ad7768_mclk_div_rates[ad7768_clk_config[idx].mclk_div];
 	st->samp_freq = DIV_ROUND_CLOSEST(st->mclk_freq,
 					  ad7768_clk_config[idx].clk_div);
 
@@ -549,8 +582,13 @@ static int ad7768_read_raw(struct iio_dev *indio_dev,
 			   int *val, int *val2, long info)
 {
 	struct ad7768_state *st = iio_priv(indio_dev);
+	const struct iio_scan_type *scan_type;
 	int scale_uv, ret;
 
+	scan_type = iio_get_current_scan_type(indio_dev, chan);
+	if (IS_ERR(scan_type))
+		return PTR_ERR(scan_type);
+
 	switch (info) {
 	case IIO_CHAN_INFO_RAW:
 		ret = iio_device_claim_direct_mode(indio_dev);
@@ -559,7 +597,7 @@ static int ad7768_read_raw(struct iio_dev *indio_dev,
 
 		ret = ad7768_scan_direct(indio_dev);
 		if (ret >= 0)
-			*val = sign_extend32(ret, chan->scan_type.realbits - 1);
+			*val = sign_extend32(ret, scan_type->realbits - 1);
 
 		iio_device_release_direct_mode(indio_dev);
 		if (ret < 0)
@@ -573,7 +611,7 @@ static int ad7768_read_raw(struct iio_dev *indio_dev,
 			return scale_uv;
 
 		*val = (scale_uv * 2) / 1000;
-		*val2 = chan->scan_type.realbits;
+		*val2 = scan_type->realbits;
 
 		return IIO_VAL_FRACTIONAL_LOG2;
 
@@ -617,11 +655,21 @@ static const struct attribute_group ad7768_group = {
 	.attrs = ad7768_attributes,
 };
 
+static int ad7768_get_current_scan_type(const struct iio_dev *indio_dev,
+					const struct iio_chan_spec *chan)
+{
+	struct ad7768_state *st = iio_priv(indio_dev);
+
+	return st->dec_rate == 8 ? AD7768_SCAN_TYPE_HIGH_SPEED :
+		AD7768_SCAN_TYPE_NORMAL;
+}
+
 static const struct iio_info ad7768_info = {
 	.attrs = &ad7768_group,
 	.read_raw = &ad7768_read_raw,
 	.write_raw = &ad7768_write_raw,
 	.read_label = ad7768_read_label,
+	.get_current_scan_type = &ad7768_get_current_scan_type,
 	.debugfs_reg_access = &ad7768_reg_access,
 };
 
@@ -676,9 +724,15 @@ static irqreturn_t ad7768_trigger_handler(int irq, void *p)
 	struct iio_poll_func *pf = p;
 	struct iio_dev *indio_dev = pf->indio_dev;
 	struct ad7768_state *st = iio_priv(indio_dev);
+	const struct iio_scan_type *scan_type;
 	int ret;
 
-	ret = spi_read(st->spi, &st->data.scan.chan, 3);
+	scan_type = iio_get_current_scan_type(indio_dev, &indio_dev->channels[0]);
+	if (IS_ERR(scan_type))
+		return PTR_ERR(scan_type);
+
+	ret = spi_read(st->spi, &st->data.scan.chan,
+		       BITS_TO_BYTES(scan_type->realbits));
 	if (ret < 0)
 		goto out;
 
-- 
2.34.1

[PATCH v4 14/17] iio: adc: ad7768-1: add support for Synchronization over SPI

[Jonathan Santos]

The synchronization method using GPIO requires the generated pulse to be
truly synchronous with the base MCLK signal. When it is not possible to
do that in hardware, the datasheet recommends using synchronization over
SPI, where the generated pulse is already synchronous with MCLK. This
requires the SYNC_OUT pin to be connected to SYNC_IN pin.

Use trigger-sources property to enable device synchronization over SPI.

Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com>
---
v4 Changes:
* None.

v3 Changes:
* Fixed args.fwnode leakage in the trigger-sources parsing.
* Synchronization over spi is enabled when the trigger-sources
  references the own device.
* Synchronization is kept within the device, and return error if the
  gpio is not defined and the trigger-sources reference does not match
  the current device. 

v2 Changes:
* Synchronization via SPI is enabled when the Sync GPIO is not defined.
* now trigger-sources property indicates the synchronization provider or
  main device. The main device will be used to drive the SYNC_IN when
  requested (via GPIO or SPI).
---
 drivers/iio/adc/ad7768-1.c | 80 ++++++++++++++++++++++++++++++++++----
 1 file changed, 72 insertions(+), 8 deletions(-)

diff --git a/drivers/iio/adc/ad7768-1.c b/drivers/iio/adc/ad7768-1.c
index a51279fa6447..c48d3e0af985 100644
--- a/drivers/iio/adc/ad7768-1.c
+++ b/drivers/iio/adc/ad7768-1.c
@@ -212,6 +212,7 @@ struct ad7768_state {
 	struct iio_trigger *trig;
 	struct gpio_desc *gpio_sync_in;
 	struct gpio_desc *gpio_reset;
+	bool en_spi_sync;
 	const char *labels[ARRAY_SIZE(ad7768_channels)];
 	/*
 	 * DMA (thus cache coherency maintenance) may require the
@@ -292,6 +293,19 @@ static const struct regmap_config ad7768_regmap24_config = {
 	.max_register = AD7768_REG24_COEFF_DATA,
 };
 
+static int ad7768_send_sync_pulse(struct ad7768_state *st)
+{
+	if (st->en_spi_sync)
+		return regmap_write(st->regmap, AD7768_REG_SYNC_RESET, 0x00);
+
+	if (st->gpio_sync_in) {
+		gpiod_set_value_cansleep(st->gpio_sync_in, 1);
+		gpiod_set_value_cansleep(st->gpio_sync_in, 0);
+	}
+
+	return 0;
+}
+
 static int ad7768_set_mode(struct ad7768_state *st,
 			   enum ad7768_conv_mode mode)
 {
@@ -389,10 +403,7 @@ static int ad7768_set_dig_fil(struct ad7768_state *st,
 		return ret;
 
 	/* A sync-in pulse is required every time the filter dec rate changes */
-	gpiod_set_value(st->gpio_sync_in, 1);
-	gpiod_set_value(st->gpio_sync_in, 0);
-
-	return 0;
+	return ad7768_send_sync_pulse(st);
 }
 
 static int ad7768_gpio_direction_input(struct gpio_chip *chip, unsigned int offset)
@@ -673,6 +684,60 @@ static const struct iio_info ad7768_info = {
 	.debugfs_reg_access = &ad7768_reg_access,
 };
 
+static int ad7768_setup_spi_sync(struct device *dev, struct ad7768_state *st)
+{
+	struct fwnode_reference_args args;
+	int ret;
+
+	ret = fwnode_property_get_reference_args(dev_fwnode(dev),
+						 "trigger-sources",
+						 "#trigger-source-cells",
+						 0, 0, &args);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to get trigger-sources reference\n");
+
+	/*
+	 * Currently, the driver supports SPI-based synchronization only for
+	 * single-device setups, where the device's own SYNC_OUT is looped back
+	 * to its SYNC_IN. Only enable this feature if the trigger-sources
+	 * references the current device.
+	 */
+	st->en_spi_sync = args.fwnode->dev == dev;
+	fwnode_handle_put(args.fwnode);
+
+	return st->en_spi_sync ? 0 : -EOPNOTSUPP;
+}
+
+static int ad7768_set_sync_source(struct device *dev, struct ad7768_state *st)
+{
+	int ret;
+
+	/*
+	 * The AD7768-1 allows two primary methods for driving the SYNC_IN pin
+	 * to synchronize one or more devices:
+	 * 1. Using a GPIO to directly drive the SYNC_IN pin.
+	 * 2. Using a SPI command, where the SYNC_OUT pin generates a
+	 *    synchronization pulse that loops back to the SYNC_IN pin.
+	 */
+	st->gpio_sync_in = devm_gpiod_get_optional(dev, "adi,sync-in",
+						   GPIOD_OUT_LOW);
+	if (IS_ERR(st->gpio_sync_in))
+		return PTR_ERR(st->gpio_sync_in);
+
+	/*
+	 * If the SYNC_IN GPIO is not defined, fall back to synchronization
+	 * over SPI.
+	 */
+	if (!st->gpio_sync_in) {
+		ret = ad7768_setup_spi_sync(dev, st);
+		if (ret)
+			return dev_err_probe(dev, ret,
+					      "No valid synchronization source provided\n");
+	}
+
+	return 0;
+}
+
 static int ad7768_setup(struct iio_dev *indio_dev)
 {
 	struct ad7768_state *st = iio_priv(indio_dev);
@@ -703,10 +768,9 @@ static int ad7768_setup(struct iio_dev *indio_dev)
 			return ret;
 	}
 
-	st->gpio_sync_in = devm_gpiod_get(&st->spi->dev, "adi,sync-in",
-					  GPIOD_OUT_LOW);
-	if (IS_ERR(st->gpio_sync_in))
-		return PTR_ERR(st->gpio_sync_in);
+	ret = ad7768_set_sync_source(&st->spi->dev, st);
+	if (ret)
+		return ret;
 
 	/* Only create a Chip GPIO if flagged for it */
 	if (device_property_read_bool(&st->spi->dev, "gpio-controller")) {
-- 
2.34.1

[PATCH v4 15/17] iio: adc: ad7768-1: replace manual attribute declaration

[Jonathan Santos]

Use read_avail callback from struct iio_info to replace the manual
declaration of sampling_frequency_available attribute.

Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com>
---
v4 Changes:
* Added ad7768_fill_samp_freq_tbl() helper function.
* Sampling frequency table is precomputed at probe.

v3 Changes:
* New patch in v3.
---
 drivers/iio/adc/ad7768-1.c | 63 +++++++++++++++++++-------------------
 1 file changed, 31 insertions(+), 32 deletions(-)

diff --git a/drivers/iio/adc/ad7768-1.c b/drivers/iio/adc/ad7768-1.c
index c48d3e0af985..0bdf2ae903c6 100644
--- a/drivers/iio/adc/ad7768-1.c
+++ b/drivers/iio/adc/ad7768-1.c
@@ -187,6 +187,7 @@ static const struct iio_chan_spec ad7768_channels[] = {
 		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
 		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
 		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),
+		.info_mask_shared_by_all_available = BIT(IIO_CHAN_INFO_SAMP_FREQ),
 		.indexed = 1,
 		.channel = 0,
 		.scan_index = 0,
@@ -208,6 +209,7 @@ struct ad7768_state {
 	unsigned int mclk_freq;
 	unsigned int dec_rate;
 	unsigned int samp_freq;
+	unsigned int samp_freq_avail[ARRAY_SIZE(ad7768_clk_config)];
 	struct completion completion;
 	struct iio_trigger *trig;
 	struct gpio_desc *gpio_sync_in;
@@ -306,6 +308,15 @@ static int ad7768_send_sync_pulse(struct ad7768_state *st)
 	return 0;
 }
 
+static void ad7768_fill_samp_freq_tbl(struct ad7768_state *st)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(ad7768_clk_config); i++)
+		st->samp_freq_avail[i] = DIV_ROUND_CLOSEST(st->mclk_freq,
+							   ad7768_clk_config[i].clk_div);
+}
+
 static int ad7768_set_mode(struct ad7768_state *st,
 			   enum ad7768_conv_mode mode)
 {
@@ -566,28 +577,6 @@ static int ad7768_set_freq(struct ad7768_state *st,
 	return 0;
 }
 
-static ssize_t ad7768_sampling_freq_avail(struct device *dev,
-					  struct device_attribute *attr,
-					  char *buf)
-{
-	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
-	struct ad7768_state *st = iio_priv(indio_dev);
-	unsigned int freq;
-	int i, len = 0;
-
-	for (i = 0; i < ARRAY_SIZE(ad7768_clk_config); i++) {
-		freq = DIV_ROUND_CLOSEST(st->mclk_freq,
-					 ad7768_clk_config[i].clk_div);
-		len += scnprintf(buf + len, PAGE_SIZE - len, "%d ", freq);
-	}
-
-	buf[len - 1] = '\n';
-
-	return len;
-}
-
-static IIO_DEV_ATTR_SAMP_FREQ_AVAIL(ad7768_sampling_freq_avail);
-
 static int ad7768_read_raw(struct iio_dev *indio_dev,
 			   struct iio_chan_spec const *chan,
 			   int *val, int *val2, long info)
@@ -635,6 +624,24 @@ static int ad7768_read_raw(struct iio_dev *indio_dev,
 	return -EINVAL;
 }
 
+static int ad7768_read_avail(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan,
+			     const int **vals, int *type, int *length,
+			     long info)
+{
+	struct ad7768_state *st = iio_priv(indio_dev);
+
+	switch (info) {
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		*vals = (int *)st->samp_freq_avail;
+		*length = ARRAY_SIZE(ad7768_clk_config);
+		*type = IIO_VAL_INT;
+		return IIO_AVAIL_LIST;
+	default:
+		return -EINVAL;
+	}
+}
+
 static int ad7768_write_raw(struct iio_dev *indio_dev,
 			    struct iio_chan_spec const *chan,
 			    int val, int val2, long info)
@@ -657,15 +664,6 @@ static int ad7768_read_label(struct iio_dev *indio_dev,
 	return sprintf(label, "%s\n", st->labels[chan->channel]);
 }
 
-static struct attribute *ad7768_attributes[] = {
-	&iio_dev_attr_sampling_frequency_available.dev_attr.attr,
-	NULL
-};
-
-static const struct attribute_group ad7768_group = {
-	.attrs = ad7768_attributes,
-};
-
 static int ad7768_get_current_scan_type(const struct iio_dev *indio_dev,
 					const struct iio_chan_spec *chan)
 {
@@ -676,8 +674,8 @@ static int ad7768_get_current_scan_type(const struct iio_dev *indio_dev,
 }
 
 static const struct iio_info ad7768_info = {
-	.attrs = &ad7768_group,
 	.read_raw = &ad7768_read_raw,
+	.read_avail = &ad7768_read_avail,
 	.write_raw = &ad7768_write_raw,
 	.read_label = ad7768_read_label,
 	.get_current_scan_type = &ad7768_get_current_scan_type,
@@ -1134,6 +1132,7 @@ static int ad7768_probe(struct spi_device *spi)
 		return PTR_ERR(st->mclk);
 
 	st->mclk_freq = clk_get_rate(st->mclk);
+	ad7768_fill_samp_freq_tbl(st);
 
 	indio_dev->channels = ad7768_channels;
 	indio_dev->num_channels = ARRAY_SIZE(ad7768_channels);
-- 
2.34.1

[PATCH v4 16/17] iio: adc: ad7768-1: add filter type and oversampling ratio attributes

[Jonathan Santos]

Separate filter type and decimation rate from the sampling frequency
attribute. The new filter type attribute enables sinc3, sinc3+rej60
and wideband filters, which were previously unavailable.

Previously, combining decimation and MCLK divider in the sampling
frequency obscured performance trade-offs. Lower MCLK divider
settings increase power usage, while lower decimation rates reduce
precision by decreasing averaging. By creating an oversampling
attribute, which controls the decimation, users gain finer control
over performance.

The addition of those attributes allows a wider range of sampling
frequencies and more access to the device features. Sampling frequency
table is updated after every digital filter paramerter change.

Co-developed-by: Pop Paul <paul.pop@analog.com>
Signed-off-by: Pop Paul <paul.pop@analog.com>
Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com>
---
v4 Changes:
* Sampling frequency table is dinamically updated after every
  filter configuration.

v3 Changes:
* removed unsed variables.
* included sinc3+rej60 filter type.
* oversampling_ratio moved to info_mask_shared_by_type.
* reordered functions to avoid foward declaration.
* simplified regmap writes.
* Removed locking.
* replaced some helper functions for direct regmap_update_bits
  calls.
* Addressed other nits.

v2 Changes:
* Decimation_rate attribute replaced for oversampling_ratio.
---
 drivers/iio/adc/ad7768-1.c | 360 +++++++++++++++++++++++++++++--------
 1 file changed, 290 insertions(+), 70 deletions(-)

diff --git a/drivers/iio/adc/ad7768-1.c b/drivers/iio/adc/ad7768-1.c
index 0bdf2ae903c6..664d0b535340 100644
--- a/drivers/iio/adc/ad7768-1.c
+++ b/drivers/iio/adc/ad7768-1.c
@@ -20,6 +20,7 @@
 #include <linux/regulator/driver.h>
 #include <linux/sysfs.h>
 #include <linux/spi/spi.h>
+#include <linux/util_macros.h>
 
 #include <linux/iio/buffer.h>
 #include <linux/iio/iio.h>
@@ -75,11 +76,15 @@
 #define AD7768_PWR_PWRMODE(x)		FIELD_PREP(AD7768_PWR_PWRMODE_MSK, x)
 
 /* AD7768_REG_DIGITAL_FILTER */
-#define AD7768_DIG_FIL_FIL_MSK		GENMASK(6, 4)
+#define AD7768_DIG_FIL_FIL_MSK		GENMASK(7, 4)
 #define AD7768_DIG_FIL_FIL(x)		FIELD_PREP(AD7768_DIG_FIL_FIL_MSK, x)
 #define AD7768_DIG_FIL_DEC_MSK		GENMASK(2, 0)
 #define AD7768_DIG_FIL_DEC_RATE(x)	FIELD_PREP(AD7768_DIG_FIL_DEC_MSK, x)
 
+/* AD7768_SINC3_DEC_RATE */
+#define AD7768_SINC3_DEC_RATE_MSB_MSK	GENMASK(12, 8)
+#define AD7768_SINC3_DEC_RATE_LSB_MSK	GENMASK(7, 0)
+
 /* AD7768_REG_CONVERSION */
 #define AD7768_CONV_MODE_MSK		GENMASK(2, 0)
 #define AD7768_CONV_MODE(x)		FIELD_PREP(AD7768_CONV_MODE_MSK, x)
@@ -124,22 +129,20 @@ enum ad7768_mclk_div {
 	AD7768_MCLK_DIV_2
 };
 
-enum ad7768_dec_rate {
-	AD7768_DEC_RATE_32 = 0,
-	AD7768_DEC_RATE_64 = 1,
-	AD7768_DEC_RATE_128 = 2,
-	AD7768_DEC_RATE_256 = 3,
-	AD7768_DEC_RATE_512 = 4,
-	AD7768_DEC_RATE_1024 = 5,
-	AD7768_DEC_RATE_8 = 9,
-	AD7768_DEC_RATE_16 = 10
+enum ad7768_filter_type {
+	AD7768_FILTER_SINC5,
+	AD7768_FILTER_SINC3,
+	AD7768_FILTER_WIDEBAND,
+	AD7768_FILTER_SINC3_REJ60,
 };
 
-struct ad7768_clk_configuration {
-	enum ad7768_mclk_div mclk_div;
-	enum ad7768_dec_rate dec_rate;
-	unsigned int clk_div;
-	enum ad7768_pwrmode pwrmode;
+enum ad7768_filter_regval {
+	AD7768_FILTER_REGVAL_SINC5 = 0,
+	AD7768_FILTER_REGVAL_SINC5_X8 = 1,
+	AD7768_FILTER_REGVAL_SINC5_X16 = 2,
+	AD7768_FILTER_REGVAL_SINC3 = 3,
+	AD7768_FILTER_REGVAL_WIDEBAND = 4,
+	AD7768_FILTER_REGVAL_SINC3_REJ60 = 11,
 };
 
 enum ad7768_scan_type {
@@ -151,18 +154,39 @@ static const int ad7768_mclk_div_rates[4] = {
 	16, 8, 4, 2,
 };
 
-static const struct ad7768_clk_configuration ad7768_clk_config[] = {
-	{ AD7768_MCLK_DIV_2, AD7768_DEC_RATE_8, 16,  AD7768_FAST_MODE },
-	{ AD7768_MCLK_DIV_2, AD7768_DEC_RATE_16, 32,  AD7768_FAST_MODE },
-	{ AD7768_MCLK_DIV_2, AD7768_DEC_RATE_32, 64, AD7768_FAST_MODE },
-	{ AD7768_MCLK_DIV_2, AD7768_DEC_RATE_64, 128, AD7768_FAST_MODE },
-	{ AD7768_MCLK_DIV_2, AD7768_DEC_RATE_128, 256, AD7768_FAST_MODE },
-	{ AD7768_MCLK_DIV_4, AD7768_DEC_RATE_128, 512, AD7768_MED_MODE },
-	{ AD7768_MCLK_DIV_4, AD7768_DEC_RATE_256, 1024, AD7768_MED_MODE },
-	{ AD7768_MCLK_DIV_4, AD7768_DEC_RATE_512, 2048, AD7768_MED_MODE },
-	{ AD7768_MCLK_DIV_4, AD7768_DEC_RATE_1024, 4096, AD7768_MED_MODE },
-	{ AD7768_MCLK_DIV_8, AD7768_DEC_RATE_1024, 8192, AD7768_MED_MODE },
-	{ AD7768_MCLK_DIV_16, AD7768_DEC_RATE_1024, 16384, AD7768_ECO_MODE },
+static const int ad7768_dec_rate_values[8] = {
+	8, 16, 32, 64, 128, 256, 512, 1024,
+};
+
+/* Decimation Rate range for each filter type */
+static const int ad7768_dec_rate_range[][3] = {
+	[AD7768_FILTER_SINC5] = { 8, 8, 1024 },
+	[AD7768_FILTER_SINC3] = { 32, 32, 163840 },
+	[AD7768_FILTER_WIDEBAND] = { 32, 32, 1024 },
+	[AD7768_FILTER_SINC3_REJ60] = { 32, 32, 163840 },
+};
+
+/*
+ * The AD7768-1 supports three primary filter types:
+ * Sinc5, Sinc3, and Wideband.
+ * However, the filter register values can also encode additional parameters
+ * such as decimation rates and 60Hz rejection. This utility function separates
+ * the filter type from these parameters.
+ */
+static const int ad7768_filter_regval_to_type[] = {
+	[AD7768_FILTER_REGVAL_SINC5] = AD7768_FILTER_SINC5,
+	[AD7768_FILTER_REGVAL_SINC5_X8] = AD7768_FILTER_SINC5,
+	[AD7768_FILTER_REGVAL_SINC5_X16] = AD7768_FILTER_SINC5,
+	[AD7768_FILTER_REGVAL_SINC3] = AD7768_FILTER_SINC3,
+	[AD7768_FILTER_REGVAL_WIDEBAND] = AD7768_FILTER_WIDEBAND,
+	[AD7768_FILTER_REGVAL_SINC3_REJ60] = AD7768_FILTER_SINC3_REJ60,
+};
+
+static const char * const ad7768_filter_enum[] = {
+	[AD7768_FILTER_SINC5] = "sinc5",
+	[AD7768_FILTER_SINC3] = "sinc3",
+	[AD7768_FILTER_WIDEBAND] = "wideband",
+	[AD7768_FILTER_SINC3_REJ60] = "sinc3+rej60"
 };
 
 static const struct iio_scan_type ad7768_scan_type[] = {
@@ -181,13 +205,34 @@ static const struct iio_scan_type ad7768_scan_type[] = {
 	},
 };
 
+static int ad7768_get_fil_type_attr(struct iio_dev *dev,
+				    const struct iio_chan_spec *chan);
+static int ad7768_set_fil_type_attr(struct iio_dev *dev,
+				    const struct iio_chan_spec *chan, unsigned int filter);
+
+static const struct iio_enum ad7768_flt_type_iio_enum = {
+	.items = ad7768_filter_enum,
+	.num_items = ARRAY_SIZE(ad7768_filter_enum),
+	.set = ad7768_set_fil_type_attr,
+	.get = ad7768_get_fil_type_attr,
+};
+
+static struct iio_chan_spec_ext_info ad7768_ext_info[] = {
+	IIO_ENUM("filter_type", IIO_SHARED_BY_ALL, &ad7768_flt_type_iio_enum),
+	IIO_ENUM_AVAILABLE("filter_type", IIO_SHARED_BY_ALL, &ad7768_flt_type_iio_enum),
+	{ },
+};
+
 static const struct iio_chan_spec ad7768_channels[] = {
 	{
 		.type = IIO_VOLTAGE,
 		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
-		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
+		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |
+					    BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
+		.info_mask_shared_by_type_available = BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
 		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),
 		.info_mask_shared_by_all_available = BIT(IIO_CHAN_INFO_SAMP_FREQ),
+		.ext_info = ad7768_ext_info,
 		.indexed = 1,
 		.channel = 0,
 		.scan_index = 0,
@@ -207,9 +252,12 @@ struct ad7768_state {
 	struct clk *mclk;
 	struct gpio_chip gpiochip;
 	unsigned int mclk_freq;
-	unsigned int dec_rate;
+	unsigned int mclk_div;
+	unsigned int oversampling_ratio;
+	enum ad7768_filter_type filter_type;
 	unsigned int samp_freq;
-	unsigned int samp_freq_avail[ARRAY_SIZE(ad7768_clk_config)];
+	unsigned int samp_freq_avail[ARRAY_SIZE(ad7768_mclk_div_rates)];
+	unsigned int samp_freq_avail_len;
 	struct completion completion;
 	struct iio_trigger *trig;
 	struct gpio_desc *gpio_sync_in;
@@ -310,11 +358,35 @@ static int ad7768_send_sync_pulse(struct ad7768_state *st)
 
 static void ad7768_fill_samp_freq_tbl(struct ad7768_state *st)
 {
-	int i;
+	int i, freq_filtered, len = 0;
+
+	freq_filtered = DIV_ROUND_CLOSEST(st->mclk_freq, st->oversampling_ratio);
+	for (i = 0; i < ARRAY_SIZE(ad7768_mclk_div_rates); i++) {
+		st->samp_freq_avail[len] = DIV_ROUND_CLOSEST(freq_filtered,
+							     ad7768_mclk_div_rates[i]);
+		/* Sampling frequency cannot be lower than the minimum of 50 SPS */
+		if (st->samp_freq_avail[len] >= 50)
+			len++;
+	}
+	st->samp_freq_avail_len = len;
+}
+
+static int ad7768_set_mclk_div(struct ad7768_state *st, unsigned int mclk_div)
+{
+	unsigned int mclk_div_value;
+
+	mclk_div_value = AD7768_PWR_MCLK_DIV(mclk_div);
+	/*
+	 * Set power mode based on mclk_div value.
+	 * ECO_MODE is only recommended for MCLK_DIV 16
+	 */
+	mclk_div_value |= mclk_div > AD7768_MCLK_DIV_16 ?
+			  AD7768_PWR_PWRMODE(AD7768_FAST_MODE) :
+			  AD7768_PWR_PWRMODE(AD7768_ECO_MODE);
 
-	for (i = 0; i < ARRAY_SIZE(ad7768_clk_config); i++)
-		st->samp_freq_avail[i] = DIV_ROUND_CLOSEST(st->mclk_freq,
-							   ad7768_clk_config[i].clk_div);
+	return regmap_update_bits(st->regmap, AD7768_REG_POWER_CLOCK,
+				  AD7768_PWR_MCLK_DIV_MSK | AD7768_PWR_PWRMODE_MSK,
+				  mclk_div_value);
 }
 
 static int ad7768_set_mode(struct ad7768_state *st,
@@ -350,7 +422,7 @@ static int ad7768_scan_direct(struct iio_dev *indio_dev)
 	 * register provides 24-bit data, the precision is reduced by
 	 * right-shifting the read value by 8 bits.
 	 */
-	if (st->dec_rate == 8)
+	if (st->oversampling_ratio == 8)
 		readval >>= 8;
 
 	/*
@@ -398,22 +470,103 @@ static int ad7768_reg_access(struct iio_dev *indio_dev,
 	return ret;
 }
 
-static int ad7768_set_dig_fil(struct ad7768_state *st,
-			      enum ad7768_dec_rate dec_rate)
+static int ad7768_set_sinc3_dec_rate(struct ad7768_state *st,
+				     unsigned int dec_rate)
 {
-	unsigned int mode;
+	unsigned int max_dec_rate;
+	u8 dec_rate_reg[2];
 	int ret;
 
-	if (dec_rate == AD7768_DEC_RATE_8 || dec_rate == AD7768_DEC_RATE_16)
-		mode = AD7768_DIG_FIL_FIL(dec_rate);
-	else
-		mode = AD7768_DIG_FIL_DEC_RATE(dec_rate);
+	/*
+	 * Maximum dec_rate is limited by the MCLK_DIV value
+	 * and by the ODR. The edge case is for MCLK_DIV = 2
+	 * ODR = 50 SPS.
+	 * max_dec_rate <= MCLK / (2 * 50)
+	 */
+	max_dec_rate = st->mclk_freq / 100;
+	dec_rate = clamp_t(unsigned int, dec_rate, 32, max_dec_rate);
+	/*
+	 * Calculate the equivalent value to sinc3 decimation ratio
+	 * to be written on the SINC3_DECIMATION_RATE register:
+	 *  Value = (DEC_RATE / 32) -1
+	 */
+	dec_rate = DIV_ROUND_UP(dec_rate, 32) - 1;
+	dec_rate_reg[0] = FIELD_GET(AD7768_SINC3_DEC_RATE_MSB_MSK, dec_rate);
+	dec_rate_reg[1] = FIELD_GET(AD7768_SINC3_DEC_RATE_LSB_MSK, dec_rate);
+	ret = regmap_bulk_write(st->regmap, AD7768_REG_SINC3_DEC_RATE_MSB,
+				dec_rate_reg, 2);
+	if (ret)
+		return ret;
 
-	ret = regmap_write(st->regmap, AD7768_REG_DIGITAL_FILTER, mode);
-	if (ret < 0)
+	st->oversampling_ratio = (dec_rate + 1) * 32;
+
+	return 0;
+}
+
+static int ad7768_configure_dig_fil(struct iio_dev *dev,
+				    enum ad7768_filter_type filter_type,
+				    unsigned int dec_rate)
+{
+	struct ad7768_state *st = iio_priv(dev);
+	unsigned int dec_rate_idx, dig_filter_regval;
+	int ret;
+
+	switch (filter_type) {
+	case AD7768_FILTER_SINC3:
+		dig_filter_regval = AD7768_DIG_FIL_FIL(AD7768_FILTER_REGVAL_SINC3);
+		break;
+	case AD7768_FILTER_SINC3_REJ60:
+		dig_filter_regval = AD7768_DIG_FIL_FIL(AD7768_FILTER_REGVAL_SINC3_REJ60);
+		break;
+	case AD7768_FILTER_WIDEBAND:
+		/* Skip decimations 8 and 16, not supported by the wideband filter */
+		dec_rate_idx = find_closest(dec_rate, &ad7768_dec_rate_values[2],
+					    ARRAY_SIZE(ad7768_dec_rate_values) - 2);
+		dig_filter_regval = AD7768_DIG_FIL_FIL(AD7768_FILTER_REGVAL_WIDEBAND) |
+				    AD7768_DIG_FIL_DEC_RATE(dec_rate_idx);
+		/* Correct the index offset */
+		dec_rate_idx += 2;
+		break;
+	case AD7768_FILTER_SINC5:
+		dec_rate_idx = find_closest(dec_rate, ad7768_dec_rate_values,
+					    ARRAY_SIZE(ad7768_dec_rate_values));
+
+		/*
+		 * Decimations 8 (idx 0) and 16 (idx 1) are set in the
+		 * FILTER[6:4] field. The other decimations are set in the
+		 * DEC_RATE[2:0] field, and the idx need to be offsetted by two.
+		 */
+		if (dec_rate_idx == 0)
+			dig_filter_regval = AD7768_DIG_FIL_FIL(AD7768_FILTER_REGVAL_SINC5_X8);
+		else if (dec_rate_idx == 1)
+			dig_filter_regval = AD7768_DIG_FIL_FIL(AD7768_FILTER_REGVAL_SINC5_X16);
+		else
+			dig_filter_regval = AD7768_DIG_FIL_FIL(AD7768_FILTER_REGVAL_SINC5) |
+					    AD7768_DIG_FIL_DEC_RATE(dec_rate_idx - 2);
+		break;
+	}
+
+	ret = regmap_write(st->regmap, AD7768_REG_DIGITAL_FILTER, dig_filter_regval);
+	if (ret)
 		return ret;
 
-	/* A sync-in pulse is required every time the filter dec rate changes */
+	st->filter_type = filter_type;
+	/*
+	 * The decimation for SINC3 filters are configured in different
+	 * registers
+	 */
+	if (filter_type == AD7768_FILTER_SINC3 ||
+	    filter_type == AD7768_FILTER_SINC3_REJ60) {
+		ret = ad7768_set_sinc3_dec_rate(st, dec_rate);
+		if (ret)
+			return ret;
+	} else {
+		st->oversampling_ratio =  ad7768_dec_rate_values[dec_rate_idx];
+	}
+
+	ad7768_fill_samp_freq_tbl(st);
+
+	/* A sync-in pulse is required after every configuration change */
 	return ad7768_send_sync_pulse(st);
 }
 
@@ -538,43 +691,69 @@ static int ad7768_gpio_init(struct iio_dev *indio_dev)
 static int ad7768_set_freq(struct ad7768_state *st,
 			   unsigned int freq)
 {
-	unsigned int diff_new, diff_old, pwr_mode, i, idx;
+	unsigned int diff_new, diff_old, i, idx;
 	int res, ret;
 
+	freq = clamp_t(unsigned int, freq, 50, 1024000);
 	diff_old = U32_MAX;
 	idx = 0;
 
-	res = DIV_ROUND_CLOSEST(st->mclk_freq, freq);
+	if (freq == 0)
+		return -EINVAL;
+
+	res = DIV_ROUND_CLOSEST(st->mclk_freq, freq * st->oversampling_ratio);
 
 	/* Find the closest match for the desired sampling frequency */
-	for (i = 0; i < ARRAY_SIZE(ad7768_clk_config); i++) {
-		diff_new = abs(res - ad7768_clk_config[i].clk_div);
+	for (i = 0; i < ARRAY_SIZE(ad7768_mclk_div_rates); i++) {
+		diff_new = abs(res - ad7768_mclk_div_rates[i]);
 		if (diff_new < diff_old) {
 			diff_old = diff_new;
 			idx = i;
 		}
 	}
 
-	/*
-	 * Set both the mclk_div and pwrmode with a single write to the
-	 * POWER_CLOCK register
-	 */
-	pwr_mode = AD7768_PWR_MCLK_DIV(ad7768_clk_config[idx].mclk_div) |
-		   AD7768_PWR_PWRMODE(ad7768_clk_config[idx].pwrmode);
-	ret = regmap_write(st->regmap, AD7768_REG_POWER_CLOCK, pwr_mode);
-	if (ret < 0)
+	/* Set both the mclk_div and pwrmode */
+	ret = ad7768_set_mclk_div(st, idx);
+	if (ret)
 		return ret;
 
-	ret =  ad7768_set_dig_fil(st, ad7768_clk_config[idx].dec_rate);
-	if (ret < 0)
+	st->samp_freq = DIV_ROUND_CLOSEST(st->mclk_freq,
+					  ad7768_mclk_div_rates[idx] * st->oversampling_ratio);
+
+	/* A sync-in pulse is required after every configuration change */
+	return ad7768_send_sync_pulse(st);
+}
+
+static int ad7768_set_fil_type_attr(struct iio_dev *dev,
+				    const struct iio_chan_spec *chan,
+				    unsigned int filter)
+{
+	struct ad7768_state *st = iio_priv(dev);
+	int ret;
+
+	ret = ad7768_configure_dig_fil(dev, filter, st->oversampling_ratio);
+	if (ret)
 		return ret;
 
-	st->dec_rate = ad7768_clk_config[idx].clk_div /
-		       ad7768_mclk_div_rates[ad7768_clk_config[idx].mclk_div];
-	st->samp_freq = DIV_ROUND_CLOSEST(st->mclk_freq,
-					  ad7768_clk_config[idx].clk_div);
+	/* Update sampling frequency */
+	return ad7768_set_freq(st, st->samp_freq);
+}
 
-	return 0;
+static int ad7768_get_fil_type_attr(struct iio_dev *dev,
+				    const struct iio_chan_spec *chan)
+{
+	struct ad7768_state *st = iio_priv(dev);
+	int ret;
+	unsigned int mode;
+
+	ret = regmap_read(st->regmap, AD7768_REG_DIGITAL_FILTER, &mode);
+	if (ret)
+		return ret;
+
+	mode = FIELD_GET(AD7768_DIG_FIL_FIL_MSK, mode);
+
+	/* From the register value, get the corresponding filter type */
+	return ad7768_filter_regval_to_type[mode];
 }
 
 static int ad7768_read_raw(struct iio_dev *indio_dev,
@@ -618,6 +797,11 @@ static int ad7768_read_raw(struct iio_dev *indio_dev,
 	case IIO_CHAN_INFO_SAMP_FREQ:
 		*val = st->samp_freq;
 
+		return IIO_VAL_INT;
+
+	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
+		*val = st->oversampling_ratio;
+
 		return IIO_VAL_INT;
 	}
 
@@ -632,9 +816,13 @@ static int ad7768_read_avail(struct iio_dev *indio_dev,
 	struct ad7768_state *st = iio_priv(indio_dev);
 
 	switch (info) {
+	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
+		*vals = (int *)ad7768_dec_rate_range[st->filter_type];
+		*type = IIO_VAL_INT;
+		return IIO_AVAIL_RANGE;
 	case IIO_CHAN_INFO_SAMP_FREQ:
 		*vals = (int *)st->samp_freq_avail;
-		*length = ARRAY_SIZE(ad7768_clk_config);
+		*length = st->samp_freq_avail_len;
 		*type = IIO_VAL_INT;
 		return IIO_AVAIL_LIST;
 	default:
@@ -642,20 +830,45 @@ static int ad7768_read_avail(struct iio_dev *indio_dev,
 	}
 }
 
-static int ad7768_write_raw(struct iio_dev *indio_dev,
-			    struct iio_chan_spec const *chan,
-			    int val, int val2, long info)
+static int __ad7768_write_raw(struct iio_dev *indio_dev,
+			      struct iio_chan_spec const *chan,
+			      int val, int val2, long info)
 {
 	struct ad7768_state *st = iio_priv(indio_dev);
+	int ret;
 
 	switch (info) {
 	case IIO_CHAN_INFO_SAMP_FREQ:
 		return ad7768_set_freq(st, val);
+
+	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
+		ret = ad7768_configure_dig_fil(indio_dev, st->filter_type, val);
+		if (ret)
+			return ret;
+
+		/* Update sampling frequency */
+		return ad7768_set_freq(st, st->samp_freq);
 	default:
 		return -EINVAL;
 	}
 }
 
+static int ad7768_write_raw(struct iio_dev *indio_dev,
+			    struct iio_chan_spec const *chan,
+			    int val, int val2, long info)
+{
+	int ret;
+
+	ret = iio_device_claim_direct_mode(indio_dev);
+	if (ret)
+		return ret;
+
+	ret = __ad7768_write_raw(indio_dev, chan, val, val2, info);
+	iio_device_release_direct_mode(indio_dev);
+
+	return ret;
+}
+
 static int ad7768_read_label(struct iio_dev *indio_dev,
 	const struct iio_chan_spec *chan, char *label)
 {
@@ -669,7 +882,7 @@ static int ad7768_get_current_scan_type(const struct iio_dev *indio_dev,
 {
 	struct ad7768_state *st = iio_priv(indio_dev);
 
-	return st->dec_rate == 8 ? AD7768_SCAN_TYPE_HIGH_SPEED :
+	return st->oversampling_ratio == 8 ? AD7768_SCAN_TYPE_HIGH_SPEED :
 		AD7768_SCAN_TYPE_NORMAL;
 }
 
@@ -777,6 +990,14 @@ static int ad7768_setup(struct iio_dev *indio_dev)
 			return ret;
 	}
 
+	/*
+	 * Set Default Digital Filter configuration:
+	 * SINC5 filter with x32 Decimation rate
+	 */
+	ret = ad7768_configure_dig_fil(indio_dev, AD7768_FILTER_SINC5, 32);
+	if (ret)
+		return ret;
+
 	/* Set the default sampling frequency to 32000 kSPS */
 	return ad7768_set_freq(st, 32000);
 }
@@ -1132,7 +1353,6 @@ static int ad7768_probe(struct spi_device *spi)
 		return PTR_ERR(st->mclk);
 
 	st->mclk_freq = clk_get_rate(st->mclk);
-	ad7768_fill_samp_freq_tbl(st);
 
 	indio_dev->channels = ad7768_channels;
 	indio_dev->num_channels = ARRAY_SIZE(ad7768_channels);
-- 
2.34.1

[PATCH v4 17/17] iio: adc: ad7768-1: add low pass -3dB cutoff attribute

[Jonathan Santos]

Ad7768-1 has a different -3db frequency multiplier depending on
the filter type configured. The cutoff frequency also varies according
to the current ODR.

Add a readonly low pass -3dB frequency cutoff attribute to clarify to
the user which bandwidth is being allowed depending on the filter
configurations.

Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com>
---
v4 Changes:
* None

v3 Changes:
* None

v2 Changes:
* New patch in v2.
---
 drivers/iio/adc/ad7768-1.c | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/adc/ad7768-1.c b/drivers/iio/adc/ad7768-1.c
index 664d0b535340..9ba00e9089bd 100644
--- a/drivers/iio/adc/ad7768-1.c
+++ b/drivers/iio/adc/ad7768-1.c
@@ -150,6 +150,17 @@ enum ad7768_scan_type {
 	AD7768_SCAN_TYPE_HIGH_SPEED,
 };
 
+/*
+ * -3dB cutoff frequency multipliers (relative to ODR) for
+ * each filter type. Values are multiplied by 1000.
+ */
+static const int ad7768_filter_3db_odr_multiplier[] = {
+	[AD7768_FILTER_SINC5] = 204,
+	[AD7768_FILTER_SINC3] = 261,
+	[AD7768_FILTER_SINC3_REJ60] = 261,
+	[AD7768_FILTER_WIDEBAND] = 433,
+};
+
 static const int ad7768_mclk_div_rates[4] = {
 	16, 8, 4, 2,
 };
@@ -228,7 +239,8 @@ static const struct iio_chan_spec ad7768_channels[] = {
 		.type = IIO_VOLTAGE,
 		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
 		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |
-					    BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
+					    BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO) |
+					    BIT(IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY),
 		.info_mask_shared_by_type_available = BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
 		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),
 		.info_mask_shared_by_all_available = BIT(IIO_CHAN_INFO_SAMP_FREQ),
@@ -762,7 +774,7 @@ static int ad7768_read_raw(struct iio_dev *indio_dev,
 {
 	struct ad7768_state *st = iio_priv(indio_dev);
 	const struct iio_scan_type *scan_type;
-	int scale_uv, ret;
+	int scale_uv, ret, temp;
 
 	scan_type = iio_get_current_scan_type(indio_dev, chan);
 	if (IS_ERR(scan_type))
@@ -802,6 +814,12 @@ static int ad7768_read_raw(struct iio_dev *indio_dev,
 	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
 		*val = st->oversampling_ratio;
 
+		return IIO_VAL_INT;
+
+	case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
+		temp = st->samp_freq * ad7768_filter_3db_odr_multiplier[st->filter_type];
+		*val = DIV_ROUND_CLOSEST(temp, 1000);
+
 		return IIO_VAL_INT;
 	}
 
-- 
2.34.1

Re: [PATCH v4 02/17] iio: adc: ad7768-1: set MOSI idle state to prevent accidental reset

[Marcelo Schmitt]

On 03/06, Jonathan Santos wrote:
> Datasheet recommends Setting the MOSI idle state to high in order to
> prevent accidental reset of the device when SCLK is free running.
> This happens when the controller clocks out a 1 followed by 63 zeros
> while the CS is held low.
> 
> Check if SPI controller supports SPI_MOSI_IDLE_HIGH flag and set it.
> 
> Fixes: a5f8c7da3dbe ("iio: adc: Add AD7768-1 ADC basic support")
> Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com>
> ---

LGTM

Reviewed-by: Marcelo Schmitt <marcelo.schmitt@analog.com>

> v4 Changes:
> * None.
> 
> v3 Changes:
> * Patch moved closer to the start of the patch set.
> 
> v2 Changes:
> * Only setup SPI_MOSI_IDLE_HIGH flag if the controller supports it, otherwise the driver
>   continues the same. I realized that using bits_per_word does not avoid the problem that
>   MOSI idle state is trying to solve. If the controller drives the MOSI low between bytes
>   during a transfer, nothing happens.

When you say nothing happens if the controller drives MOSI low between data
bytes you mean the data is still good in that case? Just trying to understand
the device behavior.

Thanks,
Marcelo

> ---
>  drivers/iio/adc/ad7768-1.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/drivers/iio/adc/ad7768-1.c b/drivers/iio/adc/ad7768-1.c
> index c3cf04311c40..2e2d50ccb744 100644
> --- a/drivers/iio/adc/ad7768-1.c
> +++ b/drivers/iio/adc/ad7768-1.c
> @@ -574,6 +574,21 @@ static int ad7768_probe(struct spi_device *spi)
>  		return -ENOMEM;
>  
>  	st = iio_priv(indio_dev);
> +	/*
> +	 * Datasheet recommends SDI line to be kept high when data is not being
> +	 * clocked out of the controller and the spi clock is free running,
> +	 * to prevent accidental reset.
> +	 * Since many controllers do not support the SPI_MOSI_IDLE_HIGH flag
> +	 * yet, only request the MOSI idle state to enable if the controller
> +	 * supports it.
> +	 */
> +	if (spi->controller->mode_bits & SPI_MOSI_IDLE_HIGH) {
> +		spi->mode |= SPI_MOSI_IDLE_HIGH;
> +		ret = spi_setup(spi);
> +		if (ret < 0)
> +			return ret;
> +	}
> +
>  	st->spi = spi;
>  
>  	st->vref = devm_regulator_get(&spi->dev, "vref");
> -- 
> 2.34.1
> 

Re: [PATCH v4 06/17] Documentation: ABI: add wideband filter type to sysfs-bus-iio

[Marcelo Schmitt]

On 03/06, Jonathan Santos wrote:
> The Wideband Low Ripple filter is used for AD7768-1 Driver.
> Document wideband filter option into filter_type_available
> attribute.
> 
> Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com>
> ---

LGTM

Reviewed-by: Marcelo Schmitt <marcelo.schmitt@analog.com>

FWIW, ADAQ7768-1 datasheet provides more info about what wideband should mean
for these devices.
"The wideband filter offers a filter profile similar to
an ideal brick wall filter, making it ideal for frequency analysis."
The proposed doc seems to portray that to some extent.

> v4 Changes:
> * None.
> 
> v3 Changes:
> * None, since we still did not agree on a better name for this filter type.
> 
> v2 Changes:
> * Removed FIR mentions.
> ---
>  Documentation/ABI/testing/sysfs-bus-iio | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
> index f83bd6829285..9b879e7732cd 100644
> --- a/Documentation/ABI/testing/sysfs-bus-iio
> +++ b/Documentation/ABI/testing/sysfs-bus-iio
> @@ -2291,6 +2291,8 @@ Description:
>  		* "sinc3+pf2" - Sinc3 + device specific Post Filter 2.
>  		* "sinc3+pf3" - Sinc3 + device specific Post Filter 3.
>  		* "sinc3+pf4" - Sinc3 + device specific Post Filter 4.
> +		* "wideband" - filter with wideband low ripple passband
> +		  and sharp transition band.
>  
>  What:		/sys/.../events/in_proximity_thresh_either_runningperiod
>  KernelVersion:	6.6
> -- 
> 2.34.1
> 

Re: [PATCH v4 08/17] iio: adc: ad7768-1: convert driver to use regmap

[Marcelo Schmitt]

On 03/06, Jonathan Santos wrote:
> Convert the AD7768-1 driver to use the regmap API for register
> access. This change simplifies and standardizes register interactions,
> reducing code duplication and improving maintainability.
> 
> Create two regmap configurations, one for 8-bit register values and
> other for 24-bit register values.
> 
> Since we are using regmap now, define the remaining registers from 0x32
> to 0x34.
> 
> Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com>
> ---
> v4 Changes:
> * Add `REGMAP24` to the register macros with 24-bit value.
> * Add `select REGMAP_SPI` line to the Kconfig.
> 
> v3 Changes:
> * Included a second register map for the 24-bit register values.
> * Added register tables to separate the 24-bit from the 8-bit values.
> 
> v2 Changes:
> * New patch in v2.
> ---
>  drivers/iio/adc/Kconfig    |   1 +
>  drivers/iio/adc/ad7768-1.c | 151 +++++++++++++++++++++++++------------
>  2 files changed, 104 insertions(+), 48 deletions(-)
> 
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 849c90203071..a2fdb7e03a66 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -277,6 +277,7 @@ config AD7766
>  config AD7768_1
>  	tristate "Analog Devices AD7768-1 ADC driver"
>  	depends on SPI
> +	select REGMAP_SPI
>  	select IIO_BUFFER
>  	select IIO_TRIGGER
>  	select IIO_TRIGGERED_BUFFER
> diff --git a/drivers/iio/adc/ad7768-1.c b/drivers/iio/adc/ad7768-1.c
> index f5509a0a36ab..04a26e5b7d5c 100644
> --- a/drivers/iio/adc/ad7768-1.c
> +++ b/drivers/iio/adc/ad7768-1.c
> @@ -12,6 +12,7 @@
>  #include <linux/gpio/consumer.h>
>  #include <linux/kernel.h>
>  #include <linux/module.h>
> +#include <linux/regmap.h>
>  #include <linux/regulator/consumer.h>
>  #include <linux/sysfs.h>
>  #include <linux/spi/spi.h>
> @@ -53,12 +54,15 @@
>  #define AD7768_REG_SPI_DIAG_ENABLE	0x28
>  #define AD7768_REG_ADC_DIAG_ENABLE	0x29
>  #define AD7768_REG_DIG_DIAG_ENABLE	0x2A
> -#define AD7768_REG_ADC_DATA		0x2C
> +#define AD7768_REG24_ADC_DATA		0x2C
>  #define AD7768_REG_MASTER_STATUS	0x2D
>  #define AD7768_REG_SPI_DIAG_STATUS	0x2E
>  #define AD7768_REG_ADC_DIAG_STATUS	0x2F
>  #define AD7768_REG_DIG_DIAG_STATUS	0x30
>  #define AD7768_REG_MCLK_COUNTER		0x31
> +#define AD7768_REG_COEFF_CONTROL	0x32
> +#define AD7768_REG24_COEFF_DATA		0x33
> +#define AD7768_REG_ACCESS_KEY		0x34
>  
>  /* AD7768_REG_POWER_CLOCK */
>  #define AD7768_PWR_MCLK_DIV_MSK		GENMASK(5, 4)
> @@ -153,6 +157,8 @@ static const struct iio_chan_spec ad7768_channels[] = {
>  
>  struct ad7768_state {
>  	struct spi_device *spi;
> +	struct regmap *regmap;
> +	struct regmap *regmap24;
>  	struct regulator *vref;
>  	struct clk *mclk;
>  	unsigned int mclk_freq;
> @@ -175,46 +181,76 @@ struct ad7768_state {
>  	} data __aligned(IIO_DMA_MINALIGN);
>  };
>  
> -static int ad7768_spi_reg_read(struct ad7768_state *st, unsigned int addr,
> -			       unsigned int len)
> -{
> -	unsigned int shift;
> -	int ret;
> +static const struct regmap_range ad7768_regmap_rd_ranges[] = {
> +	regmap_reg_range(AD7768_REG_CHIP_TYPE, AD7768_REG_DIG_DIAG_ENABLE),
> +	regmap_reg_range(AD7768_REG_MASTER_STATUS, AD7768_REG_COEFF_CONTROL),
> +	regmap_reg_range(AD7768_REG_ACCESS_KEY, AD7768_REG_ACCESS_KEY),
> +};
>  
> -	shift = 32 - (8 * len);
> -	st->data.d8[0] = AD7768_RD_FLAG_MSK(addr);
> +static const struct regmap_access_table ad7768_regmap_rd_table = {
> +	.yes_ranges = ad7768_regmap_rd_ranges,
> +	.n_yes_ranges = ARRAY_SIZE(ad7768_regmap_rd_ranges),
> +};
>  
> -	ret = spi_write_then_read(st->spi, st->data.d8, 1,
> -				  &st->data.d32, len);
> -	if (ret < 0)
> -		return ret;
> +static const struct regmap_range ad7768_regmap_wr_ranges[] = {
> +	regmap_reg_range(AD7768_REG_SCRATCH_PAD, AD7768_REG_SCRATCH_PAD),
> +	regmap_reg_range(AD7768_REG_INTERFACE_FORMAT, AD7768_REG_GPIO_WRITE),
> +	regmap_reg_range(AD7768_REG_OFFSET_HI, AD7768_REG_DIG_DIAG_ENABLE),
> +	regmap_reg_range(AD7768_REG_SPI_DIAG_STATUS, AD7768_REG_SPI_DIAG_STATUS),
> +	regmap_reg_range(AD7768_REG_COEFF_CONTROL, AD7768_REG_COEFF_CONTROL),
> +};
>  
> -	return (be32_to_cpu(st->data.d32) >> shift);
> -}
> +static const struct regmap_access_table ad7768_regmap_wr_table = {
> +	.yes_ranges = ad7768_regmap_wr_ranges,
> +	.n_yes_ranges = ARRAY_SIZE(ad7768_regmap_wr_ranges),
> +};
>  
> -static int ad7768_spi_reg_write(struct ad7768_state *st,
> -				unsigned int addr,
> -				unsigned int val)
> -{
> -	st->data.d8[0] = AD7768_WR_FLAG_MSK(addr);
> -	st->data.d8[1] = val & 0xFF;
> +static const struct regmap_config ad7768_regmap_config = {
> +	.name = "ad7768-1-8",
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +	.read_flag_mask = BIT(6),
> +	.rd_table = &ad7768_regmap_rd_table,
> +	.wr_table = &ad7768_regmap_wr_table,
> +	.max_register = AD7768_REG_ACCESS_KEY,
> +	.use_single_write = true,
> +	.use_single_read = true,
> +};
>  
> -	return spi_write(st->spi, st->data.d8, 2);
> -}
> +static const struct regmap_range ad7768_regmap24_rd_ranges[] = {
> +	regmap_reg_range(AD7768_REG24_ADC_DATA, AD7768_REG24_ADC_DATA),
> +	regmap_reg_range(AD7768_REG24_COEFF_DATA, AD7768_REG24_COEFF_DATA),

So, this device has only two registers that are 24-bit size?
Also, one of those is the ADC_DATA register which you will probably want
to read with optimized SPI messages in the future (devm_spi_optimize_message()).
That makes me wonder if the 24-bit regmap worth's the boiler plate to have it.
Does the driver access AD7768_REG24_COEFF_DATA after the patches from this
series is applied? If not, maybe drop the 24-bit regmap and implement ADC_DATA
with usual spi_message/spi_transfer interfaces?

> +};
>  
> -static int ad7768_set_mode(struct ad7768_state *st,
> -			   enum ad7768_conv_mode mode)
> -{
> -	int regval;
> +static const struct regmap_access_table ad7768_regmap24_rd_table = {
> +	.yes_ranges = ad7768_regmap24_rd_ranges,
> +	.n_yes_ranges = ARRAY_SIZE(ad7768_regmap24_rd_ranges),
> +};
>  
> -	regval = ad7768_spi_reg_read(st, AD7768_REG_CONVERSION, 1);
> -	if (regval < 0)
> -		return regval;
> +static const struct regmap_range ad7768_regmap24_wr_ranges[] = {
> +	regmap_reg_range(AD7768_REG24_COEFF_DATA, AD7768_REG24_COEFF_DATA),
> +};
>  
> -	regval &= ~AD7768_CONV_MODE_MSK;
> -	regval |= AD7768_CONV_MODE(mode);
> +static const struct regmap_access_table ad7768_regmap24_wr_table = {
> +	.yes_ranges = ad7768_regmap24_wr_ranges,
> +	.n_yes_ranges = ARRAY_SIZE(ad7768_regmap24_wr_ranges),
> +};
> +
> +static const struct regmap_config ad7768_regmap24_config = {
> +	.name = "ad7768-1-24",
> +	.reg_bits = 8,
> +	.val_bits = 24,
> +	.read_flag_mask = BIT(6),
> +	.rd_table = &ad7768_regmap24_rd_table,
> +	.wr_table = &ad7768_regmap24_wr_table,
> +	.max_register = AD7768_REG24_COEFF_DATA,
> +};
>  
> -	return ad7768_spi_reg_write(st, AD7768_REG_CONVERSION, regval);
> +static int ad7768_set_mode(struct ad7768_state *st,
> +			   enum ad7768_conv_mode mode)
> +{
> +	return regmap_update_bits(st->regmap, AD7768_REG_CONVERSION,
> +				 AD7768_CONV_MODE_MSK, AD7768_CONV_MODE(mode));
>  }
>  
>  static int ad7768_scan_direct(struct iio_dev *indio_dev)
> @@ -233,9 +269,10 @@ static int ad7768_scan_direct(struct iio_dev *indio_dev)
>  	if (!ret)
>  		return -ETIMEDOUT;
>  
> -	readval = ad7768_spi_reg_read(st, AD7768_REG_ADC_DATA, 3);
> -	if (readval < 0)
> -		return readval;
> +	ret = regmap_read(st->regmap24, AD7768_REG24_ADC_DATA, &readval);
> +	if (ret)
> +		return ret;
> +
>  	/*
>  	 * Any SPI configuration of the AD7768-1 can only be
>  	 * performed in continuous conversion mode.
> @@ -259,16 +296,23 @@ static int ad7768_reg_access(struct iio_dev *indio_dev,
>  	if (ret)
>  		return ret;
>  
> +	ret = -EINVAL;
>  	if (readval) {
> -		ret = ad7768_spi_reg_read(st, reg, 1);
> -		if (ret < 0)
> -			goto err_release;
> -		*readval = ret;
> -		ret = 0;
> +		if (regmap_check_range_table(st->regmap, reg, &ad7768_regmap_rd_table))
> +			ret = regmap_read(st->regmap, reg, readval);
> +
> +		if (regmap_check_range_table(st->regmap24, reg, &ad7768_regmap24_rd_table))
> +			ret = regmap_read(st->regmap24, reg, readval);
> +
>  	} else {
> -		ret = ad7768_spi_reg_write(st, reg, writeval);
> +		if (regmap_check_range_table(st->regmap, reg, &ad7768_regmap_wr_table))
> +			ret = regmap_write(st->regmap, reg, writeval);
> +
> +		if (regmap_check_range_table(st->regmap24, reg, &ad7768_regmap24_wr_table))
> +			ret = regmap_write(st->regmap24, reg, writeval);
> +
>  	}
> -err_release:
> +
>  	iio_device_release_direct_mode(indio_dev);
>  
>  	return ret;
> @@ -285,7 +329,7 @@ static int ad7768_set_dig_fil(struct ad7768_state *st,
>  	else
>  		mode = AD7768_DIG_FIL_DEC_RATE(dec_rate);
>  
> -	ret = ad7768_spi_reg_write(st, AD7768_REG_DIGITAL_FILTER, mode);
> +	ret = regmap_write(st->regmap, AD7768_REG_DIGITAL_FILTER, mode);
>  	if (ret < 0)
>  		return ret;
>  
> @@ -322,7 +366,7 @@ static int ad7768_set_freq(struct ad7768_state *st,
>  	 */
>  	pwr_mode = AD7768_PWR_MCLK_DIV(ad7768_clk_config[idx].mclk_div) |
>  		   AD7768_PWR_PWRMODE(ad7768_clk_config[idx].pwrmode);
> -	ret = ad7768_spi_reg_write(st, AD7768_REG_POWER_CLOCK, pwr_mode);
> +	ret = regmap_write(st->regmap, AD7768_REG_POWER_CLOCK, pwr_mode);
>  	if (ret < 0)
>  		return ret;
>  
> @@ -449,11 +493,11 @@ static int ad7768_setup(struct ad7768_state *st)
>  	 * to 10. When the sequence is detected, the reset occurs.
>  	 * See the datasheet, page 70.
>  	 */
> -	ret = ad7768_spi_reg_write(st, AD7768_REG_SYNC_RESET, 0x3);
> +	ret = regmap_write(st->regmap, AD7768_REG_SYNC_RESET, 0x3);
>  	if (ret)
>  		return ret;
>  
> -	ret = ad7768_spi_reg_write(st, AD7768_REG_SYNC_RESET, 0x2);
> +	ret = regmap_write(st->regmap, AD7768_REG_SYNC_RESET, 0x2);
>  	if (ret)
>  		return ret;
>  
> @@ -508,18 +552,19 @@ static int ad7768_buffer_postenable(struct iio_dev *indio_dev)
>  	 * continuous read mode. Subsequent data reads do not require an
>  	 * initial 8-bit write to query the ADC_DATA register.
>  	 */
> -	return ad7768_spi_reg_write(st, AD7768_REG_INTERFACE_FORMAT, 0x01);
> +	return regmap_write(st->regmap, AD7768_REG_INTERFACE_FORMAT, 0x01);
>  }
>  
>  static int ad7768_buffer_predisable(struct iio_dev *indio_dev)
>  {
>  	struct ad7768_state *st = iio_priv(indio_dev);
> +	unsigned int unused;
>  
>  	/*
>  	 * To exit continuous read mode, perform a single read of the ADC_DATA
>  	 * reg (0x2C), which allows further configuration of the device.
>  	 */
> -	return ad7768_spi_reg_read(st, AD7768_REG_ADC_DATA, 3);
> +	return regmap_read(st->regmap24, AD7768_REG24_ADC_DATA, &unused);
>  }
>  
>  static const struct iio_buffer_setup_ops ad7768_buffer_ops = {
> @@ -590,6 +635,16 @@ static int ad7768_probe(struct spi_device *spi)
>  
>  	st->spi = spi;
>  
> +	st->regmap = devm_regmap_init_spi(spi, &ad7768_regmap_config);
> +	if (IS_ERR(st->regmap))
> +		return dev_err_probe(&spi->dev, PTR_ERR(st->regmap),
> +				     "Failed to initialize regmap");
> +
> +	st->regmap24 = devm_regmap_init_spi(spi, &ad7768_regmap24_config);
> +	if (IS_ERR(st->regmap24))
> +		return dev_err_probe(&spi->dev, PTR_ERR(st->regmap24),
> +				     "Failed to initialize regmap24");
> +
>  	st->vref = devm_regulator_get(&spi->dev, "vref");
>  	if (IS_ERR(st->vref))
>  		return PTR_ERR(st->vref);
> -- 
> 2.34.1
> 

Re: [PATCH v4 09/17] iio: adc: ad7768-1: Add reset gpio

[Marcelo Schmitt]

On 03/06, Jonathan Santos wrote:
> From: Sergiu Cuciurean <sergiu.cuciurean@analog.com>
> 
> Depending on the controller, the default state of a gpio can vary. This
> change excludes the probability that the dafult state of the ADC reset
> gpio will be HIGH if it will be passed as reference in the devicetree.

The description doesn't seem to match the changes nor the patch title. You are
essentinally adding support for hardware reset. Change the commit description to
reflect that.

The default state of GPIOs would not impact device reset because (in theory)
they weren't being connected to the reset pin prevously.

> 
> Reviewed-by: David Lechner <dlechner@baylibre.com>
> Reviewed-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
> Signed-off-by: Sergiu Cuciurean <sergiu.cuciurean@analog.com>
> Co-developed-by: Jonathan Santos <Jonathan.Santos@analog.com>
> Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com>
> ---
> v4 Changes:
> * None.
> 
> v3 Changes:
> * fixed SoB order.
> * increased delay after finishing the reset action to 200us, as the
>   datasheet recommends.
> 
> v2 Changes:
> * Replaced usleep_range() for fsleep() and gpiod_direction_output() for 
>   gpiod_set_value_cansleep().
> * Reset via SPI register is performed if the Reset GPIO is not defined. 
> ---
>  drivers/iio/adc/ad7768-1.c | 36 ++++++++++++++++++++++++------------
>  1 file changed, 24 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/iio/adc/ad7768-1.c b/drivers/iio/adc/ad7768-1.c
> index 04a26e5b7d5c..86f44d28c478 100644
> --- a/drivers/iio/adc/ad7768-1.c
> +++ b/drivers/iio/adc/ad7768-1.c
> @@ -166,6 +166,7 @@ struct ad7768_state {
>  	struct completion completion;
>  	struct iio_trigger *trig;
>  	struct gpio_desc *gpio_sync_in;
> +	struct gpio_desc *gpio_reset;
>  	const char *labels[ARRAY_SIZE(ad7768_channels)];
>  	/*
>  	 * DMA (thus cache coherency maintenance) may require the
> @@ -487,19 +488,30 @@ static int ad7768_setup(struct ad7768_state *st)
>  {
>  	int ret;
>  
> -	/*
> -	 * Two writes to the SPI_RESET[1:0] bits are required to initiate
> -	 * a software reset. The bits must first be set to 11, and then
> -	 * to 10. When the sequence is detected, the reset occurs.
> -	 * See the datasheet, page 70.
> -	 */
> -	ret = regmap_write(st->regmap, AD7768_REG_SYNC_RESET, 0x3);
> -	if (ret)
> -		return ret;
> +	st->gpio_reset = devm_gpiod_get_optional(&st->spi->dev, "reset",
> +						 GPIOD_OUT_HIGH);
> +	if (IS_ERR(st->gpio_reset))
> +		return PTR_ERR(st->gpio_reset);
>  
> -	ret = regmap_write(st->regmap, AD7768_REG_SYNC_RESET, 0x2);
> -	if (ret)
> -		return ret;
> +	if (st->gpio_reset) {
> +		fsleep(10);
> +		gpiod_set_value_cansleep(st->gpio_reset, 0);
> +		fsleep(200);
> +	} else {
> +		/*
> +		 * Two writes to the SPI_RESET[1:0] bits are required to initiate
> +		 * a software reset. The bits must first be set to 11, and then
> +		 * to 10. When the sequence is detected, the reset occurs.
> +		 * See the datasheet, page 70.
> +		 */
> +		ret = regmap_write(st->regmap, AD7768_REG_SYNC_RESET, 0x3);
> +		if (ret)
> +			return ret;
> +
> +		ret = regmap_write(st->regmap, AD7768_REG_SYNC_RESET, 0x2);
> +		if (ret)
> +			return ret;
> +	}
>  
>  	st->gpio_sync_in = devm_gpiod_get(&st->spi->dev, "adi,sync-in",
>  					  GPIOD_OUT_LOW);
> -- 
> 2.34.1
> 

Re: [PATCH v4 12/17] iio: adc: ad7768-1: Add GPIO controller support

[Bartosz Golaszewski]

On Thu, Mar 6, 2025 at 10:03 PM Jonathan Santos
<Jonathan.Santos@analog.com> wrote:
>
> From: Sergiu Cuciurean <sergiu.cuciurean@analog.com>
>
> The AD7768-1 has the ability to control other local hardware (such as gain
> stages),to power down other blocks in the signal chain, or read local
> status signals over the SPI interface.
>
> Add direct mode conditional locks in the gpio callbacks to prevent register
> access when the device is in buffered mode.
>
> This change exports the AD7768-1's four gpios and makes them accessible
> at an upper layer.
>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Sergiu Cuciurean <sergiu.cuciurean@analog.com>
> Co-developed-by: Jonathan Santos <Jonathan.Santos@analog.com>
> Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com>
> ---

Acked-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

Re: [PATCH v4 04/17] dt-bindings: iio: adc: ad7768-1: Document GPIO controller

[Bartosz Golaszewski]

On Thu, Mar 6, 2025 at 10:01 PM Jonathan Santos
<Jonathan.Santos@analog.com> wrote:
>
> The AD7768-1 ADC exports four bidirectional GPIOs accessible
> via register map.
>
> Document GPIO properties necessary to enable GPIO controller for this
> device.
>
> Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
> Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com>
> ---

Acked-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

Re: [PATCH v4 10/17] iio: adc: ad7768-1: Move buffer allocation to a separate function

[Marcelo Schmitt]

On 03/06, Jonathan Santos wrote:
> From: Sergiu Cuciurean <sergiu.cuciurean@analog.com>
> 
> This change moves the buffer allocation in a separate function, making
> space for adding another type of iio buffer if needed.
> 
> Reviewed-by: David Lechner <dlechner@baylibre.com>
> Signed-off-by: Sergiu Cuciurean <sergiu.cuciurean@analog.com>
> Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com>
> ---
LGTM

Reviewed-by: Marcelo Schmitt <marcelo.schmitt@analog.com>

> v4 Changes:
> * None.
> 
> v3 Changes:
> * Added missing SoB.
> 
> v2 Changes:
> * Interrupt and completion moved out from ad7768_triggered_buffer_alloc(). 
> ---
>  drivers/iio/adc/ad7768-1.c | 44 ++++++++++++++++++++++----------------
>  1 file changed, 26 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/iio/adc/ad7768-1.c b/drivers/iio/adc/ad7768-1.c
> index 86f44d28c478..e88e9431bb7a 100644
> --- a/drivers/iio/adc/ad7768-1.c
> +++ b/drivers/iio/adc/ad7768-1.c
> @@ -619,6 +619,31 @@ static int ad7768_set_channel_label(struct iio_dev *indio_dev,
>  	return 0;
>  }
>  
> +static int ad7768_triggered_buffer_alloc(struct iio_dev *indio_dev)
> +{
> +	struct ad7768_state *st = iio_priv(indio_dev);
> +	int ret;
> +
> +	st->trig = devm_iio_trigger_alloc(indio_dev->dev.parent, "%s-dev%d",
> +					  indio_dev->name,
> +					  iio_device_id(indio_dev));
> +	if (!st->trig)
> +		return -ENOMEM;
> +
> +	st->trig->ops = &ad7768_trigger_ops;
> +	iio_trigger_set_drvdata(st->trig, indio_dev);
> +	ret = devm_iio_trigger_register(indio_dev->dev.parent, st->trig);
> +	if (ret)
> +		return ret;
> +
> +	indio_dev->trig = iio_trigger_get(st->trig);
> +
> +	return devm_iio_triggered_buffer_setup(indio_dev->dev.parent, indio_dev,
> +					       &iio_pollfunc_store_time,
> +					       &ad7768_trigger_handler,
> +					       &ad7768_buffer_ops);
> +}
> +
>  static int ad7768_probe(struct spi_device *spi)
>  {
>  	struct ad7768_state *st;
> @@ -689,20 +714,6 @@ static int ad7768_probe(struct spi_device *spi)
>  		return ret;
>  	}
>  
> -	st->trig = devm_iio_trigger_alloc(&spi->dev, "%s-dev%d",
> -					  indio_dev->name,
> -					  iio_device_id(indio_dev));
> -	if (!st->trig)
> -		return -ENOMEM;
> -
> -	st->trig->ops = &ad7768_trigger_ops;
> -	iio_trigger_set_drvdata(st->trig, indio_dev);
> -	ret = devm_iio_trigger_register(&spi->dev, st->trig);
> -	if (ret)
> -		return ret;
> -
> -	indio_dev->trig = iio_trigger_get(st->trig);
> -
>  	init_completion(&st->completion);
>  
>  	ret = ad7768_set_channel_label(indio_dev, ARRAY_SIZE(ad7768_channels));
> @@ -716,10 +727,7 @@ static int ad7768_probe(struct spi_device *spi)
>  	if (ret)
>  		return ret;
>  
> -	ret = devm_iio_triggered_buffer_setup(&spi->dev, indio_dev,
> -					      &iio_pollfunc_store_time,
> -					      &ad7768_trigger_handler,
> -					      &ad7768_buffer_ops);
> +	ret = ad7768_triggered_buffer_alloc(indio_dev);
>  	if (ret)
>  		return ret;
>  
> -- 
> 2.34.1
> 

Re: [PATCH v4 11/17] iio: adc: ad7768-1: add regulator to control VCM output

[Marcelo Schmitt]

Looks ok. One minor commnet inline.

On 03/06, Jonathan Santos wrote:
> The VCM output voltage can be used as a common-mode voltage within the
> amplifier preconditioning circuits external to the AD7768-1.
> 
> This change allows the user to configure VCM output using the regulator
> framework.
> 
> Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com>

Acked-by: Marcelo Schmitt <marcelo.schmitt@analog.com>

> ---
> v4 Changes:
> * Added iio_device_claim_direct_mode() to regulator callbacks to avoid register access
>   while in buffered mode.
> * Changed regulator name to "ad7768-1-vcm".
> * When regulator enable is called, it will set the last voltage selector configured.
> * Disabled regulator before configuring it.
> * Adressed other nits.
> 
> v3 Changes:
> * Register VCM output via the regulator framework for improved flexibility
>   and external integration.
> 
> v2 Changes:
> * VCM output support is now defined by a devicetree property, instead of 
>   and IIO attribute.
> ---
>  drivers/iio/adc/Kconfig    |   1 +
>  drivers/iio/adc/ad7768-1.c | 181 +++++++++++++++++++++++++++++++++++++
>  2 files changed, 182 insertions(+)
> 
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index a2fdb7e03a66..d8f2ed477ba7 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -277,6 +277,7 @@ config AD7766
>  config AD7768_1
>  	tristate "Analog Devices AD7768-1 ADC driver"
>  	depends on SPI
> +	select REGULATOR
>  	select REGMAP_SPI
>  	select IIO_BUFFER
>  	select IIO_TRIGGER
> diff --git a/drivers/iio/adc/ad7768-1.c b/drivers/iio/adc/ad7768-1.c
> index e88e9431bb7a..2a6317f5b582 100644
> --- a/drivers/iio/adc/ad7768-1.c
> +++ b/drivers/iio/adc/ad7768-1.c
> @@ -12,8 +12,10 @@
>  #include <linux/gpio/consumer.h>
>  #include <linux/kernel.h>
>  #include <linux/module.h>
> +#include <linux/of.h>
>  #include <linux/regmap.h>
>  #include <linux/regulator/consumer.h>
> +#include <linux/regulator/driver.h>
>  #include <linux/sysfs.h>
>  #include <linux/spi/spi.h>
>  
> @@ -80,9 +82,15 @@
>  #define AD7768_CONV_MODE_MSK		GENMASK(2, 0)
>  #define AD7768_CONV_MODE(x)		FIELD_PREP(AD7768_CONV_MODE_MSK, x)
>  
> +/* AD7768_REG_ANALOG2 */
> +#define AD7768_REG_ANALOG2_VCM_MSK	GENMASK(2, 0)
> +#define AD7768_REG_ANALOG2_VCM(x)	FIELD_PREP(AD7768_REG_ANALOG2_VCM_MSK, x)

Should we enforce macro argument evaluation here?
#define AD7768_REG_ANALOG2_VCM(x)	FIELD_PREP(AD7768_REG_ANALOG2_VCM_MSK, (x))

> +
>  #define AD7768_RD_FLAG_MSK(x)		(BIT(6) | ((x) & 0x3F))
>  #define AD7768_WR_FLAG_MSK(x)		((x) & 0x3F)
>  
> +#define AD7768_VCM_OFF			0x07
> +
>  enum ad7768_conv_mode {
>  	AD7768_CONTINUOUS,
>  	AD7768_ONE_SHOT,
> @@ -160,6 +168,8 @@ struct ad7768_state {
>  	struct regmap *regmap;
>  	struct regmap *regmap24;
>  	struct regulator *vref;
> +	struct regulator_dev *vcm_rdev;
> +	unsigned int vcm_output_sel;
>  	struct clk *mclk;
>  	unsigned int mclk_freq;
>  	unsigned int samp_freq;
> @@ -644,6 +654,172 @@ static int ad7768_triggered_buffer_alloc(struct iio_dev *indio_dev)
>  					       &ad7768_buffer_ops);
>  }
>  
> +static int ad7768_vcm_enable(struct regulator_dev *rdev)
> +{
> +	struct iio_dev *indio_dev = rdev_get_drvdata(rdev);
> +	struct ad7768_state *st = iio_priv(indio_dev);
> +	int ret, regval;
> +
> +	if (!indio_dev)
> +		return -EINVAL;
> +
> +	ret = iio_device_claim_direct_mode(indio_dev);
> +	if (ret)
> +		return ret;
> +
> +	/* To enable, set the last selected output */
> +	regval = AD7768_REG_ANALOG2_VCM(st->vcm_output_sel + 1);
> +	ret = regmap_update_bits(st->regmap, AD7768_REG_ANALOG2,
> +				 AD7768_REG_ANALOG2_VCM_MSK, regval);
> +	iio_device_release_direct_mode(indio_dev);
> +
> +	return ret;
> +}
> +
> +static int ad7768_vcm_disable(struct regulator_dev *rdev)
> +{
> +	struct iio_dev *indio_dev = rdev_get_drvdata(rdev);
> +	struct ad7768_state *st = iio_priv(indio_dev);
> +	int ret;
> +
> +	if (!indio_dev)
> +		return -EINVAL;
> +
> +	ret = iio_device_claim_direct_mode(indio_dev);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_update_bits(st->regmap, AD7768_REG_ANALOG2,
> +				 AD7768_REG_ANALOG2_VCM_MSK, AD7768_VCM_OFF);
> +	iio_device_release_direct_mode(indio_dev);
> +
> +	return ret;
> +}
> +
> +static int ad7768_vcm_is_enabled(struct regulator_dev *rdev)
> +{
> +	struct iio_dev *indio_dev = rdev_get_drvdata(rdev);
> +	struct ad7768_state *st = iio_priv(indio_dev);
> +	int ret, val;
> +
> +	if (!indio_dev)
> +		return -EINVAL;
> +
> +	ret = iio_device_claim_direct_mode(indio_dev);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_read(st->regmap, AD7768_REG_ANALOG2, &val);
> +	if (ret)
> +		goto err_release;
> +
> +	ret = FIELD_GET(AD7768_REG_ANALOG2_VCM_MSK, val) != AD7768_VCM_OFF;
> +err_release:
> +	iio_device_release_direct_mode(indio_dev);
> +
> +	return ret;
> +}
> +
> +static int ad7768_set_voltage_sel(struct regulator_dev *rdev,
> +				  unsigned int selector)
> +{
> +	unsigned int regval = AD7768_REG_ANALOG2_VCM(selector + 1);
> +	struct iio_dev *indio_dev = rdev_get_drvdata(rdev);
> +	struct ad7768_state *st = iio_priv(indio_dev);
> +	int ret;
> +
> +	if (!indio_dev)
> +		return -EINVAL;
> +
> +	ret = iio_device_claim_direct_mode(indio_dev);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_update_bits(st->regmap, AD7768_REG_ANALOG2,
> +				 AD7768_REG_ANALOG2_VCM_MSK, regval);
> +	iio_device_release_direct_mode(indio_dev);
> +	st->vcm_output_sel = selector;
> +
> +	return ret;
> +}
> +
> +static int ad7768_get_voltage_sel(struct regulator_dev *rdev)
> +{
> +	struct iio_dev *indio_dev = rdev_get_drvdata(rdev);
> +	struct ad7768_state *st = iio_priv(indio_dev);
> +	int ret, val;
> +
> +	if (!indio_dev)
> +		return -EINVAL;
> +
> +	ret = iio_device_claim_direct_mode(indio_dev);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_read(st->regmap, AD7768_REG_ANALOG2, &val);
> +	if (ret)
> +		goto err_release;
> +
> +	val = FIELD_GET(AD7768_REG_ANALOG2_VCM_MSK, val);
> +	ret = clamp(val, 1, (int)rdev->desc->n_voltages) - 1;
> +err_release:
> +	iio_device_release_direct_mode(indio_dev);
> +
> +	return ret;
> +}
> +
> +static const struct regulator_ops vcm_regulator_ops = {
> +	.enable = ad7768_vcm_enable,
> +	.disable = ad7768_vcm_disable,
> +	.is_enabled = ad7768_vcm_is_enabled,
> +	.list_voltage = regulator_list_voltage_table,
> +	.set_voltage_sel = ad7768_set_voltage_sel,
> +	.get_voltage_sel = ad7768_get_voltage_sel,
> +};
> +
> +static const unsigned int vcm_voltage_table[] = {
> +	2500000,
> +	2050000,
> +	1650000,
> +	1900000,
> +	1100000,
> +	900000,
> +};
> +
> +static const struct regulator_desc vcm_desc = {
> +	.name = "ad7768-1-vcm",
> +	.of_match = of_match_ptr("vcm-output"),
> +	.regulators_node = of_match_ptr("regulators"),
> +	.n_voltages = ARRAY_SIZE(vcm_voltage_table),
> +	.volt_table = vcm_voltage_table,
> +	.ops = &vcm_regulator_ops,
> +	.type = REGULATOR_VOLTAGE,
> +	.owner = THIS_MODULE,
> +};
> +
> +static int ad7768_register_regulators(struct device *dev, struct ad7768_state *st,
> +				      struct iio_dev *indio_dev)
> +{
> +	struct regulator_config config = {
> +		.dev = dev,
> +		.driver_data = indio_dev,
> +	};
> +	int ret;
> +
> +	/* Disable the regulator before registering it */
> +	ret = regmap_update_bits(st->regmap, AD7768_REG_ANALOG2,
> +				 AD7768_REG_ANALOG2_VCM_MSK, AD7768_VCM_OFF);
> +	if (ret)
> +		return -EINVAL;
> +
> +	st->vcm_rdev = devm_regulator_register(dev, &vcm_desc, &config);
> +	if (IS_ERR(st->vcm_rdev))
> +		return dev_err_probe(dev, PTR_ERR(st->vcm_rdev),
> +				     "failed to register VCM regulator\n");
> +
> +	return 0;
> +}
> +
>  static int ad7768_probe(struct spi_device *spi)
>  {
>  	struct ad7768_state *st;
> @@ -708,6 +884,11 @@ static int ad7768_probe(struct spi_device *spi)
>  	indio_dev->info = &ad7768_info;
>  	indio_dev->modes = INDIO_DIRECT_MODE;
>  
> +	/* Register VCM output regulator */
> +	ret = ad7768_register_regulators(&spi->dev, st, indio_dev);
> +	if (ret)
> +		return ret;
> +
>  	ret = ad7768_setup(st);
>  	if (ret < 0) {
>  		dev_err(&spi->dev, "AD7768 setup failed\n");
> -- 
> 2.34.1
> 

Re: [PATCH v4 15/17] iio: adc: ad7768-1: replace manual attribute declaration

[Marcelo Schmitt]

On 03/06, Jonathan Santos wrote:
> Use read_avail callback from struct iio_info to replace the manual
> declaration of sampling_frequency_available attribute.
> 
> Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com>

Reviewed-by: Marcelo Schmitt <marcelo.schmitt@analog.com>

> ---
> v4 Changes:
> * Added ad7768_fill_samp_freq_tbl() helper function.
> * Sampling frequency table is precomputed at probe.
> 
> v3 Changes:
> * New patch in v3.
> ---
>  drivers/iio/adc/ad7768-1.c | 63 +++++++++++++++++++-------------------
>  1 file changed, 31 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/iio/adc/ad7768-1.c b/drivers/iio/adc/ad7768-1.c
> index c48d3e0af985..0bdf2ae903c6 100644
> --- a/drivers/iio/adc/ad7768-1.c
> +++ b/drivers/iio/adc/ad7768-1.c
> @@ -187,6 +187,7 @@ static const struct iio_chan_spec ad7768_channels[] = {
>  		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
>  		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
>  		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),
> +		.info_mask_shared_by_all_available = BIT(IIO_CHAN_INFO_SAMP_FREQ),
>  		.indexed = 1,
>  		.channel = 0,
>  		.scan_index = 0,
> @@ -208,6 +209,7 @@ struct ad7768_state {
>  	unsigned int mclk_freq;
>  	unsigned int dec_rate;
>  	unsigned int samp_freq;
> +	unsigned int samp_freq_avail[ARRAY_SIZE(ad7768_clk_config)];
>  	struct completion completion;
>  	struct iio_trigger *trig;
>  	struct gpio_desc *gpio_sync_in;
> @@ -306,6 +308,15 @@ static int ad7768_send_sync_pulse(struct ad7768_state *st)
>  	return 0;
>  }
>  
> +static void ad7768_fill_samp_freq_tbl(struct ad7768_state *st)
> +{
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(ad7768_clk_config); i++)
> +		st->samp_freq_avail[i] = DIV_ROUND_CLOSEST(st->mclk_freq,
> +							   ad7768_clk_config[i].clk_div);
> +}
> +
>  static int ad7768_set_mode(struct ad7768_state *st,
>  			   enum ad7768_conv_mode mode)
>  {
> @@ -566,28 +577,6 @@ static int ad7768_set_freq(struct ad7768_state *st,
>  	return 0;
>  }
>  
> -static ssize_t ad7768_sampling_freq_avail(struct device *dev,
> -					  struct device_attribute *attr,
> -					  char *buf)
> -{
> -	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> -	struct ad7768_state *st = iio_priv(indio_dev);
> -	unsigned int freq;
> -	int i, len = 0;
> -
> -	for (i = 0; i < ARRAY_SIZE(ad7768_clk_config); i++) {
> -		freq = DIV_ROUND_CLOSEST(st->mclk_freq,
> -					 ad7768_clk_config[i].clk_div);
> -		len += scnprintf(buf + len, PAGE_SIZE - len, "%d ", freq);
> -	}
> -
> -	buf[len - 1] = '\n';
> -
> -	return len;
> -}
> -
> -static IIO_DEV_ATTR_SAMP_FREQ_AVAIL(ad7768_sampling_freq_avail);
> -
>  static int ad7768_read_raw(struct iio_dev *indio_dev,
>  			   struct iio_chan_spec const *chan,
>  			   int *val, int *val2, long info)
> @@ -635,6 +624,24 @@ static int ad7768_read_raw(struct iio_dev *indio_dev,
>  	return -EINVAL;
>  }
>  
> +static int ad7768_read_avail(struct iio_dev *indio_dev,
> +			     struct iio_chan_spec const *chan,
> +			     const int **vals, int *type, int *length,
> +			     long info)
> +{
> +	struct ad7768_state *st = iio_priv(indio_dev);
> +
> +	switch (info) {
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		*vals = (int *)st->samp_freq_avail;
> +		*length = ARRAY_SIZE(ad7768_clk_config);
> +		*type = IIO_VAL_INT;
> +		return IIO_AVAIL_LIST;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
>  static int ad7768_write_raw(struct iio_dev *indio_dev,
>  			    struct iio_chan_spec const *chan,
>  			    int val, int val2, long info)
> @@ -657,15 +664,6 @@ static int ad7768_read_label(struct iio_dev *indio_dev,
>  	return sprintf(label, "%s\n", st->labels[chan->channel]);
>  }
>  
> -static struct attribute *ad7768_attributes[] = {
> -	&iio_dev_attr_sampling_frequency_available.dev_attr.attr,
> -	NULL
> -};
> -
> -static const struct attribute_group ad7768_group = {
> -	.attrs = ad7768_attributes,
> -};
> -
>  static int ad7768_get_current_scan_type(const struct iio_dev *indio_dev,
>  					const struct iio_chan_spec *chan)
>  {
> @@ -676,8 +674,8 @@ static int ad7768_get_current_scan_type(const struct iio_dev *indio_dev,
>  }
>  
>  static const struct iio_info ad7768_info = {
> -	.attrs = &ad7768_group,
>  	.read_raw = &ad7768_read_raw,
> +	.read_avail = &ad7768_read_avail,
>  	.write_raw = &ad7768_write_raw,
>  	.read_label = ad7768_read_label,
>  	.get_current_scan_type = &ad7768_get_current_scan_type,
> @@ -1134,6 +1132,7 @@ static int ad7768_probe(struct spi_device *spi)
>  		return PTR_ERR(st->mclk);
>  
>  	st->mclk_freq = clk_get_rate(st->mclk);
> +	ad7768_fill_samp_freq_tbl(st);
>  
>  	indio_dev->channels = ad7768_channels;
>  	indio_dev->num_channels = ARRAY_SIZE(ad7768_channels);
> -- 
> 2.34.1
> 

Re: [PATCH v4 17/17] iio: adc: ad7768-1: add low pass -3dB cutoff attribute

[Marcelo Schmitt]

Looks good. Minor comment inline.

On 03/06, Jonathan Santos wrote:
> Ad7768-1 has a different -3db frequency multiplier depending on
> the filter type configured. The cutoff frequency also varies according
> to the current ODR.
> 
> Add a readonly low pass -3dB frequency cutoff attribute to clarify to
> the user which bandwidth is being allowed depending on the filter
> configurations.
> 
> Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com>
> ---

Reviewed-by: Marcelo Schmitt <marcelo.schmitt@analog.com>

> v4 Changes:
> * None
> 
> v3 Changes:
> * None
> 
> v2 Changes:
> * New patch in v2.
> ---
>  drivers/iio/adc/ad7768-1.c | 22 ++++++++++++++++++++--
>  1 file changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/adc/ad7768-1.c b/drivers/iio/adc/ad7768-1.c
> index 664d0b535340..9ba00e9089bd 100644
> --- a/drivers/iio/adc/ad7768-1.c
> +++ b/drivers/iio/adc/ad7768-1.c
> @@ -150,6 +150,17 @@ enum ad7768_scan_type {
>  	AD7768_SCAN_TYPE_HIGH_SPEED,
>  };
>  
> +/*
> + * -3dB cutoff frequency multipliers (relative to ODR) for
> + * each filter type. Values are multiplied by 1000.
> + */
> +static const int ad7768_filter_3db_odr_multiplier[] = {
> +	[AD7768_FILTER_SINC5] = 204,
> +	[AD7768_FILTER_SINC3] = 261,
> +	[AD7768_FILTER_SINC3_REJ60] = 261,

Are these constants from ADAQ7768-1 datasheet?
There it says "the sinc3 filter has a −3 dB bandwidth of 0.2617 × ODR."
If so, maybe use 262 as SINC3 multiplier?

> +	[AD7768_FILTER_WIDEBAND] = 433,
> +};
> +
>  static const int ad7768_mclk_div_rates[4] = {
>  	16, 8, 4, 2,
>  };
> @@ -228,7 +239,8 @@ static const struct iio_chan_spec ad7768_channels[] = {
>  		.type = IIO_VOLTAGE,
>  		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
>  		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |
> -					    BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
> +					    BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO) |
> +					    BIT(IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY),
>  		.info_mask_shared_by_type_available = BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
>  		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),
>  		.info_mask_shared_by_all_available = BIT(IIO_CHAN_INFO_SAMP_FREQ),
> @@ -762,7 +774,7 @@ static int ad7768_read_raw(struct iio_dev *indio_dev,
>  {
>  	struct ad7768_state *st = iio_priv(indio_dev);
>  	const struct iio_scan_type *scan_type;
> -	int scale_uv, ret;
> +	int scale_uv, ret, temp;
>  
>  	scan_type = iio_get_current_scan_type(indio_dev, chan);
>  	if (IS_ERR(scan_type))
> @@ -802,6 +814,12 @@ static int ad7768_read_raw(struct iio_dev *indio_dev,
>  	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
>  		*val = st->oversampling_ratio;
>  
> +		return IIO_VAL_INT;
> +
> +	case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
> +		temp = st->samp_freq * ad7768_filter_3db_odr_multiplier[st->filter_type];
> +		*val = DIV_ROUND_CLOSEST(temp, 1000);
> +
>  		return IIO_VAL_INT;
>  	}
>  
> -- 
> 2.34.1
> 

Re: [PATCH v4 11/17] iio: adc: ad7768-1: add regulator to control VCM output

[Jonathan Cameron]

On Fri, 7 Mar 2025 11:32:54 -0300
Marcelo Schmitt <marcelo.schmitt1@gmail.com> wrote:

> Looks ok. One minor commnet inline.
> 
> On 03/06, Jonathan Santos wrote:
> > The VCM output voltage can be used as a common-mode voltage within the
> > amplifier preconditioning circuits external to the AD7768-1.
> > 
> > This change allows the user to configure VCM output using the regulator
> > framework.
> > 
> > Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com>  
> 
> Acked-by: Marcelo Schmitt <marcelo.schmitt@analog.com>


...

> > diff --git a/drivers/iio/adc/ad7768-1.c b/drivers/iio/adc/ad7768-1.c
> > index e88e9431bb7a..2a6317f5b582 100644
> > --- a/drivers/iio/adc/ad7768-1.c
> > +++ b/drivers/iio/adc/ad7768-1.c
> > @@ -12,8 +12,10 @@
> >  #include <linux/gpio/consumer.h>
> >  #include <linux/kernel.h>
> >  #include <linux/module.h>
> > +#include <linux/of.h>
> >  #include <linux/regmap.h>
> >  #include <linux/regulator/consumer.h>
> > +#include <linux/regulator/driver.h>
> >  #include <linux/sysfs.h>
> >  #include <linux/spi/spi.h>
> >  
> > @@ -80,9 +82,15 @@
> >  #define AD7768_CONV_MODE_MSK		GENMASK(2, 0)
> >  #define AD7768_CONV_MODE(x)		FIELD_PREP(AD7768_CONV_MODE_MSK, x)
> >  
> > +/* AD7768_REG_ANALOG2 */
> > +#define AD7768_REG_ANALOG2_VCM_MSK	GENMASK(2, 0)
> > +#define AD7768_REG_ANALOG2_VCM(x)	FIELD_PREP(AD7768_REG_ANALOG2_VCM_MSK, x)  
> 
> Should we enforce macro argument evaluation here?
> #define AD7768_REG_ANALOG2_VCM(x)	FIELD_PREP(AD7768_REG_ANALOG2_VCM_MSK, (x))
> 
Yes.  Seems there is another case above as well that needs fixing up.

> > +
> >  #define AD7768_RD_FLAG_MSK(x)		(BIT(6) | ((x) & 0x3F))
> >  #define AD7768_WR_FLAG_MSK(x)		((x) & 0x3F)
> >  

Re: [PATCH v4 01/17] iio: adc: ad7768-1: Fix conversion result sign

[Jonathan Cameron]

On Thu, 6 Mar 2025 18:00:29 -0300
Jonathan Santos <Jonathan.Santos@analog.com> wrote:

> From: Sergiu Cuciurean <sergiu.cuciurean@analog.com>
> 
> The ad7768-1 ADC output code is two's complement, meaning that the voltage
> conversion result is a signed value.. Since the value is a 24 bit one,
> stored in a 32 bit variable, the sign should be extended in order to get
> the correct representation.
> 
> Also the channel description has been updated to signed representation,
> to match the ADC specifications.
> 
> Fixes: a5f8c7da3dbe ("iio: adc: Add AD7768-1 ADC basic support")
> Reviewed-by: David Lechner <dlechner@baylibre.com>
> Reviewed-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
> Signed-off-by: Sergiu Cuciurean <sergiu.cuciurean@analog.com>
> Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com>
I'm going to start picking up the first part of this series now to
reduce the scale for future versions.

I have applied this one and marked it for stable. Note that the
recent iio_device_claim_direct() series changed the handling
of *val so I applied the equivalent of this by hand.  Please
check the result in the testing branch of iio.git.

Thanks,

Jonathan

> ---
> v4 Changes:
> * none.
> 
> v3 Changes:
> * Added missing SoB.
> 
> v2 Changes:
> * Patch moved to the start of the patch series.
> ---
>  drivers/iio/adc/ad7768-1.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/adc/ad7768-1.c b/drivers/iio/adc/ad7768-1.c
> index 113703fb7245..c3cf04311c40 100644
> --- a/drivers/iio/adc/ad7768-1.c
> +++ b/drivers/iio/adc/ad7768-1.c
> @@ -142,7 +142,7 @@ static const struct iio_chan_spec ad7768_channels[] = {
>  		.channel = 0,
>  		.scan_index = 0,
>  		.scan_type = {
> -			.sign = 'u',
> +			.sign = 's',
>  			.realbits = 24,
>  			.storagebits = 32,
>  			.shift = 8,
> @@ -371,7 +371,7 @@ static int ad7768_read_raw(struct iio_dev *indio_dev,
>  
>  		ret = ad7768_scan_direct(indio_dev);
>  		if (ret >= 0)
> -			*val = ret;
> +			*val = sign_extend32(ret, chan->scan_type.realbits - 1);
>  
>  		iio_device_release_direct_mode(indio_dev);
>  		if (ret < 0)

Re: [PATCH v4 02/17] iio: adc: ad7768-1: set MOSI idle state to prevent accidental reset

[Jonathan Cameron]

On Fri, 7 Mar 2025 09:06:48 -0300
Marcelo Schmitt <marcelo.schmitt1@gmail.com> wrote:

> On 03/06, Jonathan Santos wrote:
> > Datasheet recommends Setting the MOSI idle state to high in order to
> > prevent accidental reset of the device when SCLK is free running.
> > This happens when the controller clocks out a 1 followed by 63 zeros
> > while the CS is held low.
> > 
> > Check if SPI controller supports SPI_MOSI_IDLE_HIGH flag and set it.
> > 
> > Fixes: a5f8c7da3dbe ("iio: adc: Add AD7768-1 ADC basic support")
> > Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com>
> > ---  
> 
> LGTM
> 
> Reviewed-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
Applied on assumption the answer to Marcelo's question doesn't
result in changes.

> 
> > v4 Changes:
> > * None.
> > 
> > v3 Changes:
> > * Patch moved closer to the start of the patch set.
> > 
> > v2 Changes:
> > * Only setup SPI_MOSI_IDLE_HIGH flag if the controller supports it, otherwise the driver
> >   continues the same. I realized that using bits_per_word does not avoid the problem that
> >   MOSI idle state is trying to solve. If the controller drives the MOSI low between bytes
> >   during a transfer, nothing happens.  
> 
> When you say nothing happens if the controller drives MOSI low between data
> bytes you mean the data is still good in that case? Just trying to understand
> the device behavior.
> 
> Thanks,
> Marcelo
> 
> > ---
> >  drivers/iio/adc/ad7768-1.c | 15 +++++++++++++++
> >  1 file changed, 15 insertions(+)
> > 
> > diff --git a/drivers/iio/adc/ad7768-1.c b/drivers/iio/adc/ad7768-1.c
> > index c3cf04311c40..2e2d50ccb744 100644
> > --- a/drivers/iio/adc/ad7768-1.c
> > +++ b/drivers/iio/adc/ad7768-1.c
> > @@ -574,6 +574,21 @@ static int ad7768_probe(struct spi_device *spi)
> >  		return -ENOMEM;
> >  
> >  	st = iio_priv(indio_dev);
> > +	/*
> > +	 * Datasheet recommends SDI line to be kept high when data is not being
> > +	 * clocked out of the controller and the spi clock is free running,
> > +	 * to prevent accidental reset.
> > +	 * Since many controllers do not support the SPI_MOSI_IDLE_HIGH flag
> > +	 * yet, only request the MOSI idle state to enable if the controller
> > +	 * supports it.
> > +	 */
> > +	if (spi->controller->mode_bits & SPI_MOSI_IDLE_HIGH) {
> > +		spi->mode |= SPI_MOSI_IDLE_HIGH;
> > +		ret = spi_setup(spi);
> > +		if (ret < 0)
> > +			return ret;
> > +	}
> > +
> >  	st->spi = spi;
> >  
> >  	st->vref = devm_regulator_get(&spi->dev, "vref");
> > -- 
> > 2.34.1
> >   

Re: [PATCH v4 03/17] dt-bindings: iio: adc: ad7768-1: add trigger-sources property

[Jonathan Cameron]

On Thu, 6 Mar 2025 18:00:56 -0300
Jonathan Santos <Jonathan.Santos@analog.com> wrote:

> In addition to GPIO synchronization, The AD7768-1 also supports
> synchronization over SPI, which use is recommended when the GPIO
> cannot provide a pulse synchronous with the base MCLK signal. It
> consists of looping back the SYNC_OUT to the SYNC_IN pin and send
> a command via SPI to trigger the synchronization.
> 
> Add a new trigger-sources property to enable synchronization over SPI
> and future multiple devices support. This property references the
> main device (or trigger provider) responsible for generating the
> SYNC_OUT pulse to drive the SYNC_IN of device.
> 
> While at it, add description to the interrupts property.
> 
> Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com>
To me this looks fine but there was a discussion with Conor on v3,
so I'll wait for him to take another look.

Jonathan

> ---
> v4 Changes:
> * none
> 
> v3 Changes:
> * Fixed dt-bindings errors.
> * Trigger-source is set as an alternative to sync-in-gpios, so we
>   don't break the previous ABI.
> * increased maxItems from trigger-sources to 2.
> 
> v2 Changes:
> * Patch added as replacement for adi,sync-in-spi patch.
> * addressed the request for a description to interrupts property.
> ---
>  .../bindings/iio/adc/adi,ad7768-1.yaml        | 28 +++++++++++++++++--
>  1 file changed, 25 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml
> index 3ce59d4d065f..4bcc9e20fab9 100644
> --- a/Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml
> @@ -26,7 +26,19 @@ properties:
>    clock-names:
>      const: mclk
>  
> +  trigger-sources:
> +    description:
> +      Specifies the device responsible for driving the synchronization pin,
> +      as an alternative to adi,sync-in-gpios. If the own device node is
> +      referenced, The synchronization over SPI is enabled and the SYNC_OUT
> +      output will drive the SYNC_IN pin.
> +    maxItems: 2
> +
>    interrupts:
> +    description:
> +      Specifies the interrupt line associated with the ADC. This refers
> +      to the DRDY (Data Ready) pin, which signals when conversion results are
> +      available.
>      maxItems: 1
>  
>    '#address-cells':
> @@ -57,6 +69,9 @@ properties:
>    "#io-channel-cells":
>      const: 1
>  
> +  "#trigger-source-cells":
> +    const: 0
> +
>  required:
>    - compatible
>    - reg
> @@ -65,7 +80,6 @@ required:
>    - vref-supply
>    - spi-cpol
>    - spi-cpha
> -  - adi,sync-in-gpios
>  
>  patternProperties:
>    "^channel@([0-9]|1[0-5])$":
> @@ -89,6 +103,13 @@ patternProperties:
>  allOf:
>    - $ref: /schemas/spi/spi-peripheral-props.yaml#
>  
> +  - oneOf:
> +      - required:
> +          - trigger-sources
> +          - "#trigger-source-cells"
> +      - required:
> +          - adi,sync-in-gpios
> +
>  unevaluatedProperties: false
>  
>  examples:
> @@ -99,7 +120,7 @@ examples:
>          #address-cells = <1>;
>          #size-cells = <0>;
>  
> -        adc@0 {
> +        adc0: adc@0 {
>              compatible = "adi,ad7768-1";
>              reg = <0>;
>              spi-max-frequency = <2000000>;
> @@ -108,7 +129,8 @@ examples:
>              vref-supply = <&adc_vref>;
>              interrupts = <25 IRQ_TYPE_EDGE_RISING>;
>              interrupt-parent = <&gpio>;
> -            adi,sync-in-gpios = <&gpio 22 GPIO_ACTIVE_LOW>;
> +            trigger-sources = <&adc0 0>;
> +            #trigger-source-cells = <0>;
>              reset-gpios = <&gpio 27 GPIO_ACTIVE_LOW>;
>              clocks = <&ad7768_mclk>;
>              clock-names = "mclk";

Re: [PATCH v4 06/17] Documentation: ABI: add wideband filter type to sysfs-bus-iio

[Jonathan Cameron]

On Fri, 7 Mar 2025 09:45:13 -0300
Marcelo Schmitt <marcelo.schmitt1@gmail.com> wrote:

> On 03/06, Jonathan Santos wrote:
> > The Wideband Low Ripple filter is used for AD7768-1 Driver.
> > Document wideband filter option into filter_type_available
> > attribute.
> > 
> > Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com>
> > ---  
> 
> LGTM
> 
> Reviewed-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
> 
> FWIW, ADAQ7768-1 datasheet provides more info about what wideband should mean
> for these devices.
> "The wideband filter offers a filter profile similar to
> an ideal brick wall filter, making it ideal for frequency analysis."
> The proposed doc seems to portray that to some extent.

Always a bit tricky to come up with filter docs that are specific
enough but not too specific as to be useless for other devices.
I think this is a reasonable balance.

Applied (note I haven't picked up any of the dt bindings in last
few patches yet as all are somewhat dependent on the trigger-sources
one by context at least!)

Jonathan

> 
> > v4 Changes:
> > * None.
> > 
> > v3 Changes:
> > * None, since we still did not agree on a better name for this filter type.
> > 
> > v2 Changes:
> > * Removed FIR mentions.
> > ---
> >  Documentation/ABI/testing/sysfs-bus-iio | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
> > index f83bd6829285..9b879e7732cd 100644
> > --- a/Documentation/ABI/testing/sysfs-bus-iio
> > +++ b/Documentation/ABI/testing/sysfs-bus-iio
> > @@ -2291,6 +2291,8 @@ Description:
> >  		* "sinc3+pf2" - Sinc3 + device specific Post Filter 2.
> >  		* "sinc3+pf3" - Sinc3 + device specific Post Filter 3.
> >  		* "sinc3+pf4" - Sinc3 + device specific Post Filter 4.
> > +		* "wideband" - filter with wideband low ripple passband
> > +		  and sharp transition band.
> >  
> >  What:		/sys/.../events/in_proximity_thresh_either_runningperiod
> >  KernelVersion:	6.6
> > -- 
> > 2.34.1
> >   

Re: [PATCH v4 07/17] iio: adc: ad7768-1: remove unnecessary locking

[Jonathan Cameron]

On Thu, 6 Mar 2025 18:01:51 -0300
Jonathan Santos <Jonathan.Santos@analog.com> wrote:

> The current locking is only preventing a triggered buffer Transfer and a
> debugfs register access from happening at the same time. If a register
> access happens during a buffered read, the action is doomed to fail anyway,
> since we need to write a magic value to exit continuous read mode.
> 
> Remove locking from the trigger handler and use
> iio_device_claim_direct_mode() instead in the register access function.
> 
> Reviewed-by: David Lechner <dlechner@baylibre.com>
> Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com>
I don't really want more of the old school direct_mode calls
so I applied this with following tweak.  Please take a look
at the result on testing branch of iio.git.

diff --git a/drivers/iio/adc/ad7768-1.c b/drivers/iio/adc/ad7768-1.c
index a427427aa324..5a863005aca6 100644
--- a/drivers/iio/adc/ad7768-1.c
+++ b/drivers/iio/adc/ad7768-1.c
@@ -255,9 +255,8 @@ static int ad7768_reg_access(struct iio_dev *indio_dev,
        struct ad7768_state *st = iio_priv(indio_dev);
        int ret;
 
-       ret = iio_device_claim_direct_mode(indio_dev);
-       if (ret)
-               return ret;
+       if (!iio_device_claim_direct(indio_dev))
+               return -EBUSY;
 
        if (readval) {
                ret = ad7768_spi_reg_read(st, reg, 1);
@@ -269,7 +268,7 @@ static int ad7768_reg_access(struct iio_dev *indio_dev,
                ret = ad7768_spi_reg_write(st, reg, writeval);
        }
 err_release:
-       iio_device_release_direct_mode(indio_dev);
+       iio_device_release_direct(indio_dev);
 
        return ret;
 }


> ---
> v4 Changes:
> * None.
> 
> v3 Changes:
> * Also removed the mutex_init and lock variable.
> 
> v2 Changes:
> * New patch in v2. It replaces the guard(mutex) patch. 
> ---
>  drivers/iio/adc/ad7768-1.c | 21 +++++++++------------
>  1 file changed, 9 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/iio/adc/ad7768-1.c b/drivers/iio/adc/ad7768-1.c
> index 2e2d50ccb744..f5509a0a36ab 100644
> --- a/drivers/iio/adc/ad7768-1.c
> +++ b/drivers/iio/adc/ad7768-1.c
> @@ -154,7 +154,6 @@ static const struct iio_chan_spec ad7768_channels[] = {
>  struct ad7768_state {
>  	struct spi_device *spi;
>  	struct regulator *vref;
> -	struct mutex lock;
>  	struct clk *mclk;
>  	unsigned int mclk_freq;
>  	unsigned int samp_freq;
> @@ -256,18 +255,21 @@ static int ad7768_reg_access(struct iio_dev *indio_dev,
>  	struct ad7768_state *st = iio_priv(indio_dev);
>  	int ret;
>  
> -	mutex_lock(&st->lock);
> +	ret = iio_device_claim_direct_mode(indio_dev);
> +	if (ret)
> +		return ret;
> +
>  	if (readval) {
>  		ret = ad7768_spi_reg_read(st, reg, 1);
>  		if (ret < 0)
> -			goto err_unlock;
> +			goto err_release;
>  		*readval = ret;
>  		ret = 0;
>  	} else {
>  		ret = ad7768_spi_reg_write(st, reg, writeval);
>  	}
> -err_unlock:
> -	mutex_unlock(&st->lock);
> +err_release:
> +	iio_device_release_direct_mode(indio_dev);
>  
>  	return ret;
>  }
> @@ -471,18 +473,15 @@ static irqreturn_t ad7768_trigger_handler(int irq, void *p)
>  	struct ad7768_state *st = iio_priv(indio_dev);
>  	int ret;
>  
> -	mutex_lock(&st->lock);
> -
>  	ret = spi_read(st->spi, &st->data.scan.chan, 3);
>  	if (ret < 0)
> -		goto err_unlock;
> +		goto out;
>  
>  	iio_push_to_buffers_with_timestamp(indio_dev, &st->data.scan,
>  					   iio_get_time_ns(indio_dev));
>  
> -err_unlock:
> +out:
>  	iio_trigger_notify_done(indio_dev->trig);
> -	mutex_unlock(&st->lock);
>  
>  	return IRQ_HANDLED;
>  }
> @@ -611,8 +610,6 @@ static int ad7768_probe(struct spi_device *spi)
>  
>  	st->mclk_freq = clk_get_rate(st->mclk);
>  
> -	mutex_init(&st->lock);
> -
>  	indio_dev->channels = ad7768_channels;
>  	indio_dev->num_channels = ARRAY_SIZE(ad7768_channels);
>  	indio_dev->name = spi_get_device_id(spi)->name;

Re: [PATCH v4 08/17] iio: adc: ad7768-1: convert driver to use regmap

[Jonathan Cameron]

On Fri, 7 Mar 2025 10:20:04 -0300
Marcelo Schmitt <marcelo.schmitt1@gmail.com> wrote:

> On 03/06, Jonathan Santos wrote:
> > Convert the AD7768-1 driver to use the regmap API for register
> > access. This change simplifies and standardizes register interactions,
> > reducing code duplication and improving maintainability.
> > 
> > Create two regmap configurations, one for 8-bit register values and
> > other for 24-bit register values.
> > 
> > Since we are using regmap now, define the remaining registers from 0x32
> > to 0x34.
> > 

Marcelo, crop to just the bit you reply to.  Reduces chance that
key part is missed when someone is scrolling through.

> > -static int ad7768_spi_reg_write(struct ad7768_state *st,
> > -				unsigned int addr,
> > -				unsigned int val)
> > -{
> > -	st->data.d8[0] = AD7768_WR_FLAG_MSK(addr);
> > -	st->data.d8[1] = val & 0xFF;
> > +static const struct regmap_config ad7768_regmap_config = {
> > +	.name = "ad7768-1-8",
> > +	.reg_bits = 8,
> > +	.val_bits = 8,
> > +	.read_flag_mask = BIT(6),
> > +	.rd_table = &ad7768_regmap_rd_table,
> > +	.wr_table = &ad7768_regmap_wr_table,
> > +	.max_register = AD7768_REG_ACCESS_KEY,
> > +	.use_single_write = true,
> > +	.use_single_read = true,
> > +};
> >  
> > -	return spi_write(st->spi, st->data.d8, 2);
> > -}
> > +static const struct regmap_range ad7768_regmap24_rd_ranges[] = {
> > +	regmap_reg_range(AD7768_REG24_ADC_DATA, AD7768_REG24_ADC_DATA),
> > +	regmap_reg_range(AD7768_REG24_COEFF_DATA, AD7768_REG24_COEFF_DATA),  
> 
> So, this device has only two registers that are 24-bit size?
> Also, one of those is the ADC_DATA register which you will probably want
> to read with optimized SPI messages in the future (devm_spi_optimize_message()).
> That makes me wonder if the 24-bit regmap worth's the boiler plate to have it.
> Does the driver access AD7768_REG24_COEFF_DATA after the patches from this
> series is applied? If not, maybe drop the 24-bit regmap and implement ADC_DATA
> with usual spi_message/spi_transfer interfaces?

Given it's used today and is better than a mix of regmap and non-regmap
(as no need to go SPI specific yet) I'd keep it for now.  If it ends up
completely unused, then fair enough to rip it out again.  It's not
a lot of code.

Jonathan

Re: [PATCH v4 09/17] iio: adc: ad7768-1: Add reset gpio

[Jonathan Cameron]

On Fri, 7 Mar 2025 10:42:09 -0300
Marcelo Schmitt <marcelo.schmitt1@gmail.com> wrote:

> On 03/06, Jonathan Santos wrote:
> > From: Sergiu Cuciurean <sergiu.cuciurean@analog.com>
> > 
> > Depending on the controller, the default state of a gpio can vary. This
> > change excludes the probability that the dafult state of the ADC reset
> > gpio will be HIGH if it will be passed as reference in the devicetree.  
> 
> The description doesn't seem to match the changes nor the patch title. You are
> essentinally adding support for hardware reset. Change the commit description to
> reflect that.
> 
> The default state of GPIOs would not impact device reset because (in theory)
> they weren't being connected to the reset pin prevously.
Also possible some other entity was doing appropriate reset (sometimes
it's just how the system is wired - puts appropriate voltage to come out
of reset after some other action - no kernel involvement).

Agreed in general though that this is simpler described as just implementing
hardware reset and not worry about those details.

Jonathan

> 
> > 
> > Reviewed-by: David Lechner <dlechner@baylibre.com>
> > Reviewed-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
> > Signed-off-by: Sergiu Cuciurean <sergiu.cuciurean@analog.com>
> > Co-developed-by: Jonathan Santos <Jonathan.Santos@analog.com>
> > Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com>
> > ---
> > v4 Changes:
> > * None.
> > 
> > v3 Changes:
> > * fixed SoB order.
> > * increased delay after finishing the reset action to 200us, as the
> >   datasheet recommends.
> > 
> > v2 Changes:
> > * Replaced usleep_range() for fsleep() and gpiod_direction_output() for 
> >   gpiod_set_value_cansleep().
> > * Reset via SPI register is performed if the Reset GPIO is not defined. 
> > ---
> >  drivers/iio/adc/ad7768-1.c | 36 ++++++++++++++++++++++++------------
> >  1 file changed, 24 insertions(+), 12 deletions(-)
> > 
> > diff --git a/drivers/iio/adc/ad7768-1.c b/drivers/iio/adc/ad7768-1.c
> > index 04a26e5b7d5c..86f44d28c478 100644
> > --- a/drivers/iio/adc/ad7768-1.c
> > +++ b/drivers/iio/adc/ad7768-1.c
> > @@ -166,6 +166,7 @@ struct ad7768_state {
> >  	struct completion completion;
> >  	struct iio_trigger *trig;
> >  	struct gpio_desc *gpio_sync_in;
> > +	struct gpio_desc *gpio_reset;
> >  	const char *labels[ARRAY_SIZE(ad7768_channels)];
> >  	/*
> >  	 * DMA (thus cache coherency maintenance) may require the
> > @@ -487,19 +488,30 @@ static int ad7768_setup(struct ad7768_state *st)
> >  {
> >  	int ret;
> >  
> > -	/*
> > -	 * Two writes to the SPI_RESET[1:0] bits are required to initiate
> > -	 * a software reset. The bits must first be set to 11, and then
> > -	 * to 10. When the sequence is detected, the reset occurs.
> > -	 * See the datasheet, page 70.
> > -	 */
> > -	ret = regmap_write(st->regmap, AD7768_REG_SYNC_RESET, 0x3);
> > -	if (ret)
> > -		return ret;
> > +	st->gpio_reset = devm_gpiod_get_optional(&st->spi->dev, "reset",
> > +						 GPIOD_OUT_HIGH);
> > +	if (IS_ERR(st->gpio_reset))
> > +		return PTR_ERR(st->gpio_reset);
> >  
> > -	ret = regmap_write(st->regmap, AD7768_REG_SYNC_RESET, 0x2);
> > -	if (ret)
> > -		return ret;
> > +	if (st->gpio_reset) {
> > +		fsleep(10);
> > +		gpiod_set_value_cansleep(st->gpio_reset, 0);
> > +		fsleep(200);
> > +	} else {
> > +		/*
> > +		 * Two writes to the SPI_RESET[1:0] bits are required to initiate
> > +		 * a software reset. The bits must first be set to 11, and then
> > +		 * to 10. When the sequence is detected, the reset occurs.
> > +		 * See the datasheet, page 70.
> > +		 */
> > +		ret = regmap_write(st->regmap, AD7768_REG_SYNC_RESET, 0x3);
> > +		if (ret)
> > +			return ret;
> > +
> > +		ret = regmap_write(st->regmap, AD7768_REG_SYNC_RESET, 0x2);
> > +		if (ret)
> > +			return ret;
> > +	}
> >  
> >  	st->gpio_sync_in = devm_gpiod_get(&st->spi->dev, "adi,sync-in",
> >  					  GPIOD_OUT_LOW);
> > -- 
> > 2.34.1
> >   

Re: [PATCH v4 10/17] iio: adc: ad7768-1: Move buffer allocation to a separate function

[Jonathan Cameron]

On Thu, 6 Mar 2025 18:02:46 -0300
Jonathan Santos <Jonathan.Santos@analog.com> wrote:

> From: Sergiu Cuciurean <sergiu.cuciurean@analog.com>
> 
> This change moves the buffer allocation in a separate function, making
> space for adding another type of iio buffer if needed.

Moving the trigger allocation etc as well.  I'd mention that.
Otherwise this looks fine.

> 
> Reviewed-by: David Lechner <dlechner@baylibre.com>
> Signed-off-by: Sergiu Cuciurean <sergiu.cuciurean@analog.com>
> Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com>

Re: [PATCH v4 11/17] iio: adc: ad7768-1: add regulator to control VCM output

[Jonathan Cameron]

On Thu, 6 Mar 2025 18:02:59 -0300
Jonathan Santos <Jonathan.Santos@analog.com> wrote:

> The VCM output voltage can be used as a common-mode voltage within the
> amplifier preconditioning circuits external to the AD7768-1.
> 
> This change allows the user to configure VCM output using the regulator
> framework.
> 
> Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com>
Looks fine but as likely you will be doing a v5, please switch
to the new iio_device_claim/release_direct() functions.
If I had applied up to here I'd probably just have tweaked this whilst
applying but given a few other tweaks needed, please do this one
as well for v5.

Thanks,

Jonathan

> @@ -644,6 +654,172 @@ static int ad7768_triggered_buffer_alloc(struct iio_dev *indio_dev)
>  					       &ad7768_buffer_ops);
>  }
>  
> +static int ad7768_vcm_enable(struct regulator_dev *rdev)
> +{
> +	struct iio_dev *indio_dev = rdev_get_drvdata(rdev);
> +	struct ad7768_state *st = iio_priv(indio_dev);
> +	int ret, regval;
> +
> +	if (!indio_dev)
> +		return -EINVAL;
> +
> +	ret = iio_device_claim_direct_mode(indio_dev);
As below.

> +	if (ret)
> +		return ret;
> +
> +	/* To enable, set the last selected output */
> +	regval = AD7768_REG_ANALOG2_VCM(st->vcm_output_sel + 1);
> +	ret = regmap_update_bits(st->regmap, AD7768_REG_ANALOG2,
> +				 AD7768_REG_ANALOG2_VCM_MSK, regval);
> +	iio_device_release_direct_mode(indio_dev);
> +
> +	return ret;
> +}
> +
> +static int ad7768_vcm_disable(struct regulator_dev *rdev)
> +{
> +	struct iio_dev *indio_dev = rdev_get_drvdata(rdev);
> +	struct ad7768_state *st = iio_priv(indio_dev);
> +	int ret;
> +
> +	if (!indio_dev)
> +		return -EINVAL;
> +
> +	ret = iio_device_claim_direct_mode(indio_dev);
As looks likely you'll be doing a v5 for other minor stuff please
rebase on the testing branch of iio.git (as I've picked up some of this
series) or togreg once that is pushed out and use
	if (!iio_device_claim_direct(indio_dev))
		return -EBUSY;
+ iio_device_release_direct()
to get the variants adjusted to play better with sparse.

> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_update_bits(st->regmap, AD7768_REG_ANALOG2,
> +				 AD7768_REG_ANALOG2_VCM_MSK, AD7768_VCM_OFF);
> +	iio_device_release_direct_mode(indio_dev);
> +
> +	return ret;
> +}
> +
> +static int ad7768_vcm_is_enabled(struct regulator_dev *rdev)
> +{
> +	struct iio_dev *indio_dev = rdev_get_drvdata(rdev);
> +	struct ad7768_state *st = iio_priv(indio_dev);
> +	int ret, val;
> +
> +	if (!indio_dev)
> +		return -EINVAL;
> +
> +	ret = iio_device_claim_direct_mode(indio_dev);
As above.
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_read(st->regmap, AD7768_REG_ANALOG2, &val);
> +	if (ret)
> +		goto err_release;
> +
> +	ret = FIELD_GET(AD7768_REG_ANALOG2_VCM_MSK, val) != AD7768_VCM_OFF;
> +err_release:
> +	iio_device_release_direct_mode(indio_dev);
> +
> +	return ret;
> +}
> +
> +static int ad7768_set_voltage_sel(struct regulator_dev *rdev,
> +				  unsigned int selector)
> +{
> +	unsigned int regval = AD7768_REG_ANALOG2_VCM(selector + 1);
> +	struct iio_dev *indio_dev = rdev_get_drvdata(rdev);
> +	struct ad7768_state *st = iio_priv(indio_dev);
> +	int ret;
> +
> +	if (!indio_dev)
> +		return -EINVAL;
> +
> +	ret = iio_device_claim_direct_mode(indio_dev);
As above.
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_update_bits(st->regmap, AD7768_REG_ANALOG2,
> +				 AD7768_REG_ANALOG2_VCM_MSK, regval);
> +	iio_device_release_direct_mode(indio_dev);
> +	st->vcm_output_sel = selector;
> +
> +	return ret;
> +}
> +
> +static int ad7768_get_voltage_sel(struct regulator_dev *rdev)
> +{
> +	struct iio_dev *indio_dev = rdev_get_drvdata(rdev);
> +	struct ad7768_state *st = iio_priv(indio_dev);
> +	int ret, val;
> +
> +	if (!indio_dev)
> +		return -EINVAL;
> +
> +	ret = iio_device_claim_direct_mode(indio_dev);
As above.
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_read(st->regmap, AD7768_REG_ANALOG2, &val);
> +	if (ret)
> +		goto err_release;
> +
> +	val = FIELD_GET(AD7768_REG_ANALOG2_VCM_MSK, val);
> +	ret = clamp(val, 1, (int)rdev->desc->n_voltages) - 1;
> +err_release:
> +	iio_device_release_direct_mode(indio_dev);
> +
> +	return ret;
> +}

Re: [PATCH v4 12/17] iio: adc: ad7768-1: Add GPIO controller support

[Jonathan Cameron]

On Thu, 6 Mar 2025 18:03:13 -0300
Jonathan Santos <Jonathan.Santos@analog.com> wrote:

> From: Sergiu Cuciurean <sergiu.cuciurean@analog.com>
> 
> The AD7768-1 has the ability to control other local hardware (such as gain
> stages),to power down other blocks in the signal chain, or read local
> status signals over the SPI interface.
> 
> Add direct mode conditional locks in the gpio callbacks to prevent register
> access when the device is in buffered mode.
> 
> This change exports the AD7768-1's four gpios and makes them accessible
> at an upper layer.
> 
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Sergiu Cuciurean <sergiu.cuciurean@analog.com>
> Co-developed-by: Jonathan Santos <Jonathan.Santos@analog.com>
> Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com>
More iio_device_claim_direct() conversions to make in here.
That's just a bit of unlucky timing vs that series getting merged.

Jonathan

Re: [PATCH v4 16/17] iio: adc: ad7768-1: add filter type and oversampling ratio attributes

[Jonathan Cameron]

On Thu, 6 Mar 2025 18:04:24 -0300
Jonathan Santos <Jonathan.Santos@analog.com> wrote:

> Separate filter type and decimation rate from the sampling frequency
> attribute. The new filter type attribute enables sinc3, sinc3+rej60
> and wideband filters, which were previously unavailable.
> 
> Previously, combining decimation and MCLK divider in the sampling
> frequency obscured performance trade-offs. Lower MCLK divider
> settings increase power usage, while lower decimation rates reduce
> precision by decreasing averaging. By creating an oversampling
> attribute, which controls the decimation, users gain finer control
> over performance.
> 
> The addition of those attributes allows a wider range of sampling
> frequencies and more access to the device features. Sampling frequency
> table is updated after every digital filter paramerter change.
> 
> Co-developed-by: Pop Paul <paul.pop@analog.com>
> Signed-off-by: Pop Paul <paul.pop@analog.com>
> Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com>
> ---
> v4 Changes:
> * Sampling frequency table is dinamically updated after every

Good to spell check. Dynamically

>   filter configuration.

Currently this runs into the potential race conditions we get with
read_avail callbacks.  If we update the avail values in parallel
with consumer code in a kernel driver reading them we can get tearing.
So better if possible to do it all before those interfaces are exposed
and just pick from a set of static arrays.

> +static struct iio_chan_spec_ext_info ad7768_ext_info[] = {
> +	IIO_ENUM("filter_type", IIO_SHARED_BY_ALL, &ad7768_flt_type_iio_enum),
> +	IIO_ENUM_AVAILABLE("filter_type", IIO_SHARED_BY_ALL, &ad7768_flt_type_iio_enum),
> +	{ },

No trailing comma on a terminating entry as we don't want it to be easy
to accidentally add stuff after this.

> +};


> +static int ad7768_configure_dig_fil(struct iio_dev *dev,
> +				    enum ad7768_filter_type filter_type,
> +				    unsigned int dec_rate)
> +{
> +	struct ad7768_state *st = iio_priv(dev);
> +	unsigned int dec_rate_idx, dig_filter_regval;
> +	int ret;
> +
> +	switch (filter_type) {
> +	case AD7768_FILTER_SINC3:
> +		dig_filter_regval = AD7768_DIG_FIL_FIL(AD7768_FILTER_REGVAL_SINC3);
> +		break;
> +	case AD7768_FILTER_SINC3_REJ60:
> +		dig_filter_regval = AD7768_DIG_FIL_FIL(AD7768_FILTER_REGVAL_SINC3_REJ60);
> +		break;
> +	case AD7768_FILTER_WIDEBAND:
> +		/* Skip decimations 8 and 16, not supported by the wideband filter */
> +		dec_rate_idx = find_closest(dec_rate, &ad7768_dec_rate_values[2],
> +					    ARRAY_SIZE(ad7768_dec_rate_values) - 2);
> +		dig_filter_regval = AD7768_DIG_FIL_FIL(AD7768_FILTER_REGVAL_WIDEBAND) |
> +				    AD7768_DIG_FIL_DEC_RATE(dec_rate_idx);
> +		/* Correct the index offset */
> +		dec_rate_idx += 2;
> +		break;
> +	case AD7768_FILTER_SINC5:
> +		dec_rate_idx = find_closest(dec_rate, ad7768_dec_rate_values,
> +					    ARRAY_SIZE(ad7768_dec_rate_values));
> +
> +		/*
> +		 * Decimations 8 (idx 0) and 16 (idx 1) are set in the
> +		 * FILTER[6:4] field. The other decimations are set in the
> +		 * DEC_RATE[2:0] field, and the idx need to be offsetted by two.
> +		 */
> +		if (dec_rate_idx == 0)
> +			dig_filter_regval = AD7768_DIG_FIL_FIL(AD7768_FILTER_REGVAL_SINC5_X8);
> +		else if (dec_rate_idx == 1)
> +			dig_filter_regval = AD7768_DIG_FIL_FIL(AD7768_FILTER_REGVAL_SINC5_X16);
> +		else
> +			dig_filter_regval = AD7768_DIG_FIL_FIL(AD7768_FILTER_REGVAL_SINC5) |
> +					    AD7768_DIG_FIL_DEC_RATE(dec_rate_idx - 2);
> +		break;
> +	}
> +
> +	ret = regmap_write(st->regmap, AD7768_REG_DIGITAL_FILTER, dig_filter_regval);
> +	if (ret)
>  		return ret;
>  
> -	/* A sync-in pulse is required every time the filter dec rate changes */
> +	st->filter_type = filter_type;
> +	/*
> +	 * The decimation for SINC3 filters are configured in different
> +	 * registers
> +	 */
> +	if (filter_type == AD7768_FILTER_SINC3 ||
> +	    filter_type == AD7768_FILTER_SINC3_REJ60) {
> +		ret = ad7768_set_sinc3_dec_rate(st, dec_rate);
> +		if (ret)
> +			return ret;
> +	} else {
> +		st->oversampling_ratio =  ad7768_dec_rate_values[dec_rate_idx];

Looks like an extra space after =

> +	}
> +
> +	ad7768_fill_samp_freq_tbl(st);

This is opens a potentially complex race condition if we have the an
in kernel consumer reading the data in this array as it is being updated
(currently we can't stop that happening though solutions to that problem
have been much discussed). 

There aren't that many oversampling ratios so perhaps it is better
to precalculate all the potential available values as an array indexed
by oversampling ratio.  That way all the data is const, it's just possible
to get stale pointer to the wrong entry which can always happen anyway
if the read vs update happen in different entities.

> +
> +	/* A sync-in pulse is required after every configuration change */
>  	return ad7768_send_sync_pulse(st);
>  }

>  
> +static int ad7768_write_raw(struct iio_dev *indio_dev,
> +			    struct iio_chan_spec const *chan,
> +			    int val, int val2, long info)
> +{
> +	int ret;
> +
> +	ret = iio_device_claim_direct_mode(indio_dev);

update to use if (!iio_device_claim_direct())

> +	if (ret)
> +		return ret;
> +
> +	ret = __ad7768_write_raw(indio_dev, chan, val, val2, info);
> +	iio_device_release_direct_mode(indio_dev);
> +
> +	return ret;
> +}

Re: [PATCH v4 04/17] dt-bindings: iio: adc: ad7768-1: Document GPIO controller

[Linus Walleij]

On Thu, Mar 6, 2025 at 10:01 PM Jonathan Santos
<Jonathan.Santos@analog.com> wrote:

> The AD7768-1 ADC exports four bidirectional GPIOs accessible
> via register map.
>
> Document GPIO properties necessary to enable GPIO controller for this
> device.
>
> Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
> Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: [PATCH v4 02/17] iio: adc: ad7768-1: set MOSI idle state to prevent accidental reset

[Jonathan Santos]

On 03/07, Marcelo Schmitt wrote:
> On 03/06, Jonathan Santos wrote:
> > Datasheet recommends Setting the MOSI idle state to high in order to
> > prevent accidental reset of the device when SCLK is free running.
> > This happens when the controller clocks out a 1 followed by 63 zeros
> > while the CS is held low.
> > 
> > Check if SPI controller supports SPI_MOSI_IDLE_HIGH flag and set it.
> > 
> > Fixes: a5f8c7da3dbe ("iio: adc: Add AD7768-1 ADC basic support")
> > Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com>
> > ---
> 
> LGTM
> 
> Reviewed-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
> 
> > v4 Changes:
> > * None.
> > 
> > v3 Changes:
> > * Patch moved closer to the start of the patch set.
> > 
> > v2 Changes:
> > * Only setup SPI_MOSI_IDLE_HIGH flag if the controller supports it, otherwise the driver
> >   continues the same. I realized that using bits_per_word does not avoid the problem that
> >   MOSI idle state is trying to solve. If the controller drives the MOSI low between bytes
> >   during a transfer, nothing happens.
> 
> When you say nothing happens if the controller drives MOSI low between data
> bytes you mean the data is still good in that case? Just trying to understand
> the device behavior.

Yes, it means that it does not affect the buffered reads and data is
still OK.

> 
> Thanks,
> Marcelo
> 
> > ---
> >  drivers/iio/adc/ad7768-1.c | 15 +++++++++++++++
> >  1 file changed, 15 insertions(+)
> > 
> > diff --git a/drivers/iio/adc/ad7768-1.c b/drivers/iio/adc/ad7768-1.c
> > index c3cf04311c40..2e2d50ccb744 100644
> > --- a/drivers/iio/adc/ad7768-1.c
> > +++ b/drivers/iio/adc/ad7768-1.c
> > @@ -574,6 +574,21 @@ static int ad7768_probe(struct spi_device *spi)
> >  		return -ENOMEM;
> >  
> >  	st = iio_priv(indio_dev);
> > +	/*
> > +	 * Datasheet recommends SDI line to be kept high when data is not being
> > +	 * clocked out of the controller and the spi clock is free running,
> > +	 * to prevent accidental reset.
> > +	 * Since many controllers do not support the SPI_MOSI_IDLE_HIGH flag
> > +	 * yet, only request the MOSI idle state to enable if the controller
> > +	 * supports it.
> > +	 */
> > +	if (spi->controller->mode_bits & SPI_MOSI_IDLE_HIGH) {
> > +		spi->mode |= SPI_MOSI_IDLE_HIGH;
> > +		ret = spi_setup(spi);
> > +		if (ret < 0)
> > +			return ret;
> > +	}
> > +
> >  	st->spi = spi;
> >  
> >  	st->vref = devm_regulator_get(&spi->dev, "vref");
> > -- 
> > 2.34.1
> > 

Re: [PATCH v4 16/17] iio: adc: ad7768-1: add filter type and oversampling ratio attributes

[Jonathan Santos]

On 03/08, Jonathan Cameron wrote:
> On Thu, 6 Mar 2025 18:04:24 -0300
> Jonathan Santos <Jonathan.Santos@analog.com> wrote:
> 
> > Separate filter type and decimation rate from the sampling frequency
> > attribute. The new filter type attribute enables sinc3, sinc3+rej60
> > and wideband filters, which were previously unavailable.
> > 
> > Previously, combining decimation and MCLK divider in the sampling
> > frequency obscured performance trade-offs. Lower MCLK divider
> > settings increase power usage, while lower decimation rates reduce
> > precision by decreasing averaging. By creating an oversampling
> > attribute, which controls the decimation, users gain finer control
F> > over performance.
> > 
> > The addition of those attributes allows a wider range of sampling
> > frequencies and more access to the device features. Sampling frequency
> > table is updated after every digital filter paramerter change.
> > 
> > Co-developed-by: Pop Paul <paul.pop@analog.com>
> > Signed-off-by: Pop Paul <paul.pop@analog.com>
> > Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com>
> > ---
> > v4 Changes:
> > * Sampling frequency table is dinamically updated after every
> 
> Good to spell check. Dynamically
> 
> >   filter configuration.
> 
> Currently this runs into the potential race conditions we get with
> read_avail callbacks.  If we update the avail values in parallel
> with consumer code in a kernel driver reading them we can get tearing.
> So better if possible to do it all before those interfaces are exposed
> and just pick from a set of static arrays.
> 
I understand the problem, but the number of possible sampling
frequencies is quite large because of the decimation/OSR:

-> For wideband there are 6 decimations available.
-> For Sinc5 there are 8 decimations.
-> For sinc3 (here's the problem) we have up to 5119 decimation options.
From x32 to x163,840 (mclk_div = 2) with a 32 step.

BTW, that's why we use ranges for `oversampling_ratio_available`
attribute.

To reflect all sampling frequencies combinations (fref/(mclk_div *
OSR)), we would need a considerably large array. We did not have this
problem before because we were not supporting the sinc3 filter.

> > +static struct iio_chan_spec_ext_info ad7768_ext_info[] = {
> > +	IIO_ENUM("filter_type", IIO_SHARED_BY_ALL, &ad7768_flt_type_iio_enum),
> > +	IIO_ENUM_AVAILABLE("filter_type", IIO_SHARED_BY_ALL, &ad7768_flt_type_iio_enum),
> > +	{ },
> 
> No trailing comma on a terminating entry as we don't want it to be easy
> to accidentally add stuff after this.
> 
> > +};
> 
> 
> > +static int ad7768_configure_dig_fil(struct iio_dev *dev,
> > +				    enum ad7768_filter_type filter_type,
> > +				    unsigned int dec_rate)
> > +{
> > +	struct ad7768_state *st = iio_priv(dev);
> > +	unsigned int dec_rate_idx, dig_filter_regval;
> > +	int ret;
> > +
> > +	switch (filter_type) {
> > +	case AD7768_FILTER_SINC3:
> > +		dig_filter_regval = AD7768_DIG_FIL_FIL(AD7768_FILTER_REGVAL_SINC3);
> > +		break;
> > +	case AD7768_FILTER_SINC3_REJ60:
> > +		dig_filter_regval = AD7768_DIG_FIL_FIL(AD7768_FILTER_REGVAL_SINC3_REJ60);
> > +		break;
> > +	case AD7768_FILTER_WIDEBAND:
> > +		/* Skip decimations 8 and 16, not supported by the wideband filter */
> > +		dec_rate_idx = find_closest(dec_rate, &ad7768_dec_rate_values[2],
> > +					    ARRAY_SIZE(ad7768_dec_rate_values) - 2);
> > +		dig_filter_regval = AD7768_DIG_FIL_FIL(AD7768_FILTER_REGVAL_WIDEBAND) |
> > +				    AD7768_DIG_FIL_DEC_RATE(dec_rate_idx);
> > +		/* Correct the index offset */
> > +		dec_rate_idx += 2;
> > +		break;
> > +	case AD7768_FILTER_SINC5:
> > +		dec_rate_idx = find_closest(dec_rate, ad7768_dec_rate_values,
> > +					    ARRAY_SIZE(ad7768_dec_rate_values));
> > +
> > +		/*
> > +		 * Decimations 8 (idx 0) and 16 (idx 1) are set in the
> > +		 * FILTER[6:4] field. The other decimations are set in the
> > +		 * DEC_RATE[2:0] field, and the idx need to be offsetted by two.
> > +		 */
> > +		if (dec_rate_idx == 0)
> > +			dig_filter_regval = AD7768_DIG_FIL_FIL(AD7768_FILTER_REGVAL_SINC5_X8);
> > +		else if (dec_rate_idx == 1)
> > +			dig_filter_regval = AD7768_DIG_FIL_FIL(AD7768_FILTER_REGVAL_SINC5_X16);
> > +		else
> > +			dig_filter_regval = AD7768_DIG_FIL_FIL(AD7768_FILTER_REGVAL_SINC5) |
> > +					    AD7768_DIG_FIL_DEC_RATE(dec_rate_idx - 2);
> > +		break;
> > +	}
> > +
> > +	ret = regmap_write(st->regmap, AD7768_REG_DIGITAL_FILTER, dig_filter_regval);
> > +	if (ret)
> >  		return ret;
> >  
> > -	/* A sync-in pulse is required every time the filter dec rate changes */
> > +	st->filter_type = filter_type;
> > +	/*
> > +	 * The decimation for SINC3 filters are configured in different
> > +	 * registers
> > +	 */
> > +	if (filter_type == AD7768_FILTER_SINC3 ||
> > +	    filter_type == AD7768_FILTER_SINC3_REJ60) {
> > +		ret = ad7768_set_sinc3_dec_rate(st, dec_rate);
> > +		if (ret)
> > +			return ret;
> > +	} else {
> > +		st->oversampling_ratio =  ad7768_dec_rate_values[dec_rate_idx];
> 
> Looks like an extra space after =
> 

Sorry about that

> > +	}
> > +
> > +	ad7768_fill_samp_freq_tbl(st);
> 
> This is opens a potentially complex race condition if we have the an
> in kernel consumer reading the data in this array as it is being updated
> (currently we can't stop that happening though solutions to that problem
> have been much discussed). 
> 
> There aren't that many oversampling ratios so perhaps it is better
> to precalculate all the potential available values as an array indexed

As I said above, unfortunately there are many OSR options.

> by oversampling ratio.  That way all the data is const, it's just possible
> to get stale pointer to the wrong entry which can always happen anyway
> if the read vs update happen in different entities.
> 
> > +
> > +	/* A sync-in pulse is required after every configuration change */
> >  	return ad7768_send_sync_pulse(st);
> >  }
> 
> >  
> > +static int ad7768_write_raw(struct iio_dev *indio_dev,
> > +			    struct iio_chan_spec const *chan,
> > +			    int val, int val2, long info)
> > +{
> > +	int ret;
> > +
> > +	ret = iio_device_claim_direct_mode(indio_dev);
> 
> update to use if (!iio_device_claim_direct())
> 

OK!

> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = __ad7768_write_raw(indio_dev, chan, val, val2, info);
> > +	iio_device_release_direct_mode(indio_dev);
> > +
> > +	return ret;
> > +}
> 

Re: [PATCH v4 03/17] dt-bindings: iio: adc: ad7768-1: add trigger-sources property

[David Lechner]

On 3/6/25 3:00 PM, Jonathan Santos wrote:
> In addition to GPIO synchronization, The AD7768-1 also supports
> synchronization over SPI, which use is recommended when the GPIO
> cannot provide a pulse synchronous with the base MCLK signal. It
> consists of looping back the SYNC_OUT to the SYNC_IN pin and send
> a command via SPI to trigger the synchronization.
> 
> Add a new trigger-sources property to enable synchronization over SPI
> and future multiple devices support. This property references the
> main device (or trigger provider) responsible for generating the
> SYNC_OUT pulse to drive the SYNC_IN of device.
> 
> While at it, add description to the interrupts property.
> 
> Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com>
> ---
> v4 Changes:
> * none
> 
> v3 Changes:
> * Fixed dt-bindings errors.
> * Trigger-source is set as an alternative to sync-in-gpios, so we
>   don't break the previous ABI.
> * increased maxItems from trigger-sources to 2.
> 
> v2 Changes:
> * Patch added as replacement for adi,sync-in-spi patch.
> * addressed the request for a description to interrupts property.
> ---
>  .../bindings/iio/adc/adi,ad7768-1.yaml        | 28 +++++++++++++++++--
>  1 file changed, 25 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml
> index 3ce59d4d065f..4bcc9e20fab9 100644
> --- a/Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml
> @@ -26,7 +26,19 @@ properties:
>    clock-names:
>      const: mclk
>  
> +  trigger-sources:
> +    description:
> +      Specifies the device responsible for driving the synchronization pin,
> +      as an alternative to adi,sync-in-gpios. If the own device node is
> +      referenced, The synchronization over SPI is enabled and the SYNC_OUT
> +      output will drive the SYNC_IN pin.
> +    maxItems: 2

This says maxItems: 2 but the description only describes one, namely /SYNC_IN.
IIRC, the 2nd one is for the optional /START input.

> +
>    interrupts:
> +    description:
> +      Specifies the interrupt line associated with the ADC. This refers
> +      to the DRDY (Data Ready) pin, which signals when conversion results are
> +      available.
>      maxItems: 1
>  
>    '#address-cells':
> @@ -57,6 +69,9 @@ properties:
>    "#io-channel-cells":
>      const: 1
>  
> +  "#trigger-source-cells":
> +    const: 0
> +
>  required:
>    - compatible
>    - reg
> @@ -65,7 +80,6 @@ required:
>    - vref-supply
>    - spi-cpol
>    - spi-cpha
> -  - adi,sync-in-gpios
>  
>  patternProperties:
>    "^channel@([0-9]|1[0-5])$":
> @@ -89,6 +103,13 @@ patternProperties:
>  allOf:
>    - $ref: /schemas/spi/spi-peripheral-props.yaml#
>  
> +  - oneOf:
> +      - required:
> +          - trigger-sources
> +          - "#trigger-source-cells"
> +      - required:
> +          - adi,sync-in-gpios
> +
>  unevaluatedProperties: false
>  
>  examples:
> @@ -99,7 +120,7 @@ examples:
>          #address-cells = <1>;
>          #size-cells = <0>;
>  
> -        adc@0 {
> +        adc0: adc@0 {
>              compatible = "adi,ad7768-1";
>              reg = <0>;
>              spi-max-frequency = <2000000>;
> @@ -108,7 +129,8 @@ examples:
>              vref-supply = <&adc_vref>;
>              interrupts = <25 IRQ_TYPE_EDGE_RISING>;
>              interrupt-parent = <&gpio>;
> -            adi,sync-in-gpios = <&gpio 22 GPIO_ACTIVE_LOW>;
> +            trigger-sources = <&adc0 0>;

# cells is 0 but this has one cell specified. So one or the other is incorrect.
Since there are other outputs that could be used as triggers, e.g. /DRDY could
be used as a SPI offload trigger, having 1 cell seems prudent.

> +            #trigger-source-cells = <0>;
>              reset-gpios = <&gpio 27 GPIO_ACTIVE_LOW>;
>              clocks = <&ad7768_mclk>;
>              clock-names = "mclk";

Re: [PATCH v4 05/17] dt-bindings: iio: adc: ad7768-1: document regulator provider property

[David Lechner]

On 3/6/25 3:01 PM, Jonathan Santos wrote:
> The AD7768-1 provides a buffered common-mode voltage output
> on the VCM pin that can be used to bias analog input signals.
> 
> Add regulators property to enable the use of the VCM output,
> referenced here as vcm-output, by any other device.
> 
> Acked-by: Conor Dooley <conor.dooley@microchip.com>
> Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com>
> ---
> v4 Changes:
> * replace "vcm_output" property name for "vcm-output". 
> 
> v3 Changes:
> * VCM is now provided as a regulator within the device, instead of a 
>   custom property.
> 
> v2 Changes:
> * New patch in v2.
> ---
>  .../bindings/iio/adc/adi,ad7768-1.yaml        | 21 +++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml
> index e2f9782b5fc8..12358ea9138a 100644
> --- a/Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml
> @@ -59,6 +59,19 @@ properties:
>        in any way, for example if the filter decimation rate changes.
>        As the line is active low, it should be marked GPIO_ACTIVE_LOW.
>  
> +  regulators:
> +    type: object
> +    description:
> +      list of regulators provided by this controller.
> +
> +    properties:
> +      vcm-output:
> +        $ref: /schemas/regulator/regulator.yaml#
> +        type: object
> +        unevaluatedProperties: false
> +
> +    additionalProperties: false
> +
>    reset-gpios:
>      maxItems: 1
>  
> @@ -152,6 +165,14 @@ examples:
>                  reg = <0>;
>                  label = "channel_0";
>              };
> +
> +            regulators {
> +              vcm_reg: vcm-output {
> +                regulator-name = "ad7768-1-vcm";
> +                regulator-min-microvolt = <900000>;
> +                regulator-max-microvolt = <2500000>;

Why do we have the min and max properties? Aren't these always
going to be the same for all chips? It seems unnecessary to
have to write that in the devicetree.

> +              };
> +            };
>          };
>      };
>  ...

Re: [PATCH v4 08/17] iio: adc: ad7768-1: convert driver to use regmap

[David Lechner]

On 3/6/25 3:02 PM, Jonathan Santos wrote:
> Convert the AD7768-1 driver to use the regmap API for register
> access. This change simplifies and standardizes register interactions,
> reducing code duplication and improving maintainability.
> 
> Create two regmap configurations, one for 8-bit register values and
> other for 24-bit register values.
> 
> Since we are using regmap now, define the remaining registers from 0x32
> to 0x34.
> 
> Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com>
> ---

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

> +static const struct regmap_range ad7768_regmap_rd_ranges[] = {
> +	regmap_reg_range(AD7768_REG_CHIP_TYPE, AD7768_REG_DIG_DIAG_ENABLE),

Technically, there are a few holes in here where registers are not defined
so we could split this up in to a few ranges to only include registers that
actually contain a useful value.

> +	regmap_reg_range(AD7768_REG_MASTER_STATUS, AD7768_REG_COEFF_CONTROL),
> +	regmap_reg_range(AD7768_REG_ACCESS_KEY, AD7768_REG_ACCESS_KEY),
> +};
>  

Re: [PATCH v4 08/17] iio: adc: ad7768-1: convert driver to use regmap

[David Lechner]

On 4/1/25 11:31 AM, David Lechner wrote:
> On 3/6/25 3:02 PM, Jonathan Santos wrote:
>> Convert the AD7768-1 driver to use the regmap API for register
>> access. This change simplifies and standardizes register interactions,
>> reducing code duplication and improving maintainability.
>>
>> Create two regmap configurations, one for 8-bit register values and
>> other for 24-bit register values.
>>
>> Since we are using regmap now, define the remaining registers from 0x32
>> to 0x34.
>>
>> Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com>
>> ---
> 
> Reviewed-by: David Lechner <dlechner@baylibre.com>
> 
>> +static const struct regmap_range ad7768_regmap_rd_ranges[] = {
>> +	regmap_reg_range(AD7768_REG_CHIP_TYPE, AD7768_REG_DIG_DIAG_ENABLE),
> 
> Technically, there are a few holes in here where registers are not defined
> so we could split this up in to a few ranges to only include registers that
> actually contain a useful value.
> 
>> +	regmap_reg_range(AD7768_REG_MASTER_STATUS, AD7768_REG_COEFF_CONTROL),
>> +	regmap_reg_range(AD7768_REG_ACCESS_KEY, AD7768_REG_ACCESS_KEY),
>> +};
>>  

Also just realized we can drop the AD7768_RD_FLAG_MSK and AD7768_WR_FLAG_MSK
macro definitions in this patch.

Re: [PATCH v4 14/17] iio: adc: ad7768-1: add support for Synchronization over SPI

[David Lechner]

On 3/6/25 3:03 PM, Jonathan Santos wrote:
> The synchronization method using GPIO requires the generated pulse to be
> truly synchronous with the base MCLK signal. When it is not possible to
> do that in hardware, the datasheet recommends using synchronization over
> SPI, where the generated pulse is already synchronous with MCLK. This
> requires the SYNC_OUT pin to be connected to SYNC_IN pin.
> 
> Use trigger-sources property to enable device synchronization over SPI.
> 
> Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com>
> ---
...

> +static int ad7768_setup_spi_sync(struct device *dev, struct ad7768_state *st)
> +{
> +	struct fwnode_reference_args args;
> +	int ret;
> +
> +	ret = fwnode_property_get_reference_args(dev_fwnode(dev),
> +						 "trigger-sources",
> +						 "#trigger-source-cells",
> +						 0, 0, &args);

As in the DT binding patch, we may need to allow an arg here to be
able to tell the difference between /SYNC_OUT and /DRDY triggers.

Also, in reviews on previous versions of this series, Jonathan and
Conor both mentioned that it would be sensible to allow omitting
the trigger-sources property when /SYNC_OUT is connected to /SYNC_IN
if you want to consider that.

> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to get trigger-sources reference\n");
> +
> +	/*
> +	 * Currently, the driver supports SPI-based synchronization only for
> +	 * single-device setups, where the device's own SYNC_OUT is looped back
> +	 * to its SYNC_IN. Only enable this feature if the trigger-sources
> +	 * references the current device.
> +	 */
> +	st->en_spi_sync = args.fwnode->dev == dev;
> +	fwnode_handle_put(args.fwnode);
> +
> +	return st->en_spi_sync ? 0 : -EOPNOTSUPP;
> +}

Re: [PATCH v4 15/17] iio: adc: ad7768-1: replace manual attribute declaration

[David Lechner]

On 3/6/25 3:04 PM, Jonathan Santos wrote:
> Use read_avail callback from struct iio_info to replace the manual
> declaration of sampling_frequency_available attribute.
> 
> Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com>
> ---
Reviewed-by: David Lechner <dlechner@baylibre.com>

Re: [PATCH v4 17/17] iio: adc: ad7768-1: add low pass -3dB cutoff attribute

[David Lechner]

On 3/6/25 3:04 PM, Jonathan Santos wrote:
> Ad7768-1 has a different -3db frequency multiplier depending on
> the filter type configured. The cutoff frequency also varies according
> to the current ODR.
> 
> Add a readonly low pass -3dB frequency cutoff attribute to clarify to
> the user which bandwidth is being allowed depending on the filter
> configurations.
> 
> Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com>
> ---
Reviewed-by: David Lechner <dlechner@baylibre.com>

Re: [PATCH v4 12/17] iio: adc: ad7768-1: Add GPIO controller support

[Marcelo Schmitt]

Hey Jonathan, 

The AD7768-1 GPIO controller support looks good to me.
Besides reviewing it, I've also inspired on your patch to implement GPIO
controller support for AD4170. I have one inline suggestion, though.

With that sorted out
Reviewed-by: Marcelo Schmitt <marcelo.schmitt@analog.com>

Thanks,
Marcelo

On 03/06, Jonathan Santos wrote:
> From: Sergiu Cuciurean <sergiu.cuciurean@analog.com>
> 
> The AD7768-1 has the ability to control other local hardware (such as gain
> stages),to power down other blocks in the signal chain, or read local
> status signals over the SPI interface.
> 
> Add direct mode conditional locks in the gpio callbacks to prevent register
> access when the device is in buffered mode.
> 
> This change exports the AD7768-1's four gpios and makes them accessible
> at an upper layer.
> 
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Sergiu Cuciurean <sergiu.cuciurean@analog.com>
> Co-developed-by: Jonathan Santos <Jonathan.Santos@analog.com>
> Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com>
> ---
...
> ---
>  drivers/iio/adc/ad7768-1.c | 143 ++++++++++++++++++++++++++++++++++++-
>  1 file changed, 141 insertions(+), 2 deletions(-)
> 
...
> +static int ad7768_gpio_init(struct iio_dev *indio_dev)
> +{
> +	struct ad7768_state *st = iio_priv(indio_dev);
> +	int ret;
> +
> +	ret = regmap_write(st->regmap, AD7768_REG_GPIO_CONTROL,
> +			   AD7768_GPIO_UNIVERSAL_EN);
> +	if (ret)
> +		return ret;
> +
> +	st->gpiochip = (struct gpio_chip) {
> +		.label = "ad7768_1_gpios",
> +		.base = -1,
> +		.ngpio = 4,
> +		.parent = &st->spi->dev,
> +		.can_sleep = true,
> +		.direction_input = ad7768_gpio_direction_input,
> +		.direction_output = ad7768_gpio_direction_output,
> +		.get = ad7768_gpio_get,
> +		.set = ad7768_gpio_set,
> +		.owner = THIS_MODULE,
> +	};
> +
> +	return gpiochip_add_data(&st->gpiochip, indio_dev);
Use devm_gpiochip_add_data(), otherwise the gpiochip is not removed on device
detach.

> +}
> +

Re: [PATCH v4 16/17] iio: adc: ad7768-1: add filter type and oversampling ratio attributes

[Jonathan Cameron]

On Mon, 31 Mar 2025 21:18:16 -0300
Jonathan Santos <jonath4nns@gmail.com> wrote:

> On 03/08, Jonathan Cameron wrote:
> > On Thu, 6 Mar 2025 18:04:24 -0300
> > Jonathan Santos <Jonathan.Santos@analog.com> wrote:
> >   
> > > Separate filter type and decimation rate from the sampling frequency
> > > attribute. The new filter type attribute enables sinc3, sinc3+rej60
> > > and wideband filters, which were previously unavailable.
> > > 
> > > Previously, combining decimation and MCLK divider in the sampling
> > > frequency obscured performance trade-offs. Lower MCLK divider
> > > settings increase power usage, while lower decimation rates reduce
> > > precision by decreasing averaging. By creating an oversampling
> > > attribute, which controls the decimation, users gain finer control
> F> > over performance.
> > > 
> > > The addition of those attributes allows a wider range of sampling
> > > frequencies and more access to the device features. Sampling frequency
> > > table is updated after every digital filter paramerter change.
> > > 
> > > Co-developed-by: Pop Paul <paul.pop@analog.com>
> > > Signed-off-by: Pop Paul <paul.pop@analog.com>
> > > Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com>
> > > ---
> > > v4 Changes:
> > > * Sampling frequency table is dinamically updated after every  
> > 
> > Good to spell check. Dynamically
> >   
> > >   filter configuration.  
> > 
> > Currently this runs into the potential race conditions we get with
> > read_avail callbacks.  If we update the avail values in parallel
> > with consumer code in a kernel driver reading them we can get tearing.
> > So better if possible to do it all before those interfaces are exposed
> > and just pick from a set of static arrays.
> >   
> I understand the problem, but the number of possible sampling
> frequencies is quite large because of the decimation/OSR:
> 
> -> For wideband there are 6 decimations available.
> -> For Sinc5 there are 8 decimations.
> -> For sinc3 (here's the problem) we have up to 5119 decimation options.  
> From x32 to x163,840 (mclk_div = 2) with a 32 step.
> 
> BTW, that's why we use ranges for `oversampling_ratio_available`
> attribute.

Ah, understood now.  Another case where I guess we need to fix up in the long
term.  Short term not a problem as I don't think any consumers yet read the filter
parameters anyway.

> 
> To reflect all sampling frequencies combinations (fref/(mclk_div *
> OSR)), we would need a considerably large array. We did not have this
> problem before because we were not supporting the sinc3 filter.
> 
> > > +static struct iio_chan_spec_ext_info ad7768_ext_info[] = {
> > > +	IIO_ENUM("filter_type", IIO_SHARED_BY_ALL, &ad7768_flt_type_iio_enum),
> > > +	IIO_ENUM_AVAILABLE("filter_type", IIO_SHARED_BY_ALL, &ad7768_flt_type_iio_enum),
> > > +	{ },  
> > 
> > No trailing comma on a terminating entry as we don't want it to be easy
> > to accidentally add stuff after this.
> >   
> > > +};  
> > 
> >   
> > > +static int ad7768_configure_dig_fil(struct iio_dev *dev,
> > > +				    enum ad7768_filter_type filter_type,
> > > +				    unsigned int dec_rate)
> > > +{
> > > +	struct ad7768_state *st = iio_priv(dev);
> > > +	unsigned int dec_rate_idx, dig_filter_regval;
> > > +	int ret;
> > > +
> > > +	switch (filter_type) {
> > > +	case AD7768_FILTER_SINC3:
> > > +		dig_filter_regval = AD7768_DIG_FIL_FIL(AD7768_FILTER_REGVAL_SINC3);
> > > +		break;
> > > +	case AD7768_FILTER_SINC3_REJ60:
> > > +		dig_filter_regval = AD7768_DIG_FIL_FIL(AD7768_FILTER_REGVAL_SINC3_REJ60);
> > > +		break;
> > > +	case AD7768_FILTER_WIDEBAND:
> > > +		/* Skip decimations 8 and 16, not supported by the wideband filter */
> > > +		dec_rate_idx = find_closest(dec_rate, &ad7768_dec_rate_values[2],
> > > +					    ARRAY_SIZE(ad7768_dec_rate_values) - 2);
> > > +		dig_filter_regval = AD7768_DIG_FIL_FIL(AD7768_FILTER_REGVAL_WIDEBAND) |
> > > +				    AD7768_DIG_FIL_DEC_RATE(dec_rate_idx);
> > > +		/* Correct the index offset */
> > > +		dec_rate_idx += 2;
> > > +		break;
> > > +	case AD7768_FILTER_SINC5:
> > > +		dec_rate_idx = find_closest(dec_rate, ad7768_dec_rate_values,
> > > +					    ARRAY_SIZE(ad7768_dec_rate_values));
> > > +
> > > +		/*
> > > +		 * Decimations 8 (idx 0) and 16 (idx 1) are set in the
> > > +		 * FILTER[6:4] field. The other decimations are set in the
> > > +		 * DEC_RATE[2:0] field, and the idx need to be offsetted by two.
> > > +		 */
> > > +		if (dec_rate_idx == 0)
> > > +			dig_filter_regval = AD7768_DIG_FIL_FIL(AD7768_FILTER_REGVAL_SINC5_X8);
> > > +		else if (dec_rate_idx == 1)
> > > +			dig_filter_regval = AD7768_DIG_FIL_FIL(AD7768_FILTER_REGVAL_SINC5_X16);
> > > +		else
> > > +			dig_filter_regval = AD7768_DIG_FIL_FIL(AD7768_FILTER_REGVAL_SINC5) |
> > > +					    AD7768_DIG_FIL_DEC_RATE(dec_rate_idx - 2);
> > > +		break;
> > > +	}
> > > +
> > > +	ret = regmap_write(st->regmap, AD7768_REG_DIGITAL_FILTER, dig_filter_regval);
> > > +	if (ret)
> > >  		return ret;
> > >  
> > > -	/* A sync-in pulse is required every time the filter dec rate changes */
> > > +	st->filter_type = filter_type;
> > > +	/*
> > > +	 * The decimation for SINC3 filters are configured in different
> > > +	 * registers
> > > +	 */
> > > +	if (filter_type == AD7768_FILTER_SINC3 ||
> > > +	    filter_type == AD7768_FILTER_SINC3_REJ60) {
> > > +		ret = ad7768_set_sinc3_dec_rate(st, dec_rate);
> > > +		if (ret)
> > > +			return ret;
> > > +	} else {
> > > +		st->oversampling_ratio =  ad7768_dec_rate_values[dec_rate_idx];  
> > 
> > Looks like an extra space after =
> >   
> 
> Sorry about that
> 
> > > +	}
> > > +
> > > +	ad7768_fill_samp_freq_tbl(st);  
> > 
> > This is opens a potentially complex race condition if we have the an
> > in kernel consumer reading the data in this array as it is being updated
> > (currently we can't stop that happening though solutions to that problem
> > have been much discussed). 
> > 
> > There aren't that many oversampling ratios so perhaps it is better
> > to precalculate all the potential available values as an array indexed  
> 
> As I said above, unfortunately there are many OSR options.
> 
> > by oversampling ratio.  That way all the data is const, it's just possible
> > to get stale pointer to the wrong entry which can always happen anyway
> > if the read vs update happen in different entities.
> >   
> > > +
> > > +	/* A sync-in pulse is required after every configuration change */
> > >  	return ad7768_send_sync_pulse(st);
> > >  }  
> >   
> > >  
> > > +static int ad7768_write_raw(struct iio_dev *indio_dev,
> > > +			    struct iio_chan_spec const *chan,
> > > +			    int val, int val2, long info)
> > > +{
> > > +	int ret;
> > > +
> > > +	ret = iio_device_claim_direct_mode(indio_dev);  
> > 
> > update to use if (!iio_device_claim_direct())
> >   
> 
> OK!
> 
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	ret = __ad7768_write_raw(indio_dev, chan, val, val2, info);
> > > +	iio_device_release_direct_mode(indio_dev);
> > > +
> > > +	return ret;
> > > +}  
> >