powerpc/c29xpcie: 8k page size NAND boot support base on TPL/SPL

board/freescale/c29xpcie/spl.c new file +60 -0

new file mode 100644
index 0000000..7bc8ce1
@@ -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. */

board/freescale/c29xpcie/tlb.c +0 -0

index 84844ee..11f8a37 100644
@@ -26,10 +26,20 @@ struct fsl_e_tlb_entry tlb_table[] = {
 			0, 0, BOOKE_PAGESZ_4K, 0),

Quoted 66 lines · Po Liu

board/freescale/c29xpcie/spl.c new file +60 -0

new file mode 100644
index 0000000..7bc8ce1
@@ -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. */

board/freescale/c29xpcie/spl.c line 70

Quoted 10 lines

board/freescale/c29xpcie/spl.c new file +10 -0

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

board/freescale/c29xpcie/spl.c line 73

Quoted 3 lines

board/freescale/c29xpcie/spl.c new file +3 -0

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

Quoted 7 lines

board/freescale/c29xpcie/tlb.c +0 -0

index 84844ee..11f8a37 100644
@@ -26,10 +26,20 @@ struct fsl_e_tlb_entry tlb_table[] = {
  			0, 0, BOOKE_PAGESZ_4K, 0),
 

board/freescale/c29xpcie/tlb.c struct fsl_e_tlb_entry tlb_table[] = { · line 36

Quoted 9 lines

board/freescale/c29xpcie/tlb.c +8 -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.

Quoted 2 lines · Scott Wood

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.

Quoted 3 lines · Prabhakar Kushwaha

Quoted 2 lines · Scott Wood

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

Quoted 4 lines

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.

So what happens if there's a speculative access to the non-ifdeffed 0xfffff000 when we're not booting from that (e.g. ramboot, SPL payload, SD/SPI...)?

Quoted 4 lines · Scott Wood

Quoted 3 lines · Prabhakar Kushwaha

Quoted 2 lines · Scott Wood

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

Quoted 9 lines

Quoted 4 lines

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.

So what happens if there's a speculative access to the non-ifdeffed 0xfffff000 when we're not booting from that (e.g. ramboot, SPL payload, SD/SPI...)?

If I understand the question correctly,
Ideally ramboot, SPL payload, SD/SPI should not make access to this
address. They assumed to be running from DDR whose TLB has already been created by IBR, or First stage boot loader.

-----Original Message-----
From: Scott Wood [mailto:scottwood at freescale.com]
Sent: Wednesday, December 11, 2013 2:20 AM
To: Kushwaha Prabhakar-B32579
Cc: Liu Po-B43644; u-boot at lists.denx.de; Sun York-R58495
Subject: Re: [PATCH v2 2/2] powerpc/c29xpcie: 8k page size NAND boot support base on TPL/SPL

Quoted 5 lines · Prabhakar Kushwaha

Quoted 4 lines · Scott Wood

Quoted 3 lines · Prabhakar Kushwaha

Quoted 2 lines · Scott Wood

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

boards?

Quoted 9 lines

Quoted 9 lines

Quoted 4 lines

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.

So what happens if there's a speculative access to the non-ifdeffed 0xfffff000 when we're not booting from that (e.g. ramboot, SPL payload, SD/SPI...)?

Quoted 4 lines

If I understand the question correctly,
Ideally ramboot, SPL payload, SD/SPI should not make access to
this address. They assumed to be running from DDR whose TLB has already been created by IBR, or First stage boot loader.

Speculative accesses don't come (directly) from software. They are initiated by the hardware and are not predictable.

-Scott

Quoted 24 lines

-----Original Message-----
From: Scott Wood [mailto:scottwood at freescale.com]
Sent: Wednesday, December 11, 2013 2:20 AM
To: Kushwaha Prabhakar-B32579
Cc: Liu Po-B43644; u-boot at lists.denx.de; Sun York-R58495
Subject: Re: [PATCH v2 2/2] powerpc/c29xpcie: 8k page size NAND boot support base on TPL/SPL

Quoted 5 lines · Prabhakar Kushwaha

Quoted 4 lines · Scott Wood

Quoted 3 lines · Prabhakar Kushwaha

Quoted 2 lines · Scott Wood

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

boards?

Quoted 9 lines

Quoted 9 lines

Quoted 4 lines

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.

So what happens if there's a speculative access to the non-ifdeffed 0xfffff000 when we're not booting from that (e.g. ramboot, SPL payload, SD/SPI...)?

Quoted 7 lines

Quoted 4 lines

If I understand the question correctly,
Ideally ramboot, SPL payload, SD/SPI should not make access to
this address. They assumed to be running from DDR whose TLB has already been created by IBR, or First stage boot loader.

Speculative accesses don't come (directly) from software. They are initiated by the hardware and are not predictable.

Remove the:

(quoted diff) +0 -8

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

Do not effect the NAND/NOR boot after I test in C29XPCIE.

Quoted 14 lines · Prabhakar Kushwaha

Quoted 13 lines · Scott Wood

Quoted 7 lines · Prabhakar Kushwaha

Quoted 2 lines · Scott Wood

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.

So what happens if there's a speculative access to the non-ifdeffed 0xfffff000 when we're not booting from that (e.g. ramboot, SPL payload, SD/SPI...)?

Quoted 4 lines

If I understand the question correctly,
Ideally ramboot, SPL payload, SD/SPI should not make access to this
address. They assumed to be running from DDR whose TLB has already been created by IBR, or First stage boot loader.

Speculative accesses don't come (directly) from software. They are initiated by the hardware and are not predictable.

Quoted 15 lines · Scott Wood

Quoted 14 lines · Prabhakar Kushwaha

Quoted 13 lines · Scott Wood

Quoted 7 lines · Prabhakar Kushwaha

Quoted 2 lines · Scott Wood

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.

So what happens if there's a speculative access to the non-ifdeffed 0xfffff000 when we're not booting from that (e.g. ramboot, SPL payload, SD/SPI...)?

Quoted 8 lines

Quoted 4 lines

If I understand the question correctly,
Ideally ramboot, SPL payload, SD/SPI should not make access to this
address. They assumed to be running from DDR whose TLB has already been created by IBR, or First stage boot loader.

Speculative accesses don't come (directly) from software. They are initiated by the hardware and are not predictable.

Please help me in understanding this.
What are the scenario where it is possible? means how hardware can
initiate any access?
do hardware initiated transaction require TLB?

-----Original Message-----
From: Scott Wood [mailto:scottwood at freescale.com]
Sent: Saturday, December 07, 2013 9:22 AM
To: Liu Po-B43644
Cc: u-boot at 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

Quoted 71 lines · Po Liu

(quoted diff) +0 -0

b/board/freescale/c29xpcie/spl.c new file mode 100644 index 0000000..7bc8ce1

board/freescale/c29xpcie/spl.c new file +65 -0

@@ -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();

board/freescale/c29xpcie/spl.c line 70

Quoted 4 lines

board/freescale/c29xpcie/spl.c new file +4 -0

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

board/freescale/c29xpcie/spl.c line 71

Quoted 2 lines

board/freescale/c29xpcie/spl.c new file +2 -0

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

Quoted 6 lines

(quoted diff) +0 -0

b/board/freescale/c29xpcie/tlb.c index 84844ee..11f8a37 100644

board/freescale/c29xpcie/tlb.c +0 -0

@@ -26,10 +26,20 @@ struct fsl_e_tlb_entry tlb_table[] = {
                    0, 0, BOOKE_PAGESZ_4K, 0),

board/freescale/c29xpcie/tlb.c struct fsl_e_tlb_entry tlb_table[] = { · line 36

Quoted 10 lines

board/freescale/c29xpcie/tlb.c +8 -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.

board/freescale/c29xpcie/tlb.c struct fsl_e_tlb_entry tlb_table[] = { · line 64

Quoted 5 lines

board/freescale/c29xpcie/tlb.c +1 -1

    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.

Quoted 5 lines

board/freescale/c29xpcie/tlb.c +0 -0

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

board/freescale/c29xpcie/tlb.c struct fsl_e_tlb_entry tlb_table[] = { · line 79

Quoted 6 lines

board/freescale/c29xpcie/tlb.c +2 -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

Sorry for late reply since the email system crash.

Quoted 5 lines

-----Original Message-----
From: Scott Wood [mailto:scottwood at freescale.com]
Sent: Saturday, December 07, 2013 9:22 AM
To: Liu Po-B43644
Cc: u-boot at lists.denx.de; Sun York-R58495; Kushwaha

Prabhakar-B32579

Quoted 1 line

Subject: Re: [PATCH v2 2/2] powerpc/c29xpcie: 8k page size NAND

boot

Quoted 29 lines

support base on TPL/SPL

Quoted 26 lines · Po Liu

(quoted diff) +0 -0

b/board/freescale/c29xpcie/spl.c new file mode 100644 index 0000000..7bc8ce1

board/freescale/c29xpcie/spl.c new file +20 -0

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

Quoted 8 lines

board/freescale/c29xpcie/spl.c

Quoted 8 lines

board/freescale/c29xpcie/spl.c new file +8 -0

+{
+  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;

Quoted 42 lines

Quoted 41 lines

board/freescale/c29xpcie/spl.c new file +41 -0

+  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...");

Quoted 1 line

Why do you assume tertiary? Couldn't this be SPL for SD/SPI? Or

was it

Quoted 1 line

a copy/paste error that you added things to the board config file

for

Quoted 2 lines

SD/SPI (after all, the subject line says it's a NAND patch)?

Yes, I assume this patch only for NAND boot for C29XPCIE.

board/freescale/c29xpcie/tlb.c struct fsl_e_tlb_entry tlb_table[] = { · line 64

Quoted 6 lines

SET_TLB_ENTRY(1, CONFIG_SYS_NAND_BASE, CONFIG_SYS_NAND_BASE_PHYS,

board/freescale/c29xpcie/tlb.c +1 -1

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

Quoted 6 lines

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

board/freescale/c29xpcie/tlb.c struct fsl_e_tlb_entry tlb_table[] = { · line 79

Quoted 6 lines

board/freescale/c29xpcie/tlb.c +2 -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

Quoted 6 lines

board/freescale/c29xpcie/tlb.c struct fsl_e_tlb_entry tlb_table[] = { · line 64

Quoted 4 lines

board/freescale/c29xpcie/tlb.c +1 -1

 	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.

Without MAS3_SX, the board can't run in booting from NAND.

Quoted 12 lines

Quoted 11 lines

SET_TLB_ENTRY(1, CONFIG_SYS_PLATFORM_SRAM_BASE, @@ -61,7 +72,8 @@
struct fsl_e_tlb_entry tlb_table[] = {

board/freescale/c29xpcie/tlb.c +2 -1

 			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,

Quoted 2 lines

This will have the result of mapping DDR in the SPL where it's not used, but not in the TPL where it is.

For the TPL the ddr initial by initdram(), so don't need to mapping ddr here.
I intend to change it to #if defined(CONFIG_SYS_RAMBOOT) || !defined(CONFIG_SPL_COMMON_INIT_DDR) Is it more clear?

Quoted 15 lines

-----Original Message-----
From: Wood Scott-B07421
Sent: Saturday, December 14, 2013 4:26 AM
To: Liu Po-B43644
Cc: u-boot at 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

Quoted 1 line · On Wed, 2013-12-11 at 00:16 -0600, Liu Po-B43644 wrote:

Quoted 1 line

Quoted 1 line

SET_TLB_ENTRY(1, CONFIG_SYS_NAND_BASE,

CONFIG_SYS_NAND_BASE_PHYS,

Quoted 4 lines

Quoted 4 lines

Quoted 3 lines

board/freescale/c29xpcie/tlb.c +1 -1

-			MAS3_SW|MAS3_SR, MAS2_I|MAS2_G,
+			MAS3_SX|MAS3_SW|MAS3_SR, MAS2_I|MAS2_G,
 			0, 5, BOOKE_PAGESZ_64K, 1),

Quoted 8 lines

Quoted 2 lines

Quoted 1 line

No.

Without MAS3_SX, the board can't run in booting from NAND.

Explain why you're executing from this TLB entry. This should be for accessing the NAND as I/O after you've relocated away from the boot buffer. There should be a different mapping for the boot buffer early on. None of the other NAND boot targets have needed this. Perhaps this is related to the other TLB entry that you said you could work without.

This is because for SPL, the
#define CONFIG_SPL_TEXT_BASE 0xff800000
#define CONFIG_SYS_NAND_BASE 0xff800000
Code space overlap the nand io space.

drivers/mtd/nand/fsl_ifc_spl.c +4 -0

index 9de327b..28656f2 100644
@@ -88,7 +88,11 @@ static inline int bad_block(uchar *marker, int port_size)
 		return __raw_readw((u16 *)marker) != 0xffff;
 }
 
+#ifdef CONFIG_TPL_BUILD
+void nand_spl_load_image(uint32_t offs, int uboot_size, void *vdst)
+#else
 static void nand_load(unsigned int offs, int uboot_size, uchar *dst)
+#endif
 {
 	struct fsl_ifc *ifc = IFC_BASE_ADDR;
 	uchar *buf = (uchar *)CONFIG_SYS_NAND_BASE;

Using the TPL/SPL method to booting from 8k page NAND flash.
- Add 256kB size SRAM tlb for second step booting;
- Add spl.c for TPL image boot;
- Add spl_minimal.c for minimal SPL image;
- Add C29XPCIE_NAND configure;
- Modify C29XPCIE.h for nand config and enviroment;

Signed-off-by Po Liu <[email protected]>

---
changes for v2:
- seperate the public code and c29xpcie board code;
changes for v3:
- booting log simple to "SPL" "TPL"
- remove the 8k TLB from 0xffffe000 to 0xffffffff
- change the ddr tlb mapping condition
changes for v4:
- None.
changes for v5:
- code style change.
changes for v6:
- none

Using the TPL method for nand boot by sram was already supported. Here add some code for mpc85xx ifc nand boot.

- For ifc, elbc, esdhc, espi, all need the SPL without section .resetvec.
- Use a clear function name for nand spl boot.
- Add CONFIG_SPL_DRIVERS_MISC_SUPPORT to compile the fsl_ifc.c in spl/Makefile;

Signed-off-by Po Liu <[email protected]>

---
changes for v2:
- seperate public code and c29xpcie board code
- add ifc support
changes for v3:
- remove the redundant plus
- ifc support use CONFIG_SPL_DRIVERS_MISC_SUPPORT
changes for v4:
- modify the nand_load function
- add comments in README.SPL
changes for v5:
-none
changes for v6:
- modify the readme file for the include lib file

 arch/powerpc/cpu/mpc85xx/u-boot-spl.lds | 15 ++++++++-------
 doc/README.SPL                          |  1 +
 drivers/mtd/nand/fsl_ifc_spl.c          | 31 ++++++++++++++++++++++++-------
 spl/Makefile                            |  1 +
 4 files changed, 34 insertions(+), 14 deletions(-)

Quoted 25 lines · Po Liu

Using the TPL method for nand boot by sram was already supported. Here add some code for mpc85xx ifc nand boot.

- For ifc, elbc, esdhc, espi, all need the SPL without section .resetvec.
- Use a clear function name for nand spl boot.
- Add CONFIG_SPL_DRIVERS_MISC_SUPPORT to compile the fsl_ifc.c in spl/Makefile;

Signed-off-by Po Liu <[email protected]>

---
changes for v2:
- seperate public code and c29xpcie board code
- add ifc support
changes for v3:
- remove the redundant plus
- ifc support use CONFIG_SPL_DRIVERS_MISC_SUPPORT
changes for v4:
- modify the nand_load function
- add comments in README.SPL
changes for v5:
-none
changes for v6:
- modify the readme file for the include lib file

Quoted 5 lines

 arch/powerpc/cpu/mpc85xx/u-boot-spl.lds | 15 ++++++++-------
 doc/README.SPL                          |  1 +
 drivers/mtd/nand/fsl_ifc_spl.c          | 31 ++++++++++++++++++++++++-------
 spl/Makefile                            |  1 +
 4 files changed, 34 insertions(+), 14 deletions(-)

Acked-by Scott Wood <[email protected]>

Using the TPL method for nand boot by sram was already supported. Here add some code for mpc85xx ifc nand boot.

- For ifc, elbc, esdhc, espi, all need the SPL without section .resetvec.
- Use a clear function name for nand spl boot.
- Add CONFIG_SPL_DRIVERS_MISC_SUPPORT to compile the fsl_ifc.c in spl/Makefile;

Signed-off-by Po Liu <[email protected]>

---
changes for v2:
- seperate public code and c29xpcie board code
- add ifc support
changes for v3:
- remove the redundant plus
- ifc support use CONFIG_SPL_DRIVERS_MISC_SUPPORT
changes for v4:
- modify the nand_load function
- add comments in README.SPL
changes for v5:
-none
changes for v6:
- modify the readme file for the include lib file

  arch/powerpc/cpu/mpc85xx/u-boot-spl.lds | 15 ++++++++-------
  doc/README.SPL                          |  1 +
  drivers/mtd/nand/fsl_ifc_spl.c          | 31 ++++++++++++++++++++++++-------
  spl/Makefile                            |  1 +
  4 files changed, 34 insertions(+), 14 deletions(-)

arch/powerpc/cpu/mpc85xx/u-boot-spl.lds +8 -7

index bc13267..acaa093 100644
@@ -57,7 +57,14 @@ SECTIONS
  	. = ALIGN(8);
  	__init_begin = .;
  	__init_end = .;
-/* FIXME for non-NAND SPL */
+
+/* For ifc, elbc, esdhc, espi, all need the SPL without section .resetvec */
+#ifdef CONFIG_SYS_MPC85XX_NO_RESETVEC
+	.bootpg ADDR(.text) - 0x1000 :
+	{
+		KEEP(*(.bootpg))
+	} :text = 0xffff
+#else
  #if defined(CONFIG_FSL_IFC) /* Restrict bootpg at 4K boundry for IFC */
  	.bootpg ADDR(.text) + 0x1000 :
  	{

@@ -69,12 +76,6 @@ SECTIONS
  #else
  #error unknown NAND controller
  #endif
-#ifdef CONFIG_SYS_MPC85XX_NO_RESETVEC
-	.bootpg ADDR(.text) - 0x1000 :
-	{
-		KEEP(*(.bootpg))
-	} :text = 0xffff
-#else
  	.resetvec ADDR(.text) + RESET_VECTOR_OFFSET : {
  		KEEP(*(.resetvec))
  	} = 0xffff

doc/README.SPL +1 -0

index 312a6a6..b1bc3ca 100644
@@ -62,6 +62,7 @@ CONFIG_SPL_FAT_SUPPORT (fs/fat/libfat.o)
  CONFIG_SPL_LIBGENERIC_SUPPORT (lib/libgeneric.o)
  CONFIG_SPL_POWER_SUPPORT (drivers/power/libpower.o)
  CONFIG_SPL_NAND_SUPPORT (drivers/mtd/nand/libnand.o)
+CONFIG_SPL_DRIVERS_MISC_SUPPORT (drivers/misc)
  CONFIG_SPL_DMA_SUPPORT (drivers/dma/libdma.o)
  CONFIG_SPL_POST_MEM_SUPPORT (post/drivers/memory.o)
  CONFIG_SPL_NAND_LOAD (drivers/mtd/nand/nand_spl_load.o)

drivers/mtd/nand/fsl_ifc_spl.c +0 -0

index 9de327b..6b43496 100644

Quoted 5 lines · Po Liu

drivers/mtd/nand/fsl_ifc_spl.c +0 -0

index 9de327b..6b43496 100644

drivers/mtd/nand/fsl_ifc_spl.c static inline int bad_block(uchar *marker, int port_size) · line 95

Quoted 9 lines

drivers/mtd/nand/fsl_ifc_spl.c +5 -1

@@ -88,7 +88,11 @@ static inline int bad_block(uchar *marker, int port_size)
  		return __raw_readw((u16 *)marker) != 0xffff;
  }
 
-static void nand_load(unsigned int offs, int uboot_size, uchar *dst)
+#ifdef CONFIG_TPL_BUILD
+int nand_spl_load_image(uint32_t offs, unsigned int uboot_size, void *vdst)
+#else
+static int nand_load(uint32_t offs, unsigned int uboot_size, void *vdst)
+#endif

nand_load definition is

     static void nand_load(unsigned int offs, int uboot_size, uchar *dst);

have you checked the compilation warning for non TPL framework?

Quoted 9 lines · Prabhakar Kushwaha

Quoted 8 lines · Po Liu

drivers/mtd/nand/fsl_ifc_spl.c +0 -0

index 9de327b..6b43496 100644
@@ -88,7 +88,11 @@ static inline int bad_block(uchar *marker, int port_size)
   		return __raw_readw((u16 *)marker) != 0xffff;
   }
 

Quoted 8 lines

drivers/mtd/nand/fsl_ifc_spl.c static inline int bad_block(uchar *marker, int port_size) · line 95

Quoted 6 lines

drivers/mtd/nand/fsl_ifc_spl.c +5 -1

-static void nand_load(unsigned int offs, int uboot_size, uchar *dst)
+#ifdef CONFIG_TPL_BUILD
+int nand_spl_load_image(uint32_t offs, unsigned int uboot_size, void *vdst)
+#else
+static int nand_load(uint32_t offs, unsigned int uboot_size, void *vdst)
+#endif

nand_load definition is

      static void nand_load(unsigned int offs, int uboot_size, uchar *dst);

Not anymore...

drivers/mtd/nand/fsl_ifc_spl.c +4 -0

index 9de327b..93303b4 100644
@@ -88,7 +88,11 @@ static inline int bad_block(uchar *marker, int port_size)
 		return __raw_readw((u16 *)marker) != 0xffff;
 }
 
+#ifdef CONFIG_TPL_BUILD
+void nand_spl_load_image(uint32_t offs, int uboot_size, void *vdst)
+#else
 static void nand_load(unsigned int offs, int uboot_size, uchar *dst)
+#endif
 {
 	struct fsl_ifc *ifc = IFC_BASE_ADDR;
 	uchar *buf = (uchar *)CONFIG_SYS_NAND_BASE;