| Message ID | 20211210111653.1378381-1-mathias.nyman@linux.intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | usb: hub: avoid warm port reset during USB3 disconnect | expand |
Hi Mathias, I have encountered the problem that my device (zynq ultrascale+ platform) ends up performing warm resets when disconnecting USB3 devices and this patch helped with that. Nevertheless a much higher value for the define DETECT_DISCONNECT_TRIES was needed to detect the disconnect status early: [ 211.847722] xhci-hcd xhci-hcd.0.auto: Port change event, 2-1, id 2, portsc: 0x4012c1 [ 211.847739] xhci-hcd xhci-hcd.0.auto: handle_port_status: starting port polling. [ 211.847767] hub 2-0:1.0: state 7 ports 1 chg 0000 evt 0002 [ 211.847786] xhci-hcd xhci-hcd.0.auto: Get port status 2-1 read: 0x4012c1, return 0x4002c1 [ 211.847805] usb usb2-port1: link state change [ 211.847817] xhci-hcd xhci-hcd.0.auto: clear port1 link state change, portsc: 0x12c1 [ 211.872383] xhci-hcd xhci-hcd.0.auto: Get port status 2-1 read: 0x12c1, return 0x2c1 [ 211.872415] usb usb2-port1: Wait for inactive link disconnect detect [ 211.900342] xhci-hcd xhci-hcd.0.auto: Get port status 2-1 read: 0x12c1, return 0x2c1 [ 211.900372] usb usb2-port1: Wait for inactive link disconnect detect [ 211.928342] xhci-hcd xhci-hcd.0.auto: Get port status 2-1 read: 0x12c1, return 0x2c1 [ 211.928374] usb usb2-port1: Wait for inactive link disconnect detect [ 211.948323] xhci-hcd xhci-hcd.0.auto: xhci_hub_status_data: stopping port polling. [ 211.956338] xhci-hcd xhci-hcd.0.auto: Get port status 2-1 read: 0x12c1, return 0x2c1 [ 211.956383] usb usb2-port1: Wait for inactive link disconnect detect [ 211.984345] xhci-hcd xhci-hcd.0.auto: Get port status 2-1 read: 0x12c1, return 0x2c1 [ 211.984378] usb usb2-port1: Wait for inactive link disconnect detect [ 212.012406] xhci-hcd xhci-hcd.0.auto: Get port status 2-1 read: 0x12c1, return 0x2c1 [ 212.012442] usb usb2-port1: Wait for inactive link disconnect detect [ 212.040542] xhci-hcd xhci-hcd.0.auto: Get port status 2-1 read: 0x12c1, return 0x2c1 [ 212.040586] usb usb2-port1: Wait for inactive link disconnect detect [ 212.068343] xhci-hcd xhci-hcd.0.auto: Get port status 2-1 read: 0x12c1, return 0x2c1 [ 212.068391] usb usb2-port1: Wait for inactive link disconnect detect [ 212.096344] xhci-hcd xhci-hcd.0.auto: Get port status 2-1 read: 0x12c1, return 0x2c1 [ 212.096376] usb usb2-port1: Wait for inactive link disconnect detect [ 212.124343] xhci-hcd xhci-hcd.0.auto: Get port status 2-1 read: 0x12c1, return 0x2c1 [ 212.124373] usb usb2-port1: Wait for inactive link disconnect detect [ 212.152352] xhci-hcd xhci-hcd.0.auto: Get port status 2-1 read: 0x12c1, return 0x2c1 [ 212.152391] usb usb2-port1: Wait for inactive link disconnect detect [ 212.180341] xhci-hcd xhci-hcd.0.auto: Get port status 2-1 read: 0x12c1, return 0x2c1 [ 212.180371] usb usb2-port1: Wait for inactive link disconnect detect [ 212.208366] xhci-hcd xhci-hcd.0.auto: Get port status 2-1 read: 0x12c1, return 0x2c1 [ 212.208748] usb usb2-port1: Wait for inactive link disconnect detect [ 212.224375] xhci-hcd xhci-hcd.0.auto: Can't queue urb, port error, link inactive [ 212.236342] xhci-hcd xhci-hcd.0.auto: Get port status 2-1 read: 0x12c1, return 0x2c1 [ 212.236373] usb usb2-port1: Wait for inactive link disconnect detect [ 212.251650] xhci-hcd xhci-hcd.0.auto: Port change event, 2-1, id 2, portsc: 0x202a0 [ 212.251661] xhci-hcd xhci-hcd.0.auto: handle_port_status: starting port polling. [ 212.264374] xhci-hcd xhci-hcd.0.auto: Get port status 2-1 read: 0x202a0, return 0x102a0 [ 212.264400] usb usb2-port1: Wait for inactive link disconnect detect [ 212.264442] xhci-hcd xhci-hcd.0.auto: Can't queue urb, port error, link inactive [ 212.264451] usb 2-1: Disable of device-initiated U1 failed. [ 212.264477] xhci-hcd xhci-hcd.0.auto: Can't queue urb, port error, link inactive [ 212.264484] usb 2-1: Disable of device-initiated U2 failed. [ 212.264493] xhci-hcd xhci-hcd.0.auto: Set up evaluate context for LPM MEL change. [ 212.264504] xhci-hcd xhci-hcd.0.auto: // Ding dong! [ 212.264529] xhci-hcd xhci-hcd.0.auto: Successful evaluate context command [ 212.264545] xhci-hcd xhci-hcd.0.auto: Get port status 2-1 read: 0x202a0, return 0x102a0 [ 212.264570] xhci-hcd xhci-hcd.0.auto: set port reset, actual port 2-1 status = 0x202b0 [ 212.264599] hub 2-0:1.0: state 7 ports 1 chg 0000 evt 0002 [ 212.332342] xhci-hcd xhci-hcd.0.auto: Get port status 2-1 read: 0x2a02a0, return 0x3102a0 [ 212.332390] usb usb2-port1: not reset yet, waiting 60ms [ 212.400355] xhci-hcd xhci-hcd.0.auto: Get port status 2-1 read: 0x2a02a0, return 0x3102a0 [ 212.400395] usb usb2-port1: not reset yet, waiting 200ms [ 212.416387] usb 1-1.2-port2: indicator off status 0 [ 212.416470] usb 1-1.2-port3: indicator green status 0 [ 212.416535] usb 1-1.2-port4: indicator green status 0 [ 212.416617] usb 1-1.2-port5: indicator off status 0 [ 212.416682] usb 1-1.2-port6: indicator off status 0 [ 212.608338] xhci-hcd xhci-hcd.0.auto: Get port status 2-1 read: 0x2a02a0, return 0x3102a0 [ 212.608376] usb usb2-port1: not reset yet, waiting 200ms [ 212.816348] xhci-hcd xhci-hcd.0.auto: Get port status 2-1 read: 0x2a02a0, return 0x3102a0 [ 212.816404] usb usb2-port1: not reset yet, waiting 200ms [ 213.024340] xhci-hcd xhci-hcd.0.auto: Get port status 2-1 read: 0x2a02a0, return 0x3102a0 [ 213.024376] usb usb2-port1: not reset yet, waiting 200ms [ 213.024388] xhci-hcd xhci-hcd.0.auto: clear port1 reset change, portsc: 0xa02a0 [ 213.024411] xhci-hcd xhci-hcd.0.auto: clear port1 warm(BH) reset change, portsc: 0x202a0 [ 213.024434] xhci-hcd xhci-hcd.0.auto: clear port1 link state change, portsc: 0x202a0 [ 213.024456] xhci-hcd xhci-hcd.0.auto: clear port1 connect change, portsc: 0x2a0 [ 213.024479] xhci-hcd xhci-hcd.0.auto: Get port status 2-1 read: 0x2a0, return 0x2a0 [ 213.024506] xhci-hcd xhci-hcd.0.auto: CTRL: TypeReq=0x2303 val=0x5 idx=0x301 len=0 ==> -19 [ 213.024527] usb usb2-port1: logical disconnect [ 213.024537] xhci-hcd xhci-hcd.0.auto: CTRL: TypeReq=0x2303 val=0x5 idx=0x301 len=0 ==> -19 [ 213.024583] xhci-hcd xhci-hcd.0.auto: Get port status 2-1 read: 0x2a0, return 0x2a0 [ 213.024636] usb usb2-port1: status 02a0, change 0000, 5.0 Gb/s [ 213.024645] usb 2-1: USB disconnect, device number 5 In some rare cases even #define DETECT_DISCONNECT_TRIES 20 was not enough. As I want to make sure that the warm resets will never be performed for a disconnected device (as this takes a long time), I want to enhance the value for DETECT_DISCONNECT_TRIES for my application. Are there any issues to expect when setting a too high value for DETECT_DISCONNECT_TRIES? Thanks, Tobias On 10.12.2021 12:16, Mathias Nyman wrote: > During disconnect USB-3 ports often go via SS.Inactive link error state > before the missing terminations are noticed, and link finally goes to > RxDetect state > > Avoid immediately warm-resetting ports in SS.Inactive state. > Let ports settle for a while and re-read the link status a few times 20ms > apart to see if the ports transitions out of SS.Inactive. > > According to USB 3.x spec 7.5.2, a port in SS.Inactive should > automatically check for missing far-end receiver termination every > 12 ms (SSInactiveQuietTimeout) > > The futile multiple warm reset retries of a disconnected device takes > a lot of time, also the resetting of a removed devices has caused cases > where the reset bit got stuck for a long time on xHCI roothub. > This lead to issues in detecting new devices connected to the same port > shortly after. > > Tested-by: Mark Pearson <markpearson@lenovo.com> > Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com> > --- > drivers/usb/core/hub.c | 24 +++++++++++++++++++----- > 1 file changed, 19 insertions(+), 5 deletions(-) > > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c > index 00070a8a6507..e907dfa0ca6d 100644 > --- a/drivers/usb/core/hub.c > +++ b/drivers/usb/core/hub.c > @@ -2777,6 +2777,8 @@ static unsigned hub_is_wusb(struct usb_hub *hub) > #define PORT_INIT_TRIES 4 > #endif /* CONFIG_USB_FEW_INIT_RETRIES */ > > +#define DETECT_DISCONNECT_TRIES 5 > + > #define HUB_ROOT_RESET_TIME 60 /* times are in msec */ > #define HUB_SHORT_RESET_TIME 10 > #define HUB_BH_RESET_TIME 50 > @@ -5543,6 +5545,7 @@ static void port_event(struct usb_hub *hub, int port1) > struct usb_device *udev = port_dev->child; > struct usb_device *hdev = hub->hdev; > u16 portstatus, portchange; > + int i = 0; > > connect_change = test_bit(port1, hub->change_bits); > clear_bit(port1, hub->event_bits); > @@ -5619,17 +5622,27 @@ static void port_event(struct usb_hub *hub, int port1) > connect_change = 1; > > /* > - * Warm reset a USB3 protocol port if it's in > - * SS.Inactive state. > + * Avoid trying to recover a USB3 SS.Inactive port with a warm reset if > + * the device was disconnected. A 12ms disconnect detect timer in > + * SS.Inactive state transitions the port to RxDetect automatically. > + * SS.Inactive link error state is common during device disconnect. > */ > - if (hub_port_warm_reset_required(hub, port1, portstatus)) { > - dev_dbg(&port_dev->dev, "do warm reset\n"); > - if (!udev || !(portstatus & USB_PORT_STAT_CONNECTION) > + while (hub_port_warm_reset_required(hub, port1, portstatus)) { > + if ((i++ < DETECT_DISCONNECT_TRIES) && udev) { > + u16 unused; > + > + msleep(20); > + hub_port_status(hub, port1, &portstatus, &unused); > + dev_dbg(&port_dev->dev, "Wait for inactive link disconnect detect\n"); > + continue; > + } else if (!udev || !(portstatus & USB_PORT_STAT_CONNECTION) > || udev->state == USB_STATE_NOTATTACHED) { > + dev_dbg(&port_dev->dev, "do warm reset, port only\n"); > if (hub_port_reset(hub, port1, NULL, > HUB_BH_RESET_TIME, true) < 0) > hub_port_disable(hub, port1, 1); > } else { > + dev_dbg(&port_dev->dev, "do warm reset, full device\n"); > usb_unlock_port(port_dev); > usb_lock_device(udev); > usb_reset_device(udev); > @@ -5637,6 +5650,7 @@ static void port_event(struct usb_hub *hub, int port1) > usb_lock_port(port_dev); > connect_change = 0; > } > + break; > } > > if (connect_change)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index 00070a8a6507..e907dfa0ca6d 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -2777,6 +2777,8 @@ static unsigned hub_is_wusb(struct usb_hub *hub) #define PORT_INIT_TRIES 4 #endif /* CONFIG_USB_FEW_INIT_RETRIES */ +#define DETECT_DISCONNECT_TRIES 5 + #define HUB_ROOT_RESET_TIME 60 /* times are in msec */ #define HUB_SHORT_RESET_TIME 10 #define HUB_BH_RESET_TIME 50 @@ -5543,6 +5545,7 @@ static void port_event(struct usb_hub *hub, int port1) struct usb_device *udev = port_dev->child; struct usb_device *hdev = hub->hdev; u16 portstatus, portchange; + int i = 0; connect_change = test_bit(port1, hub->change_bits); clear_bit(port1, hub->event_bits); @@ -5619,17 +5622,27 @@ static void port_event(struct usb_hub *hub, int port1) connect_change = 1; /* - * Warm reset a USB3 protocol port if it's in - * SS.Inactive state. + * Avoid trying to recover a USB3 SS.Inactive port with a warm reset if + * the device was disconnected. A 12ms disconnect detect timer in + * SS.Inactive state transitions the port to RxDetect automatically. + * SS.Inactive link error state is common during device disconnect. */ - if (hub_port_warm_reset_required(hub, port1, portstatus)) { - dev_dbg(&port_dev->dev, "do warm reset\n"); - if (!udev || !(portstatus & USB_PORT_STAT_CONNECTION) + while (hub_port_warm_reset_required(hub, port1, portstatus)) { + if ((i++ < DETECT_DISCONNECT_TRIES) && udev) { + u16 unused; + + msleep(20); + hub_port_status(hub, port1, &portstatus, &unused); + dev_dbg(&port_dev->dev, "Wait for inactive link disconnect detect\n"); + continue; + } else if (!udev || !(portstatus & USB_PORT_STAT_CONNECTION) || udev->state == USB_STATE_NOTATTACHED) { + dev_dbg(&port_dev->dev, "do warm reset, port only\n"); if (hub_port_reset(hub, port1, NULL, HUB_BH_RESET_TIME, true) < 0) hub_port_disable(hub, port1, 1); } else { + dev_dbg(&port_dev->dev, "do warm reset, full device\n"); usb_unlock_port(port_dev); usb_lock_device(udev); usb_reset_device(udev); @@ -5637,6 +5650,7 @@ static void port_event(struct usb_hub *hub, int port1) usb_lock_port(port_dev); connect_change = 0; } + break; } if (connect_change)