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

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

Hi,

On Tue, Jun 09, 2026 at 04:12:18PM +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 tb->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 TBT3 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>
> ---
> v2:
>  - Add Reviewed-by tag from Mario Limonciello.
>  - Restrict the DPR assertion to TBT3 devices, since the issue only
>    affects TBT3 devices.
>  - Only assert DPR on system shutdown/reboot (the PCI ->shutdown callback
>    forces tb->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/domain.c |  2 ++
>  drivers/thunderbolt/nhi.c    | 16 +++++++++++++++-
>  drivers/thunderbolt/switch.c |  2 +-
>  drivers/thunderbolt/tb.c     | 20 ++++++++++++++++++++
>  drivers/thunderbolt/tb.h     |  1 +
>  include/linux/thunderbolt.h  |  6 ++++++
>  6 files changed, 45 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/thunderbolt/domain.c b/drivers/thunderbolt/domain.c
> --- a/drivers/thunderbolt/domain.c
> +++ b/drivers/thunderbolt/domain.c
> @@ -460,6 +460,8 @@ int tb_domain_add(struct tb *tb, bool reset)
>  	if (ret)
>  		goto err_ctl_stop;
>  
> +	tb->host_reset = reset;
> +
>  	/* Start the domain */
>  	if (tb->cm_ops->start) {
>  		ret = tb->cm_ops->start(tb, reset);
> diff --git a/drivers/thunderbolt/nhi.c b/drivers/thunderbolt/nhi.c
> --- a/drivers/thunderbolt/nhi.c
> +++ b/drivers/thunderbolt/nhi.c
> @@ -1436,6 +1436,20 @@ static void nhi_remove(struct pci_dev *pdev)
>  	nhi_shutdown(nhi);
>  }
>  
> +static void nhi_pci_shutdown(struct pci_dev *pdev)
> +{
> +	struct tb *tb = pci_get_drvdata(pdev);
> +
> +	/*
> +	 * On system shutdown/reboot force host_reset so the connection
> +	 * manager asserts DPR to signal disconnect before removing the
> +	 * router tree. Only TBT3 devices are reset. Plain driver unload
> +	 * uses ->remove (nhi_remove) and keeps the host_reset from load.
> +	 */
> +	tb->host_reset = true;
> +	nhi_remove(pdev);
> +}

There has been changes to support non-PCIe hosts so this does not apply on
top of my next branch.

> +
>  /*
>   * The tunneled pci bridges are siblings of us. Use resume_noirq to reenable
>   * the tunnels asap. A corresponding pci quirk blocks the downstream bridges
> @@ -1558,7 +1572,7 @@ static struct pci_driver nhi_driver = {
>  	.id_table = nhi_ids,
>  	.probe = nhi_probe,
>  	.remove = nhi_remove,
> -	.shutdown = nhi_remove,
> +	.shutdown = nhi_pci_shutdown,
>  	.driver.pm = &nhi_pm_ops,
>  };
>  
> diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c
> --- 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)

Since you make it non-static please add kernel-doc as well.

> +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
> --- 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,25 @@ static void tb_stop(struct tb *tb)
>  			tb_tunnel_deactivate(tunnel);
>  		tb_tunnel_put(tunnel);
>  	}
> +	/*
> +	 * Signal disconnect to connected devices before the router tree is
> +	 * removed below. A Thunderbolt 3 device directly connected to a USB4
> +	 * host otherwise never receives a disconnect indication, leaving
> +	 * firmware to poll the dead link for up to ~60 s which on some
> +	 * platforms turns the shutdown into a warm reset. Asserting
> +	 * PORT_CS_19.DPR drives SBTX low (USB4 spec section 6.9) so the device
> +	 * detects SBRX low and goes to Uninitialized Unplugged immediately.
> +	 */
> +	if (tb->host_reset) {
> +		tb_switch_for_each_port(tb->root_switch, port) {
> +			if (!tb_port_is_null(port) || !tb_port_has_remote(port))
> +				continue;
> +			if (tb_switch_is_usb4(port->remote->sw))
> +				continue;
> +			if (tb_port_reset(port))
> +				tb_port_dbg(port, "DPR failed, continuing\n");

"downstream port reset failed, continuing\n"

> +		}
> +	}
>  	tb_switch_remove(tb->root_switch);
>  	tcm->hotplug_active = false; /* signal tb_handle_hotplug to quit */
>  }
> diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
> --- a/drivers/thunderbolt/tb.h
> +++ b/drivers/thunderbolt/tb.h
> @@ -1102,6 +1102,7 @@ int tb_port_clear_counter(struct tb_port *port, int counter);
>  int tb_port_unlock(struct tb_port *port);
>  int tb_port_enable(struct tb_port *port);
>  int tb_port_disable(struct tb_port *port);
> +int tb_port_reset(struct tb_port *port);
>  int tb_port_alloc_in_hopid(struct tb_port *port, int hopid, int max_hopid);
>  void tb_port_release_in_hopid(struct tb_port *port, int hopid);
>  int tb_port_alloc_out_hopid(struct tb_port *port, int hopid, int max_hopid);
> diff --git a/include/linux/thunderbolt.h b/include/linux/thunderbolt.h
> --- a/include/linux/thunderbolt.h
> +++ b/include/linux/thunderbolt.h
> @@ -76,5 +76,10 @@
>   * @index: Linux assigned domain number
>   * @security_level: Current security level
>   * @nboot_acl: Number of boot ACLs the domain supports
> + * @host_reset: Host router was reset on driver load, or forced on system
> + *		shutdown/reboot. When set, tb_stop() asserts DPR on connected
> + *		downstream ports to signal disconnect before tearing down the
> + *		router tree. Only TBT3 devices are reset; USB4 routers are

Thunderbolt 3

And I think this should be part of struct tb_nhi instead.

> + *		skipped.
>   * @privdata: Private connection manager specific data
>   */
> @@ -89,5 +94,6 @@ struct tb {
>  	int index;
>  	enum tb_security_level security_level;
>  	size_t nboot_acl;
> +	bool host_reset;
>  	unsigned long privdata[];
>  };
> -- 
> 2.34.1