[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20201229164728.38a7d892@archlinux>
Date: Tue, 29 Dec 2020 16:47:28 +0000
From: Jonathan Cameron <jic23@...nel.org>
To: Cristian Pop <cristian.pop@...log.com>
Cc: <linux-iio@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
<devicetree@...r.kernel.org>
Subject: Re: [PATCH v4 2/3] Documentation/ABI/testing: Add documentation for
AD5766 new ABI
On Tue, 29 Dec 2020 16:27:58 +0000
Jonathan Cameron <jic23@...nel.org> wrote:
> On Fri, 18 Dec 2020 19:12:30 +0200
> Cristian Pop <cristian.pop@...log.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@...log.com>
> Coming together nicely though a few more thoughts came to mind when reading
> this. Only significant one is whether we are better with decimal scaling
> than percentage scaling for consistency with channel scales etc?
>
> Jonathan
>
> > ---
> > Changelog v4:
> > - Change to "in_voltageY_dither_enable"
> > - Change scale to numbers + 100% for no scaling
> > - Possible dither source values: 0 - N0, 1 - N1
> > .../ABI/testing/sysfs-bus-iio-dac-ad5766 | 36 +++++++++++++++++++
> > 1 file changed, 36 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..72100edb88bb
> > --- /dev/null
> > +++ b/Documentation/ABI/testing/sysfs-bus-iio-dac-ad5766
> > @@ -0,0 +1,36 @@
> > +What: /sys/bus/iio/devices/iio:deviceX/in_voltageY_dither_pwr
>
> in_voltageY_dither_en would be more consistent with existing ABI naming.
Seems to be different in the code (dither_enable I think) so please sanity
check this file carefully against the driver.
Thanks,
Jonathan
>
> > +KernelVersion:
> > +Contact: linux-iio@...r.kernel.org
> > +Description:
> > + Dither enable. Write 1 to enable dither or 0 to disable it.
> > +
> > +What: /sys/bus/iio/devices/iio:deviceX/in_voltageY_dither_invert
> > +KernelVersion:
> > +Contact: linux-iio@...r.kernel.org
> > +Description:
> > + Inverts the dither applied to the selected DAC channel. Dither is not
> > + inverted by default. Write "1" to invert dither.
> > +
> > +What: /sys/bus/iio/devices/iio:deviceX/in_voltageY_dither_scale_available
> > +KernelVersion:
> > +Contact: linux-iio@...r.kernel.org
> > +Description:
> > + Returns possible scalings available for the current channel:
>
> Given this one is self defining and may well get generalized to other devices I'd
> got with a description that doesn't provide values.
>
> Returns possible scalings as percentages.
>
> However, thinking more on this it's a bit inconsistent with other scale values we have
> which are given in decimal. Would "1 0.75 0.5 0.25" work as well here?
>
> > + "100 75 50 25" scaling.
> > +
> > +What: /sys/bus/iio/devices/iio:deviceX/in_voltageY_dither_scale
> > +KernelVersion:
> > +Contact: linux-iio@...r.kernel.org
> > +Description:
> > + Scales the dither before it is applied to the selected channel:
> > + 100 - No scaling
> > + 75 - 75% scaling
> > + 50 - 50% scaling
> > + 25 - 25% scaling
>
> As above, I'm don't really see an advantage in providing the values. Any userspace
> ought to cope with any (finding out what is there via _available).
> "Scales the dither before it is applied to the selected channel. 1 corresponds
> to no scaling".
>
> > +
> > +What: /sys/bus/iio/devices/iio:deviceX/in_voltageY_dither_source
> > +KernelVersion:
>
> Please fill in the expected kernel version. I'll fix it if this happens to merge near
> the change over in a cycle, but it's easier to fix something that is there
> than to add it from scratch :)
>
> > +Contact: linux-iio@...r.kernel.org
> > +Description:
> > + Selects dither source applied to the selected channel. Write "0" to
> > + select N0 source, write "1" to select N1 source.
>
Powered by blists - more mailing lists