RE: [PATCH v2] iio: pressure: mpl115: Implementing low power mode by shutdown gpio

[Khandelwal, Rajat]

>>Ah. I missed there was a v2 and replied to v1.
No problem Jon and thanks for the suggestion. A delay of 5ms
was not worth it when calculation itself takes only ~3ms. 
I have implemented runtime PM keeping a delay of 200 ms
which I think should suffice. Have sent you v3. 

-----Original Message-----
From: Jonathan Cameron <jic23@xxxxxxxxxx> 
Sent: Sunday, October 2, 2022 6:52 PM
To: Khandelwal, Rajat <rajat.khandelwal@xxxxxxxxx>
Cc: lars@xxxxxxxxxx; linux-iio@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
Subject: Re: [PATCH v2] iio: pressure: mpl115: Implementing low power mode by shutdown gpio

On Tue, 27 Sep 2022 23:12:12 +0530
Rajat Khandelwal <rajat.khandelwal@xxxxxxxxx> wrote:

> MPL115 supports shutdown gpio which can be used to set the default 
> 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 sets the default mode to low-power and only exits when a 
> reading is required from the chip. This maximises 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@xxxxxxxxx>
Ah. I missed there was a v2 and replied to v1.

> ---
> 
> v2: Grammatically completing subject line
> 
>  drivers/iio/pressure/mpl115.c | 33 +++++++++++++++++++++++++++++----
>  1 file changed, 29 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iio/pressure/mpl115.c 
> b/drivers/iio/pressure/mpl115.c index 5bf5b9abe6f1..c0ad3e941d05 
> 100644
> --- a/drivers/iio/pressure/mpl115.c
> +++ b/drivers/iio/pressure/mpl115.c
> @@ -3,13 +3,12 @@
>   * mpl115.c - Support for Freescale MPL115A pressure/temperature sensor
>   *
>   * Copyright (c) 2014 Peter Meerwald <pmeerw@xxxxxxxxxx>
> - *
> - * TODO: shutdown pin
>   */
>  
>  #include <linux/module.h>
>  #include <linux/iio/iio.h>
>  #include <linux/delay.h>
> +#include <linux/gpio/consumer.h>
>  
>  #include "mpl115.h"
>  
> @@ -27,6 +26,7 @@ struct mpl115_data {
>  	s16 a0;
>  	s16 b1, b2;
>  	s16 c12;
> +	struct gpio_desc *shutdown;
>  	const struct mpl115_ops *ops;
>  };
>  
> @@ -102,13 +102,27 @@ static int mpl115_read_raw(struct iio_dev 
> *indio_dev,
>  
>  	switch (mask) {
>  	case IIO_CHAN_INFO_PROCESSED:
> -		ret = mpl115_comp_pressure(data, val, val2);
> +		if (data->shutdown) {
> +			gpiod_set_value(data->shutdown, 0);
> +			usleep_range(5000, 6000);
> +			ret = mpl115_comp_pressure(data, val, val2);
> +			gpiod_set_value(data->shutdown, 1);
> +		} else
> +			ret = mpl115_comp_pressure(data, val, val2);
> +
>  		if (ret < 0)
>  			return ret;
>  		return IIO_VAL_INT_PLUS_MICRO;
>  	case IIO_CHAN_INFO_RAW:
>  		/* temperature -5.35 C / LSB, 472 LSB is 25 C */
> -		ret = mpl115_read_temp(data);
> +		if (data->shutdown) {
> +			gpiod_set_value(data->shutdown, 0);
> +			usleep_range(5000, 6000);
> +			ret = mpl115_read_temp(data);
> +			gpiod_set_value(data->shutdown, 1);
> +		} else
> +			ret = mpl115_read_temp(data);
> +
>  		if (ret < 0)
>  			return ret;
>  		*val = ret >> 6;
> @@ -185,6 +199,17 @@ int mpl115_probe(struct device *dev, const char *name,
>  		return ret;
>  	data->c12 = ret;
>  
> +	data->shutdown = devm_gpiod_get_optional(dev, "shutdown",
> +						 GPIOD_OUT_HIGH);
> +	if (IS_ERR(data->shutdown))
> +		return dev_err_probe(dev, PTR_ERR(data->shutdown),
> +				     "cannot get shutdown gpio\n");
> +
> +	if (data->shutdown)
> +		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);