2022-04-22 18:32:22

by Jagath Jog J

[permalink] [raw]
Subject: [PATCH v4 6/9] iio: accel: bma400: Add step change event

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 <[email protected]>
---
drivers/iio/accel/bma400.h | 2 +
drivers/iio/accel/bma400_core.c | 75 +++++++++++++++++++++++++++++++++
2 files changed, 77 insertions(+)

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);
--
2.17.1


2022-05-03 00:51:47

by Jagath Jog J

[permalink] [raw]
Subject: Re: [PATCH v4 6/9] iio: accel: bma400: Add step change event

Hi Jonathan,

On Sun, May 1, 2022 at 9:52 PM Jonathan Cameron <[email protected]> wrote:
>
> On Thu, 21 Apr 2022 02:41:02 +0530
> Jagath Jog J <[email protected]> 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 <[email protected]>
> 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);
>

2022-05-03 00:56:43

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v4 6/9] iio: accel: bma400: Add step change event

On Thu, 21 Apr 2022 02:41:02 +0530
Jagath Jog J <[email protected]> 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 <[email protected]>
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);