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: <CAE3SzaR=uPtE20ZkWVXYwBrUZ=b46h1Ub=TbWArHW6XbD8qXfQ@mail.gmail.com>
Date: Fri, 29 Aug 2025 23:32:56 +0530
From: Akshay Jindal <akshayaj.lkd@...il.com>
To: Andy Shevchenko <andy.shevchenko@...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 Fri, Aug 29, 2025 at 9:16 PM Andy Shevchenko
<andy.shevchenko@...il.com> wrote:
>
> On Thu, Aug 28, 2025 at 9:47 PM Akshay Jindal <akshayaj.lkd@...il.com> wrote:
> > On Thu, Aug 28, 2025 at 7:17 AM Andy Shevchenko
> > <andy.shevchenko@...il.com> wrote:
> > > On Wed, Aug 27, 2025 at 10:19 PM Akshay Jindal <akshayaj.lkd@...il.com> wrote:
>
> ...
>
> > > > +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.
> > >
> > My rationale behind this approach is that earlier ltr390_read_raw()
> > had all the functionality
> > of the .read_raw callback so the return value whether success or
> > failure was of the core functionality.
> > But now, since the core functionality has been relocated into
> > __ltr390_read_raw(), I felt the return value
> > ltr390_read_raw should be the return value of __ltr390_read_raw().
>
> "Main" returned value. But this is not the point. The Q is, how do you
> expect to get not garbage from, e.g., powered off device, please?

I got your point.

My Rationale:
------------------
>From a functionality point of view, if before runtime PM usage, the
core retval was being
returned (success/failure), then now with runtime PM also, it should
be the same core retval
which should be returned, but I think that is not correct thinking.

Updated Approach:
-------------------------
Since we are introducing new functionality of runtime PM in the
callbacks, the return
value should reflect ANY failure in the callback, be it failure of
core functionality
or failure of runtime PM calls. With this in mind, I will be
refactoring the callbacks
as follows:

ret = pm_runtime_resume_and_get();
if (ret < 0)
     return ret; //main functionality will not be called.
retval = __ltr390_read_raw();
// not checking retval because irrespective of retval we need refcount
decrement and suspension.
ret = pm_runtime_put_autosuspend();-
if (ret < 0)
     return ret;
return retval;

Let me know your thoughts.

Thanks,
Akshay.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