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(-)
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
[Missed coreboot List Cc]
Hi Patrick,
thanks for you reply. I've attached a new patch which addresses those issues.
Thanks,
Sven. --
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.
Signed-off-by: Sven Schnelle svens@stackframe.org --- 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 | 57 +++++++++++++++++++---------- src/northbridge/intel/i945/raminit.h | 3 +- 8 files changed, 46 insertions(+), 27 deletions(-)
Am 18.02.2011 14:04, schrieb Sven Schnelle:
thanks for you reply. I've attached a new patch which addresses those issues.
Thanks
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.
Signed-off-by: Sven Schnelle svens@stackframe.org
Acked-by: Patrick Georgi patrick@georgi-clan.de
Sven Schnelle svens@stackframe.org writes:
Hi Patrick,
thanks for you reply. I've attached a new patch which addresses those issues. [..]
r6374 in svn.
Sven.
-
Georgi, Patrick -
Patrick Georgi -
Sven Schnelle