2011-03-16 20:04:14

by Dinh.Nguyen

[permalink] [raw]
Subject: [PATCHv3 1/3] ARM: mx51: Add entry for gpc_dvfs_clk

From: Dinh Nguyen <[email protected]>

For MX51 SRPG, we need to turn on the GPC clock in order to set the
SRPG registers.

Signed-off-by: Dinh Nguyen <[email protected]>
---
arch/arm/mach-mx5/clock-mx51-mx53.c | 8 ++++++++
1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-mx5/clock-mx51-mx53.c b/arch/arm/mach-mx5/clock-mx51-mx53.c
index 652ace4..18492fa 100644
--- a/arch/arm/mach-mx5/clock-mx51-mx53.c
+++ b/arch/arm/mach-mx5/clock-mx51-mx53.c
@@ -865,6 +865,13 @@ static struct clk aips_tz2_clk = {
.disable = _clk_ccgr_disable_inwait,
};

+static struct clk gpc_dvfs_clk = {
+ .enable_reg = MXC_CCM_CCGR5,
+ .enable_shift = MXC_CCM_CCGRx_CG12_OFFSET,
+ .enable = _clk_ccgr_enable,
+ .disable = _clk_ccgr_disable,
+};
+
static struct clk gpt_32k_clk = {
.id = 0,
.parent = &ckil_clk,
@@ -1448,6 +1455,7 @@ static struct clk_lookup mx51_lookups[] = {
_REGISTER_CLOCK("imx-ipuv3", NULL, ipu_clk)
_REGISTER_CLOCK("imx-ipuv3", "di0", ipu_di0_clk)
_REGISTER_CLOCK("imx-ipuv3", "di1", ipu_di1_clk)
+ _REGISTER_CLOCK(NULL, "gpc_dvfs_clk", gpc_dvfs_clk)
};

static struct clk_lookup mx53_lookups[] = {
--
1.6.0.4


2011-03-16 20:03:19

by Dinh.Nguyen

[permalink] [raw]
Subject: [PATCHv3 2/3] ARM: mx51: Implement code to allow mx51 to enter WFI

From: Dinh Nguyen <[email protected]>

Implement code for MX51 that allows the SoC to enter WFI when
arch_idle is called.

This patch is also necessary for correctly suspending the system.

Signed-off-by: Dinh Nguyen <[email protected]>
---
arch/arm/mach-mx5/Makefile | 2 +-
arch/arm/mach-mx5/system.c | 84 +++++++++++++++++++++++++++++++
arch/arm/plat-mxc/include/mach/mxc.h | 9 +++
arch/arm/plat-mxc/include/mach/system.h | 6 ++-
4 files changed, 99 insertions(+), 2 deletions(-)
create mode 100644 arch/arm/mach-mx5/system.c

diff --git a/arch/arm/mach-mx5/Makefile b/arch/arm/mach-mx5/Makefile
index 4f63048..0b9338c 100644
--- a/arch/arm/mach-mx5/Makefile
+++ b/arch/arm/mach-mx5/Makefile
@@ -3,7 +3,7 @@
#

# Object file lists.
-obj-y := cpu.o mm.o clock-mx51-mx53.o devices.o ehci.o
+obj-y := cpu.o mm.o clock-mx51-mx53.o devices.o ehci.o system.o
obj-$(CONFIG_SOC_IMX50) += mm-mx50.o

obj-$(CONFIG_CPU_FREQ_IMX) += cpu_op-mx51.o
diff --git a/arch/arm/mach-mx5/system.c b/arch/arm/mach-mx5/system.c
new file mode 100644
index 0000000..c4e9e00
--- /dev/null
+++ b/arch/arm/mach-mx5/system.c
@@ -0,0 +1,84 @@
+/*
+ * Copyright (C) 2011 Freescale Semiconductor, Inc. All Rights Reserved.
+ */
+
+/*
+ * 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/platform_device.h>
+#include <asm/io.h>
+#include <mach/hardware.h>
+#include "crm_regs.h"
+
+/* 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)
+{
+ u32 plat_lpc, arm_srpgcr, ccm_clpcr;
+ u32 empgc0, empgc1;
+ int stop_mode = 0;
+
+ /* always allow platform to issue a deep sleep mode request */
+ 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);
+
+ switch (mode) {
+ case WAIT_CLOCKED:
+ break;
+ case WAIT_UNCLOCKED:
+ ccm_clpcr |= 0x1 << MXC_CCM_CLPCR_LPM_OFFSET;
+ break;
+ case WAIT_UNCLOCKED_POWER_OFF:
+ case STOP_POWER_OFF:
+ plat_lpc |= MXC_CORTEXA8_PLAT_LPC_DSM
+ | MXC_CORTEXA8_PLAT_LPC_DBG_DSM;
+ if (mode == WAIT_UNCLOCKED_POWER_OFF) {
+ ccm_clpcr |= 0x1 << MXC_CCM_CLPCR_LPM_OFFSET;
+ ccm_clpcr &= ~MXC_CCM_CLPCR_VSTBY;
+ ccm_clpcr &= ~MXC_CCM_CLPCR_SBYOS;
+ stop_mode = 0;
+ } else {
+ ccm_clpcr |= 0x2 << MXC_CCM_CLPCR_LPM_OFFSET;
+ ccm_clpcr |= 0x3 << MXC_CCM_CLPCR_STBY_COUNT_OFFSET;
+ ccm_clpcr |= MXC_CCM_CLPCR_VSTBY;
+ ccm_clpcr |= MXC_CCM_CLPCR_SBYOS;
+ stop_mode = 1;
+ }
+
+ arm_srpgcr |= MXC_SRPGCR_PCR;
+ if (stop_mode) {
+ empgc0 |= MXC_SRPGCR_PCR;
+ empgc1 |= MXC_SRPGCR_PCR;
+ }
+
+ if (tzic_enable_wake(1) != 0)
+ return;
+ break;
+ case STOP_POWER_ON:
+ ccm_clpcr |= 0x2 << MXC_CCM_CLPCR_LPM_OFFSET;
+ break;
+ default:
+ printk(KERN_WARNING "UNKNOWN cpu power mode: %d\n", mode);
+ return;
+ }
+
+ __raw_writel(plat_lpc, MXC_CORTEXA8_PLAT_LPC);
+ __raw_writel(ccm_clpcr, MXC_CCM_CLPCR);
+ __raw_writel(arm_srpgcr, MXC_SRPG_ARM_SRPGCR);
+ /* Enable NEON SRPG for all but MX50TO1.0. */
+ __raw_writel(arm_srpgcr, MXC_SRPG_NEON_SRPGCR);
+ if (stop_mode) {
+ __raw_writel(empgc0, MXC_SRPG_EMPGC0_SRPGCR);
+ __raw_writel(empgc1, MXC_SRPG_EMPGC1_SRPGCR);
+ }
+}
+
diff --git a/arch/arm/plat-mxc/include/mach/mxc.h b/arch/arm/plat-mxc/include/mach/mxc.h
index 7e07263..6c2a371 100644
--- a/arch/arm/plat-mxc/include/mach/mxc.h
+++ b/arch/arm/plat-mxc/include/mach/mxc.h
@@ -181,6 +181,15 @@ struct cpu_op {
u32 cpu_rate;
};

