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. >> As for explaining the pseudo-differential, should it be explained? >> A voltage channel within the context of these families is actually >> differential(as there are always two inputs selected). >> The single-ended and pseudo-diff use case is actually wiring up a >> constant voltage to the selected negative input. >> >> I did not consider that this should be described, as there is no >> need for an attribute to describe it. > > I dunno, adding an explanation of it in the text for the channel type > seems trivial to do. "Both pseudo-differential mode (where the > one of differential inputs is connected to a constant voltage) and > single-ended channels will..." > >>> Also, what does "the device register configurations are the same for >>> all uses types" mean? The description here implies that you'd be reading >>> the registers to determine the configuration, but as far as I understand >>> it's the job of drivers to actually configure devices. >>> The only way I could interpret this that makes sense to me is that you're >>> trying to say that the device doesn't have registers that allow you to >>> do runtime configuration detection - but that's the norm and I would not >>> call it out here. >> >> No, I meant that the same register configuration will be set for >> both fully differential and single-ended. >> >> The user will set diff-channels = <0, 1>, bipolar(or not) and >> then they can wire whatever to those pins: >> - a differential signal >> - AVSS to 1 and a single-ended signal to 0 >> - AVSS+offset to 1 and a single-ended signal to 0 >> (which is called pseudo-differential in some datasheets) >> >> All these cases will look the same in terms of configuration > > In that case, I'd just remove this sentence from the description then. > How you configure the registers to use the device doesn't really have > anything to do with describing the configuration of the hardware. > Given it isn't related to configuration detection at runtime, what > you've got written here just makes it seem like the property is > redundant because the register settings do not change. > > Instead, use the description to talk about when the property should be > used and what software should use it to determine, e.g. "Software can > use vendor,channel-type to determine whether or not the measured voltage > is absolute or relative". I pulled that outta my ass, it might not > be what you're actually doing, but I figure you just want to know if > you're measuring from the origin or either side of it. >It's more to the "software can this property to correctly mark the channel as differential or not". Hope this is acceptable. But got it, thanks. > Cheers, > Conor.
