[LinuxBIOS] i82830 raminit.c

Joseph Smith joe at smittys.pointclark.net
Thu Jul 5 18:12:38 CEST 2007


Thanks for the feedback Corey. I have a few questions / comments.
>
>
>> /* DRC[6:4] - SDRAM Mode Select (SMS). */
>> #define RAM_COMMAND_NOP		 0x1
>> #define RAM_COMMAND_PRECHARGE	 0x2
>> #define RAM_COMMAND_MRS		 0x3
>> #define RAM_COMMAND_CBR		 0x6
>> #define RAM_COMMAND_NORMAL	 0x7
>>
>
> Hmm, 0x0 does nothing? Even if you don't use it, just state it for
> clarity's sake please.

What would I define 0x0 as?


>
>> static struct dimm_size spd_get_dimm_size(unsigned device)
>> {
>> 	/* Calculate the log base 2 size of a DIMM in bits */
>> 	struct dimm_size sz;
>> 	int value, low;
>> 	sz.side1 = 0;
>> 	sz.side2 = 0;
>>
>> 	/* test for sdram */
>> 	value = spd_read_byte(device, 2);      /* type */
>>       if (value < 0) goto hw_err;
>> 	if (value != 4) {
>> 		print_debug("SPD2 DIMM Is Not Compatable\r\n");
>> 		goto val_err;
>> 	}
>>
>> 	/* test for PC133 (i830 only supports PC133) */
>> 	value = spd_read_byte(device, 9);      /* cycle time */
>>       if (value < 0) goto hw_err;
>> 	if (value != 75) {
>> 		print_debug("SPD9 DIMM Is Not PC133 Compatable\r\n");
>> 		goto val_err;
>> 	}
>> 	value = 0;
>> 	value = spd_read_byte(device, 10);     /* access time */
>>       if (value < 0) goto hw_err;
>> 	if (value != 54) {
>> 		print_debug("SPD10 DIMM Is Not PC133 Compatable\r\n");
>> 		goto val_err;
>> 	}
>>
>> 	/* Note it might be easier to use byte 31 here, it has the DIMM size as
>> 	 * a multiple of 4MB.  The way we do it now we can size both
>> 	 * sides of an assymetric dimm.
>> 	 */
>> 	value = spd_read_byte(device, 3);	/* rows */
>> 	if (value < 0) goto hw_err;
>> 	if ((value & 0xf) == 0) {
>> 		print_debug("SPD3 Error With Rows\r\n");
>> 		goto val_err;
>> 	}
>> 	sz.side1 += value & 0xf;
>>
>> 	value = spd_read_byte(device, 4);	/* columns */
>> 	if (value < 0) goto hw_err;
>> 	if ((value & 0xf) == 0) {
>> 		print_debug("SPD4 Error With Columns\r\n");
>> 		goto val_err;
>> 	}
>> 	sz.side1 += value & 0xf;
>>
>> 	value = spd_read_byte(device, 17);	/* banks */
>> 	if (value < 0) goto hw_err;
>> 	if ((value & 0xff) == 0) {
>> 		print_debug("SPD17 Error With Banks\r\n");
>> 		goto val_err;
>> 	}
>> 	sz.side1 += log2(value & 0xff);
>>
>> 	/* Get the module data width and convert it to a power of two */
>> 	value = spd_read_byte(device, 7);	/* (high byte) */
>> 	if (value < 0) goto hw_err;
>> 	value &= 0xff;
>> 	value <<= 8;
>>
>> 	low = spd_read_byte(device, 6);	/* (low byte) */
>> 	if (low < 0) goto hw_err;
>> 	value = value | (low & 0xff);
>> 	if ((value != 72) && (value != 64)) {
>> 		print_debug("SPD6 Error With Data Width\r\n");
>> 		goto val_err;
>> 	}
>> 	sz.side1 += log2(value);
>>
>> 	/* side 2 */
>> 	value = spd_read_byte(device, 5);	/* number of physical banks */
>> 	if (value < 0) goto hw_err;
>> 	value &= 7;
>> 	if (value == 1) goto out;
>> 	if (value != 2) {
>> 		print_debug("SPD5 Error With Physical Banks\r\n");
>> 		goto val_err;
>> 	}
>>
>> 	/* Start with the symmetrical case */
>> 	sz.side2 = sz.side1;
>>
>> 	value = spd_read_byte(device, 3);	/* rows */
>> 	if (value < 0) goto hw_err;
>> 	if ((value & 0xf0) == 0) goto out;	/* If symmetrical we are done */
>> 	sz.side2 -= (value & 0x0f);		/* Subtract out rows on side 1 */
>> 	sz.side2 += ((value >> 4) & 0x0f);	/* Add in rows on side 2 */
>>
>> 	value = spd_read_byte(device, 4);	/* columns */
>> 	if (value < 0) goto hw_err;
>> 	if ((value & 0xff) == 0) {
>> 		print_debug("SPD4 Error With Side2 Rows\r\n");
>> 		goto val_err;
>> 	}
>> 	sz.side2 -= (value & 0x0f);		/* Subtract out columns on side 1 */
>> 	sz.side2 += ((value >> 4) & 0x0f);	/* Add in columsn on side 2 */
>> 	goto out;
>>
>>  val_err:
>> 	die("Bad SPD value\r\n");
>>
>> /* If an hw_error occurs report that I have no memory */
>> hw_err:
>> 	sz.side1 = 0;
>> 	sz.side2 = 0;
>>  out:
>> 	return sz;
>>
>> }
>>
>
> I really, _REALLY_ don't like this, even if it does work. Reading from
> spd byte 31 gives you the bank size, in units of 4mb, for both sides of
> an asymetrical dimm. Please look at the spd spec (Intel or Jedec) to see
> what you need to do to read it, it's a bit odd. You need to figure out
> how many places the 1 is shifted, the number of places * 4mb is the size
> of the bank, if the banks are asymetrical the smaller size is side2. And
> if you do decide to use it anyways, please add the original copyright
> holder, I think this is from e7501?
>
Your right, I don't really feel the above method is necessary. The  
only problem is this with Byte 31:

# Banks Density of Bank 1 Density of bank 2 Byte 31 contents
1       32MByte           N/A               0000 1000
2       32MByte           32MBbyte          0000 1000
2       32MByte           16MByte           0000 1100

You can see Byte 31 contents are going to be the same when the density  
is the same no matter how many banks it has. Could this pottentially  
give a false reading?? You could also use Byte 5 to determine the  
banks. Could we say something like; if byte 5 has 1 bank than byte 31  
is correct and if byte 5 has 2 banks to muliply byte 31 by 2?? But  
what about the third example above when you have more than 1 bank and  
the densities are different??

>> static long spd_set_ram_size(const struct mem_controller *ctrl,   
>> long dimm_mask)
>> {
>> 	int i;
>> 	int cum;
>>
>> 	for(i = cum = 0; i < DIMM_SOCKETS; i++) {
>> 		struct dimm_size sz;
>> 		if (dimm_mask & (1 << i)) {
>> 			sz = spd_get_dimm_size(ctrl->channel0[i]);
>>
>> 			/* WISHLIST: would be nice to display it as decimal? */
>> 			print_debug("DIMM is ");
>> 			print_debug_hex8(sz.side1);
>> 			print_debug(" On Side 1\r\n");
>> 			print_debug("DIMM is ");
>> 			print_debug_hex8(sz.side2);
>> 			print_debug(" On Side 2\r\n");
>>
>> 			/* Set the row offset, in KBytes (should this be
>> 			* Kbits?). Note that this offset is the start of the
>> 			* next row.
>> 			*/
>> 			row_offset = ((sz.side1 + sz.side2) * 1024);
>>
>
> If you're still in the initial testing stages, try to get one
> single-sided dimm working first. Also, if you have a dual-sided dimm,
> row_offset needs to be the address at the second side (as far as I
> know), so only sz.side1 * 1024. This might be chip-dependent,
> board-dependent, or I might be completely wrong...
>
My test board has 128MB on-board, Remember this is a set-top-box. To  
the registry, with the stock bios it shows up as a sindle sided  
so-dimm in socket 2. Initially I could just hardcode alot of this but  
I would like to make it versitial and have the ability to use socket 1  
also. Alot of Laptops use this chip also, so this could open up  
LinuxBios to that front.

Thanks Again - Joe




More information about the coreboot mailing list