All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH] Streamlined mpc512x fixed_sdram init sequence.
@ 2009-09-21 18:07 Martha M Stan
  2009-09-24 22:36 ` Wolfgang Denk
  0 siblings, 1 reply; 3+ messages in thread
From: Martha M Stan @ 2009-09-21 18:07 UTC (permalink / raw)
  To: u-boot

Signed-off-by: Martha M Stan <mmarx@silicontkx.com>
---
 board/davedenx/aria/aria.c              |    2 +-
 board/esd/mecp5123/mecp5123.c           |    2 +-
 board/freescale/mpc5121ads/mpc5121ads.c |    2 +-
 cpu/mpc512x/fixed_sdram.c               |  104 ++++++++++++++++++++----------
 include/asm-ppc/immap_512x.h            |    4 +
 include/asm-ppc/mpc512x.h               |    2 +-
 include/configs/aria.h                  |   22 +++----
 include/configs/mecp5123.h              |   23 +++----
 include/configs/mpc5121ads.h            |   30 ++++-----
 9 files changed, 109 insertions(+), 82 deletions(-)

diff --git a/board/davedenx/aria/aria.c b/board/davedenx/aria/aria.c
index 2064aa2..cc69c9d 100644
--- a/board/davedenx/aria/aria.c
+++ b/board/davedenx/aria/aria.c
@@ -101,7 +101,7 @@ int board_early_init_f(void)
 
 phys_size_t initdram (int board_type)
 {
-	return fixed_sdram();
+	return fixed_sdram(NULL, NULL, 0);
 }
 
 int misc_init_r(void)
diff --git a/board/esd/mecp5123/mecp5123.c b/board/esd/mecp5123/mecp5123.c
index f591e32..5139358 100644
--- a/board/esd/mecp5123/mecp5123.c
+++ b/board/esd/mecp5123/mecp5123.c
@@ -135,7 +135,7 @@ int board_early_init_f(void)
 
 phys_size_t initdram(int board_type)
 {
-	return get_ram_size(0, fixed_sdram());
+	return get_ram_size(0, fixed_sdram(NULL, NULL, 0));
 }
 
 int misc_init_r(void)
diff --git a/board/freescale/mpc5121ads/mpc5121ads.c b/board/freescale/mpc5121ads/mpc5121ads.c
index a0d7a82..8defb00 100644
--- a/board/freescale/mpc5121ads/mpc5121ads.c
+++ b/board/freescale/mpc5121ads/mpc5121ads.c
@@ -137,7 +137,7 @@ phys_size_t initdram(int board_type)
 {
 	u32 msize = 0;
 
-	msize = fixed_sdram();
+	msize = fixed_sdram(NULL, NULL, 0);
 
 	return msize;
 }
diff --git a/cpu/mpc512x/fixed_sdram.c b/cpu/mpc512x/fixed_sdram.c
index d906903..63a3035 100644
--- a/cpu/mpc512x/fixed_sdram.c
+++ b/cpu/mpc512x/fixed_sdram.c
@@ -25,18 +25,70 @@
 #include <asm/io.h>
 #include <asm/mpc512x.h>
 
+/* 
+ * MDDRC Config Runtime Settings in order of the 4 MDDRC cfg registers 
+ */
+u32 default_mddrc_config[4] = {
+	CONFIG_SYS_MDDRC_SYS_CFG,
+	CONFIG_SYS_MDDRC_TIME_CFG0,
+	CONFIG_SYS_MDDRC_TIME_CFG1,
+	CONFIG_SYS_MDDRC_TIME_CFG2
+};
+
+u32 default_init_seq[] = {
+	CONFIG_SYS_DDRCMD_NOP,
+	CONFIG_SYS_DDRCMD_NOP,
+	CONFIG_SYS_DDRCMD_NOP,
+	CONFIG_SYS_DDRCMD_NOP,
+	CONFIG_SYS_DDRCMD_NOP,
+	CONFIG_SYS_DDRCMD_NOP,
+	CONFIG_SYS_DDRCMD_NOP,
+	CONFIG_SYS_DDRCMD_NOP,
+	CONFIG_SYS_DDRCMD_NOP,
+	CONFIG_SYS_DDRCMD_NOP,
+	CONFIG_SYS_DDRCMD_PCHG_ALL,
+	CONFIG_SYS_DDRCMD_NOP,
+	CONFIG_SYS_DDRCMD_RFSH,
+	CONFIG_SYS_DDRCMD_NOP,
+	CONFIG_SYS_DDRCMD_RFSH,
+	CONFIG_SYS_DDRCMD_NOP,
+	CONFIG_SYS_MICRON_INIT_DEV_OP,
+	CONFIG_SYS_DDRCMD_NOP,
+	CONFIG_SYS_DDRCMD_EM2,
+	CONFIG_SYS_DDRCMD_NOP,
+	CONFIG_SYS_DDRCMD_PCHG_ALL,
+	CONFIG_SYS_DDRCMD_EM2,
+	CONFIG_SYS_DDRCMD_EM3,
+	CONFIG_SYS_DDRCMD_EN_DLL,
+	CONFIG_SYS_MICRON_INIT_DEV_OP,
+	CONFIG_SYS_DDRCMD_PCHG_ALL,
+	CONFIG_SYS_DDRCMD_RFSH,
+	CONFIG_SYS_MICRON_INIT_DEV_OP,
+	CONFIG_SYS_DDRCMD_OCD_DEFAULT,
+	CONFIG_SYS_DDRCMD_PCHG_ALL,
+	CONFIG_SYS_DDRCMD_NOP
+};
+
 /*
  * fixed sdram init:
  * The board doesn't use memory modules that have serial presence
  * detect or similar mechanism for discovery of the DRAM settings
  */
-long int fixed_sdram(void)
+long int fixed_sdram(u32 *mddrc_config, u32 *dram_init_seq, int seq_sz)
 {
 	volatile immap_t *im = (immap_t *)CONFIG_SYS_IMMR;
 	u32 msize = CONFIG_SYS_DDR_SIZE * 1024 * 1024;
 	u32 msize_log2 = __ilog2(msize);
 	u32 i;
 
+	/* take default settings and init sequence if necessary */
+	if (mddrc_config == 0)
+		mddrc_config = default_mddrc_config;
+	if (dram_init_seq == 0) {
+		dram_init_seq = default_init_seq;
+		seq_sz = sizeof(default_init_seq)/sizeof(u32);
+	}
+
 	/* Initialize IO Control */
 	out_be32(&im->io_ctrl.io_control_mem, IOCTRL_MUX_DDR);
 
@@ -45,8 +97,8 @@ long int fixed_sdram(void)
 	out_be32(&im->sysconf.ddrlaw.ar, msize_log2 - 1);
 	sync_law(&im->sysconf.ddrlaw.ar);
 
-	/* Enable DDR */
-	out_be32(&im->mddrc.ddr_sys_config, CONFIG_SYS_MDDRC_SYS_CFG_EN);
+	/* DDR Enable */
+	out_be32(&im->mddrc.ddr_sys_config, MDDRC_SYS_CFG_EN);
 
 	/* Initialize DDR Priority Manager */
 	out_be32(&im->mddrc.prioman_config1, CONFIG_SYS_MDDRCGRP_PM_CFG1);
@@ -73,41 +125,23 @@ long int fixed_sdram(void)
 	out_be32(&im->mddrc.lut_table4_alternate_upper, CONFIG_SYS_MDDRCGRP_LUT4_AU);
 	out_be32(&im->mddrc.lut_table4_alternate_lower, CONFIG_SYS_MDDRCGRP_LUT4_AL);
 
-	/* Initialize MDDRC */
-	out_be32(&im->mddrc.ddr_sys_config, CONFIG_SYS_MDDRC_SYS_CFG);
-	out_be32(&im->mddrc.ddr_time_config0, CONFIG_SYS_MDDRC_TIME_CFG0);
-	out_be32(&im->mddrc.ddr_time_config1, CONFIG_SYS_MDDRC_TIME_CFG1);
-	out_be32(&im->mddrc.ddr_time_config2, CONFIG_SYS_MDDRC_TIME_CFG2);
-
-	/* Initialize DDR */
-	for (i = 0; i < 10; i++)
-		out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_NOP);
+	/* 
+	 * Initialize MDDRC
+	 *  put MDDRC in CMD mode and 
+	 *  set the max time between refreshes to 0 during init process 
+	 */
+	out_be32(&im->mddrc.ddr_sys_config, mddrc_config[0] | MDDRC_SYS_CFG_CMD_MASK);
+	out_be32(&im->mddrc.ddr_time_config0, mddrc_config[1] & MDDRC_REFRESH_ZERO_MASK);
+	out_be32(&im->mddrc.ddr_time_config1, mddrc_config[2]);
+	out_be32(&im->mddrc.ddr_time_config2, mddrc_config[3]);
 
-	out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_PCHG_ALL);
-	out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_NOP);
-	out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_RFSH);
-	out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_NOP);
-	out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_RFSH);
-	out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_NOP);
-	out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_INIT_DEV_OP);
-	out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_NOP);
-	out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_EM2);
-	out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_NOP);
-	out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_PCHG_ALL);
-	out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_EM2);
-	out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_EM3);
-	out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_EN_DLL);
-	out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_INIT_DEV_OP);
-	out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_PCHG_ALL);
-	out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_RFSH);
-	out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_INIT_DEV_OP);
-	out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_OCD_DEFAULT);
-	out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_PCHG_ALL);
-	out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_NOP);
+	/* Initialize DDR with either default or supplied init sequence */
+	for (i = 0; i < seq_sz; i++)
+		out_be32(&im->mddrc.ddr_command, dram_init_seq[i]);
 
 	/* Start MDDRC */
-	out_be32(&im->mddrc.ddr_time_config0, CONFIG_SYS_MDDRC_TIME_CFG0_RUN);
-	out_be32(&im->mddrc.ddr_sys_config, CONFIG_SYS_MDDRC_SYS_CFG_RUN);
+	out_be32(&im->mddrc.ddr_time_config0, mddrc_config[1]);
+	out_be32(&im->mddrc.ddr_sys_config, mddrc_config[0]);
 
 	return msize;
 }
diff --git a/include/asm-ppc/immap_512x.h b/include/asm-ppc/immap_512x.h
index 24e6c69..79cdd80 100644
--- a/include/asm-ppc/immap_512x.h
+++ b/include/asm-ppc/immap_512x.h
@@ -341,6 +341,10 @@ typedef struct ddr512x {
 	u32 res2[0x3AD];
 } ddr512x_t;
 
+/* MDDRC SYS CFG and Timing CFG0 Registers */
+#define MDDRC_SYS_CFG_EN	0xF0000000
+#define MDDRC_SYS_CFG_CMD_MASK	0x10000000
+#define MDDRC_REFRESH_ZERO_MASK	0x0000FFFF
 
 /*
  * DMA/Messaging Unit
diff --git a/include/asm-ppc/mpc512x.h b/include/asm-ppc/mpc512x.h
index 20456f5..8ef0d9c 100644
--- a/include/asm-ppc/mpc512x.h
+++ b/include/asm-ppc/mpc512x.h
@@ -50,7 +50,7 @@ static inline void sync_law(volatile void *addr)
 /*
  * Prototypes
  */
-extern long int fixed_sdram(void);
+extern long int fixed_sdram(u32 *mddrc_config, u32 *dram_init_seq, int seq_sz);
 extern int mpc5121_diu_init(void);
 extern void ide_set_reset(int idereset);
 
diff --git a/include/configs/aria.h b/include/configs/aria.h
index 4211113..2938eac 100644
--- a/include/configs/aria.h
+++ b/include/configs/aria.h
@@ -126,7 +126,7 @@
 #define CONFIG_SYS_MDDRC_SYS_CFG     (	(1 << 31) |	/* RST_B */ \
 					(1 << 30) |	/* CKE */ \
 					(1 << 29) |	/* CLK_ON */ \
-					(1 << 28) |	/* CMD_MODE */ \
+					(0 << 28) |	/* CMD_MODE */ \
 					(4 << 25) |	/* DRAM_ROW_SELECT */ \
 					(3 << 21) |	/* DRAM_BANK_SELECT */ \
 					(0 << 18) |	/* SELF_REF_EN */ \
@@ -143,16 +143,12 @@
 					(0 <<  0) 	/* FIFO_UV_EN */ \
 				     )
 
-#define CONFIG_SYS_MDDRC_SYS_CFG_RUN	(CONFIG_SYS_MDDRC_SYS_CFG & ~(1 << 28))
+#define CONFIG_SYS_MDDRC_TIME_CFG0	0x030C3D2E
 #define CONFIG_SYS_MDDRC_TIME_CFG1	0x55D81189
 #define CONFIG_SYS_MDDRC_TIME_CFG2	0x34790863
 
-#define CONFIG_SYS_MDDRC_SYS_CFG_EN	0xF0000000
-#define CONFIG_SYS_MDDRC_TIME_CFG0	0x00003D2E
-#define CONFIG_SYS_MDDRC_TIME_CFG0_RUN	0x030C3D2E
-
-#define CONFIG_SYS_MICRON_NOP		0x01380000
-#define CONFIG_SYS_MICRON_PCHG_ALL	0x01100400
+#define CONFIG_SYS_DDRCMD_NOP		0x01380000
+#define CONFIG_SYS_DDRCMD_PCHG_ALL	0x01100400
 #define CONFIG_SYS_MICRON_EMR	     (	(1 << 24) |	/* CMD_REQ */ \
 					(0 << 22) |	/* DRAM_CS */ \
 					(0 << 21) |	/* DRAM_RAS */ \
@@ -172,7 +168,7 @@
 				     )
 #define CONFIG_SYS_MICRON_EMR2		0x01020000
 #define CONFIG_SYS_MICRON_EMR3		0x01030000
-#define CONFIG_SYS_MICRON_RFSH		0x01080000
+#define CONFIG_SYS_DDRCMD_RFSH		0x01080000
 #define CONFIG_SYS_MICRON_INIT_DEV_OP	0x01000432
 #define CONFIG_SYS_MICRON_EMR_OCD    (	(1 << 24) |	/* CMD_REQ */ \
 					(0 << 22) |	/* DRAM_CS */ \
@@ -196,10 +192,10 @@
  * Backward compatible definitions,
  * so we do not have to change cpu/mpc512x/fixed_sdram.c
  */
-#define	CONFIG_SYS_MICRON_EM2		(CONFIG_SYS_MICRON_EMR2)
-#define CONFIG_SYS_MICRON_EM3		(CONFIG_SYS_MICRON_EMR3)
-#define CONFIG_SYS_MICRON_EN_DLL	(CONFIG_SYS_MICRON_EMR)
-#define CONFIG_SYS_MICRON_OCD_DEFAULT	(CONFIG_SYS_MICRON_EMR_OCD)
+#define	CONFIG_SYS_DDRCMD_EM2		(CONFIG_SYS_MICRON_EMR2)
+#define CONFIG_SYS_DDRCMD_EM3		(CONFIG_SYS_MICRON_EMR3)
+#define CONFIG_SYS_DDRCMD_EN_DLL	(CONFIG_SYS_MICRON_EMR)
+#define CONFIG_SYS_DDRCMD_OCD_DEFAULT	(CONFIG_SYS_MICRON_EMR_OCD)
 
 /* DDR Priority Manager Configuration */
 #define CONFIG_SYS_MDDRCGRP_PM_CFG1	0x00077777
diff --git a/include/configs/mecp5123.h b/include/configs/mecp5123.h
index 1ecae00..e194c8f 100644
--- a/include/configs/mecp5123.h
+++ b/include/configs/mecp5123.h
@@ -111,22 +111,19 @@
  *	[09:05]	DRAM tRP:
  *	[04:00] DRAM tRPA
  */
-#define CONFIG_SYS_MDDRC_SYS_CFG	 0xFA804A00
-#define CONFIG_SYS_MDDRC_SYS_CFG_RUN	 0xEA804A00
+#define CONFIG_SYS_MDDRC_SYS_CFG	 0xEA804A00
+#define CONFIG_SYS_MDDRC_TIME_CFG0	 0x06183D2E
 #define CONFIG_SYS_MDDRC_TIME_CFG1	 0x68EC1168
 #define CONFIG_SYS_MDDRC_TIME_CFG2	 0x34310864
-#define CONFIG_SYS_MDDRC_SYS_CFG_EN	0xF0000000
-#define CONFIG_SYS_MDDRC_TIME_CFG0	0x00003D2E
-#define CONFIG_SYS_MDDRC_TIME_CFG0_RUN	0x06183D2E
-
-#define CONFIG_SYS_MICRON_NOP		0x01380000
-#define CONFIG_SYS_MICRON_PCHG_ALL	0x01100400
-#define CONFIG_SYS_MICRON_EM2		0x01020000
-#define CONFIG_SYS_MICRON_EM3		0x01030000
-#define CONFIG_SYS_MICRON_EN_DLL	0x01010000
-#define CONFIG_SYS_MICRON_RFSH		0x01080000
+
+#define CONFIG_SYS_DDRCMD_NOP		0x01380000
+#define CONFIG_SYS_DDRCMD_PCHG_ALL	0x01100400
+#define CONFIG_SYS_DDRCMD_EM2		0x01020000
+#define CONFIG_SYS_DDRCMD_EM3		0x01030000
+#define CONFIG_SYS_DDRCMD_EN_DLL	0x01010000
+#define CONFIG_SYS_DDRCMD_RFSH		0x01080000
 #define CONFIG_SYS_MICRON_INIT_DEV_OP	0x01000432
-#define CONFIG_SYS_MICRON_OCD_DEFAULT	0x01010780
+#define CONFIG_SYS_DDRCMD_OCD_DEFAULT	0x01010780
 
 /* DDR Priority Manager Configuration */
 #define CONFIG_SYS_MDDRCGRP_PM_CFG1	0x00077777
diff --git a/include/configs/mpc5121ads.h b/include/configs/mpc5121ads.h
index 76f174d..0c871c9 100644
--- a/include/configs/mpc5121ads.h
+++ b/include/configs/mpc5121ads.h
@@ -131,28 +131,24 @@
  *	[04:00] DRAM tRPA
  */
 #ifdef CONFIG_MPC5121ADS_REV2
-#define CONFIG_SYS_MDDRC_SYS_CFG	0xF8604A00
-#define CONFIG_SYS_MDDRC_SYS_CFG_RUN	0xE8604A00
+#define CONFIG_SYS_MDDRC_SYS_CFG	0xE8604A00
 #define CONFIG_SYS_MDDRC_TIME_CFG1	0x54EC1168
 #define CONFIG_SYS_MDDRC_TIME_CFG2	0x35210864
 #else
-#define CONFIG_SYS_MDDRC_SYS_CFG	 0xFA804A00
-#define CONFIG_SYS_MDDRC_SYS_CFG_RUN	 0xEA804A00
-#define CONFIG_SYS_MDDRC_TIME_CFG1	 0x68EC1168
-#define CONFIG_SYS_MDDRC_TIME_CFG2	 0x34310864
+#define CONFIG_SYS_MDDRC_SYS_CFG	0xEA804A00
+#define CONFIG_SYS_MDDRC_TIME_CFG1	0x68EC1168
+#define CONFIG_SYS_MDDRC_TIME_CFG2	0x34310864
 #endif
-#define CONFIG_SYS_MDDRC_SYS_CFG_EN	0xF0000000
-#define CONFIG_SYS_MDDRC_TIME_CFG0	0x00003D2E
-#define CONFIG_SYS_MDDRC_TIME_CFG0_RUN	0x06183D2E
-
-#define CONFIG_SYS_MICRON_NOP		0x01380000
-#define CONFIG_SYS_MICRON_PCHG_ALL	0x01100400
-#define CONFIG_SYS_MICRON_EM2		0x01020000
-#define CONFIG_SYS_MICRON_EM3		0x01030000
-#define CONFIG_SYS_MICRON_EN_DLL	0x01010000
-#define CONFIG_SYS_MICRON_RFSH		0x01080000
+#define CONFIG_SYS_MDDRC_TIME_CFG0	0x06183D2E
+
+#define CONFIG_SYS_DDRCMD_NOP		0x01380000
+#define CONFIG_SYS_DDRCMD_PCHG_ALL	0x01100400
+#define CONFIG_SYS_DDRCMD_EM2		0x01020000
+#define CONFIG_SYS_DDRCMD_EM3		0x01030000
+#define CONFIG_SYS_DDRCMD_EN_DLL	0x01010000
+#define CONFIG_SYS_DDRCMD_RFSH		0x01080000
 #define CONFIG_SYS_MICRON_INIT_DEV_OP	0x01000432
-#define CONFIG_SYS_MICRON_OCD_DEFAULT	0x01010780
+#define CONFIG_SYS_DDRCMD_OCD_DEFAULT	0x01010780
 
 /* DDR Priority Manager Configuration */
 #define CONFIG_SYS_MDDRCGRP_PM_CFG1	0x00077777
-- 
1.5.2.4

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* [U-Boot] [PATCH] Streamlined mpc512x fixed_sdram init sequence.
  2009-09-21 18:07 [U-Boot] [PATCH] Streamlined mpc512x fixed_sdram init sequence Martha M Stan
@ 2009-09-24 22:36 ` Wolfgang Denk
  0 siblings, 0 replies; 3+ messages in thread
