[PATCH 1/3] powerpc/fsl: add E500MC and E5500 PVR define

[PATCH 1/3] powerpc/fsl: add E500MC and E5500 PVR define

[Dongsheng Wang]

From: Wang Dongsheng <dongsheng.wang@freescale.com>

E500MC and E5500 PVR will be used in subsequent save/restore core
state patches.

Signed-off-by: Wang Dongsheng <dongsheng.wang@freescale.com>

diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
index 62b114e..cd7b630 100644
--- a/arch/powerpc/include/asm/reg.h
+++ b/arch/powerpc/include/asm/reg.h
@@ -1075,6 +1075,8 @@
 #define PVR_8560	0x80200000
 #define PVR_VER_E500V1	0x8020
 #define PVR_VER_E500V2	0x8021
+#define PVR_VER_E500MC	0x8023
+#define PVR_VER_E5500	0x8024
 #define PVR_VER_E6500	0x8040
 
 /*
-- 
1.8.5

[PATCH 2/3] powerpc/85xx: Provide two functions to save/restore the core registers

[Dongsheng Wang]

From: Wang Dongsheng <dongsheng.wang@freescale.com>

Add fsl_cpu_state_save/fsl_cpu_state_restore functions, used for deep
sleep and hibernation to save/restore core registers. We abstract out
save/restore code for use in various modules, to make them don't need
to maintain.

Currently supported processors type are E6500, E5500, E500MC, E500v2 and
E500v1.

Signed-off-by: Wang Dongsheng <dongsheng.wang@freescale.com>

diff --git a/arch/powerpc/include/asm/fsl_sleep.h b/arch/powerpc/include/asm/fsl_sleep.h
new file mode 100644
index 0000000..31c8a9b
--- /dev/null
+++ b/arch/powerpc/include/asm/fsl_sleep.h
@@ -0,0 +1,98 @@
+/*
+ * Freescale 85xx Power management set
+ *
+ * Author: Wang Dongsheng <dongsheng.wang@freescale.com>
+ *
+ * Copyright 2014 Freescale Semiconductor Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef __ASM_FSL_SLEEP_H
+#define __ASM_FSL_SLEEP_H
+
+/*
+ * Freescale 85xx Core registers set, core register map definition
+ * Address base on r3, we need to compatible with both 32-bit and 64-bit, so
+ * the data width is 64-bit(double word).
+ *
+ * Acronyms:
+ *	dw(data width)	0x08
+ *
+ * Map:
+ * General-Purpose Registers
+ *	GPR1(sp)		0
+ *	GPR2			0x8		(dw * 1)
+ *	GPR13 - GPR31		0x10 ~ 0xa0	(dw * 2 ~ dw * 20)
+ * Foating-point registers
+ *	FPR14 - FPR31		0xa8 ~ 0x130	(dw * 21 ~ dw * 38)
+ * Registers for Branch Operations
+ *	CR			0x138		(dw * 39)
+ *	LR			0x140		(dw * 40)
+ * Processor Control Registers
+ *	MSR			0x148		(dw * 41)
+ *	EPCR			0x150		(dw * 42)
+ *
+ *	Only e500, e500v2 need to save HID0 - HID1
+ *	HID0 - HID1		0x158 ~ 0x160 (dw * 43 ~ dw * 44)
+ * Timer Registers
+ *	TCR			0x168		(dw * 45)
+ *	TB(64bit)		0x170		(dw * 46)
+ *	TBU(32bit)		0x178		(dw * 47)
+ *	TBL(32bit)		0x180		(dw * 48)
+ * Interrupt Registers
+ *	IVPR			0x188		(dw * 49)
+ *	IVOR0 - IVOR15		0x190 ~ 0x208	(dw * 50 ~ dw * 65)
+ *	IVOR32 - IVOR41		0x210 ~ 0x258	(dw * 66 ~ dw * 75)
+ * Software-Use Registers
+ *	SPRG1			0x260		(dw * 76), 64-bit need to save.
+ *	SPRG3			0x268		(dw * 77), 32-bit need to save.
+ * MMU Registers
+ *	PID0 - PID2		0x270 ~ 0x280	(dw * 78 ~ dw * 80)
+ * Debug Registers
+ *	DBCR0 - DBCR2		0x288 ~ 0x298	(dw * 81 ~ dw * 83)
+ *	IAC1 - IAC4		0x2a0 ~ 0x2b8	(dw * 84 ~ dw * 87)
+ *	DAC1 - DAC2		0x2c0 ~ 0x2c8	(dw * 88 ~ dw * 89)
+ *
+ */
+
+#define SR_GPR1			0x000
+#define SR_GPR2			0x008
+#define SR_GPR13		0x010
+#define SR_FPR14		0x0a8
+#define SR_CR			0x138
+#define SR_LR			0x140
+#define SR_MSR			0x148
+#define SR_EPCR			0x150
+#define SR_HID0			0x158
+#define SR_TCR			0x168
+#define SR_TB			0x170
+#define SR_TBU			0x178
+#define SR_TBL			0x180
+#define SR_IVPR			0x188
+#define SR_IVOR0		0x190
+#define SR_IVOR32		0x210
+#define SR_SPRG1		0x260
+#define SR_SPRG3		0x268
+#define SR_PID0			0x270
+#define SR_DBCR0		0x288
+#define SR_IAC1			0x2a0
+#define SR_DAC1			0x2c0
+#define FSL_CPU_SR_SIZE		(SR_DAC1 + 0x10)
+
+#ifndef __ASSEMBLY__
+
+enum core_save_type {
+	BASE_SAVE = 0,
+	ALL_SAVE = 1,
+};
+
+extern int fsl_cpu_state_save(void *save_page, enum core_save_type type);
+extern int fsl_cpu_state_restore(void *restore_page, enum core_save_type type);
+
+#endif
+
+#endif
+
diff --git a/arch/powerpc/platforms/85xx/Makefile b/arch/powerpc/platforms/85xx/Makefile
index 25cebe7..650a01c 100644
--- a/arch/powerpc/platforms/85xx/Makefile
+++ b/arch/powerpc/platforms/85xx/Makefile
@@ -4,6 +4,7 @@
 obj-$(CONFIG_SMP) += smp.o
 
 obj-y += common.o
+obj-y += save-core.o
 
 obj-$(CONFIG_BSC9131_RDB) += bsc913x_rdb.o
 obj-$(CONFIG_C293_PCIE)   += c293pcie.o
