* Arun R B <a...@linux.vnet.ibm.com> [2009-09-01 17:08:40]:
> * Arun R Bharadwaj <a...@linux.vnet.ibm.com> [2009-09-01 17:07:04]:
>
> Cleanup drivers/cpuidle/cpuidle.c
>
> Cpuidle maintains a pm_idle_old void pointer because, currently in x86
> there is no clean way of registering and unregistering a idle function.
>
> So remove pm_idle_old and leave the responsibility of maintaining the
> list of registered idle loops to the architecture specific code. If the
> architecture registers cpuidle_idle_call as its idle loop, only then
> this loop is called.
>
It sounds as if there is a side-effect of this
patch on x86 (am I reading it incorrectly), which can be fixed, but
it will need a patch or so to get back the old behaviour on x86.
> Also remove unwanted functions cpuidle_[un]install_idle_handler,
> cpuidle_kick_cpus()
>
> Signed-off-by: Arun R Bharadwaj <a...@linux.vnet.ibm.com>
> ---
> drivers/cpuidle/cpuidle.c | 51
> +++++++++++++++------------------------------
> drivers/cpuidle/governor.c | 3 --
> 2 files changed, 17 insertions(+), 37 deletions(-)
>
> Index: linux.trees.git/drivers/cpuidle/cpuidle.c
> ===================================================================
> --- linux.trees.git.orig/drivers/cpuidle/cpuidle.c
> +++ linux.trees.git/drivers/cpuidle/cpuidle.c
> @@ -24,9 +24,14 @@ DEFINE_PER_CPU(struct cpuidle_device *,
>
> DEFINE_MUTEX(cpuidle_lock);
> LIST_HEAD(cpuidle_detected_devices);
> -static void (*pm_idle_old)(void);
>
> static int enabled_devices;
> +static int idle_function_registered;
> +
> +struct idle_function_desc cpuidle_idle_desc = {
> + .name = "cpuidle_loop",
> + .idle_func = cpuidle_idle_call,
> +};
>
> #if defined(CONFIG_ARCH_HAS_CPU_IDLE_WAIT)
> static void cpuidle_kick_cpus(void)
> @@ -54,13 +59,10 @@ static void cpuidle_idle_call(void)
>
> /* check if the device is ready */
> if (!dev || !dev->enabled) {
> - if (pm_idle_old)
> - pm_idle_old();
> - else
> #if defined(CONFIG_ARCH_HAS_DEFAULT_IDLE)
> - default_idle();
> + default_idle();
> #else
> - local_irq_enable();
> + local_irq_enable();
> #endif
> return;
> }
> @@ -94,35 +96,11 @@ static void cpuidle_idle_call(void)
> }
>
> /**
> - * cpuidle_install_idle_handler - installs the cpuidle idle loop handler
> - */
> -void cpuidle_install_idle_handler(void)
> -{
> - if (enabled_devices && (pm_idle != cpuidle_idle_call)) {
> - /* Make sure all changes finished before we switch to new idle
> */
> - smp_wmb();
> - pm_idle = cpuidle_idle_call;
> - }
> -}
> -
> -/**
> - * cpuidle_uninstall_idle_handler - uninstalls the cpuidle idle loop handler
> - */
> -void cpuidle_uninstall_idle_handler(void)
> -{
> - if (enabled_devices && pm_idle_old && (pm_idle != pm_idle_old)) {
> - pm_idle = pm_idle_old;
> - cpuidle_kick_cpus();
> - }
> -}
> -
> -/**
> * cpuidle_pause_and_lock - temporarily disables CPUIDLE
> */
> void cpuidle_pause_and_lock(void)
> {
> mutex_lock(&cpuidle_lock);
> - cpuidle_uninstall_idle_handler();
> }
>
> EXPORT_SYMBOL_GPL(cpuidle_pause_and_lock);
> @@ -132,7 +110,6 @@ EXPORT_SYMBOL_GPL(cpuidle_pause_and_lock
> */
> void cpuidle_resume_and_unlock(void)
> {
> - cpuidle_install_idle_handler();
> mutex_unlock(&cpuidle_lock);
> }
>
What does this mean for users of cpuidle_pause_and_lock/unlock?
Should we be calling register/unregister_idle_function here?
> @@ -287,6 +264,12 @@ static int __cpuidle_register_device(str
> return 0;
> }
>
> +static void register_cpuidle_idle_function(void)
> +{
> + register_idle_function(&cpuidle_idle_desc);
> +
> + idle_function_registered = 1;
Use booleans if possible, unless you intend to extend the meaning of
registered someday.
> +}
> /**
> * cpuidle_register_device - registers a CPU's idle PM feature
> * @dev: the cpu
> @@ -303,7 +286,9 @@ int cpuidle_register_device(struct cpuid
> }
>
> cpuidle_enable_device(dev);
> - cpuidle_install_idle_handler();
> +
> + if (!idle_function_registered)
> + register_cpuidle_idle_function();
>
> mutex_unlock(&cpuidle_lock);
>
> @@ -382,8 +367,6 @@ static int __init cpuidle_init(void)
> {
> int ret;
>
> - pm_idle_old = pm_idle;
> -
> ret = cpuidle_add_class_sysfs(&cpu_sysdev_class);
> if (ret)
> return ret;
> Index: linux.trees.git/drivers/cpuidle/governor.c
> ===================================================================
> --- linux.trees.git.orig/drivers/cpuidle/governor.c
> +++ linux.trees.git/drivers/cpuidle/governor.c
> @@ -48,8 +48,6 @@ int cpuidle_switch_governor(struct cpuid
> if (gov == cpuidle_curr_governor)
> return 0;
>
> - cpuidle_uninstall_idle_handler();
> -
> if (cpuidle_curr_governor) {
> list_for_each_entry(dev, &cpuidle_detected_devices, device_list)
> cpuidle_disable_device(dev);
> @@ -63,7 +61,6 @@ int cpuidle_switch_governor(struct cpuid
> return -EINVAL;
> list_for_each_entry(dev, &cpuidle_detected_devices, device_list)
> cpuidle_enable_device(dev);
> - cpuidle_install_idle_handler();
> printk(KERN_INFO "cpuidle: using governor %s\n", gov->name);
> }
>
--
Balbir
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
