Re: [PATCH] iio: gyro: mpu3050: Fix runtime PM leak on trigger errors

[Jonathan Cameron <jic23@kernel.org>]

On Fri, 14 Aug 2026 21:41:11 +0800
Ruoyu Wang <ruoyuw560@gmail.com> wrote:

> The first user of the MPU-3050 data-ready trigger takes a runtime PM
> reference before configuring the FIFO, sample engine and interrupt. If
> any of those operations fails, iio_trigger_attach_poll_func() tears down
> its IRQ resources without calling set_trigger_state(false). The buffer
> error path then releases only its preenable reference, leaving the
> trigger's reference held and preventing runtime suspend.
> 
> Use pm_runtime_resume_and_get() so a resume failure does not leave a
> usage count behind. Route later setup failures through a common unwind
> that clears hw_irq_trigger and drops the trigger's reference. Successful
> enable and disable behavior is unchanged.
> 
> This issue was found by a static analysis checker and confirmed by
> manual source review.
> 
> Fixes: f11d59d87b8622 ("iio: Move attach/detach of the poll func to the core")
> Signed-off-by: Ruoyu Wang <ruoyuw560@gmail.com>
> ---
>  drivers/iio/gyro/mpu3050-core.c | 21 +++++++++++++++------
>  1 file changed, 15 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/iio/gyro/mpu3050-core.c b/drivers/iio/gyro/mpu3050-core.c
> index d84e04e4b4314..33a98a6e9cb84 100644
> --- a/drivers/iio/gyro/mpu3050-core.c
> +++ b/drivers/iio/gyro/mpu3050-core.c
> @@ -988,20 +988,23 @@ static int mpu3050_drdy_trigger_set_state(struct iio_trigger *trig,
>  		return 0;
>  	} else {
>  		/* Else we're enabling the trigger from this point */
> -		pm_runtime_get_sync(mpu3050->dev);
> +		ret = pm_runtime_resume_and_get(mpu3050->dev);
> +		if (ret)
> +			return ret;
> +
>  		mpu3050->hw_irq_trigger = true;
>  
>  		/* Disable all things in the FIFO */
>  		ret = regmap_write(mpu3050->map, MPU3050_FIFO_EN, 0);
>  		if (ret)
> -			return ret;
> +			goto err_pm_put;

All these gotos are rather ugly.  I would consider using a helper
function so there is something like

		ret = mpu3050_dataready_do_enable();
		if (ret) {
			mpu3050->hw_irq_trigger = false;	
			pm_runtime_put_autosuspend(mpu3050->dev);
			return ret;
		}
		...


>  
>  		/* Reset and enable the FIFO */
>  		ret = regmap_set_bits(mpu3050->map, MPU3050_USR_CTRL,
>  				      MPU3050_USR_CTRL_FIFO_EN |
>  				      MPU3050_USR_CTRL_FIFO_RST);
>  		if (ret)
> -			return ret;
> +			goto err_pm_put;
>  
>  		mpu3050->pending_fifo_footer = false;
>  
> @@ -1013,12 +1016,12 @@ static int mpu3050_drdy_trigger_set_state(struct iio_trigger *trig,
>  				   MPU3050_FIFO_EN_GYRO_ZOUT |
>  				   MPU3050_FIFO_EN_FOOTER);
>  		if (ret)
> -			return ret;
> +			goto err_pm_put;
>  
>  		/* Configure the sample engine */
>  		ret = mpu3050_start_sampling(mpu3050);
>  		if (ret)
> -			return ret;
> +			goto err_pm_put;
>  
>  		/* Clear IRQ flag */
>  		ret = regmap_read(mpu3050->map, MPU3050_INT_STATUS, &val);
> @@ -1037,10 +1040,16 @@ static int mpu3050_drdy_trigger_set_state(struct iio_trigger *trig,
>  
>  		ret = regmap_write(mpu3050->map, MPU3050_INT_CFG, val);
>  		if (ret)
> -			return ret;
> +			goto err_pm_put;
>  	}
>  
>  	return 0;
> +
> +err_pm_put:
> +	mpu3050->hw_irq_trigger = false;

Clearing this on a write failure is a change, so good to call that out
in the patch description.

> +	pm_runtime_put_autosuspend(mpu3050->dev);
> +
> +	return ret;
>  }
>  
>  static const struct iio_trigger_ops mpu3050_trigger_ops = {