[PATCH 2/2] musb_host: fix isochronous Tx DMA

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

 



Multi-frame isochronous Tx URBs transfers in DMA mode never complete because
musb_host_tx() just doesn't restart DMA on a second frame, only emitting debug
message instead. In order to amend that, we need to factor out programming the
DMA transfer from musb_ep_program() into musb_tx_dma_program() (dropping bogus
REVISIT comment on the CPPI/TUSB DMA failure path), reorder the code at the end
of musb_host_tx() to facilitate the fallback to PIO iff DMA fails (dropping the
useless label, while at it), and add a variable to handle the buffer offset
consistently for both PIO and DMA modes.  We also need to add an argument to
musb_ep_program() for the same reason (it only worked correctly with non-zero
offset of the first frame in PIO mode).  Also, set the completed isochronous
frame descriptor's 'actual_length' and 'status' fields correctly in DMA mode
(in fact, the latter didn't get set at all).

Oh, and also CPPI reportedly doesn't like sending isochronous packets in the
RNDIS mode, so change the criterion for this mode to be used only on multi-
packet sends (in the single-packet case using it didn't make sense anyway)...

Signed-off-by: Pavel Kiryukhin <pkiryukhin@xxxxxxxxxxxxx>
Signed-off-by: Sergei Shtylyov <sshtylyov@xxxxxxxxxxxxx>

---
The patch is against the recent Linus' kernel, atop of the previosly sent patch
series; it requires the previosly posted patch to be applied too.  It has only
been tested with CPPI 3.0/4.1 DMA drivers, and needs to be verified with other
DMA drivers...

 drivers/usb/musb/cppi_dma.c  |    1 
 drivers/usb/musb/musb_host.c |  237 +++++++++++++++++++------------------------
 2 files changed, 107 insertions(+), 131 deletions(-)

Index: linux-2.6/drivers/usb/musb/cppi_dma.c
===================================================================
--- linux-2.6.orig/drivers/usb/musb/cppi_dma.c
+++ linux-2.6/drivers/usb/musb/cppi_dma.c
@@ -579,6 +579,7 @@ cppi_next_tx_segment(struct musb *musb, 
 	 * trigger the "send a ZLP?" confusion.
 	 */
 	rndis = (maxpacket & 0x3f) == 0
+		&& length > maxpacket
 		&& length < 0xffff
 		&& (length % maxpacket) != 0;
 
Index: linux-2.6/drivers/usb/musb/musb_host.c
===================================================================
--- linux-2.6.orig/drivers/usb/musb/musb_host.c
+++ linux-2.6/drivers/usb/musb/musb_host.c
@@ -102,9 +102,8 @@
  */
 
 
-static void musb_ep_program(struct musb *musb, u8 epnum,
-			struct urb *urb, unsigned int nOut,
-			u8 *buf, u32 len);
+static void musb_ep_program(struct musb *musb, u8 epnum, struct urb *urb,
+			    int is_out, u8 *buf, u32 ofs, u32 len);
 
 /*
  * Clear TX fifo. Needed to avoid BABBLE errors.
@@ -174,10 +173,10 @@ static void
 musb_start_urb(struct musb *musb, int is_in, struct musb_qh *qh)
 {
 	u16			frame;
-	u32			len;
-	void			*buf;
+	u32			len, ofs = 0;
 	void __iomem		*mbase =  musb->mregs;
 	struct urb		*urb = next_urb(qh);
+	void			*buf = urb->transfer_buffer;
 	struct musb_hw_ep	*hw_ep = qh->hw_ep;
 	unsigned		pipe = urb->pipe;
 	u8			address = usb_pipedevice(pipe);
@@ -200,11 +199,10 @@ musb_start_urb(struct musb *musb, int is
 	case USB_ENDPOINT_XFER_ISOC:
 		qh->iso_idx = 0;
 		qh->frame = 0;
-		buf = urb->transfer_buffer + urb->iso_frame_desc[0].offset;
+		ofs = urb->iso_frame_desc[0].offset;
 		len = urb->iso_frame_desc[0].length;
 		break;
 	default:		/* bulk, interrupt */
-		buf = urb->transfer_buffer;
 		len = urb->transfer_buffer_length;
 	}
 
@@ -217,14 +215,14 @@ musb_start_urb(struct musb *musb, int is
 			case USB_ENDPOINT_XFER_ISOC:	s = "-iso"; break;
 			default:			s = "-intr"; break;
 			}; s; }),
-			epnum, buf, len);
+			epnum, buf + ofs, len);
 
 	/* Configure endpoint */
 	if (is_in || hw_ep->is_shared_fifo)
 		hw_ep->in_qh = qh;
 	else
 		hw_ep->out_qh = qh;
-	musb_ep_program(musb, epnum, urb, !is_in, buf, len);
+	musb_ep_program(musb, epnum, urb, !is_in, buf, ofs, len);
 
 	/* transmit may have more work: start it when it is time */
 	if (is_in)
@@ -235,7 +233,6 @@ musb_start_urb(struct musb *musb, int is
 	case USB_ENDPOINT_XFER_ISOC:
 	case USB_ENDPOINT_XFER_INT:
 		DBG(3, "check whether there's still time for periodic Tx\n");
-		qh->iso_idx = 0;
 		frame = musb_readw(mbase, MUSB_FRAME);
 		/* FIXME this doesn't implement that scheduling policy ...
 		 * or handle framecounter wrapping
@@ -616,14 +613,66 @@ musb_rx_reinit(struct musb *musb, struct
 	ep->rx_reinit = 0;
 }
 
+static bool musb_tx_dma_program(struct dma_controller *dma,
+				struct musb_hw_ep *hw_ep, struct musb_qh *qh,
+				struct urb *urb, u32 offset, u32 length)
+{
+	struct dma_channel	*channel = hw_ep->tx_channel;
+	void __iomem		*epio = hw_ep->regs;
+	u16			pkt_size = qh->maxpacket;
+	u16			csr;
+	u8			mode;
+
+#ifdef	CONFIG_USB_INVENTRA_DMA
+	if (length > channel->max_len)
+		length = channel->max_len;
+
+	csr = musb_readw(epio, MUSB_TXCSR);
+	if (length > pkt_size) {
+		mode = 1;
+		csr |= MUSB_TXCSR_AUTOSET | MUSB_TXCSR_DMAMODE |
+		       MUSB_TXCSR_DMAENAB;
+	} else {
+		mode = 0;
+		csr &= ~(MUSB_TXCSR_AUTOSET | MUSB_TXCSR_DMAMODE);
+		csr |= MUSB_TXCSR_DMAENAB; /* against the programming guide */
+	}
+	channel->desired_mode = mode;
+	musb_writew(epio, MUSB_TXCSR, csr);
+#else
+	if (!is_cppi_enabled() && !tusb_dma_omap())
+		return false;
+
+	channel->actual_len = 0;
+
+	/*
+	 * TX uses "RNDIS" mode automatically but needs help
+	 * to identify the zero-length-final-packet case.
+	 */
+	mode = urb->transfer_flags & URB_ZERO_PACKET ? 1 : 0;
+#endif
+
+	qh->segsize = length;
+
+	if (!dma->channel_program(channel, pkt_size, mode,
+				  urb->transfer_dma + offset, length)) {
+		dma->channel_release(channel);
+		hw_ep->tx_channel = NULL;
+
+		csr = musb_readw(epio, MUSB_TXCSR);
+		csr &= ~(MUSB_TXCSR_AUTOSET | MUSB_TXCSR_DMAENAB);
+		musb_writew(epio, MUSB_TXCSR, csr | MUSB_TXCSR_H_WZC_BITS);
+		return false;
+	}
+	return true;
+}
 
 /*
  * Program an HDRC endpoint as per the given URB
  * Context: irqs blocked, controller lock held
  */
