Re: [PATCH v1 11/12] iio: accel: adxl345: add activity feature

From: Lothar Rubusch
Date: Tue Feb 04 2025 - 09:02:11 EST

On Sat, Feb 1, 2025 at 6:27 PM Jonathan Cameron <jic23@xxxxxxxxxx> wrote:
>
> On Tue, 28 Jan 2025 12:00:59 +0000
> Lothar Rubusch <l.rubusch@xxxxxxxxx> wrote:
>
> > Add the handling of activity events, also add sysfs entries to
> > configure threshold values to trigger the event. Allow to push the
> > event over to the iio channel.
> >
> > Signed-off-by: Lothar Rubusch <l.rubusch@xxxxxxxxx>
>
> I was going to guess these were rate of change detectors or ones
> at least slightly compensating for g, but superficially
> looks like a straight forward rising mag threshold. Not seeing need
> for new ABI.
>
> We've had those interfaces with accelerometers for a long time.
> Key is to map the pretty names in the datasheet into what is actually
> being detected. That allows for a lot better generalization across manufacturers.
>

I understand. The adxl345 should actually be nothing fancy. I prepare
something more IIO-like event handling for the next version of the
patches. Thank you for the feedback.

> > ---
> > drivers/iio/accel/adxl345_core.c | 158 ++++++++++++++++++++++++++++++-
> > 1 file changed, 154 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c
> > index 62d75d28b6fc..94c3ad818ba5 100644
> > --- a/drivers/iio/accel/adxl345_core.c
> > +++ b/drivers/iio/accel/adxl345_core.c
> > @@ -121,6 +121,8 @@
> >
> > #define ADXL345_REG_TAP_AXIS_MSK GENMASK(2, 0)
> > #define ADXL345_REG_TAP_SUPPRESS_MSK BIT(3)
> > +#define ADXL345_REG_ACT_AXIS_MSK GENMASK(6, 4)
> > +#define ADXL345_REG_ACT_ACDC_MSK BIT(7)
> >
> > enum adxl345_axis {
> > ADXL345_Z_EN = BIT(0),
> > @@ -163,6 +165,10 @@ struct adxl345_state {
> > u8 watermark;
> > u8 fifo_mode;
> >
> > + u32 act_axis_ctrl;
> > + bool act_ac;
> > + u8 act_value;
> > +
> > u32 tap_axis_ctrl;
> > u8 tap_threshold;
> > u32 tap_duration_us;
> > @@ -177,6 +183,11 @@ struct adxl345_state {
> > };
> >
> > static struct iio_event_spec adxl345_events[] = {
> > + {
> > + /* activity */
> > + .type = IIO_EV_TYPE_THRESH,
> > + .dir = IIO_EV_DIR_RISING,
> > + },
> > {
> > /* single tap */
> > .type = IIO_EV_TYPE_GESTURE,
> > @@ -276,6 +287,117 @@ static inline int adxl345_write_interrupts(struct adxl345_state *st)
> > return regmap_write(st->regmap, ADXL345_REG_INT_ENABLE, st->int_map);
> > }
> >
> > +/* act/inact */
> > +
> > +static int adxl345_write_act_axis(struct adxl345_state *st, bool en)
> > +{
> > + int ret;
> > +
> > + /*
> > + * A setting of 0 selects dc-coupled operation, and a setting of 1
> > + * enables ac-coupled operation. In dc-coupled operation, the current
> > + * acceleration magnitude is compared directly with THRESH_ACT and
> > + * 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
> > + * 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
> > + * THRESH_INACT. If the difference is less than the value in
> > + * THRESH_INACT for the time in TIME_INACT, the device is considered
> > + * inactive and the inactivity interrupt is triggered.
> > + */
> > + ret = regmap_update_bits(st->regmap, ADXL345_REG_ACT_INACT_CTRL,
> > + ADXL345_REG_ACT_ACDC_MSK, st->act_ac);
> > + if (ret)
> > + return ret;
> > +
> > + /*
> > + * The ADXL345 allows for individually enabling/disabling axis for
> > + * activity and inactivity detection, respectively. Here both axis are
> > + * kept in sync, i.e. an axis will be generally enabled or disabled for
> > + * both equally, activity and inactivity detection.
> > + */
> > + st->act_axis_ctrl = en
> > + ? st->act_axis_ctrl | ADXL345_REG_ACT_AXIS_MSK
> > + : st->act_axis_ctrl & ~ADXL345_REG_ACT_AXIS_MSK;
> > +
> > + ret = regmap_update_bits(st->regmap, ADXL345_REG_ACT_INACT_CTRL,
> > + ADXL345_REG_ACT_AXIS_MSK,
> > + st->act_axis_ctrl);
> > + if (ret)
> > + return ret;
> > +
> > + return 0;
> > +}
> > +
> > +static int adxl345_set_act_int(struct adxl345_state *st)
> > +{
> > + bool args_valid;
> > + bool axis_en;
> > +
> > + axis_en = FIELD_GET(ADXL345_REG_ACT_AXIS_MSK, st->act_axis_ctrl) > 0;
> > + args_valid = axis_en && st->act_value > 0;
> > + adxl345_intmap_switch_bit(st, args_valid, ADXL345_INT_ACTIVITY);
> > +
> > + return adxl345_write_interrupts(st);
> > +}
> > +
> > +static int _adxl345_is_act_en(struct adxl345_state *st, bool *en)
> > +{
> > + int ret;
> > + unsigned int regval;
> > +
> > + ret = adxl345_read_interrupts(st, &regval);
> > + if (ret)
> > + return ret;
> > +
> > + *en = FIELD_GET(ADXL345_INT_ACTIVITY, regval) > 0;
> > +
> > + return 0;
> > +}
> > +
> > +static int _adxl345_set_act_en(struct adxl345_state *st, bool en)
> > +{
> > + int ret;
> > +
> > + ret = adxl345_write_act_axis(st, en);
> > + if (ret)
> > + return ret;
> > +
> > + return adxl345_set_act_int(st);
> > +}
> > +
> > +static int adxl345_is_act_en(struct adxl345_state *st, bool *en)
> > +{
> > + return _adxl345_is_act_en(st, en);
> > +}
> > +
> > +static int adxl345_set_act_en(struct adxl345_state *st, bool en)
> > +{
> > + return _adxl345_set_act_en(st, en);
> > +}
> > +
> > +static int _adxl345_set_act_value(struct adxl345_state *st, u8 val)
> > +{
> > + st->act_value = val;
> > +
> > + return regmap_write(st->regmap, ADXL345_REG_THRESH_ACT, val);
> > +}
> > +
> > +static int adxl345_set_act_value(struct adxl345_state *st, u8 val)
> > +{
> > + return _adxl345_set_act_value(st, val);
> > +}
>