[PATCH] sh: hibernation support

[PATCH] sh: hibernation support

[Magnus Damm]

From: Magnus Damm <damm@igel.co.jp>

Add Suspend-to-disk / swsusp / CONFIG_HIBERNATION support
to the SuperH architecture.

To suspend, use "swapon /dev/sda2; echo disk > /sys/power/state"
To resume, pass "resume=/dev/sda2" on the kernel command line.

The patch "pm: rework includes, remove arch ifdefs V2" is
needed to allow the generic swsusp code to build properly.

Hibernation is not enabled with this patch though, a patch
setting ARCH_HIBERNATION_POSSIBLE will be submitted later.

Signed-off-by: Magnus Damm <damm@igel.co.jp>
---

 Tested on a sh7785lcr board.

 arch/sh/include/asm/suspend.h  |   14 +++
 arch/sh/kernel/Makefile_32     |    1 
 arch/sh/kernel/asm-offsets.c   |    8 ++
 arch/sh/kernel/cpu/sh3/entry.S |  144 +++++++++++++++++++++++++++++++++++++++-
 arch/sh/kernel/pm.c            |   39 ++++++++++
 5 files changed, 203 insertions(+), 3 deletions(-)

--- /dev/null
+++ work/arch/sh/include/asm/suspend.h	2009-03-02 15:23:59.000000000 +0900
@@ -0,0 +1,14 @@
+#ifndef _ASM_SH_SUSPEND_H
+#define _ASM_SH_SUSPEND_H
+
+static inline int arch_prepare_suspend(void) { return 0; }
+extern const void __nosave_begin, __nosave_end;
+
+#include <asm/ptrace.h>
+
+struct swsusp_arch_regs {
+	struct pt_regs user_regs;
+	unsigned long bank1_regs[8];
+};
+
+#endif /* _ASM_SH_SUSPEND_H */
--- 0001/arch/sh/kernel/Makefile_32
+++ work/arch/sh/kernel/Makefile_32	2009-03-02 15:23:59.000000000 +0900
@@ -30,5 +30,6 @@ obj-$(CONFIG_KPROBES)		+= kprobes.o
 obj-$(CONFIG_GENERIC_GPIO)	+= gpio.o
 obj-$(CONFIG_DYNAMIC_FTRACE)	+= ftrace.o
 obj-$(CONFIG_DUMP_CODE)		+= disassemble.o
+obj-$(CONFIG_PM)		+= pm.o
 
 EXTRA_CFLAGS += -Werror
--- 0001/arch/sh/kernel/asm-offsets.c
+++ work/arch/sh/kernel/asm-offsets.c	2009-03-02 15:23:59.000000000 +0900
@@ -12,8 +12,10 @@
 #include <linux/types.h>
 #include <linux/mm.h>
 #include <linux/kbuild.h>
+#include <linux/suspend.h>
 
 #include <asm/thread_info.h>
+#include <asm/suspend.h>
 
 int main(void)
 {
@@ -25,5 +27,11 @@ int main(void)
 	DEFINE(TI_PRE_COUNT,	offsetof(struct thread_info, preempt_count));
 	DEFINE(TI_RESTART_BLOCK,offsetof(struct thread_info, restart_block));
 
+#ifdef CONFIG_HIBERNATION
+	DEFINE(pbe_address, offsetof(struct pbe, address));
+	DEFINE(pbe_orig_address, offsetof(struct pbe, orig_address));
+	DEFINE(pbe_next, offsetof(struct pbe, next));
+	DEFINE(SWSUSP_ARCH_REGS_SIZE, sizeof(struct swsusp_arch_regs));
+#endif
 	return 0;
 }
--- 0001/arch/sh/kernel/cpu/sh3/entry.S
+++ work/arch/sh/kernel/cpu/sh3/entry.S	2009-03-02 15:26:57.000000000 +0900
@@ -323,6 +323,76 @@ skip_restore:
 #endif
 7:	.long	0x30000000
 
+#ifdef CONFIG_HIBERNATION
+! swsusp_arch_resume()
+! - copy restore_pblist pages
+! - restore registers from swsusp_arch_regs_cpu0
+
+ENTRY(swsusp_arch_resume)
+	mov.l	4f, r15
+	mov.l	1f, r4
+	mov.l	@r4, r4
+
+copy_loop:
+	mov	r4, r0
+	cmp/eq	#0, r0
+	bt	do_restore_regs
+
+	mov.l	@(pbe_address, r4), r2
+	mov.l	@(pbe_orig_address, r4), r5
+
+	mov.l	2f, r3
+	shlr2	r3
+	shlr2	r3
+copy_page:
+	dt	r3
+	mov.l	@r2+,r1   /*  16n+0 */
+	mov.l	r1,@r5
+	add	#4,r5
+	mov.l	@r2+,r1	  /*  16n+4 */
+	mov.l	r1,@r5
+	add	#4,r5
+	mov.l	@r2+,r1   /*  16n+8 */
+	mov.l	r1,@r5
+	add	#4,r5
+	mov.l	@r2+,r1   /*  16n+12 */
+	mov.l	r1,@r5
+	bf/s	copy_page
+	 add	#4,r5
+
+	bra	copy_loop
+	 mov.l	@(pbe_next, r4), r4
+
+do_restore_regs:
+	! BL=0: R7->R0 is bank0
+	mov.l	3f, r8
+	bsr	restore_regs
+	 nop
+
+	! BL=1: R7->R0 is bank1
+	lds	k2, pr
+	ldc	k3, ssr
+
+	mov.l	@r15+, r0
+	mov.l	@r15+, r1
+	mov.l	@r15+, r2
+	mov.l	@r15+, r3
+	mov.l	@r15+, r4
+	mov.l	@r15+, r5
+	mov.l	@r15+, r6
+	mov.l	@r15+, r7
+
+	rte
+	 nop
+	! BL=0: R7->R0 is bank0
+
+	.align	2
+1:	.long	restore_pblist
+2:	.long	PAGE_SIZE
+3:	.long	0x20000000 ! RB=1
+4:	.long	swsusp_arch_regs_cpu0
+#endif /* CONFIG_HIBERNATION */
+
 ! common exception handler
 #include "../../entry-common.S"
 	
@@ -362,8 +432,10 @@ general_exception:
 	 nop
 
 	! Save registers / Switch to bank 0
+	mov.l	k4, k2		! keep vector in k2
+	mov.l	1f, k4		! SR bits to clear in k4
 	bsr	save_regs	! needs original pr value in k3
-	 mov	k4, k2		! keep vector in k2
+	 nop
 
 	bra	handle_exception_special
 	 nop
@@ -471,6 +543,7 @@ handle_exception:
 
 	! Save registers / Switch to bank 0
 	mov.l	5f, k2		! vector register address
+	mov.l	1f, k4		! SR bits to clear in k4
 	bsr	save_regs	! needs original pr value in k3
 	 mov.l	@k2, k2		! read out vector and keep in k2
 
@@ -495,7 +568,7 @@ handle_exception_special:
 ! k0 contains original stack pointer*
 ! k1 trashed
 ! k3 passes original pr*
-! k4 trashed
+! k4 passes SR bitmask
 ! BL=1 on entry, on exit BL=0.
 
 save_regs:
@@ -518,8 +591,16 @@ save_regs:
 	mov.l	r8, @-r15
 
 	mov.l	0f, k3		! SR bits to set in k3
-	mov.l	1f, k4		! SR bits to clear in k4
 
