[PATCH v2 0/2] iio: adc: ti-ads1120: Add driver and dt-binding

[PATCH v2 0/2] iio: adc: ti-ads1120: Add driver and dt-binding

[Ajith Anandhan]

This patch adds support for the Texas Instruments ADS1120, a precision 
16-bit delta-sigma ADC with SPI interface.

Why a new driver?
=================
The ADS1120 requires a dedicated driver rather than extending existing 
TI ADC drivers for the following reasons:

ads124s08.c - TI ADS124S08
 
 - This is the closest match (both are delta-sigma, SPI-based)
 
 - However, significant differences exist:
 
    * Different register layout (ADS124S08 has more registers)
 
    * Different command set ADS124S08 has built-in MUX for differential
inputs
     * Different register addressing and bit fields and conversion 
timing and data retrieval.
 
would require extensive conditional code paths that might reduce
maintainability for both devices. A separate, focused driver 
seemed cleaner.

Changes in v2
=============
Driver improvements:
- Fixed all register bit definitions to use field values with FIELD_PREP/FIELD_GET
- Moved DMA buffer to end of struct with proper alignment
- Added comprehensive comments for mutex protection
- Used regmap framework for register access
- Added all 15 channels (differential, single-ended, diagnostic)
- Used guard(mutex) and cleanup.h for lock management
- Used devm_mutex_init() for proper resource management
- Fixed error handling with dev_err_probe() throughout
- Removed redundant state variables (gain, datarate cached in regmap)
- Used sign_extend32() and get_unaligned_be16() for data parsing
- Split SPI transfers for cleaner command/data separation
- Added GPL-2.0-only license identifier

Device tree binding improvements:
- Added required avdd-supply property
- Added optional vref-supply for external reference
- Added interrupts and interrupt-names for DRDY pins
- Added optional clocks property for external clock
- Added ti,avdd-is-ref boolean flag for AVDD reference selection
- Added complete example with all optional properties
- Added datasheet link in binding description

Changes in v1 (RFC)
===================
- Initial RFC submission

Ajith Anandhan (2):
  dt-bindings: iio: adc: Add TI ADS1120 binding
  iio: adc: Add support for TI ADS1120

 .../bindings/iio/adc/ti,ads1120.yaml          | 110 +++
 MAINTAINERS                                   |   7 +
 drivers/iio/adc/Kconfig                       |  10 +
 drivers/iio/adc/Makefile                      |   1 +
 drivers/iio/adc/ti-ads1120.c                  | 631 ++++++++++++++++++
 5 files changed, 759 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/ti,ads1120.yaml
 create mode 100644 drivers/iio/adc/ti-ads1120.c

-- 
2.34.1

[PATCH v2 1/2] dt-bindings: iio: adc: Add TI ADS1120 binding

[Ajith Anandhan]

Add device tree binding documentation for the Texas Instruments
ADS1120.

The binding defines required properties like compatible, reg, and
SPI configuration parameters.

Signed-off-by: Ajith Anandhan <ajithanandhan0406@gmail.com>
---
 .../bindings/iio/adc/ti,ads1120.yaml          | 109 ++++++++++++++++++
 1 file changed, 109 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/ti,ads1120.yaml

diff --git a/Documentation/devicetree/bindings/iio/adc/ti,ads1120.yaml b/Documentation/devicetree/bindings/iio/adc/ti,ads1120.yaml
new file mode 100644
index 000000000..2449094af
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/ti,ads1120.yaml
@@ -0,0 +1,109 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/adc/ti,ads1120.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Texas Instruments ADS1120 4-channel, 16-bit, 2kSPS ADC
+
+maintainers:
+  - Ajith Anandhan <ajithanandhan0406@gmail.com>
+
+description: |
+  The ADS1120 is a precision, 16-bit, analog-to-digital converter (ADC)
+  that features two differential or four single-ended inputs through a
+  flexible input multiplexer.
+
+  Datasheet: https://www.ti.com/lit/gpn/ads1120
+
+properties:
+  compatible:
+    const: ti,ads1120
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    minItems: 1
+    maxItems: 2
+    description: |
+      Interrupts for the DRDY (data ready) pin(s). The device can output
+      DRDY on a dedicated pin or multiplex it with DOUT. If both pins are
+      wired, both interrupts can be specified.
+
+  interrupt-names:
+    minItems: 1
+    maxItems: 2
+    items:
+      enum:
+        - drdy
+        - dout
+
+  avdd-supply:
+    description: |
+      Analog power supply, typically 2.3V to 5.5V.
+
+  vref-supply:
+    description: |
+      Optional external voltage reference. Can be connected to either
+      REFP0/REFN0 or REFP1/REFN1 pins. If not supplied, the internal
+      2.048V reference is used.
+
+  ti,avdd-is-ref:
+    type: boolean
+    description: |
+      If present, indicates that the AVDD supply voltage is of sufficient
+      quality and stability to be used as the voltage reference instead of
+      the internal reference. This allows the driver to select AVDD as the
+      reference source for potentially better performance.
+
+  clocks:
+    maxItems: 1
+    description: |
+      Optional external clock input. If not specified, the internal
+      oscillator is used.
+
+  spi-max-frequency:
+    maximum: 4000000
+
+  spi-cpha: true
+
+  "#io-channel-cells":
+    const: 1
+
+required:
+  - compatible
+  - reg
+
+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,ads1120";
+            reg = <0>;
+            spi-max-frequency = <4000000>;
+            spi-cpha;
+
+            interrupts-extended = <&gpio1 25 IRQ_TYPE_EDGE_FALLING>;
+            interrupt-names = "drdy";
+
+            avdd-supply = <&reg_3v3>;
+            vref-supply = <&reg_vref_2v5>;
+
+            clocks = <&clk_4mhz>;
+
+            ti,avdd-is-ref;
+
+            #io-channel-cells = <1>;
+        };
+    };
+...
-- 
2.34.1

Re: [PATCH v2 1/2] dt-bindings: iio: adc: Add TI ADS1120 binding

[Krzysztof Kozlowski]

On Sun, Nov 09, 2025 at 07:41:18PM +0530, Ajith Anandhan wrote:
> Add device tree binding documentation for the Texas Instruments
> ADS1120.

A nit, subject: drop second/last, redundant "binding". The
"dt-bindings" prefix is already stating that these are bindings.
See also:
https://elixir.bootlin.com/linux/v6.17-rc3/source/Documentation/devicetree/bindings/submitting-patches.rst#L18

> 
> The binding defines required properties like compatible, reg, and
> SPI configuration parameters.

Drop sentence, completely redundant. We can read the diff.

> 
> Signed-off-by: Ajith Anandhan <ajithanandhan0406@gmail.com>
> ---
>  .../bindings/iio/adc/ti,ads1120.yaml          | 109 ++++++++++++++++++
>  1 file changed, 109 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/ti,ads1120.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/ti,ads1120.yaml b/Documentation/devicetree/bindings/iio/adc/ti,ads1120.yaml
> new file mode 100644
> index 000000000..2449094af
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/ti,ads1120.yaml
> @@ -0,0 +1,109 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/adc/ti,ads1120.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Texas Instruments ADS1120 4-channel, 16-bit, 2kSPS ADC
> +
> +maintainers:
> +  - Ajith Anandhan <ajithanandhan0406@gmail.com>
> +
> +description: |
> +  The ADS1120 is a precision, 16-bit, analog-to-digital converter (ADC)
> +  that features two differential or four single-ended inputs through a
> +  flexible input multiplexer.
> +
> +  Datasheet: https://www.ti.com/lit/gpn/ads1120
> +
> +properties:
> +  compatible:
> +    const: ti,ads1120
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    minItems: 1
> +    maxItems: 2
> +    description: |

Do not need '|' unless you need to preserve formatting.

> +      Interrupts for the DRDY (data ready) pin(s). The device can output
> +      DRDY on a dedicated pin or multiplex it with DOUT. If both pins are
> +      wired, both interrupts can be specified.
> +
> +  interrupt-names:
> +    minItems: 1
> +    maxItems: 2
> +    items:
> +      enum:
> +        - drdy
> +        - dout

No, this cannot be flexible so much. Look at existing examples how this
is supposed to be written.

minItems: 1
items:
 - enum
 - const

> +
> +  avdd-supply:
> +    description: |


Do not need '|' unless you need to preserve formatting.


> +      Analog power supply, typically 2.3V to 5.5V.
> +
> +  vref-supply:
> +    description: |

Do not need '|' unless you need to preserve formatting.

> +      Optional external voltage reference. Can be connected to either
> +      REFP0/REFN0 or REFP1/REFN1 pins. If not supplied, the internal
> +      2.048V reference is used.
> +
> +  ti,avdd-is-ref:
> +    type: boolean
> +    description: |

Do not need '|' unless you need to preserve formatting.

> +      If present, indicates that the AVDD supply voltage is of sufficient
> +      quality and stability to be used as the voltage reference instead of
> +      the internal reference. This allows the driver to select AVDD as the
> +      reference source for potentially better performance.

Driver? Aren't you just describing the case when AVDD is connected to
REF pins?

> +
> +  clocks:
> +    maxItems: 1
> +    description: |

Do not need '|' unless you need to preserve formatting.

> +      Optional external clock input. If not specified, the internal

Drop first sentence, completely redundant. Schema defines what is
optional, not your description. Can it be something else than external
clock input? No, it cannot. Please write informative descriptions, not
just inflate the text.

> +      oscillator is used.
> +
> +  spi-max-frequency:
> +    maximum: 4000000
> +
> +  spi-cpha: true
> +
> +  "#io-channel-cells":
> +    const: 1
> +
> +required:
> +  - compatible
> +  - reg

avdd-suply

> +
> +allOf:
> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +
> +unevaluatedProperties: false

Best regards,
Krzysztof

Re: [PATCH v2 1/2] dt-bindings: iio: adc: Add TI ADS1120 binding

[Jonathan Cameron]

On Sun,  9 Nov 2025 19:41:18 +0530
Ajith Anandhan <ajithanandhan0406@gmail.com> wrote:

