* [PATCH RFC v2 0/3] dt-bindings: iio: adc: Add reference, excitation and burn-out properties
From: Kurt Borja @ 2026-06-22 19:30 UTC (permalink / raw)
To: Jonathan Cameron, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
David Lechner
Cc: Nuno Sá, Andy Shevchenko, linux-iio, devicetree,
linux-kernel, Kurt Borja
Hi all,
After submitting a patch series adding support for TI ADS126X ADCs [1],
I was made aware by David [2] that at least two more chip families,
ads1220 [3] and ads1x2c14, share very similar features (though these
chips are not really compatible between them). After that, I found one
more chip with the same features which is already upstream, the
AD4170-4.
As David explained in [2], these chips are intended to be used with
RTDs, thermocouples or other resistive sensors so they share the
following per-channel features:
- Configurable reference selection
- Burn-out Current Sources (BOCS) for diagnostic purpuses
- Excitation current sources (usually called IDACs TI) for sensor
current biasing
Given that these three features are present in all four devices and
three of these drivers are still under review, my proposal is to have
these features be described in adc.yaml and have this series merged
before the three others [1] [2] [3].
This series is sent as RFC because I still don't have much experience
with dt-bindings and I don't know if this approach or the properties are
general enough to be described like this.
No dependencies between properties were provided because not all devices
may be able to configure each one of them.
[1] https://lore.kernel.org/linux-iio/20260612-ads126x-v1-0-894c788d03ed@gmail.com/
[2] https://lore.kernel.org/linux-iio/20260615-iio-adc-ti-ads122c14-v1-0-e6bdadf7cb2b@baylibre.com/
[3] https://lore.kernel.org/linux-iio/20260610151342.44274-1-zizuzacker@gmail.com/
Signed-off-by: Kurt Borja <kuurtb@gmail.com>
---
v2:
- reference-source is now a string-array and now presents a couple of
quick examples
- excitation-* properties now do not enforce arbitrary limits
- Dropped burn-out-current-polarity because it was not general enough
- I kept burn-out-current-microamp because the discussion around it is
still ongoing
v1: https://patch.msgid.link/20260618-new-channel-props-v1-0-963c1b5cf40a@gmail.com
---
Kurt Borja (3):
dt-bindings: iio: adc: Add reference-source property
dt-bindings: iio: adc: Add excitation current sources properties
dt-bindings: iio: adc: Add burn-out current properties
Documentation/devicetree/bindings/iio/adc/adc.yaml | 38 ++++++++++++++++++++++
1 file changed, 38 insertions(+)
---
base-commit: a50909aa46dec46de3c73235fc15a7d6f763d996
change-id: 20260618-new-channel-props-4fbd52020da2
--
Thanks,
~ Kurt
^ permalink raw reply
* [PATCH RFC v2 1/3] dt-bindings: iio: adc: Add reference-source property
From: Kurt Borja @ 2026-06-22 19:30 UTC (permalink / raw)
To: Jonathan Cameron, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
David Lechner
Cc: Nuno Sá, Andy Shevchenko, linux-iio, devicetree,
linux-kernel, Kurt Borja
In-Reply-To: <20260622-new-channel-props-v2-0-aafd5369f253@gmail.com>
Some ADCs have configurable voltage reference sources for each channel.
Signed-off-by: Kurt Borja <kuurtb@gmail.com>
---
Documentation/devicetree/bindings/iio/adc/adc.yaml | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/Documentation/devicetree/bindings/iio/adc/adc.yaml b/Documentation/devicetree/bindings/iio/adc/adc.yaml
index b9bc02b5b07a4c7..fdad6b8276c934c 100644
--- a/Documentation/devicetree/bindings/iio/adc/adc.yaml
+++ b/Documentation/devicetree/bindings/iio/adc/adc.yaml
@@ -73,6 +73,19 @@ properties:
device design and can interact with other characteristics such as
settling time.
+ reference-source:
+ $ref: /schemas/types.yaml#/definitions/string-array
+ maxItems: 2
+ minItems: 1
+ description:
+ Indicates the voltage reference source or sources for this channel. Some
+ ADCs usually allow choosing between internal reference sources or a pair
+ of external pins.
+
+ If a single value is provided, it represents a single voltage reference
+ source. If two values are provided, the first one corresponds to the
+ positive source and the second to the negative source.
+
anyOf:
- oneOf:
- required:
--
2.54.0
^ permalink raw reply related
* [PATCH RFC v2 2/3] dt-bindings: iio: adc: Add excitation current sources properties
From: Kurt Borja @ 2026-06-22 19:30 UTC (permalink / raw)
To: Jonathan Cameron, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
David Lechner
Cc: Nuno Sá, Andy Shevchenko, linux-iio, devicetree,
linux-kernel, Kurt Borja
In-Reply-To: <20260622-new-channel-props-v2-0-aafd5369f253@gmail.com>
Some ADCs incorporate current sources that provide excitation current to
resistive temperature devices (RTDs), thermistors, diodes and other
resistive sensors that require constant current biasing.
Signed-off-by: Kurt Borja <kuurtb@gmail.com>
---
Documentation/devicetree/bindings/iio/adc/adc.yaml | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/Documentation/devicetree/bindings/iio/adc/adc.yaml b/Documentation/devicetree/bindings/iio/adc/adc.yaml
index fdad6b8276c934c..160a8cfa9842a86 100644
--- a/Documentation/devicetree/bindings/iio/adc/adc.yaml
+++ b/Documentation/devicetree/bindings/iio/adc/adc.yaml
@@ -86,6 +86,25 @@ properties:
source. If two values are provided, the first one corresponds to the
positive source and the second to the negative source.
+ excitation-channels:
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+ description:
+ Excitation current sources provide current to resistive temperature
+ devices (RTDs), thermistors, diodes and other resistive sensors that
+ require constant current biasing.
+
+ This array describes the mux configuration of the excitation current
+ sources.
+
+ excitation-current-microamp:
+ description:
+ Excitation current sources provide current to resistive temperature
+ devices (RTDs), thermistors, diodes and other resistive sensors that
+ require constant current biasing.
+
+ This array describes the current configuration of the excitation current
+ sources or the single matched current for all sources.
+
anyOf:
- oneOf:
- required:
--
2.54.0
^ permalink raw reply related
* [PATCH RFC v2 3/3] dt-bindings: iio: adc: Add burn-out current properties
From: Kurt Borja @ 2026-06-22 19:30 UTC (permalink / raw)
To: Jonathan Cameron, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
David Lechner
Cc: Nuno Sá, Andy Shevchenko, linux-iio, devicetree,
linux-kernel, Kurt Borja
In-Reply-To: <20260622-new-channel-props-v2-0-aafd5369f253@gmail.com>
Some ADCs incorporate burn-out current sources that provide current to
the channel's input pins for open-circuit or short-circuit detection.
Signed-off-by: Kurt Borja <kuurtb@gmail.com>
---
Documentation/devicetree/bindings/iio/adc/adc.yaml | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/Documentation/devicetree/bindings/iio/adc/adc.yaml b/Documentation/devicetree/bindings/iio/adc/adc.yaml
index 160a8cfa9842a86..9240f569d4ab7af 100644
--- a/Documentation/devicetree/bindings/iio/adc/adc.yaml
+++ b/Documentation/devicetree/bindings/iio/adc/adc.yaml
@@ -105,6 +105,12 @@ properties:
This array describes the current configuration of the excitation current
sources or the single matched current for all sources.
+ burn-out-current-microamp:
+ maxItems: 1
+ description:
+ Burn-out current sources provide current to the channel's input pins for
+ open-circuit or short-circuit detection.
+
anyOf:
- oneOf:
- required:
--
2.54.0
^ permalink raw reply related
* Re: [PATCH RFC v2 0/3] dt-bindings: iio: adc: Add reference, excitation and burn-out properties
From: David Lechner @ 2026-06-22 19:38 UTC (permalink / raw)
To: Kurt Borja, Jonathan Cameron, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: Nuno Sá, Andy Shevchenko, linux-iio, devicetree,
linux-kernel
In-Reply-To: <20260622-new-channel-props-v2-0-aafd5369f253@gmail.com>
On 6/22/26 2:30 PM, Kurt Borja wrote:
> Hi all,
>
> After submitting a patch series adding support for TI ADS126X ADCs [1],
> I was made aware by David [2] that at least two more chip families,
> ads1220 [3] and ads1x2c14, share very similar features (though these
> chips are not really compatible between them). After that, I found one
> more chip with the same features which is already upstream, the
> AD4170-4.
>
> As David explained in [2], these chips are intended to be used with
> RTDs, thermocouples or other resistive sensors so they share the
> following per-channel features:
>
> - Configurable reference selection
> - Burn-out Current Sources (BOCS) for diagnostic purpuses
> - Excitation current sources (usually called IDACs TI) for sensor
> current biasing
>
> Given that these three features are present in all four devices and
> three of these drivers are still under review, my proposal is to have
> these features be described in adc.yaml and have this series merged
> before the three others [1] [2] [3].
>
> This series is sent as RFC because I still don't have much experience
> with dt-bindings and I don't know if this approach or the properties are
> general enough to be described like this.
It will probably be easier if I just include these patches when I do
v2 of my series (if you don't mind me tweaking them a bit).
>
> No dependencies between properties were provided because not all devices
> may be able to configure each one of them.
>
> [1] https://lore.kernel.org/linux-iio/20260612-ads126x-v1-0-894c788d03ed@gmail.com/
> [2] https://lore.kernel.org/linux-iio/20260615-iio-adc-ti-ads122c14-v1-0-e6bdadf7cb2b@baylibre.com/
> [3] https://lore.kernel.org/linux-iio/20260610151342.44274-1-zizuzacker@gmail.com/
>
> Signed-off-by: Kurt Borja <kuurtb@gmail.com>
> ---
> v2:
> - reference-source is now a string-array and now presents a couple of
> quick examples
> - excitation-* properties now do not enforce arbitrary limits
> - Dropped burn-out-current-polarity because it was not general enough
> - I kept burn-out-current-microamp because the discussion around it is
> still ongoing
>
> v1: https://patch.msgid.link/20260618-new-channel-props-v1-0-963c1b5cf40a@gmail.com
>
> ---
> Kurt Borja (3):
> dt-bindings: iio: adc: Add reference-source property
> dt-bindings: iio: adc: Add excitation current sources properties
> dt-bindings: iio: adc: Add burn-out current properties
>
> Documentation/devicetree/bindings/iio/adc/adc.yaml | 38 ++++++++++++++++++++++
> 1 file changed, 38 insertions(+)
> ---
> base-commit: a50909aa46dec46de3c73235fc15a7d6f763d996
> change-id: 20260618-new-channel-props-4fbd52020da2
>
^ permalink raw reply
* Re: [PATCH RFC v2 1/3] dt-bindings: iio: adc: Add reference-source property
From: David Lechner @ 2026-06-22 19:40 UTC (permalink / raw)
To: Kurt Borja, Jonathan Cameron, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: Nuno Sá, Andy Shevchenko, linux-iio, devicetree,
linux-kernel
In-Reply-To: <20260622-new-channel-props-v2-1-aafd5369f253@gmail.com>
On 6/22/26 2:30 PM, Kurt Borja wrote:
> Some ADCs have configurable voltage reference sources for each channel.
>
> Signed-off-by: Kurt Borja <kuurtb@gmail.com>
> ---
> Documentation/devicetree/bindings/iio/adc/adc.yaml | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/iio/adc/adc.yaml b/Documentation/devicetree/bindings/iio/adc/adc.yaml
> index b9bc02b5b07a4c7..fdad6b8276c934c 100644
> --- a/Documentation/devicetree/bindings/iio/adc/adc.yaml
> +++ b/Documentation/devicetree/bindings/iio/adc/adc.yaml
> @@ -73,6 +73,19 @@ properties:
> device design and can interact with other characteristics such as
> settling time.
>
> + reference-source:
Since this is an array, the name should be `reference-sources`.
> + $ref: /schemas/types.yaml#/definitions/string-array
> + maxItems: 2
> + minItems: 1
Maybe minItems here is OK, but I don't think we should put maxItems here.
This way, it stays more flexible for other use cases.
> + description:
> + Indicates the voltage reference source or sources for this channel. Some
> + ADCs usually allow choosing between internal reference sources or a pair
> + of external pins.
> +
> + If a single value is provided, it represents a single voltage reference
> + source. If two values are provided, the first one corresponds to the
> + positive source and the second to the negative source.
> +
> anyOf:
> - oneOf:
> - required:
>
^ permalink raw reply
* Re: [PATCH RFC v2 2/3] dt-bindings: iio: adc: Add excitation current sources properties
From: David Lechner @ 2026-06-22 19:42 UTC (permalink / raw)
To: Kurt Borja, Jonathan Cameron, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: Nuno Sá, Andy Shevchenko, linux-iio, devicetree,
linux-kernel
In-Reply-To: <20260622-new-channel-props-v2-2-aafd5369f253@gmail.com>
On 6/22/26 2:30 PM, Kurt Borja wrote:
> Some ADCs incorporate current sources that provide excitation current to
> resistive temperature devices (RTDs), thermistors, diodes and other
> resistive sensors that require constant current biasing.
>
> Signed-off-by: Kurt Borja <kuurtb@gmail.com>
> ---
> Documentation/devicetree/bindings/iio/adc/adc.yaml | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/iio/adc/adc.yaml b/Documentation/devicetree/bindings/iio/adc/adc.yaml
> index fdad6b8276c934c..160a8cfa9842a86 100644
> --- a/Documentation/devicetree/bindings/iio/adc/adc.yaml
> +++ b/Documentation/devicetree/bindings/iio/adc/adc.yaml
> @@ -86,6 +86,25 @@ properties:
> source. If two values are provided, the first one corresponds to the
> positive source and the second to the negative source.
>
> + excitation-channels:
> + $ref: /schemas/types.yaml#/definitions/uint32-array
> + description:
> + Excitation current sources provide current to resistive temperature
> + devices (RTDs), thermistors, diodes and other resistive sensors that
> + require constant current biasing.
> +
> + This array describes the mux configuration of the excitation current
> + sources.
> +
> + excitation-current-microamp:
As seen in adi,ltc2983.yaml, this actually needs to be nanoamp to be able
to be flexible for all parts.
> + description:
> + Excitation current sources provide current to resistive temperature
> + devices (RTDs), thermistors, diodes and other resistive sensors that
> + require constant current biasing.
> +
> + This array describes the current configuration of the excitation current
> + sources or the single matched current for all sources.
> +
> anyOf:
> - oneOf:
> - required:
>
^ permalink raw reply
* Re: [PATCH RFC v2 3/3] dt-bindings: iio: adc: Add burn-out current properties
From: David Lechner @ 2026-06-22 19:44 UTC (permalink / raw)
To: Kurt Borja, Jonathan Cameron, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: Nuno Sá, Andy Shevchenko, linux-iio, devicetree,
linux-kernel
In-Reply-To: <20260622-new-channel-props-v2-3-aafd5369f253@gmail.com>
On 6/22/26 2:30 PM, Kurt Borja wrote:
> Some ADCs incorporate burn-out current sources that provide current to
> the channel's input pins for open-circuit or short-circuit detection.
>
> Signed-off-by: Kurt Borja <kuurtb@gmail.com>
> ---
> Documentation/devicetree/bindings/iio/adc/adc.yaml | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/iio/adc/adc.yaml b/Documentation/devicetree/bindings/iio/adc/adc.yaml
> index 160a8cfa9842a86..9240f569d4ab7af 100644
> --- a/Documentation/devicetree/bindings/iio/adc/adc.yaml
> +++ b/Documentation/devicetree/bindings/iio/adc/adc.yaml
> @@ -105,6 +105,12 @@ properties:
> This array describes the current configuration of the excitation current
> sources or the single matched current for all sources.
>
> + burn-out-current-microamp:
This also needs to be nanoamp.
> + maxItems: 1
> + description:
> + Burn-out current sources provide current to the channel's input pins for
> + open-circuit or short-circuit detection.
> +
> anyOf:
> - oneOf:
> - required:
>
^ permalink raw reply
* Re: [PATCH v4 01/16] spi: dt-bindings: add spi-max-post-config-frequency property
From: Conor Dooley @ 2026-06-22 19:46 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Santhosh Kumar K, broonie, robh, krzk+dt, conor+dt, miquel.raynal,
richard, vigneshr, pratyush, mwalle, takahiro.kuwano, linux-spi,
devicetree, linux-kernel, linux-mtd, praneeth, u-kumar1, a-dutta
In-Reply-To: <20260622-private-curly-fennec-7e1ad0@quoll>
[-- Attachment #1: Type: text/plain, Size: 2232 bytes --]
On Mon, Jun 22, 2026 at 11:14:32AM +0200, Krzysztof Kozlowski wrote:
> On Thu, Jun 18, 2026 at 01:07:10PM +0530, Santhosh Kumar K wrote:
> > Add spi-max-post-config-frequency, a generic uint32 property for SPI
> > peripherals that support two distinct clock rates: a conservative rate
> > always reachable without controller configuration, and a higher rate
> > reachable only after controller-side configuration such as PHY tuning.
> >
> > When both properties are present, spi-max-frequency gives the
> > conservative pre-configuration rate and spi-max-post-config-frequency
> > gives the higher post-configuration target.
> >
> > Signed-off-by: Santhosh Kumar K <s-k6@ti.com>
> > ---
> > .../devicetree/bindings/spi/spi-peripheral-props.yaml | 6 ++++++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml b/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
> > index 880a9f624566..ece86f65930f 100644
> > --- a/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
> > +++ b/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
> > @@ -45,6 +45,12 @@ properties:
> > description:
> > Maximum SPI clocking speed of the device in Hz.
> >
> > + spi-max-post-config-frequency:
>
> -hz
Ah d'oh. I was hung up on matching the existing property that I didn't
include -hz in my original suggestion, sorry!
> https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/property-units.yaml
>
> and you need maxItems: 1.
>
> Now, please take time and think if this should not be an array instead
> (maxItems: ...) to cover other possible cases, e.g. different tuning
> levels? IOW, having single spi-max-frequency turned out to be
> insufficient. You address that insufficiency with one more frequency,
> but what if this is going to be insufficient next month as well?
>
> I don't know, answer is rather for domain experts.
>
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + description:
> > + Maximum SPI clock frequency in Hz achievable post controller-side
> > + configuration.
>
> Best regards,
> Krzysztof
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH RFC v2 0/3] dt-bindings: iio: adc: Add reference, excitation and burn-out properties
From: Kurt Borja @ 2026-06-22 19:58 UTC (permalink / raw)
To: David Lechner, Kurt Borja, Jonathan Cameron, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: Nuno Sá, Andy Shevchenko, linux-iio, devicetree,
linux-kernel
In-Reply-To: <42e544b8-f2da-450b-92bb-99c41f1c72fe@baylibre.com>
On Mon Jun 22, 2026 at 2:38 PM -05, David Lechner wrote:
> On 6/22/26 2:30 PM, Kurt Borja wrote:
>> Hi all,
>>
>> After submitting a patch series adding support for TI ADS126X ADCs [1],
>> I was made aware by David [2] that at least two more chip families,
>> ads1220 [3] and ads1x2c14, share very similar features (though these
>> chips are not really compatible between them). After that, I found one
>> more chip with the same features which is already upstream, the
>> AD4170-4.
>>
>> As David explained in [2], these chips are intended to be used with
>> RTDs, thermocouples or other resistive sensors so they share the
>> following per-channel features:
>>
>> - Configurable reference selection
>> - Burn-out Current Sources (BOCS) for diagnostic purpuses
>> - Excitation current sources (usually called IDACs TI) for sensor
>> current biasing
>>
>> Given that these three features are present in all four devices and
>> three of these drivers are still under review, my proposal is to have
>> these features be described in adc.yaml and have this series merged
>> before the three others [1] [2] [3].
>>
>> This series is sent as RFC because I still don't have much experience
>> with dt-bindings and I don't know if this approach or the properties are
>> general enough to be described like this.
>
> It will probably be easier if I just include these patches when I do
> v2 of my series (if you don't mind me tweaking them a bit).
Sure, that's fine by me. I'll add a dependency to your series with b4.
Want me to send one more version addressing your comments before you
take it in?
--
Thanks,
~ Kurt
^ permalink raw reply
* Re: [PATCH RFC v2 0/3] dt-bindings: iio: adc: Add reference, excitation and burn-out properties
From: David Lechner @ 2026-06-22 20:01 UTC (permalink / raw)
To: Kurt Borja, Jonathan Cameron, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: Nuno Sá, Andy Shevchenko, linux-iio, devicetree,
linux-kernel
In-Reply-To: <DJFUE81GZEEG.35TP4FFBVE63B@gmail.com>
On 6/22/26 2:58 PM, Kurt Borja wrote:
> On Mon Jun 22, 2026 at 2:38 PM -05, David Lechner wrote:
>> On 6/22/26 2:30 PM, Kurt Borja wrote:
>>> Hi all,
>>>
>>> After submitting a patch series adding support for TI ADS126X ADCs [1],
>>> I was made aware by David [2] that at least two more chip families,
>>> ads1220 [3] and ads1x2c14, share very similar features (though these
>>> chips are not really compatible between them). After that, I found one
>>> more chip with the same features which is already upstream, the
>>> AD4170-4.
>>>
>>> As David explained in [2], these chips are intended to be used with
>>> RTDs, thermocouples or other resistive sensors so they share the
>>> following per-channel features:
>>>
>>> - Configurable reference selection
>>> - Burn-out Current Sources (BOCS) for diagnostic purpuses
>>> - Excitation current sources (usually called IDACs TI) for sensor
>>> current biasing
>>>
>>> Given that these three features are present in all four devices and
>>> three of these drivers are still under review, my proposal is to have
>>> these features be described in adc.yaml and have this series merged
>>> before the three others [1] [2] [3].
>>>
>>> This series is sent as RFC because I still don't have much experience
>>> with dt-bindings and I don't know if this approach or the properties are
>>> general enough to be described like this.
>>
>> It will probably be easier if I just include these patches when I do
>> v2 of my series (if you don't mind me tweaking them a bit).
>
> Sure, that's fine by me. I'll add a dependency to your series with b4.
>
> Want me to send one more version addressing your comments before you
> take it in?
>
No need. I don't mind fixing it up.
^ permalink raw reply
* Re: [PATCH RFC v2 0/3] dt-bindings: iio: adc: Add reference, excitation and burn-out properties
From: Kurt Borja @ 2026-06-22 20:05 UTC (permalink / raw)
To: David Lechner, Kurt Borja, Jonathan Cameron, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: Nuno Sá, Andy Shevchenko, linux-iio, devicetree,
linux-kernel
In-Reply-To: <f7aae511-0e23-41cf-a5b0-27782caac30b@baylibre.com>
On Mon Jun 22, 2026 at 3:01 PM -05, David Lechner wrote:
> On 6/22/26 2:58 PM, Kurt Borja wrote:
>> On Mon Jun 22, 2026 at 2:38 PM -05, David Lechner wrote:
>>> On 6/22/26 2:30 PM, Kurt Borja wrote:
>>>> Hi all,
>>>>
>>>> After submitting a patch series adding support for TI ADS126X ADCs [1],
>>>> I was made aware by David [2] that at least two more chip families,
>>>> ads1220 [3] and ads1x2c14, share very similar features (though these
>>>> chips are not really compatible between them). After that, I found one
>>>> more chip with the same features which is already upstream, the
>>>> AD4170-4.
>>>>
>>>> As David explained in [2], these chips are intended to be used with
>>>> RTDs, thermocouples or other resistive sensors so they share the
>>>> following per-channel features:
>>>>
>>>> - Configurable reference selection
>>>> - Burn-out Current Sources (BOCS) for diagnostic purpuses
>>>> - Excitation current sources (usually called IDACs TI) for sensor
>>>> current biasing
>>>>
>>>> Given that these three features are present in all four devices and
>>>> three of these drivers are still under review, my proposal is to have
>>>> these features be described in adc.yaml and have this series merged
>>>> before the three others [1] [2] [3].
>>>>
>>>> This series is sent as RFC because I still don't have much experience
>>>> with dt-bindings and I don't know if this approach or the properties are
>>>> general enough to be described like this.
>>>
>>> It will probably be easier if I just include these patches when I do
>>> v2 of my series (if you don't mind me tweaking them a bit).
>>
>> Sure, that's fine by me. I'll add a dependency to your series with b4.
>>
>> Want me to send one more version addressing your comments before you
>> take it in?
>>
>
> No need. I don't mind fixing it up.
Thanks :)
--
Thanks,
~ Kurt
^ permalink raw reply
* Re: [PATCH 2/2] dt-bindings: Drop incorrect usage of double '::'
From: Sebastian Reichel @ 2026-06-22 20:37 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Peter Griffin, Alim Akhtar, Michael Turquette,
Stephen Boyd, Brian Masney, Sylwester Nawrocki, Chanwoo Choi,
Sam Protsenko, Rob Clark, Dmitry Baryshkov, Abhinav Kumar,
Jessica Zhang, Sean Paul, Marijn Suijten, David Airlie,
Simona Vetter, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Inki Dae, Seung-Woo Kim, Kyungmin Park,
Andi Shyti, Georgi Djakov, Lee Jones, Pavel Machek, Hans Verkuil,
Mauro Carvalho Chehab, Ulf Hansson, Peter Rosin, Vinod Koul,
Neil Armstrong, Linus Walleij, Geert Uytterhoeven, Magnus Damm,
Javier Martinez Canillas, Liam Girdwood, Mark Brown,
Greg Kroah-Hartman, Jiri Slaby, Srinivas Kandagatla,
Bartlomiej Zolnierkiewicz, Rafael J. Wysocki, Daniel Lezcano,
Zhang Rui, Lukasz Luba, Jonathan Marek, Taniya Das, Robert Marko,
Christian Marangi, Stephan Gerhold, Adam Skladowski,
Sireesh Kodali, Barnabas Czeman, Imran Shaik,
Sricharan Ramabadhran, Anusha Rao, Luo Jie, Tomasz Figa,
Chanho Park, Sunyeal Hong, Shin Son, Krishna Manikandan,
Jacek Anaszewski, Jaehoon Chung, Marek Szyprowski, Alina Yu,
Andy Gross, Niklas Söderlund, Wesley Cheng, linux-arm-msm,
devicetree, linux-kernel, linux-arm-kernel, linux-samsung-soc,
linux-clk, dri-devel, freedreno, linux-i2c, linux-pm, linux-leds,
linux-media, linux-mmc, linux-phy, linux-gpio, linux-renesas-soc,
linux-serial, linux-sound, linux-usb
In-Reply-To: <20260622101606.485961-4-krzysztof.kozlowski@oss.qualcomm.com>
[-- Attachment #1: Type: text/plain, Size: 789 bytes --]
Hi,
On Mon, Jun 22, 2026 at 12:16:08PM +0200, Krzysztof Kozlowski wrote:
> There is no use of double colon '::' in YAML. OTOH, the literal style
> block, e.g. using '|' treats all characters as content [1] therefore
> single use of ':' in descriptions is perfectly fine, whenever '|' is
> used.
>
> Cleanup existing code, so the confusing style won't be re-used in new
> contributions.
>
> Link: https://yaml.org/spec/1.2.2/#literal-style [1]
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
>
> ---
>
> Intention for this patch is to go via Rob's tree.
> ---
[...]
> .../bindings/power/reset/restart-handler.yaml | 8 ++++----
Acked-by: Sebastian Reichel <sebastian.reichel@collabora.com>
[...]
Greetings,
-- Sebastian
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 3/4] memory: brcmstb_dpfe: support DPFE API v4
From: Markus Mayer @ 2026-06-22 20:38 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Florian Fainelli, Rob Herring, Conor Dooley,
Linux ARM Kernel List, Device Tree Mailing List,
Linux Kernel Mailing List
In-Reply-To: <CAGt4E5tGHJFXswic6vTx-ThN2K9xBtO8oA4ybrXg+q5cA6GYCA@mail.gmail.com>
On Thu, 28 May 2026 at 14:45, Markus Mayer <mmayer@broadcom.com> wrote:
> > > Thanks for providing justification, quite reasonable. A pity that none
> > > of the commit msgs answered this way.
> >
> > The real pity is how this API was designed, making all of this
> > necessary in the first place.
> >
> > We can definitely spell out more clearly in the commit messages what
> > is going on and why all of this is needed. I'll pull all the pieces
> > together from the various responses. As long as there's a way we can
> > reasonably implement what we need, we'll be happy.
>
> It has been a minute, but we'd like to resume this effort[1] to
> upstream these changes or some variation thereof.
>
> What are the best steps to resume this undertaking? There are still a
> few topics where I am not entirely clear on how to better explain
> things or how to address the feedback provided. My apologies for that.
> I will do my best to address whatever concerns there are.
>
> Should I put together a new pull request that contains improved commit
> messages and addresses some of the feedback and we hash out whatever
> questions remain on the new thread? Or would it be better for me to
> reply to the old thread with some of the questions that remain before
> sending a revised series?
Any advice on how to best proceed from here?
Thanks,
-Markus
> [1] https://lore.kernel.org/all/20231205184741.3092376-1-mmayer@broadcom.com/
^ permalink raw reply
* Re: [PATCH v5 2/3] counter: add GPIO-based counter driver
From: Wadim Mueller @ 2026-06-22 20:51 UTC (permalink / raw)
To: William Breathitt Gray
Cc: Oleksij Rempel, kernel, conor+dt, krzk+dt, robh, linux-iio,
devicetree, linux-kernel
In-Reply-To: <20260617074929.333876-1-wbg@kernel.org>
On Wed, 17 Jun 2026 16:49:25 +0900
William Breathitt Gray <wbg@kernel.org> wrote:
Hi William,
thanks for the review. Three things before I spin v6.
> One change I consider is whether to make Signal B optional. [...]
> I wonder whether this is substantially different enough from
> simply using the interrupt-cnt module on the respective IRQ?
> I'm CCing Oleksij and the Pengutronix team in case they wish to
> comment.
I want to keep signal-b mandatory in v6 (if no concerns from Oleksij). The single-line case is
already covered by interrupt-cnt.
> In such a configuration, we would have two Counts: Count 1 [...]
> Count 2 supports only increase/decrease modes with a Synapse for
> Signal B.
Just to confirm, plan for v6 is:
Count 0 "AB Count": A + B + optional index0, all 8 functions
Count 1 "B Count": B + optional index1, increase/decrease only
One counter_ops, dispatch on count->id. Per-count state in
struct gpio_counter_count_priv (value, ceiling, preset, preset_enabled,
enabled, function, direction), held in priv->count_priv[2] as you
suggested. prev_a/prev_b stay on priv (they describe the wire, not
the count).
For the second Index in DT I would just let index-gpios take 1..2
entries (first = count0, second = count1), no new property. Ok for
you and Conor?
> Hmm, is it a problem that priv->enabled is changed to a false state
> before the IRQs are actually disabled? Do any issues arise if an IRQ
> is handled during that brief period of time?
I guess it is a race. In v6 I will reorder:
enable=1: enable_irq(); lock; enabled = true; unlock;
enable=0: lock; enabled = false; unlock; disable_irq();
Plus a mutex around enable_write so two writers can not interleave
(disable_irq() can not run under the spinlock).
All other points from your review (kill *_delta, STATE_CHANGED for
all quadrature modes, INC/DEC both edges, drop prev_a check in
pulse-direction, ...) go into v6 too.
action_write and floor in a follow-up, as you suggested.
Thanks,
Wadim
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: iommu: Fix interrupt type in example
From: Nicolin Chen @ 2026-06-22 20:52 UTC (permalink / raw)
To: Ashish Mhetre
Cc: joro, will, robin.murphy, robh, krzk+dt, conor+dt, jonathanh,
thierry.reding, iommu, devicetree, linux-tegra, linux-kernel
In-Reply-To: <20260622065410.2780215-1-amhetre@nvidia.com>
On Mon, Jun 22, 2026 at 06:54:09AM +0000, Ashish Mhetre wrote:
> The CMDQV interrupt on Tegra264 is edge-triggered per the hardware
> interrupt documentation, but the binding example describes it as
> level-triggered. Correct the example to use IRQ_TYPE_EDGE_RISING so
> that it does not propagate the wrong trigger type.
>
> Fixes: 8a59954192eb ("dt-bindings: iommu: Add NVIDIA Tegra CMDQV support")
> Reported-by: Nicolin Chen <nicolinc@nvidia.com>
> Signed-off-by: Ashish Mhetre <amhetre@nvidia.com>
Acked-by: Nicolin Chen <nicolinc@nvidia.com>
^ permalink raw reply
* Re: [PATCH 2/2] arm64: tegra: Fix CMDQV interrupt type on Tegra264
From: Nicolin Chen @ 2026-06-22 20:58 UTC (permalink / raw)
To: Ashish Mhetre
Cc: joro, will, robin.murphy, robh, krzk+dt, conor+dt, jonathanh,
thierry.reding, iommu, devicetree, linux-tegra, linux-kernel
In-Reply-To: <20260622065410.2780215-2-amhetre@nvidia.com>
On Mon, Jun 22, 2026 at 06:54:10AM +0000, Ashish Mhetre wrote:
> The CMDQV interrupts on Tegra264 are described as level-triggered, but
> per the hardware interrupt documentation these interrupts are actually
> edge-triggered.
>
> Correct the interrupt type for all CMDQV nodes from IRQ_TYPE_LEVEL_HIGH
> to IRQ_TYPE_EDGE_RISING.
>
> Fixes: fe57d0ac4835 ("arm64: tegra: Add nodes for CMDQV")
> Reported-by: Nicolin Chen <nicolinc@nvidia.com>
> Signed-off-by: Ashish Mhetre <amhetre@nvidia.com>
Acked-by: Nicolin Chen <nicolinc@nvidia.com>
^ permalink raw reply
* [PATCH v4 0/3] iio: adc: Add support for TI ADS1110 to ti-ads1100 driver
From: Jakub Szczudlo @ 2026-06-22 22:15 UTC (permalink / raw)
To: linux-iio
Cc: andy, antoniu.miclaus, conor+dt, devicetree, dlechner, duje,
jic23, jishnu.prakash, jorge.marques, joshua.crofts1, krzk+dt,
linusw, jakubszczudlo40, linux-kernel, marcelo.schmitt,
mazziesaccount, mike.looijmans, nuno.sa, robh, sakari.ailus, wens
Add support for the TI ADS1110 to the existing ADS1100 ADC IIO driver.
The ADS1110 is pin-to-pin compatible with the ADS1100 while providing
higher resolution and an internal voltage reference. This patch series
extends driver support for ADS1110, updates device tree bindings and
Kconfig text, and improves the overall hardware description for the
TI ADS1100 family.
Tested on: Raspberry pi 3b+ with 7.0 stable kernel
Signed-off-by: Jakub Szczudlo <jakubszczudlo40@gmail.com>
---
V3 -> V4:
- make fixes patch the first change in the series
- correct error handling when short read
- use ACQUIRE macros from pm_runtime.h in new functions
- Link to v3: https://lore.kernel.org/linux-iio/20260613190957.654798-1-jakubszczudlo40@gmail.com/
V2 -> V3:
- clean patch from unreleated changes
- divide adding support for ads1110 into separate patch
- add missing changelog
- Link to v2: https://lore.kernel.org/linux-iio/20260607183542.368184-1-jakubszczudlo40@gmail.com/
V1 -> V2:
- go from creating new driver to extending ADS1100 driver to support ADS1110
- Link to v1: https://lore.kernel.org/linux-iio/20260527164312.355729-1-jakubszczudlo40@gmail.com/
Jakub Szczudlo (3):
iio: adc: Fix incorrect reading when datarate changed in single mode
dt-bindings: iio: adc: ti,ads1100: add support for ADS1110
iio: adc: Add ti-ads1110 support to ti-ads1100 driver
.../bindings/iio/adc/ti,ads1100.yaml | 10 +-
drivers/iio/adc/Kconfig | 6 +-
drivers/iio/adc/ti-ads1100.c | 153 +++++++++++++++---
3 files changed, 140 insertions(+), 29 deletions(-)
--
2.47.3
^ permalink raw reply
* [PATCH v4 1/3] iio: adc: Fix incorrect reading when datarate changed in single mode
From: Jakub Szczudlo @ 2026-06-22 22:15 UTC (permalink / raw)
To: linux-iio
Cc: andy, antoniu.miclaus, conor+dt, devicetree, dlechner, duje,
jic23, jishnu.prakash, jorge.marques, joshua.crofts1, krzk+dt,
linusw, jakubszczudlo40, linux-kernel, marcelo.schmitt,
mazziesaccount, mike.looijmans, nuno.sa, robh, sakari.ailus, wens
In-Reply-To: <20260622221550.374235-1-jakubszczudlo40@gmail.com>
When device is suspended and it is in single mode then changing
datarate doesn't make it actual wait for new measurement, so to
be sure that read after change is correct functions that changes
datarate and gain will wait for new data.
Fixes: 541880542f2b ("iio: adc: Add TI ADS1100 and ADS1000")
Signed-off-by: Jakub Szczudlo <jakubszczudlo40@gmail.com>
---
drivers/iio/adc/ti-ads1100.c | 74 ++++++++++++++++++++++++++++++++++--
1 file changed, 70 insertions(+), 4 deletions(-)
diff --git a/drivers/iio/adc/ti-ads1100.c b/drivers/iio/adc/ti-ads1100.c
index 9fe8d54cce83..e3c801381434 100644
--- a/drivers/iio/adc/ti-ads1100.c
+++ b/drivers/iio/adc/ti-ads1100.c
@@ -15,6 +15,7 @@
#include <linux/module.h>
#include <linux/init.h>
#include <linux/i2c.h>
+#include <linux/iopoll.h>
#include <linux/mutex.h>
#include <linux/property.h>
#include <linux/pm_runtime.h>
@@ -43,6 +44,9 @@
static const int ads1100_data_rate[] = { 128, 32, 16, 8 };
static const int ads1100_data_rate_bits[] = { 12, 14, 15, 16 };
+/* Timeout based on the minimum sample rate of 8 SPS (7.5s) */
+#define ADS1100_MAX_DRDY_TIMEOUT_US 7500000
+
struct ads1100_data {
struct i2c_client *client;
struct regulator *reg_vdd;
@@ -123,10 +127,49 @@ static int ads1100_get_adc_result(struct ads1100_data *data, int chan, int *val)
return 0;
}
+static bool ads1100_new_data_not_ready(struct ads1100_data *data)
+{
+ int ret;
+ u8 buffer[3];
+
+ ret = i2c_master_recv(data->client, (char *)&buffer, sizeof(buffer));
+ if (ret < 0) {
+ dev_err(&data->client->dev, "I2C read fail: %d\n", ret);
+ return true;
+ } else if (ret < 3) {
+ dev_err(&data->client->dev, "Short I2C read\n");
+ return true;
+ }
+
+ return FIELD_GET(ADS1100_CFG_ST_BSY, buffer[2]);
+}
+
+static int ads1100_poll_data_ready(struct ads1100_data *data)
+{
+ int ret;
+ u8 buffer[3];
+ bool data_ready;
+ int datarate = ads1100_data_rate[FIELD_GET(ADS1100_DR_MASK, data->config)];
+ /* To be sure we wait 5 times more than datarate */
+ unsigned long wait_time = DIV_ROUND_CLOSEST(MICRO, 5 * datarate);
+
+ /* To be sure that polled value will have value after config change */
+ ret = i2c_master_recv(data->client, (char *)&buffer, sizeof(buffer));
+ if (ret < 0) {
+ dev_err(&data->client->dev, "I2C read fail: %d\n", ret);
+ return ret;
+ }
+
+ return read_poll_timeout(ads1100_new_data_not_ready, data_ready,
+ !data_ready, wait_time,
+ ADS1100_MAX_DRDY_TIMEOUT_US, false, data);
+}
+
static int ads1100_set_scale(struct ads1100_data *data, int val, int val2)
{
int microvolts;
int gain;
+ int ret;
/* With Vdd between 2.7 and 5V, the scale is always below 1 */
if (val)
@@ -135,6 +178,12 @@ static int ads1100_set_scale(struct ads1100_data *data, int val, int val2)
if (!val2)
return -EINVAL;
+ PM_RUNTIME_ACQUIRE_AUTOSUSPEND(&data->client->dev, pm);
+
+ ret = PM_RUNTIME_ACQUIRE_ERR(&pm);
+ if (ret)
+ return ret;
+
microvolts = regulator_get_voltage(data->reg_vdd);
/*
* val2 is in 'micro' units, n = val2 / 1000000
@@ -149,19 +198,36 @@ static int ads1100_set_scale(struct ads1100_data *data, int val, int val2)
ads1100_set_config_bits(data, ADS1100_PGA_MASK, ffs(gain) - 1);
- return 0;
+ ret = ads1100_poll_data_ready(data);
+
+ return ret;
}
static int ads1100_set_data_rate(struct ads1100_data *data, int chan, int rate)
{
unsigned int i;
unsigned int size;
+ int ret;
size = data->supports_data_rate ? ARRAY_SIZE(ads1100_data_rate) : 1;
for (i = 0; i < size; i++) {
- if (ads1100_data_rate[i] == rate)
- return ads1100_set_config_bits(data, ADS1100_DR_MASK,
- FIELD_PREP(ADS1100_DR_MASK, i));
+ if (ads1100_data_rate[i] != rate)
+ continue;
+
+ PM_RUNTIME_ACQUIRE_AUTOSUSPEND(&data->client->dev, pm);
+
+ ret = PM_RUNTIME_ACQUIRE_ERR(&pm);
+ if (ret)
+ return ret;
+
+ ret = ads1100_set_config_bits(data, ADS1100_DR_MASK,
+ FIELD_PREP(ADS1100_DR_MASK, i));
+ if (ret)
+ return ret;
+
+ ret = ads1100_poll_data_ready(data);
+
+ return ret;
}
return -EINVAL;
--
2.47.3
^ permalink raw reply related
* [PATCH v4 2/3] dt-bindings: iio: adc: ti,ads1100: add support for ADS1110
From: Jakub Szczudlo @ 2026-06-22 22:15 UTC (permalink / raw)
To: linux-iio
Cc: andy, antoniu.miclaus, conor+dt, devicetree, dlechner, duje,
jic23, jishnu.prakash, jorge.marques, joshua.crofts1, krzk+dt,
linusw, jakubszczudlo40, linux-kernel, marcelo.schmitt,
mazziesaccount, mike.looijmans, nuno.sa, robh, sakari.ailus, wens,
Krzysztof Kozlowski
In-Reply-To: <20260622221550.374235-1-jakubszczudlo40@gmail.com>
Register layouts are the same as for ADS1100 but ADS1110 have different
data rates and have internal voltage reference that is always 2.048V.
Also correct order of ads so they will be sorted alphabetically.
Signed-off-by: Jakub Szczudlo <jakubszczudlo40@gmail.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
---
.../devicetree/bindings/iio/adc/ti,ads1100.yaml | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/iio/adc/ti,ads1100.yaml b/Documentation/devicetree/bindings/iio/adc/ti,ads1100.yaml
index 970ccab15e1e..28c5e2dd0ad6 100644
--- a/Documentation/devicetree/bindings/iio/adc/ti,ads1100.yaml
+++ b/Documentation/devicetree/bindings/iio/adc/ti,ads1100.yaml
@@ -4,19 +4,23 @@
$id: http://devicetree.org/schemas/iio/adc/ti,ads1100.yaml#
$schema: http://devicetree.org/meta-schemas/core.yaml#
-title: TI ADS1100/ADS1000 single channel I2C analog to digital converter
+title: TI ADS1100 and similar single channel I2C Analog to Digital Converters
maintainers:
- Mike Looijmans <mike.looijmans@topic.nl>
description: |
- Datasheet at: https://www.ti.com/lit/gpn/ads1100
+ Datasheets:
+ - https://www.ti.com/lit/gpn/ads1000
+ - https://www.ti.com/lit/gpn/ads1100
+ - https://www.ti.com/lit/gpn/ads1110
properties:
compatible:
enum:
- - ti,ads1100
- ti,ads1000
+ - ti,ads1100
+ - ti,ads1110
reg:
maxItems: 1
--
2.47.3
^ permalink raw reply related
* [PATCH v4 3/3] iio: adc: Add ti-ads1110 support to ti-ads1100 driver
From: Jakub Szczudlo @ 2026-06-22 22:15 UTC (permalink / raw)
To: linux-iio
Cc: andy, antoniu.miclaus, conor+dt, devicetree, dlechner, duje,
jic23, jishnu.prakash, jorge.marques, joshua.crofts1, krzk+dt,
linusw, jakubszczudlo40, linux-kernel, marcelo.schmitt,
mazziesaccount, mike.looijmans, nuno.sa, robh, sakari.ailus, wens
In-Reply-To: <20260622221550.374235-1-jakubszczudlo40@gmail.com>
Add ADS1110 support that have faster datarate than ADS1100, it also uses
internal voltage reference of 2.048V for measurement.
Signed-off-by: Jakub Szczudlo <jakubszczudlo40@gmail.com>
---
drivers/iio/adc/Kconfig | 6 +--
drivers/iio/adc/ti-ads1100.c | 81 +++++++++++++++++++++++++++---------
2 files changed, 64 insertions(+), 23 deletions(-)
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 1c663c98c6c9..30198335c63b 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -1765,11 +1765,11 @@ config TI_ADS1018
called ti-ads1018.
config TI_ADS1100
- tristate "Texas Instruments ADS1100 and ADS1000 ADC"
+ tristate "Texas Instruments ADS1100 and similar single channel I2C ADC"
depends on I2C
help
- If you say yes here you get support for Texas Instruments ADS1100 and
- ADS1000 ADC chips.
+ If you say yes here you get support TI ADS1100 and similar single
+ channel I2C Analog to Digital Converters.
This driver can also be built as a module. If so, the module will be
called ti-ads1100.
diff --git a/drivers/iio/adc/ti-ads1100.c b/drivers/iio/adc/ti-ads1100.c
index e3c801381434..ec79a89464fb 100644
--- a/drivers/iio/adc/ti-ads1100.c
+++ b/drivers/iio/adc/ti-ads1100.c
@@ -5,7 +5,7 @@
* Copyright (c) 2023, Topic Embedded Products
*
* Datasheet: https://www.ti.com/lit/gpn/ads1100
- * IIO driver for ADS1100 and ADS1000 ADC 16-bit I2C
+ * IIO driver for ADS1100 and similar single channel ADC 16-bit I2C
*/
#include <linux/bitfield.h>
@@ -40,20 +40,44 @@
#define ADS1100_SINGLESHOT ADS1100_CFG_SC
#define ADS1100_SLEEP_DELAY_MS 2000
+#define ADS1110_INTERNAL_REF_mV 2048
static const int ads1100_data_rate[] = { 128, 32, 16, 8 };
+static const int ads1110_data_rate[] = { 240, 60, 30, 15 };
static const int ads1100_data_rate_bits[] = { 12, 14, 15, 16 };
/* Timeout based on the minimum sample rate of 8 SPS (7.5s) */
#define ADS1100_MAX_DRDY_TIMEOUT_US 7500000
+struct ads1100_config {
+ const char *name;
+ const int *data_rate;
+ const int data_rate_count;
+ bool has_internal_vref_only;
+};
+
+static const struct ads1100_config ads1100_config = {
+ .name = "ads1100",
+ .data_rate = ads1100_data_rate,
+ .data_rate_count = ARRAY_SIZE(ads1100_data_rate),
+ .has_internal_vref_only = false,
+};
+
+static const struct ads1100_config ads1110_config = {
+ .name = "ads1110",
+ .data_rate = ads1110_data_rate,
+ .data_rate_count = ARRAY_SIZE(ads1110_data_rate),
+ .has_internal_vref_only = true,
+};
+
struct ads1100_data {
struct i2c_client *client;
struct regulator *reg_vdd;
struct mutex lock;
int scale_avail[2 * 4]; /* 4 gain settings */
+ const struct ads1100_config *ads_config;
u8 config;
- bool supports_data_rate; /* Only the ADS1100 can select the rate */
+ bool supports_data_rate;
};
static const struct iio_chan_spec ads1100_channel = {
@@ -89,6 +113,14 @@ static int ads1100_set_config_bits(struct ads1100_data *data, u8 mask, u8 value)
return 0;
};
+static int ads1100_get_vref_milivolts(struct ads1100_data *data)
+{
+ if (data->ads_config->has_internal_vref_only)
+ return ADS1110_INTERNAL_REF_mV;
+
+ return regulator_get_voltage(data->reg_vdd) / MILLI;
+}
+
static int ads1100_data_bits(struct ads1100_data *data)
{
return ads1100_data_rate_bits[FIELD_GET(ADS1100_DR_MASK, data->config)];
@@ -114,6 +146,9 @@ static int ads1100_get_adc_result(struct ads1100_data *data, int chan, int *val)
if (ret < 0) {
dev_err(&data->client->dev, "I2C read fail: %d\n", ret);
return ret;
+ } else if (ret < 2) {
+ dev_err(&data->client->dev, "Short I2C read\n");
+ return -EIO;
}
/* Value is always 16-bit 2's complement */
@@ -184,7 +219,7 @@ static int ads1100_set_scale(struct ads1100_data *data, int val, int val2)
if (ret)
return ret;
- microvolts = regulator_get_voltage(data->reg_vdd);
+ microvolts = ads1100_get_vref_milivolts(data) * (MICRO / MILLI);
/*
* val2 is in 'micro' units, n = val2 / 1000000
* result must be millivolts, d = microvolts / 1000
@@ -209,9 +244,9 @@ static int ads1100_set_data_rate(struct ads1100_data *data, int chan, int rate)
unsigned int size;
int ret;
- size = data->supports_data_rate ? ARRAY_SIZE(ads1100_data_rate) : 1;
+ size = data->supports_data_rate ? data->ads_config->data_rate_count : 1;
for (i = 0; i < size; i++) {
- if (ads1100_data_rate[i] != rate)
+ if (data->ads_config->data_rate[i] != rate)
continue;
PM_RUNTIME_ACQUIRE_AUTOSUSPEND(&data->client->dev, pm);
@@ -233,14 +268,9 @@ static int ads1100_set_data_rate(struct ads1100_data *data, int chan, int rate)
return -EINVAL;
}
-static int ads1100_get_vdd_millivolts(struct ads1100_data *data)
-{
- return regulator_get_voltage(data->reg_vdd) / (MICRO / MILLI);
-}
-
static void ads1100_calc_scale_avail(struct ads1100_data *data)
{
- int millivolts = ads1100_get_vdd_millivolts(data);
+ int millivolts = ads1100_get_vref_milivolts(data);
unsigned int i;
for (i = 0; i < ARRAY_SIZE(data->scale_avail) / 2; i++) {
@@ -262,9 +292,9 @@ static int ads1100_read_avail(struct iio_dev *indio_dev,
switch (mask) {
case IIO_CHAN_INFO_SAMP_FREQ:
*type = IIO_VAL_INT;
- *vals = ads1100_data_rate;
+ *vals = data->ads_config->data_rate;
if (data->supports_data_rate)
- *length = ARRAY_SIZE(ads1100_data_rate);
+ *length = data->ads_config->data_rate_count;
else
*length = 1;
return IIO_AVAIL_LIST;
@@ -283,6 +313,7 @@ static int ads1100_read_raw(struct iio_dev *indio_dev,
int *val2, long mask)
{
int ret;
+ int data_rate_index;
struct ads1100_data *data = iio_priv(indio_dev);
guard(mutex)(&data->lock);
@@ -299,12 +330,12 @@ static int ads1100_read_raw(struct iio_dev *indio_dev,
return IIO_VAL_INT;
case IIO_CHAN_INFO_SCALE:
/* full-scale is the supply voltage in millivolts */
- *val = ads1100_get_vdd_millivolts(data);
+ *val = ads1100_get_vref_milivolts(data);
*val2 = 15 + FIELD_GET(ADS1100_PGA_MASK, data->config);
return IIO_VAL_FRACTIONAL_LOG2;
case IIO_CHAN_INFO_SAMP_FREQ:
- *val = ads1100_data_rate[FIELD_GET(ADS1100_DR_MASK,
- data->config)];
+ data_rate_index = FIELD_GET(ADS1100_DR_MASK, data->config);
+ *val = data->ads_config->data_rate[data_rate_index];
return IIO_VAL_INT;
default:
return -EINVAL;
@@ -373,6 +404,7 @@ static int ads1100_probe(struct i2c_client *client)
struct iio_dev *indio_dev;
struct ads1100_data *data;
struct device *dev = &client->dev;
+ const struct ads1100_config *model;
int ret;
indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
@@ -404,6 +436,13 @@ static int ads1100_probe(struct i2c_client *client)
if (ret)
return ret;
+ model = i2c_get_match_data(client);
+ if (!model)
+ return dev_err_probe(dev, -EINVAL,
+ "Can't get device data from firmware\n");
+
+ data->ads_config = (struct ads1100_config *)model;
+
ret = ads1100_setup(data);
if (ret)
return dev_err_probe(dev, ret,
@@ -466,16 +505,18 @@ static DEFINE_RUNTIME_DEV_PM_OPS(ads1100_pm_ops,
NULL);
static const struct i2c_device_id ads1100_id[] = {
- { .name = "ads1100" },
- { .name = "ads1000" },
+ { .name = "ads1000", .driver_data = (kernel_ulong_t)&ads1100_config },
+ { .name = "ads1100", .driver_data = (kernel_ulong_t)&ads1100_config },
+ { .name = "ads1110", .driver_data = (kernel_ulong_t)&ads1110_config },
{ }
};
MODULE_DEVICE_TABLE(i2c, ads1100_id);
static const struct of_device_id ads1100_of_match[] = {
- {.compatible = "ti,ads1100" },
- {.compatible = "ti,ads1000" },
+ { .compatible = "ti,ads1000", .data = &ads1100_config },
+ { .compatible = "ti,ads1100", .data = &ads1100_config },
+ { .compatible = "ti,ads1110", .data = &ads1110_config },
{ }
};
--
2.47.3
^ permalink raw reply related
* Re: [PATCH v4 3/3] iio: adc: Add ti-ads1110 support to ti-ads1100 driver
From: sashiko-bot @ 2026-06-22 22:31 UTC (permalink / raw)
To: Jakub Szczudlo; +Cc: devicetree, robh, conor+dt
In-Reply-To: <20260622221550.374235-4-jakubszczudlo40@gmail.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [Medium] Hardcoded IIO device name ignores the newly added device configuration name.
- [Medium] Hardcoded ads1100_data_rate array used instead of config-specific data_rate in ads1100_poll_data_ready.
Pre-existing issues:
- [High] Missing PM runtime resume on device removal causes an unbalanced regulator_disable() call.
--
commit d015d54867f4b919a8e4d7c10d3727993b652f65
Author: Jakub Szczudlo <jakubszczudlo40@gmail.com>
iio: adc: Add ti-ads1110 support to ti-ads1100 driver
Add ADS1110 support that have faster datarate than ADS1100, it also uses
internal voltage reference of 2.048V for measurement.
> diff --git a/drivers/iio/adc/ti-ads1100.c b/drivers/iio/adc/ti-ads1100.c
> index e3c801381434d..ec79a89464fb6 100644
> --- a/drivers/iio/adc/ti-ads1100.c
> +++ b/drivers/iio/adc/ti-ads1100.c
> @@ -40,20 +40,44 @@
> #define ADS1100_SINGLESHOT ADS1100_CFG_SC
>
> #define ADS1100_SLEEP_DELAY_MS 2000
> +#define ADS1110_INTERNAL_REF_mV 2048
>
> static const int ads1100_data_rate[] = { 128, 32, 16, 8 };
> +static const int ads1110_data_rate[] = { 240, 60, 30, 15 };
[Severity: Medium]
Will this result in using the slower ads1100_data_rate array for the ADS1110
when polling for data readiness?
Looking at ads1100_poll_data_ready(), it still accesses the hardcoded
ads1100_data_rate array directly:
static int ads1100_poll_data_ready(struct ads1100_data *data)
{
...
int datarate = ads1100_data_rate[FIELD_GET(ADS1100_DR_MASK, data->config)];
/* To be sure we wait 5 times more than datarate */
unsigned long wait_time = DIV_ROUND_CLOSEST(MICRO, 5 * datarate);
...
}
Should this be updated to use data->ads_config->data_rate instead?
[ ... ]
> @@ -404,6 +436,13 @@ static int ads1100_probe(struct i2c_client *client)
> if (ret)
> return ret;
>
> + model = i2c_get_match_data(client);
> + if (!model)
> + return dev_err_probe(dev, -EINVAL,
> + "Can't get device data from firmware\n");
> +
> + data->ads_config = (struct ads1100_config *)model;
> +
[Severity: Medium]
Does this ignore the newly added device configuration name for the IIO device?
In ads1100_probe(), indio_dev->name is hardcoded earlier in the function:
indio_dev->name = "ads1100";
Should this be updated to use data->ads_config->name so the user-space
sysfs name correctly identifies the ADS1110?
[Severity: High]
This is a pre-existing issue, but does the lack of a PM runtime resume on
device removal cause an unbalanced regulator disable?
If the device auto-suspends during runtime idle, the vdd regulator is disabled
via ads1100_runtime_suspend():
static int ads1100_runtime_suspend(struct device *dev)
{
...
regulator_disable(data->reg_vdd);
...
}
When the driver is unbound, devres calls pm_runtime_disable() (which does not
resume the device), followed by ads1100_reg_disable() which attempts to call
regulator_disable() again on the already-disabled regulator, triggering a
kernel WARNING.
Could we ensure the device is resumed during driver unbind to prevent this?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260622221550.374235-1-jakubszczudlo40@gmail.com?part=3
^ permalink raw reply
* Re: [PATCH v4 1/3] iio: adc: Fix incorrect reading when datarate changed in single mode
From: sashiko-bot @ 2026-06-22 22:33 UTC (permalink / raw)
To: Jakub Szczudlo; +Cc: devicetree, robh, conor+dt
In-Reply-To: <20260622221550.374235-2-jakubszczudlo40@gmail.com>
Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
New issues:
- [High] I2C read failures inside the polling loop are improperly mapped to 'device busy', causing a 7.5-second hang and massive dmesg log flooding.
- [High] The data ready polling logic fails to synchronize because the ST/BSY bit is likely static in continuous conversion mode.
- [Medium] The `ads1100_poll_data_ready` function performs a completely redundant I2C read that wastes bus bandwidth.
Pre-existing issues:
- [High] The return value of `ads1100_set_config_bits` is ignored in `ads1100_set_scale`, leading to silent configuration failures.
- [High] Reading the ADC result immediately after runtime resume returns an invalid or stale conversion.
- [High] I2C read functions allocate transfer buffers on the stack, which is unsafe for DMA operations.
--
commit 06506d1fcb678ffc0137dbc1df3913f95ce47327
Author: Jakub Szczudlo <jakubszczudlo40@gmail.com>
Subject: iio: adc: Fix incorrect reading when datarate changed in single mode
This commit attempts to fix incorrect ADC readings after changing
datarate or gain while suspended in single mode. It introduces a
polling loop to wait for new data before returning from the configuration
functions.
> diff --git a/drivers/iio/adc/ti-ads1100.c b/drivers/iio/adc/ti-ads1100.c
> index 9fe8d54cce83c..e3c801381434d 100644
> --- a/drivers/iio/adc/ti-ads1100.c
> +++ b/drivers/iio/adc/ti-ads1100.c
[ ... ]
> @@ -123,10 +127,49 @@ static int ads1100_get_adc_result(struct ads1100_data *data, int chan, int *val)
> return 0;
> }
[Severity: High]
This is a pre-existing issue, but doesn't reading the ADC result immediately
after runtime resume return an invalid or stale conversion?
Looking at ads1100_get_adc_result():
ret = pm_runtime_resume_and_get(&data->client->dev);
if (ret < 0)
return ret;
ret = i2c_master_recv(data->client, (char *)&buffer, sizeof(buffer));
When the device auto-suspends, a subsequent sysfs read of in_voltage0_raw
calls pm_runtime_resume_and_get(), which transitions the ADC to continuous
mode and starts a new conversion. However, ads1100_get_adc_result()
immediately reads the I2C output register without waiting for the conversion
to complete, which can take up to 125ms at 8 SPS.
> +static bool ads1100_new_data_not_ready(struct ads1100_data *data)
> +{
> + int ret;
> + u8 buffer[3];
> +
> + ret = i2c_master_recv(data->client, (char *)&buffer, sizeof(buffer));
[Severity: High]
This isn't a bug introduced by this patch, but is it safe to use
stack-allocated buffers for I2C transfers?
Using stack-allocated buffers for I2C transfers violates subsystem rules and
can lead to memory corruption or kernel panics on architectures where
CONFIG_VMAP_STACK is enabled and the underlying I2C controller utilizes DMA.
> + if (ret < 0) {
> + dev_err(&data->client->dev, "I2C read fail: %d\n", ret);
> + return true;
[Severity: High]
Does returning true on hard I2C errors trap the driver in an infinite polling
loop?
Returning true signals to the read_poll_timeout() macro that the device is
merely not ready, causing it to endlessly retry for up to 7.5 seconds instead
of aborting. This masks the actual I2C error code with -ETIMEDOUT and
generates massive dmesg log flooding.
> + } else if (ret < 3) {
> + dev_err(&data->client->dev, "Short I2C read\n");
> + return true;
> + }
> +
> + return FIELD_GET(ADS1100_CFG_ST_BSY, buffer[2]);
[Severity: High]
Will this polling logic synchronize properly in continuous conversion mode?
When changing scale or datarate, PM_RUNTIME_ACQUIRE_AUTOSUSPEND() is called,
transitioning the ADC to continuous conversion mode. In continuous conversion
mode on these TI ADCs, the ST_BSY bit does not pulse to indicate new data;
it is typically static.
If ST_BSY reads as 0, ads1100_new_data_not_ready() returns false, breaking the
loop instantly. If it reads as 1, the driver would hang for 7.5 seconds.
> +}
> +
> +static int ads1100_poll_data_ready(struct ads1100_data *data)
> +{
> + int ret;
> + u8 buffer[3];
> + bool data_ready;
> + int datarate = ads1100_data_rate[FIELD_GET(ADS1100_DR_MASK, data->config)];
> + /* To be sure we wait 5 times more than datarate */
> + unsigned long wait_time = DIV_ROUND_CLOSEST(MICRO, 5 * datarate);
> +
> + /* To be sure that polled value will have value after config change */
> + ret = i2c_master_recv(data->client, (char *)&buffer, sizeof(buffer));
[Severity: Medium]
Is this initial I2C read redundant?
Executing ads1100_poll_data_ready() triggers this initial i2c_master_recv(),
discards the populated local buffer, and immediately enters
read_poll_timeout() with sleep_before_read = false. This instantly performs
another identical i2c_master_recv() inside ads1100_new_data_not_ready(),
needlessly occupying the I2C bus.
[ ... ]
> @@ -149,19 +198,36 @@ static int ads1100_set_scale(struct ads1100_data *data, int val, int val2)
>
> ads1100_set_config_bits(data, ADS1100_PGA_MASK, ffs(gain) - 1);
>
> - return 0;
> + ret = ads1100_poll_data_ready(data);
> +
> + return ret;
> }
[Severity: High]
This is a pre-existing issue, but does ads1100_set_scale() ignore the return
value of ads1100_set_config_bits()?
If the I2C write to update the hardware PGA scale fails, the error is
discarded. The function proceeds to poll for new data and may eventually
return success to userspace.
This leads to silently corrupted or miscalculated voltage measurements because
the sysfs attribute appears to have updated while the ADC is physically
operating with the old scale.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260622221550.374235-1-jakubszczudlo40@gmail.com?part=1
^ permalink raw reply
* Re: [PATCH v10 4/5] iio: adc: versal-sysmon: add threshold event support
From: Erim, Salih @ 2026-06-22 23:59 UTC (permalink / raw)
To: Jonathan Cameron
Cc: andy, dlechner, nuno.sa, robh, krzk+dt, conor+dt, conall.ogriofa,
michal.simek, linux, erimsalih, linux-iio, devicetree,
linux-kernel, Andy Shevchenko
In-Reply-To: <20260621172247.3499cab0@jic23-huawei>
Hi Jonathan,
Thank you for the review.
On 21/06/2026 17:22, Jonathan Cameron wrote:
> On Thu, 18 Jun 2026 11:14:13 +0100
> Salih Erim <salih.erim@amd.com> wrote:
>
>> Add threshold event support for temperature and supply voltage
>> channels.
>>
>> Temperature events:
>> - Rising threshold with configurable value on the device
>> temperature channel (current max across all satellites)
>> - Per-channel hysteresis as a millicelsius value
>> - Event direction is IIO_EV_DIR_RISING (hysteresis mode)
>>
>> Supply voltage events:
>> - Rising/falling threshold per supply channel
>> - Per-channel alarm enable via alarm configuration registers
>>
>> The hardware supports both window and hysteresis alarm modes for
>> temperature. This driver uses hysteresis mode, where the upper
>> threshold triggers the alarm and the lower threshold clears it
>> (re-arm point). The hardware has a single ISR bit per temperature
>> channel with no indication of which threshold was crossed, so
>> hysteresis mode is the natural fit. The lower threshold register
>> is computed internally as (upper - hysteresis).
>>
>> Hysteresis is stored in the driver as a millicelsius value,
>> initialized from the hardware registers at probe. Writing the
>> rising threshold or hysteresis recomputes the lower register.
>> ALARM_CONFIG is hard-coded to hysteresis mode during init.
>>
>> The hardware also provides a separate over-temperature (OT)
>> threshold, but it is not exposed through IIO as it serves as a
>> hardware safety mechanism for platform shutdown. OT will be
>> exposed through the thermal framework in a follow-up series.
>>
>> The interrupt handler masks active threshold interrupts (which are
>> level-sensitive) and schedules a delayed worker to poll for condition
>> clear before unmasking. When no hardware IRQ is available, event
>> specs are not attached and interrupt init is skipped, since the
>> I2C regmap backend cannot be called from atomic context.
>>
>> When disabling a supply channel alarm, the group interrupt remains
>> active if any other channel in the same alarm group still has an
>> alarm enabled.
>>
>> A devm cleanup action masks all interrupts on driver unbind to
>> prevent unhandled interrupt storms after the IRQ handler is freed.
>>
>> Signed-off-by: Salih Erim <salih.erim@amd.com>
>> Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
>
> There is some stuff from Sashiko that looks plausible.
> https://sashiko.dev/#/patchset/20260618101414.3462934-1-salih.erim%40amd.com
>
> Whilst the out of range temp thresholds might not be a bug
> that causes anything particular bad to happen it would be nice
> to report an error to userspace if a write is for something we
> can't support.
>
> There are some things that I can't figure out without data sheet
> diving so I'll leave those for you to sanity check + some I think
> we addressed in earlier discussions.
> For a few of the things it raises I talk about them inline.
Thanks for the guidance on handling Sashiko findings. I have
reviewed all of them and will reply on-list with my assessment.
>
> Note I didn't spot anything else (and probably wouldn't have
> spotted these :(
>
> Jonathan
>
>
>> ---
>> Changes in v10:
>> - Add Reviewed-by tag from Andy Shevchenko
>> - Add limits.h include for U16_MAX, S16_MIN, S16_MAX (Andy)
>>
>> Changes in v9:
>> - Add minmax.h include for clamp() (Andy)
>> - Join sysmon_supply_thresh_offset to one line, change address
>> parameter to unsigned long for consistency (Andy)
>> - Combine mask declaration with initialization in
>> sysmon_read_event_config (Andy)
>> - Rename ier to mask in sysmon_write_event_config for
>> consistency with sysmon_read_event_config (Andy)
>> - Remove blank line in sysmon_update_temp_lower between
>> semantically coupled lines (Andy)
>> - Rename unmask to ier (u32) in sysmon_unmask_temp (Andy)
>> - Variable name and type consistency audit across all
>> event functions (Andy)
>>
>> Changes in v8:
>> - Use MILLIDEGREE_PER_DEGREE in q8p7 conversion functions (Andy)
>> - Use regmap_test_bits() in sysmon_read_alarm_config (Andy)
>> - Join sysmon_parse_fw signature onto one line (Andy)
>> - Fix devm teardown race: replace devm_delayed_work_autocancel
>> with INIT_DELAYED_WORK; fold cancel_delayed_work_sync into
>> sysmon_disable_interrupts to prevent the worker from
>> re-enabling interrupts after the IRQ handler is freed (Sashiko)
>> - Drop devm-helpers.h include (no longer needed)
>>
>> Changes in v7:
>> - Move TEMP threshold event onto channel 0; drop OT as
>> separate IIO channel -- OT is a hardware safety mechanism
>> better suited for the thermal framework follow-up (Jonathan)
>> - Use single temp_channels array; attach event spec to
>> channel 0 at runtime when IRQ is available, matching the
>> pattern used for supply channels (Jonathan)
>> - Remove sysmon_temp_thresh_offset; use SYSMON_TEMP_TH_UP
>> and SYSMON_TEMP_TH_LOW defines directly at call sites
>> - Return administrative state from temp_mask in
>> read_event_config instead of transient hardware IMR
>> (Jonathan, Sashiko)
>> - Add devm_add_action_or_reset to mask all HW interrupts
>> on driver unbind (Sashiko)
>> - Remove SYSMON_CHAN_TEMP_EVENT macro, SYSMON_ADDR_TEMP_EVENT,
>> SYSMON_ADDR_OT_EVENT, SYSMON_BIT_OT, SYSMON_OT_HYST_MASK,
>> OT_TH_LOW/UP registers, ot_hysteresis from struct
>> - Simplify sysmon_get_event_mask, sysmon_update_temp_lower,
>> sysmon_init_hysteresis -- all now operate on single TEMP
>> channel only
>>
>>
>> Changes in v6:
>> - Remove types.h from header (not needed at any stage) (Andy)
>> - Macro brace on separate line for SYSMON_CHAN_TEMP_EVENT (Andy)
>> - switch(chan->type) in all event functions instead of cascading
>> if statements (Andy)
>> - switch(info) in read/write_event_value for nested
>> dispatch (Andy)
>> - Reversed xmas tree in sysmon_update_temp_lower and
>> sysmon_init_hysteresis (Andy)
>> - scoped_guard(spinlock_irq) with error check in
>> sysmon_unmask_worker (Andy)
>> - Combined regmap_read error check with || in
>> sysmon_iio_irq (Andy)
>> - Join devm_request_irq on one line (Andy)
>> - Fix fwnode_irq_get() to propagate only -EPROBE_DEFER;
>> treating all negatives as fatal broke probe on I2C nodes
>> without interrupts property
>>
>> Changes in v5:
>> - clamp() instead of clamp_t() (Andy)
>> - regmap_assign_bits() instead of separate set/clear (Andy)
>> - Remove unneeded parentheses (2 places) (Andy)
>> - for_each_set_bit on single line (Andy)
>> - regmap_clear_bits() instead of regmap_update_bits() (Andy)
>> - Simplify unmask XOR to ~status & masked_temp (Andy)
>> - Add comment explaining unmask &= ~temp_mask logic (Andy)
>> - Split container_of across two lines (Andy)
>> - Move ISR write after !isr check to avoid writing 0 (Andy)
>> - unsigned int for init_hysteresis address param (Andy)
>> - Add comment explaining error check policy in worker/IRQ (Andy)
>> - Nested size_add() for overflow-safe allocation (Andy)
>> - Propagate negative from fwnode_irq_get() for
>> EPROBE_DEFER (Andy)
>> - Pass irq instead of has_irq to sysmon_parse_fw (Andy)
>>
>> Changes in v4:
>> - Merge event channels into static temp array; two arrays
>> (with/without events) selected by has_irq (Jonathan)
>> - Event-only channels have no info_mask; their addresses are
>> logical identifiers, not readable registers
>> - Drop RAW for voltage events, keep PROCESSED only (Jonathan)
>> - Drop scan_type from event channel macro (Jonathan)
>> - Blank lines between call+error-check blocks (Jonathan)
>> - Fit under 80 chars on one line where possible (Jonathan)
>> - default case returns -EINVAL instead of break (Jonathan)
>> - sysmon_handle_event: return early in each case (Jonathan)
>> - guard(spinlock) in sysmon_iio_irq, return IRQ_NONE/IRQ_HANDLED
>> directly (Jonathan)
>> - Take irq_lock in write_event_config for temp_mask updates to
>> synchronize with unmask worker (Sashiko)
>>
>> Changes in v3:
>> - IWYU: add new includes, group iio headers with blank line (Andy)
>> - Reduce casts in millicelsius_to_q8p7, consistent style with
>> q8p7_to_millicelsius (Andy)
>> - Use clamp_t with typed constants, remove tmp & U16_MAX (Andy)
>> - Use !! to return 0/1 from read_alarm_config (Andy)
>> - Use regmap_set_bits/clear_bits in write_alarm_config (Andy)
>> - Add comment explaining spinlock is safe (I2C never reaches
>> event code path) (Andy)
>> - Add comment explaining IMR negation logic (Andy)
>> - Split read_event_value/write_event_value parameters logically
>> across lines (Andy)
>> - Move mask/shift after regmap_read error check (Andy)
>> - Remove redundant else in read_event_value and
>> write_event_value (Andy)
>> - Use named constant for hysteresis bit, if-else not ternary
>> (Andy)
>> - Loop variable declared in for() scope (Andy)
>> - Add error checks in sysmon_handle_event (Andy)
>> - Use IRQ_RETVAL() macro (Andy)
>> - Use devm_delayed_work_autocancel instead of manual INIT +
>> devm_add_action (Andy)
>> - Use FIELD_GET/FIELD_PREP for hysteresis register bits
>> (Jonathan)
>> - Split OT vs TEMP handling with FIELD_GET (Jonathan)
>> - Rework hysteresis: store as millicelsius value, hardcode
>> ALARM_CONFIG to hysteresis mode, compute lower threshold
>> from (upper - hysteresis), initialize from HW at probe
>> (Jonathan)
>> - Remove falling threshold for temperature; single event
>> spec per channel with IIO_EV_DIR_RISING (Jonathan)
>> - Push IIO_EV_DIR_RISING events for temperature,
>> IIO_EV_DIR_EITHER for voltage (Jonathan)
>>
>> Changes in v2:
>> - Reverse Christmas Tree variable ordering in all functions
>> - Named constants for hysteresis bits: SYSMON_OT_HYST_BIT,
>> SYSMON_TEMP_HYST_BIT instead of magic 0x1/0x2
>> - SYSMON_ALARM_BITS_PER_REG replaces magic number 32
>> - SYSMON_ALARM_OFFSET() helper macro deduplicates alarm register
>> offset computation
>> - BIT() macro for shift expressions in conversion functions
>> - Hysteresis input validated to single-bit range (0 or 1)
>> - Event channels only created when irq > 0 (I2C safety)
>> - Group alarm interrupt stays active while any channel in the
>> group has an alarm enabled
>> - write_event_value returns -EINVAL for unhandled types
>> - IRQ_NONE returned for spurious interrupts
>> - Q8.7 write path uses multiplication instead of left-shift
>> to avoid undefined behavior with negative temperatures
>> - (u16) mask prevents garbage in reserved register bits
>> - regmap_write return values checked for IER/IDR writes
>> - devm cleanup ordering: cancel_work before request_irq
>> drivers/iio/adc/versal-sysmon-core.c | 600 ++++++++++++++++++++++++++-
>> drivers/iio/adc/versal-sysmon.h | 36 ++
>> 2 files changed, 632 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/iio/adc/versal-sysmon-core.c b/drivers/iio/adc/versal-sysmon-core.c
>> index 03a745d3fb4..50b5228aa22 100644
>> --- a/drivers/iio/adc/versal-sysmon-core.c
>> +++ b/drivers/iio/adc/versal-sysmon-core.c
>
>
>> /*
>> * Static temperature channels (always present).
>> *
>> @@ -52,6 +102,16 @@ static const struct iio_chan_spec temp_channels[] = {
>> SYSMON_CHAN_TEMP(3, SYSMON_TEMP_MIN_MIN, "min_min"),
>> };
>>
>> +static void sysmon_q8p7_to_millicelsius(s16 raw_data, int *val)
>> +{
>> + *val = (raw_data * MILLIDEGREE_PER_DEGREE) >> SYSMON_FRACTIONAL_SHIFT;
>> +}
>> +
>> +static void sysmon_millicelsius_to_q8p7(u32 *raw_data, int val)
>> +{
>> + *raw_data = (val << SYSMON_FRACTIONAL_SHIFT) / MILLIDEGREE_PER_DEGREE;
>> +}
>> +
>> static void sysmon_supply_rawtoprocessed(int raw_data, int *val)
>> {
>> int mantissa, format, exponent;
>> @@ -69,6 +129,33 @@ static void sysmon_supply_rawtoprocessed(int raw_data, int *val)
>> *val = (mantissa * (int)MILLI) >> exponent;
>> }
>>
>> +static void sysmon_supply_processedtoraw(int val, u32 reg_val, u32 *raw_data)
>> +{
>> + int exponent = FIELD_GET(SYSMON_MODE_MASK, reg_val);
>> + int format = FIELD_GET(SYSMON_FMT_MASK, reg_val);
>> + int scale, tmp;
>> +
>> + scale = BIT(SYSMON_SUPPLY_MANTISSA_BITS - exponent);
>> + tmp = (val * scale) / (int)MILLI;
>
> See below. Overflow issue is if val is large enough that this overflows
> before tmp is clamped, possibly giving unexpected values.
Agreed. Will add input validation to prevent the overflow
before the clamp.
>
>> +
>> + if (format)
>> + tmp = clamp(tmp, S16_MIN, S16_MAX);
>> + else
>> + tmp = clamp(tmp, 0, U16_MAX);
>> +
>> + *raw_data = (u16)tmp;
>> +}
>
> ...
>
>> +
>> +static int sysmon_read_event_value(struct iio_dev *indio_dev,
>> + const struct iio_chan_spec *chan,
>> + enum iio_event_type type,
>> + enum iio_event_direction dir,
>> + enum iio_event_info info,
>> + int *val, int *val2)
>> +{
>> + struct sysmon *sysmon = iio_priv(indio_dev);
>> + unsigned int reg_val;
>> + int offset;
>> + int ret;
>> +
>> + guard(mutex)(&sysmon->lock);
>> +
>> + switch (chan->type) {
>> + case IIO_TEMP:
>> + switch (info) {
>> + case IIO_EV_INFO_VALUE:
>> + ret = regmap_read(sysmon->regmap, SYSMON_TEMP_TH_UP, ®_val);
>> + if (ret)
>> + return ret;
>> +
>> + sysmon_q8p7_to_millicelsius(reg_val, val);
>> +
>> + return IIO_VAL_INT;
>> +
>> + case IIO_EV_INFO_HYSTERESIS:
>> + *val = sysmon->temp_hysteresis;
>> + return IIO_VAL_INT;
>> +
>> + default:
>> + return -EINVAL;
>> + }
>> +
>> + case IIO_VOLTAGE:
>> + offset = sysmon_supply_thresh_offset(chan->address, dir);
>> + if (offset < 0)
>> + return offset;
>> +
>> + ret = regmap_read(sysmon->regmap, offset, ®_val);
>> + if (ret)
>> + return ret;
>> +
>> + sysmon_supply_rawtoprocessed(reg_val, val);
>> +
>> + return IIO_VAL_INT;
>> +
>> + default:
>> + return -EINVAL;
>> + }
>> +}
>> +
>> +static int sysmon_write_event_value(struct iio_dev *indio_dev,
>> + const struct iio_chan_spec *chan,
>> + enum iio_event_type type,
>> + enum iio_event_direction dir,
>> + enum iio_event_info info,
>> + int val, int val2)
>> +{
>> + struct sysmon *sysmon = iio_priv(indio_dev);
>> + unsigned int reg_val;
>> + u32 raw_val;
>> + int offset;
>> + int ret;
>> +
>> + guard(mutex)(&sysmon->lock);
>> +
>> + switch (chan->type) {
>> + case IIO_TEMP:
>> + switch (info) {
>> + case IIO_EV_INFO_VALUE:
>> + sysmon_millicelsius_to_q8p7(&raw_val, val);
> In this path, sashiko is asking whether it is possible for val to be sufficiently large
> or negative that the calculation is going to given rather unexpected results.
>
> Given in the read direction you assume it is suitable for passing in a U16, should we
> have a check here? + error out if it is out of range?
Agreed. Will add a bounds check on val before the Q8.7 conversion
and return -EINVAL if out of the representable range.
>
>> +
>> + ret = regmap_write(sysmon->regmap, SYSMON_TEMP_TH_UP, raw_val);
>> + if (ret)
>> + return ret;
>> +
>> + /* Recompute lower = upper - hysteresis */
>> + return sysmon_update_temp_lower(sysmon);
>> +
>> + case IIO_EV_INFO_HYSTERESIS:
>> + if (val < 0)
>> + return -EINVAL;
>> +
>> + sysmon->temp_hysteresis = val;
>> +
>> + return sysmon_update_temp_lower(sysmon);
>> +
>> + default:
>> + return -EINVAL;
>> + }
>> +
>> + case IIO_VOLTAGE:
>> + offset = sysmon_supply_thresh_offset(chan->address, dir);
>> + if (offset < 0)
>> + return offset;
>> +
>> + ret = regmap_read(sysmon->regmap, offset, ®_val);
>> + if (ret)
>> + return ret;
>> +
>> + sysmon_supply_processedtoraw(val, reg_val, &raw_val);
>
> There is another sashiko report about potential out of range val
> here. Probably also want to check for that just as a way to improve
> useability.
Will add a bounds check before calling sysmon_supply_processedtoraw()
as well.
>
>> +
>> + return regmap_write(sysmon->regmap, offset, raw_val);
> There is also a comment on whether this is wiping out the fields in the
> upper bits of the register. If it isn't (maybe they are read only?)
> then a comment here would be good.
The threshold registers have separate read and write semantics,
the upper bits (FMT/MODE) are returned on read but ignored on
write; only the lower 16-bit mantissa is used. Will add a comment
to make this clear.
Regards,
Salih
>
>> +
>> + default:
>> + return -EINVAL;
>> + }
>> +}
>
^ permalink raw reply
* Re: [PATCH V13 2/9] dt-bindings: iio: imu: icm42600: Add icm42607
From: Chris Morgan @ 2026-06-23 0:06 UTC (permalink / raw)
To: Jean-Baptiste Maneyrol
Cc: Jonathan Cameron, Chris Morgan, linux-iio@vger.kernel.org,
andy@kernel.org, nuno.sa@analog.com, dlechner@baylibre.com,
linux-rockchip@lists.infradead.org, devicetree@vger.kernel.org,
heiko@sntech.de, conor+dt@kernel.org, krzk+dt@kernel.org,
robh@kernel.org, andriy.shevchenko@intel.com, Krzysztof Kozlowski
In-Reply-To: <BE1P281MB1426C557A66945951D382EDDCEEF2@BE1P281MB1426.DEUP281.PROD.OUTLOOK.COM>
On Mon, Jun 22, 2026 at 09:23:28AM +0000, Jean-Baptiste Maneyrol wrote:
> Hello Chris and Jonathan,
>
> concerning dt bindings, my initial understanding was that we had a file per
> driver. But here, Chris is doing a new driver for icm42607 while adding new
> bindings here.
>
> Does it means we don't have 1 binding file per driver, and there is no need
> to create a new binding file for inv_icm42607 driver?
>
> Despite the naming, icm42607 chips are a complete new design very different
> than all other icm42600 chips. It using similar IPs for things like the FIFO,
> but all other parts are different. Especially, it doesn't use banks for
> registers access but indirect access delegated to the chip internals for
> accessing certain registers.
For what it's worth I'm not using any of those registers in the driver
currently; from what I see in the datasheets I was able to find on the
web the 42607p doesn't do the indirect register access (again unless
I'm misreading). To be fair I don't have any other icm42607 chips to
test against. The 42607c does appear to do such register access.
Thank you,
Chris
>
> Thanks,
> JB
>
> >From: Chris Morgan <macromorgan@hotmail.com>
> >
> >Add the ICM42607 and ICM42607P inertial measurement unit.
> >
> >This device is functionally very similar to the icm42600 series with a
> >very different register layout. The driver does not require an
> >interrupt for these specific chip revisions.
> >
> >Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> >Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
> >---
> > .../bindings/iio/imu/invensense,icm42600.yaml | 18 +++++++++++++++++-
> > 1 file changed, 17 insertions(+), 1 deletion(-)
> >
> >diff --git a/Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml b/Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml
> >index 9b2af104f186..81b6e85decd5 100644
> >--- a/Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml
> >+++ b/Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml
> >@@ -30,6 +30,8 @@ properties:
> > - invensense,icm42600
> > - invensense,icm42602
> > - invensense,icm42605
> >+ - invensense,icm42607
> >+ - invensense,icm42607p
> > - invensense,icm42622
> > - invensense,icm42631
> > - invensense,icm42686
> >@@ -67,10 +69,24 @@ properties:
> > required:
> > - compatible
> > - reg
> >- - interrupts
> >
> > allOf:
> > - $ref: /schemas/spi/spi-peripheral-props.yaml#
> >+ - if:
> >+ properties:
> >+ compatible:
> >+ contains:
> >+ enum:
> >+ - invensense,icm42600
> >+ - invensense,icm42602
> >+ - invensense,icm42605
> >+ - invensense,icm42622
> >+ - invensense,icm42631
> >+ - invensense,icm42686
> >+ - invensense,icm42688
> >+ then:
> >+ required:
> >+ - interrupts
> >
> > unevaluatedProperties: false
> >
> >--
> >2.43.0
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox