Heikki Krogerus wrote:
Concept was originally implemented for musb_gadget by
Felipe Balbi and Juha Yrjölä.
Signed-off-by: Heikki Krogerus <ext-heikki.krogerus@xxxxxxxxx>
NAK for now -- due to mis-implementing the ISO transfers...
---
drivers/usb/musb/musb_dma.h | 6 ++
drivers/usb/musb/musb_host.c | 168 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 174 insertions(+), 0 deletions(-)
diff --git a/drivers/usb/musb/musb_dma.h b/drivers/usb/musb/musb_dma.h
index 916065b..428b5ac 100644
--- a/drivers/usb/musb/musb_dma.h
+++ b/drivers/usb/musb/musb_dma.h
@@ -80,6 +80,12 @@ struct musb_hw_ep;
#define tusb_dma_omap() 0
#endif
+#ifdef CONFIG_USB_INVENTRA_DMA
+#define is_inventra_dma() 1
+#else
+#define is_inventra_dma() 0
+#endif
+
/* Anomaly 05000456 - USB Receive Interrupt Is Not Generated in DMA Mode 1
* Only allow DMA mode 1 to be used when the USB will actually generate the
* interrupts we expect.
diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
index b6139b7..d72969f 100644
--- a/drivers/usb/musb/musb_host.c
+++ b/drivers/usb/musb/musb_host.c
@@ -201,6 +201,174 @@ static inline void musb_release_qtd(struct musb_qtd *qtd)
qtd = NULL;
}
+/**
+ * start_dma - start dma for a transfer
+ * @musb: musb controller pointer
+ * @qtd: queue element to be transfered
+ *
+ * Context: controller locked, IRQs blocked, endpoint selected
+ */
+static int start_dma(struct musb *musb, struct musb_qtd *qtd)
+{
+ struct dma_controller *dma_controller = musb->dma_controller;
+ struct musb_hw_ep *hw_ep = qtd->qh->hw_ep;
+ struct urb *urb = qtd->urb;
+ int is_in = usb_pipein(urb->pipe);
+ struct dma_channel *dma;
+ u32 len, offset = 0;
+ u16 csr;
+
+ if (!dma_controller)
+ return -1;
+
+ dma = is_in ? hw_ep->rx_channel : hw_ep->tx_channel;
+ if (!dma) {
+ dma = dma_controller->channel_alloc(
+ dma_controller, hw_ep, !is_in);
+ if (!dma)
+ return -1;
+
+ if (is_in)
+ hw_ep->rx_channel = dma;
+ else
+ hw_ep->tx_channel = dma;
+ }
+
+ len = urb->transfer_buffer_length - urb->actual_length;
This is not true for the ISO transfers. You should only take the
length the first segment, not the whole buffer. Read the old code more
attentively please.
+ dma->actual_len = 0L;
+ qtd->segsize = min(len, dma->max_len);
+
+ /* Only supporting mode1. For transfers smaller then 512, pio is used.
Only "than".
+ */
+ if (len <= qtd->maxpacket ||
Wait, with this you're using PIO also for the *full* packets
(*including* 512 bytes long ones)! I'm quite sure you meant < here...
+ (len == qtd->maxpacket && qtd->maxpacket < 512) ||
This expression will never evaluate if len == qtd->maxpacket due to
the short-circuit nature of || and &&.
+ (musb_readw(hw_ep->regs, MUSB_RXCOUNT) < 512)) {
Don't you want to check is_in first?
+ goto small_transfer;
+ }
+
+ if (is_in && is_inventra_dma()) {
+ /* REVISIT always round-up the transfer to be multiple of the
+ * maximum package size or musb will not interrupt. AUTOREQ
Only "packet".
+ * guarantees that an extra IN token is always send at the end
+ * of every transfer.
+ *
+ * FIXME if the transfer is already multiple of the maximum
+ * package size, removing 512 from the transfer. This should
+ * not be necessary. Leaving the extra data (AUTOREQ) pending
+ * should be enough.
+ */
+ if (!(qtd->segsize % qtd->maxpacket))
+ qtd->segsize -= qtd->maxpacket;
+ else
+ qtd->segsize -= (qtd->segsize % qtd->maxpacket);
+ }
+
+ if (qtd->type == USB_ENDPOINT_XFER_ISOC)
+ offset = urb->iso_frame_desc[0].offset;
And how this is going to work for the rest of an ISO transfer? I have
a feeling you're breaking what was already working with this patchset,
i.e. ISO transfers... BTW, ISO transfers should now be carried out only
in PIO mode if I don't mistake -- due to the packet size constraint, if
I don't mistake?
+ else
+ offset = urb->actual_length;
+
+ /* We abort() so dma->actual_len gets updated */
+ musb->dma_controller->channel_abort(dma);
Why? Who needs dma->actual_len now?..
+
+ /* always use mode1 for now */
+ if (!dma_controller->channel_program(
+ dma, qtd->maxpacket,
+ true,
+ urb->transfer_dma + offset,
+ qtd->segsize)) {
+ goto ret;
+ }
+
+ if (is_in) {
+ csr = musb_readw(hw_ep->regs, MUSB_RXCSR);
+ csr |= MUSB_RXCSR_H_WZC_BITS;
+
+ csr |= MUSB_RXCSR_H_AUTOREQ;
+ musb_writew(hw_ep->regs, MUSB_RXCSR, csr);
+ if (qtd->hb_mult == 1) {
+ csr |= MUSB_RXCSR_AUTOCLEAR;
+ musb_writew(hw_ep->regs, MUSB_RXCSR, csr);
+ }
You could avoid doing these bit manipulations for DaVinci's as they
don't implement either AutoReq or AutoClear bits anyway. The same
behaviour is achieved within CPPI DMA with help of its internal registers.
+ } else {
+ csr = musb_readw(hw_ep->regs, MUSB_TXCSR);
+ csr |= MUSB_TXCSR_H_WZC_BITS;
+ csr |= MUSB_TXCSR_DMAENAB | MUSB_TXCSR_DMAMODE;
+ if (qtd->hb_mult == 1)
+ csr |= MUSB_TXCSR_AUTOSET;
Same comment about the AutoSet bit -- it doesn't exist on DaVincis.
+ csr |= MUSB_TXCSR_MODE;
+ musb_writew(hw_ep->regs, MUSB_TXCSR, csr);
+ csr = musb_readw(hw_ep->regs, MUSB_TXCSR);
+ }
+ DBG(3, "%s%d dma started, len %d, csr %04x\n",
+ is_in ? "RX" : "TX", hw_ep->epnum, qtd->segsize, csr);
+
+ return 0;
+small_transfer:
+ DBG(3, "small transfer (%d bytes), using pio\n",
+ (urb->transfer_flags & URB_SHORT_NOT_OK)
+ ? len
+ : musb_readw(hw_ep->regs, MUSB_RXCOUNT));
+ret:
You'd better call it 'error' since it's the error cleanup path.
+ dma_controller->channel_release(dma);
+ hw_ep->rx_channel = NULL;
+ hw_ep->tx_channel = NULL;
+ dma = NULL;
+ return -1;
+}
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
