At Fri, 03 Mar 2006 10:04:34 -0800,
Kristen Accardi <[EMAIL PROTECTED]> wrote:
> 
> So, this seems unlikely to me, but I just wanted to double check:  Is it
> possible that acpiphp_bus_trim will ever return an error, but there is a
> pci_dev struct that exists for this func?  If that's the case, then
> wouldn't this leave a reference to pci_dev around since pci_dev_put
> would never be called?

Hi Kristen,
I apologize for my late reply and thank you for your comment.

I think it might be happen at the following case.

 o PCI device is inserted.
 o acpi_bus_add() which has called from enable_device() failed.
 o But rest of the processes were finished cleanly.

In that case, acpiphp_bus_trim() fails. So, I stop to check
the return value of the acpiphp_bus_trim(). disable_device()
checks the existence of func->pci_dev whether
acpiphp_bus_trim() is successful or not. If it exists,
acpiphp keeps removing the PCI devices.

This patch is against 2.6.16-rc5-mm3 plus following patches.

 o Kristen's latest patch set
 o Kenji's acpiphp: Scan slots under the nested P2P bridge

I tested this patch on my tiger4 box except for dock eject
case, because I don't have any pc which has _DCK method.

Thanks,
MUNE

--
Current acpiphp does not free acpi_device structs when the
PCI devices are removed. When the PCI device is added,
acpi_bus_add() fails because acpi_device struct has already
exists. So, _PRT method does not evaluate.

This patch fixes this issue.

Signed-off-by: MUNEDA Takahiro <[EMAIL PROTECTED]>

 drivers/pci/hotplug/acpiphp_glue.c |   71 +++++++++++++++++--------------------
 1 files changed, 33 insertions(+), 38 deletions(-)

Index: linux-2.6.16-rc5-mm3/drivers/pci/hotplug/acpiphp_glue.c
===================================================================
--- linux-2.6.16-rc5-mm3.orig/drivers/pci/hotplug/acpiphp_glue.c
+++ linux-2.6.16-rc5-mm3/drivers/pci/hotplug/acpiphp_glue.c
@@ -841,36 +841,6 @@ static unsigned char acpiphp_max_busnr(s
 }
 
 
-
-/**
- *  get_func - get a pointer to acpiphp_func given a slot, device
- *  @slot: slot to search
- *  @dev:  pci_dev struct to match.
- *
- *  This function will increase the reference count of pci_dev,
- *  so callers should call pci_dev_put when complete.
- *
- */
-static struct acpiphp_func *
-get_func(struct acpiphp_slot *slot, struct pci_dev *dev)
-{
-       struct acpiphp_func *func = NULL;
-       struct pci_bus *bus = slot->bridge->pci_bus;
-       struct pci_dev *pdev;
-
-       list_for_each_entry(func, &slot->funcs, sibling) {
-               pdev = pci_get_slot(bus, PCI_DEVFN(slot->device,
-                                       func->function));
-               if (pdev) {
-                       if (pdev == dev)
-                               break;
-                       pci_dev_put(pdev);
-               }
-       }
-       return func;
-}
-
-
 /**
  * acpiphp_bus_add - add a new bus to acpi subsystem
  * @func: acpiphp_func of the bridge
@@ -917,6 +887,28 @@ acpiphp_bus_add_out:
 }
 
 
+/**
+ * acpiphp_bus_trim - trim a bus from acpi subsystem
+ * @handle: handle to acpi namespace
+ *
+ */
+int acpiphp_bus_trim(acpi_handle handle)
+{
+       struct acpi_device *device;
+       int retval;
+
+       retval = acpi_bus_get_device(handle, &device);
+       if (retval) {
+               dbg("acpi_device not found\n");
+               return retval;
+       }
+
+       retval = acpi_bus_trim(device, 1);
+       if (retval)
+               err("cannot remove from acpi list\n");
+
+       return retval;
+}
 
 /**
  * enable_device - enable, configure a slot
@@ -963,19 +955,17 @@ static int enable_device(struct acpiphp_
                        if (dev->hdr_type == PCI_HEADER_TYPE_BRIDGE ||
                            dev->hdr_type == PCI_HEADER_TYPE_CARDBUS) {
                                max = pci_scan_bridge(bus, dev, max, pass);
-                               if (pass && dev->subordinate) {
+                               if (pass && dev->subordinate)
                                        pci_bus_size_bridges(dev->subordinate);
-                                       func = get_func(slot, dev);
-                                       if (func) {
-                                               acpiphp_bus_add(func);
-                                               /* side effect of get_func */
-                                               pci_dev_put(dev);
-                                       }
-                               }
                        }
                }
        }
 
+       list_for_each (l, &slot->funcs) {
+               func = list_entry(l, struct acpiphp_func, sibling);
+               acpiphp_bus_add(func);
+       }
+
        pci_bus_assign_resources(bus);
        acpiphp_sanitize_bus(bus);
        pci_enable_bridges(bus);
@@ -1012,6 +1002,11 @@ static int disable_device(struct acpiphp
 
        list_for_each (l, &slot->funcs) {
                func = list_entry(l, struct acpiphp_func, sibling);
+
+               acpiphp_bus_trim(func->handle);
+               /* try to remove anyway.
+                * acpiphp_bus_add might have been failed */
+
                if (!func->pci_dev)
                        continue;
 
-
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

Reply via email to