diff mbox series

[1/2] dt-bindings: hwmon: Introduce ADS71x8

Message ID BE1P281MB24208CB90AF549578AA5C384EF972@BE1P281MB2420.DEUP281.PROD.OUTLOOK.COM (mailing list archive)
State Changes Requested
Headers show
Series [1/2] dt-bindings: hwmon: Introduce ADS71x8 | expand

Commit Message

Sperling, Tobias Aug. 30, 2024, 11:49 a.m. UTC
From b2e04ce5500faf274654be5284be9db4f3abefce Mon Sep 17 00:00:00 2001
From: Tobias Sperling <tobias.sperling@softing.com>
Date: Fri, 23 Aug 2024 12:08:33 +0200
Subject: [PATCH 1/2] dt-bindings: hwmon: Introduce ADS71x8

Add documentation for the driver of ADS7128 and ADS7138 12-bit, 8-channel
analog-to-digital converters. These ADCs have a wide operating range and
a wide feature set. Communication is based on an I2C interface.
The driver provides the functionality of manually reading single channels
or sequentially reading all channels automatically.

Signed-off-by: Tobias Sperling <tobias.sperling@softing.com>
---
 .../devicetree/bindings/hwmon/ti,ads71x8.yaml |  85 +++++++++++
 Documentation/hwmon/ads71x8.rst               | 140 ++++++++++++++++++
 Documentation/hwmon/index.rst                 |   1 +
 3 files changed, 226 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/hwmon/ti,ads71x8.yaml
 create mode 100644 Documentation/hwmon/ads71x8.rst

Comments

Conor Dooley Aug. 30, 2024, 1:14 p.m. UTC | #1
Hey Tobias, Guenter, Jonathan,

On Fri, Aug 30, 2024 at 11:49:53AM +0000, Sperling, Tobias wrote:
> From b2e04ce5500faf274654be5284be9db4f3abefce Mon Sep 17 00:00:00 2001
> From: Tobias Sperling <tobias.sperling@softing.com>
> Date: Fri, 23 Aug 2024 12:08:33 +0200
> Subject: [PATCH 1/2] dt-bindings: hwmon: Introduce ADS71x8
> 
> Add documentation for the driver of ADS7128 and ADS7138 12-bit, 8-channel
> analog-to-digital converters. These ADCs have a wide operating range and
> a wide feature set. Communication is based on an I2C interface.
> The driver provides the functionality of manually reading single channels
> or sequentially reading all channels automatically.
> 
> Signed-off-by: Tobias Sperling <tobias.sperling@softing.com>
> ---
>  .../devicetree/bindings/hwmon/ti,ads71x8.yaml |  85 +++++++++++

If this is a "generic" adc, why is it going into hwmon?
I would have expected this to be in iio/adc, and use more typical adc
bindings, even if the driver is in hwmon.

Guenter/Jonathan wdyt?

>  Documentation/hwmon/ads71x8.rst               | 140 ++++++++++++++++++
>  Documentation/hwmon/index.rst                 |   1 +

And these two documents are not dt-bindings, so they should either be in
their own commit or alongside the driver. Not sure how Guenter likes
things.

>  3 files changed, 226 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/hwmon/ti,ads71x8.yaml

>  create mode 100644 Documentation/hwmon/ads71x8.rst
> 
> diff --git a/Documentation/devicetree/bindings/hwmon/ti,ads71x8.yaml b/Documentation/devicetree/bindings/hwmon/ti,ads71x8.yaml
> new file mode 100644
> index 000000000000..e422c4ebd207
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/ti,ads71x8.yaml

Please make the filename match a compatible.

> @@ -0,0 +1,85 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +
> +$id: http://devicetree.org/schemas/hwmon/ti,ads71x8.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Texas Instruments ADS7128/ADS7138 Analog to Digital Converter (ADC)
> +
> +maintainers:
> +  - None

Nice trick..

> +description: |
> +  The ADS7128 is 12-Bit, 8-Channel Sampling Analog to Digital Converter (ADC)
> +  with an I2C interface.
> +
> +  Datasheets:
> +    https://www.ti.com/product/ADS7128
> +    https://www.ti.com/product/ADS7138
> +
> +properties:
> +  compatible:
> +    enum:
> +      - ti,ads7128
> +      - ti,ads7138
> +
> +  reg:
> +    maxItems: 1
> +
> +  avdd-supply:

There's also a dvdd on the ads7128.

> +    description:
> +      The regulator used as analog supply voltage as well as reference voltage.
> +
> +  ti,mode:
> +    $ref: /schemas/types.yaml#/definitions/uint8
> +    description: |
> +      Operation mode
> +      Mode 0 - Manual mode. A channel is only sampled when the according input
> +        in the sysfs is read.
> +      Mode 1 - Auto mode. All channels are automatically sampled sequentially.
> +        Reading an input returns the last valid sample. In this mode further
> +        features like statistics and interrupts are available.
> +    default: 0

I don't think this ti,mode property is suitable for bindings. sysfs is a
linux implementation detail, when to do sampling is an implementation
detail of your driver. Bindings are only supposed to describe properties
of the hardware, not set software policy.

> +
> +  ti,interval:
> +    $ref: /schemas/types.yaml#/definitions/uint16
> +    description: |
> +      Only considered in mode 1!
> +      Interval in microseconds a new sample is triggered. Is set to closest
> +      possible interval, see datasheet.

For iio devices, this is usually set from userspace, not from
devicetree, because it is usually not a hardware property, but rather
something a user may want to change at runtime.

> +    default: 1
> +
> +  interrupts:
> +    description: |
> +      Only considered in mode 1!
> +      Interrupt specifier the device's ALERT pin is connected to. Level must be
> +      IRQ_TYPE_LEVEL_LOW. If not configured the digital window comparator (DWC)
> +      is not available.
> +    maxItems: 1

You've got 8 channels on the device, so I would be expecting to see
these described here, with a reference to adc.yaml, even if the only
suitable property is "label".

