sh: hibernation support

Commit Message

Magnus Damm March 6, 2009, 6:41 a.m. UTC

From: Magnus Damm <[e-mail]>

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 <[e-mail]>
---

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

--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Paul Mundt March 6, 2009, 6:57 a.m. UTC | #1

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?
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Francesco VIRLINZI March 6, 2009, 7:06 a.m. UTC | #2

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 <[e-mail]>
>
> 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 <[e-mail]>
> ---
>
>  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 [e-mail]
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>   

--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Magnus Damm March 6, 2009, 9:53 a.m. UTC | #3

On Fri, Mar 6, 2009 at 4:06 PM, Francesco VIRLINZI
<[e-mail]> 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
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()

Francesco VIRLINZI March 6, 2009, 10:05 a.m. UTC | #4

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
>   

--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Francesco VIRLINZI March 6, 2009, 10:17 a.m. UTC | #5

Sorry
>>   
> >>> Yes I could try if it works also if we already had the hibernation 
> on DISK on sh4.
>>  
Regards
 Francesco
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Jean-Christophe PLAGNIOL-VILLARD March 6, 2009, 5:29 p.m. UTC | #6

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.
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Paul Mundt March 7, 2009, 6:12 a.m. UTC | #7

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.
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Paul Mundt March 7, 2009, 6:20 a.m. UTC | #8

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.
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Francesco VIRLINZI March 9, 2009, 9:12 a.m. UTC | #9

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 [e-mail]
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>   

--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Magnus Damm March 9, 2009, 9:16 a.m. UTC | #10

On Fri, Mar 6, 2009 at 7:05 PM, Francesco VIRLINZI
<[e-mail]> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Francesco VIRLINZI March 9, 2009, 9:27 a.m. UTC | #11

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 [e-mail]
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>   

--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Francesco VIRLINZI March 9, 2009, 10:03 a.m. UTC | #12

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 [e-mail]
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>   

--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Magnus Damm March 9, 2009, 10:57 a.m. UTC | #13

On Mon, Mar 9, 2009 at 7:03 PM, Francesco VIRLINZI
<[e-mail]> 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 [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Paul Mundt March 9, 2009, 5:35 p.m. UTC | #14

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.
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

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