[PATCH v2] iio: proximity: vl53l0x: notify trigger and clear IRQ on error paths

Stepan Ionichev posted 1 patch 4 weeks ago
drivers/iio/proximity/vl53l0x-i2c.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
[PATCH v2] iio: proximity: vl53l0x: notify trigger and clear IRQ on error paths
Posted by Stepan Ionichev 4 weeks ago
vl53l0x_trigger_handler() returns directly on the I2C read failure
paths without calling iio_trigger_notify_done() or vl53l0x_clear_irq().

A single transient i2c_smbus_read_i2c_block_data() failure (negative
errno or a short read) therefore leaves two pieces of state behind:

  - iio_trigger_notify_done() never decrements the trigger's use_count,
    so iio_trigger_poll_nested() silently drops further dispatches
    (see industrialio-trigger.c, the !atomic_read(&trig->use_count)
    guard);
  - vl53l0x_clear_irq() never writes SYSTEM_INTERRUPT_CLEAR, so the
    chip keeps the DRDY interrupt asserted.

The sensor's buffer mode stays wedged from then on, recoverable only
by re-binding the driver. The sibling driver vl53l1x-i2c.c handles
exactly the same case correctly by jumping to a "notify_and_clear_irq"
label that always calls both helpers; mirror that here.

The bogus negative-int return value cast to irqreturn_t also goes
away as a side effect.

Fixes: 762186c6e7b1 ("iio: proximity: vl53l0x-i2c: Added continuous mode support")
Signed-off-by: Stepan Ionichev <sozdayvek@gmail.com>
---
v2:
- Drop the cited error-path code from the commit body; the diff
  already shows it (per Andy)

 drivers/iio/proximity/vl53l0x-i2c.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/proximity/vl53l0x-i2c.c b/drivers/iio/proximity/vl53l0x-i2c.c
index ad3e46d47..7acb94acc 100644
--- a/drivers/iio/proximity/vl53l0x-i2c.c
+++ b/drivers/iio/proximity/vl53l0x-i2c.c
@@ -87,15 +87,14 @@ static irqreturn_t vl53l0x_trigger_handler(int irq, void *priv)
 	ret = i2c_smbus_read_i2c_block_data(data->client,
 					VL_REG_RESULT_RANGE_STATUS,
 					sizeof(buffer), buffer);
-	if (ret < 0)
-		return ret;
-	else if (ret != 12)
-		return -EREMOTEIO;
+	if (ret < 0 || ret != 12)
+		goto done;
 
 	scan.chan = get_unaligned_be16(&buffer[10]);
 	iio_push_to_buffers_with_ts(indio_dev, &scan, sizeof(scan),
 				    iio_get_time_ns(indio_dev));
 
+done:
 	iio_trigger_notify_done(indio_dev->trig);
 	vl53l0x_clear_irq(data);
 
