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

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

[agraham at amcc.com]

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
  */
-- 
1.5.5

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

[Stefan Roese]

On Wednesday 27 August 2008, agraham at 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 at denx.de
=====================================================================

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

[Wolfgang Denk]

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

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
HANDLE WITH EXTREME CARE:  This Product Contains  Minute Electrically
Charged  Particles  Moving  at  Velocities  in Excess of Five Hundred
Million Miles Per Hour.