[U-Boot] [U-Boot, 2/3] mpc85xx: Add deep sleep framework support
Scott Wood
scottwood at freescale.com
Thu Feb 13 01:44:12 CET 2014
On Sun, Jan 26, 2014 at 02:00:40PM +0800, tang yuantian wrote:
> From: Tang Yuantian <yuantian.tang at 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 at 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
More information about the U-Boot
mailing list