Re: r8152: RX stops until rebind after -EPROTO on the bulk-in endpoint

[Michal Pecio <michal.pecio@gmail.com>]

[Adding another Realtek contact I found in r8152.c]

Hi,

I found a way to artificially produce something similar, though not
mid-operation, so there is no partly completed transfer in flight.

This patch works for the simplified case, you could try if it helps
with your issues too.

It adds usb_clear_halt() and logs its return status just in case.

The commented code is my artificial trigger. To test:

  ifconfig up
  # receive a few packets
  ifconfig down
  ethtool
  ifconfig up
  # receive another packet
  # dmesg shows "transfer errors" and "usb_clear_halt(pipe_in): 0"

The rest is xHCI changes to delay execution of URBs queued in advance
until the pipe is cleared. This keeps them from failing and may actually
be necessary under heavy RX, because a concurrently processed URB could
cause usb_clear_halt() to leave the pipe desynchronized again.

In principle drivers should unlink URBs before calling that, but xHCI
is tolerant in this case. And HCDs aren't really supposed to continue
URB execution after a halt, so that was arguably a bug anyway.

The xHCI part is written a bit "oddly" to avoid conflicts with changes
between 7.2 and 7.3 (EP_DROP_PENDING).

Regards,
Michal

---
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index f61686433031..e59af0e2ca1d 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -7044,9 +7044,15 @@ static void rtl_work_func_t(struct work_struct *work)
 	    netif_carrier_ok(tp->netdev))
 		tasklet_schedule(&tp->tx_tl);
 
-	if (test_and_clear_bit(RX_EPROTO, &tp->flags) &&
-	    !list_empty(&tp->rx_done))
-		napi_schedule(&tp->napi);
+	if (test_and_clear_bit(RX_EPROTO, &tp->flags)) {
+		/* On USB3 the pipe won't work until we clear halt */
+		if (tp->udev->speed >= USB_SPEED_SUPER)
+			dev_info(&tp->intf->dev, "usb_clear_halt(pipe_in): %d\n",
+					usb_clear_halt(tp->udev, tp->pipe_in));
+
+		if (!list_empty(&tp->rx_done))
+			napi_schedule(&tp->napi);
+	}
 
 	mutex_unlock(&tp->control);
 
@@ -8935,6 +8941,9 @@ int rtl8152_get_link_ksettings(struct net_device *netdev,
 
 	mutex_lock(&tp->control);
 
+	//dev_err(&tp->intf->dev, "breaking RX\n");
+	//usb_reset_endpoint(tp->udev, 0x81);
+
 	mii_ethtool_get_link_ksettings(&tp->mii, cmd);
 
 	linkmode_mod_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT,
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index b634f1089bb1..25910c40ee74 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -556,6 +556,8 @@ void xhci_ring_ep_doorbell(struct xhci_hcd *xhci,
 	struct xhci_virt_ep *ep = &xhci->devs[slot_id]->eps[ep_index];
 	unsigned int ep_state = ep->ep_state;
 
+	if (ep_state & EP_WAIT_CLEAR)
+		return;
 	/* Don't ring the doorbell for this endpoint if there are pending
 	 * cancellations because we don't want to interrupt processing.
 	 * We don't want to restart any stream rings if there's a set dequeue
@@ -2563,8 +2567,15 @@ static void process_bulk_intr_td(struct xhci_hcd *xhci, struct xhci_virt_ep *ep,
 	case COMP_USB_TRANSACTION_ERROR:
 		if (xhci->quirks & XHCI_NO_SOFT_RETRY ||
 		    (ep->err_count++ > MAX_SOFT_RETRY) ||
-		    le32_to_cpu(slot_ctx->tt_info) & TT_SLOT)
+		    le32_to_cpu(slot_ctx->tt_info) & TT_SLOT) {
+			/* halted endpoint */
+			if (td->urb->dev->speed >= USB_SPEED_SUPER) {
+				dev_info(&td->urb->dev->dev, "waiting for ep 0x%X clear halt\n",
+						td->urb->ep->desc.bEndpointAddress);
+				ep->ep_state |= EP_WAIT_CLEAR;
+			}
 			break;
+		}
 
 		td->status = 0;
 
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 6d997e07cb84..277911ebf65c 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -1855,6 +1855,8 @@ static int xhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status)
 		xhci_dbg(xhci, "Invalidating TDs instantly on slot %d ep %d in state 0x%x\n",
 				urb->dev->slot_id, ep_index, ep->ep_state);
 		xhci_process_cancelled_tds(xhci, ep);
+	} else if (ep->ep_state & EP_WAIT_CLEAR) {
+		xhci_process_cancelled_tds(xhci, ep);
 	} else {
 		/* Otherwise, queue a new Stop Endpoint command */
 		command = xhci_alloc_command(xhci, false, GFP_ATOMIC);
@@ -3403,7 +3405,7 @@ static void xhci_endpoint_reset(struct usb_hcd *hcd,
 	/* Bail out if toggle is already being cleared by a endpoint reset */
 	spin_lock_irqsave(&xhci->lock, flags);
 	if (ep->ep_state & EP_HARD_CLEAR_TOGGLE) {
-		ep->ep_state &= ~EP_HARD_CLEAR_TOGGLE;
+		ep->ep_state &= ~(EP_HARD_CLEAR_TOGGLE | EP_WAIT_CLEAR);
 		spin_unlock_irqrestore(&xhci->lock, flags);
 		return;
 	}
@@ -3495,7 +3497,7 @@ static void xhci_endpoint_reset(struct usb_hcd *hcd,
 	xhci_free_command(xhci, stop_cmd);
 	spin_lock_irqsave(&xhci->lock, flags);
 	if (ep->ep_state & EP_SOFT_CLEAR_TOGGLE)
-		ep->ep_state &= ~EP_SOFT_CLEAR_TOGGLE;
+		ep->ep_state &= ~(EP_SOFT_CLEAR_TOGGLE | EP_WAIT_CLEAR);
 	spin_unlock_irqrestore(&xhci->lock, flags);
 }
 
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 379374a67c64..34ace464e155 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -670,6 +670,7 @@ struct xhci_virt_ep {
 	struct xhci_ring		*new_ring;
 	unsigned int			err_count;
 	unsigned int			ep_state;
+#define EP_WAIT_CLEAR		BIT(12)
 #define SET_DEQ_PENDING		BIT(0)
 #define EP_HALTED		BIT(1)	/* For stall handling */
 #define EP_STOP_CMD_PENDING	BIT(2)	/* For URB cancellation */