[U-Boot] [PATCH] ppc4xx: Fix DDR2 auto calibration on Kilauea 600MHz

Stefan Roese
2008-09-16T13:04:41Z

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 at denx.de
=====================================================================