From 2cc56f3028091578dcf0093cd00f1fee7a21515d Mon Sep 17 00:00:00 2001 From: Bjorn Helgaas Date: Sat, 14 Jun 2014 10:56:31 -0600 Subject: PCI: pciehp: Remove assumptions about which commands cause completion events We use incorrect logic to decide whether a PCIe hotplug controller generates command completion events. 5808639bfa98 ("pciehp: fix slow probing") assumed that the Slot Status "Command Completed" bit was set only for commands affecting slot power, indicators, or electromechanical interlock. That assumption is false: per sec. 6.7.3.2 of PCIe spec r3.0, a write targeting any portion of the Slot Control register is a command, and (if command completed events are supported) software must wait for a command to complete before issuing the next command. 5808639bfa98 was to fix boot-time timeouts (see bugzilla below) on a Lenovo Thinkpad R61 with an Intel hotplug controller. The controller probably has the Intel CF118 erratum, which means it doesn't report Command Completed unless the Slot Control power, indicator, or interlock bits are changed. This causes a timeout because pciehp always waits for Command Complete (if supported), regardless of which bits are changed. Remove the incorrect logic because the timeouts have been addressed differently by these changes: PCI: pciehp: Wait for hotplug command completion lazily PCI: pciehp: Compute timeout from hotplug command start time Link: https://bugzilla.kernel.org/show_bug.cgi?id=10751 Tested-by: Rajat Jain (IDT 807a controller) Signed-off-by: Bjorn Helgaas Acked-by: Yinghai Lu --- drivers/pci/hotplug/pciehp_hpc.c | 35 +++++------------------------------ 1 file changed, 5 insertions(+), 30 deletions(-) (limited to 'drivers') diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c index 720dfe5fc48a..a3a5c65def1c 100644 --- a/drivers/pci/hotplug/pciehp_hpc.c +++ b/drivers/pci/hotplug/pciehp_hpc.c @@ -185,7 +185,6 @@ static void pcie_wait_cmd(struct controller *ctrl) static void pcie_write_cmd(struct controller *ctrl, u16 cmd, u16 mask) { struct pci_dev *pdev = ctrl_dev(ctrl); - u16 slot_status; u16 slot_ctrl; mutex_lock(&ctrl->ctrl_lock); @@ -193,30 +192,6 @@ static void pcie_write_cmd(struct controller *ctrl, u16 cmd, u16 mask) /* Wait for any previous command that might still be in progress */ pcie_wait_cmd(ctrl); - pcie_capability_read_word(pdev, PCI_EXP_SLTSTA, &slot_status); - if (slot_status & PCI_EXP_SLTSTA_CC) { - pcie_capability_write_word(pdev, PCI_EXP_SLTSTA, - PCI_EXP_SLTSTA_CC); - if (!ctrl->no_cmd_complete) { - /* - * After 1 sec and CMD_COMPLETED still not set, just - * proceed forward to issue the next command according - * to spec. Just print out the error message. - */ - ctrl_dbg(ctrl, "CMD_COMPLETED not clear after 1 sec\n"); - } else if (!NO_CMD_CMPL(ctrl)) { - /* - * This controller seems to notify of command completed - * event even though it supports none of power - * controller, attention led, power led and EMI. - */ - ctrl_dbg(ctrl, "Unexpected CMD_COMPLETED. Need to wait for command completed event\n"); - ctrl->no_cmd_complete = 0; - } else { - ctrl_dbg(ctrl, "Unexpected CMD_COMPLETED. Maybe the controller is broken\n"); - } - } - pcie_capability_read_word(pdev, PCI_EXP_SLTCTL, &slot_ctrl); slot_ctrl &= ~mask; slot_ctrl |= (cmd & mask); @@ -796,14 +771,14 @@ struct controller *pcie_init(struct pcie_device *dev) mutex_init(&ctrl->ctrl_lock); init_waitqueue_head(&ctrl->queue); dbg_ctrl(ctrl); + /* * Controller doesn't notify of command completion if the "No - * Command Completed Support" bit is set in Slot Capability - * register or the controller supports none of power - * controller, attention led, power led and EMI. + * Command Completed Support" bit is set in Slot Capabilities. + * If set, it means the controller can accept hotplug commands + * with no delay between them. */ - if (NO_CMD_CMPL(ctrl) || - !(POWER_CTRL(ctrl) | ATTN_LED(ctrl) | PWR_LED(ctrl) | EMI(ctrl))) + if (NO_CMD_CMPL(ctrl)) ctrl->no_cmd_complete = 1; /* Check if Data Link Layer Link Active Reporting is implemented */ -- cgit v1.2.3-58-ga151