| Message ID | 20240527-ad4111-v3-1-7e9eddbbd3eb@analog.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add support for AD411x | expand |
On Mon, May 27, 2024 at 08:02:34PM +0300, Dumitru Ceclan via B4 Relay wrote: > From: Dumitru Ceclan <dumitru.ceclan@analog.com> > > 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@analog.com> > --- > .../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. 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. Thanks, Conor.
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@analog.com> >> >> 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@analog.com> >> --- >> .../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. 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. > 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 > > Thanks, > Conor.
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@analog.com> > >> > >> 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@analog.com> > >> --- > >> .../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. > 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. Cheers, Conor.
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@analog.com> >>>> >>>> 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@analog.com> >>>> --- >>>> .../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.
On Wed, May 29, 2024 at 04:38:53PM +0300, 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@analog.com> > >>>> + 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. What I was suggesting was more like "adi,pseudo-differential" (you don't need to set the =false or w/e, flag properties work based on present/not present). I think that would avoid the awkwardness? > >> 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 Your quoting is scuffed here, I didn't write this! > as differential or not". Hope this is acceptable. But got it, thanks. As long as you've got a description that tells the OS what the property actually represents, I'm happy.
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@analog.com> >>>>> >>>>> 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@analog.com> >>>>> --- >>>>> .../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? 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; }; If we really wanted to go all the way, we could also think about adding a common-mode-supply property in each channel node to with a common-mode-channel property to describe the voltage source that the pin is connected to.
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@analog.com> > > > > > > > > > > > > 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@analog.com> > > > > > > --- > > > > > > .../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á
On Wed, 2024-05-29 at 17:04 +0100, Conor Dooley wrote: > On Wed, May 29, 2024 at 04:38:53PM +0300, 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@analog.com> > > > > > > + 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. > > What I was suggesting was more like "adi,pseudo-differential" (you don't > need to set the =false or w/e, flag properties work based on present/not > present). I think that would avoid the awkwardness? > Yeah, that was also my understanding of your reply... But I think you're also mentioning to have this flag together with the single-channel property? I'm a bit confused because it seems to me that single-channel only gets one input while we need to select two for pseudo-differential/single-ended. Is this correct Dumitru? FWIW, I think we already have that awkwardness in the current form. If I'm not missing anything, what we have in the driver is pretty much: if (not diff && single-channel) // then current channel else // relies on the channel-type stuff So, effectively with the above we have diff-channels = <1 0>; but then wait, not so fast adi,channel-type = "single-ended" To me the above is equally awkward (not sure if there's any precedence in using diff- channels like this though)... I would like for this to be explicit... If we have diff-channels, then it's surely differential. If not we could use the single-channel property and instead of using an extra flag we could make the property having either 1 or 2 items. If we have 1, then it's a current channel. If we have 2, then it's voltage single-ended/pseudo-differential. David's suggestion is also pretty good (and I like it's more explicit about what's going on) so I would likely go with it... - Nuno Sá > > > >
On 30/05/2024 10:50, Nuno Sá wrote: > On Wed, 2024-05-29 at 17:04 +0100, Conor Dooley wrote: >> On Wed, May 29, 2024 at 04:38:53PM +0300, 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@analog.com> >>>>>>> + 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. >> >> What I was suggesting was more like "adi,pseudo-differential" (you don't >> need to set the =false or w/e, flag properties work based on present/not >> present). I think that would avoid the awkwardness? >> > > Yeah, that was also my understanding of your reply... But I think you're also > mentioning to have this flag together with the single-channel property? > > I'm a bit confused because it seems to me that single-channel only gets one input > while we need to select two for pseudo-differential/single-ended. Is this correct > Dumitru? > Yes, that is correct. > FWIW, I think we already have that awkwardness in the current form. If I'm not > missing anything, what we have in the driver is pretty much: > > if (not diff && single-channel) > // then current channel > else > // relies on the channel-type stuff > > So, effectively with the above we have > > diff-channels = <1 0>; > > but then wait, not so fast > This comment properly and comically describes the hot mess that I've come up with :))) > adi,channel-type = "single-ended" > > To me the above is equally awkward (not sure if there's any precedence in using diff- > channels like this though)... > > I would like for this to be explicit... If we have diff-channels, then it's surely > differential. > > If not we could use the single-channel property and instead of using an extra flag we > could make the property having either 1 or 2 items. If we have 1, then it's a current > channel. If we have 2, then it's voltage single-ended/pseudo-differential. > > David's suggestion is also pretty good (and I like it's more explicit about what's > going on) so I would likely go with it... > > - Nuno Sá > > Yup, as neat as it could be, I'll do it that way.
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 + required: - reg - - diff-channels required: - compatible @@ -166,7 +225,6 @@ allOf: - $ref: /schemas/spi/spi-peripheral-props.yaml# # Only ad7172-4, ad7173-8 and ad7175-8 support vref2 - # Other models have [0-3] channel registers - if: properties: compatible: @@ -187,6 +245,37 @@ allOf: - vref - refout-avss - avdd + + - if: + properties: + compatible: + contains: + enum: + - adi,ad4114 + - adi,ad4115 + - adi,ad4116 + - adi,ad7173-8 + - adi,ad7175-8 + then: + patternProperties: + "^channel@[0-9a-f]$": + properties: + reg: + maximum: 15 + + - if: + properties: + compatible: + contains: + enum: + - adi,ad7172-2 + - adi,ad7175-2 + - adi,ad7176-2 + - adi,ad7177-2 + then: + patternProperties: + "^channel@[0-9a-f]$": + properties: reg: maximum: 3 @@ -210,6 +299,35 @@ allOf: required: - adi,reference-select + - if: + properties: + compatible: + contains: + enum: + - adi,ad4111 + - adi,ad4112 + - adi,ad4114 + - adi,ad4115 + - adi,ad4116 + then: + properties: + avdd2-supply: false + + - if: + properties: + compatible: + not: + contains: + enum: + - adi,ad4111 + - adi,ad4112 + then: + patternProperties: + "^channel@[0-9a-f]$": + properties: + single-channel: false + adi,current-channel: false + - if: anyOf: - required: [clock-names]