On Feb 1, 2011, at 4:08 AM, Timur Tabi wrote:
On Mon, Jan 31, 2011 at 11:28 PM, Kumar Gala galak@kernel.crashing.org wrote:
+#if defined(SPD_EEPROM_ADDRESS) || \
- defined(SPD_EEPROM_ADDRESS1) || defined(SPD_EEPROM_ADDRESS2) || \
- defined(SPD_EEPROM_ADDRESS3) || defined(SPD_EEPROM_ADDRESS4)
+#if (CONFIG_NUM_DDR_CONTROLLERS == 1) && (CONFIG_DIMM_SLOTS_PER_CTLR == 1) +u8 spd_i2c_addr[CONFIG_NUM_DDR_CONTROLLERS][CONFIG_DIMM_SLOTS_PER_CTLR] = {
[0][0] = SPD_EEPROM_ADDRESS,+}; +#endif +#if (CONFIG_NUM_DDR_CONTROLLERS == 2) && (CONFIG_DIMM_SLOTS_PER_CTLR == 1) +u8 spd_i2c_addr[CONFIG_NUM_DDR_CONTROLLERS][CONFIG_DIMM_SLOTS_PER_CTLR] = {
[0][0] = SPD_EEPROM_ADDRESS1, /* controller 1 */[1][0] = SPD_EEPROM_ADDRESS2, /* controller 2 */+}; +#endif +#if (CONFIG_NUM_DDR_CONTROLLERS == 2) && (CONFIG_DIMM_SLOTS_PER_CTLR == 2) +u8 spd_i2c_addr[CONFIG_NUM_DDR_CONTROLLERS][CONFIG_DIMM_SLOTS_PER_CTLR] = {
[0][0] = SPD_EEPROM_ADDRESS1, /* controller 1 */[0][1] = SPD_EEPROM_ADDRESS2, /* controller 1 */[1][0] = SPD_EEPROM_ADDRESS3, /* controller 2 */[1][1] = SPD_EEPROM_ADDRESS4, /* controller 2 */+}; +#endif
Wouldn't it be easier if we just temporarily defined values for SPD_EEPROM_ADDRESSx? Like this:
#ifndef SPD_EEPROM_ADDRESS2 #define SPD_EEPROM_ADDRESS2 0 #endif ...
u8 spd_i2c_addr[2][2] = {
This is problematic because because if you notice SPD_EEPROM_ADDRESS2 is used for both controller 2 / dimm 1 & controller 1 / dimm 2.
+static void __get_spd(generic_spd_eeprom_t *spd, u8 i2c_address) +{
int ret = i2c_read(i2c_address, 0, 1, (uchar *)spd,sizeof(generic_spd_eeprom_t));if (ret) {debug("DDR: failed to read SPD from address %u\n", i2c_address);This should probably be a printf() instead.
will change
- k