[PATCH 09/13] usb: hub: detect disconnected devices

[Sascha Hauer <s.hauer@pengutronix.de>]

barebox never noticed that a USB device was unplugged. Running "usb"
again after removing a stick kept reporting it, /dev/disk0 and its
partitions stayed around and reads went to a device that was not there
anymore.

The machinery to remove a device was all there, it was just never
reached. usb_scan_port() bailed out before usb_hub_port_connect_change()
whenever the port reported no connection:

	if (!(portchange & USB_PORT_STAT_C_CONNECTION) ||
	    !(portstatus & USB_PORT_STAT_CONNECTION))

A port whose device has been unplugged has the change bit set - the hub
latches it - and the connection bit clear, which is exactly what the
second half of the condition filters out. The disconnect branch in
usb_hub_port_connect_change() could therefore never run: getting there
required the connection bit to be set, so its test for the very same bit
being clear was always false.

Let a connection change through when we have a device on that port, no
matter what the connection bit says, and decide in
usb_hub_port_connect_change() what it means: the device we knew about is
gone in either case, so remove it and enumerate a new one only if the
port reports a connection again. That also covers a device being swapped
for another one between two scans, which used to end in

	ERROR: register_device: already registered usb1-0

with the new device unusable and the old one orphaned from the port
array, so it could never be removed at all.

The previous attempt to catch that case tested for USB_PORT_STAT_ENABLE
being clear. That cannot work on xHCI: a SuperSpeed root port is enabled
by the controller during link training, so it is already enabled the
first time we look at it. Drop the test.

Ports without a device keep the old behaviour and stay in the scan list
until the connect timeout expires, so slow devices still get their
second before we give up on them.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Assisted-by: Claude:claude-opus-5
---
 drivers/usb/core/hub.c | 35 +++++++++++++++++++++++++----------
 1 file changed, 25 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index b56c658f91..18de8badf5 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -292,20 +292,29 @@ static void usb_hub_port_connect_change(struct usb_device *dev, int port,
 	/* Clear the connection change status */
 	usb_clear_port_feature(dev, port + 1, USB_PORT_FEAT_C_CONNECTION);
 
-	/* Disconnect any existing devices under this port */
-	if (dev->children[port] && !(portstatus & USB_PORT_STAT_CONNECTION)) {
+	/*
+	 * The connection changed, so whatever we knew about this port is
+	 * gone. That covers a plain unplug as well as a device that has been
+	 * replaced while we were not looking: the port reports a connection
+	 * again in that case, but not the one we have a device for.
+	 *
+	 * Note we must not test for USB_PORT_STAT_ENABLE here to tell the
+	 * two apart. An xHCI root hub enables a SuperSpeed port on its own
+	 * during link training, so the port is already enabled the first
+	 * time we see it.
+	 */
+	if (dev->children[port]) {
 		dev_dbg(&dev->dev, "port%d: disconnect detected\n", port + 1);
 		usb_remove_device(dev->children[port]);
+		dev->children[port] = NULL;
 
 		if (!dev->parent && dev->host->usbphy)
 			usb_phy_notify_disconnect(dev->host->usbphy, dev->speed);
-
-		return;
 	}
 
-	/* Remove disabled but connected devices */
-	if (dev->children[port] && !(portstatus & USB_PORT_STAT_ENABLE))
-		usb_remove_device(dev->children[port]);
+	/* Nothing connected anymore, we are done */
+	if (!(portstatus & USB_PORT_STAT_CONNECTION))
+		return;
 
 	/* Allocate a new device struct for the port */
 	usb = usb_alloc_new_device();
@@ -380,8 +389,15 @@ static void usb_scan_port(struct usb_device_scan *usb_scan)
 	dev_dbg(&dev->dev, "port%d: Status 0x%04x Change 0x%04x\n",
 			port + 1, portstatus, portchange);
 
+	/*
+	 * A connection change on a port we have a device for has to be
+	 * handled even when the port reports no connection: that is how a
+	 * disconnect looks. An empty port on the other hand keeps its change
+	 * bit set from the power-on, so leave it in the scan list until the
+	 * connect timeout expires to give slow devices time to show up.
+	 */
 	if (!(portchange & USB_PORT_STAT_C_CONNECTION) ||
-	    !(portstatus & USB_PORT_STAT_CONNECTION)) {
+	    (!(portstatus & USB_PORT_STAT_CONNECTION) && !dev->children[port])) {
 		if (get_time_ns() >= hub->connect_timeout) {
 			dev_dbg(&dev->dev, "port%d: timeout\n", port + 1);
 			/* Remove this device from scanning list */
@@ -402,8 +418,7 @@ static void usb_scan_port(struct usb_device_scan *usb_scan)
 		usb_clear_port_feature(dev, port + 1, USB_PORT_FEAT_C_BH_PORT_RESET);
 	}
 
-	/* A new USB device is ready at this point */
-	dev_dbg(&dev->dev, "port%d: USB dev found\n", port + 1);
+	dev_dbg(&dev->dev, "port%d: connection change\n", port + 1);
 
 	usb_hub_port_connect_change(dev, port, portstatus, portchange);
 

-- 
2.47.3