[U-Boot] [PATCH 1/3] mpc85xx: Add support for the supplement configuration unit register

[U-Boot] [PATCH 1/3] mpc85xx: Add support for the supplement configuration unit register

[Tang Yuantian]

From: Tang Yuantian <yuantian.tang@freescale.com>

The supplement configuration unit (SCFG) provides chip-specific
configuration and status registers for the device. It is the chip
defined module for extending the device configuration unit (DCFG)
module. It provides a set of CCSR registers in addition to those
available in the device configuration unit.
The base address for this unit is 0x0F_C000.

Signed-off-by: Tang Yuantian <Yuantian.Tang@freescale.com>
---
 arch/powerpc/include/asm/immap_85xx.h | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/arch/powerpc/include/asm/immap_85xx.h b/arch/powerpc/include/asm/immap_85xx.h
index 68c3c82..71d803b 100644
--- a/arch/powerpc/include/asm/immap_85xx.h
+++ b/arch/powerpc/include/asm/immap_85xx.h
@@ -3113,4 +3113,26 @@ struct dcsr_dcfg_regs {
 #define	DCSR_DCFG_ECC_DISABLE_USB2	0x00004000
 	u8  res_524[0x1000 - 0x524]; /* 0x524 - 0x1000 */
 };
+
+#define CONFIG_SYS_MPC85xx_SCFG \
+	(CONFIG_SYS_IMMR + CONFIG_SYS_MPC85xx_SCFG_OFFSET)
+#define CONFIG_SYS_MPC85xx_SCFG_OFFSET	0xfc000
+/* The supplement configuration unit register */
+struct ccsr_scfg {
+	u32 dpslpcr;		/* 0x000 Deep Sleep Control register */
+	u32 usb1dpslpcsr;	/* 0x004 USB1 Deep Sleep Control Status register */
+	u32 usb2dpslpcsr;	/* 0x008 USB2 Deep Sleep Control Status register */
+	u32 fmclkdpslpcr;	/* 0x00c FM Clock Deep Sleep Control register */
+	u32 res1[4];
+	u32 esgmiiselcr;	/* 0x020 Ethernet Switch SGMII Select Control register */
+	u32 res2;
+	u32 pixclkcr;		/* 0x028 Pixel Clock Control register */
+	u32 res3[245];
+	u32 qeioclkcr;		/* 0x400 QUICC Engine IO Clock Control register */
+	u32 emiiocr;		/* 0x404 EMI MDIO Control Register */
+	u32 sdhciovselcr;	/* 0x408 SDHC IO VSEL Control register */
+	u32 qmifrstcr;		/* 0x40c QMAN Interface Reset Control register */
+	u32 res4[60];
+	u32 sparecr[8];		/* 0x500 Spare Control register(0-7) */
+};
 #endif /*__IMMAP_85xx__*/
-- 
1.8.0

[U-Boot] [PATCH 2/3] mpc85xx: Add deep sleep framework support

[Tang Yuantian]

From: Tang Yuantian <yuantian.tang@freescale.com>

When system wakes up from warm reset, control is passed to the
primary core that starts executing uboot. After re-initialized some
IP blocks, like DDRC, kernel will take responsibility to continue
to restore environment it leaves before.

Signed-off-by: Tang Yuantian <Yuantian.Tang@freescale.com>
---
 arch/powerpc/cpu/mpc85xx/cpu_init.c    |  7 ++++
 arch/powerpc/cpu/mpc85xx/fdt.c         | 27 +++++++++++++++
 arch/powerpc/cpu/mpc85xx/liodn.c       | 24 +++++++++----
 arch/powerpc/cpu/mpc85xx/start.S       |  8 +++++
 arch/powerpc/include/asm/global_data.h |  1 +
 arch/powerpc/lib/board.c               | 61 +++++++++++++++++++++++++++++++---
 drivers/ddr/fsl/mpc85xx_ddr_gen3.c     | 44 +++++++++++++++++++++---
 include/fsl_ddr_sdram.h                |  6 ++++
 8 files changed, 163 insertions(+), 15 deletions(-)

diff --git a/arch/powerpc/cpu/mpc85xx/cpu_init.c b/arch/powerpc/cpu/mpc85xx/cpu_init.c
index b31efb7..376c659 100644
--- a/arch/powerpc/cpu/mpc85xx/cpu_init.c
+++ b/arch/powerpc/cpu/mpc85xx/cpu_init.c
@@ -231,6 +231,7 @@ void cpu_init_f (void)
 #ifdef CONFIG_MPC8548
 	ccsr_local_ecm_t *ecm = (void *)(CONFIG_SYS_MPC85xx_ECM_ADDR);
 	uint svr = get_svr();
+	gd = (gd_t *)(CONFIG_SYS_INIT_RAM_ADDR + CONFIG_SYS_GBL_DATA_OFFSET);
 
 	/*
 	 * CPU2 errata workaround: A core hang possible while executing
@@ -282,6 +283,12 @@ void cpu_init_f (void)
 	in_be32(&gur->dcsrcr);
 #endif
 
+#ifdef CONFIG_DEEP_SLEEP
+	/* disable the console if boot from warm reset */
+	if (in_be32(&gur->scrtsr[0]) & (1 << 3))
+		gd->flags |=
+			GD_FLG_SILENT | GD_FLG_WARM_BOOT | GD_FLG_DISABLE_CONSOLE;
+#endif
 }
 
 /* Implement a dummy function for those platforms w/o SERDES */
diff --git a/arch/powerpc/cpu/mpc85xx/fdt.c b/arch/powerpc/cpu/mpc85xx/fdt.c
index 33bc900..b3b9250 100644
--- a/arch/powerpc/cpu/mpc85xx/fdt.c
+++ b/arch/powerpc/cpu/mpc85xx/fdt.c
@@ -24,6 +24,9 @@ DECLARE_GLOBAL_DATA_PTR;
 extern void ft_qe_setup(void *blob);
 extern void ft_fixup_num_cores(void *blob);
 extern void ft_srio_setup(void *blob);
+#ifdef CONFIG_DEEP_SLEEP
+extern ulong __bss_end;
+#endif
 
 #ifdef CONFIG_MP
 #include "mp.h"
@@ -35,6 +38,9 @@ void ft_fixup_cpu(void *blob, u64 memory_limit)
 	u32 bootpg = determine_mp_bootpg(NULL);
 	u32 id = get_my_id();
 	const char *enable_method;
+#ifdef CONFIG_DEEP_SLEEP
+	ulong len;
+#endif
 
 	off = fdt_node_offset_by_prop_value(blob, -1, "device_type", "cpu", 4);
 	while (off != -FDT_ERR_NOTFOUND) {
@@ -77,6 +83,25 @@ void ft_fixup_cpu(void *blob, u64 memory_limit)
 				"device_type", "cpu", 4);
 	}
 
+#ifdef CONFIG_DEEP_SLEEP
+	/*
+	 * reserve the memory space for warm reset.
+	 * This space will be re-used next time when boot from deep sleep.
+	 * The space includes bd_t, gd_t, stack and uboot image.
+	 * Reserve 1K for stack.
+	 */
+	len = sizeof(bd_t) + sizeof(gd_t) + (1 << 10);
+	/* round up to 4K */
+	len = (len + (4096 - 1)) & ~(4096 - 1);
+
+	off = fdt_add_mem_rsv(blob, gd->relocaddr - len,
+			len + ((ulong)&__bss_end - gd->relocaddr));
+	if (off < 0)
+		printf("Failed to reserve memory for warm reset: %s\n",
+			fdt_strerror(off));
+
+#endif
+
 	/* Reserve the boot page so OSes dont use it */
 	if ((u64)bootpg < memory_limit) {
 		off = fdt_add_mem_rsv(blob, bootpg, (u64)4096);
@@ -100,6 +125,7 @@ void ft_fixup_cpu(void *blob, u64 memory_limit)
 	}
 #endif
 
+#ifndef CONFIG_DEEP_SLEEP
 	/* Reserve spin table page */
 	if (spin_tbl_addr < memory_limit) {
 		off = fdt_add_mem_rsv(blob,
@@ -108,6 +134,7 @@ void ft_fixup_cpu(void *blob, u64 memory_limit)
 			printf("Failed to reserve memory for spin table: %s\n",
 				fdt_strerror(off));
 	}
+#endif
 }
 #endif
 
diff --git a/arch/powerpc/cpu/mpc85xx/liodn.c b/arch/powerpc/cpu/mpc85xx/liodn.c
index 19e130e..898685b 100644
--- a/arch/powerpc/cpu/mpc85xx/liodn.c
+++ b/arch/powerpc/cpu/mpc85xx/liodn.c
@@ -14,6 +14,10 @@
 #include <asm/fsl_portals.h>
 #include <asm/fsl_liodn.h>
 
