[PATCH] usb: dwc2: host: rework wait_for_chhltd to handle ACK and retry on errors

[PATCH] usb: dwc2: host: rework wait_for_chhltd to handle ACK and retry on errors

[chalianis1]

From: Chali Anis <chalianis1@gmail.com>

This fixes read corruptions observed on RPi4 when accessing storage
devices behind a USB hub.

Rework the logic inspired by coreboot's dwc2 driver:

- Check HCINT for XFERCOMPL or ACK first, before waiting for CHHLTD,
  to handle the case where DMA has already completed by the time we
  get here; as done in coreboot, both are treated as a valid transfer
  completion since the device acknowledged the transaction
- Wait for CHHLTD after detecting completion and re-read HCINT to get a
  consistent snapshot, ensuring the channel has fully halted and is in a
  known state before the caller starts a new transaction, avoiding
  premature channel teardown; unrecoverable errors (STALL, BABBLE) and
  timeouts are handled at this stage to guarantee a clean state for the
  next transaction
- Add XACTERR handling, returning -EAGAIN to let the caller retry the
  whole transaction

Note: the retry loop and the speculative pre-read interact in a non-obvious
way, however this approach leads to a more consistent and robust operation
of the controller. Despite the complexity, this has been stress-tested by
reading a kernel image over USB and verifying its checksum over 500
consecutive iterations without a single failure, demonstrating reliable
operation in practice even if the code deserves further cleanup.

Signed-off-by: Chali Anis <chalianis1@gmail.com>
---
 drivers/usb/dwc2/host.c | 50 +++++++++++++++++++++++------------------
 1 file changed, 28 insertions(+), 22 deletions(-)

diff --git a/drivers/usb/dwc2/host.c b/drivers/usb/dwc2/host.c
index 93994f0be3be..2fb3d936cd31 100644
--- a/drivers/usb/dwc2/host.c
+++ b/drivers/usb/dwc2/host.c
@@ -129,35 +129,41 @@ static void dwc2_endpoint_reset(struct dwc2 *dwc2, int in, int devnum, int ep)
 
 static int wait_for_chhltd(struct dwc2 *dwc2, u8 hc, uint32_t *sub, u8 *tgl)
 {
-	int ret;
+	int ret, retry = 5;
 	uint32_t hcint, hctsiz, hcchar;
+	bool done = false;
 
-	ret = dwc2_wait_bit_set(dwc2, HCINT(hc), HCINTMSK_CHHLTD, 10000);
-	if (ret) {
-		hcchar = dwc2_readl(dwc2, HCCHAR(hc));
-		dwc2_writel(dwc2, hcchar | HCCHAR_CHDIS, HCCHAR(hc));
-		dwc2_wait_bit_set(dwc2, HCINT(hc), HCINTMSK_CHHLTD, 10000);
-		return ret;
-	}
-
-	hcint = dwc2_readl(dwc2, HCINT(hc));
+	do {
+		hcint = dwc2_readl(dwc2, HCINT(hc));
+		if (hcint & (HCINTMSK_XFERCOMPL | HCINTMSK_ACK)) {
+			hctsiz = dwc2_readl(dwc2, HCTSIZ(hc));
+			*sub = (hctsiz & TSIZ_XFERSIZE_MASK) >> TSIZ_XFERSIZE_SHIFT;
+			*tgl = (hctsiz & TSIZ_SC_MC_PID_MASK) >> TSIZ_SC_MC_PID_SHIFT;
+			dwc2_dbg(dwc2, "%s: HCINT=%08x sub=%u toggle=%d\n", __func__,
+				hcint, *sub, *tgl);
+			done = true;
+		}
 
-	if (hcint & HCINTMSK_AHBERR)
-		dwc2_err(dwc2, "%s: AHB error during internal DMA access\n",
+		if (hcint & HCINTMSK_AHBERR)
+			dwc2_err(dwc2, "%s: AHB error during internal DMA access\n",
 			   __func__);
 
-	if (hcint & HCINTMSK_XFERCOMPL) {
-		hctsiz = dwc2_readl(dwc2, HCTSIZ(hc));
-		*sub = (hctsiz & TSIZ_XFERSIZE_MASK) >> TSIZ_XFERSIZE_SHIFT;
-		*tgl = (hctsiz & TSIZ_SC_MC_PID_MASK) >> TSIZ_SC_MC_PID_SHIFT;
+		/* We wait for CHHLTD, and trap any exception for next transactions */
+		ret = dwc2_wait_bit_set(dwc2, HCINT(hc), HCINTMSK_CHHLTD, 10000);
+		hcint = dwc2_readl(dwc2, HCINT(hc));
+		if (ret || hcint & (HCINTMSK_STALL | HCINTMSK_BBLERR)) {
+			hcchar = dwc2_readl(dwc2, HCCHAR(hc));
+			dwc2_writel(dwc2, hcchar | HCCHAR_CHDIS, HCCHAR(hc));
+			return dwc2_wait_bit_set(dwc2, HCINT(hc), HCINTMSK_CHHLTD, 10000);
+		}
 
-		dwc2_dbg(dwc2, "%s: HCINT=%08x sub=%u toggle=%d\n", __func__,
-			 hcint, *sub, *tgl);
-		return 0;
-	}
+		if (done)
+			return 0;
+
+		if (hcint & (HCINTMSK_NAK | HCINTMSK_FRMOVRUN | HCINTMSK_XACTERR))
+			return -EAGAIN;
 
-	if (hcint & (HCINTMSK_NAK | HCINTMSK_FRMOVRUN))
-		return -EAGAIN;
+	} while (retry-- > 0);
 
 	dwc2_dbg(dwc2, "%s: Unknown channel status: (HCINT=%08x)\n", __func__,
 		 hcint);