p1014rdb: set ddr bus width properly depending on SVR

Let's use the more appropriate udelay for the nand_spl. While we can't make use of u-boot's full udelay we can atl east use a for loop that won't get optimized away .Since we have the bus clock we can use the timebase to calculate wall time.

Looked at reusing the u-boot udelay functions but it pulls in a lot of code as well as depends on the gd struct and would require a lot of rework

+extern u32 bus_clk;

nand_spl/board/freescale/p1010rdb/Makefile +2 -1

index 8d240ea..cdbd492 100644
@@ -39,7 +39,8 @@ CFLAGS	+= -DCONFIG_NAND_SPL
 
 SOBJS	= start.o resetvec.o ticks.o
 COBJS	= cache.o cpu_init_early.o cpu_init_nand.o fsl_law.o law.o \
-	  nand_boot.o nand_boot_fsl_ifc.o ns16550.o tlb.o tlb_table.o
+	  nand_boot.o nand_boot_fsl_ifc.o ns16550.o tlb.o tlb_table.o \
+	  ../common.o
 
 SRCS	:= $(addprefix $(obj),$(SOBJS:.o=.S) $(COBJS:.o=.c))
 OBJS	:= $(addprefix $(obj),$(SOBJS) $(COBJS))

Quoted 8 lines · Matthew McClintock

Let's use the more appropriate udelay for the nand_spl. While we can't make use of u-boot's full udelay we can atl east use a for loop that won't get optimized away .Since we have the bus clock we can use the timebase to calculate wall time.

Looked at reusing the u-boot udelay functions but it pulls in a lot of code as well as depends on the gd struct and would require a lot of rework

What's wrong with depending on the gd struct?

Quoted 1 line

+extern u32 bus_clk;

Change to gd->bus_clk as mentioned in patch 5/6.

nand_spl/board/freescale/common.c line 36

Quoted 7 lines

nand_spl/board/freescale/common.c new file +7 -0

+void udelay(unsigned long usec) {
+     u32 ticks_per_usec = bus_clk / (8 * 1000000);
+     u32 ticks = ticks_per_usec * usec;
+     u32 s = mfspr(SPRN_TBRL);
+
+     while ((mfspr(SPRN_TBRL)-s) < ticks);
+}

Opening brace goes on its own line for function definitions. Use spaces around that '-'.

Dividing the bus clock by 8 only works for e500v1/v2, but this isn't labelled as specific to those cores.

Quoted 13 lines

nand_spl/board/freescale/p1010rdb/Makefile +2 -1

index 8d240ea..cdbd492 100644
@@ -39,7 +39,8 @@ CFLAGS      += -DCONFIG_NAND_SPL
 
 SOBJS        = start.o resetvec.o ticks.o
 COBJS        = cache.o cpu_init_early.o cpu_init_nand.o fsl_law.o law.o \
-       nand_boot.o nand_boot_fsl_ifc.o ns16550.o tlb.o tlb_table.o
+       nand_boot.o nand_boot_fsl_ifc.o ns16550.o tlb.o tlb_table.o \
+       ../common.o
 

nand_spl/board/freescale/p1010rdb/Makefile ifneq ($(OBJTREE), $(SRCTREE)) · line 130

Quoted 10 lines

nand_spl/board/freescale/p1010rdb/Makefile +3 -0

 SRCS := $(addprefix $(obj),$(SOBJS:.o=.S) $(COBJS:.o=.c))
 OBJS := $(addprefix $(obj),$(SOBJS) $(COBJS))

@@ -123,6 +124,9 @@ ifneq ($(OBJTREE), $(SRCTREE))
 $(obj)nand_boot.c:
      @rm -f $(obj)nand_boot.c
      ln -s $(SRCTREE)/nand_spl/board/$(BOARDDIR)/nand_boot.c $(obj)nand_boot.c
+$(obj)../common.c:
+     @rm -f $(obj)../common.c
+     ln -s $(SRCTREE)/nand_spl/board/freescale/common.c $(obj)../common.c
 endif

