All of lore.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.