| Message ID | 20240308151023.379705-5-inv.git-commit@tdk.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Add WoM feature as an IIO event | expand |
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,
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,
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, >
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,