Re: [PATCH 2/3] dt-bindings: iio: adc: ad7192: Add AD7194 support

[Alisa-Dariana Roman]

On 14.11.2023 19:39, Krzysztof Kozlowski wrote:
> On 14/11/2023 17:02, Alisa-Dariana Roman wrote:
> > On 06.11.2023 10:56, Krzysztof Kozlowski wrote:
> > > On 05/11/2023 20:31, alisadariana@xxxxxxxxx wrote:
> > > > From: Alisa-Dariana Roman <alisa.roman@xxxxxxxxxx>
> > > >
> > > > Unlike the other AD719Xs, AD7194 has configurable differential
> > > > channels. The default configuration for these channels can be changed
> > > > from the devicetree.
> > > >
> > > > Also add an example for AD7194 devicetree.
> > > >
> > > > Signed-off-by: Alisa-Dariana Roman <alisa.roman@xxxxxxxxxx>
> > > > ---
> > > >    .../bindings/iio/adc/adi,ad7192.yaml          | 69 +++++++++++++++++++
> > > >    1 file changed, 69 insertions(+)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7192.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7192.yaml
> > > > index 16def2985ab4..b9a9f7b20670 100644
> > > > --- a/Documentation/devicetree/bindings/iio/adc/adi,ad7192.yaml
> > > > +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7192.yaml
> > > > @@ -21,8 +21,15 @@ properties:
> > > >          - adi,ad7190
> > > >          - adi,ad7192
> > > >          - adi,ad7193
> > > > +      - adi,ad7194
> > > >          - adi,ad7195
> > > >    
> > > > +  '#address-cells':
> > > > +    const: 1
> > > > +
> > > > +  '#size-cells':
> > > > +    const: 0
> > > > +
> > > >      reg:
> > > >        maxItems: 1
> > > >    
> > > > @@ -96,6 +103,31 @@ required:
> > > >      - spi-cpol
> > > >      - spi-cpha
> > > >    
> > > > +patternProperties:
> > > > +  "^channel@([0-9a-f])$":
> > > > +    type: object
> > > > +    $ref: adc.yaml
> > > > +    unevaluatedProperties: false
> > > > +
> > > > +    properties:
> > > > +      reg:
> > > > +        description: The channel index.
> > > > +        minimum: 0
> > > > +        maximum: 7
> > >
> > > Your pattern a bit above is not correct then: [0-7]
> > >
> > > > +
> > > > +      diff-channels:
> > > > +        description: |
> > > > +          The differential channel pair for Ad7194 configurable channels. The
> > > > +          first channel is the positive input, the second channel is the
> > > > +          negative input.
> > > > +        items:
> > > > +          minimum: 1
> > > > +          maximum: 16
> > > > +
> > > > +    required:
> > > > +      - reg
> > > > +      - diff-channels
> > > > +
> > > >    allOf:
> > > >      - $ref: /schemas/spi/spi-peripheral-props.yaml#
> > > >    
> > > > @@ -127,3 +159,40 @@ examples:
> > > >                adi,burnout-currents-enable;
> > > >            };
> > > >        };
> > > > +  - |
> > > > +    spi {
> > > > +        #address-cells = <1>;
> > > > +        #size-cells = <0>;
> > > > +
> > > > +        adc@0 {
> > > > +            #address-cells = <1>;
> > > > +            #size-cells = <0>;
> > > > +            compatible = "adi,ad7194";
> > > > +            reg = <0>;
> > > > +            spi-max-frequency = <1000000>;
> > > > +            spi-cpol;
> > > > +            spi-cpha;
> > > > +            clocks = <&ad7192_mclk>;
> > > > +            clock-names = "mclk";
> > > > +            interrupts = <25 0x2>;
> > > > +            interrupt-parent = <&gpio>;
> > > > +            dvdd-supply = <&dvdd>;
> > > > +            avdd-supply = <&avdd>;
> > > > +            vref-supply = <&vref>;
> > > > +
> > > > +            adi,refin2-pins-enable;
> > > > +            adi,rejection-60-Hz-enable;
> > > > +            adi,buffer-enable;
> > > > +            adi,burnout-currents-enable;
> > > > +
> > > > +            channel@0 {
> > >
> > > Why cannot you add this to the existing example?
> > >
> > >
> > >
> > > Best regards,
> > > Krzysztof
> > I added another example to highlight the fact that only AD7194 supports
> > configurable channels. How should I proceed?
>
> Bindings did not tell that, so it seems you miss that part - allOf
> constraining channels per variant.
>
> Best regards,
> Krzysztof
And should I remove the AD7194 example?

Kind regards,
Alisa-Dariana Roman