[PATCH v4 3/4] iio: adc: ad799x: use devm_add_action_or_reset for regulators

Archit Anant posted 4 patches 1 month ago
There is a newer version of this series
[PATCH v4 3/4] iio: adc: ad799x: use devm_add_action_or_reset for regulators
Posted by Archit Anant 1 month ago
Convert the manual regulator_disable() handling to use
devm_add_action_or_reset(). This guarantees that the regulators are
disabled in the correct reverse order of allocation during device
teardown or probe failure.

As part of this conversion, introduce a local device pointer to
simplify function calls and remove redundant goto labels for regulator
cleanup, as the devm framework now handles these automatically.

Suggested-by: David Lechner <dlechner@baylibre.com>
Suggested-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Signed-off-by: Archit Anant <architanant5@gmail.com>
---
 drivers/iio/adc/ad799x.c | 46 ++++++++++++++++++++++------------------
 1 file changed, 25 insertions(+), 21 deletions(-)

diff --git a/drivers/iio/adc/ad799x.c b/drivers/iio/adc/ad799x.c
index 28f1901698a6..f5ef6ff97c89 100644
--- a/drivers/iio/adc/ad799x.c
+++ b/drivers/iio/adc/ad799x.c
@@ -773,8 +773,14 @@ static const struct ad799x_chip_info ad799x_chip_info_tbl[] = {
 	},
 };
 
+static void ad799x_reg_disable(void *reg)
+{
+	regulator_disable(reg);
+}
+
 static int ad799x_probe(struct i2c_client *client)
 {
+	struct device *dev = &client->dev;
 	const struct i2c_device_id *id = i2c_client_get_device_id(client);
 	int ret;
 	int extra_config = 0;
@@ -783,7 +789,7 @@ static int ad799x_probe(struct i2c_client *client)
 	const struct ad799x_chip_info *chip_info =
 		&ad799x_chip_info_tbl[id->driver_data];
 
-	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*st));
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
 	if (indio_dev == NULL)
 		return -ENOMEM;
 
@@ -799,7 +805,7 @@ static int ad799x_probe(struct i2c_client *client)
 
 	/* TODO: Add pdata options for filtering and bit delay */
 
-	st->reg = devm_regulator_get(&client->dev, "vcc");
+	st->reg = devm_regulator_get(dev, "vcc");
 	if (IS_ERR(st->reg))
 		return PTR_ERR(st->reg);
 	ret = regulator_enable(st->reg);
@@ -807,31 +813,38 @@ static int ad799x_probe(struct i2c_client *client)
 		return ret;
 	ret = regulator_get_voltage(st->reg);
 	if (ret < 0)
-		goto error_disable_reg;
+		return ret;
 	st->vref_uV = ret;
 
+	ret = devm_add_action_or_reset(dev, ad799x_reg_disable, st->reg);
+	if (ret)
+		return ret;
 	/* check if an external reference is supplied */
 	if (chip_info->has_vref) {
-		st->vref = devm_regulator_get_optional(&client->dev, "vref");
+		st->vref = devm_regulator_get_optional(dev, "vref");
 		ret = PTR_ERR_OR_ZERO(st->vref);
 		if (ret) {
 			if (ret != -ENODEV)
-				goto error_disable_reg;
+				return ret;
 			st->vref = NULL;
-			dev_info(&client->dev, "Using VCC reference voltage\n");
+			dev_info(dev, "Using VCC reference voltage\n");
 		}
 
 		if (st->vref) {
-			dev_info(&client->dev, "Using external reference voltage\n");
+			dev_info(dev, "Using external reference voltage\n");
 			extra_config |= AD7991_REF_SEL;
 			ret = regulator_enable(st->vref);
 			if (ret)
-				goto error_disable_reg;
+				return ret;
 
 			ret = regulator_get_voltage(st->vref);
 			if (ret < 0)
-				goto error_disable_vref;
+				return ret;
 			st->vref_uV = ret;
+
+			ret = devm_add_action_or_reset(dev, ad799x_reg_disable, st->vref);
+			if (ret)
+				return ret;
 		}
 	}
 
@@ -846,15 +859,15 @@ static int ad799x_probe(struct i2c_client *client)
 
 	ret = ad799x_update_config(st, st->chip_config->default_config | extra_config);
 	if (ret)
-		goto error_disable_vref;
+		return ret;
 
 	ret = iio_triggered_buffer_setup(indio_dev, NULL,
 		&ad799x_trigger_handler, NULL);
 	if (ret)
-		goto error_disable_vref;
+		return ret;
 
 	if (client->irq > 0) {
-		ret = devm_request_threaded_irq(&client->dev,
+		ret = devm_request_threaded_irq(dev,
 						client->irq,
 						NULL,
 						ad799x_event_handler,
@@ -876,11 +889,6 @@ static int ad799x_probe(struct i2c_client *client)
 
 error_cleanup_ring:
 	iio_triggered_buffer_cleanup(indio_dev);
-error_disable_vref:
-	if (st->vref)
-		regulator_disable(st->vref);
-error_disable_reg:
-	regulator_disable(st->reg);
 
 	return ret;
 }
@@ -888,14 +896,10 @@ static int ad799x_probe(struct i2c_client *client)
 static void ad799x_remove(struct i2c_client *client)
 {
 	struct iio_dev *indio_dev = i2c_get_clientdata(client);
-	struct ad799x_state *st = iio_priv(indio_dev);
 
 	iio_device_unregister(indio_dev);
 
 	iio_triggered_buffer_cleanup(indio_dev);
-	if (st->vref)
-		regulator_disable(st->vref);
-	regulator_disable(st->reg);
 }
 
 static int ad799x_suspend(struct device *dev)
-- 
2.39.5
Re: [PATCH v4 3/4] iio: adc: ad799x: use devm_add_action_or_reset for regulators
Posted by Andy Shevchenko 1 month ago
On Sun, Mar 08, 2026 at 11:58:10PM +0530, Archit Anant wrote:
> Convert the manual regulator_disable() handling to use
> devm_add_action_or_reset(). This guarantees that the regulators are
> disabled in the correct reverse order of allocation during device
> teardown or probe failure.
> 
> As part of this conversion, introduce a local device pointer to
> simplify function calls and remove redundant goto labels for regulator
> cleanup, as the devm framework now handles these automatically.

Needs to be split to two:
- moving to managed resources (and here you introduce temporary variable to reduce the churn)
- simplifying the old code with that variable

-- 
With Best Regards,
Andy Shevchenko