[PATCH v4 07/11] iio: amplifiers: ad8366: refactor device resource management

Rodrigo Alencar via B4 Relay posted 11 patches 1 month, 2 weeks ago
There is a newer version of this series
[PATCH v4 07/11] iio: amplifiers: ad8366: refactor device resource management
Posted by Rodrigo Alencar via B4 Relay 1 month, 2 weeks ago
From: Rodrigo Alencar <rodrigo.alencar@analog.com>

Adhere modern device resource management with the following:
- Voltage regulator managed and enabled internally;
- IIO device registration handled with devm_iio_device_register();
- removal of goto's from the probe function;
- ad8366_remove() removed as it is not needed anymore;

With the drop of goto's dev_err_probe() is used to report probe errors.

Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
---
 drivers/iio/amplifiers/ad8366.c | 50 +++++++++--------------------------------
 1 file changed, 10 insertions(+), 40 deletions(-)

diff --git a/drivers/iio/amplifiers/ad8366.c b/drivers/iio/amplifiers/ad8366.c
index 67817dedd75d..343f370eae65 100644
--- a/drivers/iio/amplifiers/ad8366.c
+++ b/drivers/iio/amplifiers/ad8366.c
@@ -37,7 +37,6 @@ struct ad8366_info {
 
 struct ad8366_state {
 	struct spi_device	*spi;
-	struct regulator	*reg;
 	struct mutex            lock; /* protect sensor state */
 	unsigned char		ch[2];
 	enum ad8366_type	type;
@@ -257,14 +256,10 @@ static int ad8366_probe(struct spi_device *spi)
 	if (ret)
 		return ret;
 
-	st->reg = devm_regulator_get(&spi->dev, "vcc");
-	if (!IS_ERR(st->reg)) {
-		ret = regulator_enable(st->reg);
-		if (ret)
-			return ret;
-	}
+	ret = devm_regulator_get_enable(dev, "vcc");
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to get regulator\n");
 
-	spi_set_drvdata(spi, indio_dev);
 	st->spi = spi;
 	st->type = spi_get_device_id(spi)->driver_data;
 
@@ -278,17 +273,15 @@ static int ad8366_probe(struct spi_device *spi)
 	case ID_HMC792:
 	case ID_HMC1119:
 		rstc = devm_reset_control_get_optional_exclusive_deasserted(dev, NULL);
-		if (IS_ERR(rstc)) {
-			ret = PTR_ERR(rstc);
-			goto error_disable_reg;
-		}
+		if (IS_ERR(rstc))
+			return dev_err_probe(dev, PTR_ERR(rstc),
+					     "Failed to get reset controller\n");
+
 		indio_dev->channels = ada4961_channels;
 		indio_dev->num_channels = ARRAY_SIZE(ada4961_channels);
 		break;
 	default:
-		dev_err(&spi->dev, "Invalid device ID\n");
-		ret = -EINVAL;
-		goto error_disable_reg;
+		return dev_err_probe(dev, -EINVAL, "Invalid device ID\n");
 	}
 
 	st->info = &ad8366_infos[st->type];
@@ -298,31 +291,9 @@ static int ad8366_probe(struct spi_device *spi)
 
 	ret = ad8366_write(indio_dev, 0, 0);
 	if (ret < 0)
-		goto error_disable_reg;
+		return dev_err_probe(dev, ret, "failed to write initial gain\n");
 
-	ret = iio_device_register(indio_dev);
-	if (ret)
-		goto error_disable_reg;
-
-	return 0;
-
-error_disable_reg:
-	if (!IS_ERR(st->reg))
-		regulator_disable(st->reg);
-
-	return ret;
-}
-
-static void ad8366_remove(struct spi_device *spi)
-{
-	struct iio_dev *indio_dev = spi_get_drvdata(spi);
-	struct ad8366_state *st = iio_priv(indio_dev);
-	struct regulator *reg = st->reg;
-
-	iio_device_unregister(indio_dev);
-
-	if (!IS_ERR(reg))
-		regulator_disable(reg);
+	return devm_iio_device_register(dev, indio_dev);
 }
 
 static const struct spi_device_id ad8366_id[] = {
@@ -340,7 +311,6 @@ static struct spi_driver ad8366_driver = {
 		.name	= KBUILD_MODNAME,
 	},
 	.probe		= ad8366_probe,
-	.remove		= ad8366_remove,
 	.id_table	= ad8366_id,
 };
 

