Re: [coreboot] Patch for enableing 440BX NB to use large memory modules

[Paul Menzel]

Am Montag, den 26.04.2010, 03:01 +0200 schrieb Anders Jenbo:
> 440BX can partially use memory larger then what is supported by seeing
> them as being a max size supported module.
>
> Single sided 512MB (what I used to test), becomes a 128MB.
> A asymmetric 384MB double sided modules, with 256MB on one side and
> 128MB on the other. Becomes two sides with 128MB (a 256MB module).
> This is also enabled in at least some BIOSs (GA-BXC).

Interesting. Maybe add a link to the data sheet and tell what page this
is described on.

I have no way to test or the knowledge to acknowledge this patch. :( I
found some typos in the comments.

Maybe you can also add full stops (.) at the end of sentences.
> The patch also cleans up some whitespace.

This surely make it harder to review in the MUA. You have to apply the
patch and tell diff to ignore whitespace changes to make it easier to
review.
> Index: src/northbridge/intel/i440bx/raminit.c
>
> --- src/northbridge/intel/i440bx/raminit.c      (revision 5496)
> +++ src/northbridge/intel/i440bx/raminit.c      (working copy)

[…]
>   /* It is posible to partialy use larger then supported modeuls by

s/posible/possible/
s/partialy/partially/
s/modeuls/modules/
>    * setting them to a supported size.
>    */
>   if(sz.side1 > 128) {
>           PRINT_DEBUG("Side1 was 0x");
>           PRINT_DEBUG_HEX16(sz.side1);
>           PRINT_DEBUG(" but only 128MB will be used\n");
>           sz.side1 = 128;
>           if(sz.side2 > 128) {
>                   PRINT_DEBUG("Side2 was 0x");
>                   PRINT_DEBUG_HEX16(sz.side2);
>                   PRINT_DEBUG(" but only 128MB will be used\n");
>                   sz.side2 = 128;
>           }
>   }
>   return sz;
>
> }
>  /*
> @@ -724,15 +741,15 @@
>                     PRINT_DEBUG("Found ");
>                     if (value == SPD_MEMORY_TYPE_EDO) {
>                           edosd |= 0x02; 
>                   } else if (value == SPD_MEMORY_TYPE_SDRAM) { 
>                           edosd |= 0x04; 
>                           edosd |= 0x02;
>                   } else if (value == SPD_MEMORY_TYPE_SDRAM) {
>                           edosd |= 0x04;
>                   }
>                   PRINT_DEBUG("DIMM in slot ");
>                   PRINT_DEBUG_HEX8(i);
>                   PRINT_DEBUG("\n");
>
>                   if (edosd == 0x06) { 
>                   if (edosd == 0x06) {
>                           print_err("Mixing EDO/SDRAM unsupported!\n");
>                           die("HALT\n");
>                   }
>
> @@ -752,7 +769,7 @@
>                     /* Data width */
>                     width = spd_read_byte(device, SPD_MODULE_DATA_WIDTH_LSB);
>                   /* Exclude error checking data width from page size calculations */
>                   if (ecc) {
>                           value = spd_read_byte(device,
>
> @@ -792,6 +809,12 @@
>                                         dra = 0x1; /* 4KB */
>                                 } else if (dra == 8) {
>                                         dra = 0x2; /* 8KB */
>                           } else if (dra >= 16) {
>                                   /* Page size larger then supported are
>                                    * set to 8KB to use module partialy.
>                                    */

s/size/sizes/
s/partialy/partially/

Thanks,

Paul