* [PATCH 1/4] dt-bindings: iio: adc: add ti,ads122c14
2026-06-15 21:59 [PATCH 0/4] iio: adc: new ti-ads112c14 driver David Lechner (TI)
@ 2026-06-15 21:59 ` David Lechner (TI)
2026-06-16 0:26 ` Kurt Borja
2026-06-16 16:07 ` Conor Dooley
2026-06-15 22:00 ` [PATCH 2/4] iio: adc: add ti-ads112c14 driver David Lechner (TI)
` (3 subsequent siblings)
4 siblings, 2 replies; 20+ messages in thread
From: David Lechner (TI) @ 2026-06-15 21:59 UTC (permalink / raw)
To: Jonathan Cameron, Nuno Sá, Andy Shevchenko, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: Kurt Borja, Nguyen Minh Tien, linux-iio, devicetree, linux-kernel,
David Lechner
Add new bindings for ti,ads122c14 and similar devices.
This is an ADC that is primarily intended for use with temperature
sensors. There are a few unusual properties because of this. In
particular, the reference voltage source and current output requirements
can be different for each measurement, so these are included in the
channel bindings.
The REFP/REFN reference voltage is usually just connected to a resistor
that is being driven by the ADC's current outputs, so there is special
property for this case rather than requiring a regulator to be defined
to represent that.
ti,vref-source is reused from ti,tlv320adcx140.yaml (otherwise might
have preferred an enum of strings).
Signed-off-by: David Lechner (TI) <dlechner@baylibre.com>
---
.../devicetree/bindings/iio/adc/ti,ads112c14.yaml | 224 +++++++++++++++++++++
MAINTAINERS | 7 +
include/dt-bindings/iio/adc/ti,ads112c14.h | 11 +
3 files changed, 242 insertions(+)
diff --git a/Documentation/devicetree/bindings/iio/adc/ti,ads112c14.yaml b/Documentation/devicetree/bindings/iio/adc/ti,ads112c14.yaml
new file mode 100644
index 000000000000..dc7f37cad772
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/ti,ads112c14.yaml
@@ -0,0 +1,224 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/adc/ti,ads112c14.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Texas Instruments' ADS112C14 and similar ADC chips
+
+description: |
+ Supports the following Texas Instruments' ADC chips:
+ - ADS112C14 (16-bit)
+ - ADS122C14 (24-bit)
+
+ https://www.ti.com/lit/ds/symlink/ads122c14.pdf
+
+ These chips are primarily designed for use with temperature sensors such as
+ RTDs and thermocouples. The channel bindings reflect this in that each channel
+ represents the conditions required to make a measurement rather than strictly
+ just the physical input channels.
+
+maintainers:
+ - David Lechner <dlechner@baylibre.com>
+
+unevaluatedProperties: false
+
+properties:
+ compatible:
+ enum:
+ - ti,ads112c14
+ - ti,ads122c14
+
+ reg:
+ items:
+ - minimum: 0x40
+ maximum: 0x47
+
+ clocks:
+ maxItems: 1
+ description: Optional external clock connected to GPIO3 pin.
+
+ avdd-supply: true
+ dvdd-supply: true
+
+ refp-supply: true
+ refn-supply: true
+
+ refp-refn-resistor-ohms:
+ description:
+ The resistance of the external resistor between REFP and REFN when using
+ resistor bridge driven by current outputs for RTD measurements.
+
+ interrupts:
+ minItems: 1
+ items:
+ - description: FAULT interrupt (GPIO2 pin)
+ - description: DRDY interrupt (GPIO3 pin)
+
+ interrupt-names:
+ minItems: 1
+ maxItems: 2
+ items:
+ enum: [fault, drdy]
+
+ gpio-controller: true
+ '#gpio-cells':
+ const: 2
+
+ '#address-cells':
+ const: 1
+
+ '#size-cells':
+ const: 0
+
+patternProperties:
+ ^channel@[0-7]$:
+ $ref: adc.yaml
+
+ unevaluatedProperties: false
+
+ properties:
+ reg:
+ maximum: 16 # arbitrary limit, channel@ can be any combination of AIN0-AIN7
+
+ single-channel:
+ maximum: 7
+
+ diff-channels:
+ items:
+ maximum: 7
+
+ bipolar:
+ description:
+ Set this flag if the differential input can be negative.
+
+ excitation-channels:
+ description: AINx pins used as current output.
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+ minItems: 1
+ maxItems: 2
+ items:
+ maximum: 7
+
+ excitation-current-microamp:
+ description: The current output of the excitation channels in microamps.
+ minimum: 1
+ maximum: 1000
+
+ current-chopping:
+ $ref: /schemas/types.yaml#/definitions/flag
+ description:
+ If provided, the two excitation channels are to be used with current
+ chopping enabled.
+
+ ti,vref-source:
+ description: |
+ Indicates the source for the reference voltage for this channel.
+ 0 - Internal 2.5V reference
+ 1 - Internal 1.25V reference
+ 2 - External reference (REFP-REFN)
+ 3 - AVDD as reference
+
+ For convenience, macros for these values are available in
+ dt-bindings/iio/adc/ti,ads112c14.h.
+ $ref: /schemas/types.yaml#/definitions/uint32
+ maximum: 3
+ default: 0
+
+ dependencies:
+ excitation-channels: [ excitation-current-microamp ]
+ excitation-current-microamp: [ excitation-channels ]
+ current-chopping: [ excitation-channels ]
+
+ oneOf:
+ - required: [ single-channel ]
+ - required: [ diff-channels ]
+
+required:
+ - compatible
+ - reg
+ - avdd-supply
+ - dvdd-supply
+
+dependencies:
+ refn-supply: [ refp-supply ]
+
+allOf:
+ - oneOf:
+ - required: [ refp-supply ]
+ - required: [ refp-refn-resistor-ohms ]
+ - properties:
+ refp-supply: false
+ refn-supply: false
+ refp-refn-resistor-ohms: false
+
+examples:
+ - |
+ #include <dt-bindings/iio/adc/ti,ads112c14.h>
+
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ adc@40 {
+ compatible = "ti,ads112c14";
+ reg = <0x40>;
+
+ avdd-supply = <&avdd>;
+ dvdd-supply = <&dvdd>;
+
+ /* 3-Wire RTD: Two IDACs, One Measurement (AIN1-AIN2) */
+
+ refp-refn-resistor-ohms = <500>;
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ channel@0 {
+ reg = <0>;
+ diff-channels = <1>, <2>;
+ excitation-channels = <0>, <3>;
+ excitation-current-microamp = <500>;
+ current-chopping;
+ ti,vref-source = <ADS112C14_VREF_SOURCE_EXTERNAL>;
+ label = "rtd";
+ };
+ };
+ };
+ - |
+ #include <dt-bindings/iio/adc/ti,ads112c14.h>
+
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ adc@40 {
+ compatible = "ti,ads112c14";
+ reg = <0x40>;
+
+ avdd-supply = <&avdd>;
+ dvdd-supply = <&dvdd>;
+
+ /* Resistive Bridge Measurement With a Thermistor for Temperature Compensation*/
+
+ refp-supply = <&avdd>;
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ channel@0 {
+ reg = <0>;
+ diff-channels = <6>, <7>;
+ bipolar;
+ ti,vref-source = <ADS112C14_VREF_SOURCE_EXTERNAL>;
+ label = "bridge";
+ };
+
+ channel@1 {
+ reg = <1>;
+ diff-channels = <1>, <2>;
+ ti,vref-source = <ADS112C14_VREF_SOURCE_INTERNAL_2_5V>;
+ label = "thermistor";
+ };
+ };
+ };
diff --git a/MAINTAINERS b/MAINTAINERS
index f1caa6e5198b..9ce7c61b0c14 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -26911,6 +26911,13 @@ S: Maintained
F: Documentation/devicetree/bindings/iio/adc/ti,ads1119.yaml
F: drivers/iio/adc/ti-ads1119.c
+TI ADS112C14 ADC DRIVER
+M: David Lechner <dlechner@baylibre.com>
+L: linux-iio@vger.kernel.org
+S: Maintained
+F: Documentation/devicetree/bindings/iio/adc/ti,ads112c14.yaml
+F: include/dt-bindings/iio/adc/ti,ads112c14.h
+
TI ADS1018 ADC DRIVER
M: Kurt Borja <kuurtb@gmail.com>
L: linux-iio@vger.kernel.org
diff --git a/include/dt-bindings/iio/adc/ti,ads112c14.h b/include/dt-bindings/iio/adc/ti,ads112c14.h
new file mode 100644
index 000000000000..96906642fe41
--- /dev/null
+++ b/include/dt-bindings/iio/adc/ti,ads112c14.h
@@ -0,0 +1,11 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+
+#ifndef _DT_BINDINGS_TI_ADS112C14_H
+#define _DT_BINDINGS_TI_ADS112C14_H
+
+#define ADS112C14_VREF_SOURCE_INTERNAL_2_5V 0
+#define ADS112C14_VREF_SOURCE_INTERNAL_1_25V 1
+#define ADS112C14_VREF_SOURCE_EXTERNAL 2
+#define ADS112C14_VREF_SOURCE_AVDD 3
+
+#endif /* _DT_BINDINGS_TI_ADS112C14_H */
--
2.43.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH 1/4] dt-bindings: iio: adc: add ti,ads122c14
2026-06-15 21:59 ` [PATCH 1/4] dt-bindings: iio: adc: add ti,ads122c14 David Lechner (TI)
@ 2026-06-16 0:26 ` Kurt Borja
2026-06-16 15:22 ` David Lechner
2026-06-16 16:07 ` Conor Dooley
1 sibling, 1 reply; 20+ messages in thread
From: Kurt Borja @ 2026-06-16 0:26 UTC (permalink / raw)
To: David Lechner (TI), Jonathan Cameron, Nuno Sá,
Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Kurt Borja, Nguyen Minh Tien, linux-iio, devicetree, linux-kernel
On Mon Jun 15, 2026 at 4:59 PM -05, David Lechner (TI) wrote:
> Add new bindings for ti,ads122c14 and similar devices.
>
> This is an ADC that is primarily intended for use with temperature
> sensors. There are a few unusual properties because of this. In
> particular, the reference voltage source and current output requirements
> can be different for each measurement, so these are included in the
> channel bindings.
>
> The REFP/REFN reference voltage is usually just connected to a resistor
> that is being driven by the ADC's current outputs, so there is special
> property for this case rather than requiring a regulator to be defined
> to represent that.
>
> ti,vref-source is reused from ti,tlv320adcx140.yaml (otherwise might
> have preferred an enum of strings).
>
> Signed-off-by: David Lechner (TI) <dlechner@baylibre.com>
> ---
> .../devicetree/bindings/iio/adc/ti,ads112c14.yaml | 224 +++++++++++++++++++++
> MAINTAINERS | 7 +
> include/dt-bindings/iio/adc/ti,ads112c14.h | 11 +
> 3 files changed, 242 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/iio/adc/ti,ads112c14.yaml b/Documentation/devicetree/bindings/iio/adc/ti,ads112c14.yaml
> new file mode 100644
> index 000000000000..dc7f37cad772
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/ti,ads112c14.yaml
> @@ -0,0 +1,224 @@
[...]
> +patternProperties:
> + ^channel@[0-7]$:
> + $ref: adc.yaml
> +
> + unevaluatedProperties: false
> +
> + properties:
> + reg:
> + maximum: 16 # arbitrary limit, channel@ can be any combination of AIN0-AIN7
> +
> + single-channel:
> + maximum: 7
> +
> + diff-channels:
> + items:
> + maximum: 7
> +
> + bipolar:
> + description:
> + Set this flag if the differential input can be negative.
> +
> + excitation-channels:
I noticed this doesn't have the "ti," prefix. Is your plan to add this
to adc.yaml?
Also, do you think excitation-pins might be a better name?
> + description: AINx pins used as current output.
> + $ref: /schemas/types.yaml#/definitions/uint32-array
> + minItems: 1
> + maxItems: 2
> + items:
> + maximum: 7
> +
> + excitation-current-microamp:
> + description: The current output of the excitation channels in microamps.
> + minimum: 1
> + maximum: 1000
> +
> + current-chopping:
If you agree with the above comment, I think this too should be added.
In that case, can we call this something like
excitation-current-rotation for less ambiguity?
I say this because my device has an "IDAC rotation mode", but it also has
a "Chop Mode" which rotates analog inputs and averages consecutive
conversions.
> + $ref: /schemas/types.yaml#/definitions/flag
> + description:
> + If provided, the two excitation channels are to be used with current
> + chopping enabled.
> +
> + ti,vref-source:
> + description: |
> + Indicates the source for the reference voltage for this channel.
> + 0 - Internal 2.5V reference
> + 1 - Internal 1.25V reference
> + 2 - External reference (REFP-REFN)
> + 3 - AVDD as reference
> +
> + For convenience, macros for these values are available in
> + dt-bindings/iio/adc/ti,ads112c14.h.
> + $ref: /schemas/types.yaml#/definitions/uint32
> + maximum: 3
> + default: 0
> +
> + dependencies:
> + excitation-channels: [ excitation-current-microamp ]
> + excitation-current-microamp: [ excitation-channels ]
> + current-chopping: [ excitation-channels ]
> +
> + oneOf:
> + - required: [ single-channel ]
> + - required: [ diff-channels ]
[...]
--
Thanks,
~ Kurt
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/4] dt-bindings: iio: adc: add ti,ads122c14
2026-06-16 0:26 ` Kurt Borja
@ 2026-06-16 15:22 ` David Lechner
2026-06-16 17:31 ` Kurt Borja
0 siblings, 1 reply; 20+ messages in thread
From: David Lechner @ 2026-06-16 15:22 UTC (permalink / raw)
To: Kurt Borja, Jonathan Cameron, Nuno Sá, Andy Shevchenko,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Nguyen Minh Tien, linux-iio, devicetree, linux-kernel
On 6/15/26 7:26 PM, Kurt Borja wrote:
> On Mon Jun 15, 2026 at 4:59 PM -05, David Lechner (TI) wrote:
>> Add new bindings for ti,ads122c14 and similar devices.
>>
>> This is an ADC that is primarily intended for use with temperature
>> sensors. There are a few unusual properties because of this. In
>> particular, the reference voltage source and current output requirements
>> can be different for each measurement, so these are included in the
>> channel bindings.
>>
>> The REFP/REFN reference voltage is usually just connected to a resistor
>> that is being driven by the ADC's current outputs, so there is special
>> property for this case rather than requiring a regulator to be defined
>> to represent that.
>>
>> ti,vref-source is reused from ti,tlv320adcx140.yaml (otherwise might
>> have preferred an enum of strings).
>>
>> Signed-off-by: David Lechner (TI) <dlechner@baylibre.com>
>> ---
>> .../devicetree/bindings/iio/adc/ti,ads112c14.yaml | 224 +++++++++++++++++++++
>> MAINTAINERS | 7 +
>> include/dt-bindings/iio/adc/ti,ads112c14.h | 11 +
>> 3 files changed, 242 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/iio/adc/ti,ads112c14.yaml b/Documentation/devicetree/bindings/iio/adc/ti,ads112c14.yaml
>> new file mode 100644
>> index 000000000000..dc7f37cad772
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/iio/adc/ti,ads112c14.yaml
>> @@ -0,0 +1,224 @@
>
> [...]
>
>> +patternProperties:
>> + ^channel@[0-7]$:
>> + $ref: adc.yaml
>> +
>> + unevaluatedProperties: false
>> +
>> + properties:
>> + reg:
>> + maximum: 16 # arbitrary limit, channel@ can be any combination of AIN0-AIN7
>> +
>> + single-channel:
>> + maximum: 7
>> +
>> + diff-channels:
>> + items:
>> + maximum: 7
>> +
>> + bipolar:
>> + description:
>> + Set this flag if the differential input can be negative.
>> +
>> + excitation-channels:
>
> I noticed this doesn't have the "ti," prefix. Is your plan to add this
> to adc.yaml?
I hadn't really though about it. I guess it could make sense.
>
> Also, do you think excitation-pins might be a better name?
I used -channels to match e.g. diff-channels. The same values apply to
both properties.
>
>> + description: AINx pins used as current output.
>> + $ref: /schemas/types.yaml#/definitions/uint32-array
>> + minItems: 1
>> + maxItems: 2
>> + items:
>> + maximum: 7
>> +
>> + excitation-current-microamp:
>> + description: The current output of the excitation channels in microamps.
>> + minimum: 1
>> + maximum: 1000
>> +
>> + current-chopping:
>
> If you agree with the above comment, I think this too should be added.
> In that case, can we call this something like
> excitation-current-rotation for less ambiguity?
I think excitation-channel-rotation would be fine.
>
> I say this because my device has an "IDAC rotation mode", but it also has
> a "Chop Mode" which rotates analog inputs and averages consecutive
> conversions.
And this one sounds like it could be named input-channel-rotation. Would
this also need a 2nd set of single-channel or diff-channels to the extra
inputs?
>
>> + $ref: /schemas/types.yaml#/definitions/flag
>> + description:
>> + If provided, the two excitation channels are to be used with current
>> + chopping enabled.
>> +
>> + ti,vref-source:
>> + description: |
>> + Indicates the source for the reference voltage for this channel.
>> + 0 - Internal 2.5V reference
>> + 1 - Internal 1.25V reference
>> + 2 - External reference (REFP-REFN)
>> + 3 - AVDD as reference
>> +
>> + For convenience, macros for these values are available in
>> + dt-bindings/iio/adc/ti,ads112c14.h.
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> + maximum: 3
>> + default: 0
>> +
>> + dependencies:
>> + excitation-channels: [ excitation-current-microamp ]
>> + excitation-current-microamp: [ excitation-channels ]
>> + current-chopping: [ excitation-channels ]
>> +
>> + oneOf:
>> + - required: [ single-channel ]
>> + - required: [ diff-channels ]
>
> [...]
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/4] dt-bindings: iio: adc: add ti,ads122c14
2026-06-16 15:22 ` David Lechner
@ 2026-06-16 17:31 ` Kurt Borja
0 siblings, 0 replies; 20+ messages in thread
From: Kurt Borja @ 2026-06-16 17:31 UTC (permalink / raw)
To: David Lechner, Kurt Borja, Jonathan Cameron, Nuno Sá,
Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Nguyen Minh Tien, linux-iio, devicetree, linux-kernel
On Tue Jun 16, 2026 at 10:22 AM -05, David Lechner wrote:
> On 6/15/26 7:26 PM, Kurt Borja wrote:
>> On Mon Jun 15, 2026 at 4:59 PM -05, David Lechner (TI) wrote:
>>> Add new bindings for ti,ads122c14 and similar devices.
>>>
>>> This is an ADC that is primarily intended for use with temperature
>>> sensors. There are a few unusual properties because of this. In
>>> particular, the reference voltage source and current output requirements
>>> can be different for each measurement, so these are included in the
>>> channel bindings.
>>>
>>> The REFP/REFN reference voltage is usually just connected to a resistor
>>> that is being driven by the ADC's current outputs, so there is special
>>> property for this case rather than requiring a regulator to be defined
>>> to represent that.
>>>
>>> ti,vref-source is reused from ti,tlv320adcx140.yaml (otherwise might
>>> have preferred an enum of strings).
>>>
>>> Signed-off-by: David Lechner (TI) <dlechner@baylibre.com>
>>> ---
>>> .../devicetree/bindings/iio/adc/ti,ads112c14.yaml | 224 +++++++++++++++++++++
>>> MAINTAINERS | 7 +
>>> include/dt-bindings/iio/adc/ti,ads112c14.h | 11 +
>>> 3 files changed, 242 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/iio/adc/ti,ads112c14.yaml b/Documentation/devicetree/bindings/iio/adc/ti,ads112c14.yaml
>>> new file mode 100644
>>> index 000000000000..dc7f37cad772
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/iio/adc/ti,ads112c14.yaml
>>> @@ -0,0 +1,224 @@
>>
>> [...]
>>
>>> +patternProperties:
>>> + ^channel@[0-7]$:
>>> + $ref: adc.yaml
>>> +
>>> + unevaluatedProperties: false
>>> +
>>> + properties:
>>> + reg:
>>> + maximum: 16 # arbitrary limit, channel@ can be any combination of AIN0-AIN7
>>> +
>>> + single-channel:
>>> + maximum: 7
>>> +
>>> + diff-channels:
>>> + items:
>>> + maximum: 7
>>> +
>>> + bipolar:
>>> + description:
>>> + Set this flag if the differential input can be negative.
>>> +
>>> + excitation-channels:
>>
>> I noticed this doesn't have the "ti," prefix. Is your plan to add this
>> to adc.yaml?
>
> I hadn't really though about it. I guess it could make sense.
I think it would be valuable, given that TI is producing quite a few of
this kind of devices.
>
>>
>> Also, do you think excitation-pins might be a better name?
>
> I used -channels to match e.g. diff-channels. The same values apply to
> both properties.
>
>>
>>> + description: AINx pins used as current output.
>>> + $ref: /schemas/types.yaml#/definitions/uint32-array
>>> + minItems: 1
>>> + maxItems: 2
>>> + items:
>>> + maximum: 7
>>> +
>>> + excitation-current-microamp:
>>> + description: The current output of the excitation channels in microamps.
>>> + minimum: 1
>>> + maximum: 1000
>>> +
>>> + current-chopping:
>>
>> If you agree with the above comment, I think this too should be added.
>> In that case, can we call this something like
>> excitation-current-rotation for less ambiguity?
>
> I think excitation-channel-rotation would be fine.
I like it.
>
>>
>> I say this because my device has an "IDAC rotation mode", but it also has
>> a "Chop Mode" which rotates analog inputs and averages consecutive
>> conversions.
>
> And this one sounds like it could be named input-channel-rotation. Would
> this also need a 2nd set of single-channel or diff-channels to the extra
> inputs?
No, just one set. It rotates the positive and negative inputs.
I'll go with input-channel-rotation then!
>
>>
>>> + $ref: /schemas/types.yaml#/definitions/flag
>>> + description:
>>> + If provided, the two excitation channels are to be used with current
>>> + chopping enabled.
>>> +
>>> + ti,vref-source:
>>> + description: |
>>> + Indicates the source for the reference voltage for this channel.
>>> + 0 - Internal 2.5V reference
>>> + 1 - Internal 1.25V reference
>>> + 2 - External reference (REFP-REFN)
>>> + 3 - AVDD as reference
>>> +
>>> + For convenience, macros for these values are available in
>>> + dt-bindings/iio/adc/ti,ads112c14.h.
>>> + $ref: /schemas/types.yaml#/definitions/uint32
>>> + maximum: 3
>>> + default: 0
>>> +
>>> + dependencies:
>>> + excitation-channels: [ excitation-current-microamp ]
>>> + excitation-current-microamp: [ excitation-channels ]
>>> + current-chopping: [ excitation-channels ]
>>> +
>>> + oneOf:
>>> + - required: [ single-channel ]
>>> + - required: [ diff-channels ]
>>
>> [...]
>>
--
Thanks,
~ Kurt
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/4] dt-bindings: iio: adc: add ti,ads122c14
2026-06-15 21:59 ` [PATCH 1/4] dt-bindings: iio: adc: add ti,ads122c14 David Lechner (TI)
2026-06-16 0:26 ` Kurt Borja
@ 2026-06-16 16:07 ` Conor Dooley
1 sibling, 0 replies; 20+ messages in thread
From: Conor Dooley @ 2026-06-16 16:07 UTC (permalink / raw)
To: David Lechner (TI)
Cc: Jonathan Cameron, Nuno Sá, Andy Shevchenko, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Kurt Borja, Nguyen Minh Tien,
linux-iio, devicetree, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 9413 bytes --]
On Mon, Jun 15, 2026 at 04:59:59PM -0500, David Lechner (TI) wrote:
> Add new bindings for ti,ads122c14 and similar devices.
>
> This is an ADC that is primarily intended for use with temperature
> sensors. There are a few unusual properties because of this. In
> particular, the reference voltage source and current output requirements
> can be different for each measurement, so these are included in the
> channel bindings.
>
> The REFP/REFN reference voltage is usually just connected to a resistor
> that is being driven by the ADC's current outputs, so there is special
> property for this case rather than requiring a regulator to be defined
> to represent that.
>
> ti,vref-source is reused from ti,tlv320adcx140.yaml (otherwise might
> have preferred an enum of strings).
>
> Signed-off-by: David Lechner (TI) <dlechner@baylibre.com>
> ---
> .../devicetree/bindings/iio/adc/ti,ads112c14.yaml | 224 +++++++++++++++++++++
> MAINTAINERS | 7 +
> include/dt-bindings/iio/adc/ti,ads112c14.h | 11 +
> 3 files changed, 242 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/iio/adc/ti,ads112c14.yaml b/Documentation/devicetree/bindings/iio/adc/ti,ads112c14.yaml
> new file mode 100644
> index 000000000000..dc7f37cad772
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/ti,ads112c14.yaml
> @@ -0,0 +1,224 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/adc/ti,ads112c14.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Texas Instruments' ADS112C14 and similar ADC chips
> +
> +description: |
> + Supports the following Texas Instruments' ADC chips:
> + - ADS112C14 (16-bit)
> + - ADS122C14 (24-bit)
> +
> + https://www.ti.com/lit/ds/symlink/ads122c14.pdf
> +
> + These chips are primarily designed for use with temperature sensors such as
> + RTDs and thermocouples. The channel bindings reflect this in that each channel
> + represents the conditions required to make a measurement rather than strictly
> + just the physical input channels.
> +
> +maintainers:
> + - David Lechner <dlechner@baylibre.com>
> +
> +unevaluatedProperties: false
Weird positioning of this.
> +
> +properties:
> + compatible:
> + enum:
> + - ti,ads112c14
> + - ti,ads122c14
> +
> + reg:
> + items:
> + - minimum: 0x40
> + maximum: 0x47
> +
> + clocks:
> + maxItems: 1
> + description: Optional external clock connected to GPIO3 pin.
> +
> + avdd-supply: true
> + dvdd-supply: true
> +
> + refp-supply: true
> + refn-supply: true
> +
> + refp-refn-resistor-ohms:
Missing prefix here and elsewhere.
> + description:
> + The resistance of the external resistor between REFP and REFN when using
> + resistor bridge driven by current outputs for RTD measurements.
> +
> + interrupts:
> + minItems: 1
> + items:
> + - description: FAULT interrupt (GPIO2 pin)
> + - description: DRDY interrupt (GPIO3 pin)
> +
> + interrupt-names:
> + minItems: 1
> + maxItems: 2
> + items:
> + enum: [fault, drdy]
> +
> + gpio-controller: true
> + '#gpio-cells':
> + const: 2
> +
> + '#address-cells':
> + const: 1
> +
> + '#size-cells':
> + const: 0
> +
> +patternProperties:
> + ^channel@[0-7]$:
> + $ref: adc.yaml
> +
> + unevaluatedProperties: false
> +
> + properties:
> + reg:
> + maximum: 16 # arbitrary limit, channel@ can be any combination of AIN0-AIN7
> +
> + single-channel:
> + maximum: 7
> +
> + diff-channels:
> + items:
> + maximum: 7
> +
> + bipolar:
> + description:
> + Set this flag if the differential input can be negative.
> +
> + excitation-channels:
(here)
> + description: AINx pins used as current output.
> + $ref: /schemas/types.yaml#/definitions/uint32-array
> + minItems: 1
> + maxItems: 2
> + items:
> + maximum: 7
> +
> + excitation-current-microamp:
(here)
> + description: The current output of the excitation channels in microamps.
> + minimum: 1
> + maximum: 1000
> +
> + current-chopping:
(and here)
> + $ref: /schemas/types.yaml#/definitions/flag
> + description:
> + If provided, the two excitation channels are to be used with current
> + chopping enabled.
> +
> + ti,vref-source:
> + description: |
> + Indicates the source for the reference voltage for this channel.
> + 0 - Internal 2.5V reference
> + 1 - Internal 1.25V reference
> + 2 - External reference (REFP-REFN)
> + 3 - AVDD as reference
My usual complaint here about things you have to make macros for, could
these just be strings from the get-go?
> +
> + For convenience, macros for these values are available in
> + dt-bindings/iio/adc/ti,ads112c14.h.
> + $ref: /schemas/types.yaml#/definitions/uint32
> + maximum: 3
> + default: 0
> +
> + dependencies:
> + excitation-channels: [ excitation-current-microamp ]
> + excitation-current-microamp: [ excitation-channels ]
> + current-chopping: [ excitation-channels ]
> +
> + oneOf:
> + - required: [ single-channel ]
> + - required: [ diff-channels ]
> +
> +required:
> + - compatible
> + - reg
> + - avdd-supply
> + - dvdd-supply
> +
> +dependencies:
> + refn-supply: [ refp-supply ]
> +
> +allOf:
> + - oneOf:
"allOf: - oneOf:" is equivalent to just writing "oneOf:"
pw-bot: changes-requested
Thanks,
Conor.
> + - required: [ refp-supply ]
> + - required: [ refp-refn-resistor-ohms ]
> + - properties:
> + refp-supply: false
> + refn-supply: false
> + refp-refn-resistor-ohms: false
> +
> +examples:
> + - |
> + #include <dt-bindings/iio/adc/ti,ads112c14.h>
> +
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + adc@40 {
> + compatible = "ti,ads112c14";
> + reg = <0x40>;
> +
> + avdd-supply = <&avdd>;
> + dvdd-supply = <&dvdd>;
> +
> + /* 3-Wire RTD: Two IDACs, One Measurement (AIN1-AIN2) */
> +
> + refp-refn-resistor-ohms = <500>;
> +
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + channel@0 {
> + reg = <0>;
> + diff-channels = <1>, <2>;
> + excitation-channels = <0>, <3>;
> + excitation-current-microamp = <500>;
> + current-chopping;
> + ti,vref-source = <ADS112C14_VREF_SOURCE_EXTERNAL>;
> + label = "rtd";
> + };
> + };
> + };
> + - |
> + #include <dt-bindings/iio/adc/ti,ads112c14.h>
> +
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + adc@40 {
> + compatible = "ti,ads112c14";
> + reg = <0x40>;
> +
> + avdd-supply = <&avdd>;
> + dvdd-supply = <&dvdd>;
> +
> + /* Resistive Bridge Measurement With a Thermistor for Temperature Compensation*/
> +
> + refp-supply = <&avdd>;
> +
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + channel@0 {
> + reg = <0>;
> + diff-channels = <6>, <7>;
> + bipolar;
> + ti,vref-source = <ADS112C14_VREF_SOURCE_EXTERNAL>;
> + label = "bridge";
> + };
> +
> + channel@1 {
> + reg = <1>;
> + diff-channels = <1>, <2>;
> + ti,vref-source = <ADS112C14_VREF_SOURCE_INTERNAL_2_5V>;
> + label = "thermistor";
> + };
> + };
> + };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index f1caa6e5198b..9ce7c61b0c14 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -26911,6 +26911,13 @@ S: Maintained
> F: Documentation/devicetree/bindings/iio/adc/ti,ads1119.yaml
> F: drivers/iio/adc/ti-ads1119.c
>
> +TI ADS112C14 ADC DRIVER
> +M: David Lechner <dlechner@baylibre.com>
> +L: linux-iio@vger.kernel.org
> +S: Maintained
> +F: Documentation/devicetree/bindings/iio/adc/ti,ads112c14.yaml
> +F: include/dt-bindings/iio/adc/ti,ads112c14.h
> +
> TI ADS1018 ADC DRIVER
> M: Kurt Borja <kuurtb@gmail.com>
> L: linux-iio@vger.kernel.org
> diff --git a/include/dt-bindings/iio/adc/ti,ads112c14.h b/include/dt-bindings/iio/adc/ti,ads112c14.h
> new file mode 100644
> index 000000000000..96906642fe41
> --- /dev/null
> +++ b/include/dt-bindings/iio/adc/ti,ads112c14.h
> @@ -0,0 +1,11 @@
> +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
> +
> +#ifndef _DT_BINDINGS_TI_ADS112C14_H
> +#define _DT_BINDINGS_TI_ADS112C14_H
> +
> +#define ADS112C14_VREF_SOURCE_INTERNAL_2_5V 0
> +#define ADS112C14_VREF_SOURCE_INTERNAL_1_25V 1
> +#define ADS112C14_VREF_SOURCE_EXTERNAL 2
> +#define ADS112C14_VREF_SOURCE_AVDD 3
> +
> +#endif /* _DT_BINDINGS_TI_ADS112C14_H */
>
> --
> 2.43.0
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 2/4] iio: adc: add ti-ads112c14 driver
2026-06-15 21:59 [PATCH 0/4] iio: adc: new ti-ads112c14 driver David Lechner (TI)
2026-06-15 21:59 ` [PATCH 1/4] dt-bindings: iio: adc: add ti,ads122c14 David Lechner (TI)
@ 2026-06-15 22:00 ` David Lechner (TI)
2026-06-16 7:32 ` Andy Shevchenko
2026-06-15 22:00 ` [PATCH 3/4] iio: adc: ti-ads112c14: implement gain on internal short SYS_MON channel David Lechner (TI)
` (2 subsequent siblings)
4 siblings, 1 reply; 20+ messages in thread
From: David Lechner (TI) @ 2026-06-15 22:00 UTC (permalink / raw)
To: Jonathan Cameron, Nuno Sá, Andy Shevchenko, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: Kurt Borja, Nguyen Minh Tien, linux-iio, devicetree, linux-kernel,
David Lechner
Add a new driver for the TI ADS112C14/ADS122C14 ADC chips.
This first step is adding a very basic driver that only supports power
on/reset and reading the system monitor channels.
ADS112C14_SYS_MON_CHANNEL_SHORT is the last channel rather than being in
logical order by address to keep the voltage channels together and in
case we find we need to add variants of this channel with different
voltage reference later.
Signed-off-by: David Lechner (TI) <dlechner@baylibre.com>
---
A few other notes for review that didn't seem worth putting in the
commit message:
* I intentionally did not use bulk regmap because later we may need to
get the voltage of the avdd supply.
* I left some comments in the code where the code might look funny (e.g.
to reduce future diff) or does not exactly match the datasheet, in
which case later changes will address that.
---
MAINTAINERS | 1 +
drivers/iio/adc/Kconfig | 12 +
drivers/iio/adc/Makefile | 1 +
drivers/iio/adc/ti-ads112c14.c | 536 +++++++++++++++++++++++++++++++++++++++++
4 files changed, 550 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 9ce7c61b0c14..084146a0f01e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -26916,6 +26916,7 @@ M: David Lechner <dlechner@baylibre.com>
L: linux-iio@vger.kernel.org
S: Maintained
F: Documentation/devicetree/bindings/iio/adc/ti,ads112c14.yaml
+F: drivers/iio/adc/ti-ads112c14.c
F: include/dt-bindings/iio/adc/ti,ads112c14.h
TI ADS1018 ADC DRIVER
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 6fb0766ca27a..bfcaecc85f24 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -1787,6 +1787,18 @@ config TI_ADS1119
This driver can also be built as a module. If so, the module will be
called ti-ads1119.
+config TI_ADS112C14
+ tristate "Texas Instruments ADS112C14/ADS122C14"
+ depends on I2C
+ select REGMAP
+ select REGMAP_I2C
+ help
+ If you say yes here you get support for Texas Instruments ADS112C14,
+ ADS122C14 ADC chips.
+
+ This driver can also be built as a module. If so, the module will be
+ called ti-ads112c14.
+
config TI_ADS124S08
tristate "Texas Instruments ADS124S08"
depends on SPI
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index 707dd708912f..72f4c6b33ca8 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_ADS112C14) += ti-ads112c14.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-ads112c14.c b/drivers/iio/adc/ti-ads112c14.c
new file mode 100644
index 000000000000..97097ae2a487
--- /dev/null
+++ b/drivers/iio/adc/ti-ads112c14.c
@@ -0,0 +1,536 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * IIO driver for Texas Instruments ADS112C14 and similar ADCs.
+ *
+ * Copyright (C) 2026 Texas Instruments Incorporated - https://www.ti.com/
+ * Copyright (C) 2026 Baylibre Inc.
+ *
+ * Datasheet: https://www.ti.com/lit/ds/symlink/ads122c14.pdf
+ */
+
+#include <linux/bitfield.h>
+#include <linux/delay.h>
+#include <linux/dev_printk.h>
+#include <linux/i2c.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/types.h>
+#include <linux/math64.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/property.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+#include <linux/sysfs.h>
+#include <linux/time64.h>
+#include <linux/types.h>
+#include <linux/unaligned.h>
+#include <linux/units.h>
+
+/* Datasheet t_d(RST) - time to wait after reset before next I2C use. */
+#define ADS112C14_DELAY_RESET_us 500
+
+#define ADS112C14_CMD_RDATA 0x00
+#define ADS112C14_CMD_RREG 0x40
+#define ADS112C14_CMD_WREG 0x80
+
+#define ADS112C14_REG_DEVICE_ID 0x00
+#define ADS112C14_DEVICE_ID_BITS GENMASK(3, 0)
+
+#define ADS112C14_REG_REVISION_ID 0x01
+
+#define ADS112C14_REG_STATUS_MSB 0x02
+#define ADS112C14_STATUS_MSB_RESETN BIT(7)
+#define ADS112C14_STATUS_MSB_AVDD_UVN BIT(6)
+#define ADS112C14_STATUS_MSB_REF_UVN BIT(5)
+#define ADS112C14_STATUS_MSB_REG_MAP_CRC_FAULTN BIT(3)
+#define ADS112C14_STATUS_MSB_MEM_FAULTN BIT(2)
+#define ADS112C14_STATUS_MSB_REG_WRITE_FAULTN BIT(1)
+#define ADS112C14_STATUS_MSB_DRDY BIT(0)
+
+#define ADS112C14_REG_STATUS_LSB 0x03
+#define ADS112C14_STATUS_LSB_CONV_COUNT GENMASK(7, 4)
+#define ADS112C14_STATUS_LSB_GPIO3_DAT_IN BIT(3)
+#define ADS112C14_STATUS_LSB_GPIO2_DAT_IN BIT(2)
+#define ADS112C14_STATUS_LSB_GPIO1_DAT_IN BIT(1)
+#define ADS112C14_STATUS_LSB_GPIO0_DAT_IN BIT(0)
+
+#define ADS112C14_REG_CONVERSION_CTRL 0x04
+#define ADS112C14_CONVERSION_CTRL_RESET GENMASK(7, 2)
+#define ADS112C14_CONVERSION_CTRL_START BIT(1)
+#define ADS112C14_CONVERSION_CTRL_STOP BIT(0)
+
+#define ADS112C14_REG_DEVICE_CFG 0x05
+#define ADS112C14_DEVICE_CFG_PWDN BIT(7)
+#define ADS112C14_DEVICE_CFG_STBY_MODE BIT(6)
+#define ADS112C14_DEVICE_CFG_BOCS GENMASK(5, 4)
+#define ADS112C14_DEVICE_CFG_CLK_SEL BIT(3)
+#define ADS112C14_DEVICE_CFG_CONV_MODE BIT(2)
+#define ADS112C14_DEVICE_CFG_SPEED_MODE GENMASK(1, 0)
+
+#define ADS112C14_REG_DATA_RATE_CFG 0x06
+#define ADS112C14_DATA_RATE_CFG_DELAY GENMASK(7, 4)
+#define ADS112C14_DATA_RATE_CFG_GC_EN BIT(3)
+#define ADS112C14_DATA_RATE_CFG_FLTR_OSR GENMASK(2, 0)
+
+#define ADS112C14_REG_MUX_CFG 0x07
+#define ADS112C14_MUX_CFG_AINP GENMASK(7, 4)
+#define ADS112C14_MUX_CFG_AINN GENMASK(3, 0)
+
+#define ADS112C14_REG_GAIN_CFG 0x08
+#define ADS112C14_GAIN_CFG_SPARE BIT(7)
+#define ADS112C14_GAIN_CFG_SYS_MON GENMASK(6, 4)
+#define ADS112C14_GAIN_CFG_GAIN GENMASK(3, 0)
+
+#define ADS112C14_REG_REFERENCE_CFG 0x09
+#define ADS112C14_REFERENCE_CFG_REF_UV_EN BIT(7)
+#define ADS112C14_REFERENCE_CFG_REFP_BUF_EN BIT(5)
+#define ADS112C14_REFERENCE_CFG_REFN_BUF_EN BIT(4)
+#define ADS112C14_REFERENCE_CFG_REF_VAL BIT(2)
+#define ADS112C14_REFERENCE_CFG_REF_VAL_1_25V 0
+#define ADS112C14_REFERENCE_CFG_REF_VAL_2_5V 1
+#define ADS112C14_REFERENCE_CFG_REF_SEL GENMASK(1, 0)
+
+#define ADS112C14_REG_DIGITAL_CFG 0x0A
+#define ADS112C14_DIGITAL_CFG_REG_MAP_CRC_EN BIT(6)
+#define ADS112C14_DIGITAL_CFG_I2C_CRC_EN BIT(5)
+#define ADS112C14_DIGITAL_CFG_STATUS_EN BIT(4)
+#define ADS112C14_DIGITAL_CFG_FAULT_PIN_BEHAVIOR BIT(3)
+#define ADS112C14_DIGITAL_CFG_CODING BIT(1)
+
+#define ADS112C14_REG_GPIO_CFG 0x0B
+#define ADS112C14_GPIO_CFG_GPIO3_CFG GENMASK(7, 6)
+#define ADS112C14_GPIO_CFG_GPIO2_CFG GENMASK(5, 4)
+#define ADS112C14_GPIO_CFG_GPIO1_CFG GENMASK(3, 2)
+#define ADS112C14_GPIO_CFG_GPIO0_CFG GENMASK(1, 0)
+
+#define ADS112C14_REG_GPIO_DATA_OUTPUT 0x0C
+#define ADS112C14_GPIO_DATA_OUTPUT_GPIO3_SRC BIT(7)
+#define ADS112C14_GPIO_DATA_OUTPUT_GPIO2_SRC BIT(6)
+#define ADS112C14_GPIO_DATA_OUTPUT_GPIO3_DAT_OUT BIT(3)
+#define ADS112C14_GPIO_DATA_OUTPUT_GPIO2_DAT_OUT BIT(2)
+#define ADS112C14_GPIO_DATA_OUTPUT_GPIO1_DAT_OUT BIT(1)
+#define ADS112C14_GPIO_DATA_OUTPUT_GPIO0_DAT_OUT BIT(0)
+
+#define ADS112C14_REG_IDAC_MAG_CFG 0x0D
+#define ADS112C14_IDAC_MAG_CFG_I2MAG GENMASK(7, 4)
+#define ADS112C14_IDAC_MAG_CFG_I1MAG GENMASK(3, 0)
+
+#define ADS112C14_REG_IDAC_MUX_CFG 0x0E
+#define ADS112C14_IDAC_MUX_CFG_IUNIT BIT(7)
+#define ADS112C14_IDAC_MUX_CFG_I2MUX GENMASK(6, 4)
+#define ADS112C14_IDAC_MUX_CFG_I1MUX GENMASK(2, 0)
+
+#define ADS112C14_REG_REG_MAP_CRC 0x0F
+
+static bool ads112c14_writeable_reg(struct device *dev, unsigned int reg)
+{
+ switch (reg) {
+ case ADS112C14_REG_DEVICE_ID:
+ case ADS112C14_REG_REVISION_ID:
+ case ADS112C14_REG_STATUS_LSB:
+ return false;
+ default:
+ return true;
+ }
+}
+
+static bool ads112c14_volatile_reg(struct device *dev, unsigned int reg)
+{
+ switch (reg) {
+ case ADS112C14_REG_STATUS_MSB:
+ case ADS112C14_REG_STATUS_LSB:
+ case ADS112C14_REG_CONVERSION_CTRL:
+ return true;
+ default:
+ return false;
+ }
+}
+
+static const struct reg_default ads112c14_reg_defaults[] = {
+ { ADS112C14_REG_DEVICE_CFG, 0x00 },
+ { ADS112C14_REG_DATA_RATE_CFG, 0x00 },
+ { ADS112C14_REG_MUX_CFG, 0x00 },
+ { ADS112C14_REG_GAIN_CFG, 0x01 },
+ { ADS112C14_REG_REFERENCE_CFG, 0x00 },
+ { ADS112C14_REG_DIGITAL_CFG, 0x00 },
+ { ADS112C14_REG_GPIO_CFG, 0x00 },
+ { ADS112C14_REG_GPIO_DATA_OUTPUT, 0x00 },
+ { ADS112C14_REG_IDAC_MAG_CFG, 0x00 },
+ { ADS112C14_REG_IDAC_MUX_CFG, 0x10 },
+};
+
+static const struct regmap_config ads112c14_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+ .read_flag_mask = ADS112C14_CMD_RREG,
+ .write_flag_mask = ADS112C14_CMD_WREG,
+ .max_register = ADS112C14_REG_REG_MAP_CRC,
+ .writeable_reg = ads112c14_writeable_reg,
+ .volatile_reg = ads112c14_volatile_reg,
+ .reg_defaults = ads112c14_reg_defaults,
+ .num_reg_defaults = ARRAY_SIZE(ads112c14_reg_defaults),
+ .cache_type = REGCACHE_MAPLE,
+};
+
+struct ads112c14_chip_info {
+ const char *name;
+ u32 resolution_bits;
+};
+
+struct ads112c14_data {
+ const struct ads112c14_chip_info *chip_info;
+ struct i2c_client *client;
+ struct regmap *regmap;
+};
+
+/* Maps REF_VAL to microvolts. */
+static const u32 ads112c14_internal_ref_uV[] = {
+ [ADS112C14_REFERENCE_CFG_REF_VAL_1_25V] = 1.25 * MICRO,
+ [ADS112C14_REFERENCE_CFG_REF_VAL_2_5V] = 2.5 * MICRO,
+};
+
+/* Fixed channels for system monitor measurements. */
+#define ADS112C14_SYS_MON_CHANNEL_TEMP 100
+#define ADS112C14_SYS_MON_CHANNEL_EXT_REF 101
+#define ADS112C14_SYS_MON_CHANNEL_AVDD 102
+#define ADS112C14_SYS_MON_CHANNEL_DVDD 103
+#define ADS112C14_SYS_MON_CHANNEL_SHORT 104
+
+static const struct iio_chan_spec ads112c14_sys_mon_channels[] = {
+ {
+ .type = IIO_TEMP,
+ .indexed = 1,
+ .channel = ADS112C14_SYS_MON_CHANNEL_TEMP,
+ .address = 2,
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW)
+ | BIT(IIO_CHAN_INFO_SCALE)
+ | BIT(IIO_CHAN_INFO_OFFSET),
+ },
+ {
+ .type = IIO_VOLTAGE,
+ .indexed = 1,
+ .channel = ADS112C14_SYS_MON_CHANNEL_EXT_REF,
+ .address = 3,
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW)
+ | BIT(IIO_CHAN_INFO_SCALE),
+ },
+ {
+ .type = IIO_VOLTAGE,
+ .indexed = 1,
+ .channel = ADS112C14_SYS_MON_CHANNEL_AVDD,
+ .address = 4,
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW)
+ | BIT(IIO_CHAN_INFO_SCALE),
+ },
+ {
+ .type = IIO_VOLTAGE,
+ .indexed = 1,
+ .channel = ADS112C14_SYS_MON_CHANNEL_DVDD,
+ .address = 5,
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW)
+ | BIT(IIO_CHAN_INFO_SCALE),
+ },
+ {
+ .type = IIO_VOLTAGE,
+ .indexed = 1,
+ .channel = ADS112C14_SYS_MON_CHANNEL_SHORT,
+ .channel2 = ADS112C14_SYS_MON_CHANNEL_SHORT,
+ .differential = 1,
+ .address = 1,
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW)
+ | BIT(IIO_CHAN_INFO_SCALE),
+ },
+};
+
+static int ads112c14_prepare_sys_mon_channel(struct ads112c14_data *data,
+ const struct iio_chan_spec *chan)
+{
+ int ret;
+
+ /* TODO: GAIN is useful for shorted PGA inputs. */
+ /* All SYS_MON channels use GAIN of 1 to keep it simple. */
+ ret = regmap_update_bits(data->regmap, ADS112C14_REG_GAIN_CFG,
+ ADS112C14_GAIN_CFG_SYS_MON |
+ ADS112C14_GAIN_CFG_GAIN,
+ FIELD_PREP(ADS112C14_GAIN_CFG_SYS_MON, chan->address) |
+ FIELD_PREP(ADS112C14_GAIN_CFG_GAIN, 1));
+ if (ret)
+ return ret;
+
+ /* All SYS_MON channels use signed data to keep it simple. */
+ ret = regmap_clear_bits(data->regmap, ADS112C14_REG_DIGITAL_CFG,
+ ADS112C14_DIGITAL_CFG_CODING);
+ if (ret)
+ return ret;
+
+ /* TODO: REF_SEL does matter for shorted PGA inputs. */
+ /*
+ * REVISIT: if we implement regulator support for the REFOUT pin, we
+ * might need to make this voltage match what is required by that. In
+ * that case, we could also adjust GAIN so that we still get the same
+ * range.
+ */
+ /*
+ * Setting REF_SEL for SYS_MON doesn't matter (always uses internal
+ * reference), but we do need to set REF_VAL.
+ */
+ ret = regmap_update_bits(data->regmap, ADS112C14_REG_REFERENCE_CFG,
+ ADS112C14_REFERENCE_CFG_REF_VAL,
+ FIELD_PREP(ADS112C14_REFERENCE_CFG_REF_VAL,
+ ADS112C14_REFERENCE_CFG_REF_VAL_2_5V));
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
+static int ads112c14_single_conversion(struct ads112c14_data *data,
+ const struct iio_chan_spec *chan,
+ u8 *buf)
+{
+ u32 reg_val;
+ int ret;
+
+ if (chan->channel >= 100) {
+ ret = ads112c14_prepare_sys_mon_channel(data, chan);
+ if (ret)
+ return ret;
+ } else {
+ /* Not implemented yet. */
+ return -EINVAL;
+ }
+
+ ret = regmap_write(data->regmap, ADS112C14_REG_CONVERSION_CTRL,
+ ADS112C14_CONVERSION_CTRL_START);
+ if (ret)
+ return ret;
+
+ ret = regmap_read_poll_timeout(data->regmap,
+ ADS112C14_REG_STATUS_MSB, reg_val,
+ FIELD_GET(ADS112C14_STATUS_MSB_DRDY, reg_val),
+ 1 * USEC_PER_MSEC, 10 * USEC_PER_MSEC);
+ if (ret)
+ return ret;
+
+ return i2c_smbus_read_i2c_block_data(data->client, ADS112C14_CMD_RDATA,
+ BITS_TO_BYTES(data->chip_info->resolution_bits),
+ buf);
+}
+
+static int ads112c14_read_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan, int *val,
+ int *val2, long mask)
+{
+ struct ads112c14_data *data = iio_priv(indio_dev);
+ u32 vref_uV, fsr_bits;
+
+ /* Selecting V_REF source is not implemented yet. */
+ vref_uV = ads112c14_internal_ref_uV[ADS112C14_REFERENCE_CFG_REF_VAL_2_5V];
+
+ /* Currently, everything is using signed data. */
+ fsr_bits = data->chip_info->resolution_bits - 1;
+
+ switch (mask) {
+ case IIO_CHAN_INFO_RAW: {
+ u8 buf[3];
+ int ret;
+
+ if (!iio_device_claim_direct(indio_dev))
+ return -EBUSY;
+
+ ret = ads112c14_single_conversion(data, chan, buf);
+ iio_device_release_direct(indio_dev);
+ if (ret < 0)
+ return ret;
+
+ switch (data->chip_info->resolution_bits) {
+ case 16:
+ *val = get_unaligned_be16(buf);
+ break;
+ case 24:
+ *val = get_unaligned_be24(buf);
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ *val = sign_extend32(*val, fsr_bits);
+
+ return IIO_VAL_INT;
+ }
+ case IIO_CHAN_INFO_SCALE:
+ if (chan->type == IIO_TEMP) {
+ /* TS_TC (typical) = 405 uV/°C */
+ *val = MILLI * vref_uV / 405;
+ *val2 = fsr_bits;
+ return IIO_VAL_FRACTIONAL_LOG2;
+ }
+
+ *val = vref_uV / (MICRO / MILLI);
+ /*
+ * Last 3 SYS_MON channels (ext ref, AVDD, DVDD) need to be
+ * multiplied by 8 to account for internal attenuation of / 8.
+ */
+ *val2 = fsr_bits - (chan->address >= 3 ? 3 : 0);
+ return IIO_VAL_FRACTIONAL_LOG2;
+ case IIO_CHAN_INFO_OFFSET:
+ /* Only the temperature channel has an offset. */
+ if (chan->type != IIO_TEMP)
+ return -EINVAL;
+ /* Die temperature [°C] = 25°C + (Measured voltage – TS_Offset) / TS_TC */
+ /* TS_TC (typical) = 405 uV/°C */
+ /* TS_Offset (typical) = 119.5 mV */
+ *val = div_s64((s64)(25 * 405 - 119500) * BIT(fsr_bits), vref_uV);
+ return IIO_VAL_INT;
+ default:
+ return -EINVAL;
+ }
+}
+
+static int ads112c14_read_label(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan, char *label)
+{
+ const char *label_source;
+
+ /* System monitor channels. */
+ switch (chan->channel) {
+ case ADS112C14_SYS_MON_CHANNEL_TEMP:
+ label_source = "Internal temperature sensor";
+ break;
+ case ADS112C14_SYS_MON_CHANNEL_EXT_REF:
+ label_source = "External reference";
+ break;
+ case ADS112C14_SYS_MON_CHANNEL_AVDD:
+ label_source = "AVDD";
+ break;
+ case ADS112C14_SYS_MON_CHANNEL_DVDD:
+ label_source = "DVDD";
+ break;
+ case ADS112C14_SYS_MON_CHANNEL_SHORT:
+ label_source = "Internal short";
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return sysfs_emit(label, "%s\n", label_source);
+}
+
+static const struct iio_info ads112c14_info = {
+ .read_raw = ads112c14_read_raw,
+ .read_label = ads112c14_read_label,
+};
+
+static int ads112c14_probe(struct i2c_client *client)
+{
+ struct device *dev = &client->dev;
+ const struct ads112c14_chip_info *info;
+ struct iio_dev *indio_dev;
+ struct ads112c14_data *data;
+ u32 reg_val;
+ int ret;
+
+ info = i2c_get_match_data(client);
+
+ indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
+ if (!indio_dev)
+ return -ENOMEM;
+
+ data = iio_priv(indio_dev);
+ data->chip_info = info;
+ data->client = client;
+
+ ret = devm_regulator_get_enable(&client->dev, "dvdd");
+ if (ret)
+ return dev_err_probe(dev, ret, "failed to get dvdd regulator\n");
+
+ ret = devm_regulator_get_enable(&client->dev, "avdd");
+ if (ret)
+ return dev_err_probe(dev, ret, "failed to get avdd regulator\n");
+
+ data->regmap = devm_regmap_init_i2c(client, &ads112c14_regmap_config);
+ if (IS_ERR(data->regmap))
+ return dev_err_probe(dev, PTR_ERR(data->regmap),
+ "failed to init regmap\n");
+
+ /* Write magic reset value (0x16) to ensure known state.*/
+ ret = regmap_write(data->regmap, ADS112C14_REG_CONVERSION_CTRL,
+ FIELD_PREP(ADS112C14_CONVERSION_CTRL_RESET, 0x16));
+ /*
+ * The reset may cause an -EREMOTEIO error because of failing to get the
+ * I2C ACK at the end of the message. The device still gets reset.
+ */
+ if (ret != -EREMOTEIO)
+ return ret;
+
+ fsleep(ADS112C14_DELAY_RESET_us);
+
+ ret = regmap_read(data->regmap, ADS112C14_REG_STATUS_MSB, ®_val);
+ if (ret)
+ return ret;
+
+ if (FIELD_GET(ADS112C14_STATUS_MSB_RESETN, reg_val))
+ return dev_err_probe(dev, -EIO, "reset failed\n");
+
+ /*
+ * Clear reset bit to prepare for next probe. And clear AVDD fault since
+ * that happens on every reset.
+ */
+ ret = regmap_write(data->regmap, ADS112C14_REG_STATUS_MSB,
+ ADS112C14_STATUS_MSB_RESETN |
+ ADS112C14_STATUS_MSB_AVDD_UVN);
+ if (ret)
+ return ret;
+
+ /* Place in single-shot conversion mode to make ready for raw read. */
+ ret = regmap_set_bits(data->regmap, ADS112C14_REG_DEVICE_CFG,
+ ADS112C14_DEVICE_CFG_CONV_MODE);
+ if (ret)
+ return ret;
+
+ indio_dev->name = info->name;
+ indio_dev->modes = INDIO_DIRECT_MODE;
+ indio_dev->channels = ads112c14_sys_mon_channels;
+ indio_dev->num_channels = ARRAY_SIZE(ads112c14_sys_mon_channels);
+ indio_dev->info = &ads112c14_info;
+
+ return devm_iio_device_register(&client->dev, indio_dev);
+}
+
+static const struct ads112c14_chip_info ads112c14_chip_info = {
+ .name = "ads112c14",
+ .resolution_bits = 16,
+};
+
+static const struct ads112c14_chip_info ads122c14_chip_info = {
+ .name = "ads122c14",
+ .resolution_bits = 24,
+};
+
+static const struct of_device_id ads112c14_of_match[] = {
+ { .compatible = "ti,ads112c14", .data = &ads112c14_chip_info },
+ { .compatible = "ti,ads122c14", .data = &ads122c14_chip_info },
+ { }
+};
+MODULE_DEVICE_TABLE(of, ads112c14_of_match);
+
+static const struct i2c_device_id ads112c14_id[] = {
+ { .name = "ads112c14", .driver_data = (kernel_ulong_t)&ads112c14_chip_info },
+ { .name = "ads122c14", .driver_data = (kernel_ulong_t)&ads122c14_chip_info },
+ { }
+};
+MODULE_DEVICE_TABLE(i2c, ads112c14_id);
+
+static struct i2c_driver ads112c14_driver = {
+ .driver = {
+ .name = "ads112c14",
+ .of_match_table = ads112c14_of_match,
+ },
+ .probe = ads112c14_probe,
+ .id_table = ads112c14_id,
+};
+module_i2c_driver(ads112c14_driver);
+
+MODULE_AUTHOR("David Lechner (TI) <dlechner@baylibre.com>");
+MODULE_DESCRIPTION("TI ADS112C14 I2C ADC driver");
+MODULE_LICENSE("GPL");
--
2.43.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH 2/4] iio: adc: add ti-ads112c14 driver
2026-06-15 22:00 ` [PATCH 2/4] iio: adc: add ti-ads112c14 driver David Lechner (TI)
@ 2026-06-16 7:32 ` Andy Shevchenko
2026-06-16 15:38 ` David Lechner
0 siblings, 1 reply; 20+ messages in thread
From: Andy Shevchenko @ 2026-06-16 7:32 UTC (permalink / raw)
To: David Lechner (TI)
Cc: Jonathan Cameron, Nuno Sá, Andy Shevchenko, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Kurt Borja, Nguyen Minh Tien,
linux-iio, devicetree, linux-kernel
On Mon, Jun 15, 2026 at 05:00:00PM -0500, David Lechner (TI) wrote:
> Add a new driver for the TI ADS112C14/ADS122C14 ADC chips.
>
> This first step is adding a very basic driver that only supports power
> on/reset and reading the system monitor channels.
>
> ADS112C14_SYS_MON_CHANNEL_SHORT is the last channel rather than being in
> logical order by address to keep the voltage channels together and in
> case we find we need to add variants of this channel with different
> voltage reference later.
...
> +#include <linux/bitfield.h>
+ bitops.h // BIT(), GENMASK(), sign_extend32()
> +#include <linux/delay.h>
> +#include <linux/dev_printk.h>
> +#include <linux/i2c.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/types.h>
I believe we discussed that (with Jonathan) already and seems the trend is to
imply that iio/types.h is always included whenever iio/iio.h is included.
> +#include <linux/math64.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/property.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/sysfs.h>
> +#include <linux/time64.h>
> +#include <linux/types.h>
> +#include <linux/unaligned.h>
> +#include <linux/units.h>
...
> +/* Datasheet t_d(RST) - time to wait after reset before next I2C use. */
> +#define ADS112C14_DELAY_RESET_us 500
For µs (and second related units) we traditionally use _US suffix
(yeah, I know...). Of course, we can start a SI schema in new code
but it will look quite strange and probably more confusing.
...
> +struct ads112c14_data {
> + const struct ads112c14_chip_info *chip_info;
> + struct i2c_client *client;
No need to keep it, it's one time use and can be derived from regmap.
regmap --> dev --> i2c_client.
> + struct regmap *regmap;
> +};
...
> +static int ads112c14_read_label(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan, char *label)
> +{
> + const char *label_source;
I don't see the need of having this. Can't be returned directly?
> + /* System monitor channels. */
> + switch (chan->channel) {
> + case ADS112C14_SYS_MON_CHANNEL_TEMP:
> + label_source = "Internal temperature sensor";
> + break;
> + case ADS112C14_SYS_MON_CHANNEL_EXT_REF:
> + label_source = "External reference";
> + break;
> + case ADS112C14_SYS_MON_CHANNEL_AVDD:
> + label_source = "AVDD";
> + break;
> + case ADS112C14_SYS_MON_CHANNEL_DVDD:
> + label_source = "DVDD";
> + break;
> + case ADS112C14_SYS_MON_CHANNEL_SHORT:
> + label_source = "Internal short";
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + return sysfs_emit(label, "%s\n", label_source);
> +}
...
> +static int ads112c14_probe(struct i2c_client *client)
> +{
> + struct device *dev = &client->dev;
> + const struct ads112c14_chip_info *info;
> + struct iio_dev *indio_dev;
> + struct ads112c14_data *data;
> + u32 reg_val;
> + int ret;
> +
> + info = i2c_get_match_data(client);
NULL check (yeah) due to driver_override issue.
> + indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
Here, and below, use 'dev'.
> + if (!indio_dev)
> + return -ENOMEM;
> +
> + data = iio_priv(indio_dev);
> + data->chip_info = info;
> + data->client = client;
> +
> + ret = devm_regulator_get_enable(&client->dev, "dvdd");
> + if (ret)
> + return dev_err_probe(dev, ret, "failed to get dvdd regulator\n");
> +
> + ret = devm_regulator_get_enable(&client->dev, "avdd");
> + if (ret)
> + return dev_err_probe(dev, ret, "failed to get avdd regulator\n");
> +
> + data->regmap = devm_regmap_init_i2c(client, &ads112c14_regmap_config);
> + if (IS_ERR(data->regmap))
> + return dev_err_probe(dev, PTR_ERR(data->regmap),
> + "failed to init regmap\n");
> +
> + /* Write magic reset value (0x16) to ensure known state.*/
> + ret = regmap_write(data->regmap, ADS112C14_REG_CONVERSION_CTRL,
> + FIELD_PREP(ADS112C14_CONVERSION_CTRL_RESET, 0x16));
> + /*
> + * The reset may cause an -EREMOTEIO error because of failing to get the
> + * I2C ACK at the end of the message. The device still gets reset.
> + */
> + if (ret != -EREMOTEIO)
> + return ret;
I would do it separately as
if (ret == -EREMOTEIO)
/* ...big comment here... */
return 0;
if (ret) // which is regular pattern and doesn't need any comment.
return ret;
> + fsleep(ADS112C14_DELAY_RESET_us);
> +
> + ret = regmap_read(data->regmap, ADS112C14_REG_STATUS_MSB, ®_val);
> + if (ret)
> + return ret;
> +
> + if (FIELD_GET(ADS112C14_STATUS_MSB_RESETN, reg_val))
> + return dev_err_probe(dev, -EIO, "reset failed\n");
> +
> + /*
> + * Clear reset bit to prepare for next probe. And clear AVDD fault since
> + * that happens on every reset.
> + */
> + ret = regmap_write(data->regmap, ADS112C14_REG_STATUS_MSB,
> + ADS112C14_STATUS_MSB_RESETN |
> + ADS112C14_STATUS_MSB_AVDD_UVN);
> + if (ret)
> + return ret;
> +
> + /* Place in single-shot conversion mode to make ready for raw read. */
> + ret = regmap_set_bits(data->regmap, ADS112C14_REG_DEVICE_CFG,
> + ADS112C14_DEVICE_CFG_CONV_MODE);
> + if (ret)
> + return ret;
> +
> + indio_dev->name = info->name;
> + indio_dev->modes = INDIO_DIRECT_MODE;
> + indio_dev->channels = ads112c14_sys_mon_channels;
> + indio_dev->num_channels = ARRAY_SIZE(ads112c14_sys_mon_channels);
> + indio_dev->info = &ads112c14_info;
> +
> + return devm_iio_device_register(&client->dev, indio_dev);
> +}
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 2/4] iio: adc: add ti-ads112c14 driver
2026-06-16 7:32 ` Andy Shevchenko
@ 2026-06-16 15:38 ` David Lechner
0 siblings, 0 replies; 20+ messages in thread
From: David Lechner @ 2026-06-16 15:38 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Jonathan Cameron, Nuno Sá, Andy Shevchenko, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Kurt Borja, Nguyen Minh Tien,
linux-iio, devicetree, linux-kernel
On 6/16/26 2:32 AM, Andy Shevchenko wrote:
> On Mon, Jun 15, 2026 at 05:00:00PM -0500, David Lechner (TI) wrote:
>> Add a new driver for the TI ADS112C14/ADS122C14 ADC chips.
>>
>> This first step is adding a very basic driver that only supports power
>> on/reset and reading the system monitor channels.
>>
>> ADS112C14_SYS_MON_CHANNEL_SHORT is the last channel rather than being in
>> logical order by address to keep the voltage channels together and in
>> case we find we need to add variants of this channel with different
>> voltage reference later.
>
> ...
>> +static int ads112c14_read_label(struct iio_dev *indio_dev,
>> + struct iio_chan_spec const *chan, char *label)
>> +{
>> + const char *label_source;
>
> I don't see the need of having this. Can't be returned directly?
Then this would have to be split into two functions. One to return
it directly and one to do the sysfs_emit(). I don't think it is
worth it.
>
>> + /* System monitor channels. */
>> + switch (chan->channel) {
>> + case ADS112C14_SYS_MON_CHANNEL_TEMP:
>> + label_source = "Internal temperature sensor";
>> + break;
>> + case ADS112C14_SYS_MON_CHANNEL_EXT_REF:
>> + label_source = "External reference";
>> + break;
>> + case ADS112C14_SYS_MON_CHANNEL_AVDD:
>> + label_source = "AVDD";
>> + break;
>> + case ADS112C14_SYS_MON_CHANNEL_DVDD:
>> + label_source = "DVDD";
>> + break;
>> + case ADS112C14_SYS_MON_CHANNEL_SHORT:
>> + label_source = "Internal short";
>> + break;
>> + default:
>> + return -EINVAL;
>> + }
>> +
>> + return sysfs_emit(label, "%s\n", label_source);
>> +}
>
>> + /*
>> + * The reset may cause an -EREMOTEIO error because of failing to get the
>> + * I2C ACK at the end of the message. The device still gets reset.
>> + */
>> + if (ret != -EREMOTEIO)
>> + return ret;
>
> I would do it separately as
>
> if (ret == -EREMOTEIO)
> /* ...big comment here... */
> return 0;
We should not return early here. We just continue with the rest
of the function as normal. So I think the way I had it was
simplest. Otherwise we would need a goto or something like that.
> if (ret) // which is regular pattern and doesn't need any comment.
> return ret;
>
>> + fsleep(ADS112C14_DELAY_RESET_us);
>> +
>> + ret = regmap_read(data->regmap, ADS112C14_REG_STATUS_MSB, ®_val);
>> + if (ret)
>> + return ret;
>> +
>> + if (FIELD_GET(ADS112C14_STATUS_MSB_RESETN, reg_val))
>> + return dev_err_probe(dev, -EIO, "reset failed\n");
>> +
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 3/4] iio: adc: ti-ads112c14: implement gain on internal short SYS_MON channel
2026-06-15 21:59 [PATCH 0/4] iio: adc: new ti-ads112c14 driver David Lechner (TI)
2026-06-15 21:59 ` [PATCH 1/4] dt-bindings: iio: adc: add ti,ads122c14 David Lechner (TI)
2026-06-15 22:00 ` [PATCH 2/4] iio: adc: add ti-ads112c14 driver David Lechner (TI)
@ 2026-06-15 22:00 ` David Lechner (TI)
2026-06-16 7:58 ` Andy Shevchenko
2026-06-15 22:00 ` [PATCH 4/4] iio: adc: ti-ads112c14: add measurement channel support David Lechner (TI)
2026-06-16 0:18 ` [PATCH 0/4] iio: adc: new ti-ads112c14 driver Kurt Borja
4 siblings, 1 reply; 20+ messages in thread
From: David Lechner (TI) @ 2026-06-15 22:00 UTC (permalink / raw)
To: Jonathan Cameron, Nuno Sá, Andy Shevchenko, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: Kurt Borja, Nguyen Minh Tien, linux-iio, devicetree, linux-kernel,
David Lechner
Implement support for the programmable gain amplifier on the internal
short SYS_MON channel. This channel is used for calibration, so it is
useful to be able to set the PGA to the same gain as the external
channels. The gain setting is implemented via the `_scale` attribute.
In the future, we may want to support different reference voltages for
this channel, so the scale_available table is populated during probe
rather than being a static table.
Signed-off-by: David Lechner (TI) <dlechner@baylibre.com>
---
drivers/iio/adc/ti-ads112c14.c | 131 ++++++++++++++++++++++++++++++++++++++++-
1 file changed, 128 insertions(+), 3 deletions(-)
diff --git a/drivers/iio/adc/ti-ads112c14.c b/drivers/iio/adc/ti-ads112c14.c
index 97097ae2a487..202a449b4234 100644
--- a/drivers/iio/adc/ti-ads112c14.c
+++ b/drivers/iio/adc/ti-ads112c14.c
@@ -122,6 +122,26 @@
#define ADS112C14_REG_REG_MAP_CRC 0x0F
+/* Available gains as tenths (x10) */
+static const u32 ads112c14_pga_gains_x10[] = {
+ 5, /* 0.5 */
+ 10, /* 1 */
+ 20, /* 2 */
+ 40, /* 4 */
+ 50, /* 5 */
+ 80, /* 8 */
+ 100, /* 10 */
+ 160, /* 16 */
+ 200, /* 20 */
+ 320, /* 32 */
+ 500, /* 50 */
+ 640, /* 64 */
+ 1000, /* 100 */
+ 1280, /* 128 */
+ 2000, /* 200 */
+ 2560, /* 256 */
+};
+
static bool ads112c14_writeable_reg(struct device *dev, unsigned int reg)
{
switch (reg) {
@@ -181,6 +201,8 @@ struct ads112c14_data {
const struct ads112c14_chip_info *chip_info;
struct i2c_client *client;
struct regmap *regmap;
+ u8 sys_mon_chan_short_gain_val;
+ int sys_mon_chan_short_scale_available[ARRAY_SIZE(ads112c14_pga_gains_x10)][2];
};
/* Maps REF_VAL to microvolts. */
@@ -239,21 +261,28 @@ static const struct iio_chan_spec ads112c14_sys_mon_channels[] = {
.address = 1,
.info_mask_separate = BIT(IIO_CHAN_INFO_RAW)
| BIT(IIO_CHAN_INFO_SCALE),
+ .info_mask_separate_available = BIT(IIO_CHAN_INFO_SCALE),
},
};
static int ads112c14_prepare_sys_mon_channel(struct ads112c14_data *data,
const struct iio_chan_spec *chan)
{
+ u32 gain_val;
int ret;
- /* TODO: GAIN is useful for shorted PGA inputs. */
- /* All SYS_MON channels use GAIN of 1 to keep it simple. */
+ /*
+ * All SYS_MON channels use GAIN of 1 to keep it simple. Other than
+ * the internal short channel, where it is useful in practice.
+ */
+ gain_val = chan->channel == ADS112C14_SYS_MON_CHANNEL_SHORT ?
+ data->sys_mon_chan_short_gain_val : 1;
+
ret = regmap_update_bits(data->regmap, ADS112C14_REG_GAIN_CFG,
ADS112C14_GAIN_CFG_SYS_MON |
ADS112C14_GAIN_CFG_GAIN,
FIELD_PREP(ADS112C14_GAIN_CFG_SYS_MON, chan->address) |
- FIELD_PREP(ADS112C14_GAIN_CFG_GAIN, 1));
+ FIELD_PREP(ADS112C14_GAIN_CFG_GAIN, gain_val));
if (ret)
return ret;
@@ -366,6 +395,17 @@ static int ads112c14_read_raw(struct iio_dev *indio_dev,
return IIO_VAL_FRACTIONAL_LOG2;
}
+ if (chan->channel == ADS112C14_SYS_MON_CHANNEL_SHORT) {
+ int *scale_avail = data->sys_mon_chan_short_scale_available[
+ data->sys_mon_chan_short_gain_val
+ ];
+
+ *val = scale_avail[0];
+ *val2 = scale_avail[1];
+
+ return IIO_VAL_INT_PLUS_NANO;
+ }
+
*val = vref_uV / (MICRO / MILLI);
/*
* Last 3 SYS_MON channels (ext ref, AVDD, DVDD) need to be
@@ -387,6 +427,61 @@ static int ads112c14_read_raw(struct iio_dev *indio_dev,
}
}
+static int ads112c14_read_avail(struct iio_dev *indio_dev,
+ const struct iio_chan_spec *chan, const int **vals,
+ int *type, int *length, long mask)
+{
+ struct ads112c14_data *data = iio_priv(indio_dev);
+
+ switch (chan->channel) {
+ case ADS112C14_SYS_MON_CHANNEL_SHORT:
+ *vals = (const int *)data->sys_mon_chan_short_scale_available;
+ *length = 2 * ARRAY_SIZE(data->sys_mon_chan_short_scale_available);
+ *type = IIO_VAL_INT_PLUS_NANO;
+ return IIO_AVAIL_LIST;
+ default:
+ return -EINVAL;
+ }
+}
+
+static int ads112c14_write_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan, int val,
+ int val2, long mask)
+{
+ struct ads112c14_data *data = iio_priv(indio_dev);
+
+ switch (chan->channel) {
+ case ADS112C14_SYS_MON_CHANNEL_SHORT: {
+ IIO_DEV_ACQUIRE_DIRECT_MODE(indio_dev, claim);
+ if (IIO_DEV_ACQUIRE_FAILED(claim))
+ return -EBUSY;
+
+ for (u32 i = 0; i < ARRAY_SIZE(data->sys_mon_chan_short_scale_available); i++) {
+ if (val == data->sys_mon_chan_short_scale_available[i][0] &&
+ val2 == data->sys_mon_chan_short_scale_available[i][1]) {
+ data->sys_mon_chan_short_gain_val = i;
+ return 0;
+ }
+ }
+ return -EINVAL;
+ }
+ default:
+ return -EINVAL;
+ }
+}
+
+static int ads112c14_write_raw_get_fmt(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ long mask)
+{
+ switch (mask) {
+ case IIO_CHAN_INFO_SCALE:
+ return IIO_VAL_INT_PLUS_NANO;
+ default:
+ return IIO_VAL_INT_PLUS_MICRO;
+ }
+}
+
static int ads112c14_read_label(struct iio_dev *indio_dev,
struct iio_chan_spec const *chan, char *label)
{
@@ -418,9 +513,34 @@ static int ads112c14_read_label(struct iio_dev *indio_dev,
static const struct iio_info ads112c14_info = {
.read_raw = ads112c14_read_raw,
+ .read_avail = ads112c14_read_avail,
+ .write_raw = ads112c14_write_raw,
+ .write_raw_get_fmt = ads112c14_write_raw_get_fmt,
.read_label = ads112c14_read_label,
};
+static void ads112c14_populate_tables(struct ads112c14_data *data)
+{
+ u32 vref_uV, fsr_bits;
+ int i;
+
+ /* For now, assuming we are using 2.5V reference. */
+ vref_uV = ads112c14_internal_ref_uV[ADS112C14_REFERENCE_CFG_REF_VAL_2_5V];
+ fsr_bits = data->chip_info->resolution_bits - 1;
+
+ for (i = 0; i < ARRAY_SIZE(ads112c14_pga_gains_x10); i++) {
+ int *scale_avail = &data->sys_mon_chan_short_scale_available[i][0];
+ u32 gain_x10 = ads112c14_pga_gains_x10[i];
+
+ /* NB: slightly odd arrangement to avoid overflow. */
+ scale_avail[0] = div_u64_rem(div_u64((u64)NANO * 10 /
+ (MICRO / MILLI) * vref_uV /
+ gain_x10,
+ BIT(fsr_bits)),
+ NANO, &scale_avail[1]);
+ }
+}
+
static int ads112c14_probe(struct i2c_client *client)
{
struct device *dev = &client->dev;
@@ -472,6 +592,9 @@ static int ads112c14_probe(struct i2c_client *client)
if (FIELD_GET(ADS112C14_STATUS_MSB_RESETN, reg_val))
return dev_err_probe(dev, -EIO, "reset failed\n");
+ /* Default gain after reset is 1. */
+ data->sys_mon_chan_short_gain_val = 1;
+
/*
* Clear reset bit to prepare for next probe. And clear AVDD fault since
* that happens on every reset.
@@ -488,6 +611,8 @@ static int ads112c14_probe(struct i2c_client *client)
if (ret)
return ret;
+ ads112c14_populate_tables(data);
+
indio_dev->name = info->name;
indio_dev->modes = INDIO_DIRECT_MODE;
indio_dev->channels = ads112c14_sys_mon_channels;
--
2.43.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH 3/4] iio: adc: ti-ads112c14: implement gain on internal short SYS_MON channel
2026-06-15 22:00 ` [PATCH 3/4] iio: adc: ti-ads112c14: implement gain on internal short SYS_MON channel David Lechner (TI)
@ 2026-06-16 7:58 ` Andy Shevchenko
2026-06-16 10:03 ` Nuno Sá
0 siblings, 1 reply; 20+ messages in thread
From: Andy Shevchenko @ 2026-06-16 7:58 UTC (permalink / raw)
To: David Lechner (TI)
Cc: Jonathan Cameron, Nuno Sá, Andy Shevchenko, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Kurt Borja, Nguyen Minh Tien,
linux-iio, devicetree, linux-kernel
On Mon, Jun 15, 2026 at 05:00:01PM -0500, David Lechner (TI) wrote:
> Implement support for the programmable gain amplifier on the internal
> short SYS_MON channel. This channel is used for calibration, so it is
> useful to be able to set the PGA to the same gain as the external
> channels. The gain setting is implemented via the `_scale` attribute.
>
> In the future, we may want to support different reference voltages for
> this channel, so the scale_available table is populated during probe
> rather than being a static table.
...
> + switch (chan->channel) {
> + case ADS112C14_SYS_MON_CHANNEL_SHORT: {
> + IIO_DEV_ACQUIRE_DIRECT_MODE(indio_dev, claim);
> + if (IIO_DEV_ACQUIRE_FAILED(claim))
> + return -EBUSY;
> +
> + for (u32 i = 0; i < ARRAY_SIZE(data->sys_mon_chan_short_scale_available); i++) {
u32 here looks confusing. the entire loop can be made readable with a temporary
for the array item I believe.
> + if (val == data->sys_mon_chan_short_scale_available[i][0] &&
> + val2 == data->sys_mon_chan_short_scale_available[i][1]) {
> + data->sys_mon_chan_short_gain_val = i;
> + return 0;
> + }
> + }
for (size_t i = 0; i < ARRAY_SIZE(data->sys_mon_chan_short_scale_available); i++) {
const int *sa = data->sys_mon_chan_short_scale_available[i];
if (val == sa[0] && val2 == sa[1]) {
data->sys_mon_chan_short_gain_val = i;
return 0;
}
}
> + return -EINVAL;
> + }
> + default:
> + return -EINVAL;
> + }
> +}
...
> +static void ads112c14_populate_tables(struct ads112c14_data *data)
> +{
> + u32 vref_uV, fsr_bits;
> + int i;
size_t? unsigned int?
> + /* For now, assuming we are using 2.5V reference. */
> + vref_uV = ads112c14_internal_ref_uV[ADS112C14_REFERENCE_CFG_REF_VAL_2_5V];
> + fsr_bits = data->chip_info->resolution_bits - 1;
> +
> + for (i = 0; i < ARRAY_SIZE(ads112c14_pga_gains_x10); i++) {
> + int *scale_avail = &data->sys_mon_chan_short_scale_available[i][0];
> + u32 gain_x10 = ads112c14_pga_gains_x10[i];
> +
> + /* NB: slightly odd arrangement to avoid overflow. */
> + scale_avail[0] = div_u64_rem(div_u64((u64)NANO * 10 /
> + (MICRO / MILLI) * vref_uV /
> + gain_x10,
> + BIT(fsr_bits)),
> + NANO, &scale_avail[1]);
Oh, what about temporary variable for the inner division? Also note one trick
to avoid casting (and making it shorter).
u64 foo;
foo = div_u64(10ULL * NANO / (MICRO / MILLI) * vref_uV / gain_x10, BIT(fsr_bits));
/* NB: slightly odd arrangement to avoid overflow. */
scale_avail[0] = div_u64_rem(foo, NANO, &scale_avail[1]);
Now, with much more readability, it's visible that the first division is just a right shift.
u64 foo;
/* ...a comment to explain voodoo calculations... */
foo = (10ULL * NANO / (MICRO / MILLI) * vref_uV / gain_x10) >> fsr_bits;
scale_avail[0] = div_u64_rem(foo, NANO, &scale_avail[1]);
> + }
> +}
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 3/4] iio: adc: ti-ads112c14: implement gain on internal short SYS_MON channel
2026-06-16 7:58 ` Andy Shevchenko
@ 2026-06-16 10:03 ` Nuno Sá
0 siblings, 0 replies; 20+ messages in thread
From: Nuno Sá @ 2026-06-16 10:03 UTC (permalink / raw)
To: Andy Shevchenko
Cc: David Lechner (TI), Jonathan Cameron, Nuno Sá,
Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Kurt Borja, Nguyen Minh Tien, linux-iio, devicetree, linux-kernel
On Tue, Jun 16, 2026 at 10:58:46AM +0300, Andy Shevchenko wrote:
> On Mon, Jun 15, 2026 at 05:00:01PM -0500, David Lechner (TI) wrote:
> > Implement support for the programmable gain amplifier on the internal
> > short SYS_MON channel. This channel is used for calibration, so it is
> > useful to be able to set the PGA to the same gain as the external
> > channels. The gain setting is implemented via the `_scale` attribute.
> >
> > In the future, we may want to support different reference voltages for
> > this channel, so the scale_available table is populated during probe
> > rather than being a static table.
>
...
> > +
> > + for (i = 0; i < ARRAY_SIZE(ads112c14_pga_gains_x10); i++) {
> > + int *scale_avail = &data->sys_mon_chan_short_scale_available[i][0];
> > + u32 gain_x10 = ads112c14_pga_gains_x10[i];
> > +
> > + /* NB: slightly odd arrangement to avoid overflow. */
> > + scale_avail[0] = div_u64_rem(div_u64((u64)NANO * 10 /
> > + (MICRO / MILLI) * vref_uV /
> > + gain_x10,
> > + BIT(fsr_bits)),
> > + NANO, &scale_avail[1]);
>
> Oh, what about temporary variable for the inner division? Also note one trick
> to avoid casting (and making it shorter).
>
> u64 foo;
>
> foo = div_u64(10ULL * NANO / (MICRO / MILLI) * vref_uV / gain_x10, BIT(fsr_bits));
>
> /* NB: slightly odd arrangement to avoid overflow. */
> scale_avail[0] = div_u64_rem(foo, NANO, &scale_avail[1]);
>
> Now, with much more readability, it's visible that the first division is just a right shift.
>
> u64 foo;
>
> /* ...a comment to explain voodoo calculations... */
> foo = (10ULL * NANO / (MICRO / MILLI) * vref_uV / gain_x10) >> fsr_bits;
Did not looked enough time but also looks like maybe mul_u64_u32_shr()
could be used? And given that it handles things nicely (even uses 128bit
when possible), might open the door for more simplifications.
Anyways, agree with Andy that in the current patch it's a bit of a pain
to the eyes :).
- Nuno Sá
>
> scale_avail[0] = div_u64_rem(foo, NANO, &scale_avail[1]);
>
> > + }
> > +}
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 4/4] iio: adc: ti-ads112c14: add measurement channel support
2026-06-15 21:59 [PATCH 0/4] iio: adc: new ti-ads112c14 driver David Lechner (TI)
` (2 preceding siblings ...)
2026-06-15 22:00 ` [PATCH 3/4] iio: adc: ti-ads112c14: implement gain on internal short SYS_MON channel David Lechner (TI)
@ 2026-06-15 22:00 ` David Lechner (TI)
2026-06-16 8:36 ` Andy Shevchenko
2026-06-16 15:30 ` David Lechner
2026-06-16 0:18 ` [PATCH 0/4] iio: adc: new ti-ads112c14 driver Kurt Borja
4 siblings, 2 replies; 20+ messages in thread
From: David Lechner (TI) @ 2026-06-15 22:00 UTC (permalink / raw)
To: Jonathan Cameron, Nuno Sá, Andy Shevchenko, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: Kurt Borja, Nguyen Minh Tien, linux-iio, devicetree, linux-kernel,
David Lechner
Add support for parsing devicetree properties for measurement channels
and doing direct reads on these.
There are quite a lot of conditions that have to be met for each
measurement to be made, so quite a bit of state and algorithms are
required to handle it.
Channels are created dynamically since the number of possibilities is
unreasonably large.
Signed-off-by: David Lechner (TI) <dlechner@baylibre.com>
---
drivers/iio/adc/ti-ads112c14.c | 486 +++++++++++++++++++++++++++++++++++++----
1 file changed, 439 insertions(+), 47 deletions(-)
diff --git a/drivers/iio/adc/ti-ads112c14.c b/drivers/iio/adc/ti-ads112c14.c
index 202a449b4234..d1297445025f 100644
--- a/drivers/iio/adc/ti-ads112c14.c
+++ b/drivers/iio/adc/ti-ads112c14.c
@@ -9,8 +9,10 @@
*/
#include <linux/bitfield.h>
+#include <linux/cleanup.h>
#include <linux/delay.h>
#include <linux/dev_printk.h>
+#include <linux/device/devres.h>
#include <linux/i2c.h>
#include <linux/iio/iio.h>
#include <linux/iio/types.h>
@@ -26,6 +28,8 @@
#include <linux/unaligned.h>
#include <linux/units.h>
+#include <dt-bindings/iio/adc/ti,ads112c14.h>
+
/* Datasheet t_d(RST) - time to wait after reset before next I2C use. */
#define ADS112C14_DELAY_RESET_us 500
@@ -89,6 +93,9 @@
#define ADS112C14_REFERENCE_CFG_REF_VAL_1_25V 0
#define ADS112C14_REFERENCE_CFG_REF_VAL_2_5V 1
#define ADS112C14_REFERENCE_CFG_REF_SEL GENMASK(1, 0)
+#define ADS112C14_REFERENCE_CFG_REF_SEL_INTERNAL 0
+#define ADS112C14_REFERENCE_CFG_REF_SEL_EXTERNAL 1
+#define ADS112C14_REFERENCE_CFG_REF_SEL_AVDD 2
#define ADS112C14_REG_DIGITAL_CFG 0x0A
#define ADS112C14_DIGITAL_CFG_REG_MAP_CRC_EN BIT(6)
@@ -197,10 +204,30 @@ struct ads112c14_chip_info {
u32 resolution_bits;
};
+struct ads112c14_measurement {
+ const char *label;
+ u32 vref_source;
+ u32 idac_current_uA;
+ u8 iadc_count;
+ u8 idac1_mux;
+ u8 idac2_mux;
+ bool current_chop;
+ bool bipolar;
+ u8 gain_val;
+ int scale_available[ARRAY_SIZE(ads112c14_pga_gains_x10)][2];
+};
+
struct ads112c14_data {
const struct ads112c14_chip_info *chip_info;
struct i2c_client *client;
struct regmap *regmap;
+ u32 avdd_uV;
+ u32 ext_ref_uV;
+ bool refp_is_avdd;
+ bool refn_is_gnd;
+ u32 ext_ref_ohms;
+ struct ads112c14_measurement *measurements;
+ u32 num_measurements;
u8 sys_mon_chan_short_gain_val;
int sys_mon_chan_short_scale_available[ARRAY_SIZE(ads112c14_pga_gains_x10)][2];
};
@@ -265,12 +292,106 @@ static const struct iio_chan_spec ads112c14_sys_mon_channels[] = {
},
};
+static int ads112c14_prepare_measurement_channel(struct ads112c14_data *data,
+ const struct iio_chan_spec *chan)
+{
+ struct ads112c14_measurement *measurement = &data->measurements[chan->scan_index];
+ bool iunit = measurement->idac_current_uA > 100;
+ u32 idac1_val = measurement->iadc_count > 0 ?
+ measurement->idac_current_uA / (iunit ? 100 : 10) : 0;
+ u32 idac2_val = measurement->iadc_count > 1 ? idac1_val : 0;
+ u32 refp_buf_en, refn_buf_en;
+ u32 ref_val, ref_sel;
+ int ret;
+
+ ret = regmap_write(data->regmap, ADS112C14_REG_MUX_CFG,
+ FIELD_PREP(ADS112C14_MUX_CFG_AINP, chan->channel) |
+ FIELD_PREP(ADS112C14_MUX_CFG_AINN, chan->channel2));
+ if (ret)
+ return ret;
+
+ ret = regmap_update_bits(data->regmap, ADS112C14_REG_DIGITAL_CFG,
+ ADS112C14_DIGITAL_CFG_CODING,
+ FIELD_PREP(ADS112C14_DIGITAL_CFG_CODING,
+ measurement->bipolar ? 0 : 1));
+ if (ret)
+ return ret;
+
+ ret = regmap_update_bits(data->regmap, ADS112C14_REG_GAIN_CFG,
+ ADS112C14_GAIN_CFG_SYS_MON | ADS112C14_GAIN_CFG_GAIN,
+ FIELD_PREP(ADS112C14_GAIN_CFG_SYS_MON, 0) |
+ FIELD_PREP(ADS112C14_GAIN_CFG_GAIN,
+ measurement->gain_val));
+ if (ret)
+ return ret;
+
+ ret = regmap_write(data->regmap, ADS112C14_REG_IDAC_MAG_CFG,
+ FIELD_PREP(ADS112C14_IDAC_MAG_CFG_I2MAG, idac2_val) |
+ FIELD_PREP(ADS112C14_IDAC_MAG_CFG_I1MAG, idac1_val));
+ if (ret)
+ return ret;
+
+ ret = regmap_write(data->regmap, ADS112C14_REG_IDAC_MUX_CFG,
+ (iunit ? ADS112C14_IDAC_MUX_CFG_IUNIT : 0) |
+ FIELD_PREP(ADS112C14_IDAC_MUX_CFG_I2MUX,
+ measurement->idac2_mux) |
+ FIELD_PREP(ADS112C14_IDAC_MUX_CFG_I1MUX,
+ measurement->idac1_mux));
+ if (ret)
+ return ret;
+
+ ret = regmap_update_bits(data->regmap, ADS112C14_REG_DATA_RATE_CFG,
+ ADS112C14_DATA_RATE_CFG_GC_EN,
+ FIELD_PREP(ADS112C14_DATA_RATE_CFG_GC_EN,
+ measurement->current_chop));
+ if (ret)
+ return ret;
+
+ refp_buf_en = !data->refp_is_avdd &&
+ measurement->vref_source == ADS112C14_VREF_SOURCE_EXTERNAL;
+ refn_buf_en = !data->refn_is_gnd &&
+ measurement->vref_source == ADS112C14_VREF_SOURCE_EXTERNAL;
+
+ ref_val = measurement->vref_source == ADS112C14_VREF_SOURCE_INTERNAL_2_5V;
+
+ switch (measurement->vref_source) {
+ case ADS112C14_VREF_SOURCE_AVDD:
+ ref_sel = ADS112C14_REFERENCE_CFG_REF_SEL_AVDD;
+ break;
+ case ADS112C14_VREF_SOURCE_EXTERNAL:
+ ref_sel = ADS112C14_REFERENCE_CFG_REF_SEL_EXTERNAL;
+ break;
+ default:
+ ref_sel = ADS112C14_REFERENCE_CFG_REF_SEL_INTERNAL;
+ break;
+ }
+
+ return regmap_update_bits(data->regmap, ADS112C14_REG_REFERENCE_CFG,
+ ADS112C14_REFERENCE_CFG_REFP_BUF_EN |
+ ADS112C14_REFERENCE_CFG_REFN_BUF_EN |
+ ADS112C14_REFERENCE_CFG_REF_VAL |
+ ADS112C14_REFERENCE_CFG_REF_SEL,
+ FIELD_PREP(ADS112C14_REFERENCE_CFG_REFP_BUF_EN,
+ refp_buf_en) |
+ FIELD_PREP(ADS112C14_REFERENCE_CFG_REFN_BUF_EN,
+ refn_buf_en) |
+ FIELD_PREP(ADS112C14_REFERENCE_CFG_REF_VAL,
+ ref_val) |
+ FIELD_PREP(ADS112C14_REFERENCE_CFG_REF_SEL,
+ ref_sel));
+}
+
static int ads112c14_prepare_sys_mon_channel(struct ads112c14_data *data,
const struct iio_chan_spec *chan)
{
u32 gain_val;
int ret;
+ /*
+ * NB: IDAC registers are left as-is in case they are generating current
+ * needed for the external reference measurement.
+ */
+
/*
* All SYS_MON channels use GAIN of 1 to keep it simple. Other than
* the internal short channel, where it is useful in practice.
@@ -320,13 +441,14 @@ static int ads112c14_single_conversion(struct ads112c14_data *data,
u32 reg_val;
int ret;
- if (chan->channel >= 100) {
- ret = ads112c14_prepare_sys_mon_channel(data, chan);
+ if (chan->channel < 100) {
+ ret = ads112c14_prepare_measurement_channel(data, chan);
if (ret)
return ret;
} else {
- /* Not implemented yet. */
- return -EINVAL;
+ ret = ads112c14_prepare_sys_mon_channel(data, chan);
+ if (ret)
+ return ret;
}
ret = regmap_write(data->regmap, ADS112C14_REG_CONVERSION_CTRL,
@@ -351,13 +473,20 @@ static int ads112c14_read_raw(struct iio_dev *indio_dev,
int *val2, long mask)
{
struct ads112c14_data *data = iio_priv(indio_dev);
+ struct ads112c14_measurement *measurement;
u32 vref_uV, fsr_bits;
+ int *scale_avail;
/* Selecting V_REF source is not implemented yet. */
vref_uV = ads112c14_internal_ref_uV[ADS112C14_REFERENCE_CFG_REF_VAL_2_5V];
- /* Currently, everything is using signed data. */
- fsr_bits = data->chip_info->resolution_bits - 1;
+ if (chan->channel < 100) {
+ measurement = &data->measurements[chan->scan_index];
+ fsr_bits = data->chip_info->resolution_bits - measurement->bipolar;
+ } else {
+ /* All SYS_MON channels are using signed coding. */
+ fsr_bits = data->chip_info->resolution_bits - 1;
+ }
switch (mask) {
case IIO_CHAN_INFO_RAW: {
@@ -383,7 +512,8 @@ static int ads112c14_read_raw(struct iio_dev *indio_dev,
return -EINVAL;
}
- *val = sign_extend32(*val, fsr_bits);
+ if (!measurement || measurement->bipolar)
+ *val = sign_extend32(*val, fsr_bits);
return IIO_VAL_INT;
}
@@ -396,7 +526,7 @@ static int ads112c14_read_raw(struct iio_dev *indio_dev,
}
if (chan->channel == ADS112C14_SYS_MON_CHANNEL_SHORT) {
- int *scale_avail = data->sys_mon_chan_short_scale_available[
+ scale_avail = data->sys_mon_chan_short_scale_available[
data->sys_mon_chan_short_gain_val
];
@@ -406,13 +536,23 @@ static int ads112c14_read_raw(struct iio_dev *indio_dev,
return IIO_VAL_INT_PLUS_NANO;
}
- *val = vref_uV / (MICRO / MILLI);
- /*
- * Last 3 SYS_MON channels (ext ref, AVDD, DVDD) need to be
- * multiplied by 8 to account for internal attenuation of / 8.
- */
- *val2 = fsr_bits - (chan->address >= 3 ? 3 : 0);
- return IIO_VAL_FRACTIONAL_LOG2;
+ if (chan->channel > 100) {
+ *val = vref_uV / (MICRO / MILLI);
+ /*
+ * Last 3 SYS_MON channels (ext ref, AVDD, DVDD) need to
+ * be multiplied by 8 to account for internal attenuation
+ * of / 8.
+ */
+ *val2 = fsr_bits - (chan->address >= 3 ? 3 : 0);
+
+ return IIO_VAL_FRACTIONAL_LOG2;
+ }
+
+ scale_avail = measurement->scale_available[measurement->gain_val];
+ *val = scale_avail[0];
+ *val2 = scale_avail[1];
+
+ return IIO_VAL_INT_PLUS_NANO;
case IIO_CHAN_INFO_OFFSET:
/* Only the temperature channel has an offset. */
if (chan->type != IIO_TEMP)
@@ -433,6 +573,16 @@ static int ads112c14_read_avail(struct iio_dev *indio_dev,
{
struct ads112c14_data *data = iio_priv(indio_dev);
+ if (chan->channel < 100) {
+ struct ads112c14_measurement *measurement =
+ &data->measurements[chan->scan_index];
+
+ *vals = (const int *)measurement->scale_available;
+ *length = 2 * ARRAY_SIZE(measurement->scale_available);
+ *type = IIO_VAL_INT_PLUS_NANO;
+ return IIO_AVAIL_LIST;
+ }
+
switch (chan->channel) {
case ADS112C14_SYS_MON_CHANNEL_SHORT:
*vals = (const int *)data->sys_mon_chan_short_scale_available;
@@ -449,25 +599,31 @@ static int ads112c14_write_raw(struct iio_dev *indio_dev,
int val2, long mask)
{
struct ads112c14_data *data = iio_priv(indio_dev);
+ const int (*scale_avail)[2];
+ u8 *gain_val;
+
+ if (chan->channel == ADS112C14_SYS_MON_CHANNEL_SHORT) {
+ scale_avail = data->sys_mon_chan_short_scale_available;
+ gain_val = &data->sys_mon_chan_short_gain_val;
+ } else if (chan->channel < 100) {
+ scale_avail = data->measurements[chan->scan_index].scale_available;
+ gain_val = &data->measurements[chan->scan_index].gain_val;
+ } else {
+ return -EINVAL;
+ }
- switch (chan->channel) {
- case ADS112C14_SYS_MON_CHANNEL_SHORT: {
- IIO_DEV_ACQUIRE_DIRECT_MODE(indio_dev, claim);
- if (IIO_DEV_ACQUIRE_FAILED(claim))
- return -EBUSY;
+ IIO_DEV_ACQUIRE_DIRECT_MODE(indio_dev, claim);
+ if (IIO_DEV_ACQUIRE_FAILED(claim))
+ return -EBUSY;
- for (u32 i = 0; i < ARRAY_SIZE(data->sys_mon_chan_short_scale_available); i++) {
- if (val == data->sys_mon_chan_short_scale_available[i][0] &&
- val2 == data->sys_mon_chan_short_scale_available[i][1]) {
- data->sys_mon_chan_short_gain_val = i;
- return 0;
- }
+ for (u32 i = 0; i < ARRAY_SIZE(ads112c14_pga_gains_x10); i++) {
+ if (val == scale_avail[i][0] && val2 == scale_avail[i][1]) {
+ *gain_val = i;
+ return 0;
}
- return -EINVAL;
- }
- default:
- return -EINVAL;
}
+
+ return -EINVAL;
}
static int ads112c14_write_raw_get_fmt(struct iio_dev *indio_dev,
@@ -485,8 +641,20 @@ static int ads112c14_write_raw_get_fmt(struct iio_dev *indio_dev,
static int ads112c14_read_label(struct iio_dev *indio_dev,
struct iio_chan_spec const *chan, char *label)
{
+ struct ads112c14_data *data = iio_priv(indio_dev);
const char *label_source;
+ /* measurement channels */
+ if (chan->channel < 100) {
+ struct ads112c14_measurement *measurement =
+ &data->measurements[chan->scan_index];
+
+ if (!measurement->label)
+ return -EINVAL;
+
+ return sysfs_emit(label, "%s\n", measurement->label);
+ }
+
/* System monitor channels. */
switch (chan->channel) {
case ADS112C14_SYS_MON_CHANNEL_TEMP:
@@ -519,26 +687,187 @@ static const struct iio_info ads112c14_info = {
.read_label = ads112c14_read_label,
};
+static int ads112c14_parse_channels(struct iio_dev *indio_dev,
+ bool *need_avdd_ref, bool *need_ext_ref)
+{
+ struct ads112c14_data *data = iio_priv(indio_dev);
+ struct device *dev = &data->client->dev;
+ struct iio_chan_spec *channels;
+ u32 pair[2];
+ int i = 0;
+ int ret;
+
+ *need_avdd_ref = false;
+ *need_ext_ref = false;
+
+ data->num_measurements = device_get_named_child_node_count(dev, "channel");
+
+ data->measurements = devm_kcalloc(dev, data->num_measurements,
+ sizeof(*data->measurements), GFP_KERNEL);
+ if (!data->measurements)
+ return -ENOMEM;
+
+ data->num_measurements += ARRAY_SIZE(ads112c14_sys_mon_channels);
+
+ channels = devm_kcalloc(dev, data->num_measurements +
+ ARRAY_SIZE(ads112c14_sys_mon_channels),
+ sizeof(*channels), GFP_KERNEL);
+ if (!channels)
+ return -ENOMEM;
+
+ device_for_each_named_child_node_scoped(dev, child, "channel") {
+ struct ads112c14_measurement *measurement = &data->measurements[i];
+ struct iio_chan_spec *spec = &channels[i];
+
+ if (!fwnode_device_is_available(child))
+ continue;
+
+ spec->type = IIO_VOLTAGE;
+ spec->indexed = 1;
+ spec->scan_index = i;
+ measurement->gain_val = 1;
+
+ fwnode_property_read_string(child, "label", &measurement->label);
+
+ if (fwnode_property_present(child, "single-channel")) {
+ ret = fwnode_property_read_u32(child, "single-channel", &spec->channel);
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "failed to read single-channel property\n");
+
+ if (spec->channel >= 8)
+ return dev_err_probe(dev, -EINVAL,
+ "single-channel value must be between 0 and 7\n");
+ } else if (fwnode_property_present(child, "diff-channels")) {
+ ret = fwnode_property_read_u32_array(child, "diff-channels", pair, ARRAY_SIZE(pair));
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "failed to read diff-channels property\n");
+
+ if (pair[0] >= 8 || pair[1] >= 8)
+ return dev_err_probe(dev, -EINVAL,
+ "diff-channels values must be between 0 and 7\n");
+
+ spec->channel = pair[0];
+ spec->channel2 = pair[1];
+ spec->differential = 1;
+ } else {
+ return dev_err_probe(dev, -EINVAL,
+ "channel node missing channel type property\n");
+ }
+
+ if (fwnode_property_present(child, "excitation-channels")) {
+ ret = fwnode_property_count_u32(child, "excitation-channels");
+ if (ret < 0)
+ return dev_err_probe(dev, ret,
+ "failed to read excitation-channels property\n");
+
+ if (ret < 1 || ret > 2)
+ return dev_err_probe(dev, -EINVAL,
+ "excitation-channels property must have 1 or 2 values\n");
+
+ measurement->iadc_count = ret;
+ pair[1] = 0;
+
+ ret = fwnode_property_read_u32_array(child, "excitation-channels", pair, measurement->iadc_count);
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "failed to read excitation-channels property\n");
+
+ if (pair[0] >= 8 || pair[1] >= 8)
+ return dev_err_probe(dev, -EINVAL,
+ "excitation-channels values must be between 0 and 7\n");
+
+ measurement->idac1_mux = pair[0];
+ measurement->idac2_mux = measurement->iadc_count > 1 ? pair[1] : 0;
+
+ ret = fwnode_property_read_u32(child, "excitation-current-microamp",
+ &measurement->idac_current_uA);
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "failed to read excitation-current-microamp property\n");
+
+ measurement->current_chop = fwnode_property_read_bool(child, "current-chopping");
+ }
+
+ measurement->bipolar = fwnode_property_read_bool(child, "bipolar");
+
+ fwnode_property_read_u32(child, "ti,vref-source", &measurement->vref_source);
+ if (measurement->vref_source > ADS112C14_VREF_SOURCE_AVDD)
+ return dev_err_probe(dev, -EINVAL,
+ "invalid vref-source value\n");
+
+ if (measurement->vref_source == ADS112C14_VREF_SOURCE_AVDD)
+ *need_avdd_ref = true;
+ if (measurement->vref_source == ADS112C14_VREF_SOURCE_EXTERNAL)
+ *need_ext_ref = true;
+
+ spec->info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE);
+ spec->info_mask_separate_available = BIT(IIO_CHAN_INFO_SCALE);
+
+ i++;
+ }
+
+ memcpy(channels + i, ads112c14_sys_mon_channels, sizeof(ads112c14_sys_mon_channels));
+
+ indio_dev->channels = channels;
+ indio_dev->num_channels = i + ARRAY_SIZE(ads112c14_sys_mon_channels);
+
+ return 0;
+}
+
+static void ads112c14_populate_scale_available(int scale_avail[][2],
+ u32 vref_uV, u32 fsr_bits)
+{
+ for (u32 i = 0; i < ARRAY_SIZE(ads112c14_pga_gains_x10); i++) {
+ int *entry = scale_avail[i];
+ u32 gain_x10 = ads112c14_pga_gains_x10[i];
+
+ entry[0] = div_u64_rem(div64_u64((u64)(NANO * 10 /
+ (MICRO / MILLI)) * vref_uV,
+ (u64)gain_x10 * BIT(fsr_bits)),
+ NANO, &entry[1]);
+ }
+}
+
static void ads112c14_populate_tables(struct ads112c14_data *data)
{
u32 vref_uV, fsr_bits;
- int i;
+
+ for (u32 i = 0; i < data->num_measurements; i++) {
+ struct ads112c14_measurement *measurement = &data->measurements[i];
+
+ switch (measurement->vref_source) {
+ case ADS112C14_VREF_SOURCE_EXTERNAL:
+ if (data->ext_ref_ohms)
+ vref_uV = measurement->idac_current_uA *
+ measurement->iadc_count * data->ext_ref_ohms;
+ else
+ vref_uV = data->ext_ref_uV;
+ break;
+ case ADS112C14_VREF_SOURCE_AVDD:
+ vref_uV = data->avdd_uV;
+ break;
+ case ADS112C14_VREF_SOURCE_INTERNAL_1_25V:
+ vref_uV = ads112c14_internal_ref_uV[ADS112C14_REFERENCE_CFG_REF_VAL_1_25V];
+ break;
+ default:
+ vref_uV = ads112c14_internal_ref_uV[ADS112C14_REFERENCE_CFG_REF_VAL_2_5V];
+ break;
+ }
+
+ fsr_bits = data->chip_info->resolution_bits - measurement->bipolar;
+
+ ads112c14_populate_scale_available(measurement->scale_available,
+ vref_uV, fsr_bits);
+ }
/* For now, assuming we are using 2.5V reference. */
vref_uV = ads112c14_internal_ref_uV[ADS112C14_REFERENCE_CFG_REF_VAL_2_5V];
fsr_bits = data->chip_info->resolution_bits - 1;
- for (i = 0; i < ARRAY_SIZE(ads112c14_pga_gains_x10); i++) {
- int *scale_avail = &data->sys_mon_chan_short_scale_available[i][0];
- u32 gain_x10 = ads112c14_pga_gains_x10[i];
-
- /* NB: slightly odd arrangement to avoid overflow. */
- scale_avail[0] = div_u64_rem(div_u64((u64)NANO * 10 /
- (MICRO / MILLI) * vref_uV /
- gain_x10,
- BIT(fsr_bits)),
- NANO, &scale_avail[1]);
- }
+ ads112c14_populate_scale_available(data->sys_mon_chan_short_scale_available,
+ vref_uV, fsr_bits);
}
static int ads112c14_probe(struct i2c_client *client)
@@ -547,6 +876,9 @@ static int ads112c14_probe(struct i2c_client *client)
const struct ads112c14_chip_info *info;
struct iio_dev *indio_dev;
struct ads112c14_data *data;
+ bool need_avdd_ref, need_ext_ref;
+ u32 refp_uV = 0;
+ u32 refn_uV = 0;
u32 reg_val;
int ret;
@@ -560,13 +892,75 @@ static int ads112c14_probe(struct i2c_client *client)
data->chip_info = info;
data->client = client;
+ ret = ads112c14_parse_channels(indio_dev, &need_avdd_ref, &need_ext_ref);
+ if (ret)
+ return ret;
+
ret = devm_regulator_get_enable(&client->dev, "dvdd");
if (ret)
return dev_err_probe(dev, ret, "failed to get dvdd regulator\n");
- ret = devm_regulator_get_enable(&client->dev, "avdd");
- if (ret)
- return dev_err_probe(dev, ret, "failed to get avdd regulator\n");
+ if (need_avdd_ref) {
+ ret = devm_regulator_get_enable_read_voltage(&client->dev, "avdd");
+ if (ret)
+ return dev_err_probe(dev, ret, "failed to get avdd voltage\n");
+
+ data->avdd_uV = ret;
+ } else {
+ ret = devm_regulator_get_enable(&client->dev, "avdd");
+ if (ret)
+ return dev_err_probe(dev, ret, "failed to get avdd regulator\n");
+ }
+
+ if (device_property_present(dev, "refp-supply")) {
+ ret = devm_regulator_get_enable_read_voltage(&client->dev, "refp");
+ if (ret)
+ return dev_err_probe(dev, ret, "failed to get refp voltage\n");
+
+ refp_uV = ret;
+
+ struct fwnode_handle *refp_fwnode __free(fwnode_handle) =
+ fwnode_find_reference(dev->fwnode, "refp-supply", 0);
+ if (IS_ERR(refp_fwnode))
+ return dev_err_probe(dev, PTR_ERR(refp_fwnode),
+ "failed to get refp fwnode\n");
+
+ struct fwnode_handle *avdd_fwnode __free(fwnode_handle) =
+ fwnode_find_reference(dev->fwnode, "avdd-supply", 0);
+ if (IS_ERR(avdd_fwnode))
+ return dev_err_probe(dev, PTR_ERR(avdd_fwnode),
+ "failed to get avdd fwnode\n");
+
+ data->refp_is_avdd = refp_fwnode == avdd_fwnode;
+ }
+
+ if (device_property_present(dev, "refn-supply")) {
+ ret = devm_regulator_get_enable_read_voltage(&client->dev, "refn");
+ if (ret)
+ return dev_err_probe(dev, ret, "failed to get refn voltage\n");
+
+ refn_uV = ret;
+ } else {
+ data->refn_is_gnd = true;
+ }
+
+ data->ext_ref_uV = refp_uV - refn_uV;
+
+ if (device_property_present(dev, "refp-refn-resistor-ohms")) {
+ if (refp_uV != 0 || refn_uV != 0)
+ return dev_err_probe(dev, -EINVAL,
+ "refp-refn-resistor-ohms property should not be present when refp-supply or refn-supply is present\n");
+
+ ret = device_property_read_u32(dev, "refp-refn-resistor-ohms",
+ &data->ext_ref_ohms);
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "failed to read refp-refn-resistor-ohms property\n");
+ } else {
+ if (need_ext_ref && data->ext_ref_uV == 0)
+ return dev_err_probe(dev, -EINVAL,
+ "external reference measurements require either refp-supply or refp-refn-resistor-ohms property\n");
+ }
data->regmap = devm_regmap_init_i2c(client, &ads112c14_regmap_config);
if (IS_ERR(data->regmap))
@@ -615,8 +1009,6 @@ static int ads112c14_probe(struct i2c_client *client)
indio_dev->name = info->name;
indio_dev->modes = INDIO_DIRECT_MODE;
- indio_dev->channels = ads112c14_sys_mon_channels;
- indio_dev->num_channels = ARRAY_SIZE(ads112c14_sys_mon_channels);
indio_dev->info = &ads112c14_info;
return devm_iio_device_register(&client->dev, indio_dev);
--
2.43.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH 4/4] iio: adc: ti-ads112c14: add measurement channel support
2026-06-15 22:00 ` [PATCH 4/4] iio: adc: ti-ads112c14: add measurement channel support David Lechner (TI)
@ 2026-06-16 8:36 ` Andy Shevchenko
2026-06-16 15:55 ` David Lechner
2026-06-16 15:30 ` David Lechner
1 sibling, 1 reply; 20+ messages in thread
From: Andy Shevchenko @ 2026-06-16 8:36 UTC (permalink / raw)
To: David Lechner (TI)
Cc: Jonathan Cameron, Nuno Sá, Andy Shevchenko, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Kurt Borja, Nguyen Minh Tien,
linux-iio, devicetree, linux-kernel
On Mon, Jun 15, 2026 at 05:00:02PM -0500, David Lechner (TI) wrote:
> Add support for parsing devicetree properties for measurement channels
> and doing direct reads on these.
>
> There are quite a lot of conditions that have to be met for each
> measurement to be made, so quite a bit of state and algorithms are
> required to handle it.
>
> Channels are created dynamically since the number of possibilities is
> unreasonably large.
...
> +struct ads112c14_measurement {
> + const char *label;
> + u32 vref_source;
> + u32 idac_current_uA;
> + u8 iadc_count;
> + u8 idac1_mux;
> + u8 idac2_mux;
> + bool current_chop;
> + bool bipolar;
> + u8 gain_val;
Hmm... Can we group u8:s for a better visual?
> + int scale_available[ARRAY_SIZE(ads112c14_pga_gains_x10)][2];
> +};
...
> struct ads112c14_data {
> const struct ads112c14_chip_info *chip_info;
> struct i2c_client *client;
> struct regmap *regmap;
> + u32 avdd_uV;
> + u32 ext_ref_uV;
> + bool refp_is_avdd;
> + bool refn_is_gnd;
> + u32 ext_ref_ohms;
> + struct ads112c14_measurement *measurements;
> + u32 num_measurements;
> u8 sys_mon_chan_short_gain_val;
If you run `pahole` you will see a gap that can be easily avoided.
> int sys_mon_chan_short_scale_available[ARRAY_SIZE(ads112c14_pga_gains_x10)][2];
> };
...
> +static int ads112c14_prepare_measurement_channel(struct ads112c14_data *data,
> + const struct iio_chan_spec *chan)
> +{
> + struct ads112c14_measurement *measurement = &data->measurements[chan->scan_index];
> + bool iunit = measurement->idac_current_uA > 100;
> + u32 idac1_val = measurement->iadc_count > 0 ?
> + measurement->idac_current_uA / (iunit ? 100 : 10) : 0;
> + u32 idac2_val = measurement->iadc_count > 1 ? idac1_val : 0;
These three will look much better when decoupled from the definitions.
> + u32 refp_buf_en, refn_buf_en;
> + u32 ref_val, ref_sel;
> + int ret;
iunit = measurement->idac_current_uA > 100;
if (measurement->iadc_count > 0)
idac1_val = measurement->idac_current_uA / (iunit ? 100 : 10);
else
idac1_val = 0;
idac2_val = measurement->iadc_count > 1 ? idac1_val : 0;
> + ret = regmap_write(data->regmap, ADS112C14_REG_MUX_CFG,
> + FIELD_PREP(ADS112C14_MUX_CFG_AINP, chan->channel) |
> + FIELD_PREP(ADS112C14_MUX_CFG_AINN, chan->channel2));
> + if (ret)
> + return ret;
> +
> + ret = regmap_update_bits(data->regmap, ADS112C14_REG_DIGITAL_CFG,
> + ADS112C14_DIGITAL_CFG_CODING,
> + FIELD_PREP(ADS112C14_DIGITAL_CFG_CODING,
> + measurement->bipolar ? 0 : 1));
> + if (ret)
> + return ret;
> +
> + ret = regmap_update_bits(data->regmap, ADS112C14_REG_GAIN_CFG,
> + ADS112C14_GAIN_CFG_SYS_MON | ADS112C14_GAIN_CFG_GAIN,
> + FIELD_PREP(ADS112C14_GAIN_CFG_SYS_MON, 0) |
> + FIELD_PREP(ADS112C14_GAIN_CFG_GAIN,
> + measurement->gain_val));
> + if (ret)
> + return ret;
> +
> + ret = regmap_write(data->regmap, ADS112C14_REG_IDAC_MAG_CFG,
> + FIELD_PREP(ADS112C14_IDAC_MAG_CFG_I2MAG, idac2_val) |
> + FIELD_PREP(ADS112C14_IDAC_MAG_CFG_I1MAG, idac1_val));
> + if (ret)
> + return ret;
> +
> + ret = regmap_write(data->regmap, ADS112C14_REG_IDAC_MUX_CFG,
> + (iunit ? ADS112C14_IDAC_MUX_CFG_IUNIT : 0) |
> + FIELD_PREP(ADS112C14_IDAC_MUX_CFG_I2MUX,
> + measurement->idac2_mux) |
> + FIELD_PREP(ADS112C14_IDAC_MUX_CFG_I1MUX,
> + measurement->idac1_mux));
> + if (ret)
> + return ret;
> +
> + ret = regmap_update_bits(data->regmap, ADS112C14_REG_DATA_RATE_CFG,
> + ADS112C14_DATA_RATE_CFG_GC_EN,
> + FIELD_PREP(ADS112C14_DATA_RATE_CFG_GC_EN,
> + measurement->current_chop));
> + if (ret)
> + return ret;
> +
> + refp_buf_en = !data->refp_is_avdd &&
> + measurement->vref_source == ADS112C14_VREF_SOURCE_EXTERNAL;
> + refn_buf_en = !data->refn_is_gnd &&
> + measurement->vref_source == ADS112C14_VREF_SOURCE_EXTERNAL;
> +
> + ref_val = measurement->vref_source == ADS112C14_VREF_SOURCE_INTERNAL_2_5V;
> +
> + switch (measurement->vref_source) {
> + case ADS112C14_VREF_SOURCE_AVDD:
> + ref_sel = ADS112C14_REFERENCE_CFG_REF_SEL_AVDD;
> + break;
> + case ADS112C14_VREF_SOURCE_EXTERNAL:
> + ref_sel = ADS112C14_REFERENCE_CFG_REF_SEL_EXTERNAL;
> + break;
> + default:
> + ref_sel = ADS112C14_REFERENCE_CFG_REF_SEL_INTERNAL;
> + break;
> + }
> +
> + return regmap_update_bits(data->regmap, ADS112C14_REG_REFERENCE_CFG,
> + ADS112C14_REFERENCE_CFG_REFP_BUF_EN |
> + ADS112C14_REFERENCE_CFG_REFN_BUF_EN |
> + ADS112C14_REFERENCE_CFG_REF_VAL |
> + ADS112C14_REFERENCE_CFG_REF_SEL,
> + FIELD_PREP(ADS112C14_REFERENCE_CFG_REFP_BUF_EN,
> + refp_buf_en) |
> + FIELD_PREP(ADS112C14_REFERENCE_CFG_REFN_BUF_EN,
> + refn_buf_en) |
> + FIELD_PREP(ADS112C14_REFERENCE_CFG_REF_VAL,
> + ref_val) |
> + FIELD_PREP(ADS112C14_REFERENCE_CFG_REF_SEL,
> + ref_sel));
> +}
...
> - if (chan->channel >= 100) {
> - ret = ads112c14_prepare_sys_mon_channel(data, chan);
> + if (chan->channel < 100) {
> + ret = ads112c14_prepare_measurement_channel(data, chan);
> if (ret)
> return ret;
> } else {
> - /* Not implemented yet. */
> - return -EINVAL;
> + ret = ads112c14_prepare_sys_mon_channel(data, chan);
> + if (ret)
> + return ret;
> }
Why ping-ponging this? Leave the condition as is. Or start with the < 100 in
the previous patch.
...
> + if (chan->channel < 100) {
For the consistency's sake keep it the similar to the above.
> + measurement = &data->measurements[chan->scan_index];
> + fsr_bits = data->chip_info->resolution_bits - measurement->bipolar;
> + } else {
> + /* All SYS_MON channels are using signed coding. */
> + fsr_bits = data->chip_info->resolution_bits - 1;
> + }
...
> if (chan->channel == ADS112C14_SYS_MON_CHANNEL_SHORT) {
> - int *scale_avail = data->sys_mon_chan_short_scale_available[
> + scale_avail = data->sys_mon_chan_short_scale_available[
> data->sys_mon_chan_short_gain_val
> ];
Oh, can we simply move the index to the temporary variable and make this look
better?
...
> - for (u32 i = 0; i < ARRAY_SIZE(data->sys_mon_chan_short_scale_available); i++) {
> - if (val == data->sys_mon_chan_short_scale_available[i][0] &&
> - val2 == data->sys_mon_chan_short_scale_available[i][1]) {
> - data->sys_mon_chan_short_gain_val = i;
> - return 0;
> - }
> + for (u32 i = 0; i < ARRAY_SIZE(ads112c14_pga_gains_x10); i++) {
> + if (val == scale_avail[i][0] && val2 == scale_avail[i][1]) {
> + *gain_val = i;
> + return 0;
> }
> - return -EINVAL;
> - }
Can this be written in a better form initially to make less churn here?
...
> + /* measurement channels */
> + if (chan->channel < 100) {
> + struct ads112c14_measurement *measurement =
> + &data->measurements[chan->scan_index];
> + if (!measurement->label)
> + return -EINVAL;
Hmm... Can it be true?
> + return sysfs_emit(label, "%s\n", measurement->label);
> + }
...
> +static int ads112c14_parse_channels(struct iio_dev *indio_dev,
> + bool *need_avdd_ref, bool *need_ext_ref)
> +{
> + struct ads112c14_data *data = iio_priv(indio_dev);
> + struct device *dev = &data->client->dev;
> + struct iio_chan_spec *channels;
> + u32 pair[2];
> + int i = 0;
Why signed? Also decouple assignment from definition.
> + int ret;
> +
> + *need_avdd_ref = false;
> + *need_ext_ref = false;
> +
> + data->num_measurements = device_get_named_child_node_count(dev, "channel");
> + data->measurements = devm_kcalloc(dev, data->num_measurements,
> + sizeof(*data->measurements), GFP_KERNEL);
> + if (!data->measurements)
> + return -ENOMEM;
> +
> + data->num_measurements += ARRAY_SIZE(ads112c14_sys_mon_channels);
> +
> + channels = devm_kcalloc(dev, data->num_measurements +
> + ARRAY_SIZE(ads112c14_sys_mon_channels),
size_add()? But here you add it twice, why?
> + sizeof(*channels), GFP_KERNEL);
> + if (!channels)
> + return -ENOMEM;
> +
> + device_for_each_named_child_node_scoped(dev, child, "channel") {
> + struct ads112c14_measurement *measurement = &data->measurements[i];
> + struct iio_chan_spec *spec = &channels[i];
> +
> + if (!fwnode_device_is_available(child))
> + continue;
> +
> + spec->type = IIO_VOLTAGE;
> + spec->indexed = 1;
> + spec->scan_index = i;
> + measurement->gain_val = 1;
> +
> + fwnode_property_read_string(child, "label", &measurement->label);
> + if (fwnode_property_present(child, "single-channel")) {
> + ret = fwnode_property_read_u32(child, "single-channel", &spec->channel);
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "failed to read single-channel property\n");
> +
> + if (spec->channel >= 8)
> + return dev_err_probe(dev, -EINVAL,
> + "single-channel value must be between 0 and 7\n");
> + } else if (fwnode_property_present(child, "diff-channels")) {
> + ret = fwnode_property_read_u32_array(child, "diff-channels", pair, ARRAY_SIZE(pair));
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "failed to read diff-channels property\n");
> +
> + if (pair[0] >= 8 || pair[1] >= 8)
> + return dev_err_probe(dev, -EINVAL,
> + "diff-channels values must be between 0 and 7\n");
> +
> + spec->channel = pair[0];
> + spec->channel2 = pair[1];
> + spec->differential = 1;
> + } else {
> + return dev_err_probe(dev, -EINVAL,
> + "channel node missing channel type property\n");
> + }
Looking how it's going to spread (I mean the above pattern), perhaps it's a time to introduce bunch of
fwnode_property_read_*_optional()
and the respective device_property_read_*_optional()?
Let's start from u32 case only, as it will be most used anyway.
> + if (fwnode_property_present(child, "excitation-channels")) {
> + ret = fwnode_property_count_u32(child, "excitation-channels");
> + if (ret < 0)
> + return dev_err_probe(dev, ret,
> + "failed to read excitation-channels property\n");
> +
> + if (ret < 1 || ret > 2)
> + return dev_err_probe(dev, -EINVAL,
> + "excitation-channels property must have 1 or 2 values\n");
> +
> + measurement->iadc_count = ret;
> + pair[1] = 0;
> +
> + ret = fwnode_property_read_u32_array(child, "excitation-channels", pair, measurement->iadc_count);
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "failed to read excitation-channels property\n");
> +
> + if (pair[0] >= 8 || pair[1] >= 8)
> + return dev_err_probe(dev, -EINVAL,
> + "excitation-channels values must be between 0 and 7\n");
> +
> + measurement->idac1_mux = pair[0];
> + measurement->idac2_mux = measurement->iadc_count > 1 ? pair[1] : 0;
> +
> + ret = fwnode_property_read_u32(child, "excitation-current-microamp",
> + &measurement->idac_current_uA);
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "failed to read excitation-current-microamp property\n");
> +
> + measurement->current_chop = fwnode_property_read_bool(child, "current-chopping");
> + }
> +
> + measurement->bipolar = fwnode_property_read_bool(child, "bipolar");
> +
> + fwnode_property_read_u32(child, "ti,vref-source", &measurement->vref_source);
> + if (measurement->vref_source > ADS112C14_VREF_SOURCE_AVDD)
> + return dev_err_probe(dev, -EINVAL,
> + "invalid vref-source value\n");
> +
> + if (measurement->vref_source == ADS112C14_VREF_SOURCE_AVDD)
> + *need_avdd_ref = true;
> + if (measurement->vref_source == ADS112C14_VREF_SOURCE_EXTERNAL)
> + *need_ext_ref = true;
> +
> + spec->info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE);
> + spec->info_mask_separate_available = BIT(IIO_CHAN_INFO_SCALE);
> +
> + i++;
> + }
> +
> + memcpy(channels + i, ads112c14_sys_mon_channels, sizeof(ads112c14_sys_mon_channels));
> +
> + indio_dev->channels = channels;
> + indio_dev->num_channels = i + ARRAY_SIZE(ads112c14_sys_mon_channels);
> +
> + return 0;
> +}
...
> +static void ads112c14_populate_scale_available(int scale_avail[][2],
> + u32 vref_uV, u32 fsr_bits)
> +{
> + for (u32 i = 0; i < ARRAY_SIZE(ads112c14_pga_gains_x10); i++) {
> + int *entry = scale_avail[i];
> + u32 gain_x10 = ads112c14_pga_gains_x10[i];
> +
> + entry[0] = div_u64_rem(div64_u64((u64)(NANO * 10 /
> + (MICRO / MILLI)) * vref_uV,
> + (u64)gain_x10 * BIT(fsr_bits)),
Hmm... This differs from the previous implementation. Why?
> + NANO, &entry[1]);
> + }
> +}
...
> + if (device_property_present(dev, "refp-refn-resistor-ohms")) {
> + if (refp_uV != 0 || refn_uV != 0)
> + return dev_err_probe(dev, -EINVAL,
> + "refp-refn-resistor-ohms property should not be present when refp-supply or refn-supply is present\n");
> +
> + ret = device_property_read_u32(dev, "refp-refn-resistor-ohms",
> + &data->ext_ref_ohms);
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "failed to read refp-refn-resistor-ohms property\n");
Using
const char *propname;
...
propname = "refp-refn-resistor-ohms";
makes this
if (device_property_present(dev, propname)) {
if (refp_uV != 0 || refn_uV != 0)
return dev_err_probe(dev, -EINVAL,
"%s property should not be present when refp-supply or refn-supply is present\n",
propname);
ret = device_property_read_u32(dev, propname, &data->ext_ref_ohms);
if (ret)
return dev_err_probe(dev, ret, "failed to read %s property\n", propname);
Also the rest can be improved in the similar way.
> + } else {
> + if (need_ext_ref && data->ext_ref_uV == 0)
> + return dev_err_probe(dev, -EINVAL,
> + "external reference measurements require either refp-supply or refp-refn-resistor-ohms property\n");
> + }
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 4/4] iio: adc: ti-ads112c14: add measurement channel support
2026-06-16 8:36 ` Andy Shevchenko
@ 2026-06-16 15:55 ` David Lechner
0 siblings, 0 replies; 20+ messages in thread
From: David Lechner @ 2026-06-16 15:55 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Jonathan Cameron, Nuno Sá, Andy Shevchenko, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Kurt Borja, Nguyen Minh Tien,
linux-iio, devicetree, linux-kernel
On 6/16/26 3:36 AM, Andy Shevchenko wrote:
> On Mon, Jun 15, 2026 at 05:00:02PM -0500, David Lechner (TI) wrote:
>> Add support for parsing devicetree properties for measurement channels
>> and doing direct reads on these.
>>
>> There are quite a lot of conditions that have to be met for each
>> measurement to be made, so quite a bit of state and algorithms are
>> required to handle it.
>>
>> Channels are created dynamically since the number of possibilities is
>> unreasonably large.
>
>> + /* measurement channels */
>> + if (chan->channel < 100) {
>> + struct ads112c14_measurement *measurement =
>> + &data->measurements[chan->scan_index];
>
>> + if (!measurement->label)
>> + return -EINVAL;
>
> Hmm... Can it be true?
Yes. For some channels, label comes from the devicetree, which
may not have provided a label.
>
>> + return sysfs_emit(label, "%s\n", measurement->label);
>> + }
>
...
>> + if (fwnode_property_present(child, "single-channel")) {
>> + ret = fwnode_property_read_u32(child, "single-channel", &spec->channel);
>> + if (ret)
>> + return dev_err_probe(dev, ret,
>> + "failed to read single-channel property\n");
>> +
>> + if (spec->channel >= 8)
>> + return dev_err_probe(dev, -EINVAL,
>> + "single-channel value must be between 0 and 7\n");
>> + } else if (fwnode_property_present(child, "diff-channels")) {
>> + ret = fwnode_property_read_u32_array(child, "diff-channels", pair, ARRAY_SIZE(pair));
>> + if (ret)
>> + return dev_err_probe(dev, ret,
>> + "failed to read diff-channels property\n");
>> +
>> + if (pair[0] >= 8 || pair[1] >= 8)
>> + return dev_err_probe(dev, -EINVAL,
>> + "diff-channels values must be between 0 and 7\n");
>> +
>> + spec->channel = pair[0];
>> + spec->channel2 = pair[1];
>> + spec->differential = 1;
>> + } else {
>> + return dev_err_probe(dev, -EINVAL,
>> + "channel node missing channel type property\n");
>> + }
>
> Looking how it's going to spread (I mean the above pattern), perhaps it's a time to introduce bunch of
>
> fwnode_property_read_*_optional()
>
> and the respective device_property_read_*_optional()?
>
> Let's start from u32 case only, as it will be most used anyway.
I don't think that would be really any different from device_property_read_*
and checking for -EINVAL or ignoring the error completely. TBH, I really like
it this way with fwnode_property_present().
>
>> + if (fwnode_property_present(child, "excitation-channels")) {
>> + ret = fwnode_property_count_u32(child, "excitation-channels");
>> + if (ret < 0)
>> + return dev_err_probe(dev, ret,
>> + "failed to read excitation-channels property\n");
>> +
>> + if (ret < 1 || ret > 2)
>> + return dev_err_probe(dev, -EINVAL,
>> + "excitation-channels property must have 1 or 2 values\n");
>> +
>> + measurement->iadc_count = ret;
>> + pair[1] = 0;
>> +
>> + ret = fwnode_property_read_u32_array(child, "excitation-channels", pair, measurement->iadc_count);
>> + if (ret)
>> + return dev_err_probe(dev, ret,
>> + "failed to read excitation-channels property\n");
>> +
>> + if (pair[0] >= 8 || pair[1] >= 8)
>> + return dev_err_probe(dev, -EINVAL,
>> + "excitation-channels values must be between 0 and 7\n");
>> +
>> + measurement->idac1_mux = pair[0];
>> + measurement->idac2_mux = measurement->iadc_count > 1 ? pair[1] : 0;
>> +
>> + ret = fwnode_property_read_u32(child, "excitation-current-microamp",
>> + &measurement->idac_current_uA);
>> + if (ret)
>> + return dev_err_probe(dev, ret,
>> + "failed to read excitation-current-microamp property\n");
>> +
>> + measurement->current_chop = fwnode_property_read_bool(child, "current-chopping");
>> + }
>> +
>> + measurement->bipolar = fwnode_property_read_bool(child, "bipolar");
>> +
>> + fwnode_property_read_u32(child, "ti,vref-source", &measurement->vref_source);
>> + if (measurement->vref_source > ADS112C14_VREF_SOURCE_AVDD)
>> + return dev_err_probe(dev, -EINVAL,
>> + "invalid vref-source value\n");
>> +
>> + if (measurement->vref_source == ADS112C14_VREF_SOURCE_AVDD)
>> + *need_avdd_ref = true;
>> + if (measurement->vref_source == ADS112C14_VREF_SOURCE_EXTERNAL)
>> + *need_ext_ref = true;
>> +
>> + spec->info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE);
>> + spec->info_mask_separate_available = BIT(IIO_CHAN_INFO_SCALE);
>> +
>> + i++;
>> + }
>> +
>> + memcpy(channels + i, ads112c14_sys_mon_channels, sizeof(ads112c14_sys_mon_channels));
>> +
>> + indio_dev->channels = channels;
>> + indio_dev->num_channels = i + ARRAY_SIZE(ads112c14_sys_mon_channels);
>> +
>> + return 0;
>> +}
>
> ...
>
>> +static void ads112c14_populate_scale_available(int scale_avail[][2],
>> + u32 vref_uV, u32 fsr_bits)
>> +{
>> + for (u32 i = 0; i < ARRAY_SIZE(ads112c14_pga_gains_x10); i++) {
>> + int *entry = scale_avail[i];
>> + u32 gain_x10 = ads112c14_pga_gains_x10[i];
>> +
>> + entry[0] = div_u64_rem(div64_u64((u64)(NANO * 10 /
>> + (MICRO / MILLI)) * vref_uV,
>> + (u64)gain_x10 * BIT(fsr_bits)),
>
> Hmm... This differs from the previous implementation. Why?
Probably fixed it during testing and missed that I needed to fix
the original patch too.
>
>> + NANO, &entry[1]);
>> + }
>> +}
>
> ...
>
>> + if (device_property_present(dev, "refp-refn-resistor-ohms")) {
>> + if (refp_uV != 0 || refn_uV != 0)
>> + return dev_err_probe(dev, -EINVAL,
>> + "refp-refn-resistor-ohms property should not be present when refp-supply or refn-supply is present\n");
>> +
>> + ret = device_property_read_u32(dev, "refp-refn-resistor-ohms",
>> + &data->ext_ref_ohms);
>> + if (ret)
>> + return dev_err_probe(dev, ret,
>> + "failed to read refp-refn-resistor-ohms property\n");
>
> Using
>
> const char *propname;
> ...
> propname = "refp-refn-resistor-ohms";
>
> makes this
>
> if (device_property_present(dev, propname)) {
> if (refp_uV != 0 || refn_uV != 0)
> return dev_err_probe(dev, -EINVAL,
> "%s property should not be present when refp-supply or refn-supply is present\n",
> propname);
>
> ret = device_property_read_u32(dev, propname, &data->ext_ref_ohms);
> if (ret)
> return dev_err_probe(dev, ret, "failed to read %s property\n", propname);
>
> Also the rest can be improved in the similar way.
Hmm... maybe less error prone, but makes the code harder to read IMHO.
Will think about it.
>
>> + } else {
>> + if (need_ext_ref && data->ext_ref_uV == 0)
>> + return dev_err_probe(dev, -EINVAL,
>> + "external reference measurements require either refp-supply or refp-refn-resistor-ohms property\n");
>> + }
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 4/4] iio: adc: ti-ads112c14: add measurement channel support
2026-06-15 22:00 ` [PATCH 4/4] iio: adc: ti-ads112c14: add measurement channel support David Lechner (TI)
2026-06-16 8:36 ` Andy Shevchenko
@ 2026-06-16 15:30 ` David Lechner
1 sibling, 0 replies; 20+ messages in thread
From: David Lechner @ 2026-06-16 15:30 UTC (permalink / raw)
To: Jonathan Cameron, Nuno Sá, Andy Shevchenko, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: Kurt Borja, Nguyen Minh Tien, linux-iio, devicetree, linux-kernel
On 6/15/26 5:00 PM, David Lechner (TI) wrote:
> Add support for parsing devicetree properties for measurement channels
> and doing direct reads on these.
>
> There are quite a lot of conditions that have to be met for each
> measurement to be made, so quite a bit of state and algorithms are
> required to handle it.
>
> Channels are created dynamically since the number of possibilities is
> unreasonably large.
>
...
> static void ads112c14_populate_tables(struct ads112c14_data *data)
> {
> u32 vref_uV, fsr_bits;
> - int i;
> +
> + for (u32 i = 0; i < data->num_measurements; i++) {
> + struct ads112c14_measurement *measurement = &data->measurements[i];
> +
> + switch (measurement->vref_source) {
> + case ADS112C14_VREF_SOURCE_EXTERNAL:
> + if (data->ext_ref_ohms)
> + vref_uV = measurement->idac_current_uA *
> + measurement->iadc_count * data->ext_ref_ohms;
One thing I am considering is that if we have a resistor between
REFP and REFN rather than a voltage supply, then we should consider
the channel IIO_RESISTANCE rather than IIO_VOLTAGE.
In this case, the user doesn't care about voltage at all, but rather
the ratio of the REFP-REFN resistor to the measured resistance.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 0/4] iio: adc: new ti-ads112c14 driver
2026-06-15 21:59 [PATCH 0/4] iio: adc: new ti-ads112c14 driver David Lechner (TI)
` (3 preceding siblings ...)
2026-06-15 22:00 ` [PATCH 4/4] iio: adc: ti-ads112c14: add measurement channel support David Lechner (TI)
@ 2026-06-16 0:18 ` Kurt Borja
2026-06-16 15:21 ` David Lechner
4 siblings, 1 reply; 20+ messages in thread
From: Kurt Borja @ 2026-06-16 0:18 UTC (permalink / raw)
To: David Lechner (TI), Jonathan Cameron, Nuno Sá,
Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Kurt Borja, Nguyen Minh Tien, linux-iio, devicetree, linux-kernel
Hi David,
On Mon Jun 15, 2026 at 4:59 PM -05, David Lechner (TI) wrote:
> This adds support for TI ADS112C14 and ADS122C14 ADC chips.
>
> The closest thing we've seen to this in the kernel already is ads124s08.
> However, that has a completely different register map and the DT
> bindings are incomplete and the driver is extremely basic. So I've just
> started from scratch here.
>
> We've also had a similar submission recently for ADS1220 [1]. That chip
> is in a similar situation to ads124s08 in that it has a different
> register map (but the submitted DT bindings are better than the ones for
> ads124s08, even if still a bit incomplete). And literally as I was
> writing the previous sentence, another series [2] was sent for yet
> another similar family of chips (ADS1262). That one is even more complex
> in the feature set than the ones I am working on. I was going to polish
> up the driver a bit more before submitting it, but now it seems more
> urgent to coordinate with the other two series to align on how we would
> like to handle all of these.
>
> [1]: https://lore.kernel.org/linux-iio/20260610151342.44274-1-zizuzacker@gmail.com/
> [2]: https://lore.kernel.org/linux-iio/20260612-ads126x-v1-0-894c788d03ed@gmail.com/
>
> All of these chips have in common that they are designed for use with
> RTDs and thermocouples and so they look very similar to each other in
> terms of wiring and feature set, even if the register maps are
> different. They are in the gray area where we could either keep them
> separate because they are just different enough, or we could do like
> we've done before with ad_sigma_delta and have a bit of an abstraction
> layer for the register differences and otherwise try to share as much
> code as possible. Normally, I would lean towards keeping them separate,
> but in this case, I'm considering trying to share code because the
> devicetree bindings for the inputs is complex and is going to be mostly
> the same across all of these chips.
The channel configuration is indeed very similar for the three chips.
All three have IDAC, BOC and VREF configurations.
>
> If we decide to go the route of sharing code, we could still merge this
> series as-is and then do the refactoring to add the abstraction layer in
> a follow-up series that also adds support for the first of the other
> chips.
Do you have a proposal of how such an abstraction would look like? I do
like the idea of abstracting the firmware parsing, scales and shared
calculations.
>
> This series includes just basic support for reading single measurements
> from the ADC and gain selection via the scale attribute. I plan to
> follow this up with additional series to add support for buffered reads,
> filtering/oversampling configuration, event support, gpio controller
> support and perhaps a few other things that are slipping my mind right
> now.
>
> The most interesting part about this (that I alluded to above) is the
> way channels are handled. These are multipling ADCs with differential
> and single-ended inputs. But what sets them apart from other similar
> chips is that since they are designed for use with RTDs, there can also
> be a current output required to excite the RTD and this current output
> might be different for different channels. So the way I conceptualized
> the channels is that the devicetree specifies the conditions needed
> to take a particular measurement rather than being purely a physical
> channel.
>
> This makes things more flexible, but does make the driver a bit more
> complex. For example, knowing when the current output needs to be
> enabled or disabled. For now, I have chosen a lazy-enable where they
> are not turned on until the first measurement is taken that requires
> them, but then they stay on until another measurement is taken that
> doesn't require them. This can lead to some oddness with the diagnostic
> channels that may be measuring something that indirectly requires the
> current output (i.e. the external reference voltage when it is connected
> to a resistor rather than a power supply). This means you need to take
> a measurement that requires the current output to be enabled before the
> diagnostic channels will give accurate readings.
This is the same approach I took around the BOC, it feels kinda hacky
but it makes sense. Just an idea I thought about just now: What if we
have an additional write-only "_enable" sysfs attribute for these
channels?
The approach I took for the IDAC was to have a single configuration that
it's enabled for all channels. This makes some sense in my device when
thinking about optimal software sequencial reads, because of the
register layout, but I also see the value in having per-channel IDAC
configuration. I think I will take your approach, so we have the same
channel configuration around this.
Have you thought about how to implement the BOC? In the ADS1262 the
feature can be found "Sensor Bias". What I did was add per channel DT
properties for this too.
Another question. When you implement power management in the future,
will you enable autosuspend? IDAC currents will be lost if autosuspend
is enabled. Is this acceptable? In my case I did enable autosuspend, but
I have some doubts about this.
>
> I have also pushed a branch to [3] that contains the start of some
> documentation for this driver that can give some more insight into how
> the implementation works. It still needs some work and also documents
> some things that haven't been implemented yet, so I haven't included it
> in this series yet.
>
> [3]: https://github.com/dlech/linux/blob/b4/iio-adc-ti-ads122c14/Documentation/iio/ads112c14.rst
>
> Signed-off-by: David Lechner <dlechner@baylibre.com>
> ---
> David Lechner (TI) (4):
> dt-bindings: iio: adc: add ti,ads122c14
> iio: adc: add ti-ads112c14 driver
> iio: adc: ti-ads112c14: implement gain on internal short SYS_MON channel
> iio: adc: ti-ads112c14: add measurement channel support
>
> .../devicetree/bindings/iio/adc/ti,ads112c14.yaml | 224 +++++
> MAINTAINERS | 8 +
> drivers/iio/adc/Kconfig | 12 +
> drivers/iio/adc/Makefile | 1 +
> drivers/iio/adc/ti-ads112c14.c | 1053 ++++++++++++++++++++
> include/dt-bindings/iio/adc/ti,ads112c14.h | 11 +
> 6 files changed, 1309 insertions(+)
> ---
> base-commit: ec039126b7fac4e3af35ebccaa7c6f9b6875ba81
> change-id: 20260514-iio-adc-ti-ads122c14-d0b92479334e
>
> Best regards,
> --
> David Lechner (TI) <dlechner@baylibre.com>
--
Thanks,
~ Kurt
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 0/4] iio: adc: new ti-ads112c14 driver
2026-06-16 0:18 ` [PATCH 0/4] iio: adc: new ti-ads112c14 driver Kurt Borja
@ 2026-06-16 15:21 ` David Lechner
2026-06-16 17:26 ` Kurt Borja
0 siblings, 1 reply; 20+ messages in thread
From: David Lechner @ 2026-06-16 15:21 UTC (permalink / raw)
To: Kurt Borja, Jonathan Cameron, Nuno Sá, Andy Shevchenko,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Nguyen Minh Tien, linux-iio, devicetree, linux-kernel
On 6/15/26 7:18 PM, Kurt Borja wrote:
> Hi David,
>
> On Mon Jun 15, 2026 at 4:59 PM -05, David Lechner (TI) wrote:
>> This adds support for TI ADS112C14 and ADS122C14 ADC chips.
>>
>> The closest thing we've seen to this in the kernel already is ads124s08.
>> However, that has a completely different register map and the DT
>> bindings are incomplete and the driver is extremely basic. So I've just
>> started from scratch here.
>>
>> We've also had a similar submission recently for ADS1220 [1]. That chip
>> is in a similar situation to ads124s08 in that it has a different
>> register map (but the submitted DT bindings are better than the ones for
>> ads124s08, even if still a bit incomplete). And literally as I was
>> writing the previous sentence, another series [2] was sent for yet
>> another similar family of chips (ADS1262). That one is even more complex
>> in the feature set than the ones I am working on. I was going to polish
>> up the driver a bit more before submitting it, but now it seems more
>> urgent to coordinate with the other two series to align on how we would
>> like to handle all of these.
>>
>> [1]: https://lore.kernel.org/linux-iio/20260610151342.44274-1-zizuzacker@gmail.com/
>> [2]: https://lore.kernel.org/linux-iio/20260612-ads126x-v1-0-894c788d03ed@gmail.com/
>>
>> All of these chips have in common that they are designed for use with
>> RTDs and thermocouples and so they look very similar to each other in
>> terms of wiring and feature set, even if the register maps are
>> different. They are in the gray area where we could either keep them
>> separate because they are just different enough, or we could do like
>> we've done before with ad_sigma_delta and have a bit of an abstraction
>> layer for the register differences and otherwise try to share as much
>> code as possible. Normally, I would lean towards keeping them separate,
>> but in this case, I'm considering trying to share code because the
>> devicetree bindings for the inputs is complex and is going to be mostly
>> the same across all of these chips.
>
> The channel configuration is indeed very similar for the three chips.
> All three have IDAC, BOC and VREF configurations.
Hmm... I forgot to include the burnout current in the DT bindings. Following
the channel = "conditions for measurement" pattern that I have set out here
I guess that would mean that we would need to have the same inputs twice
when using the burnout. One "channel" would be the one used to do a "precision"
measurement and the other would be the one to do open/short circuit detection.
i2c {
#address-cells = <1>;
#size-cells = <0>;
adc@40 {
compatible = "ti,ads112c14";
reg = <0x40>;
avdd-supply = <&avdd>;
dvdd-supply = <&dvdd>;
refp-supply = <&avdd>;
#address-cells = <1>;
#size-cells = <0>;
channel@0 {
reg = <0>;
diff-channels = <1>, <2>;
excitation-channels = <0>, <3>;
excitation-current-microamp = <500>;
current-chopping;
ti,vref-source = <ADS112C14_VREF_SOURCE_EXTERNAL>;
label = "rtd-precision";
};
channel@1 {
reg = <0>;
diff-channels = <1>, <2>;
excitation-channels = <0>, <3>;
excitation-current-microamp = <500>;
burnout-current-nanoamp = <1000>;
ti,vref-source = <ADS112C14_VREF_SOURCE_EXTERNAL>;
label = "rtd-diagnostic";
};
};
};
>
>>
>> If we decide to go the route of sharing code, we could still merge this
>> series as-is and then do the refactoring to add the abstraction layer in
>> a follow-up series that also adds support for the first of the other
>> chips.
>
> Do you have a proposal of how such an abstraction would look like? I do
> like the idea of abstracting the firmware parsing, scales and shared
> calculations.
As mentioned, it would look a bit like the ad_sigma_delta ADC driver.
Basically, each chip (family with same registers) would still have it's
own driver that include the chip-specific const info structs. In this
case, these would also include function pointers for all of the functions
that need to access registers. And there will be lots of supports_... bool
flags for the shared code to use.
>
>>
>> This series includes just basic support for reading single measurements
>> from the ADC and gain selection via the scale attribute. I plan to
>> follow this up with additional series to add support for buffered reads,
>> filtering/oversampling configuration, event support, gpio controller
>> support and perhaps a few other things that are slipping my mind right
>> now.
>>
>> The most interesting part about this (that I alluded to above) is the
>> way channels are handled. These are multipling ADCs with differential
>> and single-ended inputs. But what sets them apart from other similar
>> chips is that since they are designed for use with RTDs, there can also
>> be a current output required to excite the RTD and this current output
>> might be different for different channels. So the way I conceptualized
>> the channels is that the devicetree specifies the conditions needed
>> to take a particular measurement rather than being purely a physical
>> channel.
>>
>> This makes things more flexible, but does make the driver a bit more
>> complex. For example, knowing when the current output needs to be
>> enabled or disabled. For now, I have chosen a lazy-enable where they
>> are not turned on until the first measurement is taken that requires
>> them, but then they stay on until another measurement is taken that
>> doesn't require them. This can lead to some oddness with the diagnostic
>> channels that may be measuring something that indirectly requires the
>> current output (i.e. the external reference voltage when it is connected
>> to a resistor rather than a power supply). This means you need to take
>> a measurement that requires the current output to be enabled before the
>> diagnostic channels will give accurate readings.
>
> This is the same approach I took around the BOC, it feels kinda hacky
> but it makes sense. Just an idea I thought about just now: What if we
> have an additional write-only "_enable" sysfs attribute for these
> channels?
I would not want to make a write-only attribute, we always want to be
able to read back what the current state is.
Do you mean an _enable for just the BOC? I think I would do it like I
suggested above instead.
>
> The approach I took for the IDAC was to have a single configuration that
> it's enabled for all channels. This makes some sense in my device when
> thinking about optimal software sequencial reads, because of the
> register layout, but I also see the value in having per-channel IDAC
> configuration. I think I will take your approach, so we have the same
> channel configuration around this.
There are only two IDACs but on chips with 8 AIN pins we could have two
RTDs wired up and if both require 2 IDACs, then we can't have a fixed
setting for the IDACs. This is why I made it per-channel.
>
> Have you thought about how to implement the BOC? In the ADS1262 the
> feature can be found "Sensor Bias". What I did was add per channel DT
> properties for this too.
See above.
>
> Another question. When you implement power management in the future,
> will you enable autosuspend? IDAC currents will be lost if autosuspend
> is enabled. Is this acceptable? In my case I did enable autosuspend, but
> I have some doubts about this.
I don't like to implement power management unless I have an application
that actually requires it. Otherwise, I consider it premature optimization.
It is difficult to know if we would be implementing it in a way that is
actually useful for a real-world use case. These chips seem like they
would mostly be used in an industrial setting, not in battery powered
applications, so I'm not sure anyone is going to worry about saving a few
milliamps.
>
>>
>> I have also pushed a branch to [3] that contains the start of some
>> documentation for this driver that can give some more insight into how
>> the implementation works. It still needs some work and also documents
>> some things that haven't been implemented yet, so I haven't included it
>> in this series yet.
>>
>> [3]: https://github.com/dlech/linux/blob/b4/iio-adc-ti-ads122c14/Documentation/iio/ads112c14.rst
>>
>> Signed-off-by: David Lechner <dlechner@baylibre.com>
>> ---
>> David Lechner (TI) (4):
>> dt-bindings: iio: adc: add ti,ads122c14
>> iio: adc: add ti-ads112c14 driver
>> iio: adc: ti-ads112c14: implement gain on internal short SYS_MON channel
>> iio: adc: ti-ads112c14: add measurement channel support
>>
>> .../devicetree/bindings/iio/adc/ti,ads112c14.yaml | 224 +++++
>> MAINTAINERS | 8 +
>> drivers/iio/adc/Kconfig | 12 +
>> drivers/iio/adc/Makefile | 1 +
>> drivers/iio/adc/ti-ads112c14.c | 1053 ++++++++++++++++++++
>> include/dt-bindings/iio/adc/ti,ads112c14.h | 11 +
>> 6 files changed, 1309 insertions(+)
>> ---
>> base-commit: ec039126b7fac4e3af35ebccaa7c6f9b6875ba81
>> change-id: 20260514-iio-adc-ti-ads122c14-d0b92479334e
>>
>> Best regards,
>> --
>> David Lechner (TI) <dlechner@baylibre.com>
>
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 0/4] iio: adc: new ti-ads112c14 driver
2026-06-16 15:21 ` David Lechner
@ 2026-06-16 17:26 ` Kurt Borja
2026-06-16 18:16 ` David Lechner
0 siblings, 1 reply; 20+ messages in thread
From: Kurt Borja @ 2026-06-16 17:26 UTC (permalink / raw)
To: David Lechner, Kurt Borja, Jonathan Cameron, Nuno Sá,
Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Nguyen Minh Tien, linux-iio, devicetree, linux-kernel
On Tue Jun 16, 2026 at 10:21 AM -05, David Lechner wrote:
> On 6/15/26 7:18 PM, Kurt Borja wrote:
>> Hi David,
>>
>> On Mon Jun 15, 2026 at 4:59 PM -05, David Lechner (TI) wrote:
>>> This adds support for TI ADS112C14 and ADS122C14 ADC chips.
>>>
>>> The closest thing we've seen to this in the kernel already is ads124s08.
>>> However, that has a completely different register map and the DT
>>> bindings are incomplete and the driver is extremely basic. So I've just
>>> started from scratch here.
>>>
>>> We've also had a similar submission recently for ADS1220 [1]. That chip
>>> is in a similar situation to ads124s08 in that it has a different
>>> register map (but the submitted DT bindings are better than the ones for
>>> ads124s08, even if still a bit incomplete). And literally as I was
>>> writing the previous sentence, another series [2] was sent for yet
>>> another similar family of chips (ADS1262). That one is even more complex
>>> in the feature set than the ones I am working on. I was going to polish
>>> up the driver a bit more before submitting it, but now it seems more
>>> urgent to coordinate with the other two series to align on how we would
>>> like to handle all of these.
>>>
>>> [1]: https://lore.kernel.org/linux-iio/20260610151342.44274-1-zizuzacker@gmail.com/
>>> [2]: https://lore.kernel.org/linux-iio/20260612-ads126x-v1-0-894c788d03ed@gmail.com/
>>>
>>> All of these chips have in common that they are designed for use with
>>> RTDs and thermocouples and so they look very similar to each other in
>>> terms of wiring and feature set, even if the register maps are
>>> different. They are in the gray area where we could either keep them
>>> separate because they are just different enough, or we could do like
>>> we've done before with ad_sigma_delta and have a bit of an abstraction
>>> layer for the register differences and otherwise try to share as much
>>> code as possible. Normally, I would lean towards keeping them separate,
>>> but in this case, I'm considering trying to share code because the
>>> devicetree bindings for the inputs is complex and is going to be mostly
>>> the same across all of these chips.
>>
>> The channel configuration is indeed very similar for the three chips.
>> All three have IDAC, BOC and VREF configurations.
>
> Hmm... I forgot to include the burnout current in the DT bindings. Following
> the channel = "conditions for measurement" pattern that I have set out here
> I guess that would mean that we would need to have the same inputs twice
> when using the burnout. One "channel" would be the one used to do a "precision"
> measurement and the other would be the one to do open/short circuit detection.
>
>
> i2c {
> #address-cells = <1>;
> #size-cells = <0>;
>
> adc@40 {
> compatible = "ti,ads112c14";
> reg = <0x40>;
>
> avdd-supply = <&avdd>;
> dvdd-supply = <&dvdd>;
>
> refp-supply = <&avdd>;
>
> #address-cells = <1>;
> #size-cells = <0>;
>
> channel@0 {
> reg = <0>;
> diff-channels = <1>, <2>;
> excitation-channels = <0>, <3>;
> excitation-current-microamp = <500>;
> current-chopping;
> ti,vref-source = <ADS112C14_VREF_SOURCE_EXTERNAL>;
> label = "rtd-precision";
> };
>
> channel@1 {
> reg = <0>;
> diff-channels = <1>, <2>;
> excitation-channels = <0>, <3>;
> excitation-current-microamp = <500>;
> burnout-current-nanoamp = <1000>;
> ti,vref-source = <ADS112C14_VREF_SOURCE_EXTERNAL>;
> label = "rtd-diagnostic";
> };
This would mean we wouldn't be able to use iio_chan_spec .channel and
.channel2 to describe inputs because of duplicate sysfs attributes, no?
> };
> };
>
>>
>>>
>>> If we decide to go the route of sharing code, we could still merge this
>>> series as-is and then do the refactoring to add the abstraction layer in
>>> a follow-up series that also adds support for the first of the other
>>> chips.
>>
>> Do you have a proposal of how such an abstraction would look like? I do
>> like the idea of abstracting the firmware parsing, scales and shared
>> calculations.
>
> As mentioned, it would look a bit like the ad_sigma_delta ADC driver.
> Basically, each chip (family with same registers) would still have it's
> own driver that include the chip-specific const info structs. In this
> case, these would also include function pointers for all of the functions
> that need to access registers. And there will be lots of supports_... bool
> flags for the shared code to use.
>
>>
>>>
>>> This series includes just basic support for reading single measurements
>>> from the ADC and gain selection via the scale attribute. I plan to
>>> follow this up with additional series to add support for buffered reads,
>>> filtering/oversampling configuration, event support, gpio controller
>>> support and perhaps a few other things that are slipping my mind right
>>> now.
>>>
>>> The most interesting part about this (that I alluded to above) is the
>>> way channels are handled. These are multipling ADCs with differential
>>> and single-ended inputs. But what sets them apart from other similar
>>> chips is that since they are designed for use with RTDs, there can also
>>> be a current output required to excite the RTD and this current output
>>> might be different for different channels. So the way I conceptualized
>>> the channels is that the devicetree specifies the conditions needed
>>> to take a particular measurement rather than being purely a physical
>>> channel.
>>>
>>> This makes things more flexible, but does make the driver a bit more
>>> complex. For example, knowing when the current output needs to be
>>> enabled or disabled. For now, I have chosen a lazy-enable where they
>>> are not turned on until the first measurement is taken that requires
>>> them, but then they stay on until another measurement is taken that
>>> doesn't require them. This can lead to some oddness with the diagnostic
>>> channels that may be measuring something that indirectly requires the
>>> current output (i.e. the external reference voltage when it is connected
>>> to a resistor rather than a power supply). This means you need to take
>>> a measurement that requires the current output to be enabled before the
>>> diagnostic channels will give accurate readings.
>>
>> This is the same approach I took around the BOC, it feels kinda hacky
>> but it makes sense. Just an idea I thought about just now: What if we
>> have an additional write-only "_enable" sysfs attribute for these
>> channels?
>
> I would not want to make a write-only attribute, we always want to be
> able to read back what the current state is.
Yeah, I don't know why I said WO. Reading would be fine too.
>
> Do you mean an _enable for just the BOC? I think I would do it like I
> suggested above instead.
No, no just the BOC. The BOC, IDAC and rest of side effects. Thinking
about it some more, it would be a bit redundant but clearer if proper
documentation is provided.
>
>>
>> The approach I took for the IDAC was to have a single configuration that
>> it's enabled for all channels. This makes some sense in my device when
>> thinking about optimal software sequencial reads, because of the
>> register layout, but I also see the value in having per-channel IDAC
>> configuration. I think I will take your approach, so we have the same
>> channel configuration around this.
>
> There are only two IDACs but on chips with 8 AIN pins we could have two
> RTDs wired up and if both require 2 IDACs, then we can't have a fixed
> setting for the IDACs. This is why I made it per-channel.
I'll make it per-channel too.
>
>>
>> Have you thought about how to implement the BOC? In the ADS1262 the
>> feature can be found "Sensor Bias". What I did was add per channel DT
>> properties for this too.
>
> See above.
>
>>
>> Another question. When you implement power management in the future,
>> will you enable autosuspend? IDAC currents will be lost if autosuspend
>> is enabled. Is this acceptable? In my case I did enable autosuspend, but
>> I have some doubts about this.
>
> I don't like to implement power management unless I have an application
> that actually requires it. Otherwise, I consider it premature optimization.
> It is difficult to know if we would be implementing it in a way that is
> actually useful for a real-world use case. These chips seem like they
> would mostly be used in an industrial setting, not in battery powered
> applications, so I'm not sure anyone is going to worry about saving a few
> milliamps.
Makes sense. I can probably drop the autosuspend in that case.
>
>>
>>>
>>> I have also pushed a branch to [3] that contains the start of some
>>> documentation for this driver that can give some more insight into how
>>> the implementation works. It still needs some work and also documents
>>> some things that haven't been implemented yet, so I haven't included it
>>> in this series yet.
>>>
>>> [3]: https://github.com/dlech/linux/blob/b4/iio-adc-ti-ads122c14/Documentation/iio/ads112c14.rst
>>>
>>> Signed-off-by: David Lechner <dlechner@baylibre.com>
>>> ---
>>> David Lechner (TI) (4):
>>> dt-bindings: iio: adc: add ti,ads122c14
>>> iio: adc: add ti-ads112c14 driver
>>> iio: adc: ti-ads112c14: implement gain on internal short SYS_MON channel
>>> iio: adc: ti-ads112c14: add measurement channel support
>>>
>>> .../devicetree/bindings/iio/adc/ti,ads112c14.yaml | 224 +++++
>>> MAINTAINERS | 8 +
>>> drivers/iio/adc/Kconfig | 12 +
>>> drivers/iio/adc/Makefile | 1 +
>>> drivers/iio/adc/ti-ads112c14.c | 1053 ++++++++++++++++++++
>>> include/dt-bindings/iio/adc/ti,ads112c14.h | 11 +
>>> 6 files changed, 1309 insertions(+)
>>> ---
>>> base-commit: ec039126b7fac4e3af35ebccaa7c6f9b6875ba81
>>> change-id: 20260514-iio-adc-ti-ads122c14-d0b92479334e
>>>
>>> Best regards,
>>> --
>>> David Lechner (TI) <dlechner@baylibre.com>
>>
--
Thanks,
~ Kurt
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 0/4] iio: adc: new ti-ads112c14 driver
2026-06-16 17:26 ` Kurt Borja
@ 2026-06-16 18:16 ` David Lechner
0 siblings, 0 replies; 20+ messages in thread
From: David Lechner @ 2026-06-16 18:16 UTC (permalink / raw)
To: Kurt Borja, Jonathan Cameron, Nuno Sá, Andy Shevchenko,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Nguyen Minh Tien, linux-iio, devicetree, linux-kernel
On 6/16/26 12:26 PM, Kurt Borja wrote:
> On Tue Jun 16, 2026 at 10:21 AM -05, David Lechner wrote:
>> On 6/15/26 7:18 PM, Kurt Borja wrote:
>>> On Mon Jun 15, 2026 at 4:59 PM -05, David Lechner (TI) wrote:
...
>>>> All of these chips have in common that they are designed for use with
>>>> RTDs and thermocouples and so they look very similar to each other in
>>>> terms of wiring and feature set, even if the register maps are
>>>> different. They are in the gray area where we could either keep them
>>>> separate because they are just different enough, or we could do like
>>>> we've done before with ad_sigma_delta and have a bit of an abstraction
>>>> layer for the register differences and otherwise try to share as much
>>>> code as possible. Normally, I would lean towards keeping them separate,
>>>> but in this case, I'm considering trying to share code because the
>>>> devicetree bindings for the inputs is complex and is going to be mostly
>>>> the same across all of these chips.
>>>
>>> The channel configuration is indeed very similar for the three chips.
>>> All three have IDAC, BOC and VREF configurations.
>>
>> Hmm... I forgot to include the burnout current in the DT bindings. Following
>> the channel = "conditions for measurement" pattern that I have set out here
>> I guess that would mean that we would need to have the same inputs twice
>> when using the burnout. One "channel" would be the one used to do a "precision"
>> measurement and the other would be the one to do open/short circuit detection.
>>
>>
>> i2c {
>> #address-cells = <1>;
>> #size-cells = <0>;
>>
>> adc@40 {
>> compatible = "ti,ads112c14";
>> reg = <0x40>;
>>
>> avdd-supply = <&avdd>;
>> dvdd-supply = <&dvdd>;
>>
>> refp-supply = <&avdd>;
>>
>> #address-cells = <1>;
>> #size-cells = <0>;
>>
>> channel@0 {
>> reg = <0>;
>> diff-channels = <1>, <2>;
>> excitation-channels = <0>, <3>;
>> excitation-current-microamp = <500>;
>> current-chopping;
>> ti,vref-source = <ADS112C14_VREF_SOURCE_EXTERNAL>;
>> label = "rtd-precision";
>> };
>>
>> channel@1 {
>> reg = <0>;
>> diff-channels = <1>, <2>;
>> excitation-channels = <0>, <3>;
>> excitation-current-microamp = <500>;
>> burnout-current-nanoamp = <1000>;
>> ti,vref-source = <ADS112C14_VREF_SOURCE_EXTERNAL>;
>> label = "rtd-diagnostic";
>> };
>
> This would mean we wouldn't be able to use iio_chan_spec .channel and
> .channel2 to describe inputs because of duplicate sysfs attributes, no?
>
Yes, that is a bit unfortunate. At least there the labels to tell them
apart. I guess we would just need to use consecutive channel and channel2
when dynamically allocating the channels to avoid conflict.
>>>> This makes things more flexible, but does make the driver a bit more
>>>> complex. For example, knowing when the current output needs to be
>>>> enabled or disabled. For now, I have chosen a lazy-enable where they
>>>> are not turned on until the first measurement is taken that requires
>>>> them, but then they stay on until another measurement is taken that
>>>> doesn't require them. This can lead to some oddness with the diagnostic
>>>> channels that may be measuring something that indirectly requires the
>>>> current output (i.e. the external reference voltage when it is connected
>>>> to a resistor rather than a power supply). This means you need to take
>>>> a measurement that requires the current output to be enabled before the
>>>> diagnostic channels will give accurate readings.
>>>
>>> This is the same approach I took around the BOC, it feels kinda hacky
>>> but it makes sense. Just an idea I thought about just now: What if we
>>> have an additional write-only "_enable" sysfs attribute for these
>>> channels?
>>
>> I would not want to make a write-only attribute, we always want to be
>> able to read back what the current state is.
>
> Yeah, I don't know why I said WO. Reading would be fine too.
>
>>
>> Do you mean an _enable for just the BOC? I think I would do it like I
>> suggested above instead.
>
> No, no just the BOC. The BOC, IDAC and rest of side effects. Thinking
> about it some more, it would be a bit redundant but clearer if proper
> documentation is provided.
>
I would be interested to see what Jonathan has to say about this too.
Generally, his advice has been to avoid attributes that power things
on and off if we can help it.
^ permalink raw reply [flat|nested] 20+ messages in thread