changes today.
Stefan Reinauer
stepan at openbios.org
Fri Oct 1 17:36:01 CEST 2004
Hi Yinghai,
> 2. S2882 interrupt line assignment in mptable.c
> 3. exclude range for flash_rom.
> 4. 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 that we could do this a lot nicer dynamically with a
little more stack depth available (ie with working CAR)
> @@ -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?
> - 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
> 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.
> 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 depend
> + on the PCB routing of PINTA-D
> +
> + PINTA = IRQ5
> + PINTB = IRQ9
> + PINTC = IRQ11
> + PINTD = 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)
> diff -uNr ./freebios2/src/mainboard/tyan/s2885/resourcemap.c ../freebios2/src/mainboard/tyan/s2885/resourcemap.c
> --- ./freebios2/src/mainboard/tyan/s2885/resourcemap.c 2004-03-26 13:34:04.000000000 -0800
> +++ ../freebios2/src/mainboard/tyan/s2885/resourcemap.c 2004-09-20 11:00:09.000000000 -0700
> @@ -252,8 +252,8 @@
> * [31:24] Bus Number Limit i
> * This field defines the highest bus number in configuration regin i
> */
> - PCI_ADDR(0, 0x18, 1, 0xE0), 0x0000FC88, 0x06010207,
> - PCI_ADDR(0, 0x18, 1, 0xE4), 0x0000FC88, 0x00000007,
> + PCI_ADDR(0, 0x18, 1, 0xE0), 0x0000FC88, 0x06000203, // AMD 8111 on link2 of CPU 0
> + PCI_ADDR(0, 0x18, 1, 0xE4), 0x0000FC88, 0x08070003, // AMD 8151 on link0 of CPU 0
> PCI_ADDR(0, 0x18, 1, 0xE8), 0x0000FC88, 0x00000000,
> PCI_ADDR(0, 0x18, 1, 0xEC), 0x0000FC88, 0x00000000,
> };
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.
> 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
> @@ -214,7 +214,7 @@
> console_init();
> setup_s4880_resource_map();
> needs_reset = setup_coherent_ht_domain();
> - needs_reset |= ht_setup_chain(PCI_DEV(0, 0x18, 0), 0x80);
> + needs_reset |= ht_setup_chain(PCI_DEV(0, 0x18, 0), 0xc0);
Why are you not using the same struct as above for the s27xx?
> diff -uNr ./freebios2/src/northbridge/amd/amdk8/incoherent_ht.c ../freebios2/src/northbridge/amd/amdk8/incoherent_ht.c
> --- ./freebios2/src/northbridge/amd/amdk8/incoherent_ht.c 2004-04-15 10:33:21.000000000 -0700
> +++ ../freebios2/src/northbridge/amd/amdk8/incoherent_ht.c 2004-08-24 12:03:10.000000000 -0700
> @@ -247,19 +263,21 @@
> unsigned upos;
> unsigned devreg;
> };
> -
> -static int ht_setup_chainx(device_t udev, unsigned upos, unsigned next_unitid)
> +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 ?
This is some very fundamental piece of code that should be well
understood.
Same for the northbridge.c code.
Stefan
More information about the coreboot
mailing list