Shawn,
On Tue, May 1, 2012 at 10:13 PM, Shawn Guo shawn.guo@linaro.org wrote:
On Tue, May 01, 2012 at 09:12:38PM -0500, Robert Lee wrote:
Add common cpuidle init functionality that can be used by various imx platforms.
Signed-off-by: Robert Lee rob.lee@linaro.org
arch/arm/plat-mxc/Makefile | 1 + arch/arm/plat-mxc/cpuidle.c | 80 ++++++++++++++++++++++++++++++ arch/arm/plat-mxc/include/mach/cpuidle.h | 22 ++++++++ 3 files changed, 103 insertions(+) create mode 100644 arch/arm/plat-mxc/cpuidle.c create mode 100644 arch/arm/plat-mxc/include/mach/cpuidle.h
diff --git a/arch/arm/plat-mxc/Makefile b/arch/arm/plat-mxc/Makefile index e81290c..63b064b 100644 --- a/arch/arm/plat-mxc/Makefile +++ b/arch/arm/plat-mxc/Makefile @@ -16,6 +16,7 @@ obj-$(CONFIG_MXC_ULPI) += ulpi.o obj-$(CONFIG_MXC_USE_EPIT) += epit.o obj-$(CONFIG_MXC_DEBUG_BOARD) += 3ds_debugboard.o obj-$(CONFIG_CPU_FREQ_IMX) += cpufreq.o +obj-$(CONFIG_CPU_IDLE) += cpuidle.o ifdef CONFIG_SND_IMX_SOC obj-y += ssi-fiq.o obj-y += ssi-fiq-ksym.o diff --git a/arch/arm/plat-mxc/cpuidle.c b/arch/arm/plat-mxc/cpuidle.c new file mode 100644 index 0000000..b7a5e1c --- /dev/null +++ b/arch/arm/plat-mxc/cpuidle.c @@ -0,0 +1,80 @@ +/*
- Copyright 2012 Freescale Semiconductor, Inc.
 
- Copyright 2012 Linaro Ltd.
 
- The code contained herein is licensed under the GNU General Public
 
- License. You may obtain a copy of the GNU General Public License
 
- Version 2 or later at the following locations:
 
- */
 +#include <linux/kernel.h> +#include <linux/io.h> +#include <linux/cpuidle.h> +#include <linux/hrtimer.h> +#include <linux/err.h> +#include <linux/slab.h>
+static struct cpuidle_device __percpu * imx_cpuidle_devices;
+void imx_cpuidle_devices_uninit(void) +{
- int cpu_id;
 - struct cpuidle_device *dev;
 - for_each_possible_cpu(cpu_id) {
 - dev = per_cpu_ptr(imx_cpuidle_devices, cpu_id);
 - cpuidle_unregister_device(dev);
 - }
 - free_percpu(imx_cpuidle_devices);
 +}
Does this function need to be exported? I haven't seen it being used anywhere outside this file. Also, can it be __init?
Yes to both for now and can be changed back if necessary in the future.
+int __init imx_cpuidle_init(struct cpuidle_driver *drv) +{
- struct cpuidle_device *dev;
 - int cpu_id, ret;
 - if (!drv || drv->state_count > CPUIDLE_STATE_MAX) {
 - pr_err("%s: Invalid Input\n", __func__);
 - return -EINVAL;
 - }
 - ret = cpuidle_register_driver(drv);
 - if (ret) {
 - pr_err("%s: Failed to register cpuidle driver with error: %d\n",
 - __func__, ret);
 - return ret;
 - }
 - imx_cpuidle_devices = alloc_percpu(struct cpuidle_device);
 - if (imx_cpuidle_devices == NULL) {
 - ret = -ENOMEM;
 - goto unregister_drv;
 - }
 - /* initialize state data for each cpuidle_device */
 - for_each_possible_cpu(cpu_id) {
 - dev = per_cpu_ptr(imx_cpuidle_devices, cpu_id);
 - dev->cpu = cpu_id;
 - dev->state_count = drv->state_count;
 - ret = cpuidle_register_device(dev);
 - if (ret) {
 - pr_err("%s: Failed to register cpu %u\n",
 - __func__, cpu_id);
 Nit: print ret (error code) too?
I added the printing of the error code based on Sascha's suggestion in v1 of this submission.
- goto uninit;
 - }
 - }
 - return 0;
 +uninit:
- imx_cpuidle_devices_uninit();
 +unregister_drv:
- cpuidle_unregister_driver(drv);
 - return ret;
 +} diff --git a/arch/arm/plat-mxc/include/mach/cpuidle.h b/arch/arm/plat-mxc/include/mach/cpuidle.h new file mode 100644 index 0000000..8612510 --- /dev/null +++ b/arch/arm/plat-mxc/include/mach/cpuidle.h @@ -0,0 +1,22 @@ +/*
- Copyright 2012 Freescale Semiconductor, Inc.
 
- Copyright 2012 Linaro Ltd.
 
- The code contained herein is licensed under the GNU General Public
 
- License. You may obtain a copy of the GNU General Public License
 
- Version 2 or later at the following locations:
 
- */
 +#include <linux/cpuidle.h>
+#ifdef CONFIG_CPU_IDLE +extern void imx_cpuidle_devices_uninit(void); +extern int imx_cpuidle_init(struct cpuidle_driver *drv); +#else +static inline void imx_cpuidle_devices_uninit(void) {} +static inline int imx_cpuidle_init(struct cpuidle_driver *drv) +{ return -ENODEV; }
Nit: if it can not be in the same line with function name, we usually have it be:
{ return -ENODEV; }
Understood. I was just going by what I have seen used in other places (like include/linux/cpuidle.h).
Thanks, Rob
+#endif
1.7.10
-- Regards, Shawn