From: Wolfgang Denk @ 2009-09-24 22:36 UTC (permalink / raw)
  To: u-boot

Dear Martha M Stan,

In message <12535564342520-git-send-email-mmarx@silicontkx.com> you wrote:
> Signed-off-by: Martha M Stan <mmarx@silicontkx.com>
> ---
>  board/davedenx/aria/aria.c              |    2 +-
>  board/esd/mecp5123/mecp5123.c           |    2 +-
>  board/freescale/mpc5121ads/mpc5121ads.c |    2 +-
>  cpu/mpc512x/fixed_sdram.c               |  104 ++++++++++++++++++++----------
>  include/asm-ppc/immap_512x.h            |    4 +
>  include/asm-ppc/mpc512x.h               |    2 +-
>  include/configs/aria.h                  |   22 +++----
>  include/configs/mecp5123.h              |   23 +++----
>  include/configs/mpc5121ads.h            |   30 ++++-----
>  9 files changed, 109 insertions(+), 82 deletions(-)


Applying: Streamlined mpc512x fixed_sdram init sequence.
/home/wd/git/u-boot/work/.git/rebase-apply/patch:60: trailing whitespace.
/* 
/home/wd/git/u-boot/work/.git/rebase-apply/patch:61: trailing whitespace.
 * MDDRC Config Runtime Settings in order of the 4 MDDRC cfg registers 
/home/wd/git/u-boot/work/.git/rebase-apply/patch:152: trailing whitespace.
        /* 
/home/wd/git/u-boot/work/.git/rebase-apply/patch:154: trailing whitespace.
         *  put MDDRC in CMD mode and 
/home/wd/git/u-boot/work/.git/rebase-apply/patch:155: trailing whitespace.
         *  set the max time between refreshes to 0 during init process 
warning: 5 lines applied after fixing whitespace errors.


Please pay more attention to Coding Style!


> diff --git a/cpu/mpc512x/fixed_sdram.c b/cpu/mpc512x/fixed_sdram.c
> index d906903..63a3035 100644
> --- a/cpu/mpc512x/fixed_sdram.c
> +++ b/cpu/mpc512x/fixed_sdram.c
> @@ -25,18 +25,70 @@
>  #include <asm/io.h>
>  #include <asm/mpc512x.h>
>  
> +/* 
> + * MDDRC Config Runtime Settings in order of the 4 MDDRC cfg registers 
> + */
> +u32 default_mddrc_config[4] = {
> +	CONFIG_SYS_MDDRC_SYS_CFG,
> +	CONFIG_SYS_MDDRC_TIME_CFG0,
> +	CONFIG_SYS_MDDRC_TIME_CFG1,
> +	CONFIG_SYS_MDDRC_TIME_CFG2
> +};