> +
> +required:
> +  - compatible
> +  - reg
> +  - avdd-supply
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        ads7138@10 {

This should just be "dac@".

> +            compatible = "ti,ads7138";
> +            reg = <0x10>;
> +            avdd-supply = <&reg_stb_3v3>;
> +            ti,mode = /bits/ 8 <1>;
> +            ti,interval = /bits/ 16 <1000>;
> +            interrupt-parent = <&gpio2>;
> +            interrupts = <12 IRQ_TYPE_LEVEL_LOW>;
> +            status = "okay";
> +        };
> +    };
oCheers,
Conor.
Guenter Roeck Aug. 30, 2024, 2:30 p.m. UTC | #2
On 8/30/24 06:14, Conor Dooley wrote:
> Hey Tobias, Guenter, Jonathan,
> 
> On Fri, Aug 30, 2024 at 11:49:53AM +0000, Sperling, Tobias wrote:
>>  From b2e04ce5500faf274654be5284be9db4f3abefce Mon Sep 17 00:00:00 2001
>> From: Tobias Sperling <tobias.sperling@softing.com>
>> Date: Fri, 23 Aug 2024 12:08:33 +0200
>> Subject: [PATCH 1/2] dt-bindings: hwmon: Introduce ADS71x8
>>
>> Add documentation for the driver of ADS7128 and ADS7138 12-bit, 8-channel
>> analog-to-digital converters. These ADCs have a wide operating range and
>> a wide feature set. Communication is based on an I2C interface.
>> The driver provides the functionality of manually reading single channels
>> or sequentially reading all channels automatically.
>>
>> Signed-off-by: Tobias Sperling <tobias.sperling@softing.com>
>> ---
>>   .../devicetree/bindings/hwmon/ti,ads71x8.yaml |  85 +++++++++++
> 
> If this is a "generic" adc, why is it going into hwmon?
> I would have expected this to be in iio/adc, and use more typical adc
> bindings, even if the driver is in hwmon.
> 
> Guenter/Jonathan wdyt?
> 

Same thought here. While the chip supports limits, making it suitable for
hardware monitoring, its primary use seems to be as ADC, not as hardware
monitoring device. The hardware monitoring API isn't well suited for the
fast sample rate supported by this chip.

Guenter
Krzysztof Kozlowski Aug. 31, 2024, 6:42 a.m. UTC | #3
On Fri, Aug 30, 2024 at 11:49:53AM +0000, Sperling, Tobias wrote:
> >From b2e04ce5500faf274654be5284be9db4f3abefce Mon Sep 17 00:00:00 2001

Some junk ^^^ above. Please investigate how you send patches.

> From: Tobias Sperling <tobias.sperling@softing.com>
> Date: Fri, 23 Aug 2024 12:08:33 +0200
> Subject: [PATCH 1/2] dt-bindings: hwmon: Introduce ADS71x8

And all this suggests malformed patch.

> 
> Add documentation for the driver of ADS7128 and ADS7138 12-bit, 8-channel
> analog-to-digital converters. These ADCs have a wide operating range and
> a wide feature set. Communication is based on an I2C interface.
> The driver provides the functionality of manually reading single channels
> or sequentially reading all channels automatically.
> 
> Signed-off-by: Tobias Sperling <tobias.sperling@softing.com>
> ---
>  .../devicetree/bindings/hwmon/ti,ads71x8.yaml |  85 +++++++++++
>  Documentation/hwmon/ads71x8.rst               | 140 ++++++++++++++++++
>  Documentation/hwmon/index.rst                 |   1 +

Please run scripts/checkpatch.pl and fix reported warnings. Then please
run  and (probably) fix more warnings.
Some warnings can be ignored, especially from --strict run, but the code
here looks like it needs a fix. Feel free to get in touch if the warning
is not clear.

>  3 files changed, 226 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/hwmon/ti,ads71x8.yaml
>  create mode 100644 Documentation/hwmon/ads71x8.rst
> 
> diff --git a/Documentation/devicetree/bindings/hwmon/ti,ads71x8.yaml b/Documentation/devicetree/bindings/hwmon/ti,ads71x8.yaml
> new file mode 100644
> index 000000000000..e422c4ebd207
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/ti,ads71x8.yaml
> @@ -0,0 +1,85 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +

Drop blank line

> +$id: http://devicetree.org/schemas/hwmon/ti,ads71x8.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Texas Instruments ADS7128/ADS7138 Analog to Digital Converter (ADC)
> +
> +maintainers:
> +  - None

Fidn a person... otherwise why would we care?

> +
> +description: |
> +  The ADS7128 is 12-Bit, 8-Channel Sampling Analog to Digital Converter (ADC)
> +  with an I2C interface.
> +
> +  Datasheets:
> +    https://www.ti.com/product/ADS7128
> +    https://www.ti.com/product/ADS7138
> +
> +properties:
> +  compatible:
> +    enum:
> +      - ti,ads7128
> +      - ti,ads7138
> +
> +  reg:
> +    maxItems: 1
> +
> +  avdd-supply:
> +    description:
> +      The regulator used as analog supply voltage as well as reference voltage.
> +
> +  ti,mode:
> +    $ref: /schemas/types.yaml#/definitions/uint8
> +    description: |
> +      Operation mode
> +      Mode 0 - Manual mode. A channel is only sampled when the according input
> +        in the sysfs is read.
> +      Mode 1 - Auto mode. All channels are automatically sampled sequentially.
> +        Reading an input returns the last valid sample. In this mode further
> +        features like statistics and interrupts are available.

Use string instead.

> +    default: 0
> +
> +  ti,interval:
> +    $ref: /schemas/types.yaml#/definitions/uint16
> +    description: |
> +      Only considered in mode 1!
> +      Interval in microseconds a new sample is triggered. Is set to closest
> +      possible interval, see datasheet.

User proper unit-suffix.

> +    default: 1
> +
> +  interrupts:
> +    description: |
> +      Only considered in mode 1!

What is "considered"? Driver considers? This does not matter. Describe
the hardware and if this is hardware related, you should have
allOf:if:then restricting this.

