[PATCH v3 0/3] Add support for AD7191

[PATCH v3 0/3] Add support for AD7191

[Alisa-Dariana Roman]

Thank you all for your feedback! Here is the updated series of patches!

I addressed all the replies' points, except for the one about the size of the
avail array being 1 when the pga/odr pins are pin-strapped. David raised a very
good point, but, for now, I left the size fixed to 4, since the functions for
setting the values return error anyway when they are pin-strapped.

I thought of 3 approaches:
	- dynamic allocation for the avail arrays
	- different avail array for the 2 different cases (pin-strap or gpios)
	- different channels array for the 2 different cases (probably too much)

If the current setup if not good enough, which approach would be the best?

Kind regards,
Alisa-Dariana Roman.

---

v2: https://lore.kernel.org/all/20250122132821.126600-1-alisa.roman@analog.com/

v2 -> v3:
	- correct binding title
	- remove clksel_state and clksel_gpio, assume the clksel pin is always
pinstrapped
	- rephrase clocks description accordingly
	- simplify binding constraints
	- specify in binding description that PDOWN must be connected to SPI's
controller's CS
	- add minItems for gpios in bindings
	- make scope explicit for mutex guard
	- remove spi irq check
	- add id_table to spi_driver struct
	- changed comments as suggested
	- use spi_message_init_with_transfers()
	- default returns an error in ad7191_set_mode()
	- replace hard-coded 2 with st->pga_gpios->ndescs
	- use gpiod_set_array_value_cansleep()
	- change .storagebits to 32
	- check return value for ad_sd_init()
	- change to adi,odr-value and adi,pga-value, which now accepts the value as
suggested
	- modify variables names and refactor the setup of odr and pga gpios,
indexes and available arrays into ad7191_config_setup(), since they are all
related
	- add ad7191.rst

v1: https://lore.kernel.org/all/20241221155926.81954-1-alisa.roman@analog.com/

v1 -> v2:
	- removed patch adding function in ad_sigma_delta.h/.c
	- added a function set_cs() for asserting/deasserting the cs
	- handle pinstrapping cases
	- refactored all clock handling
	- updated bindings: corrected and added new things
	- -> address of the channels is used in set_channel()
	- addressed all the other changes

[PATCH v3 1/3] dt-bindings: iio: adc: add AD7191

[Alisa-Dariana Roman]

AD7191 is a pin-programmable, ultra-low noise 24-bit sigma-delta ADC
designed for precision bridge sensor measurements. It features two
differential analog input channels, selectable output rates,
programmable gain, internal temperature sensor and simultaneous
50Hz/60Hz rejection.

Signed-off-by: Alisa-Dariana Roman <alisa.roman@analog.com>
---
 .../bindings/iio/adc/adi,ad7191.yaml          | 149 ++++++++++++++++++
 MAINTAINERS                                   |   7 +
 2 files changed, 156 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad7191.yaml

diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7191.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7191.yaml
new file mode 100644
index 000000000000..ac14096ba76c
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7191.yaml
@@ -0,0 +1,149 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright 2025 Analog Devices Inc.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/adc/adi,ad7191.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices AD7191 ADC
+
+maintainers:
+  - Alisa-Dariana Roman <alisa.roman@analog.com>
+
+description: |
+  Bindings for the Analog Devices AD7191 ADC device. Datasheet can be
+  found here:
+  https://www.analog.com/media/en/technical-documentation/data-sheets/AD7191.pdf
+  The device's PDOWN pin must be connected to the SPI controller's chip select
+  pin.
+
+properties:
+  compatible:
+    enum:
+      - adi,ad7191
+
+  reg:
+    maxItems: 1
+
+  spi-cpol: true
+
+  spi-cpha: true
+
+  clocks:
+    maxItems: 1
+    description: |
+      Must be present when CLKSEL pin is tied HIGH to select external clock
+      source (either a crystal between MCLK1 and MCLK2 pins, or a
+      CMOS-compatible clock driving MCLK2 pin). Must be absent when CLKSEL pin
+      is tied LOW to use the internal 4.92MHz clock.
+
+  interrupts:
+    maxItems: 1
+
+  avdd-supply:
+    description: AVdd voltage supply
+
+  dvdd-supply:
+    description: DVdd voltage supply
+
+  vref-supply:
+    description: Vref voltage supply
+
+  odr-gpios:
+    description: |
+      ODR1 and ODR2 pins for output data rate selection. Should be defined if
+      adi,odr-value is absent.
+    minItems: 2
+    maxItems: 2
+
+  adi,odr-value:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: |
+      Should be present if ODR pins are pin-strapped. Possible values:
+      120 Hz (ODR1=0, ODR2=0)
+      60 Hz (ODR1=0, ODR2=1)
+      50 Hz (ODR1=1, ODR2=0)
+      10 Hz (ODR1=1, ODR2=1)
+      If defined, odr-gpios must be absent.
+    enum: [120, 60, 50, 10]
+
+  pga-gpios:
+    description: |
+      PGA1 and PGA2 pins for gain selection. Should be defined if adi,pga-value
+      is absent.
+    minItems: 2
+    maxItems: 2
+
+  adi,pga-value:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: |
+      Should be present if PGA pins are pin-strapped. Possible values:
+      Gain 1 (PGA1=0, PGA2=0)
+      Gain 8 (PGA1=0, PGA2=1)
+      Gain 64 (PGA1=1, PGA2=0)
+      Gain 128 (PGA1=1, PGA2=1)
+      If defined, pga-gpios must be absent.
+    enum: [1, 8, 64, 128]
+
+  temp-gpios:
+    description: TEMP pin for temperature sensor enable.
+    maxItems: 1
+
+  chan-gpios:
+    description: CHAN pin for input channel selection.
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - avdd-supply
+  - dvdd-supply
+  - vref-supply
+  - spi-cpol
+  - spi-cpha
+  - temp-gpios
+  - chan-gpios
+
+allOf:
+  - $ref: /schemas/spi/spi-peripheral-props.yaml#
+  - oneOf:
+      - required:
+          - adi,odr-value
+      - required:
+          - odr-gpios
+  - oneOf:
+      - required:
+          - adi,pga-value
+      - required:
+          - pga-gpios
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    spi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        adc@0 {
+            compatible = "adi,ad7191";
+            reg = <0>;
+            spi-max-frequency = <1000000>;
+            spi-cpol;
+            spi-cpha;
+            clocks = <&ad7191_mclk>;
+            interrupts = <25 IRQ_TYPE_EDGE_FALLING>;
+            interrupt-parent = <&gpio>;
+            avdd-supply = <&avdd>;
+            dvdd-supply = <&dvdd>;
+            vref-supply = <&vref>;
+            adi,pga-value = <1>;
+            odr-gpios = <&gpio 23 GPIO_ACTIVE_HIGH>, <&gpio 24 GPIO_ACTIVE_HIGH>;
+            temp-gpios = <&gpio 22 GPIO_ACTIVE_HIGH>;
+            chan-gpios = <&gpio 27 GPIO_ACTIVE_HIGH>;
+        };
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index 98a3c1e46311..262beced3143 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1302,6 +1302,13 @@ W:	http://ez.analog.com/community/linux-device-drivers
 F:	Documentation/devicetree/bindings/iio/adc/adi,ad7091r*
 F:	drivers/iio/adc/ad7091r*
 
+ANALOG DEVICES INC AD7191 DRIVER
+M:	Alisa-Dariana Roman <alisa.roman@analog.com>
+L:	linux-iio@vger.kernel.org
+S:	Supported
+W:	https://ez.analog.com/linux-software-drivers
+F:	Documentation/devicetree/bindings/iio/adc/adi,ad7191.yaml
+
 ANALOG DEVICES INC AD7192 DRIVER
 M:	Alisa-Dariana Roman <alisa.roman@analog.com>
 L:	linux-iio@vger.kernel.org
-- 
2.43.0

[PATCH v3 2/3] iio: adc: ad7191: add AD7191

[Alisa-Dariana Roman]

AD7191 is a pin-programmable, ultra-low noise 24-bit sigma-delta ADC
designed for precision bridge sensor measurements. It features two
differential analog input channels, selectable output rates,
programmable gain, internal temperature sensor and simultaneous
50Hz/60Hz rejection.

Signed-off-by: Alisa-Dariana Roman <alisa.roman@analog.com>
---
 MAINTAINERS              |   1 +
 drivers/iio/adc/Kconfig  |  10 +
 drivers/iio/adc/Makefile |   1 +
 drivers/iio/adc/ad7191.c | 539 +++++++++++++++++++++++++++++++++++++++
 4 files changed, 551 insertions(+)
 create mode 100644 drivers/iio/adc/ad7191.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 262beced3143..1340c27d9e5a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1308,6 +1308,7 @@ L:	linux-iio@vger.kernel.org
 S:	Supported
 W:	https://ez.analog.com/linux-software-drivers
 F:	Documentation/devicetree/bindings/iio/adc/adi,ad7191.yaml
+F:	drivers/iio/adc/ad7191.c
 
 ANALOG DEVICES INC AD7192 DRIVER
 M:	Alisa-Dariana Roman <alisa.roman@analog.com>
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 849c90203071..70a662846aa2 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -112,6 +112,16 @@ config AD7173
 	  To compile this driver as a module, choose M here: the module will be
 	  called ad7173.
 
+config AD7191
+	tristate "Analog Devices AD7191 ADC driver"
+	depends on SPI
+	select AD_SIGMA_DELTA
+	help
+	  Say yes here to build support for Analog Devices AD7191.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called ad7191.
+
 config AD7192
 	tristate "Analog Devices AD7192 and similar ADC driver"
 	depends on SPI
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index ee19afba62b7..54335c613988 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -14,6 +14,7 @@ obj-$(CONFIG_AD7091R5) += ad7091r5.o
 obj-$(CONFIG_AD7091R8) += ad7091r8.o
 obj-$(CONFIG_AD7124) += ad7124.o
 obj-$(CONFIG_AD7173) += ad7173.o
+obj-$(CONFIG_AD7191) += ad7191.o
 obj-$(CONFIG_AD7192) += ad7192.o
 obj-$(CONFIG_AD7266) += ad7266.o
 obj-$(CONFIG_AD7280) += ad7280a.o
