2020-12-08 13:19:57

by Cristian Pop

[permalink] [raw]
Subject: [PATCH v3 1/3] dt-bindings: iio: dac: AD5766 yaml documentation

This adds device tree bindings for the AD5766 DAC.

Signed-off-by: Cristian Pop <[email protected]>
---
Changes in v3:
- Fix errors
.../bindings/iio/dac/adi,ad5766.yaml | 58 +++++++++++++++++++
1 file changed, 58 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/dac/adi,ad5766.yaml

diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ad5766.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ad5766.yaml
new file mode 100644
index 000000000000..25ec5bc0a6c6
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/dac/adi,ad5766.yaml
@@ -0,0 +1,58 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright 2020 Analog Devices Inc.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/dac/adi,ad5766.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices AD5766 DAC device driver
+
+maintainers:
+ - Cristian Pop <[email protected]>
+
+description: |
+ Bindings for the Analog Devices AD5766 current DAC device. Datasheet can be
+ found here:
+ https://www.analog.com/media/en/technical-documentation/data-sheets/ad5766-5767.pdf
+
+properties:
+ compatible:
+ enum:
+ - adi,ad5766
+ - adi,ad5767
+
+ reg:
+ maxItems: 1
+
+ spi-max-frequency:
+ maximum: 1000000
+
+ spi-cpol: true
+
+ reset-gpios:
+ description: GPIO spec for the RESET pin. If specified, it will be
+ asserted during driver probe.
+ maxItems: 1
+
+required:
+ - compatible
+ - reg
+ - spi-max-frequency
+ - spi-cpol
+
+additionalProperties: false
+
+examples:
+ - |
+ spi {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ ad5766@0 {
+ compatible = "adi,ad5766";
+ reg = <0>;
+ spi-cpol;
+ spi-max-frequency = <1000000>;
+ reset-gpios = <&gpio 22 0>;
+ };
+ };
--
2.17.1


2020-12-08 13:21:38

by Cristian Pop

[permalink] [raw]
Subject: [PATCH v3 2/3] Documentation/ABI/testing: Add documentation for AD5766 new ABI

New interface is proposed for dither functionality. This future allows
composing an external signals to the selected output channel.
The dither signal can be turned on/off, scaled, inverted, or it can be
selected from different sources.

Signed-off-by: Cristian Pop <[email protected]>
---
.../ABI/testing/sysfs-bus-iio-dac-ad5766 | 45 +++++++++++++++++++
1 file changed, 45 insertions(+)
create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-dac-ad5766

diff --git a/Documentation/ABI/testing/sysfs-bus-iio-dac-ad5766 b/Documentation/ABI/testing/sysfs-bus-iio-dac-ad5766
new file mode 100644
index 000000000000..361bbd0862df
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-iio-dac-ad5766
@@ -0,0 +1,45 @@
+What: /sys/bus/iio/devices/iio:deviceX/in_voltageY_dither_pwr
+KernelVersion:
+Contact: [email protected]
+Description:
+ Dither power on/off. Write 0 to power on dither or 1 to power it off.
+
+What: /sys/bus/iio/devices/iio:deviceX/in_voltageY_dither_invert
+KernelVersion:
+Contact: [email protected]
+Description:
+ Inverts the dither applied to the selected DAC channel. Dither signal is
+ not inverted (default) Dither signal is inverted.
+
+What: /sys/bus/iio/devices/iio:deviceX/in_voltageY_dither_scale_available
+KernelVersion:
+Contact: [email protected]
+Description:
+ Returns possible scalings available for the current channel:
+ "NO_SCALING 75%_SCALING 50%_SCALING 25%_SCALING"
+
+What: /sys/bus/iio/devices/iio:deviceX/in_voltageY_dither_scale
+KernelVersion:
+Contact: [email protected]
+Description:
+ Scales the dither before it is applied to the selected channel.
+ NO_SCALING - No scaling
+ 75%_SCALING - 75% scaling
+ 50%_SCALING - 50% scaling
+ 25%_SCALING - 25% scaling
+
+What: /sys/bus/iio/devices/iio:deviceX/in_voltageY_dither_source_available
+KernelVersion:
+Contact: [email protected]
+Description:
+ Returns possible dither sources available for the selected channel:
+ "NO_DITHER N0 N1"
+
+What: /sys/bus/iio/devices/iio:deviceX/in_voltageY_dither_source
+KernelVersion:
+Contact: [email protected]
+Description:
+ Selects dither source applied to the selected channel.
+ NO_DITHER - No dither applied
+ N0 - N0 dither signal applied
+ N1 - N1 dither signal applied
--
2.17.1

2020-12-11 04:27:40

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v3 1/3] dt-bindings: iio: dac: AD5766 yaml documentation

On Tue, 08 Dec 2020 15:19:55 +0200, Cristian Pop wrote:
> This adds device tree bindings for the AD5766 DAC.
>
> Signed-off-by: Cristian Pop <[email protected]>
> ---
> Changes in v3:
> - Fix errors
> .../bindings/iio/dac/adi,ad5766.yaml | 58 +++++++++++++++++++
> 1 file changed, 58 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/dac/adi,ad5766.yaml
>

Reviewed-by: Rob Herring <[email protected]>

2020-12-13 18:51:20

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v3 2/3] Documentation/ABI/testing: Add documentation for AD5766 new ABI

On Tue, 8 Dec 2020 15:19:56 +0200
Cristian Pop <[email protected]> wrote:

> New interface is proposed for dither functionality. This future allows
> composing an external signals to the selected output channel.
> The dither signal can be turned on/off, scaled, inverted, or it can be
> selected from different sources.
>
> Signed-off-by: Cristian Pop <[email protected]>

Great - much easier to discuss with docs.

Some comments inline. Note we need to always construct ABI with the intent
that it be easy to generalize to other devices. Sometimes that leads to
slightly different decisions than we might make for a single driver.

> ---
> .../ABI/testing/sysfs-bus-iio-dac-ad5766 | 45 +++++++++++++++++++
> 1 file changed, 45 insertions(+)
> create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-dac-ad5766
>
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-dac-ad5766 b/Documentation/ABI/testing/sysfs-bus-iio-dac-ad5766
> new file mode 100644
> index 000000000000..361bbd0862df
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-bus-iio-dac-ad5766
> @@ -0,0 +1,45 @@
> +What: /sys/bus/iio/devices/iio:deviceX/in_voltageY_dither_pwr
> +KernelVersion:
> +Contact: [email protected]
> +Description:
> + Dither power on/off. Write 0 to power on dither or 1 to power it off.

_enable is what we tend to have for similar cases of turning things on or off.

> +
> +What: /sys/bus/iio/devices/iio:deviceX/in_voltageY_dither_invert
> +KernelVersion:
> +Contact: [email protected]
> +Description:
> + Inverts the dither applied to the selected DAC channel. Dither signal is
> + not inverted (default) Dither signal is inverted.

This one is good. Clear docs and good naming.

> +
> +What: /sys/bus/iio/devices/iio:deviceX/in_voltageY_dither_scale_available
> +KernelVersion:
> +Contact: [email protected]
> +Description:
> + Returns possible scalings available for the current channel:
> + "NO_SCALING 75%_SCALING 50%_SCALING 25%_SCALING"

Needs to be numbers. 75 50 25 + I'm going to guess that no scaling == 100%?
If so then use 100 to represent that.


> +
> +What: /sys/bus/iio/devices/iio:deviceX/in_voltageY_dither_scale
> +KernelVersion:
> +Contact: [email protected]
> +Description:
> + Scales the dither before it is applied to the selected channel.
> + NO_SCALING - No scaling
> + 75%_SCALING - 75% scaling
> + 50%_SCALING - 50% scaling
> + 25%_SCALING - 25% scaling

As above and in previous review - needs to be just a number, not a string.

> +
> +What: /sys/bus/iio/devices/iio:deviceX/in_voltageY_dither_source_available
> +KernelVersion:
> +Contact: [email protected]
> +Description:
> + Returns possible dither sources available for the selected channel:
> + "NO_DITHER N0 N1"
> +
> +What: /sys/bus/iio/devices/iio:deviceX/in_voltageY_dither_source
> +KernelVersion:
> +Contact: [email protected]
> +Description:
> + Selects dither source applied to the selected channel.
> + NO_DITHER - No dither applied
> + N0 - N0 dither signal applied
> + N1 - N1 dither signal applied
So you already have an enable. If the enable isn't set I'd assume
that is same as no dither applied? If so just don't support NO_DITHER here
it doesn't give us anything extra.

I wondered about whether this should just be 0 or 1, but as these are
effectively arbitrary labels (in general, with assumption this
interface may get used for other devices!) N0, N1 is fine for this.




2020-12-13 18:51:24

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v3 1/3] dt-bindings: iio: dac: AD5766 yaml documentation

On Tue, 8 Dec 2020 15:19:55 +0200
Cristian Pop <[email protected]> wrote:

> This adds device tree bindings for the AD5766 DAC.
>
> Signed-off-by: Cristian Pop <[email protected]>
> ---
> Changes in v3:
> - Fix errors
> .../bindings/iio/dac/adi,ad5766.yaml | 58 +++++++++++++++++++
> 1 file changed, 58 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/dac/adi,ad5766.yaml
>
> diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ad5766.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ad5766.yaml
> new file mode 100644
> index 000000000000..25ec5bc0a6c6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/dac/adi,ad5766.yaml
> @@ -0,0 +1,58 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +# Copyright 2020 Analog Devices Inc.
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/dac/adi,ad5766.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Analog Devices AD5766 DAC device driver
> +
> +maintainers:
> + - Cristian Pop <[email protected]>
> +
> +description: |
> + Bindings for the Analog Devices AD5766 current DAC device. Datasheet can be
> + found here:
> + https://www.analog.com/media/en/technical-documentation/data-sheets/ad5766-5767.pdf
> +
> +properties:
> + compatible:
> + enum:
> + - adi,ad5766
> + - adi,ad5767
> +
> + reg:
> + maxItems: 1
> +
> + spi-max-frequency:
> + maximum: 1000000
> +
> + spi-cpol: true
> +
> + reset-gpios:
> + description: GPIO spec for the RESET pin. If specified, it will be
> + asserted during driver probe.
> + maxItems: 1

Given we've recently been bitten by this in another driver I took a look at the datasheet.
This is an active low pin (not reset) so document it as such and set logic
as appropriate in the driver.

> +
> +required:
> + - compatible
> + - reg
> + - spi-max-frequency
> + - spi-cpol
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + spi {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + ad5766@0 {
> + compatible = "adi,ad5766";
> + reg = <0>;
> + spi-cpol;
> + spi-max-frequency = <1000000>;
> + reset-gpios = <&gpio 22 0>;
> + };
> + };