On Tue, Dec 12, 2023 at 11:45 AM Dumitru Ceclan <mitrutzceclan@xxxxxxxxx> 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@xxxxxxxxx> > --- > 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@xxxxxxxxxx> > + > +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 > >
