Linux USB
 help / color / mirror / Atom feed
From: Mika Westerberg <mika.westerberg@linux.intel.com>
To: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
Cc: andreas.noever@gmail.com, westeri@kernel.org,
	YehezkelShB@gmail.com, linux-usb@vger.kernel.org,
	Mario Limonciello <superm1@kernel.org>,
	Sanath S <Sanath.S@amd.com>
Subject: Re: [PATCH v3] thunderbolt: Assert downstream port reset on shutdown
Date: Wed, 10 Jun 2026 12:57:17 +0200	[thread overview]
Message-ID: <20260610105717.GQ2990@black.igk.intel.com> (raw)
In-Reply-To: <20260610085006.3735139-1-Basavaraj.Natikar@amd.com>

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.

>  	.driver.pm = &nhi_pm_ops,
>  };
>  
> diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c
> index a830c82bb905..404c0693df50 100644
> --- a/drivers/thunderbolt/switch.c
> +++ b/drivers/thunderbolt/switch.c
> @@ -682,7 +682,16 @@ int tb_port_disable(struct tb_port *port)
>  	return __tb_port_enable(port, false);
>  }
>  
> -static int tb_port_reset(struct tb_port *port)
> +/**
> + * tb_port_reset() - Reset the port
> + * @port: Port to reset
> + *
> + * Resets @port. For USB4 ports this issues a USB4 port reset and for
> + * legacy ports the link controller port is reset.
> + *
> + * Return: %0 on success, negative errno otherwise.
> + */
> +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 76323255439a..b7cc6894a598 100644
> --- a/drivers/thunderbolt/tb.c
> +++ b/drivers/thunderbolt/tb.c
> @@ -2941,7 +2941,9 @@ static void tb_handle_event(struct tb *tb, enum tb_cfg_pkg_type type,
>  static void tb_stop(struct tb *tb)
>  {
>  	struct tb_cm *tcm = tb_priv(tb);
> +	struct tb_nhi *nhi = tb->nhi;
>  	struct tb_tunnel *tunnel;
> +	struct tb_port *port;
>  	struct tb_tunnel *n;
>  
>  	cancel_delayed_work(&tcm->remove_work);
> @@ -2956,6 +2958,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 (nhi->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, "downstream port reset failed, continuing\n");
> +		}
> +	}
>  	tb_switch_remove(tb->root_switch);
>  	tb->root_switch = NULL;
>  	tcm->hotplug_active = false; /* signal tb_handle_hotplug to quit */
> diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
> index ec9192b61bc0..4373336d9425 100644
> --- a/drivers/thunderbolt/tb.h
> +++ b/drivers/thunderbolt/tb.h
> @@ -1103,6 +1103,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
> index feb1af175cfd..cb1621c6b703 100644
> --- a/include/linux/thunderbolt.h
> +++ b/include/linux/thunderbolt.h
> @@ -514,6 +514,11 @@ void tb_service_properties_changed(struct tb_service *svc);
>   * @hop_count: Number of rings (end point hops) supported by NHI.
>   * @quirks: NHI specific quirks if any
>   * @domain_released: Completed when domain has been fully released
> + * @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 Thunderbolt 3 devices are reset; USB4
> + *		routers are skipped.
>   */
>  struct tb_nhi {
>  	spinlock_t lock;
> @@ -528,6 +533,7 @@ struct tb_nhi {
>  	u32 hop_count;
>  	unsigned long quirks;
>  	struct completion domain_released;
> +	bool host_reset;
>  };
>  
>  /**
> -- 
> 2.34.1

  reply	other threads:[~2026-06-10 10:57 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-10  8:50 [PATCH v3] thunderbolt: Assert downstream port reset on shutdown Basavaraj Natikar
2026-06-10 10:57 ` Mika Westerberg [this message]
2026-06-10 11:32   ` Basavaraj Natikar
2026-06-10 11:58     ` Mika Westerberg
2026-06-10 13:32       ` Basavaraj Natikar
2026-06-10 14:23         ` Mika Westerberg

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260610105717.GQ2990@black.igk.intel.com \
    --to=mika.westerberg@linux.intel.com \
    --cc=Basavaraj.Natikar@amd.com \
    --cc=Sanath.S@amd.com \
    --cc=YehezkelShB@gmail.com \
    --cc=andreas.noever@gmail.com \
    --cc=linux-usb@vger.kernel.org \
    --cc=superm1@kernel.org \
    --cc=westeri@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox