2004-10-20 18:43:35

by Deepak Saxena

[permalink] [raw]
Subject: [PATCH] Remove inclusion of <linux/irq.h> from pci/quirks.c


<linux/irq.h> states:

/*
* Please do not include this file in generic code. There is currently
* no requirement for any architecture to implement anything held
* within this file.
*
* Thanks. --rmk
*/

The latest update into pci/quirks.c did not follow this and breaks
building on ARM.

Signed-off-by: Deepak Saxena <[email protected]>

===== drivers/pci/quirks.c 1.51 vs edited =====
--- 1.51/drivers/pci/quirks.c 2004-10-18 22:26:45 -07:00
+++ edited/drivers/pci/quirks.c 2004-10-20 11:15:14 -07:00
@@ -18,7 +18,6 @@
#include <linux/pci.h>
#include <linux/init.h>
#include <linux/delay.h>
-#include <linux/irq.h>

#undef DEBUG

--
Deepak Saxena - dsaxena at plexity dot net - http://www.plexity.net/

"Unlike me, many of you have accepted the situation of your imprisonment
and will die here like rotten cabbages." - Number 6


2004-10-21 05:14:54

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] Remove inclusion of <linux/irq.h> from pci/quirks.c

Deepak Saxena <[email protected]> wrote:
>
> <linux/irq.h> states:
>
> /*
> * Please do not include this file in generic code. There is currently
> * no requirement for any architecture to implement anything held
> * within this file.
> *
> * Thanks. --rmk
> */
>
> The latest update into pci/quirks.c did not follow this and breaks
> building on ARM.

But it does break building on x86, which might prove a tad unpopular.

That x86 code shouldn't be in the generic PCI code at all, I guess. This
patch move it into io_apic.c and appears to build OK. I'll play with it a
bit more.

Also, it worries me that quirk_intel_irqbalance() is marked __devinit and
calls irqbalance_disable(), which is marked __init. I guess a fix for that
would be to mark quirk_intel_irqbalance() as __init, since it's unlikely to
be called after free_initmem(). Does Greg agree?


25-akpm/arch/i386/kernel/io_apic.c | 45 +++++++++++++++++++++++++++++++++++++
25-akpm/drivers/pci/quirks.c | 44 ------------------------------------
arch/i386/kernel/pci-dma.c | 0
3 files changed, 45 insertions(+), 44 deletions(-)

diff -puN drivers/pci/quirks.c~move-quirk_intel_irqbalance drivers/pci/quirks.c
--- 25/drivers/pci/quirks.c~move-quirk_intel_irqbalance 2004-10-20 21:51:21.111892336 -0700
+++ 25-akpm/drivers/pci/quirks.c 2004-10-20 21:51:39.829046896 -0700
@@ -1210,50 +1210,6 @@ static void __init quirk_intel_ide_combi
DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, PCI_ANY_ID, quirk_intel_ide_combined );
#endif /* CONFIG_SCSI_SATA */

-#if defined(CONFIG_X86_IO_APIC) && defined(CONFIG_SMP)
-
-void __devinit quirk_intel_irqbalance(struct pci_dev *dev)
-{
- u8 config, rev;
- u32 word;
- extern struct pci_raw_ops *raw_pci_ops;
-
- /* BIOS may enable hardware IRQ balancing for
- * E7520/E7320/E7525(revision ID 0x9 and below)
- * based platforms.
- * Disable SW irqbalance/affinity on those platforms.
- */
- pci_read_config_byte(dev, PCI_CLASS_REVISION, &rev);
- if (rev > 0x9)
- return;
-
- printk(KERN_INFO "Intel E7520/7320/7525 detected.");
-
- /* enable access to config space*/
- pci_read_config_byte(dev, 0xf4, &config);
- config |= 0x2;
- pci_write_config_byte(dev, 0xf4, config);
-
- /* read xTPR register */
- raw_pci_ops->read(0, 0, 0x40, 0x4c, 2, &word);
-
- if (!(word & (1 << 13))) {
- printk(KERN_INFO "Disabling irq balancing and affinity\n");
-#ifdef CONFIG_IRQBALANCE
- irqbalance_disable("");
-#endif
- noirqdebug_setup("");
- no_irq_affinity = 1;
- }
-
- config &= ~0x2;
- /* disable access to config space*/
- pci_write_config_byte(dev, 0xf4, config);
-}
-DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_E7320_MCH, quirk_intel_irqbalance);
-DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_E7525_MCH, quirk_intel_irqbalance);
-#endif
-
int pciehp_msi_quirk;

static void __devinit quirk_pciehp_msi(struct pci_dev *pdev)
diff -puN arch/i386/kernel/pci-dma.c~move-quirk_intel_irqbalance arch/i386/kernel/pci-dma.c
diff -puN arch/i386/kernel/io_apic.c~move-quirk_intel_irqbalance arch/i386/kernel/io_apic.c
--- 25/arch/i386/kernel/io_apic.c~move-quirk_intel_irqbalance 2004-10-20 21:52:14.060842872 -0700
+++ 25-akpm/arch/i386/kernel/io_apic.c 2004-10-20 21:52:48.960537312 -0700
@@ -31,6 +31,7 @@
#include <linux/mc146818rtc.h>
#include <linux/compiler.h>
#include <linux/acpi.h>
+#include <linux/pci.h>

#include <linux/sysdev.h>
#include <asm/io.h>
@@ -2565,3 +2566,47 @@ int io_apic_set_pci_routing (int ioapic,
}

#endif /*CONFIG_ACPI_BOOT*/
+
+#if defined(CONFIG_SMP) && defined(CONFIG_PCI)
+void __devinit quirk_intel_irqbalance(struct pci_dev *dev)
+{
+ u8 config, rev;
+ u32 word;
+ extern struct pci_raw_ops *raw_pci_ops;
+
+ /* BIOS may enable hardware IRQ balancing for
+ * E7520/E7320/E7525(revision ID 0x9 and below)
+ * based platforms.
+ * Disable SW irqbalance/affinity on those platforms.
+ */
+ pci_read_config_byte(dev, PCI_CLASS_REVISION, &rev);
+ if (rev > 0x9)
+ return;
+
+ printk(KERN_INFO "Intel E7520/7320/7525 detected.");
+
+ /* enable access to config space*/
+ pci_read_config_byte(dev, 0xf4, &config);
+ config |= 0x2;
+ pci_write_config_byte(dev, 0xf4, config);
+
+ /* read xTPR register */
+ raw_pci_ops->read(0, 0, 0x40, 0x4c, 2, &word);
+
+ if (!(word & (1 << 13))) {
+ printk(KERN_INFO "Disabling irq balancing and affinity\n");
+#ifdef CONFIG_IRQBALANCE
+ irqbalance_disable("");
+#endif
+ noirqdebug_setup("");
+ no_irq_affinity = 1;
+ }
+
+ config &= ~0x2;
+ /* disable access to config space*/
+ pci_write_config_byte(dev, 0xf4, config);
+}
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_E7320_MCH, quirk_intel_irqbalance);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_E7525_MCH, quirk_intel_irqbalance);
+#endif
+
_

2004-10-28 21:35:35

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH] Remove inclusion of <linux/irq.h> from pci/quirks.c

On Wed, Oct 20, 2004 at 09:57:50PM -0700, Andrew Morton wrote:
> Deepak Saxena <[email protected]> wrote:
> >
> > <linux/irq.h> states:
> >
> > /*
> > * Please do not include this file in generic code. There is currently
> > * no requirement for any architecture to implement anything held
> > * within this file.
> > *
> > * Thanks. --rmk
> > */
> >
> > The latest update into pci/quirks.c did not follow this and breaks
> > building on ARM.
>
> But it does break building on x86, which might prove a tad unpopular.
>
> That x86 code shouldn't be in the generic PCI code at all, I guess. This
> patch move it into io_apic.c and appears to build OK. I'll play with it a
> bit more.
>
> Also, it worries me that quirk_intel_irqbalance() is marked __devinit and
> calls irqbalance_disable(), which is marked __init. I guess a fix for that
> would be to mark quirk_intel_irqbalance() as __init, since it's unlikely to
> be called after free_initmem(). Does Greg agree?

I do agree. But I think the intel people are mucking around in this
area too and hopefully they'll fix it all up soon...

thanks,

greg k-h

2004-10-28 21:38:03

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] Remove inclusion of <linux/irq.h> from pci/quirks.c

Greg KH <[email protected]> wrote:
>
> > Also, it worries me that quirk_intel_irqbalance() is marked __devinit and
> > calls irqbalance_disable(), which is marked __init. I guess a fix for that
> > would be to mark quirk_intel_irqbalance() as __init, since it's unlikely to
> > be called after free_initmem(). Does Greg agree?
>
> I do agree. But I think the intel people are mucking around in this
> area too and hopefully they'll fix it all up soon...

It should all be fixed up now. We moved the intel-specific quirk to the
new arch/i386/kernel/quirks.c.