linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/5] iio: accel: adxl372: add peak mode
@ 2020-02-14  9:29 Alexandru Tachici
  2020-02-14  9:29 ` [PATCH 1/5] iio: accel: adxl372: Add support for FIFO " Alexandru Tachici
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Alexandru Tachici @ 2020-02-14  9:29 UTC (permalink / raw)
  To: linux-iio, linux-kernel; +Cc: jic23

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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/5] iio: accel: adxl372: Add support for FIFO peak mode
  2020-02-14  9:29 [PATCH 0/5] iio: accel: adxl372: add peak mode Alexandru Tachici
@ 2020-02-14  9:29 ` Alexandru Tachici
  2020-02-15 15:52   ` Jonathan Cameron
  2020-02-14  9:29 ` [PATCH 2/5] iio: accel: adxl372: Set iio_chan BE Alexandru Tachici
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 9+ messages in thread
From: Alexandru Tachici @ 2020-02-14  9:29 UTC (permalink / raw)
  To: linux-iio, linux-kernel; +Cc: jic23

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 | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/drivers/iio/accel/adxl372.c b/drivers/iio/accel/adxl372.c
index 67b8817995c0..bb6c2bf1a457 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,36 @@ 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 adxl372_state *st = iio_priv(dev_to_iio_dev(dev));
+	bool val;
+	int ret;
+
+	ret = kstrtobool(buf, &val);
+	if (ret)
+		return ret;
+
+	st->peak_fifo_mode_en = val;
+
+	return len;
+}
+
+static IIO_DEVICE_ATTR(peak_fifo_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 +848,16 @@ 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->fifo_set_size == 3 && st->peak_fifo_mode_en)
+		st->fifo_format = ADXL372_XYZ_PEAK_FIFO;
 	/*
 	 * 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 +930,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_peak_fifo_mode_enable.dev_attr.attr,
 	NULL,
 };
 
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/5] iio: accel: adxl372: Set iio_chan BE
  2020-02-14  9:29 [PATCH 0/5] iio: accel: adxl372: add peak mode Alexandru Tachici
  2020-02-14  9:29 ` [PATCH 1/5] iio: accel: adxl372: Add support for FIFO " Alexandru Tachici
@ 2020-02-14  9:29 ` Alexandru Tachici
  2020-02-15 15:36   ` Jonathan Cameron
  2020-02-14  9:29 ` [PATCH 3/5] iio: accel: adxl372: add sysfs for time registers Alexandru Tachici
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 9+ messages in thread
From: Alexandru Tachici @ 2020-02-14  9:29 UTC (permalink / raw)
  To: linux-iio, linux-kernel; +Cc: jic23

Data stored in the iio-buffer is BE and this
should be specified in the iio_chan_spec struct.

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

diff --git a/drivers/iio/accel/adxl372.c b/drivers/iio/accel/adxl372.c
index bb6c2bf1a457..538e5053a946 100644
--- a/drivers/iio/accel/adxl372.c
+++ b/drivers/iio/accel/adxl372.c
@@ -237,6 +237,7 @@ static const struct adxl372_axis_lookup adxl372_axis_lookup_table[] = {
 		.realbits = 12,						\
 		.storagebits = 16,					\
 		.shift = 4,						\
+		.endianness = IIO_BE,					\
 	},								\
 }
 
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 3/5] iio: accel: adxl372: add sysfs for time registers
  2020-02-14  9:29 [PATCH 0/5] iio: accel: adxl372: add peak mode Alexandru Tachici
  2020-02-14  9:29 ` [PATCH 1/5] iio: accel: adxl372: Add support for FIFO " Alexandru Tachici
  2020-02-14  9:29 ` [PATCH 2/5] iio: accel: adxl372: Set iio_chan BE Alexandru Tachici
@ 2020-02-14  9:29 ` Alexandru Tachici
  2020-02-14  9:31 ` [PATCH 4/5] iio: accel: adxl372: Add sysfs for g thresholds Alexandru Tachici
  2020-02-14  9:32 ` [PATCH 5/5] iio: accel: adxl372: Update sysfs docs Alexandru Tachici
  4 siblings, 0 replies; 9+ messages in thread
From: Alexandru Tachici @ 2020-02-14  9:29 UTC (permalink / raw)
  To: linux-iio, linux-kernel; +Cc: jic23

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 sysfs entries for the inactivity and activity
shared properties.

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

diff --git a/drivers/iio/accel/adxl372.c b/drivers/iio/accel/adxl372.c
index 538e5053a946..8bef6f2030ff 100644
--- a/drivers/iio/accel/adxl372.c
+++ b/drivers/iio/accel/adxl372.c
@@ -754,6 +754,70 @@ static IIO_DEVICE_ATTR(peak_fifo_mode_enable, 0644,
 		       adxl372_peak_fifo_en_get,
 		       adxl372_peak_fifo_en_set, 0);
 
+static ssize_t adxl372_time_activity_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->act_time_ms);
+}
+
+static ssize_t adxl372_time_activity_set(struct device *dev,
+					 struct device_attribute *attr,
+					 const char *buf, size_t len)
+{
+	struct adxl372_state *st = iio_priv(dev_to_iio_dev(dev));
+	unsigned int val;
+	int ret;
+
+	ret = kstrtouint(buf, 0, &val);
+	if (ret)
+		return ret;
+
+	ret = adxl372_set_activity_time_ms(st, val);
+	if (ret < 0)
+		return ret;
+
+	return len;
+}
+
+static IIO_DEVICE_ATTR(time_activity, 0644,
+		       adxl372_time_activity_get,
+		       adxl372_time_activity_set, 0);
+
+static ssize_t adxl372_time_inactivity_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->inact_time_ms);
+}
+
+static ssize_t adxl372_time_inactivity_set(struct device *dev,
+					   struct device_attribute *attr,
+					   const char *buf, size_t len)
+{
+	struct adxl372_state *st = iio_priv(dev_to_iio_dev(dev));
+	unsigned int val;
+	int ret;
+
+	ret = kstrtouint(buf, 0, &val);
+	if (ret)
+		return ret;
+
+	ret = adxl372_set_inactivity_time_ms(st, val);
+	if (ret)
+		return ret;
+
+	return len;
+}
+
+static IIO_DEVICE_ATTR(time_inactivity, 0644,
+		       adxl372_time_inactivity_get,
+		       adxl372_time_inactivity_set, 0);
+
 static ssize_t adxl372_show_filter_freq_avail(struct device *dev,
 					      struct device_attribute *attr,
 					      char *buf)
@@ -932,6 +996,8 @@ 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_peak_fifo_mode_enable.dev_attr.attr,
+	&iio_dev_attr_time_inactivity.dev_attr.attr,
+	&iio_dev_attr_time_activity.dev_attr.attr,
 	NULL,
 };
 
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 4/5] iio: accel: adxl372: Add sysfs for g thresholds
  2020-02-14  9:29 [PATCH 0/5] iio: accel: adxl372: add peak mode Alexandru Tachici
                   ` (2 preceding siblings ...)
  2020-02-14  9:29 ` [PATCH 3/5] iio: accel: adxl372: add sysfs for time registers Alexandru Tachici
@ 2020-02-14  9:31 ` Alexandru Tachici
  2020-02-14  9:32 ` [PATCH 5/5] iio: accel: adxl372: Update sysfs docs Alexandru Tachici
  4 siblings, 0 replies; 9+ messages in thread
From: Alexandru Tachici @ 2020-02-14  9:31 UTC (permalink / raw)
  To: linux-iio, linux-kernel; +Cc: jic23

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 <alexandru.tachici@analog.com>
---
 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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 5/5] iio: accel: adxl372: Update sysfs docs
  2020-02-14  9:29 [PATCH 0/5] iio: accel: adxl372: add peak mode Alexandru Tachici
                   ` (3 preceding siblings ...)
  2020-02-14  9:31 ` [PATCH 4/5] iio: accel: adxl372: Add sysfs for g thresholds Alexandru Tachici
@ 2020-02-14  9:32 ` Alexandru Tachici
  2020-02-15 16:09   ` Jonathan Cameron
  4 siblings, 1 reply; 9+ messages in thread
From: Alexandru Tachici @ 2020-02-14  9:32 UTC (permalink / raw)
  To: linux-iio, linux-kernel; +Cc: jic23

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   | 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@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/time_activity
+KernelVersion:
+Contact:	linux-iio@vger.kernel.org
+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:	linux-iio@vger.kernel.org
+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@vger.kernel.org
+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:	linux-iio@vger.kernel.org
+Description:
+		This attribute allows to set the inactivity threshold in 100 mg
+		(0.1 m/s^2 SI).
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/5] iio: accel: adxl372: Set iio_chan BE
  2020-02-14  9:29 ` [PATCH 2/5] iio: accel: adxl372: Set iio_chan BE Alexandru Tachici
@ 2020-02-15 15:36   ` Jonathan Cameron
  0 siblings, 0 replies; 9+ messages in thread
From: Jonathan Cameron @ 2020-02-15 15:36 UTC (permalink / raw)
  To: Alexandru Tachici; +Cc: linux-iio, linux-kernel

On Fri, 14 Feb 2020 11:29:16 +0200
Alexandru Tachici <alexandru.tachici@analog.com> wrote:

> Data stored in the iio-buffer is BE and this
> should be specified in the iio_chan_spec struct.
> 
> Signed-off-by: Alexandru Tachici <alexandru.tachici@analog.com>
To a quick glance this looks to me like a fix we should be backporting.
Please add a fixes tag if so and I'll get this lined up to go in during
the RCs.

Thanks,

Jonathan

> ---
>  drivers/iio/accel/adxl372.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/iio/accel/adxl372.c b/drivers/iio/accel/adxl372.c
> index bb6c2bf1a457..538e5053a946 100644
> --- a/drivers/iio/accel/adxl372.c
> +++ b/drivers/iio/accel/adxl372.c
> @@ -237,6 +237,7 @@ static const struct adxl372_axis_lookup adxl372_axis_lookup_table[] = {
>  		.realbits = 12,						\
>  		.storagebits = 16,					\
>  		.shift = 4,						\
> +		.endianness = IIO_BE,					\
>  	},								\
>  }
>  


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/5] iio: accel: adxl372: Add support for FIFO peak mode
  2020-02-14  9:29 ` [PATCH 1/5] iio: accel: adxl372: Add support for FIFO " Alexandru Tachici
@ 2020-02-15 15:52   ` Jonathan Cameron
  0 siblings, 0 replies; 9+ messages in thread
From: Jonathan Cameron @ 2020-02-15 15:52 UTC (permalink / raw)
  To: Alexandru Tachici; +Cc: linux-iio, linux-kernel

On Fri, 14 Feb 2020 11:29:15 +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>
I've left comments on the interface until the documentation patch.
A few other bits in here.

Thanks,

Jonathan

> ---
>  drivers/iio/accel/adxl372.c | 37 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 37 insertions(+)
> 
> diff --git a/drivers/iio/accel/adxl372.c b/drivers/iio/accel/adxl372.c
> index 67b8817995c0..bb6c2bf1a457 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,36 @@ 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 adxl372_state *st = iio_priv(dev_to_iio_dev(dev));
> +	bool val;
> +	int ret;
> +
> +	ret = kstrtobool(buf, &val);
> +	if (ret)
> +		return ret;
> +
> +	st->peak_fifo_mode_en = val;

Should reject the attempt if the buffer is already enabled.  Otherwise
the userspace interface will be rather confusing as you'll read back that
it is enabled, but not see it working.

> +
> +	return len;
> +}
> +
> +static IIO_DEVICE_ATTR(peak_fifo_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 +848,16 @@ 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->fifo_set_size == 3 && st->peak_fifo_mode_en)
> +		st->fifo_format = ADXL372_XYZ_PEAK_FIFO;

We could perhaps make this more intuitive by always enabling the 3 axis
if peak mode is on and filtering the data on it's way to the
push_to_buffer to reflect only channels enabled.

If not, perhaps a warning message?

>  	/*
>  	 * 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 +930,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_peak_fifo_mode_enable.dev_attr.attr,
>  	NULL,
>  };
>  


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 5/5] iio: accel: adxl372: Update sysfs docs
  2020-02-14  9:32 ` [PATCH 5/5] iio: accel: adxl372: Update sysfs docs Alexandru Tachici
@ 2020-02-15 16:09   ` Jonathan Cameron
  0 siblings, 0 replies; 9+ messages in thread
From: Jonathan Cameron @ 2020-02-15 16:09 UTC (permalink / raw)
  To: Alexandru Tachici; +Cc: linux-iio, linux-kernel

On Fri, 14 Feb 2020 11:32:23 +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   | 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@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.
> +
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@vger.kernel.org
> +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@vger.kernel.org
> +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@vger.kernel.org
> +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@vger.kernel.org
> +Description:
> +		This attribute allows to set the inactivity threshold in 100 mg
> +		(0.1 m/s^2 SI).


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2020-02-15 16:09 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-14  9:29 [PATCH 0/5] iio: accel: adxl372: add peak mode Alexandru Tachici
2020-02-14  9:29 ` [PATCH 1/5] iio: accel: adxl372: Add support for FIFO " Alexandru Tachici
2020-02-15 15:52   ` Jonathan Cameron
2020-02-14  9:29 ` [PATCH 2/5] iio: accel: adxl372: Set iio_chan BE Alexandru Tachici
2020-02-15 15:36   ` Jonathan Cameron
2020-02-14  9:29 ` [PATCH 3/5] iio: accel: adxl372: add sysfs for time registers Alexandru Tachici
2020-02-14  9:31 ` [PATCH 4/5] iio: accel: adxl372: Add sysfs for g thresholds Alexandru Tachici
2020-02-14  9:32 ` [PATCH 5/5] iio: accel: adxl372: Update sysfs docs Alexandru Tachici
2020-02-15 16:09   ` Jonathan Cameron

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).