[PATCH v4] iio: adc: Modernize single regulator call

[PATCH v4] iio: adc: Modernize single regulator call

[Gabriel Souza Araujo]

Replace a single instance of legacy regulator with devm_regulator_get_enable().
This change improves code clarity and aligns with modern kernel APIs.

Signed-off-by: Gabriel Souza Araujo <gabrielfsouza.araujo@gmail.com>
Co-developed-by: Cesar Bispo <cesar.bispo@ime.usp.br>
Signed-off-by: Cesar Bispo <cesar.bispo@ime.usp.br>
---
The first version [1] did not compile due to incorrect use of pointers and improper
parameterization of the devm_regulator_get_enable() function. 
These issues have been fixed, and the code now compiles successfully.
Additionally, version two [2] had an incomplete patch, which has also 
been addressed in this version. In version three [3], dev_err_probe() is used for improved error handling.

[1] https://lore.kernel.org/linux-iio/20250429160526.5934-1-cesar.bispo@ime.usp.br/
[2] https://lore.kernel.org/linux-iio/20250514185239.10078-1-gabrielfsouza.araujo@gmail.com/
[3] https://lore.kernel.org/linux-iio/20250521185807.139828-1-gabrielfsouza.araujo@gmail.com/

 drivers/iio/adc/qcom-pm8xxx-xoadc.c | 17 ++++-------------
 1 file changed, 4 insertions(+), 13 deletions(-)

diff --git a/drivers/iio/adc/qcom-pm8xxx-xoadc.c b/drivers/iio/adc/qcom-pm8xxx-xoadc.c
index 31f88cf7f7f1..ab1ed79f9f39 100644
--- a/drivers/iio/adc/qcom-pm8xxx-xoadc.c
+++ b/drivers/iio/adc/qcom-pm8xxx-xoadc.c
@@ -911,21 +911,15 @@ static int pm8xxx_xoadc_probe(struct platform_device *pdev)
 	adc->map = map;
 
 	/* Bring up regulator */
-	adc->vref = devm_regulator_get(dev, "xoadc-ref");
-	if (IS_ERR(adc->vref))
-		return dev_err_probe(dev, PTR_ERR(adc->vref),
-				     "failed to get XOADC VREF regulator\n");
-	ret = regulator_enable(adc->vref);
+	ret = devm_regulator_get_enable(dev, "xoadc-ref");
 	if (ret) {
-		dev_err(dev, "failed to enable XOADC VREF regulator\n");
-		return ret;
+		return dev_err_probe(dev, ret, "failed to enable XOADC VREF regulator\n");
 	}
 
 	ret = devm_request_threaded_irq(dev, platform_get_irq(pdev, 0),
 			pm8xxx_eoc_irq, NULL, 0, variant->name, indio_dev);
 	if (ret) {
-		dev_err(dev, "unable to request IRQ\n");
-		goto out_disable_vref;
+		return dev_err_probe(dev, ret, "unable to request IRQ\n");
 	}
 
 	indio_dev->name = variant->name;
@@ -936,7 +930,7 @@ static int pm8xxx_xoadc_probe(struct platform_device *pdev)
 
 	ret = iio_device_register(indio_dev);
 	if (ret)
-		goto out_disable_vref;
+		return ret;
 
 	ret = pm8xxx_calibrate_device(adc);
 	if (ret)
@@ -948,9 +942,6 @@ static int pm8xxx_xoadc_probe(struct platform_device *pdev)
 
 out_unreg_device:
 	iio_device_unregister(indio_dev);
-out_disable_vref:
-	regulator_disable(adc->vref);
-
 	return ret;
 }
 
-- 
2.45.2

Re: [PATCH v4] iio: adc: Modernize single regulator call

[Jonathan Cameron]

On Fri,  6 Jun 2025 22:15:22 -0300
Gabriel Souza Araujo <gabrielfsouza.araujo@gmail.com> wrote:

> Replace a single instance of legacy regulator with devm_regulator_get_enable().
> This change improves code clarity and aligns with modern kernel APIs.
I made one final change whilst applying which was to include the driver name in the patch title.

Applied but I'll be rebasing my tree on rc1 once available so for now it will only go out
as testing.

Ideally follow up with a patch to use devm_iio_device_register() and get rid of the remove()
callback entirely.

> 
> Signed-off-by: Gabriel Souza Araujo <gabrielfsouza.araujo@gmail.com>
> Co-developed-by: Cesar Bispo <cesar.bispo@ime.usp.br>
> Signed-off-by: Cesar Bispo <cesar.bispo@ime.usp.br>
> ---
> The first version [1] did not compile due to incorrect use of pointers and improper
> parameterization of the devm_regulator_get_enable() function. 
> These issues have been fixed, and the code now compiles successfully.
> Additionally, version two [2] had an incomplete patch, which has also 
> been addressed in this version. In version three [3], dev_err_probe() is used for improved error handling.
> 
> [1] https://lore.kernel.org/linux-iio/20250429160526.5934-1-cesar.bispo@ime.usp.br/
> [2] https://lore.kernel.org/linux-iio/20250514185239.10078-1-gabrielfsouza.araujo@gmail.com/
> [3] https://lore.kernel.org/linux-iio/20250521185807.139828-1-gabrielfsouza.araujo@gmail.com/
> 
>  drivers/iio/adc/qcom-pm8xxx-xoadc.c | 17 ++++-------------
>  1 file changed, 4 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/iio/adc/qcom-pm8xxx-xoadc.c b/drivers/iio/adc/qcom-pm8xxx-xoadc.c
> index 31f88cf7f7f1..ab1ed79f9f39 100644
> --- a/drivers/iio/adc/qcom-pm8xxx-xoadc.c
> +++ b/drivers/iio/adc/qcom-pm8xxx-xoadc.c
> @@ -911,21 +911,15 @@ static int pm8xxx_xoadc_probe(struct platform_device *pdev)
>  	adc->map = map;
>  
>  	/* Bring up regulator */
> -	adc->vref = devm_regulator_get(dev, "xoadc-ref");
> -	if (IS_ERR(adc->vref))
> -		return dev_err_probe(dev, PTR_ERR(adc->vref),
> -				     "failed to get XOADC VREF regulator\n");
> -	ret = regulator_enable(adc->vref);
> +	ret = devm_regulator_get_enable(dev, "xoadc-ref");
>  	if (ret) {
> -		dev_err(dev, "failed to enable XOADC VREF regulator\n");
> -		return ret;
> +		return dev_err_probe(dev, ret, "failed to enable XOADC VREF regulator\n");
>  	}
>  
>  	ret = devm_request_threaded_irq(dev, platform_get_irq(pdev, 0),
>  			pm8xxx_eoc_irq, NULL, 0, variant->name, indio_dev);
>  	if (ret) {
> -		dev_err(dev, "unable to request IRQ\n");
> -		goto out_disable_vref;
> +		return dev_err_probe(dev, ret, "unable to request IRQ\n");
>  	}
>  
>  	indio_dev->name = variant->name;
> @@ -936,7 +930,7 @@ static int pm8xxx_xoadc_probe(struct platform_device *pdev)
>  
>  	ret = iio_device_register(indio_dev);
>  	if (ret)
> -		goto out_disable_vref;
> +		return ret;
>  
>  	ret = pm8xxx_calibrate_device(adc);
>  	if (ret)
> @@ -948,9 +942,6 @@ static int pm8xxx_xoadc_probe(struct platform_device *pdev)
>  
>  out_unreg_device:
>  	iio_device_unregister(indio_dev);
> -out_disable_vref:
> -	regulator_disable(adc->vref);
> -
>  	return ret;
>  }
>  

Re: [PATCH v4] iio: adc: Modernize single regulator call

[Jonathan Cameron]

On Sat, 7 Jun 2025 16:33:53 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> On Fri,  6 Jun 2025 22:15:22 -0300
> Gabriel Souza Araujo <gabrielfsouza.araujo@gmail.com> wrote:
> 
> > Replace a single instance of legacy regulator with devm_regulator_get_enable().
> > This change improves code clarity and aligns with modern kernel APIs.  
> I made one final change whilst applying which was to include the driver name in the patch title.
> 
> Applied but I'll be rebasing my tree on rc1 once available so for now it will only go out
> as testing.
> 
> Ideally follow up with a patch to use devm_iio_device_register() and get rid of the remove()
> callback entirely.
> 
> > 
> > Signed-off-by: Gabriel Souza Araujo <gabrielfsouza.araujo@gmail.com>
> > Co-developed-by: Cesar Bispo <cesar.bispo@ime.usp.br>
> > Signed-off-by: Cesar Bispo <cesar.bispo@ime.usp.br>

