Linux IIO development
 help / color / mirror / Atom feed
* [PATCH 0/4] Add WoM feature as an IIO event
@ 2024-02-25 16:00 inv.git-commit
  2024-02-25 16:00 ` [PATCH 1/4] iio: imu: inv_mpu6050: add WoM (Wake-on-Motion) sensor inv.git-commit
                   ` (4 more replies)
  0 siblings, 5 replies; 13+ messages in thread
From: inv.git-commit @ 2024-02-25 16:00 UTC (permalink / raw)
  To: jic23; +Cc: lars, linux-iio, Jean-Baptiste Maneyrol

From: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>

Add WoM (Wake-on-Motion) feature for all chips supporting it
(all except MPU-6000/6050/9150). WoM compares the magnitude of
the current accel sample with the previous one against a threshold
and returns an interrupt event if the value is higher.

Report WoM as an accel mag_adaptive_rising IIO event, add system
wakeup functionality if WoM is on and put the chip in low-power
mode when the system is suspended. WoM threshold value is in SI
units since the chip is using an absolute value in mg.


Jean-Baptiste Maneyrol (4):
  iio: imu: inv_mpu6050: add WoM (Wake-on-Motion) sensor
  iio: imu: inv_mpu6050: add WoM event inside accel channels
  iio: imu: inv_mpu6050: add new interrupt handler for WoM events
  iio: imu: inv_mpu6050: add WoM suspend wakeup with low-power mode

 drivers/iio/imu/inv_mpu6050/inv_mpu_core.c    | 523 +++++++++++++++---
 drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h     |  33 +-
 drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c    |  17 +-
 drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c |  70 ++-
 4 files changed, 541 insertions(+), 102 deletions(-)

-- 
2.34.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 1/4] iio: imu: inv_mpu6050: add WoM (Wake-on-Motion) sensor
  2024-02-25 16:00 [PATCH 0/4] Add WoM feature as an IIO event inv.git-commit
@ 2024-02-25 16:00 ` inv.git-commit
  2024-03-03 16:56   ` Jonathan Cameron
  2024-02-25 16:00 ` [PATCH 2/4] iio: imu: inv_mpu6050: add WoM event inside accel channels inv.git-commit
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 13+ messages in thread
From: inv.git-commit @ 2024-02-25 16:00 UTC (permalink / raw)
  To: jic23; +Cc: lars, linux-iio, Jean-Baptiste Maneyrol

From: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>

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@tdk.com>
---
 drivers/iio/imu/inv_mpu6050/inv_mpu_core.c    | 237 +++++++++++++++++-
 drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h     |  17 +-
 drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c    |   6 +-
 drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c |  14 +-
 4 files changed, 261 insertions(+), 13 deletions(-)

diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
index 0e94e5335e93..fca7fc1ba4e2 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
@@ -332,7 +332,7 @@ static int inv_mpu6050_clock_switch(struct inv_mpu6050_state *st,
 int inv_mpu6050_switch_engine(struct inv_mpu6050_state *st, bool en,
 			      unsigned int mask)
 {
-	unsigned int sleep;
+	unsigned int sleep, val;
 	u8 pwr_mgmt2, user_ctrl;
 	int ret;
 
@@ -345,6 +345,14 @@ int inv_mpu6050_switch_engine(struct inv_mpu6050_state *st, bool en,
 		mask &= ~INV_MPU6050_SENSOR_TEMP;
 	if (mask & INV_MPU6050_SENSOR_MAGN && en == st->chip_config.magn_en)
 		mask &= ~INV_MPU6050_SENSOR_MAGN;
+	if (mask & INV_MPU6050_SENSOR_WOM && en == st->chip_config.wom_en)
+		mask &= ~INV_MPU6050_SENSOR_WOM;
+
+	/* force accel on if WoM is on and not going off */
+	if (!en && (mask & INV_MPU6050_SENSOR_ACCL) && st->chip_config.wom_en &&
+			!(mask & INV_MPU6050_SENSOR_WOM))
+		mask &= ~INV_MPU6050_SENSOR_ACCL;
+
 	if (mask == 0)
 		return 0;
 
@@ -439,6 +447,16 @@ int inv_mpu6050_switch_engine(struct inv_mpu6050_state *st, bool en,
 		}
 	}
 
+	/* enable/disable accel intelligence control */
+	if (mask & INV_MPU6050_SENSOR_WOM) {
+		val = en ? INV_MPU6500_BIT_ACCEL_INTEL_EN |
+			   INV_MPU6500_BIT_ACCEL_INTEL_MODE : 0;
+		ret = regmap_write(st->map, INV_MPU6500_REG_ACCEL_INTEL_CTRL, val);
+		if (ret)
+			return ret;
+		st->chip_config.wom_en = en;
+	}
+
 	return 0;
 }
 
@@ -893,6 +911,202 @@ static int inv_mpu6050_write_raw(struct iio_dev *indio_dev,
 	return result;
 }
 
+static int inv_mpu6050_set_wom_int(struct inv_mpu6050_state *st, bool on)
+{
+	unsigned int val;
+
+	val = on ? INV_MPU6500_BIT_WOM_INT_EN : 0;
+
+	return regmap_update_bits(st->map, st->reg->int_enable,
+				  INV_MPU6500_BIT_WOM_INT_EN, val);
+}
+
+static int inv_mpu6050_set_wom_threshold(struct inv_mpu6050_state *st, unsigned int value)
+{
+	struct device *pdev = regmap_get_device(st->map);
+	int result;
+
+	mutex_lock(&st->lock);
+
+	result = pm_runtime_resume_and_get(pdev);
+	if (result)
+		goto exit_unlock;
+
+	switch (st->chip_type) {
+	case INV_ICM20609:
+	case INV_ICM20689:
+	case INV_ICM20600:
+	case INV_ICM20602:
+	case INV_ICM20690:
+		st->data[0] = value;
+		st->data[1] = value;
+		st->data[2] = value;
+		result = regmap_bulk_write(st->map, INV_ICM20609_REG_ACCEL_WOM_X_THR,
+					   st->data, 3);
+		break;
+	default:
+		result = regmap_write(st->map, INV_MPU6500_REG_WOM_THRESHOLD, value);
+		break;
+	}
+	if (result)
+		goto exit_suspend;
+
+	st->chip_config.wom_threshold = value;
+
+exit_suspend:
+	pm_runtime_mark_last_busy(pdev);
+	pm_runtime_put_autosuspend(pdev);
+exit_unlock:
+	mutex_unlock(&st->lock);
+	return result;
+}
+
+static int inv_mpu6050_enable_wom(struct inv_mpu6050_state *st, bool en)
+{
+	struct device *pdev = regmap_get_device(st->map);
+	unsigned int mask;
+	int result;
+
+	if (en) {
+		result = pm_runtime_resume_and_get(pdev);
+		if (result)
+			return result;
+
+		mask = INV_MPU6050_SENSOR_ACCL | INV_MPU6050_SENSOR_WOM;
+		result = inv_mpu6050_switch_engine(st, true, mask);
+		if (result)
+			goto error_suspend;
+
+		result = inv_mpu6050_set_wom_int(st, true);
+		if (result)
+			goto error_suspend;
+	} else {
+		result = inv_mpu6050_set_wom_int(st, false);
+		if (result)
+			dev_err(pdev, "error %d disabling WoM interrupt bit", result);
+
+		/* disable only WoM and let accel be disabled by autosuspend */
+		result = inv_mpu6050_switch_engine(st, false, INV_MPU6050_SENSOR_WOM);
+		if (result) {
+			dev_err(pdev, "error %d disabling WoM force off", result);
+			/* force WoM off */
+			st->chip_config.wom_en = false;
+		}
+
+		pm_runtime_mark_last_busy(pdev);
+		pm_runtime_put_autosuspend(pdev);
+	}
+
+	return result;
+
+error_suspend:
+	pm_runtime_mark_last_busy(pdev);
+	pm_runtime_put_autosuspend(pdev);
+	return result;
+}
+
+static int inv_mpu6050_read_event_config(struct iio_dev *indio_dev,
+					 const struct iio_chan_spec *chan,
+					 enum iio_event_type type,
+					 enum iio_event_direction dir)
+{
+	struct inv_mpu6050_state *st = iio_priv(indio_dev);
+	int result;
+
+	/* support only WoM (accel mag_adaptive rising) event */
+	if (chan->type != IIO_ACCEL || type != IIO_EV_TYPE_MAG_ADAPTIVE ||
+			dir != IIO_EV_DIR_RISING)
+		return -EINVAL;
+
+	mutex_lock(&st->lock);
+	result = st->chip_config.wom_en ? 1 : 0;
+	mutex_unlock(&st->lock);
+
+	return result;
+}
+
+static int inv_mpu6050_write_event_config(struct iio_dev *indio_dev,
+					  const struct iio_chan_spec *chan,
+					  enum iio_event_type type,
+					  enum iio_event_direction dir,
+					  int state)
+{
+	struct inv_mpu6050_state *st = iio_priv(indio_dev);
+	int enable;
+	int result;
+
+	/* support only WoM (accel mag_adaptive rising) event */
+	if (chan->type != IIO_ACCEL || type != IIO_EV_TYPE_MAG_ADAPTIVE ||
+			dir != IIO_EV_DIR_RISING)
+		return -EINVAL;
+
+	enable = !!state;
+
+	mutex_lock(&st->lock);
+
+	if (st->chip_config.wom_en == enable) {
+		result = 0;
+		goto exit_unlock;
+	}
+
+	result = inv_mpu6050_enable_wom(st, enable);
+
+exit_unlock:
+	mutex_unlock(&st->lock);
+	return result;
+}
+
+static int inv_mpu6050_read_event_value(struct iio_dev *indio_dev,
+					const struct iio_chan_spec *chan,
+					enum iio_event_type type,
+					enum iio_event_direction dir,
+					enum iio_event_info info, int *val, int *val2)
+{
+	struct inv_mpu6050_state *st = iio_priv(indio_dev);
+	unsigned int value;
+
+	/* support only WoM (accel mag_adaptive rising) event threshold value */
+	if (chan->type != IIO_ACCEL || type != IIO_EV_TYPE_MAG_ADAPTIVE ||
+			dir != IIO_EV_DIR_RISING || info != IIO_EV_INFO_VALUE)
+		return -EINVAL;
+
+	/* 4mg per LSB converted in m/s² in micro (1000000) */
+	value = (unsigned int)st->chip_config.wom_threshold * 4U * 9807U;
+	*val = value / 1000000;
+	*val2 = value % 1000000;
+
+	return IIO_VAL_INT_PLUS_MICRO;
+}
+
+static int inv_mpu6050_write_event_value(struct iio_dev *indio_dev,
+					 const struct iio_chan_spec *chan,
+					 enum iio_event_type type,
+					 enum iio_event_direction dir,
+					 enum iio_event_info info, int val, int val2)
+{
+	struct inv_mpu6050_state *st = iio_priv(indio_dev);
+	const int max = 4 * 255 * 9807;
+	const int max_val = max / 1000000;
+	const int max_val2 = max % 1000000;
+	unsigned int value;
+
+	/* support only WoM (accel mag_adaptive rising) event threshold value */
+	if (chan->type != IIO_ACCEL || type != IIO_EV_TYPE_MAG_ADAPTIVE ||
+			dir != IIO_EV_DIR_RISING || info != IIO_EV_INFO_VALUE)
+		return -EINVAL;
+
+	if (val < 0 || val2 < 0)
+		return -EINVAL;
+	if (val > max_val || (val == max_val && val2 > max_val2)) {
+		val = max_val;
+		val2 = max_val2;
+	}
+	value = val * 1000000U + val2;
+	value /= (4U * 9807U);
+
+	return inv_mpu6050_set_wom_threshold(st, value);
+}
+
 /*
  *  inv_mpu6050_set_lpf() - set low pass filer based on fifo rate.
  *
@@ -1326,6 +1540,10 @@ static const struct iio_info mpu_info = {
 	.write_raw = &inv_mpu6050_write_raw,
 	.write_raw_get_fmt = &inv_write_raw_get_fmt,
 	.attrs = &inv_attribute_group,
+	.read_event_config = inv_mpu6050_read_event_config,
+	.write_event_config = inv_mpu6050_write_event_config,
+	.read_event_value = inv_mpu6050_read_event_value,
+	.write_event_value = inv_mpu6050_write_event_value,
 	.validate_trigger = inv_mpu6050_validate_trigger,
 	.debugfs_reg_access = &inv_mpu6050_reg_access,
 };
@@ -1706,6 +1924,12 @@ static int inv_mpu_resume(struct device *dev)
 	if (result)
 		goto out_unlock;
 
+	if (st->chip_config.wom_en) {
+		result = inv_mpu6050_set_wom_int(st, true);
+		if (result)
+			goto out_unlock;
+	}
+
 	if (iio_buffer_enabled(indio_dev))
 		result = inv_mpu6050_prepare_fifo(st, true);
 
@@ -1735,6 +1959,12 @@ static int inv_mpu_suspend(struct device *dev)
 			goto out_unlock;
 	}
 
+	if (st->chip_config.wom_en) {
+		result = inv_mpu6050_set_wom_int(st, false);
+		if (result)
+			goto out_unlock;
+	}
+
 	if (st->chip_config.accl_en)
 		st->suspended_sensors |= INV_MPU6050_SENSOR_ACCL;
 	if (st->chip_config.gyro_en)
@@ -1743,6 +1973,8 @@ static int inv_mpu_suspend(struct device *dev)
 		st->suspended_sensors |= INV_MPU6050_SENSOR_TEMP;
 	if (st->chip_config.magn_en)
 		st->suspended_sensors |= INV_MPU6050_SENSOR_MAGN;
+	if (st->chip_config.wom_en)
+		st->suspended_sensors |= INV_MPU6050_SENSOR_WOM;
 	result = inv_mpu6050_switch_engine(st, false, st->suspended_sensors);
 	if (result)
 		goto out_unlock;
@@ -1767,7 +1999,8 @@ static int inv_mpu_runtime_suspend(struct device *dev)
 	mutex_lock(&st->lock);
 
 	sensors = INV_MPU6050_SENSOR_ACCL | INV_MPU6050_SENSOR_GYRO |
-			INV_MPU6050_SENSOR_TEMP | INV_MPU6050_SENSOR_MAGN;
+			INV_MPU6050_SENSOR_TEMP | INV_MPU6050_SENSOR_MAGN |
+			INV_MPU6050_SENSOR_WOM;
 	ret = inv_mpu6050_switch_engine(st, false, sensors);
 	if (ret)
 		goto out_unlock;
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))
 
 #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))
 #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
+
 /* 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);
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 2/4] iio: imu: inv_mpu6050: add WoM event inside accel channels
  2024-02-25 16:00 [PATCH 0/4] Add WoM feature as an IIO event inv.git-commit
  2024-02-25 16:00 ` [PATCH 1/4] iio: imu: inv_mpu6050: add WoM (Wake-on-Motion) sensor inv.git-commit
