| Message ID | 20240818150923.20387-2-benato.denis96@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [v2] iio: bmi323: peripheral in lowest power state on suspend | expand |
On Sun, 18 Aug 2024 17:09:23 +0200 Denis Benato <benato.denis96@gmail.com> wrote: > The bmi323 is mounted on some devices that are powered > by an internal battery: help in reducing system overall power drain > while the system is in s2idle or the imu driver is not loaded > by resetting it in its lowest power draining state. > > Signed-off-by: Denis Benato <benato.denis96@gmail.com> > --- > drivers/iio/imu/bmi323/bmi323_core.c | 225 ++++++++++++++++++++++++++- > 1 file changed, 223 insertions(+), 2 deletions(-) > > diff --git a/drivers/iio/imu/bmi323/bmi323_core.c b/drivers/iio/imu/bmi323/bmi323_core.c > index 4b2b211a3e88..5d383fffe083 100644 > --- a/drivers/iio/imu/bmi323/bmi323_core.c > +++ b/drivers/iio/imu/bmi323/bmi323_core.c > @@ -118,6 +118,84 @@ static const struct bmi323_hw bmi323_hw[2] = { > }, > }; > > +struct bmi323_reg_runtime_pm { > + unsigned int reg; > +}; > + > +static const struct bmi323_reg_runtime_pm bmi323_reg_savestate[] = { Not useful to have a structure. Just use an array of unsigned int or maybe even size them to match as u8 static const u8 bmi_regs_tosave[] = { BMI323_INT_MAP1_REG, ... }; > + { > + .reg = > + }, > + { > + .reg = BMI323_INT_MAP2_REG > + }, > + { > + .reg = BMI323_IO_INT_CTR_REG > + }, > + { > + .reg = BMI323_IO_INT_CONF_REG > + }, > + { > + .reg = BMI323_ACC_CONF_REG > + }, > + { > + .reg = BMI323_GYRO_CONF_REG > + }, > + { > + .reg = BMI323_FEAT_IO0_REG > + }, > + { > + .reg = BMI323_FIFO_WTRMRK_REG > + }, > + { > + .reg = BMI323_FIFO_CONF_REG > + } > +}; > + > +static const struct bmi323_reg_runtime_pm bmi323_ext_reg_savestate[] = { Similar, just use a u8 array. > + { > + .reg = BMI323_GEN_SET1_REG > + }, > + { > + .reg = BMI323_TAP1_REG > + }, > + { > + .reg = BMI323_TAP2_REG > + }, > + { > + .reg = BMI323_TAP3_REG > + }, > + { > + .reg = BMI323_FEAT_IO0_S_TAP_MSK > + }, > + { > + .reg = BMI323_STEP_SC1_REG > + }, > + { > + .reg = BMI323_ANYMO1_REG > + }, > + { > + .reg = BMI323_NOMO1_REG > + }, > + { > + .reg = BMI323_ANYMO1_REG + BMI323_MO2_OFFSET > + }, > + { > + .reg = BMI323_NOMO1_REG + BMI323_MO2_OFFSET > + }, > + { > + .reg = BMI323_ANYMO1_REG + BMI323_MO3_OFFSET > + }, > + { > + .reg = BMI323_NOMO1_REG + BMI323_MO3_OFFSET > + } > +}; > EXPORT_SYMBOL_NS_GPL(bmi323_core_probe, IIO_BMI323); > > @@ -2125,13 +2222,137 @@ EXPORT_SYMBOL_NS_GPL(bmi323_core_probe, IIO_BMI323); > static int bmi323_core_runtime_suspend(struct device *dev) > { > struct iio_dev *indio_dev = dev_get_drvdata(dev); > + struct bmi323_data *data = iio_priv(indio_dev); > + struct bmi323_regs_runtime_pm *savestate = &data->runtime_pm_status; > + > + int ret; > > - return iio_device_suspend_triggering(indio_dev); > + guard(mutex)(&data->mutex); > + > + ret = iio_device_suspend_triggering(indio_dev); > + if (ret) > + return ret; > + > + /* > + * Save registers meant to be restored by resume pm callback. Single line comment syntax fine here. > + */ > + for (unsigned int i = 0; i < ARRAY_SIZE(bmi323_reg_savestate); i++) { > + const unsigned int reg_addr = bmi323_reg_savestate[i].reg; Once this is array of u8 I'd just use it in the two places and not bother with local variable. > + unsigned int *reg_val = &savestate->reg_settings[i]; I'd just use &savestate->reg_settings[i] inline and skip the local variable. > + > + ret = regmap_read(data->regmap, reg_addr, reg_val); > + if (ret) { > + dev_err(data->dev, "Error reading bmi323 reg 0x%x: %d\n", > + reg_addr, ret); > + return ret; > + } > + } > + > + /* > + * Save external registers meant to be restored by resume pm callback. > + */ As above, single line comment fine, if anything. Fairly obvious what is going on. > + for (unsigned int i = 0; i < ARRAY_SIZE(bmi323_ext_reg_savestate); i++) { > + const unsigned int ext_reg_addr = bmi323_reg_savestate[i].reg; > + unsigned int *ext_reg_val = &savestate->reg_settings[i]; Likewise, local variable doesn't add thing much. > + > + ret = bmi323_read_ext_reg(data, ext_reg_addr, ext_reg_val); > + if (ret) { > + dev_err(data->dev, "Error reading bmi323 external reg 0x%x: %d\n", > + ext_reg_addr, ret); > + return ret; > + } > + } > + > + /* > + * Perform soft reset to place the device in its lowest power state. > + */ > + ret = regmap_write(data->regmap, BMI323_CMD_REG, BMI323_RST_VAL); > + if (ret) > + return ret; > + > + return 0; > } > > static int bmi323_core_runtime_resume(struct device *dev) > { > struct iio_dev *indio_dev = dev_get_drvdata(dev); > + struct bmi323_data *data = iio_priv(indio_dev); > + struct bmi323_regs_runtime_pm *savestate = &data->runtime_pm_status; > + No blank line here. Doesn't add any value. > + int ret; > + > + guard(mutex)(&data->mutex); > + > + /* > + * Perform the device power-on and initial setup once again > + * after being reset in the lower power state by runtime-pm. > + */ > + ret = bmi323_init(data); > + if (!ret) > + return ret; > + > + /* Register must be cleared before changing an active config */ > + ret = regmap_write(data->regmap, BMI323_FEAT_IO0_REG, 0); > + if (ret) { > + dev_err(data->dev, "Error stopping feature engine\n"); > + return ret; > + } > + > + /* > + * Restore external registers saved by suspend pm callback. Single line comment > + */ > + for (unsigned int i = 0; i < ARRAY_SIZE(bmi323_ext_reg_savestate); i++) { > + const unsigned int ext_reg_addr = bmi323_reg_savestate[i].reg; > + const unsigned int ext_reg_val = savestate->reg_settings[i]; comments above apply here as well. > + > + ret = bmi323_write_ext_reg(data, ext_reg_addr, ext_reg_val); > + if (ret) { > + dev_err(data->dev, "Error writing bmi323 external reg 0x%x: %d\n", > + ext_reg_addr, ret); > + return ret; > + } > + } > + > + /* > + * Restore registers saved by suspend pm callback. > + */ > + for (unsigned int i = 0; i < ARRAY_SIZE(bmi323_reg_savestate); i++) { > + const unsigned int reg_addr = bmi323_reg_savestate[i].reg; > + const unsigned int reg_val = savestate->reg_settings[i]; > + > + ret = regmap_write(data->regmap, reg_addr, reg_val); > + if (ret) { > + dev_err(data->dev, "Error writing bmi323 reg 0x%x: %d\n", > + reg_addr, ret); > + return ret; > + } > + } > + > + /* > + * Clear old FIFO samples that might be generated before suspend > + * or generated from a peripheral state not equal to the saved one. > + */ > + if (data->state == BMI323_BUFFER_FIFO) { > + ret = regmap_write(data->regmap, BMI323_FIFO_CTRL_REG, > + BMI323_FIFO_FLUSH_MSK); > + if (ret) { > + dev_err(data->dev, "Error flushing FIFO buffer: %d\n", ret); > + return ret; > + } > + } > + > + unsigned int val; Variable declarations still in old school c style, so at the top of the scope. > + > + ret = regmap_read(data->regmap, BMI323_ERR_REG, &val); > + if (ret) { > + dev_err(data->dev, "Error reading bmi323 error register: %d\n", ret); > + return ret; > + } > + > + if (val) { > + dev_err(data->dev, "Sensor power error in PM = 0x%x\n", val); > + return -EINVAL; > + } > > return iio_device_resume_triggering(indio_dev); > }
diff --git a/drivers/iio/imu/bmi323/bmi323_core.c b/drivers/iio/imu/bmi323/bmi323_core.c index 4b2b211a3e88..5d383fffe083 100644 --- a/drivers/iio/imu/bmi323/bmi323_core.c +++ b/drivers/iio/imu/bmi323/bmi323_core.c @@ -118,6 +118,84 @@ static const struct bmi323_hw bmi323_hw[2] = { }, }; +struct bmi323_reg_runtime_pm { + unsigned int reg; +}; + +static const struct bmi323_reg_runtime_pm bmi323_reg_savestate[] = { + { + .reg = BMI323_INT_MAP1_REG + }, + { + .reg = BMI323_INT_MAP2_REG + }, + { + .reg = BMI323_IO_INT_CTR_REG + }, + { + .reg = BMI323_IO_INT_CONF_REG + }, + { + .reg = BMI323_ACC_CONF_REG + }, + { + .reg = BMI323_GYRO_CONF_REG + }, + { + .reg = BMI323_FEAT_IO0_REG + }, + { + .reg = BMI323_FIFO_WTRMRK_REG + }, + { + .reg = BMI323_FIFO_CONF_REG + } +}; + +static const struct bmi323_reg_runtime_pm bmi323_ext_reg_savestate[] = { + { + .reg = BMI323_GEN_SET1_REG + }, + { + .reg = BMI323_TAP1_REG + }, + { + .reg = BMI323_TAP2_REG + }, + { + .reg = BMI323_TAP3_REG + }, + { + .reg = BMI323_FEAT_IO0_S_TAP_MSK + }, + { + .reg = BMI323_STEP_SC1_REG + }, + { + .reg = BMI323_ANYMO1_REG + }, + { + .reg = BMI323_NOMO1_REG + }, + { + .reg = BMI323_ANYMO1_REG + BMI323_MO2_OFFSET + }, + { + .reg = BMI323_NOMO1_REG + BMI323_MO2_OFFSET + }, + { + .reg = BMI323_ANYMO1_REG + BMI323_MO3_OFFSET + }, + { + .reg = BMI323_NOMO1_REG + BMI323_MO3_OFFSET + } +}; + +struct bmi323_regs_runtime_pm { + unsigned int reg_settings[ARRAY_SIZE(bmi323_reg_savestate)]; + unsigned int ext_reg_settings[ARRAY_SIZE(bmi323_ext_reg_savestate)]; +}; + struct bmi323_data { struct device *dev; struct regmap *regmap; @@ -130,6 +208,7 @@ struct bmi323_data { u32 odrns[BMI323_SENSORS_CNT]; u32 odrhz[BMI323_SENSORS_CNT]; unsigned int feature_events; + struct bmi323_regs_runtime_pm runtime_pm_status; /* * Lock to protect the members of device's private data from concurrent @@ -1972,6 +2051,11 @@ static void bmi323_disable(void *data_ptr) bmi323_set_mode(data, BMI323_ACCEL, ACC_GYRO_MODE_DISABLE); bmi323_set_mode(data, BMI323_GYRO, ACC_GYRO_MODE_DISABLE); + + /* + * Place the peripheral in its lowest power consuming state. + */ + regmap_write(data->regmap, BMI323_CMD_REG, BMI323_RST_VAL); } static int bmi323_set_bw(struct bmi323_data *data, @@ -2030,6 +2114,13 @@ static int bmi323_init(struct bmi323_data *data) return dev_err_probe(data->dev, -EINVAL, "Sensor power error = 0x%x\n", val); + return 0; +} + +static int bmi323_init_reset(struct bmi323_data *data) +{ + int ret; + /* * Set the Bandwidth coefficient which defines the 3 dB cutoff * frequency in relation to the ODR. @@ -2078,12 +2169,18 @@ int bmi323_core_probe(struct device *dev) data = iio_priv(indio_dev); data->dev = dev; data->regmap = regmap; + data->irq_pin = BMI323_IRQ_DISABLED; + data->state = BMI323_IDLE; mutex_init(&data->mutex); ret = bmi323_init(data); if (ret) return -EINVAL; + ret = bmi323_init_reset(data); + if (ret) + return -EINVAL; + if (!iio_read_acpi_mount_matrix(dev, &data->orientation, "ROTM")) { ret = iio_read_mount_matrix(dev, &data->orientation); if (ret) @@ -2117,7 +2214,7 @@ int bmi323_core_probe(struct device *dev) return dev_err_probe(data->dev, ret, "Unable to register iio device\n"); - return 0; + return bmi323_fifo_disable(data); } EXPORT_SYMBOL_NS_GPL(bmi323_core_probe, IIO_BMI323); @@ -2125,13 +2222,137 @@ EXPORT_SYMBOL_NS_GPL(bmi323_core_probe, IIO_BMI323); static int bmi323_core_runtime_suspend(struct device *dev) { struct iio_dev *indio_dev = dev_get_drvdata(dev); + struct bmi323_data *data = iio_priv(indio_dev); + struct bmi323_regs_runtime_pm *savestate = &data->runtime_pm_status; + + int ret; - return iio_device_suspend_triggering(indio_dev); + guard(mutex)(&data->mutex); + + ret = iio_device_suspend_triggering(indio_dev); + if (ret) + return ret; + + /* + * Save registers meant to be restored by resume pm callback. + */ + for (unsigned int i = 0; i < ARRAY_SIZE(bmi323_reg_savestate); i++) { + const unsigned int reg_addr = bmi323_reg_savestate[i].reg; + unsigned int *reg_val = &savestate->reg_settings[i]; + + ret = regmap_read(data->regmap, reg_addr, reg_val); + if (ret) { + dev_err(data->dev, "Error reading bmi323 reg 0x%x: %d\n", + reg_addr, ret); + return ret; + } + } + + /* + * Save external registers meant to be restored by resume pm callback. + */ + for (unsigned int i = 0; i < ARRAY_SIZE(bmi323_ext_reg_savestate); i++) { + const unsigned int ext_reg_addr = bmi323_reg_savestate[i].reg; + unsigned int *ext_reg_val = &savestate->reg_settings[i]; + + ret = bmi323_read_ext_reg(data, ext_reg_addr, ext_reg_val); + if (ret) { + dev_err(data->dev, "Error reading bmi323 external reg 0x%x: %d\n", + ext_reg_addr, ret); + return ret; + } + } + + /* + * Perform soft reset to place the device in its lowest power state. + */ + ret = regmap_write(data->regmap, BMI323_CMD_REG, BMI323_RST_VAL); + if (ret) + return ret; + + return 0; } static int bmi323_core_runtime_resume(struct device *dev) { struct iio_dev *indio_dev = dev_get_drvdata(dev); + struct bmi323_data *data = iio_priv(indio_dev); + struct bmi323_regs_runtime_pm *savestate = &data->runtime_pm_status; + + int ret; + + guard(mutex)(&data->mutex); + + /* + * Perform the device power-on and initial setup once again + * after being reset in the lower power state by runtime-pm. + */ + ret = bmi323_init(data); + if (!ret) + return ret; + + /* Register must be cleared before changing an active config */ + ret = regmap_write(data->regmap, BMI323_FEAT_IO0_REG, 0); + if (ret) { + dev_err(data->dev, "Error stopping feature engine\n"); + return ret; + } + + /* + * Restore external registers saved by suspend pm callback. + */ + for (unsigned int i = 0; i < ARRAY_SIZE(bmi323_ext_reg_savestate); i++) { + const unsigned int ext_reg_addr = bmi323_reg_savestate[i].reg; + const unsigned int ext_reg_val = savestate->reg_settings[i]; + + ret = bmi323_write_ext_reg(data, ext_reg_addr, ext_reg_val); + if (ret) { + dev_err(data->dev, "Error writing bmi323 external reg 0x%x: %d\n", + ext_reg_addr, ret); + return ret; + } + } + + /* + * Restore registers saved by suspend pm callback. + */ + for (unsigned int i = 0; i < ARRAY_SIZE(bmi323_reg_savestate); i++) { + const unsigned int reg_addr = bmi323_reg_savestate[i].reg; + const unsigned int reg_val = savestate->reg_settings[i]; + + ret = regmap_write(data->regmap, reg_addr, reg_val); + if (ret) { + dev_err(data->dev, "Error writing bmi323 reg 0x%x: %d\n", + reg_addr, ret); + return ret; + } + } + + /* + * Clear old FIFO samples that might be generated before suspend + * or generated from a peripheral state not equal to the saved one. + */ + if (data->state == BMI323_BUFFER_FIFO) { + ret = regmap_write(data->regmap, BMI323_FIFO_CTRL_REG, + BMI323_FIFO_FLUSH_MSK); + if (ret) { + dev_err(data->dev, "Error flushing FIFO buffer: %d\n", ret); + return ret; + } + } + + unsigned int val; + + ret = regmap_read(data->regmap, BMI323_ERR_REG, &val); + if (ret) { + dev_err(data->dev, "Error reading bmi323 error register: %d\n", ret); + return ret; + } + + if (val) { + dev_err(data->dev, "Sensor power error in PM = 0x%x\n", val); + return -EINVAL; + } return iio_device_resume_triggering(indio_dev); }
The bmi323 is mounted on some devices that are powered by an internal battery: help in reducing system overall power drain while the system is in s2idle or the imu driver is not loaded by resetting it in its lowest power draining state. Signed-off-by: Denis Benato <benato.denis96@gmail.com> --- drivers/iio/imu/bmi323/bmi323_core.c | 225 ++++++++++++++++++++++++++- 1 file changed, 223 insertions(+), 2 deletions(-)