Re: [PATCH v3 08/12] iio: light: Remove redundant pm_runtime_mark_last_busy() calls

[Jonathan Cameron <jic23@kernel.org>]

On Mon, 25 Aug 2025 16:53:57 +0300
Sakari Ailus <sakari.ailus@linux.intel.com> 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>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Hi Sakari,

I'm going to apply this as it stands, but there has been a lot
of cut and paste in these drivers, so a common pattern is to have
a no entirely pointless power(bool enable) function.  
Previously it at least wrapped up 2 calls, now it is doing a choice
between two single line calls so makes no sense.

Anyhow, if anyone wants to clean these up that would be great.
In meantime I'd rather not leave cases of the pm_runtime_mark_last_busy()
around to get pasted into more drivers.

So applied,

Jonathan

> diff --git a/drivers/iio/light/isl29028.c b/drivers/iio/light/isl29028.c
> index 0e4284823d44..374bccad9119 100644
> --- a/drivers/iio/light/isl29028.c
> +++ b/drivers/iio/light/isl29028.c
> @@ -336,16 +336,11 @@ static int isl29028_ir_get(struct isl29028_chip *chip, int *ir_data)
>  static int isl29028_set_pm_runtime_busy(struct isl29028_chip *chip, bool on)
>  {
>  	struct device *dev = regmap_get_device(chip->regmap);
As below.

> -	int ret;
>  
> -	if (on) {
> -		ret = pm_runtime_resume_and_get(dev);
> -	} else {
> -		pm_runtime_mark_last_busy(dev);
> -		ret = pm_runtime_put_autosuspend(dev);
> -	}
> +	if (on)
> +		return pm_runtime_resume_and_get(dev);
>  
> -	return ret;
> +	return pm_runtime_put_autosuspend(dev);
>  }
>  

> diff --git a/drivers/iio/light/tsl2583.c b/drivers/iio/light/tsl2583.c
> index fc3b0c4226be..8801a491de77 100644
> --- a/drivers/iio/light/tsl2583.c
> +++ b/drivers/iio/light/tsl2583.c
> @@ -641,16 +641,10 @@ static const struct iio_chan_spec tsl2583_channels[] = {
>  
>  static int tsl2583_set_pm_runtime_busy(struct tsl2583_chip *chip, bool on)
>  {

As below.

> -	int ret;
> +	if (on)
> +		return pm_runtime_resume_and_get(&chip->client->dev);
>  
> -	if (on) {
> -		ret = pm_runtime_resume_and_get(&chip->client->dev);
> -	} else {
> -		pm_runtime_mark_last_busy(&chip->client->dev);
> -		ret = pm_runtime_put_autosuspend(&chip->client->dev);
> -	}
> -
> -	return ret;
> +	return pm_runtime_put_autosuspend(&chip->client->dev);
>  }

> diff --git a/drivers/iio/light/us5182d.c b/drivers/iio/light/us5182d.c
> index 61a0957317a1..d2f5a44892a8 100644
> --- a/drivers/iio/light/us5182d.c
> +++ b/drivers/iio/light/us5182d.c
> @@ -361,19 +361,13 @@ static int us5182d_shutdown_en(struct us5182d_data *data, u8 state)
>  
>  static int us5182d_set_power_state(struct us5182d_data *data, bool on)

As below.

>  {
> -	int ret;
> -
>  	if (data->power_mode == US5182D_ONESHOT)
>  		return 0;
>  
> -	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 (on)
> +		return pm_runtime_resume_and_get(&data->client->dev);
>  
> -	return ret;
> +	return pm_runtime_put_autosuspend(&data->client->dev);
>  }
>  
>  static int us5182d_read_value(struct us5182d_data *data,
> diff --git a/drivers/iio/light/vcnl4000.c b/drivers/iio/light/vcnl4000.c
> index cc81a30b7c70..4dbb2294a843 100644
> --- a/drivers/iio/light/vcnl4000.c
> +++ b/drivers/iio/light/vcnl4000.c
> @@ -576,16 +576,11 @@ static bool vcnl4010_is_in_periodic_mode(struct vcnl4000_data *data)
>  static int vcnl4000_set_pm_runtime_state(struct vcnl4000_data *data, bool on)

As below.

>  {
>  	struct device *dev = &data->client->dev;
> -	int ret;
>  
> -	if (on) {
> -		ret = pm_runtime_resume_and_get(dev);
> -	} else {
> -		pm_runtime_mark_last_busy(dev);
> -		ret = pm_runtime_put_autosuspend(dev);
> -	}
> +	if (on)
> +		return pm_runtime_resume_and_get(dev);
>  
> -	return ret;
> +	return pm_runtime_put_autosuspend(dev);
>  }
>  
>  static int vcnl4040_read_als_it(struct vcnl4000_data *data, int *val, int *val2)
> diff --git a/drivers/iio/light/vcnl4035.c b/drivers/iio/light/vcnl4035.c
> index 01bc99564f98..963747927425 100644
> --- a/drivers/iio/light/vcnl4035.c
> +++ b/drivers/iio/light/vcnl4035.c
> @@ -141,17 +141,12 @@ static const struct iio_trigger_ops vcnl4035_trigger_ops = {
>  
>  static int vcnl4035_set_pm_runtime_state(struct vcnl4035_data *data, bool on)

I'd like to get rid of this function.  It was of marginal benefit before it got
even simpler, not it is actively making the code worse to read.

>  {
> -	int ret;
>  	struct device *dev = &data->client->dev;
>  
> -	if (on) {
> -		ret = pm_runtime_resume_and_get(dev);
> -	} else {
> -		pm_runtime_mark_last_busy(dev);
> -		ret = pm_runtime_put_autosuspend(dev);
> -	}
> +	if (on)
> +		return pm_runtime_resume_and_get(dev);
>  
> -	return ret;
> +	return pm_runtime_put_autosuspend(dev);
>  }
>  
>  static int vcnl4035_read_info_raw(struct iio_dev *indio_dev,