[RFC/PATCH 0/2] ARM: kirkwood: Add suspend/resume support

[RFC/PATCH 0/2] ARM: kirkwood: Add suspend/resume support

[Ezequiel Garcia]

This patchset implements a very basic suspend/resume support on
Kirkwood SoC's, based on the work of Simon Guinot.
Given the CPU itself is not stopped on suspend, but just sits
waiting for interruption, this is more similar to a standby-mode,
than to a real suspend-to-RAM.

In this opportunity, I'm sending this patchset as a very early RFC,
mostly to get some feedback on the first patch that implements the
suspend/resume procedures in the Feroceon CPU. This implemenation
has been copy-pasted from the one in ARM926, since I failed to spot
any differences between the CPU's on that matter.

Maybe someone more knowledgable can take a look and provide some insights.

This applies cleanly on top of v3.10-rc7 and has been tested on an
Openblocks A6 box. Notice that the network controller clock is not gated,
because the driver still needs proper suspend/resume operations.

Thanks a lot!

Ezequiel Garcia (2):
  ARM: feroceon: Add suspend/resume operation
  ARM: kirkwood: Add basic suspend-to-RAM support

 arch/arm/Kconfig                                  |  2 +-
 arch/arm/mach-kirkwood/Makefile                   |  1 +
 arch/arm/mach-kirkwood/include/mach/bridge-regs.h |  2 +
 arch/arm/mach-kirkwood/pm.c                       | 83 +++++++++++++++++++++++
 arch/arm/mm/proc-feroceon.S                       | 26 +++++++
 5 files changed, 113 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm/mach-kirkwood/pm.c

-- 
1.8.1.5

[RFC/PATCH 1/2] ARM: feroceon: Add suspend/resume operation

[Ezequiel Garcia]

Add support for suspend/resume operations. The implemented procedures
are identical to the ones for ARM926.

Cc: Assaf Hoffman <hoffman@marvell.com>
Cc: Russell King <linux@arm.linux.org.uk>
Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 arch/arm/Kconfig            |  2 +-
 arch/arm/mm/proc-feroceon.S | 26 ++++++++++++++++++++++++++
 2 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 2651b1d..a35287b 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -2207,7 +2207,7 @@ source "kernel/power/Kconfig"
 
 config ARCH_SUSPEND_POSSIBLE
 	depends on !ARCH_S5PC100
-	depends on CPU_ARM920T || CPU_ARM926T || CPU_SA1100 || \
+	depends on CPU_ARM920T || CPU_ARM926T || CPU_FEROCEON || CPU_SA1100 || \
 		CPU_V6 || CPU_V6K || CPU_V7 || CPU_XSC3 || CPU_XSCALE || CPU_MOHAWK
 	def_bool y
 
diff --git a/arch/arm/mm/proc-feroceon.S b/arch/arm/mm/proc-feroceon.S
index 4106b09..ac9201c 100644
--- a/arch/arm/mm/proc-feroceon.S
+++ b/arch/arm/mm/proc-feroceon.S
@@ -514,6 +514,32 @@ ENTRY(cpu_feroceon_set_pte_ext)
 #endif
 	mov	pc, lr
 
+/* Suspend/resume support: taken from arch/arm/mm/proc-arm926.S */
+.globl	cpu_feroceon_suspend_size
+.equ	cpu_feroceon_suspend_size, 4 * 3
+#ifdef CONFIG_ARM_CPU_SUSPEND
+ENTRY(cpu_feroceon_do_suspend)
+	stmfd	sp!, {r4 - r6, lr}
+	mrc	p15, 0, r4, c13, c0, 0	@ PID
+	mrc	p15, 0, r5, c3, c0, 0	@ Domain ID
+	mrc	p15, 0, r6, c1, c0, 0	@ Control register
+	stmia	r0, {r4 - r6}
+	ldmfd	sp!, {r4 - r6, pc}
+ENDPROC(cpu_feroceon_do_suspend)
+
+ENTRY(cpu_feroceon_do_resume)
+	mov	ip, #0
+	mcr	p15, 0, ip, c8, c7, 0	@ invalidate I+D TLBs
+	mcr	p15, 0, ip, c7, c7, 0	@ invalidate I+D caches
+	ldmia	r0, {r4 - r6}
+	mcr	p15, 0, r4, c13, c0, 0	@ PID
+	mcr	p15, 0, r5, c3, c0, 0	@ Domain ID
+	mcr	p15, 0, r1, c2, c0, 0	@ TTB address
+	mov	r0, r6			@ control register
+	b	cpu_resume_mmu
+ENDPROC(cpu_feroceon_do_resume)
+#endif
+
 	__CPUINIT
 
 	.type	__feroceon_setup, #function