Why are you creating a link in the parent directory?

Quoted 6 lines · Scott Wood

Quoted 5 lines · Matthew McClintock

Let's use the more appropriate udelay for the nand_spl. While we can't make use of u-boot's full udelay we can atl east use a for loop that won't get optimized away .Since we have the bus clock we can use the timebase to calculate wall time.

Quoted 5 lines

Quoted 3 lines

Looked at reusing the u-boot udelay functions but it pulls in a lot of code as well as depends on the gd struct and would require a lot of rework

What's wrong with depending on the gd struct?

Perhaps the wording is wrong a bit off. It's just pulling in other stuff and as you know we are very space constrained.

Quoted 10 lines

nand_spl/board/freescale/p1010rdb/Makefile ifneq ($(OBJTREE), $(SRCTREE)) · line 130

Quoted 8 lines

nand_spl/board/freescale/p1010rdb/Makefile +3 -0

@@ -123,6 +124,9 @@ ifneq ($(OBJTREE), $(SRCTREE))
 $(obj)nand_boot.c:
      @rm -f $(obj)nand_boot.c
      ln -s $(SRCTREE)/nand_spl/board/$(BOARDDIR)/nand_boot.c $(obj)nand_boot.c
+$(obj)../common.c:
+     @rm -f $(obj)../common.c
+     ln -s $(SRCTREE)/nand_spl/board/freescale/common.c $(obj)../common.c
 endif

Why are you creating a link in the parent directory?

The typical build process picks out files needed for the build and symlinks them to nand_spl folder - or if building out of tree then it symlinks to the out of tree folder. This is true for all files in nand_spl as it currently exists not just this new file.

Quoted 11 lines · On 08/13/2012 06:14 PM, McClintock Matthew-B29882 wrote:

Quoted 10 lines · Scott Wood

Quoted 8 lines · Matthew McClintock

Let's use the more appropriate udelay for the nand_spl. While we can't make use of u-boot's full udelay we can atl east use a for loop that won't get optimized away .Since we have the bus clock we can use the timebase to calculate wall time.

Looked at reusing the u-boot udelay functions but it pulls in a lot of code as well as depends on the gd struct and would require a lot of rework

Quoted 4 lines

Quoted 1 line

What's wrong with depending on the gd struct?

Perhaps the wording is wrong a bit off. It's just pulling in other stuff and as you know we are very space constrained.

A struct definition doesn't take up space. Maybe you meant a dependency on certain specific code that puts things in the gd struct?

Quoted 11 lines

Quoted 10 lines

nand_spl/board/freescale/p1010rdb/Makefile ifneq ($(OBJTREE), $(SRCTREE)) · line 130

Quoted 8 lines

nand_spl/board/freescale/p1010rdb/Makefile +3 -0

@@ -123,6 +124,9 @@ ifneq ($(OBJTREE), $(SRCTREE))
 $(obj)nand_boot.c:
      @rm -f $(obj)nand_boot.c
      ln -s $(SRCTREE)/nand_spl/board/$(BOARDDIR)/nand_boot.c $(obj)nand_boot.c
+$(obj)../common.c:
+     @rm -f $(obj)../common.c
+     ln -s $(SRCTREE)/nand_spl/board/freescale/common.c $(obj)../common.c
 endif

Why are you creating a link in the parent directory?

Quoted 4 lines

The typical build process picks out files needed for the build and symlinks them to nand_spl folder - or if building out of tree then it symlinks to the out of tree folder. This is true for all files in nand_spl as it currently exists not just this new file.

This is the first time I've seen a link go in $(obj)../ rather than $(obj)

Quoted 12 lines · Scott Wood

Quoted 11 lines · On 08/13/2012 06:14 PM, McClintock Matthew-B29882 wrote:

Quoted 10 lines · Scott Wood

Quoted 8 lines · Matthew McClintock

Let's use the more appropriate udelay for the nand_spl. While we can't make use of u-boot's full udelay we can atl east use a for loop that won't get optimized away .Since we have the bus clock we can use the timebase to calculate wall time.

