drivers/iio/imu/bmi323/bmi323_core.c | 225 ++++++++++++++++++++++++++- 1 file changed, 223 insertions(+), 2 deletions(-)
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[] = {
+ {
+ .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);
}
--
2.46.0On 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);
> }
The bmi323 chip is part of handhelds PCs that are run on battery.
One of said PC is well-known for its short battery life, even in s2idle:
help mitigate that by putting the device in its lowest-consumption
state while the peripheral is unused.
Have runtime-pm suspend callback save used configuration registers
and runtime-pm resume callback restore saved registers to restore
the previous state.
Changelog:
- V2: patch 1:
+ change patch commit message
+ drop removal callbacks and use devm_add_action_or_reset
+ split bmi323_init in two functions
+ separate regs to save and relative value
+ drop unhelpful consts ptr modifiers
+ add a comment to explain why BMI323_FIFO_CTRL_REG is
being used in runtime resume
- V3: patch 1:
+ drop a struct array and replace with an array of
unsigned int: u8 was too small and it would have resulted
in overflow of register addresses
+ use single-line comments where possible
+ drop useless comments
+ remove intermediate variables
+ remove blank lines
Previous patches obsoleted:
https://lore.kernel.org/all/20240811161202.19818-1-benato.denis96@gmail.com
https://lore.kernel.org/all/20240818150923.20387-1-benato.denis96@gmail.com
Signed-off-by: Denis Benato <benato.denis96@gmail.com>
Denis Benato (1):
iio: bmi323: peripheral in lowest power state on suspend
drivers/iio/imu/bmi323/bmi323_core.c | 155 ++++++++++++++++++++++++++-
1 file changed, 153 insertions(+), 2 deletions(-)
--
2.46.0
On Sat, 24 Aug 2024 16:11:21 +0200 Denis Benato <benato.denis96@gmail.com> wrote: > The bmi323 chip is part of handhelds PCs that are run on battery. > > One of said PC is well-known for its short battery life, even in s2idle: > help mitigate that by putting the device in its lowest-consumption > state while the peripheral is unused. > > Have runtime-pm suspend callback save used configuration registers > and runtime-pm resume callback restore saved registers to restore > the previous state. > For future reference, don't send new versions of a patch series in reply to previous version. It's a good way to ensure your code does not get reviewed as busy maintainers and reviewers tend to start with latest threads and this style means your patch ends up way off the top of the screen! I don't know if other subsystems specifically ask for this style of reply, but the ones that I interact with all specifically ask people to not do what you have here. Jonathan > Changelog: > - V2: patch 1: > + change patch commit message > + drop removal callbacks and use devm_add_action_or_reset > + split bmi323_init in two functions > + separate regs to save and relative value > + drop unhelpful consts ptr modifiers > + add a comment to explain why BMI323_FIFO_CTRL_REG is > being used in runtime resume > - V3: patch 1: > + drop a struct array and replace with an array of > unsigned int: u8 was too small and it would have resulted > in overflow of register addresses > + use single-line comments where possible > + drop useless comments > + remove intermediate variables > + remove blank lines > > Previous patches obsoleted: > https://lore.kernel.org/all/20240811161202.19818-1-benato.denis96@gmail.com > https://lore.kernel.org/all/20240818150923.20387-1-benato.denis96@gmail.com > > Signed-off-by: Denis Benato <benato.denis96@gmail.com> > > Denis Benato (1): > iio: bmi323: peripheral in lowest power state on suspend > > drivers/iio/imu/bmi323/bmi323_core.c | 155 ++++++++++++++++++++++++++- > 1 file changed, 153 insertions(+), 2 deletions(-) >
On 26/08/24 12:41, Jonathan Cameron wrote: > On Sat, 24 Aug 2024 16:11:21 +0200 > Denis Benato <benato.denis96@gmail.com> wrote: > >> The bmi323 chip is part of handhelds PCs that are run on battery. >> >> One of said PC is well-known for its short battery life, even in s2idle: >> help mitigate that by putting the device in its lowest-consumption >> state while the peripheral is unused. >> >> Have runtime-pm suspend callback save used configuration registers >> and runtime-pm resume callback restore saved registers to restore >> the previous state. >> > For future reference, don't send new versions of a patch series > in reply to previous version. It's a good way to ensure your > code does not get reviewed as busy maintainers and reviewers > tend to start with latest threads and this style means > your patch ends up way off the top of the screen! > > I don't know if other subsystems specifically ask for this style > of reply, but the ones that I interact with all specifically ask > people to not do what you have here. > > Jonathan > Hello Jonathan, Thanks for the heads up! I didn't know and now I do. Thanks for your time, patience and guidance. Best regards, Denis >> Changelog: >> - V2: patch 1: >> + change patch commit message >> + drop removal callbacks and use devm_add_action_or_reset >> + split bmi323_init in two functions >> + separate regs to save and relative value >> + drop unhelpful consts ptr modifiers >> + add a comment to explain why BMI323_FIFO_CTRL_REG is >> being used in runtime resume >> - V3: patch 1: >> + drop a struct array and replace with an array of >> unsigned int: u8 was too small and it would have resulted >> in overflow of register addresses >> + use single-line comments where possible >> + drop useless comments >> + remove intermediate variables >> + remove blank lines >> >> Previous patches obsoleted: >> https://lore.kernel.org/all/20240811161202.19818-1-benato.denis96@gmail.com >> https://lore.kernel.org/all/20240818150923.20387-1-benato.denis96@gmail.com >> >> Signed-off-by: Denis Benato <benato.denis96@gmail.com> >> >> Denis Benato (1): >> iio: bmi323: peripheral in lowest power state on suspend >> >> drivers/iio/imu/bmi323/bmi323_core.c | 155 ++++++++++++++++++++++++++- >> 1 file changed, 153 insertions(+), 2 deletions(-) >> >
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 | 155 ++++++++++++++++++++++++++-
1 file changed, 153 insertions(+), 2 deletions(-)
diff --git a/drivers/iio/imu/bmi323/bmi323_core.c b/drivers/iio/imu/bmi323/bmi323_core.c
index 4b2b211a3e88..57be22c97cb9 100644
--- a/drivers/iio/imu/bmi323/bmi323_core.c
+++ b/drivers/iio/imu/bmi323/bmi323_core.c
@@ -118,6 +118,38 @@ static const struct bmi323_hw bmi323_hw[2] = {
},
};
+static const unsigned int bmi323_reg_savestate[] = {
+ BMI323_INT_MAP1_REG,
+ BMI323_INT_MAP2_REG,
+ BMI323_IO_INT_CTR_REG,
+ BMI323_IO_INT_CONF_REG,
+ BMI323_ACC_CONF_REG,
+ BMI323_GYRO_CONF_REG,
+ BMI323_FEAT_IO0_REG,
+ BMI323_FIFO_WTRMRK_REG,
+ BMI323_FIFO_CONF_REG
+};
+
+static const unsigned int bmi323_ext_reg_savestate[] = {
+ BMI323_GEN_SET1_REG,
+ BMI323_TAP1_REG,
+ BMI323_TAP2_REG,
+ BMI323_TAP3_REG,
+ BMI323_FEAT_IO0_S_TAP_MSK,
+ BMI323_STEP_SC1_REG,
+ BMI323_ANYMO1_REG,
+ BMI323_NOMO1_REG,
+ BMI323_ANYMO1_REG + BMI323_MO2_OFFSET,
+ BMI323_NOMO1_REG + BMI323_MO2_OFFSET,
+ BMI323_ANYMO1_REG + BMI323_MO3_OFFSET,
+ 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 +162,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 +2005,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 +2068,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 +2123,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 +2168,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 +2176,113 @@ 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;
+
+ 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++) {
+ ret = regmap_read(data->regmap, bmi323_reg_savestate[i],
+ &savestate->reg_settings[i]);
+ if (ret) {
+ dev_err(data->dev, "Error reading bmi323 reg 0x%x: %d\n",
+ bmi323_reg_savestate[i], ret);
+ return ret;
+ }
+ }
+
+ for (unsigned int i = 0; i < ARRAY_SIZE(bmi323_ext_reg_savestate); i++) {
+ ret = bmi323_read_ext_reg(data, bmi323_reg_savestate[i],
+ &savestate->reg_settings[i]);
+ if (ret) {
+ dev_err(data->dev, "Error reading bmi323 external reg 0x%x: %d\n",
+ bmi323_reg_savestate[i], 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 iio_device_suspend_triggering(indio_dev);
+ 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;
+ unsigned int val;
+ 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;
+ }
+
+ for (unsigned int i = 0; i < ARRAY_SIZE(bmi323_ext_reg_savestate); i++) {
+ ret = bmi323_write_ext_reg(data, bmi323_reg_savestate[i],
+ savestate->reg_settings[i]);
+ if (ret) {
+ dev_err(data->dev, "Error writing bmi323 external reg 0x%x: %d\n",
+ bmi323_reg_savestate[i], ret);
+ return ret;
+ }
+ }
+
+ for (unsigned int i = 0; i < ARRAY_SIZE(bmi323_reg_savestate); i++) {
+ ret = regmap_write(data->regmap, bmi323_reg_savestate[i],
+ savestate->reg_settings[i]);
+ if (ret) {
+ dev_err(data->dev, "Error writing bmi323 reg 0x%x: %d\n",
+ bmi323_reg_savestate[i], 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;
+ }
+ }
+
+ 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);
}
--
2.46.0On Sat, 24 Aug 2024 16:11:22 +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>
Applied with some whitespace tweaks.
> ---
> drivers/iio/imu/bmi323/bmi323_core.c | 155 ++++++++++++++++++++++++++-
> 1 file changed, 153 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iio/imu/bmi323/bmi323_core.c b/drivers/iio/imu/bmi323/bmi323_core.c
> index 4b2b211a3e88..57be22c97cb9 100644
> --- a/drivers/iio/imu/bmi323/bmi323_core.c
> +++ b/drivers/iio/imu/bmi323/bmi323_core.c
> @@ -118,6 +118,38 @@ static const struct bmi323_hw bmi323_hw[2] = {
> },
> };
>
> +static const unsigned int bmi323_reg_savestate[] = {
> + BMI323_INT_MAP1_REG,
> + BMI323_INT_MAP2_REG,
> + BMI323_IO_INT_CTR_REG,
> + BMI323_IO_INT_CONF_REG,
> + BMI323_ACC_CONF_REG,
> + BMI323_GYRO_CONF_REG,
> + BMI323_FEAT_IO0_REG,
> + BMI323_FIFO_WTRMRK_REG,
> + BMI323_FIFO_CONF_REG
> +};
> +
> +static const unsigned int bmi323_ext_reg_savestate[] = {
> + BMI323_GEN_SET1_REG,
> + BMI323_TAP1_REG,
> + BMI323_TAP2_REG,
> + BMI323_TAP3_REG,
> + BMI323_FEAT_IO0_S_TAP_MSK,
> + BMI323_STEP_SC1_REG,
> + BMI323_ANYMO1_REG,
> + BMI323_NOMO1_REG,
> + BMI323_ANYMO1_REG + BMI323_MO2_OFFSET,
> + BMI323_NOMO1_REG + BMI323_MO2_OFFSET,
> + BMI323_ANYMO1_REG + BMI323_MO3_OFFSET,
> + 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 +162,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 +2005,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 +2068,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 +2123,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 +2168,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 +2176,113 @@ 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;
> +
> + 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++) {
> + ret = regmap_read(data->regmap, bmi323_reg_savestate[i],
> + &savestate->reg_settings[i]);
> + if (ret) {
> + dev_err(data->dev, "Error reading bmi323 reg 0x%x: %d\n",
> + bmi323_reg_savestate[i], ret);
> + return ret;
> + }
> + }
> +
> + for (unsigned int i = 0; i < ARRAY_SIZE(bmi323_ext_reg_savestate); i++) {
> + ret = bmi323_read_ext_reg(data, bmi323_reg_savestate[i],
> + &savestate->reg_settings[i]);
> + if (ret) {
> + dev_err(data->dev, "Error reading bmi323 external reg 0x%x: %d\n",
> + bmi323_reg_savestate[i], 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 iio_device_suspend_triggering(indio_dev);
> + 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;
> + unsigned int val;
> + 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;
> + }
> +
> + for (unsigned int i = 0; i < ARRAY_SIZE(bmi323_ext_reg_savestate); i++) {
> + ret = bmi323_write_ext_reg(data, bmi323_reg_savestate[i],
> + savestate->reg_settings[i]);
> + if (ret) {
> + dev_err(data->dev, "Error writing bmi323 external reg 0x%x: %d\n",
> + bmi323_reg_savestate[i], ret);
> + return ret;
> + }
> + }
> +
> + for (unsigned int i = 0; i < ARRAY_SIZE(bmi323_reg_savestate); i++) {
> + ret = regmap_write(data->regmap, bmi323_reg_savestate[i],
> + savestate->reg_settings[i]);
> + if (ret) {
> + dev_err(data->dev, "Error writing bmi323 reg 0x%x: %d\n",
> + bmi323_reg_savestate[i], 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;
> + }
> + }
> +
> + 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);
> }
© 2016 - 2026 Red Hat, Inc.