On Mon, 21 Oct 2024 21:53:16 +0200
Vasileios Amoiridis <vassilisamir@xxxxxxxxx> wrote:
> Add runtime power management to the device. To facilitate this, add also
> a struct dev* inside the bme680_data structure to have the device
> accesible from the data structure.
Needs an update as you are now getting that from the regmap.
>
> Signed-off-by: Vasileios Amoiridis <vassilisamir@xxxxxxxxx>
> ---
> drivers/iio/chemical/bme680.h | 2 +
> drivers/iio/chemical/bme680_core.c | 126 +++++++++++++++++++++++++++--
> 2 files changed, 121 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/iio/chemical/bme680.h b/drivers/iio/chemical/bme680.h
> index e5d82a6d5b59..74e97e35e35a 100644
> --- a/drivers/iio/chemical/bme680.h
> +++ b/drivers/iio/chemical/bme680.h
> @@ -2,6 +2,7 @@
> #ifndef BME680_H_
> #define BME680_H_
>
> +#include <linux/pm.h>
> #include <linux/regmap.h>
>
> #define BME680_REG_CHIP_ID 0xD0
> @@ -82,6 +83,7 @@
> #define BME680_CALIB_RANGE_3_LEN 5
>
> extern const struct regmap_config bme680_regmap_config;
> +extern const struct dev_pm_ops bme680_dev_pm_ops;
You seem to have missed the changes that use this in the i2c and spi drivers.
> static const char bme680_oversampling_ratio_show[] = "1 2 4 8 16";
>
> static IIO_CONST_ATTR(oversampling_ratio_available,
> @@ -1091,6 +1125,39 @@ static irqreturn_t bme680_trigger_handler(int irq, void *p)
> return IRQ_HANDLED;
> }
>
> +static int bme680_buffer_preenable(struct iio_dev *indio_dev)
> +{
> + struct bme680_data *data = iio_priv(indio_dev);
> + struct device *dev = regmap_get_device(data->regmap);
> +
> + pm_runtime_get_sync(dev);
> + return 0;
> +}
> +
> +static int bme680_buffer_postdisable(struct iio_dev *indio_dev)
> +{
> + struct bme680_data *data = iio_priv(indio_dev);
> + struct device *dev = regmap_get_device(data->regmap);
> +
> + pm_runtime_mark_last_busy(dev);
> + pm_runtime_put_autosuspend(dev);
> + return 0;
> +}
> +
> +static const struct iio_buffer_setup_ops bme680_buffer_setup_ops = {
> + .preenable = bme680_buffer_preenable,
> + .postdisable = bme680_buffer_postdisable,
> +};
> +
> +static void bme680_pm_disable(void *data)
> +{
> + struct device *dev = data;
> +
> + pm_runtime_get_sync(dev);
> + pm_runtime_put_noidle(dev);
This dance is to get the device powered up on runtime pm tear down
I think? Whilst we sometimes do this, why is it needed in this particular driver?
> + pm_runtime_disable(dev);
> +}
> +
> int bme680_core_probe(struct device *dev, struct regmap *regmap,
> const char *name)
> {
> @@ -1164,15 +1231,60 @@ int bme680_core_probe(struct device *dev, struct regmap *regmap,
> ret = devm_iio_triggered_buffer_setup(dev, indio_dev,
> iio_pollfunc_store_time,
> bme680_trigger_handler,
> - NULL);
> + &bme680_buffer_setup_ops);
> if (ret)
> return dev_err_probe(dev, ret,
> "iio triggered buffer setup failed\n");
>
> + /* Enable runtime PM */
> + pm_runtime_get_noresume(dev);
> + pm_runtime_set_autosuspend_delay(dev, BME680_STARTUP_TIME_US * 100);
> + pm_runtime_use_autosuspend(dev);
> + pm_runtime_set_active(dev);
> + ret = devm_pm_runtime_enable(dev);
Take a look at what this unwinds in the devm handler. You don't need to do
as much (or possibly anything) yourself.
> + if (ret)
> + return ret;
> +
> + pm_runtime_put(dev);
> +
> + ret = devm_add_action_or_reset(dev, bme680_pm_disable, dev);
> + if (ret)
> + return ret;
> +
> return devm_iio_device_register(dev, indio_dev);
> }
> EXPORT_SYMBOL_NS_GPL(bme680_core_probe, IIO_BME680);
>
> +static int bme680_runtime_suspend(struct device *dev)
> +{
> + struct iio_dev *indio_dev = dev_get_drvdata(dev);
> + struct bme680_data *data = iio_priv(indio_dev);
> +
> + return regulator_bulk_disable(BME680_NUM_SUPPLIES, data->supplies);
> +}
> +
> +static int bme680_runtime_resume(struct device *dev)
> +{
> + struct iio_dev *indio_dev = dev_get_drvdata(dev);
> + struct bme680_data *data = iio_priv(indio_dev);
> + int ret;
> +
> + ret = regulator_bulk_enable(BME680_NUM_SUPPLIES, data->supplies);
> + if (ret)
> + return ret;
> +
> + fsleep(BME680_STARTUP_TIME_US);
> +
> + ret = bme680_chip_config(data);
> + if (ret)
> + return ret;
> +
> + return bme680_gas_config(data);
> +}
> +
> +EXPORT_RUNTIME_DEV_PM_OPS(bme680_dev_pm_ops, bme680_runtime_suspend,
> + bme680_runtime_resume, NULL);
> +
> MODULE_AUTHOR("Himanshu Jha <himanshujha199640@xxxxxxxxx>");
> MODULE_DESCRIPTION("Bosch BME680 Driver");
> MODULE_LICENSE("GPL v2");
