Hi Sascha,
On Tue, Feb 15, 2011 at 7:27 PM, Sascha Hauer s.hauer@pengutronix.dewrote:
On Fri, Feb 11, 2011 at 10:36:12AM +0100, yong.shen@linaro.org wrote:
From: Yong Shen yong.shen@freescale.com
implement cpuidle driver for iMX5 SOCs, leave cpuidle params to board related code.
Signed-off-by: Yong Shen yong.shen@freescale.com
arch/arm/mach-mx5/Makefile | 1 + arch/arm/mach-mx5/cpuidle.c | 113
+++++++++++++++++++++++++++++++++++++++++++
arch/arm/mach-mx5/cpuidle.h | 26 ++++++++++ 3 files changed, 140 insertions(+), 0 deletions(-) create mode 100644 arch/arm/mach-mx5/cpuidle.c create mode 100644 arch/arm/mach-mx5/cpuidle.h
diff --git a/arch/arm/mach-mx5/Makefile b/arch/arm/mach-mx5/Makefile index 0d43be9..12239e0 100644 --- a/arch/arm/mach-mx5/Makefile +++ b/arch/arm/mach-mx5/Makefile @@ -7,6 +7,7 @@ obj-y := cpu.o mm.o clock-mx51-mx53.o devices.o obj-$(CONFIG_SOC_IMX50) += mm-mx50.o
obj-$(CONFIG_CPU_FREQ_IMX) += cpu_op-mx51.o +obj-$(CONFIG_CPU_IDLE) += cpuidle.o obj-$(CONFIG_MACH_MX51_BABBAGE) += board-mx51_babbage.o obj-$(CONFIG_MACH_MX51_3DS) += board-mx51_3ds.o obj-$(CONFIG_MACH_MX53_EVK) += board-mx53_evk.o diff --git a/arch/arm/mach-mx5/cpuidle.c b/arch/arm/mach-mx5/cpuidle.c new file mode 100644 index 0000000..9d77c47 --- /dev/null +++ b/arch/arm/mach-mx5/cpuidle.c @@ -0,0 +1,113 @@ +/*
- arch/arm/mach-mx5/cpuidle.c
- This file is licensed under the terms of the GNU General Public
- License version 2. This program is licensed "as is" without any
- warranty of any kind, whether express or implied.
- */
+#include <linux/io.h> +#include <linux/cpuidle.h> +#include <asm/proc-fns.h> +#include <mach/hardware.h> +#include "cpuidle.h" +#include "crm_regs.h"
+static struct imx_cpuidle_params *imx_cpuidle_params; +void imx_cpuidle_board_params(struct imx_cpuidle_params *cpuidle_params) +{
imx_cpuidle_params = cpuidle_params;+}
+extern int tzic_enable_wake(int is_idle);
Please put this into a header file.
Ok, but I guess I should do it in another patch before this patch set.
+static int imx_enter_idle(struct cpuidle_device *dev,
struct cpuidle_state *state)+{
struct timeval before, after;int idle_time;u32 plat_lpc, arm_srpgcr, ccm_clpcr;u32 empgc0, empgc1;local_irq_disable();do_gettimeofday(&before);plat_lpc = __raw_readl(MXC_CORTEXA8_PLAT_LPC) &~(MXC_CORTEXA8_PLAT_LPC_DSM);ccm_clpcr = __raw_readl(MXC_CCM_CLPCR) & ~(MXC_CCM_CLPCR_LPM_MASK);arm_srpgcr = __raw_readl(MXC_SRPG_ARM_SRPGCR) & ~(MXC_SRPGCR_PCR);empgc0 = __raw_readl(MXC_SRPG_EMPGC0_SRPGCR) & ~(MXC_SRPGCR_PCR);empgc1 = __raw_readl(MXC_SRPG_EMPGC1_SRPGCR) & ~(MXC_SRPGCR_PCR);if (state == &dev->states[WAIT_CLK_ON]);An if without code? This looks strange.
Yes, a little bit. I meant to treat this like a explanation for different cases, and I also can remove it and put a comments here.
else if (state == &dev->states[WAIT_CLK_OFF])ccm_clpcr |= (0x1 << MXC_CCM_CLPCR_LPM_OFFSET);else if (state == &dev->states[WAIT_CLK_OFF_POWER_OFF]) {/* Wait unclocked, power off */plat_lpc |= MXC_CORTEXA8_PLAT_LPC_DSM| MXC_CORTEXA8_PLAT_LPC_DBG_DSM;arm_srpgcr |= MXC_SRPGCR_PCR;ccm_clpcr |= (0x1 << MXC_CCM_CLPCR_LPM_OFFSET);ccm_clpcr &= ~MXC_CCM_CLPCR_VSTBY;ccm_clpcr &= ~MXC_CCM_CLPCR_SBYOS;if (tzic_enable_wake(1) != 0) {local_irq_enable();return 0;}}__raw_writel(plat_lpc, MXC_CORTEXA8_PLAT_LPC);__raw_writel(ccm_clpcr, MXC_CCM_CLPCR);__raw_writel(arm_srpgcr, MXC_SRPG_ARM_SRPGCR);cpu_do_idle();do_gettimeofday(&after);local_irq_enable();idle_time = (after.tv_sec - before.tv_sec) * USEC_PER_SEC +(after.tv_usec - before.tv_usec);return idle_time;+}
+static struct cpuidle_driver imx_cpuidle_driver = {
.name = "imx_idle",.owner = THIS_MODULE,+};
+static DEFINE_PER_CPU(struct cpuidle_device, imx_cpuidle_device);
+static int __init imx_cpuidle_init(void) +{
struct cpuidle_device *device;int i;if (imx_cpuidle_params == NULL)return -ENODEV;cpuidle_register_driver(&imx_cpuidle_driver);device = &per_cpu(imx_cpuidle_device, smp_processor_id());device->state_count = IMX_MAX_CPUIDLE_STATE;for (i = 0; i < IMX_MAX_CPUIDLE_STATE && i < CPUIDLE_STATE_MAX;i++) {
device->states[i].enter = imx_enter_idle;device->states[i].exit_latency =imx_cpuidle_params[i].latency;
device->states[i].flags = CPUIDLE_FLAG_TIME_VALID;}strcpy(device->states[WAIT_CLK_ON].name, "WFI 0");strcpy(device->states[WAIT_CLK_ON].desc, "Wait with clock on");strcpy(device->states[WAIT_CLK_OFF].name, "WFI 1");strcpy(device->states[WAIT_CLK_OFF].desc, "Wait with clock off");strcpy(device->states[WAIT_CLK_OFF_POWER_OFF].name, "WFI 2");strcpy(device->states[WAIT_CLK_OFF_POWER_OFF].desc,"Wait with clock off and power gating");if (cpuidle_register_device(device)) {printk(KERN_ERR "imx_cpuidle_init: Failed registering\n");return -ENODEV;}return 0;+}
+late_initcall(imx_cpuidle_init);
We have a late_initcall here which needs to be protected from other cpus. On the other hand we depend on board code calling imx_cpuidle_board_params() before this initcall. I think the board code should call a imx_cpuidle_init(struct imx_cpuidle_params *cpuidle_params) instead which makes the flow of execution more clear. Also, the function should be named mx51_cpuidle_init().
Then, I assume the best way might be that remove imx_cpuidle_board_params(), and let board code call mx51_cpuidle_init(), at the same time the params can be passed in by the same funciton.
diff --git a/arch/arm/mach-mx5/cpuidle.h b/arch/arm/mach-mx5/cpuidle.h new file mode 100644 index 0000000..e5ba495 --- /dev/null +++ b/arch/arm/mach-mx5/cpuidle.h @@ -0,0 +1,26 @@ +/*
- arch/arm/mach-mx5/cpuidle.h
- This file is licensed under the terms of the GNU General Public
- License version 2. This program is licensed "as is" without any
- warranty of any kind, whether express or implied.
- */
+enum {
WAIT_CLK_ON, /* c1 */WAIT_CLK_OFF, /* c2 */WAIT_CLK_OFF_POWER_OFF, /* c3 */IMX_MAX_CPUIDLE_STATE,+};
+struct imx_cpuidle_params {
unsigned int latency;+};
+#ifdef CONFIG_CPU_IDLE +extern void imx_cpuidle_board_params(struct imx_cpuidle_params
*cpuidle_params);
+#else +inline void imx_cpuidle_board_params(struct imx_cpuidle_params
*cpuidle_params)
+{} +#endif
-- 1.7.1
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 |