PCI irq routing

[Nikolay Petukhov]

Hi, all.

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.
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.

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.

-- 
Nikolay

[ron minnich]

Very nice! Is there any way I can convince you to work on v3 instead :-)

I can't really test but this looks like pretty good design and code.

Thanks

ron

[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-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.
> 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&con...
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

[Nikolay Petukhov]

Hi.

2008/3/14, Uwe Hermann <[e-mail]>:
> 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.

Yes, other boards will work as before.
> 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.

Signed-off-by: Nikolay Petukhov <nikolay.petukhov at gmail.com>
> 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&con...
>  for a list of all images you uploaded.

I add license tags to all images.
> 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?

Yes, you are right.

-- 
Nikolay

[Uwe Hermann]

On Mon, Mar 17, 2008 at 12:34:04PM +0500, Nikolay Petukhov wrote:
> Signed-off-by: Nikolay Petukhov <nikolay.petukhov at gmail.com>

Thanks, committed in r3196 (sorry for the delay).
> I add license tags to all images.

Great, thanks!
> > 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?
>
> Yes, you are right.

Feel free to post that patch now.


Uwe.
-- 
http://www.hermann-uwe.de  | http://www.holsham-traders.de
http://www.crazy-hacks.org | http://www.unmaintained-free-software.org

[Carl-Daniel Hailfinger]

On 29.03.2008 18:00, Uwe Hermann wrote:
> On Mon, Mar 17, 2008 at 12:34:04PM +0500, Nikolay Petukhov wrote:
> > Signed-off-by: Nikolay Petukhov <nikolay.petukhov at gmail.com>
>
> Thanks, committed in r3196 (sorry for the delay).

What's the PCI IRQ routing status of v2 and v3? Does one of them have an
advantage over the other?

Regards,
Carl-Daniel

[Nikolay Petukhov]

2008/3/29, Carl-Daniel Hailfinger <[e-mail]>:
> What's the PCI IRQ routing status of v2 and v3? Does one of them have an
>  advantage over the other?
>
> Regards,
>
> Carl-Daniel

PCI IRQ routing of v2 support two platforms - AMD/LX and AMD/GX1.
PCI IRQ routing of v3 support only one platform - AMD/LX.

-- 
Nikolay