diff --git a/drivers/iio/adc/ad7191.c b/drivers/iio/adc/ad7191.c
new file mode 100644
index 000000000000..b6c1d5c25783
--- /dev/null
+++ b/drivers/iio/adc/ad7191.c
@@ -0,0 +1,539 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * AD7191 ADC driver
+ *
+ * Copyright 2025 Analog Devices Inc.
+ */
+
+#include <linux/bitfield.h>
+#include <linux/clk.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/gpio/consumer.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/mod_devicetable.h>
+#include <linux/mutex.h>
+#include <linux/property.h>
+#include <linux/regulator/consumer.h>
+#include <linux/spi/spi.h>
+#include <linux/types.h>
+#include <linux/units.h>
+
+#include <linux/iio/adc/ad_sigma_delta.h>
+#include <linux/iio/iio.h>
+
+#define ad_sigma_delta_to_ad7191(sigmad)	container_of((sigmad), struct ad7191_state, sd)
+
+#define AD7191_TEMP_CODES_PER_DEGREE	2815
+
+#define AD7191_EXT_CLK_ENABLE		0
+#define AD7191_INT_CLK_ENABLE		1
+
+#define AD7191_CHAN_MASK		BIT(0)
+#define AD7191_TEMP_MASK		BIT(1)
+
+enum ad7191_channel {
+	AD7191_CH_AIN1_AIN2,
+	AD7191_CH_AIN3_AIN4,
+	AD7191_CH_TEMP
+};
+
+/*
+ * NOTE:
+ * The AD7191 features a dual-use data out ready DOUT/RDY output.
+ * In order to avoid contentions on the SPI bus, it's therefore necessary
+ * to use SPI bus locking.
+ *
+ * The DOUT/RDY output must also be wired to an interrupt-capable GPIO.
+ *
+ * The SPI controller's chip select must be connected to the PDOWN pin
+ * of the ADC. When CS (PDOWN) is high, it powers down the device and
+ * resets the internal circuitry.
+ */
+
+struct ad7191_state {
+	struct ad_sigma_delta		sd;
+	struct mutex			lock; // to protect sensor state
+
+	struct gpio_descs		*odr_gpios;
+	struct gpio_descs		*pga_gpios;
+	struct gpio_desc		*temp_gpio;
+	struct gpio_desc		*chan_gpio;
+
+	u16				int_vref_mv;
+	u32				pga_index;
+	u32				scale_avail[4][2];
+	u32				odr_index;
+	u32				samp_freq_avail[4];
+
+	struct clk			*mclk;
+};
+
+static int ad7191_set_channel(struct ad_sigma_delta *sd, unsigned int address)
+{
+	struct ad7191_state *st = ad_sigma_delta_to_ad7191(sd);
+	u8 temp_gpio_val, chan_gpio_val;
+
+	if (!FIELD_FIT(AD7191_CHAN_MASK | AD7191_TEMP_MASK, address))
+		return -EINVAL;
+
+	chan_gpio_val = FIELD_GET(AD7191_CHAN_MASK, address);
+	temp_gpio_val = FIELD_GET(AD7191_TEMP_MASK, address);
+
+	gpiod_set_value(st->chan_gpio, chan_gpio_val);
+	gpiod_set_value(st->temp_gpio, temp_gpio_val);
+
+	return 0;
+}
+
+static int ad7191_set_cs(struct ad_sigma_delta *sigma_delta, int assert)
+{
+	struct spi_transfer t = {
+		.len = 0,
+		.cs_change = assert,
+	};
+	struct spi_message m;
+
+	spi_message_init_with_transfers(&m, &t, 1);
+
+	return spi_sync_locked(sigma_delta->spi, &m);
+}
+
+static int ad7191_set_mode(struct ad_sigma_delta *sd,
+			   enum ad_sigma_delta_mode mode)
+{
+	struct ad7191_state *st = ad_sigma_delta_to_ad7191(sd);
+
+	switch (mode) {
+	case AD_SD_MODE_CONTINUOUS:
+	case AD_SD_MODE_SINGLE:
+		return ad7191_set_cs(&st->sd, 1);
+	case AD_SD_MODE_IDLE:
+		return ad7191_set_cs(&st->sd, 0);
+	default:
+		return -EINVAL;
+	}
+}
+
+static const struct ad_sigma_delta_info ad7191_sigma_delta_info = {
+	.set_channel = ad7191_set_channel,
+	.set_mode = ad7191_set_mode,
+	.has_registers = false,
+};
+
+static int ad7191_init_regulators(struct iio_dev *indio_dev)
+{
+	struct ad7191_state *st = iio_priv(indio_dev);
+	struct device *dev = &st->sd.spi->dev;
+	int ret;
+
+	ret = devm_regulator_get_enable(dev, "avdd");
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to enable specified AVdd supply\n");
+
+	ret = devm_regulator_get_enable(dev, "dvdd");
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to enable specified DVdd supply\n");
+
+	ret = devm_regulator_get_enable_read_voltage(dev, "vref");
+	if (ret < 0)
+		return dev_err_probe(dev, ret, "Failed to get Vref voltage\n");
+
+	st->int_vref_mv = ret / 1000;
+
+	return 0;
+}
+
+static int ad7191_config_setup(struct iio_dev *indio_dev)
+{
+	struct ad7191_state *st = iio_priv(indio_dev);
+	struct device *dev = &st->sd.spi->dev;
+	/* Sampling frequencies in Hz, see Table 5 */
+	const int samp_freq[4] = {120, 60, 50, 10};
+	/* Gain options, see Table 7 */
+	const int gain[4] = {1, 8, 64, 128};
+	int odr_value, pga_value, i, ret;
+	u64 scale_uv;
+
+	st->odr_index = 0;
+	st->pga_index = 0;
+
+	ret = device_property_read_u32(dev, "adi,odr-value", &odr_value);
+	if (ret == EINVAL) {
+		st->odr_gpios = devm_gpiod_get_array(dev, "odr", GPIOD_OUT_LOW);
+		if (IS_ERR(st->odr_gpios))
+			return dev_err_probe(dev, PTR_ERR(st->odr_gpios),
+					     "Failed to get odr gpios.\n");
+	} else {
+		for (i = 0; i < ARRAY_SIZE(samp_freq); i++) {
+			if (odr_value != samp_freq[i])
+				continue;
+			st->odr_index = i;
+		}
+		st->odr_gpios = NULL;
+	}
+
+	ret = device_property_read_u32(dev, "adi,pga-value", &pga_value);
+	if (ret == EINVAL) {
+		st->pga_gpios = devm_gpiod_get_array(dev, "pga", GPIOD_OUT_LOW);
+		if (IS_ERR(st->pga_gpios))
+			return dev_err_probe(dev, PTR_ERR(st->pga_gpios),
+					     "Failed to get pga gpios.\n");
+	} else {
+		for (i = 0; i < ARRAY_SIZE(gain); i++) {
+			if (pga_value != gain[i])
+				continue;
+			st->pga_index = i;
+		}
+		st->pga_gpios = NULL;
+	}
+
+	for (i = 0; i < ARRAY_SIZE(samp_freq); i++)
+		st->samp_freq_avail[i] = samp_freq[i];
+
+	for (i = 0; i < ARRAY_SIZE(st->scale_avail); i++) {
+		scale_uv = ((u64)st->int_vref_mv * NANO) >>
+			   (indio_dev->channels[0].scan_type.realbits - 1);
+		do_div(scale_uv, gain[i]);
+		st->scale_avail[i][1] = do_div(scale_uv, NANO);
+		st->scale_avail[i][0] = scale_uv;
+	}
+
+	st->temp_gpio = devm_gpiod_get(dev, "temp", GPIOD_OUT_LOW);
+	if (IS_ERR(st->temp_gpio))
+		return dev_err_probe(dev, PTR_ERR(st->temp_gpio),
+				     "Failed to get temp gpio.\n");
+
+	st->chan_gpio = devm_gpiod_get(dev, "chan", GPIOD_OUT_LOW);
+	if (IS_ERR(st->chan_gpio))
+		return dev_err_probe(dev, PTR_ERR(st->chan_gpio),
+				     "Failed to get chan gpio.\n");
+
+	return 0;
+}
+
+static int ad7191_clock_setup(struct ad7191_state *st)
+{
+	struct device *dev = &st->sd.spi->dev;
+
+	st->mclk = devm_clk_get_optional_enabled(dev, "mclk");
+	if (IS_ERR(st->mclk))
+		return dev_err_probe(dev, PTR_ERR(st->mclk),
+				     "Failed to get mclk.\n");
+
+	return 0;
+}
+
+static int ad7191_setup(struct iio_dev *indio_dev, struct device *dev)
+{
+	struct ad7191_state *st = iio_priv(indio_dev);
+	int ret;
+
+	ret = ad7191_init_regulators(indio_dev);
+	if (ret)
+		return ret;
+
+	ret = ad7191_config_setup(indio_dev);
+	if (ret)
+		return ret;
+
+	ret = ad7191_clock_setup(st);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int ad7191_read_raw(struct iio_dev *indio_dev,
+			   struct iio_chan_spec const *chan, int *val,
+			   int *val2, long m)
+{
+	struct ad7191_state *st = iio_priv(indio_dev);
+
+	switch (m) {
+	case IIO_CHAN_INFO_RAW:
+		return ad_sigma_delta_single_conversion(indio_dev, chan, val);
+	case IIO_CHAN_INFO_SCALE:
+		switch (chan->type) {
+		case IIO_VOLTAGE: {
+			guard(mutex)(&st->lock);
+			*val = st->scale_avail[st->pga_index][0];
+			*val2 = st->scale_avail[st->pga_index][1];
+			return IIO_VAL_INT_PLUS_NANO;
+		}
+		case IIO_TEMP:
+			*val = 0;
+			*val2 = NANO / AD7191_TEMP_CODES_PER_DEGREE;
+			return IIO_VAL_INT_PLUS_NANO;
+		default:
+			return -EINVAL;
+		}
+	case IIO_CHAN_INFO_OFFSET:
+		*val = -(1 << (chan->scan_type.realbits - 1));
+		switch (chan->type) {
+		case IIO_VOLTAGE:
+			return IIO_VAL_INT;
+		case IIO_TEMP:
+			*val -= 273 * AD7191_TEMP_CODES_PER_DEGREE;
+			return IIO_VAL_INT;
+		default:
+			return -EINVAL;
+		}
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		*val = st->samp_freq_avail[st->odr_index];
+		return IIO_VAL_INT;
+	}
+
+	return -EINVAL;
+}
+
+static int ad7191_set_gain(struct ad7191_state *st, int gain_index)
+{
+	unsigned long value = gain_index;
+
+	if (!st->pga_gpios)
+		return -EPERM;
+
+	st->pga_index = gain_index;
+
+	return gpiod_set_array_value_cansleep(st->pga_gpios->ndescs,
+					      st->pga_gpios->desc,
+					      st->pga_gpios->info, &value);
+
+	return 0;
+}
+
+static int ad7191_set_samp_freq(struct ad7191_state *st, int samp_freq_index)
+{
+	unsigned long value = samp_freq_index;
+
+	if (!st->odr_gpios)
+		return -EPERM;
+
+	st->odr_index = samp_freq_index;
+
+	return gpiod_set_array_value_cansleep(st->odr_gpios->ndescs,
+					      st->odr_gpios->desc,
+					      st->odr_gpios->info, &value);
+}
+
+static int __ad7191_write_raw(struct ad7191_state *st,
+			      struct iio_chan_spec const *chan,
+			      int val, int val2, long mask)
+{
+	int i;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_SCALE: {
+		guard(mutex)(&st->lock);
+		for (i = 0; i < ARRAY_SIZE(st->scale_avail); i++) {
+			if (val2 != st->scale_avail[i][1])
+				continue;
+			return ad7191_set_gain(st, i);
+		}
+		return -EINVAL;
+	}
+	case IIO_CHAN_INFO_SAMP_FREQ: {
+		guard(mutex)(&st->lock);
+		for (i = 0; i < ARRAY_SIZE(st->samp_freq_avail); i++) {
+			if (val != st->samp_freq_avail[i])
+				continue;
+			return ad7191_set_samp_freq(st, i);
+		}
+		return -EINVAL;
+	}
+	default:
+		return -EINVAL;
+	}
+}
+
+static int ad7191_write_raw(struct iio_dev *indio_dev,
+			    struct iio_chan_spec const *chan, int val, int val2,
+			    long mask)
+{
+	struct ad7191_state *st = iio_priv(indio_dev);
+	int ret;
+
+	ret = iio_device_claim_direct_mode(indio_dev);
+	if (ret)
+		return ret;
+
+	ret = __ad7191_write_raw(st, chan, val, val2, mask);
+
+	iio_device_release_direct_mode(indio_dev);
+
+	return ret;
+}
+
+static int ad7191_write_raw_get_fmt(struct iio_dev *indio_dev,
+				    struct iio_chan_spec const *chan, long mask)
+{
+	switch (mask) {
+	case IIO_CHAN_INFO_SCALE:
+		return IIO_VAL_INT_PLUS_NANO;
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		return IIO_VAL_INT;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int ad7191_read_avail(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan, const int **vals,
+			     int *type, int *length, long mask)
+{
+	struct ad7191_state *st = iio_priv(indio_dev);
+
+	switch (mask) {
+	case IIO_CHAN_INFO_SCALE:
+		*vals = (int *)st->scale_avail;
+		*type = IIO_VAL_INT_PLUS_NANO;
+		*length = ARRAY_SIZE(st->scale_avail) * 2;
+		return IIO_AVAIL_LIST;
+
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		*vals = (int *)st->samp_freq_avail;
+		*type = IIO_VAL_INT;
+		*length = ARRAY_SIZE(st->samp_freq_avail);
+		return IIO_AVAIL_LIST;
+	}
+
+	return -EINVAL;
+}
+
+static const struct iio_info ad7191_info = {
+	.read_raw = ad7191_read_raw,
+	.write_raw = ad7191_write_raw,
+	.write_raw_get_fmt = ad7191_write_raw_get_fmt,
+	.read_avail = ad7191_read_avail,
+	.validate_trigger = ad_sd_validate_trigger,
+};
+
+static const struct iio_chan_spec ad7191_channels[] = {
+	{
+		.type = IIO_TEMP,
+		.address = AD7191_CH_TEMP,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+				      BIT(IIO_CHAN_INFO_OFFSET),
+		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),
+		.info_mask_shared_by_all_available = BIT(IIO_CHAN_INFO_SAMP_FREQ),
+		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
+		.scan_type = {
+			.sign = 'u',
+			.realbits = 24,
+			.storagebits = 32,
+			.endianness = IIO_BE,
+		},
+	},
+	{
+		.type = IIO_VOLTAGE,
+		.differential = 1,
+		.indexed = 1,
+		.channel = 1,
+		.channel2 = 2,
+		.address = AD7191_CH_AIN1_AIN2,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+				      BIT(IIO_CHAN_INFO_OFFSET),
+		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),
+		.info_mask_shared_by_all_available = BIT(IIO_CHAN_INFO_SAMP_FREQ),
+		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
+		.info_mask_shared_by_type_available = BIT(IIO_CHAN_INFO_SCALE),
+		.scan_index = 1,
+		.scan_type = {
+			.sign = 'u',
+			.realbits = 24,
+			.storagebits = 32,
+			.endianness = IIO_BE,
+		},
+	},
+	{
+		.type = IIO_VOLTAGE,
+		.differential = 1,
+		.indexed = 1,
+		.channel = 3,
+		.channel2 = 4,
+		.address = AD7191_CH_AIN3_AIN4,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+				      BIT(IIO_CHAN_INFO_OFFSET),
+		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),
+		.info_mask_shared_by_all_available = BIT(IIO_CHAN_INFO_SAMP_FREQ),
+		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
+		.info_mask_shared_by_type_available = BIT(IIO_CHAN_INFO_SCALE),
+		.scan_index = 2,
+		.scan_type = {
+			.sign = 'u',
+			.realbits = 24,
+			.storagebits = 32,
+			.endianness = IIO_BE,
+		},
+	},
+	IIO_CHAN_SOFT_TIMESTAMP(3),
+};
+
+static int ad7191_probe(struct spi_device *spi)
+{
+	struct device *dev = &spi->dev;
+	struct ad7191_state *st;
+	struct iio_dev *indio_dev;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	st = iio_priv(indio_dev);
+
+	ret = devm_mutex_init(dev, &st->lock);
+	if (ret)
+		return ret;
+
+	indio_dev->name = "ad7191";
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->channels = ad7191_channels;
+	indio_dev->num_channels = ARRAY_SIZE(ad7191_channels);
+	indio_dev->info = &ad7191_info;
+
+	ret = ad_sd_init(&st->sd, indio_dev, spi, &ad7191_sigma_delta_info);
+	if (ret)
+		return ret;
+
+	ret = devm_ad_sd_setup_buffer_and_trigger(dev, indio_dev);
+	if (ret)
+		return ret;
+
+	ret = ad7191_setup(indio_dev, dev);
+	if (ret)
+		return ret;
+
+	return devm_iio_device_register(dev, indio_dev);
+}
+
+static const struct of_device_id ad7191_of_match[] = {
+	{
+		.compatible = "adi,ad7191",
+	},
+	{ }
+};
+MODULE_DEVICE_TABLE(of, ad7191_of_match);
+
+static const struct spi_device_id ad7191_id_table[] = {
+	{ "ad7191" },
+	{ }
+};
+MODULE_DEVICE_TABLE(spi, ad7191_id_table);
+
+static struct spi_driver ad7191_driver = {
+	.driver = {
+		.name = "ad7191",
+		.of_match_table = ad7191_of_match,
+	},
+	.probe = ad7191_probe,
+	.id_table = ad7191_id_table,
+};
+module_spi_driver(ad7191_driver);
+
+MODULE_AUTHOR("Alisa-Dariana Roman <alisa.roman@analog.com>");
+MODULE_DESCRIPTION("Analog Devices AD7191 ADC");
+MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS(IIO_AD_SIGMA_DELTA);
-- 
2.43.0

