[PATCH v2] powerpc: Add hibernation support for FSL BookE processors

[PATCH v2] powerpc: Add hibernation support for FSL BookE processors

[Anton Vorontsov]

This is started as swsusp_32.S modifications, but the amount of #ifdefs
made the whole file horribly unreadable, so let's put the support into
its own separate file.

The code should be relatively easy to modify to support 44x BookEs as
well, but since I don't have any 44x to test, let's confine the code to
FSL BookE. (The only FSL-specific parts are 'flush_dcache_L1' and TLB
invalidation code).

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---

Sorry for the delayed response...

On Tue, Jan 12, 2010 at 01:34:08PM +1100, Benjamin Herrenschmidt wrote:
[...]
> Here's a quick review. Looks good but two things:
> 
>  - Please make it swsusp_booke.c, 44x support is trivial and I don't
> want to rename the file :-)

Done.

>  - Is there really an SDR1 register on FSL BookE ? It's supposed to be
> the pointer to the hash table on server ...

Thanks, fixed.

>  - You probably should save/restore the TCR and ack pending crap DEC or
> FIT interrupts in the TSR right before you kick the decrementer

Done.

>  - Nowadays, we still assume that the "loader" kernel is exactly the
> same as the "loaded" kernel on resume ?

I'm pretty sure today we do rely on this, yes. This is not some
generic code limitation though, it's just hard to test the case
when loader != loaded. In most cases it will work fine since the
loader kernel wouldn't differ a lot from the loaded kernel, so
it'll setup the low level stuff the same way.

We may try to link the loader kernel to a different address
(relocate it as in kdump case), and hope that it'll trigger
all sort of problems so that we could fix them.

Though, the better test case would be to resume the hibernated
kernel directly from the bootloader.

Thanks,

 arch/powerpc/kernel/Makefile       |    8 +-
 arch/powerpc/kernel/swsusp_booke.S |  200 ++++++++++++++++++++++++++++++++++++
 2 files changed, 206 insertions(+), 2 deletions(-)
 create mode 100644 arch/powerpc/kernel/swsusp_booke.S

diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index c002b04..33d9dda 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -57,8 +57,12 @@ obj-$(CONFIG_CRASH_DUMP)	+= crash_dump.o
 obj-$(CONFIG_E500)		+= idle_e500.o
 obj-$(CONFIG_6xx)		+= idle_6xx.o l2cr_6xx.o cpu_setup_6xx.o
 obj-$(CONFIG_TAU)		+= tau_6xx.o
-obj-$(CONFIG_HIBERNATION)	+= swsusp.o suspend.o \
-				   swsusp_$(CONFIG_WORD_SIZE).o
+obj-$(CONFIG_HIBERNATION)	+= swsusp.o suspend.o
+ifeq ($(CONFIG_FSL_BOOKE),y)
+obj-$(CONFIG_HIBERNATION)	+= swsusp_booke.o
+else
+obj-$(CONFIG_HIBERNATION)	+= swsusp_$(CONFIG_WORD_SIZE).o
+endif
 obj64-$(CONFIG_HIBERNATION)	+= swsusp_asm64.o
 obj-$(CONFIG_MODULES)		+= module.o module_$(CONFIG_WORD_SIZE).o
 obj-$(CONFIG_44x)		+= cpu_setup_44x.o
diff --git a/arch/powerpc/kernel/swsusp_booke.S b/arch/powerpc/kernel/swsusp_booke.S
new file mode 100644
index 0000000..dbffad2
--- /dev/null
+++ b/arch/powerpc/kernel/swsusp_booke.S
@@ -0,0 +1,200 @@
+/*
+ * Based on swsusp_32.S, modified for FSL BookE by
+ * Anton Vorontsov <avorontsov@ru.mvista.com>
+ * Copyright (c) 2009-2010 MontaVista Software, LLC.
+ */
+
+#include <linux/threads.h>
+#include <asm/processor.h>
+#include <asm/page.h>
+#include <asm/cputable.h>
+#include <asm/thread_info.h>
+#include <asm/ppc_asm.h>
+#include <asm/asm-offsets.h>
+#include <asm/mmu.h>
+
+/*
+ * Structure for storing CPU registers on the save area.
+ */
+#define SL_SP		0
+#define SL_PC		4
+#define SL_MSR		8
+#define SL_TCR		0xc
+#define SL_SPRG0	0x10
+#define SL_SPRG1	0x14
+#define SL_SPRG2	0x18
+#define SL_SPRG3	0x1c
+#define SL_SPRG4	0x20
+#define SL_SPRG5	0x24
+#define SL_SPRG6	0x28
+#define SL_SPRG7	0x2c
+#define SL_TBU		0x30
+#define SL_TBL		0x34
+#define SL_R2		0x38
+#define SL_CR		0x3c
+#define SL_LR		0x40
+#define SL_R12		0x44	/* r12 to r31 */
+#define SL_SIZE		(SL_R12 + 80)
+
+	.section .data
+	.align	5
+
+_GLOBAL(swsusp_save_area)
+	.space	SL_SIZE
+
+
+	.section .text
+	.align	5
+
+_GLOBAL(swsusp_arch_suspend)
+	lis	r11,swsusp_save_area@h
+	ori	r11,r11,swsusp_save_area@l
+
+	mflr	r0
+	stw	r0,SL_LR(r11)
+	mfcr	r0
+	stw	r0,SL_CR(r11)
+	stw	r1,SL_SP(r11)
+	stw	r2,SL_R2(r11)
+	stmw	r12,SL_R12(r11)
+
+	/* Save MSR & TCR */
+	mfmsr	r4
+	stw	r4,SL_MSR(r11)
+	mfspr	r4,SPRN_TCR
+	stw	r4,SL_TCR(r11)
+
+	/* Get a stable timebase and save it */
+1:	mftbu	r4
+	stw	r4,SL_TBU(r11)
+	mftb	r5
+	stw	r5,SL_TBL(r11)
+	mftbu	r3
+	cmpw	r3,r4
+	bne	1b
+
+	/* Save SPRGs */
+	mfsprg	r4,0
+	stw	r4,SL_SPRG0(r11)
+	mfsprg	r4,1
+	stw	r4,SL_SPRG1(r11)
+	mfsprg	r4,2
+	stw	r4,SL_SPRG2(r11)
+	mfsprg	r4,3
+	stw	r4,SL_SPRG3(r11)
+	mfsprg	r4,4
+	stw	r4,SL_SPRG4(r11)
+	mfsprg	r4,5
+	stw	r4,SL_SPRG5(r11)
+	mfsprg	r4,6
+	stw	r4,SL_SPRG6(r11)
+	mfsprg	r4,7
+	stw	r4,SL_SPRG7(r11)
+
+	/* Call the low level suspend stuff (we should probably have made
+	 * a stackframe...
+	 */
+	bl	swsusp_save
+
+	/* Restore LR from the save area */
+	lis	r11,swsusp_save_area@h
+	ori	r11,r11,swsusp_save_area@l
+	lwz	r0,SL_LR(r11)
+	mtlr	r0
+
+	blr
+
+_GLOBAL(swsusp_arch_resume)
+ 	sync
+
+	/* Load ptr the list of pages to copy in r3 */
+	lis	r11,(restore_pblist)@h
+	ori	r11,r11,restore_pblist@l
+	lwz	r3,0(r11)
+
+	/* Copy the pages. This is a very basic implementation, to
+	 * be replaced by something more cache efficient */
+1:
+	li	r0,256
+	mtctr	r0
+	lwz	r5,pbe_address(r3)	/* source */
+	lwz	r6,pbe_orig_address(r3)	/* destination */
+2:
+	lwz	r8,0(r5)
+	lwz	r9,4(r5)
+	lwz	r10,8(r5)
+	lwz	r11,12(r5)
+	addi	r5,r5,16
+	stw	r8,0(r6)
+	stw	r9,4(r6)
+	stw	r10,8(r6)
+	stw	r11,12(r6)
+	addi	r6,r6,16
+	bdnz	2b
+	lwz	r3,pbe_next(r3)
+	cmpwi	0,r3,0
+	bne	1b
+
+	bl flush_dcache_L1
+	bl flush_instruction_cache
+
+	lis	r11,swsusp_save_area@h
+	ori	r11,r11,swsusp_save_area@l
+
+	lwz	r4,SL_SPRG0(r11)
+	mtsprg	0,r4
+	lwz	r4,SL_SPRG1(r11)
+	mtsprg	1,r4
+	lwz	r4,SL_SPRG2(r11)
+	mtsprg	2,r4
+	lwz	r4,SL_SPRG3(r11)
+	mtsprg	3,r4
+	lwz	r4,SL_SPRG4(r11)
+	mtsprg	4,r4
+	lwz	r4,SL_SPRG5(r11)
+	mtsprg	5,r4
+	lwz	r4,SL_SPRG6(r11)
+	mtsprg	6,r4
+	lwz	r4,SL_SPRG7(r11)
+	mtsprg	7,r4
+
+	/* Invalidate TLB0 & TLB1 */
+	li	r6,0x04
+	tlbivax 0,r6
+	TLBSYNC
+	li	r6,0x0c
+	tlbivax 0,r6
+	TLBSYNC
+
+	/* restore the MSR */
+	lwz	r3,SL_MSR(r11)
+
+	/* Restore TB */
+	li	r3,0
+	mttbl	r3
+	lwz	r3,SL_TBU(r11)
+	lwz	r4,SL_TBL(r11)
+	mttbu	r3
+	mttbl	r4
+
+	/* Restore TCR and clear any pending bits in TSR. */
+	lwz	r4,SL_TCR(r11)
+	mtspr	SPRN_TCR,r4
+	lis	r4, (TSR_ENW | TSR_WIS | TSR_DIS | TSR_FIS)@h
+	mtspr	SPRN_TSR,r4
+
+	/* Kick decrementer */
+	li	r0,1
+	mtdec	r0
+
+	/* Restore the callee-saved registers and return */
+	lwz	r0,SL_CR(r11)
+	mtcr	r0
+	lwz	r2,SL_R2(r11)
+	lmw	r12,SL_R12(r11)
+	lwz	r1,SL_SP(r11)
+	lwz	r0,SL_LR(r11)
+	mtlr	r0
+
+	li	r3,0
+	blr
-- 
1.7.0

Re: [PATCH v2] powerpc: Add hibernation support for FSL BookE processors

[Anton Vorontsov]

On Thu, Feb 25, 2010 at 12:38:02AM +0300, Anton Vorontsov wrote:
> This is started as swsusp_32.S modifications, but the amount of #ifdefs
> made the whole file horribly unreadable, so let's put the support into
> its own separate file.
> 
> The code should be relatively easy to modify to support 44x BookEs as
> well, but since I don't have any 44x to test, let's confine the code to
> FSL BookE. (The only FSL-specific parts are 'flush_dcache_L1' and TLB
> invalidation code).
> 
> Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
> ---
> 
> Sorry for the delayed response...
> 
> On Tue, Jan 12, 2010 at 01:34:08PM +1100, Benjamin Herrenschmidt wrote:
> [...]
> > Here's a quick review. Looks good but two things:
> > 
> >  - Please make it swsusp_booke.c, 44x support is trivial and I don't
> > want to rename the file :-)
> 
> Done.
> 
> >  - Is there really an SDR1 register on FSL BookE ? It's supposed to be
> > the pointer to the hash table on server ...
> 
> Thanks, fixed.
> 
> >  - You probably should save/restore the TCR and ack pending crap DEC or
> > FIT interrupts in the TSR right before you kick the decrementer
> 
> Done.
> 
> >  - Nowadays, we still assume that the "loader" kernel is exactly the
> > same as the "loaded" kernel on resume ?
> 
> I'm pretty sure today we do rely on this, yes. This is not some
> generic code limitation though, it's just hard to test the case
> when loader != loaded. In most cases it will work fine since the
> loader kernel wouldn't differ a lot from the loaded kernel, so
> it'll setup the low level stuff the same way.
> 
> We may try to link the loader kernel to a different address
> (relocate it as in kdump case), and hope that it'll trigger
> all sort of problems so that we could fix them.
> 
> Though, the better test case would be to resume the hibernated
> kernel directly from the bootloader.

Kumar,

According to patchwork, this is now delegated to you. Do you
have any objections to merge this?

Thanks!

>  arch/powerpc/kernel/Makefile       |    8 +-
>  arch/powerpc/kernel/swsusp_booke.S |  200 ++++++++++++++++++++++++++++++++++++
>  2 files changed, 206 insertions(+), 2 deletions(-)
>  create mode 100644 arch/powerpc/kernel/swsusp_booke.S
> 
> diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
> index c002b04..33d9dda 100644
> --- a/arch/powerpc/kernel/Makefile
> +++ b/arch/powerpc/kernel/Makefile
> @@ -57,8 +57,12 @@ obj-$(CONFIG_CRASH_DUMP)	+= crash_dump.o
>  obj-$(CONFIG_E500)		+= idle_e500.o
>  obj-$(CONFIG_6xx)		+= idle_6xx.o l2cr_6xx.o cpu_setup_6xx.o
>  obj-$(CONFIG_TAU)		+= tau_6xx.o
> -obj-$(CONFIG_HIBERNATION)	+= swsusp.o suspend.o \
> -				   swsusp_$(CONFIG_WORD_SIZE).o
> +obj-$(CONFIG_HIBERNATION)	+= swsusp.o suspend.o
> +ifeq ($(CONFIG_FSL_BOOKE),y)
> +obj-$(CONFIG_HIBERNATION)	+= swsusp_booke.o
> +else
> +obj-$(CONFIG_HIBERNATION)	+= swsusp_$(CONFIG_WORD_SIZE).o
> +endif
>  obj64-$(CONFIG_HIBERNATION)	+= swsusp_asm64.o
>  obj-$(CONFIG_MODULES)		+= module.o module_$(CONFIG_WORD_SIZE).o
>  obj-$(CONFIG_44x)		+= cpu_setup_44x.o
> diff --git a/arch/powerpc/kernel/swsusp_booke.S b/arch/powerpc/kernel/swsusp_booke.S
> new file mode 100644
> index 0000000..dbffad2
> --- /dev/null
> +++ b/arch/powerpc/kernel/swsusp_booke.S
> @@ -0,0 +1,200 @@
> +/*
> + * Based on swsusp_32.S, modified for FSL BookE by
> + * Anton Vorontsov <avorontsov@ru.mvista.com>
> + * Copyright (c) 2009-2010 MontaVista Software, LLC.
> + */
> +
> +#include <linux/threads.h>
> +#include <asm/processor.h>
> +#include <asm/page.h>
> +#include <asm/cputable.h>
> +#include <asm/thread_info.h>
> +#include <asm/ppc_asm.h>
> +#include <asm/asm-offsets.h>
> +#include <asm/mmu.h>
> +
> +/*
> + * Structure for storing CPU registers on the save area.
> + */
> +#define SL_SP		0
> +#define SL_PC		4
> +#define SL_MSR		8
> +#define SL_TCR		0xc
> +#define SL_SPRG0	0x10
> +#define SL_SPRG1	0x14
> +#define SL_SPRG2	0x18
> +#define SL_SPRG3	0x1c
> +#define SL_SPRG4	0x20
> +#define SL_SPRG5	0x24
> +#define SL_SPRG6	0x28
> +#define SL_SPRG7	0x2c
> +#define SL_TBU		0x30
> +#define SL_TBL		0x34
> +#define SL_R2		0x38
> +#define SL_CR		0x3c
> +#define SL_LR		0x40
> +#define SL_R12		0x44	/* r12 to r31 */
> +#define SL_SIZE		(SL_R12 + 80)
> +
> +	.section .data
> +	.align	5
> +
> +_GLOBAL(swsusp_save_area)
> +	.space	SL_SIZE
> +
> +
> +	.section .text
> +	.align	5
> +
> +_GLOBAL(swsusp_arch_suspend)
> +	lis	r11,swsusp_save_area@h
> +	ori	r11,r11,swsusp_save_area@l
> +
> +	mflr	r0
> +	stw	r0,SL_LR(r11)
> +	mfcr	r0
> +	stw	r0,SL_CR(r11)
> +	stw	r1,SL_SP(r11)
> +	stw	r2,SL_R2(r11)
> +	stmw	r12,SL_R12(r11)
> +
> +	/* Save MSR & TCR */
> +	mfmsr	r4
> +	stw	r4,SL_MSR(r11)
> +	mfspr	r4,SPRN_TCR
> +	stw	r4,SL_TCR(r11)
> +
> +	/* Get a stable timebase and save it */
> +1:	mftbu	r4
> +	stw	r4,SL_TBU(r11)
> +	mftb	r5
> +	stw	r5,SL_TBL(r11)
> +	mftbu	r3
> +	cmpw	r3,r4
> +	bne	1b
> +
> +	/* Save SPRGs */
> +	mfsprg	r4,0
> +	stw	r4,SL_SPRG0(r11)
> +	mfsprg	r4,1
> +	stw	r4,SL_SPRG1(r11)
> +	mfsprg	r4,2
> +	stw	r4,SL_SPRG2(r11)
> +	mfsprg	r4,3
> +	stw	r4,SL_SPRG3(r11)
> +	mfsprg	r4,4
> +	stw	r4,SL_SPRG4(r11)
> +	mfsprg	r4,5
> +	stw	r4,SL_SPRG5(r11)
> +	mfsprg	r4,6
> +	stw	r4,SL_SPRG6(r11)
> +	mfsprg	r4,7
> +	stw	r4,SL_SPRG7(r11)
> +
> +	/* Call the low level suspend stuff (we should probably have made
> +	 * a stackframe...
> +	 */
> +	bl	swsusp_save
> +
> +	/* Restore LR from the save area */
> +	lis	r11,swsusp_save_area@h
> +	ori	r11,r11,swsusp_save_area@l
> +	lwz	r0,SL_LR(r11)
> +	mtlr	r0
> +
> +	blr
> +
> +_GLOBAL(swsusp_arch_resume)
> + 	sync
> +
> +	/* Load ptr the list of pages to copy in r3 */
> +	lis	r11,(restore_pblist)@h
> +	ori	r11,r11,restore_pblist@l
> +	lwz	r3,0(r11)
> +
> +	/* Copy the pages. This is a very basic implementation, to
> +	 * be replaced by something more cache efficient */
> +1:
> +	li	r0,256
> +	mtctr	r0
> +	lwz	r5,pbe_address(r3)	/* source */
> +	lwz	r6,pbe_orig_address(r3)	/* destination */
> +2:
> +	lwz	r8,0(r5)
> +	lwz	r9,4(r5)
> +	lwz	r10,8(r5)
> +	lwz	r11,12(r5)
> +	addi	r5,r5,16
> +	stw	r8,0(r6)
> +	stw	r9,4(r6)
> +	stw	r10,8(r6)
> +	stw	r11,12(r6)
> +	addi	r6,r6,16
> +	bdnz	2b
> +	lwz	r3,pbe_next(r3)
> +	cmpwi	0,r3,0
> +	bne	1b
> +
> +	bl flush_dcache_L1
> +	bl flush_instruction_cache
> +
> +	lis	r11,swsusp_save_area@h
> +	ori	r11,r11,swsusp_save_area@l
> +
> +	lwz	r4,SL_SPRG0(r11)
> +	mtsprg	0,r4
> +	lwz	r4,SL_SPRG1(r11)
> +	mtsprg	1,r4
> +	lwz	r4,SL_SPRG2(r11)
> +	mtsprg	2,r4
> +	lwz	r4,SL_SPRG3(r11)
> +	mtsprg	3,r4
> +	lwz	r4,SL_SPRG4(r11)
> +	mtsprg	4,r4
> +	lwz	r4,SL_SPRG5(r11)
> +	mtsprg	5,r4
> +	lwz	r4,SL_SPRG6(r11)
> +	mtsprg	6,r4
> +	lwz	r4,SL_SPRG7(r11)
> +	mtsprg	7,r4
> +
> +	/* Invalidate TLB0 & TLB1 */
> +	li	r6,0x04
> +	tlbivax 0,r6
> +	TLBSYNC
> +	li	r6,0x0c
> +	tlbivax 0,r6
> +	TLBSYNC
> +
> +	/* restore the MSR */
> +	lwz	r3,SL_MSR(r11)
> +
> +	/* Restore TB */
> +	li	r3,0
> +	mttbl	r3
> +	lwz	r3,SL_TBU(r11)
> +	lwz	r4,SL_TBL(r11)
> +	mttbu	r3
> +	mttbl	r4
> +
> +	/* Restore TCR and clear any pending bits in TSR. */
> +	lwz	r4,SL_TCR(r11)
> +	mtspr	SPRN_TCR,r4
> +	lis	r4, (TSR_ENW | TSR_WIS | TSR_DIS | TSR_FIS)@h
> +	mtspr	SPRN_TSR,r4
> +
> +	/* Kick decrementer */
> +	li	r0,1
> +	mtdec	r0
> +
> +	/* Restore the callee-saved registers and return */
> +	lwz	r0,SL_CR(r11)
> +	mtcr	r0
> +	lwz	r2,SL_R2(r11)
> +	lmw	r12,SL_R12(r11)
> +	lwz	r1,SL_SP(r11)
> +	lwz	r0,SL_LR(r11)
> +	mtlr	r0
> +
> +	li	r3,0
> +	blr
> -- 
> 1.7.0

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

Re: [PATCH v2] powerpc: Add hibernation support for FSL BookE processors

[Kumar Gala]

On Apr 15, 2010, at 1:45 PM, Anton Vorontsov wrote:

> On Thu, Feb 25, 2010 at 12:38:02AM +0300, Anton Vorontsov wrote:
>> This is started as swsusp_32.S modifications, but the amount of #ifdefs
>> made the whole file horribly unreadable, so let's put the support into
>> its own separate file.
>> 
>> The code should be relatively easy to modify to support 44x BookEs as
>> well, but since I don't have any 44x to test, let's confine the code to
>> FSL BookE. (The only FSL-specific parts are 'flush_dcache_L1' and TLB
>> invalidation code).
>> 
>> Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
>> ---
>> 
>> Sorry for the delayed response...
>> 
>> On Tue, Jan 12, 2010 at 01:34:08PM +1100, Benjamin Herrenschmidt wrote:
>> [...]
>>> Here's a quick review. Looks good but two things:
>>> 
>>> - Please make it swsusp_booke.c, 44x support is trivial and I don't
>>> want to rename the file :-)
>> 
>> Done.
>> 
>>> - Is there really an SDR1 register on FSL BookE ? It's supposed to be
>>> the pointer to the hash table on server ...
>> 
>> Thanks, fixed.
>> 
>>> - You probably should save/restore the TCR and ack pending crap DEC or
>>> FIT interrupts in the TSR right before you kick the decrementer
>> 
>> Done.
>> 
>>> - Nowadays, we still assume that the "loader" kernel is exactly the
>>> same as the "loaded" kernel on resume ?
>> 
>> I'm pretty sure today we do rely on this, yes. This is not some
>> generic code limitation though, it's just hard to test the case
>> when loader != loaded. In most cases it will work fine since the
>> loader kernel wouldn't differ a lot from the loaded kernel, so
>> it'll setup the low level stuff the same way.
>> 
>> We may try to link the loader kernel to a different address
>> (relocate it as in kdump case), and hope that it'll trigger
>> all sort of problems so that we could fix them.
>> 
>> Though, the better test case would be to resume the hibernated
>> kernel directly from the bootloader.
> 
> Kumar,
> 
> According to patchwork, this is now delegated to you. Do you
> have any objections to merge this?

Would like Scott's Ack.

- k

Re: [PATCH v2] powerpc: Add hibernation support for FSL BookE processors

[Anton Vorontsov]

On Thu, Apr 15, 2010 at 02:20:23PM -0500, Kumar Gala wrote:
[...]
> > Kumar,
> > 
> > According to patchwork, this is now delegated to you. Do you
> > have any objections to merge this?
> 
> Would like Scott's Ack.

Cc'ing.

Re: [PATCH v2] powerpc: Add hibernation support for FSL BookE processors

[Scott Wood]

Kumar Gala wrote:
> On Apr 15, 2010, at 1:45 PM, Anton Vorontsov wrote:
>> Kumar,
>>
>> According to patchwork, this is now delegated to you. Do you
>> have any objections to merge this?
> 
> Would like Scott's Ack.

I think we need to save IACn, DACn, DBCRn, PID0, and USPRG0.

Might want to also save TLB1 contents, and maybe things like HIDn, cache 
registers, etc. -- I don't think they're changeable post-boot currently, 
but it'd be good to avoid surprises if that were to change.

Otherwise ACK.

-Scott

Re: [PATCH v2] powerpc: Add hibernation support for FSL BookE processors

[Scott Wood]

Scott Wood wrote:
> Kumar Gala wrote:
>> On Apr 15, 2010, at 1:45 PM, Anton Vorontsov wrote:
>>> Kumar,
>>>
>>> According to patchwork, this is now delegated to you. Do you
>>> have any objections to merge this?
>>
>> Would like Scott's Ack.
> 
> I think we need to save IACn, DACn, DBCRn, PID0, and USPRG0.
> 
> Might want to also save TLB1 contents, and maybe things like HIDn, cache 
> registers, etc. -- I don't think they're changeable post-boot currently, 
> but it'd be good to avoid surprises if that were to change.
> 
> Otherwise ACK.

