[PATCH V3 0/2] usb: musb: fix dropped packets

[PATCH V3 0/2] usb: musb: fix dropped packets

[Andrew Goodbody]

The musb driver can drop rx packets when heavily loaded. These two
patches address two issues that can cause this. Both issues arose
when an endpoint was reprogrammed. The first patch is a logic bug
that resulted in a shared_fifo in rx mode not having its state
cleared out. The second patch fixes a race condition caused by
not stopping the dedicated endpoint for bulk packets before
rotating its queue which allowed a packet to be recieved and then
thrown away.

V3 Updated the comment to better reference the manual
V2 added a comment and removed debugging code

Andrew Goodbody (2):
  usb: musb: Ensure rx reinit occurs for shared_fifo endpoints
  usb: musb: Stop bulk endpoint while queue is rotated

 drivers/usb/musb/musb_host.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

-- 
2.7.4

[PATCH V3 1/2] usb: musb: Ensure rx reinit occurs for shared_fifo endpoints

[Andrew Goodbody]

shared_fifo endpoints would only get a previous tx state cleared
out, the rx state was only cleared for non shared_fifo endpoints
Change this so that the rx state is cleared for all endpoints.
This addresses an issue that resulted in rx packets being dropped
silently.

Signed-off-by: Andrew Goodbody <andrew.goodbody@cambrionix.com>
Cc: stable@vger.kernel.org
---
V3 no change
V2 removed debugging call

 drivers/usb/musb/musb_host.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
index b7a02ce..e5b6aba 100644
--- a/drivers/usb/musb/musb_host.c
+++ b/drivers/usb/musb/musb_host.c
@@ -594,14 +594,13 @@ musb_rx_reinit(struct musb *musb, struct musb_qh *qh, u8 epnum)
 		musb_writew(ep->regs, MUSB_TXCSR, 0);
 
 	/* scrub all previous state, clearing toggle */
-	} else {
-		csr = musb_readw(ep->regs, MUSB_RXCSR);
-		if (csr & MUSB_RXCSR_RXPKTRDY)
-			WARNING("rx%d, packet/%d ready?\n", ep->epnum,
-				musb_readw(ep->regs, MUSB_RXCOUNT));
-
-		musb_h_flush_rxfifo(ep, MUSB_RXCSR_CLRDATATOG);
 	}
+	csr = musb_readw(ep->regs, MUSB_RXCSR);
+	if (csr & MUSB_RXCSR_RXPKTRDY)
+		WARNING("rx%d, packet/%d ready?\n", ep->epnum,
+			musb_readw(ep->regs, MUSB_RXCOUNT));
+
+	musb_h_flush_rxfifo(ep, MUSB_RXCSR_CLRDATATOG);
 
 	/* target addr and (for multipoint) hub addr/port */
 	if (musb->is_multipoint) {
-- 
2.7.4

[PATCH V3 2/2] usb: musb: Stop bulk endpoint while queue is rotated

[Andrew Goodbody]

Ensure that the endpoint is stopped by clearing REQPKT before
clearing DATAERR_NAKTIMEOUT before rotating the queue on the
dedicated bulk endpoint.
This addresses an issue where a race could result in the endpoint
receiving data before it was reprogrammed resulting in a warning
about such data from musb_rx_reinit before it was thrown away.
The data thrown away was a valid packet that had been correctly
ACKed which meant the host and device got out of sync.

Signed-off-by: Andrew Goodbody <andrew.goodbody@cambrionix.com>
Cc: stable@vger.kernel.org
---
V3 removed the old comment, moved the new comment in place of the old one
   and updated it to better reference the programmers guide
V2 added comment about clearing REQPKT before DATAERR_NAKTIMEOUT

 drivers/usb/musb/musb_host.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
index e5b6aba..17421d0 100644
--- a/drivers/usb/musb/musb_host.c
+++ b/drivers/usb/musb/musb_host.c
@@ -994,9 +994,15 @@ static void musb_bulk_nak_timeout(struct musb *musb, struct musb_hw_ep *ep,
 	if (is_in) {
 		dma = is_dma_capable() ? ep->rx_channel : NULL;
 
-		/* clear nak timeout bit */
+		/*
+		 * Need to stop the transaction by clearing REQPKT first
+		 * then the NAK Timeout bit ref MUSBMHDRC USB 2.0 HIGH-SPEED
+		 * DUAL-ROLE CONTROLLER Programmer's Guide, section 9.2.2
+		 */
 		rx_csr = musb_readw(epio, MUSB_RXCSR);
 		rx_csr |= MUSB_RXCSR_H_WZC_BITS;
+		rx_csr &= ~MUSB_RXCSR_H_REQPKT;
+		musb_writew(epio, MUSB_RXCSR, rx_csr);
 		rx_csr &= ~MUSB_RXCSR_DATAERROR;
 		musb_writew(epio, MUSB_RXCSR, rx_csr);
 
-- 
2.7.4

Re: [PATCH V3 0/2] usb: musb: fix dropped packets

[Bin Liu]

Hi,

On Tue, May 24, 2016 at 09:22:32AM +0100, Andrew Goodbody wrote:
> The musb driver can drop rx packets when heavily loaded. These two
> patches address two issues that can cause this. Both issues arose
> when an endpoint was reprogrammed. The first patch is a logic bug
> that resulted in a shared_fifo in rx mode not having its state
> cleared out. The second patch fixes a race condition caused by
> not stopping the dedicated endpoint for bulk packets before
> rotating its queue which allowed a packet to be recieved and then
> thrown away.
> 
> V3 Updated the comment to better reference the manual
> V2 added a comment and removed debugging code
> 
> Andrew Goodbody (2):
>   usb: musb: Ensure rx reinit occurs for shared_fifo endpoints
>   usb: musb: Stop bulk endpoint while queue is rotated

Signed-off-by: Bin Liu <b-liu@ti.com>

Regards,
-Bin.

> 
>  drivers/usb/musb/musb_host.c | 21 +++++++++++++--------
>  1 file changed, 13 insertions(+), 8 deletions(-)
> 
> -- 
> 2.7.4
>