* [PATCH] PM / Domains: Extract code to power off/on a PM domain
@ 2014-11-10 18:39 Geert Uytterhoeven
2014-11-13 14:10 ` Pavel Machek
0 siblings, 1 reply; 3+ messages in thread
From: Geert Uytterhoeven @ 2014-11-10 18:39 UTC (permalink / raw)
To: Rafael J. Wysocki, Len Brown, Pavel Machek
Cc: linux-pm, linux-kernel, Geert Uytterhoeven
PM domains are powered on/off from various places. Some callers do
latency measurements, others don't. Consolidate using two helper
functions, which always measure the latencies, and update the stored
latencies when needed.
Other minor changes:
- Use pr_warn() instead of pr_warning(),
- There's no need to check genpd->name, %s handles NULL pointers fine,
- Make the warning format strings identical, to save memory.
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
Reviewed-by: Kevin Hilman <khilman@linaro.org>
---
v2:
- s/do_genpd_power_on/genpd_power_on/
- s/do_genpd_power_off/genpd_power_off/
- Add Reviewed-by
---
drivers/base/power/domain.c | 101 ++++++++++++++++++++++++++------------------
1 file changed, 60 insertions(+), 41 deletions(-)
diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index a841bab441147b3d..2fccf2c2575e4fa3 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -151,6 +151,59 @@ static void genpd_recalc_cpu_exit_latency(struct generic_pm_domain *genpd)
genpd->cpuidle_data->idle_state->exit_latency = usecs64;
}
+static int genpd_power_on(struct generic_pm_domain *genpd)
+{
+ ktime_t time_start;
+ s64 elapsed_ns;
+ int ret;
+
+ if (!genpd->power_on)
+ return 0;
+
+ time_start = ktime_get();
+ ret = genpd->power_on(genpd);
+ if (ret)
+ return ret;
+
+ elapsed_ns = ktime_to_ns(ktime_sub(ktime_get(), time_start));
+ if (elapsed_ns <= genpd->power_on_latency_ns)
+ return ret;
+
+ genpd->power_on_latency_ns = elapsed_ns;
+ genpd->max_off_time_changed = true;
+ genpd_recalc_cpu_exit_latency(genpd);
+ pr_warn("%s: Power-%s latency exceeded, new value %lld ns\n",
+ genpd->name, "on", elapsed_ns);
+
+ return ret;
+}
+
+static int genpd_power_off(struct generic_pm_domain *genpd)
+{
+ ktime_t time_start;
+ s64 elapsed_ns;
+ int ret;
+
+ if (!genpd->power_off)
+ return 0;
+
+ time_start = ktime_get();
+ ret = genpd->power_off(genpd);
+ if (ret == -EBUSY)
+ return ret;
+
+ elapsed_ns = ktime_to_ns(ktime_sub(ktime_get(), time_start));
+ if (elapsed_ns <= genpd->power_off_latency_ns)
+ return ret;
+
+ genpd->power_off_latency_ns = elapsed_ns;
+ genpd->max_off_time_changed = true;
+ pr_warn("%s: Power-%s latency exceeded, new value %lld ns\n",
+ genpd->name, "off", elapsed_ns);
+
+ return ret;
+}
+
/**
* __pm_genpd_poweron - Restore power to a given PM domain and its masters.
* @genpd: PM domain to power up.
@@ -222,25 +275,9 @@ static int __pm_genpd_poweron(struct generic_pm_domain *genpd)
}
}
- if (genpd->power_on) {
- ktime_t time_start = ktime_get();
- s64 elapsed_ns;
-
- ret = genpd->power_on(genpd);
- if (ret)
- goto err;
-
- elapsed_ns = ktime_to_ns(ktime_sub(ktime_get(), time_start));
- if (elapsed_ns > genpd->power_on_latency_ns) {
- genpd->power_on_latency_ns = elapsed_ns;
- genpd->max_off_time_changed = true;
- genpd_recalc_cpu_exit_latency(genpd);
- if (genpd->name)
- pr_warning("%s: Power-on latency exceeded, "
- "new value %lld ns\n", genpd->name,
- elapsed_ns);
- }
- }
+ ret = genpd_power_on(genpd);
+ if (ret)
+ goto err;
out:
genpd_set_active(genpd);
@@ -529,16 +566,11 @@ static int pm_genpd_poweroff(struct generic_pm_domain *genpd)
}
if (genpd->power_off) {
- ktime_t time_start;
- s64 elapsed_ns;
-
if (atomic_read(&genpd->sd_count) > 0) {
ret = -EBUSY;
goto out;
}
- time_start = ktime_get();
-
/*
* If sd_count > 0 at this point, one of the subdomains hasn't
* managed to call pm_genpd_poweron() for the master yet after
@@ -547,21 +579,11 @@ static int pm_genpd_poweroff(struct generic_pm_domain *genpd)
* the pm_genpd_poweron() restore power for us (this shouldn't
* happen very often).
*/
- ret = genpd->power_off(genpd);
+ ret = genpd_power_off(genpd);
if (ret == -EBUSY) {
genpd_set_active(genpd);
goto out;
}
-
- elapsed_ns = ktime_to_ns(ktime_sub(ktime_get(), time_start));
- if (elapsed_ns > genpd->power_off_latency_ns) {
- genpd->power_off_latency_ns = elapsed_ns;
- genpd->max_off_time_changed = true;
- if (genpd->name)
- pr_warning("%s: Power-off latency exceeded, "
- "new value %lld ns\n", genpd->name,
- elapsed_ns);
- }
}
genpd->status = GPD_STATE_POWER_OFF;
@@ -796,8 +818,7 @@ static void pm_genpd_sync_poweroff(struct generic_pm_domain *genpd)
|| atomic_read(&genpd->sd_count) > 0)
return;
- if (genpd->power_off)
- genpd->power_off(genpd);
+ genpd_power_off(genpd);
genpd->status = GPD_STATE_POWER_OFF;
@@ -828,8 +849,7 @@ static void pm_genpd_sync_poweron(struct generic_pm_domain *genpd)
genpd_sd_counter_inc(link->master);
}
- if (genpd->power_on)
- genpd->power_on(genpd);
+ genpd_power_on(genpd);
genpd->status = GPD_STATE_ACTIVE;
}
@@ -1251,8 +1271,7 @@ static int pm_genpd_restore_noirq(struct device *dev)
* If the domain was off before the hibernation, make
* sure it will be off going forward.
*/
- if (genpd->power_off)
- genpd->power_off(genpd);
+ genpd_power_off(genpd);
return 0;
}
--
1.9.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] PM / Domains: Extract code to power off/on a PM domain
2014-11-10 18:39 [PATCH] PM / Domains: Extract code to power off/on a PM domain Geert Uytterhoeven
@ 2014-11-13 14:10 ` Pavel Machek
2014-11-14 23:51 ` Rafael J. Wysocki
0 siblings, 1 reply; 3+ messages in thread
From: Pavel Machek @ 2014-11-13 14:10 UTC (permalink / raw)
To: Geert Uytterhoeven; +Cc: Rafael J. Wysocki, Len Brown, linux-pm, linux-kernel
On Mon 2014-11-10 19:39:19, Geert Uytterhoeven wrote:
> PM domains are powered on/off from various places. Some callers do
> latency measurements, others don't. Consolidate using two helper
> functions, which always measure the latencies, and update the stored
> latencies when needed.
>
> Other minor changes:
> - Use pr_warn() instead of pr_warning(),
> - There's no need to check genpd->name, %s handles NULL pointers fine,
> - Make the warning format strings identical, to save memory.
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
> Reviewed-by: Kevin Hilman <khilman@linaro.org>
Acked-by: Pavel Machek <pavel@ucw.cz>
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index a841bab441147b3d..2fccf2c2575e4fa3 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -151,6 +151,59 @@ static void genpd_recalc_cpu_exit_latency(struct generic_pm_domain *genpd)
> genpd->cpuidle_data->idle_state->exit_latency = usecs64;
> }
>
> +static int genpd_power_on(struct generic_pm_domain *genpd)
> +{
> + ktime_t time_start;
> + s64 elapsed_ns;
> + int ret;
> +
> + if (!genpd->power_on)
> + return 0;
> +
> + time_start = ktime_get();
> + ret = genpd->power_on(genpd);
> + if (ret)
> + return ret;
> +
> + elapsed_ns = ktime_to_ns(ktime_sub(ktime_get(), time_start));
> + if (elapsed_ns <= genpd->power_on_latency_ns)
> + return ret;
> +
> + genpd->power_on_latency_ns = elapsed_ns;
> + genpd->max_off_time_changed = true;
> + genpd_recalc_cpu_exit_latency(genpd);
> + pr_warn("%s: Power-%s latency exceeded, new value %lld ns\n",
> + genpd->name, "on", elapsed_ns);
> +
> + return ret;
> +}
> +
> +static int genpd_power_off(struct generic_pm_domain *genpd)
> +{
> + ktime_t time_start;
> + s64 elapsed_ns;
> + int ret;
> +
> + if (!genpd->power_off)
> + return 0;
> +
> + time_start = ktime_get();
> + ret = genpd->power_off(genpd);
> + if (ret == -EBUSY)
> + return ret;
> +
> + elapsed_ns = ktime_to_ns(ktime_sub(ktime_get(), time_start));
> + if (elapsed_ns <= genpd->power_off_latency_ns)
> + return ret;
> +
> + genpd->power_off_latency_ns = elapsed_ns;
> + genpd->max_off_time_changed = true;
> + pr_warn("%s: Power-%s latency exceeded, new value %lld ns\n",
> + genpd->name, "off", elapsed_ns);
> +
> + return ret;
> +}
> +
> /**
> * __pm_genpd_poweron - Restore power to a given PM domain and its masters.
> * @genpd: PM domain to power up.
> @@ -222,25 +275,9 @@ static int __pm_genpd_poweron(struct generic_pm_domain *genpd)
> }
> }
>
> - if (genpd->power_on) {
> - ktime_t time_start = ktime_get();
> - s64 elapsed_ns;
> -
> - ret = genpd->power_on(genpd);
> - if (ret)
> - goto err;
> -
> - elapsed_ns = ktime_to_ns(ktime_sub(ktime_get(), time_start));
> - if (elapsed_ns > genpd->power_on_latency_ns) {
> - genpd->power_on_latency_ns = elapsed_ns;
> - genpd->max_off_time_changed = true;
> - genpd_recalc_cpu_exit_latency(genpd);
> - if (genpd->name)
> - pr_warning("%s: Power-on latency exceeded, "
> - "new value %lld ns\n", genpd->name,
> - elapsed_ns);
> - }
> - }
> + ret = genpd_power_on(genpd);
> + if (ret)
> + goto err;
>
> out:
> genpd_set_active(genpd);
> @@ -529,16 +566,11 @@ static int pm_genpd_poweroff(struct generic_pm_domain *genpd)
> }
>
> if (genpd->power_off) {
> - ktime_t time_start;
> - s64 elapsed_ns;
> -
> if (atomic_read(&genpd->sd_count) > 0) {
> ret = -EBUSY;
> goto out;
> }
>
> - time_start = ktime_get();
> -
> /*
> * If sd_count > 0 at this point, one of the subdomains hasn't
> * managed to call pm_genpd_poweron() for the master yet after
> @@ -547,21 +579,11 @@ static int pm_genpd_poweroff(struct generic_pm_domain *genpd)
> * the pm_genpd_poweron() restore power for us (this shouldn't
> * happen very often).
> */
> - ret = genpd->power_off(genpd);
> + ret = genpd_power_off(genpd);
> if (ret == -EBUSY) {
> genpd_set_active(genpd);
> goto out;
> }
> -
> - elapsed_ns = ktime_to_ns(ktime_sub(ktime_get(), time_start));
> - if (elapsed_ns > genpd->power_off_latency_ns) {
> - genpd->power_off_latency_ns = elapsed_ns;
> - genpd->max_off_time_changed = true;
> - if (genpd->name)
> - pr_warning("%s: Power-off latency exceeded, "
> - "new value %lld ns\n", genpd->name,
> - elapsed_ns);
> - }
> }
>
> genpd->status = GPD_STATE_POWER_OFF;
> @@ -796,8 +818,7 @@ static void pm_genpd_sync_poweroff(struct generic_pm_domain *genpd)
> || atomic_read(&genpd->sd_count) > 0)
> return;
>
> - if (genpd->power_off)
> - genpd->power_off(genpd);
> + genpd_power_off(genpd);
>
> genpd->status = GPD_STATE_POWER_OFF;
>
> @@ -828,8 +849,7 @@ static void pm_genpd_sync_poweron(struct generic_pm_domain *genpd)
> genpd_sd_counter_inc(link->master);
> }
>
> - if (genpd->power_on)
> - genpd->power_on(genpd);
> + genpd_power_on(genpd);
>
> genpd->status = GPD_STATE_ACTIVE;
> }
> @@ -1251,8 +1271,7 @@ static int pm_genpd_restore_noirq(struct device *dev)
> * If the domain was off before the hibernation, make
> * sure it will be off going forward.
> */
> - if (genpd->power_off)
> - genpd->power_off(genpd);
> + genpd_power_off(genpd);
>
> return 0;
> }
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] PM / Domains: Extract code to power off/on a PM domain
2014-11-13 14:10 ` Pavel Machek
@ 2014-11-14 23:51 ` Rafael J. Wysocki
0 siblings, 0 replies; 3+ messages in thread
From: Rafael J. Wysocki @ 2014-11-14 23:51 UTC (permalink / raw)
To: Pavel Machek; +Cc: Geert Uytterhoeven, Len Brown, linux-pm, linux-kernel
On Thursday, November 13, 2014 03:10:07 PM Pavel Machek wrote:
> On Mon 2014-11-10 19:39:19, Geert Uytterhoeven wrote:
> > PM domains are powered on/off from various places. Some callers do
> > latency measurements, others don't. Consolidate using two helper
> > functions, which always measure the latencies, and update the stored
> > latencies when needed.
> >
> > Other minor changes:
> > - Use pr_warn() instead of pr_warning(),
> > - There's no need to check genpd->name, %s handles NULL pointers fine,
> > - Make the warning format strings identical, to save memory.
> >
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
> > Reviewed-by: Kevin Hilman <khilman@linaro.org>
>
> Acked-by: Pavel Machek <pavel@ucw.cz>
Patch queued up for 3.19, thanks!
> > diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> > index a841bab441147b3d..2fccf2c2575e4fa3 100644
> > --- a/drivers/base/power/domain.c
> > +++ b/drivers/base/power/domain.c
> > @@ -151,6 +151,59 @@ static void genpd_recalc_cpu_exit_latency(struct generic_pm_domain *genpd)
> > genpd->cpuidle_data->idle_state->exit_latency = usecs64;
> > }
> >
> > +static int genpd_power_on(struct generic_pm_domain *genpd)
> > +{
> > + ktime_t time_start;
> > + s64 elapsed_ns;
> > + int ret;
> > +
> > + if (!genpd->power_on)
> > + return 0;
> > +
> > + time_start = ktime_get();
> > + ret = genpd->power_on(genpd);
> > + if (ret)
> > + return ret;
> > +
> > + elapsed_ns = ktime_to_ns(ktime_sub(ktime_get(), time_start));
> > + if (elapsed_ns <= genpd->power_on_latency_ns)
> > + return ret;
> > +
> > + genpd->power_on_latency_ns = elapsed_ns;
> > + genpd->max_off_time_changed = true;
> > + genpd_recalc_cpu_exit_latency(genpd);
> > + pr_warn("%s: Power-%s latency exceeded, new value %lld ns\n",
> > + genpd->name, "on", elapsed_ns);
> > +
> > + return ret;
> > +}
> > +
> > +static int genpd_power_off(struct generic_pm_domain *genpd)
> > +{
> > + ktime_t time_start;
> > + s64 elapsed_ns;
> > + int ret;
> > +
> > + if (!genpd->power_off)
> > + return 0;
> > +
> > + time_start = ktime_get();
> > + ret = genpd->power_off(genpd);
> > + if (ret == -EBUSY)
> > + return ret;
> > +
> > + elapsed_ns = ktime_to_ns(ktime_sub(ktime_get(), time_start));
> > + if (elapsed_ns <= genpd->power_off_latency_ns)
> > + return ret;
> > +
> > + genpd->power_off_latency_ns = elapsed_ns;
> > + genpd->max_off_time_changed = true;
> > + pr_warn("%s: Power-%s latency exceeded, new value %lld ns\n",
> > + genpd->name, "off", elapsed_ns);
> > +
> > + return ret;
> > +}
> > +
> > /**
> > * __pm_genpd_poweron - Restore power to a given PM domain and its masters.
> > * @genpd: PM domain to power up.
> > @@ -222,25 +275,9 @@ static int __pm_genpd_poweron(struct generic_pm_domain *genpd)
> > }
> > }
> >
> > - if (genpd->power_on) {
> > - ktime_t time_start = ktime_get();
> > - s64 elapsed_ns;
> > -
> > - ret = genpd->power_on(genpd);
> > - if (ret)
> > - goto err;
> > -
> > - elapsed_ns = ktime_to_ns(ktime_sub(ktime_get(), time_start));
> > - if (elapsed_ns > genpd->power_on_latency_ns) {
> > - genpd->power_on_latency_ns = elapsed_ns;
> > - genpd->max_off_time_changed = true;
> > - genpd_recalc_cpu_exit_latency(genpd);
> > - if (genpd->name)
> > - pr_warning("%s: Power-on latency exceeded, "
> > - "new value %lld ns\n", genpd->name,
> > - elapsed_ns);
> > - }
> > - }
> > + ret = genpd_power_on(genpd);
> > + if (ret)
> > + goto err;
> >
> > out:
> > genpd_set_active(genpd);
> > @@ -529,16 +566,11 @@ static int pm_genpd_poweroff(struct generic_pm_domain *genpd)
> > }
> >
> > if (genpd->power_off) {
> > - ktime_t time_start;
> > - s64 elapsed_ns;
> > -
> > if (atomic_read(&genpd->sd_count) > 0) {
> > ret = -EBUSY;
> > goto out;
> > }
> >
> > - time_start = ktime_get();
> > -
> > /*
> > * If sd_count > 0 at this point, one of the subdomains hasn't
> > * managed to call pm_genpd_poweron() for the master yet after
> > @@ -547,21 +579,11 @@ static int pm_genpd_poweroff(struct generic_pm_domain *genpd)
> > * the pm_genpd_poweron() restore power for us (this shouldn't
> > * happen very often).
> > */
> > - ret = genpd->power_off(genpd);
> > + ret = genpd_power_off(genpd);
> > if (ret == -EBUSY) {
> > genpd_set_active(genpd);
> > goto out;
> > }
> > -
> > - elapsed_ns = ktime_to_ns(ktime_sub(ktime_get(), time_start));
> > - if (elapsed_ns > genpd->power_off_latency_ns) {
> > - genpd->power_off_latency_ns = elapsed_ns;
> > - genpd->max_off_time_changed = true;
> > - if (genpd->name)
> > - pr_warning("%s: Power-off latency exceeded, "
> > - "new value %lld ns\n", genpd->name,
> > - elapsed_ns);
> > - }
> > }
> >
> > genpd->status = GPD_STATE_POWER_OFF;
> > @@ -796,8 +818,7 @@ static void pm_genpd_sync_poweroff(struct generic_pm_domain *genpd)
> > || atomic_read(&genpd->sd_count) > 0)
> > return;
> >
> > - if (genpd->power_off)
> > - genpd->power_off(genpd);
> > + genpd_power_off(genpd);
> >
> > genpd->status = GPD_STATE_POWER_OFF;
> >
> > @@ -828,8 +849,7 @@ static void pm_genpd_sync_poweron(struct generic_pm_domain *genpd)
> > genpd_sd_counter_inc(link->master);
> > }
> >
> > - if (genpd->power_on)
> > - genpd->power_on(genpd);
> > + genpd_power_on(genpd);
> >
> > genpd->status = GPD_STATE_ACTIVE;
> > }
> > @@ -1251,8 +1271,7 @@ static int pm_genpd_restore_noirq(struct device *dev)
> > * If the domain was off before the hibernation, make
> > * sure it will be off going forward.
> > */
> > - if (genpd->power_off)
> > - genpd->power_off(genpd);
> > + genpd_power_off(genpd);
> >
> > return 0;
> > }
>
>
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2014-11-14 23:30 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-11-10 18:39 [PATCH] PM / Domains: Extract code to power off/on a PM domain Geert Uytterhoeven
2014-11-13 14:10 ` Pavel Machek
2014-11-14 23:51 ` Rafael J. Wysocki
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).