[PATCH v3 3/3] docs: iio: add AD7191

[Alisa-Dariana Roman]

Add documentation for AD7191 driver.

Signed-off-by: Alisa-Dariana Roman <alisa.roman@analog.com>
---
 Documentation/iio/ad7191.rst | 230 +++++++++++++++++++++++++++++++++++
 1 file changed, 230 insertions(+)
 create mode 100644 Documentation/iio/ad7191.rst

diff --git a/Documentation/iio/ad7191.rst b/Documentation/iio/ad7191.rst
new file mode 100644
index 000000000000..78aa5fefe128
--- /dev/null
+++ b/Documentation/iio/ad7191.rst
@@ -0,0 +1,230 @@
+.. SPDX-License-Identifier: GPL-2.0-only
+
+==============
+AD7191 driver
+==============
+
+Device driver for Analog Devices AD7191 ADC.
+
+Supported devices
+================
+
+* `AD7191 <https://www.analog.com/AD7191>`_
+
+The AD7191 is a high precision, low noise, 24-bit Σ-Δ ADC with integrated PGA.
+It features two differential input channels, an internal temperature sensor,and
+configurable sampling rates.
+
+Device Configuration
+===================
+
+Pin Configuration
+----------------
+
+The driver supports both pin-strapped and GPIO-controlled configurations for ODR
+(Output Data Rate) and PGA (Programmable Gain Amplifier) settings. These
+configurations are mutually exclusive - you must use either pin-strapped or GPIO
+control for each setting, not both.
+
+ODR Configuration
+^^^^^^^^^^^^^^^^
+
+The ODR can be configured either through GPIO control or pin-strapping:
+
+- When using GPIO control, specify the "odr-gpios" property in the device tree
+- For pin-strapped configuration, specify the "adi,odr-value" property in the
+device tree
+
+Available ODR settings:
+  - 120 Hz (ODR1=0, ODR2=0)
+  - 60 Hz (ODR1=0, ODR2=1)
+  - 50 Hz (ODR1=1, ODR2=0)
+  - 10 Hz (ODR1=1, ODR2=1)
+
+PGA Configuration
+^^^^^^^^^^^^^^^
+
+The PGA can be configured either through GPIO control or pin-strapping:
+
+- When using GPIO control, specify the "pga-gpios" property in the device tree
+- For pin-strapped configuration, specify the "adi,pga-value" property in the
+device tree
+
+Available PGA gain settings:
+  - 1x (PGA1=0, PGA2=0)
+  - 8x (PGA1=0, PGA2=1)
+  - 64x (PGA1=1, PGA2=0)
+  - 128x (PGA1=1, PGA2=1)
+
+Clock Configuration
+-----------------
+
+The AD7191 supports both internal and external clock sources:
+
+- When CLKSEL pin is tied LOW: Uses internal 4.92MHz clock (no clock property
+needed)
+- When CLKSEL pin is tied HIGH: Requires external clock source
+  - Can be a crystal between MCLK1 and MCLK2 pins
+  - Or a CMOS-compatible clock driving MCLK2 pin
+  - Must specify the "clocks" property in device tree when using external clock
+
+SPI Interface Requirements
+------------------------
+
+The AD7191 has specific SPI interface requirements:
+
+- The DOUT/RDY output is dual-purpose and requires SPI bus locking
+- DOUT/RDY must be connected to an interrupt-capable GPIO
+- The SPI controller's chip select must be connected to the PDOWN pin of the ADC
+- When CS (PDOWN) is high, the device powers down and resets internal circuitry
+- SPI mode 3 operation (CPOL=1, CPHA=1) is required
+
+Power Supply Requirements
+-----------------------
+
+The device requires the following power supplies:
+
+- AVdd: Analog power supply
+- DVdd: Digital power supply
+- Vref: Reference voltage supply (external)
+
+All power supplies must be specified in the device tree.
+
+Device Attributes
+===============
+
+The AD7191 provides several attributes through the IIO sysfs interface:
+
+Voltage Input Differential Channels
+---------------------------------
+
++-------------------+----------------------------------------------------------+
+| Attribute         | Description                                              |
++===================+==========================================================+
+| raw               | Raw ADC output value                                     |
++-------------------+----------------------------------------------------------+
+| scale             | Scale factor to convert raw value to voltage             |
++-------------------+----------------------------------------------------------+
+| offset            | Voltage offset                                           |
++-------------------+----------------------------------------------------------+
+| sampling_frequency| Current sampling frequency setting                       |
++-------------------+----------------------------------------------------------+
+
+Temperature Sensor
+----------------
+
++-------------------+----------------------------------------------------------+
+| Attribute         | Description                                              |
++===================+==========================================================+
+| raw               | Raw temperature sensor output value                      |
++-------------------+----------------------------------------------------------+
+| scale             | Scale factor to convert raw value to temperature         |
++-------------------+----------------------------------------------------------+
+| offset            | Temperature calibration offset                           |
++-------------------+----------------------------------------------------------+
+
+Available Attributes
+------------------
+
+The following attributes show available configuration options:
+
+- sampling_frequency_available: List of supported sampling frequencies
+- scale_available: List of supported scale factors (based on PGA settings)
+
+Channel Configuration
+===================
+
+The device provides three channels:
+
+1. Temperature Sensor
+   - 24-bit unsigned
+   - Internal temperature measurement
+   - Temperature in millidegrees Celsius
+
+2. Differential Input (AIN1-AIN2)
+   - 24-bit unsigned
+   - Differential voltage measurement
+   - Configurable gain via PGA
+
+3. Differential Input (AIN3-AIN4)
+   - 24-bit unsigned
+   - Differential voltage measurement
+   - Configurable gain via PGA
+
+Device Tree Bindings
+===================
+
+Required Properties
+-----------------
+
+- compatible: Should be "adi,ad7191"
+- reg: SPI chip select number
+- spi-max-frequency: Maximum SPI clock frequency
+- spi-cpol: Must be present (set to 1)
+- spi-cpha: Must be present (set to 1)
+- interrupts: Interrupt mapping for DOUT/RDY pin
+- avdd-supply: Analog power supply
+- dvdd-supply: Digital power supply
+- vref-supply: Reference voltage supply
+- temp-gpios: GPIO for temperature channel selection
+- chan-gpios: GPIO for input channel selection
+
+Optional Properties
+-----------------
+
+- clocks: Required when using external clock (CLKSEL=1), must be absent for
+internal clock
+- adi,odr-value: Pin-strapped ODR configuration (120, 60, 50, or 10)
+- adi,pga-value: Pin-strapped PGA configuration (1, 8, 64, or 128)
+- odr-gpios: GPIOs for ODR control (mutually exclusive with adi,odr-value)
+- pga-gpios: GPIOs for PGA control (mutually exclusive with adi,pga-value)
+
+Example Device Tree
+-----------------
+
+.. code-block:: dts
+
+    #include <dt-bindings/gpio/gpio.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    spi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        ad7191@0 {
+            compatible = "adi,ad7191";
+            reg = <0>;
+            spi-max-frequency = <1000000>;
+
+            /* Required SPI mode 3 */
+            spi-cpol;
+            spi-cpha;
+
+            /* Interrupt for DOUT/RDY pin */
+            interrupts = <25 IRQ_TYPE_EDGE_FALLING>;
+            interrupt-parent = <&gpio>;
+
+            /* Power supplies */
+            avdd-supply = <&avdd>;
+            dvdd-supply = <&dvdd>;
+            vref-supply = <&vref>;
+
+            /* Optional external clock */
+            clocks = <&ad7191_mclk>;
+
+            /* Configuration - either use GPIO control or pin-strapped values */
+            adi,pga-value = <1>;
+            odr-gpios = <&gpio 23 GPIO_ACTIVE_HIGH>,
+                       <&gpio 24 GPIO_ACTIVE_HIGH>;
+
+            /* Required GPIO controls */
+            temp-gpios = <&gpio 22 GPIO_ACTIVE_HIGH>;
+            chan-gpios = <&gpio 27 GPIO_ACTIVE_HIGH>;
+        };
+    };
+
+Buffer Support
+============
+
+This driver supports IIO triggered buffers. See Documentation/iio/iio_devbuf.rst
+for more information about IIO triggered buffers.
-- 
2.43.0

