Re: [PATCH v5 2/2] thunderbolt: Add device links for Apple T2 NHI

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

On Mon, Jul 27, 2026 at 05:18:03PM -0400, Atharva Tiwari wrote:
> Ice Lake Thunderbolt NHI on T2 Macs (2018–2020). The NHI and its
> associated PCIe Root Ports all sit directly on the Root Complex
> with no upstream port. Identify the tunneled PCIe Root Ports by
> their PCI IDs and create device links back to the NHI so that
> PCIe tunnels can be re-established after sleep.
> 
> And on other T2 Thunderbolt NHI's, like Titan Ridge, the default method
> is used to add device links.
> 
> Co-developed-by: Andre Eikmeyer <dev@deq.rocks>
> Signed-off-by: Andre Eikmeyer <dev@deq.rocks>
> 
> Signed-off-by: Atharva Tiwari <atharvatiwarilinuxdev@gmail.com>
> ---
>  drivers/thunderbolt/tb.c | 54 ++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 52 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c
> index c69c323e6952..9b80e9339728 100644
> --- a/drivers/thunderbolt/tb.c
> +++ b/drivers/thunderbolt/tb.c
> @@ -9,6 +9,7 @@
>  #include <linux/slab.h>
>  #include <linux/errno.h>
>  #include <linux/delay.h>
> +#include <linux/pci.h>

We should not be calling PCI functions anymore from the SW CM core. We have
pci.c for that.

>  #include <linux/pm_runtime.h>
>  #include <linux/platform_data/x86/apple.h>
>  
> @@ -3305,21 +3306,70 @@ static const struct tb_cm_ops tb_cm_ops = {
>  static bool tb_apple_add_links(struct tb_nhi *nhi)

I wonder if we can put this to pci.c, rename it to tb_pci_add_links()
instead.

>  {
>  	struct pci_dev *upstream, *pdev;
> -	bool ret;
> +	bool ret = false;
>  
>  	if (!x86_apple_machine)
>  		return false;

BTW, why you need to differentiate T2 vs. the rest of Apple x86? Don't this
variable do?

>  
> +	/* On T2 Macs with Ice Lake Thunderbolt NHIs,
> +	 * identify the tunneled PCIe Root Ports by their PCI IDs
> +	 * and create device links so that
> +	 * PCIe tunnels can be re-established after sleep.
> +	 */
> +	if (has_apple_t2_chip && (nhi->pdev->device == PCI_DEVICE_ID_INTEL_ICL_NHI0 ||
> +				nhi->pdev->device == PCI_DEVICE_ID_INTEL_ICL_NHI1)) {
> +		unsigned int slot, func;
> +		const struct device_link *link;
> +
> +		for (slot = 0; slot < 32; slot++) {
> +			for (func = 0; func < 8; func++) {
> +				struct pci_dev *current_pdev __free(pci_dev_put) =
> +					pci_get_slot(nhi->pdev->bus, PCI_DEVFN(slot, func));

I'm not fan of __free() and the like so let's not use it here.

Also you don't need to scan all the slots. Just look for the tunneled
downstream ports based on their PCI IDs like we do already.

> +				if (!current_pdev)
> +					continue;
> +
> +				if (!pci_is_pcie(current_pdev) || pci_pcie_type(current_pdev) !=
> +						PCI_EXP_TYPE_ROOT_PORT)
> +					continue;
> +
> +				switch (current_pdev->device) {
> +				case 0x8a1d:
> +				case 0x8a1f:
> +				case 0x8a21:
> +				case 0x8a23:
> +					break;
> +				default:
> +					continue;
> +				}
> +
> +				link = device_link_add(&current_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(&current_pdev->dev));
> +					ret = true;
> +				} else {
> +					dev_warn(&nhi->pdev->dev,
> +						"device link creation from %s failed\n",
> +						dev_name(&current_pdev->dev));
> +				}
> +			}
> +		}
> +		return ret;
> +	}
> +
>  	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:
> +	case PCI_DEVICE_ID_INTEL_TITAN_RIDGE_2C_NHI:
> +	case PCI_DEVICE_ID_INTEL_TITAN_RIDGE_4C_NHI:
>  		break;
>  	default:
>  		return false;
>  	}
> -

This is unrelated change.

>  	upstream = pci_upstream_bridge(nhi->pdev);
>  	while (upstream) {
>  		if (!pci_is_pcie(upstream))
> -- 
> 2.43.0