Re: [PATCH] iio: light: ltr390: Add runtime PM support

[David Lechner <dlechner@baylibre.com>]

On 8/22/25 1:03 PM, Akshay Jindal wrote:
> Implement runtime power management for the LTR390 sensor.
> The device would now autosuspend after 1s of idle time.
> This would save the overall power consumption by the sensor.

How much power does it actually save?

> 
> Ensure that interrupts continue to be delivered during
> runtime suspend by disabling the sensor only when no
> interrupts are enabled. This prevents loss of events
> while still allowing power savings when IRQs are unused.
> 
> Signed-off-by: Akshay Jindal <akshayaj.lkd@gmail.com>
> ---
> 
> Testing summary:
> ================
> -> Tested on Raspberrypi 4B. Following tests were performed.
> 1. Verified that /sys/bus/i2c/devices/i2c-1/1-0053/power/control contains ‘auto’ value.
> 2. Verified the /sys/bus/i2c/devices/i2c-1/1-0053/power/autosuspend_delay_ms contains 1000 which is assigned by the driver.
> 3. Changed the autosuspend_delay_ms value from 1000 to 2000ms and verified it.
> 	3.1 Verified through the timestamp that whatever autosuspend_delay_ms is set, it is being honoured.
> 4. Verified that runtime_suspend and runtime_resume callbacks are called whenever any IO is done on a channel attribute.
> 	4.1 Verified that power/runtime_status first becomes active and then becomes suspended.
> 	4.2 Verified that power/runtime_active_time keeps on increasing with a delta of autosuspend_delay_ms.
> 
> Interrupt Handling Verification:
> --------------------------------
> 1. Verified that when interrupts are enabled on the device, then the device does not get put in standby mode and keeps sampling.
> 	a. As a result interrupts are delivered to the driver and are handled.
> 2. Verified that when interrupts are disabled, the device is put in standby mode and stops sampling.
> 	a.Since there is no sampling, so no IRQs will be generated. They are only generated when the device is resumed due to I/O on some sysfs attribute from userspace.
> 
>  drivers/iio/light/ltr390.c | 246 +++++++++++++++++++++++++++++--------
>  1 file changed, 193 insertions(+), 53 deletions(-)
> 
> diff --git a/drivers/iio/light/ltr390.c b/drivers/iio/light/ltr390.c
> index 2e1cf62e8201..9e2f33a401f2 100644
> --- a/drivers/iio/light/ltr390.c
> +++ b/drivers/iio/light/ltr390.c
> @@ -30,6 +30,7 @@
>  
>  #include <linux/iio/iio.h>
>  #include <linux/iio/events.h>
> +#include <linux/pm_runtime.h>
>  
>  #include <linux/unaligned.h>
>  
> @@ -105,6 +106,7 @@ struct ltr390_data {
>  	enum ltr390_mode mode;
>  	int gain;
>  	int int_time_us;
> +	bool irq_enabled;
>  };
>  
>  static const struct regmap_range ltr390_readable_reg_ranges[] = {
> @@ -154,6 +156,25 @@ static const int ltr390_samp_freq_table[][2] = {
>  		[7] = { 500, 2000 },
>  };
>  
> +static int ltr390_set_power_state(struct ltr390_data *data, bool on)

This function does completely different things depending on if the
last argument is true or false. It should just be two separate
functions and avoid the parameter and the if statement.

> +{
> +	struct device *dev = &data->client->dev;
> +	int ret = 0;
> +
> +	if (on) {
> +		ret = pm_runtime_resume_and_get(dev);
> +		if (ret) {
> +			dev_err(dev, "failed to resume runtime PM: %d\n", ret);
> +			return ret;
> +		}
> +	} else {
> +		pm_runtime_mark_last_busy(dev);
> +		pm_runtime_put_autosuspend(dev);
> +	}
> +
> +	return ret;
> +}
> +