* [PATCH v2 0/2] Kirkwood suspend to RAM
@ 2013-08-10 14:27 Ezequiel Garcia
2013-08-10 14:27 ` [PATCH v2 1/2] ARM: feroceon: Add suspend/resume operation Ezequiel Garcia
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Ezequiel Garcia @ 2013-08-10 14:27 UTC (permalink / raw)
To: linux-arm-kernel
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.
The first patch implements suspend/resume procedures in Feroceon CPU.
This implementation has been copy-pasted from the one in ARM926, since
I failed to spot any relevant differences between the CPU's.
The only change from the previous RFC/PATCH version is the removal
of clock gating. Each device driver is in charge of such clock handling.
In other words, this patchset only implements the most basic support for
suspend/resume, any additional power savings must be added to each driver
by implementing proper suspend/resume support.
Given the amount of changes queued for Kirkwood, I've based this patchset
on mevbu's for-next (git://git.infradead.org/linux-mvebu.git for-next)
Tested on Openblocks A6.
Changes from v1:
* Removed clock gating. Each peripheral is in charge of implementing
a proper suspend/resume path, including gateable clock handling.
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 | 69 +++++++++++++++++++++++
arch/arm/mm/proc-feroceon.S | 26 +++++++++
5 files changed, 99 insertions(+), 1 deletion(-)
create mode 100644 arch/arm/mach-kirkwood/pm.c
--
1.8.1.5
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 1/2] ARM: feroceon: Add suspend/resume operation
2013-08-10 14:27 [PATCH v2 0/2] Kirkwood suspend to RAM Ezequiel Garcia
@ 2013-08-10 14:27 ` Ezequiel Garcia
2013-08-12 21:51 ` Ezequiel Garcia
2013-08-10 14:27 ` [PATCH v2 2/2] ARM: kirkwood: Add basic suspend-to-RAM support Ezequiel Garcia
2013-08-12 13:41 ` [PATCH v2 0/2] Kirkwood suspend to RAM Jason Cooper
2 siblings, 1 reply; 9+ messages in thread
From: Ezequiel Garcia @ 2013-08-10 14:27 UTC (permalink / raw)
To: linux-arm-kernel
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 98538e1..c57b437 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -2198,7 +2198,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 d5146b9..db79b62 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
+
.type __feroceon_setup, #function
__feroceon_setup:
mov r0, #0
--
1.8.1.5
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 2/2] ARM: kirkwood: Add basic suspend-to-RAM support
2013-08-10 14:27 [PATCH v2 0/2] Kirkwood suspend to RAM Ezequiel Garcia
2013-08-10 14:27 ` [PATCH v2 1/2] ARM: feroceon: Add suspend/resume operation Ezequiel Garcia
@ 2013-08-10 14:27 ` Ezequiel Garcia
2013-08-10 15:01 ` Andrew Lunn
2013-08-12 13:41 ` [PATCH v2 0/2] Kirkwood suspend to RAM Jason Cooper
2 siblings, 1 reply; 9+ messages in thread
From: Ezequiel Garcia @ 2013-08-10 14:27 UTC (permalink / raw)
To: linux-arm-kernel
Implements suspend-to-RAM support in a standby-like fashion:
when the SoC enters suspend state the memory PM units are disabled,
the DDR is set in self-refresh mode, and the CPU is set in WFI.
Signed-off-by: Simon Guinot <sguinot@lacie.com>
Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
Changes from v1:
* Removed clock gating. Each peripheral is in charge of implementing
a proper suspend/resume path, including gateable clock handling.
arch/arm/mach-kirkwood/Makefile | 1 +
arch/arm/mach-kirkwood/include/mach/bridge-regs.h | 2 +
arch/arm/mach-kirkwood/pm.c | 69 +++++++++++++++++++++++
3 files changed, 72 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 937d4e1..788db9c 100644
--- a/arch/arm/mach-kirkwood/Makefile
+++ b/arch/arm/mach-kirkwood/Makefile
@@ -1,4 +1,5 @@
obj-y += common.o pcie.o
+obj-$(CONFIG_PM) += pm.o
obj-$(CONFIG_KIRKWOOD_LEGACY) += irq.o mpp.o
obj-$(CONFIG_MACH_D2NET_V2) += d2net_v2-setup.o lacie_v2-common.o
obj-$(CONFIG_MACH_NET2BIG_V2) += netxbig_v2-setup.o lacie_v2-common.o
diff --git a/arch/arm/mach-kirkwood/include/mach/bridge-regs.h b/arch/arm/mach-kirkwood/include/mach/bridge-regs.h
index 91242c9..8b9d1c9 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..364734c
--- /dev/null
+++ b/arch/arm/mach-kirkwood/pm.c
@@ -0,0 +1,69 @@
+/*
+ * Power Management driver for Marvell Kirkwood SoCs
+ *
+ * Copyright (C) 2013 Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
+ * 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 mem_pm_ctrl;
+
+ mem_pm_ctrl = readl(MEMORY_PM_CTRL);
+
+ /* Set peripherals to low-power mode */
+ writel_relaxed(~0, MEMORY_PM_CTRL);
+
+ /* Set DDR in self-refresh */
+ writel_relaxed(0x7, ddr_operation_base);
+
+ /*
+ * Set CPU in wait-for-interrupt state.
+ * This disables the CPU core clocks,
+ * the array clocks, and also the L2 controller.
+ */
+ cpu_do_idle();
+
+ writel_relaxed(mem_pm_ctrl, MEMORY_PM_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
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 2/2] ARM: kirkwood: Add basic suspend-to-RAM support
2013-08-10 14:27 ` [PATCH v2 2/2] ARM: kirkwood: Add basic suspend-to-RAM support Ezequiel Garcia
@ 2013-08-10 15:01 ` Andrew Lunn
2013-08-10 15:20 ` Ezequiel Garcia
0 siblings, 1 reply; 9+ messages in thread
From: Andrew Lunn @ 2013-08-10 15:01 UTC (permalink / raw)
To: linux-arm-kernel
On Sat, Aug 10, 2013 at 11:27:19AM -0300, Ezequiel Garcia wrote:
> Implements suspend-to-RAM support in a standby-like fashion:
> when the SoC enters suspend state the memory PM units are disabled,
> the DDR is set in self-refresh mode, and the CPU is set in WFI.
>
> Signed-off-by: Simon Guinot <sguinot@lacie.com>
> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> ---
> Changes from v1:
>
> * Removed clock gating. Each peripheral is in charge of implementing
> a proper suspend/resume path, including gateable clock handling.
>
> arch/arm/mach-kirkwood/Makefile | 1 +
> arch/arm/mach-kirkwood/include/mach/bridge-regs.h | 2 +
> arch/arm/mach-kirkwood/pm.c | 69 +++++++++++++++++++++++
> 3 files changed, 72 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 937d4e1..788db9c 100644
> --- a/arch/arm/mach-kirkwood/Makefile
> +++ b/arch/arm/mach-kirkwood/Makefile
> @@ -1,4 +1,5 @@
> obj-y += common.o pcie.o
> +obj-$(CONFIG_PM) += pm.o
> obj-$(CONFIG_KIRKWOOD_LEGACY) += irq.o mpp.o
> obj-$(CONFIG_MACH_D2NET_V2) += d2net_v2-setup.o lacie_v2-common.o
> obj-$(CONFIG_MACH_NET2BIG_V2) += netxbig_v2-setup.o lacie_v2-common.o
> diff --git a/arch/arm/mach-kirkwood/include/mach/bridge-regs.h b/arch/arm/mach-kirkwood/include/mach/bridge-regs.h
> index 91242c9..8b9d1c9 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..364734c
> --- /dev/null
> +++ b/arch/arm/mach-kirkwood/pm.c
> @@ -0,0 +1,69 @@
> +/*
> + * Power Management driver for Marvell Kirkwood SoCs
> + *
> + * Copyright (C) 2013 Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
> + * 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 mem_pm_ctrl;
> +
> + mem_pm_ctrl = readl(MEMORY_PM_CTRL);
> +
> + /* Set peripherals to low-power mode */
> + writel_relaxed(~0, MEMORY_PM_CTRL);
> +
> + /* Set DDR in self-refresh */
> + writel_relaxed(0x7, ddr_operation_base);
> +
> + /*
> + * Set CPU in wait-for-interrupt state.
> + * This disables the CPU core clocks,
> + * the array clocks, and also the L2 controller.
> + */
> + cpu_do_idle();
> +
> + writel_relaxed(mem_pm_ctrl, MEMORY_PM_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);
Hi Ezequiel
Does this work on a multi arch kernel? Should kirkwood_pm_init() be
checking it is actually running on a kirkwood? Or call
kirkwood_pm_init() from kirkwood_dt_init()?
Another issue is that the ddr_operation_base should probably be
accessed using your atomic_io_set_clear() function, since it is used
by the cpuidle drivers as well.
Andrew
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 2/2] ARM: kirkwood: Add basic suspend-to-RAM support
2013-08-10 15:01 ` Andrew Lunn
@ 2013-08-10 15:20 ` Ezequiel Garcia
2013-08-10 16:00 ` Andrew Lunn
0 siblings, 1 reply; 9+ messages in thread
From: Ezequiel Garcia @ 2013-08-10 15:20 UTC (permalink / raw)
To: linux-arm-kernel
On Sat, Aug 10, 2013 at 05:01:15PM +0200, Andrew Lunn wrote:
> On Sat, Aug 10, 2013 at 11:27:19AM -0300, Ezequiel Garcia wrote:
> > Implements suspend-to-RAM support in a standby-like fashion:
> > when the SoC enters suspend state the memory PM units are disabled,
> > the DDR is set in self-refresh mode, and the CPU is set in WFI.
> >
> > Signed-off-by: Simon Guinot <sguinot@lacie.com>
> > Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> > ---
> > Changes from v1:
> >
> > * Removed clock gating. Each peripheral is in charge of implementing
> > a proper suspend/resume path, including gateable clock handling.
> >
> > arch/arm/mach-kirkwood/Makefile | 1 +
> > arch/arm/mach-kirkwood/include/mach/bridge-regs.h | 2 +
> > arch/arm/mach-kirkwood/pm.c | 69 +++++++++++++++++++++++
> > 3 files changed, 72 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 937d4e1..788db9c 100644
> > --- a/arch/arm/mach-kirkwood/Makefile
> > +++ b/arch/arm/mach-kirkwood/Makefile
> > @@ -1,4 +1,5 @@
> > obj-y += common.o pcie.o
> > +obj-$(CONFIG_PM) += pm.o
> > obj-$(CONFIG_KIRKWOOD_LEGACY) += irq.o mpp.o
> > obj-$(CONFIG_MACH_D2NET_V2) += d2net_v2-setup.o lacie_v2-common.o
> > obj-$(CONFIG_MACH_NET2BIG_V2) += netxbig_v2-setup.o lacie_v2-common.o
> > diff --git a/arch/arm/mach-kirkwood/include/mach/bridge-regs.h b/arch/arm/mach-kirkwood/include/mach/bridge-regs.h
> > index 91242c9..8b9d1c9 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..364734c
> > --- /dev/null
> > +++ b/arch/arm/mach-kirkwood/pm.c
> > @@ -0,0 +1,69 @@
> > +/*
> > + * Power Management driver for Marvell Kirkwood SoCs
> > + *
> > + * Copyright (C) 2013 Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
> > + * 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 mem_pm_ctrl;
> > +
> > + mem_pm_ctrl = readl(MEMORY_PM_CTRL);
> > +
> > + /* Set peripherals to low-power mode */
> > + writel_relaxed(~0, MEMORY_PM_CTRL);
> > +
> > + /* Set DDR in self-refresh */
> > + writel_relaxed(0x7, ddr_operation_base);
> > +
> > + /*
> > + * Set CPU in wait-for-interrupt state.
> > + * This disables the CPU core clocks,
> > + * the array clocks, and also the L2 controller.
> > + */
> > + cpu_do_idle();
> > +
> > + writel_relaxed(mem_pm_ctrl, MEMORY_PM_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);
>
> Hi Ezequiel
>
> Does this work on a multi arch kernel? Should kirkwood_pm_init() be
> checking it is actually running on a kirkwood? Or call
> kirkwood_pm_init() from kirkwood_dt_init()?
>
Oh, right... I think you already mentioned this. Sorry, forgot about
this completely.
On the other side, kirkwood is not multiplatform-enabled yet, right?
So there's no way this can produce any build error.
> Another issue is that the ddr_operation_base should probably be
> accessed using your atomic_io_set_clear() function, since it is used
> by the cpuidle drivers as well.
>
I think this is not needed. Both cpuidle suspend core code disables
IRQ before entering any of the suspend or cpuidle states.
I'm not sure this is sufficient in SMP context? And in any case,
do we care about SMP, given kirkwood is UP?
BTW, do you think the approach of *not* messing with clocks is ok?
Thanks for the feedback!
--
Ezequiel Garc?a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 2/2] ARM: kirkwood: Add basic suspend-to-RAM support
2013-08-10 15:20 ` Ezequiel Garcia
@ 2013-08-10 16:00 ` Andrew Lunn
2013-08-10 17:32 ` Ezequiel Garcia
0 siblings, 1 reply; 9+ messages in thread
From: Andrew Lunn @ 2013-08-10 16:00 UTC (permalink / raw)
To: linux-arm-kernel
> > > +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);
> >
> > Hi Ezequiel
> >
> > Does this work on a multi arch kernel? Should kirkwood_pm_init() be
> > checking it is actually running on a kirkwood? Or call
> > kirkwood_pm_init() from kirkwood_dt_init()?
> >
>
> Oh, right... I think you already mentioned this. Sorry, forgot about
> this completely.
>
> On the other side, kirkwood is not multiplatform-enabled yet, right?
> So there's no way this can produce any build error.
Kirkwood is not currently multiplatform, but there is nothing i know
of which will break when we do become multi-platform. I've been
keeping an eye out for such issues, and made sure that cpuidle,
cpufreq, etc are multiplatform safe. I think Thomas removed the last
blocker for DT based boards going multiplatform when PCI moved to DT.
So i don't want to add something now, which i know will break with
multiplatform. Lets do it the right way now.
> > Another issue is that the ddr_operation_base should probably be
> > accessed using your atomic_io_set_clear() function, since it is used
> > by the cpuidle drivers as well.
> >
>
> I think this is not needed. Both cpuidle suspend core code disables
> IRQ before entering any of the suspend or cpuidle states.
I've no idea how PM works, so i cannot say if its safe or not...
>
> BTW, do you think the approach of *not* messing with clocks is ok?
That is correct. The drivers should be disabling clocks, were they can
be disabled. We know kirkwood ethernet is broken and we cannot
"easily" disable its clock.
Andrew
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 2/2] ARM: kirkwood: Add basic suspend-to-RAM support
2013-08-10 16:00 ` Andrew Lunn
@ 2013-08-10 17:32 ` Ezequiel Garcia
0 siblings, 0 replies; 9+ messages in thread
From: Ezequiel Garcia @ 2013-08-10 17:32 UTC (permalink / raw)
To: linux-arm-kernel
On Sat, Aug 10, 2013 at 06:00:19PM +0200, Andrew Lunn wrote:
> > > > +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);
> > >
> > > Hi Ezequiel
> > >
> > > Does this work on a multi arch kernel? Should kirkwood_pm_init() be
> > > checking it is actually running on a kirkwood? Or call
> > > kirkwood_pm_init() from kirkwood_dt_init()?
> > >
> >
> > Oh, right... I think you already mentioned this. Sorry, forgot about
> > this completely.
> >
> > On the other side, kirkwood is not multiplatform-enabled yet, right?
> > So there's no way this can produce any build error.
>
> Kirkwood is not currently multiplatform, but there is nothing i know
> of which will break when we do become multi-platform. I've been
> keeping an eye out for such issues, and made sure that cpuidle,
> cpufreq, etc are multiplatform safe. I think Thomas removed the last
> blocker for DT based boards going multiplatform when PCI moved to DT.
> So i don't want to add something now, which i know will break with
> multiplatform. Lets do it the right way now.
>
Of course. And I'd say it's better call kirkwood_pm_init() from some place
kirkwood-specific, rather than leaving the generic arch_initcall checking
for kirkwood machine.
It looks less bloaty, uh?
> > > Another issue is that the ddr_operation_base should probably be
> > > accessed using your atomic_io_set_clear() function, since it is used
> > > by the cpuidle drivers as well.
> > >
> >
> > I think this is not needed. Both cpuidle suspend core code disables
> > IRQ before entering any of the suspend or cpuidle states.
>
> I've no idea how PM works, so i cannot say if its safe or not...
Me neither. But I'm checking the code and it's screaming "local irqs
disabled" before entering any state. Also, the ticker seems to be stopped.
I'd like to know we *really* need such register protection, given
it's not an easy task (see the atomic_io_clear_set discussion).
> >
> > BTW, do you think the approach of *not* messing with clocks is ok?
>
> That is correct. The drivers should be disabling clocks, were they can
> be disabled. We know kirkwood ethernet is broken and we cannot
> "easily" disable its clock.
>
Ok, good.
One more thing I came across while reviewing the PM code.
There's a "standby" state that seems much more appropriate than
the current "mem" state.
I looks like there's not any distinction in the PM code handling
between the two "mem" and "standby" states, but it's a bit misleading
for users to declare "suspend to RAM" when it's not.
So I think we should use that instead. I'll fix all of these and prepare a v3.
Thanks for the feedback!
--
Ezequiel Garc?a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 0/2] Kirkwood suspend to RAM
2013-08-10 14:27 [PATCH v2 0/2] Kirkwood suspend to RAM Ezequiel Garcia
2013-08-10 14:27 ` [PATCH v2 1/2] ARM: feroceon: Add suspend/resume operation Ezequiel Garcia
2013-08-10 14:27 ` [PATCH v2 2/2] ARM: kirkwood: Add basic suspend-to-RAM support Ezequiel Garcia
@ 2013-08-12 13:41 ` Jason Cooper
2 siblings, 0 replies; 9+ messages in thread
From: Jason Cooper @ 2013-08-12 13:41 UTC (permalink / raw)
To: linux-arm-kernel
Ezequiel,
On Sat, Aug 10, 2013 at 11:27:17AM -0300, Ezequiel Garcia wrote:
...
> Given the amount of changes queued for Kirkwood, I've based this patchset
> on mevbu's for-next (git://git.infradead.org/linux-mvebu.git for-next)
Please base this on a tag in mainline unless you _need_ a patch that
isn't there yet.
thx,
Jason.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 1/2] ARM: feroceon: Add suspend/resume operation
2013-08-10 14:27 ` [PATCH v2 1/2] ARM: feroceon: Add suspend/resume operation Ezequiel Garcia
@ 2013-08-12 21:51 ` Ezequiel Garcia
0 siblings, 0 replies; 9+ messages in thread
From: Ezequiel Garcia @ 2013-08-12 21:51 UTC (permalink / raw)
To: linux-arm-kernel
Russell,
On Sat, Aug 10, 2013 at 11:27:18AM -0300, Ezequiel Garcia wrote:
> 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 98538e1..c57b437 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -2198,7 +2198,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 d5146b9..db79b62 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
> +
> .type __feroceon_setup, #function
> __feroceon_setup:
> mov r0, #0
> --
> 1.8.1.5
>
Any comments for this one?
If this is OK, then I'd like to add it in the tracking system
to be merged in either v3.12 or v3.13, and be able to introduce
suspend/resume later.
--
Ezequiel Garc?a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2013-08-12 21:51 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-08-10 14:27 [PATCH v2 0/2] Kirkwood suspend to RAM Ezequiel Garcia
2013-08-10 14:27 ` [PATCH v2 1/2] ARM: feroceon: Add suspend/resume operation Ezequiel Garcia
2013-08-12 21:51 ` Ezequiel Garcia
2013-08-10 14:27 ` [PATCH v2 2/2] ARM: kirkwood: Add basic suspend-to-RAM support Ezequiel Garcia
2013-08-10 15:01 ` Andrew Lunn
2013-08-10 15:20 ` Ezequiel Garcia
2013-08-10 16:00 ` Andrew Lunn
2013-08-10 17:32 ` Ezequiel Garcia
2013-08-12 13:41 ` [PATCH v2 0/2] Kirkwood suspend to RAM Jason Cooper
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.