Oh, should also use the timebase SPRs rather than mftb/mttb.

-Scott

Re: [PATCH v2] powerpc: Add hibernation support for FSL BookE processors

[Anton Vorontsov]

On Thu, Apr 15, 2010 at 03:05:08PM -0500, Scott Wood wrote:
> Kumar Gala wrote:
> >On Apr 15, 2010, at 1:45 PM, Anton Vorontsov wrote:
> >>Kumar,
> >>
> >>According to patchwork, this is now delegated to you. Do you
> >>have any objections to merge this?
> >
> >Would like Scott's Ack.
> 
> I think we need to save IACn, DACn, DBCRn,

Does the kernel actually need these registers? I mean, they're
saved per thread anyway.

> PID0,

Kernel clears it early at boot, why would we save it?

> and USPRG0.

Currently this isn't used at all.

> Might want to also save TLB1 contents, and maybe things like HIDn,
> cache registers, etc. -- I don't think they're changeable post-boot
> currently, but it'd be good to avoid surprises if that were to
> change.

Hm. I don't really like the idea of doing things 'just in case',
it just adds a code which functionality isn't tested, and which
we will might never actually need.

Thanks,

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

[PATCH v3] powerpc: Add hibernation support for FSL BookE processors

[Anton Vorontsov]

This is started as swsusp_32.S modifications, but the amount of #ifdefs
made the whole file horribly unreadable, so let's put the support into
its own separate file.

The code should be relatively easy to modify to support 44x BookEs as
well, but since I don't have any 44x to test, let's confine the code to
FSL BookE. (The only FSL-specific parts are 'flush_dcache_L1' and TLB
invalidation code).

Signed-off-by: Anton Vorontsov <avorontsov@mvista.com>
---

On Thu, Apr 15, 2010 at 03:18:31PM -0500, Scott Wood wrote:
> Scott Wood wrote:
> >Kumar Gala wrote:
> >>On Apr 15, 2010, at 1:45 PM, Anton Vorontsov wrote:
> >>>Kumar,
> >>>
> >>>According to patchwork, this is now delegated to you. Do you
> >>>have any objections to merge this?
> >>
> >>Would like Scott's Ack.
> >
> >I think we need to save IACn, DACn, DBCRn, PID0, and USPRG0.
> >
> >Might want to also save TLB1 contents, and maybe things like HIDn,
> >cache registers, etc. -- I don't think they're changeable
> >post-boot currently, but it'd be good to avoid surprises if that
> >were to change.
> >
> >Otherwise ACK.
> 
> Oh, should also use the timebase SPRs rather than mftb/mttb.

Hm. That's for old toolchains that don't understand mftb/mttb
synonyms? OK, that would be better then.

I still don't save TLB, HIDs etc. Let's keep it simple for now.

 arch/powerpc/kernel/Makefile       |    8 +-
 arch/powerpc/kernel/swsusp_booke.S |  200 ++++++++++++++++++++++++++++++++++++
 2 files changed, 206 insertions(+), 2 deletions(-)
 create mode 100644 arch/powerpc/kernel/swsusp_booke.S

diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index 8773263..58d0572 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -57,8 +57,12 @@ obj-$(CONFIG_CRASH_DUMP)	+= crash_dump.o
 obj-$(CONFIG_E500)		+= idle_e500.o
 obj-$(CONFIG_6xx)		+= idle_6xx.o l2cr_6xx.o cpu_setup_6xx.o
 obj-$(CONFIG_TAU)		+= tau_6xx.o
-obj-$(CONFIG_HIBERNATION)	+= swsusp.o suspend.o \
-				   swsusp_$(CONFIG_WORD_SIZE).o
+obj-$(CONFIG_HIBERNATION)	+= swsusp.o suspend.o
+ifeq ($(CONFIG_FSL_BOOKE),y)
+obj-$(CONFIG_HIBERNATION)	+= swsusp_booke.o
+else
+obj-$(CONFIG_HIBERNATION)	+= swsusp_$(CONFIG_WORD_SIZE).o
+endif
 obj64-$(CONFIG_HIBERNATION)	+= swsusp_asm64.o
 obj-$(CONFIG_MODULES)		+= module.o module_$(CONFIG_WORD_SIZE).o
 obj-$(CONFIG_44x)		+= cpu_setup_44x.o
diff --git a/arch/powerpc/kernel/swsusp_booke.S b/arch/powerpc/kernel/swsusp_booke.S
new file mode 100644
index 0000000..071d120
--- /dev/null
+++ b/arch/powerpc/kernel/swsusp_booke.S
@@ -0,0 +1,200 @@
+/*
+ * Based on swsusp_32.S, modified for FSL BookE by
+ * Anton Vorontsov <avorontsov@ru.mvista.com>
+ * Copyright (c) 2009-2010 MontaVista Software, LLC.
+ */
+
+#include <linux/threads.h>
+#include <asm/processor.h>
+#include <asm/page.h>
+#include <asm/cputable.h>
+#include <asm/thread_info.h>
+#include <asm/ppc_asm.h>
+#include <asm/asm-offsets.h>
+#include <asm/mmu.h>
+
+/*
+ * Structure for storing CPU registers on the save area.
+ */
+#define SL_SP		0
+#define SL_PC		4
+#define SL_MSR		8
+#define SL_TCR		0xc
+#define SL_SPRG0	0x10
+#define SL_SPRG1	0x14
+#define SL_SPRG2	0x18
+#define SL_SPRG3	0x1c
+#define SL_SPRG4	0x20
+#define SL_SPRG5	0x24
+#define SL_SPRG6	0x28
+#define SL_SPRG7	0x2c
+#define SL_TBU		0x30
+#define SL_TBL		0x34
+#define SL_R2		0x38
+#define SL_CR		0x3c
+#define SL_LR		0x40
+#define SL_R12		0x44	/* r12 to r31 */
+#define SL_SIZE		(SL_R12 + 80)
+
+	.section .data
+	.align	5
+
+_GLOBAL(swsusp_save_area)
+	.space	SL_SIZE
+
+
+	.section .text
+	.align	5
+
+_GLOBAL(swsusp_arch_suspend)
+	lis	r11,swsusp_save_area@h
+	ori	r11,r11,swsusp_save_area@l
+
+	mflr	r0
+	stw	r0,SL_LR(r11)
+	mfcr	r0
+	stw	r0,SL_CR(r11)
+	stw	r1,SL_SP(r11)
+	stw	r2,SL_R2(r11)
+	stmw	r12,SL_R12(r11)
+
+	/* Save MSR & TCR */
+	mfmsr	r4
+	stw	r4,SL_MSR(r11)
+	mfspr	r4,SPRN_TCR
+	stw	r4,SL_TCR(r11)
+
+	/* Get a stable timebase and save it */
+1:	mfspr	r4,SPRN_TBRU
+	stw	r4,SL_TBU(r11)
+	mfspr	r5,SPRN_TBRL
+	stw	r5,SL_TBL(r11)
+	mfspr	r3,SPRN_TBRU
+	cmpw	r3,r4
+	bne	1b
+
+	/* Save SPRGs */
+	mfsprg	r4,0
+	stw	r4,SL_SPRG0(r11)
+	mfsprg	r4,1
+	stw	r4,SL_SPRG1(r11)
+	mfsprg	r4,2
+	stw	r4,SL_SPRG2(r11)
+	mfsprg	r4,3
+	stw	r4,SL_SPRG3(r11)
+	mfsprg	r4,4
+	stw	r4,SL_SPRG4(r11)
+	mfsprg	r4,5
+	stw	r4,SL_SPRG5(r11)
+	mfsprg	r4,6
+	stw	r4,SL_SPRG6(r11)
+	mfsprg	r4,7
+	stw	r4,SL_SPRG7(r11)
+
+	/* Call the low level suspend stuff (we should probably have made
+	 * a stackframe...
+	 */
+	bl	swsusp_save
+
+	/* Restore LR from the save area */
+	lis	r11,swsusp_save_area@h
+	ori	r11,r11,swsusp_save_area@l
+	lwz	r0,SL_LR(r11)
+	mtlr	r0
+
+	blr
+
+_GLOBAL(swsusp_arch_resume)
+ 	sync
+
+	/* Load ptr the list of pages to copy in r3 */
+	lis	r11,(restore_pblist)@h
+	ori	r11,r11,restore_pblist@l
+	lwz	r3,0(r11)
+
+	/* Copy the pages. This is a very basic implementation, to
+	 * be replaced by something more cache efficient */
+1:
+	li	r0,256
+	mtctr	r0
+	lwz	r5,pbe_address(r3)	/* source */
+	lwz	r6,pbe_orig_address(r3)	/* destination */
+2:
+	lwz	r8,0(r5)
+	lwz	r9,4(r5)
+	lwz	r10,8(r5)
+	lwz	r11,12(r5)
+	addi	r5,r5,16
+	stw	r8,0(r6)
+	stw	r9,4(r6)
+	stw	r10,8(r6)
+	stw	r11,12(r6)
+	addi	r6,r6,16
+	bdnz	2b
+	lwz	r3,pbe_next(r3)
+	cmpwi	0,r3,0
+	bne	1b
+
+	bl flush_dcache_L1
+	bl flush_instruction_cache
+
+	lis	r11,swsusp_save_area@h
+	ori	r11,r11,swsusp_save_area@l
+
+	lwz	r4,SL_SPRG0(r11)
+	mtsprg	0,r4
+	lwz	r4,SL_SPRG1(r11)
+	mtsprg	1,r4
+	lwz	r4,SL_SPRG2(r11)
+	mtsprg	2,r4
+	lwz	r4,SL_SPRG3(r11)
+	mtsprg	3,r4
+	lwz	r4,SL_SPRG4(r11)
+	mtsprg	4,r4
+	lwz	r4,SL_SPRG5(r11)
+	mtsprg	5,r4
+	lwz	r4,SL_SPRG6(r11)
+	mtsprg	6,r4
+	lwz	r4,SL_SPRG7(r11)
+	mtsprg	7,r4
+
+	/* Invalidate TLB0 & TLB1 */
+	li	r6,0x04
+	tlbivax 0,r6
+	TLBSYNC
+	li	r6,0x0c
+	tlbivax 0,r6
+	TLBSYNC
+
+	/* restore the MSR */
+	lwz	r3,SL_MSR(r11)
+
+	/* Restore TB */
+	li	r3,0
+	mtspr	SPRN_TBWL,r3
+	lwz	r3,SL_TBU(r11)
+	lwz	r4,SL_TBL(r11)
+	mtspr	SPRN_TBWU,r3
+	mtspr	SPRN_TBWL,r4
+
+	/* Restore TCR and clear any pending bits in TSR. */
+	lwz	r4,SL_TCR(r11)
+	mtspr	SPRN_TCR,r4
+	lis	r4, (TSR_ENW | TSR_WIS | TSR_DIS | TSR_FIS)@h
+	mtspr	SPRN_TSR,r4
+
+	/* Kick decrementer */
+	li	r0,1
+	mtdec	r0
+
+	/* Restore the callee-saved registers and return */
+	lwz	r0,SL_CR(r11)
+	mtcr	r0
+	lwz	r2,SL_R2(r11)
+	lmw	r12,SL_R12(r11)
+	lwz	r1,SL_SP(r11)
+	lwz	r0,SL_LR(r11)
+	mtlr	r0
+
+	li	r3,0
+	blr
-- 
1.7.0.3

