ACPI / hotplug / PCI: Register all devices under the given bridge

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: NRafael J. Wysocki <rafael.j.wysocki@intel.com>
Tested-by: NMika Westerberg <mika.westerberg@linux.intel.com>

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

drivers/pci/hotplug/acpiphp_glue.c

@@ -65,20 +65,6 @@ static void acpiphp_set_hpp_values(struct pci_bus *bus);
 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 *kref)
 	}
 
 	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_handle handle, u32 lvl, void *data,
 	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_handle handle, u32 lvl, void *data,
 	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_handle handle, u32 lvl, void *data,
 
 	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);
 
-	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);
+	/* Register slots for ejectable funtions only. */
+	if (acpi_pci_check_ejectable(pbus, handle)  || is_dock_device(handle)) {
+		unsigned long long sun;
+		int retval;
 
-		bridge->nr_slots--;
-		mutex_lock(&bridge_mutex);
-		list_del(&slot->node);
-		mutex_unlock(&bridge_mutex);
-		kfree(slot);
-		status = AE_OK;
-		goto err;
+		bridge->nr_slots++;
+		status = acpi_evaluate_integer(handle, "_SUN", NULL, &sun);
+		if (ACPI_FAILURE(status))
+			sun = bridge->nr_slots;
+
+		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_handle handle, u32 lvl, void *data,
 		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_handle handle, u32 lvl, void *data,
 	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 acpiphp_bridge *bridge)
 	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 acpiphp_bridge *bridge)
 				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 acpiphp_slot *slot, struct pci_dev *dev)
 
 	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_handle handle)
 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 handle, u32 type, void *data)
 	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 handle, u32 type, void *data)
 	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 work_struct *work)
 
 	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_handle handle, u32 type, void *data)
 	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_handle handle, u32 type, void *data)
  */
 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_bus *bus)
 		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_bus *bus)
 	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_bus *bus)
 	 */
 	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_bus *bus)
 				     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 */