Patchwork add SPD address mapping to i945

login
register
about
Submitter Sven Schnelle
Date 2011-02-18 11:09:18
Message ID <1298027358-30009-1-git-send-email-svens@stackframe.org>
Download mbox | patch
Permalink /patch/2678/
State Superseded
Headers show

Comments

Sven Schnelle - 2011-02-18 11:09:18
The current code works only with dual channel if Channel 0 uses SPD address
0x50/0x51, while the second channel has to use 0x52/0x53.

For hardware that uses other addresses (like the ThinkPad X60) this means we
get only one module running instead of both.

This patch adds a second parameter to sdram_initialize, which is an array with
2 * DIMM_SOCKETS members. It should contain the SPD addresses for every single
DIMM socket. If NULL is given as the second parameter, the code uses the old
addressing scheme.
---
 src/mainboard/getac/p470/romstage.c       |    2 +-
 src/mainboard/ibase/mb899/romstage.c      |    2 +-
 src/mainboard/intel/d945gclf/romstage.c   |    2 +-
 src/mainboard/kontron/986lcd-m/romstage.c |    2 +-
 src/mainboard/lenovo/x60/romstage.c       |    3 +-
 src/mainboard/roda/rk886ex/romstage.c     |    2 +-
 src/northbridge/intel/i945/raminit.c      |   75 +++++++++++++++--------------
 src/northbridge/intel/i945/raminit.h      |    3 +-
 8 files changed, 47 insertions(+), 44 deletions(-)
Patrick Georgi - 2011-02-18 11:21:59
Am Freitag, den 18.02.2011, 12:09 +0100 schrieb Sven Schnelle:
> The current code works only with dual channel if Channel 0 uses SPD address
> 0x50/0x51, while the second channel has to use 0x52/0x53.
> 
> For hardware that uses other addresses (like the ThinkPad X60) this means we
> get only one module running instead of both.
> 
> This patch adds a second parameter to sdram_initialize, which is an array with
> 2 * DIMM_SOCKETS members. It should contain the SPD addresses for every single
> DIMM socket. If NULL is given as the second parameter, the code uses the old
> addressing scheme.
Awww...

