linux-kernel - Re: [PATCH 27/80] iio: accel: Remove redundant pm_runtime_mark_last

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: <20250713144346.6fc46252@jic23-huawei>
Date: Sun, 13 Jul 2025 14:43:46 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: Matti Vaittinen <mazziesaccount@...il.com>
Cc: Sakari Ailus <sakari.ailus@...ux.intel.com>, David Lechner
 <dlechner@...libre.com>, Nuno Sá <nuno.sa@...log.com>, Andy
 Shevchenko <andy@...nel.org>, Waqar Hameed <waqar.hameed@...s.com>, Julien
 Stephan <jstephan@...libre.com>, Peter Zijlstra <peterz@...radead.org>, Bo
 Liu <liubo03@...pur.com>, Greg KH <gregkh@...uxfoundation.org>, Al Viro
 <viro@...iv.linux.org.uk>, Sean Nyekjaer <sean@...nix.com>, Marcelo Schmitt
 <marcelo.schmitt1@...il.com>, Rayyan Ansari <rayyan@...ari.sh>, Francisco
 Henriques <franciscolealhenriques@....br>, linux-iio@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH 27/80] iio: accel: Remove redundant
 pm_runtime_mark_last_busy() calls

On Thu, 10 Jul 2025 09:46:12 +0300
Matti Vaittinen <mazziesaccount@...il.com> wrote:

> On 04/07/2025 10:54, Sakari Ailus wrote:
> > pm_runtime_put_autosuspend(), pm_runtime_put_sync_autosuspend(),
> > pm_runtime_autosuspend() and pm_request_autosuspend() now include a call
> > to pm_runtime_mark_last_busy(). Remove the now-reduntant explicit call to
> > pm_runtime_mark_last_busy().
> > 
> > Signed-off-by: Sakari Ailus <sakari.ailus@...ux.intel.com>
> > ---  
> 
> Looks good to me. Just one comment (to 4 drivers) - but I'm not 
> insisting it to be addressed :)
> 
> > The cover letter of the set can be found here
> > <URL:https://lore.kernel.org/linux-pm/20250704075225.3212486-1-sakari.ailus@linux.intel.com>.
> > 
> > In brief, this patch depends on PM runtime patches adding marking the last
> > busy timestamp in autosuspend related functions. The patches are here, on
> > rc2:
> > 
> >          git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git \
> >                  pm-runtime-6.17-rc1
> > 
> >   drivers/iio/accel/bmc150-accel-core.c | 1 -
> >   drivers/iio/accel/bmi088-accel-core.c | 3 ---
> >   drivers/iio/accel/fxls8962af-core.c   | 1 -
> >   drivers/iio/accel/kxcjk-1013.c        | 1 -
> >   drivers/iio/accel/kxsd9.c             | 3 ---
> >   drivers/iio/accel/mma8452.c           | 1 -
> >   drivers/iio/accel/mma9551_core.c      | 1 -
> >   drivers/iio/accel/msa311.c            | 6 ------
> >   8 files changed, 17 deletions(-)
> > 
> > diff --git a/drivers/iio/accel/bmc150-accel-core.c b/drivers/iio/accel/bmc150-accel-core.c
> > index be5fbb0c5d29..f45beae83f8b 100644
> > --- a/drivers/iio/accel/bmc150-accel-core.c
> > +++ b/drivers/iio/accel/bmc150-accel-core.c
> > @@ -335,7 +335,6 @@ static int bmc150_accel_set_power_state(struct bmc150_accel_data *data, bool on)
> >   	if (on) {
> >   		ret = pm_runtime_resume_and_get(dev);
> >   	} else {
> > -		pm_runtime_mark_last_busy(dev);
> >   		ret = pm_runtime_put_autosuspend(dev);
> >   	}
> >     
> 
> // snip
> 
> > diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
> > index 6aefe8221296..44d770729186 100644
> > --- a/drivers/iio/accel/kxcjk-1013.c
> > +++ b/drivers/iio/accel/kxcjk-1013.c
> > @@ -637,7 +637,6 @@ static int kxcjk1013_set_power_state(struct kxcjk1013_data *data, bool on)
> >   	if (on)
> >   		ret = pm_runtime_resume_and_get(&data->client->dev);
> >   	else {
> > -		pm_runtime_mark_last_busy(&data->client->dev);
> >   		ret = pm_runtime_put_autosuspend(&data->client->dev);
> >   	}
> >   	if (ret < 0) {  
> 
> //snip
> 
> > diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
> > index aba444a980d9..5863478bab62 100644
> > --- a/drivers/iio/accel/mma8452.c
> > +++ b/drivers/iio/accel/mma8452.c
> > @@ -227,7 +227,6 @@ static int mma8452_set_runtime_pm_state(struct i2c_client *client, bool on)
> >   	if (on) {
> >   		ret = pm_runtime_resume_and_get(&client->dev);
> >   	} else {
> > -		pm_runtime_mark_last_busy(&client->dev);
> >   		ret = pm_runtime_put_autosuspend(&client->dev);
> >   	}  
> 
> //snip
> 
> >   
> > diff --git a/drivers/iio/accel/mma9551_core.c b/drivers/iio/accel/mma9551_core.c
> > index 3e7d9b79ed0e..22768f43fd24 100644
> > --- a/drivers/iio/accel/mma9551_core.c
> > +++ b/drivers/iio/accel/mma9551_core.c
> > @@ -672,7 +672,6 @@ int mma9551_set_power_state(struct i2c_client *client, bool on)
> >   	if (on)
> >   		ret = pm_runtime_resume_and_get(&client->dev);
> >   	else {
> > -		pm_runtime_mark_last_busy(&client->dev);
> >   		ret = pm_runtime_put_autosuspend(&client->dev);
> >   	}
> >     
> 
> Do these really warrant a function? (Especially for the mma9551 where 
> the function is exported). I think it'd be fine to have the 
> pm_runtime_resume_and_get() and the pm_runtime_put_autosuspend() called 
> directly without these wrappers.

Absolutely agree that they don't.  However, I think it may make
sense to clean that up in two jumps.   This series just makes the
specific drop of the unnecessary call, next series does the refactor.
That keeps this series tightly focused on one topic.

Jonathan

> 
> Anyways, this looks good to me - thanks!
> 
> Reviewed-by: Matti Vaittinen <mazziesaccount@...il.com>
> 
> Yours
> 	-- Matti
> 
> 
>