[LinuxBIOS] DIMM Page Size

Peter Stuge peter at stuge.se
Fri Jul 13 12:16:48 CEST 2007


On Fri, Jul 13, 2007 at 01:44:52AM -0400, Joseph Smith wrote:
> 			/* convert to Kilobytes */
> 			dra = ((page_size / 8) >> 10);

Please don't use bit shifting for multiplication and division.

page_size / 8 / 1024

is nicer to read, and the compiler will probably optimize both
statements to a single >> 13 anyway.

Note also that you're now using the dra variable to hold something
very different from the DRA value, while the rest of the function
pretty much has one variable for each register. Variables (ie
registers) are scarce during RAM init so I think re-use is good.


> 			/* 2KB */
> 			if (dra == 0x2) {
> 				dra = 0xF0;
> 			/* 4KB */
> 			} else if (dra == 0x4) {
> 				dra = 0xF1;
> 			/* 8KB */
> 			} else if (dra == 0x8) {
> 				dra = 0xF2;
> 			/* 16KB */
> 			} else if (dra == 0x10) {
> 				dra = 0xF3;
> 			} else {
> 				print_debug("Page Size not supported\r\n");
> 				die("HALT!\r\n");
> 			}

This could also be a switch:

switch(dra) {
case 2:
  dra=0xf0;
  break;
case 4:
  dra=0xf1;
  break;
case 8:
  dra=0xf2;
  break;
case 16:
  dra=0xf3;
  break;
default:
  print_debug("incompatible\n");
  die("bye");
}

But I think this is simpler:

dra = log2(dra);
if (dra > 3) {
  print_debug("incompatible\n");
  die("bye");
}
dra |= 0xf0;


//Peter




More information about the coreboot mailing list