On Sun, Dec 16, 2012 at 11:51 PM, Rafael J. Wysocki <rjw@xxxxxxx> wrote: > On Sunday, December 16, 2012 09:27:49 PM Yinghai Lu wrote: >> On Sun, Dec 16, 2012 at 2:25 PM, Rafael J. Wysocki <rjw@xxxxxxx> wrote: >> > From: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx> >> > >> > 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 <rafael.j.wysocki@xxxxxxxxx> >> > --- >> > >> > 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. at least it is there on linus tree today. http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=blob;f=arch/s390/pci/pci.c;h=7ed38e5e3028689543c8c6356ef49b3a45546cd6;hb=HEAD line 890 > >> 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. then how could - b->sysdata = sysdata; + b->sysdata = sys_info->sysdata; be survived ? need to change to + b->sysdata = sys_info?sys_info->sysdata : NULL; > >> > >> > 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). ok. -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
