[PATCH v2 0/6] iio: accel: adxl372: add peak mode

[PATCH v2 0/6] iio: accel: adxl372: add peak mode

[Alexandru Tachici]

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.

1. Create sysfs files for falling_period and rising_period
in events/ dir.

2. Create sysfs files for thresh_falling_value and
thresh_rising_value for each axis.

3. Set INT1 reg for acitivity/inactivity and push
event code in events fifo on irq.

4. Add additional ABIs in order to explain the
user that setting values in the events/ sysfs files
also changes device behaviour.

5. Update sysfs docs: renames, added two new
entries for the activity_detect_event/
inactivity_detect_event

Alexandru Tachici (5):
  iio: accel: adxl372: add sysfs for time registers
  iio: accel: adxl372: Add sysfs for g thresholds
  iio: accel: adxl372: add iio events
  iio: accel: adxl372: add additional events ABIs
  iio: accel: adxl372: Update sysfs docs

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

1. Device FIFO can now be set in peak mode and only over the
threshold accelerations will be stored.

Changelog V1 -> V2:
- switched from custom sysfs for setting the accel thres/timings
to standard events interface.
- renamed fifo_peak_mode_enable to buffer_peak_mode_enable
- on activity/inactivity, event codes are now pushed in
the events FIFO
- added additional custom sysfs for explaining that setting
values in the events/ sysfs files will change the device's
behaviour.

 .../ABI/testing/sysfs-bus-iio-accel-adxl372   |  30 ++
 drivers/iio/accel/adxl372.c                   | 265 +++++++++++++++++-
 2 files changed, 293 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-accel-adxl372

-- 
2.20.1

[PATCH v2 1/6] iio: accel: adxl372: Add support for FIFO peak mode

[Alexandru Tachici]

From: Stefan Popa <stefan.popa@analog.com>

By default, if all three channels (x, y, z) are enabled, sample sets of
concurrent 3-axis data is stored in the FIFO. This patch adds the option
to configure the FIFO to store peak acceleration (x, y and z) of every
over-threshold event. Since we cannot store 1 or 2 axis peak acceleration
data in the FIFO, then all three axis need to be enabled in order for this
mode to work.

Signed-off-by: Stefan Popa <stefan.popa@analog.com>
---
 drivers/iio/accel/adxl372.c | 46 +++++++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)

diff --git a/drivers/iio/accel/adxl372.c b/drivers/iio/accel/adxl372.c
index 67b8817995c0..ed93534f8dba 100644
--- a/drivers/iio/accel/adxl372.c
+++ b/drivers/iio/accel/adxl372.c
@@ -264,6 +264,7 @@ struct adxl372_state {
 	u8				int2_bitmask;
 	u16				watermark;
 	__be16				fifo_buf[ADXL372_FIFO_SIZE];
+	bool				peak_fifo_mode_en;
 };
 
 static const unsigned long adxl372_channel_masks[] = {
@@ -722,6 +723,40 @@ static int adxl372_write_raw(struct iio_dev *indio_dev,
 	}
 }
 
+static ssize_t adxl372_peak_fifo_en_get(struct device *dev,
+					struct device_attribute *attr,
+					char *buf)
+{
+	struct adxl372_state *st = iio_priv(dev_to_iio_dev(dev));
+
+	return sprintf(buf, "%d\n", st->peak_fifo_mode_en);
+}
+
+static ssize_t adxl372_peak_fifo_en_set(struct device *dev,
+					struct device_attribute *attr,
+					const char *buf, size_t len)
+{
+	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+	struct adxl372_state *st = iio_priv(indio_dev);
+	bool val;
+	int ret;
+
+	if (iio_buffer_enabled(indio_dev))
+		return -EBUSY;
+
+	ret = kstrtobool(buf, &val);
+	if (ret)
+		return ret;
+
+	st->peak_fifo_mode_en = val;
+
+	return len;
+}
+
+static IIO_DEVICE_ATTR(buffer_peak_mode_enable, 0644,
+		       adxl372_peak_fifo_en_get,
+		       adxl372_peak_fifo_en_set, 0);
+
 static ssize_t adxl372_show_filter_freq_avail(struct device *dev,
 					      struct device_attribute *attr,
 					      char *buf)
@@ -817,11 +852,21 @@ static int adxl372_buffer_postenable(struct iio_dev *indio_dev)
 	st->fifo_format = adxl372_axis_lookup_table[i].fifo_format;
 	st->fifo_set_size = bitmap_weight(indio_dev->active_scan_mask,
 					  indio_dev->masklength);
+
+	/* Configure the FIFO to store sets of impact event peak. */
+	if (st->peak_fifo_mode_en) {
+		st->fifo_format = ADXL372_XYZ_PEAK_FIFO;
+		if (st->fifo_set_size != 3)
+			dev_warn(&indio_dev->dev,
+				 "All axis must be enabled in peak mode.");
+	}
+
 	/*
 	 * The 512 FIFO samples can be allotted in several ways, such as:
 	 * 170 sample sets of concurrent 3-axis data
 	 * 256 sample sets of concurrent 2-axis data (user selectable)
 	 * 512 sample sets of single-axis data
+	 * 170 sets of impact event peak (x, y, z)
 	 */
 	if ((st->watermark * st->fifo_set_size) > ADXL372_FIFO_SIZE)
 		st->watermark = (ADXL372_FIFO_SIZE  / st->fifo_set_size);
@@ -894,6 +939,7 @@ static IIO_DEVICE_ATTR(in_accel_filter_low_pass_3db_frequency_available,
 static struct attribute *adxl372_attributes[] = {
 	&iio_const_attr_sampling_frequency_available.dev_attr.attr,
 	&iio_dev_attr_in_accel_filter_low_pass_3db_frequency_available.dev_attr.attr,
+	&iio_dev_attr_buffer_peak_mode_enable.dev_attr.attr,
 	NULL,
 };
 
-- 
2.20.1

[PATCH v2 2/6] iio: accel: adxl372: add sysfs for time registers

[Alexandru Tachici]

Currently the driver configures adxl372 to work in loop mode.
The inactivity and activity timings  decide how fast the chip
will loop through the awake and waiting states.

This patch adds standard events sysfs entries for the inactivity
and activity timings: thresh_falling_period/thresh_rising_period.

Signed-off-by: Alexandru Tachici <alexandru.tachici@analog.com>
---
 drivers/iio/accel/adxl372.c | 68 +++++++++++++++++++++++++++++++++++++
 1 file changed, 68 insertions(+)

diff --git a/drivers/iio/accel/adxl372.c b/drivers/iio/accel/adxl372.c
index ed93534f8dba..5da3c924c62d 100644
--- a/drivers/iio/accel/adxl372.c
+++ b/drivers/iio/accel/adxl372.c
@@ -222,6 +222,18 @@ static const struct adxl372_axis_lookup adxl372_axis_lookup_table[] = {
 	{ BIT(0) | BIT(1) | BIT(2), ADXL372_XYZ_FIFO },
 };
 
+static const struct iio_event_spec adxl372_events[] = {
+	{
+		.type = IIO_EV_TYPE_THRESH,
+		.dir = IIO_EV_DIR_RISING,
+		.mask_shared_by_all = BIT(IIO_EV_INFO_PERIOD),
+	}, {
+		.type = IIO_EV_TYPE_THRESH,
+		.dir = IIO_EV_DIR_FALLING,
+		.mask_shared_by_all = BIT(IIO_EV_INFO_PERIOD),
+	},
+};
+
 #define ADXL372_ACCEL_CHANNEL(index, reg, axis) {			\
 	.type = IIO_ACCEL,						\
 	.address = reg,							\
@@ -238,6 +250,8 @@ static const struct adxl372_axis_lookup adxl372_axis_lookup_table[] = {
 		.storagebits = 16,					\
 		.shift = 4,						\
 	},								\
+	.event_spec = adxl372_events,					\
+	.num_event_specs = 2						\
 }
 
 static const struct iio_chan_spec adxl372_channels[] = {
@@ -723,6 +737,58 @@ static int adxl372_write_raw(struct iio_dev *indio_dev,
 	}
 }
 
+int adxl372_read_event_value(struct iio_dev *indio_dev,
+			     const struct iio_chan_spec *chan,
+			     enum iio_event_type type,
+			     enum iio_event_direction dir,
+			     enum iio_event_info info, int *val, int *val2)
+{
+	struct adxl372_state *st = iio_priv(indio_dev);
+
+	switch (info) {
+	case IIO_EV_INFO_PERIOD:
+		switch (dir) {
+		case IIO_EV_DIR_RISING:
+			*val = st->act_time_ms;
+			*val2 = 1000;
+			return IIO_VAL_FRACTIONAL;
+		case IIO_EV_DIR_FALLING:
+			*val = st->inact_time_ms;
+			*val2 = 1000;
+			return IIO_VAL_FRACTIONAL;
+		default:
+			return -EINVAL;
+		}
+	default:
+		return -EINVAL;
+	}
+}
+
+int adxl372_write_event_value(struct iio_dev *indio_dev,
+			      const struct iio_chan_spec *chan,
+			      enum iio_event_type type,
+			      enum iio_event_direction dir,
+			      enum iio_event_info info, int val, int val2)
+{
+	struct adxl372_state *st = iio_priv(indio_dev);
+	unsigned int val_ms;
+
+	switch (info) {
+	case IIO_EV_INFO_PERIOD:
+		val_ms = val * 1000 + DIV_ROUND_UP(val2, 1000);
+		switch (dir) {
+		case IIO_EV_DIR_RISING:
+			return adxl372_set_activity_time_ms(st, val_ms);
+		case IIO_EV_DIR_FALLING:
+			return adxl372_set_inactivity_time_ms(st, val_ms);
+		default:
+			return -EINVAL;
+		}
+	default:
+		return -EINVAL;
+	}
+}
+
 static ssize_t adxl372_peak_fifo_en_get(struct device *dev,
 					struct device_attribute *attr,
 					char *buf)
@@ -952,6 +1018,8 @@ static const struct iio_info adxl372_info = {
 	.attrs = &adxl372_attrs_group,
 	.read_raw = adxl372_read_raw,
 	.write_raw = adxl372_write_raw,
+	.read_event_value = adxl372_read_event_value,
+	.write_event_value = adxl372_write_event_value,
 	.debugfs_reg_access = &adxl372_reg_access,
 	.hwfifo_set_watermark = adxl372_set_watermark,
 };
-- 
2.20.1

[PATCH v2 3/6] iio: accel: adxl372: Add sysfs for g thresholds

[Alexandru Tachici]

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 event attributes for activity and inactivity
thresholds for each axis.

Signed-off-by: Alexandru Tachici <alexandru.tachici@analog.com>
---
 drivers/iio/accel/adxl372.c | 90 +++++++++++++++++++++++++++++++++++++
 1 file changed, 90 insertions(+)

diff --git a/drivers/iio/accel/adxl372.c b/drivers/iio/accel/adxl372.c
index 5da3c924c62d..775dc4f0aaf4 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
 
@@ -226,10 +235,12 @@ static const struct iio_event_spec adxl372_events[] = {
 	{
 		.type = IIO_EV_TYPE_THRESH,
 		.dir = IIO_EV_DIR_RISING,
+		.mask_separate = BIT(IIO_EV_INFO_VALUE),
 		.mask_shared_by_all = BIT(IIO_EV_INFO_PERIOD),
 	}, {
 		.type = IIO_EV_TYPE_THRESH,
 		.dir = IIO_EV_DIR_FALLING,
+		.mask_separate = BIT(IIO_EV_INFO_VALUE),
 		.mask_shared_by_all = BIT(IIO_EV_INFO_PERIOD),
 	},
 };
@@ -290,6 +301,43 @@ static const unsigned long adxl372_channel_masks[] = {
 	0
 };
 
