[2/2] dt-bindings: iio: adc: devicetree bindings for texas instruments ads7142 iio driver

Commit Message

József Horváth May 1, 2021, 6:25 p.m. UTC

This is a device tree schema for iio driver for
 Texas Instruments ADS7142 dual-channel, programmable sensor monitor.

Datasheet: https://www.ti.com/lit/ds/symlink/ads7142.pdf

Signed-off-by: Jozsef Horvath <[e-mail]>
---
---
 .../bindings/iio/adc/ti,ads7142.yaml          | 198 ++++++++++++++++++
 1 file changed, 198 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/ti,ads7142.yaml

Comments

Jonathan Cameron May 2, 2021, 5:22 p.m. UTC | #1

On Sat, 1 May 2021 18:25:18 +0000
Jozsef Horvath <[e-mail]> wrote:

> This is a device tree schema for iio driver for
>  Texas Instruments ADS7142 dual-channel, programmable sensor monitor.
> 
> Datasheet: https://www.ti.com/lit/ds/symlink/ads7142.pdf
> 
> Signed-off-by: Jozsef Horvath <[e-mail]>
> ---
> ---
>  .../bindings/iio/adc/ti,ads7142.yaml          | 198 ++++++++++++++++++
>  1 file changed, 198 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/ti,ads7142.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/ti,ads7142.yaml b/Documentation/devicetree/bindings/iio/adc/ti,ads7142.yaml
> new file mode 100644
> index 000000000000..b4e752160156
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/ti,ads7142.yaml
> @@ -0,0 +1,198 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/iio/adc/ti,ads7142.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: Texas Instruments ADS7142 adc driver device tree bindings
> +
> +maintainers:
> +  - József Horváth <[e-mail]>
> +
> +description: |
> +  This document is for describing the required device tree parameters
> +   for ads7142 adc driver.

Document describes hardware, not a particular driver.  So just refer
to the device here.  There may well be other drives in future using
the same binding (e.g. in an RTOS).

> +  The required parameters for proper operation are described below.
> +
> +  Datasheet: https://www.ti.com/lit/ds/symlink/ads7142.pdf
> +
> +  Operation modes supportedby the driver:
> +    When the 'ti,monitoring-mode' property is not present
> +      in the devicetree node definition, the driver initiates a single
> +      conversion in the device for each read request
> +      (/sys/bus/iio/devices/iio:deviceX/in_voltageY_raw).
> +      This is a one-shot conversion, and it is called
> +      "Manual Mode" in the datasheet.
> +
> +    When the 'ti,monitoring-mode' property is present
> +      in the devicetree node definition, the driver configures
> +      the device's digital window comparator and sets the device's
> +      data buffer operation mode to pre alert data mode.
> +      The driver reads the conversion result when the BUSY/RDY interrupt
> +      fires, and keeps the value until the next BUSY/RDY interrupt
> +      or the first read request
> +      (/sys/bus/iio/devices/iio:deviceX/in_voltageY_raw).
> +      The digital window comparator and hysteresis parameters
> +      can be controlled by:
> +        - the devicetree definition of channel node
> +        - iio sysfs interfaces
> +      This is event driven conversion, and is called
> +      "Autonomous Mode with Pre Alert Data" in the datasheet.
> +      This mode can be used to wake up the system with the ALERT pin,
> +      in case when the monitored voltage level is out of the configured range.

I talked about these in the driver review so look there for comments.
Short summary is this is something userspace should have control off (assuming irq
is wired up) not dt.

> +
> +properties:
> +  compatible:
> +    const: ti,ads7142
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    description: |
> +      The BUSY/PDY pin is used as interrupt line in autonomous monitoring mode.
> +    maxItems: 1
> +
> +  vref-supply:
> +    description: Regulator for the reference voltage
> +
> +  power-supply: true

These don't match the naming on the pin diagram.

> +
> +  "#address-cells":
> +    const: 1
> +
> +  "#size-cells":
> +    const: 0
> +
> +  "#io-channel-cells":
> +    const: 1
> +
> +  ti,osc-sel:
> +    description: |
> +      If present, the driver selects the high speed oscillator.
> +      See chapter 7.3.5 Oscillator and Timing Control in datasheet.
> +    type: boolean

This looks connected to the possible sampling frequencies when in various autonomous modes.
Should it be controlled by userspace?

> +
> +  ti,n-clk:
> +    description: |
> +      nCLK is number of clocks in one conversion cycle.
> +      See chapter 7.3.5 Oscillator and Timing Control in datasheet.

Sounds like a policy decision for userspace.

> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    maximum: 255
> +    minimum: 0
> +
> +  ti,monitoring-mode:
> +    description: |
> +      If present, the driver selects the autonomous monitoring mode with pre alert data.
> +      See chapter 7.4 Device Functional Modes in datasheet.

As mentioned in the driver review, this looks like something we should control from userspace
not dt to me.

> +    type: boolean
> +
> +patternProperties:
> +  "^channel@[0-1]$":
> +    $ref: "adc.yaml"
> +    type: object
> +    description: |
> +      Represents the external channels which are connected to the ADC.
> +    properties:
> +      reg:
> +        description: |
> +          The channel number.
> +        items:
> +          minimum: 0
> +          maximum: 1
> +      "ti,threshold-falling":
> +        description: The low threshold for channel

For these, we need a strong argument presented in this doc for why they are not
a question of policy (and hence why they should be in dt at all).

> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        maximum: 4095
> +        minimum: 0
> +      "ti,threshold-rising":
> +        description: The high threshold for channel
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        maximum: 4095
> +        minimum: 0
> +      "ti,hysteresis":
> +        description: The hysteresis for both comparators for channel
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        maximum: 63
> +        minimum: 0
> +
> +    required:
> +      - reg
> +
> +    additionalProperties: false
> +
> +allOf:
> +  - if:
> +      required:
> +        - ti,monitoring-mode
> +    then:
> +      required:
> +        - interrupts
> +
> +required:
> +  - compatible
> +  - "#io-channel-cells"
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    i2c {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +      adc@18 {

I would not bother having two examples.  The second one covers more things afterall
and the binding makes it clear what is required.

> +        compatible = "ti,ads7142";
> +        reg = <0x18>;
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +        #io-channel-cells = <1>;
> +
> +        vref-supply = <&vdd_3v3_reg>;
> +        power-supply = <&vdd_1v8_reg>;
> +
> +        channel@0 {
> +          reg = <0>;
> +        };
> +
> +        channel@1 {
> +          reg = <1>;
> +        };
> +      };
> +    };
> +  - |
> +    i2c {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +      adc@1f {
> +        compatible = "ti,ads7142";
> +        reg = <0x1f>;
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        #io-channel-cells = <1>;
> +
> +        vref-supply = <&vdd_3v3_reg>;
> +        power-supply = <&vdd_1v8_reg>;
> +
> +        interrupt-parent = <&gpio>;
> +        interrupts = <7 2>;
> +
> +        ti,monitoring-mode;
> +
> +        channel@0 {
> +          reg = <0>;
> +          ti,threshold-falling = <1000>;
> +          ti,threshold-rising = <2000>;
> +          ti,hysteresis = <20>;
> +        };
> +
> +        channel@1 {
> +          reg = <1>;
> +          ti,threshold-falling = <100>;
> +          ti,threshold-rising = <2500>;
> +          ti,hysteresis = <0>;
> +        };
> +      };
> +    };
> +...
> +

József Horváth May 2, 2021, 9:10 p.m. UTC | #2

