From f97238373b8662a6d580e204df2e7bcbfa43e27a Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Sun, 24 Jan 2016 20:08:52 -0600 Subject: PM / sleep: declare __tracedata symbols as char[] rather than char Accessing more than one byte from a symbol declared simply 'char' is undefined behavior, as reported by UBSAN: UBSAN: Undefined behaviour in drivers/base/power/trace.c:178:18 load of address ffffffff8203fc78 with insufficient space for an object of type 'char' Avoid this by declaring the symbols as arrays. Signed-off-by: Eric Biggers Signed-off-by: Rafael J. Wysocki --- drivers/base/power/trace.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/base') diff --git a/drivers/base/power/trace.c b/drivers/base/power/trace.c index a311cfa4c5bd..a6975795e7f3 100644 --- a/drivers/base/power/trace.c +++ b/drivers/base/power/trace.c @@ -166,14 +166,14 @@ void generate_pm_trace(const void *tracedata, unsigned int user) } EXPORT_SYMBOL(generate_pm_trace); -extern char __tracedata_start, __tracedata_end; +extern char __tracedata_start[], __tracedata_end[]; static int show_file_hash(unsigned int value) { int match; char *tracedata; match = 0; - for (tracedata = &__tracedata_start ; tracedata < &__tracedata_end ; + for (tracedata = __tracedata_start ; tracedata < __tracedata_end ; tracedata += 2 + sizeof(unsigned long)) { unsigned short lineno = *(unsigned short *)tracedata; const char *file = *(const char **)(tracedata + 2); -- cgit v1.2.3-55-g7522 From fc5cbf0c94b6f7fd62fdddff892207290510945d Mon Sep 17 00:00:00 2001 From: Axel Haslam Date: Mon, 15 Feb 2016 11:10:51 +0100 Subject: PM / Domains: Support for multiple states Some hardware (eg. OMAP), has the ability to enter different low power modes for a given power domain. This allows for more fine grained control over the power state of the platform. As a typical example, some registers of the hardware may be implemented with retention flip-flops and be able to retain their state at lower voltages allowing for faster on/off latencies and an increased window of opportunity to enter an intermediate low power state other than "off" When trying to set a power domain to off, the genpd governor will choose the deepest state that will respect the qos constraints of all the devices and sub-domains on the power domain. The state chosen by the governor is saved in the "state_idx" field of the generic_pm_domain structure and shall be used by the power_off and power_on callbacks to perform the necessary actions to set the power domain into (and out of) the state indicated by state_idx. States must be declared in ascending order from shallowest to deepest, deepest meaning the state which takes longer to enter and exit. For platforms that don't declare any states, a single a single "off" state is used. Once all platforms are converted to use the state array, the legacy on/off latencies will be removed. [ Lina: Modified genpd state initialization and remove use of save_state_latency_ns in genpd timing data ] Suggested-by: Kevin Hilman Signed-off-by: Lina Iyer Signed-off-by: Axel Haslam Acked-by: Ulf Hansson Signed-off-by: Rafael J. Wysocki --- drivers/base/power/domain.c | 39 +++++++++++++++++++--- drivers/base/power/domain_governor.c | 64 ++++++++++++++++++++++-------------- include/linux/pm_domain.h | 11 +++++++ 3 files changed, 85 insertions(+), 29 deletions(-) (limited to 'drivers/base') diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index 301b785f9f56..4c6f46b5c444 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -104,6 +104,7 @@ static void genpd_sd_counter_inc(struct generic_pm_domain *genpd) static int genpd_power_on(struct generic_pm_domain *genpd, bool timed) { + unsigned int state_idx = genpd->state_idx; ktime_t time_start; s64 elapsed_ns; int ret; @@ -120,10 +121,10 @@ static int genpd_power_on(struct generic_pm_domain *genpd, bool timed) return ret; elapsed_ns = ktime_to_ns(ktime_sub(ktime_get(), time_start)); - if (elapsed_ns <= genpd->power_on_latency_ns) + if (elapsed_ns <= genpd->states[state_idx].power_on_latency_ns) return ret; - genpd->power_on_latency_ns = elapsed_ns; + genpd->states[state_idx].power_on_latency_ns = elapsed_ns; genpd->max_off_time_changed = true; pr_debug("%s: Power-%s latency exceeded, new value %lld ns\n", genpd->name, "on", elapsed_ns); @@ -133,6 +134,7 @@ static int genpd_power_on(struct generic_pm_domain *genpd, bool timed) static int genpd_power_off(struct generic_pm_domain *genpd, bool timed) { + unsigned int state_idx = genpd->state_idx; ktime_t time_start; s64 elapsed_ns; int ret; @@ -149,10 +151,10 @@ static int genpd_power_off(struct generic_pm_domain *genpd, bool timed) return ret; elapsed_ns = ktime_to_ns(ktime_sub(ktime_get(), time_start)); - if (elapsed_ns <= genpd->power_off_latency_ns) + if (elapsed_ns <= genpd->states[state_idx].power_off_latency_ns) return ret; - genpd->power_off_latency_ns = elapsed_ns; + genpd->states[state_idx].power_off_latency_ns = elapsed_ns; genpd->max_off_time_changed = true; pr_debug("%s: Power-%s latency exceeded, new value %lld ns\n", genpd->name, "off", elapsed_ns); @@ -585,6 +587,8 @@ static void pm_genpd_sync_poweroff(struct generic_pm_domain *genpd, || atomic_read(&genpd->sd_count) > 0) return; + /* Choose the deepest state when suspending */ + genpd->state_idx = genpd->state_count - 1; genpd_power_off(genpd, timed); genpd->status = GPD_STATE_POWER_OFF; @@ -1508,6 +1512,26 @@ void pm_genpd_init(struct generic_pm_domain *genpd, genpd->dev_ops.start = pm_clk_resume; } + if (genpd->state_idx >= GENPD_MAX_NUM_STATES) { + pr_warn("Initial state index out of bounds.\n"); + genpd->state_idx = GENPD_MAX_NUM_STATES - 1; + } + + if (genpd->state_count > GENPD_MAX_NUM_STATES) { + pr_warn("Limiting states to %d\n", GENPD_MAX_NUM_STATES); + genpd->state_count = GENPD_MAX_NUM_STATES; + } + + /* Use only one "off" state if there were no states declared */ + if (genpd->state_count == 0) { + genpd->states[0].power_on_latency_ns = + genpd->power_on_latency_ns; + genpd->states[0].power_off_latency_ns = + genpd->power_off_latency_ns; + + genpd->state_count = 1; + } + mutex_lock(&gpd_list_lock); list_add(&genpd->gpd_list_node, &gpd_list); mutex_unlock(&gpd_list_lock); @@ -1872,7 +1896,12 @@ static int pm_genpd_summary_one(struct seq_file *s, if (WARN_ON(genpd->status >= ARRAY_SIZE(status_lookup))) goto exit; - seq_printf(s, "%-30s %-15s ", genpd->name, status_lookup[genpd->status]); + seq_printf(s, "%-30s %s", genpd->name, status_lookup[genpd->status]); + + if (genpd->status == GPD_STATE_POWER_OFF) + seq_printf(s, " %-13d ", genpd->state_idx); + else + seq_printf(s, " %-15s ", ""); /* * Modifications on the list require holding locks on both diff --git a/drivers/base/power/domain_governor.c b/drivers/base/power/domain_governor.c index 1e937ac5f456..00a5436dd44b 100644 --- a/drivers/base/power/domain_governor.c +++ b/drivers/base/power/domain_governor.c @@ -98,7 +98,8 @@ static bool default_stop_ok(struct device *dev) * * This routine must be executed under the PM domain's lock. */ -static bool default_power_down_ok(struct dev_pm_domain *pd) +static bool __default_power_down_ok(struct dev_pm_domain *pd, + unsigned int state) { struct generic_pm_domain *genpd = pd_to_genpd(pd); struct gpd_link *link; @@ -106,27 +107,9 @@ static bool default_power_down_ok(struct dev_pm_domain *pd) s64 min_off_time_ns; s64 off_on_time_ns; - if (genpd->max_off_time_changed) { - struct gpd_link *link; - - /* - * We have to invalidate the cached results for the masters, so - * use the observation that default_power_down_ok() is not - * going to be called for any master until this instance - * returns. - */ - list_for_each_entry(link, &genpd->slave_links, slave_node) - link->master->max_off_time_changed = true; - - genpd->max_off_time_changed = false; - genpd->cached_power_down_ok = false; - genpd->max_off_time_ns = -1; - } else { - return genpd->cached_power_down_ok; - } + off_on_time_ns = genpd->states[state].power_off_latency_ns + + genpd->states[state].power_on_latency_ns; - off_on_time_ns = genpd->power_off_latency_ns + - genpd->power_on_latency_ns; min_off_time_ns = -1; /* @@ -186,8 +169,6 @@ static bool default_power_down_ok(struct dev_pm_domain *pd) min_off_time_ns = constraint_ns; } - genpd->cached_power_down_ok = true; - /* * If the computed minimum device off time is negative, there are no * latency constraints, so the domain can spend arbitrary time in the @@ -201,10 +182,45 @@ static bool default_power_down_ok(struct dev_pm_domain *pd) * time and the time needed to turn the domain on is the maximum * theoretical time this domain can spend in the "off" state. */ - genpd->max_off_time_ns = min_off_time_ns - genpd->power_on_latency_ns; + genpd->max_off_time_ns = min_off_time_ns - + genpd->states[state].power_on_latency_ns; return true; } +static bool default_power_down_ok(struct dev_pm_domain *pd) +{ + struct generic_pm_domain *genpd = pd_to_genpd(pd); + struct gpd_link *link; + + if (!genpd->max_off_time_changed) + return genpd->cached_power_down_ok; + + /* + * We have to invalidate the cached results for the masters, so + * use the observation that default_power_down_ok() is not + * going to be called for any master until this instance + * returns. + */ + list_for_each_entry(link, &genpd->slave_links, slave_node) + link->master->max_off_time_changed = true; + + genpd->max_off_time_ns = -1; + genpd->max_off_time_changed = false; + genpd->cached_power_down_ok = true; + genpd->state_idx = genpd->state_count - 1; + + /* Find a state to power down to, starting from the deepest. */ + while (!__default_power_down_ok(pd, genpd->state_idx)) { + if (genpd->state_idx == 0) { + genpd->cached_power_down_ok = false; + break; + } + genpd->state_idx--; + } + + return genpd->cached_power_down_ok; +} + static bool always_on_power_down_ok(struct dev_pm_domain *domain) { return false; diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h index db21d3995f7e..1726c4ac6529 100644 --- a/include/linux/pm_domain.h +++ b/include/linux/pm_domain.h @@ -19,6 +19,8 @@ /* Defines used for the flags field in the struct generic_pm_domain */ #define GENPD_FLAG_PM_CLK (1U << 0) /* PM domain uses PM clk */ +#define GENPD_MAX_NUM_STATES 8 /* Number of possible low power states */ + enum gpd_status { GPD_STATE_ACTIVE = 0, /* PM domain is active */ GPD_STATE_POWER_OFF, /* PM domain is off */ @@ -37,6 +39,11 @@ struct gpd_dev_ops { bool (*active_wakeup)(struct device *dev); }; +struct genpd_power_state { + s64 power_off_latency_ns; + s64 power_on_latency_ns; +}; + struct generic_pm_domain { struct dev_pm_domain domain; /* PM domain operations */ struct list_head gpd_list_node; /* Node in the global PM domains list */ @@ -66,6 +73,10 @@ struct generic_pm_domain { void (*detach_dev)(struct generic_pm_domain *domain, struct device *dev); unsigned int flags; /* Bit field of configs for genpd */ + struct genpd_power_state states[GENPD_MAX_NUM_STATES]; + unsigned int state_count; /* number of states */ + unsigned int state_idx; /* state that genpd will go to when off */ + }; static inline struct generic_pm_domain *pd_to_genpd(struct dev_pm_domain *pd) -- cgit v1.2.3-55-g7522 From 90e63452ac3a42c9ff10b12880429e8592cf39ec Mon Sep 17 00:00:00 2001 From: Axel Haslam Date: Mon, 15 Feb 2016 11:10:53 +0100 Subject: PM / Domains: remove old power on/off latencies Now that all known users have been converted to use state latencies, we can remove the latency field in the generic_pm_domain structure. Signed-off-by: Axel Haslam Acked-by: Ulf Hansson Signed-off-by: Rafael J. Wysocki --- drivers/base/power/domain.c | 8 +------- include/linux/pm_domain.h | 2 -- 2 files changed, 1 insertion(+), 9 deletions(-) (limited to 'drivers/base') diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index 4c6f46b5c444..e8ca290dbf9d 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -1523,14 +1523,8 @@ void pm_genpd_init(struct generic_pm_domain *genpd, } /* Use only one "off" state if there were no states declared */ - if (genpd->state_count == 0) { - genpd->states[0].power_on_latency_ns = - genpd->power_on_latency_ns; - genpd->states[0].power_off_latency_ns = - genpd->power_off_latency_ns; - + if (genpd->state_count == 0) genpd->state_count = 1; - } mutex_lock(&gpd_list_lock); list_add(&genpd->gpd_list_node, &gpd_list); diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h index 1726c4ac6529..49cd8890b873 100644 --- a/include/linux/pm_domain.h +++ b/include/linux/pm_domain.h @@ -61,9 +61,7 @@ struct generic_pm_domain { unsigned int prepared_count; /* Suspend counter of prepared devices */ bool suspend_power_off; /* Power status before system suspend */ int (*power_off)(struct generic_pm_domain *domain); - s64 power_off_latency_ns; int (*power_on)(struct generic_pm_domain *domain); - s64 power_on_latency_ns; struct gpd_dev_ops dev_ops; s64 max_off_time_ns; /* Maximum allowed "suspended" time. */ bool max_off_time_changed; -- cgit v1.2.3-55-g7522 From 6954d43292674aaacc9b1cdbc28b1b2ea0cc6445 Mon Sep 17 00:00:00 2001 From: Geert Uytterhoeven Date: Tue, 23 Feb 2016 17:49:17 +0100 Subject: PM / Domains: Restore alignment of slaves in debugfs output The slave domains are no longer aligned with the table header in the /sys/kernel/debug/pm_genpd/pm_genpd_summary output. Worse, the alignment differs depending on the actual name of the state. Format the state name and index into a buffer, and print that like before to restore alignment. Use "%u" for unsigned int while we're at it. Fixes: fc5cbf0c94b6f7fd (PM / Domains: Support for multiple states) Signed-off-by: Geert Uytterhoeven Acked-by: Ulf Hansson Tested-by: Kevin Hilman Signed-off-by: Rafael J. Wysocki --- drivers/base/power/domain.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) (limited to 'drivers/base') diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index e8ca290dbf9d..01015d85ab64 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -1882,6 +1882,7 @@ static int pm_genpd_summary_one(struct seq_file *s, struct pm_domain_data *pm_data; const char *kobj_path; struct gpd_link *link; + char state[16]; int ret; ret = mutex_lock_interruptible(&genpd->lock); @@ -1890,12 +1891,13 @@ static int pm_genpd_summary_one(struct seq_file *s, if (WARN_ON(genpd->status >= ARRAY_SIZE(status_lookup))) goto exit; - seq_printf(s, "%-30s %s", genpd->name, status_lookup[genpd->status]); - if (genpd->status == GPD_STATE_POWER_OFF) - seq_printf(s, " %-13d ", genpd->state_idx); + snprintf(state, sizeof(state), "%s %u", + status_lookup[genpd->status], genpd->state_idx); else - seq_printf(s, " %-15s ", ""); + snprintf(state, sizeof(state), "%s", + status_lookup[genpd->status]); + seq_printf(s, "%-30s %-15s ", genpd->name, state); /* * Modifications on the list require holding locks on both -- cgit v1.2.3-55-g7522 From 0ba554e45c674181a3e5c42c7a6cccbbc75a0dd7 Mon Sep 17 00:00:00 2001 From: Geert Uytterhoeven Date: Tue, 23 Feb 2016 17:49:18 +0100 Subject: PM / Domains: Join state name and index in debugfs output For low-power states, the state index is part of the state, hence join them with a hyphen in the /sys/kernel/debug/pm_genpd/pm_genpd_summary output. E.g. "off 0" becomes "off-0". Signed-off-by: Geert Uytterhoeven Acked-by: Ulf Hansson Tested-by: Kevin Hilman Signed-off-by: Rafael J. Wysocki --- drivers/base/power/domain.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/base') diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index 01015d85ab64..9e59543d314c 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -1892,7 +1892,7 @@ static int pm_genpd_summary_one(struct seq_file *s, if (WARN_ON(genpd->status >= ARRAY_SIZE(status_lookup))) goto exit; if (genpd->status == GPD_STATE_POWER_OFF) - snprintf(state, sizeof(state), "%s %u", + snprintf(state, sizeof(state), "%s-%u", status_lookup[genpd->status], genpd->state_idx); else snprintf(state, sizeof(state), "%s", -- cgit v1.2.3-55-g7522 From 076395cae20381340a0e92ad3d76fe3e280f8a1c Mon Sep 17 00:00:00 2001 From: Laurent Pinchart Date: Wed, 2 Mar 2016 01:20:38 +0200 Subject: PM / Domains: Propagate start and restore errors during runtime resume During runtime resume the return values of the start and restore steps are ignored. As a result drivers are not notified of runtime resume failures and can't propagate them up. Fix it by returning an error if either the start or restore step fails, and clean up properly in the error path. Signed-off-by: Laurent Pinchart Acked-by: Kevin Hilman Acked-by: Ulf Hansson Signed-off-by: Rafael J. Wysocki --- drivers/base/power/domain.c | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) (limited to 'drivers/base') diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index 9e59543d314c..7e44ae3366db 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -487,8 +487,13 @@ static int pm_genpd_runtime_resume(struct device *dev) if (timed && runtime_pm) time_start = ktime_get(); - genpd_start_dev(genpd, dev); - genpd_restore_dev(genpd, dev); + ret = genpd_start_dev(genpd, dev); + if (ret) + goto err_poweroff; + + ret = genpd_restore_dev(genpd, dev); + if (ret) + goto err_stop; /* Update resume latency value if the measured time exceeds it. */ if (timed && runtime_pm) { @@ -503,6 +508,17 @@ static int pm_genpd_runtime_resume(struct device *dev) } return 0; + +err_stop: + genpd_stop_dev(genpd, dev); +err_poweroff: + if (!dev->power.irq_safe) { + mutex_lock(&genpd->lock); + genpd_poweroff(genpd, 0); + mutex_unlock(&genpd->lock); + } + + return ret; } static bool pd_ignore_unused; -- cgit v1.2.3-55-g7522 From beda5fc1ff9b527059290a97b672d2ee0eb7b92f Mon Sep 17 00:00:00 2001 From: Jon Hunter Date: Fri, 4 Mar 2016 10:55:14 +0000 Subject: PM / Domains: Fix removal of a subdomain Commit 30e7a65b3fdb (PM / Domains: Ensure subdomain is not in use before removing) added a test to ensure that a subdomain is not a master to another subdomain or if any devices are using the subdomain before removing. This change incorrectly used the "slave_links" list to determine if the subdomain is a master to another subdomain, where it should have been using the "master_links" list instead. The "slave_links" list will never be empty for a subdomain and so a subdomain can never be removed. Fix this by testing if the "master_links" list is empty instead. Fixes: 30e7a65b3fdb (PM / Domains: Ensure subdomain is not in use before removing) Signed-off-by: Jon Hunter Reviewed-by: Thierry Reding Acked-by: Ulf Hansson Acked-by: Kevin Hilman Signed-off-by: Rafael J. Wysocki --- drivers/base/power/domain.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/base') diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index 7e44ae3366db..79f5d3965931 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -1398,7 +1398,7 @@ int pm_genpd_remove_subdomain(struct generic_pm_domain *genpd, mutex_lock(&subdomain->lock); mutex_lock_nested(&genpd->lock, SINGLE_DEPTH_NESTING); - if (!list_empty(&subdomain->slave_links) || subdomain->device_count) { + if (!list_empty(&subdomain->master_links) || subdomain->device_count) { pr_warn("%s: unable to remove subdomain %s\n", genpd->name, subdomain->name); ret = -EBUSY; -- cgit v1.2.3-55-g7522 From 41795a8a3cff8e4ba54236ca16c3814ba9cd7f39 Mon Sep 17 00:00:00 2001 From: Jon Hunter Date: Fri, 4 Mar 2016 10:55:15 +0000 Subject: PM / Domains: Fix potential NULL pointer dereference In the function of_genpd_get_from_provider(), we never check to see if the argument 'genpdspec' is NULL before dereferencing it. Add error checking to handle any NULL pointers. Signed-off-by: Jon Hunter Acked-by: Ulf Hansson Signed-off-by: Rafael J. Wysocki --- drivers/base/power/domain.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'drivers/base') diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index 79f5d3965931..56705b52758e 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -1702,6 +1702,9 @@ struct generic_pm_domain *of_genpd_get_from_provider( struct generic_pm_domain *genpd = ERR_PTR(-ENOENT); struct of_genpd_provider *provider; + if (!genpdspec) + return ERR_PTR(-EINVAL); + mutex_lock(&of_genpd_mutex); /* Check if we have such a provider in our array */ -- cgit v1.2.3-55-g7522