public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/10] Dynamic ACPI-PCI binding
@ 2009-06-02 15:24 Alex Chiang
  2009-06-02 15:24 ` [PATCH 01/10] ACPI: make acpi_pci_bind() static Alex Chiang
                   ` (10 more replies)
  0 siblings, 11 replies; 16+ messages in thread
From: Alex Chiang @ 2009-06-02 15:24 UTC (permalink / raw)
  To: lenb; +Cc: linux-acpi, linux-kernel, linux-pci

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(-)


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 01/10] ACPI: make acpi_pci_bind() static
  2009-06-02 15:24 [PATCH 00/10] Dynamic ACPI-PCI binding Alex Chiang
@ 2009-06-02 15:24 ` Alex Chiang
  2009-06-02 15:24 ` [PATCH 02/10] ACPI: Introduce acpi_is_root_bridge() Alex Chiang
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 16+ messages in thread
From: Alex Chiang @ 2009-06-02 15:24 UTC (permalink / raw)
  To: lenb; +Cc: linux-acpi, linux-kernel, linux-pci

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 <achiang@hp.com>
---

 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);
 


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 02/10] ACPI: Introduce acpi_is_root_bridge()
  2009-06-02 15:24 [PATCH 00/10] Dynamic ACPI-PCI binding Alex Chiang
  2009-06-02 15:24 ` [PATCH 01/10] ACPI: make acpi_pci_bind() static Alex Chiang
@ 2009-06-02 15:24 ` Alex Chiang
  2009-06-02 15:24 ` [PATCH 03/10] ACPI: export acpi_pci_find_root() Alex Chiang
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 16+ messages in thread
From: Alex Chiang @ 2009-06-02 15:24 UTC (permalink / raw)
  To: lenb; +Cc: linux-acpi, linux-kernel, linux-pci

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 <achiang@hp.com>
---

 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


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 03/10] ACPI: export acpi_pci_find_root()
  2009-06-02 15:24 [PATCH 00/10] Dynamic ACPI-PCI binding Alex Chiang
  2009-06-02 15:24 ` [PATCH 01/10] ACPI: make acpi_pci_bind() static Alex Chiang
  2009-06-02 15:24 ` [PATCH 02/10] ACPI: Introduce acpi_is_root_bridge() Alex Chiang
@ 2009-06-02 15:24 ` Alex Chiang
  2009-06-02 15:24 ` [PATCH 04/10] ACPI: Introduce acpi_get_pci_dev() Alex Chiang
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 16+ messages in thread
From: Alex Chiang @ 2009-06-02 15:24 UTC (permalink / raw)
  To: lenb; +Cc: linux-acpi, linux-kernel, linux-pci

This function is useful for pci_bind.c as well.

Signed-off-by: Alex Chiang <achiang@hp.com>
---

 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


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 04/10] ACPI: Introduce acpi_get_pci_dev()
  2009-06-02 15:24 [PATCH 00/10] Dynamic ACPI-PCI binding Alex Chiang
                   ` (2 preceding siblings ...)
  2009-06-02 15:24 ` [PATCH 03/10] ACPI: export acpi_pci_find_root() Alex Chiang
@ 2009-06-02 15:24 ` Alex Chiang
  2009-06-03 16:46   ` Bjorn Helgaas
  2009-06-02 15:25 ` [PATCH 05/10] PCI Hotplug: acpiphp: convert to acpi_get_pci_dev Alex Chiang
                   ` (6 subsequent siblings)
  10 siblings, 1 reply; 16+ messages in thread
From: Alex Chiang @ 2009-06-02 15:24 UTC (permalink / raw)
  To: lenb; +Cc: linux-acpi, linux-kernel, linux-pci

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(-)

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);


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 05/10] PCI Hotplug: acpiphp: convert to acpi_get_pci_dev
  2009-06-02 15:24 [PATCH 00/10] Dynamic ACPI-PCI binding Alex Chiang
                   ` (3 preceding siblings ...)
  2009-06-02 15:24 ` [PATCH 04/10] ACPI: Introduce acpi_get_pci_dev() Alex Chiang
@ 2009-06-02 15:25 ` Alex Chiang
  2009-06-02 15:25 ` [PATCH 06/10] ACPI: kill acpi_get_pci_id Alex Chiang
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 16+ messages in thread
From: Alex Chiang @ 2009-06-02 15:25 UTC (permalink / raw)
  To: lenb; +Cc: linux-acpi, linux-kernel, linux-pci

Now that acpi_get_pci_dev is available, let's use it instead of
acpi_get_pci_id.

Signed-off-by: Alex Chiang <achiang@hp.com>
---

 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);


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 06/10] ACPI: kill acpi_get_pci_id
  2009-06-02 15:24 [PATCH 00/10] Dynamic ACPI-PCI binding Alex Chiang
                   ` (4 preceding siblings ...)
  2009-06-02 15:25 ` [PATCH 05/10] PCI Hotplug: acpiphp: convert to acpi_get_pci_dev Alex Chiang
@ 2009-06-02 15:25 ` Alex Chiang
  2009-06-02 15:25 ` [PATCH 07/10] ACPI: eviscerate pci_bind.c Alex Chiang
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 16+ messages in thread
From: Alex Chiang @ 2009-06-02 15:25 UTC (permalink / raw)
  To: lenb; +Cc: linux-acpi, linux-kernel, linux-pci

acpi_get_pci_dev() is better, and all callers have been converted, so
eliminate acpi_get_pci_id().

Signed-off-by: Alex Chiang <achiang@hp.com>
---

 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);
 


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 07/10] ACPI: eviscerate pci_bind.c
  2009-06-02 15:24 [PATCH 00/10] Dynamic ACPI-PCI binding Alex Chiang
                   ` (5 preceding siblings ...)
  2009-06-02 15:25 ` [PATCH 06/10] ACPI: kill acpi_get_pci_id Alex Chiang
@ 2009-06-02 15:25 ` Alex Chiang
  2009-06-03 18:26   ` Bjorn Helgaas
  2009-06-02 15:25 ` [PATCH 08/10] ACPI: simplify acpi_pci_irq_add_prt() API Alex Chiang
                   ` (3 subsequent siblings)
  10 siblings, 1 reply; 16+ messages in thread
From: Alex Chiang @ 2009-06-02 15:25 UTC (permalink / raw)
  To: lenb; +Cc: linux-acpi, Bjorn Helgaas, linux-kernel, linux-pci

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 <bjorn.helgaas@hp.com>
Signed-off-by: Alex Chiang <achiang@hp.com>
---

 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 <andrew.grover@intel.com>
  *  Copyright (C) 2001, 2002 Paul Diefenbaugh <paul.s.diefenbaugh@intel.com>
+ *  Copyright (C) 2009 Hewlett-Packard Development Company, L.P.
+ *	Alex Chiang <achiang@hp.com>
  *
  * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  *
@@ -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 */
 


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 08/10] ACPI: simplify acpi_pci_irq_add_prt() API
  2009-06-02 15:24 [PATCH 00/10] Dynamic ACPI-PCI binding Alex Chiang
                   ` (6 preceding siblings ...)
  2009-06-02 15:25 ` [PATCH 07/10] ACPI: eviscerate pci_bind.c Alex Chiang
@ 2009-06-02 15:25 ` Alex Chiang
  2009-06-02 17:30   ` Bjorn Helgaas
  2009-06-02 15:25 ` [PATCH 09/10] ACPI: simplify acpi_pci_irq_del_prt() API Alex Chiang
                   ` (2 subsequent siblings)
  10 siblings, 1 reply; 16+ messages in thread
From: Alex Chiang @ 2009-06-02 15:25 UTC (permalink / raw)
  To: lenb; +Cc: linux-acpi, Bjorn Helgaas, linux-kernel, linux-pci

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 <bjorn.helgaas@hp.com>
Signed-off-by: Alex Chiang <achiang@hp.com>
---

 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) */


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 09/10] ACPI: simplify acpi_pci_irq_del_prt() API
  2009-06-02 15:24 [PATCH 00/10] Dynamic ACPI-PCI binding Alex Chiang
                   ` (7 preceding siblings ...)
  2009-06-02 15:25 ` [PATCH 08/10] ACPI: simplify acpi_pci_irq_add_prt() API Alex Chiang
@ 2009-06-02 15:25 ` Alex Chiang
  2009-06-02 15:25 ` [PATCH 10/10] ACPI: acpi_pci_unbind should clean up properly after acpi_pci_bind Alex Chiang
  2009-06-03 19:29 ` [PATCH 00/10] Dynamic ACPI-PCI binding Alex Chiang
  10 siblings, 0 replies; 16+ messages in thread
From: Alex Chiang @ 2009-06-02 15:25 UTC (permalink / raw)
  To: lenb; +Cc: linux-acpi, Bjorn Helgaas, linux-kernel, linux-pci

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 <bjorn.helgaas@hp.com>
Signed-off-by: Alex Chiang <achiang@hp.com>
---

 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) */
 


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 10/10] ACPI: acpi_pci_unbind should clean up properly after acpi_pci_bind
  2009-06-02 15:24 [PATCH 00/10] Dynamic ACPI-PCI binding Alex Chiang
                   ` (8 preceding siblings ...)
  2009-06-02 15:25 ` [PATCH 09/10] ACPI: simplify acpi_pci_irq_del_prt() API Alex Chiang
@ 2009-06-02 15:25 ` Alex Chiang
  2009-06-03 19:29 ` [PATCH 00/10] Dynamic ACPI-PCI binding Alex Chiang
  10 siblings, 0 replies; 16+ messages in thread
From: Alex Chiang @ 2009-06-02 15:25 UTC (permalink / raw)
  To: lenb; +Cc: linux-acpi, Bjorn Helgaas, linux-kernel, linux-pci

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 <bjorn.helgaas@hp.com>
Signed-off-by: Alex Chiang <achiang@hp.com>
---

 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;
 }


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 08/10] ACPI: simplify acpi_pci_irq_add_prt() API
  2009-06-02 15:25 ` [PATCH 08/10] ACPI: simplify acpi_pci_irq_add_prt() API Alex Chiang
@ 2009-06-02 17:30   ` Bjorn Helgaas
  2009-06-02 17:41     ` Alex Chiang
  0 siblings, 1 reply; 16+ messages in thread
From: Bjorn Helgaas @ 2009-06-02 17:30 UTC (permalink / raw)
  To: Alex Chiang; +Cc: lenb, linux-acpi, linux-kernel, linux-pci

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 08/10] ACPI: simplify acpi_pci_irq_add_prt() API
  2009-06-02 17:30   ` Bjorn Helgaas
