[PATCH 0/1] iio: bmi323: have the peripheral consume less power

[PATCH 0/1] iio: bmi323: have the peripheral consume less power

[Denis Benato]

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.

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.h      |   1 +
 drivers/iio/imu/bmi323/bmi323_core.c | 183 ++++++++++++++++++++++++++-
 drivers/iio/imu/bmi323/bmi323_i2c.c  |   8 ++
 drivers/iio/imu/bmi323/bmi323_spi.c  |   8 ++
 4 files changed, 194 insertions(+), 6 deletions(-)

-- 
2.46.0

[PATCH 1/1] iio: bmi323: peripheral in lowest power state on suspend

[Denis Benato]

The bmi323 is mounted on some devices that are powered
by an internal battery: help in reducing system overall power drain
while the imu is not in use by resetting it in its lowest power
draining state.

Signed-off-by: Denis Benato <benato.denis96@gmail.com>
---
 drivers/iio/imu/bmi323/bmi323.h      |   1 +
 drivers/iio/imu/bmi323/bmi323_core.c | 183 ++++++++++++++++++++++++++-
 drivers/iio/imu/bmi323/bmi323_i2c.c  |   8 ++
 drivers/iio/imu/bmi323/bmi323_spi.c  |   8 ++
 4 files changed, 194 insertions(+), 6 deletions(-)

diff --git a/drivers/iio/imu/bmi323/bmi323.h b/drivers/iio/imu/bmi323/bmi323.h
index 209bccb1f335..9fd3c5db7520 100644
--- a/drivers/iio/imu/bmi323/bmi323.h
+++ b/drivers/iio/imu/bmi323/bmi323.h
@@ -204,6 +204,7 @@
 
 struct device;
 int bmi323_core_probe(struct device *dev);
+void bmi323_core_remove(struct device *dev);
 extern const struct regmap_config bmi323_regmap_config;
 extern const struct dev_pm_ops bmi323_core_pm_ops;
 
diff --git a/drivers/iio/imu/bmi323/bmi323_core.c b/drivers/iio/imu/bmi323/bmi323_core.c
index 4b2b211a3e88..edb9ce4e66a0 100644
--- a/drivers/iio/imu/bmi323/bmi323_core.c
+++ b/drivers/iio/imu/bmi323/bmi323_core.c
@@ -118,6 +118,24 @@ static const struct bmi323_hw bmi323_hw[2] = {
 	},
 };
 
+struct bmi323_ext_regs_settings {
+	unsigned int reg;
+	unsigned int val;
+};
+
+struct bmi323_regs_settings {
+	unsigned int reg;
+	unsigned int val;
+};
+
+#define EXT_SETTING_REGISTERS 12
+#define SETTING_REGISTERS 9
+
+struct bmi323_regs_runtime_pm {
+	struct bmi323_regs_settings reg_settings[SETTING_REGISTERS];
+	struct bmi323_ext_regs_settings ext_settings[EXT_SETTING_REGISTERS];
+};
+
 struct bmi323_data {
 	struct device *dev;
 	struct regmap *regmap;
@@ -130,6 +148,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
@@ -1982,7 +2001,7 @@ static int bmi323_set_bw(struct bmi323_data *data,
 				  FIELD_PREP(BMI323_ACC_GYRO_CONF_BW_MSK, bw));
 }
 
