diff mbox

[U-Boot,v6,1/4] mpc8xxx: DDR2/DDR3: Clean up DIMM-type switch statements

Message ID 1300208664-18339-2-git-send-email-Kyle.D.Moffett@boeing.com
State Superseded
Headers show

Commit Message

Kyle Moffett March 15, 2011, 5:04 p.m. UTC 
The numeric constants in the switch statements are replaced by #defines
added to the common ddr_spd.h header.  This dramatically improves the
readability of the switch statments.

In addition, a few of the longer lines were cleaned up, and the DDR2
type for an SO-RDIMM module was added to the DDR2 switch statement.

Signed-off-by: Kyle Moffett <Kyle.D.Moffett@boeing.com>
Cc: Andy Fleming <afleming@gmail.com>
Cc: Kumar Gala <kumar.gala@freescale.com>
Cc: Kim Phillips <kim.phillips@freescale.com>

---
Changelog:
v2: Moved the constants to include/ddr_spd.h and also fixed DDR3
v3: No changes
v4: Fixed up excessively long lines
v5: No changes
v6: Rebased onto the 'next' branch of git://git.denx.de/u-boot-mpc85xx.git

 arch/powerpc/cpu/mpc8xxx/ddr/ddr2_dimm_params.c |   23 +++++++-----
 arch/powerpc/cpu/mpc8xxx/ddr/ddr3_dimm_params.c |   43 ++++++++++------------
 common/ddr_spd.c                                |    2 +-
 include/ddr_spd.h                               |   28 ++++++++++-----
 4 files changed, 53 insertions(+), 43 deletions(-)

Comments

Kumar Gala March 31, 2011, 8:30 a.m. UTC | #1 
On Mar 15, 2011, at 12:04 PM, Kyle Moffett wrote:

> The numeric constants in the switch statements are replaced by #defines
> added to the common ddr_spd.h header.  This dramatically improves the
> readability of the switch statments.
> 
> In addition, a few of the longer lines were cleaned up, and the DDR2
> type for an SO-RDIMM module was added to the DDR2 switch statement.
> 
> Signed-off-by: Kyle Moffett <Kyle.D.Moffett@boeing.com>
> Cc: Andy Fleming <afleming@gmail.com>
> Cc: Kumar Gala <kumar.gala@freescale.com>
> Cc: Kim Phillips <kim.phillips@freescale.com>
> 
> ---
> Changelog:
> v2: Moved the constants to include/ddr_spd.h and also fixed DDR3
> v3: No changes
> v4: Fixed up excessively long lines
> v5: No changes
> v6: Rebased onto the 'next' branch of git://git.denx.de/u-boot-mpc85xx.git
> 
> arch/powerpc/cpu/mpc8xxx/ddr/ddr2_dimm_params.c |   23 +++++++-----
> arch/powerpc/cpu/mpc8xxx/ddr/ddr3_dimm_params.c |   43 ++++++++++------------
> common/ddr_spd.c                                |    2 +-
> include/ddr_spd.h                               |   28 ++++++++++-----
> 4 files changed, 53 insertions(+), 43 deletions(-)

applied to 85xx next

- k
diff mbox

Patch

diff --git a/arch/powerpc/cpu/mpc8xxx/ddr/ddr2_dimm_params.c b/arch/powerpc/cpu/mpc8xxx/ddr/ddr2_dimm_params.c
index dcb37ce..b565e33 100644
--- a/arch/powerpc/cpu/mpc8xxx/ddr/ddr2_dimm_params.c
+++ b/arch/powerpc/cpu/mpc8xxx/ddr/ddr2_dimm_params.c
@@ -250,24 +250,27 @@  ddr_compute_dimm_parameters(const ddr2_spd_eeprom_t *spd,
 	pdimm->primary_sdram_width = spd->primw;
 	pdimm->ec_sdram_width = spd->ecw;
 
-	/* FIXME: what about registered SO-DIMM? */
+	/* These are all the types defined by the JEDEC DDR2 SPD 1.3 spec */
 	switch (spd->dimm_type) {
-	case 0x01:	/* RDIMM */
-	case 0x10:	/* Mini-RDIMM */
-		pdimm->registered_dimm = 1; /* register buffered */
+	case DDR2_SPD_DIMMTYPE_RDIMM:
+	case DDR2_SPD_DIMMTYPE_72B_SO_RDIMM:
+	case DDR2_SPD_DIMMTYPE_MINI_RDIMM:
+		/* Registered/buffered DIMMs */
+		pdimm->registered_dimm = 1;
 		break;
 
-	case 0x02:	/* UDIMM */
-	case 0x04:	/* SO-DIMM */
-	case 0x08:	/* Micro-DIMM */
-	case 0x20:	/* Mini-UDIMM */
-		pdimm->registered_dimm = 0;	/* unbuffered */
+	case DDR2_SPD_DIMMTYPE_UDIMM:
+	case DDR2_SPD_DIMMTYPE_SO_DIMM:
+	case DDR2_SPD_DIMMTYPE_MICRO_DIMM:
+	case DDR2_SPD_DIMMTYPE_MINI_UDIMM:
+		/* Unbuffered DIMMs */
+		pdimm->registered_dimm = 0;
 		break;
 
+	case DDR2_SPD_DIMMTYPE_72B_SO_CDIMM:
 	default:
 		printf("unknown dimm_type 0x%02X\n", spd->dimm_type);
 		return 1;
-		break;
 	}
 
 	/* SDRAM device parameters */
diff --git a/arch/powerpc/cpu/mpc8xxx/ddr/ddr3_dimm_params.c b/arch/powerpc/cpu/mpc8xxx/ddr/ddr3_dimm_params.c
index 29cea53..756b15f 100644
--- a/arch/powerpc/cpu/mpc8xxx/ddr/ddr3_dimm_params.c
+++ b/arch/powerpc/cpu/mpc8xxx/ddr/ddr3_dimm_params.c
@@ -128,24 +128,32 @@  ddr_compute_dimm_parameters(const ddr3_spd_eeprom_t *spd,
 	pdimm->data_width = pdimm->primary_sdram_width
 			  + pdimm->ec_sdram_width;
 
-	switch (spd->module_type & 0xf) {
-	case 0x01:	/* RDIMM */
-	case 0x05:	/* Mini-RDIMM */
-		pdimm->registered_dimm = 1; /* register buffered */
+	/* These are the types defined by the JEDEC DDR3 SPD spec */
+	pdimm->mirrored_dimm = 0;
+	pdimm->registered_dimm = 0;
+	switch (spd->module_type & DDR3_SPD_MODULETYPE_MASK) {
+	case DDR3_SPD_MODULETYPE_RDIMM:
+	case DDR3_SPD_MODULETYPE_MINI_RDIMM:
+		/* Registered/buffered DIMMs */
+		pdimm->registered_dimm = 1;
 		for (i = 0; i < 16; i += 2) {
-			pdimm->rcw[i] = spd->mod_section.registered.rcw[i/2] & 0x0F;
-			pdimm->rcw[i+1] = (spd->mod_section.registered.rcw[i/2] >> 4) & 0x0F;
+			u8 rcw = spd->mod_section.registered.rcw[i/2];
+			pdimm->rcw[i]   = (rcw >> 0) & 0x0F;
+			pdimm->rcw[i+1] = (rcw >> 4) & 0x0F;
 		}
 		break;
-	case 0x02:	/* UDIMM */
-	case 0x03:	/* SO-DIMM */
-	case 0x04:	/* Micro-DIMM */
-	case 0x06:	/* Mini-UDIMM */
-		pdimm->registered_dimm = 0;	/* unbuffered */
+
+	case DDR3_SPD_MODULETYPE_UDIMM:
+	case DDR3_SPD_MODULETYPE_SO_DIMM:
+	case DDR3_SPD_MODULETYPE_MICRO_DIMM:
+	case DDR3_SPD_MODULETYPE_MINI_UDIMM:
+		/* Unbuffered DIMMs */
+		if (spd->mod_section.unbuffered.addr_mapping & 0x1)
+			pdimm->mirrored_dimm = 1;
 		break;
 
 	default:
-		printf("unknown dimm_type 0x%02X\n", spd->module_type);
+		printf("unknown module_type 0x%02X\n", spd->module_type);
 		return 1;
 	}
 
@@ -303,16 +311,5 @@  ddr_compute_dimm_parameters(const ddr3_spd_eeprom_t *spd,
 	pdimm->tFAW_ps = (((spd->tFAW_msb & 0xf) << 8) | spd->tFAW_min)
 			* mtb_ps;
 
-	/*
-	 * We need check the address mirror for unbuffered DIMM
-	 * If SPD indicate the address map mirror, The DDR controller
-	 * need care it.
-	 */
-	if ((spd->module_type == SPD_MODULETYPE_UDIMM) ||
-	    (spd->module_type == SPD_MODULETYPE_SODIMM) ||
-	    (spd->module_type == SPD_MODULETYPE_MICRODIMM) ||
-	    (spd->module_type == SPD_MODULETYPE_MINIUDIMM))
-		pdimm->mirrored_dimm = spd->mod_section.unbuffered.addr_mapping & 0x1;
-
 	return 0;
 }
diff --git a/common/ddr_spd.c b/common/ddr_spd.c
index a7a30de..7a388bb 100644
--- a/common/ddr_spd.c
+++ b/common/ddr_spd.c
@@ -18,7 +18,7 @@  spd_check(const u8 *buf, u8 spd_rev, u8 spd_cksum)
 
 	/*
 	 * Check SPD revision supported
-	 * Rev 1.2 or less supported by this code
+	 * Rev 1.X or less supported by this code
 	 */
 	if (spd_rev >= 0x20) {
 		printf("SPD revision %02X not supported by this code\n",
diff --git a/include/ddr_spd.h b/include/ddr_spd.h
index 710e528..e895d61 100644
--- a/include/ddr_spd.h
+++ b/include/ddr_spd.h
@@ -304,14 +304,24 @@  extern unsigned int ddr3_spd_check(const ddr3_spd_eeprom_t *spd);
 #define SPD_MEMTYPE_DDR2_FBDIMM_PROBE	(0x0A)
 #define SPD_MEMTYPE_DDR3	(0x0B)
 
-/*
- * Byte 3 Key Byte / Module Type for DDR3 SPD
- */
-#define SPD_MODULETYPE_RDIMM		(0x01)
-#define SPD_MODULETYPE_UDIMM		(0x02)
-#define SPD_MODULETYPE_SODIMM		(0x03)
-#define SPD_MODULETYPE_MICRODIMM	(0x04)
-#define SPD_MODULETYPE_MINIRDIMM	(0x05)
-#define SPD_MODULETYPE_MINIUDIMM	(0x06)
+/* DIMM Type for DDR2 SPD (according to v1.3) */
+#define DDR2_SPD_DIMMTYPE_UNDEFINED	(0x00)
+#define DDR2_SPD_DIMMTYPE_RDIMM		(0x01)
+#define DDR2_SPD_DIMMTYPE_UDIMM		(0x02)
+#define DDR2_SPD_DIMMTYPE_SO_DIMM	(0x04)
+#define DDR2_SPD_DIMMTYPE_72B_SO_CDIMM	(0x06)
+#define DDR2_SPD_DIMMTYPE_72B_SO_RDIMM	(0x07)
+#define DDR2_SPD_DIMMTYPE_MICRO_DIMM	(0x08)
+#define DDR2_SPD_DIMMTYPE_MINI_RDIMM	(0x10)
+#define DDR2_SPD_DIMMTYPE_MINI_UDIMM	(0x20)
+
+/* Byte 3 Key Byte / Module Type for DDR3 SPD */
+#define DDR3_SPD_MODULETYPE_MASK	(0x0f)
+#define DDR3_SPD_MODULETYPE_RDIMM	(0x01)
+#define DDR3_SPD_MODULETYPE_UDIMM	(0x02)
+#define DDR3_SPD_MODULETYPE_SO_DIMM	(0x03)
+#define DDR3_SPD_MODULETYPE_MICRO_DIMM	(0x04)
+#define DDR3_SPD_MODULETYPE_MINI_RDIMM	(0x05)
+#define DDR3_SPD_MODULETYPE_MINI_UDIMM	(0x06)
 
 #endif /* _DDR_SPD_H_ */