From b0ccd9dd5dc8bf348112bb97e4f7eef6be5cf469 Mon Sep 17 00:00:00 2001
From: Lukas Wunner <lukas@wunner.de>
Date: Thu, 19 Jul 2018 17:27:40 -0500
Subject: [PATCH] PCI: pciehp: Stop blinking on slot enable failure
Git-commit: b0ccd9dd5dc8bf348112bb97e4f7eef6be5cf469
Patch-mainline: v4.19
References: FATE#326303
If the attention button is pressed to power on the slot AND the user
powers on the slot via sysfs before 5 seconds have elapsed AND powering
on the slot fails because either the slot is unoccupied OR the latch is
open, we neglect turning off the green LED so it keeps on blinking.
That's because the error path of pciehp_sysfs_enable_slot() doesn't call
pciehp_green_led_off(), unlike pciehp_power_thread() which does.
The bug has been present since 2004 when the driver was introduced.
Fix by deduplicating common code in pciehp_sysfs_enable_slot() and
pciehp_power_thread() into a wrapper function pciehp_enable_slot() and
renaming the existing function to __pciehp_enable_slot(). Same for
pciehp_disable_slot(). This will also simplify the upcoming rework of
pciehp's event handling.
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Signed-off-by: Oliver Neukum <oneukum@suse.com>
---
drivers/pci/hotplug/pciehp_core.c | 7 ---
drivers/pci/hotplug/pciehp_ctrl.c | 73 +++++++++++++++++++++-----------------
2 files changed, 42 insertions(+), 38 deletions(-)
--- a/drivers/pci/hotplug/pciehp_core.c
+++ b/drivers/pci/hotplug/pciehp_core.c
@@ -262,11 +262,8 @@ static int pciehp_probe(struct pcie_devi
slot = ctrl->slot;
pciehp_get_adapter_status(slot, &occupied);
pciehp_get_power_status(slot, &poweron);
- if (occupied && pciehp_force) {
- mutex_lock(&slot->hotplug_lock);
+ if (occupied && pciehp_force)
pciehp_enable_slot(slot);
- mutex_unlock(&slot->hotplug_lock);
- }
/* If empty slot's power status is on, turn power off */
if (!occupied && poweron && POWER_CTRL(ctrl))
pciehp_power_off_slot(slot);
@@ -310,12 +307,10 @@ static int pciehp_resume(struct pcie_dev
/* Check if slot is occupied */
pciehp_get_adapter_status(slot, &status);
- mutex_lock(&slot->hotplug_lock);
if (status)
pciehp_enable_slot(slot);
else
pciehp_disable_slot(slot);
- mutex_unlock(&slot->hotplug_lock);
return 0;
}
#endif /* PM */
--- a/drivers/pci/hotplug/pciehp_ctrl.c
+++ b/drivers/pci/hotplug/pciehp_ctrl.c
@@ -178,26 +178,13 @@ static void pciehp_power_thread(struct w
struct power_work_info *info =
container_of(work, struct power_work_info, work);
struct slot *p_slot = info->p_slot;
- int ret;
switch (info->req) {
case DISABLE_REQ:
- mutex_lock(&p_slot->hotplug_lock);
pciehp_disable_slot(p_slot);
- mutex_unlock(&p_slot->hotplug_lock);
- mutex_lock(&p_slot->lock);
- p_slot->state = STATIC_STATE;
- mutex_unlock(&p_slot->lock);
break;
case ENABLE_REQ:
- mutex_lock(&p_slot->hotplug_lock);
- ret = pciehp_enable_slot(p_slot);
- mutex_unlock(&p_slot->hotplug_lock);
- if (ret)
- pciehp_green_led_off(p_slot);
- mutex_lock(&p_slot->lock);
- p_slot->state = STATIC_STATE;
- mutex_unlock(&p_slot->lock);
+ pciehp_enable_slot(p_slot);
break;
default:
break;
@@ -387,7 +374,7 @@ static void interrupt_event_handler(stru
/*
* Note: This function must be called with slot->hotplug_lock held
*/
-int pciehp_enable_slot(struct slot *p_slot)
+static int __pciehp_enable_slot(struct slot *p_slot)
{
u8 getstatus = 0;
struct controller *ctrl = p_slot->ctrl;
@@ -418,10 +405,29 @@ int pciehp_enable_slot(struct slot *p_sl
return board_added(p_slot);
}
+int pciehp_enable_slot(struct slot *slot)
+{
+ struct controller *ctrl = slot->ctrl;
+ int ret;
+
+ mutex_lock(&slot->hotplug_lock);
+ ret = __pciehp_enable_slot(slot);
+ mutex_unlock(&slot->hotplug_lock);
+
+ if (ret && ATTN_BUTTN(ctrl))
+ pciehp_green_led_off(slot); /* may be blinking */
+
+ mutex_lock(&slot->lock);
+ slot->state = STATIC_STATE;
+ mutex_unlock(&slot->lock);
+
+ return ret;
+}
+
/*
* Note: This function must be called with slot->hotplug_lock held
*/
-int pciehp_disable_slot(struct slot *p_slot)
+static int __pciehp_disable_slot(struct slot *p_slot)
{
u8 getstatus = 0;
struct controller *ctrl = p_slot->ctrl;
@@ -441,9 +447,23 @@ int pciehp_disable_slot(struct slot *p_s
return remove_board(p_slot);
}
+int pciehp_disable_slot(struct slot *slot)
+{
+ int ret;
+
+ mutex_lock(&slot->hotplug_lock);
+ ret = __pciehp_disable_slot(slot);
+ mutex_unlock(&slot->hotplug_lock);
+
+ mutex_lock(&slot->lock);
+ slot->state = STATIC_STATE;
+ mutex_unlock(&slot->lock);
+
+ return ret;
+}
+
int pciehp_sysfs_enable_slot(struct slot *p_slot)
{
- int retval = -ENODEV;
struct controller *ctrl = p_slot->ctrl;
mutex_lock(&p_slot->lock);
@@ -453,12 +473,7 @@ int pciehp_sysfs_enable_slot(struct slot
case STATIC_STATE:
p_slot->state = POWERON_STATE;
mutex_unlock(&p_slot->lock);
- mutex_lock(&p_slot->hotplug_lock);
- retval = pciehp_enable_slot(p_slot);
- mutex_unlock(&p_slot->hotplug_lock);
- mutex_lock(&p_slot->lock);
- p_slot->state = STATIC_STATE;
- break;
+ return pciehp_enable_slot(p_slot);
case POWERON_STATE:
ctrl_info(ctrl, "Slot(%s): Already in powering on state\n",
slot_name(p_slot));
@@ -475,12 +490,11 @@ int pciehp_sysfs_enable_slot(struct slot
}
mutex_unlock(&p_slot->lock);
- return retval;
+ return -ENODEV;
}
int pciehp_sysfs_disable_slot(struct slot *p_slot)
{
- int retval = -ENODEV;
struct controller *ctrl = p_slot->ctrl;
mutex_lock(&p_slot->lock);
@@ -490,12 +504,7 @@ int pciehp_sysfs_disable_slot(struct slo
case STATIC_STATE:
p_slot->state = POWEROFF_STATE;
mutex_unlock(&p_slot->lock);
- mutex_lock(&p_slot->hotplug_lock);
- retval = pciehp_disable_slot(p_slot);
- mutex_unlock(&p_slot->hotplug_lock);
- mutex_lock(&p_slot->lock);
- p_slot->state = STATIC_STATE;
- break;
+ return pciehp_disable_slot(p_slot);
case POWEROFF_STATE:
ctrl_info(ctrl, "Slot(%s): Already in powering off state\n",
slot_name(p_slot));
@@ -512,5 +521,5 @@ int pciehp_sysfs_disable_slot(struct slo
}
mutex_unlock(&p_slot->lock);
- return retval;
+ return -ENODEV;
}