On Fri, 14 Feb 2020 11:32:23 +0200 Alexandru Tachici <alexandru.tachici@xxxxxxxxxx> wrote: > This patch adds entries in the syfs docs of ADXL372. > > Signed-off-by: Stefan Popa <stefan.popa@xxxxxxxxxx> > Signed-off-by: Alexandru Tachici <alexandru.tachici@xxxxxxxxxx> > --- > .../ABI/testing/sysfs-bus-iio-accel-adxl372 | 40 +++++++++++++++++++ > 1 file changed, 40 insertions(+) > create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-accel-adxl372 > > diff --git a/Documentation/ABI/testing/sysfs-bus-iio-accel-adxl372 b/Documentation/ABI/testing/sysfs-bus-iio-accel-adxl372 > new file mode 100644 > index 000000000000..1d74fc2ea0ac > --- /dev/null > +++ b/Documentation/ABI/testing/sysfs-bus-iio-accel-adxl372 > @@ -0,0 +1,40 @@ > +What: /sys/bus/iio/devices/iio:deviceX/peak_fifo_mode_enable > +KernelVersion: > +Contact: linux-iio@xxxxxxxxxxxxxxx > +Description: > + This attribute allows to configure the FIFO to store sample > + sets of impact event peak (x, y, z). As a precondition, all > + three channels (x, y, z) need to be enabled. > + Writing 1, peak fifo mode will be enabled, if cleared and > + all three channels are enabled, sample sets of concurrent > + 3-axis data will be stored in the FIFO. > + Hmm. I wonder if we can make this more 'standard'. I can't remember which device it was, but we once had a similar one for which we discussed whether this could be represented as a separate trigger. The basis for that would be that we only capture data when above the threshold which is effectively the same as only triggering capture when above the threshold. However, I'm not sure it really helps us compared to what you have here. Conceptually we could add the ability to do similar filtering to this in software and in that case it would definitely wouldn't make sense to have it as a trigger. So after arguing with myself I think the rough approach you have here is the best we can do. However, naming wise... It's not actually a fifo attribute, it's an attribute on 'input' to the fifo (I think). It's not specific to a a particular buffer (would also apply to any buffer_cb in use) so you are correct to have it as a general parameter. We can't use the term filter, as that's assumed to refer to the actual data and it would be confusing. So we could call it something like buffer_peak_mode_enable > +What: /sys/bus/iio/devices/iio:deviceX/time_activity > +KernelVersion: > +Contact: linux-iio@xxxxxxxxxxxxxxx > +Description: > + This attribute allows to set the activity timer in ms, > + the minimum time measured acceleration needs to overcome > + the set threshold in order to detect activity. I've been thinking about how this aligns with the existing ABI. When we have something that needs to be true for a while before an event happens we use the term period and it's in seconds. If it were simply an event this would be in_accel_thresh_x_rising_period The only difference here is that the event is not just signalled but also controls the state of the device. So... How do we indicate this? I think we should treat these as events in general and use the standard interface, but have some additional ABI to indicate that they are connected to the mode the device is running in... Perhaps have events/in_accel_thresh_x_rising_period events/in_accel_thresh_x_rising_value events/in_accel_thresh_x_falling_period events/in_accel_thresh_x_falling_value activity_detect_event (which reads in_accel_thresh_x_rising always in this case) inactivity_detect_event (which reads in_accel_thresh_x_falling always in this case). > + > +What: /sys/bus/iio/devices/iio:deviceX/time_inactivity > +KernelVersion: > +Contact: linux-iio@xxxxxxxxxxxxxxx > +Description: > + This attribute allows to set the inactivity timer in ms, > + the minimum time measured acceleration needs to be lower > + than set threshold in order to detect inactivity. > + > +What: /sys/bus/iio/devices/iio:deviceX/in_accel_x_threshold_activity > +KernelVersion: > +Contact: linux-iio@xxxxxxxxxxxxxxx > +Description: > + This attribute allows to set the activity threshold in 100 mg > + (0.1 m/s^2 SI). Just mention the SI units. That conversion is rather inaccurate anyway. + should match with base units which aren't 0.1 for acceleration. > + > +What: /sys/bus/iio/devices/iio:deviceX/in_accel_x_threshold_inactivity > +KernelVersion: > +Contact: linux-iio@xxxxxxxxxxxxxxx > +Description: > + This attribute allows to set the inactivity threshold in 100 mg > + (0.1 m/s^2 SI).