On Wed, Aug 19, 2020 at 02:59:02PM +0300, Mika Westerberg wrote:
> On older Apple systems there is currently a PCI quirk in place to block
> resume of tunneled PCIe ports until NHI (Thunderbolt controller) is
> resumed. This makes sure the PCIe tunnels are re-established before PCI
> core notices it.
>
> With device links the same thing can be done without quirks. The driver
> core will make sure the supplier (NHI) is resumed before consumers (PCIe
> downstream ports).
>
> For this reason switch the Thunderbolt driver to use device links and
> remove the PCI quirk.
>
> Cc: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
> Signed-off-by: Mika Westerberg <mika.westerberg@xxxxxxxxxxxxxxx>
Acked-by: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
> ---
> drivers/pci/quirks.c | 57 ---------------------------------
> drivers/thunderbolt/nhi.c | 66 +++++++++++++++++++++++++++++++++++++++
> 2 files changed, 66 insertions(+), 57 deletions(-)
>
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index bdf9b52567e0..a25471436523 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -3673,63 +3673,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 (!x86_apple_machine)
> - return;
> - if (pci_pcie_type(dev) != PCI_EXP_TYPE_DOWNSTREAM)
> - return;
> -
> - /*
> - * Find the NHI and confirm that we are a bridge on the Thunderbolt
> - * host controller and not on a Thunderbolt 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_2C_NHI &&
> - nhi->device != PCI_DEVICE_ID_INTEL_FALCON_RIDGE_4C_NHI)
> - || nhi->class != PCI_CLASS_SYSTEM_OTHER << 8)
> - goto out;
> - pci_info(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_2C_BRIDGE,
> - 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
>
> /*
> diff --git a/drivers/thunderbolt/nhi.c b/drivers/thunderbolt/nhi.c
> index 24d2b7eff59b..e499fe78756b 100644
> --- a/drivers/thunderbolt/nhi.c
> +++ b/drivers/thunderbolt/nhi.c
> @@ -17,6 +17,7 @@
> #include <linux/module.h>
> #include <linux/delay.h>
> #include <linux/property.h>
> +#include <linux/platform_data/x86/apple.h>
>
> #include "nhi.h"
> #include "nhi_regs.h"
> @@ -1069,6 +1070,69 @@ static bool nhi_imr_valid(struct pci_dev *pdev)
> return true;
> }
>
> +/*
> + * During suspend the Thunderbolt controller is reset and all PCIe
> + * tunnels are lost. The NHI driver will try to reestablish all tunnels
> + * during resume. This adds device links between the tunneled PCIe
> + * downstream ports and the NHI so that the device core will make sure
> + * NHI is resumed first before the rest.
> + */
> +static void tb_apple_add_links(struct tb_nhi *nhi)
> +{
> + struct pci_dev *upstream, *pdev;
> +
> + if (!x86_apple_machine)
> + return;
> +
> + switch (nhi->pdev->device) {
> + case PCI_DEVICE_ID_INTEL_LIGHT_RIDGE:
> + case PCI_DEVICE_ID_INTEL_CACTUS_RIDGE_4C:
> + case PCI_DEVICE_ID_INTEL_FALCON_RIDGE_2C_NHI:
> + case PCI_DEVICE_ID_INTEL_FALCON_RIDGE_4C_NHI:
> + break;
> + default:
> + return;
> + }
> +
> + upstream = pci_upstream_bridge(nhi->pdev);
> + while (upstream) {
> + if (!pci_is_pcie(upstream))
> + return;
> + if (pci_pcie_type(upstream) == PCI_EXP_TYPE_UPSTREAM)
> + break;
> + upstream = pci_upstream_bridge(upstream);
> + }
> +
> + if (!upstream)
> + return;
> +
> + /*
> + * For each hotplug downstream port, create add device link
> + * back to NHI so that PCIe tunnels can be re-established after
> + * sleep.
> + */
> + for_each_pci_bridge(pdev, upstream->subordinate) {
> + const struct device_link *link;
> +
> + if (!pci_is_pcie(pdev))
> + continue;
> + if (pci_pcie_type(pdev) != PCI_EXP_TYPE_DOWNSTREAM ||
> + !pdev->is_hotplug_bridge)
> + continue;
> +
> + link = device_link_add(&pdev->dev, &nhi->pdev->dev,
> + DL_FLAG_AUTOREMOVE_SUPPLIER |
> + DL_FLAG_PM_RUNTIME);
> + if (link) {
> + dev_dbg(&nhi->pdev->dev, "created link from %s\n",
> + dev_name(&pdev->dev));
> + } else {
> + dev_warn(&nhi->pdev->dev, "device link creation from %s failed\n",
> + dev_name(&pdev->dev));
> + }
> + }
> +}
> +
> static int nhi_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> {
> struct tb_nhi *nhi;
> @@ -1134,6 +1198,8 @@ static int nhi_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> return res;
> }
>
> + tb_apple_add_links(nhi);
> +
> tb = icm_probe(nhi);
> if (!tb)
> tb = tb_probe(nhi);
> --
> 2.28.0
>
