Sorry for late reply since the email system crash.
> -----Original Message----- > From: Scott Wood [mailto:scottw...@freescale.com] > Sent: Saturday, December 07, 2013 9:22 AM > To: Liu Po-B43644 > Cc: u-boot@lists.denx.de; Sun York-R58495; Kushwaha Prabhakar-B32579 > Subject: Re: [PATCH v2 2/2] powerpc/c29xpcie: 8k page size NAND boot > support base on TPL/SPL > > On Thu, 2013-12-05 at 14:19 +0800, Po Liu wrote: > > diff --git a/board/freescale/c29xpcie/spl.c > > b/board/freescale/c29xpcie/spl.c new file mode 100644 index > > 0000000..7bc8ce1 > > --- /dev/null > > +++ b/board/freescale/c29xpcie/spl.c > > @@ -0,0 +1,73 @@ > > +/* Copyright 2013 Freescale Semiconductor, Inc. > > + * > > + * SPDX-License-Identifier: GPL-2.0+ > > + */ > > + > > +#include <common.h> > > +#include <ns16550.h> > > +#include <malloc.h> > > +#include <mmc.h> > > +#include <nand.h> > > +#include <i2c.h> > > + > > +DECLARE_GLOBAL_DATA_PTR; > > + > > +ulong get_effective_memsize(void) > > +{ > > + return CONFIG_SYS_L2_SIZE; > > +} > > + > > +void board_init_f(ulong bootflag) > > +{ > > + u32 plat_ratio; > > + ccsr_gur_t *gur = (void *)CONFIG_SYS_MPC85xx_GUTS_ADDR; > > + > > + console_init_f(); > > + > > + /* initialize selected port with appropriate baud rate */ > > + plat_ratio = in_be32(&gur->porpllsr) & MPC85xx_PORPLLSR_PLAT_RATIO; > > + plat_ratio >>= 1; > > + gd->bus_clk = CONFIG_SYS_CLK_FREQ * plat_ratio; > > + > > + NS16550_init((NS16550_t)CONFIG_SYS_NS16550_COM1, > > + gd->bus_clk / 16 / CONFIG_BAUDRATE); > > + > > + /* copy code to RAM and jump to it - this should not return */ > > + /* NOTE - code has to be copied out of NAND buffer before > > + * other blocks can be read. > > + */ > > + relocate_code(CONFIG_SPL_RELOC_STACK, 0, > > +CONFIG_SPL_RELOC_TEXT_BASE); } > > + > > +void board_init_r(gd_t *gd, ulong dest_addr) { > > + /* Pointer is writable since we allocated a register for it */ > > + gd = (gd_t *)CONFIG_SPL_GD_ADDR; > > + bd_t *bd; > > + > > + memset(gd, 0, sizeof(gd_t)); > > + bd = (bd_t *)(CONFIG_SPL_GD_ADDR + sizeof(gd_t)); > > + memset(bd, 0, sizeof(bd_t)); > > + gd->bd = bd; > > + bd->bi_memstart = CONFIG_SYS_INIT_L2_ADDR; > > + bd->bi_memsize = CONFIG_SYS_L2_SIZE; > > + > > + probecpu(); > > + get_clocks(); > > + mem_malloc_init(CONFIG_SPL_RELOC_MALLOC_ADDR, > > + CONFIG_SPL_RELOC_MALLOC_SIZE); > > + > > + /* relocate environment function pointers etc. */ > > + nand_spl_load_image(CONFIG_ENV_OFFSET, CONFIG_ENV_SIZE, > > + (uchar *)CONFIG_ENV_ADDR); > > + gd->env_addr = (ulong)(CONFIG_ENV_ADDR); > > + gd->env_valid = 1; > > + > > + i2c_init_all(); > > + > > + gd->ram_size = initdram(0); > > + > > + puts("\nTertiary program loader running in sram..."); > > Why do you assume tertiary? Couldn't this be SPL for SD/SPI? Or was it > a copy/paste error that you added things to the board config file for > SD/SPI (after all, the subject line says it's a NAND patch)? > Yes, I assume this patch only for NAND boot for C29XPCIE. > > +void board_init_r(gd_t *gd, ulong dest_addr) { > > + puts("\nSecond program loader running in sram..."); > > I see that this isn't new to this patch, but we ought to be consistent > and either change this to "secondary", or change "tertiary" to "third". > > It's also probably too verbose... Simply saying "SPL\n" or "TPL\n" > would suffice to indicate progress and verify that console output is > working (if nothing is printed, then that path doesn't get tested in the > absence of a load error). > > > diff --git a/board/freescale/c29xpcie/tlb.c > > b/board/freescale/c29xpcie/tlb.c index 84844ee..11f8a37 100644 > > --- a/board/freescale/c29xpcie/tlb.c > > +++ b/board/freescale/c29xpcie/tlb.c > > @@ -26,10 +26,20 @@ struct fsl_e_tlb_entry tlb_table[] = { > > 0, 0, BOOKE_PAGESZ_4K, 0), > > > > /* TLB 1 */ > > +#ifdef CONFIG_SPL_NAND_MINIMAL > > + SET_TLB_ENTRY(1, 0xfffff000, 0xfffff000, > > + MAS3_SX|MAS3_SW|MAS3_SR, MAS2_I|MAS2_G, > > + 0, 10, BOOKE_PAGESZ_4K, 1), > > + SET_TLB_ENTRY(1, 0xffffe000, 0xffffe000, > > + MAS3_SX|MAS3_SW|MAS3_SR, MAS2_I|MAS2_G, > > + 0, 11, BOOKE_PAGESZ_4K, 1), > > +#endif > > CONFIG_SPL_NAND_MINIMAL should not exist. It was introduced by accident > after a different approach was chosen in patch review (and even then, > this wasn't what it meant). > > Prabhakar, why did you extend that to other uses? Why are both entries > ifdeffed here, but only the 0xffffe000 entry on existing boards? > > If this needs to be ifdeffed (and it probably does, if only to avoid > possible speculative instruction fetches), use (and document) > CONFIG_SPL_NAND_BOOT. I'll replace the CONFIG_SPL_NAND_MINIMAL with CONFIG_SPL_NAND_BOOT > > > SET_TLB_ENTRY(1, CONFIG_SYS_NAND_BASE, CONFIG_SYS_NAND_BASE_PHYS, > > - MAS3_SW|MAS3_SR, MAS2_I|MAS2_G, > > + MAS3_SX|MAS3_SW|MAS3_SR, MAS2_I|MAS2_G, > > 0, 5, BOOKE_PAGESZ_64K, 1), > > No. > > > SET_TLB_ENTRY(1, CONFIG_SYS_PLATFORM_SRAM_BASE, @@ -61,7 +72,8 @@ > > struct fsl_e_tlb_entry tlb_table[] = { > > MAS3_SX|MAS3_SW|MAS3_SR, 0, > > 0, 7, BOOKE_PAGESZ_256K, 1), > > > > -#ifdef CONFIG_SYS_RAMBOOT > > +#if defined(CONFIG_SYS_RAMBOOT) || (defined(CONFIG_SPL) \ > > + && !defined(CONFIG_SPL_COMMON_INIT_DDR)) > > SET_TLB_ENTRY(1, CONFIG_SYS_DDR_SDRAM_BASE, > > CONFIG_SYS_DDR_SDRAM_BASE, > > MAS3_SX|MAS3_SW|MAS3_SR, 0, > > This will have the result of mapping DDR in the SPL where it's not used, > but not in the TPL where it is. Just define: #if defined(CONFIG_SYS_RAMBOOT) || defined(CONFIG_SPL) ? I think code load image to SRAM from NAND for SPL and DDR do not need to be allocate. > > -Scott > >
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
