[PATCH v2 0/7] thunderbolt: Fix DP tunnel teardown while an async DPRX read is running

[PATCH v2 0/7] thunderbolt: Fix DP tunnel teardown while an async DPRX read is running

[Sven Peter]

Hi,

This series contains a bunch of fixes related to teardown while an async
DPRX read is running. I ran into some of these while bringing up the
Apple Silicon NHI and just haven't implemented DP tunneling support yet
such that the DPRX capabilities read can never complete and will always
time out.

None of it is specific to that hardware though so I figured it
makes sense to already send them out.

Best,

Sven

---
Changes in v2:
- Make the DP tunnel activation callback mandatory. The DPRX
  caps are now always read inside dprx_work which now also happens to
  discovered tunnels when they are activated again after hibernation
  restore.
- Mark discovered tunnels as active
- Add a cleanup commit to drop the callback data that's always the same now
- Tear down DP tunnels that are inactive in tb_stop instead of
  exposing tb_tunnel_cancel_dprx.
- Use "router" instead of "switch" in the subject
- Added comments how these issues can be reproduced
- Link to v1: https://patch.msgid.link/20260817-b4-tbt-fixes-v1-0-eded2461f5fc@kernel.org

To: Andreas Noever <andreas.noever@gmail.com>
To: Mika Westerberg <westeri@kernel.org>
To: Yehezkel Bernat <YehezkelShB@gmail.com>
Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
Cc: Konrad Dybcio <konradybcio@kernel.org>
Cc: asahi@lists.linux.dev
Cc: linux-usb@vger.kernel.org
Cc: linux-kernel@vger.kernel.org

---
Sven Peter (7):
      thunderbolt: Hold a router reference for each path hop
      thunderbolt: Make the DP tunnel activation callback mandatory
      thunderbolt: Fix domain reference leak when DPRX read is canceled
      thunderbolt: Don't access a DP tunnel after its DPRX read was canceled
      thunderbolt: Mark discovered tunnels as active
      thunderbolt: Tear down inactive DP tunnels when the domain is stopped
      thunderbolt: Drop the DP tunnel activation callback data

 drivers/thunderbolt/path.c   | 21 +++++++++++++
 drivers/thunderbolt/tb.c     | 34 ++++++++++++++-------
 drivers/thunderbolt/test.c   | 49 ++++++++++++++++++++++-------
 drivers/thunderbolt/tunnel.c | 73 ++++++++++++++++++++++++++------------------
 drivers/thunderbolt/tunnel.h | 12 +++-----
 5 files changed, 131 insertions(+), 58 deletions(-)
---
base-commit: 8d3ae59288f1e7d58d76558a6ee96d533bc5019f
change-id: 20260815-b4-tbt-fixes-a2689186016d

Best regards,
--  
Sven Peter <sven@kernel.org>

[PATCH v2 1/7] thunderbolt: Hold a router reference for each path hop

[Sven Peter]

tb_stop drops the reference to all DP tunnels but does not deactivate
them, thus nothing cancels a dprx_work still in flight (which holds its
own tunnel reference) and the tunnel can outlive tb_switch_remove. The
HopID releases in tb_path_free then operate on freed IDAs and trigger
warnings like

  ida_free called for id=8 which is not allocated.

This can be triggered by unbinding the driver while a DP tunnel is still
waiting for the DPRX capabilities read to finish. On the Apple NHI
unplugging the cable runs into just that reliably because the read can
never finish right now and because the unplug powers down the
entire USB4 complex and removes the NHI device.

Take or release a reference for both ports of each hop whenever the
HopIDs are allocated or released to ensure they have the same lifetime.

The KUnit tests allocate their switches without ever registering them so
initialize the embedded struct device there as well to make these
references work.

Fixes: d6d458d42e1e ("thunderbolt: Handle DisplayPort tunnel activation asynchronously")
Cc: stable@vger.kernel.org
Signed-off-by: Sven Peter <sven@kernel.org>
---
 drivers/thunderbolt/path.c | 21 +++++++++++++++++++++
 drivers/thunderbolt/test.c | 12 ++++++++++++
 2 files changed, 33 insertions(+)

diff --git a/drivers/thunderbolt/path.c b/drivers/thunderbolt/path.c
index b2c322e76b8a..02c5e7a2101e 100644
--- a/drivers/thunderbolt/path.c
+++ b/drivers/thunderbolt/path.c
@@ -196,6 +196,12 @@ struct tb_path *tb_path_discover(struct tb_port *src, int src_hopid,
 		path->hops[i].out_port = out_port;
 		path->hops[i].next_hop_index = next_hop;
 
+		/* Keep the ports alive, see tb_path_free() */
+		if (alloc_hopid) {
+			tb_switch_get(path->hops[i].in_port->sw);
+			tb_switch_get(path->hops[i].out_port->sw);
+		}
+
 		tb_dump_hop(&path->hops[i], &hop);
 
 		h = next_hop;
@@ -323,6 +329,10 @@ struct tb_path *tb_path_alloc(struct tb *tb, struct tb_port *src, int src_hopid,
 		path->hops[i].out_port = out_port;
 		path->hops[i].next_hop_index = out_hopid;
 
+		/* Keep the ports alive, see tb_path_free() */
+		tb_switch_get(path->hops[i].in_port->sw);
+		tb_switch_get(path->hops[i].out_port->sw);
+
 		in_hopid = out_hopid;
 	}
 
@@ -356,6 +366,17 @@ void tb_path_free(struct tb_path *path)
 			if (hop->out_port)
 				tb_port_release_out_hopid(hop->out_port,
 							  hop->next_hop_index);
+			/*
+			 * Only drop the switch references after both HopIDs
+			 * have been released: the path may be freed after the
+			 * switch was already removed (e.g. asynchronous DP
+			 * tunnel teardown) and these references are what
+			 * keeps the ports and their HopID IDAs alive.
+			 */
+			if (hop->in_port)
+				tb_switch_put(hop->in_port->sw);
+			if (hop->out_port)
+				tb_switch_put(hop->out_port->sw);
 		}
 	}
 
diff --git a/drivers/thunderbolt/test.c b/drivers/thunderbolt/test.c
index 05652ee82fbf..034c56845380 100644
--- a/drivers/thunderbolt/test.c
+++ b/drivers/thunderbolt/test.c
@@ -33,6 +33,11 @@ static void kunit_ida_init(struct kunit *test, struct ida *ida)
 	kunit_alloc_resource(test, __ida_init, __ida_destroy, GFP_KERNEL, ida);
 }
 
