Re: [PATCH v3] thunderbolt: Assert downstream port reset on shutdown

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

On Wed, Jun 10, 2026 at 05:02:56PM +0530, Basavaraj Natikar wrote:
> 
> On 6/10/2026 4:27 PM, Mika Westerberg wrote:
> > Hi,
> > 
> > On Wed, Jun 10, 2026 at 02:20:06PM +0530, Basavaraj Natikar wrote:
> > > On shutdown the connection manager tears down the router tree without
> > > signalling connected devices. A Thunderbolt 3 device directly connected
> > > to a USB4 host never receives a disconnect indication and during shutdown
> > > this can cause polling the dead link for up to 60 seconds. On some
> > > platforms this behavior leads to a warm reset instead of a shutdown due
> > > to this timeout.
> > > 
> > > Fix this by asserting PORT_CS_19.DPR on each connected downstream port
> > > before tearing down the router tree. This drives SBTX low (USB4 spec
> > > section 6.9), causing the device to detect SBRX low and transition to
> > > Uninitialized Unplugged state immediately.
> > > 
> > > Always do this on system shutdown/reboot by forcing host_reset in the
> > > PCI ->shutdown callback. On plain driver unload only do it when the host
> > > router was actually reset on load (host_reset=1), since in that case the
> > > tunnels are not preserved across reload anyway; with host_reset=0 the
> > > tunnels are kept alive across unload/reload so the links are left intact.
> > > Restrict the reset to Thunderbolt 3 devices.
> > > 
> > > Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>
> > > Co-developed-by: Sanath S <Sanath.S@amd.com>
> > > Signed-off-by: Sanath S <Sanath.S@amd.com>
> > > Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
> > > ---
> > > v3:
> > >   - Rebase on top of the thunderbolt next branch; the PCI ->shutdown hook
> > >     now lives in drivers/thunderbolt/pci.c (nhi_pci_driver_shutdown()).
> > >   - Add kernel-doc for tb_port_reset() now that it is exported.
> > >   - Move the host_reset flag into struct tb_nhi instead of struct tb, and
> > >     set it from nhi_probe() instead of tb_domain_add().
> > >   - Spell out "Thunderbolt 3" instead of "TBT3" in the kernel-doc.
> > >   - Reword the debug message to "downstream port reset failed, continuing".
> > > 
> > > v2:
> > >   - Add Reviewed-by tag from Mario Limonciello.
> > >   - Restrict the DPR assertion to Thunderbolt 3 devices, since the issue
> > >     only affects Thunderbolt 3 devices.
> > >   - Only assert DPR on system shutdown/reboot (the PCI ->shutdown callback
> > >     forces host_reset) or when the host router was actually reset on load.
> > >   - Reword comments and commit message to refer to the "router tree"
> > >     instead of the "switch tree".
> > > ---
> > >   drivers/thunderbolt/nhi.c    |  2 ++
> > >   drivers/thunderbolt/pci.c    | 16 +++++++++++++++-
> > >   drivers/thunderbolt/switch.c | 11 ++++++++++-
> > >   drivers/thunderbolt/tb.c     | 21 +++++++++++++++++++++
> > >   drivers/thunderbolt/tb.h     |  1 +
> > >   include/linux/thunderbolt.h  |  6 ++++++
> > >   6 files changed, 55 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/thunderbolt/nhi.c b/drivers/thunderbolt/nhi.c
> > > index 0f795ea58756..698fb124d529 100644
> > > --- a/drivers/thunderbolt/nhi.c
> > > +++ b/drivers/thunderbolt/nhi.c
> > > @@ -1235,6 +1235,8 @@ int nhi_probe(struct tb_nhi *nhi)
> > >   	init_completion(&nhi->domain_released);
> > > +	nhi->host_reset = host_reset;
> > > +
> > >   	res = tb_domain_add(tb, host_reset);
> > >   	if (res) {
> > >   		/*
> > > diff --git a/drivers/thunderbolt/pci.c b/drivers/thunderbolt/pci.c
> > > index bbd186c29ef7..be63a4647b59 100644
> > > --- a/drivers/thunderbolt/pci.c
> > > +++ b/drivers/thunderbolt/pci.c
> > > @@ -493,6 +493,20 @@ static void nhi_pci_remove(struct pci_dev *pdev)
> > >   	nhi_shutdown(nhi);
> > >   }
> > > +static void nhi_pci_driver_shutdown(struct pci_dev *pdev)
> > nhi_pci_shutdown()
> > 
> > > +{
> > > +	struct tb *tb = pci_get_drvdata(pdev);
> > > +	struct tb_nhi *nhi = tb->nhi;
> > > +
> > > +	/*
> > > +	 * Force host_reset so the connection manager asserts DPR and signals
> > > +	 * disconnect to connected devices before the router tree is removed.
> > > +	 * Only Thunderbolt 3 devices are reset.
> > > +	 */
> > > +	nhi->host_reset = true;
> > > +	nhi_pci_remove(pdev);
> > > +}
> > > +
> > >   static struct pci_device_id nhi_ids[] = {
> > >   	/*
> > >   	 * We have to specify class, the TB bridges use the same device and
> > > @@ -593,7 +607,7 @@ static struct pci_driver nhi_driver = {
> > >   	.id_table = nhi_ids,
> > >   	.probe = nhi_pci_probe,
> > >   	.remove = nhi_pci_remove,
> > > -	.shutdown = nhi_pci_remove,
> > > +	.shutdown = nhi_pci_driver_shutdown,
> > nhi_pci_shutdown
> > 
> > Otherwise looks good.
> 
> Thank you Mika for the review.
> 
> There is already a static nhi_pci_shutdown() in pci.c (the
> tb_nhi_ops.shutdown handler at line 233, also called from nhi_shutdown()),
> so reusing that name for the pci_driver ->shutdown callback would collide
> and fail to build.
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/westeri/thunderbolt.git/tree/drivers/thunderbolt/pci.c?h=next#n233
> 
> Would you prefer:
> 
>   1. Keeping a distinct name for the pci_driver hook, e.g.
>      nhi_pci_driver_shutdown() as in this version, or
> 
>   2. Renaming the existing tb_nhi_ops handler (say, to
>      nhi_pci_ops_shutdown()) so the new pci_driver ->shutdown callback
>      can take the nhi_pci_shutdown() name?
> 
> I'm fine with either; please let me know which you prefer and I'll send v4
> accordingly.

Put this code to nhi.c::nhi_shutdown() as that's applicable also to non-PCI
hosts, right?