[coreboot-gerrit] New patch to review for coreboot: nb/intel/x4x: Fix weird `dimm_config` in raminit

[Nico Huber (nico.h@gmx.de)]

Nico Huber (nico.h at gmx.de) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/17587

-gerrit

commit e888304c0545522ef56436c3d445de339007d541
Author: Nico Huber <nico.h at gmx.de>
Date:   Wed Nov 23 23:56:53 2016 +0100

    nb/intel/x4x: Fix weird `dimm_config` in raminit
    
    By shifting the `chan` right instead of left, values were always taken
    from the DIMMs of the first channel. The diff-stat also looks like an
    improvement.
    
    NOT TESTED! I didn't even check if it compiles.
    
    Change-Id: I605eb4f9b04520c51eea9995a2d4a1f050f02ecc
    Signed-off-by: Nico Huber <nico.h at gmx.de>
---
 src/northbridge/intel/x4x/raminit.c | 112 ++++--------------------------------
 src/northbridge/intel/x4x/x4x.h     |   4 ++
 2 files changed, 16 insertions(+), 100 deletions(-)

diff --git a/src/northbridge/intel/x4x/raminit.c b/src/northbridge/intel/x4x/raminit.c
index bc0af04..0867c31 100644
--- a/src/northbridge/intel/x4x/raminit.c
+++ b/src/northbridge/intel/x4x/raminit.c
@@ -140,108 +140,20 @@ static void sdram_read_spds(struct sysinfo *s)
 	}
 
 	FOR_EACH_POPULATED_CHANNEL(s->dimms, chan) {
-		if (s->dimms[chan>>1].sides == 0) {
-			// NC
-			if (s->dimms[(chan>>1) + 1].sides == 0) {
-				// NC/NC
-				s->dimm_config[chan] = 0;
-			} else if (s->dimms[(chan>>1) + 1].sides == 1) {
-				// NC/SS
-				if (s->dimms[(chan>>1) + 1].width == 0) {
-					// NC/8SS
-					s->dimm_config[chan] = 4;
-				} else {
-					// NC/16SS
-					s->dimm_config[chan] = 12;
-				}
+		FOR_EACH_POPULATED_DIMM_IN_CHANNEL(s->dimms, chan, i) {
+			int dimm_config;
+			if (s->dimms[i].ranks == 1) {
+				if (s->dimms[i].width == 0)	/* x8 */
+					dimm_config = 1;
+				else				/* x16 */
+					dimm_config = 3;
 			} else {
-				// NC/DS
-				if (s->dimms[(chan>>1) + 1].width == 0) {
-					// NC/8DS
-					s->dimm_config[chan] = 8;
-				} else {
-					// NOT SUPPORTED
-					die("16DS Not supported\n");
-				}
-			}
-		} else if (s->dimms[chan>>1].sides == 1) {
-			// SS
-			if (s->dimms[(chan>>1) + 1].sides == 0) {
-				// SS/NC
-				if (s->dimms[chan>>1].width == 0) {
-					// 8SS/NC
-					s->dimm_config[chan] = 1;
-				} else {
-					// 16SS/NC
-					s->dimm_config[chan] = 3;
-				}
-			} else if (s->dimms[(chan>>1) + 1].sides == 1) {
-				// SS/SS
-				if (s->dimms[chan>>1].width == 0) {
-					if (s->dimms[(chan>>1) + 1].width == 0) {
-						// 8SS/8SS
-						s->dimm_config[chan] = 5;
-					} else {
-						// 8SS/16SS
-						s->dimm_config[chan] = 13;
-					}
-				} else {
-					if (s->dimms[(chan>>1) + 1].width == 0) {
-						// 16SS/8SS
-						s->dimm_config[chan] = 7;
-					} else {
-						// 16SS/16SS
-						s->dimm_config[chan] = 15;
-					}
-				}
-			} else {
-				// SS/DS
-				if (s->dimms[chan>>1].width == 0) {
-					if (s->dimms[(chan>>1) + 1].width == 0) {
-						// 8SS/8DS
-						s->dimm_config[chan] = 9;
-					} else {
-						die("16DS not supported\n");
-					}
-				} else {
-					if (s->dimms[(chan>>1) + 1].width == 0) {
-						// 16SS/8DS
-						s->dimm_config[chan] = 11;
-					} else {
-						die("16DS not supported\n");
-					}
-				}
-			}
-		} else {
-			// DS
-			if (s->dimms[(chan>>1) + 1].sides == 0) {
-				// DS/NC
-				if (s->dimms[chan>>1].width == 0) {
-					// 8DS/NC
-					s->dimm_config[chan] = 2;
-				} else {
-					die("16DS not supported\n");
-				}
-			} else if (s->dimms[(chan>>1) + 1].sides == 1) {
-				// DS/SS
-				if (s->dimms[chan>>1].width == 0) {
-					if (s->dimms[(chan>>1) + 1].width == 0) {
-						// 8DS/8SS
-						s->dimm_config[chan] = 6;
-					} else {
-						// 8DS/16SS
-						s->dimm_config[chan] = 14;
-					}
-				} else {
-					die("16DS not supported\n");
-				}
-			} else {
-				// DS/DS
-				if (s->dimms[chan>>1].width == 0 && s->dimms[(chan>>1)+1].width == 0) {
-					// 8DS/8DS
-					s->dimm_config[chan] = 10;
-				}
+				if (s->dimms[i].width == 0)	/* x8 */
+					dimm_config = 2;
+				else
+					die("Dual-rank x16 not supported\n");
 			}
+			s->dimm_config[chan] |= dimm_config << (i - chan) * 2;
 		}
 		printk(BIOS_DEBUG, "  Config[CH%d] : %d\n", chan, s->dimm_config[chan]);
 	}
diff --git a/src/northbridge/intel/x4x/x4x.h b/src/northbridge/intel/x4x/x4x.h
index aa5db7d..2d1dee1 100644
--- a/src/northbridge/intel/x4x/x4x.h
+++ b/src/northbridge/intel/x4x/x4x.h
@@ -161,6 +161,10 @@
 	for (idx = 0; idx < TOTAL_DIMMS; ++idx)
 #define FOR_EACH_POPULATED_DIMM(dimms, idx) \
 	FOR_EACH_DIMM(idx) IF_DIMM_POPULATED(dimms, idx)
+#define FOR_EACH_DIMM_IN_CHANNEL(ch, idx) \
+	for (idx = (ch); idx < (ch) + 2; ++idx)
+#define FOR_EACH_POPULATED_DIMM_IN_CHANNEL(dimms, ch, idx) \
+	FOR_EACH_DIMM_IN_CHANNEL(ch, idx) IF_DIMM_POPULATED(dimms, idx)
 #define CHANNEL_IS_POPULATED(dimms, idx) ((dimms[idx<<1].card_type != RAW_CARD_UNPOPULATED) || (dimms[(idx<<1) + 1].card_type != RAW_CARD_UNPOPULATED))
 #define CHANNEL_IS_CARDF(dimms, idx) ((dimms[idx<<1].card_type == 0xf) || (dimms[(idx<<1) + 1].card_type == 0xf))
 #define IF_CHANNEL_POPULATED(dimms, idx) if ((dimms[idx<<1].card_type != RAW_CARD_UNPOPULATED) || (dimms[(idx<<1) + 1].card_type != RAW_CARD_UNPOPULATED))