-static int bmi323_init(struct bmi323_data *data)
+static int bmi323_init(struct bmi323_data *data, bool first_init)
 {
 	int ret, val;
 
@@ -2030,6 +2049,9 @@ static int bmi323_init(struct bmi323_data *data)
 		return dev_err_probe(data->dev, -EINVAL,
 				     "Sensor power error = 0x%x\n", val);
 
+	if (!first_init)
+		return 0;
+
 	/*
 	 * Set the Bandwidth coefficient which defines the 3 dB cutoff
 	 * frequency in relation to the ODR.
@@ -2078,9 +2100,32 @@ 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;
+	data->runtime_pm_status.reg_settings[0].reg = BMI323_INT_MAP1_REG;
+	data->runtime_pm_status.reg_settings[1].reg = BMI323_INT_MAP2_REG;
+	data->runtime_pm_status.reg_settings[2].reg = BMI323_IO_INT_CTR_REG;
+	data->runtime_pm_status.reg_settings[3].reg = BMI323_IO_INT_CONF_REG;
+	data->runtime_pm_status.reg_settings[4].reg = BMI323_ACC_CONF_REG;
+	data->runtime_pm_status.reg_settings[5].reg = BMI323_GYRO_CONF_REG;
+	data->runtime_pm_status.reg_settings[6].reg = BMI323_FEAT_IO0_REG;
+	data->runtime_pm_status.reg_settings[7].reg = BMI323_FIFO_WTRMRK_REG;
+	data->runtime_pm_status.reg_settings[8].reg = BMI323_FIFO_CONF_REG;
+	data->runtime_pm_status.ext_settings[0].reg = BMI323_GEN_SET1_REG;
+	data->runtime_pm_status.ext_settings[1].reg = BMI323_TAP1_REG;
+	data->runtime_pm_status.ext_settings[2].reg = BMI323_TAP2_REG;
+	data->runtime_pm_status.ext_settings[3].reg = BMI323_TAP3_REG;
+	data->runtime_pm_status.ext_settings[4].reg = BMI323_FEAT_IO0_S_TAP_MSK;
+	data->runtime_pm_status.ext_settings[5].reg = BMI323_STEP_SC1_REG;
+	data->runtime_pm_status.ext_settings[6].reg = BMI323_ANYMO1_REG;
+	data->runtime_pm_status.ext_settings[7].reg = BMI323_NOMO1_REG;
+	data->runtime_pm_status.ext_settings[8].reg = BMI323_ANYMO1_REG + BMI323_MO2_OFFSET;
+	data->runtime_pm_status.ext_settings[9].reg = BMI323_NOMO1_REG + BMI323_MO2_OFFSET;
+	data->runtime_pm_status.ext_settings[10].reg = BMI323_ANYMO1_REG + BMI323_MO3_OFFSET;
+	data->runtime_pm_status.ext_settings[11].reg = BMI323_NOMO1_REG + BMI323_MO3_OFFSET;
 	mutex_init(&data->mutex);
 
-	ret = bmi323_init(data);
+	ret = bmi323_init(data, true);
 	if (ret)
 		return -EINVAL;
 
@@ -2117,21 +2162,147 @@ 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);
 
+void bmi323_core_remove(struct device *dev)
+{
+	struct regmap *const regmap = dev_get_regmap(dev, NULL);
+
+	/*
+	 * Place the peripheral in its lowest power consuming state.
+	 */
+	if (regmap)
+		regmap_write(regmap, BMI323_CMD_REG, BMI323_RST_VAL);
+}
+EXPORT_SYMBOL_NS_GPL(bmi323_core_remove, IIO_BMI323);
+
 #if defined(CONFIG_PM)
 static int bmi323_core_runtime_suspend(struct device *dev)
 {
-	struct iio_dev *indio_dev = dev_get_drvdata(dev);
+	struct iio_dev *const indio_dev = dev_get_drvdata(dev);
+	struct bmi323_data *const data = iio_priv(indio_dev);
+
+	int ret = 0;
+
+	guard(mutex)(&data->mutex);
 
-	return iio_device_suspend_triggering(indio_dev);
+	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 < SETTING_REGISTERS; ++i) {
+		ret = regmap_read(data->regmap,
+			  data->runtime_pm_status.reg_settings[i].reg,
+			  &data->runtime_pm_status.reg_settings[i].val);
+		if (ret) {
+			dev_err(data->dev, "Error reading bmi323 reg 0x%x: %d\n",
+				  data->runtime_pm_status.ext_settings[i].reg, ret);
+			return ret;
+		}
+	}
+
+	/*
+	 * Save external registers meant to be restored by resume pm callback.
+	 */
+	for (unsigned int i = 0; i < EXT_SETTING_REGISTERS; ++i) {
+		ret = bmi323_read_ext_reg(data,
+			  data->runtime_pm_status.ext_settings[i].reg,
+			  &data->runtime_pm_status.ext_settings[i].val);
+		if (ret) {
+			dev_err(data->dev, "Error reading bmi323 external reg 0x%x: %d\n",
+				  data->runtime_pm_status.ext_settings[i].reg, 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 iio_dev *const indio_dev = dev_get_drvdata(dev);
+	struct bmi323_data *const data = iio_priv(indio_dev);
+
+	int ret = 0;
+
+	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, false);
+	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 < EXT_SETTING_REGISTERS; ++i) {
+		ret = bmi323_write_ext_reg(data,
+			data->runtime_pm_status.ext_settings[i].reg,
+			data->runtime_pm_status.ext_settings[i].val);
+		if (ret) {
+			dev_err(data->dev, "Error writing bmi323 external reg 0x%x: %d\n",
+				data->runtime_pm_status.ext_settings[i].reg, ret);
+			return ret;
+		}
+	}
+
+	/*
+	 * Restore registers saved by suspend pm callback.
+	 */
+	for (unsigned int i = 0; i < SETTING_REGISTERS; ++i) {
+		ret = regmap_write(data->regmap,
+			data->runtime_pm_status.reg_settings[i].reg,
+			data->runtime_pm_status.reg_settings[i].val);
+		if (ret) {
+			dev_err(data->dev, "Error writing bmi323 reg 0x%x: %d\n",
+				data->runtime_pm_status.reg_settings[i].reg, ret);
+			return ret;
+		}
+	}
+
+	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);
 }
