Just a couple quick responses, I'll get to work on changes as soon as I can coax fedora into working correctly again. minicom.cap attached, I forgot it before.
On Sat, 2007-05-26 at 15:29 +0200, Uwe Hermann wrote:
Also, the i82801aa is almost completely copy and pasted from the i82801ca, with the device IDs swapped over (bad practice, I know, but if ain't broke don't fix it).
This is change number 1 ;-)
The code is indeed fully copy+paste, which we should try to avoid. Can you change it to modify the i82801ca with #ifdef's to support the i82801aa, too? In the mainboard code you'd just have to
#define I82801AA 1
or something similar...
Agreed, as long as no further fixup is necessary. Perhaps I'll just hold onto this patch until I can fully test out the southbridge function, or else we could commit as-is and I'll do some cleanup later.
The main reason for the copy/paste was that I didn't think it would work out-of-the-box as well as it has (if at all). I was looking for something to start with and rewrite as needed, but that would be close enough to give me some useful idea what the problem was. Some fixups might still be needed to some of the device functions, especially USB and audio. I don't even have audio headers on my board, so I can't test that (I'll put a note of this in the code). In case you haven't noticed, I tend to be somewhat unimaginative and lazy, so I like to have some good or at least decent code to use as a guideline, then rip apart and fix as needed.
I should probably stress here that I haven't yet gotten serial output from a booting linux kernel although I do get to "loading kernel" and "loading initrd", see log attached. For some reason the stock ubuntu kernel either ignores that option, or else can't do it with the super io not going through its usual post-ram init. Or else the kernel just isn't booting at all. I'll work on testing all those possibilities soon.
I'll also look into the i82801er, and see what/where the big changes are. Perhaps all 3 can be merged into one big i82801xx mess.
Poll: do we want i810 or rather i82810? For the southbridges we use the full names (e.g. i82801aa), why don't we do the same with the northbridges?
(yes, that would also change i440bx to i82443bx)
Probably we should, the i440bx seems to be the only device known by its marketing name rather than part name, and some of the naming conventions marketing departments use are just plain stupid (*cough* Via *cough cough*). I'll fix the i810 as well.
+#include "i810.h"
+/*----------------------------------------------------------------------------- +Macros and definitions. +-----------------------------------------------------------------------------*/
+/* Uncomment this to enable debugging output. */ +#define DEBUG_RAM_SETUP 1
Btw, disabling this reduces the number of registers required for romcc, thus you can get further in the compile...
On i810, this isn't really a concern (yet), it already compiles/runs fine and isn't nearly as complex as the 440bx code i was working on. OTOH, it isn't as full-featured yet either, it's still using static timings and is somewhat limited in what it can handle.
if(dimm_size < 32){print_err("DIMM row sizes larger than 128MB notsupported on i810\r\n");print_err("Treating as 128MB DIMM\r\n");Will this work? Did you test it?
Well, at the moment it doesn't work at all and completely kills anything but 128mb dimms, because I messed up and flipped the comparison (<) the wrong way. Thanks for asking, I hadn't spotted this, nor had I tested with anything but my 128mb dimm after adding it. Whether it works or not remains to be seen, I'll need to finish some dynamic buffer strength stuff first (since I can't pull a working value from a system that refuses to boot).
+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 */- uint32_t row_offset;
- spd_set_dram_size(ctrl, row_offset);
- /* 1. Apply NOP. */
- PRINT_DEBUG("RAM Enable 1: Apply NOP\r\n");
- do_ram_command(ctrl, RAM_COMMAND_NOP, 0, row_offset);
- udelay(200);
Did you test the delays? It's likely that smaller values will suffice (not too important, though, RAM init is pretty fast anyway).
Yep, everything pre-ram has been tested on several 128mb single-sided dimms, guess I should have attached a boot log the first time. This was based on my copy of i440bx's raminit.c, and those delays work on there as well. They might be able to be tightened further, but I seriously doubt anyone's going to notice even a 50% speedup to a 200 microsecond delay.
+chip northbridge/intel/i810
- device pci_domain 0 on
chip southbridge/intel/i82801aa # Southbridgedevice pci 1e.0 on end # PCI Bridgedevice pci 1f.0 on # ISA/LPC? Bridge#chip superio/smsc/lpc47b272 # Super I/O# device pnp 2e.0 on end # Floppy# device pnp 2e.3 off end # Parallel port# device pnp 2e.4 on # COM1# io 0x60 = 0x3f8# irq 0x70 = 4# end# device pnp 2e.5 on end # COM2# device pnp 2e.4 on end # Power mgmt.# #device pnp 2e.5 on end # Mouse# device pnp 2e.7 on # Keyboard & Mouse?# io 0x60 = 0x60# io 0x62 = 0x64# irq 0x70 = 1# irq 0x72 = 12 # ???# end#endenddevice pci 1f.1 on end # IDEdevice pci 1f.2 on end # USBdevice pci 1f.3 on end # SMBusdevice pci 1f.5 off end # AC'97device pci 1f.6 off end # AC'97 Modem (MC'97)?end
device pci 0.0 on end # Host bridgeNot sure about this. I thought the order of the devices _does_ matter here? I.e. lower IDs must come first? Can somebody please clarify this?
I can't honestly remember why I did that, but it seems to work the same either way. Probably was part of my attempt to get the super io working, or else get rid of (fix?) some annoying log errors.
Anyways, just realized that the same superio is used by the xe7501devkit, and I doubt that would have been committed if it wasn't working. I'll give this another shot tomorrow, with a little tweaking, and check the svn logs to see what's changed since it was originally committed.
I think there's a global debug.c somewhere, and if there isn't we should create one...
There isn't that I can find, just about every mainboard has its own debug.c, and all are nearly identical. I'll do some more grepping, and see if I can find the functions in a file with a different name. Any suggestions where to put a generic one, if it comes to that? I'm thinking src/include/debug/mainboard_debug.c (and northbridge_debug.c, since it's the same way). Or else combine all the functions into one big debug.c and put it in the same place.
Index: src/mainboard/asus/mew-vm/auto.c
Oh, I'm not sure this will work or is a good idea.
I'd rather use "mew_vm" (instead of "mew-vm"). Pathnames may appear as variable names (for instance) in C code, that's also why all directory names start with a letter (i440bx, not 440bx).
Same for dashes, "mew-vm" is not a valid C variable name.
Somehow, somewhere in the build process, the dashes in the folder name(s) get converted to an underscore, so all the variables are named *mew_vm*, and this builds/runs just fine. If it still needs to be changed, just let me know.
Thanks for the review, Corey