+#ifdef CONFIG_DEEP_SLEEP
+DECLARE_GLOBAL_DATA_PTR;
+#endif
+
 int get_dpaa_liodn(enum fsl_dpaa_dev dpaa_dev, u32 *liodns, int liodn_offset)
 {
 	liodns[0] = liodn_bases[dpaa_dev].id[0] + liodn_offset;
@@ -180,14 +184,22 @@ void set_liodns(void)
 
 	/* setup FMAN block(s) liodn bases & offsets if we have one */
 #ifdef CONFIG_SYS_DPAA_FMAN
-	set_liodn(fman1_liodn_tbl, fman1_liodn_tbl_sz);
-	setup_fman_liodn_base(FSL_HW_PORTAL_FMAN1, fman1_liodn_tbl,
-				fman1_liodn_tbl_sz);
+#ifdef CONFIG_DEEP_SLEEP
+	if ((gd->flags & GD_FLG_WARM_BOOT) == 0) {
+		set_liodn(fman1_liodn_tbl, fman1_liodn_tbl_sz);
+		setup_fman_liodn_base(FSL_HW_PORTAL_FMAN1, fman1_liodn_tbl,
+					fman1_liodn_tbl_sz);
+	}
+#endif
 
 #if (CONFIG_SYS_NUM_FMAN == 2)
-	set_liodn(fman2_liodn_tbl, fman2_liodn_tbl_sz);
-	setup_fman_liodn_base(FSL_HW_PORTAL_FMAN2, fman2_liodn_tbl,
-				fman2_liodn_tbl_sz);
+#ifdef CONFIG_DEEP_SLEEP
+	if ((gd->flags & GD_FLG_WARM_BOOT) == 0) {
+		set_liodn(fman2_liodn_tbl, fman2_liodn_tbl_sz);
+		setup_fman_liodn_base(FSL_HW_PORTAL_FMAN2, fman2_liodn_tbl,
+					fman2_liodn_tbl_sz);
+	}
+#endif
 #endif
 #endif
 	/* setup PME liodn base */
diff --git a/arch/powerpc/cpu/mpc85xx/start.S b/arch/powerpc/cpu/mpc85xx/start.S
index db84d10..4e66edc 100644
--- a/arch/powerpc/cpu/mpc85xx/start.S
+++ b/arch/powerpc/cpu/mpc85xx/start.S
@@ -1671,6 +1671,14 @@ relocate_code:
 	 * Now relocate code
 	 */
 
+#ifdef CONFIG_DEEP_SLEEP
+	/* don't copy uboot again in warm reset case */
+	mr r0, r3
+	bl check_warmboot
+	cmpwi r3, 0
+	bne 7f
+	mr r3, r0
+#endif
 	cmplw	cr1,r3,r4
 	addi	r0,r5,3
 	srwi.	r0,r0,2
diff --git a/arch/powerpc/include/asm/global_data.h b/arch/powerpc/include/asm/global_data.h
index 8e59e8b..1ab05ff 100644
--- a/arch/powerpc/include/asm/global_data.h
+++ b/arch/powerpc/include/asm/global_data.h
@@ -117,6 +117,7 @@ struct arch_global_data {
 #include <asm-generic/global_data.h>
 
 #if 1
+#define GD_FLG_WARM_BOOT       0x10000
 #define DECLARE_GLOBAL_DATA_PTR     register volatile gd_t *gd asm ("r2")
 #else /* We could use plain global data, but the resulting code is bigger */
 #define XTRN_DECLARE_GLOBAL_DATA_PTR	extern
diff --git a/arch/powerpc/lib/board.c b/arch/powerpc/lib/board.c
index 34bbfca..7383e32 100644
--- a/arch/powerpc/lib/board.c
+++ b/arch/powerpc/lib/board.c
@@ -350,6 +350,13 @@ unsigned long logbuffer_base(void)
 }
 #endif
 
+#ifdef CONFIG_DEEP_SLEEP
+int check_warmboot(void)
+{
+	return !!(gd->flags & GD_FLG_WARM_BOOT);
+}
+#endif
+
 void board_init_f(ulong bootflag)
 {
 	bd_t *bd;
@@ -475,12 +482,18 @@ void board_init_f(ulong bootflag)
 
 	debug("Reserving %ldk for U-Boot at: %08lx\n", len >> 10, addr);
 
+#ifdef CONFIG_DEEP_SLEEP
 	/*
 	 * reserve memory for malloc() arena
+	 * don't reserve it in warm reset case
 	 */
-	addr_sp = addr - TOTAL_MALLOC_LEN;
-	debug("Reserving %dk for malloc() at: %08lx\n",
-	      TOTAL_MALLOC_LEN >> 10, addr_sp);
+	if ((gd->flags & GD_FLG_WARM_BOOT) == 0) {
+		addr_sp = addr - TOTAL_MALLOC_LEN;
+		debug("Reserving %dk for malloc() at: %08lx\n",
+			TOTAL_MALLOC_LEN >> 10, addr_sp);
+	} else
+		addr_sp = addr;
+#endif
 
 	/*
 	 * (permanently) allocate a Board Info struct
@@ -591,6 +604,14 @@ void board_init_r(gd_t *id, ulong dest_addr)
 {
 	bd_t *bd;
 	ulong malloc_start;
+#ifdef CONFIG_DEEP_SLEEP
+	u32 start;
+	int i;
+	struct ccsr_scfg *scfg = (void *)CONFIG_SYS_MPC85xx_SCFG;
+	u64 *src, *dst;
+	typedef void (*func_t)(void);
+	func_t kernel_resume;
+#endif
 
 #ifndef CONFIG_SYS_NO_FLASH
 	ulong flash_size;
@@ -601,6 +622,21 @@ void board_init_r(gd_t *id, ulong dest_addr)
 
 	gd->flags |= GD_FLG_RELOC;	/* tell others: relocation done */
 
+#ifdef CONFIG_DEEP_SLEEP
+	/*
+	 * restore 128-byte memory space which corrupted
+	 * by DDRC training.
+	 */
+	if (gd->flags & GD_FLG_WARM_BOOT) {
+		src = (u64 *)in_be32(&scfg->sparecr[2]);
+		dst = (u64 *)(0);
+		for (i = 0; i < 128/sizeof(u64); i++) {
+			*dst = *src;
+			dst++;
+			src++;
+		}
+	}
+#endif
 	/* The Malloc area is immediately below the monitor copy in DRAM */
 	malloc_start = dest_addr - TOTAL_MALLOC_LEN;
 
@@ -639,7 +675,10 @@ void board_init_r(gd_t *id, ulong dest_addr)
 	/*
 	 * Setup trap handlers
 	 */
-	trap_init(dest_addr);
+#ifdef CONFIG_DEEP_SLEEP
+	if ((gd->flags & GD_FLG_WARM_BOOT) == 0)
+		trap_init(dest_addr);
+#endif
 
 #ifdef CONFIG_ADDR_MAP
 	init_addr_map();
@@ -686,7 +725,10 @@ void board_init_r(gd_t *id, ulong dest_addr)
 
 	asm("sync ; isync");
 
-	mem_malloc_init(malloc_start, TOTAL_MALLOC_LEN);
+#ifdef CONFIG_DEEP_SLEEP
+	if ((gd->flags & GD_FLG_WARM_BOOT) == 0)
+		mem_malloc_init(malloc_start, TOTAL_MALLOC_LEN);
+#endif
 
 #if !defined(CONFIG_SYS_NO_FLASH)
 	puts("Flash: ");
@@ -743,6 +785,15 @@ void board_init_r(gd_t *id, ulong dest_addr)
 	/* initialize higher level parts of CPU like time base and timers */
 	cpu_init_r();
 
+#ifdef CONFIG_DEEP_SLEEP
+	/* Jump to kernel in warm reset case */
+	if (gd->flags & GD_FLG_WARM_BOOT) {
+		start = in_be32(&scfg->sparecr[1]);
+		kernel_resume = (func_t)start;
+		kernel_resume();
+	}
+#endif
+
 	WATCHDOG_RESET();
 
 #ifdef CONFIG_SPI
diff --git a/drivers/ddr/fsl/mpc85xx_ddr_gen3.c b/drivers/ddr/fsl/mpc85xx_ddr_gen3.c
index c805086..3d4f11a 100644
--- a/drivers/ddr/fsl/mpc85xx_ddr_gen3.c
+++ b/drivers/ddr/fsl/mpc85xx_ddr_gen3.c
@@ -15,6 +15,9 @@
 #error Invalid setting for CONFIG_CHIP_SELECTS_PER_CTRL
 #endif
 
+#ifdef CONFIG_DEEP_SLEEP
+DECLARE_GLOBAL_DATA_PTR;
+#endif
 
 /*
  * regs has the to-be-set values for DDR controller registers
@@ -119,7 +122,13 @@ void fsl_ddr_set_memctl_regs(const fsl_ddr_cfg_regs_t *regs,
 	out_be32(&ddr->timing_cfg_0, regs->timing_cfg_0);
 	out_be32(&ddr->timing_cfg_1, regs->timing_cfg_1);
 	out_be32(&ddr->timing_cfg_2, regs->timing_cfg_2);
-	out_be32(&ddr->sdram_cfg_2, regs->ddr_sdram_cfg_2);
+#ifdef CONFIG_DEEP_SLEEP
+	if (gd->flags & GD_FLG_WARM_BOOT)
+		out_be32(&ddr->sdram_cfg_2,
+			regs->ddr_sdram_cfg_2 & ~SDRAM_CFG2_D_INIT);
+	else
+#endif
+		out_be32(&ddr->sdram_cfg_2, regs->ddr_sdram_cfg_2);
 	out_be32(&ddr->sdram_mode, regs->ddr_sdram_mode);
 	out_be32(&ddr->sdram_mode_2, regs->ddr_sdram_mode_2);
 	out_be32(&ddr->sdram_mode_3, regs->ddr_sdram_mode_3);
@@ -132,8 +141,16 @@ void fsl_ddr_set_memctl_regs(const fsl_ddr_cfg_regs_t *regs,
 	out_be32(&ddr->sdram_interval, regs->ddr_sdram_interval);
 	out_be32(&ddr->sdram_data_init, regs->ddr_data_init);
 	out_be32(&ddr->sdram_clk_cntl, regs->ddr_sdram_clk_cntl);
-	out_be32(&ddr->init_addr, regs->ddr_init_addr);
-	out_be32(&ddr->init_ext_addr, regs->ddr_init_ext_addr);
+#ifdef CONFIG_DEEP_SLEEP
+	if (gd->flags & GD_FLG_WARM_BOOT) {
+		out_be32(&ddr->init_addr, 0);
+		out_be32(&ddr->init_ext_addr, (1 << 31));
+	} else
+#endif
+	{
+		out_be32(&ddr->init_addr, regs->ddr_init_addr);
+		out_be32(&ddr->init_ext_addr, regs->ddr_init_ext_addr);
+	}
 
 	out_be32(&ddr->timing_cfg_4, regs->timing_cfg_4);
 	out_be32(&ddr->timing_cfg_5, regs->timing_cfg_5);
@@ -374,8 +391,22 @@ step2:
 	udelay(500);
 	asm volatile("sync;isync");
 
+#ifdef CONFIG_DEEP_SLEEP
+	if (gd->flags & GD_FLG_WARM_BOOT) {
+		/* enter self-refresh */
+		setbits_be32(&ddr->sdram_cfg_2, (1 << 31));
+		/* do board specific memory setup */
+		board_mem_sleep_setup();
+	}
+#endif
+
 	/* Let the controller go */
-	temp_sdram_cfg = in_be32(&ddr->sdram_cfg) & ~SDRAM_CFG_BI;
+#ifdef CONFIG_DEEP_SLEEP
+	if (gd->flags & GD_FLG_WARM_BOOT)
+		temp_sdram_cfg = (in_be32(&ddr->sdram_cfg) | SDRAM_CFG_BI);
+	else
+#endif
+		temp_sdram_cfg = (in_be32(&ddr->sdram_cfg) & ~SDRAM_CFG_BI);
 	out_be32(&ddr->sdram_cfg, temp_sdram_cfg | SDRAM_CFG_MEM_EN);
 	asm volatile("sync;isync");
 
@@ -526,4 +557,9 @@ step2:
 		clrbits_be32(&ddr->sdram_cfg, 0x2);
 	}
 #endif /* CONFIG_SYS_FSL_ERRATUM_DDR111_DDR134 */
+#ifdef CONFIG_DEEP_SLEEP
+	if (gd->flags & GD_FLG_WARM_BOOT)
+		/* exit self-refresh */
+		clrbits_be32(&ddr->sdram_cfg_2, (1 << 31));
+#endif
 }
diff --git a/include/fsl_ddr_sdram.h b/include/fsl_ddr_sdram.h
index 16cccc7..d8ab779 100644
--- a/include/fsl_ddr_sdram.h
+++ b/include/fsl_ddr_sdram.h
@@ -356,6 +356,12 @@ static int __board_need_mem_reset(void)
 int board_need_mem_reset(void)
 	__attribute__((weak, alias("__board_need_mem_reset")));
 
+static void __board_mem_sleep_setup(void)
+{
+}
+void board_mem_sleep_setup(void)
+	__attribute__((weak, alias("__board_mem_sleep_setup")));
+
 /*
  * The 85xx boards have a common prototype for fixed_sdram so put the
  * declaration here.
-- 
1.8.0

[U-Boot] [U-Boot, 2/3] mpc85xx: Add deep sleep framework support

[Scott Wood]

On Sun, Jan 26, 2014 at 02:00:40PM +0800, tang yuantian wrote:
> From: Tang Yuantian <yuantian.tang@freescale.com>
> 
> When system wakes up from warm reset, control is passed to the
> primary core that starts executing uboot. After re-initialized some
> IP blocks, like DDRC, kernel will take responsibility to continue
> to restore environment it leaves before.
> 
> Signed-off-by: Tang Yuantian <Yuantian.Tang@freescale.com>

Is this for some specific mpc85xx chip (e.g. T1040)?  I'm pretty sure
this isn't necessary for deep sleep on mpc8536, for example.

> ---
> arch/powerpc/cpu/mpc85xx/cpu_init.c    |  7 ++++
>  arch/powerpc/cpu/mpc85xx/fdt.c         | 27 +++++++++++++++
>  arch/powerpc/cpu/mpc85xx/liodn.c       | 24 +++++++++----
>  arch/powerpc/cpu/mpc85xx/start.S       |  8 +++++
>  arch/powerpc/include/asm/global_data.h |  1 +
>  arch/powerpc/lib/board.c               | 61 +++++++++++++++++++++++++++++++---
>  drivers/ddr/fsl/mpc85xx_ddr_gen3.c     | 44 +++++++++++++++++++++---
>  include/fsl_ddr_sdram.h                |  6 ++++
>  8 files changed, 163 insertions(+), 15 deletions(-)
> 
> diff --git a/arch/powerpc/cpu/mpc85xx/cpu_init.c b/arch/powerpc/cpu/mpc85xx/cpu_init.c
> index b31efb7..376c659 100644
> --- a/arch/powerpc/cpu/mpc85xx/cpu_init.c
> +++ b/arch/powerpc/cpu/mpc85xx/cpu_init.c
> @@ -231,6 +231,7 @@ void cpu_init_f (void)
>  #ifdef CONFIG_MPC8548
>  	ccsr_local_ecm_t *ecm = (void *)(CONFIG_SYS_MPC85xx_ECM_ADDR);
>  	uint svr = get_svr();
> +	gd = (gd_t *)(CONFIG_SYS_INIT_RAM_ADDR + CONFIG_SYS_GBL_DATA_OFFSET);
>  
>  	/*
>  	 * CPU2 errata workaround: A core hang possible while executing
> @@ -282,6 +283,12 @@ void cpu_init_f (void)
>  	in_be32(&gur->dcsrcr);
>  #endif
>  
> +#ifdef CONFIG_DEEP_SLEEP

CONFIG symbols need to be documented in README...

> +	/* disable the console if boot from warm reset */
> +	if (in_be32(&gur->scrtsr[0]) & (1 << 3))
> +		gd->flags |=
> +			GD_FLG_SILENT | GD_FLG_WARM_BOOT | GD_FLG_DISABLE_CONSOLE;
> +#endif

...and it should probably be a more specific CONFIG_SYS symbol that
indicates the presence of this guts bit.

> diff --git a/arch/powerpc/cpu/mpc85xx/fdt.c b/arch/powerpc/cpu/mpc85xx/fdt.c
> index 33bc900..b3b9250 100644
> --- a/arch/powerpc/cpu/mpc85xx/fdt.c
> +++ b/arch/powerpc/cpu/mpc85xx/fdt.c
> @@ -24,6 +24,9 @@ DECLARE_GLOBAL_DATA_PTR;
>  extern void ft_qe_setup(void *blob);
>  extern void ft_fixup_num_cores(void *blob);
>  extern void ft_srio_setup(void *blob);
> +#ifdef CONFIG_DEEP_SLEEP
> +extern ulong __bss_end;
> +#endif