Re: [PATCH v2] powerpc: Add hibernation support for FSL BookE processors

[Scott Wood]

Anton Vorontsov wrote:
> On Thu, Apr 15, 2010 at 03:05:08PM -0500, Scott Wood wrote:
>> Kumar Gala wrote:
>>> On Apr 15, 2010, at 1:45 PM, Anton Vorontsov wrote:
>>>> Kumar,
>>>>
>>>> According to patchwork, this is now delegated to you. Do you
>>>> have any objections to merge this?
>>> Would like Scott's Ack.
>> I think we need to save IACn, DACn, DBCRn,
> 
> Does the kernel actually need these registers? I mean, they're
> saved per thread anyway.
> 
>> PID0,
> 
> Kernel clears it early at boot, why would we save it?

What context are we in when we suspend, and what context is expected 
after resume?  If we're guaranteed not to be on a thread that cares 
about any of this, then OK.

>> and USPRG0.
> 
> Currently this isn't used at all.

Was thinking that it's up to userspace whether to use it or not -- 
though it ought to be saved on thread switch instead.

>> Might want to also save TLB1 contents, and maybe things like HIDn,
>> cache registers, etc. -- I don't think they're changeable post-boot
>> currently, but it'd be good to avoid surprises if that were to
>> change.
> 
> Hm. I don't really like the idea of doing things 'just in case',
> it just adds a code which functionality isn't tested, and which
> we will might never actually need.

The intent was to reduce dependencies between the suspend code and what 
the rest of the kernel does -- it's likely that someone adding some 
runtime manipulation of one of these things (e.g. hugetlbfs modifying 
TLB1) would not think to check suspend/hibernation code.  It would still 
be somewhat tested, in that you would see problems if you restored the 
wrong values (just not if you failed to restore at all).

We'll need to save/restore those things anyway for deep sleep -- but I'm 
not sure whether it'll be feasible to share code with hibernate (I 
recall trying on 83xx and giving up).  In any case, it can probably wait 
until 85xx deep sleep gets pushed out.

-Scott

Re: [PATCH v3] powerpc: Add hibernation support for FSL BookE processors

[Scott Wood]

Anton Vorontsov wrote:
> +	/* Invalidate TLB0 & TLB1 */
> +	li	r6,0x04
> +	tlbivax 0,r6
> +	TLBSYNC
> +	li	r6,0x0c
> +	tlbivax 0,r6
> +	TLBSYNC

Is this needed?  Shouldn't the boot process have already given us a sane 
TLB?

-Scott

[PATCH v4] powerpc: Add hibernation support for FSL BookE processors

[Anton Vorontsov]

This is started as swsusp_32.S modifications, but the amount of #ifdefs
made the whole file horribly unreadable, so let's put the support into
its own separate file.

The code should be relatively easy to modify to support 44x BookEs as
well, but since I don't have any 44x to test, let's confine the code to
FSL BookE. (The only FSL-specific part so far is 'flush_dcache_L1'.)

Signed-off-by: Anton Vorontsov <avorontsov@mvista.com>
---

On Fri, Apr 16, 2010 at 10:54:36AM -0500, Scott Wood wrote:
> Anton Vorontsov wrote:
> >+	/* Invalidate TLB0 & TLB1 */
> >+	li	r6,0x04
> >+	tlbivax 0,r6
> >+	TLBSYNC
> >+	li	r6,0x0c
> >+	tlbivax 0,r6
> >+	TLBSYNC
> 
> Is this needed?  Shouldn't the boot process have already given us a
> sane TLB?

Thanks for catching, it seems that it's just a left over from
some debugging code and not actually needed. ..that reminded me
the time I spent inserting BookE specific code into swsusp_32.S,
and then debugging all that #ifdef mess...

This is tested on e500v2.

 arch/powerpc/kernel/Makefile       |    8 +-
 arch/powerpc/kernel/swsusp_booke.S |  192 ++++++++++++++++++++++++++++++++++++
 2 files changed, 198 insertions(+), 2 deletions(-)
 create mode 100644 arch/powerpc/kernel/swsusp_booke.S

diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index 8773263..58d0572 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -57,8 +57,12 @@ obj-$(CONFIG_CRASH_DUMP)	+= crash_dump.o
 obj-$(CONFIG_E500)		+= idle_e500.o
 obj-$(CONFIG_6xx)		+= idle_6xx.o l2cr_6xx.o cpu_setup_6xx.o
 obj-$(CONFIG_TAU)		+= tau_6xx.o
-obj-$(CONFIG_HIBERNATION)	+= swsusp.o suspend.o \
-				   swsusp_$(CONFIG_WORD_SIZE).o
+obj-$(CONFIG_HIBERNATION)	+= swsusp.o suspend.o
+ifeq ($(CONFIG_FSL_BOOKE),y)
+obj-$(CONFIG_HIBERNATION)	+= swsusp_booke.o
+else
+obj-$(CONFIG_HIBERNATION)	+= swsusp_$(CONFIG_WORD_SIZE).o
+endif
 obj64-$(CONFIG_HIBERNATION)	+= swsusp_asm64.o
 obj-$(CONFIG_MODULES)		+= module.o module_$(CONFIG_WORD_SIZE).o
 obj-$(CONFIG_44x)		+= cpu_setup_44x.o
