On Tuesday 16 September 2008, Victor Gallardo wrote:

Signed-off-by: Victor Gallardo vgallardo@amcc.com Signed-off-by: Adam Graham agraham@amcc.com

Please find some comments below.

---

board/amcc/kilauea/kilauea.c | 31 +++++++++++++++++++++++++++++++ cpu/ppc4xx/4xx_ibm_ddr2_autocalib.c | 5 ----- include/asm-ppc/ppc4xx-sdram.h | 8 ++++++++ 3 files changed, 39 insertions(+), 5 deletions(-)

diff --git a/board/amcc/kilauea/kilauea.c b/board/amcc/kilauea/kilauea.c index 7b10255..8163d16 100644 --- a/board/amcc/kilauea/kilauea.c +++ b/board/amcc/kilauea/kilauea.c @@ -374,3 +374,34 @@ int post_hotkeys_pressed(void) return 0; /* No hotkeys supported */ } #endif /* CONFIG_POST */

+#if defined(CONFIG_PPC4xx_DDR_AUTOCALIBRATION) +/*

• • This is for quicker auto calibration boot up once WRDTR and CLKTR
• • values for the kilauea board were determined and are therefore known.
• • Use these scan options for PLB bus greater than 200MHz else use
• • the defaults.
• */

+/* List of (SDRAM_WRDTR.[WDTR], SDRAM_CLKTR.[CLKP]) pairs to try */ +struct sdram_timing quick_scan_options[] = {

• {0, 3}, {1, 1}, {1, 2}, {1, 3},
• {2, 1}, {2, 2}, {2, 3}, {3, 1},
• {3, 2}, {4, 1}, {-1, -1}

+};

It's not really clear to me, why this reduced list should fix this problem seen on the 600MHz boards (it does - I tested successfully on my board). From my understanding the scan is now skipping some of the WDTR/CLKTR configurations. This will of course result in a quicker scan, but autocalibration should still work with the default full list.

I'm probably missing something here so please enlighten me. :)

+ulong ddr_scan_option(ulong default_val) +{

• u32 sdram_freq;
• PPC4xx_SYS_INFO board_cfg;
• get_sys_info(&board_cfg);
• sdram_freq = board_cfg.freqPLB;
• /* PLB bus greater than 200MHz */
• if (sdram_freq >= 0xbebc200)

Why do you write this in hex? Change it to decimal please.

• return (ulong)(quick_scan_options);
  

• else

• return (ulong)default_val;
  

+} +#endif /* CONFIG_PPC4xx_DDR_AUTOCALIBRATION */ diff --git a/cpu/ppc4xx/4xx_ibm_ddr2_autocalib.c b/cpu/ppc4xx/4xx_ibm_ddr2_autocalib.c index 83b9883..3ba8176 100644 --- a/cpu/ppc4xx/4xx_ibm_ddr2_autocalib.c +++ b/cpu/ppc4xx/4xx_ibm_ddr2_autocalib.c @@ -79,11 +79,6 @@ struct ddrautocal { u32 flags; };

-struct sdram_timing {

• u32 wrdtr;
• u32 clktr;

-};

struct sdram_timing_clks { u32 wrdtr; u32 clktr; diff --git a/include/asm-ppc/ppc4xx-sdram.h b/include/asm-ppc/ppc4xx-sdram.h index 8efa557..50148c5 100644 --- a/include/asm-ppc/ppc4xx-sdram.h +++ b/include/asm-ppc/ppc4xx-sdram.h @@ -1403,6 +1403,14 @@ #endif /* CONFIG_SDRAM_PPC4xx_DENALI_DDR2 */

#ifndef __ASSEMBLY__

+#if defined(CONFIG_PPC4xx_DDR_AUTOCALIBRATION) +struct sdram_timing {

• u32 wrdtr;
• u32 clktr;

+}; +#endif /* CONFIG_PPC4xx_DDR_AUTOCALIBRATION */

I suggest you remove this unnecessary conditional compilation option here. It doesn't hurt to have this struct declared in all configurations.

Please fix and resubmit. Thanks.

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 =====================================================================