Re: [PATCH 1/2] dt-bindings: iio: spi-dac: Add driver for SPI shift register DACs

[Rob Herring]

On Wed, Dec 13, 2023 at 10:09:09AM +0100, Mike Looijmans wrote:
> Add a driver for generic serial shift register DACs like TI DAC714.

This is not a driver.

> 
> Signed-off-by: Mike Looijmans <mike.looijmans@xxxxxxxx>
> 
> ---
> 
>  .../devicetree/bindings/iio/dac/spidac.yaml   | 69 +++++++++++++++++++
>  1 file changed, 69 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/dac/spidac.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/dac/spidac.yaml b/Documentation/devicetree/bindings/iio/dac/spidac.yaml
> new file mode 100644
> index 000000000000..be98da728594
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/dac/spidac.yaml
> @@ -0,0 +1,69 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/dac/spidac.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Generic "shift register" SPI DAC
> +
> +description:
> +  Supports simple SPI "shift register" DACs, like TI's DAC714. These DACs have
> +  no control registers or commands, they just use a clock and serial data to
> +  shift in a raw DAC value. Multiple DACs can be daisy-chained together.
> +
> +maintainers:
> +  - Mike Looijmans <mike.looijmans@xxxxxxxx>
> +
> +properties:
> +  compatible:
> +    enum:
> +      - spi-dac
> +      - ti,dac714

Why does TI chip need a specific compatible and others don't?

Are power supplies on these chips the same?

> +
> +  reg:
> +    maxItems: 1
> +
> +  ldac-gpios:
> +    description:
> +      LDAC pin to be used as a hardware trigger to update the DAC outputs. Not
> +      needed when the DACs use the chip select to update their output.
> +    maxItems: 1
> +
> +  reset-gpios:
> +    description:
> +      Optional reset pin that resets all DACs.
> +    maxItems: 1
> +
> +  num-channels:
> +    description:
> +      Number of channels (usually the number of DAC chips in series)

usually? What other possible option is there? If something else, how is 
the driver going to distinguish that?

default: 1

> +    $ref: /schemas/types.yaml#/definitions/uint32
> +
> +  bits-per-channel:

Perhaps 'channel-bits' as -bits is a standard unit suffix.

> +    description:
> +       Number of bits for each DAC output.
> +    $ref: /schemas/types.yaml#/definitions/uint32

Constraints? I assume all DACs are much less than 2^32 bits. default?

> +
> +required:
> +  - compatible
> +  - reg

Don't you always need to know how many bits?

> +
> +allOf:
> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +    spi {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        dac@1 {
> +            compatible = "spidac";
> +            reg = <0x1>;
> +            ldac-gpios = <&gpio 42 GPIO_ACTIVE_LOW>;
> +        };
> +    };
> +...
> -- 
> 2.34.1
> 
> 
> Met vriendelijke groet / kind regards,
> 
> Mike Looijmans
> System Expert
> 
> 
> TOPIC Embedded Products B.V.
> Materiaalweg 4, 5681 RJ Best
> The Netherlands
> 
> T: +31 (0) 499 33 69 69
> E: mike.looijmans@xxxxxxxx
> W: www.topic.nl
> 
> Please consider the environment before printing this e-mail