Re: [PATCH] usb: hub: avoid warm port reset during USB3 disconnect

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx>
Signed-off-by: Mathias Nyman <mathias.nyman@xxxxxxxxxxxxxxx>
---
  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)



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux