Re: [PATCH v3 2/2] usb: hub: Retry SuperSpeed for a device stuck on its USB 2.0 peer

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] usb: hub: hub_peer_reset_work() ignores quiescing state during
  teardown
--

commit 292ab4e7add6e385ccf460779fb12521a3d0b6e7
Author: Arun Iyengar <aiyengar1990@gmail.com>

usb: hub: Retry SuperSpeed for a device stuck on its USB 2.0 peer

This commit schedules work when a SuperSpeed root hub initializes to reset
any SuperSpeed-capable device connected at high speed to a peer USB 2.0
root hub port on another host controller so it can reconnect at
SuperSpeed.

> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index 199d3f535a891..6bbe5fe2c0b52 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -1076,6 +1076,105 @@ int usb_remove_device(struct usb_device *udev)
>  	return 0;
>  }
>  
> +/*
> + * A device present at boot may have connected at high speed to the USB 2.0
> + * peer before this SuperSpeed root hub was up; a USB 2.0 reset makes it retry.
> + */
> +static void hub_reset_peer_child(struct usb_device *peer_hdev, int peer_port1)
> +{
> +	struct usb_hub *peer_hub;
> +	struct usb_device *udev;
> +
> +	/* Wait for an enumeration in progress on the peer port to finish */
> +	usb_lock_device(peer_hdev);
> +	peer_hub = usb_hub_to_struct_hub(peer_hdev);
> +	if (!peer_hub || peer_port1 > peer_hdev->maxchild)
> +		goto out;
> +
> +	udev = peer_hub->ports[peer_port1 - 1]->child;
> +	if (!udev || udev->speed != USB_SPEED_HIGH)
> +		goto out;
> +
> +	/*
> +	 * The USB 2.0 half of a USB 3 hub stays at high speed next to its
> +	 * SuperSpeed half, and resetting it would disconnect its children.
> +	 */
> +	if (udev->descriptor.bDeviceClass == USB_CLASS_HUB)
> +		goto out;
> +
> +	usb_lock_device(udev);
> +	if (udev->state != USB_STATE_NOTATTACHED &&
> +	    udev->bos && udev->bos->ss_cap) {
> +		dev_info(&udev->dev,
> +			 "SuperSpeed peer port available, resetting\n");
> +		usb_reset_device(udev);
> +	}
> +	usb_unlock_device(udev);
> +out:
> +	usb_unlock_device(peer_hdev);
> +}
> +
> +static void hub_peer_reset_work(struct work_struct *ws)
> +{
> +	struct usb_hub *hub = container_of(ws, struct usb_hub, peer_reset_work);
> +	struct usb_device *hdev = hub->hdev;
> +	int port1;
> +
> +	for (port1 = 1; ; port1++) {
> +		struct usb_device *peer_hdev = NULL;
> +		struct usb_port *port_dev;
> +		int peer_port1 = 0;
> +
> +		mutex_lock(&usb_port_peer_mutex);
> +		if (port1 > hdev->maxchild) {
> +			mutex_unlock(&usb_port_peer_mutex);
> +			break;
> +		}
> +		port_dev = hub->ports[port1 - 1];
> +		if (port_dev->connector_peer && port_dev->peer) {
> +			peer_hdev = to_usb_device(port_dev->peer->dev.parent->parent);
> +			peer_port1 = port_dev->peer->portnum;
> +			/* Only root hub ports of another host controller */
> +			if (peer_hdev->parent ||
> +			    peer_hdev->bus->controller == hdev->bus->controller)
> +				peer_hdev = NULL;
> +			else
> +				usb_get_dev(peer_hdev);
> +		}
> +		mutex_unlock(&usb_port_peer_mutex);
> +
> +		if (peer_hdev) {
> +			hub_reset_peer_child(peer_hdev, peer_port1);
> +			usb_put_dev(peer_hdev);
> +		}
> +	}

[Severity: Low]
Should hub_peer_reset_work() and hub_reset_peer_child() check
hub->quiescing?

hub_disconnect() only zeroes hdev->maxchild under usb_port_peer_mutex
after hub_quiesce() has finished waiting in
cancel_work_sync(&hub->peer_reset_work).  If the SuperSpeed root hub is
quiesced or disconnected while hub_peer_reset_work() is running (for
instance, while hub_reset_peer_child() is blocked on
usb_lock_device(peer_hdev) waiting for USB 2.0 enumeration to finish), the
work item will still reset the USB 2.0 peer devices across all remaining
ports to migrate them toward a SuperSpeed root hub that is being torn
down.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113-usb-port-pairing-v3-0-d6c6edb4341c@gmail.com?part=2