Re: [PATCH 3/6] iio: imx7d_adc: Use devm_iio_device_register()

[Jonathan Cameron]

On Wed,  3 Apr 2019 00:03:22 -0700
Andrey Smirnov <andrew.smirnov@xxxxxxxxx> wrote:

> Use devm_iio_device_register() and drop explicit call to
> iio_device_unregister().
> 
> Signed-off-by: Andrey Smirnov <andrew.smirnov@xxxxxxxxx>
> Cc: Jonathan Cameron <jic23@xxxxxxxxxx>
> Cc: Hartmut Knaack <knaack.h@xxxxxx>
> Cc: Lars-Peter Clausen <lars@xxxxxxxxxx>
> Cc: Peter Meerwald-Stadler <pmeerw@xxxxxxxxxx>
> Cc: Chris Healy <cphealy@xxxxxxxxx>
> Cc: linux-iio@xxxxxxxxxxxxxxx
> Cc: linux-kernel@xxxxxxxxxxxxxxx
No to this one.  The thing to think about is the resulting order
of the unwinding that happens in remove.

Do take a look at the code flow, but in short what happens is:

1. driver.remove()
2. Devm release functions run in the opposite order to they were
   called during setup.

The upshot of the change you just made here is that we turn the power
off to the device before we remove the userspace interfaces, giving
potentially interesting failures..

There are two options to avoid this:

1. Make everything use devm_ calls (often using devm_add_action_or_reset
for the ones that don't have their own versions).

2. Stop using devm managed functions at the first non devm_ call that needs
unwinding.  From that point onwards in probe / remove you have to do everything
manually.

Jonathan

> ---
>  drivers/iio/adc/imx7d_adc.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/adc/imx7d_adc.c b/drivers/iio/adc/imx7d_adc.c
> index 72cfe9834bcb..9a46838ec7cf 100644
> --- a/drivers/iio/adc/imx7d_adc.c
> +++ b/drivers/iio/adc/imx7d_adc.c
> @@ -517,7 +517,7 @@ static int imx7d_adc_probe(struct platform_device *pdev)
>  	imx7d_adc_feature_config(info);
>  	imx7d_adc_hw_init(info);
>  
> -	ret = iio_device_register(indio_dev);
> +	ret = devm_iio_device_register(dev, indio_dev);
>  	if (ret) {
>  		imx7d_adc_power_down(info);
>  		dev_err(&pdev->dev, "Couldn't register the device.\n");
> @@ -539,8 +539,6 @@ static int imx7d_adc_remove(struct platform_device *pdev)
>  	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
>  	struct imx7d_adc *info = iio_priv(indio_dev);
>  
> -	iio_device_unregister(indio_dev);
> -
>  	imx7d_adc_power_down(info);
>  
>  	clk_disable_unprepare(info->clk);