Add common imx cpuidle initialization functionality and add a i.MX5 and i.MX6 platform cpuidle implementation.
Robert Lee (3): ARM: imx: Add common imx cpuidle init functionality. ARM: imx: Add imx5 cpuidle driver ARM: imx: Add imx6q cpuidle driver
arch/arm/mach-imx/Makefile | 4 +- arch/arm/mach-imx/cpuidle-imx5.c | 55 ++++++++++++++++++ arch/arm/mach-imx/cpuidle-imx6q.c | 33 +++++++++++ arch/arm/mach-imx/mach-imx6q.c | 2 + arch/arm/mach-imx/mm-imx5.c | 16 ++++-- arch/arm/plat-mxc/Makefile | 2 + arch/arm/plat-mxc/cpuidle.c | 89 ++++++++++++++++++++++++++++++ arch/arm/plat-mxc/include/mach/common.h | 1 + arch/arm/plat-mxc/include/mach/cpuidle.h | 26 +++++++++ 9 files changed, 222 insertions(+), 6 deletions(-) create mode 100644 arch/arm/mach-imx/cpuidle-imx5.c create mode 100644 arch/arm/mach-imx/cpuidle-imx6q.c create mode 100644 arch/arm/plat-mxc/cpuidle.c create mode 100644 arch/arm/plat-mxc/include/mach/cpuidle.h
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 | 2 + arch/arm/plat-mxc/cpuidle.c | 89 ++++++++++++++++++++++++++++++ arch/arm/plat-mxc/include/mach/cpuidle.h | 26 +++++++++ 3 files changed, 117 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..7c9e05f 100644 --- a/arch/arm/plat-mxc/Makefile +++ b/arch/arm/plat-mxc/Makefile @@ -16,6 +16,8 @@ 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..d1c9301 --- /dev/null +++ b/arch/arm/plat-mxc/cpuidle.c @@ -0,0 +1,89 @@ +/* + * 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: + * + * http://www.opensource.org/licenses/gpl-license.html + * http://www.gnu.org/copyleft/gpl.html + */ + +#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; +static struct cpuidle_driver *drv; + +void __init imx_cpuidle_set_driver(struct cpuidle_driver *p) +{ + drv = p; +} + +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); +} + +static int __init imx_cpuidle_init(void) +{ + 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\n", __func__); + 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); + goto uninit; + } + } + + return 0; + +uninit: + imx_cpuidle_devices_uninit(); + +unregister_drv: + cpuidle_unregister_driver(drv); + + return ret; +} +late_initcall(imx_cpuidle_init); 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..6509f19 --- /dev/null +++ b/arch/arm/plat-mxc/include/mach/cpuidle.h @@ -0,0 +1,26 @@ +/* + * 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: + * + * http://www.opensource.org/licenses/gpl-license.html + * http://www.gnu.org/copyleft/gpl.html + */ + +#include <linux/cpuidle.h> + +extern int imx5_cpuidle_init(void); +extern int imx6q_cpuidle_init(void); + +#ifdef CONFIG_CPU_IDLE +extern void imx_cpuidle_devices_uninit(void); +extern void imx_cpuidle_set_driver(struct cpuidle_driver *p); +extern int imx5_cpuidle_init(void); +extern int imx6q_cpuidle_init(void); +#else +static inline void imx_cpuidle_devices_uninit(void) {} +static inline void imx_cpuidle_set_driver(struct cpuidle_driver *p) {} +#endif
On Mon, 16 Apr 2012, 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 | 2 + arch/arm/plat-mxc/cpuidle.c | 89 ++++++++++++++++++++++++++++++ arch/arm/plat-mxc/include/mach/cpuidle.h | 26 +++++++++ 3 files changed, 117 insertions(+) create mode 100644 arch/arm/plat-mxc/cpuidle.c create mode 100644 arch/arm/plat-mxc/include/mach/cpuidle.h
A few pedantic/ignorable comments below.
diff --git a/arch/arm/plat-mxc/Makefile b/arch/arm/plat-mxc/Makefile index e81290c..7c9e05f 100644 --- a/arch/arm/plat-mxc/Makefile +++ b/arch/arm/plat-mxc/Makefile @@ -16,6 +16,8 @@ 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..d1c9301 --- /dev/null +++ b/arch/arm/plat-mxc/cpuidle.c @@ -0,0 +1,89 @@ +/*
- 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; +static struct cpuidle_driver *drv;
+void __init imx_cpuidle_set_driver(struct cpuidle_driver *p) +{
- drv = p;
+}
+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);
+}
+static int __init imx_cpuidle_init(void) +{
- 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);
Pointless blank line. I'd suggest removing it.
- if (ret) {
pr_err("%s: Failed to register cpuidle driver\n", __func__);return ret;- }
- imx_cpuidle_devices = alloc_percpu(struct cpuidle_device);
Same here. Pointless blank line.
- 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);goto uninit;}- }
- return 0;
+uninit:
- imx_cpuidle_devices_uninit();
+unregister_drv:
- cpuidle_unregister_driver(drv);
Blank line here seems pointless.
- return ret;
+} +late_initcall(imx_cpuidle_init); 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..6509f19 --- /dev/null +++ b/arch/arm/plat-mxc/include/mach/cpuidle.h @@ -0,0 +1,26 @@ +/*
- 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>
+extern int imx5_cpuidle_init(void); +extern int imx6q_cpuidle_init(void);
+#ifdef CONFIG_CPU_IDLE +extern void imx_cpuidle_devices_uninit(void); +extern void imx_cpuidle_set_driver(struct cpuidle_driver *p); +extern int imx5_cpuidle_init(void); +extern int imx6q_cpuidle_init(void); +#else +static inline void imx_cpuidle_devices_uninit(void) {} +static inline void imx_cpuidle_set_driver(struct cpuidle_driver *p) {} +#endif
On Mon, Apr 16, 2012 at 06:50:12PM -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
diff --git a/arch/arm/plat-mxc/Makefile b/arch/arm/plat-mxc/Makefile index e81290c..7c9e05f 100644 --- a/arch/arm/plat-mxc/Makefile +++ b/arch/arm/plat-mxc/Makefile @@ -16,6 +16,8 @@ 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..d1c9301 --- /dev/null +++ b/arch/arm/plat-mxc/cpuidle.c @@ -0,0 +1,89 @@ +/*
- 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; +static struct cpuidle_driver *drv;
+void __init imx_cpuidle_set_driver(struct cpuidle_driver *p) +{
- drv = p;
+}
You like it complicated, eh? Why do you introduce a function which sets a variable...
+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);
+}
+static int __init imx_cpuidle_init(void)
... instead of passing it directly to the function which uses it?
+{
- struct cpuidle_device *dev;
- int cpu_id, ret;
- if (!drv || drv->state_count > CPUIDLE_STATE_MAX) {
pr_err("%s: Invalid Input\n", __func__);
You introduce a pointless error message on SoCs not setting a cpuidle driver. When will people learn that initcalls do not only run on machines they have on their desk?
return -EINVAL;- }
- ret = cpuidle_register_driver(drv);
- if (ret) {
pr_err("%s: Failed to register cpuidle driver\n", __func__);
It's always nice to add the return value to the error message to get a clue *what* went wrong. The function name though is rather uninteresting.
Sascha
On Tue, Apr 17, 2012 at 2:43 AM, Sascha Hauer s.hauer@pengutronix.de wrote:
On Mon, Apr 16, 2012 at 06:50:12PM -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
diff --git a/arch/arm/plat-mxc/Makefile b/arch/arm/plat-mxc/Makefile index e81290c..7c9e05f 100644 --- a/arch/arm/plat-mxc/Makefile +++ b/arch/arm/plat-mxc/Makefile @@ -16,6 +16,8 @@ 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..d1c9301 --- /dev/null +++ b/arch/arm/plat-mxc/cpuidle.c @@ -0,0 +1,89 @@ +/*
- 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; +static struct cpuidle_driver *drv;
+void __init imx_cpuidle_set_driver(struct cpuidle_driver *p) +{
- drv = p;
+}
You like it complicated, eh? Why do you introduce a function which sets a variable...
This complication is used to deal with the timing of various levels of init calls. More explanation below.
+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);
+}
+static int __init imx_cpuidle_init(void)
... instead of passing it directly to the function which uses it?
If I called imx_cpuidle_init directly from imx5 or imx6q init routines, it would be getting called before the coreinit_call of core cpuidle causing a failure. There were various other directions to take and all seemed less desirable than this one.
One alternative would be to add a function to return the pointer to the cpuidle driver object based on the machine type. Functionality exists to identify imx5 as a machine type but not imx6q, so I couldn't use that machine based method without adding that extra code.
Another alternative would be to add a general platform lateinit_call function to each platforms that support cpuidle.
+{
- struct cpuidle_device *dev;
- int cpu_id, ret;
- if (!drv || drv->state_count > CPUIDLE_STATE_MAX) {
- pr_err("%s: Invalid Input\n", __func__);
You introduce a pointless error message on SoCs not setting a cpuidle driver. When will people learn that initcalls do not only run on machines they have on their desk?
Will fix in next version.
- return -EINVAL;
- }
- ret = cpuidle_register_driver(drv);
- if (ret) {
- pr_err("%s: Failed to register cpuidle driver\n", __func__);
It's always nice to add the return value to the error message to get a clue *what* went wrong. The function name though is rather uninteresting.
Will add the return value in the next version.
Sascha
-- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
On Tue, Apr 17, 2012 at 08:54:03AM -0500, Rob Lee wrote:
+void __init imx_cpuidle_set_driver(struct cpuidle_driver *p) +{
- drv = p;
+}
You like it complicated, eh? Why do you introduce a function which sets a variable...
This complication is used to deal with the timing of various levels of init calls. More explanation below.
Regardless of how you end up solving this, it's probably a good idea to document the rationale, perhaps cribbing from what you describe below..
If I called imx_cpuidle_init directly from imx5 or imx6q init routines, it would be getting called before the coreinit_call of core cpuidle causing a failure. There were various other directions to take and all seemed less desirable than this one.
One alternative would be to add a function to return the pointer to the cpuidle driver object based on the machine type. Functionality exists to identify imx5 as a machine type but not imx6q, so I couldn't use that machine based method without adding that extra code.
Another alternative would be to add a general platform lateinit_call function to each platforms that support cpuidle.
..in a comment; without it, the code indeed looks bizarre.
On Tue, Apr 17, 2012 at 9:13 AM, Christian Robottom Reis kiko@linaro.org wrote:
On Tue, Apr 17, 2012 at 08:54:03AM -0500, Rob Lee wrote:
+void __init imx_cpuidle_set_driver(struct cpuidle_driver *p) +{
- drv = p;
+}
You like it complicated, eh? Why do you introduce a function which sets a variable...
This complication is used to deal with the timing of various levels of init calls. More explanation below.
Regardless of how you end up solving this, it's probably a good idea to document the rationale, perhaps cribbing from what you describe below..
Agree. Adding comments to the function itself seems to be the most relevant location so I can add that information there if the function remains.
If I called imx_cpuidle_init directly from imx5 or imx6q init routines, it would be getting called before the coreinit_call of core cpuidle causing a failure. There were various other directions to take and all seemed less desirable than this one.
One alternative would be to add a function to return the pointer to the cpuidle driver object based on the machine type. Functionality exists to identify imx5 as a machine type but not imx6q, so I couldn't use that machine based method without adding that extra code.
Another alternative would be to add a general platform lateinit_call function to each platforms that support cpuidle.
..in a comment; without it, the code indeed looks bizarre.
Christian Robottom Reis, Engineering VP Brazil (GMT-3) | [+55] 16 9112 6430 | [+1] 612 216 4935 Linaro.org: Open Source Software for ARM SoCs
On Tue, Apr 17, 2012 at 08:54:03AM -0500, Rob Lee wrote:
On Tue, Apr 17, 2012 at 2:43 AM, Sascha Hauer s.hauer@pengutronix.de wrote:
On Mon, Apr 16, 2012 at 06:50:12PM -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
diff --git a/arch/arm/plat-mxc/Makefile b/arch/arm/plat-mxc/Makefile index e81290c..7c9e05f 100644 --- a/arch/arm/plat-mxc/Makefile +++ b/arch/arm/plat-mxc/Makefile @@ -16,6 +16,8 @@ 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..d1c9301 --- /dev/null +++ b/arch/arm/plat-mxc/cpuidle.c @@ -0,0 +1,89 @@ +/*
- 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; +static struct cpuidle_driver *drv;
+void __init imx_cpuidle_set_driver(struct cpuidle_driver *p) +{
- drv = p;
+}
You like it complicated, eh? Why do you introduce a function which sets a variable...
This complication is used to deal with the timing of various levels of init calls. More explanation below.
+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);
+}
+static int __init imx_cpuidle_init(void)
... instead of passing it directly to the function which uses it?
If I called imx_cpuidle_init directly from imx5 or imx6q init routines, it would be getting called before the coreinit_call of core cpuidle causing a failure. There were various other directions to take and all seemed less desirable than this one.
One alternative would be to add a function to return the pointer to the cpuidle driver object based on the machine type. Functionality exists to identify imx5 as a machine type but not imx6q, so I couldn't use that machine based method without adding that extra code.
Another alternative would be to add a general platform lateinit_call function to each platforms that support cpuidle.
Just put the initcall into mm-imx5.c and check the cpu type. Then you also don't have to make imx5_idle global.
Sascha
+void __init imx_cpuidle_set_driver(struct cpuidle_driver *p) +{
- drv = p;
+}
You like it complicated, eh? Why do you introduce a function which sets a variable...
This complication is used to deal with the timing of various levels of init calls. More explanation below.
+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);
+}
+static int __init imx_cpuidle_init(void)
... instead of passing it directly to the function which uses it?
If I called imx_cpuidle_init directly from imx5 or imx6q init routines, it would be getting called before the coreinit_call of core cpuidle causing a failure. There were various other directions to take and all seemed less desirable than this one.
One alternative would be to add a function to return the pointer to the cpuidle driver object based on the machine type. Functionality exists to identify imx5 as a machine type but not imx6q, so I couldn't use that machine based method without adding that extra code.
Another alternative would be to add a general platform lateinit_call function to each platforms that support cpuidle.
Just put the initcall into mm-imx5.c and check the cpu type. Then you also don't have to make imx5_idle global.
That solution is currently available for imx5 but for imx6q it implies adding the cpu type support for imx6q. Are you ok with that?
Sascha
-- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
If I called imx_cpuidle_init directly from imx5 or imx6q init routines, it would be getting called before the coreinit_call of core cpuidle causing a failure. There were various other directions to take and all seemed less desirable than this one.
One alternative would be to add a function to return the pointer to the cpuidle driver object based on the machine type. Functionality exists to identify imx5 as a machine type but not imx6q, so I couldn't use that machine based method without adding that extra code.
Another alternative would be to add a general platform lateinit_call function to each platforms that support cpuidle.
Just put the initcall into mm-imx5.c and check the cpu type. Then you also don't have to make imx5_idle global.
That solution is currently available for imx5 but for imx6q it implies adding the cpu type support for imx6q. Are you ok with that?
Sascha or Shawn, any further comments on my question?
Thanks, Rob
On Wed, Apr 18, 2012 at 11:18:55PM -0500, Rob Lee wrote:
If I called imx_cpuidle_init directly from imx5 or imx6q init routines, it would be getting called before the coreinit_call of core cpuidle causing a failure. There were various other directions to take and all seemed less desirable than this one.
One alternative would be to add a function to return the pointer to the cpuidle driver object based on the machine type. Functionality exists to identify imx5 as a machine type but not imx6q, so I couldn't use that machine based method without adding that extra code.
Another alternative would be to add a general platform lateinit_call function to each platforms that support cpuidle.
Just put the initcall into mm-imx5.c and check the cpu type. Then you also don't have to make imx5_idle global.
That solution is currently available for imx5 but for imx6q it implies adding the cpu type support for imx6q. Are you ok with that?
Sascha or Shawn, any further comments on my question?
I don't think we need a cpu_is_imx6q(), but having some i.MX6 specific hook at device_initcall time can't be too wrong. Shawn?
Sascha
On Thu, Apr 19, 2012 at 08:43:08AM +0200, Sascha Hauer wrote:
Sascha or Shawn, any further comments on my question?
Sorry for the late response, Rob.
I don't think we need a cpu_is_imx6q(), but having some i.MX6 specific hook at device_initcall time can't be too wrong. Shawn?
Yep, it works for me.
I don't think we need a cpu_is_imx6q(), but having some i.MX6 specific hook at device_initcall time can't be too wrong. Shawn?
Yep, it works for me.
Sascha, Shawn, thanks for the response.
Since device_initcall isn't platform specific, it seems I would still need a cpu_is_imx6q() function or similiar functionality from a device tree call. Or do you have something else in mind that I'm not seeing?
Thanks, Rob
-- Regards, Shawn
On Sun, Apr 22, 2012 at 11:44:39PM -0500, Rob Lee wrote:
I don't think we need a cpu_is_imx6q(), but having some i.MX6 specific hook at device_initcall time can't be too wrong. Shawn?
Yep, it works for me.
Sascha, Shawn, thanks for the response.
Since device_initcall isn't platform specific, it seems I would still need a cpu_is_imx6q() function or similiar functionality from a device tree call. Or do you have something else in mind that I'm not seeing?
I guess Sascha is asking for something like the following.
static int __init imx_device_init(void) { imx5_device_init(); imx6_device_init(); } device_initcall(imx_device_init)
static int __init imx6_device_init(void) { /* * do whatever needs to get done in device_initcall time */ }
On Mon, Apr 23, 2012 at 01:18:21PM +0800, Shawn Guo wrote:
On Sun, Apr 22, 2012 at 11:44:39PM -0500, Rob Lee wrote:
I don't think we need a cpu_is_imx6q(), but having some i.MX6 specific hook at device_initcall time can't be too wrong. Shawn?
Yep, it works for me.
Sascha, Shawn, thanks for the response.
Since device_initcall isn't platform specific, it seems I would still need a cpu_is_imx6q() function or similiar functionality from a device tree call. Or do you have something else in mind that I'm not seeing?
I guess Sascha is asking for something like the following.
static int __init imx_device_init(void) { imx5_device_init(); imx6_device_init(); } device_initcall(imx_device_init)
static int __init imx6_device_init(void) { /* * do whatever needs to get done in device_initcall time */ }
The problem is more how we can detect that we are actually running a i.MX6 SoC. We could directly ask the devicetree in an initcall or we could introduce a cpu_is_mx6() just like we have a macro for all other i.MX SoCs.
Sascha
On Mon, Apr 23, 2012 at 08:27:39AM +0200, Sascha Hauer wrote:
On Mon, Apr 23, 2012 at 01:18:21PM +0800, Shawn Guo wrote:
On Sun, Apr 22, 2012 at 11:44:39PM -0500, Rob Lee wrote:
I don't think we need a cpu_is_imx6q(), but having some i.MX6 specific hook at device_initcall time can't be too wrong. Shawn?
Yep, it works for me.
Sascha, Shawn, thanks for the response.
Since device_initcall isn't platform specific, it seems I would still need a cpu_is_imx6q() function or similiar functionality from a device tree call. Or do you have something else in mind that I'm not seeing?
I guess Sascha is asking for something like the following.
static int __init imx_device_init(void) { imx5_device_init(); imx6_device_init(); } device_initcall(imx_device_init)
static int __init imx6_device_init(void) { /* * do whatever needs to get done in device_initcall time */ }
The problem is more how we can detect that we are actually running a i.MX6 SoC. We could directly ask the devicetree in an initcall or we could introduce a cpu_is_mx6() just like we have a macro for all other i.MX SoCs.
Oops, my reply was a brain-dead one. Hmm, then it seems that we have to introduce cpu_is_mx6() which I tried hard to avoid. I do not have a preference between defining a macro and asking device tree.
On Mon, Apr 23, 2012 at 02:53:01PM +0800, Shawn Guo wrote:
On Mon, Apr 23, 2012 at 08:27:39AM +0200, Sascha Hauer wrote:
On Mon, Apr 23, 2012 at 01:18:21PM +0800, Shawn Guo wrote:
On Sun, Apr 22, 2012 at 11:44:39PM -0500, Rob Lee wrote:
I don't think we need a cpu_is_imx6q(), but having some i.MX6 specific hook at device_initcall time can't be too wrong. Shawn?
Yep, it works for me.
Sascha, Shawn, thanks for the response.
Since device_initcall isn't platform specific, it seems I would still need a cpu_is_imx6q() function or similiar functionality from a device tree call. Or do you have something else in mind that I'm not seeing?
I guess Sascha is asking for something like the following.
static int __init imx_device_init(void) { imx5_device_init(); imx6_device_init(); } device_initcall(imx_device_init)
static int __init imx6_device_init(void) { /* * do whatever needs to get done in device_initcall time */ }
The problem is more how we can detect that we are actually running a i.MX6 SoC. We could directly ask the devicetree in an initcall or we could introduce a cpu_is_mx6() just like we have a macro for all other i.MX SoCs.
Oops, my reply was a brain-dead one. Hmm, then it seems that we have to introduce cpu_is_mx6() which I tried hard to avoid. I do not have a preference between defining a macro and asking device tree.
Since we already have a place in early setup code in which we know that we are running on an i.MX6 I suggest for the sake of the symmetry of the universe that we introduce a cpu_is_mx6.
Sascha
On Mon, Apr 23, 2012 at 08:56:23AM +0200, Sascha Hauer wrote:
On Mon, Apr 23, 2012 at 02:53:01PM +0800, Shawn Guo wrote:
On Mon, Apr 23, 2012 at 08:27:39AM +0200, Sascha Hauer wrote:
On Mon, Apr 23, 2012 at 01:18:21PM +0800, Shawn Guo wrote:
On Sun, Apr 22, 2012 at 11:44:39PM -0500, Rob Lee wrote:
...
Since device_initcall isn't platform specific, it seems I would still need a cpu_is_imx6q() function or similiar functionality from a device tree call. Or do you have something else in mind that I'm not seeing?
I guess Sascha is asking for something like the following.
static int __init imx_device_init(void) { imx5_device_init(); imx6_device_init(); } device_initcall(imx_device_init)
static int __init imx6_device_init(void) { /* * do whatever needs to get done in device_initcall time */ }
The problem is more how we can detect that we are actually running a i.MX6 SoC. We could directly ask the devicetree in an initcall or we could introduce a cpu_is_mx6() just like we have a macro for all other i.MX SoCs.
Oops, my reply was a brain-dead one. Hmm, then it seems that we have to introduce cpu_is_mx6() which I tried hard to avoid. I do not have a preference between defining a macro and asking device tree.
Since we already have a place in early setup code in which we know that we are running on an i.MX6 I suggest for the sake of the symmetry of the universe that we introduce a cpu_is_mx6.
Let me try last time. What about having a late_initcall hook in machine_desc?
Regards, Shawn
8<---
diff --git a/arch/arm/include/asm/mach/arch.h b/arch/arm/include/asm/mach/arch.h index d7692ca..0b1c94b 100644 --- a/arch/arm/include/asm/mach/arch.h +++ b/arch/arm/include/asm/mach/arch.h @@ -43,6 +43,7 @@ struct machine_desc { void (*init_irq)(void); struct sys_timer *timer; /* system tick timer */ void (*init_machine)(void); + void (*init_late)(void); #ifdef CONFIG_MULTI_IRQ_HANDLER void (*handle_irq)(struct pt_regs *); #endif diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c index ebfac78..549f036 100644 --- a/arch/arm/kernel/setup.c +++ b/arch/arm/kernel/setup.c @@ -800,6 +800,14 @@ static int __init customize_machine(void) } arch_initcall(customize_machine);
+static int __init init_machine_late(void) +{ + if (machine_desc->init_late) + machine_desc->init_late(); + return 0; +} +late_initcall(init_machine_late); + #ifdef CONFIG_KEXEC static inline unsigned long long get_total_mem(void) { diff --git a/arch/arm/mach-imx/mach-imx6q.c b/arch/arm/mach-imx/mach-imx6q.c index da6c1d9..0e3640f 100644 --- a/arch/arm/mach-imx/mach-imx6q.c +++ b/arch/arm/mach-imx/mach-imx6q.c @@ -142,6 +142,7 @@ DT_MACHINE_START(IMX6Q, "Freescale i.MX6 Quad (Device Tree)") .handle_irq = imx6q_handle_irq, .timer = &imx6q_timer, .init_machine = imx6q_init_machine, + .init_late = imx6q_init_late, .dt_compat = imx6q_dt_compat, .restart = imx6q_restart, MACHINE_END
On Mon, Apr 23, 2012 at 03:10:15PM +0800, Shawn Guo wrote:
On Mon, Apr 23, 2012 at 08:56:23AM +0200, Sascha Hauer wrote:
On Mon, Apr 23, 2012 at 02:53:01PM +0800, Shawn Guo wrote:
On Mon, Apr 23, 2012 at 08:27:39AM +0200, Sascha Hauer wrote:
On Mon, Apr 23, 2012 at 01:18:21PM +0800, Shawn Guo wrote:
On Sun, Apr 22, 2012 at 11:44:39PM -0500, Rob Lee wrote:
...
i.MX6 SoC. We could directly ask the devicetree in an initcall or we could introduce a cpu_is_mx6() just like we have a macro for all other i.MX SoCs.
Oops, my reply was a brain-dead one. Hmm, then it seems that we have to introduce cpu_is_mx6() which I tried hard to avoid. I do not have a preference between defining a macro and asking device tree.
Since we already have a place in early setup code in which we know that we are running on an i.MX6 I suggest for the sake of the symmetry of the universe that we introduce a cpu_is_mx6.
Let me try last time. What about having a late_initcall hook in machine_desc?
Also fine with me.
Regards, Shawn
8<---
diff --git a/arch/arm/include/asm/mach/arch.h b/arch/arm/include/asm/mach/arch.h index d7692ca..0b1c94b 100644 --- a/arch/arm/include/asm/mach/arch.h +++ b/arch/arm/include/asm/mach/arch.h @@ -43,6 +43,7 @@ struct machine_desc { void (*init_irq)(void); struct sys_timer *timer; /* system tick timer */ void (*init_machine)(void);
void (*init_late)(void);#ifdef CONFIG_MULTI_IRQ_HANDLER void (*handle_irq)(struct pt_regs *); #endif diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c index ebfac78..549f036 100644 --- a/arch/arm/kernel/setup.c +++ b/arch/arm/kernel/setup.c @@ -800,6 +800,14 @@ static int __init customize_machine(void) } arch_initcall(customize_machine);
+static int __init init_machine_late(void) +{
if (machine_desc->init_late)machine_desc->init_late();return 0;+} +late_initcall(init_machine_late);
#ifdef CONFIG_KEXEC static inline unsigned long long get_total_mem(void) { diff --git a/arch/arm/mach-imx/mach-imx6q.c b/arch/arm/mach-imx/mach-imx6q.c index da6c1d9..0e3640f 100644 --- a/arch/arm/mach-imx/mach-imx6q.c +++ b/arch/arm/mach-imx/mach-imx6q.c @@ -142,6 +142,7 @@ DT_MACHINE_START(IMX6Q, "Freescale i.MX6 Quad (Device Tree)") .handle_irq = imx6q_handle_irq, .timer = &imx6q_timer, .init_machine = imx6q_init_machine,
.init_late = imx6q_init_late, .dt_compat = imx6q_dt_compat, .restart = imx6q_restart,MACHINE_END
linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Let me try last time. What about having a late_initcall hook in machine_desc?
Also fine with me.
Shall I add Shawn's patch to my imx cpuidle patchset or should the arch/arm/kernel/setup.c and arch.h changes be submitted separately? If separately, Shawn, did you want to submit this patch or should I?
Thanks, Rob
Regards, Shawn
8<---
diff --git a/arch/arm/include/asm/mach/arch.h b/arch/arm/include/asm/mach/arch.h index d7692ca..0b1c94b 100644 --- a/arch/arm/include/asm/mach/arch.h +++ b/arch/arm/include/asm/mach/arch.h @@ -43,6 +43,7 @@ struct machine_desc { void (*init_irq)(void); struct sys_timer *timer; /* system tick timer */ void (*init_machine)(void);
- void (*init_late)(void);
#ifdef CONFIG_MULTI_IRQ_HANDLER void (*handle_irq)(struct pt_regs *); #endif diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c index ebfac78..549f036 100644 --- a/arch/arm/kernel/setup.c +++ b/arch/arm/kernel/setup.c @@ -800,6 +800,14 @@ static int __init customize_machine(void) } arch_initcall(customize_machine);
+static int __init init_machine_late(void) +{
- if (machine_desc->init_late)
- machine_desc->init_late();
- return 0;
+} +late_initcall(init_machine_late);
#ifdef CONFIG_KEXEC static inline unsigned long long get_total_mem(void) { diff --git a/arch/arm/mach-imx/mach-imx6q.c b/arch/arm/mach-imx/mach-imx6q.c index da6c1d9..0e3640f 100644 --- a/arch/arm/mach-imx/mach-imx6q.c +++ b/arch/arm/mach-imx/mach-imx6q.c @@ -142,6 +142,7 @@ DT_MACHINE_START(IMX6Q, "Freescale i.MX6 Quad (Device Tree)") .handle_irq = imx6q_handle_irq, .timer = &imx6q_timer, .init_machine = imx6q_init_machine,
- .init_late = imx6q_init_late,
.dt_compat = imx6q_dt_compat, .restart = imx6q_restart, MACHINE_END
linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
-- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
On Mon, Apr 23, 2012 at 10:45:02AM -0500, Rob Lee wrote:
Let me try last time. What about having a late_initcall hook in machine_desc?
Also fine with me.
Shall I add Shawn's patch to my imx cpuidle patchset or should the arch/arm/kernel/setup.c and arch.h changes be submitted separately? If separately, Shawn, did you want to submit this patch or should I?
Strange. Russell is not in the Cc list. I remember I added Russell into Cc when I propose the idea. Added him again.
Rob,
I suggest you have changes on arch/arm/kernel/setup.c and arch.h be a separate patch, but you can still have it in the series to show why we need the changes. Cc Russell when posting the series, and see if Russell is fine with the patch. If he is, we can ask his preference how the patch should go in, submitting it to his patch tracker or we can have it go though arm-soc along with the series to save the dependency.
Regards, Shawn
8<---
diff --git a/arch/arm/include/asm/mach/arch.h b/arch/arm/include/asm/mach/arch.h index d7692ca..0b1c94b 100644 --- a/arch/arm/include/asm/mach/arch.h +++ b/arch/arm/include/asm/mach/arch.h @@ -43,6 +43,7 @@ struct machine_desc { void (*init_irq)(void); struct sys_timer *timer; /* system tick timer */ void (*init_machine)(void);
- void (*init_late)(void);
#ifdef CONFIG_MULTI_IRQ_HANDLER void (*handle_irq)(struct pt_regs *); #endif diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c index ebfac78..549f036 100644 --- a/arch/arm/kernel/setup.c +++ b/arch/arm/kernel/setup.c @@ -800,6 +800,14 @@ static int __init customize_machine(void) } arch_initcall(customize_machine);
+static int __init init_machine_late(void) +{
- if (machine_desc->init_late)
- machine_desc->init_late();
- return 0;
+} +late_initcall(init_machine_late);
#ifdef CONFIG_KEXEC static inline unsigned long long get_total_mem(void) { diff --git a/arch/arm/mach-imx/mach-imx6q.c b/arch/arm/mach-imx/mach-imx6q.c index da6c1d9..0e3640f 100644 --- a/arch/arm/mach-imx/mach-imx6q.c +++ b/arch/arm/mach-imx/mach-imx6q.c @@ -142,6 +142,7 @@ DT_MACHINE_START(IMX6Q, "Freescale i.MX6 Quad (Device Tree)") .handle_irq = imx6q_handle_irq, .timer = &imx6q_timer, .init_machine = imx6q_init_machine,
- .init_late = imx6q_init_late,
.dt_compat = imx6q_dt_compat, .restart = imx6q_restart, MACHINE_END
On Tue, Apr 24, 2012 at 09:38:43AM +0800, Shawn Guo wrote:
On Mon, Apr 23, 2012 at 10:45:02AM -0500, Rob Lee wrote:
Let me try last time. What about having a late_initcall hook in machine_desc?
Also fine with me.
Shall I add Shawn's patch to my imx cpuidle patchset or should the arch/arm/kernel/setup.c and arch.h changes be submitted separately? If separately, Shawn, did you want to submit this patch or should I?
Strange. Russell is not in the Cc list. I remember I added Russell into Cc when I propose the idea. Added him again.
I didn't see any message in this thread cc'd to me, but that's not to say I hadn't already read this patch. I don't have any comment against it, but I do wonder how often this hook would be used.
We do seem to have quite a number of late_initcall()s in arch/arm/mach-*, so it seems to be a good idea - provided someone's willing to convert all those users of late_initcall()s.
On Tue, Apr 24, 2012 at 08:54:26AM +0100, Russell King - ARM Linux wrote:
On Tue, Apr 24, 2012 at 09:38:43AM +0800, Shawn Guo wrote:
On Mon, Apr 23, 2012 at 10:45:02AM -0500, Rob Lee wrote:
Let me try last time. What about having a late_initcall hook in machine_desc?
Also fine with me.
Shall I add Shawn's patch to my imx cpuidle patchset or should the arch/arm/kernel/setup.c and arch.h changes be submitted separately? If separately, Shawn, did you want to submit this patch or should I?
Strange. Russell is not in the Cc list. I remember I added Russell into Cc when I propose the idea. Added him again.
I didn't see any message in this thread cc'd to me, but that's not to say I hadn't already read this patch. I don't have any comment against it, but I do wonder how often this hook would be used.
I guess mach-* that use common cpuidle will likely need this hook.
We do seem to have quite a number of late_initcall()s in arch/arm/mach-*, so it seems to be a good idea - provided someone's willing to convert all those users of late_initcall()s.
Agreed. The late_initcall()s in arch/arm/mach-* will not scale for long time, since we are moving toward single build.
On Tue, Apr 24, 2012 at 3:36 AM, Shawn Guo shawn.guo@linaro.org wrote:
On Tue, Apr 24, 2012 at 08:54:26AM +0100, Russell King - ARM Linux wrote:
On Tue, Apr 24, 2012 at 09:38:43AM +0800, Shawn Guo wrote:
On Mon, Apr 23, 2012 at 10:45:02AM -0500, Rob Lee wrote:
Let me try last time. What about having a late_initcall hook in machine_desc?
Also fine with me.
Shall I add Shawn's patch to my imx cpuidle patchset or should the arch/arm/kernel/setup.c and arch.h changes be submitted separately? If separately, Shawn, did you want to submit this patch or should I?
Strange. Russell is not in the Cc list. I remember I added Russell into Cc when I propose the idea. Added him again.
I didn't see any message in this thread cc'd to me, but that's not to say I hadn't already read this patch. I don't have any comment against it, but I do wonder how often this hook would be used.
I guess mach-* that use common cpuidle will likely need this hook.
We do seem to have quite a number of late_initcall()s in arch/arm/mach-*, so it seems to be a good idea - provided someone's willing to convert all those users of late_initcall()s.
Agreed. The late_initcall()s in arch/arm/mach-* will not scale for long time, since we are moving toward single build.
Thanks for the attention on this. From what I've understood, I will send another submission that includes the imx cpuidle patchset and Shawn's device tree late initcall patchset and I'll provide explanation of the two separate patchsets in the cover sheet.
-- Regards, Shawn
On Tue, Apr 24, 2012 at 10:40:35AM -0500, Rob Lee wrote:
Thanks for the attention on this. From what I've understood, I will send another submission that includes the imx cpuidle patchset and Shawn's device tree late initcall patchset and I'll provide explanation of the two separate patchsets in the cover sheet.
What I also want to see _along with_ the addition of the init_late callback is a patch set from the _same_ people fixing up the rest of arch/arm/mach-* to use it - you know, like what I do when I introduce these kinds of things.
Otherwise I'm going to NAK the change. We can't have new stuff introduced in this way and the older platforms ignored. Forward progress across the board please.
On 25 April 2012 03:51, Russell King - ARM Linux linux@arm.linux.org.uk wrote:
On Tue, Apr 24, 2012 at 10:40:35AM -0500, Rob Lee wrote:
Thanks for the attention on this. From what I've understood, I will send another submission that includes the imx cpuidle patchset and Shawn's device tree late initcall patchset and I'll provide explanation of the two separate patchsets in the cover sheet.
What I also want to see _along with_ the addition of the init_late callback is a patch set from the _same_ people fixing up the rest of arch/arm/mach-* to use it - you know, like what I do when I introduce these kinds of things.
Otherwise I'm going to NAK the change. We can't have new stuff introduced in this way and the older platforms ignored. Forward progress across the board please.
I'm working on a series to clean them up.
Regards, Shawn
Add imx5 cpuidle driver.
Signed-off-by: Robert Lee rob.lee@linaro.org --- arch/arm/mach-imx/Makefile | 2 +- arch/arm/mach-imx/cpuidle-imx5.c | 55 +++++++++++++++++++++++++++++++ arch/arm/mach-imx/mm-imx5.c | 16 ++++++--- arch/arm/plat-mxc/include/mach/common.h | 1 + 4 files changed, 69 insertions(+), 5 deletions(-) create mode 100644 arch/arm/mach-imx/cpuidle-imx5.c
diff --git a/arch/arm/mach-imx/Makefile b/arch/arm/mach-imx/Makefile index ab939c5..84b8976 100644 --- a/arch/arm/mach-imx/Makefile +++ b/arch/arm/mach-imx/Makefile @@ -9,7 +9,7 @@ obj-$(CONFIG_SOC_IMX27) += clock-imx27.o mm-imx27.o ehci-imx27.o obj-$(CONFIG_SOC_IMX31) += mm-imx3.o cpu-imx31.o clock-imx31.o iomux-imx31.o ehci-imx31.o pm-imx3.o obj-$(CONFIG_SOC_IMX35) += mm-imx3.o cpu-imx35.o clock-imx35.o ehci-imx35.o pm-imx3.o
-obj-$(CONFIG_SOC_IMX5) += cpu-imx5.o mm-imx5.o clock-mx51-mx53.o ehci-imx5.o pm-imx5.o cpu_op-mx51.o +obj-$(CONFIG_SOC_IMX5) += cpu-imx5.o mm-imx5.o clock-mx51-mx53.o ehci-imx5.o pm-imx5.o cpu_op-mx51.o cpuidle-imx5.o
# Support for CMOS sensor interface obj-$(CONFIG_MX1_VIDEO) += mx1-camera-fiq.o mx1-camera-fiq-ksym.o diff --git a/arch/arm/mach-imx/cpuidle-imx5.c b/arch/arm/mach-imx/cpuidle-imx5.c new file mode 100644 index 0000000..8f428fc --- /dev/null +++ b/arch/arm/mach-imx/cpuidle-imx5.c @@ -0,0 +1,55 @@ +/* + * 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: + * + * http://www.opensource.org/licenses/gpl-license.html + * http://www.gnu.org/copyleft/gpl.html + */ + +#include <linux/kernel.h> +#include <linux/init.h> +#include <linux/export.h> +#include <linux/cpuidle.h> +#include <linux/err.h> +#include <mach/common.h> +#include <mach/cpuidle.h> +#include <mach/hardware.h> + +static int imx5_cpuidle_enter(struct cpuidle_device *dev, + struct cpuidle_driver *drv, int idx) +{ + int ret; + + ret = imx5_idle(); + + if (ret < 0) + return ret; + + return idx; +} + +static struct cpuidle_driver imx5_cpuidle_driver = { + .name = "imx5_cpuidle", + .owner = THIS_MODULE, + .en_core_tk_irqen = 1, + .states[0] = { + .enter = imx5_cpuidle_enter, + .exit_latency = 20, /* max latency at 160MHz */ + .target_residency = 1, + .flags = CPUIDLE_FLAG_TIME_VALID, + .name = "IMX5 SRPG", + .desc = "CPU state retained,powered off", + }, + .state_count = 1, +}; + +int __init imx5_cpuidle_init(void) +{ + imx_cpuidle_set_driver(&imx5_cpuidle_driver); + + return 0; +} diff --git a/arch/arm/mach-imx/mm-imx5.c b/arch/arm/mach-imx/mm-imx5.c index e10f391..594915b 100644 --- a/arch/arm/mach-imx/mm-imx5.c +++ b/arch/arm/mach-imx/mm-imx5.c @@ -22,22 +22,29 @@ #include <mach/common.h> #include <mach/devices-common.h> #include <mach/iomux-v3.h> +#include <mach/cpuidle.h>
static struct clk *gpc_dvfs_clk;
-static void imx5_idle(void) +int imx5_idle(void) { + int ret = 0; + /* gpc clock is needed for SRPG */ if (gpc_dvfs_clk == NULL) { gpc_dvfs_clk = clk_get(NULL, "gpc_dvfs"); if (IS_ERR(gpc_dvfs_clk)) - return; + return -ENODEV; } clk_enable(gpc_dvfs_clk); mx5_cpu_lp_set(WAIT_UNCLOCKED_POWER_OFF); if (!tzic_enable_wake()) cpu_do_idle(); + else + ret = -EBUSY; clk_disable(gpc_dvfs_clk); + + return ret; }
/* @@ -103,7 +110,7 @@ void __init imx51_init_early(void) mxc_set_cpu_type(MXC_CPU_MX51); mxc_iomux_v3_init(MX51_IO_ADDRESS(MX51_IOMUXC_BASE_ADDR)); mxc_arch_reset_init(MX51_IO_ADDRESS(MX51_WDOG1_BASE_ADDR)); - arm_pm_idle = imx5_idle; + arm_pm_idle = (void *)imx5_idle; }
void __init imx53_init_early(void) @@ -203,7 +210,8 @@ void __init imx51_soc_init(void) /* i.mx51 has the i.mx35 type sdma */ imx_add_imx_sdma("imx35-sdma", MX51_SDMA_BASE_ADDR, MX51_INT_SDMA, &imx51_sdma_pdata);
- /* Setup AIPS registers */ + imx5_cpuidle_init(); + imx_set_aips(MX51_IO_ADDRESS(MX51_AIPS1_BASE_ADDR)); imx_set_aips(MX51_IO_ADDRESS(MX51_AIPS2_BASE_ADDR));
diff --git a/arch/arm/plat-mxc/include/mach/common.h b/arch/arm/plat-mxc/include/mach/common.h index 0319c4a..128a572 100644 --- a/arch/arm/plat-mxc/include/mach/common.h +++ b/arch/arm/plat-mxc/include/mach/common.h @@ -95,6 +95,7 @@ enum mx3_cpu_pwr_mode {
extern void mx3_cpu_lp_set(enum mx3_cpu_pwr_mode mode); extern void mx5_cpu_lp_set(enum mxc_cpu_pwr_mode mode); +extern int imx5_idle(void); extern void imx_print_silicon_rev(const char *cpu, int srev);
void avic_handle_irq(struct pt_regs *);
Add basic imx6q cpuidle driver. For now, only basic WFI state is supported. Deeper idle states will be added in the future.
Signed-off-by: Robert Lee rob.lee@linaro.org --- arch/arm/mach-imx/Makefile | 2 +- arch/arm/mach-imx/cpuidle-imx6q.c | 33 +++++++++++++++++++++++++++++++++ arch/arm/mach-imx/mach-imx6q.c | 2 ++ 3 files changed, 36 insertions(+), 1 deletion(-) create mode 100644 arch/arm/mach-imx/cpuidle-imx6q.c
diff --git a/arch/arm/mach-imx/Makefile b/arch/arm/mach-imx/Makefile index 84b8976..530f0fd 100644 --- a/arch/arm/mach-imx/Makefile +++ b/arch/arm/mach-imx/Makefile @@ -70,7 +70,7 @@ obj-$(CONFIG_CPU_V7) += head-v7.o AFLAGS_head-v7.o :=-Wa,-march=armv7-a obj-$(CONFIG_SMP) += platsmp.o obj-$(CONFIG_HOTPLUG_CPU) += hotplug.o -obj-$(CONFIG_SOC_IMX6Q) += clock-imx6q.o mach-imx6q.o +obj-$(CONFIG_SOC_IMX6Q) += clock-imx6q.o mach-imx6q.o cpuidle-imx6q.o
ifeq ($(CONFIG_PM),y) obj-$(CONFIG_SOC_IMX6Q) += pm-imx6q.o diff --git a/arch/arm/mach-imx/cpuidle-imx6q.c b/arch/arm/mach-imx/cpuidle-imx6q.c new file mode 100644 index 0000000..b74557f --- /dev/null +++ b/arch/arm/mach-imx/cpuidle-imx6q.c @@ -0,0 +1,33 @@ +/* + * 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: + * + * http://www.opensource.org/licenses/gpl-license.html + * http://www.gnu.org/copyleft/gpl.html + */ + +#include <linux/kernel.h> +#include <linux/init.h> +#include <linux/cpuidle.h> +#include <linux/export.h> +#include <asm/cpuidle.h> +#include <mach/cpuidle.h> + +static struct cpuidle_driver imx6q_cpuidle_driver = { + .name = "imx6q_cpuidle", + .owner = THIS_MODULE, + .en_core_tk_irqen = 1, + .states[0] = ARM_CPUIDLE_WFI_STATE, + .state_count = 1, +}; + +int __init imx6q_cpuidle_init(void) +{ + imx_cpuidle_set_driver(&imx6q_cpuidle_driver); + + return 0; +} diff --git a/arch/arm/mach-imx/mach-imx6q.c b/arch/arm/mach-imx/mach-imx6q.c index da6c1d9..6f31ca6 100644 --- a/arch/arm/mach-imx/mach-imx6q.c +++ b/arch/arm/mach-imx/mach-imx6q.c @@ -29,6 +29,7 @@ #include <asm/system_misc.h> #include <mach/common.h> #include <mach/hardware.h> +#include <mach/cpuidle.h>
void imx6q_restart(char mode, const char *cmd) { @@ -84,6 +85,7 @@ static void __init imx6q_init_machine(void) of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
imx6q_pm_init(); + imx6q_cpuidle_init(); }
static void __init imx6q_map_io(void)
-
Christian Robottom Reis -
Jesper Juhl -
Rob Lee -
Robert Lee
-
Russell King - ARM Linux -
Sascha Hauer -
Shawn Guo