@ 2024-02-25 16:00 ` inv.git-commit
  2024-03-03 16:58   ` Jonathan Cameron
  2024-02-25 16:00 ` [PATCH 3/4] iio: imu: inv_mpu6050: add new interrupt handler for WoM events inv.git-commit
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 13+ messages in thread
From: inv.git-commit @ 2024-02-25 16:00 UTC (permalink / raw)
  To: jic23; +Cc: lars, linux-iio, Jean-Baptiste Maneyrol

From: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>

Add WoM (mag_adaptive rising) event in accel channels for all
chips >= MPU-6500. This requires to create new MPU-6500 channels
as default and MPU-6050 channels for older chips.

Signed-off-by: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>
---
 drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 132 ++++++++++++++-------
 1 file changed, 89 insertions(+), 43 deletions(-)

diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
index fca7fc1ba4e2..d2544c758815 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
@@ -1303,23 +1303,34 @@ static const struct iio_chan_spec_ext_info inv_ext_info[] = {
 	{ }
 };
 
-#define INV_MPU6050_CHAN(_type, _channel2, _index)                    \
-	{                                                             \
-		.type = _type,                                        \
-		.modified = 1,                                        \
-		.channel2 = _channel2,                                \
-		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
-		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |	      \
-				      BIT(IIO_CHAN_INFO_CALIBBIAS),   \
-		.scan_index = _index,                                 \
-		.scan_type = {                                        \
-				.sign = 's',                          \
-				.realbits = 16,                       \
-				.storagebits = 16,                    \
-				.shift = 0,                           \
-				.endianness = IIO_BE,                 \
-			     },                                       \
-		.ext_info = inv_ext_info,                             \
+static const struct iio_event_spec inv_accel_events[] = {
+	{
+		.type = IIO_EV_TYPE_MAG_ADAPTIVE,
+		.dir = IIO_EV_DIR_RISING,
+		.mask_shared_by_type = BIT(IIO_EV_INFO_ENABLE) |
+				       BIT(IIO_EV_INFO_VALUE),
+	},
+};
+
+#define INV_MPU6050_CHAN(_type, _channel2, _index, _events, _events_nb) \
+	{                                                               \
+		.type = _type,                                          \
+		.modified = 1,                                          \
+		.channel2 = _channel2,                                  \
+		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),   \
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |	        \
+				      BIT(IIO_CHAN_INFO_CALIBBIAS),     \
+		.event_spec = _events,                                  \
+		.num_event_specs = _events_nb,                          \
+		.scan_index = _index,                                   \
+		.scan_type = {                                          \
+				.sign = 's',                            \
+				.realbits = 16,                         \
+				.storagebits = 16,                      \
+				.shift = 0,                             \
+				.endianness = IIO_BE,                   \
+			     },                                         \
+		.ext_info = inv_ext_info,                               \
 	}
 
 #define INV_MPU6050_TEMP_CHAN(_index)				\
@@ -1338,18 +1349,35 @@ static const struct iio_chan_spec_ext_info inv_ext_info[] = {
 		},						\
 	}
 
-static const struct iio_chan_spec inv_mpu_channels[] = {
+static const struct iio_chan_spec inv_mpu6050_channels[] = {
+	IIO_CHAN_SOFT_TIMESTAMP(INV_MPU6050_SCAN_TIMESTAMP),
+
+	INV_MPU6050_TEMP_CHAN(INV_MPU6050_SCAN_TEMP),
+
+	INV_MPU6050_CHAN(IIO_ANGL_VEL, IIO_MOD_X, INV_MPU6050_SCAN_GYRO_X, NULL, 0),
+	INV_MPU6050_CHAN(IIO_ANGL_VEL, IIO_MOD_Y, INV_MPU6050_SCAN_GYRO_Y, NULL, 0),
+	INV_MPU6050_CHAN(IIO_ANGL_VEL, IIO_MOD_Z, INV_MPU6050_SCAN_GYRO_Z, NULL, 0),
+
+	INV_MPU6050_CHAN(IIO_ACCEL, IIO_MOD_X, INV_MPU6050_SCAN_ACCL_X, NULL, 0),
+	INV_MPU6050_CHAN(IIO_ACCEL, IIO_MOD_Y, INV_MPU6050_SCAN_ACCL_Y, NULL, 0),
+	INV_MPU6050_CHAN(IIO_ACCEL, IIO_MOD_Z, INV_MPU6050_SCAN_ACCL_Z, NULL, 0),
+};
+
+static const struct iio_chan_spec inv_mpu6500_channels[] = {
 	IIO_CHAN_SOFT_TIMESTAMP(INV_MPU6050_SCAN_TIMESTAMP),
 
 	INV_MPU6050_TEMP_CHAN(INV_MPU6050_SCAN_TEMP),
 
-	INV_MPU6050_CHAN(IIO_ANGL_VEL, IIO_MOD_X, INV_MPU6050_SCAN_GYRO_X),
-	INV_MPU6050_CHAN(IIO_ANGL_VEL, IIO_MOD_Y, INV_MPU6050_SCAN_GYRO_Y),
-	INV_MPU6050_CHAN(IIO_ANGL_VEL, IIO_MOD_Z, INV_MPU6050_SCAN_GYRO_Z),
+	INV_MPU6050_CHAN(IIO_ANGL_VEL, IIO_MOD_X, INV_MPU6050_SCAN_GYRO_X, NULL, 0),
+	INV_MPU6050_CHAN(IIO_ANGL_VEL, IIO_MOD_Y, INV_MPU6050_SCAN_GYRO_Y, NULL, 0),
+	INV_MPU6050_CHAN(IIO_ANGL_VEL, IIO_MOD_Z, INV_MPU6050_SCAN_GYRO_Z, NULL, 0),
 
-	INV_MPU6050_CHAN(IIO_ACCEL, IIO_MOD_X, INV_MPU6050_SCAN_ACCL_X),
-	INV_MPU6050_CHAN(IIO_ACCEL, IIO_MOD_Y, INV_MPU6050_SCAN_ACCL_Y),
-	INV_MPU6050_CHAN(IIO_ACCEL, IIO_MOD_Z, INV_MPU6050_SCAN_ACCL_Z),
+	INV_MPU6050_CHAN(IIO_ACCEL, IIO_MOD_X, INV_MPU6050_SCAN_ACCL_X,
+			 inv_accel_events, ARRAY_SIZE(inv_accel_events)),
+	INV_MPU6050_CHAN(IIO_ACCEL, IIO_MOD_Y, INV_MPU6050_SCAN_ACCL_Y,
+			 inv_accel_events, ARRAY_SIZE(inv_accel_events)),
+	INV_MPU6050_CHAN(IIO_ACCEL, IIO_MOD_Z, INV_MPU6050_SCAN_ACCL_Z,
+			 inv_accel_events, ARRAY_SIZE(inv_accel_events)),
 };
 
 #define INV_MPU6050_SCAN_MASK_3AXIS_ACCEL	\
@@ -1401,13 +1429,13 @@ static const struct iio_chan_spec inv_mpu9150_channels[] = {
 
 	INV_MPU6050_TEMP_CHAN(INV_MPU6050_SCAN_TEMP),
 
-	INV_MPU6050_CHAN(IIO_ANGL_VEL, IIO_MOD_X, INV_MPU6050_SCAN_GYRO_X),
-	INV_MPU6050_CHAN(IIO_ANGL_VEL, IIO_MOD_Y, INV_MPU6050_SCAN_GYRO_Y),
-	INV_MPU6050_CHAN(IIO_ANGL_VEL, IIO_MOD_Z, INV_MPU6050_SCAN_GYRO_Z),
+	INV_MPU6050_CHAN(IIO_ANGL_VEL, IIO_MOD_X, INV_MPU6050_SCAN_GYRO_X, NULL, 0),
+	INV_MPU6050_CHAN(IIO_ANGL_VEL, IIO_MOD_Y, INV_MPU6050_SCAN_GYRO_Y, NULL, 0),
+	INV_MPU6050_CHAN(IIO_ANGL_VEL, IIO_MOD_Z, INV_MPU6050_SCAN_GYRO_Z, NULL, 0),
 
-	INV_MPU6050_CHAN(IIO_ACCEL, IIO_MOD_X, INV_MPU6050_SCAN_ACCL_X),
-	INV_MPU6050_CHAN(IIO_ACCEL, IIO_MOD_Y, INV_MPU6050_SCAN_ACCL_Y),
-	INV_MPU6050_CHAN(IIO_ACCEL, IIO_MOD_Z, INV_MPU6050_SCAN_ACCL_Z),
+	INV_MPU6050_CHAN(IIO_ACCEL, IIO_MOD_X, INV_MPU6050_SCAN_ACCL_X, NULL, 0),
+	INV_MPU6050_CHAN(IIO_ACCEL, IIO_MOD_Y, INV_MPU6050_SCAN_ACCL_Y, NULL, 0),
+	INV_MPU6050_CHAN(IIO_ACCEL, IIO_MOD_Z, INV_MPU6050_SCAN_ACCL_Z, NULL, 0),
 
 	/* Magnetometer resolution is 13 bits */
 	INV_MPU9X50_MAGN_CHAN(IIO_MOD_X, 13, INV_MPU9X50_SCAN_MAGN_X),
@@ -1420,13 +1448,16 @@ static const struct iio_chan_spec inv_mpu9250_channels[] = {
 
 	INV_MPU6050_TEMP_CHAN(INV_MPU6050_SCAN_TEMP),
 
-	INV_MPU6050_CHAN(IIO_ANGL_VEL, IIO_MOD_X, INV_MPU6050_SCAN_GYRO_X),
-	INV_MPU6050_CHAN(IIO_ANGL_VEL, IIO_MOD_Y, INV_MPU6050_SCAN_GYRO_Y),
-	INV_MPU6050_CHAN(IIO_ANGL_VEL, IIO_MOD_Z, INV_MPU6050_SCAN_GYRO_Z),
+	INV_MPU6050_CHAN(IIO_ANGL_VEL, IIO_MOD_X, INV_MPU6050_SCAN_GYRO_X, NULL, 0),
+	INV_MPU6050_CHAN(IIO_ANGL_VEL, IIO_MOD_Y, INV_MPU6050_SCAN_GYRO_Y, NULL, 0),
+	INV_MPU6050_CHAN(IIO_ANGL_VEL, IIO_MOD_Z, INV_MPU6050_SCAN_GYRO_Z, NULL, 0),
 
-	INV_MPU6050_CHAN(IIO_ACCEL, IIO_MOD_X, INV_MPU6050_SCAN_ACCL_X),
-	INV_MPU6050_CHAN(IIO_ACCEL, IIO_MOD_Y, INV_MPU6050_SCAN_ACCL_Y),
-	INV_MPU6050_CHAN(IIO_ACCEL, IIO_MOD_Z, INV_MPU6050_SCAN_ACCL_Z),
+	INV_MPU6050_CHAN(IIO_ACCEL, IIO_MOD_X, INV_MPU6050_SCAN_ACCL_X,
+			 inv_accel_events, ARRAY_SIZE(inv_accel_events)),
+	INV_MPU6050_CHAN(IIO_ACCEL, IIO_MOD_Y, INV_MPU6050_SCAN_ACCL_Y,
+			 inv_accel_events, ARRAY_SIZE(inv_accel_events)),
+	INV_MPU6050_CHAN(IIO_ACCEL, IIO_MOD_Z, INV_MPU6050_SCAN_ACCL_Z,
+			 inv_accel_events, ARRAY_SIZE(inv_accel_events)),
 
 	/* Magnetometer resolution is 16 bits */
 	INV_MPU9X50_MAGN_CHAN(IIO_MOD_X, 16, INV_MPU9X50_SCAN_MAGN_X),
@@ -1831,6 +1862,12 @@ int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
 		return result;
 
 	switch (chip_type) {
+	case INV_MPU6000:
+	case INV_MPU6050:
+		indio_dev->channels = inv_mpu6050_channels;
+		indio_dev->num_channels = ARRAY_SIZE(inv_mpu6050_channels);
+		indio_dev->available_scan_masks = inv_mpu_scan_masks;
+		break;
 	case INV_MPU9150:
 		indio_dev->channels = inv_mpu9150_channels;
 		indio_dev->num_channels = ARRAY_SIZE(inv_mpu9150_channels);
@@ -1844,13 +1881,13 @@ int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
 		break;
 	case INV_ICM20600:
 	case INV_ICM20602:
-		indio_dev->channels = inv_mpu_channels;
-		indio_dev->num_channels = ARRAY_SIZE(inv_mpu_channels);
+		indio_dev->channels = inv_mpu6500_channels;
+		indio_dev->num_channels = ARRAY_SIZE(inv_mpu6500_channels);
 		indio_dev->available_scan_masks = inv_icm20602_scan_masks;
 		break;
 	default:
-		indio_dev->channels = inv_mpu_channels;
-		indio_dev->num_channels = ARRAY_SIZE(inv_mpu_channels);
+		indio_dev->channels = inv_mpu6500_channels;
+		indio_dev->num_channels = ARRAY_SIZE(inv_mpu6500_channels);
 		indio_dev->available_scan_masks = inv_mpu_scan_masks;
 		break;
 	}
@@ -1859,9 +1896,18 @@ int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
 	 * auxiliary device in use. Otherwise Going back to 6-axis only.
 	 */
 	if (st->magn_disabled) {
-		indio_dev->channels = inv_mpu_channels;
-		indio_dev->num_channels = ARRAY_SIZE(inv_mpu_channels);
-		indio_dev->available_scan_masks = inv_mpu_scan_masks;
+		switch (chip_type) {
+		case INV_MPU9150:
+			indio_dev->channels = inv_mpu6050_channels;
+			indio_dev->num_channels = ARRAY_SIZE(inv_mpu6050_channels);
+			indio_dev->available_scan_masks = inv_mpu_scan_masks;
+			break;
+		default:
+			indio_dev->channels = inv_mpu6500_channels;
+			indio_dev->num_channels = ARRAY_SIZE(inv_mpu6500_channels);
+			indio_dev->available_scan_masks = inv_mpu_scan_masks;
+			break;
+		}
 	}
 
 	indio_dev->info = &mpu_info;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 3/4] iio: imu: inv_mpu6050: add new interrupt handler for WoM events
  2024-02-25 16:00 [PATCH 0/4] Add WoM feature as an IIO event inv.git-commit
  2024-02-25 16:00 ` [PATCH 1/4] iio: imu: inv_mpu6050: add WoM (Wake-on-Motion) sensor inv.git-commit
  2024-02-25 16:00 ` [PATCH 2/4] iio: imu: inv_mpu6050: add WoM event inside accel channels inv.git-commit
@ 2024-02-25 16:00 ` inv.git-commit
  2024-03-03 17:10   ` Jonathan Cameron
  2024-02-25 16:00 ` [PATCH 4/4] iio: imu: inv_mpu6050: add WoM suspend wakeup with low-power mode inv.git-commit
  2024-03-03 16:44 ` [PATCH 0/4] Add WoM feature as an IIO event Jonathan Cameron
  4 siblings, 1 reply; 13+ messages in thread
From: inv.git-commit @ 2024-02-25 16:00 UTC (permalink / raw)
  To: jic23; +Cc: lars, linux-iio, Jean-Baptiste Maneyrol

From: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>

Add new interrupt handler for generating WoM event from int status
register bits. Launch from interrupt the trigger poll function for
data buffer.

Signed-off-by: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>
---
 drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h     |  2 +
 drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c    | 11 ----
 drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c | 56 +++++++++++++++++--
 3 files changed, 53 insertions(+), 16 deletions(-)

diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
index 519c1eee96ad..9be67cebbd49 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
@@ -184,6 +184,7 @@ struct inv_mpu6050_hw {
  *  @magn_orient:       magnetometer sensor chip orientation if available.
  *  @suspended_sensors:	sensors mask of sensors turned off for suspend
  *  @data:		read buffer used for bulk reads.
+ *  @it_timestamp:	interrupt timestamp.
  */
 struct inv_mpu6050_state {
 	struct mutex lock;
@@ -209,6 +210,7 @@ struct inv_mpu6050_state {
 	unsigned int suspended_sensors;
 	bool level_shifter;
 	u8 *data;
+	s64 it_timestamp;
 };
 
 /*register and associated bit definition*/
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
index 13da6f523ca2..e282378ee2ca 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
@@ -51,21 +51,10 @@ irqreturn_t inv_mpu6050_read_fifo(int irq, void *p)
 	u32 fifo_period;
 	s64 timestamp;
 	u8 data[INV_MPU6050_OUTPUT_DATA_SIZE];
-	int int_status;
 	size_t i, nb;
 
 	mutex_lock(&st->lock);
 
-	/* ack interrupt and check status */
-	result = regmap_read(st->map, st->reg->int_status, &int_status);
-	if (result) {
-		dev_err(regmap_get_device(st->map),
-			"failed to ack interrupt\n");
-		goto flush_fifo;
-	}
-	if (!(int_status & INV_MPU6050_BIT_RAW_DATA_RDY_INT))
-		goto end_session;
-
 	if (!(st->chip_config.accl_fifo_enable |
 		st->chip_config.gyro_fifo_enable |
 		st->chip_config.magn_fifo_enable))
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
index ec2398a87f45..7ffbb9e7c100 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
@@ -6,6 +6,7 @@
 #include <linux/pm_runtime.h>
 
 #include <linux/iio/common/inv_sensors_timestamp.h>
+#include <linux/iio/events.h>
 
 #include "inv_mpu_iio.h"
 
@@ -223,6 +224,52 @@ static const struct iio_trigger_ops inv_mpu_trigger_ops = {
 	.set_trigger_state = &inv_mpu_data_rdy_trigger_set_state,
 };
 
+static irqreturn_t inv_mpu6050_interrupt_timestamp(int irq, void *p)
+{
+	struct iio_dev *indio_dev = p;
+	struct inv_mpu6050_state *st = iio_priv(indio_dev);
+
+	st->it_timestamp = iio_get_time_ns(indio_dev);
+
+	return IRQ_WAKE_THREAD;
+}
+
+static irqreturn_t inv_mpu6050_interrupt_handle(int irq, void *p)
+{
+	struct iio_dev *indio_dev = p;
+	struct inv_mpu6050_state *st = iio_priv(indio_dev);
+	unsigned int int_status = 0;
+	int result;
+
+	mutex_lock(&st->lock);
+
+	/* ack interrupt and check status */
+	result = regmap_read(st->map, st->reg->int_status, &int_status);
+	if (result) {
+		dev_err(regmap_get_device(st->map),
+			"failed to ack interrupt\n");
+		goto exit_unlock;
+	}
+
+	/* handle WoM event */
+	if (st->chip_config.wom_en && (int_status & INV_MPU6500_BIT_WOM_INT))
+		iio_push_event(indio_dev,
+			       IIO_UNMOD_EVENT_CODE(IIO_ACCEL, 0, IIO_EV_TYPE_MAG_ADAPTIVE,
+						    IIO_EV_DIR_RISING),
+				st->it_timestamp);
+
+exit_unlock:
+	mutex_unlock(&st->lock);
+
+	/* handle raw data interrupt */
+	if (int_status & INV_MPU6050_BIT_RAW_DATA_RDY_INT) {
+		indio_dev->pollfunc->timestamp = st->it_timestamp;
+		iio_trigger_poll_nested(st->trig);
+	}
+
+	return IRQ_HANDLED;
+}
+
 int inv_mpu6050_probe_trigger(struct iio_dev *indio_dev, int irq_type)
 {
 	int ret;
@@ -235,11 +282,10 @@ int inv_mpu6050_probe_trigger(struct iio_dev *indio_dev, int irq_type)
 	if (!st->trig)
 		return -ENOMEM;
 
-	ret = devm_request_irq(&indio_dev->dev, st->irq,
-			       &iio_trigger_generic_data_rdy_poll,
-			       irq_type,
-			       "inv_mpu",
-			       st->trig);
+	ret = devm_request_threaded_irq(&indio_dev->dev, st->irq,
+					&inv_mpu6050_interrupt_timestamp,
+					&inv_mpu6050_interrupt_handle,
+					irq_type, "inv_mpu", indio_dev);
 	if (ret)
 		return ret;
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 4/4] iio: imu: inv_mpu6050: add WoM suspend wakeup with low-power mode
  2024-02-25 16:00 [PATCH 0/4] Add WoM feature as an IIO event inv.git-commit
                   ` (2 preceding siblings ...)
  2024-02-25 16:00 ` [PATCH 3/4] iio: imu: inv_mpu6050: add new interrupt handler for WoM events inv.git-commit
@ 2024-02-25 16:00 ` inv.git-commit
  2024-03-03 16:44 ` [PATCH 0/4] Add WoM feature as an IIO event Jonathan Cameron
  4 siblings, 0 replies; 13+ messages in thread
From: inv.git-commit @ 2024-02-25 16:00 UTC (permalink / raw)
  To: jic23; +Cc: lars, linux-iio, Jean-Baptiste Maneyrol

From: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>

Add wakeup from suspend for WoM when enabled and put accel in
low-power mode when suspended. Requires rewriting pwr_mgmt_1
register handling and factorize out accel LPF settings.
Use a low-power rate similar to the chip sampling rate but always
lower for a best match of the sampling rate while saving power.

Signed-off-by: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>
---
 drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 160 ++++++++++++++++-----
 drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h  |  14 ++
 2 files changed, 141 insertions(+), 33 deletions(-)

diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
index d2544c758815..db02dd50c0bc 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
@@ -287,7 +287,7 @@ static const struct inv_mpu6050_hw hw_info[] = {
 };
 
 static int inv_mpu6050_pwr_mgmt_1_write(struct inv_mpu6050_state *st, bool sleep,
-					int clock, int temp_dis)
+					bool cycle, int clock, int temp_dis)
 {
 	u8 val;
 
@@ -301,6 +301,8 @@ static int inv_mpu6050_pwr_mgmt_1_write(struct inv_mpu6050_state *st, bool sleep
 		val |= INV_MPU6050_BIT_TEMP_DIS;
 	if (sleep)
 		val |= INV_MPU6050_BIT_SLEEP;
+	if (cycle)
+		val |= INV_MPU6050_BIT_CYCLE;
 
 	dev_dbg(regmap_get_device(st->map), "pwr_mgmt_1: 0x%x\n", val);
 	return regmap_write(st->map, st->reg->pwr_mgmt_1, val);
@@ -316,7 +318,7 @@ static int inv_mpu6050_clock_switch(struct inv_mpu6050_state *st,
 	case INV_MPU6000:
 	case INV_MPU9150:
 		/* old chips: switch clock manually */
-		ret = inv_mpu6050_pwr_mgmt_1_write(st, false, clock, -1);
+		ret = inv_mpu6050_pwr_mgmt_1_write(st, false, false, clock, -1);
 		if (ret)
 			return ret;
 		st->chip_config.clk = clock;
@@ -358,7 +360,7 @@ int inv_mpu6050_switch_engine(struct inv_mpu6050_state *st, bool en,
 
 	/* turn on/off temperature sensor */
 	if (mask & INV_MPU6050_SENSOR_TEMP) {
-		ret = inv_mpu6050_pwr_mgmt_1_write(st, false, -1, !en);
+		ret = inv_mpu6050_pwr_mgmt_1_write(st, false, false, -1, !en);
 		if (ret)
 			return ret;
 		st->chip_config.temp_en = en;
@@ -465,7 +467,7 @@ static int inv_mpu6050_set_power_itg(struct inv_mpu6050_state *st,
 {
 	int result;
 
-	result = inv_mpu6050_pwr_mgmt_1_write(st, !power_on, -1, -1);
+	result = inv_mpu6050_pwr_mgmt_1_write(st, !power_on, false, -1, -1);
 	if (result)
 		return result;
 
@@ -495,22 +497,9 @@ static int inv_mpu6050_set_gyro_fsr(struct inv_mpu6050_state *st,
 	return regmap_write(st->map, st->reg->gyro_config, data);
 }
 
-/*
- *  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)
+static int inv_mpu6050_set_accel_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;
-
-	/* set accel lpf */
 	switch (st->chip_type) {
 	case INV_MPU6050:
 	case INV_MPU6000:
@@ -529,6 +518,25 @@ static int inv_mpu6050_set_lpf_regs(struct inv_mpu6050_state *st,
 	return regmap_write(st->map, st->reg->accel_lpf, val);
 }
 
+/*
+ *  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;
+
+	/* set accel lpf */
+	return inv_mpu6050_set_accel_lpf_regs(st, val);
+}
+
 /*
  *  inv_mpu6050_init_config() - Initialize hardware, disable FIFO.
  *
@@ -921,6 +929,69 @@ static int inv_mpu6050_set_wom_int(struct inv_mpu6050_state *st, bool on)
 				  INV_MPU6500_BIT_WOM_INT_EN, val);
 }
 
+static int inv_mpu6050_set_wom_odr(struct inv_mpu6050_state *st, unsigned int rate)
+{
+	static const unsigned int hz[] = {500, 250, 125, 62, 31, 16, 8, 4};
+	static const unsigned int d[] = {
+		INV_MPU6050_LPOSC_500HZ, INV_MPU6050_LPOSC_250HZ,
+		INV_MPU6050_LPOSC_125HZ, INV_MPU6050_LPOSC_62HZ,
+		INV_MPU6050_LPOSC_31HZ, INV_MPU6050_LPOSC_16HZ,
+		INV_MPU6050_LPOSC_8HZ, INV_MPU6050_LPOSC_4HZ,
+	};
+	unsigned int data, i;
+
+	switch (st->chip_type) {
+	case INV_ICM20609:
+	case INV_ICM20689:
+	case INV_ICM20600:
+	case INV_ICM20602:
+	case INV_ICM20690:
+		/* nothing to do */
+		return 0;
+	default:
+		break;
+	}
+
+	data = INV_MPU6050_LPOSC_4HZ;
+	for (i = 0; i < ARRAY_SIZE(hz); ++i) {
+		if (rate >= hz[i]) {
+			data = d[i];
+			break;
+		}
+	}
+
+	dev_dbg(regmap_get_device(st->map), "lp_odr: 0x%x\n", data);
+	return regmap_write(st->map, INV_MPU6500_REG_LP_ODR, data);
+}
+
+static int inv_mpu6050_set_wom_lp(struct inv_mpu6050_state *st, bool on)
+{
+	int result;
+
+	if (on) {
+		/* set WoM low power ODR */
+		result = inv_mpu6050_set_wom_odr(st,
+				INV_MPU6050_DIVIDER_TO_FIFO_RATE(st->chip_config.divider));
+		if (result)
+			return result;
+		/* disable accel low pass filter */
+		result = inv_mpu6050_set_accel_lpf_regs(st, INV_MPU6050_FILTER_NOLPF);
+		if (result)
+			return result;
+		/* set cycle mode */
+		result = inv_mpu6050_pwr_mgmt_1_write(st, false, true, -1, -1);
+	} else {
+		/* disable cycle mode */
+		result = inv_mpu6050_pwr_mgmt_1_write(st, false, false, -1, -1);
+		if (result)
+			return result;
+		/* restore accel low pass filter */
+		result = inv_mpu6050_set_accel_lpf_regs(st, st->chip_config.lpf);
+	}
+
+	return result;
+}
+
 static int inv_mpu6050_set_wom_threshold(struct inv_mpu6050_state *st, unsigned int value)
 {
 	struct device *pdev = regmap_get_device(st->map);
@@ -1786,6 +1857,7 @@ int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
 			irq_type);
 		return -EINVAL;
 	}
+	device_set_wakeup_capable(dev, true);
 
 	st->vdd_supply = devm_regulator_get(dev, "vdd");
 	if (IS_ERR(st->vdd_supply))
@@ -1951,16 +2023,27 @@ static int inv_mpu_resume(struct device *dev)
 {
 	struct iio_dev *indio_dev = dev_get_drvdata(dev);
 	struct inv_mpu6050_state *st = iio_priv(indio_dev);
+	bool wakeup;
 	int result;
 
 	mutex_lock(&st->lock);
-	result = inv_mpu_core_enable_regulator_vddio(st);
-	if (result)
-		goto out_unlock;
 
-	result = inv_mpu6050_set_power_itg(st, true);
-	if (result)
-		goto out_unlock;
+	wakeup = device_may_wakeup(dev) && st->chip_config.wom_en;
+
+	if (wakeup) {
+		enable_irq(st->irq);
+		disable_irq_wake(st->irq);
+		result = inv_mpu6050_set_wom_lp(st, false);
+		if (result)
+			goto out_unlock;
+	} else {
+		result = inv_mpu_core_enable_regulator_vddio(st);
+		if (result)
+			goto out_unlock;
+		result = inv_mpu6050_set_power_itg(st, true);
+		if (result)
+			goto out_unlock;
+	}
 
 	pm_runtime_disable(dev);
 	pm_runtime_set_active(dev);
@@ -1970,7 +2053,7 @@ static int inv_mpu_resume(struct device *dev)
 	if (result)
 		goto out_unlock;
 
-	if (st->chip_config.wom_en) {
+	if (st->chip_config.wom_en && !wakeup) {
 		result = inv_mpu6050_set_wom_int(st, true);
 		if (result)
 			goto out_unlock;
@@ -1989,6 +2072,7 @@ static int inv_mpu_suspend(struct device *dev)
 {
 	struct iio_dev *indio_dev = dev_get_drvdata(dev);
 	struct inv_mpu6050_state *st = iio_priv(indio_dev);
+	bool wakeup;
 	int result;
 
 	mutex_lock(&st->lock);
@@ -2005,13 +2089,15 @@ static int inv_mpu_suspend(struct device *dev)
 			goto out_unlock;
 	}
 
-	if (st->chip_config.wom_en) {
+	wakeup = device_may_wakeup(dev) && st->chip_config.wom_en;
+
+	if (st->chip_config.wom_en && !wakeup) {
 		result = inv_mpu6050_set_wom_int(st, false);
 		if (result)
 			goto out_unlock;
 	}
 
-	if (st->chip_config.accl_en)
+	if (st->chip_config.accl_en && !wakeup)
 		st->suspended_sensors |= INV_MPU6050_SENSOR_ACCL;
 	if (st->chip_config.gyro_en)
 		st->suspended_sensors |= INV_MPU6050_SENSOR_GYRO;
@@ -2019,17 +2105,25 @@ static int inv_mpu_suspend(struct device *dev)
 		st->suspended_sensors |= INV_MPU6050_SENSOR_TEMP;
 	if (st->chip_config.magn_en)
 		st->suspended_sensors |= INV_MPU6050_SENSOR_MAGN;
-	if (st->chip_config.wom_en)
+	if (st->chip_config.wom_en && !wakeup)
 		st->suspended_sensors |= INV_MPU6050_SENSOR_WOM;
 	result = inv_mpu6050_switch_engine(st, false, st->suspended_sensors);
 	if (result)
 		goto out_unlock;
 
-	result = inv_mpu6050_set_power_itg(st, false);
-	if (result)
-		goto out_unlock;
+	if (wakeup) {
+		result = inv_mpu6050_set_wom_lp(st, true);
+		if (result)
+			goto out_unlock;
+		enable_irq_wake(st->irq);
+		disable_irq(st->irq);
+	} else {
+		result = inv_mpu6050_set_power_itg(st, false);
+		if (result)
+			goto out_unlock;
+		inv_mpu_core_disable_regulator_vddio(st);
+	}
 
-	inv_mpu_core_disable_regulator_vddio(st);
 out_unlock:
 	mutex_unlock(&st->lock);
 
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
index 9be67cebbd49..751e2b72aebc 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
@@ -302,6 +302,7 @@ struct inv_mpu6050_state {
 #define INV_MPU6050_REG_PWR_MGMT_1          0x6B
 #define INV_MPU6050_BIT_H_RESET             0x80
 #define INV_MPU6050_BIT_SLEEP               0x40
+#define INV_MPU6050_BIT_CYCLE               0x20
 #define INV_MPU6050_BIT_TEMP_DIS            0x08
 #define INV_MPU6050_BIT_CLK_MASK            0x7
 
@@ -333,6 +334,7 @@ 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_LP_ODR              0x1E
 #define INV_MPU6500_REG_WOM_THRESHOLD       0x1F
 #define INV_MPU6500_REG_ACCEL_INTEL_CTRL    0x69
 #define INV_MPU6500_BIT_ACCEL_INTEL_EN      BIT(7)
@@ -449,6 +451,18 @@ enum inv_mpu6050_filter_e {
 	NUM_MPU6050_FILTER
 };
 
+enum inv_mpu6050_lposc_e {
+	INV_MPU6050_LPOSC_4HZ = 4,
+	INV_MPU6050_LPOSC_8HZ,
+	INV_MPU6050_LPOSC_16HZ,
+	INV_MPU6050_LPOSC_31HZ,
+	INV_MPU6050_LPOSC_62HZ,
+	INV_MPU6050_LPOSC_125HZ,
+	INV_MPU6050_LPOSC_250HZ,
+	INV_MPU6050_LPOSC_500HZ,
+	NUM_MPU6050_LPOSC,
+};
+
 /* IIO attribute address */
 enum INV_MPU6050_IIO_ATTR_ADDR {
 	ATTR_GYRO_MATRIX,
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/4] Add WoM feature as an IIO event
  2024-02-25 16:00 [PATCH 0/4] Add WoM feature as an IIO event inv.git-commit
                   ` (3 preceding siblings ...)
  2024-02-25 16:00 ` [PATCH 4/4] iio: imu: inv_mpu6050: add WoM suspend wakeup with low-power mode inv.git-commit
@ 2024-03-03 16:44 ` Jonathan Cameron
  2024-03-04 10:50   ` Jean-Baptiste Maneyrol
  4 siblings, 1 reply; 13+ messages in thread
From: Jonathan Cameron @ 2024-03-03 16:44 UTC (permalink / raw)
  To: inv.git-commit; +Cc: lars, linux-iio, Jean-Baptiste Maneyrol

On Sun, 25 Feb 2024 16:00:23 +0000
inv.git-commit@tdk.com wrote:

> From: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>
> 
> Add WoM (Wake-on-Motion) feature for all chips supporting it
> (all except MPU-6000/6050/9150). WoM compares the magnitude of
> the current accel sample with the previous one against a threshold
> and returns an interrupt event if the value is higher.
> 
> Report WoM as an accel mag_adaptive_rising IIO event, add system
> wakeup functionality if WoM is on and put the chip in low-power
> mode when the system is suspended. WoM threshold value is in SI
> units since the chip is using an absolute value in mg.


Adaptive thresholds are normally used when a threshold is based on
some offset form a heavily filtered version of the same channel (often
with filter resets and other nastiness)  It's an obscure hack we added
because we couldn't really get these to fit with another scheme.
This seems simpler than that from your description.

As it's just against the previous reading and I assume the chip
is in a self clocking mode during this, it might be more appropriate
to use a Rate of Change Event (ROC).  There are references to
this being a threshold on highpassed sample, so I guess there might
be some filtering to make this messier? 

The control of a roc threshold will be a little more complex as
you'll need to take into account the sampling frequency.
Advantage is you end up with something easily human understandable.

A human doesn't want to have to figure out what the right value
is for the particular frequency they are sampling at.

Jonathan

> 
> 
> Jean-Baptiste Maneyrol (4):
>   iio: imu: inv_mpu6050: add WoM (Wake-on-Motion) sensor
>   iio: imu: inv_mpu6050: add WoM event inside accel channels
>   iio: imu: inv_mpu6050: add new interrupt handler for WoM events
>   iio: imu: inv_mpu6050: add WoM suspend wakeup with low-power mode
> 
>  drivers/iio/imu/inv_mpu6050/inv_mpu_core.c    | 523 +++++++++++++++---
>  drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h     |  33 +-
>  drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c    |  17 +-
>  drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c |  70 ++-
>  4 files changed, 541 insertions(+), 102 deletions(-)
> 


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/4] iio: imu: inv_mpu6050: add WoM (Wake-on-Motion) sensor
  2024-02-25 16:00 ` [PATCH 1/4] iio: imu: inv_mpu6050: add WoM (Wake-on-Motion) sensor inv.git-commit