Don't ifdef declarations.

>  #ifdef CONFIG_MP
>  #include "mp.h"
> @@ -35,6 +38,9 @@ void ft_fixup_cpu(void *blob, u64 memory_limit)
>  	u32 bootpg = determine_mp_bootpg(NULL);
>  	u32 id = get_my_id();
>  	const char *enable_method;
> +#ifdef CONFIG_DEEP_SLEEP
> +	ulong len;
> +#endif
>  
>  	off = fdt_node_offset_by_prop_value(blob, -1, "device_type", "cpu", 4);
>  	while (off != -FDT_ERR_NOTFOUND) {
> @@ -77,6 +83,25 @@ void ft_fixup_cpu(void *blob, u64 memory_limit)
>  				"device_type", "cpu", 4);
>  	}
>  
> +#ifdef CONFIG_DEEP_SLEEP
> +	/*
> +	 * reserve the memory space for warm reset.
> +	 * This space will be re-used next time when boot from deep sleep.
> +	 * The space includes bd_t, gd_t, stack and uboot image.
> +	 * Reserve 1K for stack.
> +	 */
> +	len = sizeof(bd_t) + sizeof(gd_t) + (1 << 10);
> +	/* round up to 4K */
> +	len = (len + (4096 - 1)) & ~(4096 - 1);
> +
> +	off = fdt_add_mem_rsv(blob, gd->relocaddr - len,
> +			len + ((ulong)&__bss_end - gd->relocaddr));
> +	if (off < 0)
> +		printf("Failed to reserve memory for warm reset: %s\n",
> +			fdt_strerror(off));
> +
> +#endif

Why do you need to reserve memory for this?  We didn't need to for deep
sleep on MPC8313ERDB, which also goes through U-Boot on wake.  On
MPC8313ERDB we transfer control to the kernel before relocating, so
U-Boot never touches DDR.  bd_t, gd_t, and the stack should be in locked
L1 cache, and the u-boot image should be in flash (or locked CPC if this
is not a NOR flash boot).

>  	/* Reserve the boot page so OSes dont use it */
>  	if ((u64)bootpg < memory_limit) {
>  		off = fdt_add_mem_rsv(blob, bootpg, (u64)4096);
> @@ -100,6 +125,7 @@ void ft_fixup_cpu(void *blob, u64 memory_limit)
>  	}
>  #endif
>  
> +#ifndef CONFIG_DEEP_SLEEP
>  	/* Reserve spin table page */
>  	if (spin_tbl_addr < memory_limit) {
>  		off = fdt_add_mem_rsv(blob,
> @@ -108,6 +134,7 @@ void ft_fixup_cpu(void *blob, u64 memory_limit)
>  			printf("Failed to reserve memory for spin table: %s\n",
>  				fdt_strerror(off));
>  	}
> +#endif

Explain.

> diff --git a/arch/powerpc/cpu/mpc85xx/liodn.c b/arch/powerpc/cpu/mpc85xx/liodn.c
> index 19e130e..898685b 100644
> --- a/arch/powerpc/cpu/mpc85xx/liodn.c
> +++ b/arch/powerpc/cpu/mpc85xx/liodn.c
> @@ -14,6 +14,10 @@
>  #include <asm/fsl_portals.h>
>  #include <asm/fsl_liodn.h>
>  
> +#ifdef CONFIG_DEEP_SLEEP
> +DECLARE_GLOBAL_DATA_PTR;
> +#endif

Don't ifdef declarations.

>  int get_dpaa_liodn(enum fsl_dpaa_dev dpaa_dev, u32 *liodns, int liodn_offset)
>  {
>  	liodns[0] = liodn_bases[dpaa_dev].id[0] + liodn_offset;
> @@ -180,14 +184,22 @@ void set_liodns(void)
>  
>  	/* setup FMAN block(s) liodn bases & offsets if we have one */
>  #ifdef CONFIG_SYS_DPAA_FMAN
> -	set_liodn(fman1_liodn_tbl, fman1_liodn_tbl_sz);
> -	setup_fman_liodn_base(FSL_HW_PORTAL_FMAN1, fman1_liodn_tbl,
> -				fman1_liodn_tbl_sz);
> +#ifdef CONFIG_DEEP_SLEEP
> +	if ((gd->flags & GD_FLG_WARM_BOOT) == 0) {
> +		set_liodn(fman1_liodn_tbl, fman1_liodn_tbl_sz);
> +		setup_fman_liodn_base(FSL_HW_PORTAL_FMAN1, fman1_liodn_tbl,
> +					fman1_liodn_tbl_sz);
> +	}
> +#endif
>  
>  #if (CONFIG_SYS_NUM_FMAN == 2)
> -	set_liodn(fman2_liodn_tbl, fman2_liodn_tbl_sz);
> -	setup_fman_liodn_base(FSL_HW_PORTAL_FMAN2, fman2_liodn_tbl,
> -				fman2_liodn_tbl_sz);
> +#ifdef CONFIG_DEEP_SLEEP
> +	if ((gd->flags & GD_FLG_WARM_BOOT) == 0) {
> +		set_liodn(fman2_liodn_tbl, fman2_liodn_tbl_sz);
> +		setup_fman_liodn_base(FSL_HW_PORTAL_FMAN2, fman2_liodn_tbl,
> +					fman2_liodn_tbl_sz);
> +	}
> +#endif
>  #endif
>  #endif

Aren't you breaking non-CONFIG_DEEP_SLEEP here?

Why would you be getting this far into the boot in the deep sleep wake
case?

>  	/* setup PME liodn base */
> diff --git a/arch/powerpc/cpu/mpc85xx/start.S b/arch/powerpc/cpu/mpc85xx/start.S
> index db84d10..4e66edc 100644
> --- a/arch/powerpc/cpu/mpc85xx/start.S
> +++ b/arch/powerpc/cpu/mpc85xx/start.S
> @@ -1671,6 +1671,14 @@ relocate_code:
>  	 * Now relocate code
>  	 */
>  
> +#ifdef CONFIG_DEEP_SLEEP
> +	/* don't copy uboot again in warm reset case */
> +	mr r0, r3
> +	bl check_warmboot
> +	cmpwi r3, 0
> +	bne 7f
> +	mr r3, r0
> +#endif
>  	cmplw	cr1,r3,r4
>  	addi	r0,r5,3
>  	srwi.	r0,r0,2

The surrounding asm code puts a tab after the mnemonic.  Why don't you?

Again, you shouldn't get this far in the deep sleep wake case.

> diff --git a/arch/powerpc/include/asm/global_data.h b/arch/powerpc/include/asm/global_data.h
> index 8e59e8b..1ab05ff 100644
> --- a/arch/powerpc/include/asm/global_data.h
> +++ b/arch/powerpc/include/asm/global_data.h
> @@ -117,6 +117,7 @@ struct arch_global_data {
>  #include <asm-generic/global_data.h>
>  
>  #if 1
> +#define GD_FLG_WARM_BOOT       0x10000

Why is this not with the rest of the GD_FLGs, not numerically contiguous,
and not commented?  What specifically does "warm boot" mean?  I think a
lot of people would expect it to mean something that looks like a reset
at the software level, while possibly not involving a real hardware reset
-- coming out of deep sleep is the opposite of that.

>  #define DECLARE_GLOBAL_DATA_PTR     register volatile gd_t *gd asm ("r2")
>  #else /* We could use plain global data, but the resulting code is bigger */
>  #define XTRN_DECLARE_GLOBAL_DATA_PTR	extern
> diff --git a/arch/powerpc/lib/board.c b/arch/powerpc/lib/board.c
> index 34bbfca..7383e32 100644
> --- a/arch/powerpc/lib/board.c
> +++ b/arch/powerpc/lib/board.c
> @@ -350,6 +350,13 @@ unsigned long logbuffer_base(void)
>  }
>  #endif
>  
> +#ifdef CONFIG_DEEP_SLEEP
> +int check_warmboot(void)
> +{
> +	return !!(gd->flags & GD_FLG_WARM_BOOT);
> +}
> +#endif

Why?

>  void board_init_f(ulong bootflag)
>  {
>  	bd_t *bd;
> @@ -475,12 +482,18 @@ void board_init_f(ulong bootflag)
>  
>  	debug("Reserving %ldk for U-Boot at: %08lx\n", len >> 10, addr);
>  
> +#ifdef CONFIG_DEEP_SLEEP
>  	/*
>  	 * reserve memory for malloc() arena
> +	 * don't reserve it in warm reset case
>  	 */
> -	addr_sp = addr - TOTAL_MALLOC_LEN;
> -	debug("Reserving %dk for malloc() at: %08lx\n",
> -	      TOTAL_MALLOC_LEN >> 10, addr_sp);
> +	if ((gd->flags & GD_FLG_WARM_BOOT) == 0) {
> +		addr_sp = addr - TOTAL_MALLOC_LEN;
> +		debug("Reserving %dk for malloc() at: %08lx\n",
> +			TOTAL_MALLOC_LEN >> 10, addr_sp);
> +	} else
> +		addr_sp = addr;
> +#endif

If one side of an if/else has braces, U-Boot style says to use braces on
both.

Again, you seem to be breaking the non-CONFIG_DEEP_SLEEP case.

> @@ -591,6 +604,14 @@ void board_init_r(gd_t *id, ulong dest_addr)
>  {
>  	bd_t *bd;
>  	ulong malloc_start;
> +#ifdef CONFIG_DEEP_SLEEP
> +	u32 start;
> +	int i;
> +	struct ccsr_scfg *scfg = (void *)CONFIG_SYS_MPC85xx_SCFG;
> +	u64 *src, *dst;
> +	typedef void (*func_t)(void);
> +	func_t kernel_resume;
> +#endif
>  
>  #ifndef CONFIG_SYS_NO_FLASH
>  	ulong flash_size;
> @@ -601,6 +622,21 @@ void board_init_r(gd_t *id, ulong dest_addr)
>  
>  	gd->flags |= GD_FLG_RELOC;	/* tell others: relocation done */
>  
> +#ifdef CONFIG_DEEP_SLEEP
> +	/*
> +	 * restore 128-byte memory space which corrupted
> +	 * by DDRC training.
> +	 */
> +	if (gd->flags & GD_FLG_WARM_BOOT) {
> +		src = (u64 *)in_be32(&scfg->sparecr[2]);
> +		dst = (u64 *)(0);
> +		for (i = 0; i < 128/sizeof(u64); i++) {
> +			*dst = *src;
> +			dst++;
> +			src++;
> +		}
> +	}

(u64 *)(0) is a NULL pointer.  Dereferencing NULL pointers is undefined
and GCC has been getting increasingly free with making surprising
optimizations based on that (such as assuming any code path that it knows
can reach a NULL dereference is dead code and can be removed).

Use an I/O accessor.  And yes, the mpc8313erdb code should be fixed as
well. :-)

> @@ -639,7 +675,10 @@ void board_init_r(gd_t *id, ulong dest_addr)
>  	/*
>  	 * Setup trap handlers
>  	 */
> -	trap_init(dest_addr);
> +#ifdef CONFIG_DEEP_SLEEP
> +	if ((gd->flags & GD_FLG_WARM_BOOT) == 0)
> +		trap_init(dest_addr);
> +#endif

More breakage of non-CONFIG_DEEP_SLEEP.  I'll stop here.

-Scott

[U-Boot] [U-Boot, 2/3] mpc85xx: Add deep sleep framework support

[Scott Wood]

On Thu, 2014-02-13 at 01:05 -0600, Tang Yuantian-B29983 wrote:
> Thanks for your review. Please see the reply inline.
> 
> Thanks,
> Yuantian
> 
> > -----Original Message-----
> > From: Wood Scott-B07421
> > Sent: 2014?2?13? ??? 8:44
> > To: Tang Yuantian-B29983
> > Cc: Sun York-R58495; Wood Scott-B07421; Li Yang-Leo-R58472;
> > Tang at theia.denx.de; u-boot at lists.denx.de; Kushwaha Prabhakar-B32579; Jin
> > Zhengxiong-R64188
> > Subject: Re: [U-Boot,2/3] mpc85xx: Add deep sleep framework support
> > 
> > On Sun, Jan 26, 2014 at 02:00:40PM +0800, tang yuantian wrote:
> > > From: Tang Yuantian <yuantian.tang@freescale.com>
> > >
> > > When system wakes up from warm reset, control is passed to the primary
> > > core that starts executing uboot. After re-initialized some IP blocks,
> > > like DDRC, kernel will take responsibility to continue to restore
> > > environment it leaves before.
> > >
> > > Signed-off-by: Tang Yuantian <Yuantian.Tang@freescale.com>
> > 
> > Is this for some specific mpc85xx chip (e.g. T1040)?  I'm pretty sure
> > this isn't necessary for deep sleep on mpc8536, for example.
> > 
> Currently, it is used by t1040. But we want it to be more general so that
> It can be used by later new chips.

But the mechanism is not the same for all mpc85xx derivatives.  You'll
need a more specific name.

> > > +#ifdef CONFIG_DEEP_SLEEP
> > 
> > CONFIG symbols need to be documented in README...
> > 
> Where should I add this README?

Under "85xx CPU Options" in the top-level README.

> > > +	/* disable the console if boot from warm reset */
> > > +	if (in_be32(&gur->scrtsr[0]) & (1 << 3))
> > > +		gd->flags |=
> > > +			GD_FLG_SILENT | GD_FLG_WARM_BOOT |
> > GD_FLG_DISABLE_CONSOLE; #endif
> > 
> > ...and it should probably be a more specific CONFIG_SYS symbol that
> > indicates the presence of this guts bit.
> > 
> > > diff --git a/arch/powerpc/cpu/mpc85xx/fdt.c
> > > b/arch/powerpc/cpu/mpc85xx/fdt.c index 33bc900..b3b9250 100644
> > > --- a/arch/powerpc/cpu/mpc85xx/fdt.c
> > > +++ b/arch/powerpc/cpu/mpc85xx/fdt.c
> > > @@ -24,6 +24,9 @@ DECLARE_GLOBAL_DATA_PTR;  extern void
> > > ft_qe_setup(void *blob);  extern void ft_fixup_num_cores(void *blob);
> > > extern void ft_srio_setup(void *blob);
> > > +#ifdef CONFIG_DEEP_SLEEP
> > > +extern ulong __bss_end;
> > > +#endif
> > 
> > Don't ifdef declarations.
> > 
> > >  #ifdef CONFIG_MP
> > >  #include "mp.h"
> > > @@ -35,6 +38,9 @@ void ft_fixup_cpu(void *blob, u64 memory_limit)
> > >  	u32 bootpg = determine_mp_bootpg(NULL);
> > >  	u32 id = get_my_id();
> > >  	const char *enable_method;
> > > +#ifdef CONFIG_DEEP_SLEEP
> > > +	ulong len;
> > > +#endif
> > >
> > >  	off = fdt_node_offset_by_prop_value(blob, -1, "device_type", "cpu",
> > 4);
> > >  	while (off != -FDT_ERR_NOTFOUND) {
> > > @@ -77,6 +83,25 @@ void ft_fixup_cpu(void *blob, u64 memory_limit)
> > >  				"device_type", "cpu", 4);
> > >  	}
> > >
> > > +#ifdef CONFIG_DEEP_SLEEP
> > > +	/*
> > > +	 * reserve the memory space for warm reset.
> > > +	 * This space will be re-used next time when boot from deep sleep.
> > > +	 * The space includes bd_t, gd_t, stack and uboot image.
> > > +	 * Reserve 1K for stack.
> > > +	 */
> > > +	len = sizeof(bd_t) + sizeof(gd_t) + (1 << 10);
> > > +	/* round up to 4K */
> > > +	len = (len + (4096 - 1)) & ~(4096 - 1);
> > > +
> > > +	off = fdt_add_mem_rsv(blob, gd->relocaddr - len,
> > > +			len + ((ulong)&__bss_end - gd->relocaddr));
> > > +	if (off < 0)
> > > +		printf("Failed to reserve memory for warm reset: %s\n",
> > > +			fdt_strerror(off));
> > > +
> > > +#endif
> > 
> > Why do you need to reserve memory for this?  We didn't need to for deep
> > sleep on MPC8313ERDB, which also goes through U-Boot on wake.  On
> > MPC8313ERDB we transfer control to the kernel before relocating, so U-
> > Boot never touches DDR.  bd_t, gd_t, and the stack should be in locked
> > L1 cache, and the u-boot image should be in flash (or locked CPC if this
> > is not a NOR flash boot).
> > 
> In deep sleep many IP blocks are powered off like DDRC, LIODN, cpu. These IPs
> are re-initialized after relocating.
> So, we must jump to kernel after relocating.

The DDR controller is initialized before relocating -- and of course the
CPU is powered off on MPC8313ERDB deep sleep as well.

LIODNs are a new concern for deep sleep, but that doesn't mean we should
go through a bunch of unrelated code to get there.  Refactor the LIODN
code to be usable before relocation, and not be tied to fdt fixups.

> > > +#ifndef CONFIG_DEEP_SLEEP
> > >  	/* Reserve spin table page */
> > >  	if (spin_tbl_addr < memory_limit) {
> > >  		off = fdt_add_mem_rsv(blob,
> > > @@ -108,6 +134,7 @@ void ft_fixup_cpu(void *blob, u64 memory_limit)
> > >  			printf("Failed to reserve memory for spin table: %s\n",
> > >  				fdt_strerror(off));
> > >  	}
> > > +#endif
> > 
> > Explain.
> > 
> Spin_tbl_addr has been reserved already.

Where, and why is it different for non-CONFIG_DEEP_SLEEP?

> > >  #if (CONFIG_SYS_NUM_FMAN == 2)
> > > -	set_liodn(fman2_liodn_tbl, fman2_liodn_tbl_sz);
> > > -	setup_fman_liodn_base(FSL_HW_PORTAL_FMAN2, fman2_liodn_tbl,
> > > -				fman2_liodn_tbl_sz);
> > > +#ifdef CONFIG_DEEP_SLEEP
> > > +	if ((gd->flags & GD_FLG_WARM_BOOT) == 0) {
> > > +		set_liodn(fman2_liodn_tbl, fman2_liodn_tbl_sz);
> > > +		setup_fman_liodn_base(FSL_HW_PORTAL_FMAN2, fman2_liodn_tbl,
> > > +					fman2_liodn_tbl_sz);
> > > +	}
> > > +#endif
> > >  #endif
> > >  #endif
> > 
> > Aren't you breaking non-CONFIG_DEEP_SLEEP here?
> > 
> No, if deep sleep feature is enabled, we need to further judge whether this boot is
> Coming from deep sleep by GD_FLG_WARM_BOOT flag.

My point is that if CONFIG_DEEP_SLEEP is not set, you will be skipping
those calls to set_liodn() and setup_fman_liodn_base().

#ifdef foo
	if (!bar)
		stuff();
#endif

is not equivalent to:

#ifdef foo
	if (!bar)
#endif
		stuff();

Though the latter is better written as something like:

	bool do_stuff = true;

#ifdef foo
	if (bar)
		do_stuff = false;
#endif

	if (do_stuff)
		stuff();

or

static inline bool is_bar(void)
{
#ifdef foo
	return bar;
#else
	return true;
#endif
}

...

	if (!is_bar())
		stuff();

> > > diff --git a/arch/powerpc/include/asm/global_data.h
> > > b/arch/powerpc/include/asm/global_data.h
> > > index 8e59e8b..1ab05ff 100644
> > > --- a/arch/powerpc/include/asm/global_data.h
> > > +++ b/arch/powerpc/include/asm/global_data.h
> > > @@ -117,6 +117,7 @@ struct arch_global_data {  #include
> > > <asm-generic/global_data.h>
> > >
> > >  #if 1
> > > +#define GD_FLG_WARM_BOOT       0x10000
> > 
> > Why is this not with the rest of the GD_FLGs, not numerically contiguous,
> > and not commented?  What specifically does "warm boot" mean?  I think a
> > lot of people would expect it to mean something that looks like a reset
> > at the software level, while possibly not involving a real hardware reset
> > -- coming out of deep sleep is the opposite of that.
> > 
> Archdef document says it is a warm reset boot. So I name it this way.

Hardware people sometimes use terminology differently than software
people.

> Other platforms use the same flag, like x86, although I am not sure 
> Whether it is for deep sleep.
> If you have better name, I love to use it.

It's not yet clear to me that a GD flag is needed for this.

> > >  #define DECLARE_GLOBAL_DATA_PTR     register volatile gd_t *gd asm
> > ("r2")
> > >  #else /* We could use plain global data, but the resulting code is
> > bigger */
> > >  #define XTRN_DECLARE_GLOBAL_DATA_PTR	extern
> > > diff --git a/arch/powerpc/lib/board.c b/arch/powerpc/lib/board.c index
> > > 34bbfca..7383e32 100644
> > > --- a/arch/powerpc/lib/board.c
> > > +++ b/arch/powerpc/lib/board.c
> > > @@ -350,6 +350,13 @@ unsigned long logbuffer_base(void)  }  #endif
> > >
> > > +#ifdef CONFIG_DEEP_SLEEP
> > > +int check_warmboot(void)
> > > +{
> > > +	return !!(gd->flags & GD_FLG_WARM_BOOT); } #endif
> > 
> > Why?
> > 
> Why what? Why we need it?
> 
> It is a help function and used by ASM code in which
> we can't determine whether it is a warm reset boot.

Why don't you just open code it?

> > Again, you seem to be breaking the non-CONFIG_DEEP_SLEEP case.
> > 
> No.

Yes. :-)

> > > +#ifdef CONFIG_DEEP_SLEEP
> > > +	/*
> > > +	 * restore 128-byte memory space which corrupted
> > > +	 * by DDRC training.
> > > +	 */
> > > +	if (gd->flags & GD_FLG_WARM_BOOT) {
> > > +		src = (u64 *)in_be32(&scfg->sparecr[2]);
> > > +		dst = (u64 *)(0);
> > > +		for (i = 0; i < 128/sizeof(u64); i++) {
> > > +			*dst = *src;
> > > +			dst++;
> > > +			src++;
> > > +		}
> > > +	}
> > 
> > (u64 *)(0) is a NULL pointer.  Dereferencing NULL pointers is undefined
> > and GCC has been getting increasingly free with making surprising
> > optimizations based on that (such as assuming any code path that it knows
> > can reach a NULL dereference is dead code and can be removed).
> > 
> Then how we operate 0 address if not dereferencing NULL pointer?
> 

With an I/O accessor (or other inline asm), a TLB mapping, or using a
different memory location.

-Scott

[U-Boot] [U-Boot, 2/3] mpc85xx: Add deep sleep framework support

[Tang Yuantian-B29983]

On 2014/2/15 ??? 6:21, Scott Wood wrote:
> On Thu, 2014-02-13 at 01:05 -0600, Tang Yuantian-B29983 wrote:
>> Thanks for your review. Please see the reply inline.
>>
>> Thanks,
>> Yuantian
>>
>>> -----Original Message-----
>>> From: Wood Scott-B07421
>>> Sent: 2014?2?13? ??? 8:44
>>> To: Tang Yuantian-B29983
>>> Cc: Sun York-R58495; Wood Scott-B07421; Li Yang-Leo-R58472;
>>> Tang at theia.denx.de; u-boot at lists.denx.de; Kushwaha Prabhakar-B32579; Jin
>>> Zhengxiong-R64188
>>> Subject: Re: [U-Boot,2/3] mpc85xx: Add deep sleep framework support
>>>
>>> On Sun, Jan 26, 2014 at 02:00:40PM +0800, tang yuantian wrote:
>>>> From: Tang Yuantian <yuantian.tang@freescale.com>
>>>>
>>>> When system wakes up from warm reset, control is passed to the primary
>>>> core that starts executing uboot. After re-initialized some IP blocks,
>>>> like DDRC, kernel will take responsibility to continue to restore
>>>> environment it leaves before.
>>>>
>>>> Signed-off-by: Tang Yuantian <Yuantian.Tang@freescale.com>
>>> Is this for some specific mpc85xx chip (e.g. T1040)?  I'm pretty sure
>>> this isn't necessary for deep sleep on mpc8536, for example.
>>>
>> Currently, it is used by t1040. But we want it to be more general so that
>> It can be used by later new chips.
> But the mechanism is not the same for all mpc85xx derivatives.  You'll
> need a more specific name.
OK, will name it as t104x

>>>> +#ifdef CONFIG_DEEP_SLEEP
>>> CONFIG symbols need to be documented in README...
>>>
>> Where should I add this README?
> Under "85xx CPU Options" in the top-level README.
Thanks.

>>>> +	/* disable the console if boot from warm reset */
>>>> +	if (in_be32(&gur->scrtsr[0]) & (1 << 3))
>>>> +		gd->flags |=
>>>> +			GD_FLG_SILENT | GD_FLG_WARM_BOOT |
>>> GD_FLG_DISABLE_CONSOLE; #endif
>>>
>>> ...and it should probably be a more specific CONFIG_SYS symbol that
>>> indicates the presence of this guts bit.
where should I put this CONFIG_SYS_'s definition?

>>>> diff --git a/arch/powerpc/cpu/mpc85xx/fdt.c
>>>> b/arch/powerpc/cpu/mpc85xx/fdt.c index 33bc900..b3b9250 100644
>>>> --- a/arch/powerpc/cpu/mpc85xx/fdt.c
>>>> +++ b/arch/powerpc/cpu/mpc85xx/fdt.c
>>>> @@ -24,6 +24,9 @@ DECLARE_GLOBAL_DATA_PTR;  extern void
>>>> ft_qe_setup(void *blob);  extern void ft_fixup_num_cores(void *blob);
>>>> extern void ft_srio_setup(void *blob);
>>>> +#ifdef CONFIG_DEEP_SLEEP
>>>> +extern ulong __bss_end;
>>>> +#endif
>>> Don't ifdef declarations.
>>>
>>>>   #ifdef CONFIG_MP
>>>>   #include "mp.h"
>>>> @@ -35,6 +38,9 @@ void ft_fixup_cpu(void *blob, u64 memory_limit)
>>>>   	u32 bootpg = determine_mp_bootpg(NULL);
>>>>   	u32 id = get_my_id();
>>>>   	const char *enable_method;
>>>> +#ifdef CONFIG_DEEP_SLEEP
>>>> +	ulong len;
>>>> +#endif
>>>>
>>>>   	off = fdt_node_offset_by_prop_value(blob, -1, "device_type", "cpu",
>>> 4);
>>>>   	while (off != -FDT_ERR_NOTFOUND) {
>>>> @@ -77,6 +83,25 @@ void ft_fixup_cpu(void *blob, u64 memory_limit)
>>>>   				"device_type", "cpu", 4);
>>>>   	}
>>>>
>>>> +#ifdef CONFIG_DEEP_SLEEP
>>>> +	/*
>>>> +	 * reserve the memory space for warm reset.
>>>> +	 * This space will be re-used next time when boot from deep sleep.
>>>> +	 * The space includes bd_t, gd_t, stack and uboot image.
>>>> +	 * Reserve 1K for stack.
>>>> +	 */
>>>> +	len = sizeof(bd_t) + sizeof(gd_t) + (1 << 10);
>>>> +	/* round up to 4K */
>>>> +	len = (len + (4096 - 1)) & ~(4096 - 1);
>>>> +
>>>> +	off = fdt_add_mem_rsv(blob, gd->relocaddr - len,
>>>> +			len + ((ulong)&__bss_end - gd->relocaddr));
>>>> +	if (off < 0)
>>>> +		printf("Failed to reserve memory for warm reset: %s\n",
>>>> +			fdt_strerror(off));
>>>> +
>>>> +#endif
>>> Why do you need to reserve memory for this?  We didn't need to for deep
>>> sleep on MPC8313ERDB, which also goes through U-Boot on wake.  On
>>> MPC8313ERDB we transfer control to the kernel before relocating, so U-
>>> Boot never touches DDR.  bd_t, gd_t, and the stack should be in locked
>>> L1 cache, and the u-boot image should be in flash (or locked CPC if this
>>> is not a NOR flash boot).
>>>
>> In deep sleep many IP blocks are powered off like DDRC, LIODN, cpu. These IPs
>> are re-initialized after relocating.
>> So, we must jump to kernel after relocating.
> The DDR controller is initialized before relocating -- and of course the
> CPU is powered off on MPC8313ERDB deep sleep as well.
>
> LIODNs are a new concern for deep sleep, but that doesn't mean we should
> go through a bunch of unrelated code to get there.  Refactor the LIODN
> code to be usable before relocation, and not be tied to fdt fixups.
There are other IP blocks that need to be re-initialized, like SerDes, 
SMP, PCIe and
a lot of Errata. I don't want to refactor one by one.
Besides, coding in this way will not change the current execute flow.

>>>> +#ifndef CONFIG_DEEP_SLEEP
>>>>   	/* Reserve spin table page */
>>>>   	if (spin_tbl_addr < memory_limit) {
>>>>   		off = fdt_add_mem_rsv(blob,
>>>> @@ -108,6 +134,7 @@ void ft_fixup_cpu(void *blob, u64 memory_limit)
>>>>   			printf("Failed to reserve memory for spin table: %s\n",
>>>>   				fdt_strerror(off));
>>>>   	}
>>>> +#endif
>>> Explain.
>>>
>> Spin_tbl_addr has been reserved already.
> Where, and why is it different for non-CONFIG_DEEP_SLEEP?
right above:

+	/*
+	 * reserve the memory space for warm reset.
+	 * This space will be re-used next time when boot from deep sleep.
+	 * The space includes bd_t, gd_t, stack and uboot image.
+	 * Reserve 1K for stack.
+	 */

If deep sleep is supported, the sbin_table space will be reserved above.
we don't need to reserved it anymore.

>>>>   #if (CONFIG_SYS_NUM_FMAN == 2)
>>>> -	set_liodn(fman2_liodn_tbl, fman2_liodn_tbl_sz);
>>>> -	setup_fman_liodn_base(FSL_HW_PORTAL_FMAN2, fman2_liodn_tbl,
>>>> -				fman2_liodn_tbl_sz);
>>>> +#ifdef CONFIG_DEEP_SLEEP
>>>> +	if ((gd->flags & GD_FLG_WARM_BOOT) == 0) {
>>>> +		set_liodn(fman2_liodn_tbl, fman2_liodn_tbl_sz);
>>>> +		setup_fman_liodn_base(FSL_HW_PORTAL_FMAN2, fman2_liodn_tbl,
>>>> +					fman2_liodn_tbl_sz);
>>>> +	}
>>>> +#endif
>>>>   #endif
>>>>   #endif
>>> Aren't you breaking non-CONFIG_DEEP_SLEEP here?
>>>
>> No, if deep sleep feature is enabled, we need to further judge whether this boot is
>> Coming from deep sleep by GD_FLG_WARM_BOOT flag.
> My point is that if CONFIG_DEEP_SLEEP is not set, you will be skipping
> those calls to set_liodn() and setup_fman_liodn_base().
>
> #ifdef foo
> 	if (!bar)
> 		stuff();
> #endif
>
> is not equivalent to:
>
> #ifdef foo
> 	if (!bar)
> #endif
> 		stuff();
>
> Though the latter is better written as something like:
>
> 	bool do_stuff = true;
>
> #ifdef foo
> 	if (bar)
> 		do_stuff = false;
> #endif
>
> 	if (do_stuff)
> 		stuff();
>
> or
>
> static inline bool is_bar(void)
> {
> #ifdef foo
> 	return bar;
> #else
> 	return true;
> #endif
> }
>
> ...
>
> 	if (!is_bar())
> 		stuff();
You are absolutely right. How didn't I find this?

>>>> diff --git a/arch/powerpc/include/asm/global_data.h
>>>> b/arch/powerpc/include/asm/global_data.h
>>>> index 8e59e8b..1ab05ff 100644
>>>> --- a/arch/powerpc/include/asm/global_data.h
>>>> +++ b/arch/powerpc/include/asm/global_data.h
>>>> @@ -117,6 +117,7 @@ struct arch_global_data {  #include
>>>> <asm-generic/global_data.h>
>>>>
>>>>   #if 1
>>>> +#define GD_FLG_WARM_BOOT       0x10000
>>> Why is this not with the rest of the GD_FLGs, not numerically contiguous,
>>> and not commented?  What specifically does "warm boot" mean?  I think a
>>> lot of people would expect it to mean something that looks like a reset
>>> at the software level, while possibly not involving a real hardware reset
>>> -- coming out of deep sleep is the opposite of that.
>>>
>> Archdef document says it is a warm reset boot. So I name it this way.
> Hardware people sometimes use terminology differently than software
> people.
How about warm_reset?

>> Other platforms use the same flag, like x86, although I am not sure
>> Whether it is for deep sleep.
>> If you have better name, I love to use it.
> It's not yet clear to me that a GD flag is needed for this.
GD_ flag can be added for our own purpose. We decide to use a GD flag to
indicate the deep sleep case and I think it worth it.


>>>>   #define DECLARE_GLOBAL_DATA_PTR     register volatile gd_t *gd asm
>>> ("r2")
>>>>   #else /* We could use plain global data, but the resulting code is
>>> bigger */
>>>>   #define XTRN_DECLARE_GLOBAL_DATA_PTR	extern
>>>> diff --git a/arch/powerpc/lib/board.c b/arch/powerpc/lib/board.c index
>>>> 34bbfca..7383e32 100644
>>>> --- a/arch/powerpc/lib/board.c
>>>> +++ b/arch/powerpc/lib/board.c
>>>> @@ -350,6 +350,13 @@ unsigned long logbuffer_base(void)  }  #endif
>>>>
>>>> +#ifdef CONFIG_DEEP_SLEEP
>>>> +int check_warmboot(void)
>>>> +{
>>>> +	return !!(gd->flags & GD_FLG_WARM_BOOT); } #endif
>>> Why?
>>>
>> Why what? Why we need it?
>>
>> It is a help function and used by ASM code in which
>> we can't determine whether it is a warm reset boot.
> Why don't you just open code it?
I can't check the warmboot status in ASM code.
In order to get the warmboot status in ASM code, I wrote this function.

>>> Again, you seem to be breaking the non-CONFIG_DEEP_SLEEP case.
>>>
>> No.
> Yes. :-)
>
>>>> +#ifdef CONFIG_DEEP_SLEEP
>>>> +	/*
>>>> +	 * restore 128-byte memory space which corrupted
>>>> +	 * by DDRC training.
>>>> +	 */
>>>> +	if (gd->flags & GD_FLG_WARM_BOOT) {
>>>> +		src = (u64 *)in_be32(&scfg->sparecr[2]);
>>>> +		dst = (u64 *)(0);
>>>> +		for (i = 0; i < 128/sizeof(u64); i++) {
>>>> +			*dst = *src;
>>>> +			dst++;
>>>> +			src++;
>>>> +		}
>>>> +	}
>>> (u64 *)(0) is a NULL pointer.  Dereferencing NULL pointers is undefined
>>> and GCC has been getting increasingly free with making surprising
>>> optimizations based on that (such as assuming any code path that it knows
>>> can reach a NULL dereference is dead code and can be removed).
>>>
>> Then how we operate 0 address if not dereferencing NULL pointer?
>>
> With an I/O accessor (or other inline asm), a TLB mapping, or using a
> different memory location.
we found the zero address has benefit.
I don't know how to achieve this in inline asm or TLB mapping, could you
be more specific or write a example for me?

Regards,
Yuantian
> -Scott
>
>

[U-Boot] [U-Boot, 2/3] mpc85xx: Add deep sleep framework support

[Scott Wood]

On Mon, 2014-02-24 at 15:47 +0800, Tang Yuantian-B29983 wrote:
> On 2014/2/15 ??? 6:21, Scott Wood wrote:
> > On Thu, 2014-02-13 at 01:05 -0600, Tang Yuantian-B29983 wrote:
> >> Thanks for your review. Please see the reply inline.
> >>
> >> Thanks,
> >> Yuantian
> >>
> >>> -----Original Message-----
> >>> From: Wood Scott-B07421
> >>> Sent: 2014?2?13? ??? 8:44
> >>> To: Tang Yuantian-B29983
> >>> Cc: Sun York-R58495; Wood Scott-B07421; Li Yang-Leo-R58472;
> >>> Tang at theia.denx.de; u-boot at lists.denx.de; Kushwaha Prabhakar-B32579; Jin
> >>> Zhengxiong-R64188
> >>> Subject: Re: [U-Boot,2/3] mpc85xx: Add deep sleep framework support
> >>>
> >>> On Sun, Jan 26, 2014 at 02:00:40PM +0800, tang yuantian wrote:
> >>>> From: Tang Yuantian <yuantian.tang@freescale.com>
> >>>>
> >>>> When system wakes up from warm reset, control is passed to the primary
> >>>> core that starts executing uboot. After re-initialized some IP blocks,
> >>>> like DDRC, kernel will take responsibility to continue to restore
> >>>> environment it leaves before.
> >>>>
> >>>> Signed-off-by: Tang Yuantian <Yuantian.Tang@freescale.com>
> >>> Is this for some specific mpc85xx chip (e.g. T1040)?  I'm pretty sure
> >>> this isn't necessary for deep sleep on mpc8536, for example.
> >>>
> >> Currently, it is used by t1040. But we want it to be more general so that
> >> It can be used by later new chips.
> > But the mechanism is not the same for all mpc85xx derivatives.  You'll
> > need a more specific name.
> OK, will name it as t104x
> 
> >>>> +#ifdef CONFIG_DEEP_SLEEP
> >>> CONFIG symbols need to be documented in README...
> >>>
> >> Where should I add this README?
> > Under "85xx CPU Options" in the top-level README.
> Thanks.
> 
> >>>> +	/* disable the console if boot from warm reset */
> >>>> +	if (in_be32(&gur->scrtsr[0]) & (1 << 3))
> >>>> +		gd->flags |=
> >>>> +			GD_FLG_SILENT | GD_FLG_WARM_BOOT |
> >>> GD_FLG_DISABLE_CONSOLE; #endif
> >>>
> >>> ...and it should probably be a more specific CONFIG_SYS symbol that
> >>> indicates the presence of this guts bit.
> where should I put this CONFIG_SYS_'s definition?

Under "85xx CPU Options" in the top-level README. :-)

> >>>> diff --git a/arch/powerpc/cpu/mpc85xx/fdt.c
> >>>> b/arch/powerpc/cpu/mpc85xx/fdt.c index 33bc900..b3b9250 100644
> >>>> --- a/arch/powerpc/cpu/mpc85xx/fdt.c
> >>>> +++ b/arch/powerpc/cpu/mpc85xx/fdt.c
> >>>> @@ -24,6 +24,9 @@ DECLARE_GLOBAL_DATA_PTR;  extern void
> >>>> ft_qe_setup(void *blob);  extern void ft_fixup_num_cores(void *blob);
> >>>> extern void ft_srio_setup(void *blob);
> >>>> +#ifdef CONFIG_DEEP_SLEEP
> >>>> +extern ulong __bss_end;
> >>>> +#endif
> >>> Don't ifdef declarations.
> >>>
> >>>>   #ifdef CONFIG_MP
> >>>>   #include "mp.h"
> >>>> @@ -35,6 +38,9 @@ void ft_fixup_cpu(void *blob, u64 memory_limit)
> >>>>   	u32 bootpg = determine_mp_bootpg(NULL);
> >>>>   	u32 id = get_my_id();
> >>>>   	const char *enable_method;
> >>>> +#ifdef CONFIG_DEEP_SLEEP
> >>>> +	ulong len;
> >>>> +#endif
> >>>>
> >>>>   	off = fdt_node_offset_by_prop_value(blob, -1, "device_type", "cpu",
> >>> 4);
> >>>>   	while (off != -FDT_ERR_NOTFOUND) {
> >>>> @@ -77,6 +83,25 @@ void ft_fixup_cpu(void *blob, u64 memory_limit)
> >>>>   				"device_type", "cpu", 4);
> >>>>   	}
> >>>>
> >>>> +#ifdef CONFIG_DEEP_SLEEP
> >>>> +	/*
> >>>> +	 * reserve the memory space for warm reset.
> >>>> +	 * This space will be re-used next time when boot from deep sleep.
> >>>> +	 * The space includes bd_t, gd_t, stack and uboot image.
> >>>> +	 * Reserve 1K for stack.
> >>>> +	 */
> >>>> +	len = sizeof(bd_t) + sizeof(gd_t) + (1 << 10);
> >>>> +	/* round up to 4K */
> >>>> +	len = (len + (4096 - 1)) & ~(4096 - 1);
> >>>> +
> >>>> +	off = fdt_add_mem_rsv(blob, gd->relocaddr - len,
> >>>> +			len + ((ulong)&__bss_end - gd->relocaddr));
> >>>> +	if (off < 0)
> >>>> +		printf("Failed to reserve memory for warm reset: %s\n",
> >>>> +			fdt_strerror(off));
> >>>> +
> >>>> +#endif
> >>> Why do you need to reserve memory for this?  We didn't need to for deep
> >>> sleep on MPC8313ERDB, which also goes through U-Boot on wake.  On
> >>> MPC8313ERDB we transfer control to the kernel before relocating, so U-
> >>> Boot never touches DDR.  bd_t, gd_t, and the stack should be in locked
> >>> L1 cache, and the u-boot image should be in flash (or locked CPC if this
> >>> is not a NOR flash boot).
> >>>
> >> In deep sleep many IP blocks are powered off like DDRC, LIODN, cpu. These IPs
> >> are re-initialized after relocating.
> >> So, we must jump to kernel after relocating.
> > The DDR controller is initialized before relocating -- and of course the
> > CPU is powered off on MPC8313ERDB deep sleep as well.
> >
> > LIODNs are a new concern for deep sleep, but that doesn't mean we should
> > go through a bunch of unrelated code to get there.  Refactor the LIODN
> > code to be usable before relocation, and not be tied to fdt fixups.
> There are other IP blocks that need to be re-initialized, like SerDes, 
> SMP, PCIe and
> a lot of Errata. I don't want to refactor one by one.
> Besides, coding in this way will not change the current execute flow.