-- 
2.43.0
Re: [PATCH v2] iio: proximity: vl53l0x: notify trigger and clear IRQ on error paths
Posted by David Lechner 3 weeks, 5 days ago
On 5/14/26 9:37 AM, Stepan Ionichev wrote:
> vl53l0x_trigger_handler() returns directly on the I2C read failure
> paths without calling iio_trigger_notify_done() or vl53l0x_clear_irq().
> 
> A single transient i2c_smbus_read_i2c_block_data() failure (negative
> errno or a short read) therefore leaves two pieces of state behind:
> 
>   - iio_trigger_notify_done() never decrements the trigger's use_count,
>     so iio_trigger_poll_nested() silently drops further dispatches
>     (see industrialio-trigger.c, the !atomic_read(&trig->use_count)
>     guard);
>   - vl53l0x_clear_irq() never writes SYSTEM_INTERRUPT_CLEAR, so the
>     chip keeps the DRDY interrupt asserted.
> 
> The sensor's buffer mode stays wedged from then on, recoverable only
> by re-binding the driver. The sibling driver vl53l1x-i2c.c handles
> exactly the same case correctly by jumping to a "notify_and_clear_irq"
> label that always calls both helpers; mirror that here.
> 
> The bogus negative-int return value cast to irqreturn_t also goes
> away as a side effect.
> 
> Fixes: 762186c6e7b1 ("iio: proximity: vl53l0x-i2c: Added continuous mode support")
> Signed-off-by: Stepan Ionichev <sozdayvek@gmail.com>
> ---
> v2:
> - Drop the cited error-path code from the commit body; the diff
>   already shows it (per Andy)
> 
>  drivers/iio/proximity/vl53l0x-i2c.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iio/proximity/vl53l0x-i2c.c b/drivers/iio/proximity/vl53l0x-i2c.c
> index ad3e46d47..7acb94acc 100644
> --- a/drivers/iio/proximity/vl53l0x-i2c.c
> +++ b/drivers/iio/proximity/vl53l0x-i2c.c
> @@ -87,15 +87,14 @@ static irqreturn_t vl53l0x_trigger_handler(int irq, void *priv)
>  	ret = i2c_smbus_read_i2c_block_data(data->client,
>  					VL_REG_RESULT_RANGE_STATUS,
>  					sizeof(buffer), buffer);
> -	if (ret < 0)
> -		return ret;
> -	else if (ret != 12)
> -		return -EREMOTEIO;
> +	if (ret < 0 || ret != 12)

This can just be ret != 12.

> +		goto done;
>  
>  	scan.chan = get_unaligned_be16(&buffer[10]);
>  	iio_push_to_buffers_with_ts(indio_dev, &scan, sizeof(scan),
>  				    iio_get_time_ns(indio_dev));
>  
> +done:
>  	iio_trigger_notify_done(indio_dev->trig);
>  	vl53l0x_clear_irq(data);
>
Re: [PATCH v2] iio: proximity: vl53l0x: notify trigger and clear IRQ on error paths
Posted by Jonathan Cameron 3 weeks, 5 days ago
On Sat, 16 May 2026 10:38:02 -0500
David Lechner <dlechner@baylibre.com> wrote:

> On 5/14/26 9:37 AM, Stepan Ionichev wrote:
> > vl53l0x_trigger_handler() returns directly on the I2C read failure
> > paths without calling iio_trigger_notify_done() or vl53l0x_clear_irq().
> > 
> > A single transient i2c_smbus_read_i2c_block_data() failure (negative
> > errno or a short read) therefore leaves two pieces of state behind:
> > 
> >   - iio_trigger_notify_done() never decrements the trigger's use_count,
> >     so iio_trigger_poll_nested() silently drops further dispatches
> >     (see industrialio-trigger.c, the !atomic_read(&trig->use_count)
> >     guard);
> >   - vl53l0x_clear_irq() never writes SYSTEM_INTERRUPT_CLEAR, so the
> >     chip keeps the DRDY interrupt asserted.
> > 
> > The sensor's buffer mode stays wedged from then on, recoverable only
> > by re-binding the driver. The sibling driver vl53l1x-i2c.c handles
> > exactly the same case correctly by jumping to a "notify_and_clear_irq"
> > label that always calls both helpers; mirror that here.
> > 
> > The bogus negative-int return value cast to irqreturn_t also goes
> > away as a side effect.
> > 
> > Fixes: 762186c6e7b1 ("iio: proximity: vl53l0x-i2c: Added continuous mode support")
> > Signed-off-by: Stepan Ionichev <sozdayvek@gmail.com>
> > ---
> > v2:
> > - Drop the cited error-path code from the commit body; the diff
> >   already shows it (per Andy)
> > 
> >  drivers/iio/proximity/vl53l0x-i2c.c | 7 +++----
> >  1 file changed, 3 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/iio/proximity/vl53l0x-i2c.c b/drivers/iio/proximity/vl53l0x-i2c.c
> > index ad3e46d47..7acb94acc 100644
> > --- a/drivers/iio/proximity/vl53l0x-i2c.c
> > +++ b/drivers/iio/proximity/vl53l0x-i2c.c
> > @@ -87,15 +87,14 @@ static irqreturn_t vl53l0x_trigger_handler(int irq, void *priv)
> >  	ret = i2c_smbus_read_i2c_block_data(data->client,
> >  					VL_REG_RESULT_RANGE_STATUS,
> >  					sizeof(buffer), buffer);
> > -	if (ret < 0)
> > -		return ret;
> > -	else if (ret != 12)
> > -		return -EREMOTEIO;
> > +	if (ret < 0 || ret != 12)
> 
> This can just be ret != 12.
Doh. I missed that completely.  Tweaked.

