Re: [LinuxBIOS] DIMM Page Size

[Peter Stuge]

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