diff --git a/drivers/iio/imu/bmi323/bmi323_i2c.c b/drivers/iio/imu/bmi323/bmi323_i2c.c
index 0ba5d69d8329..2d93861281eb 100644
--- a/drivers/iio/imu/bmi323/bmi323_i2c.c
+++ b/drivers/iio/imu/bmi323/bmi323_i2c.c
@@ -93,6 +93,13 @@ static int bmi323_i2c_probe(struct i2c_client *i2c)
 	return bmi323_core_probe(dev);
 }
 
+static void bmi323_i2c_remove(struct i2c_client *i2c)
+{
+	struct device *const dev = &i2c->dev;
+
+	bmi323_core_remove(dev);
+}
+
 static const struct acpi_device_id bmi323_acpi_match[] = {
 	/*
 	 * The "BOSC0200" identifier used here is not unique to bmi323 devices.
@@ -133,6 +140,7 @@ static struct i2c_driver bmi323_i2c_driver = {
 		.acpi_match_table = bmi323_acpi_match,
 	},
 	.probe = bmi323_i2c_probe,
+	.remove = bmi323_i2c_remove,
 	.id_table = bmi323_i2c_ids,
 };
 module_i2c_driver(bmi323_i2c_driver);
diff --git a/drivers/iio/imu/bmi323/bmi323_spi.c b/drivers/iio/imu/bmi323/bmi323_spi.c
index 9de3ade78d71..f42c49c471c1 100644
--- a/drivers/iio/imu/bmi323/bmi323_spi.c
+++ b/drivers/iio/imu/bmi323/bmi323_spi.c
@@ -64,6 +64,13 @@ static int bmi323_spi_probe(struct spi_device *spi)
 	return bmi323_core_probe(dev);
 }
 
+static void bmi323_spi_remove(struct spi_device *spi)
+{
+	struct device *const dev = &spi->dev;
+
+	bmi323_core_remove(dev);
+}
+
 static const struct spi_device_id bmi323_spi_ids[] = {
 	{ "bmi323" },
 	{ }
@@ -83,6 +90,7 @@ static struct spi_driver bmi323_spi_driver = {
 		.of_match_table = bmi323_of_spi_match,
 	},
 	.probe = bmi323_spi_probe,
+	.remove = bmi323_spi_remove,
 	.id_table = bmi323_spi_ids,
 };
 module_spi_driver(bmi323_spi_driver);
-- 
2.46.0

Re: [PATCH 1/1] iio: bmi323: peripheral in lowest power state on suspend

[Jonathan Cameron]

On Sun, 11 Aug 2024 18:12:02 +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 imu is not in use by resetting it in its lowest power
> draining state.
> 
> Signed-off-by: Denis Benato <benato.denis96@gmail.com>
Hi Denis.

This is rather an expensive path for runtime PM. 

Maybe mention this still only applies in s2idle.
The driver doesn't have a more sophisticated runtime pm to
bring the power down when it's simply not being used:
autosuspend etc which would need a lot more infrastructure
as we need to add when the device must be resumed + when we
are done with talking to it.

Jonathan


> ---
>  drivers/iio/imu/bmi323/bmi323.h      |   1 +
>  drivers/iio/imu/bmi323/bmi323_core.c | 183 ++++++++++++++++++++++++++-
>  drivers/iio/imu/bmi323/bmi323_i2c.c  |   8 ++
>  drivers/iio/imu/bmi323/bmi323_spi.c  |   8 ++
>  4 files changed, 194 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/iio/imu/bmi323/bmi323.h b/drivers/iio/imu/bmi323/bmi323.h
> index 209bccb1f335..9fd3c5db7520 100644
> --- a/drivers/iio/imu/bmi323/bmi323.h
> +++ b/drivers/iio/imu/bmi323/bmi323.h
> @@ -204,6 +204,7 @@
>  
>  struct device;
>  int bmi323_core_probe(struct device *dev);
> +void bmi323_core_remove(struct device *dev);
>  extern const struct regmap_config bmi323_regmap_config;
>  extern const struct dev_pm_ops bmi323_core_pm_ops;
>  
> diff --git a/drivers/iio/imu/bmi323/bmi323_core.c b/drivers/iio/imu/bmi323/bmi323_core.c
> index 4b2b211a3e88..edb9ce4e66a0 100644
> --- a/drivers/iio/imu/bmi323/bmi323_core.c
> +++ b/drivers/iio/imu/bmi323/bmi323_core.c
> @@ -118,6 +118,24 @@ static const struct bmi323_hw bmi323_hw[2] = {
>  	},
>  };
>  
> +struct bmi323_ext_regs_settings {
> +	unsigned int reg;
> +	unsigned int val;
> +};
> +
> +struct bmi323_regs_settings {
> +	unsigned int reg;
> +	unsigned int val;
> +};

Why are two types useful?

> +
> +#define EXT_SETTING_REGISTERS 12
> +#define SETTING_REGISTERS 9
> +
> +struct bmi323_regs_runtime_pm {
> +	struct bmi323_regs_settings reg_settings[SETTING_REGISTERS];
> +	struct bmi323_ext_regs_settings ext_settings[EXT_SETTING_REGISTERS];

As below. Break these part so that the register addresses are just const data
and the values are all that is stored in here. 

> +};
> +
>  struct bmi323_data {
>  	struct device *dev;
>  	struct regmap *regmap;
> @@ -130,6 +148,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
> @@ -1982,7 +2001,7 @@ static int bmi323_set_bw(struct bmi323_data *data,
>  				  FIELD_PREP(BMI323_ACC_GYRO_CONF_BW_MSK, bw));
>  }
>  
> -static int bmi323_init(struct bmi323_data *data)
> +static int bmi323_init(struct bmi323_data *data, bool first_init)
>  {
>  	int ret, val;
>  
> @@ -2030,6 +2049,9 @@ static int bmi323_init(struct bmi323_data *data)
>  		return dev_err_probe(data->dev, -EINVAL,
>  				     "Sensor power error = 0x%x\n", val);
>  

Maybe better to split the function into two parts and call both in probe() 
but not in resume()

> +	if (!first_init)
> +		return 0;
> +
>  	/*
>  	 * Set the Bandwidth coefficient which defines the 3 dB cutoff
>  	 * frequency in relation to the ODR.
> @@ -2078,9 +2100,32 @@ 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;
> +	data->runtime_pm_status.reg_settings[0].reg = BMI323_INT_MAP1_REG;

Use a local pointer to reg_settings / ext_settings to shorten all these a lot.
Or better still separate the register addresses into a const arrays with only
the values (in a 1 D array) being changed by code.

> +	data->runtime_pm_status.reg_settings[1].reg = BMI323_INT_MAP2_REG;
> +	data->runtime_pm_status.reg_settings[2].reg = BMI323_IO_INT_CTR_REG;
> +	data->runtime_pm_status.reg_settings[3].reg = BMI323_IO_INT_CONF_REG;
> +	data->runtime_pm_status.reg_settings[4].reg = BMI323_ACC_CONF_REG;
> +	data->runtime_pm_status.reg_settings[5].reg = BMI323_GYRO_CONF_REG;
> +	data->runtime_pm_status.reg_settings[6].reg = BMI323_FEAT_IO0_REG;
> +	data->runtime_pm_status.reg_settings[7].reg = BMI323_FIFO_WTRMRK_REG;
> +	data->runtime_pm_status.reg_settings[8].reg = BMI323_FIFO_CONF_REG;
> +	data->runtime_pm_status.ext_settings[0].reg = BMI323_GEN_SET1_REG;
> +	data->runtime_pm_status.ext_settings[1].reg = BMI323_TAP1_REG;
> +	data->runtime_pm_status.ext_settings[2].reg = BMI323_TAP2_REG;
> +	data->runtime_pm_status.ext_settings[3].reg = BMI323_TAP3_REG;
> +	data->runtime_pm_status.ext_settings[4].reg = BMI323_FEAT_IO0_S_TAP_MSK;
> +	data->runtime_pm_status.ext_settings[5].reg = BMI323_STEP_SC1_REG;
> +	data->runtime_pm_status.ext_settings[6].reg = BMI323_ANYMO1_REG;
> +	data->runtime_pm_status.ext_settings[7].reg = BMI323_NOMO1_REG;
> +	data->runtime_pm_status.ext_settings[8].reg = BMI323_ANYMO1_REG + BMI323_MO2_OFFSET;
> +	data->runtime_pm_status.ext_settings[9].reg = BMI323_NOMO1_REG + BMI323_MO2_OFFSET;
> +	data->runtime_pm_status.ext_settings[10].reg = BMI323_ANYMO1_REG + BMI323_MO3_OFFSET;
> +	data->runtime_pm_status.ext_settings[11].reg = BMI323_NOMO1_REG + BMI323_MO3_OFFSET;
>  	mutex_init(&data->mutex);
>  
> -	ret = bmi323_init(data);
> +	ret = bmi323_init(data, true);
>  	if (ret)
>  		return -EINVAL;
>  
> @@ -2117,21 +2162,147 @@ 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);
>  
> +void bmi323_core_remove(struct device *dev)
> +{
> +	struct regmap *const regmap = dev_get_regmap(dev, NULL);
> +
> +	/*
> +	 * Place the peripheral in its lowest power consuming state.
> +	 */
> +	if (regmap)
> +		regmap_write(regmap, BMI323_CMD_REG, BMI323_RST_VAL);
This is happening before the userspace interfaces are removed.
Unlikely to be a good idea.  Use devm_add_action_or_reset()
to register this during probe so that it is torn down
at the correct point.

> +}
> +EXPORT_SYMBOL_NS_GPL(bmi323_core_remove, IIO_BMI323);
> +
>  #if defined(CONFIG_PM)
>  static int bmi323_core_runtime_suspend(struct device *dev)