Looked at reusing the u-boot udelay functions but it pulls in a lot of code as well as depends on the gd struct and would require a lot of rework

Quoted 7 lines

Quoted 4 lines

Quoted 1 line

What's wrong with depending on the gd struct?

Perhaps the wording is wrong a bit off. It's just pulling in other stuff and as you know we are very space constrained.

A struct definition doesn't take up space. Maybe you meant a dependency on certain specific code that puts things in the gd struct?

Correct.

Quoted 16 lines

Quoted 15 lines

Quoted 10 lines

nand_spl/board/freescale/p1010rdb/Makefile ifneq ($(OBJTREE), $(SRCTREE)) · line 130

Quoted 8 lines

nand_spl/board/freescale/p1010rdb/Makefile +3 -0

@@ -123,6 +124,9 @@ ifneq ($(OBJTREE), $(SRCTREE))
 $(obj)nand_boot.c:
      @rm -f $(obj)nand_boot.c
      ln -s $(SRCTREE)/nand_spl/board/$(BOARDDIR)/nand_boot.c $(obj)nand_boot.c
+$(obj)../common.c:
+     @rm -f $(obj)../common.c
+     ln -s $(SRCTREE)/nand_spl/board/freescale/common.c $(obj)../common.c
 endif

Why are you creating a link in the parent directory?

The typical build process picks out files needed for the build and symlinks them to nand_spl folder - or if building out of tree then it symlinks to the out of tree folder. This is true for all files in nand_spl as it currently exists not just this new file.

Quoted 1 line

This is the first time I've seen a link go in $(obj)../ rather than $(obj)

It's because that's where the includes reference it. When building in tree, it does nothing because it can just reference the C file in the parent directory. When it's out of tree - we need to install the symlink in the proper location in the build folder.

We have a requirement to wait a period of time before enabling the DDR controller

Signed-off-by Matthew McClintock <[email protected]>

---
 nand_spl/board/freescale/p1023rds/Makefile    |    6 +++++-
 nand_spl/board/freescale/p1023rds/nand_boot.c |   14 ++++++++++++--
 2 files changed, 17 insertions(+), 3 deletions(-)

nand_spl/board/freescale/p1023rds/Makefile +5 -1

index 168e868..da43521 100644
@@ -34,7 +34,8 @@ CFLAGS	+= -DCONFIG_NAND_SPL
 
 SOBJS	= start.o resetvec.o
 COBJS	= cache.o cpu_init_early.o cpu_init_nand.o fsl_law.o law.o \
-	  nand_boot.o nand_boot_fsl_elbc.o ns16550.o tlb.o tlb_table.o
+	  nand_boot.o nand_boot_fsl_elbc.o ns16550.o tlb.o tlb_table.o \
+	  ../common.o
 
 SRCS	:= $(addprefix $(obj),$(SOBJS:.o=.S) $(COBJS:.o=.c))
 OBJS	:= $(addprefix $(obj),$(SOBJS) $(COBJS))

@@ -114,6 +115,9 @@ ifneq ($(OBJTREE), $(SRCTREE))
 $(obj)nand_boot.c:
 	@rm -f $(obj)nand_boot.c
 	ln -s $(SRCTREE)/nand_spl/board/$(BOARDDIR)/nand_boot.c $(obj)nand_boot.c
+$(obj)../common.c:
+	@rm -f $(obj)../common.c
+	ln -s $(SRCTREE)/nand_spl/board/freescale/common.c $(obj)../common.c
 endif
 
 #########################################################################

nand_spl/board/freescale/p1023rds/nand_boot.c +9 -1

index 0065c87..9309936 100644
@@ -25,6 +25,7 @@
 #include <asm/io.h>
 #include <nand.h>
 #include <asm/fsl_law.h>
+#include <asm/fsl_ddr_sdram.h>
 
 /* Fixed sdram init -- doesn't use serial presence detect. */
 void sdram_init(void)