@ 2009-06-02 17:41     ` Alex Chiang
  0 siblings, 0 replies; 16+ messages in thread
From: Alex Chiang @ 2009-06-02 17:41 UTC (permalink / raw)
  To: Bjorn Helgaas; +Cc: lenb, linux-acpi, linux-kernel, linux-pci

* Bjorn Helgaas <bjorn.helgaas@hp.com>:
> 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


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 04/10] ACPI: Introduce acpi_get_pci_dev()
  2009-06-02 15:24 ` [PATCH 04/10] ACPI: Introduce acpi_get_pci_dev() Alex Chiang
@ 2009-06-03 16:46   ` Bjorn Helgaas
  0 siblings, 0 replies; 16+ messages in thread
From: Bjorn Helgaas @ 2009-06-03 16:46 UTC (permalink / raw)
  To: Alex Chiang; +Cc: lenb, linux-acpi, linux-kernel, linux-pci

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
> 



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 07/10] ACPI: eviscerate pci_bind.c
  2009-06-02 15:25 ` [PATCH 07/10] ACPI: eviscerate pci_bind.c Alex Chiang
@ 2009-06-03 18:26   ` Bjorn Helgaas
  0 siblings, 0 replies; 16+ messages in thread
From: Bjorn Helgaas @ 2009-06-03 18:26 UTC (permalink / raw)
  To: Alex Chiang; +Cc: lenb, linux-acpi, linux-kernel, linux-pci

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 <bjorn.helgaas@hp.com>
> Signed-off-by: Alex Chiang <achiang@hp.com>
> ---
> 
>  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 <andrew.grover@intel.com>
>   *  Copyright (C) 2001, 2002 Paul Diefenbaugh <paul.s.diefenbaugh@intel.com>
> + *  Copyright (C) 2009 Hewlett-Packard Development Company, L.P.
> + *	Alex Chiang <achiang@hp.com>
>   *
>   * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   *
> @@ -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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 00/10] Dynamic ACPI-PCI binding
  2009-06-02 15:24 [PATCH 00/10] Dynamic ACPI-PCI binding Alex Chiang
                   ` (9 preceding siblings ...)
  2009-06-02 15:25 ` [PATCH 10/10] ACPI: acpi_pci_unbind should clean up properly after acpi_pci_bind Alex Chiang
@ 2009-06-03 19:29 ` Alex Chiang
  10 siblings, 0 replies; 16+ messages in thread
From: Alex Chiang @ 2009-06-03 19:29 UTC (permalink / raw)
  To: lenb; +Cc: linux-acpi, linux-kernel, linux-pci

Hi Len,

* Alex Chiang <achiang@hp.com>:
> 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


^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2009-06-03 19:29 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-06-02 15:24 [PATCH 00/10] Dynamic ACPI-PCI binding Alex Chiang
2009-06-02 15:24 ` [PATCH 01/10] ACPI: make acpi_pci_bind() static Alex Chiang
2009-06-02 15:24 ` [PATCH 02/10] ACPI: Introduce acpi_is_root_bridge() Alex Chiang
2009-06-02 15:24 ` [PATCH 03/10] ACPI: export acpi_pci_find_root() Alex Chiang
2009-06-02 15:24 ` [PATCH 04/10] ACPI: Introduce acpi_get_pci_dev() Alex Chiang
2009-06-03 16:46   ` Bjorn Helgaas
2009-06-02 15:25 ` [PATCH 05/10] PCI Hotplug: acpiphp: convert to acpi_get_pci_dev Alex Chiang
2009-06-02 15:25 ` [PATCH 06/10] ACPI: kill acpi_get_pci_id Alex Chiang
2009-06-02 15:25 ` [PATCH 07/10] ACPI: eviscerate pci_bind.c Alex Chiang
2009-06-03 18:26   ` Bjorn Helgaas
2009-06-02 15:25 ` [PATCH 08/10] ACPI: simplify acpi_pci_irq_add_prt() API Alex Chiang
2009-06-02 17:30   ` Bjorn Helgaas
2009-06-02 17:41     ` Alex Chiang
2009-06-02 15:25 ` [PATCH 09/10] ACPI: simplify acpi_pci_irq_del_prt() API Alex Chiang
2009-06-02 15:25 ` [PATCH 10/10] ACPI: acpi_pci_unbind should clean up properly after acpi_pci_bind Alex Chiang
2009-06-03 19:29 ` [PATCH 00/10] Dynamic ACPI-PCI binding Alex Chiang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox