[PATCH 0/2] iio: adc: Add Texas Instruments ADS1220 ADC

[PATCH 0/2] iio: adc: Add Texas Instruments ADS1220 ADC

[Nguyen Minh Tien]

This series adds support for the Texas Instruments ADS1220, a 24-bit,
2-kSPS, 4-channel delta-sigma ADC with an SPI (mode 1) interface, a
programmable gain amplifier (1 to 128), an internal 2.048V reference and
a dedicated DRDY data-ready output.

The driver supports:
 - single-ended and differential voltage channels described as
   device-tree child nodes;
 - per-channel programmable gain (via _scale) and data rate (via
   _sampling_frequency), with the matching *_available attributes;
 - the internal 2.048V reference, an external reference on REFP0/REFN0
   via a regulator, or the analog supply (AVDD) as a ratiometric
   reference for single-supply measurements;
 - single-shot conversions gated on the DRDY interrupt, or on a
   data-rate-derived delay when no interrupt is wired;
 - a DRDY-interrupt-driven triggered buffer for streaming;
 - runtime PM (power-down between conversions).

I tested this on a Lichee Pi Nano (Allwinner F1C100s) running Linux 7.0:
with a potentiometer on AIN0 (single-ended against AVSS, AVDD as the
reference), in_voltage0_raw tracks the wiper linearly across the full
0..3.3V range (0 to 0x7fffff). Nothing in the driver is board-specific -
it only uses the SPI and IIO frameworks - so it should work on any SPI
host.

This is my first kernel contribution. I modelled the driver on the
existing TI ADS-family IIO drivers - ti-ads1119 for the structure and
ti-ads124s08 for the SPI side - so I'd welcome any feedback on things
I've missed.

Nguyen Minh Tien (2):
  dt-bindings: iio: adc: Add TI ADS1220
  iio: adc: Add TI ADS1220 driver

 .../bindings/iio/adc/ti,ads1220.yaml          | 146 +++
 MAINTAINERS                                   |   7 +
 drivers/iio/adc/Kconfig                       |  12 +
 drivers/iio/adc/Makefile                      |   1 +
 drivers/iio/adc/ti-ads1220.c                  | 835 ++++++++++++++++++
 5 files changed, 1001 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/ti,ads1220.yaml
 create mode 100644 drivers/iio/adc/ti-ads1220.c


base-commit: ae696dfa47c30016cd429b9db5e70b259b8f509e
-- 
2.34.1

[PATCH 1/2] dt-bindings: iio: adc: Add TI ADS1220

[Nguyen Minh Tien]

The ADS1220 is a 24-bit, 2-kSPS, 4-channel delta-sigma ADC from Texas
Instruments with an SPI (mode 1) interface, a programmable gain amplifier,
an internal 2.048V reference and a dedicated DRDY data-ready output.

Add a device tree binding describing the SPI device and its per-input
channel child nodes (single-ended AINx or the multiplexer's differential
pairs), the optional external/AVDD reference selection and the DRDY
interrupt.

Signed-off-by: Nguyen Minh Tien <zizuzacker@gmail.com>
---
 .../bindings/iio/adc/ti,ads1220.yaml          | 146 ++++++++++++++++++
 1 file changed, 146 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/ti,ads1220.yaml

diff --git a/Documentation/devicetree/bindings/iio/adc/ti,ads1220.yaml b/Documentation/devicetree/bindings/iio/adc/ti,ads1220.yaml
new file mode 100644
index 000000000..1fedffc2a
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/ti,ads1220.yaml
@@ -0,0 +1,146 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/adc/ti,ads1220.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Texas Instruments ADS1220 ADC
+
+maintainers:
+  - Nguyen Minh Tien <zizuzacker@gmail.com>
+
+description:
+  The TI ADS1220 is a precision 24-bit, 2-kSPS, delta-sigma ADC with an SPI
+  (mode 1) interface. It provides two differential or four single-ended inputs
+  through a multiplexer, a programmable gain amplifier (gain 1 to 128), an
+  internal 2.048V reference and oscillator, two programmable excitation current
+  sources and a 50/60Hz rejection filter. A dedicated DRDY output signals when a
+  new conversion result is available.
+
+properties:
+  compatible:
+    const: ti,ads1220
+
+  reg:
+    maxItems: 1
+
+  spi-cpha: true
+
+  interrupts:
+    description: DRDY pin, signals that a new conversion result is ready.
+    maxItems: 1
+
+  avdd-supply:
+    description: Analog power supply (AVDD/AVSS).
+
+  dvdd-supply:
+    description: Digital power supply (DVDD/DGND).
+
+  vref-supply:
+    description:
+      External reference voltage (REFP0/REFN0). If omitted, the internal
+      2.048V reference is used unless ti,vref-avdd is set.
+
+  ti,vref-avdd:
+    type: boolean
+    description:
+      Use the analog supply (AVDD/AVSS) as the conversion reference instead of
+      the internal 2.048V reference. Suited to ratiometric single-supply
+      measurements (for example a potentiometer wired across AVDD), giving a
+      full 0..AVDD input range without an external reference. Ignored when
+      vref-supply is present.
+
+  "#address-cells":
+    const: 1
+
+  "#size-cells":
+    const: 0
+
+  "#io-channel-cells":
+    const: 1
+
+required:
+  - compatible
+  - reg
+  - "#address-cells"
+  - "#size-cells"
+  - avdd-supply
+  - dvdd-supply
+
+patternProperties:
+  "^channel@[0-6]$":
+    $ref: adc.yaml
+    type: object
+    description: Represents one ADC input configuration (channel).
+
+    properties:
+      reg:
+        minimum: 0
+        maximum: 6
+
+      diff-channels:
+        description:
+          Differential input pair routable by the ADS1220 multiplexer.
+        oneOf:
+          - items: [const: 0, const: 1]
+          - items: [const: 0, const: 2]
+          - items: [const: 0, const: 3]
+          - items: [const: 1, const: 2]
+          - items: [const: 1, const: 3]
+          - items: [const: 2, const: 3]
+          - items: [const: 1, const: 0]
+          - items: [const: 3, const: 2]
+
+      single-channel:
+        description:
+          Single-ended input channel AINx measured against AVSS.
+        minimum: 0
+        maximum: 3
+
+    oneOf:
+      - required: [diff-channels]
+      - required: [single-channel]
+
+    required:
+      - reg
+
+    unevaluatedProperties: false
+
+allOf:
+  - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    spi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        adc@0 {
+            compatible = "ti,ads1220";
+            reg = <0>;
+            spi-max-frequency = <2500000>;
+            spi-cpha;
+            interrupt-parent = <&pio>;
+            interrupts = <4 4 IRQ_TYPE_EDGE_FALLING>;
+            avdd-supply = <&reg_vcc3v3>;
+            dvdd-supply = <&reg_vcc3v3>;
+            #address-cells = <1>;
+            #size-cells = <0>;
+            #io-channel-cells = <1>;
+
+            channel@0 {
+                reg = <0>;
+                single-channel = <0>;
+            };
+
+            channel@1 {
+                reg = <1>;
+                diff-channels = <0 1>;
+            };
+        };
+    };
+...
-- 
2.34.1

