Received-SPF: pass (google.com: domain of linux-kernel+bounces-198933-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <efa10caa-5e78-4f3f-8cca-c61d7a01e6fd@gmail.com>
Date: Mon, 3 Jun 2024 12:46:10 +0300
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v4 1/6] dt-bindings: adc: ad7173: add support for ad411x
To: Jonathan Cameron <jic23@kernel.org>,
 Dumitru Ceclan via B4 Relay <devnull+dumitru.ceclan.analog.com@kernel.org>
Cc: dumitru.ceclan@analog.com, Lars-Peter Clausen <lars@metafoo.de>,
 Michael Hennerich <Michael.Hennerich@analog.com>,
 Rob Herring <robh@kernel.org>,
 Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
 Conor Dooley <conor+dt@kernel.org>, David Lechner <dlechner@baylibre.com>,
 linux-iio@vger.kernel.org, devicetree@vger.kernel.org,
 linux-kernel@vger.kernel.org
References: <20240531-ad4111-v4-0-64607301c057@analog.com>
 <20240531-ad4111-v4-1-64607301c057@analog.com>
 <20240601193512.0e17992b@jic23-huawei>
Content-Language: en-US
From: "Ceclan, Dumitru" <mitrutzceclan@gmail.com>
In-Reply-To: <20240601193512.0e17992b@jic23-huawei>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

On 01/06/2024 21:35, Jonathan Cameron wrote:
> On Fri, 31 May 2024 22:42:27 +0300
> Dumitru Ceclan via B4 Relay <devnull+dumitru.ceclan.analog.com@kernel.org> wrote:
> 
>> From: Dumitru Ceclan <dumitru.ceclan@analog.com>
>>
>> Add support for: AD4111, AD4112, AD4114, AD4115, AD4116.
>>
>> AD411x family ADCs support a VCOM pin. The purpose of this pin is to
>> offer a dedicated common-mode voltage input for single-ended channels.
>> This pin is specified as supporting a differential channel with VIN10 on
>> model AD4116.
>>
>> AD4111/AD4112 support current channels. Support is implemented using
>> single-channel and "adi,current-channel".
>>
>> Signed-off-by: Dumitru Ceclan <dumitru.ceclan@analog.com>
> I like the common-mode-channel solution to the pseudo differential
> description. It makes things explicit whilst avoiding an ugly differential
> but not differential mess.
> 
> However, it feels like a general thing to me not a vendor specific one.
> Perhaps makes sense to put in adc.yaml?
> 

Sure

> One other question that is more me being curious and failing to understand
> the datasheet than a request to change anything.
>> ---
>>  .../devicetree/bindings/iio/adc/adi,ad7173.yaml    | 192 ++++++++++++++++++++-
>>  1 file changed, 190 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml
>> index ea6cfcd0aff4..d8474eee553e 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,10 +145,54 @@ patternProperties:
>>          maximum: 15
>>  
>>        diff-channels:
>> +        description: |
>> +          This property is used for defining the inputs of a differential
>> +          voltage channel. The first value is the positive input and the second
>> +          value is the negative input of the channel.
>> +
>> +          Family AD411x supports a dedicated VINCOM voltage input.
>> +          To select it set the second channel to 16.
>> +            (VIN2, VINCOM) -> diff-channels = <2 16>
>> +
>> +          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)−
> 
> That's what it says on the datasheet (so fine to copy that here) but I'm curious, what does
> that mean in practice?  How can we have negative and postive signals of the difference
> between two power supply voltages where I'm fairly sure AVDD1 always greater than AVSS.
>

I have not tested that as I do not have a model that supports this wired up.
If I had to guess they are the same signal but one should be connected to the
positive input, one to the negative input...but I could be wrong.
 
> Anyhow, that's a problem for the person reading the datasheet to figure out :)
>  
>> +
>>          items:
>>            minimum: 0
>>            maximum: 31
>>  
>> +      single-channel:
>> +        description: |
>> +          This property is used for defining a current channel or the positive
>> +          input of a voltage channel (single-ended or pseudo-differential).
>> +
>> +          Models AD4111 and AD4112 support current channels.
>> +            Example: (IIN2+, IIN2−) -> single-channel = <2>
>> +          To correctly configure a current channel set the "adi,current-channel"
>> +          property to true.
>> +
>> +          To configure a single-ended/pseudo-differential channel set the
>> +          "adi,common-mode-channel" property to the desired negative voltage input.
>> +
>> +          When used as a voltage channel, special inputs are valid as well.
>> +        minimum: 0
>> +        maximum: 31
>> +
>> +      adi,common-mode-channel:
>> +        $ref: /schemas/types.yaml#/definitions/uint32
>> +        description:
>> +          This property is used for defining the negative input of a
>> +          single-ended or pseudo-differential voltage channel.
>> +
>> +          Special inputs are valid as well.
>> +        minimum: 0
>> +        maximum: 31
>> +
>