Hi Marc, Thanks for rebasing on top of my latest series.
On Tue, Oct 06 2015 at 08:27 -0600, Marc Titinger wrote:
Devices may register an intermediate retention state into the domain upon
I may agree with the usability of dynamic adding a state to the domain, but I dont see why a device attaching to a domain should bring about a new domain state. A domain should define its power states, independent of the devices that may attach. The way I see it, devices have their own idle states and domains have their own. I do see a relationship between possible domain states depending on the states of the individual devices in the domain. For ex, a CPU domain can only be in a retention state (low voltage, memory retained), if its CPU devices are in retention state, i.e, the domain cannot be powered off; alternately, the domain may be in retention or power down if the CPU devices are in power down state. Could you elaborate on why this is a need? Thanks, Lina
attaching. Currently generic domain would register an array of states upon init. This patch prepares for later insertion (sort per depth, remove). Signed-off-by: Marc Titinger <mtitinger+rene...@baylibre.com> --- drivers/base/power/domain.c | 189 +++++++++++++++++++------------------------- include/linux/pm_domain.h | 18 ++++- 2 files changed, 97 insertions(+), 110 deletions(-) diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index 3e27a2b..e5f4c00b 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -19,6 +19,7 @@ #include <linux/sched.h> #include <linux/suspend.h> #include <linux/export.h> +#include <linux/sort.h> #define GENPD_RETRY_MAX_MS 250 /* Approximate */ @@ -50,12 +51,6 @@ __retval; \ }) -#define GENPD_MAX_NAME_SIZE 20 - -static int pm_genpd_alloc_states_names(struct generic_pm_domain *genpd, - const struct genpd_power_state *st, - unsigned int st_count); - static LIST_HEAD(gpd_list); static DEFINE_MUTEX(gpd_list_lock); @@ -1364,46 +1359,6 @@ static void genpd_free_dev_data(struct device *dev, dev_pm_put_subsys_data(dev); } -static int genpd_alloc_states_data(struct generic_pm_domain *genpd, - const struct genpd_power_state *st, - unsigned int st_count) -{ - int ret = 0; - unsigned int i; - - if (IS_ERR_OR_NULL(genpd)) { - ret = -EINVAL; - goto err; - } - - if (!st || (st_count < 1)) { - ret = -EINVAL; - goto err; - } - - /* Allocate the local memory to keep the states for this genpd */ - genpd->states = kcalloc(st_count, sizeof(*st), GFP_KERNEL); - if (!genpd->states) { - ret = -ENOMEM; - goto err; - } - - for (i = 0; i < st_count; i++) { - genpd->states[i].power_on_latency_ns = - st[i].power_on_latency_ns; - genpd->states[i].power_off_latency_ns = - st[i].power_off_latency_ns; - } - - genpd->state_count = st_count; - - /* to save memory, Name allocation will happen if debug is enabled */ - pm_genpd_alloc_states_names(genpd, st, st_count); - -err: - return ret; -} - /** * __pm_genpd_add_device - Add a device to an I/O PM domain. * @genpd: PM domain to add the device to. @@ -1833,6 +1788,75 @@ static void genpd_lock_init(struct generic_pm_domain *genpd) } } + +/* +* state depth comparison function. +*/ +static int state_cmp(const void *a, const void *b) +{ + struct genpd_power_state *state_a = (struct genpd_power_state *)(a); + struct genpd_power_state *state_b = (struct genpd_power_state *)(b); + + s64 depth_a = + state_a->power_on_latency_ns + state_a->power_off_latency_ns; + s64 depth_b = + state_b->power_on_latency_ns + state_b->power_off_latency_ns; + + return (depth_a > depth_b) ? 0 : -1; +} + +/* +* TODO: antagonist routine. +*/ +int pm_genpd_insert_state(struct generic_pm_domain *genpd, + const struct genpd_power_state *state) +{ + int ret = 0; + int state_count = genpd->state_count; + + if (IS_ERR_OR_NULL(genpd) || (!state)) + ret = -EINVAL; + + if (state_count >= GENPD_POWER_STATES_MAX) + ret = -ENOMEM; + +#ifdef CONFIG_PM_ADVANCED_DEBUG + /* to save memory, Name allocation will happen if debug is enabled */ + genpd->states[state_count].name = kstrndup(state->name, + GENPD_MAX_NAME_SIZE, + GFP_KERNEL); + if (!genpd->states[state_count].name) { + pr_err("%s Failed to allocate state '%s' name.\n", + genpd->name, state->name); + ret = -ENOMEM; + } +#endif + genpd_lock(genpd); + + if (!ret) { + genpd->states[state_count].power_on_latency_ns = + state->power_on_latency_ns; + genpd->states[state_count].power_off_latency_ns = + state->power_off_latency_ns; + genpd->state_count++; + } + + /* sort from shallowest to deepest */ + sort(genpd->states, genpd->state_count, + sizeof(genpd->states[0]), state_cmp, NULL /*generic swap */); + + /* Sanity check for current state index */ + if (genpd->state_idx >= genpd->state_count) { + pr_warn("pm domain %s Invalid initial state.\n", genpd->name); + genpd->state_idx = genpd->state_count - 1; + } + + genpd_unlock(genpd); + + return ret; +} + + /** * pm_genpd_init - Initialize a generic I/O PM domain object. * @genpd: PM domain object to initialize. @@ -1846,36 +1870,24 @@ void pm_genpd_init(struct generic_pm_domain *genpd, const struct genpd_power_state *states, unsigned int state_count, bool is_off) { - static const struct genpd_power_state genpd_default_state[] = { - { - .name = "OFF", - .power_off_latency_ns = 0, - .power_on_latency_ns = 0, - }, - }; - int ret; + int i; if (IS_ERR_OR_NULL(genpd)) return; - /* If no states defined, use the default OFF state */ - if (!states || (state_count < 1)) - ret = genpd_alloc_states_data(genpd, genpd_default_state, - ARRAY_SIZE(genpd_default_state)); - else - ret = genpd_alloc_states_data(genpd, states, state_count); - - if (ret) { - pr_err("Fail to allocate states for %s\n", genpd->name); - return; - } + /* simply use an array, we wish to add/remove new retention states + from later device init/exit. */ + memset(genpd->states, 0, GENPD_POWER_STATES_MAX + * sizeof(struct genpd_power_state)); - /* Sanity check for initial state */ - if (genpd->state_idx >= genpd->state_count) { - pr_warn("pm domain %s Invalid initial state.\n", - genpd->name); - genpd->state_idx = genpd->state_count - 1; - } + if (!states || !state_count) { + /* require a provider for a default state */ + genpd->state_count = 0; + genpd->state_idx = 0; + } else + for (i = 0; i < state_count; i++) + if (pm_genpd_insert_state(genpd, &states[i])) + return; INIT_LIST_HEAD(&genpd->master_links); INIT_LIST_HEAD(&genpd->slave_links); @@ -2233,33 +2245,6 @@ EXPORT_SYMBOL_GPL(genpd_dev_pm_attach); #include <linux/kobject.h> static struct dentry *pm_genpd_debugfs_dir; -static int pm_genpd_alloc_states_names(struct generic_pm_domain *genpd, - const struct genpd_power_state *st, - unsigned int st_count) -{ - unsigned int i; - - if (IS_ERR_OR_NULL(genpd)) - return -EINVAL; - - if (genpd->state_count != st_count) { - pr_err("Invalid allocated state count\n"); - return -EINVAL; - } - - for (i = 0; i < st_count; i++) { - genpd->states[i].name = kstrndup(st[i].name, - GENPD_MAX_NAME_SIZE, GFP_KERNEL); - if (!genpd->states[i].name) { - pr_err("%s Failed to allocate state %d name.\n", - genpd->name, i); - return -ENOMEM; - } - } - - return 0; -} - /* * TODO: This function is a slightly modified version of rtpm_status_show * from sysfs.c, so generalize it. @@ -2398,12 +2383,4 @@ static void __exit pm_genpd_debug_exit(void) { debugfs_remove_recursive(pm_genpd_debugfs_dir); } -__exitcall(pm_genpd_debug_exit); -#else -static inline int pm_genpd_alloc_states_names(struct generic_pm_domain *genpd, - const struct genpd_power_state *st, - unsigned int st_count) -{ - return 0; -} #endif /* CONFIG_PM_ADVANCED_DEBUG */ diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h index 9d37292..8a4eab0 100644 --- a/include/linux/pm_domain.h +++ b/include/linux/pm_domain.h @@ -45,6 +45,13 @@ struct gpd_cpuidle_data { struct cpuidle_state *idle_state; }; + +/* Arbitrary max number of devices registering a special + * retention state with the PD, to keep things simple. + */ +#define GENPD_POWER_STATES_MAX 12 +#define GENPD_MAX_NAME_SIZE 40 + struct genpd_power_state { char *name; s64 power_off_latency_ns; @@ -80,7 +87,8 @@ struct generic_pm_domain { struct device *dev); unsigned int flags; /* Bit field of configs for genpd */ - struct genpd_power_state *states; + struct genpd_power_state states[GENPD_POWER_STATES_MAX]; + unsigned int state_count; /* number of states */ unsigned int state_idx; /* state that genpd will go to when off */ @@ -159,10 +167,12 @@ extern int pm_genpd_attach_cpuidle(struct generic_pm_domain *genpd, int state); extern int pm_genpd_name_attach_cpuidle(const char *name, int state); extern int pm_genpd_detach_cpuidle(struct generic_pm_domain *genpd); extern int pm_genpd_name_detach_cpuidle(const char *name); +extern int pm_genpd_insert_state(struct generic_pm_domain *genpd, + const struct genpd_power_state *state); extern void pm_genpd_init(struct generic_pm_domain *genpd, - struct dev_power_governor *gov, - const struct genpd_power_state *states, - unsigned int state_count, bool is_off); + struct dev_power_governor *gov, + const struct genpd_power_state *states, + unsigned int state_count, bool is_off); extern int pm_genpd_poweron(struct generic_pm_domain *genpd); extern int pm_genpd_name_poweron(const char *domain_name); -- 1.9.1
-- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/