From patchwork Sat Jun 16 19:25:00 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Lukas Wunner X-Patchwork-Id: 10468023 X-Patchwork-Delegate: bhelgaas@google.com Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id DC93A6028E for ; Sat, 16 Jun 2018 19:28:50 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id D677B27CEA for ; Sat, 16 Jun 2018 19:28:50 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id CB54B27D0E; Sat, 16 Jun 2018 19:28:50 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.9 required=2.0 tests=BAYES_00, MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 4D82427CEA for ; Sat, 16 Jun 2018 19:28:50 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932801AbeFPT2t (ORCPT ); Sat, 16 Jun 2018 15:28:49 -0400 Received: from mailout3.hostsharing.net ([176.9.242.54]:53795 "EHLO mailout3.hostsharing.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932542AbeFPT2t (ORCPT ); Sat, 16 Jun 2018 15:28:49 -0400 Received: from h08.hostsharing.net (h08.hostsharing.net [83.223.95.28]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "*.hostsharing.net", Issuer "COMODO RSA Domain Validation Secure Server CA" (not verified)) by mailout3.hostsharing.net (Postfix) with ESMTPS id C5D7F1019563D; Sat, 16 Jun 2018 21:28:47 +0200 (CEST) Received: from localhost (unknown [89.246.108.87]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by h08.hostsharing.net (Postfix) with ESMTPSA id 5279B603E110; Sat, 16 Jun 2018 21:28:47 +0200 (CEST) X-Mailbox-Line: From c78d667de7024f4ad8bcd688b93a1ebb9053f444 Mon Sep 17 00:00:00 2001 Message-Id: In-Reply-To: References: From: Lukas Wunner Date: Sat, 16 Jun 2018 21:25:00 +0200 Subject: [PATCH 11/32] PCI: pciehp: Stop blinking on slot enable failure To: Bjorn Helgaas Cc: Mika Westerberg , "Rafael J. Wysocki" , Ashok Raj , Keith Busch , Yinghai Lu , Sinan Kaya , linux-pci@vger.kernel.org Sender: linux-pci-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP 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 --- drivers/pci/hotplug/pciehp_core.c | 7 +-- drivers/pci/hotplug/pciehp_ctrl.c | 73 +++++++++++++++++-------------- 2 files changed, 42 insertions(+), 38 deletions(-) diff --git a/drivers/pci/hotplug/pciehp_core.c b/drivers/pci/hotplug/pciehp_core.c index 2ba59fc94827..8a2133856dfd 100644 --- a/drivers/pci/hotplug/pciehp_core.c +++ b/drivers/pci/hotplug/pciehp_core.c @@ -248,11 +248,8 @@ static int pciehp_probe(struct pcie_device *dev) 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); @@ -296,12 +293,10 @@ static int pciehp_resume(struct pcie_device *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 */ diff --git a/drivers/pci/hotplug/pciehp_ctrl.c b/drivers/pci/hotplug/pciehp_ctrl.c index 76a9b8c26d2b..9d6343a35db1 100644 --- a/drivers/pci/hotplug/pciehp_ctrl.c +++ b/drivers/pci/hotplug/pciehp_ctrl.c @@ -160,26 +160,13 @@ static void pciehp_power_thread(struct work_struct *work) 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; @@ -369,7 +356,7 @@ static void interrupt_event_handler(struct work_struct *work) /* * 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; @@ -400,10 +387,29 @@ int pciehp_enable_slot(struct slot *p_slot) 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; @@ -421,9 +427,23 @@ int pciehp_disable_slot(struct slot *p_slot) return 0; } +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); @@ -433,12 +453,7 @@ int pciehp_sysfs_enable_slot(struct slot *p_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)); @@ -455,12 +470,11 @@ int pciehp_sysfs_enable_slot(struct slot *p_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); @@ -470,12 +484,7 @@ int pciehp_sysfs_disable_slot(struct slot *p_slot) 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)); @@ -492,5 +501,5 @@ int pciehp_sysfs_disable_slot(struct slot *p_slot) } mutex_unlock(&p_slot->lock); - return retval; + return -ENODEV; }