Re: [PATCH] iio: imu: inv_mpu6050: add accel lpf register setting for chip >= MPU6500

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



>From: Jonathan Cameron <jic23@xxxxxxxxxx>
>Sent: Sunday, May 21, 2017 14:09
>To: Jean-Baptiste Maneyrol; linux-iio
>Subject: Re: [PATCH] iio: imu: inv_mpu6050: add accel lpf register setting for chip >= MPU6500
>    
>On 16/05/17 14:12, Jean-Baptiste Maneyrol wrote:
>> Starting from MPU6500, accelerometer dlpf is set in a separate register
>> named ACCEL_CONFIG_2.
>> Add this new register in the map and set it for the corresponding chips.
>> 
>> Signed-off-by: Jean-Baptiste Maneyrol <jmaneyrol@xxxxxxxxxxxxxx>
>Perhaps a fixes tag as we presumably want this to go to stable?  Can see some rather
>unexpected results occurring without it.
>
>Hmm. Question to follow up is should we split the ABI to allow separate control.
>Ideally we probably should but technically it'd be an unnecessary userspace
>interface change.  The question is whether we'd actually break anything?
>
>I'd certainly prefer to see the two separate attributes to control the different
>low pass filters.
>
>Jonathan
>
Hello,

here is a reworked patch with a switch/case below.

There is no attribute currently to set lpf setting. It is automatically set with the
correct value when setting the sampling rate (to avoid any aliases issues).
Since there is only 1 frequency for the chip (meaning a unique frequency for
both accel and gyro), there is no real need to set apart the accelerometer lpf value.
Better have the same behavior for both gyro and accel lpf settings, setting them
both when changing the sampling rate.

That would be a good thing to send this into stable release.
If I need to do something, please tell me how to do it.

Thanks.
Jean-Baptiste


>From 8db0b6b626c7e640590df8f271de7e8e2b409f15 Mon Sep 17 00:00:00 2001
From: Jean-Baptiste Maneyrol <jmaneyrol@xxxxxxxxxxxxxx>
Date: Mon, 22 May 2017 14:45:21 +0200
Subject: [PATCH] iio: imu: inv_mpu6050: add accel lpf setting for chip >=
 MPU6500

Starting from MPU6500, accelerometer dlpf is set in a separate register
named ACCEL_CONFIG_2.
Add this new register in the map and set it for the corresponding chips.

Signed-off-by: Jean-Baptiste Maneyrol <jmaneyrol@xxxxxxxxxxxxxx>
---
 drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 37 +++++++++++++++++++++++++++---
 drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h  |  3 +++
 2 files changed, 37 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
index 96dabbd..299755a 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
@@ -41,6 +41,7 @@
 static const struct inv_mpu6050_reg_map reg_set_6500 = {
 	.sample_rate_div	= INV_MPU6050_REG_SAMPLE_RATE_DIV,
 	.lpf                    = INV_MPU6050_REG_CONFIG,
+	.accel_lpf              = INV_MPU6500_REG_ACCEL_CONFIG_2,
 	.user_ctrl              = INV_MPU6050_REG_USER_CTRL,
 	.fifo_en                = INV_MPU6050_REG_FIFO_EN,
 	.gyro_config            = INV_MPU6050_REG_GYRO_CONFIG,
@@ -211,6 +212,37 @@ int inv_mpu6050_set_power_itg(struct inv_mpu6050_state *st, bool power_on)
 EXPORT_SYMBOL_GPL(inv_mpu6050_set_power_itg);
 
 /**
+ *  inv_mpu6050_set_lpf_regs() - set low pass filter registers, chip dependent
+ *
+ *  MPU60xx/MPU9150 use only 1 register for accelerometer + gyroscope
+ *  MPU6500 and above have a dedicated register for accelerometer
+ */
+static int inv_mpu6050_set_lpf_regs(struct inv_mpu6050_state *st,
+				    enum inv_mpu6050_filter_e val)
+{
+	int result;
+
+	result = regmap_write(st->map, st->reg->lpf, val);
+	if (result)
+		return result;
+
+	switch (st->chip_type) {
+	case INV_MPU6050:
+	case INV_MPU6000:
+	case INV_MPU9150:
+		/* old chips, nothing to do */
+		result = 0;
+		break;
+	default:
+		/* set accel lpf */
+		result = regmap_write(st->map, st->reg->accel_lpf, val);
+		break;
+	}
+
+	return result;
+}
+
+/**
  *  inv_mpu6050_init_config() - Initialize hardware, disable FIFO.
  *
  *  Initial configuration:
@@ -233,8 +265,7 @@ static int inv_mpu6050_init_config(struct iio_dev *indio_dev)
 	if (result)
 		return result;
 
-	d = INV_MPU6050_FILTER_20HZ;
-	result = regmap_write(st->map, st->reg->lpf, d);
+	result = inv_mpu6050_set_lpf_regs(st, INV_MPU6050_FILTER_20HZ);
 	if (result)
 		return result;
 
@@ -552,7 +583,7 @@ static int inv_mpu6050_set_lpf(struct inv_mpu6050_state *st, int rate)
 	while ((h < hz[i]) && (i < ARRAY_SIZE(d) - 1))
 		i++;
 	data = d[i];
-	result = regmap_write(st->map, st->reg->lpf, data);
+	result = inv_mpu6050_set_lpf_regs(st, data);
 	if (result)
 		return result;
 	st->chip_config.lpf = data;
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
index ef13de7..953a0c0 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
@@ -28,6 +28,7 @@
  *  struct inv_mpu6050_reg_map - Notable registers.
  *  @sample_rate_div:	Divider applied to gyro output rate.
  *  @lpf:		Configures internal low pass filter.
+ *  @accel_lpf:		Configures accelerometer low pass filter.
  *  @user_ctrl:		Enables/resets the FIFO.
  *  @fifo_en:		Determines which data will appear in FIFO.
  *  @gyro_config:	gyro config register.
@@ -47,6 +48,7 @@
 struct inv_mpu6050_reg_map {
 	u8 sample_rate_div;
 	u8 lpf;
+	u8 accel_lpf;
 	u8 user_ctrl;
 	u8 fifo_en;
 	u8 gyro_config;
@@ -188,6 +190,7 @@ struct inv_mpu6050_state {
 #define INV_MPU6050_FIFO_THRESHOLD           500
 
 /* mpu6500 registers */
+#define INV_MPU6500_REG_ACCEL_CONFIG_2      0x1D
 #define INV_MPU6500_REG_ACCEL_OFFSET        0x77
 
 /* delay time in milliseconds */
-- 
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Input]     [Linux Kernel]     [Linux SCSI]     [X.org]

  Powered by Linux