+	! fall-through
+
+! save_low_regs()
+! - modify SR for bank switch
+! - save r7, r6, r5, r4, r3, r2, r1, r0 on the stack
+! k3 passes bits to set in SR
+! k4 passes bits to clear in SR
+
+save_low_regs:
 	stc	sr, r8
 	or	k3, r8
 	and	k4, r8
@@ -565,6 +646,7 @@ ENTRY(handle_interrupt)
 	 PREF(k0)
 
 	! Save registers / Switch to bank 0
+	mov.l	1f, k4		! SR bits to clear in k4
 	bsr	save_regs	! needs original pr value in k3
 	 mov	#-1, k2		! default vector kept in k2
 
@@ -591,3 +673,59 @@ exception_data:
 5:	.long	EXPEVT
 6:	.long	exception_handling_table
 7:	.long	ret_from_exception
+
+#ifdef CONFIG_HIBERNATION
+! swsusp_arch_suspend()
+! - prepare pc for resume, return from function without swsusp_save on resume
+! - save registers in swsusp_arch_regs_cpu0
+! - call swsusp_save write suspend image
+
+ENTRY(swsusp_arch_suspend)
+	sts	pr, r0		! save pr in r0
+	mov	r15, r2		! save sp in r2
+	mov	r8, r5		! save r8 in r5
+	stc	sr, r1
+	ldc	r1, ssr		! save sr in ssr
+	mov.l	1f, r1
+	ldc	r1, spc		! setup pc value for resuming
+	mov.l	5f, r15		! use swsusp_arch_regs_cpu0 as stack
+	mov.l	6f, r3
+	add	r3, r15		! save from top of structure
+
+	! BL=0: R7->R0 is bank0
+	mov.l	2f, r3		! get new SR value for bank1
+	mov	#0, r4
+	bsr	save_low_regs	! switch to bank1 and save bank1 r7->r0
+	 not	r4, r4
+
+	! BL=1: R7->R0 is bank1
+	stc	r2_bank, k0	! fetch old sp from r2_bank0
+	mov.l	3f, k4		! SR bits to clear in k4
+	bsr	save_regs	! switch to bank0 and save all regs
+	 stc	r0_bank, k3	! fetch old pr from r0_bank0
+
+	! BL=0: R7->R0 is bank0
+	mov	r2, r15		! restore old sp
+	mov	r5, r8		! restore old r8
+	stc	ssr, r1
+	ldc	r1, sr		! restore old sr
+	lds	r0, pr		! restore old pr
+	mov.l	4f, r0
+	jmp	@r0
+	 nop
+
+do_swsusp_save:
+	mov	r2, r15		! restore old sp
+	mov	r5, r8		! restore old r8
+	lds	r0, pr		! restore old pr
+	rts
+	 mov	#0, r0
+
+	.align	2
+1:	.long	do_swsusp_save
+2:	.long	0x20000000 ! RB=1
+3:	.long	0xdfffffff ! RB=0
+4:	.long	swsusp_save
+5:	.long	swsusp_arch_regs_cpu0
+6:	.long	SWSUSP_ARCH_REGS_SIZE
+#endif /* CONFIG_HIBERNATION */
--- /dev/null
+++ work/arch/sh/kernel/pm.c	2009-03-02 15:23:59.000000000 +0900
@@ -0,0 +1,39 @@
+/*
+ * pm.c - SuperH power management code
+ *
+ * Copyright (C) 2009 Magnus Damm
+ *
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License.  See the file "COPYING" in the main directory of this archive
+ * for more details.
+ */
+
+#include <linux/mm.h>
+#include <linux/sched.h>
+#include <linux/suspend.h>
+#include <asm/suspend.h>
+#include <asm/tlbflush.h>
+#include <asm/page.h>
+#include <asm/fpu.h>
+
+#ifdef CONFIG_HIBERNATION
+struct swsusp_arch_regs swsusp_arch_regs_cpu0;
+
+int pfn_is_nosave(unsigned long pfn)
+{
+	unsigned long begin_pfn = __pa(&__nosave_begin) >> PAGE_SHIFT;
+	unsigned long end_pfn = PAGE_ALIGN(__pa(&__nosave_end)) >> PAGE_SHIFT;
+
+	return (pfn >= begin_pfn) && (pfn < end_pfn);
+}
+
+void save_processor_state(void)
+{
+	init_fpu(current);
+}
+
+void restore_processor_state(void)
+{
+	local_flush_tlb_all();
+}
+#endif

Re: [PATCH] sh: hibernation support

[Paul Mundt]

On Fri, Mar 06, 2009 at 03:41:56PM +0900, Magnus Damm wrote:
> --- /dev/null
> +++ work/arch/sh/include/asm/suspend.h	2009-03-02 15:23:59.000000000 +0900
> @@ -0,0 +1,14 @@
> +#ifndef _ASM_SH_SUSPEND_H
> +#define _ASM_SH_SUSPEND_H
> +
> +static inline int arch_prepare_suspend(void) { return 0; }
> +extern const void __nosave_begin, __nosave_end;
> +
These belong in asm/sections.h.

> --- 0001/arch/sh/kernel/Makefile_32
> +++ work/arch/sh/kernel/Makefile_32	2009-03-02 15:23:59.000000000 +0900
> @@ -30,5 +30,6 @@ obj-$(CONFIG_KPROBES)		+= kprobes.o
>  obj-$(CONFIG_GENERIC_GPIO)	+= gpio.o
>  obj-$(CONFIG_DYNAMIC_FTRACE)	+= ftrace.o
>  obj-$(CONFIG_DUMP_CODE)		+= disassemble.o
> +obj-$(CONFIG_PM)		+= pm.o
>  
As this is hibernation specific, you should just use CONFIG_HIBERNATION
here. pm.c is also a bit ambiguous, this probably ought to be named
something more accurate, like swsusp.c or suspend.c.

> --- 0001/arch/sh/kernel/asm-offsets.c
> +++ work/arch/sh/kernel/asm-offsets.c	2009-03-02 15:23:59.000000000 +0900
> @@ -25,5 +27,11 @@ int main(void)
>  	DEFINE(TI_PRE_COUNT,	offsetof(struct thread_info, preempt_count));
>  	DEFINE(TI_RESTART_BLOCK,offsetof(struct thread_info, restart_block));
>  
> +#ifdef CONFIG_HIBERNATION
> +	DEFINE(pbe_address, offsetof(struct pbe, address));
> +	DEFINE(pbe_orig_address, offsetof(struct pbe, orig_address));
> +	DEFINE(pbe_next, offsetof(struct pbe, next));
> +	DEFINE(SWSUSP_ARCH_REGS_SIZE, sizeof(struct swsusp_arch_regs));
> +#endif

These should all be upper-case.

> --- 0001/arch/sh/kernel/cpu/sh3/entry.S
> +++ work/arch/sh/kernel/cpu/sh3/entry.S	2009-03-02 15:26:57.000000000 +0900
> @@ -323,6 +323,76 @@ skip_restore:
>  #endif
>  7:	.long	0x30000000
>  
> +#ifdef CONFIG_HIBERNATION
> +! swsusp_arch_resume()
> +! - copy restore_pblist pages
> +! - restore registers from swsusp_arch_regs_cpu0
> +
> +ENTRY(swsusp_arch_resume)
> +	mov.l	4f, r15
> +	mov.l	1f, r4
> +	mov.l	@r4, r4
> +
Please split all of this out in to a swsusp.S instead of hacking entry.S
directly.

