| Message ID | 20200819115905.59834-17-mika.westerberg@linux.intel.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | thunderbolt: Power Management improvements | expand |
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@google.com> > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> Acked-by: Bjorn Helgaas <bhelgaas@google.com> > --- > 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 >
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);
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@google.com> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> --- drivers/pci/quirks.c | 57 --------------------------------- drivers/thunderbolt/nhi.c | 66 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 57 deletions(-)