| Message ID | 20190904091732.112281-4-sean@geanix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3,1/6] iio: imu: st_lsm6dsx: move interrupt thread to core | expand |
> The interrupt source can come from multiple sources, > fifo and wake interrupts. > Enter interrupt thread to check which interrupt that has fired. > > Signed-off-by: Sean Nyekjaer <sean@geanix.com> > --- > Changes since v1: > * None > > Changes since v2: > * None > > drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 14 +++++++------- > 1 file changed, 7 insertions(+), 7 deletions(-) > > diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c > index bb72800bf99b..513506caa460 100644 > --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c > @@ -1613,19 +1613,19 @@ static struct iio_dev *st_lsm6dsx_alloc_iiodev(struct st_lsm6dsx_hw *hw, > > static irqreturn_t st_lsm6dsx_handler_irq(int irq, void *private) > { > - struct st_lsm6dsx_hw *hw = private; > - > - return hw->sip > 0 ? IRQ_WAKE_THREAD : IRQ_NONE; > + return IRQ_WAKE_THREAD; > } > > static irqreturn_t st_lsm6dsx_handler_thread(int irq, void *private) > { > struct st_lsm6dsx_hw *hw = private; > - int count; > + int count = 0; > > - mutex_lock(&hw->fifo_lock); > - count = hw->settings->fifo_ops.read_fifo(hw); > - mutex_unlock(&hw->fifo_lock); > + if (hw->sip > 0) { > + mutex_lock(&hw->fifo_lock); > + count = st_lsm6dsx_read_fifo(hw); > + mutex_unlock(&hw->fifo_lock); This chunck is unnecessary since read_fifo() will check number of available samples in the FIFO and moreover it is obviously broken since you are running st_lsm6dsx_read_fifo directly instead of using fifo_ops pointer. > + } > > return count ? IRQ_HANDLED : IRQ_NONE; > } > -- > 2.23.0 >
On 05/09/2019 08.34, Lorenzo Bianconi wrote: > This chunck is unnecessary since read_fifo() will check number of available > samples in the FIFO and moreover it is obviously broken since you are running > st_lsm6dsx_read_fifo directly instead of using fifo_ops pointer. Thanks for the review :-) I have missed that, I have spend a lot of time rebasing "iio: imu: st_lsm6dsx: move interrupt thread to core". It could be nice if that patch could be applied in the near future :-) /Sean
> > > On 05/09/2019 08.34, Lorenzo Bianconi wrote: > > This chunck is unnecessary since read_fifo() will check number of available > > samples in the FIFO and moreover it is obviously broken since you are running > > st_lsm6dsx_read_fifo directly instead of using fifo_ops pointer. > > Thanks for the review :-) > > I have missed that, I have spend a lot of time rebasing "iio: imu: > st_lsm6dsx: move interrupt thread to core". > It could be nice if that patch could be applied in the near future :-) > > /Sean I think Jonathan will prefer the patch to be part of the series where it is actually needed (so up to him). I am fine even if you repost the single patch adding requested changes Regards, Lorenzo
diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c index bb72800bf99b..513506caa460 100644 --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c @@ -1613,19 +1613,19 @@ static struct iio_dev *st_lsm6dsx_alloc_iiodev(struct st_lsm6dsx_hw *hw, static irqreturn_t st_lsm6dsx_handler_irq(int irq, void *private) { - struct st_lsm6dsx_hw *hw = private; - - return hw->sip > 0 ? IRQ_WAKE_THREAD : IRQ_NONE; + return IRQ_WAKE_THREAD; } static irqreturn_t st_lsm6dsx_handler_thread(int irq, void *private) { struct st_lsm6dsx_hw *hw = private; - int count; + int count = 0; - mutex_lock(&hw->fifo_lock); - count = hw->settings->fifo_ops.read_fifo(hw); - mutex_unlock(&hw->fifo_lock); + if (hw->sip > 0) { + mutex_lock(&hw->fifo_lock); + count = st_lsm6dsx_read_fifo(hw); + mutex_unlock(&hw->fifo_lock); + } return count ? IRQ_HANDLED : IRQ_NONE; }
The interrupt source can come from multiple sources, fifo and wake interrupts. Enter interrupt thread to check which interrupt that has fired. Signed-off-by: Sean Nyekjaer <sean@geanix.com> --- Changes since v1: * None Changes since v2: * None drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)