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

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

On Mon, Jun 08, 2026 at 10:13:00AM -0500, Mario Limonciello wrote:
> 
> 
> On 6/8/26 01:40, Mika Westerberg wrote:
> > On Fri, Jun 05, 2026 at 11:27:52AM -0500, Basavaraj Natikar wrote:
> > > Hi,
> > > 
> > > 
> > > On 6/4/2026 12:03 AM, Mika Westerberg wrote:
> > > > Hi,
> > > > 
> > > > On Wed, Jun 03, 2026 at 11:31:46PM +0530, Basavaraj Natikar wrote:
> > > > > On shutdown the connection manager tears down the switch tree without
> > > > router tree
> > > > 
> > > > > signalling connected devices. Thunderbolt 3 devices directly connected
> > > > > to a USB4 host never receive 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 switch tree. This drives SBTX low unconditionally
> > > > router tree
> > > > 
> > > > > (USB4 spec section 6.9), causing the device to detect SBRX low and
> > > > > transition to Uninitialized Unplugged state immediately.
> > > > > 
> > > > > 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>
> > > > > ---
> > > > >    drivers/thunderbolt/switch.c |  2 +-
> > > > >    drivers/thunderbolt/tb.c     | 11 +++++++++++
> > > > >    drivers/thunderbolt/tb.h     |  1 +
> > > > >    3 files changed, 13 insertions(+), 1 deletion(-)
> > > > > 
> > > > > diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c
> > > > > index c2ad58b19e7b..52812908818b 100644
> > > > > --- a/drivers/thunderbolt/switch.c
> > > > > +++ b/drivers/thunderbolt/switch.c
> > > > > @@ -704,7 +704,7 @@ int tb_port_disable(struct tb_port *port)
> > > > >    	return __tb_port_enable(port, false);
> > > > >    }
> > > > > -static int tb_port_reset(struct tb_port *port)
> > > > > +int tb_port_reset(struct tb_port *port)
> > > > >    {
> > > > >    	if (tb_switch_is_usb4(port->sw))
> > > > >    		return port->cap_usb4 ? usb4_port_reset(port) : 0;
> > > > > diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c
> > > > > index c69c323e6952..ca57b5181422 100644
> > > > > --- a/drivers/thunderbolt/tb.c
> > > > > +++ b/drivers/thunderbolt/tb.c
> > > > > @@ -2935,6 +2935,7 @@ static void tb_stop(struct tb *tb)
> > > > >    	struct tb_cm *tcm = tb_priv(tb);
> > > > >    	struct tb_tunnel *tunnel;
> > > > >    	struct tb_tunnel *n;
> > > > > +	struct tb_port *port;
> > > > >    	cancel_delayed_work(&tcm->remove_work);
> > > > >    	/* tunnels are only present after everything has been initialized */
> > > > > @@ -2948,6 +2949,16 @@ static void tb_stop(struct tb *tb)
> > > > >    			tb_tunnel_deactivate(tunnel);
> > > > >    		tb_tunnel_put(tunnel);
> > > > >    	}
> > > > > +	/*
> > > > > +	 * Assert DPR to drive SBTX low, signalling disconnect and avoiding
> > > > > +	 * ~60 s of link polling before warm reset on shutdown.
> > > > > +	 */
> > > > > +	tb_switch_for_each_port(tb->root_switch, port) {
> > > > > +		if (!tb_port_is_null(port) || !tb_port_has_remote(port))
> > > > > +			continue;
> > > > > +		if (tb_port_reset(port))
> > > > > +			tb_port_dbg(port, "DPR on shutdown failed, continuing\n");
> > > > > +	}
> > > > But now this tears down the topology also when the driver is unloaded? If
> > > > you want to do that in shutdown there is ->shutdown hook for that.
> > > 
> > > Asserting DPR on unload is intentional and it is harmless. After a reload
> > > the driver re-probes and the router tree comes back up cleanly.
> > 
> > Well not entirely harmless as it also kills the native tunnels and now user
> > cannot get rid of that by passing host_reset=0 during load.
> > 
> > Prior this unloading the driver left the tunnels up and that was
> > intentional.
> 
> I guess I see two approaches.
> 
> 1) Move to shutdown only (your suggestion).
> 
> Corner case of user who unloads before shutdown means they could be exposed
> to this.
> 
> 2) Link the DPR behavior to host_reset module behavior.
> 
> IE If a user set host_reset=0 avoid host reset on module load and avoid DPR
> on module unload/shutdown.
> 
> If user kept host_reset=1 then do DPR on unload.
> 
> Always do on shutdown (regardless of host_reset value).
> 
> What do you think of #2?

That sounds better. Also we could restrict this more for links that are TB3
e.g no need to do for USB4, assuming that is simple to do.