[PATCH v2 00/14] iio: accel: adxl345: add interrupt based sensor events

[PATCH v2 00/14] iio: accel: adxl345: add interrupt based sensor events

[Lothar Rubusch]

Add several interrupt based sensor detection events:
- single tap
- double tap
- free fall
- activity
- inactivity
- sample frequency
- frequency g range approach
- documentation

All the needed parameters for each and methods of adjusting them, and
forward a resulting IIO event for each to the IIO channel.

Q: Please have a look into event handling, I'm not sure there seems
to be something open (if more simultaneous events arrive)?

Q: Please, let me know if the single ABI change is legitimate, where
I need to document it?

Q: Please, also have a focus on the scale factor handling, is it
correctly setup? (I think) I can see it appearing in iio_info.

Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
---
v1 -> v2:
- implementation of all events (but tap2 suppress bit) by means IIO ABI
- add sample frequency / ODR configuration
- add g ranges configuration
- add activity/inactivity using auto-sleep and powersave
- add dynamic adjustment of default values for
  activity/inactivity thresholds and time for inactivity based on ODR
  and g range (can be overwritten)
- add sensor documentation

Lothar Rubusch (14):
  iio: accel: adxl345: reorganize measurement enable
  iio: accel: adxl345: add debug register access
  iio: accel: adxl345: reorganize irq handler
  iio: accel: adxl345: refac set_interrupts and IRQ map
  iio: accel: adxl345: add single tap feature
  iio: accel: adxl345: add double tap feature
  iio: accel: adxl345: add double tap suppress bit
  iio: accel: adxl345: add freefall feature
  iio: accel: adxl345: extend sample frequency adjustments
  iio: accel: adxl345: add g-range configuration
  iio: accel: adxl345: add activity event feature
  iio: accel: adxl345: add inactivity feature
  iio: accel: adxl345: add coupling detection for activity/inactivity
  docs: iio: add documentation for adxl345 driver

 Documentation/iio/adxl345.rst    |  401 +++++++++
 drivers/iio/accel/adxl345.h      |    3 +-
 drivers/iio/accel/adxl345_core.c | 1378 ++++++++++++++++++++++++++++--
 3 files changed, 1703 insertions(+), 79 deletions(-)
 create mode 100644 Documentation/iio/adxl345.rst

-- 
2.39.5

[PATCH v2 01/14] iio: accel: adxl345: reorganize measurement enable

[Lothar Rubusch]

Move the measurement enable function up in order to have it generically
available.

This is a preparation for upcomming patches. Particular features need
to have measuring off while changing settings, and turned on again
afterwards.

Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
---
 drivers/iio/accel/adxl345_core.c | 40 ++++++++++++++++----------------
 1 file changed, 20 insertions(+), 20 deletions(-)

diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c
index ed0291bea0f5..c24692c51893 100644
--- a/drivers/iio/accel/adxl345_core.c
+++ b/drivers/iio/accel/adxl345_core.c
@@ -76,6 +76,26 @@ static const unsigned long adxl345_scan_masks[] = {
 	0
 };
 
+/**
+ * adxl345_set_measure_en() - Enable and disable measuring.
+ *
+ * @st: The device data.
+ * @en: Enable measurements, else standby mode.
+ *
+ * For lowest power operation, standby mode can be used. In standby mode,
+ * current consumption is supposed to be reduced to 0.1uA (typical). In this
+ * mode no measurements are made. Placing the device into standby mode
+ * preserves the contents of FIFO.
+ *
+ * Return: Returns 0 if successful, or a negative error value.
+ */
+static int adxl345_set_measure_en(struct adxl345_state *st, bool en)
+{
+	unsigned int val = en ? ADXL345_POWER_CTL_MEASURE : ADXL345_POWER_CTL_STANDBY;
+
+	return regmap_write(st->regmap, ADXL345_REG_POWER_CTL, val);
+}
+
 static int adxl345_set_interrupts(struct adxl345_state *st)
 {
 	int ret;
@@ -214,26 +234,6 @@ static int adxl345_write_raw_get_fmt(struct iio_dev *indio_dev,
 	}
 }
 
