From patchwork Fri Oct 11 11:13:47 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Rafael J. Wysocki" X-Patchwork-Id: 3023051 X-Patchwork-Delegate: bhelgaas@google.com Return-Path: X-Original-To: patchwork-linux-pci@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 488B7BF924 for ; Fri, 11 Oct 2013 11:02:30 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 02B142014B for ; Fri, 11 Oct 2013 11:02:29 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id A3EA42025B for ; Fri, 11 Oct 2013 11:02:27 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757861Ab3JKLCH (ORCPT ); Fri, 11 Oct 2013 07:02:07 -0400 Received: from v094114.home.net.pl ([79.96.170.134]:53687 "HELO v094114.home.net.pl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1758160Ab3JKLCE (ORCPT ); Fri, 11 Oct 2013 07:02:04 -0400 Received: from cmk237.neoplus.adsl.tpnet.pl [83.31.138.237] (HELO vostro.rjw.lan) by serwer1319399.home.pl [79.96.170.134] with SMTP (IdeaSmtpServer v0.80) id f318ba18770b31d0; Fri, 11 Oct 2013 13:02:02 +0200 From: "Rafael J. Wysocki" To: Linus Torvalds Cc: Bjorn Helgaas , Steven Rostedt , LKML , "Rafael J. Wysocki" , Mika Westerberg , Andrew Morton , Linux PCI , ACPI Devel Maling List Subject: Re: [BUG] WARN_ON(!context) in drivers/pci/hotplug/acpiphp_glue.c Date: Fri, 11 Oct 2013 13:13:47 +0200 Message-ID: <2096980.ZtIelfoX39@vostro.rjw.lan> User-Agent: KMail/4.10.5 (Linux/3.11.0+; KDE/4.10.5; x86_64; ; ) In-Reply-To: References: <20131010165905.7815defa@gandalf.local.home> <18685640.N2BnVjDmHD@vostro.rjw.lan> MIME-Version: 1.0 Sender: linux-pci-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org X-Spam-Status: No, score=-7.1 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Thursday, October 10, 2013 06:42:56 PM Linus Torvalds wrote: > On Thu, Oct 10, 2013 at 6:45 PM, Rafael J. Wysocki wrote: > > > > /* Register slots for ejectable funtions only. */ > > - if (acpi_pci_check_ejectable(pbus, handle) || is_dock_device(handle)) { > > + if ((acpi_pci_check_ejectable(pbus, handle) || is_dock_device(handle)) > > + && !(pdev && device_is_managed_by_native_pciehp(pdev))) { > > unsigned long long sun; > > int retval; > > I can't even begin to say whether this is a good solution or not, > because that if-conditional makes me want to go out and kill some > homeless people to let my aggressions out. > > Can we please agree to *never* write code like this? Ever? > > Use a well-named inline helper function where the name describes what > the f*ck the code is trying to do, and then comment the separate > issues. Because none of the above line noise makes me go "Ahh, it's > the test for an ejectable function". > > What the heck _is_ an "ejectable function" anyway? The only comment > there just makes the code even less sensible. > > Please? From: Rafael J. Wysocki Subject: ACPI / hotplug / PCI: Accept coexistence with native PCIe hotplug Allow ACPIPHP (ACPI-based PCI hotplug) to handle event signaling for devices that have already been claimed by the native PCIe hotplug (pciehp). The ACPI hotplug events are essentially re-scan, remove and eject requests. Re-scan and remove should work regardless, because they may be triggered by user space via sysfs and the ACPI eject (_EJ0) should work if the BIOS wants us to use it. There may be an issue if the BIOS signals ACPI eject and wants us to use the native eject, but that doesn't work without this change anyway. This change prevents the WARN_ON() in acpiphp_enumerate_slots() from triggering unnecessarily for bridges whose parents are managed by pciehp. Signed-off-by: Rafael J. Wysocki Reported-by: Steven Rostedt Tested-by: Steven Rostedt --- drivers/pci/hotplug/acpiphp_glue.c | 32 ++++++++++++++++++++++++++------ 1 file changed, 26 insertions(+), 6 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Index: linux-pm/drivers/pci/hotplug/acpiphp_glue.c =================================================================== --- linux-pm.orig/drivers/pci/hotplug/acpiphp_glue.c +++ linux-pm/drivers/pci/hotplug/acpiphp_glue.c @@ -259,6 +259,31 @@ static void acpiphp_dock_release(void *d put_bridge(context->func.parent); } +/** + * slot_should_be_exposed - Check whether or not to expose a slot to userland. + * @bridge: ACPIPHP bridge the slot belongs to. + * @handle: ACPI handle of a device in the slot. + */ +static inline bool slot_should_be_exposed(struct acpiphp_bridge *bridge, + acpi_handle handle) +{ + struct pci_bus *pbus = bridge->pci_bus; + struct pci_dev *pdev = bridge->pci_dev; + + /* + * Do not expose slots whose bridges are managed by pciehp, because they + * will be exposed to user space by the pciehp driver. + */ + if (pdev && device_is_managed_by_native_pciehp(pdev)) + return false; + + /* + * Expose slots for devices with either _EJ0 or _RMV and for devices + * on docking stations. + */ + return acpi_pci_check_ejectable(pbus, handle) || is_dock_device(handle); +} + /* callback routine to register each ACPI PCI slot object */ static acpi_status register_slot(acpi_handle handle, u32 lvl, void *data, void **rv) @@ -271,12 +296,8 @@ static acpi_status register_slot(acpi_ha unsigned long long adr; int device, function; struct pci_bus *pbus = bridge->pci_bus; - struct pci_dev *pdev = bridge->pci_dev; u32 val; - if (pdev && device_is_managed_by_native_pciehp(pdev)) - return AE_OK; - status = acpi_evaluate_integer(handle, "_ADR", NULL, &adr); if (ACPI_FAILURE(status)) { acpi_handle_warn(handle, "can't evaluate _ADR (%#x)\n", status); @@ -325,8 +346,7 @@ static acpi_status register_slot(acpi_ha list_add_tail(&slot->node, &bridge->slots); - /* Register slots for ejectable funtions only. */ - if (acpi_pci_check_ejectable(pbus, handle) || is_dock_device(handle)) { + if (slot_should_be_exposed(bridge, handle)) { unsigned long long sun; int retval;