Re: [PATCH 6/6] iio: imx7d_adc: Drop imx7d_adc_remove()

[Jonathan Cameron]

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

> Since imx7d_adc_remove() does exactly the same thing as
> imx7d_adc_suspend() we can use the latter together with
> devm_add_action_or_reset() to get rid of the former.
> 
> 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
Ah. OK, so you end up with one of the options I listed when rejecting
the earlier change for iio_device_register.

Please merge this patch with that one so we have a single change and don't
end up with a race condition prone state in between. Separate patches
are fine as well, but work your way down through probe rather than jumping
around like now.  Hence we'll have something valid at all stages.

Thanks,

Jonathan


> ---
>  drivers/iio/adc/imx7d_adc.c | 55 ++++++++++++++++---------------------
>  1 file changed, 23 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/iio/adc/imx7d_adc.c b/drivers/iio/adc/imx7d_adc.c
> index c2fa0ab923ee..ed95909eada2 100644
> --- a/drivers/iio/adc/imx7d_adc.c
> +++ b/drivers/iio/adc/imx7d_adc.c
> @@ -461,6 +461,24 @@ static int imx7d_adc_resume(struct device *dev)
>  	return 0;
>  }
>  
> +static int imx7d_adc_suspend(struct device *dev)
> +{
> +	struct iio_dev *indio_dev = dev_get_drvdata(dev);
> +	struct imx7d_adc *info = iio_priv(indio_dev);
> +
> +	imx7d_adc_power_down(info);
> +
> +	clk_disable_unprepare(info->clk);
> +	regulator_disable(info->vref);
> +
> +	return 0;
> +}
> +
> +static void __imx7d_adc_suspend(void *data)
> +{
> +	imx7d_adc_suspend(data);
> +}
> +
>  static int imx7d_adc_probe(struct platform_device *pdev)
>  {
>  	struct imx7d_adc *info;
> @@ -506,8 +524,6 @@ static int imx7d_adc_probe(struct platform_device *pdev)
>  		return ret;
>  	}
>  
> -	platform_set_drvdata(pdev, indio_dev);
> -
>  	init_completion(&info->completion);
>  
>  	indio_dev->name = dev_name(dev);
> @@ -531,11 +547,13 @@ static int imx7d_adc_probe(struct platform_device *pdev)
>  	if (ret)
>  		return ret;
>  
> +	ret = devm_add_action_or_reset(dev, __imx7d_adc_suspend,
> +				       &indio_dev->dev);
> +	if (ret)
> +		return ret;
> +
>  	ret = devm_iio_device_register(dev, indio_dev);
>  	if (ret) {
> -		imx7d_adc_power_down(info);
> -		clk_disable_unprepare(info->clk);
> -		regulator_disable(info->vref);
>  		dev_err(&pdev->dev, "Couldn't register the device.\n");
>  		return ret;
>  	}
> @@ -543,37 +561,10 @@ static int imx7d_adc_probe(struct platform_device *pdev)
>  	return 0;
>  }
>  
> -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);
> -
> -	imx7d_adc_power_down(info);
> -
> -	clk_disable_unprepare(info->clk);
> -	regulator_disable(info->vref);
> -
> -	return 0;
> -}
> -
> -static int __maybe_unused imx7d_adc_suspend(struct device *dev)
> -{
> -	struct iio_dev *indio_dev = dev_get_drvdata(dev);
> -	struct imx7d_adc *info = iio_priv(indio_dev);
> -
> -	imx7d_adc_power_down(info);
> -
> -	clk_disable_unprepare(info->clk);
> -	regulator_disable(info->vref);
> -
> -	return 0;
> -}
> -
>  static SIMPLE_DEV_PM_OPS(imx7d_adc_pm_ops, imx7d_adc_suspend, imx7d_adc_resume);
>  
>  static struct platform_driver imx7d_adc_driver = {
>  	.probe		= imx7d_adc_probe,
> -	.remove		= imx7d_adc_remove,
>  	.driver		= {
>  		.name	= "imx7d_adc",
>  		.of_match_table = imx7d_adc_match,