LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Jean Delvare <jdelvare@suse.de>
To: Sergey Vlasov <vsu@altlinux.ru>
Cc: Nick Piggin <nickpiggin@yahoo.com.au>,
	Alan Cox <alan@lxorguk.ukuu.org.uk>,
	Daniel Drake <dsd@reactivated.net>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	Adrian Bunk <bunk@stusta.de>
Subject: [PATCH] VIA IRQ quirk breakage fix
Date: Tue, 30 Jan 2007 13:25:58 +0100	[thread overview]
Message-ID: <200701301325.58843.jdelvare@suse.de> (raw)
In-Reply-To: <20070130144701.e793c222.vsu@altlinux.ru>

Hi Sergey,

Le Mardi 30 Janvier 2007 12:47, Sergey Vlasov a écrit :
> On Tue, 30 Jan 2007 08:54:05 +0100 Jean Delvare wrote:
> > You have an old VT82C686 south bridge, which was tagged as "special" by
> > Alan. For this chip, Alan's code only allows devices 00:00.x to be
> > quirked. As my code was merely a reimplementation of Alan's idea, it does
> > the same. Your USB controllers are at 00:07.x, so I'm not surprised they
> > weren't quirked.
> >
> > Alan, why were dev_lo and dev_hi both set to 0 for the VT82C868 in your
> > implementation? Is it a typo, or...?
> >
> > Nick, I guess the following patch would work better for you? I've listed
> > devices 00:07.x as quirkable for the VT82C686.
>
> The VT82C686 is very different from other devices, because it is
> a PCI chip, and its device number is determined by IDSEL wiring.
> Google shows that several different assignments are in use:
>
> 00:01.0:
> http://ozlabs.org/pipermail/linuxppc-embedded/2005-January/016642.html
> (but that's PPC)
>
> 00:04.0: http://www.cs.helsinki.fi/linux/linux-kernel/2001-36/0083.html
> 00:05.0: http://lkml.org/lkml/2003/12/11/78
> 00:07.0: this example and lots of other links
> 00:14.0: http://forum.freespire.org/showthread.php?t=2998

Ah, OK. Thanks for the exhaustive search and explanation. This explains
why Alan and Nick disagreed on the device number.

> > +static void quirk_via_bridge(struct pci_dev *dev)
> > +{
> > +	/* See what bridge we have and find the device ranges */
> > +	switch (dev->device) {
> > +	case PCI_DEVICE_ID_VIA_82C686:
> > +		/* 82C686 is special */
> > +		via_vlink_dev_lo = 7;
> > +		via_vlink_dev_hi = 7;
>
> So this should probably be:
>
> 		/*
> 		 * 82C686 attaches to PCI and can have any device number, but
> 		 * all its subdevices are functions of that single device.
> 		 */
> 		via_vlink_dev_lo = via_vlink_dev_hi = PCI_SLOT(dev->devfn);

Yes, this sounds like the right thing to do. So here comes the third
(and hopefully last) iteration of the patch:


Fix VIA quirks that were recently broken by Alan Cox in the upstream
kernel (commit 1597cacbe39802d86656d1f2e6329895bd2ef531).

My understanding is that pci_find_present() doesn't work yet at the
time the quirks are run. So I used a two-step quirk as is done for
some other quirks already. First we detect the VIA south bridges
and set the right low and high device limits, then we are ready to
actually run the quirks on the affected devices.

Signed-off-by: Jean Delvare <jdelvare@suse.de>
Acked-by: Alan Cox <alan@redhat.com>
---
 drivers/pci/quirks.c |   80 +++++++++++++++++++++++++++-----------------------
 1 files changed, 44 insertions(+), 36 deletions(-)

--- linux-2.6.20-rc6.orig/drivers/pci/quirks.c	2007-01-30 13:02:10.000000000 +0100
+++ linux-2.6.20-rc6/drivers/pci/quirks.c	2007-01-30 13:11:15.000000000 +0100
@@ -654,19 +654,42 @@
  *	VIA bridges which have VLink
  */
 
-static const struct pci_device_id via_vlink_fixup_tbl[] = {
-	/* Internal devices need IRQ line routing, pre VLink */
-	{ PCI_VDEVICE(VIA, PCI_DEVICE_ID_VIA_82C686), 0 },
-	{ PCI_VDEVICE(VIA, PCI_DEVICE_ID_VIA_8231), 17 },
-	/* Devices with VLink */
-	{ 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 },
-	{ PCI_VDEVICE(VIA, PCI_DEVICE_ID_VIA_8237), 15 },
-	{ PCI_VDEVICE(VIA, PCI_DEVICE_ID_VIA_8237A), 15 },
-	{ 0, },
-};
+static int via_vlink_dev_lo = -1, via_vlink_dev_hi = 18;
+
+static void quirk_via_bridge(struct pci_dev *dev)
+{
+	/* See what bridge we have and find the device ranges */
+	switch (dev->device) {
+	case PCI_DEVICE_ID_VIA_82C686:
+		/* The VT82C686 is special, it attaches to PCI and can have
+		   any device number. All its subdevices are functions of
+		   that single device. */
+		via_vlink_dev_lo = PCI_SLOT(dev->devfn);
+		via_vlink_dev_hi = PCI_SLOT(dev->devfn);
+		break;
+	case PCI_DEVICE_ID_VIA_8237:
+	case PCI_DEVICE_ID_VIA_8237A:
+		via_vlink_dev_lo = 15;
+		break;
+	case PCI_DEVICE_ID_VIA_8235:
+		via_vlink_dev_lo = 16;
+		break;
+	case PCI_DEVICE_ID_VIA_8231:
+	case PCI_DEVICE_ID_VIA_8233_0:
+	case PCI_DEVICE_ID_VIA_8233A:
+	case PCI_DEVICE_ID_VIA_8233C_0:
+		via_vlink_dev_lo = 17;
+		break;
+	}
+}
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_VIA,	PCI_DEVICE_ID_VIA_82C686,	quirk_via_bridge);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_VIA,	PCI_DEVICE_ID_VIA_8231,		quirk_via_bridge);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_VIA,	PCI_DEVICE_ID_VIA_8233_0,	quirk_via_bridge);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_VIA,	PCI_DEVICE_ID_VIA_8233A,	quirk_via_bridge);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_VIA,	PCI_DEVICE_ID_VIA_8233C_0,	quirk_via_bridge);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_VIA,	PCI_DEVICE_ID_VIA_8235,		quirk_via_bridge);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_VIA,	PCI_DEVICE_ID_VIA_8237,		quirk_via_bridge);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_VIA,	PCI_DEVICE_ID_VIA_8237A,	quirk_via_bridge);
 
 /**
  *	quirk_via_vlink		-	VIA VLink IRQ number update
@@ -675,35 +698,20 @@
  *	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
+ *	to the right place.
+ *	We only do this on systems where a VIA south bridge was detected,
+ *	and only for VIA devices on the motherboard (see quirk_via_bridge
+ *	above).
  */
 
 static void quirk_via_vlink(struct pci_dev *dev)
 {
-	const struct pci_device_id *via_vlink_fixup;
-	static int dev_lo = -1, dev_hi = 18;
 	u8 irq, new_irq;
 
-	/* Check if we have VLink and cache the result */
-
-	/* Checked already - no */
-	if (dev_lo == -2)
+	/* Check if we have VLink at all */
+	if (via_vlink_dev_lo == -1)
 		return;
 
-	/* Not checked - see what bridge we have and find the device
-	   ranges */
-
-	if (dev_lo == -1) {
-		via_vlink_fixup = pci_find_present(via_vlink_fixup_tbl);
-		if (via_vlink_fixup == NULL) {
-			dev_lo = -2;
-			return;
-		}
-		dev_lo = via_vlink_fixup->driver_data;
-		/* 82C686 is special - 0/0 */
-		if (dev_lo == 0)
-			dev_hi = 0;
-	}
 	new_irq = dev->irq;
 
 	/* Don't quirk interrupts outside the legacy IRQ range */
@@ -711,8 +719,8 @@
 		return;
 
 	/* Internal device ? */
-	if (dev->bus->number != 0 || PCI_SLOT(dev->devfn) > dev_hi ||
-		PCI_SLOT(dev->devfn) < dev_lo)
+	if (dev->bus->number != 0 || PCI_SLOT(dev->devfn) > via_vlink_dev_hi ||
+	    PCI_SLOT(dev->devfn) < via_vlink_dev_lo)
 		return;
 
 	/* This is an internal VLink device on a PIC interrupt. The BIOS


-- 
Jean Delvare
Suse L3

  reply	other threads:[~2007-01-30 12:26 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-01-24  0:33 via irq quirk breakage Nick Piggin
2007-01-29 15:00 ` Jean Delvare
2007-01-29 15:51   ` Alan
2007-01-30 12:29     ` Jean Delvare
2007-01-30 16:05       ` Alan
2007-01-30  2:35   ` Nick Piggin
2007-01-30  7:54     ` Jean Delvare
2007-01-30  8:32       ` Nick Piggin
2007-01-30 11:47       ` Sergey Vlasov
2007-01-30 12:25         ` Jean Delvare [this message]
2007-01-30 13:38           ` [PATCH] VIA IRQ quirk breakage fix Nick Piggin
2007-01-30 13:49             ` Jean Delvare
2007-01-30 16:21               ` Alan
2007-01-30 16:17                 ` Jean Delvare
2007-01-30 18:02               ` Andrew Morton
2007-01-31  2:37           ` Andrew Morton
2007-02-01  7:39             ` Jean Delvare
2007-01-30 14:56       ` via irq quirk breakage Alan
2007-01-30 14:50         ` Jeff Garzik
2007-01-30 14:25     ` Alan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=200701301325.58843.jdelvare@suse.de \
    --to=jdelvare@suse.de \
    --cc=alan@lxorguk.ukuu.org.uk \
    --cc=bunk@stusta.de \
    --cc=dsd@reactivated.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nickpiggin@yahoo.com.au \
    --cc=vsu@altlinux.ru \
    --subject='Re: [PATCH] VIA IRQ quirk breakage fix' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).