1. Patchew
  2. linux
  3. [PATCH v3 00/12] iio: accel: adxl313: add power-save on activity/inactivity
  4. View patch

[PATCH v3 09/12] iio: accel: adxl313: add activity sensing

Lothar Rubusch posted 12 patches 6 months, 4 weeks ago
Only 11 patches received!
There is a newer version of this series
[PATCH v3 09/12] iio: accel: adxl313: add activity sensing
Posted by Lothar Rubusch 6 months, 4 weeks ago 
Add possibilities to set a threshold for activity sensing. Extend the
interrupt handler to process activity interrupts. Provide functions to set
the activity threshold and to enable/disable activity sensing. Further add
a fake channel for having x, y and z axis anded on the iio channel.

This is a preparatory patch. Some of the definitions and functions are
supposed to be extended for inactivity later on.

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

diff --git a/drivers/iio/accel/adxl313_core.c b/drivers/iio/accel/adxl313_core.c
index 80991cd9bd79..74bb7cfe8a55 100644
--- a/drivers/iio/accel/adxl313_core.c
+++ b/drivers/iio/accel/adxl313_core.c
@@ -13,6 +13,7 @@
 #include <linux/overflow.h>
 #include <linux/property.h>
 #include <linux/regmap.h>
+#include <linux/units.h>
 
 #include <linux/iio/buffer.h>
 #include <linux/iio/events.h>
@@ -28,6 +29,21 @@
 
 #define ADXL313_REG_XYZ_BASE			ADXL313_REG_DATA_AXIS(0)
 
+#define ADXL313_ACT_XYZ_EN			GENMASK(6, 4)
+
+/* activity/inactivity */
+enum adxl313_activity_type {
+	ADXL313_ACTIVITY,
+};
+
+static const unsigned int adxl313_act_int_reg[] = {
+	[ADXL313_ACTIVITY] = ADXL313_INT_ACTIVITY,
+};
+
+static const unsigned int adxl313_act_thresh_reg[] = {
+	[ADXL313_ACTIVITY] = ADXL313_REG_THRESH_ACT,
+};
+
 static const struct regmap_range adxl312_readable_reg_range[] = {
 	regmap_reg_range(ADXL313_REG_DEVID0, ADXL313_REG_DEVID0),
 	regmap_reg_range(ADXL313_REG_OFS_AXIS(0), ADXL313_REG_OFS_AXIS(2)),
@@ -230,6 +246,16 @@ static const int adxl313_odr_freqs[][2] = {
 	},								\
 }
 
+static const struct iio_event_spec adxl313_fake_chan_events[] = {
+	{
+		/* activity */
+		.type = IIO_EV_TYPE_MAG,
+		.dir = IIO_EV_DIR_RISING,
+		.mask_separate = BIT(IIO_EV_INFO_ENABLE),
+		.mask_shared_by_type = BIT(IIO_EV_INFO_VALUE),
+	},
+};
+
 enum adxl313_chans {
 	chan_x, chan_y, chan_z,
 };
@@ -238,6 +264,14 @@ static const struct iio_chan_spec adxl313_channels[] = {
 	ADXL313_ACCEL_CHANNEL(0, chan_x, X),
 	ADXL313_ACCEL_CHANNEL(1, chan_y, Y),
 	ADXL313_ACCEL_CHANNEL(2, chan_z, Z),
+	{
+		.type = IIO_ACCEL,
+		.modified = 1,
+		.channel2 = IIO_MOD_X_AND_Y_AND_Z,
+		.scan_index = -1, /* Fake channel for axis AND'ing */
+		.event_spec = adxl313_fake_chan_events,
+		.num_event_specs = ARRAY_SIZE(adxl313_fake_chan_events),
+	},
 };
 
 static const unsigned long adxl313_scan_masks[] = {
@@ -300,6 +334,60 @@ static int adxl313_read_freq_avail(struct iio_dev *indio_dev,
 	}
 }
 
+static int adxl313_is_act_inact_en(struct adxl313_data *data,
+				   enum adxl313_activity_type type)
+{
+	unsigned int axis_ctrl;
+	unsigned int regval;
+	int axis_en, int_en, ret;
+
+	ret = regmap_read(data->regmap, ADXL313_REG_ACT_INACT_CTL, &axis_ctrl);
+	if (ret)
+		return ret;
+
+	/* Check if axis for activity are enabled */
+	if (type != ADXL313_ACTIVITY)
+		return 0;
+
+	axis_en = FIELD_GET(ADXL313_ACT_XYZ_EN, axis_ctrl);
+
+	/* The axis are enabled, now check if specific interrupt is enabled */
+	ret = regmap_read(data->regmap, ADXL313_REG_INT_ENABLE, &regval);
+	if (ret)
+		return ret;
+
+	int_en = adxl313_act_int_reg[type] & regval;
+
+	return axis_en && int_en;
+}
+
+static int adxl313_set_act_inact_en(struct adxl313_data *data,
+				    enum adxl313_activity_type type,
+				    bool cmd_en)
+{
+	unsigned int axis_ctrl;
+	unsigned int threshold;
+	int ret;
+
+	if (type != ADXL313_ACTIVITY)
+		return 0;
+
+	axis_ctrl = ADXL313_ACT_XYZ_EN;
+
+	ret = regmap_assign_bits(data->regmap, ADXL313_REG_ACT_INACT_CTL,
+				 axis_ctrl, cmd_en);
+	if (ret)
+		return ret;
+
+	ret = regmap_read(data->regmap, adxl313_act_thresh_reg[type], &threshold);
+	if (ret)
+		return ret;
+
+	return regmap_assign_bits(data->regmap, ADXL313_REG_INT_ENABLE,
+				  adxl313_act_int_reg[type],
+				  cmd_en && threshold);
+}
+
 static int adxl313_read_raw(struct iio_dev *indio_dev,
 			    struct iio_chan_spec const *chan,
 			    int *val, int *val2, long mask)
@@ -373,6 +461,113 @@ static int adxl313_write_raw(struct iio_dev *indio_dev,
 	}
 }
 
+static int adxl313_read_event_config(struct iio_dev *indio_dev,
+				     const struct iio_chan_spec *chan,
+				     enum iio_event_type type,
+				     enum iio_event_direction dir)
+{
+	struct adxl313_data *data = iio_priv(indio_dev);
+
+	if (type != IIO_EV_TYPE_MAG)
+		return -EINVAL;
+
+	switch (dir) {
+	case IIO_EV_DIR_RISING:
+		return adxl313_is_act_inact_en(data, ADXL313_ACTIVITY);
+	default:
+		return -EINVAL;
+	}
+}
+
+static int adxl313_write_event_config(struct iio_dev *indio_dev,
+				      const struct iio_chan_spec *chan,
+				      enum iio_event_type type,
+				      enum iio_event_direction dir,
+				      bool state)
+{
+	struct adxl313_data *data = iio_priv(indio_dev);
+
+	if (type != IIO_EV_TYPE_MAG)
+		return -EINVAL;
+
+	switch (dir) {
+	case IIO_EV_DIR_RISING:
+		return adxl313_set_act_inact_en(data, ADXL313_ACTIVITY, state);
+	default:
+		return -EINVAL;
+	}
+}
+
+static int adxl313_read_event_value(struct iio_dev *indio_dev,
+				    const struct iio_chan_spec *chan,
+				    enum iio_event_type type,
+				    enum iio_event_direction dir,
+				    enum iio_event_info info,
+				    int *val, int *val2)
+{
+	struct adxl313_data *data = iio_priv(indio_dev);
+	unsigned int act_threshold;
+	int ret;
+
+	/* Measurement stays enabled, reading from regmap cache */
+
+	if (type != IIO_EV_TYPE_MAG)
+		return -EINVAL;
+
+	if (info != IIO_EV_INFO_VALUE)
+		return -EINVAL;
+
+	switch (dir) {
+	case IIO_EV_DIR_RISING:
+		ret = regmap_read(data->regmap,
+				  adxl313_act_thresh_reg[ADXL313_ACTIVITY],
+				  &act_threshold);
+		if (ret)
+			return ret;
+		*val = act_threshold * 15625;
+		*val2 = MICRO;
+		return IIO_VAL_FRACTIONAL;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int adxl313_write_event_value(struct iio_dev *indio_dev,
+				     const struct iio_chan_spec *chan,
+				     enum iio_event_type type,
+				     enum iio_event_direction dir,
+				     enum iio_event_info info,
+				     int val, int val2)
+{
+	struct adxl313_data *data = iio_priv(indio_dev);
+	unsigned int regval;
+	int ret;
+
+	ret = adxl313_set_measure_en(data, false);
+	if (ret)
+		return ret;
+
+	if (type != IIO_EV_TYPE_MAG)
+		return -EINVAL;
+
+	if (info != IIO_EV_INFO_VALUE)
+		return -EINVAL;
+
+	/* Scale factor 15.625 mg/LSB */
+	regval = DIV_ROUND_CLOSEST(MICRO * val + val2, 15625);
+	switch (dir) {
+	case IIO_EV_DIR_RISING:
+		ret = regmap_write(data->regmap,
+				   adxl313_act_thresh_reg[ADXL313_ACTIVITY],
+				   regval);
+		if (ret)
+			return ret;
+		return adxl313_set_measure_en(data, true);
+	default:
+		return -EINVAL;
+	}
+}
+
 static int adxl313_set_watermark(struct iio_dev *indio_dev, unsigned int value)
 {
 	struct adxl313_data *data = iio_priv(indio_dev);
@@ -509,19 +704,32 @@ static int adxl313_fifo_push(struct iio_dev *indio_dev, int samples)
 
 static int adxl313_push_event(struct iio_dev *indio_dev, int int_stat)
 {
+	s64 ts = iio_get_time_ns(indio_dev);
 	struct adxl313_data *data = iio_priv(indio_dev);
 	int samples;
+	int ret = -ENOENT;
+
+	if (FIELD_GET(ADXL313_INT_ACTIVITY, 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_RISING),
+				     ts);
+		if (ret)
+			return ret;
+	}
 
 	if (FIELD_GET(ADXL313_INT_WATERMARK, int_stat)) {
 		samples = adxl313_get_samples(data);
 		if (samples < 0)
 			return samples;
 
-		return adxl313_fifo_push(indio_dev, samples);
+		ret = adxl313_fifo_push(indio_dev, samples);
 	}
 
 	/* Return error if no event data was pushed to the IIO channel. */
-	return -ENOENT;
+	return ret;
 }
 
 static irqreturn_t adxl313_irq_handler(int irq, void *p)
@@ -560,6 +768,10 @@ static int adxl313_reg_access(struct iio_dev *indio_dev, unsigned int reg,
 static const struct iio_info adxl313_info = {
 	.read_raw	= adxl313_read_raw,
 	.write_raw	= adxl313_write_raw,
+	.read_event_config = adxl313_read_event_config,
+	.write_event_config = adxl313_write_event_config,
+	.read_event_value = adxl313_read_event_value,
+	.write_event_value = adxl313_write_event_value,
 	.read_avail	= adxl313_read_freq_avail,
 	.hwfifo_set_watermark = adxl313_set_watermark,
 	.debugfs_reg_access = &adxl313_reg_access,
@@ -669,6 +881,19 @@ int adxl313_core_probe(struct device *dev,
 		if (ret)
 			return ret;
 
+		/*
+		 * Reset or configure the registers with reasonable default
+		 * values. As having 0 in most cases may result in undesirable
+		 * behavior if the interrupts are enabled.
+		 */
+		ret = regmap_write(data->regmap, ADXL313_REG_ACT_INACT_CTL, 0x00);
+		if (ret)
+			return ret;
+
+		ret = regmap_write(data->regmap, ADXL313_REG_THRESH_ACT, 0x52);
+		if (ret)
+			return ret;
+
 		ret = devm_iio_kfifo_buffer_setup(dev, indio_dev,
 						  &adxl313_buffer_ops);
 		if (ret)
-- 
2.39.5
Re: [PATCH v3 09/12] iio: accel: adxl313: add activity sensing
Posted by Jonathan Cameron 6 months, 4 weeks ago 
m adxl313_chans {
>  	chan_x, chan_y, chan_z,
>  };
> @@ -238,6 +264,14 @@ static const struct iio_chan_spec adxl313_channels[] = {
>  	ADXL313_ACCEL_CHANNEL(0, chan_x, X),
>  	ADXL313_ACCEL_CHANNEL(1, chan_y, Y),
>  	ADXL313_ACCEL_CHANNEL(2, chan_z, Z),
> +	{
> +		.type = IIO_ACCEL,
> +		.modified = 1,
> +		.channel2 = IIO_MOD_X_AND_Y_AND_Z,
I'm confused. If this is for activity it would be very unusual for it
to be X&Y&Z. 

"A setting of 1 enables x-, y-, or z-axis participation in detecting
activity or inactivity. A setting of 0 excludes the selected axis from
participation. If all axes are excluded, the function is disabled. For
activity detection, all participating axes are logically OR’ed, causing
the activity function to trigger when any of the participating axes
exceeds the threshold. For inactivity detection, all participating axes
are logically AND’ed, causing the inactivity function to trigger only
if all participating axes are below the threshold for the specified
period of time."

Which matches with what I'd expect.

> +		.scan_index = -1, /* Fake channel for axis AND'ing */
> +		.event_spec = adxl313_fake_chan_events,
> +		.num_event_specs = ARRAY_SIZE(adxl313_fake_chan_events),
> +	},
>  };
>  
>  static const unsigned long adxl313_scan_masks[] = {
> @@ -300,6 +334,60 @@ static int adxl313_read_freq_avail(struct iio_dev *indio_dev,
>  	}
>  }
Re: [PATCH v3 09/12] iio: accel: adxl313: add activity sensing
Posted by Jonathan Cameron 6 months, 4 weeks ago 
On Fri, 23 May 2025 22:35:20 +0000
Lothar Rubusch <l.rubusch@gmail.com> wrote:

> Add possibilities to set a threshold for activity sensing. Extend the
> interrupt handler to process activity interrupts. Provide functions to set
> the activity threshold and to enable/disable activity sensing. Further add
> a fake channel for having x, y and z axis anded on the iio channel.
> 
> This is a preparatory patch. Some of the definitions and functions are
> supposed to be extended for inactivity later on.
> 
> Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
One comment I found confusing.

I see this hardware is similar to our friend the axl345 so some of the outcomes
of final reviews on that series may apply here as well.

> ---
>  drivers/iio/accel/adxl313_core.c | 229 ++++++++++++++++++++++++++++++-
>  1 file changed, 227 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/accel/adxl313_core.c b/drivers/iio/accel/adxl313_core.c
> index 80991cd9bd79..74bb7cfe8a55 100644
> --- a/drivers/iio/accel/adxl313_core.c
> +++ b/drivers/iio/accel/adxl313_core.c

>  static const unsigned long adxl313_scan_masks[] = {
> @@ -300,6 +334,60 @@ static int adxl313_read_freq_avail(struct iio_dev *indio_dev,
>  	}
>  }
>  
> +static int adxl313_is_act_inact_en(struct adxl313_data *data,
> +				   enum adxl313_activity_type type)
> +{
> +	unsigned int axis_ctrl;
> +	unsigned int regval;
> +	int axis_en, int_en, ret;
> +
> +	ret = regmap_read(data->regmap, ADXL313_REG_ACT_INACT_CTL, &axis_ctrl);
> +	if (ret)
> +		return ret;
> +
> +	/* Check if axis for activity are enabled */

If all 3 axis perhaps?  Or If any axis?  I'm not sure what intent is here.


> +	if (type != ADXL313_ACTIVITY)
> +		return 0;
> +
> +	axis_en = FIELD_GET(ADXL313_ACT_XYZ_EN, axis_ctrl);
> +
> +	/* The axis are enabled, now check if specific interrupt is enabled */
> +	ret = regmap_read(data->regmap, ADXL313_REG_INT_ENABLE, &regval);
> +	if (ret)
> +		return ret;
> +
> +	int_en = adxl313_act_int_reg[type] & regval;
> +
> +	return axis_en && int_en;
> +}
Re: [PATCH v3 09/12] iio: accel: adxl313: add activity sensing
Posted by Lothar Rubusch 6 months, 3 weeks ago 
Hi Jonathan,

On Sun, May 25, 2025 at 3:04 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Fri, 23 May 2025 22:35:20 +0000
> Lothar Rubusch <l.rubusch@gmail.com> wrote:
>
> > Add possibilities to set a threshold for activity sensing. Extend the
> > interrupt handler to process activity interrupts. Provide functions to set
> > the activity threshold and to enable/disable activity sensing. Further add
> > a fake channel for having x, y and z axis anded on the iio channel.
> >
> > This is a preparatory patch. Some of the definitions and functions are
> > supposed to be extended for inactivity later on.
> >
> > Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
> One comment I found confusing.
>
> I see this hardware is similar to our friend the axl345 so some of the outcomes
> of final reviews on that series may apply here as well.

Yes. To be honest with you, I already saw several places, where I
probably need to send you some refac for the ADXL345 as well.
Implementing the same type of source a second time, sometimes leads
[me] to different[/better?] solutions and brings different insights.

>
> > ---
> >  drivers/iio/accel/adxl313_core.c | 229 ++++++++++++++++++++++++++++++-
> >  1 file changed, 227 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/iio/accel/adxl313_core.c b/drivers/iio/accel/adxl313_core.c
> > index 80991cd9bd79..74bb7cfe8a55 100644
> > --- a/drivers/iio/accel/adxl313_core.c
> > +++ b/drivers/iio/accel/adxl313_core.c
>
> >  static const unsigned long adxl313_scan_masks[] = {
> > @@ -300,6 +334,60 @@ static int adxl313_read_freq_avail(struct iio_dev *indio_dev,
> >       }
> >  }
> >
> > +static int adxl313_is_act_inact_en(struct adxl313_data *data,
> > +                                enum adxl313_activity_type type)
> > +{
> > +     unsigned int axis_ctrl;
> > +     unsigned int regval;
> > +     int axis_en, int_en, ret;
> > +
> > +     ret = regmap_read(data->regmap, ADXL313_REG_ACT_INACT_CTL, &axis_ctrl);
> > +     if (ret)
> > +             return ret;
> > +
> > +     /* Check if axis for activity are enabled */
>
> If all 3 axis perhaps?  Or If any axis?  I'm not sure what intent is here.

For the ADXL313 I do generally all axis, i.e. x-, y-, z-axis - enabled
and disabled, respectively. I'll modify the comment.

Sry about spamming the ML with my emails about the reset function. I
oversaw your other mail. Patches will be merged.

Best,
L

>
> > +     if (type != ADXL313_ACTIVITY)
> > +             return 0;
> > +
> > +     axis_en = FIELD_GET(ADXL313_ACT_XYZ_EN, axis_ctrl);
> > +
> > +     /* The axis are enabled, now check if specific interrupt is enabled */
> > +     ret = regmap_read(data->regmap, ADXL313_REG_INT_ENABLE, &regval);
> > +     if (ret)
> > +             return ret;
> > +
> > +     int_en = adxl313_act_int_reg[type] & regval;
> > +
> > +     return axis_en && int_en;
> > +}
>
Re: [PATCH v3 09/12] iio: accel: adxl313: add activity sensing
Posted by Jonathan Cameron 6 months, 3 weeks ago 
On Thu, 29 May 2025 18:22:50 +0200
Lothar Rubusch <l.rubusch@gmail.com> wrote:

> Hi Jonathan,
> 
> On Sun, May 25, 2025 at 3:04 PM Jonathan Cameron <jic23@kernel.org> wrote:
> >
> > On Fri, 23 May 2025 22:35:20 +0000
> > Lothar Rubusch <l.rubusch@gmail.com> wrote:
> >  
> > > Add possibilities to set a threshold for activity sensing. Extend the
> > > interrupt handler to process activity interrupts. Provide functions to set
> > > the activity threshold and to enable/disable activity sensing. Further add
> > > a fake channel for having x, y and z axis anded on the iio channel.
> > >
> > > This is a preparatory patch. Some of the definitions and functions are
> > > supposed to be extended for inactivity later on.
> > >
> > > Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>  
> > One comment I found confusing.
> >
> > I see this hardware is similar to our friend the axl345 so some of the outcomes
> > of final reviews on that series may apply here as well.  
> 
> Yes. To be honest with you, I already saw several places, where I
> probably need to send you some refac for the ADXL345 as well.
> Implementing the same type of source a second time, sometimes leads
> [me] to different[/better?] solutions and brings different insights.
> 
> >  
> > > ---
> > >  drivers/iio/accel/adxl313_core.c | 229 ++++++++++++++++++++++++++++++-
> > >  1 file changed, 227 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/iio/accel/adxl313_core.c b/drivers/iio/accel/adxl313_core.c
> > > index 80991cd9bd79..74bb7cfe8a55 100644
> > > --- a/drivers/iio/accel/adxl313_core.c
> > > +++ b/drivers/iio/accel/adxl313_core.c  
> >  
> > >  static const unsigned long adxl313_scan_masks[] = {
> > > @@ -300,6 +334,60 @@ static int adxl313_read_freq_avail(struct iio_dev *indio_dev,
> > >       }
> > >  }
> > >
> > > +static int adxl313_is_act_inact_en(struct adxl313_data *data,
> > > +                                enum adxl313_activity_type type)
> > > +{
> > > +     unsigned int axis_ctrl;
> > > +     unsigned int regval;
> > > +     int axis_en, int_en, ret;
> > > +
> > > +     ret = regmap_read(data->regmap, ADXL313_REG_ACT_INACT_CTL, &axis_ctrl);
> > > +     if (ret)
> > > +             return ret;
> > > +
> > > +     /* Check if axis for activity are enabled */  
> >
> > If all 3 axis perhaps?  Or If any axis?  I'm not sure what intent is here.  
> 
> For the ADXL313 I do generally all axis, i.e. x-, y-, z-axis - enabled
> and disabled, respectively. I'll modify the comment.
> 
> Sry about spamming the ML with my emails about the reset function. I
> oversaw your other mail. Patches will be merged.

Lol. I did the same thing just now. Don't worry about it!

J
> 
> Best,
> L
> 
> >  
> > > +     if (type != ADXL313_ACTIVITY)
> > > +             return 0;
> > > +
> > > +     axis_en = FIELD_GET(ADXL313_ACT_XYZ_EN, axis_ctrl);
> > > +
> > > +     /* The axis are enabled, now check if specific interrupt is enabled */
> > > +     ret = regmap_read(data->regmap, ADXL313_REG_INT_ENABLE, &regval);
> > > +     if (ret)
> > > +             return ret;
> > > +
> > > +     int_en = adxl313_act_int_reg[type] & regval;
> > > +
> > > +     return axis_en && int_en;
> > > +}  
> >

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.