Hm... we should move the first entry down... see below...


> -long int fixed_sdram(void)
> +long int fixed_sdram(u32 *mddrc_config, u32 *dram_init_seq, int seq_sz)
>  {
>  	volatile immap_t *im = (immap_t *)CONFIG_SYS_IMMR;
>  	u32 msize = CONFIG_SYS_DDR_SIZE * 1024 * 1024;
>  	u32 msize_log2 = __ilog2(msize);
>  	u32 i;
>  
> +	/* take default settings and init sequence if necessary */
> +	if (mddrc_config == 0)
> +		mddrc_config = default_mddrc_config;

Please write NULL when you mean a NULL pointer.

> +	if (dram_init_seq == 0) {
> +		dram_init_seq = default_init_seq;
> +		seq_sz = sizeof(default_init_seq)/sizeof(u32);
> +	}

Ditto.

> +	/* 
> +	 * Initialize MDDRC
> +	 *  put MDDRC in CMD mode and 
> +	 *  set the max time between refreshes to 0 during init process 
> +	 */
> +	out_be32(&im->mddrc.ddr_sys_config, mddrc_config[0] | MDDRC_SYS_CFG_CMD_MASK);
> +	out_be32(&im->mddrc.ddr_time_config0, mddrc_config[1] & MDDRC_REFRESH_ZERO_MASK);
> +	out_be32(&im->mddrc.ddr_time_config1, mddrc_config[2]);
> +	out_be32(&im->mddrc.ddr_time_config2, mddrc_config[3]);

I cannot help it, but every time I see this I think the code is
wrong. Guess I have seen too many copy & paste errors in this style.
When I see "...time_config2" I also want to see "mddrc_config[2]", i. e.
identical indices. We should reorder the struct.


To make some progress, I made the changes above, and applied the patch.

Thanks.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"The one charm of marriage is that it makes a  life  of  deception  a
neccessity."                                            - Oscar Wilde

^ permalink raw reply	[flat|nested] 3+ messages in thread

* [U-Boot] [PATCH] Streamlined mpc512x fixed_sdram init sequence.
@ 2009-09-25  0:45 m marx
  0 siblings, 0 replies; 3+ messages in thread
From: m marx @ 2009-09-25  0:45 UTC (permalink / raw)
  To: u-boot

Yes Wolfgang,

Sorry about the sloppy little problems with my 2 patches.  I need to pay
more attention to coding style ...

As far as the weirdness in array index vs immap struct naming mismatch ...

>  +u32 default_mddrc_config[4] = {
>  +CONFIG_SYS_MDDRC_SYS_CFG,
>  +CONFIG_SYS_MDDRC_TIME_CFG0,
>  +CONFIG_SYS_MDDRC_TIME_CFG1,
>  +CONFIG_SYS_MDDRC_TIME_CFG2
 
>  +/* 
>  + * Initialize MDDRC
>  + * put MDDRC in CMD mode and 
>  + * set the max time between refreshes to 0 during init process 
>  + */
>  +out_be32(mddrc.ddr_sys_config, mddrc_config[0] | MDDRC_SYS_CFG_CMD_MASK);
>  +out_be32(mddrc.ddr_time_config0, mddrc_config[1] & MDDRC_REFRESH_ZERO_MASK);
>  +out_be32(mddrc.ddr_time_config1, mddrc_config[2]);
>  +out_be32(mddrc.ddr_time_config2, mddrc_config[3]);

> I cannot help it, but every time I see this I think the code is
> wrong. Guess I have seen too many copy & paste errors in this style.
> When I see "...time_config2" I also want to see "mddrc_config[2]", i. e.
> identical indices. We should reorder the struct.

The 4 memory config registers are in memory order and unfortunatley the
register names throw you off.   Some hardware guy did this in the register
naming -- obviously !!!   So even though the array index doesn't
match the constant it does makes sense. 
What I think I should do is ... instead of default_mddrc_config calling my
array default_mddrc_reg ... or create an independant struct with just
these 4 regs and name them the same as the immap struct.   Actually -- I
like the later idea best .... I might do this soon and when I redo my 5125
patch incorporate it.

Very Bestest,
Martha
       

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2009-09-25  0:45 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-09-21 18:07 [U-Boot] [PATCH] Streamlined mpc512x fixed_sdram init sequence Martha M Stan
2009-09-24 22:36 ` Wolfgang Denk
2009-09-25  0:45 m marx

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.