Re: [PATCH v5] iio: pressure: mpl115: Implementing low power mode by shutdown gpio

From: Jonathan Cameron
Date: Sat Oct 15 2022 - 12:44:10 EST

On Mon, 10 Oct 2022 23:07:20 +0530
Rajat Khandelwal <rajat.khandelwal@xxxxxxxxxxxxxxx> wrote:

> MPL115 supports shutdown gpio which can be used to set the state
> to low power mode. Power from all internal circuits and
> registers is removed. This is done by pulling the SHDN pin to low.
> This patch enables runtime PM on MPL115 to increase power savings.
>
> According to spec., a wakeup time period of ~5 ms exists between
> waking up and actually communicating with the device. This is
> implemented using sleep delay.
>
> Signed-off-by: Rajat Khandelwal <rajat.khandelwal@xxxxxxxxxxxxxxx>
Applied to the togreg branch of iio.git and pushed out as testing until I can
rebase that tree on rc1 in a few days time.

Thanks,

Jonathan

> ---
>
> v5:
> 1. Removing build error by exporting EXPORT_NS_RUNTIME_DEV_PM_OPS
> 2. Using runtime PM for low power mode and not forcing shutdown pin
> 3. Changing patch comment
> 4. Increasing autosuspend timeout to 2 sec to make the driver more
> responsive to user
>
> drivers/iio/pressure/mpl115.c | 61 ++++++++++++++++++++++++++++++-
> drivers/iio/pressure/mpl115.h | 5 +++
> drivers/iio/pressure/mpl115_i2c.c | 1 +
> drivers/iio/pressure/mpl115_spi.c | 1 +
> 4 files changed, 67 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iio/pressure/mpl115.c b/drivers/iio/pressure/mpl115.c
> index 5bf5b9abe6f1..ec7527161844 100644
> --- a/drivers/iio/pressure/mpl115.c
> +++ b/drivers/iio/pressure/mpl115.c
> @@ -4,12 +4,13 @@
> *
> * Copyright (c) 2014 Peter Meerwald <pmeerw@xxxxxxxxxx>
> *
> - * TODO: shutdown pin
> + * TODO: synchronization with system suspend
> */
>
> #include <linux/module.h>
> #include <linux/iio/iio.h>
> #include <linux/delay.h>
> +#include <linux/gpio/consumer.h>
>
> #include "mpl115.h"
>
> @@ -27,6 +28,7 @@ struct mpl115_data {
> s16 a0;
> s16 b1, b2;
> s16 c12;
> + struct gpio_desc *shutdown;
> const struct mpl115_ops *ops;
> };
>
> @@ -102,16 +104,24 @@ static int mpl115_read_raw(struct iio_dev *indio_dev,
>
> switch (mask) {
> case IIO_CHAN_INFO_PROCESSED:
> + pm_runtime_get_sync(data->dev);
> ret = mpl115_comp_pressure(data, val, val2);
> if (ret < 0)
> return ret;
> + pm_runtime_mark_last_busy(data->dev);
> + pm_runtime_put_autosuspend(data->dev);
> +
> return IIO_VAL_INT_PLUS_MICRO;
> case IIO_CHAN_INFO_RAW:
> + pm_runtime_get_sync(data->dev);
> /* temperature -5.35 C / LSB, 472 LSB is 25 C */
> ret = mpl115_read_temp(data);
> if (ret < 0)
> return ret;
> + pm_runtime_mark_last_busy(data->dev);
> + pm_runtime_put_autosuspend(data->dev);
> *val = ret >> 6;
> +
> return IIO_VAL_INT;
> case IIO_CHAN_INFO_OFFSET:
> *val = -605;
> @@ -168,6 +178,8 @@ int mpl115_probe(struct device *dev, const char *name,
> if (ret)
> return ret;
>
> + dev_set_drvdata(dev, indio_dev);
> +
> ret = data->ops->read(data->dev, MPL115_A0);
> if (ret < 0)
> return ret;
> @@ -185,10 +193,58 @@ int mpl115_probe(struct device *dev, const char *name,
> return ret;
> data->c12 = ret;
>
> + data->shutdown = devm_gpiod_get_optional(dev, "shutdown",
> + GPIOD_OUT_LOW);
> + if (IS_ERR(data->shutdown))
> + return dev_err_probe(dev, PTR_ERR(data->shutdown),
> + "cannot get shutdown gpio\n");
> +
> + if (data->shutdown) {
> + /* Enable runtime PM */
> + pm_runtime_get_noresume(dev);
> + pm_runtime_set_active(dev);
> + pm_runtime_enable(dev);
> +
> + /*
> + * As the device takes 3 ms to come up with a fresh
> + * reading after power-on and 5 ms to actually power-on,
> + * do not shut it down unnecessarily. Set autosuspend to
> + * 2000 ms.
> + */
> + pm_runtime_set_autosuspend_delay(dev, 2000);
> + pm_runtime_use_autosuspend(dev);
> + pm_runtime_put(dev);
> +
> + dev_dbg(dev, "low-power mode enabled");
> + } else
> + dev_dbg(dev, "low-power mode disabled");
> +
> return devm_iio_device_register(dev, indio_dev);
> }
> EXPORT_SYMBOL_NS_GPL(mpl115_probe, IIO_MPL115);
>
> +static int mpl115_runtime_suspend(struct device *dev)
> +{
> + struct mpl115_data *data = iio_priv(dev_get_drvdata(dev));
> +
> + gpiod_set_value(data->shutdown, 1);
> +
> + return 0;
> +}
> +
> +static int mpl115_runtime_resume(struct device *dev)
> +{
> + struct mpl115_data *data = iio_priv(dev_get_drvdata(dev));
> +
> + gpiod_set_value(data->shutdown, 0);
> + usleep_range(5000, 6000);
> +
> + return 0;
> +}
> +
> +EXPORT_NS_RUNTIME_DEV_PM_OPS(mpl115_dev_pm_ops, mpl115_runtime_suspend,
> + mpl115_runtime_resume, NULL, IIO_MPL115);
> +
> MODULE_AUTHOR("Peter Meerwald <pmeerw@xxxxxxxxxx>");
> MODULE_DESCRIPTION("Freescale MPL115 pressure/temperature driver");
> MODULE_LICENSE("GPL");
> diff --git a/drivers/iio/pressure/mpl115.h b/drivers/iio/pressure/mpl115.h
> index 57d55eb8e661..78a0068a17bb 100644
> --- a/drivers/iio/pressure/mpl115.h
> +++ b/drivers/iio/pressure/mpl115.h
> @@ -6,6 +6,8 @@
> * Copyright (c) 2016 Akinobu Mita <akinobu.mita@xxxxxxxxx>
> */
>
> +#include <linux/pm_runtime.h>
Seems unnecessary to introduce this here.
Just provide a
> +
> #ifndef _MPL115_H_
> #define _MPL115_H_
>
> @@ -18,4 +20,7 @@ struct mpl115_ops {
> int mpl115_probe(struct device *dev, const char *name,
> const struct mpl115_ops *ops);
>
> +/*PM ops */
> +extern const struct dev_pm_ops mpl115_dev_pm_ops;
> +
> #endif
> diff --git a/drivers/iio/pressure/mpl115_i2c.c b/drivers/iio/pressure/mpl115_i2c.c
> index 099ab1c6832c..555bda1146fb 100644
> --- a/drivers/iio/pressure/mpl115_i2c.c
> +++ b/drivers/iio/pressure/mpl115_i2c.c
> @@ -53,6 +53,7 @@ MODULE_DEVICE_TABLE(i2c, mpl115_i2c_id);
> static struct i2c_driver mpl115_i2c_driver = {
> .driver = {
> .name = "mpl115",
> + .pm = pm_ptr(&mpl115_dev_pm_ops),
> },
> .probe = mpl115_i2c_probe,
> .id_table = mpl115_i2c_id,
> diff --git a/drivers/iio/pressure/mpl115_spi.c b/drivers/iio/pressure/mpl115_spi.c
> index 7feec87e2704..58d218fd90dc 100644
> --- a/drivers/iio/pressure/mpl115_spi.c
> +++ b/drivers/iio/pressure/mpl115_spi.c
> @@ -92,6 +92,7 @@ MODULE_DEVICE_TABLE(spi, mpl115_spi_ids);
> static struct spi_driver mpl115_spi_driver = {
> .driver = {
> .name = "mpl115",
> + .pm = pm_ptr(&mpl115_dev_pm_ops),
> },
> .probe = mpl115_spi_probe,
> .id_table = mpl115_spi_ids,