diff mbox series

[v4,6/6] iio: imu: st_lsm6dsx: prohibit the use of events and buffered reads simultaneously

Message ID 20190906121716.66194-6-sean@geanix.com (mailing list archive)
State New, archived
Headers show
Series [v4,1/6] iio: imu: st_lsm6dsx: move interrupt thread to core | expand

Commit Message

Sean Nyekjaer Sept. 6, 2019, 12:17 p.m. UTC
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>
---
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c | 3 +++
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c   | 3 +++
 2 files changed, 6 insertions(+)

Comments

Jonathan Cameron Sept. 7, 2019, 10:54 a.m. UTC | #1
On Fri,  6 Sep 2019 14:17:16 +0200
Sean Nyekjaer <sean@geanix.com> wrote:

> 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>
> ---
>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c | 3 +++
>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c   | 3 +++
>  2 files changed, 6 insertions(+)
> 
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> index ef579650fd52..94e8884a1db1 100644
> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> @@ -601,6 +601,9 @@ int st_lsm6dsx_update_fifo(struct st_lsm6dsx_sensor *sensor, bool enable)
>  	struct st_lsm6dsx_hw *hw = sensor->hw;
>  	int err;
>  
> +	if (hw->enable_event)
> +		return -EBUSY;
> +

This strikes me as racey.  What stops you getting past the check and before
you get to the next line of code, an event is enabled? 

More than likely I'm missing a reason that can't happen, but a comment
here to explain why will make it immediately obvious!

>  	mutex_lock(&hw->conf_lock);
>  
>  	if (hw->fifo_mode != ST_LSM6DSX_FIFO_BYPASS) {
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> index 470821b54933..fdc44ff9601b 100644
> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> @@ -1277,6 +1277,9 @@ 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;

Again, looks superficially racey.

> +
>  	if (state && hw->enable_event)
>  		return 0;
>
diff mbox series

Patch

diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
index ef579650fd52..94e8884a1db1 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
@@ -601,6 +601,9 @@  int st_lsm6dsx_update_fifo(struct st_lsm6dsx_sensor *sensor, bool enable)
 	struct st_lsm6dsx_hw *hw = sensor->hw;
 	int err;
 
+	if (hw->enable_event)
+		return -EBUSY;
+
 	mutex_lock(&hw->conf_lock);
 
 	if (hw->fifo_mode != ST_LSM6DSX_FIFO_BYPASS) {
diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
index 470821b54933..fdc44ff9601b 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
@@ -1277,6 +1277,9 @@  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;
+
 	if (state && hw->enable_event)
 		return 0;