> +copy_loop:
> +	mov	r4, r0
> +	cmp/eq	#0, r0
> +	bt	do_restore_regs
> +
tst works for this, too.

> +	mov.l	@(pbe_address, r4), r2
> +	mov.l	@(pbe_orig_address, r4), r5
> +
> +	mov.l	2f, r3
> +	shlr2	r3
> +	shlr2	r3

Consider using PAGE_SHIFT, or shifting and masking PAGE_SIZE in place.
The pre-processor can break it down in to manageable chunks for you
without having to resort to the memory load.

> +copy_page:

Please use a better name that isn't already in the global namespace.

> --- /dev/null
> +++ work/arch/sh/kernel/pm.c	2009-03-02 15:23:59.000000000 +0900
> @@ -0,0 +1,39 @@
> +/*
> + * pm.c - SuperH power management code
> + *
> + * Copyright (C) 2009 Magnus Damm
> + *
> + * This file is subject to the terms and conditions of the GNU General Public
> + * License.  See the file "COPYING" in the main directory of this archive
> + * for more details.
> + */
> +
> +#include <linux/mm.h>
> +#include <linux/sched.h>
> +#include <linux/suspend.h>
> +#include <asm/suspend.h>
> +#include <asm/tlbflush.h>
> +#include <asm/page.h>
> +#include <asm/fpu.h>
> +
> +#ifdef CONFIG_HIBERNATION
> +struct swsusp_arch_regs swsusp_arch_regs_cpu0;
> +
If you fix up the Makefile rule, you don't need the ifdef here.

Also, do you need to pre-zero swsusp_arch_regs_cpu0? Presumably this code
is also only geared at UP?

Re: [PATCH] sh: hibernation support

[Francesco VIRLINZI]

Hi Magnus
I was looking your patch and I have few question.
Is it also for sh4 with PMB? If so how are you managing the PMB?
Moreover what about interrupt controller? This means after a resume from 
hibernation
 you could have a resumed device (and driver) but the interrupt 
controller has the right irq-line not initialized.

Regards
 Francesco

