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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <1c0fe63a-e13b-4e01-9032-686298d3b632@baylibre.com>
Date: Sat, 23 Aug 2025 11:03:12 -0500
From: David Lechner <dlechner@...libre.com>
To: Akshay Jindal <akshayaj.lkd@...il.com>, anshulusr@...il.com,
 jic23@...nel.org, nuno.sa@...log.com, andy@...nel.org
Cc: shuah@...nel.org, linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] iio: light: ltr390: Add runtime PM support

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@...il.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;
> +}
> +