[PATCH 1/5] ACPI processor: Do not export acpi_idle_driver in processor.h

[PATCH 1/5] ACPI processor: Do not export acpi_idle_driver in processor.h

[Thomas Renninger]

Instead move its declaration from processor_idle.c to processor_driver.c
and declare it static as it's the only file using this struct.

Cleanup only, no functional change.

Signed-off-by: Thomas Renninger <trenn@suse.de>
CC: Len Brown <lenb@kernel.org>
CC: linux-acpi@vger.kernel.org
---
 drivers/acpi/processor_driver.c |    5 +++++
 drivers/acpi/processor_idle.c   |    5 -----
 include/acpi/processor.h        |    1 -
 3 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
index a4e0f1b..3a4fc0c 100644
--- a/drivers/acpi/processor_driver.c
+++ b/drivers/acpi/processor_driver.c
@@ -106,6 +106,11 @@ static struct acpi_driver acpi_processor_driver = {
 		},
 };
 
+static struct cpuidle_driver acpi_idle_driver = {
+	.name =		"acpi_idle",
+	.owner =	THIS_MODULE,
+};
+
 #define INSTALL_NOTIFY_HANDLER		1
 #define UNINSTALL_NOTIFY_HANDLER	2
 
diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
index 9b88f98..7fed4f3 100644
--- a/drivers/acpi/processor_idle.c
+++ b/drivers/acpi/processor_idle.c
@@ -968,11 +968,6 @@ static int acpi_idle_enter_bm(struct cpuidle_device *dev,
 	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
diff --git a/include/acpi/processor.h b/include/acpi/processor.h
index 67055f1..5b27bc3 100644
--- a/include/acpi/processor.h
+++ b/include/acpi/processor.h
@@ -333,7 +333,6 @@ int acpi_processor_power_exit(struct acpi_processor *pr,
 			      struct acpi_device *device);
 int acpi_processor_suspend(struct acpi_device * device, pm_message_t state);
 int acpi_processor_resume(struct acpi_device * device);
-extern struct cpuidle_driver acpi_idle_driver;
 
 /* in processor_thermal.c */
 int acpi_processor_get_limit_info(struct acpi_processor *pr);
-- 
1.7.6.1

[PATCH 2/5] ACPI processor: Avoid WARN message on processor driver removal

[Thomas Renninger]

Only unregister acpi_idle driver if acpi_idle driver got
registered.

Also add a static acpi_idle_active variable for easy and nicer
checking whether acpi_idle_driver is active (as suggested by
Bjorn).

Signed-off-by: Thomas Renninger <trenn@suse.de>
CC: Len Brown <lenb@kernel.org>
CC: linux-acpi@vger.kernel.org
CC: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/acpi/processor_driver.c |   11 +++++++----
 1 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
index 3a4fc0c..790623f 100644
--- a/drivers/acpi/processor_driver.c
+++ b/drivers/acpi/processor_driver.c
@@ -110,6 +110,7 @@ static struct cpuidle_driver acpi_idle_driver = {
 	.name =		"acpi_idle",
 	.owner =	THIS_MODULE,
 };
+static int acpi_idle_active;
 
 #define INSTALL_NOTIFY_HANDLER		1
 #define UNINSTALL_NOTIFY_HANDLER	2
@@ -508,8 +509,7 @@ static int __cpuinit acpi_processor_add(struct acpi_device *device)
 	acpi_processor_get_throttling_info(pr);
 	acpi_processor_get_limit_info(pr);
 
-
-	if (cpuidle_get_driver() == &acpi_idle_driver)
+	if (acpi_idle_active)
 		acpi_processor_power_init(pr, device);
 
 	pr->cdev = thermal_cooling_device_register("Processor", device,
@@ -808,6 +808,7 @@ static int __init acpi_processor_init(void)
 	if (!cpuidle_register_driver(&acpi_idle_driver)) {
 		printk(KERN_DEBUG "ACPI: %s registered with cpuidle\n",
 			acpi_idle_driver.name);
+		acpi_idle_active = 1;
 	} else {
 		printk(KERN_DEBUG "ACPI: acpi_idle yielding to %s\n",
 			cpuidle_get_driver()->name);
@@ -828,7 +829,8 @@ static int __init acpi_processor_init(void)
 	return 0;
 
 out_cpuidle:
-	cpuidle_unregister_driver(&acpi_idle_driver);
+	if (acpi_idle_active)
+		cpuidle_unregister_driver(&acpi_idle_driver);
 
 	return result;
 }
@@ -846,7 +848,8 @@ static void __exit acpi_processor_exit(void)
 
 	acpi_bus_unregister_driver(&acpi_processor_driver);
 
-	cpuidle_unregister_driver(&acpi_idle_driver);
+	if (acpi_idle_active)
+		cpuidle_unregister_driver(&acpi_idle_driver);
 
 	return;
 }
-- 
1.7.6.1

[PATCH 3/5] intel_idle: Split up and provide per CPU initialization func

[Thomas Renninger]

Function split up, should have no functional change

Signed-off-by: Thomas Renninger <trenn@suse.de>
CC: Len Brown <lenb@kernel.org>
CC: linux-acpi@vger.kernel.org
---
 drivers/idle/intel_idle.c |  108 +++++++++++++++++++++-----------------------
 include/linux/cpuidle.h   |    7 +++
 2 files changed, 59 insertions(+), 56 deletions(-)

diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
index a46dddf..c108d2a 100644
--- a/drivers/idle/intel_idle.c
+++ b/drivers/idle/intel_idle.c
@@ -397,77 +397,67 @@ static void intel_idle_cpuidle_devices_uninit(void)
 	return;
 }
 /*
- * intel_idle_cpuidle_devices_init()
+ * intel_idle_cpu_init()
  * allocate, initialize, register cpuidle_devices
+ * @cpu: cpu/core to initialize
  */
