[coreboot] PCI irq routing

[Uwe Hermann]

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-5530.patch]
> 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.
> 1. Delete duplicate code from AMD/LX based boards.
> 2. 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&contribs=user&target=Nikola&namespace=6&year=&month=-1
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.
-- 
http://www.hermann-uwe.de  | http://www.holsham-traders.de
http://www.crazy-hacks.org | http://www.unmaintained-free-software.org