Re: [PATCH] usb: musb: fix tx fifo flush handling

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 11/05/2015 12:25 PM, Felipe Balbi wrote:

Hi,

Bin Liu <b-liu@xxxxxx> writes:
Here are a few changes in musb_h_tx_flush_fifo().

- Refering to 2ccc6d30a (usb: musb: fix bit mask for CSR in
   musb_h_tx_flush_fifo()), the datasheet says that MUSB_TXCSR_FLUSHFIFO
   is only valid when MUSB_TXCSR_TXPKTRDY is set as well. It means
   MUSB_TXCSR_TXPKTRDY should be checked, not set.

- It seems to be common that sometimes (if not always) musb is unable
   to flush tx fifo during urb dequeue. But it seems to be harmless,
   since the tx fifo flush is done again in musb_ep_program() when
   re-use the hw_ep.

   But the WARN() scares many end users, so change it to dev_dbg().

- applications could queue up many tx urbs, then the 1ms delay could
   causes minutes of delay in device disconnect. So remove it to get
   better user experience. The 1ms delay does not help the flushing
   anyway.

- cleanup the debug code - related to lastcsr.

Signed-off-by: Bin Liu <b-liu@xxxxxx>
---
  drivers/usb/musb/musb_host.c | 16 ++++++----------
  1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
index 883a9ad..f9d4b4f 100644
--- a/drivers/usb/musb/musb_host.c
+++ b/drivers/usb/musb/musb_host.c
@@ -112,22 +112,18 @@ static void musb_h_tx_flush_fifo(struct musb_hw_ep *ep)
  	struct musb	*musb = ep->musb;
  	void __iomem	*epio = ep->regs;
  	u16		csr;
-	u16		lastcsr = 0;
  	int		retries = 1000;

  	csr = musb_readw(epio, MUSB_TXCSR);
-	while (csr & MUSB_TXCSR_FIFONOTEMPTY) {
-		if (csr != lastcsr)
-			dev_dbg(musb->controller, "Host TX FIFONOTEMPTY csr: %02x\n", csr);
-		lastcsr = csr;
-		csr |= MUSB_TXCSR_FLUSHFIFO | MUSB_TXCSR_TXPKTRDY;
+	while (csr & (MUSB_TXCSR_FIFONOTEMPTY | MUSB_TXCSR_TXPKTRDY)) {

this doesn't really fix the problem. This check will be true if either
or both bits are set. You want only when both are set, so it should be:

while ((csr & FIFONOTEMPTY) && (csr & TXPKTRDY))


Ahh, I should have used (csr & (MUSB_TXCSR_FIFONOTEMPTY | MUSB_TXCSR_TXPKTRDY) == MUSB_TXCSR_FIFONOTEMPTY | MUSB_TXCSR_TXPKTRDY), but it is too long. V2 comes soon.

Thanks,
-Bin.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux