Hi,
On Fri, Mar 14, 2008 at 03:53:54PM +0500, Nikolay Petukhov wrote:
Now coreboot perform irq routing for some boards. You can see this by executing commands like this: grep -r pci_assign_irqs coreboot/src/*
This basically AMD/LX based boards: pcengines/alix1c, digitallogic/msm800sev, artecgroup/dbe61, amd/norwich, amd/db800.
Also for AMD/GX1 based boards need a patch [http://www.pengutronix.de/software/ptxdist/temporary-src/references/geode-55...] for the right setup irq. AMD/GX1 based boards is: advantech/pcm-5820, asi/mb_5blmp, axus/tc320, bcom/winnet100, eaglelion/5bcm, iei/nova4899r, iei/juki-511p
I have two ideas.
- Delete duplicate code from AMD/LX based boards.
- Add irq routing for AMD/GX1 boards in coreboot.
The pirq.patch for irq routing logically consist from of two parts:
First part of pirq.patch independent from type chipsets and assign irq for ever PCI device. It part based on AMD/LX write_pirq_routing_table() function.
Second part of pirq.part depends of type chipset and set PIRQx linies in interrupt router. Now this part supports only CS5530/5536 interrupt routers.
Irq routing functionality is include through macro PIRQ_ROUTE in Config.lb.
None of the other boards use PIRQ_ROUTE now. So, if we commit this patch, will there be a change in behaviour for those boards? Or will they continue to work as before, unless someone adds PIRQ_ROUTE?
We should make sure that we don't break any boards.
It would be nice if someone with a CS5536 system could also test this patch, I don't have one.
Tested on iei/juki-511p(cs5530a), iei/pcisa-lx(cs5536) and also on TeleVideo TC7020 http://www.coreboot.org/pipermail/coreboot/2007-December/027973.html.
Looks good, thanks.
I'll test this on one or two GX1 systems and comment on how it works.
But please add a Signed-off-by to all patches, otherwise we cannot commit them, see http://www.coreboot.org/Development_Guidelines#Sign-off_Procedure No need to repost the whole patch, but please reply with an email which has your Signed-off-by.
Also, this is unrelated, but please add license tags to all your images in the wiki, too. See http://www.coreboot.org/index.php?limit=50&title=Special%3AContributions... for a list of all images you uploaded.
Last issue - I assume you also have a patch for PCIISA-LX-800 in the queue? I guess it depends on this patch being comitted, correct?
diff -Nru coreboot-v2-3132/src/arch/i386/boot/pirq_routing.c coreboot-v2-3132-pirq/src/arch/i386/boot/pirq_routing.c --- coreboot-v2-3132/src/arch/i386/boot/pirq_routing.c 2005-01-19 19:06:41.000000000 +0500 +++ coreboot-v2-3132-pirq/src/arch/i386/boot/pirq_routing.c 2008-03-11 10:53:18.000000000 +0500 @@ -1,6 +1,7 @@ #include <console/console.h> #include <arch/pirq_routing.h> #include <string.h> +#include <device/pci.h>
#if (DEBUG==1 && HAVE_PIRQ_TABLE==1) static void check_pirq_routing_table(struct irq_routing_table *rt) @@ -94,6 +95,80 @@ memcpy((void *)addr, &intel_irq_routing_table, intel_irq_routing_table.size); printk_info("done.\n"); verify_copy_pirq_routing_table(addr);
- pirq_routing_irqs(addr); return addr + intel_irq_routing_table.size;
} #endif
+#if (PIRQ_ROUTE==1 && HAVE_PIRQ_TABLE==1) +void pirq_routing_irqs(unsigned long addr) +{
- int i, j, k, num_entries;
- unsigned char irq_slot[4];
- unsigned char pirq[4] = {0, 0, 0, 0};
unsigned char -> u8?
diff -Nru coreboot-v2-3132/src/arch/i386/include/arch/pirq_routing.h coreboot-v2-3132-pirq/src/arch/i386/include/arch/pirq_routing.h --- coreboot-v2-3132/src/arch/i386/include/arch/pirq_routing.h 2005-07-07 00:17:35.000000000 +0600 +++ coreboot-v2-3132-pirq/src/arch/i386/include/arch/pirq_routing.h 2008-03-11 10:53:18.000000000 +0500 @@ -42,6 +42,12 @@ #if HAVE_PIRQ_TABLE==1 unsigned long copy_pirq_routing_table(unsigned long start); unsigned long write_pirq_routing_table(unsigned long start); +#if PIRQ_ROUTE==1 +void pirq_routing_irqs(unsigned long start); +void pirq_assign_irqs(const unsigned char pIntAtoD[4]);
u8
+#if (PIRQ_ROUTE==1 && HAVE_PIRQ_TABLE==1) +void pirq_assign_irqs(const unsigned char pIntAtoD[4])
u8
+{
- device_t pdev;
- pdev = dev_find_device(PCI_VENDOR_ID_CYRIX, PCI_DEVICE_ID_CYRIX_5530_LEGACY, 0);
The PCI IDs for CS5530 and CS5530A are the same, I assume?
/* Put the PIR table in memory and checksum. */
- return pirtable_end;
-} \ ? ????? ????? ??? ????? ??????
What's this? Seems to be some garbage which crept into the patch?
Nice patch all in all. It drops more code than it adds _and_ improves functionality -- I just like this kind of patches :)
Uwe.