2017-09-25 03:15:40

by Harinath Nampally

[permalink] [raw]
Subject: [PATCH 0/3] This patchset refactors event related functions

Harinath Nampally (3):
Following 2 patches are for refactor:
iio: accel: mma8452: Rename time step look up struct to generic
name as the values are same for all the events.
iio: accel: mma8452: Rename read/write event value callbacks to
generic function name.
Following patch adds new feature:
iio: accel: mma8452: Add single pulse/tap event detection feature
for fxls8471.

drivers/iio/accel/mma8452.c | 170 ++++++++++++++++++++++++++++++++++++++++----
1 file changed, 158 insertions(+), 12 deletions(-)

--
2.7.4


2017-09-25 03:15:46

by Harinath Nampally

[permalink] [raw]
Subject: [PATCH 3/3] iio: accel: mma8452: Add single pulse/tap event detection feature for fxls8471.

This patch adds following changes to support tap feature:
- defines pulse event related registers
- enables and handles single pulse interrupt for fxls8471
- handles IIO_EV_DIR_EITHER in read/write callbacks
because event direction for pulse is either rising or
falling.
- configures read/write event value for pulse latency register
using IIO_EV_INFO_HYSTERESIS.
- add multiple events like pulse and tranient event spec
as elements of event_spec array named 'mma8452_accel_events'

Except mma8653 chip all other chips like mma845x and
fxls8471 have single tap detection feature.
Tested thoroughly using iio_event_monitor application on
imx6ul-evk board.

Signed-off-by: Harinath Nampally <[email protected]>
---
drivers/iio/accel/mma8452.c | 156 ++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 151 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 43c3a6b..36f1b56 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -72,6 +72,19 @@
#define MMA8452_TRANSIENT_THS_MASK GENMASK(6, 0)
#define MMA8452_TRANSIENT_COUNT 0x20
#define MMA8452_TRANSIENT_CHAN_SHIFT 1
+#define MMA8452_PULSE_CFG 0x21
+#define MMA8452_PULSE_CFG_CHAN(chan) BIT(chan * 2)
+#define MMA8452_PULSE_CFG_ELE BIT(6)
+#define MMA8452_PULSE_SRC 0x22
+#define MMA8452_PULSE_SRC_XPULSE BIT(4)
+#define MMA8452_PULSE_SRC_YPULSE BIT(5)
+#define MMA8452_PULSE_SRC_ZPULSE BIT(6)
+#define MMA8452_PULSE_THS 0x23
+#define MMA8452_PULSE_THS_MASK GENMASK(6, 0)
+#define MMA8452_PULSE_COUNT 0x26
+#define MMA8452_PULSE_CHAN_SHIFT 2
+#define MMA8452_PULSE_LTCY 0x27
+
#define MMA8452_CTRL_REG1 0x2a
#define MMA8452_CTRL_ACTIVE BIT(0)
#define MMA8452_CTRL_DR_MASK GENMASK(5, 3)
@@ -91,6 +104,7 @@

#define MMA8452_INT_DRDY BIT(0)
#define MMA8452_INT_FF_MT BIT(2)
+#define MMA8452_INT_PULSE BIT(3)
#define MMA8452_INT_TRANS BIT(5)

#define MMA8451_DEVICE_ID 0x1a
@@ -155,6 +169,16 @@ static const struct mma8452_event_regs trans_ev_regs = {
.ev_count = MMA8452_TRANSIENT_COUNT,
};

