Hello.
Felipe Balbi wrote:
From: Arnaud Mandy <ext-arnaud.2.mandy@xxxxxxxxx>
new start and stop generic dma functions.
Signed-off-by: Arnaud Mandy <ext-arnaud.2.mandy@xxxxxxxxx>
Signed-off-by: Felipe Balbi <felipe.balbi@xxxxxxxxx>
[...]
diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
index 7e85325..45cd522 100644
--- a/drivers/usb/musb/musb_gadget.c
+++ b/drivers/usb/musb/musb_gadget.c
@@ -131,7 +131,185 @@ __acquires(ep->musb->lock)
ep->busy = busy;
}
-/* ----------------------------------------------------------------------- */
+/**
+ * start_dma - starts dma for a transfer
+ * @musb: musb controller pointer
+ * @epnum: endpoint number to kick dma
This parameter doesn't exist -- why it is documented then?
+ * @req: musb request to be received
+ *
+ * Context: controller locked, IRQs blocked, endpoint selected
+ */
+static int start_dma(struct musb *musb, struct musb_request *req)
+{
+ struct musb_ep *musb_ep = req->ep;
+ struct dma_controller *cntr = musb->dma_controller;
+ struct musb_hw_ep *hw_ep = musb_ep->hw_ep;
+ struct dma_channel *dma;
+ void __iomem *epio;
+ size_t transfer_size;
+ int packet_sz;
+ u16 csr;
+
+ if (!musb->use_dma || musb->dma_controller == NULL)
+ return -1;
+
+ if (musb_ep->type == USB_ENDPOINT_XFER_INT) {
+ DBG(5, "not allocating dma for interrupt endpoint\n");
+ return -1;
+ }
+
+ if (((unsigned long) req->request.buf) & 0x01) {
Parens around (unsigned long) req->request.buf) not needed.
+ DBG(5, "unaligned buffer %p for %s\n", req->request.buf,
+ musb_ep->name);
What's wrong with unaligned buffers?
+ return -1;
+ }
+
+ packet_sz = musb_ep->packet_sz;
+ transfer_size = req->request.length;
+
+ if (transfer_size < packet_sz ||
+ (transfer_size == packet_sz && packet_sz < 512)) {
Why not transfer say 500-byte packet via DMA also? I'd suggest using
some fixed and smaller PIO threshold here, like 64 bytes:
+ if (transfer_size <= packet_sz && transfer_size <= 64) {
But that logic is only good for the sane DMA controllers which can
coalesce packets. TUSB seems incapable of this, and I don't even know what
good this driver is really for...
+ DBG(4, "small transfer, using pio\n");
+ return -1;
+ }
+
+ epio = musb->endpoints[musb_ep->current_epnum].regs;
+ if (!musb_ep->is_in) {
+ csr = musb_readw(epio, MUSB_RXCSR);
+
+ /* If RXPKTRDY we might have something already waiting
+ * in the fifo. If that something is less than packet_sz
It's FIFO...
+ * it means we only have a short packet waiting in the fifo
+ * so we unload it with pio.
... and PIO. Yes, I hate lovercased acronyms. :-)
+ */
+ if (csr & MUSB_RXCSR_RXPKTRDY) {
+ u16 count;
+
+ count = musb_readw(epio, MUSB_RXCOUNT);
+ if (count < packet_sz) {
+ DBG(4, "small packet in FIFO (%d bytes), "
+ "using PIO\n", count);
+ return -1;
+ }
+ }
+ }
+
+ dma = cntr->channel_alloc(cntr, hw_ep, musb_ep->is_in);
+ if (dma == NULL) {
+ DBG(4, "unable to allocate dma channel for %s\n",
+ musb_ep->name);
+ return -1;
+ }
+
+ if (transfer_size > dma->max_len)
+ transfer_size = dma->max_len;
Not all DMA drivers currently set the 'max_len'... E.g. this will not
work with CPPI driver unless you modify it to set the proper 'max_len'.
Second. If the transfer doesn't fit into 'max_len', what you're going to
do? Your code seems to have no provision for restarting DMA from byte N.
Yes, I know, the current Mentor DMA code sucks at that too... but it seems
about right time to at last solve this limitation.
+ if (musb_ep->is_in) {
+ csr = musb_readw(epio, MUSB_TXCSR);
+ csr |= MUSB_TXCSR_DMAENAB | MUSB_TXCSR_DMAMODE;
+ csr |= MUSB_TXCSR_AUTOSET | MUSB_TXCSR_MODE;
Could be a single statement...
+ csr &= ~MUSB_TXCSR_P_UNDERRUN;
+ musb_writew(epio, MUSB_TXCSR, csr);
+ } else {
+ /* We only use mode1 dma and assume we never know the size of
DMA.
+ * the data we're receiving. For anything else, we're gonna use
+ * pio.
PIO.
+ */
+
+ /* this special sequence is necessary to get DMAReq to
+ * activate
+ */
+ csr = musb_readw(epio, MUSB_RXCSR);
+ csr |= MUSB_RXCSR_AUTOCLEAR;
+ musb_writew(epio, MUSB_RXCSR, csr);
+
+ csr |= MUSB_RXCSR_DMAENAB;
+ musb_writew(epio, MUSB_RXCSR, csr);
+
+ csr |= MUSB_RXCSR_DMAMODE;
+ musb_writew(epio, MUSB_RXCSR, csr);
+ musb_writew(epio, MUSB_RXCSR, csr);
Hmm, why twice?
+
+ csr = musb_readw(epio, MUSB_RXCSR);
+ }
+
+ musb_ep->dma = dma;
+
+ (void) cntr->channel_program(dma, packet_sz, true, req->request.dma,
Hey, for the Rx case !request->short_not_ok was passed here which was
used by the CPPI driver for its internal heurisitic. Also, for Tx case with
TUSB controller request->zero was passed. I'm not at all sure that always
passing true is correct...
+ transfer_size);
+
+ DBG(4, "%s dma started (addr 0x%08x, len %u, CSR %04x)\n",
+ musb_ep->name, req->request.dma, transfer_size, csr);
+
+ return 0;
+}
+
+/**
+ * stop_dma - stops a dma transfer and unmaps a buffer
+ * @musb: the musb controller pointer
+ * @ep: the enpoint being used
+ * @req: the request to stop
+ */
+
+static void stop_dma(struct musb *musb, struct musb_ep *ep,
+ struct musb_request *req)
+{
+ void __iomem *epio;
+
+ DBG(4, "%s dma stopped (addr 0x%08x, len %d)\n", ep->name,
+ req->request.dma, req->request.actual);
+
+ if (req->mapped) {
+ dma_unmap_single(musb->controller, req->request.dma,
+ req->request.actual, req->tx ?
+ DMA_TO_DEVICE : DMA_FROM_DEVICE);
+ req->request.dma = DMA_ADDR_INVALID;
+ req->mapped = 0;
+ } else {
+ dma_sync_single_for_cpu(musb->controller, req->request.dma,
+ req->request.actual, req->tx ?
+ DMA_TO_DEVICE : DMA_FROM_DEVICE);
+ }
+
+ epio = musb->endpoints[ep->current_epnum].regs;
+ if (req->tx) {
+ u16 csr;
+
+ csr = musb_readw(epio, MUSB_TXCSR);
+ csr &= ~(MUSB_TXCSR_DMAENAB | MUSB_TXCSR_AUTOSET);
+ musb_writew(epio, MUSB_TXCSR, csr | MUSB_TXCSR_P_WZC_BITS);
+ csr &= ~MUSB_TXCSR_DMAMODE;
+ musb_writew(epio, MUSB_TXCSR, csr | MUSB_TXCSR_P_WZC_BITS);
I think we can get away without clearing TXCSR.DMAReqMode at all...
+ } else {
+ u16 csr;
+
+ csr = musb_readw(epio, MUSB_RXCSR);
+ csr &= ~(MUSB_RXCSR_DMAENAB | MUSB_RXCSR_AUTOCLEAR);
+ musb_writew(epio, MUSB_RXCSR, csr | MUSB_RXCSR_P_WZC_BITS);
+ csr &= ~MUSB_RXCSR_DMAMODE;
+ musb_writew(epio, MUSB_RXCSR, csr | MUSB_RXCSR_P_WZC_BITS);
The docs don't have anything specific about clearing RXCSR.DMAReqMode
before or in the same write as RXCSR.DMAReqEnab (unlike their TXCSR
counterparts).
+ }
+
+ musb->dma_controller->channel_release(ep->dma);
+ ep->dma = NULL;
+}
Introducing functions without callers? That's... interesting. :-)
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
