Re: [PATCH 3/4] iio: light: apds9960: use devm_pm_runtime_enable() to fix probe error path

[Jonathan Cameron]

On Fri, 29 May 2026 15:45:43 +0500
Stepan Ionichev <sozdayvek@xxxxxxxxx> wrote:

> apds9960_probe() calls pm_runtime_enable() and then several operations
> that may fail through goto error_power_down, ending with
> iio_device_register().  None of those error paths call
> pm_runtime_disable(), so the runtime PM enable_count leaks on probe
> failure and on subsequent rebind.
> 
> Switch to devm_pm_runtime_enable() so the enable (and the matching
> dont_use_autosuspend) are torn down automatically.  The probe error path
> on devm_pm_runtime_enable() itself is a plain return because no hardware
> has been powered on at that point.  The pm_runtime_disable() and
> pm_runtime_set_suspended() calls in apds9960_remove() are dropped; the
> devm action runs after .remove() and handles the teardown.
> 
> Signed-off-by: Stepan Ionichev <sozdayvek@xxxxxxxxx>
> ---
>  drivers/iio/light/apds9960.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/light/apds9960.c b/drivers/iio/light/apds9960.c
> index 785c5dbe2d08..edbf149b931e 100644
> --- a/drivers/iio/light/apds9960.c
> +++ b/drivers/iio/light/apds9960.c
> @@ -1074,7 +1074,9 @@ static int apds9960_probe(struct i2c_client *client)
>  	if (ret)
>  		goto error_power_down;
>  
> -	pm_runtime_enable(&client->dev);
> +	ret = devm_pm_runtime_enable(&client->dev);
> +	if (ret)
> +		return ret;

Leaves the power on.  And resolving that would have the same issue
as the previous two patches with being a mix of goto and devm cleanup.

Very strong advice on this is the moment anything at all needs to be in remove()
you have to stop using devm for remaining calls and unwind them by hand.

>  	pm_runtime_set_autosuspend_delay(&client->dev, 5000);
>  	pm_runtime_use_autosuspend(&client->dev);
>  
> @@ -1123,8 +1125,6 @@ static void apds9960_remove(struct i2c_client *client)
>  	struct apds9960_data *data = iio_priv(indio_dev);
>  
>  	iio_device_unregister(indio_dev);
> -	pm_runtime_disable(&client->dev);
> -	pm_runtime_set_suspended(&client->dev);
>  	apds9960_set_powermode(data, 0);
>  }
>