>  
>  static int bmi323_core_runtime_resume(struct device *dev)
>  {
> -	struct iio_dev *indio_dev = dev_get_drvdata(dev);
> +	struct iio_dev *const indio_dev = dev_get_drvdata(dev);

Whilst true that it's const, do we care? I'd drop this
modification.

> +	struct bmi323_data *const data = iio_priv(indio_dev);
Likewise, unlikely the const really helps us.

> +
> +	int ret = 0;

Always set, so don't initialize here.

> +
> +	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, false);
> +	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 < EXT_SETTING_REGISTERS; ++i) {

i++ preferred. It's more common and doesn't matter here.

I'd use a local variable for a pointer to data->runtime_pm_status.ext_settings[i]
to make these more readable. Same for other similar cases.

> +		ret = bmi323_write_ext_reg(data,
> +			data->runtime_pm_status.ext_settings[i].reg,
> +			data->runtime_pm_status.ext_settings[i].val);
> +		if (ret) {
> +			dev_err(data->dev, "Error writing bmi323 external reg 0x%x: %d\n",
> +				data->runtime_pm_status.ext_settings[i].reg, ret);
> +			return ret;
> +		}
> +	}
> +
> +	/*
> +	 * Restore registers saved by suspend pm callback.
> +	 */
> +	for (unsigned int i = 0; i < SETTING_REGISTERS; ++i) {
> +		ret = regmap_write(data->regmap,
> +			data->runtime_pm_status.reg_settings[i].reg,
> +			data->runtime_pm_status.reg_settings[i].val);
> +		if (ret) {
> +			dev_err(data->dev, "Error writing bmi323 reg 0x%x: %d\n",
> +				data->runtime_pm_status.reg_settings[i].reg, ret);
> +			return ret;
> +		}
> +	}
> +
> +	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);
>  }