@ 2024-03-03 16:56   ` Jonathan Cameron
  2024-03-04 10:57     ` Jean-Baptiste Maneyrol
  0 siblings, 1 reply; 13+ messages in thread
From: Jonathan Cameron @ 2024-03-03 16:56 UTC (permalink / raw)
  To: inv.git-commit; +Cc: lars, linux-iio, Jean-Baptiste Maneyrol

On Sun, 25 Feb 2024 16:00:24 +0000
inv.git-commit@tdk.com wrote:

> From: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>
> 
> 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@tdk.com>

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);


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/4] iio: imu: inv_mpu6050: add WoM event inside accel channels
  2024-02-25 16:00 ` [PATCH 2/4] iio: imu: inv_mpu6050: add WoM event inside accel channels inv.git-commit
@ 2024-03-03 16:58   ` Jonathan Cameron
  0 siblings, 0 replies; 13+ messages in thread
From: Jonathan Cameron @ 2024-03-03 16:58 UTC (permalink / raw)
  To: inv.git-commit; +Cc: lars, linux-iio, Jean-Baptiste Maneyrol

On Sun, 25 Feb 2024 16:00:25 +0000
inv.git-commit@tdk.com wrote:

> From: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>
> 
> Add WoM (mag_adaptive rising) event in accel channels for all
> chips >= MPU-6500. This requires to create new MPU-6500 channels
> as default and MPU-6050 channels for older chips.
> 
> Signed-off-by: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>
Trivial comment inline - otherwise looks good beyond question of 
event type.

