[PATCH 1/3] iio: ltr501: Don't return error code in trigger handler

[PATCH 1/3] iio: ltr501: Don't return error code in trigger handler

[Lars-Peter Clausen]

IIO trigger handlers need to return one of the irqreturn_t values.
Returning an error code is not supported.

The ltr501 interrupt handler gets this right for most error paths, but
there is one case where it returns the error code.

In addition for this particular case the trigger handler does not call
`iio_trigger_notify_done()`. Which when not done keeps the triggered
disabled forever.

Modify the code so that the function returns a valid irqreturn_t value as
well as calling `iio_trigger_notify_done()` on all exit paths.

Fixes: 2690be905123 ("iio: Add Lite-On ltr501 ambient light / proximity sensor driver")
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/iio/light/ltr501.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/light/ltr501.c b/drivers/iio/light/ltr501.c
index 7e51aaac0bf8..b2983b1a9ed1 100644
--- a/drivers/iio/light/ltr501.c
+++ b/drivers/iio/light/ltr501.c
@@ -1275,7 +1275,7 @@ static irqreturn_t ltr501_trigger_handler(int irq, void *p)
 		ret = regmap_bulk_read(data->regmap, LTR501_ALS_DATA1,
 				       als_buf, sizeof(als_buf));
 		if (ret < 0)
-			return ret;
+			goto done;
 		if (test_bit(0, indio_dev->active_scan_mask))
 			scan.channels[j++] = le16_to_cpu(als_buf[1]);
 		if (test_bit(1, indio_dev->active_scan_mask))
-- 
2.20.1

[PATCH 2/3] iio: kxsd9: Don't return error code in trigger handler

[Lars-Peter Clausen]

IIO trigger handlers need to return one of the irqreturn_t values.
Returning an error code is not supported.

The kxsd9 interrupt handler returns an error code if reading the data
registers fails. In addition when exiting due to an error the trigger
handler does not call `iio_trigger_notify_done()`. Which when not done
keeps the triggered disabled forever.

Modify the code so that the function returns a valid irqreturn_t value as
well as calling `iio_trigger_notify_done()` on all exit paths.

Since we can't return the error code make sure to at least log it as part
of the error message.

Fixes: 0427a106a98a ("iio: accel: kxsd9: Add triggered buffer handling")
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/iio/accel/kxsd9.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/accel/kxsd9.c b/drivers/iio/accel/kxsd9.c
index 2faf85ca996e..552eba5e8b4f 100644
--- a/drivers/iio/accel/kxsd9.c
+++ b/drivers/iio/accel/kxsd9.c
@@ -224,14 +224,14 @@ static irqreturn_t kxsd9_trigger_handler(int irq, void *p)
 			       hw_values.chan,
 			       sizeof(hw_values.chan));
 	if (ret) {
-		dev_err(st->dev,
-			"error reading data\n");
-		return ret;
+		dev_err(st->dev, "error reading data: %d\n", ret);
+		goto out;
 	}
 
 	iio_push_to_buffers_with_timestamp(indio_dev,
 					   &hw_values,
 					   iio_get_time_ns(indio_dev));
+out:
 	iio_trigger_notify_done(indio_dev->trig);
 
 	return IRQ_HANDLED;
-- 
2.20.1

Re: [PATCH 2/3] iio: kxsd9: Don't return error code in trigger handler

[Linus Walleij]

On Sun, Oct 24, 2021 at 7:13 PM Lars-Peter Clausen <lars@metafoo.de> wrote:

> IIO trigger handlers need to return one of the irqreturn_t values.
> Returning an error code is not supported.
>
> The kxsd9 interrupt handler returns an error code if reading the data
> registers fails. In addition when exiting due to an error the trigger
> handler does not call `iio_trigger_notify_done()`. Which when not done
> keeps the triggered disabled forever.
>
> Modify the code so that the function returns a valid irqreturn_t value as
> well as calling `iio_trigger_notify_done()` on all exit paths.
>
> Since we can't return the error code make sure to at least log it as part
> of the error message.
>
> Fixes: 0427a106a98a ("iio: accel: kxsd9: Add triggered buffer handling")
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

[PATCH 3/3] iio: stk3310: Don't return error code in interrupt handler

[Lars-Peter Clausen]

Interrupt handlers must return one of the irqreturn_t values. Returning a
error code is not supported.

The stk3310 event interrupt handler returns an error code when reading the
flags register fails.

Fix the implementation to always return an irqreturn_t value.

Fixes: 3dd477acbdd1 ("iio: light: Add threshold interrupt support for STK3310")
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/iio/light/stk3310.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/light/stk3310.c b/drivers/iio/light/stk3310.c
index 07e91846307c..fc63856ed54d 100644
--- a/drivers/iio/light/stk3310.c
+++ b/drivers/iio/light/stk3310.c
@@ -546,9 +546,8 @@ static irqreturn_t stk3310_irq_event_handler(int irq, void *private)
 	mutex_lock(&data->lock);
 	ret = regmap_field_read(data->reg_flag_nf, &dir);
 	if (ret < 0) {
-		dev_err(&data->client->dev, "register read failed\n");
-		mutex_unlock(&data->lock);
-		return ret;
+		dev_err(&data->client->dev, "register read failed: %d\n", ret);
+		goto out;
 	}
 	event = IIO_UNMOD_EVENT_CODE(IIO_PROXIMITY, 1,
 				     IIO_EV_TYPE_THRESH,
@@ -560,6 +559,7 @@ static irqreturn_t stk3310_irq_event_handler(int irq, void *private)
 	ret = regmap_field_write(data->reg_flag_psint, 0);
 	if (ret < 0)
 		dev_err(&data->client->dev, "failed to reset interrupts\n");
+out:
 	mutex_unlock(&data->lock);
 
 	return IRQ_HANDLED;
-- 
2.20.1

Re: [PATCH 1/3] iio: ltr501: Don't return error code in trigger handler

[Jonathan Cameron]

On Sun, 24 Oct 2021 19:12:49 +0200
Lars-Peter Clausen <lars@metafoo.de> wrote:

> IIO trigger handlers need to return one of the irqreturn_t values.
> Returning an error code is not supported.
> 
> The ltr501 interrupt handler gets this right for most error paths, but
> there is one case where it returns the error code.
> 
> In addition for this particular case the trigger handler does not call
> `iio_trigger_notify_done()`. Which when not done keeps the triggered
> disabled forever.
> 
> Modify the code so that the function returns a valid irqreturn_t value as
> well as calling `iio_trigger_notify_done()` on all exit paths.
> 
> Fixes: 2690be905123 ("iio: Add Lite-On ltr501 ambient light / proximity sensor driver")
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Series applied to the fixes-togreg branch of iio.git and marked for stable.

Thanks,

Jonathan

> ---
>  drivers/iio/light/ltr501.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/light/ltr501.c b/drivers/iio/light/ltr501.c
> index 7e51aaac0bf8..b2983b1a9ed1 100644
> --- a/drivers/iio/light/ltr501.c
> +++ b/drivers/iio/light/ltr501.c
> @@ -1275,7 +1275,7 @@ static irqreturn_t ltr501_trigger_handler(int irq, void *p)
>  		ret = regmap_bulk_read(data->regmap, LTR501_ALS_DATA1,
>  				       als_buf, sizeof(als_buf));
>  		if (ret < 0)
> -			return ret;
> +			goto done;
>  		if (test_bit(0, indio_dev->active_scan_mask))
>  			scan.channels[j++] = le16_to_cpu(als_buf[1]);
>  		if (test_bit(1, indio_dev->active_scan_mask))