[PATCH 0/3] iio: adc: ad7923: convert driver resources routines to device-managed counterparts

[PATCH 0/3] iio: adc: ad7923: convert driver resources routines to device-managed counterparts

[Lucas Stankus]

Following the initiative proposed by Alexandru, this patch series aims
to convert the ad7923 to use only device-managed routines.

Part of the driver was already using devm_ functions, so it was possible
to convert the remainder of it without much hassle.

With that, the deregistration function was no longer necessary and could
be entirely removed from the driver.

Lucas Stankus (3):
  iio: adc: ad7923: use devm_add_action_or_reset for regulator disable
  iio: adc: ad7923: use device-managed function for triggered buffer
  iio: adc: ad7923: register device with devm_iio_device_register

 drivers/iio/adc/ad7923.c | 39 +++++++++++++--------------------------
 1 file changed, 13 insertions(+), 26 deletions(-)

-- 
2.31.0

[PATCH 1/3] iio: adc: ad7923: use devm_add_action_or_reset for regulator disable

[Lucas Stankus]

Adds a device-managed action to handle disabling the driver's regulator on
device detach.
This slightly simplifies deinitialization and enables further conversion of
the driver to device-managed routines without breaking the init order.

Signed-off-by: Lucas Stankus <lucas.p.stankus@gmail.com>
---
 drivers/iio/adc/ad7923.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/iio/adc/ad7923.c b/drivers/iio/adc/ad7923.c
index a2cc96658054..3418ef6f0857 100644
--- a/drivers/iio/adc/ad7923.c
+++ b/drivers/iio/adc/ad7923.c
@@ -293,6 +293,13 @@ static const struct iio_info ad7923_info = {
 	.update_scan_mode = ad7923_update_scan_mode,
 };
 
+static void ad7923_regulator_disable(void *data)
+{
+	struct ad7923_state *st = data;
+
+	regulator_disable(st->reg);
+}
+
 static int ad7923_probe(struct spi_device *spi)
 {
 	struct ad7923_state *st;
@@ -340,10 +347,14 @@ static int ad7923_probe(struct spi_device *spi)
 	if (ret)
 		return ret;
 
+	ret = devm_add_action_or_reset(&spi->dev, ad7923_regulator_disable, st);
+	if (ret)
+		return ret;
+
 	ret = iio_triggered_buffer_setup(indio_dev, NULL,
 					 &ad7923_trigger_handler, NULL);
 	if (ret)
-		goto error_disable_reg;
+		return ret;
 
 	ret = iio_device_register(indio_dev);
 	if (ret)
@@ -353,20 +364,15 @@ static int ad7923_probe(struct spi_device *spi)
 
 error_cleanup_ring:
 	iio_triggered_buffer_cleanup(indio_dev);
-error_disable_reg:
-	regulator_disable(st->reg);
-
 	return ret;
 }
 
 static int ad7923_remove(struct spi_device *spi)
 {
 	struct iio_dev *indio_dev = spi_get_drvdata(spi);
-	struct ad7923_state *st = iio_priv(indio_dev);
 
 	iio_device_unregister(indio_dev);
 	iio_triggered_buffer_cleanup(indio_dev);
-	regulator_disable(st->reg);
 
 	return 0;
 }
-- 
2.31.0

[PATCH 2/3] iio: adc: ad7923: use device-managed function for triggered buffer

[Lucas Stankus]

Converts the iio_triggered_buffer_setup() call to its device-managed
counterpart.
With this, the error handling routine in the ad7923_probe() function
becomes unnecessary as well as the iio_triggered_buffer_cleanup() call.

Signed-off-by: Lucas Stankus <lucas.p.stankus@gmail.com>
---
 drivers/iio/adc/ad7923.c | 15 +++------------
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/drivers/iio/adc/ad7923.c b/drivers/iio/adc/ad7923.c
index 3418ef6f0857..d07eaf3111ed 100644
--- a/drivers/iio/adc/ad7923.c
+++ b/drivers/iio/adc/ad7923.c
@@ -351,20 +351,12 @@ static int ad7923_probe(struct spi_device *spi)
 	if (ret)
 		return ret;
 