+static ssize_t adxl372_read_threshold_value(struct iio_dev *indio_dev,
+					    unsigned int addr,
+					    u16 *threshold)
+{
+	struct adxl372_state *st = iio_priv(indio_dev);
+	__be16 __regval;
+	u16 regval;
+	int ret;
+
+	ret = regmap_bulk_read(st->regmap, addr, &__regval, sizeof(__regval));
+	if (ret < 0)
+		return ret;
+
+	regval = be16_to_cpu(__regval);
+	regval >>= 5;
+
+	*threshold = regval;
+
+	return 0;
+}
+
+static ssize_t adxl372_write_threshold_value(struct iio_dev *indio_dev,
+					     unsigned int addr,
+					     u16 threshold)
+{
+	struct adxl372_state *st = iio_priv(indio_dev);
+	int ret;
+
+	ret = regmap_write(st->regmap, addr,
+			   ADXL372_THRESH_VAL_H_SEL(threshold));
+	if (ret < 0)
+		return ret;
+
+	return regmap_update_bits(st->regmap, addr + 1, GENMASK(7, 5),
+				  ADXL372_THRESH_VAL_L_SEL(threshold) << 5);
+}
+
 static int adxl372_read_axis(struct adxl372_state *st, u8 addr)
 {
 	__be16 regval;
@@ -744,8 +792,34 @@ int adxl372_read_event_value(struct iio_dev *indio_dev,
 			     enum iio_event_info info, int *val, int *val2)
 {
 	struct adxl372_state *st = iio_priv(indio_dev);
+	unsigned int addr;
+	u16 raw_value;
+	int ret;
 
 	switch (info) {
+	case IIO_EV_INFO_VALUE:
+		switch (dir) {
+		case IIO_EV_DIR_RISING:
+			addr = ADXL372_X_THRESH_ACT_H + 2 * chan->scan_index;
+			ret = adxl372_read_threshold_value(indio_dev, addr,
+							   &raw_value);
+			if (ret < 0)
+				return ret;
+			*val = raw_value * ADXL372_USCALE;
+			*val2 = 1000000;
+			return IIO_VAL_FRACTIONAL;
+		case IIO_EV_DIR_FALLING:
+			addr = ADXL372_X_THRESH_INACT_H + 2 * chan->scan_index;
+			ret =  adxl372_read_threshold_value(indio_dev, addr,
+							    &raw_value);
+			if (ret < 0)
+				return ret;
+			*val = raw_value * ADXL372_USCALE;
+			*val2 = 1000000;
+			return IIO_VAL_FRACTIONAL;
+		default:
+			return -EINVAL;
+		}
 	case IIO_EV_INFO_PERIOD:
 		switch (dir) {
 		case IIO_EV_DIR_RISING:
@@ -772,8 +846,24 @@ int adxl372_write_event_value(struct iio_dev *indio_dev,
 {
 	struct adxl372_state *st = iio_priv(indio_dev);
 	unsigned int val_ms;
+	unsigned int addr;
+	u16 raw_val;
 
 	switch (info) {
+	case IIO_EV_INFO_VALUE:
+		raw_val = DIV_ROUND_UP(val * 1000000, ADXL372_USCALE);
+		switch (dir) {
+		case IIO_EV_DIR_RISING:
+			addr = ADXL372_X_THRESH_ACT_H + 2 * chan->scan_index;
+			return adxl372_write_threshold_value(indio_dev, addr,
+							     raw_val);
+		case IIO_EV_DIR_FALLING:
+			addr = ADXL372_X_THRESH_INACT_H + 2 * chan->scan_index;
+			return adxl372_write_threshold_value(indio_dev, addr,
+							     raw_val);
+		default:
+			return -EINVAL;
+		}
 	case IIO_EV_INFO_PERIOD:
 		val_ms = val * 1000 + DIV_ROUND_UP(val2, 1000);
 		switch (dir) {
-- 
2.20.1

[PATCH v2 4/6] iio: accel: adxl372: add iio events

[Alexandru Tachici]

Set INT1 interrupts to fire when activity/inactivity
is detected by the device. On irq, push event code by calling
iio_push_event().

Signed-off-by: Alexandru Tachici <alexandru.tachici@analog.com>
---
 drivers/iio/accel/adxl372.c | 37 +++++++++++++++++++++++++++++++++++--
 1 file changed, 35 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/accel/adxl372.c b/drivers/iio/accel/adxl372.c
index 775dc4f0aaf4..e669eaaaa07e 100644
--- a/drivers/iio/accel/adxl372.c
+++ b/drivers/iio/accel/adxl372.c
@@ -114,6 +114,11 @@
 #define ADXL372_STATUS_1_AWAKE(x)		(((x) >> 6) & 0x1)
 #define ADXL372_STATUS_1_ERR_USR_REGS(x)	(((x) >> 7) & 0x1)
 
+/* ADXL372_STATUS_2 */
+#define ADXL372_STATUS_2_INACT(x)		(((x) >> 4) & 0x1)
+#define ADXL372_STATUS_2_ACT(x)			(((x) >> 5) & 0x1)
+#define ADXL372_STATUS_2_AC2(x)			(((x) >> 6) & 0x1)
+
 /* ADXL372_INT1_MAP */
 #define ADXL372_INT1_MAP_DATA_RDY_MSK		BIT(0)
 #define ADXL372_INT1_MAP_DATA_RDY_MODE(x)	(((x) & 0x1) << 0)
@@ -585,6 +590,27 @@ static int adxl372_get_status(struct adxl372_state *st,
 	return ret;
 }
 
+static void adxl372_push_event(struct iio_dev *indio_dev, s64 timestamp,
+			       u8 status2)
+{
+	unsigned int ev_dir;
+
+	if (ADXL372_STATUS_2_ACT(status2))
+		ev_dir = IIO_EV_DIR_RISING;
+
+	if (ADXL372_STATUS_2_INACT(status2))
+		ev_dir = IIO_EV_DIR_FALLING;
+
+	if (ev_dir != IIO_EV_DIR_NONE)
+		iio_push_event(indio_dev,
+			       IIO_MOD_EVENT_CODE(IIO_ACCEL,
+						  0,
+						  IIO_MOD_X_OR_Y_OR_Z,
+						  IIO_EV_TYPE_THRESH,
+						  ev_dir),
+						  timestamp);
+}
+
 static irqreturn_t adxl372_trigger_handler(int irq, void  *p)
 {
 	struct iio_poll_func *pf = p;
@@ -598,6 +624,8 @@ static irqreturn_t adxl372_trigger_handler(int irq, void  *p)
 	if (ret < 0)
 		goto err;
 
+	adxl372_push_event(indio_dev, iio_get_time_ns(indio_dev), status2);
+
 	if (st->fifo_mode != ADXL372_FIFO_BYPASSED &&
 	    ADXL372_STATUS_1_FIFO_FULL(status1)) {
 		/*
@@ -989,7 +1017,10 @@ static int adxl372_buffer_postenable(struct iio_dev *indio_dev)
 	if (ret < 0)
 		return ret;
 
-	ret = adxl372_set_interrupts(st, ADXL372_INT1_MAP_FIFO_FULL_MSK, 0);
+	mask = ADXL372_INT1_MAP_FIFO_FULL_MSK |
+	       ADXL372_INT1_MAP_ACT_MSK	 |
+	       ADXL372_INT1_MAP_INACT_MSK;
+	ret = adxl372_set_interrupts(st, mask, 0);
 	if (ret < 0)
 		goto err;
 
@@ -1067,7 +1098,9 @@ static int adxl372_dready_trig_set_state(struct iio_trigger *trig,
 	unsigned long int mask = 0;
 
 	if (state)
-		mask = ADXL372_INT1_MAP_FIFO_FULL_MSK;
+		mask = ADXL372_INT1_MAP_FIFO_FULL_MSK |
+		       ADXL372_INT1_MAP_ACT_MSK	 |
+		       ADXL372_INT1_MAP_INACT_MSK;
 
 	return adxl372_set_interrupts(st, mask, 0);
 }
-- 
2.20.1

[PATCH v2 5/6] iio: accel: adxl372: add additional events ABIs

[Alexandru Tachici]

Adxl372 uses the standard event interface. The additional
ABIs aim to explain to the user that the values set in
./events/thresh_falling_period and ./events/thresh_rising_period
control the state of the device, not just the events timings.

Signed-off-by: Alexandru Tachici <alexandru.tachici@analog.com>
---
 drivers/iio/accel/adxl372.c | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/accel/adxl372.c b/drivers/iio/accel/adxl372.c
index e669eaaaa07e..9a7fa0d796f8 100644
--- a/drivers/iio/accel/adxl372.c
+++ b/drivers/iio/accel/adxl372.c
@@ -236,6 +236,29 @@ static const struct adxl372_axis_lookup adxl372_axis_lookup_table[] = {
 	{ BIT(0) | BIT(1) | BIT(2), ADXL372_XYZ_FIFO },
 };
 
+static ssize_t adxl372_read_detect_event(struct iio_dev *indio_dev, uintptr_t p,
+					 const struct iio_chan_spec *chan,
+					 char *buf)
+{
+	return sprintf(buf, "%s", (const char *)p);
+}
+
+static const struct iio_chan_spec_ext_info adxl372_ext_info[] = {
+	{
+		.name = "activity_detect_event",
+		.shared = IIO_SHARED_BY_ALL,
+		.read = adxl372_read_detect_event,
+		.private = (uintptr_t)"in_accel_thresh_x_rising\n",
+	},
+	{
+		.name = "inactivity_detect_event",
+		.shared = IIO_SHARED_BY_ALL,
+		.read = adxl372_read_detect_event,
+		.private = (uintptr_t)"in_accel_thresh_x_falling\n",
+	},
+	{},
+};
+
 static const struct iio_event_spec adxl372_events[] = {
 	{
 		.type = IIO_EV_TYPE_THRESH,
@@ -267,7 +290,8 @@ static const struct iio_event_spec adxl372_events[] = {
 		.shift = 4,						\
 	},								\
 	.event_spec = adxl372_events,					\
-	.num_event_specs = 2						\
+	.num_event_specs = 2,						\
+	.ext_info = adxl372_ext_info,					\
 }
 
 static const struct iio_chan_spec adxl372_channels[] = {
-- 
2.20.1

[PATCH v2 6/6] iio: accel: adxl372: Update sysfs docs

[Alexandru Tachici]

This patch adds entries in the syfs docs of ADXL372.

Signed-off-by: Stefan Popa <stefan.popa@analog.com>
Signed-off-by: Alexandru Tachici <alexandru.tachici@analog.com>
---
 .../ABI/testing/sysfs-bus-iio-accel-adxl372   | 30 +++++++++++++++++++
 1 file changed, 30 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..709376b54bec
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-iio-accel-adxl372
@@ -0,0 +1,30 @@
+What:		/sys/bus/iio/devices/iio:deviceX/buffer_peak_mode_enable
+KernelVersion:
+Contact:	linux-iio@vger.kernel.org
+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/activity_detect_event
+KernelVersion:
+Contact:	linux-iio@vger.kernel.org
+Description:
+		adxl372 works in loop mode. It will loop  between activity
+		and inactivity detection mode. The thresh_rising sysfs files
+		found in events/ need to be configured in order to define when
+		the device will mark a sensed acceleration over a period of
+		time as activity.
+
+What:		/sys/bus/iio/devices/iio:deviceX/inactivity_detect_event
+KernelVersion:
+Contact:	linux-iio@vger.kernel.org
+Description:
+		adxl372 works in loop mode. It will loop  between activity
+		and inactivity detection mode. The thresh_falling sysfs files
+		found in events/ need to be configured in order to define when
+		the device will mark a sensed acceleration over a period of
+		time as inactivity.
-- 
2.20.1

Re: [PATCH v2 1/6] iio: accel: adxl372: Add support for FIFO peak mode

[Jonathan Cameron]

On Tue, 25 Feb 2020 14:09:04 +0200
Alexandru Tachici <alexandru.tachici@analog.com> wrote:

> From: Stefan Popa <stefan.popa@analog.com>
> 
> By default, if all three channels (x, y, z) are enabled, sample sets of
> concurrent 3-axis data is stored in the FIFO. This patch adds the option
> to configure the FIFO to store peak acceleration (x, y and z) of every
> over-threshold event. Since we cannot store 1 or 2 axis peak acceleration
> data in the FIFO, then all three axis need to be enabled in order for this
> mode to work.
> 
> Signed-off-by: Stefan Popa <stefan.popa@analog.com>

First apologies this was one of the sets that I didn't get to last week.

This is a bit of an odd one, but I suppose the general approach is fine.

A few questions on specifics inline.

> ---
>  drivers/iio/accel/adxl372.c | 46 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 46 insertions(+)
> 
> diff --git a/drivers/iio/accel/adxl372.c b/drivers/iio/accel/adxl372.c
> index 67b8817995c0..ed93534f8dba 100644
> --- a/drivers/iio/accel/adxl372.c
> +++ b/drivers/iio/accel/adxl372.c
> @@ -264,6 +264,7 @@ struct adxl372_state {
>  	u8				int2_bitmask;
>  	u16				watermark;
>  	__be16				fifo_buf[ADXL372_FIFO_SIZE];
> +	bool				peak_fifo_mode_en;
>  };
>  
>  static const unsigned long adxl372_channel_masks[] = {
> @@ -722,6 +723,40 @@ static int adxl372_write_raw(struct iio_dev *indio_dev,
>  	}
>  }
>  
> +static ssize_t adxl372_peak_fifo_en_get(struct device *dev,
> +					struct device_attribute *attr,
> +					char *buf)
> +{
> +	struct adxl372_state *st = iio_priv(dev_to_iio_dev(dev));
> +
> +	return sprintf(buf, "%d\n", st->peak_fifo_mode_en);
> +}
> +
> +static ssize_t adxl372_peak_fifo_en_set(struct device *dev,
> +					struct device_attribute *attr,
> +					const char *buf, size_t len)
> +{
> +	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> +	struct adxl372_state *st = iio_priv(indio_dev);
> +	bool val;
> +	int ret;
> +
> +	if (iio_buffer_enabled(indio_dev))
> +		return -EBUSY;

Prefer if you use the iio_claim_direct_mode to ensure we are in
non buffered mode until we reach a consistent state.  Then set
the variable and release direct mode.

Otherwise you might have a race between a write
to this and enabling the buffered mode.

> +
> +	ret = kstrtobool(buf, &val);
> +	if (ret)
> +		return ret;
> +
> +	st->peak_fifo_mode_en = val;
> +
> +	return len;
> +}
> +
> +static IIO_DEVICE_ATTR(buffer_peak_mode_enable, 0644,
> +		       adxl372_peak_fifo_en_get,
> +		       adxl372_peak_fifo_en_set, 0);
> +
>  static ssize_t adxl372_show_filter_freq_avail(struct device *dev,
>  					      struct device_attribute *attr,
>  					      char *buf)
> @@ -817,11 +852,21 @@ static int adxl372_buffer_postenable(struct iio_dev *indio_dev)
>  	st->fifo_format = adxl372_axis_lookup_table[i].fifo_format;
>  	st->fifo_set_size = bitmap_weight(indio_dev->active_scan_mask,
>  					  indio_dev->masklength);
> +
> +	/* Configure the FIFO to store sets of impact event peak. */
> +	if (st->peak_fifo_mode_en) {
> +		st->fifo_format = ADXL372_XYZ_PEAK_FIFO;
> +		if (st->fifo_set_size != 3)
> +			dev_warn(&indio_dev->dev,
> +				 "All axis must be enabled in peak mode.");

What happens otherwise?  Real question is should this just be an error and
result in us dropping out of buffered mode again?  How would a userspace
program that had hit this know there was an issue?

> +	}
> +
>  	/*
>  	 * The 512 FIFO samples can be allotted in several ways, such as:
>  	 * 170 sample sets of concurrent 3-axis data
>  	 * 256 sample sets of concurrent 2-axis data (user selectable)
>  	 * 512 sample sets of single-axis data
> +	 * 170 sets of impact event peak (x, y, z)
>  	 */
>  	if ((st->watermark * st->fifo_set_size) > ADXL372_FIFO_SIZE)
>  		st->watermark = (ADXL372_FIFO_SIZE  / st->fifo_set_size);
> @@ -894,6 +939,7 @@ static IIO_DEVICE_ATTR(in_accel_filter_low_pass_3db_frequency_available,
>  static struct attribute *adxl372_attributes[] = {
>  	&iio_const_attr_sampling_frequency_available.dev_attr.attr,
>  	&iio_dev_attr_in_accel_filter_low_pass_3db_frequency_available.dev_attr.attr,
> +	&iio_dev_attr_buffer_peak_mode_enable.dev_attr.attr,
>  	NULL,
>  };
>  

Re: [PATCH v2 6/6] iio: accel: adxl372: Update sysfs docs

[Jonathan Cameron]

On Tue, 25 Feb 2020 14:09:09 +0200
Alexandru Tachici <alexandru.tachici@analog.com> wrote:

> This patch adds entries in the syfs docs of ADXL372.
> 
> Signed-off-by: Stefan Popa <stefan.popa@analog.com>
> Signed-off-by: Alexandru Tachici <alexandru.tachici@analog.com>
> ---
>  .../ABI/testing/sysfs-bus-iio-accel-adxl372   | 30 +++++++++++++++++++
>  1 file changed, 30 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..709376b54bec
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-bus-iio-accel-adxl372
> @@ -0,0 +1,30 @@
> +What:		/sys/bus/iio/devices/iio:deviceX/buffer_peak_mode_enable
> +KernelVersion:
> +Contact:	linux-iio@vger.kernel.org
> +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.

As this is referring to the internal fifo (I think!) should we name it
hwfifo_peak_mode_enable to separate it from being related to the software "buffer"?


> +
> +What:		/sys/bus/iio/devices/iio:deviceX/activity_detect_event
> +KernelVersion:
> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		adxl372 works in loop mode. It will loop  between activity
> +		and inactivity detection mode. The thresh_rising sysfs files
> +		found in events/ need to be configured in order to define when
> +		the device will mark a sensed acceleration over a period of
> +		time as activity.

Hmm. As you noted in the cover letter this is a bit odd having a mixture of
an event and a more fundamental state control.

Please state what value this will provide when read.

> +
> +What:		/sys/bus/iio/devices/iio:deviceX/inactivity_detect_event
> +KernelVersion:
> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		adxl372 works in loop mode. It will loop  between activity
> +		and inactivity detection mode. The thresh_falling sysfs files
> +		found in events/ need to be configured in order to define when
> +		the device will mark a sensed acceleration over a period of
> +		time as inactivity.

Re: [PATCH v2 4/6] iio: accel: adxl372: add iio events

[Jonathan Cameron]

On Tue, 25 Feb 2020 14:09:07 +0200
Alexandru Tachici <alexandru.tachici@analog.com> wrote:

> Set INT1 interrupts to fire when activity/inactivity
> is detected by the device. On irq, push event code by calling
> iio_push_event().
> 
> Signed-off-by: Alexandru Tachici <alexandru.tachici@analog.com>
> ---
>  drivers/iio/accel/adxl372.c | 37 +++++++++++++++++++++++++++++++++++--
>  1 file changed, 35 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/accel/adxl372.c b/drivers/iio/accel/adxl372.c
> index 775dc4f0aaf4..e669eaaaa07e 100644
> --- a/drivers/iio/accel/adxl372.c
> +++ b/drivers/iio/accel/adxl372.c
> @@ -114,6 +114,11 @@
>  #define ADXL372_STATUS_1_AWAKE(x)		(((x) >> 6) & 0x1)
>  #define ADXL372_STATUS_1_ERR_USR_REGS(x)	(((x) >> 7) & 0x1)
>  
> +/* ADXL372_STATUS_2 */
> +#define ADXL372_STATUS_2_INACT(x)		(((x) >> 4) & 0x1)
> +#define ADXL372_STATUS_2_ACT(x)			(((x) >> 5) & 0x1)
> +#define ADXL372_STATUS_2_AC2(x)			(((x) >> 6) & 0x1)
> +
>  /* ADXL372_INT1_MAP */
>  #define ADXL372_INT1_MAP_DATA_RDY_MSK		BIT(0)
>  #define ADXL372_INT1_MAP_DATA_RDY_MODE(x)	(((x) & 0x1) << 0)
> @@ -585,6 +590,27 @@ static int adxl372_get_status(struct adxl372_state *st,
>  	return ret;
>  }
>  
> +static void adxl372_push_event(struct iio_dev *indio_dev, s64 timestamp,
> +			       u8 status2)
> +{
> +	unsigned int ev_dir;
> +
> +	if (ADXL372_STATUS_2_ACT(status2))
> +		ev_dir = IIO_EV_DIR_RISING;
> +
> +	if (ADXL372_STATUS_2_INACT(status2))
> +		ev_dir = IIO_EV_DIR_FALLING;
> +
> +	if (ev_dir != IIO_EV_DIR_NONE)

Unless I'm missing something we don't have an enable / disable control for this
event.  Can we add one?

> +		iio_push_event(indio_dev,
> +			       IIO_MOD_EVENT_CODE(IIO_ACCEL,
> +						  0,
> +						  IIO_MOD_X_OR_Y_OR_Z,
> +						  IIO_EV_TYPE_THRESH,
> +						  ev_dir),
> +						  timestamp);
> +}
> +
>  static irqreturn_t adxl372_trigger_handler(int irq, void  *p)
>  {
>  	struct iio_poll_func *pf = p;
> @@ -598,6 +624,8 @@ static irqreturn_t adxl372_trigger_handler(int irq, void  *p)
>  	if (ret < 0)
>  		goto err;
>  
> +	adxl372_push_event(indio_dev, iio_get_time_ns(indio_dev), status2);
> +
>  	if (st->fifo_mode != ADXL372_FIFO_BYPASSED &&
>  	    ADXL372_STATUS_1_FIFO_FULL(status1)) {
>  		/*
> @@ -989,7 +1017,10 @@ static int adxl372_buffer_postenable(struct iio_dev *indio_dev)
>  	if (ret < 0)
>  		return ret;
>  
> -	ret = adxl372_set_interrupts(st, ADXL372_INT1_MAP_FIFO_FULL_MSK, 0);
> +	mask = ADXL372_INT1_MAP_FIFO_FULL_MSK |
> +	       ADXL372_INT1_MAP_ACT_MSK	 |
> +	       ADXL372_INT1_MAP_INACT_MSK;
> +	ret = adxl372_set_interrupts(st, mask, 0);
>  	if (ret < 0)
>  		goto err;
>  
> @@ -1067,7 +1098,9 @@ static int adxl372_dready_trig_set_state(struct iio_trigger *trig,
>  	unsigned long int mask = 0;
>  
>  	if (state)
> -		mask = ADXL372_INT1_MAP_FIFO_FULL_MSK;
> +		mask = ADXL372_INT1_MAP_FIFO_FULL_MSK |
> +		       ADXL372_INT1_MAP_ACT_MSK	 |
> +		       ADXL372_INT1_MAP_INACT_MSK;
>  
>  	return adxl372_set_interrupts(st, mask, 0);
>  }

Re: [PATCH v2 2/6] iio: accel: adxl372: add sysfs for time registers

[Jonathan Cameron]

On Tue, 25 Feb 2020 14:09:05 +0200
Alexandru Tachici <alexandru.tachici@analog.com> wrote:

> Currently the driver configures adxl372 to work in loop mode.
> The inactivity and activity timings  decide how fast the chip
> will loop through the awake and waiting states.
> 
> This patch adds standard events sysfs entries for the inactivity
> and activity timings: thresh_falling_period/thresh_rising_period.
> 
> Signed-off-by: Alexandru Tachici <alexandru.tachici@analog.com>
I think this is a rare occasion where combining a couple of patches would
have made it easier to see the whole scope of the 'event' side of things.

Reality is that we need to configure enable / threshold  and period for
this to make any sense, so I'd put them all in one patch.

(noting that enable doesn't seem to exist currently..)

For the enable, we will definitely want to be able to turn these off.
Not all users are going to open the even interface as they may not care
what state we are in, just about the data that they get from the fifo.

Jonathan

> ---
>  drivers/iio/accel/adxl372.c | 68 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 68 insertions(+)
> 
> diff --git a/drivers/iio/accel/adxl372.c b/drivers/iio/accel/adxl372.c
> index ed93534f8dba..5da3c924c62d 100644
> --- a/drivers/iio/accel/adxl372.c
> +++ b/drivers/iio/accel/adxl372.c
> @@ -222,6 +222,18 @@ static const struct adxl372_axis_lookup adxl372_axis_lookup_table[] = {
>  	{ BIT(0) | BIT(1) | BIT(2), ADXL372_XYZ_FIFO },
>  };
>  
> +static const struct iio_event_spec adxl372_events[] = {
> +	{
> +		.type = IIO_EV_TYPE_THRESH,
> +		.dir = IIO_EV_DIR_RISING,
> +		.mask_shared_by_all = BIT(IIO_EV_INFO_PERIOD),
> +	}, {
> +		.type = IIO_EV_TYPE_THRESH,
> +		.dir = IIO_EV_DIR_FALLING,
> +		.mask_shared_by_all = BIT(IIO_EV_INFO_PERIOD),
> +	},
> +};
> +
>  #define ADXL372_ACCEL_CHANNEL(index, reg, axis) {			\
>  	.type = IIO_ACCEL,						\
>  	.address = reg,							\
> @@ -238,6 +250,8 @@ static const struct adxl372_axis_lookup adxl372_axis_lookup_table[] = {
>  		.storagebits = 16,					\
>  		.shift = 4,						\
>  	},								\
> +	.event_spec = adxl372_events,					\
> +	.num_event_specs = 2						\
>  }
>  
>  static const struct iio_chan_spec adxl372_channels[] = {
> @@ -723,6 +737,58 @@ static int adxl372_write_raw(struct iio_dev *indio_dev,
>  	}
>  }
>  
> +int adxl372_read_event_value(struct iio_dev *indio_dev,
> +			     const struct iio_chan_spec *chan,
> +			     enum iio_event_type type,
> +			     enum iio_event_direction dir,
> +			     enum iio_event_info info, int *val, int *val2)
> +{
> +	struct adxl372_state *st = iio_priv(indio_dev);
> +
> +	switch (info) {
> +	case IIO_EV_INFO_PERIOD:
> +		switch (dir) {
> +		case IIO_EV_DIR_RISING:
> +			*val = st->act_time_ms;
> +			*val2 = 1000;
> +			return IIO_VAL_FRACTIONAL;
> +		case IIO_EV_DIR_FALLING:
> +			*val = st->inact_time_ms;
> +			*val2 = 1000;
> +			return IIO_VAL_FRACTIONAL;
> +		default:
> +			return -EINVAL;
> +		}
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +int adxl372_write_event_value(struct iio_dev *indio_dev,
> +			      const struct iio_chan_spec *chan,
> +			      enum iio_event_type type,
> +			      enum iio_event_direction dir,
> +			      enum iio_event_info info, int val, int val2)
> +{
> +	struct adxl372_state *st = iio_priv(indio_dev);
> +	unsigned int val_ms;
> +
> +	switch (info) {
> +	case IIO_EV_INFO_PERIOD:
> +		val_ms = val * 1000 + DIV_ROUND_UP(val2, 1000);
> +		switch (dir) {
> +		case IIO_EV_DIR_RISING:
> +			return adxl372_set_activity_time_ms(st, val_ms);
> +		case IIO_EV_DIR_FALLING:
> +			return adxl372_set_inactivity_time_ms(st, val_ms);
> +		default:
> +			return -EINVAL;
> +		}
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
>  static ssize_t adxl372_peak_fifo_en_get(struct device *dev,
>  					struct device_attribute *attr,
>  					char *buf)
> @@ -952,6 +1018,8 @@ static const struct iio_info adxl372_info = {
>  	.attrs = &adxl372_attrs_group,
>  	.read_raw = adxl372_read_raw,
>  	.write_raw = adxl372_write_raw,
> +	.read_event_value = adxl372_read_event_value,
> +	.write_event_value = adxl372_write_event_value,
>  	.debugfs_reg_access = &adxl372_reg_access,
>  	.hwfifo_set_watermark = adxl372_set_watermark,
>  };