Hi, I'm currently testing the USB host driver in DMA mode and I encounter some issues with the use of simultaneous transfer on a MSC USB key(Common key with no hub inside) and a WIFI dongle RT73. My test case is the following: - I download in WIFI mode a large amount of data which I stored on USB MSC. - In the mean while, I try to read this data stored on the same MSC key.
After a certain time(~5minutes), I encounter a command abort in scsi glue coming from the hub. It seems that the USB MSC key is unlinked. I would like to know if anybody has already encounter this problem with the last MUSB driver and also if anybody can help me to understand this abortion. Thank you very much. Stéphane Cerveau -----Original Message----- From: Sergei Shtylyov [mailto:sshtyl...@ru.mvista.com] Sent: vendredi 20 février 2009 21:35 To: felipe.ba...@nokia.com; gre...@suse.de; davi...@pacbell.net Cc: davinci-linux-open-source@linux.davincidsp.com; linux-...@vger.kernel.org Subject: [PATCH 2/2] musb_host: fix isochronous Tx DMA 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 <pkiryuk...@ru.mvista.com> Signed-off-by: Sergei Shtylyov <sshtyl...@ru.mvista.com> --- 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); } _______________________________________________ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source _______________________________________________ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source