Magnus Damm ha scritto:
> From: Magnus Damm <damm@igel.co.jp>
>
> Add Suspend-to-disk / swsusp / CONFIG_HIBERNATION support
> to the SuperH architecture.
>
> To suspend, use "swapon /dev/sda2; echo disk > /sys/power/state"
> To resume, pass "resume=/dev/sda2" on the kernel command line.
>
> The patch "pm: rework includes, remove arch ifdefs V2" is
> needed to allow the generic swsusp code to build properly.
>
> Hibernation is not enabled with this patch though, a patch
> setting ARCH_HIBERNATION_POSSIBLE will be submitted later.
>
> Signed-off-by: Magnus Damm <damm@igel.co.jp>
> ---
>
>  Tested on a sh7785lcr board.
>
>  arch/sh/include/asm/suspend.h  |   14 +++
>  arch/sh/kernel/Makefile_32     |    1 
>  arch/sh/kernel/asm-offsets.c   |    8 ++
>  arch/sh/kernel/cpu/sh3/entry.S |  144 +++++++++++++++++++++++++++++++++++++++-
>  arch/sh/kernel/pm.c            |   39 ++++++++++
>  5 files changed, 203 insertions(+), 3 deletions(-)
>
> --- /dev/null
> +++ work/arch/sh/include/asm/suspend.h	2009-03-02 15:23:59.000000000 +0900
> @@ -0,0 +1,14 @@
> +#ifndef _ASM_SH_SUSPEND_H
> +#define _ASM_SH_SUSPEND_H
> +
> +static inline int arch_prepare_suspend(void) { return 0; }
> +extern const void __nosave_begin, __nosave_end;
> +
> +#include <asm/ptrace.h>
> +
> +struct swsusp_arch_regs {
> +	struct pt_regs user_regs;
> +	unsigned long bank1_regs[8];
> +};
> +
> +#endif /* _ASM_SH_SUSPEND_H */
> --- 0001/arch/sh/kernel/Makefile_32
> +++ work/arch/sh/kernel/Makefile_32	2009-03-02 15:23:59.000000000 +0900
> @@ -30,5 +30,6 @@ obj-$(CONFIG_KPROBES)		+= kprobes.o
>  obj-$(CONFIG_GENERIC_GPIO)	+= gpio.o
>  obj-$(CONFIG_DYNAMIC_FTRACE)	+= ftrace.o
>  obj-$(CONFIG_DUMP_CODE)		+= disassemble.o
> +obj-$(CONFIG_PM)		+= pm.o
>  
>  EXTRA_CFLAGS += -Werror
> --- 0001/arch/sh/kernel/asm-offsets.c
> +++ work/arch/sh/kernel/asm-offsets.c	2009-03-02 15:23:59.000000000 +0900
> @@ -12,8 +12,10 @@
>  #include <linux/types.h>
>  #include <linux/mm.h>
>  #include <linux/kbuild.h>
> +#include <linux/suspend.h>
>  
>  #include <asm/thread_info.h>
> +#include <asm/suspend.h>
>  
>  int main(void)
>  {
> @@ -25,5 +27,11 @@ int main(void)
>  	DEFINE(TI_PRE_COUNT,	offsetof(struct thread_info, preempt_count));
>  	DEFINE(TI_RESTART_BLOCK,offsetof(struct thread_info, restart_block));
>  
> +#ifdef CONFIG_HIBERNATION
> +	DEFINE(pbe_address, offsetof(struct pbe, address));
> +	DEFINE(pbe_orig_address, offsetof(struct pbe, orig_address));
> +	DEFINE(pbe_next, offsetof(struct pbe, next));
> +	DEFINE(SWSUSP_ARCH_REGS_SIZE, sizeof(struct swsusp_arch_regs));
> +#endif
>  	return 0;
>  }
> --- 0001/arch/sh/kernel/cpu/sh3/entry.S
> +++ work/arch/sh/kernel/cpu/sh3/entry.S	2009-03-02 15:26:57.000000000 +0900
> @@ -323,6 +323,76 @@ skip_restore:
>  #endif
>  7:	.long	0x30000000
>  
> +#ifdef CONFIG_HIBERNATION
> +! swsusp_arch_resume()
> +! - copy restore_pblist pages
> +! - restore registers from swsusp_arch_regs_cpu0
> +
> +ENTRY(swsusp_arch_resume)
> +	mov.l	4f, r15
> +	mov.l	1f, r4
> +	mov.l	@r4, r4
> +
> +copy_loop:
> +	mov	r4, r0
> +	cmp/eq	#0, r0
> +	bt	do_restore_regs
> +
> +	mov.l	@(pbe_address, r4), r2
> +	mov.l	@(pbe_orig_address, r4), r5
> +
> +	mov.l	2f, r3
> +	shlr2	r3
> +	shlr2	r3
> +copy_page:
> +	dt	r3
> +	mov.l	@r2+,r1   /*  16n+0 */
> +	mov.l	r1,@r5
> +	add	#4,r5
> +	mov.l	@r2+,r1	  /*  16n+4 */
> +	mov.l	r1,@r5
> +	add	#4,r5
> +	mov.l	@r2+,r1   /*  16n+8 */
> +	mov.l	r1,@r5
> +	add	#4,r5
> +	mov.l	@r2+,r1   /*  16n+12 */
> +	mov.l	r1,@r5
> +	bf/s	copy_page
> +	 add	#4,r5
> +
> +	bra	copy_loop
> +	 mov.l	@(pbe_next, r4), r4
> +
> +do_restore_regs:
> +	! BL=0: R7->R0 is bank0
> +	mov.l	3f, r8
> +	bsr	restore_regs
> +	 nop
> +
> +	! BL=1: R7->R0 is bank1
> +	lds	k2, pr
> +	ldc	k3, ssr
> +
> +	mov.l	@r15+, r0
> +	mov.l	@r15+, r1
> +	mov.l	@r15+, r2
> +	mov.l	@r15+, r3
> +	mov.l	@r15+, r4
> +	mov.l	@r15+, r5
> +	mov.l	@r15+, r6
> +	mov.l	@r15+, r7
> +
> +	rte
> +	 nop
> +	! BL=0: R7->R0 is bank0
> +
> +	.align	2
> +1:	.long	restore_pblist
> +2:	.long	PAGE_SIZE
> +3:	.long	0x20000000 ! RB=1
> +4:	.long	swsusp_arch_regs_cpu0
> +#endif /* CONFIG_HIBERNATION */
> +
>  ! common exception handler
>  #include "../../entry-common.S"
>  	
> @@ -362,8 +432,10 @@ general_exception:
>  	 nop
>  
>  	! Save registers / Switch to bank 0
> +	mov.l	k4, k2		! keep vector in k2
> +	mov.l	1f, k4		! SR bits to clear in k4
>  	bsr	save_regs	! needs original pr value in k3
> -	 mov	k4, k2		! keep vector in k2
> +	 nop
>  
>  	bra	handle_exception_special
>  	 nop
> @@ -471,6 +543,7 @@ handle_exception:
>  
>  	! Save registers / Switch to bank 0
>  	mov.l	5f, k2		! vector register address
> +	mov.l	1f, k4		! SR bits to clear in k4
>  	bsr	save_regs	! needs original pr value in k3
>  	 mov.l	@k2, k2		! read out vector and keep in k2
>  
> @@ -495,7 +568,7 @@ handle_exception_special:
>  ! k0 contains original stack pointer*
>  ! k1 trashed
>  ! k3 passes original pr*
> -! k4 trashed
> +! k4 passes SR bitmask
>  ! BL=1 on entry, on exit BL=0.
>  
>  save_regs:
> @@ -518,8 +591,16 @@ save_regs:
>  	mov.l	r8, @-r15
>  
>  	mov.l	0f, k3		! SR bits to set in k3
> -	mov.l	1f, k4		! SR bits to clear in k4
>  
> +	! fall-through
> +
> +! save_low_regs()
> +! - modify SR for bank switch
> +! - save r7, r6, r5, r4, r3, r2, r1, r0 on the stack
> +! k3 passes bits to set in SR
> +! k4 passes bits to clear in SR
> +
> +save_low_regs:
>  	stc	sr, r8
>  	or	k3, r8
>  	and	k4, r8
> @@ -565,6 +646,7 @@ ENTRY(handle_interrupt)
>  	 PREF(k0)
>  
>  	! Save registers / Switch to bank 0
> +	mov.l	1f, k4		! SR bits to clear in k4
>  	bsr	save_regs	! needs original pr value in k3
>  	 mov	#-1, k2		! default vector kept in k2
>  
> @@ -591,3 +673,59 @@ exception_data:
>  5:	.long	EXPEVT
>  6:	.long	exception_handling_table
>  7:	.long	ret_from_exception
> +
> +#ifdef CONFIG_HIBERNATION
> +! swsusp_arch_suspend()
> +! - prepare pc for resume, return from function without swsusp_save on resume
> +! - save registers in swsusp_arch_regs_cpu0
> +! - call swsusp_save write suspend image
> +
> +ENTRY(swsusp_arch_suspend)
> +	sts	pr, r0		! save pr in r0
> +	mov	r15, r2		! save sp in r2
> +	mov	r8, r5		! save r8 in r5
> +	stc	sr, r1
> +	ldc	r1, ssr		! save sr in ssr
> +	mov.l	1f, r1
> +	ldc	r1, spc		! setup pc value for resuming
> +	mov.l	5f, r15		! use swsusp_arch_regs_cpu0 as stack
> +	mov.l	6f, r3
> +	add	r3, r15		! save from top of structure
> +
> +	! BL=0: R7->R0 is bank0
> +	mov.l	2f, r3		! get new SR value for bank1
> +	mov	#0, r4
> +	bsr	save_low_regs	! switch to bank1 and save bank1 r7->r0
> +	 not	r4, r4
> +
> +	! BL=1: R7->R0 is bank1
> +	stc	r2_bank, k0	! fetch old sp from r2_bank0
> +	mov.l	3f, k4		! SR bits to clear in k4
> +	bsr	save_regs	! switch to bank0 and save all regs
> +	 stc	r0_bank, k3	! fetch old pr from r0_bank0
> +
> +	! BL=0: R7->R0 is bank0
> +	mov	r2, r15		! restore old sp
> +	mov	r5, r8		! restore old r8
> +	stc	ssr, r1
> +	ldc	r1, sr		! restore old sr
> +	lds	r0, pr		! restore old pr
> +	mov.l	4f, r0
> +	jmp	@r0
> +	 nop
> +
> +do_swsusp_save:
> +	mov	r2, r15		! restore old sp
> +	mov	r5, r8		! restore old r8
> +	lds	r0, pr		! restore old pr
> +	rts
> +	 mov	#0, r0
> +
> +	.align	2
> +1:	.long	do_swsusp_save
> +2:	.long	0x20000000 ! RB=1
> +3:	.long	0xdfffffff ! RB=0
> +4:	.long	swsusp_save
> +5:	.long	swsusp_arch_regs_cpu0
> +6:	.long	SWSUSP_ARCH_REGS_SIZE
> +#endif /* CONFIG_HIBERNATION */
> --- /dev/null
> +++ work/arch/sh/kernel/pm.c	2009-03-02 15:23:59.000000000 +0900
> @@ -0,0 +1,39 @@
> +/*
> + * pm.c - SuperH power management code
> + *
> + * Copyright (C) 2009 Magnus Damm
> + *
> + * This file is subject to the terms and conditions of the GNU General Public
> + * License.  See the file "COPYING" in the main directory of this archive
> + * for more details.
> + */
> +
> +#include <linux/mm.h>
> +#include <linux/sched.h>
> +#include <linux/suspend.h>
> +#include <asm/suspend.h>
> +#include <asm/tlbflush.h>
> +#include <asm/page.h>
> +#include <asm/fpu.h>
> +
> +#ifdef CONFIG_HIBERNATION
> +struct swsusp_arch_regs swsusp_arch_regs_cpu0;
> +
> +int pfn_is_nosave(unsigned long pfn)
> +{
> +	unsigned long begin_pfn = __pa(&__nosave_begin) >> PAGE_SHIFT;
> +	unsigned long end_pfn = PAGE_ALIGN(__pa(&__nosave_end)) >> PAGE_SHIFT;
> +
> +	return (pfn >= begin_pfn) && (pfn < end_pfn);
> +}
> +
> +void save_processor_state(void)
> +{
> +	init_fpu(current);
> +}
> +
> +void restore_processor_state(void)
> +{
> +	local_flush_tlb_all();
> +}
> +#endif
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>   

Re: [PATCH] sh: hibernation support

[Magnus Damm]