+int tzic_enable_wake(int is_idle);
+enum mxc_cpu_pwr_mode {
+ WAIT_CLOCKED, /* wfi only */
+ WAIT_UNCLOCKED, /* WAIT */
+ WAIT_UNCLOCKED_POWER_OFF, /* WAIT + SRPG */
+ STOP_POWER_ON, /* just STOP */
+ STOP_POWER_OFF, /* STOP + SRPG */
+};
+
extern struct cpu_op *(*get_cpu_op)(int *op);
#endif

diff --git a/arch/arm/plat-mxc/include/mach/system.h b/arch/arm/plat-mxc/include/mach/system.h
index 95be51b..0417da9 100644
--- a/arch/arm/plat-mxc/include/mach/system.h
+++ b/arch/arm/plat-mxc/include/mach/system.h
@@ -20,6 +20,8 @@
#include <mach/hardware.h>
#include <mach/common.h>

+extern void mx5_cpu_lp_set(enum mxc_cpu_pwr_mode mode);
+
static inline void arch_idle(void)
{
#ifdef CONFIG_ARCH_MXC91231
@@ -54,7 +56,9 @@ static inline void arch_idle(void)
"orr %0, %0, #0x00000004\n"
"mcr p15, 0, %0, c1, c0, 0\n"
: "=r" (reg));
- } else
+ } else if (cpu_is_mx51())
+ mx5_cpu_lp_set(WAIT_UNCLOCKED_POWER_OFF);
+ else
cpu_do_idle();
}

--
1.6.0.4

2011-03-16 20:03:31

by Dinh.Nguyen

[permalink] [raw]
Subject: [PATCHv3 3/3] ARM: mx51: Add support for low power suspend on MX51

From: Dinh Nguyen <[email protected]>

Adds initial low power suspend functionality to MX51.
Supports "mem" and "standby" modes.

Tested on mx51-babbage.

Signed-off-by: Dinh Nguyen <[email protected]>
---
arch/arm/mach-mx5/Makefile | 1 +
arch/arm/mach-mx5/pm-imx51.c | 69 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 70 insertions(+), 0 deletions(-)
create mode 100644 arch/arm/mach-mx5/pm-imx51.c

