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

[Jonathan Cameron <jic23@kernel.org>]

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;
> >  }
> >    
> 
>