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.5On 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
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
>
>
> >
> > > + 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;
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
© 2016 - 2025 Red Hat, Inc.