Re: [PATCH 1/2] dt-bindings: iio: adc: Add TI ADS1220

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The `diff-channels` property constraint uses invalid YAML flow sequence syntax (`items: [const: 0, const: 1]`) that will cause a fatal parser error or evaluate to a single-element list with a collapsed duplicate dictionary key.
- [Medium] The child node regex and `reg` maximum artificially limit the device tree to defining at most 7 channels, despite the hardware supporting 12 unique multiplexer configurations.
--

commit 2b3665a0096105a4bb2db6266e47dec0a0802cff
Author: Nguyen Minh Tien <zizuzacker@gmail.com>

dt-bindings: iio: adc: Add TI ADS1220

This commit adds a device tree binding for the TI ADS1220 24-bit ADC.
It describes the SPI device, its per-input channel child nodes,
external reference selection, and the DRDY interrupt.

> diff --git a/Documentation/devicetree/bindings/iio/adc/ti,ads1220.yaml b/Documentation/devicetree/bindings/iio/adc/ti,ads1220.yaml
> new file mode 100644
> index 0000000000000..1fedffc2a1475
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/ti,ads1220.yaml
[ ... ]
> +patternProperties:
> +  "^channel@[0-6]$":
> +    $ref: adc.yaml
> +    type: object
> +    description: Represents one ADC input configuration (channel).
> +
> +    properties:
> +      reg:
> +        minimum: 0
> +        maximum: 6

[Severity: Medium]
The ADS1220 hardware supports 12 unique multiplexer combinations
(8 differential pairs and 4 single-ended inputs). Does this regex and
reg maximum artificially limit the device tree to defining at most 7
channels?

System integrators might not be able to represent all available
hardware configurations if this limit is enforced.

[ ... ]
> +      diff-channels:
> +        description:
> +          Differential input pair routable by the ADS1220 multiplexer.
> +        oneOf:
> +          - items: [const: 0, const: 1]
> +          - items: [const: 0, const: 2]
> +          - items: [const: 0, const: 3]
> +          - items: [const: 1, const: 2]
> +          - items: [const: 1, const: 3]
> +          - items: [const: 2, const: 3]
> +          - items: [const: 1, const: 0]
> +          - items: [const: 3, const: 2]

[Severity: High]
Will this syntax pass the dt_binding_check validation?

In YAML, a flow sequence like [const: 0, const: 1] evaluates to a list
containing a single implicit mapping with duplicate keys. The dt-schema
toolchain utilizes a strict parser that typically fails on duplicate
dictionary keys.

If it did parse, it would likely collapse the duplicate keys into a
single element, forcing the diff-channels array to have exactly one
element instead of the expected two.

Would it be better to use a valid flow mapping sequence, such as
- items: [{const: 0}, {const: 1}]?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260610151342.44274-1-zizuzacker@gmail.com?part=1

[PATCH 2/2] iio: adc: Add TI ADS1220 driver

[Nguyen Minh Tien]

Add an IIO driver for the Texas Instruments ADS1220 24-bit delta-sigma
SPI ADC. The driver supports single-ended and differential voltage
channels described as device-tree child nodes, per-channel programmable
gain (exposed through scale) and data rate (exposed through sampling
frequency), the internal 2.048V reference, an external reference via a
regulator, or the analog supply (AVDD) as a ratiometric reference,
single-shot conversions and a DRDY-interrupt-driven triggered buffer.
Conversions are gated either on the DRDY interrupt or, when no interrupt
is wired, on a data-rate-derived delay. Runtime PM powers the device down
between conversions.

Signed-off-by: Nguyen Minh Tien <zizuzacker@gmail.com>
---
 MAINTAINERS                  |   7 +
 drivers/iio/adc/Kconfig      |  12 +
 drivers/iio/adc/Makefile     |   1 +
 drivers/iio/adc/ti-ads1220.c | 835 +++++++++++++++++++++++++++++++++++
 4 files changed, 855 insertions(+)
 create mode 100644 drivers/iio/adc/ti-ads1220.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 396d4e76d..1797af05c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -26661,6 +26661,13 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/iio/adc/ti,ads1119.yaml
 F:	drivers/iio/adc/ti-ads1119.c
 
+TI ADS1220 ADC DRIVER
+M:	Nguyen Minh Tien <zizuzacker@gmail.com>
+L:	linux-iio@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/iio/adc/ti,ads1220.yaml
+F:	drivers/iio/adc/ti-ads1220.c
+
 TI ADS1018 ADC DRIVER
 M:	Kurt Borja <kuurtb@gmail.com>
 L:	linux-iio@vger.kernel.org
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index a3a93a47b..cb17a89f0 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -1784,6 +1784,18 @@ config TI_ADS1119
          This driver can also be built as a module. If so, the module will be
          called ti-ads1119.
 
+config TI_ADS1220
+	tristate "Texas Instruments ADS1220 ADC"
+	depends on SPI
+	select IIO_BUFFER
+	select IIO_TRIGGERED_BUFFER
+	help
+	  If you say yes here you get support for Texas Instruments ADS1220
+	  24-bit, 4-channel, delta-sigma ADC chip.
+
+	  This driver can also be built as a module. If so, the module will be
+	  called ti-ads1220.
+
 config TI_ADS124S08
 	tristate "Texas Instruments ADS124S08"
 	depends on SPI
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index 707dd7089..076cd51ed 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -153,6 +153,7 @@ obj-$(CONFIG_TI_ADS1015) += ti-ads1015.o
 obj-$(CONFIG_TI_ADS1018) += ti-ads1018.o
 obj-$(CONFIG_TI_ADS1100) += ti-ads1100.o
 obj-$(CONFIG_TI_ADS1119) += ti-ads1119.o
+obj-$(CONFIG_TI_ADS1220) += ti-ads1220.o
 obj-$(CONFIG_TI_ADS124S08) += ti-ads124s08.o
 obj-$(CONFIG_TI_ADS1298) += ti-ads1298.o
 obj-$(CONFIG_TI_ADS131E08) += ti-ads131e08.o
