All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mark Lord <lkml@rtr.ca>
To: kristen.c.accardi@intel.com
Cc: Linux Kernel <linux-kernel@vger.kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	"Theodore Ts'o" <tytso@mit.edu>,
	greg@kroah.com, pcihpd-discuss@lists.sourceforge.net
Subject: [PATCH] Fix PCIe double initialization bug
Date: Sat, 17 Nov 2007 19:27:19 -0500	[thread overview]
Message-ID: <473F86E7.4020003@rtr.ca> (raw)
In-Reply-To: <4716CB9D.6080503@rtr.ca>

pciehp_fix_double_init_bug.patch:

Earlier patches to split out the hardware init for PCIe hotplug
resulted in some one-time initializations being redone on every
resume cycle.  Eg. irq/polling initialization.

This patch splits the hardware init into two parts,
and separates the one-time initializations from those
so that they only ever get done once, as intended.

Signed-off-by: Mark Lord <mlord@pobox.com>
---

This patch is for -mm and for Kristen's queue.  Not for 2.6.24.

 drivers/pci/hotplug/pciehp.h      |    2
 drivers/pci/hotplug/pciehp_core.c |    2
 drivers/pci/hotplug/pciehp_hpc.c  |  119 +++++++++++++++-------------
 3 files changed, 69 insertions(+), 54 deletions(-)

--- linux/drivers/pci/hotplug/pciehp.h.orig	2007-11-13 23:57:09.000000000 -0500
+++ linux/drivers/pci/hotplug/pciehp.h	2007-11-17 19:10:01.000000000 -0500
@@ -163,7 +163,7 @@
 int pcie_init(struct controller *ctrl, struct pcie_device *dev);
 int pciehp_enable_slot(struct slot *p_slot);
 int pciehp_disable_slot(struct slot *p_slot);
-int pcie_init_hardware(struct controller *ctrl, struct pcie_device *dev);
+int pcie_init_hardware_part2(struct controller *ctrl, struct pcie_device *dev);
 
 static inline struct slot *pciehp_find_slot(struct controller *ctrl, u8 device)
 {
--- linux/drivers/pci/hotplug/pciehp_core.c.orig	2007-11-13 23:57:09.000000000 -0500
+++ linux/drivers/pci/hotplug/pciehp_core.c	2007-11-17 19:09:43.000000000 -0500
@@ -521,7 +521,7 @@
 		u8 status;
 
 		/* reinitialize the chipset's event detection logic */
-		pcie_init_hardware(ctrl, dev);
+		pcie_init_hardware_part2(ctrl, dev);
 
 		t_slot = pciehp_find_slot(ctrl, ctrl->slot_device_offset);
 
--- linux/drivers/pci/hotplug/pciehp_hpc.c.orig	2007-11-13 23:57:09.000000000 -0500
+++ linux/drivers/pci/hotplug/pciehp_hpc.c	2007-11-17 19:13:49.000000000 -0500
@@ -1067,28 +1067,25 @@
 }
 #endif
 
-int pcie_init_hardware(struct controller *ctrl, struct pcie_device *dev)
+static int pcie_init_hardware_part1(struct controller *ctrl,
+				    struct pcie_device *dev)
 {
 	int rc;
 	u16 temp_word;
-	u16 intr_enable = 0;
 	u32 slot_cap;
 	u16 slot_status;
-	struct pci_dev *pdev;
-
-	pdev = dev->port;
 
 	rc = pciehp_readl(ctrl, SLOTCAP, &slot_cap);
 	if (rc) {
 		err("%s: Cannot read SLOTCAP register\n", __FUNCTION__);
-		goto abort_free_ctlr;
+		return -1;
 	}
 
 	/* Mask Hot-plug Interrupt Enable */
 	rc = pciehp_readw(ctrl, SLOTCTRL, &temp_word);
 	if (rc) {
 		err("%s: Cannot read SLOTCTRL register\n", __FUNCTION__);
-		goto abort_free_ctlr;
+		return -1;
 	}
 
 	dbg("%s: SLOTCTRL %x value read %x\n",
@@ -1099,62 +1096,46 @@
 	rc = pciehp_writew(ctrl, SLOTCTRL, temp_word);
 	if (rc) {
 		err("%s: Cannot write to SLOTCTRL register\n", __FUNCTION__);
-		goto abort_free_ctlr;
+		return -1;
 	}
 
 	rc = pciehp_readw(ctrl, SLOTSTATUS, &slot_status);
 	if (rc) {
 		err("%s: Cannot read SLOTSTATUS register\n", __FUNCTION__);
-		goto abort_free_ctlr;
+		return -1;
 	}
 
 	temp_word = 0x1F; /* Clear all events */
 	rc = pciehp_writew(ctrl, SLOTSTATUS, temp_word);
 	if (rc) {
 		err("%s: Cannot write to SLOTSTATUS register\n", __FUNCTION__);
-		goto abort_free_ctlr;
+		return -1;
 	}
+	return 0;
+}
 
-	if (pciehp_poll_mode) {
-		/* Install interrupt polling timer. Start with 10 sec delay */
-		init_timer(&ctrl->poll_timer);
-		start_int_poll_timer(ctrl, 10);
-	} else {
-		/* Installs the interrupt handler */
-		rc = request_irq(ctrl->pci_dev->irq, pcie_isr, IRQF_SHARED,
-				 MY_NAME, (void *)ctrl);
-		dbg("%s: request_irq %d for hpc%d (returns %d)\n",
-		    __FUNCTION__, ctrl->pci_dev->irq,
-		    atomic_read(&pciehp_num_controllers), rc);
-		if (rc) {
-			err("Can't get irq %d for the hotplug controller\n",
-			    ctrl->pci_dev->irq);
-			goto abort_free_ctlr;
-		}
-	}
-	dbg("pciehp ctrl b:d:f:irq=0x%x:%x:%x:%x\n", pdev->bus->number,
-		PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn), dev->irq);
-
-	/*
-	 * If this is the first controller to be initialized,
-	 * initialize the pciehp work queue
-	 */
-	if (atomic_add_return(1, &pciehp_num_controllers) == 1) {
-		pciehp_wq = create_singlethread_workqueue("pciehpd");
-		if (!pciehp_wq) {
-			rc = -ENOMEM;
-			goto abort_free_irq;
-		}
-	}
+int pcie_init_hardware_part2(struct controller *ctrl, struct pcie_device *dev)
+{
+	int rc;
+	u16 temp_word;
+	u16 intr_enable = 0;
+	u32 slot_cap;
+	u16 slot_status;
 
 	rc = pciehp_readw(ctrl, SLOTCTRL, &temp_word);
 	if (rc) {
 		err("%s: Cannot read SLOTCTRL register\n", __FUNCTION__);
-		goto abort_free_irq;
+		goto abort;
 	}
 
 	intr_enable = intr_enable | PRSN_DETECT_ENABLE;
 
+	rc = pciehp_readl(ctrl, SLOTCAP, &slot_cap);
+	if (rc) {
+		err("%s: Cannot read SLOTCAP register\n", __FUNCTION__);
+		goto abort;
+	}
+
 	if (ATTN_BUTTN(slot_cap))
 		intr_enable = intr_enable | ATTN_BUTTN_ENABLE;
 
@@ -1179,7 +1160,7 @@
 	rc = pciehp_writew(ctrl, SLOTCTRL, temp_word);
 	if (rc) {
 		err("%s: Cannot write to SLOTCTRL register\n", __FUNCTION__);
-		goto abort_free_irq;
+		goto abort;
 	}
 	rc = pciehp_readw(ctrl, SLOTSTATUS, &slot_status);
 	if (rc) {
@@ -1214,14 +1195,7 @@
 	}
 	if (rc)
 		err("%s : disabling interrupts failed\n", __FUNCTION__);
-
-abort_free_irq:
-	if (pciehp_poll_mode)
-		del_timer_sync(&ctrl->poll_timer);
-	else
-		free_irq(ctrl->pci_dev->irq, ctrl);
-
-abort_free_ctlr:
+abort:
 	return -1;
 }
 
@@ -1318,11 +1292,52 @@
 	ctrl->first_slot = slot_cap >> 19;
 	ctrl->ctrlcap = slot_cap & 0x0000007f;
 
-	rc = pcie_init_hardware(ctrl, dev);
+	rc = pcie_init_hardware_part1(ctrl, dev);
+	if (rc)
+		goto abort;
+
+	if (pciehp_poll_mode) {
+		/* Install interrupt polling timer. Start with 10 sec delay */
+		init_timer(&ctrl->poll_timer);
+		start_int_poll_timer(ctrl, 10);
+	} else {
+		/* Installs the interrupt handler */
+		rc = request_irq(ctrl->pci_dev->irq, pcie_isr, IRQF_SHARED,
+				 MY_NAME, (void *)ctrl);
+		dbg("%s: request_irq %d for hpc%d (returns %d)\n",
+		    __FUNCTION__, ctrl->pci_dev->irq,
+		    atomic_read(&pciehp_num_controllers), rc);
+		if (rc) {
+			err("Can't get irq %d for the hotplug controller\n",
+			    ctrl->pci_dev->irq);
+			goto abort;
+		}
+	}
+	dbg("pciehp ctrl b:d:f:irq=0x%x:%x:%x:%x\n", pdev->bus->number,
+		PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn), dev->irq);
+
+	/*
+	 * If this is the first controller to be initialized,
+	 * initialize the pciehp work queue
+	 */
+	if (atomic_add_return(1, &pciehp_num_controllers) == 1) {
+		pciehp_wq = create_singlethread_workqueue("pciehpd");
+		if (!pciehp_wq) {
+			rc = -ENOMEM;
+			goto abort_free_irq;
+		}
+	}
+
+	rc = pcie_init_hardware_part2(ctrl, dev);
 	if (rc == 0) {
 		ctrl->hpc_ops = &pciehp_hpc_ops;
 		return 0;
 	}
+abort_free_irq:
+	if (pciehp_poll_mode)
+		del_timer_sync(&ctrl->poll_timer);
+	else
+		free_irq(ctrl->pci_dev->irq, ctrl);
 abort:
 	return -1;
 }

  parent reply	other threads:[~2007-11-18  0:27 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-10-18  2:57 [PATCH 0/3] Fix two PEIe hotplug issues Mark Lord
2007-10-18  2:59 ` [PATCH 1/3] pciehp_handle_preinserted_card Mark Lord
2007-10-18  3:00 ` [PATCH 2/3] pciehp_split_pcie_init Mark Lord
2007-10-18  3:02 ` [PATCH 3/3] pciehp_resume_reinit_hardware Mark Lord
2007-10-18  3:03 ` [PATCH 1/3] pciehp_handle_preinserted_card Mark Lord
2007-10-18  3:04 ` [PATCH 2/3] pciehp_split_pcie_init Mark Lord
2007-10-18  3:05 ` [PATCH 0/3] Fix two PEIe hotplug issues Mark Lord
2007-10-18  3:09 ` Mark Lord
2007-10-18 16:13   ` Kristen Carlson Accardi
2007-10-18 17:06     ` Mark Lord
2007-10-18 17:06       ` Kristen Carlson Accardi
2007-10-18 17:49         ` Theodore Tso
2007-10-18 17:56           ` Kristen Carlson Accardi
2007-10-18 21:11             ` Mark Lord
2007-10-18 21:26           ` Mark Lord
2007-10-18  3:32 ` [PATCH 1/3] pciehp: hotplug: deal with pre-inserted ExpressCards Mark Lord
2007-10-18  3:33   ` [PATCH 2/3] pciehp: hotplug: split out hardware init from pcie_init() Mark Lord
2007-10-18  3:34     ` [PATCH 3/3] pciehp: hotplug: reinit hotplug h/w on resume from suspend Mark Lord
2007-10-18 11:15   ` [Pcihpd-discuss] [PATCH 1/3] pciehp: hotplug: deal with pre-inserted ExpressCards Kenji Kaneshige
2007-10-18 13:31     ` Mark Lord
2007-10-19  2:38       ` Kenji Kaneshige
2007-10-19  3:09         ` Mark Lord
2007-10-19  3:27           ` Kenji Kaneshige
2007-11-18  0:27 ` Mark Lord [this message]
2007-11-18 12:23   ` [PATCH] Fix PCIe double initialization bug Rafael J. Wysocki
2007-11-18 14:20     ` Mark Lord
2007-11-18 14:37       ` Mark Lord

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=473F86E7.4020003@rtr.ca \
    --to=lkml@rtr.ca \
    --cc=akpm@linux-foundation.org \
    --cc=greg@kroah.com \
    --cc=kristen.c.accardi@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pcihpd-discuss@lists.sourceforge.net \
    --cc=tytso@mit.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.