> Add device tree binding documentation for the Texas Instruments
> ADS1120.
> 
> The binding defines required properties like compatible, reg, and
> SPI configuration parameters.
> 
> Signed-off-by: Ajith Anandhan <ajithanandhan0406@gmail.com>
> ---
>  .../bindings/iio/adc/ti,ads1120.yaml          | 109 ++++++++++++++++++
>  1 file changed, 109 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/ti,ads1120.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/ti,ads1120.yaml b/Documentation/devicetree/bindings/iio/adc/ti,ads1120.yaml
> new file mode 100644
> index 000000000..2449094af
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/ti,ads1120.yaml
>
> +
> +  vref-supply:
> +    description: |
> +      Optional external voltage reference. Can be connected to either
> +      REFP0/REFN0 or REFP1/REFN1 pins. If not supplied, the internal
> +      2.048V reference is used.

How do you know which set of inputs is used?  Looks like a register
needs to be programmed to pick between them.

> +
> +  ti,avdd-is-ref:
> +    type: boolean
> +    description: |
> +      If present, indicates that the AVDD supply voltage is of sufficient
> +      quality and stability to be used as the voltage reference instead of
> +      the internal reference. This allows the driver to select AVDD as the
> +      reference source for potentially better performance.

This one is interesting as I don't recall anyone arguing this made
sense before.  In what way better performance?  Are their boards out
there where this definitely makes sense to do?

Re: [PATCH v2 1/2] dt-bindings: iio: adc: Add TI ADS1120 binding

[David Lechner]

On 11/15/25 12:31 PM, Jonathan Cameron wrote:
> On Sun,  9 Nov 2025 19:41:18 +0530
> Ajith Anandhan <ajithanandhan0406@gmail.com> wrote:
> 
>> Add device tree binding documentation for the Texas Instruments
>> ADS1120.
>>
>> The binding defines required properties like compatible, reg, and
>> SPI configuration parameters.
>>
>> Signed-off-by: Ajith Anandhan <ajithanandhan0406@gmail.com>
>> ---
>>  .../bindings/iio/adc/ti,ads1120.yaml          | 109 ++++++++++++++++++
>>  1 file changed, 109 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/iio/adc/ti,ads1120.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/iio/adc/ti,ads1120.yaml b/Documentation/devicetree/bindings/iio/adc/ti,ads1120.yaml
>> new file mode 100644
>> index 000000000..2449094af
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/iio/adc/ti,ads1120.yaml
>>
>> +
>> +  vref-supply:
>> +    description: |
>> +      Optional external voltage reference. Can be connected to either
>> +      REFP0/REFN0 or REFP1/REFN1 pins. If not supplied, the internal
>> +      2.048V reference is used.
> 
> How do you know which set of inputs is used?  Looks like a register
> needs to be programmed to pick between them.

I would just make two supply properties for this, ref0-supply and ref1-supply

> 
>> +
>> +  ti,avdd-is-ref:
>> +    type: boolean
>> +    description: |
>> +      If present, indicates that the AVDD supply voltage is of sufficient
>> +      quality and stability to be used as the voltage reference instead of
>> +      the internal reference. This allows the driver to select AVDD as the
>> +      reference source for potentially better performance.
> 
> This one is interesting as I don't recall anyone arguing this made
> sense before.  In what way better performance?  Are their boards out
> there where this definitely makes sense to do?
> 

Seems harmless to have the property even if no one ever uses it. But I would
be curious to know the answers to those questions too.

[PATCH v2 2/2] iio: adc: Add support for TI ADS1120

[Ajith Anandhan]

Add driver for the Texas Instruments ADS1120, a precision 16-bit
analog-to-digital converter with an SPI interface.

The driver supports:
- Differential and single-ended input channels
- Configurable gain (1-128 for differential, 1-4 for single-ended)
- Internal 2.048V reference
- Single-shot conversion mode

Also update MAINTAINER document.

Signed-off-by: Ajith Anandhan <ajithanandhan0406@gmail.com>
---
 MAINTAINERS                  |   7 +
 drivers/iio/adc/Kconfig      |  10 +
 drivers/iio/adc/Makefile     |   1 +
 drivers/iio/adc/ti-ads1120.c | 631 +++++++++++++++++++++++++++++++++++
 4 files changed, 649 insertions(+)
 create mode 100644 drivers/iio/adc/ti-ads1120.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 3da2c26a7..1efe88fc9 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -25613,6 +25613,13 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/iio/adc/ti,ads1119.yaml
 F:	drivers/iio/adc/ti-ads1119.c
 
+TI ADS1120 ADC DRIVER
+M:	Ajith Anandhan <ajithanandhan0406@gmail.com>
+L:	linux-iio@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/iio/adc/ti,ads1120.yaml
+F:	drivers/iio/adc/ti-ads1120.c
+
 TI ADS7924 ADC DRIVER
 M:	Hugo Villeneuve <hvilleneuve@dimonoff.com>
 L:	linux-iio@vger.kernel.org
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 58a14e683..afb7895dd 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -1655,6 +1655,16 @@ config TI_ADS1119
          This driver can also be built as a module. If so, the module will be
          called ti-ads1119.
 
+config TI_ADS1120
+	tristate "Texas Instruments ADS1120"
+	depends on SPI
+	help
+	  Say yes here to build support for Texas Instruments ADS1120
+	  4-channel, 2kSPS, 16-bit delta-sigma ADC.
+
+	  This driver can also be built as module. If so, the module
+	  will be called ti-ads1120.
+
 config TI_ADS124S08
 	tristate "Texas Instruments ADS124S08"
 	depends on SPI
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index d008f78dc..49c56b459 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -144,6 +144,7 @@ obj-$(CONFIG_TI_ADC161S626) += ti-adc161s626.o
 obj-$(CONFIG_TI_ADS1015) += ti-ads1015.o
 obj-$(CONFIG_TI_ADS1100) += ti-ads1100.o
 obj-$(CONFIG_TI_ADS1119) += ti-ads1119.o