[-- Attachment #1: Type: text/plain, Size: 721 bytes --]

On Fri, Mar 6, 2009 at 4:06 PM, Francesco VIRLINZI
<francesco.virlinzi@st.com> wrote:
> Hi Magnus
> I was looking your patch and I have few question.

Great! You should try in on some ST hardware as well. =)

> Is it also for sh4 with PMB? If so how are you managing the PMB?
> Moreover what about interrupt controller? This means after a resume from
> hibernation
> you could have a resumed device (and driver) but the interrupt controller
> has the right irq-line not initialized.

I have not tested with PMB. You are right about the interrupt
controller, but it works because swsusp resume boots up the system as
usual with an identical kernel. Look at the attached file for the
swsusp code flow...

Cheers,

/ magnus

[-- Attachment #2: pm.txt --]
[-- Type: text/plain, Size: 2338 bytes --]

CONFIG_HIBERNATE in 2.6.29

suspend operation 
------------------
swsusp code                        arch code           driver callbacks

hibernate(): [disk.c]
  hibernation_snapshot()

hibernation_snapshot():
  suspend_console()
  device_suspend(PMSG_FREEZE)                          prepare(), freeze()
  disable_nonboot_cpus()
  create_image()

create_image():
  arch_prepare_suspend()           do nothing
  local_irq_disable()
  device_power_down(PMSG_FREEZE)                       freeze_noirq()
  save_processor_state()           save registers
  swsusp_arch_suspend()            call swsusp_save()

swsusp_save() [snapshot.c]
  create hibernation image

create_image(): [disk.c]
  restore_processor_state()        restore registers
  device_power_up(PMSG_THAW)                           thaw_noirq()
  local_irq_enable()
  
hibernation_snapshot()
  enable_nonboot_cpus()
  device_resume(PMSG_THAW)                             thaw(), complete()
  resume_console()

hibernate():
  swsusp_write()
  swsusp_free()
  power_down()

power_down():
  hibernation_platform_enter()

hibernation_platform_enter():
  suspend_console()
  device_suspend(PMSG_HIBERNATE)                       prepare(), poweroff()
  disable_nonboot_cpus()
  local_irq_disable()
  device_power_down(PMSG_HIBERNATE)                    poweroff_noirq()


resume operation:
-----------------
swsusp code                        arch code           driver callbacks

software_resume() [disk.c]
  swsusp_check()
  swsusp_read()
  hibernation_restore()

hibernation_restore()
  suspend_console()
  device_suspend(PMSG_QUIESCE)                         prepare(), freeze()
  disable_nonboot_cpus()
  resume_target_kernel()

resume_target_kernel()
  local_irq_disable()
  device_power_down(PMSG_QUIESCE)                      freeze_noirq()
  save_processor_state()           save registers
  restore_highmem()
  swsusp_arch_resume()             overwrite memory

  [executing old image now, back in suspend path with in_suspend = 0]

create_image():
  restore_processor_state()        restore registers
  device_power_up(PMSG_RESTORE)                        restore_noirq()
  local_irq_enable()
  
hibernation_snapshot()
  enable_nonboot_cpus()
  device_resume(PMSG_RESTORE)                          restore(), complete()
  resume_console()

hibernate():
  swsusp_free()

Re: [PATCH] sh: hibernation support

[Francesco VIRLINZI]

Hi Magnus
>
> Great! You should try in on some ST hardware as well. =)
>   
Yes I could try if it works also if we already had the hibernation on 
memory on sh4.
>   
>> Is it also for sh4 with PMB? If so how are you managing the PMB?
>> Moreover what about interrupt controller? This means after a resume from
>> hibernation
>> you could have a resumed device (and driver) but the interrupt controller
>> has the right irq-line not initialized.
>>     
>
> I have not tested with PMB.
On PMB: some entries may be is already OK (the entries the bootloader 
prepared fot linux)
 but Linux has to force an hw-initialization of the other entries
>  You are right about the interrupt
> controller, but it works because swsusp resume boots up the system as
> usual with an identical kernel. Look at the attached file for the
> swsusp code flow...
>   
You are right until you don't use module.
If you use a "mini-kernel" with several modules... when you will resume 
from hibernation at the begin
 you boots again the 'mini-kernel'... after that you restore the 
previous image and the irq line required by
 module more probably remains as it was (not-initialized).

Regards
 Francesco
> Cheers,
>
> / magnus
>   

Re: [PATCH] sh: hibernation support

[Francesco VIRLINZI]

Sorry
>>   
> >>> Yes I could try if it works also if we already had the hibernation 
> on DISK on sh4.
>>  
Regards
 Francesco

Re: [PATCH] sh: hibernation support

[Jean-Christophe PLAGNIOL-VILLARD]

On 11:05 Fri 06 Mar     , Francesco VIRLINZI wrote:
> Hi Magnus
>>
>> Great! You should try in on some ST hardware as well. =)
>>   
> Yes I could try if it works also if we already had the hibernation on  
> memory on sh4.
>>   
>>> Is it also for sh4 with PMB? If so how are you managing the PMB?
>>> Moreover what about interrupt controller? This means after a resume from
>>> hibernation
>>> you could have a resumed device (and driver) but the interrupt controller
>>> has the right irq-line not initialized.
>>>     
>>
>> I have not tested with PMB.
> On PMB: some entries may be is already OK (the entries the bootloader  
> prepared fot linux)
> but Linux has to force an hw-initialization of the other entries
the memory init will be done by u-boot so PMB will not be a problem
the other device could be init by u-boot with a different stat than the kernel
expect, so each device will have to check its state and re-init in the good
state
>>  You are right about the interrupt
>> controller, but it works because swsusp resume boots up the system as
>> usual with an identical kernel. Look at the attached file for the
>> swsusp code flow...
>>   
> You are right until you don't use module.
> If you use a "mini-kernel" with several modules... when you will resume  
> from hibernation at the begin
> you boots again the 'mini-kernel'... after that you restore the previous 
> image and the irq line required by
> module more probably remains as it was (not-initialized).
the module will have to handle this itsself
because the kernel can not known the specicifity of each device init sequence

Best Regards,
J.

Re: [PATCH] sh: hibernation support

[Paul Mundt]

On Fri, Mar 06, 2009 at 11:05:35AM +0100, Francesco VIRLINZI wrote:
> >>Is it also for sh4 with PMB? If so how are you managing the PMB?
> >>Moreover what about interrupt controller? This means after a resume from
> >>hibernation
> >>you could have a resumed device (and driver) but the interrupt controller
> >>has the right irq-line not initialized.
> >>    
> >
> >I have not tested with PMB.
> On PMB: some entries may be is already OK (the entries the bootloader 
> prepared fot linux)
> but Linux has to force an hw-initialization of the other entries

Yes, this is something we need to add in to the patch, other platforms
have similar issues with their IOMMUs for example. Overall this is quite
trivial though, we just need to add a bit of logic to the PMB management
code to register a nosave region and copy over the memory-mapped PMB
entries. PowerPC already has similar code-paths for the same sort of
thing.

Re: [PATCH] sh: hibernation support

[Paul Mundt]