Changing the execution flow is better than adding a bunch of special
cases to the current execution flow.

Some of that reinitialization (e.g. PCIe) should be handled by Linux,
not U-Boot.

> >>>> +#ifndef CONFIG_DEEP_SLEEP
> >>>>   	/* Reserve spin table page */
> >>>>   	if (spin_tbl_addr < memory_limit) {
> >>>>   		off = fdt_add_mem_rsv(blob,
> >>>> @@ -108,6 +134,7 @@ void ft_fixup_cpu(void *blob, u64 memory_limit)
> >>>>   			printf("Failed to reserve memory for spin table: %s\n",
> >>>>   				fdt_strerror(off));
> >>>>   	}
> >>>> +#endif
> >>> Explain.
> >>>
> >> Spin_tbl_addr has been reserved already.
> > Where, and why is it different for non-CONFIG_DEEP_SLEEP?
> right above:
> 
> +	/*
> +	 * reserve the memory space for warm reset.
> +	 * This space will be re-used next time when boot from deep sleep.
> +	 * The space includes bd_t, gd_t, stack and uboot image.
> +	 * Reserve 1K for stack.
> +	 */
> 
> If deep sleep is supported, the sbin_table space will be reserved above.
> we don't need to reserved it anymore.

So the same memory is used for the spin table as for the deep sleep
stuff?  There's no conflict there?

> >>>>   #if (CONFIG_SYS_NUM_FMAN == 2)
> >>>> -	set_liodn(fman2_liodn_tbl, fman2_liodn_tbl_sz);
> >>>> -	setup_fman_liodn_base(FSL_HW_PORTAL_FMAN2, fman2_liodn_tbl,
> >>>> -				fman2_liodn_tbl_sz);
> >>>> +#ifdef CONFIG_DEEP_SLEEP
> >>>> +	if ((gd->flags & GD_FLG_WARM_BOOT) == 0) {
> >>>> +		set_liodn(fman2_liodn_tbl, fman2_liodn_tbl_sz);
> >>>> +		setup_fman_liodn_base(FSL_HW_PORTAL_FMAN2, fman2_liodn_tbl,
> >>>> +					fman2_liodn_tbl_sz);
> >>>> +	}
> >>>> +#endif
> >>>>   #endif
> >>>>   #endif
> >>> Aren't you breaking non-CONFIG_DEEP_SLEEP here?
> >>>
> >> No, if deep sleep feature is enabled, we need to further judge whether this boot is
> >> Coming from deep sleep by GD_FLG_WARM_BOOT flag.
> > My point is that if CONFIG_DEEP_SLEEP is not set, you will be skipping
> > those calls to set_liodn() and setup_fman_liodn_base().
> >
> > #ifdef foo
> > 	if (!bar)
> > 		stuff();
> > #endif
> >
> > is not equivalent to:
> >
> > #ifdef foo
> > 	if (!bar)
> > #endif
> > 		stuff();
> >
> > Though the latter is better written as something like:
> >
> > 	bool do_stuff = true;
> >
> > #ifdef foo
> > 	if (bar)
> > 		do_stuff = false;
> > #endif
> >
> > 	if (do_stuff)
> > 		stuff();
> >
> > or
> >
> > static inline bool is_bar(void)
> > {
> > #ifdef foo
> > 	return bar;
> > #else
> > 	return true;
> > #endif
> > }
> >
> > ...
> >
> > 	if (!is_bar())
> > 		stuff();
> You are absolutely right. How didn't I find this?

By not testing with CONFIG_DEEP_SLEEP off? :-)

> >>>> diff --git a/arch/powerpc/include/asm/global_data.h
> >>>> b/arch/powerpc/include/asm/global_data.h
> >>>> index 8e59e8b..1ab05ff 100644
> >>>> --- a/arch/powerpc/include/asm/global_data.h
> >>>> +++ b/arch/powerpc/include/asm/global_data.h
> >>>> @@ -117,6 +117,7 @@ struct arch_global_data {  #include
> >>>> <asm-generic/global_data.h>
> >>>>
> >>>>   #if 1
> >>>> +#define GD_FLG_WARM_BOOT       0x10000
> >>> Why is this not with the rest of the GD_FLGs, not numerically contiguous,
> >>> and not commented?  What specifically does "warm boot" mean?  I think a
> >>> lot of people would expect it to mean something that looks like a reset
> >>> at the software level, while possibly not involving a real hardware reset
> >>> -- coming out of deep sleep is the opposite of that.
> >>>
> >> Archdef document says it is a warm reset boot. So I name it this way.
> > Hardware people sometimes use terminology differently than software
> > people.
> How about warm_reset?

"warm reset" and "warm boot" mean pretty much the same thing to me.  I'd
stick with "deep sleep" terminology.

> >> Other platforms use the same flag, like x86, although I am not sure
> >> Whether it is for deep sleep.
> >> If you have better name, I love to use it.
> > It's not yet clear to me that a GD flag is needed for this.
> GD_ flag can be added for our own purpose. We decide to use a GD flag to
> indicate the deep sleep case and I think it worth it.

It depends on how far into the boot sequence we go.

> >>>>   #define DECLARE_GLOBAL_DATA_PTR     register volatile gd_t *gd asm
> >>> ("r2")
> >>>>   #else /* We could use plain global data, but the resulting code is
> >>> bigger */
> >>>>   #define XTRN_DECLARE_GLOBAL_DATA_PTR	extern
> >>>> diff --git a/arch/powerpc/lib/board.c b/arch/powerpc/lib/board.c index
> >>>> 34bbfca..7383e32 100644
> >>>> --- a/arch/powerpc/lib/board.c
> >>>> +++ b/arch/powerpc/lib/board.c
> >>>> @@ -350,6 +350,13 @@ unsigned long logbuffer_base(void)  }  #endif
> >>>>
> >>>> +#ifdef CONFIG_DEEP_SLEEP
> >>>> +int check_warmboot(void)
> >>>> +{
> >>>> +	return !!(gd->flags & GD_FLG_WARM_BOOT); } #endif
> >>> Why?
> >>>
> >> Why what? Why we need it?
> >>
> >> It is a help function and used by ASM code in which
> >> we can't determine whether it is a warm reset boot.
> > Why don't you just open code it?
> I can't check the warmboot status in ASM code.
> In order to get the warmboot status in ASM code, I wrote this function.

Why can't you check it in asm code?  See lib/asm-offsets.c.

> >>>> +	if (gd->flags & GD_FLG_WARM_BOOT) {
> >>>> +		src = (u64 *)in_be32(&scfg->sparecr[2]);
> >>>> +		dst = (u64 *)(0);
> >>>> +		for (i = 0; i < 128/sizeof(u64); i++) {
> >>>> +			*dst = *src;
> >>>> +			dst++;
> >>>> +			src++;
> >>>> +		}
> >>>> +	}
> >>> (u64 *)(0) is a NULL pointer.  Dereferencing NULL pointers is undefined
> >>> and GCC has been getting increasingly free with making surprising
> >>> optimizations based on that (such as assuming any code path that it knows
> >>> can reach a NULL dereference is dead code and can be removed).
> >>>
> >> Then how we operate 0 address if not dereferencing NULL pointer?
> >>
> > With an I/O accessor (or other inline asm), a TLB mapping, or using a
> > different memory location.
> we found the zero address has benefit.
> I don't know how to achieve this in inline asm or TLB mapping, could you
> be more specific or write a example for me?

Inline asm would be something like:

	asm("stw %1, 0(%0); stw %2, 4(%0)" : "=r" (dst) :
		"r" ((u32)(src >> 32)), "r" ((u32)src));

-Scott

[U-Boot] [U-Boot, 2/3] mpc85xx: Add deep sleep framework support

[Tang Yuantian-B29983]