+obj-$(CONFIG_TI_ADS1120) += ti-ads1120.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-ads1120.c b/drivers/iio/adc/ti-ads1120.c
new file mode 100644
index 000000000..1e1871b74
--- /dev/null
+++ b/drivers/iio/adc/ti-ads1120.c
@@ -0,0 +1,631 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * TI ADS1120 4-channel, 2kSPS, 16-bit delta-sigma ADC
+ *
+ * Datasheet: https://www.ti.com/lit/gpn/ads1120
+ *
+ * Copyright (C) 2025 Ajith Anandhan <ajithanandhan0406@gmail.com>
+ */
+
+#include <linux/bitfield.h>
+#include <linux/cleanup.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+#include <linux/spi/spi.h>
+#include <linux/unaligned.h>
+
+#include <linux/iio/iio.h>
+
+
+/* Commands */
+#define ADS1120_CMD_RESET		0x06
+#define ADS1120_CMD_START		0x08
+#define ADS1120_CMD_PWRDWN		0x02
+#define ADS1120_CMD_RDATA		0x10
+#define ADS1120_CMD_RREG		0x20
+#define ADS1120_CMD_WREG		0x40
+
+/* Registers */
+#define ADS1120_REG_CONFIG0		0x00
+#define ADS1120_REG_CONFIG1		0x01
+#define ADS1120_REG_CONFIG2		0x02
+#define ADS1120_REG_CONFIG3		0x03
+
+/* Config Register 0 bit definitions */
+#define ADS1120_CFG0_MUX_MASK		GENMASK(7, 4)
+/* Differential inputs */
+#define ADS1120_CFG0_MUX_AIN0_AIN1	0
+#define ADS1120_CFG0_MUX_AIN0_AIN2	1
+#define ADS1120_CFG0_MUX_AIN0_AIN3	2
+#define ADS1120_CFG0_MUX_AIN1_AIN2	3
+#define ADS1120_CFG0_MUX_AIN1_AIN3	4
+#define ADS1120_CFG0_MUX_AIN2_AIN3	5
+#define ADS1120_CFG0_MUX_AIN1_AIN0	6
+#define ADS1120_CFG0_MUX_AIN3_AIN2	7
+/* Single-ended inputs */
+#define ADS1120_CFG0_MUX_AIN0_AVSS	8
+#define ADS1120_CFG0_MUX_AIN1_AVSS	9
+#define ADS1120_CFG0_MUX_AIN2_AVSS	10
+#define ADS1120_CFG0_MUX_AIN3_AVSS	11
+/* Diagnostic inputs */
+#define ADS1120_CFG0_MUX_REFP_REFN_4	12
+#define ADS1120_CFG0_MUX_AVDD_AVSS_4	13
+#define ADS1120_CFG0_MUX_SHORTED	14
+
+#define ADS1120_CFG0_GAIN_MASK		GENMASK(3, 1)
+#define ADS1120_CFG0_GAIN_1		0
+#define ADS1120_CFG0_GAIN_2		1
+#define ADS1120_CFG0_GAIN_4		2
+#define ADS1120_CFG0_GAIN_8		3
+#define ADS1120_CFG0_GAIN_16		4
+#define ADS1120_CFG0_GAIN_32		5
+#define ADS1120_CFG0_GAIN_64		6
+#define ADS1120_CFG0_GAIN_128		7
+
+#define ADS1120_CFG0_PGA_BYPASS		BIT(0)
+
+/* Config Register 1 bit definitions */
+#define ADS1120_CFG1_DR_MASK		GENMASK(7, 5)
+#define ADS1120_CFG1_DR_20SPS		0
+#define ADS1120_CFG1_DR_45SPS		1
+#define ADS1120_CFG1_DR_90SPS		2
+#define ADS1120_CFG1_DR_175SPS		3
+#define ADS1120_CFG1_DR_330SPS		4
+#define ADS1120_CFG1_DR_600SPS		5
+#define ADS1120_CFG1_DR_1000SPS		6
+
+#define ADS1120_CFG1_MODE_MASK		GENMASK(4, 3)
+#define ADS1120_CFG1_MODE_NORMAL	0
+#define ADS1120_CFG1_MODE_DUTY		1
+#define ADS1120_CFG1_MODE_TURBO		2
+
+#define ADS1120_CFG1_CM_MASK		BIT(2)
+#define ADS1120_CFG1_CM_SINGLE		0
+#define ADS1120_CFG1_CM_CONTINUOUS	1
+
+#define ADS1120_CFG1_TS_EN		BIT(1)
+#define ADS1120_CFG1_BCS_EN		BIT(0)
+
+/* Config Register 2 bit definitions */
+#define ADS1120_CFG2_VREF_MASK		GENMASK(7, 6)
+#define ADS1120_CFG2_VREF_INTERNAL	0
+#define ADS1120_CFG2_VREF_EXT_REFP0	1
+#define ADS1120_CFG2_VREF_EXT_AIN0	2
+#define ADS1120_CFG2_VREF_AVDD		3
+
+#define ADS1120_CFG2_REJECT_MASK	GENMASK(5, 4)
+#define ADS1120_CFG2_REJECT_OFF		0
+#define ADS1120_CFG2_REJECT_50_60	1
+#define ADS1120_CFG2_REJECT_50		2
+#define ADS1120_CFG2_REJECT_60		3
+
+#define ADS1120_CFG2_PSW_EN		BIT(3)
+
+#define ADS1120_CFG2_IDAC_MASK		GENMASK(2, 0)
+#define ADS1120_CFG2_IDAC_OFF		0
+#define ADS1120_CFG2_IDAC_10UA		1
+#define ADS1120_CFG2_IDAC_50UA		2
+#define ADS1120_CFG2_IDAC_100UA		3
+#define ADS1120_CFG2_IDAC_250UA		4
+#define ADS1120_CFG2_IDAC_500UA		5
+#define ADS1120_CFG2_IDAC_1000UA	6
+#define ADS1120_CFG2_IDAC_1500UA	7
+
+/* Config Register 3 bit definitions */
+#define ADS1120_CFG3_IDAC1_MASK		GENMASK(7, 5)
+#define ADS1120_CFG3_IDAC1_DISABLED	0
+#define ADS1120_CFG3_IDAC1_AIN0		1
+#define ADS1120_CFG3_IDAC1_AIN1		2
+#define ADS1120_CFG3_IDAC1_AIN2		3
+#define ADS1120_CFG3_IDAC1_AIN3		4
+#define ADS1120_CFG3_IDAC1_REFP0	5
+#define ADS1120_CFG3_IDAC1_REFN0	6
+
+#define ADS1120_CFG3_IDAC2_MASK		GENMASK(4, 2)
+#define ADS1120_CFG3_IDAC2_DISABLED	0
+#define ADS1120_CFG3_IDAC2_AIN0		1
+#define ADS1120_CFG3_IDAC2_AIN1		2
+#define ADS1120_CFG3_IDAC2_AIN2		3
+#define ADS1120_CFG3_IDAC2_AIN3		4
+#define ADS1120_CFG3_IDAC2_REFP0	5
+#define ADS1120_CFG3_IDAC2_REFN0	6
+
+#define ADS1120_CFG3_DRDYM_MASK		BIT(1)
+#define ADS1120_CFG3_DRDYM_DRDY_ONLY	0
+#define ADS1120_CFG3_DRDYM_BOTH		1
+
+/* Conversion time for 20 SPS */
+#define ADS1120_CONV_TIME_MS		51
+
+/* Internal reference voltage in millivolts */
+#define ADS1120_VREF_INTERNAL_MV	2048
+
+
+struct ads1120_state {
+	struct spi_device	*spi;
+	struct regmap		*regmap;
+	/*
+	 * Protects chip configuration and ADC reads to ensure
+	 * consistent channel/gain settings during conversions.
+	 */
+	struct mutex		lock;
+
+	int vref_mv;
+
+	/* DMA-safe buffer for SPI transfers */
+	u8 data[4] __aligned(IIO_DMA_MINALIGN);
+};
+
+
+static const int ads1120_gain_values[] = { 1, 2, 4, 8, 16, 32, 64, 128 };
+static const int ads1120_low_gain_values[] = { 1, 2, 4 };
+
+/* Differential channel macro */
+#define ADS1120_DIFF_CHANNEL(index, chan1, chan2)		\
+{								\
+	.type = IIO_VOLTAGE,					\
+	.indexed = 1,						\
+	.channel = chan1,					\
+	.channel2 = chan2,					\
+	.differential = 1,					\
+	.address = index,					\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |		\
+			      BIT(IIO_CHAN_INFO_SCALE),		\
+	.info_mask_separate_available = BIT(IIO_CHAN_INFO_SCALE), \
+}
+
+/* Single-ended channel macro */
+#define ADS1120_SINGLE_CHANNEL(index, chan)			\
+{								\
+	.type = IIO_VOLTAGE,					\
+	.indexed = 1,						\
+	.channel = chan,					\
+	.address = index,					\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |		\
+			      BIT(IIO_CHAN_INFO_SCALE),		\
+	.info_mask_separate_available = BIT(IIO_CHAN_INFO_SCALE), \
+}
+
+/* Diagnostic channel macro */
+#define ADS1120_DIAG_CHANNEL(index, label)			\
+{								\
+	.type = IIO_VOLTAGE,					\
+	.indexed = 1,						\
+	.channel = index,					\
+	.address = index,					\
+	.extend_name = label,					\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |		\
+			      BIT(IIO_CHAN_INFO_SCALE),		\
+	.info_mask_separate_available = BIT(IIO_CHAN_INFO_SCALE), \
+}
+
+static const struct iio_chan_spec ads1120_channels[] = {
+	/* Differential inputs */
+	ADS1120_DIFF_CHANNEL(ADS1120_CFG0_MUX_AIN0_AIN1, 0, 1),
+	ADS1120_DIFF_CHANNEL(ADS1120_CFG0_MUX_AIN0_AIN2, 0, 2),
+	ADS1120_DIFF_CHANNEL(ADS1120_CFG0_MUX_AIN0_AIN3, 0, 3),
+	ADS1120_DIFF_CHANNEL(ADS1120_CFG0_MUX_AIN1_AIN2, 1, 2),
+	ADS1120_DIFF_CHANNEL(ADS1120_CFG0_MUX_AIN1_AIN3, 1, 3),
+	ADS1120_DIFF_CHANNEL(ADS1120_CFG0_MUX_AIN2_AIN3, 2, 3),
+	ADS1120_DIFF_CHANNEL(ADS1120_CFG0_MUX_AIN1_AIN0, 1, 0),
+	ADS1120_DIFF_CHANNEL(ADS1120_CFG0_MUX_AIN3_AIN2, 3, 2),
+	/* Single-ended inputs */
+	ADS1120_SINGLE_CHANNEL(ADS1120_CFG0_MUX_AIN0_AVSS, 0),
+	ADS1120_SINGLE_CHANNEL(ADS1120_CFG0_MUX_AIN1_AVSS, 1),
+	ADS1120_SINGLE_CHANNEL(ADS1120_CFG0_MUX_AIN2_AVSS, 2),
+	ADS1120_SINGLE_CHANNEL(ADS1120_CFG0_MUX_AIN3_AVSS, 3),
+	/* Diagnostic inputs */
+	ADS1120_DIAG_CHANNEL(ADS1120_CFG0_MUX_REFP_REFN_4, "ref_div4"),
+	ADS1120_DIAG_CHANNEL(ADS1120_CFG0_MUX_AVDD_AVSS_4, "avdd_div4"),
+	ADS1120_DIAG_CHANNEL(ADS1120_CFG0_MUX_SHORTED, "shorted"),
+};
+
+static bool ads1120_is_differential_channel(const struct iio_chan_spec *chan)
+{
+	return chan->address <= ADS1120_CFG0_MUX_AIN3_AIN2;
+}
+
+static int ads1120_write_cmd(struct ads1120_state *st, u8 cmd)
+{
+	st->data[0] = cmd;
+	return spi_write(st->spi, st->data, sizeof(st->data[0]));
+}
+
+static int ads1120_reset(struct ads1120_state *st)
+{
+	int ret;
+
+	ret = ads1120_write_cmd(st, ADS1120_CMD_RESET);
+	if (ret)
+		return ret;
+
+	/*
+	 * Datasheet specifies reset takes 50us + 32 * t(CLK)
+	 * where t(CLK) = 1/4.096MHz (~0.24us).
+	 * 50us + 32 * 0.24us = ~58us. Use 200us to be safe.
+	 */
+	fsleep(200);
+
+	regcache_mark_dirty(st->regmap);
+
+	return 0;
+}
+
+static int ads1120_set_mux(struct ads1120_state *st, u8 mux_val)
+{
+	if (mux_val > ADS1120_CFG0_MUX_SHORTED)
+		return -EINVAL;
+
+	return regmap_update_bits(st->regmap, ADS1120_REG_CONFIG0,
+				  ADS1120_CFG0_MUX_MASK,
+				  FIELD_PREP(ADS1120_CFG0_MUX_MASK, mux_val));
+}
+
+static int ads1120_set_gain(struct ads1120_state *st, int gain_val)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(ads1120_gain_values); i++) {
+		if (ads1120_gain_values[i] == gain_val)
+			break;
+	}
+
+	if (i == ARRAY_SIZE(ads1120_gain_values))
+		return -EINVAL;
+
+	return regmap_update_bits(st->regmap, ADS1120_REG_CONFIG0,
+				  ADS1120_CFG0_GAIN_MASK,
+				  FIELD_PREP(ADS1120_CFG0_GAIN_MASK, i));
+}
+
+static int ads1120_get_gain(struct ads1120_state *st)
+{
+	unsigned int val;
+	int ret;
+
+	ret = regmap_read(st->regmap, ADS1120_REG_CONFIG0, &val);
+	if (ret)
+		return ret;
+
+	return ads1120_gain_values[FIELD_GET(ADS1120_CFG0_GAIN_MASK, val)];
+}
+
+static int ads1120_read_raw_adc(struct ads1120_state *st, int *val)
+{
+	int ret;
+	struct spi_transfer xfer[2] = {
+		{
+			.tx_buf = st->data,
+			.len = 1,
+		}, {
+			.rx_buf = st->data,
+			.len = 2,
+		}
+	};
+
+	st->data[0] = ADS1120_CMD_RDATA;
+
+	ret = spi_sync_transfer(st->spi, xfer, ARRAY_SIZE(xfer));
+	if (ret)
+		return ret;
+
+	*val = sign_extend32(get_unaligned_be16(st->data), 15);
+
+	return 0;
+}
+
+static int ads1120_read_measurement(struct ads1120_state *st,
+				    const struct iio_chan_spec *chan, int *val)
+{
+	int ret;
+
+	ret = ads1120_set_mux(st, chan->address);
+	if (ret)
+		return ret;
+
+	ret = ads1120_write_cmd(st, ADS1120_CMD_START);
+	if (ret)
+		return ret;
+
+	msleep(ADS1120_CONV_TIME_MS);
+
+	return ads1120_read_raw_adc(st, val);
+}
+
+static int ads1120_read_raw(struct iio_dev *indio_dev,
+			    struct iio_chan_spec const *chan,
+			    int *val, int *val2, long mask)
+{
+	struct ads1120_state *st = iio_priv(indio_dev);
+	int ret, gain;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		guard(mutex)(&st->lock);
+		ret = ads1120_read_measurement(st, chan, val);
+		if (ret)
+			return ret;
+		return IIO_VAL_INT;
+
+	case IIO_CHAN_INFO_SCALE:
+		/*
+		 * Scale = Vref / (gain * 2^15)
+		 * Return in format: val / 2^val2
+		 */
+		gain = ads1120_get_gain(st);
+		if (gain < 0)
+			return gain;
+
+		*val = st->vref_mv;
+		*val2 = gain * 15;
+		return IIO_VAL_FRACTIONAL_LOG2;
+
+	default:
+		return -EINVAL;
+	}
+}
+
+static int ads1120_write_raw(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan,
+			     int val, int val2, long mask)
+{
+	struct ads1120_state *st = iio_priv(indio_dev);
+
+	switch (mask) {
+	case IIO_CHAN_INFO_SCALE:
+		/*
+		 * Validate gain for single-ended and diagnostic channels.
+		 * These channels require PGA bypass and support only
+		 * gains 1, 2, and 4.
+		 */
+		if (!ads1120_is_differential_channel(chan) && val > 4)
+			return -EINVAL;
+
+		guard(mutex)(&st->lock);
+		return ads1120_set_gain(st, val);
+
+	default:
+		return -EINVAL;
+	}
+}
+
+static int ads1120_read_avail(struct iio_dev *indio_dev,
+			      struct iio_chan_spec const *chan,
+			      const int **vals, int *type, int *length,
+			      long mask)
+{
+	switch (mask) {
+	case IIO_CHAN_INFO_SCALE:
+		if (ads1120_is_differential_channel(chan)) {
+			*vals = ads1120_gain_values;
+			*length = ARRAY_SIZE(ads1120_gain_values);
+		} else {
+			*vals = ads1120_low_gain_values;
+			*length = ARRAY_SIZE(ads1120_low_gain_values);
+		}
+		*type = IIO_VAL_INT;
+		return IIO_AVAIL_LIST;
+
+	default:
+		return -EINVAL;
+	}
+}
+
+static const struct iio_info ads1120_info = {
+	.read_raw = ads1120_read_raw,
+	.write_raw = ads1120_write_raw,
+	.read_avail = ads1120_read_avail,
+};
+
+/* Regmap read function for ADS1120 */
+static int ads1120_regmap_read(void *context, const void *reg_buf,
+			       size_t reg_size, void *val_buf, size_t val_size)
+{
+	struct ads1120_state *st = context;
+	u8 reg = *(u8 *)reg_buf;
+	u8 *val = val_buf;
+	int ret;
+	struct spi_transfer xfer[2] = {
+		{
+			.tx_buf = st->data,
+			.len = 1,
+		}, {
+			.rx_buf = val,
+			.len = val_size,
+		}
+	};
+
+	if (reg > ADS1120_REG_CONFIG3)
+		return -EINVAL;
+
+	/* RREG command: 0010rr00 where rr is register address */
+	st->data[0] = ADS1120_CMD_RREG | (reg << 2);
+
+	ret = spi_sync_transfer(st->spi, xfer, ARRAY_SIZE(xfer));
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+/* Regmap write function for ADS1120 */
+static int ads1120_regmap_write(void *context, const void *data, size_t count)
+{
+	struct ads1120_state *st = context;
+	const u8 *buf = data;
+
+	if (count != 2)
+		return -EINVAL;
+
+	/* WREG command: 0100rr00 where rr is register address */
+	st->data[0] = ADS1120_CMD_WREG | (buf[0] << 2);
+	st->data[1] = buf[1];
+
+	return spi_write(st->spi, st->data, 2);
+}
+
+static const struct regmap_bus ads1120_regmap_bus = {
+	.read = ads1120_regmap_read,
+	.write = ads1120_regmap_write,
+	.reg_format_endian_default = REGMAP_ENDIAN_BIG,
+	.val_format_endian_default = REGMAP_ENDIAN_BIG,
+};
+
+static const struct regmap_config ads1120_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.max_register = ADS1120_REG_CONFIG3,
+	.cache_type = REGCACHE_FLAT,
+};
+
+static int ads1120_init(struct ads1120_state *st)
+{
+	int ret;
+
+	ret = ads1120_reset(st);
+	if (ret)
+		return dev_err_probe(&st->spi->dev, ret,
+					"Failed to reset device\n");
+
+	/*
+	 * Configure Register 0:
+	 * - Input MUX: AIN0/AVSS
+	 * - Gain: 1
+	 * - PGA bypass enabled. When gain is set > 4, this bit is
+	 *   automatically ignored by the hardware and PGA is enabled,
+	 *   so it's safe to leave it set.
+	 */
+	ret = regmap_write(st->regmap, ADS1120_REG_CONFIG0,
+			   FIELD_PREP(ADS1120_CFG0_MUX_MASK,
+				      ADS1120_CFG0_MUX_AIN0_AVSS) |
+			   FIELD_PREP(ADS1120_CFG0_GAIN_MASK,
+				      ADS1120_CFG0_GAIN_1) |
+			   ADS1120_CFG0_PGA_BYPASS);
+	if (ret)
+		return ret;
+
+	/*
+	 * Configure Register 1:
+	 * - Data rate: 20 SPS (for single-shot mode)
+	 * - Operating mode: Normal
+	 * - Conversion mode: Single-shot
+	 * - Temperature sensor: Disabled
+	 * - Burnout current: Disabled
+	 */
+	ret = regmap_write(st->regmap, ADS1120_REG_CONFIG1,
+			   FIELD_PREP(ADS1120_CFG1_DR_MASK,
+				      ADS1120_CFG1_DR_20SPS) |
+			   FIELD_PREP(ADS1120_CFG1_MODE_MASK,
+				      ADS1120_CFG1_MODE_NORMAL) |
+			   FIELD_PREP(ADS1120_CFG1_CM_MASK,
+				      ADS1120_CFG1_CM_SINGLE) |
+			   FIELD_PREP(ADS1120_CFG1_TS_EN, 0) |
+			   FIELD_PREP(ADS1120_CFG1_BCS_EN, 0));
+	if (ret)
+		return ret;
+
+	/*
+	 * Configure Register 2:
+	 * - Voltage reference: Internal 2.048V
+	 * - 50/60Hz rejection: Off
+	 * - Power switch: Disabled
+	 * - IDAC current: Off
+	 */
+	ret = regmap_write(st->regmap, ADS1120_REG_CONFIG2,
+			   FIELD_PREP(ADS1120_CFG2_VREF_MASK,
+				      ADS1120_CFG2_VREF_INTERNAL) |
+			   FIELD_PREP(ADS1120_CFG2_REJECT_MASK,
+				      ADS1120_CFG2_REJECT_OFF) |
+			   FIELD_PREP(ADS1120_CFG2_PSW_EN, 0) |
+			   FIELD_PREP(ADS1120_CFG2_IDAC_MASK,
+				      ADS1120_CFG2_IDAC_OFF));
+	if (ret)
+		return ret;
+
+	/*
+	 * Configure Register 3:
+	 * - IDAC1: Disabled
+	 * - IDAC2: Disabled
+	 * - DRDY mode: Only reflects data ready status
+	 */
+	ret = regmap_write(st->regmap, ADS1120_REG_CONFIG3,
+			   FIELD_PREP(ADS1120_CFG3_IDAC1_MASK,
+				      ADS1120_CFG3_IDAC1_DISABLED) |
+			   FIELD_PREP(ADS1120_CFG3_IDAC2_MASK,
+				      ADS1120_CFG3_IDAC2_DISABLED) |
+			   FIELD_PREP(ADS1120_CFG3_DRDYM_MASK,
+				      ADS1120_CFG3_DRDYM_DRDY_ONLY));
+	if (ret)
+		return ret;
+
+	st->vref_mv = ADS1120_VREF_INTERNAL_MV;
+
+	return 0;
+}
+
+static int ads1120_probe(struct spi_device *spi)
+{
+	struct device *dev = &spi->dev;
+	struct iio_dev *indio_dev;
+	struct ads1120_state *st;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	st = iio_priv(indio_dev);
+	st->spi = spi;
+
+	ret = devm_mutex_init(dev, &st->lock);
+	if (ret)
+		return ret;
+
+	st->regmap = devm_regmap_init(dev, &ads1120_regmap_bus, st,
+				      &ads1120_regmap_config);
+	if (IS_ERR(st->regmap))
+		return dev_err_probe(dev, PTR_ERR(st->regmap),
+					"Failed to initialize regmap\n");
+
+	indio_dev->name = "ads1120";
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->channels = ads1120_channels;
+	indio_dev->num_channels = ARRAY_SIZE(ads1120_channels);
+	indio_dev->info = &ads1120_info;
+
+	ret = ads1120_init(st);
+	if (ret)
+		return dev_err_probe(dev, ret,
+					"Failed to initialize device\n");
+
+	return devm_iio_device_register(dev, indio_dev);
+}
+
+static const struct of_device_id ads1120_of_match[] = {
+	{ .compatible = "ti,ads1120" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, ads1120_of_match);
+
+static const struct spi_device_id ads1120_id[] = {
+	{ "ads1120" },
+	{ }
+};
+MODULE_DEVICE_TABLE(spi, ads1120_id);
+
+static struct spi_driver ads1120_driver = {
+	.driver = {
+		.name = "ads1120",
+		.of_match_table = ads1120_of_match,
+	},
+	.probe = ads1120_probe,
+	.id_table = ads1120_id,
+};
+module_spi_driver(ads1120_driver);
+
+MODULE_AUTHOR("Ajith Anandhan <ajithanandhan0406@gmail.com>");
+MODULE_DESCRIPTION("Texas Instruments ADS1120 ADC driver");
+MODULE_LICENSE("GPL");
-- 
2.34.1

Re: [PATCH v2 2/2] iio: adc: Add support for TI ADS1120

[Andy Shevchenko]

On Sun, Nov 09, 2025 at 07:41:19PM +0530, Ajith Anandhan wrote:
> Add driver for the Texas Instruments ADS1120, a precision 16-bit
> analog-to-digital converter with an SPI interface.
> 
> The driver supports:
> - Differential and single-ended input channels
> - Configurable gain (1-128 for differential, 1-4 for single-ended)
> - Internal 2.048V reference
> - Single-shot conversion mode

> Also update MAINTAINER document.

Unneeded sentence in the commit message (may be located in the comment block,
though).

...

Many are still missing... Please, follow IWYU principle.

> +#include <linux/bitfield.h>
> +#include <linux/cleanup.h>
> +#include <linux/delay.h>

> +#include <linux/device.h>

Not see why you need this and not

dev_printk.h
device/devres.h

instead.

> +#include <linux/err.h>
> +#include <linux/module.h>
> +#include <linux/regmap.h>
> +#include <linux/spi/spi.h>
> +#include <linux/unaligned.h>

...

> +/* Internal reference voltage in millivolts */
> +#define ADS1120_VREF_INTERNAL_MV	2048

_mV

*Yes, it's okay to use small letter in this case (it's all about proper units).

