Thank You for fixing this ... also .. "marx" was in pop3 properties - thanks for catching it. Your powers of observation are very keen !

-Martha

-----Original Message----- From: Wolfgang Denk wd@denx.de Sent 10/4/2009 4:56:08 PM To: u-boot@lists.denx.de Cc: Martha M Stan mmarx@silicontkx.com, Wolfgang Denk wd@denx.de Subject: [PATCH] mpc512x: fix fixed_sdram() init code.

Commit 054197ba and later fixes used an array to initialize some of the MDDRC parameters; however, the use of an array turned out to be a bad idea as it was not possible to correlate structure entries to array indices in readable and reliable way. Now we use a struct instead, which makes this self-explanatory.

Signed-off-by: Wolfgang Denk wd@denx.de --- board/freescale/mpc5121ads/mpc5121ads.c | 12 +++++----- cpu/mpc512x/fixed_sdram.c | 33 +++++++++++++++++------------- include/asm-ppc/immap_512x.h | 10 +++++++++ include/asm-ppc/mpc512x.h | 3 +- 4 files changed, 37 insertions(+), 21 deletions(-)

diff --git a/board/freescale/mpc5121ads/mpc5121ads.c b/board/freescale/mpc5121ads/mpc5121ads.c index 13bd73c..2fa3650 100644 --- a/board/freescale/mpc5121ads/mpc5121ads.c +++ b/board/freescale/mpc5121ads/mpc5121ads.c @@ -169,11 +169,11 @@ phys_size_t initdram(int board_type) * Elpida MDDRC and initialization settings are an alternative * to the Default Micron ones for all but the earliest Rev 4 boards */ -u32 elpida_mddrc_config[4] = { -CONFIG_SYS_MDDRC_TIME_CFG0, -CONFIG_SYS_MDDRC_TIME_CFG1_ELPIDA, -CONFIG_SYS_MDDRC_TIME_CFG2_ELPIDA, -CONFIG_SYS_MDDRC_SYS_CFG_ELPIDA, +ddr512x_config_t elpida_mddrc_config = { +.ddr_sys_config = CONFIG_SYS_MDDRC_SYS_CFG_ELPIDA, +.ddr_time_config0 = CONFIG_SYS_MDDRC_TIME_CFG0, +.ddr_time_config1 = CONFIG_SYS_MDDRC_TIME_CFG1_ELPIDA, +.ddr_time_config2 = CONFIG_SYS_MDDRC_TIME_CFG2_ELPIDA, };

u32 elpida_init_sequence[] = { @@ -229,7 +229,7 @@ phys_size_t initdram(int board_type) if (is_micron()) { msize = fixed_sdram(NULL, NULL, 0); } else { -msize = fixed_sdram(elpida_mddrc_config, +msize = fixed_sdram(&elpida_mddrc_config, elpida_init_sequence, sizeof(elpida_init_sequence)/sizeof(u32)); } diff --git a/cpu/mpc512x/fixed_sdram.c b/cpu/mpc512x/fixed_sdram.c index 673d61e..442b5fc 100644 --- a/cpu/mpc512x/fixed_sdram.c +++ b/cpu/mpc512x/fixed_sdram.c @@ -26,13 +26,13 @@ #include asm/mpc512x.h

/* - * MDDRC Config Runtime Settings in order of the 4 MDDRC cfg registers + * MDDRC Config Runtime Settings */ -u32 default_mddrc_config[4] = { -CONFIG_SYS_MDDRC_TIME_CFG0,/* time_config0 */ -CONFIG_SYS_MDDRC_TIME_CFG1,/* time_config1 */ -CONFIG_SYS_MDDRC_TIME_CFG2,/* time_config2 */ -CONFIG_SYS_MDDRC_SYS_CFG,/* sys_config*/ +ddr512x_config_t default_mddrc_config = { +.ddr_sys_config = CONFIG_SYS_MDDRC_SYS_CFG, +.ddr_time_config0 = CONFIG_SYS_MDDRC_TIME_CFG0, +.ddr_time_config1 = CONFIG_SYS_MDDRC_TIME_CFG1, +.ddr_time_config2 = CONFIG_SYS_MDDRC_TIME_CFG2, };

u32 default_init_seq[] = { @@ -74,7 +74,8 @@ u32 default_init_seq[] = { * 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(u32 *mddrc_config, u32 *dram_init_seq, int seq_sz) +long int fixed_sdram(ddr512x_config_t *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; @@ -83,7 +84,7 @@ long int fixed_sdram(u32 *mddrc_config, u32 *dram_init_seq, int seq_sz)

/* take default settings and init sequence if necessary */ if (mddrc_config == NULL) -mddrc_config = default_mddrc_config; +mddrc_config = &default_mddrc_config; if (dram_init_seq == NULL) { dram_init_seq = default_init_seq; seq_sz = sizeof(default_init_seq)/sizeof(u32); @@ -130,18 +131,22 @@ long int fixed_sdram(u32 *mddrc_config, u32 *dram_init_seq, int seq_sz) * 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[3] | MDDRC_SYS_CFG_CMD_MASK); -out_be32(mddrc.ddr_time_config0, mddrc_config[0] & MDDRC_REFRESH_ZERO_MASK); -out_be32(mddrc.ddr_time_config1, mddrc_config[1]); -out_be32(mddrc.ddr_time_config2, mddrc_config[2]); +out_be32(mddrc.ddr_sys_config, +mddrc_config-ddr_sys_config | MDDRC_SYS_CFG_CMD_MASK); +out_be32(mddrc.ddr_time_config0, +mddrc_config-ddr_time_config0 & MDDRC_REFRESH_ZERO_MASK); +out_be32(mddrc.ddr_time_config1, +mddrc_config-ddr_time_config1); +out_be32(mddrc.ddr_time_config2, +mddrc_config-ddr_time_config2);

/* Initialize DDR with either default or supplied init sequence */ for (i = 0; i seq_sz; i++) out_be32(mddrc.ddr_command, dram_init_seq[i]);

/* Start MDDRC */ -out_be32(mddrc.ddr_time_config0, mddrc_config[0]); -out_be32(mddrc.ddr_sys_config, mddrc_config[3]); +out_be32(mddrc.ddr_time_config0, mddrc_config-ddr_time_config0); +out_be32(mddrc.ddr_sys_config, mddrc_config-ddr_sys_config);

return msize; } diff --git a/include/asm-ppc/immap_512x.h b/include/asm-ppc/immap_512x.h index 79cdd80..bdc6ff2 100644 --- a/include/asm-ppc/immap_512x.h +++ b/include/asm-ppc/immap_512x.h @@ -347,6 +347,16 @@ typedef struct ddr512x { #define MDDRC_REFRESH_ZERO_MASK0x0000FFFF

/* + * DDR Memory Controller Configuration settings + */ +typedef struct ddr512x_config { +u32 ddr_sys_config;/* System Configuration Register */ +u32 ddr_time_config0;/* Timing Configuration Register */ +u32 ddr_time_config1;/* Timing Configuration Register */ +u32 ddr_time_config2;/* Timing Configuration Register */ +} ddr512x_config_t; + +/* * DMA/Messaging Unit */ typedef struct dma512x { diff --git a/include/asm-ppc/mpc512x.h b/include/asm-ppc/mpc512x.h index 8ef0d9c..960e229 100644 --- a/include/asm-ppc/mpc512x.h +++ b/include/asm-ppc/mpc512x.h @@ -50,7 +50,8 @@ static inline void sync_law(volatile void *addr) /* * Prototypes */ -extern long int fixed_sdram(u32 *mddrc_config, u32 *dram_init_seq, int seq_sz); +extern long int fixed_sdram(ddr512x_config_t *mddrc_config, +u32 *dram_init_seq, int seq_sz); extern int mpc5121_diu_init(void); extern void ide_set_reset(int idereset);