diff --git a/arch/powerpc/platforms/85xx/save-core.S b/arch/powerpc/platforms/85xx/save-core.S
new file mode 100644
index 0000000..a6b93b8
--- /dev/null
+++ b/arch/powerpc/platforms/85xx/save-core.S
@@ -0,0 +1,497 @@
+/*
+ * Freescale Power Management, Save/Restore core state
+ *
+ * Copyright 2014 Freescale Semiconductor, Inc.
+ * Author: Wang Dongsheng <dongsheng.wang@freescale.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ */
+
+#include <asm/ppc_asm.h>
+#include <asm/fsl_sleep.h>
+
+/*
+ * Freescale 85xx Cores
+ * Support Core List:
+ * E500v1, E500v2, E500MC, E5500, E6500.
+ */
+
+ /*
+ * Save/Restore register operation define
+ */
+#define LOAD_SAVE_ADDRESS	\
+	mr	r10, r3
+
+#ifdef CONFIG_PPC64
+#define PPC_STD(sreg, offset, areg) \
+	std	sreg, (offset)(areg)
+#define PPC_LD(lreg, offset, areg) \
+	ld	lreg, (offset)(areg)
+
+#define PPC_STFD(sreg, offset, areg) \
+	stfd	sreg, (offset)(areg)
+#define PPC_LFD(lreg, offset, areg) \
+	lfd	lreg, (offset)(areg)
+#else
+#define PPC_STD(sreg, offset, areg) \
+	stw	sreg, (offset)(areg)
+#define PPC_LD(lreg, offset, areg) \
+	lwz	lreg, (offset)(areg)
+
+#define PPC_STFD(sreg, offset, areg) \
+	stfs	sreg, (offset)(areg)
+#define PPC_LFD(lreg, offset, areg) \
+	lfs	lreg, (offset)(areg)
+#endif
+
+#define do_save_gpr_reg(reg, addr) \
+	mr	r0, reg ;\
+	PPC_STD(r0, addr, r10)
+
+#define do_restore_gpr_reg(reg, addr) \
+	PPC_LD(r0, addr, r10) ;\
+	mr	reg, r0
+
+#define do_save_fpr_reg(reg, addr) \
+	fmr	fr0, reg ;\
+	PPC_STFD(fr0, addr, r10)
+
+#define do_restore_fpr_reg(reg, addr) \
+	PPC_LFD(fr0, addr, r10) ;\
+	fmr	reg, fr0
+
+#define do_save_spr_reg(reg, addr) \
+	mfspr	r0, SPRN_##reg ;\
+	PPC_STD(r0, addr, r10)
+
+#define do_restore_spr_reg(reg, addr) \
+	PPC_LD(r0, addr, r10) ;\
+	mtspr	SPRN_##reg, r0
+
+#define do_save_special_reg(special, addr) \
+	mf##special	r0 ;\
+	PPC_STD(r0, addr, r10)
+#define do_restore_special_reg(special, addr) \
+	PPC_LD(r0, addr, r10) ;\
+	mt##special	r0
+
+#define do_sr_general_gpr_regs(func) \
+	do_##func##_gpr_reg(r1, SR_GPR1) ;\
+	do_##func##_gpr_reg(r2, SR_GPR2) ;\
+	do_##func##_gpr_reg(r13, SR_GPR13 + 0x00) ;\
+	do_##func##_gpr_reg(r14, SR_GPR13 + 0x08) ;\
+	do_##func##_gpr_reg(r15, SR_GPR13 + 0x10) ;\
+	do_##func##_gpr_reg(r16, SR_GPR13 + 0x18) ;\
+	do_##func##_gpr_reg(r17, SR_GPR13 + 0x20) ;\
+	do_##func##_gpr_reg(r18, SR_GPR13 + 0x28) ;\
+	do_##func##_gpr_reg(r19, SR_GPR13 + 0x30) ;\
+	do_##func##_gpr_reg(r20, SR_GPR13 + 0x38) ;\
+	do_##func##_gpr_reg(r21, SR_GPR13 + 0x40) ;\
+	do_##func##_gpr_reg(r22, SR_GPR13 + 0x48) ;\
+	do_##func##_gpr_reg(r23, SR_GPR13 + 0x50) ;\
+	do_##func##_gpr_reg(r24, SR_GPR13 + 0x58) ;\
+	do_##func##_gpr_reg(r25, SR_GPR13 + 0x60) ;\
+	do_##func##_gpr_reg(r26, SR_GPR13 + 0x68) ;\
+	do_##func##_gpr_reg(r27, SR_GPR13 + 0x70) ;\
+	do_##func##_gpr_reg(r28, SR_GPR13 + 0x78) ;\
+	do_##func##_gpr_reg(r29, SR_GPR13 + 0x80) ;\
+	do_##func##_gpr_reg(r30, SR_GPR13 + 0x88) ;\
+	do_##func##_gpr_reg(r31, SR_GPR13 + 0x90)
+
+#define do_sr_fpr_regs(func) \
+	do_##func##_fpr_reg(fr14, SR_FPR14 + 0x00) ;\
+	do_##func##_fpr_reg(fr15, SR_FPR14 + 0x08) ;\
+	do_##func##_fpr_reg(fr16, SR_FPR14 + 0x10) ;\
+	do_##func##_fpr_reg(fr17, SR_FPR14 + 0x18) ;\
+	do_##func##_fpr_reg(fr18, SR_FPR14 + 0x20) ;\
+	do_##func##_fpr_reg(fr19, SR_FPR14 + 0x28) ;\
+	do_##func##_fpr_reg(fr20, SR_FPR14 + 0x30) ;\
+	do_##func##_fpr_reg(fr21, SR_FPR14 + 0x38) ;\
+	do_##func##_fpr_reg(fr22, SR_FPR14 + 0x40) ;\
+	do_##func##_fpr_reg(fr23, SR_FPR14 + 0x48) ;\
+	do_##func##_fpr_reg(fr24, SR_FPR14 + 0x50) ;\
+	do_##func##_fpr_reg(fr25, SR_FPR14 + 0x58) ;\
+	do_##func##_fpr_reg(fr26, SR_FPR14 + 0x60) ;\
+	do_##func##_fpr_reg(fr27, SR_FPR14 + 0x68) ;\
+	do_##func##_fpr_reg(fr28, SR_FPR14 + 0x70) ;\
+	do_##func##_fpr_reg(fr29, SR_FPR14 + 0x78) ;\
+	do_##func##_fpr_reg(fr30, SR_FPR14 + 0x80) ;\
+	do_##func##_fpr_reg(fr31, SR_FPR14 + 0x88)
+
+#define do_sr_general_branch_regs(func) \
+	do_##func##_special_reg(CR, SR_CR)
+
+#define do_sr_general_pcr_regs(func) \
+	do_##func##_special_reg(MSR, SR_MSR) ;\
+	do_##func##_spr_reg(EPCR, SR_EPCR) ;\
+	do_##func##_spr_reg(HID0, SR_HID0 + 0x00)
+
+#define do_sr_e500_pcr_regs(func) \
+	do_##func##_spr_reg(HID1, SR_HID0 + 0x08)
+
+#define do_sr_save_tb_regs \
+	do_save_spr_reg(TBRU, SR_TBU) ;\
+	do_save_spr_reg(TBRL, SR_TBL)
+
+#define do_sr_restore_tb_regs \
+	do_restore_spr_reg(TBWU, SR_TBU) ;\
+	do_restore_spr_reg(TBWL, SR_TBL)
+
+#define do_sr_general_time_regs(func) \
+	do_sr_##func##_tb_regs	;\
+	do_##func##_spr_reg(TCR, SR_TCR)
+
+#define do_sr_interrupt_regs(func) \
+	do_##func##_spr_reg(IVPR, SR_IVPR) ;\
+	do_##func##_spr_reg(IVOR0, SR_IVOR0 + 0x00) ;\
+	do_##func##_spr_reg(IVOR1, SR_IVOR0 + 0x08) ;\
+	do_##func##_spr_reg(IVOR2, SR_IVOR0 + 0x10) ;\
+	do_##func##_spr_reg(IVOR3, SR_IVOR0 + 0x18) ;\
+	do_##func##_spr_reg(IVOR4, SR_IVOR0 + 0x20) ;\
+	do_##func##_spr_reg(IVOR5, SR_IVOR0 + 0x28) ;\
+	do_##func##_spr_reg(IVOR6, SR_IVOR0 + 0x30) ;\
+	do_##func##_spr_reg(IVOR7, SR_IVOR0 + 0x38) ;\
+	do_##func##_spr_reg(IVOR8, SR_IVOR0 + 0x40) ;\
+	do_##func##_spr_reg(IVOR10, SR_IVOR0 + 0x50) ;\
+	do_##func##_spr_reg(IVOR11, SR_IVOR0 + 0x58) ;\
+	do_##func##_spr_reg(IVOR12, SR_IVOR0 + 0x60) ;\
+	do_##func##_spr_reg(IVOR13, SR_IVOR0 + 0x68) ;\
+	do_##func##_spr_reg(IVOR14, SR_IVOR0 + 0x70) ;\
+	do_##func##_spr_reg(IVOR15, SR_IVOR0 + 0x78)
+
+#define do_e6500_sr_interrupt_regs(func) \
+	do_##func##_spr_reg(IVOR9, SR_IVOR0 + 0x48) ;\
+	do_##func##_spr_reg(IVOR32, SR_IVOR32 + 0x00) ;\
+	do_##func##_spr_reg(IVOR33, SR_IVOR32 + 0x08) ;\
+	do_##func##_spr_reg(IVOR35, SR_IVOR32 + 0x18) ;\
+	do_##func##_spr_reg(IVOR36, SR_IVOR32 + 0x20) ;\
+	do_##func##_spr_reg(IVOR37, SR_IVOR32 + 0x28) ;\
+	do_##func##_spr_reg(IVOR38, SR_IVOR32 + 0x30) ;\
+	do_##func##_spr_reg(IVOR39, SR_IVOR32 + 0x38) ;\
+	do_##func##_spr_reg(IVOR40, SR_IVOR32 + 0x40) ;\
+	do_##func##_spr_reg(IVOR41, SR_IVOR32 + 0x48)
+
+#define do_e5500_sr_interrupt_regs(func) \
+	do_##func##_spr_reg(IVOR9, SR_IVOR0 + 0x48) ;\
+	do_##func##_spr_reg(IVOR35, SR_IVOR32 + 0x18) ;\
+	do_##func##_spr_reg(IVOR36, SR_IVOR32 + 0x20) ;\
+	do_##func##_spr_reg(IVOR37, SR_IVOR32 + 0x28) ;\
+	do_##func##_spr_reg(IVOR38, SR_IVOR32 + 0x30) ;\
+	do_##func##_spr_reg(IVOR39, SR_IVOR32 + 0x38) ;\
+	do_##func##_spr_reg(IVOR40, SR_IVOR32 + 0x40) ;\
+	do_##func##_spr_reg(IVOR41, SR_IVOR32 + 0x48)
+
+#define do_e500_sr_interrupt_regs(func) \
+	do_##func##_spr_reg(IVOR32, SR_IVOR32 + 0x00) ;\
+	do_##func##_spr_reg(IVOR33, SR_IVOR32 + 0x08) ;\
+	do_##func##_spr_reg(IVOR34, SR_IVOR32 + 0x10)
+
+#define do_e500mc_sr_interrupt_regs(func) \
+	do_##func##_spr_reg(IVOR9, SR_IVOR0 + 0x48) ;\
+	do_##func##_spr_reg(IVOR35, SR_IVOR32 + 0x18) ;\
+	do_##func##_spr_reg(IVOR36, SR_IVOR32 + 0x20) ;\
+	do_##func##_spr_reg(IVOR37, SR_IVOR32 + 0x28) ;\
+	do_##func##_spr_reg(IVOR38, SR_IVOR32 + 0x30) ;\
+	do_##func##_spr_reg(IVOR39, SR_IVOR32 + 0x38) ;\
+	do_##func##_spr_reg(IVOR40, SR_IVOR32 + 0x40) ;\
+	do_##func##_spr_reg(IVOR41, SR_IVOR32 + 0x48)
+
+#define do_sr_general_software_regs(func) \
+	do_##func##_spr_reg(SPRG1, SR_SPRG1) ;\
+	do_##func##_spr_reg(SPRG3, SR_SPRG3)
+
+#define do_sr_general_mmu_regs(func) \
+	do_##func##_spr_reg(PID0, SR_PID0 + 0x00)
+
+#define do_sr_e500_mmu_regs(func) \
+	do_##func##_spr_reg(PID1, SR_PID0 + 0x08) ;\
+	do_##func##_spr_reg(PID2, SR_PID0 + 0x10)
+
+#define do_sr_debug_regs(func) \
+	do_##func##_spr_reg(DBCR0, SR_DBCR0 + 0x00) ;\
+	do_##func##_spr_reg(DBCR1, SR_DBCR0 + 0x08) ;\
+	do_##func##_spr_reg(DBCR2, SR_DBCR0 + 0x10) ;\
+	do_##func##_spr_reg(IAC1, SR_IAC1 + 0x00) ;\
+	do_##func##_spr_reg(IAC2, SR_IAC1 + 0x08) ;\
+	do_##func##_spr_reg(DAC1, SR_DAC1 + 0x00) ;\
+	do_##func##_spr_reg(DAC2, SR_DAC1 + 0x08)
+
+#define do_e6500_sr_debug_regs(func) \
+	do_##func##_spr_reg(IAC3, SR_IAC1 + 0x10) ;\
+	do_##func##_spr_reg(IAC4, SR_IAC1 + 0x18)
+
+/*
+ * Freescale 85xx Cores, Save/Restore core registers.
+ */
+_GLOBAL(core_registers_save_area)
+	.space FSL_CPU_SR_SIZE
+
+	.section .text
+	.align	5
+_GLOBAL(fsl_cpu_base_save)
+	do_sr_general_gpr_regs(save)
+	do_sr_general_branch_regs(save)
+	do_sr_general_pcr_regs(save)
+	do_sr_general_software_regs(save)
+	do_sr_general_mmu_regs(save)
+
+	/*
+	 * Need to save float-point registers if MSR[FP] = 1.
+	 */
+	mfmsr	r12
+	andi.	r12, r12, MSR_FP
+	beq	1f
+	do_sr_fpr_regs(save)
+
+1:
+	mfspr	r5, SPRN_TBRU
+	do_sr_general_time_regs(save)
+	mfspr	r6, SPRN_TBRU
+	cmpw	r5, r6
+	bne	1b
+
+	blr
+
+_GLOBAL(fsl_cpu_base_restore)
+	do_sr_general_gpr_regs(restore)
+	do_sr_general_branch_regs(restore)
+	do_sr_general_pcr_regs(restore)
+	do_sr_general_software_regs(restore)
+	do_sr_general_mmu_regs(restore)
+
+	isync
+
+	/*
+	 * Need to restore float-point registers if MSR[FP] = 1.
+	 */
+	mfmsr	r12
+	andi.	r12, r12, MSR_FP
+	beq	1f
+	do_sr_fpr_regs(restore)
+
+1:
+	/* Restore Time registers */
+	/* clear tb lower to avoid wrap */
+	li	r0, 0
+	mtspr	SPRN_TBWL, r0
+	do_sr_general_time_regs(restore)
+
+	lis	r0, (TSR_ENW | TSR_WIS | TSR_DIS | TSR_FIS)@h
+	mtspr	SPRN_TSR, r0
+
+	/* Kick decrementer */
+	li	r0, 1
+	mtdec	r0
+
+	blr
+
+/* Base registers, e500v1, e500v2 need to do some special save/restore */
+_GLOBAL(e500_base_special_save)
+	lis	r12, 0
+	ori	r12, r12, PVR_VER_E500V1@l
+	cmpw	r11, r12
+	beq	500f
+
+	lis	r12, 0
+	ori	r12, r12, PVR_VER_E500V2@l
+	cmpw	r11, r12
+	bne	1f
+
+500:
+	do_sr_e500_pcr_regs(save)
+	do_sr_e500_mmu_regs(save)
+1:
+	blr
+
+_GLOBAL(e500_base_special_restore)
+	lis	r12, 0
+	ori	r12, r12, PVR_VER_E500V1@l
+	cmpw	r11, r12
+	beq	500f
+
+	lis	r12, 0
+	ori	r12, r12, PVR_VER_E500V2@l
+	cmpw	r11, r12
+	bne	1f
+
+500:
+	do_sr_e500_pcr_regs(save)
+	do_sr_e500_mmu_regs(save)
+1:
+	blr
+
+_GLOBAL(fsl_cpu_append_save)
+	mfspr	r0, SPRN_PVR
+	rlwinm	r11, r0, 16, 16, 31
+
+	lis	r12, 0
+	ori	r12, r12, PVR_VER_E6500@l
+	cmpw	r11, r12
+	beq	e6500_append_save
+
+	lis	r12, 0
+	ori	r12, r12, PVR_VER_E5500@l
+	cmpw	r11, r12
+	beq	e5500_append_save
+
+	lis	r12, 0
+	ori	r12, r12, PVR_VER_E500MC@l
+	cmpw	r11, r12
+	beq	e500mc_append_save
+
+	lis	r12, 0
+	ori	r12, r12, PVR_VER_E500V2@l
+	cmpw	r11, r12
+	beq	e500v2_append_save
+
+	lis	r12, 0
+	ori	r12, r12, PVR_VER_E500V1@l
+	cmpw	r11, r12
+	beq	e500v1_append_save
+
+	b	1f
+
+e6500_append_save:
+	do_e6500_sr_interrupt_regs(save)
+	do_e6500_sr_debug_regs(save)
+	b	1f
+
+e5500_append_save:
+	do_e5500_sr_interrupt_regs(save)
+	b	1f
+
+e500mc_append_save:
+	do_e500mc_sr_interrupt_regs(save)
+	b	1f
+
+e500v2_append_save:
+e500v1_append_save:
+	do_e500_sr_interrupt_regs(save)
+
+1:
+	do_sr_interrupt_regs(save)
+	do_sr_debug_regs(save)
+
+	blr
+
+_GLOBAL(fsl_cpu_append_restore)
+	mfspr	r0, SPRN_PVR
+	rlwinm	r11, r0, 16, 16, 31
+
+	lis	r12, 0
+	ori	r12, r12, PVR_VER_E6500@l
+	cmpw	r11, r12
+	beq	e6500_append_restore
+
+	lis	r12, 0
+	ori	r12, r12, PVR_VER_E5500@l
+	cmpw	r11, r12
+	beq	e5500_append_restore
+
+	lis	r12, 0
+	ori	r12, r12, PVR_VER_E500MC@l
+	cmpw	r11, r12
+	beq	e500mc_append_restore
+
+	lis	r12, 0
+	ori	r12, r12, PVR_VER_E500V2@l
+	cmpw	r11, r12
+	beq	e500v2_append_restore
+
+	lis	r12, 0
+	ori	r12, r12, PVR_VER_E500V1@l
+	cmpw	r11, r12
+	beq	e500v1_append_restore
+
+	b	1f
+
+e6500_append_restore:
+	do_e6500_sr_interrupt_regs(restore)
+	do_e6500_sr_debug_regs(restore)
+	b	1f
+
+e5500_append_restore:
+	do_e5500_sr_interrupt_regs(restore)
+	b	1f
+
+e500mc_append_restore:
+	do_e500mc_sr_interrupt_regs(restore)
+	b	1f
+
+e500v2_append_restore:
+e500v1_append_restore:
+	do_e500_sr_interrupt_regs(restore)
+
+1:
+	do_sr_interrupt_regs(restore)
+	do_sr_debug_regs(restore)
+
+	sync
+
+	blr
+
+/*
+ * r3 = the virtual address of buffer
+ * r4 = suspend type, 0-BASE_SAVE, 1-ALL_SAVE
+ */
+_GLOBAL(fsl_cpu_state_save)
+	mflr	r9
+	LOAD_SAVE_ADDRESS
+
+	/* save the return address to SR_LR */
+	do_save_gpr_reg(r9, SR_LR)
+
+	/* if core_save_type is BASE_SAVE, goto 1f */
+	cmpwi	r4, 0
+	beq	1f
+
+	bl	fsl_cpu_append_save
+
+1:
+	bl	e500_base_special_save
+
+	bl	fsl_cpu_base_save
+
+	li	r3, 0
+	mtlr	r9
+	blr
+
+/*
+ * r3 = the virtual address of buffer
+ * r4 = suspend type, 0-BASE_SAVE, 1-ALL_SAVE
+ */
+_GLOBAL(fsl_cpu_state_restore)
+	mflr	r9
+	LOAD_SAVE_ADDRESS
+
+	/*
+	 * Disable machine checks and critical exceptions,
+	 * if core_save_type is ALL_SAVE, we will restore interrupt
+	 * IVORs registers.
+	 */
+	mfmsr	r5
+	rlwinm	r5, r5, 0, ~MSR_CE
+	rlwinm	r5, r5, 0, ~MSR_ME
+	mtmsr	r5
+	isync
+
+	/* if core_save_type is BASE_SAVE, goto 1f */
+	cmpwi	r4, 0
+	beq	1f
+
+	bl	fsl_cpu_append_restore
+
+1:
+	bl	e500_base_special_restore
+
+	bl	fsl_cpu_base_restore
+
+	/* return the return address of the save time */
+	do_restore_gpr_reg(r9, SR_LR)
+
+	li	r3, 0
+	mtlr	r9
+	blr
-- 
1.8.5

