Multi-frame isochronous Tx URBs transfers in DMA mode never complete with the CPPI/TUSB DMA drivers because musb_host_tx() just doesn't restart DMA on the second frame, only emitting a debug message instead; they complete, but in PIO mode, with the Inventra DMA driver as it exits the DMA mode after sending the first packet and musb_host_tx() thinks that the transfer is happening in PIO mode afterwards. 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 for multi- packet transfers (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> --- Reworked the patch to make Inventra DMA actually transfer multi-frame ISO Tx URBs in DMA mode... drivers/usb/musb/cppi_dma.c | 1 drivers/usb/musb/musb_host.c | 235 +++++++++++++++++++------------------------ 2 files changed, 106 insertions(+), 130 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 @@ -101,9 +101,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. @@ -171,10 +170,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); @@ -197,7 +196,7 @@ 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 */ @@ -214,14 +213,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) @@ -232,7 +231,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 @@ -614,14 +612,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; @@ -748,87 +798,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 */ @@ -883,7 +860,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( @@ -1132,8 +1109,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; @@ -1151,7 +1127,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; @@ -1187,7 +1163,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) { @@ -1219,30 +1195,28 @@ 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; } /* 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 { @@ -1254,10 +1228,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; } } } @@ -1276,28 +1248,31 @@ 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 (usb_pipeisoc(pipe) && dma) { + if (musb_tx_dma_program(musb->dma_controller, hw_ep, qh, urb, + offset, length)) + return; + } else if (tx_csr & MUSB_TXCSR_DMAENAB) { + 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