diff --git a/arch/arm/mach-mx5/Makefile b/arch/arm/mach-mx5/Makefile
index 0b9338c..787cb80 100644
--- a/arch/arm/mach-mx5/Makefile
+++ b/arch/arm/mach-mx5/Makefile
@@ -6,6 +6,7 @@
obj-y := cpu.o mm.o clock-mx51-mx53.o devices.o ehci.o system.o
obj-$(CONFIG_SOC_IMX50) += mm-mx50.o

+obj-$(CONFIG_PM) += pm-imx51.o
obj-$(CONFIG_CPU_FREQ_IMX) += cpu_op-mx51.o
obj-$(CONFIG_MACH_MX51_BABBAGE) += board-mx51_babbage.o
obj-$(CONFIG_MACH_MX51_3DS) += board-mx51_3ds.o
diff --git a/arch/arm/mach-mx5/pm-imx51.c b/arch/arm/mach-mx5/pm-imx51.c
new file mode 100644
index 0000000..4a1d915
--- /dev/null
+++ b/arch/arm/mach-mx5/pm-imx51.c
@@ -0,0 +1,69 @@
+/*
+ * Copyright (C) 2011 Freescale Semiconductor, Inc. All Rights Reserved.
+ *
+ * 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/suspend.h>
+#include <linux/clk.h>
+#include <asm/mach/map.h>
+#include <asm/cacheflush.h>
+#include <asm/tlb.h>
+#include <mach/system.h>
+#include "crm_regs.h"
+
+struct clk *gpc_dvfs_clk;
+
+static int mx5_suspend_enter(suspend_state_t state)
+{
+ if (gpc_dvfs_clk == NULL)
+ gpc_dvfs_clk = clk_get(NULL, "gpc_dvfs_clk");
+
+ clk_enable(gpc_dvfs_clk);
+ switch (state) {
+ case PM_SUSPEND_MEM:
+ mx5_cpu_lp_set(STOP_POWER_OFF);
+ break;
+ case PM_SUSPEND_STANDBY:
+ mx5_cpu_lp_set(WAIT_UNCLOCKED_POWER_OFF);
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ if (state == PM_SUSPEND_MEM) {
+ local_flush_tlb_all();
+ flush_cache_all();
+
+ /*clear the EMPGC0/1 bits */
+ __raw_writel(0, MXC_SRPG_EMPGC0_SRPGCR);
+ __raw_writel(0, MXC_SRPG_EMPGC1_SRPGCR);
+ }
+ cpu_do_idle();
+ clk_disable(gpc_dvfs_clk);
+
+ return 0;
+}
+
+static int mx5_pm_valid(suspend_state_t state)
+{
+ return (state > PM_SUSPEND_ON && state <= PM_SUSPEND_MAX);
+}
+
+static const struct platform_suspend_ops mx5_suspend_ops = {
+ .valid = mx5_pm_valid,
+ .enter = mx5_suspend_enter,
+};
+
+static int __init mx5_pm_init(void)
+{
+ if (cpu_is_mx51())
+ suspend_set_ops(&mx5_suspend_ops);
+
+ return 0;
+}
+device_initcall(mx5_pm_init);
--
1.6.0.4

2011-03-17 08:00:37

by Sascha Hauer

[permalink] [raw]
Subject: Re: [PATCHv3 1/3] ARM: mx51: Add entry for gpc_dvfs_clk

