diff mbox

[U-Boot,2/7] mpc8xxx: DDR2/3: Use human-readable SPD DIMM-type constants

Message ID 1298311199-18775-3-git-send-email-Kyle.D.Moffett@boeing.com
State Changes Requested
Headers show

Commit Message

Kyle Moffett Feb. 21, 2011, 5:59 p.m. UTC 
Use #define constants to enhance readability of DDR2/3 SPD parsing code.
Also add the DDR2 type for an SO-RDIMM module to the switch statement.

Signed-off-by: Kyle Moffett <Kyle.D.Moffett@boeing.com>
---
 arch/powerpc/cpu/mpc8xxx/ddr/ddr2_dimm_params.c |   34 +++++++++++++++------
 arch/powerpc/cpu/mpc8xxx/ddr/ddr3_dimm_params.c |   37 ++++++++++-------------
 common/ddr_spd.c                                |    2 +-
 include/ddr_spd.h                               |   28 ++++++++++++-----
 4 files changed, 60 insertions(+), 41 deletions(-)

Comments

Wolfgang Denk Feb. 21, 2011, 9:03 p.m. UTC | #1 
Dear Kyle Moffett,

In message <1298311199-18775-3-git-send-email-Kyle.D.Moffett@boeing.com> you wrote:
> Use #define constants to enhance readability of DDR2/3 SPD parsing code.
> Also add the DDR2 type for an SO-RDIMM module to the switch statement.
> 
> Signed-off-by: Kyle Moffett <Kyle.D.Moffett@boeing.com>
> ---
>  arch/powerpc/cpu/mpc8xxx/ddr/ddr2_dimm_params.c |   34 +++++++++++++++------
>  arch/powerpc/cpu/mpc8xxx/ddr/ddr3_dimm_params.c |   37 ++++++++++-------------
>  common/ddr_spd.c                                |    2 +-
>  include/ddr_spd.h                               |   28 ++++++++++++-----
>  4 files changed, 60 insertions(+), 41 deletions(-)
...
> +	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 */
> +		pdimm->registered_dimm = 0;
> +		pdimm->mirrored_dimm = spd->mod_section.unbuffered.addr_mapping & 0x1;
>  		break;

Line too long.  Please fix globally.

Best regards,

Wolfgang Denk
Kyle Moffett Feb. 21, 2011, 9:45 p.m. UTC | #2 
On Feb 21, 2011, at 16:03, Wolfgang Denk wrote:
> In message <1298311199-18775-3-git-send-email-Kyle.D.Moffett@boeing.com> you wrote:
>> Use #define constants to enhance readability of DDR2/3 SPD parsing code.
>> Also add the DDR2 type for an SO-RDIMM module to the switch statement.
>> 
>> Signed-off-by: Kyle Moffett <Kyle.D.Moffett@boeing.com>
>> ---
>> arch/powerpc/cpu/mpc8xxx/ddr/ddr2_dimm_params.c |   34 +++++++++++++++------
>> arch/powerpc/cpu/mpc8xxx/ddr/ddr3_dimm_params.c |   37 ++++++++++-------------
>> common/ddr_spd.c                                |    2 +-
>> include/ddr_spd.h                               |   28 ++++++++++++-----
>> 4 files changed, 60 insertions(+), 41 deletions(-)
> ...
>> +	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 */
>> +		pdimm->registered_dimm = 0;
>> +		pdimm->mirrored_dimm = spd->mod_section.unbuffered.addr_mapping & 0x1;
>> 		break;
> 
> Line too long.  Please fix globally.

Ok, will fix.  Although, this one is only 87 characters and some of the other lines already in that file are 93 characters long.

Cheers,
Kyle Moffett
Wolfgang Denk Feb. 21, 2011, 9:57 p.m. UTC | #3 
Dear "Moffett, Kyle D",

In message <325E6BE9-3FCD-4798-813A-26CD3CCC5954@boeing.com> you wrote:
>
> > Line too long.  Please fix globally.
> 
> Ok, will fix.  Although, this one is only 87 characters and some of the =
> other lines already in that file are 93 characters long.

It would be nice if you could provide a separate patch (to go
completely separate, or earlier in the series) to fix the other lines
as well.  Thanks.

Best regards,

Wolfgang Denk
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..d1f0f2b 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..e31f201 100644
--- a/arch/powerpc/cpu/mpc8xxx/ddr/ddr3_dimm_params.c
+++ b/arch/powerpc/cpu/mpc8xxx/ddr/ddr3_dimm_params.c
@@ -128,24 +128,30 @@  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 */
+	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;
+		pdimm->mirrored_dimm = 0;
 		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;
 		}
 		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 */
+		pdimm->registered_dimm = 0;
+		pdimm->mirrored_dimm = spd->mod_section.unbuffered.addr_mapping & 0x1;
 		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 +309,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_ */