From 8b7c8b46f111ae56df4fd196fcb0fd2495f3b966 Mon Sep 17 00:00:00 2001 From: Keith Busch Date: Wed, 3 Aug 2016 17:29:45 -0600 Subject: PCI: pciehp: Clear attention LED on device add Clear the LED attention status after a successful device add. It is possible the attention LED was on from a previous power fault or link failure, and a subsequent successful device insert insertion should clear it. Signed-off-by: Keith Busch Signed-off-by: Bjorn Helgaas --- drivers/pci/hotplug/pciehp_ctrl.c | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers/pci/hotplug') diff --git a/drivers/pci/hotplug/pciehp_ctrl.c b/drivers/pci/hotplug/pciehp_ctrl.c index 880978b..7ea3e61 100644 --- a/drivers/pci/hotplug/pciehp_ctrl.c +++ b/drivers/pci/hotplug/pciehp_ctrl.c @@ -120,6 +120,7 @@ static int board_added(struct slot *p_slot) } pciehp_green_led_on(p_slot); + pciehp_set_attention_status(p_slot, 0); return 0; err_exit: -- cgit v1.1 From a8499f20d30e0f9632017fd27a8f1d8c146a6a33 Mon Sep 17 00:00:00 2001 From: Bjorn Helgaas Date: Thu, 8 Sep 2016 17:30:38 -0500 Subject: PCI: pciehp: Rename pcie_isr() locals for clarity Rename "detected" and "intr_loc" to "status" and "events" for clarity. "status" is the value we read from the Slot Status register; "events" is the set of hot-plug events we need to process. No functional change intended. Tested-by: Lukas Wunner Signed-off-by: Bjorn Helgaas Reviewed-by: Mika Westerberg --- drivers/pci/hotplug/pciehp_hpc.c | 46 ++++++++++++++++++++++------------------ 1 file changed, 25 insertions(+), 21 deletions(-) (limited to 'drivers/pci/hotplug') diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c index 08e84d6..264df36 100644 --- a/drivers/pci/hotplug/pciehp_hpc.c +++ b/drivers/pci/hotplug/pciehp_hpc.c @@ -542,7 +542,7 @@ static irqreturn_t pcie_isr(int irq, void *dev_id) struct pci_bus *subordinate = pdev->subordinate; struct pci_dev *dev; struct slot *slot = ctrl->slot; - u16 detected, intr_loc; + u16 status, events; u8 present; bool link; @@ -555,31 +555,35 @@ static irqreturn_t pcie_isr(int irq, void *dev_id) * serviced, we need to re-inspect Slot Status register after * clearing what is presumed to be the last pending interrupt. */ - intr_loc = 0; + events = 0; do { - pcie_capability_read_word(pdev, PCI_EXP_SLTSTA, &detected); - if (detected == (u16) ~0) { + pcie_capability_read_word(pdev, PCI_EXP_SLTSTA, &status); + if (status == (u16) ~0) { ctrl_info(ctrl, "%s: no response from device\n", __func__); return IRQ_HANDLED; } - detected &= (PCI_EXP_SLTSTA_ABP | PCI_EXP_SLTSTA_PFD | - PCI_EXP_SLTSTA_PDC | - PCI_EXP_SLTSTA_CC | PCI_EXP_SLTSTA_DLLSC); - detected &= ~intr_loc; - intr_loc |= detected; - if (!intr_loc) + /* + * Slot Status contains plain status bits as well as event + * notification bits; right now we only want the event bits. + */ + status &= (PCI_EXP_SLTSTA_ABP | PCI_EXP_SLTSTA_PFD | + PCI_EXP_SLTSTA_PDC | PCI_EXP_SLTSTA_CC | + PCI_EXP_SLTSTA_DLLSC); + status &= ~events; + events |= status; + if (!events) return IRQ_NONE; - if (detected) + if (status) pcie_capability_write_word(pdev, PCI_EXP_SLTSTA, - intr_loc); - } while (detected); + events); + } while (status); - ctrl_dbg(ctrl, "pending interrupts %#06x from Slot Status\n", intr_loc); + ctrl_dbg(ctrl, "pending interrupts %#06x from Slot Status\n", events); /* Check Command Complete Interrupt Pending */ - if (intr_loc & PCI_EXP_SLTSTA_CC) { + if (events & PCI_EXP_SLTSTA_CC) { ctrl->cmd_busy = 0; smp_mb(); wake_up(&ctrl->queue); @@ -589,24 +593,24 @@ static irqreturn_t pcie_isr(int irq, void *dev_id) list_for_each_entry(dev, &subordinate->devices, bus_list) { if (dev->ignore_hotplug) { ctrl_dbg(ctrl, "ignoring hotplug event %#06x (%s requested no hotplug)\n", - intr_loc, pci_name(dev)); + events, pci_name(dev)); return IRQ_HANDLED; } } } - if (!(intr_loc & ~PCI_EXP_SLTSTA_CC)) + if (!(events & ~PCI_EXP_SLTSTA_CC)) return IRQ_HANDLED; /* Check Attention Button Pressed */ - if (intr_loc & PCI_EXP_SLTSTA_ABP) { + if (events & PCI_EXP_SLTSTA_ABP) { ctrl_info(ctrl, "Button pressed on Slot(%s)\n", slot_name(slot)); pciehp_queue_interrupt_event(slot, INT_BUTTON_PRESS); } /* Check Presence Detect Changed */ - if (intr_loc & PCI_EXP_SLTSTA_PDC) { + if (events & PCI_EXP_SLTSTA_PDC) { pciehp_get_adapter_status(slot, &present); ctrl_info(ctrl, "Card %spresent on Slot(%s)\n", present ? "" : "not ", slot_name(slot)); @@ -615,13 +619,13 @@ static irqreturn_t pcie_isr(int irq, void *dev_id) } /* Check Power Fault Detected */ - if ((intr_loc & PCI_EXP_SLTSTA_PFD) && !ctrl->power_fault_detected) { + if ((events & PCI_EXP_SLTSTA_PFD) && !ctrl->power_fault_detected) { ctrl->power_fault_detected = 1; ctrl_err(ctrl, "Power fault on slot %s\n", slot_name(slot)); pciehp_queue_interrupt_event(slot, INT_POWER_FAULT); } - if (intr_loc & PCI_EXP_SLTSTA_DLLSC) { + if (events & PCI_EXP_SLTSTA_DLLSC) { link = pciehp_check_link_active(ctrl); ctrl_info(ctrl, "slot(%s): Link %s event\n", slot_name(slot), link ? "Up" : "Down"); -- cgit v1.1 From 70e8b40176c75d3544024e7c934720b11a8a11bf Mon Sep 17 00:00:00 2001 From: Bjorn Helgaas Date: Thu, 8 Sep 2016 16:43:40 -0500 Subject: PCI: pciehp: Return IRQ_NONE when we can't read interrupt status After 1469d17dd341 ("PCI: pciehp: Handle invalid data when reading from non-existent devices"), we returned IRQ_HANDLED when we failed to read interrupt status from the bridge. I think it's better to return IRQ_NONE, as we do in other cases where there's no interrupt pending. This will facilitate refactoring the loop in pcie_isr(): we'll be able to call the ISR in a loop as long as it returns IRQ_HANDLED. Return IRQ_NONE if we couldn't read interrupt status. Tested-by: Lukas Wunner Signed-off-by: Bjorn Helgaas Reviewed-by: Mika Westerberg --- drivers/pci/hotplug/pciehp_hpc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/pci/hotplug') diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c index 264df36..b8efe1b 100644 --- a/drivers/pci/hotplug/pciehp_hpc.c +++ b/drivers/pci/hotplug/pciehp_hpc.c @@ -561,7 +561,7 @@ static irqreturn_t pcie_isr(int irq, void *dev_id) if (status == (u16) ~0) { ctrl_info(ctrl, "%s: no response from device\n", __func__); - return IRQ_HANDLED; + return IRQ_NONE; } /* -- cgit v1.1 From fad214b0aa726ee21adcac4308d388efcb89d6bd Mon Sep 17 00:00:00 2001 From: Mayurkumar Patel Date: Thu, 8 Sep 2016 15:07:56 -0500 Subject: PCI: pciehp: Process all hotplug events before looking for new ones Previously we accumulated hotplug events, then processed them, essentially like this: events = 0 do { status = read(Slot Status) status &= EVENT_MASK # only look at events events |= status # accumulate events write(Slot Status, events) # clear events } while (status) process events The problem is that as soon as we clear events in Slot Status, the hardware may send notifications for new events, and we lose information about the first events. For example, we might see two Presence Detect Changed events, but lose the fact that the slot was temporarily empty: read PCI_EXP_SLTSTA_PDC set, PCI_EXP_SLTSTA_PDS clear # slot empty write PCI_EXP_SLTSTA_PDC # clear PDC event read PCI_EXP_SLTSTA_PDC set, PCI_EXP_SLTSTA_PDS set # slot occupied The current code does not process a removal; it only processes the insertion, which fails because we didn't remove the original device. To avoid this problem, read Slot Status once and process all the events before reading it again, like this: do { read events clear events process events } while (events) [bhelgaas: changelog, add external loop around pciehp_isr()] Tested-by: Lukas Wunner Signed-off-by: Mayurkumar Patel Signed-off-by: Bjorn Helgaas Reviewed-by: Mika Westerberg --- drivers/pci/hotplug/pciehp_hpc.c | 58 ++++++++++++++++++++++------------------ 1 file changed, 32 insertions(+), 26 deletions(-) (limited to 'drivers/pci/hotplug') diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c index b8efe1b..625fa6a 100644 --- a/drivers/pci/hotplug/pciehp_hpc.c +++ b/drivers/pci/hotplug/pciehp_hpc.c @@ -535,7 +535,7 @@ void pciehp_power_off_slot(struct slot *slot) PCI_EXP_SLTCTL_PWR_OFF); } -static irqreturn_t pcie_isr(int irq, void *dev_id) +static irqreturn_t pciehp_isr(int irq, void *dev_id) { struct controller *ctrl = (struct controller *)dev_id; struct pci_dev *pdev = ctrl_dev(ctrl); @@ -550,36 +550,23 @@ static irqreturn_t pcie_isr(int irq, void *dev_id) if (pdev->current_state == PCI_D3cold) return IRQ_NONE; + pcie_capability_read_word(pdev, PCI_EXP_SLTSTA, &status); + if (status == (u16) ~0) { + ctrl_info(ctrl, "%s: no response from device\n", __func__); + return IRQ_NONE; + } + /* - * In order to guarantee that all interrupt events are - * serviced, we need to re-inspect Slot Status register after - * clearing what is presumed to be the last pending interrupt. + * Slot Status contains plain status bits as well as event + * notification bits; right now we only want the event bits. */ - events = 0; - do { - pcie_capability_read_word(pdev, PCI_EXP_SLTSTA, &status); - if (status == (u16) ~0) { - ctrl_info(ctrl, "%s: no response from device\n", - __func__); - return IRQ_NONE; - } - - /* - * Slot Status contains plain status bits as well as event - * notification bits; right now we only want the event bits. - */ - status &= (PCI_EXP_SLTSTA_ABP | PCI_EXP_SLTSTA_PFD | + events = status & (PCI_EXP_SLTSTA_ABP | PCI_EXP_SLTSTA_PFD | PCI_EXP_SLTSTA_PDC | PCI_EXP_SLTSTA_CC | PCI_EXP_SLTSTA_DLLSC); - status &= ~events; - events |= status; - if (!events) - return IRQ_NONE; - if (status) - pcie_capability_write_word(pdev, PCI_EXP_SLTSTA, - events); - } while (status); + if (!events) + return IRQ_NONE; + pcie_capability_write_word(pdev, PCI_EXP_SLTSTA, events); ctrl_dbg(ctrl, "pending interrupts %#06x from Slot Status\n", events); /* Check Command Complete Interrupt Pending */ @@ -636,6 +623,25 @@ static irqreturn_t pcie_isr(int irq, void *dev_id) return IRQ_HANDLED; } +static irqreturn_t pcie_isr(int irq, void *dev_id) +{ + irqreturn_t rc, handled = IRQ_NONE; + + /* + * To guarantee that all interrupt events are serviced, we need to + * re-inspect Slot Status register after clearing what is presumed + * to be the last pending interrupt. + */ + do { + rc = pciehp_isr(irq, dev_id); + if (rc == IRQ_HANDLED) + handled = IRQ_HANDLED; + } while (rc == IRQ_HANDLED); + + /* Return IRQ_HANDLED if we handled one or more events */ + return handled; +} + void pcie_enable_notification(struct controller *ctrl) { u16 cmd, mask; -- cgit v1.1 From 0c923d1da394b96727b813d1e64412b72f1dc580 Mon Sep 17 00:00:00 2001 From: Mayurkumar Patel Date: Fri, 9 Sep 2016 09:10:17 -0500 Subject: PCI: pciehp: Don't re-read Slot Status when queuing hotplug event Previously we read Slot Status to learn about hotplug events, then cleared the events, then re-read Slot Status to find out what happened. But Slot Status might have changed before the second read. Capture the Slot Status once before clearing the events. Also capture the Link Status if we had a link status change. [bhelgaas: changelog, split to separate patch] Tested-by: Lukas Wunner Signed-off-by: Mayurkumar Patel Signed-off-by: Bjorn Helgaas Reviewed-by: Mika Westerberg --- drivers/pci/hotplug/pciehp_hpc.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) (limited to 'drivers/pci/hotplug') diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c index 625fa6a..fe99b45 100644 --- a/drivers/pci/hotplug/pciehp_hpc.c +++ b/drivers/pci/hotplug/pciehp_hpc.c @@ -566,6 +566,10 @@ static irqreturn_t pciehp_isr(int irq, void *dev_id) if (!events) return IRQ_NONE; + /* Capture link status before clearing interrupts */ + if (events & PCI_EXP_SLTSTA_DLLSC) + link = pciehp_check_link_active(ctrl); + pcie_capability_write_word(pdev, PCI_EXP_SLTSTA, events); ctrl_dbg(ctrl, "pending interrupts %#06x from Slot Status\n", events); @@ -598,7 +602,7 @@ static irqreturn_t pciehp_isr(int irq, void *dev_id) /* Check Presence Detect Changed */ if (events & PCI_EXP_SLTSTA_PDC) { - pciehp_get_adapter_status(slot, &present); + present = !!(status & PCI_EXP_SLTSTA_PDS); ctrl_info(ctrl, "Card %spresent on Slot(%s)\n", present ? "" : "not ", slot_name(slot)); pciehp_queue_interrupt_event(slot, present ? INT_PRESENCE_ON : @@ -613,7 +617,6 @@ static irqreturn_t pciehp_isr(int irq, void *dev_id) } if (events & PCI_EXP_SLTSTA_DLLSC) { - link = pciehp_check_link_active(ctrl); ctrl_info(ctrl, "slot(%s): Link %s event\n", slot_name(slot), link ? "Up" : "Down"); pciehp_queue_interrupt_event(slot, link ? INT_LINK_UP : -- cgit v1.1 From 69bd3c5b28e7c988886efb3c9a7614a612eef45d Mon Sep 17 00:00:00 2001 From: Mayurkumar Patel Date: Tue, 23 Aug 2016 08:58:51 +0000 Subject: PCI: pciehp: Don't re-read Slot Status when handling surprise event Previously we read Slot Status when handling a surprise event. But Slot Status might have changed since we identified the event, and the event_type already tells us whether to enable or disable the slot, so there's no need to read it again. Remove handle_surprise_event() and queue the power work directly. [bhelgaas: changelog] Tested-by: Lukas Wunner Signed-off-by: Mayurkumar Patel Signed-off-by: Bjorn Helgaas Reviewed-by: Mika Westerberg Acked-by: Rajat Jain --- drivers/pci/hotplug/pciehp_ctrl.c | 18 ++---------------- 1 file changed, 2 insertions(+), 16 deletions(-) (limited to 'drivers/pci/hotplug') diff --git a/drivers/pci/hotplug/pciehp_ctrl.c b/drivers/pci/hotplug/pciehp_ctrl.c index 7ea3e61..a787684 100644 --- a/drivers/pci/hotplug/pciehp_ctrl.c +++ b/drivers/pci/hotplug/pciehp_ctrl.c @@ -302,20 +302,6 @@ static void handle_button_press_event(struct slot *p_slot) /* * Note: This function must be called with slot->lock held */ -static void handle_surprise_event(struct slot *p_slot) -{ - u8 getstatus; - - pciehp_get_adapter_status(p_slot, &getstatus); - if (!getstatus) - pciehp_queue_power_work(p_slot, DISABLE_REQ); - else - pciehp_queue_power_work(p_slot, ENABLE_REQ); -} - -/* - * Note: This function must be called with slot->lock held - */ static void handle_link_event(struct slot *p_slot, u32 event) { struct controller *ctrl = p_slot->ctrl; @@ -378,14 +364,14 @@ static void interrupt_event_handler(struct work_struct *work) pciehp_green_led_off(p_slot); break; case INT_PRESENCE_ON: - handle_surprise_event(p_slot); + pciehp_queue_power_work(p_slot, ENABLE_REQ); break; case INT_PRESENCE_OFF: /* * Regardless of surprise capability, we need to * definitely remove a card that has been pulled out! */ - handle_surprise_event(p_slot); + pciehp_queue_power_work(p_slot, DISABLE_REQ); break; case INT_LINK_UP: case INT_LINK_DOWN: -- cgit v1.1 From 4947793916e31ec0c4c56f979e2aff89d15480bf Mon Sep 17 00:00:00 2001 From: Bjorn Helgaas Date: Thu, 8 Sep 2016 15:15:24 -0500 Subject: PCI: pciehp: Remove unnecessary guard In pcie_isr(), we return early if no status bits other than PCI_EXP_SLTSTA_CC are set. This was introduced by dbd79aed1aea ("pciehp: fix NULL dereference in interrupt handler"), but it is no longer necessary because all the subsequent pcie_isr() code is already predicated on a status bit being set. Remove the unnecessary test for ~PCI_EXP_SLTSTA_CC. No functional change intended. Tested-by: Lukas Wunner Signed-off-by: Bjorn Helgaas Reviewed-by: Mika Westerberg --- drivers/pci/hotplug/pciehp_hpc.c | 3 --- 1 file changed, 3 deletions(-) (limited to 'drivers/pci/hotplug') diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c index fe99b45..60e1d55 100644 --- a/drivers/pci/hotplug/pciehp_hpc.c +++ b/drivers/pci/hotplug/pciehp_hpc.c @@ -590,9 +590,6 @@ static irqreturn_t pciehp_isr(int irq, void *dev_id) } } - if (!(events & ~PCI_EXP_SLTSTA_CC)) - return IRQ_HANDLED; - /* Check Attention Button Pressed */ if (events & PCI_EXP_SLTSTA_ABP) { ctrl_info(ctrl, "Button pressed on Slot(%s)\n", -- cgit v1.1 From 6e49b304e379f93c8aa7ebb164628aec1209f371 Mon Sep 17 00:00:00 2001 From: Bjorn Helgaas Date: Thu, 8 Sep 2016 15:19:58 -0500 Subject: PCI: pciehp: Clean up dmesg "Slot(%s)" messages Print slot name consistently as "Slot(%s)". I don't know whether that's ideal, but we can at least do it the same way all the time. No functional change intended. Tested-by: Lukas Wunner Signed-off-by: Bjorn Helgaas Reviewed-by: Mika Westerberg --- drivers/pci/hotplug/pciehp_ctrl.c | 56 +++++++++++++++++++-------------------- drivers/pci/hotplug/pciehp_hpc.c | 12 ++++----- 2 files changed, 33 insertions(+), 35 deletions(-) (limited to 'drivers/pci/hotplug') diff --git a/drivers/pci/hotplug/pciehp_ctrl.c b/drivers/pci/hotplug/pciehp_ctrl.c index a787684..bf50f26 100644 --- a/drivers/pci/hotplug/pciehp_ctrl.c +++ b/drivers/pci/hotplug/pciehp_ctrl.c @@ -106,7 +106,7 @@ static int board_added(struct slot *p_slot) /* Check for a power fault */ if (ctrl->power_fault_detected || pciehp_query_power_fault(p_slot)) { - ctrl_err(ctrl, "Power fault on slot %s\n", slot_name(p_slot)); + ctrl_err(ctrl, "Slot(%s): Power fault\n", slot_name(p_slot)); retval = -EIO; goto err_exit; } @@ -254,11 +254,11 @@ static void handle_button_press_event(struct slot *p_slot) pciehp_get_power_status(p_slot, &getstatus); if (getstatus) { p_slot->state = BLINKINGOFF_STATE; - ctrl_info(ctrl, "PCI slot #%s - powering off due to button press\n", + ctrl_info(ctrl, "Slot(%s): Powering off due to button press\n", slot_name(p_slot)); } else { p_slot->state = BLINKINGON_STATE; - ctrl_info(ctrl, "PCI slot #%s - powering on due to button press\n", + ctrl_info(ctrl, "Slot(%s) Powering on due to button press\n", slot_name(p_slot)); } /* blink green LED and turn off amber */ @@ -273,14 +273,14 @@ static void handle_button_press_event(struct slot *p_slot) * press the attention again before the 5 sec. limit * expires to cancel hot-add or hot-remove */ - ctrl_info(ctrl, "Button cancel on Slot(%s)\n", slot_name(p_slot)); + ctrl_info(ctrl, "Slot(%s): Button cancel\n", slot_name(p_slot)); cancel_delayed_work(&p_slot->work); if (p_slot->state == BLINKINGOFF_STATE) pciehp_green_led_on(p_slot); else pciehp_green_led_off(p_slot); pciehp_set_attention_status(p_slot, 0); - ctrl_info(ctrl, "PCI slot #%s - action canceled due to button press\n", + ctrl_info(ctrl, "Slot(%s): Action canceled due to button press\n", slot_name(p_slot)); p_slot->state = STATIC_STATE; break; @@ -291,10 +291,12 @@ static void handle_button_press_event(struct slot *p_slot) * this means that the previous attention button action * to hot-add or hot-remove is undergoing */ - ctrl_info(ctrl, "Button ignore on Slot(%s)\n", slot_name(p_slot)); + ctrl_info(ctrl, "Slot(%s): Button ignored\n", + slot_name(p_slot)); break; default: - ctrl_warn(ctrl, "ignoring invalid state %#x\n", p_slot->state); + ctrl_err(ctrl, "Slot(%s): Ignoring invalid state %#x\n", + slot_name(p_slot), p_slot->state); break; } } @@ -317,31 +319,27 @@ static void handle_link_event(struct slot *p_slot, u32 event) break; case POWERON_STATE: if (event == INT_LINK_UP) { - ctrl_info(ctrl, - "Link Up event ignored on slot(%s): already powering on\n", + ctrl_info(ctrl, "Slot(%s): Link Up event ignored; already powering on\n", slot_name(p_slot)); } else { - ctrl_info(ctrl, - "Link Down event queued on slot(%s): currently getting powered on\n", + ctrl_info(ctrl, "Slot(%s): Link Down event queued; currently getting powered on\n", slot_name(p_slot)); pciehp_queue_power_work(p_slot, DISABLE_REQ); } break; case POWEROFF_STATE: if (event == INT_LINK_UP) { - ctrl_info(ctrl, - "Link Up event queued on slot(%s): currently getting powered off\n", + ctrl_info(ctrl, "Slot(%s): Link Up event queued; currently getting powered off\n", slot_name(p_slot)); pciehp_queue_power_work(p_slot, ENABLE_REQ); } else { - ctrl_info(ctrl, - "Link Down event ignored on slot(%s): already powering off\n", + ctrl_info(ctrl, "Slot(%s): Link Down event ignored; already powering off\n", slot_name(p_slot)); } break; default: - ctrl_err(ctrl, "ignoring invalid state %#x on slot(%s)\n", - p_slot->state, slot_name(p_slot)); + ctrl_err(ctrl, "Slot(%s): Ignoring invalid state %#x\n", + slot_name(p_slot), p_slot->state); break; } } @@ -396,13 +394,13 @@ int pciehp_enable_slot(struct slot *p_slot) pciehp_get_adapter_status(p_slot, &getstatus); if (!getstatus) { - ctrl_info(ctrl, "No adapter on slot(%s)\n", slot_name(p_slot)); + ctrl_info(ctrl, "Slot(%s): No adapter\n", slot_name(p_slot)); return -ENODEV; } if (MRL_SENS(p_slot->ctrl)) { pciehp_get_latch_status(p_slot, &getstatus); if (getstatus) { - ctrl_info(ctrl, "Latch open on slot(%s)\n", + ctrl_info(ctrl, "Slot(%s): Latch open\n", slot_name(p_slot)); return -ENODEV; } @@ -411,7 +409,7 @@ int pciehp_enable_slot(struct slot *p_slot) if (POWER_CTRL(p_slot->ctrl)) { pciehp_get_power_status(p_slot, &getstatus); if (getstatus) { - ctrl_info(ctrl, "Already enabled on slot(%s)\n", + ctrl_info(ctrl, "Slot(%s): Already enabled\n", slot_name(p_slot)); return -EINVAL; } @@ -440,7 +438,7 @@ int pciehp_disable_slot(struct slot *p_slot) if (POWER_CTRL(p_slot->ctrl)) { pciehp_get_power_status(p_slot, &getstatus); if (!getstatus) { - ctrl_info(ctrl, "Already disabled on slot(%s)\n", + ctrl_info(ctrl, "Slot(%s): Already disabled\n", slot_name(p_slot)); return -EINVAL; } @@ -468,17 +466,17 @@ int pciehp_sysfs_enable_slot(struct slot *p_slot) p_slot->state = STATIC_STATE; break; case POWERON_STATE: - ctrl_info(ctrl, "Slot %s is already in powering on state\n", + ctrl_info(ctrl, "Slot(%s): Already in powering on state\n", slot_name(p_slot)); break; case BLINKINGOFF_STATE: case POWEROFF_STATE: - ctrl_info(ctrl, "Already enabled on slot %s\n", + ctrl_info(ctrl, "Slot(%s): Already enabled\n", slot_name(p_slot)); break; default: - ctrl_err(ctrl, "invalid state %#x on slot %s\n", - p_slot->state, slot_name(p_slot)); + ctrl_err(ctrl, "Slot(%s): Invalid state %#x\n", + slot_name(p_slot), p_slot->state); break; } mutex_unlock(&p_slot->lock); @@ -505,17 +503,17 @@ int pciehp_sysfs_disable_slot(struct slot *p_slot) p_slot->state = STATIC_STATE; break; case POWEROFF_STATE: - ctrl_info(ctrl, "Slot %s is already in powering off state\n", + ctrl_info(ctrl, "Slot(%s): Already in powering off state\n", slot_name(p_slot)); break; case BLINKINGON_STATE: case POWERON_STATE: - ctrl_info(ctrl, "Already disabled on slot %s\n", + ctrl_info(ctrl, "Slot(%s): Already disabled\n", slot_name(p_slot)); break; default: - ctrl_err(ctrl, "invalid state %#x on slot %s\n", - p_slot->state, slot_name(p_slot)); + ctrl_err(ctrl, "Slot(%s): Invalid state %#x\n", + slot_name(p_slot), p_slot->state); break; } mutex_unlock(&p_slot->lock); diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c index 60e1d55..4582fdf 100644 --- a/drivers/pci/hotplug/pciehp_hpc.c +++ b/drivers/pci/hotplug/pciehp_hpc.c @@ -592,7 +592,7 @@ static irqreturn_t pciehp_isr(int irq, void *dev_id) /* Check Attention Button Pressed */ if (events & PCI_EXP_SLTSTA_ABP) { - ctrl_info(ctrl, "Button pressed on Slot(%s)\n", + ctrl_info(ctrl, "Slot(%s): Attention button pressed\n", slot_name(slot)); pciehp_queue_interrupt_event(slot, INT_BUTTON_PRESS); } @@ -600,8 +600,8 @@ static irqreturn_t pciehp_isr(int irq, void *dev_id) /* Check Presence Detect Changed */ if (events & PCI_EXP_SLTSTA_PDC) { present = !!(status & PCI_EXP_SLTSTA_PDS); - ctrl_info(ctrl, "Card %spresent on Slot(%s)\n", - present ? "" : "not ", slot_name(slot)); + ctrl_info(ctrl, "Slot(%s): Card %spresent\n", slot_name(slot), + present ? "" : "not "); pciehp_queue_interrupt_event(slot, present ? INT_PRESENCE_ON : INT_PRESENCE_OFF); } @@ -609,13 +609,13 @@ static irqreturn_t pciehp_isr(int irq, void *dev_id) /* Check Power Fault Detected */ if ((events & PCI_EXP_SLTSTA_PFD) && !ctrl->power_fault_detected) { ctrl->power_fault_detected = 1; - ctrl_err(ctrl, "Power fault on slot %s\n", slot_name(slot)); + ctrl_err(ctrl, "Slot(%s): Power fault\n", slot_name(slot)); pciehp_queue_interrupt_event(slot, INT_POWER_FAULT); } if (events & PCI_EXP_SLTSTA_DLLSC) { - ctrl_info(ctrl, "slot(%s): Link %s event\n", - slot_name(slot), link ? "Up" : "Down"); + ctrl_info(ctrl, "Slot(%s): Link %s\n", slot_name(slot), + link ? "Up" : "Down"); pciehp_queue_interrupt_event(slot, link ? INT_LINK_UP : INT_LINK_DOWN); } -- cgit v1.1 From 29a654e59f3698d70d85e289fc5ce7261493bba2 Mon Sep 17 00:00:00 2001 From: Bjorn Helgaas Date: Wed, 7 Sep 2016 17:50:30 -0500 Subject: PCI: pciehp: Remove useless pciehp_get_latch_status() calls Long ago, we updated a "switch_save" field based on the latch status. But switch_save was unused, and ed6cbcf2ac70 ("[PATCH] pciehp: miscellaneous cleanups") removed it. We no longer use the latch status, so remove calls to pciehp_get_latch_status(). No functional change intended. Tested-by: Lukas Wunner Signed-off-by: Bjorn Helgaas Reviewed-by: Mika Westerberg --- drivers/pci/hotplug/pciehp_ctrl.c | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) (limited to 'drivers/pci/hotplug') diff --git a/drivers/pci/hotplug/pciehp_ctrl.c b/drivers/pci/hotplug/pciehp_ctrl.c index bf50f26..efe69e8 100644 --- a/drivers/pci/hotplug/pciehp_ctrl.c +++ b/drivers/pci/hotplug/pciehp_ctrl.c @@ -389,7 +389,6 @@ static void interrupt_event_handler(struct work_struct *work) int pciehp_enable_slot(struct slot *p_slot) { u8 getstatus = 0; - int rc; struct controller *ctrl = p_slot->ctrl; pciehp_get_adapter_status(p_slot, &getstatus); @@ -415,13 +414,7 @@ int pciehp_enable_slot(struct slot *p_slot) } } - pciehp_get_latch_status(p_slot, &getstatus); - - rc = board_added(p_slot); - if (rc) - pciehp_get_latch_status(p_slot, &getstatus); - - return rc; + return board_added(p_slot); } /* -- cgit v1.1 From 576243b3f9eaa47ab568ac49574b3a095c2365f1 Mon Sep 17 00:00:00 2001 From: Keith Busch Date: Tue, 13 Sep 2016 10:31:59 -0600 Subject: PCI: pciehp: Allow exclusive userspace control of indicators PCIe hotplug supports optional Attention and Power Indicators, which are used internally by pciehp. Users can't control the Power Indicator, but they can control the Attention Indicator by writing to a sysfs "attention" file. The Slot Control register has two bits for each indicator, and the PCIe spec defines the encodings for each as (Reserved/On/Blinking/Off). For sysfs "attention" writes, pciehp_set_attention_status() maps into these encodings, so the only useful write values are 0 (Off), 1 (On), and 2 (Blinking). However, some platforms use all four bits for platform-specific indicators, and they need to allow direct user control of them while preventing pciehp from using them at all. Add a "hotplug_user_indicators" flag to the pci_dev structure. When set, pciehp does not use either the Attention Indicator or the Power Indicator, and the low four bits (values 0x0 - 0xf) of sysfs "attention" write values are written directly to the Attention Indicator Control and Power Indicator Control fields. [bhelgaas: changelog, rename flag and accessors to s/attention/indicator/] Signed-off-by: Keith Busch Signed-off-by: Bjorn Helgaas --- drivers/pci/hotplug/pciehp.h | 3 +++ drivers/pci/hotplug/pciehp_core.c | 3 +++ drivers/pci/hotplug/pciehp_hpc.c | 27 +++++++++++++++++++++++++++ 3 files changed, 33 insertions(+) (limited to 'drivers/pci/hotplug') diff --git a/drivers/pci/hotplug/pciehp.h b/drivers/pci/hotplug/pciehp.h index e764918..37d70b5 100644 --- a/drivers/pci/hotplug/pciehp.h +++ b/drivers/pci/hotplug/pciehp.h @@ -152,6 +152,9 @@ bool pciehp_check_link_active(struct controller *ctrl); void pciehp_release_ctrl(struct controller *ctrl); int pciehp_reset_slot(struct slot *slot, int probe); +int pciehp_set_raw_indicator_status(struct hotplug_slot *h_slot, u8 status); +int pciehp_get_raw_indicator_status(struct hotplug_slot *h_slot, u8 *status); + static inline const char *slot_name(struct slot *slot) { return hotplug_slot_name(slot->hotplug_slot); diff --git a/drivers/pci/hotplug/pciehp_core.c b/drivers/pci/hotplug/pciehp_core.c index ac531e6..276e39c 100644 --- a/drivers/pci/hotplug/pciehp_core.c +++ b/drivers/pci/hotplug/pciehp_core.c @@ -114,6 +114,9 @@ static int init_slot(struct controller *ctrl) if (ATTN_LED(ctrl)) { ops->get_attention_status = get_attention_status; ops->set_attention_status = set_attention_status; + } else if (ctrl->pcie->port->hotplug_user_indicators) { + ops->get_attention_status = pciehp_get_raw_indicator_status; + ops->set_attention_status = pciehp_set_raw_indicator_status; } /* register this slot with the hotplug pci core */ diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c index 4582fdf..b57fc6d 100644 --- a/drivers/pci/hotplug/pciehp_hpc.c +++ b/drivers/pci/hotplug/pciehp_hpc.c @@ -355,6 +355,18 @@ static int pciehp_link_enable(struct controller *ctrl) return __pciehp_link_set(ctrl, true); } +int pciehp_get_raw_indicator_status(struct hotplug_slot *hotplug_slot, + u8 *status) +{ + struct slot *slot = hotplug_slot->private; + struct pci_dev *pdev = ctrl_dev(slot->ctrl); + u16 slot_ctrl; + + pcie_capability_read_word(pdev, PCI_EXP_SLTCTL, &slot_ctrl); + *status = (slot_ctrl & (PCI_EXP_SLTCTL_AIC | PCI_EXP_SLTCTL_PIC)) >> 6; + return 0; +} + void pciehp_get_attention_status(struct slot *slot, u8 *status) { struct controller *ctrl = slot->ctrl; @@ -431,6 +443,17 @@ int pciehp_query_power_fault(struct slot *slot) return !!(slot_status & PCI_EXP_SLTSTA_PFD); } +int pciehp_set_raw_indicator_status(struct hotplug_slot *hotplug_slot, + u8 status) +{ + struct slot *slot = hotplug_slot->private; + struct controller *ctrl = slot->ctrl; + + pcie_write_cmd_nowait(ctrl, status << 6, + PCI_EXP_SLTCTL_AIC | PCI_EXP_SLTCTL_PIC); + return 0; +} + void pciehp_set_attention_status(struct slot *slot, u8 value) { struct controller *ctrl = slot->ctrl; @@ -814,6 +837,10 @@ struct controller *pcie_init(struct pcie_device *dev) } ctrl->pcie = dev; pcie_capability_read_dword(pdev, PCI_EXP_SLTCAP, &slot_cap); + + if (pdev->hotplug_user_indicators) + slot_cap &= ~(PCI_EXP_SLTCAP_AIP | PCI_EXP_SLTCAP_PIP); + ctrl->slot_cap = slot_cap; mutex_init(&ctrl->ctrl_lock); init_waitqueue_head(&ctrl->queue); -- cgit v1.1