2017-09-25 10:07:51

by Harinath Nampally

[permalink] [raw]
Subject: [PATCH 0/3] Refactor event related code

Rename some struct names and function names to
improve code readability.

Harinath Nampally (3):
iio: accel: mma8452: Rename structs holding event configuration
registers to more appropriate names.
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.

drivers/iio/accel/mma8452.c | 180 +++++++++++++++++++++++++++++++++++++++-----
1 file changed, 163 insertions(+), 17 deletions(-)

--
2.7.4


2017-09-25 10:07:58

by Harinath Nampally

[permalink] [raw]
Subject: [PATCH 4/4] 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 10:08:34

by Harinath Nampally

[permalink] [raw]
Subject: [PATCH 2/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 10:07:54

by Harinath Nampally

[permalink] [raw]
Subject: [PATCH 1/3] iio: accel: mma8452: Rename structs holding event configuration registers to more appropriate names.

Improves code readability, no impact on functionality.

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

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 6194169..3472e7e 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -135,7 +135,7 @@ struct mma8452_event_regs {
u8 ev_count;
};

-static const struct mma8452_event_regs ev_regs_accel_falling = {
+static const struct mma8452_event_regs ff_mt_ev_regs = {
.ev_cfg = MMA8452_FF_MT_CFG,
.ev_cfg_ele = MMA8452_FF_MT_CFG_ELE,
.ev_cfg_chan_shift = MMA8452_FF_MT_CHAN_SHIFT,
@@ -145,7 +145,7 @@ static const struct mma8452_event_regs ev_regs_accel_falling = {
.ev_count = MMA8452_FF_MT_COUNT
};

-static const struct mma8452_event_regs ev_regs_accel_rising = {
+static const struct mma8452_event_regs trans_ev_regs = {
.ev_cfg = MMA8452_TRANSIENT_CFG,
.ev_cfg_ele = MMA8452_TRANSIENT_CFG_ELE,
.ev_cfg_chan_shift = MMA8452_TRANSIENT_CHAN_SHIFT,
@@ -777,12 +777,12 @@ static int mma8452_get_event_regs(struct mma8452_data *data,
& MMA8452_INT_TRANS) &&
(data->chip_info->enabled_events
& MMA8452_INT_TRANS))
- *ev_reg = &ev_regs_accel_rising;
+ *ev_reg = &trans_ev_regs;
else
- *ev_reg = &ev_regs_accel_falling;
+ *ev_reg = &ff_mt_ev_regs;
return 0;
case IIO_EV_DIR_FALLING:
- *ev_reg = &ev_regs_accel_falling;
+ *ev_reg = &ff_mt_ev_regs;
return 0;
default:
return -EINVAL;
--
2.7.4

2017-09-25 10:08:00

by Harinath Nampally

[permalink] [raw]
Subject: [PATCH 3/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 10:11:52

by Martin Kepplinger

[permalink] [raw]
Subject: Re: [PATCH 0/3] Refactor event related code

Am 25.09.2017 12:07 schrieb Harinath Nampally:
> Rename some struct names and function names to
> improve code readability.
>
> Harinath Nampally (3):
> iio: accel: mma8452: Rename structs holding event configuration
> registers to more appropriate names.
> 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.
>

First, this is at least v2 of this series. Second, it still seems to
include
a 4th unrelated patch.

It's sometimes not trivial to get email right. Why not test sending it
privately
to yourself first?

2017-09-25 10:12:46

by Harinath Nampally

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

Please disregard this patch set as this includes an unrelated patch
'[PATCH 4/4] iio: accel: mma8452: Add single pulse/tap event detection
feature for fxls8471.'

On Mon, Sep 25, 2017 at 6:07 AM, Harinath Nampally
<[email protected]> wrote:
> 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 10:17:33

by Harinath Nampally

[permalink] [raw]
Subject: Re: [PATCH 0/3] Refactor event related code

> First, this is at least v2 of this series. Second, it still seems to include
> a 4th unrelated patch.
ok I was not sure whether to make it v2 as the change was to only cover letter
and no code changes. But now I know, will send v3 of this series.

> It's sometimes not trivial to get email right. Why not test sending it privately
> to yourself first?
Sure will do, sorry for the convenience.

Thanks,
Harinath

On Mon, Sep 25, 2017 at 6:11 AM, Martin Kepplinger <[email protected]> wrote:
> Am 25.09.2017 12:07 schrieb Harinath Nampally:
>>
>> Rename some struct names and function names to
>> improve code readability.
>>
>> Harinath Nampally (3):
>> iio: accel: mma8452: Rename structs holding event configuration
>> registers to more appropriate names.
>> 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.
>>
>
> First, this is at least v2 of this series. Second, it still seems to include
> a 4th unrelated patch.
>
> It's sometimes not trivial to get email right. Why not test sending it
> privately
> to yourself first?

2017-09-25 10:25:06

by Martin Kepplinger

[permalink] [raw]
Subject: Re: [PATCH 0/3] Refactor event related code

Am 25.09.2017 12:16 schrieb harinath Nampally:
>> First, this is at least v2 of this series. Second, it still seems to
>> include
>> a 4th unrelated patch.
> ok I was not sure whether to make it v2 as the change was to only cover
> letter
> and no code changes. But now I know, will send v3 of this series.
>

as soon as you don't do an exact resend, increment the version number.
When
reviewers can ignore all previous versions, they instantly know. And for
the case
we still want to look at everything, you should include a persistent
revision history
description below the email's --- dashes.

But actually you should have read this and much more in the
Documentation
directory before sending.