Re: [PATCH 5/5] thunderbolt: Cancel the DPRX read when the domain is stopped

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

Hi,

On Mon, Aug 17, 2026 at 09:54:02PM +0200, Sven Peter wrote:
> tb_stop only tears down DMA tunnels so a DP tunnel that is still
> waiting for dprx_work to complete keeps that work queued while the
> routers are removed and the control channel is stopped. The work only
> stops once the DPRX timeout has passed and because it requeues itself
> until then the flush_workqueue in tb_domain_remove won't wait for its
> final run. The callback then runs against a domain that is already torn
> down. A reference to that domain is kept so the completion waiting for
> that domain to disappear in unbind will block until the timeout is
> eventually reached.
> 
> Just cancel the work in tb_stop. This doesn't affect DP tunnels that are
> already alive and keeps those displays working.
> 
> Fixes: d6d458d42e1e ("thunderbolt: Handle DisplayPort tunnel activation asynchronously")
> Cc: stable@vger.kernel.org
> Signed-off-by: Sven Peter <sven@kernel.org>
> ---
> I also didn't run into this but noticed it when fixing the hop alloc thing
> and think it makes sense to fix it anyway.
> ---
>  drivers/thunderbolt/tb.c     | 5 ++++-
>  drivers/thunderbolt/tunnel.c | 9 +++++++++
>  drivers/thunderbolt/tunnel.h | 1 +
>  3 files changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c
> index e368a6b53f64..f7e68372da09 100644
> --- a/drivers/thunderbolt/tb.c
> +++ b/drivers/thunderbolt/tb.c
> @@ -2958,10 +2958,13 @@ static void tb_stop(struct tb *tb)
>  		/*
>  		 * DMA tunnels require the driver to be functional so we
>  		 * tear them down. Other protocol tunnels can be left
> -		 * intact.
> +		 * intact but a DPRX capabilities read that is still in
> +		 * flight has to be canceled before the routers go away.
>  		 */
>  		if (tb_tunnel_is_dma(tunnel))
>  			tb_tunnel_deactivate(tunnel);
> +		else if (tb_tunnel_is_dp(tunnel))
> +			tb_tunnel_cancel_dprx(tunnel);

I prefer not to expose "non-generic" functions for the CM if possible. I
wonder if this would work:

		else if (tb_tunnel_is_dp(tunnel) && !tb_tunnel_is_active(tunnel))
			tb_tunnel_deactivate(tunnel);


>  		tb_tunnel_put(tunnel);
>  	}
>  	tb_switch_remove(tb->root_switch);
> diff --git a/drivers/thunderbolt/tunnel.c b/drivers/thunderbolt/tunnel.c
> index 52fa90786ff8..5b1ae5a0c12b 100644
> --- a/drivers/thunderbolt/tunnel.c
> +++ b/drivers/thunderbolt/tunnel.c
> @@ -2487,6 +2487,15 @@ void tb_tunnel_deactivate(struct tb_tunnel *tunnel)
>  	tb_tunnel_set_active(tunnel, false);
>  }
>  
> +/**
> + * tb_tunnel_cancel_dprx() - Cancel the DPRX capabilities read work
> + * @tunnel: tunnel to cancel the DPRX capabilities read work for
> + */
> +void tb_tunnel_cancel_dprx(struct tb_tunnel *tunnel)
> +{
> +	tb_dp_dprx_stop(tunnel);
> +}
> +
>  /**
>   * tb_tunnel_port_on_path() - Does the tunnel go through port
>   * @tunnel: Tunnel to check
> diff --git a/drivers/thunderbolt/tunnel.h b/drivers/thunderbolt/tunnel.h
> index 4878763a82b3..9de5fac04269 100644
> --- a/drivers/thunderbolt/tunnel.h
> +++ b/drivers/thunderbolt/tunnel.h
> @@ -138,6 +138,7 @@ struct tb_tunnel *tb_tunnel_alloc_usb3(struct tb *tb, struct tb_port *up,
>  void tb_tunnel_put(struct tb_tunnel *tunnel);
>  int tb_tunnel_activate(struct tb_tunnel *tunnel);
>  void tb_tunnel_deactivate(struct tb_tunnel *tunnel);
> +void tb_tunnel_cancel_dprx(struct tb_tunnel *tunnel);
>  
>  /**
>   * tb_tunnel_is_active() - Is tunnel fully activated
> 
> -- 
> 2.55.0
>