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
