> -----Original Message----- > From: linux-usb-owner@xxxxxxxxxxxxxxx [mailto:linux-usb-owner@xxxxxxxxxxxxxxx] On Behalf Of Stephane > Cerveau > Sent: Monday, February 23, 2009 5:03 PM > To: Sergei Shtylyov; felipe.balbi@xxxxxxxxx; gregkh@xxxxxxx; david-b@xxxxxxxxxxx > Cc: davinci-linux-open-source@xxxxxxxxxxxxxxxxxxxx; linux-usb@xxxxxxxxxxxxxxx > Subject: USB host Issue: scsi command abort > > 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. How many endpoints are supported in your musb platform? I think it is issue when all bulk is going to one endpoint (bulk-mux). You may try below two patches. [1] http://marc.info/?l=linux-usb&m=123393008601134&w=2 [2] http://marc.info/?l=linux-usb&m=123393015101594&w=2 Regards, Ajay > > 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:sshtylyov@xxxxxxxxxxxxx] > Sent: vendredi 20 février 2009 21:35 > To: felipe.balbi@xxxxxxxxx; gregkh@xxxxxxx; david-b@xxxxxxxxxxx > Cc: davinci-linux-open-source@xxxxxxxxxxxxxxxxxxxx; linux-usb@xxxxxxxxxxxxxxx > 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 <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); > } > > > > > _______________________________________________ > Davinci-linux-open-source mailing list > Davinci-linux-open-source@xxxxxxxxxxxxxxxxxxxx > http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source > > -- > 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 -- 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
