Re: [PATCH] VIA quirk fixup, additional PCI IDs

[Chris Wedgwood <cw@f00f.org>]

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