usb: xhci: Simplify xhci_quiesce()

The function reads USBCMD, clears some bits and writes it back. Its treatment of the Run bit is weird: the bit is usually written as 0, as we would expect, but it may also be written as 1 if both its current value and USBSTS.HCHalted are observed as 1.

Per xHCI 5.4.2, HCHalted is 0 whenever Run is 1, so the above can only happen due to buggy HW or SW, e.g. concurrent xhci_quiesce() and xhci_start() execution.

It's unclear why we should treat such cases specially and write the bit as 1. The logic comes from original PoC implementation and has never been explained. Just write 0 every time, which looks like the safer choice when the intent is to stop the xHC.

We could get in trouble if clearing Run causes some very broken xHC to start running after it was halted, but no such case has been documented. It seems the logic was just poorly thought out.

---

Hi Mathias,

I'm not aware of this causing any problem. But I wasted a few minutes trying to understand the point of this code, until I was forced to conclude that it doesn't seem to have any point whatsoever.

Maybe getting rid of it will save somebody else another few minutes. The removal seems harmless.

 drivers/usb/host/xhci.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

index ccd07e4495cb..acdc9254b92c 100644
@@ -102,17 +102,10 @@ int xhci_handshake(void __iomem *ptr, u32 mask, u32 done, u64 timeout_us)
  */
 void xhci_quiesce(struct xhci_hcd *xhci)
 {
-	u32 halted;
 	u32 cmd;
-	u32 mask;
-
-	mask = ~(XHCI_IRQS);
-	halted = readl(&xhci->op_regs->status) & STS_HALT;
-	if (!halted)
-		mask &= ~CMD_RUN;
 
 	cmd = readl(&xhci->op_regs->command);
-	cmd &= mask;
+	cmd &= ~(CMD_RUN | XHCI_IRQS);
 	writel(cmd, &xhci->op_regs->command);
 }
 

2.48.1

Thanks, adding this patch

-Mathias