> +      Interrupt specifier the device's ALERT pin is connected to. Level must be
> +      IRQ_TYPE_LEVEL_LOW. If not configured the digital window comparator (DWC)
> +      is not available.
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - avdd-supply
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        ads7138@10 {

Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation

> +            compatible = "ti,ads7138";
> +            reg = <0x10>;
> +            avdd-supply = <&reg_stb_3v3>;
> +            ti,mode = /bits/ 8 <1>;
> +            ti,interval = /bits/ 16 <1000>;
> +            interrupt-parent = <&gpio2>;
> +            interrupts = <12 IRQ_TYPE_LEVEL_LOW>;
> +            status = "okay";

Drop

Best regards,
Krzysztof
Jonathan Cameron Aug. 31, 2024, 12:18 p.m. UTC | #4
On Fri, 30 Aug 2024 07:30:16 -0700
Guenter Roeck <linux@roeck-us.net> wrote:

> On 8/30/24 06:14, Conor Dooley wrote:
> > Hey Tobias, Guenter, Jonathan,
> > 
> > On Fri, Aug 30, 2024 at 11:49:53AM +0000, Sperling, Tobias wrote:  
> >>  From b2e04ce5500faf274654be5284be9db4f3abefce Mon Sep 17 00:00:00 2001
> >> From: Tobias Sperling <tobias.sperling@softing.com>
> >> Date: Fri, 23 Aug 2024 12:08:33 +0200
> >> Subject: [PATCH 1/2] dt-bindings: hwmon: Introduce ADS71x8
> >>
> >> Add documentation for the driver of ADS7128 and ADS7138 12-bit, 8-channel
> >> analog-to-digital converters. These ADCs have a wide operating range and
> >> a wide feature set. Communication is based on an I2C interface.
> >> The driver provides the functionality of manually reading single channels
> >> or sequentially reading all channels automatically.
> >>
> >> Signed-off-by: Tobias Sperling <tobias.sperling@softing.com>
> >> ---
> >>   .../devicetree/bindings/hwmon/ti,ads71x8.yaml |  85 +++++++++++  
> > 
> > If this is a "generic" adc, why is it going into hwmon?
> > I would have expected this to be in iio/adc, and use more typical adc
> > bindings, even if the driver is in hwmon.
> > 
> > Guenter/Jonathan wdyt?
> >   
> 
> Same thought here. While the chip supports limits, making it suitable for
> hardware monitoring, its primary use seems to be as ADC, not as hardware
> monitoring device. The hardware monitoring API isn't well suited for the
> fast sample rate supported by this chip.

Agreed, looks like a typical IIO ADC.

If the particular board needs it for hardware monitoring we have
the bridge that should work for that (iio-hwmon).

Jonathan

> 
> Guenter
>
Jonathan Cameron Aug. 31, 2024, 12:21 p.m. UTC | #5
> > +  ti,mode:
> > +    $ref: /schemas/types.yaml#/definitions/uint8
> > +    description: |
> > +      Operation mode
> > +      Mode 0 - Manual mode. A channel is only sampled when the according input
> > +        in the sysfs is read.
> > +      Mode 1 - Auto mode. All channels are automatically sampled sequentially.
> > +        Reading an input returns the last valid sample. In this mode further
> > +        features like statistics and interrupts are available.
> > +    default: 0  
> 
> I don't think this ti,mode property is suitable for bindings. sysfs is a
> linux implementation detail, when to do sampling is an implementation
> detail of your driver. Bindings are only supposed to describe properties
> of the hardware, not set software policy.

Agreed. With an IIO driver this will become a switch based on what usespace
interfaces are enabled.
So if events are on or buffered data capture, enable automode.
If just sysfs reads, then manual mode is fine.


> > +
> > +        ads7138@10 {  
> 
> This should just be "dac@".

adc :)

> 
> > +            compatible = "ti,ads7138";
> > +            reg = <0x10>;
> > +            avdd-supply = <&reg_stb_3v3>;
> > +            ti,mode = /bits/ 8 <1>;
> > +            ti,interval = /bits/ 16 <1000>;
> > +            interrupt-parent = <&gpio2>;
> > +            interrupts = <12 IRQ_TYPE_LEVEL_LOW>;
> > +            status = "okay";
> > +        };
> > +    };  
> oCheers,
> Conor.
Sperling, Tobias Sept. 2, 2024, 12:58 p.m. UTC | #6
Hi,
thanks for the feedback in general.

> >  Documentation/hwmon/ads71x8.rst               | 140 ++++++++++++++++++
> >  Documentation/hwmon/index.rst                 |   1 +
> 
> And these two documents are not dt-bindings, so they should either be in
> their own commit or alongside the driver. Not sure how Guenter likes
> things.

Ok, probably misunderstood some documentation then, that everything in
Documentation/ should be a separate commit. Would move it then alongside
the driver, if that's fine from your side.

> > +description: |
> > +  The ADS7128 is 12-Bit, 8-Channel Sampling Analog to Digital Converter (ADC)
> > +  with an I2C interface.
> > +
> > +  Datasheets:
> > +    https://www.ti.com/product/ADS7128
> > +    https://www.ti.com/product/ADS7138
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - ti,ads7128
> > +      - ti,ads7138
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  avdd-supply:
> 
> There's also a dvdd on the ads7128.

Yes, but it doesn't affect anything in the driver. Does it still need to be documented then?
 
> oCheers,
> Conor.

