Right now I feel like taffy, stretched thin and pulled in all directions. My apologies for not responding sooner.
Stefan Reinauer stepan@openbios.org writes:
Hi Yinghai,
- S2882 interrupt line assignment in mptable.c
- exclude range for flash_rom.
- better ht_setup_chainx() in incoherent_ht.c ---> change share bus 0 to
different bus for different incoherent HT link. 5. fix one bug about io and mem allocation for multi ht links. --- in northbridge.c Mark it if used before assign final value. 6. enable amd/quartet to init multi incoherent HT link in auto.c --- not test, no MB on hand. 7. enable S2885 to init multi incoherent HT link in auto.c
diff -uNr ./freebios2/src/mainboard/amd/quartet/auto.c
../freebios2/src/mainboard/amd/quartet/auto.c
--- ./freebios2/src/mainboard/amd/quartet/auto.c 2004-04-28 01:08:06.000000000
-0700
+++ ../freebios2/src/mainboard/amd/quartet/auto.c 2004-09-29
09:30:57.000000000 -0700
@@ -100,11 +100,12 @@ */
uint32_t ret = 0x00010101; /* default row entry */
+/* static const unsigned int rows_2p[2][2] = { {0x00030101, 0x00010202}, {0x00010202, 0x00030101} }; +*/
static const unsigned int rows_4p[4][4] = { {0x00070101, 0x00010202, 0x00030404, 0x00010204}, @@ -114,9 +115,11 @@ };
if (!(node >= maxnodes || row >= maxnodes)) { +/* if (maxnodes == 2) ret = rows_2p[node][row]; if (maxnodes == 4) +*/ ret = rows_4p[node][row]; }
I am in doubt this is a good idea. It will likely break 4p systems with only 2 cpus installed. The better idea might be to mask the according links decently.
Which reminds me I need to push my patch that properly handles the case of not having all of your cpus plugged in.
Which reminds me that we could do this a lot nicer dynamically with a little more stack depth available (ie with working CAR)
Quite possibly. I'm not holding my breath on this one.
@@ -199,6 +203,20 @@ };
static const struct ht_chain ht_c[] = {{ /* Link 2 of CPU0 */.udev = PCI_DEV(0, 0x18, 0),.upos = 0xc0,- .devreg = 0xe0, /* Preset bus num in resource map */
},{ /* Link 1 of CPU1 */.udev = PCI_DEV(0, 0x19, 0),.upos = 0xa0,- .devreg = 0xe4, /* Preset bus num in resource map */
},};int needs_reset;
enable_lapic();
Can we generate above struct from the information we have in the config tool? at least the cpu link should be there. What's the bus num again?
Ideally we could just put a loop through the HT links on the cpus and if the link is up and not coherent enumerate it. I suspect that would be simples.
- asm("jmp __normal_image"
- asm volatile ("jmp __normal_image"
Can't this be fixed in romcc instead? If we have to put an asm volatile statement instead of every asm, we could as well just do #define asm asm volatile or the adequate change in the romcc code
The rules with romcc are exactly the same as the rules in gcc.
The rdtsc case I'm not certain about. But for the rest romcc will optimize out a non-volatile asm that has neither inputs or outputs. If romcc knew the jumps affected control flow there would not be a problem. Since it does not those statements need to be volatile.
diff -uNr ./freebios2/src/mainboard/tyan/s2882/irq_tables.c
../freebios2/src/mainboard/tyan/s2882/irq_tables.c
--- ./freebios2/src/mainboard/tyan/s2882/irq_tables.c 2004-03-12
07:13:31.000000000 -0800
+++ ../freebios2/src/mainboard/tyan/s2882/irq_tables.c 2004-07-02
10:01:48.000000000 -0700
@@ -18,7 +18,7 @@ 0x746b, /* Device */ 0, /* Crap (miniport) */ { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 }, /* u8 rfu[11] */
- 0x8d, /* u8 checksum , this hase to set to some value that would give 0
after the sum of all bytes for this structure (including checksum) */
- 0xff, /* u8 checksum , this hase to set to some value that would give 0
after the sum of all bytes for this structure (including checksum) */
{ {1,(4<<3)|0, {{0x1, 0xdef8}, {0x2, 0xdef8}, {0x3, 0xdef8}, {0x4, 0xdef8}}, 0,
0},
{0x4,0, {{0, 0}, {0, 0}, {0, 0}, {0x4, 0xdef8}}, 0, 0},To get this right, does Linux expect the PIRQ table to be in ROM in any condition? The code will correct the table's checksum in RAM, and the goal should definitely be to create all the table in ram during bootup. I saw phoenix bios almost writes a nice dynamic device tree into the pirq table, including CPUs and busses, instead of just putting the most necessary lines for the pci slots in. This sounds like a way to go.
Linux looks at 0xf00000 which is either ROM or RAM. I'm looking at finding the time to do a complete rewrite based on our dynamic device tree. What exists currently is a major pain.
mptable.c
+#if ASSIGN_IRQ smp_write_intsrc(mc, mp_INT, MP_IRQ_TRIGGER_LEVEL|MP_IRQ_POLARITY_LOW, 0x1,
(4<<2)|3, 0x2, 0x13);
- {
device_t dev;dev = dev_find_device(PCI_VENDOR_ID_AMD, 0x746b, 0);if (dev) {/* initialize PCI interupts - these assignments dependon the PCB routing of PINTA-DPINTA = IRQ5PINTB = IRQ9PINTC = IRQ11PINTD = IRQ10*/pci_write_config16(dev, 0x56, 0xab95);}}+#endif
+#if ASSIGN_IRQ
printk_info("setting Onboard AMD Southbridge \n");static const unsigned char slotIrqs_1_4[4] = { 5, 9, 11, 10 };pci_assign_irqs(1, 4, slotIrqs_1_4);+#endif
Does this belong into the mptable generation? I would assume it better fits into the "driver" code pieces
Otherwise I see that we have to shift all motherboards to use the above mechanism, ending up with dozens of half broken mp tables (worse than it is now already)
True but at least at the moment it is motherboard specific code. So it is ``only'' a bad example. This is equally doable with just the pirq table, but the result (setting the interrupt line register is desirable.
I remember I commited some code a long time ago that detected the southbridge link and set it correctly. Looks like it got dumped by accident. We should do this again, and the same for the 8151.
Hmm. Something like that surely.
diff -uNr ./freebios2/src/mainboard/tyan/s4880/auto.c
../freebios2/src/mainboard/tyan/s4880/auto.c
--- ./freebios2/src/mainboard/tyan/s4880/auto.c 2004-04-24 16:01:19.000000000
-0700
+++ ../freebios2/src/mainboard/tyan/s4880/auto.c 2004-08-31 18:57:46.000000000
-0700
@@ -28,7 +28,7 @@ set_bios_reset();
/* enable cf9 */
pci_write_config8(PCI_DEV(1, 0x04, 3), 0x41, 0xf1);
pci_write_config8(PCI_DEV(0, 0x04, 3), 0x41, 0xf1);Is this not configurable in the config file, as well, by now, in some places? We need to factorize this kind of code a lot more.. Every change hurts, because the opteron board support code is drifting in many directions
Something things we do need to setup early, and resets look like one of them.
+static int ht_setup_chainx(device_t udev, unsigned upos, unsigned bus) {
- unsigned last_unitid;
unsigned next_unitid, last_unitid; unsigned uoffs; int reset_needed=0;
uoffs = PCI_HT_HOST_OFFS;
next_unitid = 1;
do { uint32_t id; uint8_t pos; unsigned flags, count;
device_t dev = PCI_DEV(0, 0, 0);
device_t dev = PCI_DEV(bus, 0, 0);last_unitid = next_unitid;
id = pci_read_config32(dev, PCI_VENDOR_ID);
@@ -293,8 +328,7 @@ next_unitid += count;
} while((last_unitid != next_unitid) && (next_unitid <= 0x1f));
- if(reset_needed!=0) next_unitid |= 0xffff0000;
- return next_unitid;
- return reset_needed;
}
Did we not support icht chains hanging off any bus other than 0 before?! Can you add some comments to the changes you made to incoherent_ht.c ?
Stefan it is not devices hanging of bus 0. It is what bus number the chain will be assigned. For the same reason the generic code does not use bus 0, this code is simplest if we use another bus number as well.
This is some very fundamental piece of code that should be well understood. Same for the northbridge.c code.
What is happening in northbridge.c is a legitimate bug fix. Because we may not set the resource for a while. I would prefer not to use the pci configuration registers as scratch registers, but that may be the simplest way to do things.
Eric