* [PATCH] iio: accel: mma8452: Bugfix to enbale and allow different events to work parallely. @ 2017-08-14 22:44 Harinath Nampally 2017-08-16 13:12 ` Peter Meerwald-Stadler 0 siblings, 1 reply; 7+ messages in thread From: Harinath Nampally @ 2017-08-14 22:44 UTC (permalink / raw) To: jic23 Cc: knaack.h, lars, pmeerw, gregkh, linux-iio, linux-kernel, amsfield22, martink This driver supports multiple devices like mma8653, mma8652, mma8452, mma8453 and fxls8471. Almost all these devices have more than one event. Current driver design hardcodes the event specific information, so only one event can be supported by this driver and current design doesn't have the flexibility to add more events. This patch fixes by detaching the event related information from chip_info struct, and based on channel type and event direction the corresponding event configuration registers are picked dynamically. Hence multiple events can be handled in read/write callbacks. Changes are thoroughly tested on fxls8471 device on imx6UL Eval board using iio_event_monitor user space program. After this fix both Freefall and Transient events are handled by the driver without any conflicts. Signed-off-by: Harinath Nampally <harinath922@gmail.com> --- drivers/iio/accel/mma8452.c | 309 ++++++++++++++++++++------------------------ 1 file changed, 141 insertions(+), 168 deletions(-) diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c index eb6e3dc..1b83e52 100644 --- a/drivers/iio/accel/mma8452.c +++ b/drivers/iio/accel/mma8452.c @@ -59,7 +59,9 @@ #define MMA8452_FF_MT_THS 0x17 #define MMA8452_FF_MT_THS_MASK 0x7f #define MMA8452_FF_MT_COUNT 0x18 +#define MMA8452_FF_MT_CHAN_SHIFT 3 #define MMA8452_TRANSIENT_CFG 0x1d +#define MMA8452_TRANSIENT_CFG_CHAN(chan) BIT(chan + 1) #define MMA8452_TRANSIENT_CFG_HPF_BYP BIT(0) #define MMA8452_TRANSIENT_CFG_ELE BIT(4) #define MMA8452_TRANSIENT_SRC 0x1e @@ -69,6 +71,7 @@ #define MMA8452_TRANSIENT_THS 0x1f #define MMA8452_TRANSIENT_THS_MASK GENMASK(6, 0) #define MMA8452_TRANSIENT_COUNT 0x20 +#define MMA8452_TRANSIENT_CHAN_SHIFT 1 #define MMA8452_CTRL_REG1 0x2a #define MMA8452_CTRL_ACTIVE BIT(0) #define MMA8452_CTRL_DR_MASK GENMASK(5, 3) @@ -107,6 +110,26 @@ struct mma8452_data { const struct mma_chip_info *chip_info; }; + /** + * struct mma8452_event_regs - chip specific data related to events + * @ev_cfg: event config register address + * @ev_src: event source register address + * @ev_ths: event threshold register address + * @ev_ths_mask: mask for the threshold value + * @ev_count: event count (period) register address + * + * Since not all chips supported by the driver support comparing high pass + * filtered data for events (interrupts), different interrupt sources are + * used for different chips and the relevant registers are included here. + */ +struct mma8452_event_regs { + u8 ev_cfg; + u8 ev_src; + u8 ev_ths; + u8 ev_ths_mask; + u8 ev_count; +}; + /** * struct mma_chip_info - chip specific data * @chip_id: WHO_AM_I register's value @@ -116,40 +139,12 @@ struct mma8452_data { * @mma_scales: scale factors for converting register values * to m/s^2; 3 modes: 2g, 4g, 8g; 2 integers * per mode: m/s^2 and micro m/s^2 - * @ev_cfg: event config register address - * @ev_cfg_ele: latch bit in event config register - * @ev_cfg_chan_shift: number of the bit to enable events in X - * direction; in event config register - * @ev_src: event source register address - * @ev_src_xe: bit in event source register that indicates - * an event in X direction - * @ev_src_ye: bit in event source register that indicates - * an event in Y direction - * @ev_src_ze: bit in event source register that indicates - * an event in Z direction - * @ev_ths: event threshold register address - * @ev_ths_mask: mask for the threshold value - * @ev_count: event count (period) register address - * - * Since not all chips supported by the driver support comparing high pass - * filtered data for events (interrupts), different interrupt sources are - * used for different chips and the relevant registers are included here. */ struct mma_chip_info { u8 chip_id; const struct iio_chan_spec *channels; int num_channels; const int mma_scales[3][2]; - u8 ev_cfg; - u8 ev_cfg_ele; - u8 ev_cfg_chan_shift; - u8 ev_src; - u8 ev_src_xe; - u8 ev_src_ye; - u8 ev_src_ze; - u8 ev_ths; - u8 ev_ths_mask; - u8 ev_count; }; enum { @@ -602,9 +597,8 @@ static int mma8452_set_power_mode(struct mma8452_data *data, u8 mode) static int mma8452_freefall_mode_enabled(struct mma8452_data *data) { int val; - const struct mma_chip_info *chip = data->chip_info; - val = i2c_smbus_read_byte_data(data->client, chip->ev_cfg); + val = i2c_smbus_read_byte_data(data->client, MMA8452_FF_MT_CFG); if (val < 0) return val; @@ -614,29 +608,28 @@ static int mma8452_freefall_mode_enabled(struct mma8452_data *data) static int mma8452_set_freefall_mode(struct mma8452_data *data, bool state) { int val; - const struct mma_chip_info *chip = data->chip_info; if ((state && mma8452_freefall_mode_enabled(data)) || (!state && !(mma8452_freefall_mode_enabled(data)))) return 0; - val = i2c_smbus_read_byte_data(data->client, chip->ev_cfg); + val = i2c_smbus_read_byte_data(data->client, MMA8452_FF_MT_CFG); if (val < 0) return val; if (state) { - val |= BIT(idx_x + chip->ev_cfg_chan_shift); - val |= BIT(idx_y + chip->ev_cfg_chan_shift); - val |= BIT(idx_z + chip->ev_cfg_chan_shift); + val |= BIT(idx_x + MMA8452_FF_MT_CHAN_SHIFT); + val |= BIT(idx_y + MMA8452_FF_MT_CHAN_SHIFT); + val |= BIT(idx_z + MMA8452_FF_MT_CHAN_SHIFT); val &= ~MMA8452_FF_MT_CFG_OAE; } else { - val &= ~BIT(idx_x + chip->ev_cfg_chan_shift); - val &= ~BIT(idx_y + chip->ev_cfg_chan_shift); - val &= ~BIT(idx_z + chip->ev_cfg_chan_shift); + val &= ~BIT(idx_x + MMA8452_FF_MT_CHAN_SHIFT); + val &= ~BIT(idx_y + MMA8452_FF_MT_CHAN_SHIFT); + val &= ~BIT(idx_z + MMA8452_FF_MT_CHAN_SHIFT); val |= MMA8452_FF_MT_CFG_OAE; } - return mma8452_change_config(data, chip->ev_cfg, val); + return mma8452_change_config(data, MMA8452_FF_MT_CFG, val); } static int mma8452_set_hp_filter_frequency(struct mma8452_data *data, @@ -740,6 +733,39 @@ static int mma8452_write_raw(struct iio_dev *indio_dev, return ret; } +static int mma8452_get_event_regs(const struct iio_chan_spec *chan, + enum iio_event_direction dir, + struct mma8452_event_regs *ev_regs + ) +{ + if (!chan) + return -EINVAL; + switch (chan->type) { + case IIO_ACCEL: + switch (dir) { + case IIO_EV_DIR_FALLING: + ev_regs->ev_cfg = MMA8452_FF_MT_CFG; + ev_regs->ev_src = MMA8452_FF_MT_SRC; + ev_regs->ev_ths = MMA8452_FF_MT_THS; + ev_regs->ev_ths_mask = MMA8452_FF_MT_THS_MASK; + ev_regs->ev_count = MMA8452_FF_MT_COUNT; + break; + case IIO_EV_DIR_RISING: + ev_regs->ev_cfg = MMA8452_TRANSIENT_CFG; + ev_regs->ev_src = MMA8452_TRANSIENT_SRC; + ev_regs->ev_ths = MMA8452_TRANSIENT_THS; + ev_regs->ev_count = MMA8452_TRANSIENT_COUNT; + break; + default: + return -EINVAL; + } + break; + default: + return -EINVAL; + } + return 0; +} + static int mma8452_read_thresh(struct iio_dev *indio_dev, const struct iio_chan_spec *chan, enum iio_event_type type, @@ -749,21 +775,23 @@ static int mma8452_read_thresh(struct iio_dev *indio_dev, { struct mma8452_data *data = iio_priv(indio_dev); int ret, us, power_mode; + struct mma8452_event_regs ev_regs; + ret = mma8452_get_event_regs(chan, dir, &ev_regs); + if (ret) + return ret; switch (info) { case IIO_EV_INFO_VALUE: - ret = i2c_smbus_read_byte_data(data->client, - data->chip_info->ev_ths); + ret = i2c_smbus_read_byte_data(data->client, ev_regs.ev_ths); if (ret < 0) return ret; - *val = ret & data->chip_info->ev_ths_mask; + *val = ret & ev_regs.ev_ths_mask; return IIO_VAL_INT; case IIO_EV_INFO_PERIOD: - ret = i2c_smbus_read_byte_data(data->client, - data->chip_info->ev_count); + ret = i2c_smbus_read_byte_data(data->client, ev_regs.ev_count); if (ret < 0) return ret; @@ -809,14 +837,18 @@ static int mma8452_write_thresh(struct iio_dev *indio_dev, { struct mma8452_data *data = iio_priv(indio_dev); int ret, reg, steps; + struct mma8452_event_regs ev_regs; + + ret = mma8452_get_event_regs(chan, dir, &ev_regs); + if (ret) + return ret; switch (info) { case IIO_EV_INFO_VALUE: - if (val < 0 || val > MMA8452_TRANSIENT_THS_MASK) + if (val < 0 || val > ev_regs.ev_ths_mask) return -EINVAL; - return mma8452_change_config(data, data->chip_info->ev_ths, - val); + return mma8452_change_config(data, ev_regs.ev_ths, val); case IIO_EV_INFO_PERIOD: ret = mma8452_get_power_mode(data); @@ -830,8 +862,7 @@ static int mma8452_write_thresh(struct iio_dev *indio_dev, if (steps < 0 || steps > 0xff) return -EINVAL; - return mma8452_change_config(data, data->chip_info->ev_count, - steps); + return mma8452_change_config(data, ev_regs.ev_count, steps); case IIO_EV_INFO_HIGH_PASS_FILTER_3DB: reg = i2c_smbus_read_byte_data(data->client, @@ -861,23 +892,23 @@ static int mma8452_read_event_config(struct iio_dev *indio_dev, enum iio_event_direction dir) { struct mma8452_data *data = iio_priv(indio_dev); - const struct mma_chip_info *chip = data->chip_info; int ret; - switch (dir) { - case IIO_EV_DIR_FALLING: - return mma8452_freefall_mode_enabled(data); - case IIO_EV_DIR_RISING: - if (mma8452_freefall_mode_enabled(data)) - return 0; + switch (chan->type) { + case IIO_ACCEL: + switch (dir) { + case IIO_EV_DIR_FALLING: + return mma8452_freefall_mode_enabled(data); + case IIO_EV_DIR_RISING: + ret = i2c_smbus_read_byte_data(data->client, MMA8452_TRANSIENT_CFG); + if (ret < 0) + return ret; - ret = i2c_smbus_read_byte_data(data->client, - data->chip_info->ev_cfg); - if (ret < 0) - return ret; + return !!(ret & MMA8452_TRANSIENT_CFG_CHAN(chan->scan_index)); - return !!(ret & BIT(chan->scan_index + - chip->ev_cfg_chan_shift)); + default: + return -EINVAL; + } default: return -EINVAL; } @@ -890,39 +921,33 @@ static int mma8452_write_event_config(struct iio_dev *indio_dev, int state) { struct mma8452_data *data = iio_priv(indio_dev); - const struct mma_chip_info *chip = data->chip_info; int val, ret; ret = mma8452_set_runtime_pm_state(data->client, state); if (ret) return ret; - switch (dir) { - case IIO_EV_DIR_FALLING: - return mma8452_set_freefall_mode(data, state); - case IIO_EV_DIR_RISING: - val = i2c_smbus_read_byte_data(data->client, chip->ev_cfg); - if (val < 0) - return val; - - if (state) { - if (mma8452_freefall_mode_enabled(data)) { - val &= ~BIT(idx_x + chip->ev_cfg_chan_shift); - val &= ~BIT(idx_y + chip->ev_cfg_chan_shift); - val &= ~BIT(idx_z + chip->ev_cfg_chan_shift); - val |= MMA8452_FF_MT_CFG_OAE; - } - val |= BIT(chan->scan_index + chip->ev_cfg_chan_shift); - } else { - if (mma8452_freefall_mode_enabled(data)) - return 0; - - val &= ~BIT(chan->scan_index + chip->ev_cfg_chan_shift); + switch (chan->type) { + case IIO_ACCEL: + switch (dir) { + case IIO_EV_DIR_FALLING: + return mma8452_set_freefall_mode(data, state); + case IIO_EV_DIR_RISING: + val = i2c_smbus_read_byte_data(data->client, MMA8452_TRANSIENT_CFG); + if (val < 0) + return val; + + if (state) + val |= MMA8452_TRANSIENT_CFG_CHAN(chan->scan_index); + else + val &= ~MMA8452_TRANSIENT_CFG_CHAN(chan->scan_index); + + val |= MMA8452_TRANSIENT_CFG_ELE; + + return mma8452_change_config(data, MMA8452_TRANSIENT_CFG, val); + default: + return -EINVAL; } - - val |= chip->ev_cfg_ele; - - return mma8452_change_config(data, chip->ev_cfg, val); default: return -EINVAL; } @@ -934,35 +959,25 @@ static void mma8452_transient_interrupt(struct iio_dev *indio_dev) s64 ts = iio_get_time_ns(indio_dev); int src; - src = i2c_smbus_read_byte_data(data->client, data->chip_info->ev_src); + src = i2c_smbus_read_byte_data(data->client, MMA8452_TRANSIENT_SRC); if (src < 0) return; - if (mma8452_freefall_mode_enabled(data)) { - iio_push_event(indio_dev, - IIO_MOD_EVENT_CODE(IIO_ACCEL, 0, - IIO_MOD_X_AND_Y_AND_Z, - IIO_EV_TYPE_MAG, - IIO_EV_DIR_FALLING), - ts); - return; - } - - if (src & data->chip_info->ev_src_xe) + if (src & MMA8452_TRANSIENT_SRC_XTRANSE) iio_push_event(indio_dev, IIO_MOD_EVENT_CODE(IIO_ACCEL, 0, IIO_MOD_X, IIO_EV_TYPE_MAG, IIO_EV_DIR_RISING), ts); - if (src & data->chip_info->ev_src_ye) + if (src & MMA8452_TRANSIENT_SRC_YTRANSE) iio_push_event(indio_dev, IIO_MOD_EVENT_CODE(IIO_ACCEL, 0, IIO_MOD_Y, IIO_EV_TYPE_MAG, IIO_EV_DIR_RISING), ts); - if (src & data->chip_info->ev_src_ze) + if (src & MMA8452_TRANSIENT_SRC_ZTRANSE) iio_push_event(indio_dev, IIO_MOD_EVENT_CODE(IIO_ACCEL, 0, IIO_MOD_Z, IIO_EV_TYPE_MAG, @@ -974,23 +989,41 @@ static irqreturn_t mma8452_interrupt(int irq, void *p) { struct iio_dev *indio_dev = p; struct mma8452_data *data = iio_priv(indio_dev); - const struct mma_chip_info *chip = data->chip_info; int ret = IRQ_NONE; - int src; + int src, enabled_interrupts; src = i2c_smbus_read_byte_data(data->client, MMA8452_INT_SRC); if (src < 0) return IRQ_NONE; + enabled_interrupts = i2c_smbus_read_byte_data(data->client, + MMA8452_CTRL_REG4); + if (enabled_interrupts < 0) + return enabled_interrupts; + + if (!(src & enabled_interrupts)) + return IRQ_NONE; + if (src & MMA8452_INT_DRDY) { iio_trigger_poll_chained(indio_dev->trig); ret = IRQ_HANDLED; } - if ((src & MMA8452_INT_TRANS && - chip->ev_src == MMA8452_TRANSIENT_SRC) || - (src & MMA8452_INT_FF_MT && - chip->ev_src == MMA8452_FF_MT_SRC)) { + if (src & MMA8452_INT_FF_MT) { + if (mma8452_freefall_mode_enabled(data)) { + s64 ts = iio_get_time_ns(indio_dev); + + iio_push_event(indio_dev, + IIO_MOD_EVENT_CODE(IIO_ACCEL, 0, + IIO_MOD_X_AND_Y_AND_Z, + IIO_EV_TYPE_MAG, + IIO_EV_DIR_FALLING), + ts); + } + ret = IRQ_HANDLED; + } + + if (src & MMA8452_INT_TRANS) { mma8452_transient_interrupt(indio_dev); ret = IRQ_HANDLED; } @@ -1222,96 +1255,36 @@ static const struct mma_chip_info mma_chip_info_table[] = { * g * N * 1000000 / 2048 for N = 2, 4, 8 and g=9.80665 */ .mma_scales = { {0, 2394}, {0, 4788}, {0, 9577} }, - .ev_cfg = MMA8452_TRANSIENT_CFG, - .ev_cfg_ele = MMA8452_TRANSIENT_CFG_ELE, - .ev_cfg_chan_shift = 1, - .ev_src = MMA8452_TRANSIENT_SRC, - .ev_src_xe = MMA8452_TRANSIENT_SRC_XTRANSE, - .ev_src_ye = MMA8452_TRANSIENT_SRC_YTRANSE, - .ev_src_ze = MMA8452_TRANSIENT_SRC_ZTRANSE, - .ev_ths = MMA8452_TRANSIENT_THS, - .ev_ths_mask = MMA8452_TRANSIENT_THS_MASK, - .ev_count = MMA8452_TRANSIENT_COUNT, }, [mma8452] = { .chip_id = MMA8452_DEVICE_ID, .channels = mma8452_channels, .num_channels = ARRAY_SIZE(mma8452_channels), .mma_scales = { {0, 9577}, {0, 19154}, {0, 38307} }, - .ev_cfg = MMA8452_TRANSIENT_CFG, - .ev_cfg_ele = MMA8452_TRANSIENT_CFG_ELE, - .ev_cfg_chan_shift = 1, - .ev_src = MMA8452_TRANSIENT_SRC, - .ev_src_xe = MMA8452_TRANSIENT_SRC_XTRANSE, - .ev_src_ye = MMA8452_TRANSIENT_SRC_YTRANSE, - .ev_src_ze = MMA8452_TRANSIENT_SRC_ZTRANSE, - .ev_ths = MMA8452_TRANSIENT_THS, - .ev_ths_mask = MMA8452_TRANSIENT_THS_MASK, - .ev_count = MMA8452_TRANSIENT_COUNT, }, [mma8453] = { .chip_id = MMA8453_DEVICE_ID, .channels = mma8453_channels, .num_channels = ARRAY_SIZE(mma8453_channels), .mma_scales = { {0, 38307}, {0, 76614}, {0, 153228} }, - .ev_cfg = MMA8452_TRANSIENT_CFG, - .ev_cfg_ele = MMA8452_TRANSIENT_CFG_ELE, - .ev_cfg_chan_shift = 1, - .ev_src = MMA8452_TRANSIENT_SRC, - .ev_src_xe = MMA8452_TRANSIENT_SRC_XTRANSE, - .ev_src_ye = MMA8452_TRANSIENT_SRC_YTRANSE, - .ev_src_ze = MMA8452_TRANSIENT_SRC_ZTRANSE, - .ev_ths = MMA8452_TRANSIENT_THS, - .ev_ths_mask = MMA8452_TRANSIENT_THS_MASK, - .ev_count = MMA8452_TRANSIENT_COUNT, }, [mma8652] = { .chip_id = MMA8652_DEVICE_ID, .channels = mma8652_channels, .num_channels = ARRAY_SIZE(mma8652_channels), .mma_scales = { {0, 9577}, {0, 19154}, {0, 38307} }, - .ev_cfg = MMA8452_FF_MT_CFG, - .ev_cfg_ele = MMA8452_FF_MT_CFG_ELE, - .ev_cfg_chan_shift = 3, - .ev_src = MMA8452_FF_MT_SRC, - .ev_src_xe = MMA8452_FF_MT_SRC_XHE, - .ev_src_ye = MMA8452_FF_MT_SRC_YHE, - .ev_src_ze = MMA8452_FF_MT_SRC_ZHE, - .ev_ths = MMA8452_FF_MT_THS, - .ev_ths_mask = MMA8452_FF_MT_THS_MASK, - .ev_count = MMA8452_FF_MT_COUNT, }, [mma8653] = { .chip_id = MMA8653_DEVICE_ID, .channels = mma8653_channels, .num_channels = ARRAY_SIZE(mma8653_channels), .mma_scales = { {0, 38307}, {0, 76614}, {0, 153228} }, - .ev_cfg = MMA8452_FF_MT_CFG, - .ev_cfg_ele = MMA8452_FF_MT_CFG_ELE, - .ev_cfg_chan_shift = 3, - .ev_src = MMA8452_FF_MT_SRC, - .ev_src_xe = MMA8452_FF_MT_SRC_XHE, - .ev_src_ye = MMA8452_FF_MT_SRC_YHE, - .ev_src_ze = MMA8452_FF_MT_SRC_ZHE, - .ev_ths = MMA8452_FF_MT_THS, - .ev_ths_mask = MMA8452_FF_MT_THS_MASK, - .ev_count = MMA8452_FF_MT_COUNT, }, [fxls8471] = { .chip_id = FXLS8471_DEVICE_ID, .channels = mma8451_channels, .num_channels = ARRAY_SIZE(mma8451_channels), .mma_scales = { {0, 2394}, {0, 4788}, {0, 9577} }, - .ev_cfg = MMA8452_TRANSIENT_CFG, - .ev_cfg_ele = MMA8452_TRANSIENT_CFG_ELE, - .ev_cfg_chan_shift = 1, - .ev_src = MMA8452_TRANSIENT_SRC, - .ev_src_xe = MMA8452_TRANSIENT_SRC_XTRANSE, - .ev_src_ye = MMA8452_TRANSIENT_SRC_YTRANSE, - .ev_src_ze = MMA8452_TRANSIENT_SRC_ZTRANSE, - .ev_ths = MMA8452_TRANSIENT_THS, - .ev_ths_mask = MMA8452_TRANSIENT_THS_MASK, - .ev_count = MMA8452_TRANSIENT_COUNT, }, }; -- 2.7.4 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] iio: accel: mma8452: Bugfix to enbale and allow different events to work parallely. 2017-08-14 22:44 [PATCH] iio: accel: mma8452: Bugfix to enbale and allow different events to work parallely Harinath Nampally @ 2017-08-16 13:12 ` Peter Meerwald-Stadler 2017-08-17 0:39 ` Harinath Nampally 0 siblings, 1 reply; 7+ messages in thread From: Peter Meerwald-Stadler @ 2017-08-16 13:12 UTC (permalink / raw) To: Harinath Nampally Cc: jic23, knaack.h, lars, gregkh, linux-iio, linux-kernel, amsfield22, martink Hello, fixes are always welcome, some comments regarding the patch in subject: typo "enable" > This driver supports multiple devices like mma8653, mma8652, mma8452, mma8453 and > fxls8471. Almost all these devices have more than one event. Current driver design > hardcodes the event specific information, so only one event can be supported by this > driver and current design doesn't have the flexibility to add more events. > This patch fixes by detaching the event related information from chip_info struct, > and based on channel type and event direction the corresponding event configuration registers > are picked dynamically. Hence multiple events can be handled in read/write callbacks. which chip can have which event(s)? > Changes are thoroughly tested on fxls8471 device on imx6UL Eval board using iio_event_monitor user space program. > > After this fix both Freefall and Transient events are handled by the driver without any conflicts. thanks, p. > Signed-off-by: Harinath Nampally <harinath922@gmail.com> > --- > drivers/iio/accel/mma8452.c | 309 ++++++++++++++++++++------------------------ > 1 file changed, 141 insertions(+), 168 deletions(-) > > diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c > index eb6e3dc..1b83e52 100644 > --- a/drivers/iio/accel/mma8452.c > +++ b/drivers/iio/accel/mma8452.c > @@ -59,7 +59,9 @@ > #define MMA8452_FF_MT_THS 0x17 > #define MMA8452_FF_MT_THS_MASK 0x7f > #define MMA8452_FF_MT_COUNT 0x18 > +#define MMA8452_FF_MT_CHAN_SHIFT 3 > #define MMA8452_TRANSIENT_CFG 0x1d > +#define MMA8452_TRANSIENT_CFG_CHAN(chan) BIT(chan + 1) > #define MMA8452_TRANSIENT_CFG_HPF_BYP BIT(0) > #define MMA8452_TRANSIENT_CFG_ELE BIT(4) > #define MMA8452_TRANSIENT_SRC 0x1e > @@ -69,6 +71,7 @@ > #define MMA8452_TRANSIENT_THS 0x1f > #define MMA8452_TRANSIENT_THS_MASK GENMASK(6, 0) > #define MMA8452_TRANSIENT_COUNT 0x20 > +#define MMA8452_TRANSIENT_CHAN_SHIFT 1 > #define MMA8452_CTRL_REG1 0x2a > #define MMA8452_CTRL_ACTIVE BIT(0) > #define MMA8452_CTRL_DR_MASK GENMASK(5, 3) > @@ -107,6 +110,26 @@ struct mma8452_data { > const struct mma_chip_info *chip_info; > }; > > + /** > + * struct mma8452_event_regs - chip specific data related to events > + * @ev_cfg: event config register address > + * @ev_src: event source register address > + * @ev_ths: event threshold register address > + * @ev_ths_mask: mask for the threshold value > + * @ev_count: event count (period) register address > + * > + * Since not all chips supported by the driver support comparing high pass > + * filtered data for events (interrupts), different interrupt sources are > + * used for different chips and the relevant registers are included here. > + */ > +struct mma8452_event_regs { > + u8 ev_cfg; > + u8 ev_src; > + u8 ev_ths; > + u8 ev_ths_mask; > + u8 ev_count; > +}; > + > /** > * struct mma_chip_info - chip specific data > * @chip_id: WHO_AM_I register's value > @@ -116,40 +139,12 @@ struct mma8452_data { > * @mma_scales: scale factors for converting register values > * to m/s^2; 3 modes: 2g, 4g, 8g; 2 integers > * per mode: m/s^2 and micro m/s^2 > - * @ev_cfg: event config register address > - * @ev_cfg_ele: latch bit in event config register > - * @ev_cfg_chan_shift: number of the bit to enable events in X > - * direction; in event config register > - * @ev_src: event source register address > - * @ev_src_xe: bit in event source register that indicates > - * an event in X direction > - * @ev_src_ye: bit in event source register that indicates > - * an event in Y direction > - * @ev_src_ze: bit in event source register that indicates > - * an event in Z direction > - * @ev_ths: event threshold register address > - * @ev_ths_mask: mask for the threshold value > - * @ev_count: event count (period) register address > - * > - * Since not all chips supported by the driver support comparing high pass > - * filtered data for events (interrupts), different interrupt sources are > - * used for different chips and the relevant registers are included here. > */ > struct mma_chip_info { > u8 chip_id; > const struct iio_chan_spec *channels; > int num_channels; > const int mma_scales[3][2]; > - u8 ev_cfg; > - u8 ev_cfg_ele; > - u8 ev_cfg_chan_shift; > - u8 ev_src; > - u8 ev_src_xe; > - u8 ev_src_ye; > - u8 ev_src_ze; > - u8 ev_ths; > - u8 ev_ths_mask; > - u8 ev_count; > }; > > enum { > @@ -602,9 +597,8 @@ static int mma8452_set_power_mode(struct mma8452_data *data, u8 mode) > static int mma8452_freefall_mode_enabled(struct mma8452_data *data) > { > int val; > - const struct mma_chip_info *chip = data->chip_info; > > - val = i2c_smbus_read_byte_data(data->client, chip->ev_cfg); > + val = i2c_smbus_read_byte_data(data->client, MMA8452_FF_MT_CFG); > if (val < 0) > return val; > > @@ -614,29 +608,28 @@ static int mma8452_freefall_mode_enabled(struct mma8452_data *data) > static int mma8452_set_freefall_mode(struct mma8452_data *data, bool state) > { > int val; > - const struct mma_chip_info *chip = data->chip_info; > > if ((state && mma8452_freefall_mode_enabled(data)) || > (!state && !(mma8452_freefall_mode_enabled(data)))) > return 0; > > - val = i2c_smbus_read_byte_data(data->client, chip->ev_cfg); > + val = i2c_smbus_read_byte_data(data->client, MMA8452_FF_MT_CFG); > if (val < 0) > return val; > > if (state) { > - val |= BIT(idx_x + chip->ev_cfg_chan_shift); > - val |= BIT(idx_y + chip->ev_cfg_chan_shift); > - val |= BIT(idx_z + chip->ev_cfg_chan_shift); > + val |= BIT(idx_x + MMA8452_FF_MT_CHAN_SHIFT); > + val |= BIT(idx_y + MMA8452_FF_MT_CHAN_SHIFT); > + val |= BIT(idx_z + MMA8452_FF_MT_CHAN_SHIFT); > val &= ~MMA8452_FF_MT_CFG_OAE; > } else { > - val &= ~BIT(idx_x + chip->ev_cfg_chan_shift); > - val &= ~BIT(idx_y + chip->ev_cfg_chan_shift); > - val &= ~BIT(idx_z + chip->ev_cfg_chan_shift); > + val &= ~BIT(idx_x + MMA8452_FF_MT_CHAN_SHIFT); > + val &= ~BIT(idx_y + MMA8452_FF_MT_CHAN_SHIFT); > + val &= ~BIT(idx_z + MMA8452_FF_MT_CHAN_SHIFT); > val |= MMA8452_FF_MT_CFG_OAE; > } > > - return mma8452_change_config(data, chip->ev_cfg, val); > + return mma8452_change_config(data, MMA8452_FF_MT_CFG, val); > } > > static int mma8452_set_hp_filter_frequency(struct mma8452_data *data, > @@ -740,6 +733,39 @@ static int mma8452_write_raw(struct iio_dev *indio_dev, > return ret; > } > > +static int mma8452_get_event_regs(const struct iio_chan_spec *chan, > + enum iio_event_direction dir, > + struct mma8452_event_regs *ev_regs > + ) > +{ > + if (!chan) > + return -EINVAL; > + switch (chan->type) { > + case IIO_ACCEL: > + switch (dir) { > + case IIO_EV_DIR_FALLING: > + ev_regs->ev_cfg = MMA8452_FF_MT_CFG; > + ev_regs->ev_src = MMA8452_FF_MT_SRC; > + ev_regs->ev_ths = MMA8452_FF_MT_THS; > + ev_regs->ev_ths_mask = MMA8452_FF_MT_THS_MASK; > + ev_regs->ev_count = MMA8452_FF_MT_COUNT; > + break; > + case IIO_EV_DIR_RISING: > + ev_regs->ev_cfg = MMA8452_TRANSIENT_CFG; > + ev_regs->ev_src = MMA8452_TRANSIENT_SRC; > + ev_regs->ev_ths = MMA8452_TRANSIENT_THS; ev_ths_mask is not set here > + ev_regs->ev_count = MMA8452_TRANSIENT_COUNT; > + break; > + default: > + return -EINVAL; > + } > + break; > + default: > + return -EINVAL; > + } > + return 0; could replace 'breaks' with return 0 to save some lines and simplify control flow > +} > + > static int mma8452_read_thresh(struct iio_dev *indio_dev, > const struct iio_chan_spec *chan, > enum iio_event_type type, > @@ -749,21 +775,23 @@ static int mma8452_read_thresh(struct iio_dev *indio_dev, > { > struct mma8452_data *data = iio_priv(indio_dev); > int ret, us, power_mode; > + struct mma8452_event_regs ev_regs; > > + ret = mma8452_get_event_regs(chan, dir, &ev_regs); > + if (ret) > + return ret; > switch (info) { > case IIO_EV_INFO_VALUE: > - ret = i2c_smbus_read_byte_data(data->client, > - data->chip_info->ev_ths); > + ret = i2c_smbus_read_byte_data(data->client, ev_regs.ev_ths); > if (ret < 0) > return ret; > > - *val = ret & data->chip_info->ev_ths_mask; > + *val = ret & ev_regs.ev_ths_mask; > > return IIO_VAL_INT; > > case IIO_EV_INFO_PERIOD: > - ret = i2c_smbus_read_byte_data(data->client, > - data->chip_info->ev_count); > + ret = i2c_smbus_read_byte_data(data->client, ev_regs.ev_count); > if (ret < 0) > return ret; > > @@ -809,14 +837,18 @@ static int mma8452_write_thresh(struct iio_dev *indio_dev, > { > struct mma8452_data *data = iio_priv(indio_dev); > int ret, reg, steps; > + struct mma8452_event_regs ev_regs; > + > + ret = mma8452_get_event_regs(chan, dir, &ev_regs); > + if (ret) > + return ret; > > switch (info) { > case IIO_EV_INFO_VALUE: > - if (val < 0 || val > MMA8452_TRANSIENT_THS_MASK) > + if (val < 0 || val > ev_regs.ev_ths_mask) > return -EINVAL; > > - return mma8452_change_config(data, data->chip_info->ev_ths, > - val); > + return mma8452_change_config(data, ev_regs.ev_ths, val); > > case IIO_EV_INFO_PERIOD: > ret = mma8452_get_power_mode(data); > @@ -830,8 +862,7 @@ static int mma8452_write_thresh(struct iio_dev *indio_dev, > if (steps < 0 || steps > 0xff) > return -EINVAL; > > - return mma8452_change_config(data, data->chip_info->ev_count, > - steps); > + return mma8452_change_config(data, ev_regs.ev_count, steps); > > case IIO_EV_INFO_HIGH_PASS_FILTER_3DB: > reg = i2c_smbus_read_byte_data(data->client, > @@ -861,23 +892,23 @@ static int mma8452_read_event_config(struct iio_dev *indio_dev, > enum iio_event_direction dir) > { > struct mma8452_data *data = iio_priv(indio_dev); > - const struct mma_chip_info *chip = data->chip_info; > int ret; > > - switch (dir) { > - case IIO_EV_DIR_FALLING: > - return mma8452_freefall_mode_enabled(data); > - case IIO_EV_DIR_RISING: > - if (mma8452_freefall_mode_enabled(data)) > - return 0; > + switch (chan->type) { > + case IIO_ACCEL: this adds a check for chan-type == IIO_ACCESS, so strictly this would be an unrelated change... > + switch (dir) { > + case IIO_EV_DIR_FALLING: > + return mma8452_freefall_mode_enabled(data); > + case IIO_EV_DIR_RISING: > + ret = i2c_smbus_read_byte_data(data->client, MMA8452_TRANSIENT_CFG); > + if (ret < 0) > + return ret; > > - ret = i2c_smbus_read_byte_data(data->client, > - data->chip_info->ev_cfg); > - if (ret < 0) > - return ret; > + return !!(ret & MMA8452_TRANSIENT_CFG_CHAN(chan->scan_index)); > > - return !!(ret & BIT(chan->scan_index + > - chip->ev_cfg_chan_shift)); > + default: > + return -EINVAL; > + } > default: > return -EINVAL; > } > @@ -890,39 +921,33 @@ static int mma8452_write_event_config(struct iio_dev *indio_dev, > int state) > { > struct mma8452_data *data = iio_priv(indio_dev); > - const struct mma_chip_info *chip = data->chip_info; > int val, ret; > > ret = mma8452_set_runtime_pm_state(data->client, state); > if (ret) > return ret; > > - switch (dir) { > - case IIO_EV_DIR_FALLING: > - return mma8452_set_freefall_mode(data, state); > - case IIO_EV_DIR_RISING: > - val = i2c_smbus_read_byte_data(data->client, chip->ev_cfg); > - if (val < 0) > - return val; > - > - if (state) { > - if (mma8452_freefall_mode_enabled(data)) { > - val &= ~BIT(idx_x + chip->ev_cfg_chan_shift); > - val &= ~BIT(idx_y + chip->ev_cfg_chan_shift); > - val &= ~BIT(idx_z + chip->ev_cfg_chan_shift); > - val |= MMA8452_FF_MT_CFG_OAE; > - } > - val |= BIT(chan->scan_index + chip->ev_cfg_chan_shift); > - } else { > - if (mma8452_freefall_mode_enabled(data)) > - return 0; > - > - val &= ~BIT(chan->scan_index + chip->ev_cfg_chan_shift); > + switch (chan->type) { > + case IIO_ACCEL: > + switch (dir) { > + case IIO_EV_DIR_FALLING: > + return mma8452_set_freefall_mode(data, state); > + case IIO_EV_DIR_RISING: > + val = i2c_smbus_read_byte_data(data->client, MMA8452_TRANSIENT_CFG); > + if (val < 0) > + return val; > + > + if (state) > + val |= MMA8452_TRANSIENT_CFG_CHAN(chan->scan_index); > + else > + val &= ~MMA8452_TRANSIENT_CFG_CHAN(chan->scan_index); > + > + val |= MMA8452_TRANSIENT_CFG_ELE; > + > + return mma8452_change_config(data, MMA8452_TRANSIENT_CFG, val); > + default: > + return -EINVAL; > } > - > - val |= chip->ev_cfg_ele; > - > - return mma8452_change_config(data, chip->ev_cfg, val); > default: > return -EINVAL; > } > @@ -934,35 +959,25 @@ static void mma8452_transient_interrupt(struct iio_dev *indio_dev) > s64 ts = iio_get_time_ns(indio_dev); > int src; > > - src = i2c_smbus_read_byte_data(data->client, data->chip_info->ev_src); > + src = i2c_smbus_read_byte_data(data->client, MMA8452_TRANSIENT_SRC); > if (src < 0) > return; > > - if (mma8452_freefall_mode_enabled(data)) { > - iio_push_event(indio_dev, > - IIO_MOD_EVENT_CODE(IIO_ACCEL, 0, > - IIO_MOD_X_AND_Y_AND_Z, > - IIO_EV_TYPE_MAG, > - IIO_EV_DIR_FALLING), > - ts); > - return; > - } > - > - if (src & data->chip_info->ev_src_xe) > + if (src & MMA8452_TRANSIENT_SRC_XTRANSE) > iio_push_event(indio_dev, > IIO_MOD_EVENT_CODE(IIO_ACCEL, 0, IIO_MOD_X, > IIO_EV_TYPE_MAG, > IIO_EV_DIR_RISING), > ts); > > - if (src & data->chip_info->ev_src_ye) > + if (src & MMA8452_TRANSIENT_SRC_YTRANSE) > iio_push_event(indio_dev, > IIO_MOD_EVENT_CODE(IIO_ACCEL, 0, IIO_MOD_Y, > IIO_EV_TYPE_MAG, > IIO_EV_DIR_RISING), > ts); > > - if (src & data->chip_info->ev_src_ze) > + if (src & MMA8452_TRANSIENT_SRC_ZTRANSE) > iio_push_event(indio_dev, > IIO_MOD_EVENT_CODE(IIO_ACCEL, 0, IIO_MOD_Z, > IIO_EV_TYPE_MAG, > @@ -974,23 +989,41 @@ static irqreturn_t mma8452_interrupt(int irq, void *p) > { > struct iio_dev *indio_dev = p; > struct mma8452_data *data = iio_priv(indio_dev); > - const struct mma_chip_info *chip = data->chip_info; > int ret = IRQ_NONE; > - int src; > + int src, enabled_interrupts; > > src = i2c_smbus_read_byte_data(data->client, MMA8452_INT_SRC); > if (src < 0) > return IRQ_NONE; > > + enabled_interrupts = i2c_smbus_read_byte_data(data->client, > + MMA8452_CTRL_REG4); > + if (enabled_interrupts < 0) > + return enabled_interrupts; > + > + if (!(src & enabled_interrupts)) > + return IRQ_NONE; > + > if (src & MMA8452_INT_DRDY) { > iio_trigger_poll_chained(indio_dev->trig); > ret = IRQ_HANDLED; > } > > - if ((src & MMA8452_INT_TRANS && > - chip->ev_src == MMA8452_TRANSIENT_SRC) || > - (src & MMA8452_INT_FF_MT && > - chip->ev_src == MMA8452_FF_MT_SRC)) { > + if (src & MMA8452_INT_FF_MT) { > + if (mma8452_freefall_mode_enabled(data)) { > + s64 ts = iio_get_time_ns(indio_dev); > + > + iio_push_event(indio_dev, > + IIO_MOD_EVENT_CODE(IIO_ACCEL, 0, > + IIO_MOD_X_AND_Y_AND_Z, > + IIO_EV_TYPE_MAG, > + IIO_EV_DIR_FALLING), > + ts); > + } > + ret = IRQ_HANDLED; > + } > + > + if (src & MMA8452_INT_TRANS) { > mma8452_transient_interrupt(indio_dev); > ret = IRQ_HANDLED; > } > @@ -1222,96 +1255,36 @@ static const struct mma_chip_info mma_chip_info_table[] = { > * g * N * 1000000 / 2048 for N = 2, 4, 8 and g=9.80665 > */ > .mma_scales = { {0, 2394}, {0, 4788}, {0, 9577} }, > - .ev_cfg = MMA8452_TRANSIENT_CFG, > - .ev_cfg_ele = MMA8452_TRANSIENT_CFG_ELE, > - .ev_cfg_chan_shift = 1, > - .ev_src = MMA8452_TRANSIENT_SRC, > - .ev_src_xe = MMA8452_TRANSIENT_SRC_XTRANSE, > - .ev_src_ye = MMA8452_TRANSIENT_SRC_YTRANSE, > - .ev_src_ze = MMA8452_TRANSIENT_SRC_ZTRANSE, > - .ev_ths = MMA8452_TRANSIENT_THS, > - .ev_ths_mask = MMA8452_TRANSIENT_THS_MASK, > - .ev_count = MMA8452_TRANSIENT_COUNT, > }, > [mma8452] = { > .chip_id = MMA8452_DEVICE_ID, > .channels = mma8452_channels, > .num_channels = ARRAY_SIZE(mma8452_channels), > .mma_scales = { {0, 9577}, {0, 19154}, {0, 38307} }, > - .ev_cfg = MMA8452_TRANSIENT_CFG, > - .ev_cfg_ele = MMA8452_TRANSIENT_CFG_ELE, > - .ev_cfg_chan_shift = 1, > - .ev_src = MMA8452_TRANSIENT_SRC, > - .ev_src_xe = MMA8452_TRANSIENT_SRC_XTRANSE, > - .ev_src_ye = MMA8452_TRANSIENT_SRC_YTRANSE, > - .ev_src_ze = MMA8452_TRANSIENT_SRC_ZTRANSE, > - .ev_ths = MMA8452_TRANSIENT_THS, > - .ev_ths_mask = MMA8452_TRANSIENT_THS_MASK, > - .ev_count = MMA8452_TRANSIENT_COUNT, > }, > [mma8453] = { > .chip_id = MMA8453_DEVICE_ID, > .channels = mma8453_channels, > .num_channels = ARRAY_SIZE(mma8453_channels), > .mma_scales = { {0, 38307}, {0, 76614}, {0, 153228} }, > - .ev_cfg = MMA8452_TRANSIENT_CFG, > - .ev_cfg_ele = MMA8452_TRANSIENT_CFG_ELE, > - .ev_cfg_chan_shift = 1, > - .ev_src = MMA8452_TRANSIENT_SRC, > - .ev_src_xe = MMA8452_TRANSIENT_SRC_XTRANSE, > - .ev_src_ye = MMA8452_TRANSIENT_SRC_YTRANSE, > - .ev_src_ze = MMA8452_TRANSIENT_SRC_ZTRANSE, > - .ev_ths = MMA8452_TRANSIENT_THS, > - .ev_ths_mask = MMA8452_TRANSIENT_THS_MASK, > - .ev_count = MMA8452_TRANSIENT_COUNT, > }, > [mma8652] = { > .chip_id = MMA8652_DEVICE_ID, > .channels = mma8652_channels, > .num_channels = ARRAY_SIZE(mma8652_channels), > .mma_scales = { {0, 9577}, {0, 19154}, {0, 38307} }, > - .ev_cfg = MMA8452_FF_MT_CFG, > - .ev_cfg_ele = MMA8452_FF_MT_CFG_ELE, > - .ev_cfg_chan_shift = 3, > - .ev_src = MMA8452_FF_MT_SRC, > - .ev_src_xe = MMA8452_FF_MT_SRC_XHE, > - .ev_src_ye = MMA8452_FF_MT_SRC_YHE, > - .ev_src_ze = MMA8452_FF_MT_SRC_ZHE, > - .ev_ths = MMA8452_FF_MT_THS, > - .ev_ths_mask = MMA8452_FF_MT_THS_MASK, > - .ev_count = MMA8452_FF_MT_COUNT, > }, > [mma8653] = { > .chip_id = MMA8653_DEVICE_ID, > .channels = mma8653_channels, > .num_channels = ARRAY_SIZE(mma8653_channels), > .mma_scales = { {0, 38307}, {0, 76614}, {0, 153228} }, > - .ev_cfg = MMA8452_FF_MT_CFG, > - .ev_cfg_ele = MMA8452_FF_MT_CFG_ELE, > - .ev_cfg_chan_shift = 3, > - .ev_src = MMA8452_FF_MT_SRC, > - .ev_src_xe = MMA8452_FF_MT_SRC_XHE, > - .ev_src_ye = MMA8452_FF_MT_SRC_YHE, > - .ev_src_ze = MMA8452_FF_MT_SRC_ZHE, > - .ev_ths = MMA8452_FF_MT_THS, > - .ev_ths_mask = MMA8452_FF_MT_THS_MASK, > - .ev_count = MMA8452_FF_MT_COUNT, > }, > [fxls8471] = { > .chip_id = FXLS8471_DEVICE_ID, > .channels = mma8451_channels, > .num_channels = ARRAY_SIZE(mma8451_channels), > .mma_scales = { {0, 2394}, {0, 4788}, {0, 9577} }, > - .ev_cfg = MMA8452_TRANSIENT_CFG, > - .ev_cfg_ele = MMA8452_TRANSIENT_CFG_ELE, > - .ev_cfg_chan_shift = 1, > - .ev_src = MMA8452_TRANSIENT_SRC, > - .ev_src_xe = MMA8452_TRANSIENT_SRC_XTRANSE, > - .ev_src_ye = MMA8452_TRANSIENT_SRC_YTRANSE, > - .ev_src_ze = MMA8452_TRANSIENT_SRC_ZTRANSE, > - .ev_ths = MMA8452_TRANSIENT_THS, > - .ev_ths_mask = MMA8452_TRANSIENT_THS_MASK, > - .ev_count = MMA8452_TRANSIENT_COUNT, > }, > }; > > -- Peter Meerwald-Stadler Mobile: +43 664 24 44 418 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] iio: accel: mma8452: Bugfix to enbale and allow different events to work parallely. 2017-08-16 13:12 ` Peter Meerwald-Stadler @ 2017-08-17 0:39 ` Harinath Nampally 2017-08-17 11:24 ` Martin Kepplinger 0 siblings, 1 reply; 7+ messages in thread From: Harinath Nampally @ 2017-08-17 0:39 UTC (permalink / raw) To: Peter Meerwald-Stadler Cc: jic23, knaack.h, lars, gregkh, linux-iio, linux-kernel, amsfield22, martink > Hello, > > fixes are always welcome, some comments regarding the patch Thanks for the review. > > in subject: typo "enable" Will fix it. >> This driver supports multiple devices like mma8653, mma8652, mma8452, mma8453 and >> fxls8471. Almost all these devices have more than one event. Current driver design >> hardcodes the event specific information, so only one event can be supported by this >> driver and current design doesn't have the flexibility to add more events. > >> This patch fixes by detaching the event related information from chip_info struct, >> and based on channel type and event direction the corresponding event configuration registers >> are picked dynamically. Hence multiple events can be handled in read/write callbacks. > which chip can have which event(s)? I am planning to add 'supported events' field in struct mma_chip_info which indicates which chip can have which events. During initialization in 'mma_chip_info_table' would set this 'supported events' field for each chip. But I wonder should I add those changes as part of this patch? > >> Changes are thoroughly tested on fxls8471 device on imx6UL Eval board using iio_event_monitor user space program. >> >> After this fix both Freefall and Transient events are handled by the driver without any conflicts. > thanks, p. > >> Signed-off-by: Harinath Nampally <harinath922@gmail.com> >> --- >> drivers/iio/accel/mma8452.c | 309 ++++++++++++++++++++------------------------ >> 1 file changed, 141 insertions(+), 168 deletions(-) >> >> diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c >> index eb6e3dc..1b83e52 100644 >> --- a/drivers/iio/accel/mma8452.c >> +++ b/drivers/iio/accel/mma8452.c >> @@ -59,7 +59,9 @@ >> #define MMA8452_FF_MT_THS 0x17 >> #define MMA8452_FF_MT_THS_MASK 0x7f >> #define MMA8452_FF_MT_COUNT 0x18 >> +#define MMA8452_FF_MT_CHAN_SHIFT 3 >> #define MMA8452_TRANSIENT_CFG 0x1d >> +#define MMA8452_TRANSIENT_CFG_CHAN(chan) BIT(chan + 1) >> #define MMA8452_TRANSIENT_CFG_HPF_BYP BIT(0) >> #define MMA8452_TRANSIENT_CFG_ELE BIT(4) >> #define MMA8452_TRANSIENT_SRC 0x1e >> @@ -69,6 +71,7 @@ >> #define MMA8452_TRANSIENT_THS 0x1f >> #define MMA8452_TRANSIENT_THS_MASK GENMASK(6, 0) >> #define MMA8452_TRANSIENT_COUNT 0x20 >> +#define MMA8452_TRANSIENT_CHAN_SHIFT 1 >> #define MMA8452_CTRL_REG1 0x2a >> #define MMA8452_CTRL_ACTIVE BIT(0) >> #define MMA8452_CTRL_DR_MASK GENMASK(5, 3) >> @@ -107,6 +110,26 @@ struct mma8452_data { >> const struct mma_chip_info *chip_info; >> }; >> >> + /** >> + * struct mma8452_event_regs - chip specific data related to events >> + * @ev_cfg: event config register address >> + * @ev_src: event source register address >> + * @ev_ths: event threshold register address >> + * @ev_ths_mask: mask for the threshold value >> + * @ev_count: event count (period) register address >> + * >> + * Since not all chips supported by the driver support comparing high pass >> + * filtered data for events (interrupts), different interrupt sources are >> + * used for different chips and the relevant registers are included here. >> + */ >> +struct mma8452_event_regs { >> + u8 ev_cfg; >> + u8 ev_src; >> + u8 ev_ths; >> + u8 ev_ths_mask; >> + u8 ev_count; >> +}; >> + >> /** >> * struct mma_chip_info - chip specific data >> * @chip_id: WHO_AM_I register's value >> @@ -116,40 +139,12 @@ struct mma8452_data { >> * @mma_scales: scale factors for converting register values >> * to m/s^2; 3 modes: 2g, 4g, 8g; 2 integers >> * per mode: m/s^2 and micro m/s^2 >> - * @ev_cfg: event config register address >> - * @ev_cfg_ele: latch bit in event config register >> - * @ev_cfg_chan_shift: number of the bit to enable events in X >> - * direction; in event config register >> - * @ev_src: event source register address >> - * @ev_src_xe: bit in event source register that indicates >> - * an event in X direction >> - * @ev_src_ye: bit in event source register that indicates >> - * an event in Y direction >> - * @ev_src_ze: bit in event source register that indicates >> - * an event in Z direction >> - * @ev_ths: event threshold register address >> - * @ev_ths_mask: mask for the threshold value >> - * @ev_count: event count (period) register address >> - * >> - * Since not all chips supported by the driver support comparing high pass >> - * filtered data for events (interrupts), different interrupt sources are >> - * used for different chips and the relevant registers are included here. >> */ >> struct mma_chip_info { >> u8 chip_id; >> const struct iio_chan_spec *channels; >> int num_channels; >> const int mma_scales[3][2]; >> - u8 ev_cfg; >> - u8 ev_cfg_ele; >> - u8 ev_cfg_chan_shift; >> - u8 ev_src; >> - u8 ev_src_xe; >> - u8 ev_src_ye; >> - u8 ev_src_ze; >> - u8 ev_ths; >> - u8 ev_ths_mask; >> - u8 ev_count; >> }; >> >> enum { >> @@ -602,9 +597,8 @@ static int mma8452_set_power_mode(struct mma8452_data *data, u8 mode) >> static int mma8452_freefall_mode_enabled(struct mma8452_data *data) >> { >> int val; >> - const struct mma_chip_info *chip = data->chip_info; >> >> - val = i2c_smbus_read_byte_data(data->client, chip->ev_cfg); >> + val = i2c_smbus_read_byte_data(data->client, MMA8452_FF_MT_CFG); >> if (val < 0) >> return val; >> >> @@ -614,29 +608,28 @@ static int mma8452_freefall_mode_enabled(struct mma8452_data *data) >> static int mma8452_set_freefall_mode(struct mma8452_data *data, bool state) >> { >> int val; >> - const struct mma_chip_info *chip = data->chip_info; >> >> if ((state && mma8452_freefall_mode_enabled(data)) || >> (!state && !(mma8452_freefall_mode_enabled(data)))) >> return 0; >> >> - val = i2c_smbus_read_byte_data(data->client, chip->ev_cfg); >> + val = i2c_smbus_read_byte_data(data->client, MMA8452_FF_MT_CFG); >> if (val < 0) >> return val; >> >> if (state) { >> - val |= BIT(idx_x + chip->ev_cfg_chan_shift); >> - val |= BIT(idx_y + chip->ev_cfg_chan_shift); >> - val |= BIT(idx_z + chip->ev_cfg_chan_shift); >> + val |= BIT(idx_x + MMA8452_FF_MT_CHAN_SHIFT); >> + val |= BIT(idx_y + MMA8452_FF_MT_CHAN_SHIFT); >> + val |= BIT(idx_z + MMA8452_FF_MT_CHAN_SHIFT); >> val &= ~MMA8452_FF_MT_CFG_OAE; >> } else { >> - val &= ~BIT(idx_x + chip->ev_cfg_chan_shift); >> - val &= ~BIT(idx_y + chip->ev_cfg_chan_shift); >> - val &= ~BIT(idx_z + chip->ev_cfg_chan_shift); >> + val &= ~BIT(idx_x + MMA8452_FF_MT_CHAN_SHIFT); >> + val &= ~BIT(idx_y + MMA8452_FF_MT_CHAN_SHIFT); >> + val &= ~BIT(idx_z + MMA8452_FF_MT_CHAN_SHIFT); >> val |= MMA8452_FF_MT_CFG_OAE; >> } >> >> - return mma8452_change_config(data, chip->ev_cfg, val); >> + return mma8452_change_config(data, MMA8452_FF_MT_CFG, val); >> } >> >> static int mma8452_set_hp_filter_frequency(struct mma8452_data *data, >> @@ -740,6 +733,39 @@ static int mma8452_write_raw(struct iio_dev *indio_dev, >> return ret; >> } >> >> +static int mma8452_get_event_regs(const struct iio_chan_spec *chan, >> + enum iio_event_direction dir, >> + struct mma8452_event_regs *ev_regs >> + ) >> +{ >> + if (!chan) >> + return -EINVAL; >> + switch (chan->type) { >> + case IIO_ACCEL: >> + switch (dir) { >> + case IIO_EV_DIR_FALLING: >> + ev_regs->ev_cfg = MMA8452_FF_MT_CFG; >> + ev_regs->ev_src = MMA8452_FF_MT_SRC; >> + ev_regs->ev_ths = MMA8452_FF_MT_THS; >> + ev_regs->ev_ths_mask = MMA8452_FF_MT_THS_MASK; >> + ev_regs->ev_count = MMA8452_FF_MT_COUNT; >> + break; >> + case IIO_EV_DIR_RISING: >> + ev_regs->ev_cfg = MMA8452_TRANSIENT_CFG; >> + ev_regs->ev_src = MMA8452_TRANSIENT_SRC; >> + ev_regs->ev_ths = MMA8452_TRANSIENT_THS; > ev_ths_mask is not set here > Good catch! Will fix it. >> + ev_regs->ev_count = MMA8452_TRANSIENT_COUNT; >> + break; >> + default: >> + return -EINVAL; >> + } >> + break; >> + default: >> + return -EINVAL; >> + } >> + return 0; > could replace 'breaks' with return 0 to save some lines and simplify > control flow Sure. >> +} >> + >> static int mma8452_read_thresh(struct iio_dev *indio_dev, >> const struct iio_chan_spec *chan, >> enum iio_event_type type, >> @@ -749,21 +775,23 @@ static int mma8452_read_thresh(struct iio_dev *indio_dev, >> { >> struct mma8452_data *data = iio_priv(indio_dev); >> int ret, us, power_mode; >> + struct mma8452_event_regs ev_regs; >> >> + ret = mma8452_get_event_regs(chan, dir, &ev_regs); >> + if (ret) >> + return ret; >> switch (info) { >> case IIO_EV_INFO_VALUE: >> - ret = i2c_smbus_read_byte_data(data->client, >> - data->chip_info->ev_ths); >> + ret = i2c_smbus_read_byte_data(data->client, ev_regs.ev_ths); >> if (ret < 0) >> return ret; >> >> - *val = ret & data->chip_info->ev_ths_mask; >> + *val = ret & ev_regs.ev_ths_mask; >> >> return IIO_VAL_INT; >> >> case IIO_EV_INFO_PERIOD: >> - ret = i2c_smbus_read_byte_data(data->client, >> - data->chip_info->ev_count); >> + ret = i2c_smbus_read_byte_data(data->client, ev_regs.ev_count); >> if (ret < 0) >> return ret; >> >> @@ -809,14 +837,18 @@ static int mma8452_write_thresh(struct iio_dev *indio_dev, >> { >> struct mma8452_data *data = iio_priv(indio_dev); >> int ret, reg, steps; >> + struct mma8452_event_regs ev_regs; >> + >> + ret = mma8452_get_event_regs(chan, dir, &ev_regs); >> + if (ret) >> + return ret; >> >> switch (info) { >> case IIO_EV_INFO_VALUE: >> - if (val < 0 || val > MMA8452_TRANSIENT_THS_MASK) >> + if (val < 0 || val > ev_regs.ev_ths_mask) >> return -EINVAL; >> >> - return mma8452_change_config(data, data->chip_info->ev_ths, >> - val); >> + return mma8452_change_config(data, ev_regs.ev_ths, val); >> >> case IIO_EV_INFO_PERIOD: >> ret = mma8452_get_power_mode(data); >> @@ -830,8 +862,7 @@ static int mma8452_write_thresh(struct iio_dev *indio_dev, >> if (steps < 0 || steps > 0xff) >> return -EINVAL; >> >> - return mma8452_change_config(data, data->chip_info->ev_count, >> - steps); >> + return mma8452_change_config(data, ev_regs.ev_count, steps); >> >> case IIO_EV_INFO_HIGH_PASS_FILTER_3DB: >> reg = i2c_smbus_read_byte_data(data->client, >> @@ -861,23 +892,23 @@ static int mma8452_read_event_config(struct iio_dev *indio_dev, >> enum iio_event_direction dir) >> { >> struct mma8452_data *data = iio_priv(indio_dev); >> - const struct mma_chip_info *chip = data->chip_info; >> int ret; >> >> - switch (dir) { >> - case IIO_EV_DIR_FALLING: >> - return mma8452_freefall_mode_enabled(data); >> - case IIO_EV_DIR_RISING: >> - if (mma8452_freefall_mode_enabled(data)) >> - return 0; >> + switch (chan->type) { >> + case IIO_ACCEL: > this adds a check for chan-type == IIO_ACCESS, so strictly this would be > an unrelated change... Ok will remove it from this patch. >> + switch (dir) { >> + case IIO_EV_DIR_FALLING: >> + return mma8452_freefall_mode_enabled(data); >> + case IIO_EV_DIR_RISING: >> + ret = i2c_smbus_read_byte_data(data->client, MMA8452_TRANSIENT_CFG); >> + if (ret < 0) >> + return ret; >> >> - ret = i2c_smbus_read_byte_data(data->client, >> - data->chip_info->ev_cfg); >> - if (ret < 0) >> - return ret; >> + return !!(ret & MMA8452_TRANSIENT_CFG_CHAN(chan->scan_index)); >> >> - return !!(ret & BIT(chan->scan_index + >> - chip->ev_cfg_chan_shift)); >> + default: >> + return -EINVAL; >> + } >> default: >> return -EINVAL; >> } >> @@ -890,39 +921,33 @@ static int mma8452_write_event_config(struct iio_dev *indio_dev, >> int state) >> { >> struct mma8452_data *data = iio_priv(indio_dev); >> - const struct mma_chip_info *chip = data->chip_info; >> int val, ret; >> >> ret = mma8452_set_runtime_pm_state(data->client, state); >> if (ret) >> return ret; >> >> - switch (dir) { >> - case IIO_EV_DIR_FALLING: >> - return mma8452_set_freefall_mode(data, state); >> - case IIO_EV_DIR_RISING: >> - val = i2c_smbus_read_byte_data(data->client, chip->ev_cfg); >> - if (val < 0) >> - return val; >> - >> - if (state) { >> - if (mma8452_freefall_mode_enabled(data)) { >> - val &= ~BIT(idx_x + chip->ev_cfg_chan_shift); >> - val &= ~BIT(idx_y + chip->ev_cfg_chan_shift); >> - val &= ~BIT(idx_z + chip->ev_cfg_chan_shift); >> - val |= MMA8452_FF_MT_CFG_OAE; >> - } >> - val |= BIT(chan->scan_index + chip->ev_cfg_chan_shift); >> - } else { >> - if (mma8452_freefall_mode_enabled(data)) >> - return 0; >> - >> - val &= ~BIT(chan->scan_index + chip->ev_cfg_chan_shift); >> + switch (chan->type) { >> + case IIO_ACCEL: >> + switch (dir) { >> + case IIO_EV_DIR_FALLING: >> + return mma8452_set_freefall_mode(data, state); >> + case IIO_EV_DIR_RISING: >> + val = i2c_smbus_read_byte_data(data->client, MMA8452_TRANSIENT_CFG); >> + if (val < 0) >> + return val; >> + >> + if (state) >> + val |= MMA8452_TRANSIENT_CFG_CHAN(chan->scan_index); >> + else >> + val &= ~MMA8452_TRANSIENT_CFG_CHAN(chan->scan_index); >> + >> + val |= MMA8452_TRANSIENT_CFG_ELE; >> + >> + return mma8452_change_config(data, MMA8452_TRANSIENT_CFG, val); >> + default: >> + return -EINVAL; >> } >> - >> - val |= chip->ev_cfg_ele; >> - >> - return mma8452_change_config(data, chip->ev_cfg, val); >> default: >> return -EINVAL; >> } >> @@ -934,35 +959,25 @@ static void mma8452_transient_interrupt(struct iio_dev *indio_dev) >> s64 ts = iio_get_time_ns(indio_dev); >> int src; >> >> - src = i2c_smbus_read_byte_data(data->client, data->chip_info->ev_src); >> + src = i2c_smbus_read_byte_data(data->client, MMA8452_TRANSIENT_SRC); >> if (src < 0) >> return; >> >> - if (mma8452_freefall_mode_enabled(data)) { >> - iio_push_event(indio_dev, >> - IIO_MOD_EVENT_CODE(IIO_ACCEL, 0, >> - IIO_MOD_X_AND_Y_AND_Z, >> - IIO_EV_TYPE_MAG, >> - IIO_EV_DIR_FALLING), >> - ts); >> - return; >> - } >> - >> - if (src & data->chip_info->ev_src_xe) >> + if (src & MMA8452_TRANSIENT_SRC_XTRANSE) >> iio_push_event(indio_dev, >> IIO_MOD_EVENT_CODE(IIO_ACCEL, 0, IIO_MOD_X, >> IIO_EV_TYPE_MAG, >> IIO_EV_DIR_RISING), >> ts); >> >> - if (src & data->chip_info->ev_src_ye) >> + if (src & MMA8452_TRANSIENT_SRC_YTRANSE) >> iio_push_event(indio_dev, >> IIO_MOD_EVENT_CODE(IIO_ACCEL, 0, IIO_MOD_Y, >> IIO_EV_TYPE_MAG, >> IIO_EV_DIR_RISING), >> ts); >> >> - if (src & data->chip_info->ev_src_ze) >> + if (src & MMA8452_TRANSIENT_SRC_ZTRANSE) >> iio_push_event(indio_dev, >> IIO_MOD_EVENT_CODE(IIO_ACCEL, 0, IIO_MOD_Z, >> IIO_EV_TYPE_MAG, >> @@ -974,23 +989,41 @@ static irqreturn_t mma8452_interrupt(int irq, void *p) >> { >> struct iio_dev *indio_dev = p; >> struct mma8452_data *data = iio_priv(indio_dev); >> - const struct mma_chip_info *chip = data->chip_info; >> int ret = IRQ_NONE; >> - int src; >> + int src, enabled_interrupts; >> >> src = i2c_smbus_read_byte_data(data->client, MMA8452_INT_SRC); >> if (src < 0) >> return IRQ_NONE; >> >> + enabled_interrupts = i2c_smbus_read_byte_data(data->client, >> + MMA8452_CTRL_REG4); >> + if (enabled_interrupts < 0) >> + return enabled_interrupts; >> + >> + if (!(src & enabled_interrupts)) >> + return IRQ_NONE; >> + >> if (src & MMA8452_INT_DRDY) { >> iio_trigger_poll_chained(indio_dev->trig); >> ret = IRQ_HANDLED; >> } >> >> - if ((src & MMA8452_INT_TRANS && >> - chip->ev_src == MMA8452_TRANSIENT_SRC) || >> - (src & MMA8452_INT_FF_MT && >> - chip->ev_src == MMA8452_FF_MT_SRC)) { >> + if (src & MMA8452_INT_FF_MT) { >> + if (mma8452_freefall_mode_enabled(data)) { >> + s64 ts = iio_get_time_ns(indio_dev); >> + >> + iio_push_event(indio_dev, >> + IIO_MOD_EVENT_CODE(IIO_ACCEL, 0, >> + IIO_MOD_X_AND_Y_AND_Z, >> + IIO_EV_TYPE_MAG, >> + IIO_EV_DIR_FALLING), >> + ts); >> + } >> + ret = IRQ_HANDLED; >> + } >> + >> + if (src & MMA8452_INT_TRANS) { >> mma8452_transient_interrupt(indio_dev); >> ret = IRQ_HANDLED; >> } >> @@ -1222,96 +1255,36 @@ static const struct mma_chip_info mma_chip_info_table[] = { >> * g * N * 1000000 / 2048 for N = 2, 4, 8 and g=9.80665 >> */ >> .mma_scales = { {0, 2394}, {0, 4788}, {0, 9577} }, >> - .ev_cfg = MMA8452_TRANSIENT_CFG, >> - .ev_cfg_ele = MMA8452_TRANSIENT_CFG_ELE, >> - .ev_cfg_chan_shift = 1, >> - .ev_src = MMA8452_TRANSIENT_SRC, >> - .ev_src_xe = MMA8452_TRANSIENT_SRC_XTRANSE, >> - .ev_src_ye = MMA8452_TRANSIENT_SRC_YTRANSE, >> - .ev_src_ze = MMA8452_TRANSIENT_SRC_ZTRANSE, >> - .ev_ths = MMA8452_TRANSIENT_THS, >> - .ev_ths_mask = MMA8452_TRANSIENT_THS_MASK, >> - .ev_count = MMA8452_TRANSIENT_COUNT, >> }, >> [mma8452] = { >> .chip_id = MMA8452_DEVICE_ID, >> .channels = mma8452_channels, >> .num_channels = ARRAY_SIZE(mma8452_channels), >> .mma_scales = { {0, 9577}, {0, 19154}, {0, 38307} }, >> - .ev_cfg = MMA8452_TRANSIENT_CFG, >> - .ev_cfg_ele = MMA8452_TRANSIENT_CFG_ELE, >> - .ev_cfg_chan_shift = 1, >> - .ev_src = MMA8452_TRANSIENT_SRC, >> - .ev_src_xe = MMA8452_TRANSIENT_SRC_XTRANSE, >> - .ev_src_ye = MMA8452_TRANSIENT_SRC_YTRANSE, >> - .ev_src_ze = MMA8452_TRANSIENT_SRC_ZTRANSE, >> - .ev_ths = MMA8452_TRANSIENT_THS, >> - .ev_ths_mask = MMA8452_TRANSIENT_THS_MASK, >> - .ev_count = MMA8452_TRANSIENT_COUNT, >> }, >> [mma8453] = { >> .chip_id = MMA8453_DEVICE_ID, >> .channels = mma8453_channels, >> .num_channels = ARRAY_SIZE(mma8453_channels), >> .mma_scales = { {0, 38307}, {0, 76614}, {0, 153228} }, >> - .ev_cfg = MMA8452_TRANSIENT_CFG, >> - .ev_cfg_ele = MMA8452_TRANSIENT_CFG_ELE, >> - .ev_cfg_chan_shift = 1, >> - .ev_src = MMA8452_TRANSIENT_SRC, >> - .ev_src_xe = MMA8452_TRANSIENT_SRC_XTRANSE, >> - .ev_src_ye = MMA8452_TRANSIENT_SRC_YTRANSE, >> - .ev_src_ze = MMA8452_TRANSIENT_SRC_ZTRANSE, >> - .ev_ths = MMA8452_TRANSIENT_THS, >> - .ev_ths_mask = MMA8452_TRANSIENT_THS_MASK, >> - .ev_count = MMA8452_TRANSIENT_COUNT, >> }, >> [mma8652] = { >> .chip_id = MMA8652_DEVICE_ID, >> .channels = mma8652_channels, >> .num_channels = ARRAY_SIZE(mma8652_channels), >> .mma_scales = { {0, 9577}, {0, 19154}, {0, 38307} }, >> - .ev_cfg = MMA8452_FF_MT_CFG, >> - .ev_cfg_ele = MMA8452_FF_MT_CFG_ELE, >> - .ev_cfg_chan_shift = 3, >> - .ev_src = MMA8452_FF_MT_SRC, >> - .ev_src_xe = MMA8452_FF_MT_SRC_XHE, >> - .ev_src_ye = MMA8452_FF_MT_SRC_YHE, >> - .ev_src_ze = MMA8452_FF_MT_SRC_ZHE, >> - .ev_ths = MMA8452_FF_MT_THS, >> - .ev_ths_mask = MMA8452_FF_MT_THS_MASK, >> - .ev_count = MMA8452_FF_MT_COUNT, >> }, >> [mma8653] = { >> .chip_id = MMA8653_DEVICE_ID, >> .channels = mma8653_channels, >> .num_channels = ARRAY_SIZE(mma8653_channels), >> .mma_scales = { {0, 38307}, {0, 76614}, {0, 153228} }, >> - .ev_cfg = MMA8452_FF_MT_CFG, >> - .ev_cfg_ele = MMA8452_FF_MT_CFG_ELE, >> - .ev_cfg_chan_shift = 3, >> - .ev_src = MMA8452_FF_MT_SRC, >> - .ev_src_xe = MMA8452_FF_MT_SRC_XHE, >> - .ev_src_ye = MMA8452_FF_MT_SRC_YHE, >> - .ev_src_ze = MMA8452_FF_MT_SRC_ZHE, >> - .ev_ths = MMA8452_FF_MT_THS, >> - .ev_ths_mask = MMA8452_FF_MT_THS_MASK, >> - .ev_count = MMA8452_FF_MT_COUNT, >> }, >> [fxls8471] = { >> .chip_id = FXLS8471_DEVICE_ID, >> .channels = mma8451_channels, >> .num_channels = ARRAY_SIZE(mma8451_channels), >> .mma_scales = { {0, 2394}, {0, 4788}, {0, 9577} }, >> - .ev_cfg = MMA8452_TRANSIENT_CFG, >> - .ev_cfg_ele = MMA8452_TRANSIENT_CFG_ELE, >> - .ev_cfg_chan_shift = 1, >> - .ev_src = MMA8452_TRANSIENT_SRC, >> - .ev_src_xe = MMA8452_TRANSIENT_SRC_XTRANSE, >> - .ev_src_ye = MMA8452_TRANSIENT_SRC_YTRANSE, >> - .ev_src_ze = MMA8452_TRANSIENT_SRC_ZTRANSE, >> - .ev_ths = MMA8452_TRANSIENT_THS, >> - .ev_ths_mask = MMA8452_TRANSIENT_THS_MASK, >> - .ev_count = MMA8452_TRANSIENT_COUNT, >> }, >> }; >> >> On 08/16/2017 09:12 AM, Peter Meerwald-Stadler wrote: > Hello, > > fixes are always welcome, some comments regarding the patch > > in subject: typo "enable" > >> This driver supports multiple devices like mma8653, mma8652, mma8452, mma8453 and >> fxls8471. Almost all these devices have more than one event. Current driver design >> hardcodes the event specific information, so only one event can be supported by this >> driver and current design doesn't have the flexibility to add more events. > >> This patch fixes by detaching the event related information from chip_info struct, >> and based on channel type and event direction the corresponding event configuration registers >> are picked dynamically. Hence multiple events can be handled in read/write callbacks. > which chip can have which event(s)? > >> Changes are thoroughly tested on fxls8471 device on imx6UL Eval board using iio_event_monitor user space program. >> >> After this fix both Freefall and Transient events are handled by the driver without any conflicts. > thanks, p. > >> Signed-off-by: Harinath Nampally <harinath922@gmail.com> >> --- >> drivers/iio/accel/mma8452.c | 309 ++++++++++++++++++++------------------------ >> 1 file changed, 141 insertions(+), 168 deletions(-) >> >> diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c >> index eb6e3dc..1b83e52 100644 >> --- a/drivers/iio/accel/mma8452.c >> +++ b/drivers/iio/accel/mma8452.c >> @@ -59,7 +59,9 @@ >> #define MMA8452_FF_MT_THS 0x17 >> #define MMA8452_FF_MT_THS_MASK 0x7f >> #define MMA8452_FF_MT_COUNT 0x18 >> +#define MMA8452_FF_MT_CHAN_SHIFT 3 >> #define MMA8452_TRANSIENT_CFG 0x1d >> +#define MMA8452_TRANSIENT_CFG_CHAN(chan) BIT(chan + 1) >> #define MMA8452_TRANSIENT_CFG_HPF_BYP BIT(0) >> #define MMA8452_TRANSIENT_CFG_ELE BIT(4) >> #define MMA8452_TRANSIENT_SRC 0x1e >> @@ -69,6 +71,7 @@ >> #define MMA8452_TRANSIENT_THS 0x1f >> #define MMA8452_TRANSIENT_THS_MASK GENMASK(6, 0) >> #define MMA8452_TRANSIENT_COUNT 0x20 >> +#define MMA8452_TRANSIENT_CHAN_SHIFT 1 >> #define MMA8452_CTRL_REG1 0x2a >> #define MMA8452_CTRL_ACTIVE BIT(0) >> #define MMA8452_CTRL_DR_MASK GENMASK(5, 3) >> @@ -107,6 +110,26 @@ struct mma8452_data { >> const struct mma_chip_info *chip_info; >> }; >> >> + /** >> + * struct mma8452_event_regs - chip specific data related to events >> + * @ev_cfg: event config register address >> + * @ev_src: event source register address >> + * @ev_ths: event threshold register address >> + * @ev_ths_mask: mask for the threshold value >> + * @ev_count: event count (period) register address >> + * >> + * Since not all chips supported by the driver support comparing high pass >> + * filtered data for events (interrupts), different interrupt sources are >> + * used for different chips and the relevant registers are included here. >> + */ >> +struct mma8452_event_regs { >> + u8 ev_cfg; >> + u8 ev_src; >> + u8 ev_ths; >> + u8 ev_ths_mask; >> + u8 ev_count; >> +}; >> + >> /** >> * struct mma_chip_info - chip specific data >> * @chip_id: WHO_AM_I register's value >> @@ -116,40 +139,12 @@ struct mma8452_data { >> * @mma_scales: scale factors for converting register values >> * to m/s^2; 3 modes: 2g, 4g, 8g; 2 integers >> * per mode: m/s^2 and micro m/s^2 >> - * @ev_cfg: event config register address >> - * @ev_cfg_ele: latch bit in event config register >> - * @ev_cfg_chan_shift: number of the bit to enable events in X >> - * direction; in event config register >> - * @ev_src: event source register address >> - * @ev_src_xe: bit in event source register that indicates >> - * an event in X direction >> - * @ev_src_ye: bit in event source register that indicates >> - * an event in Y direction >> - * @ev_src_ze: bit in event source register that indicates >> - * an event in Z direction >> - * @ev_ths: event threshold register address >> - * @ev_ths_mask: mask for the threshold value >> - * @ev_count: event count (period) register address >> - * >> - * Since not all chips supported by the driver support comparing high pass >> - * filtered data for events (interrupts), different interrupt sources are >> - * used for different chips and the relevant registers are included here. >> */ >> struct mma_chip_info { >> u8 chip_id; >> const struct iio_chan_spec *channels; >> int num_channels; >> const int mma_scales[3][2]; >> - u8 ev_cfg; >> - u8 ev_cfg_ele; >> - u8 ev_cfg_chan_shift; >> - u8 ev_src; >> - u8 ev_src_xe; >> - u8 ev_src_ye; >> - u8 ev_src_ze; >> - u8 ev_ths; >> - u8 ev_ths_mask; >> - u8 ev_count; >> }; >> >> enum { >> @@ -602,9 +597,8 @@ static int mma8452_set_power_mode(struct mma8452_data *data, u8 mode) >> static int mma8452_freefall_mode_enabled(struct mma8452_data *data) >> { >> int val; >> - const struct mma_chip_info *chip = data->chip_info; >> >> - val = i2c_smbus_read_byte_data(data->client, chip->ev_cfg); >> + val = i2c_smbus_read_byte_data(data->client, MMA8452_FF_MT_CFG); >> if (val < 0) >> return val; >> >> @@ -614,29 +608,28 @@ static int mma8452_freefall_mode_enabled(struct mma8452_data *data) >> static int mma8452_set_freefall_mode(struct mma8452_data *data, bool state) >> { >> int val; >> - const struct mma_chip_info *chip = data->chip_info; >> >> if ((state && mma8452_freefall_mode_enabled(data)) || >> (!state && !(mma8452_freefall_mode_enabled(data)))) >> return 0; >> >> - val = i2c_smbus_read_byte_data(data->client, chip->ev_cfg); >> + val = i2c_smbus_read_byte_data(data->client, MMA8452_FF_MT_CFG); >> if (val < 0) >> return val; >> >> if (state) { >> - val |= BIT(idx_x + chip->ev_cfg_chan_shift); >> - val |= BIT(idx_y + chip->ev_cfg_chan_shift); >> - val |= BIT(idx_z + chip->ev_cfg_chan_shift); >> + val |= BIT(idx_x + MMA8452_FF_MT_CHAN_SHIFT); >> + val |= BIT(idx_y + MMA8452_FF_MT_CHAN_SHIFT); >> + val |= BIT(idx_z + MMA8452_FF_MT_CHAN_SHIFT); >> val &= ~MMA8452_FF_MT_CFG_OAE; >> } else { >> - val &= ~BIT(idx_x + chip->ev_cfg_chan_shift); >> - val &= ~BIT(idx_y + chip->ev_cfg_chan_shift); >> - val &= ~BIT(idx_z + chip->ev_cfg_chan_shift); >> + val &= ~BIT(idx_x + MMA8452_FF_MT_CHAN_SHIFT); >> + val &= ~BIT(idx_y + MMA8452_FF_MT_CHAN_SHIFT); >> + val &= ~BIT(idx_z + MMA8452_FF_MT_CHAN_SHIFT); >> val |= MMA8452_FF_MT_CFG_OAE; >> } >> >> - return mma8452_change_config(data, chip->ev_cfg, val); >> + return mma8452_change_config(data, MMA8452_FF_MT_CFG, val); >> } >> >> static int mma8452_set_hp_filter_frequency(struct mma8452_data *data, >> @@ -740,6 +733,39 @@ static int mma8452_write_raw(struct iio_dev *indio_dev, >> return ret; >> } >> >> +static int mma8452_get_event_regs(const struct iio_chan_spec *chan, >> + enum iio_event_direction dir, >> + struct mma8452_event_regs *ev_regs >> + ) >> +{ >> + if (!chan) >> + return -EINVAL; >> + switch (chan->type) { >> + case IIO_ACCEL: >> + switch (dir) { >> + case IIO_EV_DIR_FALLING: >> + ev_regs->ev_cfg = MMA8452_FF_MT_CFG; >> + ev_regs->ev_src = MMA8452_FF_MT_SRC; >> + ev_regs->ev_ths = MMA8452_FF_MT_THS; >> + ev_regs->ev_ths_mask = MMA8452_FF_MT_THS_MASK; >> + ev_regs->ev_count = MMA8452_FF_MT_COUNT; >> + break; >> + case IIO_EV_DIR_RISING: >> + ev_regs->ev_cfg = MMA8452_TRANSIENT_CFG; >> + ev_regs->ev_src = MMA8452_TRANSIENT_SRC; >> + ev_regs->ev_ths = MMA8452_TRANSIENT_THS; > > ev_ths_mask is not set here > >> + ev_regs->ev_count = MMA8452_TRANSIENT_COUNT; >> + break; >> + default: >> + return -EINVAL; >> + } >> + break; >> + default: >> + return -EINVAL; >> + } >> + return 0; > could replace 'breaks' with return 0 to save some lines and simplify > control flow > >> +} >> + >> static int mma8452_read_thresh(struct iio_dev *indio_dev, >> const struct iio_chan_spec *chan, >> enum iio_event_type type, >> @@ -749,21 +775,23 @@ static int mma8452_read_thresh(struct iio_dev *indio_dev, >> { >> struct mma8452_data *data = iio_priv(indio_dev); >> int ret, us, power_mode; >> + struct mma8452_event_regs ev_regs; >> >> + ret = mma8452_get_event_regs(chan, dir, &ev_regs); >> + if (ret) >> + return ret; >> switch (info) { >> case IIO_EV_INFO_VALUE: >> - ret = i2c_smbus_read_byte_data(data->client, >> - data->chip_info->ev_ths); >> + ret = i2c_smbus_read_byte_data(data->client, ev_regs.ev_ths); >> if (ret < 0) >> return ret; >> >> - *val = ret & data->chip_info->ev_ths_mask; >> + *val = ret & ev_regs.ev_ths_mask; >> >> return IIO_VAL_INT; >> >> case IIO_EV_INFO_PERIOD: >> - ret = i2c_smbus_read_byte_data(data->client, >> - data->chip_info->ev_count); >> + ret = i2c_smbus_read_byte_data(data->client, ev_regs.ev_count); >> if (ret < 0) >> return ret; >> >> @@ -809,14 +837,18 @@ static int mma8452_write_thresh(struct iio_dev *indio_dev, >> { >> struct mma8452_data *data = iio_priv(indio_dev); >> int ret, reg, steps; >> + struct mma8452_event_regs ev_regs; >> + >> + ret = mma8452_get_event_regs(chan, dir, &ev_regs); >> + if (ret) >> + return ret; >> >> switch (info) { >> case IIO_EV_INFO_VALUE: >> - if (val < 0 || val > MMA8452_TRANSIENT_THS_MASK) >> + if (val < 0 || val > ev_regs.ev_ths_mask) >> return -EINVAL; >> >> - return mma8452_change_config(data, data->chip_info->ev_ths, >> - val); >> + return mma8452_change_config(data, ev_regs.ev_ths, val); >> >> case IIO_EV_INFO_PERIOD: >> ret = mma8452_get_power_mode(data); >> @@ -830,8 +862,7 @@ static int mma8452_write_thresh(struct iio_dev *indio_dev, >> if (steps < 0 || steps > 0xff) >> return -EINVAL; >> >> - return mma8452_change_config(data, data->chip_info->ev_count, >> - steps); >> + return mma8452_change_config(data, ev_regs.ev_count, steps); >> >> case IIO_EV_INFO_HIGH_PASS_FILTER_3DB: >> reg = i2c_smbus_read_byte_data(data->client, >> @@ -861,23 +892,23 @@ static int mma8452_read_event_config(struct iio_dev *indio_dev, >> enum iio_event_direction dir) >> { >> struct mma8452_data *data = iio_priv(indio_dev); >> - const struct mma_chip_info *chip = data->chip_info; >> int ret; >> >> - switch (dir) { >> - case IIO_EV_DIR_FALLING: >> - return mma8452_freefall_mode_enabled(data); >> - case IIO_EV_DIR_RISING: >> - if (mma8452_freefall_mode_enabled(data)) >> - return 0; >> + switch (chan->type) { >> + case IIO_ACCEL: > this adds a check for chan-type == IIO_ACCESS, so strictly this would be > an unrelated change... > >> + switch (dir) { >> + case IIO_EV_DIR_FALLING: >> + return mma8452_freefall_mode_enabled(data); >> + case IIO_EV_DIR_RISING: >> + ret = i2c_smbus_read_byte_data(data->client, MMA8452_TRANSIENT_CFG); >> + if (ret < 0) >> + return ret; >> >> - ret = i2c_smbus_read_byte_data(data->client, >> - data->chip_info->ev_cfg); >> - if (ret < 0) >> - return ret; >> + return !!(ret & MMA8452_TRANSIENT_CFG_CHAN(chan->scan_index)); >> >> - return !!(ret & BIT(chan->scan_index + >> - chip->ev_cfg_chan_shift)); >> + default: >> + return -EINVAL; >> + } >> default: >> return -EINVAL; >> } >> @@ -890,39 +921,33 @@ static int mma8452_write_event_config(struct iio_dev *indio_dev, >> int state) >> { >> struct mma8452_data *data = iio_priv(indio_dev); >> - const struct mma_chip_info *chip = data->chip_info; >> int val, ret; >> >> ret = mma8452_set_runtime_pm_state(data->client, state); >> if (ret) >> return ret; >> >> - switch (dir) { >> - case IIO_EV_DIR_FALLING: >> - return mma8452_set_freefall_mode(data, state); >> - case IIO_EV_DIR_RISING: >> - val = i2c_smbus_read_byte_data(data->client, chip->ev_cfg); >> - if (val < 0) >> - return val; >> - >> - if (state) { >> - if (mma8452_freefall_mode_enabled(data)) { >> - val &= ~BIT(idx_x + chip->ev_cfg_chan_shift); >> - val &= ~BIT(idx_y + chip->ev_cfg_chan_shift); >> - val &= ~BIT(idx_z + chip->ev_cfg_chan_shift); >> - val |= MMA8452_FF_MT_CFG_OAE; >> - } >> - val |= BIT(chan->scan_index + chip->ev_cfg_chan_shift); >> - } else { >> - if (mma8452_freefall_mode_enabled(data)) >> - return 0; >> - >> - val &= ~BIT(chan->scan_index + chip->ev_cfg_chan_shift); >> + switch (chan->type) { >> + case IIO_ACCEL: >> + switch (dir) { >> + case IIO_EV_DIR_FALLING: >> + return mma8452_set_freefall_mode(data, state); >> + case IIO_EV_DIR_RISING: >> + val = i2c_smbus_read_byte_data(data->client, MMA8452_TRANSIENT_CFG); >> + if (val < 0) >> + return val; >> + >> + if (state) >> + val |= MMA8452_TRANSIENT_CFG_CHAN(chan->scan_index); >> + else >> + val &= ~MMA8452_TRANSIENT_CFG_CHAN(chan->scan_index); >> + >> + val |= MMA8452_TRANSIENT_CFG_ELE; >> + >> + return mma8452_change_config(data, MMA8452_TRANSIENT_CFG, val); >> + default: >> + return -EINVAL; >> } >> - >> - val |= chip->ev_cfg_ele; >> - >> - return mma8452_change_config(data, chip->ev_cfg, val); >> default: >> return -EINVAL; >> } >> @@ -934,35 +959,25 @@ static void mma8452_transient_interrupt(struct iio_dev *indio_dev) >> s64 ts = iio_get_time_ns(indio_dev); >> int src; >> >> - src = i2c_smbus_read_byte_data(data->client, data->chip_info->ev_src); >> + src = i2c_smbus_read_byte_data(data->client, MMA8452_TRANSIENT_SRC); >> if (src < 0) >> return; >> >> - if (mma8452_freefall_mode_enabled(data)) { >> - iio_push_event(indio_dev, >> - IIO_MOD_EVENT_CODE(IIO_ACCEL, 0, >> - IIO_MOD_X_AND_Y_AND_Z, >> - IIO_EV_TYPE_MAG, >> - IIO_EV_DIR_FALLING), >> - ts); >> - return; >> - } >> - >> - if (src & data->chip_info->ev_src_xe) >> + if (src & MMA8452_TRANSIENT_SRC_XTRANSE) >> iio_push_event(indio_dev, >> IIO_MOD_EVENT_CODE(IIO_ACCEL, 0, IIO_MOD_X, >> IIO_EV_TYPE_MAG, >> IIO_EV_DIR_RISING), >> ts); >> >> - if (src & data->chip_info->ev_src_ye) >> + if (src & MMA8452_TRANSIENT_SRC_YTRANSE) >> iio_push_event(indio_dev, >> IIO_MOD_EVENT_CODE(IIO_ACCEL, 0, IIO_MOD_Y, >> IIO_EV_TYPE_MAG, >> IIO_EV_DIR_RISING), >> ts); >> >> - if (src & data->chip_info->ev_src_ze) >> + if (src & MMA8452_TRANSIENT_SRC_ZTRANSE) >> iio_push_event(indio_dev, >> IIO_MOD_EVENT_CODE(IIO_ACCEL, 0, IIO_MOD_Z, >> IIO_EV_TYPE_MAG, >> @@ -974,23 +989,41 @@ static irqreturn_t mma8452_interrupt(int irq, void *p) >> { >> struct iio_dev *indio_dev = p; >> struct mma8452_data *data = iio_priv(indio_dev); >> - const struct mma_chip_info *chip = data->chip_info; >> int ret = IRQ_NONE; >> - int src; >> + int src, enabled_interrupts; >> >> src = i2c_smbus_read_byte_data(data->client, MMA8452_INT_SRC); >> if (src < 0) >> return IRQ_NONE; >> >> + enabled_interrupts = i2c_smbus_read_byte_data(data->client, >> + MMA8452_CTRL_REG4); >> + if (enabled_interrupts < 0) >> + return enabled_interrupts; >> + >> + if (!(src & enabled_interrupts)) >> + return IRQ_NONE; >> + >> if (src & MMA8452_INT_DRDY) { >> iio_trigger_poll_chained(indio_dev->trig); >> ret = IRQ_HANDLED; >> } >> >> - if ((src & MMA8452_INT_TRANS && >> - chip->ev_src == MMA8452_TRANSIENT_SRC) || >> - (src & MMA8452_INT_FF_MT && >> - chip->ev_src == MMA8452_FF_MT_SRC)) { >> + if (src & MMA8452_INT_FF_MT) { >> + if (mma8452_freefall_mode_enabled(data)) { >> + s64 ts = iio_get_time_ns(indio_dev); >> + >> + iio_push_event(indio_dev, >> + IIO_MOD_EVENT_CODE(IIO_ACCEL, 0, >> + IIO_MOD_X_AND_Y_AND_Z, >> + IIO_EV_TYPE_MAG, >> + IIO_EV_DIR_FALLING), >> + ts); >> + } >> + ret = IRQ_HANDLED; >> + } >> + >> + if (src & MMA8452_INT_TRANS) { >> mma8452_transient_interrupt(indio_dev); >> ret = IRQ_HANDLED; >> } >> @@ -1222,96 +1255,36 @@ static const struct mma_chip_info mma_chip_info_table[] = { >> * g * N * 1000000 / 2048 for N = 2, 4, 8 and g=9.80665 >> */ >> .mma_scales = { {0, 2394}, {0, 4788}, {0, 9577} }, >> - .ev_cfg = MMA8452_TRANSIENT_CFG, >> - .ev_cfg_ele = MMA8452_TRANSIENT_CFG_ELE, >> - .ev_cfg_chan_shift = 1, >> - .ev_src = MMA8452_TRANSIENT_SRC, >> - .ev_src_xe = MMA8452_TRANSIENT_SRC_XTRANSE, >> - .ev_src_ye = MMA8452_TRANSIENT_SRC_YTRANSE, >> - .ev_src_ze = MMA8452_TRANSIENT_SRC_ZTRANSE, >> - .ev_ths = MMA8452_TRANSIENT_THS, >> - .ev_ths_mask = MMA8452_TRANSIENT_THS_MASK, >> - .ev_count = MMA8452_TRANSIENT_COUNT, >> }, >> [mma8452] = { >> .chip_id = MMA8452_DEVICE_ID, >> .channels = mma8452_channels, >> .num_channels = ARRAY_SIZE(mma8452_channels), >> .mma_scales = { {0, 9577}, {0, 19154}, {0, 38307} }, >> - .ev_cfg = MMA8452_TRANSIENT_CFG, >> - .ev_cfg_ele = MMA8452_TRANSIENT_CFG_ELE, >> - .ev_cfg_chan_shift = 1, >> - .ev_src = MMA8452_TRANSIENT_SRC, >> - .ev_src_xe = MMA8452_TRANSIENT_SRC_XTRANSE, >> - .ev_src_ye = MMA8452_TRANSIENT_SRC_YTRANSE, >> - .ev_src_ze = MMA8452_TRANSIENT_SRC_ZTRANSE, >> - .ev_ths = MMA8452_TRANSIENT_THS, >> - .ev_ths_mask = MMA8452_TRANSIENT_THS_MASK, >> - .ev_count = MMA8452_TRANSIENT_COUNT, >> }, >> [mma8453] = { >> .chip_id = MMA8453_DEVICE_ID, >> .channels = mma8453_channels, >> .num_channels = ARRAY_SIZE(mma8453_channels), >> .mma_scales = { {0, 38307}, {0, 76614}, {0, 153228} }, >> - .ev_cfg = MMA8452_TRANSIENT_CFG, >> - .ev_cfg_ele = MMA8452_TRANSIENT_CFG_ELE, >> - .ev_cfg_chan_shift = 1, >> - .ev_src = MMA8452_TRANSIENT_SRC, >> - .ev_src_xe = MMA8452_TRANSIENT_SRC_XTRANSE, >> - .ev_src_ye = MMA8452_TRANSIENT_SRC_YTRANSE, >> - .ev_src_ze = MMA8452_TRANSIENT_SRC_ZTRANSE, >> - .ev_ths = MMA8452_TRANSIENT_THS, >> - .ev_ths_mask = MMA8452_TRANSIENT_THS_MASK, >> - .ev_count = MMA8452_TRANSIENT_COUNT, >> }, >> [mma8652] = { >> .chip_id = MMA8652_DEVICE_ID, >> .channels = mma8652_channels, >> .num_channels = ARRAY_SIZE(mma8652_channels), >> .mma_scales = { {0, 9577}, {0, 19154}, {0, 38307} }, >> - .ev_cfg = MMA8452_FF_MT_CFG, >> - .ev_cfg_ele = MMA8452_FF_MT_CFG_ELE, >> - .ev_cfg_chan_shift = 3, >> - .ev_src = MMA8452_FF_MT_SRC, >> - .ev_src_xe = MMA8452_FF_MT_SRC_XHE, >> - .ev_src_ye = MMA8452_FF_MT_SRC_YHE, >> - .ev_src_ze = MMA8452_FF_MT_SRC_ZHE, >> - .ev_ths = MMA8452_FF_MT_THS, >> - .ev_ths_mask = MMA8452_FF_MT_THS_MASK, >> - .ev_count = MMA8452_FF_MT_COUNT, >> }, >> [mma8653] = { >> .chip_id = MMA8653_DEVICE_ID, >> .channels = mma8653_channels, >> .num_channels = ARRAY_SIZE(mma8653_channels), >> .mma_scales = { {0, 38307}, {0, 76614}, {0, 153228} }, >> - .ev_cfg = MMA8452_FF_MT_CFG, >> - .ev_cfg_ele = MMA8452_FF_MT_CFG_ELE, >> - .ev_cfg_chan_shift = 3, >> - .ev_src = MMA8452_FF_MT_SRC, >> - .ev_src_xe = MMA8452_FF_MT_SRC_XHE, >> - .ev_src_ye = MMA8452_FF_MT_SRC_YHE, >> - .ev_src_ze = MMA8452_FF_MT_SRC_ZHE, >> - .ev_ths = MMA8452_FF_MT_THS, >> - .ev_ths_mask = MMA8452_FF_MT_THS_MASK, >> - .ev_count = MMA8452_FF_MT_COUNT, >> }, >> [fxls8471] = { >> .chip_id = FXLS8471_DEVICE_ID, >> .channels = mma8451_channels, >> .num_channels = ARRAY_SIZE(mma8451_channels), >> .mma_scales = { {0, 2394}, {0, 4788}, {0, 9577} }, >> - .ev_cfg = MMA8452_TRANSIENT_CFG, >> - .ev_cfg_ele = MMA8452_TRANSIENT_CFG_ELE, >> - .ev_cfg_chan_shift = 1, >> - .ev_src = MMA8452_TRANSIENT_SRC, >> - .ev_src_xe = MMA8452_TRANSIENT_SRC_XTRANSE, >> - .ev_src_ye = MMA8452_TRANSIENT_SRC_YTRANSE, >> - .ev_src_ze = MMA8452_TRANSIENT_SRC_ZTRANSE, >> - .ev_ths = MMA8452_TRANSIENT_THS, >> - .ev_ths_mask = MMA8452_TRANSIENT_THS_MASK, >> - .ev_count = MMA8452_TRANSIENT_COUNT, >> }, >> }; >> >> ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] iio: accel: mma8452: Bugfix to enbale and allow different events to work parallely. 2017-08-17 0:39 ` Harinath Nampally @ 2017-08-17 11:24 ` Martin Kepplinger 2017-08-17 11:55 ` Harinath Nampally 0 siblings, 1 reply; 7+ messages in thread From: Martin Kepplinger @ 2017-08-17 11:24 UTC (permalink / raw) To: Harinath Nampally, Peter Meerwald-Stadler Cc: jic23, knaack.h, lars, gregkh, linux-iio, linux-kernel, amsfield22 Am 17=2E August 2017 02:39:05 MESZ schrieb Harinath Nampally <harinath922@= gmail=2Ecom>: >> Hello, >> >> fixes are always welcome, some comments regarding the patch >Thanks for the review=2E >> >> in subject: typo "enable" >Will fix it=2E >>> This driver supports multiple devices like mma8653, mma8652, >mma8452, mma8453 and >>> fxls8471=2E Almost all these devices have more than one event=2E Curre= nt >driver design >>> hardcodes the event specific information, so only one event can be >supported by this >>> driver and current design doesn't have the flexibility to add more >events=2E >> =20 >>> This patch fixes by detaching the event related information from >chip_info struct, >>> and based on channel type and event direction the corresponding >event configuration registers >>> are picked dynamically=2E Hence multiple events can be handled in >read/write callbacks=2E >> which chip can have which event(s)? >I am planning to add 'supported events' field in > >struct mma_chip_info which indicates which chip can have which events=2E >During initialization in 'mma_chip_info_table' would set this >'supported events' field for each chip=2E >But I wonder should I add those changes as part of this patch? is it necessary or can it be documentation? And this patch should have been called "v2"=2E please include a persistent= version history to v3 of this patch=2E thanks > >> =20 >>> Changes are thoroughly tested on fxls8471 device on imx6UL Eval >board using iio_event_monitor user space program=2E >>> >>> After this fix both Freefall and Transient events are handled by the >driver without any conflicts=2E >> thanks, p=2E >> =20 >>> Signed-off-by: Harinath Nampally <harinath922@gmail=2Ecom> >>> --- >>> drivers/iio/accel/mma8452=2Ec | 309 >++++++++++++++++++++------------------------ >>> 1 file changed, 141 insertions(+), 168 deletions(-) >>> >>> diff --git a/drivers/iio/accel/mma8452=2Ec >b/drivers/iio/accel/mma8452=2Ec >>> index eb6e3dc=2E=2E1b83e52 100644 >>> --- a/drivers/iio/accel/mma8452=2Ec >>> +++ b/drivers/iio/accel/mma8452=2Ec >>> @@ -59,7 +59,9 @@ >>> #define MMA8452_FF_MT_THS 0x17 >>> #define MMA8452_FF_MT_THS_MASK 0x7f >>> #define MMA8452_FF_MT_COUNT 0x18 >>> +#define MMA8452_FF_MT_CHAN_SHIFT 3 >>> #define MMA8452_TRANSIENT_CFG 0x1d >>> +#define MMA8452_TRANSIENT_CFG_CHAN(chan) BIT(chan + 1) >>> #define MMA8452_TRANSIENT_CFG_HPF_BYP BIT(0) >>> #define MMA8452_TRANSIENT_CFG_ELE BIT(4) >>> #define MMA8452_TRANSIENT_SRC 0x1e >>> @@ -69,6 +71,7 @@ >>> #define MMA8452_TRANSIENT_THS 0x1f >>> #define MMA8452_TRANSIENT_THS_MASK GENMASK(6, 0) >>> #define MMA8452_TRANSIENT_COUNT 0x20 >>> +#define MMA8452_TRANSIENT_CHAN_SHIFT 1 >>> #define MMA8452_CTRL_REG1 0x2a >>> #define MMA8452_CTRL_ACTIVE BIT(0) >>> #define MMA8452_CTRL_DR_MASK GENMASK(5, 3) >>> @@ -107,6 +110,26 @@ struct mma8452_data { >>> const struct mma_chip_info *chip_info; >>> }; >>> =20 >>> + /** >>> + * struct mma8452_event_regs - chip specific data related to >events >>> + * @ev_cfg: event config register address >>> + * @ev_src: event source register address >>> + * @ev_ths: event threshold register address >>> + * @ev_ths_mask: mask for the threshold value >>> + * @ev_count: event count (period) register address >>> + * >>> + * Since not all chips supported by the driver support comparing >high pass >>> + * filtered data for events (interrupts), different interrupt >sources are >>> + * used for different chips and the relevant registers are >included here=2E >>> + */ >>> +struct mma8452_event_regs { >>> + u8 ev_cfg; >>> + u8 ev_src; >>> + u8 ev_ths; >>> + u8 ev_ths_mask; >>> + u8 ev_count; >>> +}; >>> + >>> /** >>> * struct mma_chip_info - chip specific data >>> * @chip_id: WHO_AM_I register's value >>> @@ -116,40 +139,12 @@ struct mma8452_data { >>> * @mma_scales: scale factors for converting register values >>> * to m/s^2; 3 modes: 2g, 4g, 8g; 2 integers >>> * per mode: m/s^2 and micro m/s^2 >>> - * @ev_cfg: event config register address >>> - * @ev_cfg_ele: latch bit in event config register >>> - * @ev_cfg_chan_shift: number of the bit to enable events in X >>> - * direction; in event config register >>> - * @ev_src: event source register address >>> - * @ev_src_xe: bit in event source register that indicates >>> - * an event in X direction >>> - * @ev_src_ye: bit in event source register that indicates >>> - * an event in Y direction >>> - * @ev_src_ze: bit in event source register that indicates >>> - * an event in Z direction >>> - * @ev_ths: event threshold register address >>> - * @ev_ths_mask: mask for the threshold value >>> - * @ev_count: event count (period) register address >>> - * >>> - * Since not all chips supported by the driver support comparing >high pass >>> - * filtered data for events (interrupts), different interrupt >sources are >>> - * used for different chips and the relevant registers are included >here=2E >>> */ >>> struct mma_chip_info { >>> u8 chip_id; >>> const struct iio_chan_spec *channels; >>> int num_channels; >>> const int mma_scales[3][2]; >>> - u8 ev_cfg; >>> - u8 ev_cfg_ele; >>> - u8 ev_cfg_chan_shift; >>> - u8 ev_src; >>> - u8 ev_src_xe; >>> - u8 ev_src_ye; >>> - u8 ev_src_ze; >>> - u8 ev_ths; >>> - u8 ev_ths_mask; >>> - u8 ev_count; >>> }; >>> =20 >>> enum { >>> @@ -602,9 +597,8 @@ static int mma8452_set_power_mode(struct >mma8452_data *data, u8 mode) >>> static int mma8452_freefall_mode_enabled(struct mma8452_data >*data) >>> { >>> int val; >>> - const struct mma_chip_info *chip =3D data->chip_info; >>> =20 >>> - val =3D i2c_smbus_read_byte_data(data->client, chip->ev_cfg); >>> + val =3D i2c_smbus_read_byte_data(data->client, MMA8452_FF_MT_CFG); >>> if (val < 0) >>> return val; >>> =20 >>> @@ -614,29 +608,28 @@ static int >mma8452_freefall_mode_enabled(struct mma8452_data *data) >>> static int mma8452_set_freefall_mode(struct mma8452_data *data, >bool state) >>> { >>> int val; >>> - const struct mma_chip_info *chip =3D data->chip_info; >>> =20 >>> if ((state && mma8452_freefall_mode_enabled(data)) || >>> (!state && !(mma8452_freefall_mode_enabled(data)))) >>> return 0; >>> =20 >>> - val =3D i2c_smbus_read_byte_data(data->client, chip->ev_cfg); >>> + val =3D i2c_smbus_read_byte_data(data->client, MMA8452_FF_MT_CFG); >>> if (val < 0) >>> return val; >>> =20 >>> if (state) { >>> - val |=3D BIT(idx_x + chip->ev_cfg_chan_shift); >>> - val |=3D BIT(idx_y + chip->ev_cfg_chan_shift); >>> - val |=3D BIT(idx_z + chip->ev_cfg_chan_shift); >>> + val |=3D BIT(idx_x + MMA8452_FF_MT_CHAN_SHIFT); >>> + val |=3D BIT(idx_y + MMA8452_FF_MT_CHAN_SHIFT); >>> + val |=3D BIT(idx_z + MMA8452_FF_MT_CHAN_SHIFT); >>> val &=3D ~MMA8452_FF_MT_CFG_OAE; >>> } else { >>> - val &=3D ~BIT(idx_x + chip->ev_cfg_chan_shift); >>> - val &=3D ~BIT(idx_y + chip->ev_cfg_chan_shift); >>> - val &=3D ~BIT(idx_z + chip->ev_cfg_chan_shift); >>> + val &=3D ~BIT(idx_x + MMA8452_FF_MT_CHAN_SHIFT); >>> + val &=3D ~BIT(idx_y + MMA8452_FF_MT_CHAN_SHIFT); >>> + val &=3D ~BIT(idx_z + MMA8452_FF_MT_CHAN_SHIFT); >>> val |=3D MMA8452_FF_MT_CFG_OAE; >>> } >>> =20 >>> - return mma8452_change_config(data, chip->ev_cfg, val); >>> + return mma8452_change_config(data, MMA8452_FF_MT_CFG, val); >>> } >>> =20 >>> static int mma8452_set_hp_filter_frequency(struct mma8452_data >*data, >>> @@ -740,6 +733,39 @@ static int mma8452_write_raw(struct iio_dev >*indio_dev, >>> return ret; >>> } >>> =20 >>> +static int mma8452_get_event_regs(const struct iio_chan_spec *chan, >>> + enum iio_event_direction dir, >>> + struct mma8452_event_regs *ev_regs >>> + ) >>> +{ >>> + if (!chan) >>> + return -EINVAL; >>> + switch (chan->type) { >>> + case IIO_ACCEL: >>> + switch (dir) { >>> + case IIO_EV_DIR_FALLING: >>> + ev_regs->ev_cfg =3D MMA8452_FF_MT_CFG; >>> + ev_regs->ev_src =3D MMA8452_FF_MT_SRC; >>> + ev_regs->ev_ths =3D MMA8452_FF_MT_THS; >>> + ev_regs->ev_ths_mask =3D MMA8452_FF_MT_THS_MASK; >>> + ev_regs->ev_count =3D MMA8452_FF_MT_COUNT; >>> + break; >>> + case IIO_EV_DIR_RISING: >>> + ev_regs->ev_cfg =3D MMA8452_TRANSIENT_CFG; >>> + ev_regs->ev_src =3D MMA8452_TRANSIENT_SRC; >>> + ev_regs->ev_ths =3D MMA8452_TRANSIENT_THS; >> ev_ths_mask is not set here >> >Good catch! Will fix it=2E >>> + ev_regs->ev_count =3D MMA8452_TRANSIENT_COUNT; >>> + break; >>> + default: >>> + return -EINVAL; >>> + } >>> + break; >>> + default: >>> + return -EINVAL; >>> + } >>> + return 0; >> could replace 'breaks' with return 0 to save some lines and simplify >> control flow >Sure=2E >>> +} >>> + >>> static int mma8452_read_thresh(struct iio_dev *indio_dev, >>> const struct iio_chan_spec *chan, >>> enum iio_event_type type, >>> @@ -749,21 +775,23 @@ static int mma8452_read_thresh(struct iio_dev >*indio_dev, >>> { >>> struct mma8452_data *data =3D iio_priv(indio_dev); >>> int ret, us, power_mode; >>> + struct mma8452_event_regs ev_regs; >>> =20 >>> + ret =3D mma8452_get_event_regs(chan, dir, &ev_regs); >>> + if (ret) >>> + return ret; >>> switch (info) { >>> case IIO_EV_INFO_VALUE: >>> - ret =3D i2c_smbus_read_byte_data(data->client, >>> - data->chip_info->ev_ths); >>> + ret =3D i2c_smbus_read_byte_data(data->client, ev_regs=2Eev_ths); >>> if (ret < 0) >>> return ret; >>> =20 >>> - *val =3D ret & data->chip_info->ev_ths_mask; >>> + *val =3D ret & ev_regs=2Eev_ths_mask; >>> =20 >>> return IIO_VAL_INT; >>> =20 >>> case IIO_EV_INFO_PERIOD: >>> - ret =3D i2c_smbus_read_byte_data(data->client, >>> - data->chip_info->ev_count); >>> + ret =3D i2c_smbus_read_byte_data(data->client, ev_regs=2Eev_count); >>> if (ret < 0) >>> return ret; >>> =20 >>> @@ -809,14 +837,18 @@ static int mma8452_write_thresh(struct iio_dev >*indio_dev, >>> { >>> struct mma8452_data *data =3D iio_priv(indio_dev); >>> int ret, reg, steps; >>> + struct mma8452_event_regs ev_regs; >>> + >>> + ret =3D mma8452_get_event_regs(chan, dir, &ev_regs); >>> + if (ret) >>> + return ret; >>> =20 >>> switch (info) { >>> case IIO_EV_INFO_VALUE: >>> - if (val < 0 || val > MMA8452_TRANSIENT_THS_MASK) >>> + if (val < 0 || val > ev_regs=2Eev_ths_mask) >>> return -EINVAL; >>> =20 >>> - return mma8452_change_config(data, data->chip_info->ev_ths, >>> - val); >>> + return mma8452_change_config(data, ev_regs=2Eev_ths, val); >>> =20 >>> case IIO_EV_INFO_PERIOD: >>> ret =3D mma8452_get_power_mode(data); >>> @@ -830,8 +862,7 @@ static int mma8452_write_thresh(struct iio_dev >*indio_dev, >>> if (steps < 0 || steps > 0xff) >>> return -EINVAL; >>> =20 >>> - return mma8452_change_config(data, data->chip_info->ev_count, >>> - steps); >>> + return mma8452_change_config(data, ev_regs=2Eev_count, steps); >>> =20 >>> case IIO_EV_INFO_HIGH_PASS_FILTER_3DB: >>> reg =3D i2c_smbus_read_byte_data(data->client, >>> @@ -861,23 +892,23 @@ static int mma8452_read_event_config(struct >iio_dev *indio_dev, >>> enum iio_event_direction dir) >>> { >>> struct mma8452_data *data =3D iio_priv(indio_dev); >>> - const struct mma_chip_info *chip =3D data->chip_info; >>> int ret; >>> =20 >>> - switch (dir) { >>> - case IIO_EV_DIR_FALLING: >>> - return mma8452_freefall_mode_enabled(data); >>> - case IIO_EV_DIR_RISING: >>> - if (mma8452_freefall_mode_enabled(data)) >>> - return 0; >>> + switch (chan->type) { >>> + case IIO_ACCEL: >> this adds a check for chan-type =3D=3D IIO_ACCESS, so strictly this wou= ld >be >> an unrelated change=2E=2E=2E >Ok will remove it from this patch=2E >>> + switch (dir) { >>> + case IIO_EV_DIR_FALLING: >>> + return mma8452_freefall_mode_enabled(data); >>> + case IIO_EV_DIR_RISING: >>> + ret =3D i2c_smbus_read_byte_data(data->client, >MMA8452_TRANSIENT_CFG); >>> + if (ret < 0) >>> + return ret; >>> =20 >>> - ret =3D i2c_smbus_read_byte_data(data->client, >>> - data->chip_info->ev_cfg); >>> - if (ret < 0) >>> - return ret; >>> + return !!(ret & MMA8452_TRANSIENT_CFG_CHAN(chan->scan_index)); >>> =20 >>> - return !!(ret & BIT(chan->scan_index + >>> - chip->ev_cfg_chan_shift)); >>> + default: >>> + return -EINVAL; >>> + } >>> default: >>> return -EINVAL; >>> } >>> @@ -890,39 +921,33 @@ static int mma8452_write_event_config(struct >iio_dev *indio_dev, >>> int state) >>> { >>> struct mma8452_data *data =3D iio_priv(indio_dev); >>> - const struct mma_chip_info *chip =3D data->chip_info; >>> int val, ret; >>> =20 >>> ret =3D mma8452_set_runtime_pm_state(data->client, state); >>> if (ret) >>> return ret; >>> =20 >>> - switch (dir) { >>> - case IIO_EV_DIR_FALLING: >>> - return mma8452_set_freefall_mode(data, state); >>> - case IIO_EV_DIR_RISING: >>> - val =3D i2c_smbus_read_byte_data(data->client, chip->ev_cfg); >>> - if (val < 0) >>> - return val; >>> - >>> - if (state) { >>> - if (mma8452_freefall_mode_enabled(data)) { >>> - val &=3D ~BIT(idx_x + chip->ev_cfg_chan_shift); >>> - val &=3D ~BIT(idx_y + chip->ev_cfg_chan_shift); >>> - val &=3D ~BIT(idx_z + chip->ev_cfg_chan_shift); >>> - val |=3D MMA8452_FF_MT_CFG_OAE; >>> - } >>> - val |=3D BIT(chan->scan_index + chip->ev_cfg_chan_shift); >>> - } else { >>> - if (mma8452_freefall_mode_enabled(data)) >>> - return 0; >>> - >>> - val &=3D ~BIT(chan->scan_index + chip->ev_cfg_chan_shift); >>> + switch (chan->type) { >>> + case IIO_ACCEL: >>> + switch (dir) { >>> + case IIO_EV_DIR_FALLING: >>> + return mma8452_set_freefall_mode(data, state); >>> + case IIO_EV_DIR_RISING: >>> + val =3D i2c_smbus_read_byte_data(data->client, >MMA8452_TRANSIENT_CFG); >>> + if (val < 0) >>> + return val; >>> + >>> + if (state) >>> + val |=3D MMA8452_TRANSIENT_CFG_CHAN(chan->scan_index); >>> + else >>> + val &=3D ~MMA8452_TRANSIENT_CFG_CHAN(chan->scan_index); >>> + >>> + val |=3D MMA8452_TRANSIENT_CFG_ELE; >>> + >>> + return mma8452_change_config(data, MMA8452_TRANSIENT_CFG, val); >>> + default: >>> + return -EINVAL; >>> } >>> - >>> - val |=3D chip->ev_cfg_ele; >>> - >>> - return mma8452_change_config(data, chip->ev_cfg, val); >>> default: >>> return -EINVAL; >>> } >>> @@ -934,35 +959,25 @@ static void mma8452_transient_interrupt(struct >iio_dev *indio_dev) >>> s64 ts =3D iio_get_time_ns(indio_dev); >>> int src; >>> =20 >>> - src =3D i2c_smbus_read_byte_data(data->client, >data->chip_info->ev_src); >>> + src =3D i2c_smbus_read_byte_data(data->client, >MMA8452_TRANSIENT_SRC); >>> if (src < 0) >>> return; >>> =20 >>> - if (mma8452_freefall_mode_enabled(data)) { >>> - iio_push_event(indio_dev, >>> - IIO_MOD_EVENT_CODE(IIO_ACCEL, 0, >>> - IIO_MOD_X_AND_Y_AND_Z, >>> - IIO_EV_TYPE_MAG, >>> - IIO_EV_DIR_FALLING), >>> - ts); >>> - return; >>> - } >>> - >>> - if (src & data->chip_info->ev_src_xe) >>> + if (src & MMA8452_TRANSIENT_SRC_XTRANSE) >>> iio_push_event(indio_dev, >>> IIO_MOD_EVENT_CODE(IIO_ACCEL, 0, IIO_MOD_X, >>> IIO_EV_TYPE_MAG, >>> IIO_EV_DIR_RISING), >>> ts); >>> =20 >>> - if (src & data->chip_info->ev_src_ye) >>> + if (src & MMA8452_TRANSIENT_SRC_YTRANSE) >>> iio_push_event(indio_dev, >>> IIO_MOD_EVENT_CODE(IIO_ACCEL, 0, IIO_MOD_Y, >>> IIO_EV_TYPE_MAG, >>> IIO_EV_DIR_RISING), >>> ts); >>> =20 >>> - if (src & data->chip_info->ev_src_ze) >>> + if (src & MMA8452_TRANSIENT_SRC_ZTRANSE) >>> iio_push_event(indio_dev, >>> IIO_MOD_EVENT_CODE(IIO_ACCEL, 0, IIO_MOD_Z, >>> IIO_EV_TYPE_MAG, >>> @@ -974,23 +989,41 @@ static irqreturn_t mma8452_interrupt(int irq, >void *p) >>> { >>> struct iio_dev *indio_dev =3D p; >>> struct mma8452_data *data =3D iio_priv(indio_dev); >>> - const struct mma_chip_info *chip =3D data->chip_info; >>> int ret =3D IRQ_NONE; >>> - int src; >>> + int src, enabled_interrupts; >>> =20 >>> src =3D i2c_smbus_read_byte_data(data->client, MMA8452_INT_SRC); >>> if (src < 0) >>> return IRQ_NONE; >>> =20 >>> + enabled_interrupts =3D i2c_smbus_read_byte_data(data->client, >>> + MMA8452_CTRL_REG4); >>> + if (enabled_interrupts < 0) >>> + return enabled_interrupts; >>> + >>> + if (!(src & enabled_interrupts)) >>> + return IRQ_NONE; >>> + >>> if (src & MMA8452_INT_DRDY) { >>> iio_trigger_poll_chained(indio_dev->trig); >>> ret =3D IRQ_HANDLED; >>> } >>> =20 >>> - if ((src & MMA8452_INT_TRANS && >>> - chip->ev_src =3D=3D MMA8452_TRANSIENT_SRC) || >>> - (src & MMA8452_INT_FF_MT && >>> - chip->ev_src =3D=3D MMA8452_FF_MT_SRC)) { >>> + if (src & MMA8452_INT_FF_MT) { >>> + if (mma8452_freefall_mode_enabled(data)) { >>> + s64 ts =3D iio_get_time_ns(indio_dev); >>> + >>> + iio_push_event(indio_dev, >>> + IIO_MOD_EVENT_CODE(IIO_ACCEL, 0, >>> + IIO_MOD_X_AND_Y_AND_Z, >>> + IIO_EV_TYPE_MAG, >>> + IIO_EV_DIR_FALLING), >>> + ts); >>> + } >>> + ret =3D IRQ_HANDLED; >>> + } >>> + >>> + if (src & MMA8452_INT_TRANS) { >>> mma8452_transient_interrupt(indio_dev); >>> ret =3D IRQ_HANDLED; >>> } >>> @@ -1222,96 +1255,36 @@ static const struct mma_chip_info >mma_chip_info_table[] =3D { >>> * g * N * 1000000 / 2048 for N =3D 2, 4, 8 and g=3D9=2E80665 >>> */ >>> =2Emma_scales =3D { {0, 2394}, {0, 4788}, {0, 9577} }, >>> - =2Eev_cfg =3D MMA8452_TRANSIENT_CFG, >>> - =2Eev_cfg_ele =3D MMA8452_TRANSIENT_CFG_ELE, >>> - =2Eev_cfg_chan_shift =3D 1, >>> - =2Eev_src =3D MMA8452_TRANSIENT_SRC, >>> - =2Eev_src_xe =3D MMA8452_TRANSIENT_SRC_XTRANSE, >>> - =2Eev_src_ye =3D MMA8452_TRANSIENT_SRC_YTRANSE, >>> - =2Eev_src_ze =3D MMA8452_TRANSIENT_SRC_ZTRANSE, >>> - =2Eev_ths =3D MMA8452_TRANSIENT_THS, >>> - =2Eev_ths_mask =3D MMA8452_TRANSIENT_THS_MASK, >>> - =2Eev_count =3D MMA8452_TRANSIENT_COUNT, >>> }, >>> [mma8452] =3D { >>> =2Echip_id =3D MMA8452_DEVICE_ID, >>> =2Echannels =3D mma8452_channels, >>> =2Enum_channels =3D ARRAY_SIZE(mma8452_channels), >>> =2Emma_scales =3D { {0, 9577}, {0, 19154}, {0, 38307} }, >>> - =2Eev_cfg =3D MMA8452_TRANSIENT_CFG, >>> - =2Eev_cfg_ele =3D MMA8452_TRANSIENT_CFG_ELE, >>> - =2Eev_cfg_chan_shift =3D 1, >>> - =2Eev_src =3D MMA8452_TRANSIENT_SRC, >>> - =2Eev_src_xe =3D MMA8452_TRANSIENT_SRC_XTRANSE, >>> - =2Eev_src_ye =3D MMA8452_TRANSIENT_SRC_YTRANSE, >>> - =2Eev_src_ze =3D MMA8452_TRANSIENT_SRC_ZTRANSE, >>> - =2Eev_ths =3D MMA8452_TRANSIENT_THS, >>> - =2Eev_ths_mask =3D MMA8452_TRANSIENT_THS_MASK, >>> - =2Eev_count =3D MMA8452_TRANSIENT_COUNT, >>> }, >>> [mma8453] =3D { >>> =2Echip_id =3D MMA8453_DEVICE_ID, >>> =2Echannels =3D mma8453_channels, >>> =2Enum_channels =3D ARRAY_SIZE(mma8453_channels), >>> =2Emma_scales =3D { {0, 38307}, {0, 76614}, {0, 153228} }, >>> - =2Eev_cfg =3D MMA8452_TRANSIENT_CFG, >>> - =2Eev_cfg_ele =3D MMA8452_TRANSIENT_CFG_ELE, >>> - =2Eev_cfg_chan_shift =3D 1, >>> - =2Eev_src =3D MMA8452_TRANSIENT_SRC, >>> - =2Eev_src_xe =3D MMA8452_TRANSIENT_SRC_XTRANSE, >>> - =2Eev_src_ye =3D MMA8452_TRANSIENT_SRC_YTRANSE, >>> - =2Eev_src_ze =3D MMA8452_TRANSIENT_SRC_ZTRANSE, >>> - =2Eev_ths =3D MMA8452_TRANSIENT_THS, >>> - =2Eev_ths_mask =3D MMA8452_TRANSIENT_THS_MASK, >>> - =2Eev_count =3D MMA8452_TRANSIENT_COUNT, >>> }, >>> [mma8652] =3D { >>> =2Echip_id =3D MMA8652_DEVICE_ID, >>> =2Echannels =3D mma8652_channels, >>> =2Enum_channels =3D ARRAY_SIZE(mma8652_channels), >>> =2Emma_scales =3D { {0, 9577}, {0, 19154}, {0, 38307} }, >>> - =2Eev_cfg =3D MMA8452_FF_MT_CFG, >>> - =2Eev_cfg_ele =3D MMA8452_FF_MT_CFG_ELE, >>> - =2Eev_cfg_chan_shift =3D 3, >>> - =2Eev_src =3D MMA8452_FF_MT_SRC, >>> - =2Eev_src_xe =3D MMA8452_FF_MT_SRC_XHE, >>> - =2Eev_src_ye =3D MMA8452_FF_MT_SRC_YHE, >>> - =2Eev_src_ze =3D MMA8452_FF_MT_SRC_ZHE, >>> - =2Eev_ths =3D MMA8452_FF_MT_THS, >>> - =2Eev_ths_mask =3D MMA8452_FF_MT_THS_MASK, >>> - =2Eev_count =3D MMA8452_FF_MT_COUNT, >>> }, >>> [mma8653] =3D { >>> =2Echip_id =3D MMA8653_DEVICE_ID, >>> =2Echannels =3D mma8653_channels, >>> =2Enum_channels =3D ARRAY_SIZE(mma8653_channels), >>> =2Emma_scales =3D { {0, 38307}, {0, 76614}, {0, 153228} }, >>> - =2Eev_cfg =3D MMA8452_FF_MT_CFG, >>> - =2Eev_cfg_ele =3D MMA8452_FF_MT_CFG_ELE, >>> - =2Eev_cfg_chan_shift =3D 3, >>> - =2Eev_src =3D MMA8452_FF_MT_SRC, >>> - =2Eev_src_xe =3D MMA8452_FF_MT_SRC_XHE, >>> - =2Eev_src_ye =3D MMA8452_FF_MT_SRC_YHE, >>> - =2Eev_src_ze =3D MMA8452_FF_MT_SRC_ZHE, >>> - =2Eev_ths =3D MMA8452_FF_MT_THS, >>> - =2Eev_ths_mask =3D MMA8452_FF_MT_THS_MASK, >>> - =2Eev_count =3D MMA8452_FF_MT_COUNT, >>> }, >>> [fxls8471] =3D { >>> =2Echip_id =3D FXLS8471_DEVICE_ID, >>> =2Echannels =3D mma8451_channels, >>> =2Enum_channels =3D ARRAY_SIZE(mma8451_channels), >>> =2Emma_scales =3D { {0, 2394}, {0, 4788}, {0, 9577} }, >>> - =2Eev_cfg =3D MMA8452_TRANSIENT_CFG, >>> - =2Eev_cfg_ele =3D MMA8452_TRANSIENT_CFG_ELE, >>> - =2Eev_cfg_chan_shift =3D 1, >>> - =2Eev_src =3D MMA8452_TRANSIENT_SRC, >>> - =2Eev_src_xe =3D MMA8452_TRANSIENT_SRC_XTRANSE, >>> - =2Eev_src_ye =3D MMA8452_TRANSIENT_SRC_YTRANSE, >>> - =2Eev_src_ze =3D MMA8452_TRANSIENT_SRC_ZTRANSE, >>> - =2Eev_ths =3D MMA8452_TRANSIENT_THS, >>> - =2Eev_ths_mask =3D MMA8452_TRANSIENT_THS_MASK, >>> - =2Eev_count =3D MMA8452_TRANSIENT_COUNT, >>> }, >>> }; >>> =20 >>> > >On 08/16/2017 09:12 AM, Peter Meerwald-Stadler wrote: >> Hello, >> >> fixes are always welcome, some comments regarding the patch >> >> in subject: typo "enable" >> >>> This driver supports multiple devices like mma8653, mma8652, >mma8452, mma8453 and >>> fxls8471=2E Almost all these devices have more than one event=2E Curre= nt >driver design >>> hardcodes the event specific information, so only one event can be >supported by this >>> driver and current design doesn't have the flexibility to add more >events=2E >> =20 >>> This patch fixes by detaching the event related information from >chip_info struct, >>> and based on channel type and event direction the corresponding >event configuration registers >>> are picked dynamically=2E Hence multiple events can be handled in >read/write callbacks=2E >> which chip can have which event(s)? >> =20 >>> Changes are thoroughly tested on fxls8471 device on imx6UL Eval >board using iio_event_monitor user space program=2E >>> >>> After this fix both Freefall and Transient events are handled by the >driver without any conflicts=2E >> thanks, p=2E >> =20 >>> Signed-off-by: Harinath Nampally <harinath922@gmail=2Ecom> >>> --- >>> drivers/iio/accel/mma8452=2Ec | 309 >++++++++++++++++++++------------------------ >>> 1 file changed, 141 insertions(+), 168 deletions(-) >>> >>> diff --git a/drivers/iio/accel/mma8452=2Ec >b/drivers/iio/accel/mma8452=2Ec >>> index eb6e3dc=2E=2E1b83e52 100644 >>> --- a/drivers/iio/accel/mma8452=2Ec >>> +++ b/drivers/iio/accel/mma8452=2Ec >>> @@ -59,7 +59,9 @@ >>> #define MMA8452_FF_MT_THS 0x17 >>> #define MMA8452_FF_MT_THS_MASK 0x7f >>> #define MMA8452_FF_MT_COUNT 0x18 >>> +#define MMA8452_FF_MT_CHAN_SHIFT 3 >>> #define MMA8452_TRANSIENT_CFG 0x1d >>> +#define MMA8452_TRANSIENT_CFG_CHAN(chan) BIT(chan + 1) >>> #define MMA8452_TRANSIENT_CFG_HPF_BYP BIT(0) >>> #define MMA8452_TRANSIENT_CFG_ELE BIT(4) >>> #define MMA8452_TRANSIENT_SRC 0x1e >>> @@ -69,6 +71,7 @@ >>> #define MMA8452_TRANSIENT_THS 0x1f >>> #define MMA8452_TRANSIENT_THS_MASK GENMASK(6, 0) >>> #define MMA8452_TRANSIENT_COUNT 0x20 >>> +#define MMA8452_TRANSIENT_CHAN_SHIFT 1 >>> #define MMA8452_CTRL_REG1 0x2a >>> #define MMA8452_CTRL_ACTIVE BIT(0) >>> #define MMA8452_CTRL_DR_MASK GENMASK(5, 3) >>> @@ -107,6 +110,26 @@ struct mma8452_data { >>> const struct mma_chip_info *chip_info; >>> }; >>> =20 >>> + /** >>> + * struct mma8452_event_regs - chip specific data related to >events >>> + * @ev_cfg: event config register address >>> + * @ev_src: event source register address >>> + * @ev_ths: event threshold register address >>> + * @ev_ths_mask: mask for the threshold value >>> + * @ev_count: event count (period) register address >>> + * >>> + * Since not all chips supported by the driver support comparing >high pass >>> + * filtered data for events (interrupts), different interrupt >sources are >>> + * used for different chips and the relevant registers are >included here=2E >>> + */ >>> +struct mma8452_event_regs { >>> + u8 ev_cfg; >>> + u8 ev_src; >>> + u8 ev_ths; >>> + u8 ev_ths_mask; >>> + u8 ev_count; >>> +}; >>> + >>> /** >>> * struct mma_chip_info - chip specific data >>> * @chip_id: WHO_AM_I register's value >>> @@ -116,40 +139,12 @@ struct mma8452_data { >>> * @mma_scales: scale factors for converting register values >>> * to m/s^2; 3 modes: 2g, 4g, 8g; 2 integers >>> * per mode: m/s^2 and micro m/s^2 >>> - * @ev_cfg: event config register address >>> - * @ev_cfg_ele: latch bit in event config register >>> - * @ev_cfg_chan_shift: number of the bit to enable events in X >>> - * direction; in event config register >>> - * @ev_src: event source register address >>> - * @ev_src_xe: bit in event source register that indicates >>> - * an event in X direction >>> - * @ev_src_ye: bit in event source register that indicates >>> - * an event in Y direction >>> - * @ev_src_ze: bit in event source register that indicates >>> - * an event in Z direction >>> - * @ev_ths: event threshold register address >>> - * @ev_ths_mask: mask for the threshold value >>> - * @ev_count: event count (period) register address >>> - * >>> - * Since not all chips supported by the driver support comparing >high pass >>> - * filtered data for events (interrupts), different interrupt >sources are >>> - * used for different chips and the relevant registers are included >here=2E >>> */ >>> struct mma_chip_info { >>> u8 chip_id; >>> const struct iio_chan_spec *channels; >>> int num_channels; >>> const int mma_scales[3][2]; >>> - u8 ev_cfg; >>> - u8 ev_cfg_ele; >>> - u8 ev_cfg_chan_shift; >>> - u8 ev_src; >>> - u8 ev_src_xe; >>> - u8 ev_src_ye; >>> - u8 ev_src_ze; >>> - u8 ev_ths; >>> - u8 ev_ths_mask; >>> - u8 ev_count; >>> }; >>> =20 >>> enum { >>> @@ -602,9 +597,8 @@ static int mma8452_set_power_mode(struct >mma8452_data *data, u8 mode) >>> static int mma8452_freefall_mode_enabled(struct mma8452_data >*data) >>> { >>> int val; >>> - const struct mma_chip_info *chip =3D data->chip_info; >>> =20 >>> - val =3D i2c_smbus_read_byte_data(data->client, chip->ev_cfg); >>> + val =3D i2c_smbus_read_byte_data(data->client, MMA8452_FF_MT_CFG); >>> if (val < 0) >>> return val; >>> =20 >>> @@ -614,29 +608,28 @@ static int >mma8452_freefall_mode_enabled(struct mma8452_data *data) >>> static int mma8452_set_freefall_mode(struct mma8452_data *data, >bool state) >>> { >>> int val; >>> - const struct mma_chip_info *chip =3D data->chip_info; >>> =20 >>> if ((state && mma8452_freefall_mode_enabled(data)) || >>> (!state && !(mma8452_freefall_mode_enabled(data)))) >>> return 0; >>> =20 >>> - val =3D i2c_smbus_read_byte_data(data->client, chip->ev_cfg); >>> + val =3D i2c_smbus_read_byte_data(data->client, MMA8452_FF_MT_CFG); >>> if (val < 0) >>> return val; >>> =20 >>> if (state) { >>> - val |=3D BIT(idx_x + chip->ev_cfg_chan_shift); >>> - val |=3D BIT(idx_y + chip->ev_cfg_chan_shift); >>> - val |=3D BIT(idx_z + chip->ev_cfg_chan_shift); >>> + val |=3D BIT(idx_x + MMA8452_FF_MT_CHAN_SHIFT); >>> + val |=3D BIT(idx_y + MMA8452_FF_MT_CHAN_SHIFT); >>> + val |=3D BIT(idx_z + MMA8452_FF_MT_CHAN_SHIFT); >>> val &=3D ~MMA8452_FF_MT_CFG_OAE; >>> } else { >>> - val &=3D ~BIT(idx_x + chip->ev_cfg_chan_shift); >>> - val &=3D ~BIT(idx_y + chip->ev_cfg_chan_shift); >>> - val &=3D ~BIT(idx_z + chip->ev_cfg_chan_shift); >>> + val &=3D ~BIT(idx_x + MMA8452_FF_MT_CHAN_SHIFT); >>> + val &=3D ~BIT(idx_y + MMA8452_FF_MT_CHAN_SHIFT); >>> + val &=3D ~BIT(idx_z + MMA8452_FF_MT_CHAN_SHIFT); >>> val |=3D MMA8452_FF_MT_CFG_OAE; >>> } >>> =20 >>> - return mma8452_change_config(data, chip->ev_cfg, val); >>> + return mma8452_change_config(data, MMA8452_FF_MT_CFG, val); >>> } >>> =20 >>> static int mma8452_set_hp_filter_frequency(struct mma8452_data >*data, >>> @@ -740,6 +733,39 @@ static int mma8452_write_raw(struct iio_dev >*indio_dev, >>> return ret; >>> } >>> =20 >>> +static int mma8452_get_event_regs(const struct iio_chan_spec *chan, >>> + enum iio_event_direction dir, >>> + struct mma8452_event_regs *ev_regs >>> + ) >>> +{ >>> + if (!chan) >>> + return -EINVAL; >>> + switch (chan->type) { >>> + case IIO_ACCEL: >>> + switch (dir) { >>> + case IIO_EV_DIR_FALLING: >>> + ev_regs->ev_cfg =3D MMA8452_FF_MT_CFG; >>> + ev_regs->ev_src =3D MMA8452_FF_MT_SRC; >>> + ev_regs->ev_ths =3D MMA8452_FF_MT_THS; >>> + ev_regs->ev_ths_mask =3D MMA8452_FF_MT_THS_MASK; >>> + ev_regs->ev_count =3D MMA8452_FF_MT_COUNT; >>> + break; >>> + case IIO_EV_DIR_RISING: >>> + ev_regs->ev_cfg =3D MMA8452_TRANSIENT_CFG; >>> + ev_regs->ev_src =3D MMA8452_TRANSIENT_SRC; >>> + ev_regs->ev_ths =3D MMA8452_TRANSIENT_THS; >> >> ev_ths_mask is not set here >> >>> + ev_regs->ev_count =3D MMA8452_TRANSIENT_COUNT; >>> + break; >>> + default: >>> + return -EINVAL; >>> + } >>> + break; >>> + default: >>> + return -EINVAL; >>> + } >>> + return 0; >> could replace 'breaks' with return 0 to save some lines and simplify >> control flow >> >>> +} >>> + >>> static int mma8452_read_thresh(struct iio_dev *indio_dev, >>> const struct iio_chan_spec *chan, >>> enum iio_event_type type, >>> @@ -749,21 +775,23 @@ static int mma8452_read_thresh(struct iio_dev >*indio_dev, >>> { >>> struct mma8452_data *data =3D iio_priv(indio_dev); >>> int ret, us, power_mode; >>> + struct mma8452_event_regs ev_regs; >>> =20 >>> + ret =3D mma8452_get_event_regs(chan, dir, &ev_regs); >>> + if (ret) >>> + return ret; >>> switch (info) { >>> case IIO_EV_INFO_VALUE: >>> - ret =3D i2c_smbus_read_byte_data(data->client, >>> - data->chip_info->ev_ths); >>> + ret =3D i2c_smbus_read_byte_data(data->client, ev_regs=2Eev_ths); >>> if (ret < 0) >>> return ret; >>> =20 >>> - *val =3D ret & data->chip_info->ev_ths_mask; >>> + *val =3D ret & ev_regs=2Eev_ths_mask; >>> =20 >>> return IIO_VAL_INT; >>> =20 >>> case IIO_EV_INFO_PERIOD: >>> - ret =3D i2c_smbus_read_byte_data(data->client, >>> - data->chip_info->ev_count); >>> + ret =3D i2c_smbus_read_byte_data(data->client, ev_regs=2Eev_count); >>> if (ret < 0) >>> return ret; >>> =20 >>> @@ -809,14 +837,18 @@ static int mma8452_write_thresh(struct iio_dev >*indio_dev, >>> { >>> struct mma8452_data *data =3D iio_priv(indio_dev); >>> int ret, reg, steps; >>> + struct mma8452_event_regs ev_regs; >>> + >>> + ret =3D mma8452_get_event_regs(chan, dir, &ev_regs); >>> + if (ret) >>> + return ret; >>> =20 >>> switch (info) { >>> case IIO_EV_INFO_VALUE: >>> - if (val < 0 || val > MMA8452_TRANSIENT_THS_MASK) >>> + if (val < 0 || val > ev_regs=2Eev_ths_mask) >>> return -EINVAL; >>> =20 >>> - return mma8452_change_config(data, data->chip_info->ev_ths, >>> - val); >>> + return mma8452_change_config(data, ev_regs=2Eev_ths, val); >>> =20 >>> case IIO_EV_INFO_PERIOD: >>> ret =3D mma8452_get_power_mode(data); >>> @@ -830,8 +862,7 @@ static int mma8452_write_thresh(struct iio_dev >*indio_dev, >>> if (steps < 0 || steps > 0xff) >>> return -EINVAL; >>> =20 >>> - return mma8452_change_config(data, data->chip_info->ev_count, >>> - steps); >>> + return mma8452_change_config(data, ev_regs=2Eev_count, steps); >>> =20 >>> case IIO_EV_INFO_HIGH_PASS_FILTER_3DB: >>> reg =3D i2c_smbus_read_byte_data(data->client, >>> @@ -861,23 +892,23 @@ static int mma8452_read_event_config(struct >iio_dev *indio_dev, >>> enum iio_event_direction dir) >>> { >>> struct mma8452_data *data =3D iio_priv(indio_dev); >>> - const struct mma_chip_info *chip =3D data->chip_info; >>> int ret; >>> =20 >>> - switch (dir) { >>> - case IIO_EV_DIR_FALLING: >>> - return mma8452_freefall_mode_enabled(data); >>> - case IIO_EV_DIR_RISING: >>> - if (mma8452_freefall_mode_enabled(data)) >>> - return 0; >>> + switch (chan->type) { >>> + case IIO_ACCEL: >> this adds a check for chan-type =3D=3D IIO_ACCESS, so strictly this wou= ld >be >> an unrelated change=2E=2E=2E >> >>> + switch (dir) { >>> + case IIO_EV_DIR_FALLING: >>> + return mma8452_freefall_mode_enabled(data); >>> + case IIO_EV_DIR_RISING: >>> + ret =3D i2c_smbus_read_byte_data(data->client, >MMA8452_TRANSIENT_CFG); >>> + if (ret < 0) >>> + return ret; >>> =20 >>> - ret =3D i2c_smbus_read_byte_data(data->client, >>> - data->chip_info->ev_cfg); >>> - if (ret < 0) >>> - return ret; >>> + return !!(ret & MMA8452_TRANSIENT_CFG_CHAN(chan->scan_index)); >>> =20 >>> - return !!(ret & BIT(chan->scan_index + >>> - chip->ev_cfg_chan_shift)); >>> + default: >>> + return -EINVAL; >>> + } >>> default: >>> return -EINVAL; >>> } >>> @@ -890,39 +921,33 @@ static int mma8452_write_event_config(struct >iio_dev *indio_dev, >>> int state) >>> { >>> struct mma8452_data *data =3D iio_priv(indio_dev); >>> - const struct mma_chip_info *chip =3D data->chip_info; >>> int val, ret; >>> =20 >>> ret =3D mma8452_set_runtime_pm_state(data->client, state); >>> if (ret) >>> retu --=20 Martin Kepplinger http://martinkepplinger=2Ecom sent from mobile ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] iio: accel: mma8452: Bugfix to enbale and allow different events to work parallely. 2017-08-17 11:24 ` Martin Kepplinger @ 2017-08-17 11:55 ` Harinath Nampally 2017-08-17 14:40 ` Jonathan Cameron 0 siblings, 1 reply; 7+ messages in thread From: Harinath Nampally @ 2017-08-17 11:55 UTC (permalink / raw) To: Martin Kepplinger, Peter Meerwald-Stadler Cc: jic23, knaack.h, lars, gregkh, linux-iio, linux-kernel, amsfield22 > This patch fixes by detaching the event related information from > chip_info struct, >>> and based on channel type and event direction the corresponding > event configuration registers >>> are picked dynamically. Hence multiple events can be handled in > read/write callbacks. >> which chip can have which event(s)? > I am planning to add 'supported events' field in > > struct mma_chip_info which indicates which chip can have which events. > During initialization in 'mma_chip_info_table' would set this > 'supported events' field for each chip. > But I wonder should I add those changes as part of this patch? > is it necessary or can it be documentation? I think its not necessary as we only have Freefall and Transient events for now. Ok I will just update the documentation. > > And this patch should have been called "v2". please include a persistent version history to v3 of this patch. Sure I will send v3 patch, should I use '--in-reply-to' option of git send-email to send v3 patch as reply to original thread? On 08/17/2017 07:24 AM, Martin Kepplinger wrote: >>>> This patch fixes by detaching the event related information from >> chip_info struct, >>>> and based on channel type and event direction the corresponding >> event configuration registers >>>> are picked dynamically. Hence multiple events can be handled in >> read/write callbacks. >>> which chip can have which event(s)? >> I am planning to add 'supported events' field in >> >> struct mma_chip_info which indicates which chip can have which events. >> During initialization in 'mma_chip_info_table' would set this >> 'supported events' field for each chip. >> But I wonder should I add those changes as part of this patch? > is it necessary or can it be documentation? > > And this patch should have been called "v2". please include a persistent version history to v3 of this patch. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] iio: accel: mma8452: Bugfix to enbale and allow different events to work parallely. 2017-08-17 11:55 ` Harinath Nampally @ 2017-08-17 14:40 ` Jonathan Cameron 2017-08-19 0:05 ` Harinath Nampally 0 siblings, 1 reply; 7+ messages in thread From: Jonathan Cameron @ 2017-08-17 14:40 UTC (permalink / raw) To: Harinath Nampally Cc: Martin Kepplinger, Peter Meerwald-Stadler, knaack.h, lars, gregkh, linux-iio, linux-kernel, amsfield22 On Thu, 17 Aug 2017 07:55:45 -0400 Harinath Nampally <harinath922@gmail.com> wrote: > > This patch fixes by detaching the event related information from > > chip_info struct, > >>> and based on channel type and event direction the corresponding > > event configuration registers > >>> are picked dynamically. Hence multiple events can be handled in > > read/write callbacks. > >> which chip can have which event(s)? > > I am planning to add 'supported events' field in One small point. Don't put the word bugfix in the title (and fix spelling of enable!). I know this is obviously a false restriction on the driver, but it doesn't not work, it is just limited in features without this. This issue is that this is not really material that should be going into stable kernels. It's an improvement though so good to have it! Jonathan > > > > struct mma_chip_info which indicates which chip can have which events. > > During initialization in 'mma_chip_info_table' would set this > > 'supported events' field for each chip. > > But I wonder should I add those changes as part of this patch? > > is it necessary or can it be documentation? > I think its not necessary as we only have Freefall and Transient events > for now. > Ok I will just update the documentation. > > > > And this patch should have been called "v2". please include a persistent version history to v3 of this patch. > Sure I will send v3 patch, should I use '--in-reply-to' option of git > send-email to send v3 patch as reply to > original thread? > > On 08/17/2017 07:24 AM, Martin Kepplinger wrote: > >>>> This patch fixes by detaching the event related information from > >> chip_info struct, > >>>> and based on channel type and event direction the corresponding > >> event configuration registers > >>>> are picked dynamically. Hence multiple events can be handled in > >> read/write callbacks. > >>> which chip can have which event(s)? > >> I am planning to add 'supported events' field in > >> > >> struct mma_chip_info which indicates which chip can have which events. > >> During initialization in 'mma_chip_info_table' would set this > >> 'supported events' field for each chip. > >> But I wonder should I add those changes as part of this patch? > > is it necessary or can it be documentation? > > > > And this patch should have been called "v2". please include a persistent version history to v3 of this patch. > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] iio: accel: mma8452: Bugfix to enbale and allow different events to work parallely. 2017-08-17 14:40 ` Jonathan Cameron @ 2017-08-19 0:05 ` Harinath Nampally 0 siblings, 0 replies; 7+ messages in thread From: Harinath Nampally @ 2017-08-19 0:05 UTC (permalink / raw) To: Jonathan Cameron Cc: Martin Kepplinger, Peter Meerwald-Stadler, knaack.h, lars, gregkh, linux-iio, linux-kernel, amsfield22 > This patch fixes by detaching the event related information from > chip_info struct, >>> and based on channel type and event direction the corresponding > event configuration registers >>> are picked dynamically. Hence multiple events can be handled in > read/write callbacks. >> which chip can have which event(s)? > I am planning to add 'supported events' field in > One small point. Don't put the word bugfix in the title (and fix > spelling of enable!). I know this is obviously a false restriction > on the driver, but it doesn't not work, it is just limited in features > without this. Sure, thanks for letting me know. On 08/17/2017 10:40 AM, Jonathan Cameron wrote: >>> This patch fixes by detaching the event related information from >>> chip_info struct, >>>>> and based on channel type and event direction the corresponding >>> event configuration registers >>>>> are picked dynamically. Hence multiple events can be handled in >>> read/write callbacks. >>>> which chip can have which event(s)? >>> I am planning to add 'supported events' field in > One small point. Don't put the word bugfix in the title (and fix > spelling of enable!). I know this is obviously a false restriction > on the driver, but it doesn't not work, it is just limited in features > without this. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-08-19 0:05 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-08-14 22:44 [PATCH] iio: accel: mma8452: Bugfix to enbale and allow different events to work parallely Harinath Nampally 2017-08-16 13:12 ` Peter Meerwald-Stadler 2017-08-17 0:39 ` Harinath Nampally 2017-08-17 11:24 ` Martin Kepplinger 2017-08-17 11:55 ` Harinath Nampally 2017-08-17 14:40 ` Jonathan Cameron 2017-08-19 0:05 ` Harinath Nampally
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).