Re: [PATCH v2 1/4] iio: imu: inv_mpu6050: fix error path not turning chip back off

[Jonathan Cameron <jic23@jic23.retrosnub.co.uk>]

On Mon, 9 Apr 2018 16:57:05 +0200
Jean-Baptiste Maneyrol <jmaneyrol@invensense.com> wrote:

> Some functions are turning the chip on and not back off in error
> path. With set_power function using a reference counter that
> would keep the chip on forever.
> 
> Signed-off-by: Jean-Baptiste Maneyrol <jmaneyrol@invensense.com>
> ---
>   drivers/iio/imu/inv_mpu6050/inv_mpu_core.c    | 12 ++++++---
>   drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c | 35 
> ++++++++++++++++++---------
>   2 files changed, 32 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c 
> b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> index 7d64be3..18c8866 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> @@ -262,27 +262,31 @@ static int inv_mpu6050_init_config(struct iio_dev 
> *indio_dev)
Patch is fine in terms of content, but it is line wrapped..

Could you resend this series, rebased on the current content of the
testing branch of iio.git please.

Please roll all 4 up as a series as otherwise they can be a pain
to apply.

As you know this driver code is changing fairly often at the moment
making it far more likely I'll get a merge wrong!

Thanks,

Jonathan


>   	d = (INV_MPU6050_FSR_2000DPS << INV_MPU6050_GYRO_CONFIG_FSR_SHIFT);
>   	result = regmap_write(st->map, st->reg->gyro_config, d);
>   	if (result)
> -		return result;
> +		goto error_power_off;
> 
>   	result = inv_mpu6050_set_lpf_regs(st, INV_MPU6050_FILTER_20HZ);
>   	if (result)
> -		return result;
> +		goto error_power_off;
> 
>   	d = INV_MPU6050_ONE_K_HZ / INV_MPU6050_INIT_FIFO_RATE - 1;
>   	result = regmap_write(st->map, st->reg->sample_rate_div, d);
>   	if (result)
> -		return result;
> +		goto error_power_off;
> 
>   	d = (INV_MPU6050_FS_02G << INV_MPU6050_ACCL_CONFIG_FSR_SHIFT);
>   	result = regmap_write(st->map, st->reg->accl_config, d);
>   	if (result)
> -		return result;
> +		goto error_power_off;
> 
>   	memcpy(&st->chip_config, hw_info[st->chip_type].config,
>   	       sizeof(struct inv_mpu6050_chip_config));
>   	result = inv_mpu6050_set_power_itg(st, false);
> 
>   	return result;
> +
> +error_power_off:
> +	inv_mpu6050_set_power_itg(st, false);
> +	return result;
>   }
> 
>   static int inv_mpu6050_sensor_set(struct inv_mpu6050_state  *st, int reg,
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c 
> b/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
> index f963f9f..27aa976 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
> @@ -53,45 +53,58 @@ static int inv_mpu6050_set_enable(struct iio_dev 
> *indio_dev, bool enable)
>   			result = inv_mpu6050_switch_engine(st, true,
>   					INV_MPU6050_BIT_PWR_GYRO_STBY);
>   			if (result)
> -				return result;
> +				goto error_power_off;
>   		}
>   		if (st->chip_config.accl_fifo_enable) {
>   			result = inv_mpu6050_switch_engine(st, true,
>   					INV_MPU6050_BIT_PWR_ACCL_STBY);
>   			if (result)
> -				return result;
> +				goto error_gyro_off;
>   		}
>   		result = inv_reset_fifo(indio_dev);
>   		if (result)
> -			return result;
> +			goto error_accl_off;
>   	} else {
>   		result = regmap_write(st->map, st->reg->fifo_en, 0);
>   		if (result)
> -			return result;
> +			goto error_accl_off;
> 
>   		result = regmap_write(st->map, st->reg->int_enable, 0);
>   		if (result)
> -			return result;
> +			goto error_accl_off;
> 
>   		result = regmap_write(st->map, st->reg->user_ctrl, 0);
>   		if (result)
> -			return result;
> +			goto error_accl_off;
> 
>   		result = inv_mpu6050_switch_engine(st, false,
> -					INV_MPU6050_BIT_PWR_GYRO_STBY);
> +					INV_MPU6050_BIT_PWR_ACCL_STBY);
>   		if (result)
> -			return result;
> +			goto error_accl_off;
> 
>   		result = inv_mpu6050_switch_engine(st, false,
> -					INV_MPU6050_BIT_PWR_ACCL_STBY);
> +					INV_MPU6050_BIT_PWR_GYRO_STBY);
>   		if (result)
> -			return result;
> +			goto error_gyro_off;
> +
>   		result = inv_mpu6050_set_power_itg(st, false);
>   		if (result)
> -			return result;
> +			goto error_power_off;
>   	}
> 
>   	return 0;
> +
> +error_accl_off:
> +	if (st->chip_config.accl_fifo_enable)
> +		inv_mpu6050_switch_engine(st, false,
> +					  INV_MPU6050_BIT_PWR_ACCL_STBY);
> +error_gyro_off:
> +	if (st->chip_config.gyro_fifo_enable)
> +		inv_mpu6050_switch_engine(st, false,
> +					  INV_MPU6050_BIT_PWR_GYRO_STBY);
> +error_power_off:
> +	inv_mpu6050_set_power_itg(st, false);
> +	return result;
>   }
> 
>   /**