cpuidle acpi driver: fix oops on AC<->DC
authorVenkatesh Pallipadi <venkatesh.pallipadi@intel.com>
Mon, 19 May 2008 23:09:27 +0000 (19:09 -0400)
committerLen Brown <len.brown@intel.com>
Wed, 11 Jun 2008 23:13:45 +0000 (19:13 -0400)
cpuidle and acpi driver interaction bug with the way cpuidle_register_driver()
is called. Due to this bug, there will be oops on
AC<->DC on some systems, where they support C-states in one DC and not in AC.

The current code does
ON BOOT:
Look at CST and other C-state info to see whether more than C1 is
supported. If it is, then acpi processor_idle does a
cpuidle_register_driver() call, which internally enables the device.

ON CST change notification (AC<->DC) and on suspend-resume:
acpi driver temporarily disables device, updates the device with
any new C-states, and reenables the device.

The problem is is on boot, there are no C2, C3 states supported and we skip
the register. Later on AC<->DC, we may get a CST notification and we try
to reevaluate CST and enabled the device, without actually registering it.
This causes breakage as we try to create /sys fs sub directory, without the
parent directory which is created at register time.

Thanks to Sanjeev for reporting the problem here.
http://bugzilla.kernel.org/show_bug.cgi?id=10394

Signed-off-by: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>
Signed-off-by: Len Brown <len.brown@intel.com>
drivers/acpi/processor_idle.c
drivers/cpuidle/cpuidle.c
include/linux/cpuidle.h

index 2dd2c1f3a01ca11bf10ae1543d57f9d3fcd71dea..556ee1585192e10c0f35d79630d6d3a934cbcf8d 100644 (file)
@@ -1669,6 +1669,7 @@ static int acpi_processor_setup_cpuidle(struct acpi_processor *pr)
                return -EINVAL;
        }
 
+       dev->cpu = pr->id;
        for (i = 0; i < CPUIDLE_STATE_MAX; i++) {
                dev->states[i].name[0] = '\0';
                dev->states[i].desc[0] = '\0';
@@ -1738,7 +1739,7 @@ static int acpi_processor_setup_cpuidle(struct acpi_processor *pr)
 
 int acpi_processor_cst_has_changed(struct acpi_processor *pr)
 {
-       int ret;
+       int ret = 0;
 
        if (boot_option_idle_override)
                return 0;
@@ -1756,8 +1757,10 @@ int acpi_processor_cst_has_changed(struct acpi_processor *pr)
        cpuidle_pause_and_lock();
        cpuidle_disable_device(&pr->power.dev);
        acpi_processor_get_power_info(pr);
-       acpi_processor_setup_cpuidle(pr);
-       ret = cpuidle_enable_device(&pr->power.dev);
+       if (pr->flags.power) {
+               acpi_processor_setup_cpuidle(pr);
+               ret = cpuidle_enable_device(&pr->power.dev);
+       }
        cpuidle_resume_and_unlock();
 
        return ret;
@@ -1813,7 +1816,6 @@ int __cpuinit acpi_processor_power_init(struct acpi_processor *pr,
        if (pr->flags.power) {
 #ifdef CONFIG_CPU_IDLE
                acpi_processor_setup_cpuidle(pr);
-               pr->power.dev.cpu = pr->id;
                if (cpuidle_register_device(&pr->power.dev))
                        return -EIO;
 #endif
@@ -1850,8 +1852,7 @@ int acpi_processor_power_exit(struct acpi_processor *pr,
                return 0;
 
 #ifdef CONFIG_CPU_IDLE
-       if (pr->flags.power)
-               cpuidle_unregister_device(&pr->power.dev);
+       cpuidle_unregister_device(&pr->power.dev);
 #endif
        pr->flags.power_setup_done = 0;
 
index fc555a90bb211b651e20c254751ae08f00fffd1a..23554b676d6e1d348964e952705eed5f6ff7cd91 100644 (file)
@@ -38,6 +38,8 @@ static void cpuidle_kick_cpus(void)
 static void cpuidle_kick_cpus(void) {}
 #endif
 
+static int __cpuidle_register_device(struct cpuidle_device *dev);
+
 /**
  * cpuidle_idle_call - the main idle loop
  *
@@ -138,6 +140,12 @@ int cpuidle_enable_device(struct cpuidle_device *dev)
        if (!dev->state_count)
                return -EINVAL;
 
+       if (dev->registered == 0) {
+               ret = __cpuidle_register_device(dev);
+               if (ret)
+                       return ret;
+       }
+
        if ((ret = cpuidle_add_state_sysfs(dev)))
                return ret;
 
@@ -232,10 +240,13 @@ static void poll_idle_init(struct cpuidle_device *dev) {}
 #endif /* CONFIG_ARCH_HAS_CPU_RELAX */
 
 /**
- * cpuidle_register_device - registers a CPU's idle PM feature
+ * __cpuidle_register_device - internal register function called before register
+ * and enable routines
  * @dev: the cpu
+ *
+ * cpuidle_lock mutex must be held before this is called
  */
-int cpuidle_register_device(struct cpuidle_device *dev)
+static int __cpuidle_register_device(struct cpuidle_device *dev)
 {
        int ret;
        struct sys_device *sys_dev = get_cpu_sysdev((unsigned long)dev->cpu);
@@ -247,18 +258,34 @@ int cpuidle_register_device(struct cpuidle_device *dev)
 
        init_completion(&dev->kobj_unregister);
 
-       mutex_lock(&cpuidle_lock);
-
        poll_idle_init(dev);
 
        per_cpu(cpuidle_devices, dev->cpu) = dev;
        list_add(&dev->device_list, &cpuidle_detected_devices);
        if ((ret = cpuidle_add_sysfs(sys_dev))) {
-               mutex_unlock(&cpuidle_lock);
                module_put(cpuidle_curr_driver->owner);
                return ret;
        }
 
+       dev->registered = 1;
+       return 0;
+}
+
+/**
+ * cpuidle_register_device - registers a CPU's idle PM feature
+ * @dev: the cpu
+ */
+int cpuidle_register_device(struct cpuidle_device *dev)
+{
+       int ret;
+
+       mutex_lock(&cpuidle_lock);
+
+       if ((ret = __cpuidle_register_device(dev))) {
+               mutex_unlock(&cpuidle_lock);
+               return ret;
+       }
+
        cpuidle_enable_device(dev);
        cpuidle_install_idle_handler();
 
@@ -278,6 +305,9 @@ void cpuidle_unregister_device(struct cpuidle_device *dev)
 {
        struct sys_device *sys_dev = get_cpu_sysdev((unsigned long)dev->cpu);
 
+       if (dev->registered == 0)
+               return;
+
        cpuidle_pause_and_lock();
 
        cpuidle_disable_device(dev);
index 51e6b1e520e6feeaefaf71d692be486a128d1d22..dcf77fa826b5a3a0fcac47cdf7c5eaccf67f0521 100644 (file)
@@ -82,6 +82,7 @@ struct cpuidle_state_kobj {
 };
 
 struct cpuidle_device {
+       unsigned int            registered:1;
        unsigned int            enabled:1;
        unsigned int            cpu;