After looking again via your other patch I notice this is wrong and I've dropped it for
now.  Look again at what devm_ does and consider the remove() callback for this function.

If you want to continue please send a little series with both patches in it that resolves
the problem (hint drop the regulator_disable in remove() as it will be disabled by the
devm_ at the right point in the remove sequence.

Thanks,

Jonathan

> > ---
> > The first version [1] did not compile due to incorrect use of pointers and improper
> > parameterization of the devm_regulator_get_enable() function. 
> > These issues have been fixed, and the code now compiles successfully.
> > Additionally, version two [2] had an incomplete patch, which has also 
> > been addressed in this version. In version three [3], dev_err_probe() is used for improved error handling.
> > 
> > [1] https://lore.kernel.org/linux-iio/20250429160526.5934-1-cesar.bispo@ime.usp.br/
> > [2] https://lore.kernel.org/linux-iio/20250514185239.10078-1-gabrielfsouza.araujo@gmail.com/
> > [3] https://lore.kernel.org/linux-iio/20250521185807.139828-1-gabrielfsouza.araujo@gmail.com/
> > 
> >  drivers/iio/adc/qcom-pm8xxx-xoadc.c | 17 ++++-------------
> >  1 file changed, 4 insertions(+), 13 deletions(-)
> > 
> > diff --git a/drivers/iio/adc/qcom-pm8xxx-xoadc.c b/drivers/iio/adc/qcom-pm8xxx-xoadc.c
> > index 31f88cf7f7f1..ab1ed79f9f39 100644
> > --- a/drivers/iio/adc/qcom-pm8xxx-xoadc.c
> > +++ b/drivers/iio/adc/qcom-pm8xxx-xoadc.c
> > @@ -911,21 +911,15 @@ static int pm8xxx_xoadc_probe(struct platform_device *pdev)
> >  	adc->map = map;
> >  
> >  	/* Bring up regulator */
> > -	adc->vref = devm_regulator_get(dev, "xoadc-ref");
> > -	if (IS_ERR(adc->vref))
> > -		return dev_err_probe(dev, PTR_ERR(adc->vref),
> > -				     "failed to get XOADC VREF regulator\n");
> > -	ret = regulator_enable(adc->vref);
> > +	ret = devm_regulator_get_enable(dev, "xoadc-ref");
> >  	if (ret) {
> > -		dev_err(dev, "failed to enable XOADC VREF regulator\n");
> > -		return ret;
> > +		return dev_err_probe(dev, ret, "failed to enable XOADC VREF regulator\n");
> >  	}
> >  
> >  	ret = devm_request_threaded_irq(dev, platform_get_irq(pdev, 0),
> >  			pm8xxx_eoc_irq, NULL, 0, variant->name, indio_dev);
> >  	if (ret) {
> > -		dev_err(dev, "unable to request IRQ\n");
> > -		goto out_disable_vref;
> > +		return dev_err_probe(dev, ret, "unable to request IRQ\n");
> >  	}
> >  
> >  	indio_dev->name = variant->name;
> > @@ -936,7 +930,7 @@ static int pm8xxx_xoadc_probe(struct platform_device *pdev)
> >  
> >  	ret = iio_device_register(indio_dev);
> >  	if (ret)
> > -		goto out_disable_vref;
> > +		return ret;
> >  
> >  	ret = pm8xxx_calibrate_device(adc);
> >  	if (ret)
> > @@ -948,9 +942,6 @@ static int pm8xxx_xoadc_probe(struct platform_device *pdev)
> >  
> >  out_unreg_device:
> >  	iio_device_unregister(indio_dev);
> > -out_disable_vref:
> > -	regulator_disable(adc->vref);
> > -
> >  	return ret;
> >  }
> >    
> 
>