Jonathan

> ---
>  drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 132 ++++++++++++++-------
>  1 file changed, 89 insertions(+), 43 deletions(-)
> 
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> index fca7fc1ba4e2..d2544c758815 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> @@ -1303,23 +1303,34 @@ static const struct iio_chan_spec_ext_info inv_ext_info[] = {
>  	{ }
>  };
>  
> -#define INV_MPU6050_CHAN(_type, _channel2, _index)                    \
> -	{                                                             \
> -		.type = _type,                                        \
> -		.modified = 1,                                        \
> -		.channel2 = _channel2,                                \
> -		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
> -		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |	      \
> -				      BIT(IIO_CHAN_INFO_CALIBBIAS),   \
> -		.scan_index = _index,                                 \
> -		.scan_type = {                                        \
> -				.sign = 's',                          \
> -				.realbits = 16,                       \
> -				.storagebits = 16,                    \
> -				.shift = 0,                           \
> -				.endianness = IIO_BE,                 \
> -			     },                                       \
> -		.ext_info = inv_ext_info,                             \
> +static const struct iio_event_spec inv_accel_events[] = {
> +	{
> +		.type = IIO_EV_TYPE_MAG_ADAPTIVE,
> +		.dir = IIO_EV_DIR_RISING,
> +		.mask_shared_by_type = BIT(IIO_EV_INFO_ENABLE) |
> +				       BIT(IIO_EV_INFO_VALUE),
> +	},
> +};
> +
> +#define INV_MPU6050_CHAN(_type, _channel2, _index, _events, _events_nb) \
> +	{                                                               \
> +		.type = _type,                                          \
> +		.modified = 1,                                          \
> +		.channel2 = _channel2,                                  \
> +		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),   \
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |	        \
> +				      BIT(IIO_CHAN_INFO_CALIBBIAS),     \
> +		.event_spec = _events,                                  \
> +		.num_event_specs = _events_nb,                          \
> +		.scan_index = _index,                                   \
> +		.scan_type = {                                          \
> +				.sign = 's',                            \
Maybe take opportunity to tidy up these intents. One tab is enough.
			.sign = 's',

> +				.realbits = 16,                         \
> +				.storagebits = 16,                      \
> +				.shift = 0,                             \
> +				.endianness = IIO_BE,                   \
> +			     },                                         \
		},
