> -----Original Message----- > From: Jonathan Cameron <jic23@xxxxxxxxxx> > Sent: Sunday, January 16, 2022 1:21 PM > To: Sa, Nuno <Nuno.Sa@xxxxxxxxxx> > Cc: linux-iio@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; Rob > Herring <robh+dt@xxxxxxxxxx>; Lars-Peter Clausen > <lars@xxxxxxxxxx>; Hennerich, Michael > <Michael.Hennerich@xxxxxxxxxx> > Subject: Re: [PATCH v2 2/3] iio: ABI: add ABI file for the LTC2688 DAC > > [External] > > On Sat, 15 Jan 2022 10:27:04 +0100 > Nuno Sá <nuno.sa@xxxxxxxxxx> wrote: > > > Define the sysfs interface for toggle or dither capable channels. > Dither > > capable channels will have the extended interface: > > > > * out_voltageY_dither_en > > * out_voltageY_dither_raw > > * out_voltageY_dither_offset > > * out_voltageY_dither_raw_available > > * out_voltageY_dither_frequency > > * out_voltageY_dither_frequency_available > > * out_voltageY_dither_phase > > * out_voltageY_dither_phase_available > > > > Toggle enabled channels will have: > > > > * out_voltageY_toggle_en > > * out_voltageY_raw0 > > * out_voltageY_raw1 > > * out_voltageY_symbol > > Maybe worth just stating the normal interface here as well because > it's not clear from these examples if > out_voltageY_raw still exists for toggle enabled channels (I'm assuming > not?) Yes, it does not exist as It would not make sense :)... > > > > Signed-off-by: Nuno Sá <nuno.sa@xxxxxxxxxx> > ABI seems good to me, just a few comments on details of the > documentation. > Thanks, > > Jonathan > > --- > > .../ABI/testing/sysfs-bus-iio-dac-ltc2688 | 80 > +++++++++++++++++++ > > MAINTAINERS | 1 + > > 2 files changed, 81 insertions(+) > > create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-dac- > ltc2688 > > > > diff --git a/Documentation/ABI/testing/sysfs-bus-iio-dac-ltc2688 > b/Documentation/ABI/testing/sysfs-bus-iio-dac-ltc2688 > > new file mode 100644 > > index 000000000000..38d1df81c6dc > > --- /dev/null > > +++ b/Documentation/ABI/testing/sysfs-bus-iio-dac-ltc2688 > > @@ -0,0 +1,80 @@ > > +What: > /sys/bus/iio/devices/iio:deviceX/out_voltageY_dither_en > > +KernelVersion: 5.17 > > +Contact: linux-iio@xxxxxxxxxxxxxxx > > +Description: > > + Dither enable. Write 1 to enable dither or 0 to disable it. > > + > > +What: > /sys/bus/iio/devices/iio:deviceX/out_voltageY_dither_raw > > +KernelVersion: 5.17 > > +Contact: linux-iio@xxxxxxxxxxxxxxx > > +Description: > > + This raw, unscaled value refers to the dither signal > amplitude. > > + The same scale as in out_voltageY_raw applies. > However, the > > + offset might be different as it's always 0 for this > attribute. > > We'll have to be careful if we ever generalize these docs but what you > have here > is fine whilst it applies to just this device. > > > + > > +What: > /sys/bus/iio/devices/iio:deviceX/out_voltageY_dither_raw_av > ailable > > +KernelVersion: 5.17 > > +Contact: linux-iio@xxxxxxxxxxxxxxx > > +Description: > > + Available range for dither raw amplitude values. > > + > > +What: > /sys/bus/iio/devices/iio:deviceX/out_voltageY_dither_offset > > +KernelVersion: 5.17 > > +Contact: linux-iio@xxxxxxxxxxxxxxx > > +Description: > > + Offset applied to out_voltageY_dither_raw. Read only > attribute > > + always set to 0. > > + > > +What: > /sys/bus/iio/devices/iio:deviceX/out_voltageY_dither_freque > ncy > > +KernelVersion: 5.17 > > +Contact: linux-iio@xxxxxxxxxxxxxxx > > +Description: > > + Sets the dither signal frequency. > Units. > > > + > > +What: > /sys/bus/iio/devices/iio:deviceX/out_voltageY_dither_freque > ncy_available > > +KernelVersion: 5.17 > > +Contact: linux-iio@xxxxxxxxxxxxxxx > > +Description: > > + Returns the available values for the dither frequency. > > + > > +What: > /sys/bus/iio/devices/iio:deviceX/out_voltageY_dither_phase > > +KernelVersion: 5.17 > > +Contact: linux-iio@xxxxxxxxxxxxxxx > > +Description: > > + Sets the dither signal phase. > > Units. Radians, or refer to the main _phase docs and say it's the same. > > > + > > +What: > /sys/bus/iio/devices/iio:deviceX/out_voltageY_dither_phase_ > available > > +KernelVersion: 5.17 > > +Contact: linux-iio@xxxxxxxxxxxxxxx > > +Description: > > + Returns the available values for the dither phase. > > + > > +What: > /sys/bus/iio/devices/iio:deviceX/out_voltageY_toggle_en > > +KernelVersion: 5.17 > > +Contact: linux-iio@xxxxxxxxxxxxxxx > > +Description: > > + Toggle enable. Write 1 to enable toggle or 0 to disable > it. > > Say why this is useful (presumably toggle with a clock rather than via > _symbol) > > > + > > +What: > /sys/bus/iio/devices/iio:deviceX/out_voltageY_raw0 > > +KernelVersion: 5.17 > > +Contact: linux-iio@xxxxxxxxxxxxxxx > > +Description: > > + It has the same meaning as out_voltageY_raw. This > attribute is > > + specific to toggle enabled channels and refers to the > DAC output > > + code in INPUT_A. The same scale, offset, etc applies. > > Same as what? > > > + > > +What: > /sys/bus/iio/devices/iio:deviceX/out_voltageY_raw1 > > +KernelVersion: 5.17 > > +Contact: linux-iio@xxxxxxxxxxxxxxx > > +Description: > > + Same as out_voltageY_raw0 but referring to the DAC > output code > > + in INPUT_B. > > You could combine this with previous and have two what lines. Might > allow > a slightly more compact clear description. Did not remembered I could do that... > > > + > > +What: > /sys/bus/iio/devices/iio:deviceX/out_voltageY_symbol > > Ah. That answers one of my binding related questions :) You have > kept > software control as an option for toggle. Yeah... It was your suggestion to expose this one only when no pin is given in dts 😉. > > +KernelVersion: 5.17 > > +Contact: linux-iio@xxxxxxxxxxxxxxx > > +Description: > > + Performs a SW toggle. This attribute is specific to toggle > > + enabled channels and allows to toggle between > out_voltageY_raw > > _raw0 > > > + and out_voltageY_raw1 through software. Writing 0 > will select > > + out_voltageY_raw while 1 selects out_voltageY_raw1. > _raw0 Forgot to change it here and above :/ - Nuno Sá