| Message ID | 20090602152456.7346.64853.stgit@bob.kio (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
On Tuesday 02 June 2009 09:24:56 am Alex Chiang wrote: > Convert an ACPI CA handle to a struct pci_dev. > > Performing this lookup dynamically allows us to get rid of the > ACPI-PCI binding code, which: > > - eliminates struct acpi_device vs struct pci_dev lifetime issues > - lays more groundwork for eliminating .start from acpi_device_ops > and thus simplifying ACPI drivers > - whacks out a lot of code > > This change also changes the time-space tradeoff ever so slightly. > > Looking up the ACPI-PCI binding is never in the performance path, and by > eliminating this caching, we save 24 bytes for each _ADR device in the > ACPI namespace. Just for future changelog readers, I think this space savings actually occurs in the "eviscerate pci_bind.c" patch where you remove the struct acpi_pci_data. I definitely agree that you're making the right time/space tradeoff. > Signed-off-by: Alex Chiang <achiang@hp.com> > --- > > drivers/acpi/pci_bind.c | 83 +++++++++++++++++++++++++++++++++++++++++++ > include/acpi/acpi_drivers.h | 1 + > 2 files changed, 84 insertions(+), 0 deletions(-) > > diff --git a/drivers/acpi/pci_bind.c b/drivers/acpi/pci_bind.c > index 236765c..584fa30 100644 > --- a/drivers/acpi/pci_bind.c > +++ b/drivers/acpi/pci_bind.c > @@ -56,6 +56,89 @@ static void acpi_pci_data_handler(acpi_handle handle, u32 function, > return; > } > > +struct acpi_handle_node { > + struct list_head node; > + acpi_handle handle; > +}; > + > +/** > + * acpi_get_pci_dev - convert ACPI CA handle to struct pci_dev > + * @handle: the handle in question > + * > + * Given an ACPI CA handle, the desired PCI device is located in the > + * list of PCI devices. > + * > + * If the device is found, its reference count is increased and this > + * function returns a pointer to its data structure. The caller must > + * decrement the reference count by calling pci_dev_put(). > + * If no device is found, %NULL is returned. > + */ > +struct pci_dev *acpi_get_pci_dev(acpi_handle handle) > +{ > + int dev, fn; > + unsigned long long adr; > + acpi_status status; > + acpi_handle phandle; > + struct pci_bus *pbus; > + struct pci_dev *pdev = NULL; > + struct acpi_handle_node *node, *tmp; > + struct acpi_pci_root *root; > + LIST_HEAD(device_list); > + > + /* > + * Walk up the ACPI CA namespace until we reach a PCI root bridge. > + */ > + phandle = handle; > + while (!acpi_is_root_bridge(phandle)) { > + node = kzalloc(sizeof(struct acpi_handle_node), GFP_KERNEL); > + if (!node) > + goto out; Since there's no cleanup to do at "out", a simple "return NULL" here is a bit more obvious. Whoops, I think there actually *is* a little cleanup to do -- I think we leak the list if we take these early error exits. > + > + INIT_LIST_HEAD(&node->node); > + node->handle = phandle; > + list_add(&node->node, &device_list); > + > + status = acpi_get_parent(phandle, &phandle); > + if (ACPI_FAILURE(status)) > + goto out; > + } > + > + root = acpi_pci_find_root(phandle); > + if (!root) > + goto out; > + > + pbus = pci_find_bus(root->id.segment, root->id.bus); > + if (!pbus) > + goto out; Isn't this simply "root->bus"? > + /* > + * Now, walk back down the PCI device tree until we return to our > + * original handle. Assumes that everything between the PCI root > + * bridge and the device we're looking for must be a P2P bridge. > + */ > + list_for_each_entry_safe(node, tmp, &device_list, node) { > + acpi_handle hnd = node->handle; > + status = acpi_evaluate_integer(hnd, "_ADR", NULL, &adr); > + if (ACPI_FAILURE(status)) > + goto out; > + dev = (adr >> 16) & 0xffff; > + fn = adr & 0xffff; > + > + list_del(&node->node); > + kfree(node); > + > + pdev = pci_get_slot(pbus, PCI_DEVFN(dev, fn)); > + if (hnd == handle) > + break; > + > + pbus = pdev->subordinate; > + pci_dev_put(pdev); > + } > +out: > + return pdev; > +} > +EXPORT_SYMBOL_GPL(acpi_get_pci_dev); > + > /** > * acpi_get_pci_id > * ------------------ > diff --git a/include/acpi/acpi_drivers.h b/include/acpi/acpi_drivers.h > index bbe9207..1ef529b 100644 > --- a/include/acpi/acpi_drivers.h > +++ b/include/acpi/acpi_drivers.h > @@ -97,6 +97,7 @@ void acpi_pci_irq_del_prt(int segment, int bus); > > struct pci_bus; > > +struct pci_dev *acpi_get_pci_dev(acpi_handle); > acpi_status acpi_get_pci_id(acpi_handle handle, struct acpi_pci_id *id); > int acpi_pci_bind_root(struct acpi_device *device, struct acpi_pci_id *id, > struct pci_bus *bus); > > -- > To unsubscribe from this list: send the line "unsubscribe linux-acpi" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- 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
diff --git a/drivers/acpi/pci_bind.c b/drivers/acpi/pci_bind.c index 236765c..584fa30 100644 --- a/drivers/acpi/pci_bind.c +++ b/drivers/acpi/pci_bind.c @@ -56,6 +56,89 @@ static void acpi_pci_data_handler(acpi_handle handle, u32 function, return; } +struct acpi_handle_node { + struct list_head node; + acpi_handle handle; +}; + +/** + * acpi_get_pci_dev - convert ACPI CA handle to struct pci_dev + * @handle: the handle in question + * + * Given an ACPI CA handle, the desired PCI device is located in the + * list of PCI devices. + * + * If the device is found, its reference count is increased and this + * function returns a pointer to its data structure. The caller must + * decrement the reference count by calling pci_dev_put(). + * If no device is found, %NULL is returned. + */ +struct pci_dev *acpi_get_pci_dev(acpi_handle handle) +{ + int dev, fn; + unsigned long long adr; + acpi_status status; + acpi_handle phandle; + struct pci_bus *pbus; + struct pci_dev *pdev = NULL; + struct acpi_handle_node *node, *tmp; + struct acpi_pci_root *root; + LIST_HEAD(device_list); + + /* + * Walk up the ACPI CA namespace until we reach a PCI root bridge. + */ + phandle = handle; + while (!acpi_is_root_bridge(phandle)) { + node = kzalloc(sizeof(struct acpi_handle_node), GFP_KERNEL); + if (!node) + goto out; + + INIT_LIST_HEAD(&node->node); + node->handle = phandle; + list_add(&node->node, &device_list); + + status = acpi_get_parent(phandle, &phandle); + if (ACPI_FAILURE(status)) + goto out; + } + + root = acpi_pci_find_root(phandle); + if (!root) + goto out; + + pbus = pci_find_bus(root->id.segment, root->id.bus); + if (!pbus) + goto out; + + /* + * Now, walk back down the PCI device tree until we return to our + * original handle. Assumes that everything between the PCI root + * bridge and the device we're looking for must be a P2P bridge. + */ + list_for_each_entry_safe(node, tmp, &device_list, node) { + acpi_handle hnd = node->handle; + status = acpi_evaluate_integer(hnd, "_ADR", NULL, &adr); + if (ACPI_FAILURE(status)) + goto out; + dev = (adr >> 16) & 0xffff; + fn = adr & 0xffff; + + list_del(&node->node); + kfree(node); + + pdev = pci_get_slot(pbus, PCI_DEVFN(dev, fn)); + if (hnd == handle) + break; + + pbus = pdev->subordinate; + pci_dev_put(pdev); + } +out: + return pdev; +} +EXPORT_SYMBOL_GPL(acpi_get_pci_dev); + /** * acpi_get_pci_id * ------------------ diff --git a/include/acpi/acpi_drivers.h b/include/acpi/acpi_drivers.h index bbe9207..1ef529b 100644 --- a/include/acpi/acpi_drivers.h +++ b/include/acpi/acpi_drivers.h @@ -97,6 +97,7 @@ void acpi_pci_irq_del_prt(int segment, int bus); struct pci_bus; +struct pci_dev *acpi_get_pci_dev(acpi_handle); acpi_status acpi_get_pci_id(acpi_handle handle, struct acpi_pci_id *id); int acpi_pci_bind_root(struct acpi_device *device, struct acpi_pci_id *id, struct pci_bus *bus);
Convert an ACPI CA handle to a struct pci_dev. Performing this lookup dynamically allows us to get rid of the ACPI-PCI binding code, which: - eliminates struct acpi_device vs struct pci_dev lifetime issues - lays more groundwork for eliminating .start from acpi_device_ops and thus simplifying ACPI drivers - whacks out a lot of code This change also changes the time-space tradeoff ever so slightly. Looking up the ACPI-PCI binding is never in the performance path, and by eliminating this caching, we save 24 bytes for each _ADR device in the ACPI namespace. Signed-off-by: Alex Chiang <achiang@hp.com> --- drivers/acpi/pci_bind.c | 83 +++++++++++++++++++++++++++++++++++++++++++ include/acpi/acpi_drivers.h | 1 + 2 files changed, 84 insertions(+), 0 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