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

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

On Tue, 21 Jul 2026, Atharva Tiwari wrote:

> From: Andre Eikmeyer <andre@negmaster.com>
> 
> 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. Apple's ACPI tables name Thunderbolt root
> ports as TRP0, TRP1, etc. Find them and create device links back
> to the NHI so that PCIe tunnels can be re-established after sleep.
> 
> Co-developed-by: Atharva Tiwari <atharvatiwarilinuxdev@gmail.com>
> Signed-off-by: Atharva Tiwari <atharvatiwarilinuxdev@gmail.com>
> 
> Signed-off-by: Andre Eikmeyer <andre@negmaster.com>
> ---
>  drivers/thunderbolt/tb.c | 51 +++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 50 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c
> index c69c323e6952..0cdffcf577cd 100644
> --- a/drivers/thunderbolt/tb.c
> +++ b/drivers/thunderbolt/tb.c
> @@ -6,6 +6,7 @@
>   * Copyright (C) 2019, Intel Corporation
>   */
>  
> +#include <linux/acpi.h>
>  #include <linux/slab.h>
>  #include <linux/errno.h>
>  #include <linux/delay.h>
> @@ -3305,11 +3306,59 @@ static const struct tb_cm_ops tb_cm_ops = {
>  static bool tb_apple_add_links(struct tb_nhi *nhi)
>  {
>  	struct pci_dev *upstream, *pdev;

This file seems to have a pre-existing lack of pci.h include, please add 
it now when doing pci related code.

> -	bool ret;
> +	bool ret = false;
>  
>  	if (!x86_apple_machine)
>  		return false;
>  
> +	/* On T2 Macs. the root ports are stored in ACPI as TRP0,
> +	 * TRP1, etc. Find them and create device links
> +	 * so that PCIe tunnels can be re-established after
> +	 * sleep.
> +	 */
> +	if (has_apple_t2_chip && IS_ENABLED(CONFIG_ACPI)) {
> +		struct acpi_device *adev;
> +		unsigned int slot, func;
> +		const struct device_link *link;
> +		const char *bid;
> +
> +		for (slot = 0; slot < 32; slot++) {
> +			for (func = 0; func < 8; func++) {
> +				pdev = pci_get_slot(nhi->pdev->bus, PCI_DEVFN(slot, func));
> +				if (!pdev)
> +					continue;
> +
> +				if (!pci_is_pcie(pdev) || pci_pcie_type(pdev) !=
> +						PCI_EXP_TYPE_ROOT_PORT)
> +					goto put_pdev;
> +
> +				adev = ACPI_COMPANION(&pdev->dev);
> +				if (!adev)
> +					goto put_pdev;
> +
> +				bid = acpi_device_bid(adev);
> +				if (strncmp(bid, "TRP", 3) != 0)
> +					goto put_pdev;
> +
> +				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));

Please only use balanced braces.

> +
> +put_pdev:
> +	pci_dev_put(pdev);

I think you can use __free() with it and avoid the label. You cannot then 
reuse the existing pdev variable for it but declare it in this block while 
you assign to it.

This line would be misaligned anyway but better get rid of goto logic 
anyway.

> +			}
> +		}
> +		return ret;
> +	}
> +
>  	switch (nhi->pdev->device) {
>  	case PCI_DEVICE_ID_INTEL_LIGHT_RIDGE:
>  	case PCI_DEVICE_ID_INTEL_CACTUS_RIDGE_4C:
> 

-- 
 i.