diff mbox

pci: Simplify hotplug mch quirk.

Message ID m1bplnuzq4.fsf@fess.ebiederm.org (mailing list archive)
State Accepted, archived
Headers show

Commit Message

Eric W. Biederman Sept. 7, 2009, 4:48 a.m. UTC 
There is a very old quirk for the intel E7502 E7320 and E7525 memory
controller hubs that disables usage of msi interrupts on pcie hotplug
bridges of those devices, and disables changing the affinity of irqs.

Today all we have to do to disable msi on a specific device is to set
dev->no_msi, which is much more straightforward than the previous
logic.

The re-running of this fixup after pci hotplug happens below these
devices is totally bogus.  All of the state we change is pure software
state and we don't change the hardware at all.  Which means hotplug on
the lower devices doesn't have a chance to change this state.  So we
can safely remove the special case from the pciehp driver and the pcie
portdriver.

I suspect the special case was someone's expermental debug code that
slipped in. Certainly it isn't mentioned in commit
6fb8880a61510295aece04a542767161f624dffe aka BKrev:
41966101LJ_ogfOU0m2aE6teZfQnuQ where the code first appears.

Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>

---
 drivers/pci/hotplug/pciehp_ctrl.c |    5 -----
 drivers/pci/hotplug/pciehp_pci.c  |    5 -----
 drivers/pci/pci.h                 |    1 -
 drivers/pci/pcie/portdrv_core.c   |    5 -----
 drivers/pci/quirks.c              |    5 ++---
 5 files changed, 2 insertions(+), 19 deletions(-)

Comments

