On Mon, 22 May 2017 12:49:50 +0000
Jean-Baptiste Maneyrol <JManeyrol@xxxxxxxxxxxxxx> wrote:
> >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.
Fair enough. Thanks for the explanation. Please add a comment in the code
to this effect.
>
> 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.
Please post new versions as a separate email titled
[PATCH v2] iio:...
It makes it easy for people to see there is a new version without searching
in the thread and also makes it trivial to apply without hand editing.
I'll sort out flagging it for stable. If you can figure out a suitable
fixes tag that would be good, though here I guess there may be
not clear answer to that question. I can't remember what devices were
supported when!
Jonathan
>
> 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 */
--
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
