diff mbox series

[04/28] iio: accel: bmc150-accel: Balanced runtime pm + use pm_runtime_resume_and_get()

Message ID 20210509113354.660190-5-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>

A call to pm_runtime_put_noidle() doesn't match any call that would
result in a get().  It is safe because runtime pm core protects against
the reference counter going 0, but it makes it harder to understand the
code.

Whilst here use pm_runtime_resume_and_get() to tidy things up.
The Coccinelle script didn't get this one due to more complex code
structure.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Cc: Hans de Goede <hdegoede@redhat.com>
---
 drivers/iio/accel/bmc150-accel-core.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

Comments

Hans de Goede May 9, 2021, 1:52 p.m. UTC | #1
Hi,

On 5/9/21 1:33 PM, Jonathan Cameron wrote:
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> A call to pm_runtime_put_noidle() doesn't match any call that would
> result in a get().  It is safe because runtime pm core protects against
> the reference counter going 0, but it makes it harder to understand the
> code.
> 
> Whilst here use pm_runtime_resume_and_get() to tidy things up.
> The Coccinelle script didn't get this one due to more complex code
> structure.
> 
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Cc: Hans de Goede <hdegoede@redhat.com>

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans


> ---
>  drivers/iio/accel/bmc150-accel-core.c | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/drivers/iio/accel/bmc150-accel-core.c b/drivers/iio/accel/bmc150-accel-core.c
> index 62a164a7b852..43cfadf8f6b7 100644
> --- a/drivers/iio/accel/bmc150-accel-core.c
> +++ b/drivers/iio/accel/bmc150-accel-core.c
> @@ -389,7 +389,7 @@ static int bmc150_accel_set_power_state(struct bmc150_accel_data *data, bool on)
>  	int ret;
>  
>  	if (on) {
> -		ret = pm_runtime_get_sync(dev);
> +		ret = pm_runtime_resume_and_get(dev);
>  	} else {
>  		pm_runtime_mark_last_busy(dev);
>  		ret = pm_runtime_put_autosuspend(dev);
> @@ -398,9 +398,6 @@ static int bmc150_accel_set_power_state(struct bmc150_accel_data *data, bool on)
>  	if (ret < 0) {
>  		dev_err(dev,
>  			"Failed: %s for %d\n", __func__, on);
> -		if (on)
> -			pm_runtime_put_noidle(dev);
> -
>  		return ret;
>  	}
>  
> @@ -1836,7 +1833,6 @@ int bmc150_accel_core_remove(struct device *dev)
>  
>  	pm_runtime_disable(dev);
>  	pm_runtime_set_suspended(dev);
> -	pm_runtime_put_noidle(dev);
>  
>  	bmc150_accel_unregister_triggers(data, BMC150_ACCEL_TRIGGERS - 1);
>  
>
Mauro Carvalho Chehab May 12, 2021, 1:36 p.m. UTC | #2
Em Sun,  9 May 2021 12:33:30 +0100
Jonathan Cameron <jic23@kernel.org> escreveu:

> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> A call to pm_runtime_put_noidle() doesn't match any call that would
> result in a get().  It is safe because runtime pm core protects against
> the reference counter going 0, but it makes it harder to understand the
> code.
> 
> Whilst here use pm_runtime_resume_and_get() to tidy things up.
> The Coccinelle script didn't get this one due to more complex code
> structure.
> 
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Cc: Hans de Goede <hdegoede@redhat.com>

LGTM.

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


> ---
>  drivers/iio/accel/bmc150-accel-core.c | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/drivers/iio/accel/bmc150-accel-core.c b/drivers/iio/accel/bmc150-accel-core.c
> index 62a164a7b852..43cfadf8f6b7 100644
> --- a/drivers/iio/accel/bmc150-accel-core.c
> +++ b/drivers/iio/accel/bmc150-accel-core.c
> @@ -389,7 +389,7 @@ static int bmc150_accel_set_power_state(struct bmc150_accel_data *data, bool on)
>  	int ret;
>  
>  	if (on) {
> -		ret = pm_runtime_get_sync(dev);
> +		ret = pm_runtime_resume_and_get(dev);
>  	} else {
>  		pm_runtime_mark_last_busy(dev);
>  		ret = pm_runtime_put_autosuspend(dev);
> @@ -398,9 +398,6 @@ static int bmc150_accel_set_power_state(struct bmc150_accel_data *data, bool on)
>  	if (ret < 0) {
>  		dev_err(dev,
>  			"Failed: %s for %d\n", __func__, on);
> -		if (on)
> -			pm_runtime_put_noidle(dev);
> -
>  		return ret;
>  	}
>  
> @@ -1836,7 +1833,6 @@ int bmc150_accel_core_remove(struct device *dev)
>  
>  	pm_runtime_disable(dev);
>  	pm_runtime_set_suspended(dev);
> -	pm_runtime_put_noidle(dev);
>  
>  	bmc150_accel_unregister_triggers(data, BMC150_ACCEL_TRIGGERS - 1);
>  



Thanks,
Mauro
Jonathan Cameron May 13, 2021, 4:41 p.m. UTC | #3
On Sun, 9 May 2021 15:52:44 +0200
Hans de Goede <hdegoede@redhat.com> wrote:

> Hi,
> 
> On 5/9/21 1:33 PM, Jonathan Cameron wrote:
> > From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > 
> > A call to pm_runtime_put_noidle() doesn't match any call that would
> > result in a get().  It is safe because runtime pm core protects against
> > the reference counter going 0, but it makes it harder to understand the
> > code.
> > 
> > Whilst here use pm_runtime_resume_and_get() to tidy things up.
> > The Coccinelle script didn't get this one due to more complex code
> > structure.
> > 
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > Cc: Hans de Goede <hdegoede@redhat.com>  
> 
> Thanks, patch looks good to me:
> 
> Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Applied to the togreg branch of iio.git, pushed out as testing for all the
normal reasons.

Thanks,

Jonathan

> 
> Regards,
> 
> Hans
> 
> 
> > ---
> >  drivers/iio/accel/bmc150-accel-core.c | 6 +-----
> >  1 file changed, 1 insertion(+), 5 deletions(-)
> > 
> > diff --git a/drivers/iio/accel/bmc150-accel-core.c b/drivers/iio/accel/bmc150-accel-core.c
> > index 62a164a7b852..43cfadf8f6b7 100644
> > --- a/drivers/iio/accel/bmc150-accel-core.c
> > +++ b/drivers/iio/accel/bmc150-accel-core.c
> > @@ -389,7 +389,7 @@ static int bmc150_accel_set_power_state(struct bmc150_accel_data *data, bool on)
> >  	int ret;
> >  
> >  	if (on) {
> > -		ret = pm_runtime_get_sync(dev);
> > +		ret = pm_runtime_resume_and_get(dev);
> >  	} else {
> >  		pm_runtime_mark_last_busy(dev);
> >  		ret = pm_runtime_put_autosuspend(dev);
> > @@ -398,9 +398,6 @@ static int bmc150_accel_set_power_state(struct bmc150_accel_data *data, bool on)
> >  	if (ret < 0) {
> >  		dev_err(dev,
> >  			"Failed: %s for %d\n", __func__, on);
> > -		if (on)
> > -			pm_runtime_put_noidle(dev);
> > -
> >  		return ret;
> >  	}
> >  
> > @@ -1836,7 +1833,6 @@ int bmc150_accel_core_remove(struct device *dev)
> >  
> >  	pm_runtime_disable(dev);
> >  	pm_runtime_set_suspended(dev);
> > -	pm_runtime_put_noidle(dev);
> >  
> >  	bmc150_accel_unregister_triggers(data, BMC150_ACCEL_TRIGGERS - 1);
> >  
> >   
>
diff mbox series

Patch

diff --git a/drivers/iio/accel/bmc150-accel-core.c b/drivers/iio/accel/bmc150-accel-core.c
index 62a164a7b852..43cfadf8f6b7 100644
--- a/drivers/iio/accel/bmc150-accel-core.c
+++ b/drivers/iio/accel/bmc150-accel-core.c
@@ -389,7 +389,7 @@  static int bmc150_accel_set_power_state(struct bmc150_accel_data *data, bool on)
 	int ret;
 
 	if (on) {
-		ret = pm_runtime_get_sync(dev);
+		ret = pm_runtime_resume_and_get(dev);
 	} else {
 		pm_runtime_mark_last_busy(dev);
 		ret = pm_runtime_put_autosuspend(dev);
@@ -398,9 +398,6 @@  static int bmc150_accel_set_power_state(struct bmc150_accel_data *data, bool on)
 	if (ret < 0) {
 		dev_err(dev,
 			"Failed: %s for %d\n", __func__, on);
-		if (on)
-			pm_runtime_put_noidle(dev);
-
 		return ret;
 	}
 
@@ -1836,7 +1833,6 @@  int bmc150_accel_core_remove(struct device *dev)
 
 	pm_runtime_disable(dev);
 	pm_runtime_set_suspended(dev);
-	pm_runtime_put_noidle(dev);
 
 	bmc150_accel_unregister_triggers(data, BMC150_ACCEL_TRIGGERS - 1);