-- 
1.8.1.5

[RFC/PATCH 2/2] ARM: kirkwood: Add basic suspend-to-RAM support

[Ezequiel Garcia]

Implements suspend-to-RAM support in a standby-like fashion:
when the SoC enters suspend state the peripheral clocks are gated
and the memory PM units are disabled.

However the network controller clock is not gated for now, since there is
still some work to be done in the driver to provide proper suspend/resume
operations.

Signed-off-by: Simon Guinot <sguinot@lacie.com>
Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 arch/arm/mach-kirkwood/Makefile                   |  1 +
 arch/arm/mach-kirkwood/include/mach/bridge-regs.h |  2 +
 arch/arm/mach-kirkwood/pm.c                       | 83 +++++++++++++++++++++++
 3 files changed, 86 insertions(+)
 create mode 100644 arch/arm/mach-kirkwood/pm.c

diff --git a/arch/arm/mach-kirkwood/Makefile b/arch/arm/mach-kirkwood/Makefile
index e1f3735..2c56aa0 100644
--- a/arch/arm/mach-kirkwood/Makefile
+++ b/arch/arm/mach-kirkwood/Makefile
@@ -1,4 +1,5 @@
 obj-y				+= common.o irq.o pcie.o mpp.o
+obj-$(CONFIG_PM)		+= pm.o
 
 obj-$(CONFIG_MACH_D2NET_V2)		+= d2net_v2-setup.o lacie_v2-common.o
 obj-$(CONFIG_MACH_DB88F6281_BP)		+= db88f6281-bp-setup.o
diff --git a/arch/arm/mach-kirkwood/include/mach/bridge-regs.h b/arch/arm/mach-kirkwood/include/mach/bridge-regs.h
index 5c82b7d..fad3a35 100644
--- a/arch/arm/mach-kirkwood/include/mach/bridge-regs.h
+++ b/arch/arm/mach-kirkwood/include/mach/bridge-regs.h
@@ -78,4 +78,6 @@
 #define CGC_TDM			(1 << 20)
 #define CGC_RESERVED		(0x6 << 21)
 
+#define MEMORY_PM_CTRL		(BRIDGE_VIRT_BASE + 0x118)
+
 #endif
diff --git a/arch/arm/mach-kirkwood/pm.c b/arch/arm/mach-kirkwood/pm.c
new file mode 100644
index 0000000..5624b66
--- /dev/null
+++ b/arch/arm/mach-kirkwood/pm.c
@@ -0,0 +1,83 @@
+/*
+ * Power Management driver for Marvell Kirkwood SoCs
+ *
+ * Copyright (C) 2013 Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
+ * Copyright (C) 2010 Simon Guinot <sguinot@lacie.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License,
+ * version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/kernel.h>
+#include <linux/suspend.h>
+#include <linux/io.h>
+#include <mach/bridge-regs.h>
+
+
+static void __iomem *ddr_operation_base;
+
+static void kirkwood_suspend_mem(void)
+{
+	u32 clk_ctrl, mem_pm_ctrl;
+
+	clk_ctrl = readl(CLOCK_GATING_CTRL);
+	mem_pm_ctrl = readl(MEMORY_PM_CTRL);
+
+	/*
+	 * Gate clocks, except for so-called 'Runit' unit, which apparently
+	 * corresponds to NAND, SPI and BootROM.
+	 *
+	 * Also, we skip the gating of the GE0 and GE1 clocks for now.
+	 *
+	 * Contrary to my expectation, the SDRAM clock can also be disabled and
+	 * the suspend-resume cycle will complete successfully.
+	 */
+	writel(CGC_GE1 | CGC_GE0 | CGC_RUNIT, CLOCK_GATING_CTRL);
+
+	/* Set peripherals to low-power mode */
+	writel(~0, MEMORY_PM_CTRL);
+
+	/*
+	 * Set DDR in self-refresh.
+	 */
+	writel(0x7, ddr_operation_base);
+
+	/*
+	 * Set CPU in wait-for-interrupt state.
+	 */
+	cpu_do_idle();
+
+	writel(mem_pm_ctrl, MEMORY_PM_CTRL);
+	writel(clk_ctrl, CLOCK_GATING_CTRL);
+}
+
+static int kirkwood_suspend_enter(suspend_state_t state)
+{
+	switch (state) {
+	case PM_SUSPEND_MEM:
+		kirkwood_suspend_mem();
+		break;
+	default:
+		return -EINVAL;
+	}
+	return 0;
+}
+
+static const struct platform_suspend_ops kirkwood_suspend_ops = {
+	.enter = kirkwood_suspend_enter,
+	.valid = suspend_valid_only_mem,
+};
+
+static int __init kirkwood_pm_init(void)
+{
+	ddr_operation_base = ioremap(DDR_OPERATION_BASE, 4);
+	suspend_set_ops(&kirkwood_suspend_ops);
+	return 0;
+}
+arch_initcall(kirkwood_pm_init);
-- 
1.8.1.5