Regards
Tobias
Sperling, Tobias Sept. 2, 2024, 1:04 p.m. UTC | #7
> On Fri, 30 Aug 2024 07:30:16 -0700
> Guenter Roeck <linux@roeck-us.net> wrote:
> 
> > On 8/30/24 06:14, Conor Dooley wrote:
> > > Hey Tobias, Guenter, Jonathan,
> > >
> > > On Fri, Aug 30, 2024 at 11:49:53AM +0000, Sperling, Tobias wrote:
> > >>  From b2e04ce5500faf274654be5284be9db4f3abefce Mon Sep 17 00:00:00
> 2001
> > >> From: Tobias Sperling <tobias.sperling@softing.com>
> > >> Date: Fri, 23 Aug 2024 12:08:33 +0200
> > >> Subject: [PATCH 1/2] dt-bindings: hwmon: Introduce ADS71x8
> > >>
> > >> Add documentation for the driver of ADS7128 and ADS7138 12-bit, 8-channel
> > >> analog-to-digital converters. These ADCs have a wide operating range and
> > >> a wide feature set. Communication is based on an I2C interface.
> > >> The driver provides the functionality of manually reading single channels
> > >> or sequentially reading all channels automatically.
> > >>
> > >> Signed-off-by: Tobias Sperling <tobias.sperling@softing.com>
> > >> ---
> > >>   .../devicetree/bindings/hwmon/ti,ads71x8.yaml |  85 +++++++++++
> > >
> > > If this is a "generic" adc, why is it going into hwmon?
> > > I would have expected this to be in iio/adc, and use more typical adc
> > > bindings, even if the driver is in hwmon.
> > >
> > > Guenter/Jonathan wdyt?
> > >
> >
> > Same thought here. While the chip supports limits, making it suitable for
> > hardware monitoring, its primary use seems to be as ADC, not as hardware
> > monitoring device. The hardware monitoring API isn't well suited for the
> > fast sample rate supported by this chip.
> 
> Agreed, looks like a typical IIO ADC.
> 
> If the particular board needs it for hardware monitoring we have
> the bridge that should work for that (iio-hwmon).

Just some addition. In theory the chip also provides the possibility to use some
channels as GPIO making it not only work as ADC.
But yes, driver mainly implements reading of the ADC. Will try to make it an
IIO ADC device then.

> Jonathan
> 
> >
> > Guenter
> >

Regards
Tobias
Sperling, Tobias Sept. 2, 2024, 1:24 p.m. UTC | #8
> > > +  ti,mode:
> > > +    $ref: /schemas/types.yaml#/definitions/uint8
> > > +    description: |
> > > +      Operation mode
> > > +      Mode 0 - Manual mode. A channel is only sampled when the according
> input
> > > +        in the sysfs is read.
> > > +      Mode 1 - Auto mode. All channels are automatically sampled
> sequentially.
> > > +        Reading an input returns the last valid sample. In this mode further
> > > +        features like statistics and interrupts are available.
> > > +    default: 0
> >
> > I don't think this ti,mode property is suitable for bindings. sysfs is a
> > linux implementation detail, when to do sampling is an implementation
> > detail of your driver. Bindings are only supposed to describe properties
> > of the hardware, not set software policy.
> 
> Agreed. With an IIO driver this will become a switch based on what usespace
> interfaces are enabled.
> So if events are on or buffered data capture, enable automode.
> If just sysfs reads, then manual mode is fine.

Not quite sure if I understood you correctly. With the mode I intended to give
control about the sampling behavior.
In manual mode channels are only sampled if they are accessed/read.
In auto mode they are sampled all the time sequentially. This also offers to use
some extended features, like triggering an interrupt if a measurement crosses a
defined limit.
So the mode mainly affects the hardware behavior and just offers the possibility
to catch that in userspace, if configured accordingly, but that's not a must-have.

Anyway, did I understood it correctly, that you suggest to configure the mode
according some symbols in the kconfig and check that with #ifdef? Do you have
the specific symbol names for me or a driver as example, so I can have a look?

Thanks and regards
Tobias
Sperling, Tobias Sept. 2, 2024, 1:48 p.m. UTC | #9
> On Fri, Aug 30, 2024 at 11:49:53AM +0000, Sperling, Tobias wrote:
> > >From b2e04ce5500faf274654be5284be9db4f3abefce Mon Sep 17 00:00:00
> 2001
> 
> Some junk ^^^ above. Please investigate how you send patches.

Yeah also saw this line, but of course tried to apply the patch again after sending it
as mail and that worked fine. But just checked again and seems like this line can be
dropped.
And yes, I sent the patches manually, as we likely have some restrictions for smtp,
but as I was able to apply them again it's fine I guess.

> > From: Tobias Sperling <tobias.sperling@softing.com>
> > Date: Fri, 23 Aug 2024 12:08:33 +0200
> > Subject: [PATCH 1/2] dt-bindings: hwmon: Introduce ADS71x8
> 
> And all this suggests malformed patch.

Why? If I drop this I'm not able to apply the patch, so I think this should be fine.

> >
> > Add documentation for the driver of ADS7128 and ADS7138 12-bit, 8-channel
> > analog-to-digital converters. These ADCs have a wide operating range and
> > a wide feature set. Communication is based on an I2C interface.
> > The driver provides the functionality of manually reading single channels
> > or sequentially reading all channels automatically.
> >
> > Signed-off-by: Tobias Sperling <tobias.sperling@softing.com>
> > ---
> >  .../devicetree/bindings/hwmon/ti,ads71x8.yaml |  85 +++++++++++
> >  Documentation/hwmon/ads71x8.rst               | 140 ++++++++++++++++++
> >  Documentation/hwmon/index.rst                 |   1 +
> 
> Please run scripts/checkpatch.pl and fix reported warnings. Then please
> run  and (probably) fix more warnings.
> Some warnings can be ignored, especially from --strict run, but the code
> here looks like it needs a fix. Feel free to get in touch if the warning
> is not clear.

Had done this already before submitting the patches (at least without --strict),
but only reports a warning about splitting the patch (which I got wrong here)
and updating the maintainers.
I guess you were about suggesting a second script to run. Which one is that?

