[PATCH] pci quirks: Sort out the VIA mess once and for all (hopefully)

[PATCH] pci quirks: Sort out the VIA mess once and for all (hopefully)

[Alan Cox]

Perform the VIA fixup only on the devices on the V-Link on the VIA
bridges that have it, and nowhere else.

Doing this we move the PCI_VDEVICE macro to pci.h and document it. We
also add pci_find_present which works like pci_device_present but
returns the matching id and is more useful. pci_device_present becomes a
trivial user of pci_find_present instead.

Boots for me but going by history this wants quite a bit of testing
before anyone can be sure it actually fixes all the cases we have to
deal with. If you have a VIA board (problem or otherwise) try this patch
versus 2.6.19-rc4-mm (and probably rc5-mm once it appears) and let me
know if it sorts out your box.

If you have a system where the onboard devices still fail then please
send me a bug report and include the "dmesg" output, lspci -vvxxx, and
info on any kernels that apparently did work.

[Andrew - please don't merge this yet, I need to fix things like caching
the pci_find_device to work again before its ready for -mm]

Signed-off-by: Alan Cox <alan@redhat.com>

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-07 10:27:14.000000000 +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_vlink_fixup_tbl);
+	if (via_vlink_fixup == NULL)
 		return;
 
 	new_irq = dev->irq;
@@ -691,15 +685,23 @@
 	if (!new_irq || new_irq > 15)
 		return;
 
+	/* Internal device ? */
+	if (dev->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);
 	}
 }
-DECLARE_PCI_FIXUP_ENABLE(PCI_VENDOR_ID_VIA, PCI_ANY_ID, quirk_via_irq);
+DECLARE_PCI_FIXUP_ENABLE(PCI_VENDOR_ID_VIA, PCI_ANY_ID, quirk_via_vlink);
 
 /*
  * VIA VT82C598 has its device ID settable and many BIOSes
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.000000000 +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/libata.h linux-2.6.19-rc4-mm1/include/linux/libata.h
--- linux.vanilla-2.6.19-rc4-mm1/include/linux/libata.h	2006-10-31 21:11:50.000000000 +0000
+++ linux-2.6.19-rc4-mm1/include/linux/libata.h	2006-11-07 10:07:10.000000000 +0000
@@ -109,11 +109,6 @@
 /* defines only for the constants which don't work well as enums */
 #define ATA_TAG_POISON		0xfafbfcfdU
 
-/* move to PCI layer? */
-#define PCI_VDEVICE(vendor, device)		\
-	PCI_VENDOR_ID_##vendor, (device),	\
-	PCI_ANY_ID, PCI_ANY_ID, 0, 0
-
 static inline struct device *pci_dev_to_dev(struct pci_dev *pdev)
 {
 	return &pdev->dev;
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-07 10:07:06.000000000 +0000
@@ -389,6 +390,21 @@
 	.vendor = PCI_ANY_ID, .device = PCI_ANY_ID, \
 	.subvendor = PCI_ANY_ID, .subdevice = PCI_ANY_ID
 
+/**
+ * PCI_VDEVICE - macro used to describe a specific pci device in short form
+ * @vend: the vendor name
+ * @dev: the 16 bit PCI Device ID
+ *
+ * This macro is used to create a struct pci_device_id that matches a
+ * specific PCI device.  The vendor, device, subvendor, and subdevice
+ * fields will be set to PCI_ANY_ID. The macro allows the next field
+ * to follow as the device private data.
+ */
+ 
+#define PCI_VDEVICE(vendor, device)		\
+	PCI_VENDOR_ID_##vendor, (device),	\
+	PCI_ANY_ID, PCI_ANY_ID, 0, 0
+
 /* these external functions are only available when PCI support is enabled */
 #ifdef CONFIG_PCI
 
@@ -461,6 +477,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 +701,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: [PATCH] pci quirks: Sort out the VIA mess once and for all (hopefully)

[Jeff Garzik]

Alan Cox wrote:
> diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.19-rc4-mm1/include/linux/libata.h linux-2.6.19-rc4-mm1/include/linux/libata.h
> --- linux.vanilla-2.6.19-rc4-mm1/include/linux/libata.h	2006-10-31 21:11:50.000000000 +0000
> +++ linux-2.6.19-rc4-mm1/include/linux/libata.h	2006-11-07 10:07:10.000000000 +0000
> @@ -109,11 +109,6 @@
>  /* defines only for the constants which don't work well as enums */
>  #define ATA_TAG_POISON		0xfafbfcfdU
>  
> -/* move to PCI layer? */
> -#define PCI_VDEVICE(vendor, device)		\
> -	PCI_VENDOR_ID_##vendor, (device),	\
> -	PCI_ANY_ID, PCI_ANY_ID, 0, 0
> -
>  static inline struct device *pci_dev_to_dev(struct pci_dev *pdev)
>  {


nit:  don't remove the "move to PCI layer?" comment, it also applies to 
code below that which you are moving.

Patch overall seems sane to me.

	Jeff

Re: [PATCH] pci quirks: Sort out the VIA mess once and for all ?(hopefully)

[Luca Tettamanti]

Alan Cox <alan@lxorguk.ukuu.org.uk> ha scritto:
> 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-07 10:07:06.000000000 +0000
> @@ -389,6 +390,21 @@
>        .vendor = PCI_ANY_ID, .device = PCI_ANY_ID, \
>        .subvendor = PCI_ANY_ID, .subdevice = PCI_ANY_ID
> 
> +/**
> + * PCI_VDEVICE - macro used to describe a specific pci device in short form
> + * @vend: the vendor name
> + * @dev: the 16 bit PCI Device ID
> + *
> + * This macro is used to create a struct pci_device_id that matches a
> + * specific PCI device.  The vendor, device, subvendor, and subdevice
> + * fields will be set to PCI_ANY_ID. The macro allows the next field

Hello Alan,
the comment doesn't match the macro: vendor and device are passed by the
caller, they're not PCI_ANY_ID.

> + * to follow as the device private data.
> + */
> + 
> +#define PCI_VDEVICE(vendor, device)            \
> +       PCI_VENDOR_ID_##vendor, (device),       \
> +       PCI_ANY_ID, PCI_ANY_ID, 0, 0
> +
> /* these external functions are only available when PCI support is enabled */
> #ifdef CONFIG_PCI

Luca
-- 
Recursion n.:
	See Recursion.

Re: [PATCH] pci quirks: Sort out the VIA mess once and for all (hopefully)

[Daniel Drake]

Hi Alan,

Thanks for spending time working on this. Sorry that I never followed up 
after my previous attempt, I've been really busy having starting a 
proper job.

Alan Cox wrote:
> Boots for me but going by history this wants quite a bit of testing
> before anyone can be sure it actually fixes all the cases we have to
> deal with. If you have a VIA board (problem or otherwise) try this patch
> versus 2.6.19-rc4-mm (and probably rc5-mm once it appears) and let me
> know if it sorts out your box.

I just noticed that my earlier patch is included as of 2.6.19-rc, so 
actually your patch applies there and results can be compared. (if 
you're in strong objection to it's mainline inclusion, I could ask for 
it to be reverted, but it at does solve a lot of problems for users over 
the previous state with no reported problems...)

I now have access to a VIA system. It's not really an indicative test, 
as with my patch, no devices get quirked (due to some being outside the 
legacy IRQ range and others failing the new_irq != irq test). However I 
could at least confirm that your patch didn't cause any devices to be 
quirked again.

Thanks,
Daniel

Re: [PATCH] pci quirks: Sort out the VIA mess once and for all (hopefully)

[Alan Cox]

Ar Mer, 2006-11-08 am 15:04 -0500, ysgrifennodd Daniel Drake:
> Hi Alan,
> 
> Thanks for spending time working on this. Sorry that I never followed up 
> after my previous attempt, I've been really busy having starting a 
> proper job.

No problem. As if I don't leave a wake of things other people pick up
when I get too busy 8)

> I just noticed that my earlier patch is included as of 2.6.19-rc, so 
> actually your patch applies there and results can be compared. (if 
> you're in strong objection to it's mainline inclusion, I could ask for 
> it to be reverted, but it at does solve a lot of problems for users over 
> the previous state with no reported problems...)

I think your patch is going to break stuff, without the patch will break
more, and hopefully this patch will not break anything - but there is
risk. I think it's up to Linus what he wants to do for .19

Alan

Re: [PATCH] pci quirks: Sort out the VIA mess once and for all (hopefully)

[Sergio Monteiro Basto]

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

First a salute to subject :) I hope so!

I will try the patch on my old laptop, which need the quirks, soon as
possible but I don't know if can be in this month.

On Wed, 2006-11-08 at 20:39 +0000, Alan Cox wrote:
> I think your patch is going to break stuff, without the patch will
> break more, 

My point of view is: The stuff was already breaked in kernels before
2.6.16. And the patch is a improvement from kernel 2.6.16. Not perfect,
we now.

> and hopefully this patch will not break anything - but there is risk.
> I think it's up to Linus what he wants to do for .19 

yap, it a risk because, drivers team tend to workaround the problems and
when we change IRQ routing, in this case correctly, workaround may blow
it.

Thanks,
-- 
Sérgio M.B.

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 2166 bytes --]

Re: [PATCH] pci quirks: Sort out the VIA mess once and for all (hopefully)

[Sergio Monteiro Basto]

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

Hi, 
I don't have the original message just my reply, 
I had try a FC6-respin dvd.iso ( which have kernel 2.6.20) in my old
laptop (http://sergiomb.no-ip.org/laptop/ ) which need "VIA IRQ quirk
fixup"  and I had many problems with booting after dig a little I found
that quirks aren't applied anymore .
So the latest patch of Alan Cox on this subject which is already on
kernel 2.6.20, hasn't been tested and doesn't work on my laptop and
probably in all compaq pressario 700 , I think.

Thanks,
-- 
Sérgio M.B.

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 2192 bytes --]