[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAMknhBGNPvxegL+YbnLGoKjA=P3Vx=x+39aXuMgq+cv2KgdeLw@mail.gmail.com>
Date: Wed, 15 May 2024 17:37:43 -0500
From: David Lechner <dlechner@...libre.com>
To: dumitru.ceclan@...log.com
Cc: Lars-Peter Clausen <lars@...afoo.de>, Michael Hennerich <Michael.Hennerich@...log.com>,
Jonathan Cameron <jic23@...nel.org>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>, Conor Dooley <conor+dt@...nel.org>,
linux-iio@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, Dumitru Ceclan <mitrutzceclan@...il.com>
Subject: Re: [PATCH v2 1/9] dt-bindings: adc: ad7173: add support for ad411x
On Tue, May 14, 2024 at 2:23 AM Dumitru Ceclan via B4 Relay
<devnull+dumitru.ceclan.analog.com@...nel.org> wrote:
>
> From: Dumitru Ceclan <dumitru.ceclan@...log.com>
>
> Add support for: AD4111, AD4112, AD4114, AD4115, AD4116.
>
> AD411x family ADCs support a VCOM pin, dedicated for single-ended usage.
> AD4111/AD4112 support current channels, usage is implemented by
> specifying channel reg values bigger than 15.
>
> Signed-off-by: Dumitru Ceclan <dumitru.ceclan@...log.com>
> ---
> .../devicetree/bindings/iio/adc/adi,ad7173.yaml | 118 ++++++++++++++++++++-
> 1 file changed, 117 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml
> index ea6cfcd0aff4..6cc3514f5ed8 100644
> --- a/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml
> @@ -19,7 +19,18 @@ description: |
> primarily for measurement of signals close to DC but also delivers
> outstanding performance with input bandwidths out to ~10kHz.
>
> + Analog Devices AD411x ADC's:
> + The AD411X family encompasses a series of low power, low noise, 24-bit,
> + sigma-delta analog-to-digital converters that offer a versatile range of
> + specifications. They integrate an analog front end suitable for processing
> + fully differential/single-ended and bipolar voltage inputs.
> +
> Datasheets for supported chips:
> + https://www.analog.com/media/en/technical-documentation/data-sheets/AD4111.pdf
> + https://www.analog.com/media/en/technical-documentation/data-sheets/AD4112.pdf
> + https://www.analog.com/media/en/technical-documentation/data-sheets/AD4114.pdf
> + https://www.analog.com/media/en/technical-documentation/data-sheets/AD4115.pdf
> + https://www.analog.com/media/en/technical-documentation/data-sheets/AD4116.pdf
> https://www.analog.com/media/en/technical-documentation/data-sheets/AD7172-2.pdf
> https://www.analog.com/media/en/technical-documentation/data-sheets/AD7172-4.pdf
> https://www.analog.com/media/en/technical-documentation/data-sheets/AD7173-8.pdf
> @@ -31,6 +42,11 @@ description: |
> properties:
> compatible:
> enum:
> + - adi,ad4111
> + - adi,ad4112
> + - adi,ad4114
> + - adi,ad4115
> + - adi,ad4116
> - adi,ad7172-2
> - adi,ad7172-4
> - adi,ad7173-8
> @@ -129,6 +145,31 @@ patternProperties:
> maximum: 15
>
> diff-channels:
> + description: |
> + For using current channels specify select the current inputs
> + and enable the adi,current-channel property.
> +
> + Family AD411x supports a dedicated VCOM voltage input.
Did you mean VINCOM? Searching the datasheets for "VCOM" comes up empty.
> + To select it set the second channel to 16.
> + (VIN2, VCOM) -> diff-channels = <2 16>
Jonathan mentioned this in v1 with regard to the current inputs, but
it applies here too. There is a new proposed single-channel property
[1] that would be preferred when an input is used as a single-ended or
pseudo-differential input (i.e. with VINCOM or ADCIN15).
[1]: https://lore.kernel.org/linux-iio/20240514120222.56488-5-alisa.roman@analog.com/
> +
> + There are special values that can be selected besides the voltage
> + analog inputs:
> + 21: REF+
> + 22: REF−
> + Supported only by AD7172-2, AD7172-4, AD7175-2, AD7175-8, AD7177-2:
> + 19: ((AVDD1 − AVSS)/5)+
> + 20: ((AVDD1 − AVSS)/5)−
> + Supported only by AD4111, AD4112:
> + 12: IIN3+
> + 11: IIN3−
> + 13: IIN2+
> + 10: IIN2−
> + 14: IIN1+
> + 9: IIN1−
> + 15: IIN0+
> + 8: IIN0−
I just made a late reply on v1 where Jonathan suggested that the
current inputs are differential with a similar comment to this:
It doesn't seem to me like current inputs are differential if they are
only measuring one current. They take 2 pins because you need a way
for current to come in and go back out, but the datasheet calls them
single-ended inputs.
> +
> items:
> minimum: 0
> maximum: 31
> @@ -154,6 +195,23 @@ patternProperties:
> - avdd
> default: refout-avss
>
> + adi,current-channel:
> + description: |
> + Signal that the selected inputs are current channels.
> + Only available on AD4111 and AD4112.
> + type: boolean
> +
> + adi,channel-type:
> + description:
> + Used to differentiate between different channel types as the device
> + register configurations are the same for all usage types.
> + $ref: /schemas/types.yaml#/definitions/string
> + enum:
> + - single-ended
> + - pseudo-differential
> + - differential
> + default: differential
> +
As suggested above, we should soon have diff-channels and
single-channel to differentiate between (fully) differential and
single-ended. Do we actually need to differentiate between
single-ended and pseudo-differential though?
I think the AD4116 datasheet is the only one that uses both of those
terms. It gives the examples that for "single-ended" ADCIN15 would be
connected to AVSS and for "pseudo-differential" ADCIN15 would be
connected to REFOUT (AVSS + 2.5 V). So the only difference seems to be
if the voltage on ADCIN15 is == 0V or != 0V.
To make this like other pseudo-differential chips we have done
recently, it seems to me like we should have an adcin15-supply
property to describe the ADCIN15 input. Then we could use that
property to determine single-ended vs. pseudo-differential (if there
actually is a need for that) and we wouldn't need the adi,channel-type
property.
> required:
> - reg
> - diff-channels
> @@ -166,7 +224,6 @@ allOf:
> - $ref: /schemas/spi/spi-peripheral-props.yaml#
>
> # Only ad7172-4, ad7173-8 and ad7175-8 support vref2
> - # Other models have [0-3] channel registers
> - if:
> properties:
> compatible:
> @@ -187,6 +244,37 @@ allOf:
> - vref
> - refout-avss
> - avdd
> +
> + - if:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - adi,ad4114
> + - adi,ad4115
> + - adi,ad4116
> + - adi,ad7173-8
> + - adi,ad7175-8
> + then:
> + patternProperties:
> + "^channel@[0-9a-f]$":
> + properties:
> + reg:
> + maximum: 15
As discussed recently in the the very similar ad719x bindings [2], we
may have been misunderstanding this limit so far. 15 is a bit
artificially low since input pins can be used more than once in
different "channels". But that is really an issues with the existing
bindings, not just this patch.
[2]: https://lore.kernel.org/linux-iio/20240511122955.2372f56e@jic23-huawei/
> +
> + - if:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - adi,ad7172-2
> + - adi,ad7175-2
> + - adi,ad7176-2
> + - adi,ad7177-2
> + then:
> + patternProperties:
> + "^channel@[0-9a-f]$":
> + properties:
> reg:
> maximum: 3
>
> @@ -210,6 +298,34 @@ allOf:
> required:
> - adi,reference-select
>
> + - if:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - adi,ad4111
> + - adi,ad4112
> + - adi,ad4114
> + - adi,ad4115
> + - adi,ad4116
> + then:
> + properties:
> + avdd2-supply: false
> +
> + - if:
> + properties:
> + compatible:
> + not:
> + contains:
> + enum:
> + - adi,ad4111
> + - adi,ad4112
> + then:
> + patternProperties:
> + "^channel@[0-9a-f]$":
> + properties:
> + adi,current-channel: false
> +
> - if:
> anyOf:
> - required: [clock-names]
>
> --
> 2.43.0
>
>
Powered by blists - more mailing lists