An earlier commit (75cf7456dd87335f574dcd53c4ae616a2ad71a11) changed
an overly-zealous PCI quirk to only poke those VIA devices that need
it. However, some PCI devices were not included in what I hope is now
the full list.
This should I hope correct this.
Thanks to Masoud Sharbiani <[email protected]> for pointing this out
and testing the fix.
Signed-of-By: Chris Wedgwood <[email protected]>
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 19e2b17..0d36d50 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -634,6 +634,9 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_V
* 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 void quirk_via_irq(struct pci_dev *dev)
{
@@ -648,6 +651,10 @@ static void quirk_via_irq(struct pci_dev
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);
Chris Wedgwood <[email protected]> wrote:
>
> An earlier commit (75cf7456dd87335f574dcd53c4ae616a2ad71a11) changed
> an overly-zealous PCI quirk to only poke those VIA devices that need
> it. However, some PCI devices were not included in what I hope is now
> the full list.
>
> This should I hope correct this.
>
> Thanks to Masoud Sharbiani <[email protected]> for pointing this out
> and testing the fix.
This looks like a 2.6.17-worthy fix, but it's not clear. Help. What
happens if 2.6.17 doesn't have this??
>
> Signed-of-By: Chris Wedgwood <[email protected]>
Wanna buy an "f"?
On Tue, May 09, 2006 at 12:59:16PM -0700, Andrew Morton wrote:
> Chris Wedgwood <[email protected]> wrote:
> >
> > An earlier commit (75cf7456dd87335f574dcd53c4ae616a2ad71a11) changed
> > an overly-zealous PCI quirk to only poke those VIA devices that need
> > it. However, some PCI devices were not included in what I hope is now
> > the full list.
> >
> > This should I hope correct this.
> >
> > Thanks to Masoud Sharbiani <[email protected]> for pointing this out
> > and testing the fix.
>
> This looks like a 2.6.17-worthy fix, but it's not clear. Help. What
> happens if 2.6.17 doesn't have this??
We won't run the quirk on machines that used to have it run,
so we get buggered up irq routing.
Dave
--
http://www.codemonkey.org.uk
On Tue, May 09, 2006 at 12:59:16PM -0700, Andrew Morton wrote:
> This looks like a 2.6.17-worthy fix, but it's not clear. Help.
> What happens if 2.6.17 doesn't have this??
Some machines with this hardware stop working.
> Wanna buy an "f"?
Sure. How much?
Dave Jones <[email protected]> wrote:
>
> On Tue, May 09, 2006 at 12:59:16PM -0700, Andrew Morton wrote:
> > Chris Wedgwood <[email protected]> wrote:
> > >
> > > An earlier commit (75cf7456dd87335f574dcd53c4ae616a2ad71a11) changed
> > > an overly-zealous PCI quirk to only poke those VIA devices that need
> > > it. However, some PCI devices were not included in what I hope is now
> > > the full list.
> > >
> > > This should I hope correct this.
> > >
> > > Thanks to Masoud Sharbiani <[email protected]> for pointing this out
> > > and testing the fix.
> >
> > This looks like a 2.6.17-worthy fix, but it's not clear. Help. What
> > happens if 2.6.17 doesn't have this??
>
> We won't run the quirk on machines that used to have it run,
> so we get buggered up irq routing.
>
OK..
We used to have
DECLARE_PCI_FIXUP_ENABLE(PCI_VENDOR_ID_VIA, PCI_ANY_ID, quirk_via_irq);
and we now have
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);
which is rather a step backwards, because we need to keep that list updated
now, and we'll fail to catch new devices.
What happens if we just revert 75cf7456dd87335f574dcd53c4ae616a2ad71a11?
It says
Alan Cox pointed out that the VIA 'IRQ fixup' was erroneously running
on my system which has no VIA southbridge (but I do have a VIA IEEE
1394 device).
but so what? Did anything actually go wrong? Is it likely to go wrong in
the future?
Is there was a problem, is there something we can do at runtime in
quirk_via_irq() to avoid the problem, rather than expanding out the fixup
list in this manner?
On Tue, May 09, 2006 at 01:41:01PM -0700, Andrew Morton wrote:
> DECLARE_PCI_FIXUP_ENABLE(PCI_VENDOR_ID_VIA, PCI_ANY_ID,
> quirk_via_irq);
which is wrong, we don't want to apply the qurik for *ALL* via devices
the comment says we want it only for on-ship devices
> 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);
>
> which is rather a step backwards, because we need to keep that list
> updated now, and we'll fail to catch new devices.
as best as i can tell this is a complete list
i tried to get details of people with hardware and was told to check
in a museum :-)
also, what *new* devices? this hardware is pretty ancient what we have
listed now should be close to everything
> What happens if we just revert
> 75cf7456dd87335f574dcd53c4ae616a2ad71a11?
the quirk gets run for VIA chips on chipsets that don't need it, so
the interrupt line is masked down when it sometimes shouldn't be (does
it mean they will always be IO-APIC-edge / PIC in that case?)
> Is there was a problem, is there something we can do at runtime in
> quirk_via_irq() to avoid the problem, rather than expanding out the
> fixup list in this manner?
from reading the comment and chedcking over pci_ids.h i think the list
is complete
in fact, i'm not sure that PCI_DEVICE_ID_VIA_82C586_0 and
PCI_DEVICE_ID_VIA_82C686 are strictly speaking supposed to be in the
list, it's unclear there and seems harmless enough