Added support for event when there is a detection of single step
or double 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 | 72 +++++++++++++++++++++++++++++++++
2 files changed, 74 insertions(+)
diff --git a/drivers/iio/accel/bma400.h b/drivers/iio/accel/bma400.h
index 65bbc80cbb7f..c833119bb42e 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
@@ -54,6 +55,7 @@
#define BMA400_STEP_CNT3_REG 0x17
#define BMA400_STEP_STAT_REG 0x18
#define BMA400_STEP_INT_MSK BIT(0)
+#define BMA400_STEP_STAT_MASK GENMASK(1, 0)
/*
* Read-write configuration registers
diff --git a/drivers/iio/accel/bma400_core.c b/drivers/iio/accel/bma400_core.c
index 305643e99eb5..79321e41df51 100644
--- a/drivers/iio/accel/bma400_core.c
+++ b/drivers/iio/accel/bma400_core.c
@@ -26,6 +26,7 @@
#include <linux/iio/trigger.h>
#include <linux/iio/triggered_buffer.h>
#include <linux/iio/trigger_consumer.h>
+#include <linux/iio/events.h>
#include "bma400.h"
@@ -69,6 +70,7 @@ struct bma400_data {
int scale;
struct iio_trigger *trig;
int steps_enabled;
+ bool step_event_en;
/* Correct time stamp alignment */
struct {
__be16 buff[3];
@@ -166,6 +168,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, \
@@ -208,6 +216,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),
@@ -858,6 +868,59 @@ 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:
+ switch (type) {
+ case IIO_EV_TYPE_CHANGE:
+ return data->steps_enabled;
+ default:
+ return -EINVAL;
+ }
+ default:
+ return -EINVAL;
+ }
+ 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)
+{
+ int ret;
+ struct bma400_data *data = iio_priv(indio_dev);
+
+ switch (chan->type) {
+ case IIO_STEPS:
+ switch (type) {
+ case IIO_EV_TYPE_CHANGE:
+ mutex_lock(&data->mutex);
+ ret = regmap_update_bits(data->regmap,
+ BMA400_INT12_MAP_REG,
+ BMA400_STEP_INT_MSK,
+ FIELD_PREP(BMA400_STEP_INT_MSK,
+ state));
+ mutex_unlock(&data->mutex);
+ if (ret)
+ return ret;
+ data->steps_enabled = state;
+ return 0;
+ default:
+ return -EINVAL;
+ }
+ default:
+ return -EINVAL;
+ }
+ return 0;
+}
+
static int bma400_data_rdy_trigger_set_state(struct iio_trigger *trig,
bool state)
{
@@ -890,6 +953,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 = {
@@ -945,6 +1010,13 @@ static irqreturn_t bma400_interrupt(int irq, void *private)
if (status & BMA400_INT_DRDY_MSK)
iio_trigger_poll_chained(data->trig);
+ if (status & (BMA400_STEP_STAT_MASK << 8)) {
+ 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),
+ iio_get_time_ns(indio_dev));
+ }
out:
return IRQ_HANDLED;
}
--
2.17.1
On Sat, 19 Mar 2022 23:40:23 +0530
Jagath Jog J <[email protected]> wrote:
> Added support for event when there is a detection of single step
> or double step change.
This is new - do we have any way of saying if it was single or
double that was detected?
> INT1 pin is used to interrupt and event
> is pushed to userspace.
My main question in here is the interaction between the event
enable and the channel enable in the previous patch.
If the channel is enabled, the event will read as enabled but
it looks to me like the interrupt will never happen...
So I think you need to also check if the interrupt is enabled.
The other way around is fine as any IIO ABI is allowed to affect
any other so when you enable the interrupt it can also enable
the channel in general.
>
> Signed-off-by: Jagath Jog J <[email protected]>
> ---
> drivers/iio/accel/bma400.h | 2 +
> drivers/iio/accel/bma400_core.c | 72 +++++++++++++++++++++++++++++++++
> 2 files changed, 74 insertions(+)
>
> diff --git a/drivers/iio/accel/bma400.h b/drivers/iio/accel/bma400.h
> index 65bbc80cbb7f..c833119bb42e 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
> @@ -54,6 +55,7 @@
> #define BMA400_STEP_CNT3_REG 0x17
> #define BMA400_STEP_STAT_REG 0x18
> #define BMA400_STEP_INT_MSK BIT(0)
> +#define BMA400_STEP_STAT_MASK GENMASK(1, 0)
>
> /*
> * Read-write configuration registers
> diff --git a/drivers/iio/accel/bma400_core.c b/drivers/iio/accel/bma400_core.c
> index 305643e99eb5..79321e41df51 100644
> --- a/drivers/iio/accel/bma400_core.c
> +++ b/drivers/iio/accel/bma400_core.c
> @@ -26,6 +26,7 @@
> #include <linux/iio/trigger.h>
> #include <linux/iio/triggered_buffer.h>
> #include <linux/iio/trigger_consumer.h>
> +#include <linux/iio/events.h>
>
> #include "bma400.h"
>
> @@ -69,6 +70,7 @@ struct bma400_data {
> int scale;
> struct iio_trigger *trig;
> int steps_enabled;
> + bool step_event_en;
> /* Correct time stamp alignment */
> struct {
> __be16 buff[3];
> @@ -166,6 +168,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, \
> @@ -208,6 +216,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),
> @@ -858,6 +868,59 @@ 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:
> + switch (type) {
> + case IIO_EV_TYPE_CHANGE:
> + return data->steps_enabled;
> + default:
> + return -EINVAL;
> + }
> + default:
> + return -EINVAL;
> + }
> + 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)
> +{
> + int ret;
> + struct bma400_data *data = iio_priv(indio_dev);
> +
> + switch (chan->type) {
> + case IIO_STEPS:
> + switch (type) {
> + case IIO_EV_TYPE_CHANGE:
> + mutex_lock(&data->mutex);
> + ret = regmap_update_bits(data->regmap,
> + BMA400_INT12_MAP_REG,
> + BMA400_STEP_INT_MSK,
> + FIELD_PREP(BMA400_STEP_INT_MSK,
> + state));
> + mutex_unlock(&data->mutex);
> + if (ret)
> + return ret;
> + data->steps_enabled = state;
How does this interact with the use in the previous patch?
> + return 0;
> + default:
> + return -EINVAL;
> + }
> + default:
> + return -EINVAL;
> + }
> + return 0;
> +}
> +
> static int bma400_data_rdy_trigger_set_state(struct iio_trigger *trig,
> bool state)
> {
> @@ -890,6 +953,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 = {
> @@ -945,6 +1010,13 @@ static irqreturn_t bma400_interrupt(int irq, void *private)
> if (status & BMA400_INT_DRDY_MSK)
> iio_trigger_poll_chained(data->trig);
>
> + if (status & (BMA400_STEP_STAT_MASK << 8)) {
FIELD_GET() would be cleaner
> + 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),
> + iio_get_time_ns(indio_dev));
> + }
> out:
> return IRQ_HANDLED;
> }
On Sat, Mar 19, 2022 at 8:10 PM Jagath Jog J <[email protected]> wrote:
>
> Added support for event when there is a detection of single step
> or double step change. INT1 pin is used to interrupt and event
> is pushed to userspace.
...
> #include <linux/iio/trigger.h>
> #include <linux/iio/triggered_buffer.h>
> #include <linux/iio/trigger_consumer.h>
> +#include <linux/iio/events.h>
Keep this block ordered,
...
> + switch (chan->type) {
> + case IIO_STEPS:
> + switch (type) {
> + case IIO_EV_TYPE_CHANGE:
> + return data->steps_enabled;
> + default:
> + return -EINVAL;
> + }
> + default:
> + return -EINVAL;
> + }
> + return 0;
Dead code.
...
> + case IIO_STEPS:
> + switch (type) {
> + case IIO_EV_TYPE_CHANGE:
> + mutex_lock(&data->mutex);
> + ret = regmap_update_bits(data->regmap,
> + BMA400_INT12_MAP_REG,
> + BMA400_STEP_INT_MSK,
> + FIELD_PREP(BMA400_STEP_INT_MSK,
> + state));
> + mutex_unlock(&data->mutex);
> + if (ret)
> + return ret;
> + data->steps_enabled = state;
> + return 0;
> + default:
> + return -EINVAL;
> + }
> + default:
> + return -EINVAL;
> + }
> + return 0;
Ditto.
--
With Best Regards,
Andy Shevchenko
Hello Jonathan,
On Sun, Mar 20, 2022 at 05:37:26PM +0000, Jonathan Cameron wrote:
> On Sat, 19 Mar 2022 23:40:23 +0530
> Jagath Jog J <[email protected]> wrote:
>
> > Added support for event when there is a detection of single step
> > or double step change.
>
> This is new - do we have any way of saying if it was single or
> double that was detected?
Sorry, I could have mentioned this properly. Device will detect single step.
If the sensor misses the detection of single step then it will indicate
as double step for the next step. (Register - INT-STAT1)
I will correct this in the next patch.
>
> > INT1 pin is used to interrupt and event
> > is pushed to userspace.
>
> My main question in here is the interaction between the event
> enable and the channel enable in the previous patch.
Channel enable will start the counter when sensor detects the steps,
But there will be no interrupts for each step detection since INT pin
mapping is not done here.
>
> If the channel is enabled, the event will read as enabled but
> it looks to me like the interrupt will never happen...
>
> So I think you need to also check if the interrupt is enabled.
Sorry my mistake, I will maintain separate member in device
structure for event.
>
> The other way around is fine as any IIO ABI is allowed to affect
> any other so when you enable the interrupt it can also enable
> the channel in general.
Then I will enable channel also while enabling interrupt.
>
> >
> > Signed-off-by: Jagath Jog J <[email protected]>
> > ---
> > drivers/iio/accel/bma400.h | 2 +
> > drivers/iio/accel/bma400_core.c | 72 +++++++++++++++++++++++++++++++++
> > 2 files changed, 74 insertions(+)
> >
> > diff --git a/drivers/iio/accel/bma400.h b/drivers/iio/accel/bma400.h
> > index 65bbc80cbb7f..c833119bb42e 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
> > @@ -54,6 +55,7 @@
> > #define BMA400_STEP_CNT3_REG 0x17
> > #define BMA400_STEP_STAT_REG 0x18
> > #define BMA400_STEP_INT_MSK BIT(0)
> > +#define BMA400_STEP_STAT_MASK GENMASK(1, 0)
> >
> > /*
> > * Read-write configuration registers
> > diff --git a/drivers/iio/accel/bma400_core.c b/drivers/iio/accel/bma400_core.c
> > index 305643e99eb5..79321e41df51 100644
> > --- a/drivers/iio/accel/bma400_core.c
> > +++ b/drivers/iio/accel/bma400_core.c
> > @@ -26,6 +26,7 @@
> > #include <linux/iio/trigger.h>
> > #include <linux/iio/triggered_buffer.h>
> > #include <linux/iio/trigger_consumer.h>
> > +#include <linux/iio/events.h>
> >
> > #include "bma400.h"
> >
> > @@ -69,6 +70,7 @@ struct bma400_data {
> > int scale;
> > struct iio_trigger *trig;
> > int steps_enabled;
> > + bool step_event_en;
> > /* Correct time stamp alignment */
> > struct {
> > __be16 buff[3];
> > @@ -166,6 +168,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, \
> > @@ -208,6 +216,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),
> > @@ -858,6 +868,59 @@ 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:
> > + switch (type) {
> > + case IIO_EV_TYPE_CHANGE:
> > + return data->steps_enabled;
> > + default:
> > + return -EINVAL;
> > + }
> > + default:
> > + return -EINVAL;
> > + }
> > + 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)
> > +{
> > + int ret;
> > + struct bma400_data *data = iio_priv(indio_dev);
> > +
> > + switch (chan->type) {
> > + case IIO_STEPS:
> > + switch (type) {
> > + case IIO_EV_TYPE_CHANGE:
> > + mutex_lock(&data->mutex);
> > + ret = regmap_update_bits(data->regmap,
> > + BMA400_INT12_MAP_REG,
> > + BMA400_STEP_INT_MSK,
> > + FIELD_PREP(BMA400_STEP_INT_MSK,
> > + state));
> > + mutex_unlock(&data->mutex);
> > + if (ret)
> > + return ret;
> > + data->steps_enabled = state;
>
> How does this interact with the use in the previous patch?
>
> > + return 0;
> > + default:
> > + return -EINVAL;
> > + }
> > + default:
> > + return -EINVAL;
> > + }
> > + return 0;
> > +}
> > +
> > static int bma400_data_rdy_trigger_set_state(struct iio_trigger *trig,
> > bool state)
> > {
> > @@ -890,6 +953,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 = {
> > @@ -945,6 +1010,13 @@ static irqreturn_t bma400_interrupt(int irq, void *private)
> > if (status & BMA400_INT_DRDY_MSK)
> > iio_trigger_poll_chained(data->trig);
> >
> > + if (status & (BMA400_STEP_STAT_MASK << 8)) {
>
> FIELD_GET() would be cleaner
>
> > + 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),
> > + iio_get_time_ns(indio_dev));
> > + }
> > out:
> > return IRQ_HANDLED;
> > }
>