[PATCH v2 0/2] iio: adc: Add support for Texas Instruments ADS112C04

[PATCH v2 0/2] iio: adc: Add support for Texas Instruments ADS112C04

[Kyle Hsieh]

This patch series introduces support for the Texas Instruments ADS112C04
Analog-to-Digital Converters.

The ADS112C04 (16-bit) is precision, low-power, delta-sigma ADCs with
an I2C interface. They feature a flexible input multiplexer supporting
single-ended and differential measurements, a programmable gain amplifier,
and an internal voltage reference.

Note: While this chip shares similarities with the ADS112C14 (currently 
being upstreamed by David Lechner), the register maps and feature sets 
are sufficiently different to warrant a separate driver. However, the
DT bindings and channel parsing logic have been aligned with the
ADS112C14 conventions.

This initial submission provides a minimal feature set (single-shot 
conversions and basic DRDY interrupt) covering current use cases.

Signed-off-by: Kyle Hsieh <kylehsieh1995@gmail.com>
---
Changes in v2:
- Replaced `vref-supply` with `refp-supply` and `refn-supply` to accurately reflect hardware.
- Refactored the driver to dynamically parse channel configurations and routing from DT child nodes.
- Modernized the driver using kernel macros.
- Handled endianness elegantly.
- Added hardware reset fallback logic.
- Inherited IRQ trigger type from device tree instead of hardcoding.
- Fixed a bug where the MUX software cache could desync from hardware if the I2C write failed.
- Added strict return value checking for all I2C writes during probe.
- Updated the `i2c_device_id` array to use C99 named initializers.
- Link to v1: https://lore.kernel.org/r/20260728-ti-ads112c04-driver-v1-0-475efe4e2b78@gmail.com

---
Kyle Hsieh (2):
      dt-bindings: iio: adc: ti,ads112c04: Add binding for ADS112C04
      iio: adc: ti-ads112c04: Add support for TI ADS112C04

 .../devicetree/bindings/iio/adc/ti,ads112c04.yaml  | 122 +++++++
 drivers/iio/adc/Kconfig                            |  10 +
 drivers/iio/adc/Makefile                           |   1 +
 drivers/iio/adc/ti-ads112c04.c                     | 378 +++++++++++++++++++++
 4 files changed, 511 insertions(+)
---
base-commit: 4539944e515183668109bdf4d0c3d7d228383d88
change-id: 20260724-ti-ads112c04-driver-be7e89047834

Best regards,
-- 
Kyle Hsieh <kylehsieh1995@gmail.com>

[PATCH v2 1/2] dt-bindings: iio: adc: ti,ads112c04: Add binding for ADS112C04

[Kyle Hsieh]

Add device tree binding documentation for Texas Instruments ADS112C04
I2C Analog-to-Digital Converters.

These devices provide 4-channel, 16-bit delta-sigma ADCs with an I2C
interface, programmable gain amplifier (PGA), and data-ready (DRDY)
interrupt output.

The binding uses child nodes to dynamically define the connected
single-ended or differential channels.

Signed-off-by: Kyle Hsieh <kylehsieh1995@gmail.com>
---
 .../devicetree/bindings/iio/adc/ti,ads112c04.yaml  | 122 +++++++++++++++++++++
 1 file changed, 122 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/adc/ti,ads112c04.yaml b/Documentation/devicetree/bindings/iio/adc/ti,ads112c04.yaml
new file mode 100644
index 000000000000..6a5ffda84b80
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/ti,ads112c04.yaml
@@ -0,0 +1,122 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/adc/ti,ads112c04.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Texas Instruments ADS112C04 ADC
+
+maintainers:
+  - Kyle Hsieh <kylehsieh1995@gmail.com>
+
+description: |
+  The ADS112C04 (16-bit) are precision analog-to-digital converters (ADCs)
+  with an I2C interface. They feature a flexible input multiplexer, a
+  low-noise programmable gain amplifier (PGA), two programmable excitation
+  current sources, a voltage reference, and a precision temperature sensor.
+
+properties:
+  compatible:
+    enum:
+      - ti,ads112c04
+
+  reg:
+    maxItems: 1
+    description: I2C address of the device.
+
+  interrupts:
+    maxItems: 1
+    description: Data ready (DRDY) interrupt output.
+
+  "#address-cells":
+    const: 1
+
+  "#size-cells":
+    const: 0
+
+  reset-gpios:
+    maxItems: 1
+    description: GPIO connected to the RESET pin. Active low.
+
+  avdd-supply: true
+  dvdd-supply: true
+
+  refp-supply: true
+  refn-supply: true
+
+  ti,refp-refn-resistor-ohms:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: Resistance of the external resistor between REFP and REFN.
+
+patternProperties:
+  "^channel@[0-9a-f]$":
+    $ref: adc.yaml
+    unevaluatedProperties: false
+    properties:
+      reg:
+        items:
+          - maximum: 15
+
+      single-channel:
+        maximum: 3
+
+      diff-channels:
+        items:
+          maximum: 3
+
+    oneOf:
+      - required: [ single-channel ]
+      - required: [ diff-channels ]
+
+required:
+  - compatible
+  - reg
+  - avdd-supply
+  - dvdd-supply
+
+dependencies:
+  refn-supply: [ refp-supply ]
+
+oneOf:
+  - required: [ refp-supply ]
+  - required: [ "ti,refp-refn-resistor-ohms" ]
+  - properties:
+      refp-supply: false
+      refn-supply: false
+      ti,refp-refn-resistor-ohms: false
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/gpio/gpio.h>
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        adc@40 {
+            compatible = "ti,ads112c04";
+            reg = <0x40>;
+            interrupt-parent = <&gpio>;
+            interrupts = <12 IRQ_TYPE_EDGE_FALLING>;
+
+            reset-gpios = <&gpio 13 GPIO_ACTIVE_LOW>;
+            avdd-supply = <&vdd_3v3_reg>;
+            dvdd-supply = <&vdd_3v3_reg>;
+            refp-supply = <&vref_reg>;
+
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            channel@0 {
+                reg = <0>;
+                diff-channels = <0>, <1>;
+            };
+
+            channel@1 {
+                reg = <1>;
+                single-channel = <2>;
+            };
+        };
+    };

-- 
2.34.1

Re: [PATCH v2 1/2] dt-bindings: iio: adc: ti,ads112c04: Add binding for ADS112C04

[Rob Herring (Arm)]

On Fri, 31 Jul 2026 10:58:24 +0800, Kyle Hsieh wrote:
> Add device tree binding documentation for Texas Instruments ADS112C04
> I2C Analog-to-Digital Converters.
> 
> These devices provide 4-channel, 16-bit delta-sigma ADCs with an I2C
> interface, programmable gain amplifier (PGA), and data-ready (DRDY)
> interrupt output.
> 
> The binding uses child nodes to dynamically define the connected
> single-ended or differential channels.
> 
> Signed-off-by: Kyle Hsieh <kylehsieh1995@gmail.com>
> ---
>  .../devicetree/bindings/iio/adc/ti,ads112c04.yaml  | 122 +++++++++++++++++++++
>  1 file changed, 122 insertions(+)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/adc/ti,ads112c04.yaml: properties:ti,refp-refn-resistor-ohms: '$ref' should not be valid under {'const': '$ref'}
	hint: Standard unit suffix properties don't need a type $ref
	from schema $id: http://devicetree.org/meta-schemas/core.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.kernel.org/project/devicetree/patch/20260731-ti-ads112c04-driver-v2-1-aab0168c3c01@gmail.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [PATCH v2 1/2] dt-bindings: iio: adc: ti,ads112c04: Add binding for ADS112C04

[David Lechner]

On 7/30/26 9:58 PM, Kyle Hsieh wrote:
> Add device tree binding documentation for Texas Instruments ADS112C04
> I2C Analog-to-Digital Converters.
> 
> These devices provide 4-channel, 16-bit delta-sigma ADCs with an I2C
> interface, programmable gain amplifier (PGA), and data-ready (DRDY)
> interrupt output.
> 
> The binding uses child nodes to dynamically define the connected
> single-ended or differential channels.
> 
> Signed-off-by: Kyle Hsieh <kylehsieh1995@gmail.com>
> ---
>  .../devicetree/bindings/iio/adc/ti,ads112c04.yaml  | 122 +++++++++++++++++++++
>  1 file changed, 122 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/ti,ads112c04.yaml b/Documentation/devicetree/bindings/iio/adc/ti,ads112c04.yaml
> new file mode 100644
> index 000000000000..6a5ffda84b80
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/ti,ads112c04.yaml
> @@ -0,0 +1,122 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/adc/ti,ads112c04.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Texas Instruments ADS112C04 ADC
> +
> +maintainers:
> +  - Kyle Hsieh <kylehsieh1995@gmail.com>
> +
> +description: |

The | isn't needed in this case. (It is only needed if we needed
to preserve formatting, or if there is a : in the text that would
be interpreted as a dict key.)

> +  The ADS112C04 (16-bit) are precision analog-to-digital converters (ADCs)
> +  with an I2C interface. They feature a flexible input multiplexer, a
> +  low-noise programmable gain amplifier (PGA), two programmable excitation
> +  current sources, a voltage reference, and a precision temperature sensor.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - ti,ads112c04
> +
> +  reg:
> +    maxItems: 1
> +    description: I2C address of the device.
> +
> +  interrupts:
> +    maxItems: 1
> +    description: Data ready (DRDY) interrupt output.
> +
> +  "#address-cells":
> +    const: 1
> +
> +  "#size-cells":
> +    const: 0
> +
> +  reset-gpios:
> +    maxItems: 1
> +    description: GPIO connected to the RESET pin. Active low.
> +
> +  avdd-supply: true
> +  dvdd-supply: true
> +
> +  refp-supply: true
> +  refn-supply: true
> +
> +  ti,refp-refn-resistor-ohms:
> +    $ref: /schemas/types.yaml#/definitions/uint32

As the bot said, don't need $ref here because it is a standard unit.
(So don't forget to run `make dt_binding_check DT_SCHEMA_FILES=ads112c04`
before sending.)

> +    description: Resistance of the external resistor between REFP and REFN.
> +
> +patternProperties:
> +  "^channel@[0-9a-f]$":
> +    $ref: adc.yaml
> +    unevaluatedProperties: false
> +    properties:
> +      reg:
> +        items:
> +          - maximum: 15

This chip doesn't allow arbitrary combinations, so there
are actually only 12 possible single-channel + diff-channels
values. So maximum: 11 and channel@[0-9a-b] would make sense.

> +
> +      single-channel:
> +        maximum: 3
> +
> +      diff-channels:
> +        items:
> +          maximum: 3

For this chip, we should also have excitation-channels, excitation-current-nanoamp,
and reference-sources.

> +
> +    oneOf:
> +      - required: [ single-channel ]
> +      - required: [ diff-channels ]
> +

Re: [PATCH v2 1/2] dt-bindings: iio: adc: ti,ads112c04: Add binding for ADS112C04

[David Lechner]

On 7/31/26 8:52 AM, David Lechner wrote:
> On 7/30/26 9:58 PM, Kyle Hsieh wrote:
>> Add device tree binding documentation for Texas Instruments ADS112C04
>> I2C Analog-to-Digital Converters.
>>
...

>> +
>> +      single-channel:
>> +        maximum: 3
>> +
>> +      diff-channels:
>> +        items:
>> +          maximum: 3
> 
> For this chip, we should also have excitation-channels, excitation-current-nanoamp,
> and reference-sources.

And burn-out-current-nanoamp.

> 
>> +
>> +    oneOf:
>> +      - required: [ single-channel ]
>> +      - required: [ diff-channels ]
>> +

Re: [PATCH v2 1/2] dt-bindings: iio: adc: ti,ads112c04: Add binding for ADS112C04

[Kyle Hsieh]

On Fri, Jul 31, 2026 at 9:52 PM David Lechner <dlechner@baylibre.com> wrote:
>
> On 7/30/26 9:58 PM, Kyle Hsieh wrote:
> > Add device tree binding documentation for Texas Instruments ADS112C04
> > I2C Analog-to-Digital Converters.
> >
> > These devices provide 4-channel, 16-bit delta-sigma ADCs with an I2C
> > interface, programmable gain amplifier (PGA), and data-ready (DRDY)
> > interrupt output.
> >
> > The binding uses child nodes to dynamically define the connected
> > single-ended or differential channels.
> >
> > Signed-off-by: Kyle Hsieh <kylehsieh1995@gmail.com>
> > ---
> >  .../devicetree/bindings/iio/adc/ti,ads112c04.yaml  | 122 +++++++++++++++++++++
> >  1 file changed, 122 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/iio/adc/ti,ads112c04.yaml b/Documentation/devicetree/bindings/iio/adc/ti,ads112c04.yaml
> > new file mode 100644
> > index 000000000000..6a5ffda84b80
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/adc/ti,ads112c04.yaml
> > @@ -0,0 +1,122 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/iio/adc/ti,ads112c04.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Texas Instruments ADS112C04 ADC
> > +
> > +maintainers:
> > +  - Kyle Hsieh <kylehsieh1995@gmail.com>
> > +
> > +description: |
>
> The | isn't needed in this case. (It is only needed if we needed
> to preserve formatting, or if there is a : in the text that would
> be interpreted as a dict key.)
Understood. I will remove the `|` from in v3.
>
> > +  The ADS112C04 (16-bit) are precision analog-to-digital converters (ADCs)
> > +  with an I2C interface. They feature a flexible input multiplexer, a
> > +  low-noise programmable gain amplifier (PGA), two programmable excitation
> > +  current sources, a voltage reference, and a precision temperature sensor.
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - ti,ads112c04
> > +
> > +  reg:
> > +    maxItems: 1
> > +    description: I2C address of the device.
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +    description: Data ready (DRDY) interrupt output.
> > +
> > +  "#address-cells":
> > +    const: 1
> > +
> > +  "#size-cells":
> > +    const: 0
> > +
> > +  reset-gpios:
> > +    maxItems: 1
> > +    description: GPIO connected to the RESET pin. Active low.
> > +
> > +  avdd-supply: true
> > +  dvdd-supply: true
> > +
> > +  refp-supply: true
> > +  refn-supply: true
> > +
> > +  ti,refp-refn-resistor-ohms:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
>
> As the bot said, don't need $ref here because it is a standard unit.
> (So don't forget to run `make dt_binding_check DT_SCHEMA_FILES=ads112c04`
> before sending.)
Yes, the `dt_binding_check` robot caught this right away. I will
removed the redundant `$ref` and resolved the warning locally.
>
> > +    description: Resistance of the external resistor between REFP and REFN.
> > +
> > +patternProperties:
> > +  "^channel@[0-9a-f]$":
> > +    $ref: adc.yaml
> > +    unevaluatedProperties: false
> > +    properties:
> > +      reg:
> > +        items:
> > +          - maximum: 15
>
> This chip doesn't allow arbitrary combinations, so there
> are actually only 12 possible single-channel + diff-channels
> values. So maximum: 11 and channel@[0-9a-b] would make sense.
I will update the regex and set the reg maximum to 11 to strictly reflect
the 12 possible MUX configurations supported by the hardware.
>
> > +
> > +      single-channel:
> > +        maximum: 3
> > +
> > +      diff-channels:
> > +        items:
> > +          maximum: 3
>
> For this chip, we should also have excitation-channels, excitation-current-nanoamp,
> and reference-sources.
I will add these properties to the binding to fully describe
the hardware capabilities.
>
> > +
> > +    oneOf:
> > +      - required: [ single-channel ]
> > +      - required: [ diff-channels ]
> > +

Re: [PATCH v2 1/2] dt-bindings: iio: adc: ti,ads112c04: Add binding for ADS112C04

[David Lechner]

On 8/2/26 9:32 PM, Kyle Hsieh wrote:
> On Fri, Jul 31, 2026 at 9:52 PM David Lechner <dlechner@baylibre.com> wrote:
>>
>> On 7/30/26 9:58 PM, Kyle Hsieh wrote:
>>> Add device tree binding documentation for Texas Instruments ADS112C04
>>> I2C Analog-to-Digital Converters.
>>>
>>> These devices provide 4-channel, 16-bit delta-sigma ADCs with an I2C
>>> interface, programmable gain amplifier (PGA), and data-ready (DRDY)
>>> interrupt output.
>>>
>>> The binding uses child nodes to dynamically define the connected
>>> single-ended or differential channels.
>>>
>>> Signed-off-by: Kyle Hsieh <kylehsieh1995@gmail.com>
>>> ---
>>>  .../devicetree/bindings/iio/adc/ti,ads112c04.yaml  | 122 +++++++++++++++++++++
>>>  1 file changed, 122 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/iio/adc/ti,ads112c04.yaml b/Documentation/devicetree/bindings/iio/adc/ti,ads112c04.yaml
>>> new file mode 100644
>>> index 000000000000..6a5ffda84b80
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/iio/adc/ti,ads112c04.yaml
>>> @@ -0,0 +1,122 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/iio/adc/ti,ads112c04.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Texas Instruments ADS112C04 ADC
>>> +
>>> +maintainers:
>>> +  - Kyle Hsieh <kylehsieh1995@gmail.com>
>>> +
>>> +description: |
>>
>> The | isn't needed in this case. (It is only needed if we needed
>> to preserve formatting, or if there is a : in the text that would
>> be interpreted as a dict key.)
> Understood. I will remove the `|` from in v3.
>>
>>> +  The ADS112C04 (16-bit) are precision analog-to-digital converters (ADCs)
>>> +  with an I2C interface. They feature a flexible input multiplexer, a
>>> +  low-noise programmable gain amplifier (PGA), two programmable excitation
>>> +  current sources, a voltage reference, and a precision temperature sensor.
>>> +
>>> +properties:
>>> +  compatible:
>>> +    enum:
>>> +      - ti,ads112c04
>>> +
>>> +  reg:
>>> +    maxItems: 1
>>> +    description: I2C address of the device.
>>> +
>>> +  interrupts:
>>> +    maxItems: 1
>>> +    description: Data ready (DRDY) interrupt output.
>>> +
>>> +  "#address-cells":
>>> +    const: 1
>>> +
>>> +  "#size-cells":
>>> +    const: 0
>>> +
>>> +  reset-gpios:
>>> +    maxItems: 1
>>> +    description: GPIO connected to the RESET pin. Active low.
>>> +
>>> +  avdd-supply: true
>>> +  dvdd-supply: true
>>> +
>>> +  refp-supply: true
>>> +  refn-supply: true
>>> +
>>> +  ti,refp-refn-resistor-ohms:
>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>
>> As the bot said, don't need $ref here because it is a standard unit.
>> (So don't forget to run `make dt_binding_check DT_SCHEMA_FILES=ads112c04`
>> before sending.)
> Yes, the `dt_binding_check` robot caught this right away. I will
> removed the redundant `$ref` and resolved the warning locally.
>>
>>> +    description: Resistance of the external resistor between REFP and REFN.
>>> +
>>> +patternProperties:
>>> +  "^channel@[0-9a-f]$":
>>> +    $ref: adc.yaml
>>> +    unevaluatedProperties: false
>>> +    properties:
>>> +      reg:
>>> +        items:
>>> +          - maximum: 15
>>
>> This chip doesn't allow arbitrary combinations, so there
>> are actually only 12 possible single-channel + diff-channels
>> values. So maximum: 11 and channel@[0-9a-b] would make sense.
> I will update the regex and set the reg maximum to 11 to strictly reflect
> the 12 possible MUX configurations supported by the hardware.
>>
>>> +
>>> +      single-channel:
>>> +        maximum: 3
>>> +
>>> +      diff-channels:
>>> +        items:
>>> +          maximum: 3
>>
>> For this chip, we should also have excitation-channels, excitation-current-nanoamp,
>> and reference-sources.
> I will add these properties to the binding to fully describe
> the hardware capabilities.
>>
>>> +
>>> +    oneOf:
>>> +      - required: [ single-channel ]
>>> +      - required: [ diff-channels ]
>>> +

In the future, you don't need to reply to comments you agree with.
Just reply to ones you don't agree with or have further questions
on. We have way too many mailing lists emails to read, so anything
we can cut out helps.

[PATCH v2 2/2] iio: adc: ti-ads112c04: Add support for TI ADS112C04

[Kyle Hsieh]

Add IIO driver support for the Texas Instruments ADS112C04 (16-bit)
delta-sigma ADCs.

The driver implements:
- Single-shot conversions using the IIO raw read interface.
- Dynamic parsing of single-ended and differential channels from
  device tree child nodes.
- Hardware interrupt support via the DRDY pin, falling back to
  software polling if no IRQ is provided.
- Scale calculation based on the internal 2.048V reference.
- Dynamic reference voltage scaling via the regulator subsystem
  (refp-supply/refn-supply), falling back to the internal 2.048V
  reference if not specified.
- Hardware reset fallback using GPIO.

Signed-off-by: Kyle Hsieh <kylehsieh1995@gmail.com>
---
 drivers/iio/adc/Kconfig        |  10 ++
 drivers/iio/adc/Makefile       |   1 +
 drivers/iio/adc/ti-ads112c04.c | 378 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 389 insertions(+)

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 3755a81c1efd..402e841bc083 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -1789,6 +1789,16 @@ config TI_ADS1119
          This driver can also be built as a module. If so, the module will be
          called ti-ads1119.
 