Re: [PATCH v3 1/3] dt-bindings: iio: adc: add AD7191

[Rob Herring]

On Wed, Jan 29, 2025 at 04:29:02PM +0200, Alisa-Dariana Roman wrote:
> AD7191 is a pin-programmable, ultra-low noise 24-bit sigma-delta ADC
> designed for precision bridge sensor measurements. It features two
> differential analog input channels, selectable output rates,
> programmable gain, internal temperature sensor and simultaneous
> 50Hz/60Hz rejection.
> 
> Signed-off-by: Alisa-Dariana Roman <alisa.roman@analog.com>
> ---
>  .../bindings/iio/adc/adi,ad7191.yaml          | 149 ++++++++++++++++++
>  MAINTAINERS                                   |   7 +
>  2 files changed, 156 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad7191.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7191.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7191.yaml
> new file mode 100644
> index 000000000000..ac14096ba76c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7191.yaml
> @@ -0,0 +1,149 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +# Copyright 2025 Analog Devices Inc.
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/adc/adi,ad7191.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Analog Devices AD7191 ADC
> +
> +maintainers:
> +  - Alisa-Dariana Roman <alisa.roman@analog.com>
> +
> +description: |
> +  Bindings for the Analog Devices AD7191 ADC device. Datasheet can be
> +  found here:
> +  https://www.analog.com/media/en/technical-documentation/data-sheets/AD7191.pdf
> +  The device's PDOWN pin must be connected to the SPI controller's chip select
> +  pin.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - adi,ad7191
> +
> +  reg:
> +    maxItems: 1
> +
> +  spi-cpol: true
> +
> +  spi-cpha: true
> +
> +  clocks:
> +    maxItems: 1
> +    description: |

Don't need '|' if no formatting.

> +      Must be present when CLKSEL pin is tied HIGH to select external clock
> +      source (either a crystal between MCLK1 and MCLK2 pins, or a
> +      CMOS-compatible clock driving MCLK2 pin). Must be absent when CLKSEL pin
> +      is tied LOW to use the internal 4.92MHz clock.
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  avdd-supply:
> +    description: AVdd voltage supply
> +
> +  dvdd-supply:
> +    description: DVdd voltage supply
> +
> +  vref-supply:
> +    description: Vref voltage supply
> +
> +  odr-gpios:
> +    description: |

Don't need '|' if no formatting.

> +      ODR1 and ODR2 pins for output data rate selection. Should be defined if
> +      adi,odr-value is absent.
> +    minItems: 2
> +    maxItems: 2
> +
> +  adi,odr-value:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: |
> +      Should be present if ODR pins are pin-strapped. Possible values:
> +      120 Hz (ODR1=0, ODR2=0)
> +      60 Hz (ODR1=0, ODR2=1)
> +      50 Hz (ODR1=1, ODR2=0)
> +      10 Hz (ODR1=1, ODR2=1)
> +      If defined, odr-gpios must be absent.
> +    enum: [120, 60, 50, 10]
> +
> +  pga-gpios:
> +    description: |

Don't need '|' if no formatting.

With those fixed,

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>

> +      PGA1 and PGA2 pins for gain selection. Should be defined if adi,pga-value
> +      is absent.
> +    minItems: 2
> +    maxItems: 2

Re: [PATCH v3 0/3] Add support for AD7191

[Jonathan Cameron]

On Wed, 29 Jan 2025 16:29:01 +0200
Alisa-Dariana Roman <alisadariana@gmail.com> wrote:

> Thank you all for your feedback! Here is the updated series of patches!
> 
> I addressed all the replies' points, except for the one about the size of the
> avail array being 1 when the pga/odr pins are pin-strapped. David raised a very
> good point, but, for now, I left the size fixed to 4, since the functions for
> setting the values return error anyway when they are pin-strapped.
Shouldn't have them pretending they can be changed when they can't
(which is what I think you mean about size fixed to 4). So this needs resolving.
> 
> I thought of 3 approaches:
> 	- dynamic allocation for the avail arrays
Probably best avoided.

> 	- different avail array for the 2 different cases (pin-strap or gpios)
That works for me.

> 	- different channels array for the 2 different cases (probably too much)

It is a bit odd to list avail when only one value, but not wrong as such so
I think no need to go this far, though maybe there is a userspace library
this might confuse?

> 
> If the current setup if not good enough, which approach would be the best?
> 
> Kind regards,
> Alisa-Dariana Roman.
> 
> ---
> 
> v2: https://lore.kernel.org/all/20250122132821.126600-1-alisa.roman@analog.com/
> 
> v2 -> v3:
> 	- correct binding title
> 	- remove clksel_state and clksel_gpio, assume the clksel pin is always
> pinstrapped
> 	- rephrase clocks description accordingly
> 	- simplify binding constraints
> 	- specify in binding description that PDOWN must be connected to SPI's
> controller's CS
> 	- add minItems for gpios in bindings
> 	- make scope explicit for mutex guard
> 	- remove spi irq check
> 	- add id_table to spi_driver struct
> 	- changed comments as suggested
> 	- use spi_message_init_with_transfers()
> 	- default returns an error in ad7191_set_mode()
> 	- replace hard-coded 2 with st->pga_gpios->ndescs
> 	- use gpiod_set_array_value_cansleep()
> 	- change .storagebits to 32
> 	- check return value for ad_sd_init()
> 	- change to adi,odr-value and adi,pga-value, which now accepts the value as
> suggested
> 	- modify variables names and refactor the setup of odr and pga gpios,
> indexes and available arrays into ad7191_config_setup(), since they are all
> related
> 	- add ad7191.rst
> 
> v1: https://lore.kernel.org/all/20241221155926.81954-1-alisa.roman@analog.com/
> 
> v1 -> v2:
> 	- removed patch adding function in ad_sigma_delta.h/.c
> 	- added a function set_cs() for asserting/deasserting the cs
> 	- handle pinstrapping cases
> 	- refactored all clock handling
> 	- updated bindings: corrected and added new things
> 	- -> address of the channels is used in set_channel()  
> 	- addressed all the other changes
> 

Re: [PATCH v3 2/3] iio: adc: ad7191: add AD7191

[Jonathan Cameron]

On Wed, 29 Jan 2025 16:29:03 +0200
Alisa-Dariana Roman <alisadariana@gmail.com> wrote:

> AD7191 is a pin-programmable, ultra-low noise 24-bit sigma-delta ADC
> designed for precision bridge sensor measurements. It features two
> differential analog input channels, selectable output rates,
> programmable gain, internal temperature sensor and simultaneous
> 50Hz/60Hz rejection.
> 
> Signed-off-by: Alisa-Dariana Roman <alisa.roman@analog.com>
Hi Alisa-Dariana

Seeing as a v4 is needed for the available thing you mention
in the cover letter, a few minor things inline I might otherwise
have just tidied up whilst applying.

Thanks,

Jonathan


> diff --git a/drivers/iio/adc/ad7191.c b/drivers/iio/adc/ad7191.c
> new file mode 100644
> index 000000000000..b6c1d5c25783
> --- /dev/null
> +++ b/drivers/iio/adc/ad7191.c

> +
> +#include <linux/iio/adc/ad_sigma_delta.h>
> +#include <linux/iio/iio.h>
> +
> +#define ad_sigma_delta_to_ad7191(sigmad)	container_of((sigmad), struct ad7191_state, sd)
#define ad_sigma_delta_to_ad7191(sigmad)	\
	container_of((sigmad), struct ad7191_state, sd)

Given it is very long otherwise.

