| Message ID | 20231212104451.22522-1-mitrutzceclan@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v8,1/2] dt-bindings: adc: add AD7173 | expand |
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