From: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx> Rework register_slot() to create a struct acpiphp_func object for every function it is called for and to create acpiphp slots for all of them. Although acpiphp_register_hotplug_slot() is only called for the slots whose functions are identified as "ejectable", so that user space can manipulate them, the ACPIPHP notify handler, handle_hotplug_event(), is now installed for all of the registered functions (that aren't dock stations) and hotplug events may be handled for all of them. As a result, essentially, all PCI bridges represented by objects in the ACPI namespace are now going to be "hotplug" bridges and that may affect resources allocation in general, although it shouldn't lead to problems. This allows the code to be simplified substantially and addresses the problem where bus check or device check notifications for some PCI bridges or devices are not handled, because those devices are not recognized as "ejectable" or there appear to be no "ejectable" devices under those bridges. Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx> --- drivers/pci/hotplug/acpiphp.h | 1 drivers/pci/hotplug/acpiphp_glue.c | 230 ++++++++++++------------------------- 2 files changed, 75 insertions(+), 156 deletions(-) Index: linux-pm/drivers/pci/hotplug/acpiphp_glue.c =================================================================== --- linux-pm.orig/drivers/pci/hotplug/acpiphp_glue.c +++ linux-pm/drivers/pci/hotplug/acpiphp_glue.c @@ -65,20 +65,6 @@ static void acpiphp_set_hpp_values(struc static void hotplug_event(acpi_handle handle, u32 type, void *data); static void free_bridge(struct kref *kref); -/* callback routine to check for the existence of a pci dock device */ -static acpi_status -is_pci_dock_device(acpi_handle handle, u32 lvl, void *context, void **rv) -{ - int *count = (int *)context; - - if (is_dock_device(handle)) { - (*count)++; - return AE_CTRL_TERMINATE; - } else { - return AE_OK; - } -} - static void acpiphp_context_handler(acpi_handle handle, void *context) { /* Intentionally empty. */ @@ -179,14 +165,16 @@ static void free_bridge(struct kref *kre } context = bridge->context; - /* Release the reference acquired by acpiphp_enumerate_slots(). */ - if (context->handler_for_func) + /* Root bridges will not have hotplug context. */ + if (context) { + /* Release the reference taken by acpiphp_enumerate_slots(). */ put_bridge(context->func->slot->bridge); + context->bridge = NULL; + acpiphp_put_context(context); + } put_device(&bridge->pci_bus->dev); pci_dev_put(bridge->pci_dev); - context->bridge = NULL; - acpiphp_put_context(context); kfree(bridge); mutex_unlock(&acpiphp_context_lock); @@ -282,28 +270,24 @@ static acpi_status register_slot(acpi_ha struct acpiphp_slot *slot; struct acpiphp_func *newfunc; acpi_status status = AE_OK; - unsigned long long adr, sun; - int device, function, retval; + unsigned long long adr; + int device, function; struct pci_bus *pbus = bridge->pci_bus; - struct pci_dev *pdev; + struct pci_dev *pdev = bridge->pci_dev; u32 val; - if (!acpi_pci_check_ejectable(pbus, handle) && !is_dock_device(handle)) + if (pdev && device_is_managed_by_native_pciehp(pdev)) return AE_OK; status = acpi_evaluate_integer(handle, "_ADR", NULL, &adr); if (ACPI_FAILURE(status)) { - warn("can't evaluate _ADR (%#x)\n", status); + acpi_handle_warn(handle, "can't evaluate _ADR (%#x)\n", status); return AE_OK; } device = (adr >> 16) & 0xffff; function = adr & 0xffff; - pdev = bridge->pci_dev; - if (pdev && device_is_managed_by_native_pciehp(pdev)) - return AE_OK; - newfunc = kzalloc(sizeof(struct acpiphp_func), GFP_KERNEL); if (!newfunc) return AE_NO_MEMORY; @@ -338,23 +322,10 @@ static acpi_status register_slot(acpi_ha if (acpi_has_method(handle, "_DCK")) newfunc->flags |= FUNC_HAS_DCK; - status = acpi_evaluate_integer(handle, "_SUN", NULL, &sun); - if (ACPI_FAILURE(status)) { - /* - * use the count of the number of slots we've found - * for the number of the slot - */ - sun = bridge->nr_slots+1; - } - /* search for objects that share the same slot */ list_for_each_entry(slot, &bridge->slots, node) - if (slot->device == device) { - if (slot->sun != sun) - warn("sibling found, but _SUN doesn't match!\n"); - + if (slot->device == device) goto slot_found; - } slot = kzalloc(sizeof(struct acpiphp_slot), GFP_KERNEL); if (!slot) { @@ -364,34 +335,38 @@ static acpi_status register_slot(acpi_ha slot->bridge = bridge; slot->device = device; - slot->sun = sun; INIT_LIST_HEAD(&slot->funcs); mutex_init(&slot->crit_sect); mutex_lock(&bridge_mutex); list_add_tail(&slot->node, &bridge->slots); mutex_unlock(&bridge_mutex); - bridge->nr_slots++; - dbg("found ACPI PCI Hotplug slot %llu at PCI %04x:%02x:%02x\n", - slot->sun, pci_domain_nr(pbus), pbus->number, device); + /* Register slots for ejectable funtions only. */ + if (acpi_pci_check_ejectable(pbus, handle) || is_dock_device(handle)) { + unsigned long long sun; + int retval; - retval = acpiphp_register_hotplug_slot(slot); - if (retval) { - if (retval == -EBUSY) - warn("Slot %llu already registered by another " - "hotplug driver\n", slot->sun); - else - warn("acpiphp_register_hotplug_slot failed " - "(err code = 0x%x)\n", retval); + bridge->nr_slots++; + status = acpi_evaluate_integer(handle, "_SUN", NULL, &sun); + if (ACPI_FAILURE(status)) + sun = bridge->nr_slots; - bridge->nr_slots--; - mutex_lock(&bridge_mutex); - list_del(&slot->node); - mutex_unlock(&bridge_mutex); - kfree(slot); - status = AE_OK; - goto err; + slot->sun = sun; + dbg("found ACPI PCI Hotplug slot %llu at PCI %04x:%02x:%02x\n", + slot->sun, pci_domain_nr(pbus), pbus->number, device); + + retval = acpiphp_register_hotplug_slot(slot); + if (retval) { + bridge->nr_slots--; + if (retval == -EBUSY) + warn("Slot %llu already registered by another " + "hotplug driver\n", slot->sun); + else + warn("acpiphp_register_hotplug_slot failed " + "(err code = 0x%x)\n", retval); + } + /* Even if the slot registration fails, we can still use it. */ } slot_found: @@ -421,10 +396,9 @@ static acpi_status register_slot(acpi_ha status = acpi_install_notify_handler(handle, ACPI_SYSTEM_NOTIFY, handle_hotplug_event, context); - if (ACPI_SUCCESS(status)) - context->handler_for_func = true; - else - err("failed to register interrupt notify handler\n"); + if (ACPI_FAILURE(status)) + acpi_handle_err(handle, + "failed to install notify handler\n"); } return AE_OK; @@ -438,18 +412,6 @@ static acpi_status register_slot(acpi_ha return status; } - -/* see if it's worth looking at this bridge */ -static int detect_ejectable_slots(acpi_handle handle) -{ - int found = acpi_pci_detect_ejectable(handle); - if (!found) { - acpi_walk_namespace(ACPI_TYPE_DEVICE, handle, (u32)1, - is_pci_dock_device, NULL, (void *)&found, NULL); - } - return found; -} - static struct acpiphp_bridge *acpiphp_handle_to_bridge(acpi_handle handle) { struct acpiphp_context *context; @@ -473,14 +435,6 @@ static void cleanup_bridge(struct acpiph struct acpiphp_slot *slot; struct acpiphp_func *func; acpi_status status; - acpi_handle handle = bridge->handle; - - if (!bridge->context->handler_for_func) { - status = acpi_remove_notify_handler(handle, ACPI_SYSTEM_NOTIFY, - handle_hotplug_event); - if (ACPI_FAILURE(status)) - err("failed to remove notify handler\n"); - } list_for_each_entry(slot, &bridge->slots, node) { list_for_each_entry(func, &slot->funcs, sibling) { @@ -488,7 +442,6 @@ static void cleanup_bridge(struct acpiph unregister_hotplug_dock_device(func->handle); } if (!(func->flags & FUNC_HAS_DCK)) { - func->context->handler_for_func = false; status = acpi_remove_notify_handler(func->handle, ACPI_SYSTEM_NOTIFY, handle_hotplug_event); @@ -678,9 +631,7 @@ static void check_hotplug_bridge(struct list_for_each_entry(func, &slot->funcs, sibling) { if (PCI_FUNC(dev->devfn) == func->function) { - /* check if this bridge has ejectable slots */ - if ((detect_ejectable_slots(func->handle) > 0)) - dev->is_hotplug_bridge = 1; + dev->is_hotplug_bridge = 1; break; } } @@ -988,8 +939,8 @@ void acpiphp_check_host_bridge(acpi_hand static void hotplug_event(acpi_handle handle, u32 type, void *data) { struct acpiphp_context *context = data; + struct acpiphp_func *func = context->func; struct acpiphp_bridge *bridge; - struct acpiphp_func *func; char objname[64]; struct acpi_buffer buffer = { .length = sizeof(objname), .pointer = objname }; @@ -999,11 +950,6 @@ static void hotplug_event(acpi_handle ha if (bridge) get_bridge(bridge); - /* - * If context->func is not NULL, we are holding a reference to its - * parent bridge, so it won't go away until we drop that reference. - */ - func = context->func; mutex_unlock(&acpiphp_context_lock); acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer); @@ -1041,9 +987,6 @@ static void hotplug_event(acpi_handle ha case ACPI_NOTIFY_EJECT_REQUEST: /* request device eject */ dbg("%s: Device eject notify on %s\n", __func__, objname); - if (!func) - break; - if (bridge && !(bridge->flags & BRIDGE_HAS_EJ0)) break; @@ -1090,14 +1033,7 @@ static void hotplug_event_work(struct wo acpi_scan_lock_release(); kfree(hp_work); /* allocated in handle_hotplug_event() */ - - mutex_lock(&acpiphp_context_lock); - if (context->func) - put_bridge(context->func->slot->bridge); - else - acpiphp_put_context(context); - - mutex_unlock(&acpiphp_context_lock); + put_bridge(context->func->slot->bridge); } /** @@ -1115,13 +1051,8 @@ static void handle_hotplug_event(acpi_ha mutex_lock(&acpiphp_context_lock); context = acpiphp_get_context(handle); if (context) { - if (context->func) { - get_bridge(context->func->slot->bridge); - acpiphp_put_context(context); - } else if (!context->bridge) { - acpiphp_put_context(context); - context = NULL; - } + get_bridge(context->func->slot->bridge); + acpiphp_put_context(context); } mutex_unlock(&acpiphp_context_lock); /* @@ -1142,7 +1073,6 @@ static void handle_hotplug_event(acpi_ha */ void acpiphp_enumerate_slots(struct pci_bus *bus) { - struct acpiphp_context *context; struct acpiphp_bridge *bridge; acpi_handle handle; acpi_status status; @@ -1151,7 +1081,7 @@ void acpiphp_enumerate_slots(struct pci_ return; handle = ACPI_HANDLE(bus->bridge); - if (!handle || detect_ejectable_slots(handle) <= 0) + if (!handle) return; bridge = kzalloc(sizeof(struct acpiphp_bridge), GFP_KERNEL); @@ -1166,21 +1096,6 @@ void acpiphp_enumerate_slots(struct pci_ bridge->pci_dev = pci_dev_get(bus->self); bridge->pci_bus = bus; - mutex_lock(&acpiphp_context_lock); - context = acpiphp_get_context(handle); - if (!context) { - context = acpiphp_init_context(handle); - if (!context) { - mutex_unlock(&acpiphp_context_lock); - acpi_handle_err(handle, "No hotplug context\n"); - kfree(bridge); - return; - } - } - bridge->context = context; - context->bridge = bridge; - mutex_unlock(&acpiphp_context_lock); - /* * Grab a ref to the subordinate PCI bus in case the bus is * removed via PCI core logical hotplug. The ref pins the bus @@ -1188,6 +1103,35 @@ void acpiphp_enumerate_slots(struct pci_ */ get_device(&bus->dev); + if (!pci_is_root_bus(bridge->pci_bus)) { + struct acpiphp_context *context; + + /* + * This bridge should have been registered as a hotplug function + * under its parent, so the context has to be there. If not, we + * are in deep goo. + */ + mutex_lock(&acpiphp_context_lock); + context = acpiphp_get_context(handle); + if (WARN_ON(!context || !context->func)) { + mutex_unlock(&acpiphp_context_lock); + put_device(&bus->dev); + kfree(bridge); + return; + } + bridge->context = context; + context->bridge = bridge; + /* Get a reference to the parent bridge. */ + get_bridge(context->func->slot->bridge); + mutex_unlock(&acpiphp_context_lock); + } + + status = acpi_get_handle(bridge->handle, "_EJ0", &handle); + if (ACPI_SUCCESS(status)) { + dbg("found ejectable p2p bridge\n"); + bridge->flags |= BRIDGE_HAS_EJ0; + } + /* must be added to the list prior to calling register_slot */ mutex_lock(&bridge_mutex); list_add(&bridge->list, &bridge_list); @@ -1198,33 +1142,9 @@ void acpiphp_enumerate_slots(struct pci_ register_slot, NULL, bridge, NULL); if (ACPI_FAILURE(status)) { acpi_handle_err(bridge->handle, "failed to register slots\n"); - goto err; - } - - if (pci_is_root_bus(bridge->pci_bus)) - return; - - if (acpi_has_method(bridge->handle, "_EJ0")) { - dbg("found ejectable p2p bridge\n"); - bridge->flags |= BRIDGE_HAS_EJ0; - } - if (context->handler_for_func) { - /* Notify handler already installed. */ - get_bridge(context->func->slot->bridge); - return; + cleanup_bridge(bridge); + put_bridge(bridge); } - - /* install notify handler for P2P bridges */ - status = acpi_install_notify_handler(bridge->handle, ACPI_SYSTEM_NOTIFY, - handle_hotplug_event, NULL); - if (ACPI_SUCCESS(status)) - return; - - acpi_handle_err(bridge->handle, "failed to register notify handler\n"); - - err: - cleanup_bridge(bridge); - put_bridge(bridge); } /* Destroy hotplug slots associated with the PCI bus */ Index: linux-pm/drivers/pci/hotplug/acpiphp.h =================================================================== --- linux-pm.orig/drivers/pci/hotplug/acpiphp.h +++ linux-pm/drivers/pci/hotplug/acpiphp.h @@ -134,7 +134,6 @@ struct acpiphp_context { struct acpiphp_func *func; struct acpiphp_bridge *bridge; unsigned int refcount; - bool handler_for_func; }; /* -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html