Re: [coreboot] [PATCH] 440BX registered SDRAM support

[Keith Hui]

On Wed, Apr 6, 2011 at 6:19 PM, Idwer Vollering  wrote:
> Can you add a #define for SPD_MEMORY_TYPE_REGISTERED_SDRAM to
> src/include/spd.h as well ? If that is relevant to do, ofcourse.

There isn't a separate "registered SDRAM" type under SPD_MEMORY_TYPE.
The SDRAM module being registered is described in byte 21, as
(SPD_MODULE_ATTRIBUTES & MODULE_REGISTERED).

*snip*
> > /* If any installed DIMM is *not* registered, this system cannot be
> > configured for registered SDRAM.
> > By registered, only the address and control lines need to be, which
> > we can tell by reading SPD byte 21, bit 1.
> > */
> > value = spd_read_byte(device, SPD_MODULE_ATTRIBUTES);
> > PRINT_DEBUG("DIMM is ");
> > if ((value & 0x02) == 0) {

Speaking of which, this should be done to the patch:
(Signed-off-by: Keith Hui )

-			if ((value & 0x02) == 0) {
+			if ((value & MODULE_REGISTERED) == 0) {
> > regsd = 0;
> > PRINT_DEBUG("not ");
> > }
> > PRINT_DEBUG("registered\n");

*snip*
> > /* Set DRAMC[4:3] to proper memory type (EDO/SDRAM).
> > TODO: Registered SDRAM support.
> > */
> > edosd &= 0x07;
> > if (edosd & 0x02) {
> > edosd |= 0x00;
> > } else if (edosd & 0x04) {
> > edosd |= 0x08;
> > /* Set DRAMC[4:3] to proper memory type (EDO/SDRAM/Registered SDRAM). */
> > /* i will be used to set DRAMC[4:3]. */
> > if (regsd && sd) {
> > i = 0x10; // Registered SDRAM
>
> The datasheets says that this are bits: i = 0x2, not 0x10.
> > } else if (sd) {
> > i = 0x08; // SDRAM
>
> i = 0x1, not 0x8

The relevant bits are in bits 4-3 of the register. So it actually is
(2 << 3) and (1 << 3), which becomes what you see. This is done so I
don't have to shift the bits later, which save a few instructions.

Thanks
Keith