From 137b944e100278d696826cf25c83014ac17473fe Mon Sep 17 00:00:00 2001 From: Daniel Lezcano Date: Wed, 12 Jun 2013 15:08:48 +0200 Subject: cpuidle: Make it clear that governors cannot be modules cpufreq governors are defined as modules in the code, but the Kconfig options do not allow them to be built as modules. This is not really a problem, but the cpuidle init ordering is: the cpuidle init functions (framework and driver) and then the governors. That leads to some weirdness in the cpuidle framework. Namely, cpuidle_register_device() calls cpuidle_enable_device() which fails at the first attempt, because governors have not been registered yet. When a governor is registered, the framework calls cpuidle_enable_device() again which runs __cpuidle_register_device() only then. Of course, for that to work, the cpuidle_enable_device() return value has to be ignored by cpuidle_register_device(). Instead of having this cyclic call graph and relying on a positive side effects of the hackish back and forth cpuidle_enable_device() calls it is better to fix the cpuidle init ordering. To that end, replace the module init code with postcore_initcall() so we have: * cpuidle framework : core_initcall * cpuidle governors : postcore_initcall * cpuidle drivers : device_initcall and remove the corresponding module exit code as it is dead anyway (governors can't be built as modules). [rjw: Changelog] Signed-off-by: Daniel Lezcano Signed-off-by: Rafael J. Wysocki diff --git a/drivers/cpuidle/governors/ladder.c b/drivers/cpuidle/governors/ladder.c index 9b78405..9f08e8c 100644 --- a/drivers/cpuidle/governors/ladder.c +++ b/drivers/cpuidle/governors/ladder.c @@ -192,14 +192,4 @@ static int __init init_ladder(void) return cpuidle_register_governor(&ladder_governor); } -/** - * exit_ladder - exits the governor - */ -static void __exit exit_ladder(void) -{ - cpuidle_unregister_governor(&ladder_governor); -} - -MODULE_LICENSE("GPL"); -module_init(init_ladder); -module_exit(exit_ladder); +postcore_initcall(init_ladder); diff --git a/drivers/cpuidle/governors/menu.c b/drivers/cpuidle/governors/menu.c index fe343a0..743138c 100644 --- a/drivers/cpuidle/governors/menu.c +++ b/drivers/cpuidle/governors/menu.c @@ -540,14 +540,4 @@ static int __init init_menu(void) return cpuidle_register_governor(&menu_governor); } -/** - * exit_menu - exits the governor - */ -static void __exit exit_menu(void) -{ - cpuidle_unregister_governor(&menu_governor); -} - -MODULE_LICENSE("GPL"); -module_init(init_menu); -module_exit(exit_menu); +postcore_initcall(init_menu); -- cgit v0.10.2 From 10b9d3f8a4d5c82bff5b232a0063669dc0e0d725 Mon Sep 17 00:00:00 2001 From: Daniel Lezcano Date: Wed, 12 Jun 2013 15:08:49 +0200 Subject: cpuidle: Check cpuidle_enable_device() return value We previously changed the ordering of the cpuidle framework initialization so that the governors are registered before the drivers which can register their devices right from the start. Now, we can safely remove the __cpuidle_register_device() call hack in cpuidle_enable_device() and check if the driver has been registered before enabling it. Then, cpuidle_register_device() can consistently check the cpuidle_enable_device() return value when enabling the device. [rjw: Changelog] Signed-off-by: Daniel Lezcano Signed-off-by: Rafael J. Wysocki diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c index fdc432f..4deed97 100644 --- a/drivers/cpuidle/cpuidle.c +++ b/drivers/cpuidle/cpuidle.c @@ -292,15 +292,12 @@ int cpuidle_enable_device(struct cpuidle_device *dev) if (!drv || !cpuidle_curr_governor) return -EIO; + if (!dev->registered) + return -EINVAL; + if (!dev->state_count) dev->state_count = drv->state_count; - if (dev->registered == 0) { - ret = __cpuidle_register_device(dev); - if (ret) - return ret; - } - poll_idle_init(drv); ret = cpuidle_add_device_sysfs(dev); @@ -415,13 +412,17 @@ int cpuidle_register_device(struct cpuidle_device *dev) return ret; } - cpuidle_enable_device(dev); + ret = cpuidle_enable_device(dev); + if (ret) { + mutex_unlock(&cpuidle_lock); + return ret; + } + cpuidle_install_idle_handler(); mutex_unlock(&cpuidle_lock); return 0; - } EXPORT_SYMBOL_GPL(cpuidle_register_device); -- cgit v0.10.2 From f89ae89e2790341587f7132f0552c941342115e9 Mon Sep 17 00:00:00 2001 From: Daniel Lezcano Date: Wed, 12 Jun 2013 15:08:50 +0200 Subject: cpuidle: Fix white space to follow CodingStyle Fix white space in the cpuidle code to follow the rules described in CodingStyle. No changes in behavior should result from this. [rjw: Changelog] Signed-off-by: Daniel Lezcano Signed-off-by: Rafael J. Wysocki diff --git a/drivers/cpuidle/sysfs.c b/drivers/cpuidle/sysfs.c index 428754a..7d4448a 100644 --- a/drivers/cpuidle/sysfs.c +++ b/drivers/cpuidle/sysfs.c @@ -33,7 +33,8 @@ static ssize_t show_available_governors(struct device *dev, mutex_lock(&cpuidle_lock); list_for_each_entry(tmp, &cpuidle_governors, governor_list) { - if (i >= (ssize_t) ((PAGE_SIZE/sizeof(char)) - CPUIDLE_NAME_LEN - 2)) + if (i >= (ssize_t) ((PAGE_SIZE/sizeof(char)) - + CPUIDLE_NAME_LEN - 2)) goto out; i += scnprintf(&buf[i], CPUIDLE_NAME_LEN, "%s ", tmp->name); } @@ -168,11 +169,13 @@ struct cpuidle_attr { #define kobj_to_cpuidledev(k) container_of(k, struct cpuidle_device, kobj) #define attr_to_cpuidleattr(a) container_of(a, struct cpuidle_attr, attr) -static ssize_t cpuidle_show(struct kobject * kobj, struct attribute * attr ,char * buf) + +static ssize_t cpuidle_show(struct kobject *kobj, struct attribute *attr, + char *buf) { int ret = -EIO; struct cpuidle_device *dev = kobj_to_cpuidledev(kobj); - struct cpuidle_attr * cattr = attr_to_cpuidleattr(attr); + struct cpuidle_attr *cattr = attr_to_cpuidleattr(attr); if (cattr->show) { mutex_lock(&cpuidle_lock); @@ -182,12 +185,12 @@ static ssize_t cpuidle_show(struct kobject * kobj, struct attribute * attr ,char return ret; } -static ssize_t cpuidle_store(struct kobject * kobj, struct attribute * attr, - const char * buf, size_t count) +static ssize_t cpuidle_store(struct kobject *kobj, struct attribute *attr, + const char *buf, size_t count) { int ret = -EIO; struct cpuidle_device *dev = kobj_to_cpuidledev(kobj); - struct cpuidle_attr * cattr = attr_to_cpuidleattr(attr); + struct cpuidle_attr *cattr = attr_to_cpuidleattr(attr); if (cattr->store) { mutex_lock(&cpuidle_lock); @@ -237,8 +240,8 @@ static ssize_t show_state_##_name(struct cpuidle_state *state, \ #define define_store_state_ull_function(_name) \ static ssize_t store_state_##_name(struct cpuidle_state *state, \ - struct cpuidle_state_usage *state_usage, \ - const char *buf, size_t size) \ + struct cpuidle_state_usage *state_usage, \ + const char *buf, size_t size) \ { \ unsigned long long value; \ int err; \ @@ -256,14 +259,16 @@ static ssize_t store_state_##_name(struct cpuidle_state *state, \ #define define_show_state_ull_function(_name) \ static ssize_t show_state_##_name(struct cpuidle_state *state, \ - struct cpuidle_state_usage *state_usage, char *buf) \ + struct cpuidle_state_usage *state_usage, \ + char *buf) \ { \ return sprintf(buf, "%llu\n", state_usage->_name);\ } #define define_show_state_str_function(_name) \ static ssize_t show_state_##_name(struct cpuidle_state *state, \ - struct cpuidle_state_usage *state_usage, char *buf) \ + struct cpuidle_state_usage *state_usage, \ + char *buf) \ { \ if (state->_name[0] == '\0')\ return sprintf(buf, "\n");\ @@ -309,8 +314,9 @@ struct cpuidle_state_kobj { #define kobj_to_state(k) (kobj_to_state_obj(k)->state) #define kobj_to_state_usage(k) (kobj_to_state_obj(k)->state_usage) #define attr_to_stateattr(a) container_of(a, struct cpuidle_state_attr, attr) -static ssize_t cpuidle_state_show(struct kobject * kobj, - struct attribute * attr ,char * buf) + +static ssize_t cpuidle_state_show(struct kobject *kobj, struct attribute *attr, + char * buf) { int ret = -EIO; struct cpuidle_state *state = kobj_to_state(kobj); @@ -323,8 +329,8 @@ static ssize_t cpuidle_state_show(struct kobject * kobj, return ret; } -static ssize_t cpuidle_state_store(struct kobject *kobj, - struct attribute *attr, const char *buf, size_t size) +static ssize_t cpuidle_state_store(struct kobject *kobj, struct attribute *attr, + const char *buf, size_t size) { int ret = -EIO; struct cpuidle_state *state = kobj_to_state(kobj); @@ -449,8 +455,8 @@ static void cpuidle_driver_sysfs_release(struct kobject *kobj) complete(&driver_kobj->kobj_unregister); } -static ssize_t cpuidle_driver_show(struct kobject *kobj, struct attribute * attr, - char * buf) +static ssize_t cpuidle_driver_show(struct kobject *kobj, struct attribute *attr, + char *buf) { int ret = -EIO; struct cpuidle_driver_kobj *driver_kobj = kobj_to_driver_kobj(kobj); -- cgit v0.10.2 From 728ce22b696f9f1404a74d7b2279a65933553a1b Mon Sep 17 00:00:00 2001 From: Daniel Lezcano Date: Wed, 12 Jun 2013 15:08:51 +0200 Subject: cpuidle: Make cpuidle's sysfs directory dynamically allocated The cpuidle sysfs code is designed to have a single instance of per CPU cpuidle directory. It is not possible to remove the sysfs entry and create it again. This is not a problem with the current code but future changes will add CPU hotplug support to enable/disable the device, so it will need to remove the sysfs entry like other subsystems do. That won't be possible without this change, because the kobj is a static object which can't be reused for kobj_init_and_add(). Add cpuidle_device_kobj to be allocated dynamically when adding/removing a sysfs entry which is consistent with the other cpuidle's sysfs entries. An added benefit is that the sysfs code is now more self-contained and the includes needed for sysfs can be moved from cpuidle.h directly into sysfs.c so as to reduce the total number of headers dragged along with cpuidle.h. [rjw: Changelog] Signed-off-by: Daniel Lezcano Signed-off-by: Rafael J. Wysocki diff --git a/drivers/cpuidle/sysfs.c b/drivers/cpuidle/sysfs.c index 7d4448a..8739cc0 100644 --- a/drivers/cpuidle/sysfs.c +++ b/drivers/cpuidle/sysfs.c @@ -11,8 +11,10 @@ #include #include #include +#include #include #include +#include #include "cpuidle.h" @@ -167,14 +169,27 @@ struct cpuidle_attr { #define define_one_rw(_name, show, store) \ static struct cpuidle_attr attr_##_name = __ATTR(_name, 0644, show, store) -#define kobj_to_cpuidledev(k) container_of(k, struct cpuidle_device, kobj) #define attr_to_cpuidleattr(a) container_of(a, struct cpuidle_attr, attr) +struct cpuidle_device_kobj { + struct cpuidle_device *dev; + struct completion kobj_unregister; + struct kobject kobj; +}; + +static inline struct cpuidle_device *to_cpuidle_device(struct kobject *kobj) +{ + struct cpuidle_device_kobj *kdev = + container_of(kobj, struct cpuidle_device_kobj, kobj); + + return kdev->dev; +} + static ssize_t cpuidle_show(struct kobject *kobj, struct attribute *attr, char *buf) { int ret = -EIO; - struct cpuidle_device *dev = kobj_to_cpuidledev(kobj); + struct cpuidle_device *dev = to_cpuidle_device(kobj); struct cpuidle_attr *cattr = attr_to_cpuidleattr(attr); if (cattr->show) { @@ -189,7 +204,7 @@ static ssize_t cpuidle_store(struct kobject *kobj, struct attribute *attr, const char *buf, size_t count) { int ret = -EIO; - struct cpuidle_device *dev = kobj_to_cpuidledev(kobj); + struct cpuidle_device *dev = to_cpuidle_device(kobj); struct cpuidle_attr *cattr = attr_to_cpuidleattr(attr); if (cattr->store) { @@ -207,9 +222,10 @@ static const struct sysfs_ops cpuidle_sysfs_ops = { static void cpuidle_sysfs_release(struct kobject *kobj) { - struct cpuidle_device *dev = kobj_to_cpuidledev(kobj); + struct cpuidle_device_kobj *kdev = + container_of(kobj, struct cpuidle_device_kobj, kobj); - complete(&dev->kobj_unregister); + complete(&kdev->kobj_unregister); } static struct kobj_type ktype_cpuidle = { @@ -377,6 +393,7 @@ static int cpuidle_add_state_sysfs(struct cpuidle_device *device) { int i, ret = -ENOMEM; struct cpuidle_state_kobj *kobj; + struct cpuidle_device_kobj *kdev = device->kobj_dev; struct cpuidle_driver *drv = cpuidle_get_cpu_driver(device); /* state statistics */ @@ -389,7 +406,7 @@ static int cpuidle_add_state_sysfs(struct cpuidle_device *device) init_completion(&kobj->kobj_unregister); ret = kobject_init_and_add(&kobj->kobj, &ktype_state_cpuidle, - &device->kobj, "state%d", i); + &kdev->kobj, "state%d", i); if (ret) { kfree(kobj); goto error_state; @@ -506,6 +523,7 @@ static struct kobj_type ktype_driver_cpuidle = { static int cpuidle_add_driver_sysfs(struct cpuidle_device *dev) { struct cpuidle_driver_kobj *kdrv; + struct cpuidle_device_kobj *kdev = dev->kobj_dev; struct cpuidle_driver *drv = cpuidle_get_cpu_driver(dev); int ret; @@ -517,7 +535,7 @@ static int cpuidle_add_driver_sysfs(struct cpuidle_device *dev) init_completion(&kdrv->kobj_unregister); ret = kobject_init_and_add(&kdrv->kobj, &ktype_driver_cpuidle, - &dev->kobj, "driver"); + &kdev->kobj, "driver"); if (ret) { kfree(kdrv); return ret; @@ -586,16 +604,28 @@ void cpuidle_remove_device_sysfs(struct cpuidle_device *device) */ int cpuidle_add_sysfs(struct cpuidle_device *dev) { + struct cpuidle_device_kobj *kdev; struct device *cpu_dev = get_cpu_device((unsigned long)dev->cpu); int error; - init_completion(&dev->kobj_unregister); + kdev = kzalloc(sizeof(*kdev), GFP_KERNEL); + if (!kdev) + return -ENOMEM; + kdev->dev = dev; + dev->kobj_dev = kdev; + + init_completion(&kdev->kobj_unregister); + + error = kobject_init_and_add(&kdev->kobj, &ktype_cpuidle, &cpu_dev->kobj, + "cpuidle"); + if (error) { + kfree(kdev); + return error; + } - error = kobject_init_and_add(&dev->kobj, &ktype_cpuidle, &cpu_dev->kobj, - "cpuidle"); - if (!error) - kobject_uevent(&dev->kobj, KOBJ_ADD); - return error; + kobject_uevent(&kdev->kobj, KOBJ_ADD); + + return 0; } /** @@ -604,6 +634,9 @@ int cpuidle_add_sysfs(struct cpuidle_device *dev) */ void cpuidle_remove_sysfs(struct cpuidle_device *dev) { - kobject_put(&dev->kobj); - wait_for_completion(&dev->kobj_unregister); + struct cpuidle_device_kobj *kdev = dev->kobj_dev; + + kobject_put(&kdev->kobj); + wait_for_completion(&kdev->kobj_unregister); + kfree(kdev); } diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h index 0bc4b74..b922db5 100644 --- a/include/linux/cpuidle.h +++ b/include/linux/cpuidle.h @@ -13,8 +13,6 @@ #include #include -#include -#include #include #define CPUIDLE_STATE_MAX 10 @@ -61,6 +59,8 @@ struct cpuidle_state { #define CPUIDLE_DRIVER_FLAGS_MASK (0xFFFF0000) +struct cpuidle_device_kobj; + struct cpuidle_device { unsigned int registered:1; unsigned int enabled:1; @@ -71,9 +71,8 @@ struct cpuidle_device { struct cpuidle_state_usage states_usage[CPUIDLE_STATE_MAX]; struct cpuidle_state_kobj *kobjs[CPUIDLE_STATE_MAX]; struct cpuidle_driver_kobj *kobj_driver; + struct cpuidle_device_kobj *kobj_dev; struct list_head device_list; - struct kobject kobj; - struct completion kobj_unregister; #ifdef CONFIG_ARCH_NEEDS_CPU_IDLE_COUPLED int safe_state_index; -- cgit v0.10.2 From 1a7064380e6639e1cc5ed609ceb1ed46f3f188e3 Mon Sep 17 00:00:00 2001 From: Daniel Lezcano Date: Wed, 12 Jun 2013 15:08:52 +0200 Subject: cpuidle: Add missing forward declarations of structures Add missing forward declarations of struct cpuidle_state_kobj and struct cpuidle_driver_kobj in cpuidle.h. [rjw: Changelog] Signed-off-by: Daniel Lezcano Signed-off-by: Rafael J. Wysocki diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h index b922db5..781addc 100644 --- a/include/linux/cpuidle.h +++ b/include/linux/cpuidle.h @@ -60,6 +60,8 @@ struct cpuidle_state { #define CPUIDLE_DRIVER_FLAGS_MASK (0xFFFF0000) struct cpuidle_device_kobj; +struct cpuidle_state_kobj; +struct cpuidle_driver_kobj; struct cpuidle_device { unsigned int registered:1; -- cgit v0.10.2 From f6bb51a53a7535c79d6c65862d6b48e83340b337 Mon Sep 17 00:00:00 2001 From: Daniel Lezcano Date: Wed, 12 Jun 2013 15:08:53 +0200 Subject: cpuidle: Introduce __cpuidle_unregister_device() To reduce code duplication related to the unregistration of cpuidle devices, introduce __cpuidle_unregister_device() and move all of the unregistration code to that function. [rjw: Changelog] Signed-off-by: Daniel Lezcano Signed-off-by: Rafael J. Wysocki diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c index 4deed97..d78c6d8 100644 --- a/drivers/cpuidle/cpuidle.c +++ b/drivers/cpuidle/cpuidle.c @@ -42,8 +42,6 @@ void disable_cpuidle(void) off = 1; } -static int __cpuidle_register_device(struct cpuidle_device *dev); - /** * cpuidle_play_dead - cpu off-lining * @@ -357,6 +355,15 @@ void cpuidle_disable_device(struct cpuidle_device *dev) EXPORT_SYMBOL_GPL(cpuidle_disable_device); +static void __cpuidle_unregister_device(struct cpuidle_device *dev) +{ + struct cpuidle_driver *drv = cpuidle_get_cpu_driver(dev); + + list_del(&dev->device_list); + per_cpu(cpuidle_devices, dev->cpu) = NULL; + module_put(drv->owner); +} + /** * __cpuidle_register_device - internal register function called before register * and enable routines @@ -374,24 +381,15 @@ static int __cpuidle_register_device(struct cpuidle_device *dev) per_cpu(cpuidle_devices, dev->cpu) = dev; list_add(&dev->device_list, &cpuidle_detected_devices); - ret = cpuidle_add_sysfs(dev); - if (ret) - goto err_sysfs; ret = cpuidle_coupled_register_device(dev); - if (ret) - goto err_coupled; + if (ret) { + __cpuidle_unregister_device(dev); + return ret; + } dev->registered = 1; return 0; - -err_coupled: - cpuidle_remove_sysfs(dev); -err_sysfs: - list_del(&dev->device_list); - per_cpu(cpuidle_devices, dev->cpu) = NULL; - module_put(drv->owner); - return ret; } /** @@ -407,22 +405,30 @@ int cpuidle_register_device(struct cpuidle_device *dev) mutex_lock(&cpuidle_lock); - if ((ret = __cpuidle_register_device(dev))) { - mutex_unlock(&cpuidle_lock); - return ret; - } + ret = __cpuidle_register_device(dev); + if (ret) + goto out_unlock; + + ret = cpuidle_add_sysfs(dev); + if (ret) + goto out_unregister; ret = cpuidle_enable_device(dev); - if (ret) { - mutex_unlock(&cpuidle_lock); - return ret; - } + if (ret) + goto out_sysfs; cpuidle_install_idle_handler(); +out_unlock: mutex_unlock(&cpuidle_lock); - return 0; + return ret; + +out_sysfs: + cpuidle_remove_sysfs(dev); +out_unregister: + __cpuidle_unregister_device(dev); + goto out_unlock; } EXPORT_SYMBOL_GPL(cpuidle_register_device); @@ -433,8 +439,6 @@ EXPORT_SYMBOL_GPL(cpuidle_register_device); */ void cpuidle_unregister_device(struct cpuidle_device *dev) { - struct cpuidle_driver *drv = cpuidle_get_cpu_driver(dev); - if (dev->registered == 0) return; @@ -443,14 +447,12 @@ void cpuidle_unregister_device(struct cpuidle_device *dev) cpuidle_disable_device(dev); cpuidle_remove_sysfs(dev); - list_del(&dev->device_list); - per_cpu(cpuidle_devices, dev->cpu) = NULL; + + __cpuidle_unregister_device(dev); cpuidle_coupled_unregister_device(dev); cpuidle_resume_and_unlock(); - - module_put(drv->owner); } EXPORT_SYMBOL_GPL(cpuidle_unregister_device); -- cgit v0.10.2 From 5df0aa7341bd94ca2023a60c64c63faeb6ec209d Mon Sep 17 00:00:00 2001 From: Daniel Lezcano Date: Wed, 12 Jun 2013 15:08:54 +0200 Subject: cpuidle: Introduce __cpuidle_device_init() Add __cpuidle_device_init() for initializing the cpuidle_device structure. [rjw: Changelog] Signed-off-by: Daniel Lezcano Signed-off-by: Rafael J. Wysocki diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c index d78c6d8..5b63185 100644 --- a/drivers/cpuidle/cpuidle.c +++ b/drivers/cpuidle/cpuidle.c @@ -276,7 +276,7 @@ static void poll_idle_init(struct cpuidle_driver *drv) {} */ int cpuidle_enable_device(struct cpuidle_device *dev) { - int ret, i; + int ret; struct cpuidle_driver *drv; if (!dev) @@ -306,12 +306,6 @@ int cpuidle_enable_device(struct cpuidle_device *dev) (ret = cpuidle_curr_governor->enable(drv, dev))) goto fail_sysfs; - for (i = 0; i < dev->state_count; i++) { - dev->states_usage[i].usage = 0; - dev->states_usage[i].time = 0; - } - dev->last_residency = 0; - smp_wmb(); dev->enabled = 1; @@ -364,6 +358,14 @@ static void __cpuidle_unregister_device(struct cpuidle_device *dev) module_put(drv->owner); } +static int __cpuidle_device_init(struct cpuidle_device *dev) +{ + memset(dev->states_usage, 0, sizeof(dev->states_usage)); + dev->last_residency = 0; + + return 0; +} + /** * __cpuidle_register_device - internal register function called before register * and enable routines @@ -405,6 +407,10 @@ int cpuidle_register_device(struct cpuidle_device *dev) mutex_lock(&cpuidle_lock); + ret = __cpuidle_device_init(dev); + if (ret) + goto out_unlock; + ret = __cpuidle_register_device(dev); if (ret) goto out_unlock; -- cgit v0.10.2 From c878a52d3c7cabab5b24460825c24eafd8be7058 Mon Sep 17 00:00:00 2001 From: Daniel Lezcano Date: Wed, 12 Jun 2013 15:08:55 +0200 Subject: cpuidle: Check if device is already registered Make __cpuidle_register_device() check whether or not the device has been registered already and return -EBUSY immediately if that's the case. [rjw: Changelog] Signed-off-by: Daniel Lezcano Signed-off-by: Rafael J. Wysocki diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c index 5b63185..d75040d 100644 --- a/drivers/cpuidle/cpuidle.c +++ b/drivers/cpuidle/cpuidle.c @@ -400,13 +400,16 @@ static int __cpuidle_register_device(struct cpuidle_device *dev) */ int cpuidle_register_device(struct cpuidle_device *dev) { - int ret; + int ret = -EBUSY; if (!dev) return -EINVAL; mutex_lock(&cpuidle_lock); + if (dev->registered) + goto out_unlock; + ret = __cpuidle_device_init(dev); if (ret) goto out_unlock; -- cgit v0.10.2 From b98e01ad4ed9da6882b41983319361502aded307 Mon Sep 17 00:00:00 2001 From: Sahara Date: Wed, 3 Jul 2013 17:49:55 +0900 Subject: cpuidle: Add Kconfig.arm and move calxeda, kirkwood and zynq Add Kconfig.arm for ARM cpuidle drivers and moves calxeda, kirkwood and zynq to Kconfig.arm. Like in the cpufreq menu, "CPU Idle" menu is added to drivers/cpuidle/Kconfig. Signed-off-by: Sahara diff --git a/drivers/cpuidle/Kconfig b/drivers/cpuidle/Kconfig index 0e2cd5c..b3fb81d 100644 --- a/drivers/cpuidle/Kconfig +++ b/drivers/cpuidle/Kconfig @@ -1,5 +1,6 @@ +menu "CPU Idle" -menuconfig CPU_IDLE +config CPU_IDLE bool "CPU idle PM support" default y if ACPI || PPC_PSERIES select CPU_IDLE_GOV_LADDER if (!NO_HZ && !NO_HZ_IDLE) @@ -29,20 +30,13 @@ config CPU_IDLE_GOV_MENU bool "Menu governor (for tickless system)" default y -config CPU_IDLE_CALXEDA - bool "CPU Idle Driver for Calxeda processors" - depends on ARCH_HIGHBANK - select ARM_CPU_SUSPEND - help - Select this to enable cpuidle on Calxeda processors. - -config CPU_IDLE_ZYNQ - bool "CPU Idle Driver for Xilinx Zynq processors" - depends on ARCH_ZYNQ - help - Select this to enable cpuidle on Xilinx Zynq processors. +menu "ARM CPU Idle Drivers" +depends on ARM +source "drivers/cpuidle/Kconfig.arm" +endmenu endif config ARCH_NEEDS_CPU_IDLE_COUPLED def_bool n +endmenu diff --git a/drivers/cpuidle/Kconfig.arm b/drivers/cpuidle/Kconfig.arm new file mode 100644 index 0000000..2c8f17d --- /dev/null +++ b/drivers/cpuidle/Kconfig.arm @@ -0,0 +1,21 @@ +# +# ARM CPU Idle drivers +# + +config ARM_HIGHBANK_CPUIDLE + bool "CPU Idle Driver for Calxeda processors" + depends on ARCH_HIGHBANK + select ARM_CPU_SUSPEND + help + Select this to enable cpuidle on Calxeda processors. + +config ARM_KIRKWOOD_CPUIDLE + bool "CPU Idle Driver for Marvell Kirkwood SoCs" + depends on ARCH_KIRKWOOD + help + This adds the CPU Idle driver for Marvell Kirkwood SoCs. + +config ARM_ZYNQ_CPUIDLE + bool "CPU Idle Driver for Xilinx Zynq processors" + depends on ARCH_ZYNQ + help diff --git a/drivers/cpuidle/Makefile b/drivers/cpuidle/Makefile index 8767a7b..77cf767 100644 --- a/drivers/cpuidle/Makefile +++ b/drivers/cpuidle/Makefile @@ -5,6 +5,8 @@ obj-y += cpuidle.o driver.o governor.o sysfs.o governors/ obj-$(CONFIG_ARCH_NEEDS_CPU_IDLE_COUPLED) += coupled.o -obj-$(CONFIG_CPU_IDLE_CALXEDA) += cpuidle-calxeda.o -obj-$(CONFIG_ARCH_KIRKWOOD) += cpuidle-kirkwood.o -obj-$(CONFIG_CPU_IDLE_ZYNQ) += cpuidle-zynq.o +################################################################################## +# ARM SoC drivers +obj-$(CONFIG_ARM_HIGHBANK_CPUIDLE) += cpuidle-calxeda.o +obj-$(CONFIG_ARM_KIRKWOOD_CPUIDLE) += cpuidle-kirkwood.o +obj-$(CONFIG_ARM_ZYNQ_CPUIDLE) += cpuidle-zynq.o -- cgit v0.10.2 From 7006b8a1facc5dcafb2afa27a1209d924f99a2eb Mon Sep 17 00:00:00 2001 From: Daniel Lezcano Date: Fri, 28 Jun 2013 12:09:09 +0200 Subject: ARM: davinci: cpuidle: Fix target residency The commit 19976c2a88d125aec16b9255c7197c297bbdd637 changed the target residency to 100000, assuming this is a careless mistake. The same happened to the at91's cpuidle driver. Fix it by putting the initial value to 10000. Signed-off-by: Daniel Lezcano Acked-by: Sekhar Nori diff --git a/arch/arm/mach-davinci/cpuidle.c b/arch/arm/mach-davinci/cpuidle.c index 36aef3a..f1ac1c9 100644 --- a/arch/arm/mach-davinci/cpuidle.c +++ b/arch/arm/mach-davinci/cpuidle.c @@ -65,7 +65,7 @@ static struct cpuidle_driver davinci_idle_driver = { .states[1] = { .enter = davinci_enter_idle, .exit_latency = 10, - .target_residency = 100000, + .target_residency = 10000, .flags = CPUIDLE_FLAG_TIME_VALID, .name = "DDR SR", .desc = "WFI and DDR Self Refresh", -- cgit v0.10.2 From 8025395f39c97e332362c24f7c33c86acdd4cd90 Mon Sep 17 00:00:00 2001 From: Linus Walleij Date: Wed, 10 Jul 2013 15:35:26 +0200 Subject: ARM: ux500: cpuidle: Instantiate the driver from platform device To break the dependency on the "id.h" file we move the cpuidle driver to a platform device. Now we only call the probe() on this driver if we find a corresponding platform device (which is spawned from the PRCMU MFD driver). Signed-off-by: Linus Walleij Signed-off-by: Daniel Lezcano diff --git a/arch/arm/mach-ux500/cpuidle.c b/arch/arm/mach-ux500/cpuidle.c index a45dd09..c0cb37c 100644 --- a/arch/arm/mach-ux500/cpuidle.c +++ b/arch/arm/mach-ux500/cpuidle.c @@ -16,12 +16,12 @@ #include #include #include +#include #include #include #include "db8500-regs.h" -#include "id.h" static atomic_t master = ATOMIC_INIT(0); static DEFINE_SPINLOCK(master_lock); @@ -113,11 +113,8 @@ static struct cpuidle_driver ux500_idle_driver = { .state_count = 2, }; -int __init ux500_idle_init(void) +static int __init dbx500_cpuidle_probe(struct platform_device *pdev) { - if (!(cpu_is_u8500_family() || cpu_is_ux540_family())) - return -ENODEV; - /* Configure wake up reasons */ prcmu_enable_wakeups(PRCMU_WAKEUP(ARM) | PRCMU_WAKEUP(RTC) | PRCMU_WAKEUP(ABB)); @@ -125,4 +122,12 @@ int __init ux500_idle_init(void) return cpuidle_register(&ux500_idle_driver, NULL); } -device_initcall(ux500_idle_init); +static struct platform_driver dbx500_cpuidle_plat_driver = { + .driver = { + .name = "cpuidle-dbx500", + .owner = THIS_MODULE, + }, + .probe = dbx500_cpuidle_probe, +}; + +module_platform_driver(dbx500_cpuidle_plat_driver); diff --git a/drivers/mfd/db8500-prcmu.c b/drivers/mfd/db8500-prcmu.c index 3c157fa..0d68eb1 100644 --- a/drivers/mfd/db8500-prcmu.c +++ b/drivers/mfd/db8500-prcmu.c @@ -3094,6 +3094,10 @@ static struct mfd_cell db8500_prcmu_devs[] = { .pdata_size = sizeof(db8500_cpufreq_table), }, { + .name = "cpuidle-dbx500", + .of_compatible = "stericsson,cpuidle-dbx500", + }, + { .name = "db8500-thermal", .num_resources = ARRAY_SIZE(db8500_thsens_resources), .resources = db8500_thsens_resources, -- cgit v0.10.2 From 8457246b106cc4e53f2534309dbe35a973c7d046 Mon Sep 17 00:00:00 2001 From: Linus Walleij Date: Wed, 10 Jul 2013 15:52:01 +0200 Subject: ARM: ux500: cpuidle: Remove pointless include The db8500-regs.h file does not need to be included by the cpuidle driver. Signed-off-by: Linus Walleij Signed-off-by: Daniel Lezcano diff --git a/arch/arm/mach-ux500/cpuidle.c b/arch/arm/mach-ux500/cpuidle.c index c0cb37c..e056465 100644 --- a/arch/arm/mach-ux500/cpuidle.c +++ b/arch/arm/mach-ux500/cpuidle.c @@ -21,8 +21,6 @@ #include #include -#include "db8500-regs.h" - static atomic_t master = ATOMIC_INIT(0); static DEFINE_SPINLOCK(master_lock); -- cgit v0.10.2 From d3f2950f2adeea3da0317e952914b59adaa4cdb3 Mon Sep 17 00:00:00 2001 From: Daniel Lezcano Date: Wed, 10 Jul 2013 16:02:04 +0200 Subject: ARM: ux500: cpuidle: Move ux500 cpuidle driver to drivers/cpuidle There is no more dependency with arch/arm headers, so we can safely move the driver to the drivers/cpuidle directory. Signed-off-by: Daniel Lezcano Acked-by: Linus Walleij diff --git a/arch/arm/mach-ux500/Makefile b/arch/arm/mach-ux500/Makefile index bf9b6be..fe1f3e2 100644 --- a/arch/arm/mach-ux500/Makefile +++ b/arch/arm/mach-ux500/Makefile @@ -4,7 +4,6 @@ obj-y := cpu.o devices.o devices-common.o \ id.o usb.o timer.o pm.o -obj-$(CONFIG_CPU_IDLE) += cpuidle.o obj-$(CONFIG_CACHE_L2X0) += cache-l2x0.o obj-$(CONFIG_UX500_SOC_DB8500) += cpu-db8500.o devices-db8500.o obj-$(CONFIG_MACH_MOP500) += board-mop500.o board-mop500-sdi.o \ diff --git a/arch/arm/mach-ux500/cpuidle.c b/arch/arm/mach-ux500/cpuidle.c deleted file mode 100644 index e056465..0000000 --- a/arch/arm/mach-ux500/cpuidle.c +++ /dev/null @@ -1,131 +0,0 @@ -/* - * Copyright (c) 2012 Linaro : Daniel Lezcano (IBM) - * - * Based on the work of Rickard Andersson - * and Jonas Aaberg . - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License version 2 as - * published by the Free Software Foundation. - */ - -#include -#include -#include -#include -#include -#include -#include -#include - -#include -#include - -static atomic_t master = ATOMIC_INIT(0); -static DEFINE_SPINLOCK(master_lock); - -static inline int ux500_enter_idle(struct cpuidle_device *dev, - struct cpuidle_driver *drv, int index) -{ - int this_cpu = smp_processor_id(); - bool recouple = false; - - if (atomic_inc_return(&master) == num_online_cpus()) { - - /* With this lock, we prevent the other cpu to exit and enter - * this function again and become the master */ - if (!spin_trylock(&master_lock)) - goto wfi; - - /* decouple the gic from the A9 cores */ - if (prcmu_gic_decouple()) { - spin_unlock(&master_lock); - goto out; - } - - /* If an error occur, we will have to recouple the gic - * manually */ - recouple = true; - - /* At this state, as the gic is decoupled, if the other - * cpu is in WFI, we have the guarantee it won't be wake - * up, so we can safely go to retention */ - if (!prcmu_is_cpu_in_wfi(this_cpu ? 0 : 1)) - goto out; - - /* The prcmu will be in charge of watching the interrupts - * and wake up the cpus */ - if (prcmu_copy_gic_settings()) - goto out; - - /* Check in the meantime an interrupt did - * not occur on the gic ... */ - if (prcmu_gic_pending_irq()) - goto out; - - /* ... and the prcmu */ - if (prcmu_pending_irq()) - goto out; - - /* Go to the retention state, the prcmu will wait for the - * cpu to go WFI and this is what happens after exiting this - * 'master' critical section */ - if (prcmu_set_power_state(PRCMU_AP_IDLE, true, true)) - goto out; - - /* When we switch to retention, the prcmu is in charge - * of recoupling the gic automatically */ - recouple = false; - - spin_unlock(&master_lock); - } -wfi: - cpu_do_idle(); -out: - atomic_dec(&master); - - if (recouple) { - prcmu_gic_recouple(); - spin_unlock(&master_lock); - } - - return index; -} - -static struct cpuidle_driver ux500_idle_driver = { - .name = "ux500_idle", - .owner = THIS_MODULE, - .states = { - ARM_CPUIDLE_WFI_STATE, - { - .enter = ux500_enter_idle, - .exit_latency = 70, - .target_residency = 260, - .flags = CPUIDLE_FLAG_TIME_VALID | - CPUIDLE_FLAG_TIMER_STOP, - .name = "ApIdle", - .desc = "ARM Retention", - }, - }, - .safe_state_index = 0, - .state_count = 2, -}; - -static int __init dbx500_cpuidle_probe(struct platform_device *pdev) -{ - /* Configure wake up reasons */ - prcmu_enable_wakeups(PRCMU_WAKEUP(ARM) | PRCMU_WAKEUP(RTC) | - PRCMU_WAKEUP(ABB)); - - return cpuidle_register(&ux500_idle_driver, NULL); -} - -static struct platform_driver dbx500_cpuidle_plat_driver = { - .driver = { - .name = "cpuidle-dbx500", - .owner = THIS_MODULE, - }, - .probe = dbx500_cpuidle_probe, -}; - -module_platform_driver(dbx500_cpuidle_plat_driver); diff --git a/drivers/cpuidle/Kconfig.arm b/drivers/cpuidle/Kconfig.arm index 2c8f17d..b330219 100644 --- a/drivers/cpuidle/Kconfig.arm +++ b/drivers/cpuidle/Kconfig.arm @@ -19,3 +19,11 @@ config ARM_ZYNQ_CPUIDLE bool "CPU Idle Driver for Xilinx Zynq processors" depends on ARCH_ZYNQ help + Select this to enable cpuidle on Xilinx Zynq processors. + +config ARM_U8500_CPUIDLE + bool "Cpu Idle Driver for the ST-E u8500 processors" + depends on ARCH_U8500 + help + Select this to enable cpuidle for ST-E u8500 processors + diff --git a/drivers/cpuidle/Makefile b/drivers/cpuidle/Makefile index 77cf767..0b9d200 100644 --- a/drivers/cpuidle/Makefile +++ b/drivers/cpuidle/Makefile @@ -10,3 +10,4 @@ obj-$(CONFIG_ARCH_NEEDS_CPU_IDLE_COUPLED) += coupled.o obj-$(CONFIG_ARM_HIGHBANK_CPUIDLE) += cpuidle-calxeda.o obj-$(CONFIG_ARM_KIRKWOOD_CPUIDLE) += cpuidle-kirkwood.o obj-$(CONFIG_ARM_ZYNQ_CPUIDLE) += cpuidle-zynq.o +obj-$(CONFIG_ARM_U8500_CPUIDLE) += cpuidle-ux500.o diff --git a/drivers/cpuidle/cpuidle-ux500.c b/drivers/cpuidle/cpuidle-ux500.c new file mode 100644 index 0000000..e056465 --- /dev/null +++ b/drivers/cpuidle/cpuidle-ux500.c @@ -0,0 +1,131 @@ +/* + * Copyright (c) 2012 Linaro : Daniel Lezcano (IBM) + * + * Based on the work of Rickard Andersson + * and Jonas Aaberg . + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include + +static atomic_t master = ATOMIC_INIT(0); +static DEFINE_SPINLOCK(master_lock); + +static inline int ux500_enter_idle(struct cpuidle_device *dev, + struct cpuidle_driver *drv, int index) +{ + int this_cpu = smp_processor_id(); + bool recouple = false; + + if (atomic_inc_return(&master) == num_online_cpus()) { + + /* With this lock, we prevent the other cpu to exit and enter + * this function again and become the master */ + if (!spin_trylock(&master_lock)) + goto wfi; + + /* decouple the gic from the A9 cores */ + if (prcmu_gic_decouple()) { + spin_unlock(&master_lock); + goto out; + } + + /* If an error occur, we will have to recouple the gic + * manually */ + recouple = true; + + /* At this state, as the gic is decoupled, if the other + * cpu is in WFI, we have the guarantee it won't be wake + * up, so we can safely go to retention */ + if (!prcmu_is_cpu_in_wfi(this_cpu ? 0 : 1)) + goto out; + + /* The prcmu will be in charge of watching the interrupts + * and wake up the cpus */ + if (prcmu_copy_gic_settings()) + goto out; + + /* Check in the meantime an interrupt did + * not occur on the gic ... */ + if (prcmu_gic_pending_irq()) + goto out; + + /* ... and the prcmu */ + if (prcmu_pending_irq()) + goto out; + + /* Go to the retention state, the prcmu will wait for the + * cpu to go WFI and this is what happens after exiting this + * 'master' critical section */ + if (prcmu_set_power_state(PRCMU_AP_IDLE, true, true)) + goto out; + + /* When we switch to retention, the prcmu is in charge + * of recoupling the gic automatically */ + recouple = false; + + spin_unlock(&master_lock); + } +wfi: + cpu_do_idle(); +out: + atomic_dec(&master); + + if (recouple) { + prcmu_gic_recouple(); + spin_unlock(&master_lock); + } + + return index; +} + +static struct cpuidle_driver ux500_idle_driver = { + .name = "ux500_idle", + .owner = THIS_MODULE, + .states = { + ARM_CPUIDLE_WFI_STATE, + { + .enter = ux500_enter_idle, + .exit_latency = 70, + .target_residency = 260, + .flags = CPUIDLE_FLAG_TIME_VALID | + CPUIDLE_FLAG_TIMER_STOP, + .name = "ApIdle", + .desc = "ARM Retention", + }, + }, + .safe_state_index = 0, + .state_count = 2, +}; + +static int __init dbx500_cpuidle_probe(struct platform_device *pdev) +{ + /* Configure wake up reasons */ + prcmu_enable_wakeups(PRCMU_WAKEUP(ARM) | PRCMU_WAKEUP(RTC) | + PRCMU_WAKEUP(ABB)); + + return cpuidle_register(&ux500_idle_driver, NULL); +} + +static struct platform_driver dbx500_cpuidle_plat_driver = { + .driver = { + .name = "cpuidle-dbx500", + .owner = THIS_MODULE, + }, + .probe = dbx500_cpuidle_probe, +}; + +module_platform_driver(dbx500_cpuidle_plat_driver); -- cgit v0.10.2 From c78e43601b8f6d06bd92753e5f4ae51e4d6a1284 Mon Sep 17 00:00:00 2001 From: Geert Uytterhoeven Date: Fri, 26 Jul 2013 00:14:04 +0200 Subject: SH: cpuidle: Add missing parameter for cpuidle_register() arch/sh/kernel/cpu/shmobile/cpuidle.c: In function 'sh_mobile_setup_cpuidle': arch/sh/kernel/cpu/shmobile/cpuidle.c:102:2: error: too few arguments to function 'cpuidle_register' include/linux/cpuidle.h:129:12: note: declared here arch/sh/kernel/cpu/shmobile/cpuidle.c:94:6: warning: unused variable 'ret' [-Wunused-variable] Add the missing parameter, and remove the unused variable. Both introduced by commit b181a3b03f866776211f22390c185c4da1dc45ff ("SH: cpuidle: use init/exit common routine"). Signed-off-by: Geert Uytterhoeven Signed-off-by: Daniel Lezcano diff --git a/arch/sh/kernel/cpu/shmobile/cpuidle.c b/arch/sh/kernel/cpu/shmobile/cpuidle.c index d306225..e3abfd4 100644 --- a/arch/sh/kernel/cpu/shmobile/cpuidle.c +++ b/arch/sh/kernel/cpu/shmobile/cpuidle.c @@ -91,13 +91,11 @@ static struct cpuidle_driver cpuidle_driver = { int __init sh_mobile_setup_cpuidle(void) { - int ret; - if (sh_mobile_sleep_supported & SUSP_SH_SF) cpuidle_driver.states[1].disabled = false; if (sh_mobile_sleep_supported & SUSP_SH_STANDBY) cpuidle_driver.states[2].disabled = false; - return cpuidle_register(&cpuidle_driver); + return cpuidle_register(&cpuidle_driver, NULL); } -- cgit v0.10.2 From 3e0c190dbf51f587ac78b3b1bcd85648a7dbe166 Mon Sep 17 00:00:00 2001 From: Jingoo Han Date: Fri, 9 Aug 2013 16:09:15 +0900 Subject: cpuidle: calxeda: Add missing __iomem annotation Added missing __iomem annotation in order to fix the following sparse warnings: drivers/cpuidle/cpuidle-calxeda.c:44:24: warning: incorrect type in argument 1 (different address spaces) drivers/cpuidle/cpuidle-calxeda.c:44:24: expected void [noderef] * drivers/cpuidle/cpuidle-calxeda.c:44:24: got void *extern [addressable] [toplevel] scu_base_addr drivers/cpuidle/cpuidle-calxeda.c:56:24: warning: incorrect type in argument 1 (different address spaces) drivers/cpuidle/cpuidle-calxeda.c:56:24: expected void [noderef] * drivers/cpuidle/cpuidle-calxeda.c:56:24: got void *extern [addressable] [toplevel] scu_base_addr Signed-off-by: Jingoo Han Signed-off-by: Daniel Lezcano diff --git a/drivers/cpuidle/cpuidle-calxeda.c b/drivers/cpuidle/cpuidle-calxeda.c index 0e6e408..3460584 100644 --- a/drivers/cpuidle/cpuidle-calxeda.c +++ b/drivers/cpuidle/cpuidle-calxeda.c @@ -35,7 +35,7 @@ #include extern void highbank_set_cpu_jump(int cpu, void *jump_addr); -extern void *scu_base_addr; +extern void __iomem *scu_base_addr; static noinline void calxeda_idle_restore(void) { -- cgit v0.10.2 From 75d6137da195a1e059e1c814cb95635e168f85c0 Mon Sep 17 00:00:00 2001 From: Jingoo Han Date: Fri, 9 Aug 2013 16:11:34 +0900 Subject: cpuidle: kirkwood: Make kirkwood_cpuidle_remove function static This local symbol is used only in this file. Fix the following sparse warnings: drivers/cpuidle/cpuidle-kirkwood.c:73:5: warning: symbol 'kirkwood_cpuidle_remove' was not declared. Should it be static ? Signed-off-by: Jingoo Han Acked-by: Jason Cooper Signed-off-by: Daniel Lezcano diff --git a/drivers/cpuidle/cpuidle-kirkwood.c b/drivers/cpuidle/cpuidle-kirkwood.c index 521b0a7..3ca3a2e 100644 --- a/drivers/cpuidle/cpuidle-kirkwood.c +++ b/drivers/cpuidle/cpuidle-kirkwood.c @@ -70,7 +70,7 @@ static int kirkwood_cpuidle_probe(struct platform_device *pdev) return cpuidle_register(&kirkwood_idle_driver, NULL); } -int kirkwood_cpuidle_remove(struct platform_device *pdev) +static int kirkwood_cpuidle_remove(struct platform_device *pdev) { cpuidle_unregister(&kirkwood_idle_driver); return 0; -- cgit v0.10.2 From 81455e72211e41972cca1d4686bc07b2e93b81e1 Mon Sep 17 00:00:00 2001 From: Julia Lawall Date: Wed, 14 Aug 2013 11:11:12 +0200 Subject: cpuidle-kirkwood.c: simplify use of devm_ioremap_resource() Remove unneeded error handling on the result of a call to platform_get_resource when the value is passed to devm_ioremap_resource(). A simplified version of the semantic patch that makes this change is as follows: (http://coccinelle.lip6.fr/) // @@ expression pdev,res,n,e,e1; expression ret != 0; identifier l; @@ - res = platform_get_resource(pdev, IORESOURCE_MEM, n); ... when != res - if (res == NULL) { ... \(goto l;\|return ret;\) } ... when != res + res = platform_get_resource(pdev, IORESOURCE_MEM, n); e = devm_ioremap_resource(e1, res); // Signed-off-by: Julia Lawall Signed-off-by: Rafael J. Wysocki diff --git a/drivers/cpuidle/cpuidle-kirkwood.c b/drivers/cpuidle/cpuidle-kirkwood.c index 3ca3a2e..41ba843 100644 --- a/drivers/cpuidle/cpuidle-kirkwood.c +++ b/drivers/cpuidle/cpuidle-kirkwood.c @@ -60,9 +60,6 @@ static int kirkwood_cpuidle_probe(struct platform_device *pdev) struct resource *res; res = platform_get_resource(pdev, IORESOURCE_MEM, 0); - if (res == NULL) - return -EINVAL; - ddr_operation_base = devm_ioremap_resource(&pdev->dev, res); if (IS_ERR(ddr_operation_base)) return PTR_ERR(ddr_operation_base); -- cgit v0.10.2 From 330647a9501fe8f93a8ae9361417e51ee0bebd7e Mon Sep 17 00:00:00 2001 From: Tuukka Tikkanen Date: Wed, 14 Aug 2013 19:02:34 +0300 Subject: cpuidle: Ignore interval prediction result when timer is shorter This patch prevents cpuidle menu governor from using repeating interval prediction result if the idle period predicted is longer than the one allowed by shortest running timer. Signed-off-by: Tuukka Tikkanen Signed-off-by: Rafael J. Wysocki diff --git a/drivers/cpuidle/governors/menu.c b/drivers/cpuidle/governors/menu.c index cbbb73b..3516973 100644 --- a/drivers/cpuidle/governors/menu.c +++ b/drivers/cpuidle/governors/menu.c @@ -238,10 +238,13 @@ again: * * The typical interval is obtained when standard deviation is small * or standard deviation is small compared to the average interval. + * + * Use this result only if there is no timer to wake us up sooner. */ if (((avg > stddev * 6) && (divisor * 4 >= INTERVALS * 3)) || stddev <= 20) { - data->predicted_us = avg; + if (data->expected_us > avg) + data->predicted_us = avg; return; } else if ((divisor * 4) > INTERVALS * 3) { -- cgit v0.10.2 From 017099e25fb7e482a249d36a654556d32f601f71 Mon Sep 17 00:00:00 2001 From: Tuukka Tikkanen Date: Wed, 14 Aug 2013 19:02:35 +0300 Subject: cpuidle: Rearrange code and comments in get_typical_interval() This patch rearranges a if-return-elsif-goto-fi-return sequence into if-return-fi-if-return-fi-goto sequence. The functionality remains the same. Also, a lengthy comment that did not describe the functionality in the order it occurs is split into half and top half is moved closer to actual implementation it describes. Signed-off-by: Tuukka Tikkanen Signed-off-by: Rafael J. Wysocki diff --git a/drivers/cpuidle/governors/menu.c b/drivers/cpuidle/governors/menu.c index 3516973..f1fadbe 100644 --- a/drivers/cpuidle/governors/menu.c +++ b/drivers/cpuidle/governors/menu.c @@ -228,14 +228,6 @@ again: do_div(stddev, divisor); stddev = int_sqrt(stddev); /* - * If we have outliers to the upside in our distribution, discard - * those by setting the threshold to exclude these outliers, then - * calculate the average and standard deviation again. Once we get - * down to the bottom 3/4 of our samples, stop excluding samples. - * - * This can deal with workloads that have long pauses interspersed - * with sporadic activity with a bunch of short pauses. - * * The typical interval is obtained when standard deviation is small * or standard deviation is small compared to the average interval. * @@ -246,12 +238,22 @@ again: if (data->expected_us > avg) data->predicted_us = avg; return; - - } else if ((divisor * 4) > INTERVALS * 3) { - /* Exclude the max interval */ - thresh = max - 1; - goto again; } + + /* + * If we have outliers to the upside in our distribution, discard + * those by setting the threshold to exclude these outliers, then + * calculate the average and standard deviation again. Once we get + * down to the bottom 3/4 of our samples, stop excluding samples. + * + * This can deal with workloads that have long pauses interspersed + * with sporadic activity with a bunch of short pauses. + */ + if ((divisor * 4) <= INTERVALS * 3) + return; + + thresh = max - 1; + goto again; } /** -- cgit v0.10.2 From 0d6a7ffa4cc67cc70bf1f4e24fbb0747632845a2 Mon Sep 17 00:00:00 2001 From: Tuukka Tikkanen Date: Wed, 14 Aug 2013 19:02:36 +0300 Subject: cpuidle: Check called function parameter in get_typical_interval() get_typical_interval() uses int_sqrt() in calculation of standard deviation. The formal parameter of int_sqrt() is unsigned long, which may on some platforms be smaller than the 64 bit unsigned integer used as the actual parameter. The overflow can occur frequently when actual idle period lengths are in hundreds of milliseconds. This patch adds a check for such overflow and rejects the candidate average when an overflow would occur. Signed-off-by: Tuukka Tikkanen Signed-off-by: Rafael J. Wysocki diff --git a/drivers/cpuidle/governors/menu.c b/drivers/cpuidle/governors/menu.c index f1fadbe..af7b3f6 100644 --- a/drivers/cpuidle/governors/menu.c +++ b/drivers/cpuidle/governors/menu.c @@ -226,18 +226,26 @@ again: } } do_div(stddev, divisor); - stddev = int_sqrt(stddev); /* * The typical interval is obtained when standard deviation is small * or standard deviation is small compared to the average interval. * + * int_sqrt() formal parameter type is unsigned long. When the + * greatest difference to an outlier exceeds ~65 ms * sqrt(divisor) + * the resulting squared standard deviation exceeds the input domain + * of int_sqrt on platforms where unsigned long is 32 bits in size. + * In such case reject the candidate average. + * * Use this result only if there is no timer to wake us up sooner. */ - if (((avg > stddev * 6) && (divisor * 4 >= INTERVALS * 3)) + if (likely(stddev <= ULONG_MAX)) { + stddev = int_sqrt(stddev); + if (((avg > stddev * 6) && (divisor * 4 >= INTERVALS * 3)) || stddev <= 20) { - if (data->expected_us > avg) - data->predicted_us = avg; - return; + if (data->expected_us > avg) + data->predicted_us = avg; + return; + } } /* -- cgit v0.10.2 From 4cd46bca8ca8ad20cadf74fd9b76364c46ed0f31 Mon Sep 17 00:00:00 2001 From: Tuukka Tikkanen Date: Wed, 14 Aug 2013 19:02:37 +0300 Subject: cpuidle: CodingStyle: Break up multiple assignments on single line The function get_typical_interval() initializes a number of variables that are immediately after declarations assigned constant values. In addition, there are multiple assignments on a single line, which is explicitly forbidden by Documentation/CodingStyle. This patch removes redundant initial values for the variables and breaks up the multiple assignment line. Signed-off-by: Tuukka Tikkanen Signed-off-by: Rafael J. Wysocki diff --git a/drivers/cpuidle/governors/menu.c b/drivers/cpuidle/governors/menu.c index af7b3f6..94a3037 100644 --- a/drivers/cpuidle/governors/menu.c +++ b/drivers/cpuidle/governors/menu.c @@ -199,14 +199,17 @@ static u64 div_round64(u64 dividend, u32 divisor) */ static void get_typical_interval(struct menu_device *data) { - int i = 0, divisor = 0; - uint64_t max = 0, avg = 0, stddev = 0; + int i, divisor; + uint64_t max, avg, stddev; int64_t thresh = LLONG_MAX; /* Discard outliers above this value. */ again: /* first calculate average and standard deviation of the past */ - max = avg = divisor = stddev = 0; + max = 0; + avg = 0; + divisor = 0; + stddev = 0; for (i = 0; i < INTERVALS; i++) { int64_t value = data->intervals[i]; if (value <= thresh) { -- cgit v0.10.2 From 939e33b7fcd4980f21ff4c9558eb27fe81d16cdb Mon Sep 17 00:00:00 2001 From: Tuukka Tikkanen Date: Wed, 14 Aug 2013 19:02:38 +0300 Subject: cpuidle: Fix menu_device->intervals type Struct menu_device member intervals is declared as u32, but the value stored is (unsigned) int. The type is changed to match the value being stored. Signed-off-by: Tuukka Tikkanen Signed-off-by: Rafael J. Wysocki diff --git a/drivers/cpuidle/governors/menu.c b/drivers/cpuidle/governors/menu.c index 94a3037..ee88838 100644 --- a/drivers/cpuidle/governors/menu.c +++ b/drivers/cpuidle/governors/menu.c @@ -118,7 +118,7 @@ struct menu_device { unsigned int exit_us; unsigned int bucket; u64 correction_factor[BUCKETS]; - u32 intervals[INTERVALS]; + unsigned int intervals[INTERVALS]; int interval_ptr; }; -- cgit v0.10.2 From 0e96d5adcfef22f86e4463909728d63f88944749 Mon Sep 17 00:00:00 2001 From: Tuukka Tikkanen Date: Wed, 14 Aug 2013 19:02:39 +0300 Subject: cpuidle: Fix variable domains in get_typical_interval() The menu governor uses a static function get_typical_interval() to try to detect a repeating pattern of wakeups. The previous interval durations are stored as an array of unsigned ints, but the arithmetic in the function is performed exclusively as 64 bit values, even when the value stored in a variable is known not to exceed unsigned int, which may be smaller and more efficient on some platforms. This patch changes the types of varibles used to store some intermediates, the maximum and and the cutoff threshold to unsigned ints. Average and standard deviation are still treated as 64 bit values, even when the values are known to be within the domain of unsigned int, to avoid casts to ensure correct integer promotion for arithmetic operations. Signed-off-by: Tuukka Tikkanen Signed-off-by: Rafael J. Wysocki diff --git a/drivers/cpuidle/governors/menu.c b/drivers/cpuidle/governors/menu.c index ee88838..a56081c 100644 --- a/drivers/cpuidle/governors/menu.c +++ b/drivers/cpuidle/governors/menu.c @@ -200,18 +200,19 @@ static u64 div_round64(u64 dividend, u32 divisor) static void get_typical_interval(struct menu_device *data) { int i, divisor; - uint64_t max, avg, stddev; - int64_t thresh = LLONG_MAX; /* Discard outliers above this value. */ + unsigned int max, thresh; + uint64_t avg, stddev; + + thresh = UINT_MAX; /* Discard outliers above this value */ again: - /* first calculate average and standard deviation of the past */ + /* First calculate the average of past intervals */ max = 0; avg = 0; divisor = 0; - stddev = 0; for (i = 0; i < INTERVALS; i++) { - int64_t value = data->intervals[i]; + unsigned int value = data->intervals[i]; if (value <= thresh) { avg += value; divisor++; @@ -221,8 +222,10 @@ again: } do_div(avg, divisor); + /* Then try to determine standard deviation */ + stddev = 0; for (i = 0; i < INTERVALS; i++) { - int64_t value = data->intervals[i]; + unsigned int value = data->intervals[i]; if (value <= thresh) { int64_t diff = value - avg; stddev += diff * diff; -- cgit v0.10.2 From decd51bbcd7fd949840da4cc634f6b70baa1b512 Mon Sep 17 00:00:00 2001 From: Tuukka Tikkanen Date: Wed, 14 Aug 2013 19:02:40 +0300 Subject: cpuidle: Add a comment warning about possible overflow The menu governor has a number of tunable constants that may be changed in the source. If certain combination of values are chosen, an overflow is possible when the correction_factor is being recalculated. This patch adds a warning regarding this possibility and describes the change needed for fixing the issue. The change should not be permanently enabled, as it will hurt performance when it is not needed. Signed-off-by: Tuukka Tikkanen Signed-off-by: Rafael J. Wysocki diff --git a/drivers/cpuidle/governors/menu.c b/drivers/cpuidle/governors/menu.c index a56081c..a8b31b0 100644 --- a/drivers/cpuidle/governors/menu.c +++ b/drivers/cpuidle/governors/menu.c @@ -21,6 +21,15 @@ #include #include +/* + * Please note when changing the tuning values: + * If (MAX_INTERESTING-1) * RESOLUTION > UINT_MAX, the result of + * a scaling operation multiplication may overflow on 32 bit platforms. + * In that case, #define RESOLUTION as ULL to get 64 bit result: + * #define RESOLUTION 1024ULL + * + * The default values do not overflow. + */ #define BUCKETS 12 #define INTERVALS 8 #define RESOLUTION 1024 -- cgit v0.10.2 From 51f245b895e3fe4cbac0b2633e54a1830864a83f Mon Sep 17 00:00:00 2001 From: Tuukka Tikkanen Date: Wed, 14 Aug 2013 19:02:41 +0300 Subject: cpuidle: Change struct menu_device field types Field predicted_us value can never exceed expected_us value, but it has a potentially larger type. As there is no need for additional 32 bits of zeroes on 32 bit plaforms, change the type of predicted_us to match the type of expected_us. Field correction_factor is used to store a value that cannot exceed the product of RESOLUTION and DECAY (default 1024*8 = 8192). The constants cannot in practice be incremented to such values, that they'd overflow unsigned int even on 32 bit systems, so the type is changed to avoid unnecessary 64 bit arithmetic on 32 bit systems. One multiplication of (now) 32 bit values needs an added cast to avoid truncation of the result and has been added. In order to avoid another multiplication from 32 bit domain to 64 bit domain, the new correction_factor calculation has been changed from new = old * (DECAY-1) / DECAY to new = old - old / DECAY, which with infinite precision would yeild exactly the same result, but now changes the direction of rounding. The impact is not significant as the maximum accumulated difference cannot exceed the value of DECAY, which is relatively small compared to product of RESOLUTION and DECAY (8 / 8192). Signed-off-by: Tuukka Tikkanen Signed-off-by: Rafael J. Wysocki diff --git a/drivers/cpuidle/governors/menu.c b/drivers/cpuidle/governors/menu.c index a8b31b0..cf7f2f0 100644 --- a/drivers/cpuidle/governors/menu.c +++ b/drivers/cpuidle/governors/menu.c @@ -123,10 +123,10 @@ struct menu_device { int needs_update; unsigned int expected_us; - u64 predicted_us; + unsigned int predicted_us; unsigned int exit_us; unsigned int bucket; - u64 correction_factor[BUCKETS]; + unsigned int correction_factor[BUCKETS]; unsigned int intervals[INTERVALS]; int interval_ptr; }; @@ -321,8 +321,13 @@ static int menu_select(struct cpuidle_driver *drv, struct cpuidle_device *dev) if (data->correction_factor[data->bucket] == 0) data->correction_factor[data->bucket] = RESOLUTION * DECAY; - /* Make sure to round up for half microseconds */ - data->predicted_us = div_round64(data->expected_us * data->correction_factor[data->bucket], + /* + * Force the result of multiplication to be 64 bits even if both + * operands are 32 bits. + * Make sure to round up for half microseconds. + */ + data->predicted_us = div_round64((uint64_t)data->expected_us * + data->correction_factor[data->bucket], RESOLUTION * DECAY); get_typical_interval(data); @@ -388,7 +393,7 @@ static void menu_update(struct cpuidle_driver *drv, struct cpuidle_device *dev) unsigned int last_idle_us = cpuidle_get_last_residency(dev); struct cpuidle_state *target = &drv->states[last_idx]; unsigned int measured_us; - u64 new_factor; + unsigned int new_factor; /* * Ugh, this idle state doesn't support residency measurements, so we @@ -409,10 +414,9 @@ static void menu_update(struct cpuidle_driver *drv, struct cpuidle_device *dev) measured_us -= data->exit_us; - /* update our correction ratio */ - - new_factor = data->correction_factor[data->bucket] - * (DECAY - 1) / DECAY; + /* Update our correction ratio */ + new_factor = data->correction_factor[data->bucket]; + new_factor -= new_factor / DECAY; if (data->expected_us > 0 && measured_us < MAX_INTERESTING) new_factor += RESOLUTION * measured_us / data->expected_us; @@ -425,9 +429,11 @@ static void menu_update(struct cpuidle_driver *drv, struct cpuidle_device *dev) /* * We don't want 0 as factor; we always want at least - * a tiny bit of estimated time. + * a tiny bit of estimated time. Fortunately, due to rounding, + * new_factor will stay nonzero regardless of measured_us values + * and the compiler can eliminate this test as long as DECAY > 1. */ - if (new_factor == 0) + if (DECAY == 1 && unlikely(new_factor == 0)) new_factor = 1; data->correction_factor[data->bucket] = new_factor; -- cgit v0.10.2