Re: [PATCH] iio: pressure: rohm-bm1390: notify trigger on all error paths

[David Lechner]

On 5/17/26 11:08 AM, Stepan Ionichev wrote:
> bm1390_trigger_handler() has three error returns:
> 
> 	if (ret || !status)
> 		return IRQ_NONE;          /* status read failed */
> 	...
> 	if (ret) {
> 		dev_warn(...);
> 		return IRQ_NONE;          /* pressure read failed */
> 	}
> 	...
> 	if (ret) {
> 		dev_warn(...);
> 		return IRQ_HANDLED;       /* temp read failed */
> 	}
> 
> None of them call iio_trigger_notify_done(). The success path at the
> end does, so on a single transient regmap or pressure-read error the
> trigger never sees its use_count decremented, and the
> !atomic_read(&trig->use_count) guard in iio_trigger_poll_chained()
> drops every subsequent dispatch for that trigger. The buffered-data
> flow stays wedged until the trigger is detached.
> 
> The IRQ_HANDLED return on the temperature path additionally leaves
> the temp branch's last partial state in &data->buf.temp without
> pushing the sample, which is the existing intended behaviour; only
> the missing notify_done() needs fixing.
> 
> Funnel all returns through a single 'done' label that calls
> iio_trigger_notify_done() before returning the saved irqreturn_t.
> 
> Fixes: 81ca5979b6ed ("iio: pressure: Support ROHM BU1390")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Stepan Ionichev <sozdayvek@xxxxxxxxx>
> ---
>  drivers/iio/pressure/rohm-bm1390.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/iio/pressure/rohm-bm1390.c b/drivers/iio/pressure/rohm-bm1390.c
> index 08146ca0f..c18352399 100644
> --- a/drivers/iio/pressure/rohm-bm1390.c
> +++ b/drivers/iio/pressure/rohm-bm1390.c
> @@ -626,12 +626,15 @@ static irqreturn_t bm1390_trigger_handler(int irq, void *p)
>  	struct iio_poll_func *pf = p;
>  	struct iio_dev *idev = pf->indio_dev;
>  	struct bm1390_data *data = iio_priv(idev);
> +	irqreturn_t result = IRQ_HANDLED;
>  	int ret, status;
>  
>  	/* DRDY is acked by reading status reg */
>  	ret = regmap_read(data->regmap, BM1390_REG_STATUS, &status);
> -	if (ret || !status)
> -		return IRQ_NONE;
> +	if (ret || !status) {
> +		result = IRQ_NONE;

IRQ_NONE means that the interrupt wasn't handled, so it won't be cleared
and the handler will likely just run again immediately. So it probably
isn't the right thing to be returning in the first place.

> +		goto done;
> +	}
>  
>  	dev_dbg(data->dev, "DRDY trig status 0x%x\n", status);
>  
> @@ -639,7 +642,8 @@ static irqreturn_t bm1390_trigger_handler(int irq, void *p)
>  		ret = bm1390_pressure_read(data, &data->buf.pressure);
>  		if (ret) {
>  			dev_warn(data->dev, "sample read failed %d\n", ret);
> -			return IRQ_NONE;
> +			result = IRQ_NONE;
> +			goto done;
>  		}
>  	}
>  
> @@ -648,15 +652,16 @@ static irqreturn_t bm1390_trigger_handler(int irq, void *p)
>  				       &data->buf.temp, sizeof(data->buf.temp));
>  		if (ret) {
>  			dev_warn(data->dev, "temp read failed %d\n", ret);
> -			return IRQ_HANDLED;
> +			goto done;
>  		}
>  	}
>  
>  	iio_push_to_buffers_with_ts(idev, &data->buf, sizeof(data->buf),
>  				    data->timestamp);
> +done:
>  	iio_trigger_notify_done(idev->trig);
>  
> -	return IRQ_HANDLED;
> +	return result;
>  }
>  
>  /* Get timestamps and wake the thread if we need to read data */