-- 
2.43.0
Re: [PATCH v4 07/11] iio: amplifiers: ad8366: refactor device resource management
Posted by Andy Shevchenko 1 month, 2 weeks ago
On Tue, Feb 10, 2026 at 07:42:07PM +0000, Rodrigo Alencar via B4 Relay wrote:

> Adhere modern device resource management with the following:
> - Voltage regulator managed and enabled internally;
> - IIO device registration handled with devm_iio_device_register();
> - removal of goto's from the probe function;
> - ad8366_remove() removed as it is not needed anymore;
> 
> With the drop of goto's dev_err_probe() is used to report probe errors.

I think the regulator change should be split and go before the previous patch,
because that one affects the ordering in the error path and remove stage.

Otherwise LGTM.

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH v4 07/11] iio: amplifiers: ad8366: refactor device resource management
Posted by Rodrigo Alencar 1 month, 2 weeks ago
On 26/02/10 10:05PM, Andy Shevchenko wrote:
> On Tue, Feb 10, 2026 at 07:42:07PM +0000, Rodrigo Alencar via B4 Relay wrote:
> 
> > Adhere modern device resource management with the following:
> > - Voltage regulator managed and enabled internally;
> > - IIO device registration handled with devm_iio_device_register();
> > - removal of goto's from the probe function;
> > - ad8366_remove() removed as it is not needed anymore;
> > 
> > With the drop of goto's dev_err_probe() is used to report probe errors.
> 
> I think the regulator change should be split and go before the previous patch,
> because that one affects the ordering in the error path and remove stage.

OK, that can be done, but the same way, the error check would change as
well as the dev_err_probe() args, not seeing much benefit there as the same
lines will have to be touched anyways.

-- 
Kind regards,

Rodrigo Alencar
Re: [PATCH v4 07/11] iio: amplifiers: ad8366: refactor device resource management
Posted by Andy Shevchenko 1 month, 2 weeks ago
On Wed, Feb 11, 2026 at 12:10:11PM +0000, Rodrigo Alencar wrote:
> On 26/02/10 10:05PM, Andy Shevchenko wrote:
> > On Tue, Feb 10, 2026 at 07:42:07PM +0000, Rodrigo Alencar via B4 Relay wrote:
> > 
> > > Adhere modern device resource management with the following:
> > > - Voltage regulator managed and enabled internally;
> > > - IIO device registration handled with devm_iio_device_register();
> > > - removal of goto's from the probe function;
> > > - ad8366_remove() removed as it is not needed anymore;
> > > 
> > > With the drop of goto's dev_err_probe() is used to report probe errors.
> > 
> > I think the regulator change should be split and go before the previous patch,
> > because that one affects the ordering in the error path and remove stage.
> 
> OK, that can be done, but the same way,
> the error check would change as well as the dev_err_probe() args, not seeing
> much benefit there as the same lines will have to be touched anyways.

The point is to range the problems and target the more serious one first.
I consider the wrong ordering, (mis)use of devm_*() are in a priority to
just a simple cleanup.

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH v4 07/11] iio: amplifiers: ad8366: refactor device resource management
Posted by Jonathan Cameron 1 month, 2 weeks ago
On Wed, 11 Feb 2026 15:30:44 +0200
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:

> On Wed, Feb 11, 2026 at 12:10:11PM +0000, Rodrigo Alencar wrote:
> > On 26/02/10 10:05PM, Andy Shevchenko wrote:  
> > > On Tue, Feb 10, 2026 at 07:42:07PM +0000, Rodrigo Alencar via B4 Relay wrote:
> > >   
> > > > Adhere modern device resource management with the following:
> > > > - Voltage regulator managed and enabled internally;
> > > > - IIO device registration handled with devm_iio_device_register();
> > > > - removal of goto's from the probe function;
> > > > - ad8366_remove() removed as it is not needed anymore;
> > > > 
> > > > With the drop of goto's dev_err_probe() is used to report probe errors.  
> > > 
> > > I think the regulator change should be split and go before the previous patch,
> > > because that one affects the ordering in the error path and remove stage.  
> > 
> > OK, that can be done, but the same way,
> > the error check would change as well as the dev_err_probe() args, not seeing
> > much benefit there as the same lines will have to be touched anyways.  
> 
> The point is to range the problems and target the more serious one first.
> I consider the wrong ordering, (mis)use of devm_*() are in a priority to
> just a simple cleanup.
> 
Took me a minute to see what was meant here, but having realized original
code unwound as power off then reset when it had setup as power on then reset,
I agree entirely with Andy that should be fixed in the earlier patch.

Thanks,

Jonathan