Dynamic ACPI-PCI binding
Subsystems (3)
v2 cover Dynamic ACPI-PCI binding
Hi Len,
I hope this isn't too late for the .31 merge window. If so, no big deal; this patchset isn't urgent, it's just a nice cleanup that was inspired from investigating the recent bug in acpi_pci_bind() (fixed by dacd254).
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), and has been boot tested on ia64 and x86. I also performed physical PCI hotplug tests using acpiphp on ia64. I do not have an acpiphp-capable x86 platform.
I didn't test the changes in the video driver either, as I don't have the hardware.
v1 -> v2
- rearrange series into a more logical order
- much simpler acpi_is_root_bridge() implementation
- no longer export acpi_pci_find_root()
- no longer leak memory in acpi_get_pci_dev()
- no longer leak references in acpi_pci_unbind/acpi_pci_bind
- convert video driver to use acpi_get_pci_dev()
- kill off acpi_get_physical_pci_device()
- incorporate Bjorn's other comments. :)
---
Alex Chiang (11):
ACPI: kill acpi_get_physical_pci_device()
ACPI: video: convert to acpi_get_pci_dev
ACPI: kill acpi_get_pci_id
PCI Hotplug: acpiphp: convert to acpi_get_pci_dev
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: Introduce acpi_get_pci_dev()
ACPI: Introduce acpi_is_root_bridge()
ACPI: make acpi_pci_bind() static
drivers/acpi/glue.c | 40 ----- drivers/acpi/pci_bind.c | 315 +++++------------------------------- drivers/acpi/pci_irq.c | 17 +- drivers/acpi/pci_root.c | 112 ++++++++++++- drivers/acpi/video.c | 6 - drivers/acpi/video_detect.c | 9 + drivers/pci/hotplug/acpi_pcihp.c | 40 ----- drivers/pci/hotplug/acpiphp_glue.c | 27 +-- include/acpi/acpi_bus.h | 2 include/acpi/acpi_drivers.h | 10 - include/linux/pci_hotplug.h | 1 11 files changed, 179 insertions(+), 400 deletions(-)
PATCH v2 1/11 ACPI: make acpi_pci_bind() static
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.
---
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(-)
drivers/acpi/pci_bind.c +2 -1
@@ -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;
drivers/acpi/pci_root.c +0 -2
@@ -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.
include/acpi/acpi_drivers.h +0 -1
@@ -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);
PATCH v2 2/11 ACPI: Introduce acpi_is_root_bridge()
In reply to Alex ChiangReturns 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.
The implementation becomes much simpler as well.
---
drivers/acpi/pci_root.c | 24 ++++++++++++++++++++++ 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, 28 insertions(+), 40 deletions(-)
drivers/acpi/pci_root.c +24 -0
@@ -142,6 +142,30 @@ 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. +* +* Note: we could make this API take a struct acpi_device * instead, but +* for now, it's more convenient to operate on an acpi_handle. +*/ +int acpi_is_root_bridge (acpi_handle handle) +{ +int ret; +struct acpi_device *device; + + ret =acpi_bus_get_device (handle, &device); +if (ret) +return 0 ; + + ret =acpi_match_device_ids (device, root_device_ids); +if (ret) +return 0 ; +else +return 1 ; +} +EXPORT_SYMBOL_GPL (acpi_is_root_bridge); +static acpi_status get_root_bridge_busnr_callback (struct acpi_resource *resource,void *data) {
drivers/pci/hotplug/acpi_pcihp.c +2 -38
@@ -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;
drivers/pci/hotplug/acpiphp_glue.c +1 -1
@@ -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)++;
include/acpi/acpi_bus.h +1 -0
@@ -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))
include/linux/pci_hotplug.h +0 -1
@@ -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
PATCH v2 3/11 ACPI: Introduce acpi_get_pci_dev()
In reply to Alex ChiangConvert 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 lays the groundwork for eliminating much of pci_bind.c.
Although pci_root.c may not be the most logical place for this change, putting it here saves us from having to export acpi_pci_find_root.
---
drivers/acpi/pci_root.c | 81 +++++++++++++++++++++++++++++++++++++++++++ include/acpi/acpi_drivers.h | 1 + 2 files changed, 82 insertions(+), 0 deletions(-)
drivers/acpi/pci_root.c +81 -0
@@ -329,6 +329,87 @@ static struct acpi_pci_root *acpi_pci_find_root(acpi_handle handle)return NULL ; } +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 = 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 (node, &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 ; + + pdev =pci_get_slot (pbus,PCI_DEVFN (dev, fn)); +if (hnd == handle) +break ; + + pbus = pdev->subordinate; +pci_dev_put (pdev); + } +out: +list_for_each_entry_safe (node, tmp, &device_list, node) +kfree (node); + +return pdev; +} +EXPORT_SYMBOL_GPL (acpi_get_pci_dev); +/** * acpi_pci_osc_control_set - commit requested control to Firmware * @handle: acpi_handle for the target ACPI object
include/acpi/acpi_drivers.h +1 -0
@@ -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);
PATCH v2 4/11 ACPI: eviscerate pci_bind.c
In reply to Alex ChiangNow 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 patch 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.
This patch lays further groundwork to eventually eliminate the acpi_driver_ops.bind callback.
---
drivers/acpi/pci_bind.c | 251 +++++++------------------------------------ drivers/acpi/pci_root.c | 2 include/acpi/acpi_drivers.h | 3 - 3 files changed, 42 insertions(+), 214 deletions(-)
drivers/acpi/pci_bind.c +40 -211
@@ -24,12 +24,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>
@@ -109,240 +104,74 @@ acpi_status acpi_get_pci_id(acpi_handle handle, struct acpi_pci_id *id)EXPORT_SYMBOL (acpi_get_pci_id); -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)); +pci_dev_put (dev); +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); + goto out; - 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; +out: +pci_dev_put (dev); +return 0 ; } -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) { -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 ; }
drivers/acpi/pci_root.c +1 -1
@@ -603,7 +603,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;
include/acpi/acpi_drivers.h +1 -2
@@ -99,8 +99,7 @@ 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); +int acpi_pci_bind_root (struct acpi_device *device);/* Arch-defined function to add a bus to the system */
PATCH v2 5/11 ACPI: simplify acpi_pci_irq_add_prt() API
In reply to Alex ChiangA 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.
---
drivers/acpi/pci_bind.c | 8 ++++---- drivers/acpi/pci_irq.c | 10 +++++----- drivers/acpi/pci_root.c | 3 +-- include/acpi/acpi_drivers.h | 2 +- 4 files changed, 11 insertions(+), 12 deletions(-)
drivers/acpi/pci_bind.c +4 -4
@@ -124,7 +124,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);
@@ -157,11 +157,11 @@ static int acpi_pci_bind(struct acpi_device *device) goto out;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); out:pci_dev_put (dev);
drivers/acpi/pci_irq.c +5 -5
@@ -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); }
drivers/acpi/pci_root.c +1 -2
@@ -614,8 +614,7 @@ 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, root->bus);/* * Scan and bind all _ADR-Based Devices
include/acpi/acpi_drivers.h +1 -1
@@ -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) */
PATCH v2 6/11 ACPI: simplify acpi_pci_irq_del_prt() API
In reply to Alex ChiangThere 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.
---
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(-)
drivers/acpi/pci_bind.c +1 -2
@@ -113,8 +113,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);pci_dev_put (dev);return 0 ;
drivers/acpi/pci_irq.c +4 -3
@@ -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); }
include/acpi/acpi_drivers.h +1 -1
@@ -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) */
PATCH v2 7/11 ACPI: acpi_pci_unbind should clean up properly after acpi_pci_bind
In reply to Alex ChiangIn 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.
---
drivers/acpi/pci_bind.c | 11 +++++++++-- 1 files changed, 9 insertions(+), 2 deletions(-)
drivers/acpi/pci_bind.c +9 -2
@@ -106,14 +106,21 @@ EXPORT_SYMBOL(acpi_get_pci_id);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);pci_dev_put (dev);return 0 ;
Re: [PATCH v2 07/11] ACPI: acpi_pci_unbind should clean up properly after acpi_pci_bind
Quoted 31 lines · Alex Chiang
In
acpi_pci_bind, we add_PRTinformation for non-bridge devices, but we never delete that information inacpi_pci_unbind.We also set
device->ops.bindanddevice->ops.unbind, but never clear them out.Let's make
acpi_pci_unbindclean up what we did inacpi_pci_bind.Cc Bjorn Helgaas <[email protected]>Signed-off-by Alex Chiang <[email protected]>---
drivers/acpi/pci_bind.c | 11 +++++++++-- 1 files changed, 9 insertions(+), 2 deletions(-)drivers/acpi/pci_bind.c +1
-1@@ -106,14 +106,21 @@ EXPORT_SYMBOL(acpi_get_pci_id);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)
drivers/acpi/pci_bind.c EXPORT_SYMBOL(acpi_get_pci_id); · line 124
Quoted 10 lines
drivers/acpi/pci_bind.c +8
-1-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);
I have a concern about this change.
The acpi_pci_irq_del_prt() against dev->bus removes not only the _PRT entries for PCI function corresponding to specified acpi_device, but also other _PRT entries for working PCI devices/functions on the same bus. As a result, interrupt initialization for those PCI functions would no longer work properly after that.
So I think we should not call acpi_pci_irq_del_prt() against dev->bus.
Thanks,
Kenji Kaneshige
Re: [PATCH v2 07/11] ACPI: acpi_pci_unbind should clean up properly after acpi_pci_bind
* Kenji Kaneshige <[email protected]>:
Quoted 11 lines
Quoted 1 line
I have a concern about this change.
The
acpi_pci_irq_del_prt()againstdev->busremoves not only the_PRTentries for PCI function corresponding to specifiedacpi_device, but also other_PRTentries for working PCI devices/functions on the same bus. As a result, interrupt initialization for those PCI functions would no longer work properly after that.
Quoted 2 lines
So I think we should not call
acpi_pci_irq_del_prt()againstdev->bus.
Thanks for the review. I agree with you.
Here is a respun version of this patch.
From: Alex Chiang <[email protected]>
ACPI: acpi_pci_unbind should clean up properly after acpi_pci_bind
In acpi_pci_bind, we set device->ops.bind and device->ops.unbind, but never clear them out.
---
drivers/acpi/pci_bind.c +5 -3
@@ -109,11 +109,13 @@ static int acpi_pci_unbind(struct acpi_device *device)struct pci_dev *dev; dev =acpi_get_pci_dev (device->handle); -if (!dev) +if (!dev || !dev->subordinate)return 0 ; -if (dev->subordinate) -acpi_pci_irq_del_prt (dev->subordinate); +acpi_pci_irq_del_prt (dev->subordinate); + + device->ops.bind =NULL ; + device->ops.unbind =NULL ;pci_dev_put (dev);return 0 ;
Re: [PATCH v2 07/11] ACPI: acpi_pci_unbind should clean up properly after acpi_pci_bind
Quoted 11 lines · Alex Chiang
* Kenji Kaneshige <[email protected]>:
Quoted 10 lines
I have a concern about this change.
The
acpi_pci_irq_del_prt()againstdev->busremoves not only the_PRTentries for PCI function corresponding to specifiedacpi_device, but also other_PRTentries for working PCI devices/functions on the same bus. As a result, interrupt initialization for those PCI functions would no longer work properly after that.
Quoted 4 lines
Quoted 2 lines
So I think we should not call
acpi_pci_irq_del_prt()againstdev->bus.Thanks for the review. I agree with you.
I agree that this respun version makes things more the way they were, so in that sense, it should do no harm. But I still have the niggling concern that .bind() adds _PRT info for non-bridges, and there's no corresponding removal. There should be some path that makes this more symmetric.
Quoted 34 lines
Here is a respun version of this patch.
From: Alex Chiang <[email protected]>
ACPI:
acpi_pci_unbindshould clean up properly afteracpi_pci_bindIn
acpi_pci_bind, we setdevice->ops.bindanddevice->ops.unbind, but never clear them out.Cc Bjorn Helgaas <[email protected]>Signed-off-by Alex Chiang <[email protected]>---
drivers/acpi/pci_bind.c +5
-3@@ -109,11 +109,13 @@ static int acpi_pci_unbind(struct acpi_device *device)struct pci_dev *dev; dev =acpi_get_pci_dev (device->handle); -if (!dev) +if (!dev || !dev->subordinate)return 0 ; -if (dev->subordinate) -acpi_pci_irq_del_prt (dev->subordinate); +acpi_pci_irq_del_prt (dev->subordinate); + + device->ops.bind =NULL ; + device->ops.unbind =NULL ;pci_dev_put (dev);return 0 ;
Re: [PATCH v2 07/11] ACPI: acpi_pci_unbind should clean up properly after acpi_pci_bind
* Bjorn Helgaas <[email protected]>:
Quoted 15 lines · Alex Chiang
Quoted 14 lines
* Kenji Kaneshige <[email protected]>:
Quoted 12 lines
I have a concern about this change.
The
acpi_pci_irq_del_prt()againstdev->busremoves not only the_PRTentries for PCI function corresponding to specifiedacpi_device, but also other_PRTentries for working PCI devices/functions on the same bus. As a result, interrupt initialization for those PCI functions would no longer work properly after that.So I think we should not call
acpi_pci_irq_del_prt()againstdev->bus.
Quoted 7 lines
Quoted 1 line
Thanks for the review. I agree with you.
I agree that this respun version makes things more the way they were, so in that sense, it should do no harm. But I still have the niggling concern that .bind() adds
_PRTinfo for non-bridges, and there's no corresponding removal. There should be some path that makes this more symmetric.
The comment in acpi_pci_bind doesn't seem to know if you can even have a _PRT for non-bridges. The spec (3.0b) says that _PRT is required for all root bridges, but doesn't mention anything about non-bridge devices.
My gut feeling is the way to cure the symmetry is to remove the path in .bind() that adds _PRT for non-bridges, but I'm a little hesitant to do that without knowing for sure.
I think for this patch, the Hippocratic approach is the right one for now.
Thanks.
/ac
Quoted 36 lines
Quoted 34 lines
Here is a respun version of this patch.
From: Alex Chiang <[email protected]>
ACPI:
acpi_pci_unbindshould clean up properly afteracpi_pci_bindIn
acpi_pci_bind, we setdevice->ops.bindanddevice->ops.unbind, but never clear them out.Cc Bjorn Helgaas <[email protected]>Signed-off-by Alex Chiang <[email protected]>---
drivers/acpi/pci_bind.c +5
-3@@ -109,11 +109,13 @@ static int acpi_pci_unbind(struct acpi_device *device)struct pci_dev *dev; dev =acpi_get_pci_dev (device->handle); -if (!dev) +if (!dev || !dev->subordinate)return 0 ; -if (dev->subordinate) -acpi_pci_irq_del_prt (dev->subordinate); +acpi_pci_irq_del_prt (dev->subordinate); + + device->ops.bind =NULL ; + device->ops.unbind =NULL ;pci_dev_put (dev);return 0 ;
Re: [PATCH v2 07/11] ACPI: acpi_pci_unbind should clean up properly after acpi_pci_bind
In reply to Bjorn Helgaas* Bjorn Helgaas <[email protected]>:
Quoted 15 lines · Alex Chiang
Quoted 14 lines
* Kenji Kaneshige <[email protected]>:
Quoted 12 lines
I have a concern about this change.
The
acpi_pci_irq_del_prt()againstdev->busremoves not only the_PRTentries for PCI function corresponding to specifiedacpi_device, but also other_PRTentries for working PCI devices/functions on the same bus. As a result, interrupt initialization for those PCI functions would no longer work properly after that.So I think we should not call
acpi_pci_irq_del_prt()againstdev->bus.
Quoted 7 lines
Quoted 1 line
Thanks for the review. I agree with you.
I agree that this respun version makes things more the way they were, so in that sense, it should do no harm. But I still have the niggling concern that .bind() adds
_PRTinfo for non-bridges, and there's no corresponding removal. There should be some path that makes this more symmetric.
Hm, in another forum, you suggested that dynamic PRT lookups might be a solution, which I kinda like.
So, the plan that I would prefer is:
a) get this patchset in [and we 'do no harm' here so _hopefully_ aren't introducing regressions]
b) work on dynamic PRT lookups in a future patchset.
Thanks.
/ac
Re: [PATCH v2 07/11] ACPI: acpi_pci_unbind should clean up properly after acpi_pci_bind
In reply to Alex ChiangQuoted 33 lines · Alex Chiang
* Kenji Kaneshige <[email protected]>:
Quoted 10 lines
Quoted 1 line
I have a concern about this change.
The
acpi_pci_irq_del_prt()againstdev->busremoves not only the_PRTentries for PCI function corresponding to specifiedacpi_device, but also other_PRTentries for working PCI devices/functions on the same bus. As a result, interrupt initialization for those PCI functions would no longer work properly after that.Quoted 2 lines
So I think we should not call
acpi_pci_irq_del_prt()againstdev->bus.Thanks for the review. I agree with you.
Here is a respun version of this patch.
From: Alex Chiang <[email protected]>
ACPI:
acpi_pci_unbindshould clean up properly afteracpi_pci_bindIn
acpi_pci_bind, we setdevice->ops.bindanddevice->ops.unbind, but never clear them out.Cc Bjorn Helgaas <[email protected]>Signed-off-by Alex Chiang <[email protected]>---
drivers/acpi/pci_bind.c +0
-0
drivers/acpi/pci_bind.c static int acpi_pci_unbind(struct acpi_device *device) · line 113
Quoted 6 lines
drivers/acpi/pci_bind.c +1
-1@@ -109,11 +109,13 @@ static int acpi_pci_unbind(struct acpi_device *device)struct pci_dev *dev; dev =acpi_get_pci_dev (device->handle); -if (!dev) +if (!dev || !dev->subordinate)return 0 ;
This would leak the pci_dev's refcount
if dev != NULL && dev->subordinate == NULL.
Thanks,
Kenji Kaneshige
Quoted 16 lines
drivers/acpi/pci_bind.c +4
-3-if (dev->subordinate) -acpi_pci_irq_del_prt (dev->subordinate); +acpi_pci_irq_del_prt (dev->subordinate); + + device->ops.bind =NULL ; + device->ops.unbind =NULL ;pci_dev_put (dev);return 0 ; --To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 07/11] ACPI: acpi_pci_unbind should clean up properly after acpi_pci_bind
* Kenji Kaneshige <[email protected]>:
Quoted 14 lines
drivers/acpi/pci_bind.c static int acpi_pci_unbind(struct acpi_device *device) · line 112
Quoted 10 lines · Alex Chiang
drivers/acpi/pci_bind.c +1
-1@@ -109,11 +109,13 @@ static int acpi_pci_unbind(struct acpi_device *device)struct pci_dev *dev; dev =acpi_get_pci_dev (device->handle); -if (!dev) +if (!dev || !dev->subordinate)return 0 ;This would leak the pci_dev's refcount
if dev != NULL &&dev->subordinate== NULL.
Yes, of course you are correct. Thanks for correcting my sloppiness.
Here is the correct fix.
From: Alex Chiang <[email protected]>
ACPI: acpi_pci_unbind should clean up properly after acpi_pci_bind
In acpi_pci_bind, we set device->ops.bind and device->ops.unbind, but never clear them out.
---
drivers/acpi/pci_bind.c +7 -4
@@ -109,12 +109,15 @@ static int acpi_pci_unbind(struct acpi_device *device)struct pci_dev *dev; dev =acpi_get_pci_dev (device->handle); -if (!dev) -return 0 ; +if (!dev || !dev->subordinate) + goto out: -if (dev->subordinate) -acpi_pci_irq_del_prt (dev->subordinate); + acpi_pci_irq_del_prt(dev->subordinate); + device->ops.bind =NULL ; + device->ops.unbind =NULL ; + +out:pci_dev_put (dev);return 0 ; }
PATCH v2 8/11 PCI Hotplug: acpiphp: convert to acpi_get_pci_dev
In reply to Alex ChiangNow that acpi_get_pci_dev is available, let's use it instead of acpi_get_pci_id.
---
drivers/pci/hotplug/acpiphp_glue.c | 25 +++++++------------------ 1 files changed, 7 insertions(+), 18 deletions(-)
drivers/pci/hotplug/acpiphp_glue.c +7 -18
@@ -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);
@@ -1416,6 +1404,7 @@ static int acpiphp_configure_bridge (acpi_handle handle)acpiphp_set_hpp_values (handle, bus);pci_enable_bridges (bus);acpiphp_configure_ioapics (handle); +pci_dev_put (dev);return 0 ; }
Jesse Barnes
AppliedRe: [PATCH v2 08/11] PCI Hotplug: acpiphp: convert to acpi_get_pci_dev
Quoted 7 lines · On Wed, 03 Jun 2009 23:58:57 -0600 Alex Chiang <[email protected]> wrote:
Now that
acpi_get_pci_devis available, let's use it instead ofacpi_get_pci_id.Cc Jesse Barnes <[email protected]>Signed-off-by Alex Chiang <[email protected]>---
Applied to linux-next, thanks.
Signature · 1 line
Jesse Barnes, Intel Open Source Technology Center
Jesse Barnes
Acked-byRe: [PATCH v2 08/11] PCI Hotplug: acpiphp: convert to acpi_get_pci_dev
Quoted 6 lines · On Thu, 11 Jun 2009 14:48:45 -0700 Jesse Barnes <[email protected]> wrote:
Quoted 3 lines · On Wed, 03 Jun 2009 23:58:57 -0600 Alex Chiang <[email protected]> wrote:
Now that
acpi_get_pci_devis available, let's use it instead ofacpi_get_pci_id.
Quoted 6 lines
Quoted 4 lines
Cc Jesse Barnes <[email protected]>Signed-off-by Alex Chiang <[email protected]>---
Applied to linux-next, thanks.
Oops, since this one depends on others in the series it's probably best if they just go in through Len's tree. This one has my
too.
Signature · 1 line
Jesse Barnes, Intel Open Source Technology Center
PATCH v2 9/11 ACPI: kill acpi_get_pci_id
In reply to Alex Chiangacpi_get_pci_dev() is better, and all callers have been converted, so eliminate acpi_get_pci_id().
---
drivers/acpi/pci_bind.c | 71 ------------------------------------------- include/acpi/acpi_drivers.h | 1 - 2 files changed, 0 insertions(+), 72 deletions(-)
drivers/acpi/pci_bind.c +0 -71
@@ -33,77 +33,6 @@#define _COMPONENT ACPI_PCI_COMPONENTACPI_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 ; -} - -/** -* 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_unbind (struct acpi_device *device) {struct pci_bus *bus;
include/acpi/acpi_drivers.h +0 -1
@@ -98,7 +98,6 @@ void acpi_pci_irq_del_prt(struct pci_bus *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);/* Arch-defined function to add a bus to the system */
PATCH v2 10/11 ACPI: video: convert to acpi_get_pci_dev
In reply to Alex ChiangNow that acpi_get_pci_dev is available, let's use it instead of acpi_get_physical_pci_device()
---
drivers/acpi/video.c | 6 +++--- drivers/acpi/video_detect.c | 9 +++++---- 2 files changed, 8 insertions(+), 7 deletions(-)
drivers/acpi/video.c +3 -3
@@ -1054,15 +1054,15 @@ static void acpi_video_bus_find_cap(struct acpi_video_bus *video)static int acpi_video_bus_check (struct acpi_video_bus *video) {acpi_status status = -ENOENT ; -struct device *dev; +struct pci_dev *dev;if (!video)return -EINVAL ; - dev =acpi_get_physical_pci_device (video->device->handle); + dev =acpi_get_pci_dev (video->device->handle);if (!dev)return -ENODEV ; -put_device (dev); +pci_dev_put (dev);/* Since there is no HID, CID and so on for VGA driver, we have * to check well known required nodes.
drivers/acpi/video_detect.c +5 -4
@@ -10,7 +10,7 @@* assinged * * After PCI devices are glued with ACPI devices -* acpi_get_physical_pci_device() can be called to identify ACPI graphics +* acpi_get_pci_dev() can be called to identify ACPI graphics * devices for which a real graphics card is plugged in * * Now acpi_video_get_capabilities() can be called to check which
@@ -36,6 +36,7 @@#include <linux/acpi.h> #include <linux/dmi.h> +#include <linux/pci.h> ACPI_MODULE_NAME ("video" );#define _COMPONENT ACPI_VIDEO_COMPONENT
@@ -109,7 +110,7 @@ static acpi_status find_video(acpi_handle handle, u32 lvl, void *context, void **rv) { long *cap = context; -struct device *dev; +struct pci_dev *dev;struct acpi_device *acpi_dev; const struct acpi_device_id video_ids[] = {
@@ -120,10 +121,10 @@ find_video(acpi_handle handle, u32 lvl, void *context, void **rv)return AE_OK ;if (!acpi_match_device_ids (acpi_dev, video_ids)) { - dev =acpi_get_physical_pci_device (handle); + dev =acpi_get_pci_dev (handle);if (!dev)return AE_OK ; -put_device (dev); +pci_dev_put (dev); *cap |=acpi_is_video_device (acpi_dev); }return AE_OK ;
PATCH v2 11/11 ACPI: kill acpi_get_physical_pci_device()
In reply to Alex Chiangacpi_get_pci_dev() is (hopefully) better, and all callers have been converted, so let's get rid of this duplicated functionality.
---
drivers/acpi/glue.c | 40 ---------------------------------------- include/acpi/acpi_bus.h | 1 - 2 files changed, 0 insertions(+), 41 deletions(-)
drivers/acpi/glue.c +0 -40
@@ -140,46 +140,6 @@ struct device *acpi_get_physical_device(acpi_handle handle)EXPORT_SYMBOL (acpi_get_physical_device); -/* ToDo: When a PCI bridge is found, return the PCI device behind the bridge -* This should work in general, but did not on a Lenovo T61 for the -* graphics card. But this must be fixed when the PCI device is -* bound and the kernel device struct is attached to the acpi device -* Note: A success call will increase reference count by one -* Do call put_device(dev) on the returned device then -*/ -struct device *acpi_get_physical_pci_device (acpi_handle handle) -{ -struct device *dev; -long long device_id; -acpi_status status; - - status = -acpi_evaluate_integer (handle,"_ADR" ,NULL , &device_id); - -if (ACPI_FAILURE (status)) -return NULL ; - -/* We need to attempt to determine whether the _ADR refers to a -PCI device or not. There's no terribly good way to do this, -so the best we can hope for is to assume that there'll never -be a device in the host bridge */ -if (device_id >=0x10000 ) { -/* It looks like a PCI device. Does it exist? */ - dev =acpi_get_physical_device (handle); - }else { -/* It doesn't look like a PCI device. Does its parent -exist? */ -acpi_handle phandle; -if (acpi_get_parent (handle, &phandle)) -return NULL ; - dev =acpi_get_physical_device (phandle); - } -if (!dev) -return NULL ; -return dev; -} -EXPORT_SYMBOL (acpi_get_physical_pci_device); -static int acpi_bind_one (struct device *dev,acpi_handle handle) {struct acpi_device *acpi_dev;
include/acpi/acpi_bus.h +0 -1
@@ -368,7 +368,6 @@ struct acpi_bus_type {int register_acpi_bus_type (struct acpi_bus_type *);int unregister_acpi_bus_type (struct acpi_bus_type *);struct device *acpi_get_physical_device (acpi_handle ); -struct device *acpi_get_physical_pci_device (acpi_handle );/* helper */ acpi_handle acpi_get_child (acpi_handle ,acpi_integer );