> > +$id:
> https://deu01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevicetree.
> org%2Fschemas%2Fhwmon%2Fti%2Cads71x8.yaml%23&data=05%7C02%7C%7C
> ff09fedbe2744394f78508dcc9881ee7%7Cfe3606fad3974238999768dcd7851f64
> %7C1%7C0%7C638606833686313557%7CUnknown%7CTWFpbGZsb3d8eyJWIjoi
> MC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C
> %7C%7C&sdata=vZaCpdaNzELpNNnd6wp5P9MNLQTnAmWXYD%2BNKQYCJ78%
> 3D&reserved=0
> > +$schema:
> https://deu01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevicetree.
> org%2Fmeta-
> schemas%2Fcore.yaml%23&data=05%7C02%7C%7Cff09fedbe2744394f78508dcc
> 9881ee7%7Cfe3606fad3974238999768dcd7851f64%7C1%7C0%7C63860683368
> 6326954%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2lu
> MzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=EJflznuTZYGR
> BRjULwohiHk8gPF9iRusSbmF8CKkl5Q%3D&reserved=0
> > +
> > +title: Texas Instruments ADS7128/ADS7138 Analog to Digital Converter (ADC)
> > +
> > +maintainers:
> > +  - None
> 
> Fidn a person... otherwise why would we care?

I know it's not nice, but we are likely not implementing further features, but OK, will
add myself then.

> > +    default: 1
> > +
> > +  interrupts:
> > +    description: |
> > +      Only considered in mode 1!
> 
> What is "considered"? Driver considers? This does not matter. Describe
> the hardware and if this is hardware related, you should have
> allOf:if:then restricting this.

It's possible to define a mode, either manual sampling or autosampling. In the
latter mode, also the hardware capabilities change, e.g. the driver is able to
trigger an interrupt so defining the interrupt only makes sense in that mode.
Will have a look to allOf:if:then then.

> > +            compatible = "ti,ads7138";
> > +            reg = <0x10>;
> > +            avdd-supply = <&reg_stb_3v3>;
> > +            ti,mode = /bits/ 8 <1>;
> > +            ti,interval = /bits/ 16 <1000>;
> > +            interrupt-parent = <&gpio2>;
> > +            interrupts = <12 IRQ_TYPE_LEVEL_LOW>;
> > +            status = "okay";
> 
> Drop

I guess, I shall only drop the "status" not the whole section?

> Best regards,
> Krzysztof

Regards,
Tobias
Guenter Roeck Sept. 2, 2024, 1:49 p.m. UTC | #10
On 9/2/24 05:58, Sperling, Tobias wrote:
> Hi,
> thanks for the feedback in general.
> 
>>>   Documentation/hwmon/ads71x8.rst               | 140 ++++++++++++++++++
>>>   Documentation/hwmon/index.rst                 |   1 +
>>
>> And these two documents are not dt-bindings, so they should either be in
>> their own commit or alongside the driver. Not sure how Guenter likes
>> things.
> 
> Ok, probably misunderstood some documentation then, that everything in
> Documentation/ should be a separate commit. Would move it then alongside

Sure, but it doesn't say that it should be a _single_ separate commit.

Guenter
Jonathan Cameron Sept. 2, 2024, 2:17 p.m. UTC | #11
On Mon, 2 Sep 2024 13:24:59 +0000
"Sperling, Tobias" <Tobias.Sperling@Softing.com> wrote:

> > > > +  ti,mode:
> > > > +    $ref: /schemas/types.yaml#/definitions/uint8
> > > > +    description: |
> > > > +      Operation mode
> > > > +      Mode 0 - Manual mode. A channel is only sampled when the according  
> > input  
> > > > +        in the sysfs is read.
> > > > +      Mode 1 - Auto mode. All channels are automatically sampled  
> > sequentially.  
> > > > +        Reading an input returns the last valid sample. In this mode further
> > > > +        features like statistics and interrupts are available.
> > > > +    default: 0  
> > >
> > > I don't think this ti,mode property is suitable for bindings. sysfs is a
> > > linux implementation detail, when to do sampling is an implementation
> > > detail of your driver. Bindings are only supposed to describe properties
> > > of the hardware, not set software policy.  
> > 
> > Agreed. With an IIO driver this will become a switch based on what usespace
> > interfaces are enabled.
> > So if events are on or buffered data capture, enable automode.
> > If just sysfs reads, then manual mode is fine.  
> 
> Not quite sure if I understood you correctly. With the mode I intended to give
> control about the sampling behavior.
> In manual mode channels are only sampled if they are accessed/read.
> In auto mode they are sampled all the time sequentially. This also offers to use
> some extended features, like triggering an interrupt if a measurement crosses a
> defined limit.
> So the mode mainly affects the hardware behavior and just offers the possibility
> to catch that in userspace, if configured accordingly, but that's not a must-have.
> 
> Anyway, did I understood it correctly, that you suggest to configure the mode
> according some symbols in the kconfig and check that with #ifdef? Do you have
> the specific symbol names for me or a driver as example, so I can have a look?
No, this is not a build time of firmware config question. It is a question of
what the user is 'doing' with the device. Configure the mode according to what
userspace has enabled.

If it enables threshold detection, then turn on continuous mode.
If it enables capture of data via a chardev (so fast path) then turn on continuous
mode.  If neither of those, then run in manual mode.

Jonathan

> 
> Thanks and regards
> Tobias
>
Krzysztof Kozlowski Sept. 2, 2024, 4:02 p.m. UTC | #12
On 02/09/2024 15:48, Sperling, Tobias wrote:
>> On Fri, Aug 30, 2024 at 11:49:53AM +0000, Sperling, Tobias wrote:
>>> >From b2e04ce5500faf274654be5284be9db4f3abefce Mon Sep 17 00:00:00
>> 2001
>>
>> Some junk ^^^ above. Please investigate how you send patches.
> 
> Yeah also saw this line, but of course tried to apply the patch again after sending it
> as mail and that worked fine. But just checked again and seems like this line can be
> dropped.
> And yes, I sent the patches manually, as we likely have some restrictions for smtp,
> but as I was able to apply them again it's fine I guess.
> 
>>> From: Tobias Sperling <tobias.sperling@softing.com>
>>> Date: Fri, 23 Aug 2024 12:08:33 +0200
>>> Subject: [PATCH 1/2] dt-bindings: hwmon: Introduce ADS71x8
>>
>> And all this suggests malformed patch.
> 
> Why? If I drop this I'm not able to apply the patch, so I think this should be fine.

