Cleanup up imx5 idle code and enable imx5 low powe idle for imx53.
Add common imx cpuidle initialization functionality and add a i.MX5 and i.MX6Q platform cpuidle implementation.
Based on v3.4-rc7 plus recently submitted/accepted MACHINE_INIT late_initcall patch: http://www.spinics.net/lists/arm-kernel/msg171620.html
v4 changes: * Added several imx5 idle cleanups to series. * Modified imx_io_p2v function to allow common imx5 AIPS2 bus virutal address * Added comment to tzic_wakeup_enable(). * Movied imx5 idle code from mm-imx5.c to pm-imx5.c. * Removed unnecessary time consuming code execution that ran on each idle instance. * modified imx5_pm_init to be a late_initcall * added late_initcall to all imx53 MACHINE_START entries. * enabled imx5 low power idle for imx53 * rebased cpuidle driver on top of imx5 cleanup changes. * modified cpuidle driver exit time to reflect removal of unnecessary code
v3 changes: * removed file introduced in v1 no longer needed after v2 [per Shawn Guo] * re-ordered added #includes in alphabetical order [per Shawn Guo] * Remove if(!drv) check to allow handling by stack trace [per Sascha Hauer] * Added missing return value in error meesage [per Shawn Guo] * fixed (void *) casting problem pointed out Sasha Hauer. Used a different type of casting to properly give warning on improper casting: arm_pm_idle = (void (*)(void))imx5_idle; Used this casting instead of adding a dummy caller function because adding the dummy function appears to unnecessarily introduce the following additional operations: static void imx5_pm_idle(void) { a0: e1a0c00d mov ip, sp a4: e92dd800 push {fp, ip, lr, pc} a8: e24cb004 sub fp, ip, #4 imx5_idle(); ac: ebffffd3 bl 0 <imx5_idle> } b0: e89da800 ldm sp, {fp, sp, pc}
http://permalink.gmane.org/gmane.linux.linaro.devel/11653
v2 changes: * Removed some unnecessary spaces [per Jess Juhl] * Added return value for an error message [per Sascha Hauer] * Reworked init scheme to use device tree late_initcall [per Sascha and Shawn] * Moved imx6q and imx5 cpuidle functionality to existing files.
https://lkml.org/lkml/2012/5/1/363
v1 initial submission:
https://lkml.org/lkml/2012/4/16/644
Robert Lee (7): ARM: imx: Modify IMX_IO_P2V macro ARM: imx: Add comments to tzic_enable_waker() ARM: imx: clean and consolidate imx5 suspend and idle code ARM: imx: Enable imx53 low power idle ARM: imx: Add common imx cpuidle init functionality. ARM: imx: Add imx5 cpuidle ARM: imx: Add imx6q cpuidle driver
arch/arm/mach-imx/clock-mx51-mx53.c | 1 + arch/arm/mach-imx/imx53-dt.c | 1 + arch/arm/mach-imx/mach-imx6q.c | 19 ++++++ arch/arm/mach-imx/mach-mx53_ard.c | 1 + arch/arm/mach-imx/mach-mx53_evk.c | 1 + arch/arm/mach-imx/mach-mx53_loco.c | 1 + arch/arm/mach-imx/mach-mx53_smd.c | 1 + arch/arm/mach-imx/mm-imx5.c | 25 ++----- arch/arm/mach-imx/pm-imx5.c | 103 +++++++++++++++++++++-------- arch/arm/plat-mxc/Makefile | 1 + arch/arm/plat-mxc/cpuidle.c | 80 ++++++++++++++++++++++ arch/arm/plat-mxc/include/mach/common.h | 4 +- arch/arm/plat-mxc/include/mach/cpuidle.h | 22 ++++++ arch/arm/plat-mxc/include/mach/hardware.h | 25 ++++--- arch/arm/plat-mxc/tzic.c | 4 ++ 15 files changed, 231 insertions(+), 58 deletions(-) create mode 100644 arch/arm/plat-mxc/cpuidle.c create mode 100644 arch/arm/plat-mxc/include/mach/cpuidle.h
A change is needed in the IMX_IO_P2V macro to allow all imx5 platforms to use common definitions when accessing registers of peripherals on the AIPS2 bus. With this change, IMX_IO_P2V(MX50_AIPS2_BASE_ADDR) == IMX_IO_P2V(MX51_AIPS2_BASE_ADDR) == IMX_IO_P2V(MX53_AIPS2_BASE_ADDR).
This change was tested for mapping conflicts using the iop2v script found at git://git.pengutronix.de/git/ukl/imx-iop2v.git and by performing a bootup of a default build using imx_v6_v7_defconfig on a imx51 babbage board and imx53 loco board. The comments were modified to reflect the output given by the script which shows the virtual address mappings.
Signed-off-by: Robert Lee rob.lee@linaro.org --- arch/arm/plat-mxc/include/mach/hardware.h | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-)
diff --git a/arch/arm/plat-mxc/include/mach/hardware.h b/arch/arm/plat-mxc/include/mach/hardware.h index 0630513..071afd0 100644 --- a/arch/arm/plat-mxc/include/mach/hardware.h +++ b/arch/arm/plat-mxc/include/mach/hardware.h @@ -50,7 +50,7 @@ * IO 0x00200000+0x100000 -> 0xf4000000+0x100000 * mx21: * AIPI 0x10000000+0x100000 -> 0xf4400000+0x100000 - * SAHB1 0x80000000+0x100000 -> 0xf4000000+0x100000 + * SAHB1 0x80000000+0x100000 -> 0xf5000000+0x100000 * X_MEMC 0xdf000000+0x004000 -> 0xf5f00000+0x004000 * mx25: * AIPS1 0x43f00000+0x100000 -> 0xf5300000+0x100000 @@ -58,47 +58,50 @@ * AVIC 0x68000000+0x100000 -> 0xf5800000+0x100000 * mx27: * AIPI 0x10000000+0x100000 -> 0xf4400000+0x100000 - * SAHB1 0x80000000+0x100000 -> 0xf4000000+0x100000 + * SAHB1 0x80000000+0x100000 -> 0xf5000000+0x100000 * X_MEMC 0xd8000000+0x100000 -> 0xf5c00000+0x100000 * mx31: * AIPS1 0x43f00000+0x100000 -> 0xf5300000+0x100000 * AIPS2 0x53f00000+0x100000 -> 0xf5700000+0x100000 * AVIC 0x68000000+0x100000 -> 0xf5800000+0x100000 - * X_MEMC 0xb8000000+0x010000 -> 0xf4c00000+0x010000 + * X_MEMC 0xb8000000+0x010000 -> 0xf5c00000+0x010000 * SPBA0 0x50000000+0x100000 -> 0xf5400000+0x100000 * mx35: * AIPS1 0x43f00000+0x100000 -> 0xf5300000+0x100000 * AIPS2 0x53f00000+0x100000 -> 0xf5700000+0x100000 * AVIC 0x68000000+0x100000 -> 0xf5800000+0x100000 - * X_MEMC 0xb8000000+0x010000 -> 0xf4c00000+0x010000 + * X_MEMC 0xb8000000+0x010000 -> 0xf5c00000+0x010000 * SPBA0 0x50000000+0x100000 -> 0xf5400000+0x100000 * mx50: * TZIC 0x0fffc000+0x004000 -> 0xf4bfc000+0x004000 - * SPBA0 0x50000000+0x100000 -> 0xf5400000+0x100000 * AIPS1 0x53f00000+0x100000 -> 0xf5700000+0x100000 + * SPBA0 0x50000000+0x100000 -> 0xf5400000+0x100000 * AIPS2 0x63f00000+0x100000 -> 0xf5300000+0x100000 * mx51: - * TZIC 0xe0000000+0x004000 -> 0xf5000000+0x004000 + * TZIC 0x0fffc000+0x004000 -> 0xf4bfc000+0x004000 * IRAM 0x1ffe0000+0x020000 -> 0xf4fe0000+0x020000 + * DEBUG 0x60000000+0x100000 -> 0xf5000000+0x100000 * SPBA0 0x70000000+0x100000 -> 0xf5400000+0x100000 * AIPS1 0x73f00000+0x100000 -> 0xf5700000+0x100000 - * AIPS2 0x83f00000+0x100000 -> 0xf4300000+0x100000 + * AIPS2 0x83f00000+0x100000 -> 0xf5300000+0x100000 * mx53: * TZIC 0x0fffc000+0x004000 -> 0xf4bfc000+0x004000 + * DEBUG 0x40000000+0x100000 -> 0xf5000000+0x100000 * SPBA0 0x50000000+0x100000 -> 0xf5400000+0x100000 * AIPS1 0x53f00000+0x100000 -> 0xf5700000+0x100000 * AIPS2 0x63f00000+0x100000 -> 0xf5300000+0x100000 * mx6q: - * SCU 0x00a00000+0x001000 -> 0xf4000000+0x001000 + * SCU 0x00a00000+0x004000 -> 0xf4000000+0x004000 * CCM 0x020c4000+0x004000 -> 0xf42c4000+0x004000 - * ANATOP 0x020c8000+0x001000 -> 0xf42c8000+0x001000 + * ANATOP 0x020c8000+0x004000 -> 0xf42c8000+0x004000 * UART4 0x021f0000+0x004000 -> 0xf42f0000+0x004000 */ #define IMX_IO_P2V(x) ( \ - 0xf4000000 + \ + (((x) & 0x80000000) >> 7) | \ + (0xf4000000 + \ (((x) & 0x50000000) >> 6) + \ (((x) & 0x0b000000) >> 4) + \ - (((x) & 0x000fffff))) + (((x) & 0x000fffff))))
#define IMX_IO_ADDRESS(x) IOMEM(IMX_IO_P2V(x))
Add additional comments to the tzic_enable_wake() funciton to clarify its intended usage.
Signed-off-by: Robert Lee rob.lee@linaro.org --- arch/arm/plat-mxc/tzic.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/arch/arm/plat-mxc/tzic.c b/arch/arm/plat-mxc/tzic.c index 98308ec..a45dbea 100644 --- a/arch/arm/plat-mxc/tzic.c +++ b/arch/arm/plat-mxc/tzic.c @@ -190,6 +190,10 @@ void __init tzic_init_irq(void __iomem *irqbase) * tzic_enable_wake() - enable wakeup interrupt * * @return 0 if successful; non-zero otherwise + * + * This function provides an interrupt synchronization point that is required + * by tzic enabled platforms before entering imx specific low power modes (ie, + * those low power modes beyond the WAIT_CLOCKED basic ARM WFI only mode). */ int tzic_enable_wake(void) {
The imx5 idle code that existed in mm-imx5.c is moved to pm-imx5.c. The imx5_pm_init call is now exported and called during the MACHINE_START late_init in supported imx5 platforms.
Remove various enabling/disabling of the gpc_dvfs clock and enable it once during initialization. This is a very low power clock that must be enabled during low power operations.
There are only two "suspend_state_t" imx5 low power modes ever used. STOP_POWER_OFF for suspend to mem and WAIT_UNCLOCKED_POWER_OFF for idle and suspend to standby. The latter mode only requires 500 nanoseconds of extra hardware exit time beyond a basic WFI operation (WAIT_CLOCKED mode) so no other idle mode is necessary. Given this information, it is more efficient to keep the registers in the often used WAIT_UNCLOCKED_POWER_OFF state and only to and from the STOP_POWER_OFF register state as needed when suspend to mem is required.
Signed-off-by: Robert Lee rob.lee@linaro.org --- arch/arm/mach-imx/mm-imx5.c | 20 +--------- arch/arm/mach-imx/pm-imx5.c | 63 ++++++++++++++++++------------- arch/arm/plat-mxc/include/mach/common.h | 3 +- 3 files changed, 40 insertions(+), 46 deletions(-)
diff --git a/arch/arm/mach-imx/mm-imx5.c b/arch/arm/mach-imx/mm-imx5.c index d6b7e9f..bb38747 100644 --- a/arch/arm/mach-imx/mm-imx5.c +++ b/arch/arm/mach-imx/mm-imx5.c @@ -15,7 +15,6 @@ #include <linux/init.h> #include <linux/clk.h>
-#include <asm/system_misc.h> #include <asm/mach/map.h>
#include <mach/hardware.h> @@ -23,23 +22,6 @@ #include <mach/devices-common.h> #include <mach/iomux-v3.h>
-static struct clk *gpc_dvfs_clk; - -static void imx5_idle(void) -{ - /* 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; - } - clk_enable(gpc_dvfs_clk); - mx5_cpu_lp_set(WAIT_UNCLOCKED_POWER_OFF); - if (!tzic_enable_wake()) - cpu_do_idle(); - clk_disable(gpc_dvfs_clk); -} - /* * Define the MX50 memory map. */ @@ -103,7 +85,6 @@ 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; }
void __init imx53_init_early(void) @@ -238,4 +219,5 @@ void __init imx53_soc_init(void) void __init imx51_init_late(void) { mx51_neon_fixup(); + imx5_pm_init(); } diff --git a/arch/arm/mach-imx/pm-imx5.c b/arch/arm/mach-imx/pm-imx5.c index e26a9cb..6e62d79 100644 --- a/arch/arm/mach-imx/pm-imx5.c +++ b/arch/arm/mach-imx/pm-imx5.c @@ -13,18 +13,27 @@ #include <linux/io.h> #include <linux/err.h> #include <asm/cacheflush.h> +#include <asm/system_misc.h> #include <asm/tlbflush.h> #include <mach/common.h> #include <mach/hardware.h> #include "crm-regs-imx5.h"
-static struct clk *gpc_dvfs_clk; +/* + * The WAIT_UNCLOCKED_POWER_OFF state only requires <= 500ns to exit. + * This is also the lowest power state possible without affecting + * non-cpu parts of the system. For these reasons, imx5 should default + * to always using this state for cpu idling. The PM_SUSPEND_STANDBY also + * uses this state and needs to take no action when registers remain confgiured + * for this state. + */ +#define IMX5_DEFAULT_CPU_IDLE_STATE WAIT_UNCLOCKED_POWER_OFF
/* * set cpu low power mode before WFI instruction. This function is called * mx5 because it can be used for mx50, mx51, and mx53. */ -void mx5_cpu_lp_set(enum mxc_cpu_pwr_mode mode) +static void mx5_cpu_lp_set(enum mxc_cpu_pwr_mode mode) { u32 plat_lpc, arm_srpgcr, ccm_clpcr; u32 empgc0, empgc1; @@ -87,11 +96,6 @@ void mx5_cpu_lp_set(enum mxc_cpu_pwr_mode mode) } }
-static int mx5_suspend_prepare(void) -{ - return clk_prepare_enable(gpc_dvfs_clk); -} - static int mx5_suspend_enter(suspend_state_t state) { switch (state) { @@ -99,7 +103,7 @@ static int mx5_suspend_enter(suspend_state_t state) mx5_cpu_lp_set(STOP_POWER_OFF); break; case PM_SUSPEND_STANDBY: - mx5_cpu_lp_set(WAIT_UNCLOCKED_POWER_OFF); + /* DEFAULT_IDLE_STATE already configured */ break; default: return -EINVAL; @@ -114,12 +118,10 @@ static int mx5_suspend_enter(suspend_state_t state) __raw_writel(0, MXC_SRPG_EMPGC1_SRPGCR); } cpu_do_idle(); - return 0; -}
-static void mx5_suspend_finish(void) -{ - clk_disable_unprepare(gpc_dvfs_clk); + /* return registers to default idle state */ + mx5_cpu_lp_set(IMX5_DEFAULT_CPU_IDLE_STATE); + return 0; }
static int mx5_pm_valid(suspend_state_t state) @@ -129,25 +131,34 @@ static int mx5_pm_valid(suspend_state_t state)
static const struct platform_suspend_ops mx5_suspend_ops = { .valid = mx5_pm_valid, - .prepare = mx5_suspend_prepare, .enter = mx5_suspend_enter, - .finish = mx5_suspend_finish, };
-static int __init mx5_pm_init(void) +static void imx5_pm_idle(void) +{ + if (likely(!tzic_enable_wake())) + cpu_do_idle(); +} + +int __init imx5_pm_init(void) { - if (!cpu_is_mx51() && !cpu_is_mx53()) - return 0; + int ret; + struct clk *gpc_dvfs_clk = clk_get(NULL, "gpc_dvfs"); + + if (IS_ERR(gpc_dvfs_clk)) + return (int)gpc_dvfs_clk; + + ret = clk_enable(gpc_dvfs_clk); + if (ret) + return ret; + + if (cpu_is_mx51()) + suspend_set_ops(&mx5_suspend_ops);
- if (gpc_dvfs_clk == NULL) - gpc_dvfs_clk = clk_get(NULL, "gpc_dvfs"); + arm_pm_idle = imx5_pm_idle;
- if (!IS_ERR(gpc_dvfs_clk)) { - if (cpu_is_mx51()) - suspend_set_ops(&mx5_suspend_ops); - } else - return -EPERM; + /* Set the registers to the default cpu idle state. */ + mx5_cpu_lp_set(IMX5_DEFAULT_CPU_IDLE_STATE);
return 0; } -device_initcall(mx5_pm_init); diff --git a/arch/arm/plat-mxc/include/mach/common.h b/arch/arm/plat-mxc/include/mach/common.h index cf663d8..5660e1e 100644 --- a/arch/arm/plat-mxc/include/mach/common.h +++ b/arch/arm/plat-mxc/include/mach/common.h @@ -95,7 +95,6 @@ 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 void imx_print_silicon_rev(const char *cpu, int srev);
void avic_handle_irq(struct pt_regs *); @@ -146,8 +145,10 @@ extern void imx6q_clock_map_io(void);
#ifdef CONFIG_PM extern void imx6q_pm_init(void); +extern int imx5_pm_init(void); #else static inline void imx6q_pm_init(void) {} +static inline int imx5_pm_init(void) { return -ENODEV; } #endif
#ifdef CONFIG_NEON
Hi Robert,
Overall this looks ok now, some comments inline.
Sascha
On Tue, May 15, 2012 at 09:33:32PM -0500, Robert Lee wrote:
The imx5 idle code that existed in mm-imx5.c is moved to pm-imx5.c. The imx5_pm_init call is now exported and called during the MACHINE_START late_init in supported imx5 platforms.
Remove various enabling/disabling of the gpc_dvfs clock and enable it once during initialization. This is a very low power clock that must be enabled during low power operations.
There are only two "suspend_state_t" imx5 low power modes ever used. STOP_POWER_OFF for suspend to mem and WAIT_UNCLOCKED_POWER_OFF for idle and suspend to standby. The latter mode only requires 500 nanoseconds of extra hardware exit time beyond a basic WFI operation (WAIT_CLOCKED mode) so no other idle mode is necessary. Given this information, it is more efficient to keep the registers in the often used WAIT_UNCLOCKED_POWER_OFF state and only to and from the STOP_POWER_OFF register state as needed when suspend to mem is required.
Signed-off-by: Robert Lee rob.lee@linaro.org
arch/arm/mach-imx/mm-imx5.c | 20 +--------- arch/arm/mach-imx/pm-imx5.c | 63 ++++++++++++++++++------------- arch/arm/plat-mxc/include/mach/common.h | 3 +- 3 files changed, 40 insertions(+), 46 deletions(-)
diff --git a/arch/arm/mach-imx/mm-imx5.c b/arch/arm/mach-imx/mm-imx5.c index d6b7e9f..bb38747 100644 --- a/arch/arm/mach-imx/mm-imx5.c +++ b/arch/arm/mach-imx/mm-imx5.c @@ -15,7 +15,6 @@ #include <linux/init.h> #include <linux/clk.h> -#include <asm/system_misc.h> #include <asm/mach/map.h> #include <mach/hardware.h> @@ -23,23 +22,6 @@ #include <mach/devices-common.h> #include <mach/iomux-v3.h> -static struct clk *gpc_dvfs_clk;
-static void imx5_idle(void) -{
- /* 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;- }
- clk_enable(gpc_dvfs_clk);
- mx5_cpu_lp_set(WAIT_UNCLOCKED_POWER_OFF);
- if (!tzic_enable_wake())
cpu_do_idle();- clk_disable(gpc_dvfs_clk);
-}
/*
- Define the MX50 memory map.
*/ @@ -103,7 +85,6 @@ 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;
} void __init imx53_init_early(void) @@ -238,4 +219,5 @@ void __init imx53_soc_init(void) void __init imx51_init_late(void) { mx51_neon_fixup();
- imx5_pm_init();
} diff --git a/arch/arm/mach-imx/pm-imx5.c b/arch/arm/mach-imx/pm-imx5.c index e26a9cb..6e62d79 100644 --- a/arch/arm/mach-imx/pm-imx5.c +++ b/arch/arm/mach-imx/pm-imx5.c @@ -13,18 +13,27 @@ #include <linux/io.h> #include <linux/err.h> #include <asm/cacheflush.h> +#include <asm/system_misc.h> #include <asm/tlbflush.h> #include <mach/common.h> #include <mach/hardware.h> #include "crm-regs-imx5.h" -static struct clk *gpc_dvfs_clk; +/*
- The WAIT_UNCLOCKED_POWER_OFF state only requires <= 500ns to exit.
- This is also the lowest power state possible without affecting
- non-cpu parts of the system. For these reasons, imx5 should default
- to always using this state for cpu idling. The PM_SUSPEND_STANDBY also
- uses this state and needs to take no action when registers remain confgiured
- for this state.
- */
+#define IMX5_DEFAULT_CPU_IDLE_STATE WAIT_UNCLOCKED_POWER_OFF /*
- set cpu low power mode before WFI instruction. This function is called
- mx5 because it can be used for mx50, mx51, and mx53.
*/ -void mx5_cpu_lp_set(enum mxc_cpu_pwr_mode mode) +static void mx5_cpu_lp_set(enum mxc_cpu_pwr_mode mode) { u32 plat_lpc, arm_srpgcr, ccm_clpcr; u32 empgc0, empgc1; @@ -87,11 +96,6 @@ void mx5_cpu_lp_set(enum mxc_cpu_pwr_mode mode) } } -static int mx5_suspend_prepare(void) -{
- return clk_prepare_enable(gpc_dvfs_clk);
-}
static int mx5_suspend_enter(suspend_state_t state) { switch (state) { @@ -99,7 +103,7 @@ static int mx5_suspend_enter(suspend_state_t state) mx5_cpu_lp_set(STOP_POWER_OFF); break; case PM_SUSPEND_STANDBY:
mx5_cpu_lp_set(WAIT_UNCLOCKED_POWER_OFF);
/* DEFAULT_IDLE_STATE already configured */@@ -114,12 +118,10 @@ static int mx5_suspend_enter(suspend_state_t state) __raw_writel(0, MXC_SRPG_EMPGC1_SRPGCR); } cpu_do_idle();
- return 0;
-} -static void mx5_suspend_finish(void) -{
- clk_disable_unprepare(gpc_dvfs_clk);
- /* return registers to default idle state */
- mx5_cpu_lp_set(IMX5_DEFAULT_CPU_IDLE_STATE);
- return 0;
} static int mx5_pm_valid(suspend_state_t state) @@ -129,25 +131,34 @@ static int mx5_pm_valid(suspend_state_t state) static const struct platform_suspend_ops mx5_suspend_ops = { .valid = mx5_pm_valid,
- .prepare = mx5_suspend_prepare, .enter = mx5_suspend_enter,
- .finish = mx5_suspend_finish,
}; -static int __init mx5_pm_init(void) +static void imx5_pm_idle(void) +{
- if (likely(!tzic_enable_wake()))
cpu_do_idle();+}
+int __init imx5_pm_init(void) {
- if (!cpu_is_mx51() && !cpu_is_mx53())
return 0;
- int ret;
- struct clk *gpc_dvfs_clk = clk_get(NULL, "gpc_dvfs");
- if (IS_ERR(gpc_dvfs_clk))
return (int)gpc_dvfs_clk;
PTR_ERR please
- ret = clk_enable(gpc_dvfs_clk);
clk_prepare_enable
- if (ret)
return ret;- if (cpu_is_mx51())
suspend_set_ops(&mx5_suspend_ops);
Now this is called only in i.MX51 context, so you can skip the cpu_is
- if (gpc_dvfs_clk == NULL)
gpc_dvfs_clk = clk_get(NULL, "gpc_dvfs");
- arm_pm_idle = imx5_pm_idle;
- if (!IS_ERR(gpc_dvfs_clk)) {
if (cpu_is_mx51())suspend_set_ops(&mx5_suspend_ops);- } else
return -EPERM;
- /* Set the registers to the default cpu idle state. */
- mx5_cpu_lp_set(IMX5_DEFAULT_CPU_IDLE_STATE);
return 0; } -device_initcall(mx5_pm_init); diff --git a/arch/arm/plat-mxc/include/mach/common.h b/arch/arm/plat-mxc/include/mach/common.h index cf663d8..5660e1e 100644 --- a/arch/arm/plat-mxc/include/mach/common.h +++ b/arch/arm/plat-mxc/include/mach/common.h @@ -95,7 +95,6 @@ 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 void imx_print_silicon_rev(const char *cpu, int srev); void avic_handle_irq(struct pt_regs *); @@ -146,8 +145,10 @@ extern void imx6q_clock_map_io(void); #ifdef CONFIG_PM extern void imx6q_pm_init(void); +extern int imx5_pm_init(void); #else static inline void imx6q_pm_init(void) {} +static inline int imx5_pm_init(void) { return -ENODEV; }
Why not make it return void like imx6? We do not check the return value anyway.
Sascha
On Wed, May 16, 2012 at 12:38 PM, Sascha Hauer s.hauer@pengutronix.de wrote:
Hi Robert,
Overall this looks ok now, some comments inline.
Sascha
On Tue, May 15, 2012 at 09:33:32PM -0500, Robert Lee wrote:
The imx5 idle code that existed in mm-imx5.c is moved to pm-imx5.c. The imx5_pm_init call is now exported and called during the MACHINE_START late_init in supported imx5 platforms.
Remove various enabling/disabling of the gpc_dvfs clock and enable it once during initialization. This is a very low power clock that must be enabled during low power operations.
There are only two "suspend_state_t" imx5 low power modes ever used. STOP_POWER_OFF for suspend to mem and WAIT_UNCLOCKED_POWER_OFF for idle and suspend to standby. The latter mode only requires 500 nanoseconds of extra hardware exit time beyond a basic WFI operation (WAIT_CLOCKED mode) so no other idle mode is necessary. Given this information, it is more efficient to keep the registers in the often used WAIT_UNCLOCKED_POWER_OFF state and only to and from the STOP_POWER_OFF register state as needed when suspend to mem is required.
Signed-off-by: Robert Lee rob.lee@linaro.org
arch/arm/mach-imx/mm-imx5.c | 20 +--------- arch/arm/mach-imx/pm-imx5.c | 63 ++++++++++++++++++------------- arch/arm/plat-mxc/include/mach/common.h | 3 +- 3 files changed, 40 insertions(+), 46 deletions(-)
diff --git a/arch/arm/mach-imx/mm-imx5.c b/arch/arm/mach-imx/mm-imx5.c index d6b7e9f..bb38747 100644 --- a/arch/arm/mach-imx/mm-imx5.c +++ b/arch/arm/mach-imx/mm-imx5.c @@ -15,7 +15,6 @@ #include <linux/init.h> #include <linux/clk.h>
-#include <asm/system_misc.h> #include <asm/mach/map.h>
#include <mach/hardware.h> @@ -23,23 +22,6 @@ #include <mach/devices-common.h> #include <mach/iomux-v3.h>
-static struct clk *gpc_dvfs_clk;
-static void imx5_idle(void) -{
- /* 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;
- }
- clk_enable(gpc_dvfs_clk);
- mx5_cpu_lp_set(WAIT_UNCLOCKED_POWER_OFF);
- if (!tzic_enable_wake())
- cpu_do_idle();
- clk_disable(gpc_dvfs_clk);
-}
/* * Define the MX50 memory map. */ @@ -103,7 +85,6 @@ 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;
}
void __init imx53_init_early(void) @@ -238,4 +219,5 @@ void __init imx53_soc_init(void) void __init imx51_init_late(void) { mx51_neon_fixup();
- imx5_pm_init();
} diff --git a/arch/arm/mach-imx/pm-imx5.c b/arch/arm/mach-imx/pm-imx5.c index e26a9cb..6e62d79 100644 --- a/arch/arm/mach-imx/pm-imx5.c +++ b/arch/arm/mach-imx/pm-imx5.c @@ -13,18 +13,27 @@ #include <linux/io.h> #include <linux/err.h> #include <asm/cacheflush.h> +#include <asm/system_misc.h> #include <asm/tlbflush.h> #include <mach/common.h> #include <mach/hardware.h> #include "crm-regs-imx5.h"
-static struct clk *gpc_dvfs_clk; +/*
- The WAIT_UNCLOCKED_POWER_OFF state only requires <= 500ns to exit.
- This is also the lowest power state possible without affecting
- non-cpu parts of the system. For these reasons, imx5 should default
- to always using this state for cpu idling. The PM_SUSPEND_STANDBY also
- uses this state and needs to take no action when registers remain confgiured
- for this state.
- */
+#define IMX5_DEFAULT_CPU_IDLE_STATE WAIT_UNCLOCKED_POWER_OFF
/* * set cpu low power mode before WFI instruction. This function is called * mx5 because it can be used for mx50, mx51, and mx53. */ -void mx5_cpu_lp_set(enum mxc_cpu_pwr_mode mode) +static void mx5_cpu_lp_set(enum mxc_cpu_pwr_mode mode) { u32 plat_lpc, arm_srpgcr, ccm_clpcr; u32 empgc0, empgc1; @@ -87,11 +96,6 @@ void mx5_cpu_lp_set(enum mxc_cpu_pwr_mode mode) } }
-static int mx5_suspend_prepare(void) -{
- return clk_prepare_enable(gpc_dvfs_clk);
-}
static int mx5_suspend_enter(suspend_state_t state) { switch (state) { @@ -99,7 +103,7 @@ static int mx5_suspend_enter(suspend_state_t state) mx5_cpu_lp_set(STOP_POWER_OFF); break; case PM_SUSPEND_STANDBY:
- mx5_cpu_lp_set(WAIT_UNCLOCKED_POWER_OFF);
- /* DEFAULT_IDLE_STATE already configured */
break; default: return -EINVAL; @@ -114,12 +118,10 @@ static int mx5_suspend_enter(suspend_state_t state) __raw_writel(0, MXC_SRPG_EMPGC1_SRPGCR); } cpu_do_idle();
- return 0;
-}
-static void mx5_suspend_finish(void) -{
- clk_disable_unprepare(gpc_dvfs_clk);
- /* return registers to default idle state */
- mx5_cpu_lp_set(IMX5_DEFAULT_CPU_IDLE_STATE);
- return 0;
}
static int mx5_pm_valid(suspend_state_t state) @@ -129,25 +131,34 @@ static int mx5_pm_valid(suspend_state_t state)
static const struct platform_suspend_ops mx5_suspend_ops = { .valid = mx5_pm_valid,
- .prepare = mx5_suspend_prepare,
.enter = mx5_suspend_enter,
- .finish = mx5_suspend_finish,
};
-static int __init mx5_pm_init(void) +static void imx5_pm_idle(void) +{
- if (likely(!tzic_enable_wake()))
- cpu_do_idle();
+}
+int __init imx5_pm_init(void) {
- if (!cpu_is_mx51() && !cpu_is_mx53())
- return 0;
- int ret;
- struct clk *gpc_dvfs_clk = clk_get(NULL, "gpc_dvfs");
- if (IS_ERR(gpc_dvfs_clk))
- return (int)gpc_dvfs_clk;
PTR_ERR please
Thanks. Will fix in v5.
- ret = clk_enable(gpc_dvfs_clk);
clk_prepare_enable
Thanks. Will fix in v5.
- if (ret)
- return ret;
- if (cpu_is_mx51())
- suspend_set_ops(&mx5_suspend_ops);
Now this is called only in i.MX51 context, so you can skip the cpu_is
After this patch series this is also called in i.MX53 context. I'm not confident about the i.MX53 suspend/resume and would prefer to perform further testing and address issues with it in a separate patch series.
- if (gpc_dvfs_clk == NULL)
- gpc_dvfs_clk = clk_get(NULL, "gpc_dvfs");
- arm_pm_idle = imx5_pm_idle;
- if (!IS_ERR(gpc_dvfs_clk)) {
- if (cpu_is_mx51())
- suspend_set_ops(&mx5_suspend_ops);
- } else
- return -EPERM;
- /* Set the registers to the default cpu idle state. */
- mx5_cpu_lp_set(IMX5_DEFAULT_CPU_IDLE_STATE);
return 0; } -device_initcall(mx5_pm_init); diff --git a/arch/arm/plat-mxc/include/mach/common.h b/arch/arm/plat-mxc/include/mach/common.h index cf663d8..5660e1e 100644 --- a/arch/arm/plat-mxc/include/mach/common.h +++ b/arch/arm/plat-mxc/include/mach/common.h @@ -95,7 +95,6 @@ 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 void imx_print_silicon_rev(const char *cpu, int srev);
void avic_handle_irq(struct pt_regs *); @@ -146,8 +145,10 @@ extern void imx6q_clock_map_io(void);
#ifdef CONFIG_PM extern void imx6q_pm_init(void); +extern int imx5_pm_init(void); #else static inline void imx6q_pm_init(void) {} +static inline int imx5_pm_init(void) { return -ENODEV; }
Why not make it return void like imx6? We do not check the return value anyway.
Will change in v5.
Thanks, Rob
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 Thu, May 17, 2012 at 09:34:26AM -0500, Rob Lee wrote:
On Wed, May 16, 2012 at 12:38 PM, Sascha Hauer s.hauer@pengutronix.de wrote:
Hi Robert,
Overall this looks ok now, some comments inline.
- return ret;
- if (cpu_is_mx51())
- suspend_set_ops(&mx5_suspend_ops);
Now this is called only in i.MX51 context, so you can skip the cpu_is
After this patch series this is also called in i.MX53 context. I'm not confident about the i.MX53 suspend/resume and would prefer to perform further testing and address issues with it in a separate patch series.
In this case there is also the possibility that you add a imx51_pm_init in which you set the suspend_ops and call imx5_pm_init afterwards. The cpu_is_macros a kind of deprecated. Instead we settled on calling functions with the exact SoC name from the (dt-) board files and call the more generic function from the SoC specific ones.
Sascha
Add various functionality needed to enable a imx53 low power idle state. This includes adding the imx53 gpc_dvfs clock and making a common imx5_late_init function and initializing all imx53 MACHINE_STATE late_init calls to imx5_late_init.
Signed-off-by: Robert Lee rob.lee@linaro.org --- arch/arm/mach-imx/clock-mx51-mx53.c | 1 + arch/arm/mach-imx/imx53-dt.c | 1 + arch/arm/mach-imx/mach-mx53_ard.c | 1 + arch/arm/mach-imx/mach-mx53_evk.c | 1 + arch/arm/mach-imx/mach-mx53_loco.c | 1 + arch/arm/mach-imx/mach-mx53_smd.c | 1 + arch/arm/mach-imx/mm-imx5.c | 7 ++++++- arch/arm/plat-mxc/include/mach/common.h | 1 + 8 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/arch/arm/mach-imx/clock-mx51-mx53.c b/arch/arm/mach-imx/clock-mx51-mx53.c index 0847050..decedc6 100644 --- a/arch/arm/mach-imx/clock-mx51-mx53.c +++ b/arch/arm/mach-imx/clock-mx51-mx53.c @@ -1529,6 +1529,7 @@ static struct clk_lookup mx53_lookups[] = { _REGISTER_CLOCK("imx-ssi.1", NULL, ssi2_clk) _REGISTER_CLOCK("imx-ssi.2", NULL, ssi3_clk) _REGISTER_CLOCK("imx-keypad", NULL, dummy_clk) + _REGISTER_CLOCK(NULL, "gpc_dvfs", gpc_dvfs_clk) _REGISTER_CLOCK("pata_imx", NULL, pata_clk) _REGISTER_CLOCK("imx53-ahci.0", "ahci", sata_clk) _REGISTER_CLOCK("imx53-ahci.0", "ahci_phy", ahci_phy_clk) diff --git a/arch/arm/mach-imx/imx53-dt.c b/arch/arm/mach-imx/imx53-dt.c index 4172279..39d2ca7 100644 --- a/arch/arm/mach-imx/imx53-dt.c +++ b/arch/arm/mach-imx/imx53-dt.c @@ -125,6 +125,7 @@ DT_MACHINE_START(IMX53_DT, "Freescale i.MX53 (Device Tree Support)") .handle_irq = imx53_handle_irq, .timer = &imx53_timer, .init_machine = imx53_dt_init, + .init_late = imx5_init_late, .dt_compat = imx53_dt_board_compat, .restart = mxc_restart, MACHINE_END diff --git a/arch/arm/mach-imx/mach-mx53_ard.c b/arch/arm/mach-imx/mach-mx53_ard.c index 0564198..afb3d14 100644 --- a/arch/arm/mach-imx/mach-mx53_ard.c +++ b/arch/arm/mach-imx/mach-mx53_ard.c @@ -266,5 +266,6 @@ MACHINE_START(MX53_ARD, "Freescale MX53 ARD Board") .handle_irq = imx53_handle_irq, .timer = &mx53_ard_timer, .init_machine = mx53_ard_board_init, + .init_late = imx5_init_late, .restart = mxc_restart, MACHINE_END diff --git a/arch/arm/mach-imx/mach-mx53_evk.c b/arch/arm/mach-imx/mach-mx53_evk.c index 5a72188..929969f 100644 --- a/arch/arm/mach-imx/mach-mx53_evk.c +++ b/arch/arm/mach-imx/mach-mx53_evk.c @@ -174,5 +174,6 @@ MACHINE_START(MX53_EVK, "Freescale MX53 EVK Board") .handle_irq = imx53_handle_irq, .timer = &mx53_evk_timer, .init_machine = mx53_evk_board_init, + .init_late = imx5_init_late, .restart = mxc_restart, MACHINE_END diff --git a/arch/arm/mach-imx/mach-mx53_loco.c b/arch/arm/mach-imx/mach-mx53_loco.c index 37f67ca..bc5012b 100644 --- a/arch/arm/mach-imx/mach-mx53_loco.c +++ b/arch/arm/mach-imx/mach-mx53_loco.c @@ -316,5 +316,6 @@ MACHINE_START(MX53_LOCO, "Freescale MX53 LOCO Board") .handle_irq = imx53_handle_irq, .timer = &mx53_loco_timer, .init_machine = mx53_loco_board_init, + .init_late = imx5_init_late, .restart = mxc_restart, MACHINE_END diff --git a/arch/arm/mach-imx/mach-mx53_smd.c b/arch/arm/mach-imx/mach-mx53_smd.c index 8e972c5..568190b 100644 --- a/arch/arm/mach-imx/mach-mx53_smd.c +++ b/arch/arm/mach-imx/mach-mx53_smd.c @@ -163,5 +163,6 @@ MACHINE_START(MX53_SMD, "Freescale MX53 SMD Board") .handle_irq = imx53_handle_irq, .timer = &mx53_smd_timer, .init_machine = mx53_smd_board_init, + .init_late = imx5_init_late, .restart = mxc_restart, MACHINE_END diff --git a/arch/arm/mach-imx/mm-imx5.c b/arch/arm/mach-imx/mm-imx5.c index bb38747..7740739 100644 --- a/arch/arm/mach-imx/mm-imx5.c +++ b/arch/arm/mach-imx/mm-imx5.c @@ -216,8 +216,13 @@ void __init imx53_soc_init(void) ARRAY_SIZE(imx53_audmux_res)); }
+void __init imx5_init_late(void) +{ + imx5_pm_init(); +} + void __init imx51_init_late(void) { mx51_neon_fixup(); - imx5_pm_init(); + imx5_init_late(); } diff --git a/arch/arm/plat-mxc/include/mach/common.h b/arch/arm/plat-mxc/include/mach/common.h index 5660e1e..6d2e910 100644 --- a/arch/arm/plat-mxc/include/mach/common.h +++ b/arch/arm/plat-mxc/include/mach/common.h @@ -54,6 +54,7 @@ extern void imx50_soc_init(void); extern void imx51_soc_init(void); extern void imx53_soc_init(void); extern void imx51_init_late(void); +extern void imx5_init_late(void); extern void epit_timer_init(struct clk *timer_clk, void __iomem *base, int irq); extern void mxc_timer_init(struct clk *timer_clk, void __iomem *, int); extern int mx1_clocks_init(unsigned long fref);
On Tue, May 15, 2012 at 09:33:33PM -0500, Robert Lee wrote:
Add various functionality needed to enable a imx53 low power idle state. This includes adding the imx53 gpc_dvfs clock and making a common imx5_late_init function and initializing all imx53 MACHINE_STATE late_init calls to imx5_late_init.
Signed-off-by: Robert Lee rob.lee@linaro.org
arch/arm/mach-imx/clock-mx51-mx53.c | 1 + arch/arm/mach-imx/imx53-dt.c | 1 + arch/arm/mach-imx/mach-mx53_ard.c | 1 + arch/arm/mach-imx/mach-mx53_evk.c | 1 + arch/arm/mach-imx/mach-mx53_loco.c | 1 + arch/arm/mach-imx/mach-mx53_smd.c | 1 + arch/arm/mach-imx/mm-imx5.c | 7 ++++++- arch/arm/plat-mxc/include/mach/common.h | 1 + 8 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/arch/arm/mach-imx/clock-mx51-mx53.c b/arch/arm/mach-imx/clock-mx51-mx53.c index 0847050..decedc6 100644 --- a/arch/arm/mach-imx/clock-mx51-mx53.c +++ b/arch/arm/mach-imx/clock-mx51-mx53.c @@ -1529,6 +1529,7 @@ static struct clk_lookup mx53_lookups[] = { _REGISTER_CLOCK("imx-ssi.1", NULL, ssi2_clk) _REGISTER_CLOCK("imx-ssi.2", NULL, ssi3_clk) _REGISTER_CLOCK("imx-keypad", NULL, dummy_clk)
- _REGISTER_CLOCK(NULL, "gpc_dvfs", gpc_dvfs_clk)
This has to be rebased against the common clock patches.
.timer = &mx53_smd_timer, .init_machine = mx53_smd_board_init,
- .init_late = imx5_init_late, .restart = mxc_restart,
MACHINE_END diff --git a/arch/arm/mach-imx/mm-imx5.c b/arch/arm/mach-imx/mm-imx5.c index bb38747..7740739 100644 --- a/arch/arm/mach-imx/mm-imx5.c +++ b/arch/arm/mach-imx/mm-imx5.c @@ -216,8 +216,13 @@ void __init imx53_soc_init(void) ARRAY_SIZE(imx53_audmux_res)); } +void __init imx5_init_late(void) +{
- imx5_pm_init();
+}
void __init imx51_init_late(void) { mx51_neon_fixup();
- imx5_pm_init();
- imx5_init_late();
}
Where would you add i.MX53 specific code above? Hint: imx5_init_late is the wrong function name.
Sascha
On Wed, May 16, 2012 at 12:47 PM, Sascha Hauer s.hauer@pengutronix.de wrote:
On Tue, May 15, 2012 at 09:33:33PM -0500, Robert Lee wrote:
Add various functionality needed to enable a imx53 low power idle state. This includes adding the imx53 gpc_dvfs clock and making a common imx5_late_init function and initializing all imx53 MACHINE_STATE late_init calls to imx5_late_init.
Signed-off-by: Robert Lee rob.lee@linaro.org
arch/arm/mach-imx/clock-mx51-mx53.c | 1 + arch/arm/mach-imx/imx53-dt.c | 1 + arch/arm/mach-imx/mach-mx53_ard.c | 1 + arch/arm/mach-imx/mach-mx53_evk.c | 1 + arch/arm/mach-imx/mach-mx53_loco.c | 1 + arch/arm/mach-imx/mach-mx53_smd.c | 1 + arch/arm/mach-imx/mm-imx5.c | 7 ++++++- arch/arm/plat-mxc/include/mach/common.h | 1 + 8 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/arch/arm/mach-imx/clock-mx51-mx53.c b/arch/arm/mach-imx/clock-mx51-mx53.c index 0847050..decedc6 100644 --- a/arch/arm/mach-imx/clock-mx51-mx53.c +++ b/arch/arm/mach-imx/clock-mx51-mx53.c @@ -1529,6 +1529,7 @@ static struct clk_lookup mx53_lookups[] = { _REGISTER_CLOCK("imx-ssi.1", NULL, ssi2_clk) _REGISTER_CLOCK("imx-ssi.2", NULL, ssi3_clk) _REGISTER_CLOCK("imx-keypad", NULL, dummy_clk)
- _REGISTER_CLOCK(NULL, "gpc_dvfs", gpc_dvfs_clk)
This has to be rebased against the common clock patches.
Ok.
.timer = &mx53_smd_timer, .init_machine = mx53_smd_board_init,
- .init_late = imx5_init_late,
.restart = mxc_restart, MACHINE_END diff --git a/arch/arm/mach-imx/mm-imx5.c b/arch/arm/mach-imx/mm-imx5.c index bb38747..7740739 100644 --- a/arch/arm/mach-imx/mm-imx5.c +++ b/arch/arm/mach-imx/mm-imx5.c @@ -216,8 +216,13 @@ void __init imx53_soc_init(void) ARRAY_SIZE(imx53_audmux_res)); }
+void __init imx5_init_late(void) +{
- imx5_pm_init();
+}
void __init imx51_init_late(void) { mx51_neon_fixup();
- imx5_pm_init();
- imx5_init_late();
}
Where would you add i.MX53 specific code above? Hint: imx5_init_late is the wrong function name.
I added imx5_init_late for late_init functionality that is common among all imx5. For example, in the future imx50 may use it as well. But I can remove this and repeat the imx5_pm_init() calls for each platform if you prefer that.
Thanks, Rob
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 Thu, May 17, 2012 at 09:46:21AM -0500, Rob Lee wrote:
On Wed, May 16, 2012 at 12:47 PM, Sascha Hauer s.hauer@pengutronix.de wrote:
+void __init imx5_init_late(void) +{
- imx5_pm_init();
+}
void __init imx51_init_late(void) { mx51_neon_fixup();
- imx5_pm_init();
- imx5_init_late();
}
Where would you add i.MX53 specific code above? Hint: imx5_init_late is the wrong function name.
I added imx5_init_late for late_init functionality that is common among all imx5. For example, in the future imx50 may use it as well. But I can remove this and repeat the imx5_pm_init() calls for each platform if you prefer that.
The point is that the init_late callback should have a imx53_* name, otherwise if you call it imx5_* there is no place to add imx53 only functionality. You can always call imx5 specific things from imx53 context, but not the other way round.
Sascha
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..d4cb511 --- /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: + * + * http://www.opensource.org/licenses/gpl-license.html + * http://www.gnu.org/copyleft/gpl.html + */ + +#include <linux/cpuidle.h> +#include <linux/err.h> +#include <linux/hrtimer.h> +#include <linux/io.h> +#include <linux/kernel.h> +#include <linux/slab.h> + +static struct cpuidle_device __percpu * imx_cpuidle_devices; + +static void __init 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); +} + +int __init imx_cpuidle_init(struct cpuidle_driver *drv) +{ + struct cpuidle_device *dev; + int cpu_id, ret; + + if (drv->state_count > CPUIDLE_STATE_MAX) { + pr_err("%s: state_count exceeds maximum\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, error: %d\n", + __func__, cpu_id, ret); + 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..bc932d1 --- /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: + * + * http://www.opensource.org/licenses/gpl-license.html + * http://www.gnu.org/copyleft/gpl.html + */ + +#include <linux/cpuidle.h> + +#ifdef CONFIG_CPU_IDLE +extern int imx_cpuidle_init(struct cpuidle_driver *drv); +#else +static inline int imx_cpuidle_init(struct cpuidle_driver *drv) +{ + return -ENODEV; +} +#endif
On Tue, May 15, 2012 at 09:33:34PM -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
+#ifdef CONFIG_CPU_IDLE +extern int imx_cpuidle_init(struct cpuidle_driver *drv); +#else +static inline int imx_cpuidle_init(struct cpuidle_driver *drv) +{
- return -ENODEV;
+} +#endif
You should return succesfully here. Think about it, if imx_cpuidle_init fails you basically do nothing except maybe printing an error message which will be irritating when it appears on a kernel with cpuidle disabled.
Sascha
On Wed, May 16, 2012 at 12:55 PM, Sascha Hauer s.hauer@pengutronix.de wrote:
On Tue, May 15, 2012 at 09:33:34PM -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
+#ifdef CONFIG_CPU_IDLE +extern int imx_cpuidle_init(struct cpuidle_driver *drv); +#else +static inline int imx_cpuidle_init(struct cpuidle_driver *drv) +{
- return -ENODEV;
+} +#endif
You should return succesfully here. Think about it, if imx_cpuidle_init fails you basically do nothing except maybe printing an error message which will be irritating when it appears on a kernel with cpuidle disabled.
Oops, yeah, I missed that. Will fix in v5.
Thanks, Rob
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 |
Add cpuidle driver for imx5 platform.
Signed-off-by: Robert Lee rob.lee@linaro.org --- arch/arm/mach-imx/pm-imx5.c | 44 ++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 41 insertions(+), 3 deletions(-)
diff --git a/arch/arm/mach-imx/pm-imx5.c b/arch/arm/mach-imx/pm-imx5.c index 6e62d79..966c71b 100644 --- a/arch/arm/mach-imx/pm-imx5.c +++ b/arch/arm/mach-imx/pm-imx5.c @@ -12,10 +12,12 @@ #include <linux/clk.h> #include <linux/io.h> #include <linux/err.h> +#include <linux/export.h> #include <asm/cacheflush.h> #include <asm/system_misc.h> #include <asm/tlbflush.h> #include <mach/common.h> +#include <mach/cpuidle.h> #include <mach/hardware.h> #include "crm-regs-imx5.h"
@@ -134,12 +136,48 @@ static const struct platform_suspend_ops mx5_suspend_ops = { .enter = mx5_suspend_enter, };
-static void imx5_pm_idle(void) +static inline int imx5_cpu_do_idle(void) { - if (likely(!tzic_enable_wake())) + int ret = tzic_enable_wake(); + + if (likely(!ret)) cpu_do_idle(); + + return ret; }
+static void imx5_pm_idle(void) +{ + imx5_cpu_do_idle(); +} + +static int imx5_cpuidle_enter(struct cpuidle_device *dev, + struct cpuidle_driver *drv, int idx) +{ + int ret; + + ret = imx5_cpu_do_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 = 2, + .target_residency = 1, + .flags = CPUIDLE_FLAG_TIME_VALID, + .name = "IMX5 SRPG", + .desc = "CPU state retained,powered off", + }, + .state_count = 1, +}; + int __init imx5_pm_init(void) { int ret; @@ -160,5 +198,5 @@ int __init imx5_pm_init(void) /* Set the registers to the default cpu idle state. */ mx5_cpu_lp_set(IMX5_DEFAULT_CPU_IDLE_STATE);
- return 0; + return imx_cpuidle_init(&imx5_cpuidle_driver); }
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/mach-imx6q.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)
diff --git a/arch/arm/mach-imx/mach-imx6q.c b/arch/arm/mach-imx/mach-imx6q.c index da6c1d9..4fc1fe7 100644 --- a/arch/arm/mach-imx/mach-imx6q.c +++ b/arch/arm/mach-imx/mach-imx6q.c @@ -10,7 +10,9 @@ * http://www.gnu.org/copyleft/gpl.html */
+#include <linux/cpuidle.h> #include <linux/delay.h> +#include <linux/export.h> #include <linux/init.h> #include <linux/io.h> #include <linux/irq.h> @@ -21,6 +23,7 @@ #include <linux/of_platform.h> #include <linux/phy.h> #include <linux/micrel_phy.h> +#include <asm/cpuidle.h> #include <asm/smp_twd.h> #include <asm/hardware/cache-l2x0.h> #include <asm/hardware/gic.h> @@ -28,8 +31,10 @@ #include <asm/mach/time.h> #include <asm/system_misc.h> #include <mach/common.h> +#include <mach/cpuidle.h> #include <mach/hardware.h>
+ void imx6q_restart(char mode, const char *cmd) { struct device_node *np; @@ -86,6 +91,19 @@ static void __init imx6q_init_machine(void) imx6q_pm_init(); }
+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, +}; + +static void __init imx6q_init_late(void) +{ + imx_cpuidle_init(&imx6q_cpuidle_driver); +} + static void __init imx6q_map_io(void) { imx_lluart_map_io(); @@ -142,6 +160,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
-
Rob Lee -
Robert Lee
-
Sascha Hauer