VIA IRQ quirk missing PCI ids since 2.6.16.17

VIA IRQ quirk missing PCI ids since 2.6.16.17

[Wilco Beekhuizen]

Hi, since 2.6.17.17 in drivers/pci/quirks.c (quirk_via_irq) all VIA
chipsets are listed seperately instead of the "include everything"
PCI_ANY_ID.

This is however problematic with my chipset.
The ethernet controller, a VT6102 (Rhine-II) and the audio controller,
VT8233/A/8235/8237 need a fix to work.
Including PCI_ANY_ID again fixes these problems but is of course a
pretty evil fix. The problem is I can't find out which PCI ids to
include. I'm new to this list so suggestions are welcome.

Wilco

Re: VIA IRQ quirk missing PCI ids since 2.6.16.17

[Sergio Monteiro Basto]

On Mon, 2006-11-06 at 12:38 +0100, Wilco Beekhuizen wrote:
> Hi, since 2.6.17.17 in drivers/pci/quirks.c (quirk_via_irq) all VIA
> chipsets are listed seperately instead of the "include everything"
> PCI_ANY_ID.
> 
> This is however problematic with my chipset.
> The ethernet controller, a VT6102 (Rhine-II) and the audio controller,
> VT8233/A/8235/8237 need a fix to work.
> Including PCI_ANY_ID again fixes these problems but is of course a
> pretty evil fix. The problem is I can't find out which PCI ids to
> include. I'm new to this list so suggestions are welcome.

this is the latest patch 
ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.18/2.6.18-mm1/broken-out/via-irq-quirk-behaviour-change.patch
about this issue please try it and report the experience :) 


> 
> Wilco
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: VIA IRQ quirk missing PCI ids since 2.6.16.17

[Alan Cox]

Ar Llu, 2006-11-06 am 12:47 +0000, ysgrifennodd Sergio Monteiro Basto:
> this is the latest patch 
> ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.18/2.6.18-mm1/broken-out/via-irq-quirk-behaviour-change.patch
> about this issue please try it and report the experience :) 

There are several minor problems with this patch:

- V-Link which is what we are dealing with is only found in the VIA 8233
and later devices and early stuff should not be touched
- It whacks external devices

I think something like this (untested) would be better

diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.19-rc4-mm1/drivers/pci/quirks.c linux-2.6.19-rc4-mm1/drivers/pci/quirks.c
--- linux.vanilla-2.6.19-rc4-mm1/drivers/pci/quirks.c	2006-10-31 21:11:49.000000000 +0000
+++ linux-2.6.19-rc4-mm1/drivers/pci/quirks.c	2006-11-06 20:39:03.026705152 +0000
@@ -641,48 +641,42 @@
 DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_VIA,	PCI_DEVICE_ID_VIA_82C586_3,	quirk_via_acpi );
 DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_VIA,	PCI_DEVICE_ID_VIA_82C686_4,	quirk_via_acpi );
 
