Re: [PATCH v3 1/1] dt-bindings: iio: dds: Add AD9832/AD9835 binding

[Jonathan Cameron <jic23@kernel.org>]

On Sat,  9 May 2026 01:37:45 +0000
Hungyu Lin <dennylin0707@gmail.com> wrote:

> Add devicetree binding documentation for AD9832 and AD9835 DDS devices.
> 
> These devices are SPI-controlled direct digital synthesizers
> requiring AVDD, DVDD supplies and an external master clock.
> 
> Require spi-cpol since AD9832/AD9835 use SPI mode 2.
> Update the example to include spi-cpol and use a generic node name.
> 
> Signed-off-by: Hungyu Lin <dennylin0707@gmail.com>
As per v1 discussion that is ongoing - we haven't in the past put
dt-bindings in place until we are ready to move the driver in question
out of staging.  IIRC these are a fair way from reaching that stage.

Anyhow with that in mind a few quick comments inline.

Thanks,

Jonathan

> ---
>  .../bindings/iio/dds/adi,ad9832.yaml          | 91 +++++++++++++++++++
>  1 file changed, 91 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/dds/adi,ad9832.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/dds/adi,ad9832.yaml b/Documentation/devicetree/bindings/iio/dds/adi,ad9832.yaml
> new file mode 100644
> index 000000000000..9f3e41cb0a0d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/dds/adi,ad9832.yaml
> @@ -0,0 +1,91 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/dds/adi,ad9832.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Analog Devices AD9832 / AD9835 DDS
> +
> +maintainers:
> +  - Hungyu Lin <dennylin0707@gmail.com>
> +
> +description: |
> +  Binding for Analog Devices AD9832 and AD9835 SPI-controlled direct
> +  digital synthesizers (DDS). The devices use an external master clock
> +  and are controlled over a serial SPI-compatible interface.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - adi,ad9832
> +      - adi,ad9835
> +
> +  reg:
> +    maxItems: 1
> +
> +  spi-max-frequency:
> +    maximum: 20000000
> +
> +  spi-cpol: true
> +
> +  clocks:
> +    maxItems: 1
> +    description:
> +      External master clock input.
> +
> +  clock-names:
> +    const: mclk

Do we need to name it if there is only one?

> +
> +  avdd-supply:
> +    description:
> +      Analog power supply.
> +
> +  dvdd-supply:
> +    description:
> +      Digital power supply.
Personally I'd not bother with the descriptions as
avdd is always analog supply and where you have one of those
dvdd is the digital one.  Absolutely fine to have

   avdd-supply: true
   dvdd-supply: true

And no more info about htem.

> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +  - avdd-supply
> +  - dvdd-supply
> +  - spi-cpol
> +
> +allOf:
> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    mclk: clock {
> +      compatible = "fixed-clock";
> +      #clock-cells = <0>;
> +      clock-frequency = <25000000>;
> +    };
> +
> +    vdd_3v3: regulator {
> +      compatible = "regulator-fixed";
> +      regulator-name = "vdd_3v3";
> +      regulator-min-microvolt = <3300000>;
> +      regulator-max-microvolt = <3300000>;
> +      regulator-always-on;
> +    };

See other binding examples. Normally we just refer to clocks
and regulators etc that are magically coming from somewhere outside
of the example snippet. So shouldn't need any of the stuff above here.

> +
> +    spi {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +
> +      dds@0 {
> +        compatible = "adi,ad9832";
> +        reg = <0>;
> +        spi-max-frequency = <20000000>;
> +        spi-cpol;
> +        clocks = <&mclk>;
> +        clock-names = "mclk";
> +        avdd-supply = <&vdd_3v3>;
> +        dvdd-supply = <&vdd_3v3>;
> +      };
> +    };