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

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

 



Hello.

Nicolas Boichat wrote:

(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

The above test should be under --- tearline to simplify the work for the maintainer (everything under --- will be dropped automatically when applying the patch).

===

MUSB - Fix iso short packets when using Inventra DMA

   There's not need to duplicate subject.

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

   What about them? :-)

). 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 .
[...]
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))) {

   Parens not necessary either around % or !=...

+				DBG(4, "Flushing FIFO...\n");
+				musb_writew(epio, MUSB_TXCSR,
+					csr | MUSB_TXCSR_FLUSHFIFO);

   Shouldn't you write to TXCSR twice to flush the double-buffered FIFO.

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

   Parns not needed around 'musb_channel->transmit'.

+					((channel->desired_mode == 0) ||

   And neither around ==...

+					((channel->actual_len %
+					  musb_channel->max_packet_sz) != 0))) {

   And neither around % or !=...

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

   Why set DMAReqEnab once again?

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