Re: [coreboot] [PATCH] add SPD address mapping to i945

[Georgi, Patrick]

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