diff mbox

[1/2] PM / Domains: Remove redundant pm_runtime_get|put*() in pm_genpd_prepare()

Message ID 1461312955-8547-2-git-send-email-ulf.hansson@linaro.org (mailing list archive)
State Not Applicable, archived
Headers show

Commit Message

Ulf Hansson April 22, 2016, 8:15 a.m. UTC
The PM core increases the runtime PM usage count in the system PM prepare
phase. This makes some of the pm_runtime_get|put*() calls in
pm_gendp_prepare() redundant, so let's remove them.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/base/power/domain.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

Comments

Kevin Hilman April 22, 2016, 6:43 p.m. UTC | #1
Ulf Hansson <ulf.hansson@linaro.org> writes:

> The PM core increases the runtime PM usage count in the system PM prepare
> phase.

nit: increases/decreases ... prepare/complete

> This makes some of the pm_runtime_get|put*() calls in
> pm_gendp_prepare() redundant, so let's remove them.

s/gendp/genpd/

And also add pm_genpd_complete()

> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>

Otherwise...

Reviewed-by: Kevin Hilman <khilman@baylibre.com>

> ---
>  drivers/base/power/domain.c | 10 ++--------
>  1 file changed, 2 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index 4ce4ce0..60a3573 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -730,14 +730,11 @@ static int pm_genpd_prepare(struct device *dev)
>  	 * at this point and a system wakeup event should be reported if it's
>  	 * set up to wake up the system from sleep states.
>  	 */
> -	pm_runtime_get_noresume(dev);
>  	if (pm_runtime_barrier(dev) && device_may_wakeup(dev))
>  		pm_wakeup_event(dev, 0);
>  
> -	if (pm_wakeup_pending()) {
> -		pm_runtime_put(dev);
> +	if (pm_wakeup_pending())
>  		return -EBUSY;
> -	}
>  
>  	if (resume_needed(dev, genpd))
>  		pm_runtime_resume(dev);
> @@ -751,10 +748,8 @@ static int pm_genpd_prepare(struct device *dev)
>  
>  	mutex_unlock(&genpd->lock);
>  
> -	if (genpd->suspend_power_off) {
> -		pm_runtime_put_noidle(dev);
> +	if (genpd->suspend_power_off)
>  		return 0;
> -	}
>  
>  	/*
>  	 * The PM domain must be in the GPD_STATE_ACTIVE state at this point,
> @@ -776,7 +771,6 @@ static int pm_genpd_prepare(struct device *dev)
>  		pm_runtime_enable(dev);
>  	}
>  
> -	pm_runtime_put(dev);
>  	return ret;
>  }
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ulf Hansson April 25, 2016, 8:21 a.m. UTC | #2
On 22 April 2016 at 20:43, Kevin Hilman <khilman@baylibre.com> wrote:
> Ulf Hansson <ulf.hansson@linaro.org> writes:
>
>> The PM core increases the runtime PM usage count in the system PM prepare
>> phase.
>
> nit: increases/decreases ... prepare/complete
>
>> This makes some of the pm_runtime_get|put*() calls in
>> pm_gendp_prepare() redundant, so let's remove them.
>
> s/gendp/genpd/
>
> And also add pm_genpd_complete()

As a matter fact that isn't the case. There's no pm_runtime_put*() in
pm_genpd_complete().

Although, perhaps you were thinking about enabling/disabling of
runtime PM, as that is being done in pm_genpd_prepare() and
pm_genpd_complete(). I intend to change as well, but in a separate
patch.

>
>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>
> Otherwise...
>
> Reviewed-by: Kevin Hilman <khilman@baylibre.com>
>

Thanks for reviewing, I will update the change log accordingly!

Kind regards
Uffe

>> ---
>>  drivers/base/power/domain.c | 10 ++--------
>>  1 file changed, 2 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
>> index 4ce4ce0..60a3573 100644
>> --- a/drivers/base/power/domain.c
>> +++ b/drivers/base/power/domain.c
>> @@ -730,14 +730,11 @@ static int pm_genpd_prepare(struct device *dev)
>>        * at this point and a system wakeup event should be reported if it's
>>        * set up to wake up the system from sleep states.
>>        */
>> -     pm_runtime_get_noresume(dev);
>>       if (pm_runtime_barrier(dev) && device_may_wakeup(dev))
>>               pm_wakeup_event(dev, 0);
>>
>> -     if (pm_wakeup_pending()) {
>> -             pm_runtime_put(dev);
>> +     if (pm_wakeup_pending())
>>               return -EBUSY;
>> -     }
>>
>>       if (resume_needed(dev, genpd))
>>               pm_runtime_resume(dev);
>> @@ -751,10 +748,8 @@ static int pm_genpd_prepare(struct device *dev)
>>
>>       mutex_unlock(&genpd->lock);
>>
>> -     if (genpd->suspend_power_off) {
>> -             pm_runtime_put_noidle(dev);
>> +     if (genpd->suspend_power_off)
>>               return 0;
>> -     }
>>
>>       /*
>>        * The PM domain must be in the GPD_STATE_ACTIVE state at this point,
>> @@ -776,7 +771,6 @@ static int pm_genpd_prepare(struct device *dev)
>>               pm_runtime_enable(dev);
>>       }
>>
>> -     pm_runtime_put(dev);
>>       return ret;
>>  }
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rafael J. Wysocki April 25, 2016, 11:46 p.m. UTC | #3
On Monday, April 25, 2016 10:21:34 AM Ulf Hansson wrote:
> On 22 April 2016 at 20:43, Kevin Hilman <khilman@baylibre.com> wrote:
> > Ulf Hansson <ulf.hansson@linaro.org> writes:
> >
> >> The PM core increases the runtime PM usage count in the system PM prepare
> >> phase.
> >
> > nit: increases/decreases ... prepare/complete
> >
> >> This makes some of the pm_runtime_get|put*() calls in
> >> pm_gendp_prepare() redundant, so let's remove them.
> >
> > s/gendp/genpd/
> >
> > And also add pm_genpd_complete()
> 
> As a matter fact that isn't the case. There's no pm_runtime_put*() in
> pm_genpd_complete().
> 
> Although, perhaps you were thinking about enabling/disabling of
> runtime PM, as that is being done in pm_genpd_prepare() and
> pm_genpd_complete(). I intend to change as well, but in a separate
> patch.
> 
> >
> >> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> >
> > Otherwise...
> >
> > Reviewed-by: Kevin Hilman <khilman@baylibre.com>
> >
> 
> Thanks for reviewing, I will update the change log accordingly!

OK, I'll be expecting an update of the series then.

Thanks,
Rafael

--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Kevin Hilman April 27, 2016, 6:10 p.m. UTC | #4
Ulf Hansson <ulf.hansson@linaro.org> writes:

> On 22 April 2016 at 20:43, Kevin Hilman <khilman@baylibre.com> wrote:
>> Ulf Hansson <ulf.hansson@linaro.org> writes:
>>
>>> The PM core increases the runtime PM usage count in the system PM prepare
>>> phase.
>>
>> nit: increases/decreases ... prepare/complete
>>
>>> This makes some of the pm_runtime_get|put*() calls in
>>> pm_gendp_prepare() redundant, so let's remove them.
>>
>> s/gendp/genpd/
>>
>> And also add pm_genpd_complete()
>
> As a matter fact that isn't the case. There's no pm_runtime_put*() in
> pm_genpd_complete().
>
> Although, perhaps you were thinking about enabling/disabling of
> runtime PM, as that is being done in pm_genpd_prepare() and
> pm_genpd_complete(). I intend to change as well, but in a separate
> patch.

Actually, I was thinking of device_complete() where there is the _put()

>>
>>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>>
>> Otherwise...
>>
>> Reviewed-by: Kevin Hilman <khilman@baylibre.com>
>>
>
> Thanks for reviewing, I will update the change log accordingly!

Thanks,

Kevin
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 4ce4ce0..60a3573 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -730,14 +730,11 @@  static int pm_genpd_prepare(struct device *dev)
 	 * at this point and a system wakeup event should be reported if it's
 	 * set up to wake up the system from sleep states.
 	 */
-	pm_runtime_get_noresume(dev);
 	if (pm_runtime_barrier(dev) && device_may_wakeup(dev))
 		pm_wakeup_event(dev, 0);
 
-	if (pm_wakeup_pending()) {
-		pm_runtime_put(dev);
+	if (pm_wakeup_pending())
 		return -EBUSY;
-	}
 
 	if (resume_needed(dev, genpd))
 		pm_runtime_resume(dev);
@@ -751,10 +748,8 @@  static int pm_genpd_prepare(struct device *dev)
 
 	mutex_unlock(&genpd->lock);
 
-	if (genpd->suspend_power_off) {
-		pm_runtime_put_noidle(dev);
+	if (genpd->suspend_power_off)
 		return 0;
-	}
 
 	/*
 	 * The PM domain must be in the GPD_STATE_ACTIVE state at this point,
@@ -776,7 +771,6 @@  static int pm_genpd_prepare(struct device *dev)
 		pm_runtime_enable(dev);
 	}
 
-	pm_runtime_put(dev);
 	return ret;
 }