On Sun, 25 Feb 2024 16:00:24 +0000
inv.git-commit@xxxxxxx wrote:
> From: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@xxxxxxx>
>
> WoM is a threshold test on accel value comparing actual sample
> with previous one. It maps best to magnitude adaptive rising
> event.
> Add support of a new WOM sensor and functions for handling the
> corresponding mag_adaptive_rising event. The event value is in
> SI units. Ensure WOM is stopped and restarted at suspend-resume
> and handle usage with buffer data ready interrupt.
>
> Signed-off-by: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@xxxxxxx>
A few questions inline. Things don't seem to align perfectly with the
few datasheets I opened.
Jonathan
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> index 5950e2419ebb..519c1eee96ad 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> @@ -88,11 +88,12 @@ enum inv_devices {
> INV_NUM_PARTS
> };
>
> -/* chip sensors mask: accelerometer, gyroscope, temperature, magnetometer */
> +/* chip sensors mask: accelerometer, gyroscope, temperature, magnetometer, WoM */
> #define INV_MPU6050_SENSOR_ACCL BIT(0)
> #define INV_MPU6050_SENSOR_GYRO BIT(1)
> #define INV_MPU6050_SENSOR_TEMP BIT(2)
> #define INV_MPU6050_SENSOR_MAGN BIT(3)
> +#define INV_MPU6050_SENSOR_WOM BIT(4)
>
> /**
> * struct inv_mpu6050_chip_config - Cached chip configuration data.
> @@ -104,6 +105,7 @@ enum inv_devices {
> * @gyro_en: gyro engine enabled
> * @temp_en: temperature sensor enabled
> * @magn_en: magn engine (i2c master) enabled
> + * @wom_en: Wake-on-Motion enabled
> * @accl_fifo_enable: enable accel data output
> * @gyro_fifo_enable: enable gyro data output
> * @temp_fifo_enable: enable temp data output
> @@ -119,12 +121,14 @@ struct inv_mpu6050_chip_config {
> unsigned int gyro_en:1;
> unsigned int temp_en:1;
> unsigned int magn_en:1;
> + unsigned int wom_en:1;
> unsigned int accl_fifo_enable:1;
> unsigned int gyro_fifo_enable:1;
> unsigned int temp_fifo_enable:1;
> unsigned int magn_fifo_enable:1;
> u8 divider;
> u8 user_ctrl;
> + u8 wom_threshold;
> };
>
> /*
> @@ -256,12 +260,14 @@ struct inv_mpu6050_state {
> #define INV_MPU6050_REG_INT_ENABLE 0x38
> #define INV_MPU6050_BIT_DATA_RDY_EN 0x01
> #define INV_MPU6050_BIT_DMP_INT_EN 0x02
> +#define INV_MPU6500_BIT_WOM_INT_EN (BIT(7) | BIT(6) | BIT(5))
GENMASK? or build it up as I'm guessing those are the 3 axis?
However I opened the datasheet from the tdk website and only bit(6) is mentioned.
>
> #define INV_MPU6050_REG_RAW_ACCEL 0x3B
> #define INV_MPU6050_REG_TEMPERATURE 0x41
> #define INV_MPU6050_REG_RAW_GYRO 0x43
>
> #define INV_MPU6050_REG_INT_STATUS 0x3A
> +#define INV_MPU6500_BIT_WOM_INT (BIT(7) | BIT(6) | BIT(5))
Likewise on this, the mpu6500 datasheet only mentions bit(6)
> #define INV_MPU6050_BIT_FIFO_OVERFLOW_INT 0x10
> #define INV_MPU6050_BIT_RAW_DATA_RDY_INT 0x01
>
> @@ -301,6 +307,11 @@ struct inv_mpu6050_state {
> #define INV_MPU6050_BIT_PWR_ACCL_STBY 0x38
> #define INV_MPU6050_BIT_PWR_GYRO_STBY 0x07
>
> +/* ICM20609 registers */
> +#define INV_ICM20609_REG_ACCEL_WOM_X_THR 0x20
> +#define INV_ICM20609_REG_ACCEL_WOM_Y_THR 0x21
> +#define INV_ICM20609_REG_ACCEL_WOM_Z_THR 0x22
This one does mention all 3 bits for the enable and status registers.
Perhaps there is more inter chip variation than currently covered?
I don't like writing reserved bits unless we have a clear statement
(and a comment here) that it is correct thing to do.
> +
> /* ICM20602 register */
> #define INV_ICM20602_REG_I2C_IF 0x70
> #define INV_ICM20602_BIT_I2C_IF_DIS 0x40
> @@ -320,6 +331,10 @@ struct inv_mpu6050_state {
> /* mpu6500 registers */
> #define INV_MPU6500_REG_ACCEL_CONFIG_2 0x1D
> #define INV_ICM20689_BITS_FIFO_SIZE_MAX 0xC0
> +#define INV_MPU6500_REG_WOM_THRESHOLD 0x1F
> +#define INV_MPU6500_REG_ACCEL_INTEL_CTRL 0x69
> +#define INV_MPU6500_BIT_ACCEL_INTEL_EN BIT(7)
> +#define INV_MPU6500_BIT_ACCEL_INTEL_MODE BIT(6)
> #define INV_MPU6500_REG_ACCEL_OFFSET 0x77
>
> /* delay time in milliseconds */
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
> index 66d4ba088e70..13da6f523ca2 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
> @@ -33,10 +33,8 @@ static int inv_reset_fifo(struct iio_dev *indio_dev)
>
> reset_fifo_fail:
> dev_err(regmap_get_device(st->map), "reset fifo failed %d\n", result);
> - result = regmap_write(st->map, st->reg->int_enable,
> - INV_MPU6050_BIT_DATA_RDY_EN);
> -
> - return result;
> + return regmap_update_bits(st->map, st->reg->int_enable,
> + INV_MPU6050_BIT_DATA_RDY_EN, INV_MPU6050_BIT_DATA_RDY_EN);
> }
>
> /*
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
> index 676704f9151f..ec2398a87f45 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
> @@ -134,11 +134,13 @@ int inv_mpu6050_prepare_fifo(struct inv_mpu6050_state *st, bool enable)
> ret = regmap_write(st->map, st->reg->user_ctrl, d);
> if (ret)
> return ret;
> - /* enable interrupt */
> - ret = regmap_write(st->map, st->reg->int_enable,
> - INV_MPU6050_BIT_DATA_RDY_EN);
> + /* enable data interrupt */
> + ret = regmap_update_bits(st->map, st->reg->int_enable,
> + INV_MPU6050_BIT_DATA_RDY_EN, INV_MPU6050_BIT_DATA_RDY_EN);
> } else {
> - ret = regmap_write(st->map, st->reg->int_enable, 0);
> + /* disable data interrupt */
> + ret = regmap_update_bits(st->map, st->reg->int_enable,
> + INV_MPU6050_BIT_DATA_RDY_EN, 0);
> if (ret)
> return ret;
> ret = regmap_write(st->map, st->reg->fifo_en, 0);
> @@ -171,9 +173,9 @@ static int inv_mpu6050_set_enable(struct iio_dev *indio_dev, bool enable)
> return result;
> /*
> * In case autosuspend didn't trigger, turn off first not
> - * required sensors.
> + * required sensors excepted WoM
> */
> - result = inv_mpu6050_switch_engine(st, false, ~scan);
> + result = inv_mpu6050_switch_engine(st, false, ~scan & ~INV_MPU6050_SENSOR_WOM);
> if (result)
> goto error_power_off;
> result = inv_mpu6050_switch_engine(st, true, scan);
