From: Alex Williamson <alex.williamson@redhat.com>
To: bhelgaas@google.com, linux-pci@vger.kernel.org
Cc: linux-kernel@vger.kernel.org, mstowe@redhat.com
Subject: [PATCH] PCI: pciehp: Wait for hotplug command completion where necessary
Date: Mon, 08 Jun 2015 17:10:50 -0600 [thread overview]
Message-ID: <20150608230833.18712.45528.stgit@gimli.home> (raw)
The commit referenced below deferred waiting for command completion
until the start of the next command, allowing hardware to do the
latching asynchronously. Unfortunately, being ready to accept a new
command is the only indication we have that the previous command is
completed. In cases where we need that state change to be enabled, we
must still wait for completion. For instance, pciehp_reset_slot()
attempts to disable anything that might generate a surprise hotplug on
slots that support presence detection. If we don't wait for those
settings to latch before the secondary bus reset, we negate any value
in attempting to prevent the spurious hotplug.
Create a base function with optional wait and helper functions so that
pcie_write_cmd() turns back into the "safe" interface which waits
before and after issuing a command and add pcie_write_cmd_nowait(),
which eliminates the trailing wait for asynchronous completion. The
following functions are returned to their previous behavior:
pciehp_power_on_slot
pciehp_power_off_slot
pcie_disable_notification
pciehp_reset_slot
The rationale is that pciehp_power_on_slot() enables the link and
therefore relies on completion of power-on. pciehp_power_off_slot()
and pcie_disable_notification() need a wait because data structures
may be freed after these calls and continued signaling from the device
would be unexpected. And, of course, pciehp_reset_slot() needs to
wait for the scenario outlined above.
Fixes: 3461a068661c ("PCI: pciehp: Wait for hotplug command completion lazily")
Cc: stable@vger.kernel.org # v3.17+
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
drivers/pci/hotplug/pciehp_hpc.c | 52 ++++++++++++++++++++++++++++----------
1 file changed, 38 insertions(+), 14 deletions(-)
diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c
index 0ebf754..6d68688 100644
--- a/drivers/pci/hotplug/pciehp_hpc.c
+++ b/drivers/pci/hotplug/pciehp_hpc.c
@@ -176,20 +176,17 @@ static void pcie_wait_cmd(struct controller *ctrl)
jiffies_to_msecs(jiffies - ctrl->cmd_started));
}
-/**
- * pcie_write_cmd - Issue controller command
- * @ctrl: controller to which the command is issued
- * @cmd: command value written to slot control register
- * @mask: bitmask of slot control register to be modified
- */
-static void pcie_write_cmd(struct controller *ctrl, u16 cmd, u16 mask)
+static void pcie_do_write_cmd(struct controller *ctrl, u16 cmd,
+ u16 mask, bool wait)
{
struct pci_dev *pdev = ctrl_dev(ctrl);
u16 slot_ctrl;
mutex_lock(&ctrl->ctrl_lock);
- /* Wait for any previous command that might still be in progress */
+ /*
+ * Always wait for any previous command that might still be in progress
+ */
pcie_wait_cmd(ctrl);
pcie_capability_read_word(pdev, PCI_EXP_SLTCTL, &slot_ctrl);
@@ -201,9 +198,33 @@ static void pcie_write_cmd(struct controller *ctrl, u16 cmd, u16 mask)
ctrl->cmd_started = jiffies;
ctrl->slot_ctrl = slot_ctrl;
+ /*
+ * Optionally wait for the hardware to be ready for a new command,
+ * indicating completion of the above issued command.
+ */
+ if (wait)
+ pcie_wait_cmd(ctrl);
+
mutex_unlock(&ctrl->ctrl_lock);
}
+/**
+ * pcie_write_cmd - Issue controller command
+ * @ctrl: controller to which the command is issued
+ * @cmd: command value written to slot control register
+ * @mask: bitmask of slot control register to be modified
+ */
+static void pcie_write_cmd(struct controller *ctrl, u16 cmd, u16 mask)
+{
+ pcie_do_write_cmd(ctrl, cmd, mask, true);
+}
+
+/* Same as above without waiting for the hardware to latch */
+static void pcie_write_cmd_nowait(struct controller *ctrl, u16 cmd, u16 mask)
+{
+ pcie_do_write_cmd(ctrl, cmd, mask, false);
+}
+
bool pciehp_check_link_active(struct controller *ctrl)
{
struct pci_dev *pdev = ctrl_dev(ctrl);
@@ -422,7 +443,7 @@ void pciehp_set_attention_status(struct slot *slot, u8 value)
default:
return;
}
- pcie_write_cmd(ctrl, slot_cmd, PCI_EXP_SLTCTL_AIC);
+ pcie_write_cmd_nowait(ctrl, slot_cmd, PCI_EXP_SLTCTL_AIC);
ctrl_dbg(ctrl, "%s: SLOTCTRL %x write cmd %x\n", __func__,
pci_pcie_cap(ctrl->pcie->port) + PCI_EXP_SLTCTL, slot_cmd);
}
@@ -434,7 +455,8 @@ void pciehp_green_led_on(struct slot *slot)
if (!PWR_LED(ctrl))
return;
- pcie_write_cmd(ctrl, PCI_EXP_SLTCTL_PWR_IND_ON, PCI_EXP_SLTCTL_PIC);
+ pcie_write_cmd_nowait(ctrl, PCI_EXP_SLTCTL_PWR_IND_ON,
+ PCI_EXP_SLTCTL_PIC);
ctrl_dbg(ctrl, "%s: SLOTCTRL %x write cmd %x\n", __func__,
pci_pcie_cap(ctrl->pcie->port) + PCI_EXP_SLTCTL,
PCI_EXP_SLTCTL_PWR_IND_ON);
@@ -447,7 +469,8 @@ void pciehp_green_led_off(struct slot *slot)
if (!PWR_LED(ctrl))
return;
- pcie_write_cmd(ctrl, PCI_EXP_SLTCTL_PWR_IND_OFF, PCI_EXP_SLTCTL_PIC);
+ pcie_write_cmd_nowait(ctrl, PCI_EXP_SLTCTL_PWR_IND_OFF,
+ PCI_EXP_SLTCTL_PIC);
ctrl_dbg(ctrl, "%s: SLOTCTRL %x write cmd %x\n", __func__,
pci_pcie_cap(ctrl->pcie->port) + PCI_EXP_SLTCTL,
PCI_EXP_SLTCTL_PWR_IND_OFF);
@@ -460,7 +483,8 @@ void pciehp_green_led_blink(struct slot *slot)
if (!PWR_LED(ctrl))
return;
- pcie_write_cmd(ctrl, PCI_EXP_SLTCTL_PWR_IND_BLINK, PCI_EXP_SLTCTL_PIC);
+ pcie_write_cmd_nowait(ctrl, PCI_EXP_SLTCTL_PWR_IND_BLINK,
+ PCI_EXP_SLTCTL_PIC);
ctrl_dbg(ctrl, "%s: SLOTCTRL %x write cmd %x\n", __func__,
pci_pcie_cap(ctrl->pcie->port) + PCI_EXP_SLTCTL,
PCI_EXP_SLTCTL_PWR_IND_BLINK);
@@ -613,7 +637,7 @@ void pcie_enable_notification(struct controller *ctrl)
PCI_EXP_SLTCTL_HPIE | PCI_EXP_SLTCTL_CCIE |
PCI_EXP_SLTCTL_DLLSCE);
- pcie_write_cmd(ctrl, cmd, mask);
+ pcie_write_cmd_nowait(ctrl, cmd, mask);
ctrl_dbg(ctrl, "%s: SLOTCTRL %x write cmd %x\n", __func__,
pci_pcie_cap(ctrl->pcie->port) + PCI_EXP_SLTCTL, cmd);
}
@@ -664,7 +688,7 @@ int pciehp_reset_slot(struct slot *slot, int probe)
pci_reset_bridge_secondary_bus(ctrl->pcie->port);
pcie_capability_write_word(pdev, PCI_EXP_SLTSTA, stat_mask);
- pcie_write_cmd(ctrl, ctrl_mask, ctrl_mask);
+ pcie_write_cmd_nowait(ctrl, ctrl_mask, ctrl_mask);
ctrl_dbg(ctrl, "%s: SLOTCTRL %x write cmd %x\n", __func__,
pci_pcie_cap(ctrl->pcie->port) + PCI_EXP_SLTCTL, ctrl_mask);
if (pciehp_poll_mode)
next reply other threads:[~2015-06-08 23:10 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-06-08 23:10 Alex Williamson [this message]
2015-06-09 15:49 ` [PATCH] PCI: pciehp: Wait for hotplug command completion where necessary Bjorn Helgaas
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=20150608230833.18712.45528.stgit@gimli.home \
--to=alex.williamson@redhat.com \
--cc=bhelgaas@google.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=mstowe@redhat.com \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).