On Fri, Mar 06, 2009 at 06:29:51PM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 11:05 Fri 06 Mar     , Francesco VIRLINZI wrote:
> > Hi Magnus
> >>
> >> Great! You should try in on some ST hardware as well. =)
> >>   
> > Yes I could try if it works also if we already had the hibernation on  
> > memory on sh4.
> >>   
> >>> Is it also for sh4 with PMB? If so how are you managing the PMB?
> >>> Moreover what about interrupt controller? This means after a resume from
> >>> hibernation
> >>> you could have a resumed device (and driver) but the interrupt controller
> >>> has the right irq-line not initialized.
> >>>     
> >>
> >> I have not tested with PMB.
> > On PMB: some entries may be is already OK (the entries the bootloader  
> > prepared fot linux)
> > but Linux has to force an hw-initialization of the other entries
> the memory init will be done by u-boot so PMB will not be a problem
> the other device could be init by u-boot with a different stat than the kernel
> expect, so each device will have to check its state and re-init in the good
> state

u-boot knows nothing about the PMB, and even if it did, the best it could
do is set up fixed entries for P1/P2 identity mapping and so on. If
u-boot started playing with the PMB, the first thing we would do in the
kernel would be to blow away all of the u-boot mappings and replace them
with something we can trust. The same applies to the TLB, although we
don't have to be as careful there when flushing.

The issue is not with the fixed mappings that are established at boot,
but the additional mappings that are created dynamically. As PMB entries
are not faulted, we can't simply depend on the TLB miss to take care of
re-establishing the mappings. Any sort of device with lazy initialization
or dynamic state that is not restored in the boot path needs to be
written out to the suspend image.

Re: [PATCH] sh: hibernation support

[Francesco VIRLINZI]

Hi Paul

>  PowerPC already has similar code-paths for the same sort of
> thing.
>   
I had a fix but now it doesn't match with the latest git.
Thanks for the suggestion I will see the code.
Regards
 Francesco
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>   

Re: [PATCH] sh: hibernation support

[Magnus Damm]

On Fri, Mar 6, 2009 at 7:05 PM, Francesco VIRLINZI
<francesco.virlinzi@st.com> wrote:
> Yes I could try if it works also if we already had the hibernation on memory
> on sh4.

FYI, I'm hacking on CONIFG_SUSPEND as well - will post that later this week.

> If you use a "mini-kernel" with several modules... when you will resume from
> hibernation at the begin
> you boots again the 'mini-kernel'... after that you restore the previous
> image and the irq line required by
> module more probably remains as it was (not-initialized).

Ok, good point. I'll add intc suspend/resume to my todo list!

Thanks!

/ magnus

Re: [PATCH] sh: hibernation support

[Francesco VIRLINZI]

Hi Jean-Christophe
>>>   
>>>       
>> You are right until you don't use module.
>> If you use a "mini-kernel" with several modules... when you will resume  
>> from hibernation at the begin
>> you boots again the 'mini-kernel'... after that you restore the previous 
>> image and the irq line required by
>> module more probably remains as it was (not-initialized).
>>     
> the module will have to handle this itsself
> because the kernel can not known the specicifity of each device init sequence
>   
My issue was on interrupt controller initialization.

A module loaded during runtime can do request_irq/free_irq to manage the 
irq line initialization.
I'm assuming during the suspend the module doesn't have to free the irq 
and on resume require again the same irq (just to initialize the irq 
line on the interrupt controller).

I think it's more realistic a module (the device_driver) turns-off 
its-own "irq capability" on the device (not on the interrupt 
controller), this means on resume the module (device_driver)  will 
turn-on the irq capability (again on the device)... but the irq line on 
the interrupt controller still  remain not-initialized.

I hope this clarify my view.

Regards
 Francesco

> Best Regards,
> J.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>   

Re: [PATCH] sh: hibernation support

[Francesco VIRLINZI]

Hi Magnus

I'm sorry if I'm stressing you but you will have similar problem also 
with the clock framework.
A simple
 -  clk_get_rate(...)
could return a wrong value if in the previous session someone changed 
the clock rate (from init value) and
 you don't force again in the hw the resumed "clk->rate".

Regards
 Francesco

>> If you use a "mini-kernel" with several modules... when you will resume from
>> hibernation at the begin
>> you boots again the 'mini-kernel'... after that you restore the previous
>> image and the irq line required by
>> module more probably remains as it was (not-initialized).
>>     
>
> Ok, good point. I'll add intc suspend/resume to my todo list!
>
> Thanks!
>
> / magnus
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>   

Re: [PATCH] sh: hibernation support

[Magnus Damm]

On Mon, Mar 9, 2009 at 7:03 PM, Francesco VIRLINZI
<francesco.virlinzi@st.com> wrote:
> I'm sorry if I'm stressing you but you will have similar problem also with
> the clock framework.
> A simple
> -  clk_get_rate(...)
> could return a wrong value if in the previous session someone changed the
> clock rate (from init value) and
> you don't force again in the hw the resumed "clk->rate".

Yeah, the clock framework needs more work. =)

/ magnus

Re: [PATCH] sh: hibernation support

[Paul Mundt]

On Mon, Mar 09, 2009 at 10:12:30AM +0100, Francesco VIRLINZI wrote:
> Hi Paul
> 
> > PowerPC already has similar code-paths for the same sort of
> >thing.
> >  
> I had a fix but now it doesn't match with the latest git.
> Thanks for the suggestion I will see the code.

Feel free to post what you already have, even if it doesn't apply. This
sort of stuff by nature tends to take several iterations before it's in
any shape to merge anyways.

Re: [PATCH] sh: hibernation support

[Francesco VIRLINZI]

[-- Attachment #1: Type: text/plain, Size: 799 bytes --]

Hi Magnus
In the clock framework I would propose the attached solution
Regards
 Francesco
Magnus Damm ha scritto:
> On Mon, Mar 9, 2009 at 7:03 PM, Francesco VIRLINZI
> <francesco.virlinzi@st.com> wrote:
>   
>> I'm sorry if I'm stressing you but you will have similar problem also with
>> the clock framework.
>> A simple
>> -  clk_get_rate(...)
>> could return a wrong value if in the previous session someone changed the
>> clock rate (from init value) and
>> you don't force again in the hw the resumed "clk->rate".
>>     
>
> Yeah, the clock framework needs more work. =)
>
> / magnus
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>   


[-- Attachment #2: 0003-sh_clk-Added-clks-sysdevice-to-support-hibernation.patch --]
[-- Type: text/x-patch, Size: 2544 bytes --]

From 7d2249e98d304181b3deaa806ee475873e822328 Mon Sep 17 00:00:00 2001
From: Francesco Virlinzi <francesco.virlinzi@st.com>
Date: Tue, 10 Mar 2009 10:23:23 +0100
Subject: [PATCH] sh_clk: Added clks sysdevice to support hibernation

This patch adds the clk_sysdev device to restore the right clocks
 setting after a resume from hibernation.

Signed-off-by: Francesco Virlinzi <francesco.virlinzi@st.com>
---
 arch/sh/kernel/cpu/clock.c |   62 ++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 62 insertions(+), 0 deletions(-)

diff --git a/arch/sh/kernel/cpu/clock.c b/arch/sh/kernel/cpu/clock.c
index dc53def..06fcbf0 100644
--- a/arch/sh/kernel/cpu/clock.c
+++ b/arch/sh/kernel/cpu/clock.c
@@ -20,6 +20,8 @@
 #include <linux/mutex.h>
 #include <linux/list.h>
 #include <linux/kref.h>
+#include <linux/kobject.h>
+#include <linux/sysdev.h>
 #include <linux/seq_file.h>
 #include <linux/err.h>
 #include <linux/platform_device.h>
@@ -382,6 +384,56 @@ static int show_clocks(char *buf, char **start, off_t off,
 	return p - buf;
 }
 
+static int clks_suspend(struct sys_device *dev, pm_message_t state)
+{
+	static pm_message_t prev_state;
+	struct clk *clkp;
+
+	switch (state.event) {
+	case PM_EVENT_ON:
+		/* Resumeing from hibernation */
+		if (prev_state.event == PM_EVENT_FREEZE) {
+			list_for_each_entry(clkp, &clock_list, node)
+				if (likely(clkp->ops)) {
+					if (likely(clkp->ops->set_parent))
+						clkp->ops->set_parent(clkp,
+							clkp->parent);
+					if (likely(clkp->ops->set_rate))
+						clkp->ops->set_rate(clkp,
+							clkp->rate, NO_CHANGE);
+					if (likely(clkp->ops->recalc))
+						clkp->ops->recalc(clkp);
+					}
+		}
+		break;
+	case PM_EVENT_FREEZE:
+		break;
+	case PM_EVENT_SUSPEND:
+		break;
+	}
+
+	prev_state = state;
+	return 0;
+}
+
+static int clks_resume(struct sys_device *dev)
+{
+	return clks_suspend(dev, PMSG_ON);
+}
+
+static struct sysdev_class clks_class = {
+	.kset.kobj.name = "clks",
+};
+
+static struct sysdev_driver clks_driver = {
+	.suspend = clks_suspend,
+	.resume = clks_resume,
+};
+
+static struct sys_device clks_dev = {
+	.cls = &clks_class,
+};
+
 int __init clk_init(void)
 {
 	int i, ret = 0;
@@ -404,6 +456,16 @@ int __init clk_init(void)
 	return ret;
 }
 
+static int __init clk_sysdev_init(void)
+{
+
+	sysdev_class_register(&clks_class);
+	sysdev_driver_register(&clks_class, &clks_driver);
+	sysdev_register(&clks_dev);
+	return 0;
+}
+subsys_initcall(clk_sysdev_init);
+
 static int __init clk_proc_init(void)
 {
 	struct proc_dir_entry *p;
-- 
1.5.6.6

Re: [PATCH] sh: hibernation support

[Magnus Damm]

[-- Attachment #1: Type: text/plain, Size: 354 bytes --]

Hi Francesco,

On Tue, Mar 10, 2009 at 10:19 PM, Francesco VIRLINZI
<francesco.virlinzi@st.com> wrote:
> Hi Magnus
> In the clock framework I would propose the attached solution

Thanks for your help! I tested your patch but I need the attached fix.

Can you please include my fix and resubmit inline? Feel free to add an
ack from me.

Thanks!

/ magnus

[-- Attachment #2: 0003-sh_clk-Added-clks-sysdevice-to-support-hibernation-fix.patch --]
[-- Type: text/x-patch, Size: 583 bytes --]

--- 0003/arch/sh/kernel/cpu/clock.c
+++ work/arch/sh/kernel/cpu/clock.c	2009-03-11 12:36:44.000000000 +0900
@@ -369,7 +369,7 @@ static int clks_resume(struct sys_device
 }
 
 static struct sysdev_class clks_class = {
-	.kset.kobj.name = "clks",
+	.name = "clks",
 };
 
 static struct sysdev_driver clks_driver = {
@@ -405,10 +405,10 @@ int __init clk_init(void)
 
 static int __init clk_sysdev_init(void)
 {
-
 	sysdev_class_register(&clks_class);
 	sysdev_driver_register(&clks_class, &clks_driver);
 	sysdev_register(&clks_dev);
+
 	return 0;
 }
 subsys_initcall(clk_sysdev_init);

Re: [PATCH] sh: hibernation support

[Francesco VIRLINZI]

Hi Magnus.

> Thanks for your help! I tested your patch but I need the attached fix.
>
> Can you please include my fix and resubmit inline? Feel free to add an
> ack from me.
>   
Ok.
While we are 'fixing' the clock framework on resume from hibernation.
I'd like to do some similar thing on suspend/resume from standby.
I was thinking to add some flags/capability able to say to the framework 
what it has to be done in standby.
Something like:

#define CLK_TURNOFF_ON_SUSPEND   ....
#define CLK_REDUCE_ON_SUSPEND ...

In this manner in the single file where the clock tree is described we 
have a clear view of all the scenario
 (normal runtime, suspend, hibernation)...
Moreover this should not impact "medium" power level.
Regards
 Francesco

Re: [PATCH] sh: hibernation support

[Magnus Damm]

Hi again Francesco,

On Wed, Mar 11, 2009 at 3:50 PM, Francesco VIRLINZI
<francesco.virlinzi@st.com> wrote:
> While we are 'fixing' the clock framework on resume from hibernation.
> I'd like to do some similar thing on suspend/resume from standby.

Good plan! I'd like to have this working for suspend as well.

> I was thinking to add some flags/capability able to say to the framework
> what it has to be done in standby.
> Something like:
>
> #define CLK_TURNOFF_ON_SUSPEND   ....
> #define CLK_REDUCE_ON_SUSPEND ...
>
> In this manner in the single file where the clock tree is described we have
> a clear view of all the scenario
> (normal runtime, suspend, hibernation)...
> Moreover this should not impact "medium" power level.

I guess your goal is to minimize power consumption by turning off
unused clocks or at least set them to a minimal value before
suspending. I'd like to do something like that at least. For the clock
framework i think we should simply minimize the clock when the usage
count drops to zero. And let the drivers disable the clock as long as
it's not needed for wakeup.

If you're talking about frequency control then maybe cpufreq is a good
option. I'm not sure about this yet, it's still on my TODO list.

For the overall system my gut feeling is that it would be useful to
define a set of standby modes, and let the suspend code or cpuidle
enter the best mode possible after checking dependencies.

Right now I have the following modes setup for SuperH Mobile:

#define SUSP_MODE_SLEEP                (SUSP_SH_SLEEP)
#define SUSP_MODE_SLEEP_SF     (SUSP_SH_SLEEP | SUSP_SH_SF)
#define SUSP_MODE_STANDBY_SF   (SUSP_SH_STANDBY | SUSP_SH_SF)

The first is just plain old light sleep using the sleep instruction -
nothing exciting.

The second is a bit better since it puts the system ram in
self-refresh mode but the processor is still sleeping lightly. Before
entering this mode the code needs to make sure that no hardware is
using the system memory, ie we cannot enter this if some hardware is
bus mastering. We can however enter this mode waiting for incoming
data on a SCIF.

The third mode turns off the clock so system ram self-refresh is
needed. We cannot enter this mode if hardware blocks that use the
clocks are active. Or if we need the clock enabled to be able to
wakeup. =)

There is one additional level that turns off the power to the hardware
blocks as well. This mode is close to hibernation from a software
point of view. I guess a good match is to use dev_pm_ops->freeze to
save hardware state before suspending and use thaw to restore state
when resuming. Obviously we can't power off hardware blocks needed for
wakeup.

How is this compared to your architecture(s)?
Are you using cpuidle?
What about cpufreq?
Do you have some profiles setup for run time power management today?

Cheers,

/ magnus

Re: [PATCH] sh: hibernation support

[Francesco VIRLINZI]

Hi Magnus
>
> Good plan! I'd like to have this working for suspend as well.
Good.
>
> I guess your goal is to minimize power consumption by turning off
> unused clocks or at least set them to a minimal value before
> suspending.
Yes.
>  I'd like to do something like that at least. For the clock
> framework i think we should simply minimize the clock when the usage
> count drops to zero. And let the drivers disable the clock as long as
> it's not needed for wakeup.
Unfortunately it isn't so easy.
If we have one-to-one clock per device a simple counter is enough, but 
if you have shared clocks (i.e. a clock X shared between a two wakeup 
devices than on suspend the first device could asks "clock_X @ xx MHz" 
the second device could asks "clock_X @ yy MHz"  and as platform device  
the final rate of clock X will be based on how the devices were 
registered...
Moreover without a notification of the rate of each clock a wakeup 
device will be broken and it doesn't know it is broken.

> ...
> For the overall system my gut feeling is that it would be useful to
> define a set of standby modes, and let the suspend code or cpuidle
> enter the best mode possible after checking dependencies.
Not so easy for us because we have some A-SMP therefore an "idle linux" 
doesn't mean an idle hardware.
>
> Right now I have the following modes setup for SuperH Mobile:
>
> #define SUSP_MODE_SLEEP                (SUSP_SH_SLEEP)
> #define SUSP_MODE_SLEEP_SF     (SUSP_SH_SLEEP | SUSP_SH_SF)
> #define SUSP_MODE_STANDBY_SF   (SUSP_SH_STANDBY | SUSP_SH_SF)
>
>
> There is one additional level that turns off the power to the hardware
> blocks as well. This mode is close to hibernation from a software
> point of view.
Yes also us we plan something like that.
>  I guess a good match is to use dev_pm_ops->freeze to
> save hardware state before suspending and use thaw to restore state
> when resuming.

>  Obviously we can't power off hardware blocks needed for
> wakeup.
>
> How is this compared to your architecture(s)?
On  SUSP_MODE_SLEEP_SF and SUSP_MODE_STANDBY_SF.
If I understood you are speaking on 'sleep' and 'deep sleep' capability 
of sh4. In this case in the ST40 this difference was removed.
But the behavioral is similar (i.e.: mem in self-refresh, clocks 
disabled/reduced... wakeup devices still able to works).

> Are you using cpuidle?
Not currently and I think it will be not so easy to use for us.
> What about cpufreq?
Supported.
> Do you have some profiles setup for run time power management today?
We have a kernel tool to create "power profile" sits on top of 
cpufreq-clockfm-device model and until possible (currently possible) it 
doesn't touch any device driver; this means the device aren't aware of a 
change in term of power profile... they see change in term of 
clock_rate-device_state.

Regards
 Francesco

Re: [PATCH] sh: hibernation support

[Magnus Damm]

On Wed, Mar 11, 2009 at 10:20 PM, Francesco VIRLINZI
<francesco.virlinzi@st.com> wrote:
>>  I'd like to do something like that at least. For the clock
>> framework i think we should simply minimize the clock when the usage
>> count drops to zero. And let the drivers disable the clock as long as
>> it's not needed for wakeup.
>
> Unfortunately it isn't so easy.
> If we have one-to-one clock per device a simple counter is enough, but if
> you have shared clocks (i.e. a clock X shared between a two wakeup devices
> than on suspend the first device could asks "clock_X @ xx MHz" the second
> device could asks "clock_X @ yy MHz"  and as platform device  the final rate
> of clock X will be based on how the devices were registered...

I understand the problem with the ordering. I wonder if converting
your shared clock into one parent clock and two children would help?
Each device connects to one child clock, and when requesting a clock
rate this rate gets stored in each child clock. The parent simply
determines the clock by MAX(c1, c2) where c1 and c2 are the child
clock rates.

> Moreover without a notification of the rate of each clock a wakeup device
> will be broken and it doesn't know it is broken.

Yeah, notification is needed.

>> ...
>> For the overall system my gut feeling is that it would be useful to
>> define a set of standby modes, and let the suspend code or cpuidle
>> enter the best mode possible after checking dependencies.
>
> Not so easy for us because we have some A-SMP therefore an "idle linux"
> doesn't mean an idle hardware.

Sure, but someone needs to be in control of power management. Even on
single core systems "idle linux" doesn't mean idle hardware. Say that
some hardware engine is programmed to crunch some data. The cpu is
idle. We cannot enter deep sleep because the hardware engine needs the
clock. But after the hardware engine is done it will issue an
interrupt, and the processor can wake up, do it's thing and go back to
sleep - deep sleep this time since the hardware engine isn't active.

I think the case above is very similar to multi core power management.
If you have time, please explain more about issues you've experienced
related to A-SMP.

In my mind, at least each independent domain needs some central point
(master) of power management. So if some processor would be a slave,
how about notifying the master before sleeping and let the master
handle the power management. This action may be the last part of some
application processor code where the master writes commands to an
mailbox and receives the results and get an interrupt as notification.

In the case above the application processor can sleep after that and
the power management code in the main processor will know this state.
I guess it becomes more difficult if the application processor
receives an interrupt from some other source and wakes up, but the
main processor needs to take the system ram out of self-refresh before
the application processor can start serving the interrupt..

> If I understood you are speaking on 'sleep' and 'deep sleep' capability of
> sh4. In this case in the ST40 this difference was removed.
> But the behavioral is similar (i.e.: mem in self-refresh, clocks
> disabled/reduced... wakeup devices still able to works).

The states that I mentioned are from SuperH Mobile. "sleep" and "deep
sleep" are black and white terms in a very gray world, so I should do
my best to stay away from them in the future. Other SH devices from
Renesas also do different levels of power management. In the end I
think this stuff is pretty standard all over the place, except the
clock dependencies that differ from architecture to architecture and
between processor families.

>> Are you using cpuidle?
>
> Not currently and I think it will be not so easy to use for us.

Because of A-SMP?
I have a cpuidle prototype running, but I need to improve the clock
framework code to track dependencies better before I can integrate it
upstream.

>> What about cpufreq?
>
> Supported.

Still on my TODO-list. The SuperH Mobile devices have quite a few
restrictions on which divider combinations that are allowed for the
system clocks. So if someone changes the processor clock for instance,
many other clocks may need to follow.

>> Do you have some profiles setup for run time power management today?
>
> We have a kernel tool to create "power profile" sits on top of
> cpufreq-clockfm-device model and until possible (currently possible) it
> doesn't touch any device driver; this means the device aren't aware of a
> change in term of power profile... they see change in term of
> clock_rate-device_state.

I see. Some partial system suspend (some devices suspended only) would
be good to have, don't you think?

Thanks for your comments!

/ magnus

Re: [PATCH] sh: hibernation support

[Francesco VIRLINZI]

Hi Magnus
>
> Yeah, notification is needed.
... Yes unfortunately...
>
>>> Are you using cpuidle?
>> Not currently and I think it will be not so easy to use for us.
>
> Because of A-SMP?
Yes
> I have a cpuidle prototype running, but I need to improve the clock
> framework code to track dependencies better before I can integrate it
> upstream.
Interesting when available I will see it.
>
>>> What about cpufreq?
>> Supported.
>
> Still on my TODO-list. The SuperH Mobile devices have quite a few
> restrictions on which divider combinations that are allowed for the
> system clocks. So if someone changes the processor clock for instance,
> many other clocks may need to follow.
Yes I understand... the famous sh4_clk/sh4_per/module_clk ratios..) ...
We still have some platform with these constraints...
What I can say you is that the timer-tmu.c is already ready for this kind
 of operation and you will have no impact on the system timer or No_HZ 
system.

>
> I see. Some partial system suspend (some devices suspended only) would
> be good to have, don't you think?
Yes I agree. But I prefer a tool to leave the people free to create 
their own 'profile'.

Ciao
 Francesco