OK, it works with b4, but seeing duplicated subject is not expected and
might not work with all tools.

> 
>>>
>>> Add documentation for the driver of ADS7128 and ADS7138 12-bit, 8-channel
>>> analog-to-digital converters. These ADCs have a wide operating range and
>>> a wide feature set. Communication is based on an I2C interface.
>>> The driver provides the functionality of manually reading single channels
>>> or sequentially reading all channels automatically.
>>>
>>> Signed-off-by: Tobias Sperling <tobias.sperling@softing.com>
>>> ---
>>>  .../devicetree/bindings/hwmon/ti,ads71x8.yaml |  85 +++++++++++
>>>  Documentation/hwmon/ads71x8.rst               | 140 ++++++++++++++++++
>>>  Documentation/hwmon/index.rst                 |   1 +
>>
>> Please run scripts/checkpatch.pl and fix reported warnings. Then please
>> run  and (probably) fix more warnings.
>> Some warnings can be ignored, especially from --strict run, but the code
>> here looks like it needs a fix. Feel free to get in touch if the warning
>> is not clear.
> 
> Had done this already before submitting the patches (at least without --strict),
> but only reports a warning about splitting the patch (which I got wrong here)
> and updating the maintainers.
> I guess you were about suggesting a second script to run. Which one is that?

Please split the patches.

> 
>>> +$id:
>> https://deu01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevicetree.
>> org%2Fschemas%2Fhwmon%2Fti%2Cads71x8.yaml%23&data=05%7C02%7C%7C
>> ff09fedbe2744394f78508dcc9881ee7%7Cfe3606fad3974238999768dcd7851f64
>> %7C1%7C0%7C638606833686313557%7CUnknown%7CTWFpbGZsb3d8eyJWIjoi
>> MC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C
>> %7C%7C&sdata=vZaCpdaNzELpNNnd6wp5P9MNLQTnAmWXYD%2BNKQYCJ78%
>> 3D&reserved=0
>>> +$schema:
>> https://deu01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevicetree.
>> org%2Fmeta-
>> schemas%2Fcore.yaml%23&data=05%7C02%7C%7Cff09fedbe2744394f78508dcc
>> 9881ee7%7Cfe3606fad3974238999768dcd7851f64%7C1%7C0%7C63860683368
>> 6326954%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2lu
>> MzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=EJflznuTZYGR
>> BRjULwohiHk8gPF9iRusSbmF8CKkl5Q%3D&reserved=0
>>> +
>>> +title: Texas Instruments ADS7128/ADS7138 Analog to Digital Converter (ADC)
>>> +
>>> +maintainers:
>>> +  - None
>>
>> Fidn a person... otherwise why would we care?
> 
> I know it's not nice, but we are likely not implementing further features, but OK, will
> add myself then.
> 
>>> +    default: 1
>>> +
>>> +  interrupts:
>>> +    description: |
>>> +      Only considered in mode 1!
>>
>> What is "considered"? Driver considers? This does not matter. Describe
>> the hardware and if this is hardware related, you should have
>> allOf:if:then restricting this.
> 
> It's possible to define a mode, either manual sampling or autosampling. In the
> latter mode, also the hardware capabilities change, e.g. the driver is able to
> trigger an interrupt so defining the interrupt only makes sense in that mode.
> Will have a look to allOf:if:then then.
> 
>>> +            compatible = "ti,ads7138";
>>> +            reg = <0x10>;
>>> +            avdd-supply = <&reg_stb_3v3>;
>>> +            ti,mode = /bits/ 8 <1>;
>>> +            ti,interval = /bits/ 16 <1000>;
>>> +            interrupt-parent = <&gpio2>;
>>> +            interrupts = <12 IRQ_TYPE_LEVEL_LOW>;
>>> +            status = "okay";
>>
>> Drop
> 
> I guess, I shall only drop the "status" not the whole section?

Only status

Best regards,
Krzysztof
Guenter Roeck Sept. 2, 2024, 5:01 p.m. UTC | #13
On 9/2/24 09:02, Krzysztof Kozlowski wrote:
> On 02/09/2024 15:48, Sperling, Tobias wrote:
>>> On Fri, Aug 30, 2024 at 11:49:53AM +0000, Sperling, Tobias wrote:
>>>> >From b2e04ce5500faf274654be5284be9db4f3abefce Mon Sep 17 00:00:00
>>> 2001
>>>
>>> Some junk ^^^ above. Please investigate how you send patches.
>>
>> Yeah also saw this line, but of course tried to apply the patch again after sending it
>> as mail and that worked fine. But just checked again and seems like this line can be
>> dropped.
>> And yes, I sent the patches manually, as we likely have some restrictions for smtp,
>> but as I was able to apply them again it's fine I guess.
>>
>>>> From: Tobias Sperling <tobias.sperling@softing.com>
>>>> Date: Fri, 23 Aug 2024 12:08:33 +0200
>>>> Subject: [PATCH 1/2] dt-bindings: hwmon: Introduce ADS71x8
>>>
>>> And all this suggests malformed patch.
>>
>> Why? If I drop this I'm not able to apply the patch, so I think this should be fine.
> 
> OK, it works with b4, but seeing duplicated subject is not expected and
> might not work with all tools.
> 
>>
>>>>
>>>> Add documentation for the driver of ADS7128 and ADS7138 12-bit, 8-channel
>>>> analog-to-digital converters. These ADCs have a wide operating range and
>>>> a wide feature set. Communication is based on an I2C interface.
>>>> The driver provides the functionality of manually reading single channels
>>>> or sequentially reading all channels automatically.
>>>>
>>>> Signed-off-by: Tobias Sperling <tobias.sperling@softing.com>
>>>> ---
>>>>   .../devicetree/bindings/hwmon/ti,ads71x8.yaml |  85 +++++++++++
>>>>   Documentation/hwmon/ads71x8.rst               | 140 ++++++++++++++++++
>>>>   Documentation/hwmon/index.rst                 |   1 +
>>>
>>> Please run scripts/checkpatch.pl and fix reported warnings. Then please
>>> run  and (probably) fix more warnings.
>>> Some warnings can be ignored, especially from --strict run, but the code
>>> here looks like it needs a fix. Feel free to get in touch if the warning
>>> is not clear.
>>
>> Had done this already before submitting the patches (at least without --strict),
>> but only reports a warning about splitting the patch (which I got wrong here)
>> and updating the maintainers.
>> I guess you were about suggesting a second script to run. Which one is that?
> 
> Please split the patches.
> 

I suspect you may have to be more specific. There need to be two patches
in Documentation/, one with the bindings and another one with the driver
documentation.

Thanks,
Guenter
Jonathan Cameron Sept. 3, 2024, 7:40 p.m. UTC | #14
On Mon, 2 Sep 2024 13:04:55 +0000
"Sperling, Tobias" <Tobias.Sperling@Softing.com> wrote:

> > On Fri, 30 Aug 2024 07:30:16 -0700
> > Guenter Roeck <linux@roeck-us.net> wrote:
> >   
> > > On 8/30/24 06:14, Conor Dooley wrote:  
> > > > Hey Tobias, Guenter, Jonathan,
> > > >
> > > > On Fri, Aug 30, 2024 at 11:49:53AM +0000, Sperling, Tobias wrote:  
> > > >>  From b2e04ce5500faf274654be5284be9db4f3abefce Mon Sep 17 00:00:00  
> > 2001  
> > > >> From: Tobias Sperling <tobias.sperling@softing.com>
> > > >> Date: Fri, 23 Aug 2024 12:08:33 +0200
> > > >> Subject: [PATCH 1/2] dt-bindings: hwmon: Introduce ADS71x8
> > > >>
> > > >> Add documentation for the driver of ADS7128 and ADS7138 12-bit, 8-channel
> > > >> analog-to-digital converters. These ADCs have a wide operating range and
> > > >> a wide feature set. Communication is based on an I2C interface.
> > > >> The driver provides the functionality of manually reading single channels
> > > >> or sequentially reading all channels automatically.
> > > >>
> > > >> Signed-off-by: Tobias Sperling <tobias.sperling@softing.com>
> > > >> ---
> > > >>   .../devicetree/bindings/hwmon/ti,ads71x8.yaml |  85 +++++++++++  
> > > >
> > > > If this is a "generic" adc, why is it going into hwmon?
> > > > I would have expected this to be in iio/adc, and use more typical adc
> > > > bindings, even if the driver is in hwmon.
> > > >
> > > > Guenter/Jonathan wdyt?
> > > >  
> > >
> > > Same thought here. While the chip supports limits, making it suitable for
> > > hardware monitoring, its primary use seems to be as ADC, not as hardware
> > > monitoring device. The hardware monitoring API isn't well suited for the
> > > fast sample rate supported by this chip.  
> > 
> > Agreed, looks like a typical IIO ADC.
> > 
> > If the particular board needs it for hardware monitoring we have
> > the bridge that should work for that (iio-hwmon).  
> 
> Just some addition. In theory the chip also provides the possibility to use some
> channels as GPIO making it not only work as ADC.
> But yes, driver mainly implements reading of the ADC. Will try to make it an
> IIO ADC device then.
That's fairly common.  If you want to support it then provide the gpio_chip
etc and +CC the relevant maintainers and mailing lists.

Jonathan

