| Message ID | 20240809-ad7625_r1-v2-1-f85e7ac83150@baylibre.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | iio: adc: add new ad7625 driver | expand |
On 09/08/2024 20:41, Trevor Gamblin wrote: > Add a binding specification for the Analog Devices Inc. AD7625, > AD7626, AD7960, and AD7961 ADCs. > Thank you for your patch. There is something to discuss/improve. > +allOf: > + - if: > + required: > + - ref-supply > + then: > + # refin-supply is not needed if ref-supply is given Not needed or not allowed? Schema says the latter. > + properties: > + refin-supply: false > + - if: > + required: > + - refin-supply > + then: > + # ref-supply is not needed if refin-supply is given > + properties: > + ref-supply: false > + - if: > + properties: > + compatible: > + contains: > + enum: > + - adi,ad7625 > + - adi,ad7626 > + then: > + properties: > + en2-gpios: false > + en3-gpios: false > + adi,en2-always-on: false > + adi,en3-always-on: false > + > + - if: > + properties: > + compatible: > + contains: > + enum: > + - adi,ad7960 > + - adi,ad7961 > + then: > + # ad796x parts must have one of the two supplies > + oneOf: > + - required: [ref-supply] > + - required: [refin-supply] That's duplicating first and second if. And all three - comment, first if:then: and this one here is kind of contradictory so I don't know what you want to achieve. > + > +additionalProperties: false > + > +examples: > + - | > + adc { > + compatible = "adi,ad7625"; > + vdd1-supply = <&supply_5V>; > + vdd2-supply = <&supply_2_5V>; > + vio-supply = <&supply_2_5V>; > + io-backends = <&axi_adc>; > + clocks = <&ref_clk>; > + pwms = <&axi_pwm_gen 0 0>, <&axi_pwm_gen 1 0>; > + pwm-names = "cnv", "clk_gate"; Make example complete - en0 or en1 GPIOs or whatever else is applicable. Best regards, Krzysztof
On 2024-08-10 8:10 a.m., Krzysztof Kozlowski wrote: > On 09/08/2024 20:41, Trevor Gamblin wrote: >> Add a binding specification for the Analog Devices Inc. AD7625, >> AD7626, AD7960, and AD7961 ADCs. >> > Thank you for your patch. There is something to discuss/improve. > >> +allOf: >> + - if: >> + required: >> + - ref-supply >> + then: >> + # refin-supply is not needed if ref-supply is given > Not needed or not allowed? Schema says the latter. Yes, this is poor wording on my part. I will fix it to say "not allowed". > >> + properties: >> + refin-supply: false >> + - if: >> + required: >> + - refin-supply >> + then: >> + # ref-supply is not needed if refin-supply is given >> + properties: >> + ref-supply: false >> + - if: >> + properties: >> + compatible: >> + contains: >> + enum: >> + - adi,ad7625 >> + - adi,ad7626 >> + then: >> + properties: >> + en2-gpios: false >> + en3-gpios: false >> + adi,en2-always-on: false >> + adi,en3-always-on: false >> + >> + - if: >> + properties: >> + compatible: >> + contains: >> + enum: >> + - adi,ad7960 >> + - adi,ad7961 >> + then: >> + # ad796x parts must have one of the two supplies >> + oneOf: >> + - required: [ref-supply] >> + - required: [refin-supply] > That's duplicating first and second if. And all three - comment, first > if:then: and this one here is kind of contradictory so I don't know what > you want to achieve. It sounds like there's a better way for me to specify this, but I'm not exactly sure how. The AD762x parts can operate without external references, so the intent was that neither REF nor REFIN was required in the bindings, but if one is given then the other can't be. For the AD796x parts, one of REF or REFIN must be provided, but not both. If REFIN is provided, then REF doesn't need an input because a reference voltage is generated on REF. If REF is provided, then REFIN is tied to ground. Maybe there's a simpler way for me to specify the whole block? > > >> + >> +additionalProperties: false >> + >> +examples: >> + - | >> + adc { >> + compatible = "adi,ad7625"; >> + vdd1-supply = <&supply_5V>; >> + vdd2-supply = <&supply_2_5V>; >> + vio-supply = <&supply_2_5V>; >> + io-backends = <&axi_adc>; >> + clocks = <&ref_clk>; >> + pwms = <&axi_pwm_gen 0 0>, <&axi_pwm_gen 1 0>; >> + pwm-names = "cnv", "clk_gate"; > Make example complete - en0 or en1 GPIOs or whatever else is applicable. Will do, thank you. > > > Best regards, > Krzysztof >
On 12/08/2024 15:41, Trevor Gamblin wrote: > > On 2024-08-10 8:10 a.m., Krzysztof Kozlowski wrote: >> On 09/08/2024 20:41, Trevor Gamblin wrote: >>> Add a binding specification for the Analog Devices Inc. AD7625, >>> AD7626, AD7960, and AD7961 ADCs. >>> >> Thank you for your patch. There is something to discuss/improve. >> >>> +allOf: >>> + - if: >>> + required: >>> + - ref-supply >>> + then: >>> + # refin-supply is not needed if ref-supply is given >> Not needed or not allowed? Schema says the latter. > Yes, this is poor wording on my part. I will fix it to say "not allowed". so just drop it. No need to repeat schema - it is obvious from the comment. OTOH, if you want to keep any of such comments, then make it useful - explain WHY it is not allowed. Because the WHY is not visible from the code. >> >>> + properties: >>> + refin-supply: false >>> + - if: >>> + required: >>> + - refin-supply >>> + then: >>> + # ref-supply is not needed if refin-supply is given >>> + properties: >>> + ref-supply: false >>> + - if: >>> + properties: >>> + compatible: >>> + contains: >>> + enum: >>> + - adi,ad7625 >>> + - adi,ad7626 >>> + then: >>> + properties: >>> + en2-gpios: false >>> + en3-gpios: false >>> + adi,en2-always-on: false >>> + adi,en3-always-on: false >>> + >>> + - if: >>> + properties: >>> + compatible: >>> + contains: >>> + enum: >>> + - adi,ad7960 >>> + - adi,ad7961 >>> + then: >>> + # ad796x parts must have one of the two supplies >>> + oneOf: >>> + - required: [ref-supply] >>> + - required: [refin-supply] >> That's duplicating first and second if. And all three - comment, first >> if:then: and this one here is kind of contradictory so I don't know what >> you want to achieve. > > It sounds like there's a better way for me to specify this, but I'm not > exactly sure how. > > The AD762x parts can operate without external references, so the intent > was that neither REF nor REFIN was required in the bindings, but if one > is given then the other can't be. > > For the AD796x parts, one of REF or REFIN must be provided, but not > both. If REFIN is provided, then REF doesn't need an input because a > reference voltage is generated on REF. If REF is provided, then REFIN is > tied to ground. > > Maybe there's a simpler way for me to specify the whole block? Ah, now I see. Looks correct. I am not sure if it could be coded simpler. Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7625.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7625.yaml new file mode 100644 index 000000000000..8192c269dc2f --- /dev/null +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7625.yaml @@ -0,0 +1,175 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/iio/adc/adi,ad7625.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Analog Devices Fast PulSAR Analog to Digital Converters + +maintainers: + - Michael Hennerich <Michael.Hennerich@analog.com> + - Nuno Sá <nuno.sa@analog.com> + +description: | + A family of single channel differential analog to digital converters. + + * https://www.analog.com/en/products/ad7625.html + * https://www.analog.com/en/products/ad7626.html + * https://www.analog.com/en/products/ad7960.html + * https://www.analog.com/en/products/ad7961.html + +properties: + compatible: + enum: + - adi,ad7625 + - adi,ad7626 + - adi,ad7960 + - adi,ad7961 + + vdd1-supply: true + vdd2-supply: true + vio-supply: true + + ref-supply: + description: + Voltage regulator for the external reference voltage (REF). + + refin-supply: + description: + Voltage regulator for the reference buffer input (REFIN). + + clocks: + description: + The clock connected to the CLK pins, gated by the clk_gate PWM. + maxItems: 1 + + pwms: + items: + - description: PWM connected to the CNV input on the ADC. + - description: PWM that gates the clock connected to the ADC's CLK input. + + pwm-names: + items: + - const: cnv + - const: clk_gate + + io-backends: + description: + The AXI ADC IP block connected to the D+/- and DCO+/- lines of the + ADC. An example backend can be found at + http://analogdevicesinc.github.io/hdl/projects/pulsar_lvds/index.html. + maxItems: 1 + + adi,no-dco: + $ref: /schemas/types.yaml#/definitions/flag + description: + Indicates the wiring of the DCO+/- lines. If true, then they are + grounded and the device is in self-clocked mode. If this is not + present, then the device is in echoed clock mode. + + adi,en0-always-on: + $ref: /schemas/types.yaml#/definitions/flag + description: + Indicates if EN0 is hard-wired to the high state. If neither this + nor en0-gpios are present, then EN0 is hard-wired low. + + adi,en1-always-on: + $ref: /schemas/types.yaml#/definitions/flag + description: + Indicates if EN1 is hard-wired to the high state. If neither this + nor en1-gpios are present, then EN1 is hard-wired low. + + adi,en2-always-on: + $ref: /schemas/types.yaml#/definitions/flag + description: + Indicates if EN2 is hard-wired to the high state. If neither this + nor en2-gpios are present, then EN2 is hard-wired low. + + adi,en3-always-on: + $ref: /schemas/types.yaml#/definitions/flag + description: + Indicates if EN3 is hard-wired to the high state. If neither this + nor en3-gpios are present, then EN3 is hard-wired low. + + en0-gpios: + description: + Configurable EN0 pin. + + en1-gpios: + description: + Configurable EN1 pin. + + en2-gpios: + description: + Configurable EN2 pin. + + en3-gpios: + description: + Configurable EN3 pin. + +required: + - compatible + - vdd1-supply + - vdd2-supply + - vio-supply + - clocks + - pwms + - pwm-names + - io-backends + +allOf: + - if: + required: + - ref-supply + then: + # refin-supply is not needed if ref-supply is given + properties: + refin-supply: false + - if: + required: + - refin-supply + then: + # ref-supply is not needed if refin-supply is given + properties: + ref-supply: false + - if: + properties: + compatible: + contains: + enum: + - adi,ad7625 + - adi,ad7626 + then: + properties: + en2-gpios: false + en3-gpios: false + adi,en2-always-on: false + adi,en3-always-on: false + + - if: + properties: + compatible: + contains: + enum: + - adi,ad7960 + - adi,ad7961 + then: + # ad796x parts must have one of the two supplies + oneOf: + - required: [ref-supply] + - required: [refin-supply] + +additionalProperties: false + +examples: + - | + adc { + compatible = "adi,ad7625"; + vdd1-supply = <&supply_5V>; + vdd2-supply = <&supply_2_5V>; + vio-supply = <&supply_2_5V>; + io-backends = <&axi_adc>; + clocks = <&ref_clk>; + pwms = <&axi_pwm_gen 0 0>, <&axi_pwm_gen 1 0>; + pwm-names = "cnv", "clk_gate"; + }; diff --git a/MAINTAINERS b/MAINTAINERS index 42decde38320..2361f92751dd 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1260,6 +1260,15 @@ F: Documentation/devicetree/bindings/iio/addac/adi,ad74413r.yaml F: drivers/iio/addac/ad74413r.c F: include/dt-bindings/iio/addac/adi,ad74413r.h +ANALOG DEVICES INC AD7625 DRIVER +M: Michael Hennerich <Michael.Hennerich@analog.com> +M: Nuno Sá <nuno.sa@analog.com> +R: Trevor Gamblin <tgamblin@baylibre.com> +S: Supported +W: https://ez.analog.com/linux-software-drivers +W: http://analogdevicesinc.github.io/hdl/projects/pulsar_lvds/index.html +F: Documentation/devicetree/bindings/iio/adc/adi,ad7625.yaml + ANALOG DEVICES INC AD7768-1 DRIVER M: Michael Hennerich <Michael.Hennerich@analog.com> L: linux-iio@vger.kernel.org
Add a binding specification for the Analog Devices Inc. AD7625, AD7626, AD7960, and AD7961 ADCs. Signed-off-by: Trevor Gamblin <tgamblin@baylibre.com> --- .../devicetree/bindings/iio/adc/adi,ad7625.yaml | 175 +++++++++++++++++++++ MAINTAINERS | 9 ++ 2 files changed, 184 insertions(+)