* [PATCH v10 1/7] iio: accel: adxl345: simplify interrupt mapping
2025-06-22 15:50 [PATCH v10 0/7] iio: accel: adxl345: add interrupt based sensor events Lothar Rubusch
@ 2025-06-22 15:50 ` Lothar Rubusch
2025-06-23 9:24 ` Andy Shevchenko
2025-06-22 15:50 ` [PATCH v10 2/7] iio: accel: adxl345: simplify reading the FIFO Lothar Rubusch
` (6 subsequent siblings)
7 siblings, 1 reply; 29+ messages in thread
From: Lothar Rubusch @ 2025-06-22 15:50 UTC (permalink / raw)
To: lars, Michael.Hennerich, jic23, dlechner, nuno.sa, andy, corbet
Cc: linux-iio, linux-kernel, linux-doc, eraretuya, l.rubusch
Refactor the sensor interrupt mapping by utilizing regmap_assign_bits(),
which accepts a boolean value directly. Introduce a helper function to
streamline the identification of the configured interrupt line pin. Also,
use identifiers from units.h to represent the full 8-bit register when
setting bits.
This is a purely refactoring change and does not affect functionality.
Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
---
drivers/iio/accel/adxl345_core.c | 34 +++++++++++++++++++-------------
1 file changed, 20 insertions(+), 14 deletions(-)
diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c
index b60794812738..6c493272a5b0 100644
--- a/drivers/iio/accel/adxl345_core.c
+++ b/drivers/iio/accel/adxl345_core.c
@@ -1088,6 +1088,19 @@ static const struct iio_info adxl345_info = {
.hwfifo_set_watermark = adxl345_set_watermark,
};
+static int _get_int_line(struct device *dev, int *irq)
+{
+ *irq = fwnode_irq_get_byname(dev_fwnode(dev), "INT1");
+ if (*irq > 0)
+ return ADXL345_INT1;
+
+ *irq = fwnode_irq_get_byname(dev_fwnode(dev), "INT2");
+ if (*irq > 0)
+ return ADXL345_INT2;
+
+ return ADXL345_INT_NONE;
+}
+
/**
* adxl345_core_probe() - Probe and setup for the accelerometer.
* @dev: Driver model representation of the device
@@ -1203,23 +1216,16 @@ int adxl345_core_probe(struct device *dev, struct regmap *regmap,
if (ret)
return ret;
- irq = fwnode_irq_get_byname(dev_fwnode(dev), "INT1");
- if (irq < 0) {
- intio = ADXL345_INT2;
- irq = fwnode_irq_get_byname(dev_fwnode(dev), "INT2");
- if (irq < 0)
- intio = ADXL345_INT_NONE;
- }
-
+ intio = _get_int_line(dev, &irq);
if (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.
+ * In the INT map register, bits set to 0 route their
+ * corresponding interrupts to the INT1 pin, while bits set to 1
+ * route them to the INT2 pin. The intio should handle this
+ * mapping accordingly.
*/
- regval = intio ? 0xff : 0;
-
- ret = regmap_write(st->regmap, ADXL345_REG_INT_MAP, regval);
+ ret = regmap_assign_bits(st->regmap, ADXL345_REG_INT_MAP,
+ U8_MAX, intio);
if (ret)
return ret;
--
2.39.5
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH v10 1/7] iio: accel: adxl345: simplify interrupt mapping
2025-06-22 15:50 ` [PATCH v10 1/7] iio: accel: adxl345: simplify interrupt mapping Lothar Rubusch
@ 2025-06-23 9:24 ` Andy Shevchenko
0 siblings, 0 replies; 29+ messages in thread
From: Andy Shevchenko @ 2025-06-23 9:24 UTC (permalink / raw)
To: Lothar Rubusch
Cc: lars, Michael.Hennerich, jic23, dlechner, nuno.sa, andy, corbet,
linux-iio, linux-kernel, linux-doc, eraretuya
On Sun, Jun 22, 2025 at 03:50:04PM +0000, Lothar Rubusch wrote:
> Refactor the sensor interrupt mapping by utilizing regmap_assign_bits(),
> which accepts a boolean value directly. Introduce a helper function to
> streamline the identification of the configured interrupt line pin. Also,
> use identifiers from units.h to represent the full 8-bit register when
> setting bits.
>
> This is a purely refactoring change and does not affect functionality.
> +static int _get_int_line(struct device *dev, int *irq)
Oh, I should have been clear, I meant the suffix of the name, one still needs
a namespace for this: adxl345_get_int_line().
With that fixed,
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v10 2/7] iio: accel: adxl345: simplify reading the FIFO
2025-06-22 15:50 [PATCH v10 0/7] iio: accel: adxl345: add interrupt based sensor events Lothar Rubusch
2025-06-22 15:50 ` [PATCH v10 1/7] iio: accel: adxl345: simplify interrupt mapping Lothar Rubusch
@ 2025-06-22 15:50 ` Lothar Rubusch
2025-06-22 15:50 ` [PATCH v10 3/7] iio: accel: adxl345: add activity event feature Lothar Rubusch
` (5 subsequent siblings)
7 siblings, 0 replies; 29+ messages in thread
From: Lothar Rubusch @ 2025-06-22 15:50 UTC (permalink / raw)
To: lars, Michael.Hennerich, jic23, dlechner, nuno.sa, andy, corbet
Cc: linux-iio, linux-kernel, linux-doc, eraretuya, l.rubusch
Bulk FIFO reading can be streamlined by eliminating redundant variables and
simplifying the process of reading x-, y-, and z-axis measurement sets.
This is a refactoring change with no expected impact on functionality.
Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
---
drivers/iio/accel/adxl345_core.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c
index 6c493272a5b0..3821fe7cf2a0 100644
--- a/drivers/iio/accel/adxl345_core.c
+++ b/drivers/iio/accel/adxl345_core.c
@@ -885,15 +885,12 @@ static int adxl345_get_samples(struct adxl345_state *st)
*/
static int adxl345_fifo_transfer(struct adxl345_state *st, int samples)
{
- size_t count;
int i, ret = 0;
- /* count is the 3x the fifo_buf element size, hence 6B */
- count = sizeof(st->fifo_buf[0]) * ADXL345_DIRS;
for (i = 0; i < samples; i++) {
- /* read 3x 2 byte elements from base address into next fifo_buf position */
ret = regmap_bulk_read(st->regmap, ADXL345_REG_XYZ_BASE,
- st->fifo_buf + (i * count / 2), count);
+ st->fifo_buf + (i * ADXL345_DIRS),
+ sizeof(st->fifo_buf[0]) * ADXL345_DIRS);
if (ret)
return ret;
--
2.39.5
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v10 3/7] iio: accel: adxl345: add activity event feature
2025-06-22 15:50 [PATCH v10 0/7] iio: accel: adxl345: add interrupt based sensor events Lothar Rubusch
2025-06-22 15:50 ` [PATCH v10 1/7] iio: accel: adxl345: simplify interrupt mapping Lothar Rubusch
2025-06-22 15:50 ` [PATCH v10 2/7] iio: accel: adxl345: simplify reading the FIFO Lothar Rubusch
@ 2025-06-22 15:50 ` Lothar Rubusch
2025-06-23 9:34 ` Andy Shevchenko
2025-06-22 15:50 ` [PATCH v10 4/7] iio: accel: adxl345: add inactivity feature Lothar Rubusch
` (4 subsequent siblings)
7 siblings, 1 reply; 29+ messages in thread
From: Lothar Rubusch @ 2025-06-22 15:50 UTC (permalink / raw)
To: lars, Michael.Hennerich, jic23, dlechner, nuno.sa, andy, corbet
Cc: linux-iio, linux-kernel, linux-doc, eraretuya, l.rubusch
Enable the sensor to detect activity and trigger interrupts accordingly.
Activity events are determined based on a threshold, which is initialized
to a sensible default during probe. This default value is adopted from the
legacy ADXL345 input driver to maintain consistent behavior.
The combination of activity detection, ODR configuration, and range
settings lays the groundwork for the activity/inactivity hysteresis
mechanism, which will be implemented in a subsequent patch. As such,
portions of this patch prepare switch-case structures to support those
upcoming changes.
Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
---
drivers/iio/accel/adxl345_core.c | 244 ++++++++++++++++++++++++++++++-
1 file changed, 241 insertions(+), 3 deletions(-)
diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c
index 3821fe7cf2a0..99b590e67967 100644
--- a/drivers/iio/accel/adxl345_core.c
+++ b/drivers/iio/accel/adxl345_core.c
@@ -36,11 +36,16 @@
#define ADXL345_REG_TAP_AXIS_MSK GENMASK(2, 0)
#define ADXL345_REG_TAP_SUPPRESS_MSK BIT(3)
#define ADXL345_REG_TAP_SUPPRESS BIT(3)
+#define ADXL345_REG_ACT_AXIS_MSK GENMASK(6, 4)
#define ADXL345_TAP_Z_EN BIT(0)
#define ADXL345_TAP_Y_EN BIT(1)
#define ADXL345_TAP_X_EN BIT(2)
+#define ADXL345_ACT_Z_EN BIT(4)
+#define ADXL345_ACT_Y_EN BIT(5)
+#define ADXL345_ACT_X_EN BIT(6)
+
/* single/double tap */
enum adxl345_tap_type {
ADXL345_SINGLE_TAP,
@@ -64,6 +69,19 @@ static const unsigned int adxl345_tap_time_reg[] = {
[ADXL345_TAP_TIME_DUR] = ADXL345_REG_DUR,
};
+/* activity/inactivity */
+enum adxl345_activity_type {
+ ADXL345_ACTIVITY,
+};
+
+static const unsigned int adxl345_act_int_reg[] = {
+ [ADXL345_ACTIVITY] = ADXL345_INT_ACTIVITY,
+};
+
+static const unsigned int adxl345_act_thresh_reg[] = {
+ [ADXL345_ACTIVITY] = ADXL345_REG_THRESH_ACT,
+};
+
enum adxl345_odr {
ADXL345_ODR_0P10HZ = 0,
ADXL345_ODR_0P20HZ,
@@ -144,6 +162,13 @@ struct adxl345_state {
};
static struct iio_event_spec adxl345_events[] = {
+ {
+ /* activity */
+ .type = IIO_EV_TYPE_MAG,
+ .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,
@@ -237,6 +262,90 @@ static int adxl345_set_measure_en(struct adxl345_state *st, bool en)
ADXL345_POWER_CTL_MEASURE, en);
}
+/* activity / inactivity */
+
+static int adxl345_is_act_inact_en(struct adxl345_state *st,
+ enum adxl345_activity_type type)
+{
+ unsigned int axis_ctrl;
+ unsigned int regval;
+ bool en;
+ int ret;
+
+ ret = regmap_read(st->regmap, ADXL345_REG_ACT_INACT_CTRL, &axis_ctrl);
+ if (ret)
+ return ret;
+
+ /* Check if axis for activity are enabled */
+ switch (type) {
+ case ADXL345_ACTIVITY:
+ en = FIELD_GET(ADXL345_ACT_X_EN, axis_ctrl) |
+ FIELD_GET(ADXL345_ACT_Y_EN, axis_ctrl) |
+ FIELD_GET(ADXL345_ACT_Z_EN, axis_ctrl);
+ if (!en)
+ return false;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ /* Check if specific interrupt is enabled */
+ ret = regmap_read(st->regmap, ADXL345_REG_INT_ENABLE, ®val);
+ if (ret)
+ return ret;
+
+ return adxl345_act_int_reg[type] & regval;
+}
+
+static int adxl345_set_act_inact_en(struct adxl345_state *st,
+ enum adxl345_activity_type type,
+ bool cmd_en)
+{
+ unsigned int axis_ctrl;
+ unsigned int threshold;
+ int ret;
+
+ if (cmd_en) {
+ /* When turning on, check if threshold is valid */
+ ret = regmap_read(st->regmap,
+ adxl345_act_thresh_reg[type],
+ &threshold);
+ if (ret)
+ return ret;
+
+ if (!threshold) /* Just ignore the command if threshold is 0 */
+ return 0;
+ }
+
+ /* Start modifying configuration registers */
+ ret = adxl345_set_measure_en(st, false);
+ if (ret)
+ return ret;
+
+ /* Enable axis according to the command */
+ switch (type) {
+ case ADXL345_ACTIVITY:
+ axis_ctrl = ADXL345_ACT_X_EN | ADXL345_ACT_Y_EN |
+ ADXL345_ACT_Z_EN;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ ret = regmap_assign_bits(st->regmap, ADXL345_REG_ACT_INACT_CTRL,
+ axis_ctrl, cmd_en);
+ if (ret)
+ return ret;
+
+ /* Enable the interrupt line, according to the command */
+ ret = regmap_assign_bits(st->regmap, ADXL345_REG_INT_ENABLE,
+ adxl345_act_int_reg[type], cmd_en);
+ if (ret)
+ return ret;
+
+ return adxl345_set_measure_en(st, true);
+}
+
/* tap */
static int _adxl345_set_tap_int(struct adxl345_state *st,
@@ -624,6 +733,31 @@ static int adxl345_write_raw(struct iio_dev *indio_dev,
return adxl345_set_measure_en(st, true);
}
+static int adxl345_read_mag_config(struct adxl345_state *st,
+ enum iio_event_direction dir,
+ enum adxl345_activity_type type_act)
+{
+ switch (dir) {
+ case IIO_EV_DIR_RISING:
+ return !!adxl345_is_act_inact_en(st, type_act);
+ default:
+ return -EINVAL;
+ }
+}
+
+static int adxl345_write_mag_config(struct adxl345_state *st,
+ enum iio_event_direction dir,
+ enum adxl345_activity_type type_act,
+ bool state)
+{
+ switch (dir) {
+ case IIO_EV_DIR_RISING:
+ return adxl345_set_act_inact_en(st, type_act, state);
+ default:
+ return -EINVAL;
+ }
+}
+
static int adxl345_read_event_config(struct iio_dev *indio_dev,
const struct iio_chan_spec *chan,
enum iio_event_type type,
@@ -634,6 +768,9 @@ static int adxl345_read_event_config(struct iio_dev *indio_dev,
int ret;
switch (type) {
+ case IIO_EV_TYPE_MAG:
+ return adxl345_read_mag_config(st, dir,
+ ADXL345_ACTIVITY);
case IIO_EV_TYPE_GESTURE:
switch (dir) {
case IIO_EV_DIR_SINGLETAP:
@@ -665,6 +802,10 @@ static int adxl345_write_event_config(struct iio_dev *indio_dev,
struct adxl345_state *st = iio_priv(indio_dev);
switch (type) {
+ case IIO_EV_TYPE_MAG:
+ return adxl345_write_mag_config(st, dir,
+ ADXL345_ACTIVITY,
+ state);
case IIO_EV_TYPE_GESTURE:
switch (dir) {
case IIO_EV_DIR_SINGLETAP:
@@ -679,6 +820,58 @@ static int adxl345_write_event_config(struct iio_dev *indio_dev,
}
}
+static int adxl345_read_mag_value(struct adxl345_state *st,
+ enum iio_event_direction dir,
+ enum iio_event_info info,
+ enum adxl345_activity_type type_act,
+ int *val, int *val2)
+{
+ unsigned int threshold;
+ int ret;
+
+ switch (info) {
+ case IIO_EV_INFO_VALUE:
+ switch (dir) {
+ case IIO_EV_DIR_RISING:
+ ret = regmap_read(st->regmap,
+ adxl345_act_thresh_reg[type_act],
+ &threshold);
+ if (ret)
+ return ret;
+ *val = 62500 * threshold;
+ *val2 = MICRO;
+ return IIO_VAL_FRACTIONAL;
+ default:
+ return -EINVAL;
+ }
+ default:
+ return -EINVAL;
+ }
+}
+
+static int adxl345_write_mag_value(struct adxl345_state *st,
+ enum iio_event_direction dir,
+ enum iio_event_info info,
+ enum adxl345_activity_type type_act,
+ int val, int val2)
+{
+ switch (info) {
+ case IIO_EV_INFO_VALUE:
+ /* Scaling factor 62.5mg/LSB, i.e. ~16g corresponds to 0xff */
+ val = DIV_ROUND_CLOSEST(val * MICRO + val2, 62500);
+ switch (dir) {
+ case IIO_EV_DIR_RISING:
+ return regmap_write(st->regmap,
+ adxl345_act_thresh_reg[type_act],
+ val);
+ 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,
@@ -691,6 +884,10 @@ static int adxl345_read_event_value(struct iio_dev *indio_dev,
int ret;
switch (type) {
+ case IIO_EV_TYPE_MAG:
+ return adxl345_read_mag_value(st, dir, info,
+ ADXL345_ACTIVITY,
+ val, val2);
case IIO_EV_TYPE_GESTURE:
switch (info) {
case IIO_EV_INFO_VALUE:
@@ -741,6 +938,13 @@ static int adxl345_write_event_value(struct iio_dev *indio_dev,
return ret;
switch (type) {
+ case IIO_EV_TYPE_MAG:
+ ret = adxl345_write_mag_value(st, dir, info,
+ ADXL345_ACTIVITY,
+ val, val2);
+ if (ret)
+ return ret;
+ break;
case IIO_EV_TYPE_GESTURE:
switch (info) {
case IIO_EV_INFO_VALUE:
@@ -980,7 +1184,8 @@ static int adxl345_fifo_push(struct iio_dev *indio_dev,
}
static int adxl345_push_event(struct iio_dev *indio_dev, int int_stat,
- enum iio_modifier tap_dir)
+ enum iio_modifier tap_dir,
+ enum iio_modifier act_dir)
{
s64 ts = iio_get_time_ns(indio_dev);
struct adxl345_state *st = iio_priv(indio_dev);
@@ -1007,6 +1212,16 @@ 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_dir,
+ IIO_EV_TYPE_MAG,
+ IIO_EV_DIR_RISING),
+ ts);
+ if (ret)
+ return ret;
+ }
+
if (FIELD_GET(ADXL345_INT_WATERMARK, int_stat)) {
samples = adxl345_get_samples(st);
if (samples < 0)
@@ -1034,6 +1249,7 @@ static irqreturn_t adxl345_irq_handler(int irq, void *p)
struct adxl345_state *st = iio_priv(indio_dev);
unsigned int regval;
enum iio_modifier tap_dir = IIO_NO_MOD;
+ enum iio_modifier act_dir = IIO_NO_MOD;
u32 axis_ctrl;
int int_stat;
int ret;
@@ -1042,7 +1258,8 @@ static irqreturn_t adxl345_irq_handler(int irq, void *p)
if (ret)
return IRQ_NONE;
- if (FIELD_GET(ADXL345_REG_TAP_AXIS_MSK, axis_ctrl)) {
+ if (FIELD_GET(ADXL345_REG_TAP_AXIS_MSK, axis_ctrl) ||
+ FIELD_GET(ADXL345_REG_ACT_AXIS_MSK, axis_ctrl)) {
ret = regmap_read(st->regmap, ADXL345_REG_ACT_TAP_STATUS, ®val);
if (ret)
return IRQ_NONE;
@@ -1053,12 +1270,19 @@ static irqreturn_t adxl345_irq_handler(int irq, void *p)
tap_dir = IIO_MOD_Y;
else if (FIELD_GET(ADXL345_TAP_X_EN, regval))
tap_dir = IIO_MOD_X;
+
+ if (FIELD_GET(ADXL345_ACT_Z_EN, regval))
+ act_dir = IIO_MOD_Z;
+ else if (FIELD_GET(ADXL345_ACT_Y_EN, regval))
+ act_dir = IIO_MOD_Y;
+ else if (FIELD_GET(ADXL345_ACT_X_EN, regval))
+ act_dir = IIO_MOD_X;
}
if (regmap_read(st->regmap, ADXL345_REG_INT_SOURCE, &int_stat))
return IRQ_NONE;
- if (adxl345_push_event(indio_dev, int_stat, tap_dir))
+ if (adxl345_push_event(indio_dev, int_stat, tap_dir, act_dir))
goto err;
if (FIELD_GET(ADXL345_INT_OVERRUN, int_stat))
@@ -1226,6 +1450,20 @@ int adxl345_core_probe(struct device *dev, struct regmap *regmap,
if (ret)
return ret;
+ /*
+ * Initialized with sensible default values to streamline
+ * sensor operation. These defaults are partly derived from
+ * the previous input driver for the ADXL345 and partly
+ * based on the recommendations provided in the datasheet.
+ */
+ ret = regmap_write(st->regmap, ADXL345_REG_ACT_INACT_CTRL, 0);
+ if (ret)
+ return ret;
+
+ ret = regmap_write(st->regmap, ADXL345_REG_THRESH_ACT, 6);
+ if (ret)
+ return ret;
+
ret = regmap_write(st->regmap, ADXL345_REG_THRESH_TAP, tap_threshold);
if (ret)
return ret;
--
2.39.5
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH v10 3/7] iio: accel: adxl345: add activity event feature
2025-06-22 15:50 ` [PATCH v10 3/7] iio: accel: adxl345: add activity event feature Lothar Rubusch
@ 2025-06-23 9:34 ` Andy Shevchenko
2025-06-23 20:57 ` Lothar Rubusch
0 siblings, 1 reply; 29+ messages in thread
From: Andy Shevchenko @ 2025-06-23 9:34 UTC (permalink / raw)
To: Lothar Rubusch
Cc: lars, Michael.Hennerich, jic23, dlechner, nuno.sa, andy, corbet,
linux-iio, linux-kernel, linux-doc, eraretuya
On Sun, Jun 22, 2025 at 03:50:06PM +0000, Lothar Rubusch wrote:
> Enable the sensor to detect activity and trigger interrupts accordingly.
> Activity events are determined based on a threshold, which is initialized
> to a sensible default during probe. This default value is adopted from the
> legacy ADXL345 input driver to maintain consistent behavior.
>
> The combination of activity detection, ODR configuration, and range
> settings lays the groundwork for the activity/inactivity hysteresis
> mechanism, which will be implemented in a subsequent patch. As such,
> portions of this patch prepare switch-case structures to support those
> upcoming changes.
...
> +static int adxl345_set_act_inact_en(struct adxl345_state *st,
> + enum adxl345_activity_type type,
> + bool cmd_en)
> +{
> + unsigned int axis_ctrl;
> + unsigned int threshold;
> + int ret;
> +
> + if (cmd_en) {
> + /* When turning on, check if threshold is valid */
> + ret = regmap_read(st->regmap,
> + adxl345_act_thresh_reg[type],
> + &threshold);
Can occupy less LoCs.
> + if (ret)
> + return ret;
> +
> + if (!threshold) /* Just ignore the command if threshold is 0 */
> + return 0;
> + }
> +
> + /* Start modifying configuration registers */
> + ret = adxl345_set_measure_en(st, false);
> + if (ret)
> + return ret;
> +
> + /* Enable axis according to the command */
> + switch (type) {
> + case ADXL345_ACTIVITY:
> + axis_ctrl = ADXL345_ACT_X_EN | ADXL345_ACT_Y_EN |
> + ADXL345_ACT_Z_EN;
I think
axis_ctrl =
ADXL345_ACT_X_EN | ADXL345_ACT_Y_EN | ADXL345_ACT_Z_EN;
is slightly better to read.
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + ret = regmap_assign_bits(st->regmap, ADXL345_REG_ACT_INACT_CTRL,
> + axis_ctrl, cmd_en);
> + if (ret)
> + return ret;
> +
> + /* Enable the interrupt line, according to the command */
> + ret = regmap_assign_bits(st->regmap, ADXL345_REG_INT_ENABLE,
> + adxl345_act_int_reg[type], cmd_en);
> + if (ret)
> + return ret;
> +
> + return adxl345_set_measure_en(st, true);
> +}
...
> + case IIO_EV_TYPE_MAG:
> + return adxl345_read_mag_config(st, dir,
> + ADXL345_ACTIVITY);
It looks like you set the editor to wrap at 72 characters, but here the single
line less than 80! Note that the limit is *exactly* 80 character.
...
> + case IIO_EV_TYPE_MAG:
> + return adxl345_write_mag_config(st, dir,
> + ADXL345_ACTIVITY,
Ditto.
...
> + return adxl345_read_mag_value(st, dir, info,
> + ADXL345_ACTIVITY,
> + val, val2);
Ditto and so on...
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH v10 3/7] iio: accel: adxl345: add activity event feature
2025-06-23 9:34 ` Andy Shevchenko
@ 2025-06-23 20:57 ` Lothar Rubusch
2025-06-24 7:28 ` Andy Shevchenko
2025-06-28 16:05 ` Jonathan Cameron
0 siblings, 2 replies; 29+ messages in thread
From: Lothar Rubusch @ 2025-06-23 20:57 UTC (permalink / raw)
To: Andy Shevchenko
Cc: lars, Michael.Hennerich, jic23, dlechner, nuno.sa, andy, corbet,
linux-iio, linux-kernel, linux-doc, eraretuya
Hi Andy,
Thank you so much. I really appreciate your quick feedback. I'll try
to implement
the changes in another version, as far as needed.
Talking about the 80 characters, let me give an anser inlined down below.
On Mon, Jun 23, 2025 at 11:34 AM Andy Shevchenko
<andriy.shevchenko@intel.com> wrote:
>
> On Sun, Jun 22, 2025 at 03:50:06PM +0000, Lothar Rubusch wrote:
> > Enable the sensor to detect activity and trigger interrupts accordingly.
> > Activity events are determined based on a threshold, which is initialized
> > to a sensible default during probe. This default value is adopted from the
> > legacy ADXL345 input driver to maintain consistent behavior.
> >
> > The combination of activity detection, ODR configuration, and range
> > settings lays the groundwork for the activity/inactivity hysteresis
> > mechanism, which will be implemented in a subsequent patch. As such,
> > portions of this patch prepare switch-case structures to support those
> > upcoming changes.
>
> ...
>
> > +static int adxl345_set_act_inact_en(struct adxl345_state *st,
> > + enum adxl345_activity_type type,
> > + bool cmd_en)
> > +{
> > + unsigned int axis_ctrl;
> > + unsigned int threshold;
> > + int ret;
> > +
> > + if (cmd_en) {
> > + /* When turning on, check if threshold is valid */
>
> > + ret = regmap_read(st->regmap,
> > + adxl345_act_thresh_reg[type],
> > + &threshold);
>
> Can occupy less LoCs.
>
> > + if (ret)
> > + return ret;
> > +
> > + if (!threshold) /* Just ignore the command if threshold is 0 */
> > + return 0;
> > + }
> > +
> > + /* Start modifying configuration registers */
> > + ret = adxl345_set_measure_en(st, false);
> > + if (ret)
> > + return ret;
> > +
> > + /* Enable axis according to the command */
> > + switch (type) {
> > + case ADXL345_ACTIVITY:
>
> > + axis_ctrl = ADXL345_ACT_X_EN | ADXL345_ACT_Y_EN |
> > + ADXL345_ACT_Z_EN;
>
> I think
>
> axis_ctrl =
> ADXL345_ACT_X_EN | ADXL345_ACT_Y_EN | ADXL345_ACT_Z_EN;
>
> is slightly better to read.
>
Agree.
> > + break;
> > + default:
> > + return -EINVAL;
> > + }
> > +
> > + ret = regmap_assign_bits(st->regmap, ADXL345_REG_ACT_INACT_CTRL,
> > + axis_ctrl, cmd_en);
> > + if (ret)
> > + return ret;
> > +
> > + /* Enable the interrupt line, according to the command */
> > + ret = regmap_assign_bits(st->regmap, ADXL345_REG_INT_ENABLE,
> > + adxl345_act_int_reg[type], cmd_en);
> > + if (ret)
> > + return ret;
> > +
> > + return adxl345_set_measure_en(st, true);
> > +}
>
> ...
>
> > + case IIO_EV_TYPE_MAG:
> > + return adxl345_read_mag_config(st, dir,
> > + ADXL345_ACTIVITY);
>
> It looks like you set the editor to wrap at 72 characters, but here the single
> line less than 80! Note that the limit is *exactly* 80 character.
>
I have my setup adjusted to 80 characters. Anyway, the cases here is
different, it needs
to be seen in context of the follow up patches. I tried to prepare the
patches now in a way
where changes are mostly "added". Is this correct and desired patch preparation?
In the particular case, this patch now adds ACTIVITY. A follow up
patch will add INACTIVITY.
Since this is still building up, it will add yet another argument to
those functions, i.e.
> > + return adxl345_write_mag_config(st, dir,
> > + ADXL345_ACTIVITY,
will become, later
> > return adxl345_write_mag_config(st, dir,
> > ADXL345_ACTIVITY,
> > + ADXL345_INACTIVITY,
To make the change more additive, I did linebreaks earlier than 80
characters. Is this
legitimate in this case?
If so, I'll keep all related formatting as is (and will only change
the other requests).
Otherwise, I can do it differently and adopt all the formatting
changes to prioritize 80 characters.
Please let me know, what you think.
Best,
L
> ...
>
> > + case IIO_EV_TYPE_MAG:
> > + return adxl345_write_mag_config(st, dir,
> > + ADXL345_ACTIVITY,
>
> Ditto.
>
> ...
>
> > + return adxl345_read_mag_value(st, dir, info,
> > + ADXL345_ACTIVITY,
> > + val, val2);
>
> Ditto and so on...
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH v10 3/7] iio: accel: adxl345: add activity event feature
2025-06-23 20:57 ` Lothar Rubusch
@ 2025-06-24 7:28 ` Andy Shevchenko
2025-06-28 16:05 ` Jonathan Cameron
1 sibling, 0 replies; 29+ messages in thread
From: Andy Shevchenko @ 2025-06-24 7:28 UTC (permalink / raw)
To: Lothar Rubusch
Cc: lars, Michael.Hennerich, jic23, dlechner, nuno.sa, andy, corbet,
linux-iio, linux-kernel, linux-doc, eraretuya
On Mon, Jun 23, 2025 at 10:57:39PM +0200, Lothar Rubusch wrote:
> On Mon, Jun 23, 2025 at 11:34 AM Andy Shevchenko
> <andriy.shevchenko@intel.com> wrote:
> > On Sun, Jun 22, 2025 at 03:50:06PM +0000, Lothar Rubusch wrote:
...
> > > + case IIO_EV_TYPE_MAG:
> > > + return adxl345_read_mag_config(st, dir,
> > > + ADXL345_ACTIVITY);
> >
> > It looks like you set the editor to wrap at 72 characters, but here the single
> > line less than 80! Note that the limit is *exactly* 80 character.
> >
>
> I have my setup adjusted to 80 characters. Anyway, the cases here is
> different, it needs
> to be seen in context of the follow up patches. I tried to prepare the
> patches now in a way
> where changes are mostly "added". Is this correct and desired patch preparation?
>
> In the particular case, this patch now adds ACTIVITY. A follow up
> patch will add INACTIVITY.
> Since this is still building up, it will add yet another argument to
> those functions, i.e.
> > > + return adxl345_write_mag_config(st, dir,
> > > + ADXL345_ACTIVITY,
>
> will become, later
> > > return adxl345_write_mag_config(st, dir,
> > > ADXL345_ACTIVITY,
> > > + ADXL345_INACTIVITY,
Yeah, but with the difference that you still remove the added line in the case
above (as this example is not the same as what we are talking about).
I think you wanted more something like
return adxl345_read_mag_config(st, dir,
ADXL345_ACTIVITY);
ito become
return adxl345_read_mag_config(st, dir,
ADXL345_INACTIVITY,
ADXL345_ACTIVITY);
> To make the change more additive, I did linebreaks earlier than 80
> characters. Is this
> legitimate in this case?
I think so.
> If so, I'll keep all related formatting as is (and will only change
> the other requests).
Sure.
> Otherwise, I can do it differently and adopt all the formatting
> changes to prioritize 80 characters.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v10 3/7] iio: accel: adxl345: add activity event feature
2025-06-23 20:57 ` Lothar Rubusch
2025-06-24 7:28 ` Andy Shevchenko
@ 2025-06-28 16:05 ` Jonathan Cameron
1 sibling, 0 replies; 29+ messages in thread
From: Jonathan Cameron @ 2025-06-28 16:05 UTC (permalink / raw)
To: Lothar Rubusch
Cc: Andy Shevchenko, lars, Michael.Hennerich, dlechner, nuno.sa, andy,
corbet, linux-iio, linux-kernel, linux-doc, eraretuya
> >
> > > + axis_ctrl = ADXL345_ACT_X_EN | ADXL345_ACT_Y_EN |
> > > + ADXL345_ACT_Z_EN;
> >
> > I think
> >
> > axis_ctrl =
> > ADXL345_ACT_X_EN | ADXL345_ACT_Y_EN | ADXL345_ACT_Z_EN;
> >
> > is slightly better to read.
Ugly enough I'd just go long on this one - it's on a little over 80 chars anyway.
axis_ctrl = ADXL345_ACT_X_EN | ADXL345_ACT_Y_EN | ADXL345_ACT_Z_EN;
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v10 4/7] iio: accel: adxl345: add inactivity feature
2025-06-22 15:50 [PATCH v10 0/7] iio: accel: adxl345: add interrupt based sensor events Lothar Rubusch
` (2 preceding siblings ...)
2025-06-22 15:50 ` [PATCH v10 3/7] iio: accel: adxl345: add activity event feature Lothar Rubusch
@ 2025-06-22 15:50 ` Lothar Rubusch
2025-06-23 9:44 ` Andy Shevchenko
2025-06-22 15:50 ` [PATCH v10 5/7] iio: accel: adxl345: add coupling detection for activity/inactivity Lothar Rubusch
` (3 subsequent siblings)
7 siblings, 1 reply; 29+ messages in thread
From: Lothar Rubusch @ 2025-06-22 15:50 UTC (permalink / raw)
To: lars, Michael.Hennerich, jic23, dlechner, nuno.sa, andy, corbet
Cc: linux-iio, linux-kernel, linux-doc, eraretuya, l.rubusch
Add support for the sensor’s inactivity feature in the driver. When both
activity and inactivity detection are enabled, the sensor sets a link bit
that ties the two functions together. This also enables auto-sleep mode,
allowing the sensor to automatically enter sleep state upon detecting
inactivity.
Inactivity detection relies on a configurable threshold and a specified
time period. If sensor measurements remain below the threshold for the
defined duration, the sensor transitions to the inactivity state.
When an Output Data Rate (ODR) is set, the inactivity time period is
automatically adjusted to a sensible default. Higher ODRs result in shorter
inactivity timeouts, while lower ODRs allow longer durations-within
reasonable upper and lower bounds. This is important because features like
auto-sleep operate effectively only between 12.5 Hz and 400 Hz. These
defaults are applied when the sample rate is modified, but users can
override them by explicitly setting a custom inactivity timeout.
Similarly, configuring the g-range provides default threshold values for
both activity and inactivity detection. These are implicit defaults meant
to simplify configuration, but they can also be manually overridden as
needed.
Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
---
drivers/iio/accel/adxl345_core.c | 240 ++++++++++++++++++++++++++++++-
1 file changed, 236 insertions(+), 4 deletions(-)
diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c
index 99b590e67967..3faf1af013c7 100644
--- a/drivers/iio/accel/adxl345_core.c
+++ b/drivers/iio/accel/adxl345_core.c
@@ -37,11 +37,17 @@
#define ADXL345_REG_TAP_SUPPRESS_MSK BIT(3)
#define ADXL345_REG_TAP_SUPPRESS 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)
#define ADXL345_TAP_Z_EN BIT(0)
#define ADXL345_TAP_Y_EN BIT(1)
#define ADXL345_TAP_X_EN BIT(2)
+#define ADXL345_INACT_Z_EN BIT(0)
+#define ADXL345_INACT_Y_EN BIT(1)
+#define ADXL345_INACT_X_EN BIT(2)
+
#define ADXL345_ACT_Z_EN BIT(4)
#define ADXL345_ACT_Y_EN BIT(5)
#define ADXL345_ACT_X_EN BIT(6)
@@ -72,14 +78,17 @@ static const unsigned int adxl345_tap_time_reg[] = {
/* activity/inactivity */
enum adxl345_activity_type {
ADXL345_ACTIVITY,
+ ADXL345_INACTIVITY,
};
static const unsigned int adxl345_act_int_reg[] = {
[ADXL345_ACTIVITY] = ADXL345_INT_ACTIVITY,
+ [ADXL345_INACTIVITY] = ADXL345_INT_INACTIVITY,
};
static const unsigned int adxl345_act_thresh_reg[] = {
[ADXL345_ACTIVITY] = ADXL345_REG_THRESH_ACT,
+ [ADXL345_INACTIVITY] = ADXL345_REG_THRESH_INACT,
};
enum adxl345_odr {
@@ -147,6 +156,14 @@ static const int adxl345_fullres_range_tbl[][2] = {
[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;
@@ -213,10 +230,29 @@ enum adxl345_chans {
chan_x, chan_y, chan_z,
};
+static const struct iio_event_spec adxl345_fake_chan_events[] = {
+ {
+ /* inactivity */
+ .type = IIO_EV_TYPE_MAG,
+ .dir = IIO_EV_DIR_FALLING,
+ .mask_separate = BIT(IIO_EV_INFO_ENABLE),
+ .mask_shared_by_type = BIT(IIO_EV_INFO_VALUE) |
+ BIT(IIO_EV_INFO_PERIOD),
+ },
+};
+
static const struct iio_chan_spec adxl345_channels[] = {
ADXL345_CHANNEL(0, chan_x, X),
ADXL345_CHANNEL(1, chan_y, Y),
ADXL345_CHANNEL(2, chan_z, Z),
+ {
+ .type = IIO_ACCEL,
+ .modified = 1,
+ .channel2 = IIO_MOD_X_AND_Y_AND_Z,
+ .scan_index = -1, /* Fake channel */
+ .event_spec = adxl345_fake_chan_events,
+ .num_event_specs = ARRAY_SIZE(adxl345_fake_chan_events),
+ },
};
static const unsigned long adxl345_scan_masks[] = {
@@ -264,6 +300,52 @@ static int adxl345_set_measure_en(struct adxl345_state *st, bool en)
/* activity / inactivity */
+/**
+ * adxl345_set_inact_time - Configure inactivity time explicitly or by ODR.
+ * @st: The sensor state instance.
+ * @val_s: A desired time value, between 0 and 255.
+ *
+ * Inactivity time can be configured between 1 and 255 seconds. If a user sets
+ * val_s to 0, a default inactivity time is calculated automatically (since 0 is
+ * also invalid and undefined by the sensor).
+ *
+ * In such cases, power consumption should be considered: the inactivity period
+ * should be shorter at higher sampling frequencies and longer at lower ones.
+ * Specifically, for frequencies above 255 Hz, the default is set to 10 seconds;
+ * for frequencies below 10 Hz, it defaults to 255 seconds.
+ *
+ * The calculation method subtracts the integer part of the configured sample
+ * frequency from 255 to estimate the inactivity time in seconds. Sub-Hertz
+ * values are ignored in this approximation. Since the recommended output data
+ * rates (ODRs) for features like activity/inactivity detection, sleep modes,
+ * and free fall range between 12.5 Hz and 400 Hz, frequencies outside this
+ * range will either use the defined boundary defaults or require explicit
+ * configuration via val_s.
+ *
+ * Return: 0 or error value.
+ */
+static int adxl345_set_inact_time(struct adxl345_state *st, u32 val_s)
+{
+ int max_boundary = U8_MAX;
+ int min_boundary = 10;
+ unsigned int val = min(val_s, U8_MAX);
+ enum adxl345_odr odr;
+ unsigned int regval;
+ int ret;
+
+ if (val == 0) {
+ ret = regmap_read(st->regmap, ADXL345_REG_BW_RATE, ®val);
+ if (ret)
+ return ret;
+
+ odr = FIELD_GET(ADXL345_BW_RATE_MSK, regval);
+ val = clamp(max_boundary - adxl345_odr_tbl[odr][0],
+ min_boundary, max_boundary);
+ }
+
+ return regmap_write(st->regmap, ADXL345_REG_TIME_INACT, val);
+}
+
static int adxl345_is_act_inact_en(struct adxl345_state *st,
enum adxl345_activity_type type)
{
@@ -285,6 +367,13 @@ static int adxl345_is_act_inact_en(struct adxl345_state *st,
if (!en)
return false;
break;
+ case ADXL345_INACTIVITY:
+ en = FIELD_GET(ADXL345_INACT_X_EN, axis_ctrl) |
+ FIELD_GET(ADXL345_INACT_Y_EN, axis_ctrl) |
+ FIELD_GET(ADXL345_INACT_Z_EN, axis_ctrl);
+ if (!en)
+ return false;
+ break;
default:
return -EINVAL;
}
@@ -297,12 +386,32 @@ static int adxl345_is_act_inact_en(struct adxl345_state *st,
return adxl345_act_int_reg[type] & regval;
}
+static int adxl345_set_act_inact_linkbit(struct adxl345_state *st,
+ enum adxl345_activity_type type,
+ bool en)
+{
+ int act_en, inact_en;
+
+ act_en = adxl345_is_act_inact_en(st, ADXL345_ACTIVITY);
+ if (act_en < 0)
+ return act_en;
+
+ inact_en = adxl345_is_act_inact_en(st, ADXL345_INACTIVITY);
+ if (inact_en < 0)
+ return inact_en;
+
+ return regmap_assign_bits(st->regmap, ADXL345_REG_POWER_CTL,
+ ADXL345_POWER_CTL_INACT_MSK,
+ en && act_en && inact_en);
+}
+
static int adxl345_set_act_inact_en(struct adxl345_state *st,
enum adxl345_activity_type type,
bool cmd_en)
{
unsigned int axis_ctrl;
unsigned int threshold;
+ unsigned int period;
int ret;
if (cmd_en) {
@@ -315,6 +424,18 @@ static int adxl345_set_act_inact_en(struct adxl345_state *st,
if (!threshold) /* Just ignore the command if threshold is 0 */
return 0;
+
+ /* When turning on inactivity, check if inact time is valid */
+ if (type == ADXL345_INACTIVITY) {
+ ret = regmap_read(st->regmap,
+ ADXL345_REG_TIME_INACT,
+ &period);
+ if (ret)
+ return ret;
+
+ if (!period)
+ return 0;
+ }
}
/* Start modifying configuration registers */
@@ -328,6 +449,10 @@ static int adxl345_set_act_inact_en(struct adxl345_state *st,
axis_ctrl = ADXL345_ACT_X_EN | ADXL345_ACT_Y_EN |
ADXL345_ACT_Z_EN;
break;
+ case ADXL345_INACTIVITY:
+ axis_ctrl = ADXL345_INACT_X_EN | ADXL345_INACT_Y_EN |
+ ADXL345_INACT_Z_EN;
+ break;
default:
return -EINVAL;
}
@@ -343,6 +468,11 @@ static int adxl345_set_act_inact_en(struct adxl345_state *st,
if (ret)
return ret;
+ /* Set link-bit and auto-sleep only when ACT and INACT are enabled */
+ ret = adxl345_set_act_inact_linkbit(st, type, cmd_en);
+ if (ret)
+ return ret;
+
return adxl345_set_measure_en(st, true);
}
@@ -575,9 +705,16 @@ static int adxl345_find_odr(struct adxl345_state *st, int val,
static int adxl345_set_odr(struct adxl345_state *st, enum adxl345_odr odr)
{
- return regmap_update_bits(st->regmap, ADXL345_REG_BW_RATE,
+ 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;
+
+ /* update inactivity time by ODR */
+ return adxl345_set_inact_time(st, 0);
}
static int adxl345_find_range(struct adxl345_state *st, int val, int val2,
@@ -598,9 +735,49 @@ static int adxl345_find_range(struct adxl345_state *st, int val, int val2,
static int adxl345_set_range(struct adxl345_state *st, enum adxl345_range range)
{
- return regmap_update_bits(st->regmap, ADXL345_REG_DATA_FORMAT,
+ unsigned int act_threshold, inact_threshold;
+ unsigned int range_old;
+ unsigned int regval;
+ int ret;
+
+ ret = regmap_read(st->regmap, ADXL345_REG_DATA_FORMAT, ®val);
+ if (ret)
+ return ret;
+ range_old = FIELD_GET(ADXL345_DATA_FORMAT_RANGE, regval);
+
+ ret = regmap_read(st->regmap,
+ adxl345_act_thresh_reg[ADXL345_ACTIVITY],
+ &act_threshold);
+ if (ret)
+ return ret;
+
+ ret = regmap_read(st->regmap,
+ adxl345_act_thresh_reg[ADXL345_INACTIVITY],
+ &inact_threshold);
+ if (ret)
+ return 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;
+
+ act_threshold = act_threshold * adxl345_range_factor_tbl[range_old]
+ / adxl345_range_factor_tbl[range];
+ act_threshold = min(U8_MAX, max(1, act_threshold));
+
+ inact_threshold = inact_threshold * adxl345_range_factor_tbl[range_old]
+ / adxl345_range_factor_tbl[range];
+ inact_threshold = min(U8_MAX, max(1, inact_threshold));
+
+ ret = regmap_write(st->regmap, adxl345_act_thresh_reg[ADXL345_ACTIVITY],
+ act_threshold);
+ if (ret)
+ return ret;
+
+ return regmap_write(st->regmap, adxl345_act_thresh_reg[ADXL345_INACTIVITY],
+ inact_threshold);
}
static int adxl345_read_avail(struct iio_dev *indio_dev,
@@ -735,11 +912,14 @@ static int adxl345_write_raw(struct iio_dev *indio_dev,
static int adxl345_read_mag_config(struct adxl345_state *st,
enum iio_event_direction dir,
- enum adxl345_activity_type type_act)
+ enum adxl345_activity_type type_act,
+ enum adxl345_activity_type type_inact)
{
switch (dir) {
case IIO_EV_DIR_RISING:
return !!adxl345_is_act_inact_en(st, type_act);
+ case IIO_EV_DIR_FALLING:
+ return !!adxl345_is_act_inact_en(st, type_inact);
default:
return -EINVAL;
}
@@ -748,11 +928,14 @@ static int adxl345_read_mag_config(struct adxl345_state *st,
static int adxl345_write_mag_config(struct adxl345_state *st,
enum iio_event_direction dir,
enum adxl345_activity_type type_act,
+ enum adxl345_activity_type type_inact,
bool state)
{
switch (dir) {
case IIO_EV_DIR_RISING:
return adxl345_set_act_inact_en(st, type_act, state);
+ case IIO_EV_DIR_FALLING:
+ return adxl345_set_act_inact_en(st, type_inact, state);
default:
return -EINVAL;
}
@@ -770,7 +953,8 @@ static int adxl345_read_event_config(struct iio_dev *indio_dev,
switch (type) {
case IIO_EV_TYPE_MAG:
return adxl345_read_mag_config(st, dir,
- ADXL345_ACTIVITY);
+ ADXL345_ACTIVITY,
+ ADXL345_INACTIVITY);
case IIO_EV_TYPE_GESTURE:
switch (dir) {
case IIO_EV_DIR_SINGLETAP:
@@ -805,6 +989,7 @@ static int adxl345_write_event_config(struct iio_dev *indio_dev,
case IIO_EV_TYPE_MAG:
return adxl345_write_mag_config(st, dir,
ADXL345_ACTIVITY,
+ ADXL345_INACTIVITY,
state);
case IIO_EV_TYPE_GESTURE:
switch (dir) {
@@ -824,9 +1009,11 @@ static int adxl345_read_mag_value(struct adxl345_state *st,
enum iio_event_direction dir,
enum iio_event_info info,
enum adxl345_activity_type type_act,
+ enum adxl345_activity_type type_inact,
int *val, int *val2)
{
unsigned int threshold;
+ unsigned int period;
int ret;
switch (info) {
@@ -841,9 +1028,26 @@ static int adxl345_read_mag_value(struct adxl345_state *st,
*val = 62500 * threshold;
*val2 = MICRO;
return IIO_VAL_FRACTIONAL;
+ case IIO_EV_DIR_FALLING:
+ ret = regmap_read(st->regmap,
+ adxl345_act_thresh_reg[type_inact],
+ &threshold);
+ if (ret)
+ return ret;
+ *val = 62500 * threshold;
+ *val2 = MICRO;
+ return IIO_VAL_FRACTIONAL;
default:
return -EINVAL;
}
+ case IIO_EV_INFO_PERIOD:
+ ret = regmap_read(st->regmap,
+ ADXL345_REG_TIME_INACT,
+ &period);
+ if (ret)
+ return ret;
+ *val = period;
+ return IIO_VAL_INT;
default:
return -EINVAL;
}
@@ -853,6 +1057,7 @@ static int adxl345_write_mag_value(struct adxl345_state *st,
enum iio_event_direction dir,
enum iio_event_info info,
enum adxl345_activity_type type_act,
+ enum adxl345_activity_type type_inact,
int val, int val2)
{
switch (info) {
@@ -864,9 +1069,15 @@ static int adxl345_write_mag_value(struct adxl345_state *st,
return regmap_write(st->regmap,
adxl345_act_thresh_reg[type_act],
val);
+ case IIO_EV_DIR_FALLING:
+ return regmap_write(st->regmap,
+ adxl345_act_thresh_reg[type_inact],
+ val);
default:
return -EINVAL;
}
+ case IIO_EV_INFO_PERIOD:
+ return adxl345_set_inact_time(st, val);
default:
return -EINVAL;
}
@@ -887,6 +1098,7 @@ static int adxl345_read_event_value(struct iio_dev *indio_dev,
case IIO_EV_TYPE_MAG:
return adxl345_read_mag_value(st, dir, info,
ADXL345_ACTIVITY,
+ ADXL345_INACTIVITY,
val, val2);
case IIO_EV_TYPE_GESTURE:
switch (info) {
@@ -941,6 +1153,7 @@ static int adxl345_write_event_value(struct iio_dev *indio_dev,
case IIO_EV_TYPE_MAG:
ret = adxl345_write_mag_value(st, dir, info,
ADXL345_ACTIVITY,
+ ADXL345_INACTIVITY,
val, val2);
if (ret)
return ret;
@@ -1222,6 +1435,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_AND_Y_AND_Z,
+ IIO_EV_TYPE_MAG,
+ IIO_EV_DIR_FALLING),
+ ts);
+ if (ret)
+ return ret;
+ }
+
if (FIELD_GET(ADXL345_INT_WATERMARK, int_stat)) {
samples = adxl345_get_samples(st);
if (samples < 0)
@@ -1460,10 +1684,18 @@ int adxl345_core_probe(struct device *dev, struct regmap *regmap,
if (ret)
return ret;
+ ret = regmap_write(st->regmap, ADXL345_REG_TIME_INACT, 0x37);
+ if (ret)
+ return ret;
+
ret = regmap_write(st->regmap, ADXL345_REG_THRESH_ACT, 6);
if (ret)
return ret;
+ ret = regmap_write(st->regmap, ADXL345_REG_THRESH_INACT, 4);
+ if (ret)
+ return ret;
+
ret = regmap_write(st->regmap, ADXL345_REG_THRESH_TAP, tap_threshold);
if (ret)
return ret;
--
2.39.5
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH v10 4/7] iio: accel: adxl345: add inactivity feature
2025-06-22 15:50 ` [PATCH v10 4/7] iio: accel: adxl345: add inactivity feature Lothar Rubusch
@ 2025-06-23 9:44 ` Andy Shevchenko
2025-06-23 21:06 ` Lothar Rubusch
2025-06-28 16:08 ` Jonathan Cameron
0 siblings, 2 replies; 29+ messages in thread
From: Andy Shevchenko @ 2025-06-23 9:44 UTC (permalink / raw)
To: Lothar Rubusch
Cc: lars, Michael.Hennerich, jic23, dlechner, nuno.sa, andy, corbet,
linux-iio, linux-kernel, linux-doc, eraretuya
On Sun, Jun 22, 2025 at 03:50:07PM +0000, Lothar Rubusch wrote:
> Add support for the sensor’s inactivity feature in the driver. When both
> activity and inactivity detection are enabled, the sensor sets a link bit
> that ties the two functions together. This also enables auto-sleep mode,
> allowing the sensor to automatically enter sleep state upon detecting
> inactivity.
>
> Inactivity detection relies on a configurable threshold and a specified
> time period. If sensor measurements remain below the threshold for the
> defined duration, the sensor transitions to the inactivity state.
>
> When an Output Data Rate (ODR) is set, the inactivity time period is
> automatically adjusted to a sensible default. Higher ODRs result in shorter
> inactivity timeouts, while lower ODRs allow longer durations-within
> reasonable upper and lower bounds. This is important because features like
> auto-sleep operate effectively only between 12.5 Hz and 400 Hz. These
> defaults are applied when the sample rate is modified, but users can
> override them by explicitly setting a custom inactivity timeout.
>
> Similarly, configuring the g-range provides default threshold values for
> both activity and inactivity detection. These are implicit defaults meant
> to simplify configuration, but they can also be manually overridden as
> needed.
...
> +static int adxl345_set_inact_time(struct adxl345_state *st, u32 val_s)
> +{
> + int max_boundary = U8_MAX;
> + int min_boundary = 10;
> + unsigned int val = min(val_s, U8_MAX);
Wondering if it's possible to refer here to max_boundary?
In any case, split this assignment since it will be easier
to maintain.
> + enum adxl345_odr odr;
> + unsigned int regval;
> + int ret;
val = min(val_s, max_boundary);
> + if (val == 0) {
> + ret = regmap_read(st->regmap, ADXL345_REG_BW_RATE, ®val);
> + if (ret)
> + return ret;
> +
> + odr = FIELD_GET(ADXL345_BW_RATE_MSK, regval);
> + val = clamp(max_boundary - adxl345_odr_tbl[odr][0],
> + min_boundary, max_boundary);
> + }
> +
> + return regmap_write(st->regmap, ADXL345_REG_TIME_INACT, val);
> +}
...
> + case ADXL345_INACTIVITY:
> + en = FIELD_GET(ADXL345_INACT_X_EN, axis_ctrl) |
> + FIELD_GET(ADXL345_INACT_Y_EN, axis_ctrl) |
> + FIELD_GET(ADXL345_INACT_Z_EN, axis_ctrl);
As I pointed out earlier. the indentation is supposed to be on the same colomn
for 'F' letters.
> + if (!en)
> + return false;
> + break;
...
> + ret = regmap_read(st->regmap,
> + ADXL345_REG_TIME_INACT,
> + &period);
There is plenty of room on the previous lines. Depending on the next
changes (which I believe unlikely touch this) it may be packed to two
lines with a logical split, like
ret = regmap_read(st->regmap,
ADXL345_REG_TIME_INACT, &period);
It again seems the byproduct of the too strict settings of the wrap limit in
your editor.
...
> + case ADXL345_INACTIVITY:
> + axis_ctrl = ADXL345_INACT_X_EN | ADXL345_INACT_Y_EN |
> + ADXL345_INACT_Z_EN;
Consider
axis_ctrl =
ADXL345_INACT_X_EN | ADXL345_INACT_Y_EN | ADXL345_INACT_Z_EN;
(yes, I see that it's longer than 80, but it might worth doing it for the sake of
consistency with the previous suggestion).
...
> static int adxl345_set_range(struct adxl345_state *st, enum adxl345_range range)
> {
> - return regmap_update_bits(st->regmap, ADXL345_REG_DATA_FORMAT,
> + 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;
If it's a code from the previous patch, it might make sense to introduce ret
there.
> }
...
> + case IIO_EV_INFO_PERIOD:
> + ret = regmap_read(st->regmap,
> + ADXL345_REG_TIME_INACT,
> + &period);
Too short lines.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH v10 4/7] iio: accel: adxl345: add inactivity feature
2025-06-23 9:44 ` Andy Shevchenko
@ 2025-06-23 21:06 ` Lothar Rubusch
2025-06-24 7:33 ` Andy Shevchenko
2025-06-28 16:08 ` Jonathan Cameron
1 sibling, 1 reply; 29+ messages in thread
From: Lothar Rubusch @ 2025-06-23 21:06 UTC (permalink / raw)
To: Andy Shevchenko
Cc: lars, Michael.Hennerich, jic23, dlechner, nuno.sa, andy, corbet,
linux-iio, linux-kernel, linux-doc, eraretuya
On Mon, Jun 23, 2025 at 11:44 AM Andy Shevchenko
<andriy.shevchenko@intel.com> wrote:
>
> On Sun, Jun 22, 2025 at 03:50:07PM +0000, Lothar Rubusch wrote:
> > Add support for the sensor’s inactivity feature in the driver. When both
> > activity and inactivity detection are enabled, the sensor sets a link bit
> > that ties the two functions together. This also enables auto-sleep mode,
> > allowing the sensor to automatically enter sleep state upon detecting
> > inactivity.
> >
> > Inactivity detection relies on a configurable threshold and a specified
> > time period. If sensor measurements remain below the threshold for the
> > defined duration, the sensor transitions to the inactivity state.
> >
> > When an Output Data Rate (ODR) is set, the inactivity time period is
> > automatically adjusted to a sensible default. Higher ODRs result in shorter
> > inactivity timeouts, while lower ODRs allow longer durations-within
> > reasonable upper and lower bounds. This is important because features like
> > auto-sleep operate effectively only between 12.5 Hz and 400 Hz. These
> > defaults are applied when the sample rate is modified, but users can
> > override them by explicitly setting a custom inactivity timeout.
> >
> > Similarly, configuring the g-range provides default threshold values for
> > both activity and inactivity detection. These are implicit defaults meant
> > to simplify configuration, but they can also be manually overridden as
> > needed.
>
> ...
>
> > +static int adxl345_set_inact_time(struct adxl345_state *st, u32 val_s)
> > +{
> > + int max_boundary = U8_MAX;
> > + int min_boundary = 10;
> > + unsigned int val = min(val_s, U8_MAX);
>
> Wondering if it's possible to refer here to max_boundary?
> In any case, split this assignment since it will be easier
> to maintain.
>
> > + enum adxl345_odr odr;
> > + unsigned int regval;
> > + int ret;
>
> val = min(val_s, max_boundary);
>
Well, yes, that's what I had initially. Then min() needed unsigned
int, where clamp() - down below - needed signed int. At the end of the
day, I set up min() here, but later this will disappear. I was
wondering, if it's actually needed anymore, when doing clamp() anyway.
The story is a bit longer, since original version (I think I never
submitted) I started with clamp(), ran into signed / unsigned and
difference from max, that I skipped clamp() until when you complained
about it: "use clamp()"
Long story short, I'll verify this in my tests, but probably I'll
rather drop a call to min() here.
> > + if (val == 0) {
> > + ret = regmap_read(st->regmap, ADXL345_REG_BW_RATE, ®val);
> > + if (ret)
> > + return ret;
> > +
> > + odr = FIELD_GET(ADXL345_BW_RATE_MSK, regval);
> > + val = clamp(max_boundary - adxl345_odr_tbl[odr][0],
> > + min_boundary, max_boundary);
> > + }
> > +
> > + return regmap_write(st->regmap, ADXL345_REG_TIME_INACT, val);
> > +}
>
> ...
>
> > + case ADXL345_INACTIVITY:
> > + en = FIELD_GET(ADXL345_INACT_X_EN, axis_ctrl) |
> > + FIELD_GET(ADXL345_INACT_Y_EN, axis_ctrl) |
> > + FIELD_GET(ADXL345_INACT_Z_EN, axis_ctrl);
>
> As I pointed out earlier. the indentation is supposed to be on the same colomn
> for 'F' letters.
>
Let me allow a stupid question, when you mean on the same column, the
above is wrong? Can you give me an example here how to fix it?
Best,
L
> > + if (!en)
> > + return false;
> > + break;
>
> ...
>
> > + ret = regmap_read(st->regmap,
> > + ADXL345_REG_TIME_INACT,
> > + &period);
>
> There is plenty of room on the previous lines. Depending on the next
> changes (which I believe unlikely touch this) it may be packed to two
> lines with a logical split, like
>
> ret = regmap_read(st->regmap,
> ADXL345_REG_TIME_INACT, &period);
>
> It again seems the byproduct of the too strict settings of the wrap limit in
> your editor.
>
> ...
>
> > + case ADXL345_INACTIVITY:
> > + axis_ctrl = ADXL345_INACT_X_EN | ADXL345_INACT_Y_EN |
> > + ADXL345_INACT_Z_EN;
>
> Consider
> axis_ctrl =
> ADXL345_INACT_X_EN | ADXL345_INACT_Y_EN | ADXL345_INACT_Z_EN;
>
> (yes, I see that it's longer than 80, but it might worth doing it for the sake of
> consistency with the previous suggestion).
>
>
> ...
>
> > static int adxl345_set_range(struct adxl345_state *st, enum adxl345_range range)
> > {
> > - return regmap_update_bits(st->regmap, ADXL345_REG_DATA_FORMAT,
>
> > + 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;
>
> If it's a code from the previous patch, it might make sense to introduce ret
> there.
>
> > }
>
> ...
>
> > + case IIO_EV_INFO_PERIOD:
> > + ret = regmap_read(st->regmap,
> > + ADXL345_REG_TIME_INACT,
> > + &period);
>
> Too short lines.
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH v10 4/7] iio: accel: adxl345: add inactivity feature
2025-06-23 21:06 ` Lothar Rubusch
@ 2025-06-24 7:33 ` Andy Shevchenko
2025-06-24 8:27 ` Lothar Rubusch
0 siblings, 1 reply; 29+ messages in thread
From: Andy Shevchenko @ 2025-06-24 7:33 UTC (permalink / raw)
To: Lothar Rubusch
Cc: lars, Michael.Hennerich, jic23, dlechner, nuno.sa, andy, corbet,
linux-iio, linux-kernel, linux-doc, eraretuya
On Mon, Jun 23, 2025 at 11:06:44PM +0200, Lothar Rubusch wrote:
> On Mon, Jun 23, 2025 at 11:44 AM Andy Shevchenko
> <andriy.shevchenko@intel.com> wrote:
...
> > > + case ADXL345_INACTIVITY:
> > > + en = FIELD_GET(ADXL345_INACT_X_EN, axis_ctrl) |
> > > + FIELD_GET(ADXL345_INACT_Y_EN, axis_ctrl) |
> > > + FIELD_GET(ADXL345_INACT_Z_EN, axis_ctrl);
> >
> > As I pointed out earlier. the indentation is supposed to be on the same colomn
> > for 'F' letters.
> >
>
> Let me allow a stupid question, when you mean on the same column, the
> above is wrong? Can you give me an example here how to fix it?
Your mail client mangles the original text (TABs) and it's most likely
impossible to see on your side what I meant (I already answered once with
the example).
Here is the example, use https://lore.kernel.org/linux-iio to see it via Web
en = FIELD_GET(ADXL345_INACT_X_EN, axis_ctrl) |
FIELD_GET(ADXL345_INACT_Y_EN, axis_ctrl) |
FIELD_GET(ADXL345_INACT_Z_EN, axis_ctrl);
All 'F' letters occupy the same (by number) column in the sequential lines.
P.S.
Also you seems ignored my ask to remove the context you are not replying to.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v10 4/7] iio: accel: adxl345: add inactivity feature
2025-06-24 7:33 ` Andy Shevchenko
@ 2025-06-24 8:27 ` Lothar Rubusch
0 siblings, 0 replies; 29+ messages in thread
From: Lothar Rubusch @ 2025-06-24 8:27 UTC (permalink / raw)
To: Andy Shevchenko
Cc: lars, Michael.Hennerich, jic23, dlechner, nuno.sa, andy, corbet,
linux-iio, linux-kernel, linux-doc, eraretuya
...
> P.S.
> Also you seems ignored my ask to remove the context you are not replying to.
>
Sorry, I did not understand right away. Now I got you. I'll take care
of it. Thanks for the advice.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v10 4/7] iio: accel: adxl345: add inactivity feature
2025-06-23 9:44 ` Andy Shevchenko
2025-06-23 21:06 ` Lothar Rubusch
@ 2025-06-28 16:08 ` Jonathan Cameron
2025-06-28 21:10 ` Lothar Rubusch
1 sibling, 1 reply; 29+ messages in thread
From: Jonathan Cameron @ 2025-06-28 16:08 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Lothar Rubusch, lars, Michael.Hennerich, dlechner, nuno.sa, andy,
corbet, linux-iio, linux-kernel, linux-doc, eraretuya
> ...
>
> > + case ADXL345_INACTIVITY:
> > + axis_ctrl = ADXL345_INACT_X_EN | ADXL345_INACT_Y_EN |
> > + ADXL345_INACT_Z_EN;
>
> Consider
> axis_ctrl =
> ADXL345_INACT_X_EN | ADXL345_INACT_Y_EN | ADXL345_INACT_Z_EN;
>
> (yes, I see that it's longer than 80, but it might worth doing it for the sake of
> consistency with the previous suggestion).
Hmm. I'd go longer rather than do that just because it looks really ugly.
axis_ctrl = ADXL345_INACT_X_EN | ADXL345_INACT_Y_EN | ADXL345_INACT_Z_EN;
I don't care that much as long as long lines are justified by readability. Here
I think either Andy's suggestion or the all on one line are justified.
Tomorrow I may have a different view :(
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v10 4/7] iio: accel: adxl345: add inactivity feature
2025-06-28 16:08 ` Jonathan Cameron
@ 2025-06-28 21:10 ` Lothar Rubusch
2025-06-29 7:30 ` Andy Shevchenko
0 siblings, 1 reply; 29+ messages in thread
From: Lothar Rubusch @ 2025-06-28 21:10 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Andy Shevchenko, lars, Michael.Hennerich, dlechner, nuno.sa, andy,
corbet, linux-iio, linux-kernel, linux-doc, eraretuya
Hi Jonathan, Andy and the ML,
Thank you both for the review and feedback. I'll prepare another
version for the 313 and the 345.
On Sat, Jun 28, 2025 at 6:08 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
>
> > ...
> >
> > > + case ADXL345_INACTIVITY:
> > > + axis_ctrl = ADXL345_INACT_X_EN | ADXL345_INACT_Y_EN |
> > > + ADXL345_INACT_Z_EN;
> >
> > Consider
> > axis_ctrl =
> > ADXL345_INACT_X_EN | ADXL345_INACT_Y_EN | ADXL345_INACT_Z_EN;
> >
> > (yes, I see that it's longer than 80, but it might worth doing it for the sake of
> > consistency with the previous suggestion).
> Hmm. I'd go longer rather than do that just because it looks really ugly.
>
> axis_ctrl = ADXL345_INACT_X_EN | ADXL345_INACT_Y_EN | ADXL345_INACT_Z_EN;
>
> I don't care that much as long as long lines are justified by readability. Here
> I think either Andy's suggestion or the all on one line are justified.
>
> Tomorrow I may have a different view :(
>
As I’ve seen quite a bit of discussion around this. In fact, using
binary OR here might not even be necessary, since I can define
ADXL345_ACT_XYZ_EN and ADXL345_INACT_XYZ_EN directly and OR the fields
in the header. If you have no objections, I’ll likely prepare this
change for the next version.
Best,
L
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v10 4/7] iio: accel: adxl345: add inactivity feature
2025-06-28 21:10 ` Lothar Rubusch
@ 2025-06-29 7:30 ` Andy Shevchenko
2025-06-29 16:28 ` Jonathan Cameron
0 siblings, 1 reply; 29+ messages in thread
From: Andy Shevchenko @ 2025-06-29 7:30 UTC (permalink / raw)
To: Lothar Rubusch
Cc: Jonathan Cameron, Andy Shevchenko, lars, Michael.Hennerich,
dlechner, nuno.sa, andy, corbet, linux-iio, linux-kernel,
linux-doc, eraretuya
On Sun, Jun 29, 2025 at 12:11 AM Lothar Rubusch <l.rubusch@gmail.com> wrote:
> On Sat, Jun 28, 2025 at 6:08 PM Jonathan Cameron <jic23@kernel.org> wrote:
...
> > > > + axis_ctrl = ADXL345_INACT_X_EN | ADXL345_INACT_Y_EN |
> > > > + ADXL345_INACT_Z_EN;
> > >
> > > Consider
> > > axis_ctrl =
> > > ADXL345_INACT_X_EN | ADXL345_INACT_Y_EN | ADXL345_INACT_Z_EN;
> > >
> > > (yes, I see that it's longer than 80, but it might worth doing it for the sake of
> > > consistency with the previous suggestion).
> > Hmm. I'd go longer rather than do that just because it looks really ugly.
> >
> > axis_ctrl = ADXL345_INACT_X_EN | ADXL345_INACT_Y_EN | ADXL345_INACT_Z_EN;
> >
> > I don't care that much as long as long lines are justified by readability. Here
> > I think either Andy's suggestion or the all on one line are justified.
> >
> > Tomorrow I may have a different view :(
> >
>
> As I’ve seen quite a bit of discussion around this. In fact, using
> binary OR here might not even be necessary, since I can define
> ADXL345_ACT_XYZ_EN and ADXL345_INACT_XYZ_EN directly and OR the fields
> in the header. If you have no objections, I’ll likely prepare this
> change for the next version.
Actually I like your idea. This will be sustainable over style
preference changes.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v10 4/7] iio: accel: adxl345: add inactivity feature
2025-06-29 7:30 ` Andy Shevchenko
@ 2025-06-29 16:28 ` Jonathan Cameron
0 siblings, 0 replies; 29+ messages in thread
From: Jonathan Cameron @ 2025-06-29 16:28 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Lothar Rubusch, Andy Shevchenko, lars, Michael.Hennerich,
dlechner, nuno.sa, andy, corbet, linux-iio, linux-kernel,
linux-doc, eraretuya
On Sun, 29 Jun 2025 10:30:15 +0300
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> On Sun, Jun 29, 2025 at 12:11 AM Lothar Rubusch <l.rubusch@gmail.com> wrote:
> > On Sat, Jun 28, 2025 at 6:08 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> ...
>
> > > > > + axis_ctrl = ADXL345_INACT_X_EN | ADXL345_INACT_Y_EN |
> > > > > + ADXL345_INACT_Z_EN;
> > > >
> > > > Consider
> > > > axis_ctrl =
> > > > ADXL345_INACT_X_EN | ADXL345_INACT_Y_EN | ADXL345_INACT_Z_EN;
> > > >
> > > > (yes, I see that it's longer than 80, but it might worth doing it for the sake of
> > > > consistency with the previous suggestion).
> > > Hmm. I'd go longer rather than do that just because it looks really ugly.
> > >
> > > axis_ctrl = ADXL345_INACT_X_EN | ADXL345_INACT_Y_EN | ADXL345_INACT_Z_EN;
> > >
> > > I don't care that much as long as long lines are justified by readability. Here
> > > I think either Andy's suggestion or the all on one line are justified.
> > >
> > > Tomorrow I may have a different view :(
> > >
> >
> > As I’ve seen quite a bit of discussion around this. In fact, using
> > binary OR here might not even be necessary, since I can define
> > ADXL345_ACT_XYZ_EN and ADXL345_INACT_XYZ_EN directly and OR the fields
> > in the header. If you have no objections, I’ll likely prepare this
> > change for the next version.
>
> Actually I like your idea. This will be sustainable over style
> preference changes.
>
Agreed.
Jonathan
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v10 5/7] iio: accel: adxl345: add coupling detection for activity/inactivity
2025-06-22 15:50 [PATCH v10 0/7] iio: accel: adxl345: add interrupt based sensor events Lothar Rubusch
` (3 preceding siblings ...)
2025-06-22 15:50 ` [PATCH v10 4/7] iio: accel: adxl345: add inactivity feature Lothar Rubusch
@ 2025-06-22 15:50 ` Lothar Rubusch
2025-06-23 10:11 ` Andy Shevchenko
2025-06-22 15:50 ` [PATCH v10 6/7] iio: accel: adxl345: extend inactivity time for less than 1s Lothar Rubusch
` (2 subsequent siblings)
7 siblings, 1 reply; 29+ messages in thread
From: Lothar Rubusch @ 2025-06-22 15:50 UTC (permalink / raw)
To: lars, Michael.Hennerich, jic23, dlechner, nuno.sa, andy, corbet
Cc: linux-iio, linux-kernel, linux-doc, eraretuya, l.rubusch
Enable AC/DC coupling configuration for activity and inactivity detection
by setting the AC/DC bit. Extend existing magnitude-based detection with
adaptive AC-coupled mode.
Use DC-coupled mode to compare acceleration samples directly against
configured thresholds. Use AC-coupled mode to compare samples against a
reference taken at the start of activity detection. Implement DC-coupled
events using MAG, and AC-coupled events using MAG_ADAPTIVE.
Expose configuration of thresholds and periods via separate sysfs handles.
Note that both coupling modes share the same sensor registers, so activity
or inactivity detection cannot be configured for both AC and DC
simultaneously. Apply the most recently configured mode.
Simplify event handling and support adaptive AC-coupling.
Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
---
drivers/iio/accel/adxl345_core.c | 253 +++++++++++++++++++++++++++++--
1 file changed, 238 insertions(+), 15 deletions(-)
diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c
index 3faf1af013c7..c5a3dac5f938 100644
--- a/drivers/iio/accel/adxl345_core.c
+++ b/drivers/iio/accel/adxl345_core.c
@@ -38,6 +38,8 @@
#define ADXL345_REG_TAP_SUPPRESS BIT(3)
#define ADXL345_REG_ACT_AXIS_MSK GENMASK(6, 4)
#define ADXL345_REG_INACT_AXIS_MSK GENMASK(2, 0)
+#define ADXL345_REG_ACT_ACDC_MSK BIT(7)
+#define ADXL345_REG_INACT_ACDC_MSK BIT(3)
#define ADXL345_POWER_CTL_INACT_MSK (ADXL345_POWER_CTL_AUTO_SLEEP | ADXL345_POWER_CTL_LINK)
#define ADXL345_TAP_Z_EN BIT(0)
@@ -52,6 +54,9 @@
#define ADXL345_ACT_Y_EN BIT(5)
#define ADXL345_ACT_X_EN BIT(6)
+#define ADXL345_COUPLING_DC 0
+#define ADXL345_COUPLING_AC 1
+
/* single/double tap */
enum adxl345_tap_type {
ADXL345_SINGLE_TAP,
@@ -79,16 +84,29 @@ static const unsigned int adxl345_tap_time_reg[] = {
enum adxl345_activity_type {
ADXL345_ACTIVITY,
ADXL345_INACTIVITY,
+ ADXL345_ACTIVITY_AC,
+ ADXL345_INACTIVITY_AC,
};
static const unsigned int adxl345_act_int_reg[] = {
[ADXL345_ACTIVITY] = ADXL345_INT_ACTIVITY,
[ADXL345_INACTIVITY] = ADXL345_INT_INACTIVITY,
+ [ADXL345_ACTIVITY_AC] = ADXL345_INT_ACTIVITY,
+ [ADXL345_INACTIVITY_AC] = ADXL345_INT_INACTIVITY,
};
static const unsigned int adxl345_act_thresh_reg[] = {
[ADXL345_ACTIVITY] = ADXL345_REG_THRESH_ACT,
[ADXL345_INACTIVITY] = ADXL345_REG_THRESH_INACT,
+ [ADXL345_ACTIVITY_AC] = ADXL345_REG_THRESH_ACT,
+ [ADXL345_INACTIVITY_AC] = ADXL345_REG_THRESH_INACT,
+};
+
+static const unsigned int adxl345_act_acdc_msk[] = {
+ [ADXL345_ACTIVITY] = ADXL345_REG_ACT_ACDC_MSK,
+ [ADXL345_INACTIVITY] = ADXL345_REG_INACT_ACDC_MSK,
+ [ADXL345_ACTIVITY_AC] = ADXL345_REG_ACT_ACDC_MSK,
+ [ADXL345_INACTIVITY_AC] = ADXL345_REG_INACT_ACDC_MSK,
};
enum adxl345_odr {
@@ -186,6 +204,13 @@ static struct iio_event_spec adxl345_events[] = {
.mask_shared_by_type = BIT(IIO_EV_INFO_ENABLE) |
BIT(IIO_EV_INFO_VALUE),
},
+ {
+ /* activity, ac bit set */
+ .type = IIO_EV_TYPE_MAG_ADAPTIVE,
+ .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,
@@ -239,6 +264,14 @@ static const struct iio_event_spec adxl345_fake_chan_events[] = {
.mask_shared_by_type = BIT(IIO_EV_INFO_VALUE) |
BIT(IIO_EV_INFO_PERIOD),
},
+ {
+ /* inactivity, AC bit set */
+ .type = IIO_EV_TYPE_MAG_ADAPTIVE,
+ .dir = IIO_EV_DIR_FALLING,
+ .mask_separate = BIT(IIO_EV_INFO_ENABLE),
+ .mask_shared_by_type = BIT(IIO_EV_INFO_VALUE) |
+ BIT(IIO_EV_INFO_PERIOD),
+ },
};
static const struct iio_chan_spec adxl345_channels[] = {
@@ -346,12 +379,114 @@ static int adxl345_set_inact_time(struct adxl345_state *st, u32 val_s)
return regmap_write(st->regmap, ADXL345_REG_TIME_INACT, val);
}
+/**
+ * adxl345_is_act_inact_ac() - Verify if AC or DC coupling is currently enabled.
+ *
+ * @st: The device data.
+ * @type: The activity or inactivity type.
+ *
+ * Given a type of activity / inactivity combined with either AC coupling set or
+ * default to DC, this function verifies if the combination is currently
+ * configured, hence enabled or not.
+ *
+ * Return: true if configured coupling matches the provided type, else a negative
+ * error value.
+ */
+static int adxl345_is_act_inact_ac(struct adxl345_state *st,
+ enum adxl345_activity_type type)
+{
+ unsigned int regval;
+ bool coupling;
+ int ret;
+
+ ret = regmap_read(st->regmap, ADXL345_REG_ACT_INACT_CTRL, ®val);
+ if (ret)
+ return ret;
+
+ coupling = adxl345_act_acdc_msk[type] & regval;
+
+ switch (type) {
+ case ADXL345_ACTIVITY:
+ case ADXL345_INACTIVITY:
+ return coupling == ADXL345_COUPLING_DC;
+ case ADXL345_ACTIVITY_AC:
+ case ADXL345_INACTIVITY_AC:
+ return coupling == ADXL345_COUPLING_AC;
+ default:
+ return -EINVAL;
+ }
+}
+
+/**
+ * adxl345_set_act_inact_ac() - Configure AC coupling or DC coupling.
+ *
+ * @st: The device data.
+ * @type: Provide a type of activity or inactivity.
+ * @cmd_en: enable or disable AC coupling.
+ *
+ * Enables AC coupling or DC coupling depending on the provided type argument.
+ * Note: Activity and inactivity can be either AC coupled or DC coupled not
+ * both at the same time.
+ *
+ * Return: 0 if successful, else error value.
+ */
+static int adxl345_set_act_inact_ac(struct adxl345_state *st,
+ enum adxl345_activity_type type,
+ bool cmd_en)
+{
+ unsigned int act_inact_ac;
+
+ if (type == ADXL345_ACTIVITY_AC || type == ADXL345_INACTIVITY_AC)
+ act_inact_ac = ADXL345_COUPLING_AC && cmd_en;
+ else
+ act_inact_ac = ADXL345_COUPLING_DC && cmd_en;
+
+ /*
+ * A setting of false selects dc-coupled operation, and a setting of
+ * true enables ac-coupled operation. In dc-coupled operation, the
+ * current acceleration magnitude is compared directly with
+ * ADXL345_REG_THRESH_ACT and ADXL345_REG_THRESH_INACT to determine
+ * whether activity or inactivity is 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 of acceleration are then compared to this
+ * reference value, and if the magnitude of the difference exceeds the
+ * ADXL345_REG_THRESH_ACT value, the device triggers an activity
+ * interrupt.
+ *
+ * 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
+ * ADXL345_REG_THRESH_INACT. If the difference is less than the value in
+ * ADXL345_REG_THRESH_INACT for the time in ADXL345_REG_TIME_INACT, the
+ * device is considered inactive and the inactivity interrupt is
+ * triggered. [quoted from p. 24, ADXL345 datasheet Rev. G]
+ *
+ * In a conclusion, the first acceleration snapshot sample which hit the
+ * threshold in a particular direction is always taken as acceleration
+ * reference value to that direction. Since for the hardware activity
+ * and inactivity depend on the x/y/z axis, so do ac and dc coupling.
+ * Note, this sw driver always enables or disables all three x/y/z axis
+ * for detection via act_axis_ctrl and inact_axis_ctrl, respectively.
+ * Where in dc-coupling samples are compared against the thresholds, in
+ * ac-coupling measurement difference to the first acceleration
+ * reference value are compared against the threshold. So, ac-coupling
+ * allows for a bit more dynamic compensation depending on the initial
+ * sample.
+ */
+ return regmap_assign_bits(st->regmap, ADXL345_REG_ACT_INACT_CTRL,
+ adxl345_act_acdc_msk[type], act_inact_ac);
+}
+
static int adxl345_is_act_inact_en(struct adxl345_state *st,
enum adxl345_activity_type type)
{
unsigned int axis_ctrl;
unsigned int regval;
- bool en;
+ bool int_en, en;
int ret;
ret = regmap_read(st->regmap, ADXL345_REG_ACT_INACT_CTRL, &axis_ctrl);
@@ -361,6 +496,7 @@ static int adxl345_is_act_inact_en(struct adxl345_state *st,
/* Check if axis for activity are enabled */
switch (type) {
case ADXL345_ACTIVITY:
+ case ADXL345_ACTIVITY_AC:
en = FIELD_GET(ADXL345_ACT_X_EN, axis_ctrl) |
FIELD_GET(ADXL345_ACT_Y_EN, axis_ctrl) |
FIELD_GET(ADXL345_ACT_Z_EN, axis_ctrl);
@@ -368,6 +504,7 @@ static int adxl345_is_act_inact_en(struct adxl345_state *st,
return false;
break;
case ADXL345_INACTIVITY:
+ case ADXL345_INACTIVITY_AC:
en = FIELD_GET(ADXL345_INACT_X_EN, axis_ctrl) |
FIELD_GET(ADXL345_INACT_Y_EN, axis_ctrl) |
FIELD_GET(ADXL345_INACT_Z_EN, axis_ctrl);
@@ -383,23 +520,39 @@ static int adxl345_is_act_inact_en(struct adxl345_state *st,
if (ret)
return ret;
- return adxl345_act_int_reg[type] & regval;
+ int_en = adxl345_act_int_reg[type] & regval;
+ if (!int_en)
+ return false;
+
+ /* Check if configured coupling matches provided type */
+ return adxl345_is_act_inact_ac(st, type);
}
static int adxl345_set_act_inact_linkbit(struct adxl345_state *st,
enum adxl345_activity_type type,
bool en)
{
- int act_en, inact_en;
+ int act_en, act_ac_en, inact_en, inact_ac_en;
act_en = adxl345_is_act_inact_en(st, ADXL345_ACTIVITY);
if (act_en < 0)
return act_en;
+ act_ac_en = adxl345_is_act_inact_en(st, ADXL345_ACTIVITY_AC);
+ if (act_ac_en < 0)
+ return act_ac_en;
+
inact_en = adxl345_is_act_inact_en(st, ADXL345_INACTIVITY);
if (inact_en < 0)
return inact_en;
+ inact_ac_en = adxl345_is_act_inact_en(st, ADXL345_INACTIVITY_AC);
+ if (inact_ac_en < 0)
+ return inact_ac_en;
+
+ inact_en = inact_en || inact_ac_en;
+ act_en = act_en || act_ac_en;
+
return regmap_assign_bits(st->regmap, ADXL345_REG_POWER_CTL,
ADXL345_POWER_CTL_INACT_MSK,
en && act_en && inact_en);
@@ -426,7 +579,7 @@ static int adxl345_set_act_inact_en(struct adxl345_state *st,
return 0;
/* When turning on inactivity, check if inact time is valid */
- if (type == ADXL345_INACTIVITY) {
+ if (type == ADXL345_INACTIVITY || type == ADXL345_INACTIVITY_AC) {
ret = regmap_read(st->regmap,
ADXL345_REG_TIME_INACT,
&period);
@@ -436,6 +589,16 @@ static int adxl345_set_act_inact_en(struct adxl345_state *st,
if (!period)
return 0;
}
+ } else {
+ /*
+ * When turning off an activity, ensure that the correct
+ * coupling event is specified. This step helps prevent misuse -
+ * for example, if an AC-coupled activity is active and the
+ * current call attempts to turn off a DC-coupled activity, this
+ * inconsistency should be detected here.
+ */
+ if (adxl345_is_act_inact_ac(st, type) <= 0)
+ return 0;
}
/* Start modifying configuration registers */
@@ -446,10 +609,12 @@ static int adxl345_set_act_inact_en(struct adxl345_state *st,
/* Enable axis according to the command */
switch (type) {
case ADXL345_ACTIVITY:
+ case ADXL345_ACTIVITY_AC:
axis_ctrl = ADXL345_ACT_X_EN | ADXL345_ACT_Y_EN |
ADXL345_ACT_Z_EN;
break;
case ADXL345_INACTIVITY:
+ case ADXL345_INACTIVITY_AC:
axis_ctrl = ADXL345_INACT_X_EN | ADXL345_INACT_Y_EN |
ADXL345_INACT_Z_EN;
break;
@@ -462,6 +627,11 @@ static int adxl345_set_act_inact_en(struct adxl345_state *st,
if (ret)
return ret;
+ /* Update AC/DC-coupling according to the command */
+ ret = adxl345_set_act_inact_ac(st, type, cmd_en);
+ if (ret)
+ return ret;
+
/* Enable the interrupt line, according to the command */
ret = regmap_assign_bits(st->regmap, ADXL345_REG_INT_ENABLE,
adxl345_act_int_reg[type], cmd_en);
@@ -955,6 +1125,10 @@ static int adxl345_read_event_config(struct iio_dev *indio_dev,
return adxl345_read_mag_config(st, dir,
ADXL345_ACTIVITY,
ADXL345_INACTIVITY);
+ case IIO_EV_TYPE_MAG_ADAPTIVE:
+ return adxl345_read_mag_config(st, dir,
+ ADXL345_ACTIVITY_AC,
+ ADXL345_INACTIVITY_AC);
case IIO_EV_TYPE_GESTURE:
switch (dir) {
case IIO_EV_DIR_SINGLETAP:
@@ -991,6 +1165,11 @@ static int adxl345_write_event_config(struct iio_dev *indio_dev,
ADXL345_ACTIVITY,
ADXL345_INACTIVITY,
state);
+ case IIO_EV_TYPE_MAG_ADAPTIVE:
+ return adxl345_write_mag_config(st, dir,
+ ADXL345_ACTIVITY_AC,
+ ADXL345_INACTIVITY_AC,
+ state);
case IIO_EV_TYPE_GESTURE:
switch (dir) {
case IIO_EV_DIR_SINGLETAP:
@@ -1100,6 +1279,11 @@ static int adxl345_read_event_value(struct iio_dev *indio_dev,
ADXL345_ACTIVITY,
ADXL345_INACTIVITY,
val, val2);
+ case IIO_EV_TYPE_MAG_ADAPTIVE:
+ return adxl345_read_mag_value(st, dir, info,
+ ADXL345_ACTIVITY_AC,
+ ADXL345_INACTIVITY_AC,
+ val, val2);
case IIO_EV_TYPE_GESTURE:
switch (info) {
case IIO_EV_INFO_VALUE:
@@ -1158,6 +1342,14 @@ static int adxl345_write_event_value(struct iio_dev *indio_dev,
if (ret)
return ret;
break;
+ case IIO_EV_TYPE_MAG_ADAPTIVE:
+ ret = adxl345_write_mag_value(st, dir, info,
+ ADXL345_ACTIVITY_AC,
+ ADXL345_INACTIVITY_AC,
+ val, val2);
+ if (ret)
+ return ret;
+ break;
case IIO_EV_TYPE_GESTURE:
switch (info) {
case IIO_EV_INFO_VALUE:
@@ -1402,6 +1594,7 @@ static int adxl345_push_event(struct iio_dev *indio_dev, int int_stat,
{
s64 ts = iio_get_time_ns(indio_dev);
struct adxl345_state *st = iio_priv(indio_dev);
+ unsigned int regval;
int samples;
int ret = -ENOENT;
@@ -1426,22 +1619,52 @@ static int adxl345_push_event(struct iio_dev *indio_dev, int int_stat,
}
if (FIELD_GET(ADXL345_INT_ACTIVITY, int_stat)) {
- ret = iio_push_event(indio_dev,
- IIO_MOD_EVENT_CODE(IIO_ACCEL, 0, act_dir,
- IIO_EV_TYPE_MAG,
- IIO_EV_DIR_RISING),
- ts);
+ ret = regmap_read(st->regmap, ADXL345_REG_ACT_INACT_CTRL, ®val);
+ if (ret)
+ return ret;
+
+ if (FIELD_GET(ADXL345_REG_ACT_ACDC_MSK, regval)) {
+ /* AC coupled */
+ ret = iio_push_event(indio_dev,
+ IIO_MOD_EVENT_CODE(IIO_ACCEL, 0, act_dir,
+ IIO_EV_TYPE_MAG_ADAPTIVE,
+ IIO_EV_DIR_RISING),
+ ts);
+
+ } else {
+ /* DC coupled, relying on THRESH */
+ ret = iio_push_event(indio_dev,
+ IIO_MOD_EVENT_CODE(IIO_ACCEL, 0, act_dir,
+ IIO_EV_TYPE_MAG,
+ IIO_EV_DIR_RISING),
+ ts);
+ }
if (ret)
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_AND_Y_AND_Z,
- IIO_EV_TYPE_MAG,
- IIO_EV_DIR_FALLING),
- ts);
+ ret = regmap_read(st->regmap, ADXL345_REG_ACT_INACT_CTRL, ®val);
+ if (ret)
+ return ret;
+
+ if (FIELD_GET(ADXL345_REG_INACT_ACDC_MSK, regval)) {
+ /* AC coupled */
+ ret = iio_push_event(indio_dev,
+ IIO_MOD_EVENT_CODE(IIO_ACCEL, 0,
+ IIO_MOD_X_AND_Y_AND_Z,
+ IIO_EV_TYPE_MAG_ADAPTIVE,
+ IIO_EV_DIR_FALLING),
+ ts);
+ } else {
+ /* DC coupled, relying on THRESH */
+ ret = iio_push_event(indio_dev,
+ IIO_MOD_EVENT_CODE(IIO_ACCEL, 0,
+ IIO_MOD_X_AND_Y_AND_Z,
+ IIO_EV_TYPE_MAG,
+ IIO_EV_DIR_FALLING),
+ ts);
+ }
if (ret)
return ret;
}
--
2.39.5
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH v10 5/7] iio: accel: adxl345: add coupling detection for activity/inactivity
2025-06-22 15:50 ` [PATCH v10 5/7] iio: accel: adxl345: add coupling detection for activity/inactivity Lothar Rubusch
@ 2025-06-23 10:11 ` Andy Shevchenko
0 siblings, 0 replies; 29+ messages in thread
From: Andy Shevchenko @ 2025-06-23 10:11 UTC (permalink / raw)
To: Lothar Rubusch
Cc: lars, Michael.Hennerich, jic23, dlechner, nuno.sa, andy, corbet,
linux-iio, linux-kernel, linux-doc, eraretuya
On Sun, Jun 22, 2025 at 03:50:08PM +0000, Lothar Rubusch wrote:
> Enable AC/DC coupling configuration for activity and inactivity detection
> by setting the AC/DC bit. Extend existing magnitude-based detection with
> adaptive AC-coupled mode.
>
> Use DC-coupled mode to compare acceleration samples directly against
> configured thresholds. Use AC-coupled mode to compare samples against a
> reference taken at the start of activity detection. Implement DC-coupled
> events using MAG, and AC-coupled events using MAG_ADAPTIVE.
>
> Expose configuration of thresholds and periods via separate sysfs handles.
> Note that both coupling modes share the same sensor registers, so activity
> or inactivity detection cannot be configured for both AC and DC
> simultaneously. Apply the most recently configured mode.
>
> Simplify event handling and support adaptive AC-coupling.
...
> static int adxl345_set_act_inact_linkbit(struct adxl345_state *st,
> enum adxl345_activity_type type,
> bool en)
> {
> - int act_en, inact_en;
> + int act_en, act_ac_en, inact_en, inact_ac_en;
Just make it two,
int act_ac_en, inact_ac_en;
int act_en, inact_en;
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v10 6/7] iio: accel: adxl345: extend inactivity time for less than 1s
2025-06-22 15:50 [PATCH v10 0/7] iio: accel: adxl345: add interrupt based sensor events Lothar Rubusch
` (4 preceding siblings ...)
2025-06-22 15:50 ` [PATCH v10 5/7] iio: accel: adxl345: add coupling detection for activity/inactivity Lothar Rubusch
@ 2025-06-22 15:50 ` Lothar Rubusch
2025-06-23 10:17 ` Andy Shevchenko
2025-06-22 15:50 ` [PATCH v10 7/7] docs: iio: add documentation for adxl345 driver Lothar Rubusch
2025-06-23 9:45 ` [PATCH v10 0/7] iio: accel: adxl345: add interrupt based sensor events Andy Shevchenko
7 siblings, 1 reply; 29+ messages in thread
From: Lothar Rubusch @ 2025-06-22 15:50 UTC (permalink / raw)
To: lars, Michael.Hennerich, jic23, dlechner, nuno.sa, andy, corbet
Cc: linux-iio, linux-kernel, linux-doc, eraretuya, l.rubusch
Inactivity and free-fall events are essentially the same type of sensor
events. Therefore, inactivity detection (normally set for periods between 1
and 255 seconds) can be extended for shorter durations to support free-fall
detection.
For periods shorter than 1 second, the driver automatically configures the
threshold and duration using the free-fall register. For periods longer
than 1 second, it uses the inactivity threshold and duration using the
inactivity registers.
When using the free-fall register, the link bit is not set, which means
auto-sleep cannot be enabled if activity detection is also active.
Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
---
drivers/iio/accel/adxl345_core.c | 158 +++++++++++++++++++++----------
1 file changed, 109 insertions(+), 49 deletions(-)
diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c
index c5a3dac5f938..f1f92635bc21 100644
--- a/drivers/iio/accel/adxl345_core.c
+++ b/drivers/iio/accel/adxl345_core.c
@@ -40,12 +40,15 @@
#define ADXL345_REG_INACT_AXIS_MSK GENMASK(2, 0)
#define ADXL345_REG_ACT_ACDC_MSK BIT(7)
#define ADXL345_REG_INACT_ACDC_MSK BIT(3)
+#define ADXL345_REG_NO_ACDC_MSK 0x00
#define ADXL345_POWER_CTL_INACT_MSK (ADXL345_POWER_CTL_AUTO_SLEEP | ADXL345_POWER_CTL_LINK)
#define ADXL345_TAP_Z_EN BIT(0)
#define ADXL345_TAP_Y_EN BIT(1)
#define ADXL345_TAP_X_EN BIT(2)
+#define ADXL345_ACT_INACT_NO_AXIS_EN 0x00
+
#define ADXL345_INACT_Z_EN BIT(0)
#define ADXL345_INACT_Y_EN BIT(1)
#define ADXL345_INACT_X_EN BIT(2)
@@ -86,6 +89,7 @@ enum adxl345_activity_type {
ADXL345_INACTIVITY,
ADXL345_ACTIVITY_AC,
ADXL345_INACTIVITY_AC,
+ ADXL345_INACTIVITY_FF,
};
static const unsigned int adxl345_act_int_reg[] = {
@@ -93,6 +97,7 @@ static const unsigned int adxl345_act_int_reg[] = {
[ADXL345_INACTIVITY] = ADXL345_INT_INACTIVITY,
[ADXL345_ACTIVITY_AC] = ADXL345_INT_ACTIVITY,
[ADXL345_INACTIVITY_AC] = ADXL345_INT_INACTIVITY,
+ [ADXL345_INACTIVITY_FF] = ADXL345_INT_FREE_FALL,
};
static const unsigned int adxl345_act_thresh_reg[] = {
@@ -100,6 +105,7 @@ static const unsigned int adxl345_act_thresh_reg[] = {
[ADXL345_INACTIVITY] = ADXL345_REG_THRESH_INACT,
[ADXL345_ACTIVITY_AC] = ADXL345_REG_THRESH_ACT,
[ADXL345_INACTIVITY_AC] = ADXL345_REG_THRESH_INACT,
+ [ADXL345_INACTIVITY_FF] = ADXL345_REG_THRESH_FF,
};
static const unsigned int adxl345_act_acdc_msk[] = {
@@ -107,6 +113,7 @@ static const unsigned int adxl345_act_acdc_msk[] = {
[ADXL345_INACTIVITY] = ADXL345_REG_INACT_ACDC_MSK,
[ADXL345_ACTIVITY_AC] = ADXL345_REG_ACT_ACDC_MSK,
[ADXL345_INACTIVITY_AC] = ADXL345_REG_INACT_ACDC_MSK,
+ [ADXL345_INACTIVITY_FF] = ADXL345_REG_NO_ACDC_MSK,
};
enum adxl345_odr {
@@ -189,6 +196,9 @@ struct adxl345_state {
u8 watermark;
u8 fifo_mode;
+ u8 inact_threshold;
+ u32 inact_time_ms;
+
u32 tap_duration_us;
u32 tap_latent_us;
u32 tap_window_us;
@@ -333,10 +343,29 @@ static int adxl345_set_measure_en(struct adxl345_state *st, bool en)
/* activity / inactivity */
+static int adxl345_set_inact_threshold(struct adxl345_state *st,
+ unsigned int threshold)
+{
+ int ret;
+
+ st->inact_threshold = min(U8_MAX, threshold);
+
+ ret = regmap_write(st->regmap,
+ adxl345_act_thresh_reg[ADXL345_INACTIVITY],
+ st->inact_threshold);
+ if (ret)
+ return ret;
+
+ return regmap_write(st->regmap,
+ adxl345_act_thresh_reg[ADXL345_INACTIVITY_FF],
+ st->inact_threshold);
+}
+
/**
* adxl345_set_inact_time - Configure inactivity time explicitly or by ODR.
* @st: The sensor state instance.
- * @val_s: A desired time value, between 0 and 255.
+ * @val_int: The inactivity time, integer part.
+ * @val_fract: The inactivity time, fractional part when val_int is 0.
*
* Inactivity time can be configured between 1 and 255 seconds. If a user sets
* val_s to 0, a default inactivity time is calculated automatically (since 0 is
@@ -357,16 +386,18 @@ static int adxl345_set_measure_en(struct adxl345_state *st, bool en)
*
* Return: 0 or error value.
*/
-static int adxl345_set_inact_time(struct adxl345_state *st, u32 val_s)
+static int adxl345_set_inact_time(struct adxl345_state *st, u32 val_int,
+ u32 val_fract)
{
int max_boundary = U8_MAX;
int min_boundary = 10;
- unsigned int val = min(val_s, U8_MAX);
+ unsigned int val;
enum adxl345_odr odr;
unsigned int regval;
int ret;
- if (val == 0) {
+ if (val_int == 0 && val_fract == 0) {
+ /* Generated inactivity time based on ODR */
ret = regmap_read(st->regmap, ADXL345_REG_BW_RATE, ®val);
if (ret)
return ret;
@@ -374,9 +405,31 @@ static int adxl345_set_inact_time(struct adxl345_state *st, u32 val_s)
odr = FIELD_GET(ADXL345_BW_RATE_MSK, regval);
val = clamp(max_boundary - adxl345_odr_tbl[odr][0],
min_boundary, max_boundary);
+ st->inact_time_ms = MILLI * val;
+
+ /* Inactivity time in s */
+ return regmap_write(st->regmap, ADXL345_REG_TIME_INACT, val);
+ } else if (val_int == 0 && val_fract > 0) {
+ /* time < 1s, free-fall */
+
+ /*
+ * Datasheet max. value is 255 * 5000 us = 1.275000 seconds.
+ *
+ * Recommended values between 100ms and 350ms (0x14 to 0x46)
+ */
+ st->inact_time_ms = DIV_ROUND_UP(val_fract, MILLI);
+
+ return regmap_write(st->regmap, ADXL345_REG_TIME_FF,
+ DIV_ROUND_CLOSEST(val_fract, 5));
+ } else if (val_int > 0) {
+ /* Time >= 1s, inactivity */
+ st->inact_time_ms = MILLI * val_int;
+
+ return regmap_write(st->regmap, ADXL345_REG_TIME_INACT, val_int);
}
- return regmap_write(st->regmap, ADXL345_REG_TIME_INACT, val);
+ /* Do not support negative or wrong input. */
+ return -EINVAL;
}
/**
@@ -399,6 +452,9 @@ static int adxl345_is_act_inact_ac(struct adxl345_state *st,
bool coupling;
int ret;
+ if (type == ADXL345_INACTIVITY_FF)
+ return true;
+
ret = regmap_read(st->regmap, ADXL345_REG_ACT_INACT_CTRL, ®val);
if (ret)
return ret;
@@ -511,6 +567,9 @@ static int adxl345_is_act_inact_en(struct adxl345_state *st,
if (!en)
return false;
break;
+ case ADXL345_INACTIVITY_FF:
+ en = true;
+ break;
default:
return -EINVAL;
}
@@ -542,15 +601,20 @@ static int adxl345_set_act_inact_linkbit(struct adxl345_state *st,
if (act_ac_en < 0)
return act_ac_en;
- inact_en = adxl345_is_act_inact_en(st, ADXL345_INACTIVITY);
- if (inact_en < 0)
- return inact_en;
+ if (type == ADXL345_INACTIVITY_FF) {
+ inact_en = false;
+ } else {
+ inact_en = adxl345_is_act_inact_en(st, ADXL345_INACTIVITY);
+ if (inact_en < 0)
+ return inact_en;
- inact_ac_en = adxl345_is_act_inact_en(st, ADXL345_INACTIVITY_AC);
- if (inact_ac_en < 0)
- return inact_ac_en;
+ inact_ac_en = adxl345_is_act_inact_en(st, ADXL345_INACTIVITY_AC);
+ if (inact_ac_en < 0)
+ return inact_ac_en;
+
+ inact_en = inact_en || inact_ac_en;
+ }
- inact_en = inact_en || inact_ac_en;
act_en = act_en || act_ac_en;
return regmap_assign_bits(st->regmap, ADXL345_REG_POWER_CTL,
@@ -569,11 +633,15 @@ static int adxl345_set_act_inact_en(struct adxl345_state *st,
if (cmd_en) {
/* When turning on, check if threshold is valid */
- ret = regmap_read(st->regmap,
- adxl345_act_thresh_reg[type],
- &threshold);
- if (ret)
- return ret;
+ if (type == ADXL345_ACTIVITY || type == ADXL345_ACTIVITY_AC) {
+ ret = regmap_read(st->regmap,
+ adxl345_act_thresh_reg[type],
+ &threshold);
+ if (ret)
+ return ret;
+ } else {
+ threshold = st->inact_threshold;
+ }
if (!threshold) /* Just ignore the command if threshold is 0 */
return 0;
@@ -618,6 +686,9 @@ static int adxl345_set_act_inact_en(struct adxl345_state *st,
axis_ctrl = ADXL345_INACT_X_EN | ADXL345_INACT_Y_EN |
ADXL345_INACT_Z_EN;
break;
+ case ADXL345_INACTIVITY_FF:
+ axis_ctrl = ADXL345_ACT_INACT_NO_AXIS_EN;
+ break;
default:
return -EINVAL;
}
@@ -884,7 +955,7 @@ static int adxl345_set_odr(struct adxl345_state *st, enum adxl345_odr odr)
return ret;
/* update inactivity time by ODR */
- return adxl345_set_inact_time(st, 0);
+ return adxl345_set_inact_time(st, 0, 0);
}
static int adxl345_find_range(struct adxl345_state *st, int val, int val2,
@@ -921,12 +992,6 @@ static int adxl345_set_range(struct adxl345_state *st, enum adxl345_range range)
if (ret)
return ret;
- ret = regmap_read(st->regmap,
- adxl345_act_thresh_reg[ADXL345_INACTIVITY],
- &inact_threshold);
- if (ret)
- return ret;
-
ret = regmap_update_bits(st->regmap, ADXL345_REG_DATA_FORMAT,
ADXL345_DATA_FORMAT_RANGE,
FIELD_PREP(ADXL345_DATA_FORMAT_RANGE, range));
@@ -937,6 +1002,7 @@ static int adxl345_set_range(struct adxl345_state *st, enum adxl345_range range)
/ adxl345_range_factor_tbl[range];
act_threshold = min(U8_MAX, max(1, act_threshold));
+ inact_threshold = st->inact_threshold;
inact_threshold = inact_threshold * adxl345_range_factor_tbl[range_old]
/ adxl345_range_factor_tbl[range];
inact_threshold = min(U8_MAX, max(1, inact_threshold));
@@ -946,8 +1012,7 @@ static int adxl345_set_range(struct adxl345_state *st, enum adxl345_range range)
if (ret)
return ret;
- return regmap_write(st->regmap, adxl345_act_thresh_reg[ADXL345_INACTIVITY],
- inact_threshold);
+ return adxl345_set_inact_threshold(st, inact_threshold);
}
static int adxl345_read_avail(struct iio_dev *indio_dev,
@@ -1192,7 +1257,6 @@ static int adxl345_read_mag_value(struct adxl345_state *st,
int *val, int *val2)
{
unsigned int threshold;
- unsigned int period;
int ret;
switch (info) {
@@ -1208,25 +1272,16 @@ static int adxl345_read_mag_value(struct adxl345_state *st,
*val2 = MICRO;
return IIO_VAL_FRACTIONAL;
case IIO_EV_DIR_FALLING:
- ret = regmap_read(st->regmap,
- adxl345_act_thresh_reg[type_inact],
- &threshold);
- if (ret)
- return ret;
- *val = 62500 * threshold;
+ *val = 62500 * st->inact_threshold;
*val2 = MICRO;
return IIO_VAL_FRACTIONAL;
default:
return -EINVAL;
}
case IIO_EV_INFO_PERIOD:
- ret = regmap_read(st->regmap,
- ADXL345_REG_TIME_INACT,
- &period);
- if (ret)
- return ret;
- *val = period;
- return IIO_VAL_INT;
+ *val = st->inact_time_ms;
+ *val2 = MILLI;
+ return IIO_VAL_FRACTIONAL;
default:
return -EINVAL;
}
@@ -1249,14 +1304,12 @@ static int adxl345_write_mag_value(struct adxl345_state *st,
adxl345_act_thresh_reg[type_act],
val);
case IIO_EV_DIR_FALLING:
- return regmap_write(st->regmap,
- adxl345_act_thresh_reg[type_inact],
- val);
+ return adxl345_set_inact_threshold(st, val);
default:
return -EINVAL;
}
case IIO_EV_INFO_PERIOD:
- return adxl345_set_inact_time(st, val);
+ return adxl345_set_inact_time(st, val, val2);
default:
return -EINVAL;
}
@@ -1669,6 +1722,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_AND_Y_AND_Z,
+ IIO_EV_TYPE_MAG,
+ IIO_EV_DIR_FALLING),
+ ts);
+ if (ret)
+ return ret;
+ }
+
if (FIELD_GET(ADXL345_INT_WATERMARK, int_stat)) {
samples = adxl345_get_samples(st);
if (samples < 0)
@@ -1907,15 +1971,11 @@ int adxl345_core_probe(struct device *dev, struct regmap *regmap,
if (ret)
return ret;
- ret = regmap_write(st->regmap, ADXL345_REG_TIME_INACT, 0x37);
- if (ret)
- return ret;
-
ret = regmap_write(st->regmap, ADXL345_REG_THRESH_ACT, 6);
if (ret)
return ret;
- ret = regmap_write(st->regmap, ADXL345_REG_THRESH_INACT, 4);
+ ret = adxl345_set_inact_threshold(st, 4);
if (ret)
return ret;
--
2.39.5
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH v10 6/7] iio: accel: adxl345: extend inactivity time for less than 1s
2025-06-22 15:50 ` [PATCH v10 6/7] iio: accel: adxl345: extend inactivity time for less than 1s Lothar Rubusch
@ 2025-06-23 10:17 ` Andy Shevchenko
2025-06-23 21:21 ` Lothar Rubusch
0 siblings, 1 reply; 29+ messages in thread
From: Andy Shevchenko @ 2025-06-23 10:17 UTC (permalink / raw)
To: Lothar Rubusch
Cc: lars, Michael.Hennerich, jic23, dlechner, nuno.sa, andy, corbet,
linux-iio, linux-kernel, linux-doc, eraretuya
On Sun, Jun 22, 2025 at 03:50:09PM +0000, Lothar Rubusch wrote:
> Inactivity and free-fall events are essentially the same type of sensor
> events. Therefore, inactivity detection (normally set for periods between 1
> and 255 seconds) can be extended for shorter durations to support free-fall
> detection.
>
> For periods shorter than 1 second, the driver automatically configures the
> threshold and duration using the free-fall register. For periods longer
> than 1 second, it uses the inactivity threshold and duration using the
> inactivity registers.
>
> When using the free-fall register, the link bit is not set, which means
> auto-sleep cannot be enabled if activity detection is also active.
...
> -static int adxl345_set_inact_time(struct adxl345_state *st, u32 val_s)
> +static int adxl345_set_inact_time(struct adxl345_state *st, u32 val_int,
> + u32 val_fract)
> {
> int max_boundary = U8_MAX;
> int min_boundary = 10;
> - unsigned int val = min(val_s, U8_MAX);
> + unsigned int val;
You see, I even suggested splitting this assignment to begin with.
The change will be clearer with that done.
> enum adxl345_odr odr;
> unsigned int regval;
> int ret;
>
> - if (val == 0) {
> + if (val_int == 0 && val_fract == 0) {
> + /* Generated inactivity time based on ODR */
> ret = regmap_read(st->regmap, ADXL345_REG_BW_RATE, ®val);
> if (ret)
> return ret;
> odr = FIELD_GET(ADXL345_BW_RATE_MSK, regval);
> val = clamp(max_boundary - adxl345_odr_tbl[odr][0],
> min_boundary, max_boundary);
> + st->inact_time_ms = MILLI * val;
> +
> + /* Inactivity time in s */
> + return regmap_write(st->regmap, ADXL345_REG_TIME_INACT, val);
> + } else if (val_int == 0 && val_fract > 0) {
val_fract check is not needed here.
> + /* time < 1s, free-fall */
> +
> + /*
> + * Datasheet max. value is 255 * 5000 us = 1.275000 seconds.
> + *
> + * Recommended values between 100ms and 350ms (0x14 to 0x46)
> + */
> + st->inact_time_ms = DIV_ROUND_UP(val_fract, MILLI);
> +
> + return regmap_write(st->regmap, ADXL345_REG_TIME_FF,
> + DIV_ROUND_CLOSEST(val_fract, 5));
> + } else if (val_int > 0) {
if now is redundant here, right?
> + /* Time >= 1s, inactivity */
> + st->inact_time_ms = MILLI * val_int;
> +
> + return regmap_write(st->regmap, ADXL345_REG_TIME_INACT, val_int);
> }
>
> - return regmap_write(st->regmap, ADXL345_REG_TIME_INACT, val);
> + /* Do not support negative or wrong input. */
> + return -EINVAL;
> }
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH v10 6/7] iio: accel: adxl345: extend inactivity time for less than 1s
2025-06-23 10:17 ` Andy Shevchenko
@ 2025-06-23 21:21 ` Lothar Rubusch
2025-06-24 7:37 ` Andy Shevchenko
0 siblings, 1 reply; 29+ messages in thread
From: Lothar Rubusch @ 2025-06-23 21:21 UTC (permalink / raw)
To: Andy Shevchenko
Cc: lars, Michael.Hennerich, jic23, dlechner, nuno.sa, andy, corbet,
linux-iio, linux-kernel, linux-doc, eraretuya
Hi Andy,
This is a tricky one, I'll give some examples why (I think) the code
is needed as is.
On Mon, Jun 23, 2025 at 12:17 PM Andy Shevchenko
<andriy.shevchenko@intel.com> wrote:
>
> On Sun, Jun 22, 2025 at 03:50:09PM +0000, Lothar Rubusch wrote:
> > Inactivity and free-fall events are essentially the same type of sensor
> > events. Therefore, inactivity detection (normally set for periods between 1
> > and 255 seconds) can be extended for shorter durations to support free-fall
> > detection.
> >
> > For periods shorter than 1 second, the driver automatically configures the
> > threshold and duration using the free-fall register. For periods longer
> > than 1 second, it uses the inactivity threshold and duration using the
> > inactivity registers.
> >
> > When using the free-fall register, the link bit is not set, which means
> > auto-sleep cannot be enabled if activity detection is also active.
>
> ...
>
> > -static int adxl345_set_inact_time(struct adxl345_state *st, u32 val_s)
> > +static int adxl345_set_inact_time(struct adxl345_state *st, u32 val_int,
> > + u32 val_fract)
> > {
> > int max_boundary = U8_MAX;
> > int min_boundary = 10;
> > - unsigned int val = min(val_s, U8_MAX);
> > + unsigned int val;
>
> You see, I even suggested splitting this assignment to begin with.
> The change will be clearer with that done.
>
> > enum adxl345_odr odr;
> > unsigned int regval;
> > int ret;
> >
> > - if (val == 0) {
> > + if (val_int == 0 && val_fract == 0) {
The case for 0sec, 0.0 or setting "0" and fract will consequently be
"0". 0 is an invalid input for this period and sensor, so it will
default to an optimized period based on given ODR.
> > + /* Generated inactivity time based on ODR */
> > ret = regmap_read(st->regmap, ADXL345_REG_BW_RATE, ®val);
> > if (ret)
> > return ret;
>
> > odr = FIELD_GET(ADXL345_BW_RATE_MSK, regval);
> > val = clamp(max_boundary - adxl345_odr_tbl[odr][0],
> > min_boundary, max_boundary);
> > + st->inact_time_ms = MILLI * val;
> > +
> > + /* Inactivity time in s */
> > + return regmap_write(st->regmap, ADXL345_REG_TIME_INACT, val);
> > + } else if (val_int == 0 && val_fract > 0) {
>
> val_fract check is not needed here.
>
Case for e.g. 0.123, numbers under 1s. This goes into the free-fall register.
> > + /* time < 1s, free-fall */
> > +
> > + /*
> > + * Datasheet max. value is 255 * 5000 us = 1.275000 seconds.
> > + *
> > + * Recommended values between 100ms and 350ms (0x14 to 0x46)
> > + */
> > + st->inact_time_ms = DIV_ROUND_UP(val_fract, MILLI);
> > +
> > + return regmap_write(st->regmap, ADXL345_REG_TIME_FF,
> > + DIV_ROUND_CLOSEST(val_fract, 5));
> > + } else if (val_int > 0) {
>
> if now is redundant here, right?
>
So, this will be 1s through 255s. Periods above 1sec. This goes into
the inactivity register.
> > + /* Time >= 1s, inactivity */
> > + st->inact_time_ms = MILLI * val_int;
> > +
> > + return regmap_write(st->regmap, ADXL345_REG_TIME_INACT, val_int);
> > }
> >
> > - return regmap_write(st->regmap, ADXL345_REG_TIME_INACT, val);
> > + /* Do not support negative or wrong input. */
> > + return -EINVAL;
> > }
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH v10 6/7] iio: accel: adxl345: extend inactivity time for less than 1s
2025-06-23 21:21 ` Lothar Rubusch
@ 2025-06-24 7:37 ` Andy Shevchenko
2025-06-24 8:18 ` Lothar Rubusch
0 siblings, 1 reply; 29+ messages in thread
From: Andy Shevchenko @ 2025-06-24 7:37 UTC (permalink / raw)
To: Lothar Rubusch
Cc: lars, Michael.Hennerich, jic23, dlechner, nuno.sa, andy, corbet,
linux-iio, linux-kernel, linux-doc, eraretuya
On Mon, Jun 23, 2025 at 11:21:01PM +0200, Lothar Rubusch wrote:
> On Mon, Jun 23, 2025 at 12:17 PM Andy Shevchenko
> <andriy.shevchenko@intel.com> wrote:
> > On Sun, Jun 22, 2025 at 03:50:09PM +0000, Lothar Rubusch wrote:
...
> > > -static int adxl345_set_inact_time(struct adxl345_state *st, u32 val_s)
> > > +static int adxl345_set_inact_time(struct adxl345_state *st, u32 val_int,
> > > + u32 val_fract)
> > > {
> > > int max_boundary = U8_MAX;
> > > int min_boundary = 10;
> > > - unsigned int val = min(val_s, U8_MAX);
> > > + unsigned int val;
> >
> > You see, I even suggested splitting this assignment to begin with.
> > The change will be clearer with that done.
> >
> > > enum adxl345_odr odr;
> > > unsigned int regval;
> > > int ret;
> > >
> > > - if (val == 0) {
> > > + if (val_int == 0 && val_fract == 0) {
>
> The case for 0sec, 0.0 or setting "0" and fract will consequently be
> "0". 0 is an invalid input for this period and sensor, so it will
> default to an optimized period based on given ODR.
>
> > > + /* Generated inactivity time based on ODR */
> > > ret = regmap_read(st->regmap, ADXL345_REG_BW_RATE, ®val);
> > > if (ret)
> > > return ret;
> >
> > > odr = FIELD_GET(ADXL345_BW_RATE_MSK, regval);
> > > val = clamp(max_boundary - adxl345_odr_tbl[odr][0],
> > > min_boundary, max_boundary);
> > > + st->inact_time_ms = MILLI * val;
> > > +
> > > + /* Inactivity time in s */
> > > + return regmap_write(st->regmap, ADXL345_REG_TIME_INACT, val);
> > > + } else if (val_int == 0 && val_fract > 0) {
> >
> > val_fract check is not needed here.
>
> Case for e.g. 0.123, numbers under 1s. This goes into the free-fall register.
0.0 is already checked above, and since the val_fract is unsigned this is check
is redundant.
> > > + /* time < 1s, free-fall */
> > > +
> > > + /*
> > > + * Datasheet max. value is 255 * 5000 us = 1.275000 seconds.
> > > + *
> > > + * Recommended values between 100ms and 350ms (0x14 to 0x46)
> > > + */
> > > + st->inact_time_ms = DIV_ROUND_UP(val_fract, MILLI);
> > > +
> > > + return regmap_write(st->regmap, ADXL345_REG_TIME_FF,
> > > + DIV_ROUND_CLOSEST(val_fract, 5));
> > > + } else if (val_int > 0) {
> >
> > if now is redundant here, right?
>
> So, this will be 1s through 255s. Periods above 1sec. This goes into
> the inactivity register.
See above,
> > > + /* Time >= 1s, inactivity */
> > > + st->inact_time_ms = MILLI * val_int;
> > > +
> > > + return regmap_write(st->regmap, ADXL345_REG_TIME_INACT, val_int);
> > > }
> > >
> > > - return regmap_write(st->regmap, ADXL345_REG_TIME_INACT, val);
> > > + /* Do not support negative or wrong input. */
> > > + return -EINVAL;
> > > }
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH v10 6/7] iio: accel: adxl345: extend inactivity time for less than 1s
2025-06-24 7:37 ` Andy Shevchenko
@ 2025-06-24 8:18 ` Lothar Rubusch
2025-06-24 8:36 ` Andy Shevchenko
0 siblings, 1 reply; 29+ messages in thread
From: Lothar Rubusch @ 2025-06-24 8:18 UTC (permalink / raw)
To: Andy Shevchenko
Cc: lars, Michael.Hennerich, jic23, dlechner, nuno.sa, andy, corbet,
linux-iio, linux-kernel, linux-doc, eraretuya
On Tue, Jun 24, 2025 at 9:38 AM Andy Shevchenko
<andriy.shevchenko@intel.com> wrote:
>
> On Mon, Jun 23, 2025 at 11:21:01PM +0200, Lothar Rubusch wrote:
> > On Mon, Jun 23, 2025 at 12:17 PM Andy Shevchenko
> > <andriy.shevchenko@intel.com> wrote:
> > > On Sun, Jun 22, 2025 at 03:50:09PM +0000, Lothar Rubusch wrote:
>
> ...
>
> > > > -static int adxl345_set_inact_time(struct adxl345_state *st, u32 val_s)
> > > > +static int adxl345_set_inact_time(struct adxl345_state *st, u32 val_int,
> > > > + u32 val_fract)
> > > > {
> > > > int max_boundary = U8_MAX;
> > > > int min_boundary = 10;
> > > > - unsigned int val = min(val_s, U8_MAX);
> > > > + unsigned int val;
> > >
> > > You see, I even suggested splitting this assignment to begin with.
> > > The change will be clearer with that done.
> > >
> > > > enum adxl345_odr odr;
> > > > unsigned int regval;
> > > > int ret;
> > > >
> > > > - if (val == 0) {
> > > > + if (val_int == 0 && val_fract == 0) {
> >
> > The case for 0sec, 0.0 or setting "0" and fract will consequently be
> > "0". 0 is an invalid input for this period and sensor, so it will
> > default to an optimized period based on given ODR.
> >
> > > > + /* Generated inactivity time based on ODR */
> > > > ret = regmap_read(st->regmap, ADXL345_REG_BW_RATE, ®val);
> > > > if (ret)
> > > > return ret;
> > >
> > > > odr = FIELD_GET(ADXL345_BW_RATE_MSK, regval);
> > > > val = clamp(max_boundary - adxl345_odr_tbl[odr][0],
> > > > min_boundary, max_boundary);
> > > > + st->inact_time_ms = MILLI * val;
> > > > +
> > > > + /* Inactivity time in s */
> > > > + return regmap_write(st->regmap, ADXL345_REG_TIME_INACT, val);
> > > > + } else if (val_int == 0 && val_fract > 0) {
> > >
> > > val_fract check is not needed here.
> >
> > Case for e.g. 0.123, numbers under 1s. This goes into the free-fall register.
>
> 0.0 is already checked above, and since the val_fract is unsigned this is check
> is redundant.
>
> > > > + /* time < 1s, free-fall */
> > > > +
> > > > + /*
> > > > + * Datasheet max. value is 255 * 5000 us = 1.275000 seconds.
> > > > + *
> > > > + * Recommended values between 100ms and 350ms (0x14 to 0x46)
> > > > + */
> > > > + st->inact_time_ms = DIV_ROUND_UP(val_fract, MILLI);
> > > > +
> > > > + return regmap_write(st->regmap, ADXL345_REG_TIME_FF,
> > > > + DIV_ROUND_CLOSEST(val_fract, 5));
> > > > + } else if (val_int > 0) {
> > >
> > > if now is redundant here, right?
> >
> > So, this will be 1s through 255s. Periods above 1sec. This goes into
> > the inactivity register.
>
> See above,
>
I agree, that checking for val_fract is actually done as a sub case of
val_int, and only if val_int was 0. So, would the following make it
clearer?
if (val_int == 0) {
if (val_fract == 0) {
// 0 provided, default values
} else {
// >0s, e.g. 0.123s, use free-fall register
} else {
// 1s - 255s, use inactivity register
}
Actually - I did not touch that - I saw some places where I'm already
using nested if/else in the third level. I guess, by the style advice
according to switch/case, this also applies to if/else, right?
If yes, when the according parts go into another round, I might give
it a try to separate as well using helper functions.
Best,
L
> > > > + /* Time >= 1s, inactivity */
> > > > + st->inact_time_ms = MILLI * val_int;
> > > > +
> > > > + return regmap_write(st->regmap, ADXL345_REG_TIME_INACT, val_int);
> > > > }
> > > >
> > > > - return regmap_write(st->regmap, ADXL345_REG_TIME_INACT, val);
> > > > + /* Do not support negative or wrong input. */
> > > > + return -EINVAL;
> > > > }
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH v10 6/7] iio: accel: adxl345: extend inactivity time for less than 1s
2025-06-24 8:18 ` Lothar Rubusch
@ 2025-06-24 8:36 ` Andy Shevchenko
0 siblings, 0 replies; 29+ messages in thread
From: Andy Shevchenko @ 2025-06-24 8:36 UTC (permalink / raw)
To: Lothar Rubusch
Cc: lars, Michael.Hennerich, jic23, dlechner, nuno.sa, andy, corbet,
linux-iio, linux-kernel, linux-doc, eraretuya
On Tue, Jun 24, 2025 at 10:18:35AM +0200, Lothar Rubusch wrote:
> On Tue, Jun 24, 2025 at 9:38 AM Andy Shevchenko
> <andriy.shevchenko@intel.com> wrote:
> > On Mon, Jun 23, 2025 at 11:21:01PM +0200, Lothar Rubusch wrote:
> > > On Mon, Jun 23, 2025 at 12:17 PM Andy Shevchenko
> > > <andriy.shevchenko@intel.com> wrote:
> > > > On Sun, Jun 22, 2025 at 03:50:09PM +0000, Lothar Rubusch wrote:
...
> > > > > - if (val == 0) {
> > > > > + if (val_int == 0 && val_fract == 0) {
> > >
> > > The case for 0sec, 0.0 or setting "0" and fract will consequently be
> > > "0". 0 is an invalid input for this period and sensor, so it will
> > > default to an optimized period based on given ODR.
> > >
> > > > > + /* Generated inactivity time based on ODR */
> > > > > ret = regmap_read(st->regmap, ADXL345_REG_BW_RATE, ®val);
> > > > > if (ret)
> > > > > return ret;
> > > >
> > > > > odr = FIELD_GET(ADXL345_BW_RATE_MSK, regval);
> > > > > val = clamp(max_boundary - adxl345_odr_tbl[odr][0],
> > > > > min_boundary, max_boundary);
> > > > > + st->inact_time_ms = MILLI * val;
> > > > > +
> > > > > + /* Inactivity time in s */
> > > > > + return regmap_write(st->regmap, ADXL345_REG_TIME_INACT, val);
> > > > > + } else if (val_int == 0 && val_fract > 0) {
> > > >
> > > > val_fract check is not needed here.
> > >
> > > Case for e.g. 0.123, numbers under 1s. This goes into the free-fall register.
> >
> > 0.0 is already checked above, and since the val_fract is unsigned this is check
> > is redundant.
> >
> > > > > + /* time < 1s, free-fall */
> > > > > +
> > > > > + /*
> > > > > + * Datasheet max. value is 255 * 5000 us = 1.275000 seconds.
> > > > > + *
> > > > > + * Recommended values between 100ms and 350ms (0x14 to 0x46)
> > > > > + */
> > > > > + st->inact_time_ms = DIV_ROUND_UP(val_fract, MILLI);
> > > > > +
> > > > > + return regmap_write(st->regmap, ADXL345_REG_TIME_FF,
> > > > > + DIV_ROUND_CLOSEST(val_fract, 5));
> > > > > + } else if (val_int > 0) {
> > > >
> > > > if now is redundant here, right?
> > >
> > > So, this will be 1s through 255s. Periods above 1sec. This goes into
> > > the inactivity register.
> >
> > See above,
> >
>
> I agree, that checking for val_fract is actually done as a sub case of
> val_int, and only if val_int was 0. So, would the following make it
> clearer?
>
> if (val_int == 0) {
> if (val_fract == 0) {
> // 0 provided, default values
> } else {
> // >0s, e.g. 0.123s, use free-fall register
> } else {
> // 1s - 255s, use inactivity register
> }
>
> Actually - I did not touch that - I saw some places where I'm already
> using nested if/else in the third level. I guess, by the style advice
> according to switch/case, this also applies to if/else, right?
>
> If yes, when the according parts go into another round, I might give
> it a try to separate as well using helper functions.
You can think through the patches. It might make sense to consider as well this
helper_1()
{
// for default
}
helper_2()
{
// for free-fall
}
helper_3()
{
// for inactive
}
...
if ()
helper_1();
else if ()
helper_2();
else
helper_3();
> > > > > + /* Time >= 1s, inactivity */
> > > > > + st->inact_time_ms = MILLI * val_int;
> > > > > +
> > > > > + return regmap_write(st->regmap, ADXL345_REG_TIME_INACT, val_int);
> > > > > }
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v10 7/7] docs: iio: add documentation for adxl345 driver
2025-06-22 15:50 [PATCH v10 0/7] iio: accel: adxl345: add interrupt based sensor events Lothar Rubusch
` (5 preceding siblings ...)
2025-06-22 15:50 ` [PATCH v10 6/7] iio: accel: adxl345: extend inactivity time for less than 1s Lothar Rubusch
@ 2025-06-22 15:50 ` Lothar Rubusch
2025-06-28 16:17 ` Jonathan Cameron
2025-06-23 9:45 ` [PATCH v10 0/7] iio: accel: adxl345: add interrupt based sensor events Andy Shevchenko
7 siblings, 1 reply; 29+ messages in thread
From: Lothar Rubusch @ 2025-06-22 15:50 UTC (permalink / raw)
To: lars, Michael.Hennerich, jic23, dlechner, nuno.sa, andy, corbet
Cc: linux-iio, linux-kernel, linux-doc, eraretuya, l.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 | 443 ++++++++++++++++++++++++++++++++++
Documentation/iio/index.rst | 1 +
2 files changed, 444 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..c5525267ea12
--- /dev/null
+++ b/Documentation/iio/adxl345.rst
@@ -0,0 +1,443 @@
+.. 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
+====================
+
+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
+-------------
+
+Specific IIO events are triggered by their corresponding interrupts. The sensor
+driver supports either none or a single active interrupt (INT) line, selectable
+from the two available options: INT1 or INT2. The active INT line should be
+specified in the device tree. If no INT line is configured, the sensor defaults
+to FIFO bypass mode, where event detection is disabled and only X, Y, and Z axis
+measurements are available.
+
+The table below lists the ADXL345-related device files located in the
+device-specific path: ``/sys/bus/iio/devices/iio:deviceX/events``.
+Note that activity and inactivity detection are DC-coupled by default;
+therefore, only the AC-coupled activity and inactivity events are explicitly
+listed.
+
++---------------------------------------------+---------------------------------------------+
+| 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_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_period | Inactivity time in seconds |
++---------------------------------------------+---------------------------------------------+
+| in_accel_mag_falling_value | Inactivity threshold value in 62.5/LSB |
++---------------------------------------------+---------------------------------------------+
+| in_accel_mag_adaptive_rising_en | Enable AC coupled activity on X axis |
++---------------------------------------------+---------------------------------------------+
+| in_accel_mag_adaptive_falling_period | AC coupled inactivity time in seconds |
++---------------------------------------------+---------------------------------------------+
+| in_accel_mag_adaptive_falling_value | AC coupled inactivity threshold in 62.5/LSB |
++---------------------------------------------+---------------------------------------------+
+| in_accel_mag_adaptive_rising_value | AC coupled activity threshold in 62.5/LSB |
++---------------------------------------------+---------------------------------------------+
+| in_accel_mag_rising_en | Enable activity detection on X axis |
++---------------------------------------------+---------------------------------------------+
+| in_accel_mag_rising_value | Activity threshold value in 62.5/LSB |
++---------------------------------------------+---------------------------------------------+
+| in_accel_x_gesture_singletap_en | Enable single tap detection on X axis |
++---------------------------------------------+---------------------------------------------+
+| in_accel_x&y&z_mag_falling_en | Enable inactivity detection on all axis |
++---------------------------------------------+---------------------------------------------+
+| in_accel_x&y&z_mag_adaptive_falling_en | Enable AC coupled inactivity on all 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 |
++---------------------------------------------+---------------------------------------------+
+
+Please refer to the sensor's datasheet for a detailed description of this
+functionality.
+
+Manually setting the **ODR** will cause the driver to estimate default values
+for inactivity detection timing, where higher ODR values correspond to longer
+default wait times, and lower ODR values to shorter ones. If these defaults do
+not meet your application’s needs, you can explicitly configure the inactivity
+wait time. Setting this value to 0 will revert to the default behavior.
+
+When changing the **g range** configuration, the driver attempts to estimate
+appropriate activity and inactivity thresholds by scaling the default values
+based on the ratio of the previous range to the new one. The resulting threshold
+will never be zero and will always fall between 1 and 255, corresponding to up
+to 62.5 g/LSB as specified in the datasheet. However, you can override these
+estimated thresholds by setting explicit values.
+
+When **activity** and **inactivity** events are enabled, the driver
+automatically manages hysteresis behavior by setting the **link** and
+**auto-sleep** bits. The link bit connects the activity and inactivity
+functions, so that one follows the other. The auto-sleep function puts the
+sensor into sleep mode when inactivity is detected, reducing power consumption
+to the sub-12.5 Hz rate.
+
+The inactivity time is configurable between 1 and 255 seconds. In addition to
+inactivity detection, the sensor also supports free-fall detection, which, from
+the IIO perspective, is treated as a fall in magnitude across all axes. In
+sensor terms, free-fall is defined using an inactivity period ranging from 0.000
+to 1.000 seconds.
+
+The driver behaves as follows:
+* If the configured inactivity period is 1 second or more, the driver uses the
+ sensor's inactivity register. This allows the event to be linked with
+ activity detection, use auto-sleep, and be either AC- or DC-coupled.
+
+* If the inactivity period is less than 1 second, the event is treated as plain
+ inactivity or free-fall detection. In this case, auto-sleep and coupling
+ (AC/DC) are not applied.
+
+* If an inactivity time of 0 seconds is configured, the driver selects a
+ heuristically determined default period (greater than 1 second) to optimize
+ power consumption. This also uses the inactivity register.
+
+Note: It is recommended to use the activity, inactivity, or free-fall registers
+when operating with an ODR between 12.5 Hz and 400 Hz. According to the
+datasheet, the recommended free-fall threshold is between 300 mg and 600 mg
+(register values 0x05 to 0x09), and the suggested free-fall time ranges from
+100 ms to 350 ms (register values 0x14 to 0x46).
+
+In DC-coupled mode, the current acceleration magnitude is directly compared to
+the values in the THRESH_ACT and THRESH_INACT registers to determine activity or
+inactivity. In contrast, AC-coupled activity detection uses the acceleration
+value at the start of detection as a reference point, and subsequent samples are
+compared against this reference. While DC-coupling is the default mode-comparing
+live values to fixed thresholds-AC-coupling relies on an internal filter
+relative to the configured threshold.
+
+AC and DC coupling modes are configured separately for activity and inactivity
+detection, but only one mode can be active at a time for each. For example, if
+AC-coupled activity detection is enabled and then DC-coupled mode is set, only
+DC-coupled activity detection will be active. In other words, only the most
+recent configuration is applied.
+
+**Single tap** detection can be configured per the datasheet by setting the
+threshold and duration parameters. When only single tap detection is enabled,
+the single tap interrupt triggers as soon as the acceleration exceeds the
+threshold (marking the start of the duration) and then falls below it, provided
+the duration limit is not exceeded. If both single tap and double tap detections
+are enabled, the single tap interrupt is triggered only after the double tap
+event has been either confirmed or dismissed.
+
+To configure **double tap** detection, you must also set the window and latency
+parameters in microseconds (µs). The latency period begins once the single tap
+signal drops below the threshold and acts as a waiting time during which any
+spikes are ignored for double tap detection. After the latency period ends, the
+detection window starts. If the acceleration rises above the threshold and then
+falls below it again within this window, a double tap event is triggered upon
+the fall below the threshold.
+
+Double tap event detection is thoroughly explained in the datasheet. After a
+single tap event is detected, a double tap event may follow, provided the signal
+meets certain criteria. However, double tap detection can be invalidated for
+three reasons:
+
+* If the **suppress bit** is set, any acceleration spike above the tap
+ threshold during the tap latency period immediately invalidates the double tap
+ detection. In other words, no spikes are allowed during latency when the
+ suppress bit is active.
+
+* The double tap event is invalid if the acceleration is above the threshold at
+ the start of the double tap window.
+
+* Double tap detection is also invalidated if the acceleration duration exceeds
+ the limit set by the duration register.
+
+For double tap detection, the same duration applies as for single tap: the
+acceleration must rise above the threshold and then fall below it within the
+specified duration. Note that the suppress bit is typically enabled when double
+tap detection is active.
+
+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 formula:
+
+.. 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
+
+ ## AC coupled activity, threshold [62.5/LSB]
+ root:/sys/bus/iio/devices/iio:device0> echo 6 > ./events/in_accel_mag_adaptive_rising_value
+
+ ## AC coupled inactivity, threshold, [62.5/LSB]
+ root:/sys/bus/iio/devices/iio:device0> echo 4 > ./events/in_accel_mag_adaptive_falling_value
+
+ ## AC coupled inactivity, time [s]
+ root:/sys/bus/iio/devices/iio:device0> echo 3 > ./events/in_accel_mag_adaptive_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
+
+ ## AC coupled activity, enable
+ root:/sys/bus/iio/devices/iio:device0> echo 1 > ./events/in_accel_mag_adaptive_rising_en
+
+ ## AC coupled inactivity, enable
+ root:/sys/bus/iio/devices/iio:device0> echo 1 > ./events/in_accel_x\&y\&z_mag_adaptive_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: mag, direction: rising
+ Event: time: 1739063415963770218, type: accel(z), channel: 0, evtype: mag, 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: mag, direction: falling
+ Event: time: 1739063436539080713, type: accel(x&y&z), channel: 0, evtype: mag, direction: falling
+ Event: time: 1739063438357970381, type: accel(z), channel: 0, evtype: mag, direction: rising
+ Event: time: 1739063446726161586, type: accel(z), channel: 0, evtype: mag, direction: rising
+ Event: time: 1739063446727892670, type: accel(z), channel: 0, evtype: mag, direction: rising
+ Event: time: 1739063446743019768, type: accel(z), channel: 0, evtype: mag, direction: rising
+ Event: time: 1739063446744650696, type: accel(z), channel: 0, evtype: mag, 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: mag, direction: falling
+ ...
+
+Activity and inactivity belong together and indicate state changes as follows
+
+.. code-block:: bash
+
+ root:# iio_event_monitor adxl345
+ Found IIO device with name adxl345 with device number 0
+ Event: time: 1744648001133946293, type: accel(x), channel: 0, evtype: mag, direction: rising
+ <after inactivity time elapsed>
+ Event: time: 1744648057724775499, type: accel(x&y&z), channel: 0, evtype: mag, 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.
diff --git a/Documentation/iio/index.rst b/Documentation/iio/index.rst
index 2d6afc5a8ed5..c333720c1c9f 100644
--- a/Documentation/iio/index.rst
+++ b/Documentation/iio/index.rst
@@ -32,6 +32,7 @@ Industrial I/O Kernel Drivers
adis16480
adis16550
adxl380
+ adxl345
bno055
ep93xx_adc
opt4060
--
2.39.5
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH v10 7/7] docs: iio: add documentation for adxl345 driver
2025-06-22 15:50 ` [PATCH v10 7/7] docs: iio: add documentation for adxl345 driver Lothar Rubusch
@ 2025-06-28 16:17 ` Jonathan Cameron
0 siblings, 0 replies; 29+ messages in thread
From: Jonathan Cameron @ 2025-06-28 16:17 UTC (permalink / raw)
To: Lothar Rubusch
Cc: lars, Michael.Hennerich, dlechner, nuno.sa, andy, corbet,
linux-iio, linux-kernel, linux-doc, eraretuya
On Sun, 22 Jun 2025 15:50:10 +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>
I'd add one more patch after this to talk about inactivity vs freefall and the
restrictions around that.
Do it separately rather than integrating in here so that it is easy
to spot that potentially controversial bit.
I'm not a huge fan, but I think it is a viable way to support what we have
here. I'd kind of like a nice bit of documentation to cover what you have
in the patch description of patch 8 just so we can remember what is going
on with out checking the git logs.
Otherwise I took a quick look through the rest of the series and didn't
have anything to add.
Thanks,
Jonathan
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v10 0/7] iio: accel: adxl345: add interrupt based sensor events
2025-06-22 15:50 [PATCH v10 0/7] iio: accel: adxl345: add interrupt based sensor events Lothar Rubusch
` (6 preceding siblings ...)
2025-06-22 15:50 ` [PATCH v10 7/7] docs: iio: add documentation for adxl345 driver Lothar Rubusch
@ 2025-06-23 9:45 ` Andy Shevchenko
7 siblings, 0 replies; 29+ messages in thread
From: Andy Shevchenko @ 2025-06-23 9:45 UTC (permalink / raw)
To: Lothar Rubusch
Cc: lars, Michael.Hennerich, jic23, dlechner, nuno.sa, andy, corbet,
linux-iio, linux-kernel, linux-doc, eraretuya
On Sun, Jun 22, 2025 at 03:50:03PM +0000, Lothar Rubusch wrote:
> Add several interrupt based sensor detection events:
> - refactoring and fixes
> - activity/inactivity linked and auto-sleep
> - AC-coupled activity/inactivity
> - Extend inactivity for inactivity under 1s (using free-fall register)
> - documentation
Thanks for a new version, looks much better with new helpers.
I still have a few nit-picks, though.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 29+ messages in thread