-/**
- * adxl345_set_measure_en() - Enable and disable measuring.
- *
- * @st: The device data.
- * @en: Enable measurements, else standby mode.
- *
- * For lowest power operation, standby mode can be used. In standby mode,
- * current consumption is supposed to be reduced to 0.1uA (typical). In this
- * mode no measurements are made. Placing the device into standby mode
- * preserves the contents of FIFO.
- *
- * Return: Returns 0 if successful, or a negative error value.
- */
-static int adxl345_set_measure_en(struct adxl345_state *st, bool en)
-{
-	unsigned int val = en ? ADXL345_POWER_CTL_MEASURE : ADXL345_POWER_CTL_STANDBY;
-
-	return regmap_write(st->regmap, ADXL345_REG_POWER_CTL, val);
-}
-
 static void adxl345_powerdown(void *ptr)
 {
 	struct adxl345_state *st = ptr;
-- 
2.39.5

[PATCH v2 02/14] iio: accel: adxl345: add debug register access

[Lothar Rubusch]

Add the possibility to verify the content of the configuration
registers of the sensor in preparation for upcomming feature
implementations.

Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
---
 drivers/iio/accel/adxl345_core.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c
index c24692c51893..468d562de227 100644
--- a/drivers/iio/accel/adxl345_core.c
+++ b/drivers/iio/accel/adxl345_core.c
@@ -202,6 +202,16 @@ static int adxl345_write_raw(struct iio_dev *indio_dev,
 	return -EINVAL;
 }
 
+static int adxl345_reg_access(struct iio_dev *indio_dev, unsigned int reg,
+			      unsigned int writeval, unsigned int *readval)
+{
+	struct adxl345_state *st = iio_priv(indio_dev);
+
+	if (readval)
+		return regmap_read(st->regmap, reg, readval);
+	return regmap_write(st->regmap, reg, writeval);
+}
+
 static int adxl345_set_watermark(struct iio_dev *indio_dev, unsigned int value)
 {
 	struct adxl345_state *st = iio_priv(indio_dev);
@@ -467,6 +477,7 @@ static const struct iio_info adxl345_info = {
 	.read_raw	= adxl345_read_raw,
 	.write_raw	= adxl345_write_raw,
 	.write_raw_get_fmt	= adxl345_write_raw_get_fmt,
+	.debugfs_reg_access = &adxl345_reg_access,
 	.hwfifo_set_watermark = adxl345_set_watermark,
 };
 
-- 
2.39.5

[PATCH v2 03/14] iio: accel: adxl345: reorganize irq handler

[Lothar Rubusch]

Reorganize the IRQ handler. Move the overrun handling to the bottom.
Overrun leads to reset the interrupt register. This also happens at
evaluation of a particular interrupt event. First evaluate an event
if possible, then fall back to overrun handling. Additionally simplify
fetching the interrupt status function.

Both is in preparation to build interrupt handling up for the handling
of different detected events, implemented in follow up patches.

Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
---
 drivers/iio/accel/adxl345.h      |  1 -
 drivers/iio/accel/adxl345_core.c | 26 +++++++++-----------------
 2 files changed, 9 insertions(+), 18 deletions(-)

diff --git a/drivers/iio/accel/adxl345.h b/drivers/iio/accel/adxl345.h
index 517e494ba555..bc6d634bd85c 100644
--- a/drivers/iio/accel/adxl345.h
+++ b/drivers/iio/accel/adxl345.h
@@ -43,7 +43,6 @@
 #define ADXL345_REG_INT_ENABLE		0x2E
 #define ADXL345_REG_INT_MAP		0x2F
 #define ADXL345_REG_INT_SOURCE		0x30
-#define ADXL345_REG_INT_SOURCE_MSK	0xFF
 #define ADXL345_REG_DATA_FORMAT		0x31
 #define ADXL345_REG_XYZ_BASE		0x32
 #define ADXL345_REG_DATA_AXIS(index)				\
diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c
index 468d562de227..2928c1c0760f 100644
--- a/drivers/iio/accel/adxl345_core.c
+++ b/drivers/iio/accel/adxl345_core.c
@@ -107,8 +107,7 @@ static int adxl345_set_interrupts(struct adxl345_state *st)
 	 * interrupts to the INT1 pin, whereas bits set to 1 send their respective
 	 * interrupts to the INT2 pin. The intio shall convert this accordingly.
 	 */
-	int_map = FIELD_GET(ADXL345_REG_INT_SOURCE_MSK,
-			    st->intio ? st->int_map : ~st->int_map);
+	int_map = st->intio ? st->int_map : ~st->int_map;
 
 	ret = regmap_write(st->regmap, ADXL345_REG_INT_MAP, int_map);
 	if (ret)
@@ -404,16 +403,9 @@ static const struct iio_buffer_setup_ops adxl345_buffer_ops = {
 	.predisable = adxl345_buffer_predisable,
 };
 
-static int adxl345_get_status(struct adxl345_state *st)
+static int adxl345_get_status(struct adxl345_state *st, unsigned int *int_stat)
 {
-	int ret;
-	unsigned int regval;
-
-	ret = regmap_read(st->regmap, ADXL345_REG_INT_SOURCE, &regval);
-	if (ret < 0)
-		return ret;
-
-	return FIELD_GET(ADXL345_REG_INT_SOURCE_MSK, regval);
+	return regmap_read(st->regmap, ADXL345_REG_INT_SOURCE, int_stat);
 }
 
 static int adxl345_fifo_push(struct iio_dev *indio_dev,
@@ -449,14 +441,10 @@ static irqreturn_t adxl345_irq_handler(int irq, void *p)
 	int int_stat;
 	int samples;
 
-	int_stat = adxl345_get_status(st);
-	if (int_stat <= 0)
+	if (adxl345_get_status(st, &int_stat))
 		return IRQ_NONE;
 
-	if (int_stat & ADXL345_INT_OVERRUN)
-		goto err;
-
-	if (int_stat & ADXL345_INT_WATERMARK) {
+	if (FIELD_GET(ADXL345_INT_WATERMARK, int_stat)) {
 		samples = adxl345_get_samples(st);
 		if (samples < 0)
 			goto err;
@@ -464,6 +452,10 @@ static irqreturn_t adxl345_irq_handler(int irq, void *p)
 		if (adxl345_fifo_push(indio_dev, samples) < 0)
 			goto err;
 	}
+
+	if (FIELD_GET(ADXL345_INT_OVERRUN, int_stat))
+		goto err;
+
 	return IRQ_HANDLED;
 
 err:
-- 
2.39.5

Re: [PATCH v2 03/14] iio: accel: adxl345: reorganize irq handler

[Jonathan Cameron]

On Mon, 10 Feb 2025 11:01:08 +0000
Lothar Rubusch <l.rubusch@gmail.com> wrote:

> Reorganize the IRQ handler. Move the overrun handling to the bottom.
> Overrun leads to reset the interrupt register. This also happens at
> evaluation of a particular interrupt event. First evaluate an event
> if possible, then fall back to overrun handling. Additionally simplify
> fetching the interrupt status function.
> 
> Both is in preparation to build interrupt handling up for the handling
> of different detected events, implemented in follow up patches.
> 
> Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
Hi Lothar,

Changes are fine, but I think we should reevaluate if the little
helper to get_status is actually useful any more.  Probably best
to just drop it as part of this patch.

Jonathan

> ---
>  drivers/iio/accel/adxl345.h      |  1 -
>  drivers/iio/accel/adxl345_core.c | 26 +++++++++-----------------
>  2 files changed, 9 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/iio/accel/adxl345.h b/drivers/iio/accel/adxl345.h
> index 517e494ba555..bc6d634bd85c 100644
> --- a/drivers/iio/accel/adxl345.h
> +++ b/drivers/iio/accel/adxl345.h
> @@ -43,7 +43,6 @@
>  #define ADXL345_REG_INT_ENABLE		0x2E
>  #define ADXL345_REG_INT_MAP		0x2F
>  #define ADXL345_REG_INT_SOURCE		0x30
> -#define ADXL345_REG_INT_SOURCE_MSK	0xFF
>  #define ADXL345_REG_DATA_FORMAT		0x31
>  #define ADXL345_REG_XYZ_BASE		0x32
>  #define ADXL345_REG_DATA_AXIS(index)				\
> diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c
> index 468d562de227..2928c1c0760f 100644
> --- a/drivers/iio/accel/adxl345_core.c
> +++ b/drivers/iio/accel/adxl345_core.c
> @@ -107,8 +107,7 @@ static int adxl345_set_interrupts(struct adxl345_state *st)
>  	 * interrupts to the INT1 pin, whereas bits set to 1 send their respective
>  	 * interrupts to the INT2 pin. The intio shall convert this accordingly.
>  	 */
> -	int_map = FIELD_GET(ADXL345_REG_INT_SOURCE_MSK,
> -			    st->intio ? st->int_map : ~st->int_map);
> +	int_map = st->intio ? st->int_map : ~st->int_map;
>  
>  	ret = regmap_write(st->regmap, ADXL345_REG_INT_MAP, int_map);
>  	if (ret)
> @@ -404,16 +403,9 @@ static const struct iio_buffer_setup_ops adxl345_buffer_ops = {
>  	.predisable = adxl345_buffer_predisable,
>  };
>  
> -static int adxl345_get_status(struct adxl345_state *st)
> +static int adxl345_get_status(struct adxl345_state *st, unsigned int *int_stat)
>  {
> -	int ret;
> -	unsigned int regval;
> -
> -	ret = regmap_read(st->regmap, ADXL345_REG_INT_SOURCE, &regval);
> -	if (ret < 0)
> -		return ret;
> -
> -	return FIELD_GET(ADXL345_REG_INT_SOURCE_MSK, regval);
> +	return regmap_read(st->regmap, ADXL345_REG_INT_SOURCE, int_stat);

After this cleanup/refactoring this function no longer has a reason to exist.
Just call regmap_read() directly inline.

>  }
>  
>  static int adxl345_fifo_push(struct iio_dev *indio_dev,
> @@ -449,14 +441,10 @@ static irqreturn_t adxl345_irq_handler(int irq, void *p)
>  	int int_stat;
>  	int samples;
>  
> -	int_stat = adxl345_get_status(st);
> -	if (int_stat <= 0)
> +	if (adxl345_get_status(st, &int_stat))

	if (regmap_read(st->regmap, ADXL345_REG_INT_SOURCE, &int_stat))

>  		return IRQ_NONE;
>  
> -	if (int_stat & ADXL345_INT_OVERRUN)
> -		goto err;
> -
> -	if (int_stat & ADXL345_INT_WATERMARK) {
> +	if (FIELD_GET(ADXL345_INT_WATERMARK, int_stat)) {
>  		samples = adxl345_get_samples(st);
>  		if (samples < 0)
>  			goto err;
> @@ -464,6 +452,10 @@ static irqreturn_t adxl345_irq_handler(int irq, void *p)
>  		if (adxl345_fifo_push(indio_dev, samples) < 0)
>  			goto err;
>  	}
> +
> +	if (FIELD_GET(ADXL345_INT_OVERRUN, int_stat))
> +		goto err;
> +
>  	return IRQ_HANDLED;
>  
>  err:

[PATCH v2 04/14] iio: accel: adxl345: refac set_interrupts and IRQ map

[Lothar Rubusch]

Split the current set_interrupts() functionality. Separate writing the
interrupt map from writing the interrupt enable register.

Move writing the interrupt map into the probe(). The interrupt map will
setup which event finally will go over the INT line. Thus, all events
are mapped to this interrupt line now once at the beginning.

On the other side the function set_interrupts() will now be focussed on
enabling interrupts for event features. Thus it will be renamed to
write_interrupts() to better distinguish from further usage of get/set
in the conext of the sensor features.

Also, add the missing initial reset of the interrupt enable register.

Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
---
 drivers/iio/accel/adxl345_core.c | 46 +++++++++++++++-----------------
 1 file changed, 22 insertions(+), 24 deletions(-)

diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c
index 2928c1c0760f..910ad21279ed 100644
--- a/drivers/iio/accel/adxl345_core.c
+++ b/drivers/iio/accel/adxl345_core.c
@@ -14,10 +14,10 @@
 #include <linux/regmap.h>
 #include <linux/units.h>
 
-#include <linux/iio/iio.h>
-#include <linux/iio/sysfs.h>
 #include <linux/iio/buffer.h>
+#include <linux/iio/iio.h>
 #include <linux/iio/kfifo_buf.h>
+#include <linux/iio/sysfs.h>
 
 #include "adxl345.h"
 
@@ -96,26 +96,6 @@ static int adxl345_set_measure_en(struct adxl345_state *st, bool en)
 	return regmap_write(st->regmap, ADXL345_REG_POWER_CTL, val);
 }
 
-static int adxl345_set_interrupts(struct adxl345_state *st)
-{
-	int ret;
-	unsigned int int_enable = st->int_map;
-	unsigned int int_map;
-
-	/*
-	 * Any bits set to 0 in the INT map register send their respective
-	 * interrupts to the INT1 pin, whereas bits set to 1 send their respective
-	 * interrupts to the INT2 pin. The intio shall convert this accordingly.
-	 */
-	int_map = st->intio ? st->int_map : ~st->int_map;
-
-	ret = regmap_write(st->regmap, ADXL345_REG_INT_MAP, int_map);
-	if (ret)
-		return ret;
-
-	return regmap_write(st->regmap, ADXL345_REG_INT_ENABLE, int_enable);
-}
-
 static int adxl345_read_raw(struct iio_dev *indio_dev,
 			    struct iio_chan_spec const *chan,
 			    int *val, int *val2, long mask)
@@ -376,7 +356,7 @@ static int adxl345_buffer_postenable(struct iio_dev *indio_dev)
 	struct adxl345_state *st = iio_priv(indio_dev);
 	int ret;
 
-	ret = adxl345_set_interrupts(st);
+	ret = regmap_write(st->regmap, ADXL345_REG_INT_ENABLE, st->int_map);
 	if (ret < 0)
 		return ret;
 
@@ -395,7 +375,7 @@ static int adxl345_buffer_predisable(struct iio_dev *indio_dev)
 		return ret;
 
 	st->int_map = 0x00;
-	return adxl345_set_interrupts(st);
+	return regmap_write(st->regmap, ADXL345_REG_INT_ENABLE, st->int_map);
 }
 
 static const struct iio_buffer_setup_ops adxl345_buffer_ops = {
@@ -514,6 +494,8 @@ int adxl345_core_probe(struct device *dev, struct regmap *regmap,
 		return -ENODEV;
 	st->fifo_delay = fifo_delay_default;
 
+	st->int_map = 0x00;			/* reset interrupts */
+
 	indio_dev->name = st->info->name;
 	indio_dev->info = &adxl345_info;
 	indio_dev->modes = INDIO_DIRECT_MODE;
@@ -521,6 +503,11 @@ int adxl345_core_probe(struct device *dev, struct regmap *regmap,
 	indio_dev->num_channels = ARRAY_SIZE(adxl345_channels);
 	indio_dev->available_scan_masks = adxl345_scan_masks;
 
+	/* Reset interrupts at start up */
+	ret = regmap_write(st->regmap, ADXL345_REG_INT_ENABLE, st->int_map);
+	if (ret)
+		return ret;
+
 	if (setup) {
 		/* Perform optional initial bus specific configuration */
 		ret = setup(dev, st->regmap);
@@ -571,6 +558,17 @@ int adxl345_core_probe(struct device *dev, struct regmap *regmap,
 	}
 
 	if (st->intio != ADXL345_INT_NONE) {
+		/*
+		 * Any bits set to 0 in the INT map register send their respective
+		 * interrupts to the INT1 pin, whereas bits set to 1 send their respective
+		 * interrupts to the INT2 pin. The intio shall convert this accordingly.
+		 */
+		regval = st->intio ? 0xff : 0;
+
+		ret = regmap_write(st->regmap, ADXL345_REG_INT_MAP, regval);
+		if (ret)
+			return ret;
+
 		/* FIFO_STREAM mode is going to be activated later */
 		ret = devm_iio_kfifo_buffer_setup(dev, indio_dev, &adxl345_buffer_ops);
 		if (ret)
-- 
2.39.5

Re: [PATCH v2 04/14] iio: accel: adxl345: refac set_interrupts and IRQ map

[Jonathan Cameron]

On Mon, 10 Feb 2025 11:01:09 +0000
Lothar Rubusch <l.rubusch@gmail.com> wrote:

> Split the current set_interrupts() functionality. Separate writing the
> interrupt map from writing the interrupt enable register.
> 
> Move writing the interrupt map into the probe(). The interrupt map will
> setup which event finally will go over the INT line. Thus, all events
> are mapped to this interrupt line now once at the beginning.
> 
> On the other side the function set_interrupts() will now be focussed on
> enabling interrupts for event features. Thus it will be renamed to
> write_interrupts() to better distinguish from further usage of get/set
> in the conext of the sensor features.
> 
> Also, add the missing initial reset of the interrupt enable register.
> 
> Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>

After this cleanup (which is good) I wonder why we need to store
intmap at all?  Can we not just get it back from the regmap cache
in the few places we need it (mostly as RMW updates I think).

Jonathan

> ---
>  drivers/iio/accel/adxl345_core.c | 46 +++++++++++++++-----------------
>  1 file changed, 22 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c
> index 2928c1c0760f..910ad21279ed 100644
> --- a/drivers/iio/accel/adxl345_core.c
> +++ b/drivers/iio/accel/adxl345_core.c
> @@ -14,10 +14,10 @@
>  #include <linux/regmap.h>
>  #include <linux/units.h>
>  
> -#include <linux/iio/iio.h>
> -#include <linux/iio/sysfs.h>
>  #include <linux/iio/buffer.h>
> +#include <linux/iio/iio.h>
>  #include <linux/iio/kfifo_buf.h>
> +#include <linux/iio/sysfs.h>

Unrelated change.  Should not be in this patch

>  
>  #include "adxl345.h"
>  
> @@ -96,26 +96,6 @@ static int adxl345_set_measure_en(struct adxl345_state *st, bool en)
>  	return regmap_write(st->regmap, ADXL345_REG_POWER_CTL, val);
>  }
>  
> -static int adxl345_set_interrupts(struct adxl345_state *st)
> -{
> -	int ret;
> -	unsigned int int_enable = st->int_map;
> -	unsigned int int_map;
> -
> -	/*
> -	 * Any bits set to 0 in the INT map register send their respective
> -	 * interrupts to the INT1 pin, whereas bits set to 1 send their respective
> -	 * interrupts to the INT2 pin. The intio shall convert this accordingly.
> -	 */
> -	int_map = st->intio ? st->int_map : ~st->int_map;
> -
> -	ret = regmap_write(st->regmap, ADXL345_REG_INT_MAP, int_map);
> -	if (ret)
> -		return ret;
> -
> -	return regmap_write(st->regmap, ADXL345_REG_INT_ENABLE, int_enable);
> -}
> -
>  static int adxl345_read_raw(struct iio_dev *indio_dev,
>  			    struct iio_chan_spec const *chan,
>  			    int *val, int *val2, long mask)
> @@ -376,7 +356,7 @@ static int adxl345_buffer_postenable(struct iio_dev *indio_dev)
>  	struct adxl345_state *st = iio_priv(indio_dev);
>  	int ret;
>  
> -	ret = adxl345_set_interrupts(st);
> +	ret = regmap_write(st->regmap, ADXL345_REG_INT_ENABLE, st->int_map);
>  	if (ret < 0)
>  		return ret;
>  
> @@ -395,7 +375,7 @@ static int adxl345_buffer_predisable(struct iio_dev *indio_dev)
>  		return ret;
>  
>  	st->int_map = 0x00;

Maybe you deal with this in a later patch, but in general, why do we need a cached value?
Maybe we should rely on regmap caching to get it back from the register cheaply wherever
it is needed.


> -	return adxl345_set_interrupts(st);
> +	return regmap_write(st->regmap, ADXL345_REG_INT_ENABLE, st->int_map);
>  }
>  
>  static const struct iio_buffer_setup_ops adxl345_buffer_ops = {
> @@ -514,6 +494,8 @@ int adxl345_core_probe(struct device *dev, struct regmap *regmap,
>  		return -ENODEV;
>  	st->fifo_delay = fifo_delay_default;
>  
> +	st->int_map = 0x00;			/* reset interrupts */
> +
>  	indio_dev->name = st->info->name;
>  	indio_dev->info = &adxl345_info;
>  	indio_dev->modes = INDIO_DIRECT_MODE;
> @@ -521,6 +503,11 @@ int adxl345_core_probe(struct device *dev, struct regmap *regmap,
>  	indio_dev->num_channels = ARRAY_SIZE(adxl345_channels);
>  	indio_dev->available_scan_masks = adxl345_scan_masks;
>  
> +	/* Reset interrupts at start up */
> +	ret = regmap_write(st->regmap, ADXL345_REG_INT_ENABLE, st->int_map);
> +	if (ret)
> +		return ret;
> +
>  	if (setup) {
>  		/* Perform optional initial bus specific configuration */
>  		ret = setup(dev, st->regmap);
> @@ -571,6 +558,17 @@ int adxl345_core_probe(struct device *dev, struct regmap *regmap,
>  	}
>  
>  	if (st->intio != ADXL345_INT_NONE) {
> +		/*
> +		 * Any bits set to 0 in the INT map register send their respective
> +		 * interrupts to the INT1 pin, whereas bits set to 1 send their respective
> +		 * interrupts to the INT2 pin. The intio shall convert this accordingly.
> +		 */
> +		regval = st->intio ? 0xff : 0;
> +
> +		ret = regmap_write(st->regmap, ADXL345_REG_INT_MAP, regval);
> +		if (ret)
> +			return ret;
> +
>  		/* FIFO_STREAM mode is going to be activated later */
>  		ret = devm_iio_kfifo_buffer_setup(dev, indio_dev, &adxl345_buffer_ops);
>  		if (ret)

[PATCH v2 05/14] iio: accel: adxl345: add single tap feature

[Lothar Rubusch]

Add the single tap feature with a threshold in 62.5mg/LSB points and a
scaled duration in us. Keep singletap threshold and duration using means
of IIO ABI. Extend the channels for single enable x/y/z axis of the
feature but also initialize threshold and duration with reasonable
content. When an interrupt is caught it will be pushed to the according
IIO channel.

Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
---
 drivers/iio/accel/adxl345_core.c | 369 ++++++++++++++++++++++++++++++-
 1 file changed, 367 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c
index 910ad21279ed..304147a4ca60 100644
--- a/drivers/iio/accel/adxl345_core.c
+++ b/drivers/iio/accel/adxl345_core.c
@@ -8,6 +8,7 @@
  */
 
 #include <linux/bitfield.h>
+#include <linux/bitops.h>
 #include <linux/interrupt.h>
 #include <linux/module.h>
 #include <linux/property.h>
@@ -15,6 +16,7 @@
 #include <linux/units.h>
 
 #include <linux/iio/buffer.h>
+#include <linux/iio/events.h>
 #include <linux/iio/iio.h>
 #include <linux/iio/kfifo_buf.h>
 #include <linux/iio/sysfs.h>
@@ -31,6 +33,33 @@
 #define ADXL345_INT1			0
 #define ADXL345_INT2			1
 
+#define ADXL345_REG_TAP_AXIS_MSK	GENMASK(2, 0)
+
+enum adxl345_axis {
+	ADXL345_Z_EN = BIT(0),
+	ADXL345_Y_EN = BIT(1),
+	ADXL345_X_EN = BIT(2),
+	/* Suppress double tap detection if value > tap threshold */
+	ADXL345_TAP_SUPPRESS = BIT(3),
+};
+
+/* single/double tap */
+enum adxl345_tap_type {
+	ADXL345_SINGLE_TAP,
+};
+
+static const unsigned int adxl345_tap_int_reg[2] = {
+	[ADXL345_SINGLE_TAP] = ADXL345_INT_SINGLE_TAP,
+};
+
+enum adxl345_tap_time_type {
+	ADXL345_TAP_TIME_DUR,
+};
+
+static const unsigned int adxl345_tap_time_reg[3] = {
+	[ADXL345_TAP_TIME_DUR] = ADXL345_REG_DUR,
+};
+
 struct adxl345_state {
 	const struct adxl345_chip_info *info;
 	struct regmap *regmap;
@@ -40,9 +69,25 @@ struct adxl345_state {
 	u8 int_map;
 	u8 watermark;
 	u8 fifo_mode;
+
+	u32 tap_axis_ctrl;
+	u8 tap_threshold;
+	u32 tap_duration_us;
+
 	__le16 fifo_buf[ADXL345_DIRS * ADXL345_FIFO_SIZE + 1] __aligned(IIO_DMA_MINALIGN);
 };
 
+static struct iio_event_spec adxl345_events[] = {
+	{
+		/* single tap */
+		.type = IIO_EV_TYPE_GESTURE,
+		.dir = IIO_EV_DIR_SINGLETAP,
+		.mask_separate = BIT(IIO_EV_INFO_ENABLE),
+		.mask_shared_by_type = BIT(IIO_EV_INFO_VALUE) |
+			BIT(IIO_EV_INFO_TIMEOUT),
+	},
+};
+
 #define ADXL345_CHANNEL(index, reg, axis) {					\
 	.type = IIO_ACCEL,						\
 	.modified = 1,							\
@@ -59,6 +104,8 @@ struct adxl345_state {
 		.storagebits = 16,			\
 		.endianness = IIO_LE,			\
 	},						\
+	.event_spec = adxl345_events,			\
+	.num_event_specs = ARRAY_SIZE(adxl345_events),	\
 }
 
 enum adxl345_chans {
@@ -96,6 +143,126 @@ static int adxl345_set_measure_en(struct adxl345_state *st, bool en)
 	return regmap_write(st->regmap, ADXL345_REG_POWER_CTL, val);
 }
 
+/* tap */
+
+static int adxl345_write_tap_axis(struct adxl345_state *st,
+				  enum adxl345_axis axis, bool en)
+{
+	st->tap_axis_ctrl = FIELD_GET(ADXL345_REG_TAP_AXIS_MSK,
+				      en ? st->tap_axis_ctrl | axis
+				      : st->tap_axis_ctrl & ~axis);
+
+	return regmap_update_bits(st->regmap, ADXL345_REG_TAP_AXIS,
+				  ADXL345_REG_TAP_AXIS_MSK,
+				  FIELD_PREP(ADXL345_REG_TAP_AXIS_MSK,
+					     st->tap_axis_ctrl));
+}
+
+static int _adxl345_set_tap_int(struct adxl345_state *st,
+				enum adxl345_tap_type type, bool state)
+{
+	bool axis_valid;
+	bool singletap_args_valid = false;
+	bool en = false;
+
+	axis_valid = FIELD_GET(ADXL345_REG_TAP_AXIS_MSK, st->tap_axis_ctrl) > 0;
+
+	/*
+	 * Note: A value of 0 for threshold and/or dur may result in undesirable
+	 *	 behavior if single tap/double tap interrupts are enabled.
+	 */
+	singletap_args_valid = st->tap_threshold > 0 && st->tap_duration_us > 0;
+
+	if (type == ADXL345_SINGLE_TAP)
+		en = axis_valid && singletap_args_valid;
+
+	if (state && en)
+		__set_bit(ilog2(adxl345_tap_int_reg[type]),
+			  (unsigned long *)&st->int_map);
+	else
+		__clear_bit(ilog2(adxl345_tap_int_reg[type]),
+			    (unsigned long *)&st->int_map);
+
+	return regmap_write(st->regmap, ADXL345_REG_INT_ENABLE, st->int_map);
+}
+
+static int adxl345_is_tap_en(struct adxl345_state *st,
+			     enum adxl345_tap_type type, bool *en)
+{
+	int ret;
+	unsigned int regval;
+
+	ret = regmap_read(st->regmap, ADXL345_REG_INT_ENABLE, &regval);
+	if (ret)
+		return ret;
+
+	*en = (adxl345_tap_int_reg[type] & regval) > 0;
+
+	return 0;
+}
+
+static int adxl345_set_singletap_en(struct adxl345_state *st,
+				    enum adxl345_axis axis, bool en)
+{
+	int ret;
+
+	ret = adxl345_write_tap_axis(st, axis, en);
+	if (ret)
+		return ret;
+
+	return _adxl345_set_tap_int(st, ADXL345_SINGLE_TAP, en);
+}
+
+static int adxl345_set_tap_threshold(struct adxl345_state *st, u8 val)
+{
+	int ret;
+
+	ret = regmap_write(st->regmap, ADXL345_REG_THRESH_TAP, min(val, 0xFF));
+	if (ret)
+		return ret;
+
+	st->tap_threshold = val;
+
+	return 0;
+}
+
+static int _adxl345_set_tap_time(struct adxl345_state *st,
+				 enum adxl345_tap_time_type type, u32 val_us)
+{
+	unsigned int regval;
+
+	switch (type) {
+	case ADXL345_TAP_TIME_DUR:
+		st->tap_duration_us = val_us;
+		break;
+	}
+
+	/*
+	 * The scale factor is 1250us / LSB for tap_window_us and tap_latent_us.
+	 * For tap_duration_us the scale factor is 625us / LSB.
+	 */
+	if (type == ADXL345_TAP_TIME_DUR)
+		regval = DIV_ROUND_CLOSEST(val_us, 625);
+	else
+		return 0;
+
+	return regmap_write(st->regmap, adxl345_tap_time_reg[type], regval);
+}
+
+static int adxl345_set_tap_duration(struct adxl345_state *st, u32 val_int,
+				    u32 val_fract_us)
+{
+	/*
+	 * Max value is 255 * 625 us = 0.159375 seconds
+	 *
+	 * Note: the scaling is similar to the scaling in the ADXL380
+	 */
+	if (val_int || val_fract_us > 159375)
+		return -EINVAL;
+
+	return _adxl345_set_tap_time(st, ADXL345_TAP_TIME_DUR, val_fract_us);
+}
+
 static int adxl345_read_raw(struct iio_dev *indio_dev,
 			    struct iio_chan_spec const *chan,
 			    int *val, int *val2, long mask)
@@ -181,6 +348,147 @@ static int adxl345_write_raw(struct iio_dev *indio_dev,
 	return -EINVAL;
 }
 
+static int adxl345_read_event_config(struct iio_dev *indio_dev,
+				     const struct iio_chan_spec *chan,
+				     enum iio_event_type type,
+				     enum iio_event_direction dir)
+{
+	struct adxl345_state *st = iio_priv(indio_dev);
+	bool int_en;
+	bool axis_en;
+	int ret = -EFAULT;
+
+	switch (type) {
+	case IIO_EV_TYPE_GESTURE:
+		switch (chan->channel2) {
+		case IIO_MOD_X:
+			axis_en = FIELD_GET(ADXL345_X_EN, st->tap_axis_ctrl);
+			break;
+		case IIO_MOD_Y:
+			axis_en = FIELD_GET(ADXL345_Y_EN, st->tap_axis_ctrl);
+			break;
+		case IIO_MOD_Z:
+			axis_en = FIELD_GET(ADXL345_Z_EN, st->tap_axis_ctrl);
+			break;
+		default:
+			axis_en = ADXL345_TAP_SUPPRESS;
+		}
+
+		switch (dir) {
+		case IIO_EV_DIR_SINGLETAP:
+			ret = adxl345_is_tap_en(st, ADXL345_SINGLE_TAP, &int_en);
+			if (ret)
+				return ret;
+			return int_en && axis_en;
+		default:
+			return -EINVAL;
+		}
+	default:
+		return -EINVAL;
+	}
+}
+
+static int adxl345_write_event_config(struct iio_dev *indio_dev,
+				      const struct iio_chan_spec *chan,
+				      enum iio_event_type type,
+				      enum iio_event_direction dir,
+				      int state)
+{
+	struct adxl345_state *st = iio_priv(indio_dev);
+	enum adxl345_axis axis;
+
+	switch (type) {
+	case IIO_EV_TYPE_GESTURE:
+		switch (chan->channel2) {
+		case IIO_MOD_X:
+			axis = ADXL345_X_EN;
+			break;
+		case IIO_MOD_Y:
+			axis = ADXL345_Y_EN;
+			break;
+		case IIO_MOD_Z:
+			axis = ADXL345_Z_EN;
+			break;
+		default:
+			axis = ADXL345_TAP_SUPPRESS;
+		}
+
+		switch (dir) {
+		case IIO_EV_DIR_SINGLETAP:
+			return adxl345_set_singletap_en(st, axis, state);
+		default:
+			return -EINVAL;
+		}
+	default:
+		return -EINVAL;
+	}
+}
+
+static int adxl345_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 adxl345_state *st = iio_priv(indio_dev);
+
+	switch (type) {
+	case IIO_EV_TYPE_GESTURE:
+		switch (info) {
+		case IIO_EV_INFO_VALUE:
+			/*
+			 * The scale factor is 62.5mg/LSB (i.e. 0xFF = 16g) but
+			 * not applied here.
+			 */
+			*val = sign_extend32(st->tap_threshold, 7);
+			return IIO_VAL_INT;
+		case IIO_EV_INFO_TIMEOUT:
+			*val = st->tap_duration_us;
+			*val2 = 1000000;
+			return IIO_VAL_FRACTIONAL;
+		default:
+			return -EINVAL;
+		}
+	default:
+		return -EINVAL;
+	}
+}
+
+static int adxl345_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 adxl345_state *st = iio_priv(indio_dev);
+	int ret;
+
+	ret = adxl345_set_measure_en(st, false);
+	if (ret)
+		return ret;
+
+	switch (type) {
+	case IIO_EV_TYPE_GESTURE:
+		switch (info) {
+		case IIO_EV_INFO_VALUE:
+			ret = adxl345_set_tap_threshold(st, val);
+			break;
+		case IIO_EV_INFO_TIMEOUT:
+			ret = adxl345_set_tap_duration(st, val, val2);
+			break;
+		default:
+			ret = -EINVAL;
+		}
+		break;
+	default:
+		ret = -EINVAL;
+	}
+
+	if (ret)
+		return ret; /* measurement stays off */
+
+	return adxl345_set_measure_en(st, true);
+}
+
 static int adxl345_reg_access(struct iio_dev *indio_dev, unsigned int reg,
 			      unsigned int writeval, unsigned int *readval)
 {
@@ -383,8 +691,33 @@ static const struct iio_buffer_setup_ops adxl345_buffer_ops = {
 	.predisable = adxl345_buffer_predisable,
 };
 
-static int adxl345_get_status(struct adxl345_state *st, unsigned int *int_stat)
+static int adxl345_get_status(struct adxl345_state *st, unsigned int *int_stat,
+			      enum iio_modifier *act_tap_dir)
 {
+	unsigned int regval;
+	bool check_tap_stat;
+	int ret;
+
+	*act_tap_dir = IIO_NO_MOD;
+	check_tap_stat = FIELD_GET(ADXL345_REG_TAP_AXIS_MSK, st->tap_axis_ctrl) > 0;
+
+	if (check_tap_stat) {
+		/* ACT_TAP_STATUS should be read before clearing the interrupt */
+		ret = regmap_read(st->regmap, ADXL345_REG_ACT_TAP_STATUS, &regval);
+		if (ret)
+			return ret;
+
+		if ((FIELD_GET(ADXL345_Z_EN, regval >> 4)
+				| FIELD_GET(ADXL345_Z_EN, regval)) > 0)
+			*act_tap_dir = IIO_MOD_Z;
+		else if ((FIELD_GET(ADXL345_Y_EN, regval >> 4)
+				| FIELD_GET(ADXL345_Y_EN, regval)) > 0)
+			*act_tap_dir = IIO_MOD_Y;
+		else if ((FIELD_GET(ADXL345_X_EN, regval >> 4)
+				| FIELD_GET(ADXL345_X_EN, regval)) > 0)
+			*act_tap_dir = IIO_MOD_X;
+	}
+
 	return regmap_read(st->regmap, ADXL345_REG_INT_SOURCE, int_stat);
 }
 
@@ -407,6 +740,26 @@ static int adxl345_fifo_push(struct iio_dev *indio_dev,
 	return 0;
 }
 
+static int adxl345_push_event(struct iio_dev *indio_dev, int int_stat,
+			      enum iio_modifier act_tap_dir)
+{
+	s64 ts = iio_get_time_ns(indio_dev);
+	int ret;
+
+	if (FIELD_GET(ADXL345_INT_SINGLE_TAP, int_stat)) {
+		ret = iio_push_event(indio_dev,
+				     IIO_MOD_EVENT_CODE(IIO_ACCEL, 0,
+							act_tap_dir,
+							IIO_EV_TYPE_GESTURE,
+							IIO_EV_DIR_SINGLETAP),
+				     ts);
+		if (ret)
+			return ret;
+	}
+
+	return -ENOENT;
+}
+
 /**
  * adxl345_irq_handler() - Handle irqs of the ADXL345.
  * @irq: The irq being handled.
@@ -419,11 +772,15 @@ static irqreturn_t adxl345_irq_handler(int irq, void *p)
 	struct iio_dev *indio_dev = p;
 	struct adxl345_state *st = iio_priv(indio_dev);
 	int int_stat;
+	enum iio_modifier act_tap_dir;
 	int samples;
 
-	if (adxl345_get_status(st, &int_stat))
+	if (adxl345_get_status(st, &int_stat, &act_tap_dir))
 		return IRQ_NONE;
 
+	if (adxl345_push_event(indio_dev, int_stat, act_tap_dir) == 0)
+		return IRQ_HANDLED;
+
 	if (FIELD_GET(ADXL345_INT_WATERMARK, int_stat)) {
 		samples = adxl345_get_samples(st);
 		if (samples < 0)
@@ -449,6 +806,10 @@ static const struct iio_info adxl345_info = {
 	.read_raw	= adxl345_read_raw,
 	.write_raw	= adxl345_write_raw,
 	.write_raw_get_fmt	= adxl345_write_raw_get_fmt,
+	.read_event_config = adxl345_read_event_config,
+	.write_event_config = adxl345_write_event_config,
+	.read_event_value = adxl345_read_event_value,
+	.write_event_value = adxl345_write_event_value,
 	.debugfs_reg_access = &adxl345_reg_access,
 	.hwfifo_set_watermark = adxl345_set_watermark,
 };
@@ -496,6 +857,10 @@ int adxl345_core_probe(struct device *dev, struct regmap *regmap,
 
 	st->int_map = 0x00;			/* reset interrupts */
 
+	/* Init with reasonable values */
+	st->tap_threshold = 48;			/*   48 [0x30] -> ~3g     */
+	st->tap_duration_us = 16;		/*   16 [0x10] -> .010    */
+
 	indio_dev->name = st->info->name;
 	indio_dev->info = &adxl345_info;
 	indio_dev->modes = INDIO_DIRECT_MODE;
-- 
2.39.5

Re: [PATCH v2 05/14] iio: accel: adxl345: add single tap feature

[Jonathan Cameron]

On Mon, 10 Feb 2025 11:01:10 +0000
Lothar Rubusch <l.rubusch@gmail.com> wrote:

> Add the single tap feature with a threshold in 62.5mg/LSB points and a
> scaled duration in us. Keep singletap threshold and duration using means
> of IIO ABI. Extend the channels for single enable x/y/z axis of the
> feature but also initialize threshold and duration with reasonable
> content. When an interrupt is caught it will be pushed to the according
> IIO channel.
> 
> Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
Hi Lothar,

Various comments inline,

Thanks,

Jonathan

> ---
>  drivers/iio/accel/adxl345_core.c | 369 ++++++++++++++++++++++++++++++-
>  1 file changed, 367 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c
> index 910ad21279ed..304147a4ca60 100644
> --- a/drivers/iio/accel/adxl345_core.c
> +++ b/drivers/iio/accel/adxl345_core.c
> @@ -8,6 +8,7 @@
>   */

> +};
> +
> +static const unsigned int adxl345_tap_int_reg[2] = {

Why force 2?  Just use []

> +	[ADXL345_SINGLE_TAP] = ADXL345_INT_SINGLE_TAP,
> +};
> +
> +enum adxl345_tap_time_type {
> +	ADXL345_TAP_TIME_DUR,
> +};
> +
> +static const unsigned int adxl345_tap_time_reg[3] = {
Why force 3?
> +	[ADXL345_TAP_TIME_DUR] = ADXL345_REG_DUR,
> +};

> +static int _adxl345_set_tap_int(struct adxl345_state *st,
> +				enum adxl345_tap_type type, bool state)
> +{
> +	bool axis_valid;
> +	bool singletap_args_valid = false;
> +	bool en = false;
> +
> +	axis_valid = FIELD_GET(ADXL345_REG_TAP_AXIS_MSK, st->tap_axis_ctrl) > 0;
> +
> +	/*
> +	 * Note: A value of 0 for threshold and/or dur may result in undesirable
> +	 *	 behavior if single tap/double tap interrupts are enabled.
> +	 */
> +	singletap_args_valid = st->tap_threshold > 0 && st->tap_duration_us > 0;
> +
> +	if (type == ADXL345_SINGLE_TAP)
> +		en = axis_valid && singletap_args_valid;
> +
> +	if (state && en)
> +		__set_bit(ilog2(adxl345_tap_int_reg[type]),
> +			  (unsigned long *)&st->int_map);

That's not a good idea (I think I probably mislead you here). Casting
across integer pointer types isn't a good way to go.

> +	else
> +		__clear_bit(ilog2(adxl345_tap_int_reg[type]),
> +			    (unsigned long *)&st->int_map);
> +
> +	return regmap_write(st->regmap, ADXL345_REG_INT_ENABLE, st->int_map);

As in previous, if we rely on the regmap cache then can just use
the regmap_set_bit() regmap_clear_bit() type helpers here.

> +}
>
> +static int adxl345_read_event_config(struct iio_dev *indio_dev,
> +				     const struct iio_chan_spec *chan,
> +				     enum iio_event_type type,
> +				     enum iio_event_direction dir)
> +{
> +	struct adxl345_state *st = iio_priv(indio_dev);
> +	bool int_en;
> +	bool axis_en;
> +	int ret = -EFAULT;
> +
> +	switch (type) {
> +	case IIO_EV_TYPE_GESTURE:
> +		switch (chan->channel2) {
> +		case IIO_MOD_X:
> +			axis_en = FIELD_GET(ADXL345_X_EN, st->tap_axis_ctrl);
> +			break;
> +		case IIO_MOD_Y:
> +			axis_en = FIELD_GET(ADXL345_Y_EN, st->tap_axis_ctrl);
> +			break;
> +		case IIO_MOD_Z:
> +			axis_en = FIELD_GET(ADXL345_Z_EN, st->tap_axis_ctrl);
> +			break;
> +		default:
> +			axis_en = ADXL345_TAP_SUPPRESS;

Add a break here.  Not strictly necessary but hardens against odd code
changes in future.

> +		}
> +
> +		switch (dir) {
> +		case IIO_EV_DIR_SINGLETAP:
> +			ret = adxl345_is_tap_en(st, ADXL345_SINGLE_TAP, &int_en);
> +			if (ret)
> +				return ret;
> +			return int_en && axis_en;
> +		default:
> +			return -EINVAL;
> +		}
> +	default:
> +		return -EINVAL;
> +	}
> +}

> +
> +static int adxl345_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)

My general preference in IIO is to keep to older 80 char line wrap limit unless
longer lines help readability. Even then try to keep them only a little longer.
So I'd prefer these parameters wrapped a bit more!

> +{

> -static int adxl345_get_status(struct adxl345_state *st, unsigned int *int_stat)
> +static int adxl345_get_status(struct adxl345_state *st, unsigned int *int_stat,
> +			      enum iio_modifier *act_tap_dir)
>  {
> +	unsigned int regval;
> +	bool check_tap_stat;
> +	int ret;
> +
> +	*act_tap_dir = IIO_NO_MOD;
> +	check_tap_stat = FIELD_GET(ADXL345_REG_TAP_AXIS_MSK, st->tap_axis_ctrl) > 0;
> +
> +	if (check_tap_stat) {
> +		/* ACT_TAP_STATUS should be read before clearing the interrupt */
> +		ret = regmap_read(st->regmap, ADXL345_REG_ACT_TAP_STATUS, &regval);
> +		if (ret)
> +			return ret;
> +

This double bit pattern is odd enough that I think it needs a comment or
separate defines for the ACT_X source and TAP_X source

> +		if ((FIELD_GET(ADXL345_Z_EN, regval >> 4)
> +				| FIELD_GET(ADXL345_Z_EN, regval)) > 0)
> +			*act_tap_dir = IIO_MOD_Z;
> +		else if ((FIELD_GET(ADXL345_Y_EN, regval >> 4)
> +				| FIELD_GET(ADXL345_Y_EN, regval)) > 0)
> +			*act_tap_dir = IIO_MOD_Y;
> +		else if ((FIELD_GET(ADXL345_X_EN, regval >> 4)
> +				| FIELD_GET(ADXL345_X_EN, regval)) > 0)
> +			*act_tap_dir = IIO_MOD_X;
> +	}
> +

[PATCH v2 06/14] iio: accel: adxl345: add double tap feature

[Lothar Rubusch]

Add the double tap feature of the sensor. The interrupt handler needs
to catch and forward the event to the IIO channel. The single tap
implementation now is extended to deal with double tap as well.

Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
---
 drivers/iio/accel/adxl345_core.c | 102 ++++++++++++++++++++++++++++++-
 1 file changed, 100 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c
index 304147a4ca60..cf35a8f9f432 100644
--- a/drivers/iio/accel/adxl345_core.c
+++ b/drivers/iio/accel/adxl345_core.c
@@ -46,17 +46,23 @@ enum adxl345_axis {
 /* single/double tap */
 enum adxl345_tap_type {
 	ADXL345_SINGLE_TAP,
+	ADXL345_DOUBLE_TAP,
 };
 
 static const unsigned int adxl345_tap_int_reg[2] = {
 	[ADXL345_SINGLE_TAP] = ADXL345_INT_SINGLE_TAP,
+	[ADXL345_DOUBLE_TAP] = ADXL345_INT_DOUBLE_TAP,
 };
 
 enum adxl345_tap_time_type {
+	ADXL345_TAP_TIME_LATENT,
+	ADXL345_TAP_TIME_WINDOW,
 	ADXL345_TAP_TIME_DUR,
 };
 
 static const unsigned int adxl345_tap_time_reg[3] = {
+	[ADXL345_TAP_TIME_LATENT] = ADXL345_REG_LATENT,
+	[ADXL345_TAP_TIME_WINDOW] = ADXL345_REG_WINDOW,
 	[ADXL345_TAP_TIME_DUR] = ADXL345_REG_DUR,
 };
 
@@ -73,6 +79,8 @@ struct adxl345_state {
 	u32 tap_axis_ctrl;
 	u8 tap_threshold;
 	u32 tap_duration_us;
+	u32 tap_latent_us;
+	u32 tap_window_us;
 
 	__le16 fifo_buf[ADXL345_DIRS * ADXL345_FIFO_SIZE + 1] __aligned(IIO_DMA_MINALIGN);
 };
@@ -86,6 +94,14 @@ static struct iio_event_spec adxl345_events[] = {
 		.mask_shared_by_type = BIT(IIO_EV_INFO_VALUE) |
 			BIT(IIO_EV_INFO_TIMEOUT),
 	},
+	{
+		/* double tap */
+		.type = IIO_EV_TYPE_GESTURE,
+		.dir = IIO_EV_DIR_DOUBLETAP,
+		.mask_shared_by_type = BIT(IIO_EV_INFO_ENABLE) |
+			BIT(IIO_EV_INFO_RESET_TIMEOUT) |
+			BIT(IIO_EV_INFO_TAP2_MIN_DELAY),
+	},
 };
 
 #define ADXL345_CHANNEL(index, reg, axis) {					\
@@ -163,6 +179,7 @@ static int _adxl345_set_tap_int(struct adxl345_state *st,
 {
 	bool axis_valid;
 	bool singletap_args_valid = false;
+	bool doubletap_args_valid = false;
 	bool en = false;
 
 	axis_valid = FIELD_GET(ADXL345_REG_TAP_AXIS_MSK, st->tap_axis_ctrl) > 0;
@@ -173,8 +190,16 @@ static int _adxl345_set_tap_int(struct adxl345_state *st,
 	 */
 	singletap_args_valid = st->tap_threshold > 0 && st->tap_duration_us > 0;
 
-	if (type == ADXL345_SINGLE_TAP)
+	if (type == ADXL345_SINGLE_TAP) {
 		en = axis_valid && singletap_args_valid;
+	} else {
+		/* doubletap: Window must be equal or greater than latent! */
+		doubletap_args_valid = st->tap_latent_us > 0 &&
+			st->tap_window_us > 0 &&
+			st->tap_window_us >= st->tap_latent_us;
+
+		en = axis_valid && singletap_args_valid && doubletap_args_valid;
+	}
 
 	if (state && en)
 		__set_bit(ilog2(adxl345_tap_int_reg[type]),
@@ -213,6 +238,11 @@ static int adxl345_set_singletap_en(struct adxl345_state *st,
 	return _adxl345_set_tap_int(st, ADXL345_SINGLE_TAP, en);
 }
 
+static int adxl345_set_doubletap_en(struct adxl345_state *st, bool en)
+{
+	return _adxl345_set_tap_int(st, ADXL345_DOUBLE_TAP, en);
+}
+
 static int adxl345_set_tap_threshold(struct adxl345_state *st, u8 val)
 {
 	int ret;
@@ -232,6 +262,12 @@ static int _adxl345_set_tap_time(struct adxl345_state *st,
 	unsigned int regval;
 
 	switch (type) {
+	case ADXL345_TAP_TIME_WINDOW:
+		st->tap_window_us = val_us;
+		break;
+	case ADXL345_TAP_TIME_LATENT:
+		st->tap_latent_us = val_us;
+		break;
 	case ADXL345_TAP_TIME_DUR:
 		st->tap_duration_us = val_us;
 		break;
@@ -244,7 +280,7 @@ static int _adxl345_set_tap_time(struct adxl345_state *st,
 	if (type == ADXL345_TAP_TIME_DUR)
 		regval = DIV_ROUND_CLOSEST(val_us, 625);
 	else
-		return 0;
+		regval = DIV_ROUND_CLOSEST(val_us, 1250);
 
 	return regmap_write(st->regmap, adxl345_tap_time_reg[type], regval);
 }
@@ -263,6 +299,34 @@ static int adxl345_set_tap_duration(struct adxl345_state *st, u32 val_int,
 	return _adxl345_set_tap_time(st, ADXL345_TAP_TIME_DUR, val_fract_us);
 }
 
+static int adxl345_set_tap_window(struct adxl345_state *st, u32 val_int,
+				  u32 val_fract_us)
+{
+	/*
+	 * Max value is 255 * 1250 us = 0.318750 seconds
+	 *
+	 * Note: the scaling is similar to the scaling in the ADXL380
+	 */
+	if (val_int || val_fract_us > 318750)
+		return -EINVAL;
+
+	return _adxl345_set_tap_time(st, ADXL345_TAP_TIME_WINDOW, val_fract_us);
+}
+
+static int adxl345_set_tap_latent(struct adxl345_state *st, u32 val_int,
+				  u32 val_fract_us)
+{
+	/*
+	 * Max value is 255 * 1250 us = 0.318750 seconds
+	 *
+	 * Note: the scaling is similar to the scaling in the ADXL380
+	 */
+	if (val_int || val_fract_us > 318750)
+		return -EINVAL;
+
+	return _adxl345_set_tap_time(st, ADXL345_TAP_TIME_LATENT, val_fract_us);
+}
+
 static int adxl345_read_raw(struct iio_dev *indio_dev,
 			    struct iio_chan_spec const *chan,
 			    int *val, int *val2, long mask)
@@ -380,6 +444,11 @@ static int adxl345_read_event_config(struct iio_dev *indio_dev,
 			if (ret)
 				return ret;
 			return int_en && axis_en;
+		case IIO_EV_DIR_DOUBLETAP:
+			ret = adxl345_is_tap_en(st, ADXL345_DOUBLE_TAP, &int_en);
+			if (ret)
+				return ret;
+			return int_en;
 		default:
 			return -EINVAL;
 		}
@@ -416,6 +485,8 @@ static int adxl345_write_event_config(struct iio_dev *indio_dev,
 		switch (dir) {
 		case IIO_EV_DIR_SINGLETAP:
 			return adxl345_set_singletap_en(st, axis, state);
+		case IIO_EV_DIR_DOUBLETAP:
+			return adxl345_set_doubletap_en(st, state);
 		default:
 			return -EINVAL;
 		}
@@ -444,6 +515,14 @@ static int adxl345_read_event_value(struct iio_dev *indio_dev, const struct iio_
 			*val = st->tap_duration_us;
 			*val2 = 1000000;
 			return IIO_VAL_FRACTIONAL;
+		case IIO_EV_INFO_RESET_TIMEOUT:
+			*val = st->tap_window_us;
+			*val2 = 1000000;
+			return IIO_VAL_FRACTIONAL;
+		case IIO_EV_INFO_TAP2_MIN_DELAY:
+			*val = st->tap_latent_us;
+			*val2 = 1000000;
+			return IIO_VAL_FRACTIONAL;
 		default:
 			return -EINVAL;
 		}
@@ -475,6 +554,12 @@ static int adxl345_write_event_value(struct iio_dev *indio_dev,
 		case IIO_EV_INFO_TIMEOUT:
 			ret = adxl345_set_tap_duration(st, val, val2);
 			break;
+		case IIO_EV_INFO_RESET_TIMEOUT:
+			ret = adxl345_set_tap_window(st, val, val2);
+			break;
+		case IIO_EV_INFO_TAP2_MIN_DELAY:
+			ret = adxl345_set_tap_latent(st, val, val2);
+			break;
 		default:
 			ret = -EINVAL;
 		}
@@ -757,6 +842,17 @@ static int adxl345_push_event(struct iio_dev *indio_dev, int int_stat,
 			return ret;
 	}
 
+	if (FIELD_GET(ADXL345_INT_DOUBLE_TAP, int_stat)) {
+		ret = iio_push_event(indio_dev,
+				     IIO_MOD_EVENT_CODE(IIO_ACCEL, 0,
+							act_tap_dir,
+							IIO_EV_TYPE_GESTURE,
+							IIO_EV_DIR_DOUBLETAP),
+				     ts);
+		if (ret)
+			return ret;
+	}
+
 	return -ENOENT;
 }
 
@@ -860,6 +956,8 @@ int adxl345_core_probe(struct device *dev, struct regmap *regmap,
 	/* Init with reasonable values */
 	st->tap_threshold = 48;			/*   48 [0x30] -> ~3g     */
 	st->tap_duration_us = 16;		/*   16 [0x10] -> .010    */
+	st->tap_window_us = 64;			/*   64 [0x40] -> .080    */
+	st->tap_latent_us = 16;			/*   16 [0x10] -> .020    */
 
 	indio_dev->name = st->info->name;
 	indio_dev->info = &adxl345_info;
-- 
2.39.5

Re: [PATCH v2 06/14] iio: accel: adxl345: add double tap feature

[Jonathan Cameron]

On Mon, 10 Feb 2025 11:01:11 +0000
Lothar Rubusch <l.rubusch@gmail.com> wrote:

> Add the double tap feature of the sensor. The interrupt handler needs
> to catch and forward the event to the IIO channel. The single tap
> implementation now is extended to deal with double tap as well.
> 
> Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
> ---
>  drivers/iio/accel/adxl345_core.c | 102 ++++++++++++++++++++++++++++++-
>  1 file changed, 100 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c
> index 304147a4ca60..cf35a8f9f432 100644
> --- a/drivers/iio/accel/adxl345_core.c
> +++ b/drivers/iio/accel/adxl345_core.c
> @@ -46,17 +46,23 @@ enum adxl345_axis {
>  /* single/double tap */
>  enum adxl345_tap_type {
>  	ADXL345_SINGLE_TAP,
> +	ADXL345_DOUBLE_TAP,
>  };
>  
>  static const unsigned int adxl345_tap_int_reg[2] = {

Ah. Here comes the rest.  Just use [] so no need to update.
This size should only matter the other way around where you
are doing an ARRAY_SIZE() or similar (might not even need that!)

>  	[ADXL345_SINGLE_TAP] = ADXL345_INT_SINGLE_TAP,
> +	[ADXL345_DOUBLE_TAP] = ADXL345_INT_DOUBLE_TAP,
>  };
>  
>  enum adxl345_tap_time_type {
> +	ADXL345_TAP_TIME_LATENT,
> +	ADXL345_TAP_TIME_WINDOW,
>  	ADXL345_TAP_TIME_DUR,
>  };
>  
>  static const unsigned int adxl345_tap_time_reg[3] = {
> +	[ADXL345_TAP_TIME_LATENT] = ADXL345_REG_LATENT,
> +	[ADXL345_TAP_TIME_WINDOW] = ADXL345_REG_WINDOW,
>  	[ADXL345_TAP_TIME_DUR] = ADXL345_REG_DUR,
>  };
>  
> @@ -73,6 +79,8 @@ struct adxl345_state {
>  	u32 tap_axis_ctrl;
>  	u8 tap_threshold;
>  	u32 tap_duration_us;
> +	u32 tap_latent_us;
> +	u32 tap_window_us;
>  
>  	__le16 fifo_buf[ADXL345_DIRS * ADXL345_FIFO_SIZE + 1] __aligned(IIO_DMA_MINALIGN);
>  };
> @@ -86,6 +94,14 @@ static struct iio_event_spec adxl345_events[] = {
>  		.mask_shared_by_type = BIT(IIO_EV_INFO_VALUE) |
>  			BIT(IIO_EV_INFO_TIMEOUT),
>  	},
> +	{
> +		/* double tap */

Kind of obvious comment given the dir, so could just not mention it.

> +		.type = IIO_EV_TYPE_GESTURE,
> +		.dir = IIO_EV_DIR_DOUBLETAP,
> +		.mask_shared_by_type = BIT(IIO_EV_INFO_ENABLE) |
> +			BIT(IIO_EV_INFO_RESET_TIMEOUT) |
> +			BIT(IIO_EV_INFO_TAP2_MIN_DELAY),
> +	},
>  };

> +

[PATCH v2 07/14] iio: accel: adxl345: add double tap suppress bit

[Lothar Rubusch]

Add the suppress bit feature to the double tap feature.

Any tap event is defined by a rising signal edge above threshold, i.e.
duration time starts counting; and the falling edge under threshold
within duration time, i.e. then the tap event is issued. This means
duration is used individually for each tap event.

For double tap detection after a single tap, a latency time needs to be
specified. Usually tap events, i.e. spikes above and returning below
threshold will be ignored within latency. After latency, the window
time starts counting for a second tap detection which has to happen
within a duration time.

If the suppress bit is not set, spikes within latency time are ignored.
Setting the suppress bit will invalidate the double tap function. The
sensor will thus be able to save the window time for double tap
detection, and follow a more strict definition of what signal qualifies
for a double tap.

This brings in a new ABI functionality.
---
Q: Perhaps there is already some IIO ABI for it? If not, please let me
know which ABI documentation to extend. There will be a documentation
patch also later in this series.

Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
---
 drivers/iio/accel/adxl345_core.c | 82 ++++++++++++++++++++++++++++++++
 1 file changed, 82 insertions(+)

diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c
index cf35a8f9f432..b6966fee3e3d 100644
--- a/drivers/iio/accel/adxl345_core.c
+++ b/drivers/iio/accel/adxl345_core.c
@@ -34,6 +34,7 @@
 #define ADXL345_INT2			1
 
 #define ADXL345_REG_TAP_AXIS_MSK	GENMASK(2, 0)
+#define ADXL345_REG_TAP_SUPPRESS_MSK	BIT(3)
 
 enum adxl345_axis {
 	ADXL345_Z_EN = BIT(0),
@@ -81,6 +82,7 @@ struct adxl345_state {
 	u32 tap_duration_us;
 	u32 tap_latent_us;
 	u32 tap_window_us;
+	bool tap_suppressed;
 
 	__le16 fifo_buf[ADXL345_DIRS * ADXL345_FIFO_SIZE + 1] __aligned(IIO_DMA_MINALIGN);
 };
@@ -243,6 +245,31 @@ static int adxl345_set_doubletap_en(struct adxl345_state *st, bool en)
 	return _adxl345_set_tap_int(st, ADXL345_DOUBLE_TAP, en);
 }
 
+static int adxl345_is_suppressed_en(struct adxl345_state *st, bool *en)
+{
+	*en = st->tap_suppressed;
+
+	return 0;
+}
+
+static int adxl345_set_suppressed_en(struct adxl345_state *st, bool en)
+{
+	unsigned long regval = 0;
+	int ret;
+
+	en ? __set_bit(ilog2(ADXL345_TAP_SUPPRESS), &regval)
+		: __clear_bit(ilog2(ADXL345_TAP_SUPPRESS), &regval);
+
+	ret = regmap_update_bits(st->regmap, ADXL345_REG_TAP_AXIS,
+				 ADXL345_REG_TAP_SUPPRESS_MSK, regval);
+	if (ret)
+		return ret;
+
+	st->tap_suppressed = en;
+
+	return 0;
+}
+
 static int adxl345_set_tap_threshold(struct adxl345_state *st, u8 val)
 {
 	int ret;
@@ -616,6 +643,60 @@ static int adxl345_write_raw_get_fmt(struct iio_dev *indio_dev,
 	}
 }
 
+static ssize_t in_accel_gesture_doubletap_suppressed_en_show(struct device *dev,
+							     struct device_attribute *attr,
+							     char *buf)
+{
+	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+	struct adxl345_state *st = iio_priv(indio_dev);
+	bool en;
+	int val, ret;
+
+	ret = adxl345_is_suppressed_en(st, &en);
+	if (ret)
+		return ret;
+	val = en ? 1 : 0;
+
+	return iio_format_value(buf, IIO_VAL_INT, 1, &val);
+}
+
+static ssize_t in_accel_gesture_doubletap_suppressed_en_store(struct device *dev,
+							      struct device_attribute *attr,
+							      const char *buf, size_t len)
+{
+	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+	struct adxl345_state *st = iio_priv(indio_dev);
+	int val, ret;
+
+	ret = kstrtoint(buf, 0, &val);
+	if (ret)
+		return ret;
+
+	ret = adxl345_set_measure_en(st, false);
+	if (ret)
+		return ret;
+
+	ret = adxl345_set_suppressed_en(st, val > 0);
+	if (ret)
+		return ret;
+
+	ret =  adxl345_set_measure_en(st, true);
+	if (ret)
+		return ret;
+
+	return len;
+}
+static IIO_DEVICE_ATTR_RW(in_accel_gesture_doubletap_suppressed_en, 0);
+
+static struct attribute *adxl345_event_attrs[] = {
+	&iio_dev_attr_in_accel_gesture_doubletap_suppressed_en.dev_attr.attr,
+	NULL
+};
+
+static const struct attribute_group adxl345_event_attrs_group = {
+	.attrs = adxl345_event_attrs,
+};
+
 static void adxl345_powerdown(void *ptr)
 {
 	struct adxl345_state *st = ptr;
@@ -899,6 +980,7 @@ static irqreturn_t adxl345_irq_handler(int irq, void *p)
 
 static const struct iio_info adxl345_info = {
 	.attrs		= &adxl345_attrs_group,
+	.event_attrs	= &adxl345_event_attrs_group,
 	.read_raw	= adxl345_read_raw,
 	.write_raw	= adxl345_write_raw,
 	.write_raw_get_fmt	= adxl345_write_raw_get_fmt,
-- 
2.39.5

Re: [PATCH v2 07/14] iio: accel: adxl345: add double tap suppress bit

[Jonathan Cameron]

On Mon, 10 Feb 2025 11:01:12 +0000
Lothar Rubusch <l.rubusch@gmail.com> wrote:

> Add the suppress bit feature to the double tap feature.
> 
> Any tap event is defined by a rising signal edge above threshold, i.e.
> duration time starts counting; and the falling edge under threshold
> within duration time, i.e. then the tap event is issued. This means
> duration is used individually for each tap event.
> 
> For double tap detection after a single tap, a latency time needs to be
> specified. Usually tap events, i.e. spikes above and returning below
> threshold will be ignored within latency. After latency, the window
> time starts counting for a second tap detection which has to happen
> within a duration time.
> 
> If the suppress bit is not set, spikes within latency time are ignored.
> Setting the suppress bit will invalidate the double tap function. The
> sensor will thus be able to save the window time for double tap
> detection, and follow a more strict definition of what signal qualifies
> for a double tap.

Silly question.  Is there a reason this function would ever be
turned off?   Seems like a sensible heuristic that would not stop
genuine double taps being detected.  Maybe we just always leave it on?

Sometimes the best ABI is the one that doesn't exist as userspace
can't use it wrong.

Jonathan


> 
> This brings in a new ABI functionality.
> ---
> Q: Perhaps there is already some IIO ABI for it? If not, please let me
> know which ABI documentation to extend. There will be a documentation
> patch also later in this series.
> 
> Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
> ---
>  drivers/iio/accel/adxl345_core.c | 82 ++++++++++++++++++++++++++++++++
>  1 file changed, 82 insertions(+)
> 
> diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c
> index cf35a8f9f432..b6966fee3e3d 100644
> --- a/drivers/iio/accel/adxl345_core.c
> +++ b/drivers/iio/accel/adxl345_core.c
> @@ -34,6 +34,7 @@
>  #define ADXL345_INT2			1
>  
>  #define ADXL345_REG_TAP_AXIS_MSK	GENMASK(2, 0)
> +#define ADXL345_REG_TAP_SUPPRESS_MSK	BIT(3)
>  
>  enum adxl345_axis {
>  	ADXL345_Z_EN = BIT(0),
> @@ -81,6 +82,7 @@ struct adxl345_state {
>  	u32 tap_duration_us;
>  	u32 tap_latent_us;
>  	u32 tap_window_us;
> +	bool tap_suppressed;
>  
>  	__le16 fifo_buf[ADXL345_DIRS * ADXL345_FIFO_SIZE + 1] __aligned(IIO_DMA_MINALIGN);
>  };
> @@ -243,6 +245,31 @@ static int adxl345_set_doubletap_en(struct adxl345_state *st, bool en)
>  	return _adxl345_set_tap_int(st, ADXL345_DOUBLE_TAP, en);
>  }
>  
> +static int adxl345_is_suppressed_en(struct adxl345_state *st, bool *en)
> +{
> +	*en = st->tap_suppressed;
> +
> +	return 0;
> +}
> +
> +static int adxl345_set_suppressed_en(struct adxl345_state *st, bool en)
> +{
> +	unsigned long regval = 0;
> +	int ret;
> +
> +	en ? __set_bit(ilog2(ADXL345_TAP_SUPPRESS), &regval)
> +		: __clear_bit(ilog2(ADXL345_TAP_SUPPRESS), &regval);
> +
> +	ret = regmap_update_bits(st->regmap, ADXL345_REG_TAP_AXIS,
> +				 ADXL345_REG_TAP_SUPPRESS_MSK, regval);
> +	if (ret)
> +		return ret;
> +
> +	st->tap_suppressed = en;
> +
> +	return 0;
> +}
> +
>  static int adxl345_set_tap_threshold(struct adxl345_state *st, u8 val)
>  {
>  	int ret;
> @@ -616,6 +643,60 @@ static int adxl345_write_raw_get_fmt(struct iio_dev *indio_dev,
>  	}
>  }
>  
> +static ssize_t in_accel_gesture_doubletap_suppressed_en_show(struct device *dev,
> +							     struct device_attribute *attr,
> +							     char *buf)
> +{
> +	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> +	struct adxl345_state *st = iio_priv(indio_dev);
> +	bool en;
> +	int val, ret;
> +
> +	ret = adxl345_is_suppressed_en(st, &en);
> +	if (ret)
> +		return ret;
> +	val = en ? 1 : 0;
> +
> +	return iio_format_value(buf, IIO_VAL_INT, 1, &val);
> +}
> +
> +static ssize_t in_accel_gesture_doubletap_suppressed_en_store(struct device *dev,
> +							      struct device_attribute *attr,
> +							      const char *buf, size_t len)
> +{
> +	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> +	struct adxl345_state *st = iio_priv(indio_dev);
> +	int val, ret;
> +
> +	ret = kstrtoint(buf, 0, &val);
> +	if (ret)
> +		return ret;
> +
> +	ret = adxl345_set_measure_en(st, false);
> +	if (ret)
> +		return ret;
> +
> +	ret = adxl345_set_suppressed_en(st, val > 0);
> +	if (ret)
> +		return ret;
> +
> +	ret =  adxl345_set_measure_en(st, true);
> +	if (ret)
> +		return ret;
> +
> +	return len;
> +}
> +static IIO_DEVICE_ATTR_RW(in_accel_gesture_doubletap_suppressed_en, 0);
> +
> +static struct attribute *adxl345_event_attrs[] = {
> +	&iio_dev_attr_in_accel_gesture_doubletap_suppressed_en.dev_attr.attr,
> +	NULL
> +};
> +
> +static const struct attribute_group adxl345_event_attrs_group = {
> +	.attrs = adxl345_event_attrs,
> +};
> +
>  static void adxl345_powerdown(void *ptr)
>  {
>  	struct adxl345_state *st = ptr;
> @@ -899,6 +980,7 @@ static irqreturn_t adxl345_irq_handler(int irq, void *p)
>  
>  static const struct iio_info adxl345_info = {
>  	.attrs		= &adxl345_attrs_group,
> +	.event_attrs	= &adxl345_event_attrs_group,
>  	.read_raw	= adxl345_read_raw,
>  	.write_raw	= adxl345_write_raw,
>  	.write_raw_get_fmt	= adxl345_write_raw_get_fmt,

Re: [PATCH v2 07/14] iio: accel: adxl345: add double tap suppress bit

[Lothar Rubusch]

Dear Jonathan, find my answer down below.

On Sun, Feb 16, 2025 at 6:28 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Mon, 10 Feb 2025 11:01:12 +0000
> Lothar Rubusch <l.rubusch@gmail.com> wrote:
>
> > Add the suppress bit feature to the double tap feature.
> >
> > Any tap event is defined by a rising signal edge above threshold, i.e.
> > duration time starts counting; and the falling edge under threshold
> > within duration time, i.e. then the tap event is issued. This means
> > duration is used individually for each tap event.
> >
> > For double tap detection after a single tap, a latency time needs to be
> > specified. Usually tap events, i.e. spikes above and returning below
> > threshold will be ignored within latency. After latency, the window
> > time starts counting for a second tap detection which has to happen
> > within a duration time.
> >
> > If the suppress bit is not set, spikes within latency time are ignored.
> > Setting the suppress bit will invalidate the double tap function. The
> > sensor will thus be able to save the window time for double tap
> > detection, and follow a more strict definition of what signal qualifies
> > for a double tap.
>
> Silly question.  Is there a reason this function would ever be
> turned off?   Seems like a sensible heuristic that would not stop
> genuine double taps being detected.  Maybe we just always leave it on?
>
> Sometimes the best ABI is the one that doesn't exist as userspace
> can't use it wrong.
>
> Jonathan
>

hehehe..  you already mentioned this point, I guess. At least I tried
to put my understanding of it into the lengthy comment of the patch.
Well, patches with lengthy comments.... this seems to go into the same
direction as the wisdom of better limiting userspace interfaces in
general ;)

TBH you have probably seen far more sensors than me, as I'm doing this
just as hobbyist to learn and for fun. I only can provide my
understanding of the particular datasheet.
I think, to set or not to set this bit changes little. It influences a
bit how restrictive the latency period is handled at detection.
Doubletaps are detected with or without having the "suppress" bit set.
If set, AFAIK it could be harder to detect doubletaps. So to speak,
you could reduce "noise" in double tapping (?), or if one receives too
many double taps...(?) perhaps,  ..eh.. legal reasons?! Personally,
I'd liked to play with this sensor a bit, and I found it then useful
to have some kind of knob to change a bit and see what happens without
really messing things up.
As I'm not too familiar with the accelerometer scene and such kind of
"power user settings". I'm unsure if there are typical usecases here.
I would agree that usually one would leave that in one  setting,
turned on or off (unless he/she enters in obsession with double taps).

Perhaps I'll change this patch so that it's always set or not set (to
bring it initially into a defined state), but no sysfs is around.
Let's see. If you think I'd better just drop it entirly, let me know
then.

Best,
L

>
> >
> > This brings in a new ABI functionality.
> > ---
> > Q: Perhaps there is already some IIO ABI for it? If not, please let me
> > know which ABI documentation to extend. There will be a documentation
> > patch also later in this series.
> >
> > Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
> > ---
> >  drivers/iio/accel/adxl345_core.c | 82 ++++++++++++++++++++++++++++++++
> >  1 file changed, 82 insertions(+)
> >
> > diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c
> > index cf35a8f9f432..b6966fee3e3d 100644
> > --- a/drivers/iio/accel/adxl345_core.c
> > +++ b/drivers/iio/accel/adxl345_core.c
> > @@ -34,6 +34,7 @@
> >  #define ADXL345_INT2                 1
> >
> >  #define ADXL345_REG_TAP_AXIS_MSK     GENMASK(2, 0)
> > +#define ADXL345_REG_TAP_SUPPRESS_MSK BIT(3)
> >
> >  enum adxl345_axis {
> >       ADXL345_Z_EN = BIT(0),
> > @@ -81,6 +82,7 @@ struct adxl345_state {
> >       u32 tap_duration_us;
> >       u32 tap_latent_us;
> >       u32 tap_window_us;
> > +     bool tap_suppressed;
> >
> >       __le16 fifo_buf[ADXL345_DIRS * ADXL345_FIFO_SIZE + 1] __aligned(IIO_DMA_MINALIGN);
> >  };
> > @@ -243,6 +245,31 @@ static int adxl345_set_doubletap_en(struct adxl345_state *st, bool en)
> >       return _adxl345_set_tap_int(st, ADXL345_DOUBLE_TAP, en);
> >  }
> >
> > +static int adxl345_is_suppressed_en(struct adxl345_state *st, bool *en)
> > +{
> > +     *en = st->tap_suppressed;
> > +
> > +     return 0;
> > +}
> > +
> > +static int adxl345_set_suppressed_en(struct adxl345_state *st, bool en)
> > +{
> > +     unsigned long regval = 0;
> > +     int ret;
> > +
> > +     en ? __set_bit(ilog2(ADXL345_TAP_SUPPRESS), &regval)
> > +             : __clear_bit(ilog2(ADXL345_TAP_SUPPRESS), &regval);
> > +
> > +     ret = regmap_update_bits(st->regmap, ADXL345_REG_TAP_AXIS,
> > +                              ADXL345_REG_TAP_SUPPRESS_MSK, regval);
> > +     if (ret)
> > +             return ret;
> > +
> > +     st->tap_suppressed = en;
> > +
> > +     return 0;
> > +}
> > +
> >  static int adxl345_set_tap_threshold(struct adxl345_state *st, u8 val)
> >  {
> >       int ret;
> > @@ -616,6 +643,60 @@ static int adxl345_write_raw_get_fmt(struct iio_dev *indio_dev,
> >       }
> >  }
> >
> > +static ssize_t in_accel_gesture_doubletap_suppressed_en_show(struct device *dev,
> > +                                                          struct device_attribute *attr,
> > +                                                          char *buf)
> > +{
> > +     struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> > +     struct adxl345_state *st = iio_priv(indio_dev);
> > +     bool en;
> > +     int val, ret;
> > +
> > +     ret = adxl345_is_suppressed_en(st, &en);
> > +     if (ret)
> > +             return ret;
> > +     val = en ? 1 : 0;
> > +
> > +     return iio_format_value(buf, IIO_VAL_INT, 1, &val);
> > +}
> > +
> > +static ssize_t in_accel_gesture_doubletap_suppressed_en_store(struct device *dev,
> > +                                                           struct device_attribute *attr,
> > +                                                           const char *buf, size_t len)
> > +{
> > +     struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> > +     struct adxl345_state *st = iio_priv(indio_dev);
> > +     int val, ret;
> > +
> > +     ret = kstrtoint(buf, 0, &val);
> > +     if (ret)
> > +             return ret;
> > +
> > +     ret = adxl345_set_measure_en(st, false);
> > +     if (ret)
> > +             return ret;
> > +
> > +     ret = adxl345_set_suppressed_en(st, val > 0);
> > +     if (ret)
> > +             return ret;
> > +
> > +     ret =  adxl345_set_measure_en(st, true);
> > +     if (ret)
> > +             return ret;
> > +
> > +     return len;
> > +}
> > +static IIO_DEVICE_ATTR_RW(in_accel_gesture_doubletap_suppressed_en, 0);
> > +
> > +static struct attribute *adxl345_event_attrs[] = {
> > +     &iio_dev_attr_in_accel_gesture_doubletap_suppressed_en.dev_attr.attr,
> > +     NULL
> > +};
> > +
> > +static const struct attribute_group adxl345_event_attrs_group = {
> > +     .attrs = adxl345_event_attrs,
> > +};
> > +
> >  static void adxl345_powerdown(void *ptr)
> >  {
> >       struct adxl345_state *st = ptr;
> > @@ -899,6 +980,7 @@ static irqreturn_t adxl345_irq_handler(int irq, void *p)
> >
> >  static const struct iio_info adxl345_info = {
> >       .attrs          = &adxl345_attrs_group,
> > +     .event_attrs    = &adxl345_event_attrs_group,
> >       .read_raw       = adxl345_read_raw,
> >       .write_raw      = adxl345_write_raw,
> >       .write_raw_get_fmt      = adxl345_write_raw_get_fmt,
>

Re: [PATCH v2 07/14] iio: accel: adxl345: add double tap suppress bit

[Jonathan Cameron]

On Tue, 18 Feb 2025 23:29:46 +0100
Lothar Rubusch <l.rubusch@gmail.com> wrote:

> Dear Jonathan, find my answer down below.
> 
> On Sun, Feb 16, 2025 at 6:28 PM Jonathan Cameron <jic23@kernel.org> wrote:
> >
> > On Mon, 10 Feb 2025 11:01:12 +0000
> > Lothar Rubusch <l.rubusch@gmail.com> wrote:
> >  
> > > Add the suppress bit feature to the double tap feature.
> > >
> > > Any tap event is defined by a rising signal edge above threshold, i.e.
> > > duration time starts counting; and the falling edge under threshold
> > > within duration time, i.e. then the tap event is issued. This means
> > > duration is used individually for each tap event.
> > >
> > > For double tap detection after a single tap, a latency time needs to be
> > > specified. Usually tap events, i.e. spikes above and returning below
> > > threshold will be ignored within latency. After latency, the window
> > > time starts counting for a second tap detection which has to happen
> > > within a duration time.
> > >
> > > If the suppress bit is not set, spikes within latency time are ignored.
> > > Setting the suppress bit will invalidate the double tap function. The
> > > sensor will thus be able to save the window time for double tap
> > > detection, and follow a more strict definition of what signal qualifies
> > > for a double tap.  
> >
> > Silly question.  Is there a reason this function would ever be
> > turned off?   Seems like a sensible heuristic that would not stop
> > genuine double taps being detected.  Maybe we just always leave it on?
> >
> > Sometimes the best ABI is the one that doesn't exist as userspace
> > can't use it wrong.
> >
> > Jonathan
> >  
> 
> hehehe..  you already mentioned this point, I guess. At least I tried
> to put my understanding of it into the lengthy comment of the patch.
> Well, patches with lengthy comments.... this seems to go into the same
> direction as the wisdom of better limiting userspace interfaces in
> general ;)
> 
> TBH you have probably seen far more sensors than me, as I'm doing this
> just as hobbyist to learn and for fun. I only can provide my
> understanding of the particular datasheet.
> I think, to set or not to set this bit changes little. It influences a
> bit how restrictive the latency period is handled at detection.
> Doubletaps are detected with or without having the "suppress" bit set.
> If set, AFAIK it could be harder to detect doubletaps. So to speak,
> you could reduce "noise" in double tapping (?), or if one receives too
> many double taps...(?) perhaps,  ..eh.. legal reasons?! Personally,
> I'd liked to play with this sensor a bit, and I found it then useful
> to have some kind of knob to change a bit and see what happens without
> really messing things up.
> As I'm not too familiar with the accelerometer scene and such kind of
> "power user settings". I'm unsure if there are typical usecases here.
> I would agree that usually one would leave that in one  setting,
> turned on or off (unless he/she enters in obsession with double taps).
> 
> Perhaps I'll change this patch so that it's always set or not set (to
> bring it initially into a defined state), but no sysfs is around.
> Let's see. If you think I'd better just drop it entirly, let me know
> then.
I think default to always set.  We can revisit the ABI question later
if turns out to have be something people change in practice!

Jonathan

> 
> Best,
> L
> 
> >  
> > >
> > > This brings in a new ABI functionality.
> > > ---
> > > Q: Perhaps there is already some IIO ABI for it? If not, please let me
> > > know which ABI documentation to extend. There will be a documentation
> > > patch also later in this series.
> > >
> > > Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
> > > ---
> > >  drivers/iio/accel/adxl345_core.c | 82 ++++++++++++++++++++++++++++++++
> > >  1 file changed, 82 insertions(+)
> > >
> > > diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c
> > > index cf35a8f9f432..b6966fee3e3d 100644
> > > --- a/drivers/iio/accel/adxl345_core.c
> > > +++ b/drivers/iio/accel/adxl345_core.c
> > > @@ -34,6 +34,7 @@
> > >  #define ADXL345_INT2                 1
> > >
> > >  #define ADXL345_REG_TAP_AXIS_MSK     GENMASK(2, 0)
> > > +#define ADXL345_REG_TAP_SUPPRESS_MSK BIT(3)
> > >
> > >  enum adxl345_axis {
> > >       ADXL345_Z_EN = BIT(0),
> > > @@ -81,6 +82,7 @@ struct adxl345_state {
> > >       u32 tap_duration_us;
> > >       u32 tap_latent_us;
> > >       u32 tap_window_us;
> > > +     bool tap_suppressed;
> > >
> > >       __le16 fifo_buf[ADXL345_DIRS * ADXL345_FIFO_SIZE + 1] __aligned(IIO_DMA_MINALIGN);
> > >  };
> > > @@ -243,6 +245,31 @@ static int adxl345_set_doubletap_en(struct adxl345_state *st, bool en)
> > >       return _adxl345_set_tap_int(st, ADXL345_DOUBLE_TAP, en);
> > >  }
> > >
> > > +static int adxl345_is_suppressed_en(struct adxl345_state *st, bool *en)
> > > +{
> > > +     *en = st->tap_suppressed;
> > > +
> > > +     return 0;
> > > +}
> > > +
> > > +static int adxl345_set_suppressed_en(struct adxl345_state *st, bool en)
> > > +{
> > > +     unsigned long regval = 0;
> > > +     int ret;
> > > +
> > > +     en ? __set_bit(ilog2(ADXL345_TAP_SUPPRESS), &regval)
> > > +             : __clear_bit(ilog2(ADXL345_TAP_SUPPRESS), &regval);
> > > +
> > > +     ret = regmap_update_bits(st->regmap, ADXL345_REG_TAP_AXIS,
> > > +                              ADXL345_REG_TAP_SUPPRESS_MSK, regval);
> > > +     if (ret)
> > > +             return ret;
> > > +
> > > +     st->tap_suppressed = en;
> > > +
> > > +     return 0;
> > > +}
> > > +
> > >  static int adxl345_set_tap_threshold(struct adxl345_state *st, u8 val)
> > >  {
> > >       int ret;
> > > @@ -616,6 +643,60 @@ static int adxl345_write_raw_get_fmt(struct iio_dev *indio_dev,
> > >       }
> > >  }
> > >
> > > +static ssize_t in_accel_gesture_doubletap_suppressed_en_show(struct device *dev,
> > > +                                                          struct device_attribute *attr,
> > > +                                                          char *buf)
> > > +{
> > > +     struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> > > +     struct adxl345_state *st = iio_priv(indio_dev);
> > > +     bool en;
> > > +     int val, ret;
> > > +
> > > +     ret = adxl345_is_suppressed_en(st, &en);
> > > +     if (ret)
> > > +             return ret;
> > > +     val = en ? 1 : 0;
> > > +
> > > +     return iio_format_value(buf, IIO_VAL_INT, 1, &val);
> > > +}
> > > +
> > > +static ssize_t in_accel_gesture_doubletap_suppressed_en_store(struct device *dev,
> > > +                                                           struct device_attribute *attr,
> > > +                                                           const char *buf, size_t len)
> > > +{
> > > +     struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> > > +     struct adxl345_state *st = iio_priv(indio_dev);
> > > +     int val, ret;
> > > +
> > > +     ret = kstrtoint(buf, 0, &val);
> > > +     if (ret)
> > > +             return ret;
> > > +
> > > +     ret = adxl345_set_measure_en(st, false);
> > > +     if (ret)
> > > +             return ret;
> > > +
> > > +     ret = adxl345_set_suppressed_en(st, val > 0);
> > > +     if (ret)
> > > +             return ret;
> > > +
> > > +     ret =  adxl345_set_measure_en(st, true);
> > > +     if (ret)
> > > +             return ret;
> > > +
> > > +     return len;
> > > +}
> > > +static IIO_DEVICE_ATTR_RW(in_accel_gesture_doubletap_suppressed_en, 0);
> > > +
> > > +static struct attribute *adxl345_event_attrs[] = {
> > > +     &iio_dev_attr_in_accel_gesture_doubletap_suppressed_en.dev_attr.attr,
> > > +     NULL
> > > +};
> > > +
> > > +static const struct attribute_group adxl345_event_attrs_group = {
> > > +     .attrs = adxl345_event_attrs,
> > > +};
> > > +
> > >  static void adxl345_powerdown(void *ptr)
> > >  {
> > >       struct adxl345_state *st = ptr;
> > > @@ -899,6 +980,7 @@ static irqreturn_t adxl345_irq_handler(int irq, void *p)
> > >
> > >  static const struct iio_info adxl345_info = {
> > >       .attrs          = &adxl345_attrs_group,
> > > +     .event_attrs    = &adxl345_event_attrs_group,
> > >       .read_raw       = adxl345_read_raw,
> > >       .write_raw      = adxl345_write_raw,
> > >       .write_raw_get_fmt      = adxl345_write_raw_get_fmt,  
> >  
> 

[PATCH v2 08/14] iio: accel: adxl345: add freefall feature

[Lothar Rubusch]

Add the freefall detection of the sensor together with a threshold and
time parameter. A freefall event is detected if the measuring signal
falls below the threshold.

Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
---
 drivers/iio/accel/adxl345_core.c | 118 +++++++++++++++++++++++++++++++
 1 file changed, 118 insertions(+)

diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c
index b6966fee3e3d..56c5a4d85d71 100644
--- a/drivers/iio/accel/adxl345_core.c
+++ b/drivers/iio/accel/adxl345_core.c
@@ -84,6 +84,9 @@ struct adxl345_state {
 	u32 tap_window_us;
 	bool tap_suppressed;
 
+	u8 ff_threshold;
+	u32 ff_time_ms;
+
 	__le16 fifo_buf[ADXL345_DIRS * ADXL345_FIFO_SIZE + 1] __aligned(IIO_DMA_MINALIGN);
 };
 
@@ -104,6 +107,14 @@ static struct iio_event_spec adxl345_events[] = {
 			BIT(IIO_EV_INFO_RESET_TIMEOUT) |
 			BIT(IIO_EV_INFO_TAP2_MIN_DELAY),
 	},
+	{
+		/* free fall */
+		.type = IIO_EV_TYPE_MAG,
+		.dir = IIO_EV_DIR_FALLING,
+		.mask_shared_by_type = BIT(IIO_EV_INFO_ENABLE) |
+			BIT(IIO_EV_INFO_VALUE) |
+			BIT(IIO_EV_INFO_PERIOD),
+	},
 };
 
 #define ADXL345_CHANNEL(index, reg, axis) {					\
@@ -354,6 +365,68 @@ static int adxl345_set_tap_latent(struct adxl345_state *st, u32 val_int,
 	return _adxl345_set_tap_time(st, ADXL345_TAP_TIME_LATENT, val_fract_us);
 }
 
+/* ff */
+
+static int adxl345_is_ff_en(struct adxl345_state *st, bool *en)
+{
+	int ret;
+	unsigned int regval;
+
+	ret = regmap_read(st->regmap, ADXL345_REG_INT_ENABLE, &regval);
+	if (ret)
+		return ret;
+
+	*en = FIELD_GET(ADXL345_INT_FREE_FALL, st->int_map) > 0;
+
+	return 0;
+}
+
+static int adxl345_set_ff_en(struct adxl345_state *st, bool cmd_en)
+{
+	bool en = cmd_en && st->ff_threshold > 0 && st->ff_time_ms > 0;
+
+	en ? __set_bit(ilog2(ADXL345_INT_FREE_FALL), (unsigned long *)&st->int_map)
+		: __clear_bit(ilog2(ADXL345_INT_FREE_FALL), (unsigned long *)&st->int_map);
+
+	return regmap_write(st->regmap, ADXL345_REG_INT_ENABLE, st->int_map);
+}
+
+static int adxl345_set_ff_threshold(struct adxl345_state *st, u8 val)
+{
+	int ret;
+
+	ret = regmap_write(st->regmap, ADXL345_REG_THRESH_FF, val);
+	if (ret)
+		return ret;
+
+	st->ff_threshold = val;
+
+	return 0;
+}
+
+static int adxl345_set_ff_time(struct adxl345_state *st, u32 val_int,
+			       u32 val_fract_us)
+{
+	unsigned int regval;
+	int val_ms;
+
+	/*
+	 * max value is 255 * 5000 us = 1.275000 seconds
+	 *
+	 * Note: the scaling is similar to the scaling in the ADXL380
+	 */
+	if (1000000 * val_int + val_fract_us > 1275000)
+		return -EINVAL;
+
+	val_ms = val_int * 1000 + DIV_ROUND_UP(val_fract_us, 1000);
+	st->ff_time_ms = val_ms;
+
+	regval = DIV_ROUND_CLOSEST(val_ms, 5);
+
+	/* Values between 100ms and 350ms (0x14 to 0x46) are recommended. */
+	return regmap_write(st->regmap, ADXL345_REG_TIME_FF, min(regval, 0xff));
+}
+
 static int adxl345_read_raw(struct iio_dev *indio_dev,
 			    struct iio_chan_spec const *chan,
 			    int *val, int *val2, long mask)
@@ -479,6 +552,11 @@ static int adxl345_read_event_config(struct iio_dev *indio_dev,
 		default:
 			return -EINVAL;
 		}
+	case IIO_EV_TYPE_MAG:
+		ret = adxl345_is_ff_en(st, &int_en);
+		if (ret)
+			return ret;
+		return int_en;
 	default:
 		return -EINVAL;
 	}
@@ -517,6 +595,8 @@ static int adxl345_write_event_config(struct iio_dev *indio_dev,
 		default:
 			return -EINVAL;
 		}
+	case IIO_EV_TYPE_MAG:
+		return adxl345_set_ff_en(st, state);
 	default:
 		return -EINVAL;
 	}
@@ -553,6 +633,18 @@ static int adxl345_read_event_value(struct iio_dev *indio_dev, const struct iio_
 		default:
 			return -EINVAL;
 		}
+	case IIO_EV_TYPE_MAG:
+		switch (info) {
+		case IIO_EV_INFO_VALUE:
+			*val = st->ff_threshold;
+			return IIO_VAL_INT;
+		case IIO_EV_INFO_PERIOD:
+			*val = st->ff_time_ms;
+			*val2 = 1000;
+			return IIO_VAL_FRACTIONAL;
+		default:
+			return -EINVAL;
+		}
 	default:
 		return -EINVAL;
 	}
@@ -591,6 +683,18 @@ static int adxl345_write_event_value(struct iio_dev *indio_dev,
 			ret = -EINVAL;
 		}
 		break;
+	case IIO_EV_TYPE_MAG:
+		switch (info) {
+		case IIO_EV_INFO_VALUE:
+			ret = adxl345_set_ff_threshold(st, val);
+			break;
+		case IIO_EV_INFO_PERIOD:
+			ret = adxl345_set_ff_time(st, val, val2);
+			break;
+		default:
+			ret = -EINVAL;
+		}
+		break;
 	default:
 		ret = -EINVAL;
 	}
@@ -934,6 +1038,17 @@ static int adxl345_push_event(struct iio_dev *indio_dev, int int_stat,
 			return ret;
 	}
 
+	if (FIELD_GET(ADXL345_INT_FREE_FALL, int_stat)) {
+		ret = iio_push_event(indio_dev,
+				     IIO_MOD_EVENT_CODE(IIO_ACCEL, 0,
+							IIO_MOD_X_OR_Y_OR_Z,
+							IIO_EV_TYPE_MAG,
+							IIO_EV_DIR_FALLING),
+				     ts);
+		if (ret)
+			return ret;
+	}
+
 	return -ENOENT;
 }
 
@@ -1041,6 +1156,9 @@ int adxl345_core_probe(struct device *dev, struct regmap *regmap,
 	st->tap_window_us = 64;			/*   64 [0x40] -> .080    */
 	st->tap_latent_us = 16;			/*   16 [0x10] -> .020    */
 
+	st->ff_threshold = 8;			/*    8 [0x08]            */
+	st->ff_time_ms = 32;			/*   32 [0x20] -> 0.16    */
+
 	indio_dev->name = st->info->name;
 	indio_dev->info = &adxl345_info;
 	indio_dev->modes = INDIO_DIRECT_MODE;
-- 
2.39.5

Re: [PATCH v2 08/14] iio: accel: adxl345: add freefall feature

[Jonathan Cameron]

On Mon, 10 Feb 2025 11:01:13 +0000
Lothar Rubusch <l.rubusch@gmail.com> wrote:

> Add the freefall detection of the sensor together with a threshold and
> time parameter. A freefall event is detected if the measuring signal
> falls below the threshold.
> 
> Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
Hi Lothar,

A few follow on comments from earlier suggestions.

Jonathan

> ---
>  drivers/iio/accel/adxl345_core.c | 118 +++++++++++++++++++++++++++++++
>  1 file changed, 118 insertions(+)
> 
> diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c
> index b6966fee3e3d..56c5a4d85d71 100644
> --- a/drivers/iio/accel/adxl345_core.c
> +++ b/drivers/iio/accel/adxl345_core.c
> @@ -84,6 +84,9 @@ struct adxl345_state {
>  	u32 tap_window_us;
>  	bool tap_suppressed;
>  
> +	u8 ff_threshold;
> +	u32 ff_time_ms;
> +
>  	__le16 fifo_buf[ADXL345_DIRS * ADXL345_FIFO_SIZE + 1] __aligned(IIO_DMA_MINALIGN);
>  };
>  
> @@ -104,6 +107,14 @@ static struct iio_event_spec adxl345_events[] = {
>  			BIT(IIO_EV_INFO_RESET_TIMEOUT) |
>  			BIT(IIO_EV_INFO_TAP2_MIN_DELAY),
>  	},
> +	{
> +		/* free fall */
> +		.type = IIO_EV_TYPE_MAG,
> +		.dir = IIO_EV_DIR_FALLING,
> +		.mask_shared_by_type = BIT(IIO_EV_INFO_ENABLE) |
> +			BIT(IIO_EV_INFO_VALUE) |
> +			BIT(IIO_EV_INFO_PERIOD),
> +	},
>  };
>  
>  #define ADXL345_CHANNEL(index, reg, axis) {					\
> @@ -354,6 +365,68 @@ static int adxl345_set_tap_latent(struct adxl345_state *st, u32 val_int,
>  	return _adxl345_set_tap_time(st, ADXL345_TAP_TIME_LATENT, val_fract_us);
>  }
>  
> +/* ff */

Spell it out.  Otherwise this comment doesn't add anything given function
names all include ff.

> +
> +static int adxl345_is_ff_en(struct adxl345_state *st, bool *en)
> +{
> +	int ret;
> +	unsigned int regval;
> +
> +	ret = regmap_read(st->regmap, ADXL345_REG_INT_ENABLE, &regval);

Why read into regval then use intmap?
I would just use regval.

> +	if (ret)
> +		return ret;
> +
> +	*en = FIELD_GET(ADXL345_INT_FREE_FALL, st->int_map) > 0;

It's a single bit isn't it?  In which case just set to that value
which will be 1 or 0 anyway.

I'm not sure this function adds enough to be worth keeping.
Maybe just do the regmap_read() and FIELD_GET at the call site.

> +
> +	return 0;
> +}
> +
> +static int adxl345_set_ff_en(struct adxl345_state *st, bool cmd_en)
> +{
> +	bool en = cmd_en && st->ff_threshold > 0 && st->ff_time_ms > 0;
> +
> +	en ? __set_bit(ilog2(ADXL345_INT_FREE_FALL), (unsigned long *)&st->int_map)
> +		: __clear_bit(ilog2(ADXL345_INT_FREE_FALL), (unsigned long *)&st->int_map);
> +
> +	return regmap_write(st->regmap, ADXL345_REG_INT_ENABLE, st->int_map);

As before, if we just use the regmap cache can use the regmap functions
for clearing and setting bits.

> +}
> +
>

[PATCH v2 09/14] iio: accel: adxl345: extend sample frequency adjustments

[Lothar Rubusch]

Introduce enums and functions to work with the sample frequency
adjustments. Let the sample frequency adjust via IIO and configure
a reasonable default.

Replace the old static sample frequency handling. The patch is in
preparation for activity/inactivity handling.

Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
---
 drivers/iio/accel/adxl345.h      |   2 +-
 drivers/iio/accel/adxl345_core.c | 163 ++++++++++++++++++++++++-------
 2 files changed, 127 insertions(+), 38 deletions(-)

diff --git a/drivers/iio/accel/adxl345.h b/drivers/iio/accel/adxl345.h
index bc6d634bd85c..0a549d5898c2 100644
--- a/drivers/iio/accel/adxl345.h
+++ b/drivers/iio/accel/adxl345.h
@@ -69,7 +69,7 @@
  * BW_RATE bits - Bandwidth and output data rate. The default value is
  * 0x0A, which translates to a 100 Hz output data rate
  */
-#define ADXL345_BW_RATE			GENMASK(3, 0)
+#define ADXL345_BW_RATE_MSK		GENMASK(3, 0)
 #define ADXL345_BW_LOW_POWER		BIT(4)
 #define ADXL345_BASE_RATE_NANO_HZ	97656250LL
 
diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c
index 56c5a4d85d71..08ad71875c5e 100644
--- a/drivers/iio/accel/adxl345_core.c
+++ b/drivers/iio/accel/adxl345_core.c
@@ -67,6 +67,45 @@ static const unsigned int adxl345_tap_time_reg[3] = {
 	[ADXL345_TAP_TIME_DUR] = ADXL345_REG_DUR,
 };
 
+enum adxl345_odr {
+	ADXL345_ODR_0P10HZ = 0,
+	ADXL345_ODR_0P20HZ,
+	ADXL345_ODR_0P39HZ,
+	ADXL345_ODR_0P78HZ,
+	ADXL345_ODR_1P56HZ,
+	ADXL345_ODR_3P13HZ,
+	ADXL345_ODR_6P25HZ,
+	ADXL345_ODR_12P50HZ,
+	ADXL345_ODR_25HZ,
+	ADXL345_ODR_50HZ,
+	ADXL345_ODR_100HZ,
+	ADXL345_ODR_200HZ,
+	ADXL345_ODR_400HZ,
+	ADXL345_ODR_800HZ,
+	ADXL345_ODR_1600HZ,
+	ADXL345_ODR_3200HZ,
+};
+
+/* Certain features recommend 12.5 Hz - 400 Hz ODR */
+static const int adxl345_odr_tbl[][2] = {
+	[ADXL345_ODR_0P10HZ]	= {   0,  97000},
+	[ADXL345_ODR_0P20HZ]	= {   0, 195000},
+	[ADXL345_ODR_0P39HZ]	= {   0, 390000},
+	[ADXL345_ODR_0P78HZ]	= {   0, 781000},
+	[ADXL345_ODR_1P56HZ]	= {   1, 562000},
+	[ADXL345_ODR_3P13HZ]	= {   3, 125000},
+	[ADXL345_ODR_6P25HZ]	= {   6, 250000},
+	[ADXL345_ODR_12P50HZ]	= {  12, 500000},
+	[ADXL345_ODR_25HZ]	= {  25, 0},
+	[ADXL345_ODR_50HZ]	= {  50, 0},
+	[ADXL345_ODR_100HZ]	= { 100, 0},
+	[ADXL345_ODR_200HZ]	= { 200, 0},
+	[ADXL345_ODR_400HZ]	= { 400, 0},
+	[ADXL345_ODR_800HZ]	= { 800, 0},
+	[ADXL345_ODR_1600HZ]	= {1600, 0},
+	[ADXL345_ODR_3200HZ]	= {3200, 0},
+};
+
 struct adxl345_state {
 	const struct adxl345_chip_info *info;
 	struct regmap *regmap;
@@ -77,6 +116,8 @@ struct adxl345_state {
 	u8 watermark;
 	u8 fifo_mode;
 
+	enum adxl345_odr odr;
+
 	u32 tap_axis_ctrl;
 	u8 tap_threshold;
 	u32 tap_duration_us;
@@ -126,6 +167,7 @@ static struct iio_event_spec adxl345_events[] = {
 		BIT(IIO_CHAN_INFO_CALIBBIAS),				\
 	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |		\
 		BIT(IIO_CHAN_INFO_SAMP_FREQ),				\
+	.info_mask_shared_by_type_available = BIT(IIO_CHAN_INFO_SAMP_FREQ),		\
 	.scan_index = (index),				\
 	.scan_type = {					\
 		.sign = 's',				\
@@ -427,13 +469,61 @@ static int adxl345_set_ff_time(struct adxl345_state *st, u32 val_int,
 	return regmap_write(st->regmap, ADXL345_REG_TIME_FF, min(regval, 0xff));
 }
 
+static int adxl345_find_odr(struct adxl345_state *st, int val,
+			    int val2, enum adxl345_odr *odr)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(adxl345_odr_tbl); i++)
+		if (val == adxl345_odr_tbl[i][0] &&
+		    val2 == adxl345_odr_tbl[i][1])
+			break;
+
+	if (i == ARRAY_SIZE(adxl345_odr_tbl))
+		return -EINVAL;
+
+	*odr = i;
+
+	return 0;
+}
+
+static int adxl345_set_odr(struct adxl345_state *st, enum adxl345_odr odr)
+{
+	int ret;
+
+	ret = regmap_update_bits(st->regmap, ADXL345_REG_BW_RATE,
+				 ADXL345_BW_RATE_MSK,
+				 FIELD_PREP(ADXL345_BW_RATE_MSK, odr));
+	if (ret)
+		return ret;
+
+	st->odr = odr;
+
+	return 0;
+}
+
+static int adxl345_read_avail(struct iio_dev *indio_dev,
+			      struct iio_chan_spec const *chan,
+			      const int **vals, int *type,
+			      int *length, long mask)
+{
+	switch (mask) {
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		*vals = (int *)adxl345_odr_tbl;
+		*type = IIO_VAL_INT_PLUS_MICRO;
+		*length = ARRAY_SIZE(adxl345_odr_tbl) * 2;
+		return IIO_AVAIL_LIST;
+	}
+
+	return -EINVAL;
+}
+
 static int adxl345_read_raw(struct iio_dev *indio_dev,
 			    struct iio_chan_spec const *chan,
 			    int *val, int *val2, long mask)
 {
 	struct adxl345_state *st = iio_priv(indio_dev);
 	__le16 accel;
-	long long samp_freq_nhz;
 	unsigned int regval;
 	int ret;
 
@@ -469,15 +559,9 @@ static int adxl345_read_raw(struct iio_dev *indio_dev,
 
 		return IIO_VAL_INT;
 	case IIO_CHAN_INFO_SAMP_FREQ:
-		ret = regmap_read(st->regmap, ADXL345_REG_BW_RATE, &regval);
-		if (ret < 0)
-			return ret;
-
-		samp_freq_nhz = ADXL345_BASE_RATE_NANO_HZ <<
-				(regval & ADXL345_BW_RATE);
-		*val = div_s64_rem(samp_freq_nhz, NANOHZ_PER_HZ, val2);
-
-		return IIO_VAL_INT_PLUS_NANO;
+		*val = adxl345_odr_tbl[st->odr][0];
+		*val2 = adxl345_odr_tbl[st->odr][1];
+		return IIO_VAL_INT_PLUS_MICRO;
 	}
 
 	return -EINVAL;
@@ -488,7 +572,12 @@ static int adxl345_write_raw(struct iio_dev *indio_dev,
 			     int val, int val2, long mask)
 {
 	struct adxl345_state *st = iio_priv(indio_dev);
-	s64 n;
+	enum adxl345_odr odr;
+	int ret;
+
+	ret = adxl345_set_measure_en(st, false);
+	if (ret)
+		return ret;
 
 	switch (mask) {
 	case IIO_CHAN_INFO_CALIBBIAS:
@@ -496,20 +585,24 @@ static int adxl345_write_raw(struct iio_dev *indio_dev,
 		 * 8-bit resolution at +/- 2g, that is 4x accel data scale
 		 * factor
 		 */
-		return regmap_write(st->regmap,
-				    ADXL345_REG_OFS_AXIS(chan->address),
-				    val / 4);
+		ret = regmap_write(st->regmap,
+				   ADXL345_REG_OFS_AXIS(chan->address),
+				   val / 4);
+		break;
 	case IIO_CHAN_INFO_SAMP_FREQ:
-		n = div_s64(val * NANOHZ_PER_HZ + val2,
-			    ADXL345_BASE_RATE_NANO_HZ);
-
-		return regmap_update_bits(st->regmap, ADXL345_REG_BW_RATE,
-					  ADXL345_BW_RATE,
-					  clamp_val(ilog2(n), 0,
-						    ADXL345_BW_RATE));
+		ret = adxl345_find_odr(st, val, val2, &odr);
+		if (ret)
+			return ret;
+		ret = adxl345_set_odr(st, odr);
+		break;
+	default:
+		return -EINVAL;
 	}
 
-	return -EINVAL;
+	if (ret)
+		return ret;
+
+	return adxl345_set_measure_en(st, true);
 }
 
 static int adxl345_read_event_config(struct iio_dev *indio_dev,
@@ -741,7 +834,7 @@ static int adxl345_write_raw_get_fmt(struct iio_dev *indio_dev,
 	case IIO_CHAN_INFO_CALIBBIAS:
 		return IIO_VAL_INT;
 	case IIO_CHAN_INFO_SAMP_FREQ:
-		return IIO_VAL_INT_PLUS_NANO;
+		return IIO_VAL_INT_PLUS_MICRO;
 	default:
 		return -EINVAL;
 	}
@@ -808,19 +901,6 @@ static void adxl345_powerdown(void *ptr)
 	adxl345_set_measure_en(st, false);
 }
 
-static IIO_CONST_ATTR_SAMP_FREQ_AVAIL(
-"0.09765625 0.1953125 0.390625 0.78125 1.5625 3.125 6.25 12.5 25 50 100 200 400 800 1600 3200"
-);
-
-static struct attribute *adxl345_attrs[] = {
-	&iio_const_attr_sampling_frequency_available.dev_attr.attr,
-	NULL
-};
-
-static const struct attribute_group adxl345_attrs_group = {
-	.attrs = adxl345_attrs,
-};
-
 static int adxl345_set_fifo(struct adxl345_state *st)
 {
 	int ret;
@@ -1094,10 +1174,10 @@ static irqreturn_t adxl345_irq_handler(int irq, void *p)
 }
 
 static const struct iio_info adxl345_info = {
-	.attrs		= &adxl345_attrs_group,
 	.event_attrs	= &adxl345_event_attrs_group,
 	.read_raw	= adxl345_read_raw,
 	.write_raw	= adxl345_write_raw,
+	.read_avail	= adxl345_read_avail,
 	.write_raw_get_fmt	= adxl345_write_raw_get_fmt,
 	.read_event_config = adxl345_read_event_config,
 	.write_event_config = adxl345_write_event_config,
@@ -1166,6 +1246,15 @@ int adxl345_core_probe(struct device *dev, struct regmap *regmap,
 	indio_dev->num_channels = ARRAY_SIZE(adxl345_channels);
 	indio_dev->available_scan_masks = adxl345_scan_masks;
 
+	/*
+	 * Using I2C at 100kHz would limit the maximum ODR to 200Hz, operation
+	 * at an output rate above the recommended maximum may result in
+	 * undesired behavior.
+	 */
+	ret = adxl345_set_odr(st, ADXL345_ODR_200HZ);
+	if (ret)
+		return ret;
+
 	/* Reset interrupts at start up */
 	ret = regmap_write(st->regmap, ADXL345_REG_INT_ENABLE, st->int_map);
 	if (ret)
-- 
2.39.5

Re: [PATCH v2 09/14] iio: accel: adxl345: extend sample frequency adjustments

[Jonathan Cameron]

On Mon, 10 Feb 2025 11:01:14 +0000
Lothar Rubusch <l.rubusch@gmail.com> wrote:

> Introduce enums and functions to work with the sample frequency
> adjustments. Let the sample frequency adjust via IIO and configure
> a reasonable default.
> 
> Replace the old static sample frequency handling. The patch is in
> preparation for activity/inactivity handling.
> 
> Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>

I noticed now that we don't actually have regmap caching enabled.
I think it will make things sufficiently simpler that it is worth
doing even though you have to specify which registers are volatile etc.


> +
> +static int adxl345_set_odr(struct adxl345_state *st, enum adxl345_odr odr)
> +{
> +	int ret;
> +
> +	ret = regmap_update_bits(st->regmap, ADXL345_REG_BW_RATE,
> +				 ADXL345_BW_RATE_MSK,
> +				 FIELD_PREP(ADXL345_BW_RATE_MSK, odr));
> +	if (ret)
> +		return ret;
> +
> +	st->odr = odr;
Why do we need to keep a copy cached? Seems like we can get it from
regmap cache easily enough?

> +
> +	return 0;
> +}

> @@ -488,7 +572,12 @@ static int adxl345_write_raw(struct iio_dev *indio_dev,
>  			     int val, int val2, long mask)
>  {
>  	struct adxl345_state *st = iio_priv(indio_dev);
> -	s64 n;
> +	enum adxl345_odr odr;
> +	int ret;
> +
> +	ret = adxl345_set_measure_en(st, false);

Please add some more on why this is now necessary to the patch description.

> +	if (ret)
> +		return ret;
>  
>  	switch (mask) {
>  	case IIO_CHAN_INFO_CALIBBIAS:
> @@ -496,20 +585,24 @@ static int adxl345_write_raw(struct iio_dev *indio_dev,
>  		 * 8-bit resolution at +/- 2g, that is 4x accel data scale
>  		 * factor
>  		 */
> -		return regmap_write(st->regmap,
> -				    ADXL345_REG_OFS_AXIS(chan->address),
> -				    val / 4);
> +		ret = regmap_write(st->regmap,
> +				   ADXL345_REG_OFS_AXIS(chan->address),
> +				   val / 4);
> +		break;
>  	case IIO_CHAN_INFO_SAMP_FREQ:
> -		n = div_s64(val * NANOHZ_PER_HZ + val2,
> -			    ADXL345_BASE_RATE_NANO_HZ);
> -
> -		return regmap_update_bits(st->regmap, ADXL345_REG_BW_RATE,
> -					  ADXL345_BW_RATE,
> -					  clamp_val(ilog2(n), 0,
> -						    ADXL345_BW_RATE));
> +		ret = adxl345_find_odr(st, val, val2, &odr);
> +		if (ret)
> +			return ret;
> +		ret = adxl345_set_odr(st, odr);
> +		break;
> +	default:
> +		return -EINVAL;
>  	}
>  
> -	return -EINVAL;
> +	if (ret)
> +		return ret;
> +
> +	return adxl345_set_measure_en(st, true);
>  }

[PATCH v2 10/14] iio: accel: adxl345: add g-range configuration

[Lothar Rubusch]

Introduce means to configure and work with the available g-ranges
keeping the precision of 13 digits.

This is in preparation for the activity/inactivity feature.

Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
---
 drivers/iio/accel/adxl345_core.c | 92 ++++++++++++++++++++++++++++++--
 1 file changed, 89 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c
index 08ad71875c5e..ea7bfe193d31 100644
--- a/drivers/iio/accel/adxl345_core.c
+++ b/drivers/iio/accel/adxl345_core.c
@@ -86,6 +86,13 @@ enum adxl345_odr {
 	ADXL345_ODR_3200HZ,
 };
 
+enum adxl345_range {
+	ADXL345_2G_RANGE = 0,
+	ADXL345_4G_RANGE,
+	ADXL345_8G_RANGE,
+	ADXL345_16G_RANGE,
+};
+
 /* Certain features recommend 12.5 Hz - 400 Hz ODR */
 static const int adxl345_odr_tbl[][2] = {
 	[ADXL345_ODR_0P10HZ]	= {   0,  97000},
@@ -106,6 +113,33 @@ static const int adxl345_odr_tbl[][2] = {
 	[ADXL345_ODR_3200HZ]	= {3200, 0},
 };
 
+/*
+ * Full resolution frequency table:
+ * (g * 2 * 9.80665) / (2^(resolution) - 1)
+ *
+ * resolution := 13 (full)
+ * g := 2|4|8|16
+ *
+ *  2g at 13bit: 0.004789
+ *  4g at 13bit: 0.009578
+ *  8g at 13bit: 0.019156
+ * 16g at 16bit: 0.038312
+ */
+static const int adxl345_fullres_range_tbl[][2] = {
+	[ADXL345_2G_RANGE]  = {0, 4789},
+	[ADXL345_4G_RANGE]  = {0, 9578},
+	[ADXL345_8G_RANGE]  = {0, 19156},
+	[ADXL345_16G_RANGE] = {0, 38312},
+};
+
+/* scaling */
+static const int adxl345_range_factor_tbl[] = {
+	[ADXL345_2G_RANGE]  = 1,
+	[ADXL345_4G_RANGE]  = 2,
+	[ADXL345_8G_RANGE]  = 4,
+	[ADXL345_16G_RANGE] = 8,
+};
+
 struct adxl345_state {
 	const struct adxl345_chip_info *info;
 	struct regmap *regmap;
@@ -117,6 +151,7 @@ struct adxl345_state {
 	u8 fifo_mode;
 
 	enum adxl345_odr odr;
+	enum adxl345_range range;
 
 	u32 tap_axis_ctrl;
 	u8 tap_threshold;
@@ -167,7 +202,8 @@ static struct iio_event_spec adxl345_events[] = {
 		BIT(IIO_CHAN_INFO_CALIBBIAS),				\
 	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |		\
 		BIT(IIO_CHAN_INFO_SAMP_FREQ),				\
-	.info_mask_shared_by_type_available = BIT(IIO_CHAN_INFO_SAMP_FREQ),		\
+	.info_mask_shared_by_type_available = BIT(IIO_CHAN_INFO_SCALE) | \
+		BIT(IIO_CHAN_INFO_SAMP_FREQ),		\
 	.scan_index = (index),				\
 	.scan_type = {					\
 		.sign = 's',				\
@@ -502,12 +538,50 @@ static int adxl345_set_odr(struct adxl345_state *st, enum adxl345_odr odr)
 	return 0;
 }
 
+static int adxl345_find_range(struct adxl345_state *st, int val, int val2,
+			      enum adxl345_range *range)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(adxl345_fullres_range_tbl); i++)
+		if (val == adxl345_fullres_range_tbl[i][0] &&
+		    val2 == adxl345_fullres_range_tbl[i][1])
+			break;
+
+	if (i == ARRAY_SIZE(adxl345_fullres_range_tbl))
+		return -EINVAL;
+
+	*range = i;
+
+	return 0;
+}
+
+static int adxl345_set_range(struct adxl345_state *st, enum adxl345_range range)
+{
+	int ret;
+
+	ret = regmap_update_bits(st->regmap, ADXL345_REG_DATA_FORMAT,
+				 ADXL345_DATA_FORMAT_RANGE,
+				 FIELD_PREP(ADXL345_DATA_FORMAT_RANGE, range));
+	if (ret)
+		return ret;
+
+	st->range = range;
+
+	return 0;
+}
+
 static int adxl345_read_avail(struct iio_dev *indio_dev,
 			      struct iio_chan_spec const *chan,
 			      const int **vals, int *type,
 			      int *length, long mask)
 {
 	switch (mask) {
+	case IIO_CHAN_INFO_SCALE:
+		*vals = (int *)adxl345_fullres_range_tbl;
+		*type = IIO_VAL_INT_PLUS_MICRO;
+		*length = ARRAY_SIZE(adxl345_fullres_range_tbl) * 2;
+		return IIO_AVAIL_LIST;
 	case IIO_CHAN_INFO_SAMP_FREQ:
 		*vals = (int *)adxl345_odr_tbl;
 		*type = IIO_VAL_INT_PLUS_MICRO;
@@ -543,8 +617,8 @@ static int adxl345_read_raw(struct iio_dev *indio_dev,
 		*val = sign_extend32(le16_to_cpu(accel), 12);
 		return IIO_VAL_INT;
 	case IIO_CHAN_INFO_SCALE:
-		*val = 0;
-		*val2 = st->info->uscale;
+		*val = adxl345_fullres_range_tbl[st->range][0];
+		*val2 = adxl345_fullres_range_tbl[st->range][1];
 		return IIO_VAL_INT_PLUS_MICRO;
 	case IIO_CHAN_INFO_CALIBBIAS:
 		ret = regmap_read(st->regmap,
@@ -572,6 +646,7 @@ static int adxl345_write_raw(struct iio_dev *indio_dev,
 			     int val, int val2, long mask)
 {
 	struct adxl345_state *st = iio_priv(indio_dev);
+	enum adxl345_range range;
 	enum adxl345_odr odr;
 	int ret;
 
@@ -595,6 +670,12 @@ static int adxl345_write_raw(struct iio_dev *indio_dev,
 			return ret;
 		ret = adxl345_set_odr(st, odr);
 		break;
+	case IIO_CHAN_INFO_SCALE:
+		ret = adxl345_find_range(st, val, val2,	&range);
+		if (ret)
+			return ret;
+		ret = adxl345_set_range(st, range);
+		break;
 	default:
 		return -EINVAL;
 	}
@@ -833,6 +914,8 @@ static int adxl345_write_raw_get_fmt(struct iio_dev *indio_dev,
 	switch (mask) {
 	case IIO_CHAN_INFO_CALIBBIAS:
 		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_SCALE:
+		return IIO_VAL_INT_PLUS_MICRO;
 	case IIO_CHAN_INFO_SAMP_FREQ:
 		return IIO_VAL_INT_PLUS_MICRO;
 	default:
@@ -1252,6 +1335,9 @@ int adxl345_core_probe(struct device *dev, struct regmap *regmap,
 	 * undesired behavior.
 	 */
 	ret = adxl345_set_odr(st, ADXL345_ODR_200HZ);
+	if (ret)
+		return ret;
+	ret = adxl345_set_range(st, ADXL345_16G_RANGE);
 	if (ret)
 		return ret;
 
-- 
2.39.5

Re: [PATCH v2 10/14] iio: accel: adxl345: add g-range configuration

[Jonathan Cameron]

On Mon, 10 Feb 2025 11:01:15 +0000
Lothar Rubusch <l.rubusch@gmail.com> wrote:

> Introduce means to configure and work with the available g-ranges
> keeping the precision of 13 digits.
> 
> This is in preparation for the activity/inactivity feature.
> 
> Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
A few really trivial comments on this one.

> ---
>  drivers/iio/accel/adxl345_core.c | 92 ++++++++++++++++++++++++++++++--
>  1 file changed, 89 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c
> index 08ad71875c5e..ea7bfe193d31 100644
> --- a/drivers/iio/accel/adxl345_core.c
> +++ b/drivers/iio/accel/adxl345_core.c
> @@ -86,6 +86,13 @@ enum adxl345_odr {
>  	ADXL345_ODR_3200HZ,
>  };
>  
> +enum adxl345_range {
> +	ADXL345_2G_RANGE = 0,
> +	ADXL345_4G_RANGE,
> +	ADXL345_8G_RANGE,
> +	ADXL345_16G_RANGE,
> +};
> +
>  /* Certain features recommend 12.5 Hz - 400 Hz ODR */
>  static const int adxl345_odr_tbl[][2] = {
>  	[ADXL345_ODR_0P10HZ]	= {   0,  97000},
> @@ -106,6 +113,33 @@ static const int adxl345_odr_tbl[][2] = {
>  	[ADXL345_ODR_3200HZ]	= {3200, 0},
>  };
>  
> +/*
> + * Full resolution frequency table:
> + * (g * 2 * 9.80665) / (2^(resolution) - 1)
> + *
> + * resolution := 13 (full)
> + * g := 2|4|8|16
> + *
> + *  2g at 13bit: 0.004789
> + *  4g at 13bit: 0.009578
> + *  8g at 13bit: 0.019156
> + * 16g at 16bit: 0.038312
> + */
> +static const int adxl345_fullres_range_tbl[][2] = {
> +	[ADXL345_2G_RANGE]  = {0, 4789},

I'm slowly trying to standardize a few more corners of the kernel style for IIO at least.
This hits one of them. So if you don't mind adding spaces after { and before }
it will one day save me time on a cleanup series.

> +	[ADXL345_4G_RANGE]  = {0, 9578},
> +	[ADXL345_8G_RANGE]  = {0, 19156},
> +	[ADXL345_16G_RANGE] = {0, 38312},
> +};


> @@ -1252,6 +1335,9 @@ int adxl345_core_probe(struct device *dev, struct regmap *regmap,
>  	 * undesired behavior.
>  	 */
>  	ret = adxl345_set_odr(st, ADXL345_ODR_200HZ);
> +	if (ret)
> +		return ret;

Trivial but I'd put a blank line here for slightly improved readability.

> +	ret = adxl345_set_range(st, ADXL345_16G_RANGE);
>  	if (ret)
>  		return ret;
>  

[PATCH v2 11/14] iio: accel: adxl345: add activity event feature

[Lothar Rubusch]

Make the sensor detect and issue interrupts at activity. Activity
events are configured by a threshold.

Activity, together with ODR and range setting are preparing a setup
together with inactivity coming in a follow up patch.

Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
---
 drivers/iio/accel/adxl345_core.c | 172 ++++++++++++++++++++++++++++++-
 1 file changed, 171 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c
index ea7bfe193d31..16dea2a222d9 100644
--- a/drivers/iio/accel/adxl345_core.c
+++ b/drivers/iio/accel/adxl345_core.c
@@ -35,6 +35,7 @@
 
 #define ADXL345_REG_TAP_AXIS_MSK	GENMASK(2, 0)
 #define ADXL345_REG_TAP_SUPPRESS_MSK	BIT(3)
+#define ADXL345_REG_ACT_AXIS_MSK	GENMASK(6, 4)
 
 enum adxl345_axis {
 	ADXL345_Z_EN = BIT(0),
@@ -67,6 +68,23 @@ static const unsigned int adxl345_tap_time_reg[3] = {
 	[ADXL345_TAP_TIME_DUR] = ADXL345_REG_DUR,
 };
 
+/* activity/inactivity */
+enum adxl345_activity_type {
+	ADXL345_ACTIVITY,
+};
+
+static const unsigned int adxl345_act_int_reg[2] = {
+	[ADXL345_ACTIVITY] = ADXL345_INT_ACTIVITY,
+};
+
+static const unsigned int adxl345_act_thresh_reg[2] = {
+	[ADXL345_ACTIVITY] = ADXL345_REG_THRESH_ACT,
+};
+
+static const unsigned int adxl345_act_axis_msk[2] = {
+	[ADXL345_ACTIVITY] = ADXL345_REG_ACT_AXIS_MSK,
+};
+
 enum adxl345_odr {
 	ADXL345_ODR_0P10HZ = 0,
 	ADXL345_ODR_0P20HZ,
@@ -153,6 +171,9 @@ struct adxl345_state {
 	enum adxl345_odr odr;
 	enum adxl345_range range;
 
+	u32 act_axis_ctrl;
+	u8 act_threshold;
+
 	u32 tap_axis_ctrl;
 	u8 tap_threshold;
 	u32 tap_duration_us;
@@ -167,6 +188,13 @@ struct adxl345_state {
 };
 
 static struct iio_event_spec adxl345_events[] = {
+	{
+		/* activity */
+		.type = IIO_EV_TYPE_THRESH,
+		.dir = IIO_EV_DIR_RISING,
+		.mask_shared_by_type = BIT(IIO_EV_INFO_ENABLE) |
+			BIT(IIO_EV_INFO_VALUE),
+	},
 	{
 		/* single tap */
 		.type = IIO_EV_TYPE_GESTURE,
@@ -250,6 +278,84 @@ static int adxl345_set_measure_en(struct adxl345_state *st, bool en)
 	return regmap_write(st->regmap, ADXL345_REG_POWER_CTL, val);
 }
 
+/* act/inact */
+
+static int adxl345_write_act_axis(struct adxl345_state *st,
+				  enum adxl345_activity_type type, bool en)
+{
+	int ret;
+
+	/*
+	 * The ADXL345 allows for individually enabling/disabling axis for
+	 * activity and inactivity detection, respectively. Here both axis are
+	 * kept in sync, i.e. an axis will be generally enabled or disabled for
+	 * both equally, activity and inactivity detection.
+	 */
+	if (type == ADXL345_ACTIVITY) {
+		st->act_axis_ctrl = en
+			? st->act_axis_ctrl | ADXL345_REG_ACT_AXIS_MSK
+			: st->act_axis_ctrl & ~ADXL345_REG_ACT_AXIS_MSK;
+
+		ret = regmap_update_bits(st->regmap, ADXL345_REG_ACT_INACT_CTRL,
+					 adxl345_act_axis_msk[type],
+					 st->act_axis_ctrl);
+		if (ret)
+			return ret;
+	}
+	return 0;
+}
+
+static int adxl345_is_act_inact_en(struct adxl345_state *st,
+				   enum adxl345_activity_type type, bool *en)
+{
+	int ret;
+	unsigned int regval;
+
+	ret = regmap_read(st->regmap, ADXL345_REG_INT_ENABLE, &regval);
+	if (ret)
+		return ret;
+
+	*en = (adxl345_act_int_reg[type] & regval) > 0;
+
+	return 0;
+}
+
+static int adxl345_set_act_inact_en(struct adxl345_state *st,
+				    enum adxl345_activity_type type, bool cmd_en)
+{
+	bool axis_en, en = false;
+	int ret;
+
+	ret = adxl345_write_act_axis(st, type, cmd_en);
+	if (ret)
+		return ret;
+
+	if (type == ADXL345_ACTIVITY) {
+		axis_en = FIELD_GET(ADXL345_REG_ACT_AXIS_MSK, st->act_axis_ctrl) > 0;
+		en = axis_en && st->act_threshold > 0;
+	}
+
+	en ? __set_bit(ilog2(adxl345_act_int_reg[type]), (unsigned long *)&st->int_map)
+		: __clear_bit(ilog2(adxl345_act_int_reg[type]), (unsigned long *)&st->int_map);
+
+	return regmap_write(st->regmap, ADXL345_REG_INT_ENABLE, st->int_map);
+}
+
+static int adxl345_set_act_inact_threshold(struct adxl345_state *st,
+					   enum adxl345_activity_type type, u8 val)
+{
+	int ret;
+
+	ret = regmap_write(st->regmap, adxl345_act_thresh_reg[type], val);
+	if (ret)
+		return ret;
+
+	if (type == ADXL345_ACTIVITY)
+		st->act_threshold = val;
+
+	return 0;
+}
+
 /* tap */
 
 static int adxl345_write_tap_axis(struct adxl345_state *st,
@@ -697,6 +803,16 @@ static int adxl345_read_event_config(struct iio_dev *indio_dev,
 	int ret = -EFAULT;
 
 	switch (type) {
+	case IIO_EV_TYPE_THRESH:
+		switch (dir) {
+		case IIO_EV_DIR_RISING:
+			ret = adxl345_is_act_inact_en(st, ADXL345_ACTIVITY, &int_en);
+			if (ret)
+				return ret;
+			return int_en;
+		default:
+			return -EINVAL;
+		}
 	case IIO_EV_TYPE_GESTURE:
 		switch (chan->channel2) {
 		case IIO_MOD_X:
@@ -746,6 +862,13 @@ static int adxl345_write_event_config(struct iio_dev *indio_dev,
 	enum adxl345_axis axis;
 
 	switch (type) {
+	case IIO_EV_TYPE_THRESH:
+		switch (dir) {
+		case IIO_EV_DIR_RISING:
+			return adxl345_set_act_inact_en(st, ADXL345_ACTIVITY, state);
+		default:
+			return -EINVAL;
+		}
 	case IIO_EV_TYPE_GESTURE:
 		switch (chan->channel2) {
 		case IIO_MOD_X:
@@ -783,6 +906,19 @@ static int adxl345_read_event_value(struct iio_dev *indio_dev, const struct iio_
 	struct adxl345_state *st = iio_priv(indio_dev);
 
 	switch (type) {
+	case IIO_EV_TYPE_THRESH:
+		switch (info) {
+		case IIO_EV_INFO_VALUE:
+			switch (dir) {
+			case IIO_EV_DIR_RISING:
+				*val = st->act_threshold;
+				return IIO_VAL_INT;
+			default:
+				return -EINVAL;
+			}
+		default:
+			return -EINVAL;
+		}
 	case IIO_EV_TYPE_GESTURE:
 		switch (info) {
 		case IIO_EV_INFO_VALUE:
@@ -839,6 +975,21 @@ static int adxl345_write_event_value(struct iio_dev *indio_dev,
 		return ret;
 
 	switch (type) {
+	case IIO_EV_TYPE_THRESH:
+		switch (info) {
+		case IIO_EV_INFO_VALUE:
+			switch (dir) {
+			case IIO_EV_DIR_RISING:
+				ret = adxl345_set_act_inact_threshold(st, ADXL345_ACTIVITY, val);
+				break;
+			default:
+				ret = -EINVAL;
+			}
+			break;
+		default:
+			ret = -EINVAL;
+		}
+		break;
 	case IIO_EV_TYPE_GESTURE:
 		switch (info) {
 		case IIO_EV_INFO_VALUE:
@@ -1129,12 +1280,14 @@ static int adxl345_get_status(struct adxl345_state *st, unsigned int *int_stat,
 {
 	unsigned int regval;
 	bool check_tap_stat;
+	bool check_act_stat;
 	int ret;
 
 	*act_tap_dir = IIO_NO_MOD;
 	check_tap_stat = FIELD_GET(ADXL345_REG_TAP_AXIS_MSK, st->tap_axis_ctrl) > 0;
+	check_act_stat = FIELD_GET(ADXL345_REG_ACT_AXIS_MSK, st->act_axis_ctrl) > 0;
 
-	if (check_tap_stat) {
+	if (check_tap_stat || check_act_stat) {
 		/* ACT_TAP_STATUS should be read before clearing the interrupt */
 		ret = regmap_read(st->regmap, ADXL345_REG_ACT_TAP_STATUS, &regval);
 		if (ret)
@@ -1201,6 +1354,17 @@ static int adxl345_push_event(struct iio_dev *indio_dev, int int_stat,
 			return ret;
 	}
 
+	if (FIELD_GET(ADXL345_INT_ACTIVITY, int_stat)) {
+		ret = iio_push_event(indio_dev,
+				     IIO_MOD_EVENT_CODE(IIO_ACCEL, 0,
+							act_tap_dir,
+							IIO_EV_TYPE_THRESH,
+							IIO_EV_DIR_RISING),
+				     ts);
+		if (ret)
+			return ret;
+	}
+
 	if (FIELD_GET(ADXL345_INT_FREE_FALL, int_stat)) {
 		ret = iio_push_event(indio_dev,
 				     IIO_MOD_EVENT_CODE(IIO_ACCEL, 0,
@@ -1311,6 +1475,12 @@ int adxl345_core_probe(struct device *dev, struct regmap *regmap,
 		return -ENODEV;
 	st->fifo_delay = fifo_delay_default;
 
+	/*
+	 * If the feature is enabled, scan all axis for activity and or
+	 * inactivity, and set activity and inactivity to the same ac / dc
+	 * setup.
+	 */
+	st->act_axis_ctrl = ADXL345_REG_ACT_AXIS_MSK;
 	st->int_map = 0x00;			/* reset interrupts */
 
 	/* Init with reasonable values */
-- 
2.39.5

Re: [PATCH v2 11/14] iio: accel: adxl345: add activity event feature

[Jonathan Cameron]

On Mon, 10 Feb 2025 11:01:16 +0000
Lothar Rubusch <l.rubusch@gmail.com> wrote:

> Make the sensor detect and issue interrupts at activity. Activity
> events are configured by a threshold.
> 
> Activity, together with ODR and range setting are preparing a setup
> together with inactivity coming in a follow up patch.
> 
> Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
> ---
>  drivers/iio/accel/adxl345_core.c | 172 ++++++++++++++++++++++++++++++-
>  1 file changed, 171 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c
> index ea7bfe193d31..16dea2a222d9 100644
> --- a/drivers/iio/accel/adxl345_core.c
> +++ b/drivers/iio/accel/adxl345_core.c
> @@ -35,6 +35,7 @@
>  
>  #define ADXL345_REG_TAP_AXIS_MSK	GENMASK(2, 0)
>  #define ADXL345_REG_TAP_SUPPRESS_MSK	BIT(3)
> +#define ADXL345_REG_ACT_AXIS_MSK	GENMASK(6, 4)
>  
>  enum adxl345_axis {
>  	ADXL345_Z_EN = BIT(0),
> @@ -67,6 +68,23 @@ static const unsigned int adxl345_tap_time_reg[3] = {
>  	[ADXL345_TAP_TIME_DUR] = ADXL345_REG_DUR,
>  };
>  
> +/* activity/inactivity */
> +enum adxl345_activity_type {
> +	ADXL345_ACTIVITY,
> +};
> +
> +static const unsigned int adxl345_act_int_reg[2] = {
> +	[ADXL345_ACTIVITY] = ADXL345_INT_ACTIVITY,
As before [] for these to avoid having to set to 1 here and
update in next patch.

> +};
> +
> +static const unsigned int adxl345_act_thresh_reg[2] = {
> +	[ADXL345_ACTIVITY] = ADXL345_REG_THRESH_ACT,
> +};
> +
> +static const unsigned int adxl345_act_axis_msk[2] = {
> +	[ADXL345_ACTIVITY] = ADXL345_REG_ACT_AXIS_MSK,
> +};

> +
> +static int adxl345_set_act_inact_en(struct adxl345_state *st,
> +				    enum adxl345_activity_type type, bool cmd_en)
> +{
> +	bool axis_en, en = false;
> +	int ret;
> +
> +	ret = adxl345_write_act_axis(st, type, cmd_en);
> +	if (ret)
> +		return ret;
> +
> +	if (type == ADXL345_ACTIVITY) {
> +		axis_en = FIELD_GET(ADXL345_REG_ACT_AXIS_MSK, st->act_axis_ctrl) > 0;
> +		en = axis_en && st->act_threshold > 0;
> +	}
> +
> +	en ? __set_bit(ilog2(adxl345_act_int_reg[type]), (unsigned long *)&st->int_map)
> +		: __clear_bit(ilog2(adxl345_act_int_reg[type]), (unsigned long *)&st->int_map);

Similar comment to earlier on __set_bit()

[PATCH v2 12/14] iio: accel: adxl345: add inactivity feature

[Lothar Rubusch]

Add the inactivity feature of the sensor. When activity and inactivity
is enabled, a link bit will be set linking activity and inactivity
handling. Additionally, the auto-sleep mode will be enabled. Due to the
link bit the sensor is going to auto-sleep when inactivity was
detected.

Inactivity detection needs a threshold to be configured, and a time
after which it will go into inactivity state if measurements under
threshold.

When a ODR is configured this time for inactivity is adjusted with a
corresponding reasonable default value, in order to have higher
frequencies and lower inactivity times, and lower sample frequency but
give more time until inactivity. Both with reasonable upper and lower
boundaries, since many of the sensor's features (e.g. auto-sleep) will
need to operate beween 12.5 Hz and 400 Hz. This is a default setting
when actively changing sample frequency, explicitly setting the time
until inactivity will overwrite the default.

Similarly, setting the g-range will provide a default value for the
activity and inactivity thresholds. Both are implicit defaults, but
equally can be overwritten to be explicitly configured.

Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
---
 drivers/iio/accel/adxl345_core.c | 140 ++++++++++++++++++++++++++++++-
 1 file changed, 139 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c
index 16dea2a222d9..7de869fac799 100644
--- a/drivers/iio/accel/adxl345_core.c
+++ b/drivers/iio/accel/adxl345_core.c
@@ -36,6 +36,8 @@
 #define ADXL345_REG_TAP_AXIS_MSK	GENMASK(2, 0)
 #define ADXL345_REG_TAP_SUPPRESS_MSK	BIT(3)
 #define ADXL345_REG_ACT_AXIS_MSK	GENMASK(6, 4)
+#define ADXL345_REG_INACT_AXIS_MSK	GENMASK(2, 0)
+#define ADXL345_POWER_CTL_INACT_MSK	(ADXL345_POWER_CTL_AUTO_SLEEP | ADXL345_POWER_CTL_LINK)
 
 enum adxl345_axis {
 	ADXL345_Z_EN = BIT(0),
@@ -71,18 +73,22 @@ static const unsigned int adxl345_tap_time_reg[3] = {
 /* activity/inactivity */
 enum adxl345_activity_type {
 	ADXL345_ACTIVITY,
+	ADXL345_INACTIVITY,
 };
 
 static const unsigned int adxl345_act_int_reg[2] = {
 	[ADXL345_ACTIVITY] = ADXL345_INT_ACTIVITY,
+	[ADXL345_INACTIVITY] = ADXL345_INT_INACTIVITY,
 };
 
 static const unsigned int adxl345_act_thresh_reg[2] = {
 	[ADXL345_ACTIVITY] = ADXL345_REG_THRESH_ACT,
+	[ADXL345_INACTIVITY] = ADXL345_REG_THRESH_INACT,
 };
 
 static const unsigned int adxl345_act_axis_msk[2] = {
 	[ADXL345_ACTIVITY] = ADXL345_REG_ACT_AXIS_MSK,
+	[ADXL345_INACTIVITY] = ADXL345_REG_INACT_AXIS_MSK,
 };
 
 enum adxl345_odr {
@@ -174,6 +180,10 @@ struct adxl345_state {
 	u32 act_axis_ctrl;
 	u8 act_threshold;
 
+	u32 inact_axis_ctrl;
+	u8 inact_threshold;
+	u8 inact_time_s;
+
 	u32 tap_axis_ctrl;
 	u8 tap_threshold;
 	u32 tap_duration_us;
@@ -195,6 +205,14 @@ static struct iio_event_spec adxl345_events[] = {
 		.mask_shared_by_type = BIT(IIO_EV_INFO_ENABLE) |
 			BIT(IIO_EV_INFO_VALUE),
 	},
+	{
+		/* inactivity */
+		.type = IIO_EV_TYPE_THRESH,
+		.dir = IIO_EV_DIR_FALLING,
+		.mask_shared_by_type = BIT(IIO_EV_INFO_ENABLE) |
+			BIT(IIO_EV_INFO_VALUE) |
+			BIT(IIO_EV_INFO_PERIOD),
+	},
 	{
 		/* single tap */
 		.type = IIO_EV_TYPE_GESTURE,
@@ -301,6 +319,17 @@ static int adxl345_write_act_axis(struct adxl345_state *st,
 					 st->act_axis_ctrl);
 		if (ret)
 			return ret;
+
+	} else {
+		st->inact_axis_ctrl = en
+			? st->inact_axis_ctrl | ADXL345_REG_INACT_AXIS_MSK
+			: st->inact_axis_ctrl & ~ADXL345_REG_INACT_AXIS_MSK;
+
+		ret = regmap_update_bits(st->regmap, ADXL345_REG_ACT_INACT_CTRL,
+					 adxl345_act_axis_msk[type],
+					 st->inact_axis_ctrl);
+		if (ret)
+			return ret;
 	}
 	return 0;
 }
@@ -324,6 +353,7 @@ static int adxl345_set_act_inact_en(struct adxl345_state *st,
 				    enum adxl345_activity_type type, bool cmd_en)
 {
 	bool axis_en, en = false;
+	unsigned long autosleep = 0;
 	int ret;
 
 	ret = adxl345_write_act_axis(st, type, cmd_en);
@@ -333,12 +363,24 @@ static int adxl345_set_act_inact_en(struct adxl345_state *st,
 	if (type == ADXL345_ACTIVITY) {
 		axis_en = FIELD_GET(ADXL345_REG_ACT_AXIS_MSK, st->act_axis_ctrl) > 0;
 		en = axis_en && st->act_threshold > 0;
+	} else {
+		axis_en = FIELD_GET(ADXL345_REG_INACT_AXIS_MSK, st->inact_axis_ctrl) > 0;
+		en = axis_en && st->inact_threshold > 0 &&
+			st->inact_time_s > 0;
 	}
 
 	en ? __set_bit(ilog2(adxl345_act_int_reg[type]), (unsigned long *)&st->int_map)
 		: __clear_bit(ilog2(adxl345_act_int_reg[type]), (unsigned long *)&st->int_map);
 
-	return regmap_write(st->regmap, ADXL345_REG_INT_ENABLE, st->int_map);
+	ret = regmap_write(st->regmap, ADXL345_REG_INT_ENABLE, st->int_map);
+	if (ret)
+		return ret;
+
+	en ? __set_bit(ilog2(ADXL345_POWER_CTL_INACT_MSK), &autosleep)
+		: __clear_bit(ilog2(ADXL345_POWER_CTL_INACT_MSK), &autosleep);
+
+	return regmap_update_bits(st->regmap, ADXL345_REG_POWER_CTL,
+				  ADXL345_POWER_CTL_INACT_MSK, autosleep);
 }
 
 static int adxl345_set_act_inact_threshold(struct adxl345_state *st,
@@ -352,6 +394,48 @@ static int adxl345_set_act_inact_threshold(struct adxl345_state *st,
 
 	if (type == ADXL345_ACTIVITY)
 		st->act_threshold = val;
+	else
+		st->inact_threshold = val;
+
+	return 0;
+}
+
+/**
+ * adxl345_set_inact_time_s - Configure inactivity time explicitly or by ODR.
+ * @st: The sensor state instance.
+ * @val_s: A desired time value, between 0 and 255.
+ *
+ * If val_s is 0, a default inactivity time will be computed. It should take
+ * power consumption into consideration. Thus it shall be shorter for higher
+ * frequencies and longer for lower frequencies. Hence, frequencies above 255 Hz
+ * shall default to 10 s and frequencies below 10 Hz shall result in 255 s to
+ * detect inactivity.
+ *
+ * The approach simply subtracts the pre-decimal figure of the configured
+ * sample frequency from 255 s to compute inactivity time [s]. Sub-Hz are thus
+ * ignored in this estimation. The recommended ODRs for various features
+ * (activity/inactivity, sleep modes, free fall, etc.) lie between 12.5 Hz and
+ * 400 Hz, thus higher or lower frequencies will result in the boundary
+ * defaults or need to be explicitly specified via val_s.
+ *
+ * Return: 0 or error value.
+ */
+static int adxl345_set_inact_time_s(struct adxl345_state *st, u32 val_s)
+{
+	unsigned int max_boundary = 255;
+	unsigned int min_boundary = 10;
+	unsigned int val = min(val_s, max_boundary);
+	int ret;
+
+	if (val == 0)
+		val = (adxl345_odr_tbl[st->odr][0] > max_boundary)
+			? min_boundary : max_boundary -	adxl345_odr_tbl[st->odr][0];
+
+	ret = regmap_write(st->regmap, ADXL345_REG_TIME_INACT, val);
+	if (ret)
+		return ret;
+
+	st->inact_time_s = val;
 
 	return 0;
 }
@@ -641,6 +725,11 @@ static int adxl345_set_odr(struct adxl345_state *st, enum adxl345_odr odr)
 
 	st->odr = odr;
 
+	/* update inactivity time by ODR */
+	ret = adxl345_set_inact_time_s(st, 0);
+	if (ret)
+		return ret;
+
 	return 0;
 }
 
@@ -672,6 +761,24 @@ static int adxl345_set_range(struct adxl345_state *st, enum adxl345_range range)
 	if (ret)
 		return ret;
 
+	st->act_threshold = st->act_threshold
+		* adxl345_range_factor_tbl[st->range]
+		/ adxl345_range_factor_tbl[range];
+	st->act_threshold = min(255, max(1, st->inact_threshold));
+
+	st->inact_threshold = st->inact_threshold
+		* adxl345_range_factor_tbl[st->range]
+		/ adxl345_range_factor_tbl[range];
+	st->inact_threshold = min(255, max(1, st->inact_threshold));
+
+	ret = adxl345_set_act_inact_threshold(st, ADXL345_ACTIVITY, st->act_threshold);
+	if (ret)
+		return ret;
+
+	ret = adxl345_set_act_inact_threshold(st, ADXL345_INACTIVITY, st->inact_threshold);
+	if (ret)
+		return ret;
+
 	st->range = range;
 
 	return 0;
@@ -810,6 +917,11 @@ static int adxl345_read_event_config(struct iio_dev *indio_dev,
 			if (ret)
 				return ret;
 			return int_en;
+		case IIO_EV_DIR_FALLING:
+			ret = adxl345_is_act_inact_en(st, ADXL345_INACTIVITY, &int_en);
+			if (ret)
+				return ret;
+			return int_en;
 		default:
 			return -EINVAL;
 		}
@@ -866,6 +978,8 @@ static int adxl345_write_event_config(struct iio_dev *indio_dev,
 		switch (dir) {
 		case IIO_EV_DIR_RISING:
 			return adxl345_set_act_inact_en(st, ADXL345_ACTIVITY, state);
+		case IIO_EV_DIR_FALLING:
+			return adxl345_set_act_inact_en(st, ADXL345_INACTIVITY, state);
 		default:
 			return -EINVAL;
 		}
@@ -913,9 +1027,15 @@ static int adxl345_read_event_value(struct iio_dev *indio_dev, const struct iio_
 			case IIO_EV_DIR_RISING:
 				*val = st->act_threshold;
 				return IIO_VAL_INT;
+			case IIO_EV_DIR_FALLING:
+				*val = st->inact_threshold;
+				return IIO_VAL_INT;
 			default:
 				return -EINVAL;
 			}
+		case IIO_EV_INFO_PERIOD:
+			*val = st->inact_time_s;
+			return IIO_VAL_INT;
 		default:
 			return -EINVAL;
 		}
@@ -982,10 +1102,16 @@ static int adxl345_write_event_value(struct iio_dev *indio_dev,
 			case IIO_EV_DIR_RISING:
 				ret = adxl345_set_act_inact_threshold(st, ADXL345_ACTIVITY, val);
 				break;
+			case IIO_EV_DIR_FALLING:
+				ret = adxl345_set_act_inact_threshold(st, ADXL345_INACTIVITY, val);
+				break;
 			default:
 				ret = -EINVAL;
 			}
 			break;
+		case IIO_EV_INFO_PERIOD:
+			ret = adxl345_set_inact_time_s(st, val);
+			break;
 		default:
 			ret = -EINVAL;
 		}
@@ -1365,6 +1491,17 @@ static int adxl345_push_event(struct iio_dev *indio_dev, int int_stat,
 			return ret;
 	}
 
+	if (FIELD_GET(ADXL345_INT_INACTIVITY, int_stat)) {
+		ret = iio_push_event(indio_dev,
+				     IIO_MOD_EVENT_CODE(IIO_ACCEL, 0,
+							IIO_MOD_X_OR_Y_OR_Z,
+							IIO_EV_TYPE_THRESH,
+							IIO_EV_DIR_FALLING),
+				     ts);
+		if (ret)
+			return ret;
+	}
+
 	if (FIELD_GET(ADXL345_INT_FREE_FALL, int_stat)) {
 		ret = iio_push_event(indio_dev,
 				     IIO_MOD_EVENT_CODE(IIO_ACCEL, 0,
@@ -1481,6 +1618,7 @@ int adxl345_core_probe(struct device *dev, struct regmap *regmap,
 	 * setup.
 	 */
 	st->act_axis_ctrl = ADXL345_REG_ACT_AXIS_MSK;
+	st->inact_axis_ctrl = ADXL345_REG_INACT_AXIS_MSK;
 	st->int_map = 0x00;			/* reset interrupts */
 
 	/* Init with reasonable values */
-- 
2.39.5

Re: [PATCH v2 12/14] iio: accel: adxl345: add inactivity feature

[Jonathan Cameron]

On Mon, 10 Feb 2025 11:01:17 +0000
Lothar Rubusch <l.rubusch@gmail.com> wrote:

> Add the inactivity feature of the sensor. When activity and inactivity
> is enabled, a link bit will be set linking activity and inactivity

are enabled, a link...

> handling. Additionally, the auto-sleep mode will be enabled. Due to the
> link bit the sensor is going to auto-sleep when inactivity was
> detected.
> 
> Inactivity detection needs a threshold to be configured, and a time
> after which it will go into inactivity state if measurements under
> threshold.
> 
> When a ODR is configured this time for inactivity is adjusted with a
> corresponding reasonable default value, in order to have higher
> frequencies and lower inactivity times, and lower sample frequency but
> give more time until inactivity. Both with reasonable upper and lower
> boundaries, since many of the sensor's features (e.g. auto-sleep) will
> need to operate beween 12.5 Hz and 400 Hz. This is a default setting
> when actively changing sample frequency, explicitly setting the time
> until inactivity will overwrite the default.
> 
> Similarly, setting the g-range will provide a default value for the
> activity and inactivity thresholds. Both are implicit defaults, but
> equally can be overwritten to be explicitly configured.
> 
> Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
A few things follow through from earlier patches (I'll assume you'll
apply those throughout).  Otherwise LGTM. 

[PATCH v2 13/14] iio: accel: adxl345: add coupling detection for activity/inactivity

[Lothar Rubusch]

Add coupling activity/inactivity detection by the AC/DC bit. This is an
addititional enhancement for the detection of activity states and
completes the activity / inactivity feature of the ADXL345.

Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
---
 drivers/iio/accel/adxl345_core.c | 77 ++++++++++++++++++++++++++++++++
 1 file changed, 77 insertions(+)

diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c
index 7de869fac799..411ae7bf6b97 100644
--- a/drivers/iio/accel/adxl345_core.c
+++ b/drivers/iio/accel/adxl345_core.c
@@ -36,7 +36,9 @@
 #define ADXL345_REG_TAP_AXIS_MSK	GENMASK(2, 0)
 #define ADXL345_REG_TAP_SUPPRESS_MSK	BIT(3)
 #define ADXL345_REG_ACT_AXIS_MSK	GENMASK(6, 4)
+#define ADXL345_REG_ACT_ACDC_MSK	BIT(7)
 #define ADXL345_REG_INACT_AXIS_MSK	GENMASK(2, 0)
+#define ADXL345_REG_INACT_ACDC_MSK	BIT(3)
 #define ADXL345_POWER_CTL_INACT_MSK	(ADXL345_POWER_CTL_AUTO_SLEEP | ADXL345_POWER_CTL_LINK)
 
 enum adxl345_axis {
@@ -86,6 +88,11 @@ static const unsigned int adxl345_act_thresh_reg[2] = {
 	[ADXL345_INACTIVITY] = ADXL345_REG_THRESH_INACT,
 };
 
+static const unsigned int adxl345_act_acdc_msk[2] = {
+	[ADXL345_ACTIVITY] = ADXL345_REG_ACT_ACDC_MSK,
+	[ADXL345_INACTIVITY] = ADXL345_REG_INACT_ACDC_MSK,
+};
+
 static const unsigned int adxl345_act_axis_msk[2] = {
 	[ADXL345_ACTIVITY] = ADXL345_REG_ACT_AXIS_MSK,
 	[ADXL345_INACTIVITY] = ADXL345_REG_INACT_AXIS_MSK,
@@ -178,9 +185,11 @@ struct adxl345_state {
 	enum adxl345_range range;
 
 	u32 act_axis_ctrl;
+	bool act_ac;
 	u8 act_threshold;
 
 	u32 inact_axis_ctrl;
+	bool inact_ac;
 	u8 inact_threshold;
 	u8 inact_time_s;
 
@@ -237,6 +246,18 @@ static struct iio_event_spec adxl345_events[] = {
 			BIT(IIO_EV_INFO_VALUE) |
 			BIT(IIO_EV_INFO_PERIOD),
 	},
+	{
+		/* activity, activity - ac bit */
+		.type = IIO_EV_TYPE_MAG_REFERENCED,
+		.dir = IIO_EV_DIR_RISING,
+		.mask_shared_by_type = BIT(IIO_EV_INFO_ENABLE),
+	},
+	{
+		/* activity, inactivity - ac bit */
+		.type = IIO_EV_TYPE_MAG_REFERENCED,
+		.dir = IIO_EV_DIR_FALLING,
+		.mask_shared_by_type = BIT(IIO_EV_INFO_ENABLE),
+	},
 };
 
 #define ADXL345_CHANNEL(index, reg, axis) {					\
@@ -334,6 +355,35 @@ static int adxl345_write_act_axis(struct adxl345_state *st,
 	return 0;
 }
 
+static int adxl345_is_act_inact_ac(struct adxl345_state *st,
+				   enum adxl345_activity_type type, bool *ac)
+{
+	if (type == ADXL345_ACTIVITY)
+		*ac = st->act_ac;
+	else
+		*ac = st->inact_ac;
+
+	return 0;
+}
+
+static int adxl345_set_act_inact_ac(struct adxl345_state *st,
+				    enum adxl345_activity_type type, bool ac)
+{
+	int ret;
+
+	ret = regmap_update_bits(st->regmap, ADXL345_REG_ACT_INACT_CTRL,
+				 adxl345_act_acdc_msk[type], ac);
+	if (ret)
+		return ret;
+
+	if (type == ADXL345_ACTIVITY)
+		st->act_ac = ac;
+	else
+		st->inact_ac = ac;
+
+	return 0;
+}
+
 static int adxl345_is_act_inact_en(struct adxl345_state *st,
 				   enum adxl345_activity_type type, bool *en)
 {
@@ -959,6 +1009,21 @@ static int adxl345_read_event_config(struct iio_dev *indio_dev,
 		if (ret)
 			return ret;
 		return int_en;
+	case IIO_EV_TYPE_MAG_REFERENCED:
+		switch (dir) {
+		case IIO_EV_DIR_RISING:
+			ret = adxl345_is_act_inact_ac(st, ADXL345_ACTIVITY, &int_en);
+			if (ret)
+				return ret;
+			return int_en;
+		case IIO_EV_DIR_FALLING:
+			ret = adxl345_is_act_inact_ac(st, ADXL345_INACTIVITY, &int_en);
+			if (ret)
+				return ret;
+			return int_en;
+		default:
+			return -EINVAL;
+		}
 	default:
 		return -EINVAL;
 	}
@@ -1008,6 +1073,16 @@ static int adxl345_write_event_config(struct iio_dev *indio_dev,
 		}
 	case IIO_EV_TYPE_MAG:
 		return adxl345_set_ff_en(st, state);
+	case IIO_EV_TYPE_MAG_REFERENCED:
+		switch (dir) {
+		case IIO_EV_DIR_RISING:
+			return adxl345_set_act_inact_ac(st, ADXL345_ACTIVITY, state);
+		case IIO_EV_DIR_FALLING:
+			return adxl345_set_act_inact_ac(st, ADXL345_INACTIVITY, state);
+		default:
+			return -EINVAL;
+		}
+
 	default:
 		return -EINVAL;
 	}
@@ -1619,6 +1694,8 @@ int adxl345_core_probe(struct device *dev, struct regmap *regmap,
 	 */
 	st->act_axis_ctrl = ADXL345_REG_ACT_AXIS_MSK;
 	st->inact_axis_ctrl = ADXL345_REG_INACT_AXIS_MSK;
+	st->inact_ac = 0;			/*    0 [dc]              */
+	st->act_ac = 0;
 	st->int_map = 0x00;			/* reset interrupts */
 
 	/* Init with reasonable values */
-- 
2.39.5

Re: [PATCH v2 13/14] iio: accel: adxl345: add coupling detection for activity/inactivity

[Jonathan Cameron]

On Mon, 10 Feb 2025 11:01:18 +0000
Lothar Rubusch <l.rubusch@gmail.com> wrote:

> Add coupling activity/inactivity detection by the AC/DC bit. This is an
> addititional enhancement for the detection of activity states and
> completes the activity / inactivity feature of the ADXL345.

Give a little more info here.  This is a somewhat unusual feature.
The AC description is odd terminology as it is really about
referencing to when the event is enabled, nothing is alternating.

The activity one is fairly simple.  The inactivity seems not.
I'm not entirely sure what the value used as reference is:

"Similarly, in ac-coupled operation for inactivity detection,
a reference value is used for comparison and is updated whenever
the device exceeds the inactivity threshold. After the reference
value is selected, the device compares the magnitude of the
difference between the reference value and the current acceleration
with THRESH_INACT. If the difference is less than the value in
THRESH_INACT for the time in TIME_INACT, the device is considered
inactive and the inactivity interrupt is triggered."

What is it updated to?  Is it updated to effectively the maximum
activity value? Or just a snap shot of whatever triggered the
inactivity be left (so takes into account which axis the
activity occurred on.)

I've no idea.

The definition we have for mag referenced is vague enough
to incorporate this so not a problem if we can't figure out
exactly what it is!

Jonathan



> 
> Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
> ---
>  drivers/iio/accel/adxl345_core.c | 77 ++++++++++++++++++++++++++++++++
>  1 file changed, 77 insertions(+)
> 
> diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c
> index 7de869fac799..411ae7bf6b97 100644
> --- a/drivers/iio/accel/adxl345_core.c
> +++ b/drivers/iio/accel/adxl345_core.c
> @@ -36,7 +36,9 @@
>  #define ADXL345_REG_TAP_AXIS_MSK	GENMASK(2, 0)
>  #define ADXL345_REG_TAP_SUPPRESS_MSK	BIT(3)
>  #define ADXL345_REG_ACT_AXIS_MSK	GENMASK(6, 4)
> +#define ADXL345_REG_ACT_ACDC_MSK	BIT(7)
>  #define ADXL345_REG_INACT_AXIS_MSK	GENMASK(2, 0)
> +#define ADXL345_REG_INACT_ACDC_MSK	BIT(3)
>  #define ADXL345_POWER_CTL_INACT_MSK	(ADXL345_POWER_CTL_AUTO_SLEEP | ADXL345_POWER_CTL_LINK)
>  
>  enum adxl345_axis {
> @@ -86,6 +88,11 @@ static const unsigned int adxl345_act_thresh_reg[2] = {
>  	[ADXL345_INACTIVITY] = ADXL345_REG_THRESH_INACT,
>  };
>  
> +static const unsigned int adxl345_act_acdc_msk[2] = {
> +	[ADXL345_ACTIVITY] = ADXL345_REG_ACT_ACDC_MSK,
> +	[ADXL345_INACTIVITY] = ADXL345_REG_INACT_ACDC_MSK,
> +};
> +
>  static const unsigned int adxl345_act_axis_msk[2] = {
>  	[ADXL345_ACTIVITY] = ADXL345_REG_ACT_AXIS_MSK,
>  	[ADXL345_INACTIVITY] = ADXL345_REG_INACT_AXIS_MSK,
> @@ -178,9 +185,11 @@ struct adxl345_state {
>  	enum adxl345_range range;
>  
>  	u32 act_axis_ctrl;
> +	bool act_ac;
>  	u8 act_threshold;
>  
>  	u32 inact_axis_ctrl;
> +	bool inact_ac;
>  	u8 inact_threshold;
>  	u8 inact_time_s;
>  
> @@ -237,6 +246,18 @@ static struct iio_event_spec adxl345_events[] = {
>  			BIT(IIO_EV_INFO_VALUE) |
>  			BIT(IIO_EV_INFO_PERIOD),
>  	},
> +	{
> +		/* activity, activity - ac bit */
> +		.type = IIO_EV_TYPE_MAG_REFERENCED,
> +		.dir = IIO_EV_DIR_RISING,
> +		.mask_shared_by_type = BIT(IIO_EV_INFO_ENABLE),
> +	},
> +	{
> +		/* activity, inactivity - ac bit */
> +		.type = IIO_EV_TYPE_MAG_REFERENCED,
> +		.dir = IIO_EV_DIR_FALLING,
> +		.mask_shared_by_type = BIT(IIO_EV_INFO_ENABLE),
> +	},
>  };
>  
>  #define ADXL345_CHANNEL(index, reg, axis) {					\
> @@ -334,6 +355,35 @@ static int adxl345_write_act_axis(struct adxl345_state *st,
>  	return 0;
>  }
>  
> +static int adxl345_is_act_inact_ac(struct adxl345_state *st,
> +				   enum adxl345_activity_type type, bool *ac)
> +{
> +	if (type == ADXL345_ACTIVITY)
> +		*ac = st->act_ac;
> +	else
> +		*ac = st->inact_ac;
> +
> +	return 0;
> +}
> +
> +static int adxl345_set_act_inact_ac(struct adxl345_state *st,
> +				    enum adxl345_activity_type type, bool ac)
> +{
> +	int ret;
> +
> +	ret = regmap_update_bits(st->regmap, ADXL345_REG_ACT_INACT_CTRL,
> +				 adxl345_act_acdc_msk[type], ac);
> +	if (ret)
> +		return ret;
> +
> +	if (type == ADXL345_ACTIVITY)
> +		st->act_ac = ac;
> +	else
> +		st->inact_ac = ac;
> +
> +	return 0;
> +}
> +
>  static int adxl345_is_act_inact_en(struct adxl345_state *st,
>  				   enum adxl345_activity_type type, bool *en)
>  {
> @@ -959,6 +1009,21 @@ static int adxl345_read_event_config(struct iio_dev *indio_dev,
>  		if (ret)
>  			return ret;
>  		return int_en;
> +	case IIO_EV_TYPE_MAG_REFERENCED:
> +		switch (dir) {
> +		case IIO_EV_DIR_RISING:
> +			ret = adxl345_is_act_inact_ac(st, ADXL345_ACTIVITY, &int_en);
> +			if (ret)
> +				return ret;
> +			return int_en;
> +		case IIO_EV_DIR_FALLING:
> +			ret = adxl345_is_act_inact_ac(st, ADXL345_INACTIVITY, &int_en);
> +			if (ret)
> +				return ret;
> +			return int_en;
> +		default:
> +			return -EINVAL;
> +		}
>  	default:
>  		return -EINVAL;
>  	}
> @@ -1008,6 +1073,16 @@ static int adxl345_write_event_config(struct iio_dev *indio_dev,
>  		}
>  	case IIO_EV_TYPE_MAG:
>  		return adxl345_set_ff_en(st, state);
> +	case IIO_EV_TYPE_MAG_REFERENCED:
> +		switch (dir) {
> +		case IIO_EV_DIR_RISING:
> +			return adxl345_set_act_inact_ac(st, ADXL345_ACTIVITY, state);
> +		case IIO_EV_DIR_FALLING:
> +			return adxl345_set_act_inact_ac(st, ADXL345_INACTIVITY, state);
> +		default:
> +			return -EINVAL;
> +		}
> +
>  	default:
>  		return -EINVAL;
>  	}
> @@ -1619,6 +1694,8 @@ int adxl345_core_probe(struct device *dev, struct regmap *regmap,
>  	 */
>  	st->act_axis_ctrl = ADXL345_REG_ACT_AXIS_MSK;
>  	st->inact_axis_ctrl = ADXL345_REG_INACT_AXIS_MSK;
> +	st->inact_ac = 0;			/*    0 [dc]              */
> +	st->act_ac = 0;
>  	st->int_map = 0x00;			/* reset interrupts */
>  
>  	/* Init with reasonable values */

[PATCH v2 14/14] docs: iio: add documentation for adxl345 driver

[Lothar Rubusch]

The documentation describes the ADXL345 driver, IIO interface,
interface usage and configuration.

Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
---
 Documentation/iio/adxl345.rst | 401 ++++++++++++++++++++++++++++++++++
 1 file changed, 401 insertions(+)
 create mode 100644 Documentation/iio/adxl345.rst

diff --git a/Documentation/iio/adxl345.rst b/Documentation/iio/adxl345.rst
new file mode 100644
index 000000000000..52bea3e7c4ac
--- /dev/null
+++ b/Documentation/iio/adxl345.rst
@@ -0,0 +1,401 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+===============
+ADXL345 driver
+===============
+
+This driver supports Analog Device's ADXL345/375 on SPI/I2C bus.
+
+1. Supported devices
+====================
+
+* `ADXL345 <https://www.analog.com/ADXL345>`_
+* `ADXL375 <https://www.analog.com/ADXL375>`_
+
+The ADXL345 is a generic purpose low power, 3-axis accelerometer with selectable
+measurement ranges. The ADXL345 supports the ±2 g, ±4 g, ±8 g, and ±16 g ranges.
+
+2. Device attributes
+====================
+
+Accelerometer measurements are always provided.
+
+Each IIO device, has a device folder under ``/sys/bus/iio/devices/iio:deviceX``,
+where X is the IIO index of the device. Under these folders reside a set of
+device files, depending on the characteristics and features of the hardware
+device in questions. These files are consistently generalized and documented in
+the IIO ABI documentation.
+
+The following table shows the ADXL345 related device files, found in the
+specific device folder path ``/sys/bus/iio/devices/iio:deviceX``.
+
++-------------------------------------------+----------------------------------------------------------+
+| 3-Axis Accelerometer related device files | Description                                              |
++-------------------------------------------+----------------------------------------------------------+
+| in_accel_sampling_frequency               | Currently selected sample rate.                          |
++-------------------------------------------+----------------------------------------------------------+
+| in_accel_sampling_frequency_available     | Available sampling frequency configurations.             |
++-------------------------------------------+----------------------------------------------------------+
+| in_accel_scale                            | Scale/range for the accelerometer channels.              |
++-------------------------------------------+----------------------------------------------------------+
+| in_accel_scale_available                  | Available scale ranges for the accelerometer channel.    |
++-------------------------------------------+----------------------------------------------------------+
+| in_accel_x_calibbias                      | Calibration offset for the X-axis accelerometer channel. |
++-------------------------------------------+----------------------------------------------------------+
+| in_accel_x_raw                            | Raw X-axis accelerometer channel value.                  |
++-------------------------------------------+----------------------------------------------------------+
+| in_accel_y_calibbias                      | y-axis acceleration offset correction                    |
++-------------------------------------------+----------------------------------------------------------+
+| in_accel_y_raw                            | Raw Y-axis accelerometer channel value.                  |
++-------------------------------------------+----------------------------------------------------------+
+| in_accel_z_calibbias                      | Calibration offset for the Z-axis accelerometer channel. |
++-------------------------------------------+----------------------------------------------------------+
+| in_accel_z_raw                            | Raw Z-axis accelerometer channel value.                  |
++-------------------------------------------+----------------------------------------------------------+
+
+Channel processed values
+-------------------------
+
+A channel value can be read from its _raw attribute. The value returned is the
+raw value as reported by the devices. To get the processed value of the channel,
+apply the following formula:
+
+.. code-block:: bash
+
+        processed value = (_raw + _offset) * _scale
+
+Where _offset and _scale are device attributes. If no _offset attribute is
+present, simply assume its value is 0.
+
++-------------------------------------+---------------------------+
+| Channel type                        | Measurement unit          |
++-------------------------------------+---------------------------+
+| Acceleration on X, Y, and Z axis    | Meters per second squared |
++-------------------------------------+---------------------------+
+
+Sensor events
+-------------
+
+Particular IIO events will be triggered by the corresponding interrupts. The
+sensor driver supports no or one active INT line, where the sensor has two
+possible INT IOs. Configure the used INT line in the devicetree. If no INT line
+is configured, the sensor falls back to FIFO bypass mode and no events are
+possible, only X, Y and Z axis measurements are possible.
+
+The following table shows the ADXL345 related device files, found in the
+specific device folder path ``/sys/bus/iio/devices/iio:deviceX/events``.
+
++---------------------------------------------+-----------------------------------------+
+| Event handle                                | Description                             |
++---------------------------------------------+-----------------------------------------+
+| in_accel_gesture_doubletap_en               | Enable double tap detection on all axis |
++---------------------------------------------+-----------------------------------------+
+| in_accel_gesture_doubletap_reset_timeout    | Double tap window in [us]               |
++---------------------------------------------+-----------------------------------------+
+| in_accel_gesture_doubletap_suppressedbit_en | Enable double tap suppress bit          |
++---------------------------------------------+-----------------------------------------+
+| in_accel_gesture_doubletap_tap2_min_delay   | Double tap latent in [us]               |
++---------------------------------------------+-----------------------------------------+
+| in_accel_gesture_singletap_timeout          | Single tap duration in [us]             |
++---------------------------------------------+-----------------------------------------+
+| in_accel_gesture_singletap_value            | Single tap threshold value in 62.5/LSB  |
++---------------------------------------------+-----------------------------------------+
+| in_accel_mag_falling_en                     | Enable free fall detection              |
++---------------------------------------------+-----------------------------------------+
+| in_accel_mag_falling_period                 | Free fall time in [us]                  |
++---------------------------------------------+-----------------------------------------+
+| in_accel_mag_falling_value                  | Free fall threshold value in 62.5/LSB   |
++---------------------------------------------+-----------------------------------------+
+| in_accel_mag_referenced_falling_en          | Set 1 to couple inactivity AC, 0 for DC |
++---------------------------------------------+-----------------------------------------+
+| in_accel_mag_referenced_rising_en           | Set 1 to couple activity AC, 0 for DC   |
++---------------------------------------------+-----------------------------------------+
+| in_accel_thresh_falling_en                  | Enable inactivity detection             |
++---------------------------------------------+-----------------------------------------+
+| in_accel_thresh_falling_period              | Inactivity time in seconds              |
++---------------------------------------------+-----------------------------------------+
+| in_accel_thresh_falling_value               | Inactivity threshold value in 62.5/LSB  |
++---------------------------------------------+-----------------------------------------+
+| in_accel_thresh_rising_en                   | Enable activity detection               |
++---------------------------------------------+-----------------------------------------+
+| in_accel_thresh_rising_value                | Activity threshold value in 62.5/LSB    |
++---------------------------------------------+-----------------------------------------+
+| in_accel_x_gesture_singletap_en             | Enable single tap detection on X axis   |
++---------------------------------------------+-----------------------------------------+
+| in_accel_y_gesture_singletap_en             | Enable single tap detection on Y axis   |
++---------------------------------------------+-----------------------------------------+
+| in_accel_z_gesture_singletap_en             | Enable single tap detection on Z axis   |
++---------------------------------------------+-----------------------------------------+
+
+Find a detailed description of a particular functionality in the sensor
+datasheet.
+
+Setting the ODR explicitly will result in estimated adjusted default values
+for the inactivity time detection, where higher frequencies shall default to
+longer wait periods, and vice versa. It is also possible to explicetly
+configure inactivity wait times, if the defaulting approach does not match
+application requirements. Setting 0 here, will fall back to default setting.
+
+The g range configuration also tries to estimate activity and inactivity
+thresholds when switching to another g range. The default range will be
+factorized by the relation of old range divided by new range. The value never
+becomes 0 and will be at least 1 and at most 255 i.e. 62.5g/LSB according to
+the datasheet. Nevertheless activity and inactivity thresholds can be
+overwritten by explicit values.
+
+When activity and inactivity events are enabled, the driver automatically will
+set link bit and autosleep bit. The link bit serially links the activity and
+inactivity functions. On the other side, the autosleep function switches the
+sensor to sleep mode if the inactivity function is enabled. This will reduce
+current consumption to the sub-12.5Hz rate.
+
+In dc-coupled operation, the current acceleration magnitude is compared
+directly with THRESH_ACT and THRESH_INACT registers to determine whether
+activity or inactivity was detected. In ac-coupled operation for activity
+detection, the acceleration value at the start of activity detection is taken
+as a reference value. New samples are then compared to this reference value.
+
+Single tap detection can be configured according to the datasheet by specifying
+threshold and duration. If only the single tap is in use, the single tap
+interrupt is triggered when the acceleration goes above threshold (i.e. DUR
+start) and below the threshold, as long as duration is not exceeded. If single
+tap and double tap are in use, the single tap is triggered when the doulbe tap
+event has been either validated or invalidated.
+
+For double tap configure additionally window and latency in [us]. Latency
+starts counting when the single tap goes below threshold and is a waiting
+period, any spikes here are ignored for double tap detection. After latency,
+the window starts. Any rise above threshold, with a consequent fall below
+threshold within window time, rises a double tap signal when going below
+threshold.
+
+A double tap event can be invalidated in three ways: If the suppress bit is set,
+any acceleration spike above the threshold already during the latency time
+invalidates the double tap detection immediately, i.e. during latence must not
+occur spikes for double tap detection with suppress bit set.
+A double tap event is invalidated if acceleration lies above the threshold at
+the start of the window time for the double tap detection.
+Additionally a double tap event can be invalidated if an acceleration exceeds
+the time limit for taps, set by duration register, since also for the double
+tap the same value for duration counts, i.e. when rising above threshold the
+fall below threshold has to be within duration time.
+
+A free fall event will be detected if the signal goes below the configured
+threshold, for the configured time [us].
+
+Note, that activity/inactivy, as also freefall is recommended for 12.5 Hz ODR
+up to 400 Hz.
+
+Usage examples
+--------------
+
+Show device name:
+
+.. code-block:: bash
+
+        root:/sys/bus/iio/devices/iio:device0> cat name
+        adxl345
+
+Show accelerometer channels value:
+
+.. code-block:: bash
+
+        root:/sys/bus/iio/devices/iio:device0> cat in_accel_x_raw
+        -1
+        root:/sys/bus/iio/devices/iio:device0> cat in_accel_y_raw
+        2
+        root:/sys/bus/iio/devices/iio:device0> cat in_accel_z_raw
+        -253
+
+Set calibration offset for accelerometer channels:
+
+.. code-block:: bash
+
+        root:/sys/bus/iio/devices/iio:device0> cat in_accel_x_calibbias
+        0
+
+        root:/sys/bus/iio/devices/iio:device0> echo 50 > in_accel_x_calibbias
+        root:/sys/bus/iio/devices/iio:device0> cat in_accel_x_calibbias
+        50
+
+Given the 13-bit full resolution, the available ranges are calculated by the
+following forumla:
+
+.. code-block:: bash
+
+        (g * 2 * 9.80665) / (2^(resolution) - 1) * 100; for g := 2|4|8|16
+
+Scale range configuration:
+
+.. code-block:: bash
+
+        root:/sys/bus/iio/devices/iio:device0> cat ./in_accel_scale
+        0.478899
+        root:/sys/bus/iio/devices/iio:device0> cat ./in_accel_scale_available
+        0.478899 0.957798 1.915595 3.831190
+
+        root:/sys/bus/iio/devices/iio:device0> echo 1.915595 > ./in_accel_scale
+        root:/sys/bus/iio/devices/iio:device0> cat ./in_accel_scale
+        1.915595
+
+Set output data rate (ODR):
+
+.. code-block:: bash
+
+        root:/sys/bus/iio/devices/iio:device0> cat ./in_accel_sampling_frequency
+        200.000000
+
+        root:/sys/bus/iio/devices/iio:device0> cat ./in_accel_sampling_frequency_available
+        0.097000 0.195000 0.390000 0.781000 1.562000 3.125000 6.250000 12.500000 25.000000 50.000000 100.000000 200.000000 400.000000 800.000000 1600.000000 3200.000000
+
+        root:/sys/bus/iio/devices/iio:device0> echo 1.562000 > ./in_accel_sampling_frequency
+        root:/sys/bus/iio/devices/iio:device0> cat ./in_accel_sampling_frequency
+        1.562000
+
+Configure one or several events:
+
+.. code-block:: bash
+
+        root:> cd /sys/bus/iio/devices/iio:device0
+
+        root:/sys/bus/iio/devices/iio:device0> echo 1 > ./buffer0/in_accel_x_en
+        root:/sys/bus/iio/devices/iio:device0> echo 1 > ./buffer0/in_accel_y_en
+        root:/sys/bus/iio/devices/iio:device0> echo 1 > ./buffer0/in_accel_z_en
+
+        root:/sys/bus/iio/devices/iio:device0> echo 1 > ./scan_elements/in_accel_x_en
+        root:/sys/bus/iio/devices/iio:device0> echo 1 > ./scan_elements/in_accel_y_en
+        root:/sys/bus/iio/devices/iio:device0> echo 1 > ./scan_elements/in_accel_z_en
+
+        root:/sys/bus/iio/devices/iio:device0> echo 14   > ./in_accel_x_calibbias
+        root:/sys/bus/iio/devices/iio:device0> echo 2    > ./in_accel_y_calibbias
+        root:/sys/bus/iio/devices/iio:device0> echo -250 > ./in_accel_z_calibbias
+
+        root:/sys/bus/iio/devices/iio:device0> echo 24 > ./buffer0/length
+
+        ## activity, threshold [62.5/LSB]
+        root:/sys/bus/iio/devices/iio:device0> echo 6 > ./events/in_accel_thresh_rising_value
+
+        ## inactivity, threshold, [62.5/LSB]
+        root:/sys/bus/iio/devices/iio:device0> echo 4 > ./events/in_accel_thresh_falling_value
+
+        ## inactivity, time [s]
+        root:/sys/bus/iio/devices/iio:device0> echo 3 > ./events/in_accel_thresh_falling_period
+
+        ## singletap, threshold
+        root:/sys/bus/iio/devices/iio:device0> echo 35 > ./events/in_accel_gesture_singletap_value
+
+        ## singletap, duration [us]
+        root:/sys/bus/iio/devices/iio:device0> echo 0.001875  > ./events/in_accel_gesture_singletap_timeout
+
+        ## doubletap, window [us]
+        root:/sys/bus/iio/devices/iio:device0> echo 0.025 > ./events/in_accel_gesture_doubletap_reset_timeout
+
+        ## doubletap, latent [us]
+        root:/sys/bus/iio/devices/iio:device0> echo 0.025 > ./events/in_accel_gesture_doubletap_tap2_min_delay
+
+        ## freefall, threshold [62.5/LSB]
+        root:/sys/bus/iio/devices/iio:device0> echo 8 > ./events/in_accel_mag_falling_value
+
+        ## freefall, time [ms]
+        root:/sys/bus/iio/devices/iio:device0> echo 1.25 > ./events/in_accel_mag_falling_period
+
+        ## activity, enable
+        root:/sys/bus/iio/devices/iio:device0> echo 1 > ./events/in_accel_thresh_rising_en
+
+        ## inactivity, enable
+        root:/sys/bus/iio/devices/iio:device0> echo 1 > ./events/in_accel_thresh_falling_en
+
+        ## freefall, enable
+        root:/sys/bus/iio/devices/iio:device0> echo 1 > ./events/in_accel_mag_falling_en
+
+        ## singletap, enable
+        root:/sys/bus/iio/devices/iio:device0> echo 1 > ./events/in_accel_x_gesture_singletap_en
+        root:/sys/bus/iio/devices/iio:device0> echo 1 > ./events/in_accel_y_gesture_singletap_en
+        root:/sys/bus/iio/devices/iio:device0> echo 1 > ./events/in_accel_z_gesture_singletap_en
+
+        ## doubletap, enable
+        root:/sys/bus/iio/devices/iio:device0> echo 1 > ./events/in_accel_gesture_doubletap_en
+
+Verify incoming events:
+
+.. code-block:: bash
+
+        root:# iio_event_monitor adxl345
+        Found IIO device with name adxl345 with device number 0
+        Event: time: 1739063415957073383, type: accel(z), channel: 0, evtype: thresh, direction: rising
+        Event: time: 1739063415963770218, type: accel(z), channel: 0, evtype: thresh, direction: rising
+        Event: time: 1739063416002563061, type: accel(z), channel: 0, evtype: gesture, direction: singletap
+        Event: time: 1739063426271128739, type: accel(x|y|z), channel: 0, evtype: thresh, direction: falling
+        Event: time: 1739063436539080713, type: accel(x|y|z), channel: 0, evtype: thresh, direction: falling
+        Event: time: 1739063438357970381, type: accel(z), channel: 0, evtype: thresh, direction: rising
+        Event: time: 1739063446726161586, type: accel(z), channel: 0, evtype: thresh, direction: rising
+        Event: time: 1739063446727892670, type: accel(z), channel: 0, evtype: thresh, direction: rising
+        Event: time: 1739063446743019768, type: accel(z), channel: 0, evtype: thresh, direction: rising
+        Event: time: 1739063446744650696, type: accel(z), channel: 0, evtype: thresh, direction: rising
+        Event: time: 1739063446763559386, type: accel(z), channel: 0, evtype: gesture, direction: singletap
+        Event: time: 1739063448818126480, type: accel(x|y|z), channel: 0, evtype: thresh, direction: falling
+        ...
+
+3. Device buffers
+=================
+
+This driver supports IIO buffers.
+
+All devices support retrieving the raw acceleration and temperature measurements
+using buffers.
+
+Usage examples
+--------------
+
+Select channels for buffer read:
+
+.. code-block:: bash
+
+        root:/sys/bus/iio/devices/iio:device0> echo 1 > scan_elements/in_accel_x_en
+        root:/sys/bus/iio/devices/iio:device0> echo 1 > scan_elements/in_accel_y_en
+        root:/sys/bus/iio/devices/iio:device0> echo 1 > scan_elements/in_accel_z_en
+
+Set the number of samples to be stored in the buffer:
+
+.. code-block:: bash
+
+        root:/sys/bus/iio/devices/iio:device0> echo 10 > buffer/length
+
+Enable buffer readings:
+
+.. code-block:: bash
+
+        root:/sys/bus/iio/devices/iio:device0> echo 1 > buffer/enable
+
+Obtain buffered data:
+
+.. code-block:: bash
+
+        root:> iio_readdev -b 16 -s 1024 adxl345 | hexdump -d
+        WARNING: High-speed mode not enabled
+        0000000   00003   00012   00013   00005   00010   00011   00005   00011
+        0000010   00013   00004   00012   00011   00003   00012   00014   00007
+        0000020   00011   00013   00004   00013   00014   00003   00012   00013
+        0000030   00004   00012   00013   00005   00011   00011   00005   00012
+        0000040   00014   00005   00012   00014   00004   00010   00012   00004
+        0000050   00013   00011   00003   00011   00012   00005   00011   00013
+        0000060   00003   00012   00012   00003   00012   00012   00004   00012
+        0000070   00012   00003   00013   00013   00003   00013   00012   00005
+        0000080   00012   00013   00003   00011   00012   00005   00012   00013
+        0000090   00003   00013   00011   00005   00013   00014   00003   00012
+        00000a0   00012   00003   00012   00013   00004   00012   00015   00004
+        00000b0   00014   00011   00003   00014   00013   00004   00012   00011
+        00000c0   00004   00012   00013   00004   00014   00011   00004   00013
+        00000d0   00012   00002   00014   00012   00005   00012   00013   00005
+        00000e0   00013   00013   00003   00013   00013   00005   00012   00013
+        00000f0   00004   00014   00015   00005   00012   00011   00005   00012
+        ...
+
+See ``Documentation/iio/iio_devbuf.rst`` for more information about how buffered
+data is structured.
+
+4. IIO Interfacing Tools
+========================
+
+See ``Documentation/iio/iio_tools.rst`` for the description of the available IIO
+interfacing tools.
-- 
2.39.5

Re: [PATCH v2 14/14] docs: iio: add documentation for adxl345 driver

[Jonathan Cameron]

On Mon, 10 Feb 2025 11:01:19 +0000
Lothar Rubusch <l.rubusch@gmail.com> wrote:

> The documentation describes the ADXL345 driver, IIO interface,
> interface usage and configuration.
> 
> Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
Hi Lothar,

I was running out of time so this was a little more superficial review than I would
have liked to have done.  I've commented on a few bits that I was confuse about in the driver.
Document is showing how useful these are :)

Jonathan

> +
> +Particular IIO events will be triggered by the corresponding interrupts. The
> +sensor driver supports no or one active INT line, where the sensor has two
> +possible INT IOs. Configure the used INT line in the devicetree. If no INT line
> +is configured, the sensor falls back to FIFO bypass mode and no events are
> +possible, only X, Y and Z axis measurements are possible.
> +
> +The following table shows the ADXL345 related device files, found in the
> +specific device folder path ``/sys/bus/iio/devices/iio:deviceX/events``.
> +
> ++---------------------------------------------+-----------------------------------------+
> +| Event handle                                | Description                             |
> ++---------------------------------------------+-----------------------------------------+
> +| in_accel_gesture_doubletap_en               | Enable double tap detection on all axis |
> ++---------------------------------------------+-----------------------------------------+
> +| in_accel_gesture_doubletap_reset_timeout    | Double tap window in [us]               |
> ++---------------------------------------------+-----------------------------------------+
> +| in_accel_gesture_doubletap_suppressedbit_en | Enable double tap suppress bit          |
> ++---------------------------------------------+-----------------------------------------+
> +| in_accel_gesture_doubletap_tap2_min_delay   | Double tap latent in [us]               |
> ++---------------------------------------------+-----------------------------------------+
> +| in_accel_gesture_singletap_timeout          | Single tap duration in [us]             |
> ++---------------------------------------------+-----------------------------------------+
> +| in_accel_gesture_singletap_value            | Single tap threshold value in 62.5/LSB  |
> ++---------------------------------------------+-----------------------------------------+
> +| in_accel_mag_falling_en                     | Enable free fall detection              |
> ++---------------------------------------------+-----------------------------------------+
> +| in_accel_mag_falling_period                 | Free fall time in [us]                  |
> ++---------------------------------------------+-----------------------------------------+
> +| in_accel_mag_falling_value                  | Free fall threshold value in 62.5/LSB   |
> ++---------------------------------------------+-----------------------------------------+
> +| in_accel_mag_referenced_falling_en          | Set 1 to couple inactivity AC, 0 for DC |

That interface doesn't work align with the normal ABI.


Should be a selection of either mag_referenced_falling_en or another event to
pick between AC and DC. I assume thresh_falling_en though i'll admit to being a little
lost in what this hardware is doing!

> ++---------------------------------------------+-----------------------------------------+
> +| in_accel_mag_referenced_rising_en           | Set 1 to couple activity AC, 0 for DC   |
> ++---------------------------------------------+-----------------------------------------+
> +| in_accel_thresh_falling_en                  | Enable inactivity detection             |
> ++---------------------------------------------+-----------------------------------------+
> +| in_accel_thresh_falling_period              | Inactivity time in seconds              |
> ++---------------------------------------------+-----------------------------------------+
> +| in_accel_thresh_falling_value               | Inactivity threshold value in 62.5/LSB  |
> ++---------------------------------------------+-----------------------------------------+
> +| in_accel_thresh_rising_en                   | Enable activity detection               |
> ++---------------------------------------------+-----------------------------------------+
> +| in_accel_thresh_rising_value                | Activity threshold value in 62.5/LSB    |
> ++---------------------------------------------+-----------------------------------------+
> +| in_accel_x_gesture_singletap_en             | Enable single tap detection on X axis   |
> ++---------------------------------------------+-----------------------------------------+
> +| in_accel_y_gesture_singletap_en             | Enable single tap detection on Y axis   |
> ++---------------------------------------------+-----------------------------------------+
> +| in_accel_z_gesture_singletap_en             | Enable single tap detection on Z axis   |
> ++---------------------------------------------+-----------------------------------------+
> +
> +Find a detailed description of a particular functionality in the sensor
> +datasheet.
> +
> +Setting the ODR explicitly will result in estimated adjusted default values
> +for the inactivity time detection, where higher frequencies shall default to
> +longer wait periods, and vice versa. It is also possible to explicetly
> +configure inactivity wait times, if the defaulting approach does not match
> +application requirements. Setting 0 here, will fall back to default setting.
> +
> +The g range configuration also tries to estimate activity and inactivity
> +thresholds when switching to another g range. The default range will be
> +factorized by the relation of old range divided by new range. The value never
> +becomes 0 and will be at least 1 and at most 255 i.e. 62.5g/LSB according to
> +the datasheet. Nevertheless activity and inactivity thresholds can be
> +overwritten by explicit values.
> +
> +When activity and inactivity events are enabled, the driver automatically will
> +set link bit and autosleep bit. The link bit serially links the activity and
> +inactivity functions. 

So this is a sort of hysteresis?  We may want to describe that more directly.

> On the other side, the autosleep function switches the
> +sensor to sleep mode if the inactivity function is enabled. This will reduce
> +current consumption to the sub-12.5Hz rate.
> +
> +In dc-coupled operation, the current acceleration magnitude is compared
> +directly with THRESH_ACT and THRESH_INACT registers to determine whether
> +activity or inactivity was detected. In ac-coupled operation for activity
> +detection, the acceleration value at the start of activity detection is taken
> +as a reference value. New samples are then compared to this reference value.

That applies for activity - the inactivity description is rather more confusing.

> +
> +Single tap detection can be configured according to the datasheet by specifying
> +threshold and duration. If only the single tap is in use, the single tap
> +interrupt is triggered when the acceleration goes above threshold (i.e. DUR
> +start) and below the threshold, as long as duration is not exceeded. If single
> +tap and double tap are in use, the single tap is triggered when the doulbe tap
> +event has been either validated or invalidated.
> +
> +For double tap configure additionally window and latency in [us]. Latency
> +starts counting when the single tap goes below threshold and is a waiting
> +period, any spikes here are ignored for double tap detection. After latency,
> +the window starts. Any rise above threshold, with a consequent fall below
> +threshold within window time, rises a double tap signal when going below
> +threshold.
> +
> +A double tap event can be invalidated in three ways: If the suppress bit is set,
> +any acceleration spike above the threshold already during the latency time
> +invalidates the double tap detection immediately, i.e. during latence must not
> +occur spikes for double tap detection with suppress bit set.
> +A double tap event is invalidated if acceleration lies above the threshold at
> +the start of the window time for the double tap detection.
> +Additionally a double tap event can be invalidated if an acceleration exceeds
> +the time limit for taps, set by duration register, since also for the double
> +tap the same value for duration counts, i.e. when rising above threshold the
> +fall below threshold has to be within duration time.
> +
> +A free fall event will be detected if the signal goes below the configured
> +threshold, for the configured time [us].
> +
> +Note, that activity/inactivy, as also freefall is recommended for 12.5 Hz ODR
> +up to 400 Hz.
> +
>