From: Rodrigo Alencar <rodrigo.alencar@analog.com>
Adhere modern device resource management with the following:
- Voltage regulator managed and enabled internally;
- Proper mutex lifecycle with devm_mutex_init(), replacing mutex_init();
- 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;
Also, dev_err_probe() is used to report probe errors with created local
device pointer.
Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
---
drivers/iio/amplifiers/ad8366.c | 60 ++++++++++++-----------------------------
1 file changed, 17 insertions(+), 43 deletions(-)
diff --git a/drivers/iio/amplifiers/ad8366.c b/drivers/iio/amplifiers/ad8366.c
index d06ac786501c..0f3fc79ebbf3 100644
--- a/drivers/iio/amplifiers/ad8366.c
+++ b/drivers/iio/amplifiers/ad8366.c
@@ -40,7 +40,6 @@ struct ad8366_info {
struct ad8366_state {
struct spi_device *spi;
- struct regulator *reg;
struct mutex lock; /* protect sensor state */
struct gpio_desc *reset_gpio;
unsigned char ch[2];
@@ -245,25 +244,25 @@ static const struct iio_chan_spec ada4961_channels[] = {
static int ad8366_probe(struct spi_device *spi)
{
+ struct device *dev = &spi->dev;
struct iio_dev *indio_dev;
struct ad8366_state *st;
int ret;
- indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
+ indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
if (indio_dev == NULL)
return -ENOMEM;
st = iio_priv(indio_dev);
- 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");
+
+ ret = devm_mutex_init(dev, &st->lock);
+ if (ret)
+ return ret;
- spi_set_drvdata(spi, indio_dev);
- mutex_init(&st->lock);
st->spi = spi;
st->type = spi_get_device_id(spi)->driver_data;
@@ -276,18 +275,16 @@ static int ad8366_probe(struct spi_device *spi)
case ID_ADL5240:
case ID_HMC792:
case ID_HMC1119:
- st->reset_gpio = devm_gpiod_get_optional(&spi->dev, "reset", GPIOD_OUT_HIGH);
- if (IS_ERR(st->reset_gpio)) {
- ret = PTR_ERR(st->reset_gpio);
- goto error_disable_reg;
- }
+ st->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
+ if (IS_ERR(st->reset_gpio))
+ return dev_err_probe(dev, PTR_ERR(st->reset_gpio),
+ "Failed to get reset GPIO\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];
@@ -297,31 +294,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[] = {
@@ -339,7 +314,6 @@ static struct spi_driver ad8366_driver = {
.name = KBUILD_MODNAME,
},
.probe = ad8366_probe,
- .remove = ad8366_remove,
.id_table = ad8366_id,
};
--
2.43.0
On Mon, Jan 26, 2026 at 01:51:04PM +0000, Rodrigo Alencar via B4 Relay wrote: > Adhere modern device resource management with the following: > - Voltage regulator managed and enabled internally; > - Proper mutex lifecycle with devm_mutex_init(), replacing mutex_init(); > - 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; > > Also, dev_err_probe() is used to report probe errors with created local > device pointer. And also it uses the temporary dev variable in the cases that are not covered by the above. So, three changes in one patch... Dunno if Jonathan is okay with this. I would rather split it. Code wise LGTM. -- With Best Regards, Andy Shevchenko
On 26/01/27 11:12PM, Andy Shevchenko wrote: > On Mon, Jan 26, 2026 at 01:51:04PM +0000, Rodrigo Alencar via B4 Relay wrote: > > > Adhere modern device resource management with the following: > > - Voltage regulator managed and enabled internally; > > - Proper mutex lifecycle with devm_mutex_init(), replacing mutex_init(); > > - 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; > > > > Also, dev_err_probe() is used to report probe errors with created local > > device pointer. > > And also it uses the temporary dev variable in the cases that are not covered > by the above. > > So, three changes in one patch... I can split. Using proper devm functions allowed me to drop goto's and ad8366_remove(). Dropping the goto's allowed for the usage of dev_err_probe(). I could place the temporary dev variable in a separate patch. > > Dunno if Jonathan is okay with this. I would rather split it. > Code wise LGTM. Yeah I can now see it coming. Thanks for the review. -- Kind regards, Rodrigo Alencar
On Wed, 28 Jan 2026 09:31:10 +0000 Rodrigo Alencar <455.rodrigo.alencar@gmail.com> wrote: > On 26/01/27 11:12PM, Andy Shevchenko wrote: > > On Mon, Jan 26, 2026 at 01:51:04PM +0000, Rodrigo Alencar via B4 Relay wrote: > > > > > Adhere modern device resource management with the following: > > > - Voltage regulator managed and enabled internally; > > > - Proper mutex lifecycle with devm_mutex_init(), replacing mutex_init(); > > > - 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; > > > > > > Also, dev_err_probe() is used to report probe errors with created local > > > device pointer. > > > > And also it uses the temporary dev variable in the cases that are not covered > > by the above. > > > > So, three changes in one patch... > > I can split. Using proper devm functions allowed me to drop goto's and > ad8366_remove(). Dropping the goto's allowed for the usage of > dev_err_probe(). As general rule I prefer one patch one change, though just occasionally I'll take the expedient view if a series is otherwise ready to merge and one change is really really small. J > > I could place the temporary dev variable in a separate patch. > > > > > Dunno if Jonathan is okay with this. I would rather split it. > > Code wise LGTM. > > Yeah I can now see it coming. Thanks for the review. >
© 2016 - 2026 Red Hat, Inc.