...

> +struct ads1120_state {
> +	struct spi_device	*spi;
> +	struct regmap		*regmap;

I'm not sure why do you need separate regmap and spi transactions at the same
time. The commit message also kept silent about this. Needs a justification.

In any case the spi device can be derived from regmap, so definitely you don't
need both.

> +	/*
> +	 * Protects chip configuration and ADC reads to ensure
> +	 * consistent channel/gain settings during conversions.
> +	 */
> +	struct mutex		lock;

No header for this type.

> +	int vref_mv;

_mV

> +	/* DMA-safe buffer for SPI transfers */
> +	u8 data[4] __aligned(IIO_DMA_MINALIGN);

No header for this type and __aligned attribute.

> +};

...

> +	struct spi_transfer xfer[2] = {

You may leave []

> +		{
> +			.tx_buf = st->data,
> +			.len = 1,
> +		}, {
> +			.rx_buf = st->data,
> +			.len = 2,
> +		}
> +	};
> +
> +	*val = sign_extend32(get_unaligned_be16(st->data), 15);

No header for this API.

> +	return 0;
> +}

...

> +static int ads1120_read_measurement(struct ads1120_state *st,
> +				    const struct iio_chan_spec *chan, int *val)
> +{
> +	int ret;
> +
> +	ret = ads1120_set_mux(st, chan->address);
> +	if (ret)
> +		return ret;
> +
> +	ret = ads1120_write_cmd(st, ADS1120_CMD_START);
> +	if (ret)
> +		return ret;

Needs a comment explaining this rather big delay.

> +	msleep(ADS1120_CONV_TIME_MS);
> +
> +	return ads1120_read_raw_adc(st, val);
> +}