> 
> > +		goto done;
> >  
> >  	scan.chan = get_unaligned_be16(&buffer[10]);
> >  	iio_push_to_buffers_with_ts(indio_dev, &scan, sizeof(scan),
> >  				    iio_get_time_ns(indio_dev));
> >  
> > +done:
> >  	iio_trigger_notify_done(indio_dev->trig);
> >  	vl53l0x_clear_irq(data);
> >  
>
Re: [PATCH v2] iio: proximity: vl53l0x: notify trigger and clear IRQ on error paths
Posted by Jonathan Cameron 3 weeks, 6 days ago
On Thu, 14 May 2026 19:37:10 +0500
Stepan Ionichev <sozdayvek@gmail.com> wrote:

> vl53l0x_trigger_handler() returns directly on the I2C read failure
> paths without calling iio_trigger_notify_done() or vl53l0x_clear_irq().
> 
> A single transient i2c_smbus_read_i2c_block_data() failure (negative
> errno or a short read) therefore leaves two pieces of state behind:
> 
>   - iio_trigger_notify_done() never decrements the trigger's use_count,
>     so iio_trigger_poll_nested() silently drops further dispatches
>     (see industrialio-trigger.c, the !atomic_read(&trig->use_count)
>     guard);
>   - vl53l0x_clear_irq() never writes SYSTEM_INTERRUPT_CLEAR, so the
>     chip keeps the DRDY interrupt asserted.
> 
> The sensor's buffer mode stays wedged from then on, recoverable only
> by re-binding the driver. The sibling driver vl53l1x-i2c.c handles
> exactly the same case correctly by jumping to a "notify_and_clear_irq"
> label that always calls both helpers; mirror that here.
> 
> The bogus negative-int return value cast to irqreturn_t also goes
> away as a side effect.
> 
> Fixes: 762186c6e7b1 ("iio: proximity: vl53l0x-i2c: Added continuous mode support")
> Signed-off-by: Stepan Ionichev <sozdayvek@gmail.com>
Applied and marked for stable, not so much because it wedges (as probably
won't recover anyway) but more to get rid of a negative error return in an
irq handler.


> ---
> v2:
> - Drop the cited error-path code from the commit body; the diff
>   already shows it (per Andy)
> 
>  drivers/iio/proximity/vl53l0x-i2c.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iio/proximity/vl53l0x-i2c.c b/drivers/iio/proximity/vl53l0x-i2c.c
> index ad3e46d47..7acb94acc 100644
> --- a/drivers/iio/proximity/vl53l0x-i2c.c
> +++ b/drivers/iio/proximity/vl53l0x-i2c.c
> @@ -87,15 +87,14 @@ static irqreturn_t vl53l0x_trigger_handler(int irq, void *priv)
>  	ret = i2c_smbus_read_i2c_block_data(data->client,
>  					VL_REG_RESULT_RANGE_STATUS,
>  					sizeof(buffer), buffer);
> -	if (ret < 0)
> -		return ret;
> -	else if (ret != 12)
> -		return -EREMOTEIO;
> +	if (ret < 0 || ret != 12)
> +		goto done;
>  
>  	scan.chan = get_unaligned_be16(&buffer[10]);
>  	iio_push_to_buffers_with_ts(indio_dev, &scan, sizeof(scan),
>  				    iio_get_time_ns(indio_dev));
>  
> +done:
>  	iio_trigger_notify_done(indio_dev->trig);
>  	vl53l0x_clear_irq(data);
>