-static int intel_idle_cpuidle_devices_init(void)
+int intel_idle_cpu_init(int cpu)
 {
-	int i, cstate;
+	int cstate;
 	struct cpuidle_device *dev;
 
-	intel_idle_cpuidle_devices = alloc_percpu(struct cpuidle_device);
-	if (intel_idle_cpuidle_devices == NULL)
-		return -ENOMEM;
+	dev = per_cpu_ptr(intel_idle_cpuidle_devices, cpu);
 
-	for_each_online_cpu(i) {
-		dev = per_cpu_ptr(intel_idle_cpuidle_devices, i);
+	dev->state_count = 1;
 
-		dev->state_count = 1;
-
-		for (cstate = 1; cstate < MWAIT_MAX_NUM_CSTATES; ++cstate) {
-			int num_substates;
-
-			if (cstate > max_cstate) {
-				printk(PREFIX "max_cstate %d reached\n",
-					max_cstate);
-				break;
-			}
-
-			/* does the state exist in CPUID.MWAIT? */
-			num_substates = (mwait_substates >> ((cstate) * 4))
-						& MWAIT_SUBSTATE_MASK;
-			if (num_substates == 0)
-				continue;
-			/* is the state not enabled? */
-			if (cpuidle_state_table[cstate].enter == NULL) {
-				/* does the driver not know about the state? */
-				if (*cpuidle_state_table[cstate].name == '\0')
-					pr_debug(PREFIX "unaware of model 0x%x"
-						" MWAIT %d please"
-						" contact lenb@kernel.org",
-					boot_cpu_data.x86_model, cstate);
-				continue;
-			}
-
-			if ((cstate > 2) &&
-				!boot_cpu_has(X86_FEATURE_NONSTOP_TSC))
-				mark_tsc_unstable("TSC halts in idle"
-					" states deeper than C2");
-
-			dev->states[dev->state_count] =	/* structure copy */
-				cpuidle_state_table[cstate];
-
-			dev->state_count += 1;
+	for (cstate = 1; cstate < MWAIT_MAX_NUM_CSTATES; ++cstate) {
+		int num_substates;
+
+		if (cstate > max_cstate) {
+			printk(PREFIX "max_cstate %d reached\n", max_cstate);
+			break;
 		}
 
-		dev->cpu = i;
-		if (cpuidle_register_device(dev)) {
-			pr_debug(PREFIX "cpuidle_register_device %d failed!\n",
-				 i);
-			intel_idle_cpuidle_devices_uninit();
-			return -EIO;
+		/* does the state exist in CPUID.MWAIT? */
+		num_substates = (mwait_substates >> ((cstate) * 4))
+			& MWAIT_SUBSTATE_MASK;
+		if (num_substates == 0)
+			continue;
+		/* is the state not enabled? */
+		if (cpuidle_state_table[cstate].enter == NULL) {
+			/* does the driver not know about the state? */
+			if (*cpuidle_state_table[cstate].name == '\0')
+				pr_debug(PREFIX "unaware of model 0x%x  MWAIT "
+					 "%d please contact lenb@kernel.org",
+					 boot_cpu_data.x86_model, cstate);
+			continue;
 		}
+
+		if ((cstate > 2) && !boot_cpu_has(X86_FEATURE_NONSTOP_TSC))
+			mark_tsc_unstable("TSC halts in idle"
+					  " states deeper than C2");
+
+		dev->states[dev->state_count] =	/* structure copy */
+			cpuidle_state_table[cstate];
+		dev->state_count += 1;
+	}
+	dev->cpu = cpu;
+	if (cpuidle_register_device(dev)) {
+		pr_debug(PREFIX "cpuidle_register_device %d failed!\n", cpu);
+		intel_idle_cpuidle_devices_uninit();
+		return -EIO;
 	}
 	if (auto_demotion_disable_flags)
-		smp_call_function(auto_demotion_disable, NULL, 1);
+		smp_call_function_single(cpu, auto_demotion_disable, NULL, 1);
 
 	return 0;
 }
+EXPORT_SYMBOL(intel_idle_cpu_init);
 
 
 static int __init intel_idle_init(void)
 {
-	int retval;
+	int retval, i;
 
 	/* Do not load intel_idle at all for now if idle= is passed */
 	if (boot_option_idle_override != IDLE_NO_OVERRIDE)
@@ -484,10 +474,16 @@ static int __init intel_idle_init(void)
 		return retval;
 	}
 
-	retval = intel_idle_cpuidle_devices_init();
-	if (retval) {
-		cpuidle_unregister_driver(&intel_idle_driver);
-		return retval;
+	intel_idle_cpuidle_devices = alloc_percpu(struct cpuidle_device);
+	if (intel_idle_cpuidle_devices == NULL)
+		return -ENOMEM;
+
+	for_each_online_cpu(i) {
+		retval = intel_idle_cpu_init(i);
+		if (retval) {
+			cpuidle_unregister_driver(&intel_idle_driver);
+			return retval;
+		}
 	}
 
 	return 0;
diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h
index b51629e..15bd1bd 100644
--- a/include/linux/cpuidle.h
+++ b/include/linux/cpuidle.h
@@ -179,7 +179,14 @@ struct cpuidle_governor {
 extern int cpuidle_register_governor(struct cpuidle_governor *gov);
 extern void cpuidle_unregister_governor(struct cpuidle_governor *gov);
 
+#ifdef CONFIG_INTEL_IDLE
+extern int intel_idle_cpu_init(int cpu);
 #else
+static inline int intel_idle_cpu_init(int cpu) { return -1; }
+#endif
+
+#else
+static inline int intel_idle_cpu_init(int cpu) { return -1; }
 
 static inline int cpuidle_register_governor(struct cpuidle_governor *gov)
 {return 0;}
-- 
1.7.6.1

[PATCH 4/5] ACPI processor: Fix error path, also remove sysdev link

[Thomas Renninger]

Signed-off-by: Thomas Renninger <trenn@suse.de>
CC: Len Brown <lenb@kernel.org>
CC: linux-acpi@vger.kernel.org
---
 drivers/acpi/processor_driver.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
index 790623f..3e5763f 100644
--- a/drivers/acpi/processor_driver.c
+++ b/drivers/acpi/processor_driver.c
@@ -545,6 +545,7 @@ err_thermal_unregister:
 	thermal_cooling_device_unregister(pr->cdev);
 err_power_exit:
 	acpi_processor_power_exit(pr, device);
+	sysfs_remove_link(&device->dev.kobj, "sysdev");
 err_free_cpumask:
 	free_cpumask_var(pr->throttling.shared_cpu_map);
 
-- 
1.7.6.1

[PATCH 5/5] ACPI processor: Remove unneeded variable passed by acpi_processor_hotadd_init

[Thomas Renninger]

This is a very small part taken from patches which afaik
are coming from Yunhong Jiang for Xen.
Xen CPU hotplug things not existing in Linus kernel yet were
removed.

Cleanup only: no functional change.

Signed-off-by: Thomas Renninger <trenn@suse.de>
CC: Len Brown <lenb@kernel.org>
CC: linux-acpi@vger.kernel.org
CC: Bjorn Helgaas <bhelgaas@google.com>
CC: Jiang, Yunhong <yunhong.jiang@intel.com>
---
 drivers/acpi/processor_driver.c |   17 ++++++++---------
 1 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
index 3e5763f..66887a5 100644
--- a/drivers/acpi/processor_driver.c
+++ b/drivers/acpi/processor_driver.c
@@ -82,7 +82,7 @@ MODULE_LICENSE("GPL");
 static int acpi_processor_add(struct acpi_device *device);
 static int acpi_processor_remove(struct acpi_device *device, int type);
 static void acpi_processor_notify(struct acpi_device *device, u32 event);
-static acpi_status acpi_processor_hotadd_init(acpi_handle handle, int *p_cpu);
+static acpi_status acpi_processor_hotadd_init(struct acpi_processor *pr);
 static int acpi_processor_handle_eject(struct acpi_processor *pr);
 
 
@@ -330,10 +330,8 @@ static int acpi_processor_get_info(struct acpi_device *device)
 	 *  they are physically not present.
 	 */
 	if (pr->id == -1) {
-		if (ACPI_FAILURE
-		    (acpi_processor_hotadd_init(pr->handle, &pr->id))) {
+		if (ACPI_FAILURE (acpi_processor_hotadd_init(pr->handle)))
 			return -ENODEV;
-		}
 	}
 	/*
 	 * On some boxes several processors use the same processor bus id.
@@ -727,18 +725,19 @@ processor_walk_namespace_cb(acpi_handle handle,
 	return (AE_OK);
 }
 
-static acpi_status acpi_processor_hotadd_init(acpi_handle handle, int *p_cpu)
+static acpi_status acpi_processor_hotadd_init(struct acpi_processor *pr)
 {
+	acpi_handle handle = pr->handle;
 
 	if (!is_processor_present(handle)) {
 		return AE_ERROR;
 	}
 
-	if (acpi_map_lsapic(handle, p_cpu))
+	if (acpi_map_lsapic(handle, &pr-id))
 		return AE_ERROR;
 
-	if (arch_register_cpu(*p_cpu)) {
-		acpi_unmap_lsapic(*p_cpu);
+	if (arch_register_cpu(pr->id)) {
+		acpi_unmap_lsapic(pr->id);
 		return AE_ERROR;
 	}
 
@@ -755,7 +754,7 @@ static int acpi_processor_handle_eject(struct acpi_processor *pr)
 	return (0);
 }
 #else
-static acpi_status acpi_processor_hotadd_init(acpi_handle handle, int *p_cpu)
+static acpi_status acpi_processor_hotadd_init(struct acpi_processor *pr)
 {
 	return AE_ERROR;
 }
-- 
1.7.6.1

Re: [PATCH 3/5] intel_idle: Split up and provide per CPU initialization func

[Thomas Renninger]

On Thursday 17 November 2011 23:36:59 Thomas Renninger wrote:

> +EXPORT_SYMBOL(intel_idle_cpu_init);
Just realized: It would be nicer if this is EXPORT_SYMBOL_GPL?
Also: This one is not used yet, but would be if processor_core.c
hotplug parts get enhanced.

Len: Would be great if you could modify this manually to EXPORT_SYMBOL_GPL
if you agree with the rest.

Thanks,

  Thomas

Re: [PATCH 2/5] ACPI processor: Avoid WARN message on processor driver removal

[Len Brown]

On 11/17/2011 05:36 PM, Thomas Renninger wrote:

> Only unregister acpi_idle driver if acpi_idle driver got
> registered.
> 
> Also add a static acpi_idle_active variable for easy and nicer
> checking whether acpi_idle_driver is active (as suggested by
> Bjorn).
> 
> Signed-off-by: Thomas Renninger <trenn@suse.de>
> CC: Len Brown <lenb@kernel.org>
> CC: linux-acpi@vger.kernel.org
> CC: Bjorn Helgaas <bhelgaas@google.com>
> ---
>  drivers/acpi/processor_driver.c |   11 +++++++----
>  1 files changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
> index 3a4fc0c..790623f 100644
> --- a/drivers/acpi/processor_driver.c
> +++ b/drivers/acpi/processor_driver.c
> @@ -110,6 +110,7 @@ static struct cpuidle_driver acpi_idle_driver = {
>  	.name =		"acpi_idle",
>  	.owner =	THIS_MODULE,
>  };
> +static int acpi_idle_active;
>  
>  #define INSTALL_NOTIFY_HANDLER		1
>  #define UNINSTALL_NOTIFY_HANDLER	2
> @@ -508,8 +509,7 @@ static int __cpuinit acpi_processor_add(struct acpi_device *device)
>  	acpi_processor_get_throttling_info(pr);
>  	acpi_processor_get_limit_info(pr);
>  
> -
> -	if (cpuidle_get_driver() == &acpi_idle_driver)
> +	if (acpi_idle_active)
>  		acpi_processor_power_init(pr, device);


I think the old cold reads better than the new code,
since I don't have to guess what "acpi_idle_active" means...

Perhaps you can use the existing mechanism where
it is missing to guard the unregister and re-fresh?

thanks,
-Len

Re: [PATCH 3/5] intel_idle: Split up and provide per CPU initialization func

[Len Brown]

On 11/17/2011 05:36 PM, Thomas Renninger wrote:

> Function split up, should have no functional change


benefit?

Re: [PATCH 5/5] ACPI processor: Remove unneeded variable passed by acpi_processor_hotadd_init

[Len Brown]

On 11/17/2011 05:37 PM, Thomas Renninger wrote:

> This is a very small part taken from patches which afaik
> are coming from Yunhong Jiang for Xen.
> Xen CPU hotplug things not existing in Linus kernel yet were
> removed.
> 
> Cleanup only: no functional change.
> 
> Signed-off-by: Thomas Renninger <trenn@suse.de>
> CC: Len Brown <lenb@kernel.org>
> CC: linux-acpi@vger.kernel.org
> CC: Bjorn Helgaas <bhelgaas@google.com>
> CC: Jiang, Yunhong <yunhong.jiang@intel.com>
> ---
>  drivers/acpi/processor_driver.c |   17 ++++++++---------
>  1 files changed, 8 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
> index 3e5763f..66887a5 100644
> --- a/drivers/acpi/processor_driver.c
> +++ b/drivers/acpi/processor_driver.c
> @@ -82,7 +82,7 @@ MODULE_LICENSE("GPL");
>  static int acpi_processor_add(struct acpi_device *device);
>  static int acpi_processor_remove(struct acpi_device *device, int type);
>  static void acpi_processor_notify(struct acpi_device *device, u32 event);
> -static acpi_status acpi_processor_hotadd_init(acpi_handle handle, int *p_cpu);
> +static acpi_status acpi_processor_hotadd_init(struct acpi_processor *pr);
>  static int acpi_processor_handle_eject(struct acpi_processor *pr);
>  
>  
> @@ -330,10 +330,8 @@ static int acpi_processor_get_info(struct acpi_device *device)
>  	 *  they are physically not present.
>  	 */
>  	if (pr->id == -1) {
> -		if (ACPI_FAILURE
> -		    (acpi_processor_hotadd_init(pr->handle, &pr->id))) {
> +		if (ACPI_FAILURE (acpi_processor_hotadd_init(pr->handle)))
>  			return -ENODEV;
> -		}
>  	}
>  	/*
>  	 * On some boxes several processors use the same processor bus id.
> @@ -727,18 +725,19 @@ processor_walk_namespace_cb(acpi_handle handle,
>  	return (AE_OK);
>  }
>  
> -static acpi_status acpi_processor_hotadd_init(acpi_handle handle, int *p_cpu)
> +static acpi_status acpi_processor_hotadd_init(struct acpi_processor *pr)
>  {
> +	acpi_handle handle = pr->handle;
>  
>  	if (!is_processor_present(handle)) {
>  		return AE_ERROR;
>  	}
>  
> -	if (acpi_map_lsapic(handle, p_cpu))
> +	if (acpi_map_lsapic(handle, &pr-id))


eh?

>  		return AE_ERROR;
>  
> -	if (arch_register_cpu(*p_cpu)) {
> -		acpi_unmap_lsapic(*p_cpu);
> +	if (arch_register_cpu(pr->id)) {
> +		acpi_unmap_lsapic(pr->id);
>  		return AE_ERROR;
>  	}
>  
> @@ -755,7 +754,7 @@ static int acpi_processor_handle_eject(struct acpi_processor *pr)
>  	return (0);
>  }
>  #else
> -static acpi_status acpi_processor_hotadd_init(acpi_handle handle, int *p_cpu)
> +static acpi_status acpi_processor_hotadd_init(struct acpi_processor *pr)
>  {
>  	return AE_ERROR;
>  }

Re: [PATCH 1/5] ACPI processor: Do not export acpi_idle_driver in processor.h

[Len Brown]

On 11/17/2011 05:36 PM, Thomas Renninger wrote:

> Instead move its declaration from processor_idle.c to processor_driver.c
> and declare it static as it's the only file using this struct.
> 
> Cleanup only, no functional change.
> 
> Signed-off-by: Thomas Renninger <trenn@suse.de>
> CC: Len Brown <lenb@kernel.org>
> CC: linux-acpi@vger.kernel.org
> ---
>  drivers/acpi/processor_driver.c |    5 +++++
>  drivers/acpi/processor_idle.c   |    5 -----
>  include/acpi/processor.h        |    1 -
>  3 files changed, 5 insertions(+), 6 deletions(-)
> 


drivers/acpi/processor_idle.c: In function ‘acpi_processor_setup_cpuidle_states’:
drivers/acpi/processor_idle.c:1032:32: error: ‘acpi_idle_driver’ undeclared (first use in this function)
drivers/acpi/processor_idle.c:1032:32: note: each undeclared identifier is reported only once for each function it appears in
drivers/acpi/processor_idle.c: In function ‘acpi_processor_cst_has_changed’:
drivers/acpi/processor_idle.c:1158:29: error: ‘acpi_idle_driver’ undeclared (first use in this function)
drivers/acpi/processor_idle.c: In function ‘acpi_processor_power_init’:
drivers/acpi/processor_idle.c:1239:38: error: ‘acpi_idle_driver’ undeclared (first use in this function)
drivers/acpi/processor_idle.c: In function ‘acpi_processor_power_exit’:
drivers/acpi/processor_idle.c:1270:31: error: ‘acpi_idle_driver’ undeclared (first use in this function)

eh?


> diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
> index a4e0f1b..3a4fc0c 100644
> --- a/drivers/acpi/processor_driver.c
> +++ b/drivers/acpi/processor_driver.c
> @@ -106,6 +106,11 @@ static struct acpi_driver acpi_processor_driver = {
>  		},
>  };
>  
> +static struct cpuidle_driver acpi_idle_driver = {
> +	.name =		"acpi_idle",
> +	.owner =	THIS_MODULE,
> +};
> +
>  #define INSTALL_NOTIFY_HANDLER		1
>  #define UNINSTALL_NOTIFY_HANDLER	2
>  
> diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
> index 9b88f98..7fed4f3 100644
> --- a/drivers/acpi/processor_idle.c
> +++ b/drivers/acpi/processor_idle.c
> @@ -968,11 +968,6 @@ static int acpi_idle_enter_bm(struct cpuidle_device *dev,
>  	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
> diff --git a/include/acpi/processor.h b/include/acpi/processor.h
> index 67055f1..5b27bc3 100644
> --- a/include/acpi/processor.h
> +++ b/include/acpi/processor.h
> @@ -333,7 +333,6 @@ int acpi_processor_power_exit(struct acpi_processor *pr,
>  			      struct acpi_device *device);
>  int acpi_processor_suspend(struct acpi_device * device, pm_message_t state);
>  int acpi_processor_resume(struct acpi_device * device);
> -extern struct cpuidle_driver acpi_idle_driver;
>  
>  /* in processor_thermal.c */
>  int acpi_processor_get_limit_info(struct acpi_processor *pr);


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

Re: [PATCH 3/5] intel_idle: Split up and provide per CPU initialization func

[Thomas Renninger]

On Tuesday, January 17, 2012 12:06:19 PM Len Brown wrote:
> On 11/17/2011 05:36 PM, Thomas Renninger wrote:
> 
> > Function split up, should have no functional change
> 
> benefit?
The idea was:
Provide a basic init func which can be called in ACPI CPU hotplug
context safely before the CPU is brought up the first time and:
struct cpuinfo_x86
(like CPU features) of the new CPU is not set up yet.
Do the idle/throttling/... init later when the physically hotplugged
CPU is onlined the first time (and therefore got fully booted
and initialized).

While I had this working, I now have a better idea which does
not need this split:
When the ACPI CPU hotplug event is caught, fill up cpu_data(new_cpu)
data as much and good as possilbe, do something like:
memcpy(&cpu_data(new_cpu), &boot_cpu_data, sizeof(struct cpuinfo_x86);
and adjust:
phys_proc_id
cpu_core_id
apicid
initial_apicid
...
Then cpuidle (and throttling,...) can be initialized without the need
of booting up the CPU first.

Not sure whether the memcpy would succeed if the CPU is not (and never
was) online. From what I saw it should work, but I am not sure.
Tejun: Do you know that?

Another option would be what Bjorn Helgaas suggested:
Immediately online the CPU, but this would heavily interfere with
the current (generic code) approach, but after some thinking and
looking at the code: this shouldn't bring much benefit.

   Thomas

Re: [PATCH 1/5] ACPI processor: Do not export acpi_idle_driver in processor.h

[Thomas Renninger]

On Tuesday, January 17, 2012 12:25:08 PM Len Brown wrote:
> On 11/17/2011 05:36 PM, Thomas Renninger wrote:
> 
> > Instead move its declaration from processor_idle.c to processor_driver.c
> > and declare it static as it's the only file using this struct.
> > 
> > Cleanup only, no functional change.
> > 
> > Signed-off-by: Thomas Renninger <trenn@suse.de>
> > CC: Len Brown <lenb@kernel.org>
> > CC: linux-acpi@vger.kernel.org
> > ---
> >  drivers/acpi/processor_driver.c |    5 +++++
> >  drivers/acpi/processor_idle.c   |    5 -----
> >  include/acpi/processor.h        |    1 -
> >  3 files changed, 5 insertions(+), 6 deletions(-)
> > 
> 
> 
> drivers/acpi/processor_idle.c: In function ‘acpi_processor_setup_cpuidle_states’:
> drivers/acpi/processor_idle.c:1032:32: error: ‘acpi_idle_driver’ undeclared (first use in this function)
> drivers/acpi/processor_idle.c:1032:32: note: each undeclared identifier is reported only once for each function it appears in
> drivers/acpi/processor_idle.c: In function ‘acpi_processor_cst_has_changed’:
> drivers/acpi/processor_idle.c:1158:29: error: ‘acpi_idle_driver’ undeclared (first use in this function)
> drivers/acpi/processor_idle.c: In function ‘acpi_processor_power_init’:
> drivers/acpi/processor_idle.c:1239:38: error: ‘acpi_idle_driver’ undeclared (first use in this function)
> drivers/acpi/processor_idle.c: In function ‘acpi_processor_power_exit’:
> drivers/acpi/processor_idle.c:1270:31: error: ‘acpi_idle_driver’ undeclared (first use in this function)
> 
> eh?
Oh, this one does not work anymore since:
commit 46bcfad7a819bd17ac4e831b04405152d59784ab
Author: Deepthi Dharwar <deepthi@linux.vnet.ibm.com>
Date:   Fri Oct 28 16:20:42 2011 +0530

    cpuidle: Single/Global registration of idle states

got merged in recently.
Please ignore this patch.

   Thomas
--
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

Re: [PATCH 2/5] ACPI processor: Avoid WARN message on processor driver removal

[Thomas Renninger]

On Tuesday, January 17, 2012 12:02:39 PM Len Brown wrote:
> On 11/17/2011 05:36 PM, Thomas Renninger wrote:
...

> I think the old cold reads better than the new code,
> since I don't have to guess what "acpi_idle_active" means...
Sigh, Bjorn suggested it exactly the other way around.
I don't mind which way, back to my original post half a year
ago...
 
  Thomas

Re: [PATCH 2/5] ACPI processor: Avoid WARN message on processor driver removal

[Bjorn Helgaas]

On Tue, Jan 17, 2012 at 7:25 AM, Thomas Renninger <trenn@suse.de> wrote:
> On Tuesday, January 17, 2012 12:02:39 PM Len Brown wrote:
>> On 11/17/2011 05:36 PM, Thomas Renninger wrote:
> ...
>
>> I think the old cold reads better than the new code,
>> since I don't have to guess what "acpi_idle_active" means...
> Sigh, Bjorn suggested it exactly the other way around.
> I don't mind which way, back to my original post half a year
> ago...

I don't care very much either way.  I think I just thought it was
nicer for the driver to locally remember "I registered, therefore I
should unregister/init new CPUs."  Apparently there can be several
idle drivers, and I don't know what happens if you register more than
one or what cpuidle_get_driver() means in that case.

Bjorn

Re: [PATCH 2/5] ACPI processor: Avoid WARN message on processor driver removal

[Thomas Renninger]

On Tuesday, January 17, 2012 12:02:39 PM Len Brown wrote:
> On 11/17/2011 05:36 PM, Thomas Renninger wrote:
> 
> > Only unregister acpi_idle driver if acpi_idle driver got
> > registered.
> > 
> > Also add a static acpi_idle_active variable for easy and nicer
> > checking whether acpi_idle_driver is active (as suggested by
> > Bjorn).
> > 
> > Signed-off-by: Thomas Renninger <trenn@suse.de>
> > CC: Len Brown <lenb@kernel.org>
> > CC: linux-acpi@vger.kernel.org
> > CC: Bjorn Helgaas <bhelgaas@google.com>
> > ---
> >  drivers/acpi/processor_driver.c |   11 +++++++----
> >  1 files changed, 7 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
> > index 3a4fc0c..790623f 100644
> > --- a/drivers/acpi/processor_driver.c
> > +++ b/drivers/acpi/processor_driver.c
> > @@ -110,6 +110,7 @@ static struct cpuidle_driver acpi_idle_driver = {
> >  	.name =		"acpi_idle",
> >  	.owner =	THIS_MODULE,
> >  };
> > +static int acpi_idle_active;
> >  
> >  #define INSTALL_NOTIFY_HANDLER		1
> >  #define UNINSTALL_NOTIFY_HANDLER	2
> > @@ -508,8 +509,7 @@ static int __cpuinit acpi_processor_add(struct acpi_device *device)
> >  	acpi_processor_get_throttling_info(pr);
> >  	acpi_processor_get_limit_info(pr);
> >  
> > -
> > -	if (cpuidle_get_driver() == &acpi_idle_driver)
> > +	if (acpi_idle_active)
> >  		acpi_processor_power_init(pr, device);
> 
> 
> I think the old cold reads better than the new code,
> since I don't have to guess what "acpi_idle_active" means...
> 
> Perhaps you can use the existing mechanism where
> it is missing to guard the unregister and re-fresh?
With commit 46bcfad7a819bd17ac4e831b04405152d59784ab
Author: Deepthi Dharwar <deepthi@linux.vnet.ibm.com>
Date:   Fri Oct 28 16:20:42 2011 +0530

    cpuidle: Single/Global registration of idle states

the offending unregistering line is not needed anymore at all
and a new patch would look similar to:
@@ -846,7 +848,8 @@ static void __exit acpi_processor_exit(void)
 
        acpi_bus_unregister_driver(&acpi_processor_driver);
 
-       cpuidle_unregister_driver(&acpi_idle_driver);
 
        return;
 }

I'll send something with Deepthi in CC to double check.

   Thomas

Re: [PATCH 3/5] intel_idle: Split up and provide per CPU initialization func

[Thomas Renninger]

On Tuesday, January 17, 2012 02:26:40 PM Thomas Renninger wrote:
> On Tuesday, January 17, 2012 12:06:19 PM Len Brown wrote:
> > On 11/17/2011 05:36 PM, Thomas Renninger wrote:
> > 
> > > Function split up, should have no functional change
> > 
> > benefit?
> The idea was:
> Provide a basic init func which can be called in ACPI CPU hotplug
> context safely before the CPU is brought up the first time and:
> struct cpuinfo_x86
> (like CPU features) of the new CPU is not set up yet.
> Do the idle/throttling/... init later when the physically hotplugged
> CPU is onlined the first time (and therefore got fully booted
> and initialized).
> 
> While I had this working, I now have a better idea which does
> not need this split:
Wait, this cannot work.
A cpu core must/should be online to get cpuidle initialized correctly.

> When the ACPI CPU hotplug event is caught, fill up cpu_data(new_cpu)
> data as much and good as possilbe, do something like:
> memcpy(&cpu_data(new_cpu), &boot_cpu_data, sizeof(struct cpuinfo_x86);
> and adjust:
> phys_proc_id
> cpu_core_id
> apicid
> initial_apicid
> ...
> Then cpuidle (and throttling,...) can be initialized without the need
> of booting up the CPU first.
> 
> Not sure whether the memcpy would succeed if the CPU is not (and never
> was) online. From what I saw it should work, but I am not sure.
> Tejun: Do you know that?
Never mind. If I think it really could make sense, I better post an
example code snippet what I'd like to achieve.

   Thomas