Hello, I would like to submit my code for the RCA RM4100. Also code for the Intel i82830 Nothbridge, that works great. Everything works great except the ethernet and tv-out. By submitting the code maybe someone could help me figure out the ethernet problem? Anyways here it is:
Oops I forgot this:
Signed-off-by: Joseph Smith joe@smittys.pointclark.net
Thanks - Joe
Quoting joe@smittys.pointclark.net:
Hello, I would like to submit my code for the RCA RM4100. Also code for the Intel i82830 Nothbridge, that works great. Everything works great except the and tv-out.
Dis-reguard the last patch, this is the real one. A e100 kernel driver developer helped me figure out that the ethernet was an irq routing issue, fixed! It is working great now:-)))
Signed-off-by: Joseph Smith joe@smittys.pointclark.net
Thanks - Joe
On Mon, Feb 25, 2008 at 04:31:15PM -0500, joe@smittys.pointclark.net wrote: ..
tv-out
Dis-reguard the last patch, this is the real one. A e100 kernel driver developer helped me figure out that the ethernet was an irq routing issue, fixed! It is working great now:-)))
Good news!
Do you have any idea about the TV out?
Also see the comments below:
+++ src/northbridge/intel/i82830/raminit.c (revision 0)
..
+static void set_dram_timing(const struct mem_controller *ctrl) +{
- /* Set the value for DRAM Timing Register */
- /* TODO: Configure the value according to SPD values. */
- pci_write_config32(ctrl->d0, DRT, 0x00000010);
+}
+static void set_dram_buffer_strength(const struct mem_controller *ctrl) +{
- /* TODO: This needs to be set according to the DRAM tech
* (x8, x16, or x32). Argh, Intel provides no docs on this!* Currently, it needs to be pulled from the output of* lspci -xxx Rx92- */
- /* Set the value for System Memory Buffer Strength Control Registers */
- pci_write_config32(ctrl->d0, BUFF_SC, 0xFC9B491B);
+}
How about these two TODOs? In the second case I think the comment refers to running the command under the factory BIOS - in that case it would be good to clarify that.
+static void sdram_enable(int controllers, const struct mem_controller *ctrl) +{
..
- /* 4. Mode register set. Wait two memory cycles. */
- /* TODO: Set offset according to DRT values */
- PRINT_DEBUG("RAM Enable 4: Mode register set\r\n");
- do_ram_command(ctrl, RAM_COMMAND_MRS, 0x1d0);
Can this be done?
+++ targets/rca/rm4100/Config.lb (revision 0)
..
+romimage "fallback"
- option USE_FALLBACK_IMAGE = 1
- option FALLBACK_SIZE = ROM_SIZE
- option COREBOOT_EXTRA_VERSION = "_RM4100"
+# payload /etc/hosts
This last line makes no sense so I would remove it before the commit.
//Peter
On Tue, 2008-02-26 at 02:27 +0100, Peter Stuge wrote:
On Mon, Feb 25, 2008 at 04:31:15PM -0500, joe@smittys.pointclark.net wrote: ..
tv-out
Dis-reguard the last patch, this is the real one. A e100 kernel driver developer helped me figure out that the ethernet was an irq routing issue, fixed! It is working great now:-)))
Good news!
Do you have any idea about the TV out?
Also see the comments below:
+++ src/northbridge/intel/i82830/raminit.c (revision 0)
..
+static void set_dram_timing(const struct mem_controller *ctrl) +{
- /* Set the value for DRAM Timing Register */
- /* TODO: Configure the value according to SPD values. */
- pci_write_config32(ctrl->d0, DRT, 0x00000010);
+}
+static void set_dram_buffer_strength(const struct mem_controller *ctrl) +{
- /* TODO: This needs to be set according to the DRAM tech
* (x8, x16, or x32). Argh, Intel provides no docs on this!* Currently, it needs to be pulled from the output of* lspci -xxx Rx92- */
- /* Set the value for System Memory Buffer Strength Control Registers */
- pci_write_config32(ctrl->d0, BUFF_SC, 0xFC9B491B);
+}
How about these two TODOs? In the second case I think the comment refers to running the command under the factory BIOS - in that case it would be good to clarify that.
The memory on the rm4100 is onboard. There's a header for a memory slot, but there's more unknown hardware missing to hook that up. The only version with memory slots is the Thomson IP1001 (somewhat rare, essentially the same board with a different branding), and perhaps some internal RCA prototypes. This info is courtesy of Joe and his site ;)
That said, the BUFF_SC registers are the same deal as on the i810, the explanation in the datasheet isn't specific or clear about how to set them. The rest of the registers are fairly simple to set, but without the ability to try other memory sticks or with real spd data, it might look right on the rm4100, but fail in practice.
+static void sdram_enable(int controllers, const struct mem_controller *ctrl) +{
..
- /* 4. Mode register set. Wait two memory cycles. */
- /* TODO: Set offset according to DRT values */
- PRINT_DEBUG("RAM Enable 4: Mode register set\r\n");
- do_ram_command(ctrl, RAM_COMMAND_MRS, 0x1d0);
Can this be done?
Same deal as above. I can put together another patch later, and put the code in ifdefs with a note to that effect.
-Corey
+++ targets/rca/rm4100/Config.lb (revision 0)
..
+romimage "fallback"
- option USE_FALLBACK_IMAGE = 1
- option FALLBACK_SIZE = ROM_SIZE
- option COREBOOT_EXTRA_VERSION = "_RM4100"
+# payload /etc/hosts
This last line makes no sense so I would remove it before the commit.
//Peter
Quoting Peter Stuge peter@stuge.se:
On Mon, Feb 25, 2008 at 04:31:15PM -0500, joe@smittys.pointclark.net wrote: ..
tv-out
Dis-reguard the last patch, this is the real one. A e100 kernel driver developer helped me figure out that the ethernet was an irq routing issue, fixed! It is working great now:-)))
Good news!
Yeh, over a year on this baby......
Do you have any idea about the TV out?
Yes ..ideas. I have several others that are going to work on the tv-out with me, that's why I would like to get the source posted.
Also see the comments below:
+++ src/northbridge/intel/i82830/raminit.c (revision 0)
..
+static void set_dram_timing(const struct mem_controller *ctrl) +{
- /* Set the value for DRAM Timing Register */
- /* TODO: Configure the value according to SPD values. */
- pci_write_config32(ctrl->d0, DRT, 0x00000010);
+}
+static void set_dram_buffer_strength(const struct mem_controller *ctrl) +{
- /* TODO: This needs to be set according to the DRAM tech
* (x8, x16, or x32). Argh, Intel provides no docs on this!* Currently, it needs to be pulled from the output of* lspci -xxx Rx92- */
- /* Set the value for System Memory Buffer Strength Control Registers */
- pci_write_config32(ctrl->d0, BUFF_SC, 0xFC9B491B);
+}
How about these two TODOs? In the second case I think the comment refers to running the command under the factory BIOS - in that case it would be good to clarify that.
TODO #1 - The onboard memory has CAS 3 so this is manually set for now. TODO #2 - That came from Corey's i810 code. And he is right, I couldn't find any docs on this so it just got set manually.
+static void sdram_enable(int controllers, const struct mem_controller *ctrl) +{
..
- /* 4. Mode register set. Wait two memory cycles. */
- /* TODO: Set offset according to DRT values */
- PRINT_DEBUG("RAM Enable 4: Mode register set\r\n");
- do_ram_command(ctrl, RAM_COMMAND_MRS, 0x1d0);
Can this be done?
Sure it can. The addr_offset (0x1d0) is based on the memory's CAS# Latency. The onboard memory has CAS 3 so this is manually set for now. I think some of the other Intel chips already have this setup, but they are a little different than this one.
+++ targets/rca/rm4100/Config.lb (revision 0)
..
+romimage "fallback"
- option USE_FALLBACK_IMAGE = 1
- option FALLBACK_SIZE = ROM_SIZE
- option COREBOOT_EXTRA_VERSION = "_RM4100"
+# payload /etc/hosts
This last line makes no sense so I would remove it before the commit.
Sure.
//Peter
-- coreboot mailing list coreboot@coreboot.org http://www.coreboot.org/mailman/listinfo/coreboot
Thanks - Joe
On Mon, Feb 25, 2008 at 08:51:00PM -0500, Corey Osgood wrote:
- /* Set the value for DRAM Timing Register */
- /* TODO: Configure the value according to SPD values. */
- pci_write_config32(ctrl->d0, DRT, 0x00000010);
How about these two TODOs?
The memory on the rm4100 is onboard.
I guess this port could use the SPD emulation a la ALIX then?
The only version with memory slots is the Thomson IP1001
Perhaps another port for that, then?
without the ability to try other memory sticks or with real spd data, it might look right on the rm4100, but fail in practice.
The problem is that this is in (supposedly) generic 830 code.
- /* TODO: Set offset according to DRT values */
- PRINT_DEBUG("RAM Enable 4: Mode register set\r\n");
- do_ram_command(ctrl, RAM_COMMAND_MRS, 0x1d0);
Can this be done?
Same deal as above.
Yes, exactly.
I can put together another patch later, and put the code in ifdefs with a note to that effect.
Hm? ifdef what?
//Peter
Quoting Peter Stuge peter@stuge.se:
On Mon, Feb 25, 2008 at 08:51:00PM -0500, Corey Osgood wrote:
- /* Set the value for DRAM Timing Register */
- /* TODO: Configure the value according to SPD values. */
- pci_write_config32(ctrl->d0, DRT, 0x00000010);
How about these two TODOs?
The memory on the rm4100 is onboard.
I guess this port could use the SPD emulation a la ALIX then?
possible? Just a fake spd_table setup for now.
The only version with memory slots is the Thomson IP1001
Perhaps another port for that, then?
Possible if you could find one. And trust me the people that do have them are not willing to share:-(
without the ability to try other memory sticks or with real spd data, it might look right on the rm4100, but fail in practice.
The problem is that this is in (supposedly) generic 830 code.
It pretty much is. First of all PC133 SO-DIMMS only come in cas3 or cas2 but setting DRT to cas3 is not hurting anything. So your memory only runs a tiny bit slower, an average user probably wouldn't notice the difference. But it does need to be done eventually (DRT). Besides DRT everything else is spd detectable. Did you check out function Corey and I came up with to get the memory size using spd 31. I still think that is pretty cool.
- /* TODO: Set offset according to DRT values */
- PRINT_DEBUG("RAM Enable 4: Mode register set\r\n");
- do_ram_command(ctrl, RAM_COMMAND_MRS, 0x1d0);
Can this be done?
Same deal as above.
Yes, exactly.
I can put together another patch later, and put the code in ifdefs with a note to that effect.
Hm? ifdef what?
//Peter
-- coreboot mailing list coreboot@coreboot.org http://www.coreboot.org/mailman/listinfo/coreboot
Thanks - Joe
Quoting Corey Osgood corey.osgood@gmail.com:
On Tue, 2008-02-26 at 02:27 +0100, Peter Stuge wrote:
On Mon, Feb 25, 2008 at 04:31:15PM -0500, joe@smittys.pointclark.net wrote: ..
tv-out
Dis-reguard the last patch, this is the real one. A e100 kernel driver developer helped me figure out that the ethernet was an irq routing issue, fixed! It is working great now:-)))
Good news!
Do you have any idea about the TV out?
Also see the comments below:
+++ src/northbridge/intel/i82830/raminit.c (revision 0)
..
+static void set_dram_timing(const struct mem_controller *ctrl) +{
- /* Set the value for DRAM Timing Register */
- /* TODO: Configure the value according to SPD values. */
- pci_write_config32(ctrl->d0, DRT, 0x00000010);
+}
+static void set_dram_buffer_strength(const struct mem_controller *ctrl) +{
- /* TODO: This needs to be set according to the DRAM tech
* (x8, x16, or x32). Argh, Intel provides no docs on this!* Currently, it needs to be pulled from the output of* lspci -xxx Rx92- */
- /* Set the value for System Memory Buffer Strength Control Registers */
- pci_write_config32(ctrl->d0, BUFF_SC, 0xFC9B491B);
+}
How about these two TODOs? In the second case I think the comment refers to running the command under the factory BIOS - in that case it would be good to clarify that.
The memory on the rm4100 is onboard. There's a header for a memory slot, but there's more unknown hardware missing to hook that up. The only version with memory slots is the Thomson IP1001 (somewhat rare, essentially the same board with a different branding), and perhaps some internal RCA prototypes. This info is courtesy of Joe and his site ;)
That said, the BUFF_SC registers are the same deal as on the i810, the explanation in the datasheet isn't specific or clear about how to set them. The rest of the registers are fairly simple to set, but without the ability to try other memory sticks or with real spd data, it might look right on the rm4100, but fail in practice.
This is not necessarily true. It is setup so the controller actually reads the fake spd table just like it was real (kind of tricks the controller). You can change the values in the spd table around to get different results. That's how I tested it. The only thing I couldn't test is the second so-dimm slot. Like Corey said above the header is missing....
+static void sdram_enable(int controllers, const struct
mem_controller *ctrl)
+{
..
- /* 4. Mode register set. Wait two memory cycles. */
- /* TODO: Set offset according to DRT values */
- PRINT_DEBUG("RAM Enable 4: Mode register set\r\n");
- do_ram_command(ctrl, RAM_COMMAND_MRS, 0x1d0);
Can this be done?
Same deal as above. I can put together another patch later, and put the code in ifdefs with a note to that effect.
-Corey
+++ targets/rca/rm4100/Config.lb (revision 0)
..
+romimage "fallback"
- option USE_FALLBACK_IMAGE = 1
- option FALLBACK_SIZE = ROM_SIZE
- option COREBOOT_EXTRA_VERSION = "_RM4100"
+# payload /etc/hosts
This last line makes no sense so I would remove it before the commit.
//Peter
-- coreboot mailing list coreboot@coreboot.org http://www.coreboot.org/mailman/listinfo/coreboot
Thanks - Joe
On Mon, 2008-02-25 at 21:20 -0500, joe@smittys.pointclark.net wrote:
Quoting Peter Stuge peter@stuge.se:
On Mon, Feb 25, 2008 at 08:51:00PM -0500, Corey Osgood wrote:
without the ability to try other memory sticks or with real spd data, it might look right on the rm4100, but fail in practice.
The problem is that this is in (supposedly) generic 830 code.
It pretty much is. First of all PC133 SO-DIMMS only come in cas3 or cas2 but setting DRT to cas3 is not hurting anything. So your memory only runs a tiny bit slower, an average user probably wouldn't notice the difference. But it does need to be done eventually (DRT). Besides DRT everything else is spd detectable. Did you check out function Corey and I came up with to get the memory size using spd 31. I still think that is pretty cool.
What he said ;)
Moving on:
+static void spd_set_dram_size(const struct mem_controller *ctrl) +{
- int i, value, drb1, drb2;
- for (i = 0; i < DIMM_SOCKETS; i++) {
struct dimm_size sz;unsigned device;device = ctrl->channel0[i];drb1 = 0;drb2 = 0;/* First check if a DIMM is actually present. */if (spd_read_byte(device, 2) == 0x4) {print_debug("Found DIMM in slot ");print_debug_hex8(i);print_debug("\r\n");sz = spd_get_dimm_size(device);/* WISHLIST: would be nice to display it as decimal? */
I saw a small bit of code to do this just a little while ago, if only I could remember where...
print_debug("DIMM is 0x");print_debug_hex16(sz.side1);print_debug(" on side 1\r\n");print_debug("DIMM is 0x");print_debug_hex16(sz.side2);print_debug(" on side 2\r\n");/* The i82830 can't handle DIMMs smaller than 32MB per* side or larger than 256MB per side. It also can* only support a symmetrical dual-sided dimm.*/if ((sz.side1 < 32)) {print_err("DIMMs smaller than 32MB per side\r\n");print_err("are not supported on this board\r\n");
board -> northbridge or chipset, in all of these.
die("HALT\r\n");
Why die?
}if ((sz.side2 != 0) && (sz.side2 < 32)) {print_err("DIMMs smaller than 32MB per side\r\n");print_err("are not supported on this board\r\n");die("HALT\r\n");}if ((sz.side1 > 256) || (sz.side2 > 256)) {
side1 should always be the larger side, and i830 doesn't support asymetrical dimms, so you can drop the side2 check.
print_err("DIMMs larger than 256MB per side\r\n");print_err("are not supported on this board\r\n");die("HALT\r\n");}if ((sz.side2 != 0) && (sz.side1 != sz.side2)) {print_err("This board only supports\r\n");print_err("symmetrical dual-sided DIMMs\r\n");die("HALT\r\n");}
This should be moved right into the detection routine. If the northbridge can't handle it, then just die if sz.side1 |= sz.side2. The other option is to set sz.side2 to 0, and try to use it like a single-sided dimm.
/* We need to divide size by 32 to set up the
* DRB registers.*/if (sz.side1 > 0) {
if(sz.side1) works as well. unsigned can never be negative.
drb1 = sz.side1 >> 5;
Please don't use bitwise shifts for multiplication/division, same applies for other places.
}if (sz.side2 > 0) {drb2 = sz.side2 >> 5;}
Please excuse me for jumping around, trying to keep this short:
+static void do_ram_command(const struct mem_controller *ctrl, uint32_t command, uint32_t addr_offset) +{
<snip> + + /* Read from (DIMM start address + addr_offset). */ + read32(0 + addr_offset); //first offset is always 0 +}
This isn't ready for multiple dimms yet. See the cn700 patch I recently sent (but haven't committed yet, I think it was acked).
I think that's everything, I'm hitting the sack.
-Coreu
On Mon, Feb 25, 2008 at 10:18:01PM -0500, Corey Osgood wrote:
- if ((sz.side2 != 0) && (sz.side1 != sz.side2)) {
print_err("This board only supports\r\n");print_err("symmetrical dual-sided DIMMs\r\n");die("HALT\r\n");- }
This should be moved right into the detection routine. If the northbridge can't handle it, then just die if sz.side1 |= sz.side2. The other option is to set sz.side2 to 0, and try to use it like a single-sided dimm.
Degrading like this with a very loud message would be a very cool feature.
drb1 = sz.side1 >> 5;Please don't use bitwise shifts for multiplication/division, same applies for other places.
+1
//Peter
Quoting Corey Osgood corey.osgood@gmail.com:
On Mon, 2008-02-25 at 21:20 -0500, joe@smittys.pointclark.net wrote:
Quoting Peter Stuge peter@stuge.se:
On Mon, Feb 25, 2008 at 08:51:00PM -0500, Corey Osgood wrote:
without the ability to try other memory sticks or with real spd data, it might look right on the rm4100, but fail in practice.
The problem is that this is in (supposedly) generic 830 code.
It pretty much is. First of all PC133 SO-DIMMS only come in cas3 or cas2 but setting DRT to cas3 is not hurting anything. So your memory only runs a tiny bit slower, an average user probably wouldn't notice the difference. But it does need to be done eventually (DRT). Besides DRT everything else is spd detectable. Did you check out function Corey and I came up with to get the memory size using spd 31. I still think that is pretty cool.
What he said ;)
Moving on:
+static void spd_set_dram_size(const struct mem_controller *ctrl) +{
- int i, value, drb1, drb2;
- for (i = 0; i < DIMM_SOCKETS; i++) {
struct dimm_size sz;unsigned device;device = ctrl->channel0[i];drb1 = 0;drb2 = 0;/* First check if a DIMM is actually present. */if (spd_read_byte(device, 2) == 0x4) {print_debug("Found DIMM in slot ");print_debug_hex8(i);print_debug("\r\n");sz = spd_get_dimm_size(device);/* WISHLIST: would be nice to display it as decimal? */I saw a small bit of code to do this just a little while ago, if only I could remember where...
That would be cool.
print_debug("DIMM is 0x");print_debug_hex16(sz.side1);print_debug(" on side 1\r\n");print_debug("DIMM is 0x");print_debug_hex16(sz.side2);print_debug(" on side 2\r\n");/* The i82830 can't handle DIMMs smaller than 32MB per* side or larger than 256MB per side. It also can* only support a symmetrical dual-sided dimm.*/if ((sz.side1 < 32)) {print_err("DIMMs smaller than 32MB per side\r\n");print_err("are not supported on this board\r\n");board -> northbridge or chipset, in all of these.
Not sure what you mean here.
die("HALT\r\n");Why die?
Die becaause the memory is not supported, right?
}if ((sz.side2 != 0) && (sz.side2 < 32)) {print_err("DIMMs smaller than 32MB per side\r\n");print_err("are not supported on this board\r\n");die("HALT\r\n");}if ((sz.side1 > 256) || (sz.side2 > 256)) {side1 should always be the larger side, and i830 doesn't support asymetrical dimms, so you can drop the side2 check.
ok will change
print_err("DIMMs larger than 256MB per side\r\n");print_err("are not supported on this board\r\n");die("HALT\r\n");}if ((sz.side2 != 0) && (sz.side1 != sz.side2)) {print_err("This board only supports\r\n");print_err("symmetrical dual-sided DIMMs\r\n");die("HALT\r\n");}This should be moved right into the detection routine. If the northbridge can't handle it, then just die if sz.side1 |= sz.side2. The other option is to set sz.side2 to 0, and try to use it like a single-sided dimm.
Hmm above is saying that IF THERE IS A SECOND SIDE and it does not equal the first side than die, otherwise it is assumed it is single sided and moves on.
"if sz.side1 |= sz.side2" does not work for single sided so-dimms, correct?
/* We need to divide size by 32 to set up the
* DRB registers.*/if (sz.side1 > 0) {if(sz.side1) works as well. unsigned can never be negative.
ok will change
drb1 = sz.side1 >> 5;Please don't use bitwise shifts for multiplication/division, same applies for other places.
}if (sz.side2 > 0) {drb2 = sz.side2 >> 5;}Please excuse me for jumping around, trying to keep this short:
ok will change, but there was quite a discussion about this a while ago and half thought it was ok and half didn't. So which is correct in "C" standards?
+static void do_ram_command(const struct mem_controller *ctrl, uint32_t command, uint32_t addr_offset) +{
<snip> + + /* Read from (DIMM start address + addr_offset). */ + read32(0 + addr_offset); //first offset is always 0 +}
This isn't ready for multiple dimms yet. See the cn700 patch I recently sent (but haven't committed yet, I think it was acked).
ok will do.
I think that's everything, I'm hitting the sack.
-Coreu
Thanks - Joe
On Mon, Feb 25, 2008 at 11:12:58PM -0500, joe@smittys.pointclark.net wrote:
- print_err("are not supported on this board\r\n");
board -> northbridge or chipset, in all of these.
Not sure what you mean here.
Change the word "board" to either "northbridge" or "chipset" because this isn't board-specific.
die("HALT\r\n");Why die?
Die becaause the memory is not supported, right?
Maybe it is possible to degrade and carry on?
if ((sz.side2 != 0) && (sz.side1 != sz.side2)) {print_err("This board only supports\r\n");print_err("symmetrical dual-sided DIMMs\r\n");die("HALT\r\n");}This should be moved right into the detection routine. If the northbridge can't handle it, then just die if sz.side1 |= sz.side2. The other option is to set sz.side2 to 0, and try to use it like a single-sided dimm.
Hmm above is saying that IF THERE IS A SECOND SIDE and it does not equal the first side than die, otherwise it is assumed it is single sided and moves on.
"if sz.side1 |= sz.side2" does not work for single sided so-dimms, correct?
Yes. I think that was a typo, it should probably have been != instead.
drb1 = sz.side1 >> 5;Please don't use bitwise shifts for multiplication/division,
ok will change, but there was quite a discussion about this a while ago and half thought it was ok and half didn't. So which is correct in "C" standards?
I say depends on the case. If the algorithm is mathematical I say use / and * but if it is primarily about moving bits then shift is fine.
//Peter
if ((sz.side2 != 0) && (sz.side1 != sz.side2)) {print_err("This board only supports\r\n");print_err("symmetrical dual-sided DIMMs\r\n");die("HALT\r\n");}This should be moved right into the detection routine. If the northbridge can't handle it, then just die if sz.side1 |= sz.side2. The other option is to set sz.side2 to 0, and try to use it like a single-sided dimm.
ok how about this. Below is saying that IF THERE IS A SECOND SIDE and it does not equal the first side than give a message, use as a single sided dimm, and move on.
/* The i82830 only supports a symmetrical dual-sided dimm * and can't handle DIMMs smaller than 32MB per * side or larger than 256MB per side. */ if ((sz.side2 != 0) && (sz.side1 != sz.side2)) { print_err("This northbridge only supports\r\n"); print_err("symmetrical dual-sided DIMMs\r\n"); print_err("booting as a single-sided DIMM\r\n"); sz.side2 = 0; } if ((sz.side1 < 32)) { print_err("DIMMs smaller than 32MB per side\r\n"); print_err("are not supported on this northbridge\r\n"); die("HALT\r\n"); }
if ((sz.side1 > 256)) { print_err("DIMMs larger than 256MB per side\r\n"); print_err("are not supported on this northbridge\r\n"); die("HALT\r\n"); }
Thanks - Joe
On Tue, Feb 26, 2008 at 10:04:59AM -0500, joe@smittys.pointclark.net wrote:
ok how about this.
I like it!
//Peter
+static void do_ram_command(const struct mem_controller *ctrl, uint32_t command, uint32_t addr_offset) +{
<snip> + + /* Read from (DIMM start address + addr_offset). */ + read32(0 + addr_offset); //first offset is always 0 +}
This isn't ready for multiple dimms yet. See the cn700 patch I recently sent (but haven't committed yet, I think it was acked).
Oh you mean this? What do I need to do to adapt it for the i82830?
+ /* NOTE: Dual-sided ready */ read32(0 + addr_offset); + for(i = 0; i < (ARRAY_SIZE(ctrl->channel0) * 2); i++) { + reg8 = pci_read_config8(ctrl->d0f3, 0x40 + i); + if(reg8) read32((reg8 << 26) + addr_offset); + }
Thanks - Joe
On Tue, 2008-02-26 at 16:19 -0500, joe@smittys.pointclark.net wrote:
+static void do_ram_command(const struct mem_controller *ctrl, uint32_t command, uint32_t addr_offset) +{
<snip> + + /* Read from (DIMM start address + addr_offset). */ + read32(0 + addr_offset); //first offset is always 0 +}
This isn't ready for multiple dimms yet. See the cn700 patch I recently sent (but haven't committed yet, I think it was acked).
Oh you mean this? What do I need to do to adapt it for the i82830?
- /* NOTE: Dual-sided ready */ read32(0 + addr_offset);
- for(i = 0; i < (ARRAY_SIZE(ctrl->channel0) * 2); i++) {
reg8 = pci_read_config8(ctrl->d0f3, 0x40 + i);- if(reg8) read32((reg8 << 26) + addr_offset);
- }
This reads from register 0x40 + i, where 0x40 is the top of the first dimm, and i counts to the max number of dimms. Reg 0x40 contains bits 33:26 of the top address, hence the << 26. So you'd need to adjust both of these to fit the i830's DRB mechanism. If the i830 uses an ugly format like the i810 does, then it may be easier to store the size in scratch registers somewhere after it's calculated, and then use those values instead. Sorry I can't get more specific, I've got to head out to my next service call.
-Corey
Quoting Corey Osgood corey.osgood@gmail.com:
On Tue, 2008-02-26 at 16:19 -0500, joe@smittys.pointclark.net wrote:
+static void do_ram_command(const struct mem_controller *ctrl, uint32_t command, uint32_t addr_offset) +{
<snip> + + /* Read from (DIMM start address + addr_offset). */ + read32(0 + addr_offset); //first offset is always 0 +}
This isn't ready for multiple dimms yet. See the cn700 patch I recently sent (but haven't committed yet, I think it was acked).
Oh you mean this? What do I need to do to adapt it for the i82830?
- /* NOTE: Dual-sided ready */ read32(0 + addr_offset);
- for(i = 0; i < (ARRAY_SIZE(ctrl->channel0) * 2); i++) {
reg8 = pci_read_config8(ctrl->d0f3, 0x40 + i);- if(reg8) read32((reg8 << 26) + addr_offset);
- }
This reads from register 0x40 + i, where 0x40 is the top of the first dimm, and i counts to the max number of dimms.
Why read this from a register?? Why not just use (i = 0; i < DIMM_SOCKETS; i++)
Reg 0x40 contains bits 33:26 of the top address, hence the << 26. So you'd need to adjust both of these to fit the i830's DRB mechanism. If the i830 uses an ugly format like the i810 does, then it may be easier to store the size in scratch registers somewhere after it's calculated, and then use those values instead. Sorry I can't get more specific, I've got to head out to my next service call.
The DRB i830 is not ugly it is very simple, simple, simple. So simple that it just calculates the memory size in ticks of 32 per side * the next side up to 4 sides. Like this:
Example: 128MB SINGLE sided so-dimm in slot 1 128MB DOUBLE sided so-dimm in slot 2
There are 4 DRB registers(actually 6 but but only 4 that are usable - Intel design glitch), 1 for each side. So in this example they would look like this:
DRB1 = 0x04 DRB2 = 0x04 DRB3 = 0x06 DRB4 = 0x08
So if we needed to convert this we would do something like this (DRB * 32) * 1024 correct? But if we just need to calculate the end of so-dimm #1 and the start of so-dimm #2 the only important register would be DRB2 (side 2 of so-dimm #1) and DRB3(side 1 of so-dimm #2), correct?
Thanks - Joe
Quoting joe@smittys.pointclark.net:
Quoting Corey Osgood corey.osgood@gmail.com:
On Tue, 2008-02-26 at 16:19 -0500, joe@smittys.pointclark.net wrote:
+static void do_ram_command(const struct mem_controller *ctrl, uint32_t command, uint32_t addr_offset) +{
<snip> + + /* Read from (DIMM start address + addr_offset). */ + read32(0 + addr_offset); //first offset is always 0 +}
This isn't ready for multiple dimms yet. See the cn700 patch I recently sent (but haven't committed yet, I think it was acked).
Oh you mean this? What do I need to do to adapt it for the i82830?
- /* NOTE: Dual-sided ready */ read32(0 + addr_offset);
- for(i = 0; i < (ARRAY_SIZE(ctrl->channel0) * 2); i++) {
reg8 = pci_read_config8(ctrl->d0f3, 0x40 + i);- if(reg8) read32((reg8 << 26) + addr_offset);
- }
This reads from register 0x40 + i, where 0x40 is the top of the first dimm, and i counts to the max number of dimms.
Why read this from a register?? Why not just use (i = 0; i < DIMM_SOCKETS; i++)
Reg 0x40 contains bits 33:26 of the top address, hence the << 26. So you'd need to adjust both of these to fit the i830's DRB mechanism. If the i830 uses an ugly format like the i810 does, then it may be easier to store the size in scratch registers somewhere after it's calculated, and then use those values instead. Sorry I can't get more specific, I've got to head out to my next service call.
The DRB i830 is not ugly it is very simple, simple, simple. So simple that it just calculates the memory size in ticks of 32 per side
(Sorry typo I meant) +
the next side up to 4 sides. Like this:
Example: 128MB SINGLE sided so-dimm in slot 1 128MB DOUBLE sided so-dimm in slot 2
There are 4 DRB registers(actually 6 but but only 4 that are usable - Intel design glitch), 1 for each side. So in this example they would look like this:
DRB1 = 0x04 DRB2 = 0x04 DRB3 = 0x06 DRB4 = 0x08
So if we needed to convert this we would do something like this (DRB * 32) * 1024 correct? But if we just need to calculate the end of so-dimm #1 and the start of so-dimm #2 the only important register would be DRB2 (side 2 of so-dimm #1) and DRB3(side 1 of so-dimm #2), correct?
Thanks - Joe
-- coreboot mailing list coreboot@coreboot.org http://www.coreboot.org/mailman/listinfo/coreboot
Thanks - Joe
On Tue, 2008-02-26 at 21:33 -0500, joe@smittys.pointclark.net wrote:
Quoting joe@smittys.pointclark.net:
Quoting Corey Osgood corey.osgood@gmail.com:
On Tue, 2008-02-26 at 16:19 -0500, joe@smittys.pointclark.net wrote:
+static void do_ram_command(const struct mem_controller *ctrl, uint32_t command, uint32_t addr_offset) +{
<snip> + + /* Read from (DIMM start address + addr_offset). */ + read32(0 + addr_offset); //first offset is always 0 +}
This isn't ready for multiple dimms yet. See the cn700 patch I recently sent (but haven't committed yet, I think it was acked).
Oh you mean this? What do I need to do to adapt it for the i82830?
- /* NOTE: Dual-sided ready */ read32(0 + addr_offset);
- for(i = 0; i < (ARRAY_SIZE(ctrl->channel0) * 2); i++) {
reg8 = pci_read_config8(ctrl->d0f3, 0x40 + i);- if(reg8) read32((reg8 << 26) + addr_offset);
- }
This reads from register 0x40 + i, where 0x40 is the top of the first dimm, and i counts to the max number of dimms.
Why read this from a register?? Why not just use (i = 0; i < DIMM_SOCKETS; i++)
ARRAY_SIZE(ctrl->channel0) is the number of spd addresses, which can be less than the max number of dimms.
Reg 0x40 contains bits 33:26 of the top address, hence the << 26. So you'd need to adjust both of these to fit the i830's DRB mechanism. If the i830 uses an ugly format like the i810 does, then it may be easier to store the size in scratch registers somewhere after it's calculated, and then use those values instead. Sorry I can't get more specific, I've got to head out to my next service call.
The DRB i830 is not ugly it is very simple, simple, simple. So simple that it just calculates the memory size in ticks of 32 per side
(Sorry typo I meant) +
the next side up to 4 sides. Like this:
Example: 128MB SINGLE sided so-dimm in slot 1 128MB DOUBLE sided so-dimm in slot 2
There are 4 DRB registers(actually 6 but but only 4 that are usable - Intel design glitch), 1 for each side. So in this example they would look like this:
DRB1 = 0x04 DRB2 = 0x04 DRB3 = 0x06 DRB4 = 0x08
So if we needed to convert this we would do something like this (DRB * 32) * 1024 correct? But if we just need to calculate the end of so-dimm #1 and the start of so-dimm #2 the only important register would be DRB2 (side 2 of so-dimm #1) and DRB3(side 1 of so-dimm #2), correct?
The modified version would be something like read32(0 + addr_offset); for(i = 0; i < ARRAY_SIZE(ctrl->channel0); i++) { reg8 = pci_read_config8(ctrl->d0/*?*/, DRB1 + i); read32(reg8 * 32); }
I've dropped the if(reg8) because cn700 doesn't program unused DRBs, hence they're 0, but that obviously wouldn't work here. And I just realized I didn't do the cn700 the way I meant to, should have read the start addresses, not the end ones (simple fix).
-Corey
Quoting Corey Osgood corey.osgood@gmail.com:
On Tue, 2008-02-26 at 21:33 -0500, joe@smittys.pointclark.net wrote:
Quoting joe@smittys.pointclark.net:
Quoting Corey Osgood corey.osgood@gmail.com:
On Tue, 2008-02-26 at 16:19 -0500, joe@smittys.pointclark.net wrote:
> +static void do_ram_command(const struct mem_controller *ctrl, > uint32_t command, uint32_t addr_offset) > +{ > <snip> > + > + /* Read from (DIMM start address + addr_offset). */ > + read32(0 + addr_offset); //first offset is always 0 > +}
This isn't ready for multiple dimms yet. See the cn700 patch I recently sent (but haven't committed yet, I think it was acked).
Oh you mean this? What do I need to do to adapt it for the i82830?
- /* NOTE: Dual-sided ready */ read32(0 + addr_offset);
- for(i = 0; i < (ARRAY_SIZE(ctrl->channel0) * 2); i++) {
reg8 = pci_read_config8(ctrl->d0f3, 0x40 + i);- if(reg8) read32((reg8 << 26) + addr_offset);
- }
This reads from register 0x40 + i, where 0x40 is the top of the first dimm, and i counts to the max number of dimms.
Why read this from a register?? Why not just use (i = 0; i < DIMM_SOCKETS; i++)
ARRAY_SIZE(ctrl->channel0) is the number of spd addresses, which can be less than the max number of dimms.
Reg 0x40 contains bits 33:26 of the top address, hence the << 26. So you'd need to adjust both of these to fit the i830's DRB mechanism. If the i830 uses an ugly format like the i810 does, then it may be easier to store the size in scratch registers somewhere after it's calculated, and then use those values instead. Sorry I can't get more specific, I've got to head out to my next service call.
The DRB i830 is not ugly it is very simple, simple, simple. So simple that it just calculates the memory size in ticks of 32 per side
(Sorry typo I meant) +
the next side up to 4 sides. Like this:
Example: 128MB SINGLE sided so-dimm in slot 1 128MB DOUBLE sided so-dimm in slot 2
There are 4 DRB registers(actually 6 but but only 4 that are usable - Intel design glitch), 1 for each side. So in this example they would look like this:
DRB1 = 0x04 DRB2 = 0x04 DRB3 = 0x06 DRB4 = 0x08
So if we needed to convert this we would do something like this (DRB * 32) * 1024 correct? But if we just need to calculate the end of so-dimm #1 and the start of so-dimm #2 the only important register would be DRB2 (side 2 of so-dimm #1) and DRB3(side 1 of so-dimm #2), correct?
The modified version would be something like read32(0 + addr_offset); for(i = 0; i < ARRAY_SIZE(ctrl->channel0); i++) { reg8 = pci_read_config8(ctrl->d0/*?*/, DRB1 + i); read32(reg8 * 32); }
Ok starting to make sense but what is all this about? /*?*/ wouldn't it just be reg8 = pci_read_config8(ctrl->d0, DRB1 = i); This is going to show our results in MB, is that what we want? For accuracy shouldn't it be in bytes? Sorry for so many questions, I just want to get this right.
Thanks - Joe
On Wed, Feb 27, 2008 at 7:16 AM, joe@smittys.pointclark.net wrote:
Quoting Corey Osgood corey.osgood@gmail.com:
On Tue, 2008-02-26 at 21:33 -0500, joe@smittys.pointclark.net wrote:
Quoting joe@smittys.pointclark.net:
Quoting Corey Osgood corey.osgood@gmail.com:
On Tue, 2008-02-26 at 16:19 -0500, joe@smittys.pointclark.net wrote:
>> +static void do_ram_command(const struct mem_controller *ctrl, >> uint32_t command, uint32_t addr_offset) >> +{ >> <snip> >> + >> + /* Read from (DIMM start address + addr_offset). */ >> + read32(0 + addr_offset); //first offset is always 0 >> +} > > This isn't ready for multiple dimms yet. See the cn700 patch I > recently sent (but haven't committed yet, I think it was acked). > Oh you mean this? What do I need to do to adapt it for the i82830?
/* NOTE: Dual-sided ready */ read32(0 + addr_offset);for(i = 0; i < (ARRAY_SIZE(ctrl->channel0) * 2); i++) {reg8 = pci_read_config8(ctrl->d0f3, 0x40 + i);if(reg8) read32((reg8 << 26) + addr_offset);- }
This reads from register 0x40 + i, where 0x40 is the top of the
first
dimm, and i counts to the max number of dimms.
Why read this from a register?? Why not just use (i = 0; i < DIMM_SOCKETS; i++)
ARRAY_SIZE(ctrl->channel0) is the number of spd addresses, which can be less than the max number of dimms.
Reg 0x40 contains bits 33:26 of the top address, hence the << 26. So you'd need to adjust
both
of these to fit the i830's DRB mechanism. If the i830 uses an ugly format like the i810 does, then it may be easier to store the size
in
scratch registers somewhere after it's calculated, and then use
those
values instead. Sorry I can't get more specific, I've got to head
out to
my next service call.
The DRB i830 is not ugly it is very simple, simple, simple. So simple that it just calculates the memory size in ticks of 32 per side
(Sorry typo I meant) +
the next side up to 4 sides. Like this:
Example: 128MB SINGLE sided so-dimm in slot 1 128MB DOUBLE sided so-dimm in slot 2
There are 4 DRB registers(actually 6 but but only 4 that are usable - Intel design glitch), 1 for each side. So in this example they would look like this:
DRB1 = 0x04 DRB2 = 0x04 DRB3 = 0x06 DRB4 = 0x08
So if we needed to convert this we would do something like this (DRB
- 1024 correct? But if we just need to calculate the end of
so-dimm #1 and the start of so-dimm #2 the only important register would be DRB2 (side 2 of so-dimm #1) and DRB3(side 1 of so-dimm #2), correct?
The modified version would be something like read32(0 + addr_offset); for(i = 0; i < ARRAY_SIZE(ctrl->channel0); i++) { reg8 = pci_read_config8(ctrl->d0/*?*/, DRB1 + i); read32(reg8 * 32); }
Ok starting to make sense but what is all this about? /*?*/ wouldn't it just be reg8 = pci_read_config8(ctrl->d0, DRB1 = i);
DRB1 + i, but yes. The /*?*/ is an inline comment, because I wasn't sure if it was actually device 0 function 0 that had the memory registers.
This is going to show our results in MB, is that what we want? For accuracy shouldn't it be in bytes? Sorry for so many questions, I just want to get this right.
Right again, it would need to be read32(reg8 * 32 * 1024 * 1024) (If I'm thinking correctly).
-Corey
Thanks - Joe
I hate to say it Corey, but this just sends it into an infinite loop. Does it make a difference that the onboard memory is located in slot2 and slot1 is empty? Here is what I have:
Thanks - Joe
static void do_ram_command(const struct mem_controller *ctrl, uint32_t command, uint32_t addr_offset) { int i; uint8_t reg8; uint32_t reg32;
/* Configure the RAM command. */ reg32 = pci_read_config32(ctrl->d0, DRC); /* Clear bits 29, 10-8, 6-4. */ reg32 &= 0xdffff88f; reg32 |= command << 4; /* If RAM_COMMAND_NORMAL set the refresh mode and IC bit. */ if (command == RAM_COMMAND_NORMAL) { reg32 |= ((RAM_COMMAND_REFRESH << 8) | (RAM_COMMAND_IC << 29)); } pci_write_config32(ctrl->d0, DRC, reg32);
/* RAM_COMMAND_NORMAL affects only the memory controller and doesn't need to be "sent" to the DIMMs. */ /* if (command == RAM_COMMAND_NORMAL) return; */
PRINT_DEBUG(" Sending RAM command 0x"); PRINT_DEBUG_HEX32(reg32); PRINT_DEBUG(" to 0x"); PRINT_DEBUG_HEX32(0 + addr_offset); PRINT_DEBUG("\r\n");
/* NOTE: Dual-sided ready */ read32(0 + addr_offset); for(i = 0; i < ARRAY_SIZE(ctrl->channel0); i++) { reg8 = pci_read_config8(ctrl->d0, DRB + i); read32(((reg8 * 32) * 1024) * 1024); } }
On Tue, Mar 4, 2008 at 11:37 PM, joe@smittys.pointclark.net wrote:
I hate to say it Corey, but this just sends it into an infinite loop. Does it make a difference that the onboard memory is located in slot2 and slot1 is empty? Here is what I have:
Thanks - Joe
Try this instead, IIRC there are 4 DRBs. If it fails to build with too few registers, I think you can nuke the extra variable and just do multiple reads to the same location, or else reuse reg32 in place of reg8_2. I'm not really sure why the previous version failed, it looks sound to me. It might be interesting to put in some debugging output, to see what's going wrong.
static void do_ram_command(const struct mem_controller *ctrl, uint32_t command, uint32_t addr_offset) { int i; uint8_t reg8, reg8_2 = 0; uint32_t reg32;
/* Configure the RAM command. */ reg32 = pci_read_config32(ctrl->d0, DRC); /* Clear bits 29, 10-8, 6-4. */ reg32 &= 0xdffff88f; reg32 |= command << 4; /* If RAM_COMMAND_NORMAL set the refresh mode and IC bit. */ if (command == RAM_COMMAND_NORMAL) { reg32 |= ((RAM_COMMAND_REFRESH << 8) | (RAM_COMMAND_IC << 29)); } pci_write_config32(ctrl->d0, DRC, reg32);
/* RAM_COMMAND_NORMAL affects only the memory controller and doesn't need to be "sent" to the DIMMs. */ /* if (command == RAM_COMMAND_NORMAL) return; */
PRINT_DEBUG(" Sending RAM command 0x"); PRINT_DEBUG_HEX32(reg32); PRINT_DEBUG(" to 0x"); PRINT_DEBUG_HEX32(0 + addr_offset); PRINT_DEBUG("\r\n");
/* NOTE: Dual-sided ready */ read32(0 + addr_offset); for(i = 0; i < 4; i++) { reg8 = pci_read_config8(ctrl->d0, DRB + i); if(reg8 != reg8_2) read32(reg8 * 32 * 1024 * 1024); //extra parentheses aren't necessary reg8_2 = reg8; } }
Quoting Corey Osgood corey.osgood@gmail.com:
Try this instead, IIRC there are 4 DRBs. If it fails to build with too few registers, I think you can nuke the extra variable and just do multiple reads to the same location, or else reuse reg32 in place of reg8_2. I'm not really sure why the previous version failed, it looks sound to me. It might be interesting to put in some debugging output, to see what's going wrong.
Yep that worked great:-) Here is a revised patch with all the suggested changes.
Signed-off-by: Joseph Smith joe@smittys.pointclark.net
Thanks - Joe
On Sat, Mar 08, 2008 at 11:17:24PM -0500, joe@smittys.pointclark.net wrote:
Quoting Corey Osgood corey.osgood@gmail.com:
Try this instead, IIRC there are 4 DRBs. If it fails to build with too few registers, I think you can nuke the extra variable and just do multiple reads to the same location, or else reuse reg32 in place of reg8_2. I'm not really sure why the previous version failed, it looks sound to me. It might be interesting to put in some debugging output, to see what's going wrong.
Yep that worked great:-) Here is a revised patch with all the suggested changes.
Signed-off-by: Joseph Smith joe@smittys.pointclark.net
Thanks, committed in r3129 with variour cleanups for now. Will post further suggestions and/or patches later. Please add a wiki status page for the board which says which parts already work and which don't.
Uwe.
Quoting Uwe Hermann uwe@hermann-uwe.de:
On Sat, Mar 08, 2008 at 11:17:24PM -0500, joe@smittys.pointclark.net wrote:
Quoting Corey Osgood corey.osgood@gmail.com:
Try this instead, IIRC there are 4 DRBs. If it fails to build with too few registers, I think you can nuke the extra variable and just do multiple reads to the same location, or else reuse reg32 in place of reg8_2. I'm not really sure why the previous version failed, it looks sound to me. It might be interesting to put in some debugging output, to see what's going wrong.
Yep that worked great:-) Here is a revised patch with all the suggested changes.
Signed-off-by: Joseph Smith joe@smittys.pointclark.net
Thanks, committed in r3129 with variour cleanups for now. Will post further suggestions and/or patches later. Please add a wiki status page for the board which says which parts already work and which don't.
Thanks Uwe. Any suggestions, comments and/or patches are welcome. At this point the only thing that isn't working is the tv-out. But that is soon to come. Oh there is the flashrom issue too......but that will be solved soon also (hopefully). Who do I need to talk to about getting Wiki access, Ron??
Thanks - Joe
-
Corey Osgood -
joe＠smittys.pointclark.net -
Peter Stuge -
Uwe Hermann