diff --git a/arch/powerpc/kernel/swsusp_booke.S b/arch/powerpc/kernel/swsusp_booke.S
new file mode 100644
index 0000000..bea3f67
--- /dev/null
+++ b/arch/powerpc/kernel/swsusp_booke.S
@@ -0,0 +1,192 @@
+/*
+ * Based on swsusp_32.S, modified for FSL BookE by
+ * Anton Vorontsov <avorontsov@ru.mvista.com>
+ * Copyright (c) 2009-2010 MontaVista Software, LLC.
+ */
+
+#include <linux/threads.h>
+#include <asm/processor.h>
+#include <asm/page.h>
+#include <asm/cputable.h>
+#include <asm/thread_info.h>
+#include <asm/ppc_asm.h>
+#include <asm/asm-offsets.h>
+#include <asm/mmu.h>
+
+/*
+ * Structure for storing CPU registers on the save area.
+ */
+#define SL_SP		0
+#define SL_PC		4
+#define SL_MSR		8
+#define SL_TCR		0xc
+#define SL_SPRG0	0x10
+#define SL_SPRG1	0x14
+#define SL_SPRG2	0x18
+#define SL_SPRG3	0x1c
+#define SL_SPRG4	0x20
+#define SL_SPRG5	0x24
+#define SL_SPRG6	0x28
+#define SL_SPRG7	0x2c
+#define SL_TBU		0x30
+#define SL_TBL		0x34
+#define SL_R2		0x38
+#define SL_CR		0x3c
+#define SL_LR		0x40
+#define SL_R12		0x44	/* r12 to r31 */
+#define SL_SIZE		(SL_R12 + 80)
+
+	.section .data
+	.align	5
+
+_GLOBAL(swsusp_save_area)
+	.space	SL_SIZE
+
+
+	.section .text
+	.align	5
+
+_GLOBAL(swsusp_arch_suspend)
+	lis	r11,swsusp_save_area@h
+	ori	r11,r11,swsusp_save_area@l
+
+	mflr	r0
+	stw	r0,SL_LR(r11)
+	mfcr	r0
+	stw	r0,SL_CR(r11)
+	stw	r1,SL_SP(r11)
+	stw	r2,SL_R2(r11)
+	stmw	r12,SL_R12(r11)
+
+	/* Save MSR & TCR */
+	mfmsr	r4
+	stw	r4,SL_MSR(r11)
+	mfspr	r4,SPRN_TCR
+	stw	r4,SL_TCR(r11)
+
+	/* Get a stable timebase and save it */
+1:	mfspr	r4,SPRN_TBRU
+	stw	r4,SL_TBU(r11)
+	mfspr	r5,SPRN_TBRL
+	stw	r5,SL_TBL(r11)
+	mfspr	r3,SPRN_TBRU
+	cmpw	r3,r4
+	bne	1b
+
+	/* Save SPRGs */
+	mfsprg	r4,0
+	stw	r4,SL_SPRG0(r11)
+	mfsprg	r4,1
+	stw	r4,SL_SPRG1(r11)
+	mfsprg	r4,2
+	stw	r4,SL_SPRG2(r11)
+	mfsprg	r4,3
+	stw	r4,SL_SPRG3(r11)
+	mfsprg	r4,4
+	stw	r4,SL_SPRG4(r11)
+	mfsprg	r4,5
+	stw	r4,SL_SPRG5(r11)
+	mfsprg	r4,6
+	stw	r4,SL_SPRG6(r11)
+	mfsprg	r4,7
+	stw	r4,SL_SPRG7(r11)
+
+	/* Call the low level suspend stuff (we should probably have made
+	 * a stackframe...
+	 */
+	bl	swsusp_save
+
+	/* Restore LR from the save area */
+	lis	r11,swsusp_save_area@h
+	ori	r11,r11,swsusp_save_area@l
+	lwz	r0,SL_LR(r11)
+	mtlr	r0
+
+	blr
+
+_GLOBAL(swsusp_arch_resume)
+ 	sync
+
+	/* Load ptr the list of pages to copy in r3 */
+	lis	r11,(restore_pblist)@h
+	ori	r11,r11,restore_pblist@l
+	lwz	r3,0(r11)
+
+	/* Copy the pages. This is a very basic implementation, to
+	 * be replaced by something more cache efficient */
+1:
+	li	r0,256
+	mtctr	r0
+	lwz	r5,pbe_address(r3)	/* source */
+	lwz	r6,pbe_orig_address(r3)	/* destination */
+2:
+	lwz	r8,0(r5)
+	lwz	r9,4(r5)
+	lwz	r10,8(r5)
+	lwz	r11,12(r5)
+	addi	r5,r5,16
+	stw	r8,0(r6)
+	stw	r9,4(r6)
+	stw	r10,8(r6)
+	stw	r11,12(r6)
+	addi	r6,r6,16
+	bdnz	2b
+	lwz	r3,pbe_next(r3)
+	cmpwi	0,r3,0
+	bne	1b
+
+	bl flush_dcache_L1
+	bl flush_instruction_cache
+
+	lis	r11,swsusp_save_area@h
+	ori	r11,r11,swsusp_save_area@l
+
+	lwz	r4,SL_SPRG0(r11)
+	mtsprg	0,r4
+	lwz	r4,SL_SPRG1(r11)
+	mtsprg	1,r4
+	lwz	r4,SL_SPRG2(r11)
+	mtsprg	2,r4
+	lwz	r4,SL_SPRG3(r11)
+	mtsprg	3,r4
+	lwz	r4,SL_SPRG4(r11)
+	mtsprg	4,r4
+	lwz	r4,SL_SPRG5(r11)
+	mtsprg	5,r4
+	lwz	r4,SL_SPRG6(r11)
+	mtsprg	6,r4
+	lwz	r4,SL_SPRG7(r11)
+	mtsprg	7,r4
+
+	/* restore the MSR */
+	lwz	r3,SL_MSR(r11)
+
+	/* Restore TB */
+	li	r3,0
+	mtspr	SPRN_TBWL,r3
+	lwz	r3,SL_TBU(r11)
+	lwz	r4,SL_TBL(r11)
+	mtspr	SPRN_TBWU,r3
+	mtspr	SPRN_TBWL,r4
+
+	/* Restore TCR and clear any pending bits in TSR. */
+	lwz	r4,SL_TCR(r11)
+	mtspr	SPRN_TCR,r4
+	lis	r4, (TSR_ENW | TSR_WIS | TSR_DIS | TSR_FIS)@h
+	mtspr	SPRN_TSR,r4
+
+	/* Kick decrementer */
+	li	r0,1
+	mtdec	r0
+
+	/* Restore the callee-saved registers and return */
+	lwz	r0,SL_CR(r11)
+	mtcr	r0
+	lwz	r2,SL_R2(r11)
+	lmw	r12,SL_R12(r11)
+	lwz	r1,SL_SP(r11)
+	lwz	r0,SL_LR(r11)
+	mtlr	r0
+
+	li	r3,0
+	blr
-- 
1.7.0.3

Re: [PATCH v2] powerpc: Add hibernation support for FSL BookE processors

[Anton Vorontsov]

On Fri, Apr 16, 2010 at 10:48:09AM -0500, Scott Wood wrote:
> Anton Vorontsov wrote:
> >On Thu, Apr 15, 2010 at 03:05:08PM -0500, Scott Wood wrote:
> >>Kumar Gala wrote:
> >>>On Apr 15, 2010, at 1:45 PM, Anton Vorontsov wrote:
> >>>>Kumar,
> >>>>
> >>>>According to patchwork, this is now delegated to you. Do you
> >>>>have any objections to merge this?
> >>>Would like Scott's Ack.
> >>I think we need to save IACn, DACn, DBCRn,
> >
> >Does the kernel actually need these registers? I mean, they're
> >saved per thread anyway.
> >
> >>PID0,
> >
> >Kernel clears it early at boot, why would we save it?
> 
> What context are we in when we suspend, and what context is expected
> after resume?  If we're guaranteed not to be on a thread that cares
> about any of this, then OK.

Yeah, we're in the kernel context on suspend and resume.

> >>and USPRG0.
> >
> >Currently this isn't used at all.
> 
> Was thinking that it's up to userspace whether to use it or not --
> though it ought to be saved on thread switch instead.
> 
> >>Might want to also save TLB1 contents, and maybe things like HIDn,
> >>cache registers, etc. -- I don't think they're changeable post-boot
> >>currently, but it'd be good to avoid surprises if that were to
> >>change.
> >
> >Hm. I don't really like the idea of doing things 'just in case',
> >it just adds a code which functionality isn't tested, and which
> >we will might never actually need.
> 
> The intent was to reduce dependencies between the suspend code and
> what the rest of the kernel does -- it's likely that someone adding
> some runtime manipulation of one of these things (e.g. hugetlbfs
> modifying TLB1) would not think to check suspend/hibernation code.
> It would still be somewhat tested, in that you would see problems if
> you restored the wrong values (just not if you failed to restore at
> all).

