Sorry it's taken so long to respond to this.
I propose keeping SPD_xxx for byte addresses that are common to DDR and DDR2, and using SPD_DDR_xxx and SPD_DDR2_xxx for addresses that are defined differently or are unique to DDR and DDR2. Since in many cases, JEDEC kept the same byte address description for DDR2 but changed the interpretation of the bits, I also propose qualifying all of the field/bit-definitions with DDR_ or DDR2_, even if the interpretation happens to be the same. The only exception would be the memory type values, which have to be common to all memory types.
The reason for keeping SPD_xxx for byte addresses common to DDR and DDR2 is that code that supports both (i.e. E7520) might get confusing to read if we didn't do this. For example, that code reads parameters like the number of columns only once, so calling the byte address SPD_DDR_NUM_COLUMNS or SPD_DDR2_NUM_COLUMNS
Does this sound like a reasonable compromise? The spd.h file would become something like this:
#ifndef __SPD_H_DEFINED #define __SPD_H_DEFINED
// Byte numbers // (Essentially) common to DDR & DDR2 #define SPD_MEMORY_TYPE 2 #define SPD_NUM_ROWS 3 #define SPD_NUM_COLUMNS 4 #define SPD_MODULE_VOLTAGE 8 #define SPD_MIN_CYCLE_TIME_AT_CAS_MAX 9 #define SPD_DIMM_CONFIG_TYPE 11 #define SPD_REFRESH 12 #define SPD_PRIMARY_DRAM_WIDTH 13 #define SPD_SUPPORTED_BURST_LENGTHS 16 #define SPD_NUM_BANKS_PER_DRAM 17 #define SPD_ACCEPTABLE_CAS_LATENCIES 18 #define SPD_MODULE_ATTRIBUTES 21 #define SPD_MIN_ROW_PRECHARGE_TIME 27 #define SPD_MIN_ROW_ACTIVE_DELAY 28 #define SPD_MIN_RAS_TO_CAS_DELAY 29 #define SPD_MIN_ACTIVE_TO_PRECHARGE_DELAY 30 #define SPD_ADDRESS_CMD_HOLD 33 #define SPD_MIN_ACTIVE_TO_ACTIVE_TIME 41 #define SPD_MIN_AUTO_REFRESH_CMD_PERIOD 42
// DDR-specific #define SPD_DDR_NUM_DIMM_BANKS 5 #define SPD_DDR_MODULE_DATA_WIDTH_LSB 6 #define SPD_DDR_MODULE_DATA_WIDTH_MSB 7 #define SPD_DDR_MIN_CYCLE_TIME_AT_CAS_REDUCED_05 23 #define SPD_DDR_MIN_CYCLE_TIME_AT_CAS_REDUCED_10 25
// DDR2-specific #define SPD_DDR2_NUM_DIMM_RANKS 5 #define SPD_DDR2_MODULE_DATA_WIDTH 6 #define SPD_DDR2_MIN_CYCLE_TIME_AT_CAS_REDUCED_1 23 #define SPD_DDR2_MIN_CYCLE_TIME_AT_CAS_REDUCED_2 25
/****************************************************/ // SPD_MEMORY_TYPE values #define MEMORY_TYPE_SDRAM_DDR 7 #define MEMORY_TYPE_DDR2_SDRAM 8
// SPD_MODULE_VOLTAGE values #define DDR_VOLTAGE_SSTL2_5 4 #define DDR2_VOLTAGE_SSTL2_5 4
// SPD_DIMM_CONFIG_TYPE values #define DDR_ERROR_SCHEME_NONE 0 #define DDR_ERROR_SCHEME_PARITY 1 #define DDR_ERROR_SCHEME_ECC 2 #define DDR2_DATA_PARITY (1<<0) #define DDR2_DATA_ECC (1<<1) #define DDR2_ADDRESS_PARITY (1<<2)
// SPD_ACCEPTABLE_CAS_LATENCIES values #define DDR_CAS_LATENCY_1_0 0x01 #define DDR_CAS_LATENCY_1_5 0x02 #define DDR_CAS_LATENCY_2_0 0x04 #define DDR_CAS_LATENCY_2_5 0x08 #define DDR_CAS_LATENCY_3_0 0x10 #define DDR_CAS_LATENCY_3_5 0x20 #define DDR_CAS_LATENCY_4_0 0x40 #define DDR2_CAS_LATENCY_2 (1<<2) #define DDR2_CAS_LATENCY_3 (1<<3) #define DDR2_CAS_LATENCY_4 (1<<4) #define DDR2_CAS_LATENCY_5 (1<<5) #define DDR2_CAS_LATENCY_6 (1<<6)
// SPD_SUPPORTED_BURST_LENGTHS values #define DDR_BURST_LENGTH_1 1 #define DDR_BURST_LENGTH_2 2 #define DDR_BURST_LENGTH_4 4 #define DDR_BURST_LENGTH_8 8 #define DDR_BURST_LENGTH_PAGE (1<<7) #define DDR2_BURST_LENGTH_4 4 #define DDR2_BURST_LENGTH_8 8
// SPD_MODULE_ATTRIBUTES values #define DDR_MODULE_BUFFERED 1 #define DDR_MODULE_REGISTERED 2
#endif // __SPD_H_DEFINED
It's a sticky problem, since undoubtedly there will be more changes when the next DRAM technology is standardized, and (theoretically, at least) there can be differences in definitions even between revisions of the standard for a particular memory type. Hopefully a scheme like this will work for awhile, even if it isn't perfect.
------------------------------------------------------------------------ Steven J. Magnani "I claim this network for MARS! www.digidescorp.com Earthling, return my space modulator!"
#include <standard.disclaimer>
-----Original Message----- From: Lu, Yinghai [mailto:yinghai.lu@amd.com] Sent: Wednesday, September 14, 2005 1:11 PM To: Steven J. Magnani; linuxbios@openbios.org Subject: RE: [LinuxBIOS] E7501 Raminit rewrite
Every define have SPD_DDR_ prefix, or with ddr_spd.h and SPD_ prefix....
YH
-----Original Message----- From: Steven J. Magnani [mailto:steve@digidescorp.com] Sent: Wednesday, September 14, 2005 11:06 AM To: Lu, Yinghai; linuxbios@openbios.org Subject: RE: [LinuxBIOS] E7501 Raminit rewrite
Can you be more specific about the changes you'd like to see?
Steve
-----Original Message----- From: Lu, Yinghai [mailto:yinghai.lu@amd.com] Sent: Tuesday, September 13, 2005 10:27 AM To: Steven J. Magnani; linuxbios@openbios.org Subject: RE: [LinuxBIOS] E7501 Raminit rewrite
It's good to have spd.h there.
But the name should be changed for support DDR2...etc.
YH
-----Original Message----- From: linuxbios-bounces@openbios.org [mailto:linuxbios-bounces@openbios.org] On Behalf Of Steven J. Magnani Sent: Tuesday, September 13, 2005 8:08 AM To: linuxbios@openbios.org Subject: [LinuxBIOS] E7501 Raminit rewrite
This one ends up being a little more than a "patch". I ended up reorganizing the E7501 raminit code so I could follow it, and fixed some bugs along the way.
Significant logical changes:
* Added support for "fast" (64-clock) refresh * Added code to support remap window for 3 - 4 GB systems * Fixed premature configuration of true row boundaries that resulted in some sections of DRAM not receiving JEDEC commands (see http://openbios.org/pipermail/linuxbios/2005-June/011752.html). * Redefined RCOMP_MMIO so that RCOMP registers can be configured on systems where A20M# is asserted. * Disabled subsystem (vendor) ID configuration * #ifdef'd out suspicious looking code (see http://openbios.org/pipermail/linuxbios/2005-June/011759.html) * Added optional run-time checking of dual-channel compatibility of installed DIMMs * Move JEDEC SPD and SDRAM definitions into reusable #include files
Probably, the rewritten code should be reviewed as if it were new. My hope is that it is now easy enough to follow that this shouldn't be too bad.
The original code is, of course, in Subversion. Since I reordered the functions as part of the rewrite, a 'diff' of of the new code against the original won't tell you much. I've attached a reordered copy of the Subversion code that won't compile, but should be a little easier to diff against the new code so you can see what's changed.
I will hold off on committing this for awhile to give people time to look at it. I know that radical changes are a pain to review, but IMHO the improvement in maintainability is significant.
New files that are part of this 'patch', but not included here (I committed these to Subversion): src/include/spd.h src/include/sdram_mode.h src/include/northbridge/intel/e7501/e7501.h
Thanks, Steve
------------------------------------------------------------------------ Steven J. Magnani "I claim this network for MARS! www.digidescorp.com Earthling, return my space modulator!"
#include <standard.disclaimer>