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