[PATCH v4 0/3] Add support for AD7191

[PATCH v4 0/3] Add support for AD7191

[Alisa-Dariana Roman]

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

Kind regards,
Alisa-Dariana Roman.

---

v3: https://lore.kernel.org/all/20250129143054.225322-1-alisa.roman@analog.com/

v3 -> v4:
	- addressed all replies for v3
	- refactored the scale and sampling frequencies configurations to use 2
different arrays for gpio case vs pinstrap case

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 v4 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>
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
---
 .../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..801ed319ee82
--- /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 v4 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 | 559 +++++++++++++++++++++++++++++++++++++++
 4 files changed, 571 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..4a9e66853294
--- /dev/null
+++ b/drivers/iio/adc/ad7191.c
@@ -0,0 +1,559 @@
+// 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; /* Protect device 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				scale_avail_gpio[4][2];
+	u32				scale_avail_pinstrap[1][2];
+	const u32			(*scale_avail)[2];
+	size_t				scale_avail_size;
+	u32				scale_index;
+	u32				samp_freq_avail_gpio[4];
+	u32				samp_freq_avail_pinstrap[1];
+	const u32			*samp_freq_avail;
+	size_t				samp_freq_avail_size;
+	u32				samp_freq_index;
+
+	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, odr_index, pga_value, pga_index, i, ret;
+	u64 scale_uv;
+
+	st->samp_freq_index = 0;
+	st->scale_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");
+
+		for (i = 0; i < ARRAY_SIZE(samp_freq); i++)
+			st->samp_freq_avail_gpio[i] = samp_freq[i];
+
+		st->samp_freq_avail = st->samp_freq_avail_gpio;
+		st->samp_freq_avail_size = ARRAY_SIZE(st->samp_freq_avail_gpio);
+	} else {
+		for (i = 0; i < ARRAY_SIZE(samp_freq); i++) {
+			if (odr_value != samp_freq[i])
+				continue;
+			odr_index = i;
+		}
+
+		st->samp_freq_avail_pinstrap[0] = samp_freq[odr_index];
+
+		st->samp_freq_avail = st->samp_freq_avail_pinstrap;
+		st->samp_freq_avail_size = ARRAY_SIZE(st->samp_freq_avail_pinstrap);
+
+		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");
+
+		for (i = 0; i < ARRAY_SIZE(st->scale_avail_gpio); 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_gpio[i][1] = do_div(scale_uv, NANO);
+			st->scale_avail_gpio[i][0] = scale_uv;
+		}
+
+		st->scale_avail = st->scale_avail_gpio;
+		st->scale_avail_size = ARRAY_SIZE(st->scale_avail_gpio);
+	} else {
+		for (i = 0; i < ARRAY_SIZE(gain); i++) {
+			if (pga_value != gain[i])
+				continue;
+			pga_index = i;
+		}
+
+		scale_uv = ((u64)st->int_vref_mv * NANO) >>
+			(indio_dev->channels[0].scan_type.realbits - 1);
+		do_div(scale_uv, gain[pga_index]);
+		st->scale_avail_pinstrap[0][1] = do_div(scale_uv, NANO);
+		st->scale_avail_pinstrap[0][0] = scale_uv;
+
+		st->scale_avail = st->scale_avail_pinstrap;
+		st->scale_avail_size = ARRAY_SIZE(st->scale_avail_pinstrap);
+
+		st->pga_gpios = NULL;
+	}
+
+	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;
+
+	return ad7191_clock_setup(st);
+}
+
+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->scale_index][0];
+			*val2 = st->scale_avail[st->scale_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->samp_freq_index];
+		return IIO_VAL_INT;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int ad7191_set_gain(struct ad7191_state *st, int gain_index)
+{
+	unsigned long value = gain_index;
+
+	st->scale_index = gain_index;
+
+	return gpiod_set_array_value_cansleep(st->pga_gpios->ndescs,
+					      st->pga_gpios->desc,
+					      st->pga_gpios->info, &value);
+}
+
+static int ad7191_set_samp_freq(struct ad7191_state *st, int samp_freq_index)
+{
+	unsigned long value = samp_freq_index;
+
+	st->samp_freq_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: {
+		if (!st->pga_gpios)
+			return -EPERM;
+		guard(mutex)(&st->lock);
+		for (i = 0; i < ARRAY_SIZE(st->scale_avail_gpio); i++) {
+			if (val2 != st->scale_avail_gpio[i][1])
+				continue;
+			return ad7191_set_gain(st, i);
+		}
+		return -EINVAL;
+	}
+	case IIO_CHAN_INFO_SAMP_FREQ: {
+		if (!st->odr_gpios)
+			return -EPERM;
+		guard(mutex)(&st->lock);
+		for (i = 0; i < ARRAY_SIZE(st->samp_freq_avail_gpio); i++) {
+			if (val != st->samp_freq_avail_gpio[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 = st->scale_avail_size * 2;
+		return IIO_AVAIL_LIST;
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		*vals = (int *)st->samp_freq_avail;
+		*type = IIO_VAL_INT;
+		*length = st->samp_freq_avail_size;
+		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 v4 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 | 250 +++++++++++++++++++++++++++++++++++
 Documentation/iio/index.rst  |   1 +
 2 files changed, 251 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..b55f3c13e45a
--- /dev/null
+++ b/Documentation/iio/ad7191.rst
@@ -0,0 +1,250 @@
+.. 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.
diff --git a/Documentation/iio/index.rst b/Documentation/iio/index.rst
index 5710f5b9e958..9235f38f47a1 100644
--- a/Documentation/iio/index.rst
+++ b/Documentation/iio/index.rst
@@ -20,6 +20,7 @@ Industrial I/O Kernel Drivers
 
    ad4000
    ad4695
+   ad7191
    ad7380
    ad7606
    ad7625
-- 
2.43.0

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

[David Lechner]

On 2/3/25 7:31 AM, Alisa-Dariana Roman wrote:
> Add documentation for AD7191 driver.
> 
> Signed-off-by: Alisa-Dariana Roman <alisa.roman@analog.com>
> ---
>  Documentation/iio/ad7191.rst | 250 +++++++++++++++++++++++++++++++++++
>  Documentation/iio/index.rst  |   1 +
>  2 files changed, 251 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..b55f3c13e45a
> --- /dev/null
> +++ b/Documentation/iio/ad7191.rst
> @@ -0,0 +1,250 @@
> +.. 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
> +=====================
> +

I would call this section the `Devicetree/wiring` section since devicetree has
to do with how the chip is wired up.

The existing `Device Tree Bindings` section at the end pretty much just repeats
the DT bindings .yml doc, so we could drop that section from this doc.

What you have written in this section already covers it very well.

> +--------------------
> +Pin Configuration
> +--------------------

I think it looks the nicest when the --- exactly line up with the text and I
think that is the usual kernel style as well.

Example:

-----------------
Pin Configuration
-----------------

Same applies to the rest of the doc.


> +===================
> +Device Attributes
> +===================

There isn't really anything unusual about attributes on this chip compared to
typical ADCs, so I would be OK if we left out this section. If we do keep it
though, I think we should write out the full attribute name since some are
separate, some shared_by_type and some shared_by_all, so they have different
prefixes.

> +
> +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)
> +

One of these days, we should probably write a generic page on the common
attributes raw/scale/offset and somewhat common sampling_frequency/
oversampling_ratio (probably a few more that I'm forgetting). :-)

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

[David Lechner]

On 2/3/25 7:31 AM, 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>
> ---

...

> +struct ad7191_state {
> +	struct ad_sigma_delta		sd;
> +	struct mutex			lock; /* Protect device 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				scale_avail_gpio[4][2];
> +	u32				scale_avail_pinstrap[1][2];
> +	const u32			(*scale_avail)[2];

This feels a bit reduant to have two arrays and then a pointer to one of those
arrays. We could just have a single static const array of 4 and use that in both
cases. (also see further comments later)

> +	size_t				scale_avail_size;
> +	u32				scale_index;
> +	u32				samp_freq_avail_gpio[4];
> +	u32				samp_freq_avail_pinstrap[1];
> +	const u32			*samp_freq_avail;

ditto

> +	size_t				samp_freq_avail_size;
> +	u32				samp_freq_index;
> +
> +	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_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 };

As per my earlier suggestion, we can make this static const...

> +	/* Gain options, see Table 7 */
> +	const int gain[4] = { 1, 8, 64, 128 };

ditto

> +	int odr_value, odr_index, pga_value, pga_index, i, ret;
> +	u64 scale_uv;
> +
> +	st->samp_freq_index = 0;
> +	st->scale_index = 0;
> +
> +	ret = device_property_read_u32(dev, "adi,odr-value", &odr_value);

Shoud also check if (ret && ret != -EINVAL) first to catch other errors like
someone put a string in the .dts instead of a u32.

> +	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");
> +
> +		for (i = 0; i < ARRAY_SIZE(samp_freq); i++)
> +			st->samp_freq_avail_gpio[i] = samp_freq[i];
> +
> +		st->samp_freq_avail = st->samp_freq_avail_gpio;
> +		st->samp_freq_avail_size = ARRAY_SIZE(st->samp_freq_avail_gpio);

...then here instead of copying...

		st->samp_freq_avail = samp_freq;
		st->samp_freq_avail_size = ARRAY_SIZE(samp_freq);

> +	} else {
> +		for (i = 0; i < ARRAY_SIZE(samp_freq); i++) {
> +			if (odr_value != samp_freq[i])
> +				continue;
> +			odr_index = i;

missing break;?

Also, should we error if match not found? Otherwise we could have uninitalized
odr_index;

> +		}
> +
> +		st->samp_freq_avail_pinstrap[0] = samp_freq[odr_index];
> +
> +		st->samp_freq_avail = st->samp_freq_avail_pinstrap;
> +		st->samp_freq_avail_size = ARRAY_SIZE(st->samp_freq_avail_pinstrap);
> +

and here...

		st->samp_freq_avail = &samp_freq[odr_index];
		st->samp_freq_avail_size = 1;

> +		st->odr_gpios = NULL;
> +	}
> +
> +	ret = device_property_read_u32(dev, "adi,pga-value", &pga_value);

ditto about error checking

> +	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");
> +
> +		for (i = 0; i < ARRAY_SIZE(st->scale_avail_gpio); 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_gpio[i][1] = do_div(scale_uv, NANO);
> +			st->scale_avail_gpio[i][0] = scale_uv;
> +		}
> +
> +		st->scale_avail = st->scale_avail_gpio;
> +		st->scale_avail_size = ARRAY_SIZE(st->scale_avail_gpio);
> +	} else {
> +		for (i = 0; i < ARRAY_SIZE(gain); i++) {
> +			if (pga_value != gain[i])
> +				continue;
> +			pga_index = i;
> +		}
> +
> +		scale_uv = ((u64)st->int_vref_mv * NANO) >>
> +			(indio_dev->channels[0].scan_type.realbits - 1);
> +		do_div(scale_uv, gain[pga_index]);
> +		st->scale_avail_pinstrap[0][1] = do_div(scale_uv, NANO);
> +		st->scale_avail_pinstrap[0][0] = scale_uv;
> +
> +		st->scale_avail = st->scale_avail_pinstrap;
> +		st->scale_avail_size = ARRAY_SIZE(st->scale_avail_pinstrap);
> +
> +		st->pga_gpios = NULL;
> +	}

and ditto about st->scale_avail and pinstrap matching for loop

> +
> +	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_set_gain(struct ad7191_state *st, int gain_index)
> +{
> +	unsigned long value = gain_index;
> +
> +	st->scale_index = gain_index;
> +
> +	return gpiod_set_array_value_cansleep(st->pga_gpios->ndescs,
> +					      st->pga_gpios->desc,
> +					      st->pga_gpios->info, &value);
> +}

Depending on timing, we might be able to take advantage of [1].

But it isn't merged yet and needs another revision and you are very fast, so
don't wait on it. ;-)

[1]: https://lore.kernel.org/linux-iio/20250131-gpio-set-array-helper-v1-0-991c8ccb4d6e@baylibre.com/


> +
> +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),

Should this one be info_mask_separate?

Since this is a multiplexed ADC and not simelutaneous sampling, I would expect
that if the ORD pins are set to 10Hz (0.1s period), then a buffered read with
all channels enabled would take 0.3s to do the 3 samples (effective sample rate
of 3.33Hz), but if only one channel was enabled in the buffer, then it only
takes 0.1s to do all of the samples (effective sample rate is 10Hz).

The iio convention is to use info_mask_separate for the sampling_frequency
attribute to indicate that the rate only applies to each individual channel
and not the combined rate to do one "set" of samples for all enabled channels.

A sampling_frequency attribute that was shared_by_all would mean that on each
period equivlent to this rate, all samples are read no matter how many channels
were enabled for a buffered read.

> +		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
> +		.scan_type = {
> +			.sign = 'u',
> +			.realbits = 24,
> +			.storagebits = 32,
> +			.endianness = IIO_BE,
> +		},
> +	},

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

[Jonathan Cameron]

> 
> 
> > +
> > +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),  
> 
> Should this one be info_mask_separate?
> 
> Since this is a multiplexed ADC and not simelutaneous sampling, I would expect
> that if the ORD pins are set to 10Hz (0.1s period), then a buffered read with
> all channels enabled would take 0.3s to do the 3 samples (effective sample rate
> of 3.33Hz), but if only one channel was enabled in the buffer, then it only
> takes 0.1s to do all of the samples (effective sample rate is 10Hz).
> 
> The iio convention is to use info_mask_separate for the sampling_frequency
> attribute to indicate that the rate only applies to each individual channel
> and not the combined rate to do one "set" of samples for all enabled channels.
> 
> A sampling_frequency attribute that was shared_by_all would mean that on each
> period equivlent to this rate, all samples are read no matter how many channels
> were enabled for a buffered read.

I think this device channel selection is onehot.  As such only one channel
is ever enabled at a time. See num_slots in ad_sigma_delta
which is set to 1 if not specified. We could argue that maybe one day the
infrastructure will be added to make that code deal with dynamic channel 
changes, settling time and all that mess though so maybe it should be
separate even though it makes no difference today.

Jonathan

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

[Jonathan Cameron]

On Mon,  3 Feb 2025 15:31:27 +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,

Just one trivial additional comment from me as you'll be doing a v5
anyway.  If it had just been this I'd have tweaked it whilst applying.

Thanks,

Jonathan

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

> +static int ad7191_setup(struct iio_dev *indio_dev, struct device *dev)

dev isn't used, so drop that parameter.

> +{
> +	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;
> +
> +	return ad7191_clock_setup(st);
> +}