Re: [PATCH] iio: imu: inv_mpu6050: fix suspend/resume with runtime power

[Jonathan Cameron <jic23@kernel.org>]

On Tue, 31 Mar 2020 15:38:50 +0200
Jean-Baptiste Maneyrol <jmaneyrol@invensense.com> wrote:

> Suspend/resume were not working correctly with pm runtime.

Need more info than that!
Anyhow, when you say "not working correctly" what is happening that
is wrong?

Jonathan


> Now suspend check if the chip is already suspended, and
> resume put runtime pm in the correct state.
> 
> Fixes: 4599cac84614 ("iio: imu: inv_mpu6050: use runtime pm with autosuspend")
> Signed-off-by: Jean-Baptiste Maneyrol <jmaneyrol@invensense.com>
> ---
>  drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> index e4b0d368c2f9..a58bab03f0b0 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> @@ -1636,6 +1636,10 @@ static int __maybe_unused inv_mpu_resume(struct device *dev)
>  	if (result)
>  		goto out_unlock;
>  
> +	pm_runtime_disable(dev);
> +	pm_runtime_set_active(dev);
> +	pm_runtime_enable(dev);
> +

Looking at the docs, we should do this if we were previously suspended and no longer
are.  Not sure we should do it we weren't previously in runtime suspend?

I guess it is idempotent anyway so if we were previously enabled we just set it again.
So probably fine.

Jonathan

>  	result = inv_mpu6050_switch_engine(st, true, st->suspended_sensors);
>  	if (result)
>  		goto out_unlock;
> @@ -1657,13 +1661,18 @@ static int __maybe_unused inv_mpu_suspend(struct device *dev)
>  
>  	mutex_lock(&st->lock);
>  
> +	st->suspended_sensors = 0;
> +	if (pm_runtime_suspended(dev)) {
> +		result = 0;
> +		goto out_unlock;
> +	}
> +
>  	if (iio_buffer_enabled(indio_dev)) {
>  		result = inv_mpu6050_prepare_fifo(st, false);
>  		if (result)
>  			goto out_unlock;
>  	}
>  
> -	st->suspended_sensors = 0;
>  	if (st->chip_config.accl_en)
>  		st->suspended_sensors |= INV_MPU6050_SENSOR_ACCL;
>  	if (st->chip_config.gyro_en)