* [PATCH v2 1/4] iio: imu: inv_mpu6050: add WoM (Wake-on-Motion) sensor
2024-03-08 15:10 [PATCH v2 0/4] Add WoM feature as an IIO event inv.git-commit
@ 2024-03-08 15:10 ` inv.git-commit
2024-03-10 14:59 ` Jonathan Cameron
2024-03-08 15:10 ` [PATCH v2 2/4] iio: imu: inv_mpu6050: add WoM event as accel event inv.git-commit
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: inv.git-commit @ 2024-03-08 15:10 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 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
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v2 1/4] iio: imu: inv_mpu6050: add WoM (Wake-on-Motion) sensor
2024-03-08 15:10 ` [PATCH v2 1/4] iio: imu: inv_mpu6050: add WoM (Wake-on-Motion) sensor inv.git-commit
@ 2024-03-10 14:59 ` Jonathan Cameron
0 siblings, 0 replies; 10+ messages in thread
From: Jonathan Cameron @ 2024-03-10 14:59 UTC (permalink / raw)
To: inv.git-commit; +Cc: lars, linux-iio, Jean-Baptiste Maneyrol
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?
> };
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 2/4] iio: imu: inv_mpu6050: add WoM event as accel event
2024-03-08 15:10 [PATCH v2 0/4] Add WoM feature as an IIO event inv.git-commit
2024-03-08 15:10 ` [PATCH v2 1/4] iio: imu: inv_mpu6050: add WoM (Wake-on-Motion) sensor inv.git-commit
@ 2024-03-08 15:10 ` inv.git-commit
2024-03-10 15:01 ` Jonathan Cameron
2024-03-08 15:10 ` [PATCH v2 3/4] iio: imu: inv_mpu6050: add new interrupt handler for WoM events inv.git-commit
2024-03-08 15:10 ` [PATCH v2 4/4] iio: imu: inv_mpu6050: add WoM suspend wakeup with low-power mode inv.git-commit
3 siblings, 1 reply; 10+ messages in thread
From: inv.git-commit @ 2024-03-08 15:10 UTC (permalink / raw)
To: jic23; +Cc: lars, linux-iio, Jean-Baptiste Maneyrol
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
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v2 3/4] iio: imu: inv_mpu6050: add new interrupt handler for WoM events
2024-03-08 15:10 [PATCH v2 0/4] Add WoM feature as an IIO event inv.git-commit
2024-03-08 15:10 ` [PATCH v2 1/4] iio: imu: inv_mpu6050: add WoM (Wake-on-Motion) sensor inv.git-commit
2024-03-08 15:10 ` [PATCH v2 2/4] iio: imu: inv_mpu6050: add WoM event as accel event inv.git-commit
@ 2024-03-08 15:10 ` inv.git-commit
2024-03-08 15:10 ` [PATCH v2 4/4] iio: imu: inv_mpu6050: add WoM suspend wakeup with low-power mode inv.git-commit
3 siblings, 0 replies; 10+ messages in thread
From: inv.git-commit @ 2024-03-08 15:10 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 | 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
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v2 4/4] iio: imu: inv_mpu6050: add WoM suspend wakeup with low-power mode
2024-03-08 15:10 [PATCH v2 0/4] Add WoM feature as an IIO event inv.git-commit
` (2 preceding siblings ...)
2024-03-08 15:10 ` [PATCH v2 3/4] iio: imu: inv_mpu6050: add new interrupt handler for WoM events inv.git-commit
@ 2024-03-08 15:10 ` inv.git-commit
2024-03-14 14:52 ` Jonathan Cameron
3 siblings, 1 reply; 10+ messages in thread
From: inv.git-commit @ 2024-03-08 15:10 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
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
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v2 4/4] iio: imu: inv_mpu6050: add WoM suspend wakeup with low-power mode
2024-03-08 15:10 ` [PATCH v2 4/4] iio: imu: inv_mpu6050: add WoM suspend wakeup with low-power mode inv.git-commit
@ 2024-03-14 14:52 ` Jonathan Cameron
2024-03-14 16:44 ` Jean-Baptiste Maneyrol
0 siblings, 1 reply; 10+ messages in thread
From: Jonathan Cameron @ 2024-03-14 14:52 UTC (permalink / raw)
To: inv.git-commit; +Cc: lars, linux-iio, Jean-Baptiste Maneyrol
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,
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v2 4/4] iio: imu: inv_mpu6050: add WoM suspend wakeup with low-power mode
2024-03-14 14:52 ` Jonathan Cameron
@ 2024-03-14 16:44 ` Jean-Baptiste Maneyrol
2024-03-16 13:27 ` Jonathan Cameron
0 siblings, 1 reply; 10+ messages in thread
From: Jean-Baptiste Maneyrol @ 2024-03-14 16:44 UTC (permalink / raw)
To: Jonathan Cameron; +Cc: lars@metafoo.de, linux-iio@vger.kernel.org
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,
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v2 4/4] iio: imu: inv_mpu6050: add WoM suspend wakeup with low-power mode
2024-03-14 16:44 ` Jean-Baptiste Maneyrol
@ 2024-03-16 13:27 ` Jonathan Cameron
0 siblings, 0 replies; 10+ messages in thread
From: Jonathan Cameron @ 2024-03-16 13:27 UTC (permalink / raw)
To: Jean-Baptiste Maneyrol; +Cc: lars@metafoo.de, linux-iio@vger.kernel.org
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,
>
^ permalink raw reply [flat|nested] 10+ messages in thread