On 07/11/2018 05:26 PM, Lorenzo Bianconi wrote:
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
yes, you beat me to it but yes, that is what I was thinking about.
--
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