-	ret = iio_triggered_buffer_setup(indio_dev, NULL,
-					 &ad7923_trigger_handler, NULL);
+	ret = devm_iio_triggered_buffer_setup(&spi->dev, indio_dev, NULL,
+					      &ad7923_trigger_handler, NULL);
 	if (ret)
 		return ret;
 
-	ret = iio_device_register(indio_dev);
-	if (ret)
-		goto error_cleanup_ring;
-
-	return 0;
-
-error_cleanup_ring:
-	iio_triggered_buffer_cleanup(indio_dev);
-	return ret;
+	return iio_device_register(indio_dev);
 }
 
 static int ad7923_remove(struct spi_device *spi)
@@ -372,7 +364,6 @@ static int ad7923_remove(struct spi_device *spi)
 	struct iio_dev *indio_dev = spi_get_drvdata(spi);
 
 	iio_device_unregister(indio_dev);
-	iio_triggered_buffer_cleanup(indio_dev);
 
 	return 0;
 }
-- 
2.31.0

[PATCH 3/3] iio: adc: ad7923: register device with devm_iio_device_register

[Lucas Stankus]

Registers the device using the devm variant.
This is the final step of converting the ad7923 to only use devm routines,
meaning that the ad7923_remove() function is no longer needed to release
resources on device detach.

Signed-off-by: Lucas Stankus <lucas.p.stankus@gmail.com>
---
 drivers/iio/adc/ad7923.c | 12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/drivers/iio/adc/ad7923.c b/drivers/iio/adc/ad7923.c
index d07eaf3111ed..f7af2f194789 100644
--- a/drivers/iio/adc/ad7923.c
+++ b/drivers/iio/adc/ad7923.c
@@ -356,16 +356,7 @@ static int ad7923_probe(struct spi_device *spi)
 	if (ret)
 		return ret;
 
-	return iio_device_register(indio_dev);
-}
-
-static int ad7923_remove(struct spi_device *spi)
-{
-	struct iio_dev *indio_dev = spi_get_drvdata(spi);
-
-	iio_device_unregister(indio_dev);
-
-	return 0;
+	return devm_iio_device_register(&spi->dev, indio_dev);
 }
 
 static const struct spi_device_id ad7923_id[] = {
@@ -398,7 +389,6 @@ static struct spi_driver ad7923_driver = {
 		.of_match_table = ad7923_of_match,
 	},
 	.probe		= ad7923_probe,
-	.remove		= ad7923_remove,
 	.id_table	= ad7923_id,
 };
 module_spi_driver(ad7923_driver);
-- 
2.31.0

Re: [PATCH 3/3] iio: adc: ad7923: register device with devm_iio_device_register

[Lars-Peter Clausen]

On 3/28/21 11:46 PM, Lucas Stankus wrote:
> Registers the device using the devm variant.
> This is the final step of converting the ad7923 to only use devm routines,
> meaning that the ad7923_remove() function is no longer needed to release
> resources on device detach.
> 
> Signed-off-by: Lucas Stankus <lucas.p.stankus@gmail.com>

Hi,

Thanks for the patches.T his looks good, just one small comment.

> ---
>   drivers/iio/adc/ad7923.c | 12 +-----------
>   1 file changed, 1 insertion(+), 11 deletions(-)
> 
> diff --git a/drivers/iio/adc/ad7923.c b/drivers/iio/adc/ad7923.c
> index d07eaf3111ed..f7af2f194789 100644
> --- a/drivers/iio/adc/ad7923.c
> +++ b/drivers/iio/adc/ad7923.c
> @@ -356,16 +356,7 @@ static int ad7923_probe(struct spi_device *spi)
>   	if (ret)
>   		return ret;
>   
> -	return iio_device_register(indio_dev);
> -}
> -
> -static int ad7923_remove(struct spi_device *spi)
> -{
> -	struct iio_dev *indio_dev = spi_get_drvdata(spi);

This removes the last user of get_drvdata() on the SPI device. This means you 
can also remove the spi_set_drvdata() in the probe function.

> -
> -	iio_device_unregister(indio_dev);
> -
> -	return 0;
> +	return devm_iio_device_register(&spi->dev, indio_dev);
>   }