-static void musb_ep_program(struct musb *musb, u8 epnum,
-			struct urb *urb, unsigned int is_out,
-			u8 *buf, u32 len)
+static void musb_ep_program(struct musb *musb, u8 epnum, struct urb *urb,
+			    int is_out,	u8 *buf, u32 ofs, u32 len)
 {
 	struct dma_controller	*dma_controller;
 	struct dma_channel	*dma_channel;
@@ -750,87 +799,14 @@ static void musb_ep_program(struct musb 
 		else
 			load_count = min((u32) packet_sz, len);
 
-#ifdef CONFIG_USB_INVENTRA_DMA
-		if (dma_channel) {
-			qh->segsize = min(len, dma_channel->max_len);
-			if (qh->segsize <= packet_sz)
-				dma_channel->desired_mode = 0;
-			else
-				dma_channel->desired_mode = 1;
-
-			if (dma_channel->desired_mode == 0)
-				/* Against the programming guide */
-				csr |= MUSB_TXCSR_DMAENAB;
-			else
-				csr |= MUSB_TXCSR_AUTOSET |
-				       MUSB_TXCSR_DMAMODE |
-				       MUSB_TXCSR_DMAENAB;
-			musb_writew(epio, MUSB_TXCSR, csr);
-
-			dma_ok = dma_controller->channel_program(
-					dma_channel, packet_sz,
-					dma_channel->desired_mode,
-					urb->transfer_dma,
-					qh->segsize);
-			if (dma_ok) {
-				load_count = 0;
-			} else {
-				dma_controller->channel_release(dma_channel);
-				if (is_out)
-					hw_ep->tx_channel = NULL;
-				else
-					hw_ep->rx_channel = NULL;
-				dma_channel = NULL;
-
-				/*
-				 * The programming guide says that we must only
-				 * clear the DMAReqEnab bit before DMAReqMode...
-				 */
-				csr = musb_readw(epio, MUSB_TXCSR);
-				csr &= ~(MUSB_TXCSR_DMAENAB |
-					 MUSB_TXCSR_AUTOSET);
-				musb_writew(epio, MUSB_TXCSR, csr);
-				csr &= ~MUSB_TXCSR_DMAMODE;
-				musb_writew(epio, MUSB_TXCSR, csr);
-			}
-		}
-#endif
-
-		/* candidate for DMA */
-		if ((is_cppi_enabled() || tusb_dma_omap()) && dma_channel) {
-
-			/* Defer enabling DMA */
-			dma_channel->actual_len = 0L;
-			qh->segsize = len;
-
-			/* TX uses "rndis" mode automatically, but needs help
-			 * to identify the zero-length-final-packet case.
-			 */
-			dma_ok = dma_controller->channel_program(
-					dma_channel, packet_sz,
-					(urb->transfer_flags
-							& URB_ZERO_PACKET)
-						== URB_ZERO_PACKET,
-					urb->transfer_dma,
-					qh->segsize);
-			if (dma_ok) {
-				load_count = 0;
-			} else {
-				dma_controller->channel_release(dma_channel);
-				hw_ep->tx_channel = NULL;
-				dma_channel = NULL;
-
-				/* REVISIT there's an error path here that
-				 * needs handling:  can't do dma, but
-				 * there's no pio buffer address...
-				 */
-			}
-		}
+		if (dma_channel && musb_tx_dma_program(dma_controller, hw_ep,
+						       qh, urb, ofs, len))
+			load_count = 0;
 
 		if (load_count) {
 			/* PIO to load FIFO */
 			qh->segsize = load_count;
-			musb_write_fifo(hw_ep, load_count, buf);
+			musb_write_fifo(hw_ep, load_count, buf + ofs);
 		}
 
 		/* re-enable interrupt */
@@ -885,7 +861,7 @@ static void musb_ep_program(struct musb 
 						dma_channel, packet_sz,
 						!(urb->transfer_flags
 							& URB_SHORT_NOT_OK),
-						urb->transfer_dma,
+						urb->transfer_dma + ofs,
 						qh->segsize);
 				if (!dma_ok) {
 					dma_controller->channel_release(
@@ -1134,8 +1110,7 @@ void musb_host_tx(struct musb *musb, u8 
 	int			pipe;
 	bool			done = false;
 	u16			tx_csr;
-	size_t			wLength = 0;
-	u8			*buf = NULL;
+	size_t			offset = 0, length = 0;
 	struct urb		*urb;
 	struct musb_hw_ep	*hw_ep = musb->endpoints + epnum;
 	void __iomem		*epio = hw_ep->regs;
@@ -1153,7 +1128,7 @@ void musb_host_tx(struct musb *musb, u8 
 	/* with CPPI, DMA sometimes triggers "extra" irqs */
 	if (!urb) {
 		DBG(4, "extra TX%d ready, csr %04x\n", epnum, tx_csr);
-		goto finish;
+		return;
 	}
 
 	pipe = urb->pipe;
@@ -1189,7 +1164,7 @@ void musb_host_tx(struct musb *musb, u8 
 		musb_writew(epio, MUSB_TXCSR,
 				MUSB_TXCSR_H_WZC_BITS
 				| MUSB_TXCSR_TXPKTRDY);
-		goto finish;
+		return;
 	}
 
 	if (status) {
@@ -1221,8 +1196,7 @@ void musb_host_tx(struct musb *musb, u8 
 	/* second cppi case */
 	if (dma_channel_status(dma) == MUSB_DMA_STATUS_BUSY) {
 		DBG(4, "extra TX%d ready, csr %04x\n", epnum, tx_csr);
-		goto finish;
-
+		return;
 	}
 
 	if (is_dma_capable() && dma && !status) {
@@ -1291,24 +1265,23 @@ void musb_host_tx(struct musb *musb, u8 
 
 	/* REVISIT this looks wrong... */
 	if (!status || dma || usb_pipeisoc(pipe)) {
-		if (dma)
-			wLength = dma->actual_len;
-		else
-			wLength = qh->segsize;
-		qh->offset += wLength;
+		length = dma ? dma->actual_len : qh->segsize;
+		qh->offset += length;
 
 		if (usb_pipeisoc(pipe)) {
 			struct usb_iso_packet_descriptor	*d;
 
 			d = urb->iso_frame_desc + qh->iso_idx;
-			d->actual_length = qh->segsize;
-			if (++qh->iso_idx >= urb->number_of_packets) {
+			d->actual_length = length;
+			d->status = status;
+			if (++qh->iso_idx >= urb->number_of_packets)
 				done = true;
-			} else {
+			else {
 				d++;
-				buf = urb->transfer_buffer + d->offset;
-				wLength = d->length;
+				offset = d->offset;
+				length = d->length;
 			}
+
 		} else if (dma) {
 			done = true;
 		} else {
@@ -1320,10 +1293,8 @@ void musb_host_tx(struct musb *musb, u8 
 						& URB_ZERO_PACKET))
 				done = true;
 			if (!done) {
-				buf = urb->transfer_buffer
-						+ qh->offset;
-				wLength = urb->transfer_buffer_length
-						- qh->offset;
+				offset = qh->offset;
+				length = urb->transfer_buffer_length - offset;
 			}
 		}
 	}
@@ -1342,28 +1313,32 @@ void musb_host_tx(struct musb *musb, u8 
 		urb->status = status;
 		urb->actual_length = qh->offset;
 		musb_advance_schedule(musb, urb, hw_ep, USB_DIR_OUT);
+		return;
+	} else	if (tx_csr & MUSB_TXCSR_DMAENAB) {
+		if (usb_pipeisoc(pipe) && dma) {
+			if (musb_tx_dma_program(musb->dma_controller, hw_ep, qh,						urb, offset, length))
+				return;
+		} else {
+			DBG(1, "not complete, but DMA enabled?\n");
+			return;
+		}
+	}
 
-	} else if (!(tx_csr & MUSB_TXCSR_DMAENAB)) {
-		/* WARN_ON(!buf); */
-
-		/* REVISIT:  some docs say that when hw_ep->tx_double_buffered,
-		 * (and presumably, fifo is not half-full) we should write TWO
-		 * packets before updating TXCSR ... other docs disagree ...
-		 */
-		/* PIO:  start next packet in this URB */
-		if (wLength > qh->maxpacket)
-			wLength = qh->maxpacket;
-		musb_write_fifo(hw_ep, wLength, buf);
-		qh->segsize = wLength;
-
-		musb_ep_select(mbase, epnum);
-		musb_writew(epio, MUSB_TXCSR,
-				MUSB_TXCSR_H_WZC_BITS | MUSB_TXCSR_TXPKTRDY);
-	} else
-		DBG(1, "not complete, but dma enabled?\n");
+	/*
+	 * PIO: start next packet in this URB.
+	 *
+	 * REVISIT: some docs say that when hw_ep->tx_double_buffered,
+	 * (and presumably, FIFO is not half-full) we should write *two*
+	 * packets before updating TXCSR; other docs disagree...
+	 */
+	if (length > qh->maxpacket)
+		length = qh->maxpacket;
+	musb_write_fifo(hw_ep, length, urb->transfer_buffer + offset);
+	qh->segsize = length;
 
-finish:
-	return;
+	musb_ep_select(mbase, epnum);
+	musb_writew(epio, MUSB_TXCSR,
+		    MUSB_TXCSR_H_WZC_BITS | MUSB_TXCSR_TXPKTRDY);
 }
 
 

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