| Message ID | 1531297928-3824-1-git-send-email-jramirez@baylibre.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
> Currently IRQ_NONE is returned only when there is no data on the fifo. > > When there is no data on the fifo the driver can not push to the > buffers and therefore user space readers polling for data available > will not be awoken and continue to wait. > > This commit just extends the same semantics to fifo read errors. Hi Jorge, IRQ_NONE is used to indicate this interrupt is not intended for this driver (this could happen if the irq line is in open-drain). If the interrupt is for st_lsm6dsx I would prefer to return IRQ_HANDLED even in case of error. Regards, Lorenzo > > Signed-off-by: Jorge Ramirez-Ortiz <jramirez@baylibre.com> > --- > drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c > index 4994f92..4959923 100644 > --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c > @@ -472,7 +472,7 @@ static irqreturn_t st_lsm6dsx_handler_thread(int irq, void *private) > count = st_lsm6dsx_read_fifo(hw); > mutex_unlock(&hw->fifo_lock); > > - return !count ? IRQ_NONE : IRQ_HANDLED; > + return (!count || count < 0) ? IRQ_NONE : IRQ_HANDLED; > } > > static int st_lsm6dsx_buffer_preenable(struct iio_dev *iio_dev) > -- > 2.7.4 > -- To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 07/11/2018 02:29 PM, Lorenzo Bianconi wrote: >> Currently IRQ_NONE is returned only when there is no data on the fifo. >> >> When there is no data on the fifo the driver can not push to the >> buffers and therefore user space readers polling for data available >> will not be awoken and continue to wait. >> >> This commit just extends the same semantics to fifo read errors. > Hi Jorge, > > IRQ_NONE is used to indicate this interrupt is not intended for this driver > (this could happen if the irq line is in open-drain). If the interrupt is for > st_lsm6dsx I would prefer to return IRQ_HANDLED even in case of error. yes I understand. This was just a trivial attempt (I guess a really bad idea) to get some debug info (via /proc/irq/.../spurious) any time the driver read (spi/i2c) fails when processing the data ready irq. do you think it would make sense to add a dev_err to st_lsm6dsx_i2c_read/st_lsm6dsx_spi_read? at the moment the driver would fail silently thanks for coming back to me despite the bad patch > > Regards, > Lorenzo > >> Signed-off-by: Jorge Ramirez-Ortiz <jramirez@baylibre.com> >> --- >> drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c >> index 4994f92..4959923 100644 >> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c >> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c >> @@ -472,7 +472,7 @@ static irqreturn_t st_lsm6dsx_handler_thread(int irq, void *private) >> count = st_lsm6dsx_read_fifo(hw); >> mutex_unlock(&hw->fifo_lock); >> >> - return !count ? IRQ_NONE : IRQ_HANDLED; >> + return (!count || count < 0) ? IRQ_NONE : IRQ_HANDLED; >> } >> >> static int st_lsm6dsx_buffer_preenable(struct iio_dev *iio_dev) >> -- >> 2.7.4 >> -- To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> On 07/11/2018 02:29 PM, Lorenzo Bianconi wrote: > > > Currently IRQ_NONE is returned only when there is no data on the fifo. > > > > > > When there is no data on the fifo the driver can not push to the > > > buffers and therefore user space readers polling for data available > > > will not be awoken and continue to wait. > > > > > > This commit just extends the same semantics to fifo read errors. > > Hi Jorge, > > > > IRQ_NONE is used to indicate this interrupt is not intended for this driver > > (this could happen if the irq line is in open-drain). If the interrupt is for > > st_lsm6dsx I would prefer to return IRQ_HANDLED even in case of error. > > yes I understand. > > This was just a trivial attempt (I guess a really bad idea) to get some > debug info (via /proc/irq/.../spurious) any time the driver read (spi/i2c) > fails when processing the data ready irq. No worries, but it is not a good idea to add that info in irq/spurious > do you think it would make sense to add a dev_err to > st_lsm6dsx_i2c_read/st_lsm6dsx_spi_read? at the moment the driver would fail > silently Could be usefull, afaiu you need to know if fifo read fails for any reason, correct? > > thanks for coming back to me despite the bad patch No worries :) Regards, Lorenzo > > > > > Regards, > > Lorenzo > > > > > Signed-off-by: Jorge Ramirez-Ortiz <jramirez@baylibre.com> > > > --- > > > drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c > > > index 4994f92..4959923 100644 > > > --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c > > > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c > > > @@ -472,7 +472,7 @@ static irqreturn_t st_lsm6dsx_handler_thread(int irq, void *private) > > > count = st_lsm6dsx_read_fifo(hw); > > > mutex_unlock(&hw->fifo_lock); > > > - return !count ? IRQ_NONE : IRQ_HANDLED; > > > + return (!count || count < 0) ? IRQ_NONE : IRQ_HANDLED; > > > } > > > static int st_lsm6dsx_buffer_preenable(struct iio_dev *iio_dev) > > > -- > > > 2.7.4 > > > > -- To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c index 4994f92..4959923 100644 --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c @@ -472,7 +472,7 @@ static irqreturn_t st_lsm6dsx_handler_thread(int irq, void *private) count = st_lsm6dsx_read_fifo(hw); mutex_unlock(&hw->fifo_lock); - return !count ? IRQ_NONE : IRQ_HANDLED; + return (!count || count < 0) ? IRQ_NONE : IRQ_HANDLED; } static int st_lsm6dsx_buffer_preenable(struct iio_dev *iio_dev)
Currently IRQ_NONE is returned only when there is no data on the fifo. When there is no data on the fifo the driver can not push to the buffers and therefore user space readers polling for data available will not be awoken and continue to wait. This commit just extends the same semantics to fifo read errors. Signed-off-by: Jorge Ramirez-Ortiz <jramirez@baylibre.com> --- drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)