| Message ID | 1510197177-9434-1-git-send-email-harinath922@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 2017-11-09 04:12, Harinath Nampally wrote: > This patch adds following related changes: > - 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 > - adds 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 which has fxls8471. > > Signed-off-by: Harinath Nampally <harinath922@gmail.com> > --- What tree is this written against? It doesn't apply to the current -next anyways. > 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) > I think the defintions would deserve to be in a separate patch, but that's debatable. > #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[] = { I would prefer having this renaming in a separate patch too - with a good expanation about why. (Don't be afraid of longer commit messages) > { > + //trasient event typo? > .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), \ that would go in the mentioned separate renaming-patch > } > > #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, > }, > }; > > -- To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> > This patch adds following related changes: > > - 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 > > - adds 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 which has fxls8471. > > > > Signed-off-by: Harinath Nampally <harinath922@gmail.com> > > --- > What tree is this written against? It doesn't apply to the current -next > anyways. Thanks for the review. It is actually against 'testing' branch, I think two of my earlier patches are not yet applied to any branch, that might be reason this patch is not good against current -next or 'togreg'. > I think the defintions would deserve to be in a separate patch, but > that's debatable. Yes, I would argue that definitions are not a logical change. > > .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), \ > that would go in the mentioned separate renaming-patch OK so I will make a patch set; patch 1/2 to just rename 'mma8452_transient_event[]' to 'mma8452_accel_events[]'(without adding pulse event). and everything else would go in 2/2. Does that makes sense? Thanks, Harinath On Fri, Nov 10, 2017 at 5:35 PM, Martin Kepplinger <martink@posteo.de> wrote: > On 2017-11-09 04:12, Harinath Nampally wrote: >> This patch adds following related changes: >> - 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 >> - adds 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 which has fxls8471. >> >> Signed-off-by: Harinath Nampally <harinath922@gmail.com> >> --- > > What tree is this written against? It doesn't apply to the current -next > anyways. > >> 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) >> > I think the defintions would deserve to be in a separate patch, but > that's debatable. > >> #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[] = { > > I would prefer having this renaming in a separate patch too - with a > good expanation about why. (Don't be afraid of longer commit messages) > >> { >> + //trasient event > > typo? > >> .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), \ > > that would go in the mentioned separate renaming-patch > >> } >> >> #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, >> }, >> }; >> >> > -- To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Am 14.11.2017 05:36 schrieb harinath Nampally: >> > This patch adds following related changes: >> > - 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 >> > - adds 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 which has fxls8471. >> > >> > Signed-off-by: Harinath Nampally <harinath922@gmail.com> >> > --- >> What tree is this written against? It doesn't apply to the current >> -next >> anyways. > Thanks for the review. > It is actually against 'testing' branch, I think two of my earlier > patches are not yet applied to > any branch, that might be reason this patch is not good against > current -next or 'togreg'. > >> I think the defintions would deserve to be in a separate patch, but >> that's debatable. > Yes, I would argue that definitions are not a logical change. > I would argue definitions don't break the build and maybe slightly better support features like bisect or revert :) >> > .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), \ >> that would go in the mentioned separate renaming-patch > OK so I will make a patch set; patch 1/2 to just rename > 'mma8452_transient_event[]' > to 'mma8452_accel_events[]'(without adding pulse event). > and everything else would go in 2/2. Does that makes sense? > It does to me. -- To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 8 Nov 2017 22:12:57 -0500 Harinath Nampally <harinath922@gmail.com> wrote: > This patch adds following related changes: > - 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 > - adds 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 which has fxls8471. > > Signed-off-by: Harinath Nampally <harinath922@gmail.com> The use of an either direction magnitude event is misleading. Tap detection requires a timed sequence of events - a rapid increase in acceleration followed by a rapid decrease. I don't think we can fit tap detection into the existing event types (which is annoying but there we are). So.. I'd propose (and I want as much feedback on this as possible) IIO_EV_TYPE_TAP Then abuse iio_event_direction to specify single or double (the concept of direction doesn't really apply to these in that sense so we'd always have them set to none). The alternative would be to full out for an abstract representation of these events (like step detection) and define a channel type IIO_TAP then detect a change in the number of taps (IIO_DIR_NONE), but given we have directional taps we would need the modifier for that. Hence no means of describing whether it is a single or double tap short of burning channel taps. I'm also not sure we want to remove the association with a particular sensor channel. Hence I think I prefer option 1 but would like other's input on this... Jonathan > --- > 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, > }, > }; > -- To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> > This patch adds following related changes: > > - 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 > > - adds 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 which has fxls8471. > > > > Signed-off-by: Harinath Nampally <harinath922@gmail.com> > > The use of an either direction magnitude event is misleading. > Tap detection requires a timed sequence of events - a rapid > increase in acceleration followed by a rapid decrease. Yes I agree. > So.. I'd propose (and I want as much feedback on this as possible) > > IIO_EV_TYPE_TAP > > Then abuse iio_event_direction to specify single or double (the concept > of direction doesn't really apply to these in that sense so we'd always > have them set to none). Sure, I think this would be very useful as there are lot of modern accelerometers with tap event support. Thanks, Harinath On Sun, Nov 19, 2017 at 9:47 AM, Jonathan Cameron <jic23@kernel.org> wrote: > On Wed, 8 Nov 2017 22:12:57 -0500 > Harinath Nampally <harinath922@gmail.com> wrote: > >> This patch adds following related changes: >> - 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 >> - adds 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 which has fxls8471. >> >> Signed-off-by: Harinath Nampally <harinath922@gmail.com> > > The use of an either direction magnitude event is misleading. > Tap detection requires a timed sequence of events - a rapid > increase in acceleration followed by a rapid decrease. > > I don't think we can fit tap detection into the existing > event types (which is annoying but there we are). > > So.. I'd propose (and I want as much feedback on this as possible) > > IIO_EV_TYPE_TAP > > Then abuse iio_event_direction to specify single or double (the concept > of direction doesn't really apply to these in that sense so we'd always > have them set to none). > > The alternative would be to full out for an abstract representation of > these events (like step detection) and define a channel type > IIO_TAP then detect a change in the number of taps (IIO_DIR_NONE), but > given we have directional taps we would need the modifier for that. > Hence no means of describing whether it is a single or double tap > short of burning channel taps. I'm also not sure we want to > remove the association with a particular sensor channel. > > Hence I think I prefer option 1 but would like other's input on this... > > Jonathan > >> --- >> 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, >> }, >> }; >> > -- To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, 19 Nov 2017 20:48:21 -0500 harinath Nampally <harinath922@gmail.com> wrote: > > > This patch adds following related changes: > > > - 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 > > > - adds 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 which has fxls8471. > > > > > > Signed-off-by: Harinath Nampally <harinath922@gmail.com> > > > > The use of an either direction magnitude event is misleading. > > Tap detection requires a timed sequence of events - a rapid > > increase in acceleration followed by a rapid decrease. > Yes I agree. > > > So.. I'd propose (and I want as much feedback on this as possible) > > > > IIO_EV_TYPE_TAP > > > > Then abuse iio_event_direction to specify single or double (the concept > > of direction doesn't really apply to these in that sense so we'd always > > have them set to none). > Sure, I think this would be very useful as there are lot of modern > accelerometers > with tap event support. It's been true since the start of IIO but we just kept kicking this question into the long grass as we didn't really have the surrounding support that would be ideally needed. They are only really useful in the general sense if we finally do something with the iio-input bridge and put together the in kernel event consumer interface we have never actually written... It's been on the todo list for at least 5 years... oops :( > > Thanks, > Harinath > > On Sun, Nov 19, 2017 at 9:47 AM, Jonathan Cameron <jic23@kernel.org> wrote: > > On Wed, 8 Nov 2017 22:12:57 -0500 > > Harinath Nampally <harinath922@gmail.com> wrote: > > > >> This patch adds following related changes: > >> - 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 > >> - adds 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 which has fxls8471. > >> > >> Signed-off-by: Harinath Nampally <harinath922@gmail.com> > > > > The use of an either direction magnitude event is misleading. > > Tap detection requires a timed sequence of events - a rapid > > increase in acceleration followed by a rapid decrease. > > > > I don't think we can fit tap detection into the existing > > event types (which is annoying but there we are). > > > > So.. I'd propose (and I want as much feedback on this as possible) > > > > IIO_EV_TYPE_TAP > > > > Then abuse iio_event_direction to specify single or double (the concept > > of direction doesn't really apply to these in that sense so we'd always > > have them set to none). > > > > The alternative would be to full out for an abstract representation of > > these events (like step detection) and define a channel type > > IIO_TAP then detect a change in the number of taps (IIO_DIR_NONE), but > > given we have directional taps we would need the modifier for that. > > Hence no means of describing whether it is a single or double tap > > short of burning channel taps. I'm also not sure we want to > > remove the association with a particular sensor channel. > > > > Hence I think I prefer option 1 but would like other's input on this... > > > > Jonathan > > > >> --- > >> 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, > >> }, > >> }; > >> > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-iio" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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, }, };
This patch adds following related changes: - 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 - adds 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 which has fxls8471. Signed-off-by: Harinath Nampally <harinath922@gmail.com> --- drivers/iio/accel/mma8452.c | 156 ++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 151 insertions(+), 5 deletions(-)