diff mbox series

[02/28] iio: light: isl29028: Balance runtime pm + use pm_runtime_resume_and_get()

Message ID 20210509113354.660190-3-jic23@kernel.org (mailing list archive)
State New, archived
Headers show
Series IIO: Runtime PM related cleanups. | expand

Commit Message

Jonathan Cameron May 9, 2021, 11:33 a.m. UTC
From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

In remove this driver called pm_runtime_put_noidle() but there is
no matching get operation.  This does not cause any problems because
the reference counter will not change if already zero, but it
does make the code harder to reason about so should be dropped.

Whilst we are here, use pm_runtime_resume_and_get() to replace open
coded version.
Found using coccicheck script under review at:
https://lore.kernel.org/lkml/20210427141946.2478411-1-Julia.Lawall@inria.fr/

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Cc: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
---
 drivers/iio/light/isl29028.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

Comments

Mauro Carvalho Chehab May 12, 2021, 1:33 p.m. UTC | #1
Em Sun,  9 May 2021 12:33:28 +0100
Jonathan Cameron <jic23@kernel.org> escreveu:

> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> In remove this driver called pm_runtime_put_noidle() but there is
> no matching get operation.  This does not cause any problems because
> the reference counter will not change if already zero, but it
> does make the code harder to reason about so should be dropped.
> 
> Whilst we are here, use pm_runtime_resume_and_get() to replace open
> coded version.
> Found using coccicheck script under review at:
> https://lore.kernel.org/lkml/20210427141946.2478411-1-Julia.Lawall@inria.fr/
> 
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Cc: Sebastian Reichel <sebastian.reichel@collabora.co.uk>

LGTM.

Reviewed-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

> ---
>  drivers/iio/light/isl29028.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/drivers/iio/light/isl29028.c b/drivers/iio/light/isl29028.c
> index 2f8b494f3e08..9de3262aa688 100644
> --- a/drivers/iio/light/isl29028.c
> +++ b/drivers/iio/light/isl29028.c
> @@ -339,9 +339,7 @@ static int isl29028_set_pm_runtime_busy(struct isl29028_chip *chip, bool on)
>  	int ret;
>  
>  	if (on) {
> -		ret = pm_runtime_get_sync(dev);
> -		if (ret < 0)
> -			pm_runtime_put_noidle(dev);
> +		ret = pm_runtime_resume_and_get(dev);
>  	} else {
>  		pm_runtime_mark_last_busy(dev);
>  		ret = pm_runtime_put_autosuspend(dev);
> @@ -647,7 +645,6 @@ static int isl29028_remove(struct i2c_client *client)
>  
>  	pm_runtime_disable(&client->dev);
>  	pm_runtime_set_suspended(&client->dev);
> -	pm_runtime_put_noidle(&client->dev);
>  
>  	return isl29028_clear_configure_reg(chip);
>  }



Thanks,
Mauro
Jonathan Cameron May 13, 2021, 4:37 p.m. UTC | #2
On Wed, 12 May 2021 15:33:35 +0200
Mauro Carvalho Chehab <mchehab+huawei@kernel.org> wrote:

> Em Sun,  9 May 2021 12:33:28 +0100
> Jonathan Cameron <jic23@kernel.org> escreveu:
> 
> > From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > 
> > In remove this driver called pm_runtime_put_noidle() but there is
> > no matching get operation.  This does not cause any problems because
> > the reference counter will not change if already zero, but it
> > does make the code harder to reason about so should be dropped.
> > 
> > Whilst we are here, use pm_runtime_resume_and_get() to replace open
> > coded version.
> > Found using coccicheck script under review at:
> > https://lore.kernel.org/lkml/20210427141946.2478411-1-Julia.Lawall@inria.fr/
> > 
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > Cc: Sebastian Reichel <sebastian.reichel@collabora.co.uk>  
> 
> LGTM.
> 
> Reviewed-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

Applied to the togreg branch of iio.git

Thanks,

Jonathan

> 
> > ---
> >  drivers/iio/light/isl29028.c | 5 +----
> >  1 file changed, 1 insertion(+), 4 deletions(-)
> > 
> > diff --git a/drivers/iio/light/isl29028.c b/drivers/iio/light/isl29028.c
> > index 2f8b494f3e08..9de3262aa688 100644
> > --- a/drivers/iio/light/isl29028.c
> > +++ b/drivers/iio/light/isl29028.c
> > @@ -339,9 +339,7 @@ static int isl29028_set_pm_runtime_busy(struct isl29028_chip *chip, bool on)
> >  	int ret;
> >  
> >  	if (on) {
> > -		ret = pm_runtime_get_sync(dev);
> > -		if (ret < 0)
> > -			pm_runtime_put_noidle(dev);
> > +		ret = pm_runtime_resume_and_get(dev);
> >  	} else {
> >  		pm_runtime_mark_last_busy(dev);
> >  		ret = pm_runtime_put_autosuspend(dev);
> > @@ -647,7 +645,6 @@ static int isl29028_remove(struct i2c_client *client)
> >  
> >  	pm_runtime_disable(&client->dev);
> >  	pm_runtime_set_suspended(&client->dev);
> > -	pm_runtime_put_noidle(&client->dev);
> >  
> >  	return isl29028_clear_configure_reg(chip);
> >  }  
> 
> 
> 
> Thanks,
> Mauro
Linus Walleij May 17, 2021, 9:44 p.m. UTC | #3
On Sun, May 9, 2021 at 1:36 PM Jonathan Cameron <jic23@kernel.org> wrote:

> Whilst we are here, use pm_runtime_resume_and_get() to replace open
> coded version.

Wow that is a new API. Why didn't I know about it...
Oh it is only from november last year that is why.
I guess we should switch to this everywhere.

> Found using coccicheck script under review at:
> https://lore.kernel.org/lkml/20210427141946.2478411-1-Julia.Lawall@inria.fr/
>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Cc: Sebastian Reichel <sebastian.reichel@collabora.co.uk>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij
diff mbox series

Patch

diff --git a/drivers/iio/light/isl29028.c b/drivers/iio/light/isl29028.c
index 2f8b494f3e08..9de3262aa688 100644
--- a/drivers/iio/light/isl29028.c
+++ b/drivers/iio/light/isl29028.c
@@ -339,9 +339,7 @@  static int isl29028_set_pm_runtime_busy(struct isl29028_chip *chip, bool on)
 	int ret;
 
 	if (on) {
-		ret = pm_runtime_get_sync(dev);
-		if (ret < 0)
-			pm_runtime_put_noidle(dev);
+		ret = pm_runtime_resume_and_get(dev);
 	} else {
 		pm_runtime_mark_last_busy(dev);
 		ret = pm_runtime_put_autosuspend(dev);
@@ -647,7 +645,6 @@  static int isl29028_remove(struct i2c_client *client)
 
 	pm_runtime_disable(&client->dev);
 	pm_runtime_set_suspended(&client->dev);
-	pm_runtime_put_noidle(&client->dev);
 
 	return isl29028_clear_configure_reg(chip);
 }