[PATCH] VIA IRQ quirk fixup only in XT_PIC mode Take 2

[PATCH] VIA IRQ quirk fixup only in XT_PIC mode Take 2

[Sergio Monteiro Basto]

Hi, this patch (now for 2.6.18-rc2) is more readable.

Thanks,
--
Sérgio M. B. 

Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
Cc: "Scott J. Harmon" <harmon@ksu.edu>
Cc: Andrew Morton <akpm@osdl.org>
Cc: Chris Wedgwood <cw@f00f.org>
Cc: Greg KH <greg@kroah.com>
Signed-off-by: Sergio Monteiro Basto <sergio@sergiomb.no-ip.org>
---
 quirks.c |   14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

--- linux-2.6.17.i686/drivers/pci/quirks.c.orig	2006-07-28 12:59:04.000000000 +0100
+++ linux-2.6.17.i686/drivers/pci/quirks.c	2006-07-28 13:26:49.000000000 +0100
@@ -648,11 +648,17 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_V
  *
  * Some of the on-chip devices are actually '586 devices' so they are
  * listed here.
+ * 
+ * if flags say that we have working apic(s), we don't need to quirk these
+ * devices
  */
 static void quirk_via_irq(struct pci_dev *dev)
 {
 	u8 irq, new_irq;
 
+	if ((smp_found_config && !skip_ioapic_setup && nr_ioapics) || cpu_has_apic)
+		return;
+
 	new_irq = dev->irq & 0xf;
 	pci_read_config_byte(dev, PCI_INTERRUPT_LINE, &irq);
 	if (new_irq != irq) {
@@ -662,13 +668,7 @@ 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);
+DECLARE_PCI_FIXUP_ENABLE(PCI_VENDOR_ID_VIA, PCI_ANY_ID, quirk_via_irq);
 
 /*
  * VIA VT82C598 has its device ID settable and many BIOSes

Re: [PATCH] VIA IRQ quirk fixup only in XT_PIC mode Take 2

[Andrew Morton]

On Fri, 28 Jul 2006 14:01:01 +0100
Sergio Monteiro Basto <sergio@sergiomb.no-ip.org> wrote:

> Hi, this patch (now for 2.6.18-rc2) is more readable.

It has no changelog, and this sort of patch does need a lenghty one, please.

What relationship does it have to
ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.18-rc2/2.6.18-rc2-mm1/broken-out/pci-quirk_via_irq-behaviour-change.patch?
 If it is better, why?

Re: [PATCH] VIA IRQ quirk fixup only in XT_PIC mode Take 2

[Sergio Monteiro Basto]

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

ok, 
Begging by answer the question, Daniel Drake, clearly, say:
"Nobody really has a full understanding of the problem and the
issues ...". 
As I can understand, make a sort of blacklist with IDs of the
problematic VIAs, which probably will always miss some IDs, but the main
problem is in lasts lines of this email. The way how is design the list,
could be one excellent idea, inclusive for other situations on VIAs, I
don't put this in question. 

After reviewing what I had write about this, the changelog: 
(please correct me the English and cut some parts) 
Well, a good new, I already have my old laptop again which have the most
problematic VIA (PCI_DEVICE_ID_VIA_82C686). At the time I bought it, I
had problematic stuffs when kernel was trying enable APIC, on this
laptop, was one Local APIC, later we had the boot option nolapic to
workaround this. So the VIA works very well without lapic enabled.
Someone give to this problem a name of "the APIC victims", happened on
VIAs, Nvidias and probably on others boards. 
But this laptop need the quirks; nobody have put this in question,
VIA_82C686 has been always on list of the quirks. And btw, is important,
_need_ ACPI, with acpi=off, 1/2 of the computer: power manager and
hot-key, don't work at all. 
A few of the story about my laptop http://sergiomb.no-ip.org/laptop/

So, a few month ago, I bought, what I bought, one VIA of course :) with
this IDs: PCI_DEVICE_ID_VIA_82C586_1 and _2.
With this new VIA, I have exactly, the opposite problem, I found, on
this VIA with IO-APIC working well, that quirks aren't good/needed .
After, I found this interesting email http://lkml.org/lkml/2005/8/13/30
by Karsten Wiese, after, Alan Cox writes this
http://lkml.org/lkml/2005/8/16/160 (on same thread) and Karsten Wiese
end ups with the solution on http://lkml.org/lkml/2005/8/18/92, which I
want try to implement and that's it, come this idea, that may the full
understanding the problem.  
Conclusion, I have 2 VIAs with almost same IDs (others reporters have
with exactly the same IDs) and in ones I need the quirks and in others
don't, why ? one don't have apics enabled, the other have it !?!

Thanks, 

On Fri, 2006-07-28 at 12:24 -0700, Andrew Morton wrote:
> On Fri, 28 Jul 2006 14:01:01 +0100
> Sergio Monteiro Basto <sergio@sergiomb.no-ip.org> wrote:
> 
> > Hi, this patch (now for 2.6.18-rc2) is more readable.
> 
> It has no changelog, and this sort of patch does need a lenghty one, please.
> 
> What relationship does it have to
> ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.18-rc2/2.6.18-rc2-mm1/broken-out/pci-quirk_via_irq-behaviour-change.patch?
>  If it is better, why?
-- 
Sérgio M.B.

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

Re: [PATCH] VIA IRQ quirk fixup only in XT_PIC mode Take 2

[Daniel Drake]

Sergio Monteiro Basto wrote:
> Hi, this patch (now for 2.6.18-rc2) is more readable.

Ok, it looks like I was wrong in my earlier mails, ACPI vs APIC 
confusion. In the bug reports I mentioned, APIC was not being used in 
any circumstances (however it is still a bit strange how these systems 
do not need quirks when acpi=off).

I'm reasonably certain that this patch will apply the quirks on the 
affected systems again, so I'm happy for it to be applied, people will 
be able to use their hardware again. However I'm not sure how good a 
solution it is, because in some circumstances it will apply the quirks 
to VIA PCI cards on non-VIA boards, which was the reason we messed with 
this code in the first place. We could possibly merge it with the 
southbridge detection hack, but it gets a bit silly at that point...

Jeff/Chris: any thoughts?

> Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
> Cc: "Scott J. Harmon" <harmon@ksu.edu>
> Cc: Andrew Morton <akpm@osdl.org>
> Cc: Chris Wedgwood <cw@f00f.org>
> Cc: Greg KH <greg@kroah.com>
> Signed-off-by: Sergio Monteiro Basto <sergio@sergiomb.no-ip.org>
> ---
>  quirks.c |   14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> --- linux-2.6.17.i686/drivers/pci/quirks.c.orig	2006-07-28 12:59:04.000000000 +0100
> +++ linux-2.6.17.i686/drivers/pci/quirks.c	2006-07-28 13:26:49.000000000 +0100
> @@ -648,11 +648,17 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_V
>   *
>   * Some of the on-chip devices are actually '586 devices' so they are
>   * listed here.
> + * 
> + * if flags say that we have working apic(s), we don't need to quirk these
> + * devices
>   */
>  static void quirk_via_irq(struct pci_dev *dev)
>  {
>  	u8 irq, new_irq;
>  
> +	if ((smp_found_config && !skip_ioapic_setup && nr_ioapics) || cpu_has_apic)
> +		return;
> +
>  	new_irq = dev->irq & 0xf;
>  	pci_read_config_byte(dev, PCI_INTERRUPT_LINE, &irq);
>  	if (new_irq != irq) {
> @@ -662,13 +668,7 @@ 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);
> +DECLARE_PCI_FIXUP_ENABLE(PCI_VENDOR_ID_VIA, PCI_ANY_ID, quirk_via_irq);
>  
>  /*
>   * VIA VT82C598 has its device ID settable and many BIOSes
> 
> 
> 
> 

Re: [PATCH] VIA IRQ quirk fixup only in XT_PIC mode Take 2

[Sergio Monteiro Basto]

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

Hi, Sorry for so late answer , I have been in holidays and  busy.

I remember check my emails that I send to Len Brown about this subject.
And I found, what I want, is just revert one patch of Bjorn Helgaas :)
between kernel 2.6.12-rc5 and 6.13.

Check this out
http://sourceforge.net/mailarchive/message.php?msg_id=11858102

so I propose this patch like this :

 static void quirk_via_irq(struct pci_dev *dev)
  {
  	u8 irq, new_irq;
  
 +#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
  	new_irq = dev->irq & 0xf;
  	pci_read_config_byte(dev, PCI_INTERRUPT_LINE, &irq);
  	if (new_irq != irq) {
 
On Sat, 2006-08-12 at 23:47 +0100, Daniel Drake wrote:
> Sergio Monteiro Basto wrote:
> > Hi, this patch (now for 2.6.18-rc2) is more readable.
> 
> Ok, it looks like I was wrong in my earlier mails, ACPI vs APIC 
> confusion. In the bug reports I mentioned, APIC was not being used in 
> any circumstances (however it is still a bit strange how these systems 
> do not need quirks when acpi=off).

I still convinced that we need patch even with acpi=off , the problem
doesn't appear only on boot time.


> 
> I'm reasonably certain that this patch will apply the quirks on the 
> affected systems again, so I'm happy for it to be applied, people will 
> be able to use their hardware again. However I'm not sure how good a 
> solution it is, because in some circumstances it will apply the quirks 
> to VIA PCI cards on non-VIA boards, which was the reason we messed with 
> this code in the first place. We could possibly merge it with the 
> southbridge detection hack, but it gets a bit silly at that point...
> 
> Jeff/Chris: any thoughts?
> 
> > Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
> > Cc: "Scott J. Harmon" <harmon@ksu.edu>
> > Cc: Andrew Morton <akpm@osdl.org>
> > Cc: Chris Wedgwood <cw@f00f.org>
> > Cc: Greg KH <greg@kroah.com>
> > Signed-off-by: Sergio Monteiro Basto <sergio@sergiomb.no-ip.org>
> > ---
> >  quirks.c |   14 +++++++-------
> >  1 file changed, 7 insertions(+), 7 deletions(-)
> > 
> > --- linux-2.6.17.i686/drivers/pci/quirks.c.orig	2006-07-28 12:59:04.000000000 +0100
> > +++ linux-2.6.17.i686/drivers/pci/quirks.c	2006-07-28 13:26:49.000000000 +0100
> > @@ -648,11 +648,17 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_V
> >   *
> >   * Some of the on-chip devices are actually '586 devices' so they are
> >   * listed here.
> > + * 
> > + * if flags say that we have working apic(s), we don't need to quirk these
> > + * devices
> >   */
> >  static void quirk_via_irq(struct pci_dev *dev)
> >  {
> >  	u8 irq, new_irq;
> >  
> > +	if ((smp_found_config && !skip_ioapic_setup && nr_ioapics) || cpu_has_apic)
> > +		return;
> > +
> >  	new_irq = dev->irq & 0xf;
> >  	pci_read_config_byte(dev, PCI_INTERRUPT_LINE, &irq);
> >  	if (new_irq != irq) {
> > @@ -662,13 +668,7 @@ 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);
> > +DECLARE_PCI_FIXUP_ENABLE(PCI_VENDOR_ID_VIA, PCI_ANY_ID, quirk_via_irq);
> >  
> >  /*
> >   * VIA VT82C598 has its device ID settable and many BIOSes
> > 
> > 
> > 
> > 
> 

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

Re: [PATCH] VIA IRQ quirk fixup only in XT_PIC mode Take 2

[Daniel Drake]

Sergio Monteiro Basto wrote:
> I remember check my emails that I send to Len Brown about this subject.
> And I found, what I want, is just revert one patch of Bjorn Helgaas :)
> between kernel 2.6.12-rc5 and 6.13.

It does look like this patch was under discussion of being reverted 
before. See http://lkml.org/lkml/2005/9/26/183

The following comment still stands when we just revert Bjorn's change:

>> I'm reasonably certain that this patch will apply the quirks on the 
>> affected systems again, so I'm happy for it to be applied, people will 
>> be able to use their hardware again. However I'm not sure how good a 
>> solution it is, because in some circumstances it will apply the quirks 
>> to VIA PCI cards on non-VIA boards, which was the reason we messed with 
>> this code in the first place. We could possibly merge it with the 
>> southbridge detection hack, but it gets a bit silly at that point...

So perhaps the best solution is a combination of reverting Bjorn's 
patch, adding Linus' suggested change, and adding my southbridge hack.

Daniel

Re: [PATCH] VIA IRQ quirk fixup only in XT_PIC mode Take 2

[Sergio Monteiro Basto]

On Tue, 2006-08-29 at 23:46 -0400, Daniel Drake wrote:
> Sergio Monteiro Basto wrote:
> > I remember check my emails that I send to Len Brown about this subject.
> > And I found, what I want, is just revert one patch of Bjorn Helgaas :)
> > between kernel 2.6.12-rc5 and 6.13.
> 
> It does look like this patch was under discussion of being reverted 
> before. See http://lkml.org/lkml/2005/9/26/183

To be honest, I prefer put again this :

 +#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

about Linus suggestion : 
-	new_irq = dev->irq & 0xf;
+	new_irq = dev->irq;
+	if (!new_irq || new_irq >= 15)
+		return;

no, we have problem with VIA SATA controllers which have irq lower than
15 

Thanks,

> 
> The following comment still stands when we just revert Bjorn's change:
> 
> >> I'm reasonably certain that this patch will apply the quirks on the 
> >> affected systems again, so I'm happy for it to be applied, people will 
> >> be able to use their hardware again. However I'm not sure how good a 
> >> solution it is, because in some circumstances it will apply the quirks 
> >> to VIA PCI cards on non-VIA boards, which was the reason we messed with 
> >> this code in the first place. We could possibly merge it with the 
> >> southbridge detection hack, but it gets a bit silly at that point...
> 
> So perhaps the best solution is a combination of reverting Bjorn's 
> patch, adding Linus' suggested change, and adding my southbridge hack.
> 
> Daniel

Re: [PATCH] VIA IRQ quirk fixup only in XT_PIC mode Take 2

[Daniel Drake]

Sergio Monteiro Basto wrote:
>> It does look like this patch was under discussion of being reverted 
>> before. See http://lkml.org/lkml/2005/9/26/183
> 
> To be honest, I prefer put again this :
> 
>  +#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

Isn't this exactly the same as what was being suggested?

> about Linus suggestion : 
> -	new_irq = dev->irq & 0xf;
> +	new_irq = dev->irq;
> +	if (!new_irq || new_irq >= 15)
> +		return;
> 
> no, we have problem with VIA SATA controllers which have irq lower than
> 15 

Any chance you can provide a link to this example so that we can 
document the decision in the commit message?

Thanks,
Daniel

Re: [PATCH] VIA IRQ quirk fixup only in XT_PIC mode Take 2

[Sergio Monteiro Basto]

On Wed, 2006-08-30 at 12:13 -0400, Daniel Drake wrote:

> > about Linus suggestion : 
> > -	new_irq = dev->irq & 0xf;
> > +	new_irq = dev->irq;
> > +	if (!new_irq || new_irq >= 15)
> > +		return;
> > 
> > no, we have problem with VIA SATA controllers which have irq lower than
> > 15 
> 
> Any chance you can provide a link to this example so that we can 
> document the decision in the commit message?

http://lkml.org/lkml/2006/7/30/59



Thanks,
--
Sérgio M. B.

Re: [PATCH] VIA IRQ quirk fixup only in XT_PIC mode Take 2

[Daniel Drake]

Sergio Monteiro Basto wrote:
> On Wed, 2006-08-30 at 12:13 -0400, Daniel Drake wrote:
> 
>>> about Linus suggestion : 
>>> -	new_irq = dev->irq & 0xf;
>>> +	new_irq = dev->irq;
>>> +	if (!new_irq || new_irq >= 15)
>>> +		return;
>>>
>>> no, we have problem with VIA SATA controllers which have irq lower than
>>> 15 
>> Any chance you can provide a link to this example so that we can 
>> document the decision in the commit message?
> 
> http://lkml.org/lkml/2006/7/30/59

I'm confused. Heikki's report is about a sata_sil controller and he 
didn't include any dmesg output so I don't know how you can conclude 
that quirking an IRQ to something less than 15 was the fix...

Also note that the fix was *not* quirking the device at all (your patch 
ensured that the quirks didn't run because IO-APIC was enabled), this 
hardly seems like an accurate way of arguing that quirks that change the 
IRQ to something less than 15 are *required*...

Daniel


Daniel

Re: [PATCH] VIA IRQ quirk fixup only in XT_PIC mode Take 2

[Sergio Monteiro Basto]

On Tue, 2006-09-05 at 09:37 -0400, Daniel Drake wrote:
> Sergio Monteiro Basto wrote:
> > On Wed, 2006-08-30 at 12:13 -0400, Daniel Drake wrote:
> > 
> >>> about Linus suggestion : 
> >>> -   new_irq = dev->irq & 0xf;
> >>> +   new_irq = dev->irq;
> >>> +   if (!new_irq || new_irq >= 15)
> >>> +           return;
> >>>
> >>> no, we have problem with VIA SATA controllers which have irq lower
> than
> >>> 15 
> >> Any chance you can provide a link to this example so that we can 
> >> document the decision in the commit message?
> > 
> > http://lkml.org/lkml/2006/7/30/59
> 
> I'm confused. Heikki's report is about a sata_sil controller and he 
> didn't include any dmesg output so I don't know how you can conclude 
> that quirking an IRQ to something less than 15 was the fix...


> Also note that the fix was *not* quirking the device at all (your
> patch 
> ensured that the quirks didn't run because IO-APIC was enabled), this 
> hardly seems like an accurate way of arguing that quirks that change
> the 
> IRQ to something less than 15 are *required*... 

hum hum, "Vladimir B. Savkin" report on Asus A8V (kernel 2.6.14+) 
and Heikki Orsila talks about is ASUS K8V SE Deluxe on amd64 with SiI
3114 SATA controller. 
yes, when Heikki Orsila reports a positive test, I thought was talking
about "Vladimir B. Savkin" reports. 
Is petty that "Vladimir B. Savkin" don't response.
and Heikki Orsila don't show his dmesg :) 

thanks, 
--
Sérgio M.