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

[Matti Vaittinen]

On 18/05/2026 12:42, Stepan Ionichev wrote:
> bm1390_trigger_handler() returns from three error paths without
> calling iio_trigger_notify_done(). The success path at the end
> does, so on a single transient regmap or read failure the trigger
> use_count is never decremented, and the !atomic_read(&trig->use_count)
> guard in iio_trigger_poll_chained() drops every subsequent dispatch.
> The buffered-data flow stays wedged until the trigger is detached.

I still believe the use-count should be decremented by the IIO, after it 
has called trigger handlers. (Unless there is an use-case where the 
use-count is not decremented.) Well, let's wait for a little while so 
Jonathan & others have time to comment. I have been wrong at times ;)

> Funnel all returns through a single done label that calls
> iio_trigger_notify_done() and reports the outcome via IRQ_RETVAL().
>
> Fixes: 81ca5979b6ed ("iio: pressure: Support ROHM BU1390")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Stepan Ionichev <sozdayvek@xxxxxxxxx>
> ---
> v2:
> - Use a bool and IRQ_RETVAL() instead of irqreturn_t (Andy)
>
> v1: https://lore.kernel.org/all/20260517160801.269-1-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..81368e578 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);
> +	bool handled = true;

I would inverse the logic. At this point, the IRQ is _not_ handled. 
Hence I'd default this false and only toggled it to true when the IRQ is 
indeed successfully acked and data is read. That should allow you to 
touch the 'handled' only once after the initialization.

>   	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) {
> +		handled = false;
> +		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;
> +			handled = false;
> +			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 IRQ_RETVAL(handled);
>   }
>   
>   /* Get timestamps and wake the thread if we need to read data */


Yours,
	-- Matti

---
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

~~ When things go utterly wrong vim users can always type :help! ~~