On Wed, Mar 16, 2011 at 03:03:05PM -0500, [email protected] wrote:
> From: Dinh Nguyen <[email protected]>
>
> For MX51 SRPG, we need to turn on the GPC clock in order to set the
> SRPG registers.
>
> Signed-off-by: Dinh Nguyen <[email protected]>
> ---
> arch/arm/mach-mx5/clock-mx51-mx53.c | 8 ++++++++
> 1 files changed, 8 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/mach-mx5/clock-mx51-mx53.c b/arch/arm/mach-mx5/clock-mx51-mx53.c
> index 652ace4..18492fa 100644
> --- a/arch/arm/mach-mx5/clock-mx51-mx53.c
> +++ b/arch/arm/mach-mx5/clock-mx51-mx53.c
> @@ -865,6 +865,13 @@ static struct clk aips_tz2_clk = {
> .disable = _clk_ccgr_disable_inwait,
> };
>
> +static struct clk gpc_dvfs_clk = {
> + .enable_reg = MXC_CCM_CCGR5,
> + .enable_shift = MXC_CCM_CCGRx_CG12_OFFSET,
> + .enable = _clk_ccgr_enable,
> + .disable = _clk_ccgr_disable,
> +};
> +
> static struct clk gpt_32k_clk = {
> .id = 0,
> .parent = &ckil_clk,
> @@ -1448,6 +1455,7 @@ static struct clk_lookup mx51_lookups[] = {
> _REGISTER_CLOCK("imx-ipuv3", NULL, ipu_clk)
> _REGISTER_CLOCK("imx-ipuv3", "di0", ipu_di0_clk)
> _REGISTER_CLOCK("imx-ipuv3", "di1", ipu_di1_clk)
> + _REGISTER_CLOCK(NULL, "gpc_dvfs_clk", gpc_dvfs_clk)

Please remove the _clk. The context makes it clear already that it's a
clock.

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 |

2011-03-17 08:18:37

by Sascha Hauer

[permalink] [raw]
Subject: Re: [PATCHv3 2/3] ARM: mx51: Implement code to allow mx51 to enter WFI

On Wed, Mar 16, 2011 at 03:03:06PM -0500, [email protected] wrote:
> From: Dinh Nguyen <[email protected]>
>
> Implement code for MX51 that allows the SoC to enter WFI when
> arch_idle is called.
>
> This patch is also necessary for correctly suspending the system.
>
> Signed-off-by: Dinh Nguyen <[email protected]>
> ---
> arch/arm/mach-mx5/Makefile | 2 +-
> arch/arm/mach-mx5/system.c | 84 +++++++++++++++++++++++++++++++
> arch/arm/plat-mxc/include/mach/mxc.h | 9 +++
> arch/arm/plat-mxc/include/mach/system.h | 6 ++-
> 4 files changed, 99 insertions(+), 2 deletions(-)
> create mode 100644 arch/arm/mach-mx5/system.c
>
> diff --git a/arch/arm/mach-mx5/Makefile b/arch/arm/mach-mx5/Makefile
> index 4f63048..0b9338c 100644
> --- a/arch/arm/mach-mx5/Makefile
> +++ b/arch/arm/mach-mx5/Makefile
> @@ -3,7 +3,7 @@
> #
>
> # Object file lists.
> -obj-y := cpu.o mm.o clock-mx51-mx53.o devices.o ehci.o
> +obj-y := cpu.o mm.o clock-mx51-mx53.o devices.o ehci.o system.o
> obj-$(CONFIG_SOC_IMX50) += mm-mx50.o
>
> obj-$(CONFIG_CPU_FREQ_IMX) += cpu_op-mx51.o
> diff --git a/arch/arm/mach-mx5/system.c b/arch/arm/mach-mx5/system.c
> new file mode 100644
> index 0000000..c4e9e00
> --- /dev/null
> +++ b/arch/arm/mach-mx5/system.c
> @@ -0,0 +1,84 @@
> +/*
> + * Copyright (C) 2011 Freescale Semiconductor, Inc. All Rights Reserved.
> + */
> +
> +/*
> + * 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/platform_device.h>
> +#include <asm/io.h>
> +#include <mach/hardware.h>
> +#include "crm_regs.h"
> +
> +/* 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)
> +{
> + u32 plat_lpc, arm_srpgcr, ccm_clpcr;
> + u32 empgc0, empgc1;
> + int stop_mode = 0;
> +
> + /* always allow platform to issue a deep sleep mode request */
> + 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);

You read empgc0/1 and mask the MXC_SRPGCR_PCR bit...

> +
> + switch (mode) {
> + case WAIT_CLOCKED:
> + break;
> + case WAIT_UNCLOCKED:
> + ccm_clpcr |= 0x1 << MXC_CCM_CLPCR_LPM_OFFSET;
> + break;
> + case WAIT_UNCLOCKED_POWER_OFF:
> + case STOP_POWER_OFF:
> + plat_lpc |= MXC_CORTEXA8_PLAT_LPC_DSM
> + | MXC_CORTEXA8_PLAT_LPC_DBG_DSM;
> + if (mode == WAIT_UNCLOCKED_POWER_OFF) {
> + ccm_clpcr |= 0x1 << MXC_CCM_CLPCR_LPM_OFFSET;
> + ccm_clpcr &= ~MXC_CCM_CLPCR_VSTBY;
> + ccm_clpcr &= ~MXC_CCM_CLPCR_SBYOS;
> + stop_mode = 0;
> + } else {
> + ccm_clpcr |= 0x2 << MXC_CCM_CLPCR_LPM_OFFSET;
> + ccm_clpcr |= 0x3 << MXC_CCM_CLPCR_STBY_COUNT_OFFSET;
> + ccm_clpcr |= MXC_CCM_CLPCR_VSTBY;
> + ccm_clpcr |= MXC_CCM_CLPCR_SBYOS;
> + stop_mode = 1;
> + }
> +
> + arm_srpgcr |= MXC_SRPGCR_PCR;
> + if (stop_mode) {
> + empgc0 |= MXC_SRPGCR_PCR;
> + empgc1 |= MXC_SRPGCR_PCR;

... Then in stop_mode you set the bit ...

> + }
> +
> + if (tzic_enable_wake(1) != 0)
> + return;
> + break;
> + case STOP_POWER_ON:
> + ccm_clpcr |= 0x2 << MXC_CCM_CLPCR_LPM_OFFSET;
> + break;
> + default:
> + printk(KERN_WARNING "UNKNOWN cpu power mode: %d\n", mode);
> + return;
> + }
> +
> + __raw_writel(plat_lpc, MXC_CORTEXA8_PLAT_LPC);
> + __raw_writel(ccm_clpcr, MXC_CCM_CLPCR);
> + __raw_writel(arm_srpgcr, MXC_SRPG_ARM_SRPGCR);
> + /* Enable NEON SRPG for all but MX50TO1.0. */
> + __raw_writel(arm_srpgcr, MXC_SRPG_NEON_SRPGCR);
> + if (stop_mode) {
> + __raw_writel(empgc0, MXC_SRPG_EMPGC0_SRPGCR);
> + __raw_writel(empgc1, MXC_SRPG_EMPGC1_SRPGCR);

... and only in stop mode you write the value back. Can't we do the
empgc0 handling here completely to make the code more clean?

Also, I'm missing the implementation of 'all but MX50TO1.0'.

> + }
> +}
> +
> diff --git a/arch/arm/plat-mxc/include/mach/mxc.h b/arch/arm/plat-mxc/include/mach/mxc.h
> index 7e07263..6c2a371 100644
> --- a/arch/arm/plat-mxc/include/mach/mxc.h
> +++ b/arch/arm/plat-mxc/include/mach/mxc.h
> @@ -181,6 +181,15 @@ struct cpu_op {
> u32 cpu_rate;
> };
>
> +int tzic_enable_wake(int is_idle);
> +enum mxc_cpu_pwr_mode {
> + WAIT_CLOCKED, /* wfi only */
> + WAIT_UNCLOCKED, /* WAIT */
> + WAIT_UNCLOCKED_POWER_OFF, /* WAIT + SRPG */
> + STOP_POWER_ON, /* just STOP */
> + STOP_POWER_OFF, /* STOP + SRPG */
> +};
> +
> extern struct cpu_op *(*get_cpu_op)(int *op);
> #endif
>
> diff --git a/arch/arm/plat-mxc/include/mach/system.h b/arch/arm/plat-mxc/include/mach/system.h
> index 95be51b..0417da9 100644
> --- a/arch/arm/plat-mxc/include/mach/system.h
> +++ b/arch/arm/plat-mxc/include/mach/system.h
> @@ -20,6 +20,8 @@
> #include <mach/hardware.h>
> #include <mach/common.h>
>
> +extern void mx5_cpu_lp_set(enum mxc_cpu_pwr_mode mode);
> +
> static inline void arch_idle(void)
> {
> #ifdef CONFIG_ARCH_MXC91231
> @@ -54,7 +56,9 @@ static inline void arch_idle(void)
> "orr %0, %0, #0x00000004\n"
> "mcr p15, 0, %0, c1, c0, 0\n"
> : "=r" (reg));
> - } else
> + } else if (cpu_is_mx51())
> + mx5_cpu_lp_set(WAIT_UNCLOCKED_POWER_OFF);
> + else

