From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751843Ab2LQHp7 (ORCPT ); Mon, 17 Dec 2012 02:45:59 -0500 Received: from hydra.sisk.pl ([212.160.235.94]:35971 "EHLO hydra.sisk.pl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751392Ab2LQHp6 (ORCPT ); Mon, 17 Dec 2012 02:45:58 -0500 From: "Rafael J. Wysocki" To: Yinghai Lu Cc: ACPI Devel Maling List , Greg Kroah-Hartman , LKML , Bjorn Helgaas , Benjamin Herrenschmidt , David Miller , tony.luck@intel.com, "H. Peter Anvin" , Jiang Liu Subject: Re: [PATCH] ACPI / PCI: Set root bridge ACPI handle in advance Date: Mon, 17 Dec 2012 08:51:05 +0100 Message-ID: <6849411.jDP7VSUtXm@vostro.rjw.lan> User-Agent: KMail/4.9.3 (Linux/3.7.0; KDE/4.9.3; x86_64; ; ) In-Reply-To: References: <1558789.gX8cmBgyDV@vostro.rjw.lan> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="utf-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sunday, December 16, 2012 09:27:49 PM Yinghai Lu wrote: > On Sun, Dec 16, 2012 at 2:25 PM, Rafael J. Wysocki wrote: > > From: Rafael J. Wysocki > > > > The ACPI handles of PCI root bridges need to be known to > > acpi_bind_one(), so that it can create the appropriate > > "firmware_node" and "physical_node" files for them, but currently > > the way it gets to know those handles is not exactly straightforward > > (to put it lightly). > > > > This is how it works, roughly: > > > > 1. acpi_bus_scan() finds the handle of a PCI root bridge, > > creates a struct acpi_device object for it and passes that > > object to acpi_pci_root_add(). > > > > 2. acpi_pci_root_add() creates a struct acpi_pci_root object, > > populates its "device" field with its argument's address > > (device->handle is the ACPI handle found in step 1). > > > > 3. The struct acpi_pci_root object created in step 2 is passed > > to pci_acpi_scan_root() and used to get resources that are > > passed to pci_create_root_bus(). > > > > 4. pci_create_root_bus() creates a struct pci_host_bridge object > > and passes its "dev" member to device_register(). > > > > 5. platform_notify(), which for systems with ACPI is set to > > acpi_platform_notify(), is called. > > > > So far, so good. Now it starts to be "interesting". > > > > 6. acpi_find_bridge_device() is used to find the ACPI handle of > > the given device (which is the PCI root bridge) and executes > > acpi_pci_find_root_bridge(), among other things, for the > > given device object. > > > > 7. acpi_pci_find_root_bridge() uses the name (sic!) of the given > > device object to extract the segment and bus numbers of the PCI > > root bridge and passes them to acpi_get_pci_rootbridge_handle(). > > > > 8. acpi_get_pci_rootbridge_handle() browses the list of ACPI PCI > > root bridges and finds the one that matches the given segment > > and bus numbers. Its handle is then used to initialize the > > ACPI handle of the PCI root bridge's device object by > > acpi_bind_one(). However, this is *exactly* the ACPI handle we > > started with in step 1. > > > > Needless to say, this is quite embarassing, but it may be avoided > > thanks to commit f3fd0c8 (ACPI: Allow ACPI handles of devices to be > > initialized in advance), which makes it possible to initialize the > > ACPI handle of a device before passing it to device_register(). > > Namely, if pci_acpi_scan_root() could easily pass the root bridge's > > ACPI handle to pci_create_root_bus(), the latter could set the ACPI > > handle in its struct pci_host_bridge object's "dev" member before > > passing it to device_register() and steps 6-8 above wouldn't be > > necessary any more. > > > > To make that happen I decided to repurpose the 4th argument of > > pci_create_root_bus(), because that allowed me to avoid defining > > additional callbacks or similar things and didn't seem to impact > > architectures without ACPI substantially. > > > > Only x86 and ia64 are affected directly, there should be no > > functional changes resulting from this on other architectures. > > that is good one to avoid that find_root_bridge... > > > > > Signed-off-by: Rafael J. Wysocki > > --- > > > > Should apply to the current Linus' tree, boots correctly on x86(-64). > > > > --- > > arch/ia64/pci/pci.c | 5 ++++- > > arch/powerpc/kernel/pci-common.c | 3 ++- > > arch/sparc/kernel/pci.c | 3 ++- > > arch/x86/pci/acpi.c | 5 ++++- > > drivers/acpi/pci_root.c | 18 ------------------ > > drivers/pci/pci-acpi.c | 19 ------------------- > > drivers/pci/probe.c | 16 +++++++++++----- > > include/acpi/acpi_bus.h | 1 - > > include/linux/pci.h | 9 ++++++++- > > 9 files changed, 31 insertions(+), 48 deletions(-) > > you need to update other arch for pci_create_root_bus > > arch/powerpc/kernel/pci-common.c: bus = > pci_create_root_bus(hose->parent, hose->first_busno, I thought I addressed this one, didn't I? > arch/s390/pci/pci.c: zdev->bus = pci_create_root_bus(NULL, > ZPCI_BUS_NR, &pci_root_ops, This one appears to have been removed. There's no pci_create_root_bus() in all arch/s390, as far as I can say. > arch/sparc/kernel/pci.c: bus = pci_create_root_bus(parent, > pbm->pci_first_busno, pbm->pci_ops, I modified this one too, is that not sufficient? > drivers/parisc/dino.c: dino_dev->hba.hba_bus = bus = > pci_create_root_bus(&dev->dev, > drivers/parisc/lba_pci.c: pci_create_root_bus(&dev->dev, > lba_dev->hba.bus_num.start, These two pass NULL as the 4th argument to pci_create_root_bus() and don't need to be updated, AFAICS. > > > > Index: linux/arch/x86/pci/acpi.c > > =================================================================== > > --- linux.orig/arch/x86/pci/acpi.c > > +++ linux/arch/x86/pci/acpi.c > > @@ -450,6 +450,7 @@ struct pci_bus * __devinit pci_acpi_scan > > LIST_HEAD(resources); > > struct pci_bus *bus = NULL; > > struct pci_sysdata *sd; > > + struct pci_root_sys_info si; > > int node; > > #ifdef CONFIG_ACPI_NUMA > > int pxm; > > @@ -486,6 +487,8 @@ struct pci_bus * __devinit pci_acpi_scan > > sd = &info->sd; > > sd->domain = domain; > > sd->node = node; > > + si.acpi_node.handle = device->handle; > > + si.sysdata = sd; > > maybe you can try to have si.acpi_handle directly ? I did it this way for handle to be compiled out when CONFIG_ACPI is not set (struct acpi_dev_node is an empty structure in that case). Thanks, Rafael -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center.