Yep, I understand the intent, though, I'd like 'git reset --hard
this_commit' to bring a minimal, but still fully functional hibernate,
without unneeded cruft. We might add more savings/restores later [or
sooner, when needed], but that won't matter that much since we'll
have a point in the git history with that minimal support.

I could even implement more savings and restores on top of this
patch, but really, I don't see a hurry in doing this now. :-)
So that's why I don't.

Thanks,

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

Re: [PATCH v4] powerpc: Add hibernation support for FSL BookE processors

[Kumar Gala]

On Apr 16, 2010, at 2:03 PM, Anton Vorontsov wrote:

> This is started as swsusp_32.S modifications, but the amount of =
#ifdefs
> made the whole file horribly unreadable, so let's put the support into
> its own separate file.
>=20
> The code should be relatively easy to modify to support 44x BookEs as
> well, but since I don't have any 44x to test, let's confine the code =
to
> FSL BookE. (The only FSL-specific part so far is 'flush_dcache_L1'.)
>=20
> Signed-off-by: Anton Vorontsov <avorontsov@mvista.com>
> ---
>=20
> On Fri, Apr 16, 2010 at 10:54:36AM -0500, Scott Wood wrote:
>> Anton Vorontsov wrote:
>>> +	/* Invalidate TLB0 & TLB1 */
>>> +	li	r6,0x04
>>> +	tlbivax 0,r6
>>> +	TLBSYNC
>>> +	li	r6,0x0c
>>> +	tlbivax 0,r6
>>> +	TLBSYNC
>>=20
>> Is this needed?  Shouldn't the boot process have already given us a
>> sane TLB?
>=20
> Thanks for catching, it seems that it's just a left over from
> some debugging code and not actually needed. ..that reminded me
> the time I spent inserting BookE specific code into swsusp_32.S,
> and then debugging all that #ifdef mess...
>=20
> This is tested on e500v2.
>=20
> arch/powerpc/kernel/Makefile       |    8 +-
> arch/powerpc/kernel/swsusp_booke.S |  192 =
++++++++++++++++++++++++++++++++++++
> 2 files changed, 198 insertions(+), 2 deletions(-)
> create mode 100644 arch/powerpc/kernel/swsusp_booke.S

Scott, you ok (ACK) this version or still need tweaks?

- k=

Re: [PATCH v4] powerpc: Add hibernation support for FSL BookE processors

[Scott Wood]

On 04/16/2010 02:03 PM, Anton Vorontsov wrote:
> +	/* restore the MSR */
> +	lwz	r3,SL_MSR(r11)
> +
> +	/* Restore TB */
> +	li	r3,0

Missing mtmsr?

Otherwise ACK.

-Scott

[PATCH v5] powerpc: Add hibernation support for FSL BookE processors

[Anton Vorontsov]

This is started as swsusp_32.S modifications, but the amount of #ifdefs
made the whole file horribly unreadable, so let's put the support into
its own separate file.

The code should be relatively easy to modify to support 44x BookEs as
well, but since I don't have any 44x to test, let's confine the code to
FSL BookE. (The only FSL-specific part so far is 'flush_dcache_L1'.)

Signed-off-by: Anton Vorontsov <avorontsov@mvista.com>
Acked-by: Scott Wood <scottwood@freescale.com>
---

On Mon, May 17, 2010 at 01:12:37PM -0500, Scott Wood wrote:
> On 04/16/2010 02:03 PM, Anton Vorontsov wrote:
> >+	/* restore the MSR */
> >+	lwz	r3,SL_MSR(r11)
> >+
> >+	/* Restore TB */
> >+	li	r3,0
> 
> Missing mtmsr?
> 
> Otherwise ACK.

Fixed, thanks!

 arch/powerpc/kernel/Makefile       |    8 +-
 arch/powerpc/kernel/swsusp_booke.S |  193 ++++++++++++++++++++++++++++++++++++
 2 files changed, 199 insertions(+), 2 deletions(-)
 create mode 100644 arch/powerpc/kernel/swsusp_booke.S

diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index 8773263..58d0572 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -57,8 +57,12 @@ obj-$(CONFIG_CRASH_DUMP)	+= crash_dump.o
 obj-$(CONFIG_E500)		+= idle_e500.o
 obj-$(CONFIG_6xx)		+= idle_6xx.o l2cr_6xx.o cpu_setup_6xx.o
 obj-$(CONFIG_TAU)		+= tau_6xx.o
-obj-$(CONFIG_HIBERNATION)	+= swsusp.o suspend.o \
-				   swsusp_$(CONFIG_WORD_SIZE).o
+obj-$(CONFIG_HIBERNATION)	+= swsusp.o suspend.o
+ifeq ($(CONFIG_FSL_BOOKE),y)
+obj-$(CONFIG_HIBERNATION)	+= swsusp_booke.o
+else
+obj-$(CONFIG_HIBERNATION)	+= swsusp_$(CONFIG_WORD_SIZE).o
+endif
 obj64-$(CONFIG_HIBERNATION)	+= swsusp_asm64.o
 obj-$(CONFIG_MODULES)		+= module.o module_$(CONFIG_WORD_SIZE).o
 obj-$(CONFIG_44x)		+= cpu_setup_44x.o
