From patchwork Sat Sep 15 03:05:07 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jiang Liu X-Patchwork-Id: 1461371 X-Patchwork-Delegate: bhelgaas@google.com Return-Path: X-Original-To: patchwork-linux-pci@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork1.kernel.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by patchwork1.kernel.org (Postfix) with ESMTP id D65FF40220 for ; Sat, 15 Sep 2012 03:15:41 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751198Ab2IODNi (ORCPT ); Fri, 14 Sep 2012 23:13:38 -0400 Received: from mail-pb0-f46.google.com ([209.85.160.46]:33180 "EHLO mail-pb0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756013Ab2IODNO (ORCPT ); Fri, 14 Sep 2012 23:13:14 -0400 Received: by pbbrr13 with SMTP id rr13so6509443pbb.19 for ; Fri, 14 Sep 2012 20:13:14 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=from:to:cc:subject:date:message-id:x-mailer:in-reply-to:references; bh=gYWsJp/rm2uLJxqJiCw4HHQqkxY/lZZA5Ry357f16/c=; b=oZJp9JPsFTt1GRtkY5iKrVPkIef/QoVdOYPMBorURtEB7cmItB3mET0RCwOJwSjYq6 CtayeTOw27wa/2P3QQh8smGoto2Iu1FM9OcW7aTphHkgr7SDHWtDEsW9M7mzImvpvIrm e/SmzNE5Ng6HaiDZrd/um/QbLddwzcPvR9ebbYabsPtIkbpNyX1Lbun9kPGrRDQrtwMj WN3Hqsx9ti0ArmHM3bh/q3fRSHJw9EIVlzoxacgr5u9mhjdZude0mF9tcRzBPVEvEY6Z HOipv3ybceWpQ1RNIb1hEFp1W7630HcgsO2A93AFv3kxkP9XBagS0huJDbbOIUL2Sqoi myxA== Received: by 10.68.135.67 with SMTP id pq3mr8320894pbb.49.1347678793979; Fri, 14 Sep 2012 20:13:13 -0700 (PDT) Received: from localhost.localdomain ([221.221.18.122]) by mx.google.com with ESMTPS id jz10sm2092777pbc.8.2012.09.14.20.13.05 (version=TLSv1/SSLv3 cipher=OTHER); Fri, 14 Sep 2012 20:13:13 -0700 (PDT) From: Jiang Liu To: Bjorn Helgaas , Len Brown Cc: Tony Luck , Jiang Liu , Yinghai Lu , Kenji Kaneshige , Yijing Wang , Jiang Liu , linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org, linux-acpi@vger.kernel.org Subject: [PATCH v2 4/9] ACPI/pci_bind: correctly update binding relationship for PCI hotplug Date: Sat, 15 Sep 2012 11:05:07 +0800 Message-Id: <1347678312-11124-5-git-send-email-jiang.liu@huawei.com> X-Mailer: git-send-email 1.7.9.5 In-Reply-To: <1347678312-11124-1-git-send-email-jiang.liu@huawei.com> References: <1347678312-11124-1-git-send-email-jiang.liu@huawei.com> Sender: linux-pci-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org From: Jiang Liu Currently pci_bind.c is used to maintain binding relationship between ACPI and PCI devices. But it's broken when handling PCI hotplug events. For the acpiphp driver, it's designed to update the binding relationship when PCI hotplug event happens, but the implementation is broken. For PCI device hot-adding: enable_device() pci_scan_slot() acpiphp_bus_add() acpi_bus_add() acpi_pci_bind() acpi_get_pci_dev() return NULL because dev->archdata.acpi_handle is still unset return without updating actual binding relationship pci_bus_add_devices() pci_bus_add_device() device_add() platform_notify() acpi_bind_one() set dev->archdata.acpi_handle For PCI device hot-removal, disable_device() pci_stop_bus_device() __pci_remove_bus_device() acpiphp_bus_trim() acpi_bus_remove() acpi_pci_unbind() return without really unbinding because PCI device has already been destroyed For other PCI hotplug drivers, they even don't bother to update binding relationships. So hook into acpi_bind_one()/acpi_unbind_one() in drivers/acpi/glue.c to update PCI<->ACPI binding relationship. Signed-off-by: Jiang Liu Signed-off-by: Yijing Wang --- drivers/acpi/glue.c | 6 ++- drivers/acpi/internal.h | 2 + drivers/acpi/pci_bind.c | 101 +++++++++++++++++++++++++++++++---------------- drivers/acpi/pci_root.c | 40 +++++-------------- 4 files changed, 81 insertions(+), 68 deletions(-) diff --git a/drivers/acpi/glue.c b/drivers/acpi/glue.c index 243ee85..4904700 100644 --- a/drivers/acpi/glue.c +++ b/drivers/acpi/glue.c @@ -121,7 +121,6 @@ acpi_handle acpi_get_child(acpi_handle parent, u64 address) 1, do_acpi_find_child, NULL, &find, NULL); return find.handle; } - EXPORT_SYMBOL(acpi_get_child); /* Link ACPI devices with physical devices */ @@ -142,7 +141,6 @@ struct device *acpi_get_physical_device(acpi_handle handle) return get_device(dev); return NULL; } - EXPORT_SYMBOL(acpi_get_physical_device); static int acpi_bind_one(struct device *dev, acpi_handle handle) @@ -162,6 +160,8 @@ static int acpi_bind_one(struct device *dev, acpi_handle handle) } dev->archdata.acpi_handle = handle; + acpi_pci_bind_notify(dev->archdata.acpi_handle, dev, true); + status = acpi_bus_get_device(handle, &acpi_dev); if (!ACPI_FAILURE(status)) { int ret; @@ -193,6 +193,8 @@ static int acpi_unbind_one(struct device *dev) sysfs_remove_link(&acpi_dev->dev.kobj, "physical_node"); } + acpi_pci_bind_notify(dev->archdata.acpi_handle, dev, false); + acpi_detach_data(dev->archdata.acpi_handle, acpi_glue_data_handler); dev->archdata.acpi_handle = NULL; diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h index ca75b9c..5396b20 100644 --- a/drivers/acpi/internal.h +++ b/drivers/acpi/internal.h @@ -93,4 +93,6 @@ static inline int suspend_nvs_save(void) { return 0; } static inline void suspend_nvs_restore(void) {} #endif +void acpi_pci_bind_notify(acpi_handle handle, struct device *dev, bool bind); + #endif /* _ACPI_INTERNAL_H_ */ diff --git a/drivers/acpi/pci_bind.c b/drivers/acpi/pci_bind.c index 2ef0409..320e698 100644 --- a/drivers/acpi/pci_bind.c +++ b/drivers/acpi/pci_bind.c @@ -35,40 +35,41 @@ #define _COMPONENT ACPI_PCI_COMPONENT ACPI_MODULE_NAME("pci_bind"); -static int acpi_pci_unbind(struct acpi_device *device) -{ - struct pci_dev *dev; - - dev = acpi_get_pci_dev(device->handle); - if (!dev) - goto out; +static int acpi_pci_bind_cb(struct acpi_device *device); +static int acpi_pci_unbind(struct acpi_device *device, struct pci_dev *dev) +{ device_set_run_wake(&dev->dev, false); pci_acpi_remove_pm_notifier(device); - if (!dev->subordinate) - goto out; + if (dev->subordinate) { + acpi_pci_irq_del_prt(dev->subordinate); + device->ops.bind = NULL; + device->ops.unbind = NULL; + } + + return 0; +} - acpi_pci_irq_del_prt(dev->subordinate); +static int acpi_pci_unbind_cb(struct acpi_device *device) +{ + int rc = 0; + struct pci_dev *dev; - device->ops.bind = NULL; - device->ops.unbind = NULL; + dev = acpi_get_pci_dev(device->handle); + if (dev) { + rc = acpi_pci_unbind(device, dev); + pci_dev_put(dev); + } -out: - pci_dev_put(dev); - return 0; + return rc; } -static int acpi_pci_bind(struct acpi_device *device) +static int acpi_pci_bind(struct acpi_device *device, struct pci_dev *dev) { acpi_status status; acpi_handle handle; struct pci_bus *bus; - struct pci_dev *dev; - - dev = acpi_get_pci_dev(device->handle); - if (!dev) - return 0; pci_acpi_add_pm_notifier(device, dev); if (device->wakeup.flags.run_wake) @@ -83,8 +84,8 @@ static int acpi_pci_bind(struct acpi_device *device) "Device %04x:%02x:%02x.%d is a PCI bridge\n", pci_domain_nr(dev->bus), dev->bus->number, PCI_SLOT(dev->devfn), PCI_FUNC(dev->devfn))); - device->ops.bind = acpi_pci_bind; - device->ops.unbind = acpi_pci_unbind; + device->ops.bind = acpi_pci_bind_cb; + device->ops.unbind = acpi_pci_unbind_cb; } /* @@ -96,25 +97,55 @@ static int acpi_pci_bind(struct acpi_device *device) * TBD: Can _PRTs exist within the scope of non-bridge PCI devices? */ status = acpi_get_handle(device->handle, METHOD_NAME__PRT, &handle); - if (ACPI_FAILURE(status)) - goto out; + if (ACPI_SUCCESS(status)) { + if (dev->subordinate) + bus = dev->subordinate; + else + bus = dev->bus; + acpi_pci_irq_add_prt(device->handle, bus); + } - if (dev->subordinate) - bus = dev->subordinate; - else - bus = dev->bus; + return 0; +} - acpi_pci_irq_add_prt(device->handle, bus); +static int acpi_pci_bind_cb(struct acpi_device *device) +{ + int rc = 0; + struct pci_dev *dev; -out: - pci_dev_put(dev); - return 0; + dev = acpi_get_pci_dev(device->handle); + if (dev) { + rc = acpi_pci_bind(device, dev); + pci_dev_put(dev); + } + + return rc; } int acpi_pci_bind_root(struct acpi_device *device) { - device->ops.bind = acpi_pci_bind; - device->ops.unbind = acpi_pci_unbind; + device->ops.bind = acpi_pci_bind_cb; + device->ops.unbind = acpi_pci_unbind_cb; return 0; } + +void acpi_pci_bind_notify(acpi_handle handle, struct device *dev, bool bind) +{ + struct acpi_device *acpi_dev; + + if (!dev_is_pci(dev)) + return; + if (acpi_bus_get_device(handle, &acpi_dev) || !acpi_dev) + return; + + if (acpi_dev->parent) { + if (bind) { + if (acpi_dev->parent->ops.bind) + acpi_pci_bind(acpi_dev, to_pci_dev(dev)); + } else { + if (acpi_dev->parent->ops.unbind) + acpi_pci_unbind(acpi_dev, to_pci_dev(dev)); + } + } +} diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c index 72a2c98..7509034 100644 --- a/drivers/acpi/pci_root.c +++ b/drivers/acpi/pci_root.c @@ -195,21 +195,6 @@ static acpi_status try_get_root_bridge_busnr(acpi_handle handle, return AE_OK; } -static void acpi_pci_bridge_scan(struct acpi_device *device) -{ - int status; - struct acpi_device *child = NULL; - - if (device->flags.bus_address) - if (device->parent && device->parent->ops.bind) { - status = device->parent->ops.bind(device); - if (!status) { - list_for_each_entry(child, &device->children, node) - acpi_pci_bridge_scan(child); - } - } -} - static u8 pci_osc_uuid_str[] = "33DB4D5B-1FF7-401C-9657-7441C03DD766"; static acpi_status acpi_pci_run_osc(acpi_handle handle, @@ -456,7 +441,6 @@ static int __devinit acpi_pci_root_add(struct acpi_device *device) int result; struct acpi_pci_root *root; acpi_handle handle; - struct acpi_device *child; u32 flags, base_flags; root = kzalloc(sizeof(struct acpi_pci_root), GFP_KERNEL); @@ -521,6 +505,15 @@ static int __devinit acpi_pci_root_add(struct acpi_device *device) /* TBD: Locking */ list_add_tail(&root->node, &acpi_pci_roots); + /* + * Attach ACPI-PCI Context + * ----------------------- + * Thus binding the ACPI and PCI devices. + */ + result = acpi_pci_bind_root(device); + if (result) + goto end; + printk(KERN_INFO PREFIX "%s [%s] (domain %04x %pR)\n", acpi_device_name(device), acpi_device_bid(device), root->segment, &root->secondary); @@ -542,15 +535,6 @@ static int __devinit acpi_pci_root_add(struct acpi_device *device) } /* - * Attach ACPI-PCI Context - * ----------------------- - * Thus binding the ACPI and PCI devices. - */ - result = acpi_pci_bind_root(device); - if (result) - goto end; - - /* * PCI Routing Table * ----------------- * Evaluate and parse _PRT, if exists. @@ -559,12 +543,6 @@ static int __devinit acpi_pci_root_add(struct acpi_device *device) if (ACPI_SUCCESS(status)) result = acpi_pci_irq_add_prt(device->handle, root->bus); - /* - * Scan and bind all _ADR-Based Devices - */ - list_for_each_entry(child, &device->children, node) - acpi_pci_bridge_scan(child); - /* Indicate support for various _OSC capabilities. */ if (pci_ext_cfg_avail(root->bus->self)) flags |= OSC_EXT_PCI_CONFIG_SUPPORT;