On 2014/2/25 ??? 3:11, Scott Wood wrote:
> On Mon, 2014-02-24 at 15:47 +0800, Tang Yuantian-B29983 wrote:
>> On 2014/2/15 ??? 6:21, Scott Wood wrote:
>>> On Thu, 2014-02-13 at 01:05 -0600, Tang Yuantian-B29983 wrote:
>>>> Thanks for your review. Please see the reply inline.
>>>>
>>>> Thanks,
>>>> Yuantian
>>>>
>>>>> -----Original Message-----
>>>>> From: Wood Scott-B07421
>>>>> Sent: 2014?2?13? ??? 8:44
>>>>> To: Tang Yuantian-B29983
>>>>> Cc: Sun York-R58495; Wood Scott-B07421; Li Yang-Leo-R58472;
>>>>> Tang at theia.denx.de; u-boot at lists.denx.de; Kushwaha Prabhakar-B32579; Jin
>>>>> Zhengxiong-R64188
>>>>> Subject: Re: [U-Boot,2/3] mpc85xx: Add deep sleep framework support
>>>>>
>>>>> On Sun, Jan 26, 2014 at 02:00:40PM +0800, tang yuantian wrote:
>>>>>> From: Tang Yuantian <yuantian.tang@freescale.com>
>>>>>>
>>>>>> When system wakes up from warm reset, control is passed to the primary
>>>>>> core that starts executing uboot. After re-initialized some IP blocks,
>>>>>> like DDRC, kernel will take responsibility to continue to restore
>>>>>> environment it leaves before.
>>>>>>
>>>>>> Signed-off-by: Tang Yuantian <Yuantian.Tang@freescale.com>
>>>>> Is this for some specific mpc85xx chip (e.g. T1040)?  I'm pretty sure
>>>>> this isn't necessary for deep sleep on mpc8536, for example.
>>>>>
>>>> Currently, it is used by t1040. But we want it to be more general so that
>>>> It can be used by later new chips.
>>> But the mechanism is not the same for all mpc85xx derivatives.  You'll
>>> need a more specific name.
>> OK, will name it as t104x
>>
>>>>>> +#ifdef CONFIG_DEEP_SLEEP
>>>>> CONFIG symbols need to be documented in README...
>>>>>
>>>> Where should I add this README?
>>> Under "85xx CPU Options" in the top-level README.
>> Thanks.
>>
>>>>>> +	/* disable the console if boot from warm reset */
>>>>>> +	if (in_be32(&gur->scrtsr[0]) & (1 << 3))
>>>>>> +		gd->flags |=
>>>>>> +			GD_FLG_SILENT | GD_FLG_WARM_BOOT |
>>>>> GD_FLG_DISABLE_CONSOLE; #endif
>>>>>
>>>>> ...and it should probably be a more specific CONFIG_SYS symbol that
>>>>> indicates the presence of this guts bit.
>> where should I put this CONFIG_SYS_'s definition?
> Under "85xx CPU Options" in the top-level README. :-)
I don't find other gut bit that defined in this README. Do I need to?
Just we are clear that you want me to add a CONFIG_SYS_'s definition for 
(1 << 3) bit in GUTS, right?

>>>>>> diff --git a/arch/powerpc/cpu/mpc85xx/fdt.c
>>>>>> b/arch/powerpc/cpu/mpc85xx/fdt.c index 33bc900..b3b9250 100644
>>>>>> --- a/arch/powerpc/cpu/mpc85xx/fdt.c
>>>>>> +++ b/arch/powerpc/cpu/mpc85xx/fdt.c
>>>>>> @@ -24,6 +24,9 @@ DECLARE_GLOBAL_DATA_PTR;  extern void
>>>>>> ft_qe_setup(void *blob);  extern void ft_fixup_num_cores(void *blob);
>>>>>> extern void ft_srio_setup(void *blob);
>>>>>> +#ifdef CONFIG_DEEP_SLEEP
>>>>>> +extern ulong __bss_end;
>>>>>> +#endif
>>>>> Don't ifdef declarations.
>>>>>
>>>>>>    #ifdef CONFIG_MP
>>>>>>    #include "mp.h"
>>>>>> @@ -35,6 +38,9 @@ void ft_fixup_cpu(void *blob, u64 memory_limit)
>>>>>>    	u32 bootpg = determine_mp_bootpg(NULL);
>>>>>>    	u32 id = get_my_id();
>>>>>>    	const char *enable_method;
>>>>>> +#ifdef CONFIG_DEEP_SLEEP
>>>>>> +	ulong len;
>>>>>> +#endif
>>>>>>
>>>>>>    	off = fdt_node_offset_by_prop_value(blob, -1, "device_type", "cpu",
>>>>> 4);
>>>>>>    	while (off != -FDT_ERR_NOTFOUND) {
>>>>>> @@ -77,6 +83,25 @@ void ft_fixup_cpu(void *blob, u64 memory_limit)
>>>>>>    				"device_type", "cpu", 4);
>>>>>>    	}
>>>>>>
>>>>>> +#ifdef CONFIG_DEEP_SLEEP
>>>>>> +	/*
>>>>>> +	 * reserve the memory space for warm reset.
>>>>>> +	 * This space will be re-used next time when boot from deep sleep.
>>>>>> +	 * The space includes bd_t, gd_t, stack and uboot image.
>>>>>> +	 * Reserve 1K for stack.
>>>>>> +	 */
>>>>>> +	len = sizeof(bd_t) + sizeof(gd_t) + (1 << 10);
>>>>>> +	/* round up to 4K */
>>>>>> +	len = (len + (4096 - 1)) & ~(4096 - 1);
>>>>>> +
>>>>>> +	off = fdt_add_mem_rsv(blob, gd->relocaddr - len,
>>>>>> +			len + ((ulong)&__bss_end - gd->relocaddr));
>>>>>> +	if (off < 0)
>>>>>> +		printf("Failed to reserve memory for warm reset: %s\n",
>>>>>> +			fdt_strerror(off));
>>>>>> +
>>>>>> +#endif
>>>>> Why do you need to reserve memory for this?  We didn't need to for deep
>>>>> sleep on MPC8313ERDB, which also goes through U-Boot on wake.  On
>>>>> MPC8313ERDB we transfer control to the kernel before relocating, so U-
>>>>> Boot never touches DDR.  bd_t, gd_t, and the stack should be in locked
>>>>> L1 cache, and the u-boot image should be in flash (or locked CPC if this
>>>>> is not a NOR flash boot).
>>>>>
>>>> In deep sleep many IP blocks are powered off like DDRC, LIODN, cpu. These IPs
>>>> are re-initialized after relocating.
>>>> So, we must jump to kernel after relocating.
>>> The DDR controller is initialized before relocating -- and of course the
>>> CPU is powered off on MPC8313ERDB deep sleep as well.
>>>
>>> LIODNs are a new concern for deep sleep, but that doesn't mean we should
>>> go through a bunch of unrelated code to get there.  Refactor the LIODN
>>> code to be usable before relocation, and not be tied to fdt fixups.
>> There are other IP blocks that need to be re-initialized, like SerDes,
>> SMP, PCIe and
>> a lot of Errata. I don't want to refactor one by one.
>> Besides, coding in this way will not change the current execute flow.
> Changing the execution flow is better than adding a bunch of special
> cases to the current execution flow.
>
> Some of that reinitialization (e.g. PCIe) should be handled by Linux,
> not U-Boot.
That's not that easy. If you refactor a function you need to refactor 
many other codes it depends on.
There are also a number of Errata. They are not wrapped in one function.
Refactoring all these is really a big challenge. Just image how many 
codes need to be refactored
between the first code line in start.S and cpu_init_r().
We believe at least cpu_init_r() needs to be executed before jumping to 
kernel.

