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> >>>>>>> ... >>>> >>>> The AD717x family supports pseudo differential channels as well... should >>>> this change be applied to them too? It is just the case that the documentation >>>> does not mentions this use case. >>> >>> Maybe you could argue that if we used the REF- for the negative input. >>> Otherwise I think it falls into the category where there isn't a clearly defined >>> pseudo differential mode. >>> >> >> While re-reading docs I've noticed that AD7176-2 mentions pseudo differential usage: >> "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. Because there is >> a crosspoint multiplexer, the user can set any of the analog inputs >> as the common pin. An example of such a scenario is to connect >> the AIN4 pin to AVSS or to the REFOUT voltage (that is, AVSS >> + 2.5 V) and select this input when configuring the crosspoint >> multiplexer. When using the AD7176-2 with pseudo differential >> inputs, the INL specification is degraded." >> >> As the crosspoint mux is present on all models it really makes me think that this >> paragraph applies to all models in the family > > Interesting indeed. So is your thinking that we need to support this > or take that "degraded" comment to imply that we should not bother > (at least until someone actually shouts that they want to do this?) > My perspective is that support for this is already existent, the chips do not need any special configuration in that use-case. If we want to be correct in how the channel will be presented to the user, besides setting to false the IIO differential flag I do not see what else should be done. >> >>>> >>>> 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. > 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...) > 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 >> >> 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. 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. >> >> >>> 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 :)
