Re: [PATCH v3 2/2] iio: gyro: mpu3050: Fix runtime PM refcount leak in trigger enable path

[Jonathan Cameron]

On Tue,  3 Nov 2026 17:20:02 +0800
Xiangwu Lai <laixiangwu@xxxxxxxxxxxxxxxxxxxx> wrote:

> From: laixiangwu <laixiangwu@xxxxxxxxxxxxxxxxxxxx>
> 
> mpu3050_drdy_trigger_enable() calls pm_runtime_get_sync() when
> enabling the data ready trigger, but all five error returns in the
> enable path return directly without the matching
> pm_runtime_put_autosuspend(). The disable path, in contrast, drops
> the reference correctly.
> 
> As a result, a failed trigger enable leaves the runtime PM usage
> counter incremented and the sensor can never runtime suspend. It also
> leaves hw_irq_trigger set to true, claiming the hardware IRQ is in
> use when it is not.
> 
> Use pm_runtime_resume_and_get(), which does not leave the usage
> counter incremented when resume fails, and add an error label that
> drops the runtime PM reference and clears hw_irq_trigger so that a
> failed enable returns with the device in the same state as before
> the attempt.
> 
> Fixes: 3904b28efb2c ("iio: gyro: Add driver for the MPU-3050 gyroscope")
> Signed-off-by: laixiangwu <laixiangwu@xxxxxxxxxxxxxxxxxxxx>

Other than the missing error return I called out in the previous patch this
looks fine.

> ---
> 
> Changes in v3:
> - none; reposted in a new top-level thread.
> 
> Changes in v2:
> - Rebase the fix on a new precursor patch that splits the enable and
>   disable paths of mpu3050_drdy_trigger_set_state() into separate
>   helpers, as suggested by the reviewer, so the error label only
>   applies to the enable path.
> - Use pm_runtime_resume_and_get() instead of pm_runtime_get_sync(),
>   as suggested by the reviewer; it also keeps the usage counter
>   balanced when the resume fails, so no manual
>   pm_runtime_put_noidle() is needed in the caller.
> 
> Link: https://lore.kernel.org/r/20261101092327.197-3-laixiangwu@xxxxxxxxxxxxxxxxxxxx
> Link: https://lore.kernel.org/r/20261030084305.1660-1-laixiangwu@xxxxxxxxxxxxxxxxxxxx
>  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 d966911ab..fc305564d 100644
> --- a/drivers/iio/gyro/mpu3050-core.c
> +++ b/drivers/iio/gyro/mpu3050-core.c
> @@ -957,20 +957,23 @@ static int mpu3050_drdy_trigger_enable(struct mpu3050 *mpu3050)
>  	unsigned int val;
>  	int ret;
>  
> -	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_put_pm;
>  
>  	/* 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_put_pm;
>  
>  	mpu3050->pending_fifo_footer = false;
>  
> @@ -982,12 +985,12 @@ static int mpu3050_drdy_trigger_enable(struct mpu3050 *mpu3050)
>  			   MPU3050_FIFO_EN_GYRO_ZOUT |
>  			   MPU3050_FIFO_EN_FOOTER);
>  	if (ret)
> -		return ret;
> +		goto err_put_pm;
>  
>  	/* Configure the sample engine */
>  	ret = mpu3050_start_sampling(mpu3050);
>  	if (ret)
> -		return ret;
> +		goto err_put_pm;
>  
>  	/* Clear IRQ flag */
>  	ret = regmap_read(mpu3050->map, MPU3050_INT_STATUS, &val);
> @@ -1006,9 +1009,15 @@ static int mpu3050_drdy_trigger_enable(struct mpu3050 *mpu3050)
>  
>  	ret = regmap_write(mpu3050->map, MPU3050_INT_CFG, val);
>  	if (ret)
> -		return ret;
> +		goto err_put_pm;
>  
>  	return 0;
> +
> +err_put_pm:
> +	pm_runtime_put_autosuspend(mpu3050->dev);
> +	mpu3050->hw_irq_trigger = false;
> +
> +	return ret;
>  }
>  
>  /**