-/*
- * Via 686A/B:  The PCI_INTERRUPT_LINE register for the on-chip
- * devices, USB0/1, AC97, MC97, and ACPI, has an unusual feature:
- * when written, it makes an internal connection to the PIC.
- * For these devices, this register is defined to be 4 bits wide.
- * Normally this is fine.  However for IO-APIC motherboards, or
- * non-x86 architectures (yes Via exists on PPC among other places),
- * we must mask the PCI_INTERRUPT_LINE value versus 0xf to get
- * interrupts delivered properly.
- *
- * Some of the on-chip devices are actually '586 devices' so they are
- * listed here.
- */
-
-static int via_irq_fixup_needed = -1;
 
 /*
- * As some VIA hardware is available in PCI-card form, we need to restrict
- * this quirk to VIA PCI hardware built onto VIA-based motherboards only.
- * We try to locate a VIA southbridge before deciding whether the quirk
- * should be applied.
- */
-static const struct pci_device_id via_irq_fixup_tbl[] = {
-	{
-		.vendor 	= PCI_VENDOR_ID_VIA,
-		.device		= PCI_ANY_ID,
-		.subvendor	= PCI_ANY_ID,
-		.subdevice	= PCI_ANY_ID,
-		.class		= PCI_CLASS_BRIDGE_ISA << 8,
-		.class_mask	= 0xffff00,
-	},
+ *	VIA bridges which have VLink
+ */
+ 
+static const struct pci_device_id via_vlink_fixup_tbl[] = {
+	{ PCI_VDEVICE(VIA, PCI_DEVICE_ID_VIA_8233_0), 17},
+	{ PCI_VDEVICE(VIA, PCI_DEVICE_ID_VIA_8233A), 17 },
+	{ PCI_VDEVICE(VIA, PCI_DEVICE_ID_VIA_8233C_0), 17 },
+	{ PCI_VDEVICE(VIA, PCI_DEVICE_ID_VIA_8235), 16 },
+	/* May not be needed for the 8237 */
+	{ PCI_VDEVICE(VIA, PCI_DEVICE_ID_VIA_8237), 15 },
+	{ PCI_VDEVICE(VIA, PCI_DEVICE_ID_VIA_8237A), 15 },
 	{ 0, },
 };
 
-static void quirk_via_irq(struct pci_dev *dev)
+/**
+ *	quirk_via_vlink		-	VIA VLink IRQ number update
+ *	@dev: PCI device
+ *
+ *	If the device we are dealing with is on a PIC IRQ we need to
+ *	ensure that the IRQ line register which usually is not relevant
+ *	for PCI cards, is actually written so that interrupts get sent
+ *	to the right place
+ */
+
+static void quirk_via_vlink(struct pci_dev *dev)
 {
+	static struct pci_device_id *via_vlink_fixup = NULL;
 	u8 irq, new_irq;
 
-	if (via_irq_fixup_needed == -1)
-		via_irq_fixup_needed = pci_dev_present(via_irq_fixup_tbl);
-
-	if (!via_irq_fixup_needed)
+	/* Check if we have VLink and cache the result */
+	
+	if (via_vlink_fixup == NULL)
+		via_vlink_fixup = pci_find_present(via_irq_fixup_tbl);
+	if (via_vlink_fixup == NULL)
 		return;
 
 	new_irq = dev->irq;
@@ -691,9 +685,17 @@
 	if (!new_irq || new_irq > 15)
 		return;
 
+	/* Internal device ? */
+	if (pdev->bus->number != 0 || PCI_FUNC(dev->devfn) > 18 || 
+		PCI_FUNC(dev->devfn) < via_vlink_fixup->driver_data)
+		return;
+	
+	/* This is an internal VLink device on a PIC interrupt. The BIOS
+	   ought to have set this but may not have, so we redo it */
+	
 	pci_read_config_byte(dev, PCI_INTERRUPT_LINE, &irq);
 	if (new_irq != irq) {
-		printk(KERN_INFO "PCI: VIA IRQ fixup for %s, from %d to %d\n",
+		printk(KERN_INFO "PCI: VIA VLink IRQ fixup for %s, from %d to %d\n",
 			pci_name(dev), irq, new_irq);
 		udelay(15);	/* unknown if delay really needed */
 		pci_write_config_byte(dev, PCI_INTERRUPT_LINE, new_irq);
diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.19-rc4-mm1/drivers/pci/search.c linux-2.6.19-rc4-mm1/drivers/pci/search.c
--- linux.vanilla-2.6.19-rc4-mm1/drivers/pci/search.c	2006-10-31 21:11:31.000000000 +0000
+++ linux-2.6.19-rc4-mm1/drivers/pci/search.c	2006-11-06 20:30:55.548813056 +0000
@@ -413,30 +413,17 @@
 	return dev;
 }
 
-/**
- * pci_dev_present - Returns 1 if device matching the device list is present, 0 if not.
- * @ids: A pointer to a null terminated list of struct pci_device_id structures
- * that describe the type of PCI device the caller is trying to find.
- *
- * Obvious fact: You do not have a reference to any device that might be found
- * by this function, so if that device is removed from the system right after
- * this function is finished, the value will be stale.  Use this function to
- * find devices that are usually built into a system, or for a general hint as
- * to if another device happens to be present at this specific moment in time.
- */
-int pci_dev_present(const struct pci_device_id *ids)
+struct pci_device_id *pci_find_present(const struct pci_device_id *ids)
 {
 	struct pci_dev *dev;
-	int found = 0;
+	struct pci_device_id * found = NULL;
 
 	WARN_ON(in_interrupt());
 	down_read(&pci_bus_sem);
 	while (ids->vendor || ids->subvendor || ids->class_mask) {
 		list_for_each_entry(dev, &pci_devices, global_list) {
-			if (pci_match_one_device(ids, dev)) {
-				found = 1;
-				goto exit;
-			}
+			if ((found = pci_match_one_device(ids, dev)) != NULL)
+				break;
 		}
 		ids++;
 	}
@@ -444,7 +431,26 @@
 	up_read(&pci_bus_sem);
 	return found;
 }
+
+/**
+ * pci_dev_present - Returns 1 if device matching the device list is present, 0 if not.
+ * @ids: A pointer to a null terminated list of struct pci_device_id structures
+ * that describe the type of PCI device the caller is trying to find.
+ *
+ * Obvious fact: You do not have a reference to any device that might be found
+ * by this function, so if that device is removed from the system right after
+ * this function is finished, the value will be stale.  Use this function to
+ * find devices that are usually built into a system, or for a general hint as
+ * to if another device happens to be present at this specific moment in time.
+ */
+
+int pci_dev_present(const struct pci_device_id *ids)
+{
+	return pci_find_present(ids) == NULL ? 0 : 1;
+}
+
 EXPORT_SYMBOL(pci_dev_present);
+EXPORT_SYMBOL(pci_find_present);
 
 EXPORT_SYMBOL(pci_find_device);
 EXPORT_SYMBOL(pci_find_device_reverse);
diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.19-rc4-mm1/include/linux/pci.h linux-2.6.19-rc4-mm1/include/linux/pci.h
--- linux.vanilla-2.6.19-rc4-mm1/include/linux/pci.h	2006-10-31 21:11:50.000000000 +0000
+++ linux-2.6.19-rc4-mm1/include/linux/pci.h	2006-11-06 20:31:30.524495944 +0000
@@ -461,6 +461,7 @@
 struct pci_dev *pci_get_bus_and_slot (unsigned int bus, unsigned int devfn);
 struct pci_dev *pci_get_class (unsigned int class, struct pci_dev *from);
 int pci_dev_present(const struct pci_device_id *ids);
+struct pci_device_id *pci_find_present(const struct pci_device_id *ids);
 
 int pci_bus_read_config_byte (struct pci_bus *bus, unsigned int devfn, int where, u8 *val);
 int pci_bus_read_config_word (struct pci_bus *bus, unsigned int devfn, int where, u16 *val);
@@ -684,6 +685,7 @@
 { return NULL; }
 
 #define pci_dev_present(ids)	(0)
+#define pci_find_present(ids)	(NULL)
 #define pci_dev_put(dev)	do { } while (0)
 
 static inline void pci_set_master(struct pci_dev *dev) { }

Re: VIA IRQ quirk missing PCI ids since 2.6.16.17

[Dave Jones]

On Mon, Nov 06, 2006 at 09:13:45PM +0000, Alan Cox wrote:

 > +static const struct pci_device_id via_vlink_fixup_tbl[] = {
 > +	{ PCI_VDEVICE(VIA, PCI_DEVICE_ID_VIA_8233_0), 17},
 > +	{ PCI_VDEVICE(VIA, PCI_DEVICE_ID_VIA_8233A), 17 },
 > +	{ PCI_VDEVICE(VIA, PCI_DEVICE_ID_VIA_8233C_0), 17 },
 > +	{ PCI_VDEVICE(VIA, PCI_DEVICE_ID_VIA_8235), 16 },
 > +	/* May not be needed for the 8237 */
 > +	{ PCI_VDEVICE(VIA, PCI_DEVICE_ID_VIA_8237), 15 },
 > +	{ PCI_VDEVICE(VIA, PCI_DEVICE_ID_VIA_8237A), 15 },
 >  	{ 0, },

This got me wondering what PCI_VDEVICE was, so I went looking.
It's a libata'ism it seems with the comment..

/* move to PCI layer? */

Which sounds like a good idea to me.  But until this is moved,
does quirks.c actually compile with this patch? I don't see
an include of linux/libata.h there.

When it gets moved to the PCI layer, I wonder if it'd be worth
doing the same thing to the second argument, so that we'd be
able to do..

	{ PCI_VDEVICE(VIA, VIA_8233_0), 17},

Or maybe even..

	{ PCI_VDEVICE(VIA, 8233_0), 17},

?

	Dave

-- 
http://www.codemonkey.org.uk

Re: VIA IRQ quirk missing PCI ids since 2.6.16.17

[Alan Cox]

Ar Llu, 2006-11-06 am 20:25 -0500, ysgrifennodd Dave Jones:
> This got me wondering what PCI_VDEVICE was, so I went looking.
> It's a libata'ism it seems with the comment..
> 
> /* move to PCI layer? */
> 
> Which sounds like a good idea to me.  But until this is moved,
> does quirks.c actually compile with this patch? I don't see
> an include of linux/libata.h there.

Probably not - as I said "not tested"

Re: VIA IRQ quirk missing PCI ids since 2.6.16.17

[Wilco Beekhuizen]

Why was this changed in the stable kernel anyway, especially in a
micro-stability update? It seems to me it breaks more than it fixes.

Re: VIA IRQ quirk missing PCI ids since 2.6.16.17

[Sergio Monteiro Basto]

Hopefully in 2.6.19, we will get the patch that I mention in this
thread , Had you test it ? 
 
On Wed, 2006-11-08 at 09:54 +0100, Wilco Beekhuizen wrote:
> Why was this changed in the stable kernel anyway, especially in a
> micro-stability update? It seems to me it breaks more than it fixes.

Re: VIA IRQ quirk missing PCI ids since 2.6.16.17

[Wilco Beekhuizen]

Yes your patch works fine but I saw some objections from Alan.

Re: VIA IRQ quirk missing PCI ids since 2.6.16.17

[Alan Cox]

Ar Mer, 2006-11-08 am 09:54 +0100, ysgrifennodd Wilco Beekhuizen:
> Why was this changed in the stable kernel anyway, especially in a
> micro-stability update? It seems to me it breaks more than it fixes.

Because it suffered from an acute case of being wrong. The blanket patch
proposed by Sergio is also wrong. Both break valid correct and working
systems while fixing some others.

The draft patch I posted fixes up precisely the right devices on
precisely the right bridges and nothing else which should mean we now
have a patch that gets all cases right.

Re: VIA IRQ quirk missing PCI ids since 2.6.16.17

[Greg KH]

On Mon, Nov 06, 2006 at 08:25:19PM -0500, Dave Jones wrote:
> On Mon, Nov 06, 2006 at 09:13:45PM +0000, Alan Cox wrote:
> 
>  > +static const struct pci_device_id via_vlink_fixup_tbl[] = {
>  > +	{ PCI_VDEVICE(VIA, PCI_DEVICE_ID_VIA_8233_0), 17},
>  > +	{ PCI_VDEVICE(VIA, PCI_DEVICE_ID_VIA_8233A), 17 },
>  > +	{ PCI_VDEVICE(VIA, PCI_DEVICE_ID_VIA_8233C_0), 17 },
>  > +	{ PCI_VDEVICE(VIA, PCI_DEVICE_ID_VIA_8235), 16 },
>  > +	/* May not be needed for the 8237 */
>  > +	{ PCI_VDEVICE(VIA, PCI_DEVICE_ID_VIA_8237), 15 },
>  > +	{ PCI_VDEVICE(VIA, PCI_DEVICE_ID_VIA_8237A), 15 },
>  >  	{ 0, },
> 
> This got me wondering what PCI_VDEVICE was, so I went looking.
> It's a libata'ism it seems with the comment..
> 
> /* move to PCI layer? */

I have no objection to moving this to pci.h.  Feel free to send me a
patch.

thanks,

greg k-h

Re: VIA IRQ quirk missing PCI ids since 2.6.16.17

[Jeff Garzik]

Dave Jones wrote:
> On Mon, Nov 06, 2006 at 09:13:45PM +0000, Alan Cox wrote:
> 
>  > +static const struct pci_device_id via_vlink_fixup_tbl[] = {
>  > +	{ PCI_VDEVICE(VIA, PCI_DEVICE_ID_VIA_8233_0), 17},
>  > +	{ PCI_VDEVICE(VIA, PCI_DEVICE_ID_VIA_8233A), 17 },
>  > +	{ PCI_VDEVICE(VIA, PCI_DEVICE_ID_VIA_8233C_0), 17 },
>  > +	{ PCI_VDEVICE(VIA, PCI_DEVICE_ID_VIA_8235), 16 },
>  > +	/* May not be needed for the 8237 */
>  > +	{ PCI_VDEVICE(VIA, PCI_DEVICE_ID_VIA_8237), 15 },
>  > +	{ PCI_VDEVICE(VIA, PCI_DEVICE_ID_VIA_8237A), 15 },
>  >  	{ 0, },
> 
> This got me wondering what PCI_VDEVICE was, so I went looking.
> It's a libata'ism it seems with the comment..
> 
> /* move to PCI layer? */
> 
> Which sounds like a good idea to me.  But until this is moved,
> does quirks.c actually compile with this patch? I don't see
> an include of linux/libata.h there.
> 
> When it gets moved to the PCI layer, I wonder if it'd be worth
> doing the same thing to the second argument, so that we'd be
> able to do..
> 
> 	{ PCI_VDEVICE(VIA, VIA_8233_0), 17},
> 
> Or maybe even..
> 
> 	{ PCI_VDEVICE(VIA, 8233_0), 17},

Won't work, libata passes hex constants as the second argument...  which 
is the policy I'm encouraging for all places where the PCI_DEVICE_ID_xxx 
is only used in a single place.

	Jeff

Re: VIA IRQ quirk missing PCI ids since 2.6.16.17

[Sergey Vlasov]

[-- Attachment #1: Type: text/plain, Size: 6350 bytes --]

On Mon, 06 Nov 2006 21:13:45 +0000 Alan Cox wrote:

> Ar Llu, 2006-11-06 am 12:47 +0000, ysgrifennodd Sergio Monteiro Basto:
> > this is the latest patch
> > ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.18/2.6.18-mm1/broken-out/via-irq-quirk-behaviour-change.patch
> > about this issue please try it and report the experience :)
>
> There are several minor problems with this patch:
>
> - V-Link which is what we are dealing with is only found in the VIA 8233
> and later devices and early stuff should not be touched
> - It whacks external devices
>
> I think something like this (untested) would be better
>
> diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.19-rc4-mm1/drivers/pci/quirks.c linux-2.6.19-rc4-mm1/drivers/pci/quirks.c
> --- linux.vanilla-2.6.19-rc4-mm1/drivers/pci/quirks.c	2006-10-31 21:11:49.000000000 +0000
> +++ linux-2.6.19-rc4-mm1/drivers/pci/quirks.c	2006-11-06 20:39:03.026705152 +0000
> @@ -641,48 +641,42 @@
>  DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_VIA,	PCI_DEVICE_ID_VIA_82C586_3,	quirk_via_acpi );
>  DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_VIA,	PCI_DEVICE_ID_VIA_82C686_4,	quirk_via_acpi );
>
> -/*
> - * Via 686A/B:  The PCI_INTERRUPT_LINE register for the on-chip
> - * devices, USB0/1, AC97, MC97, and ACPI, has an unusual feature:
> - * when written, it makes an internal connection to the PIC.
> - * For these devices, this register is defined to be 4 bits wide.
> - * Normally this is fine.  However for IO-APIC motherboards, or
> - * non-x86 architectures (yes Via exists on PPC among other places),
> - * we must mask the PCI_INTERRUPT_LINE value versus 0xf to get
> - * interrupts delivered properly.
> - *
> - * Some of the on-chip devices are actually '586 devices' so they are
> - * listed here.
> - */
> -
> -static int via_irq_fixup_needed = -1;
>
>  /*
> - * As some VIA hardware is available in PCI-card form, we need to restrict
> - * this quirk to VIA PCI hardware built onto VIA-based motherboards only.
> - * We try to locate a VIA southbridge before deciding whether the quirk
> - * should be applied.
> - */
> -static const struct pci_device_id via_irq_fixup_tbl[] = {
> -	{
> -		.vendor 	= PCI_VENDOR_ID_VIA,
> -		.device		= PCI_ANY_ID,
> -		.subvendor	= PCI_ANY_ID,
> -		.subdevice	= PCI_ANY_ID,
> -		.class		= PCI_CLASS_BRIDGE_ISA << 8,
> -		.class_mask	= 0xffff00,
> -	},
> + *	VIA bridges which have VLink
> + */
> +
> +static const struct pci_device_id via_vlink_fixup_tbl[] = {
> +	{ PCI_VDEVICE(VIA, PCI_DEVICE_ID_VIA_8233_0), 17},
> +	{ PCI_VDEVICE(VIA, PCI_DEVICE_ID_VIA_8233A), 17 },
> +	{ PCI_VDEVICE(VIA, PCI_DEVICE_ID_VIA_8233C_0), 17 },
> +	{ PCI_VDEVICE(VIA, PCI_DEVICE_ID_VIA_8235), 16 },

Hmm, the old comment mentions 686A/B explicitly - seems that these old
chips also use PCI_INTERRUPT_LINE to control interrupt routing.  Is it
correct to ignore them here?  Yes, that chips used PCI and not VLink,
but they also had an internal PIC (and even internal IO-APIC).

Unfortunately, I no longer have such hardware available.

> +	/* May not be needed for the 8237 */
> +	{ PCI_VDEVICE(VIA, PCI_DEVICE_ID_VIA_8237), 15 },
> +	{ PCI_VDEVICE(VIA, PCI_DEVICE_ID_VIA_8237A), 15 },

8237 definitely uses PCI_INTERRUPT_LINE to control interrupt routing in
PIC mode - tested with the audio part by writing bogus values with
setpci and checking whether interrupts are delivered.

>  	{ 0, },
>  };
>
> -static void quirk_via_irq(struct pci_dev *dev)
> +/**
> + *	quirk_via_vlink		-	VIA VLink IRQ number update
> + *	@dev: PCI device
> + *
> + *	If the device we are dealing with is on a PIC IRQ we need to
> + *	ensure that the IRQ line register which usually is not relevant
> + *	for PCI cards, is actually written so that interrupts get sent
> + *	to the right place
> + */
> +
> +static void quirk_via_vlink(struct pci_dev *dev)
>  {
> +	static struct pci_device_id *via_vlink_fixup = NULL;
>  	u8 irq, new_irq;
>
> -	if (via_irq_fixup_needed == -1)
> -		via_irq_fixup_needed = pci_dev_present(via_irq_fixup_tbl);
> -
> -	if (!via_irq_fixup_needed)
> +	/* Check if we have VLink and cache the result */
> +
> +	if (via_vlink_fixup == NULL)
> +		via_vlink_fixup = pci_find_present(via_irq_fixup_tbl);
> +	if (via_vlink_fixup == NULL)
>  		return;

If there is no VIA ISA bridge in the system, this won't cache anything.

>
>  	new_irq = dev->irq;
> @@ -691,9 +685,17 @@
>  	if (!new_irq || new_irq > 15)
>  		return;
>
> +	/* Internal device ? */
> +	if (pdev->bus->number != 0 || PCI_FUNC(dev->devfn) > 18 ||
> +		PCI_FUNC(dev->devfn) < via_vlink_fixup->driver_data)
> +		return;
> +
> +	/* This is an internal VLink device on a PIC interrupt. The BIOS
> +	   ought to have set this but may not have, so we redo it */
> +
>  	pci_read_config_byte(dev, PCI_INTERRUPT_LINE, &irq);
>  	if (new_irq != irq) {
> -		printk(KERN_INFO "PCI: VIA IRQ fixup for %s, from %d to %d\n",
> +		printk(KERN_INFO "PCI: VIA VLink IRQ fixup for %s, from %d to %d\n",
>  			pci_name(dev), irq, new_irq);
>  		udelay(15);	/* unknown if delay really needed */
>  		pci_write_config_byte(dev, PCI_INTERRUPT_LINE, new_irq);
> diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.19-rc4-mm1/drivers/pci/search.c linux-2.6.19-rc4-mm1/drivers/pci/search.c
> --- linux.vanilla-2.6.19-rc4-mm1/drivers/pci/search.c	2006-10-31 21:11:31.000000000 +0000
> +++ linux-2.6.19-rc4-mm1/drivers/pci/search.c	2006-11-06 20:30:55.548813056 +0000
> @@ -413,30 +413,17 @@
>  	return dev;
>  }
>
> -/**
> - * pci_dev_present - Returns 1 if device matching the device list is present, 0 if not.
> - * @ids: A pointer to a null terminated list of struct pci_device_id structures
> - * that describe the type of PCI device the caller is trying to find.
> - *
> - * Obvious fact: You do not have a reference to any device that might be found
> - * by this function, so if that device is removed from the system right after
> - * this function is finished, the value will be stale.  Use this function to
> - * find devices that are usually built into a system, or for a general hint as
> - * to if another device happens to be present at this specific moment in time.
> - */
> -int pci_dev_present(const struct pci_device_id *ids)
> +struct pci_device_id *pci_find_present(const struct pci_device_id *ids)

New API without proper refcounting?  Ewww.

[...]

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: VIA IRQ quirk missing PCI ids since 2.6.16.17

[Alan Cox]

Ar Mer, 2006-11-08 am 20:22 +0300, ysgrifennodd Sergey Vlasov:
> Hmm, the old comment mentions 686A/B explicitly - seems that these old
> chips also use PCI_INTERRUPT_LINE to control interrupt routing.  Is it
> correct to ignore them here?  Yes, that chips used PCI and not VLink,
> but they also had an internal PIC (and even internal IO-APIC).
> 
> Unfortunately, I no longer have such hardware available.

I have enough docs to extend this approach to those chips if neccessary.
Anyone got an old 686 board to check.

> > +	/* May not be needed for the 8237 */
> > +	{ PCI_VDEVICE(VIA, PCI_DEVICE_ID_VIA_8237), 15 },
> > +	{ PCI_VDEVICE(VIA, PCI_DEVICE_ID_VIA_8237A), 15 },
> 
> 8237 definitely uses PCI_INTERRUPT_LINE to control interrupt routing in
> PIC mode - tested with the audio part by writing bogus values with
> setpci and checking whether interrupts are delivered.

Ok

> If there is no VIA ISA bridge in the system, this won't cache anything.

I no, thats noted in the comments when I posted the diff. If it works
I'll cache ->driver_data instead.

> > -int pci_dev_present(const struct pci_device_id *ids)
> > +struct pci_device_id *pci_find_present(const struct pci_device_id *ids)
> 
> New API without proper refcounting?  Ewww.

pci_device_id objects are not refcounted and don't vanish underneath us.
Devices may but we aren't dealing in devices. The function operates
under the list lock internally so should be safe.

Alan

Re: VIA IRQ quirk missing PCI ids since 2.6.16.17

[Sergey Vlasov]

[-- Attachment #1: Type: text/plain, Size: 486 bytes --]

On Wed, Nov 08, 2006 at 06:05:30PM +0000, Alan Cox wrote:
> > > +struct pci_device_id *pci_find_present(const struct pci_device_id *ids)
> > 
> > New API without proper refcounting?  Ewww.
> 
> pci_device_id objects are not refcounted and don't vanish underneath us.
> Devices may but we aren't dealing in devices. The function operates
> under the list lock internally so should be safe.

Oops, sorry, did not read the patch carefully enough...  Please ignore
this comment.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: VIA IRQ quirk missing PCI ids since 2.6.16.17

[Alan Cox]

Ar Llu, 2006-11-06 am 12:38 +0100, ysgrifennodd Wilco Beekhuizen:
> Including PCI_ANY_ID again fixes these problems but is of course a
> pretty evil fix. The problem is I can't find out which PCI ids to
> include. I'm new to this list so suggestions are welcome.

As far as I can make out the correct answer is "none", and then fix the
actual problems in the boxes that need these workarounds. They break as
much if not more than they cure.

Please boot your system without the workarounds (ie a current kernel).
Then do lspci -vxxx and save that to a file. Then go back to a kernel
with working networking and email me and/or the list that result.

>From that I can take a look at what we've actually got set up for IRQ
routing and try to work out why it might not be working and how we got
where we did.

Alan