Kenji Kaneshige Sept. 7, 2009, 6:25 a.m. UTC | #1 
Eric W. Biederman wrote:
> There is a very old quirk for the intel E7502 E7320 and E7525 memory
> controller hubs that disables usage of msi interrupts on pcie hotplug
> bridges of those devices, and disables changing the affinity of irqs.
> 
> Today all we have to do to disable msi on a specific device is to set
> dev->no_msi, which is much more straightforward than the previous
> logic.
> 
> The re-running of this fixup after pci hotplug happens below these
> devices is totally bogus.  All of the state we change is pure software
> state and we don't change the hardware at all.  Which means hotplug on
> the lower devices doesn't have a chance to change this state.  So we
> can safely remove the special case from the pciehp driver and the pcie
> portdriver.
> 
> I suspect the special case was someone's expermental debug code that
> slipped in. Certainly it isn't mentioned in commit
> 6fb8880a61510295aece04a542767161f624dffe aka BKrev:
> 41966101LJ_ogfOU0m2aE6teZfQnuQ where the code first appears.
> 
> Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
> 
> ---
>  drivers/pci/hotplug/pciehp_ctrl.c |    5 -----
>  drivers/pci/hotplug/pciehp_pci.c  |    5 -----
>  drivers/pci/pci.h                 |    1 -
>  drivers/pci/pcie/portdrv_core.c   |    5 -----
>  drivers/pci/quirks.c              |    5 ++---
>  5 files changed, 2 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/pci/hotplug/pciehp_ctrl.c b/drivers/pci/hotplug/pciehp_ctrl.c
> index 8aab8ed..b97cb4c 100644
> --- a/drivers/pci/hotplug/pciehp_ctrl.c
> +++ b/drivers/pci/hotplug/pciehp_ctrl.c
> @@ -246,11 +246,6 @@ static int board_added(struct slot *p_slot)
>  		goto err_exit;
>  	}
>  
> -	/*
> -	 * Some PCI Express root ports require fixup after hot-plug operation.
> -	 */
> -	if (pcie_mch_quirk)
> -		pci_fixup_device(pci_fixup_final, ctrl->pci_dev);
>  	if (PWR_LED(ctrl))
>    		p_slot->hpc_ops->green_led_on(p_slot);
>  
> diff --git a/drivers/pci/hotplug/pciehp_pci.c b/drivers/pci/hotplug/pciehp_pci.c
> index 10f9566..af295d0 100644
> --- a/drivers/pci/hotplug/pciehp_pci.c
> +++ b/drivers/pci/hotplug/pciehp_pci.c
> @@ -285,11 +285,6 @@ int pciehp_unconfigure_device(struct slot *p_slot)
>  		}
>  		pci_dev_put(temp);
>  	}
> -	/*
> -	 * Some PCI Express root ports require fixup after hot-plug operation.
> -	 */
> -	if (pcie_mch_quirk)
> -		pci_fixup_device(pci_fixup_final, p_slot->ctrl->pci_dev);
>  
>  	return rc;
>  }
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index f73bcbe..4bb29b7 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -133,7 +133,6 @@ static inline int pci_no_d1d2(struct pci_dev *dev)
>  	return (dev->no_d1d2 || parent_dstates);
>  
>  }
> -extern int pcie_mch_quirk;
>  extern struct device_attribute pci_dev_attrs[];
>  extern struct device_attribute dev_attr_cpuaffinity;
>  extern struct device_attribute dev_attr_cpulistaffinity;
> diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c
> index 13ffdc3..af9bca3 100644
> --- a/drivers/pci/pcie/portdrv_core.c
> +++ b/drivers/pci/pcie/portdrv_core.c
> @@ -191,10 +191,6 @@ static int assign_interrupt_mode(struct pci_dev *dev, int *vectors, int mask)
>  	int irq, interrupt_mode = PCIE_PORT_NO_IRQ;
>  	int i;
>  
> -	/* Check MSI quirk */
> -	if (port_data->port_type == PCIE_RC_PORT && pcie_mch_quirk)
> -		goto Fallback;
> -
>  	/* Try to use MSI-X if supported */
>  	if (!pcie_port_enable_msix(dev, vectors, mask))
>  		return PCIE_PORT_MSIX_MODE;
> @@ -203,7 +199,6 @@ static int assign_interrupt_mode(struct pci_dev *dev, int *vectors, int mask)
>  	if (!pci_enable_msi(dev))
>  		interrupt_mode = PCIE_PORT_MSI_MODE;
>  
> - Fallback:
>  	if (interrupt_mode == PCIE_PORT_NO_IRQ && dev->pin)
>  		interrupt_mode = PCIE_PORT_INTx_MODE;
>  
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index 06b9656..6d15879 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -31,8 +31,6 @@ int isa_dma_bridge_buggy;
>  EXPORT_SYMBOL(isa_dma_bridge_buggy);
>  int pci_pci_problems;
>  EXPORT_SYMBOL(pci_pci_problems);
> -int pcie_mch_quirk;
> -EXPORT_SYMBOL(pcie_mch_quirk);
>  
>  #ifdef CONFIG_PCI_QUIRKS
>  /*
> @@ -1499,7 +1497,8 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL,	PCI_DEVICE_ID_INTEL_EESSC,	quirk_a
>  
>  static void __devinit quirk_pcie_mch(struct pci_dev *pdev)
>  {
> -	pcie_mch_quirk = 1;
> +	pci_msi_off(pdev);
> +	pdev->no_msi = 1;
>  }
>  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,	PCI_DEVICE_ID_INTEL_E7520_MCH,	quirk_pcie_mch);
>  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,	PCI_DEVICE_ID_INTEL_E7320_MCH,	quirk_pcie_mch);

Looks good to me.

Reviewed-by: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>


--
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
Jesse Barnes Sept. 9, 2009, 9:11 p.m. UTC | #2 
On Sun, 06 Sep 2009 21:48:35 -0700
ebiederm@xmission.com (Eric W. Biederman) wrote:

> 
> There is a very old quirk for the intel E7502 E7320 and E7525 memory
> controller hubs that disables usage of msi interrupts on pcie hotplug
> bridges of those devices, and disables changing the affinity of irqs.
> 
> Today all we have to do to disable msi on a specific device is to set
> dev->no_msi, which is much more straightforward than the previous
> logic.
> 
> The re-running of this fixup after pci hotplug happens below these
> devices is totally bogus.  All of the state we change is pure software
> state and we don't change the hardware at all.  Which means hotplug on
> the lower devices doesn't have a chance to change this state.  So we
> can safely remove the special case from the pciehp driver and the pcie
> portdriver.
> 
> I suspect the special case was someone's expermental debug code that
> slipped in. Certainly it isn't mentioned in commit
> 6fb8880a61510295aece04a542767161f624dffe aka BKrev:
> 41966101LJ_ogfOU0m2aE6teZfQnuQ where the code first appears.
> 
> Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>

Applied to linux-next, thanks.
diff mbox

Patch

diff --git a/drivers/pci/hotplug/pciehp_ctrl.c b/drivers/pci/hotplug/pciehp_ctrl.c
index 8aab8ed..b97cb4c 100644
--- a/drivers/pci/hotplug/pciehp_ctrl.c
+++ b/drivers/pci/hotplug/pciehp_ctrl.c
@@ -246,11 +246,6 @@  static int board_added(struct slot *p_slot)
 		goto err_exit;
 	}
 
-	/*
-	 * Some PCI Express root ports require fixup after hot-plug operation.
-	 */
-	if (pcie_mch_quirk)
-		pci_fixup_device(pci_fixup_final, ctrl->pci_dev);
 	if (PWR_LED(ctrl))
   		p_slot->hpc_ops->green_led_on(p_slot);
 
diff --git a/drivers/pci/hotplug/pciehp_pci.c b/drivers/pci/hotplug/pciehp_pci.c
index 10f9566..af295d0 100644
--- a/drivers/pci/hotplug/pciehp_pci.c
+++ b/drivers/pci/hotplug/pciehp_pci.c
@@ -285,11 +285,6 @@  int pciehp_unconfigure_device(struct slot *p_slot)
 		}
 		pci_dev_put(temp);
 	}
-	/*
-	 * Some PCI Express root ports require fixup after hot-plug operation.
-	 */
-	if (pcie_mch_quirk)
-		pci_fixup_device(pci_fixup_final, p_slot->ctrl->pci_dev);
 
 	return rc;
 }
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index f73bcbe..4bb29b7 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -133,7 +133,6 @@  static inline int pci_no_d1d2(struct pci_dev *dev)
 	return (dev->no_d1d2 || parent_dstates);
 
 }
-extern int pcie_mch_quirk;
 extern struct device_attribute pci_dev_attrs[];
 extern struct device_attribute dev_attr_cpuaffinity;
 extern struct device_attribute dev_attr_cpulistaffinity;
diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c
index 13ffdc3..af9bca3 100644
--- a/drivers/pci/pcie/portdrv_core.c
+++ b/drivers/pci/pcie/portdrv_core.c
@@ -191,10 +191,6 @@  static int assign_interrupt_mode(struct pci_dev *dev, int *vectors, int mask)
 	int irq, interrupt_mode = PCIE_PORT_NO_IRQ;
 	int i;
 
-	/* Check MSI quirk */
-	if (port_data->port_type == PCIE_RC_PORT && pcie_mch_quirk)
-		goto Fallback;
-
 	/* Try to use MSI-X if supported */
 	if (!pcie_port_enable_msix(dev, vectors, mask))
 		return PCIE_PORT_MSIX_MODE;
@@ -203,7 +199,6 @@  static int assign_interrupt_mode(struct pci_dev *dev, int *vectors, int mask)
 	if (!pci_enable_msi(dev))
 		interrupt_mode = PCIE_PORT_MSI_MODE;
 
- Fallback:
 	if (interrupt_mode == PCIE_PORT_NO_IRQ && dev->pin)
 		interrupt_mode = PCIE_PORT_INTx_MODE;
 
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 06b9656..6d15879 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -31,8 +31,6 @@  int isa_dma_bridge_buggy;
 EXPORT_SYMBOL(isa_dma_bridge_buggy);
 int pci_pci_problems;
 EXPORT_SYMBOL(pci_pci_problems);
-int pcie_mch_quirk;
-EXPORT_SYMBOL(pcie_mch_quirk);
 
 #ifdef CONFIG_PCI_QUIRKS
 /*
@@ -1499,7 +1497,8 @@  DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL,	PCI_DEVICE_ID_INTEL_EESSC,	quirk_a
 
 static void __devinit quirk_pcie_mch(struct pci_dev *pdev)
 {
-	pcie_mch_quirk = 1;
+	pci_msi_off(pdev);
+	pdev->no_msi = 1;
 }
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,	PCI_DEVICE_ID_INTEL_E7520_MCH,	quirk_pcie_mch);
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,	PCI_DEVICE_ID_INTEL_E7320_MCH,	quirk_pcie_mch);