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?
Why will urb->hcpriv still be active?
> Add a flag name init_stage to avoid reset the device.
Why do you want to avoid resetting the device?
Doesn't the reset code already include a check for whether the device is
disconnected?
>
> 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?
Alan Stern
