diff mbox series

[v3,2/3] Documentation/ABI/testing: Add documentation for AD5766 new ABI

Message ID 20201208131957.34381-2-cristian.pop@analog.com (mailing list archive)
State New, archived
Headers show
Series [v3,1/3] dt-bindings: iio: dac: AD5766 yaml documentation | expand

Commit Message

Cristian Pop Dec. 8, 2020, 1:19 p.m. UTC
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 <cristian.pop@analog.com>
---
 .../ABI/testing/sysfs-bus-iio-dac-ad5766      | 45 +++++++++++++++++++
 1 file changed, 45 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-dac-ad5766

Comments

Jonathan Cameron Dec. 13, 2020, 2:47 p.m. UTC | #1
On Tue, 8 Dec 2020 15:19:56 +0200
Cristian Pop <cristian.pop@analog.com> 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 <cristian.pop@analog.com>

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:	linux-iio@vger.kernel.org
> +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:	linux-iio@vger.kernel.org
> +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:	linux-iio@vger.kernel.org
> +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:	linux-iio@vger.kernel.org
> +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:	linux-iio@vger.kernel.org
> +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:	linux-iio@vger.kernel.org
> +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.
diff mbox series

Patch

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:	linux-iio@vger.kernel.org
+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:	linux-iio@vger.kernel.org
+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:	linux-iio@vger.kernel.org
+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:	linux-iio@vger.kernel.org
+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:	linux-iio@vger.kernel.org
+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:	linux-iio@vger.kernel.org
+Description:
+		Selects dither source applied to the selected channel.
+		NO_DITHER - No dither applied
+		N0 - N0 dither signal applied
+		N1 - N1 dither signal applied