| Message ID | 20190909095618.70801-1-sean@geanix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | None | expand |
> When events and buffered reads is enabled simultaneously, and the first > event accours the interrupt pin stays high. > > This can be reverted when we find a solution to allow events and > buffered reads simultaneously. > > Signed-off-by: Sean Nyekjaer <sean@geanix.com> > --- > Changes since v4: > * Use fifo configuration mutex to prevent a race in hw->enable_event > check. > > Changes since v5: > * Updated do not return without unlocking mutexes > > drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c | 5 +++++ > drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 13 ++++++++++--- > 2 files changed, 15 insertions(+), 3 deletions(-) > > diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c > index ef579650fd52..b87a1872bc60 100644 > --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c > @@ -603,6 +603,11 @@ int st_lsm6dsx_update_fifo(struct st_lsm6dsx_sensor *sensor, bool enable) > > mutex_lock(&hw->conf_lock); > > + if (hw->enable_event) { > + err = -EBUSY; > + goto out; > + } > + > if (hw->fifo_mode != ST_LSM6DSX_FIFO_BYPASS) { > err = st_lsm6dsx_flush_fifo(hw); > if (err < 0) > diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c > index 00ba14d15c13..a13d0c154b82 100644 > --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c > @@ -1340,8 +1340,12 @@ static int st_lsm6dsx_write_event_config(struct iio_dev *iio_dev, > if (type != IIO_EV_TYPE_THRESH) > return -EINVAL; > > - if (hw->fifo_mode != ST_LSM6DSX_FIFO_BYPASS) > - return -EBUSY; > + mutex_unlock(&hw->conf_lock); I _really_ do not think you are testing these patches since there are plenty of errors here: 1- mutex_unlock instead of mutex_lock 2- returning without releasing the lock Please put more attention next time and test you patches Lorenzo > + > + if (hw->fifo_mode != ST_LSM6DSX_FIFO_BYPASS) { > + err = -EBUSY; > + goto out; > + } > > /* do not enable events if they are already enabled */ > if (state && hw->enable_event) > @@ -1357,7 +1361,10 @@ static int st_lsm6dsx_write_event_config(struct iio_dev *iio_dev, > > hw->enable_event = state; > > - return 0; > +out: > + mutex_unlock(&hw->conf_lock); > + > + return err; > } > > int st_lsm6dsx_set_watermark(struct iio_dev *iio_dev, unsigned int val) > -- > 2.23.0 >
On 09/09/2019 12.40, Lorenzo Bianconi wrote: > I_really_ do not think you are testing these patches since there are plenty of > errors here: > > 1- mutex_unlock instead of mutex_lock Again, my bad :) The error show quite quickly: WARNING: bad unlock balance detected! > 2- returning without releasing the lock > > Please put more attention next time and test you patches Will do... Thanks for the review /Sean
diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c index ef579650fd52..b87a1872bc60 100644 --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c @@ -603,6 +603,11 @@ int st_lsm6dsx_update_fifo(struct st_lsm6dsx_sensor *sensor, bool enable) mutex_lock(&hw->conf_lock); + if (hw->enable_event) { + err = -EBUSY; + goto out; + } + if (hw->fifo_mode != ST_LSM6DSX_FIFO_BYPASS) { err = st_lsm6dsx_flush_fifo(hw); if (err < 0) diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c index 00ba14d15c13..a13d0c154b82 100644 --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c @@ -1340,8 +1340,12 @@ static int st_lsm6dsx_write_event_config(struct iio_dev *iio_dev, if (type != IIO_EV_TYPE_THRESH) return -EINVAL; - if (hw->fifo_mode != ST_LSM6DSX_FIFO_BYPASS) - return -EBUSY; + mutex_unlock(&hw->conf_lock); + + if (hw->fifo_mode != ST_LSM6DSX_FIFO_BYPASS) { + err = -EBUSY; + goto out; + } /* do not enable events if they are already enabled */ if (state && hw->enable_event) @@ -1357,7 +1361,10 @@ static int st_lsm6dsx_write_event_config(struct iio_dev *iio_dev, hw->enable_event = state; - return 0; +out: + mutex_unlock(&hw->conf_lock); + + return err; } int st_lsm6dsx_set_watermark(struct iio_dev *iio_dev, unsigned int val)
When events and buffered reads is enabled simultaneously, and the first event accours the interrupt pin stays high. This can be reverted when we find a solution to allow events and buffered reads simultaneously. Signed-off-by: Sean Nyekjaer <sean@geanix.com> --- Changes since v4: * Use fifo configuration mutex to prevent a race in hw->enable_event check. Changes since v5: * Updated do not return without unlocking mutexes drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c | 5 +++++ drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 13 ++++++++++--- 2 files changed, 15 insertions(+), 3 deletions(-)