some cpuidle code considerations and improvements

Daniel Lezcano
2011-11-03T19:18:16Z

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(-)

Index: net/drivers/cpuidle/driver.c
===================================================================
--- net.orig/drivers/cpuidle/driver.c 2011-11-03 16:50:57.866646738 +0100
+++ net/drivers/cpuidle/driver.c 2011-11-03 16:53:07.843291270 +0100
@@ -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);
Index: net/include/linux/cpuidle.h
===================================================================
--- net.orig/include/linux/cpuidle.h 2011-11-03 16:53:07.823291175 +0100
+++ net/include/linux/cpuidle.h 2011-11-03 16:53:07.843291270 +0100
@@ -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);

Index: net/arch/arm/mach-at91/cpuidle.c
===================================================================
--- net.orig/arch/arm/mach-at91/cpuidle.c 2011-11-03
16:50:57.914646997 +0100
+++ net/arch/arm/mach-at91/cpuidle.c 2011-11-03 16:53:07.843291270 +0100
@@ -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;
Index: net/arch/arm/mach-davinci/cpuidle.c
===================================================================
--- net.orig/arch/arm/mach-davinci/cpuidle.c 2011-11-03
16:50:57.906646945 +0100
+++ net/arch/arm/mach-davinci/cpuidle.c 2011-11-03 16:53:07.843291270 +0100
@@ -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;
Index: net/arch/arm/mach-exynos4/cpuidle.c
===================================================================
--- net.orig/arch/arm/mach-exynos4/cpuidle.c 2011-11-03
16:50:57.882646829 +0100
+++ net/arch/arm/mach-exynos4/cpuidle.c 2011-11-03 16:53:07.843291270 +0100
@@ -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);
Index: net/arch/arm/mach-kirkwood/cpuidle.c
===================================================================
--- net.orig/arch/arm/mach-kirkwood/cpuidle.c 2011-11-03
16:50:57.926647050 +0100
+++ net/arch/arm/mach-kirkwood/cpuidle.c 2011-11-03
16:53:07.847291288 +0100
@@ -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;
Index: net/arch/arm/mach-omap2/cpuidle34xx.c
===================================================================
--- net.orig/arch/arm/mach-omap2/cpuidle34xx.c 2011-11-03
16:50:57.890646862 +0100
+++ net/arch/arm/mach-omap2/cpuidle34xx.c 2011-11-03
16:53:07.847291288 +0100
@@ -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 */
Index: net/arch/arm/mach-shmobile/cpuidle.c
===================================================================
--- net.orig/arch/arm/mach-shmobile/cpuidle.c 2011-11-03
16:50:57.898646916 +0100
+++ net/arch/arm/mach-shmobile/cpuidle.c 2011-11-03
16:53:07.847291288 +0100
@@ -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';
Index: net/arch/sh/kernel/cpu/shmobile/cpuidle.c
===================================================================
--- net.orig/arch/sh/kernel/cpu/shmobile/cpuidle.c 2011-11-03
16:50:57.874646801 +0100
+++ net/arch/sh/kernel/cpu/shmobile/cpuidle.c 2011-11-03
16:53:07.847291288 +0100
@@ -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';
Index: net/drivers/acpi/processor_driver.c
===================================================================
--- net.orig/drivers/acpi/processor_driver.c 2011-11-03
16:53:07.831291213 +0100
+++ net/drivers/acpi/processor_driver.c 2011-11-03 16:53:07.847291288 +0100
@@ -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;
}
Index: net/drivers/acpi/processor_idle.c
===================================================================
--- net.orig/drivers/acpi/processor_idle.c 2011-11-03
16:50:57.834646616 +0100
+++ net/drivers/acpi/processor_idle.c 2011-11-03 16:53:07.847291288 +0100
@@ -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
Index: net/drivers/idle/intel_idle.c
===================================================================
--- net.orig/drivers/idle/intel_idle.c 2011-11-03 16:50:57.850646654 +0100
+++ net/drivers/idle/intel_idle.c 2011-11-03 17:25:15.750834746 +0100
@@ -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);
Index: net/drivers/cpuidle/sysfs.c
===================================================================
--- net.orig/drivers/cpuidle/sysfs.c 2011-11-03 16:53:23.279367823 +0100
+++ net/drivers/cpuidle/sysfs.c 2011-11-03 16:54:02.123560434 +0100
@@ -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");

--
<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 majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html