public inbox for devicetree@vger.kernel.org
 help / color / mirror / Atom feed
From: Jonathan Cameron <jic23@kernel.org>
To: Francesco Dolcini <francesco@dolcini.it>
Cc: "João Paulo Gonçalves" <jpaulo.silvagoncalves@gmail.com>,
	"Lars-Peter Clausen" <lars@metafoo.de>,
	"Rob Herring" <robh@kernel.org>,
	"Krzysztof Kozlowski" <krzk+dt@kernel.org>,
	"Conor Dooley" <conor+dt@kernel.org>,
	"João Paulo Gonçalves" <joao.goncalves@toradex.com>,
	linux-iio@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	"Francesco Dolcini" <francesco.dolcini@toradex.com>
Subject: Re: [PATCH v2 1/2] dt-bindings: iio: adc: add ti,ads1119
Date: Sun, 9 Jun 2024 11:17:40 +0100	[thread overview]
Message-ID: <20240609111740.1c61ce07@jic23-huawei> (raw)
In-Reply-To: <20240606163529.87528-2-francesco@dolcini.it>

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

> From: João Paulo Gonçalves <joao.goncalves@toradex.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@toradex.com>
> Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.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@gmail.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@kernel.org>
>  S:	Odd Fixes
>  F:	drivers/gpio/gpio-thunderx.c
>  
> +TI ADS1119 ADC DRIVER
> +M:	Francesco Dolcini <francesco@dolcini.it>
> +M:	João Paulo Gonçalves <jpaulo.silvagoncalves@gmail.com>
> +L:	linux-iio@vger.kernel.org
> +S:	Maintained
> +F:	Documentation/devicetree/bindings/iio/adc/ti,ads1119.yaml
> +
>  TI ADS7924 ADC DRIVER
>  M:	Hugo Villeneuve <hvilleneuve@dimonoff.com>
>  L:	linux-iio@vger.kernel.org


      parent reply	other threads:[~2024-06-09 10:18 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-06-06 16:35 [PATCH v2 0/2] iio: adc: ti-ads1119: Add driver Francesco Dolcini
2024-06-06 16:35 ` [PATCH v2 1/2] dt-bindings: iio: adc: add ti,ads1119 Francesco Dolcini
2024-06-06 17:07   ` Conor Dooley
2024-06-06 20:49   ` Rob Herring
2024-06-06 22:11     ` Francesco Dolcini
2024-06-09 10:17   ` Jonathan Cameron [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240609111740.1c61ce07@jic23-huawei \
    --to=jic23@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=francesco.dolcini@toradex.com \
    --cc=francesco@dolcini.it \
    --cc=joao.goncalves@toradex.com \
    --cc=jpaulo.silvagoncalves@gmail.com \
    --cc=krzk+dt@kernel.org \
    --cc=lars@metafoo.de \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=robh@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox