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: <20250830190806.7dd92eb9@jic23-huawei>
Date: Sat, 30 Aug 2025 19:08:06 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: Akshay Jindal <akshayaj.lkd@...il.com>
Cc: anshulusr@...il.com, 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] iio: light: ltr390: Add runtime PM support

On Tue, 26 Aug 2025 02:29:12 +0530
Akshay Jindal <akshayaj.lkd@...il.com> wrote:

> Hi Jonathan,
> Thanks for your review. Please see my followup inline.
> 
> On Mon, Aug 25, 2025 at 7:56 PM Jonathan Cameron <jic23@...nel.org> wrote:
> >
> > On Fri, 22 Aug 2025 23:33:26 +0530
> > Akshay Jindal <akshayaj.lkd@...il.com> wrote:  
> > > +
> > > +     if (!state) {
> > > +             ret = regmap_clear_bits(data->regmap, LTR390_INT_CFG, LTR390_LS_INT_EN);
> > > +             data->irq_enabled = false;  
> >
> > Just take an extra reference to runtime pm on enable of event and put it disable.
> > Then no need for special handling with a local flag etc.  
> 
> Consider a scenario, where the user only disables the event instead of
> enabling it,
> (i.e. user wrote 0 on the sysfs attribute before it was 1). In this case,
> If enable means inc ref count and disable means dec ref count, then
> this would lead to refcount underflow and the suspend callback will
> not be called.
> 
> To handle this case, we would need to check whether irq/event was
> enabled or not.
> For that either we can use the local flag as I did, or I would need to
> do a read and test
> for the interrupt bit being set. I feel using the local flag would be
> cleaner and would
> require less code.
> If you are fine with local flag usage, then shall I not stick to only
> local flag usage?
> 

Normally we'd check the register to find out what whether the event
is enabled or not.  If we are asking for the state it is already in
then just return having done nothing.  If bus reads are an overhead
worth avoiding regcache will ensure we only do it once.

Then if we are doing something, do the runtime pm get / put as appropriate.

Jonathan


> Thanks,
> Akshay


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