[U-Boot] [PATCH] ppc4xx: For the Kilauea board include the new PPC4xx SDRAM Controller DDR autocalibration routine.

From: Adam Graham agraham@amcc.com
Signed-off-by: Adam Graham agraham@amcc.com --- cpu/ppc4xx/44x_spd_ddr2.c | 58 ++++++++++++++++++++++++++++++--------- cpu/ppc4xx/Makefile | 1 + include/asm-ppc/ppc4xx-sdram.h | 2 +- include/configs/kilauea.h | 15 ++++++++++ 4 files changed, 61 insertions(+), 15 deletions(-)
diff --git a/cpu/ppc4xx/44x_spd_ddr2.c b/cpu/ppc4xx/44x_spd_ddr2.c index 001f2c1..fc1ae5f 100644 --- a/cpu/ppc4xx/44x_spd_ddr2.c +++ b/cpu/ppc4xx/44x_spd_ddr2.c @@ -60,7 +60,8 @@ "SDRAM_" #mnemonic, SDRAM_##mnemonic, data); \ } while (0)
-static inline void ppc4xx_ibm_ddr2_register_dump(void); +inline void ppc4xx_ibm_ddr2_register_dump(void); +void blank_string(int);
#if defined(CONFIG_SPD_EEPROM)
@@ -2291,18 +2292,6 @@ static unsigned long is_ecc_enabled(void) return ecc; }
-static void blank_string(int size) -{ - int i; - - for (i=0; i<size; i++) - putc('\b'); - for (i=0; i<size; i++) - putc(' '); - for (i=0; i<size; i++) - putc('\b'); -} - #ifdef CONFIG_DDR_ECC /*-----------------------------------------------------------------------------+ * program_ecc. @@ -2966,6 +2955,10 @@ static void test(void)
#else /* CONFIG_SPD_EEPROM */
+#if defined(CONFIG_PPC4xx_DDR_AUTOCALIBRATION) +extern u32 DQS_autocalibration(void); +#endif /* CONFIG_PPC4xx_DDR_AUTOCALIBRATION */ + /*----------------------------------------------------------------------------- * Function: initdram * Description: Configures the PPC405EX(r) DDR1/DDR2 SDRAM memory @@ -3065,9 +3058,12 @@ phys_size_t initdram(int board_type) /* Set Delay Control Registers */
mtsdram(SDRAM_DLCR, CFG_SDRAM0_DLCR); + +#if !defined(CONFIG_PPC4xx_DDR_AUTOCALIBRATION) mtsdram(SDRAM_RDCC, CFG_SDRAM0_RDCC); mtsdram(SDRAM_RQDC, CFG_SDRAM0_RQDC); mtsdram(SDRAM_RFDC, CFG_SDRAM0_RFDC); +#endif /* !CONFIG_PPC4xx_DDR_AUTOCALIBRATION */
/* * Enable Controller by SDRAM0_MCOPT2[DCEN] = 1: @@ -3076,18 +3072,52 @@ phys_size_t initdram(int board_type) mfsdram(SDRAM_MCOPT2, val); mtsdram(SDRAM_MCOPT2, val | SDRAM_MCOPT2_DCEN_ENABLE);
+#if defined(CONFIG_PPC4xx_DDR_AUTOCALIBRATION) +#if !defined(CONFIG_NAND_U_BOOT) && !defined(CONFIG_NAND_SPL) + /*------------------------------------------------------------------ + | DQS calibration. + +-----------------------------------------------------------------*/ + DQS_autocalibration(); +#endif /* !defined(CONFIG_NAND_U_BOOT) && !defined(CONFIG_NAND_SPL) */ +#endif /* CONFIG_PPC4xx_DDR_AUTOCALIBRATION */ + #if defined(CONFIG_DDR_ECC) ecc_init(CFG_SDRAM_BASE, CFG_MBYTES_SDRAM << 20); #endif /* defined(CONFIG_DDR_ECC) */
ppc4xx_ibm_ddr2_register_dump(); + +#if defined(CONFIG_PPC4xx_DDR_AUTOCALIBRATION) + /* + * Clear potential errors resulting from auto-calibration. + * If not done, then we could get an interrupt later on when + * exceptions are enabled. + */ + asm volatile ("mfspr 0,0x23c":::"r0" ); + asm volatile ("mtspr 0x23c,0":::"r0" ); +#endif /* CONFIG_PPC4xx_DDR_AUTOCALIBRATION */ + #endif /* !defined(CONFIG_NAND_U_BOOT) || defined(CONFIG_NAND_SPL) */
return (CFG_MBYTES_SDRAM << 20); } #endif /* CONFIG_SPD_EEPROM */
-static inline void ppc4xx_ibm_ddr2_register_dump(void) +#if !defined(CONFIG_NAND_U_BOOT) && !defined(CONFIG_NAND_SPL) +void blank_string(int size) +{ + int i; + + for (i=0; i<size; i++) + putc('\b'); + for (i=0; i<size; i++) + putc(' '); + for (i=0; i<size; i++) + putc('\b'); +} +#endif /* !defined(CONFIG_NAND_U_BOOT) && !defined(CONFIG_NAND_SPL) */ + +inline void ppc4xx_ibm_ddr2_register_dump(void) { #if defined(DEBUG) printf("\nPPC4xx IBM DDR2 Register Dump:\n"); diff --git a/cpu/ppc4xx/Makefile b/cpu/ppc4xx/Makefile index c773400..c9788b8 100644 --- a/cpu/ppc4xx/Makefile +++ b/cpu/ppc4xx/Makefile @@ -35,6 +35,7 @@ SOBJS += kgdb.o COBJS := 40x_spd_sdram.o COBJS += 44x_spd_ddr.o COBJS += 44x_spd_ddr2.o +COBJS += 4xx_ddr2.o COBJS += 4xx_pci.o COBJS += 4xx_pcie.o COBJS += bedbug_405.o diff --git a/include/asm-ppc/ppc4xx-sdram.h b/include/asm-ppc/ppc4xx-sdram.h index 0174d62..9848390 100644 --- a/include/asm-ppc/ppc4xx-sdram.h +++ b/include/asm-ppc/ppc4xx-sdram.h @@ -256,6 +256,7 @@ #define SDRAM_DLYCAL_DLCV_ENCODE(x) (((x)<<2) & SDRAM_DLYCAL_DLCV_MASK) #define SDRAM_DLYCAL_DLCV_DECODE(x) (((x) & SDRAM_DLYCAL_DLCV_MASK)>>2)
+#if !defined(CONFIG_405EX) /* * Memory queue defines */ @@ -293,7 +294,6 @@
#define SDRAM_PLBADDUHB (SDRAMQ_DCR_BASE+0x10) /* PLB base address upper 32 LL */
-#if !defined(CONFIG_405EX) /* * Memory Bank 0-7 configuration */ diff --git a/include/configs/kilauea.h b/include/configs/kilauea.h index a475f97..e417295 100644 --- a/include/configs/kilauea.h +++ b/include/configs/kilauea.h @@ -222,6 +222,18 @@ * DDR SDRAM *----------------------------------------------------------------------*/ #define CFG_MBYTES_SDRAM (256) /* 256MB */ +#define CONFIG_PPC4xx_DDR_AUTOCALIBRATION /* IBM DDR autocalibration */ + +/* + * DDR Autocalibration Method_B is the default. + * Note: DDR Autocalibration Method_A scans the full range of possible PPC4xx + * SDRAM Controller DDR autocalibration values and takes a lot longer + * to run than Method_B. + * (See the Method_A and Method_B algorithm discription in the file: + * cpu/ppc4xx/4xx_ddr2.c) + * Define to use DDR autocalibration Method_A + */ +#undef CONFIG_PPC4xx_DDR_METHOD_A
#define CFG_SDRAM0_MB0CF_BASE (( 0 << 20) + CFG_SDRAM_BASE)
@@ -386,6 +398,9 @@ #define CONFIG_HAS_ETH1 1 /* add support for "eth1addr" */ #define CONFIG_PHY1_ADDR 2
+/* Debug messages for the DDR autocalibration */ +#define CONFIG_AUTOCALIB "silent\0" /* default is non-verbose */ + /* * Default environment variables */