+static const struct mma8452_event_regs pulse_ev_regs = {
+ .ev_cfg = MMA8452_PULSE_CFG,
+ .ev_cfg_ele = MMA8452_PULSE_CFG_ELE,
+ .ev_cfg_chan_shift = MMA8452_PULSE_CHAN_SHIFT,
+ .ev_src = MMA8452_PULSE_SRC,
+ .ev_ths = MMA8452_PULSE_THS,
+ .ev_ths_mask = MMA8452_PULSE_THS_MASK,
+ .ev_count = MMA8452_PULSE_COUNT,
+};
+
/**
* struct mma_chip_info - chip specific data
* @chip_id: WHO_AM_I register's value
@@ -784,6 +808,14 @@ static int mma8452_get_event_regs(struct mma8452_data *data,
case IIO_EV_DIR_FALLING:
*ev_reg = &ff_mt_ev_regs;
return 0;
+ case IIO_EV_DIR_EITHER:
+ if (!(data->chip_info->all_events
+ & MMA8452_INT_PULSE) ||
+ !(data->chip_info->enabled_events
+ & MMA8452_INT_PULSE))
+ return -EINVAL;
+ *ev_reg = &pulse_ev_regs;
+ return 0;
default:
return -EINVAL;
}
@@ -848,6 +880,25 @@ static int mma8452_read_event_value(struct iio_dev *indio_dev,
return ret;
}

+ case IIO_EV_INFO_HYSTERESIS:
+ if (!(data->chip_info->all_events & MMA8452_INT_PULSE) ||
+ !(data->chip_info->enabled_events & MMA8452_INT_PULSE))
+ return -EINVAL;
+
+ ret = i2c_smbus_read_byte_data(data->client,
+ MMA8452_PULSE_LTCY);
+ if (ret < 0)
+ return ret;
+
+ power_mode = mma8452_get_power_mode(data);
+ if (power_mode < 0)
+ return power_mode;
+
+ us = ret * mma8452_time_step_us[power_mode][
+ mma8452_get_odr_index(data)];
+ *val = us / USEC_PER_SEC;
+ *val2 = us % USEC_PER_SEC;
+
return IIO_VAL_INT_PLUS_MICRO;

default:
@@ -908,6 +959,24 @@ static int mma8452_write_event_value(struct iio_dev *indio_dev,

return mma8452_change_config(data, MMA8452_TRANSIENT_CFG, reg);

+ case IIO_EV_INFO_HYSTERESIS:
+ if (!(data->chip_info->all_events & MMA8452_INT_PULSE) ||
+ !(data->chip_info->enabled_events & MMA8452_INT_PULSE))
+ return -EINVAL;
+
+ ret = mma8452_get_power_mode(data);
+ if (ret < 0)
+ return ret;
+
+ steps = (val * USEC_PER_SEC + val2) /
+ mma8452_time_step_us[ret][
+ mma8452_get_odr_index(data)];
+
+ if (steps < 0 || steps > 0xff)
+ return -EINVAL;
+
+ return mma8452_change_config(data, MMA8452_PULSE_LTCY, steps);
+
default:
return -EINVAL;
}
@@ -937,6 +1006,14 @@ static int mma8452_read_event_config(struct iio_dev *indio_dev,

return !!(ret & BIT(chan->scan_index +
ev_regs->ev_cfg_chan_shift));
+ case IIO_EV_DIR_EITHER:
+ ret = i2c_smbus_read_byte_data(data->client,
+ ev_regs->ev_cfg);
+ if (ret < 0)
+ return ret;
+
+ return !!(ret & BIT(chan->scan_index *
+ ev_regs->ev_cfg_chan_shift));
default:
return -EINVAL;
}
@@ -988,6 +1065,25 @@ static int mma8452_write_event_config(struct iio_dev *indio_dev,
val |= ev_regs->ev_cfg_ele;

return mma8452_change_config(data, ev_regs->ev_cfg, val);
+
+ case IIO_EV_DIR_EITHER:
+ val = i2c_smbus_read_byte_data(data->client, ev_regs->ev_cfg);
+ if (val < 0)
+ return val;
+
+ if (state) {
+ val &= ~BIT(chan->scan_index *
+ ev_regs->ev_cfg_chan_shift);
+ val |= BIT(chan->scan_index *
+ ev_regs->ev_cfg_chan_shift);
+ } else {
+ val &= ~BIT(chan->scan_index *
+ ev_regs->ev_cfg_chan_shift);
+ }
+
+ val |= ev_regs->ev_cfg_ele;
+
+ return mma8452_change_config(data, ev_regs->ev_cfg, val);
default:
return -EINVAL;
}
@@ -1025,6 +1121,38 @@ static void mma8452_transient_interrupt(struct iio_dev *indio_dev)
ts);
}

+static void mma8452_pulse_interrupt(struct iio_dev *indio_dev)
+{
+ struct mma8452_data *data = iio_priv(indio_dev);
+ s64 ts = iio_get_time_ns(indio_dev);
+ int src;
+
+ src = i2c_smbus_read_byte_data(data->client, MMA8452_PULSE_SRC);
+ if (src < 0)
+ return;
+
+ if (src & MMA8452_PULSE_SRC_XPULSE)
+ iio_push_event(indio_dev,
+ IIO_MOD_EVENT_CODE(IIO_ACCEL, 0, IIO_MOD_X,
+ IIO_EV_TYPE_MAG,
+ IIO_EV_DIR_EITHER),
+ ts);
+
+ if (src & MMA8452_PULSE_SRC_YPULSE)
+ iio_push_event(indio_dev,
+ IIO_MOD_EVENT_CODE(IIO_ACCEL, 0, IIO_MOD_Y,
+ IIO_EV_TYPE_MAG,
+ IIO_EV_DIR_EITHER),
+ ts);
+
+ if (src & MMA8452_PULSE_SRC_ZPULSE)
+ iio_push_event(indio_dev,
+ IIO_MOD_EVENT_CODE(IIO_ACCEL, 0, IIO_MOD_Z,
+ IIO_EV_TYPE_MAG,
+ IIO_EV_DIR_EITHER),
+ ts);
+}
+
static irqreturn_t mma8452_interrupt(int irq, void *p)
{
struct iio_dev *indio_dev = p;
@@ -1063,6 +1191,11 @@ static irqreturn_t mma8452_interrupt(int irq, void *p)
ret = IRQ_HANDLED;
}

+ if (src & MMA8452_INT_PULSE) {
+ mma8452_pulse_interrupt(indio_dev);
+ ret = IRQ_HANDLED;
+ }
+
return ret;
}

@@ -1130,8 +1263,10 @@ static const struct iio_event_spec mma8652_freefall_event[] = {
},
};

-static const struct iio_event_spec mma8452_transient_event[] = {
+
+static const struct iio_event_spec mma8452_accel_events[] = {
{
+ //trasient event
.type = IIO_EV_TYPE_MAG,
.dir = IIO_EV_DIR_RISING,
.mask_separate = BIT(IIO_EV_INFO_ENABLE),
@@ -1139,6 +1274,15 @@ static const struct iio_event_spec mma8452_transient_event[] = {
BIT(IIO_EV_INFO_PERIOD) |
BIT(IIO_EV_INFO_HIGH_PASS_FILTER_3DB)
},
+ {
+ //pulse event
+ .type = IIO_EV_TYPE_MAG,
+ .dir = IIO_EV_DIR_EITHER,
+ .mask_separate = BIT(IIO_EV_INFO_ENABLE),
+ .mask_shared_by_type = BIT(IIO_EV_INFO_VALUE) |
+ BIT(IIO_EV_INFO_PERIOD) |
+ BIT(IIO_EV_INFO_HYSTERESIS)
+ },
};

static const struct iio_event_spec mma8452_motion_event[] = {
@@ -1202,8 +1346,8 @@ static struct attribute_group mma8452_event_attribute_group = {
.shift = 16 - (bits), \
.endianness = IIO_BE, \
}, \
- .event_spec = mma8452_transient_event, \
- .num_event_specs = ARRAY_SIZE(mma8452_transient_event), \
+ .event_spec = mma8452_accel_events, \
+ .num_event_specs = ARRAY_SIZE(mma8452_accel_events), \
}

#define MMA8652_CHANNEL(axis, idx, bits) { \
@@ -1368,9 +1512,11 @@ static const struct mma_chip_info mma_chip_info_table[] = {
*/
.all_events = MMA8452_INT_DRDY |
MMA8452_INT_TRANS |
- MMA8452_INT_FF_MT,
+ MMA8452_INT_FF_MT |
+ MMA8452_INT_PULSE,
.enabled_events = MMA8452_INT_TRANS |
- MMA8452_INT_FF_MT,
+ MMA8452_INT_FF_MT |
+ MMA8452_INT_PULSE,
},
};

--
2.7.4

2017-09-25 03:15:44

by Harinath Nampally

[permalink] [raw]
Subject: [PATCH 2/3] iio: accel: mma8452: Rename read/write event value callbacks to generic function name.

'mma8452_read_thresh' and 'mma8452_write_thresh' functions
does more than just read/write threshold values.
They also handle IIO_EV_INFO_HIGH_PASS_FILTER_3DB and
IIO_EV_INFO_PERIOD therefore renaming to generic names.

Improves code readability, no impact on functionality.

Signed-off-by: Harinath Nampally <[email protected]>
---
drivers/iio/accel/mma8452.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 74b6221..43c3a6b 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -792,7 +792,7 @@ static int mma8452_get_event_regs(struct mma8452_data *data,
}
}

-static int mma8452_read_thresh(struct iio_dev *indio_dev,
+static int mma8452_read_event_value(struct iio_dev *indio_dev,
const struct iio_chan_spec *chan,
enum iio_event_type type,
enum iio_event_direction dir,
@@ -855,7 +855,7 @@ static int mma8452_read_thresh(struct iio_dev *indio_dev,
}
}

-static int mma8452_write_thresh(struct iio_dev *indio_dev,
+static int mma8452_write_event_value(struct iio_dev *indio_dev,
const struct iio_chan_spec *chan,
enum iio_event_type type,
enum iio_event_direction dir,
@@ -1391,8 +1391,8 @@ static const struct iio_info mma8452_info = {
.read_raw = &mma8452_read_raw,
.write_raw = &mma8452_write_raw,
.event_attrs = &mma8452_event_attribute_group,
- .read_event_value = &mma8452_read_thresh,
- .write_event_value = &mma8452_write_thresh,
+ .read_event_value = &mma8452_read_event_value,
+ .write_event_value = &mma8452_write_event_value,
.read_event_config = &mma8452_read_event_config,
.write_event_config = &mma8452_write_event_config,
.debugfs_reg_access = &mma8452_reg_access_dbg,
--
2.7.4

2017-09-25 03:16:13

by Harinath Nampally

[permalink] [raw]
Subject: [PATCH 1/3] iio: accel: mma8452: Rename time step look up struct to generic name as the values are same for all the events.

Improves code readability, no impact on functionality.

Signed-off-by: Harinath Nampally <[email protected]>
---
drivers/iio/accel/mma8452.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 3472e7e..74b6221 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -284,7 +284,7 @@ static const int mma8452_samp_freq[8][2] = {
};

/* Datasheet table: step time "Relationship with the ODR" (sample frequency) */
-static const unsigned int mma8452_transient_time_step_us[4][8] = {
+static const unsigned int mma8452_time_step_us[4][8] = {
{ 1250, 2500, 5000, 10000, 20000, 20000, 20000, 20000 }, /* normal */
{ 1250, 2500, 5000, 10000, 20000, 80000, 80000, 80000 }, /* l p l n */
{ 1250, 2500, 2500, 2500, 2500, 2500, 2500, 2500 }, /* high res*/
@@ -826,7 +826,7 @@ static int mma8452_read_thresh(struct iio_dev *indio_dev,
if (power_mode < 0)
return power_mode;

- us = ret * mma8452_transient_time_step_us[power_mode][
+ us = ret * mma8452_time_step_us[power_mode][
mma8452_get_odr_index(data)];
*val = us / USEC_PER_SEC;
*val2 = us % USEC_PER_SEC;
@@ -883,7 +883,7 @@ static int mma8452_write_thresh(struct iio_dev *indio_dev,
return ret;

steps = (val * USEC_PER_SEC + val2) /
- mma8452_transient_time_step_us[ret][
+ mma8452_time_step_us[ret][
mma8452_get_odr_index(data)];

if (steps < 0 || steps > 0xff)
--
2.7.4

2017-09-25 06:23:55

by Martin Kepplinger

[permalink] [raw]
Subject: Re: [PATCH 0/3] This patchset refactors event related functions

Am 25.09.2017 05:15 schrieb Harinath Nampally:
> Harinath Nampally (3):
> Following 2 patches are for refactor:
> iio: accel: mma8452: Rename time step look up struct to generic
> name as the values are same for all the events.
> iio: accel: mma8452: Rename read/write event value callbacks to
> generic function name.
> Following patch adds new feature:
> iio: accel: mma8452: Add single pulse/tap event detection feature
> for fxls8471.
>

I guess it's fine for now, but please don't call your patchset "This
patchset does...".
Just say "Refactor..." or whatever like you do in your patches.

2017-09-25 06:37:23

by Martin Kepplinger

[permalink] [raw]
Subject: Re: [PATCH 3/3] iio: accel: mma8452: Add single pulse/tap event detection feature for fxls8471.

Am 25.09.2017 05:15 schrieb Harinath Nampally:
> This patch adds following changes to support tap feature:
> - defines pulse event related registers
> - enables and handles single pulse interrupt for fxls8471
> - handles IIO_EV_DIR_EITHER in read/write callbacks
> because event direction for pulse is either rising or
> falling.
> - configures read/write event value for pulse latency register
> using IIO_EV_INFO_HYSTERESIS.
> - add multiple events like pulse and tranient event spec
> as elements of event_spec array named 'mma8452_accel_events'
>
> Except mma8653 chip all other chips like mma845x and
> fxls8471 have single tap detection feature.
> Tested thoroughly using iio_event_monitor application on
> imx6ul-evk board.
>

Doesn't fit in your patchset. I'd like to review this (most likely as
soon
as it applies to -next) so either post this seperately or rename your
cover letter.

thanks

martin