xhci: rework xhci internal endpoint halt state detection.

When xhci_requires_manual_halt_cleanup() was written it wasn't clear
that the xhci internal endpoint halt state always needs to be cleared
with a reset endpoint command. Functional stall cases additionally halt
the device side endpoint which requires class driver to clear the device
side halt with a CLEAR_FEATURE(ENDPOINT_HALT) request as well.

Clean up, rename, and make sure the new function always return true
when internal endpoint state is halted, including stall cases.

Based on related cleanup suggestion code by Niklas Neronin

cc: Niklas Neronin <niklas.neronin@linux.intel.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
Link: https://lore.kernel.org/r/20240626124835.1023046-15-mathias.nyman@linux.intel.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 900bf34174f9a15ea3ea599d8bce7a83821eb645..3479c9cb5d332d0aa1eea1ce4d47eb755d0be698 100644 (file)
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -2130,28 +2130,34 @@ static void xhci_clear_hub_tt_buffer(struct xhci_hcd *xhci, struct xhci_td *td,
        }
 }
 
-/* Check if an error has halted the endpoint ring.  The class driver will
- * cleanup the halt for a non-default control endpoint if we indicate a stall.
- * However, a babble and other errors also halt the endpoint ring, and the class
- * driver won't clear the halt in that case, so we need to issue a Set Transfer
- * Ring Dequeue Pointer command manually.
+/*
+ * Check if xhci internal endpoint state has gone to a "halt" state due to an
+ * error or stall, including default control pipe protocol stall.
+ * The internal halt needs to be cleared with a reset endpoint command.
+ *
+ * External device side is also halted in functional stall cases. Class driver
+ * will clear the device halt with a CLEAR_FEATURE(ENDPOINT_HALT) request later.
  */
-static int xhci_requires_manual_halt_cleanup(struct xhci_ep_ctx *ep_ctx, unsigned int trb_comp_code)
+static bool xhci_halted_host_endpoint(struct xhci_ep_ctx *ep_ctx, unsigned int comp_code)
 {
-       /* TRB completion codes that may require a manual halt cleanup */
-       if (trb_comp_code == COMP_USB_TRANSACTION_ERROR ||
-                       trb_comp_code == COMP_BABBLE_DETECTED_ERROR ||
-                       trb_comp_code == COMP_SPLIT_TRANSACTION_ERROR)
-               /* The 0.95 spec says a babbling control endpoint
-                * is not halted. The 0.96 spec says it is.  Some HW
-                * claims to be 0.95 compliant, but it halts the control
-                * endpoint anyway.  Check if a babble halted the
-                * endpoint.
+       /* Stall halts both internal and device side endpoint */
+       if (comp_code == COMP_STALL_ERROR)
+               return true;
+
+       /* TRB completion codes that may require internal halt cleanup */
+       if (comp_code == COMP_USB_TRANSACTION_ERROR ||
+           comp_code == COMP_BABBLE_DETECTED_ERROR ||
+           comp_code == COMP_SPLIT_TRANSACTION_ERROR)
+               /*
+                * The 0.95 spec says a babbling control endpoint is not halted.
+                * The 0.96 spec says it is. Some HW claims to be 0.95
+                * compliant, but it halts the control endpoint anyway.
+                * Check endpoint context if endpoint is halted.
                 */
                if (GET_EP_CTX_STATE(ep_ctx) == EP_STATE_HALTED)
-                       return 1;
+                       return true;
 
-       return 0;
+       return false;
 }
 
 int xhci_is_vendor_info_code(struct xhci_hcd *xhci, unsigned int trb_comp_code)
@@ -2321,7 +2327,7 @@ static int process_ctrl_td(struct xhci_hcd *xhci, struct xhci_virt_ep *ep,
        case COMP_STOPPED_LENGTH_INVALID:
                goto finish_td;
        default:
-               if (!xhci_requires_manual_halt_cleanup(ep_ctx, trb_comp_code))
+               if (!xhci_halted_host_endpoint(ep_ctx, trb_comp_code))
                        break;
                xhci_dbg(xhci, "TRB error %u, halted endpoint index = %u\n",
                         trb_comp_code, ep->ep_index);
@@ -2791,11 +2797,9 @@ static int handle_tx_event(struct xhci_hcd *xhci,
                                xhci_dbg(xhci, "td_list is empty while skip flag set. Clear skip flag for slot %u ep %u.\n",
                                         slot_id, ep_index);
                        }
-                       if (trb_comp_code == COMP_STALL_ERROR ||
-                           xhci_requires_manual_halt_cleanup(ep_ctx, trb_comp_code)) {
-                               xhci_handle_halted_endpoint(xhci, ep, NULL,
-                                                           EP_HARD_RESET);
-                       }
+                       if (xhci_halted_host_endpoint(ep_ctx, trb_comp_code))
+                               xhci_handle_halted_endpoint(xhci, ep, NULL, EP_HARD_RESET);
+
                        return 0;
                }
 
@@ -2911,10 +2915,8 @@ static int handle_tx_event(struct xhci_hcd *xhci,
                 */
 
                if (trb_is_noop(ep_trb)) {
-                       if (trb_comp_code == COMP_STALL_ERROR ||
-                           xhci_requires_manual_halt_cleanup(ep_ctx, trb_comp_code))
-                               xhci_handle_halted_endpoint(xhci, ep, td,
-                                                           EP_HARD_RESET);
+                       if (xhci_halted_host_endpoint(ep_ctx, trb_comp_code))
+                               xhci_handle_halted_endpoint(xhci, ep, td, EP_HARD_RESET);
                } else {
                        td->status = status;