> 
> > Jonathan
> >   
> > >
> > > Guenter
> > >  
> 
> Regards
> Tobias
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/hwmon/ti,ads71x8.yaml b/Documentation/devicetree/bindings/hwmon/ti,ads71x8.yaml
new file mode 100644
index 000000000000..e422c4ebd207
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/ti,ads71x8.yaml
@@ -0,0 +1,85 @@ 
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+
+$id: http://devicetree.org/schemas/hwmon/ti,ads71x8.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Texas Instruments ADS7128/ADS7138 Analog to Digital Converter (ADC)
+
+maintainers:
+  - None
+
+description: |
+  The ADS7128 is 12-Bit, 8-Channel Sampling Analog to Digital Converter (ADC)
+  with an I2C interface.
+
+  Datasheets:
+    https://www.ti.com/product/ADS7128
+    https://www.ti.com/product/ADS7138
+
+properties:
+  compatible:
+    enum:
+      - ti,ads7128
+      - ti,ads7138
+
+  reg:
+    maxItems: 1
+
+  avdd-supply:
+    description:
+      The regulator used as analog supply voltage as well as reference voltage.
+
+  ti,mode:
+    $ref: /schemas/types.yaml#/definitions/uint8
+    description: |
+      Operation mode
+      Mode 0 - Manual mode. A channel is only sampled when the according input
+        in the sysfs is read.
+      Mode 1 - Auto mode. All channels are automatically sampled sequentially.
+        Reading an input returns the last valid sample. In this mode further
+        features like statistics and interrupts are available.
+    default: 0
+
+  ti,interval:
+    $ref: /schemas/types.yaml#/definitions/uint16
+    description: |
+      Only considered in mode 1!
+      Interval in microseconds a new sample is triggered. Is set to closest
+      possible interval, see datasheet.
+    default: 1
+
+  interrupts:
+    description: |
+      Only considered in mode 1!
+      Interrupt specifier the device's ALERT pin is connected to. Level must be
+      IRQ_TYPE_LEVEL_LOW. If not configured the digital window comparator (DWC)
+      is not available.
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - avdd-supply
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        ads7138@10 {
+            compatible = "ti,ads7138";
+            reg = <0x10>;
+            avdd-supply = <&reg_stb_3v3>;
+            ti,mode = /bits/ 8 <1>;
+            ti,interval = /bits/ 16 <1000>;
+            interrupt-parent = <&gpio2>;
+            interrupts = <12 IRQ_TYPE_LEVEL_LOW>;
+            status = "okay";
+        };
+    };
diff --git a/Documentation/hwmon/ads71x8.rst b/Documentation/hwmon/ads71x8.rst
new file mode 100644
index 000000000000..383669c1f8c5
--- /dev/null
+++ b/Documentation/hwmon/ads71x8.rst
@@ -0,0 +1,140 @@ 
+.. SPDX-License-Identifier: GPL-2.0-or-later
+
+Kernel driver ads71x8
+=====================
+
+Supported chips:
+
+  * Texas Instruments ADS7138
+
+	Prefix: 'ads7128'
+
+	Datasheet: Publicly available at the Texas Instruments website:
+	http://focus.ti.com/lit/ds/symlink/ads7128.pdf
+
+  * Texas Instruments ADS7138
+
+	Prefix: 'ads7138'
+
+	Datasheet: Publicly available at the Texas Instruments website:
+	http://focus.ti.com/lit/ds/symlink/ads7138.pdf
+
+Author: Tobias Sperling <tobias.sperling@softing.com>
+	(based on ads7828 by Steve Hardy)
+
+Description
+-----------
+
+This driver implements support for the Texas Instruments ADS7128 and ADS7138,
+which are 8-channel 12-bit A/D converters.
+
+The chip requires an external analog supply voltage AVDD which is also used as
+reference voltage. If it is missing or too low, the chip won't show up as I2C
+device.
+
+The driver can be run in different modes. In manual mode a new (averaged) sample
+is created when the according input is read.
+
+In auto mode all channels are sampled sequentially automatically. Reading an
+input returns the last valid sample. In this mode there are also further
+features like statistics and the possibility to trigger an interrupt if a
+voltage drops/raises below/above a specific value (DWC - Digital Window
+Comparator).
+The overall update time (after which all channels are updated) depends on the
+number of samples, the update interval and the amount of channels (8).
+
+	update time = samples * update_interval * 8
+
+There is no reliable way to identify this chip, so the driver will not scan
+some addresses to try to auto-detect it. That means that you will have to
+statically declare the device in the device tree.
+
+sysfs-Interface
+---------------
+
+The following interfaces are available in all modes.
+
++----------------+----+---------------------------------------------+
+| in[0-7]_input  | ro | Voltage in mV sampled at channel [0-7]      |
++----------------+----+---------------------------------------------+
+| samples        | rw | Number of samples used for averaging 1-128. |
+|                |    | Automatically set to closest power of 2.    |
++----------------+----+---------------------------------------------+
+| calibrate      | rw | Write any value greater than 0 to trigger   |
+|                |    | self-calibration. Reads as 0 if finished.   |
++----------------+----+---------------------------------------------+
+
+If the device is running in auto mode there are also the following interfaces.
+
++------------------+----+-----------------------------------------------------+
+| in[0-7]_max      | ro | Maximum value in mV that occurred at channel [0-7]  |
++------------------+----+-----------------------------------------------------+
+| in[0-7]_min      | ro | Minimal value in mV that occurred at channel [0-7]  |
++------------------+----+-----------------------------------------------------+
+| update_interval  | ro | Time in microseconds after which the next sample is |
+|                  |    | executed.                                           |
++------------------+----+-----------------------------------------------------+
+
+If the device is running in auto mode and the interrupt is configured also the
+following interfaces are added. If CONFIG_SYSFS is set in the kernel
+configuration it is also possible to poll the 'alrarms', see example below.
+
++--------------------+----+---------------------------------------------------+
+| alarms             | ro | | Contains the flags of DWC events. Once read it  |
+|                    |    |   is reset to 0.                                  |
+|                    |    | | BIT0 equals the low event flag of channel 0.    |
+|                    |    | | BIT7 equals the low event flag of channel 7.    |
+|                    |    | | BIT8 equals the high event flag of channel 0.   |
+|                    |    | | BIT15 equals the high event flag of channel 7.  |
++--------------------+----+---------------------------------------------------+
+| in[0-7]_max_alarm  | rw | Set high threshold in mV of DWC for channel [0-7] |
++--------------------+----+---------------------------------------------------+
+| in[0-7]_min_alarm  | rw | Set low threshold in mV of DWC for channel [0-7]  |
++--------------------+----+---------------------------------------------------+
+
+Example
+-------
+
+.. code:: c
+
+	#include <stdio.h>
+	#include <stdlib.h>
+	#include <fcntl.h>
+	#include <sys/select.h>
+	#include <unistd.h>
+
+	int main(void)
+	{
+		int		retval, fd;
+		fd_set	exceptfds;
+		char	buf[16];
+
+		fd = open("/sys/class/hwmon/hwmon1/alarms", O_RDONLY);
+
+		while (1) {
+
+			FD_ZERO(&exceptfds);
+			FD_SET(fd, &exceptfds);
+
+			/* Must be assigned to 'exceptional conditions'. For poll() use
+				POLLPRI. */
+			retval = select(fd + 1, NULL, NULL, &exceptfds, NULL);
+			if (retval == -1)
+				perror("select()");
+			else if (retval) {
+				/* Close and reopen is required, since it's a sysfs file */
+				close(fd);
+				fd = open("/sys/class/hwmon/hwmon1/alarms", O_RDONLY);
+				retval = read(fd, buf, sizeof(buf));
+				printf("Received: %.*s\n", retval,buf);
+			}
+		}
+
+	close(fd);
+	exit(EXIT_SUCCESS);
+	}
+
+Notes
+-----
+
+TODO support for GPIOs, ADC hysteresis and counts is missing yet.
diff --git a/Documentation/hwmon/index.rst b/Documentation/hwmon/index.rst
index 913c11390a45..a54df7af27ea 100644
--- a/Documentation/hwmon/index.rst
+++ b/Documentation/hwmon/index.rst
@@ -33,6 +33,7 @@  Hardware Monitoring Kernel Drivers
    adm1275
    adm9240
    adp1050
+   ads71x8
    ads7828
    adt7410
    adt7411