...

> +/* Regmap write function for ADS1120 */
> +static int ads1120_regmap_write(void *context, const void *data, size_t count)
> +{
> +	struct ads1120_state *st = context;
> +	const u8 *buf = data;
> +
> +	if (count != 2)
> +		return -EINVAL;
> +
> +	/* WREG command: 0100rr00 where rr is register address */
> +	st->data[0] = ADS1120_CMD_WREG | (buf[0] << 2);
> +	st->data[1] = buf[1];
> +
> +	return spi_write(st->spi, st->data, 2);

Wondering if there is a correlation between count == 2 and this 2. If it has
1:1 relationship, perhaps use count directly here?

> +}

...

> +static const struct regmap_config ads1120_regmap_config = {
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +	.max_register = ADS1120_REG_CONFIG3,

> +	.cache_type = REGCACHE_FLAT,

Why not MAPPLE? Or scattered FLAT?

> +};

...

> +static int ads1120_init(struct ads1120_state *st)
> +{
> +	int ret;

	struct device *dev = ... // from regmap

> +	ret = ads1120_reset(st);
> +	if (ret)
> +		return dev_err_probe(&st->spi->dev, ret,
> +					"Failed to reset device\n");

		return dev_err_probe(dev, ret, "Failed to reset device\n");

> +	/*
> +	 * Configure Register 0:
> +	 * - Input MUX: AIN0/AVSS
> +	 * - Gain: 1
> +	 * - PGA bypass enabled. When gain is set > 4, this bit is
> +	 *   automatically ignored by the hardware and PGA is enabled,
> +	 *   so it's safe to leave it set.
> +	 */
> +	ret = regmap_write(st->regmap, ADS1120_REG_CONFIG0,
> +			   FIELD_PREP(ADS1120_CFG0_MUX_MASK,
> +				      ADS1120_CFG0_MUX_AIN0_AVSS) |

I would do it on a single line...

> +			   FIELD_PREP(ADS1120_CFG0_GAIN_MASK,
> +				      ADS1120_CFG0_GAIN_1) |

...and this despite being long, But it's up to you and maintainers.
Same for all similar cases.

> +			   ADS1120_CFG0_PGA_BYPASS);
> +	if (ret)
> +		return ret;
> +
> +	/*
> +	 * Configure Register 1:
> +	 * - Data rate: 20 SPS (for single-shot mode)
> +	 * - Operating mode: Normal
> +	 * - Conversion mode: Single-shot
> +	 * - Temperature sensor: Disabled
> +	 * - Burnout current: Disabled
> +	 */
> +	ret = regmap_write(st->regmap, ADS1120_REG_CONFIG1,
> +			   FIELD_PREP(ADS1120_CFG1_DR_MASK,
> +				      ADS1120_CFG1_DR_20SPS) |
> +			   FIELD_PREP(ADS1120_CFG1_MODE_MASK,
> +				      ADS1120_CFG1_MODE_NORMAL) |
> +			   FIELD_PREP(ADS1120_CFG1_CM_MASK,
> +				      ADS1120_CFG1_CM_SINGLE) |
> +			   FIELD_PREP(ADS1120_CFG1_TS_EN, 0) |
> +			   FIELD_PREP(ADS1120_CFG1_BCS_EN, 0));
> +	if (ret)
> +		return ret;
> +
> +	/*
> +	 * Configure Register 2:
> +	 * - Voltage reference: Internal 2.048V
> +	 * - 50/60Hz rejection: Off
> +	 * - Power switch: Disabled
> +	 * - IDAC current: Off
> +	 */
> +	ret = regmap_write(st->regmap, ADS1120_REG_CONFIG2,
> +			   FIELD_PREP(ADS1120_CFG2_VREF_MASK,
> +				      ADS1120_CFG2_VREF_INTERNAL) |
> +			   FIELD_PREP(ADS1120_CFG2_REJECT_MASK,
> +				      ADS1120_CFG2_REJECT_OFF) |
> +			   FIELD_PREP(ADS1120_CFG2_PSW_EN, 0) |
> +			   FIELD_PREP(ADS1120_CFG2_IDAC_MASK,
> +				      ADS1120_CFG2_IDAC_OFF));
> +	if (ret)
> +		return ret;
> +
> +	/*
> +	 * Configure Register 3:
> +	 * - IDAC1: Disabled
> +	 * - IDAC2: Disabled
> +	 * - DRDY mode: Only reflects data ready status
> +	 */
> +	ret = regmap_write(st->regmap, ADS1120_REG_CONFIG3,
> +			   FIELD_PREP(ADS1120_CFG3_IDAC1_MASK,
> +				      ADS1120_CFG3_IDAC1_DISABLED) |
> +			   FIELD_PREP(ADS1120_CFG3_IDAC2_MASK,
> +				      ADS1120_CFG3_IDAC2_DISABLED) |
> +			   FIELD_PREP(ADS1120_CFG3_DRDYM_MASK,
> +				      ADS1120_CFG3_DRDYM_DRDY_ONLY));
> +	if (ret)
> +		return ret;
> +
> +	st->vref_mv = ADS1120_VREF_INTERNAL_MV;
> +
> +	return 0;
> +}

...

> +static int ads1120_probe(struct spi_device *spi)
> +{
> +	struct device *dev = &spi->dev;
> +	struct iio_dev *indio_dev;
> +	struct ads1120_state *st;
> +	int ret;
> +
> +	indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	st = iio_priv(indio_dev);
> +	st->spi = spi;
> +
> +	ret = devm_mutex_init(dev, &st->lock);
> +	if (ret)
> +		return ret;
> +
> +	st->regmap = devm_regmap_init(dev, &ads1120_regmap_bus, st,
> +				      &ads1120_regmap_config);
> +	if (IS_ERR(st->regmap))
> +		return dev_err_probe(dev, PTR_ERR(st->regmap),
> +					"Failed to initialize regmap\n");
> +
> +	indio_dev->name = "ads1120";
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->channels = ads1120_channels;
> +	indio_dev->num_channels = ARRAY_SIZE(ads1120_channels);

No header for ARRAY_SIZE().

> +	indio_dev->info = &ads1120_info;
> +
> +	ret = ads1120_init(st);
> +	if (ret)
> +		return dev_err_probe(dev, ret,
> +					"Failed to initialize device\n");

Besides broken indentation this may be a single line.

> +	return devm_iio_device_register(dev, indio_dev);
> +}

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 2/2] iio: adc: Add support for TI ADS1120

[Andy Shevchenko]

On Sun, Nov 09, 2025 at 07:03:56PM +0200, Andy Shevchenko wrote:
> On Sun, Nov 09, 2025 at 07:41:19PM +0530, Ajith Anandhan wrote:

...

> Many are still missing... Please, follow IWYU principle.
> 
> > +#include <linux/bitfield.h>
> > +#include <linux/cleanup.h>
> > +#include <linux/delay.h>
> 
> > +#include <linux/device.h>
> 
> Not see why you need this and not
> 
> dev_printk.h
> device/devres.h

Actually even devres.h is not needed if I did not miss anything.

> instead.
> 
> > +#include <linux/err.h>
> > +#include <linux/module.h>
> > +#include <linux/regmap.h>
> > +#include <linux/spi/spi.h>
> > +#include <linux/unaligned.h>

...

Overall looks much better than previous version.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 2/2] iio: adc: Add support for TI ADS1120

[kernel test robot]

Hi Ajith,

kernel test robot noticed the following build errors:

[auto build test ERROR on jic23-iio/togreg]
[also build test ERROR on linus/master v6.18-rc5 next-20251110]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Ajith-Anandhan/dt-bindings-iio-adc-Add-TI-ADS1120-binding/20251109-221245
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
patch link:    https://lore.kernel.org/r/20251109141119.561756-3-ajithanandhan0406%40gmail.com
patch subject: [PATCH v2 2/2] iio: adc: Add support for TI ADS1120
config: loongarch-allmodconfig (https://download.01.org/0day-ci/archive/20251110/202511101707.NSNVObH4-lkp@intel.com/config)
compiler: clang version 19.1.7 (https://github.com/llvm/llvm-project cd708029e0b2869e80abe31ddb175f7c35361f90)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251110/202511101707.NSNVObH4-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202511101707.NSNVObH4-lkp@intel.com/

All error/warnings (new ones prefixed by >>):

>> drivers/iio/adc/ti-ads1120.c:347:3: warning: label followed by a declaration is a C23 extension [-Wc23-extensions]
     347 |                 guard(mutex)(&st->lock);
         |                 ^
   include/linux/cleanup.h:401:2: note: expanded from macro 'guard'
     401 |         CLASS(_name, __UNIQUE_ID(guard))
         |         ^
   include/linux/cleanup.h:290:2: note: expanded from macro 'CLASS'
     290 |         class_##_name##_t var __cleanup(class_##_name##_destructor) =   \
         |         ^
   <scratch space>:10:1: note: expanded from here
      10 | class_mutex_t
         | ^
>> drivers/iio/adc/ti-ads1120.c:366:2: error: cannot jump from switch statement to this case label
     366 |         default:
         |         ^
   drivers/iio/adc/ti-ads1120.c:347:3: note: jump bypasses initialization of variable with __attribute__((cleanup))
     347 |                 guard(mutex)(&st->lock);
         |                 ^
   include/linux/cleanup.h:401:15: note: expanded from macro 'guard'
     401 |         CLASS(_name, __UNIQUE_ID(guard))
         |                      ^
   include/linux/compiler.h:166:29: note: expanded from macro '__UNIQUE_ID'
     166 | #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
         |                             ^
   include/linux/compiler_types.h:84:22: note: expanded from macro '__PASTE'
      84 | #define __PASTE(a,b) ___PASTE(a,b)
         |                      ^
   include/linux/compiler_types.h:83:23: note: expanded from macro '___PASTE'
      83 | #define ___PASTE(a,b) a##b
         |                       ^
   <scratch space>:8:1: note: expanded from here
       8 | __UNIQUE_ID_guard560
         | ^
   drivers/iio/adc/ti-ads1120.c:353:2: error: cannot jump from switch statement to this case label
     353 |         case IIO_CHAN_INFO_SCALE:
         |         ^
   drivers/iio/adc/ti-ads1120.c:347:3: note: jump bypasses initialization of variable with __attribute__((cleanup))
     347 |                 guard(mutex)(&st->lock);
         |                 ^
   include/linux/cleanup.h:401:15: note: expanded from macro 'guard'
     401 |         CLASS(_name, __UNIQUE_ID(guard))
         |                      ^
   include/linux/compiler.h:166:29: note: expanded from macro '__UNIQUE_ID'
     166 | #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
         |                             ^
   include/linux/compiler_types.h:84:22: note: expanded from macro '__PASTE'
      84 | #define __PASTE(a,b) ___PASTE(a,b)
         |                      ^
   include/linux/compiler_types.h:83:23: note: expanded from macro '___PASTE'
      83 | #define ___PASTE(a,b) a##b
         |                       ^
   <scratch space>:8:1: note: expanded from here
       8 | __UNIQUE_ID_guard560
         | ^
   drivers/iio/adc/ti-ads1120.c:390:2: error: cannot jump from switch statement to this case label
     390 |         default:
         |         ^
   drivers/iio/adc/ti-ads1120.c:387:3: note: jump bypasses initialization of variable with __attribute__((cleanup))
     387 |                 guard(mutex)(&st->lock);
         |                 ^
   include/linux/cleanup.h:401:15: note: expanded from macro 'guard'
     401 |         CLASS(_name, __UNIQUE_ID(guard))
         |                      ^
   include/linux/compiler.h:166:29: note: expanded from macro '__UNIQUE_ID'
     166 | #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
         |                             ^
   include/linux/compiler_types.h:84:22: note: expanded from macro '__PASTE'
      84 | #define __PASTE(a,b) ___PASTE(a,b)
         |                      ^
   include/linux/compiler_types.h:83:23: note: expanded from macro '___PASTE'
      83 | #define ___PASTE(a,b) a##b
         |                       ^
   <scratch space>:17:1: note: expanded from here
      17 | __UNIQUE_ID_guard561
         | ^
   1 warning and 3 errors generated.


vim +366 drivers/iio/adc/ti-ads1120.c

   337	
   338	static int ads1120_read_raw(struct iio_dev *indio_dev,
   339				    struct iio_chan_spec const *chan,
   340				    int *val, int *val2, long mask)
   341	{
   342		struct ads1120_state *st = iio_priv(indio_dev);
   343		int ret, gain;
   344	
   345		switch (mask) {
   346		case IIO_CHAN_INFO_RAW:
 > 347			guard(mutex)(&st->lock);
   348			ret = ads1120_read_measurement(st, chan, val);
   349			if (ret)
   350				return ret;
   351			return IIO_VAL_INT;
   352	
   353		case IIO_CHAN_INFO_SCALE:
   354			/*
   355			 * Scale = Vref / (gain * 2^15)
   356			 * Return in format: val / 2^val2
   357			 */
   358			gain = ads1120_get_gain(st);
   359			if (gain < 0)
   360				return gain;
   361	
   362			*val = st->vref_mv;
   363			*val2 = gain * 15;
   364			return IIO_VAL_FRACTIONAL_LOG2;
   365	
 > 366		default:
   367			return -EINVAL;
   368		}
   369	}
   370	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v2 2/2] iio: adc: Add support for TI ADS1120

[Jonathan Cameron]

On Sun,  9 Nov 2025 19:41:19 +0530
Ajith Anandhan <ajithanandhan0406@gmail.com> wrote:

> Add driver for the Texas Instruments ADS1120, a precision 16-bit
> analog-to-digital converter with an SPI interface.
> 
> The driver supports:
> - Differential and single-ended input channels
> - Configurable gain (1-128 for differential, 1-4 for single-ended)
> - Internal 2.048V reference
> - Single-shot conversion mode
> 
> Also update MAINTAINER document.
> 
> Signed-off-by: Ajith Anandhan <ajithanandhan0406@gmail.com>
Hi Ajith

A few comments from me to add to Andy's review. 

Jonathan

> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index d008f78dc..49c56b459 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -144,6 +144,7 @@ obj-$(CONFIG_TI_ADC161S626) += ti-adc161s626.o
>  obj-$(CONFIG_TI_ADS1015) += ti-ads1015.o
>  obj-$(CONFIG_TI_ADS1100) += ti-ads1100.o
>  obj-$(CONFIG_TI_ADS1119) += ti-ads1119.o
> +obj-$(CONFIG_TI_ADS1120) += ti-ads1120.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-ads1120.c b/drivers/iio/adc/ti-ads1120.c
> new file mode 100644
> index 000000000..1e1871b74
> --- /dev/null
> +++ b/drivers/iio/adc/ti-ads1120.c

> +
> +/* Differential channel macro */
> +#define ADS1120_DIFF_CHANNEL(index, chan1, chan2)		\
> +{								\
> +	.type = IIO_VOLTAGE,					\
> +	.indexed = 1,						\
> +	.channel = chan1,					\
> +	.channel2 = chan2,					\
> +	.differential = 1,					\
> +	.address = index,					\
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |		\
> +			      BIT(IIO_CHAN_INFO_SCALE),		\
> +	.info_mask_separate_available = BIT(IIO_CHAN_INFO_SCALE), \
> +}
> +
> +/* Single-ended channel macro */
> +#define ADS1120_SINGLE_CHANNEL(index, chan)			\
> +{								\
> +	.type = IIO_VOLTAGE,					\
> +	.indexed = 1,						\
> +	.channel = chan,					\
> +	.address = index,					\
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |		\
> +			      BIT(IIO_CHAN_INFO_SCALE),		\
I note that scale is the same for all channels. So why have separate
attributes?

> +	.info_mask_separate_available = BIT(IIO_CHAN_INFO_SCALE), \
> +}
> +
> +/* Diagnostic channel macro */
> +#define ADS1120_DIAG_CHANNEL(index, label)			\
> +{								\
> +	.type = IIO_VOLTAGE,					\
> +	.indexed = 1,						\
> +	.channel = index,					\
> +	.address = index,					\
> +	.extend_name = label,					\

We very rarely allow extend)name in new drivers.
It is a real pain for userspace code to deal with, so we now
put that info behind the get_label() callback.

It's also fairly rare that we put diagnostic channels out.  Might
be better to push those to debugfs and keep the main interface
less confusing.

Shorted is sometimes done as a differential channel with itself
(doesn't matter which one). It's made a little more complex here
as it is specified as shorting both to (AVDD + AVDSS) / 2 .


> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |		\
> +			      BIT(IIO_CHAN_INFO_SCALE),		\
> +	.info_mask_separate_available = BIT(IIO_CHAN_INFO_SCALE), \
> +}
> +
> +static const struct iio_chan_spec ads1120_channels[] = {
> +	/* Differential inputs */
> +	ADS1120_DIFF_CHANNEL(ADS1120_CFG0_MUX_AIN0_AIN1, 0, 1),
> +	ADS1120_DIFF_CHANNEL(ADS1120_CFG0_MUX_AIN0_AIN2, 0, 2),
> +	ADS1120_DIFF_CHANNEL(ADS1120_CFG0_MUX_AIN0_AIN3, 0, 3),
> +	ADS1120_DIFF_CHANNEL(ADS1120_CFG0_MUX_AIN1_AIN2, 1, 2),
> +	ADS1120_DIFF_CHANNEL(ADS1120_CFG0_MUX_AIN1_AIN3, 1, 3),
> +	ADS1120_DIFF_CHANNEL(ADS1120_CFG0_MUX_AIN2_AIN3, 2, 3),
> +	ADS1120_DIFF_CHANNEL(ADS1120_CFG0_MUX_AIN1_AIN0, 1, 0),
> +	ADS1120_DIFF_CHANNEL(ADS1120_CFG0_MUX_AIN3_AIN2, 3, 2),
> +	/* Single-ended inputs */
> +	ADS1120_SINGLE_CHANNEL(ADS1120_CFG0_MUX_AIN0_AVSS, 0),
> +	ADS1120_SINGLE_CHANNEL(ADS1120_CFG0_MUX_AIN1_AVSS, 1),
> +	ADS1120_SINGLE_CHANNEL(ADS1120_CFG0_MUX_AIN2_AVSS, 2),
> +	ADS1120_SINGLE_CHANNEL(ADS1120_CFG0_MUX_AIN3_AVSS, 3),
> +	/* Diagnostic inputs */
> +	ADS1120_DIAG_CHANNEL(ADS1120_CFG0_MUX_REFP_REFN_4, "ref_div4"),
> +	ADS1120_DIAG_CHANNEL(ADS1120_CFG0_MUX_AVDD_AVSS_4, "avdd_div4"),
> +	ADS1120_DIAG_CHANNEL(ADS1120_CFG0_MUX_SHORTED, "shorted"),
> +};

> +
> +/* Regmap read function for ADS1120 */
> +static int ads1120_regmap_read(void *context, const void *reg_buf,
> +			       size_t reg_size, void *val_buf, size_t val_size)
> +{
> +	struct ads1120_state *st = context;
> +	u8 reg = *(u8 *)reg_buf;
> +	u8 *val = val_buf;
> +	int ret;
> +	struct spi_transfer xfer[2] = {
> +		{
> +			.tx_buf = st->data,
> +			.len = 1,
> +		}, {
> +			.rx_buf = val,
> +			.len = val_size,
> +		}
> +	};
> +
> +	if (reg > ADS1120_REG_CONFIG3)
> +		return -EINVAL;
> +
> +	/* RREG command: 0010rr00 where rr is register address */
> +	st->data[0] = ADS1120_CMD_RREG | (reg << 2);
> +
> +	ret = spi_sync_transfer(st->spi, xfer, ARRAY_SIZE(xfer));

return spi_sync_transfer()

> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}

> +
> +static const struct regmap_config ads1120_regmap_config = {
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +	.max_register = ADS1120_REG_CONFIG3,
> +	.cache_type = REGCACHE_FLAT,

Andy covered this already but unless you have strong reason for
something else just use REGCACHE_MAPLE.  If you do have a reason
add a comment here to stop it being changed by someone else.

> +};

> +
> +static int ads1120_probe(struct spi_device *spi)
> +{

> +	indio_dev->name = "ads1120";
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->channels = ads1120_channels;
> +	indio_dev->num_channels = ARRAY_SIZE(ads1120_channels);
> +	indio_dev->info = &ads1120_info;
> +
> +	ret = ads1120_init(st);
> +	if (ret)
> +		return dev_err_probe(dev, ret,
> +					"Failed to initialize device\n");
Align as.
		return dev_err_probe(dev, ret,
				     "Failed to initialize device\n");

Same for all other similar cases.


> +
> +	return devm_iio_device_register(dev, indio_dev);
> +}

Re: [PATCH v2 2/2] iio: adc: Add support for TI ADS1120

[David Lechner]

On 11/9/25 8:11 AM, Ajith Anandhan wrote:
> Add driver for the Texas Instruments ADS1120, a precision 16-bit
> analog-to-digital converter with an SPI interface.
> 

...

> +#define ADS1120_CFG0_GAIN_MASK		GENMASK(3, 1)
> +#define ADS1120_CFG0_GAIN_1		0
> +#define ADS1120_CFG0_GAIN_2		1
> +#define ADS1120_CFG0_GAIN_4		2
> +#define ADS1120_CFG0_GAIN_8		3
> +#define ADS1120_CFG0_GAIN_16		4
> +#define ADS1120_CFG0_GAIN_32		5
> +#define ADS1120_CFG0_GAIN_64		6
> +#define ADS1120_CFG0_GAIN_128		7

We could avoid these macros by just doing ilog2(gain).

> +
> +#define ADS1120_CFG0_PGA_BYPASS		BIT(0)
> +
> +/* Config Register 1 bit definitions */
> +#define ADS1120_CFG1_DR_MASK		GENMASK(7, 5)
> +#define ADS1120_CFG1_DR_20SPS		0
> +#define ADS1120_CFG1_DR_45SPS		1
> +#define ADS1120_CFG1_DR_90SPS		2
> +#define ADS1120_CFG1_DR_175SPS		3
> +#define ADS1120_CFG1_DR_330SPS		4
> +#define ADS1120_CFG1_DR_600SPS		5
> +#define ADS1120_CFG1_DR_1000SPS		6

I would avoid writing macros for values we don't use yet. For example,
it may make more sense to have a lookup table when it comes to actually
implementing something that uses this.

Same applies to the rest below.

> +
> +#define ADS1120_CFG1_MODE_MASK		GENMASK(4, 3)
> +#define ADS1120_CFG1_MODE_NORMAL	0
> +#define ADS1120_CFG1_MODE_DUTY		1
> +#define ADS1120_CFG1_MODE_TURBO		2
> +
> +#define ADS1120_CFG1_CM_MASK		BIT(2)
> +#define ADS1120_CFG1_CM_SINGLE		0
> +#define ADS1120_CFG1_CM_CONTINUOUS	1
> +
> +#define ADS1120_CFG1_TS_EN		BIT(1)
> +#define ADS1120_CFG1_BCS_EN		BIT(0)
> +
> +/* Config Register 2 bit definitions */
> +#define ADS1120_CFG2_VREF_MASK		GENMASK(7, 6)
> +#define ADS1120_CFG2_VREF_INTERNAL	0
> +#define ADS1120_CFG2_VREF_EXT_REFP0	1
> +#define ADS1120_CFG2_VREF_EXT_AIN0	2
> +#define ADS1120_CFG2_VREF_AVDD		3
> +
> +#define ADS1120_CFG2_REJECT_MASK	GENMASK(5, 4)
> +#define ADS1120_CFG2_REJECT_OFF		0
> +#define ADS1120_CFG2_REJECT_50_60	1
> +#define ADS1120_CFG2_REJECT_50		2
> +#define ADS1120_CFG2_REJECT_60		3
> +
> +#define ADS1120_CFG2_PSW_EN		BIT(3)
> +
> +#define ADS1120_CFG2_IDAC_MASK		GENMASK(2, 0)
> +#define ADS1120_CFG2_IDAC_OFF		0
> +#define ADS1120_CFG2_IDAC_10UA		1
> +#define ADS1120_CFG2_IDAC_50UA		2
> +#define ADS1120_CFG2_IDAC_100UA		3
> +#define ADS1120_CFG2_IDAC_250UA		4
> +#define ADS1120_CFG2_IDAC_500UA		5
> +#define ADS1120_CFG2_IDAC_1000UA	6
> +#define ADS1120_CFG2_IDAC_1500UA	7
> +
> +/* Config Register 3 bit definitions */
> +#define ADS1120_CFG3_IDAC1_MASK		GENMASK(7, 5)
> +#define ADS1120_CFG3_IDAC1_DISABLED	0
> +#define ADS1120_CFG3_IDAC1_AIN0		1
> +#define ADS1120_CFG3_IDAC1_AIN1		2
> +#define ADS1120_CFG3_IDAC1_AIN2		3
> +#define ADS1120_CFG3_IDAC1_AIN3		4
> +#define ADS1120_CFG3_IDAC1_REFP0	5
> +#define ADS1120_CFG3_IDAC1_REFN0	6
> +
> +#define ADS1120_CFG3_IDAC2_MASK		GENMASK(4, 2)
> +#define ADS1120_CFG3_IDAC2_DISABLED	0
> +#define ADS1120_CFG3_IDAC2_AIN0		1
> +#define ADS1120_CFG3_IDAC2_AIN1		2
> +#define ADS1120_CFG3_IDAC2_AIN2		3
> +#define ADS1120_CFG3_IDAC2_AIN3		4
> +#define ADS1120_CFG3_IDAC2_REFP0	5
> +#define ADS1120_CFG3_IDAC2_REFN0	6
> +
> +#define ADS1120_CFG3_DRDYM_MASK		BIT(1)
> +#define ADS1120_CFG3_DRDYM_DRDY_ONLY	0
> +#define ADS1120_CFG3_DRDYM_BOTH		1
> +
> +/* Conversion time for 20 SPS */
> +#define ADS1120_CONV_TIME_MS		51
> +
> +/* Internal reference voltage in millivolts */
> +#define ADS1120_VREF_INTERNAL_MV	2048
> +
> +
> +struct ads1120_state {
> +	struct spi_device	*spi;
> +	struct regmap		*regmap;
> +	/*
> +	 * Protects chip configuration and ADC reads to ensure
> +	 * consistent channel/gain settings during conversions.
> +	 */
> +	struct mutex		lock;
> +
> +	int vref_mv;
> +
> +	/* DMA-safe buffer for SPI transfers */
> +	u8 data[4] __aligned(IIO_DMA_MINALIGN);
> +};
> +
> +
> +static const int ads1120_gain_values[] = { 1, 2, 4, 8, 16, 32, 64, 128 };
> +static const int ads1120_low_gain_values[] = { 1, 2, 4 };
> +
> +/* Differential channel macro */
> +#define ADS1120_DIFF_CHANNEL(index, chan1, chan2)		\
> +{								\
> +	.type = IIO_VOLTAGE,					\
> +	.indexed = 1,						\
> +	.channel = chan1,					\
> +	.channel2 = chan2,					\
> +	.differential = 1,					\
> +	.address = index,					\
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |		\
> +			      BIT(IIO_CHAN_INFO_SCALE),		\
> +	.info_mask_separate_available = BIT(IIO_CHAN_INFO_SCALE), \
> +}
> +
> +/* Single-ended channel macro */
> +#define ADS1120_SINGLE_CHANNEL(index, chan)			\
> +{								\
> +	.type = IIO_VOLTAGE,					\
> +	.indexed = 1,						\
> +	.channel = chan,					\
> +	.address = index,					\
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |		\
> +			      BIT(IIO_CHAN_INFO_SCALE),		\
> +	.info_mask_separate_available = BIT(IIO_CHAN_INFO_SCALE), \
> +}
> +
> +/* Diagnostic channel macro */
> +#define ADS1120_DIAG_CHANNEL(index, label)			\
> +{								\
> +	.type = IIO_VOLTAGE,					\
> +	.indexed = 1,						\
> +	.channel = index,					\
> +	.address = index,					\
> +	.extend_name = label,					\
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |		\
> +			      BIT(IIO_CHAN_INFO_SCALE),		\
> +	.info_mask_separate_available = BIT(IIO_CHAN_INFO_SCALE), \
> +}
> +
> +static const struct iio_chan_spec ads1120_channels[] = {
> +	/* Differential inputs */
> +	ADS1120_DIFF_CHANNEL(ADS1120_CFG0_MUX_AIN0_AIN1, 0, 1),
> +	ADS1120_DIFF_CHANNEL(ADS1120_CFG0_MUX_AIN0_AIN2, 0, 2),
> +	ADS1120_DIFF_CHANNEL(ADS1120_CFG0_MUX_AIN0_AIN3, 0, 3),
> +	ADS1120_DIFF_CHANNEL(ADS1120_CFG0_MUX_AIN1_AIN2, 1, 2),
> +	ADS1120_DIFF_CHANNEL(ADS1120_CFG0_MUX_AIN1_AIN3, 1, 3),
> +	ADS1120_DIFF_CHANNEL(ADS1120_CFG0_MUX_AIN2_AIN3, 2, 3),
> +	ADS1120_DIFF_CHANNEL(ADS1120_CFG0_MUX_AIN1_AIN0, 1, 0),
> +	ADS1120_DIFF_CHANNEL(ADS1120_CFG0_MUX_AIN3_AIN2, 3, 2),
> +	/* Single-ended inputs */
> +	ADS1120_SINGLE_CHANNEL(ADS1120_CFG0_MUX_AIN0_AVSS, 0),
> +	ADS1120_SINGLE_CHANNEL(ADS1120_CFG0_MUX_AIN1_AVSS, 1),
> +	ADS1120_SINGLE_CHANNEL(ADS1120_CFG0_MUX_AIN2_AVSS, 2),
> +	ADS1120_SINGLE_CHANNEL(ADS1120_CFG0_MUX_AIN3_AVSS, 3),
> +	/* Diagnostic inputs */
> +	ADS1120_DIAG_CHANNEL(ADS1120_CFG0_MUX_REFP_REFN_4, "ref_div4"),
> +	ADS1120_DIAG_CHANNEL(ADS1120_CFG0_MUX_AVDD_AVSS_4, "avdd_div4"),
> +	ADS1120_DIAG_CHANNEL(ADS1120_CFG0_MUX_SHORTED, "shorted"),
> +};

Usually we don't make macros for the index. When it comes to adding
buffer support, having the macros makes it harder to see which scan_index
belongs to which channel. And if buffer support is planned for later,
it would make sense to use .scan_index instead of .address to avoid
duplicating that info later.


> +static int ads1120_set_gain(struct ads1120_state *st, int gain_val)
> +{
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(ads1120_gain_values); i++) {
> +		if (ads1120_gain_values[i] == gain_val)
> +			break;
> +	}
> +
> +	if (i == ARRAY_SIZE(ads1120_gain_values))
> +		return -EINVAL;

Since there is only one gain register, we should store this in a
per-channel variable, then set the gain in the register in
ads1120_set_mux() instead (and it would make sense to rename
that function as well to something like ads1120_configure_channel()).

> +
> +	return regmap_update_bits(st->regmap, ADS1120_REG_CONFIG0,
> +				  ADS1120_CFG0_GAIN_MASK,
> +				  FIELD_PREP(ADS1120_CFG0_GAIN_MASK, i));
> +}
> +

