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

[Prabhakar Kushwaha]

On 12/7/2013 6:51 AM, Scott Wood wrote:
> 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).
I was not aware of this. My mistake :(

> Prabhakar, why did you extend that to other uses?  Why are both entries
> ifdeffed here, but only the 0xffffe000 entry on existing boards?

both entry should not be in ifdef. p1010rdb/bsc9131rdb/bsc9132qds does 
not have this.
i dont think NOR boot tested after this patch. NOR boot will not work 
after applying this patch.

> 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.
Yes, I will suggest to have CONFIG_SPL_NAND_BOOT instead of 
CONFIG_SPL_NAND_MINIMAL.
I will fix for p1010rdb, bsc9131rdb, bsc9132qds.

Regards,
Prabhakar