> +
> +#define AD7191_TEMP_CODES_PER_DEGREE	2815
> +
> +#define AD7191_EXT_CLK_ENABLE		0
> +#define AD7191_INT_CLK_ENABLE		1
> +
> +#define AD7191_CHAN_MASK		BIT(0)
> +#define AD7191_TEMP_MASK		BIT(1)
> +
> +enum ad7191_channel {
> +	AD7191_CH_AIN1_AIN2,
> +	AD7191_CH_AIN3_AIN4,
> +	AD7191_CH_TEMP
Add a trailing comma. Maybe there will be more channels on some
compatible part that we add after this.

> +};
> +
> +/*
> + * NOTE:
> + * The AD7191 features a dual-use data out ready DOUT/RDY output.
> + * In order to avoid contentions on the SPI bus, it's therefore necessary
> + * to use SPI bus locking.
> + *
> + * The DOUT/RDY output must also be wired to an interrupt-capable GPIO.
> + *
> + * The SPI controller's chip select must be connected to the PDOWN pin
> + * of the ADC. When CS (PDOWN) is high, it powers down the device and
> + * resets the internal circuitry.
> + */
> +
> +struct ad7191_state {
> +	struct ad_sigma_delta		sd;
> +	struct mutex			lock; // to protect sensor state
Only use old style /* */ in IIO code with exception of the SPDX
corner case.  This is just a consistency thing hanging over
from when that was only thing used in the kernel.


> +
> +	struct gpio_descs		*odr_gpios;
> +	struct gpio_descs		*pga_gpios;
> +	struct gpio_desc		*temp_gpio;
> +	struct gpio_desc		*chan_gpio;
> +
> +	u16				int_vref_mv;
> +	u32				pga_index;
> +	u32				scale_avail[4][2];
> +	u32				odr_index;
> +	u32				samp_freq_avail[4];
> +
> +	struct clk			*mclk;
> +};