> +		.ext_info = inv_ext_info,                               \
>  	}



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/4] iio: imu: inv_mpu6050: add new interrupt handler for WoM events
  2024-02-25 16:00 ` [PATCH 3/4] iio: imu: inv_mpu6050: add new interrupt handler for WoM events inv.git-commit
@ 2024-03-03 17:10   ` Jonathan Cameron
  2024-03-04 11:11     ` Jean-Baptiste Maneyrol
  0 siblings, 1 reply; 13+ messages in thread
From: Jonathan Cameron @ 2024-03-03 17:10 UTC (permalink / raw)
  To: inv.git-commit; +Cc: lars, linux-iio, Jean-Baptiste Maneyrol

On Sun, 25 Feb 2024 16:00:26 +0000
inv.git-commit@tdk.com wrote:

> From: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>
> 
> Add new interrupt handler for generating WoM event from int status
> register bits. Launch from interrupt the trigger poll function for
> data buffer.
> 
> Signed-off-by: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>
> ---
>  drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h     |  2 +
>  drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c    | 11 ----
>  drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c | 56 +++++++++++++++++--
>  3 files changed, 53 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> index 519c1eee96ad..9be67cebbd49 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> @@ -184,6 +184,7 @@ struct inv_mpu6050_hw {
>   *  @magn_orient:       magnetometer sensor chip orientation if available.
>   *  @suspended_sensors:	sensors mask of sensors turned off for suspend
>   *  @data:		read buffer used for bulk reads.
> + *  @it_timestamp:	interrupt timestamp.
>   */
>  struct inv_mpu6050_state {
>  	struct mutex lock;
> @@ -209,6 +210,7 @@ struct inv_mpu6050_state {
>  	unsigned int suspended_sensors;
>  	bool level_shifter;
>  	u8 *data;
> +	s64 it_timestamp;
>  };
>  
>  /*register and associated bit definition*/
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
> index 13da6f523ca2..e282378ee2ca 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
> @@ -51,21 +51,10 @@ irqreturn_t inv_mpu6050_read_fifo(int irq, void *p)
>  	u32 fifo_period;
>  	s64 timestamp;
>  	u8 data[INV_MPU6050_OUTPUT_DATA_SIZE];
> -	int int_status;
>  	size_t i, nb;
>  
>  	mutex_lock(&st->lock);
>  
> -	/* ack interrupt and check status */
> -	result = regmap_read(st->map, st->reg->int_status, &int_status);
> -	if (result) {
> -		dev_err(regmap_get_device(st->map),
> -			"failed to ack interrupt\n");
> -		goto flush_fifo;
> -	}
> -	if (!(int_status & INV_MPU6050_BIT_RAW_DATA_RDY_INT))
> -		goto end_session;
> -
>  	if (!(st->chip_config.accl_fifo_enable |
>  		st->chip_config.gyro_fifo_enable |
>  		st->chip_config.magn_fifo_enable))
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
> index ec2398a87f45..7ffbb9e7c100 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
> @@ -6,6 +6,7 @@
>  #include <linux/pm_runtime.h>
>  
>  #include <linux/iio/common/inv_sensors_timestamp.h>
> +#include <linux/iio/events.h>
>  
>  #include "inv_mpu_iio.h"
>  
> @@ -223,6 +224,52 @@ static const struct iio_trigger_ops inv_mpu_trigger_ops = {
>  	.set_trigger_state = &inv_mpu_data_rdy_trigger_set_state,
>  };
>  
> +static irqreturn_t inv_mpu6050_interrupt_timestamp(int irq, void *p)
> +{
> +	struct iio_dev *indio_dev = p;
> +	struct inv_mpu6050_state *st = iio_priv(indio_dev);
> +
> +	st->it_timestamp = iio_get_time_ns(indio_dev);
> +
> +	return IRQ_WAKE_THREAD;

I think you can use iio_pollfunc_store_time().

> +}
> +
> +static irqreturn_t inv_mpu6050_interrupt_handle(int irq, void *p)
> +{
> +	struct iio_dev *indio_dev = p;
> +	struct inv_mpu6050_state *st = iio_priv(indio_dev);
> +	unsigned int int_status = 0;
> +	int result;
> +
> +	mutex_lock(&st->lock);
> +
> +	/* ack interrupt and check status */
> +	result = regmap_read(st->map, st->reg->int_status, &int_status);
> +	if (result) {
> +		dev_err(regmap_get_device(st->map),
> +			"failed to ack interrupt\n");
> +		goto exit_unlock;
> +	}
> +
> +	/* handle WoM event */
> +	if (st->chip_config.wom_en && (int_status & INV_MPU6500_BIT_WOM_INT))
> +		iio_push_event(indio_dev,
> +			       IIO_UNMOD_EVENT_CODE(IIO_ACCEL, 0, IIO_EV_TYPE_MAG_ADAPTIVE,
> +						    IIO_EV_DIR_RISING),
Maybe should be modified.

Hmm. Is this magnitude of the overall acceleration or is it a per channel thing?

If it's overall then we need to know how they are combined to describe this right.
If it's an X or Y or Z thing we do have a modifier for that though I kind of
regret adding that and wish now we'd just always reported 3 events.
(problem with those X_OR_Y_OR_Z modifiers is they don't generalize well)

> +				st->it_timestamp);
> +
> +exit_unlock:
> +	mutex_unlock(&st->lock);
> +
> +	/* handle raw data interrupt */
> +	if (int_status & INV_MPU6050_BIT_RAW_DATA_RDY_INT) {
> +		indio_dev->pollfunc->timestamp = st->it_timestamp;

As above, why not use the standard function for this.
I don't think anything will have cleared it.

> +		iio_trigger_poll_nested(st->trig);
> +	}
> +
> +	return IRQ_HANDLED;
> +}
> +
>  int inv_mpu6050_probe_trigger(struct iio_dev *indio_dev, int irq_type)
>  {
>  	int ret;
> @@ -235,11 +282,10 @@ int inv_mpu6050_probe_trigger(struct iio_dev *indio_dev, int irq_type)
>  	if (!st->trig)
>  		return -ENOMEM;
>  
> -	ret = devm_request_irq(&indio_dev->dev, st->irq,
> -			       &iio_trigger_generic_data_rdy_poll,
> -			       irq_type,
> -			       "inv_mpu",
> -			       st->trig);
> +	ret = devm_request_threaded_irq(&indio_dev->dev, st->irq,
> +					&inv_mpu6050_interrupt_timestamp,
> +					&inv_mpu6050_interrupt_handle,
> +					irq_type, "inv_mpu", indio_dev);
>  	if (ret)
>  		return ret;
>  


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/4] Add WoM feature as an IIO event
  2024-03-03 16:44 ` [PATCH 0/4] Add WoM feature as an IIO event Jonathan Cameron
@ 2024-03-04 10:50   ` Jean-Baptiste Maneyrol
  0 siblings, 0 replies; 13+ messages in thread
From: Jean-Baptiste Maneyrol @ 2024-03-04 10:50 UTC (permalink / raw)
  To: Jonathan Cameron; +Cc: lars@metafoo.de, linux-iio@vger.kernel.org

Hi Jonathan,

it makes sense indeed, using a value in m/s2 per second and computing the corresponding threshold based on the sampling frequency. Just that this will be a check against the absolute value of accel, no sign supported. That's why I was thinking about magnitude more than threshold.

It will effectively be more complex to handle because it will depend on the sampling frequency and will require to adapt the threshold value when changing sampling frequency.

I will rework this series to switch to ROC instead.

Thanks,
JB


From: Jonathan Cameron <jic23@kernel.org>
Sent: Sunday, March 3, 2024 17:44
To: INV Git Commit <INV.git-commit@tdk.com>
Cc: lars@metafoo.de <lars@metafoo.de>; linux-iio@vger.kernel.org <linux-iio@vger.kernel.org>; Jean-Baptiste Maneyrol <Jean-Baptiste.Maneyrol@tdk.com>
Subject: Re: [PATCH 0/4] Add WoM feature as an IIO event 
 
On Sun, 25 Feb 2024 16: 00: 23 +0000 inv. git-commit@ tdk. com wrote: > From: Jean-Baptiste Maneyrol <jean-baptiste. maneyrol@ tdk. com> > > Add WoM (Wake-on-Motion) feature for all chips supporting it > (all except MPU-6000/6050/9150).  
ZjQcmQRYFpfptBannerStart
This Message Is From an External Sender 
This message came from outside your organization. 
 
ZjQcmQRYFpfptBannerEnd
On Sun, 25 Feb 2024 16:00:23 +0000
inv.git-commit@tdk.com wrote:

> From: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>
> 
> Add WoM (Wake-on-Motion) feature for all chips supporting it
> (all except MPU-6000/6050/9150). WoM compares the magnitude of
> the current accel sample with the previous one against a threshold
> and returns an interrupt event if the value is higher.
> 
> Report WoM as an accel mag_adaptive_rising IIO event, add system
> wakeup functionality if WoM is on and put the chip in low-power
> mode when the system is suspended. WoM threshold value is in SI
> units since the chip is using an absolute value in mg.


Adaptive thresholds are normally used when a threshold is based on
some offset form a heavily filtered version of the same channel (often
with filter resets and other nastiness)  It's an obscure hack we added
because we couldn't really get these to fit with another scheme.
This seems simpler than that from your description.

As it's just against the previous reading and I assume the chip
is in a self clocking mode during this, it might be more appropriate
to use a Rate of Change Event (ROC).  There are references to
this being a threshold on highpassed sample, so I guess there might
be some filtering to make this messier? 

The control of a roc threshold will be a little more complex as
you'll need to take into account the sampling frequency.
Advantage is you end up with something easily human understandable.

A human doesn't want to have to figure out what the right value
is for the particular frequency they are sampling at.

Jonathan

> 
> 
> Jean-Baptiste Maneyrol (4):
>   iio: imu: inv_mpu6050: add WoM (Wake-on-Motion) sensor
>   iio: imu: inv_mpu6050: add WoM event inside accel channels
>   iio: imu: inv_mpu6050: add new interrupt handler for WoM events
>   iio: imu: inv_mpu6050: add WoM suspend wakeup with low-power mode
> 
>  drivers/iio/imu/inv_mpu6050/inv_mpu_core.c    | 523 +++++++++++++++---
>  drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h     |  33 +-
>  drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c    |  17 +-
>  drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c |  70 ++-
>  4 files changed, 541 insertions(+), 102 deletions(-)
> 


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/4] iio: imu: inv_mpu6050: add WoM (Wake-on-Motion) sensor
  2024-03-03 16:56   ` Jonathan Cameron
@ 2024-03-04 10:57     ` Jean-Baptiste Maneyrol
  0 siblings, 0 replies; 13+ messages in thread
From: Jean-Baptiste Maneyrol @ 2024-03-04 10:57 UTC (permalink / raw)
  To: Jonathan Cameron; +Cc: lars@metafoo.de, linux-iio@vger.kernel.org

Hi Jonathan,

you're right that for MPU-6500 this is not documented.

In fact if I remember well, all chips were supposed to support 3-axes but it never works as expected until ICM-20609. For MPU-6500, only 1 bit is advertised, on ICM-20608 you have to use the 3 bits at the same time, and starting with ICM-20609 you can use the 3 separate bits.

Using the 3 bits is working for all chips when testing, but certainly it would be better to comply with the datasheet and use only 1 bit for MPU-6500.

Thanks for spotting this.

Best regards,
JB

From: Jonathan Cameron <jic23@kernel.org>
Sent: Sunday, March 3, 2024 17:56
To: INV Git Commit <INV.git-commit@tdk.com>
Cc: lars@metafoo.de <lars@metafoo.de>; linux-iio@vger.kernel.org <linux-iio@vger.kernel.org>; Jean-Baptiste Maneyrol <Jean-Baptiste.Maneyrol@tdk.com>
Subject: Re: [PATCH 1/4] iio: imu: inv_mpu6050: add WoM (Wake-on-Motion) sensor 
 
On Sun, 25 Feb 2024 16: 00: 24 +0000 inv. git-commit@ tdk. com wrote: > From: Jean-Baptiste Maneyrol <jean-baptiste. maneyrol@ tdk. com> > > WoM is a threshold test on accel value comparing actual sample > with previous one. It maps 
ZjQcmQRYFpfptBannerStart
This Message Is From an External Sender 
This message came from outside your organization. 
 
ZjQcmQRYFpfptBannerEnd
On Sun, 25 Feb 2024 16:00:24 +0000
inv.git-commit@tdk.com wrote:

> From: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>
> 
> 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@tdk.com>

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);



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/4] iio: imu: inv_mpu6050: add new interrupt handler for WoM events
  2024-03-03 17:10   ` Jonathan Cameron
@ 2024-03-04 11:11     ` Jean-Baptiste Maneyrol
  2024-03-09 17:38       ` Jonathan Cameron
  0 siblings, 1 reply; 13+ messages in thread
From: Jean-Baptiste Maneyrol @ 2024-03-04 11:11 UTC (permalink / raw)
  To: Jonathan Cameron; +Cc: lars@metafoo.de, linux-iio@vger.kernel.org

>From: Jonathan Cameron <jic23@kernel.org>
>Sent: Sunday, March 3, 2024 18:10
>To: INV Git Commit <INV.git-commit@tdk.com>
>Cc: lars@metafoo.de <lars@metafoo.de>; linux-iio@vger.kernel.org <linux-iio@vger.kernel.org>; Jean-Baptiste Maneyrol <Jean-Baptiste.Maneyrol@tdk.com>
>Subject: Re: [PATCH 3/4] iio: imu: inv_mpu6050: add new interrupt handler for WoM events 
> 
>On Sun, 25 Feb 2024 16: 00: 26 +0000 inv. git-commit@ tdk. com wrote: > From: Jean-Baptiste Maneyrol <jean-baptiste. maneyrol@ tdk. com> > > Add new interrupt handler for generating WoM event from int status > register bits. Launch 
>ZjQcmQRYFpfptBannerStart
>This Message Is From an External Sender 
>This message came from outside your organization. 
> 
>ZjQcmQRYFpfptBannerEnd
>On Sun, 25 Feb 2024 16:00:26 +0000
>inv.git-commit@tdk.com wrote:
>
>> From: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>
>> 
>> Add new interrupt handler for generating WoM event from int status
>> register bits. Launch from interrupt the trigger poll function for
>> data buffer.
>> 
>> Signed-off-by: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>
>> ---
>>  drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h     |  2 +
>>  drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c    | 11 ----
>>  drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c | 56 +++++++++++++++++--
>>  3 files changed, 53 insertions(+), 16 deletions(-)
>> 
>> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
>> index 519c1eee96ad..9be67cebbd49 100644
>> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
>> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
>> @@ -184,6 +184,7 @@ struct inv_mpu6050_hw {
>>   *  @magn_orient:       magnetometer sensor chip orientation if available.
>>   *  @suspended_sensors:	sensors mask of sensors turned off for suspend
>>   *  @data:		read buffer used for bulk reads.
>> + *  @it_timestamp:	interrupt timestamp.
>>   */
>>  struct inv_mpu6050_state {
>>  	struct mutex lock;
>> @@ -209,6 +210,7 @@ struct inv_mpu6050_state {
>>  	unsigned int suspended_sensors;
>>  	bool level_shifter;
>>  	u8 *data;
>> +	s64 it_timestamp;
>>  };
>>  
>>  /*register and associated bit definition*/
>> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
>> index 13da6f523ca2..e282378ee2ca 100644
>> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
>> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
>> @@ -51,21 +51,10 @@ irqreturn_t inv_mpu6050_read_fifo(int irq, void *p)
>>  	u32 fifo_period;
>>  	s64 timestamp;
>>  	u8 data[INV_MPU6050_OUTPUT_DATA_SIZE];
>> -	int int_status;
>>  	size_t i, nb;
>>  
>>  	mutex_lock(&st->lock);
>>  
>> -	/* ack interrupt and check status */
>> -	result = regmap_read(st->map, st->reg->int_status, &int_status);
>> -	if (result) {
>> -		dev_err(regmap_get_device(st->map),
>> -			"failed to ack interrupt\n");
>> -		goto flush_fifo;
>> -	}
>> -	if (!(int_status & INV_MPU6050_BIT_RAW_DATA_RDY_INT))
>> -		goto end_session;
>> -
>>  	if (!(st->chip_config.accl_fifo_enable |
>>  		st->chip_config.gyro_fifo_enable |
>>  		st->chip_config.magn_fifo_enable))
>> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
>> index ec2398a87f45..7ffbb9e7c100 100644
>> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
>> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
>> @@ -6,6 +6,7 @@
>>  #include <linux/pm_runtime.h>
>>  
>>  #include <linux/iio/common/inv_sensors_timestamp.h>
>> +#include <linux/iio/events.h>
>>  
>>  #include "inv_mpu_iio.h"
>>  
>> @@ -223,6 +224,52 @@ static const struct iio_trigger_ops inv_mpu_trigger_ops = {
>>  	.set_trigger_state = &inv_mpu_data_rdy_trigger_set_state,
>>  };
>>  
>> +static irqreturn_t inv_mpu6050_interrupt_timestamp(int irq, void *p)
>> +{
>> +	struct iio_dev *indio_dev = p;
>> +	struct inv_mpu6050_state *st = iio_priv(indio_dev);
>> +
>> +	st->it_timestamp = iio_get_time_ns(indio_dev);
>> +
>> +	return IRQ_WAKE_THREAD;
>
>I think you can use iio_pollfunc_store_time().

The problem here is that interrupt can happen without buffer/trigger on, when only WoM is on.
In my understanding, poll function is not attached when buffer is off and poll function is NULL.

>
>> +}
>> +
>> +static irqreturn_t inv_mpu6050_interrupt_handle(int irq, void *p)
>> +{
>> +	struct iio_dev *indio_dev = p;
>> +	struct inv_mpu6050_state *st = iio_priv(indio_dev);
>> +	unsigned int int_status = 0;
>> +	int result;
>> +
>> +	mutex_lock(&st->lock);
>> +
>> +	/* ack interrupt and check status */
>> +	result = regmap_read(st->map, st->reg->int_status, &int_status);
>> +	if (result) {
>> +		dev_err(regmap_get_device(st->map),
>> +			"failed to ack interrupt\n");
>> +		goto exit_unlock;
>> +	}
>> +
>> +	/* handle WoM event */
>> +	if (st->chip_config.wom_en && (int_status & INV_MPU6500_BIT_WOM_INT))
>> +		iio_push_event(indio_dev,
>> +			       IIO_UNMOD_EVENT_CODE(IIO_ACCEL, 0, IIO_EV_TYPE_MAG_ADAPTIVE,
>> +						    IIO_EV_DIR_RISING),
>Maybe should be modified.
>
>Hmm. Is this magnitude of the overall acceleration or is it a per channel thing?
>
>If it's overall then we need to know how they are combined to describe this right.
>If it's an X or Y or Z thing we do have a modifier for that though I kind of
>regret adding that and wish now we'd just always reported 3 events.
>(problem with those X_OR_Y_OR_Z modifiers is they don't generalize well)
>

This an OR of the 3 axes. We can use X_OR_Y_OR_Z then.

>> +				st->it_timestamp);
>> +
>> +exit_unlock:
>> +	mutex_unlock(&st->lock);
>> +
>> +	/* handle raw data interrupt */
>> +	if (int_status & INV_MPU6050_BIT_RAW_DATA_RDY_INT) {
>> +		indio_dev->pollfunc->timestamp = st->it_timestamp;
>
>As above, why not use the standard function for this.
>I don't think anything will have cleared it.

We need to use the timestamp taken in the hard interrupt handler, that's why I am not calling iio_pollfunc_store_time here.
And I am not calling it in the hard handler because in my understanding poll function is not attached when buffer is off.

>
>> +		iio_trigger_poll_nested(st->trig);
>> +	}
>> +
>> +	return IRQ_HANDLED;
>> +}
>> +
>>  int inv_mpu6050_probe_trigger(struct iio_dev *indio_dev, int irq_type)
>>  {
>>  	int ret;
>> @@ -235,11 +282,10 @@ int inv_mpu6050_probe_trigger(struct iio_dev *indio_dev, int irq_type)
>>  	if (!st->trig)
>>  		return -ENOMEM;
>>  
>> -	ret = devm_request_irq(&indio_dev->dev, st->irq,
>> -			       &iio_trigger_generic_data_rdy_poll,
>> -			       irq_type,
>> -			       "inv_mpu",
>> -			       st->trig);
>> +	ret = devm_request_threaded_irq(&indio_dev->dev, st->irq,
>> +					&inv_mpu6050_interrupt_timestamp,
>> +					&inv_mpu6050_interrupt_handle,
>> +					irq_type, "inv_mpu", indio_dev);
>>  	if (ret)
>>  		return ret;
>>  
>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/4] iio: imu: inv_mpu6050: add new interrupt handler for WoM events
  2024-03-04 11:11     ` Jean-Baptiste Maneyrol
@ 2024-03-09 17:38       ` Jonathan Cameron
  0 siblings, 0 replies; 13+ messages in thread
From: Jonathan Cameron @ 2024-03-09 17:38 UTC (permalink / raw)
  To: Jean-Baptiste Maneyrol; +Cc: lars@metafoo.de, linux-iio@vger.kernel.org


> >>  		st->chip_config.magn_fifo_enable))
> >> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
> >> index ec2398a87f45..7ffbb9e7c100 100644
> >> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
> >> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
> >> @@ -6,6 +6,7 @@
> >>  #include <linux/pm_runtime.h>
> >>  
> >>  #include <linux/iio/common/inv_sensors_timestamp.h>
> >> +#include <linux/iio/events.h>
> >>  
> >>  #include "inv_mpu_iio.h"
> >>  
> >> @@ -223,6 +224,52 @@ static const struct iio_trigger_ops inv_mpu_trigger_ops = {
> >>  	.set_trigger_state = &inv_mpu_data_rdy_trigger_set_state,
> >>  };
> >>  
> >> +static irqreturn_t inv_mpu6050_interrupt_timestamp(int irq, void *p)
> >> +{
> >> +	struct iio_dev *indio_dev = p;
> >> +	struct inv_mpu6050_state *st = iio_priv(indio_dev);
> >> +
> >> +	st->it_timestamp = iio_get_time_ns(indio_dev);
> >> +
> >> +	return IRQ_WAKE_THREAD;  
> >
> >I think you can use iio_pollfunc_store_time().  
> 
> The problem here is that interrupt can happen without buffer/trigger on, when only WoM is on.
> In my understanding, poll function is not attached when buffer is off and poll function is NULL.

Ah ok - I'd missed that mattered here.


^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2024-03-09 17:38 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-02-25 16:00 [PATCH 0/4] Add WoM feature as an IIO event inv.git-commit
2024-02-25 16:00 ` [PATCH 1/4] iio: imu: inv_mpu6050: add WoM (Wake-on-Motion) sensor inv.git-commit
2024-03-03 16:56   ` Jonathan Cameron
2024-03-04 10:57     ` Jean-Baptiste Maneyrol
2024-02-25 16:00 ` [PATCH 2/4] iio: imu: inv_mpu6050: add WoM event inside accel channels inv.git-commit
2024-03-03 16:58   ` Jonathan Cameron
2024-02-25 16:00 ` [PATCH 3/4] iio: imu: inv_mpu6050: add new interrupt handler for WoM events inv.git-commit
2024-03-03 17:10   ` Jonathan Cameron
2024-03-04 11:11     ` Jean-Baptiste Maneyrol
2024-03-09 17:38       ` Jonathan Cameron
2024-02-25 16:00 ` [PATCH 4/4] iio: imu: inv_mpu6050: add WoM suspend wakeup with low-power mode inv.git-commit
2024-03-03 16:44 ` [PATCH 0/4] Add WoM feature as an IIO event Jonathan Cameron
2024-03-04 10:50   ` Jean-Baptiste Maneyrol

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox