[PATCH v1] iio: adc: Use devm_iio_device_register() and dev_err_probe()

[PATCH v1] iio: adc: Use devm_iio_device_register() and dev_err_probe()

[César Bispo]

Replace iio_device_register() with devm_iio_device_register() to let the
core handle cleanup automatically. This simplifies the driver and avoids
manual error paths.

Also use dev_err_probe() for improved error handling and cleaner logs
when deferrals happen.

Signed-off-by: Cesar Bispo <cesar.bispo@ime.usp.br>
Co-developed-by: Gabriel Ferreira <gabrielfsouza.araujo@usp.br>
Signed-off-by: Gabriel Ferreira <gabrielfsouza.araujo@usp.br>
---
 drivers/iio/adc/qcom-pm8xxx-xoadc.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/adc/qcom-pm8xxx-xoadc.c b/drivers/iio/adc/qcom-pm8xxx-xoadc.c
index 31f88cf7f7f1..447a924eca6d 100644
--- a/drivers/iio/adc/qcom-pm8xxx-xoadc.c
+++ b/drivers/iio/adc/qcom-pm8xxx-xoadc.c
@@ -934,20 +934,18 @@ static int pm8xxx_xoadc_probe(struct platform_device *pdev)
 	indio_dev->channels = adc->iio_chans;
 	indio_dev->num_channels = adc->nchans;
 
-	ret = iio_device_register(indio_dev);
+	ret = devm_iio_device_register(dev, indio_dev);
 	if (ret)
-		goto out_disable_vref;
+		return dev_err_probe(dev, ret, "Unable to register %s\n", indio_dev->name);
 
 	ret = pm8xxx_calibrate_device(adc);
 	if (ret)
-		goto out_unreg_device;
+		return ret;
 
 	dev_info(dev, "%s XOADC driver enabled\n", variant->name);
 
 	return 0;
 
-out_unreg_device:
-	iio_device_unregister(indio_dev);
 out_disable_vref:
 	regulator_disable(adc->vref);
 
-- 
2.43.0

Re: [PATCH v1] iio: adc: Use devm_iio_device_register() and dev_err_probe()

[Jonathan Cameron]

On Tue, 24 Jun 2025 23:33:51 -0300
"César Bispo" <dm.cesaraugusto@gmail.com> wrote:

> Replace iio_device_register() with devm_iio_device_register() to let the
> core handle cleanup automatically. This simplifies the driver and avoids
> manual error paths.
> 
> Also use dev_err_probe() for improved error handling and cleaner logs
> when deferrals happen.
> 
Hi. Always include the driver name in the patch title so that people
can quickly see if they care about it not!

iio: adc: qcom-pm8xxx-xoadc: USe devm_iio_device_register() and dev_err_probe()

> Signed-off-by: Cesar Bispo <cesar.bispo@ime.usp.br>
> Co-developed-by: Gabriel Ferreira <gabrielfsouza.araujo@usp.br>
> Signed-off-by: Gabriel Ferreira <gabrielfsouza.araujo@usp.br>
> ---
>  drivers/iio/adc/qcom-pm8xxx-xoadc.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/iio/adc/qcom-pm8xxx-xoadc.c b/drivers/iio/adc/qcom-pm8xxx-xoadc.c
> index 31f88cf7f7f1..447a924eca6d 100644
> --- a/drivers/iio/adc/qcom-pm8xxx-xoadc.c
> +++ b/drivers/iio/adc/qcom-pm8xxx-xoadc.c
> @@ -934,20 +934,18 @@ static int pm8xxx_xoadc_probe(struct platform_device *pdev)
>  	indio_dev->channels = adc->iio_chans;
>  	indio_dev->num_channels = adc->nchans;
>  
> -	ret = iio_device_register(indio_dev);
> +	ret = devm_iio_device_register(dev, indio_dev);
Look at how devm works before considering any such changes.
This change creates a race where you have turned the
power off before removing the userspace interfaces which is a very bad idea.

>  	if (ret)
> -		goto out_disable_vref;
> +		return dev_err_probe(dev, ret, "Unable to register %s\n", indio_dev->name);
>  
>  	ret = pm8xxx_calibrate_device(adc);
>  	if (ret)
> -		goto out_unreg_device;
> +		return ret;
>  
>  	dev_info(dev, "%s XOADC driver enabled\n", variant->name);
>  
>  	return 0;
>  
> -out_unreg_device:
> -	iio_device_unregister(indio_dev);
>  out_disable_vref:
>  	regulator_disable(adc->vref);
>  

Re: [PATCH v1] iio: adc: qcom-pm8xxx-xoadc: Use devm_iio_device_register() and dev_err_probe()

[César Bispo]

From: Cesar Bispo <cesar.bispo@ime.usp.br>

Hi Jonathan,

Thanks for your feedback.

Just to clarify, the use of `devm_iio_device_register()` in this patch was based on your previous suggestion [1], so I assumed it would be appropriate in this context.

I now understand there might be a risk of userspace-visible interfaces persisting after the device is powered off, which could lead to race conditions.

Would you recommend dropping the `devm_` conversion for now and only keeping the `dev_err_probe()` change?

I'm happy to send a v2 accordingly.

[1] https://lore.kernel.org/linux-iio/20250607163353.47e83e77@jic23-huawei/

Re: [PATCH v1] iio: adc: qcom-pm8xxx-xoadc: Use devm_iio_device_register() and dev_err_probe()

[Jonathan Cameron]

On Wed, 25 Jun 2025 10:27:27 -0300
"César Bispo" <dm.cesaraugusto@gmail.com> wrote:

> From: Cesar Bispo <cesar.bispo@ime.usp.br>
> 
> Hi Jonathan,
> 
> Thanks for your feedback.
> 
> Just to clarify, the use of `devm_iio_device_register()` in this patch was based on your previous suggestion [1], so I assumed it would be appropriate in this context.
> 
> I now understand there might be a risk of userspace-visible interfaces persisting after the device is powered off, which could lead to race conditions.
> 
> Would you recommend dropping the `devm_` conversion for now and only keeping the `dev_err_probe()` change?
> 
> I'm happy to send a v2 accordingly.
> 
> [1] https://lore.kernel.org/linux-iio/20250607163353.47e83e77@jic23-huawei/

I was suggesting the use of devm_iio_device_register()  'on top' of the changes
in that patch. My confusion here is that you've sent out code based on the
tree before that patch and hence the regulator handling is still manual.

Looking again at that patch it has a bug. Replied there and dropped that patch.


Jonathan