> +
> +static int ad7191_config_setup(struct iio_dev *indio_dev)
> +{
> +	struct ad7191_state *st = iio_priv(indio_dev);
> +	struct device *dev = &st->sd.spi->dev;
> +	/* Sampling frequencies in Hz, see Table 5 */
> +	const int samp_freq[4] = {120, 60, 50, 10};

Space after { and before } in all such places.
This is just my local IIO preference as nice to pick a standard.
I often just tweak this whilst applying but seeing as you
are going to be doing a v4 anyway please tidy it up!

> +	/* Gain options, see Table 7 */
> +	const int gain[4] = {1, 8, 64, 128};
> +	int odr_value, pga_value, i, ret;
> +	u64 scale_uv;
...


> +
> +static int ad7191_setup(struct iio_dev *indio_dev, struct device *dev)
> +{
> +	struct ad7191_state *st = iio_priv(indio_dev);
> +	int ret;
> +
> +	ret = ad7191_init_regulators(indio_dev);
> +	if (ret)
> +		return ret;
> +
> +	ret = ad7191_config_setup(indio_dev);
> +	if (ret)
> +		return ret;
> +
> +	ret = ad7191_clock_setup(st);
> +	if (ret)
> +		return ret;
> +
> +	return 0;
	return ad7191_clock_setup(st);
and save a few lines of code.

> +}
> +
> +static int ad7191_read_raw(struct iio_dev *indio_dev,
> +			   struct iio_chan_spec const *chan, int *val,
> +			   int *val2, long m)
> +{
> +	struct ad7191_state *st = iio_priv(indio_dev);
> +
> +	switch (m) {
> +	case IIO_CHAN_INFO_RAW:
> +		return ad_sigma_delta_single_conversion(indio_dev, chan, val);
> +	case IIO_CHAN_INFO_SCALE:
> +		switch (chan->type) {
> +		case IIO_VOLTAGE: {
> +			guard(mutex)(&st->lock);
> +			*val = st->scale_avail[st->pga_index][0];
> +			*val2 = st->scale_avail[st->pga_index][1];
> +			return IIO_VAL_INT_PLUS_NANO;
> +		}
> +		case IIO_TEMP:
> +			*val = 0;
> +			*val2 = NANO / AD7191_TEMP_CODES_PER_DEGREE;
> +			return IIO_VAL_INT_PLUS_NANO;
> +		default:
> +			return -EINVAL;
> +		}
> +	case IIO_CHAN_INFO_OFFSET:
> +		*val = -(1 << (chan->scan_type.realbits - 1));
> +		switch (chan->type) {
> +		case IIO_VOLTAGE:
> +			return IIO_VAL_INT;
> +		case IIO_TEMP:
> +			*val -= 273 * AD7191_TEMP_CODES_PER_DEGREE;
> +			return IIO_VAL_INT;
> +		default:
> +			return -EINVAL;
> +		}
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		*val = st->samp_freq_avail[st->odr_index];
> +		return IIO_VAL_INT;
> +	}
> +

Can't get here so drop this.  I guess the bot hasn't tested this one yet
or we should have seen unreachable warnings.

> +	return -EINVAL;
> +}
> +
> +static int ad7191_set_gain(struct ad7191_state *st, int gain_index)
> +{
> +	unsigned long value = gain_index;
> +
> +	if (!st->pga_gpios)
> +		return -EPERM;
> +
> +	st->pga_index = gain_index;
> +
> +	return gpiod_set_array_value_cansleep(st->pga_gpios->ndescs,
> +					      st->pga_gpios->desc,
> +					      st->pga_gpios->info, &value);
> +
> +	return 0;

double return.  Drop this bonus one!

> +}
> +

Re: [PATCH v3 3/3] docs: iio: add AD7191

[Jonathan Cameron]

On Wed, 29 Jan 2025 16:29:04 +0200
Alisa-Dariana Roman <alisadariana@gmail.com> wrote:

> Add documentation for AD7191 driver.
> 
> Signed-off-by: Alisa-Dariana Roman <alisa.roman@analog.com>
Hi Alisa-Dariana,

Please could you build this with make html_docs

It needs to be added to the index as well to build.

Then check formatting of titles and bullet points etc. Looks like
a few bits of formatting of the rst aren't quite right but I haven't
built it to be sure of that.

Content looks fine to me.

Thanks,

Jonathan

> ---
>  Documentation/iio/ad7191.rst | 230 +++++++++++++++++++++++++++++++++++
>  1 file changed, 230 insertions(+)
>  create mode 100644 Documentation/iio/ad7191.rst
> 
> diff --git a/Documentation/iio/ad7191.rst b/Documentation/iio/ad7191.rst
> new file mode 100644
> index 000000000000..78aa5fefe128
> --- /dev/null
> +++ b/Documentation/iio/ad7191.rst
> @@ -0,0 +1,230 @@
> +.. SPDX-License-Identifier: GPL-2.0-only
> +
> +==============
> +AD7191 driver
> +==============
> +
> +Device driver for Analog Devices AD7191 ADC.
> +
> +Supported devices
> +================
> +
> +* `AD7191 <https://www.analog.com/AD7191>`_
> +
> +The AD7191 is a high precision, low noise, 24-bit Σ-Δ ADC with integrated PGA.
> +It features two differential input channels, an internal temperature sensor,and
> +configurable sampling rates.
> +
> +Device Configuration
> +===================
> +
> +Pin Configuration
> +----------------
> +
> +The driver supports both pin-strapped and GPIO-controlled configurations for ODR
> +(Output Data Rate) and PGA (Programmable Gain Amplifier) settings. These
> +configurations are mutually exclusive - you must use either pin-strapped or GPIO
> +control for each setting, not both.
> +
> +ODR Configuration
> +^^^^^^^^^^^^^^^^
> +
> +The ODR can be configured either through GPIO control or pin-strapping:
> +
> +- When using GPIO control, specify the "odr-gpios" property in the device tree
> +- For pin-strapped configuration, specify the "adi,odr-value" property in the
> +device tree

This doesn't look like correct rst for bullet points. Please
build the docs and sanity check this.  I believe it needs to be aligned
and you may nee some blank lines in a few places.

> +
> +Available ODR settings:
> +  - 120 Hz (ODR1=0, ODR2=0)
> +  - 60 Hz (ODR1=0, ODR2=1)
> +  - 50 Hz (ODR1=1, ODR2=0)
> +  - 10 Hz (ODR1=1, ODR2=1)
> +
> +PGA Configuration
> +^^^^^^^^^^^^^^^
> +
> +The PGA can be configured either through GPIO control or pin-strapping:
> +
> +- When using GPIO control, specify the "pga-gpios" property in the device tree
> +- For pin-strapped configuration, specify the "adi,pga-value" property in the
> +device tree
> +
> +Available PGA gain settings:
> +  - 1x (PGA1=0, PGA2=0)
> +  - 8x (PGA1=0, PGA2=1)
> +  - 64x (PGA1=1, PGA2=0)
> +  - 128x (PGA1=1, PGA2=1)
> +
> +Clock Configuration
> +-----------------
> +
> +The AD7191 supports both internal and external clock sources:
> +
> +- When CLKSEL pin is tied LOW: Uses internal 4.92MHz clock (no clock property
> +needed)
> +- When CLKSEL pin is tied HIGH: Requires external clock source
> +  - Can be a crystal between MCLK1 and MCLK2 pins
> +  - Or a CMOS-compatible clock driving MCLK2 pin
> +  - Must specify the "clocks" property in device tree when using external clock
> +
> +SPI Interface Requirements
> +------------------------
> +
> +The AD7191 has specific SPI interface requirements:
> +
> +- The DOUT/RDY output is dual-purpose and requires SPI bus locking
> +- DOUT/RDY must be connected to an interrupt-capable GPIO
> +- The SPI controller's chip select must be connected to the PDOWN pin of the ADC
> +- When CS (PDOWN) is high, the device powers down and resets internal circuitry
> +- SPI mode 3 operation (CPOL=1, CPHA=1) is required
> +
> +Power Supply Requirements
> +-----------------------
> +
> +The device requires the following power supplies:
> +
> +- AVdd: Analog power supply
> +- DVdd: Digital power supply
> +- Vref: Reference voltage supply (external)
> +
> +All power supplies must be specified in the device tree.
> +
> +Device Attributes
> +===============

Not enough =

Check all these as well after make html_docs


> +
> +The AD7191 provides several attributes through the IIO sysfs interface:
> +
> +Voltage Input Differential Channels
> +---------------------------------
> +
> ++-------------------+----------------------------------------------------------+
> +| Attribute         | Description                                              |
> ++===================+==========================================================+
> +| raw               | Raw ADC output value                                     |
> ++-------------------+----------------------------------------------------------+
> +| scale             | Scale factor to convert raw value to voltage             |
> ++-------------------+----------------------------------------------------------+
> +| offset            | Voltage offset                                           |
> ++-------------------+----------------------------------------------------------+
> +| sampling_frequency| Current sampling frequency setting                       |
> ++-------------------+----------------------------------------------------------+
> +
> +Temperature Sensor
> +----------------
> +
> ++-------------------+----------------------------------------------------------+
> +| Attribute         | Description                                              |
> ++===================+==========================================================+
> +| raw               | Raw temperature sensor output value                      |
> ++-------------------+----------------------------------------------------------+
> +| scale             | Scale factor to convert raw value to temperature         |
> ++-------------------+----------------------------------------------------------+
> +| offset            | Temperature calibration offset                           |
> ++-------------------+----------------------------------------------------------+
> +
> +Available Attributes
> +------------------
> +
> +The following attributes show available configuration options:
> +
> +- sampling_frequency_available: List of supported sampling frequencies
> +- scale_available: List of supported scale factors (based on PGA settings)
> +
> +Channel Configuration
> +===================
> +
> +The device provides three channels:
> +
> +1. Temperature Sensor
> +   - 24-bit unsigned
> +   - Internal temperature measurement
> +   - Temperature in millidegrees Celsius
> +
> +2. Differential Input (AIN1-AIN2)
> +   - 24-bit unsigned
> +   - Differential voltage measurement
> +   - Configurable gain via PGA
> +
> +3. Differential Input (AIN3-AIN4)
> +   - 24-bit unsigned
> +   - Differential voltage measurement
> +   - Configurable gain via PGA
> +
> +Device Tree Bindings
> +===================
> +
> +Required Properties
> +-----------------
> +
> +- compatible: Should be "adi,ad7191"
> +- reg: SPI chip select number
> +- spi-max-frequency: Maximum SPI clock frequency
> +- spi-cpol: Must be present (set to 1)
> +- spi-cpha: Must be present (set to 1)
> +- interrupts: Interrupt mapping for DOUT/RDY pin
> +- avdd-supply: Analog power supply
> +- dvdd-supply: Digital power supply
> +- vref-supply: Reference voltage supply
> +- temp-gpios: GPIO for temperature channel selection
> +- chan-gpios: GPIO for input channel selection
> +
> +Optional Properties
> +-----------------
> +
> +- clocks: Required when using external clock (CLKSEL=1), must be absent for
> +internal clock
> +- adi,odr-value: Pin-strapped ODR configuration (120, 60, 50, or 10)
> +- adi,pga-value: Pin-strapped PGA configuration (1, 8, 64, or 128)
> +- odr-gpios: GPIOs for ODR control (mutually exclusive with adi,odr-value)
> +- pga-gpios: GPIOs for PGA control (mutually exclusive with adi,pga-value)
> +
> +Example Device Tree
> +-----------------
> +
> +.. code-block:: dts
> +
> +    #include <dt-bindings/gpio/gpio.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    spi {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        ad7191@0 {
> +            compatible = "adi,ad7191";
> +            reg = <0>;
> +            spi-max-frequency = <1000000>;
> +
> +            /* Required SPI mode 3 */
> +            spi-cpol;
> +            spi-cpha;
> +
> +            /* Interrupt for DOUT/RDY pin */
> +            interrupts = <25 IRQ_TYPE_EDGE_FALLING>;
> +            interrupt-parent = <&gpio>;
> +
> +            /* Power supplies */
> +            avdd-supply = <&avdd>;
> +            dvdd-supply = <&dvdd>;
> +            vref-supply = <&vref>;
> +
> +            /* Optional external clock */
> +            clocks = <&ad7191_mclk>;
> +
> +            /* Configuration - either use GPIO control or pin-strapped values */
> +            adi,pga-value = <1>;
> +            odr-gpios = <&gpio 23 GPIO_ACTIVE_HIGH>,
> +                       <&gpio 24 GPIO_ACTIVE_HIGH>;
> +
> +            /* Required GPIO controls */
> +            temp-gpios = <&gpio 22 GPIO_ACTIVE_HIGH>;
> +            chan-gpios = <&gpio 27 GPIO_ACTIVE_HIGH>;
> +        };
> +    };
> +
> +Buffer Support
> +============
> +
> +This driver supports IIO triggered buffers. See Documentation/iio/iio_devbuf.rst
> +for more information about IIO triggered buffers.