Re: [PATCH 27/80] iio: accel: Remove redundant pm_runtime_mark_last_busy() calls

[Jonathan Cameron <jic23@kernel.org>]

On Thu, 10 Jul 2025 09:46:12 +0300
Matti Vaittinen <mazziesaccount@gmail.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@linux.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@gmail.com>
> 
> Yours
> 	-- Matti
> 
> 
>