Re: [PATCH 3/3] iio: adc: ti-ads1015: convert to fully managed resources

[Jonathan Cameron]

On Sat, 18 Jul 2026 13:20:16 +0530
Archit Anant <architanant5@xxxxxxxxx> wrote:

> Refactor the driver to use devm_ allocations and power
> management, allowing for the complete removal of the
> ads1015_remove()
> 
> Key changes:
> - Use devm_add_action_or_reset() to ensure the ADC is safely powered
> down upon driver removal.
> - Move to devm_pm_runtime_set_active_enabled() to manage the runtime
> PM lifecycle.
> - Update the PM operations to use DEFINE_RUNTIME_DEV_PM_OPS and
> pm_ptr() for modern compiler saftey.
> - Convert iio_device_register() and mutex_init() to their devm_
> variants.
> - Remove the thus obsolete ads1015_remove() function.

See below. I'm fairly sure this is also fixing a bug.  Ideally you'd
fix that first at the start of the series then refactor it later.

Jonathan

> 
> Signed-off-by: Archit Anant <architanant5@xxxxxxxxx>
> ---
>  drivers/iio/adc/ti-ads1015.c | 58 ++++++++++++++++--------------------
>  1 file changed, 26 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/iio/adc/ti-ads1015.c b/drivers/iio/adc/ti-ads1015.c
> index 0232a4ca2d77..57e5fa593b31 100644
> --- a/drivers/iio/adc/ti-ads1015.c
> +++ b/drivers/iio/adc/ti-ads1015.c
> @@ -930,6 +930,14 @@ static int ads1015_set_conv_mode(struct ads1015_data *data, int mode)
>  				  mode << ADS1015_CFG_MOD_SHIFT);
>  }
>  
> +static void ads1015_power_off(void *data)
> +{
> +	struct ads1015_data *st = data;
> +
> +	/* power down single shot mode */
> +	ads1015_set_conv_mode(st, ADS1015_SINGLESHOT);
> +}
> +
>  static int ads1015_probe(struct i2c_client *client)
>  {
>  	const struct ads1015_chip_data *chip;
> @@ -950,7 +958,9 @@ static int ads1015_probe(struct i2c_client *client)
>  	data = iio_priv(indio_dev);
>  	i2c_set_clientdata(client, indio_dev);
>  
> -	mutex_init(&data->lock);
> +	ret = devm_mutex_init(dev, &data->lock);
> +	if (ret)
> +		return ret;
>  
>  	indio_dev->name = ADS1015_DRV_NAME;
>  	indio_dev->modes = INDIO_DIRECT_MODE;
> @@ -1033,38 +1043,23 @@ static int ads1015_probe(struct i2c_client *client)
>  
>  	data->conv_invalid = true;
>  
> -	ret = pm_runtime_set_active(dev);
> +	ret = devm_add_action_or_reset(dev, ads1015_power_off, data);
>  	if (ret)
>  		return ret;
> -	pm_runtime_set_autosuspend_delay(dev, ADS1015_SLEEP_DELAY_MS);
> -	pm_runtime_use_autosuspend(dev);
> -	pm_runtime_enable(dev);
>  
> -	ret = iio_device_register(indio_dev);
> -	if (ret < 0) {
> -		dev_err(dev, "Failed to register IIO device\n");
> +	ret = devm_pm_runtime_set_active_enabled(dev);
> +	if (ret)
>  		return ret;

Worth noting this was a bug as noting turned off the runtime
pm if we got an error in iio_device_register().  That is fixed
by this patch I think.

> -	}
>  
> -	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);
> +	pm_runtime_set_autosuspend_delay(dev, ADS1015_SLEEP_DELAY_MS);
> +	pm_runtime_use_autosuspend(dev);
>  
> -	/* power down single shot mode */
> -	ret = ads1015_set_conv_mode(data, ADS1015_SINGLESHOT);
> +	ret = devm_iio_device_register(dev, indio_dev);
>  	if (ret)
> -		dev_warn(&client->dev, "Failed to power down (%pe)\n",
> -			 ERR_PTR(ret));
> +		return dev_err_probe(dev, ret,
> +				     "Failed to register IIO device\n");
> +
> +	return 0;
>  }
>  
>  #ifdef CONFIG_PM
> @@ -1090,10 +1085,10 @@ static int ads1015_runtime_resume(struct device *dev)
>  }
>  #endif
>  
> -static const struct dev_pm_ops ads1015_pm_ops = {
> -	SET_RUNTIME_PM_OPS(ads1015_runtime_suspend,
> -			   ads1015_runtime_resume, NULL)
> -};
> +static DEFINE_RUNTIME_DEV_PM_OPS(ads1015_pm_ops,

Sashiko points this out.  Switching to this and leaving the CONFIG_PM
ifdef above is a bug.
https://sashiko.dev/#/patchset/20260718075016.27750-1-architanant5%40gmail.com

I didn't chase through whether you can also safely remove the other
one further up in the code.

> +				  ads1015_runtime_suspend,
> +				  ads1015_runtime_resume,
> +				  NULL);
>  
>  static const struct ads1015_chip_data ads1015_data = {
>  	.channels	= ads1015_channels,
> @@ -1148,10 +1143,9 @@ static struct i2c_driver ads1015_driver = {
>  	.driver = {
>  		.name = ADS1015_DRV_NAME,
>  		.of_match_table = ads1015_of_match,
> -		.pm = &ads1015_pm_ops,
> +		.pm = pm_ptr(&ads1015_pm_ops),
>  	},
>  	.probe		= ads1015_probe,
> -	.remove		= ads1015_remove,
>  	.id_table	= ads1015_id,
>  };
>