On Wednesday 27 August 2008, agraham@amcc.com wrote:
From: Adam Graham agraham@amcc.com
Thanks. Please find some comments below.
Signed-off-by: Adam Graham agraham@amcc.com
cpu/ppc4xx/44x_spd_ddr2.c | 58 ++++++++++++++++++++++++++++++--------- cpu/ppc4xx/Makefile | 1 + include/asm-ppc/ppc4xx-sdram.h | 2 +- include/configs/kilauea.h | 15 ++++++++++ 4 files changed, 61 insertions(+), 15 deletions(-)
diff --git a/cpu/ppc4xx/44x_spd_ddr2.c b/cpu/ppc4xx/44x_spd_ddr2.c index 001f2c1..fc1ae5f 100644 --- a/cpu/ppc4xx/44x_spd_ddr2.c +++ b/cpu/ppc4xx/44x_spd_ddr2.c @@ -60,7 +60,8 @@ "SDRAM_" #mnemonic, SDRAM_##mnemonic, data); \ } while (0)
-static inline void ppc4xx_ibm_ddr2_register_dump(void); +inline void ppc4xx_ibm_ddr2_register_dump(void); +void blank_string(int);
#if defined(CONFIG_SPD_EEPROM)
@@ -2291,18 +2292,6 @@ static unsigned long is_ecc_enabled(void) return ecc; }
-static void blank_string(int size) -{
- int i;
- for (i=0; i<size; i++)
putc('\b');
- for (i=0; i<size; i++)
putc(' ');
- for (i=0; i<size; i++)
putc('\b');
-}
Why is this needed? This change doesn't really match the patch description. I suggest you move those Kilauea unrelated changes into a separate patch.
#ifdef CONFIG_DDR_ECC
/*------------------------------------------------------------------------- ----+ * program_ecc. @@ -2966,6 +2955,10 @@ static void test(void)
#else /* CONFIG_SPD_EEPROM */
+#if defined(CONFIG_PPC4xx_DDR_AUTOCALIBRATION) +extern u32 DQS_autocalibration(void); +#endif /* CONFIG_PPC4xx_DDR_AUTOCALIBRATION */
/*------------------------------------------------------------------------- ---- * Function: initdram
- Description: Configures the PPC405EX(r) DDR1/DDR2 SDRAM memory
@@ -3065,9 +3058,12 @@ phys_size_t initdram(int board_type) /* Set Delay Control Registers */
mtsdram(SDRAM_DLCR, CFG_SDRAM0_DLCR);
+#if !defined(CONFIG_PPC4xx_DDR_AUTOCALIBRATION) mtsdram(SDRAM_RDCC, CFG_SDRAM0_RDCC); mtsdram(SDRAM_RQDC, CFG_SDRAM0_RQDC); mtsdram(SDRAM_RFDC, CFG_SDRAM0_RFDC); +#endif /* !CONFIG_PPC4xx_DDR_AUTOCALIBRATION */
/* * Enable Controller by SDRAM0_MCOPT2[DCEN] = 1: @@ -3076,18 +3072,52 @@ phys_size_t initdram(int board_type) mfsdram(SDRAM_MCOPT2, val); mtsdram(SDRAM_MCOPT2, val | SDRAM_MCOPT2_DCEN_ENABLE);
+#if defined(CONFIG_PPC4xx_DDR_AUTOCALIBRATION) +#if !defined(CONFIG_NAND_U_BOOT) && !defined(CONFIG_NAND_SPL)
/*------------------------------------------------------------------ + | DQS calibration.
+-----------------------------------------------------------------*/
Please use the recommended comment style. Everywhere in this patch.
DQS_autocalibration();
+#endif /* !defined(CONFIG_NAND_U_BOOT) && !defined(CONFIG_NAND_SPL) */ +#endif /* CONFIG_PPC4xx_DDR_AUTOCALIBRATION */
#if defined(CONFIG_DDR_ECC) ecc_init(CFG_SDRAM_BASE, CFG_MBYTES_SDRAM << 20); #endif /* defined(CONFIG_DDR_ECC) */
ppc4xx_ibm_ddr2_register_dump();
+#if defined(CONFIG_PPC4xx_DDR_AUTOCALIBRATION)
/*
* Clear potential errors resulting from auto-calibration.
* If not done, then we could get an interrupt later on when
* exceptions are enabled.
*/
asm volatile ("mfspr 0,0x23c":::"r0" );
asm volatile ("mtspr 0x23c,0":::"r0" );
That's MCSR, right? Is the 405EX the only 405 variant which has this register? We should add move the MCSR access functions from ppc440.h to ppc4xx.h so that they can be used for 405EX too.
+#endif /* CONFIG_PPC4xx_DDR_AUTOCALIBRATION */
#endif /* !defined(CONFIG_NAND_U_BOOT) || defined(CONFIG_NAND_SPL) */
return (CFG_MBYTES_SDRAM << 20); } #endif /* CONFIG_SPD_EEPROM */
-static inline void ppc4xx_ibm_ddr2_register_dump(void) +#if !defined(CONFIG_NAND_U_BOOT) && !defined(CONFIG_NAND_SPL) +void blank_string(int size) +{
- int i;
- for (i=0; i<size; i++)
putc('\b');
- for (i=0; i<size; i++)
putc(' ');
- for (i=0; i<size; i++)
putc('\b');
+} +#endif /* !defined(CONFIG_NAND_U_BOOT) && !defined(CONFIG_NAND_SPL) */
+inline void ppc4xx_ibm_ddr2_register_dump(void) { #if defined(DEBUG) printf("\nPPC4xx IBM DDR2 Register Dump:\n"); diff --git a/cpu/ppc4xx/Makefile b/cpu/ppc4xx/Makefile index c773400..c9788b8 100644 --- a/cpu/ppc4xx/Makefile +++ b/cpu/ppc4xx/Makefile @@ -35,6 +35,7 @@ SOBJS += kgdb.o COBJS := 40x_spd_sdram.o COBJS += 44x_spd_ddr.o COBJS += 44x_spd_ddr2.o +COBJS += 4xx_ddr2.o COBJS += 4xx_pci.o COBJS += 4xx_pcie.o COBJS += bedbug_405.o diff --git a/include/asm-ppc/ppc4xx-sdram.h b/include/asm-ppc/ppc4xx-sdram.h index 0174d62..9848390 100644 --- a/include/asm-ppc/ppc4xx-sdram.h +++ b/include/asm-ppc/ppc4xx-sdram.h @@ -256,6 +256,7 @@ #define SDRAM_DLYCAL_DLCV_ENCODE(x) (((x)<<2) & SDRAM_DLYCAL_DLCV_MASK) #define SDRAM_DLYCAL_DLCV_DECODE(x) (((x) & SDRAM_DLYCAL_DLCV_MASK)>>2)
+#if !defined(CONFIG_405EX) /*
- Memory queue defines
*/ @@ -293,7 +294,6 @@
#define SDRAM_PLBADDUHB (SDRAMQ_DCR_BASE+0x10) /* PLB base address upper 32 LL */
-#if !defined(CONFIG_405EX) /*
- Memory Bank 0-7 configuration
*/ diff --git a/include/configs/kilauea.h b/include/configs/kilauea.h index a475f97..e417295 100644 --- a/include/configs/kilauea.h +++ b/include/configs/kilauea.h @@ -222,6 +222,18 @@
- DDR SDRAM
*----------------------------------------------------------------------*/ #define CFG_MBYTES_SDRAM (256) /* 256MB */ +#define CONFIG_PPC4xx_DDR_AUTOCALIBRATION /* IBM DDR autocalibration */
+/*
- DDR Autocalibration Method_B is the default.
- Note: DDR Autocalibration Method_A scans the full range of possible
PPC4xx + * SDRAM Controller DDR autocalibration values and takes a lot longer + * to run than Method_B.
- (See the Method_A and Method_B algorithm discription in the file:
- cpu/ppc4xx/4xx_ddr2.c)
- Define to use DDR autocalibration Method_A
- */
+#undef CONFIG_PPC4xx_DDR_METHOD_A
#define CFG_SDRAM0_MB0CF_BASE (( 0 << 20) + CFG_SDRAM_BASE)
@@ -386,6 +398,9 @@ #define CONFIG_HAS_ETH1 1 /* add support for "eth1addr" */ #define CONFIG_PHY1_ADDR 2
+/* Debug messages for the DDR autocalibration */ +#define CONFIG_AUTOCALIB "silent\0" /* default is non-verbose */
Please move this define to the DDR2 defines above.
Best regards. Stefan
===================================================================== DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de =====================================================================

Dear Adam,
in message 1219816308-9501-1-git-send-email-agraham@amcc.com you wrote:
From: Adam Graham agraham@amcc.com
Signed-off-by: Adam Graham agraham@amcc.com
cpu/ppc4xx/44x_spd_ddr2.c | 58 ++++++++++++++++++++++++++++++--------- cpu/ppc4xx/Makefile | 1 + include/asm-ppc/ppc4xx-sdram.h | 2 +- include/configs/kilauea.h | 15 ++++++++++ 4 files changed, 61 insertions(+), 15 deletions(-)
Please note that I mentiononly issues not already pointed out by Stefan.
- Please use TABs for indentation and vertical alignment, not spaces (piping your code through "unexpand -a" might help, assuming you don't have fancy printf() format strings with multiple spaces).
- Please mind the maximum line length.
+/* Debug messages for the DDR autocalibration */ +#define CONFIG_AUTOCALIB "silent\0" /* default is non-verbose */
Where is #define actually being used? It looks dangerous to me. In most cases, you will use such #defines within "#ifdef" constrcuts without actually caring about the value; and the trailing '\0' makes me especially nervous as it looks as if you were intending to use this somewhere are part of the environment settings, but I cannot find any such code.
Something seems to be missing here?
Best regards,
Wolfgang Denk
participants (3)
-
agraham@amcc.com
-
Stefan Roese
-
Wolfgang Denk