+config TI_ADS112C04
+	tristate "Texas Instruments ADS112C04 ADC"
+	depends on I2C
+	help
+	  If you say yes here you get support for Texas Instruments
+	  ADS112C04 (16-bit) I2C analog to digital converters.
+
+	  This driver can also be built as a module. If so, the module will be
+	  called ti-ads112c04.
+
 config TI_ADS124S08
 	tristate "Texas Instruments ADS124S08"
 	depends on SPI
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index 707dd708912f..ebf9d4047a5a 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_ADS112C04)	+= ti-ads112c04.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-ads112c04.c b/drivers/iio/adc/ti-ads112c04.c
new file mode 100644
index 000000000000..28d3be81934f
--- /dev/null
+++ b/drivers/iio/adc/ti-ads112c04.c
@@ -0,0 +1,378 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Texas Instruments ADS112C04 16-bit I2C ADC driver
+ * Based on TI Reference Code and standard Linux IIO framework.
+ */
+
+#include <linux/module.h>
+#include <linux/i2c.h>
+#include <linux/delay.h>
+#include <linux/interrupt.h>
+#include <linux/mutex.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+#include <linux/regulator/consumer.h>
+#include <linux/bitfield.h>
+#include <linux/iopoll.h>
+#include <linux/property.h>
+#include <linux/gpio/consumer.h>
+
+/* ADS112C04 Commands */
+#define ADS112C04_CMD_RESET         0x06
+#define ADS112C04_CMD_START_SYNC    0x08
+#define ADS112C04_CMD_POWERDOWN     0x02
+#define ADS112C04_CMD_RDATA         0x10
+#define ADS112C04_CMD_RREG(reg)     (0x20 | ((reg) << 2))
+#define ADS112C04_CMD_WREG(reg)     (0x40 | ((reg) << 2))
+
+/* Registers */
+#define ADS112C04_REG_CONFIG0       0x00
+#define ADS112C04_REG_CONFIG1       0x01
+#define ADS112C04_REG_CONFIG2       0x02
+#define ADS112C04_REG_CONFIG3       0x03
+
+#define ADS112C04_DRDY_MASK         BIT(7)
+#define ADS112C04_MUX_MASK          GENMASK(7, 4)
+
+struct ads112c04_state {
+	struct i2c_client *client;
+	/* Protects concurrent ADC reads and device configuration */
+	struct mutex lock;
+	struct completion completion;
+	struct regulator *vref_reg;
+	int vref_mv;
+	u8 config0;
+	u8 config1;
+};
+
+static int ads112c04_write_cmd(struct i2c_client *client, u8 cmd)
+{
+	int ret = i2c_master_send(client, &cmd, 1);
+
+	return ret < 0 ? ret : 0;
+}
+
+static int ads112c04_read_reg(struct i2c_client *client, u8 reg, u8 *val)
+{
+	u8 cmd = ADS112C04_CMD_RREG(reg);
+	int ret;
+
+	ret = i2c_master_send(client, &cmd, 1);
+	if (ret < 0)
+		return ret;
+
+	ret = i2c_master_recv(client, val, 1);
+	return ret < 0 ? ret : 0;
+}
+
+static int ads112c04_write_reg(struct i2c_client *client, u8 reg, u8 val)
+{
+	u8 buf[2] = { ADS112C04_CMD_WREG(reg), val };
+	int ret;
+
+	ret = i2c_master_send(client, buf, 2);
+	return ret < 0 ? ret : 0;
+}
+
+static int ads112c04_wait_for_data(struct ads112c04_state *st)
+{
+	int ret;
+	u8 val;
+
+	if (st->client->irq > 0) {
+		ret = wait_for_completion_timeout(&st->completion, msecs_to_jiffies(1000));
+		if (!ret)
+			return -ETIMEDOUT;
+		return 0;
+	}
+
+	return read_poll_timeout(ads112c04_read_reg, ret,
+				 (ret < 0 || (val & ADS112C04_DRDY_MASK)),
+				 1000, 1000000, false,
+				 st->client, ADS112C04_REG_CONFIG2, &val);
+}
+
+static int ads112c04_read_data(struct ads112c04_state *st, int *val)
+{
+	u8 cmd = ADS112C04_CMD_RDATA;
+	__be16 buf;
+	int ret;
+
+	ret = i2c_master_send(st->client, &cmd, 1);
+	if (ret < 0)
+		return ret;
+
+	ret = i2c_master_recv(st->client, (u8 *)&buf, 2);
+	if (ret < 0)
+		return ret;
+
+	*val = sign_extend32(be16_to_cpu(buf), 15);
+	return 0;
+}
+
+static int ads112c04_get_adc_result(struct ads112c04_state *st,
+				    struct iio_chan_spec const *chan,
+				    int *val)
+{
+	int ret;
+	u8 mux, new_config0;
+
+	mux = FIELD_PREP(ADS112C04_MUX_MASK, chan->address);
+	new_config0 = (st->config0 & 0x0F) | mux;
+
+	if (st->config0 != new_config0) {
+		ret = ads112c04_write_reg(st->client, ADS112C04_REG_CONFIG0, new_config0);
+		if (ret < 0)
+			return ret;
+		st->config0 = new_config0;
+	}
+
+	reinit_completion(&st->completion);
+
+	ret = ads112c04_write_cmd(st->client, ADS112C04_CMD_START_SYNC);
+	if (ret < 0)
+		return ret;
+
+	ret = ads112c04_wait_for_data(st);
+	if (ret < 0)
+		return ret;
+
+	return ads112c04_read_data(st, val);
+}
+
+static int ads112c04_read_raw(struct iio_dev *indio_dev,
+			      struct iio_chan_spec const *chan,
+			      int *val, int *val2, long mask)
+{
+	struct ads112c04_state *st = iio_priv(indio_dev);
+	int ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		mutex_lock(&st->lock);
+		ret = ads112c04_get_adc_result(st, chan, val);
+		mutex_unlock(&st->lock);
+
+		if (ret < 0)
+			return ret;
+		return IIO_VAL_INT;
+
+	case IIO_CHAN_INFO_SCALE:
+		*val = st->vref_mv;
+		*val2 = 15;
+		return IIO_VAL_FRACTIONAL_LOG2;
+
+	default:
+		return -EINVAL;
+	}
+}
+
+static irqreturn_t ads112c04_irq_handler(int irq, void *private)
+{
+	struct iio_dev *indio_dev = private;
+	struct ads112c04_state *st = iio_priv(indio_dev);
+
+	complete(&st->completion);
+
+	return IRQ_HANDLED;
+}
+
+static const struct iio_info ads112c04_info = {
+	.read_raw = ads112c04_read_raw,
+};
+
+static void ads112c04_regulator_disable(void *data)
+{
+	regulator_disable(data);
+}
+
+static int ads112c04_parse_channels(struct iio_dev *indio_dev)
+{
+	struct device *dev = indio_dev->dev.parent;
+	struct iio_chan_spec *channels;
+	u32 num_channels, i = 0, pair[2];
+
+	num_channels = device_get_named_child_node_count(dev, "channel");
+	if (!num_channels)
+		return -EINVAL;
+
+	channels = devm_kcalloc(dev, num_channels, sizeof(*channels), GFP_KERNEL);
+	if (!channels)
+		return -ENOMEM;
+
+	device_for_each_named_child_node_scoped(dev, child, "channel") {
+		struct iio_chan_spec *spec = &channels[i];
+
+		spec->type = IIO_VOLTAGE;
+		spec->indexed = 1;
+		spec->info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE);
+		spec->scan_index = i;
+
+		if (fwnode_property_present(child, "single-channel")) {
+			fwnode_property_read_u32(child, "single-channel", &pair[0]);
+			spec->channel = pair[0];
+			spec->differential = 0;
+			spec->address = 0x08 + pair[0];
+		} else if (fwnode_property_present(child, "diff-channels")) {
+			fwnode_property_read_u32_array(child, "diff-channels", pair, 2);
+			spec->channel = pair[0];
+			spec->channel2 = pair[1];
+			spec->differential = 1;
+
+			if (pair[0] == 0 && pair[1] == 1)
+				spec->address = 0x00;
+			else if (pair[0] == 0 && pair[1] == 2)
+				spec->address = 0x01;
+			else if (pair[0] == 0 && pair[1] == 3)
+				spec->address = 0x02;
+			else if (pair[0] == 1 && pair[1] == 0)
+				spec->address = 0x03;
+			else if (pair[0] == 1 && pair[1] == 2)
+				spec->address = 0x04;
+			else if (pair[0] == 1 && pair[1] == 3)
+				spec->address = 0x05;
+			else if (pair[0] == 2 && pair[1] == 3)
+				spec->address = 0x06;
+			else if (pair[0] == 3 && pair[1] == 2)
+				spec->address = 0x07;
+			else
+				return -EINVAL;
+		} else {
+			return -EINVAL;
+		}
+		i++;
+	}
+
+	indio_dev->channels = channels;
+	indio_dev->num_channels = num_channels;
+
+	return 0;
+}
+
+static int ads112c04_probe(struct i2c_client *client)
+{
+	struct iio_dev *indio_dev;
+	struct ads112c04_state *st;
+	struct gpio_desc *reset_gpio;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*st));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	st = iio_priv(indio_dev);
+	st->client = client;
+
+	ret = devm_mutex_init(&client->dev, &st->lock);
+	if (ret)
+		return ret;
+
+	init_completion(&st->completion);
+
+	indio_dev->name = "ads112c04";
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->info = &ads112c04_info;
+
+	ret = ads112c04_parse_channels(indio_dev);
+	if (ret)
+		return ret;
+
+	ret = devm_regulator_get_enable(&client->dev, "avdd");
+	if (ret)
+		return dev_err_probe(&client->dev, ret, "failed to get avdd regulator\n");
+
+	ret = devm_regulator_get_enable(&client->dev, "dvdd");
+	if (ret)
+		return dev_err_probe(&client->dev, ret, "failed to get dvdd regulator\n");
+
+	st->vref_reg = devm_regulator_get_optional(&client->dev, "refp");
+	if (IS_ERR(st->vref_reg)) {
+		ret = PTR_ERR(st->vref_reg);
+		if (ret == -ENODEV) {
+			st->vref_mv = 2048;
+			st->config1 = 0x00;
+		} else {
+			return ret;
+		}
+	} else {
+		ret = regulator_enable(st->vref_reg);
+		if (ret)
+			return ret;
+
+		ret = devm_add_action_or_reset(&client->dev, ads112c04_regulator_disable,
+					       st->vref_reg);
+		if (ret)
+			return ret;
+
+		ret = regulator_get_voltage(st->vref_reg);
+		if (ret < 0)
+			return ret;
+
+		st->vref_mv = ret / 1000;
+		st->config1 = 0x02;
+	}
+
+	reset_gpio = devm_gpiod_get_optional(&client->dev, "reset", GPIOD_OUT_LOW);
+	if (IS_ERR(reset_gpio))
+		return PTR_ERR(reset_gpio);
+
+	if (reset_gpio) {
+		gpiod_set_value_cansleep(reset_gpio, 1);
+		fsleep(1000);
+		gpiod_set_value_cansleep(reset_gpio, 0);
+	} else {
+		ret = ads112c04_write_cmd(client, ADS112C04_CMD_RESET);
+		if (ret < 0)
+			return ret;
+	}
+
+	fsleep(1000);
+
+	st->config0 = 0x01;
+	ret = ads112c04_write_reg(client, ADS112C04_REG_CONFIG0, st->config0);
+	if (ret)
+		return ret;
+
+	ret = ads112c04_write_reg(client, ADS112C04_REG_CONFIG1, st->config1);
+	if (ret)
+		return ret;
+
+	if (client->irq > 0) {
+		ret = devm_request_irq(&client->dev, client->irq,
+				       ads112c04_irq_handler,
+				       0,
+				       indio_dev->name, indio_dev);
+		if (ret) {
+			dev_err(&client->dev, "Failed to request DRDY IRQ\n");
+			return ret;
+		}
+	}
+
+	return devm_iio_device_register(&client->dev, indio_dev);
+}
+
+static const struct i2c_device_id ads112c04_id[] = {
+	{ .name = "ads112c04", .driver_data = 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, ads112c04_id);
+
+static const struct of_device_id ads112c04_of_match[] = {
+	{ .compatible = "ti,ads112c04" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, ads112c04_of_match);
+
+static struct i2c_driver ads112c04_driver = {
+	.driver = {
+		.name = "ads112c04",
+		.of_match_table = ads112c04_of_match,
+	},
+	.probe = ads112c04_probe,
+	.id_table = ads112c04_id,
+};
+module_i2c_driver(ads112c04_driver);
+
+MODULE_AUTHOR("Kyle Hsieh <kylehsieh1995@gmail.com>");
+MODULE_DESCRIPTION("Texas Instruments ADS112C04 ADC driver");
+MODULE_LICENSE("GPL");

-- 
2.34.1

Re: [PATCH v2 2/2] iio: adc: ti-ads112c04: Add support for TI ADS112C04

[Joshua Crofts]

On Fri, 31 Jul 2026 10:58:25 +0800
Kyle Hsieh <kylehsieh1995@gmail.com> wrote:
> diff --git a/drivers/iio/adc/ti-ads112c04.c b/drivers/iio/adc/ti-ads112c04.c
> new file mode 100644
> index 000000000000..28d3be81934f
> --- /dev/null
> +++ b/drivers/iio/adc/ti-ads112c04.c

Hi Kyle, quick review from me, comments inline. Additionally, please
check Sashiko's review as there are some move severe issues (mostly
I2C stuff), see it here:
https://sashiko.dev/#/patchset/20260731-ti-ads112c04-driver-v2-0-aab0168c3c01%40gmail.com

> @@ -0,0 +1,378 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Texas Instruments ADS112C04 16-bit I2C ADC driver
> + * Based on TI Reference Code and standard Linux IIO framework.

Usually we'd add a Copyright (c) 2026 your_name_here your_email_here
and maybe a datasheet link for easy lookup.

> + */
> +
> +#include <linux/module.h>
> +#include <linux/i2c.h>
> +#include <linux/delay.h>
> +#include <linux/interrupt.h>
> +#include <linux/mutex.h>

Please sort your headers alphabetically.

> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>

Group <linux/iio/*> headers separately and add them after the generic
<linux/*> headers.

> +#include <linux/regulator/consumer.h>
> +#include <linux/bitfield.h>
> +#include <linux/iopoll.h>
> +#include <linux/property.h>
> +#include <linux/gpio/consumer.h>

Additionally, you're missing jiffies.h, err.h, bitops.h, types.h

> +/* ADS112C04 Commands */

Unnecessary comment IMO, it's clear that these are commands from the
*_CMD_* part (same goes for your registers comment).

> +#define ADS112C04_CMD_RESET         0x06
> +#define ADS112C04_CMD_START_SYNC    0x08
> +#define ADS112C04_CMD_POWERDOWN     0x02
> +#define ADS112C04_CMD_RDATA         0x10
> +#define ADS112C04_CMD_RREG(reg)     (0x20 | ((reg) << 2))
> +#define ADS112C04_CMD_WREG(reg)     (0x40 | ((reg) << 2))
> +
> +/* Registers */
> +#define ADS112C04_REG_CONFIG0       0x00
> +#define ADS112C04_REG_CONFIG1       0x01
> +#define ADS112C04_REG_CONFIG2       0x02
> +#define ADS112C04_REG_CONFIG3       0x03
> +
> +#define ADS112C04_DRDY_MASK         BIT(7)
> +#define ADS112C04_MUX_MASK          GENMASK(7, 4)
> +
> +struct ads112c04_state {
> +	struct i2c_client *client;
> +	/* Protects concurrent ADC reads and device configuration */
> +	struct mutex lock;
> +	struct completion completion;
> +	struct regulator *vref_reg;
> +	int vref_mv;

vref_mV, this is a good exception to the no camelCase rule, as it's a SI
unit.

> +	u8 config0;
> +	u8 config1;
> +};
> +
> +static int ads112c04_write_cmd(struct i2c_client *client, u8 cmd)
> +{
> +	int ret = i2c_master_send(client, &cmd, 1);
> +
> +	return ret < 0 ? ret : 0;
> +}
> +
> +static int ads112c04_read_reg(struct i2c_client *client, u8 reg, u8 *val)
> +{
> +	u8 cmd = ADS112C04_CMD_RREG(reg);
> +	int ret;
> +
> +	ret = i2c_master_send(client, &cmd, 1);

i2c_master_send returns either the amount of bytes sent or an error
code. If the device NACKs, the function will return 0 (zero bytes sent)
but this will be interpreted as success.

> +	if (ret < 0)
> +		return ret;
> +
> +	ret = i2c_master_recv(client, val, 1);
> +	return ret < 0 ? ret : 0;
> +}
> +
> +static int ads112c04_write_reg(struct i2c_client *client, u8 reg, u8 val)
> +{
> +	u8 buf[2] = { ADS112C04_CMD_WREG(reg), val };
> +	int ret;
> +
> +	ret = i2c_master_send(client, buf, 2);

Use sizeof, don't hardcode the buffer sizes.

> +	return ret < 0 ? ret : 0;
> +}
> +
> +static int ads112c04_wait_for_data(struct ads112c04_state *st)
> +{
> +	int ret;
> +	u8 val;
> +
> +	if (st->client->irq > 0) {
> +		ret = wait_for_completion_timeout(&st->completion, msecs_to_jiffies(1000));
> +		if (!ret)
> +			return -ETIMEDOUT;
> +		return 0;
> +	}
> +
> +	return read_poll_timeout(ads112c04_read_reg, ret,
> +				 (ret < 0 || (val & ADS112C04_DRDY_MASK)),
> +				 1000, 1000000, false,

Sashiko points out that read_poll_timeout discards any I2C read errors and returns
0. Remove the ret < 0 condition.

> +				 st->client, ADS112C04_REG_CONFIG2, &val);
> +}
> +
> +static int ads112c04_read_data(struct ads112c04_state *st, int *val)
> +{
> +	u8 cmd = ADS112C04_CMD_RDATA;
> +	__be16 buf;
> +	int ret;
> +
> +	ret = i2c_master_send(st->client, &cmd, 1);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = i2c_master_recv(st->client, (u8 *)&buf, 2);
> +	if (ret < 0)
> +		return ret;
> +
> +	*val = sign_extend32(be16_to_cpu(buf), 15);
> +	return 0;
> +}
> +
> +static int ads112c04_get_adc_result(struct ads112c04_state *st,
> +				    struct iio_chan_spec const *chan,
> +				    int *val)
> +{
> +	int ret;
> +	u8 mux, new_config0;

Reverse xmas tree order please.
> +
> +	mux = FIELD_PREP(ADS112C04_MUX_MASK, chan->address);
> +	new_config0 = (st->config0 & 0x0F) | mux;
> +
> +	if (st->config0 != new_config0) {
> +		ret = ads112c04_write_reg(st->client, ADS112C04_REG_CONFIG0, new_config0);
> +		if (ret < 0)
> +			return ret;
> +		st->config0 = new_config0;
> +	}
> +
> +	reinit_completion(&st->completion);
> +
> +	ret = ads112c04_write_cmd(st->client, ADS112C04_CMD_START_SYNC);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = ads112c04_wait_for_data(st);
> +	if (ret < 0)
> +		return ret;
> +
> +	return ads112c04_read_data(st, val);
> +}
> +
> +static int ads112c04_read_raw(struct iio_dev *indio_dev,
> +			      struct iio_chan_spec const *chan,
> +			      int *val, int *val2, long mask)
> +{
> +	struct ads112c04_state *st = iio_priv(indio_dev);
> +	int ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		mutex_lock(&st->lock);
> +		ret = ads112c04_get_adc_result(st, chan, val);
> +		mutex_unlock(&st->lock);
> +
> +		if (ret < 0)
> +			return ret;
> +		return IIO_VAL_INT;
> +
> +	case IIO_CHAN_INFO_SCALE:
> +		*val = st->vref_mv;
> +		*val2 = 15;
> +		return IIO_VAL_FRACTIONAL_LOG2;
> +
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static irqreturn_t ads112c04_irq_handler(int irq, void *private)
> +{
> +	struct iio_dev *indio_dev = private;
> +	struct ads112c04_state *st = iio_priv(indio_dev);
> +
> +	complete(&st->completion);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static const struct iio_info ads112c04_info = {
> +	.read_raw = ads112c04_read_raw,
> +};
> +
> +static void ads112c04_regulator_disable(void *data)
> +{
> +	regulator_disable(data);
> +}
> +
> +static int ads112c04_parse_channels(struct iio_dev *indio_dev)
> +{
> +	struct device *dev = indio_dev->dev.parent;
> +	struct iio_chan_spec *channels;
> +	u32 num_channels, i = 0, pair[2];
> +
> +	num_channels = device_get_named_child_node_count(dev, "channel");
> +	if (!num_channels)
> +		return -EINVAL;
> +
> +	channels = devm_kcalloc(dev, num_channels, sizeof(*channels), GFP_KERNEL);
> +	if (!channels)
> +		return -ENOMEM;
> +
> +	device_for_each_named_child_node_scoped(dev, child, "channel") {
> +		struct iio_chan_spec *spec = &channels[i];
> +
> +		spec->type = IIO_VOLTAGE;
> +		spec->indexed = 1;
> +		spec->info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE);
> +		spec->scan_index = i;
> +
> +		if (fwnode_property_present(child, "single-channel")) {
> +			fwnode_property_read_u32(child, "single-channel", &pair[0]);

The return value of fwnode_property_read_u32 isn't checked, meaning that
pair[0] will contain stack garbage.

> +			spec->channel = pair[0];
> +			spec->differential = 0;
> +			spec->address = 0x08 + pair[0];
> +		} else if (fwnode_property_present(child, "diff-channels")) {
> +			fwnode_property_read_u32_array(child, "diff-channels", pair, 2);
> +			spec->channel = pair[0];
> +			spec->channel2 = pair[1];
> +			spec->differential = 1;
> +
> +			if (pair[0] == 0 && pair[1] == 1)
> +				spec->address = 0x00;
> +			else if (pair[0] == 0 && pair[1] == 2)
> +				spec->address = 0x01;
> +			else if (pair[0] == 0 && pair[1] == 3)
> +				spec->address = 0x02;
> +			else if (pair[0] == 1 && pair[1] == 0)
> +				spec->address = 0x03;
> +			else if (pair[0] == 1 && pair[1] == 2)
> +				spec->address = 0x04;
> +			else if (pair[0] == 1 && pair[1] == 3)
> +				spec->address = 0x05;
> +			else if (pair[0] == 2 && pair[1] == 3)
> +				spec->address = 0x06;
> +			else if (pair[0] == 3 && pair[1] == 2)
> +				spec->address = 0x07;
> +			else
> +				return -EINVAL;
> +		} else {
> +			return -EINVAL;
> +		}
> +		i++;
> +	}
> +
> +	indio_dev->channels = channels;
> +	indio_dev->num_channels = num_channels;
> +
> +	return 0;
> +}
> +
> +static int ads112c04_probe(struct i2c_client *client)
> +{
> +	struct iio_dev *indio_dev;
> +	struct ads112c04_state *st;
> +	struct gpio_desc *reset_gpio;

Reverse xmas tree order here as well.

> +	int ret;
> +
> +	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*st));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	st = iio_priv(indio_dev);
> +	st->client = client;
> +
> +	ret = devm_mutex_init(&client->dev, &st->lock);
> +	if (ret)
> +		return ret;
> +
> +	init_completion(&st->completion);
> +
> +	indio_dev->name = "ads112c04";
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->info = &ads112c04_info;
> +
> +	ret = ads112c04_parse_channels(indio_dev);
> +	if (ret)
> +		return ret;
> +
> +	ret = devm_regulator_get_enable(&client->dev, "avdd");
> +	if (ret)
> +		return dev_err_probe(&client->dev, ret, "failed to get avdd regulator\n");
> +
> +	ret = devm_regulator_get_enable(&client->dev, "dvdd");
> +	if (ret)
> +		return dev_err_probe(&client->dev, ret, "failed to get dvdd regulator\n");
> +
> +	st->vref_reg = devm_regulator_get_optional(&client->dev, "refp");
> +	if (IS_ERR(st->vref_reg)) {
> +		ret = PTR_ERR(st->vref_reg);
> +		if (ret == -ENODEV) {
> +			st->vref_mv = 2048;
> +			st->config1 = 0x00;
> +		} else {
> +			return ret;
> +		}
> +	} else {
> +		ret = regulator_enable(st->vref_reg);
> +		if (ret)
> +			return ret;
> +
> +		ret = devm_add_action_or_reset(&client->dev, ads112c04_regulator_disable,
> +					       st->vref_reg);
> +		if (ret)
> +			return ret;
> +
> +		ret = regulator_get_voltage(st->vref_reg);
> +		if (ret < 0)
> +			return ret;
> +
> +		st->vref_mv = ret / 1000;
> +		st->config1 = 0x02;
> +	}
> +
> +	reset_gpio = devm_gpiod_get_optional(&client->dev, "reset", GPIOD_OUT_LOW);
> +	if (IS_ERR(reset_gpio))
> +		return PTR_ERR(reset_gpio);
> +
> +	if (reset_gpio) {
> +		gpiod_set_value_cansleep(reset_gpio, 1);
> +		fsleep(1000);
> +		gpiod_set_value_cansleep(reset_gpio, 0);
> +	} else {
> +		ret = ads112c04_write_cmd(client, ADS112C04_CMD_RESET);
> +		if (ret < 0)
> +			return ret;
> +	}
> +
> +	fsleep(1000);

Why 1000? Add a comment that links to the datasheet or an explanation.

> +
> +	st->config0 = 0x01;
> +	ret = ads112c04_write_reg(client, ADS112C04_REG_CONFIG0, st->config0);
> +	if (ret)
> +		return ret;
> +
> +	ret = ads112c04_write_reg(client, ADS112C04_REG_CONFIG1, st->config1);
> +	if (ret)
> +		return ret;
> +
> +	if (client->irq > 0) {
> +		ret = devm_request_irq(&client->dev, client->irq,
> +				       ads112c04_irq_handler,
> +				       0,
> +				       indio_dev->name, indio_dev);
> +		if (ret) {
> +			dev_err(&client->dev, "Failed to request DRDY IRQ\n");
> +			return ret;

Just return ret, devm_request_irq() already prints an error message on
failure.

-- 
Kind regards,
Joshua Crofts

Re: [PATCH v2 2/2] iio: adc: ti-ads112c04: Add support for TI ADS112C04

[Jonathan Cameron]

On Fri, 31 Jul 2026 11:27:50 +0200
Joshua Crofts <joshua.crofts1@gmail.com> wrote:

> On Fri, 31 Jul 2026 10:58:25 +0800
> Kyle Hsieh <kylehsieh1995@gmail.com> wrote:
> > diff --git a/drivers/iio/adc/ti-ads112c04.c b/drivers/iio/adc/ti-ads112c04.c
> > new file mode 100644
> > index 000000000000..28d3be81934f
> > --- /dev/null
> > +++ b/drivers/iio/adc/ti-ads112c04.c  
> 
> Hi Kyle, quick review from me, comments inline. Additionally, please
> check Sashiko's review as there are some move severe issues (mostly
> I2C stuff), see it here:
> https://sashiko.dev/#/patchset/20260731-ti-ads112c04-driver-v2-0-aab0168c3c01%40gmail.com
Be careful with these.   Some may be misleading or the correct
response may be in a very different place to sashiko suggests.
For example we wouldn't typically bother to defend against nonsense
interrupt types from DT, so if level isn't a plausible type then
state what is as a comment in the DT.

Also, multi master doesn't seems like something we should worry
too much about.  That's not to say there isn't a better way to handle that
transaction.

> > +static int ads112c04_read_reg(struct i2c_client *client, u8 reg, u8 *val)
> > +{
> > +	u8 cmd = ADS112C04_CMD_RREG(reg);
> > +	int ret;
> > +
> > +	ret = i2c_master_send(client, &cmd, 1);  
> 
> i2c_master_send returns either the amount of bytes sent or an error
> code. If the device NACKs, the function will return 0 (zero bytes sent)
> but this will be interpreted as success.

This one is fun and not necessarily that simple. (I used to assume it was!)
Mostly nacks will result in error codes, but there are other obscure
reasons it might return 0.  

Anyhow, whatever the reason, agreed we annoyingly have to check these to see if
they are not 0.

Or, better, as per David's comment use the smbus command if that is possible.
> 
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	ret = i2c_master_recv(client, val, 1);
> > +	return ret < 0 ? ret : 0;
> > +}

Re: [PATCH v2 2/2] iio: adc: ti-ads112c04: Add support for TI ADS112C04

[Kyle Hsieh]

Hi Jonathan,
Thank you for stepping in and providing the perspective!

On Sun, Aug 2, 2026 at 6:59 AM Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Fri, 31 Jul 2026 11:27:50 +0200
> Joshua Crofts <joshua.crofts1@gmail.com> wrote:
>
> > On Fri, 31 Jul 2026 10:58:25 +0800
> > Kyle Hsieh <kylehsieh1995@gmail.com> wrote:
> > > diff --git a/drivers/iio/adc/ti-ads112c04.c b/drivers/iio/adc/ti-ads112c04.c
> > > new file mode 100644
> > > index 000000000000..28d3be81934f
> > > --- /dev/null
> > > +++ b/drivers/iio/adc/ti-ads112c04.c
> >
> > Hi Kyle, quick review from me, comments inline. Additionally, please
> > check Sashiko's review as there are some move severe issues (mostly
> > I2C stuff), see it here:
> > https://sashiko.dev/#/patchset/20260731-ti-ads112c04-driver-v2-0-aab0168c3c01%40gmail.com
> Be careful with these.   Some may be misleading or the correct
> response may be in a very different place to sashiko suggests.
> For example we wouldn't typically bother to defend against nonsense
> interrupt types from DT, so if level isn't a plausible type then
> state what is as a comment in the DT.
Point well taken. I have reverted the IRQ flags parameter back to `0`
in the `devm_request_irq()`
call to let the driver naturally inherit the trigger type from the
device tree, rather than hardcoding
`IRQF_TRIGGER_FALLING` just to appease the bot's edge-case warning.
I've also made sure the expected edge-falling behavior is clearly
documented in the DT binding example.
>
> Also, multi master doesn't seems like something we should worry
> too much about.  That's not to say there isn't a better way to handle that
> transaction.
>
> > > +static int ads112c04_read_reg(struct i2c_client *client, u8 reg, u8 *val)
> > > +{
> > > +   u8 cmd = ADS112C04_CMD_RREG(reg);
> > > +   int ret;
> > > +
> > > +   ret = i2c_master_send(client, &cmd, 1);
> >
> > i2c_master_send returns either the amount of bytes sent or an error
> > code. If the device NACKs, the function will return 0 (zero bytes sent)
> > but this will be interpreted as success.
>
> This one is fun and not necessarily that simple. (I used to assume it was!)
> Mostly nacks will result in error codes, but there are other obscure
> reasons it might return 0.
>
> Anyhow, whatever the reason, agreed we annoyingly have to check these to see if
> they are not 0.
>
> Or, better, as per David's comment use the smbus command if that is possible.
Taking both your and David's advice, I have completely replaced the
raw `i2c_master_send/recv`
transfers with `i2c_smbus_read_byte_data`, `i2c_smbus_write_byte_data`, and
`i2c_smbus_read_word_data`.

This elegantly handles the transactions and completely side-steps the
headache of parsing
those obscure `0` return values from `i2c_master_send`.

Thanks again for the guidance!

Best regards,
Kyle Hsieh
> >
> > > +   if (ret < 0)
> > > +           return ret;
> > > +
> > > +   ret = i2c_master_recv(client, val, 1);
> > > +   return ret < 0 ? ret : 0;
> > > +}

Re: [PATCH v2 2/2] iio: adc: ti-ads112c04: Add support for TI ADS112C04

[Kyle Hsieh]

Hi Joshua,
Thanks for the quick and thorough review!

On Fri, Jul 31, 2026 at 5:27 PM Joshua Crofts <joshua.crofts1@gmail.com> wrote:
>
> On Fri, 31 Jul 2026 10:58:25 +0800
> Kyle Hsieh <kylehsieh1995@gmail.com> wrote:
> > diff --git a/drivers/iio/adc/ti-ads112c04.c b/drivers/iio/adc/ti-ads112c04.c
> > new file mode 100644
> > index 000000000000..28d3be81934f
> > --- /dev/null
> > +++ b/drivers/iio/adc/ti-ads112c04.c
>
> Hi Kyle, quick review from me, comments inline. Additionally, please
> check Sashiko's review as there are some move severe issues (mostly
> I2C stuff), see it here:
> https://sashiko.dev/#/patchset/20260731-ti-ads112c04-driver-v2-0-aab0168c3c01%40gmail.com
>
> > @@ -0,0 +1,378 @@
> > +// SPDX-License-Identifier: GPL-2ㄏ.0-only
> > +/*
> > + * Texas Instruments ADS112C04 16-bit I2C ADC driver
> > + * Based on TI Reference Code and standard Linux IIO framework.
>
> Usually we'd add a Copyright (c) 2026 your_name_here your_email_here
> and maybe a datasheet link for easy lookup.
I will add both the copyright line and the datasheet link to the file header.
>
> > + */
> > +
> > +#include <linux/module.h>
> > +#include <linux/i2c.h>
> > +#include <linux/delay.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/mutex.h>
>
> Please sort your headers alphabetically.
>
> > +#include <linux/iio/iio.h>
> > +#include <linux/iio/sysfs.h>
>
> Group <linux/iio/*> headers separately and add them after the generic
> <linux/*> headers.
>
> > +#include <linux/regulator/consumer.h>
> > +#include <linux/bitfield.h>
> > +#include <linux/iopoll.h>
> > +#include <linux/property.h>
> > +#include <linux/gpio/consumer.h>
>
> Additionally, you're missing jiffies.h, err.h, bitops.h, types.h
All headers are now sorted alphabetically, correctly grouped, and the
missing ones have been included.
>
> > +/* ADS112C04 Commands */
>
> Unnecessary comment IMO, it's clear that these are commands from the
> *_CMD_* part (same goes for your registers comment).
Removed the redundant block comments for commands, registers, and masks.
>
> > +#define ADS112C04_CMD_RESET         0x06
> > +#define ADS112C04_CMD_START_SYNC    0x08
> > +#define ADS112C04_CMD_POWERDOWN     0x02
> > +#define ADS112C04_CMD_RDATA         0x10
> > +#define ADS112C04_CMD_RREG(reg)     (0x20 | ((reg) << 2))
> > +#define ADS112C04_CMD_WREG(reg)     (0x40 | ((reg) << 2))
> > +
> > +/* Registers */
> > +#define ADS112C04_REG_CONFIG0       0x00
> > +#define ADS112C04_REG_CONFIG1       0x01
> > +#define ADS112C04_REG_CONFIG2       0x02
> > +#define ADS112C04_REG_CONFIG3       0x03
> > +
> > +#define ADS112C04_DRDY_MASK         BIT(7)
> > +#define ADS112C04_MUX_MASK          GENMASK(7, 4)
> > +
> > +struct ads112c04_state {
> > +     struct i2c_client *client;
> > +     /* Protects concurrent ADC reads and device configuration */
> > +     struct mutex lock;
> > +     struct completion completion;
> > +     struct regulator *vref_reg;
> > +     int vref_mv;
>
> vref_mV, this is a good exception to the no camelCase rule, as it's a SI
> unit.
Renamed vref_mv to vref_mV.
>
> > +     u8 config0;
> > +     u8 config1;
> > +};
> > +
> > +static int ads112c04_write_cmd(struct i2c_client *client, u8 cmd)
> > +{
> > +     int ret = i2c_master_send(client, &cmd, 1);
> > +
> > +     return ret < 0 ? ret : 0;
> > +}
> > +
> > +static int ads112c04_read_reg(struct i2c_client *client, u8 reg, u8 *val)
> > +{
> > +     u8 cmd = ADS112C04_CMD_RREG(reg);
> > +     int ret;
> > +
> > +     ret = i2c_master_send(client, &cmd, 1);
>
> i2c_master_send returns either the amount of bytes sent or an error
> code. If the device NACKs, the function will return 0 (zero bytes sent)
> but this will be interpreted as success.
>
> > +     if (ret < 0)
> > +             return ret;
> > +
> > +     ret = i2c_master_recv(client, val, 1);
> > +     return ret < 0 ? ret : 0;
> > +}
> > +
> > +static int ads112c04_write_reg(struct i2c_client *client, u8 reg, u8 val)
> > +{
> > +     u8 buf[2] = { ADS112C04_CMD_WREG(reg), val };
> > +     int ret;
> > +
> > +     ret = i2c_master_send(client, buf, 2);
>
> Use sizeof, don't hardcode the buffer sizes.
>
> > +     return ret < 0 ? ret : 0;
> > +}
> > +
> > +static int ads112c04_wait_for_data(struct ads112c04_state *st)
> > +{
> > +     int ret;
> > +     u8 val;
> > +
> > +     if (st->client->irq > 0) {
> > +             ret = wait_for_completion_timeout(&st->completion, msecs_to_jiffies(1000));
> > +             if (!ret)
> > +                     return -ETIMEDOUT;
> > +             return 0;
> > +     }
> > +
> > +     return read_poll_timeout(ads112c04_read_reg, ret,
> > +                              (ret < 0 || (val & ADS112C04_DRDY_MASK)),
> > +                              1000, 1000000, false,
>
> Sashiko points out that read_poll_timeout discards any I2C read errors and returns
> 0. Remove the ret < 0 condition.
I will explicitly check the I2C return value right after the
read_poll_timeout macro finishes.
>
> > +                              st->client, ADS112C04_REG_CONFIG2, &val);
> > +}
> > +
> > +static int ads112c04_read_data(struct ads112c04_state *st, int *val)
> > +{
> > +     u8 cmd = ADS112C04_CMD_RDATA;
> > +     __be16 buf;
> > +     int ret;
> > +
> > +     ret = i2c_master_send(st->client, &cmd, 1);
> > +     if (ret < 0)
> > +             return ret;
> > +
> > +     ret = i2c_master_recv(st->client, (u8 *)&buf, 2);
> > +     if (ret < 0)
> > +             return ret;
> > +
> > +     *val = sign_extend32(be16_to_cpu(buf), 15);
> > +     return 0;
> > +}
> > +
> > +static int ads112c04_get_adc_result(struct ads112c04_state *st,
> > +                                 struct iio_chan_spec const *chan,
> > +                                 int *val)
> > +{
> > +     int ret;
> > +     u8 mux, new_config0;
>
> Reverse xmas tree order please.
> > +
> > +     mux = FIELD_PREP(ADS112C04_MUX_MASK, chan->address);
> > +     new_config0 = (st->config0 & 0x0F) | mux;
> > +
> > +     if (st->config0 != new_config0) {
> > +             ret = ads112c04_write_reg(st->client, ADS112C04_REG_CONFIG0, new_config0);
> > +             if (ret < 0)
> > +                     return ret;
> > +             st->config0 = new_config0;
> > +     }
> > +
> > +     reinit_completion(&st->completion);
> > +
> > +     ret = ads112c04_write_cmd(st->client, ADS112C04_CMD_START_SYNC);
> > +     if (ret < 0)
> > +             return ret;
> > +
> > +     ret = ads112c04_wait_for_data(st);
> > +     if (ret < 0)
> > +             return ret;
> > +
> > +     return ads112c04_read_data(st, val);
> > +}
> > +
> > +static int ads112c04_read_raw(struct iio_dev *indio_dev,
> > +                           struct iio_chan_spec const *chan,
> > +                           int *val, int *val2, long mask)
> > +{
> > +     struct ads112c04_state *st = iio_priv(indio_dev);
> > +     int ret;
> > +
> > +     switch (mask) {
> > +     case IIO_CHAN_INFO_RAW:
> > +             mutex_lock(&st->lock);
> > +             ret = ads112c04_get_adc_result(st, chan, val);
> > +             mutex_unlock(&st->lock);
> > +
> > +             if (ret < 0)
> > +                     return ret;
> > +             return IIO_VAL_INT;
> > +
> > +     case IIO_CHAN_INFO_SCALE:
> > +             *val = st->vref_mv;
> > +             *val2 = 15;
> > +             return IIO_VAL_FRACTIONAL_LOG2;
> > +
> > +     default:
> > +             return -EINVAL;
> > +     }
> > +}
> > +
> > +static irqreturn_t ads112c04_irq_handler(int irq, void *private)
> > +{
> > +     struct iio_dev *indio_dev = private;
> > +     struct ads112c04_state *st = iio_priv(indio_dev);
> > +
> > +     complete(&st->completion);
> > +
> > +     return IRQ_HANDLED;
> > +}
> > +
> > +static const struct iio_info ads112c04_info = {
> > +     .read_raw = ads112c04_read_raw,
> > +};
> > +
> > +static void ads112c04_regulator_disable(void *data)
> > +{
> > +     regulator_disable(data);
> > +}
> > +
> > +static int ads112c04_parse_channels(struct iio_dev *indio_dev)
> > +{
> > +     struct device *dev = indio_dev->dev.parent;
> > +     struct iio_chan_spec *channels;
> > +     u32 num_channels, i = 0, pair[2];
> > +
> > +     num_channels = device_get_named_child_node_count(dev, "channel");
> > +     if (!num_channels)
> > +             return -EINVAL;
> > +
> > +     channels = devm_kcalloc(dev, num_channels, sizeof(*channels), GFP_KERNEL);
> > +     if (!channels)
> > +             return -ENOMEM;
> > +
> > +     device_for_each_named_child_node_scoped(dev, child, "channel") {
> > +             struct iio_chan_spec *spec = &channels[i];
> > +
> > +             spec->type = IIO_VOLTAGE;
> > +             spec->indexed = 1;
> > +             spec->info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE);
> > +             spec->scan_index = i;
> > +
> > +             if (fwnode_property_present(child, "single-channel")) {
> > +                     fwnode_property_read_u32(child, "single-channel", &pair[0]);
>
> The return value of fwnode_property_read_u32 isn't checked, meaning that
> pair[0] will contain stack garbage.
>
> > +                     spec->channel = pair[0];
> > +                     spec->differential = 0;
> > +                     spec->address = 0x08 + pair[0];
> > +             } else if (fwnode_property_present(child, "diff-channels")) {
> > +                     fwnode_property_read_u32_array(child, "diff-channels", pair, 2);
> > +                     spec->channel = pair[0];
> > +                     spec->channel2 = pair[1];
> > +                     spec->differential = 1;
> > +
> > +                     if (pair[0] == 0 && pair[1] == 1)
> > +                             spec->address = 0x00;
> > +                     else if (pair[0] == 0 && pair[1] == 2)
> > +                             spec->address = 0x01;
> > +                     else if (pair[0] == 0 && pair[1] == 3)
> > +                             spec->address = 0x02;
> > +                     else if (pair[0] == 1 && pair[1] == 0)
> > +                             spec->address = 0x03;
> > +                     else if (pair[0] == 1 && pair[1] == 2)
> > +                             spec->address = 0x04;
> > +                     else if (pair[0] == 1 && pair[1] == 3)
> > +                             spec->address = 0x05;
> > +                     else if (pair[0] == 2 && pair[1] == 3)
> > +                             spec->address = 0x06;
> > +                     else if (pair[0] == 3 && pair[1] == 2)
> > +                             spec->address = 0x07;
> > +                     else
> > +                             return -EINVAL;
> > +             } else {
> > +                     return -EINVAL;
> > +             }
> > +             i++;
> > +     }
> > +
> > +     indio_dev->channels = channels;
> > +     indio_dev->num_channels = num_channels;
> > +
> > +     return 0;
> > +}
> > +
> > +static int ads112c04_probe(struct i2c_client *client)
> > +{
> > +     struct iio_dev *indio_dev;
> > +     struct ads112c04_state *st;
> > +     struct gpio_desc *reset_gpio;
>
> Reverse xmas tree order here as well.
I will fix the variable declaration ordering across the functions
>
> > +     int ret;
> > +
> > +     indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*st));
> > +     if (!indio_dev)
> > +             return -ENOMEM;
> > +
> > +     st = iio_priv(indio_dev);
> > +     st->client = client;
> > +
> > +     ret = devm_mutex_init(&client->dev, &st->lock);
> > +     if (ret)
> > +             return ret;
> > +
> > +     init_completion(&st->completion);
> > +
> > +     indio_dev->name = "ads112c04";
> > +     indio_dev->modes = INDIO_DIRECT_MODE;
> > +     indio_dev->info = &ads112c04_info;
> > +
> > +     ret = ads112c04_parse_channels(indio_dev);
> > +     if (ret)
> > +             return ret;
> > +
> > +     ret = devm_regulator_get_enable(&client->dev, "avdd");
> > +     if (ret)
> > +             return dev_err_probe(&client->dev, ret, "failed to get avdd regulator\n");
> > +
> > +     ret = devm_regulator_get_enable(&client->dev, "dvdd");
> > +     if (ret)
> > +             return dev_err_probe(&client->dev, ret, "failed to get dvdd regulator\n");
> > +
> > +     st->vref_reg = devm_regulator_get_optional(&client->dev, "refp");
> > +     if (IS_ERR(st->vref_reg)) {
> > +             ret = PTR_ERR(st->vref_reg);
> > +             if (ret == -ENODEV) {
> > +                     st->vref_mv = 2048;
> > +                     st->config1 = 0x00;
> > +             } else {
> > +                     return ret;
> > +             }
> > +     } else {
> > +             ret = regulator_enable(st->vref_reg);
> > +             if (ret)
> > +                     return ret;
> > +
> > +             ret = devm_add_action_or_reset(&client->dev, ads112c04_regulator_disable,
> > +                                            st->vref_reg);
> > +             if (ret)
> > +                     return ret;
> > +
> > +             ret = regulator_get_voltage(st->vref_reg);
> > +             if (ret < 0)
> > +                     return ret;
> > +
> > +             st->vref_mv = ret / 1000;
> > +             st->config1 = 0x02;
> > +     }
> > +
> > +     reset_gpio = devm_gpiod_get_optional(&client->dev, "reset", GPIOD_OUT_LOW);
> > +     if (IS_ERR(reset_gpio))
> > +             return PTR_ERR(reset_gpio);
> > +
> > +     if (reset_gpio) {
> > +             gpiod_set_value_cansleep(reset_gpio, 1);
> > +             fsleep(1000);
> > +             gpiod_set_value_cansleep(reset_gpio, 0);
> > +     } else {
> > +             ret = ads112c04_write_cmd(client, ADS112C04_CMD_RESET);
> > +             if (ret < 0)
> > +                     return ret;
> > +     }
> > +
> > +     fsleep(1000);
>
> Why 1000? Add a comment that links to the datasheet or an explanation.
I've reduced this timeout to 100ms and added a comment explaining that it
corresponds to the chip's slowest data rate of 20 SPS.
>
> > +
> > +     st->config0 = 0x01;
> > +     ret = ads112c04_write_reg(client, ADS112C04_REG_CONFIG0, st->config0);
> > +     if (ret)
> > +             return ret;
> > +
> > +     ret = ads112c04_write_reg(client, ADS112C04_REG_CONFIG1, st->config1);
> > +     if (ret)
> > +             return ret;
> > +
> > +     if (client->irq > 0) {
> > +             ret = devm_request_irq(&client->dev, client->irq,
> > +                                    ads112c04_irq_handler,
> > +                                    0,
> > +                                    indio_dev->name, indio_dev);
> > +             if (ret) {
> > +                     dev_err(&client->dev, "Failed to request DRDY IRQ\n");
> > +                     return ret;
>
> Just return ret, devm_request_irq() already prints an error message on
> failure.
I will drop the redundant dev_err print.
>
> --
> Kind regards,
> Joshua Crofts

Thanks again for helping polish this driver!
Best regards,
Kyle Hsieh

Re: [PATCH v2 2/2] iio: adc: ti-ads112c04: Add support for TI ADS112C04

[David Lechner]

On 7/30/26 9:58 PM, Kyle Hsieh wrote:
> Add IIO driver support for the Texas Instruments ADS112C04 (16-bit)
> delta-sigma ADCs.
> 
> The driver implements:
> - Single-shot conversions using the IIO raw read interface.
> - Dynamic parsing of single-ended and differential channels from
>   device tree child nodes.
> - Hardware interrupt support via the DRDY pin, falling back to
>   software polling if no IRQ is provided.
> - Scale calculation based on the internal 2.048V reference.
> - Dynamic reference voltage scaling via the regulator subsystem
>   (refp-supply/refn-supply), falling back to the internal 2.048V
>   reference if not specified.
> - Hardware reset fallback using GPIO.
> 
> Signed-off-by: Kyle Hsieh <kylehsieh1995@gmail.com>
> ---
>  drivers/iio/adc/Kconfig        |  10 ++
>  drivers/iio/adc/Makefile       |   1 +
>  drivers/iio/adc/ti-ads112c04.c | 378 +++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 389 insertions(+)
> 
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 3755a81c1efd..402e841bc083 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -1789,6 +1789,16 @@ config TI_ADS1119
>           This driver can also be built as a module. If so, the module will be
>           called ti-ads1119.
>  
> +config TI_ADS112C04
> +	tristate "Texas Instruments ADS112C04 ADC"
> +	depends on I2C
> +	help
> +	  If you say yes here you get support for Texas Instruments
> +	  ADS112C04 (16-bit) I2C analog to digital converters.
> +
> +	  This driver can also be built as a module. If so, the module will be
> +	  called ti-ads112c04.
> +
>  config TI_ADS124S08
>  	tristate "Texas Instruments ADS124S08"
>  	depends on SPI
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index 707dd708912f..ebf9d4047a5a 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_ADS112C04)	+= ti-ads112c04.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-ads112c04.c b/drivers/iio/adc/ti-ads112c04.c
> new file mode 100644
> index 000000000000..28d3be81934f
> --- /dev/null
> +++ b/drivers/iio/adc/ti-ads112c04.c
> @@ -0,0 +1,378 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Texas Instruments ADS112C04 16-bit I2C ADC driver
> + * Based on TI Reference Code and standard Linux IIO framework.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/i2c.h>
> +#include <linux/delay.h>
> +#include <linux/interrupt.h>
> +#include <linux/mutex.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/bitfield.h>
> +#include <linux/iopoll.h>
> +#include <linux/property.h>
> +#include <linux/gpio/consumer.h>
> +
> +/* ADS112C04 Commands */
> +#define ADS112C04_CMD_RESET         0x06
> +#define ADS112C04_CMD_START_SYNC    0x08
> +#define ADS112C04_CMD_POWERDOWN     0x02
> +#define ADS112C04_CMD_RDATA         0x10
> +#define ADS112C04_CMD_RREG(reg)     (0x20 | ((reg) << 2))
> +#define ADS112C04_CMD_WREG(reg)     (0x40 | ((reg) << 2))
> +
> +/* Registers */
> +#define ADS112C04_REG_CONFIG0       0x00
> +#define ADS112C04_REG_CONFIG1       0x01
> +#define ADS112C04_REG_CONFIG2       0x02
> +#define ADS112C04_REG_CONFIG3       0x03
> +
> +#define ADS112C04_DRDY_MASK         BIT(7)
> +#define ADS112C04_MUX_MASK          GENMASK(7, 4)

We like to include the name of the register in the mask names
(and usually don't bother with _MASK to keep it shorter) and
organize the fields under the registers with a bit of indent.

Something like:

#define ADS112C04_REG_CONFIG0       0x00
#define   ADS112C04_CONFIG0_MUX       GENMASK(7, 4)
#define ADS112C04_REG_CONFIG1       0x01
#define ADS112C04_REG_CONFIG2       0x02
#define   ADS112C04_CONFIG2_DRDY      BIT(7)
#define ADS112C04_REG_CONFIG3       0x03

> +
> +struct ads112c04_state {
> +	struct i2c_client *client;
> +	/* Protects concurrent ADC reads and device configuration */
> +	struct mutex lock;
> +	struct completion completion;
> +	struct regulator *vref_reg;
> +	int vref_mv;
> +	u8 config0;
> +	u8 config1;
> +};
> +
> +static int ads112c04_write_cmd(struct i2c_client *client, u8 cmd)
> +{
> +	int ret = i2c_master_send(client, &cmd, 1);
> +
> +	return ret < 0 ? ret : 0;
> +}
> +
> +static int ads112c04_read_reg(struct i2c_client *client, u8 reg, u8 *val)
> +{
> +	u8 cmd = ADS112C04_CMD_RREG(reg);
> +	int ret;
> +
> +	ret = i2c_master_send(client, &cmd, 1);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = i2c_master_recv(client, val, 1);
> +	return ret < 0 ? ret : 0;

This can be done in one I2C call.

	ret = i2c_smbus_read_byte_data(client, cmd);
	if (ret < 0)
		reutrn ret;

	*val = ret;

	return 0;


> +}
> +
> +static int ads112c04_write_reg(struct i2c_client *client, u8 reg, u8 val)
> +{
> +	u8 buf[2] = { ADS112C04_CMD_WREG(reg), val };
> +	int ret;
> +
> +	ret = i2c_master_send(client, buf, 2);
> +	return ret < 0 ? ret : 0;

And here it can be:

	u8 cmd = ADS112C04_CMD_WREG(reg);

	return i2c_smbus_write_byte_data(client, cmd, val);

> +}
> +
> +static int ads112c04_wait_for_data(struct ads112c04_state *st)
> +{
> +	int ret;
> +	u8 val;

The slowest data rate is 20 SPS, so wouldn't 100 ms timeout be more than
enough? 1 second seems a bit long.


> +
> +	if (st->client->irq > 0) {
> +		ret = wait_for_completion_timeout(&st->completion, msecs_to_jiffies(1000));
> +		if (!ret)
> +			return -ETIMEDOUT;
> +		return 0;
> +	}
> +
> +	return read_poll_timeout(ads112c04_read_reg, ret,
> +				 (ret < 0 || (val & ADS112C04_DRDY_MASK)),
> +				 1000, 1000000, false,

When there is more than 3 or 4 zeros, it is helpful to use macros, like
1 * MICRO.

> +				 st->client, ADS112C04_REG_CONFIG2, &val);
> +}
> +
> +static int ads112c04_read_data(struct ads112c04_state *st, int *val)
> +{
> +	u8 cmd = ADS112C04_CMD_RDATA;
> +	__be16 buf;

I would call this data instead of buf.

> +	int ret;
> +
> +	ret = i2c_master_send(st->client, &cmd, 1);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = i2c_master_recv(st->client, (u8 *)&buf, 2);

sizeof(buf) instead of 2.

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


Or more simply ...

	ret = i2c_smbus_read_word_data(client, cmd);
	if (ret < 0)
		return ret;

	*val = sign_extend32(be16_to_cpu(ret), 15);

> +
> +	*val = sign_extend32(be16_to_cpu(buf), 15);
> +	return 0;
> +}
> +
> +static int ads112c04_get_adc_result(struct ads112c04_state *st,
> +				    struct iio_chan_spec const *chan,
> +				    int *val)
> +{
> +	int ret;
> +	u8 mux, new_config0;
> +
> +	mux = FIELD_PREP(ADS112C04_MUX_MASK, chan->address);
> +	new_config0 = (st->config0 & 0x0F) | mux;

~ADS112C04_MUX_MASK instead of 0x0F. 

Or:

	new_config0 = st->config0;
	FIELD_MODIFY(ADS112C04_MUX_MASK, &new_config0, chan->address);

> +
> +	if (st->config0 != new_config0) {
> +		ret = ads112c04_write_reg(st->client, ADS112C04_REG_CONFIG0, new_config0);
> +		if (ret < 0)
> +			return ret;
> +		st->config0 = new_config0;
> +	}
> +
> +	reinit_completion(&st->completion);
> +
> +	ret = ads112c04_write_cmd(st->client, ADS112C04_CMD_START_SYNC);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = ads112c04_wait_for_data(st);
> +	if (ret < 0)
> +		return ret;
> +
> +	return ads112c04_read_data(st, val);
> +}
> +
> +static int ads112c04_read_raw(struct iio_dev *indio_dev,
> +			      struct iio_chan_spec const *chan,
> +			      int *val, int *val2, long mask)
> +{
> +	struct ads112c04_state *st = iio_priv(indio_dev);
> +	int ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		mutex_lock(&st->lock);
> +		ret = ads112c04_get_adc_result(st, chan, val);
> +		mutex_unlock(&st->lock);
> +
> +		if (ret < 0)
> +			return ret;
> +		return IIO_VAL_INT;
> +
> +	case IIO_CHAN_INFO_SCALE:
> +		*val = st->vref_mv;
> +		*val2 = 15;
> +		return IIO_VAL_FRACTIONAL_LOG2;
> +
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static irqreturn_t ads112c04_irq_handler(int irq, void *private)
> +{
> +	struct iio_dev *indio_dev = private;
> +	struct ads112c04_state *st = iio_priv(indio_dev);
> +
> +	complete(&st->completion);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static const struct iio_info ads112c04_info = {
> +	.read_raw = ads112c04_read_raw,
> +};
> +
> +static void ads112c04_regulator_disable(void *data)
> +{
> +	regulator_disable(data);
> +}
> +
> +static int ads112c04_parse_channels(struct iio_dev *indio_dev)
> +{
> +	struct device *dev = indio_dev->dev.parent;
> +	struct iio_chan_spec *channels;
> +	u32 num_channels, i = 0, pair[2];
> +
> +	num_channels = device_get_named_child_node_count(dev, "channel");
> +	if (!num_channels)
> +		return -EINVAL;
> +
> +	channels = devm_kcalloc(dev, num_channels, sizeof(*channels), GFP_KERNEL);
> +	if (!channels)
> +		return -ENOMEM;
> +
> +	device_for_each_named_child_node_scoped(dev, child, "channel") {
> +		struct iio_chan_spec *spec = &channels[i];
> +
> +		spec->type = IIO_VOLTAGE;
> +		spec->indexed = 1;
> +		spec->info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE);
> +		spec->scan_index = i;

scan_index isn't currently used, so we could leave that out for now.

> +
> +		if (fwnode_property_present(child, "single-channel")) {
> +			fwnode_property_read_u32(child, "single-channel", &pair[0]);
> +			spec->channel = pair[0];
> +			spec->differential = 0;

differential is already 0, so don't need to set it here.

> +			spec->address = 0x08 + pair[0];
> +		} else if (fwnode_property_present(child, "diff-channels")) {
> +			fwnode_property_read_u32_array(child, "diff-channels", pair, 2);
> +			spec->channel = pair[0];
> +			spec->channel2 = pair[1];
> +			spec->differential = 1;
> +
> +			if (pair[0] == 0 && pair[1] == 1)
> +				spec->address = 0x00;
> +			else if (pair[0] == 0 && pair[1] == 2)
> +				spec->address = 0x01;
> +			else if (pair[0] == 0 && pair[1] == 3)
> +				spec->address = 0x02;
> +			else if (pair[0] == 1 && pair[1] == 0)
> +				spec->address = 0x03;
> +			else if (pair[0] == 1 && pair[1] == 2)
> +				spec->address = 0x04;
> +			else if (pair[0] == 1 && pair[1] == 3)
> +				spec->address = 0x05;
> +			else if (pair[0] == 2 && pair[1] == 3)
> +				spec->address = 0x06;
> +			else if (pair[0] == 3 && pair[1] == 2)
> +				spec->address = 0x07;
> +			else
> +				return -EINVAL;

Could be useful to use dev_err_probe() to print a message in this case. Helps
when you make a typo in the devicetree.

> +		} else {
> +			return -EINVAL;
> +		}

I would also check if the other properties mentioned in the DT binding review
exist here and return error if they do since they aren't implemented. See
similar example below with refp-refn properties.

> +		i++;
> +	}
> +
> +	indio_dev->channels = channels;
> +	indio_dev->num_channels = num_channels;

This should be i. num_channels could actually be > i if any node has status = "disabled";.

> +
> +	return 0;
> +}
> +
> +static int ads112c04_probe(struct i2c_client *client)
> +{
> +	struct iio_dev *indio_dev;
> +	struct ads112c04_state *st;
> +	struct gpio_desc *reset_gpio;
> +	int ret;
> +

Would be nice to make a local dev variable so we don't have to
write &clinet->dev so much.

> +	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*st));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	st = iio_priv(indio_dev);
> +	st->client = client;
> +
> +	ret = devm_mutex_init(&client->dev, &st->lock);
> +	if (ret)
> +		return ret;
> +
> +	init_completion(&st->completion);
> +
> +	indio_dev->name = "ads112c04";
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->info = &ads112c04_info;
> +
> +	ret = ads112c04_parse_channels(indio_dev);
> +	if (ret)
> +		return ret;
> +
> +	ret = devm_regulator_get_enable(&client->dev, "avdd");
> +	if (ret)
> +		return dev_err_probe(&client->dev, ret, "failed to get avdd regulator\n");
> +
> +	ret = devm_regulator_get_enable(&client->dev, "dvdd");
> +	if (ret)
> +		return dev_err_probe(&client->dev, ret, "failed to get dvdd regulator\n");
> +

We've been tending towards writing below like:

	if (device_property_present(dev, "refp-supply") {
		ret = devm_regulator_get_enable_read_voltage(dev, "refp");
		if (ret < 0)
			return dev_err_probe(dev, ret, "failed to read REFP voltage\n");

		...
	}

And I would check the other ref properties too even if we don't implement them.

	if (device_property_present(dev, "refn-supply")
		return dev_err_probe(dev, -EOPNOTSUPP, "refn-supply is not implemented\n");

	if (device_property_present(dev, "ti,refp-refn-resistor-ohms")
		return dev_err_probe(dev, -EOPNOTSUPP, "ti,refp-refn-resistor-ohms is not implemented\n");

> +	st->vref_reg = devm_regulator_get_optional(&client->dev, "refp");
> +	if (IS_ERR(st->vref_reg)) {
> +		ret = PTR_ERR(st->vref_reg);
> +		if (ret == -ENODEV) {
> +			st->vref_mv = 2048;
> +			st->config1 = 0x00;
> +		} else {
> +			return ret;
> +		}
> +	} else {
> +		ret = regulator_enable(st->vref_reg);
> +		if (ret)
> +			return ret;
> +
> +		ret = devm_add_action_or_reset(&client->dev, ads112c04_regulator_disable,
> +					       st->vref_reg);
> +		if (ret)
> +			return ret;
> +
> +		ret = regulator_get_voltage(st->vref_reg);
> +		if (ret < 0)
> +			return ret;
> +
> +		st->vref_mv = ret / 1000;

We've been writing this like:

		st->vref_mv = ret / (MICRO / MILLI);

> +		st->config1 = 0x02;

Add a macro and use FIELD_PREP().

> +	}

And as in the DT bindings reply, we should go ahead and make the reference
voltage per channel. Even if we don't need it now, it would be hard to change
it in the future without breaking existing users.

> +
> +	reset_gpio = devm_gpiod_get_optional(&client->dev, "reset", GPIOD_OUT_LOW);

If we make this GPIOD_OUT_HIGH, then we save a line later.

> +	if (IS_ERR(reset_gpio))
> +		return PTR_ERR(reset_gpio);
> +
> +	if (reset_gpio) {
> +		gpiod_set_value_cansleep(reset_gpio, 1);
> +		fsleep(1000);
> +		gpiod_set_value_cansleep(reset_gpio, 0);
> +	} else {
> +		ret = ads112c04_write_cmd(client, ADS112C04_CMD_RESET);
> +		if (ret < 0)
> +			return ret;
> +	}
> +
> +	fsleep(1000);
> +
> +	st->config0 = 0x01;

Needs a macro and FIELD_PREP().

Also, since we are parsing channels now, can/should we leave PGA enabled
for diff-channels? Otherwise add a comment explaining current choice.

> +	ret = ads112c04_write_reg(client, ADS112C04_REG_CONFIG0, st->config0);
> +	if (ret)
> +		return ret;
> +
> +	ret = ads112c04_write_reg(client, ADS112C04_REG_CONFIG1, st->config1);
> +	if (ret)
> +		return ret;
> +
> +	if (client->irq > 0) {
> +		ret = devm_request_irq(&client->dev, client->irq,
> +				       ads112c04_irq_handler,
> +				       0,

Can put 0 on the previous line.

> +				       indio_dev->name, indio_dev);
> +		if (ret) {
> +			dev_err(&client->dev, "Failed to request DRDY IRQ\n");
> +			return ret;
> +		}
> +	}
> +
> +	return devm_iio_device_register(&client->dev, indio_dev);
> +}
> +
> +static const struct i2c_device_id ads112c04_id[] = {
> +	{ .name = "ads112c04", .driver_data = 0 },

No need for .driver_data since it is 0.

> +	{ }
> +};
> +MODULE_DEVICE_TABLE(i2c, ads112c04_id);
> +
> +static const struct of_device_id ads112c04_of_match[] = {
> +	{ .compatible = "ti,ads112c04" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, ads112c04_of_match);
> +
> +static struct i2c_driver ads112c04_driver = {
> +	.driver = {
> +		.name = "ads112c04",
> +		.of_match_table = ads112c04_of_match,
> +	},
> +	.probe = ads112c04_probe,
> +	.id_table = ads112c04_id,
> +};
> +module_i2c_driver(ads112c04_driver);
> +
> +MODULE_AUTHOR("Kyle Hsieh <kylehsieh1995@gmail.com>");
> +MODULE_DESCRIPTION("Texas Instruments ADS112C04 ADC driver");
> +MODULE_LICENSE("GPL");
> 

Re: [PATCH v2 2/2] iio: adc: ti-ads112c04: Add support for TI ADS112C04

[David Lechner]

On 7/31/26 9:54 AM, David Lechner wrote:
> On 7/30/26 9:58 PM, Kyle Hsieh wrote:
>> Add IIO driver support for the Texas Instruments ADS112C04 (16-bit)
>> delta-sigma ADCs.
>>

...

>> +	st->config0 = 0x01;
> 
> Needs a macro and FIELD_PREP().

Correction: This one is a bit, so FIELD_PREP() is not needed.

> 
> Also, since we are parsing channels now, can/should we leave PGA enabled
> for diff-channels? Otherwise add a comment explaining current choice.
> 

Re: [PATCH v2 2/2] iio: adc: ti-ads112c04: Add support for TI ADS112C04

[Kyle Hsieh]

On Fri, Jul 31, 2026 at 10:54 PM David Lechner <dlechner@baylibre.com> wrote:
>
> On 7/30/26 9:58 PM, Kyle Hsieh wrote:
> > Add IIO driver support for the Texas Instruments ADS112C04 (16-bit)
> > delta-sigma ADCs.
> >
> > The driver implements:
> > - Single-shot conversions using the IIO raw read interface.
> > - Dynamic parsing of single-ended and differential channels from
> >   device tree child nodes.
> > - Hardware interrupt support via the DRDY pin, falling back to
> >   software polling if no IRQ is provided.
> > - Scale calculation based on the internal 2.048V reference.
> > - Dynamic reference voltage scaling via the regulator subsystem
> >   (refp-supply/refn-supply), falling back to the internal 2.048V
> >   reference if not specified.
> > - Hardware reset fallback using GPIO.
> >
> > Signed-off-by: Kyle Hsieh <kylehsieh1995@gmail.com>
> > ---
> >  drivers/iio/adc/Kconfig        |  10 ++
> >  drivers/iio/adc/Makefile       |   1 +
> >  drivers/iio/adc/ti-ads112c04.c | 378 +++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 389 insertions(+)
> >
> > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> > index 3755a81c1efd..402e841bc083 100644
> > --- a/drivers/iio/adc/Kconfig
> > +++ b/drivers/iio/adc/Kconfig
> > @@ -1789,6 +1789,16 @@ config TI_ADS1119
> >           This driver can also be built as a module. If so, the module will be
> >           called ti-ads1119.
> >
> > +config TI_ADS112C04
> > +     tristate "Texas Instruments ADS112C04 ADC"
> > +     depends on I2C
> > +     help
> > +       If you say yes here you get support for Texas Instruments
> > +       ADS112C04 (16-bit) I2C analog to digital converters.
> > +
> > +       This driver can also be built as a module. If so, the module will be
> > +       called ti-ads112c04.
> > +
> >  config TI_ADS124S08
> >       tristate "Texas Instruments ADS124S08"
> >       depends on SPI
> > diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> > index 707dd708912f..ebf9d4047a5a 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_ADS112C04)   += ti-ads112c04.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-ads112c04.c b/drivers/iio/adc/ti-ads112c04.c
> > new file mode 100644
> > index 000000000000..28d3be81934f
> > --- /dev/null
> > +++ b/drivers/iio/adc/ti-ads112c04.c
> > @@ -0,0 +1,378 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Texas Instruments ADS112C04 16-bit I2C ADC driver
> > + * Based on TI Reference Code and standard Linux IIO framework.
> > + */
> > +
> > +#include <linux/module.h>
> > +#include <linux/i2c.h>
> > +#include <linux/delay.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/mutex.h>
> > +#include <linux/iio/iio.h>
> > +#include <linux/iio/sysfs.h>
> > +#include <linux/regulator/consumer.h>
> > +#include <linux/bitfield.h>
> > +#include <linux/iopoll.h>
> > +#include <linux/property.h>
> > +#include <linux/gpio/consumer.h>
> > +
> > +/* ADS112C04 Commands */
> > +#define ADS112C04_CMD_RESET         0x06
> > +#define ADS112C04_CMD_START_SYNC    0x08
> > +#define ADS112C04_CMD_POWERDOWN     0x02
> > +#define ADS112C04_CMD_RDATA         0x10
> > +#define ADS112C04_CMD_RREG(reg)     (0x20 | ((reg) << 2))
> > +#define ADS112C04_CMD_WREG(reg)     (0x40 | ((reg) << 2))
> > +
> > +/* Registers */
> > +#define ADS112C04_REG_CONFIG0       0x00
> > +#define ADS112C04_REG_CONFIG1       0x01
> > +#define ADS112C04_REG_CONFIG2       0x02
> > +#define ADS112C04_REG_CONFIG3       0x03
> > +
> > +#define ADS112C04_DRDY_MASK         BIT(7)
> > +#define ADS112C04_MUX_MASK          GENMASK(7, 4)
>
> We like to include the name of the register in the mask names
> (and usually don't bother with _MASK to keep it shorter) and
> organize the fields under the registers with a bit of indent.
>
> Something like:
>
> #define ADS112C04_REG_CONFIG0       0x00
> #define   ADS112C04_CONFIG0_MUX       GENMASK(7, 4)
> #define ADS112C04_REG_CONFIG1       0x01
> #define ADS112C04_REG_CONFIG2       0x02
> #define   ADS112C04_CONFIG2_DRDY      BIT(7)
> #define ADS112C04_REG_CONFIG3       0x03
>
Thanks guiding, I will updated to use this naming convention with
proper indentation.
> > +
> > +struct ads112c04_state {
> > +     struct i2c_client *client;
> > +     /* Protects concurrent ADC reads and device configuration */
> > +     struct mutex lock;
> > +     struct completion completion;
> > +     struct regulator *vref_reg;
> > +     int vref_mv;
> > +     u8 config0;
> > +     u8 config1;
> > +};
> > +
> > +static int ads112c04_write_cmd(struct i2c_client *client, u8 cmd)
> > +{
> > +     int ret = i2c_master_send(client, &cmd, 1);
> > +
> > +     return ret < 0 ? ret : 0;
> > +}
> > +
> > +static int ads112c04_read_reg(struct i2c_client *client, u8 reg, u8 *val)
> > +{
> > +     u8 cmd = ADS112C04_CMD_RREG(reg);
> > +     int ret;
> > +
> > +     ret = i2c_master_send(client, &cmd, 1);
> > +     if (ret < 0)
> > +             return ret;
> > +
> > +     ret = i2c_master_recv(client, val, 1);
> > +     return ret < 0 ? ret : 0;
>
> This can be done in one I2C call.
>
>         ret = i2c_smbus_read_byte_data(client, cmd);
>         if (ret < 0)
>                 reutrn ret;
>
>         *val = ret;
>
>         return 0;
>
>
> > +}
> > +
> > +static int ads112c04_write_reg(struct i2c_client *client, u8 reg, u8 val)
> > +{
> > +     u8 buf[2] = { ADS112C04_CMD_WREG(reg), val };
> > +     int ret;
> > +
> > +     ret = i2c_master_send(client, buf, 2);
> > +     return ret < 0 ? ret : 0;
>
> And here it can be:
>
>         u8 cmd = ADS112C04_CMD_WREG(reg);
>
>         return i2c_smbus_write_byte_data(client, cmd, val);
>
Transitioning to the SMBus APIs has significantly cleaned up ads112c04_read_reg,
ads112c04_write_reg, and ads112c04_read_data. It also handles the I2C
error checking beautifully.
> > +}
> > +
> > +static int ads112c04_wait_for_data(struct ads112c04_state *st)
> > +{
> > +     int ret;
> > +     u8 val;
>
> The slowest data rate is 20 SPS, so wouldn't 100 ms timeout be more than
> enough? 1 second seems a bit long.
I will reduce the timeout to 100ms.
>
>
> > +
> > +     if (st->client->irq > 0) {
> > +             ret = wait_for_completion_timeout(&st->completion, msecs_to_jiffies(1000));
> > +             if (!ret)
> > +                     return -ETIMEDOUT;
> > +             return 0;
> > +     }
> > +
> > +     return read_poll_timeout(ads112c04_read_reg, ret,
> > +                              (ret < 0 || (val & ADS112C04_DRDY_MASK)),
> > +                              1000, 1000000, false,
>
> When there is more than 3 or 4 zeros, it is helpful to use macros, like
> 1 * MICRO.
Replaced the hardcoded 1000000 with 100 * USEC_PER_MSEC.
>
> > +                              st->client, ADS112C04_REG_CONFIG2, &val);
> > +}
> > +
> > +static int ads112c04_read_data(struct ads112c04_state *st, int *val)
> > +{
> > +     u8 cmd = ADS112C04_CMD_RDATA;
> > +     __be16 buf;
>
> I would call this data instead of buf.
>
> > +     int ret;
> > +
> > +     ret = i2c_master_send(st->client, &cmd, 1);
> > +     if (ret < 0)
> > +             return ret;
> > +
> > +     ret = i2c_master_recv(st->client, (u8 *)&buf, 2);
>
> sizeof(buf) instead of 2.
>
> > +     if (ret < 0)
> > +             return ret;
>
>
> Or more simply ...
>
>         ret = i2c_smbus_read_word_data(client, cmd);
>         if (ret < 0)
>                 return ret;
>
>         *val = sign_extend32(be16_to_cpu(ret), 15);
>
This is much cleaner than manually managing the buffer and i2c_master_recv.
> > +
> > +     *val = sign_extend32(be16_to_cpu(buf), 15);
> > +     return 0;
> > +}
> > +
> > +static int ads112c04_get_adc_result(struct ads112c04_state *st,
> > +                                 struct iio_chan_spec const *chan,
> > +                                 int *val)
> > +{
> > +     int ret;
> > +     u8 mux, new_config0;
> > +
> > +     mux = FIELD_PREP(ADS112C04_MUX_MASK, chan->address);
> > +     new_config0 = (st->config0 & 0x0F) | mux;
>
> ~ADS112C04_MUX_MASK instead of 0x0F.
>
> Or:
>
>         new_config0 = st->config0;
>         FIELD_MODIFY(ADS112C04_MUX_MASK, &new_config0, chan->address);
>
I am going to use st->config0 & ~ADS112C04_CONFIG0_MUX combined with
`FIELD_PREP().
> > +
> > +     if (st->config0 != new_config0) {
> > +             ret = ads112c04_write_reg(st->client, ADS112C04_REG_CONFIG0, new_config0);
> > +             if (ret < 0)
> > +                     return ret;
> > +             st->config0 = new_config0;
> > +     }
> > +
> > +     reinit_completion(&st->completion);
> > +
> > +     ret = ads112c04_write_cmd(st->client, ADS112C04_CMD_START_SYNC);
> > +     if (ret < 0)
> > +             return ret;
> > +
> > +     ret = ads112c04_wait_for_data(st);
> > +     if (ret < 0)
> > +             return ret;
> > +
> > +     return ads112c04_read_data(st, val);
> > +}
> > +
> > +static int ads112c04_read_raw(struct iio_dev *indio_dev,
> > +                           struct iio_chan_spec const *chan,
> > +                           int *val, int *val2, long mask)
> > +{
> > +     struct ads112c04_state *st = iio_priv(indio_dev);
> > +     int ret;
> > +
> > +     switch (mask) {
> > +     case IIO_CHAN_INFO_RAW:
> > +             mutex_lock(&st->lock);
> > +             ret = ads112c04_get_adc_result(st, chan, val);
> > +             mutex_unlock(&st->lock);
> > +
> > +             if (ret < 0)
> > +                     return ret;
> > +             return IIO_VAL_INT;
> > +
> > +     case IIO_CHAN_INFO_SCALE:
> > +             *val = st->vref_mv;
> > +             *val2 = 15;
> > +             return IIO_VAL_FRACTIONAL_LOG2;
> > +
> > +     default:
> > +             return -EINVAL;
> > +     }
> > +}
> > +
> > +static irqreturn_t ads112c04_irq_handler(int irq, void *private)
> > +{
> > +     struct iio_dev *indio_dev = private;
> > +     struct ads112c04_state *st = iio_priv(indio_dev);
> > +
> > +     complete(&st->completion);
> > +
> > +     return IRQ_HANDLED;
> > +}
> > +
> > +static const struct iio_info ads112c04_info = {
> > +     .read_raw = ads112c04_read_raw,
> > +};
> > +
> > +static void ads112c04_regulator_disable(void *data)
> > +{
> > +     regulator_disable(data);
> > +}
> > +
> > +static int ads112c04_parse_channels(struct iio_dev *indio_dev)
> > +{
> > +     struct device *dev = indio_dev->dev.parent;
> > +     struct iio_chan_spec *channels;
> > +     u32 num_channels, i = 0, pair[2];
> > +
> > +     num_channels = device_get_named_child_node_count(dev, "channel");
> > +     if (!num_channels)
> > +             return -EINVAL;
> > +
> > +     channels = devm_kcalloc(dev, num_channels, sizeof(*channels), GFP_KERNEL);
> > +     if (!channels)
> > +             return -ENOMEM;
> > +
> > +     device_for_each_named_child_node_scoped(dev, child, "channel") {
> > +             struct iio_chan_spec *spec = &channels[i];
> > +
> > +             spec->type = IIO_VOLTAGE;
> > +             spec->indexed = 1;
> > +             spec->info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE);
> > +             spec->scan_index = i;
>
> scan_index isn't currently used, so we could leave that out for now.
Removed.
>
> > +
> > +             if (fwnode_property_present(child, "single-channel")) {
> > +                     fwnode_property_read_u32(child, "single-channel", &pair[0]);
> > +                     spec->channel = pair[0];
> > +                     spec->differential = 0;
>
> differential is already 0, so don't need to set it here.
>
> > +                     spec->address = 0x08 + pair[0];
> > +             } else if (fwnode_property_present(child, "diff-channels")) {
> > +                     fwnode_property_read_u32_array(child, "diff-channels", pair, 2);
> > +                     spec->channel = pair[0];
> > +                     spec->channel2 = pair[1];
> > +                     spec->differential = 1;
> > +
> > +                     if (pair[0] == 0 && pair[1] == 1)
> > +                             spec->address = 0x00;
> > +                     else if (pair[0] == 0 && pair[1] == 2)
> > +                             spec->address = 0x01;
> > +                     else if (pair[0] == 0 && pair[1] == 3)
> > +                             spec->address = 0x02;
> > +                     else if (pair[0] == 1 && pair[1] == 0)
> > +                             spec->address = 0x03;
> > +                     else if (pair[0] == 1 && pair[1] == 2)
> > +                             spec->address = 0x04;
> > +                     else if (pair[0] == 1 && pair[1] == 3)
> > +                             spec->address = 0x05;
> > +                     else if (pair[0] == 2 && pair[1] == 3)
> > +                             spec->address = 0x06;
> > +                     else if (pair[0] == 3 && pair[1] == 2)
> > +                             spec->address = 0x07;
> > +                     else
> > +                             return -EINVAL;
>
> Could be useful to use dev_err_probe() to print a message in this case. Helps
> when you make a typo in the devicetree.
Remove the redundant `differential = 0` assignment.
I will also add `dev_err_probe()` to provide clear error messages for invalid
or missing channel properties.
>
> > +             } else {
> > +                     return -EINVAL;
> > +             }
>
> I would also check if the other properties mentioned in the DT binding review
> exist here and return error if they do since they aren't implemented. See
> similar example below with refp-refn properties.
 have added checks in both ads112c04_parse_channels and ads112c04_probe.
If properties like excitation-channels, refn-supply, or
ti,refp-refn-resistor-ohms are
present in the DT, the driver now returns -EOPNOTSUPP via dev_err_probe().
>
> > +             i++;
> > +     }
> > +
> > +     indio_dev->channels = channels;
> > +     indio_dev->num_channels = num_channels;
>
> This should be i. num_channels could actually be > i if any node has status = "disabled";.
Fixed. indio_dev->num_channels is now assigned the value of i.
>
> > +
> > +     return 0;
> > +}
> > +
> > +static int ads112c04_probe(struct i2c_client *client)
> > +{
> > +     struct iio_dev *indio_dev;
> > +     struct ads112c04_state *st;
> > +     struct gpio_desc *reset_gpio;
> > +     int ret;
> > +
>
> Would be nice to make a local dev variable so we don't have to
> write &clinet->dev so much.
Add struct device *dev = &client->dev; and updated the function accordingly.
>
> > +     indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*st));
> > +     if (!indio_dev)
> > +             return -ENOMEM;
> > +
> > +     st = iio_priv(indio_dev);
> > +     st->client = client;
> > +
> > +     ret = devm_mutex_init(&client->dev, &st->lock);
> > +     if (ret)
> > +             return ret;
> > +
> > +     init_completion(&st->completion);
> > +
> > +     indio_dev->name = "ads112c04";
> > +     indio_dev->modes = INDIO_DIRECT_MODE;
> > +     indio_dev->info = &ads112c04_info;
> > +
> > +     ret = ads112c04_parse_channels(indio_dev);
> > +     if (ret)
> > +             return ret;
> > +
> > +     ret = devm_regulator_get_enable(&client->dev, "avdd");
> > +     if (ret)
> > +             return dev_err_probe(&client->dev, ret, "failed to get avdd regulator\n");
> > +
> > +     ret = devm_regulator_get_enable(&client->dev, "dvdd");
> > +     if (ret)
> > +             return dev_err_probe(&client->dev, ret, "failed to get dvdd regulator\n");
> > +
>
> We've been tending towards writing below like:
>
>         if (device_property_present(dev, "refp-supply") {
>                 ret = devm_regulator_get_enable_read_voltage(dev, "refp");
>                 if (ret < 0)
>                         return dev_err_probe(dev, ret, "failed to read REFP voltage\n");
>
>                 ...
>         }
>
> And I would check the other ref properties too even if we don't implement them.
>
>         if (device_property_present(dev, "refn-supply")
>                 return dev_err_probe(dev, -EOPNOTSUPP, "refn-supply is not implemented\n");
>
>         if (device_property_present(dev, "ti,refp-refn-resistor-ohms")
>                 return dev_err_probe(dev, -EOPNOTSUPP, "ti,refp-refn-resistor-ohms is not implemented\n");
>
> > +     st->vref_reg = devm_regulator_get_optional(&client->dev, "refp");
> > +     if (IS_ERR(st->vref_reg)) {
> > +             ret = PTR_ERR(st->vref_reg);
> > +             if (ret == -ENODEV) {
> > +                     st->vref_mv = 2048;
> > +                     st->config1 = 0x00;
> > +             } else {
> > +                     return ret;
> > +             }
> > +     } else {
> > +             ret = regulator_enable(st->vref_reg);
> > +             if (ret)
> > +                     return ret;
> > +
> > +             ret = devm_add_action_or_reset(&client->dev, ads112c04_regulator_disable,
> > +                                            st->vref_reg);
> > +             if (ret)
> > +                     return ret;
> > +
> > +             ret = regulator_get_voltage(st->vref_reg);
> > +             if (ret < 0)
> > +                     return ret;
> > +
> > +             st->vref_mv = ret / 1000;
>
> We've been writing this like:
>
>                 st->vref_mv = ret / (MICRO / MILLI);
>
> > +             st->config1 = 0x02;
>
> Add a macro and use FIELD_PREP().
>
> > +     }
>
> And as in the DT bindings reply, we should go ahead and make the reference
> voltage per channel. Even if we don't need it now, it would be hard to change
> it in the future without breaking existing users.
>
> > +
> > +     reset_gpio = devm_gpiod_get_optional(&client->dev, "reset", GPIOD_OUT_LOW);
>
> If we make this GPIOD_OUT_HIGH, then we save a line later.
I will change to GPIOD_OUT_HIGH and removed the explicit set-to-1 line.
>
> > +     if (IS_ERR(reset_gpio))
> > +             return PTR_ERR(reset_gpio);
> > +
> > +     if (reset_gpio) {
> > +             gpiod_set_value_cansleep(reset_gpio, 1);
> > +             fsleep(1000);
> > +             gpiod_set_value_cansleep(reset_gpio, 0);
> > +     } else {
> > +             ret = ads112c04_write_cmd(client, ADS112C04_CMD_RESET);
> > +             if (ret < 0)
> > +                     return ret;
> > +     }
> > +
> > +     fsleep(1000);
> > +
> > +     st->config0 = 0x01;
>
> Needs a macro and FIELD_PREP().
>
> Also, since we are parsing channels now, can/should we leave PGA enabled
> for diff-channels? Otherwise add a comment explaining current choice.
For this minimal initial submission, I've opted to keep the PGA
bypassed (disabled)
by default for all channels.
This ensures users can safely measure up to VDD without hitting the PGA's
absolute input voltage limits.
>
> > +     ret = ads112c04_write_reg(client, ADS112C04_REG_CONFIG0, st->config0);
> > +     if (ret)
> > +             return ret;
> > +
> > +     ret = ads112c04_write_reg(client, ADS112C04_REG_CONFIG1, st->config1);
> > +     if (ret)
> > +             return ret;
> > +
> > +     if (client->irq > 0) {
> > +             ret = devm_request_irq(&client->dev, client->irq,
> > +                                    ads112c04_irq_handler,
> > +                                    0,
>
> Can put 0 on the previous line.
Formatting fixed.
>
> > +                                    indio_dev->name, indio_dev);
> > +             if (ret) {
> > +                     dev_err(&client->dev, "Failed to request DRDY IRQ\n");
> > +                     return ret;
> > +             }
> > +     }
> > +
> > +     return devm_iio_device_register(&client->dev, indio_dev);
> > +}
> > +
> > +static const struct i2c_device_id ads112c04_id[] = {
> > +     { .name = "ads112c04", .driver_data = 0 },
>
> No need for .driver_data since it is 0.
Removed. Thanks again for your time and the detailed guidance!
Best regards,
Kyle Hsieh
>
> > +     { }
> > +};
> > +MODULE_DEVICE_TABLE(i2c, ads112c04_id);
> > +
> > +static const struct of_device_id ads112c04_of_match[] = {
> > +     { .compatible = "ti,ads112c04" },
> > +     { }
> > +};
> > +MODULE_DEVICE_TABLE(of, ads112c04_of_match);
> > +
> > +static struct i2c_driver ads112c04_driver = {
> > +     .driver = {
> > +             .name = "ads112c04",
> > +             .of_match_table = ads112c04_of_match,
> > +     },
> > +     .probe = ads112c04_probe,
> > +     .id_table = ads112c04_id,
> > +};
> > +module_i2c_driver(ads112c04_driver);
> > +
> > +MODULE_AUTHOR("Kyle Hsieh <kylehsieh1995@gmail.com>");
> > +MODULE_DESCRIPTION("Texas Instruments ADS112C04 ADC driver");
> > +MODULE_LICENSE("GPL");
> >
>