[U-Boot] [PATCH v2 2/2] powerpc/c29xpcie: 8k page size NAND boot support base on TPL/SPL

[Scott Wood]

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)?

> +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.

>  	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.

-Scott