[PATCH] PCI: pciehp: Wait for hotplug command completion where necessary

From: Alex Williamson
Date: Mon Jun 08 2015 - 19:11:01 EST


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@xxxxxxxxxxxxxxx # v3.17+
Signed-off-by: Alex Williamson <alex.williamson@xxxxxxxxxx>
---
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)

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/