On 28/04/2024 20:13, Jonathan Cameron wrote: > On Tue, 23 Apr 2024 11:18:47 +0300 > "Ceclan, Dumitru" <mitrutzceclan@xxxxxxxxx> wrote: > >> On 20/04/2024 17:33, Jonathan Cameron wrote: >>> On Mon, 15 Apr 2024 21:42:50 +0300 >>> "Ceclan, Dumitru" <mitrutzceclan@xxxxxxxxx> wrote: >>> >>>> On 13/04/2024 13:49, Jonathan Cameron wrote: >>>>> On Tue, 9 Apr 2024 11:08:28 +0300 >>>>> "Ceclan, Dumitru" <mitrutzceclan@xxxxxxxxx> wrote: >>>>> >>>>>> On 06/04/2024 17:53, Jonathan Cameron wrote: >>>>>>> On Wed, 3 Apr 2024 10:40:39 -0500 >>>>>>> David Lechner <dlechner@xxxxxxxxxxxx> wrote: >>>>>>> >>>>>>>> On Wed, Apr 3, 2024 at 2:43 AM Ceclan, Dumitru <mitrutzceclan@xxxxxxxxx> wrote: >>>>>>>>> >>>>>>>>> On 01/04/2024 22:37, David Lechner wrote: >>>>>>>>>> On Mon, Apr 1, 2024 at 10:10 AM Dumitru Ceclan via B4 Relay >>>>>>>>>> <devnull+dumitru.ceclan.analog.com@xxxxxxxxxx> wrote: >>>>>>>>>>> >>>>>>>>>>> From: Dumitru Ceclan <dumitru.ceclan@xxxxxxxxxx> >>>>>>>>> ... >> >>>> >>>>>> >>>>>> I think that a distinction needs to be made here: >>>>>> - When a device is only pseudo differential, sure, it is in a different category >>>>>> - When a device is fully differential and you are using it as pseudo-differential >>>>>> you are having two inputs compared to one another >>>>>> >>>>>> I would need more clarification is why would supply-vincom be a requirement. >>>>>> The voltage supplied to VINCOM will not be used in any computation within >>>>>> the driver. From the perspective of getting the data it doesn't matter if >>>>>> you are using the channel in a pseudo-differential, single ended or fully >>>>>> differential manner. >>>>> >>>>> I'd missed until now that the datasheet (I looked ad4114) says aincomm should be connected to analog >>>>> ground so indeed nothing to turn on in this case and no offset to supply >>>>> (the offset will be 0 so we don't present it). >>>>> >>>>> I'll note the datasheet describes the VINCOM as follows. >>>>> >>>>> Voltage-Input Copmmon. Voltage inputs are reference to VINCOM when the inputs are configured >>>>> as single-ended. Connect AINCOM to analog ground. >>>>> >>>>> The reference to single ended is pretty clear hint to me that this case >>>>> is not a differential channel. The more complex case is the one David >>>>> raised of the AD4116 where we have actual pseudo differential inputs. >>>>> >>>> >>>> Alright, from my perspective they all pass through the same mux but okay, >>>> not differential. The only issue would differentiating cases in AD4116 where >>>> the pair VIN10 - VINCOM is specified as single-ended or differential pair. >>>> >>>> Also, AD4116: >>>> "0101101111 ADCIN11, ADCIN15. (pseudo differential or differential pair) >>>> 0110001111 ADCIN12, ADCIN15. (pseudo differential or differential pair) >>>> 0110101111 ADCIN13, ADCIN15. (pseudo differential or differential pair) >>>> 0111001111 ADCIN14, ADCIN15. (pseudo differential or differential pair)" >>>> >>>> Not really sure where the "actual pseudo differential" sits. >>>> >>>> Would you agree with having device tree flags that specifies how is the >>>> channel used: single-ended, pseudo-differential, differential. >>>> For the first two, the differential flag will not be set in IIO. >>> >>> Yes. I think that makes sense - though as you observe in some cases >>> the actual device settings end up the same (the ad4116 note above). >>> >> This precisely why I suggest this approach, because a channel used as >> single-ended, pseudo or fully differential will have the same register >> configuration on all models. I do not see any other way to know from >> the driver this information. >> >>> If a given channel supports single-ended and pseudo-differential is >>> that really just a low reference change (I assume from an input to the >>> the IO ground)? Or is there more going on? >>> >> I'm not sure if I understood what was said here. The reference specified >> in the channel setup does not need to change. > > So what is the effective difference? My assumption was that single-ended > means reference to 0V in all cases. Pseudo differential means reference > to an input that is common across multiple channels, but not necessarily 0V? > I do not have a clear answer... This ADI page for example, defines pseudo-differential precisely what we assumed (non 0V on IN-): "On the negative input IN- you apply a DC voltage to shift your signal." https://ez.analog.com/data_converters/precision_adcs/w/documents/2875/difference-between-pseudo-differential-and-single-ended-mode-of-an-adc While this page defines what we call single-ended as "pseudo-differential unipolar". https://www.analog.com/en/resources/technical-articles/sar-adc-input-types.html These two use-cases are not clearly differentiated from one another, what we are referring to as pseudo-differential is called single-ended in some datasheets. I've made a summary of how each datasheet defines these cases: AD411x (1, 2, 4, 5) only mention single-ended as IN- selecting VINCOM. (connected to AVSS) AD4116 mentions both single-ended and pseudo (pseudo specified as IN- connected to a non 0V reference) AD717x (2-2, 3-8) Does not contain pseudo differential references Single-ended is exemplified as IN- connected to 0V or a non 0V reference. AD717x (5-2, 5-8, 7-2) Mentions at the start that pseudo differential is supported. Single-ended is exemplified as both IN- connected to 0V and a non 0V reference. AD7172-4 only mentions single-ended with the example IN- connected to AVSS. (This model does not have the internal reference) AD7176-2 is special, as has only the pseudo differential section but calls them single-ended: "Pseudo Differential Inputs The user can also choose to measure four different single-ended analog inputs. In this case, each of the analog inputs is converted as being the difference between the single-ended input to be measured and a set analog input common pin." >> >>> If it's the reference, then can we provide that as the binding control >>> signal? We have other drivers that do that (though we could perhaps make >>> it more generic) e.g. adi,ad7124 with adi,reference-select >>> We already have adi,reference-select in the binding and driver, I do not >> see how it could help the driver differentiate between (single, pseudo...) > > Indeed that doesn't work. Problem in this discussion is I've normally forgotten > the earlier discussion when I come back to it :( >> >>> I don't like that binding because it always ends up have a local enum >>> of values, but can't really think of a better solution. >>> >> >>>> >>>>>> >>>>>> Regarding VINX VINX+1, it is not mandatory to respect those, from AD4111 page27: >>>>>> "Due to the matching resistors on the analog front end, the >>>>>> differential inputs must be paired together in the following >>>>>> pairs: VIN0 and VIN1, VIN2 and VIN3, VIN4 and VIN5, and >>>>>> VIN6 and VIN7. If any two voltage inputs are paired in a >>>>>> configuration other than what is described in this data sheet, >>>>>> the accuracy of the device cannot be guaranteed." >>>>> >>>>> OK, but I'll assume no 'good' customer of ADI will do that as any support >>>>> engineer would grumpily point at that statement if they ever reported >>>>> a problem :) >>>>> >>>>>> >>>>>> Tried the device and it works as fully differential when pairing any >>>>>> VINx with VINCOM. Still works when selecting VINCOM as the positive >>>>>> input of the ADC. >>>>>> >>>>>> I really see this as overly complicated and unnecessary. These families >>>>>> of ADCs are fully differential. If you are using it to measure a single ended >>>>>> (Be it compared to 0V or pseudo differential where you are comparing to Vref/2 >>>>>> and obtaining results [Vref/2 , -Vref/2]) the final result will not require knowing >>>>>> the common voltage. >>>>> >>>>> For single ended VINCOM should be tied to analog 0V. If the chip docs allowed >>>>> you to tie it to a different voltage then the single ended mode would be offset >>>>> wrt to that value. >>>>> >>>>> For the AD4116 case in pseudo differential mode, You would need an ADCIN15 supply because >>>>> that is not connected to analog 0V. If the device is being used in a pseudo differential >>>>> mode that provides a fixed offset voltage. >>>>> >>>>> So my preference (though I could maybe be convinced it's not worth the effort) >>>>> is to treat pseudo differential as single ended channels where 'negative' pin is >>>>> providing a fixed voltage (or 0V if that's relevant). Thus measurements provided >>>>> to userspace include the information of that offset. >>>>> >>>> >>>> What do you mean by offset? I currently understand that the user will have >>>> a way of reading the voltage of that specific supply from the driver. >>> >>> How? We could do it that way, but we don't have existing ABI for this that >>> I can think of. >>> >> Expose a voltage channel which is not reading from the device...but that is >> too much of a hack to be accepted here > > We have done things like that for a few corner cases where we were really stuck > but it is indeed nasty and hard to comprehend. Also so far they've been 'out' > channels I think which doesn't make sense here. > >>>> >>>> If you mean provide a different channel offset value when using it as >>>> pseudo-differential then I would disagree >>> >>> Provided to user space as _offset on the channel, userspace can either >>> incorporate it if it wants to compute absolute (relative to some 0V somewhere) value >>> or ignore it if it only wants the difference from the reference value. >>> >>> I'm open to discussion other ABI options, but this is the one we most naturally have >>> available. >> _offset is already used when the bipolar coding is enabled on the channel >> and is computed along datasheet specifications of how data should be processed, >> this is why I disagree with this. > > OK. It would be easy enough to apply an offset to that, but it would > complicate the driver and could seem a little odd. > >> >> This feels over-engineered, most of the times if a channel is pseudo >> differential, the relevant measurement will be the differences between >> those two inputs. >> >> If a user needs to know the voltage on the common input, he just needs to >> also configure a single ended channel with the common input where the >> negative AIN is connected to AVSS. > > OK. I'm somewhat convinced that this is enough of a pain to describe that > we should just rely on them having some other way to get that offset if they > are deliberately using it to shift the range. We can revisit if it ever > becomes a problem. > > So, I think the conclusion is just don't represent AIN-COMM (or similar) > as an explicit voltage we can measure. > Perfect. I'll keep the adi,channel-type attribute in the next version (that is there just to control the differential flag from the iio channel struct) and we'll see if it's alright. >>>> >>>>> We haven't handled pseudo differential channels that well in the past, but the >>>>> recent discussions have lead to a cleaner overall solution and it would be good >>>>> to be consistent going forwards. We could deprecate the previous bindings in >>>>> existing drivers, but that is a job for another day (possibly never happens!) >>>>> >>>> >>>> I really hope that a clean solution could be obtained for this driver as well :) >>> >>> I bet you wish sometimes that you had easier parts to write drivers for! :) >>> These continue to stretch the boundaries which is good, but slow. >>> >>> Jonathan >> >> Not easier, fewer crammed into the same driver :) > > I sympathise! It's been an annoyingly busy kernel cycle in the day job. I was hoping to > get back to you sooner so that more of this was fresh(ish) in my mind :( > > My gut feeling is that this is a case for documentation / really detailed cover > letter for the next version to make sure we have come to at least a (mostly) > consistent conclusion. > > Jonathan >
