On Wed, 2024-05-29 at 17:04 -0500, David Lechner wrote: > On 5/29/24 8:38 AM, Ceclan, Dumitru wrote: > > On 28/05/2024 20:52, Conor Dooley wrote: > > > On Tue, May 28, 2024 at 03:16:07PM +0300, Ceclan, Dumitru wrote: > > > > On 27/05/2024 20:48, Conor Dooley wrote: > > > > > On Mon, May 27, 2024 at 08:02:34PM +0300, Dumitru Ceclan via B4 Relay > > > > > wrote: > > > > > > From: Dumitru Ceclan <dumitru.ceclan@xxxxxxxxxx> > > > > > > > > > > > > Add support for: AD4111, AD4112, AD4114, AD4115, AD4116. > > > > > > > > > > > > AD411x family ADCs support a VCOM pin, dedicated for single-ended usage. > > > > > > AD4111/AD4112 support current channels, usage is implemented by > > > > > > specifying channel reg values bigger than 15. > > > > > > > > > > > > Signed-off-by: Dumitru Ceclan <dumitru.ceclan@xxxxxxxxxx> > > > > > > --- > > > > > > .../devicetree/bindings/iio/adc/adi,ad7173.yaml | 122 > > > > > > ++++++++++++++++++++- > > > > > > 1 file changed, 120 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..5b1af382dad3 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,36 @@ patternProperties: > > > > > > maximum: 15 > > > > > > > > > > > > diff-channels: > > > > > > + description: | > > > > > > + For using current channels specify select the current inputs > > > > > > + and enable the adi,current-channel property. > > > > > > + > > > > > > + 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)− > > > > > > + > > > > > > items: > > > > > > minimum: 0 > > > > > > maximum: 31 > > > > > > > > > > > > + single-channel: > > > > > > + description: | > > > > > > + Models AD4111 and AD4112 support single-ended current > > > > > > channels. > > > > > > + To select the desired current input, specify the desired input > > > > > > pair: > > > > > > + (IIN2+, IIN2−) -> single-channel = <2> > > > > > > + > > > > > > + items: > > > > > > + minimum: 1 > > > > > > + maximum: 16 > > > > > > + > > > > > > adi,reference-select: > > > > > > description: | > > > > > > Select the reference source to use when converting on > > > > > > @@ -154,9 +196,26 @@ patternProperties: > > > > > > - avdd > > > > > > default: refout-avss > > > > > > > > > > > > + adi,current-channel: > > > > > > + description: | > > > > > > + Signal that the selected inputs are current channels. > > > > > > + Only available on AD4111 and AD4112. > > > > > > + type: boolean > > > > > > + > > > > > > + adi,channel-type: > > > > > > + description: > > > > > > + Used to differentiate between different channel types as the > > > > > > device > > > > > > + register configurations are the same for all usage types. > > > > > > + Both pseudo-differential and single-ended channels will use > > > > > > the > > > > > > + single-ended specifier. > > > > > > + $ref: /schemas/types.yaml#/definitions/string > > > > > > + enum: > > > > > > + - single-ended > > > > > > + - differential > > > > > > + default: differential > > > > > > > > > > I dunno if my brain just ain't workin' right today, or if this is not > > > > > sufficiently explained, but why is this property needed? You've got > > > > > diff-channels and single-channels already, why can you not infer the > > > > > information you need from them? What should software do with this > > > > > information? > > > > > Additionally, "pseudo-differential" is not explained in this binding. > > > > > > > > In previous thread we arrived to the conclusion single-ended and > > > > pseudo-differential channels should be marked with the flag > > > > "differential=false" in the IIO channel struct. This cannot > > > > really be inferred as any input pair could be used in that > > > > manner and the only difference would be in external wiring. > > > > > > > > Single-channels cannot be used to define such a channel as > > > > two voltage inputs need to be selected. Also, we are already > > > > using single-channel to define the current channels. > > > > > > If I understand correctly, the property could be simplified to a flag > > > then, since it's only the pseudo differential mode that you cannot be > > > sure of? > > > You know when you're single-ended based on single-channel, so the > > > additional info you need is only in the pseudo-differential case. > > > > > Yes, it could just be a boolean flag. The only thing I have against > > that is the awkwardness of having both diff-channels and > > differential=false within a channel definition. > > > > No, there is no uncertainty regarding pseudo-differential, it's > > basically single-ended. > > > > We cannot use single-channel for voltage channels, two voltage > > inputs need to be specified. And again, single-channel will be > > used here for the current channels. > > Instead of using diff-channels for single-ended/pseudo-differential > plus a property that says "actually not differential" could we just > add a second common-mode-channel property to specify the second > input pin that is connected to the common mode voltage source? > Yeah, I definitely don't like having diff-channels and then go "oh, not really a differential channel". So, if could avoid that, it would be great! > Just to make sure I'm understanding, single-ended means common mode > voltage is 0V (or AVSS for this chip, I guess) and pseudo-differential > means the common mode voltage is something else other than that. > In other words, single-ended is just a special case of pseudo-differential. > So effectively, no difference that we need to describe? > > So something like this could work? > > > /* a fully differential voltage input channel */ > channel@0 { > reg = <0>; > bipolar; > diff-channels = <0 1>; /* VIN0 is +, VIN1 is - */ > adi,reference-select = "vref"; > }; > > /* a single-ended voltage input channel */ > channel@1 { > reg = <1>; > /* no bipolar since common mode is 0V */ > single-channel = <2>; /* VIN2 is input */ > common-mode-channel = <3>; /* VIN3 connected to 0V */ > }; > > /* a pseudo-differential voltage input channel */ > channel@2 { > reg = <2>; > bipolar; /* since common mode is not 0V */ > single-channel = <4>; /* VIN4 is input */ > common-mode-channel = <5>; /* VIN5 connected to Vref / 2 */ > adi,reference-select = "vref"; > }; > > /* a current input channel */ > channel@3 { > reg = <3>; > bipolar; > /* 0 is not the same pin as channel@0 because of > * the adi,current-channel flag */ > single-channel = <0>; /* using IIN0+ and IIN0- pins */ > adi,current-channel; Unless I'm missing something, this flag is also not being used anywhere in the current series? - Nuno Sá