diff mbox series

[v4,6/9] iio: accel: bma400: Add step change event

Message ID 20220420211105.14654-7-jagathjog1996@gmail.com (mailing list archive)
State Changes Requested
Headers show
Series iio: accel: bma400: Add buffer, step and activity/inactivity | expand

Commit Message

Jagath Jog J April 20, 2022, 9:11 p.m. UTC 
Added support for event when there is a detection of step change.
INT1 pin is used to interrupt and event is pushed to userspace.

Signed-off-by: Jagath Jog J <jagathjog1996@gmail.com>
---
 drivers/iio/accel/bma400.h      |  2 +
 drivers/iio/accel/bma400_core.c | 75 +++++++++++++++++++++++++++++++++
 2 files changed, 77 insertions(+)

Comments

Jonathan Cameron May 1, 2022, 4:31 p.m. UTC | #1 
On Thu, 21 Apr 2022 02:41:02 +0530
Jagath Jog J <jagathjog1996@gmail.com> wrote:

> Added support for event when there is a detection of step change.
> INT1 pin is used to interrupt and event is pushed to userspace.
> 
> Signed-off-by: Jagath Jog J <jagathjog1996@gmail.com>
Hi Jagath,

A query about handling of multiple interrupts...

> ---
>  drivers/iio/accel/bma400.h      |  2 +
>  drivers/iio/accel/bma400_core.c | 75 +++++++++++++++++++++++++++++++++
>  2 files changed, 77 insertions(+)
> 
>   * Read-write configuration registers
> diff --git a/drivers/iio/accel/bma400_core.c b/drivers/iio/accel/bma400_core.c
> index aafb5a40944d..fe101df7b773 100644
> --- a/drivers/iio/accel/bma400_core.c
> +++ b/drivers/iio/accel/bma400_core.c

