* [PATCH] USB: hub: Move typec deattach before children disconnections in usb_disconnect()
@ 2025-07-20 21:08 Marco Tormento
2025-07-21 1:21 ` Alan Stern
0 siblings, 1 reply; 5+ messages in thread
From: Marco Tormento @ 2025-07-20 21:08 UTC (permalink / raw)
To: gregkh; +Cc: linux-usb, linux-kernel, Marco Tormento
I have a Thinkpad T480s, it has 2 USB typec ports one of which is part of
the thunderbolt connector. On the thunderbolt typec port I plug a BenQ
EX3501R external monitor that has an integrated USB hub with a couple of
HID devices connected to it. On the other typec port I plug an Apple
Macbook USB-C power brick.
Steps to reproduce the issue this patch is supposed to fix:
- plug the power brick into typec port
- plug the monitor into the other typec port (thunderbolt)
- unplug the monitor
- unplug the power brick: get a warning stack trace saying "kernfs: can not
remove 'typec', no directory" in typec_unregister_partner()
- plug the power brick again
- plug the monitor again
- unplug the monitor
- plug the monitor again: get a warning stack trace saying "sysfs: cannot
create duplicate filename
'/devices/platform/USBC000:00/typec/port0/port0-partner/3-1'" in
typec_partner_link_device() called by typec_partner_attach()
From that point on if you do the following steps a couple of times you get
a kernel oops with a general protection fault in
typec_unregister_partner(), system becomes unstable and a restart is
required:
- unplug the monitor
- unplug the power brick
- plug the power brick
- plug the monitor
- unplug the monitor
- unplug the power brick
If I plug the power brick to thunderbolt and the monitor to the other typec
port none of the above happens.
I tracked down the issue to the following logic:
- power brick is plugged in
- monitor is plugged in
- when I unplug the monitor in usb_disconnect() hub_disconnect_children()
calls usb_disconnect() recursively, and this results in
connector_unbind() invoked on all connectors, which resets
port_dev->connector to NULL on the ports
- typec_deattach() is called for each device that has a parent, which in
turn should fire typec_partner_deattach()
- port_dev->connector is NULL though, so typec_partner_deattach() is not
called and port->usb2_dev is not set to NULL even though the hub device
is actually gone
Now if I plug the monitor again:
- since the type_partner_deattach() was not invoked before, some links are
still there and typec_partner_link_device() complains
If I unplug the power brick instead:
- ucsi_handle_connector_change() is invoked and typec_unregister_partner()
is executed, port->usb2_dev is not NULL so we try to unlink the device
from the partner with typec_partner_unlink_device(), but 'typec'
directory doesn't exist anymore so it fails in the sysfs_remove_link()
call
The reason why at some point kernel oops when I unplug the power brick a
final time is this: in typec_unregister_partner() when
typec_partner_unlink_device() is called port->usb2_dev->kobj ref count is
zero (the device is gone already), so when accessing usb_dev->kobj fields
it's just a matter of time and memory will be overwritten with something
else, which will cause access to protected memory and an oops.
In order to fix the above I tried moving the code that handles typec
deattachment from the parent before all the disconnections, this way
typec_partner_deattach() is invoked for the partner, port->usb2_dev is
cleared and typec_unregister_partner() is happy.
Note
It's still not clear to me why the hub device of the monitor is linked with
the typec port partner of the power brick in /sys/class/typec. The
thunderbolt controller surely has something to do with it, since it doesn't
happen if the monitor is plugged into the standard typec port instead, but
I haven't fully understood why it happens yet: maybe this is the actual
bug?
e.g.
Plug the power brick:
- /sys/class/typec/port0-partner appears
Plug the monitor:
- /sys/class/typec/port1-partner appears
- /sys/class/typec/port0-partner/3-1 appears, which is the monitor hub
If I plug the monitor first and then the power brick
/sys/class/typec/port0-partner/3-1 doesn't show up.
Disclaimer
I just started looking at kernel code for the first time a week ago, so all
of the above might be spectacularly wrong or a better way to fix the issue
might exist. Initially I did a quick fix that was just checking
port->usb2_dev->kobj ref count in typec_partner_unlink_device() and bailing
out if it was zero, but then I decided to dig deeper just for fun.
This patch has solved my problem, I'm running it on laptop since last week
without any issue, but it might have side-effects I'm not aware of.
I really hope I'm not wasting your time, thanks for all the great work you
do on the Linux kernel!
Signed-off-by: Marco Tormento <mtormento80@gmail.com>
---
drivers/usb/core/hub.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 256fe8c86828..dfe4ba192faf 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -2334,16 +2334,6 @@ void usb_disconnect(struct usb_device **pdev)
usb_lock_device(udev);
- hub_disconnect_children(udev);
-
- /* deallocate hcd/hardware state ... nuking all pending urbs and
- * cleaning up all state associated with the current configuration
- * so that the hardware is now fully quiesced.
- */
- dev_dbg(&udev->dev, "unregistering device\n");
- usb_disable_device(udev, 0);
- usb_hcd_synchronize_unlinks(udev);
-
if (udev->parent) {
port1 = udev->portnum;
hub = usb_hub_to_struct_hub(udev->parent);
@@ -2362,6 +2352,16 @@ void usb_disconnect(struct usb_device **pdev)
typec_deattach(port_dev->connector, &udev->dev);
}
+ hub_disconnect_children(udev);
+
+ /* deallocate hcd/hardware state ... nuking all pending urbs and
+ * cleaning up all state associated with the current configuration
+ * so that the hardware is now fully quiesced.
+ */
+ dev_dbg(&udev->dev, "unregistering device\n");
+ usb_disable_device(udev, 0);
+ usb_hcd_synchronize_unlinks(udev);
+
usb_remove_ep_devs(&udev->ep0);
usb_unlock_device(udev);
--
2.50.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] USB: hub: Move typec deattach before children disconnections in usb_disconnect()
2025-07-20 21:08 [PATCH] USB: hub: Move typec deattach before children disconnections in usb_disconnect() Marco Tormento
@ 2025-07-21 1:21 ` Alan Stern
2025-07-21 23:18 ` Marco Tormento
0 siblings, 1 reply; 5+ messages in thread
From: Alan Stern @ 2025-07-21 1:21 UTC (permalink / raw)
To: Marco Tormento; +Cc: gregkh, linux-usb, linux-kernel
On Sun, Jul 20, 2025 at 11:08:47PM +0200, Marco Tormento wrote:
> I tracked down the issue to the following logic:
> - power brick is plugged in
> - monitor is plugged in
> - when I unplug the monitor in usb_disconnect() hub_disconnect_children()
> calls usb_disconnect() recursively, and this results in
> connector_unbind() invoked on all connectors, which resets
> port_dev->connector to NULL on the ports
I'm not a typec expert; in fact I know practically nothing about it.
Nevertheless, this sounds strange. The recursive usb_disconnect() calls
should affect the connectors to the monitor's children and the monitor's
own ports, not the connector or port on the monitor's parent hub.
> - typec_deattach() is called for each device that has a parent, which in
> turn should fire typec_partner_deattach()
> - port_dev->connector is NULL though, so typec_partner_deattach() is not
> called and port->usb2_dev is not set to NULL even though the hub device
> is actually gone
But that's port_dev->connector for the port on the monitor's parent hub,
which shouldn't have been affected.
> In order to fix the above I tried moving the code that handles typec
> deattachment from the parent before all the disconnections, this way
> typec_partner_deattach() is invoked for the partner, port->usb2_dev is
> cleared and typec_unregister_partner() is happy.
In essence, you're moving the typec_deattach() call up before the
hub_disconnect_children() and usb_disable_device() calls. (The
sysfs_remove_link() calls get moved up too, but they probably don't
matter much.) I'm not saying this is the wrong thing to do or that it
will cause problems, but it does seem odd. In particular, it's not the
reverse order of the way these things were set up when the devices were
originally detected.
Alan Stern
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] USB: hub: Move typec deattach before children disconnections in usb_disconnect()
2025-07-21 1:21 ` Alan Stern
@ 2025-07-21 23:18 ` Marco Tormento
2025-07-22 3:05 ` Alan Stern
0 siblings, 1 reply; 5+ messages in thread
From: Marco Tormento @ 2025-07-21 23:18 UTC (permalink / raw)
To: Alan Stern; +Cc: gregkh, linux-usb, linux-kernel
On Mon, 21 Jul 2025 at 03:21, Alan Stern <stern@rowland.harvard.edu> wrote:
> I'm not a typec expert; in fact I know practically nothing about it.
> Nevertheless, this sounds strange. The recursive usb_disconnect() calls
> should affect the connectors to the monitor's children and the monitor's
> own ports, not the connector or port on the monitor's parent hub.
What you wrote makes total sense, let me add some detail though.
When I plug the monitor to the thunderbolt port, 3 usb hubs pop up, but only 2
are backed by XHCI Host Controllers: usb3 and usb4.
usb 3-1 instead is using usb3 and it has 3 devices connected to it: mouse,
keyboard and a mysterious billboard device:
xhci_hcd 0000:3c:00.0: xHCI Host Controller
xhci_hcd 0000:3c:00.0: new USB bus registered, assigned bus number 3
xhci_hcd 0000:3c:00.0: hcc params 0x200077c1 hci version 0x110 quirks
0x0000000200009810
xhci_hcd 0000:3c:00.0: xHCI Host Controller
xhci_hcd 0000:3c:00.0: new USB bus registered, assigned bus number 4
xhci_hcd 0000:3c:00.0: Host supports USB 3.1 Enhanced SuperSpeed
usb usb3: New USB device found, idVendor=1d6b, idProduct=0002, bcdDevice= 6.15
usb usb3: New USB device strings: Mfr=3, Product=2, SerialNumber=1
usb usb3: Product: xHCI Host Controller
usb usb3: Manufacturer: Linux 6.15.7-arch1-1-mentor xhci-hcd
usb usb3: SerialNumber: 0000:3c:00.0
hub 3-0:1.0: USB hub found
hub 3-0:1.0: 2 ports detected
usb usb4: New USB device found, idVendor=1d6b, idProduct=0003, bcdDevice= 6.15
usb usb4: New USB device strings: Mfr=3, Product=2, SerialNumber=1
usb usb4: Product: xHCI Host Controller
usb usb4: Manufacturer: Linux 6.15.7-arch1-1-mentor xhci-hcd
usb usb4: SerialNumber: 0000:3c:00.0
hub 4-0:1.0: USB hub found
hub 4-0:1.0: 2 ports detected
typec port0: bound usb3-port1 (ops connector_ops [usbcore])
typec port0: bound usb4-port1 (ops connector_ops [usbcore])
typec port1: bound usb3-port2 (ops connector_ops [usbcore])
typec port1: bound usb4-port2 (ops connector_ops [usbcore])
usb 3-1: new high-speed USB device number 2 using xhci_hcd
usb 3-1: New USB device found, idVendor=0bda, idProduct=5411, bcdDevice= 1.36
usb 3-1: New USB device strings: Mfr=1, Product=2, SerialNumber=0
usb 3-1: Product: 4-Port USB 2.0 Hub
usb 3-1: Manufacturer: Generic
hub 3-1:1.0: USB hub found
hub 3-1:1.0: 3 ports detected
usb 3-1.3: new full-speed USB device number 3 using xhci_hcd
usb 3-1.3: not running at top speed; connect to a high speed hub
usb 3-1.3: New USB device found, idVendor=0bda, idProduct=5400, bcdDevice= 1.07
usb 3-1.3: New USB device strings: Mfr=17, Product=18, SerialNumber=19
usb 3-1.3: Product: BillBoard Device
usb 3-1.3: Manufacturer: Realtek
usb 3-1.3: SerialNumber: 123456789ABCDEFGH
usb 3-1.2: new full-speed USB device number 4 using xhci_hcd
usb 3-1.2: New USB device found, idVendor=0951, idProduct=16e6, bcdDevice=21.08
usb 3-1.2: New USB device strings: Mfr=1, Product=2, SerialNumber=0
usb 3-1.2: Product: HyperX Alloy Origins Core
usb 3-1.2: Manufacturer: Kingston
usb 3-1.1: new full-speed USB device number 5 using xhci_hcd
usb 3-1.1: New USB device found, idVendor=1a7c, idProduct=0197, bcdDevice= 1.06
usb 3-1.1: New USB device strings: Mfr=1, Product=3, SerialNumber=0
usb 3-1.1: Product: Evoluent VerticalMouse D
usb 3-1.1: Manufacturer: Kingsis Peripherals
When I unplug the monitor though, usb 3-1 is not processed as part of
hub_disconnect_children() of usb3 hub, as I would expect.
It is processed on its own (added some debugging log to some functions):
usb 3-1: [usb_disconnect] debugging
usb 3-1: [usb_disconnect] USB disconnect, device number 2
usb 3-1: [hub_disconnect_children] debugging hub_disconnect_children()
usb 3-1: [hub_disconnect_children] disconnecting child 0
usb 3-1.1: [usb_disconnect] debugging
usb 3-1.1: [usb_disconnect] USB disconnect, device number 5
usb 3-1.1: [hub_disconnect_children] debugging hub_disconnect_children()
usb 3-1.1: [usb_disconnect] unregistering device
usbhid 3-1.1:1.0: [usb_unbind_interface] debugging
usb 3-1.1: [usb_disconnect] parent found
usb 3-1.1: [usb_disconnect] removing port 3-1-port1 from hub 3-1:1.0: 0
usb 3-1.1: [usb_disconnect] done with the device
usb 3-1: [hub_disconnect_children] disconnecting child 1
usb 3-1.2: [usb_disconnect] debugging
usb 3-1.2: [usb_disconnect] USB disconnect, device number 4
usb 3-1.2: [hub_disconnect_children] debugging hub_disconnect_children()
usb 3-1.2: [usb_disconnect] unregistering device
usbhid 3-1.2:1.0: [usb_unbind_interface] debugging
xhci_hcd 0000:3c:00.0: remove, state 4
usb usb4: [usb_disconnect] debugging
usb usb4: [usb_disconnect] USB disconnect, device number 1
usb usb4: [hub_disconnect_children] debugging hub_disconnect_children()
usb usb4: [usb_disconnect] unregistering device
hub 4-0:1.0: [usb_unbind_interface] debugging
usb usb4: [hub_disconnect] debugging
usb usb4-port2: [usb_hub_remove_port_device] debugging: port 1
typec port1: [connector_unbind] unbinding connector from usb4-port2
typec port1: [connector_unbind] unbinding connector from usb3-port2
usb usb4-port1: [usb_hub_remove_port_device] debugging: port 0
typec port0: [connector_unbind] unbinding connector from usb4-port1
typec port0: [connector_unbind] unbinding connector from usb3-port1
usb usb4: [usb_disconnect] done with the device
xhci_hcd 0000:3c:00.0: USB bus 4 deregistered
xhci_hcd 0000:3c:00.0: xHCI host controller not responding, assume dead
xhci_hcd 0000:3c:00.0: remove, state 1
usb usb3: [usb_disconnect] debugging
usb usb3: [usb_disconnect] USB disconnect, device number 1
typec port1-partner: [typec_unregister_partner] debugging
typec port1-partner: [typec_unregister_partner] unregistering from port: port1
usbhid 3-1.2:1.1: [usb_unbind_interface] debugging
usbhid 3-1.2:1.2: [usb_unbind_interface] debugging
usb 3-1.2: [usb_disconnect] parent found
usb 3-1.2: [usb_disconnect] removing port 3-1-port2 from hub 3-1:1.0: 1
usb 3-1.2: [usb_disconnect] done with the device
usb 3-1: [hub_disconnect_children] disconnecting child 2
usb 3-1.3: [usb_disconnect] debugging
usb 3-1.3: [usb_disconnect] USB disconnect, device number 3
usb 3-1.3: [hub_disconnect_children] debugging hub_disconnect_children()
usb 3-1.3: [usb_disconnect] unregistering device
usb 3-1.3: [usb_disconnect] parent found
usb 3-1.3: [usb_disconnect] removing port 3-1-port3 from hub 3-1:1.0: 2
usb 3-1.3: [usb_disconnect] done with the device
usb 3-1: [usb_disconnect] unregistering device
hub 3-1:1.0: [usb_unbind_interface] debugging
usb 3-1: [hub_disconnect] debugging
usb 3-1-port3: [usb_hub_remove_port_device] debugging: port 2
usb 3-1-port2: [usb_hub_remove_port_device] debugging: port 1
usb 3-1-port1: [usb_hub_remove_port_device] debugging: port 0
usb 3-1: [usb_disconnect] parent found
usb 3-1: [usb_disconnect] removing port usb3-port1 from hub 3-0:1.0: 0
usb 3-1: [usb_disconnect] done with the device
usb usb3: [hub_disconnect_children] debugging hub_disconnect_children()
usb usb3: [usb_disconnect] unregistering device
hub 3-0:1.0: [usb_unbind_interface] debugging
usb usb3: [hub_disconnect] debugging
usb usb3-port2: [usb_hub_remove_port_device] debugging: port 1
usb usb3-port1: [usb_hub_remove_port_device] debugging: port 0
usb usb3: [usb_disconnect] done with the device
xhci_hcd 0000:3c:00.0: Host halt failed, -19
xhci_hcd 0000:3c:00.0: Host not accessible, reset failed.
xhci_hcd 0000:3c:00.0: USB bus 3 deregistered
As you can see typec port connectors are unbound during usb4 usb_disconnect(),
so when usb 3-1 tries to typec_deattach() after it disconnected all its children
the connector is not there anymore and type_partner_deattach() is not invoked.
Maybe usb 3-1 should be disconnected as a child of usb3, but even in that case
we would still end up in the same situation because it's usb4 disconnection that
is doing the unbinding.
Since there's no dependency between usb3 and usb4 they can be
disconnected in any order and it's just a matter of luck as it is right now.
Hope things make a little bit more sense now.
Marco Tormento
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] USB: hub: Move typec deattach before children disconnections in usb_disconnect()
2025-07-21 23:18 ` Marco Tormento
@ 2025-07-22 3:05 ` Alan Stern
2025-07-22 21:59 ` Marco Tormento
0 siblings, 1 reply; 5+ messages in thread
From: Alan Stern @ 2025-07-22 3:05 UTC (permalink / raw)
To: Marco Tormento; +Cc: gregkh, linux-usb, linux-kernel
On Tue, Jul 22, 2025 at 01:18:25AM +0200, Marco Tormento wrote:
> On Mon, 21 Jul 2025 at 03:21, Alan Stern <stern@rowland.harvard.edu> wrote:
> > I'm not a typec expert; in fact I know practically nothing about it.
> > Nevertheless, this sounds strange. The recursive usb_disconnect() calls
> > should affect the connectors to the monitor's children and the monitor's
> > own ports, not the connector or port on the monitor's parent hub.
>
> What you wrote makes total sense, let me add some detail though.
> When I plug the monitor to the thunderbolt port, 3 usb hubs pop up, but only 2
> are backed by XHCI Host Controllers: usb3 and usb4.
I don't know what you mean when you say "backed by". usb3 and usb4 are
the two root hubs of the xHCI host controller. usb3-1 is an internal
hub (presumably built into the monitor) attached to the usb3 root hub.
> usb 3-1 instead is using usb3 and it has 3 devices connected to it: mouse,
> keyboard and a mysterious billboard device:
>
> xhci_hcd 0000:3c:00.0: xHCI Host Controller
> xhci_hcd 0000:3c:00.0: new USB bus registered, assigned bus number 3
> xhci_hcd 0000:3c:00.0: hcc params 0x200077c1 hci version 0x110 quirks
> 0x0000000200009810
> xhci_hcd 0000:3c:00.0: xHCI Host Controller
> xhci_hcd 0000:3c:00.0: new USB bus registered, assigned bus number 4
> xhci_hcd 0000:3c:00.0: Host supports USB 3.1 Enhanced SuperSpeed
> usb usb3: New USB device found, idVendor=1d6b, idProduct=0002, bcdDevice= 6.15
> usb usb3: New USB device strings: Mfr=3, Product=2, SerialNumber=1
> usb usb3: Product: xHCI Host Controller
> usb usb3: Manufacturer: Linux 6.15.7-arch1-1-mentor xhci-hcd
> usb usb3: SerialNumber: 0000:3c:00.0
> hub 3-0:1.0: USB hub found
> hub 3-0:1.0: 2 ports detected
> usb usb4: New USB device found, idVendor=1d6b, idProduct=0003, bcdDevice= 6.15
> usb usb4: New USB device strings: Mfr=3, Product=2, SerialNumber=1
> usb usb4: Product: xHCI Host Controller
> usb usb4: Manufacturer: Linux 6.15.7-arch1-1-mentor xhci-hcd
> usb usb4: SerialNumber: 0000:3c:00.0
> hub 4-0:1.0: USB hub found
> hub 4-0:1.0: 2 ports detected
> typec port0: bound usb3-port1 (ops connector_ops [usbcore])
> typec port0: bound usb4-port1 (ops connector_ops [usbcore])
> typec port1: bound usb3-port2 (ops connector_ops [usbcore])
> typec port1: bound usb4-port2 (ops connector_ops [usbcore])
Why are the usb3 port connectors getting bound at this point rather than
when usb3 was registered?
> usb 3-1: new high-speed USB device number 2 using xhci_hcd
> usb 3-1: New USB device found, idVendor=0bda, idProduct=5411, bcdDevice= 1.36
> usb 3-1: New USB device strings: Mfr=1, Product=2, SerialNumber=0
> usb 3-1: Product: 4-Port USB 2.0 Hub
> usb 3-1: Manufacturer: Generic
> hub 3-1:1.0: USB hub found
> hub 3-1:1.0: 3 ports detected
> usb 3-1.3: new full-speed USB device number 3 using xhci_hcd
> usb 3-1.3: not running at top speed; connect to a high speed hub
I'm actually puzzled by this line. Apparently the mysterious Realtek
BillBoard Device claims that it is capable of connecting at high speed,
but even though it was attached to a high-speed hub it only connected at
full speed. Things like this have shown up in other users' reports but
I never bothered to point out the inconsistencies to anyone.
Either the device is lying about its capabilities or else it is
malfunctioning. Either way, I guess there isn't anything the kernel can
do about it.
> usb 3-1.3: New USB device found, idVendor=0bda, idProduct=5400, bcdDevice= 1.07
> usb 3-1.3: New USB device strings: Mfr=17, Product=18, SerialNumber=19
> usb 3-1.3: Product: BillBoard Device
> usb 3-1.3: Manufacturer: Realtek
> usb 3-1.3: SerialNumber: 123456789ABCDEFGH
> usb 3-1.2: new full-speed USB device number 4 using xhci_hcd
> usb 3-1.2: New USB device found, idVendor=0951, idProduct=16e6, bcdDevice=21.08
> usb 3-1.2: New USB device strings: Mfr=1, Product=2, SerialNumber=0
> usb 3-1.2: Product: HyperX Alloy Origins Core
> usb 3-1.2: Manufacturer: Kingston
> usb 3-1.1: new full-speed USB device number 5 using xhci_hcd
> usb 3-1.1: New USB device found, idVendor=1a7c, idProduct=0197, bcdDevice= 1.06
> usb 3-1.1: New USB device strings: Mfr=1, Product=3, SerialNumber=0
> usb 3-1.1: Product: Evoluent VerticalMouse D
> usb 3-1.1: Manufacturer: Kingsis Peripherals
>
> When I unplug the monitor though, usb 3-1 is not processed as part of
> hub_disconnect_children() of usb3 hub, as I would expect.
Are you sure about this? How did you reach that conclusion?
> It is processed on its own (added some debugging log to some functions):
>
> usb 3-1: [usb_disconnect] debugging
> usb 3-1: [usb_disconnect] USB disconnect, device number 2
> usb 3-1: [hub_disconnect_children] debugging hub_disconnect_children()
> usb 3-1: [hub_disconnect_children] disconnecting child 0
> usb 3-1.1: [usb_disconnect] debugging
> usb 3-1.1: [usb_disconnect] USB disconnect, device number 5
> usb 3-1.1: [hub_disconnect_children] debugging hub_disconnect_children()
> usb 3-1.1: [usb_disconnect] unregistering device
> usbhid 3-1.1:1.0: [usb_unbind_interface] debugging
> usb 3-1.1: [usb_disconnect] parent found
> usb 3-1.1: [usb_disconnect] removing port 3-1-port1 from hub 3-1:1.0: 0
> usb 3-1.1: [usb_disconnect] done with the device
> usb 3-1: [hub_disconnect_children] disconnecting child 1
> usb 3-1.2: [usb_disconnect] debugging
> usb 3-1.2: [usb_disconnect] USB disconnect, device number 4
> usb 3-1.2: [hub_disconnect_children] debugging hub_disconnect_children()
> usb 3-1.2: [usb_disconnect] unregistering device
> usbhid 3-1.2:1.0: [usb_unbind_interface] debugging
> xhci_hcd 0000:3c:00.0: remove, state 4
> usb usb4: [usb_disconnect] debugging
> usb usb4: [usb_disconnect] USB disconnect, device number 1
> usb usb4: [hub_disconnect_children] debugging hub_disconnect_children()
> usb usb4: [usb_disconnect] unregistering device
> hub 4-0:1.0: [usb_unbind_interface] debugging
> usb usb4: [hub_disconnect] debugging
> usb usb4-port2: [usb_hub_remove_port_device] debugging: port 1
> typec port1: [connector_unbind] unbinding connector from usb4-port2
> typec port1: [connector_unbind] unbinding connector from usb3-port2
> usb usb4-port1: [usb_hub_remove_port_device] debugging: port 0
> typec port0: [connector_unbind] unbinding connector from usb4-port1
> typec port0: [connector_unbind] unbinding connector from usb3-port1
> usb usb4: [usb_disconnect] done with the device
> xhci_hcd 0000:3c:00.0: USB bus 4 deregistered
> xhci_hcd 0000:3c:00.0: xHCI host controller not responding, assume dead
> xhci_hcd 0000:3c:00.0: remove, state 1
> usb usb3: [usb_disconnect] debugging
> usb usb3: [usb_disconnect] USB disconnect, device number 1
> typec port1-partner: [typec_unregister_partner] debugging
> typec port1-partner: [typec_unregister_partner] unregistering from port: port1
> usbhid 3-1.2:1.1: [usb_unbind_interface] debugging
> usbhid 3-1.2:1.2: [usb_unbind_interface] debugging
> usb 3-1.2: [usb_disconnect] parent found
> usb 3-1.2: [usb_disconnect] removing port 3-1-port2 from hub 3-1:1.0: 1
> usb 3-1.2: [usb_disconnect] done with the device
> usb 3-1: [hub_disconnect_children] disconnecting child 2
> usb 3-1.3: [usb_disconnect] debugging
> usb 3-1.3: [usb_disconnect] USB disconnect, device number 3
> usb 3-1.3: [hub_disconnect_children] debugging hub_disconnect_children()
> usb 3-1.3: [usb_disconnect] unregistering device
> usb 3-1.3: [usb_disconnect] parent found
> usb 3-1.3: [usb_disconnect] removing port 3-1-port3 from hub 3-1:1.0: 2
> usb 3-1.3: [usb_disconnect] done with the device
> usb 3-1: [usb_disconnect] unregistering device
> hub 3-1:1.0: [usb_unbind_interface] debugging
> usb 3-1: [hub_disconnect] debugging
> usb 3-1-port3: [usb_hub_remove_port_device] debugging: port 2
> usb 3-1-port2: [usb_hub_remove_port_device] debugging: port 1
> usb 3-1-port1: [usb_hub_remove_port_device] debugging: port 0
> usb 3-1: [usb_disconnect] parent found
> usb 3-1: [usb_disconnect] removing port usb3-port1 from hub 3-0:1.0: 0
> usb 3-1: [usb_disconnect] done with the device
> usb usb3: [hub_disconnect_children] debugging hub_disconnect_children()
> usb usb3: [usb_disconnect] unregistering device
> hub 3-0:1.0: [usb_unbind_interface] debugging
> usb usb3: [hub_disconnect] debugging
> usb usb3-port2: [usb_hub_remove_port_device] debugging: port 1
> usb usb3-port1: [usb_hub_remove_port_device] debugging: port 0
> usb usb3: [usb_disconnect] done with the device
> xhci_hcd 0000:3c:00.0: Host halt failed, -19
> xhci_hcd 0000:3c:00.0: Host not accessible, reset failed.
> xhci_hcd 0000:3c:00.0: USB bus 3 deregistered
>
> As you can see typec port connectors are unbound during usb4 usb_disconnect(),
> so when usb 3-1 tries to typec_deattach() after it disconnected all its children
> the connector is not there anymore and type_partner_deattach() is not invoked.
Yes. I would guess this happens because of the way the port connectors
were registered in the first place (see my question above).
> Maybe usb 3-1 should be disconnected as a child of usb3, but even in that case
> we would still end up in the same situation because it's usb4 disconnection that
> is doing the unbinding.
> Since there's no dependency between usb3 and usb4 they can be
> disconnected in any order and it's just a matter of luck as it is right now.
That last sentence is totally wrong. usb3 and usb4 are closely
connected, since they represent logical components of the same xHCI
controller, and they will always be unregistered in the same order.
usb3 is the USB-2 root hub (the one connected to the physical wires
carrying the USB-2 low/full/high-speed signals), and usb4 is the USB-3
root hub (the one connected to the physical wires carrying the USB-3
SuperSpeed and SuperSpeedPlus signals).
> Hope things make a little bit more sense now.
This still leaves the puzzle about why the typec things are handled this
way. In particular, if connector_unbind undoes the binding of the typec
ports that happened when usb4 was registered, then what action is
typec_unregister_partner supposed to undo, and when was it supposed to
happen? As a general rule, disconnection and unregistration actions
take place in the reverse order of the corresponding connection and
registration actions.
Questions like this are best directed at the maintainers of the
USB-4/Thunderbolt and typec subsystems.
Alan Stern
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] USB: hub: Move typec deattach before children disconnections in usb_disconnect()
2025-07-22 3:05 ` Alan Stern
@ 2025-07-22 21:59 ` Marco Tormento
0 siblings, 0 replies; 5+ messages in thread
From: Marco Tormento @ 2025-07-22 21:59 UTC (permalink / raw)
To: Alan Stern; +Cc: gregkh, linux-usb, linux-kernel
On Tue, 22 Jul 2025 at 05:05, Alan Stern <stern@rowland.harvard.edu> wrote:
>
> On Tue, Jul 22, 2025 at 01:18:25AM +0200, Marco Tormento wrote:
> > On Mon, 21 Jul 2025 at 03:21, Alan Stern <stern@rowland.harvard.edu> wrote:
> > > I'm not a typec expert; in fact I know practically nothing about it.
> > > Nevertheless, this sounds strange. The recursive usb_disconnect() calls
> > > should affect the connectors to the monitor's children and the monitor's
> > > own ports, not the connector or port on the monitor's parent hub.
> >
> > What you wrote makes total sense, let me add some detail though.
> > When I plug the monitor to the thunderbolt port, 3 usb hubs pop up, but only 2
> > are backed by XHCI Host Controllers: usb3 and usb4.
>
> I don't know what you mean when you say "backed by". usb3 and usb4 are
> the two root hubs of the xHCI host controller. usb3-1 is an internal
> hub (presumably built into the monitor) attached to the usb3 root hub.
Mine was a failed attempt at expressing what you wrote :-).
My understanding is that usb3 and usb4 root hubs are somehow part of the
thunderbolt controller, because they are not brought up if I plug the monitor
into the other typec port.
> > typec port0: bound usb3-port1 (ops connector_ops [usbcore])
> > typec port0: bound usb4-port1 (ops connector_ops [usbcore])
> > typec port1: bound usb3-port2 (ops connector_ops [usbcore])
> > typec port1: bound usb4-port2 (ops connector_ops [usbcore])
>
> Why are the usb3 port connectors getting bound at this point rather than
> when usb3 was registered?
Not sure, but maybe this is a consequence of the fact usb4 is on a shared hcd of
usb3 pci device? See the stack traces below.
> > When I unplug the monitor though, usb 3-1 is not processed as part of
> > hub_disconnect_children() of usb3 hub, as I would expect.
>
> Are you sure about this? How did you reach that conclusion?
I checked the stack traces of usb_disconnect().
usb 3-1 is triggered by hub_event():
usb 3-1: [usb_disconnect] debugging()
...cut...
Workqueue: usb_hub_wq hub_event [usbcore]
Call Trace:
<TASK>
dump_stack_lvl+0x5d/0x80
usb_disconnect+0x38/0x2f4 [usbcore d46dddc5902949f0a284aff7ecf8d6e861774323]
hub_event.cold+0x73e/0xc50 [usbcore d46dddc5902949f0a284aff7ecf8d6e861774323]
...cut
usb4 is not:
usb usb4: [usb_disconnect] debugging()
...cut...
Workqueue: kacpi_hotplug acpi_hotplug_work_fn
Call Trace:
<TASK>
dump_stack_lvl+0x5d/0x80
usb_disconnect+0x38/0x2f4 [usbcore d46dddc5902949f0a284aff7ecf8d6e861774323]
usb_remove_hcd.cold+0xc6/0x1bb [usbcore
d46dddc5902949f0a284aff7ecf8d6e861774323]
xhci_pci_remove+0x6a/0xd0 [xhci_pci 89d1427fd34e7648221e69cd90b20ad6e1fd2fd6]
pci_device_remove+0x47/0xc0
...cut
usb3 is a little bit different compared to usb4, it has a call to
usb_hcd_pci_remove():
usb usb3: [usb_disconnect] debugging()
...cut...
Workqueue: kacpi_hotplug acpi_hotplug_work_fn
Call Trace:
<TASK>
dump_stack_lvl+0x5d/0x80
usb_disconnect+0x38/0x2f4 [usbcore d46dddc5902949f0a284aff7ecf8d6e861774323]
usb_remove_hcd.cold+0xc6/0x1bb [usbcore
d46dddc5902949f0a284aff7ecf8d6e861774323]
usb_hcd_pci_remove+0x7a/0x110 [usbcore
d46dddc5902949f0a284aff7ecf8d6e861774323]
xhci_pci_remove+0x89/0xd0 [xhci_pci 89d1427fd34e7648221e69cd90b20ad6e1fd2fd6]
pci_device_remove+0x47/0xc0
...cut
There's a branch in xhci_pci_remove() on xhci->shared_hcd being valid.
So it seems:
- xhci_pci_remove() is called
- xhci->shared_hcd is valid so usb_remove_hcd() is called which triggers usb4
usb_disconnect()
- usb_hcd_pci_remove() is then called which does a lot of stuff + calls
usb_remove_hcd() which triggers usb3 usb_disconnect()
No idea what this shared hcd implies, by the way.
I also got this stack trace of the port unbinding, which happens
in typec_aggregate_unbind() in the usb4 usb_disconnect() trace:
usb usb4: [hub_disconnect] debugging
usb usb4-port2: [usb_hub_remove_port_device] debugging: port 1
...cut...
Workqueue: kacpi_hotplug acpi_hotplug_work_fn
Call Trace:
<TASK>
dump_stack_lvl+0x5d/0x80
typec_aggregate_unbind+0x12/0x30 [typec
6de6f75aa8c9f6a0a8c73a05ed9c668a2b7634cf]
component_del+0xb8/0x150
usb_hub_remove_port_device+0xf8/0x115 [usbcore
d46dddc5902949f0a284aff7ecf8d6e861774323]
hub_disconnect+0x95/0x160 [usbcore d46dddc5902949f0a284aff7ecf8d6e861774323]
usb_unbind_interface+0xb6/0x1ef [usbcore
d46dddc5902949f0a284aff7ecf8d6e861774323]
? kernfs_remove_by_name_ns+0xbe/0xe0
device_release_driver_internal+0x19e/0x200
bus_remove_device+0xc2/0x130
device_del+0x160/0x3d0
? kobject_put+0xa2/0x200
usb_disable_device+0xf4/0x220 [usbcore
d46dddc5902949f0a284aff7ecf8d6e861774323]
usb_disconnect+0x117/0x2f4 [usbcore d46dddc5902949f0a284aff7ecf8d6e861774323]
usb_remove_hcd.cold+0xc6/0x1bb [usbcore
d46dddc5902949f0a284aff7ecf8d6e861774323]
xhci_pci_remove+0x6a/0xd0 [xhci_pci 89d1427fd34e7648221e69cd90b20ad6e1fd2fd6]
pci_device_remove+0x47/0xc0
...cut...
</TASK>
typec port1: [connector_unbind] unbinding connector from usb4-port2
typec port1: [connector_unbind] unbinding connector from usb3-port2
> > Maybe usb 3-1 should be disconnected as a child of usb3, but even in that case
> > we would still end up in the same situation because it's usb4 disconnection that
> > is doing the unbinding.
> > Since there's no dependency between usb3 and usb4 they can be
> > disconnected in any order and it's just a matter of luck as it is right now.
>
> That last sentence is totally wrong. usb3 and usb4 are closely
> connected, since they represent logical components of the same xHCI
> controller, and they will always be unregistered in the same order.
> usb3 is the USB-2 root hub (the one connected to the physical wires
> carrying the USB-2 low/full/high-speed signals), and usb4 is the USB-3
> root hub (the one connected to the physical wires carrying the USB-3
> SuperSpeed and SuperSpeedPlus signals).
That makes a lot of sense.
Unfortunately I realized this could be the order half an hour after having
pressed "send"...
Thank you for the explanation!
> This still leaves the puzzle about why the typec things are handled this
> way. In particular, if connector_unbind undoes the binding of the typec
> ports that happened when usb4 was registered, then what action is
> typec_unregister_partner supposed to undo, and when was it supposed to
> happen? As a general rule, disconnection and unregistration actions
> take place in the reverse order of the corresponding connection and
> registration actions.
>
> Questions like this are best directed at the maintainers of the
> USB-4/Thunderbolt and typec subsystems.
Should I just cc them here or should I open a new thread with them?
Marco Tormento
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-07-22 21:59 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-20 21:08 [PATCH] USB: hub: Move typec deattach before children disconnections in usb_disconnect() Marco Tormento
2025-07-21 1:21 ` Alan Stern
2025-07-21 23:18 ` Marco Tormento
2025-07-22 3:05 ` Alan Stern
2025-07-22 21:59 ` Marco Tormento
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).