2020-02-14 09:31:56

by Alexandru Tachici

[permalink] [raw]
Subject: [PATCH 0/5] iio: accel: adxl372: add peak mode

This series adds the posibility to configure
the device, from sysfs, to work in peak mode. This enables
adxl372 to capture only over threshold accelerations.

Alexandru Tachici (4):
iio: accel: adxl372: Set iio_chan BE
iio: accel: adxl372: add sysfs for time registers
iio: accel: adxl372: Add sysfs for g thresholds
iio: accel: adxl372: Update sysfs docs

Stefan Popa (1):
iio: accel: adxl372: Add support for FIFO peak mode

.../ABI/testing/sysfs-bus-iio-accel-adxl372 | 40 ++++
drivers/iio/accel/adxl372.c | 195 ++++++++++++++++++
2 files changed, 235 insertions(+)
create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-accel-adxl372

--
2.20.1


2020-02-14 09:32:44

by Alexandru Tachici

[permalink] [raw]
Subject: [PATCH 4/5] iio: accel: adxl372: Add sysfs for g thresholds

Adxl372 has configurable thresholds for all 3 axis
that define activity and inactivity.
The driver sets the default inactivity threshold to 100mg
and the activity threshold to 1g. These values are not
ideal for all applications.

This patch adds device attributes for activity and inactivity
thresholds for each axis.

Signed-off-by: Alexandru Tachici <[email protected]>
---
drivers/iio/accel/adxl372.c | 91 +++++++++++++++++++++++++++++++++++++
1 file changed, 91 insertions(+)

diff --git a/drivers/iio/accel/adxl372.c b/drivers/iio/accel/adxl372.c
index 8bef6f2030ff..ab154699e23a 100644
--- a/drivers/iio/accel/adxl372.c
+++ b/drivers/iio/accel/adxl372.c
@@ -5,6 +5,7 @@
* Copyright 2018 Analog Devices Inc.
*/

+#include <linux/bitfield.h>
#include <linux/bitops.h>
#include <linux/interrupt.h>
#include <linux/irq.h>
@@ -131,6 +132,14 @@
#define ADXL372_INT1_MAP_LOW_MSK BIT(7)
#define ADXL372_INT1_MAP_LOW_MODE(x) (((x) & 0x1) << 7)

+/* ADX372_THRESH */
+#define ADXL372_THRESH_VAL_H_MSK GENMASK(10, 3)
+#define ADXL372_THRESH_VAL_H_SEL(x) \
+ FIELD_GET(ADXL372_THRESH_VAL_H_MSK, x)
+#define ADXL372_THRESH_VAL_L_MSK GENMASK(2, 0)
+#define ADXL372_THRESH_VAL_L_SEL(x) \
+ FIELD_GET(ADXL372_THRESH_VAL_L_MSK, x)
+
/* The ADXL372 includes a deep, 512 sample FIFO buffer */
#define ADXL372_FIFO_SIZE 512

@@ -222,6 +231,32 @@ static const struct adxl372_axis_lookup adxl372_axis_lookup_table[] = {
{ BIT(0) | BIT(1) | BIT(2), ADXL372_XYZ_FIFO },
};

+static ssize_t adxl372_read_threshold_value(struct iio_dev *, uintptr_t,
+ const struct iio_chan_spec *,
+ char *);
+
+static ssize_t adxl372_write_threshold_value(struct iio_dev *, uintptr_t,
+ struct iio_chan_spec const *,
+ const char *, size_t);
+
+static const struct iio_chan_spec_ext_info adxl372_ext_info[] = {
+ {
+ .name = "threshold_activity",
+ .shared = IIO_SEPARATE,
+ .read = adxl372_read_threshold_value,
+ .write = adxl372_write_threshold_value,
+ .private = ADXL372_X_THRESH_ACT_H,
+ },
+ {
+ .name = "threshold_inactivity",
+ .shared = IIO_SEPARATE,
+ .read = adxl372_read_threshold_value,
+ .write = adxl372_write_threshold_value,
+ .private = ADXL372_X_THRESH_INACT_H,
+ },
+ {},
+};
+
#define ADXL372_ACCEL_CHANNEL(index, reg, axis) { \
.type = IIO_ACCEL, \
.address = reg, \
@@ -239,6 +274,7 @@ static const struct adxl372_axis_lookup adxl372_axis_lookup_table[] = {
.shift = 4, \
.endianness = IIO_BE, \
}, \
+ .ext_info = adxl372_ext_info, \
}

