Ok, I think I am ready to test my i82830 raminit.c. I started this from the i82810 code, but I want it to be able to configure an assymetric dimm. If anyone has a few minutes can you check this out, and give me some feedback??
Thanks - Joe
Joseph Smith wrote:
Ok, I think I am ready to test my i82830 raminit.c. I started this from the i82810 code, but I want it to be able to configure an assymetric dimm. If anyone has a few minutes can you check this out, and give me some feedback??
Thanks - Joe
/*
- This file is part of the LinuxBIOS project.
- Originally written for the i82810 by:
- Copyright (C) 2007 Uwe Hermann uwe@hermann-uwe.de
- Copyright (C) 2007 Corey Osgood corey@slightlyhackish.com
- Modified for the i82830 by:
I think you can drop these, perhaps with a comment below stating you're the one that ported it. Besides, what Uwe wrote was actually for the i440bx...
/* Uncomment this to enable debugging output. */ // #define DEBUG_RAM_SETUP 1
You'll probably want that uncommented before you begin testing.
/* 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.
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?
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...
static void sdram_enable(int controllers, const struct mem_controller *ctrl) {
int i;
/* Todo: this will currently work with either one dual sided or two * single sided DIMMs. Needs to work with 2 dual sided DIMMs in the * long run. */ long mask; uint32_t row_offset;
mask = spd_detect_dimms(ctrl); spd_set_dram_size(ctrl, mask);
/* 1. Apply NOP. */ PRINT_DEBUG("RAM Enable 1: Apply NOP\r\n"); do_ram_command(ctrl, RAM_COMMAND_NOP, 0, row_offset);
row_offset needs to be passed to spd_set_dram_size and set there, in the long run. For now, just set row_offset to 0 and don't worry about it, except possibly during MRS..might want to comment it out in do_ram_command as well, just to be safe.
Other than that, looks good, so good luck with the testing!
-Corey
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
On Thu, Jul 05, 2007 at 12:12:38PM -0400, Joseph Smith wrote:
/* 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?
Check the datasheet (is it publically available?). The description should be where the DRC register is described, I guess.
Uwe.
Joseph Smith wrote:
<snip>
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 potentially 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??
Below is a simple example. There are better/faster ways to do it, but this should just work. Also, coding style is probably very broken, blame that on Thunderbird.
int i, spd_byte_31, spd_byte_5; spd_byte_31 = smbus_read_byte(device, 31); //please rename device to dimm spd_byte_5 = smbus_read_byte(device, 5);
for (i = 8; i >= 0; i--) { /* Find the larger value. The larger side is always side1 */ if (spd_byte_31 & (1 << i) == (1 << i)) { sz.side1 = i; break; } }
/* Set to 0 in case it's single sided */ sz.side2 = 0;
/* Test if it's a dual-sided dimm */ if (spd_byte_5 > 1) { /* Test to see if there's a second value, if so it's asymmetrical */ if (spd_byte_31 != (1 << i)) { /* Find the second value, picking up where we left off */ /* i-- done initially to make sure we don't get the same value again */ for (i--; i >= 0; i--) { if (spd_byte_31 == (1 << i) { sz.side2 = i; break; } } /* If not, it's symmetrical */ else { sz.side2 = sz.side1; } return sz; //sizes are now in units of 4mb, so 1 = 4mb, 2 = 8mb, etc. }
BTW, this has finally made me see the correlation that the i810 uses! Expect a patch sometime for some major cleanup.
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.
Does that dimm provide spd data? lm-sensors dimms-detect.pl should tell you from userspace, or else dump_spd_data from linuxbios. If not, ignore it completely for now, just act like it doesn't exist and hardcode it in later. If so, use that as your test dimm/slot. I suspect it's probably the former. Once you have one dimm working, try for another. Going for both the first time through would take a lot of luck.
-Corey
My bad, missed a closing bracket (possibly more, very tired right how):
if (spd_byte_31 != (1 << i)) { /* Find the second value, picking up where we left off */ /* i-- done initially to make sure we don't get the same valueagain */ for (i--; i >= 0; i--) { if (spd_byte_31 == (1 << i) { sz.side2 = i; break; }
}
} /* If not, it's symmetrical */
int i, spd_byte_31, spd_byte_5; spd_byte_31 = smbus_read_byte(device, 31); //please rename device to dimm spd_byte_5 = smbus_read_byte(device, 5);
for (i = 8; i >= 0; i--) { /* Find the larger value. The larger side is always side1 */ if (spd_byte_31 & (1 << i) == (1 << i)) { sz.side1 = i; break; } }
/* Set to 0 in case it's single sided */ sz.side2 = 0;
/* Test if it's a dual-sided dimm */ if (spd_byte_5 > 1) { /* Test to see if there's a second value, if so it's asymmetrical */ if (spd_byte_31 != (1 << i)) { /* Find the second value, picking up where we left off */ /* i-- done initially to make sure we don't get the same value again */ for (i--; i >= 0; i--) { if (spd_byte_31 == (1 << i) { sz.side2 = i; break; } } } /* If not, it's symmetrical */ else { sz.side2 = sz.side1; } return sz; //sizes are now in units of 4mb, so 1 = 4mb, 2 = 8mb, etc. }
-Corey
Corey, I have been playing around with this a bit. I really like it! The only thing I do not understand is how you get ticks of 4mb out of it. Say we have a single sided 128mb dimm. i is going to end up being 7 correct? 4 * 7 is not 128, I'm confused..... Couldn't you just make the end result something like sz.side1 = (1 << i) after it breaks out of the for loop? This would give you the total without messing around with ticks of 4mb right?
Thanks - Joe
Joseph Smith wrote:
int i, spd_byte_31, spd_byte_5; spd_byte_31 = smbus_read_byte(device, 31); //please rename device to dimm spd_byte_5 = smbus_read_byte(device, 5);
for (i = 8; i >= 0; i--) { /* Find the larger value. The larger side is always side1 */ if (spd_byte_31 & (1 << i) == (1 << i)) { sz.side1 = i; break; } }
/* Set to 0 in case it's single sided */ sz.side2 = 0;
/* Test if it's a dual-sided dimm */ if (spd_byte_5 > 1) { /* Test to see if there's a second value, if so it's asymmetrical */ if (spd_byte_31 != (1 << i)) { /* Find the second value, picking up where we left off */ /* i-- done initially to make sure we don't get the same value again */ for (i--; i >= 0; i--) { if (spd_byte_31 == (1 << i) { sz.side2 = i; break; } } } /* If not, it's symmetrical */ else { sz.side2 = sz.side1; } return sz; //sizes are now in units of 4mb, so 1 = 4mb, 2 = 8mb, etc. }
-Corey
Corey, I have been playing around with this a bit. I really like it! The only thing I do not understand is how you get ticks of 4mb out of it. Say we have a single sided 128mb dimm. i is going to end up being 7 correct? 4 * 7 is not 128, I'm confused..... Couldn't you just make the end result something like sz.side1 = (1 << i) after it breaks out of the for loop? This would give you the total without messing around with ticks of 4mb right?
Yep, you're right. I didn't read correctly and screwed it up royally. Setting sz.sideX = (1 << i) is the correct way to get units of 4mb out of it. I wish there was an easier way to write the code, but this seems like the easiest way to figure out if there are 2 sides or not and separate the values.
-Corey
-
Corey Osgood -
Joseph Smith -
Uwe Hermann