The most recent VIA IRQ quirk changes have broken various VIA devices for
some users. We are not able to add these devices to the blacklist as they
are also available in PCI-card form, and running the quirk on these devices
brings us back to square one (running the VIA quirk on non-VIA boards where
the quirk is not needed).
This patch, based on suggestions from Sergey Vlasov, implements a scheme
similar to but more restrictive than the scheme we had in 2.6.16 and
earlier. It runs the quirk on all VIA hardware, but *only* if a VIA
southbridge was detected on the system.
Based on suggestions and much investigation from from Sergio Monteiro Basto,
this patch also partially reverts commit
93cffffa19960464a52f9c78d9a6150270d23785 from Bjorn Helgaas. It is not clear
if this was ever the correct fix, see http://lkml.org/lkml/2005/9/26/183
This patch additionally includes a change suggested by Linus in that thread,
where we only quirk devices on non-legacy IRQs.
There is still a downside to this patch: if the user inserts a VIA PCI card
into a VIA-based motherboard, in some circumstances the quirk will also run on
the VIA PCI card. This corner case is hard to avoid.
Signed-off-by: Daniel Drake <[email protected]>
---
Andrew, please replace the current -mm patch with this one. The general idea
is that we consider applying the quirk to a lot more devices than we currently
do (more comparable to 2.6.16 and previous), but we add various bail-out
conditions to actually end up applying the quirks much less.
I am fairly confident that we'll still be quirking enough hardware to not
cause any breakage, but we can't be sure until it has seen some testing. This
is compile-tested only.
Stian Jordet: You're on CC due to a discussion linked to from above where
it appeared that you needed Bjorn's patch. Please test this patch against
unmodified 2.6.17 or 2.6.18-rc and let us know if there are any problems.
Index: linux/drivers/pci/quirks.c
===================================================================
--- linux.orig/drivers/pci/quirks.c
+++ linux/drivers/pci/quirks.c
@@ -650,26 +650,59 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_V
* 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,
+ },
+ { 0, },
+};
+
static void quirk_via_irq(struct pci_dev *dev)
{
u8 irq, new_irq;
- new_irq = dev->irq & 0xf;
+#ifdef CONFIG_X86_IO_APIC
+ if (nr_ioapics && !skip_ioapic_setup)
+ return;
+#endif
+#ifdef CONFIG_ACPI
+ if (acpi_irq_model != ACPI_IRQ_MODEL_PIC)
+ return;
+#endif
+
+ if (via_irq_fixup_needed == -1)
+ via_irq_fixup_needed = pci_dev_present(via_irq_fixup_tbl);
+
+ if (!via_irq_fixup_needed)
+ return;
+
+ new_irq = dev->irq;
+ if (!new_irq || new_irq >= 15)
+ return;
+
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 PIC 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_DEVICE_ID_VIA_82C586_0, quirk_via_irq);
-DECLARE_PCI_FIXUP_ENABLE(PCI_VENDOR_ID_VIA, PCI_DEVICE_ID_VIA_82C586_1, quirk_via_irq);
-DECLARE_PCI_FIXUP_ENABLE(PCI_VENDOR_ID_VIA, PCI_DEVICE_ID_VIA_82C586_2, quirk_via_irq);
-DECLARE_PCI_FIXUP_ENABLE(PCI_VENDOR_ID_VIA, PCI_DEVICE_ID_VIA_82C586_3, quirk_via_irq);
-DECLARE_PCI_FIXUP_ENABLE(PCI_VENDOR_ID_VIA, PCI_DEVICE_ID_VIA_82C686, quirk_via_irq);
-DECLARE_PCI_FIXUP_ENABLE(PCI_VENDOR_ID_VIA, PCI_DEVICE_ID_VIA_82C686_4, quirk_via_irq);
-DECLARE_PCI_FIXUP_ENABLE(PCI_VENDOR_ID_VIA, PCI_DEVICE_ID_VIA_82C686_5, quirk_via_irq);
+DECLARE_PCI_FIXUP_ENABLE(PCI_VENDOR_ID_VIA, PCI_ANY_ID, quirk_via_irq);
/*
* VIA VT82C598 has its device ID settable and many BIOSes
Daniel Drake wrote:
> Stian Jordet: You're on CC due to a discussion linked to from above where
> it appeared that you needed Bjorn's patch. Please test this patch against
> unmodified 2.6.17 or 2.6.18-rc and let us know if there are any problems.
>
No more usb for me with this patch :P
When usb is loaded I get this in dmesg:
irq 9: nobody cared (try booting with the "irqpoll" option)
[<c01302d3>] __report_bad_irq+0x2b/0x69
[<c01304bd>] note_interrupt+0x1ac/0x1e3
[<c0246175>] acpi_irq+0xb/0x14
[<c012fadb>] handle_IRQ_event+0x23/0x49
[<c012fbb3>] __do_IRQ+0xb2/0xe6
[<c0104b3d>] do_IRQ+0x43/0x52
[<c01031ea>] common_interrupt+0x1a/0x20
[<c0101620>] default_idle+0x0/0x59
[<c0101651>] default_idle+0x31/0x59
[<c01016d7>] cpu_idle+0x5e/0x74
[<c053d6d7>] start_kernel+0x353/0x35a
handlers:
[<c024616a>] (acpi_irq+0x0/0x14)
Disabling IRQ #9
and while USB has the same irq in /proc/interrupts now as earlier, usb
doesn't work (or sometimes it does, just dog slow!). Acpi is of course
disabled, so no acpi events neither.
I have to admit, that this computer have some weird behaviour - see
http://bugzilla.kernel.org/show_bug.cgi?id=2874 - but still, it's been
working well for many years now, before this patch. While I do
understand that I seem to be the only one affected by this bug, I hope
someone will help me look into another way to solve my problems if this
patch get applied.
Thanks.
-Stian
Stian Jordet wrote:
> Daniel Drake wrote:
>> Stian Jordet: You're on CC due to a discussion linked to from above where
>> it appeared that you needed Bjorn's patch. Please test this patch against
>> unmodified 2.6.17 or 2.6.18-rc and let us know if there are any problems.
>>
> No more usb for me with this patch :P
Please send dmesg from both a working kernel and a patched kernel, and
/proc/interrupts from both too.
Thanks,
Daniel
On man, 2006-09-04 at 22:39 -0400, Daniel Drake wrote:
> Stian Jordet wrote:
> > Daniel Drake wrote:
> >> Stian Jordet: You're on CC due to a discussion linked to from above where
> >> it appeared that you needed Bjorn's patch. Please test this patch against
> >> unmodified 2.6.17 or 2.6.18-rc and let us know if there are any problems.
> >>
> > No more usb for me with this patch :P
>
> Please send dmesg from both a working kernel and a patched kernel, and
> /proc/interrupts from both too.
Here dmesg and /proc/interrupts with and without the patch. Both are
2.6.18-rc6...
Thanks :)
-Stian
Stian Jordet wrote:
> On man, 2006-09-04 at 22:39 -0400, Daniel Drake wrote:
>> Stian Jordet wrote:
>>> Daniel Drake wrote:
>>>> Stian Jordet: You're on CC due to a discussion linked to from above where
>>>> it appeared that you needed Bjorn's patch. Please test this patch against
>>>> unmodified 2.6.17 or 2.6.18-rc and let us know if there are any problems.
>>>>
>>> No more usb for me with this patch :P
>> Please send dmesg from both a working kernel and a patched kernel, and
>> /proc/interrupts from both too.
>
> Here dmesg and /proc/interrupts with and without the patch. Both are
> 2.6.18-rc6...
Sergio,
Stian appears to be walking proof that quirks are sometimes required in
IO-APIC mode.
My next move would be to modify the patch to not revert Bjorn's changes
(but leave Linus' modification in place, alongside the southbridge
detection). Any thoughts?
Daniel
On Wed, 2006-09-06 at 19:49 -0400, Daniel Drake wrote:
> Sergio,
>
> Stian appears to be walking proof that quirks are sometimes required
> in
> IO-APIC mode.
>
> My next move would be to modify the patch to not revert Bjorn's
> changes
> (but leave Linus' modification in place, alongside the southbridge
> detection). Any thoughts?
Hi Daniel, since you ask for thoughts :)
yap, is the obvious conclusion, but no, my bet is one problem with USB
and USB guys could put the USB things working.
I just had remember, my Asrock with VIA8237 and VIA SATA (where I am
write now) is working without quirks and USB guys made a patch, by
coincidence. Since then have been working great.
http://bugzilla.kernel.org/show_bug.cgi?id=6419#c19
About Linus patch I have to correct me about what I had write,
http://lkml.org/lkml/2005/9/27/113
?(it used to say "if we have an IO-APIC, don't do this" (my patch), now
it says "if this irq is bound to an IO-APIC, don't do this")?
Or my patch or the Linus patch, not both.
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -546,7 +546,10 @@ static void quirk_via_irq(struct pci_dev
{
u8 irq, new_irq;
- new_irq = dev->irq & 0xf;
+ new_irq = dev->irq;
+ if (!new_irq || new_irq >= 15)
+ return;
+
pci_read_config_byte(dev, PCI_INTERRUPT_LINE, &irq);
if (new_irq != irq) {
but I look to this Linus patch and I see 2 bugs
one should be > not >= and new_irq after tests new_irq should be dev->irq & 0xf;
like this:
- new_irq = dev->irq & 0xf;
+ new_irq = dev->irq;
+ if (!new_irq || new_irq > 15)
+ return;
+ new_irq = dev->irq & 0xf;
pci_read_config_byte(dev, PCI_INTERRUPT_LINE, &irq);
if (new_irq != irq) {
or simply :
+ if (!dev->irq || dev->irq > 15)
+ return;
new_irq = dev->irq & 0xf;
About Stian computer, looking for /proc/interrupts
11: 30696 27559 IO-APIC-level uhci_hcd:usb1, uhci_hcd:usb2, uhci_hcd:usb3
have USB on irq 11, with IO-APIC-level, which less acpi is not normal on
low numbers ( <=15 ) be IO-APIC-level, normally is IO-APIC-edge.
Could be a ACPI problem .
Thanks,
--
S?rgio M. B.
Sergio Monteiro Basto wrote:
> yap, is the obvious conclusion, but no, my bet is one problem with USB
> and USB guys could put the USB things working.
I'm not convinced, I'm pretty sure they'd say something like "this
appears to be an IRQ routing problem" and send it somewhere else. I saw
a bug report (introduced by the recent mainline via_irq_quirk() changes)
where exactly this happened, but I can't find it.
> I just had remember, my Asrock with VIA8237 and VIA SATA (where I am
> write now) is working without quirks and USB guys made a patch, by
> coincidence. Since then have been working great.
> http://bugzilla.kernel.org/show_bug.cgi?id=6419#c19
Where's the patch?
This report seems to be inconclusive. Your USB problem (comment #19) was
clearly something to do with UHCI itself, whereas Stian's problem is
much more generic and outside the control of the USB HCD: nobody cared
Plus the only issue related to IRQ routing on that bug is triggered by
the closed nvidia driver...
> About Linus patch I have to correct me about what I had write,
> http://lkml.org/lkml/2005/9/27/113
> ?(it used to say "if we have an IO-APIC, don't do this" (my patch), now
> it says "if this irq is bound to an IO-APIC, don't do this")?
> Or my patch or the Linus patch, not both.
Sorry, I can't figure out what you are trying to say here. Can you
rephrase it?
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -546,7 +546,10 @@ static void quirk_via_irq(struct pci_dev
> {
> u8 irq, new_irq;
>
> - new_irq = dev->irq & 0xf;
> + new_irq = dev->irq;
> + if (!new_irq || new_irq >= 15)
> + return;
> +
> pci_read_config_byte(dev, PCI_INTERRUPT_LINE, &irq);
> if (new_irq != irq) {
>
> but I look to this Linus patch and I see 2 bugs
> one should be > not >=
I think you might be right here. Firstly IRQ 15 is a legacy IRQ,
secondly the existing "&15" thing has no effect on IRQ 15 obviously.
> and new_irq after tests new_irq should be dev->irq & 0xf;
> like this:
> - new_irq = dev->irq & 0xf;
> + new_irq = dev->irq;
> + if (!new_irq || new_irq > 15)
> + return;
> + new_irq = dev->irq & 0xf;
> pci_read_config_byte(dev, PCI_INTERRUPT_LINE, &irq);
> if (new_irq != irq) {
No, there is no bug, think about the logic:
We bail out if dev->irq is higher than 15. Therefore when we get to the
lines of code in question, dev->irq is 15 or less. Performing a logical
AND operation with the value 15 (0xf) is going to have no effect at all.
> About Stian computer, looking for /proc/interrupts
>
> 11: 30696 27559 IO-APIC-level uhci_hcd:usb1, uhci_hcd:usb2, uhci_hcd:usb3
>
> have USB on irq 11, with IO-APIC-level, which less acpi is not normal on
> low numbers ( <=15 ) be IO-APIC-level, normally is IO-APIC-edge.
> Could be a ACPI problem .
I beg to differ. Usually APIC interrupts are level triggered. In fact
(just as an example!) most NAPI-based network drivers will not work with
edge-triggered interrupts. Additionally, if my understanding is correct,
multiple devices sharing an edge triggered interrupt is bad news
(interrupts likely to get lost so devices do not get serviced).
Daniel
Hi,
Sorry for the empty email before
On Wed, 2006-09-06 at 23:47 -0400, Daniel Drake wrote:
> > http://bugzilla.kernel.org/show_bug.cgi?id=6419#c19
>
> Where's the patch?
on kernel 2.6.18-rc4 and
« Here is a patch which should fix the "host controller process error"
problem:
http://marc.theaimsgroup.com/?l=linux-usb-devel&m=115435540308759&w=2
Alan Stern »
> This report seems to be inconclusive. Your USB problem (comment #19)
> was
> clearly something to do with UHCI itself, whereas Stian's problem is
> much more generic and outside the control of the USB HCD: nobody cared
> Plus the only issue related to IRQ routing on that bug is triggered
> by
> the closed nvidia driver...
>
> > About Linus patch I have to correct me about what I had write,
> > http://lkml.org/lkml/2005/9/27/113
> > «(it used to say "if we have an IO-APIC, don't do this" (my patch),
> now
> > it says "if this irq is bound to an IO-APIC, don't do this")»
> > Or my patch or the Linus patch, not both.
>
> Sorry, I can't figure out what you are trying to say here. Can you
> rephrase it?
the statment was write by Linus on http://lkml.org/lkml/2005/9/27/113
my patch don't quirk any device if we are working on IO-APIC,
Linus simply know if a interrupt is > 15 we are working on IO-APIC and
just don't quirk irq > 15
> > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> > --- a/drivers/pci/quirks.c
> > +++ b/drivers/pci/quirks.c
> > @@ -546,7 +546,10 @@ static void quirk_via_irq(struct pci_dev
> > {
> > u8 irq, new_irq;
> >
> > - new_irq = dev->irq & 0xf;
> > + new_irq = dev->irq;
> > + if (!new_irq || new_irq >= 15)
> > + return;
> > +
> > pci_read_config_byte(dev, PCI_INTERRUPT_LINE, &irq);
> > if (new_irq != irq) {
> >
> > but I look to this Linus patch and I see 2 bugs
> > one should be > not >=
>
> I think you might be right here. Firstly IRQ 15 is a legacy IRQ,
> secondly the existing "&15" thing has no effect on IRQ 15 obviously.
>
> > and new_irq after tests new_irq should be dev->irq & 0xf;
> > like this:
> > - new_irq = dev->irq & 0xf;
> > + new_irq = dev->irq;
> > + if (!new_irq || new_irq > 15)
> > + return;
> > + new_irq = dev->irq & 0xf;
> > pci_read_config_byte(dev, PCI_INTERRUPT_LINE, &irq);
> > if (new_irq != irq) {
>
> No, there is no bug, think about the logic:
>
> We bail out if dev->irq is higher than 15. Therefore when we get to
> the
> lines of code in question, dev->irq is 15 or less. Performing a
> logical
> AND operation with the value 15 (0xf) is going to have no effect at
> all.
>
ok
Thanks,
tor, 07,.09.2006 kl. 12.43 +0100, skrev Sergio Monteiro Basto:
> the statment was write by Linus on http://lkml.org/lkml/2005/9/27/113
> my patch don't quirk any device if we are working on IO-APIC,
> Linus simply know if a interrupt is > 15 we are working on IO-APIC and
> just don't quirk irq > 15
>
> > > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> > > --- a/drivers/pci/quirks.c
> > > +++ b/drivers/pci/quirks.c
> > > @@ -546,7 +546,10 @@ static void quirk_via_irq(struct pci_dev
> > > {
> > > u8 irq, new_irq;
> > >
> > > - new_irq = dev->irq & 0xf;
> > > + new_irq = dev->irq;
> > > + if (!new_irq || new_irq >= 15)
> > > + return;
> > > +
> > > pci_read_config_byte(dev, PCI_INTERRUPT_LINE, &irq);
> > > if (new_irq != irq) {
After a quick test tonight, this patch seems to work nicely for me. (I
know I told Linux that it didn't, don't know what's changed). Does this
fix the problems other people is having as well? If so, that would be
nice :)
Thanks.
-Stian