...

> +static int ads1120_read_raw(struct iio_dev *indio_dev,
> +			    struct iio_chan_spec const *chan,
> +			    int *val, int *val2, long mask)
> +{
> +	struct ads1120_state *st = iio_priv(indio_dev);
> +	int ret, gain;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		guard(mutex)(&st->lock);
> +		ret = ads1120_read_measurement(st, chan, val);
> +		if (ret)
> +			return ret;
> +		return IIO_VAL_INT;
> +
> +	case IIO_CHAN_INFO_SCALE:
> +		/*
> +		 * Scale = Vref / (gain * 2^15)
> +		 * Return in format: val / 2^val2
> +		 */
> +		gain = ads1120_get_gain(st);
> +		if (gain < 0)
> +			return gain;
> +
> +		*val = st->vref_mv;
> +		*val2 = gain * 15;

I think this would need to be `*val2 = ilog2(gain) + 15`.

> +		return IIO_VAL_FRACTIONAL_LOG2;
> +
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +



> +
> +/* Regmap read function for ADS1120 */
> +static int ads1120_regmap_read(void *context, const void *reg_buf,
> +			       size_t reg_size, void *val_buf, size_t val_size)
> +{
> +	struct ads1120_state *st = context;
> +	u8 reg = *(u8 *)reg_buf;
> +	u8 *val = val_buf;
> +	int ret;
> +	struct spi_transfer xfer[2] = {
> +		{
> +			.tx_buf = st->data,
> +			.len = 1,
> +		}, {
> +			.rx_buf = val,
> +			.len = val_size,
> +		}
> +	};
> +
> +	if (reg > ADS1120_REG_CONFIG3)
> +		return -EINVAL;

This check should not be needed because of .maxregister.

> +
> +	/* RREG command: 0010rr00 where rr is register address */
> +	st->data[0] = ADS1120_CMD_RREG | (reg << 2);
> +
> +	ret = spi_sync_transfer(st->spi, xfer, ARRAY_SIZE(xfer));
> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +/* Regmap write function for ADS1120 */
> +static int ads1120_regmap_write(void *context, const void *data, size_t count)
> +{
> +	struct ads1120_state *st = context;
> +	const u8 *buf = data;
> +
> +	if (count != 2)
> +		return -EINVAL;
> +
> +	/* WREG command: 0100rr00 where rr is register address */
> +	st->data[0] = ADS1120_CMD_WREG | (buf[0] << 2);
> +	st->data[1] = buf[1];
> +
> +	return spi_write(st->spi, st->data, 2);
> +}

I don't see anyting unusal about these read/write functions. We should
be able to use the existing spi_regmap with the proper configuration
instead of making a custom regmap_bus.

> +
> +static const struct regmap_bus ads1120_regmap_bus = {
> +	.read = ads1120_regmap_read,
> +	.write = ads1120_regmap_write,
> +	.reg_format_endian_default = REGMAP_ENDIAN_BIG,
> +	.val_format_endian_default = REGMAP_ENDIAN_BIG,
> +};
> +
> +static const struct regmap_config ads1120_regmap_config = {
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +	.max_register = ADS1120_REG_CONFIG3,
> +	.cache_type = REGCACHE_FLAT,
> +};
> +
> +static int ads1120_init(struct ads1120_state *st)
> +{
> +	int ret;

It's just a few extra lines to turn on the avdd supply here before
resettting.

> +
> +	ret = ads1120_reset(st);
> +	if (ret)
> +		return dev_err_probe(&st->spi->dev, ret,
> +					"Failed to reset device\n");
> +
> +	/*
> +	 * Configure Register 0:
> +	 * - Input MUX: AIN0/AVSS
> +	 * - Gain: 1
> +	 * - PGA bypass enabled. When gain is set > 4, this bit is
> +	 *   automatically ignored by the hardware and PGA is enabled,
> +	 *   so it's safe to leave it set.
> +	 */
> +	ret = regmap_write(st->regmap, ADS1120_REG_CONFIG0,
> +			   FIELD_PREP(ADS1120_CFG0_MUX_MASK,
> +				      ADS1120_CFG0_MUX_AIN0_AVSS) |
> +			   FIELD_PREP(ADS1120_CFG0_GAIN_MASK,
> +				      ADS1120_CFG0_GAIN_1) |
> +			   ADS1120_CFG0_PGA_BYPASS);
> +	if (ret)
> +		return ret;
> +
> +	/*
> +	 * Configure Register 1:
> +	 * - Data rate: 20 SPS (for single-shot mode)
> +	 * - Operating mode: Normal
> +	 * - Conversion mode: Single-shot
> +	 * - Temperature sensor: Disabled
> +	 * - Burnout current: Disabled
> +	 */
> +	ret = regmap_write(st->regmap, ADS1120_REG_CONFIG1,
> +			   FIELD_PREP(ADS1120_CFG1_DR_MASK,
> +				      ADS1120_CFG1_DR_20SPS) |
> +			   FIELD_PREP(ADS1120_CFG1_MODE_MASK,
> +				      ADS1120_CFG1_MODE_NORMAL) |
> +			   FIELD_PREP(ADS1120_CFG1_CM_MASK,
> +				      ADS1120_CFG1_CM_SINGLE) |
> +			   FIELD_PREP(ADS1120_CFG1_TS_EN, 0) |
> +			   FIELD_PREP(ADS1120_CFG1_BCS_EN, 0));
> +	if (ret)
> +		return ret;
> +
> +	/*
> +	 * Configure Register 2:
> +	 * - Voltage reference: Internal 2.048V
> +	 * - 50/60Hz rejection: Off
> +	 * - Power switch: Disabled
> +	 * - IDAC current: Off
> +	 */
> +	ret = regmap_write(st->regmap, ADS1120_REG_CONFIG2,
> +			   FIELD_PREP(ADS1120_CFG2_VREF_MASK,
> +				      ADS1120_CFG2_VREF_INTERNAL) |
> +			   FIELD_PREP(ADS1120_CFG2_REJECT_MASK,
> +				      ADS1120_CFG2_REJECT_OFF) |
> +			   FIELD_PREP(ADS1120_CFG2_PSW_EN, 0) |
> +			   FIELD_PREP(ADS1120_CFG2_IDAC_MASK,
> +				      ADS1120_CFG2_IDAC_OFF));
> +	if (ret)
> +		return ret;
> +
> +	/*
> +	 * Configure Register 3:
> +	 * - IDAC1: Disabled
> +	 * - IDAC2: Disabled
> +	 * - DRDY mode: Only reflects data ready status
> +	 */
> +	ret = regmap_write(st->regmap, ADS1120_REG_CONFIG3,
> +			   FIELD_PREP(ADS1120_CFG3_IDAC1_MASK,
> +				      ADS1120_CFG3_IDAC1_DISABLED) |
> +			   FIELD_PREP(ADS1120_CFG3_IDAC2_MASK,
> +				      ADS1120_CFG3_IDAC2_DISABLED) |
> +			   FIELD_PREP(ADS1120_CFG3_DRDYM_MASK,
> +				      ADS1120_CFG3_DRDYM_DRDY_ONLY));
> +	if (ret)
> +		return ret;
> +

If we want to wait for a later patch to support external reference,
I would consider to check for the devicetree properties here and
print a warning that they aren't supported if present. Maybe others
think that is too much noise though? Especially if you plan to add
it soon.

> +	st->vref_mv = ADS1120_VREF_INTERNAL_MV;
> +
> +	return 0;
> +}
> +