>  
>  static const struct iio_trigger_ops bma400_trigger_ops = {
> @@ -971,6 +1035,7 @@ static irqreturn_t bma400_interrupt(int irq, void *private)
>  {
>  	struct iio_dev *indio_dev = private;
>  	struct bma400_data *data = iio_priv(indio_dev);
> +	s64 timestamp = iio_get_time_ns(indio_dev);
>  	int ret;
>  
>  	/* Lock to protect the data->status */
> @@ -981,6 +1046,16 @@ static irqreturn_t bma400_interrupt(int irq, void *private)
>  	if (ret)
>  		goto unlock_err;
>  
> +	if (FIELD_GET(BMA400_STEP_STAT_MASK, le16_to_cpu(data->status))) {
> +		iio_push_event(indio_dev,
> +			       IIO_EVENT_CODE(IIO_STEPS, 0, IIO_NO_MOD,
> +					      IIO_EV_DIR_NONE,
> +					      IIO_EV_TYPE_CHANGE, 0, 0, 0),
> +			       timestamp);
> +		mutex_unlock(&data->mutex);

Is it possible for two interrupt sources to be active at the same time?
Given the device is clearing interrupts on read (which is unusual enough to
make me check that on the datasheet) you will loose any other events.

Normal trick is to act on all set bits and if any of them were acted on
return HANDLED.

> +		return IRQ_HANDLED;
> +	}
> +
>  	if (FIELD_GET(BMA400_INT_DRDY_MSK, le16_to_cpu(data->status))) {
>  		mutex_unlock(&data->mutex);
>  		iio_trigger_poll_chained(data->trig);
Jagath Jog J May 1, 2022, 8:27 p.m. UTC | #2 
Hi Jonathan,

On Sun, May 1, 2022 at 9:52 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Thu, 21 Apr 2022 02:41:02 +0530
> Jagath Jog J <jagathjog1996@gmail.com> wrote:
>
> > Added support for event when there is a detection of step change.
> > INT1 pin is used to interrupt and event is pushed to userspace.
> >
> > Signed-off-by: Jagath Jog J <jagathjog1996@gmail.com>
> Hi Jagath,
>
> A query about handling of multiple interrupts...
>
> > ---
> >  drivers/iio/accel/bma400.h      |  2 +
> >  drivers/iio/accel/bma400_core.c | 75 +++++++++++++++++++++++++++++++++
> >  2 files changed, 77 insertions(+)
> >
> >   * Read-write configuration registers
> > diff --git a/drivers/iio/accel/bma400_core.c b/drivers/iio/accel/bma400_core.c
> > index aafb5a40944d..fe101df7b773 100644
> > --- a/drivers/iio/accel/bma400_core.c
> > +++ b/drivers/iio/accel/bma400_core.c
>
> >
> >  static const struct iio_trigger_ops bma400_trigger_ops = {
> > @@ -971,6 +1035,7 @@ static irqreturn_t bma400_interrupt(int irq, void *private)
> >  {
> >       struct iio_dev *indio_dev = private;
> >       struct bma400_data *data = iio_priv(indio_dev);
> > +     s64 timestamp = iio_get_time_ns(indio_dev);
> >       int ret;
> >
> >       /* Lock to protect the data->status */
> > @@ -981,6 +1046,16 @@ static irqreturn_t bma400_interrupt(int irq, void *private)
> >       if (ret)
> >               goto unlock_err;
> >
> > +     if (FIELD_GET(BMA400_STEP_STAT_MASK, le16_to_cpu(data->status))) {
> > +             iio_push_event(indio_dev,
> > +                            IIO_EVENT_CODE(IIO_STEPS, 0, IIO_NO_MOD,
> > +                                           IIO_EV_DIR_NONE,
> > +                                           IIO_EV_TYPE_CHANGE, 0, 0, 0),
> > +                            timestamp);
> > +             mutex_unlock(&data->mutex);
>
> Is it possible for two interrupt sources to be active at the same time?

Yeah, it is possible when multiple interrupts are enabled like data ready,
step and generic interrupts.

> Given the device is clearing interrupts on read (which is unusual enough to
> make me check that on the datasheet) you will loose any other events.
>
> Normal trick is to act on all set bits and if any of them were acted on
> return HANDLED.

Then I will push all the events that occurred and then in the end I will return
HANDLED so that none of the events are missed.
I will change this in the next version.

Thank you,
Jagath


>
> > +             return IRQ_HANDLED;
> > +     }
> > +
> >       if (FIELD_GET(BMA400_INT_DRDY_MSK, le16_to_cpu(data->status))) {
> >               mutex_unlock(&data->mutex);
> >               iio_trigger_poll_chained(data->trig);
>
diff mbox series

Patch

diff --git a/drivers/iio/accel/bma400.h b/drivers/iio/accel/bma400.h
index 32c08f8b0b98..0faa40fdbbf8 100644
--- a/drivers/iio/accel/bma400.h
+++ b/drivers/iio/accel/bma400.h
@@ -39,6 +39,7 @@ 
 #define BMA400_INT_STAT0_REG        0x0e
 #define BMA400_INT_STAT1_REG        0x0f
 #define BMA400_INT_STAT2_REG        0x10
+#define BMA400_INT12_MAP_REG        0x23
 
 /* Temperature register */
 #define BMA400_TEMP_DATA_REG        0x11
@@ -55,6 +56,7 @@ 
 #define BMA400_STEP_STAT_REG        0x18
 #define BMA400_STEP_INT_MSK         BIT(0)
 #define BMA400_STEP_RAW_LEN         0x03
+#define BMA400_STEP_STAT_MASK       GENMASK(9, 8)
 
 /*
  * Read-write configuration registers
diff --git a/drivers/iio/accel/bma400_core.c b/drivers/iio/accel/bma400_core.c
index aafb5a40944d..fe101df7b773 100644
--- a/drivers/iio/accel/bma400_core.c
+++ b/drivers/iio/accel/bma400_core.c
@@ -25,6 +25,7 @@ 
 
 #include <linux/iio/iio.h>
 #include <linux/iio/buffer.h>
+#include <linux/iio/events.h>
 #include <linux/iio/trigger.h>
 #include <linux/iio/trigger_consumer.h>
 #include <linux/iio/triggered_buffer.h>
@@ -71,6 +72,7 @@  struct bma400_data {
 	int scale;
 	struct iio_trigger *trig;
 	int steps_enabled;
+	bool step_event_en;
 	/* Correct time stamp alignment */
 	struct {
 		__le16 buff[3];
@@ -169,6 +171,12 @@  static const struct iio_chan_spec_ext_info bma400_ext_info[] = {
 	{ }
 };
 
+static const struct iio_event_spec bma400_step_detect_event = {
+	.type = IIO_EV_TYPE_CHANGE,
+	.dir = IIO_EV_DIR_NONE,
+	.mask_separate = BIT(IIO_EV_INFO_ENABLE),
+};
+
 #define BMA400_ACC_CHANNEL(_index, _axis) { \
 	.type = IIO_ACCEL, \
 	.modified = 1, \
@@ -211,6 +219,8 @@  static const struct iio_chan_spec bma400_channels[] = {
 		.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) |
 				      BIT(IIO_CHAN_INFO_ENABLE),
 		.scan_index = -1, /* No buffer support */
+		.event_spec = &bma400_step_detect_event,
+		.num_event_specs = 1,
 	},
 	IIO_CHAN_SOFT_TIMESTAMP(4),
 };
@@ -898,6 +908,58 @@  static int bma400_write_raw_get_fmt(struct iio_dev *indio_dev,
 	}
 }
 
+static int bma400_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 bma400_data *data = iio_priv(indio_dev);
+
+	switch (chan->type) {
+	case IIO_STEPS:
+		return data->step_event_en;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int bma400_steps_event_enable(struct bma400_data *data, int state)
+{
+	int ret;
+
+	ret = bma400_enable_steps(data, 1);
+	if (ret)
+		return ret;
+
+	ret = regmap_update_bits(data->regmap, BMA400_INT12_MAP_REG,
+				 BMA400_STEP_INT_MSK,
+				 FIELD_PREP(BMA400_STEP_INT_MSK,
+					    state));
+	if (ret)
+		return ret;
+	data->step_event_en = state;
+	return 0;
+}
+
+static int bma400_write_event_config(struct iio_dev *indio_dev,
+				     const struct iio_chan_spec *chan,
+				     enum iio_event_type type,
+				     enum iio_event_direction dir, int state)
+{
+	struct bma400_data *data = iio_priv(indio_dev);
+	int ret;
+
+	switch (chan->type) {
+	case IIO_STEPS:
+		mutex_lock(&data->mutex);
+		ret = bma400_steps_event_enable(data, state);
+		mutex_unlock(&data->mutex);
+		return ret;
+	default:
+		return -EINVAL;
+	}
+}
+
 static int bma400_data_rdy_trigger_set_state(struct iio_trigger *trig,
 					     bool state)
 {
@@ -926,6 +988,8 @@  static const struct iio_info bma400_info = {
 	.read_avail        = bma400_read_avail,
 	.write_raw         = bma400_write_raw,
 	.write_raw_get_fmt = bma400_write_raw_get_fmt,
+	.read_event_config = bma400_read_event_config,
+	.write_event_config = bma400_write_event_config,
 };
 
 static const struct iio_trigger_ops bma400_trigger_ops = {
@@ -971,6 +1035,7 @@  static irqreturn_t bma400_interrupt(int irq, void *private)
 {
 	struct iio_dev *indio_dev = private;
 	struct bma400_data *data = iio_priv(indio_dev);
+	s64 timestamp = iio_get_time_ns(indio_dev);
 	int ret;
 
 	/* Lock to protect the data->status */
@@ -981,6 +1046,16 @@  static irqreturn_t bma400_interrupt(int irq, void *private)
 	if (ret)
 		goto unlock_err;
 
+	if (FIELD_GET(BMA400_STEP_STAT_MASK, le16_to_cpu(data->status))) {
+		iio_push_event(indio_dev,
+			       IIO_EVENT_CODE(IIO_STEPS, 0, IIO_NO_MOD,
+					      IIO_EV_DIR_NONE,
+					      IIO_EV_TYPE_CHANGE, 0, 0, 0),
+			       timestamp);
+		mutex_unlock(&data->mutex);
+		return IRQ_HANDLED;
+	}
+
 	if (FIELD_GET(BMA400_INT_DRDY_MSK, le16_to_cpu(data->status))) {
 		mutex_unlock(&data->mutex);
 		iio_trigger_poll_chained(data->trig);