Re: [PATCH v2 2/2] iio: adc: sun4i-gpadc-iio: Fix error handle when devm_iio_device_register() failed in sun4i_gpadc_probe()

[Jonathan Cameron <jic23@kernel.org>]

On Thu, 20 Oct 2022 20:40:45 +0800
Zhang Qilong <zhangqilong3@huawei.com> wrote:

> If devm_iio_device_register() failed, the thermal_zone may have been
> registered. So we need call thermal_zone_of_sensor_unregister() when
> CONFIG_THERMAL_OF is enabled. We fix it by adding a err_register and
> gotoing it when devm_iio_device_register() failed.

This doesn't look right.  Any devm_ registered calls should be cleaned
up automatically if we fail the probe.

However, there is an issue in what you've hightlighed here in that
on the remove path we will have ripped out the iio_map_register() before
we remove the thermal zone that is dependent on it.

Easiest fix for that is probably to use devm_iio_array_map_register()
to allow the managed handling to clean that up at the correct point.

Follow on that path you may also be able to use devm_pm_runtime_enable()
here to deal with the tear down of runtime pm (which is currently wrong
anyway as no autosuspend disable.)

Thanks,

Jonathan



> 
> Fixes: d1caa9905538 ("iio: adc: add support for Allwinner SoCs ADC")
> Signed-off-by: Zhang Qilong <zhangqilong3@huawei.com>
> ---
>  drivers/iio/adc/sun4i-gpadc-iio.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
> index d2535dd28af8..04717571cb2e 100644
> --- a/drivers/iio/adc/sun4i-gpadc-iio.c
> +++ b/drivers/iio/adc/sun4i-gpadc-iio.c
> @@ -656,11 +656,17 @@ static int sun4i_gpadc_probe(struct platform_device *pdev)
>  	ret = devm_iio_device_register(&pdev->dev, indio_dev);
>  	if (ret < 0) {
>  		dev_err(&pdev->dev, "could not register the device\n");
> -		goto err_map;
> +		goto err_register;
>  	}
>  
>  	return 0;
>  
> +err_register:
> +	if (IS_ENABLED(CONFIG_THERMAL_OF)) {
> +		devm_thermal_of_zone_unregister(info->sensor_device,
> +						info->tzd);
> +		info->tzd = NULL;
> +	}
>  err_map:
>  	if (!info->no_irq && IS_ENABLED(CONFIG_THERMAL_OF))
>  		iio_map_array_unregister(indio_dev);