[PATCH v2] iio: adc: ti-ads1015: use devm helpers to fix probe error paths

[PATCH v2] iio: adc: ti-ads1015: use devm helpers to fix probe error paths

[Stepan Ionichev]

Once ads1015_probe() gets past ads1015_set_conv_mode(CONTINUOUS), two
resources leak on its error paths and on driver unbind:

  - pm_runtime_enable() is left enabled if iio_device_register() fails;
    on subsequent probe/rebind the runtime PM tracking complains about
    an unbalanced enable.
  - The CONTINUOUS conversion mode written to the chip is never restored
    on any error path after that point, so on probe failure the chip is
    left running.

Convert all three teardown operations of ads1015_remove() to devm so
that the error paths and the unbind path share the same unwind in
reverse-of-setup order:

  - ads1015_set_conv_mode(SINGLESHOT) becomes a devm action registered
    immediately after CONTINUOUS mode is enabled.
  - pm_runtime_enable() becomes devm_pm_runtime_enable().
  - iio_device_register() becomes devm_iio_device_register(), so the
    iio device is unregistered first on unbind.

ads1015_remove() is no longer needed and is dropped.

Signed-off-by: Stepan Ionichev <sozdayvek@gmail.com>
---
v2:
- Full devm conversion: register a devm action for set_conv_mode(SINGLESHOT),
  switch to devm_iio_device_register, drop ads1015_remove() entirely.  This
  avoids mixing devm with manual unwind (Jonathan) and also fixes the
  pre-existing continuous-mode leak on probe failure.
- Drop the unrelated dev_err() -> dev_err_probe() conversion from v1 (Joshua).

v1: https://lore.kernel.org/all/20260529101011.3030-1-sozdayvek@gmail.com/

 drivers/iio/adc/ti-ads1015.c | 44 +++++++++++++++---------------------
 1 file changed, 18 insertions(+), 26 deletions(-)

diff --git a/drivers/iio/adc/ti-ads1015.c b/drivers/iio/adc/ti-ads1015.c
index c7ffe47449e2..d7f4d0d5bcf9 100644
--- a/drivers/iio/adc/ti-ads1015.c
+++ b/drivers/iio/adc/ti-ads1015.c
@@ -930,6 +930,17 @@ static int ads1015_set_conv_mode(struct ads1015_data *data, int mode)
 				  mode << ADS1015_CFG_MOD_SHIFT);
 }
 
+static void ads1015_power_down(void *p)
+{
+	struct ads1015_data *data = p;
+	int ret;
+
+	ret = ads1015_set_conv_mode(data, ADS1015_SINGLESHOT);
+	if (ret)
+		dev_warn(regmap_get_device(data->regmap),
+			 "Failed to power down (%pe)\n", ERR_PTR(ret));
+}
+
 static int ads1015_probe(struct i2c_client *client)
 {
 	const struct ads1015_chip_data *chip;
@@ -1030,6 +1041,10 @@ static int ads1015_probe(struct i2c_client *client)
 	if (ret)
 		return ret;
 
+	ret = devm_add_action_or_reset(&client->dev, ads1015_power_down, data);
+	if (ret)
+		return ret;
+
 	data->conv_invalid = true;
 
 	ret = pm_runtime_set_active(&client->dev);
@@ -1037,33 +1052,11 @@ static int ads1015_probe(struct i2c_client *client)
 		return ret;
 	pm_runtime_set_autosuspend_delay(&client->dev, ADS1015_SLEEP_DELAY_MS);
 	pm_runtime_use_autosuspend(&client->dev);
-	pm_runtime_enable(&client->dev);
-
-	ret = iio_device_register(indio_dev);
-	if (ret < 0) {
-		dev_err(&client->dev, "Failed to register IIO device\n");
+	ret = devm_pm_runtime_enable(&client->dev);
+	if (ret)
 		return ret;
-	}
-
-	return 0;
-}
 
-static void ads1015_remove(struct i2c_client *client)
-{
-	struct iio_dev *indio_dev = i2c_get_clientdata(client);
-	struct ads1015_data *data = iio_priv(indio_dev);
-	int ret;
-
-	iio_device_unregister(indio_dev);
-
-	pm_runtime_disable(&client->dev);
-	pm_runtime_set_suspended(&client->dev);
-
-	/* power down single shot mode */
-	ret = ads1015_set_conv_mode(data, ADS1015_SINGLESHOT);
-	if (ret)
-		dev_warn(&client->dev, "Failed to power down (%pe)\n",
-			 ERR_PTR(ret));
+	return devm_iio_device_register(&client->dev, indio_dev);
 }
 
 #ifdef CONFIG_PM
@@ -1150,7 +1143,6 @@ static struct i2c_driver ads1015_driver = {
 		.pm = &ads1015_pm_ops,
 	},
 	.probe		= ads1015_probe,
-	.remove		= ads1015_remove,
 	.id_table	= ads1015_id,
 };
 
-- 
2.43.0

Re: [PATCH v2] iio: adc: ti-ads1015: use devm helpers to fix probe error paths

[Jonathan Cameron]

On Fri, 29 May 2026 23:37:08 +0500
Stepan Ionichev <sozdayvek@gmail.com> wrote:

> Once ads1015_probe() gets past ads1015_set_conv_mode(CONTINUOUS), two
> resources leak on its error paths and on driver unbind:
> 
>   - pm_runtime_enable() is left enabled if iio_device_register() fails;
>     on subsequent probe/rebind the runtime PM tracking complains about
>     an unbalanced enable.
>   - The CONTINUOUS conversion mode written to the chip is never restored
>     on any error path after that point, so on probe failure the chip is
>     left running.
> 
> Convert all three teardown operations of ads1015_remove() to devm so
> that the error paths and the unbind path share the same unwind in
> reverse-of-setup order:
> 
>   - ads1015_set_conv_mode(SINGLESHOT) becomes a devm action registered
>     immediately after CONTINUOUS mode is enabled.
>   - pm_runtime_enable() becomes devm_pm_runtime_enable().
>   - iio_device_register() becomes devm_iio_device_register(), so the
>     iio device is unregistered first on unbind.
> 
> ads1015_remove() is no longer needed and is dropped.
> 
> Signed-off-by: Stepan Ionichev <sozdayvek@gmail.com>
These are always a bit fiddly so I was hoping to get some more eyes on this
one.  Ah well, long enough and I think it's fine and sashiko is happy so applied.

Thanks,

Jonathan

> ---
> v2:
> - Full devm conversion: register a devm action for set_conv_mode(SINGLESHOT),
>   switch to devm_iio_device_register, drop ads1015_remove() entirely.  This
>   avoids mixing devm with manual unwind (Jonathan) and also fixes the
>   pre-existing continuous-mode leak on probe failure.
> - Drop the unrelated dev_err() -> dev_err_probe() conversion from v1 (Joshua).
> 
> v1: https://lore.kernel.org/all/20260529101011.3030-1-sozdayvek@gmail.com/
> 
>  drivers/iio/adc/ti-ads1015.c | 44 +++++++++++++++---------------------
>  1 file changed, 18 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/iio/adc/ti-ads1015.c b/drivers/iio/adc/ti-ads1015.c
> index c7ffe47449e2..d7f4d0d5bcf9 100644
> --- a/drivers/iio/adc/ti-ads1015.c
> +++ b/drivers/iio/adc/ti-ads1015.c
> @@ -930,6 +930,17 @@ static int ads1015_set_conv_mode(struct ads1015_data *data, int mode)
>  				  mode << ADS1015_CFG_MOD_SHIFT);
>  }
>  
> +static void ads1015_power_down(void *p)
> +{
> +	struct ads1015_data *data = p;
> +	int ret;
> +
> +	ret = ads1015_set_conv_mode(data, ADS1015_SINGLESHOT);
> +	if (ret)
> +		dev_warn(regmap_get_device(data->regmap),
> +			 "Failed to power down (%pe)\n", ERR_PTR(ret));
> +}
> +
>  static int ads1015_probe(struct i2c_client *client)
>  {
>  	const struct ads1015_chip_data *chip;
> @@ -1030,6 +1041,10 @@ static int ads1015_probe(struct i2c_client *client)
>  	if (ret)
>  		return ret;
>  
> +	ret = devm_add_action_or_reset(&client->dev, ads1015_power_down, data);
> +	if (ret)
> +		return ret;
> +
>  	data->conv_invalid = true;
>  
>  	ret = pm_runtime_set_active(&client->dev);
> @@ -1037,33 +1052,11 @@ static int ads1015_probe(struct i2c_client *client)
>  		return ret;
>  	pm_runtime_set_autosuspend_delay(&client->dev, ADS1015_SLEEP_DELAY_MS);
>  	pm_runtime_use_autosuspend(&client->dev);
> -	pm_runtime_enable(&client->dev);
> -
> -	ret = iio_device_register(indio_dev);
> -	if (ret < 0) {
> -		dev_err(&client->dev, "Failed to register IIO device\n");
> +	ret = devm_pm_runtime_enable(&client->dev);
> +	if (ret)
>  		return ret;
> -	}
> -
> -	return 0;
> -}
>  
> -static void ads1015_remove(struct i2c_client *client)
> -{
> -	struct iio_dev *indio_dev = i2c_get_clientdata(client);
> -	struct ads1015_data *data = iio_priv(indio_dev);
> -	int ret;
> -
> -	iio_device_unregister(indio_dev);
> -
> -	pm_runtime_disable(&client->dev);
> -	pm_runtime_set_suspended(&client->dev);
> -
> -	/* power down single shot mode */
> -	ret = ads1015_set_conv_mode(data, ADS1015_SINGLESHOT);
> -	if (ret)
> -		dev_warn(&client->dev, "Failed to power down (%pe)\n",
> -			 ERR_PTR(ret));
> +	return devm_iio_device_register(&client->dev, indio_dev);
>  }
>  
>  #ifdef CONFIG_PM
> @@ -1150,7 +1143,6 @@ static struct i2c_driver ads1015_driver = {
>  		.pm = &ads1015_pm_ops,
>  	},
>  	.probe		= ads1015_probe,
> -	.remove		= ads1015_remove,
>  	.id_table	= ads1015_id,
>  };
>