Date: Sun, 9 Jun 2024 11:17:40 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: Francesco Dolcini <francesco@...cini.it>
Cc: João Paulo Gonçalves
 <jpaulo.silvagoncalves@...il.com>, Lars-Peter Clausen <lars@...afoo.de>,
 Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Conor Dooley <conor+dt@...nel.org>, João Paulo
 Gonçalves <joao.goncalves@...adex.com>,
 linux-iio@...r.kernel.org, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org, Francesco Dolcini
 <francesco.dolcini@...adex.com>
Subject: Re: [PATCH v2 1/2] dt-bindings: iio: adc: add ti,ads1119

On Thu,  6 Jun 2024 18:35:28 +0200
Francesco Dolcini <francesco@...cini.it> wrote:

> From: João Paulo Gonçalves <joao.goncalves@...adex.com>
> 
> Add devicetree bindings for Texas Instruments ADS1119 16-bit ADC
> with I2C interface.
> 
> Datasheet: https://www.ti.com/lit/gpn/ads1119
> Signed-off-by: João Paulo Gonçalves <joao.goncalves@...adex.com>
> Signed-off-by: Francesco Dolcini <francesco.dolcini@...adex.com>

I missed it on previous version but you only have description in here
for vref and few devices power them selves from something called vref!

The binding should describe the other power supplies as well and mark
them as required.

We've left these out too many times in the past and ended up having
a patch very soon after adding them. Better to have them from the start.
Driver should just use devm_regulator_get_enabled() to turn them on and
register them to be turned off on driver removal, and ignore them
after that.  If anyone has controlled supplies and wants to do more
sophisticated handling they can add it later.

Note that even though the dt-binding will list them as required, if
a particular DTS doesn't provide them the regulator framework will
give a dummy uncontrolled regulator to represent the assumption that
the power is always there.  We still document them as required in
the dt-binding though as other OS may not be so clever.

Only needs minimal entries though - see inline

Jonathan

> ---
> v2:
>  - add diff-channels and single-channel
>  - add XOR check to make diff/single channel property required 
>  - add interrupts, reset-gpios and vref-supply to the example 
>  - fix missing additionalProperties/unevaluatedProperties warning in channels
>  - remove ti,gain and ti,datarate as they aren't fixed hw properties
>  - remove unnecessary | 
> ---
>  .../bindings/iio/adc/ti,ads1119.yaml          | 148 ++++++++++++++++++
>  MAINTAINERS                                   |   7 +
>  2 files changed, 155 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/ti,ads1119.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/ti,ads1119.yaml b/Documentation/devicetree/bindings/iio/adc/ti,ads1119.yaml
> new file mode 100644
> index 000000000000..cbf0d4ef3a11
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/ti,ads1119.yaml
> @@ -0,0 +1,148 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/adc/ti,ads1119.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Texas Instruments ADS1119 ADC
> +
> +maintainers:
> +  - João Paulo Gonçalves <jpaulo.silvagoncalves@...il.com>
> +
> +description:
> +  The TI ADS1119 is a precision 16-bit ADC over I2C that offers single-ended and
> +  differential measurements using a multiplexed input. It features a programmable
> +  gain, a programmable sample rate, an internal oscillator and voltage reference,
> +  and a 50/60Hz rejection filter.
> +
> +properties:
> +  compatible:
> +    const: ti,ads1119
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  reset-gpios:
> +    maxItems: 1
> +
avdd-supply: true
dvdd-supply: true

> +  vref-supply:
> +    description:
> +      ADC external reference voltage (VREF).
> +
> +  "#address-cells":
> +    const: 1
> +
> +  "#size-cells":
> +    const: 0
> +
> +  "#io-channel-cells":
> +    const: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - "#address-cells"
> +  - "#size-cells"
    - avdd-supply
    - dvdd-supply

> +
> +patternProperties:
> +  "^channel@([0-6])$":
> +    $ref: adc.yaml
> +    type: object
> +    properties:
> +      reg:
> +        minimum: 0
> +        maximum: 6
> +
> +      diff-channels:
> +        description:
> +          Differential input channels AIN0-AIN1, AIN2-AIN3 and AIN1-AIN2.
> +        oneOf:
> +          - items:
> +              - const: 0
> +              - const: 1
> +          - items:
> +              - const: 2
> +              - const: 3
> +          - items:
> +              - const: 1
> +              - const: 2
> +
> +      single-channel:
> +        description:
> +          Single-ended input channels AIN0, AIN1, AIN2 and AIN3.
> +        minimum: 0
> +        maximum: 3
> +
> +    oneOf:
> +      - required:
> +          - diff-channels
> +      - required:
> +          - single-channel
> +
> +    required:
> +      - reg
> +
> +    unevaluatedProperties: false
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +
> +    #include <dt-bindings/gpio/gpio.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        adc@40 {
> +            compatible = "ti,ads1119";
> +            reg = <0x40>;
> +            interrupt-parent = <&gpio1>;
> +            interrupts = <25 IRQ_TYPE_EDGE_FALLING>;
> +            reset-gpios = <&gpio1 10 GPIO_ACTIVE_LOW>;
> +            vref-supply = <&reg_vref_ads1119>;
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +            #io-channel-cells = <1>;
> +
> +            channel@0 {
> +                reg = <0>;
> +                single-channel = <0>;
> +            };
> +
> +            channel@1 {
> +                reg = <1>;
> +                diff-channels = <0 1>;
> +            };
> +
> +            channel@2 {
> +                reg = <2>;
> +                single-channel = <3>;
> +            };
> +
> +            channel@3 {
> +                reg = <3>;
> +                single-channel = <1>;
> +            };
> +
> +            channel@4 {
> +                reg = <4>;
> +                single-channel = <2>;
> +            };
> +
> +            channel@5 {
> +                reg = <5>;
> +                diff-channels = <1 2>;
> +            };
> +
> +            channel@6 {
> +                reg = <6>;
> +                diff-channels = <2 3>;
> +            };
> +        };
> +    };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index d6c90161c7bf..f1b2c4b815e2 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -22380,6 +22380,13 @@ M:	Robert Richter <rric@...nel.org>
>  S:	Odd Fixes
>  F:	drivers/gpio/gpio-thunderx.c
>  
> +TI ADS1119 ADC DRIVER
> +M:	Francesco Dolcini <francesco@...cini.it>
> +M:	João Paulo Gonçalves <jpaulo.silvagoncalves@...il.com>
> +L:	linux-iio@...r.kernel.org
> +S:	Maintained
> +F:	Documentation/devicetree/bindings/iio/adc/ti,ads1119.yaml
> +
>  TI ADS7924 ADC DRIVER
>  M:	Hugo Villeneuve <hvilleneuve@...onoff.com>
>  L:	linux-iio@...r.kernel.org

Powered by blists - more mailing lists