Dear Martha M Stan,
In message 12525956963103-git-send-email-mmarx@silicontkx.com you wrote:
From: Martha Marx mmarx@silicontkx.com
Rev 3 and earlier stay with Micron settings. Rev 4 boards manufactured before Nov-2008 Serial Number #1180 also stay with Micron settings. All new boards use a slightly slower Elpida setting. CONFIG_SYS_ELPIDA_MICRON_MIX sets up this detection and use.
Signed-off-by: Martha Marx Stan mmarx@silicontkx.com
board/freescale/mpc5121ads/mpc5121ads.c | 34 ++++++++++++ cpu/mpc512x/fixed_sdram.c | 88 +++++++++++++++++++++++------- include/configs/mpc5121ads.h | 36 ++++++++----- 3 files changed, 124 insertions(+), 34 deletions(-)
diff --git a/board/freescale/mpc5121ads/mpc5121ads.c b/board/freescale/mpc5121ads/mpc5121ads.c index a0d7a82..c6bbbad 100644 --- a/board/freescale/mpc5121ads/mpc5121ads.c +++ b/board/freescale/mpc5121ads/mpc5121ads.c
...
+#ifdef CONFIG_SYS_ELPIDA_MICRON_MIX +u32 is_micron(void){
Incorrect brace style.
Why is this u23 instead of a plain int?
- ushort brd_rev = *(vu_short *) (CONFIG_SYS_CPLD_BASE + 0x00);
Please use I/O accessors to read from device registers.
diff --git a/cpu/mpc512x/fixed_sdram.c b/cpu/mpc512x/fixed_sdram.c index 5be02f7..063f60a 100644 --- a/cpu/mpc512x/fixed_sdram.c +++ b/cpu/mpc512x/fixed_sdram.c @@ -25,6 +25,9 @@ #include <asm/io.h> #include <asm/mpc512x.h>
+#ifdef CONFIG_SYS_ELPIDA_MICRON_MIX +extern u32 is_micron(void); +#endif
This is ugly. A protoype in a header file should be used to allow for consistency checing. But stop. This file here has the only place where this function ever gets called, so why don't you move that function here? Or is there any special reason to have it in board/freescale/mpc5121ads/mpc5121ads.c ?
/*
- fixed sdram init:
- The board doesn't use memory modules that have serial presence
@@ -36,6 +39,11 @@ long int fixed_sdram(void) u32 msize = CONFIG_SYS_DDR_SIZE * 1024 * 1024; u32 msize_log2 = __ilog2(msize); u32 i; +#ifdef CONFIG_SYS_ELPIDA_MICRON_MIX
- u32 use_micron = is_micron();
+#else
- u32 use_micron = 1;
+#endif
Is there a special reason for u32 versus int?
Why do you leave use_micron defined in the "#else" case? I think we could omit this completely, then?
/* Initialize IO Control */ out_be32(&im->io_ctrl.io_control_mem, IOCTRL_MUX_DDR); @@ -74,10 +82,19 @@ long int fixed_sdram(void) out_be32(&im->mddrc.lut_table4_alternate_lower, MDDRCGRP_LUT4_AL);
/* Initialize MDDRC */
- out_be32(&im->mddrc.ddr_sys_config, MDDRC_SYS_CFG);
- out_be32(&im->mddrc.ddr_time_config0, MDDRC_TIME_CFG0);
- out_be32(&im->mddrc.ddr_time_config1, MDDRC_TIME_CFG1);
- out_be32(&im->mddrc.ddr_time_config2, MDDRC_TIME_CFG2);
- if (use_micron) {
out_be32(&im->mddrc.ddr_sys_config, MDDRC_SYS_CFG);out_be32(&im->mddrc.ddr_time_config0, MDDRC_TIME_CFG0);out_be32(&im->mddrc.ddr_time_config1, MDDRC_TIME_CFG1);out_be32(&im->mddrc.ddr_time_config2, MDDRC_TIME_CFG2);+#ifdef CONFIG_SYS_ELPIDA_MICRON_MIX
- } else {
out_be32(&im->mddrc.ddr_sys_config, MDDRC_SYS_CFG_ELPIDA);out_be32(&im->mddrc.ddr_time_config0, MDDRC_TIME_CFG0);out_be32(&im->mddrc.ddr_time_config1, MDDRC_TIME_CFG1_ELPIDA);out_be32(&im->mddrc.ddr_time_config2, MDDRC_TIME_CFG2_ELPIDA);+#endif
Ditto.
...
- if (use_micron) {
out_be32(&im->mddrc.ddr_command, DDR_MICRON_INIT_DEV_OP);out_be32(&im->mddrc.ddr_command, DDR_NOP);out_be32(&im->mddrc.ddr_command, DDR_EM2);out_be32(&im->mddrc.ddr_command, DDR_NOP);out_be32(&im->mddrc.ddr_command, DDR_PCHG_ALL);out_be32(&im->mddrc.ddr_command, DDR_EM2);out_be32(&im->mddrc.ddr_command, DDR_EM3);out_be32(&im->mddrc.ddr_command, DDR_EN_DLL);out_be32(&im->mddrc.ddr_command, DDR_MICRON_INIT_DEV_OP);out_be32(&im->mddrc.ddr_command, DDR_PCHG_ALL);out_be32(&im->mddrc.ddr_command, DDR_RFSH);out_be32(&im->mddrc.ddr_command, DDR_MICRON_INIT_DEV_OP);out_be32(&im->mddrc.ddr_command, DDR_OCD_DEFAULT);out_be32(&im->mddrc.ddr_command, DDR_PCHG_ALL);out_be32(&im->mddrc.ddr_command, DDR_NOP);- /* Start MDDRC */
out_be32(&im->mddrc.ddr_time_config0, MDDRC_TIME_CFG0_RUN);
Incorrect indentation.
} else {
out_be32(&im->mddrc.ddr_command, DDR_EM2);out_be32(&im->mddrc.ddr_command, DDR_EM3);out_be32(&im->mddrc.ddr_command, DDR_EN_DLL);out_be32(&im->mddrc.ddr_command, DDR_RES_DLL);out_be32(&im->mddrc.ddr_command, DDR_PCHG_ALL);out_be32(&im->mddrc.ddr_command, DDR_RFSH);out_be32(&im->mddrc.ddr_command, DDR_RFSH);out_be32(&im->mddrc.ddr_command, DDR_RFSH);out_be32(&im->mddrc.ddr_command, DDR_ELPIDA_INIT_DEV_OP);udelay(200);out_be32(&im->mddrc.ddr_command, DDR_OCD_DEFAULT);out_be32(&im->mddrc.ddr_command, DDR_OCD_EXIT);out_be32(&im->mddrc.ddr_command, DDR_PCHG_ALL);out_be32(&im->mddrc.ddr_command, DDR_NOP);out_be32(&im->mddrc.ddr_command, DDR_OCD_DEFAULT);out_be32(&im->mddrc.ddr_command, DDR_OCD_EXIT);out_be32(&im->mddrc.ddr_command, DDR_PCHG_ALL);for (i = 0; i < 10; i++)out_be32(&im->mddrc.ddr_command, DDR_NOP);/* Start MDDRC */
Incorrect indentation.
diff --git a/include/configs/mpc5121ads.h b/include/configs/mpc5121ads.h index 5df51e3..a618b16 100644 --- a/include/configs/mpc5121ads.h +++ b/include/configs/mpc5121ads.h
...
+#define MDDRC_SYS_CFG_ELPIDA 0xFA802B00 +#define MDDRC_SYS_CFG_ELPIDA_RUN 0xEA802B00 +#define MDDRC_TIME_CFG1_ELPIDA 0x690e1189 +#define MDDRC_TIME_CFG2_ELPIDA 0x35310864
These are all configurable parameters that should be named CONFIG_SYS_*.
Best regards,
Wolfgang Denk