PCI: pciehp: Fix hot-add vs powerfault detection order
authorKeith Busch <keith.busch@intel.com>
Wed, 5 Sep 2018 20:35:41 +0000 (14:35 -0600)
committerBjorn Helgaas <bhelgaas@google.com>
Tue, 11 Sep 2018 13:47:42 +0000 (08:47 -0500)
If both hot-add and power fault were observed in a single interrupt, we
handled the hot-add first, then the power fault, in this path:

  pciehp_ist
    if (events & (PDC | DLLSC))
      pciehp_handle_presence_or_link_change
        case OFF_STATE:
          pciehp_enable_slot
            __pciehp_enable_slot
              board_added
                pciehp_power_on_slot
                  ctrl->power_fault_detected = 0
                  pcie_write_cmd(ctrl, PCI_EXP_SLTCTL_PWR_ON, PCI_EXP_SLTCTL_PCC)
                pciehp_green_led_on(p_slot)             # power LED on
pciehp_set_attention_status(p_slot, 0)  # attention LED off
    if ((events & PFD) && !ctrl->power_fault_detected)
      ctrl->power_fault_detected = 1
      pciehp_set_attention_status(1)                    # attention LED on
      pciehp_green_led_off(slot)                        # power LED off

This left the attention indicator on (even though the hot-add succeeded)
and the power indicator off (even though the slot power was on).

Fix this by checking for power faults before checking for new devices.

Prior to 0e94916e6091, this was successful because everything was chained
through work queues and the order was:

  INT_PRESENCE_ON -> INT_POWER_FAULT -> ENABLE_REQ

The ENABLE_REQ cleared the power fault at the end, but now everything is
handled inline with the interrupt thread, such that the work ENABLE_REQ was
doing happens before power fault handling now.

Fixes: 0e94916e6091 ("PCI: pciehp: Handle events synchronously")
Signed-off-by: Keith Busch <keith.busch@intel.com>
[bhelgaas: changelog]
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Reviewed-by: Lukas Wunner <lukas@wunner.de>
drivers/pci/hotplug/pciehp_hpc.c

index 7136e34309250aa2a3cc0d2880f563a3b183e895..a938abdb41ceeb575dc62af740d78d278104280c 100644 (file)
@@ -496,7 +496,7 @@ int pciehp_power_on_slot(struct slot *slot)
        u16 slot_status;
        int retval;
 
-       /* Clear sticky power-fault bit from previous power failures */
+       /* Clear power-fault bit from previous power failures */
        pcie_capability_read_word(pdev, PCI_EXP_SLTSTA, &slot_status);
        if (slot_status & PCI_EXP_SLTSTA_PFD)
                pcie_capability_write_word(pdev, PCI_EXP_SLTSTA,
@@ -646,6 +646,14 @@ static irqreturn_t pciehp_ist(int irq, void *dev_id)
                pciehp_handle_button_press(slot);
        }
 
+       /* Check Power Fault Detected */
+       if ((events & PCI_EXP_SLTSTA_PFD) && !ctrl->power_fault_detected) {
+               ctrl->power_fault_detected = 1;
+               ctrl_err(ctrl, "Slot(%s): Power fault\n", slot_name(slot));
+               pciehp_set_attention_status(slot, 1);
+               pciehp_green_led_off(slot);
+       }
+
        /*
         * Disable requests have higher priority than Presence Detect Changed
         * or Data Link Layer State Changed events.
@@ -657,14 +665,6 @@ static irqreturn_t pciehp_ist(int irq, void *dev_id)
                pciehp_handle_presence_or_link_change(slot, events);
        up_read(&ctrl->reset_lock);
 
-       /* Check Power Fault Detected */
-       if ((events & PCI_EXP_SLTSTA_PFD) && !ctrl->power_fault_detected) {
-               ctrl->power_fault_detected = 1;
-               ctrl_err(ctrl, "Slot(%s): Power fault\n", slot_name(slot));
-               pciehp_set_attention_status(slot, 1);
-               pciehp_green_led_off(slot);
-       }
-
        pci_config_pm_runtime_put(pdev);
        wake_up(&ctrl->requester);
        return IRQ_HANDLED;