linux-kernel - Re: [PATCH v2] iio: light: ltr390: Implement 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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAHp75VdQB673=qXBp0mPUHxQGxM=Z1CNSmEpP82OprVn++Y5uw@mail.gmail.com>
Date: Thu, 28 Aug 2025 04:46:22 +0300
From: Andy Shevchenko <andy.shevchenko@...il.com>
To: Akshay Jindal <akshayaj.lkd@...il.com>
Cc: anshulusr@...il.com, jic23@...nel.org, dlechner@...libre.com, 
	nuno.sa@...log.com, andy@...nel.org, shuah@...nel.org, 
	linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] iio: light: ltr390: Implement runtime PM support

On Wed, Aug 27, 2025 at 10:19 PM Akshay Jindal <akshayaj.lkd@...il.com> wrote:
>
> Implement runtime power management for the LTR390 sensor. The device
> autosuspends after 1s of idle time, reducing current consumption from
> 100 µA in active mode to 1 µA in standby mode as per the datasheet.
>
> Ensure that interrupts continue to be delivered with runtime PM.
> Since the LTR390 cannot be used as a wakeup source during runtime
> suspend, therefore increment the runtime PM refcount when enabling
> events and decrement it when disabling events or powering down.
> This prevents event loss while still allowing power savings when IRQs
> are unused.

...

> -> Preserve the order of header file includes.

Seems needs more

...

> --- a/drivers/iio/light/ltr390.c
> +++ b/drivers/iio/light/ltr390.c
> @@ -27,6 +27,7 @@
>  #include <linux/module.h>
>  #include <linux/mutex.h>
>  #include <linux/regmap.h>
> +#include <linux/pm_runtime.h>

With the given context 'p' should be before 'r'.

...

> +static int ltr390_read_raw(struct iio_dev *iio_device,
> +                          struct iio_chan_spec const *chan, int *val,
> +                          int *val2, long mask)
> +{
> +       int ret, retval;
> +       struct ltr390_data *data = iio_priv(iio_device);
> +       struct device *dev = &data->client->dev;
> +
> +       ret = pm_runtime_resume_and_get(dev);
> +       if (ret < 0)
> +               dev_err(dev, "runtime PM failed to resume: %d\n", ret);

If it fails, there is no point to read the value, it will be garbage
or even can make the bus stuck.

> +       retval = __ltr390_read_raw(iio_device, chan, val, val2, mask);
> +
> +       ret = pm_runtime_put_autosuspend(dev);
> +       if (ret < 0)
> +               dev_err(dev, "runtime PM failed to autosuspend: %d\n", ret);

Taking into account the above, I do not see how this message is
useful. If we fail here, we can't do much about it, but printing a
message will exaggerate the logs.

> +       return retval;
> +}

...

> +static int ltr390_write_raw(struct iio_dev *indio_dev, struct iio_chan_spec const *chan,
> +                       int val, int val2, long mask)

As per above.

...

> +static int ltr390_read_event_value(struct iio_dev *indio_dev,
> +                               const struct iio_chan_spec *chan,
> +                               enum iio_event_type type,
> +                               enum iio_event_direction dir,
> +                               enum iio_event_info info,
> +                               int *val, int *val2)

Ditto and so on...

-- 
With Best Regards,
Andy Shevchenko