diff --git a/drivers/iio/adc/ti-ads1220.c b/drivers/iio/adc/ti-ads1220.c
new file mode 100644
index 000000000..06bcc8841
--- /dev/null
+++ b/drivers/iio/adc/ti-ads1220.c
@@ -0,0 +1,835 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Texas Instruments ADS1220 ADC driver
+ *
+ * Datasheet: https://www.ti.com/lit/gpn/ads1220
+ *
+ * Copyright (C) 2026 Nguyen Minh Tien <zizuzacker@gmail.com>
+ */
+
+#include <linux/bitfield.h>
+#include <linux/bits.h>
+#include <linux/completion.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/interrupt.h>
+#include <linux/log2.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/property.h>
+#include <linux/regulator/consumer.h>
+#include <linux/pm_runtime.h>
+#include <linux/spi/spi.h>
+#include <linux/units.h>
+#include <linux/unaligned.h>
+
+#include <linux/iio/buffer.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/trigger.h>
+#include <linux/iio/triggered_buffer.h>
+#include <linux/iio/trigger_consumer.h>
+
+/* SPI commands (Table 8-7) */
+#define ADS1220_CMD_RESET	0x06
+#define ADS1220_CMD_START	0x08
+#define ADS1220_CMD_POWERDOWN	0x02
+#define ADS1220_CMD_RDATA	0x10
+#define ADS1220_CMD_RREG	0x20
+#define ADS1220_CMD_WREG	0x40
+/* RREG/WREG operate on one register (nn = 0 => 1 byte) at address rr */
+#define ADS1220_CMD_RREG_REG(reg)	(ADS1220_CMD_RREG | ((reg) << 2))
+#define ADS1220_CMD_WREG_REG(reg)	(ADS1220_CMD_WREG | ((reg) << 2))
+
+/* Configuration registers (Table 8-8) */
+#define ADS1220_REG_CONFIG0	0x00
+#define ADS1220_REG_CONFIG1	0x01
+#define ADS1220_REG_CONFIG2	0x02
+#define ADS1220_REG_CONFIG3	0x03
+#define ADS1220_MAX_REG		ADS1220_REG_CONFIG3
+
+/* CONFIG0 */
+#define ADS1220_CFG0_MUX	GENMASK(7, 4)
+#define ADS1220_CFG0_GAIN	GENMASK(3, 1)
+#define ADS1220_CFG0_PGA_BYPASS	BIT(0)
+
+/* CONFIG1 */
+#define ADS1220_CFG1_DR		GENMASK(7, 5)
+#define ADS1220_CFG1_MODE	GENMASK(4, 3)
+#define ADS1220_CFG1_CM		BIT(2)
+#define ADS1220_CFG1_TS		BIT(1)
+#define ADS1220_CFG1_BCS	BIT(0)
+
+/* CONFIG2 */
+#define ADS1220_CFG2_VREF	GENMASK(7, 6)
+#define ADS1220_CFG2_FILTER	GENMASK(5, 4)
+#define ADS1220_CFG2_PSW	BIT(3)
+#define ADS1220_CFG2_IDAC	GENMASK(2, 0)
+
+/* CONFIG3 */
+#define ADS1220_CFG3_I1MUX	GENMASK(7, 5)
+#define ADS1220_CFG3_I2MUX	GENMASK(4, 2)
+#define ADS1220_CFG3_DRDYM	BIT(1)
+
+/* VREF[1:0] sources */
+#define ADS1220_VREF_INTERNAL	0
+#define ADS1220_VREF_REFP0_REFN0 1
+#define ADS1220_VREF_AIN0_AIN3	2
+#define ADS1220_VREF_AVDD	3
+#define ADS1220_INTERNAL_VREF_uV 2048000
+
+/* Input multiplexer codes (Table 8-10) */
+#define ADS1220_MUX_SINGLE(ain)	(0x8 | (ain))	/* AINx vs AVSS */
+#define ADS1220_MUX_SHORTED	0x0e		/* (AVDD + AVSS) / 2 */
+
+#define ADS1220_DATA_BYTES	3
+#define ADS1220_DATA_BITS	24
+
+#define ADS1220_NUM_GAINS	8	/* 1, 2, 4, 8, 16, 32, 64, 128 */
+#define ADS1220_MAX_SE_GAIN	4	/* single-ended forces PGA bypass */
+
+#define ADS1220_MAX_CHANNELS	7	/* 4 single-ended + 3 differential-ish */
+#define ADS1220_MAX_AIN		4
+
+/* Worst-case single conversion: 20 SPS => 50 ms, plus margin. */
+#define ADS1220_CONV_TIMEOUT_MS	100
+#define ADS1220_CONV_MARGIN_US	2000
+
+#define ADS1220_SUSPEND_DELAY_MS 2000
+
+/* Data rate (samples per second) in normal mode, indexed by DR[2:0]. */
+static const int ads1220_datarates[] = {
+	20, 45, 90, 175, 330, 600, 1000,
+};
+
+/*
+ * Available scales expressed as gain reciprocals (val / val2), matching the
+ * convention used by the sibling ti-ads1119 driver: writing 0.25 selects a
+ * gain of 4. The full list is used for differential channels; single-ended
+ * channels (which force the PGA into bypass) are limited to the first three
+ * entries (gains 1, 2, 4).
+ */
+static const int ads1220_scale_avail[] = {
+	1, 1,
+	1, 2,
+	1, 4,
+	1, 8,
+	1, 16,
+	1, 32,
+	1, 64,
+	1, 128,
+};
+
+#define ADS1220_SE_SCALE_AVAIL_LEN	(3 * 2)
+#define ADS1220_SCALE_AVAIL_LEN		ARRAY_SIZE(ads1220_scale_avail)
+
+struct ads1220_channel_config {
+	unsigned int mux;
+	unsigned int gain;
+	unsigned int datarate;
+	bool single_ended;
+};
+
+struct ads1220_state {
+	struct spi_device *spi;
+	struct completion completion;
+	struct iio_trigger *trig;
+	struct ads1220_channel_config *channels_cfg;
+	unsigned int num_channels_cfg;
+	int vref_uV;
+	unsigned int vref_source;
+
+	/*
+	 * DMA-safe buffers. tx is used for command/register writes, rx for
+	 * register and conversion-result reads. scan holds one sample plus a
+	 * timestamp for the triggered buffer.
+	 */
+	u8 tx[2] __aligned(IIO_DMA_MINALIGN);
+	u8 rx[ADS1220_DATA_BYTES];
+	struct {
+		s32 sample;
+		aligned_s64 timestamp;
+	} scan;
+};
+
+static int ads1220_command(struct ads1220_state *st, u8 cmd)
+{
+	st->tx[0] = cmd;
+
+	return spi_write(st->spi, st->tx, 1);
+}
+
+static int ads1220_write_reg(struct ads1220_state *st, u8 reg, u8 val)
+{
+	st->tx[0] = ADS1220_CMD_WREG_REG(reg);
+	st->tx[1] = val;
+
+	return spi_write(st->spi, st->tx, 2);
+}
+
+static int ads1220_read_reg(struct ads1220_state *st, u8 reg, u8 *val)
+{
+	int ret;
+
+	st->tx[0] = ADS1220_CMD_RREG_REG(reg);
+
+	ret = spi_write_then_read(st->spi, st->tx, 1, st->rx, 1);
+	if (ret)
+		return ret;
+
+	*val = st->rx[0];
+
+	return 0;
+}
+
+static int ads1220_reset(struct ads1220_state *st)
+{
+	int ret;
+
+	ret = ads1220_command(st, ADS1220_CMD_RESET);
+	if (ret)
+		return ret;
+
+	/* Wait at least 50us + 32 x tCLK after RESET before any command. */
+	fsleep(100);
+
+	return 0;
+}
+
+static unsigned int ads1220_datarate_to_code(unsigned int datarate)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(ads1220_datarates); i++)
+		if (ads1220_datarates[i] == datarate)
+			return i;
+
+	return 0;
+}
+
+static int ads1220_configure(struct ads1220_state *st, unsigned int mux,
+			     unsigned int gain, unsigned int datarate,
+			     bool single_ended, bool continuous)
+{
+	u8 reg0, reg1;
+	int ret;
+
+	reg0 = FIELD_PREP(ADS1220_CFG0_MUX, mux) |
+	       FIELD_PREP(ADS1220_CFG0_GAIN, ilog2(gain));
+	/*
+	 * For single-ended inputs (AINN = AVSS) the PGA must be bypassed; the
+	 * datasheet only allows gains 1, 2 and 4 in that case.
+	 */
+	if (single_ended)
+		reg0 |= ADS1220_CFG0_PGA_BYPASS;
+
+	ret = ads1220_write_reg(st, ADS1220_REG_CONFIG0, reg0);
+	if (ret)
+		return ret;
+
+	reg1 = FIELD_PREP(ADS1220_CFG1_DR, ads1220_datarate_to_code(datarate));
+	if (continuous)
+		reg1 |= ADS1220_CFG1_CM;
+
+	return ads1220_write_reg(st, ADS1220_REG_CONFIG1, reg1);
+}
+
+static int ads1220_read_sample(struct ads1220_state *st, unsigned int datarate,
+			       int *val)
+{
+	int ret;
+
+	if (st->spi->irq) {
+		unsigned long timeout = msecs_to_jiffies(ADS1220_CONV_TIMEOUT_MS);
+
+		if (!wait_for_completion_timeout(&st->completion, timeout))
+			return -ETIMEDOUT;
+	} else {
+		/*
+		 * No DRDY interrupt: wait for the conversion to finish. In
+		 * single-shot mode the result stays latched until the next
+		 * START, so waiting longer than one conversion is harmless;
+		 * wait two periods plus a margin to comfortably cover the
+		 * oscillator start-up and its tolerance.
+		 */
+		fsleep(2 * DIV_ROUND_UP(MICRO, datarate) + ADS1220_CONV_MARGIN_US);
+	}
+
+	/*
+	 * Once DRDY is low the result can be clocked out directly, MSB first,
+	 * without an RDATA command (datasheet section 8.5.4).
+	 */
+	ret = spi_read(st->spi, st->rx, ADS1220_DATA_BYTES);
+	if (ret)
+		return ret;
+
+	*val = sign_extend32(get_unaligned_be24(st->rx), ADS1220_DATA_BITS - 1);
+
+	return 0;
+}
+
+static int ads1220_single_conversion(struct ads1220_state *st,
+				     const struct iio_chan_spec *chan,
+				     int *val, bool calib_offset)
+{
+	struct device *dev = &st->spi->dev;
+	struct ads1220_channel_config *cfg = &st->channels_cfg[chan->address];
+	unsigned int mux = cfg->mux;
+	bool single_ended = cfg->single_ended;
+	int ret;
+
+	if (calib_offset) {
+		mux = ADS1220_MUX_SHORTED;
+		single_ended = false;
+	}
+
+	ret = pm_runtime_resume_and_get(dev);
+	if (ret)
+		return ret;
+
+	ret = ads1220_configure(st, mux, cfg->gain, cfg->datarate,
+				single_ended, false);
+	if (ret)
+		goto out;
+
+	if (st->spi->irq)
+		reinit_completion(&st->completion);
+
+	ret = ads1220_command(st, ADS1220_CMD_START);
+	if (ret)
+		goto out;
+
+	ret = ads1220_read_sample(st, cfg->datarate, val);
+	if (ret)
+		goto out;
+
+	ret = IIO_VAL_INT;
+out:
+	pm_runtime_mark_last_busy(dev);
+	pm_runtime_put_autosuspend(dev);
+
+	return ret;
+}
+
+static int ads1220_read_raw(struct iio_dev *indio_dev,
+			    const struct iio_chan_spec *chan,
+			    int *val, int *val2, long mask)
+{
+	struct ads1220_state *st = iio_priv(indio_dev);
+	struct ads1220_channel_config *cfg = &st->channels_cfg[chan->address];
+	int ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		if (!iio_device_claim_direct(indio_dev))
+			return -EBUSY;
+		ret = ads1220_single_conversion(st, chan, val, false);
+		iio_device_release_direct(indio_dev);
+		return ret;
+	case IIO_CHAN_INFO_OFFSET:
+		if (!iio_device_claim_direct(indio_dev))
+			return -EBUSY;
+		ret = ads1220_single_conversion(st, chan, val, true);
+		iio_device_release_direct(indio_dev);
+		return ret;
+	case IIO_CHAN_INFO_SCALE:
+		/* scale [mV] = vref / (gain * 2^23); gain is a power of two. */
+		*val = st->vref_uV / MILLI;
+		*val2 = (chan->scan_type.realbits - 1) + ilog2(cfg->gain);
+		return IIO_VAL_FRACTIONAL_LOG2;
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		*val = cfg->datarate;
+		return IIO_VAL_INT;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int ads1220_read_avail(struct iio_dev *indio_dev,
+			      const struct iio_chan_spec *chan,
+			      const int **vals, int *type, int *length,
+			      long mask)
+{
+	struct ads1220_state *st = iio_priv(indio_dev);
+	struct ads1220_channel_config *cfg = &st->channels_cfg[chan->address];
+
+	switch (mask) {
+	case IIO_CHAN_INFO_SCALE:
+		*type = IIO_VAL_FRACTIONAL;
+		*vals = ads1220_scale_avail;
+		*length = cfg->single_ended ? ADS1220_SE_SCALE_AVAIL_LEN :
+					      ADS1220_SCALE_AVAIL_LEN;
+		return IIO_AVAIL_LIST;
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		*type = IIO_VAL_INT;
+		*vals = ads1220_datarates;
+		*length = ARRAY_SIZE(ads1220_datarates);
+		return IIO_AVAIL_LIST;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int ads1220_write_raw(struct iio_dev *indio_dev,
+			     const struct iio_chan_spec *chan,
+			     int val, int val2, long mask)
+{
+	struct ads1220_state *st = iio_priv(indio_dev);
+	struct ads1220_channel_config *cfg = &st->channels_cfg[chan->address];
+	unsigned int gain;
+	int i;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_SCALE:
+		/* The available scales are the gain reciprocals (e.g. 1/4). */
+		if (val == 0 && val2 == 0)
+			return -EINVAL;
+
+		gain = MICRO / (val * MICRO + val2);
+		if (!is_power_of_2(gain) || gain > BIT(ADS1220_NUM_GAINS - 1))
+			return -EINVAL;
+		if (cfg->single_ended && gain > ADS1220_MAX_SE_GAIN)
+			return -EINVAL;
+
+		cfg->gain = gain;
+		return 0;
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		for (i = 0; i < ARRAY_SIZE(ads1220_datarates); i++) {
+			if (ads1220_datarates[i] == val) {
+				cfg->datarate = val;
+				return 0;
+			}
+		}
+		return -EINVAL;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int ads1220_debugfs_reg_access(struct iio_dev *indio_dev,
+				      unsigned int reg, unsigned int writeval,
+				      unsigned int *readval)
+{
+	struct ads1220_state *st = iio_priv(indio_dev);
+	u8 val;
+	int ret;
+
+	if (reg > ADS1220_MAX_REG)
+		return -EINVAL;
+
+	if (readval) {
+		ret = ads1220_read_reg(st, reg, &val);
+		if (ret)
+			return ret;
+		*readval = val;
+		return 0;
+	}
+
+	return ads1220_write_reg(st, reg, writeval);
+}
+
+static const struct iio_info ads1220_info = {
+	.read_raw = ads1220_read_raw,
+	.read_avail = ads1220_read_avail,
+	.write_raw = ads1220_write_raw,
+	.debugfs_reg_access = ads1220_debugfs_reg_access,
+};
+
+static int ads1220_buffer_preenable(struct iio_dev *indio_dev)
+{
+	struct ads1220_state *st = iio_priv(indio_dev);
+	struct device *dev = &st->spi->dev;
+	struct ads1220_channel_config *cfg;
+	unsigned int index;
+	int ret;
+
+	index = find_first_bit(indio_dev->active_scan_mask,
+			       iio_get_masklength(indio_dev));
+	cfg = &st->channels_cfg[index];
+
+	ret = pm_runtime_resume_and_get(dev);
+	if (ret)
+		return ret;
+
+	ret = ads1220_configure(st, cfg->mux, cfg->gain, cfg->datarate,
+				cfg->single_ended, true);
+	if (ret)
+		goto err;
+
+	ret = ads1220_command(st, ADS1220_CMD_START);
+	if (ret)
+		goto err;
+
+	return 0;
+err:
+	pm_runtime_mark_last_busy(dev);
+	pm_runtime_put_autosuspend(dev);
+	return ret;
+}
+
+static int ads1220_buffer_postdisable(struct iio_dev *indio_dev)
+{
+	struct ads1220_state *st = iio_priv(indio_dev);
+	struct device *dev = &st->spi->dev;
+
+	pm_runtime_mark_last_busy(dev);
+	pm_runtime_put_autosuspend(dev);
+
+	return 0;
+}
+
+static const struct iio_buffer_setup_ops ads1220_buffer_setup_ops = {
+	.preenable = ads1220_buffer_preenable,
+	.postdisable = ads1220_buffer_postdisable,
+	.validate_scan_mask = &iio_validate_scan_mask_onehot,
+};
+
+static const struct iio_trigger_ops ads1220_trigger_ops = {
+	.validate_device = &iio_trigger_validate_own_device,
+};
+
+static irqreturn_t ads1220_irq_handler(int irq, void *dev_id)
+{
+	struct iio_dev *indio_dev = dev_id;
+	struct ads1220_state *st = iio_priv(indio_dev);
+
+	if (iio_buffer_enabled(indio_dev) && iio_trigger_using_own(indio_dev))
+		iio_trigger_poll(indio_dev->trig);
+	else
+		complete(&st->completion);
+
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t ads1220_trigger_handler(int irq, void *p)
+{
+	struct iio_poll_func *pf = p;
+	struct iio_dev *indio_dev = pf->indio_dev;
+	struct ads1220_state *st = iio_priv(indio_dev);
+	int ret;
+
+	ret = spi_read(st->spi, st->rx, ADS1220_DATA_BYTES);
+	if (ret) {
+		dev_err(&st->spi->dev, "Failed to read sample: %d\n", ret);
+		goto done;
+	}
+
+	st->scan.sample = sign_extend32(get_unaligned_be24(st->rx),
+					ADS1220_DATA_BITS - 1);
+
+	iio_push_to_buffers_with_ts(indio_dev, &st->scan, sizeof(st->scan),
+				    iio_get_time_ns(indio_dev));
+done:
+	iio_trigger_notify_done(indio_dev->trig);
+
+	return IRQ_HANDLED;
+}
+
+static int ads1220_map_mux(struct device *dev, u32 ain_pos, u32 ain_neg,
+			   bool differential, unsigned int *mux,
+			   bool *single_ended)
+{
+	static const u8 diff_mux[ADS1220_MAX_AIN][ADS1220_MAX_AIN] = {
+		[0][1] = 0x0, [0][2] = 0x1, [0][3] = 0x2,
+		[1][2] = 0x3, [1][3] = 0x4, [1][0] = 0x6,
+		[2][3] = 0x5,
+		[3][2] = 0x7,
+	};
+
+	if (!differential) {
+		if (ain_pos >= ADS1220_MAX_AIN)
+			return -EINVAL;
+		*mux = ADS1220_MUX_SINGLE(ain_pos);
+		*single_ended = true;
+		return 0;
+	}
+
+	if (ain_pos >= ADS1220_MAX_AIN || ain_neg >= ADS1220_MAX_AIN)
+		return -EINVAL;
+
+	/* Only the input pairs the multiplexer can route are valid. */
+	if (ain_pos == ain_neg || (diff_mux[ain_pos][ain_neg] == 0 &&
+				   !(ain_pos == 0 && ain_neg == 1)))
+		return -EINVAL;
+
+	*mux = diff_mux[ain_pos][ain_neg];
+	*single_ended = false;
+
+	return 0;
+}
+
+static int ads1220_alloc_channels(struct iio_dev *indio_dev)
+{
+	const struct iio_chan_spec ads1220_channel = {
+		.type = IIO_VOLTAGE,
+		.indexed = 1,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+				      BIT(IIO_CHAN_INFO_SCALE) |
+				      BIT(IIO_CHAN_INFO_OFFSET) |
+				      BIT(IIO_CHAN_INFO_SAMP_FREQ),
+		.info_mask_separate_available = BIT(IIO_CHAN_INFO_SCALE) |
+						BIT(IIO_CHAN_INFO_SAMP_FREQ),
+		.scan_type = {
+			.sign = 's',
+			.realbits = ADS1220_DATA_BITS,
+			.storagebits = 32,
+			.endianness = IIO_CPU,
+		},
+	};
+	const struct iio_chan_spec ads1220_ts = IIO_CHAN_SOFT_TIMESTAMP(0);
+	struct ads1220_state *st = iio_priv(indio_dev);
+	struct device *dev = &st->spi->dev;
+	struct iio_chan_spec *channels, *chan;
+	unsigned int num_channels, i = 0;
+	int ret;
+
+	st->num_channels_cfg = device_get_child_node_count(dev);
+	if (st->num_channels_cfg == 0 ||
+	    st->num_channels_cfg > ADS1220_MAX_CHANNELS)
+		return dev_err_probe(dev, -EINVAL,
+				     "Invalid channel count %u (max %u)\n",
+				     st->num_channels_cfg, ADS1220_MAX_CHANNELS);
+
+	st->channels_cfg = devm_kcalloc(dev, st->num_channels_cfg,
+					sizeof(*st->channels_cfg), GFP_KERNEL);
+	if (!st->channels_cfg)
+		return -ENOMEM;
+
+	/* One extra channel for the timestamp. */
+	num_channels = st->num_channels_cfg + 1;
+	channels = devm_kcalloc(dev, num_channels, sizeof(*channels),
+				GFP_KERNEL);
+	if (!channels)
+		return -ENOMEM;
+
+	device_for_each_child_node_scoped(dev, child) {
+		struct ads1220_channel_config *cfg = &st->channels_cfg[i];
+		bool differential;
+		u32 ain[2];
+
+		differential = fwnode_property_present(child, "diff-channels");
+		if (differential)
+			ret = fwnode_property_read_u32_array(child,
+							     "diff-channels",
+							     ain, 2);
+		else
+			ret = fwnode_property_read_u32(child, "single-channel",
+						       &ain[0]);
+		if (ret)
+			return dev_err_probe(dev, ret,
+					     "Failed to read channel property\n");
+
+		ret = ads1220_map_mux(dev, ain[0], ain[1], differential,
+				      &cfg->mux, &cfg->single_ended);
+		if (ret)
+			return dev_err_probe(dev, ret,
+					     "Invalid input combination\n");
+
+		cfg->gain = 1;
+		cfg->datarate = ads1220_datarates[0];
+
+		chan = &channels[i];
+		*chan = ads1220_channel;
+		chan->channel = ain[0];
+		chan->address = i;
+		chan->scan_index = i;
+		if (differential) {
+			chan->channel2 = ain[1];
+			chan->differential = 1;
+		}
+
+		i++;
+	}
+
+	channels[i] = ads1220_ts;
+	channels[i].scan_index = i;
+
+	indio_dev->channels = channels;
+	indio_dev->num_channels = num_channels;
+
+	return 0;
+}
+
+static int ads1220_init(struct ads1220_state *st)
+{
+	u8 reg2;
+	int ret;
+
+	ret = ads1220_reset(st);
+	if (ret)
+		return ret;
+
+	reg2 = FIELD_PREP(ADS1220_CFG2_VREF, st->vref_source);
+
+	ret = ads1220_write_reg(st, ADS1220_REG_CONFIG2, reg2);
+	if (ret)
+		return ret;
+
+	/* DRDY only on the dedicated pin (DRDYM = 0). */
+	return ads1220_write_reg(st, ADS1220_REG_CONFIG3, 0);
+}
+
+static void ads1220_powerdown(void *data)
+{
+	struct ads1220_state *st = data;
+
+	ads1220_command(st, ADS1220_CMD_POWERDOWN);
+}
+
+static int ads1220_probe(struct spi_device *spi)
+{
+	struct device *dev = &spi->dev;
+	struct iio_dev *indio_dev;
+	struct ads1220_state *st;
+	int avdd_uV;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	st = iio_priv(indio_dev);
+	st->spi = spi;
+	spi_set_drvdata(spi, indio_dev);
+
+	/* The ADS1220 uses SPI mode 1 (CPOL = 0, CPHA = 1). */
+	spi->mode |= SPI_CPHA;
+	spi->bits_per_word = 8;
+	ret = spi_setup(spi);
+	if (ret)
+		return dev_err_probe(dev, ret, "SPI setup failed\n");
+
+	indio_dev->name = "ads1220";
+	indio_dev->info = &ads1220_info;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+
+	ret = devm_regulator_get_enable(dev, "dvdd");
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to enable dvdd\n");
+
+	avdd_uV = devm_regulator_get_enable_read_voltage(dev, "avdd");
+	if (avdd_uV < 0)
+		return dev_err_probe(dev, avdd_uV, "Failed to get avdd\n");
+
+	/*
+	 * Reference source, in priority order:
+	 *  - external reference on REFP0/REFN0 if a "vref" regulator is given;
+	 *  - the analog supply (AVDD) for ratiometric single-supply setups if
+	 *    "ti,vref-avdd" is set - no extra pins, full 0..AVDD input range;
+	 *  - otherwise the internal 2.048V reference.
+	 */
+	st->vref_uV = devm_regulator_get_enable_read_voltage(dev, "vref");
+	if (st->vref_uV >= 0) {
+		st->vref_source = ADS1220_VREF_REFP0_REFN0;
+	} else if (st->vref_uV != -ENODEV) {
+		return dev_err_probe(dev, st->vref_uV, "Failed to get vref\n");
+	} else if (device_property_read_bool(dev, "ti,vref-avdd")) {
+		st->vref_source = ADS1220_VREF_AVDD;
+		st->vref_uV = avdd_uV;
+	} else {
+		st->vref_source = ADS1220_VREF_INTERNAL;
+		st->vref_uV = ADS1220_INTERNAL_VREF_uV;
+	}
+
+	ret = ads1220_alloc_channels(indio_dev);
+	if (ret)
+		return ret;
+
+	init_completion(&st->completion);
+
+	ret = devm_iio_triggered_buffer_setup(dev, indio_dev, NULL,
+					      ads1220_trigger_handler,
+					      &ads1220_buffer_setup_ops);
+	if (ret)
+		return dev_err_probe(dev, ret,
+				     "Failed to set up IIO buffer\n");
+
+	if (spi->irq > 0) {
+		ret = devm_request_irq(dev, spi->irq, ads1220_irq_handler,
+				       IRQF_NO_THREAD, "ads1220", indio_dev);
+		if (ret)
+			return dev_err_probe(dev, ret,
+					     "Failed to request irq\n");
+
+		st->trig = devm_iio_trigger_alloc(dev, "%s-dev%d",
+						  indio_dev->name,
+						  iio_device_id(indio_dev));
+		if (!st->trig)
+			return -ENOMEM;
+
+		st->trig->ops = &ads1220_trigger_ops;
+		iio_trigger_set_drvdata(st->trig, indio_dev);
+
+		ret = devm_iio_trigger_register(dev, st->trig);
+		if (ret)
+			return dev_err_probe(dev, ret,
+					     "Failed to register trigger\n");
+	}
+
+	ret = ads1220_init(st);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to initialize device\n");
+
+	pm_runtime_set_autosuspend_delay(dev, ADS1220_SUSPEND_DELAY_MS);
+	pm_runtime_use_autosuspend(dev);
+	pm_runtime_set_active(dev);
+
+	ret = devm_pm_runtime_enable(dev);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to enable pm runtime\n");
+
+	ret = devm_add_action_or_reset(dev, ads1220_powerdown, st);
+	if (ret)
+		return ret;
+
+	return devm_iio_device_register(dev, indio_dev);
+}
+
+static int ads1220_runtime_suspend(struct device *dev)
+{
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
+	struct ads1220_state *st = iio_priv(indio_dev);
+
+	return ads1220_command(st, ADS1220_CMD_POWERDOWN);
+}
+
+static int ads1220_runtime_resume(struct device *dev)
+{
+	/*
+	 * A START/SYNC command wakes the analog parts from power-down; it is
+	 * issued by the conversion path, so there is nothing to do here beyond
+	 * letting the device settle after the supplies are active again.
+	 */
+	fsleep(100);
+
+	return 0;
+}
+
+static DEFINE_RUNTIME_DEV_PM_OPS(ads1220_pm_ops, ads1220_runtime_suspend,
+				 ads1220_runtime_resume, NULL);
+
+static const struct spi_device_id ads1220_id[] = {
+	{ "ads1220" },
+	{ }
+};
+MODULE_DEVICE_TABLE(spi, ads1220_id);
+
+static const struct of_device_id ads1220_of_match[] = {
+	{ .compatible = "ti,ads1220" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, ads1220_of_match);
+
+static struct spi_driver ads1220_driver = {
+	.driver = {
+		.name = "ads1220",
+		.of_match_table = ads1220_of_match,
+		.pm = pm_ptr(&ads1220_pm_ops),
+	},
+	.probe = ads1220_probe,
+	.id_table = ads1220_id,
+};
+module_spi_driver(ads1220_driver);
+
+MODULE_DESCRIPTION("Texas Instruments ADS1220 ADC Driver");
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Nguyen Minh Tien <zizuzacker@gmail.com>");
-- 
2.34.1

Re: [PATCH 2/2] iio: adc: Add TI ADS1220 driver

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 13 potential issue(s) to consider:
- [Critical] An integer overflow when calculating scale gain leads to a divide-by-zero kernel panic on 32-bit platforms.
- [High] Inconsistent formatting for IIO_CHAN_INFO_SCALE breaks IIO UAPI contracts. read_raw returns the true scale multiplier, but write_raw expects a gain reciprocal, and read_avail exposes gain reciprocals.
- [High] Using iio_validate_scan_mask_onehot breaks timestamp functionality and allows an out-of-bounds array read in buffer_preenable.
- [High] Missing `.validate_trigger` allows external triggers to read asynchronously without the RDATA command, causing data corruption.
- [High] The `rx` buffer is not DMA-aligned, violating DMA API requirements and risking memory corruption during SPI transfers.
- [High] Disabling the triggered buffer leaves the ADC in continuous conversion mode, causing DRDY interrupt storms.
- [High] Missing mutex lock protection for the shared `st->tx` and `st->rx` buffers.
- [High] Using IRQF_NO_THREAD for a potentially level-triggered interrupt without clearing the interrupt in hardware causes an interrupt storm.
- [Medium] ads1220_write_raw modifies channel configuration without claiming direct mode, causing races with active buffers and single-shot conversions.
- [Medium] IIO_CHAN_INFO_OFFSET returns the positive uncalibrated raw value instead of its negated value, causing the offset error to be doubled in user-space.
- [Medium] Concurrent SPI writes can occur during module removal due to incorrect `devm` cleanup order.
- [Medium] ads1220_debugfs_reg_access communicates with the device without resuming it from power-down.
- [Medium] The driver unconditionally fails probe if the AVDD regulator voltage cannot be read, breaking support for dummy regulators.
--

commit 1ae738a6247e14166f63bc753a84967fb3d54773
Author: Nguyen Minh Tien <zizuzacker@gmail.com>

iio: adc: Add TI ADS1220 driver

Add an IIO driver for the Texas Instruments ADS1220 24-bit delta-sigma
SPI ADC. The driver supports single-ended and differential voltage
channels described as device-tree child nodes, per-channel programmable
gain (exposed through scale) and data rate, internal and external reference
options, single-shot conversions and a DRDY-interrupt-driven triggered buffer.

diff --git a/drivers/iio/adc/ti-ads1220.c b/drivers/iio/adc/ti-ads1220.c
new file mode 100644
index 0000000000000..06bcc88418b4c
--- /dev/null
+++ b/drivers/iio/adc/ti-ads1220.c
[ ... ]
> +struct ads1220_state {
> +	struct spi_device *spi;
> +	struct completion completion;
> +	struct iio_trigger *trig;
> +	struct ads1220_channel_config *channels_cfg;
> +	unsigned int num_channels_cfg;
> +	int vref_uV;
> +	unsigned int vref_source;
> +
> +	/*
> +	 * DMA-safe buffers. tx is used for command/register writes, rx for
> +	 * register and conversion-result reads. scan holds one sample plus a
> +	 * timestamp for the triggered buffer.
> +	 */
> +	u8 tx[2] __aligned(IIO_DMA_MINALIGN);
> +	u8 rx[ADS1220_DATA_BYTES];

[Severity: High]
Does placing the rx array immediately after the 2-byte tx array without
alignment cause it to share the same cacheline? If st->rx is passed to
spi_read(), mapping this unaligned sub-cacheline buffer for DMA might
corrupt adjacent fields on architectures without hardware cache coherence.

> +	struct {
> +		s32 sample;
> +		aligned_s64 timestamp;
> +	} scan;
> +};
[ ... ]
> +static int ads1220_read_raw(struct iio_dev *indio_dev,
> +			    const struct iio_chan_spec *chan,
> +			    int *val, int *val2, long mask)
> +{
> +	struct ads1220_state *st = iio_priv(indio_dev);
> +	struct ads1220_channel_config *cfg = &st->channels_cfg[chan->address];
> +	int ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		if (!iio_device_claim_direct(indio_dev))
> +			return -EBUSY;
> +		ret = ads1220_single_conversion(st, chan, val, false);
> +		iio_device_release_direct(indio_dev);
> +		return ret;
> +	case IIO_CHAN_INFO_OFFSET:
> +		if (!iio_device_claim_direct(indio_dev))
> +			return -EBUSY;
> +		ret = ads1220_single_conversion(st, chan, val, true);

[Severity: Medium]
Should the hardware offset be negated before returning it here?
ads1220_single_conversion() returns the raw measurement of the shorted inputs.
Since the standard IIO calibration formula applied by user-space is
`value = (raw + offset) * scale`, returning a positive offset when the hardware
reads a positive value will apply the offset in the wrong direction and double
the error.

> +		iio_device_release_direct(indio_dev);
> +		return ret;
> +	case IIO_CHAN_INFO_SCALE:
> +		/* scale [mV] = vref / (gain * 2^23); gain is a power of two. */
> +		*val = st->vref_uV / MILLI;
> +		*val2 = (chan->scan_type.realbits - 1) + ilog2(cfg->gain);
> +		return IIO_VAL_FRACTIONAL_LOG2;

[Severity: High]
Are the IIO_CHAN_INFO_SCALE interfaces handling values symmetrically?
ads1220_read_raw() returns the actual physical scale multiplier here, but
ads1220_write_raw() treats the user input as a gain reciprocal, and
ads1220_read_avail() exposes gain reciprocals instead of voltage scales.
If a user reads the scale and writes the exact same value back, could this
evaluate to an invalid gain and return -EINVAL?

> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		*val = cfg->datarate;
> +		return IIO_VAL_INT;
> +	default:
> +		return -EINVAL;
> +	}
> +}
[ ... ]
> +static int ads1220_write_raw(struct iio_dev *indio_dev,
> +			     const struct iio_chan_spec *chan,
> +			     int val, int val2, long mask)
> +{
> +	struct ads1220_state *st = iio_priv(indio_dev);
> +	struct ads1220_channel_config *cfg = &st->channels_cfg[chan->address];
> +	unsigned int gain;
> +	int i;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_SCALE:
> +		/* The available scales are the gain reciprocals (e.g. 1/4). */
> +		if (val == 0 && val2 == 0)
> +			return -EINVAL;
> +
> +		gain = MICRO / (val * MICRO + val2);

[Severity: Critical]
Can the expression `(val * MICRO + val2)` overflow on 32-bit platforms if a
user writes a crafted string (e.g., "4294.967296") to the sysfs scale
attribute? If the multiplication wraps to exactly zero, does the subsequent
`gain = MICRO / ...` division result in a divide-by-zero kernel panic?

> +		if (!is_power_of_2(gain) || gain > BIT(ADS1220_NUM_GAINS - 1))
> +			return -EINVAL;
> +		if (cfg->single_ended && gain > ADS1220_MAX_SE_GAIN)
> +			return -EINVAL;
> +
> +		cfg->gain = gain;
> +		return 0;
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		for (i = 0; i < ARRAY_SIZE(ads1220_datarates); i++) {
> +			if (ads1220_datarates[i] == val) {
> +				cfg->datarate = val;

[Severity: Medium]
Does modifying cfg->gain and cfg->datarate in ads1220_write_raw() require
serialization via iio_device_claim_direct()? Modifying these without the lock
might race with an active triggered buffer or single-shot conversions,
potentially desynchronizing driver state and hardware or corrupting timeout
calculations.

> +				return 0;
> +			}
> +		}
> +		return -EINVAL;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int ads1220_debugfs_reg_access(struct iio_dev *indio_dev,
> +				      unsigned int reg, unsigned int writeval,
> +				      unsigned int *readval)
> +{
> +	struct ads1220_state *st = iio_priv(indio_dev);
> +	u8 val;
> +	int ret;
> +
> +	if (reg > ADS1220_MAX_REG)
> +		return -EINVAL;
> +
> +	if (readval) {
> +		ret = ads1220_read_reg(st, reg, &val);

[Severity: Medium]
Does ads1220_debugfs_reg_access() need to call pm_runtime_resume_and_get()
before initiating SPI transfers? If the device is in the autosuspended
POWERDOWN state, these transactions might fail or return garbage data.

> +		if (ret)
> +			return ret;
> +		*readval = val;
> +		return 0;
> +	}
> +
> +	return ads1220_write_reg(st, reg, writeval);

[Severity: High]
Are the shared st->tx and st->rx buffers protected from concurrent access
here? Writing to st->tx directly via debugfs or pm_runtime_suspend without
locking could race with an active triggered buffer reading in
ads1220_trigger_handler(), potentially overwriting the tx buffer mid-transfer
and sending corrupted SPI commands.

> +}
> +
> +static const struct iio_info ads1220_info = {
> +	.read_raw = ads1220_read_raw,
> +	.read_avail = ads1220_read_avail,
> +	.write_raw = ads1220_write_raw,
> +	.debugfs_reg_access = ads1220_debugfs_reg_access,
> +};

[Severity: High]
Does this struct need a .validate_trigger callback to prevent arbitrary
external triggers from being attached? Without it, external triggers could
execute asynchronously to the ADC's data-ready signal. The trigger handler
uses spi_read() directly without an RDATA command, which according to the
datasheet could return corrupted data if a register update overlaps.

> +
> +static int ads1220_buffer_preenable(struct iio_dev *indio_dev)
> +{
> +	struct ads1220_state *st = iio_priv(indio_dev);
> +	struct device *dev = &st->spi->dev;
> +	struct ads1220_channel_config *cfg;
> +	unsigned int index;
> +	int ret;
> +
> +	index = find_first_bit(indio_dev->active_scan_mask,
> +			       iio_get_masklength(indio_dev));
> +	cfg = &st->channels_cfg[index];

[Severity: High]
What happens if a user enables ONLY the timestamp channel? The active scan
mask would have exactly one bit set at index = st->num_channels_cfg. Will
find_first_bit() return this index and cause an out-of-bounds array read
on st->channels_cfg[index]?

> +
> +	ret = pm_runtime_resume_and_get(dev);
> +	if (ret)
> +		return ret;
> +
> +	ret = ads1220_configure(st, cfg->mux, cfg->gain, cfg->datarate,
> +				cfg->single_ended, true);
> +	if (ret)
> +		goto err;
> +
> +	ret = ads1220_command(st, ADS1220_CMD_START);
> +	if (ret)
> +		goto err;
> +
> +	return 0;
> +err:
> +	pm_runtime_mark_last_busy(dev);
> +	pm_runtime_put_autosuspend(dev);
> +	return ret;
> +}
> +
> +static int ads1220_buffer_postdisable(struct iio_dev *indio_dev)
> +{
> +	struct ads1220_state *st = iio_priv(indio_dev);
> +	struct device *dev = &st->spi->dev;
> +
> +	pm_runtime_mark_last_busy(dev);
> +	pm_runtime_put_autosuspend(dev);

[Severity: High]
Does the device need to be explicitly stopped or powered down here?
Dropping the PM runtime reference with a 2000ms autosuspend delay leaves
the ADC running in continuous conversion mode. The hardware might keep
asserting the DRDY interrupt, but since the buffer is disabled, the IRQ
handler will only complete the completion without reading data to clear
the interrupt, potentially causing an immediate IRQ storm.

> +
> +	return 0;
> +}
> +
> +static const struct iio_buffer_setup_ops ads1220_buffer_setup_ops = {
> +	.preenable = ads1220_buffer_preenable,
> +	.postdisable = ads1220_buffer_postdisable,
> +	.validate_scan_mask = &iio_validate_scan_mask_onehot,
> +};

[Severity: High]
Is iio_validate_scan_mask_onehot the correct validator for this device?
This validator requires exactly one bit in the mask to be set. Since the
IIO core includes the timestamp channel in the active_scan_mask, enabling
one data channel plus the timestamp results in 2 bits being set, which
will cause the validator to reject valid configurations.

[ ... ]
> +static int ads1220_probe(struct spi_device *spi)
> +{
> +	struct device *dev = &spi->dev;
> +	struct iio_dev *indio_dev;
> +	struct ads1220_state *st;
> +	int avdd_uV;
> +	int ret;
[ ... ]
> +	ret = devm_regulator_get_enable(dev, "dvdd");
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to enable dvdd\n");
> +
> +	avdd_uV = devm_regulator_get_enable_read_voltage(dev, "avdd");
> +	if (avdd_uV < 0)
> +		return dev_err_probe(dev, avdd_uV, "Failed to get avdd\n");

[Severity: Medium]
Should the driver fail probe unconditionally here? If the devicetree does
not explicitly model the AVDD supply, the core passes a dummy regulator.
Reading the voltage from a dummy regulator returns -EINVAL, which might
prevent the driver from probing entirely on such systems, even when
ti,vref-avdd is not specified.

[ ... ]
> +	if (spi->irq > 0) {
> +		ret = devm_request_irq(dev, spi->irq, ads1220_irq_handler,
> +				       IRQF_NO_THREAD, "ads1220", indio_dev);

[Severity: High]
Can using IRQF_NO_THREAD without clearing the physical DRDY line cause an
interrupt storm? The hardirq handler immediately returns IRQ_HANDLED
without issuing an SPI transaction to read the data. If the interrupt is
configured as level-triggered, the interrupt controller might repeatedly
re-fire the interrupt upon return. Does this need IRQF_ONESHOT with a
threaded handler, or an explicit enforcement of IRQF_TRIGGER_FALLING?

> +		if (ret)
> +			return dev_err_probe(dev, ret,
> +					     "Failed to request irq\n");
> +
> +		st->trig = devm_iio_trigger_alloc(dev, "%s-dev%d",
> +						  indio_dev->name,
> +						  iio_device_id(indio_dev));
> +		if (!st->trig)
> +			return -ENOMEM;
> +
> +		st->trig->ops = &ads1220_trigger_ops;
> +		iio_trigger_set_drvdata(st->trig, indio_dev);
> +
> +		ret = devm_iio_trigger_register(dev, st->trig);
> +		if (ret)
> +			return dev_err_probe(dev, ret,
> +					     "Failed to register trigger\n");
> +	}
> +
> +	ret = ads1220_init(st);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to initialize device\n");
> +
> +	pm_runtime_set_autosuspend_delay(dev, ADS1220_SUSPEND_DELAY_MS);
> +	pm_runtime_use_autosuspend(dev);
> +	pm_runtime_set_active(dev);
> +
> +	ret = devm_pm_runtime_enable(dev);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to enable pm runtime\n");
> +
> +	ret = devm_add_action_or_reset(dev, ads1220_powerdown, st);

[Severity: Medium]
Will registering ads1220_powerdown after pm_runtime_enable cause a teardown
ordering issue? Since devm executes cleanups in reverse order, powerdown
would execute before PM runtime is disabled. Could the PM autosuspend
workqueue trigger ads1220_runtime_suspend at the same moment powerdown
is executing, causing a data race on the unprotected st->tx buffer?

> +	if (ret)
> +		return ret;
> +
> +	return devm_iio_device_register(dev, indio_dev);
> +}
> +
> +static int ads1220_runtime_suspend(struct device *dev)
> +{
> +	struct iio_dev *indio_dev = dev_get_drvdata(dev);
> +	struct ads1220_state *st = iio_priv(indio_dev);
> +
> +	return ads1220_command(st, ADS1220_CMD_POWERDOWN);
> +}

[Severity: High]
Does ads1220_runtime_suspend() overwrite the shared st->tx buffer without
locking? This asynchronous PM runtime suspend could execute concurrently
with an active triggered buffer reading or other SPI transactions, corrupting
the sent SPI commands.

> +
> +static int ads1220_runtime_resume(struct device *dev)
> +{

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260610151342.44274-1-zizuzacker@gmail.com?part=2