Please keep spd_read_byte alone, we have a somewhat unified API there
across the chipsets (and I hope to clean it up even more at some point,
see http://www.coreboot.org/Infrastructure_Projects#Refactor_SMBUS_code)

Maybe you could create a function like

int get_dimm_no(struct sys_info *sys_info, int device) {
 if (sys_info->spd_addresses)
  return sys_info->spd_addresses[device];
 else
  return DIMM0 + device;
}

and use that to determine the addresses where currently "DIMM0+i" is
used instead?

While this would be changed in a clean-up like referred to above, it
would be less of an effort than to essentially undo your changes.

That should give you the same result with not changing any internal
APIs.


Thanks,
Patrick

Patch

diff --git a/src/mainboard/getac/p470/romstage.c b/src/mainboard/getac/p470/romstage.c
index 270a7bd..83cd91e 100644
--- a/src/mainboard/getac/p470/romstage.c
+++ b/src/mainboard/getac/p470/romstage.c
@@ -332,7 +332,7 @@  void main(unsigned long bist)
 	dump_spd_registers();
 #endif
 
-	sdram_initialize(boot_mode);
+	sdram_initialize(boot_mode, NULL);
 
 	/* Perform some initialization that must run before stage2 */
 	early_ich7_init();
diff --git a/src/mainboard/ibase/mb899/romstage.c b/src/mainboard/ibase/mb899/romstage.c
index d3a0299..e248a27 100644
--- a/src/mainboard/ibase/mb899/romstage.c
+++ b/src/mainboard/ibase/mb899/romstage.c
@@ -283,7 +283,7 @@  void main(unsigned long bist)
 	dump_spd_registers();
 #endif
 
-	sdram_initialize(boot_mode);
+	sdram_initialize(boot_mode, NULL);
 
 	/* Perform some initialization that must run before stage2 */
 	early_ich7_init();
diff --git a/src/mainboard/intel/d945gclf/romstage.c b/src/mainboard/intel/d945gclf/romstage.c
index f705673..6dfc144 100644
--- a/src/mainboard/intel/d945gclf/romstage.c
+++ b/src/mainboard/intel/d945gclf/romstage.c
@@ -243,7 +243,7 @@  void main(unsigned long bist)
 	dump_spd_registers();
 #endif
 
-	sdram_initialize(boot_mode);
+	sdram_initialize(boot_mode, NULL);
 
 	/* Perform some initialization that must run before stage2 */
 	early_ich7_init();
diff --git a/src/mainboard/kontron/986lcd-m/romstage.c b/src/mainboard/kontron/986lcd-m/romstage.c
index 925c93e..2cf73e6 100644
--- a/src/mainboard/kontron/986lcd-m/romstage.c
+++ b/src/mainboard/kontron/986lcd-m/romstage.c
@@ -382,7 +382,7 @@  void main(unsigned long bist)
 	dump_spd_registers();
 #endif
 
-	sdram_initialize(boot_mode);
+	sdram_initialize(boot_mode, NULL);
 
 	/* Perform some initialization that must run before stage2 */
 	early_ich7_init();
diff --git a/src/mainboard/lenovo/x60/romstage.c b/src/mainboard/lenovo/x60/romstage.c
index 5578324..e2b4a40 100644
--- a/src/mainboard/lenovo/x60/romstage.c
+++ b/src/mainboard/lenovo/x60/romstage.c
@@ -294,6 +294,7 @@  void main(unsigned long bist)
 {
 	u32 reg32;
 	int boot_mode = 0;
+	const u8 spd_addrmap[2 * DIMM_SOCKETS] = { 0x50, 0x52, 0x51, 0x53 };
 
 	if (bist == 0)
 		enable_lapic();
@@ -357,7 +358,7 @@  void main(unsigned long bist)
 	dump_spd_registers();
 #endif
 
-	sdram_initialize(boot_mode);
+	sdram_initialize(boot_mode, spd_addrmap);
 
 	/* Perform some initialization that must run before stage2 */
 	early_ich7_init();
diff --git a/src/mainboard/roda/rk886ex/romstage.c b/src/mainboard/roda/rk886ex/romstage.c
index fce53da..0797699 100644
--- a/src/mainboard/roda/rk886ex/romstage.c
+++ b/src/mainboard/roda/rk886ex/romstage.c
@@ -320,7 +320,7 @@  void main(unsigned long bist)
 	dump_spd_registers();
 #endif
 
-	sdram_initialize(boot_mode);
+	sdram_initialize(boot_mode, NULL);
 
 	/* Perform some initialization that must run before stage2 */
 	early_ich7_init();
diff --git a/src/northbridge/intel/i945/raminit.c b/src/northbridge/intel/i945/raminit.c
index 1c93435..0970e87 100644
--- a/src/northbridge/intel/i945/raminit.c
+++ b/src/northbridge/intel/i945/raminit.c
@@ -54,9 +54,12 @@  struct cbmem_entry *get_cbmem_toc(void)
 #define RAM_EMRS_2			(0x1 << 21)
 #define RAM_EMRS_3			(0x2 << 21)
 
-static inline int spd_read_byte(unsigned device, unsigned address)
+static inline int spd_read_byte(struct sys_info *sys_info, unsigned device, unsigned address)
 {
-	return smbus_read_byte(device, address);
+	if (sys_info->spd_addresses)
+		return smbus_read_byte(sys_info->spd_addresses[device], address);
+	else
+		return smbus_read_byte(DIMM0 + device, address);
 }
 
 static __attribute__((noinline)) void do_ram_command(u32 command)
@@ -367,7 +370,7 @@  static void sdram_get_dram_configuration(struct sys_info *sysinfo)
 	 */
 
 	for (i=0; i<(2 * DIMM_SOCKETS); i++) {
-		u8 reg8, device = DIMM0 + i;
+		u8 reg8;
 
 		/* Initialize the socket information with a sane value */
 		sysinfo->dimm[i] = SYSINFO_DIMM_NOT_POPULATED;
@@ -382,24 +385,24 @@  static void sdram_get_dram_configuration(struct sys_info *sysinfo)
 
 		printk(BIOS_DEBUG, "DDR II Channel %d Socket %d: ", (i >> 1), (i & 1));
 
-		if (spd_read_byte(device, SPD_MEMORY_TYPE) != SPD_MEMORY_TYPE_SDRAM_DDR2) {
+		if (spd_read_byte(sysinfo, i, SPD_MEMORY_TYPE) != SPD_MEMORY_TYPE_SDRAM_DDR2) {
 			printk(BIOS_DEBUG, "N/A\n");
 			continue;
 		}
 
-		reg8 = spd_read_byte(device, SPD_DIMM_CONFIG_TYPE);
+		reg8 = spd_read_byte(sysinfo, i, SPD_DIMM_CONFIG_TYPE);
 		if (reg8 == ERROR_SCHEME_ECC)
 			die("Error: ECC memory not supported by this chipset\n");
 
-		reg8 = spd_read_byte(device, SPD_MODULE_ATTRIBUTES);
+		reg8 = spd_read_byte(sysinfo, i, SPD_MODULE_ATTRIBUTES);
 		if (reg8 & MODULE_BUFFERED)
 			die("Error: Buffered memory not supported by this chipset\n");
 		if (reg8 & MODULE_REGISTERED)
 			die("Error: Registered memory not supported by this chipset\n");
 
-		switch (spd_read_byte(device, SPD_PRIMARY_SDRAM_WIDTH)) {
+		switch (spd_read_byte(sysinfo, i, SPD_PRIMARY_SDRAM_WIDTH)) {
 		case 0x08:
-			switch (spd_read_byte(device, SPD_NUM_DIMM_BANKS) & 0x0f) {
+			switch (spd_read_byte(sysinfo, i, SPD_NUM_DIMM_BANKS) & 0x0f) {
 			case 1:
 				printk(BIOS_DEBUG, "x8DDS\n");
 				sysinfo->dimm[i] = SYSINFO_DIMM_X8DDS;
@@ -413,7 +416,7 @@  static void sdram_get_dram_configuration(struct sys_info *sysinfo)
 			}
 			break;
 		case 0x10:
-			switch (spd_read_byte(device, SPD_NUM_DIMM_BANKS) & 0x0f) {
+			switch (spd_read_byte(sysinfo, i, SPD_NUM_DIMM_BANKS) & 0x0f) {
 			case 1:
 				printk(BIOS_DEBUG, "x16DS\n");
 				sysinfo->dimm[i] = SYSINFO_DIMM_X16DS;
@@ -458,7 +461,7 @@  static void sdram_verify_package_type(struct sys_info * sysinfo)
 			continue;
 
 		/* Is the current DIMM a stacked DIMM? */
-		if (spd_read_byte(DIMM0 + i, SPD_NUM_DIMM_BANKS) & (1 << 4))
+		if (spd_read_byte(sysinfo, i, SPD_NUM_DIMM_BANKS) & (1 << 4))
 			sysinfo->package = 1;
 	}
 }
@@ -475,7 +478,7 @@  static u8 sdram_possible_cas_latencies(struct sys_info * sysinfo)
 
 	for (i=0; i<2*DIMM_SOCKETS; i++) {
 		if (sysinfo->dimm[i] != SYSINFO_DIMM_NOT_POPULATED)
-			cas_mask &= spd_read_byte(DIMM0 + i, SPD_ACCEPTABLE_CAS_LATENCIES);
+			cas_mask &= spd_read_byte(sysinfo, i, SPD_ACCEPTABLE_CAS_LATENCIES);
 	}
 
 	if(!cas_mask) {
@@ -536,7 +539,7 @@  static void sdram_detect_cas_latency_and_ram_speed(struct sys_info * sysinfo, u8
 				continue;
 			}
 
-			current_cas_mask = spd_read_byte(DIMM0 + i, SPD_ACCEPTABLE_CAS_LATENCIES);
+			current_cas_mask = spd_read_byte(sysinfo, i, SPD_ACCEPTABLE_CAS_LATENCIES);
 
 			while (current_cas_mask) {
 				int highest_supported_cas = 0, current_cas = 0;
@@ -562,11 +565,11 @@  static void sdram_detect_cas_latency_and_ram_speed(struct sys_info * sysinfo, u8
 
 				idx = highest_supported_cas - current_cas;
 				PRINTK_DEBUG("idx=%d, ", idx);
-				PRINTK_DEBUG("tCLK=%x, ", spd_read_byte(DIMM0 + i, spd_lookup_table[2*idx]));
-				PRINTK_DEBUG("tAC=%x", spd_read_byte(DIMM0 + i, spd_lookup_table[(2*idx)+1]));
+				PRINTK_DEBUG("tCLK=%x, ", spd_read_byte(sysinfo, i, spd_lookup_table[2*idx]));
+				PRINTK_DEBUG("tAC=%x", spd_read_byte(sysinfo, i, spd_lookup_table[(2*idx)+1]));
 
-				if (spd_read_byte(DIMM0 + i, spd_lookup_table[2*idx]) <= ddr2_speeds_table[2*j] &&
-						spd_read_byte(DIMM0 + i, spd_lookup_table[(2*idx)+1]) <= ddr2_speeds_table[(2*j)+1]) {
+				if (spd_read_byte(sysinfo, i, spd_lookup_table[2*idx]) <= ddr2_speeds_table[2*j] &&
+						spd_read_byte(sysinfo, i, spd_lookup_table[(2*idx)+1]) <= ddr2_speeds_table[(2*j)+1]) {
 					PRINTK_DEBUG(":    OK\n");
 					break;
 				}
@@ -630,7 +633,7 @@  static void sdram_detect_smallest_tRAS(struct sys_info * sysinfo)
 		if (sysinfo->dimm[i] == SYSINFO_DIMM_NOT_POPULATED)
 			continue;
 
-		reg8 = spd_read_byte(DIMM0 + i, SPD_MIN_ACTIVE_TO_PRECHARGE_DELAY);
+		reg8 = spd_read_byte(sysinfo, i, SPD_MIN_ACTIVE_TO_PRECHARGE_DELAY);
 		if (!reg8) {
 			die("Invalid tRAS value.\n");
 		}
@@ -670,7 +673,7 @@  static void sdram_detect_smallest_tRP(struct sys_info * sysinfo)
 		if (sysinfo->dimm[i] == SYSINFO_DIMM_NOT_POPULATED)
 			continue;
 
-		reg8 = spd_read_byte(DIMM0 + i, SPD_MIN_ROW_PRECHARGE_TIME);
+		reg8 = spd_read_byte(sysinfo, i, SPD_MIN_ROW_PRECHARGE_TIME);
 		if (!reg8) {
 			die("Invalid tRP value.\n");
 		}
@@ -711,7 +714,7 @@  static void sdram_detect_smallest_tRCD(struct sys_info * sysinfo)
 		if (sysinfo->dimm[i] == SYSINFO_DIMM_NOT_POPULATED)
 			continue;
 
-		reg8 = spd_read_byte(DIMM0 + i, SPD_MIN_RAS_TO_CAS_DELAY);
+		reg8 = spd_read_byte(sysinfo, i, SPD_MIN_RAS_TO_CAS_DELAY);
 		if (!reg8) {
 			die("Invalid tRCD value.\n");
 		}
@@ -751,7 +754,7 @@  static void sdram_detect_smallest_tWR(struct sys_info * sysinfo)
 		if (sysinfo->dimm[i] == SYSINFO_DIMM_NOT_POPULATED)
 			continue;
 
-		reg8 = spd_read_byte(DIMM0 + i, SPD_WRITE_RECOVERY_TIME);
+		reg8 = spd_read_byte(sysinfo, i, SPD_WRITE_RECOVERY_TIME);
 		if (!reg8) {
 			die("Invalid tWR value.\n");
 		}
@@ -832,7 +835,7 @@  static void sdram_detect_smallest_refresh(struct sys_info * sysinfo)
 		if (sysinfo->dimm[i] == SYSINFO_DIMM_NOT_POPULATED)
 			continue;
 
-		refresh = spd_read_byte(DIMM0 + i, SPD_REFRESH) & ~(1 << 7);
+		refresh = spd_read_byte(sysinfo, i, SPD_REFRESH) & ~(1 << 7);
 
 		/* 15.6us */
 		if (!refresh)
@@ -860,7 +863,7 @@  static void sdram_verify_burst_length(struct sys_info * sysinfo)
 		if (sysinfo->dimm[i] == SYSINFO_DIMM_NOT_POPULATED)
 			continue;
 
-		if (!(spd_read_byte(DIMM0 + i, SPD_SUPPORTED_BURST_LENGTHS) & SPD_BURST_LENGTH_8))
+		if (!(spd_read_byte(sysinfo, i, SPD_SUPPORTED_BURST_LENGTHS) & SPD_BURST_LENGTH_8))
 			die("Only DDR-II RAM with burst length 8 is supported by this chipset.\n");
 	}
 }
@@ -1392,7 +1395,7 @@  struct dimm_size {
 	unsigned long side2;
 };
 
-static struct dimm_size sdram_get_dimm_size(u16 device)
+static struct dimm_size sdram_get_dimm_size(struct sys_info *sysinfo, u16 device)
 {
 	/* Calculate the log base 2 size of a DIMM in bits */
 	struct dimm_size sz;
@@ -1401,35 +1404,35 @@  static struct dimm_size sdram_get_dimm_size(u16 device)
 	sz.side1 = 0;
 	sz.side2 = 0;
 
-	rows = spd_read_byte(device, SPD_NUM_ROWS);	/* rows */
+	rows = spd_read_byte(sysinfo, device, SPD_NUM_ROWS);	/* rows */
 	if (rows < 0) goto hw_err;
 	if ((rows & 0xf) == 0) goto val_err;
 	sz.side1 += rows & 0xf;
 
-	columns = spd_read_byte(device, SPD_NUM_COLUMNS);	/* columns */
+	columns = spd_read_byte(sysinfo, device, SPD_NUM_COLUMNS);	/* columns */
 	if (columns < 0) goto hw_err;
 	if ((columns & 0xf) == 0) goto val_err;
 	sz.side1 += columns & 0xf;
 
-	value = spd_read_byte(device, SPD_NUM_BANKS_PER_SDRAM);	/* banks */
+	value = spd_read_byte(sysinfo, device, SPD_NUM_BANKS_PER_SDRAM);	/* banks */
 	if (value < 0) goto hw_err;
 	if ((value & 0xff) == 0) goto val_err;
 	sz.side1 += log2(value & 0xff);
 
 	/* Get the module data width and convert it to a power of two */
-	value = spd_read_byte(device, SPD_MODULE_DATA_WIDTH_MSB);	/* (high byte) */
+	value = spd_read_byte(sysinfo, device, SPD_MODULE_DATA_WIDTH_MSB);	/* (high byte) */
 	if (value < 0) goto hw_err;
 	value &= 0xff;
 	value <<= 8;
 
-	low = spd_read_byte(device, SPD_MODULE_DATA_WIDTH_LSB);	/* (low byte) */
+	low = spd_read_byte(sysinfo, device, SPD_MODULE_DATA_WIDTH_LSB);	/* (low byte) */
 	if (low < 0) goto hw_err;
 	value = value | (low & 0xff);
 	if ((value != 72) && (value != 64)) goto val_err;
 	sz.side1 += log2(value);
 
 	/* side 2 */
-	value = spd_read_byte(device, SPD_NUM_DIMM_BANKS);	/* number of physical banks */
+	value = spd_read_byte(sysinfo, device, SPD_NUM_DIMM_BANKS);	/* number of physical banks */
 
 	if (value < 0) goto hw_err;
 	value &= 7;
@@ -1480,9 +1483,9 @@  static void sdram_detect_dimm_size(struct sys_info * sysinfo)
 		if (sysinfo->dimm[i] == SYSINFO_DIMM_NOT_POPULATED)
 			continue;
 
-		sz = sdram_get_dimm_size(DIMM0 + i);
+		sz = sdram_get_dimm_size(sysinfo, i);
 
-		sysinfo->banks[i] = spd_read_byte(DIMM0 + i, SPD_NUM_BANKS_PER_SDRAM);	/* banks */
+		sysinfo->banks[i] = spd_read_byte(sysinfo, i, SPD_NUM_BANKS_PER_SDRAM);	/* banks */
 
 		if (sz.side1 < 30)
 			die("DDR-II rank size smaller than 128MB is not supported.\n");
@@ -1567,19 +1570,16 @@  static int sdram_set_row_attributes(struct sys_info *sysinfo)
 
 	printk(BIOS_DEBUG, "Setting row attributes... \n");
 	for(i=0; i < 2 * DIMM_SOCKETS; i++) {
-		u16 device;
 		u8 columnsrows;
 
 		if (sysinfo->dimm[i] == SYSINFO_DIMM_NOT_POPULATED) {
 			continue;
 		}
 
-		device = DIMM0 + i;
-
-		value = spd_read_byte(device, SPD_NUM_ROWS);	/* rows */
+		value = spd_read_byte(sysinfo, i, SPD_NUM_ROWS);	/* rows */
 		columnsrows = (value & 0x0f);
 
-		value = spd_read_byte(device, SPD_NUM_COLUMNS);	/* columns */
+		value = spd_read_byte(sysinfo, i, SPD_NUM_COLUMNS);	/* columns */
 		columnsrows |= (value & 0xf) << 4;
 
 		switch (columnsrows) {
@@ -3039,7 +3039,7 @@  static void sdram_setup_processor_side(void)
 /**
  * @param boot_path: 0 = normal, 1 = reset, 2 = resume from s3
  */
-void sdram_initialize(int boot_path)
+void sdram_initialize(int boot_path, const u8 *spd_addresses)
 {
 	struct sys_info sysinfo;
 	u8 reg8, cas_mask;
@@ -3049,6 +3049,7 @@  void sdram_initialize(int boot_path)
 	memset(&sysinfo, 0, sizeof(sysinfo));
 
 	sysinfo.boot_path = boot_path;
+	sysinfo.spd_addresses = spd_addresses;
 
 	/* Look at the type of DIMMs and verify all DIMMs are x8 or x16 width */
 	sdram_get_dram_configuration(&sysinfo);
diff --git a/src/northbridge/intel/i945/raminit.h b/src/northbridge/intel/i945/raminit.h
index ede194c..4f77e8e 100644
--- a/src/northbridge/intel/i945/raminit.h
+++ b/src/northbridge/intel/i945/raminit.h
@@ -63,11 +63,12 @@  struct sys_info {
 	u8 banks[2 * DIMM_SOCKETS];
 
 	u8 banksize[2 * 2 * DIMM_SOCKETS];
+	const u8 *spd_addresses;
 
 } __attribute__ ((packed));
 
 void receive_enable_adjust(struct sys_info *sysinfo);
-void sdram_initialize(int boot_path);
+void sdram_initialize(int boot_path, const u8 *sdram_addresses);
 unsigned long get_top_of_ram(void);
 int fixup_i945_errata(void);
 void udelay(u32 us);