[PATCH 3/3] powerpc/fsl: Use the new interface to save or restore registers

[Dongsheng Wang]

From: Wang Dongsheng <dongsheng.wang@freescale.com>

Use fsl_cpu_state_save/fsl_cpu_state_restore to save/restore registers.
Use the functions to save/restore registers, so we don't need to
maintain the code.

Signed-off-by: Wang Dongsheng <dongsheng.wang@freescale.com>

diff --git a/arch/powerpc/kernel/swsusp_booke.S b/arch/powerpc/kernel/swsusp_booke.S
index 553c140..b5992db 100644
--- a/arch/powerpc/kernel/swsusp_booke.S
+++ b/arch/powerpc/kernel/swsusp_booke.S
@@ -4,92 +4,28 @@
  * 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
-
+#include <asm/fsl_sleep.h>
 
 	.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
+	mflr	r15
+	lis	r3, core_registers_save_area@h
+	ori	r3, r3, core_registers_save_area@l
+
+	/* Save base register */
+	li	r4, 0
+	bl	fsl_cpu_state_save
 
-	/* Save SPRGs */
-	mfspr	r4,SPRN_SPRG0
-	stw	r4,SL_SPRG0(r11)
-	mfspr	r4,SPRN_SPRG1
-	stw	r4,SL_SPRG1(r11)
-	mfspr	r4,SPRN_SPRG2
-	stw	r4,SL_SPRG2(r11)
-	mfspr	r4,SPRN_SPRG3
-	stw	r4,SL_SPRG3(r11)
-	mfspr	r4,SPRN_SPRG4
-	stw	r4,SL_SPRG4(r11)
-	mfspr	r4,SPRN_SPRG5
-	stw	r4,SL_SPRG5(r11)
-	mfspr	r4,SPRN_SPRG6
-	stw	r4,SL_SPRG6(r11)
-	mfspr	r4,SPRN_SPRG7
-	stw	r4,SL_SPRG7(r11)
+	/* Save LR */
+	lis	r3, core_registers_save_area@h
+	ori	r3, r3, core_registers_save_area@l
+	stw	r15, SR_LR(r3)
 
 	/* Call the low level suspend stuff (we should probably have made
 	 * a stackframe...
@@ -97,11 +33,12 @@ _GLOBAL(swsusp_arch_suspend)
 	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
+	lis	r3, core_registers_save_area@h
+	ori	r3, r3, core_registers_save_area@l
+	lwz	r15, SR_LR(r3)
+	mtlr	r15
 
+	li	r3, 0
 	blr
 
 _GLOBAL(swsusp_arch_resume)
@@ -138,9 +75,6 @@ _GLOBAL(swsusp_arch_resume)
 	bl flush_dcache_L1
 	bl flush_instruction_cache
 
-	lis	r11,swsusp_save_area@h
-	ori	r11,r11,swsusp_save_area@l
-
 	/*
 	 * Mappings from virtual addresses to physical addresses may be
 	 * different than they were prior to restoring hibernation state. 
@@ -149,53 +83,12 @@ _GLOBAL(swsusp_arch_resume)
 	 */
 	bl	_tlbil_all
 
