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

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

[Basavaraj Natikar]

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)
+{
+	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,
 	.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

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

[Mika Westerberg]

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

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

[Basavaraj Natikar]

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.

Thanks,
--
Basavaraj

>
>>   	.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

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

[Mika Westerberg]

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?

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

[Basavaraj Natikar]

Hi,


On 6/10/2026 5:28 PM, Mika Westerberg wrote:
> 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?

I tried that, but nhi_shutdown() runs too late in the teardown to be
useful here. On the shutdown/remove path:
   nhi_pci_remove()
     -> tb_domain_remove()
          -> cm_ops->stop() == tb_stop()   /* reads host_reset, asserts DPR */
     -> nhi_shutdown()                      /* ring/HW teardown, runs last */
tb_stop() is what reads host_reset and asserts the DPR, and it runs
inside tb_domain_remove() -- before nhi_shutdown(). So host_reset has to
be set *before* tb_domain_remove(), and nhi_shutdown() would set it after
tb_stop() has already run.

To still keep it generic for non-PCI hosts, how about a small generic
helper in nhi.c that just forces the flag, called from the bus ->shutdown
callback before the normal removal:
   /* nhi.c */
   void nhi_host_shutdown(struct tb_nhi *nhi)
   {
     /* Force DPR on connected Thunderbolt 3 devices, see tb_stop(). */
     nhi->host_reset = true;
   }
   /* pci.c */
   static void nhi_pci_host_shutdown(struct pci_dev *pdev)
   {
     struct tb *tb = pci_get_drvdata(pdev);
     nhi_host_shutdown(tb->nhi);
     nhi_pci_remove(pdev);
   }
   ...
     .shutdown = nhi_pci_host_shutdown,

This leaves nhi_pci_remove() (and nhi_shutdown()) unchanged, and the
host_reset logic lives in nhi.c so a future non-PCI host can reuse
nhi_host_shutdown() the same way. Does that work for you, or would you

prefer it shaped differently?

Thanks,
--
Basavaraj

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

[Mika Westerberg]

Hi,

On Wed, Jun 10, 2026 at 07:02:02PM +0530, Basavaraj Natikar wrote:
> Hi,
> 
> 
> On 6/10/2026 5:28 PM, Mika Westerberg wrote:
> > 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?
> 
> I tried that, but nhi_shutdown() runs too late in the teardown to be
> useful here. On the shutdown/remove path:
>   nhi_pci_remove()
>     -> tb_domain_remove()
>          -> cm_ops->stop() == tb_stop()   /* reads host_reset, asserts DPR */
>     -> nhi_shutdown()                      /* ring/HW teardown, runs last */
> tb_stop() is what reads host_reset and asserts the DPR, and it runs
> inside tb_domain_remove() -- before nhi_shutdown(). So host_reset has to
> be set *before* tb_domain_remove(), and nhi_shutdown() would set it after
> tb_stop() has already run.

Ah okay.

> To still keep it generic for non-PCI hosts, how about a small generic
> helper in nhi.c that just forces the flag, called from the bus ->shutdown
> callback before the normal removal:
>   /* nhi.c */
>   void nhi_host_shutdown(struct tb_nhi *nhi)
>   {
>     /* Force DPR on connected Thunderbolt 3 devices, see tb_stop(). */
>     nhi->host_reset = true;
>   }
>   /* pci.c */
>   static void nhi_pci_host_shutdown(struct pci_dev *pdev)
>   {
>     struct tb *tb = pci_get_drvdata(pdev);
>     nhi_host_shutdown(tb->nhi);
>     nhi_pci_remove(pdev);
>   }
>   ...
>     .shutdown = nhi_pci_host_shutdown,
> 
> This leaves nhi_pci_remove() (and nhi_shutdown()) unchanged, and the
> host_reset logic lives in nhi.c so a future non-PCI host can reuse
> nhi_host_shutdown() the same way. Does that work for you, or would you
> 
> prefer it shaped differently?

I really would like to name the PCI driver shutdown hook as
nhi_pci_shutdown. What about this:

1. Rename nhi_pci_shutdown -> nhi_pci_release_irq as that's what it does
   (keep the callback member name shutdown still)

2. Rename nhi_pci_remove -> nhi_pci_do_remove that takes new parameter
   'reset' or so and passes this down to the tb_domain_remove() and also
   ->tb_stop(). 

3. Add nhi_pci_shutdown() that calls nhi_pci_do_remove(pdev, true);
4. Add nhi_pci_remove() that calls nhi_pci_do_remove(pdev, false);

Or something along those lines. If it looks horrible then we can do the
same but with the flag instead of passing the parameter down.