Message-ID: <Z4Znnk0Cz3wvH6Vp@debian-BULLSEYE-live-builder-AMD64>
Date: Tue, 14 Jan 2025 10:33:18 -0300
From: Marcelo Schmitt <marcelo.schmitt1@...il.com>
To: David Lechner <dlechner@...libre.com>
Cc: Marcelo Schmitt <marcelo.schmitt@...log.com>, linux-iio@...r.kernel.org,
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
	jic23@...nel.org, lars@...afoo.de, Michael.Hennerich@...log.com,
	robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org,
	ana-maria.cusco@...log.com
Subject: Re: [RFC PATCH 4/4] Documentation: iio: Add ADC documentation

Hi David, thank you for your suggestions.
I think I've applied most of them and will soon send a v2 only with the docs.
Replying here mostly on the comments I didn't comply with.

On 12/18, David Lechner wrote:
> On 12/18/24 8:38 AM, Marcelo Schmitt wrote:
> > ADCs can have different input configurations such that developers can get
> > confused when trying to model some of them into IIO channels.
> > 
...
> > 
> > Add documentation about common ADC characteristics and IIO support for them.
> > 
...
> > +In the ADC driver, `differential = 1` is set into `struct iio_chan_spec` for the
> > +channel. See ``include/linux/iio/iio.h`` for more information.
> > +
> > +1.2.2 Differential Unipolar Channels
> > +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 
> To be consistent with the other sections, move unipolar before bipolar.

I had differential unipolar before differential bipolar on a
preliminary version (not sent to the mailing list), but it lead to a much more
intricate explanation of differential unipolar. That's why I prefer to keep
the Differential Unipolar section after Differential Bipolar.

> 
> > +
> > +For **differential unipolar** channels, the analog voltage at the positive input
> > +must also be higher than the voltage at the negative input. Thus, the actual
> > +input range allowed to a differential unipolar channel is IN- to +VREF. Because
> > +IN+ is allowed to swing with the measured analog signal and the input setup must
> > +guarantee IN+ will not go below IN- (nor IN- will raise above IN+), most
> > +differential unipolar channel setups have IN- fixed to a known voltage that does
> > +not fall within the voltage range expected for the measured signal. This leads
> > +to a setup that is equivalent to a pseudo-differential channel. Thus,
> > +differential unipolar channels are actually pseudo-differential unipolar
> > +channels.
> 
> The diagrams are really helpful, so please add a diagram in this section as well.

There is no diagram for Differential Unipolar. What would be the
Differential Unipolar diagram is the diagram for Pseudo-Differential Unipolar.
Having Differential Unipolar section here also makes it closer to the
Pseudo-Differential Unipolar diagram.

> 
...
> > +
> > +1.3.1 Pseudo-differential Unipolar Channels
> > +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > +
> > +::
> > +
> > +  -------- +VREF ------          +-------------------+
> > +    ´ `       ´ `               /                    |
> > +  /     \   /     \   /    --- <  IN+                |
> > +         `-´       `-´          |                    |
> > +  --------- IN- -------         |            ADC     |
> 
> The bottom rail should be GND, not IN-. Typically, the common mode voltage is
> VREF / 2. In other words it is halfway between the two rails.

IN- may be above GND (e.g. at VREF / 2 as a typical common mode voltage).
In that case, the minimum voltage for IN+ (i.e. the bottom rail) would be VREF / 2.
The generic constraint would be that IN+ does not fall below IN-.
See bipolar/unipolar configuration section of AD4170 datasheet page 46.
https://www.analog.com/media/en/technical-documentation/data-sheets/ad4170-4.pdf
On that example, VREF is actually a differential voltage reference that is
2.5V nominal voltage (halfway between 5V AVDD and 0V AVSS).
If IN+ is allowed to go below IN-, then this becomes Pseudo-differential Bipolar.

> 
> > +                                |                    |
> > +  Common-mode voltage -->  --- <  IN-                |
> > +                                \       +VREF  -VREF |
> > +                                 +-------------------+
> > +                                          ^       ^
> > +                                          |       +---- External -VREF
> 
> This is unipolar, so would not expect -VREF here.

I think IN- could in theory be negative (bellow GND) if the ADC inputs are
true bipolar inputs. Though, I have never seen such thing so can't say
for sure. Anyway, -VREF is not doing anything on this setup so I omitted it in v2.

> 
> > +                                   External +VREF
> > +
> > +A **pseudo-differential unipolar** input has the limitations a differential
> > +unipolar channel would have, meaning the analog voltage to the positive input
> > +IN+ must stay within IN- to +VREF. The fixed voltage to IN- is sometimes called
> > +common-mode voltage and it must be within -VREF to +VREF as would be expected
> > +from the signal to any differential channel negative input.
> > +
> > +In pseudo-differential configuration, the voltage measured from IN+ is not
> > +relative to GND (as it would be for a single-ended channel) but to IN-, which
> > +causes the measurement to always be offset by IN- volts. To allow applications
> > +to calculate IN+ voltage with respect to system ground, the IIO channel may
> > +provide an `_offset` attribute to report the channel offset to user space.
> 
> In some chips though, the common mode voltage may be GND. (Example is AD7944
> that calls this "ground sense"). So in that case, there is no common mode
> supply or ``_offset`` attribute.
> 
Added a comment about it omitting the ``_offset`` in those cases.
My understanding is that, because the common mode voltage (or ground sense in
AD7944's case) is at GND, the ``_offset`` is always zero and that's why the
``_offset`` attribute is not needed in that case. Whenever the common mode
voltage is at something other than GND, we would need ``_offset`` to be able to
get the voltage relative to GND.
Oh well, that's just another way of saying what you already told me I guess.

Powered by blists - more mailing lists