lkml.org 
[lkml]   [2022]   [Aug]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] usb: core: stop USB enumeration if too many retries
    On Sat, Aug 27, 2022 at 01:53:45AM +0800, Ray Chi wrote:
    > On Fri, Aug 26, 2022 at 10:55 PM Alan Stern <stern@rowland.harvard.edu> wrote:
    > >
    > > On Fri, Aug 26, 2022 at 03:58:39PM +0800, Ray Chi wrote:
    > > > If a broken accessory connected to a USB host, usbcore might
    > > > keep doing enumeration retries and it will take a long time to
    > > > cause system unstable.
    > > >
    > > > This patch provides a quirk to specific USB ports of the hub to
    > > > stop USB enumeration if needed.
    > >
    > > Why only to specific ports?
    >
    > The specific port means it is connected to a broken accessory.

    Then this patch should be about connections to broken accessories, not
    too many retries. The quirk should make the hub driver permanently
    ignore the port; whether the power is on or off doesn't matter.

    Also, you might want to check whether the port connection is fixed
    rather than hot-unpluggable. If the broken accessory can be unplugged
    from the port then you don't want to disable the port.

    > > > Signed-off-by: Ray Chi <raychi@google.com>
    > > > ---
    > > > drivers/usb/core/hub.c | 33 +++++++++++++++++++++++++++++++++
    > > > include/linux/usb.h | 3 +++
    > > > 2 files changed, 36 insertions(+)
    > > >
    > > > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
    > > > index 2633acde7ac1..0f4097440ffb 100644
    > > > --- a/drivers/usb/core/hub.c
    > > > +++ b/drivers/usb/core/hub.c
    > > > @@ -3081,6 +3081,28 @@ static int hub_port_reset(struct usb_hub *hub, int port1,
    > > > return status;
    > > > }
    > > >
    > > > +/* Stop enumerate if the port met errors and quirk is set */
    > > > +static bool hub_port_stop_enumerate(struct usb_hub *hub, int port1, int retries)
    > > > +{
    > > > + struct usb_port *port_dev = hub->ports[port1 - 1];
    > > > + struct usb_device *hdev = hub->hdev;
    > > > +
    > > > + if (retries < (PORT_INIT_TRIES - 1) / 2)
    > > > + return false;
    > > > +
    > > > + /*
    > > > + * Some USB hosts can't take a long time to keep doing enumeration
    > > > + * retry. After doing half of the retries, we would turn off the port
    > > > + * power to stop enumeration if the quirk is set.
    > > > + */
    > > > + if (port_dev->quirks & USB_PORT_QUIRK_STOP_ENUM) {
    > > > + usb_hub_set_port_power(hdev, hub, port1, false);
    > >
    > > Why turn the port power off? Aren't there better ways to stop the
    > > enumeration attempts? When will the power ever get turned back on?
    > >
    >
    > Because the broken accessory is still connected to the port. Even if we stop
    > the enumeration attempts, the port change event will issue a new port
    > initialization.

    If you know the accessory is broken then there's no reason to pay
    attention to the port change events in the first place. You can simply
    make port_event() avoid calling hub_port_connect_change() if the quirk
    is set.

    Alan Stern

    > The implementation is used for dual-role devices, the port power could
    > be turned on
    > when the host mode restarts again.
    >
    > > Why not use the initial_descriptor_timeout module parameter for this
    > > purpose? That's the sort of thing it was meant for.
    > >
    >
    > As I mentioned above, the usbcore driver will keep doing enumeration attempts if
    > the broken accessory is still connected. It never stops. This is why I
    > want to turn
    > off the port to stop enumeration.
    >
    > > Alan Stern
    >
    > Thanks,
    > Ray

    \
     
     \ /
      Last update: 2022-08-26 21:04    [W:3.053 / U:0.016 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site