static const struct iio_chan_spec adxl372_channels[] = {
@@ -277,6 +313,61 @@ static const unsigned long adxl372_channel_masks[] = {
0
};

+static ssize_t adxl372_read_threshold_value(struct iio_dev *indio_dev,
+ uintptr_t private,
+ const struct iio_chan_spec *chan,
+ char *buf)
+{
+ struct adxl372_state *st = iio_priv(indio_dev);
+ unsigned int addr;
+ __be16 __regval;
+ u16 regval;
+ int ret;
+
+ addr = (unsigned int)chan->ext_info->private;
+ addr = addr + chan->scan_index * 2;
+
+ ret = regmap_bulk_read(st->regmap, addr, &__regval, sizeof(__regval));
+ if (ret < 0)
+ return ret;
+
+ regval = be16_to_cpu(__regval);
+ regval >>= 5;
+
+ return sprintf(buf, "%d\n", regval);
+}
+
+static ssize_t adxl372_write_threshold_value(struct iio_dev *indio_dev,
+ uintptr_t private,
+ const struct iio_chan_spec *chan,
+ const char *buf,
+ size_t len)
+{
+ struct adxl372_state *st = iio_priv(indio_dev);
+ unsigned int addr;
+ u16 threshold;
+ int ret;
+
+ ret = kstrtou16(buf, 0, &threshold);
+ if (ret < 0)
+ return ret;
+
+ addr = chan->ext_info->private;
+ addr = addr + chan->scan_index * 2;
+
+ ret = regmap_write(st->regmap, addr,
+ ADXL372_THRESH_VAL_H_SEL(threshold));
+ if (ret < 0)
+ return ret;
+
+ ret = regmap_update_bits(st->regmap, addr + 1, GENMASK(7, 5),
+ ADXL372_THRESH_VAL_L_SEL(threshold) << 5);
+ if (ret < 0)
+ return ret;
+
+ return len;
+}
+
static int adxl372_read_axis(struct adxl372_state *st, u8 addr)
{
__be16 regval;
--
2.20.1

2020-02-14 09:34:00

by Alexandru Tachici

[permalink] [raw]
Subject: [PATCH 5/5] iio: accel: adxl372: Update sysfs docs

This patch adds entries in the syfs docs of ADXL372.

Signed-off-by: Stefan Popa <[email protected]>
Signed-off-by: Alexandru Tachici <[email protected]>
---
.../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: [email protected]
+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.
+
+What: /sys/bus/iio/devices/iio:deviceX/time_activity
+KernelVersion:
+Contact: [email protected]
+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.
+
+What: /sys/bus/iio/devices/iio:deviceX/time_inactivity
+KernelVersion:
+Contact: [email protected]
+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: [email protected]
+Description:
+ This attribute allows to set the activity threshold in 100 mg
+ (0.1 m/s^2 SI).
+
+What: /sys/bus/iio/devices/iio:deviceX/in_accel_x_threshold_inactivity
+KernelVersion:
+Contact: [email protected]
+Description:
+ This attribute allows to set the inactivity threshold in 100 mg
+ (0.1 m/s^2 SI).
--
2.20.1

2020-02-15 16:11:14

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH 5/5] iio: accel: adxl372: Update sysfs docs

On Fri, 14 Feb 2020 11:32:23 +0200
Alexandru Tachici <[email protected]> wrote:

> This patch adds entries in the syfs docs of ADXL372.
>
> Signed-off-by: Stefan Popa <[email protected]>
> Signed-off-by: Alexandru Tachici <[email protected]>
> ---
> .../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: [email protected]
> +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: [email protected]
> +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: [email protected]
> +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: [email protected]
> +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: [email protected]
> +Description:
> + This attribute allows to set the inactivity threshold in 100 mg
> + (0.1 m/s^2 SI).