On 05/14/2013 03:21 PM, Daniel Lezcano wrote:
Commit bf4d1b5ddb78f86078ac6ae0415802d5f0c68f92 brought the multiple driver support. The code added a couple of new API to register the driver per cpu. That led to some code complexity to handle the kernel config options when the multiple driver support is enabled or not, which is not really necessary. The code has to be compatible when the multiple driver support is not enabled, and the multiple driver support has to be compatible with the old api.
This patch removes this API, which is not yet used by any driver but needed for the HMP cpuidle drivers which will come soon, and replaces its usage by a cpumask pointer in the cpuidle driver structure telling what cpus are handled by the driver. That let the API cpuidle_[un]register_driver to be used for the multipled driver support and also the cpuidle_[un]register functions, added recently in the cpuidle framework.
The current code, a bit poor in comments, has been commented and simplified.
Signed-off-by: Daniel Lezcano daniel.lezcano@linaro.org
[...]
+/**
- __cpuidle_register_driver: do some sanity checks, initializes the driver,
- assign the driver to the global cpuidle driver variable(s) and setup the
- broadcast timer if the cpuidle driver has some states which shutdown the
- local timer.
- @drv: a valid pointer to a struct cpuidle_driver
- Returns 0 on success, < 0 otherwise
- */
+static int __cpuidle_register_driver(struct cpuidle_driver *drv) { int ret;
- spin_lock(&cpuidle_driver_lock);
- ret = __cpuidle_register_driver(drv, cpu);
- spin_unlock(&cpuidle_driver_lock);
- if (!drv || !drv->state_count)
return -EINVAL;
- return ret;
-}
- if (cpuidle_disabled())
return -ENODEV;-void cpuidle_unregister_cpu_driver(struct cpuidle_driver *drv, int cpu) -{
- spin_lock(&cpuidle_driver_lock);
- __cpuidle_unregister_driver(drv, cpu);
- spin_unlock(&cpuidle_driver_lock);
-}
- ret = __cpuidle_driver_init(drv);
- if (ret)
return ret;-/**
- cpuidle_register_driver - registers a driver
- @drv: the driver
- */
-int cpuidle_register_driver(struct cpuidle_driver *drv) -{
- int ret;
- ret = __cpuidle_set_driver(drv);
- if (ret)
return ret;
- spin_lock(&cpuidle_driver_lock);
- ret = __cpuidle_register_all_cpu_driver(drv);
- spin_unlock(&cpuidle_driver_lock);
- if (drv->bctimer)
on_each_cpu_mask(drv->cpumask, cpuidle_setup_broadcast_timer,(void *)CLOCK_EVT_NOTIFY_BROADCAST_ON, 1);
- return ret;
- return 0;
} -EXPORT_SYMBOL_GPL(cpuidle_register_driver); /**
- cpuidle_unregister_driver - unregisters a driver
- @drv: the driver
- __cpuidle_unregister_driver: checks the driver is no longer in use, reset the
- global cpuidle driver variable(s) and disable the timer broadcast
- notification mechanism if it was in use.
- @drv: a valid pointer to a struct cpuidle_driver
*/
- Returns 0 on success, < 0 otherwise
-void cpuidle_unregister_driver(struct cpuidle_driver *drv) +static void __cpuidle_unregister_driver(struct cpuidle_driver *drv) {
- spin_lock(&cpuidle_driver_lock);
- __cpuidle_unregister_all_cpu_driver(drv);
- spin_unlock(&cpuidle_driver_lock);
-} -EXPORT_SYMBOL_GPL(cpuidle_unregister_driver);
-#else
-static struct cpuidle_driver *cpuidle_curr_driver;
- if (!WARN_ON(drv->refcnt > 0))
return;
Shouldn't it be if (WARN_ON(drv->refcnt > 0)) return; ?
-static inline void __cpuidle_set_cpu_driver(struct cpuidle_driver *drv, int cpu) -{
- cpuidle_curr_driver = drv;
-}
- __cpuidle_unset_driver(drv);
-static inline struct cpuidle_driver *__cpuidle_get_cpu_driver(int cpu) -{
- return cpuidle_curr_driver;
- if (drv->bctimer) {
drv->bctimer = 0;on_each_cpu_mask(drv->cpumask, cpuidle_setup_broadcast_timer,(void *)CLOCK_EVT_NOTIFY_BROADCAST_OFF, 1);- }
}
Also, shouldn't things be undone in reverse order as they were done in __cpuidle_register_driver()? I mean disabling the timer broadcast notification before calling __cpuidle_unset_driver().
Regards, Francesco