some cpuidle code considerations and improvements
Subsystems (11)
Hi All,
I am investigating on how we consolidate the cpuidle code and factor out some parts in order to move the duplicate code we found in the different arch specific cpuidle code to the core where it is the place it belongs to.
AFAICS, the cpuidle modules can only be loaded at once but we have in each cpuidle file a struct cpuidle_driver global static variable duplicated.
Would it makes sense to have single struct cpuidle_driver for driver/cpuidle/driver.c and let the different arch specific to register the module with their name ?
Here is an example to illustrate the idea:
arch/arm/mach-at91/cpuidle.c | 7 +------ arch/arm/mach-davinci/cpuidle.c | 7 +------ arch/arm/mach-exynos4/cpuidle.c | 7 +------ arch/arm/mach-kirkwood/cpuidle.c | 7 +------ arch/arm/mach-omap2/cpuidle34xx.c | 7 +------ arch/arm/mach-shmobile/cpuidle.c | 6 +----- arch/sh/kernel/cpu/shmobile/cpuidle.c | 6 +----- drivers/acpi/processor_driver.c | 9 ++++----- drivers/acpi/processor_idle.c | 5 ----- drivers/cpuidle/driver.c | 31 ++++++++++++------------------- drivers/cpuidle/sysfs.c | 2 +- drivers/idle/intel_idle.c | 10 +++------- include/linux/cpuidle.h | 5 +++-- 13 files changed, 30 insertions(+), 79 deletions(-)
drivers/cpuidle/driver.c 2011-11-03 16:53:07.843291270 +0100 renamed from drivers/cpuidle/driver.c 2011-11-03 16:50:57.866646738 +0100 +12 -19
@@ -14,30 +14,30 @@ #include "cpuidle.h" -static struct cpuidle_driver *cpuidle_curr_driver; +static struct cpuidle_driver driver = { + .owner = THIS_MODULE +}; + DEFINE_SPINLOCK(cpuidle_driver_lock); /** * cpuidle_register_driver - registers a driver * @drv: the driver */ -int cpuidle_register_driver(struct cpuidle_driver *drv) +int cpuidle_register_driver(const char *name) { - if (!drv) + if (!name) return -EINVAL; if (cpuidle_disabled()) return -ENODEV; spin_lock(&cpuidle_driver_lock); - if (cpuidle_curr_driver) { - spin_unlock(&cpuidle_driver_lock); - return -EBUSY; - } - cpuidle_curr_driver = drv; + if (!driver.name) + driver.name = name; spin_unlock(&cpuidle_driver_lock); - return 0; + return driver.name == name ? 0 : -EBUSY; } EXPORT_SYMBOL_GPL(cpuidle_register_driver);
@@ -47,7 +47,7 @@ */ struct cpuidle_driver *cpuidle_get_driver(void) { - return cpuidle_curr_driver; + return &driver; } EXPORT_SYMBOL_GPL(cpuidle_get_driver);
@@ -55,17 +55,10 @@ * cpuidle_unregister_driver - unregisters a driver * @drv: the driver */ -void cpuidle_unregister_driver(struct cpuidle_driver *drv) +void cpuidle_unregister_driver(void) { - if (drv != cpuidle_curr_driver) { - WARN(1, "invalid cpuidle_unregister_driver(%s)\n", - drv->name); - return; - } - spin_lock(&cpuidle_driver_lock); - cpuidle_curr_driver = NULL; + driver.name = NULL; spin_unlock(&cpuidle_driver_lock); } - EXPORT_SYMBOL_GPL(cpuidle_unregister_driver);
include/linux/cpuidle.h 2011-11-03 16:53:07.843291270 +0100 renamed from include/linux/cpuidle.h 2011-11-03 16:53:07.823291175 +0100 +3 -2
@@ -125,9 +125,10 @@ extern void disable_cpuidle(void); extern int cpuidle_idle_call(void); -extern int cpuidle_register_driver(struct cpuidle_driver *drv); +extern int cpuidle_register_driver(const char *name); struct cpuidle_driver *cpuidle_get_driver(void); -extern void cpuidle_unregister_driver(struct cpuidle_driver *drv); +extern void cpuidle_unregister_driver(void); + extern int cpuidle_register_device(struct cpuidle_device *dev); extern void cpuidle_unregister_device(struct cpuidle_device *dev);
arch/arm/mach-at91/cpuidle.c 2011-11-03 16:53:07.843291270 +0100 renamed from arch/arm/mach-at91/cpuidle.c 2011-11-03 16:50:57.914646997 +0100 +1 -6
@@ -26,11 +26,6 @@ static DEFINE_PER_CPU(struct cpuidle_device, at91_cpuidle_device); -static struct cpuidle_driver at91_idle_driver = { - .name = "at91_idle", - .owner = THIS_MODULE, -}; - /* Actual code that puts the SoC in different idle states */ static int at91_enter_idle(struct cpuidle_device *dev, struct cpuidle_state *state)
@@ -63,7 +58,7 @@ { struct cpuidle_device *device; - cpuidle_register_driver(&at91_idle_driver); + cpuidle_register_driver("at91_idle"); device = &per_cpu(at91_cpuidle_device, smp_processor_id()); device->state_count = AT91_MAX_STATES;
arch/arm/mach-davinci/cpuidle.c 2011-11-03 16:53:07.843291270 +0100 renamed from arch/arm/mach-davinci/cpuidle.c 2011-11-03 16:50:57.906646945 +0100 +1 -6
@@ -32,11 +32,6 @@ /* fields in davinci_ops.flags */ #define DAVINCI_CPUIDLE_FLAGS_DDR2_PWDN BIT(0) -static struct cpuidle_driver davinci_idle_driver = { - .name = "cpuidle-davinci", - .owner = THIS_MODULE, -}; - static DEFINE_PER_CPU(struct cpuidle_device, davinci_cpuidle_device); static void __iomem *ddr2_reg_base;
@@ -116,7 +111,7 @@ ddr2_reg_base = pdata->ddr2_ctlr_base; - ret = cpuidle_register_driver(&davinci_idle_driver); + ret = cpuidle_register_driver("cpuidle-davinci"); if (ret) { dev_err(&pdev->dev, "failed to register driver\n"); return ret;
arch/arm/mach-exynos4/cpuidle.c 2011-11-03 16:53:07.843291270 +0100 renamed from arch/arm/mach-exynos4/cpuidle.c 2011-11-03 16:50:57.882646829 +0100 +1 -6
@@ -31,11 +31,6 @@ static DEFINE_PER_CPU(struct cpuidle_device, exynos4_cpuidle_device); -static struct cpuidle_driver exynos4_idle_driver = { - .name = "exynos4_idle", - .owner = THIS_MODULE, -}; - static int exynos4_enter_idle(struct cpuidle_device *dev, struct cpuidle_state *state) {
@@ -60,7 +55,7 @@ int i, max_cpuidle_state, cpu_id; struct cpuidle_device *device; - cpuidle_register_driver(&exynos4_idle_driver); + cpuidle_register_driver("exynos4_idle"); for_each_cpu(cpu_id, cpu_online_mask) { device = &per_cpu(exynos4_cpuidle_device, cpu_id);
arch/arm/mach-kirkwood/cpuidle.c 2011-11-03 16:53:07.847291288 +0100 renamed from arch/arm/mach-kirkwood/cpuidle.c 2011-11-03 16:50:57.926647050 +0100 +1 -6
@@ -23,11 +23,6 @@ #define KIRKWOOD_MAX_STATES 2 -static struct cpuidle_driver kirkwood_idle_driver = { - .name = "kirkwood_idle", - .owner = THIS_MODULE, -}; - static DEFINE_PER_CPU(struct cpuidle_device, kirkwood_cpuidle_device); /* Actual code that puts the SoC in different idle states */
@@ -65,7 +60,7 @@ { struct cpuidle_device *device; - cpuidle_register_driver(&kirkwood_idle_driver); + cpuidle_register_driver("kirkwood_idle"); device = &per_cpu(kirkwood_cpuidle_device, smp_processor_id()); device->state_count = KIRKWOOD_MAX_STATES;
arch/arm/mach-omap2/cpuidle34xx.c 2011-11-03 16:53:07.847291288 +0100 renamed from arch/arm/mach-omap2/cpuidle34xx.c 2011-11-03 16:50:57.890646862 +0100 +1 -6
@@ -296,11 +296,6 @@ return; } -struct cpuidle_driver omap3_idle_driver = { - .name = "omap3_idle", - .owner = THIS_MODULE, -}; - /* Helper to fill the C-state common data and register the driver_data */ static inline struct omap3_idle_statedata *_fill_cstate( struct cpuidle_device *dev,
@@ -337,7 +332,7 @@ per_pd = pwrdm_lookup("per_pwrdm"); cam_pd = pwrdm_lookup("cam_pwrdm"); - cpuidle_register_driver(&omap3_idle_driver); + cpuidle_register_driver("omap3_idle"); dev = &per_cpu(omap3_idle_dev, smp_processor_id()); /* C1 . MPU WFI + Core active */
arch/arm/mach-shmobile/cpuidle.c 2011-11-03 16:53:07.847291288 +0100 renamed from arch/arm/mach-shmobile/cpuidle.c 2011-11-03 16:50:57.898646916 +0100 +1 -5
@@ -47,10 +47,6 @@ } static struct cpuidle_device shmobile_cpuidle_dev; -static struct cpuidle_driver shmobile_cpuidle_driver = { - .name = "shmobile_cpuidle", - .owner = THIS_MODULE, -}; void (*shmobile_cpuidle_setup)(struct cpuidle_device *dev);
@@ -60,7 +56,7 @@ struct cpuidle_state *state; int i; - cpuidle_register_driver(&shmobile_cpuidle_driver); + cpuidle_register_driver("shmobile_cpuidle"); for (i = 0; i < CPUIDLE_STATE_MAX; i++) { dev->states[i].name[0] = '\0';
arch/sh/kernel/cpu/shmobile/cpuidle.c 2011-11-03 16:53:07.847291288 +0100 renamed from arch/sh/kernel/cpu/shmobile/cpuidle.c 2011-11-03 16:50:57.874646801 +0100 +1 -5
@@ -54,10 +54,6 @@ } static struct cpuidle_device cpuidle_dev; -static struct cpuidle_driver cpuidle_driver = { - .name = "sh_idle", - .owner = THIS_MODULE, -}; void sh_mobile_setup_cpuidle(void) {
@@ -65,7 +61,7 @@ struct cpuidle_state *state; int i; - cpuidle_register_driver(&cpuidle_driver); + cpuidle_register_driver("sh_idle"); for (i = 0; i < CPUIDLE_STATE_MAX; i++) { dev->states[i].name[0] = '\0';
drivers/acpi/processor_driver.c 2011-11-03 16:53:07.847291288 +0100 renamed from drivers/acpi/processor_driver.c 2011-11-03 16:53:07.831291213 +0100 +3 -4
@@ -801,9 +801,8 @@ memset(&errata, 0, sizeof(errata)); - if (!cpuidle_register_driver(&acpi_idle_driver)) { - printk(KERN_DEBUG "ACPI: %s registered with cpuidle\n", - acpi_idle_driver.name); + if (!cpuidle_register_driver("acpi_idle")) { + printk(KERN_DEBUG "ACPI: acpi_idle registered with cpuidle\n"); acpi_cpuidle_driver = true; } else { printk(KERN_DEBUG "ACPI: acpi_idle yielding to %s\n",
@@ -825,7 +824,7 @@ return 0; out_cpuidle: - cpuidle_unregister_driver(&acpi_idle_driver); + cpuidle_unregister_driver(); return result;
}
@@ -843,7 +842,7 @@
acpi_bus_unregister_driver(&acpi_processor_driver);
- cpuidle_unregister_driver(&acpi_idle_driver);
+ cpuidle_unregister_driver();
return;
}
drivers/acpi/processor_idle.c 2011-11-03 16:53:07.847291288 +0100 renamed from drivers/acpi/processor_idle.c 2011-11-03 16:50:57.834646616 +0100 +0 -5
@@ -968,11 +968,6 @@ return idle_time; } -struct cpuidle_driver acpi_idle_driver = { - .name = "acpi_idle", - .owner = THIS_MODULE, -}; - /** * acpi_processor_setup_cpuidle - prepares and configures CPUIDLE * @pr: the ACPI processor
drivers/idle/intel_idle.c 2011-11-03 17:25:15.750834746 +0100 renamed from drivers/idle/intel_idle.c 2011-11-03 16:50:57.850646654 +0100 +3 -7
@@ -67,10 +67,6 @@ #define INTEL_IDLE_VERSION "0.4" #define PREFIX "intel_idle: " -static struct cpuidle_driver intel_idle_driver = { - .name = "intel_idle", - .owner = THIS_MODULE, -}; /* intel_idle.max_cstate=0 disables driver */ static int max_cstate = MWAIT_MAX_NUM_CSTATES - 1;
@@ -477,7 +473,7 @@ if (retval) return retval; - retval = cpuidle_register_driver(&intel_idle_driver); + retval = cpuidle_register_driver("intel_idle"); if (retval) { printk(KERN_DEBUG PREFIX "intel_idle yielding to %s", cpuidle_get_driver()->name);
@@ -486,7 +482,7 @@ retval = intel_idle_cpuidle_devices_init(); if (retval) { - cpuidle_unregister_driver(&intel_idle_driver); + cpuidle_unregister_driver(); return retval; }
@@ -496,7 +492,7 @@ static void __exit intel_idle_exit(void) { intel_idle_cpuidle_devices_uninit(); - cpuidle_unregister_driver(&intel_idle_driver); + cpuidle_unregister_driver(); if (lapic_timer_reliable_states != LAPIC_TIMER_ALWAYS_RELIABLE) { smp_call_function(__setup_broadcast_timer, (void
*)false, 1);
drivers/cpuidle/sysfs.c 2011-11-03 16:54:02.123560434 +0100 renamed from drivers/cpuidle/sysfs.c 2011-11-03 16:53:23.279367823 +0100 +1 -1
@@ -50,7 +50,7 @@ struct cpuidle_driver *cpuidle_driver = cpuidle_get_driver(); spin_lock(&cpuidle_driver_lock); - if (cpuidle_driver) + if (cpuidle_driver->name) ret = sprintf(buf, "%s\n", cpuidle_driver->name); else ret = sprintf(buf, "none\n");
Signature · 10 lines
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | <http://twitter.com/#!/linaroorg> Twitter | <http://www.linaro.org/linaro-blog/> Blog -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html