>>>>>> +#ifndef CONFIG_DEEP_SLEEP
>>>>>>    	/* Reserve spin table page */
>>>>>>    	if (spin_tbl_addr < memory_limit) {
>>>>>>    		off = fdt_add_mem_rsv(blob,
>>>>>> @@ -108,6 +134,7 @@ void ft_fixup_cpu(void *blob, u64 memory_limit)
>>>>>>    			printf("Failed to reserve memory for spin table: %s\n",
>>>>>>    				fdt_strerror(off));
>>>>>>    	}
>>>>>> +#endif
>>>>> Explain.
>>>>>
>>>> Spin_tbl_addr has been reserved already.
>>> Where, and why is it different for non-CONFIG_DEEP_SLEEP?
>> right above:
>>
>> +	/*
>> +	 * reserve the memory space for warm reset.
>> +	 * This space will be re-used next time when boot from deep sleep.
>> +	 * The space includes bd_t, gd_t, stack and uboot image.
>> +	 * Reserve 1K for stack.
>> +	 */
>>
>> If deep sleep is supported, the sbin_table space will be reserved above.
>> we don't need to reserved it anymore.
> So the same memory is used for the spin table as for the deep sleep
> stuff?  There's no conflict there?
This space is not used by deep sleep. It is used by spin_table and uboot 
itself at first.
I just keep it not to free.

>>>>>>    #if (CONFIG_SYS_NUM_FMAN == 2)
>>>>>> -	set_liodn(fman2_liodn_tbl, fman2_liodn_tbl_sz);
>>>>>> -	setup_fman_liodn_base(FSL_HW_PORTAL_FMAN2, fman2_liodn_tbl,
>>>>>> -				fman2_liodn_tbl_sz);
>>>>>> +#ifdef CONFIG_DEEP_SLEEP
>>>>>> +	if ((gd->flags & GD_FLG_WARM_BOOT) == 0) {
>>>>>> +		set_liodn(fman2_liodn_tbl, fman2_liodn_tbl_sz);
>>>>>> +		setup_fman_liodn_base(FSL_HW_PORTAL_FMAN2, fman2_liodn_tbl,
>>>>>> +					fman2_liodn_tbl_sz);
>>>>>> +	}
>>>>>> +#endif
>>>>>>    #endif
>>>>>>    #endif
>>>>> Aren't you breaking non-CONFIG_DEEP_SLEEP here?
>>>>>
>>>> No, if deep sleep feature is enabled, we need to further judge whether this boot is
>>>> Coming from deep sleep by GD_FLG_WARM_BOOT flag.
>>> My point is that if CONFIG_DEEP_SLEEP is not set, you will be skipping
>>> those calls to set_liodn() and setup_fman_liodn_base().
>>>
>>> #ifdef foo
>>> 	if (!bar)
>>> 		stuff();
>>> #endif
>>>
>>> is not equivalent to:
>>>
>>> #ifdef foo
>>> 	if (!bar)
>>> #endif
>>> 		stuff();
>>>
>>> Though the latter is better written as something like:
>>>
>>> 	bool do_stuff = true;
>>>
>>> #ifdef foo
>>> 	if (bar)
>>> 		do_stuff = false;
>>> #endif
>>>
>>> 	if (do_stuff)
>>> 		stuff();
>>>
>>> or
>>>
>>> static inline bool is_bar(void)
>>> {
>>> #ifdef foo
>>> 	return bar;
>>> #else
>>> 	return true;
>>> #endif
>>> }
>>>
>>> ...
>>>
>>> 	if (!is_bar())
>>> 		stuff();
>> You are absolutely right. How didn't I find this?
> By not testing with CONFIG_DEEP_SLEEP off? :-)
You are right again. :(
When I turn off DEEP_SLEEP, I found some compile warnings.

>>>>>> diff --git a/arch/powerpc/include/asm/global_data.h
>>>>>> b/arch/powerpc/include/asm/global_data.h
>>>>>> index 8e59e8b..1ab05ff 100644
>>>>>> --- a/arch/powerpc/include/asm/global_data.h
>>>>>> +++ b/arch/powerpc/include/asm/global_data.h
>>>>>> @@ -117,6 +117,7 @@ struct arch_global_data {  #include
>>>>>> <asm-generic/global_data.h>
>>>>>>
>>>>>>    #if 1
>>>>>> +#define GD_FLG_WARM_BOOT       0x10000
>>>>> Why is this not with the rest of the GD_FLGs, not numerically contiguous,
>>>>> and not commented?  What specifically does "warm boot" mean?  I think a
>>>>> lot of people would expect it to mean something that looks like a reset
>>>>> at the software level, while possibly not involving a real hardware reset
>>>>> -- coming out of deep sleep is the opposite of that.
>>>>>
>>>> Archdef document says it is a warm reset boot. So I name it this way.
>>> Hardware people sometimes use terminology differently than software
>>> people.
>> How about warm_reset?
> "warm reset" and "warm boot" mean pretty much the same thing to me.  I'd
> stick with "deep sleep" terminology.
This flag indicates the BOOT method. that is not related to deep sleep.
Some other features can use this flag if they want to.
We have power on reset, hardware reset currently. So warm_reset is 
acceptable.

>>>> Other platforms use the same flag, like x86, although I am not sure
>>>> Whether it is for deep sleep.
>>>> If you have better name, I love to use it.
>>> It's not yet clear to me that a GD flag is needed for this.
>> GD_ flag can be added for our own purpose. We decide to use a GD flag to
>> indicate the deep sleep case and I think it worth it.
> It depends on how far into the boot sequence we go.
As I said above, at least cpu_init_r() is finished.

>>>>>>    #define DECLARE_GLOBAL_DATA_PTR     register volatile gd_t *gd asm
>>>>> ("r2")
>>>>>>    #else /* We could use plain global data, but the resulting code is
>>>>> bigger */
>>>>>>    #define XTRN_DECLARE_GLOBAL_DATA_PTR	extern
>>>>>> diff --git a/arch/powerpc/lib/board.c b/arch/powerpc/lib/board.c index
>>>>>> 34bbfca..7383e32 100644
>>>>>> --- a/arch/powerpc/lib/board.c
>>>>>> +++ b/arch/powerpc/lib/board.c
>>>>>> @@ -350,6 +350,13 @@ unsigned long logbuffer_base(void)  }  #endif
>>>>>>
>>>>>> +#ifdef CONFIG_DEEP_SLEEP
>>>>>> +int check_warmboot(void)
>>>>>> +{
>>>>>> +	return !!(gd->flags & GD_FLG_WARM_BOOT); } #endif
>>>>> Why?
>>>>>
>>>> Why what? Why we need it?
>>>>
>>>> It is a help function and used by ASM code in which
>>>> we can't determine whether it is a warm reset boot.
>>> Why don't you just open code it?
>> I can't check the warmboot status in ASM code.
>> In order to get the warmboot status in ASM code, I wrote this function.
> Why can't you check it in asm code?  See lib/asm-offsets.c.
Not find in both kernel and uboot source code.

>>>>>> +	if (gd->flags & GD_FLG_WARM_BOOT) {
>>>>>> +		src = (u64 *)in_be32(&scfg->sparecr[2]);
>>>>>> +		dst = (u64 *)(0);
>>>>>> +		for (i = 0; i < 128/sizeof(u64); i++) {
>>>>>> +			*dst = *src;
>>>>>> +			dst++;
>>>>>> +			src++;
>>>>>> +		}
>>>>>> +	}
>>>>> (u64 *)(0) is a NULL pointer.  Dereferencing NULL pointers is undefined
>>>>> and GCC has been getting increasingly free with making surprising
>>>>> optimizations based on that (such as assuming any code path that it knows
>>>>> can reach a NULL dereference is dead code and can be removed).
>>>>>
>>>> Then how we operate 0 address if not dereferencing NULL pointer?
>>>>
>>> With an I/O accessor (or other inline asm), a TLB mapping, or using a
>>> different memory location.
>> we found the zero address has benefit.
>> I don't know how to achieve this in inline asm or TLB mapping, could you
>> be more specific or write a example for me?
> Inline asm would be something like:
>
> 	asm("stw %1, 0(%0); stw %2, 4(%0)" : "=r" (dst) :
> 		"r" ((u32)(src >> 32)), "r" ((u32)src));
Thank you very much. As I said in other thread, if you are sure about 
32bits thing, I will change it.

Regards,
Yuantian

> -Scott
>
>

[U-Boot] [U-Boot, 2/3] mpc85xx: Add deep sleep framework support

[Tang Yuantian-B29983]

On 2014/2/25 ??? 3:11, Scott Wood wrote:
>>>> Why what? Why we need it?
>>>>
>>>> It is a help function and used by ASM code in which
>>>> we can't determine whether it is a warm reset boot.
>>> Why don't you just open code it?
>> I can't check the warmboot status in ASM code.
>> In order to get the warmboot status in ASM code, I wrote this function.
> Why can't you check it in asm code?  See lib/asm-offsets.c.
Found it. Still learn how to use it.

Thanks,
Yuantian

>>>>>> +	if (gd->flags & GD_FLG_WARM_BOOT) {
>>>>>> +		src = (u64 *)in_be32(&scfg->sparecr[2]);
>>>>>> +		dst = (u64 *)(0);
>>>>>> +		for (i = 0; i < 128/sizeof(u64); i++) {
>>>>>> +			*dst = *src;
>>>>>> +			dst++;
>>>>>> +			src++;
>>>>>> +		}
>>>>>> +	}
>>>>> (u64 *)(0) is a NULL pointer.  Dereferencing NULL pointers is undefined
>>>>> and GCC has been getting increasingly free with making surprising
>>>>> optimizations based on that (such as assuming any code path that it knows
>>>>> can reach a NULL dereference is dead code and can be removed).
>>>>>
>>>> Then how we operate 0 address if not dereferencing NULL pointer?
>>>>
>>> With an I/O accessor (or other inline asm), a TLB mapping, or using a
>>> different memory location.
>> we found the zero address has benefit.
>> I don't know how to achieve this in inline asm or TLB mapping, could you
>> be more specific or write a example for me?
> Inline asm would be something like:
>
> 	asm("stw %1, 0(%0); stw %2, 4(%0)" : "=r" (dst) :
> 		"r" ((u32)(src >> 32)), "r" ((u32)src));
>
> -Scott
>
>

[U-Boot] [U-Boot, 2/3] mpc85xx: Add deep sleep framework support

[Scott Wood]

On Thu, 2014-02-13 at 02:12 -0600, Tang Yuantian-B29983 wrote:
> > Use an I/O accessor. 
> In_be64?? No such function.

Why do you need in_be64() rather than two in_be32()s?

-Scott

[U-Boot] test

[Tang Yuantian]

test.

? 2014/2/15 ??? 6:59, Scott Wood ??:
> On Thu, 2014-02-13 at 02:12 -0600, Tang Yuantian-B29983 wrote:
>>> Use an I/O accessor.
>> In_be64?? No such function.
> Why do you need in_be64() rather than two in_be32()s?
>
> -Scott
>
>

[U-Boot] [U-Boot, 2/3] mpc85xx: Add deep sleep framework support

[Scott Wood]

On Sun, 2014-02-16 at 21:35 -0600, Tang Yuantian-B29983 wrote:
> > -----Original Message-----
> > From: Wood Scott-B07421
> > To: Tang Yuantian-B29983
> > Cc: Sun York-R58495; Li Yang-Leo-R58472; u-boot at lists.denx.de; Kushwaha
> > Prabhakar-B32579; Jin Zhengxiong-R64188
> > Subject: Re: [U-Boot,2/3] mpc85xx: Add deep sleep framework support
> > 
> > On Thu, 2014-02-13 at 02:12 -0600, Tang Yuantian-B29983 wrote:
> > > > Use an I/O accessor.
> > > In_be64?? No such function.
> > 
> > Why do you need in_be64() rather than two in_be32()s?
> > 
> Avoid ECC error. Although, according to my test, in_be32() works too.

Why would you get an ECC error?

-Scott

[U-Boot] [U-Boot, 2/3] mpc85xx: Add deep sleep framework support

[Tang Yuantian-B29983]

On 2014/2/18 ??? 3:18, Scott Wood wrote:
> On Sun, 2014-02-16 at 21:35 -0600, Tang Yuantian-B29983 wrote:
>>> -----Original Message-----
>>> From: Wood Scott-B07421
>>> To: Tang Yuantian-B29983
>>> Cc: Sun York-R58495; Li Yang-Leo-R58472; u-boot at lists.denx.de; Kushwaha
>>> Prabhakar-B32579; Jin Zhengxiong-R64188
>>> Subject: Re: [U-Boot,2/3] mpc85xx: Add deep sleep framework support
>>>
>>> On Thu, 2014-02-13 at 02:12 -0600, Tang Yuantian-B29983 wrote:
>>>>> Use an I/O accessor.
>>>> In_be64?? No such function.
>>> Why do you need in_be64() rather than two in_be32()s?
>>>
>> Avoid ECC error. Although, according to my test, in_be32() works too.
> Why would you get an ECC error?
>
> -Scott
DDR operation is always in 64bits. if writing a 32bits to memory, you 
need to
read a 32bits first, and combine it to form a 64bits. when the new 
64bits is written
to memory, ECC occurs.
I was required to do so by hardware team.

Regards,
Yuantian

>

[U-Boot] [U-Boot, 2/3] mpc85xx: Add deep sleep framework support

[Scott Wood]

On Mon, 2014-02-24 at 14:44 +0800, Tang Yuantian-B29983 wrote:
> On 2014/2/18 ??? 3:18, Scott Wood wrote:
> > On Sun, 2014-02-16 at 21:35 -0600, Tang Yuantian-B29983 wrote:
> >>> -----Original Message-----
> >>> From: Wood Scott-B07421
> >>> To: Tang Yuantian-B29983
> >>> Cc: Sun York-R58495; Li Yang-Leo-R58472; u-boot at lists.denx.de; Kushwaha
> >>> Prabhakar-B32579; Jin Zhengxiong-R64188
> >>> Subject: Re: [U-Boot,2/3] mpc85xx: Add deep sleep framework support
> >>>
> >>> On Thu, 2014-02-13 at 02:12 -0600, Tang Yuantian-B29983 wrote:
> >>>>> Use an I/O accessor.
> >>>> In_be64?? No such function.
> >>> Why do you need in_be64() rather than two in_be32()s?
> >>>
> >> Avoid ECC error. Although, according to my test, in_be32() works too.
> > Why would you get an ECC error?
> >
> > -Scott
> DDR operation is always in 64bits. if writing a 32bits to memory, you 
> need to
> read a 32bits first, and combine it to form a 64bits. when the new 
> 64bits is written
> to memory, ECC occurs.
> I was required to do so by hardware team.

U-Boot on PPC is a 32-bit binary (even on 64-bit hardware), so the
compiler is already turning that into two 32-bit writes.

-Scott

[U-Boot] [U-Boot, 2/3] mpc85xx: Add deep sleep framework support

[Tang Yuantian-B29983]

On 2014/2/25 ??? 3:11, Scott Wood wrote:
> On Mon, 2014-02-24 at 14:44 +0800, Tang Yuantian-B29983 wrote:
>> On 2014/2/18 ??? 3:18, Scott Wood wrote:
>>> On Sun, 2014-02-16 at 21:35 -0600, Tang Yuantian-B29983 wrote:
>>>>> -----Original Message-----
>>>>> From: Wood Scott-B07421
>>>>> To: Tang Yuantian-B29983
>>>>> Cc: Sun York-R58495; Li Yang-Leo-R58472; u-boot at lists.denx.de; Kushwaha
>>>>> Prabhakar-B32579; Jin Zhengxiong-R64188
>>>>> Subject: Re: [U-Boot,2/3] mpc85xx: Add deep sleep framework support
>>>>>
>>>>> On Thu, 2014-02-13 at 02:12 -0600, Tang Yuantian-B29983 wrote:
>>>>>>> Use an I/O accessor.
>>>>>> In_be64?? No such function.
>>>>> Why do you need in_be64() rather than two in_be32()s?
>>>>>
>>>> Avoid ECC error. Although, according to my test, in_be32() works too.
>>> Why would you get an ECC error?
>>>
>>> -Scott
>> DDR operation is always in 64bits. if writing a 32bits to memory, you
>> need to
>> read a 32bits first, and combine it to form a 64bits. when the new
>> 64bits is written
>> to memory, ECC occurs.
>> I was required to do so by hardware team.
> U-Boot on PPC is a 32-bit binary (even on 64-bit hardware), so the
> compiler is already turning that into two 32-bit writes.
>
> -Scott
I quote: (from Welker James A.-RA8497 )
3) You only need 8-byte (or multiple of 8-byte) transactions when 
initializing the memory.  Typically, we would simply use 2, 64-byte 
writes to DRAM. This is because a sub-doubleword transactions will 
result in a read-modify-write, which would encounter an ECC error (and 
then mask the write data).  After the first 128-bytes of memory have 
been re-initialized, you can issue any transactions to those locations 
again.

I think the transactions between CPU and DDRC are always in 64 bits 
physically because DDRC has 64 bits data bus.
But I am sure about this.
If you are sure about this, I will change as your suggestion.

Regards,
Yuantian

[U-Boot] [PATCH 3/3] mpc85xx: Add deep sleep support on T1040QDS

[Tang Yuantian]

From: Tang Yuantian <yuantian.tang@freescale.com>

Add deep sleep support on T1040QDS platforms.

Signed-off-by: Tang Yuantian <Yuantian.Tang@freescale.com>
---
 board/freescale/t1040qds/t1040qds.c | 12 ++++++++++++
 include/configs/T1040QDS.h          |  4 ++++
 2 files changed, 16 insertions(+)

diff --git a/board/freescale/t1040qds/t1040qds.c b/board/freescale/t1040qds/t1040qds.c
index de3ea5c..2494da6 100644
--- a/board/freescale/t1040qds/t1040qds.c
+++ b/board/freescale/t1040qds/t1040qds.c
@@ -18,6 +18,7 @@
 #include <asm/fsl_portals.h>
 #include <asm/fsl_liodn.h>
 #include <fm_eth.h>
+#include <asm/mpc85xx_gpio.h>
 
 #include "../common/qixis.h"
 #include "t1040qds.h"
@@ -244,3 +245,14 @@ int board_need_mem_reset(void)
 {
 	return 1;
 }
+
+#ifdef CONFIG_DEEP_SLEEP
+void board_mem_sleep_setup(void)
+{
+	/* does not provide HW signals for power management */
+	QIXIS_WRITE(pwr_ctl[1], (QIXIS_READ(pwr_ctl[1]) & ~0x2));
+	/* Disable MCKE isolation */
+	gpio_set_value(2, 0);
+	udelay(1);
+}
+#endif
diff --git a/include/configs/T1040QDS.h b/include/configs/T1040QDS.h
index 91b511b..9c5daf1 100644
--- a/include/configs/T1040QDS.h
+++ b/include/configs/T1040QDS.h
@@ -36,6 +36,10 @@
 #define CONFIG_SYS_FSL_PBL_RCW $(SRCTREE)/board/freescale/t1040qds/t1040_rcw.cfg
 #endif
 
+/* support deep sleep */
+#define CONFIG_DEEP_SLEEP
+#define CONFIG_SILENT_CONSOLE
+
 /* High Level Configuration Options */
 #define CONFIG_BOOKE
 #define CONFIG_E500			/* BOOKE e500 family */
-- 
1.8.0