lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:	Sun, 08 Mar 2015 11:06:48 +0000
From:	Jonathan Cameron <jic23@...nel.org>
To:	"Suman, Viorel" <viorel.suman@...el.com>,
	"Reus, Adriana" <adriana.reus@...el.com>,
	"linux-iio@...r.kernel.org" <linux-iio@...r.kernel.org>
CC:	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"Pandruvada, Srinivas" <srinivas.pandruvada@...el.com>,
	Ge Gao <ggao@...ensense.com>
Subject: Re: [PATCH] iio:inv-mpu6050: Fix inconsistency for the scale channel

On 04/03/15 13:57, Suman, Viorel wrote:
> 
>> -----Original Message-----
>> From: linux-iio-owner@...r.kernel.org [mailto:linux-iio-
>> owner@...r.kernel.org] On Behalf Of Adriana Reus
>> Sent: Monday, February 23, 2015 4:41 PM
>> To: jic23@...nel.org; linux-iio@...r.kernel.org
>> Cc: linux-kernel@...r.kernel.org; Pandruvada, Srinivas; Reus, Adriana
>> Subject: [PATCH] iio:inv-mpu6050: Fix inconsistency for the scale channel
>>
>> Fix inconsistency in the semantics of the scale attribute.
>> For scale the write_raw function was considering the scale table index and
>> writing the appropriate value into the range register, while for read_raw it
>> was outputting the actual scale.
>> Fix this behaviour and adhere to the iio ABI specification.
>>
>> Signed-off-by: Adriana Reus <adriana.reus@...el.com>
> 
> Reviewed-by: Viorel Suman <viorel.suman@...el.com>
Good catch.  I'd just like to give Ge an opportunity to comment
before applying it.

Please do CC the original driver author as often they've moved on to
other stuff and aren't actively reading linux-iio anymore.

Jonathan
> 
>> ---
>>  drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 56 ++++++++++++++++--
>> ------------
>>  1 file changed, 30 insertions(+), 26 deletions(-)
>>
>> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
>> b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
>> index 9be9b20..4cf056d 100644
>> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
>> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
>> @@ -401,42 +401,46 @@ error_read_raw:
>>  	}
>>  }
>>
>> -static int inv_mpu6050_write_fsr(struct inv_mpu6050_state *st, int fsr)
>> +static int inv_mpu6050_write_gyro_scale(struct inv_mpu6050_state *st,
>> +int val)
>>  {
>> -	int result;
>> +	int result, i;
>>  	u8 d;
>>
>> -	if (fsr < 0 || fsr > INV_MPU6050_MAX_GYRO_FS_PARAM)
>> -		return -EINVAL;
>> -	if (fsr == st->chip_config.fsr)
>> -		return 0;
>> +	for (i = 0; i < ARRAY_SIZE(gyro_scale_6050); ++i) {
>> +		if (gyro_scale_6050[i] == val) {
>> +			d = (i <<
>> INV_MPU6050_GYRO_CONFIG_FSR_SHIFT);
>> +			result = inv_mpu6050_write_reg(st,
>> +					st->reg->gyro_config, d);
>> +			if (result)
>> +				return result;
>>
>> -	d = (fsr << INV_MPU6050_GYRO_CONFIG_FSR_SHIFT);
>> -	result = inv_mpu6050_write_reg(st, st->reg->gyro_config, d);
>> -	if (result)
>> -		return result;
>> -	st->chip_config.fsr = fsr;
>> +			st->chip_config.fsr = i;
>> +			return 0;
>> +		}
>> +	}
>>
>> -	return 0;
>> +	return -EINVAL;
>>  }
>>
>> -static int inv_mpu6050_write_accel_fs(struct inv_mpu6050_state *st, int
>> fs)
>> +static int inv_mpu6050_write_accel_scale(struct inv_mpu6050_state *st,
>> +int val)
>>  {
>> -	int result;
>> +	int result, i;
>>  	u8 d;
>>
>> -	if (fs < 0 || fs > INV_MPU6050_MAX_ACCL_FS_PARAM)
>> -		return -EINVAL;
>> -	if (fs == st->chip_config.accl_fs)
>> -		return 0;
>> +	for (i = 0; i < ARRAY_SIZE(accel_scale); ++i) {
>> +		if (accel_scale[i] == val) {
>> +			d = (i << INV_MPU6050_ACCL_CONFIG_FSR_SHIFT);
>> +			result = inv_mpu6050_write_reg(st,
>> +					st->reg->accl_config, d);
>> +			if (result)
>> +				return result;
>>
>> -	d = (fs << INV_MPU6050_ACCL_CONFIG_FSR_SHIFT);
>> -	result = inv_mpu6050_write_reg(st, st->reg->accl_config, d);
>> -	if (result)
>> -		return result;
>> -	st->chip_config.accl_fs = fs;
>> +			st->chip_config.accl_fs = i;
>> +			return 0;
>> +		}
>> +	}
>>
>> -	return 0;
>> +	return -EINVAL;
>>  }
>>
>>  static int inv_mpu6050_write_raw(struct iio_dev *indio_dev, @@ -463,10
>> +467,10 @@ static int inv_mpu6050_write_raw(struct iio_dev *indio_dev,
>>  	case IIO_CHAN_INFO_SCALE:
>>  		switch (chan->type) {
>>  		case IIO_ANGL_VEL:
>> -			result = inv_mpu6050_write_fsr(st, val);
>> +			result = inv_mpu6050_write_gyro_scale(st, val2);
>>  			break;
>>  		case IIO_ACCEL:
>> -			result = inv_mpu6050_write_accel_fs(st, val);
>> +			result = inv_mpu6050_write_accel_scale(st, val2);
>>  			break;
>>  		default:
>>  			result = -EINVAL;
>> --
>> 1.9.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-iio" in the
>> body of a message to majordomo@...r.kernel.org More majordomo info
>> at  http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists