On Fri, May 25, 2007 at 05:47:51AM -0400, Corey Osgood wrote:
See attached patch. There's still some work to be done on this, but my laptop's out of commission for a week or so, so I figure I'll let anyone who's bored do some hacking. As is, it does boot a payload, and probably would do a kernel with either acpi or irq tables, but should be considered a WIP. Feedback and comments are appreciated!
Nice! This looks very good. I'd like to make some changes before we commit, though.
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...
I haven't updated any of the license headers to be GPLv2 ones, mainly because I don't want to make a bad assumption. Not quite sure what to do with them...
Maybe you can modify the i82801er code? That was originally named ich5_r and written by Eric and/or Yinghai so we can assume it's GPLv2 (all of Eric's code is).
If the i82801er is too different, please modify the i82801ca and add the proper license headers. The i82801ca is based on the i82801er code, and was written by svn user "magnanisj" (not sure who that is).
Index: src/northbridge/intel/i810/Config.lb
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)
+#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...
- for(i = 0; i < DIMM_SOCKETS; i++)
^
- {
/* First check if a DIMM is actually present */
if(smbus_read_byte(ctrl->channel0[i], 2) == 4)
^ space needed
(coding guidelines, http://linuxbios.org/Development_Guidelines#Coding_Style)
dimm_size = smbus_read_byte(ctrl->channel0[i], 31);
smbus_read_byte -> spd_read_byte
31 -> SPD_BANK_DENSITY
/* The i810 can't handle dimms larger than 128MB per side */
/* Note: the factory BIOS just dies if it spots this :D */
Nice entry for a "Why LinuxBIOS sucks less" list in the wiki ;-)
if(dimm_size < 32)
{
print_err("DIMM row sizes larger than 128MB not
supported on i810\r\n");
print_err("Treating as 128MB DIMM\r\n");
Will this work? Did you test it?
/* If the dimm is dual-sided, the DRP value is +2 */
/* TODO: Figure out asymetrical configurations */
if((smbus_read_byte(ctrl->channel0[i], 127) | 0xf) == 0xff)
smbus_read_byte -> spd_read_byte
127 -> SPD_INTEL_SPEC_100_MHZ
- /* TODO */
- pci_write_config8(ctrl->d0, PAM, 0x00);
This should probably be set to "Read/Write, all accesses are directed to system memory DRAM." for all areas (?)
+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).
Index: src/northbridge/intel/i810/debug.c
--- src/northbridge/intel/i810/debug.c (revision 0) +++ src/northbridge/intel/i810/debug.c (revision 0) @@ -0,0 +1,35 @@
+static void dump_spd_registers(const struct mem_controller *ctrl) +{
- int i;
- print_debug("\r\n");
- for(i = 0; i < 4; i++) {
unsigned device;
device = ctrl->channel0[i];
if (device) {
int j;
print_debug("dimm: ");
print_debug_hex8(i);
print_debug(".0: ");
print_debug_hex8(device);
for(j = 0; j < 256; j++) {
int status;
unsigned char byte;
if ((j & 0xf) == 0) {
print_debug("\r\n");
print_debug_hex8(j);
print_debug(": ");
}
status = smbus_read_byte(device, j);
if (status < 0) {
print_debug("bad device\r\n");
break;
}
byte = status & 0xff;
print_debug_hex8(byte);
print_debug_char(' ');
}
print_debug("\r\n");
}
- }
+}
I'd like to drop this. There are hundreds of debug.c all over the place. Using src/sdram/generic_dump_spd.c should work just the same, correct?
+/* Table which returns the ram size in MB when fed the DRP[7:4] or [3:0] value */
ram -> RAM. Please use the correct spelling of all terms, even in comments.
+static void pci_domain_set_resources(device_t dev) +{
- device_t mc_dev;
uint32_t pci_tolm;
pci_tolm = find_pci_tolm(&dev->link[0]);
- mc_dev = dev->link[0].children;
- if (mc_dev) {
/* Figure out which areas are/should be occupied by RAM.
* This is all computed in kilobytes and converted to/from
* the memory controller right at the edges.
* Having different variables in different units is
* too confusing to get right. Kilobytes are good up to
* 4 Terabytes of RAM...
*/
unsigned long tomk, tolmk;
int idx;
int drp_value;
/* Get the value of the total memory, which we stored in scratch
* in raminit.c. The units are ticks of 8MB, i.e. 1 means 8MB.
*/
/*First get the value for DIMM 0 */
drp_value = pci_read_config8(mc_dev, DRP) & 0xf;
/* Translate it to MB and add to tomk */
tomk = (unsigned long)(translate_i810_to_mb[drp_value]);
/* Now do the same for DIMM 1 */
drp_value = pci_read_config8(mc_dev, DRP) >> 4;
Just a minor comment:
drp_value >>= 4
should work, too? No need to read the same value twice from the register...
/* Convert tomk from MB to KB */
tomk = tomk << 10;
tomk *= 1024;
/* Compute the top of Low memory */
tolmk = pci_tolm >> 10;
tolmk = pci_tolm / 1024;
+chip northbridge/intel/i810
- device pci_domain 0 on
chip southbridge/intel/i82801aa # Southbridge
device pci 1e.0 on end # PCI Bridge
device 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
#end
end
device pci 1f.1 on end # IDE
device pci 1f.2 on end # USB
device pci 1f.3 on end # SMBus
device pci 1f.5 off end # AC'97
device pci 1f.6 off end # AC'97 Modem (MC'97)?
end
device pci 0.0 on end # Host bridge
Not 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?
Index: src/mainboard/asus/mew-vm/debug.c
--- src/mainboard/asus/mew-vm/debug.c (revision 0) +++ src/mainboard/asus/mew-vm/debug.c (revision 0) @@ -0,0 +1,66 @@ +/* Generic debug ops, used by many mainboards */ +static void print_debug_pci_dev(unsigned dev) +{
- print_debug("PCI: ");
- print_debug_hex8((dev >> 16) & 0xff);
- print_debug_char(':');
- print_debug_hex8((dev >> 11) & 0x1f);
- print_debug_char('.');
- print_debug_hex8((dev >> 8) & 7);
+}
+static void print_pci_devices(void) +{
- device_t dev;
- for(dev = PCI_DEV(0, 0, 0);
dev <= PCI_DEV(0, 0x1f, 0x7);
dev += PCI_DEV(0,0,1)) {
uint32_t id;
id = pci_read_config32(dev, PCI_VENDOR_ID);
if (((id & 0xffff) == 0x0000) || ((id & 0xffff) == 0xffff) ||
(((id >> 16) & 0xffff) == 0xffff) ||
(((id >> 16) & 0xffff) == 0x0000)) {
continue;
}
print_debug_pci_dev(dev);
print_debug("\r\n");
- }
+}
+static void dump_pci_device(unsigned dev) +{
- int i;
- print_debug_pci_dev(dev);
- print_debug("\r\n");
- for(i = 0; i <= 255; i++) {
unsigned char val;
if ((i & 0x0f) == 0) {
print_debug_hex8(i);
print_debug_char(':');
}
val = pci_read_config8(dev, i);
print_debug_char(' ');
print_debug_hex8(val);
if ((i & 0x0f) == 0x0f) {
print_debug("\r\n");
}
- }
+}
+static void dump_pci_devices(void) +{
- device_t dev;
- for(dev = PCI_DEV(0, 0, 0);
dev <= PCI_DEV(0, 0x1f, 0x7);
dev += PCI_DEV(0,0,1)) {
uint32_t id;
id = pci_read_config32(dev, PCI_VENDOR_ID);
if (((id & 0xffff) == 0x0000) || ((id & 0xffff) == 0xffff) ||
(((id >> 16) & 0xffff) == 0xffff) ||
(((id >> 16) & 0xffff) == 0x0000)) {
continue;
}
dump_pci_device(dev);
- }
+}
Dito, please drop this.
I think there's a global debug.c somewhere, and if there isn't we should create one...
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.
+void udelay(int usecs) +{
- int i;
- for(i = 0; i < usecs; i++)
outb(i&0xff, 0x80);
+}
Is this needed? There's a global version somewhere.
Index: targets/asus/mew-vm/Config.lb
--- targets/asus/mew-vm/Config.lb (revision 0) +++ targets/asus/mew-vm/Config.lb (revision 0)
Please add a copyright header, here.
- option ROM_IMAGE_SIZE=0x10000
option ROM_IMAGE_SIZE = 64 * 1024
(this should really be done for all boards, makes the stuff a lot more readable IMO)
With the above changes I think we can commit the code.
Uwe.