Message ID | 20231212104451.22522-1-mitrutzceclan@gmail.com (mailing list archive) |
---|---|
State | Changes Requested |
Headers | show |
Series | [v8,1/2] dt-bindings: adc: add AD7173 | expand |
On Tue, Dec 12, 2023 at 11:45 AM Dumitru Ceclan <mitrutzceclan@gmail.com> wrote: > > The AD7173 family offer a complete integrated Sigma-Delta ADC solution > which can be used in high precision, low noise single channel applications > or higher speed multiplexed applications. The Sigma-Delta ADC is intended > primarily for measurement of signals close to DC but also delivers > outstanding performance with input bandwidths out to ~10kHz. As stated in [1], we should try to make complete bindings. I think more could be done here to make this more complete. Most notably, the gpio-controller binding is missing. Also maybe something is needed to describe how the SYNC/ERROR pin is wired up since it can be an input or an output with different functions? [1]: https://www.kernel.org/doc/html/latest/devicetree/bindings/writing-bindings.html > > Signed-off-by: Dumitru Ceclan <mitrutzceclan@gmail.com> > --- > V7->V8 > - include missing fix from V6 Including the cumulative changelog for all revisions would be helpful to reviewers who haven't been following closely. > > .../bindings/iio/adc/adi,ad7173.yaml | 170 ++++++++++++++++++ > 1 file changed, 170 insertions(+) > create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml > > diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml > new file mode 100644 > index 000000000000..25a5404ee353 > --- /dev/null > +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml > @@ -0,0 +1,170 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +# Copyright 2023 Analog Devices Inc. > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/iio/adc/adi,ad7173.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Analog Devices AD7173 ADC > + > +maintainers: > + - Ceclan Dumitru <dumitru.ceclan@analog.com> > + > +description: | > + Bindings for the Analog Devices AD717X ADC's. Datasheets for supported chips: > + https://www.analog.com/media/en/technical-documentation/data-sheets/AD7172-2.pdf > + https://www.analog.com/media/en/technical-documentation/data-sheets/AD7173-8.pdf > + https://www.analog.com/media/en/technical-documentation/data-sheets/AD7175-2.pdf > + https://www.analog.com/media/en/technical-documentation/data-sheets/AD7176-2.pdf > + > +properties: > + compatible: > + enum: > + - adi,ad7172-2 > + - adi,ad7173-8 > + - adi,ad7175-2 > + - adi,ad7176-2 > + > + reg: > + maxItems: 1 > + > + interrupts: > + maxItems: 1 Shouldn't this be 2? The datasheet says there is a "Data Output Ready" signal on the DOUT/RDY pin and an "Error Output" on the SYNC/ERROR pin. Although I could see how RDY could be considered part of the SPI bus. In any case, a description explaining what the interrupt is would be useful. > + > + '#address-cells': > + const: 1 > + > + '#size-cells': > + const: 0 > + > + spi-max-frequency: > + maximum: 20000000 > + > + refin-supply: > + description: external reference supply, can be used as reference for conversion. > + > + refin2-supply: > + description: external reference supply, can be used as reference for conversion. > + > + avdd-supply: > + description: avdd supply, can be used as reference for conversion. What about other supplies? AVDD1, AVDD2, IOVDD. > + > +patternProperties: > + "^channel@[0-9a-f]$": > + type: object > + $ref: adc.yaml > + unevaluatedProperties: false > + > + properties: > + reg: > + minimum: 0 > + maximum: 15 > + > + diff-channels: > + items: > + minimum: 0 > + maximum: 31 Do we need to add overrides to limit the maximums for each compatible string? > + > + adi,reference-select: > + description: | > + Select the reference source to use when converting on > + the specific channel. Valid values are: > + refin : REFIN(+)/REFIN(−). > + refin2 : REFIN2(+)/REFIN2(−) > + refout-avss: REFOUT/AVSS (Internal reference) > + avdd : AVDD > + > + External reference refin2 only available on ad7173-8. > + If not specified, internal reference used. > + enum: > + - refin > + - refin2 > + - refout-avss > + - avdd > + default: refout-avss Missing string type? > + > + required: > + - reg > + - diff-channels Individual analog inputs can be used as single-ended or in pairs as differential, right? If so, diff-channels should not be required to allow for single-ended use. And we would need to add something like a single-ended-channel property to adc.yaml to allow mapping analog input pins to channels similar to how diff-channels works, I think (I don't see anything like that there already)? So maybe something like: oneOf: - required: single-ended-channel - required: diff-channels > + > +required: > + - compatible > + - reg > + - interrupts > + > +allOf: > + - $ref: /schemas/spi/spi-peripheral-props.yaml# > + > + - if: > + properties: > + compatible: > + not: > + contains: > + const: adi,ad7173-8 > + then: > + properties: > + refin2-supply: false > + patternProperties: > + "^channel@[0-9a-f]$": > + properties: > + adi,reference-select: > + enum: > + - refin > + - refout-avss > + - avdd > + > +unevaluatedProperties: false > + > +examples: > + - | > + #include <dt-bindings/gpio/gpio.h> > + #include <dt-bindings/interrupt-controller/irq.h> > + > + spi { > + #address-cells = <1>; > + #size-cells = <0>; > + > + adc@0 { > + compatible = "adi,ad7173-8"; > + reg = <0>; > + > + #address-cells = <1>; > + #size-cells = <0>; > + > + interrupts = <25 IRQ_TYPE_EDGE_FALLING>; > + interrupt-parent = <&gpio>; > + spi-max-frequency = <5000000>; > + > + refin-supply = <&dummy_regulator>; > + > + channel@0 { > + reg = <0>; > + bipolar; > + diff-channels = <0 1>; > + adi,reference-select = "refin"; > + }; > + > + channel@1 { > + reg = <1>; > + diff-channels = <2 3>; > + }; > + > + channel@2 { > + reg = <2>; > + bipolar; > + diff-channels = <4 5>; > + }; > + > + channel@3 { > + reg = <3>; > + bipolar; > + diff-channels = <6 7>; > + }; > + > + channel@4 { > + reg = <4>; > + diff-channels = <8 9>; > + adi,reference-select = "avdd"; > + }; > + }; > + }; > -- > 2.42.0 > >
On 12/12/23 17:09, David Lechner wrote: > On Tue, Dec 12, 2023 at 11:45 AM Dumitru Ceclan <mitrutzceclan@gmail.com> wrote: >> >> The AD7173 family offer a complete integrated Sigma-Delta ADC solution >> which can be used in high precision, low noise single channel applications >> or higher speed multiplexed applications. The Sigma-Delta ADC is intended >> primarily for measurement of signals close to DC but also delivers >> outstanding performance with input bandwidths out to ~10kHz. > > As stated in [1], we should try to make complete bindings. I think > more could be done here to make this more complete. Most notably, the > gpio-controller binding is missing. Also maybe something is needed to > describe how the SYNC/ERROR pin is wired up since it can be an input > or an output with different functions? > GPIO-controller: '#gpio-cells': const: 2 gpio-controller: true Like this, in properties? Sync can only be an output, Error is configurable. Are there any examples for how something like this is described? ... >> + interrupts: >> + maxItems: 1 > > Shouldn't this be 2? The datasheet says there is a "Data Output Ready" > signal on the DOUT/RDY pin and an "Error Output" on the SYNC/ERROR > pin. Although I could see how RDY could be considered part of the SPI > bus. In any case, a description explaining what the interrupt is would > be useful. > I do not see how there could be 2 interrupts. DOUT/RDY is used as an interrupt when waiting for a conversion to finalize. Sync and Error are sepparate pins, Sync(if enabled) works only as an input that resets the modulator and the digital filter. Error can be configured as input, output or ERROR output (OR between all internal error sources). Would this be alright interrupts: description: Conversion completion interrupt. Pin is shared with SPI DOUT. maxItems: 1 ... >> + >> +patternProperties: >> + "^channel@[0-9a-f]$": >> + type: object >> + $ref: adc.yaml >> + unevaluatedProperties: false >> + >> + properties: >> + reg: >> + minimum: 0 >> + maximum: 15 >> + >> + diff-channels: >> + items: >> + minimum: 0 >> + maximum: 31 > > Do we need to add overrides to limit the maximums for each compatible string? > Just to be sure, in the allOf section? If yes, is there any other more elegant method to obtain this behavior? ... >> + >> + required: >> + - reg >> + - diff-channels > > Individual analog inputs can be used as single-ended or in pairs as > differential, right? If so, diff-channels should not be required to > allow for single-ended use. > > And we would need to add something like a single-ended-channel > property to adc.yaml to allow mapping analog input pins to channels > similar to how diff-channels works, I think (I don't see anything like > that there already)? > > So maybe something like: > > oneOf: > - required: > single-ended-channel > - required: > diff-channels > All channels must specify 2 analog input sources, there is no input source wired by default to AVSS. In my opinion, there is no need to specify channels as single-ended because that would require a property that specifies the input that is wired to AVSS.
On Thu, Dec 14, 2023 at 1:43 PM Ceclan Dumitru <mitrutzceclan@gmail.com> wrote: > > > > On 12/12/23 17:09, David Lechner wrote: > > On Tue, Dec 12, 2023 at 11:45 AM Dumitru Ceclan <mitrutzceclan@gmail.com> wrote: > >> > >> The AD7173 family offer a complete integrated Sigma-Delta ADC solution > >> which can be used in high precision, low noise single channel applications > >> or higher speed multiplexed applications. The Sigma-Delta ADC is intended > >> primarily for measurement of signals close to DC but also delivers > >> outstanding performance with input bandwidths out to ~10kHz. > > > > As stated in [1], we should try to make complete bindings. I think > > more could be done here to make this more complete. Most notably, the > > gpio-controller binding is missing. Also maybe something is needed to > > describe how the SYNC/ERROR pin is wired up since it can be an input > > or an output with different functions? > > > > GPIO-controller: > '#gpio-cells': > > const: 2 > > > gpio-controller: true > Like this, in properties? Yes (with not so many blank lines). > > Sync can only be an output, Error is configurable. Are there any > examples for how something like this is described? > Configurable pins sounds like a pinmux. Sounds a bit overkill to specify everything for a pin-controller for one pin if no one is ever going to use it. But I will leave it to the DT maintainers to say how complete the bindings should be. > ... > > >> + interrupts: > >> + maxItems: 1 > > > > Shouldn't this be 2? The datasheet says there is a "Data Output Ready" > > signal on the DOUT/RDY pin and an "Error Output" on the SYNC/ERROR > > pin. Although I could see how RDY could be considered part of the SPI > > bus. In any case, a description explaining what the interrupt is would > > be useful. > > > > I do not see how there could be 2 interrupts. DOUT/RDY is used as an > interrupt when waiting for a conversion to finalize. > > Sync and Error are sepparate pins, Sync(if enabled) works only as an > input that resets the modulator and the digital filter. I only looked at the AD7172-2 datasheet and pin 15 is labeled SYNC/ERROR. Maybe they are separate pins on other chips? > > Error can be configured as input, output or ERROR output (OR between all > internal error sources). > > Would this be alright > interrupts: > > description: Conversion completion interrupt. > Pin is shared with SPI DOUT. > maxItems: 1 Since ERROR is an output, I would expect it to be an interrupt. The RDY output, on the other hand, would be wired to a SPI controller with the SPI_READY feature (I use the Linux flag name here because I'm not aware of a corresponding devicetree flag). So I don't think the RDY signal would be an interrupt. > > ... > > >> + > >> +patternProperties: > >> + "^channel@[0-9a-f]$": > >> + type: object > >> + $ref: adc.yaml > >> + unevaluatedProperties: false > >> + > >> + properties: > >> + reg: > >> + minimum: 0 > >> + maximum: 15 > >> + > >> + diff-channels: > >> + items: > >> + minimum: 0 > >> + maximum: 31 > > > > Do we need to add overrides to limit the maximums for each compatible string? > > > > Just to be sure, in the allOf section? > If yes, is there any other more elegant method to obtain this behavior? I'm not sure. I would like to know if there is a more elegant way as well. ;-) > > ... > > >> + > >> + required: > >> + - reg > >> + - diff-channels > > > > Individual analog inputs can be used as single-ended or in pairs as > > differential, right? If so, diff-channels should not be required to > > allow for single-ended use. > > > > And we would need to add something like a single-ended-channel > > property to adc.yaml to allow mapping analog input pins to channels > > similar to how diff-channels works, I think (I don't see anything like > > that there already)? > > > > So maybe something like: > > > > oneOf: > > - required: > > single-ended-channel > > - required: > > diff-channels > > > All channels must specify 2 analog input sources, there is no input > source wired by default to AVSS. > > In my opinion, there is no need to specify channels as single-ended > because that would require a property that specifies the input that is > wired to AVSS. Makes sense to me.
On 12/14/23 18:12, David Lechner wrote: > On Thu, Dec 14, 2023 at 1:43 PM Ceclan Dumitru <mitrutzceclan@gmail.com> wrote: >> On 12/12/23 17:09, David Lechner wrote: >>> On Tue, Dec 12, 2023 at 11:45 AM Dumitru Ceclan <mitrutzceclan@gmail.com> wrote: >> ... >> >>>> + interrupts: >>>> + maxItems: 1 >>> >>> Shouldn't this be 2? The datasheet says there is a "Data Output Ready" >>> signal on the DOUT/RDY pin and an "Error Output" on the SYNC/ERROR >>> pin. Although I could see how RDY could be considered part of the SPI >>> bus. In any case, a description explaining what the interrupt is would >>> be useful. >>> >> >> I do not see how there could be 2 interrupts. DOUT/RDY is used as an >> interrupt when waiting for a conversion to finalize. >> >> Sync and Error are sepparate pins, Sync(if enabled) works only as an >> input that resets the modulator and the digital filter. > > I only looked at the AD7172-2 datasheet and pin 15 is labeled > SYNC/ERROR. Maybe they are separate pins on other chips? Yep, sorry, missed that. All other supported models have them separate. > >> >> Error can be configured as input, output or ERROR output (OR between all >> internal error sources). >> >> Would this be alright >> interrupts: >> >> description: Conversion completion interrupt. >> Pin is shared with SPI DOUT. >> maxItems: 1 > > Since ERROR is an output, I would expect it to be an interrupt. The > RDY output, on the other hand, would be wired to a SPI controller with > the SPI_READY feature (I use the Linux flag name here because I'm not > aware of a corresponding devicetree flag). So I don't think the RDY > signal would be an interrupt. > Error does not have the purpose to be an interrupt. The only interrupt used from this chip is the one from the DOUT/~RDY pin. Sure, it is wired to the SPI controller, but when you can't also receive interrupts on that very same CPU pin an issue arises. So that pin is also wired to another GPIO with interrupt support. This is the same way that ad4130.yaml is written for example (with the exception that ad4130 supports configuring where the interrupt is routed). In regards to SPI_READY _BITUL(7) /* slave pulls low to pause */: the ad_sigma_delta framework (if it can be called that) is written to expect a pin interrupt, not to use SPI_READY controller feature.
On Thu, 14 Dec 2023 19:03:28 +0200 Ceclan Dumitru <mitrutzceclan@gmail.com> wrote: > On 12/14/23 18:12, David Lechner wrote: > > On Thu, Dec 14, 2023 at 1:43 PM Ceclan Dumitru <mitrutzceclan@gmail.com> wrote: > >> On 12/12/23 17:09, David Lechner wrote: > >>> On Tue, Dec 12, 2023 at 11:45 AM Dumitru Ceclan <mitrutzceclan@gmail.com> wrote: > > >> ... > >> > >>>> + interrupts: > >>>> + maxItems: 1 > >>> > >>> Shouldn't this be 2? The datasheet says there is a "Data Output Ready" > >>> signal on the DOUT/RDY pin and an "Error Output" on the SYNC/ERROR > >>> pin. Although I could see how RDY could be considered part of the SPI > >>> bus. In any case, a description explaining what the interrupt is would > >>> be useful. > >>> > >> > >> I do not see how there could be 2 interrupts. DOUT/RDY is used as an > >> interrupt when waiting for a conversion to finalize. > >> > >> Sync and Error are sepparate pins, Sync(if enabled) works only as an > >> input that resets the modulator and the digital filter. > > > > I only looked at the AD7172-2 datasheet and pin 15 is labeled > > SYNC/ERROR. Maybe they are separate pins on other chips? > > Yep, sorry, missed that. All other supported models have them separate. > > > >> > >> Error can be configured as input, output or ERROR output (OR between all > >> internal error sources). > >> > >> Would this be alright > >> interrupts: > >> > >> description: Conversion completion interrupt. > >> Pin is shared with SPI DOUT. > >> maxItems: 1 > > > > Since ERROR is an output, I would expect it to be an interrupt. The > > RDY output, on the other hand, would be wired to a SPI controller with > > the SPI_READY feature (I use the Linux flag name here because I'm not > > aware of a corresponding devicetree flag). So I don't think the RDY > > signal would be an interrupt. > > > > Error does not have the purpose to be an interrupt. The only interrupt > used from this chip is the one from the DOUT/~RDY pin. Sure, it is wired > to the SPI controller, but when you can't also receive interrupts on > that very same CPU pin an issue arises. So that pin is also wired to > another GPIO with interrupt support. You've lost me. It's a pin that has a state change when an error condition occurs. Why not an interrupt? Doesn't matter that the driver doesn't use this functionality. dt-bindings should be as comprehensive as possible. Given it's a multipurpose pin you'd also want to support it as a gpio to be complete alongside the other GPIOs. > > This is the same way that ad4130.yaml is written for example (with the > exception that ad4130 supports configuring where the interrupt is routed). > > In regards to SPI_READY _BITUL(7) /* slave pulls low to pause */: the > ad_sigma_delta framework (if it can be called that) is written to expect > a pin interrupt, not to use SPI_READY controller feature. SPI_READY is supported by only a couple of controllers. I'm not even that sure exactly how it is defined and whether that lines up with this usecase. From some old asci art. https://lore.kernel.org/all/1456747459-8559-1-git-send-email-linux@rempel-privat.de/ Flow control: Ready Sequence Master CS |-----1\_______________________| Slave FC |--------2\____________________| DATA |-----------3\_________________| So you set master and then wait for a flow control pin (the ready signal) before you can actually talk to the device. Here we are indicating data is ready to be be read out. So I don't 'think' SPI_READY applies. Jonathan >
On Sun, Dec 17, 2023 at 7:50 AM Jonathan Cameron <jic23@kernel.org> wrote: > > On Thu, 14 Dec 2023 19:03:28 +0200 > Ceclan Dumitru <mitrutzceclan@gmail.com> wrote: > > > On 12/14/23 18:12, David Lechner wrote: > > > On Thu, Dec 14, 2023 at 1:43 PM Ceclan Dumitru <mitrutzceclan@gmail.com> wrote: > > >> On 12/12/23 17:09, David Lechner wrote: > > >>> On Tue, Dec 12, 2023 at 11:45 AM Dumitru Ceclan <mitrutzceclan@gmail.com> wrote: > > > > >> ... > > >> > > >>>> + interrupts: > > >>>> + maxItems: 1 > > >>> > > >>> Shouldn't this be 2? The datasheet says there is a "Data Output Ready" > > >>> signal on the DOUT/RDY pin and an "Error Output" on the SYNC/ERROR > > >>> pin. Although I could see how RDY could be considered part of the SPI > > >>> bus. In any case, a description explaining what the interrupt is would > > >>> be useful. > > >>> > > >> > > >> I do not see how there could be 2 interrupts. DOUT/RDY is used as an > > >> interrupt when waiting for a conversion to finalize. > > >> > > >> Sync and Error are sepparate pins, Sync(if enabled) works only as an > > >> input that resets the modulator and the digital filter. > > > > > > I only looked at the AD7172-2 datasheet and pin 15 is labeled > > > SYNC/ERROR. Maybe they are separate pins on other chips? > > > > Yep, sorry, missed that. All other supported models have them separate. > > > > > > > >> > > >> Error can be configured as input, output or ERROR output (OR between all > > >> internal error sources). > > >> > > >> Would this be alright > > >> interrupts: > > >> > > >> description: Conversion completion interrupt. > > >> Pin is shared with SPI DOUT. > > >> maxItems: 1 > > > > > > Since ERROR is an output, I would expect it to be an interrupt. The > > > RDY output, on the other hand, would be wired to a SPI controller with > > > the SPI_READY feature (I use the Linux flag name here because I'm not > > > aware of a corresponding devicetree flag). So I don't think the RDY > > > signal would be an interrupt. > > > > > > > Error does not have the purpose to be an interrupt. The only interrupt > > used from this chip is the one from the DOUT/~RDY pin. Sure, it is wired > > to the SPI controller, but when you can't also receive interrupts on > > that very same CPU pin an issue arises. So that pin is also wired to > > another GPIO with interrupt support. > > You've lost me. It's a pin that has a state change when an error condition > occurs. Why not an interrupt? Doesn't matter that the driver doesn't > use this functionality. dt-bindings should be as comprehensive as possible. > Given it's a multipurpose pin you'd also want to support it as a gpio to be > complete alongside the other GPIOs. > > > > > This is the same way that ad4130.yaml is written for example (with the > > exception that ad4130 supports configuring where the interrupt is routed). > > > > In regards to SPI_READY _BITUL(7) /* slave pulls low to pause */: the > > ad_sigma_delta framework (if it can be called that) is written to expect > > a pin interrupt, not to use SPI_READY controller feature. > > SPI_READY is supported by only a couple of controllers. I'm not even that > sure exactly how it is defined and whether that lines up with this usecase. > From some old asci art. https://lore.kernel.org/all/1456747459-8559-1-git-send-email-linux@rempel-privat.de/ > > Flow control: Ready Sequence > Master CS |-----1\_______________________| > Slave FC |--------2\____________________| > DATA |-----------3\_________________| > > So you set master and then wait for a flow control pin (the ready signal) before > you can actually talk to the device. > > Here we are indicating data is ready to be be read out. > > So I don't 'think' SPI_READY applies. > > Jonathan > I'm not arguing that SPI_READY applies in this particular case, but FWIW it does seem to me like... 1) SPI_READY could be implemented in any SPI driver using a GPIO interrupt (similar to how we already have GPIO CS) 2) In cases where the SPI controller does have actual hardware support for SPI_READY and the ADC chip A) uses CS to trigger a conversion and B) has a "busy" signal that goes low when the conversion is complete, then the SPI_READY feature could be used to make reading sample data more efficient by avoiding any CPU intervention between CS assertion and starting the data xfer due to waiting for the conversion to complete either by waiting for an interrupt or sleeping for a fixed amount of time.
On Sun, 17 Dec 2023 19:00:32 -0600 David Lechner <dlechner@baylibre.com> wrote: > On Sun, Dec 17, 2023 at 7:50 AM Jonathan Cameron <jic23@kernel.org> wrote: > > > > On Thu, 14 Dec 2023 19:03:28 +0200 > > Ceclan Dumitru <mitrutzceclan@gmail.com> wrote: > > > > > On 12/14/23 18:12, David Lechner wrote: > > > > On Thu, Dec 14, 2023 at 1:43 PM Ceclan Dumitru <mitrutzceclan@gmail.com> wrote: > > > >> On 12/12/23 17:09, David Lechner wrote: > > > >>> On Tue, Dec 12, 2023 at 11:45 AM Dumitru Ceclan <mitrutzceclan@gmail.com> wrote: > > > > > > >> ... > > > >> > > > >>>> + interrupts: > > > >>>> + maxItems: 1 > > > >>> > > > >>> Shouldn't this be 2? The datasheet says there is a "Data Output Ready" > > > >>> signal on the DOUT/RDY pin and an "Error Output" on the SYNC/ERROR > > > >>> pin. Although I could see how RDY could be considered part of the SPI > > > >>> bus. In any case, a description explaining what the interrupt is would > > > >>> be useful. > > > >>> > > > >> > > > >> I do not see how there could be 2 interrupts. DOUT/RDY is used as an > > > >> interrupt when waiting for a conversion to finalize. > > > >> > > > >> Sync and Error are sepparate pins, Sync(if enabled) works only as an > > > >> input that resets the modulator and the digital filter. > > > > > > > > I only looked at the AD7172-2 datasheet and pin 15 is labeled > > > > SYNC/ERROR. Maybe they are separate pins on other chips? > > > > > > Yep, sorry, missed that. All other supported models have them separate. > > > > > > > > > > > >> > > > >> Error can be configured as input, output or ERROR output (OR between all > > > >> internal error sources). > > > >> > > > >> Would this be alright > > > >> interrupts: > > > >> > > > >> description: Conversion completion interrupt. > > > >> Pin is shared with SPI DOUT. > > > >> maxItems: 1 > > > > > > > > Since ERROR is an output, I would expect it to be an interrupt. The > > > > RDY output, on the other hand, would be wired to a SPI controller with > > > > the SPI_READY feature (I use the Linux flag name here because I'm not > > > > aware of a corresponding devicetree flag). So I don't think the RDY > > > > signal would be an interrupt. > > > > > > > > > > Error does not have the purpose to be an interrupt. The only interrupt > > > used from this chip is the one from the DOUT/~RDY pin. Sure, it is wired > > > to the SPI controller, but when you can't also receive interrupts on > > > that very same CPU pin an issue arises. So that pin is also wired to > > > another GPIO with interrupt support. > > > > You've lost me. It's a pin that has a state change when an error condition > > occurs. Why not an interrupt? Doesn't matter that the driver doesn't > > use this functionality. dt-bindings should be as comprehensive as possible. > > Given it's a multipurpose pin you'd also want to support it as a gpio to be > > complete alongside the other GPIOs. > > > > > > > > This is the same way that ad4130.yaml is written for example (with the > > > exception that ad4130 supports configuring where the interrupt is routed). > > > > > > In regards to SPI_READY _BITUL(7) /* slave pulls low to pause */: the > > > ad_sigma_delta framework (if it can be called that) is written to expect > > > a pin interrupt, not to use SPI_READY controller feature. > > > > SPI_READY is supported by only a couple of controllers. I'm not even that > > sure exactly how it is defined and whether that lines up with this usecase. > > From some old asci art. https://lore.kernel.org/all/1456747459-8559-1-git-send-email-linux@rempel-privat.de/ > > > > Flow control: Ready Sequence > > Master CS |-----1\_______________________| > > Slave FC |--------2\____________________| > > DATA |-----------3\_________________| > > > > So you set master and then wait for a flow control pin (the ready signal) before > > you can actually talk to the device. > > > > Here we are indicating data is ready to be be read out. > > > > So I don't 'think' SPI_READY applies. > > > > Jonathan > > > > I'm not arguing that SPI_READY applies in this particular case, but > FWIW it does seem to me like... > > 1) SPI_READY could be implemented in any SPI driver using a GPIO > interrupt (similar to how we already have GPIO CS) > 2) In cases where the SPI controller does have actual hardware support > for SPI_READY and the ADC chip A) uses CS to trigger a conversion and > B) has a "busy" signal that goes low when the conversion is complete, > then the SPI_READY feature could be used to make reading sample data > more efficient by avoiding any CPU intervention between CS assertion > and starting the data xfer due to waiting for the conversion to > complete either by waiting for an interrupt or sleeping for a fixed > amount of time. Agreed that SPI_READY is possible if an SPI controller uses GPIOs for CS and that signal. If not a GPIO for CS then the SPI_READY should also be hardware managed. I could potentially be adapted to this sort of case if conditions like the CS being active before the ready is set is taking into account. This is a bit like SPI in general - far too many things that could be built and no particular standards for them. Jonathan
diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml new file mode 100644 index 000000000000..25a5404ee353 --- /dev/null +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml @@ -0,0 +1,170 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +# Copyright 2023 Analog Devices Inc. +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/iio/adc/adi,ad7173.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Analog Devices AD7173 ADC + +maintainers: + - Ceclan Dumitru <dumitru.ceclan@analog.com> + +description: | + Bindings for the Analog Devices AD717X ADC's. Datasheets for supported chips: + https://www.analog.com/media/en/technical-documentation/data-sheets/AD7172-2.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/AD7173-8.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/AD7175-2.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/AD7176-2.pdf + +properties: + compatible: + enum: + - adi,ad7172-2 + - adi,ad7173-8 + - adi,ad7175-2 + - adi,ad7176-2 + + reg: + maxItems: 1 + + interrupts: + maxItems: 1 + + '#address-cells': + const: 1 + + '#size-cells': + const: 0 + + spi-max-frequency: + maximum: 20000000 + + refin-supply: + description: external reference supply, can be used as reference for conversion. + + refin2-supply: + description: external reference supply, can be used as reference for conversion. + + avdd-supply: + description: avdd supply, can be used as reference for conversion. + +patternProperties: + "^channel@[0-9a-f]$": + type: object + $ref: adc.yaml + unevaluatedProperties: false + + properties: + reg: + minimum: 0 + maximum: 15 + + diff-channels: + items: + minimum: 0 + maximum: 31 + + adi,reference-select: + description: | + Select the reference source to use when converting on + the specific channel. Valid values are: + refin : REFIN(+)/REFIN(−). + refin2 : REFIN2(+)/REFIN2(−) + refout-avss: REFOUT/AVSS (Internal reference) + avdd : AVDD + + External reference refin2 only available on ad7173-8. + If not specified, internal reference used. + enum: + - refin + - refin2 + - refout-avss + - avdd + default: refout-avss + + required: + - reg + - diff-channels + +required: + - compatible + - reg + - interrupts + +allOf: + - $ref: /schemas/spi/spi-peripheral-props.yaml# + + - if: + properties: + compatible: + not: + contains: + const: adi,ad7173-8 + then: + properties: + refin2-supply: false + patternProperties: + "^channel@[0-9a-f]$": + properties: + adi,reference-select: + enum: + - refin + - refout-avss + - avdd + +unevaluatedProperties: false + +examples: + - | + #include <dt-bindings/gpio/gpio.h> + #include <dt-bindings/interrupt-controller/irq.h> + + spi { + #address-cells = <1>; + #size-cells = <0>; + + adc@0 { + compatible = "adi,ad7173-8"; + reg = <0>; + + #address-cells = <1>; + #size-cells = <0>; + + interrupts = <25 IRQ_TYPE_EDGE_FALLING>; + interrupt-parent = <&gpio>; + spi-max-frequency = <5000000>; + + refin-supply = <&dummy_regulator>; + + channel@0 { + reg = <0>; + bipolar; + diff-channels = <0 1>; + adi,reference-select = "refin"; + }; + + channel@1 { + reg = <1>; + diff-channels = <2 3>; + }; + + channel@2 { + reg = <2>; + bipolar; + diff-channels = <4 5>; + }; + + channel@3 { + reg = <3>; + bipolar; + diff-channels = <6 7>; + }; + + channel@4 { + reg = <4>; + diff-channels = <8 9>; + adi,reference-select = "avdd"; + }; + }; + };
The AD7173 family offer a complete integrated Sigma-Delta ADC solution which can be used in high precision, low noise single channel applications or higher speed multiplexed applications. The Sigma-Delta ADC is intended primarily for measurement of signals close to DC but also delivers outstanding performance with input bandwidths out to ~10kHz. Signed-off-by: Dumitru Ceclan <mitrutzceclan@gmail.com> --- V7->V8 - include missing fix from V6 .../bindings/iio/adc/adi,ad7173.yaml | 170 ++++++++++++++++++ 1 file changed, 170 insertions(+) create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml