Re: [PATCH RFC 1/3] dt-bindings: iio: adc: add AD762x/AD796x ADCs

[Trevor Gamblin <tgamblin@baylibre.com>]

Hello,

On 2024-08-03 10:35 a.m., Jonathan Cameron wrote:
> On Wed, 31 Jul 2024 09:48:03 -0400
> Trevor Gamblin <tgamblin@baylibre.com> wrote:
>
>> This adds a binding specification for the Analog Devices Inc. AD7625,
>> AD7626, AD7960, and AD7961 ADCs.
> Given the RFC question is effectively about the binding and may influence
> it a lot - make sure it's talked about here!
>
>> Signed-off-by: Trevor Gamblin <tgamblin@baylibre.com>
>> ---
>>   .../devicetree/bindings/iio/adc/adi,ad7625.yaml    | 176 +++++++++++++++++++++
>>   MAINTAINERS                                        |   9 ++
>>   2 files changed, 185 insertions(+)
>>
>> 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..e88db0ac2534
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7625.yaml
>> @@ -0,0 +1,176 @@
>> +# 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
>> +  in a LFCSP package. Note that these bindings are for the device when
>> +  used with the PulSAR LVDS project:
>> +  http://analogdevicesinc.github.io/hdl/projects/pulsar_lvds/index.html.
> As per the discussion in the cover letter I think the need to represent
> if the DCO+ is connected between ADC and LVDS converter strongly suggests
> we shouldn't represent it as one aggregate device.

Just to be sure, do you mean that the PulSAR LVDS functionality should 
be split into its own driver and then utilized by ad7625?

Thank you for the feedback. I'll work on updates for all of your replies.

- Trevor

>
>> +
>> +  * 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:
>> +    description: A supply that powers the analog and digital circuitry.
> Doesn't really tell us anything. I'd just go with
>      vdd1-supply: true
>      vdd2-supply: true
>      vio-supply: true
>
>
>> +
>> +  vdd2-supply:
>> +    description: A supply that powers the analog and digital circuitry.
>> +
>> +  vio-supply:
>> +    description: A supply for the inputs and outputs.
>> +
>> +  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:
>> +    maxItems: 2
>> +
>> +  pwm-names:
>> +    maxItems: 2
>> +    items:
>> +      - const: cnv
>> +        description: PWM connected to the CNV input on the ADC.
>> +      - const: clk_gate
>> +        description: PWM that gates the clock connected to the ADC's CLK input.
>> +
>> +  io-backends:
>> +    description:
>> +      The AXI ADC IP block connected to the D+/- and DCO+/- lines of the ADC.
> So you have a backend. Great - we have something to indicate a connection
> to or not for the DCO+/o lines.  It's a bit ugly to just repesent it as a clk
> but that would I think work.
>
>> +    maxItems: 1
>> +
>> +  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.
> It's unfortunate there isn't a special 'fixed' gpio-chip option where we could
> just query it is fixed and what the state of the pin is.  This is getting
> quite common so would be good to have a better solution.
>
> Linus, Bartosz - is there a better way to do this?
>
>> +
>> +  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
>> +
>> +- 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
>> +    allOf:
>> +      # ref-supply and refin-supply are mutually-exclusive (neither is also
>> +      # valid)
>> +      - if:
>> +          required:
>> +            - ref-supply
>> +        then:
>> +          properties:
>> +            refin-supply: false
>> +      - if:
>> +          required:
>> +            - refin-supply
>> +        then:
>> +          properties:
>> +            ref-supply: false
>> +
>> +- if:
>> +  properties:
>> +    compatible:
>> +      contains:
>> +        enum:
>> +	  - adi,ad7960
>> +	  - adi,ad7961
>> +  then:
>> +    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>;
>> +        clock = <&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
>>