Re: [PATCH 0/3] iio: adc: ad7923: convert driver resources routines to device-managed counterparts

[Alexandru Ardelean]

On Mon, Mar 29, 2021 at 12:48 AM Lucas Stankus
<lucas.p.stankus@gmail.com> wrote:
>
> Following the initiative proposed by Alexandru, this patch series aims
> to convert the ad7923 to use only device-managed routines.
>

This idea is becoming popular it seems :)

Thanks to Lars for pointing out that spi_set_drvdata() omission.
With that fixed:

Reviewed-by: Alexandru Ardelean <ardeleanalex@gmail.com>

If you want, you can also search for more of these xxx_set_drvdata() omissions.
There were more conversions to devm_ that forgot to remove those.
Maybe a cocci script would be nice to find them.
But all this is optional. Only if you want.

> Part of the driver was already using devm_ functions, so it was possible
> to convert the remainder of it without much hassle.
>
> With that, the deregistration function was no longer necessary and could
> be entirely removed from the driver.
>
> Lucas Stankus (3):
>   iio: adc: ad7923: use devm_add_action_or_reset for regulator disable
>   iio: adc: ad7923: use device-managed function for triggered buffer
>   iio: adc: ad7923: register device with devm_iio_device_register
>
>  drivers/iio/adc/ad7923.c | 39 +++++++++++++--------------------------
>  1 file changed, 13 insertions(+), 26 deletions(-)
>
> --
> 2.31.0
>

Re: [PATCH 3/3] iio: adc: ad7923: register device with devm_iio_device_register

[Jonathan Cameron]

On Mon, 29 Mar 2021 09:06:14 +0200
Lars-Peter Clausen <lars@metafoo.de> wrote:

> On 3/28/21 11:46 PM, Lucas Stankus wrote:
> > Registers the device using the devm variant.
> > This is the final step of converting the ad7923 to only use devm routines,
> > meaning that the ad7923_remove() function is no longer needed to release
> > resources on device detach.
> > 
> > Signed-off-by: Lucas Stankus <lucas.p.stankus@gmail.com>  
> 
> Hi,
> 
> Thanks for the patches.T his looks good, just one small comment.

On basis of saving everyone time, I've fixed up the comment from Lars
below whilst applying.

Applied to the togreg branch of iio.git and pushed out as testing for
the autobuilders to see if they can find anything we missed.

Thanks,

Jonathan

> 
> > ---
> >   drivers/iio/adc/ad7923.c | 12 +-----------
> >   1 file changed, 1 insertion(+), 11 deletions(-)
> > 
> > diff --git a/drivers/iio/adc/ad7923.c b/drivers/iio/adc/ad7923.c
> > index d07eaf3111ed..f7af2f194789 100644
> > --- a/drivers/iio/adc/ad7923.c
> > +++ b/drivers/iio/adc/ad7923.c
> > @@ -356,16 +356,7 @@ static int ad7923_probe(struct spi_device *spi)
> >   	if (ret)
> >   		return ret;
> >   
> > -	return iio_device_register(indio_dev);
> > -}
> > -
> > -static int ad7923_remove(struct spi_device *spi)
> > -{
> > -	struct iio_dev *indio_dev = spi_get_drvdata(spi);  
> 
> This removes the last user of get_drvdata() on the SPI device. This means you 
> can also remove the spi_set_drvdata() in the probe function.
> 
> > -
> > -	iio_device_unregister(indio_dev);
> > -
> > -	return 0;
> > +	return devm_iio_device_register(&spi->dev, indio_dev);
> >   }