From patchwork Thu May 31 13:51:17 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mika Westerberg X-Patchwork-Id: 10441143 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 0F260602BD for ; Thu, 31 May 2018 13:51:29 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id F2C3B296CD for ; Thu, 31 May 2018 13:51:28 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id E75DC296D1; Thu, 31 May 2018 13:51:28 +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 7FCBB296CD for ; Thu, 31 May 2018 13:51:28 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755211AbeEaNv1 (ORCPT ); Thu, 31 May 2018 09:51:27 -0400 Received: from mga02.intel.com ([134.134.136.20]:17711 "EHLO mga02.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755042AbeEaNvY (ORCPT ); Thu, 31 May 2018 09:51:24 -0400 X-Amp-Result: UNSCANNABLE X-Amp-File-Uploaded: False Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga101.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 31 May 2018 06:51:23 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.49,463,1520924400"; d="scan'208";a="60141796" Received: from lahna.fi.intel.com (HELO lahna) ([10.237.72.157]) by fmsmga001.fm.intel.com with SMTP; 31 May 2018 06:51:18 -0700 Received: by lahna (sSMTP sendmail emulation); Thu, 31 May 2018 16:51:17 +0300 Date: Thu, 31 May 2018 16:51:17 +0300 From: Mika Westerberg To: Bjorn Helgaas Cc: Bjorn Helgaas , "Rafael J . Wysocki" , Len Brown , Mario.Limonciello@dell.com, Michael Jamet , Yehezkel Bernat , Andy Shevchenko , Lukas Wunner , linux-pci@vger.kernel.org, linux-acpi@vger.kernel.org Subject: Re: [PATCH v8 2/7] PCI: Introduce shpchp_is_native() Message-ID: <20180531135117.GX15419@lahna.fi.intel.com> References: <20180528124756.78512-1-mika.westerberg@linux.intel.com> <20180528124756.78512-3-mika.westerberg@linux.intel.com> <20180530202343.GO39853@bhelgaas-glaptop.roam.corp.google.com> <20180531065852.GV15419@lahna.fi.intel.com> <20180531131202.GA76395@bhelgaas-glaptop.roam.corp.google.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20180531131202.GA76395@bhelgaas-glaptop.roam.corp.google.com> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo User-Agent: Mutt/1.9.5 (2018-04-13) 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 On Thu, May 31, 2018 at 08:12:02AM -0500, Bjorn Helgaas wrote: > Maybe I'm reading your patches wrong. It looks to me like shpchp will > claim GOLAM_7450, which means shpchp will register slots, program the > SHPC, handle hotplug interrupts, etc. > > But since shpchp_is_native() returns false, acpiphp thinks *it* should > handle hotplug. For example, I think that given some ACPI > prerequisites (_EJ0/_RMV/etc), both will call pci_hp_register(): > > shpc_probe > is_shpc_capable # true for GOLAM_7450 > init_slots > pci_hp_register > > acpi_pci_add_slots > acpiphp_enumerate_slots > acpi_walk_namespace(..., acpiphp_add_context) > acpiphp_add_context > hotplug_is_native # false for GOLAM_7450 > acpiphp_register_hotplug_slot > pci_hp_register > > It is true that the same situation occurred before your patches, since > acpiphp_add_context() only checked pciehp_is_native(). In fact, with > the existing code, shpchp and acpiphp could both try to manage *any* > SHPC, not just GOLAM_7450. > > I think the current series fixes 99% of that problem and it seems like > we should try to do that last 1% at the same time so the SHPC code > makes more sense. Would the following fix the last 1% for you? Applies on top of this patch. diff --git a/drivers/pci/hotplug/shpchp.h b/drivers/pci/hotplug/shpchp.h index 9675ab757323..516e4835019c 100644 --- a/drivers/pci/hotplug/shpchp.h +++ b/drivers/pci/hotplug/shpchp.h @@ -105,7 +105,6 @@ struct controller { }; /* Define AMD SHPC ID */ -#define PCI_DEVICE_ID_AMD_GOLAM_7450 0x7450 #define PCI_DEVICE_ID_AMD_POGO_7458 0x7458 /* AMD PCI-X bridge registers */ diff --git a/drivers/pci/hotplug/shpchp_core.c b/drivers/pci/hotplug/shpchp_core.c index 0f3711404c2b..e91be287f292 100644 --- a/drivers/pci/hotplug/shpchp_core.c +++ b/drivers/pci/hotplug/shpchp_core.c @@ -270,22 +270,12 @@ static int get_adapter_status(struct hotplug_slot *hotplug_slot, u8 *value) return 0; } -static int is_shpc_capable(struct pci_dev *dev) -{ - if (dev->vendor == PCI_VENDOR_ID_AMD && - dev->device == PCI_DEVICE_ID_AMD_GOLAM_7450) - return 1; - if (acpi_get_hp_hw_control_from_firmware(dev)) - return 0; - return 1; -} - static int shpc_probe(struct pci_dev *pdev, const struct pci_device_id *ent) { int rc; struct controller *ctrl; - if (!is_shpc_capable(pdev)) + if (acpi_get_hp_hw_control_from_firmware(pdev)) return -ENODEV; ctrl = kzalloc(sizeof(*ctrl), GFP_KERNEL); diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c index bbc4ea70505a..fd1c0ee33805 100644 --- a/drivers/pci/pci-acpi.c +++ b/drivers/pci/pci-acpi.c @@ -408,6 +408,14 @@ bool shpchp_is_native(struct pci_dev *bridge) if (!IS_ENABLED(CONFIG_HOTPLUG_PCI_SHPC)) return false; + /* + * It is assumed that AMD GOLAM chips support SHPC but they do not + * have SHPC capability. + */ + if (bridge->vendor == PCI_VENDOR_ID_AMD && + bridge->device == PCI_DEVICE_ID_AMD_GOLAM_7450) + return true; + if (!pci_find_capability(bridge, PCI_CAP_ID_SHPC)) return false; diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h index 883cb7bf78aa..5aace6cca0d7 100644 --- a/include/linux/pci_ids.h +++ b/include/linux/pci_ids.h @@ -561,6 +561,7 @@ #define PCI_DEVICE_ID_AMD_OPUS_7443 0x7443 #define PCI_DEVICE_ID_AMD_VIPER_7443 0x7443 #define PCI_DEVICE_ID_AMD_OPUS_7445 0x7445 +#define PCI_DEVICE_ID_AMD_GOLAM_7450 0x7450 #define PCI_DEVICE_ID_AMD_8111_PCI 0x7460 #define PCI_DEVICE_ID_AMD_8111_LPC 0x7468 #define PCI_DEVICE_ID_AMD_8111_IDE 0x7469