This patch series eliminates static boot-time binding of ACPI
and PCI devices, and introduces an API to perform this lookup
during runtime.
This change has the following advantages:
- eliminates struct acpi_device vs struct pci_dev lifetime issues
- lays groundwork for eliminating .bind/.unbind from acpi_device_ops
- lays more groundwork for eliminating .start from acpi_device_ops
and thus simplifying ACPI drivers
- whacks out a lot of code
This patchset is based on lenb/test (66c74fa1d4).
---
Alex Chiang (10):
ACPI: acpi_pci_unbind should clean up properly after acpi_pci_bind
ACPI: simplify acpi_pci_irq_del_prt() API
ACPI: simplify acpi_pci_irq_add_prt() API
ACPI: eviscerate pci_bind.c
ACPI: kill acpi_get_pci_id
PCI Hotplug: acpiphp: convert to acpi_get_pci_dev
ACPI: Introduce acpi_get_pci_dev()
ACPI: export acpi_pci_find_root()
ACPI: Introduce acpi_is_root_bridge()
ACPI: make acpi_pci_bind() static
drivers/acpi/pci_bind.c | 374 +++++++++++-------------------------
drivers/acpi/pci_irq.c | 17 +-
drivers/acpi/pci_root.c | 83 +++++---
drivers/pci/hotplug/acpi_pcihp.c | 40 ----
drivers/pci/hotplug/acpiphp_glue.c | 26 +--
include/acpi/acpi_bus.h | 14 +
include/acpi/acpi_drivers.h | 10 -
include/linux/pci_hotplug.h | 1
8 files changed, 202 insertions(+), 363 deletions(-)
acpi_pci_root_add() explicitly assigns device->ops.bind, and later
calls acpi_pci_bind_root(), which also does the same thing.
We don't need to repeat ourselves; removing the explicit assignment
allows us to make acpi_pci_bind() static.
Signed-off-by: Alex Chiang <[email protected]>
---
drivers/acpi/pci_bind.c | 3 ++-
drivers/acpi/pci_root.c | 2 --
include/acpi/acpi_drivers.h | 1 -
3 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/acpi/pci_bind.c b/drivers/acpi/pci_bind.c
index bc46de3..236765c 100644
--- a/drivers/acpi/pci_bind.c
+++ b/drivers/acpi/pci_bind.c
@@ -44,6 +44,7 @@ struct acpi_pci_data {
struct pci_dev *dev;
};
+static int acpi_pci_bind(struct acpi_device *device);
static int acpi_pci_unbind(struct acpi_device *device);
static void acpi_pci_data_handler(acpi_handle handle, u32 function,
@@ -108,7 +109,7 @@ acpi_status acpi_get_pci_id(acpi_handle handle, struct acpi_pci_id *id)
EXPORT_SYMBOL(acpi_get_pci_id);
-int acpi_pci_bind(struct acpi_device *device)
+static int acpi_pci_bind(struct acpi_device *device)
{
int result = 0;
acpi_status status;
diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index 196f97d..ca8dba3 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -386,8 +386,6 @@ static int __devinit acpi_pci_root_add(struct acpi_device *device)
strcpy(acpi_device_class(device), ACPI_PCI_ROOT_CLASS);
device->driver_data = root;
- device->ops.bind = acpi_pci_bind;
-
/*
* All supported architectures that use ACPI have support for
* PCI domains, so we indicate this in _OSC support capabilities.
diff --git a/include/acpi/acpi_drivers.h b/include/acpi/acpi_drivers.h
index 5e8ed3a..bbe9207 100644
--- a/include/acpi/acpi_drivers.h
+++ b/include/acpi/acpi_drivers.h
@@ -98,7 +98,6 @@ void acpi_pci_irq_del_prt(int segment, int bus);
struct pci_bus;
acpi_status acpi_get_pci_id(acpi_handle handle, struct acpi_pci_id *id);
-int acpi_pci_bind(struct acpi_device *device);
int acpi_pci_bind_root(struct acpi_device *device, struct acpi_pci_id *id,
struct pci_bus *bus);
Returns whether an ACPI CA node is a PCI root bridge or not.
This API is generically useful, and shouldn't just be a hotplug function.
Note that this patch does not simply move code around. The old
implementation did not check for PCIe _HIDs. The new implementation
does check, similar to the logic in acpi_ev_is_pci_root_bridge().
Signed-off-by: Alex Chiang <[email protected]>
---
drivers/acpi/pci_root.c | 41 ++++++++++++++++++++++++++++++++++++
drivers/pci/hotplug/acpi_pcihp.c | 40 ++---------------------------------
drivers/pci/hotplug/acpiphp_glue.c | 2 +-
include/acpi/acpi_bus.h | 1 +
include/linux/pci_hotplug.h | 1 -
5 files changed, 45 insertions(+), 40 deletions(-)
diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index ca8dba3..853a6e4 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -142,6 +142,47 @@ acpi_handle acpi_get_pci_rootbridge_handle(unsigned int seg, unsigned int bus)
EXPORT_SYMBOL_GPL(acpi_get_pci_rootbridge_handle);
+/**
+ * acpi_is_root_bridge - determine whether an ACPI CA node is a PCI root bridge
+ * @handle - the ACPI CA node in question.
+ */
+int acpi_is_root_bridge(acpi_handle handle)
+{
+ acpi_status status;
+ struct acpi_device_info *info;
+ struct acpi_buffer buffer = {ACPI_ALLOCATE_BUFFER, NULL};
+ int i, ret = 0;
+
+ status = acpi_get_object_info(handle, &buffer);
+ if (ACPI_FAILURE(status))
+ goto out;
+
+ info = buffer.pointer;
+ if ((info->valid & ACPI_VALID_HID) &&
+ !strcmp(PCI_ROOT_HID_STRING, info->hardware_id.value)) {
+ ret = 1;
+ goto out;
+ }
+ if ((info->valid & ACPI_VALID_HID) &&
+ !strcmp(PCI_EXPRESS_ROOT_HID_STRING, info->hardware_id.value)) {
+ ret = 1;
+ goto out;
+ }
+ if (info->valid & ACPI_VALID_CID) {
+ for (i = 0; i < info->compatibility_id.count; i++) {
+ if (!strcmp(PCI_ROOT_HID_STRING,
+ info->compatibility_id.id[i].value)) {
+ ret = 1;
+ goto out;
+ }
+ }
+ }
+out:
+ kfree(buffer.pointer);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(acpi_is_root_bridge);
+
static acpi_status
get_root_bridge_busnr_callback(struct acpi_resource *resource, void *data)
{
diff --git a/drivers/pci/hotplug/acpi_pcihp.c b/drivers/pci/hotplug/acpi_pcihp.c
index fbc63d5..eb15958 100644
--- a/drivers/pci/hotplug/acpi_pcihp.c
+++ b/drivers/pci/hotplug/acpi_pcihp.c
@@ -354,7 +354,7 @@ acpi_status acpi_get_hp_params_from_firmware(struct pci_bus *bus,
status = acpi_run_hpp(handle, hpp);
if (ACPI_SUCCESS(status))
break;
- if (acpi_root_bridge(handle))
+ if (acpi_is_root_bridge(handle))
break;
status = acpi_get_parent(handle, &phandle);
if (ACPI_FAILURE(status))
@@ -428,7 +428,7 @@ int acpi_get_hp_hw_control_from_firmware(struct pci_dev *pdev, u32 flags)
status = acpi_run_oshp(handle);
if (ACPI_SUCCESS(status))
goto got_one;
- if (acpi_root_bridge(handle))
+ if (acpi_is_root_bridge(handle))
break;
chandle = handle;
status = acpi_get_parent(chandle, &handle);
@@ -449,42 +449,6 @@ got_one:
}
EXPORT_SYMBOL(acpi_get_hp_hw_control_from_firmware);
-/* acpi_root_bridge - check to see if this acpi object is a root bridge
- *
- * @handle - the acpi object in question.
- */
-int acpi_root_bridge(acpi_handle handle)
-{
- acpi_status status;
- struct acpi_device_info *info;
- struct acpi_buffer buffer = {ACPI_ALLOCATE_BUFFER, NULL};
- int i;
-
- status = acpi_get_object_info(handle, &buffer);
- if (ACPI_SUCCESS(status)) {
- info = buffer.pointer;
- if ((info->valid & ACPI_VALID_HID) &&
- !strcmp(PCI_ROOT_HID_STRING,
- info->hardware_id.value)) {
- kfree(buffer.pointer);
- return 1;
- }
- if (info->valid & ACPI_VALID_CID) {
- for (i=0; i < info->compatibility_id.count; i++) {
- if (!strcmp(PCI_ROOT_HID_STRING,
- info->compatibility_id.id[i].value)) {
- kfree(buffer.pointer);
- return 1;
- }
- }
- }
- kfree(buffer.pointer);
- }
- return 0;
-}
-EXPORT_SYMBOL_GPL(acpi_root_bridge);
-
-
static int is_ejectable(acpi_handle handle)
{
acpi_status status;
diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c
index 3a6064b..fc6636e 100644
--- a/drivers/pci/hotplug/acpiphp_glue.c
+++ b/drivers/pci/hotplug/acpiphp_glue.c
@@ -1631,7 +1631,7 @@ find_root_bridges(acpi_handle handle, u32 lvl, void *context, void **rv)
{
int *count = (int *)context;
- if (acpi_root_bridge(handle)) {
+ if (acpi_is_root_bridge(handle)) {
acpi_install_notify_handler(handle, ACPI_SYSTEM_NOTIFY,
handle_hotplug_event_bridge, NULL);
(*count)++;
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index 0f50167..494088b 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -372,6 +372,7 @@ struct device *acpi_get_physical_pci_device(acpi_handle);
/* helper */
acpi_handle acpi_get_child(acpi_handle, acpi_integer);
+int acpi_is_root_bridge(acpi_handle);
acpi_handle acpi_get_pci_rootbridge_handle(unsigned int, unsigned int);
#define DEVICE_ACPI_HANDLE(dev) ((acpi_handle)((dev)->archdata.acpi_handle))
diff --git a/include/linux/pci_hotplug.h b/include/linux/pci_hotplug.h
index 2099874..a3576ef 100644
--- a/include/linux/pci_hotplug.h
+++ b/include/linux/pci_hotplug.h
@@ -226,7 +226,6 @@ struct hotplug_params {
extern acpi_status acpi_get_hp_params_from_firmware(struct pci_bus *bus,
struct hotplug_params *hpp);
int acpi_get_hp_hw_control_from_firmware(struct pci_dev *dev, u32 flags);
-int acpi_root_bridge(acpi_handle handle);
int acpi_pci_check_ejectable(struct pci_bus *pbus, acpi_handle handle);
int acpi_pci_detect_ejectable(struct pci_bus *pbus);
#endif
This function is useful for pci_bind.c as well.
Signed-off-by: Alex Chiang <[email protected]>
---
drivers/acpi/pci_root.c | 33 ++++++++++-----------------------
include/acpi/acpi_bus.h | 13 +++++++++++++
2 files changed, 23 insertions(+), 23 deletions(-)
diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index 853a6e4..bcab69a 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -61,19 +61,6 @@ static struct acpi_driver acpi_pci_root_driver = {
},
};
-struct acpi_pci_root {
- struct list_head node;
- struct acpi_device * device;
- struct acpi_pci_id id;
- struct pci_bus *bus;
-
- u32 osc_support_set; /* _OSC state of support bits */
- u32 osc_control_set; /* _OSC state of control bits */
- u32 osc_control_qry; /* the latest _OSC query result */
-
- u32 osc_queried:1; /* has _OSC control been queried? */
-};
-
static LIST_HEAD(acpi_pci_roots);
static struct acpi_pci_driver *sub_driver;
@@ -183,6 +170,16 @@ out:
}
EXPORT_SYMBOL_GPL(acpi_is_root_bridge);
+struct acpi_pci_root *acpi_pci_find_root(acpi_handle handle)
+{
+ struct acpi_pci_root *root;
+ list_for_each_entry(root, &acpi_pci_roots, node) {
+ if (root->device->handle == handle)
+ return root;
+ }
+ return NULL;
+}
+
static acpi_status
get_root_bridge_busnr_callback(struct acpi_resource *resource, void *data)
{
@@ -336,16 +333,6 @@ static acpi_status acpi_pci_osc_support(struct acpi_pci_root *root, u32 flags)
return status;
}
-static struct acpi_pci_root *acpi_pci_find_root(acpi_handle handle)
-{
- struct acpi_pci_root *root;
- list_for_each_entry(root, &acpi_pci_roots, node) {
- if (root->device->handle == handle)
- return root;
- }
- return NULL;
-}
-
/**
* acpi_pci_osc_control_set - commit requested control to Firmware
* @handle: acpi_handle for the target ACPI object
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index 494088b..38f2bff 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -371,9 +371,22 @@ struct device *acpi_get_physical_device(acpi_handle);
struct device *acpi_get_physical_pci_device(acpi_handle);
/* helper */
+struct acpi_pci_root {
+ struct list_head node;
+ struct acpi_device *device;
+ struct acpi_pci_id id;
+ struct pci_bus *bus;
+
+ u32 osc_support_set; /* _OSC state of support bits */
+ u32 osc_control_set; /* _OSC state of control bits */
+ u32 osc_control_qry; /* the latest _OSC query result */
+
+ u32 osc_queried:1; /* has _OSC control been queried? */
+};
acpi_handle acpi_get_child(acpi_handle, acpi_integer);
int acpi_is_root_bridge(acpi_handle);
acpi_handle acpi_get_pci_rootbridge_handle(unsigned int, unsigned int);
+struct acpi_pci_root *acpi_pci_find_root(acpi_handle);
#define DEVICE_ACPI_HANDLE(dev) ((acpi_handle)((dev)->archdata.acpi_handle))
#ifdef CONFIG_PM_SLEEP
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 <[email protected]>
---
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;
+
+ 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);
Now that acpi_get_pci_dev is available, let's use it instead of
acpi_get_pci_id.
Signed-off-by: Alex Chiang <[email protected]>
---
drivers/pci/hotplug/acpiphp_glue.c | 24 ++++++------------------
1 files changed, 6 insertions(+), 18 deletions(-)
diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c
index fc6636e..a288e4e 100644
--- a/drivers/pci/hotplug/acpiphp_glue.c
+++ b/drivers/pci/hotplug/acpiphp_glue.c
@@ -678,18 +678,9 @@ static void remove_bridge(acpi_handle handle)
static struct pci_dev * get_apic_pci_info(acpi_handle handle)
{
- struct acpi_pci_id id;
- struct pci_bus *bus;
struct pci_dev *dev;
- if (ACPI_FAILURE(acpi_get_pci_id(handle, &id)))
- return NULL;
-
- bus = pci_find_bus(id.segment, id.bus);
- if (!bus)
- return NULL;
-
- dev = pci_get_slot(bus, PCI_DEVFN(id.device, id.function));
+ dev = acpi_get_pci_dev(handle);
if (!dev)
return NULL;
@@ -1396,19 +1387,16 @@ static void acpiphp_sanitize_bus(struct pci_bus *bus)
/* Program resources in newly inserted bridge */
static int acpiphp_configure_bridge (acpi_handle handle)
{
- struct acpi_pci_id pci_id;
+ struct pci_dev *dev;
struct pci_bus *bus;
- if (ACPI_FAILURE(acpi_get_pci_id(handle, &pci_id))) {
+ dev = acpi_get_pci_dev(handle);
+ if (!dev) {
err("cannot get PCI domain and bus number for bridge\n");
return -EINVAL;
}
- bus = pci_find_bus(pci_id.segment, pci_id.bus);
- if (!bus) {
- err("cannot find bus %d:%d\n",
- pci_id.segment, pci_id.bus);
- return -EINVAL;
- }
+
+ bus = dev->bus;
pci_bus_size_bridges(bus);
pci_bus_assign_resources(bus);
There is no need to pass a segment/bus tuple to this API, as the callsite
always has a struct pci_bus. We can derive segment/bus from the
struct pci_bus, so let's take this opportunit to simplify the API and
make life easier for the callers.
Cc: Bjorn Helgaas <[email protected]>
Signed-off-by: Alex Chiang <[email protected]>
---
drivers/acpi/pci_bind.c | 3 +--
drivers/acpi/pci_irq.c | 7 ++++---
include/acpi/acpi_drivers.h | 2 +-
3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/acpi/pci_bind.c b/drivers/acpi/pci_bind.c
index 96264d2..9185b54 100644
--- a/drivers/acpi/pci_bind.c
+++ b/drivers/acpi/pci_bind.c
@@ -127,8 +127,7 @@ static int acpi_pci_unbind(struct acpi_device *device)
return 0;
if (dev->subordinate)
- acpi_pci_irq_del_prt(pci_domain_nr(dev->bus),
- dev->subordinate->number);
+ acpi_pci_irq_del_prt(dev->subordinate);
return 0;
}
diff --git a/drivers/acpi/pci_irq.c b/drivers/acpi/pci_irq.c
index 3ed944c..ef9509e 100644
--- a/drivers/acpi/pci_irq.c
+++ b/drivers/acpi/pci_irq.c
@@ -280,16 +280,17 @@ int acpi_pci_irq_add_prt(acpi_handle handle, struct pci_bus *bus)
return 0;
}
-void acpi_pci_irq_del_prt(int segment, int bus)
+void acpi_pci_irq_del_prt(struct pci_bus *bus)
{
struct acpi_prt_entry *entry, *tmp;
printk(KERN_DEBUG
"ACPI: Delete PCI Interrupt Routing Table for %04x:%02x\n",
- segment, bus);
+ pci_domain_nr(bus), bus->number);
spin_lock(&acpi_prt_lock);
list_for_each_entry_safe(entry, tmp, &acpi_prt_list, list) {
- if (segment == entry->id.segment && bus == entry->id.bus) {
+ if (pci_domain_nr(bus) == entry->id.segment
+ && bus->number == entry->id.bus) {
list_del(&entry->list);
kfree(entry);
}
diff --git a/include/acpi/acpi_drivers.h b/include/acpi/acpi_drivers.h
index c21d83f..f4906f6 100644
--- a/include/acpi/acpi_drivers.h
+++ b/include/acpi/acpi_drivers.h
@@ -91,7 +91,7 @@ int acpi_pci_link_free_irq(acpi_handle handle);
/* ACPI PCI Interrupt Routing (pci_irq.c) */
int acpi_pci_irq_add_prt(acpi_handle handle, struct pci_bus *bus);
-void acpi_pci_irq_del_prt(int segment, int bus);
+void acpi_pci_irq_del_prt(struct pci_bus *bus);
/* ACPI PCI Device Binding (pci_bind.c) */
acpi_get_pci_dev() is better, and all callers have been converted, so
eliminate acpi_get_pci_id().
Signed-off-by: Alex Chiang <[email protected]>
---
drivers/acpi/pci_bind.c | 52 -------------------------------------------
include/acpi/acpi_drivers.h | 1 -
2 files changed, 0 insertions(+), 53 deletions(-)
diff --git a/drivers/acpi/pci_bind.c b/drivers/acpi/pci_bind.c
index 584fa30..10e3ffc 100644
--- a/drivers/acpi/pci_bind.c
+++ b/drivers/acpi/pci_bind.c
@@ -139,58 +139,6 @@ out:
}
EXPORT_SYMBOL_GPL(acpi_get_pci_dev);
-/**
- * acpi_get_pci_id
- * ------------------
- * This function is used by the ACPI Interpreter (a.k.a. Core Subsystem)
- * to resolve PCI information for ACPI-PCI devices defined in the namespace.
- * This typically occurs when resolving PCI operation region information.
- */
-acpi_status acpi_get_pci_id(acpi_handle handle, struct acpi_pci_id *id)
-{
- int result = 0;
- acpi_status status = AE_OK;
- struct acpi_device *device = NULL;
- struct acpi_pci_data *data = NULL;
-
-
- if (!id)
- return AE_BAD_PARAMETER;
-
- result = acpi_bus_get_device(handle, &device);
- if (result) {
- printk(KERN_ERR PREFIX
- "Invalid ACPI Bus context for device %s\n",
- acpi_device_bid(device));
- return AE_NOT_EXIST;
- }
-
- status = acpi_get_data(handle, acpi_pci_data_handler, (void **)&data);
- if (ACPI_FAILURE(status) || !data) {
- ACPI_EXCEPTION((AE_INFO, status,
- "Invalid ACPI-PCI context for device %s",
- acpi_device_bid(device)));
- return status;
- }
-
- *id = data->id;
-
- /*
- id->segment = data->id.segment;
- id->bus = data->id.bus;
- id->device = data->id.device;
- id->function = data->id.function;
- */
-
- ACPI_DEBUG_PRINT((ACPI_DB_INFO,
- "Device %s has PCI address %04x:%02x:%02x.%d\n",
- acpi_device_bid(device), id->segment, id->bus,
- id->device, id->function));
-
- return AE_OK;
-}
-
-EXPORT_SYMBOL(acpi_get_pci_id);
static int acpi_pci_bind(struct acpi_device *device)
{
diff --git a/include/acpi/acpi_drivers.h b/include/acpi/acpi_drivers.h
index 1ef529b..310f5ff 100644
--- a/include/acpi/acpi_drivers.h
+++ b/include/acpi/acpi_drivers.h
@@ -98,7 +98,6 @@ 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);
In acpi_pci_bind, we add _PRT information for non-bridge devices, but
we never delete that information in acpi_pci_unbind.
We also set device->ops.bind and device->ops.unbind, but never clear
them out.
Let's make acpi_pci_unbind clean up what we did in acpi_pci_bind.
Cc: Bjorn Helgaas <[email protected]>
Signed-off-by: Alex Chiang <[email protected]>
---
drivers/acpi/pci_bind.c | 11 +++++++++--
1 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/acpi/pci_bind.c b/drivers/acpi/pci_bind.c
index 9185b54..c507e37 100644
--- a/drivers/acpi/pci_bind.c
+++ b/drivers/acpi/pci_bind.c
@@ -120,14 +120,21 @@ EXPORT_SYMBOL_GPL(acpi_get_pci_dev);
static int acpi_pci_unbind(struct acpi_device *device)
{
+ struct pci_bus *bus;
struct pci_dev *dev;
dev = acpi_get_pci_dev(device->handle);
if (!dev)
return 0;
- if (dev->subordinate)
- acpi_pci_irq_del_prt(dev->subordinate);
+ if (dev->subordinate) {
+ bus = dev->subordinate;
+ device->ops.bind = NULL;
+ device->ops.unbind = NULL;
+ } else
+ bus = dev->bus;
+
+ acpi_pci_irq_del_prt(bus);
return 0;
}
Now that we can dynamically convert an ACPI CA handle to a
struct pci_dev at runtime, there's no need to statically bind
them during boot.
acpi_pci_bind/unbind are vastly simplified, and are only used
to evaluate _PRT methods on P2P bridges and non-bridge children.
This patchset lays further groundwork to eventually eliminate
the acpi_driver_ops.bind callback.
Cc: Bjorn Helgaas <[email protected]>
Signed-off-by: Alex Chiang <[email protected]>
---
drivers/acpi/pci_bind.c | 268 ++++++-------------------------------------
drivers/acpi/pci_root.c | 2
include/acpi/acpi_drivers.h | 3
3 files changed, 41 insertions(+), 232 deletions(-)
diff --git a/drivers/acpi/pci_bind.c b/drivers/acpi/pci_bind.c
index 10e3ffc..0469cf2 100644
--- a/drivers/acpi/pci_bind.c
+++ b/drivers/acpi/pci_bind.c
@@ -3,6 +3,8 @@
*
* Copyright (C) 2001, 2002 Andy Grover <[email protected]>
* Copyright (C) 2001, 2002 Paul Diefenbaugh <[email protected]>
+ * Copyright (C) 2009 Hewlett-Packard Development Company, L.P.
+ * Alex Chiang <[email protected]>
*
* ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
*
@@ -24,12 +26,7 @@
*/
#include <linux/kernel.h>
-#include <linux/module.h>
-#include <linux/init.h>
#include <linux/types.h>
-#include <linux/proc_fs.h>
-#include <linux/spinlock.h>
-#include <linux/pm.h>
#include <linux/pci.h>
#include <linux/acpi.h>
#include <acpi/acpi_bus.h>
@@ -38,24 +35,6 @@
#define _COMPONENT ACPI_PCI_COMPONENT
ACPI_MODULE_NAME("pci_bind");
-struct acpi_pci_data {
- struct acpi_pci_id id;
- struct pci_bus *bus;
- struct pci_dev *dev;
-};
-
-static int acpi_pci_bind(struct acpi_device *device);
-static int acpi_pci_unbind(struct acpi_device *device);
-
-static void acpi_pci_data_handler(acpi_handle handle, u32 function,
- void *context)
-{
-
- /* TBD: Anything we need to do here? */
-
- return;
-}
-
struct acpi_handle_node {
struct list_head node;
acpi_handle handle;
@@ -139,241 +118,72 @@ out:
}
EXPORT_SYMBOL_GPL(acpi_get_pci_dev);
-
-static int acpi_pci_bind(struct acpi_device *device)
+static int acpi_pci_unbind(struct acpi_device *device)
{
- int result = 0;
- acpi_status status;
- struct acpi_pci_data *data;
- struct acpi_pci_data *pdata;
- struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
- acpi_handle handle;
-
- if (!device || !device->parent)
- return -EINVAL;
-
- data = kzalloc(sizeof(struct acpi_pci_data), GFP_KERNEL);
- if (!data)
- return -ENOMEM;
-
- status = acpi_get_name(device->handle, ACPI_FULL_PATHNAME, &buffer);
- if (ACPI_FAILURE(status)) {
- kfree(data);
- return -ENODEV;
- }
-
- ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Binding PCI device [%s]...\n",
- (char *)buffer.pointer));
+ struct pci_dev *dev;
- /*
- * Segment & Bus
- * -------------
- * These are obtained via the parent device's ACPI-PCI context.
- */
- status = acpi_get_data(device->parent->handle, acpi_pci_data_handler,
- (void **)&pdata);
- if (ACPI_FAILURE(status) || !pdata || !pdata->bus) {
- ACPI_EXCEPTION((AE_INFO, status,
- "Invalid ACPI-PCI context for parent device %s",
- acpi_device_bid(device->parent)));
- result = -ENODEV;
- goto end;
- }
- data->id.segment = pdata->id.segment;
- data->id.bus = pdata->bus->number;
+ dev = acpi_get_pci_dev(device->handle);
+ if (!dev)
+ return 0;
- /*
- * Device & Function
- * -----------------
- * These are simply obtained from the device's _ADR method. Note
- * that a value of zero is valid.
- */
- data->id.device = device->pnp.bus_address >> 16;
- data->id.function = device->pnp.bus_address & 0xFFFF;
+ if (dev->subordinate)
+ acpi_pci_irq_del_prt(pci_domain_nr(dev->bus),
+ dev->subordinate->number);
- ACPI_DEBUG_PRINT((ACPI_DB_INFO, "...to %04x:%02x:%02x.%d\n",
- data->id.segment, data->id.bus, data->id.device,
- data->id.function));
+ return 0;
+}
- /*
- * TBD: Support slot devices (e.g. function=0xFFFF).
- */
+static int acpi_pci_bind(struct acpi_device *device)
+{
+ acpi_status status;
+ acpi_handle handle;
+ unsigned char bus;
+ struct pci_dev *dev;
- /*
- * Locate PCI Device
- * -----------------
- * Locate matching device in PCI namespace. If it doesn't exist
- * this typically means that the device isn't currently inserted
- * (e.g. docking station, port replicator, etc.).
- */
- data->dev = pci_get_slot(pdata->bus,
- PCI_DEVFN(data->id.device, data->id.function));
- if (!data->dev) {
- ACPI_DEBUG_PRINT((ACPI_DB_INFO,
- "Device %04x:%02x:%02x.%d not present in PCI namespace\n",
- data->id.segment, data->id.bus,
- data->id.device, data->id.function));
- result = -ENODEV;
- goto end;
- }
- if (!data->dev->bus) {
- printk(KERN_ERR PREFIX
- "Device %04x:%02x:%02x.%d has invalid 'bus' field\n",
- data->id.segment, data->id.bus,
- data->id.device, data->id.function);
- result = -ENODEV;
- goto end;
- }
+ dev = acpi_get_pci_dev(device->handle);
+ if (!dev)
+ return 0;
/*
- * PCI Bridge?
- * -----------
- * If so, set the 'bus' field and install the 'bind' function to
- * facilitate callbacks for all of its children.
+ * Install the 'bind' function to facilitate callbacks for
+ * children of the P2P bridge.
*/
- if (data->dev->subordinate) {
+ if (dev->subordinate) {
ACPI_DEBUG_PRINT((ACPI_DB_INFO,
"Device %04x:%02x:%02x.%d is a PCI bridge\n",
- data->id.segment, data->id.bus,
- data->id.device, data->id.function));
- data->bus = data->dev->subordinate;
+ 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;
}
/*
- * Attach ACPI-PCI Context
- * -----------------------
- * Thus binding the ACPI and PCI devices.
- */
- status = acpi_attach_data(device->handle, acpi_pci_data_handler, data);
- if (ACPI_FAILURE(status)) {
- ACPI_EXCEPTION((AE_INFO, status,
- "Unable to attach ACPI-PCI context to device %s",
- acpi_device_bid(device)));
- result = -ENODEV;
- goto end;
- }
-
- /*
- * PCI Routing Table
- * -----------------
- * Evaluate and parse _PRT, if exists. This code is independent of
- * PCI bridges (above) to allow parsing of _PRT objects within the
- * scope of non-bridge devices. Note that _PRTs within the scope of
- * a PCI bridge assume the bridge's subordinate bus number.
+ * Evaluate and parse _PRT, if exists. This code allows parsing of
+ * _PRT objects within the scope of non-bridge devices. Note that
+ * _PRTs within the scope of a PCI bridge assume the bridge's
+ * subordinate bus number.
*
* TBD: Can _PRTs exist within the scope of non-bridge PCI devices?
*/
status = acpi_get_handle(device->handle, METHOD_NAME__PRT, &handle);
- if (ACPI_SUCCESS(status)) {
- if (data->bus) /* PCI-PCI bridge */
- acpi_pci_irq_add_prt(device->handle, data->id.segment,
- data->bus->number);
- else /* non-bridge PCI device */
- acpi_pci_irq_add_prt(device->handle, data->id.segment,
- data->id.bus);
- }
-
- end:
- kfree(buffer.pointer);
- if (result) {
- pci_dev_put(data->dev);
- kfree(data);
- }
- return result;
-}
-
-static int acpi_pci_unbind(struct acpi_device *device)
-{
- int result = 0;
- acpi_status status;
- struct acpi_pci_data *data;
- struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
-
-
- if (!device || !device->parent)
- return -EINVAL;
-
- status = acpi_get_name(device->handle, ACPI_FULL_PATHNAME, &buffer);
if (ACPI_FAILURE(status))
- return -ENODEV;
-
- ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Unbinding PCI device [%s]...\n",
- (char *) buffer.pointer));
- kfree(buffer.pointer);
+ return 0;
- status =
- acpi_get_data(device->handle, acpi_pci_data_handler,
- (void **)&data);
- if (ACPI_FAILURE(status)) {
- result = -ENODEV;
- goto end;
- }
+ if (dev->subordinate)
+ bus = dev->subordinate->number;
+ else
+ bus = dev->bus->number;
- status = acpi_detach_data(device->handle, acpi_pci_data_handler);
- if (ACPI_FAILURE(status)) {
- ACPI_EXCEPTION((AE_INFO, status,
- "Unable to detach data from device %s",
- acpi_device_bid(device)));
- result = -ENODEV;
- goto end;
- }
- if (data->dev->subordinate) {
- acpi_pci_irq_del_prt(data->id.segment, data->bus->number);
- }
- pci_dev_put(data->dev);
- kfree(data);
+ acpi_pci_irq_add_prt(device->handle, pci_domain_nr(dev->bus), bus);
- end:
- return result;
+ return 0;
}
int
-acpi_pci_bind_root(struct acpi_device *device,
- struct acpi_pci_id *id, struct pci_bus *bus)
+acpi_pci_bind_root(struct acpi_device *device)
{
- int result = 0;
- acpi_status status;
- struct acpi_pci_data *data = NULL;
- struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
-
- if (!device || !id || !bus) {
- return -EINVAL;
- }
-
- data = kzalloc(sizeof(struct acpi_pci_data), GFP_KERNEL);
- if (!data)
- return -ENOMEM;
-
- data->id = *id;
- data->bus = bus;
device->ops.bind = acpi_pci_bind;
device->ops.unbind = acpi_pci_unbind;
- status = acpi_get_name(device->handle, ACPI_FULL_PATHNAME, &buffer);
- if (ACPI_FAILURE(status)) {
- kfree (data);
- return -ENODEV;
- }
-
- ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Binding PCI root bridge [%s] to "
- "%04x:%02x\n", (char *)buffer.pointer,
- id->segment, id->bus));
-
- status = acpi_attach_data(device->handle, acpi_pci_data_handler, data);
- if (ACPI_FAILURE(status)) {
- ACPI_EXCEPTION((AE_INFO, status,
- "Unable to attach ACPI-PCI context to device %s",
- (char *)buffer.pointer));
- result = -ENODEV;
- goto end;
- }
-
- end:
- kfree(buffer.pointer);
- if (result != 0)
- kfree(data);
-
- return result;
+ return 0;
}
diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index bcab69a..25ddbb6 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -526,7 +526,7 @@ static int __devinit acpi_pci_root_add(struct acpi_device *device)
* -----------------------
* Thus binding the ACPI and PCI devices.
*/
- result = acpi_pci_bind_root(device, &root->id, root->bus);
+ result = acpi_pci_bind_root(device);
if (result)
goto end;
diff --git a/include/acpi/acpi_drivers.h b/include/acpi/acpi_drivers.h
index 310f5ff..1c50f4f 100644
--- a/include/acpi/acpi_drivers.h
+++ b/include/acpi/acpi_drivers.h
@@ -98,8 +98,7 @@ void acpi_pci_irq_del_prt(int segment, int bus);
struct pci_bus;
struct pci_dev *acpi_get_pci_dev(acpi_handle);
-int acpi_pci_bind_root(struct acpi_device *device, struct acpi_pci_id *id,
- struct pci_bus *bus);
+int acpi_pci_bind_root(struct acpi_device *device);
/* Arch-defined function to add a bus to the system */
A PCI domain cannot change as you descend down subordinate buses, which
makes the 'segment' argument to acpi_pci_irq_add_prt() useless.
Change the interface to take a struct pci_bus *, from whence we can derive
the bus number and segment. Reducing the number of arguments makes life
simpler for callers.
Cc: Bjorn Helgaas <[email protected]>
Signed-off-by: Alex Chiang <[email protected]>
---
drivers/acpi/pci_bind.c | 8 ++++----
drivers/acpi/pci_irq.c | 10 +++++-----
drivers/acpi/pci_root.c | 5 +++--
include/acpi/acpi_drivers.h | 2 +-
4 files changed, 13 insertions(+), 12 deletions(-)
diff --git a/drivers/acpi/pci_bind.c b/drivers/acpi/pci_bind.c
index 0469cf2..96264d2 100644
--- a/drivers/acpi/pci_bind.c
+++ b/drivers/acpi/pci_bind.c
@@ -137,7 +137,7 @@ static int acpi_pci_bind(struct acpi_device *device)
{
acpi_status status;
acpi_handle handle;
- unsigned char bus;
+ struct pci_bus *bus;
struct pci_dev *dev;
dev = acpi_get_pci_dev(device->handle);
@@ -170,11 +170,11 @@ static int acpi_pci_bind(struct acpi_device *device)
return 0;
if (dev->subordinate)
- bus = dev->subordinate->number;
+ bus = dev->subordinate;
else
- bus = dev->bus->number;
+ bus = dev->bus;
- acpi_pci_irq_add_prt(device->handle, pci_domain_nr(dev->bus), bus);
+ acpi_pci_irq_add_prt(device->handle, bus);
return 0;
}
diff --git a/drivers/acpi/pci_irq.c b/drivers/acpi/pci_irq.c
index 51b9f82..3ed944c 100644
--- a/drivers/acpi/pci_irq.c
+++ b/drivers/acpi/pci_irq.c
@@ -182,7 +182,7 @@ static void do_prt_fixups(struct acpi_prt_entry *entry,
}
}
-static int acpi_pci_irq_add_entry(acpi_handle handle, int segment, int bus,
+static int acpi_pci_irq_add_entry(acpi_handle handle, struct pci_bus *bus,
struct acpi_pci_routing_table *prt)
{
struct acpi_prt_entry *entry;
@@ -196,8 +196,8 @@ static int acpi_pci_irq_add_entry(acpi_handle handle, int segment, int bus,
* 1=INTA, 2=INTB. We use the PCI encoding throughout, so convert
* it here.
*/
- entry->id.segment = segment;
- entry->id.bus = bus;
+ entry->id.segment = pci_domain_nr(bus);
+ entry->id.bus = bus->number;
entry->id.device = (prt->address >> 16) & 0xFFFF;
entry->pin = prt->pin + 1;
@@ -242,7 +242,7 @@ static int acpi_pci_irq_add_entry(acpi_handle handle, int segment, int bus,
return 0;
}
-int acpi_pci_irq_add_prt(acpi_handle handle, int segment, int bus)
+int acpi_pci_irq_add_prt(acpi_handle handle, struct pci_bus *bus)
{
acpi_status status;
struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
@@ -271,7 +271,7 @@ int acpi_pci_irq_add_prt(acpi_handle handle, int segment, int bus)
entry = buffer.pointer;
while (entry && (entry->length > 0)) {
- acpi_pci_irq_add_entry(handle, segment, bus, entry);
+ acpi_pci_irq_add_entry(handle, bus, entry);
entry = (struct acpi_pci_routing_table *)
((unsigned long)entry + entry->length);
}
diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index 25ddbb6..aa67f72 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -537,8 +537,9 @@ static int __devinit acpi_pci_root_add(struct acpi_device *device)
*/
status = acpi_get_handle(device->handle, METHOD_NAME__PRT, &handle);
if (ACPI_SUCCESS(status))
- result = acpi_pci_irq_add_prt(device->handle, root->id.segment,
- root->id.bus);
+ result = acpi_pci_irq_add_prt(device->handle,
+ pci_find_bus(root->id.segment,
+ root->id.bus));
/*
* Scan and bind all _ADR-Based Devices
diff --git a/include/acpi/acpi_drivers.h b/include/acpi/acpi_drivers.h
index 1c50f4f..c21d83f 100644
--- a/include/acpi/acpi_drivers.h
+++ b/include/acpi/acpi_drivers.h
@@ -90,7 +90,7 @@ int acpi_pci_link_free_irq(acpi_handle handle);
/* ACPI PCI Interrupt Routing (pci_irq.c) */
-int acpi_pci_irq_add_prt(acpi_handle handle, int segment, int bus);
+int acpi_pci_irq_add_prt(acpi_handle handle, struct pci_bus *bus);
void acpi_pci_irq_del_prt(int segment, int bus);
/* ACPI PCI Device Binding (pci_bind.c) */
On Tuesday 02 June 2009 09:25:16 am Alex Chiang wrote:
> A PCI domain cannot change as you descend down subordinate buses, which
> makes the 'segment' argument to acpi_pci_irq_add_prt() useless.
>
> Change the interface to take a struct pci_bus *, from whence we can derive
> the bus number and segment. Reducing the number of arguments makes life
> simpler for callers.
Nice patch.
> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
> index 25ddbb6..aa67f72 100644
> --- a/drivers/acpi/pci_root.c
> +++ b/drivers/acpi/pci_root.c
> @@ -537,8 +537,9 @@ static int __devinit acpi_pci_root_add(struct acpi_device *device)
> */
> status = acpi_get_handle(device->handle, METHOD_NAME__PRT, &handle);
> if (ACPI_SUCCESS(status))
> - result = acpi_pci_irq_add_prt(device->handle, root->id.segment,
> - root->id.bus);
> + result = acpi_pci_irq_add_prt(device->handle,
> + pci_find_bus(root->id.segment,
> + root->id.bus));
I think you can just do this:
acpi_pci_irq_add_prt(device->handle, root->bus);
Bjorn
* Bjorn Helgaas <[email protected]>:
> On Tuesday 02 June 2009 09:25:16 am Alex Chiang wrote:
> > A PCI domain cannot change as you descend down subordinate buses, which
> > makes the 'segment' argument to acpi_pci_irq_add_prt() useless.
> >
> > Change the interface to take a struct pci_bus *, from whence we can derive
> > the bus number and segment. Reducing the number of arguments makes life
> > simpler for callers.
>
> Nice patch.
>
> > diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
> > index 25ddbb6..aa67f72 100644
> > --- a/drivers/acpi/pci_root.c
> > +++ b/drivers/acpi/pci_root.c
> > @@ -537,8 +537,9 @@ static int __devinit acpi_pci_root_add(struct acpi_device *device)
> > */
> > status = acpi_get_handle(device->handle, METHOD_NAME__PRT, &handle);
> > if (ACPI_SUCCESS(status))
> > - result = acpi_pci_irq_add_prt(device->handle, root->id.segment,
> > - root->id.bus);
> > + result = acpi_pci_irq_add_prt(device->handle,
> > + pci_find_bus(root->id.segment,
> > + root->id.bus));
>
> I think you can just do this:
>
> acpi_pci_irq_add_prt(device->handle, root->bus);
Ah, indeed you are right, thanks Bjorn.
Len, would you like me to respin, or can you just make the
modification for me?
Thanks.
/ac
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 <[email protected]>
> ---
>
> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
On Tuesday 02 June 2009 09:25:11 am Alex Chiang wrote:
> Now that we can dynamically convert an ACPI CA handle to a
> struct pci_dev at runtime, there's no need to statically bind
> them during boot.
>
> acpi_pci_bind/unbind are vastly simplified, and are only used
> to evaluate _PRT methods on P2P bridges and non-bridge children.
>
> This patchset lays further groundwork to eventually eliminate
> the acpi_driver_ops.bind callback.
>
> Cc: Bjorn Helgaas <[email protected]>
> Signed-off-by: Alex Chiang <[email protected]>
> ---
>
> drivers/acpi/pci_bind.c | 268 ++++++-------------------------------------
> drivers/acpi/pci_root.c | 2
> include/acpi/acpi_drivers.h | 3
> 3 files changed, 41 insertions(+), 232 deletions(-)
>
> diff --git a/drivers/acpi/pci_bind.c b/drivers/acpi/pci_bind.c
> index 10e3ffc..0469cf2 100644
> --- a/drivers/acpi/pci_bind.c
> +++ b/drivers/acpi/pci_bind.c
> @@ -3,6 +3,8 @@
> *
> * Copyright (C) 2001, 2002 Andy Grover <[email protected]>
> * Copyright (C) 2001, 2002 Paul Diefenbaugh <[email protected]>
> + * Copyright (C) 2009 Hewlett-Packard Development Company, L.P.
> + * Alex Chiang <[email protected]>
> *
> * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> *
> @@ -24,12 +26,7 @@
> */
>
> #include <linux/kernel.h>
> -#include <linux/module.h>
> -#include <linux/init.h>
> #include <linux/types.h>
> -#include <linux/proc_fs.h>
> -#include <linux/spinlock.h>
> -#include <linux/pm.h>
> #include <linux/pci.h>
> #include <linux/acpi.h>
> #include <acpi/acpi_bus.h>
> @@ -38,24 +35,6 @@
> #define _COMPONENT ACPI_PCI_COMPONENT
> ACPI_MODULE_NAME("pci_bind");
>
> -struct acpi_pci_data {
> - struct acpi_pci_id id;
> - struct pci_bus *bus;
> - struct pci_dev *dev;
> -};
> -
> -static int acpi_pci_bind(struct acpi_device *device);
> -static int acpi_pci_unbind(struct acpi_device *device);
> -
> -static void acpi_pci_data_handler(acpi_handle handle, u32 function,
> - void *context)
> -{
> -
> - /* TBD: Anything we need to do here? */
> -
> - return;
> -}
> -
> struct acpi_handle_node {
> struct list_head node;
> acpi_handle handle;
> @@ -139,241 +118,72 @@ out:
> }
> EXPORT_SYMBOL_GPL(acpi_get_pci_dev);
>
> -
> -static int acpi_pci_bind(struct acpi_device *device)
> +static int acpi_pci_unbind(struct acpi_device *device)
> {
> - int result = 0;
> - acpi_status status;
> - struct acpi_pci_data *data;
> - struct acpi_pci_data *pdata;
> - struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
> - acpi_handle handle;
> -
> - if (!device || !device->parent)
> - return -EINVAL;
> -
> - data = kzalloc(sizeof(struct acpi_pci_data), GFP_KERNEL);
> - if (!data)
> - return -ENOMEM;
> -
> - status = acpi_get_name(device->handle, ACPI_FULL_PATHNAME, &buffer);
> - if (ACPI_FAILURE(status)) {
> - kfree(data);
> - return -ENODEV;
> - }
> -
> - ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Binding PCI device [%s]...\n",
> - (char *)buffer.pointer));
> + struct pci_dev *dev;
>
> - /*
> - * Segment & Bus
> - * -------------
> - * These are obtained via the parent device's ACPI-PCI context.
> - */
> - status = acpi_get_data(device->parent->handle, acpi_pci_data_handler,
> - (void **)&pdata);
> - if (ACPI_FAILURE(status) || !pdata || !pdata->bus) {
> - ACPI_EXCEPTION((AE_INFO, status,
> - "Invalid ACPI-PCI context for parent device %s",
> - acpi_device_bid(device->parent)));
> - result = -ENODEV;
> - goto end;
> - }
> - data->id.segment = pdata->id.segment;
> - data->id.bus = pdata->bus->number;
> + dev = acpi_get_pci_dev(device->handle);
> + if (!dev)
> + return 0;
I think you need some pci_put_dev() calls to correspond with the new
acpi_get_pci_dev() calls.
> - /*
> - * Device & Function
> - * -----------------
> - * These are simply obtained from the device's _ADR method. Note
> - * that a value of zero is valid.
> - */
> - data->id.device = device->pnp.bus_address >> 16;
> - data->id.function = device->pnp.bus_address & 0xFFFF;
> + if (dev->subordinate)
> + acpi_pci_irq_del_prt(pci_domain_nr(dev->bus),
> + dev->subordinate->number);
>
> - ACPI_DEBUG_PRINT((ACPI_DB_INFO, "...to %04x:%02x:%02x.%d\n",
> - data->id.segment, data->id.bus, data->id.device,
> - data->id.function));
> + return 0;
> +}
>
> - /*
> - * TBD: Support slot devices (e.g. function=0xFFFF).
> - */
> +static int acpi_pci_bind(struct acpi_device *device)
> +{
> + acpi_status status;
> + acpi_handle handle;
> + unsigned char bus;
> + struct pci_dev *dev;
>
> - /*
> - * Locate PCI Device
> - * -----------------
> - * Locate matching device in PCI namespace. If it doesn't exist
> - * this typically means that the device isn't currently inserted
> - * (e.g. docking station, port replicator, etc.).
> - */
> - data->dev = pci_get_slot(pdata->bus,
> - PCI_DEVFN(data->id.device, data->id.function));
> - if (!data->dev) {
> - ACPI_DEBUG_PRINT((ACPI_DB_INFO,
> - "Device %04x:%02x:%02x.%d not present in PCI namespace\n",
> - data->id.segment, data->id.bus,
> - data->id.device, data->id.function));
> - result = -ENODEV;
> - goto end;
> - }
> - if (!data->dev->bus) {
> - printk(KERN_ERR PREFIX
> - "Device %04x:%02x:%02x.%d has invalid 'bus' field\n",
> - data->id.segment, data->id.bus,
> - data->id.device, data->id.function);
> - result = -ENODEV;
> - goto end;
> - }
> + dev = acpi_get_pci_dev(device->handle);
> + if (!dev)
> + return 0;
>
> /*
> - * PCI Bridge?
> - * -----------
> - * If so, set the 'bus' field and install the 'bind' function to
> - * facilitate callbacks for all of its children.
> + * Install the 'bind' function to facilitate callbacks for
> + * children of the P2P bridge.
> */
> - if (data->dev->subordinate) {
> + if (dev->subordinate) {
> ACPI_DEBUG_PRINT((ACPI_DB_INFO,
> "Device %04x:%02x:%02x.%d is a PCI bridge\n",
> - data->id.segment, data->id.bus,
> - data->id.device, data->id.function));
> - data->bus = data->dev->subordinate;
> + 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;
> }
>
> /*
> - * Attach ACPI-PCI Context
> - * -----------------------
> - * Thus binding the ACPI and PCI devices.
> - */
> - status = acpi_attach_data(device->handle, acpi_pci_data_handler, data);
> - if (ACPI_FAILURE(status)) {
> - ACPI_EXCEPTION((AE_INFO, status,
> - "Unable to attach ACPI-PCI context to device %s",
> - acpi_device_bid(device)));
> - result = -ENODEV;
> - goto end;
> - }
> -
> - /*
> - * PCI Routing Table
> - * -----------------
> - * Evaluate and parse _PRT, if exists. This code is independent of
> - * PCI bridges (above) to allow parsing of _PRT objects within the
> - * scope of non-bridge devices. Note that _PRTs within the scope of
> - * a PCI bridge assume the bridge's subordinate bus number.
> + * Evaluate and parse _PRT, if exists. This code allows parsing of
> + * _PRT objects within the scope of non-bridge devices. Note that
> + * _PRTs within the scope of a PCI bridge assume the bridge's
> + * subordinate bus number.
> *
> * TBD: Can _PRTs exist within the scope of non-bridge PCI devices?
> */
> status = acpi_get_handle(device->handle, METHOD_NAME__PRT, &handle);
> - if (ACPI_SUCCESS(status)) {
> - if (data->bus) /* PCI-PCI bridge */
> - acpi_pci_irq_add_prt(device->handle, data->id.segment,
> - data->bus->number);
> - else /* non-bridge PCI device */
> - acpi_pci_irq_add_prt(device->handle, data->id.segment,
> - data->id.bus);
> - }
> -
> - end:
> - kfree(buffer.pointer);
> - if (result) {
> - pci_dev_put(data->dev);
> - kfree(data);
> - }
> - return result;
> -}
> -
> -static int acpi_pci_unbind(struct acpi_device *device)
> -{
> - int result = 0;
> - acpi_status status;
> - struct acpi_pci_data *data;
> - struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
> -
> -
> - if (!device || !device->parent)
> - return -EINVAL;
> -
> - status = acpi_get_name(device->handle, ACPI_FULL_PATHNAME, &buffer);
> if (ACPI_FAILURE(status))
> - return -ENODEV;
> -
> - ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Unbinding PCI device [%s]...\n",
> - (char *) buffer.pointer));
> - kfree(buffer.pointer);
> + return 0;
>
> - status =
> - acpi_get_data(device->handle, acpi_pci_data_handler,
> - (void **)&data);
> - if (ACPI_FAILURE(status)) {
> - result = -ENODEV;
> - goto end;
> - }
> + if (dev->subordinate)
> + bus = dev->subordinate->number;
> + else
> + bus = dev->bus->number;
>
> - status = acpi_detach_data(device->handle, acpi_pci_data_handler);
> - if (ACPI_FAILURE(status)) {
> - ACPI_EXCEPTION((AE_INFO, status,
> - "Unable to detach data from device %s",
> - acpi_device_bid(device)));
> - result = -ENODEV;
> - goto end;
> - }
> - if (data->dev->subordinate) {
> - acpi_pci_irq_del_prt(data->id.segment, data->bus->number);
> - }
> - pci_dev_put(data->dev);
> - kfree(data);
> + acpi_pci_irq_add_prt(device->handle, pci_domain_nr(dev->bus), bus);
>
> - end:
> - return result;
> + return 0;
> }
>
> int
> -acpi_pci_bind_root(struct acpi_device *device,
> - struct acpi_pci_id *id, struct pci_bus *bus)
> +acpi_pci_bind_root(struct acpi_device *device)
Since you're slicing and dicing anyway, can you make the proto
indentation here match the rest of the file ("int acpi_pci...")?
> {
> - int result = 0;
> - acpi_status status;
> - struct acpi_pci_data *data = NULL;
> - struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
> -
> - if (!device || !id || !bus) {
> - return -EINVAL;
> - }
> -
> - data = kzalloc(sizeof(struct acpi_pci_data), GFP_KERNEL);
> - if (!data)
> - return -ENOMEM;
> -
> - data->id = *id;
> - data->bus = bus;
> device->ops.bind = acpi_pci_bind;
> device->ops.unbind = acpi_pci_unbind;
>
> - status = acpi_get_name(device->handle, ACPI_FULL_PATHNAME, &buffer);
> - if (ACPI_FAILURE(status)) {
> - kfree (data);
> - return -ENODEV;
> - }
> -
> - ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Binding PCI root bridge [%s] to "
> - "%04x:%02x\n", (char *)buffer.pointer,
> - id->segment, id->bus));
> -
> - status = acpi_attach_data(device->handle, acpi_pci_data_handler, data);
> - if (ACPI_FAILURE(status)) {
> - ACPI_EXCEPTION((AE_INFO, status,
> - "Unable to attach ACPI-PCI context to device %s",
> - (char *)buffer.pointer));
> - result = -ENODEV;
> - goto end;
> - }
> -
> - end:
> - kfree(buffer.pointer);
> - if (result != 0)
> - kfree(data);
> -
> - return result;
> + return 0;
> }
> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
> index bcab69a..25ddbb6 100644
> --- a/drivers/acpi/pci_root.c
> +++ b/drivers/acpi/pci_root.c
> @@ -526,7 +526,7 @@ static int __devinit acpi_pci_root_add(struct acpi_device *device)
> * -----------------------
> * Thus binding the ACPI and PCI devices.
> */
> - result = acpi_pci_bind_root(device, &root->id, root->bus);
> + result = acpi_pci_bind_root(device);
> if (result)
> goto end;
>
> diff --git a/include/acpi/acpi_drivers.h b/include/acpi/acpi_drivers.h
> index 310f5ff..1c50f4f 100644
> --- a/include/acpi/acpi_drivers.h
> +++ b/include/acpi/acpi_drivers.h
> @@ -98,8 +98,7 @@ void acpi_pci_irq_del_prt(int segment, int bus);
> struct pci_bus;
>
> struct pci_dev *acpi_get_pci_dev(acpi_handle);
> -int acpi_pci_bind_root(struct acpi_device *device, struct acpi_pci_id *id,
> - struct pci_bus *bus);
> +int acpi_pci_bind_root(struct acpi_device *device);
>
> /* Arch-defined function to add a bus to the system */
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
Hi Len,
* Alex Chiang <[email protected]>:
> This patch series eliminates static boot-time binding of ACPI
> and PCI devices, and introduces an API to perform this lookup
> during runtime.
>
> This change has the following advantages:
>
> - eliminates struct acpi_device vs struct pci_dev lifetime issues
> - lays groundwork for eliminating .bind/.unbind from acpi_device_ops
> - lays more groundwork for eliminating .start from acpi_device_ops
> and thus simplifying ACPI drivers
> - whacks out a lot of code
>
> This patchset is based on lenb/test (66c74fa1d4).
I'm going to address Bjorn's comments and respin this series.
Thanks.
/ac