Re: [PATCH 28/28] iio: adc: rcar-gyroadc: Use pm_runtime_resume_and_get() and check in probe()

[Mauro Carvalho Chehab <mchehab@kernel.org>]

Em Sun,  9 May 2021 12:33:54 +0100
Jonathan Cameron <jic23@kernel.org> escreveu:

> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> 1 instance found using coccicheck script under review at:
> https://lore.kernel.org/lkml/20210427141946.2478411-1-Julia.Lawall@inria.fr/
> The other instance changed did not check for failure of the
> pm_runtime_get_sync() so that is added.
> 
> Note the remaining pm_runtime_get_sync() call is left alone because it
> is not obvious what to do on failure to power up in remove()
> 
> This is a prequel to taking a closer look at the runtime pm in IIO drivers
> in general.
> 
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Cc: Marek Vasut <marek.vasut@gmail.com>

LGTM.

Reviewed-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

> ---
>  drivers/iio/adc/rcar-gyroadc.c | 15 +++++++--------
>  1 file changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/iio/adc/rcar-gyroadc.c b/drivers/iio/adc/rcar-gyroadc.c
> index 9f38cf3c7dc2..a48895046408 100644
> --- a/drivers/iio/adc/rcar-gyroadc.c
> +++ b/drivers/iio/adc/rcar-gyroadc.c
> @@ -162,18 +162,13 @@ static const struct iio_chan_spec rcar_gyroadc_iio_channels_3[] = {
>  static int rcar_gyroadc_set_power(struct rcar_gyroadc *priv, bool on)
>  {
>  	struct device *dev = priv->dev;
> -	int ret;
>  
>  	if (on) {
> -		ret = pm_runtime_get_sync(dev);
> -		if (ret < 0)
> -			pm_runtime_put_noidle(dev);
> +		return pm_runtime_resume_and_get(dev);
>  	} else {
>  		pm_runtime_mark_last_busy(dev);
> -		ret = pm_runtime_put_autosuspend(dev);
> +		return pm_runtime_put_autosuspend(dev);
>  	}
> -
> -	return ret;
>  }
>  
>  static int rcar_gyroadc_read_raw(struct iio_dev *indio_dev,
> @@ -535,7 +530,10 @@ static int rcar_gyroadc_probe(struct platform_device *pdev)
>  	pm_runtime_use_autosuspend(dev);
>  	pm_runtime_enable(dev);
>  
> -	pm_runtime_get_sync(dev);
> +	ret = pm_runtime_resume_and_get(dev);
> +	if (ret)
> +		goto err_power_up;
> +
>  	rcar_gyroadc_hw_init(priv);
>  	rcar_gyroadc_hw_start(priv);
>  
> @@ -552,6 +550,7 @@ static int rcar_gyroadc_probe(struct platform_device *pdev)
>  err_iio_device_register:
>  	rcar_gyroadc_hw_stop(priv);
>  	pm_runtime_put_sync(dev);
> +err_power_up:
>  	pm_runtime_disable(dev);
>  	pm_runtime_set_suspended(dev);
>  	clk_disable_unprepare(priv->clk);



Thanks,
Mauro