+static void tb_test_switch_release(struct device *dev)
+{
+	/* The memory is owned by KUnit, nothing to do here */
+}
+
 static struct tb_switch *alloc_switch(struct kunit *test, u64 route,
 				      u8 upstream_port, u8 max_port_number)
 {
@@ -44,6 +49,13 @@ static struct tb_switch *alloc_switch(struct kunit *test, u64 route,
 	if (!sw)
 		return NULL;
 
+	/*
+	 * The paths take a reference to their switches and those devices
+	 * have to be initialized for that to work.
+	 */
+	sw->dev.release = tb_test_switch_release;
+	device_initialize(&sw->dev);
+
 	sw->config.upstream_port_number = upstream_port;
 	sw->config.depth = tb_route_length(route);
 	sw->config.route_hi = upper_32_bits(route);

-- 
2.55.0

Re: [PATCH v2 1/7] thunderbolt: Hold a router reference for each path hop

[Mika Westerberg]

Hi,

On Sun, Aug 23, 2026 at 06:09:14PM +0200, Sven Peter wrote:
> tb_stop drops the reference to all DP tunnels but does not deactivate
> them, thus nothing cancels a dprx_work still in flight (which holds its
> own tunnel reference) and the tunnel can outlive tb_switch_remove. The
> HopID releases in tb_path_free then operate on freed IDAs and trigger
> warnings like
> 
>   ida_free called for id=8 which is not allocated.
> 
> This can be triggered by unbinding the driver while a DP tunnel is still
> waiting for the DPRX capabilities read to finish. On the Apple NHI
> unplugging the cable runs into just that reliably because the read can
> never finish right now and because the unplug powers down the
> entire USB4 complex and removes the NHI device.

Thanks for adding this.

Is this behaviour due to something missing still on PM side or this is how
it is designed to work on Apple silicon? This resembles the early PC way
where ACPI dealt with all the hotplug PCIe stuff and the host router was
only present when a cable was connected. I would kind of expect that Apple
did this using "RTD3" way so keeping the host router present and the OS
then deals with putting it into D3 and back.

> Take or release a reference for both ports of each hop whenever the
> HopIDs are allocated or released to ensure they have the same lifetime.
> 
> The KUnit tests allocate their switches without ever registering them so
> initialize the embedded struct device there as well to make these
> references work.
> 
> Fixes: d6d458d42e1e ("thunderbolt: Handle DisplayPort tunnel activation asynchronously")
> Cc: stable@vger.kernel.org
> Signed-off-by: Sven Peter <sven@kernel.org>
> ---
>  drivers/thunderbolt/path.c | 21 +++++++++++++++++++++
>  drivers/thunderbolt/test.c | 12 ++++++++++++
>  2 files changed, 33 insertions(+)
> 
> diff --git a/drivers/thunderbolt/path.c b/drivers/thunderbolt/path.c
> index b2c322e76b8a..02c5e7a2101e 100644
> --- a/drivers/thunderbolt/path.c
> +++ b/drivers/thunderbolt/path.c
> @@ -196,6 +196,12 @@ struct tb_path *tb_path_discover(struct tb_port *src, int src_hopid,
>  		path->hops[i].out_port = out_port;
>  		path->hops[i].next_hop_index = next_hop;
>  
> +		/* Keep the ports alive, see tb_path_free() */
> +		if (alloc_hopid) {
> +			tb_switch_get(path->hops[i].in_port->sw);
> +			tb_switch_get(path->hops[i].out_port->sw);
> +		}

I wonder if we can put this in tb_port_alloc_in/out_hopid() instead? That
would be more "natural" IMHO.

> +
>  		tb_dump_hop(&path->hops[i], &hop);
>  
>  		h = next_hop;
> @@ -323,6 +329,10 @@ struct tb_path *tb_path_alloc(struct tb *tb, struct tb_port *src, int src_hopid,
>  		path->hops[i].out_port = out_port;
>  		path->hops[i].next_hop_index = out_hopid;
>  
> +		/* Keep the ports alive, see tb_path_free() */
> +		tb_switch_get(path->hops[i].in_port->sw);
> +		tb_switch_get(path->hops[i].out_port->sw);
> +
>  		in_hopid = out_hopid;
>  	}
>  
> @@ -356,6 +366,17 @@ void tb_path_free(struct tb_path *path)
>  			if (hop->out_port)
>  				tb_port_release_out_hopid(hop->out_port,
>  							  hop->next_hop_index);
> +			/*
> +			 * Only drop the switch references after both HopIDs

Let's use "router" universally.

> +			 * have been released: the path may be freed after the
> +			 * switch was already removed (e.g. asynchronous DP
> +			 * tunnel teardown) and these references are what
> +			 * keeps the ports and their HopID IDAs alive.
> +			 */
> +			if (hop->in_port)
> +				tb_switch_put(hop->in_port->sw);
> +			if (hop->out_port)
> +				tb_switch_put(hop->out_port->sw);
>  		}
>  	}
>  
> diff --git a/drivers/thunderbolt/test.c b/drivers/thunderbolt/test.c
> index 05652ee82fbf..034c56845380 100644
> --- a/drivers/thunderbolt/test.c
> +++ b/drivers/thunderbolt/test.c
> @@ -33,6 +33,11 @@ static void kunit_ida_init(struct kunit *test, struct ida *ida)
>  	kunit_alloc_resource(test, __ida_init, __ida_destroy, GFP_KERNEL, ida);
>  }
>  
> +static void tb_test_switch_release(struct device *dev)
> +{
> +	/* The memory is owned by KUnit, nothing to do here */
> +}
> +
>  static struct tb_switch *alloc_switch(struct kunit *test, u64 route,
>  				      u8 upstream_port, u8 max_port_number)
>  {
> @@ -44,6 +49,13 @@ static struct tb_switch *alloc_switch(struct kunit *test, u64 route,
>  	if (!sw)
>  		return NULL;
>  
> +	/*
> +	 * The paths take a reference to their switches and those devices
> +	 * have to be initialized for that to work.
> +	 */
> +	sw->dev.release = tb_test_switch_release;
> +	device_initialize(&sw->dev);

The idea was that we don't use this as real device but if we go this route
then I think we should call put_device() to release it and check for any
subtleties device_initialize() possibly does.

> +
>  	sw->config.upstream_port_number = upstream_port;
>  	sw->config.depth = tb_route_length(route);
>  	sw->config.route_hi = upper_32_bits(route);
> 
> -- 
> 2.55.0
> 

Re: [PATCH v2 1/7] thunderbolt: Hold a router reference for each path hop

[Sven Peter]

Hi,


On 8/24/26 12:42, Mika Westerberg wrote:
> Hi,
>
> On Sun, Aug 23, 2026 at 06:09:14PM +0200, Sven Peter wrote:
>> tb_stop drops the reference to all DP tunnels but does not deactivate
>> them, thus nothing cancels a dprx_work still in flight (which holds its
>> own tunnel reference) and the tunnel can outlive tb_switch_remove. The
>> HopID releases in tb_path_free then operate on freed IDAs and trigger
>> warnings like
>>
>>    ida_free called for id=8 which is not allocated.
>>
>> This can be triggered by unbinding the driver while a DP tunnel is still
>> waiting for the DPRX capabilities read to finish. On the Apple NHI
>> unplugging the cable runs into just that reliably because the read can
>> never finish right now and because the unplug powers down the
>> entire USB4 complex and removes the NHI device.
> Thanks for adding this.
>
> Is this behaviour due to something missing still on PM side or this is how
> it is designed to work on Apple silicon? This resembles the early PC way
> where ACPI dealt with all the hotplug PCIe stuff and the host router was
> only present when a cable was connected. I would kind of expect that Apple
> did this using "RTD3" way so keeping the host router present and the OS
> then deals with putting it into D3 and back.

So unfortunately the entire USB hardware is best described as horribly 
broken :(
For USB2 already we only ever receive a single hotplug interrupt in 
dwc3. If the first device is unplugged and another one plugged in 
nothing happens. The only way around that is to tear down dwc3/xhci and 
the PHY completely, assert all external reset lines and then bring them 
up again.
Upstream drivers/usb/dwc3/dwc3-apple.c contains more details in the very 
first comment at the top.

With USB4 this gets a bit worse: There's a block called "ACIO" (Apple 
Converged I/O) which can only be brought up correctly after the PHY has 
been switched to Thunderbolt/USB4 mode. This block has a co-processor 
which then exposes the host router, NHI, IOMMU, etc. to our address space.
We then have to write cable information into a vendor-specific 
capabilities register and only then do we get the hotplug event and the 
link comes up. This register appears to be effectively write-once after 
each boot of the co-processor. If I try to write 0 that still works but 
trying to write the value for the next connection then crashes the 
co-processor with a very helpful message along the lines of "assert 4357 
violated".
So there's no way around first tearing everything down, then shutting 
down the co-processor and the entire ACIO block and then finally 
bringing it all up again after the next cable is connected. I've tried 
to find ways around this but didn't succeed without documentation and 
could only reproduce what XNU does.


>
>> Take or release a reference for both ports of each hop whenever the
>> HopIDs are allocated or released to ensure they have the same lifetime.
>>
>> The KUnit tests allocate their switches without ever registering them so
>> initialize the embedded struct device there as well to make these
>> references work.
>>
>> Fixes: d6d458d42e1e ("thunderbolt: Handle DisplayPort tunnel activation asynchronously")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Sven Peter <sven@kernel.org>
>> ---
>>   drivers/thunderbolt/path.c | 21 +++++++++++++++++++++
>>   drivers/thunderbolt/test.c | 12 ++++++++++++
>>   2 files changed, 33 insertions(+)
>>
>> diff --git a/drivers/thunderbolt/path.c b/drivers/thunderbolt/path.c
>> index b2c322e76b8a..02c5e7a2101e 100644
>> --- a/drivers/thunderbolt/path.c
>> +++ b/drivers/thunderbolt/path.c
>> @@ -196,6 +196,12 @@ struct tb_path *tb_path_discover(struct tb_port *src, int src_hopid,
>>   		path->hops[i].out_port = out_port;
>>   		path->hops[i].next_hop_index = next_hop;
>>   
>> +		/* Keep the ports alive, see tb_path_free() */
>> +		if (alloc_hopid) {
>> +			tb_switch_get(path->hops[i].in_port->sw);
>> +			tb_switch_get(path->hops[i].out_port->sw);
>> +		}
> I wonder if we can put this in tb_port_alloc_in/out_hopid() instead? That
> would be more "natural" IMHO.

Sure, I'll give it a try.
>
>> +
>>   		tb_dump_hop(&path->hops[i], &hop);
>>   
>>   		h = next_hop;
>> @@ -323,6 +329,10 @@ struct tb_path *tb_path_alloc(struct tb *tb, struct tb_port *src, int src_hopid,
>>   		path->hops[i].out_port = out_port;
>>   		path->hops[i].next_hop_index = out_hopid;
>>   
>> +		/* Keep the ports alive, see tb_path_free() */
>> +		tb_switch_get(path->hops[i].in_port->sw);
>> +		tb_switch_get(path->hops[i].out_port->sw);
>> +
>>   		in_hopid = out_hopid;
>>   	}
>>   
>> @@ -356,6 +366,17 @@ void tb_path_free(struct tb_path *path)
>>   			if (hop->out_port)
>>   				tb_port_release_out_hopid(hop->out_port,
>>   							  hop->next_hop_index);
>> +			/*
>> +			 * Only drop the switch references after both HopIDs
> Let's use "router" universally.

Sure!

>
>> +			 * have been released: the path may be freed after the
>> +			 * switch was already removed (e.g. asynchronous DP
>> +			 * tunnel teardown) and these references are what
>> +			 * keeps the ports and their HopID IDAs alive.
>> +			 */
>> +			if (hop->in_port)
>> +				tb_switch_put(hop->in_port->sw);
>> +			if (hop->out_port)
>> +				tb_switch_put(hop->out_port->sw);
>>   		}
>>   	}
>>   
>> diff --git a/drivers/thunderbolt/test.c b/drivers/thunderbolt/test.c
>> index 05652ee82fbf..034c56845380 100644
>> --- a/drivers/thunderbolt/test.c
>> +++ b/drivers/thunderbolt/test.c
>> @@ -33,6 +33,11 @@ static void kunit_ida_init(struct kunit *test, struct ida *ida)
>>   	kunit_alloc_resource(test, __ida_init, __ida_destroy, GFP_KERNEL, ida);
>>   }
>>   
>> +static void tb_test_switch_release(struct device *dev)
>> +{
>> +	/* The memory is owned by KUnit, nothing to do here */
>> +}
>> +
>>   static struct tb_switch *alloc_switch(struct kunit *test, u64 route,
>>   				      u8 upstream_port, u8 max_port_number)
>>   {
>> @@ -44,6 +49,13 @@ static struct tb_switch *alloc_switch(struct kunit *test, u64 route,
>>   	if (!sw)
>>   		return NULL;
>>   
>> +	/*
>> +	 * The paths take a reference to their switches and those devices
>> +	 * have to be initialized for that to work.
>> +	 */
>> +	sw->dev.release = tb_test_switch_release;
>> +	device_initialize(&sw->dev);
> The idea was that we don't use this as real device but if we go this route
> then I think we should call put_device() to release it and check for any
> subtleties device_initialize() possibly does.

Ack.


Best,


Sven

Re: [PATCH v2 1/7] thunderbolt: Hold a router reference for each path hop

[Mika Westerberg]

Hi,

On Mon, Aug 24, 2026 at 01:06:12PM +0200, Sven Peter wrote:
> So unfortunately the entire USB hardware is best described as horribly
> broken :(

:-(

> For USB2 already we only ever receive a single hotplug interrupt in dwc3. If
> the first device is unplugged and another one plugged in nothing happens.
> The only way around that is to tear down dwc3/xhci and the PHY completely,
> assert all external reset lines and then bring them up again.
> Upstream drivers/usb/dwc3/dwc3-apple.c contains more details in the very
> first comment at the top.
> 
> With USB4 this gets a bit worse: There's a block called "ACIO" (Apple
> Converged I/O) which can only be brought up correctly after the PHY has been
> switched to Thunderbolt/USB4 mode. This block has a co-processor which then
> exposes the host router, NHI, IOMMU, etc. to our address space.
> We then have to write cable information into a vendor-specific capabilities
> register and only then do we get the hotplug event and the link comes up.
> This register appears to be effectively write-once after each boot of the
> co-processor. If I try to write 0 that still works but trying to write the
> value for the next connection then crashes the co-processor with a very
> helpful message along the lines of "assert 4357 violated".
> So there's no way around first tearing everything down, then shutting down
> the co-processor and the entire ACIO block and then finally bringing it all
> up again after the next cable is connected. I've tried to find ways around
> this but didn't succeed without documentation and could only reproduce what
> XNU does.

Thanks for sharing the interesting details! I totally understand the pain
of reverse engineering things. The initial USB4/TB driver was also result
of a huge reverse engineering effort by Andreas Noever, but it was and
still is functional, and we still support Apple TB1/2 hardware (although
not too much testing is done these days, I fire up my Cactus Ridge based
Mac maybe once a year).

But okay it does not sound like the PC ACPI hotplug "solution" we had at
least. It's just a much more complex subsystem with many interactions that
need to be done in certain order which is not documented anywhere ;-)

Re: [PATCH v2 1/7] thunderbolt: Hold a router reference for each path hop

[Sven Peter]

On 8/24/26 13:37, Mika Westerberg wrote:
> Hi,
>
> On Mon, Aug 24, 2026 at 01:06:12PM +0200, Sven Peter wrote:
>> So unfortunately the entire USB hardware is best described as horribly
>> broken :(
> :-(
>
>> For USB2 already we only ever receive a single hotplug interrupt in dwc3. If
>> the first device is unplugged and another one plugged in nothing happens.
>> The only way around that is to tear down dwc3/xhci and the PHY completely,
>> assert all external reset lines and then bring them up again.
>> Upstream drivers/usb/dwc3/dwc3-apple.c contains more details in the very
>> first comment at the top.
>>
>> With USB4 this gets a bit worse: There's a block called "ACIO" (Apple
>> Converged I/O) which can only be brought up correctly after the PHY has been
>> switched to Thunderbolt/USB4 mode. This block has a co-processor which then
>> exposes the host router, NHI, IOMMU, etc. to our address space.
>> We then have to write cable information into a vendor-specific capabilities
>> register and only then do we get the hotplug event and the link comes up.
>> This register appears to be effectively write-once after each boot of the
>> co-processor. If I try to write 0 that still works but trying to write the
>> value for the next connection then crashes the co-processor with a very
>> helpful message along the lines of "assert 4357 violated".
>> So there's no way around first tearing everything down, then shutting down
>> the co-processor and the entire ACIO block and then finally bringing it all
>> up again after the next cable is connected. I've tried to find ways around
>> this but didn't succeed without documentation and could only reproduce what
>> XNU does.
> Thanks for sharing the interesting details! I totally understand the pain
> of reverse engineering things. The initial USB4/TB driver was also result
> of a huge reverse engineering effort by Andreas Noever, but it was and
> still is functional, and we still support Apple TB1/2 hardware (although
> not too much testing is done these days, I fire up my Cactus Ridge based
> Mac maybe once a year).

Hah, nice! I wasn't aware the initial version of this driver was built 
for Apple hardware!
Let's see if we can the Apple Silicon changes into a shape where they 
still work 10+ years from now as well :-)


Sven

[PATCH v2 2/7] thunderbolt: Make the DP tunnel activation callback mandatory

[Sven Peter]

tb_tunnel_alloc_dp() takes an optional callback which is run from
dprx_work once the DPRX capabilities read has completed. Without that
callback tb_dp_dprx_start() reads the capabilities synchronously and
never queues the work. It however always takes a tunnel reference which
is only dropped by dprx_work itself or by tb_dp_dprx_stop() when
cancel_delayed_work() actually canceled that work. That reference is
thus leaked for every tunnel without a callback.

The only tunnels without one are those from tb_tunnel_discover_dp(),
which are activated again when restoring from hibernation.
Pass the callback to tb_tunnel_discover_dp() as well and drop the
synchronous path such that the DPRX capabilities are always read from
dprx_work. Hibernation restore then also no longer blocks for up to 12
seconds while waiting for that read to complete.

Also fix up the KUnit tests.

Fixes: d6d458d42e1e ("thunderbolt: Handle DisplayPort tunnel activation asynchronously")
Cc: stable@vger.kernel.org
Signed-off-by: Sven Peter <sven@kernel.org>
---
 drivers/thunderbolt/tb.c     |  4 +++-
 drivers/thunderbolt/test.c   | 37 +++++++++++++++++++++++-----------
 drivers/thunderbolt/tunnel.c | 47 ++++++++++++++++++++++++--------------------
 drivers/thunderbolt/tunnel.h |  8 +++++---
 4 files changed, 60 insertions(+), 36 deletions(-)

diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c
index f43f2d952372..29b9879c40d8 100644
--- a/drivers/thunderbolt/tb.c
+++ b/drivers/thunderbolt/tb.c
@@ -89,6 +89,7 @@ static void tb_dp_resource_unavailable(struct tb *tb, struct tb_port *port,
 				       const char *reason);
 static void tb_queue_dp_bandwidth_request(struct tb *tb, u64 route, u8 port,
 					  int retry, unsigned long delay);
+static void tb_dp_tunnel_active(struct tb_tunnel *tunnel, void *data);
 
 static void tb_queue_hotplug(struct tb *tb, u64 route, u8 port, bool unplug)
 {
@@ -385,7 +386,8 @@ static void tb_switch_discover_tunnels(struct tb_switch *sw,
 
 		switch (port->config.type) {
 		case TB_TYPE_DP_HDMI_IN:
-			tunnel = tb_tunnel_discover_dp(tb, port, alloc_hopids);
+			tunnel = tb_tunnel_discover_dp(tb, port, alloc_hopids,
+						       tb_dp_tunnel_active, tb);
 			tb_increase_tmu_accuracy(tunnel);
 			break;
 
diff --git a/drivers/thunderbolt/test.c b/drivers/thunderbolt/test.c
index 034c56845380..fc3f647bf664 100644
--- a/drivers/thunderbolt/test.c
+++ b/drivers/thunderbolt/test.c
@@ -1398,6 +1398,10 @@ static void tb_test_tunnel_pcie(struct kunit *test)
 	tb_tunnel_put(tunnel1);
 }
 
+static void tb_test_dp_tunnel_active(struct tb_tunnel *tunnel, void *data)
+{
+}
+
 static void tb_test_tunnel_dp(struct kunit *test)
 {
 	struct tb_switch *host, *dev;
@@ -1418,7 +1422,8 @@ static void tb_test_tunnel_dp(struct kunit *test)
 	in = &host->ports[5];
 	out = &dev->ports[13];
 
-	tunnel = tb_tunnel_alloc_dp(NULL, in, out, 1, 0, 0, NULL, NULL);
+	tunnel = tb_tunnel_alloc_dp(NULL, in, out, 1, 0, 0,
+				    tb_test_dp_tunnel_active, NULL);
 	KUNIT_ASSERT_NOT_NULL(test, tunnel);
 	KUNIT_EXPECT_EQ(test, tunnel->type, TB_TUNNEL_DP);
 	KUNIT_EXPECT_PTR_EQ(test, tunnel->src_port, in);
@@ -1464,7 +1469,8 @@ static void tb_test_tunnel_dp_chain(struct kunit *test)
 	in = &host->ports[5];
 	out = &dev4->ports[14];
 
-	tunnel = tb_tunnel_alloc_dp(NULL, in, out, 1, 0, 0, NULL, NULL);
+	tunnel = tb_tunnel_alloc_dp(NULL, in, out, 1, 0, 0,
+				    tb_test_dp_tunnel_active, NULL);
 	KUNIT_ASSERT_NOT_NULL(test, tunnel);
 	KUNIT_EXPECT_EQ(test, tunnel->type, TB_TUNNEL_DP);
 	KUNIT_EXPECT_PTR_EQ(test, tunnel->src_port, in);
@@ -1514,7 +1520,8 @@ static void tb_test_tunnel_dp_tree(struct kunit *test)
 	in = &dev2->ports[13];
 	out = &dev5->ports[13];
 
-	tunnel = tb_tunnel_alloc_dp(NULL, in, out, 1, 0, 0, NULL, NULL);
+	tunnel = tb_tunnel_alloc_dp(NULL, in, out, 1, 0, 0,
+				    tb_test_dp_tunnel_active, NULL);
 	KUNIT_ASSERT_NOT_NULL(test, tunnel);
 	KUNIT_EXPECT_EQ(test, tunnel->type, TB_TUNNEL_DP);
 	KUNIT_EXPECT_PTR_EQ(test, tunnel->src_port, in);
@@ -1579,7 +1586,8 @@ static void tb_test_tunnel_dp_max_length(struct kunit *test)
 	in = &dev6->ports[13];
 	out = &dev12->ports[13];
 
-	tunnel = tb_tunnel_alloc_dp(NULL, in, out, 1, 0, 0, NULL, NULL);
+	tunnel = tb_tunnel_alloc_dp(NULL, in, out, 1, 0, 0,
+				    tb_test_dp_tunnel_active, NULL);
 	KUNIT_ASSERT_NOT_NULL(test, tunnel);
 	KUNIT_EXPECT_EQ(test, tunnel->type, TB_TUNNEL_DP);
 	KUNIT_EXPECT_PTR_EQ(test, tunnel->src_port, in);
@@ -1649,7 +1657,8 @@ static void tb_test_tunnel_3dp(struct kunit *test)
 	out2 = &dev5->ports[13];
 	out3 = &dev4->ports[14];
 
-	tunnel1 = tb_tunnel_alloc_dp(NULL, in1, out1, 1, 0, 0, NULL, NULL);
+	tunnel1 = tb_tunnel_alloc_dp(NULL, in1, out1, 1, 0, 0,
+				     tb_test_dp_tunnel_active, NULL);
 	KUNIT_ASSERT_TRUE(test, tunnel1 != NULL);
 	KUNIT_EXPECT_EQ(test, tunnel1->type, TB_TUNNEL_DP);
 	KUNIT_EXPECT_PTR_EQ(test, tunnel1->src_port, in1);
@@ -1657,7 +1666,8 @@ static void tb_test_tunnel_3dp(struct kunit *test)
 	KUNIT_ASSERT_EQ(test, tunnel1->npaths, 3);
 	KUNIT_ASSERT_EQ(test, tunnel1->paths[0]->path_length, 3);
 
-	tunnel2 = tb_tunnel_alloc_dp(NULL, in2, out2, 1, 0, 0, NULL, NULL);
+	tunnel2 = tb_tunnel_alloc_dp(NULL, in2, out2, 1, 0, 0,
+				     tb_test_dp_tunnel_active, NULL);
 	KUNIT_ASSERT_TRUE(test, tunnel2 != NULL);
 	KUNIT_EXPECT_EQ(test, tunnel2->type, TB_TUNNEL_DP);
 	KUNIT_EXPECT_PTR_EQ(test, tunnel2->src_port, in2);
@@ -1665,7 +1675,8 @@ static void tb_test_tunnel_3dp(struct kunit *test)
 	KUNIT_ASSERT_EQ(test, tunnel2->npaths, 3);
 	KUNIT_ASSERT_EQ(test, tunnel2->paths[0]->path_length, 4);
 
-	tunnel3 = tb_tunnel_alloc_dp(NULL, in3, out3, 1, 0, 0, NULL, NULL);
+	tunnel3 = tb_tunnel_alloc_dp(NULL, in3, out3, 1, 0, 0,
+				     tb_test_dp_tunnel_active, NULL);
 	KUNIT_ASSERT_TRUE(test, tunnel3 != NULL);
 	KUNIT_EXPECT_EQ(test, tunnel3->type, TB_TUNNEL_DP);
 	KUNIT_EXPECT_PTR_EQ(test, tunnel3->src_port, in3);
@@ -1763,7 +1774,8 @@ static void tb_test_tunnel_port_on_path(struct kunit *test)
 	in = &dev2->ports[13];
 	out = &dev5->ports[13];
 
-	dp_tunnel = tb_tunnel_alloc_dp(NULL, in, out, 1, 0, 0, NULL, NULL);
+	dp_tunnel = tb_tunnel_alloc_dp(NULL, in, out, 1, 0, 0,
+				       tb_test_dp_tunnel_active, NULL);
 	KUNIT_ASSERT_NOT_NULL(test, dp_tunnel);
 
 	KUNIT_EXPECT_TRUE(test, tb_tunnel_port_on_path(dp_tunnel, in));
@@ -2195,7 +2207,8 @@ static void tb_test_credit_alloc_dp(struct kunit *test)
 	in = &host->ports[5];
 	out = &dev->ports[14];
 
-	tunnel = tb_tunnel_alloc_dp(NULL, in, out, 1, 0, 0, NULL, NULL);
+	tunnel = tb_tunnel_alloc_dp(NULL, in, out, 1, 0, 0,
+				    tb_test_dp_tunnel_active, NULL);
 	KUNIT_ASSERT_NOT_NULL(test, tunnel);
 	KUNIT_ASSERT_EQ(test, tunnel->npaths, (size_t)3);
 
@@ -2431,7 +2444,8 @@ static struct tb_tunnel *TB_TEST_DP_TUNNEL1(struct kunit *test,
 
 	in = &host->ports[5];
 	out = &dev->ports[13];
-	dp_tunnel1 = tb_tunnel_alloc_dp(NULL, in, out, 1, 0, 0, NULL, NULL);
+	dp_tunnel1 = tb_tunnel_alloc_dp(NULL, in, out, 1, 0, 0,
+					tb_test_dp_tunnel_active, NULL);
 	KUNIT_ASSERT_NOT_NULL(test, dp_tunnel1);
 	KUNIT_ASSERT_EQ(test, dp_tunnel1->npaths, (size_t)3);
 
@@ -2468,7 +2482,8 @@ static struct tb_tunnel *TB_TEST_DP_TUNNEL2(struct kunit *test,
 
 	in = &host->ports[6];
 	out = &dev->ports[14];
-	dp_tunnel2 = tb_tunnel_alloc_dp(NULL, in, out, 1, 0, 0, NULL, NULL);
+	dp_tunnel2 = tb_tunnel_alloc_dp(NULL, in, out, 1, 0, 0,
+					tb_test_dp_tunnel_active, NULL);
 	KUNIT_ASSERT_NOT_NULL(test, dp_tunnel2);
 	KUNIT_ASSERT_EQ(test, dp_tunnel2->npaths, (size_t)3);
 
diff --git a/drivers/thunderbolt/tunnel.c b/drivers/thunderbolt/tunnel.c
index b7f32305f14a..1f978fddaeed 100644
--- a/drivers/thunderbolt/tunnel.c
+++ b/drivers/thunderbolt/tunnel.c
@@ -1106,8 +1106,7 @@ static void tb_dp_dprx_work(struct work_struct *work)
 		mutex_unlock(&tb->lock);
 	}
 
-	if (tunnel->callback)
-		tunnel->callback(tunnel, tunnel->callback_data);
+	tunnel->callback(tunnel, tunnel->callback_data);
 	tb_tunnel_put(tunnel);
 }
 
@@ -1120,15 +1119,10 @@ static int tb_dp_dprx_start(struct tb_tunnel *tunnel)
 	tb_tunnel_get(tunnel);
 
 	tunnel->dprx_started = true;
+	tunnel->dprx_timeout = dprx_timeout_to_ktime(dprx_timeout);
+	queue_delayed_work(tunnel->tb->wq, &tunnel->dprx_work, 0);
 
-	if (tunnel->callback) {
-		tunnel->dprx_timeout = dprx_timeout_to_ktime(dprx_timeout);
-		queue_delayed_work(tunnel->tb->wq, &tunnel->dprx_work, 0);
-		return -EINPROGRESS;
-	}
-
-	return tb_dp_is_usb4(tunnel->src_port->sw) ?
-		tb_dp_wait_dprx(tunnel, dprx_timeout) : 0;
+	return -EINPROGRESS;
 }
 
 static void tb_dp_dprx_stop(struct tb_tunnel *tunnel)
@@ -1579,20 +1573,28 @@ static void tb_dp_dump(struct tb_tunnel *tunnel)
  * @tb: Pointer to the domain structure
  * @in: DP in adapter
  * @alloc_hopid: Allocate HopIDs from visited ports
+ * @callback: Callback that is called when the DP tunnel is fully
+ *	      activated (or there is an error)
+ * @callback_data: Data for @callback
  *
  * If @in adapter is active, follows the tunnel to the DP out adapter
  * and back. Returns the discovered tunnel or %NULL if there was no
- * tunnel.
+ * tunnel. See tb_tunnel_alloc_dp() for @callback.
  *
  * Return: Pointer to &struct tb_tunnel or %NULL if no tunnel found.
  */
 struct tb_tunnel *tb_tunnel_discover_dp(struct tb *tb, struct tb_port *in,
-					bool alloc_hopid)
+					bool alloc_hopid,
+					void (*callback)(struct tb_tunnel *, void *),
+					void *callback_data)
 {
 	struct tb_tunnel *tunnel;
 	struct tb_port *port;
 	struct tb_path *path;
 
+	if (WARN_ON(!callback))
+		return NULL;
+
 	if (!tb_dp_port_is_enabled(in))
 		return NULL;
 
@@ -1608,6 +1610,9 @@ struct tb_tunnel *tb_tunnel_discover_dp(struct tb *tb, struct tb_port *in,
 	tunnel->alloc_bandwidth = tb_dp_alloc_bandwidth;
 	tunnel->consumed_bandwidth = tb_dp_consumed_bandwidth;
 	tunnel->src_port = in;
+	tunnel->callback = callback;
+	tunnel->callback_data = callback_data;
+	INIT_DELAYED_WORK(&tunnel->dprx_work, tb_dp_dprx_work);
 
 	path = tb_path_discover(in, TB_DP_VIDEO_HOPID, NULL, -1,
 				&tunnel->dst_port, "Video", alloc_hopid);
@@ -1674,16 +1679,16 @@ struct tb_tunnel *tb_tunnel_discover_dp(struct tb *tb, struct tb_port *in,
  *	    %0 if no available bandwidth.
  * @max_down: Maximum available downstream bandwidth for the DP tunnel.
  *	      %0 if no available bandwidth.
- * @callback: Optional callback that is called when the DP tunnel is
- *	      fully activated (or there is an error)
- * @callback_data: Optional data for @callback
+ * @callback: Callback that is called when the DP tunnel is fully
+ *	      activated (or there is an error)
+ * @callback_data: Data for @callback
  *
  * Allocates a tunnel between @in and @out that is capable of tunneling
- * Display Port traffic. If @callback is not %NULL it will be called
- * after tb_tunnel_activate() once the tunnel has been fully activated.
- * It can call tb_tunnel_is_active() to check if activation was
- * successful (or if it returns %false there was some sort of issue).
- * The @callback is called without @tb->lock held.
+ * Display Port traffic. The @callback is called after tb_tunnel_activate()
+ * once the tunnel has been fully activated. It can call
+ * tb_tunnel_is_active() to check if activation was successful (or if it
+ * returns %false there was some sort of issue). The @callback is called
+ * without @tb->lock held.
  *
  * Return: Pointer to @struct tb_tunnel or %NULL in case of failure.
  */
@@ -1698,7 +1703,7 @@ struct tb_tunnel *tb_tunnel_alloc_dp(struct tb *tb, struct tb_port *in,
 	struct tb_path *path;
 	bool pm_support;
 
-	if (WARN_ON(!in->cap_adap || !out->cap_adap))
+	if (WARN_ON(!in->cap_adap || !out->cap_adap || !callback))
 		return NULL;
 
 	tunnel = tb_tunnel_alloc(tb, 3, TB_TUNNEL_DP);
diff --git a/drivers/thunderbolt/tunnel.h b/drivers/thunderbolt/tunnel.h
index 4878763a82b3..7d1d255ab5a7 100644
--- a/drivers/thunderbolt/tunnel.h
+++ b/drivers/thunderbolt/tunnel.h
@@ -66,8 +66,8 @@ enum tb_tunnel_state {
  * @dprx_canceled: Was DPRX capabilities read poll canceled
  * @dprx_timeout: If set DPRX capabilities read poll work will timeout after this passes
  * @dprx_work: Worker that is scheduled to poll completion of DPRX capabilities read
- * @callback: Optional callback called when DP tunnel is fully activated
- * @callback_data: Optional data for @callback
+ * @callback: Callback called when DP tunnel is fully activated
+ * @callback_data: Data for @callback
  * @paths: All paths required by the tunnel
  */
 struct tb_tunnel {
@@ -117,7 +117,9 @@ struct tb_tunnel *tb_tunnel_alloc_pci(struct tb *tb, struct tb_port *up,
 bool tb_tunnel_reserved_pci(struct tb_port *port, int *reserved_up,
 			    int *reserved_down);
 struct tb_tunnel *tb_tunnel_discover_dp(struct tb *tb, struct tb_port *in,
-					bool alloc_hopid);
+					bool alloc_hopid,
+					void (*callback)(struct tb_tunnel *, void *),
+					void *callback_data);
 struct tb_tunnel *tb_tunnel_alloc_dp(struct tb *tb, struct tb_port *in,
 				     struct tb_port *out, int link_nr,
 				     int max_up, int max_down,

-- 
2.55.0

Re: [PATCH v2 2/7] thunderbolt: Make the DP tunnel activation callback mandatory

[Mika Westerberg]

On Sun, Aug 23, 2026 at 06:09:15PM +0200, Sven Peter wrote:
> tb_tunnel_alloc_dp() takes an optional callback which is run from
> dprx_work once the DPRX capabilities read has completed. Without that
> callback tb_dp_dprx_start() reads the capabilities synchronously and
> never queues the work. It however always takes a tunnel reference which
> is only dropped by dprx_work itself or by tb_dp_dprx_stop() when
> cancel_delayed_work() actually canceled that work. That reference is
> thus leaked for every tunnel without a callback.
> 
> The only tunnels without one are those from tb_tunnel_discover_dp(),
> which are activated again when restoring from hibernation.
> Pass the callback to tb_tunnel_discover_dp() as well and drop the
> synchronous path such that the DPRX capabilities are always read from
> dprx_work. Hibernation restore then also no longer blocks for up to 12
> seconds while waiting for that read to complete.
> 
> Also fix up the KUnit tests.
> 
> Fixes: d6d458d42e1e ("thunderbolt: Handle DisplayPort tunnel activation asynchronously")
> Cc: stable@vger.kernel.org
> Signed-off-by: Sven Peter <sven@kernel.org>
> ---
>  drivers/thunderbolt/tb.c     |  4 +++-
>  drivers/thunderbolt/test.c   | 37 +++++++++++++++++++++++-----------
>  drivers/thunderbolt/tunnel.c | 47 ++++++++++++++++++++++++--------------------
>  drivers/thunderbolt/tunnel.h |  8 +++++---
>  4 files changed, 60 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c
> index f43f2d952372..29b9879c40d8 100644
> --- a/drivers/thunderbolt/tb.c
> +++ b/drivers/thunderbolt/tb.c
> @@ -89,6 +89,7 @@ static void tb_dp_resource_unavailable(struct tb *tb, struct tb_port *port,
>  				       const char *reason);
>  static void tb_queue_dp_bandwidth_request(struct tb *tb, u64 route, u8 port,
>  					  int retry, unsigned long delay);
> +static void tb_dp_tunnel_active(struct tb_tunnel *tunnel, void *data);

If possible move the whole function here instead of forward declaration.

>  
>  static void tb_queue_hotplug(struct tb *tb, u64 route, u8 port, bool unplug)
>  {
> @@ -385,7 +386,8 @@ static void tb_switch_discover_tunnels(struct tb_switch *sw,
>  
>  		switch (port->config.type) {
>  		case TB_TYPE_DP_HDMI_IN:
> -			tunnel = tb_tunnel_discover_dp(tb, port, alloc_hopids);
> +			tunnel = tb_tunnel_discover_dp(tb, port, alloc_hopids,
> +						       tb_dp_tunnel_active, tb);

Re: [PATCH v2 2/7] thunderbolt: Make the DP tunnel activation callback mandatory

[Sven Peter]

On 8/24/26 12:45, Mika Westerberg wrote:
> On Sun, Aug 23, 2026 at 06:09:15PM +0200, Sven Peter wrote:
>> tb_tunnel_alloc_dp() takes an optional callback which is run from
>> dprx_work once the DPRX capabilities read has completed. Without that
>> callback tb_dp_dprx_start() reads the capabilities synchronously and
>> never queues the work. It however always takes a tunnel reference which
>> is only dropped by dprx_work itself or by tb_dp_dprx_stop() when
>> cancel_delayed_work() actually canceled that work. That reference is
>> thus leaked for every tunnel without a callback.
>>
>> The only tunnels without one are those from tb_tunnel_discover_dp(),
>> which are activated again when restoring from hibernation.
>> Pass the callback to tb_tunnel_discover_dp() as well and drop the
>> synchronous path such that the DPRX capabilities are always read from
>> dprx_work. Hibernation restore then also no longer blocks for up to 12
>> seconds while waiting for that read to complete.
>>
>> Also fix up the KUnit tests.
>>
>> Fixes: d6d458d42e1e ("thunderbolt: Handle DisplayPort tunnel activation asynchronously")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Sven Peter <sven@kernel.org>
>> ---
>>   drivers/thunderbolt/tb.c     |  4 +++-
>>   drivers/thunderbolt/test.c   | 37 +++++++++++++++++++++++-----------
>>   drivers/thunderbolt/tunnel.c | 47 ++++++++++++++++++++++++--------------------
>>   drivers/thunderbolt/tunnel.h |  8 +++++---
>>   4 files changed, 60 insertions(+), 36 deletions(-)
>>
>> diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c
>> index f43f2d952372..29b9879c40d8 100644
>> --- a/drivers/thunderbolt/tb.c
>> +++ b/drivers/thunderbolt/tb.c
>> @@ -89,6 +89,7 @@ static void tb_dp_resource_unavailable(struct tb *tb, struct tb_port *port,
>>   				       const char *reason);
>>   static void tb_queue_dp_bandwidth_request(struct tb *tb, u64 route, u8 port,
>>   					  int retry, unsigned long delay);
>> +static void tb_dp_tunnel_active(struct tb_tunnel *tunnel, void *data);
> If possible move the whole function here instead of forward declaration.

It calls a bunch of helpers that are only defined further down and I'd 
have to move all of them as well (or forward declare them which defeats 
the purpose of doing that)


Sven

Re: [PATCH v2 2/7] thunderbolt: Make the DP tunnel activation callback mandatory

[Mika Westerberg]

On Mon, Aug 24, 2026 at 04:13:44PM +0200, Sven Peter wrote:
> 
> 
> On 8/24/26 12:45, Mika Westerberg wrote:
> > On Sun, Aug 23, 2026 at 06:09:15PM +0200, Sven Peter wrote:
> > > tb_tunnel_alloc_dp() takes an optional callback which is run from
> > > dprx_work once the DPRX capabilities read has completed. Without that
> > > callback tb_dp_dprx_start() reads the capabilities synchronously and
> > > never queues the work. It however always takes a tunnel reference which
> > > is only dropped by dprx_work itself or by tb_dp_dprx_stop() when
> > > cancel_delayed_work() actually canceled that work. That reference is
> > > thus leaked for every tunnel without a callback.
> > > 
> > > The only tunnels without one are those from tb_tunnel_discover_dp(),
> > > which are activated again when restoring from hibernation.
> > > Pass the callback to tb_tunnel_discover_dp() as well and drop the
> > > synchronous path such that the DPRX capabilities are always read from
> > > dprx_work. Hibernation restore then also no longer blocks for up to 12
> > > seconds while waiting for that read to complete.
> > > 
> > > Also fix up the KUnit tests.
> > > 
> > > Fixes: d6d458d42e1e ("thunderbolt: Handle DisplayPort tunnel activation asynchronously")
> > > Cc: stable@vger.kernel.org
> > > Signed-off-by: Sven Peter <sven@kernel.org>
> > > ---
> > >   drivers/thunderbolt/tb.c     |  4 +++-
> > >   drivers/thunderbolt/test.c   | 37 +++++++++++++++++++++++-----------
> > >   drivers/thunderbolt/tunnel.c | 47 ++++++++++++++++++++++++--------------------
> > >   drivers/thunderbolt/tunnel.h |  8 +++++---
> > >   4 files changed, 60 insertions(+), 36 deletions(-)
> > > 
> > > diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c
> > > index f43f2d952372..29b9879c40d8 100644
> > > --- a/drivers/thunderbolt/tb.c
> > > +++ b/drivers/thunderbolt/tb.c
> > > @@ -89,6 +89,7 @@ static void tb_dp_resource_unavailable(struct tb *tb, struct tb_port *port,
> > >   				       const char *reason);
> > >   static void tb_queue_dp_bandwidth_request(struct tb *tb, u64 route, u8 port,
> > >   					  int retry, unsigned long delay);
> > > +static void tb_dp_tunnel_active(struct tb_tunnel *tunnel, void *data);
> > If possible move the whole function here instead of forward declaration.
> 
> It calls a bunch of helpers that are only defined further down and I'd have
> to move all of them as well (or forward declare them which defeats the
> purpose of doing that)

Okay then this is fine.

[PATCH v2 3/7] thunderbolt: Fix domain reference leak when DPRX read is canceled

[Sven Peter]

tb_tunnel_one_dp takes a domain reference which is only dropped once
tb_dp_tunnel_active has run on the work queue. If that work is
cancelled that reference is leaked. Since
commit f5cc545f5969 ("thunderbolt: Wait for tb_domain_release() to
complete when driver is removed") instead of just leaking memory this
now also blocks in the completion wait forever when unbinding the
driver.

This can be triggered whenever a DP tunnel is torn down before the DPRX
read has completed, e.g. by unplugging within the timeout, and then
unbinding the driver.

That reference only exists to keep the domain around while the DPRX work
is scheduled so let the work itself own it: take it in tb_dp_dprx_start
and drop it in both places that end the work. Get/put are then paired
inside the same file and it doesn't matter anymore if the callback ever
runs.

Fixes: d6d458d42e1e ("thunderbolt: Handle DisplayPort tunnel activation asynchronously")
Cc: stable@vger.kernel.org
Signed-off-by: Sven Peter <sven@kernel.org>
---
 drivers/thunderbolt/tb.c     |  6 +-----
 drivers/thunderbolt/tunnel.c | 12 +++++++++---
 2 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c
index 29b9879c40d8..ef4413581b2a 100644
--- a/drivers/thunderbolt/tb.c
+++ b/drivers/thunderbolt/tb.c
@@ -1966,8 +1966,6 @@ static void tb_dp_tunnel_active(struct tb_tunnel *tunnel, void *data)
 		tb_dp_resource_unavailable(tb, in, "DPRX negotiation failed");
 	}
 	mutex_unlock(&tb->lock);
-
-	tb_domain_put(tb);
 }
 
 static void tb_tunnel_one_dp(struct tb *tb, struct tb_port *in,
@@ -2028,8 +2026,7 @@ static void tb_tunnel_one_dp(struct tb *tb, struct tb_port *in,
 	       available_up, available_down);
 
 	tunnel = tb_tunnel_alloc_dp(tb, in, out, link_nr, available_up,
-				    available_down, tb_dp_tunnel_active,
-				    tb_domain_get(tb));
+				    available_down, tb_dp_tunnel_active, tb);
 	if (!tunnel) {
 		tb_port_dbg(out, "could not allocate DP tunnel\n");
 		goto err_reclaim_usb;
@@ -2050,7 +2047,6 @@ static void tb_tunnel_one_dp(struct tb *tb, struct tb_port *in,
 	tb_tunnel_put(tunnel);
 err_reclaim_usb:
 	tb_reclaim_usb3_bandwidth(tb, in, out);
-	tb_domain_put(tb);
 err_detach_group:
 	tb_detach_bandwidth_group(in);
 err_dealloc_dp:
diff --git a/drivers/thunderbolt/tunnel.c b/drivers/thunderbolt/tunnel.c
index 1f978fddaeed..00c5a1933544 100644
--- a/drivers/thunderbolt/tunnel.c
+++ b/drivers/thunderbolt/tunnel.c
@@ -1108,15 +1108,17 @@ static void tb_dp_dprx_work(struct work_struct *work)
 
 	tunnel->callback(tunnel, tunnel->callback_data);
 	tb_tunnel_put(tunnel);
+	tb_domain_put(tb);
 }
 
 static int tb_dp_dprx_start(struct tb_tunnel *tunnel)
 {
 	/*
-	 * Bump up the reference to keep the tunnel around. It will be
-	 * dropped in tb_dp_dprx_stop() once the tunnel is deactivated.
+	 * Bump up the references to keep the tunnel and the domain around
+	 * until the work has run or has been canceled.
 	 */
 	tb_tunnel_get(tunnel);
+	tb_domain_get(tunnel->tb);
 
 	tunnel->dprx_started = true;
 	tunnel->dprx_timeout = dprx_timeout_to_ktime(dprx_timeout);
@@ -1127,11 +1129,15 @@ static int tb_dp_dprx_start(struct tb_tunnel *tunnel)
 
 static void tb_dp_dprx_stop(struct tb_tunnel *tunnel)
 {
+	struct tb *tb = tunnel->tb;
+
 	if (tunnel->dprx_started) {
 		tunnel->dprx_started = false;
 		tunnel->dprx_canceled = true;
-		if (cancel_delayed_work(&tunnel->dprx_work))
+		if (cancel_delayed_work(&tunnel->dprx_work)) {
 			tb_tunnel_put(tunnel);
+			tb_domain_put(tb);
+		}
 	}
 }
 

-- 
2.55.0

[PATCH v2 4/7] thunderbolt: Don't access a DP tunnel after its DPRX read was canceled

[Sven Peter]

tb_dp_dprx_work checks dprx_canceled before it takes tb->lock so it
misses a tb_dp_dprx_stop that could not cancel the already running
work. It then polls the DPRX capabilities and runs the callback for a
tunnel that has already been torn down while the domain is suspending or
going away.

This can be hit by cancelling the DPRX read from outside the ordered
tb->wq: During suspend tb_disconnect_and_release_dp does just this and
with a later patch tb_stop will do it as well. The latter in
combination with the Apple NHI where the DPRX read never completed is
how I hit this.

Check the flag with tb->lock held instead and check it again in
tb_dp_tunnel_active because the callback runs after the lock has been
dropped again.

Also clear the flag in tb_dp_dprx_start so that it only ever describes
the work that is currently in flight.

Fixes: d6d458d42e1e ("thunderbolt: Handle DisplayPort tunnel activation asynchronously")
Cc: stable@vger.kernel.org
Signed-off-by: Sven Peter <sven@kernel.org>
---
 drivers/thunderbolt/tb.c     | 12 ++++++++++++
 drivers/thunderbolt/tunnel.c | 11 +++++++++--
 2 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c
index ef4413581b2a..088323cd876d 100644
--- a/drivers/thunderbolt/tb.c
+++ b/drivers/thunderbolt/tb.c
@@ -1912,6 +1912,18 @@ static void tb_dp_tunnel_active(struct tb_tunnel *tunnel, void *data)
 	struct tb *tb = data;
 
 	mutex_lock(&tb->lock);
+
+	/*
+	 * If the DPRX read was canceled the tunnel is already being torn
+	 * down by whoever canceled it. Do not touch the adapters here
+	 * because the routers may be gone by now.
+	 */
+	if (tunnel->dprx_canceled) {
+		tb_tunnel_dbg(tunnel, "DPRX read canceled, not activating\n");
+		mutex_unlock(&tb->lock);
+		return;
+	}
+
 	if (tb_tunnel_is_active(tunnel)) {
 		int consumed_up, consumed_down, ret;
 
diff --git a/drivers/thunderbolt/tunnel.c b/drivers/thunderbolt/tunnel.c
index 00c5a1933544..5f536635908f 100644
--- a/drivers/thunderbolt/tunnel.c
+++ b/drivers/thunderbolt/tunnel.c
@@ -1090,8 +1090,14 @@ static void tb_dp_dprx_work(struct work_struct *work)
 	struct tb_tunnel *tunnel = container_of(work, typeof(*tunnel), dprx_work.work);
 	struct tb *tb = tunnel->tb;
 
+	/*
+	 * The DPRX read can be canceled while this work is waiting for
+	 * tb->lock. Check the flag only once it is held: while the lock is
+	 * held the tunnel cannot be torn down under us and the adapters are
+	 * safe to access.
+	 */
+	mutex_lock(&tb->lock);
 	if (!tunnel->dprx_canceled) {
-		mutex_lock(&tb->lock);
 		if (tb_dp_is_usb4(tunnel->src_port->sw) &&
 		    tb_dp_wait_dprx(tunnel, TB_DPRX_WAIT_TIMEOUT)) {
 			if (ktime_before(ktime_get(), tunnel->dprx_timeout)) {
@@ -1103,8 +1109,8 @@ static void tb_dp_dprx_work(struct work_struct *work)
 		} else {
 			tb_tunnel_set_active(tunnel, true);
 		}
-		mutex_unlock(&tb->lock);
 	}
+	mutex_unlock(&tb->lock);
 
 	tunnel->callback(tunnel, tunnel->callback_data);
 	tb_tunnel_put(tunnel);
@@ -1121,6 +1127,7 @@ static int tb_dp_dprx_start(struct tb_tunnel *tunnel)
 	tb_domain_get(tunnel->tb);
 
 	tunnel->dprx_started = true;
+	tunnel->dprx_canceled = false;
 	tunnel->dprx_timeout = dprx_timeout_to_ktime(dprx_timeout);
 	queue_delayed_work(tunnel->tb->wq, &tunnel->dprx_work, 0);
 

-- 
2.55.0

[PATCH v2 5/7] thunderbolt: Mark discovered tunnels as active

[Sven Peter]

Discovered tunnels that have been activated by whatever was running
before us stay in TB_TUNNEL_INACTIVE until hibernation restore such that
anything depending on tb_tunnel_is_active() skips them. Mark them active
during discovery instead. These now also emit TUNNEL_EVENT=activated
uevents during discovery.

Fixes: d6d458d42e1e ("thunderbolt: Handle DisplayPort tunnel activation asynchronously")
Cc: stable@vger.kernel.org
Signed-off-by: Sven Peter <sven@kernel.org>
---
 drivers/thunderbolt/tunnel.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/thunderbolt/tunnel.c b/drivers/thunderbolt/tunnel.c
index 5f536635908f..3785e29cf92b 100644
--- a/drivers/thunderbolt/tunnel.c
+++ b/drivers/thunderbolt/tunnel.c
@@ -507,6 +507,7 @@ struct tb_tunnel *tb_tunnel_discover_pci(struct tb *tb, struct tb_port *down,
 		goto err_deactivate;
 	}
 
+	tb_tunnel_set_active(tunnel, true);
 	tb_tunnel_dbg(tunnel, "discovered\n");
 	return tunnel;
 
@@ -1671,6 +1672,7 @@ struct tb_tunnel *tb_tunnel_discover_dp(struct tb *tb, struct tb_port *in,
 
 	tb_dp_dump(tunnel);
 
+	tb_tunnel_set_active(tunnel, true);
 	tb_tunnel_dbg(tunnel, "discovered\n");
 	return tunnel;
 
@@ -2299,6 +2301,7 @@ struct tb_tunnel *tb_tunnel_discover_usb3(struct tb *tb, struct tb_port *down,
 			tb_usb3_reclaim_available_bandwidth;
 	}
 
+	tb_tunnel_set_active(tunnel, true);
 	tb_tunnel_dbg(tunnel, "discovered\n");
 	return tunnel;
 

-- 
2.55.0

[PATCH v2 6/7] thunderbolt: Tear down inactive DP tunnels when the domain is stopped

[Sven Peter]

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.

Tear down DP tunnels that are not active yet as well which also cancels
that work. Tunnels for displays that are already alive are untouched and
keep 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 | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c
index 088323cd876d..921adba3544f 100644
--- a/drivers/thunderbolt/tb.c
+++ b/drivers/thunderbolt/tb.c
@@ -2958,12 +2958,14 @@ static void tb_stop(struct tb *tb)
 	/* tunnels are only present after everything has been initialized */
 	list_for_each_entry_safe(tunnel, n, &tcm->tunnel_list, list) {
 		/*
-		 * DMA tunnels require the driver to be functional so we
-		 * tear them down. Other protocol tunnels can be left
-		 * intact.
+		 * DMA tunnels and DP tunnels which are not yet active require
+		 * the driver to be functional so we tear them down.
+		 * Other protocol tunnels can be left intact.
 		 */
 		if (tb_tunnel_is_dma(tunnel))
 			tb_tunnel_deactivate(tunnel);
+		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);

-- 
2.55.0

[PATCH v2 7/7] thunderbolt: Drop the DP tunnel activation callback data

[Sven Peter]

The callback data is always the domain the tunnel belongs to which the
callback can just take from the tunnel itself.

Signed-off-by: Sven Peter <sven@kernel.org>
---
 drivers/thunderbolt/tb.c     | 10 +++++-----
 drivers/thunderbolt/test.c   | 24 ++++++++++++------------
 drivers/thunderbolt/tunnel.c | 12 +++---------
 drivers/thunderbolt/tunnel.h | 10 +++-------
 4 files changed, 23 insertions(+), 33 deletions(-)

diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c
index 921adba3544f..40a5a3ebb31d 100644
--- a/drivers/thunderbolt/tb.c
+++ b/drivers/thunderbolt/tb.c
@@ -89,7 +89,7 @@ static void tb_dp_resource_unavailable(struct tb *tb, struct tb_port *port,
 				       const char *reason);
 static void tb_queue_dp_bandwidth_request(struct tb *tb, u64 route, u8 port,
 					  int retry, unsigned long delay);
-static void tb_dp_tunnel_active(struct tb_tunnel *tunnel, void *data);
+static void tb_dp_tunnel_active(struct tb_tunnel *tunnel);
 
 static void tb_queue_hotplug(struct tb *tb, u64 route, u8 port, bool unplug)
 {
@@ -387,7 +387,7 @@ static void tb_switch_discover_tunnels(struct tb_switch *sw,
 		switch (port->config.type) {
 		case TB_TYPE_DP_HDMI_IN:
 			tunnel = tb_tunnel_discover_dp(tb, port, alloc_hopids,
-						       tb_dp_tunnel_active, tb);
+						       tb_dp_tunnel_active);
 			tb_increase_tmu_accuracy(tunnel);
 			break;
 
@@ -1905,11 +1905,11 @@ static struct tb_port *tb_find_dp_out(struct tb *tb, struct tb_port *in)
 	return NULL;
 }
 
-static void tb_dp_tunnel_active(struct tb_tunnel *tunnel, void *data)
+static void tb_dp_tunnel_active(struct tb_tunnel *tunnel)
 {
 	struct tb_port *in = tunnel->src_port;
 	struct tb_port *out = tunnel->dst_port;
-	struct tb *tb = data;
+	struct tb *tb = tunnel->tb;
 
 	mutex_lock(&tb->lock);
 
@@ -2038,7 +2038,7 @@ static void tb_tunnel_one_dp(struct tb *tb, struct tb_port *in,
 	       available_up, available_down);
 
 	tunnel = tb_tunnel_alloc_dp(tb, in, out, link_nr, available_up,
-				    available_down, tb_dp_tunnel_active, tb);
+				    available_down, tb_dp_tunnel_active);
 	if (!tunnel) {
 		tb_port_dbg(out, "could not allocate DP tunnel\n");
 		goto err_reclaim_usb;
diff --git a/drivers/thunderbolt/test.c b/drivers/thunderbolt/test.c
index fc3f647bf664..9e128c26f003 100644
--- a/drivers/thunderbolt/test.c
+++ b/drivers/thunderbolt/test.c
@@ -1398,7 +1398,7 @@ static void tb_test_tunnel_pcie(struct kunit *test)
 	tb_tunnel_put(tunnel1);
 }
 
-static void tb_test_dp_tunnel_active(struct tb_tunnel *tunnel, void *data)
+static void tb_test_dp_tunnel_active(struct tb_tunnel *tunnel)
 {
 }
 
@@ -1423,7 +1423,7 @@ static void tb_test_tunnel_dp(struct kunit *test)
 	out = &dev->ports[13];
 
 	tunnel = tb_tunnel_alloc_dp(NULL, in, out, 1, 0, 0,
-				    tb_test_dp_tunnel_active, NULL);
+				    tb_test_dp_tunnel_active);
 	KUNIT_ASSERT_NOT_NULL(test, tunnel);
 	KUNIT_EXPECT_EQ(test, tunnel->type, TB_TUNNEL_DP);
 	KUNIT_EXPECT_PTR_EQ(test, tunnel->src_port, in);
@@ -1470,7 +1470,7 @@ static void tb_test_tunnel_dp_chain(struct kunit *test)
 	out = &dev4->ports[14];
 
 	tunnel = tb_tunnel_alloc_dp(NULL, in, out, 1, 0, 0,
-				    tb_test_dp_tunnel_active, NULL);
+				    tb_test_dp_tunnel_active);
 	KUNIT_ASSERT_NOT_NULL(test, tunnel);
 	KUNIT_EXPECT_EQ(test, tunnel->type, TB_TUNNEL_DP);
 	KUNIT_EXPECT_PTR_EQ(test, tunnel->src_port, in);
@@ -1521,7 +1521,7 @@ static void tb_test_tunnel_dp_tree(struct kunit *test)
 	out = &dev5->ports[13];
 
 	tunnel = tb_tunnel_alloc_dp(NULL, in, out, 1, 0, 0,
-				    tb_test_dp_tunnel_active, NULL);
+				    tb_test_dp_tunnel_active);
 	KUNIT_ASSERT_NOT_NULL(test, tunnel);
 	KUNIT_EXPECT_EQ(test, tunnel->type, TB_TUNNEL_DP);
 	KUNIT_EXPECT_PTR_EQ(test, tunnel->src_port, in);
@@ -1587,7 +1587,7 @@ static void tb_test_tunnel_dp_max_length(struct kunit *test)
 	out = &dev12->ports[13];
 
 	tunnel = tb_tunnel_alloc_dp(NULL, in, out, 1, 0, 0,
-				    tb_test_dp_tunnel_active, NULL);
+				    tb_test_dp_tunnel_active);
 	KUNIT_ASSERT_NOT_NULL(test, tunnel);
 	KUNIT_EXPECT_EQ(test, tunnel->type, TB_TUNNEL_DP);
 	KUNIT_EXPECT_PTR_EQ(test, tunnel->src_port, in);
@@ -1658,7 +1658,7 @@ static void tb_test_tunnel_3dp(struct kunit *test)
 	out3 = &dev4->ports[14];
 
 	tunnel1 = tb_tunnel_alloc_dp(NULL, in1, out1, 1, 0, 0,
-				     tb_test_dp_tunnel_active, NULL);
+				     tb_test_dp_tunnel_active);
 	KUNIT_ASSERT_TRUE(test, tunnel1 != NULL);
 	KUNIT_EXPECT_EQ(test, tunnel1->type, TB_TUNNEL_DP);
 	KUNIT_EXPECT_PTR_EQ(test, tunnel1->src_port, in1);
@@ -1667,7 +1667,7 @@ static void tb_test_tunnel_3dp(struct kunit *test)
 	KUNIT_ASSERT_EQ(test, tunnel1->paths[0]->path_length, 3);
 
 	tunnel2 = tb_tunnel_alloc_dp(NULL, in2, out2, 1, 0, 0,
-				     tb_test_dp_tunnel_active, NULL);
+				     tb_test_dp_tunnel_active);
 	KUNIT_ASSERT_TRUE(test, tunnel2 != NULL);
 	KUNIT_EXPECT_EQ(test, tunnel2->type, TB_TUNNEL_DP);
 	KUNIT_EXPECT_PTR_EQ(test, tunnel2->src_port, in2);
@@ -1676,7 +1676,7 @@ static void tb_test_tunnel_3dp(struct kunit *test)
 	KUNIT_ASSERT_EQ(test, tunnel2->paths[0]->path_length, 4);
 
 	tunnel3 = tb_tunnel_alloc_dp(NULL, in3, out3, 1, 0, 0,
-				     tb_test_dp_tunnel_active, NULL);
+				     tb_test_dp_tunnel_active);
 	KUNIT_ASSERT_TRUE(test, tunnel3 != NULL);
 	KUNIT_EXPECT_EQ(test, tunnel3->type, TB_TUNNEL_DP);
 	KUNIT_EXPECT_PTR_EQ(test, tunnel3->src_port, in3);
@@ -1775,7 +1775,7 @@ static void tb_test_tunnel_port_on_path(struct kunit *test)
 	out = &dev5->ports[13];
 
 	dp_tunnel = tb_tunnel_alloc_dp(NULL, in, out, 1, 0, 0,
-				       tb_test_dp_tunnel_active, NULL);
+				       tb_test_dp_tunnel_active);
 	KUNIT_ASSERT_NOT_NULL(test, dp_tunnel);
 
 	KUNIT_EXPECT_TRUE(test, tb_tunnel_port_on_path(dp_tunnel, in));
@@ -2208,7 +2208,7 @@ static void tb_test_credit_alloc_dp(struct kunit *test)
 	out = &dev->ports[14];
 
 	tunnel = tb_tunnel_alloc_dp(NULL, in, out, 1, 0, 0,
-				    tb_test_dp_tunnel_active, NULL);
+				    tb_test_dp_tunnel_active);
 	KUNIT_ASSERT_NOT_NULL(test, tunnel);
 	KUNIT_ASSERT_EQ(test, tunnel->npaths, (size_t)3);
 
@@ -2445,7 +2445,7 @@ static struct tb_tunnel *TB_TEST_DP_TUNNEL1(struct kunit *test,
 	in = &host->ports[5];
 	out = &dev->ports[13];
 	dp_tunnel1 = tb_tunnel_alloc_dp(NULL, in, out, 1, 0, 0,
-					tb_test_dp_tunnel_active, NULL);
+					tb_test_dp_tunnel_active);
 	KUNIT_ASSERT_NOT_NULL(test, dp_tunnel1);
 	KUNIT_ASSERT_EQ(test, dp_tunnel1->npaths, (size_t)3);
 
@@ -2483,7 +2483,7 @@ static struct tb_tunnel *TB_TEST_DP_TUNNEL2(struct kunit *test,
 	in = &host->ports[6];
 	out = &dev->ports[14];
 	dp_tunnel2 = tb_tunnel_alloc_dp(NULL, in, out, 1, 0, 0,
-					tb_test_dp_tunnel_active, NULL);
+					tb_test_dp_tunnel_active);
 	KUNIT_ASSERT_NOT_NULL(test, dp_tunnel2);
 	KUNIT_ASSERT_EQ(test, dp_tunnel2->npaths, (size_t)3);
 
diff --git a/drivers/thunderbolt/tunnel.c b/drivers/thunderbolt/tunnel.c
index 3785e29cf92b..cbffb1e612b6 100644
--- a/drivers/thunderbolt/tunnel.c
+++ b/drivers/thunderbolt/tunnel.c
@@ -1113,7 +1113,7 @@ static void tb_dp_dprx_work(struct work_struct *work)
 	}
 	mutex_unlock(&tb->lock);
 
-	tunnel->callback(tunnel, tunnel->callback_data);
+	tunnel->callback(tunnel);
 	tb_tunnel_put(tunnel);
 	tb_domain_put(tb);
 }
@@ -1589,7 +1589,6 @@ static void tb_dp_dump(struct tb_tunnel *tunnel)
  * @alloc_hopid: Allocate HopIDs from visited ports
  * @callback: Callback that is called when the DP tunnel is fully
  *	      activated (or there is an error)
- * @callback_data: Data for @callback
  *
  * If @in adapter is active, follows the tunnel to the DP out adapter
  * and back. Returns the discovered tunnel or %NULL if there was no
@@ -1599,8 +1598,7 @@ static void tb_dp_dump(struct tb_tunnel *tunnel)
  */
 struct tb_tunnel *tb_tunnel_discover_dp(struct tb *tb, struct tb_port *in,
 					bool alloc_hopid,
-					void (*callback)(struct tb_tunnel *, void *),
-					void *callback_data)
+					void (*callback)(struct tb_tunnel *))
 {
 	struct tb_tunnel *tunnel;
 	struct tb_port *port;
@@ -1625,7 +1623,6 @@ struct tb_tunnel *tb_tunnel_discover_dp(struct tb *tb, struct tb_port *in,
 	tunnel->consumed_bandwidth = tb_dp_consumed_bandwidth;
 	tunnel->src_port = in;
 	tunnel->callback = callback;
-	tunnel->callback_data = callback_data;
 	INIT_DELAYED_WORK(&tunnel->dprx_work, tb_dp_dprx_work);
 
 	path = tb_path_discover(in, TB_DP_VIDEO_HOPID, NULL, -1,
@@ -1696,7 +1693,6 @@ struct tb_tunnel *tb_tunnel_discover_dp(struct tb *tb, struct tb_port *in,
  *	      %0 if no available bandwidth.
  * @callback: Callback that is called when the DP tunnel is fully
  *	      activated (or there is an error)
- * @callback_data: Data for @callback
  *
  * Allocates a tunnel between @in and @out that is capable of tunneling
  * Display Port traffic. The @callback is called after tb_tunnel_activate()
@@ -1710,8 +1706,7 @@ struct tb_tunnel *tb_tunnel_discover_dp(struct tb *tb, struct tb_port *in,
 struct tb_tunnel *tb_tunnel_alloc_dp(struct tb *tb, struct tb_port *in,
 				     struct tb_port *out, int link_nr,
 				     int max_up, int max_down,
-				     void (*callback)(struct tb_tunnel *, void *),
-				     void *callback_data)
+				     void (*callback)(struct tb_tunnel *))
 {
 	struct tb_tunnel *tunnel;
 	struct tb_path **paths;
@@ -1737,7 +1732,6 @@ struct tb_tunnel *tb_tunnel_alloc_dp(struct tb *tb, struct tb_port *in,
 	tunnel->max_up = max_up;
 	tunnel->max_down = max_down;
 	tunnel->callback = callback;
-	tunnel->callback_data = callback_data;
 	INIT_DELAYED_WORK(&tunnel->dprx_work, tb_dp_dprx_work);
 
 	paths = tunnel->paths;
diff --git a/drivers/thunderbolt/tunnel.h b/drivers/thunderbolt/tunnel.h
index 7d1d255ab5a7..28f49f7e9f56 100644
--- a/drivers/thunderbolt/tunnel.h
+++ b/drivers/thunderbolt/tunnel.h
@@ -67,7 +67,6 @@ enum tb_tunnel_state {
  * @dprx_timeout: If set DPRX capabilities read poll work will timeout after this passes
  * @dprx_work: Worker that is scheduled to poll completion of DPRX capabilities read
  * @callback: Callback called when DP tunnel is fully activated
- * @callback_data: Data for @callback
  * @paths: All paths required by the tunnel
  */
 struct tb_tunnel {
@@ -104,8 +103,7 @@ struct tb_tunnel {
 	bool dprx_canceled;
 	ktime_t dprx_timeout;
 	struct delayed_work dprx_work;
-	void (*callback)(struct tb_tunnel *tunnel, void *data);
-	void *callback_data;
+	void (*callback)(struct tb_tunnel *tunnel);
 
 	struct tb_path *paths[] __counted_by(npaths);
 };
@@ -118,13 +116,11 @@ bool tb_tunnel_reserved_pci(struct tb_port *port, int *reserved_up,
 			    int *reserved_down);
 struct tb_tunnel *tb_tunnel_discover_dp(struct tb *tb, struct tb_port *in,
 					bool alloc_hopid,
-					void (*callback)(struct tb_tunnel *, void *),
-					void *callback_data);
+					void (*callback)(struct tb_tunnel *));
 struct tb_tunnel *tb_tunnel_alloc_dp(struct tb *tb, struct tb_port *in,
 				     struct tb_port *out, int link_nr,
 				     int max_up, int max_down,
-				     void (*callback)(struct tb_tunnel *, void *),
-				     void *callback_data);
+				     void (*callback)(struct tb_tunnel *));
 struct tb_tunnel *tb_tunnel_alloc_dma(struct tb *tb, struct tb_port *nhi,
 				      struct tb_port *dst, int transmit_path,
 				      int transmit_ring, int receive_path,

-- 
2.55.0