diff mbox

[2/7] x86/acpi: calling mp_config_acpi_gsi in mp_register_gsi -v2

Message ID 4A01C387.7090103@kernel.org (mailing list archive)
State Not Applicable, archived
Headers show

Commit Message

Yinghai Lu May 6, 2009, 5:06 p.m. UTC
it seems that change about calling mp_config_acpi_gsi in acpi_pci_irq_enable()
never get into mainline.

actually we could call that in mp_register_gsi with struct device...
also could make mp_config_acpi_gsi to be static

v2: some calling could have to be pci device, need to check that

[Impact: make update_mptable working]

Signed-off-by: Yinghai Lu <yinghai@kernel.org>

---
 arch/x86/include/asm/mpspec.h |    9 -----
 arch/x86/kernel/acpi/boot.c   |   66 +++++++++++++++++++++++++-----------------
 2 files changed, 40 insertions(+), 35 deletions(-)

--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Len Brown May 14, 2009, 4:20 p.m. UTC | #1
mp_config_acpi_gsi() has been dead code since the day it went into 2.6.27.
Rather than repairing that, I'd rather see it removed.

The justification for 2944e16b25e7fb8b5ee0dd9dc7197a0f9e523cfd
"x86: update mptable" was:

    1. kexec kernel with acpi=off
    2. work around BIOSes where acpi routing is working, but mptable is
       not right, so can use kernel/kexec to start other OSes that don't have
       good acpi support.
    
    command line: update_mptable

So this is a command line option to allow
MPS code to call into ACPI code to support kexec from an ACPI
mode into non-ACPI mode -- all without a single comment in the code.

I'm not convinced that the scenario that this code was
intended to address justifies adding unreadable code to
an area of the kernel which is notoriously over-complicated
and fragile.  Doing so is the opposite of optimizing for maintenance.

No, I don't want to see any MPS code ever call into ACPI code.

thanks,
Len Brown, Intel Open Source Technology Center
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Yinghai Lu May 14, 2009, 5:43 p.m. UTC | #2
Len Brown wrote:
> mp_config_acpi_gsi() has been dead code since the day it went into 2.6.27.
> Rather than repairing that, I'd rather see it removed.
> 
> The justification for 2944e16b25e7fb8b5ee0dd9dc7197a0f9e523cfd
> "x86: update mptable" was:
> 
>     1. kexec kernel with acpi=off
>     2. work around BIOSes where acpi routing is working, but mptable is
>        not right, so can use kernel/kexec to start other OSes that don't have
>        good acpi support.
>     
>     command line: update_mptable
> 
> So this is a command line option to allow
> MPS code to call into ACPI code to support kexec from an ACPI
> mode into non-ACPI mode -- all without a single comment in the code.
> 
> I'm not convinced that the scenario that this code was
> intended to address justifies adding unreadable code to
> an area of the kernel which is notoriously over-complicated
> and fragile.  Doing so is the opposite of optimizing for maintenance.

also could use it to show BIOS engineers what is correct mptable and that could help them to 
fix the problem in mptable creating when the system have a lot pci cards with bridge in the card.

> 
> No, I don't want to see any MPS code ever call into ACPI code.

how about:
make enable_update_mptable global, and in mp_register_gsi only call mp_config_acpi_gsi()
when enable_update_mptable == 1

YH

--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

Index: linux-2.6/arch/x86/include/asm/mpspec.h
===================================================================
--- linux-2.6.orig/arch/x86/include/asm/mpspec.h
+++ linux-2.6/arch/x86/include/asm/mpspec.h
@@ -77,17 +77,8 @@  extern int mp_register_gsi(struct device
 				 int active_high_low);
 extern int acpi_probe_gsi(void);
 #ifdef CONFIG_X86_IO_APIC
-extern int mp_config_acpi_gsi(unsigned char number, unsigned int devfn, u8 pin,
-				u32 gsi, int triggering, int polarity);
 extern int mp_find_ioapic(int gsi);
 extern int mp_find_ioapic_pin(int ioapic, int gsi);
-#else
-static inline int
-mp_config_acpi_gsi(unsigned char number, unsigned int devfn, u8 pin,
-		   u32 gsi, int triggering, int polarity)
-{
-	return 0;
-}
 #endif
 #else /* !CONFIG_ACPI: */
 static inline int acpi_probe_gsi(void)
Index: linux-2.6/arch/x86/kernel/acpi/boot.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/acpi/boot.c
+++ linux-2.6/arch/x86/kernel/acpi/boot.c
@@ -33,6 +33,7 @@ 
 #include <linux/irq.h>
 #include <linux/bootmem.h>
 #include <linux/ioport.h>
+#include <linux/pci.h>
 
 #include <asm/pgtable.h>
 #include <asm/io_apic.h>
@@ -1158,6 +1159,44 @@  void __init mp_config_acpi_legacy_irqs(v
 	}
 }
 
+static int mp_config_acpi_gsi(struct device *dev, u32 gsi, int triggering,
+			int polarity)
+{
+#ifdef CONFIG_X86_MPPARSE
+	struct mpc_intsrc mp_irq;
+	struct pci_dev *pdev;
+	unsigned char number;
+	unsigned int devfn;
+	int ioapic;
+	u8 pin;
+
+	if (!acpi_ioapic)
+		return 0;
+	if (!dev)
+		return 0;
+	if (dev->bus != &pci_bus_type)
+		return 0;
+
+	pdev = to_pci_dev(dev);
+	number = pdev->bus->number;
+	devfn = pdev->devfn;
+	pin = pdev->pin;
+	/* print the entry should happen on mptable identically */
+	mp_irq.type = MP_INTSRC;
+	mp_irq.irqtype = mp_INT;
+	mp_irq.irqflag = (triggering == ACPI_EDGE_SENSITIVE ? 4 : 0x0c) |
+				(polarity == ACPI_ACTIVE_HIGH ? 1 : 3);
+	mp_irq.srcbus = number;
+	mp_irq.srcbusirq = (((devfn >> 3) & 0x1f) << 2) | ((pin - 1) & 3);
+	ioapic = mp_find_ioapic(gsi);
+	mp_irq.dstapic = mp_ioapic_routing[ioapic].apic_id;
+	mp_irq.dstirq = mp_find_ioapic_pin(ioapic, gsi);
+
+	save_mp_irq(&mp_irq);
+#endif
+	return 0;
+}
+
 int mp_register_gsi(struct device *dev, u32 gsi, int triggering, int polarity)
 {
 	int ioapic;
@@ -1189,6 +1228,7 @@  int mp_register_gsi(struct device *dev,
 		       ioapic_pin);
 		return gsi;
 	}
+	mp_config_acpi_gsi(dev, gsi, triggering, polarity);
 
 	/*
 	 * Avoid pin reprogramming.  PRTs typically include entries
@@ -1208,32 +1248,6 @@  int mp_register_gsi(struct device *dev,
 	return gsi;
 }
 
-int mp_config_acpi_gsi(unsigned char number, unsigned int devfn, u8 pin,
-			u32 gsi, int triggering, int polarity)
-{
-#ifdef CONFIG_X86_MPPARSE
-	struct mpc_intsrc mp_irq;
-	int ioapic;
-
-	if (!acpi_ioapic)
-		return 0;
-
-	/* print the entry should happen on mptable identically */
-	mp_irq.type = MP_INTSRC;
-	mp_irq.irqtype = mp_INT;
-	mp_irq.irqflag = (triggering == ACPI_EDGE_SENSITIVE ? 4 : 0x0c) |
-				(polarity == ACPI_ACTIVE_HIGH ? 1 : 3);
-	mp_irq.srcbus = number;
-	mp_irq.srcbusirq = (((devfn >> 3) & 0x1f) << 2) | ((pin - 1) & 3);
-	ioapic = mp_find_ioapic(gsi);
-	mp_irq.dstapic = mp_ioapic_routing[ioapic].apic_id;
-	mp_irq.dstirq = mp_find_ioapic_pin(ioapic, gsi);
-
-	save_mp_irq(&mp_irq);
-#endif
-	return 0;
-}
-
 /*
  * Parse IOAPIC related entries in MADT
  * returns 0 on success, < 0 on error