lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Z4wUPtDfnmQ53L0k@debian-BULLSEYE-live-builder-AMD64>
Date: Sat, 18 Jan 2025 17:51:10 -0300
From: Marcelo Schmitt <marcelo.schmitt1@...il.com>
To: Jonathan Cameron <jic23@...nel.org>
Cc: David Lechner <dlechner@...libre.com>,
	Marcelo Schmitt <marcelo.schmitt@...log.com>,
	linux-iio@...r.kernel.org, linux-doc@...r.kernel.org,
	linux-kernel@...r.kernel.org, lars@...afoo.de, corbet@....net
Subject: Re: [PATCH v2 1/1] Documentation: iio: Add ADC documentation

Hi Jonathan,

Thanks for having another look at this.
I'll do the changes suggested.

Thanks,
Marcelo

On 01/18, Jonathan Cameron wrote:
> On Wed, 15 Jan 2025 11:23:24 -0600
> David Lechner <dlechner@...libre.com> wrote:
> 
...
> > > +1.1 Single-ended channels
> > > +-------------------------
> > > +
> > > +Single-ended channels digitize the analog input voltage relative to ground and
> > > +can be either unipolar or bipolar.
> > > +
> > > +1.1.1 Single-ended Unipolar Channels
> > > +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > > +
> > > +::
> > > +
> > > +  ---------- VREF -------------
> > > +      ´ `           ´ `                  _____________
> > > +    /     \       /     \               /             |
> > > +   /       \     /       \         --- <  IN    ADC   |
> > > +            \   /         \   /         \             |
> > > +             `-´           `-´           \       VREF |
> > > +  -------- GND (0V) -----------           +-----------+
> > > +                                                  ^
> > > +                                                  |
> > > +                                             External VREF
> > > +
> > > +The input voltage to a **single-ended unipolar** channel is allowed to swing
> > > +from GND to VREF (where VREF is a voltage reference with electrical potential
> > > +higher than system ground). The maximum input voltage is also called VFS
> > > +(full-scale input voltage), with VFS being determined by VREF. The voltage
> 
> Just to align with acronym perhaps
> Voltage input Full-Scale 
> 
Okay, will do.

> > > +reference may be provided from an external supply or derived from the chip power
> > > +source.
> > > +
> > > +A single-ended unipolar channel could be described in device tree like the
> > > +following example::  
> > 
> > We should probably mention somewhere that channel@ nodes are only needed for
> > chips that don't have uniform inputs.
> 
> They are allowed in all cases. For SoCs ADCs it's not unusual to have them even
> if nothing exciting going on because they want to hide channels that aren't
> wired to anything.  For stand along ADCs that is less common because people
> don't buy a device with lots of channels intending to only use a few.
> 
> But sure, mention they may not be provided.

Sure, will mention that. 

> 
> > 
> > > +
> > > +    adc@0 {
> > > +        ...
> > > +        #address-cells = <1>;
> > > +        #size-cells = <0>;
> > > +
> > > +        channel@0 {
> > > +            reg = <0>;  
> > 
> > If a chip has mixed differential and single-ended, single-channel could also
> > be needed here in the case where reg is an arbitrary number and doesn't match
> > the input pin number.
> 
> Indeed - that is a weird corner that would be good to highlight.

Ack, will add a word about that too.

> 
> > 
> > > +        };
> > > +    };
> > > +
> > > +See ``Documentation/devicetree/bindings/iio/adc/adc.yaml`` for the complete
> > > +documentation of ADC specific device tree properties.
> > > +
> > > +
...
> > > +1.2.2 Differential Unipolar Channels
> > > +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > > +
> > > +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.  
> > 
> > I don't think this is equevent to pseudo-differential unipolar. That one has
> > a common mode voltage supply on the negative input. This one has a full range
> > signal on the negative input. This is the diagram I was expecting here.
> > 
> > ::
> > 
> >   -------- VREF -------
> >     ´ `       ´ `               +-------------------+
> >   /     \   /     \   /        /                    |
> >          `-´       `-´    --- <  IN+                |
> >   ------ GND (0V) -----        |                    |
> >                                |            ADC     |
> >   -------- VREF -------        |                    |
> >         ´ `       ´ `     --- <  IN-                |
> >   \   /     \   /     \        \       VREF         |
> >    `-´       `-´                +-------------------+
> >   ------ GND (0V) -----                  ^        
> >                                          |       
> >                                   External VREF
> 
> If it's unipolar, output must be positive which isn't true here.
> Do we actually see differential unipolar except for the pseudo case with
> common mode voltage?   Seems like a weird device.