diff --git a/arch/powerpc/kernel/swsusp_booke.S b/arch/powerpc/kernel/swsusp_booke.S
new file mode 100644
index 0000000..b674d6d
--- /dev/null
+++ b/arch/powerpc/kernel/swsusp_booke.S
@@ -0,0 +1,193 @@
+/*
+ * Based on swsusp_32.S, modified for FSL BookE by
+ * Anton Vorontsov <avorontsov@ru.mvista.com>
+ * Copyright (c) 2009-2010 MontaVista Software, LLC.
+ */
+
+#include <linux/threads.h>
+#include <asm/processor.h>
+#include <asm/page.h>
+#include <asm/cputable.h>
+#include <asm/thread_info.h>
+#include <asm/ppc_asm.h>
+#include <asm/asm-offsets.h>
+#include <asm/mmu.h>
+
+/*
+ * Structure for storing CPU registers on the save area.
+ */
+#define SL_SP		0
+#define SL_PC		4
+#define SL_MSR		8
+#define SL_TCR		0xc
+#define SL_SPRG0	0x10
+#define SL_SPRG1	0x14
+#define SL_SPRG2	0x18
+#define SL_SPRG3	0x1c
+#define SL_SPRG4	0x20
+#define SL_SPRG5	0x24
+#define SL_SPRG6	0x28
+#define SL_SPRG7	0x2c
+#define SL_TBU		0x30
+#define SL_TBL		0x34
+#define SL_R2		0x38
+#define SL_CR		0x3c
+#define SL_LR		0x40
+#define SL_R12		0x44	/* r12 to r31 */
+#define SL_SIZE		(SL_R12 + 80)
+
+	.section .data
+	.align	5
+
+_GLOBAL(swsusp_save_area)
+	.space	SL_SIZE
+
+
+	.section .text
+	.align	5
+
+_GLOBAL(swsusp_arch_suspend)
+	lis	r11,swsusp_save_area@h
+	ori	r11,r11,swsusp_save_area@l
+
+	mflr	r0
+	stw	r0,SL_LR(r11)
+	mfcr	r0
+	stw	r0,SL_CR(r11)
+	stw	r1,SL_SP(r11)
+	stw	r2,SL_R2(r11)
+	stmw	r12,SL_R12(r11)
+
+	/* Save MSR & TCR */
+	mfmsr	r4
+	stw	r4,SL_MSR(r11)
+	mfspr	r4,SPRN_TCR
+	stw	r4,SL_TCR(r11)
+
+	/* Get a stable timebase and save it */
+1:	mfspr	r4,SPRN_TBRU
+	stw	r4,SL_TBU(r11)
+	mfspr	r5,SPRN_TBRL
+	stw	r5,SL_TBL(r11)
+	mfspr	r3,SPRN_TBRU
+	cmpw	r3,r4
+	bne	1b
+
+	/* Save SPRGs */
+	mfsprg	r4,0
+	stw	r4,SL_SPRG0(r11)
+	mfsprg	r4,1
+	stw	r4,SL_SPRG1(r11)
+	mfsprg	r4,2
+	stw	r4,SL_SPRG2(r11)
+	mfsprg	r4,3
+	stw	r4,SL_SPRG3(r11)
+	mfsprg	r4,4
+	stw	r4,SL_SPRG4(r11)
+	mfsprg	r4,5
+	stw	r4,SL_SPRG5(r11)
+	mfsprg	r4,6
+	stw	r4,SL_SPRG6(r11)
+	mfsprg	r4,7
+	stw	r4,SL_SPRG7(r11)
+
+	/* Call the low level suspend stuff (we should probably have made
+	 * a stackframe...
+	 */
+	bl	swsusp_save
+
+	/* Restore LR from the save area */
+	lis	r11,swsusp_save_area@h
+	ori	r11,r11,swsusp_save_area@l
+	lwz	r0,SL_LR(r11)
+	mtlr	r0
+
+	blr
+
+_GLOBAL(swsusp_arch_resume)
+ 	sync
+
+	/* Load ptr the list of pages to copy in r3 */
+	lis	r11,(restore_pblist)@h
+	ori	r11,r11,restore_pblist@l
+	lwz	r3,0(r11)
+
+	/* Copy the pages. This is a very basic implementation, to
+	 * be replaced by something more cache efficient */
+1:
+	li	r0,256
+	mtctr	r0
+	lwz	r5,pbe_address(r3)	/* source */
+	lwz	r6,pbe_orig_address(r3)	/* destination */
+2:
+	lwz	r8,0(r5)
+	lwz	r9,4(r5)
+	lwz	r10,8(r5)
+	lwz	r11,12(r5)
+	addi	r5,r5,16
+	stw	r8,0(r6)
+	stw	r9,4(r6)
+	stw	r10,8(r6)
+	stw	r11,12(r6)
+	addi	r6,r6,16
+	bdnz	2b
+	lwz	r3,pbe_next(r3)
+	cmpwi	0,r3,0
+	bne	1b
+
+	bl flush_dcache_L1
+	bl flush_instruction_cache
+
+	lis	r11,swsusp_save_area@h
+	ori	r11,r11,swsusp_save_area@l
+
+	lwz	r4,SL_SPRG0(r11)
+	mtsprg	0,r4
+	lwz	r4,SL_SPRG1(r11)
+	mtsprg	1,r4
+	lwz	r4,SL_SPRG2(r11)
+	mtsprg	2,r4
+	lwz	r4,SL_SPRG3(r11)
+	mtsprg	3,r4
+	lwz	r4,SL_SPRG4(r11)
+	mtsprg	4,r4
+	lwz	r4,SL_SPRG5(r11)
+	mtsprg	5,r4
+	lwz	r4,SL_SPRG6(r11)
+	mtsprg	6,r4
+	lwz	r4,SL_SPRG7(r11)
+	mtsprg	7,r4
+
+	/* restore the MSR */
+	lwz	r3,SL_MSR(r11)
+	mtmsr	r3
+
+	/* Restore TB */
+	li	r3,0
+	mtspr	SPRN_TBWL,r3
+	lwz	r3,SL_TBU(r11)
+	lwz	r4,SL_TBL(r11)
+	mtspr	SPRN_TBWU,r3
+	mtspr	SPRN_TBWL,r4
+
+	/* Restore TCR and clear any pending bits in TSR. */
+	lwz	r4,SL_TCR(r11)
+	mtspr	SPRN_TCR,r4
+	lis	r4, (TSR_ENW | TSR_WIS | TSR_DIS | TSR_FIS)@h
+	mtspr	SPRN_TSR,r4
+
+	/* Kick decrementer */
+	li	r0,1
+	mtdec	r0
+
+	/* Restore the callee-saved registers and return */
+	lwz	r0,SL_CR(r11)
+	mtcr	r0
+	lwz	r2,SL_R2(r11)
+	lmw	r12,SL_R12(r11)
+	lwz	r1,SL_SP(r11)
+	lwz	r0,SL_LR(r11)
+	mtlr	r0
+
+	li	r3,0
+	blr
-- 
1.7.0.5

Re: [PATCH v5] powerpc: Add hibernation support for FSL BookE processors

[Kumar Gala]

On May 17, 2010, at 1:56 PM, Anton Vorontsov wrote:

> This is started as swsusp_32.S modifications, but the amount of =
#ifdefs
> made the whole file horribly unreadable, so let's put the support into
> its own separate file.
>=20
> The code should be relatively easy to modify to support 44x BookEs as
> well, but since I don't have any 44x to test, let's confine the code =
to
> FSL BookE. (The only FSL-specific part so far is 'flush_dcache_L1'.)
>=20
> Signed-off-by: Anton Vorontsov <avorontsov@mvista.com>
> Acked-by: Scott Wood <scottwood@freescale.com>
> ---
>=20
> On Mon, May 17, 2010 at 01:12:37PM -0500, Scott Wood wrote:
>> On 04/16/2010 02:03 PM, Anton Vorontsov wrote:
>>> +	/* restore the MSR */
>>> +	lwz	r3,SL_MSR(r11)
>>> +
>>> +	/* Restore TB */
>>> +	li	r3,0
>>=20
>> Missing mtmsr?
>>=20
>> Otherwise ACK.
>=20
> Fixed, thanks!
>=20
> arch/powerpc/kernel/Makefile       |    8 +-
> arch/powerpc/kernel/swsusp_booke.S |  193 =
++++++++++++++++++++++++++++++++++++
> 2 files changed, 199 insertions(+), 2 deletions(-)
> create mode 100644 arch/powerpc/kernel/swsusp_booke.S

applied to next (fixed one white space issue).

- k=

Re: [PATCH v5] powerpc: Add hibernation support for FSL BookE processors

[Kumar Gala]

On May 17, 2010, at 1:56 PM, Anton Vorontsov wrote:

> This is started as swsusp_32.S modifications, but the amount of =
#ifdefs
> made the whole file horribly unreadable, so let's put the support into
> its own separate file.
>=20
> The code should be relatively easy to modify to support 44x BookEs as
> well, but since I don't have any 44x to test, let's confine the code =
to
> FSL BookE. (The only FSL-specific part so far is 'flush_dcache_L1'.)
>=20
> Signed-off-by: Anton Vorontsov <avorontsov@mvista.com>
> Acked-by: Scott Wood <scottwood@freescale.com>
> ---
>=20
> On Mon, May 17, 2010 at 01:12:37PM -0500, Scott Wood wrote:
>> On 04/16/2010 02:03 PM, Anton Vorontsov wrote:
>>> +	/* restore the MSR */
>>> +	lwz	r3,SL_MSR(r11)
>>> +
>>> +	/* Restore TB */
>>> +	li	r3,0
>>=20
>> Missing mtmsr?
>>=20
>> Otherwise ACK.
>=20
> Fixed, thanks!
>=20
> arch/powerpc/kernel/Makefile       |    8 +-
> arch/powerpc/kernel/swsusp_booke.S |  193 =
++++++++++++++++++++++++++++++++++++
> 2 files changed, 199 insertions(+), 2 deletions(-)
> create mode 100644 arch/powerpc/kernel/swsusp_booke.S

Is there board specific code to go along with this?

When I enable I get:

arch/powerpc/kernel/built-in.o: In function `cpu_idle':
/home/galak/git/master/powerpc/arch/powerpc/kernel/idle.c:98: undefined =
reference to `cpu_die'
make: *** [.tmp_vmlinux1] Error 1

- k=

Re: [PATCH v5] powerpc: Add hibernation support for FSL BookE processors

[Anton Vorontsov]

On Mon, May 17, 2010 at 04:22:37PM -0500, Kumar Gala wrote:
[...]
> > arch/powerpc/kernel/Makefile       |    8 +-
> > arch/powerpc/kernel/swsusp_booke.S |  193 ++++++++++++++++++++++++++++++++++++
> > 2 files changed, 199 insertions(+), 2 deletions(-)
> > create mode 100644 arch/powerpc/kernel/swsusp_booke.S
> 
> Is there board specific code to go along with this?

Nope.

> When I enable I get:
> 
> arch/powerpc/kernel/built-in.o: In function `cpu_idle':
> /home/galak/git/master/powerpc/arch/powerpc/kernel/idle.c:98: undefined reference to `cpu_die'
> make: *** [.tmp_vmlinux1] Error 1

This isn't caused by hibernation or my patch specifically.
Instead, it's a long standing issue, enabling CONFIG_SMP and
CONFIG_SUSPEND is enough to trigger this. Patch exists:

http://www.mail-archive.com/linuxppc-dev@lists.ozlabs.org/msg27982.html

No idea why it didn't make Linus' tree.

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2