[PATCH] usb: xhci: Simplify xhci_quiesce()

[PATCH] usb: xhci: Simplify xhci_quiesce()

[Michal Pecio]

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.

Signed-off-by: Michal Pecio <michal.pecio@gmail.com>
---

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(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index ccd07e4495cb..acdc9254b92c 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -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