@@ -53,12 +54,21 @@ void sdram_init(void)
 	out_be32(&ddr->ddr_wrlvl_cntl, CONFIG_SYS_DDR_WRLVL_CNTL);
 	out_be32(&ddr->ddr_cdr1, CONFIG_SYS_DDR_CDR_1);
 	out_be32(&ddr->ddr_cdr2, CONFIG_SYS_DDR_CDR_2);
-	out_be32(&ddr->sdram_cfg, CONFIG_SYS_DDR_CONTROL);
+	/* Set, but do not enable the memory */
+	out_be32(&ddr->sdram_cfg, CONFIG_SYS_DDR_CONTROL & ~SDRAM_CFG_MEM_EN);
+
+	asm volatile("sync;isync");
+	udelay(500);
+
+	/* Let the controller go */
+	out_be32(&ddr->sdram_cfg, in_be32(&ddr->sdram_cfg) | SDRAM_CFG_MEM_EN);
 }

Quoted 60 lines · Matthew McClintock

We have a requirement to wait a period of time before enabling the DDR controller

Signed-off-by Matthew McClintock <[email protected]>

---
 nand_spl/board/freescale/p1023rds/Makefile    |    6 +++++-
 nand_spl/board/freescale/p1023rds/nand_boot.c |   14 ++++++++++++--
 2 files changed, 17 insertions(+), 3 deletions(-)

nand_spl/board/freescale/p1023rds/Makefile +5 -1

index 168e868..da43521 100644
@@ -34,7 +34,8 @@ CFLAGS      += -DCONFIG_NAND_SPL
 
 SOBJS        = start.o resetvec.o
 COBJS        = cache.o cpu_init_early.o cpu_init_nand.o fsl_law.o law.o \
-       nand_boot.o nand_boot_fsl_elbc.o ns16550.o tlb.o tlb_table.o
+       nand_boot.o nand_boot_fsl_elbc.o ns16550.o tlb.o tlb_table.o \
+       ../common.o
 
 SRCS := $(addprefix $(obj),$(SOBJS:.o=.S) $(COBJS:.o=.c))
 OBJS := $(addprefix $(obj),$(SOBJS) $(COBJS))

@@ -114,6 +115,9 @@ ifneq ($(OBJTREE), $(SRCTREE))
 $(obj)nand_boot.c:
      @rm -f $(obj)nand_boot.c
      ln -s $(SRCTREE)/nand_spl/board/$(BOARDDIR)/nand_boot.c $(obj)nand_boot.c
+$(obj)../common.c:
+     @rm -f $(obj)../common.c
+     ln -s $(SRCTREE)/nand_spl/board/freescale/common.c $(obj)../common.c
 endif
 
 #########################################################################

nand_spl/board/freescale/p1023rds/nand_boot.c +9 -1

index 0065c87..9309936 100644
@@ -25,6 +25,7 @@
 #include <asm/io.h>
 #include <nand.h>
 #include <asm/fsl_law.h>
+#include <asm/fsl_ddr_sdram.h>
 
 /* Fixed sdram init -- doesn't use serial presence detect. */
 void sdram_init(void)

@@ -53,12 +54,21 @@ void sdram_init(void)
      out_be32(&ddr->ddr_wrlvl_cntl, CONFIG_SYS_DDR_WRLVL_CNTL);
      out_be32(&ddr->ddr_cdr1, CONFIG_SYS_DDR_CDR_1);
      out_be32(&ddr->ddr_cdr2, CONFIG_SYS_DDR_CDR_2);
-     out_be32(&ddr->sdram_cfg, CONFIG_SYS_DDR_CONTROL);
+     /* Set, but do not enable the memory */
+     out_be32(&ddr->sdram_cfg, CONFIG_SYS_DDR_CONTROL & ~SDRAM_CFG_MEM_EN);
+
+     asm volatile("sync;isync");
+     udelay(500);
+
+     /* Let the controller go */
+     out_be32(&ddr->sdram_cfg, in_be32(&ddr->sdram_cfg) | SDRAM_CFG_MEM_EN);
 }

