Hello Jonathan, -- Antoniu Miclăuş > -----Original Message----- > From: Jonathan Cameron <jic23@xxxxxxxxxx> > Sent: Friday, November 12, 2021 7:56 PM > To: Miclaus, Antoniu <Antoniu.Miclaus@xxxxxxxxxx> > Cc: robh+dt@xxxxxxxxxx; linux-iio@xxxxxxxxxxxxxxx; > devicetree@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx > Subject: Re: [PATCH 4/4] iio:filter:admv8818: Add sysfs ABI documentation > > [External] > > On Tue, 9 Nov 2021 14:31:27 +0200 > Antoniu Miclaus <antoniu.miclaus@xxxxxxxxxx> wrote: > > > Add initial ABI documentation for admv8818 filter sysfs interfaces. > > > > Signed-off-by: Antoniu Miclaus <antoniu.miclaus@xxxxxxxxxx> > > --- > > .../ABI/testing/sysfs-bus-iio-filter-admv8818 | 60 +++++++++++++++++++ > > 1 file changed, 60 insertions(+) > > create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-filter- > admv8818 > > > > diff --git a/Documentation/ABI/testing/sysfs-bus-iio-filter-admv8818 > b/Documentation/ABI/testing/sysfs-bus-iio-filter-admv8818 > > new file mode 100644 > > index 000000000000..7fa5b0819055 > > --- /dev/null > > +++ b/Documentation/ABI/testing/sysfs-bus-iio-filter-admv8818 > > @@ -0,0 +1,60 @@ > > +What: > /sys/bus/iio/devices/iio:deviceX/out_altvoltageY_filter_high_pass_3 > db_frequency > > +KernelVersion: > > +Contact: linux-iio@xxxxxxxxxxxxxxx > > +Description: > > + The cut-off frequency of the ADMV8818 high pass filter. The > value is scaled using > > + the `out_altvoltageY_scale` attribute so that GHz frequencies > are valid inputs, > > + The accepted range of values for the frequencies is between > 1.75GHz and 19.9GHz. > > + > > + The default value for the scale is 1000000, therefore MHz > frequency values are > > + passed as input. > > I don't think this ABI really works unfortunately. What we are talking here is > a bunch of > selectable filters and one high pass + one low pass filter max can be enabled > at a time. > > So two options, we either have simply a single > out_altvoltage_filter_low_pass_3db_frequency > out_altvoltage_filter_high_pass_3db_frequency > Probably both with index 0 and index free channels are a silly idea given it's > fine to just have > one with index 0. > > or if there is sufficient reason to setup a selectable set of options then > we could look at indexed filters and a _symbol type selection which may > seem > odd but generalises fairly well from Phase Shift Keying type symbol stuff we > have had before (though still in staging because no one has cleaned the > drivers > up yet). > > > > + > > +What: > /sys/bus/iio/devices/iio:deviceX/out_altvoltageY_filter_low_pass_3 > db_frequency > > +KernelVersion: > > +Contact: linux-iio@xxxxxxxxxxxxxxx > > +Description: > > + The cut-off frequency of the ADMV8818 low pass filter. The > value is scaled using > > + the `out_altvoltageY_scale` attribute so that GHz frequencies > are valid inputs, > > + The accepted range of values for the frequencies is between > 2.05GHz and 18.85GHz. > > + > > + The default value for the scale is 1000000, therefore MHz > frequency values are > > + passed as input. > > + > > +What: > /sys/bus/iio/devices/iio:deviceX/out_altvoltageY_scale > > +KernelVersion: > > +Contact: linux-iio@xxxxxxxxxxxxxxx > > +Description: > > + Scale high pass and lowpass filter frequency values to Hz. > > + > > +What: > /sys/bus/iio/devices/iio:deviceX/out_altvoltageY_mode_available > > +KernelVersion: > > +Contact: linux-iio@xxxxxxxxxxxxxxx > > +Description: > > + Reading this returns the valid values that can be written to > the > > + on_altvoltage0_mode attribute: > > + > > + - auto -> enable/register the clock rate notifier > > Hmm I'm wondering about the usecases of this. > > If this is being used with a clk device, then I think only the notifier option > makes much > sense. If it's not a clk that linux is aware of then manual makes more sense. > > > + - manual -> disable/unregister the clock rate notifier > > + - bypass -> bypass LPF/HPF and disable/unregister the clock > rate notifier > > This should be separate enable for the two filters though I think we've use > the value 0 > to mean this in the past. The bypasses look to be per filter anyway, so a > single > mode is insufficiently flexible. > > In the vast majority of cases, mode attributes are not used because they are > always device > specific and hence generic code has no idea what to do with them. > As I mentioned also in the dt-bindings comments, these attributes were added because they were requested by the users of the application in which this part was involved. If you think these attributes/properties are way too custom, I can drop them. Same goes for the bandwidth attribute. > > + > > +What: > /sys/bus/iio/devices/iio:deviceX/out_altvoltageY_mode > > +KernelVersion: > > +Contact: linux-iio@xxxxxxxxxxxxxxx > > +Description: > > + This attribute configures the filter mode. > > + Reading returns the actual mode. > > + > > +What: > /sys/bus/iio/devices/iio:deviceX/out_altvoltageY_filter_band_pass_ > bandwidth_3db_frequency > > +KernelVersion: > > +Contact: linux-iio@xxxxxxxxxxxxxxx > > +Description: > > + Store the band pass bandwidth frequency value applied. > > + Reading returns the bandwidth frequency scaled. > > The device has no concept of bandpass that I can find so why are we > introducing it? > Let the user set the two filters to achieve this result. Userspace can do the > maths for us :) > > > + > > + > > +What: > /sys/bus/iio/devices/iio:deviceX/out_altvoltageY_filter_band_pass_ > center_frequency > > +KernelVersion: > > +Contact: linux-iio@xxxxxxxxxxxxxxx > > +Description: > > + Store the band pass center frequency value applied. > > + Reading returns the center frequency scaled.
