thunderbolt: Replace PCI quirk for downstream bridges

When resuming from system sleep, the downstream bridges of a Thunderbolt
controller must not resume before the NHI has reestablished the PCI
tunnels. However the NHI is not a parent of the downstream bridges,
so this requirement is not fulfilled automatically by the PM core and we
have to open code it instead. Up until now we've done this with a PCI
quirk, which has the following disadvantages:

- The code for the NHI resides in drivers/thunderbolt/, whereas the code
  for the downstream bridges resides in drivers/pci/quirks.c, which is
  somewhat surprising and non-obvious in particular for newcomers.

- Whenever support for an additional Thunderbolt controller is added,
  its PCI device ID needs to be amended in both places, which invites
  mistakes. E.g. commit a42fb35 ("thunderbolt: Allow loading of
  module on recent Apple MacBooks with thunderbolt 2 controller")
  relaxed the subvendor and subdevice ID in the NHI code but forgot to
  also change the downstream bridge code.

- Since the PCI quirk cannot keep any state, it has to search for the
  NHI over and over again on every resume.

Overcome these disadvantages by resuming the NHI from the upstream
bridge driver. The upstream bridge *is* a parent of the downstream
bridges, so this is functionally equivalent to the PCI quirk. Also
needs less code.

Cc: Andreas Noever <andreas.noever@gmail.com>
Signed-off-by: Lukas Wunner <lukas@wunner.de>

Author: l1k

drivers/pci/quirks.c

@@ -3291,59 +3291,6 @@ static void quirk_apple_poweroff_thunderbolt(struct pci_dev *dev)
 DECLARE_PCI_FIXUP_SUSPEND_LATE(PCI_VENDOR_ID_INTEL,
 			       PCI_DEVICE_ID_INTEL_CACTUS_RIDGE_4C,
 			       quirk_apple_poweroff_thunderbolt);
-
-/*
- * Apple: Wait for the thunderbolt controller to reestablish pci tunnels.
- *
- * During suspend the thunderbolt controller is reset and all pci
- * tunnels are lost. The NHI driver will try to reestablish all tunnels
- * during resume. We have to manually wait for the NHI since there is
- * no parent child relationship between the NHI and the tunneled
- * bridges.
- */
-static void quirk_apple_wait_for_thunderbolt(struct pci_dev *dev)
-{
-	struct pci_dev *sibling = NULL;
-	struct pci_dev *nhi = NULL;
-
-	if (!dmi_match(DMI_BOARD_VENDOR, "Apple Inc."))
-		return;
-	if (pci_pcie_type(dev) != PCI_EXP_TYPE_DOWNSTREAM)
-		return;
-	/*
-	 * Find the NHI and confirm that we are a bridge on the tb host
-	 * controller and not on a tb endpoint.
-	 */
-	sibling = pci_get_slot(dev->bus, 0x0);
-	if (sibling == dev)
-		goto out; /* we are the downstream bridge to the NHI */
-	if (!sibling || !sibling->subordinate)
-		goto out;
-	nhi = pci_get_slot(sibling->subordinate, 0x0);
-	if (!nhi)
-		goto out;
-	if (nhi->vendor != PCI_VENDOR_ID_INTEL
-		    || (nhi->device != PCI_DEVICE_ID_INTEL_LIGHT_RIDGE &&
-			nhi->device != PCI_DEVICE_ID_INTEL_CACTUS_RIDGE_4C &&
-			nhi->device != PCI_DEVICE_ID_INTEL_FALCON_RIDGE_4C_NHI)
-		    || nhi->subsystem_vendor != 0x2222
-		    || nhi->subsystem_device != 0x1111)
-		goto out;
-	dev_info(&dev->dev, "quirk: waiting for thunderbolt to reestablish PCI tunnels...\n");
-	device_pm_wait_for_dev(&dev->dev, &nhi->dev);
-out:
-	pci_dev_put(nhi);
-	pci_dev_put(sibling);
-}
-DECLARE_PCI_FIXUP_RESUME_EARLY(PCI_VENDOR_ID_INTEL,
-			       PCI_DEVICE_ID_INTEL_LIGHT_RIDGE,
-			       quirk_apple_wait_for_thunderbolt);
-DECLARE_PCI_FIXUP_RESUME_EARLY(PCI_VENDOR_ID_INTEL,
-			       PCI_DEVICE_ID_INTEL_CACTUS_RIDGE_4C,
-			       quirk_apple_wait_for_thunderbolt);
-DECLARE_PCI_FIXUP_RESUME_EARLY(PCI_VENDOR_ID_INTEL,
-			       PCI_DEVICE_ID_INTEL_FALCON_RIDGE_4C_BRIDGE,
-			       quirk_apple_wait_for_thunderbolt);
 #endif
 
 static void pci_do_fixups(struct pci_dev *dev, struct pci_fixup *f,

drivers/thunderbolt/nhi.c

@@ -503,7 +503,7 @@ static int nhi_suspend_noirq(struct device *dev)
 	return 0;
 }
 
-static int nhi_resume_noirq(struct device *dev)
+int nhi_resume_noirq(struct device *dev)
 {
 	struct pci_dev *pdev = to_pci_dev(dev);
 	struct tb *tb = pci_get_drvdata(pdev);
@@ -627,18 +627,18 @@ static void nhi_remove(struct pci_dev *pdev)
 }
 
 /*
- * The tunneled pci bridges are siblings of us. Use resume_noirq to reenable
- * the tunnels asap. A corresponding pci quirk blocks the downstream bridges
- * resume_noirq until we are done.
+ * .resume_noirq and .restore_noirq are not listed here because they're not
+ * called from the PM core, but from the upstream bridge driver. This ensures
+ * that the NHI can reestablish the pci tunnels before the downstream bridges
+ * and the devices attached to them resume. (The NHI is not a parent of the
+ * downstream bridges, but the upstream bridge is.)
  */
 static const struct dev_pm_ops nhi_pm_ops = {
 	.suspend_noirq = nhi_suspend_noirq,
-	.resume_noirq = nhi_resume_noirq,
 	.freeze_noirq = nhi_suspend_noirq, /*
 					    * we just disable hotplug, the
 					    * pci-tunnels stay alive.
 					    */
-	.restore_noirq = nhi_resume_noirq,
 	.runtime_suspend = nhi_suspend_noirq,
 	.runtime_resume = nhi_resume_noirq,
 };

drivers/thunderbolt/upstream.c

@@ -93,11 +93,36 @@
 
 struct tb_upstream {
 	struct pci_dev *nhi;
+	struct pci_dev *dsb0;
 	unsigned long long wake_gpe; /* hotplug interrupt during powerdown */
 	acpi_handle set_power; /* method to power controller up/down */
 	acpi_handle get_power; /* method to query power state */
 };
 
+int nhi_resume_noirq(struct device *dev);
+
+static int upstream_resume_noirq(struct pcie_device *dev)
+{
+	struct tb_upstream *upstream = get_service_data(dev);
+
+	if (!upstream->nhi->dev.driver)
+		return 0;
+
+	/*
+	 * During suspend the thunderbolt controller is reset and all pci
+	 * tunnels are lost. The NHI driver needs to reestablish all tunnels
+	 * before the downstream bridges resume. There is no parent child
+	 * relationship between the NHI and the tunneled bridges, but there is
+	 * between them and the upstream bridge. Hence the NHI needs to be
+	 * resumed by us rather than the PM core.
+	 */
+	pci_set_power_state(upstream->dsb0, PCI_D0);
+	pci_restore_state(upstream->dsb0);
+	pci_set_power_state(upstream->nhi, PCI_D0);
+	pci_restore_state(upstream->nhi);
+	return nhi_resume_noirq(&upstream->nhi->dev);
+}
+
 static int upstream_prepare(struct pcie_device *dev)
 {
 	struct tb_upstream *upstream = get_service_data(dev);
@@ -257,7 +282,6 @@ static int upstream_probe(struct pcie_device *dev)
 {
 	struct tb_upstream *upstream;
 	struct acpi_handle *nhi_handle;
-	struct pci_dev *dsb0;
 
 	/* host controllers only */
 	if (!dev->port->bus->self ||
@@ -269,13 +293,10 @@ static int upstream_probe(struct pcie_device *dev)
 		return -ENOMEM;
 
 	/* find Downstream Bridge 0 and NHI */
-	dsb0 = pci_get_slot(dev->port->subordinate, 0);
-	if (!dsb0 || !dsb0->subordinate) {
-		pci_dev_put(dsb0);
-		return -ENODEV;
-	}
-	upstream->nhi = pci_get_slot(dsb0->subordinate, 0);
-	pci_dev_put(dsb0);
+	upstream->dsb0 = pci_get_slot(dev->port->subordinate, 0);
+	if (!upstream->dsb0 || !upstream->dsb0->subordinate)
+		goto err;
+	upstream->nhi = pci_get_slot(upstream->dsb0->subordinate, 0);
 	if (!upstream->nhi || !pci_match_id(nhi_ids, upstream->nhi))
 		goto err;
 	nhi_handle = ACPI_HANDLE(&upstream->nhi->dev);
@@ -317,6 +338,7 @@ static int upstream_probe(struct pcie_device *dev)
 
 err:
 	pci_dev_put(upstream->nhi);
+	pci_dev_put(upstream->dsb0);
 	return -ENODEV;
 }
 
@@ -329,6 +351,7 @@ static void upstream_remove(struct pcie_device *dev)
 		dev_err(&dev->device, "cannot remove GPE handler\n");
 
 	pci_dev_put(upstream->nhi);
+	pci_dev_put(upstream->dsb0);
 	set_service_data(dev, NULL);
 }
 
@@ -342,4 +365,5 @@ struct pcie_port_service_driver upstream_driver = {
 	.complete		= upstream_complete,
 	.runtime_suspend	= upstream_runtime_suspend,
 	.runtime_resume		= upstream_runtime_resume,
+	.resume_noirq		= upstream_resume_noirq,
 };