[PATCH v2 0/4] Add WoM feature as an IIO event

[PATCH v2 0/4] Add WoM feature as an IIO event

[inv.git-commit]

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.

WoM is checking against all axes and maps best to ROC since it
compares the last 2 samples. Thus report WoM as an accel
x|y|z roc_rising IIO event, add system wakeup functionality if
WoM is on and put the chip in low-power mode when the system
is suspended. Corresponding ROC value is in SI units since
the chip is using an absolute value in mg.

v2:
- Rework to use accel x|y|z roc for reporting WoM event
- Use only datasheet advertised bits for MPU-6500 family chips

Jean-Baptiste Maneyrol (4):
  iio: imu: inv_mpu6050: add WoM (Wake-on-Motion) sensor
  iio: imu: inv_mpu6050: add WoM event as accel event
  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    | 529 ++++++++++++++++--
 drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h     |  35 +-
 drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c    |  17 +-
 drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c |  86 ++-
 4 files changed, 600 insertions(+), 67 deletions(-)

-- 
2.34.1

[PATCH v2 1/4] iio: imu: inv_mpu6050: add WoM (Wake-on-Motion) sensor

[inv.git-commit]

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 roc rising event.
Add support of a new WOM sensor and functions for handling the
corresponding roc_rising event. The event value is in SI units.
Ensure WOM is stopped and restarted at suspend-resume, handle
usage with buffer data ready interrupt, and handle change in
sampling rate impacting already set roc value.

Signed-off-by: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>
---
 drivers/iio/imu/inv_mpu6050/inv_mpu_core.c    | 293 +++++++++++++++++-
 drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h     |  19 +-
 drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c    |   6 +-
 drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c |  14 +-
 4 files changed, 319 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..ad42be809f09 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
@@ -15,6 +15,7 @@
 #include <linux/acpi.h>
 #include <linux/platform_device.h>
 #include <linux/regulator/consumer.h>
+#include <linux/math64.h>
 #include <linux/pm.h>
 #include <linux/pm_runtime.h>
 #include <linux/property.h>
@@ -332,7 +333,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 +346,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 +448,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 +912,251 @@ static int inv_mpu6050_write_raw(struct iio_dev *indio_dev,
 	return result;
 }
 