> diff --git a/drivers/iio/imu/bmi323/bmi323_spi.c b/drivers/iio/imu/bmi323/bmi323_spi.c
> index 9de3ade78d71..f42c49c471c1 100644
> --- a/drivers/iio/imu/bmi323/bmi323_spi.c
> +++ b/drivers/iio/imu/bmi323/bmi323_spi.c
> @@ -64,6 +64,13 @@ static int bmi323_spi_probe(struct spi_device *spi)
>  	return bmi323_core_probe(dev);
>  }
>  
> +static void bmi323_spi_remove(struct spi_device *spi)
> +{
> +	struct device *const dev = &spi->dev;
> +
> +	bmi323_core_remove(dev);
Register anything that needs doing with devm_add_action_or_reset()
after whatever is undoing is first done in probe().

There shouldn't be a need for a remove function and it is very
likely to have ordering issues given driver previously didn't
have one.

> +}
> +
>  static const struct spi_device_id bmi323_spi_ids[] = {
>  	{ "bmi323" },
>  	{ }
> @@ -83,6 +90,7 @@ static struct spi_driver bmi323_spi_driver = {
>  		.of_match_table = bmi323_of_spi_match,
>  	},
>  	.probe = bmi323_spi_probe,
> +	.remove = bmi323_spi_remove,
>  	.id_table = bmi323_spi_ids,
>  };
>  module_spi_driver(bmi323_spi_driver);

[PATCH v2 0/1] iio: bmi323: have the peripheral consume less power

[Denis Benato]

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

Previous patches obsoleted:
https://lore.kernel.org/all/20240811161202.19818-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 | 219 ++++++++++++++++++++++++++-
 1 file changed, 217 insertions(+), 2 deletions(-)

-- 
2.46.0

[PATCH v2] iio: bmi323: peripheral in lowest power state on suspend

[Denis Benato]

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.0

Re: [PATCH v2] iio: bmi323: peripheral in lowest power state on suspend

[Jonathan Cameron]

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);
>  }

[PATCH v3 0/1] iio: bmi323: have the peripheral consume less power

[Denis Benato]

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

[PATCH v3 1/1] iio: bmi323: peripheral in lowest power state on suspend

[Denis Benato]

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.0

Re: [PATCH v3 1/1] iio: bmi323: peripheral in lowest power state on suspend

[Jonathan Cameron]

On 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);
>  }

Re: [PATCH v3 0/1] iio: bmi323: have the peripheral consume less power

[Jonathan Cameron]

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(-)
> 

Re: [PATCH v3 0/1] iio: bmi323: have the peripheral consume less power

[Denis Benato]

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(-)
>>
>