Re: [PATCH v8 1/3] thunderbolt: Shift tb_apple_add_links to pci.c

[Mika Westerberg <mika.westerberg@linux.intel.com>]

On Mon, Aug 17, 2026 at 06:13:22AM -0400, Atharva Tiwari wrote:
> Shift tb_apple_add_links to pci.c as tb_apple_add_links calls
> pci functions.
> 
> Suggested-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> Signed-off-by: Atharva Tiwari <atharvatiwarilinuxdev@gmail.com>
> ---
>  drivers/thunderbolt/pci.c | 73 +++++++++++++++++++++++++++++++++++++++
>  drivers/thunderbolt/tb.c  | 72 +-------------------------------------
>  drivers/thunderbolt/tb.h  |  1 +
>  3 files changed, 75 insertions(+), 71 deletions(-)
> 
> diff --git a/drivers/thunderbolt/pci.c b/drivers/thunderbolt/pci.c
> index bbd186c29..c3d5a36ab 100644
> --- a/drivers/thunderbolt/pci.c
> +++ b/drivers/thunderbolt/pci.c
> @@ -18,6 +18,7 @@
>  #include <linux/property.h>
>  #include <linux/string_helpers.h>
>  #include <linux/suspend.h>
> +#include <linux/platform_data/x86/apple.h>
>  
>  #include "nhi.h"
>  #include "nhi_regs.h"
> @@ -261,6 +262,53 @@ static const struct tb_nhi_ops pci_nhi_default_ops = {
>  	.init_interrupts = nhi_pci_init_msi,
>  };
>  
> +static bool tb_pci_add_links_discrete(struct pci_dev *nhi_pdev)
> +{
> +	struct pci_dev *pdev, *upstream = pci_upstream_bridge(nhi_pdev);
> +	bool ret = false;
> +
> +	while (upstream) {
> +		if (!pci_is_pcie(upstream))
> +			return false;
> +		if (pci_pcie_type(upstream) == PCI_EXP_TYPE_UPSTREAM)
> +			break;
> +		upstream = pci_upstream_bridge(upstream);
> +	}
> +
> +	if (!upstream)
> +		return false;
> +
> +	/*
> +	 * For each hotplug downstream port, create add device link
> +	 * back to NHI so that PCIe tunnels can be re-established after
> +	 * sleep.
> +	 */
> +	pci_lock_rescan_remove();

Is this really needed? We are not running any of the PCI rescan or removal
functionality, I would expect it is enough to hold the references during
this.

If this turns out to be needed please add a line to the commit message
explaining why it is needed.

> +	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_pciehp)
> +			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));
> +			ret = true;
> +		} else {
> +			dev_warn(&nhi_pdev->dev, "device link creation from %s failed\n",
> +				 dev_name(&pdev->dev));
> +		}
> +	}
> +	pci_unlock_rescan_remove();
> +
> +	return ret;
> +}
>  /* Ice Lake specific NHI operations */
>  
>  #define ICL_LC_MAILBOX_TIMEOUT	500 /* ms */
> @@ -444,6 +492,31 @@ static const struct tb_nhi_ops icl_nhi_ops = {
>  	.init_interrupts = nhi_pci_init_msi,
>  };
>  
> +/*
> + * 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.
> + */
> +bool tb_pci_add_links(struct tb_nhi *nhi)
> +{
> +	struct pci_dev *nhi_pdev = to_pci_dev(nhi->dev);
> +
> +	if (!x86_apple_machine)
> +		return false;
> +
> +	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:
> +		return tb_pci_add_links_discrete(nhi_pdev);
> +	default:
> +		return false;
> +	}
> +}
> +
>  static int nhi_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  {
>  	struct device *dev = &pdev->dev;
> diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c
> index f43f2d952..9c2fc7ff1 100644
> --- a/drivers/thunderbolt/tb.c
> +++ b/drivers/thunderbolt/tb.c
> @@ -10,7 +10,6 @@
>  #include <linux/errno.h>
>  #include <linux/delay.h>
>  #include <linux/pm_runtime.h>
> -#include <linux/platform_data/x86/apple.h>
>  
>  #include "tb.h"
>  #include "tb_regs.h"
> @@ -3302,75 +3301,6 @@ static const struct tb_cm_ops tb_cm_ops = {
>  	.disconnect_xdomain_paths = tb_disconnect_xdomain_paths,
>  };
>  
> -/*
> - * 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 bool tb_apple_add_links(struct tb_nhi *nhi)
> -{
> -	struct pci_dev *nhi_pdev = to_pci_dev(nhi->dev);
> -	struct pci_dev *upstream, *pdev;
> -	bool ret;
> -
> -	if (!x86_apple_machine)
> -		return false;
> -
> -	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 false;
> -	}
> -
> -	upstream = pci_upstream_bridge(nhi_pdev);
> -	while (upstream) {
> -		if (!pci_is_pcie(upstream))
> -			return false;
> -		if (pci_pcie_type(upstream) == PCI_EXP_TYPE_UPSTREAM)
> -			break;
> -		upstream = pci_upstream_bridge(upstream);
> -	}
> -
> -	if (!upstream)
> -		return false;
> -
> -	/*
> -	 * For each hotplug downstream port, create add device link
> -	 * back to NHI so that PCIe tunnels can be re-established after
> -	 * sleep.
> -	 */
> -	ret = false;
> -	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_pciehp)
> -			continue;
> -
> -		link = device_link_add(&pdev->dev, nhi->dev,
> -				       DL_FLAG_AUTOREMOVE_SUPPLIER |
> -				       DL_FLAG_PM_RUNTIME);
> -		if (link) {
> -			dev_dbg(nhi->dev, "created link from %s\n",
> -				dev_name(&pdev->dev));
> -			ret = true;
> -		} else {
> -			dev_warn(nhi->dev, "device link creation from %s failed\n",
> -				 dev_name(&pdev->dev));
> -		}
> -	}
> -
> -	return ret;
> -}
> -
>  struct tb *tb_probe(struct tb_nhi *nhi)
>  {
>  	struct tb_cm *tcm;
> @@ -3400,7 +3330,7 @@ struct tb *tb_probe(struct tb_nhi *nhi)
>  	 * before the PCIe/USB stack is resumed so complain here if we
>  	 * found them missing.
>  	 */
> -	if (!tb_apple_add_links(nhi) && !tb_acpi_add_links(nhi))
> +	if (!tb_pci_add_links(nhi) && !tb_acpi_add_links(nhi))
>  		tb_warn(tb, "device links to tunneled native ports are missing!\n");
>  
>  	return tb;
> diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
> index ec9192b61..6a285fe7f 100644
> --- a/drivers/thunderbolt/tb.h
> +++ b/drivers/thunderbolt/tb.h
> @@ -1506,6 +1506,7 @@ static inline bool usb4_port_device_is_offline(const struct usb4_port *usb4)
>  }
>  
>  void tb_check_quirks(struct tb_switch *sw);
> +bool tb_pci_add_links(struct tb_nhi *nhi);
>  
>  #ifdef CONFIG_ACPI
>  bool tb_acpi_add_links(struct tb_nhi *nhi);
> -- 
> 2.43.0