Re: [PATCH] Fix MUSB short isochronous packets, with Inventra DMA

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

 




(sorry for the spam, this one _should_ be good)
Hi all,

See the patch below. This works on the BeagleBoard C4 (TI OMAP3530),
but it would be nice if people can test it with some other hardware 
using the MUSB block.

Thank you,

Best regards,

Nicolas

===

MUSB - Fix iso short packets when using Inventra DMA

From: Nicolas Boichat <nicolas@xxxxxxxxxx>

This patch fixes isochronous short packets (i.e., length < maximum packet
size) handling in the MUSB driver, when the Inventra DMA engine is active.

It is based on this tree:
http://arago-project.org/git/people/sriram/ti-psp-omap.git?p=people/sriram/ti-psp-omap.git;a=shortlog;h=refs/heads/OMAPPSP_03.00.02.07
but seems to apply cleanly against a 2.6.35-rc5 kernel (untested though).

For short packets (mode-0 or mode-1 DMA), MUSB_TXCSR_TXPKTRDY must be set
manually by the driver. This was previously done in musb_g_tx (musb_gadget.c),
but incorrectly (all csr flags were cleared, and only MUSB_TXCSR_MODE and
MUSB_TXCSR_TXPKTRDY). Fixing that problem allows some requests to be
transfered correctly, but multiple requests were often put together in one
USB packet, which I believe should not be done, and caused problems if the
packet size was not a multiple of 4.

Instead, MUSB_TXCSR_TXPKTRDY is set in dma_controller_irq (musbhsdma.c), just
like host mode transfers, then, musb_g_tx forces the packet to be flushed, by
setting MUSB_TXCSR_FLUSHFIFO.

A more complete description is available at
http://beagleboard-usbsniffer.blogspot.com/2010/07/musb-isochronous-transfers-fixed.html
and the gadgetfs driver used to reproduce the problem is available at
http://elinux.org/BeagleBoard/GSoC/USBSniffer#MUSB_testing_code .
---
 drivers/usb/musb/musb_gadget.c |   30 +++++++++++++++++++-----------
 drivers/usb/musb/musbhsdma.c   |   21 ++++++++++++---------
 2 files changed, 31 insertions(+), 20 deletions(-)

diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
index 82bcb0d..eea05e5 100644
--- a/drivers/usb/musb/musb_gadget.c
+++ b/drivers/usb/musb/musb_gadget.c
@@ -495,18 +495,23 @@ void musb_g_tx(struct musb *musb, u8 epnum)
 		}
 
 		if (is_dma || request->actual == request->length) {
+#ifdef CONFIG_USB_INVENTRA_DMA
+			if (is_dma && (!dma->desired_mode ||
+					((request->actual %
+						musb_ep->packet_sz) != 0))) {
+				DBG(4, "Flushing FIFO...\n");
+				musb_writew(epio, MUSB_TXCSR,
+					csr | MUSB_TXCSR_FLUSHFIFO);
+				return;
+			}
+#endif
+
 			/*
 			 * First, maybe a terminating short packet. Some DMA
 			 * engines might handle this by themselves.
 			 */
-			if ((request->zero && request->length
-				&& request->length % musb_ep->packet_sz == 0)
-#ifdef CONFIG_USB_INVENTRA_DMA
-				|| (is_dma && (!dma->desired_mode ||
-					(request->actual &
-						(musb_ep->packet_sz - 1))))
-#endif
-			) {
+			if ((request->zero && request->length &&
+				request->length % musb_ep->packet_sz == 0)) {
 				/*
 				 * On DMA completion, FIFO may not be
 				 * available yet...
@@ -514,9 +519,12 @@ void musb_g_tx(struct musb *musb, u8 epnum)
 				if (csr & MUSB_TXCSR_TXPKTRDY)
 					return;
 
-				DBG(4, "sending zero pkt\n");
-				musb_writew(epio, MUSB_TXCSR, MUSB_TXCSR_MODE
-						| MUSB_TXCSR_TXPKTRDY);
+				DBG(4, "sending zero pkt (zero=%d, length=%d,"
+					" actual=%d, dma->desired_mode=%d)\n",
+					request->zero, request->length,
+					request->actual, dma->desired_mode);
+				musb_writew(epio, MUSB_TXCSR,
+					csr | MUSB_TXCSR_TXPKTRDY);
 				request->zero = 0;
 			}
 
diff --git a/drivers/usb/musb/musbhsdma.c b/drivers/usb/musb/musbhsdma.c
index 22a2978..a07e6a3 100644
--- a/drivers/usb/musb/musbhsdma.c
+++ b/drivers/usb/musb/musbhsdma.c
@@ -530,12 +530,10 @@ static irqreturn_t dma_controller_irq(int irq, void *private_data)
 				channel->status = MUSB_DMA_STATUS_FREE;
 
 				/* completed */
-				if ((devctl & MUSB_DEVCTL_HM)
-					&& (musb_channel->transmit)
-					&& ((channel->desired_mode == 0)
-					    || (channel->actual_len &
-					    (musb_channel->max_packet_sz - 1)))
-				    ) {
+				if ((musb_channel->transmit) &&
+					((channel->desired_mode == 0) ||
+					((channel->actual_len %
+					  musb_channel->max_packet_sz) != 0))) {
 					u8  epnum  = musb_channel->epnum;
 					int offset = MUSB_EP_OFFSET(epnum,
 								    MUSB_TXCSR);
@@ -547,11 +545,16 @@ static irqreturn_t dma_controller_irq(int irq, void *private_data)
 					 */
 					musb_ep_select(mbase, epnum);
 					txcsr = musb_readw(mbase, offset);
-					txcsr &= ~(MUSB_TXCSR_DMAENAB
+
+					if (channel->desired_mode == 1) {
+						txcsr &= ~(MUSB_TXCSR_DMAENAB
 							| MUSB_TXCSR_AUTOSET);
-					musb_writew(mbase, offset, txcsr);
+						musb_writew(mbase, offset, txcsr);
+						txcsr &= ~MUSB_TXCSR_DMAMODE;
+						txcsr |= MUSB_TXCSR_DMAENAB;
+					}
+
 					/* Send out the packet */
-					txcsr &= ~MUSB_TXCSR_DMAMODE;
 					txcsr |=  MUSB_TXCSR_TXPKTRDY;
 					musb_writew(mbase, offset, txcsr);
 				}

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