On 3/4/2023 12:05 AM, Alan Stern wrote:
On Fri, Mar 03, 2023 at 05:28:38PM +0800, Linyu Yuan wrote:
When start probe hub, during INIT, INTT2, INIT3 stage, when link state
change to inactive, currently it will reset the device, maybe it will
trigger warning in usb_submit_urb() due to urb->hcpriv is still active.
You need to explain this in much greater detail.
What will reset the device?
What is the code path for this reset?
will share more code path.
Why will urb->hcpriv still be active?
still can't explain, that's why add patch#1 to get more urb infol
Add a flag name init_stage to avoid reset the device.
Why do you want to avoid resetting the device?
at INIT stage, external hub still under enumeration process, i think
there is no need to reset.
Doesn't the reset code already include a check for whether the device is
disconnected?
the problem is port is inactive state, but device still in software
connect state,
there is no disconnect check in reset code.
Signed-off-by: Linyu Yuan <quic_linyyuan@xxxxxxxxxxx>
---
drivers/usb/core/hub.c | 20 +++++++++++++++++++-
drivers/usb/core/hub.h | 1 +
2 files changed, 20 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 97a0f8f..3cb1137 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -1290,6 +1290,7 @@ static void hub_activate(struct usb_hub *hub, enum hub_activation_type type)
if (type == HUB_INIT2 || type == HUB_INIT3) {
/* Allow autosuspend if it was suppressed */
disconnected:
+ hub->init_stage = 0;
usb_autopm_put_interface_async(to_usb_interface(hub->intfdev));
device_unlock(&hdev->dev);
}
@@ -1872,6 +1873,7 @@ static int hub_probe(struct usb_interface *intf, const struct usb_device_id *id)
kref_init(&hub->kref);
hub->intfdev = &intf->dev;
hub->hdev = hdev;
+ hub->init_stage = 1;
INIT_DELAYED_WORK(&hub->leds, led_work);
INIT_DELAYED_WORK(&hub->init_work, NULL);
INIT_WORK(&hub->events, hub_event);
@@ -5587,6 +5589,21 @@ static void port_over_current_notify(struct usb_port *port_dev)
kfree(port_dev_path);
}
+static bool port_child_avoid_reset(struct usb_device *udev)
+{
+ struct usb_hub *hub;
+
+ if (udev->descriptor.bDeviceClass == USB_CLASS_HUB &&
+ udev->state == USB_STATE_CONFIGURED) {
+ hub = usb_get_intfdata(udev->actconfig->interface[0]);
+
+ if (hub && hub->init_stage)
+ return true;
+ }
+
+ return false;
+}
+
static void port_event(struct usb_hub *hub, int port1)
__must_hold(&port_dev->status_lock)
{
@@ -5699,7 +5716,8 @@ static void port_event(struct usb_hub *hub, int port1)
dev_dbg(&port_dev->dev, "do warm reset, full device\n");
usb_unlock_port(port_dev);
usb_lock_device(udev);
- usb_reset_device(udev);
+ if (!port_child_avoid_reset(udev))
+ usb_reset_device(udev);
usb_unlock_device(udev);
Doesn't usb_lock_device() already prevent this code from running during
the INIT, INIT2, and INIT3 stages of hub preparation?
as it use some delay worker to complete the INIT stage, as i know it
will not lock device
when worker is not start.
do you have any better suggestion about this point ?
Alan Stern