I don't think we have differential unipolar with IN- allowed to float (i.e. not
set to a constant voltage). Haven't seen any of those. Can't think of how we
would support such thing either. See my reply to David for more thoughts on this.

> 
> > 
> > > +
> > > +1.3 Pseudo-differential Channels
> > > +--------------------------------
> > > +
> > > +There is a third ADC input type which is called pseudo-differential or
> > > +single-ended to differential configuration. A pseudo-differential channel is
> > > +similar to a differential channel in that it also measures IN+ relative to IN-.
> > > +However, unlike differential channels, the negative input is limited to a narrow
> > > +(taken as constant) voltage range while only IN+ is allowed to swing. A
> > > +pseudo-differential channel can be made out from a differential pair of inputs
> > > +by restricting the negative input to a known voltage while allowing only the
> > > +positive input to swing. Aside from that, some parts have a COM pin that allows
> > > +single-ended inputs to be referenced to a common-mode voltage, making them
> > > +pseudo-differential channels. Often, the common mode input voltage
> > > +can be nicely described in the device tree as a voltage regulator (e.g.
> > > +``com-supply``) since it is basically a constant voltage source.
> > > +
> > > +1.3.1 Pseudo-differential Unipolar Channels
> > > +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > > +
> > > +::
> > > +
> > > +  -------- +VREF ------          +-------------------+
> > > +    ´ `       ´ `               /                    |
> > > +  /     \   /     \   /    --- <  IN+                |
> > > +         `-´       `-´          |                    |
> > > +  --------- IN- -------         |            ADC     |  
> > 
> > The bottom rail should be GND (or -VREF), not IN-. Making it GND in the diagram
> > would be consistent with the other unipolar diagrams and reflect most typical
> > cases. I think the counterexample you gave of AD4170-4 is the unusual case
> > rather than the typical case.
> > 
> > FWIW, when I was first learning this stuff, I didn't really understand bipolar
> > vs. unipolar until I saw diagrams that showed 0V as the bottom rail for unipolar
> > and negative VREF as the bottom rail for bipolar. Even if it isn't strictly
> > true in all cases, seeing the pattern was more helpful. Hard to say if most
> > other people think like me though. :-)
> 
> Maybe IN- (typically GND) is appropriate?
Sounds good to me.
Can also add a typical case (with IN- at GND) example if desired.

> > 
...
> > > +
> > > +1.3.2 Pseudo-differential Bipolar Channels
> > > +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > > +
> > > +::
> > > +
> > > +  -------- +VREF ------          +-------------------+
> > > +    ´ `       ´ `               /                    |
> > > +  /     \   /     \   /    --- <  IN+                |
> > > +         `-´       `-´          |                    |
> > > +  -------- -VREF ------         |            ADC     |
> > > +                                |                    |
> > > +  Common-mode voltage -->  --- <  IN-                |
> > > +                                \       +VREF  -VREF |
> > > +                                 +-------------------+
> > > +                                          ^       ^
> > > +                                          |       +---- External -VREF
> > > +                                   External +VREF
> > > +
> > > +A **pseudo-differential bipolar** input is not limited by the level at IN- but
> > > +it will be limited to -VREF or to GND on the lower end of the input range  
> > 
> > 
> > If it was limited to GND, then it would be uniploar. It is only bipolar if
> > -VREF is less than 0V.
> 
> I'm not sure on that.  See comment on your example above. That is
> what I think a Pseudo-differential bipolar channel is (and it matches the
> first thing google gave me).
> 
> Key here is that common mode voltage on IN- is between -VREF and +VREF so
> we can swing past it and get both positive an negative.

Yes, exactly.

> 
> > 
...
> > > --- a/Documentation/iio/index.rst
> > > +++ b/Documentation/iio/index.rst
> > > @@ -7,6 +7,7 @@ Industrial I/O
> > >  .. toctree::
> > >     :maxdepth: 1
> > >  
> > > +   iio_adc
> > >     iio_configfs
> > >     iio_devbuf
> > >     iio_dmabuf_api
> > > 
> > > base-commit: 9b75dd1b7d6b98699a104c6b1eec0c8817e5fd4b  
> > 
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