Re: [PATCH v2 2/4] thunderbolt: Separate out common NHI bits

[Mika Westerberg]

On Tue, May 12, 2026 at 03:43:12PM +0200, Konrad Dybcio wrote:
> On 5/12/26 3:20 PM, Mika Westerberg wrote:
> > On Tue, May 12, 2026 at 03:06:58PM +0200, Konrad Dybcio wrote:
> >> On 5/4/26 8:54 AM, Mika Westerberg wrote:
> >>> Hi,
> 
> [...]
> 
> >>>> +/*
> >>>> + * 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_apple_add_links(struct tb_nhi *nhi)
> >>>
> >>> Okay you moved it here good. I think we can call it in nhi_pci_probe()
> >>> directly so no need to expose outside.
> >>
> >> Yeah that seems like a good idea. It's already there, behind N calls
> >> in the software CM case.
> >>
> >> Do we have to check the CM type though, or do you think it'd be fine
> >> to just call it unconditionally? (either because there are presumably
> >> no Apple machines with ICM or because these devlinks would be harmless?)
> > 
> > I think you can call it unconditionally. It only does something for TB1-2
> > Apple systems.
> > 
> > For Apple TB3 we used to start ICM firmware but this was changed as the
> > driver learned SW CM. However, we never setup any device links so this
> > would not change anything.
> 
> OK. I'm keeping tb_acpi_add_link() as-is, since that's both bus- and
> arch-independent.
> 
> However, doing just something like:
> 
> diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c
> index cb5d028de3bc..f5ddc8ddb8bb 100644
> --- a/drivers/thunderbolt/tb.c
> +++ b/drivers/thunderbolt/tb.c
> @@ -3327,7 +3327,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_acpi_add_links(nhi))
>                 tb_warn(tb, "device links to tunneled native ports are missing!\n");
> 
> 
> diff --git a/drivers/thunderbolt/pci.c b/drivers/thunderbolt/pci.c
> index ca50e3584cac..e0abd1d503c5 100644
> --- a/drivers/thunderbolt/pci.c
> +++ b/drivers/thunderbolt/pci.c
> @@ -294,6 +294,8 @@ static int nhi_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  
>         pci_set_master(pdev);
>  
> +       tb_apple_add_links(nhi)
> +
>         return nhi_probe(&nhi_pci->nhi);
>  }
> 
> 
> Will cause the warning to show up. And adding something like
> `nhi->device_links_done` is a little ugly.. Ideas?

Ah in Qualcomm case? We are going to add tb_of_add_links() as well, right (or
something along thoese lines)? Then tb.c does:

       if (!tb_acpi_add_links(nhi) && !tb_of_add_links(nhi))
                 tb_warn(tb, "device links to tunneled native ports are missing!\n");

In the meantime it is okay to have that warn because we really do want to
have those links in place :)