+static u64 inv_mpu6050_convert_wom_to_roc(unsigned int threshold, unsigned int freq_div)
+{
+	/* 4mg per LSB converted in m/s² in micro (1000000) */
+	const unsigned int convert = 4U * 9807U;
+	u64 value;
+
+	value = threshold * convert;
+	/* compute the differential by multiplying by the frequency */
+	value = div_u64(value * INV_MPU6050_INTERNAL_FREQ_HZ, freq_div);
+
+	return value;
+}
+
+static unsigned int inv_mpu6050_convert_roc_to_wom(u64 roc, unsigned int freq_div)
+{
+	/* 4mg per LSB converted in m/s² in micro (1000000) */
+	const unsigned int convert = 4U * 9807U;
+	u64 value;
+	unsigned int threshold;
+
+	/* return 0 only if roc is 0 */
+	if (roc == 0)
+		return 0;
+
+	value = div_u64(roc * freq_div, convert * INV_MPU6050_INTERNAL_FREQ_HZ);
+	/* limit value to 8 bits and prevent 0*/
+	if (value == 0)
+		threshold = 1;
+	else if (value > 255)
+		threshold = 255;
+	else
+		threshold = value;
+
+	return threshold;
+}
+
+static int inv_mpu6050_set_wom_int(struct inv_mpu6050_state *st, bool on)
+{
+	unsigned int reg_val, val;
+
+	switch (st->chip_type) {
+	case INV_MPU6050:
+	case INV_MPU6500:
+	case INV_MPU6515:
+	case INV_MPU6880:
+	case INV_MPU6000:
+	case INV_MPU9150:
+	case INV_MPU9250:
+	case INV_MPU9255:
+		reg_val = INV_MPU6500_BIT_WOM_INT_EN;
+		break;
+	default:
+		reg_val = INV_ICM20608_BIT_WOM_INT_EN;
+		break;
+	}
+
+	val = on ? reg_val : 0;
+
+	return regmap_update_bits(st->map, st->reg->int_enable, reg_val, val);
+}
+
+static int inv_mpu6050_set_wom_threshold(struct inv_mpu6050_state *st, u64 value,
+					 unsigned int freq_div)
+{
+	unsigned int threshold;
+	int result;
+
+	/* convert roc to wom threshold and convert back to handle clipping */
+	threshold = inv_mpu6050_convert_roc_to_wom(value, freq_div);
+	value = inv_mpu6050_convert_wom_to_roc(threshold, freq_div);
+
+	dev_dbg(regmap_get_device(st->map), "wom_threshold: 0x%x\n", threshold);
+
+	switch (st->chip_type) {
+	case INV_ICM20609:
+	case INV_ICM20689:
+	case INV_ICM20600:
+	case INV_ICM20602:
+	case INV_ICM20690:
+		st->data[0] = threshold;
+		st->data[1] = threshold;
+		st->data[2] = threshold;
+		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, threshold);
+		break;
+	}
+	if (!result)
+		st->chip_config.roc_threshold = value;
+
+	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 roc rising) event */
+	if (chan->type != IIO_ACCEL || type != IIO_EV_TYPE_ROC || 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 roc rising) event */
+	if (chan->type != IIO_ACCEL || type != IIO_EV_TYPE_ROC || 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);
+	u32 rem;
+
+	/* support only WoM (accel roc rising) event value */
+	if (chan->type != IIO_ACCEL || type != IIO_EV_TYPE_ROC ||
+			dir != IIO_EV_DIR_RISING || info != IIO_EV_INFO_VALUE)
+		return -EINVAL;
+
+	/* return value in micro */
+	*val = div_u64_rem(st->chip_config.roc_threshold, 1000000U, &rem);
+	*val2 = rem;
+
+	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);
+	struct device *pdev = regmap_get_device(st->map);
+	u64 value;
+	int result;
+
+	/* support only WoM (accel roc rising) event value */
+	if (chan->type != IIO_ACCEL || type != IIO_EV_TYPE_ROC ||
+			dir != IIO_EV_DIR_RISING || info != IIO_EV_INFO_VALUE)
+		return -EINVAL;
+
+	if (val < 0 || val2 < 0)
+		return -EINVAL;
+
+	mutex_lock(&st->lock);
+
+	result = pm_runtime_resume_and_get(pdev);
+	if (result)
+		goto exit_unlock;
+
+	value = (u64)val * 1000000ULL + (u64)val2;
+	result = inv_mpu6050_set_wom_threshold(st, value, INV_MPU6050_FREQ_DIVIDER(st));
+
+	pm_runtime_mark_last_busy(pdev);
+	pm_runtime_put_autosuspend(pdev);
+
+exit_unlock:
+	mutex_unlock(&st->lock);
+	return result;
+}
+
 /*
  *  inv_mpu6050_set_lpf() - set low pass filer based on fifo rate.
  *
@@ -989,6 +1253,12 @@ inv_mpu6050_fifo_rate_store(struct device *dev, struct device_attribute *attr,
 	if (result)
 		goto fifo_rate_fail_power_off;
 
+	/* update wom threshold since roc is dependent on sampling frequency */
+	result = inv_mpu6050_set_wom_threshold(st, st->chip_config.roc_threshold,
+					       INV_MPU6050_FREQ_DIVIDER(st));
+	if (result)
+		goto fifo_rate_fail_power_off;
+
 	pm_runtime_mark_last_busy(pdev);
 fifo_rate_fail_power_off:
 	pm_runtime_put_autosuspend(pdev);
@@ -1326,6 +1596,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 +1980,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 +2015,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 +2029,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 +2055,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..19adccf388cf 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;
+	u64 roc_threshold;
 };
 
 /*
@@ -256,12 +260,16 @@ 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(6)
+#define INV_ICM20608_BIT_WOM_INT_EN         GENMASK(7, 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(6)
+#define INV_ICM20608_BIT_WOM_INT            GENMASK(7, 5)
 #define INV_MPU6050_BIT_FIFO_OVERFLOW_INT   0x10
 #define INV_MPU6050_BIT_RAW_DATA_RDY_INT    0x01
 
@@ -301,6 +309,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 +333,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

[PATCH v2 2/4] iio: imu: inv_mpu6050: add WoM event as accel event

[inv.git-commit]

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

Add WoM (roc rising) event as accel x_or_y_or_z event 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 | 67 +++++++++++++++++++---
 1 file changed, 59 insertions(+), 8 deletions(-)

diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
index ad42be809f09..ddc905bb74d6 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
@@ -1359,6 +1359,15 @@ static const struct iio_chan_spec_ext_info inv_ext_info[] = {
 	{ }
 };
 
+static const struct iio_event_spec inv_wom_events[] = {
+	{
+		.type = IIO_EV_TYPE_ROC,
+		.dir = IIO_EV_DIR_RISING,
+		.mask_separate = BIT(IIO_EV_INFO_ENABLE) |
+				 BIT(IIO_EV_INFO_VALUE),
+	},
+};
+
 #define INV_MPU6050_CHAN(_type, _channel2, _index)                    \
 	{                                                             \
 		.type = _type,                                        \
@@ -1394,7 +1403,17 @@ static const struct iio_chan_spec_ext_info inv_ext_info[] = {
 		},						\
 	}
 
-static const struct iio_chan_spec inv_mpu_channels[] = {
+#define INV_MPU6050_EVENT_CHAN(_type, _channel2, _events, _events_nb)	\
+{									\
+	.type = _type,							\
+	.modified = 1,							\
+	.channel2 = _channel2,						\
+	.event_spec = _events,						\
+	.num_event_specs = _events_nb,					\
+	.scan_index = -1,						\
+}
+
+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),
@@ -1408,6 +1427,23 @@ static const struct iio_chan_spec inv_mpu_channels[] = {
 	INV_MPU6050_CHAN(IIO_ACCEL, IIO_MOD_Z, INV_MPU6050_SCAN_ACCL_Z),
 };
 
+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_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_EVENT_CHAN(IIO_ACCEL, IIO_MOD_X_OR_Y_OR_Z,
+			       inv_wom_events, ARRAY_SIZE(inv_wom_events)),
+};
+
 #define INV_MPU6050_SCAN_MASK_3AXIS_ACCEL	\
 	(BIT(INV_MPU6050_SCAN_ACCL_X)		\
 	| BIT(INV_MPU6050_SCAN_ACCL_Y)		\
@@ -1887,6 +1923,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);
@@ -1900,13 +1942,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;
 	}
@@ -1915,9 +1957,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

[PATCH v2 3/4] iio: imu: inv_mpu6050: add new interrupt handler for WoM events

[inv.git-commit]

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 | 72 +++++++++++++++++--
 3 files changed, 69 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 19adccf388cf..e97a63ad2c31 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..585e5112f7a5 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,68 @@ 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, wom_bits;
+	int result;
+
+	switch (st->chip_type) {
+	case INV_MPU6050:
+	case INV_MPU6500:
+	case INV_MPU6515:
+	case INV_MPU6880:
+	case INV_MPU6000:
+	case INV_MPU9150:
+	case INV_MPU9250:
+	case INV_MPU9255:
+		wom_bits = INV_MPU6500_BIT_WOM_INT;
+		break;
+	default:
+		wom_bits = INV_ICM20608_BIT_WOM_INT;
+		break;
+	}
+
+	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");
+		int_status = 0;
+		goto exit_unlock;
+	}
+
+	/* handle WoM event */
+	if (st->chip_config.wom_en && (int_status & wom_bits))
+		iio_push_event(indio_dev,
+				IIO_MOD_EVENT_CODE(IIO_ACCEL, 0, IIO_MOD_X_OR_Y_OR_Z,
+						   IIO_EV_TYPE_ROC, 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 +298,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

[PATCH v2 4/4] iio: imu: inv_mpu6050: add WoM suspend wakeup with low-power mode

[inv.git-commit]

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
and adjust threshold to follow the required roc value.

Signed-off-by: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>
---
 drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 175 +++++++++++++++++----
 drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h  |  14 ++
 2 files changed, 156 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 ddc905bb74d6..17844390b786 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
@@ -288,7 +288,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;
 
@@ -302,6 +302,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);
@@ -317,7 +319,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;
@@ -359,7 +361,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;
@@ -466,7 +468,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;
 
@@ -496,22 +498,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:
@@ -530,6 +519,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.
  *
@@ -1007,6 +1015,84 @@ static int inv_mpu6050_set_wom_threshold(struct inv_mpu6050_state *st, u64 value
 	return result;
 }
 
+static int inv_mpu6050_set_lp_odr(struct inv_mpu6050_state *st, unsigned int freq_div,
+				  unsigned int *lp_div)
+{
+	static const unsigned int freq_dividers[] = {2, 4, 8, 16, 32, 64, 128, 256};
+	static const unsigned int reg_values[] = {
+		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 val, i;
+
+	switch (st->chip_type) {
+	case INV_ICM20609:
+	case INV_ICM20689:
+	case INV_ICM20600:
+	case INV_ICM20602:
+	case INV_ICM20690:
+		/* nothing to do */
+		*lp_div = INV_MPU6050_FREQ_DIVIDER(st);
+		return 0;
+	default:
+		break;
+	}
+
+	/* found the nearest superior frequency divider */
+	i = ARRAY_SIZE(reg_values) - 1;
+	val = reg_values[i];
+	*lp_div = freq_dividers[i];
+	for (i = 0; i < ARRAY_SIZE(freq_dividers); ++i) {
+		if (freq_div <= freq_dividers[i]) {
+			val = reg_values[i];
+			*lp_div = freq_dividers[i];
+			break;
+		}
+	}
+
+	dev_dbg(regmap_get_device(st->map), "lp_odr: 0x%x\n", val);
+	return regmap_write(st->map, INV_MPU6500_REG_LP_ODR, val);
+}
+
+static int inv_mpu6050_set_wom_lp(struct inv_mpu6050_state *st, bool on)
+{
+	unsigned int lp_div;
+	int result;
+
+	if (on) {
+		/* set low power ODR */
+		result = inv_mpu6050_set_lp_odr(st, INV_MPU6050_FREQ_DIVIDER(st), &lp_div);
+		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;
+		/* update wom threshold with new low-power frequency divider */
+		result = inv_mpu6050_set_wom_threshold(st, st->chip_config.roc_threshold, lp_div);
+		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 wom threshold */
+		result = inv_mpu6050_set_wom_threshold(st, st->chip_config.roc_threshold,
+						       INV_MPU6050_FREQ_DIVIDER(st));
+		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_enable_wom(struct inv_mpu6050_state *st, bool en)
 {
 	struct device *pdev = regmap_get_device(st->map);
@@ -1847,6 +1933,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))
@@ -2012,16 +2099,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);
@@ -2031,7 +2129,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;
@@ -2050,6 +2148,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);
@@ -2066,13 +2165,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;
@@ -2080,17 +2181,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 e97a63ad2c31..6ba9d42b2537 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
@@ -304,6 +304,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
 
@@ -335,6 +336,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)
@@ -451,6 +453,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

Re: [PATCH v2 1/4] iio: imu: inv_mpu6050: add WoM (Wake-on-Motion) sensor

[Jonathan Cameron]

On Fri,  8 Mar 2024 15:10:20 +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

Very short wrap on this message. Wrap at 75 chars for commit messages.

> with previous one. It maps best to roc rising event.
> Add support of a new WOM sensor and functions for handling the
> corresponding roc_rising event. The event value is in SI units.
> Ensure WOM is stopped and restarted at suspend-resume, handle
> usage with buffer data ready interrupt, and handle change in
> sampling rate impacting already set roc value.
> 
> Signed-off-by: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>
Hi Jean-Baptiste

Some minor comments inline

Jonathan

> ---
>  drivers/iio/imu/inv_mpu6050/inv_mpu_core.c    | 293 +++++++++++++++++-
>  drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h     |  19 +-
>  drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c    |   6 +-
>  drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c |  14 +-
>  4 files changed, 319 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..ad42be809f09 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> @@ -15,6 +15,7 @@

> +static u64 inv_mpu6050_convert_wom_to_roc(unsigned int threshold, unsigned int freq_div)
> +{
> +	/* 4mg per LSB converted in m/s² in micro (1000000) */
> +	const unsigned int convert = 4U * 9807U;
> +	u64 value;
> +
> +	value = threshold * convert;
> +	/* compute the differential by multiplying by the frequency */
> +	value = div_u64(value * INV_MPU6050_INTERNAL_FREQ_HZ, freq_div);

	return div_u64();


> +
> +	return value;
> +}
> +
> +static unsigned int inv_mpu6050_convert_roc_to_wom(u64 roc, unsigned int freq_div)
> +{
> +	/* 4mg per LSB converted in m/s² in micro (1000000) */
> +	const unsigned int convert = 4U * 9807U;
> +	u64 value;
> +	unsigned int threshold;
> +
> +	/* return 0 only if roc is 0 */
> +	if (roc == 0)
> +		return 0;
> +
> +	value = div_u64(roc * freq_div, convert * INV_MPU6050_INTERNAL_FREQ_HZ);
> +	/* limit value to 8 bits and prevent 0*/
> +	if (value == 0)
> +		threshold = 1;

	return min(255, max(1, value));

unless this open code flow is needed for later changes.


> +	else if (value > 255)
> +		threshold = 255;
> +	else
> +		threshold = value;
> +
> +	return threshold;
> +}
> +

...

> +static int inv_mpu6050_set_wom_threshold(struct inv_mpu6050_state *st, u64 value,
> +					 unsigned int freq_div)
> +{
> +	unsigned int threshold;
> +	int result;
> +
> +	/* convert roc to wom threshold and convert back to handle clipping */
> +	threshold = inv_mpu6050_convert_roc_to_wom(value, freq_div);
> +	value = inv_mpu6050_convert_wom_to_roc(threshold, freq_div);
> +
> +	dev_dbg(regmap_get_device(st->map), "wom_threshold: 0x%x\n", threshold);
> +
> +	switch (st->chip_type) {
> +	case INV_ICM20609:
> +	case INV_ICM20689:
> +	case INV_ICM20600:
> +	case INV_ICM20602:
> +	case INV_ICM20690:
> +		st->data[0] = threshold;
> +		st->data[1] = threshold;
> +		st->data[2] = threshold;
> +		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, threshold);
> +		break;
> +	}
> +	if (!result)
> +		st->chip_config.roc_threshold = value;

	if (result)
		return result;

	st->chip_config.roc_threshold = value;

	return 0;

More code, but keeping all error handling out of line makes for easier code review.

> +
> +	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 roc rising) event */
> +	if (chan->type != IIO_ACCEL || type != IIO_EV_TYPE_ROC || dir != IIO_EV_DIR_RISING)

Long line - consider a line break.  I don't think it will hurt readability
and I'd still prefer IIO mostly stays under 80 chars

> +		return -EINVAL;
> +
> +	enable = !!state;
> +
> +	mutex_lock(&st->lock);

At some point we should switch to automated unlocking.
	guard(mutex)(&st->lock);
as can do nice simple direct returns without the goto.


> +
> +	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);
> +	u32 rem;
> +
> +	/* support only WoM (accel roc rising) event value */
> +	if (chan->type != IIO_ACCEL || type != IIO_EV_TYPE_ROC ||
> +			dir != IIO_EV_DIR_RISING || info != IIO_EV_INFO_VALUE)

Odd alignment.  dir should align with chan.

> +		return -EINVAL;
> +
> +	/* return value in micro */
> +	*val = div_u64_rem(st->chip_config.roc_threshold, 1000000U, &rem);
> +	*val2 = rem;
> +
> +	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);
> +	struct device *pdev = regmap_get_device(st->map);
> +	u64 value;
> +	int result;
> +
> +	/* support only WoM (accel roc rising) event value */
> +	if (chan->type != IIO_ACCEL || type != IIO_EV_TYPE_ROC ||
> +			dir != IIO_EV_DIR_RISING || info != IIO_EV_INFO_VALUE)

As above, odd alignment.

> +		return -EINVAL;
> +
> +	if (val < 0 || val2 < 0)
> +		return -EINVAL;
> +
> +	mutex_lock(&st->lock);
> +
> +	result = pm_runtime_resume_and_get(pdev);
> +	if (result)
> +		goto exit_unlock;
> +
> +	value = (u64)val * 1000000ULL + (u64)val2;
> +	result = inv_mpu6050_set_wom_threshold(st, value, INV_MPU6050_FREQ_DIVIDER(st));
> +
> +	pm_runtime_mark_last_busy(pdev);
> +	pm_runtime_put_autosuspend(pdev);
> +
> +exit_unlock:
> +	mutex_unlock(&st->lock);
> +	return result;
> +}
> +
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> index 5950e2419ebb..19adccf388cf 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;
> +	u64 roc_threshold;
No docs?
>  };
>

Re: [PATCH v2 2/4] iio: imu: inv_mpu6050: add WoM event as accel event

[Jonathan Cameron]

On Fri,  8 Mar 2024 15:10:21 +0000
inv.git-commit@tdk.com wrote:

> From: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>
> 
> Add WoM (roc rising) event as accel x_or_y_or_z event 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>
LGTM

Re: [PATCH v2 4/4] iio: imu: inv_mpu6050: add WoM suspend wakeup with low-power mode

[Jonathan Cameron]

On Fri,  8 Mar 2024 15:10:23 +0000
inv.git-commit@tdk.com wrote:

> 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
> and adjust threshold to follow the required roc value.
> 
> Signed-off-by: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>
A few comments inline, but nothing significant that needs changes.
Not sure why this didn't send the other day - just found it still open :(

Jonathan

> ---

> +
> +static int inv_mpu6050_set_wom_lp(struct inv_mpu6050_state *st, bool on)

You could just split this given almost nothing shared between the two branches.

> +{
> +	unsigned int lp_div;
> +	int result;
> +
> +	if (on) {
> +		/* set low power ODR */
> +		result = inv_mpu6050_set_lp_odr(st, INV_MPU6050_FREQ_DIVIDER(st), &lp_div);
> +		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;
> +		/* update wom threshold with new low-power frequency divider */
> +		result = inv_mpu6050_set_wom_threshold(st, st->chip_config.roc_threshold, lp_div);
> +		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 wom threshold */
> +		result = inv_mpu6050_set_wom_threshold(st, st->chip_config.roc_threshold,
> +						       INV_MPU6050_FREQ_DIVIDER(st));
> +		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_enable_wom(struct inv_mpu6050_state *st, bool en)
>  {
>  	struct device *pdev = regmap_get_device(st->map);
> @@ -1847,6 +1933,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))
> @@ -2012,16 +2099,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);
A very good case for using guard(mutex)(&st->lock); but that can be a future series.

> -	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);
> @@ -2031,7 +2129,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;
...

> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> index e97a63ad2c31..6ba9d42b2537 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> @@ -304,6 +304,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

Side note but why don't we use BIT(x) for these?

>  #define INV_MPU6050_BIT_CLK_MASK            0x7
>  
> @@ -335,6 +336,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)
> @@ -451,6 +453,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,

Trivial but no comma needed on a NUM type last entry as we'll never
add anything after it.

> +};
> +
>  /* IIO attribute address */
>  enum INV_MPU6050_IIO_ATTR_ADDR {
>  	ATTR_GYRO_MATRIX,

Re: [PATCH v2 4/4] iio: imu: inv_mpu6050: add WoM suspend wakeup with low-power mode

[Jean-Baptiste Maneyrol]

Hello Jonathan,

I already issued a V3 of this patch.

I have switched to guard(mutex) in the V3, but too bad I didn't get your comments sooner.
Certainly, we could use BIT() macro, but I didn't want to change the other defines or have 1 BIT() in the middle. But do as you prefer.

Thanks for reviewing V3 of the series and for your comments,
JB

From: Jonathan Cameron <jic23@kernel.org>
Sent: Thursday, March 14, 2024 15:52
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 v2 4/4] iio: imu: inv_mpu6050: add WoM suspend wakeup with low-power mode
 
This Message Is From an External Sender
This message came from outside your organization.
 
On Fri,  8 Mar 2024 15:10:23 +0000
inv.git-commit@tdk.com wrote:

> 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
> and adjust threshold to follow the required roc value.
> 
> Signed-off-by: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>
A few comments inline, but nothing significant that needs changes.
Not sure why this didn't send the other day - just found it still open :(

Jonathan

> ---

> +
> +static int inv_mpu6050_set_wom_lp(struct inv_mpu6050_state *st, bool on)

You could just split this given almost nothing shared between the two branches.

> +{
> +	unsigned int lp_div;
> +	int result;
> +
> +	if (on) {
> +		/* set low power ODR */
> +		result = inv_mpu6050_set_lp_odr(st, INV_MPU6050_FREQ_DIVIDER(st), &lp_div);
> +		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;
> +		/* update wom threshold with new low-power frequency divider */
> +		result = inv_mpu6050_set_wom_threshold(st, st->chip_config.roc_threshold, lp_div);
> +		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 wom threshold */
> +		result = inv_mpu6050_set_wom_threshold(st, st->chip_config.roc_threshold,
> +						       INV_MPU6050_FREQ_DIVIDER(st));
> +		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_enable_wom(struct inv_mpu6050_state *st, bool en)
>  {
>  	struct device *pdev = regmap_get_device(st->map);
> @@ -1847,6 +1933,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))
> @@ -2012,16 +2099,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);
A very good case for using guard(mutex)(&st->lock); but that can be a future series.

> -	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);
> @@ -2031,7 +2129,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;
...

> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> index e97a63ad2c31..6ba9d42b2537 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> @@ -304,6 +304,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

Side note but why don't we use BIT(x) for these?

>  #define INV_MPU6050_BIT_CLK_MASK            0x7
>  
> @@ -335,6 +336,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)
> @@ -451,6 +453,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,

Trivial but no comma needed on a NUM type last entry as we'll never
add anything after it.

> +};
> +
>  /* IIO attribute address */
>  enum INV_MPU6050_IIO_ATTR_ADDR {
>  	ATTR_GYRO_MATRIX,

Re: [PATCH v2 4/4] iio: imu: inv_mpu6050: add WoM suspend wakeup with low-power mode

[Jonathan Cameron]

On Thu, 14 Mar 2024 16:44:23 +0000
Jean-Baptiste Maneyrol <Jean-Baptiste.Maneyrol@tdk.com> wrote:

> Hello Jonathan,
> 
> I already issued a V3 of this patch.
> 
> I have switched to guard(mutex) in the V3, but too bad I didn't get your comments sooner.
> Certainly, we could use BIT() macro, but I didn't want to change the other defines or have 1 BIT() in the middle. But do as you prefer.

It would need them all tidied up in a separate patch I think.

Jonathan

> 
> Thanks for reviewing V3 of the series and for your comments,
> JB
> 
> From: Jonathan Cameron <jic23@kernel.org>
> Sent: Thursday, March 14, 2024 15:52
> 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 v2 4/4] iio: imu: inv_mpu6050: add WoM suspend wakeup with low-power mode
>  
> This Message Is From an External Sender
> This message came from outside your organization.
>  
> On Fri,  8 Mar 2024 15:10:23 +0000
> inv.git-commit@tdk.com wrote:
> 
> > 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
> > and adjust threshold to follow the required roc value.
> > 
> > Signed-off-by: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>  
> A few comments inline, but nothing significant that needs changes.
> Not sure why this didn't send the other day - just found it still open :(
> 
> Jonathan
> 
> > ---  
> 
> > +
> > +static int inv_mpu6050_set_wom_lp(struct inv_mpu6050_state *st, bool on)  
> 
> You could just split this given almost nothing shared between the two branches.
> 
> > +{
> > +	unsigned int lp_div;
> > +	int result;
> > +
> > +	if (on) {
> > +		/* set low power ODR */
> > +		result = inv_mpu6050_set_lp_odr(st, INV_MPU6050_FREQ_DIVIDER(st), &lp_div);
> > +		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;
> > +		/* update wom threshold with new low-power frequency divider */
> > +		result = inv_mpu6050_set_wom_threshold(st, st->chip_config.roc_threshold, lp_div);
> > +		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 wom threshold */
> > +		result = inv_mpu6050_set_wom_threshold(st, st->chip_config.roc_threshold,
> > +						       INV_MPU6050_FREQ_DIVIDER(st));
> > +		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_enable_wom(struct inv_mpu6050_state *st, bool en)
> >  {
> >  	struct device *pdev = regmap_get_device(st->map);
> > @@ -1847,6 +1933,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))
> > @@ -2012,16 +2099,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);  
> A very good case for using guard(mutex)(&st->lock); but that can be a future series.
> 
> > -	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);
> > @@ -2031,7 +2129,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;  
> ...
> 
> > diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> > index e97a63ad2c31..6ba9d42b2537 100644
> > --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> > +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> > @@ -304,6 +304,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  
> 
> Side note but why don't we use BIT(x) for these?
> 
> >  #define INV_MPU6050_BIT_CLK_MASK            0x7
> >  
> > @@ -335,6 +336,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)
> > @@ -451,6 +453,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,  
> 
> Trivial but no comma needed on a NUM type last entry as we'll never
> add anything after it.
> 
> > +};
> > +
> >  /* IIO attribute address */
> >  enum INV_MPU6050_IIO_ATTR_ADDR {
> >  	ATTR_GYRO_MATRIX,  
>