On Sun, May 02, 2021 at 06:22:55PM +0100, Jonathan Cameron wrote:
> On Sat, 1 May 2021 18:25:18 +0000
> Jozsef Horvath <[e-mail]> wrote:
> 
> > This is a device tree schema for iio driver for
> >  Texas Instruments ADS7142 dual-channel, programmable sensor monitor.
> > 
> > Datasheet: https://www.ti.com/lit/ds/symlink/ads7142.pdf
> > 
> > Signed-off-by: Jozsef Horvath <[e-mail]>
> > ---
> > ---
> >  .../bindings/iio/adc/ti,ads7142.yaml          | 198 ++++++++++++++++++
> >  1 file changed, 198 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/iio/adc/ti,ads7142.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/iio/adc/ti,ads7142.yaml b/Documentation/devicetree/bindings/iio/adc/ti,ads7142.yaml
> > new file mode 100644
> > index 000000000000..b4e752160156
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/adc/ti,ads7142.yaml
> > @@ -0,0 +1,198 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: "http://devicetree.org/schemas/iio/adc/ti,ads7142.yaml#"
> > +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> > +
> > +title: Texas Instruments ADS7142 adc driver device tree bindings
> > +
> > +maintainers:
> > +  - József Horváth <[e-mail]>
> > +
> > +description: |
> > +  This document is for describing the required device tree parameters
> > +   for ads7142 adc driver.
> 
> Document describes hardware, not a particular driver.  So just refer
> to the device here.  There may well be other drives in future using
> the same binding (e.g. in an RTOS).
> 
> > +  The required parameters for proper operation are described below.
> > +
> > +  Datasheet: https://www.ti.com/lit/ds/symlink/ads7142.pdf
> > +
> > +  Operation modes supportedby the driver:
> > +    When the 'ti,monitoring-mode' property is not present
> > +      in the devicetree node definition, the driver initiates a single
> > +      conversion in the device for each read request
> > +      (/sys/bus/iio/devices/iio:deviceX/in_voltageY_raw).
> > +      This is a one-shot conversion, and it is called
> > +      "Manual Mode" in the datasheet.
> > +
> > +    When the 'ti,monitoring-mode' property is present
> > +      in the devicetree node definition, the driver configures
> > +      the device's digital window comparator and sets the device's
> > +      data buffer operation mode to pre alert data mode.
> > +      The driver reads the conversion result when the BUSY/RDY interrupt
> > +      fires, and keeps the value until the next BUSY/RDY interrupt
> > +      or the first read request
> > +      (/sys/bus/iio/devices/iio:deviceX/in_voltageY_raw).
> > +      The digital window comparator and hysteresis parameters
> > +      can be controlled by:
> > +        - the devicetree definition of channel node
> > +        - iio sysfs interfaces
> > +      This is event driven conversion, and is called
> > +      "Autonomous Mode with Pre Alert Data" in the datasheet.
> > +      This mode can be used to wake up the system with the ALERT pin,
> > +      in case when the monitored voltage level is out of the configured range.
> 
> I talked about these in the driver review so look there for comments.
> Short summary is this is something userspace should have control off (assuming irq
> is wired up) not dt.
> 
> > +
> > +properties:
> > +  compatible:
> > +    const: ti,ads7142
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    description: |
> > +      The BUSY/PDY pin is used as interrupt line in autonomous monitoring mode.
> > +    maxItems: 1
> > +
> > +  vref-supply:
> > +    description: Regulator for the reference voltage
> > +
> > +  power-supply: true
> 
> These don't match the naming on the pin diagram.

I'll cange it to dvdd, and vref to avdd.

> 
> > +
> > +  "#address-cells":
> > +    const: 1
> > +
> > +  "#size-cells":
> > +    const: 0
> > +
> > +  "#io-channel-cells":
> > +    const: 1
> > +
> > +  ti,osc-sel:
> > +    description: |
> > +      If present, the driver selects the high speed oscillator.
> > +      See chapter 7.3.5 Oscillator and Timing Control in datasheet.
> > +    type: boolean
> 
> This looks connected to the possible sampling frequencies when in various autonomous modes.
> Should it be controlled by userspace?

The sampling frequency is controlled with the osc-sel and n-clk.
I'll remove n-clk from sysfs.

> 
> > +
> > +  ti,n-clk:
> > +    description: |
> > +      nCLK is number of clocks in one conversion cycle.
> > +      See chapter 7.3.5 Oscillator and Timing Control in datasheet.
> 
> Sounds like a policy decision for userspace.
> 
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    maximum: 255
> > +    minimum: 0
> > +
> > +  ti,monitoring-mode:
> > +    description: |
> > +      If present, the driver selects the autonomous monitoring mode with pre alert data.
> > +      See chapter 7.4 Device Functional Modes in datasheet.
> 
> As mentioned in the driver review, this looks like something we should control from userspace
> not dt to me.
> 

I would keep this here, but it will be an enum.

> > +    type: boolean
> > +
> > +patternProperties:
> > +  "^channel@[0-1]$":
> > +    $ref: "adc.yaml"
> > +    type: object
> > +    description: |
> > +      Represents the external channels which are connected to the ADC.
> > +    properties:
> > +      reg:
> > +        description: |
> > +          The channel number.
> > +        items:
> > +          minimum: 0
> > +          maximum: 1
> > +      "ti,threshold-falling":
> > +        description: The low threshold for channel
> 
> For these, we need a strong argument presented in this doc for why they are not
> a question of policy (and hence why they should be in dt at all).

I'll remove all threshold and hysteresys from dt.

> 
> > +        $ref: /schemas/types.yaml#/definitions/uint32
> > +        maximum: 4095
> > +        minimum: 0
> > +      "ti,threshold-rising":
> > +        description: The high threshold for channel
> > +        $ref: /schemas/types.yaml#/definitions/uint32
> > +        maximum: 4095
> > +        minimum: 0
> > +      "ti,hysteresis":
> > +        description: The hysteresis for both comparators for channel
> > +        $ref: /schemas/types.yaml#/definitions/uint32
> > +        maximum: 63
> > +        minimum: 0
> > +
> > +    required:
> > +      - reg
> > +
> > +    additionalProperties: false
> > +
> > +allOf:
> > +  - if:
> > +      required:
> > +        - ti,monitoring-mode
> > +    then:
> > +      required:
> > +        - interrupts
> > +
> > +required:
> > +  - compatible
> > +  - "#io-channel-cells"
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    i2c {
> > +      #address-cells = <1>;
> > +      #size-cells = <0>;
> > +      adc@18 {
> 
> I would not bother having two examples.  The second one covers more things afterall
> and the binding makes it clear what is required.
> 

I do this because of the conditional requirement of interrupts.

> > +        compatible = "ti,ads7142";
> > +        reg = <0x18>;
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +        #io-channel-cells = <1>;
> > +
> > +        vref-supply = <&vdd_3v3_reg>;
> > +        power-supply = <&vdd_1v8_reg>;
> > +
> > +        channel@0 {
> > +          reg = <0>;
> > +        };
> > +
> > +        channel@1 {
> > +          reg = <1>;
> > +        };
> > +      };
> > +    };
> > +  - |
> > +    i2c {
> > +      #address-cells = <1>;
> > +      #size-cells = <0>;
> > +      adc@1f {
> > +        compatible = "ti,ads7142";
> > +        reg = <0x1f>;
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +
> > +        #io-channel-cells = <1>;
> > +
> > +        vref-supply = <&vdd_3v3_reg>;
> > +        power-supply = <&vdd_1v8_reg>;
> > +
> > +        interrupt-parent = <&gpio>;
> > +        interrupts = <7 2>;
> > +
> > +        ti,monitoring-mode;
> > +
> > +        channel@0 {
> > +          reg = <0>;
> > +          ti,threshold-falling = <1000>;
> > +          ti,threshold-rising = <2000>;
> > +          ti,hysteresis = <20>;
> > +        };
> > +
> > +        channel@1 {
> > +          reg = <1>;
> > +          ti,threshold-falling = <100>;
> > +          ti,threshold-rising = <2500>;
> > +          ti,hysteresis = <0>;
> > +        };
> > +      };
> > +    };
> > +...
> > +
>

Jonathan Cameron May 3, 2021, 10:30 a.m. UTC | #3

On Sun, 2 May 2021 21:10:20 +0000
József Horváth <[e-mail]> wrote:

Hi József,

> >   
> > > +
> > > +  "#address-cells":
> > > +    const: 1
> > > +
> > > +  "#size-cells":
> > > +    const: 0
> > > +
> > > +  "#io-channel-cells":
> > > +    const: 1
> > > +
> > > +  ti,osc-sel:
> > > +    description: |
> > > +      If present, the driver selects the high speed oscillator.
> > > +      See chapter 7.3.5 Oscillator and Timing Control in datasheet.
> > > +    type: boolean  
> > 
> > This looks connected to the possible sampling frequencies when in various autonomous modes.
> > Should it be controlled by userspace?  
> 
> The sampling frequency is controlled with the osc-sel and n-clk.
> I'll remove n-clk from sysfs.

Not sure I follow that.  I think we should only have these controlled via
sysfs.  It will be a bit complex as two related controls, but that is a common situation
and there is usually a sensible combination of options that makes sense.

For example, if we can meet the sampling frequency requested with the lower
power oscillator we go for that.

> 
> >   
> > > +
> > > +  ti,n-clk:
> > > +    description: |
> > > +      nCLK is number of clocks in one conversion cycle.
> > > +      See chapter 7.3.5 Oscillator and Timing Control in datasheet.  
> > 
> > Sounds like a policy decision for userspace.
> >   
> > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > +    maximum: 255
> > > +    minimum: 0
> > > +
> > > +  ti,monitoring-mode:
> > > +    description: |
> > > +      If present, the driver selects the autonomous monitoring mode with pre alert data.
> > > +      See chapter 7.4 Device Functional Modes in datasheet.  
> > 
> > As mentioned in the driver review, this looks like something we should control from userspace
> > not dt to me.
> >   
> 
> I would keep this here, but it will be an enum.

Sorry, but no.  As mentioned in the driver thread, this doesn't sound like
a characteristic of the hardware (board etc) so it doesn't belong in DT.
It may be challenging to implement an interface that makes sense, but
that doesn't mean we can avoid doing it.

> 
> > > +    type: boolean
> > > +
> > > +patternProperties:
> > > +  "^channel@[0-1]$":
> > > +    $ref: "adc.yaml"
> > > +    type: object
> > > +    description: |
> > > +      Represents the external channels which are connected to the ADC.
> > > +    properties:
> > > +      reg:
> > > +        description: |
> > > +          The channel number.
> > > +        items:
> > > +          minimum: 0
> > > +          maximum: 1
> > > +      "ti,threshold-falling":
> > > +        description: The low threshold for channel  
> > 
> > For these, we need a strong argument presented in this doc for why they are not
> > a question of policy (and hence why they should be in dt at all).  
> 
> I'll remove all threshold and hysteresys from dt.
> 
> >   
> > > +        $ref: /schemas/types.yaml#/definitions/uint32
> > > +        maximum: 4095
> > > +        minimum: 0
> > > +      "ti,threshold-rising":
> > > +        description: The high threshold for channel
> > > +        $ref: /schemas/types.yaml#/definitions/uint32
> > > +        maximum: 4095
> > > +        minimum: 0
> > > +      "ti,hysteresis":
> > > +        description: The hysteresis for both comparators for channel
> > > +        $ref: /schemas/types.yaml#/definitions/uint32
> > > +        maximum: 63
> > > +        minimum: 0
> > > +
> > > +    required:
> > > +      - reg
> > > +
> > > +    additionalProperties: false
> > > +
> > > +allOf:
> > > +  - if:
> > > +      required:
> > > +        - ti,monitoring-mode
> > > +    then:
> > > +      required:
> > > +        - interrupts
> > > +
> > > +required:
> > > +  - compatible
> > > +  - "#io-channel-cells"
> > > +
> > > +additionalProperties: false
> > > +
> > > +examples:
> > > +  - |
> > > +    i2c {
> > > +      #address-cells = <1>;
> > > +      #size-cells = <0>;
> > > +      adc@18 {  
> > 
> > I would not bother having two examples.  The second one covers more things afterall
> > and the binding makes it clear what is required.
> >   
> 
> I do this because of the conditional requirement of interrupts.

Understood, but that is clearly expressed by the 'required' above.
It's easy enough to understand how to not have parts that aren't
required without an example.

> 
> > > +        compatible = "ti,ads7142";
> > > +        reg = <0x18>;
> > > +        #address-cells = <1>;
> > > +        #size-cells = <0>;
> > > +        #io-channel-cells = <1>;
> > > +
> > > +        vref-supply = <&vdd_3v3_reg>;
> > > +        power-supply = <&vdd_1v8_reg>;
> > > +
> > > +        channel@0 {
> > > +          reg = <0>;
> > > +        };
> > > +
> > > +        channel@1 {
> > > +          reg = <1>;
> > > +        };
> > > +      };
> > > +    };
> > > +  - |
> > > +    i2c {
> > > +      #address-cells = <1>;
> > > +      #size-cells = <0>;
> > > +      adc@1f {
> > > +        compatible = "ti,ads7142";
> > > +        reg = <0x1f>;
> > > +        #address-cells = <1>;
> > > +        #size-cells = <0>;
> > > +
> > > +        #io-channel-cells = <1>;
> > > +
> > > +        vref-supply = <&vdd_3v3_reg>;
> > > +        power-supply = <&vdd_1v8_reg>;
> > > +
> > > +        interrupt-parent = <&gpio>;
> > > +        interrupts = <7 2>;
> > > +
> > > +        ti,monitoring-mode;
> > > +
> > > +        channel@0 {
> > > +          reg = <0>;
> > > +          ti,threshold-falling = <1000>;
> > > +          ti,threshold-rising = <2000>;
> > > +          ti,hysteresis = <20>;
> > > +        };
> > > +
> > > +        channel@1 {
> > > +          reg = <1>;
> > > +          ti,threshold-falling = <100>;
> > > +          ti,threshold-rising = <2500>;
> > > +          ti,hysteresis = <0>;
> > > +        };
> > > +      };
> > > +    };
> > > +...
> > > +  
> >

József Horváth May 3, 2021, 7:42 p.m. UTC | #4

On Mon, May 03, 2021 at 11:30:08AM +0100, Jonathan Cameron wrote:

Hi Jonathan,

> On Sun, 2 May 2021 21:10:20 +0000
> József Horváth <[e-mail]> wrote:
> 
> Hi József,
> 
> > >   
> > > > +
> > > > +  "#address-cells":
> > > > +    const: 1
> > > > +
> > > > +  "#size-cells":
> > > > +    const: 0
> > > > +
> > > > +  "#io-channel-cells":
> > > > +    const: 1
> > > > +
> > > > +  ti,osc-sel:
> > > > +    description: |
> > > > +      If present, the driver selects the high speed oscillator.
> > > > +      See chapter 7.3.5 Oscillator and Timing Control in datasheet.
> > > > +    type: boolean  
> > > 
> > > This looks connected to the possible sampling frequencies when in various autonomous modes.
> > > Should it be controlled by userspace?  
> > 
> > The sampling frequency is controlled with the osc-sel and n-clk.
> > I'll remove n-clk from sysfs.
> 
> Not sure I follow that.  I think we should only have these controlled via
> sysfs.  It will be a bit complex as two related controls, but that is a common situation
> and there is usually a sensible combination of options that makes sense.
> 
> For example, if we can meet the sampling frequency requested with the lower
> power oscillator we go for that.
> 

Ok, I'll do that.

> > 
> > >   
> > > > +
> > > > +  ti,n-clk:
> > > > +    description: |
> > > > +      nCLK is number of clocks in one conversion cycle.
> > > > +      See chapter 7.3.5 Oscillator and Timing Control in datasheet.  
> > > 
> > > Sounds like a policy decision for userspace.
> > >   
> > > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > > +    maximum: 255
> > > > +    minimum: 0
> > > > +
> > > > +  ti,monitoring-mode:
> > > > +    description: |
> > > > +      If present, the driver selects the autonomous monitoring mode with pre alert data.
> > > > +      See chapter 7.4 Device Functional Modes in datasheet.  
> > > 
> > > As mentioned in the driver review, this looks like something we should control from userspace
> > > not dt to me.
> > >   
> > 
> > I would keep this here, but it will be an enum.
> 
> Sorry, but no.  As mentioned in the driver thread, this doesn't sound like
> a characteristic of the hardware (board etc) so it doesn't belong in DT.
> It may be challenging to implement an interface that makes sense, but
> that doesn't mean we can avoid doing it.

Ok, I'll bring it to sysfs.

> 
> > 
> > > > +    type: boolean
> > > > +
> > > > +patternProperties:
> > > > +  "^channel@[0-1]$":
> > > > +    $ref: "adc.yaml"
> > > > +    type: object
> > > > +    description: |
> > > > +      Represents the external channels which are connected to the ADC.
> > > > +    properties:
> > > > +      reg:
> > > > +        description: |
> > > > +          The channel number.
> > > > +        items:
> > > > +          minimum: 0
> > > > +          maximum: 1
> > > > +      "ti,threshold-falling":
> > > > +        description: The low threshold for channel  
> > > 
> > > For these, we need a strong argument presented in this doc for why they are not
> > > a question of policy (and hence why they should be in dt at all).  
> > 
> > I'll remove all threshold and hysteresys from dt.
> > 
> > >   
> > > > +        $ref: /schemas/types.yaml#/definitions/uint32
> > > > +        maximum: 4095
> > > > +        minimum: 0
> > > > +      "ti,threshold-rising":
> > > > +        description: The high threshold for channel
> > > > +        $ref: /schemas/types.yaml#/definitions/uint32
> > > > +        maximum: 4095
> > > > +        minimum: 0
> > > > +      "ti,hysteresis":
> > > > +        description: The hysteresis for both comparators for channel
> > > > +        $ref: /schemas/types.yaml#/definitions/uint32
> > > > +        maximum: 63
> > > > +        minimum: 0
> > > > +
> > > > +    required:
> > > > +      - reg
> > > > +
> > > > +    additionalProperties: false
> > > > +
> > > > +allOf:
> > > > +  - if:
> > > > +      required:
> > > > +        - ti,monitoring-mode
> > > > +    then:
> > > > +      required:
> > > > +        - interrupts
> > > > +
> > > > +required:
> > > > +  - compatible
> > > > +  - "#io-channel-cells"
> > > > +
> > > > +additionalProperties: false
> > > > +
> > > > +examples:
> > > > +  - |
> > > > +    i2c {
> > > > +      #address-cells = <1>;
> > > > +      #size-cells = <0>;
> > > > +      adc@18 {  
> > > 
> > > I would not bother having two examples.  The second one covers more things afterall
> > > and the binding makes it clear what is required.
> > >   
> > 
> > I do this because of the conditional requirement of interrupts.
> 
> Understood, but that is clearly expressed by the 'required' above.
> It's easy enough to understand how to not have parts that aren't
> required without an example.

I'll bring the monitoring mode to sysfs, and in that case, the conditional requirement
 will be pointless, and interrupt property will be optional, so you are right, one example
 will be enough.

> 
> > 
> > > > +        compatible = "ti,ads7142";
> > > > +        reg = <0x18>;
> > > > +        #address-cells = <1>;
> > > > +        #size-cells = <0>;
> > > > +        #io-channel-cells = <1>;
> > > > +
> > > > +        vref-supply = <&vdd_3v3_reg>;
> > > > +        power-supply = <&vdd_1v8_reg>;
> > > > +
> > > > +        channel@0 {
> > > > +          reg = <0>;
> > > > +        };
> > > > +
> > > > +        channel@1 {
> > > > +          reg = <1>;
> > > > +        };
> > > > +      };
> > > > +    };
> > > > +  - |
> > > > +    i2c {
> > > > +      #address-cells = <1>;
> > > > +      #size-cells = <0>;
> > > > +      adc@1f {
> > > > +        compatible = "ti,ads7142";
> > > > +        reg = <0x1f>;
> > > > +        #address-cells = <1>;
> > > > +        #size-cells = <0>;
> > > > +
> > > > +        #io-channel-cells = <1>;
> > > > +
> > > > +        vref-supply = <&vdd_3v3_reg>;
> > > > +        power-supply = <&vdd_1v8_reg>;
> > > > +
> > > > +        interrupt-parent = <&gpio>;
> > > > +        interrupts = <7 2>;
> > > > +
> > > > +        ti,monitoring-mode;
> > > > +
> > > > +        channel@0 {
> > > > +          reg = <0>;
> > > > +          ti,threshold-falling = <1000>;
> > > > +          ti,threshold-rising = <2000>;
> > > > +          ti,hysteresis = <20>;
> > > > +        };
> > > > +
> > > > +        channel@1 {
> > > > +          reg = <1>;
> > > > +          ti,threshold-falling = <100>;
> > > > +          ti,threshold-rising = <2500>;
> > > > +          ti,hysteresis = <0>;
> > > > +        };
> > > > +      };
> > > > +    };
> > > > +...
> > > > +  
> > >   
> 

Best regards
Jozsef

Patch

diff --git a/Documentation/devicetree/bindings/iio/adc/ti,ads7142.yaml b/Documentation/devicetree/bindings/iio/adc/ti,ads7142.yaml
new file mode 100644
index 000000000000..b4e752160156
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/ti,ads7142.yaml
@@ -0,0 +1,198 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/iio/adc/ti,ads7142.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Texas Instruments ADS7142 adc driver device tree bindings
+
+maintainers:
+  - József Horváth <[e-mail]>
+
+description: |
+  This document is for describing the required device tree parameters
+   for ads7142 adc driver.
+  The required parameters for proper operation are described below.
+
+  Datasheet: https://www.ti.com/lit/ds/symlink/ads7142.pdf
+
+  Operation modes supportedby the driver:
+    When the 'ti,monitoring-mode' property is not present
+      in the devicetree node definition, the driver initiates a single
+      conversion in the device for each read request
+      (/sys/bus/iio/devices/iio:deviceX/in_voltageY_raw).
+      This is a one-shot conversion, and it is called
+      "Manual Mode" in the datasheet.
+
+    When the 'ti,monitoring-mode' property is present
+      in the devicetree node definition, the driver configures
+      the device's digital window comparator and sets the device's
+      data buffer operation mode to pre alert data mode.
+      The driver reads the conversion result when the BUSY/RDY interrupt
+      fires, and keeps the value until the next BUSY/RDY interrupt
+      or the first read request
+      (/sys/bus/iio/devices/iio:deviceX/in_voltageY_raw).
+      The digital window comparator and hysteresis parameters
+      can be controlled by:
+        - the devicetree definition of channel node
+        - iio sysfs interfaces
+      This is event driven conversion, and is called
+      "Autonomous Mode with Pre Alert Data" in the datasheet.
+      This mode can be used to wake up the system with the ALERT pin,
+      in case when the monitored voltage level is out of the configured range.
+
+properties:
+  compatible:
+    const: ti,ads7142
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    description: |
+      The BUSY/PDY pin is used as interrupt line in autonomous monitoring mode.
+    maxItems: 1
+
+  vref-supply:
+    description: Regulator for the reference voltage
+
+  power-supply: true
+
+  "#address-cells":
+    const: 1
+
+  "#size-cells":
+    const: 0
+
+  "#io-channel-cells":
+    const: 1
+
+  ti,osc-sel:
+    description: |
+      If present, the driver selects the high speed oscillator.
+      See chapter 7.3.5 Oscillator and Timing Control in datasheet.
+    type: boolean
+
+  ti,n-clk:
+    description: |
+      nCLK is number of clocks in one conversion cycle.
+      See chapter 7.3.5 Oscillator and Timing Control in datasheet.
+    $ref: /schemas/types.yaml#/definitions/uint32
+    maximum: 255
+    minimum: 0
+
+  ti,monitoring-mode:
+    description: |
+      If present, the driver selects the autonomous monitoring mode with pre alert data.
+      See chapter 7.4 Device Functional Modes in datasheet.
+    type: boolean
+
+patternProperties:
+  "^channel@[0-1]$":
+    $ref: "adc.yaml"
+    type: object
+    description: |
+      Represents the external channels which are connected to the ADC.
+    properties:
+      reg:
+        description: |
+          The channel number.
+        items:
+          minimum: 0
+          maximum: 1
+      "ti,threshold-falling":
+        description: The low threshold for channel
+        $ref: /schemas/types.yaml#/definitions/uint32
+        maximum: 4095
+        minimum: 0
+      "ti,threshold-rising":
+        description: The high threshold for channel
+        $ref: /schemas/types.yaml#/definitions/uint32
+        maximum: 4095
+        minimum: 0
+      "ti,hysteresis":
+        description: The hysteresis for both comparators for channel
+        $ref: /schemas/types.yaml#/definitions/uint32
+        maximum: 63
+        minimum: 0
+
+    required:
+      - reg
+
+    additionalProperties: false
+
+allOf:
+  - if:
+      required:
+        - ti,monitoring-mode
+    then:
+      required:
+        - interrupts
+
+required:
+  - compatible
+  - "#io-channel-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+      adc@18 {
+        compatible = "ti,ads7142";
+        reg = <0x18>;
+        #address-cells = <1>;
+        #size-cells = <0>;
+        #io-channel-cells = <1>;
+
+        vref-supply = <&vdd_3v3_reg>;
+        power-supply = <&vdd_1v8_reg>;
+
+        channel@0 {
+          reg = <0>;
+        };
+
+        channel@1 {
+          reg = <1>;
+        };
+      };
+    };
+  - |
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+      adc@1f {
+        compatible = "ti,ads7142";
+        reg = <0x1f>;
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        #io-channel-cells = <1>;
+
+        vref-supply = <&vdd_3v3_reg>;
+        power-supply = <&vdd_1v8_reg>;
+
+        interrupt-parent = <&gpio>;
+        interrupts = <7 2>;
+
+        ti,monitoring-mode;
+
+        channel@0 {
+          reg = <0>;
+          ti,threshold-falling = <1000>;
+          ti,threshold-rising = <2000>;
+          ti,hysteresis = <20>;
+        };
+
+        channel@1 {
+          reg = <1>;
+          ti,threshold-falling = <100>;
+          ti,threshold-rising = <2500>;
+          ti,hysteresis = <0>;
+        };
+      };
+    };
+...
+