On Jul 11, Jorge Ramirez-Ortiz wrote:
> 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
do you mean something like (just compiled, not tested):
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
@@ -298,8 +298,11 @@ static int st_lsm6dsx_read_fifo(struct st_lsm6dsx_hw *hw)
err = regmap_bulk_read(hw->regmap,
hw->settings->fifo_ops.fifo_diff.addr,
&fifo_status, sizeof(fifo_status));
- if (err < 0)
+ if (err < 0) {
+ dev_err(hw->dev, "failed to read fifo status reg (err=%d)\n",
+ err);
return err;
+ }
if (fifo_status & cpu_to_le16(ST_LSM6DSX_FIFO_EMPTY_MASK))
return 0;
@@ -313,8 +316,12 @@ static int st_lsm6dsx_read_fifo(struct st_lsm6dsx_hw *hw)
for (read_len = 0; read_len < fifo_len; read_len += pattern_len) {
err = st_lsm6dsx_read_block(hw, hw->buff, pattern_len);
- if (err < 0)
+ if (err < 0) {
+ dev_err(hw->dev,
+ "failed to read pattern from fifo (err=%d)\n",
+ err);
return err;
+ }
/*
* Data are written to the FIFO with a specific pattern
@@ -385,8 +392,11 @@ static int st_lsm6dsx_read_fifo(struct st_lsm6dsx_hw *hw)
if (unlikely(reset_ts)) {
err = st_lsm6dsx_reset_hw_ts(hw);
- if (err < 0)
+ if (err < 0) {
+ dev_err(hw->dev, "failed to reset hw ts (err=%d)\n",
+ err);
return err;
+ }
}
return read_len;
}
Regards,
Lorenzo
>
> thanks for coming back to me despite the bad patch
>
> >
> > Regards,
> > Lorenzo
> >
> > > Signed-off-by: Jorge Ramirez-Ortiz <jramirez@xxxxxxxxxxxx>
> > > ---
> > > 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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