Have you tried compiling this on !i.MX5 systems?

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 |

2011-03-17 08:21:09

by Sascha Hauer

[permalink] [raw]
Subject: Re: [PATCHv3 3/3] ARM: mx51: Add support for low power suspend on MX51

On Wed, Mar 16, 2011 at 03:03:07PM -0500, [email protected] wrote:
> +
> +struct clk *gpc_dvfs_clk;
> +
> +static int mx5_suspend_enter(suspend_state_t state)
> +{
> + if (gpc_dvfs_clk == NULL)
> + gpc_dvfs_clk = clk_get(NULL, "gpc_dvfs_clk");

clk_get can fail and NULL can be a valid clock.

Sascha

> +
> + clk_enable(gpc_dvfs_clk);
> + switch (state) {
> + case PM_SUSPEND_MEM:
> + mx5_cpu_lp_set(STOP_POWER_OFF);
> + break;
> + case PM_SUSPEND_STANDBY:
> + mx5_cpu_lp_set(WAIT_UNCLOCKED_POWER_OFF);
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + if (state == PM_SUSPEND_MEM) {
> + local_flush_tlb_all();
> + flush_cache_all();
> +
> + /*clear the EMPGC0/1 bits */
> + __raw_writel(0, MXC_SRPG_EMPGC0_SRPGCR);
> + __raw_writel(0, MXC_SRPG_EMPGC1_SRPGCR);
> + }
> + cpu_do_idle();
> + clk_disable(gpc_dvfs_clk);
> +
> + return 0;
> +}
> +
> +static int mx5_pm_valid(suspend_state_t state)
> +{
> + return (state > PM_SUSPEND_ON && state <= PM_SUSPEND_MAX);
> +}
> +
> +static const struct platform_suspend_ops mx5_suspend_ops = {
> + .valid = mx5_pm_valid,
> + .enter = mx5_suspend_enter,
> +};
> +
> +static int __init mx5_pm_init(void)
> +{
> + if (cpu_is_mx51())
> + suspend_set_ops(&mx5_suspend_ops);
> +
> + return 0;
> +}
> +device_initcall(mx5_pm_init);
> --
> 1.6.0.4
>
>
>

--
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 |

2011-03-17 08:29:29

by Arnaud Patard

[permalink] [raw]
Subject: Re: [PATCHv3 3/3] ARM: mx51: Add support for low power suspend on MX51

Sascha Hauer <[email protected]> writes:

Hi,

> On Wed, Mar 16, 2011 at 03:03:07PM -0500, [email protected] wrote:
>> +
>> +struct clk *gpc_dvfs_clk;
>> +
>> +static int mx5_suspend_enter(suspend_state_t state)
>> +{
>> + if (gpc_dvfs_clk == NULL)
>> + gpc_dvfs_clk = clk_get(NULL, "gpc_dvfs_clk");
>
> clk_get can fail and NULL can be a valid clock.

So, I guess one may want to fail here if one can't get the clock. The
suspend will fail in hard to debug way if this clock is not enabled
at this point.

Arnaud

2011-03-17 10:21:59

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: [PATCHv3 2/3] ARM: mx51: Implement code to allow mx51 to enter WFI

On Wed, Mar 16, 2011 at 03:03:06PM -0500, [email protected] wrote:
> +#include <linux/platform_device.h>
> +#include <asm/io.h>

linux/io.h please.

2011-03-17 10:31:40

by Lothar Waßmann

[permalink] [raw]
Subject: Re: [PATCHv3 3/3] ARM: mx51: Add support for low power suspend on MX51

Hi,

[email protected] writes:
> From: Dinh Nguyen <[email protected]>
>
> Adds initial low power suspend functionality to MX51.
> Supports "mem" and "standby" modes.
>
> Tested on mx51-babbage.
>
> Signed-off-by: Dinh Nguyen <[email protected]>
> ---
> arch/arm/mach-mx5/Makefile | 1 +
> arch/arm/mach-mx5/pm-imx51.c | 69 ++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 70 insertions(+), 0 deletions(-)
> create mode 100644 arch/arm/mach-mx5/pm-imx51.c
>
> diff --git a/arch/arm/mach-mx5/Makefile b/arch/arm/mach-mx5/Makefile
> index 0b9338c..787cb80 100644
> --- a/arch/arm/mach-mx5/Makefile
> +++ b/arch/arm/mach-mx5/Makefile
> @@ -6,6 +6,7 @@
> obj-y := cpu.o mm.o clock-mx51-mx53.o devices.o ehci.o system.o
> obj-$(CONFIG_SOC_IMX50) += mm-mx50.o
>
> +obj-$(CONFIG_PM) += pm-imx51.o
> obj-$(CONFIG_CPU_FREQ_IMX) += cpu_op-mx51.o
> obj-$(CONFIG_MACH_MX51_BABBAGE) += board-mx51_babbage.o
> obj-$(CONFIG_MACH_MX51_3DS) += board-mx51_3ds.o
> diff --git a/arch/arm/mach-mx5/pm-imx51.c b/arch/arm/mach-mx5/pm-imx51.c
> new file mode 100644
> index 0000000..4a1d915
> --- /dev/null
> +++ b/arch/arm/mach-mx5/pm-imx51.c
> @@ -0,0 +1,69 @@
> +/*
> + * Copyright (C) 2011 Freescale Semiconductor, Inc. All Rights Reserved.
> + *
> + * 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/suspend.h>
> +#include <linux/clk.h>
> +#include <asm/mach/map.h>
> +#include <asm/cacheflush.h>
> +#include <asm/tlb.h>
> +#include <mach/system.h>
> +#include "crm_regs.h"
> +
> +struct clk *gpc_dvfs_clk;
> +
static?

> +static int mx5_suspend_enter(suspend_state_t state)
> +{
> + if (gpc_dvfs_clk == NULL)
> + gpc_dvfs_clk = clk_get(NULL, "gpc_dvfs_clk");
> +
> + clk_enable(gpc_dvfs_clk);
>
error check?


Lothar Waßmann
--
___________________________________________________________

Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996

http://www.karo-electronics.de | [email protected]
___________________________________________________________

2011-03-17 16:52:51

by Nguyen Dinh-R00091

[permalink] [raw]
Subject: RE: [PATCHv3 2/3] ARM: mx51: Implement code to allow mx51 to enter WFI

Hi Sascha,


>-----Original Message-----
>From: Sascha Hauer [mailto:[email protected]]
>Sent: Thursday, March 17, 2011 3:19 AM
>To: Nguyen Dinh-R00091
>Cc: [email protected]; [email protected]; [email protected];
>[email protected]; [email protected]; Vaidyanathan Ranjani-RA5478; Zhang Lily-
>R58066; [email protected]
>Subject: Re: [PATCHv3 2/3] ARM: mx51: Implement code to allow mx51 to enter WFI
>
>On Wed, Mar 16, 2011 at 03:03:06PM -0500, [email protected] wrote:
>> From: Dinh Nguyen <[email protected]>
>>
>> Implement code for MX51 that allows the SoC to enter WFI when
>> arch_idle is called.
>>
>> This patch is also necessary for correctly suspending the system.
>>
>> Signed-off-by: Dinh Nguyen <[email protected]>
>> ---
>> arch/arm/mach-mx5/Makefile | 2 +-
>> arch/arm/mach-mx5/system.c | 84 +++++++++++++++++++++++++++++++
>> arch/arm/plat-mxc/include/mach/mxc.h | 9 +++
>> arch/arm/plat-mxc/include/mach/system.h | 6 ++-
>> 4 files changed, 99 insertions(+), 2 deletions(-)
>> create mode 100644 arch/arm/mach-mx5/system.c
>>
>> diff --git a/arch/arm/mach-mx5/Makefile b/arch/arm/mach-mx5/Makefile
>> index 4f63048..0b9338c 100644
>> --- a/arch/arm/mach-mx5/Makefile
>> +++ b/arch/arm/mach-mx5/Makefile
>> @@ -3,7 +3,7 @@
>> #
>>
>> # Object file lists.
>> -obj-y := cpu.o mm.o clock-mx51-mx53.o devices.o ehci.o
>> +obj-y := cpu.o mm.o clock-mx51-mx53.o devices.o ehci.o system.o
>> obj-$(CONFIG_SOC_IMX50) += mm-mx50.o
>>
>> obj-$(CONFIG_CPU_FREQ_IMX) += cpu_op-mx51.o
>> diff --git a/arch/arm/mach-mx5/system.c b/arch/arm/mach-mx5/system.c
>> new file mode 100644
>> index 0000000..c4e9e00
>> --- /dev/null
>> +++ b/arch/arm/mach-mx5/system.c
>> @@ -0,0 +1,84 @@
>> +/*
>> + * Copyright (C) 2011 Freescale Semiconductor, Inc. All Rights Reserved.
>> + */
>> +
>> +/*
>> + * 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/platform_device.h>
>> +#include <asm/io.h>
>> +#include <mach/hardware.h>
>> +#include "crm_regs.h"
>> +
>> +/* 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)
>> +{
>> + u32 plat_lpc, arm_srpgcr, ccm_clpcr;
>> + u32 empgc0, empgc1;
>> + int stop_mode = 0;
>> +
>> + /* always allow platform to issue a deep sleep mode request */
>> + 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);
>
>You read empgc0/1 and mask the MXC_SRPGCR_PCR bit...
>
>> +
>> + switch (mode) {
>> + case WAIT_CLOCKED:
>> + break;
>> + case WAIT_UNCLOCKED:
>> + ccm_clpcr |= 0x1 << MXC_CCM_CLPCR_LPM_OFFSET;
>> + break;
>> + case WAIT_UNCLOCKED_POWER_OFF:
>> + case STOP_POWER_OFF:
>> + plat_lpc |= MXC_CORTEXA8_PLAT_LPC_DSM
>> + | MXC_CORTEXA8_PLAT_LPC_DBG_DSM;
>> + if (mode == WAIT_UNCLOCKED_POWER_OFF) {
>> + ccm_clpcr |= 0x1 << MXC_CCM_CLPCR_LPM_OFFSET;
>> + ccm_clpcr &= ~MXC_CCM_CLPCR_VSTBY;
>> + ccm_clpcr &= ~MXC_CCM_CLPCR_SBYOS;
>> + stop_mode = 0;
>> + } else {
>> + ccm_clpcr |= 0x2 << MXC_CCM_CLPCR_LPM_OFFSET;
>> + ccm_clpcr |= 0x3 << MXC_CCM_CLPCR_STBY_COUNT_OFFSET;
>> + ccm_clpcr |= MXC_CCM_CLPCR_VSTBY;
>> + ccm_clpcr |= MXC_CCM_CLPCR_SBYOS;
>> + stop_mode = 1;
>> + }
>> +
>> + arm_srpgcr |= MXC_SRPGCR_PCR;
>> + if (stop_mode) {
>> + empgc0 |= MXC_SRPGCR_PCR;
>> + empgc1 |= MXC_SRPGCR_PCR;
>
>... Then in stop_mode you set the bit ...
>
>> + }
>> +
>> + if (tzic_enable_wake(1) != 0)
>> + return;
>> + break;
>> + case STOP_POWER_ON:
>> + ccm_clpcr |= 0x2 << MXC_CCM_CLPCR_LPM_OFFSET;
>> + break;
>> + default:
>> + printk(KERN_WARNING "UNKNOWN cpu power mode: %d\n", mode);
>> + return;
>> + }
>> +
>> + __raw_writel(plat_lpc, MXC_CORTEXA8_PLAT_LPC);
>> + __raw_writel(ccm_clpcr, MXC_CCM_CLPCR);
>> + __raw_writel(arm_srpgcr, MXC_SRPG_ARM_SRPGCR);
>> + /* Enable NEON SRPG for all but MX50TO1.0. */
>> + __raw_writel(arm_srpgcr, MXC_SRPG_NEON_SRPGCR);
>> + if (stop_mode) {
>> + __raw_writel(empgc0, MXC_SRPG_EMPGC0_SRPGCR);
>> + __raw_writel(empgc1, MXC_SRPG_EMPGC1_SRPGCR);
>
>... and only in stop mode you write the value back. Can't we do the
>empgc0 handling here completely to make the code more clean?
>
>Also, I'm missing the implementation of 'all but MX50TO1.0'.
>
>> + }
>> +}
>> +
>> diff --git a/arch/arm/plat-mxc/include/mach/mxc.h b/arch/arm/plat-mxc/include/mach/mxc.h
>> index 7e07263..6c2a371 100644
>> --- a/arch/arm/plat-mxc/include/mach/mxc.h
>> +++ b/arch/arm/plat-mxc/include/mach/mxc.h
>> @@ -181,6 +181,15 @@ struct cpu_op {
>> u32 cpu_rate;
>> };
>>
>> +int tzic_enable_wake(int is_idle);
>> +enum mxc_cpu_pwr_mode {
>> + WAIT_CLOCKED, /* wfi only */
>> + WAIT_UNCLOCKED, /* WAIT */
>> + WAIT_UNCLOCKED_POWER_OFF, /* WAIT + SRPG */
>> + STOP_POWER_ON, /* just STOP */
>> + STOP_POWER_OFF, /* STOP + SRPG */
>> +};
>> +
>> extern struct cpu_op *(*get_cpu_op)(int *op);
>> #endif
>>
>> diff --git a/arch/arm/plat-mxc/include/mach/system.h b/arch/arm/plat-mxc/include/mach/system.h
>> index 95be51b..0417da9 100644
>> --- a/arch/arm/plat-mxc/include/mach/system.h
>> +++ b/arch/arm/plat-mxc/include/mach/system.h
>> @@ -20,6 +20,8 @@
>> #include <mach/hardware.h>
>> #include <mach/common.h>
>>
>> +extern void mx5_cpu_lp_set(enum mxc_cpu_pwr_mode mode);
>> +
>> static inline void arch_idle(void)
>> {
>> #ifdef CONFIG_ARCH_MXC91231
>> @@ -54,7 +56,9 @@ static inline void arch_idle(void)
>> "orr %0, %0, #0x00000004\n"
>> "mcr p15, 0, %0, c1, c0, 0\n"
>> : "=r" (reg));
>> - } else
>> + } else if (cpu_is_mx51())
>> + mx5_cpu_lp_set(WAIT_UNCLOCKED_POWER_OFF);
>> + else
>
>Have you tried compiling this on !i.MX5 systems?

Yes, I have and its build fine for mx3_defconfig. I did find a build issue with mx27_defconfig, but it's not related to this patch:

In file included from arch/arm/mm/init.c:27:
/arm/include/asm/tlb.h: In function 'tlb_flush_mmu':
arch/arm/include/asm/tlb.h:104: error: implicit declaration of function 'release_pages'
arch/arm/include/asm/tlb.h: In function 'tlb_remove_page':
arch/arm/include/asm/tlb.h:168: error: implicit declaration of function 'page_cache_release'
make[1]: *** [arch/arm/mm/init.o] Error 1
make: *** [arch/arm/mm] Error 2

Thanks for the review, v4 is on its way.

Dinh
>
>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 |

2011-03-17 21:12:30

by Uwe Kleine-König

[permalink] [raw]
Subject: Re: [PATCHv3 2/3] ARM: mx51: Implement code to allow mx51 to enter WFI

Hi Dinh,

On Thu, Mar 17, 2011 at 04:52:44PM +0000, Nguyen Dinh-R00091 wrote:
> Yes, I have and its build fine for mx3_defconfig. I did find a build issue with mx27_defconfig, but it's not related to this patch:
>
> In file included from arch/arm/mm/init.c:27:
> /arm/include/asm/tlb.h: In function 'tlb_flush_mmu':
> arch/arm/include/asm/tlb.h:104: error: implicit declaration of function 'release_pages'
> arch/arm/include/asm/tlb.h: In function 'tlb_remove_page':
> arch/arm/include/asm/tlb.h:168: error: implicit declaration of function 'page_cache_release'
> make[1]: *** [arch/arm/mm/init.o] Error 1
> make: *** [arch/arm/mm] Error 2
This is already fixed in Russell's tree, and I guess in Linus', too.

Best regards
Uwe

--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | http://www.pengutronix.de/ |