[RFC/PATCH 2/2] ARM: kirkwood: Add basic suspend-to-RAM support

[Ezequiel Garcia]

(Yet another resend, this time to public ML as well)

On Wed, Jul 10, 2013 at 02:13:48PM -0300, Ezequiel Garcia wrote:
> (Resending it with some more Ccs)
> 
> On Wed, Jul 10, 2013 at 03:07:02PM +0200, Andrew Lunn wrote:
> > On Mon, Jul 01, 2013 at 07:47:29PM -0300, Ezequiel Garcia wrote:
> > > Implements suspend-to-RAM support in a standby-like fashion:
> > > when the SoC enters suspend state the peripheral clocks are gated
> > > and the memory PM units are disabled.
> > > 
> > > However the network controller clock is not gated for now, since there is
> > > still some work to be done in the driver to provide proper suspend/resume
> > > operations.
> > > 
> > > Signed-off-by: Simon Guinot <sguinot@lacie.com>
> > > Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> > > ---
> > >  arch/arm/mach-kirkwood/Makefile                   |  1 +
> > >  arch/arm/mach-kirkwood/include/mach/bridge-regs.h |  2 +
> > >  arch/arm/mach-kirkwood/pm.c                       | 83 +++++++++++++++++++++++
> > >  3 files changed, 86 insertions(+)
> > >  create mode 100644 arch/arm/mach-kirkwood/pm.c
> > > 
> > > diff --git a/arch/arm/mach-kirkwood/Makefile b/arch/arm/mach-kirkwood/Makefile
> > > index e1f3735..2c56aa0 100644
> > > --- a/arch/arm/mach-kirkwood/Makefile
> > > +++ b/arch/arm/mach-kirkwood/Makefile
> > > @@ -1,4 +1,5 @@
> > >  obj-y				+= common.o irq.o pcie.o mpp.o
> > > +obj-$(CONFIG_PM)		+= pm.o
> > >  
> > >  obj-$(CONFIG_MACH_D2NET_V2)		+= d2net_v2-setup.o lacie_v2-common.o
> > >  obj-$(CONFIG_MACH_DB88F6281_BP)		+= db88f6281-bp-setup.o
> > > diff --git a/arch/arm/mach-kirkwood/include/mach/bridge-regs.h b/arch/arm/mach-kirkwood/include/mach/bridge-regs.h
> > > index 5c82b7d..fad3a35 100644
> > > --- a/arch/arm/mach-kirkwood/include/mach/bridge-regs.h
> > > +++ b/arch/arm/mach-kirkwood/include/mach/bridge-regs.h
> > > @@ -78,4 +78,6 @@
> > >  #define CGC_TDM			(1 << 20)
> > >  #define CGC_RESERVED		(0x6 << 21)
> > >  
> > > +#define MEMORY_PM_CTRL		(BRIDGE_VIRT_BASE + 0x118)
> > > +
> > >  #endif
> > > diff --git a/arch/arm/mach-kirkwood/pm.c b/arch/arm/mach-kirkwood/pm.c
> > > new file mode 100644
> > > index 0000000..5624b66
> > > --- /dev/null
> > > +++ b/arch/arm/mach-kirkwood/pm.c
> > > @@ -0,0 +1,83 @@
> > > +/*
> > > + * Power Management driver for Marvell Kirkwood SoCs
> > > + *
> > > + * Copyright (C) 2013 Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> > > + * Copyright (C) 2010 Simon Guinot <sguinot@lacie.com>
> > > + *
> > > + * This program is free software; you can redistribute it and/or modify
> > > + * it under the terms of the GNU General Public License,
> > > + * version 2 of the License.
> > > + *
> > > + * This program is distributed in the hope that it will be useful,
> > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > > + * GNU General Public License for more details.
> > > + */
> > > +
> > > +#include <linux/kernel.h>
> > > +#include <linux/suspend.h>
> > > +#include <linux/io.h>
> > > +#include <mach/bridge-regs.h>
> > > +
> > > +
> > > +static void __iomem *ddr_operation_base;
> > > +
> > > +static void kirkwood_suspend_mem(void)
> > > +{
> > > +	u32 clk_ctrl, mem_pm_ctrl;
> > > +
> > > +	clk_ctrl = readl(CLOCK_GATING_CTRL);
> > > +	mem_pm_ctrl = readl(MEMORY_PM_CTRL);
> > > +
> > > +	/*
> > > +	 * Gate clocks, except for so-called 'Runit' unit, which apparently
> > > +	 * corresponds to NAND, SPI and BootROM.
> > > +	 *
> > > +	 * Also, we skip the gating of the GE0 and GE1 clocks for now.
> > > +	 *
> > > +	 * Contrary to my expectation, the SDRAM clock can also be disabled and
> > > +	 * the suspend-resume cycle will complete successfully.
> > > +	 */
> > > +	writel(CGC_GE1 | CGC_GE0 | CGC_RUNIT, CLOCK_GATING_CTRL);
> > > +
> > > +	/* Set peripherals to low-power mode */
> > > +	writel(~0, MEMORY_PM_CTRL);
> > > +
> > > +	/*
> > > +	 * Set DDR in self-refresh.
> > > +	 */
> > > +	writel(0x7, ddr_operation_base);
> > > +
> > > +	/*
> > > +	 * Set CPU in wait-for-interrupt state.
> > > +	 */
> > > +	cpu_do_idle();
> > > +
> > > +	writel(mem_pm_ctrl, MEMORY_PM_CTRL);
> > > +	writel(clk_ctrl, CLOCK_GATING_CTRL);
> > > +}
> > > +
> > > +static int kirkwood_suspend_enter(suspend_state_t state)
> > > +{
> > > +	switch (state) {
> > > +	case PM_SUSPEND_MEM:
> > > +		kirkwood_suspend_mem();
> > > +		break;
> > > +	default:
> > > +		return -EINVAL;
> > > +	}
> > > +	return 0;
> > > +}
> > > +
> > > +static const struct platform_suspend_ops kirkwood_suspend_ops = {
> > > +	.enter = kirkwood_suspend_enter,
> > > +	.valid = suspend_valid_only_mem,
> > > +};
> > > +
> > > +static int __init kirkwood_pm_init(void)
> > > +{
> > > +	ddr_operation_base = ioremap(DDR_OPERATION_BASE, 4);
> > 
> > Sorry for not replying earlier.
> > 
> 
> No problem. I've been testing this a lot lately, so I'll send a v2
> -more or less- soon.
> 
> FWIW, the v2 will remove *completely* the clock gating from this file,
> since I consider that to be each driver's business and don't think
> it's a good idea to mess with that.
> 
> Before I send the v2 I'd like to perform some (basic) power measurements
> to test the effect of this.
> 
> > Have you tried this in combination with the kirkwood cpuidle code?
> > That also ioremap()s the DDR_OPERATION_BASE. Are two drivers allowed
> > to do this?
> > 
> 
> Hum... yes, I'm pretty sure I've been testing them together.
> 
> AFAIK, nothing prevents you from ioremapping twice the same area,
> it's request_mem_region() what handle such situations, not ioremap.
> 
> However, now that you bring this to my mind: we should re-visit this
> approach as it's a bit racy. Maybe the cpuidle can export a function
> to set the self-refresh mode?
> 
> > And a dumb question. What actually brings it out of suspend mode?
> > Interrupts on GPIO lines?
> > 
> 
> I'm not entirely sure, but I guess the CPU will get out of the
> Wait-for-interrupt state by any enabled interruption source.
> Currently, I'm waking it with a console key stroke or a GPIO button,
> but there may be other means of resuming.
> 
> Thanks a lot for looking at this!

-- 
Ezequiel Garc?a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com