-	lwz	r4,SL_SPRG0(r11)
-	mtspr	SPRN_SPRG0,r4
-	lwz	r4,SL_SPRG1(r11)
-	mtspr	SPRN_SPRG1,r4
-	lwz	r4,SL_SPRG2(r11)
-	mtspr	SPRN_SPRG2,r4
-	lwz	r4,SL_SPRG3(r11)
-	mtspr	SPRN_SPRG3,r4
-	lwz	r4,SL_SPRG4(r11)
-	mtspr	SPRN_SPRG4,r4
-	lwz	r4,SL_SPRG5(r11)
-	mtspr	SPRN_SPRG5,r4
-	lwz	r4,SL_SPRG6(r11)
-	mtspr	SPRN_SPRG6,r4
-	lwz	r4,SL_SPRG7(r11)
-	mtspr	SPRN_SPRG7,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
+	lis	r3, core_registers_save_area@h
+	ori	r3, r3, core_registers_save_area@l
+
+	/* Restore base register */
+	li	r4, 0
+	bl	fsl_cpu_state_restore
 
 	li	r3,0
 	blr
-- 
1.8.5

Re: [PATCH 3/3] powerpc/fsl: Use the new interface to save or restore registers

[Scott Wood]

On Tue, 2014-01-14 at 15:59 +0800, Dongsheng Wang wrote:
> From: Wang Dongsheng <dongsheng.wang@freescale.com>
> 
> Use fsl_cpu_state_save/fsl_cpu_state_restore to save/restore registers.
> Use the functions to save/restore registers, so we don't need to
> maintain the code.
> 
> Signed-off-by: Wang Dongsheng <dongsheng.wang@freescale.com>