nand_spl/board/freescale/p1023rds/nand_boot.c void sdram_init(void) · line 74

Quoted 10 lines

nand_spl/board/freescale/p1023rds/nand_boot.c +3 -1

 
+u32 bus_clk;
+
 void board_init_f(ulong bootflag)
 {
-     u32 plat_ratio, bus_clk;
+     u32 plat_ratio;
      ccsr_gur_t *gur = (void *)CONFIG_SYS_MPC85xx_GUTS_ADDR;
 
      /* initialize selected port with appropriate baud rate */

This is before relocation, so you can't use global variables. Use gd->bus_clk.

In our SDK where this code has already been applied, I spent some debug time tracking why the first word of the SPL was getting corrupted (the BSS goes right after the end of the NAND buffer and it wraps around), when trying to find the cause of other corruption (the rest was bad DDR settings).

This change reduces the SPL size by removing the redundant syncs produced by out_be32 and just replies on one final sync

Done with:

sed -r '/in_be32/b; s/(out_be32)\(([^,]*),\s+(.*)\)/__raw_writel(\3, \2)/g' -i git grep --name-only sdram_init nand_spl/

Signed-off-by Matthew McClintock <[email protected]>

---
 nand_spl/board/freescale/p1010rdb/nand_boot.c     |   54 ++++++++++-----------
 nand_spl/board/freescale/p1023rds/nand_boot.c     |   42 ++++++++--------
 nand_spl/board/freescale/p1_p2_rdb_pc/nand_boot.c |   48 +++++++++---------
 3 files changed, 71 insertions(+), 73 deletions(-)

Quoted 5 lines · Matthew McClintock

This change reduces the SPL size by removing the redundant syncs produced by out_be32 and just replies on one final sync

Done with:

Quoted 8 lines

sed -r '/in_be32/b; s/(out_be32)\(([^,]*),\s+(.*)\)/__raw_writel(\3, \2)/g' -i git grep --name-only sdram_init nand_spl/

Signed-off-by Matthew McClintock <[email protected]>

---
 nand_spl/board/freescale/p1010rdb/nand_boot.c     |   54 ++++++++++-----------
 nand_spl/board/freescale/p1023rds/nand_boot.c     |   42 ++++++++--------
 nand_spl/board/freescale/p1_p2_rdb_pc/nand_boot.c |   48 +++++++++---------
 3 files changed, 71 insertions(+), 73 deletions(-)

This should come first if the other patches break without it, to preserve bisectability.

Note that I'm going to try to convert this stuff (at least one board as an example, but hopefully it should be easy enough to do additional boards once the first is done) to the new spl Really Soon Now(tm), so it doesn't make much sense to fiddle around with the old stuff right now unless I miss the merge window. I'll incorporate these changes into the new-spl version. I may do that by applying these patches first, but I'd rather they not go via the mpc85xx tree (and please CC me on NAND patches).

-Scott

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Quoted 15 lines · Scott Wood

Quoted 13 lines · Matthew McClintock

This change reduces the SPL size by removing the redundant syncs produced by out_be32 and just replies on one final sync

Done with:

sed -r '/in_be32/b; s/(out_be32)\(([^,]*),\s+(.*)\)/__raw_writel(\3, \2)/g' -i git grep --name-only sdram_init nand_spl/

Signed-off-by Matthew McClintock <[email protected]>

---
 nand_spl/board/freescale/p1010rdb/nand_boot.c     |   54 ++++++++++-----------
 nand_spl/board/freescale/p1023rds/nand_boot.c     |   42 ++++++++--------
 nand_spl/board/freescale/p1_p2_rdb_pc/nand_boot.c |   48 +++++++++---------
 3 files changed, 71 insertions(+), 73 deletions(-)

Quoted 2 lines

This should come first if the other patches break without it, to preserve bisectability.

I think they all work on at least some compilers.... not that that's extremely helpful.

Quoted 16 lines · On 08/13/2012 06:31 PM, McClintock Matthew-B29882 wrote:

Quoted 15 lines · Scott Wood

Quoted 13 lines · Matthew McClintock

This change reduces the SPL size by removing the redundant syncs produced by out_be32 and just replies on one final sync

Done with:

sed -r '/in_be32/b; s/(out_be32)\(([^,]*),\s+(.*)\)/__raw_writel(\3, \2)/g' -i git grep --name-only sdram_init nand_spl/

Signed-off-by Matthew McClintock <[email protected]>

---
 nand_spl/board/freescale/p1010rdb/nand_boot.c     |   54 ++++++++++-----------
 nand_spl/board/freescale/p1023rds/nand_boot.c     |   42 ++++++++--------
 nand_spl/board/freescale/p1_p2_rdb_pc/nand_boot.c |   48 +++++++++---------
 3 files changed, 71 insertions(+), 73 deletions(-)

Quoted 5 lines

Quoted 2 lines

This should come first if the other patches break without it, to preserve bisectability.

I think they all work on at least some compilers.... not that that's extremely helpful.

Was this patch meant to free up room for the other patches, or was it just bundled in because it's touching the same code?

Quoted 5 lines · Matthew McClintock

This change reduces the SPL size by removing the redundant syncs produced by out_be32 and just replies on one final sync

Done with:

Quoted 8 lines

sed -r '/in_be32/b; s/(out_be32)\(([^,]*),\s+(.*)\)/__raw_writel(\3, \2)/g' -i git grep --name-only sdram_init nand_spl/

Signed-off-by Matthew McClintock <[email protected]>

---
 nand_spl/board/freescale/p1010rdb/nand_boot.c     |   54 ++++++++++-----------
 nand_spl/board/freescale/p1023rds/nand_boot.c     |   42 ++++++++--------
 nand_spl/board/freescale/p1_p2_rdb_pc/nand_boot.c |   48 +++++++++---------
 3 files changed, 71 insertions(+), 73 deletions(-)

This should come first if the other patches break without it, to preserve bisectability.

Note that I'm going to try to convert this stuff (at least one board as an example, but hopefully it should be easy enough to do additional boards once the first is done) to the new spl Really Soon Now(tm), so it doesn't make much sense to fiddle around with the old stuff right now unless I miss the merge window. I'll incorporate these changes into the new-spl version. I may do that by applying these patches first, but I'd rather they not go via the mpc85xx tree (and please CC me on NAND patches).

Quoted 18 lines · Scott Wood

Quoted 5 lines · Matthew McClintock

This change reduces the SPL size by removing the redundant syncs produced by out_be32 and just replies on one final sync

Done with:

Quoted 8 lines

sed -r '/in_be32/b; s/(out_be32)\(([^,]*),\s+(.*)\)/__raw_writel(\3, \2)/g' -i git grep --name-only sdram_init nand_spl/

Signed-off-by Matthew McClintock <[email protected]>

---

nand_spl/board/freescale/p1010rdb/nand_boot.c     |   54 ++++++++++-----------
nand_spl/board/freescale/p1023rds/nand_boot.c     |   42 ++++++++--------
nand_spl/board/freescale/p1_p2_rdb_pc/nand_boot.c |   48 +++++++++---------
3 files changed, 71 insertions(+), 73 deletions(-)

This should come first if the other patches break without it, to preserve bisectability.

Quoted 7 lines

Note that I'm going to try to convert this stuff (at least one board as an example, but hopefully it should be easy enough to do additional boards once the first is done) to the new spl Really Soon Now(tm), so it doesn't make much sense to fiddle around with the old stuff right now unless I miss the merge window. I'll incorporate these changes into the new-spl version. I may do that by applying these patches first, but I'd rather they not go via the mpc85xx tree (and please CC me on NAND patches).

I'm not going to have this working by the end of the merge window, so these patches can go in as is. Andy, do you want to take them or should I?

-Scott

I'm ok if you take them. I should be doing a push today, so either way works for me.