[PATCH RFC RFT] usb: hcd: Add a usb_device argument to hc_driver.endpoint_reset()

[Michał Pecio]

xHCI needs usb_device in this callback so it employed some hacks that
proved unreliable in the long term and made the code a little tricky.

Make USB core supply it directly and simplify xhci_endpoint_reset().
Use xhci_check_args() to prevent resetting emulated endpoints of root
hubs and to deduplicate argument validation and improve debuggability.

Update ehci_endpoint_reset(), which is the only other such callback,
to accept (and ignore) the new argument.

This fixes the root cause of a 6.15-rc1 regression reported by Paul,
which I was able to reproduce locally. It also solves the general
problem of xhci_endpoint_reset() becoming a no-op after device reset
or changing configuration or altsetting. Although nobody complained
because halted endpoints are reset automatically by xhci_hcd, it was
a bug - sometimes class drivers want to reset not halted endpoints.

Reported-by: Paul Menzel <pmenzel@xxxxxxxxxxxxx>
Closes: https://lore.kernel.org/linux-usb/c279bd85-3069-4841-b1be-20507ac9f2d7@xxxxxxxxxxxxx/
Signed-off-by: Michal Pecio <michal.pecio@xxxxxxxxx>
---

Is such change acceptable to interested parties?

It solves the problem completely for me, because as Alan said,
core calls endpoint_reset() after installing a new config or alt
to notify HCDs that ignore reset_device(), and also those which
implement it incompletely, I guess ;)

Unlike clearing EP_STALLED on reset_device() or drop_endpoint(),
this also fixes cases when another STALL happens after device
reset and the device is not reset again. For example, I see that
when I insert a card after the original problem happens.

At this point I can insert or remove the card, plug or unplug
the reader and reload ums-realtek in any order, it all works.

Paul, could you check if this patch works on your hardware too?

 drivers/usb/core/hcd.c      |  2 +-
 drivers/usb/host/ehci-hcd.c |  3 ++-
 drivers/usb/host/xhci.c     | 27 ++++++++-------------------
 include/linux/usb/hcd.h     |  2 +-
 4 files changed, 12 insertions(+), 22 deletions(-)

diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index a63c793bac21..d2433807a397 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -1986,7 +1986,7 @@ void usb_hcd_reset_endpoint(struct usb_device *udev,
 	struct usb_hcd *hcd = bus_to_hcd(udev->bus);
 
 	if (hcd->driver->endpoint_reset)
-		hcd->driver->endpoint_reset(hcd, ep);
+		hcd->driver->endpoint_reset(hcd, udev, ep);
 	else {
 		int epnum = usb_endpoint_num(&ep->desc);
 		int is_out = usb_endpoint_dir_out(&ep->desc);
diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index 6d1d190c914d..813cdedb14ab 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -1044,7 +1044,8 @@ ehci_endpoint_disable (struct usb_hcd *hcd, struct usb_host_endpoint *ep)
 }
 
 static void
-ehci_endpoint_reset(struct usb_hcd *hcd, struct usb_host_endpoint *ep)
+ehci_endpoint_reset(struct usb_hcd *hcd, struct usb_device *udev,
+		    struct usb_host_endpoint *ep)
 {
 	struct ehci_hcd		*ehci = hcd_to_ehci(hcd);
 	struct ehci_qh		*qh;
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 0452b8d65832..5bf89ba7e2b8 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -3161,11 +3161,10 @@ static void xhci_endpoint_disable(struct usb_hcd *hcd,
  * resume. A new vdev will be allocated later by xhci_discover_or_reset_device()
  */
 
-static void xhci_endpoint_reset(struct usb_hcd *hcd,
+static void xhci_endpoint_reset(struct usb_hcd *hcd, struct usb_device *udev,
 		struct usb_host_endpoint *host_ep)
 {
 	struct xhci_hcd *xhci;
-	struct usb_device *udev;
 	struct xhci_virt_device *vdev;
 	struct xhci_virt_ep *ep;
 	struct xhci_input_control_ctx *ctrl_ctx;
@@ -3175,7 +3174,12 @@ static void xhci_endpoint_reset(struct usb_hcd *hcd,
 	u32 ep_flag;
 	int err;
 
+	err = xhci_check_args(hcd, udev, host_ep, 1, true, __func__);
+	if (err <= 0)
+		return;
+
 	xhci = hcd_to_xhci(hcd);
+	vdev = xhci->devs[udev->slot_id];
 	ep_index = xhci_get_endpoint_index(&host_ep->desc);
 
 	/*
@@ -3185,28 +3189,13 @@ static void xhci_endpoint_reset(struct usb_hcd *hcd,
 	 */
 	if (usb_endpoint_xfer_control(&host_ep->desc) && ep_index == 0) {
 
-		udev = container_of(host_ep, struct usb_device, ep0);
-		if (udev->speed != USB_SPEED_FULL || !udev->slot_id)
-			return;
-
-		vdev = xhci->devs[udev->slot_id];
-		if (!vdev || vdev->udev != udev)
-			return;
-
-		xhci_check_ep0_maxpacket(xhci, vdev);
+		if (udev->speed == USB_SPEED_FULL)
+			xhci_check_ep0_maxpacket(xhci, vdev);
 
 		/* Nothing else should be done here for ep0 during ep reset */
 		return;
 	}
 
-	if (!host_ep->hcpriv)
-		return;
-	udev = (struct usb_device *) host_ep->hcpriv;
-	vdev = xhci->devs[udev->slot_id];
-
-	if (!udev->slot_id || !vdev)
-		return;
-
 	ep = &vdev->eps[ep_index];
 
 	spin_lock_irqsave(&xhci->lock, flags);
diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
index ac95e7c89df5..179c85337eff 100644
--- a/include/linux/usb/hcd.h
+++ b/include/linux/usb/hcd.h
@@ -304,7 +304,7 @@ struct hc_driver {
 
 	/* (optional) reset any endpoint state such as sequence number
 	   and current window */
-	void	(*endpoint_reset)(struct usb_hcd *hcd,
+	void	(*endpoint_reset)(struct usb_hcd *hcd, struct usb_device *udev,
 			struct usb_host_endpoint *ep);
 
 	/* root hub support */
-- 
2.48.1