Is there any functional change with this patchset (e.g. suspend
supported on chips where it wasn't before), or is it just cleanup?  A
cover letter would be useful to describe the purpose of the overall
patchset when it isn't obvious.

> 
> diff --git a/arch/powerpc/kernel/swsusp_booke.S b/arch/powerpc/kernel/swsusp_booke.S
> index 553c140..b5992db 100644
> --- a/arch/powerpc/kernel/swsusp_booke.S
> +++ b/arch/powerpc/kernel/swsusp_booke.S
> @@ -4,92 +4,28 @@
>   * 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
> -
> +#include <asm/fsl_sleep.h>
>  
>  	.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
> +	mflr	r15
> +	lis	r3, core_registers_save_area@h
> +	ori	r3, r3, core_registers_save_area@l
> +
> +	/* Save base register */
> +	li	r4, 0
> +	bl	fsl_cpu_state_save
>  
> -	/* Save SPRGs */
> -	mfspr	r4,SPRN_SPRG0
> -	stw	r4,SL_SPRG0(r11)
> -	mfspr	r4,SPRN_SPRG1
> -	stw	r4,SL_SPRG1(r11)
> -	mfspr	r4,SPRN_SPRG2
> -	stw	r4,SL_SPRG2(r11)
> -	mfspr	r4,SPRN_SPRG3
> -	stw	r4,SL_SPRG3(r11)
> -	mfspr	r4,SPRN_SPRG4
> -	stw	r4,SL_SPRG4(r11)
> -	mfspr	r4,SPRN_SPRG5
> -	stw	r4,SL_SPRG5(r11)
> -	mfspr	r4,SPRN_SPRG6
> -	stw	r4,SL_SPRG6(r11)
> -	mfspr	r4,SPRN_SPRG7
> -	stw	r4,SL_SPRG7(r11)
> +	/* Save LR */
> +	lis	r3, core_registers_save_area@h
> +	ori	r3, r3, core_registers_save_area@l
> +	stw	r15, SR_LR(r3)
>  
>  	/* Call the low level suspend stuff (we should probably have made
>  	 * a stackframe...
> @@ -97,11 +33,12 @@ _GLOBAL(swsusp_arch_suspend)
>  	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
> +	lis	r3, core_registers_save_area@h
> +	ori	r3, r3, core_registers_save_area@l
> +	lwz	r15, SR_LR(r3)
> +	mtlr	r15
>  
> +	li	r3, 0
>  	blr
>  
>  _GLOBAL(swsusp_arch_resume)
> @@ -138,9 +75,6 @@ _GLOBAL(swsusp_arch_resume)
>  	bl flush_dcache_L1
>  	bl flush_instruction_cache
>  
> -	lis	r11,swsusp_save_area@h
> -	ori	r11,r11,swsusp_save_area@l
> -
>  	/*
>  	 * Mappings from virtual addresses to physical addresses may be
>  	 * different than they were prior to restoring hibernation state. 
> @@ -149,53 +83,12 @@ _GLOBAL(swsusp_arch_resume)
>  	 */
>  	bl	_tlbil_all
>  
> -	lwz	r4,SL_SPRG0(r11)
> -	mtspr	SPRN_SPRG0,r4
> -	lwz	r4,SL_SPRG1(r11)
> -	mtspr	SPRN_SPRG1,r4
> -	lwz	r4,SL_SPRG2(r11)
> -	mtspr	SPRN_SPRG2,r4
> -	lwz	r4,SL_SPRG3(r11)
> -	mtspr	SPRN_SPRG3,r4
> -	lwz	r4,SL_SPRG4(r11)
> -	mtspr	SPRN_SPRG4,r4
> -	lwz	r4,SL_SPRG5(r11)
> -	mtspr	SPRN_SPRG5,r4
> -	lwz	r4,SL_SPRG6(r11)
> -	mtspr	SPRN_SPRG6,r4
> -	lwz	r4,SL_SPRG7(r11)
> -	mtspr	SPRN_SPRG7,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
> +	lis	r3, core_registers_save_area@h
> +	ori	r3, r3, core_registers_save_area@l
> +
> +	/* Restore base register */
> +	li	r4, 0
> +	bl	fsl_cpu_state_restore

Why are you calling anything with "fsl" in the name from code that is
supposed to be for all booke?

-Scott

Re: [PATCH 2/3] powerpc/85xx: Provide two functions to save/restore the core registers

[Scott Wood]

On Tue, 2014-01-14 at 15:59 +0800, Dongsheng Wang wrote:
> From: Wang Dongsheng <dongsheng.wang@freescale.com>
> 
> Add fsl_cpu_state_save/fsl_cpu_state_restore functions, used for deep
> sleep and hibernation to save/restore core registers. We abstract out
> save/restore code for use in various modules, to make them don't need
> to maintain.
> 
> Currently supported processors type are E6500, E5500, E500MC, E500v2 and
> E500v1.
> 
> Signed-off-by: Wang Dongsheng <dongsheng.wang@freescale.com>

What is there that is specfic to a particular core type that can't be
handled from C code?

> +	/*
> +	 * Need to save float-point registers if MSR[FP] = 1.
> +	 */
> +	mfmsr	r12
> +	andi.	r12, r12, MSR_FP
> +	beq	1f
> +	do_sr_fpr_regs(save)

C code should have already ensured that MSR[FP] is not 1 (and thus the
FP context has been saved).

> +/*
> + * r3 = the virtual address of buffer
> + * r4 = suspend type, 0-BASE_SAVE, 1-ALL_SAVE

#define these magic numbers, and define what is meant by "base save"
versus "all save".

-Scott

RE: [PATCH 3/3] powerpc/fsl: Use the new interface to save or restore registers

[Dongsheng.Wang]

DQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogV29vZCBTY290dC1CMDc0
MjENCj4gU2VudDogV2VkbmVzZGF5LCBKYW51YXJ5IDE1LCAyMDE0IDc6MzAgQU0NCj4gVG86IFdh
bmcgRG9uZ3NoZW5nLUI0MDUzNA0KPiBDYzogYmVuaEBrZXJuZWwuY3Jhc2hpbmcub3JnOyBaaGFv
IENoZW5odWktQjM1MzM2OyBhbnRvbkBlbm9tc2cub3JnOyBsaW51eHBwYy0NCj4gZGV2QGxpc3Rz
Lm96bGFicy5vcmcNCj4gU3ViamVjdDogUmU6IFtQQVRDSCAzLzNdIHBvd2VycGMvZnNsOiBVc2Ug
dGhlIG5ldyBpbnRlcmZhY2UgdG8gc2F2ZSBvciByZXN0b3JlDQo+IHJlZ2lzdGVycw0KPiANCj4g
T24gVHVlLCAyMDE0LTAxLTE0IGF0IDE1OjU5ICswODAwLCBEb25nc2hlbmcgV2FuZyB3cm90ZToN
Cj4gPiBGcm9tOiBXYW5nIERvbmdzaGVuZyA8ZG9uZ3NoZW5nLndhbmdAZnJlZXNjYWxlLmNvbT4N
Cj4gPg0KPiA+IFVzZSBmc2xfY3B1X3N0YXRlX3NhdmUvZnNsX2NwdV9zdGF0ZV9yZXN0b3JlIHRv
IHNhdmUvcmVzdG9yZSByZWdpc3RlcnMuDQo+ID4gVXNlIHRoZSBmdW5jdGlvbnMgdG8gc2F2ZS9y
ZXN0b3JlIHJlZ2lzdGVycywgc28gd2UgZG9uJ3QgbmVlZCB0bw0KPiA+IG1haW50YWluIHRoZSBj
b2RlLg0KPiA+DQo+ID4gU2lnbmVkLW9mZi1ieTogV2FuZyBEb25nc2hlbmcgPGRvbmdzaGVuZy53
YW5nQGZyZWVzY2FsZS5jb20+DQo+IA0KPiBJcyB0aGVyZSBhbnkgZnVuY3Rpb25hbCBjaGFuZ2Ug
d2l0aCB0aGlzIHBhdGNoc2V0IChlLmcuIHN1c3BlbmQNCj4gc3VwcG9ydGVkIG9uIGNoaXBzIHdo
ZXJlIGl0IHdhc24ndCBiZWZvcmUpLCBvciBpcyBpdCBqdXN0IGNsZWFudXA/ICBBDQo+IGNvdmVy
IGxldHRlciB3b3VsZCBiZSB1c2VmdWwgdG8gZGVzY3JpYmUgdGhlIHB1cnBvc2Ugb2YgdGhlIG92
ZXJhbGwNCj4gcGF0Y2hzZXQgd2hlbiBpdCBpc24ndCBvYnZpb3VzLg0KPiANCg0KWWVzLCBqdXN0
IGNsZWFudXAuLg0KDQo+ID4gKw0KPiA+ICsJLyogUmVzdG9yZSBiYXNlIHJlZ2lzdGVyICovDQo+
ID4gKwlsaQlyNCwgMA0KPiA+ICsJYmwJZnNsX2NwdV9zdGF0ZV9yZXN0b3JlDQo+IA0KPiBXaHkg
YXJlIHlvdSBjYWxsaW5nIGFueXRoaW5nIHdpdGggImZzbCIgaW4gdGhlIG5hbWUgZnJvbSBjb2Rl
IHRoYXQgaXMNCj4gc3VwcG9zZWQgdG8gYmUgZm9yIGFsbCBib29rZT8NCj4gDQpFMjAwLCBFMzAw
IG5vdCBzdXBwb3J0Lg0KU3VwcG9ydCBFNTAwLCBFNTAwdjIsIEU1MDBNQywgRTU1MDAsIEU2NTAw
Lg0KDQpEbyB5b3UgaGF2ZSBhbnkgc3VnZ2VzdGlvbnMgYWJvdXQgdGhpcz8NCg0KVGhhbmtzLA0K
LURvbmdzaGVuZw0KDQo=

RE: [PATCH 2/3] powerpc/85xx: Provide two functions to save/restore the core registers

[Dongsheng.Wang]

DQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogV29vZCBTY290dC1CMDc0
MjENCj4gU2VudDogV2VkbmVzZGF5LCBKYW51YXJ5IDE1LCAyMDE0IDc6NTEgQU0NCj4gVG86IFdh
bmcgRG9uZ3NoZW5nLUI0MDUzNA0KPiBDYzogYmVuaEBrZXJuZWwuY3Jhc2hpbmcub3JnOyBaaGFv
IENoZW5odWktQjM1MzM2OyBhbnRvbkBlbm9tc2cub3JnOyBsaW51eHBwYy0NCj4gZGV2QGxpc3Rz
Lm96bGFicy5vcmcNCj4gU3ViamVjdDogUmU6IFtQQVRDSCAyLzNdIHBvd2VycGMvODV4eDogUHJv
dmlkZSB0d28gZnVuY3Rpb25zIHRvIHNhdmUvcmVzdG9yZSB0aGUNCj4gY29yZSByZWdpc3RlcnMN
Cj4gDQo+IE9uIFR1ZSwgMjAxNC0wMS0xNCBhdCAxNTo1OSArMDgwMCwgRG9uZ3NoZW5nIFdhbmcg
d3JvdGU6DQo+ID4gRnJvbTogV2FuZyBEb25nc2hlbmcgPGRvbmdzaGVuZy53YW5nQGZyZWVzY2Fs
ZS5jb20+DQo+ID4NCj4gPiBBZGQgZnNsX2NwdV9zdGF0ZV9zYXZlL2ZzbF9jcHVfc3RhdGVfcmVz
dG9yZSBmdW5jdGlvbnMsIHVzZWQgZm9yIGRlZXANCj4gPiBzbGVlcCBhbmQgaGliZXJuYXRpb24g
dG8gc2F2ZS9yZXN0b3JlIGNvcmUgcmVnaXN0ZXJzLiBXZSBhYnN0cmFjdCBvdXQNCj4gPiBzYXZl
L3Jlc3RvcmUgY29kZSBmb3IgdXNlIGluIHZhcmlvdXMgbW9kdWxlcywgdG8gbWFrZSB0aGVtIGRv
bid0IG5lZWQNCj4gPiB0byBtYWludGFpbi4NCj4gPg0KPiA+IEN1cnJlbnRseSBzdXBwb3J0ZWQg
cHJvY2Vzc29ycyB0eXBlIGFyZSBFNjUwMCwgRTU1MDAsIEU1MDBNQywgRTUwMHYyDQo+ID4gYW5k
IEU1MDB2MS4NCj4gPg0KPiA+IFNpZ25lZC1vZmYtYnk6IFdhbmcgRG9uZ3NoZW5nIDxkb25nc2hl
bmcud2FuZ0BmcmVlc2NhbGUuY29tPg0KPiANCj4gV2hhdCBpcyB0aGVyZSB0aGF0IGlzIHNwZWNm
aWMgdG8gYSBwYXJ0aWN1bGFyIGNvcmUgdHlwZSB0aGF0IGNhbid0IGJlIGhhbmRsZWQNCj4gZnJv
bSBDIGNvZGU/DQo+IA0KDQpJbiB0aGUgY29udGV4dCBvZiB0aGUgY2FsbGluZywgbWF5YmUgbm90
IGluIEMgZW52aXJvbm1lbnQuKERlZXAgc2xlZXAgd2l0aG91dA0KQyBlbnZpcm9ubWVudCB3aGVu
IGNhbGxpbmcgdGhvc2UgaW50ZXJmYWNlcykNCg0KPiA+ICsJLyoNCj4gPiArCSAqIE5lZWQgdG8g
c2F2ZSBmbG9hdC1wb2ludCByZWdpc3RlcnMgaWYgTVNSW0ZQXSA9IDEuDQo+ID4gKwkgKi8NCj4g
PiArCW1mbXNyCXIxMg0KPiA+ICsJYW5kaS4JcjEyLCByMTIsIE1TUl9GUA0KPiA+ICsJYmVxCTFm
DQo+ID4gKwlkb19zcl9mcHJfcmVncyhzYXZlKQ0KPiANCj4gQyBjb2RlIHNob3VsZCBoYXZlIGFs
cmVhZHkgZW5zdXJlZCB0aGF0IE1TUltGUF0gaXMgbm90IDEgKGFuZCB0aHVzIHRoZSBGUA0KPiBj
b250ZXh0IGhhcyBiZWVuIHNhdmVkKS4NCj4gDQoNClllcywgcmlnaHQuIEJ1dCBJIG1lYW4gaWYg
dGhlIEZQIHN0aWxsIHVzZSBpbiBjb3JlIHNhdmUgZmxvdywgd2UgbmVlZCB0byBzYXZlIGl0Lg0K
SW4gdGhpcyBwcm9jZXNzLCBpIGRvbid0IGNhcmUgd2hhdCBvdGhlciBjb2RlIGRvLCB3ZSBuZWVk
IHRvIGZvY3VzIG9uIG5vdCBsb3NpbmcNCnZhbHVhYmxlIGRhdGEuDQoNCj4gPiArLyoNCj4gPiAr
ICogcjMgPSB0aGUgdmlydHVhbCBhZGRyZXNzIG9mIGJ1ZmZlcg0KPiA+ICsgKiByNCA9IHN1c3Bl
bmQgdHlwZSwgMC1CQVNFX1NBVkUsIDEtQUxMX1NBVkUNCj4gDQo+ICNkZWZpbmUgdGhlc2UgbWFn
aWMgbnVtYmVycywgYW5kIGRlZmluZSB3aGF0IGlzIG1lYW50IGJ5ICJiYXNlIHNhdmUiDQo+IHZl
cnN1cyAiYWxsIHNhdmUiLg0KDQpPaywgdGhhbmtzLg0KDQotRG9uZ3NoZW5nDQoNCg==

Re: [PATCH 3/3] powerpc/fsl: Use the new interface to save or restore registers

[Scott Wood]

On Tue, 2014-01-14 at 20:57 -0600, Wang Dongsheng-B40534 wrote:
> 
> > -----Original Message-----
> > From: Wood Scott-B07421
> > Sent: Wednesday, January 15, 2014 7:30 AM
> > To: Wang Dongsheng-B40534
> > Cc: benh@kernel.crashing.org; Zhao Chenhui-B35336; anton@enomsg.org; linuxppc-
> > dev@lists.ozlabs.org
> > Subject: Re: [PATCH 3/3] powerpc/fsl: Use the new interface to save or restore
> > registers
> > 
> > On Tue, 2014-01-14 at 15:59 +0800, Dongsheng Wang wrote:
> > > From: Wang Dongsheng <dongsheng.wang@freescale.com>
> > >
> > > Use fsl_cpu_state_save/fsl_cpu_state_restore to save/restore registers.
> > > Use the functions to save/restore registers, so we don't need to
> > > maintain the code.
> > >
> > > Signed-off-by: Wang Dongsheng <dongsheng.wang@freescale.com>
> > 
> > Is there any functional change with this patchset (e.g. suspend
> > supported on chips where it wasn't before), or is it just cleanup?  A
> > cover letter would be useful to describe the purpose of the overall
> > patchset when it isn't obvious.
> > 
> 
> Yes, just cleanup..

It seems to be introducing complexity rather than removing it.  Is this
cleanup needed to prepare for adding new functionality?

Plus, I'm skeptical that this is functionally equivalent.  It looks like
the new code saves a lot more than the old code does.  Why?

> > > +
> > > +	/* Restore base register */
> > > +	li	r4, 0
> > > +	bl	fsl_cpu_state_restore
> > 
> > Why are you calling anything with "fsl" in the name from code that is
> > supposed to be for all booke?
> > 
> E200, E300 not support.
> Support E500, E500v2, E500MC, E5500, E6500.
> 
> Do you have any suggestions about this?

What about non-FSL booke such as 44x?

Or if this file never supported 44x, rename it appropriately.

-Scott

Re: [PATCH 2/3] powerpc/85xx: Provide two functions to save/restore the core registers

[Scott Wood]

On Tue, 2014-01-14 at 21:30 -0600, Wang Dongsheng-B40534 wrote:
> 
> > -----Original Message-----
> > From: Wood Scott-B07421
> > Sent: Wednesday, January 15, 2014 7:51 AM
> > To: Wang Dongsheng-B40534
> > Cc: benh@kernel.crashing.org; Zhao Chenhui-B35336; anton@enomsg.org; linuxppc-
> > dev@lists.ozlabs.org
> > Subject: Re: [PATCH 2/3] powerpc/85xx: Provide two functions to save/restore the
> > core registers
> > 
> > On Tue, 2014-01-14 at 15:59 +0800, Dongsheng Wang wrote:
> > > From: Wang Dongsheng <dongsheng.wang@freescale.com>
> > >
> > > Add fsl_cpu_state_save/fsl_cpu_state_restore functions, used for deep
> > > sleep and hibernation to save/restore core registers. We abstract out
> > > save/restore code for use in various modules, to make them don't need
> > > to maintain.
> > >
> > > Currently supported processors type are E6500, E5500, E500MC, E500v2
> > > and E500v1.
> > >
> > > Signed-off-by: Wang Dongsheng <dongsheng.wang@freescale.com>
> > 
> > What is there that is specfic to a particular core type that can't be handled
> > from C code?
> > 
> 
> In the context of the calling, maybe not in C environment.(Deep sleep without
> C environment when calling those interfaces)

Could you provide a concrete example?

> > > +	/*
> > > +	 * Need to save float-point registers if MSR[FP] = 1.
> > > +	 */
> > > +	mfmsr	r12
> > > +	andi.	r12, r12, MSR_FP
> > > +	beq	1f
> > > +	do_sr_fpr_regs(save)
> > 
> > C code should have already ensured that MSR[FP] is not 1 (and thus the FP
> > context has been saved).
> > 
> 
> Yes, right. But I mean if the FP still use in core save flow, we need to save it.
> In this process, i don't care what other code do, we need to focus on not losing
> valuable data.

It is not allowed to use FP at that point.

-Scott

RE: [PATCH 3/3] powerpc/fsl: Use the new interface to save or restore registers

[Dongsheng.Wang]

PiA+ID4gPiBVc2UgZnNsX2NwdV9zdGF0ZV9zYXZlL2ZzbF9jcHVfc3RhdGVfcmVzdG9yZSB0byBz
YXZlL3Jlc3RvcmUgcmVnaXN0ZXJzLg0KPiA+ID4gPiBVc2UgdGhlIGZ1bmN0aW9ucyB0byBzYXZl
L3Jlc3RvcmUgcmVnaXN0ZXJzLCBzbyB3ZSBkb24ndCBuZWVkIHRvDQo+ID4gPiA+IG1haW50YWlu
IHRoZSBjb2RlLg0KPiA+ID4gPg0KPiA+ID4gPiBTaWduZWQtb2ZmLWJ5OiBXYW5nIERvbmdzaGVu
ZyA8ZG9uZ3NoZW5nLndhbmdAZnJlZXNjYWxlLmNvbT4NCj4gPiA+DQo+ID4gPiBJcyB0aGVyZSBh
bnkgZnVuY3Rpb25hbCBjaGFuZ2Ugd2l0aCB0aGlzIHBhdGNoc2V0IChlLmcuIHN1c3BlbmQNCj4g
PiA+IHN1cHBvcnRlZCBvbiBjaGlwcyB3aGVyZSBpdCB3YXNuJ3QgYmVmb3JlKSwgb3IgaXMgaXQg
anVzdCBjbGVhbnVwPyAgQQ0KPiA+ID4gY292ZXIgbGV0dGVyIHdvdWxkIGJlIHVzZWZ1bCB0byBk
ZXNjcmliZSB0aGUgcHVycG9zZSBvZiB0aGUgb3ZlcmFsbA0KPiA+ID4gcGF0Y2hzZXQgd2hlbiBp
dCBpc24ndCBvYnZpb3VzLg0KPiA+ID4NCj4gPg0KPiA+IFllcywganVzdCBjbGVhbnVwLi4NCj4g
DQo+IEl0IHNlZW1zIHRvIGJlIGludHJvZHVjaW5nIGNvbXBsZXhpdHkgcmF0aGVyIHRoYW4gcmVt
b3ZpbmcgaXQuICBJcyB0aGlzDQo+IGNsZWFudXAgbmVlZGVkIHRvIHByZXBhcmUgZm9yIGFkZGlu
ZyBuZXcgZnVuY3Rpb25hbGl0eT8NCj4gDQo+IFBsdXMsIEknbSBza2VwdGljYWwgdGhhdCB0aGlz
IGlzIGZ1bmN0aW9uYWxseSBlcXVpdmFsZW50LiAgSXQgbG9va3MgbGlrZQ0KPiB0aGUgbmV3IGNv
ZGUgc2F2ZXMgYSBsb3QgbW9yZSB0aGFuIHRoZSBvbGQgY29kZSBkb2VzLiAgV2h5Pw0KPiANCg0K
QWN0dWFsbHksIEkgd2FudCB0byB0YWtlIGEgcHJhY3RpY2FsIGV4YW1wbGUgdG8gcHVzaCB0aGUg
c2F2ZS9yZXN0b3JlIHBhdGNoZXMuDQpBbmQgdGhpcyBpcyBhbHNvIHJlYXNvbmFibGUgZm9yIDMy
Yml0LWhpYmVybmF0aW9uLCB0aGUgY29kZSBpcyBtb3JlIGNsZWFuLiA6KQ0KSSB0aGluayBJIG5l
ZWQgdG8gY2hhbmdlIHRoZSBkZXNjcmlwdGlvbiBvZiB0aGUgcGF0Y2guDQoNCj4gPiA+ID4gKw0K
PiA+ID4gPiArCS8qIFJlc3RvcmUgYmFzZSByZWdpc3RlciAqLw0KPiA+ID4gPiArCWxpCXI0LCAw
DQo+ID4gPiA+ICsJYmwJZnNsX2NwdV9zdGF0ZV9yZXN0b3JlDQo+ID4gPg0KPiA+ID4gV2h5IGFy
ZSB5b3UgY2FsbGluZyBhbnl0aGluZyB3aXRoICJmc2wiIGluIHRoZSBuYW1lIGZyb20gY29kZSB0
aGF0IGlzDQo+ID4gPiBzdXBwb3NlZCB0byBiZSBmb3IgYWxsIGJvb2tlPw0KPiA+ID4NCj4gPiBF
MjAwLCBFMzAwIG5vdCBzdXBwb3J0Lg0KPiA+IFN1cHBvcnQgRTUwMCwgRTUwMHYyLCBFNTAwTUMs
IEU1NTAwLCBFNjUwMC4NCj4gPg0KPiA+IERvIHlvdSBoYXZlIGFueSBzdWdnZXN0aW9ucyBhYm91
dCB0aGlzPw0KPiANCj4gV2hhdCBhYm91dCBub24tRlNMIGJvb2tlIHN1Y2ggYXMgNDR4Pw0KPiAN
Cj4gT3IgaWYgdGhpcyBmaWxlIG5ldmVyIHN1cHBvcnRlZCA0NHgsIHJlbmFtZSBpdCBhcHByb3By
aWF0ZWx5Lg0KPiANCkN1cnJlbnRseSBkb2VzIG5vdCBzdXBwb3J0LiBvayBjaGFuZ2UgdGhlIG5h
bWUgZmlyc3QsIGlmIGxhdGVyIHN1cHBvcnQsIGFuZA0KdGhlbiBhZ2FpbiB0byBtb2RpZnkgdGhl
IG5hbWUgb2YgdGhpcyBmdW5jdGlvbi4NCg0KSG93IGFib3V0IDg1eHhfY3B1X3N0YXRlX3Jlc3Rv
cmU/DQoNClRoYW5rcywNCi1Eb25nc2hlbmcNCg==

RE: [PATCH 2/3] powerpc/85xx: Provide two functions to save/restore the core registers

[Dongsheng.Wang]

PiA+ID4gV2hhdCBpcyB0aGVyZSB0aGF0IGlzIHNwZWNmaWMgdG8gYSBwYXJ0aWN1bGFyIGNvcmUg
dHlwZSB0aGF0IGNhbid0IGJlDQo+IGhhbmRsZWQNCj4gPiA+IGZyb20gQyBjb2RlPw0KPiA+ID4N
Cj4gPg0KPiA+IEluIHRoZSBjb250ZXh0IG9mIHRoZSBjYWxsaW5nLCBtYXliZSBub3QgaW4gQyBl
bnZpcm9ubWVudC4oRGVlcCBzbGVlcCB3aXRob3V0DQo+ID4gQyBlbnZpcm9ubWVudCB3aGVuIGNh
bGxpbmcgdGhvc2UgaW50ZXJmYWNlcykNCj4gDQo+IENvdWxkIHlvdSBwcm92aWRlIGEgY29uY3Jl
dGUgZXhhbXBsZT8NCj4gDQoNCjopDQpEZWVwIHNsZWVwLCB0aGUgcGF0Y2hlcyB3aWxsIGNvbWVz
IG91dCBzb29uLg0KDQo+ID4gPiA+ICsJLyoNCj4gPiA+ID4gKwkgKiBOZWVkIHRvIHNhdmUgZmxv
YXQtcG9pbnQgcmVnaXN0ZXJzIGlmIE1TUltGUF0gPSAxLg0KPiA+ID4gPiArCSAqLw0KPiA+ID4g
PiArCW1mbXNyCXIxMg0KPiA+ID4gPiArCWFuZGkuCXIxMiwgcjEyLCBNU1JfRlANCj4gPiA+ID4g
KwliZXEJMWYNCj4gPiA+ID4gKwlkb19zcl9mcHJfcmVncyhzYXZlKQ0KPiA+ID4NCj4gPiA+IEMg
Y29kZSBzaG91bGQgaGF2ZSBhbHJlYWR5IGVuc3VyZWQgdGhhdCBNU1JbRlBdIGlzIG5vdCAxIChh
bmQgdGh1cyB0aGUgRlANCj4gPiA+IGNvbnRleHQgaGFzIGJlZW4gc2F2ZWQpLg0KPiA+ID4NCj4g
Pg0KPiA+IFllcywgcmlnaHQuIEJ1dCBJIG1lYW4gaWYgdGhlIEZQIHN0aWxsIHVzZSBpbiBjb3Jl
IHNhdmUgZmxvdywgd2UgbmVlZCB0byBzYXZlDQo+IGl0Lg0KPiA+IEluIHRoaXMgcHJvY2Vzcywg
aSBkb24ndCBjYXJlIHdoYXQgb3RoZXIgY29kZSBkbywgd2UgbmVlZCB0byBmb2N1cyBvbiBub3QN
Cj4gbG9zaW5nDQo+ID4gdmFsdWFibGUgZGF0YS4NCj4gDQo+IEl0IGlzIG5vdCBhbGxvd2VkIHRv
IHVzZSBGUCBhdCB0aGF0IHBvaW50Lg0KPiANCklmIE1TUltGUF0gbm90IGFjdGl2ZSwgdGhhdCBp
cyBGUCBub3QgYWxsb3dlZCB0byB1c2UuDQpCdXQgaGVyZSBpcyBhIG5vcm1hbCBqdWRnbWVudCwg
aWYgTVNSW0ZQXSBpcyBhY3RpdmUsIHRoaXMgbWVhbnMgdGhhdCB0aGUgZmxvYXRpbmcNCnBvaW50
IG1vZHVsZSBpcyBiZWluZyB1c2VkLiBJIG9mZmVyIGlzIGEgZnVuY3Rpb24gb2YgdGhlIGludGVy
ZmFjZSwgd2UgZG9uJ3Qga25vdw0Kd2hlcmUgaXMgdGhlIGZ1bmN0aW9uIHdpbGwgYmUgY2FsbGVk
LiBKdXN0IGJlY2F1c2Ugd2UgY2FsbCB0aGlzIGZ1bmN0aW9uIGluIHRoZQ0KY29udGV4dCBvZiB1
bmNlcnRhaW50eSwgd2UgbmVlZCB0aGlzIGp1ZGdtZW50IHRvIGVuc3VyZSB0aGF0IG5vIGRhdGEg
aXMgbG9zdC4NCg0KVGhhbmtzLA0KLURvbmdzaGVuZw0K

Re: [PATCH 2/3] powerpc/85xx: Provide two functions to save/restore the core registers

[Scott Wood]

On Mon, 2014-01-20 at 00:03 -0600, Wang Dongsheng-B40534 wrote:
> > > > > +	/*
> > > > > +	 * Need to save float-point registers if MSR[FP] = 1.
> > > > > +	 */
> > > > > +	mfmsr	r12
> > > > > +	andi.	r12, r12, MSR_FP
> > > > > +	beq	1f
> > > > > +	do_sr_fpr_regs(save)
> > > >
> > > > C code should have already ensured that MSR[FP] is not 1 (and thus the FP
> > > > context has been saved).
> > > >
> > >
> > > Yes, right. But I mean if the FP still use in core save flow, we need to save
> > it.
> > > In this process, i don't care what other code do, we need to focus on not
> > losing
> > > valuable data.
> > 
> > It is not allowed to use FP at that point.
> > 
> If MSR[FP] not active, that is FP not allowed to use.
> But here is a normal judgment, if MSR[FP] is active, this means that the floating
> point module is being used. I offer is a function of the interface, we don't know
> where is the function will be called. Just because we call this function in the
> context of uncertainty, we need this judgment to ensure that no data is lost.

The whole point of calling enable_kernel_fp() in C code before
suspending is to ensure that the FP state gets saved.  If FP is used
after that point it is a bug.  If you're worried about such bugs, then
clear MSR[FP] after calling enable_kernel_fp(), rather than adding
redundant state saving.

-Scott

RE: [PATCH 2/3] powerpc/85xx: Provide two functions to save/restore the core registers

[Dongsheng.Wang]

DQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogV29vZCBTY290dC1CMDc0
MjENCj4gU2VudDogVHVlc2RheSwgSmFudWFyeSAyMSwgMjAxNCA5OjA2IEFNDQo+IFRvOiBXYW5n
IERvbmdzaGVuZy1CNDA1MzQNCj4gQ2M6IGJlbmhAa2VybmVsLmNyYXNoaW5nLm9yZzsgWmhhbyBD
aGVuaHVpLUIzNTMzNjsgYW50b25AZW5vbXNnLm9yZzsgbGludXhwcGMtDQo+IGRldkBsaXN0cy5v
emxhYnMub3JnDQo+IFN1YmplY3Q6IFJlOiBbUEFUQ0ggMi8zXSBwb3dlcnBjLzg1eHg6IFByb3Zp
ZGUgdHdvIGZ1bmN0aW9ucyB0byBzYXZlL3Jlc3RvcmUgdGhlDQo+IGNvcmUgcmVnaXN0ZXJzDQo+
IA0KPiBPbiBNb24sIDIwMTQtMDEtMjAgYXQgMDA6MDMgLTA2MDAsIFdhbmcgRG9uZ3NoZW5nLUI0
MDUzNCB3cm90ZToNCj4gPiA+ID4gPiA+ICsJLyoNCj4gPiA+ID4gPiA+ICsJICogTmVlZCB0byBz
YXZlIGZsb2F0LXBvaW50IHJlZ2lzdGVycyBpZiBNU1JbRlBdID0gMS4NCj4gPiA+ID4gPiA+ICsJ
ICovDQo+ID4gPiA+ID4gPiArCW1mbXNyCXIxMg0KPiA+ID4gPiA+ID4gKwlhbmRpLglyMTIsIHIx
MiwgTVNSX0ZQDQo+ID4gPiA+ID4gPiArCWJlcQkxZg0KPiA+ID4gPiA+ID4gKwlkb19zcl9mcHJf
cmVncyhzYXZlKQ0KPiA+ID4gPiA+DQo+ID4gPiA+ID4gQyBjb2RlIHNob3VsZCBoYXZlIGFscmVh
ZHkgZW5zdXJlZCB0aGF0IE1TUltGUF0gaXMgbm90IDEgKGFuZCB0aHVzIHRoZQ0KPiBGUA0KPiA+
ID4gPiA+IGNvbnRleHQgaGFzIGJlZW4gc2F2ZWQpLg0KPiA+ID4gPiA+DQo+ID4gPiA+DQo+ID4g
PiA+IFllcywgcmlnaHQuIEJ1dCBJIG1lYW4gaWYgdGhlIEZQIHN0aWxsIHVzZSBpbiBjb3JlIHNh
dmUgZmxvdywgd2UgbmVlZCB0bw0KPiBzYXZlDQo+ID4gPiBpdC4NCj4gPiA+ID4gSW4gdGhpcyBw
cm9jZXNzLCBpIGRvbid0IGNhcmUgd2hhdCBvdGhlciBjb2RlIGRvLCB3ZSBuZWVkIHRvIGZvY3Vz
IG9uIG5vdA0KPiA+ID4gbG9zaW5nDQo+ID4gPiA+IHZhbHVhYmxlIGRhdGEuDQo+ID4gPg0KPiA+
ID4gSXQgaXMgbm90IGFsbG93ZWQgdG8gdXNlIEZQIGF0IHRoYXQgcG9pbnQuDQo+ID4gPg0KPiA+
IElmIE1TUltGUF0gbm90IGFjdGl2ZSwgdGhhdCBpcyBGUCBub3QgYWxsb3dlZCB0byB1c2UuDQo+
ID4gQnV0IGhlcmUgaXMgYSBub3JtYWwganVkZ21lbnQsIGlmIE1TUltGUF0gaXMgYWN0aXZlLCB0
aGlzIG1lYW5zIHRoYXQgdGhlDQo+IGZsb2F0aW5nDQo+ID4gcG9pbnQgbW9kdWxlIGlzIGJlaW5n
IHVzZWQuIEkgb2ZmZXIgaXMgYSBmdW5jdGlvbiBvZiB0aGUgaW50ZXJmYWNlLCB3ZSBkb24ndA0K
PiBrbm93DQo+ID4gd2hlcmUgaXMgdGhlIGZ1bmN0aW9uIHdpbGwgYmUgY2FsbGVkLiBKdXN0IGJl
Y2F1c2Ugd2UgY2FsbCB0aGlzIGZ1bmN0aW9uIGluDQo+IHRoZQ0KPiA+IGNvbnRleHQgb2YgdW5j
ZXJ0YWludHksIHdlIG5lZWQgdGhpcyBqdWRnbWVudCB0byBlbnN1cmUgdGhhdCBubyBkYXRhIGlz
IGxvc3QuDQo+IA0KPiBUaGUgd2hvbGUgcG9pbnQgb2YgY2FsbGluZyBlbmFibGVfa2VybmVsX2Zw
KCkgaW4gQyBjb2RlIGJlZm9yZQ0KPiBzdXNwZW5kaW5nIGlzIHRvIGVuc3VyZSB0aGF0IHRoZSBG
UCBzdGF0ZSBnZXRzIHNhdmVkLiAgSWYgRlAgaXMgdXNlZA0KPiBhZnRlciB0aGF0IHBvaW50IGl0
IGlzIGEgYnVnLiAgSWYgeW91J3JlIHdvcnJpZWQgYWJvdXQgc3VjaCBidWdzLCB0aGVuDQo+IGNs
ZWFyIE1TUltGUF0gYWZ0ZXIgY2FsbGluZyBlbmFibGVfa2VybmVsX2ZwKCksIHJhdGhlciB0aGFu
IGFkZGluZw0KPiByZWR1bmRhbnQgc3RhdGUgc2F2aW5nLg0KPiANCg0KZW5hYmxlX2tlcm5lbF9m
cCgpIGNhbGxpbmcgaW4gTUVNIHN1c3BlbmQgZmxvdy4NCkhpYmVybmF0aW9uIGlzIGRpZmZlcmVu
dCB3aXRoIE1FTSBzdXNwZW5kLCBhbmQgSSdtIG5vdCBzdXJlIHdoZXJlIHdpbGwgY2FsbCB0aGlz
DQppbnRlcmZhY2UsIHNvIHdlIG5lZWQgdG8gZW5zdXJlIHRoZSBpbnRlZ3JpdHkgb2YgdGhlIGNv
cmUgc2F2aW5nLiBJIGRvbid0IHRoaW5rDQp0aGlzIGNvZGUgaXMgKnJlZHVuZGFudCouIEkgdHJ1
c3QgdGhhdCB0aGUga2VybmVsIGNhbiBrZWVwIHRoZSBGUCByZWxhdGVkDQpvcGVyYXRpb25zLCB0
aGF0J3Mgd2h5IGEganVkZ21lbnQgaXMgaGVyZS4gOikNCg0KVGhhbmtzLA0KLURvbmdzaGVuZw0K

Re: [PATCH 3/3] powerpc/fsl: Use the new interface to save or restore registers

[Scott Wood]

On Sun, 2014-01-19 at 23:57 -0600, Wang Dongsheng-B40534 wrote:
> > > > > Use fsl_cpu_state_save/fsl_cpu_state_restore to save/restore registers.
> > > > > Use the functions to save/restore registers, so we don't need to
> > > > > maintain the code.
> > > > >
> > > > > Signed-off-by: Wang Dongsheng <dongsheng.wang@freescale.com>
> > > >
> > > > Is there any functional change with this patchset (e.g. suspend
> > > > supported on chips where it wasn't before), or is it just cleanup?  A
> > > > cover letter would be useful to describe the purpose of the overall
> > > > patchset when it isn't obvious.
> > > >
> > >
> > > Yes, just cleanup..
> > 
> > It seems to be introducing complexity rather than removing it.  Is this
> > cleanup needed to prepare for adding new functionality?
> > 
> > Plus, I'm skeptical that this is functionally equivalent.  It looks like
> > the new code saves a lot more than the old code does.  Why?
> > 
> 
> Actually, I want to take a practical example to push the save/restore patches.
> And this is also reasonable for 32bit-hibernation, the code is more clean. :)
> I think I need to change the description of the patch.
> 
> > > > > +
> > > > > +	/* Restore base register */
> > > > > +	li	r4, 0
> > > > > +	bl	fsl_cpu_state_restore
> > > >
> > > > Why are you calling anything with "fsl" in the name from code that is
> > > > supposed to be for all booke?
> > > >
> > > E200, E300 not support.
> > > Support E500, E500v2, E500MC, E5500, E6500.
> > >
> > > Do you have any suggestions about this?
> > 
> > What about non-FSL booke such as 44x?
> > 
> > Or if this file never supported 44x, rename it appropriately.
> > 
> Currently does not support. ok change the name first, if later support, and
> then again to modify the name of this function.
> 
> How about 85xx_cpu_state_restore?

Symbols can't begin with numbers.  booke_cpu_state_restore would be
better (it would still provide a place for 44x to be added if somebody
actually cared about doing so).

I'm still not convinced that asm code is the place to do this, though.

-Scott

Re: [PATCH 2/3] powerpc/85xx: Provide two functions to save/restore the core registers

[Scott Wood]

On Mon, 2014-01-20 at 20:43 -0600, Wang Dongsheng-B40534 wrote:
> 
> > -----Original Message-----
> > From: Wood Scott-B07421
> > Sent: Tuesday, January 21, 2014 9:06 AM
> > To: Wang Dongsheng-B40534
> > Cc: benh@kernel.crashing.org; Zhao Chenhui-B35336; anton@enomsg.org; linuxppc-
> > dev@lists.ozlabs.org
> > Subject: Re: [PATCH 2/3] powerpc/85xx: Provide two functions to save/restore the
> > core registers
> > 
> > On Mon, 2014-01-20 at 00:03 -0600, Wang Dongsheng-B40534 wrote:
> > > > > > > +	/*
> > > > > > > +	 * Need to save float-point registers if MSR[FP] = 1.
> > > > > > > +	 */
> > > > > > > +	mfmsr	r12
> > > > > > > +	andi.	r12, r12, MSR_FP
> > > > > > > +	beq	1f
> > > > > > > +	do_sr_fpr_regs(save)
> > > > > >
> > > > > > C code should have already ensured that MSR[FP] is not 1 (and thus the
> > FP
> > > > > > context has been saved).
> > > > > >
> > > > >
> > > > > Yes, right. But I mean if the FP still use in core save flow, we need to
> > save
> > > > it.
> > > > > In this process, i don't care what other code do, we need to focus on not
> > > > losing
> > > > > valuable data.
> > > >
> > > > It is not allowed to use FP at that point.
> > > >
> > > If MSR[FP] not active, that is FP not allowed to use.
> > > But here is a normal judgment, if MSR[FP] is active, this means that the
> > floating
> > > point module is being used. I offer is a function of the interface, we don't
> > know
> > > where is the function will be called. Just because we call this function in
> > the
> > > context of uncertainty, we need this judgment to ensure that no data is lost.
> > 
> > The whole point of calling enable_kernel_fp() in C code before
> > suspending is to ensure that the FP state gets saved.  If FP is used
> > after that point it is a bug.  If you're worried about such bugs, then
> > clear MSR[FP] after calling enable_kernel_fp(), rather than adding
> > redundant state saving.
> > 
> 
> enable_kernel_fp() calling in MEM suspend flow.
> Hibernation is different with MEM suspend, and I'm not sure where will call this
> interface, so we need to ensure the integrity of the core saving. I don't think
> this code is *redundant*. I trust that the kernel can keep the FP related
> operations, that's why a judgment is here. :)

For hibernation, save_processor_state() is called first, which does
flush_fp_to_thread() which has a similar effect (though I wonder if it's
being called on the correct task for non-SMP).

-Scott

RE: [PATCH 2/3] powerpc/85xx: Provide two functions to save/restore the core registers

[Dongsheng.Wang]

PiA+ID4NCj4gPiA+IFRoZSB3aG9sZSBwb2ludCBvZiBjYWxsaW5nIGVuYWJsZV9rZXJuZWxfZnAo
KSBpbiBDIGNvZGUgYmVmb3JlDQo+ID4gPiBzdXNwZW5kaW5nIGlzIHRvIGVuc3VyZSB0aGF0IHRo
ZSBGUCBzdGF0ZSBnZXRzIHNhdmVkLiAgSWYgRlAgaXMgdXNlZA0KPiA+ID4gYWZ0ZXIgdGhhdCBw
b2ludCBpdCBpcyBhIGJ1Zy4gIElmIHlvdSdyZSB3b3JyaWVkIGFib3V0IHN1Y2ggYnVncywgdGhl
bg0KPiA+ID4gY2xlYXIgTVNSW0ZQXSBhZnRlciBjYWxsaW5nIGVuYWJsZV9rZXJuZWxfZnAoKSwg
cmF0aGVyIHRoYW4gYWRkaW5nDQo+ID4gPiByZWR1bmRhbnQgc3RhdGUgc2F2aW5nLg0KPiA+ID4N
Cj4gPg0KPiA+IGVuYWJsZV9rZXJuZWxfZnAoKSBjYWxsaW5nIGluIE1FTSBzdXNwZW5kIGZsb3cu
DQo+ID4gSGliZXJuYXRpb24gaXMgZGlmZmVyZW50IHdpdGggTUVNIHN1c3BlbmQsIGFuZCBJJ20g
bm90IHN1cmUgd2hlcmUgd2lsbCBjYWxsDQo+IHRoaXMNCj4gPiBpbnRlcmZhY2UsIHNvIHdlIG5l
ZWQgdG8gZW5zdXJlIHRoZSBpbnRlZ3JpdHkgb2YgdGhlIGNvcmUgc2F2aW5nLiBJIGRvbid0DQo+
IHRoaW5rDQo+ID4gdGhpcyBjb2RlIGlzICpyZWR1bmRhbnQqLiBJIHRydXN0IHRoYXQgdGhlIGtl
cm5lbCBjYW4ga2VlcCB0aGUgRlAgcmVsYXRlZA0KPiA+IG9wZXJhdGlvbnMsIHRoYXQncyB3aHkg
YSBqdWRnbWVudCBpcyBoZXJlLiA6KQ0KPiANCj4gRm9yIGhpYmVybmF0aW9uLCBzYXZlX3Byb2Nl
c3Nvcl9zdGF0ZSgpIGlzIGNhbGxlZCBmaXJzdCwgd2hpY2ggZG9lcw0KPiBmbHVzaF9mcF90b190
aHJlYWQoKSB3aGljaCBoYXMgYSBzaW1pbGFyIGVmZmVjdCAodGhvdWdoIEkgd29uZGVyIGlmIGl0
J3MNCj4gYmVpbmcgY2FsbGVkIG9uIHRoZSBjb3JyZWN0IHRhc2sgZm9yIG5vbi1TTVApLg0KPiAN
ClllcywgdGhhbmtzLCBJIG1pc3MgdGhpcyBjb2RlLjopDQoNCkJ1dCBJIHN0aWxsIHRoaW5rIHdl
IG5lZWQgdG8ga2VlcCB0aGlzIGp1ZGdtZW50LCBiZWNhdXNlIGkgcHJvdmlkZSBhbiBBUEkuDQpJ
ZiB5b3Ugc3RpbGwgaW5zaXN0IG9uIEkgY2FuIHJlbW92ZSAqRlAqLCBidXQgSSBkb24ndCB3YW50
IHRvIGRvIHRoaXMuLjopDQo=

RE: [PATCH 3/3] powerpc/fsl: Use the new interface to save or restore registers

[Dongsheng.Wang]

PiA+IEN1cnJlbnRseSBkb2VzIG5vdCBzdXBwb3J0LiBvayBjaGFuZ2UgdGhlIG5hbWUgZmlyc3Qs
IGlmIGxhdGVyIHN1cHBvcnQsIGFuZA0KPiA+IHRoZW4gYWdhaW4gdG8gbW9kaWZ5IHRoZSBuYW1l
IG9mIHRoaXMgZnVuY3Rpb24uDQo+ID4NCj4gPiBIb3cgYWJvdXQgODV4eF9jcHVfc3RhdGVfcmVz
dG9yZT8NCj4gDQo+IFN5bWJvbHMgY2FuJ3QgYmVnaW4gd2l0aCBudW1iZXJzLiAgYm9va2VfY3B1
X3N0YXRlX3Jlc3RvcmUgd291bGQgYmUNCj4gYmV0dGVyIChpdCB3b3VsZCBzdGlsbCBwcm92aWRl
IGEgcGxhY2UgZm9yIDQ0eCB0byBiZSBhZGRlZCBpZiBzb21lYm9keQ0KPiBhY3R1YWxseSBjYXJl
ZCBhYm91dCBkb2luZyBzbykuDQo+IA0KOikuIFRoYW5rcy4NCg0KLURvbmdzaGVuZw0KDQo=

Re: [PATCH 2/3] powerpc/85xx: Provide two functions to save/restore the core registers

[Scott Wood]

On Wed, 2014-01-22 at 20:49 -0600, Wang Dongsheng-B40534 wrote:
> > > >
> > > > The whole point of calling enable_kernel_fp() in C code before
> > > > suspending is to ensure that the FP state gets saved.  If FP is used
> > > > after that point it is a bug.  If you're worried about such bugs, then
> > > > clear MSR[FP] after calling enable_kernel_fp(), rather than adding
> > > > redundant state saving.
> > > >
> > >
> > > enable_kernel_fp() calling in MEM suspend flow.
> > > Hibernation is different with MEM suspend, and I'm not sure where will call
> > this
> > > interface, so we need to ensure the integrity of the core saving. I don't
> > think
> > > this code is *redundant*. I trust that the kernel can keep the FP related
> > > operations, that's why a judgment is here. :)
> > 
> > For hibernation, save_processor_state() is called first, which does
> > flush_fp_to_thread() which has a similar effect (though I wonder if it's
> > being called on the correct task for non-SMP).
> > 
> Yes, thanks, I miss this code.:)
> 
> But I still think we need to keep this judgment, because i provide an API.
> If you still insist on I can remove *FP*, but I don't want to do this..:)

I insist.  Redundant code wastes review and maintenance bandwidth, and
is a potential source of bugs.

-Scott