Re: [PATCH] Workaround for hardware problem in host mode for the MUSB chipset. Add missed TXPKTRDY check.

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

 



Hello.

Hans Petter SELASKY wrote:

diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
index 877d20b..b14a5ff 100644
--- a/drivers/usb/musb/musb_host.c
+++ b/drivers/usb/musb/musb_host.c
@@ -106,24 +106,41 @@ static void musb_ep_program(struct musb *musb, u8 epnum,
 static void musb_h_tx_flush_fifo(struct musb_hw_ep *ep)
 {
 	void __iomem	*epio = ep->regs;
+	void __iomem	*regs = ep->musb->mregs;
 	u16		csr;
-	u16		lastcsr = 0;
-	int		retries = 1000;
+	u8		addr;
+	int		retries = 3000; /* 3ms */
+ /*
+	 * NOTE: We are using a hack here because the FIFO-FLUSH
+	 * bit is broken in hardware! The hack consists of changing

    Broken on which version of MUSB core?

All, I guess.

   Hm...

+	 * gone rule.
+	 */
+	addr = musb_readb(regs, MUSB_BUSCTL_OFFSET(ep->epnum, MUSB_TXFUNCADDR));
 	csr = musb_readw(epio, MUSB_TXCSR);
 	while (csr & MUSB_TXCSR_FIFONOTEMPTY) {

Besides, flushing is only documented to work when there's a full packet in FIFO, so this condition should probably change...

I mean TxPktRdy bit should be set for the flush to work. It's even recommended to set it along with FlushFIFO, IIRC...

If you read on, you will see that on errors the TX FIFO will get flushed too.

 		csr = musb_readw(epio, MUSB_TXCSR);
-		if (WARN(retries-- < 1,
-				"Could not flush host TX%d fifo: csr: %04x\n",
-				ep->epnum, csr))
-			return;
-		mdelay(1);
+		retries--;
+		if (retries == 0) {
+			/* can happen if the USB clocks are OFF */

    So what? Why you changed WARN() to DBG()?

We can get a timeout in this loop during USB cable unplug, because then then the chip's clock is no longer running,

   What? How does chip clock relate to USB cable plug/unplug?! :-O

and there is no response to any FIFO commands. This is not a serious problem so the WARN() was changed into DBG().

I think the failure to flush FIFO is serious enough event, deserving a WARN()...

+			DBG(3, "Could not flush host TX%d "
+				"fifo: csr=0x%04x\n", ep->epnum, csr);
+			break;
+		}
+		udelay(1);
 	}
+	/* clear any errors */
+	csr &= ~(MUSB_TXCSR_H_ERROR
+		| MUSB_TXCSR_H_RXSTALL
+		| MUSB_TXCSR_H_NAKTIMEOUT);
+	musb_writew(epio, MUSB_TXCSR, csr);

    Why?

Because when the 3-strikes and you're gone activates it will cause some error bits to be set which we should not report in the next TX transfer on the given FIFO.

Well, I think "the 3 strikes" only causes certain bit to be set -- namely Error...

@@ -1141,7 +1158,9 @@ void musb_host_tx(struct musb *musb, u8 epnum)
 		DBG(3, "TX 3strikes on ep=%d\n", epnum);
status = -ETIMEDOUT;
-
+	} else if (tx_csr & MUSB_TXCSR_TXPKTRDY) {
+		/* BUSY - can happen during USB transfer cancel */
+		return;

    Is this change somehow releated to yor proposed workaround?

No. This is a different issue, which actually causes data loss during stress tests.

   Shouldn't it be in a different patch then?

WBR, Sergei

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